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.
Comment | File | Size | Author |
---|---|---|---|
#20 | status_admin_page2.png | 20.66 KB | nedjo |
#19 | project-admin-configurable-status-options_2.patch | 36.25 KB | nedjo |
#18 | confirm_delete.png | 4.03 KB | nedjo |
#17 | project-admin-configurable-status-options.patch | 32.77 KB | nedjo |
#8 | status_admin_page.png | 13.61 KB | nedjo |
Comments
Comment #1
nedjoAttached 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:
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).
Comment #2
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedWouldn'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".
Comment #3
Bèr Kessels CreditAttribution: Bèr Kessels commentedI 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.
Comment #4
moshe weitzman CreditAttribution: moshe weitzman commentedsounds real useful ... i like 'ready to commit' instead of 'queued for application'.
Comment #5
TDobes CreditAttribution: TDobes commented+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.
Comment #6
Dries CreditAttribution: Dries commentedI'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.
Comment #7
(not verified) CreditAttribution: commentedHere 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.
Comment #8
nedjo[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?"
Comment #9
tangent CreditAttribution: tangent commentedIn 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
Resolution types
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
Comment #10
tangent CreditAttribution: tangent commentedAbove, I meant "when a 'final' patch has been submitted then the issue can be set to 'resolved'." Sorry for the noise.
Comment #11
Bèr Kessels CreditAttribution: Bèr Kessels commentedtangent: 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.
Comment #12
nedjoI 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.
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?
Comment #13
nedjoSince 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:
This patch addresses the first three of these issues. It:
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
Comment #14
tangent CreditAttribution: tangent commentedDid you forget to attach the patch?
Comment #15
nedjoSorry, I was adding further explanation of what I'd areadly done, what I meant was "the patch I submitted above" (update #7).
Comment #16
nedjoFrom drupal-dev (Dries):
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?
Comment #17
nedjoHere'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 inproject_issues
tosid
, matching the name of the linked field in the newproject_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....Comment #18
nedjoAnd a screenshot showing the new confirmation page for project issue status deletion.
Comment #19
nedjoThanks 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.
Comment #20
nedjoHere 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.
Comment #21
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedNedjo, why is the "status options" screen at project/issues/status_settings? Doesn't make too much sense to me.
Comment #22
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedI'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?
Comment #23
nedjoThanks 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.
Comment #24
Dries CreditAttribution: Dries commentedDid 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.
Comment #25
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedI'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.
Comment #26
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedCommitted, thanks Nedjo!
Comment #27
nedjoAnd 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?
Comment #28
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedI've made update files for both pgsql and mysql and explained their use in the readme.
Comment #29
(not verified) CreditAttribution: commentedComment #30
(not verified) CreditAttribution: commentedComment #31
(not verified) CreditAttribution: commentedComment #32
(not verified) CreditAttribution: commentedComment #33
(not verified) CreditAttribution: commentedComment #34
(not verified) CreditAttribution: commentedComment #35
(not verified) CreditAttribution: commented