Remove hard-coded status options

Robrecht Jacques - July 30, 2005 - 19:58
Project:Project issue tracking
Version:6.x-1.x-dev
Component:Issues
Category:bug report
Priority:normal
Assigned:dww
Status:active
Description

I like the new "patch (code needs review)", "patch (code needs work)" and "patch (ready to be commited)", but when you now open a projects issue list on drupal.org, not all patches are shown anymore.

The default list of issues to show is: "active, fixed, patch (code needs review)". So when someone sets the status to "patch (ready to be commited)" the patch "disappear" from the default "Issues" page of the project.

I propose to put the three "patch (*)" in the default list.

#1

nedjo - September 20, 2005 - 22:42
Assigned to:Anonymous» nedjo

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

#2

nedjo - September 29, 2005 - 03:49
Title:new status not shown in default list» Remove hard-coded status options

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?

#3

Boris Mann - September 29, 2005 - 03:54

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

#4

dww - January 26, 2007 - 04:27
Version:x.y.z» 5.x-1.x-dev
Assigned to:nedjo» dww

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...

#5

dww - January 26, 2007 - 07:25
Status:active» needs review

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

AttachmentSize
project_issue_default_query_states_UI.patch.txt 12.94 KB

#6

dww - January 26, 2007 - 07:36

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

screenshot of the new admin UI attached.

AttachmentSize
project_issue_default_query_states_UI.png 205.8 KB

#7

dww - January 26, 2007 - 08:11
Status:needs review» active

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).

#8

hunmonk - September 29, 2007 - 13:24
Version:5.x-1.x-dev» x.y.z
Status:active» postponed

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

#9

dww - January 29, 2008 - 03:30
Project:Project» Project issue tracking
Version:x.y.z» 5.x-2.x-dev

Moving to the right queue...

#10

jurgenhaas - October 13, 2009 - 11:01

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.

#11

dww - October 13, 2009 - 19:39
Version:5.x-2.x-dev» 6.x-1.x-dev

@jurgenhaas: please read comment #7.

#12

jurgenhaas - October 14, 2009 - 05:32

@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.

#13

dww - October 14, 2009 - 06:21
Status:postponed» active

@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....

 
 

Drupal is a registered trademark of Dries Buytaert.