Remove hard-coded status options

Yep, good idea, I'll work on a patch.

I've committed a temporary hack--adding the new patch ids to the default lists. But the real issue is that, when I prepared the patch making issue status options configurable, I overlooked the fact that the status ids were extensively hard-coded in various places in the module. To complete the status configurability, we need to deal with this issue. Hard-coded status ids currently fill a number of needs.

• function project_cron() selects issues with sid = 2 (fixed) and sets them to sid = 7 (closed).
• Project module line 124, selects sid=2 (fixed) issues.
• Project module lines 761 and 762, selects sid=1 or sid=2 (active or fixed). I can't immediately tell what this is for.
• Various places where issues with sid =1, 2, or 8 (active, fixed, or patch). This is what I've now updated to include sid = 13 or 14 , patch (needs work) and patch (ready to commit).

And various others.

The problem is of course that changing or deleting the status options, now possible through the issue status admin page, will lead to unexpected results.

The most obvious option is to pull the various needed status id values into configuration options. We already have one such: the default status to use for a new issue. There could be another setting for "default list status", with a default value of array(1, 2, 8, 13, 14), that we could use where the current hard-coded array is. The downside is that we'd need at least four or five new configuration options--and some of them will be a bit confusing. For example, the cron one. "Pick the status that the cron event should convert from, and the one it should convert to."

Thoughts? Should I go ahead and pull out these hard-coded status ids into configuration options?

They have to be pulled out into configuration. It is complex, but can ship with "sane" defaults.

bump...

this is driving me crazy. the incessant chorus to add "patch (to be ported)" to the default queries must be silenced, but the last thing i want is to further hard-code d.o-specific status values directly into issue.inc, or to maintain a locally modified issue.inc on d.o. :(

my current proposal:

1) add a new column of checkboxes to the admin/project/project-issue-status page called "In default queries" and rename the existing "Default" to "Default status" (better name for "In default queries" welcome).

2) instead of storing this info in a new column in the {project_issue_state} table and messing with a DB update to alter the schema and an additional db_query()/SELECT every time we need it, let's just store the array as a setting in the {variable} table (at least for now).

3) add a define() for a new, even more sane *default* list for brand new sites (i.e., *just* 'active', 'fixed', and maybe 'postponed'), and use that as the arg for variable_get() in all the places we call it.

4) fix all the places in issue.inc and project_issue.module that hard-code their own list, and use variable_get() instead.

for now, let's punt on these other issues, and fix those after the above proposal is done:
- hook_cron() [project_issue.module]
- project_mail_reminder() (hardcodes sid==1 or 2) [mail.inc]
- project_mail_digest() (hardcodes sid==1) [mail.inc]
- project_issue_statistics (hardcodes sid>1) [issue.inc]

see also http://drupal.org/node/112909, which i just submitted about the fact that we're not always using the default state.

anyway, i might just crank this out, b/c it's driving me nuts. ;) however, i *really* shouldn't be spending time on this right now, so i might have to unassign myself in a little while...

turned out it was easier (and probably better in the long run) to just store this use-for-default-queries property directly in the {project_issue_state} table, even though that required a DB schema update.

here's the patch. tested heavily on my local site. seems to behave splendidly. ;) i'll probably install it on s.d.o and a) make sure the DB upgrade works fine there, and b) see how nicely it works.

however, if anyone (nedjo?) wants to review, that'd be swell. ;)

thanks,
-derek

now installed on s.d.o. behold:
http://scratch.drupal.org/project/issues

screenshot of the new admin UI attached.

i got chx to review this in IRC, and he was happy. committed to HEAD, installed on d.o. update.php ran. all patch-related states are now in the default query.

setting back to active for the other remain stuff in here (cron, etc).

too big for now, let's wait until the next major branch.

Moving to the right queue...

I'm having that issue in 6.x (see http://drupal.org/node/583404) and I wonder if I need to use the patch from #5 or if this was meanwhile committed to HEAD of this project.

@jurgenhaas: please read comment #7.

@dww Sorry, I may not see the forrest for all the trees. I'm using the latest dev version of project_issue and still have the hard coded status options in project_issue_project_page_link_alter, so what do I need to do to get rid of them?

I do appologize if I'm missing the obvious here.

@jurgenhaas: To quote myself from comment #7:

setting back to active for the other remain stuff in here (cron, etc).

This is still not done, which is why this issue is still open. ;)

In your specific case, project_issue_project_page_link_alter() is an implementation of hook_project_page_link_alter(), which you're free to implement yourself and alter what project_issue is doing....

Hi,
I propos something like this for the hard coded statuses in the hook_project_page_link_alter implementation project_issue_project_page_link_alter() in project_issue.module:

function project_issue_project_page_link_alter(&$links, $node) {
  // TODO: Assumes "patch" issue status values (http://drupal.org/node/27865).
  // Prepend a link to the development section to view pending patches.
  if (!empty($node->project['uri'])) {
    $statusStr = 'status[]=' . implode('&status[]=', array_map('urlencode', project_issue_default_states()));
    $patches['pending_patches'] = l(t('View pending patches'), 'project/issues/search/'. $node->project['uri'], array('query' => $statusStr));
    $links['development']['links'] = $patches + $links['development']['links'];
  }
}

It just uses the project_issue_default_states() values for the "View pending patches" link in setad of 'status[]=8&status[]=13&status[]=14'. Works for us.

There is no check for empty array because administration won't allow none selected for the "In default queries" column.

Subscribe. Was trying to think of a good way to do this for one of my sites, but hit a mental wall.