There are currently 2 permissions available which control access to issues, as far as I can determine.

1. access project issues
allows the user to view issues
2. create project issues
allows the user to create new issues
allows the user to comment on an existing issue
allows the user to change any and all fields (e.g., status, assignment, project, priority)

Currently, it seems that anonymous users are granted both permissions on Drupal.org which effectively renders them useless. While it may be reasonable to allow anonymous visitors to create new issues, it is counterproductive and confusing to allow anonymous users to change the status or other fields of an issue. It may even be desirable to restrict modification of certain fields to specific roles (.e.g., priority, status) so that issues may be "moderated".

I propose the following:
1. Create a new permission, possibly "edit project issues", which controls the ability to change issue information fields or the issue title.
2. Make use of the "post comments" and/or "post comments without approval" permissions to control who can add comments to an issue instead of using the "create project issues" permission as is currently done.

Following these enhancements, I believe that the "post comments" permission should be restricted to authenticated users on Drupal.org to reduce the number of accidental anonymous comments. The "edit project issues" permission should be restricted to an "issue manager" role so that issues can follow a proper lifecycle in a controlled way.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

nedjo’s picture

Attached is a first cut at how to improve the granularity of issue status setting.

I've introduced two new status settings ("reviewed" and "queued for application") and rearranged the existing ones slightly (reordered them, and changed "fixed" to "applied"--since while bugs are fixed, feature requests are not).

The new system looks like this:

$options = array(1 => t('active'), 8 => t('patch'), 9 => t('reviewed'), 10 => t('queued for application'), 2 => t('applied'), 3 => t('duplicate'), 4 => t('postponed'), 5 => t("won't fix"), 6 => t('by design'), 7 => t('closed'));

A status of "reviewed" indicates that a qualified developer has reviewed the patch and found it to work as claimed, without raising any obvious issues. A status of "queued for application" means that a qualfied developer deems the patch to meet Drupal approval criteria.

I've also created new permission settings so that each status has its own permission. This will allow finely grained control over who can bump up the priority of an issue. For example, a large group of developers could be given "reviewed" setting permission, and a more refined group given "queued for application" permission.

Together, the new status settings and fine-grained setting permissions are intended to enable Drupal - or other projects using this module - to improve community participation and input in reviewing and approving proposed changes.

This patch follows on this drupal-devel discussion.

I haven't tested the patch as I haven't (yet) set up a Drupal CVS install (and project.module CVS doesn't work with Drupal 4.5).

killes@www.drop.org’s picture

Wouldn't it make more sense to make the status an admin configurable field? I recall that people wanted to use project module for non-software projects and didn't like the current approach.

Oh, and I have a long standing feature request from #drupal: We need the status "user on crack".

Bèr Kessels’s picture

I have proposed before (but thats loooong ago) to use taxonomy for this.
Pro: Free hierarchy, Free admin interface, Free relations, free configuration options.
Cons: Admins need to know taxonomy in order to define issue stati. But thats minor IMO.

moshe weitzman’s picture

sounds real useful ... i like 'ready to commit' instead of 'queued for application'.

TDobes’s picture

+1 on making it user-configurable and/or using taxonomy... it'd make the project module much more useful in other applications.

We use an older version of the project module internally for tracking status of trouble tickets with equipment, for which I have manually hacked in different status fields. Users who do not know PHP might not have this ability.

Dries’s picture

I'm much more interested in these status fields: http://lists.drupal.org/archives/drupal-devel/2005-01/msg00948.html

The difference is subtle but I'd like to believe it is easier to separate the good patches from the bad using my scheme.

Anonymous’s picture

Here is a revised patch addressing comments to date. I've made the status options admin configurable, with a default set of options designed for use on drupal.org. Descriptions to follow with screenshot.

nedjo’s picture

FileSize
13.61 KB

[That last update was me.]

The attached screenshot shows the admin interface for mass updating existing statuses (or is that "stati"?), deleting existing ones, and adding new ones (at the bottom of the form).

I've followed Moshe's suggestion for changing 'queued for application' to 'ready to commit' (it is indeed clearer).

I've also incorporated Dries' suggested options. I agree that they are useful designators. They don't however replace the ones I've added. "Needs work" and "testers needed" (Dries' suggestions) address the question "How can we designate patches as needing more work or evaluation?" "Reviewed" and "ready to commit" address the distinct question "How can we enable drupal developers to designate patches as working/meeting acceptance criteria?"

tangent’s picture

In my opinion the majority of items being used for issue status are not "status" items. For example, "By design" is not a status. Closed is a status and "by design" is the reason for the status, or the "resolution". I think if we're going to the trouble of updating issue management to this extend we should do it right and add a resolution field and move the resolution items there.

Using the Bugzilla bug lifecycle[1] as a model, we could use something like the following.

Status types

unconfirmed
default status for new issues
active
bug has been confirmed and has been assigned
resolved (currently "patch")
a patch has been submitted and requires testing
verified
the patch is confirmed to correctly resolve the issue
closed
the issue is closed and a resolution is set

Resolution types

fixed
the bug was resolved
duplicate
this issue documents a bug or request which is already documented in another issue
wontfix
this issue documents a bug or request which will not be resolved for reasons which should be documented within the issue
postponed
this issue will be addressed at a later time
invalid
the reported issue does not exist or is not applicable

The new "resolved" and "verified" status items will serve Dries needs as follows. When I "final" bug has been submitted the issue may be set to "resolved" status. This will alert everyone that the patch "needs testing" and inspection by the code owner. When the patch has been tested and verified then the issue may be set to "verified" status which will alert commiters that the patch may be applied. If a patch is not suitable and the issue cannot be moved to "verified" then the issue should be set back to "active" so that the issue assignee may fix the patch.

My intent with this issue was to address issue access in as simple a way as possible and not everything else that is being discussed here but perhaps issue management needed to be overhauled instead. The issue title should almost certainly be changed but I'll leave that for another time or person.

[1] http://www.bugzilla.org/docs/tip/html/lifecycle.html

tangent’s picture

Above, I meant "when a 'final' patch has been submitted then the issue can be set to 'resolved'." Sorry for the noise.

Bèr Kessels’s picture

tangent: I do not think your ideas aremisplaced here. I like tangents ideas, since they are:
simple
standard (bugzilla is the de facto bug standard in OSS land)
and extendible.

nedjo’s picture

I believe I somewhat misread your original issue. Following up on the drupal-devel discussion I cited, I had already begun work on my patch and was about to create an issue. Then, seeing yours, likewise focusing on issue status and permission granularity, I took it to be basically similar, possibly even also based on the recent drupal-devel discussion. Now, however, I see that what you were proposing was broader. I'm focussing exclusively on the "status" (and, as you point out "resolution") issue, with the primary aims of improving the ability of project members to participate in issue processing and decision making. A second aim, raised in the course of discussion, is to make status options customizable.

Does it make sense to continue to pursue these aims in this issue? Should I instead create a new one?

On the subject of whether to separate out "status" from "resolution", I agree that the two are logically distinct, and had looked to the Bugzilla example. But there are some issues with following the Bugzilla model.

  • Bugzilla's implicit decision-making model is distinct from Drupal's. Therefore, the workflow and steps don't perfectly match.
  • Bugzilla uses a somewhat complex set of rules to determine what changes are permissible when. Implementing these might conflict with the aim of making issue status configurable. Separating status from resolution without any rules, however, could lead to confusing results, as not all status options are appropriate for all resolutions.
  • Limited to bugs as opposed to the multiple issues addressed by the project module, Bugzilla's approach maps only partially.

My feeling at present is that it makes most sense simply to improve the existing "status" options. So long as we can do so in a logical way, such that each option is clear and mutually exclusive, I think it will work okay. That said, if someone wants to take a crack at implementing a solution pulling out resolutions from statuses, I'd be happy to look at it.

Of course, I'd appreciate direct evaluation of the patch I've produced. Does it work for people? Does it accomplish the aims?

nedjo’s picture

Since I believe that the proposed improvements to issue management have the potential to significantly benefit Drupal development, I'm eager to see this issue resolved.

Recent discussions have identified a number of weaknesses in the current status options:

  • There is no way to indicate that a given patch needs work or testing, or that it has been tested or evaluated and found sound.
  • There is no way to assign certain status-setting permissions to given user groups (functionality that is common in other issue-managing systems, like Bugzilla).
  • There is no way (short of hacking up the code) to change or adjust the status options.
  • The single field, "status", mixes up what are arguably two distinct questions: the status per se, and the "resolution".

This patch addresses the first three of these issues. It:

  • Adds four new default status indicators, based on needs identified in discussions (and renames and rearranges existing ones slightly). The revised indicators are:
    • unconfirmed
    • active
    • needs work
    • patch
    • testers needed
    • reviewed
    • ready to commit
    • applied
    • duplicate
    • postponed
    • won't fix
    • by design
    • closed
  • Makes the assignment of each status a permission, so that the ability to set certain status indicators (e.g., "ready to commit") can be assigned to a given role.
  • Adds an administrative interface for configuring status options, enabling easy customization.

I certainly see potential merit in the additional suggestion of separating out a "resolution" field (thanks, tangent, for your clear and well thought out explication), and - if there's some indication of support, particularly from the project maintainers - I would be happy to code such a solution.

In sum, as well as significantly improving the tools for Drupal development, the patch provides new flexibility for the project.module, improving its suitability for a wide range of project applications.

Please let me know if people indeed wish to take the step of separating out resolutions, or if there are other remaining problems blocking this issue's application.

Thanks, Nedjo

tangent’s picture

Did you forget to attach the patch?

nedjo’s picture

Sorry, I was adding further explanation of what I'd areadly done, what I meant was "the patch I submitted above" (update #7).

nedjo’s picture

From drupal-dev (Dries):

I think I'm OK with your patch, I just haven't had time to look at it yet or test it -- and apparently no one else had either. While the screenshot looks nice, it is unclear what happens to the existing issues when status fields are added, removed or renamed. I have been meaning to test this.

A good question, and not one I've closely considered. I was thinking of the act of changing or deleting existing status options as something one would do at the setup stage. Because the options are tracked by id, renaming them could have unintended effects (as existing issues would have that newly named status, perhaps inappropriately). Deleting statuses would have the effect of leaving existing issues with invalid status ids, generating problems.

Possible approach:

1. On status option renaming, display, e.g., "[number] of existing issues will be renamed from [old status name] to [new status name]. Are you sure you want to proceed?"

2. On status option deletion, display option, e.g., "There are [number] existing issues with the status of [status to be deleted]. Please select a new status for these issues:

[select box with remaining issue statuses]

Would this meet the need?

nedjo’s picture

Here's a revised patch addressing Dries's requests. Changes:

* help text for issue status administration explains implications of changing existing status names (and also further tips)
* when you go to delete an existing status, you get a confirmation page with a select to reassign existing issues a new status (see screen shot to come)

For consistency, I also changed the state field in project_issues to sid, matching the name of the linked field in the new project_issue_state table. This required many changes in several files. I tested to try to ensure I didn't introduce errors, but a bit more testing wouldn't hurt....

nedjo’s picture

FileSize
4.03 KB

And a screenshot showing the new confirmation page for project issue status deletion.

nedjo’s picture

Thanks to Gerhard for a detailed review of the previous version of this patch. I've made a number of changes to address his comments and suggestions and attached the revised patch.

This is a much more complex patch than what I originally had in mind with this issue, but I hope it's also a fuller solution.

Significant changes:

1. Added default status option, automatically settable by all posters.

2. Added ability to designate specific options as settable by issue author/poster. This could be used, e.g., to allow posters to close their own issues, e.g., after they are resolved.

Additional tweaks:

1. Fixed error with status validation--now tests correctly

2. Stripped ' character from status option names for use in permissions.

3. Used theme_confirm() for confirmation.

4. If only one status option is available to the user, i.e. the default one, it is presented as uneditable text (and a hidden form field) instead of a select box.

Under the new permissions system, a particular poster or updater to an issue will be able to select from status options that meet at least one of the following criteria:

* is the default option
* is an option that the user has access to
* the user is the original author of the option and the option is set to be available to authors.

If only the default option is available, no select box will appear.

nedjo’s picture

FileSize
20.66 KB

Here is a screenshot of the revised status issue admin page, with the default options displayed.

The following SQL queries should be issued to update from the previous version (in MySQL). They change the existing 'state' field to 'sid' in the project_issues table, create the new project_issue_state table, and load default data for that new table.

ALTER TABLE project_issues CHANGE state sid int(10) unsigned NOT NULL default '0';

CREATE TABLE project_issue_state (
  sid int(10) unsigned NOT NULL auto_increment,
  name varchar(32) NOT NULL default '',
  weight tinyint(2) DEFAULT '0' NOT NULL,
  author_has tinyint(2) DEFAULT '0' NOT NULL,
  PRIMARY KEY  (sid)
) TYPE=MyISAM;

INSERT INTO project_issue_state VALUES (1, 'active', -13, 0);
INSERT INTO project_issue_state VALUES (2, 'applied', 1, 0);
INSERT INTO project_issue_state VALUES (3, 'duplicate', 4, 0);
INSERT INTO project_issue_state VALUES (4, 'postponed', 6, 0);
INSERT INTO project_issue_state VALUES (5, 'won\'t fix', 9, 0);
INSERT INTO project_issue_state VALUES (6, 'by design', 11, 0);
INSERT INTO project_issue_state VALUES (7, 'closed', 13, 1);
INSERT INTO project_issue_state VALUES (8, 'patch', -8, 0);
INSERT INTO project_issue_state VALUES (9, 'needs work', -11, 0);
INSERT INTO project_issue_state VALUES (10, 'testers needed', -6, 0);
INSERT INTO project_issue_state VALUES (11, 'reviewed', -3, 0);
INSERT INTO project_issue_state VALUES (12, 'ready to commit', -1, 0);
killes@www.drop.org’s picture

Nedjo, why is the "status options" screen at project/issues/status_settings? Doesn't make too much sense to me.

killes@www.drop.org’s picture

I've moved the status_settings screen to admin/settings/project/status_settings. I've fixed some bugs, too.

The only thing I am not completely happy about is the permissions screen. project module has now (in the basic setup) close to 20 permissions. The use of single quotes also breakes the permission screen (guess that is a more general bug).

Considering that the form fields for the status names can be shortened quite a bit, I would consider not using the user_permissions setup at all, but use a project.module specific solution that could be set in the same screen. Comments?

nedjo’s picture

Thanks for the comments and improvements.

Yes, the module's list of permissions is long. While there may be something to be said for keeping permissions in one place, in practice we're implementing permissions in various places (e.g., to access particular nodes and terms), so doing so here too should be okay.

So I agree with your suggestion of including the permission handling on the issue status admin page.

We'd likely not use the same format as the regular permissions page, which takes too much space. I'd suggest a multiple select for the role names.

I'd be willing to do this, but hey, if you want to take care of it as part of your refining, killes, please do! And could you post your revised version so others can use it for testing? Thanks.

Dries’s picture

Did a quick review:

1. We don't use underscores (_) in URLs; use dash (-) instead.

2. The status settings page is somewhat confusing.

2.a. What is the radio button for? It confused me.

2.b. Why are there multiple blank lines? I would add a single blank line.

2.c. The operations column has no header.

Other than that, it looks nice, although I haven't tested it in depth.

killes@www.drop.org’s picture

I've changed the urls to be "status", there is already one "settings" in them.

The radio button is explained just a few lines above the table. I think that we might be able to do without if we agree that the status with the least weight should be the default.

I've changed the three lines to one.

Operations has now the appropriate header.

I am going to commit it with the radio button now. It doesn't really hurt and I am short in time.

killes@www.drop.org’s picture

Committed, thanks Nedjo!

nedjo’s picture

And thanks for your work fixing it up.

To upgrade, current installations of the project module will need to run SQL queries like the ones I gave above in comment #20. Should this go into a separate file? Or into the readme?

killes@www.drop.org’s picture

I've made update files for both pgsql and mysql and explained their use in the readme.

Anonymous’s picture

Anonymous’s picture

Anonymous’s picture

Anonymous’s picture

Anonymous’s picture

Anonymous’s picture

Anonymous’s picture

Status: Fixed » Closed (fixed)