| Project: | Project issue tracking |
| Version: | 6.x-1.x-dev |
| Component: | User interface |
| Category: | bug report |
| Priority: | normal |
| Assigned: | mr.baileys |
| Status: | needs work |
| Issue tags: | 6.x-1.0 wishlist |
Issue Summary
right now, if project admins go to the edit tab on their project nodes, click on the issues subtab, and modify any existing components, the DB is left in a somewhat lame state where all issues and follow ups that ever used the old name still use the old name, and when you try to follow up again, it's flagged as a validation error and you have to select a new component manually.
instead of a text area that you can edit freely, there should be a table of the current components, with "rename" and "delete" operations, and a text field to add a new one. if you click "rename", it should bring you to a page where you enter the new name, and if you press "submit", it will go through and repair the DB to fix all existing issues and followups for the project that referenced the old name, in addition to fixing the record in the {project_issue_projects} table.
Comments
#1
subscribe
#2
#3
@hunmonk: i know you're trying to clear out the issue queue, but i'm not convinced that just marking things you don't want to work on as "postponed" is the most helpful approach. ;) for example, there are a handful of drupal core components on d.o we'd like to rename, if we had this working. sure, it's not top priority, but that's why this isn't marked "critical". in my mind, "postponed" means it's basically blocked on some other specific issue or event before we can make progress on it. in this case, it's just a question of getting around to it -- it's not blocked on anything except having time to work on it.
#4
@dww: c'mon, dude -- you know me better than that. :)
imo there was a perfectly valid reason for marking this postponed (along with many other issues) -- i judged it as something that was best fixed on the next major branch. that's a perfectly valid reason for postponing an issue, and it makes the issue queue easier to manage for the issues that are of current concern.
now that project issue has been branched for DRUPAL-5, it makes sense to me to mark this as active again.
note: imo, one of the reasons the postponed status is so rarely used is because currently it's a pain in the ass to mass edit issue states. if i wasn't, i think we'd use this kind of temporary classification a lot more.
#5
Maybe something for the upcoming D6 release? Attached is a patch (and 2 screenshots of the UI after the change) that makes the following changes:
Renaming and deleting components change every single reference to that component:
No changes were made to the database or to the way components are stored, so this change should be transparent outside of the admin UI.
Patch is working, but still needs validation, refinement, and it has only been tested with small numbers of issues/comments for the time being. I wanted to check in to get some input before putting more work in.
I'm unsure about the paths for the new component pages: I was going to put these under the "project/xxx"-paths, but as I understand these are now "legacy"-paths?
#6
Adding 6.x-1.0 wishlist tag
#7
Patch needs reroll:
apfel-ethernet /drupal/project6/drupal/sites/all/modules/project_issue: patch -p0 --dry-run < project_issue_components.patchpatching file issue.inc
Hunk #1 FAILED at 14.
Hunk #2 FAILED at 33.
Hunk #3 succeeded at 1210 with fuzz 2 (offset -38 lines).
2 out of 3 hunks FAILED -- saving rejects to file issue.inc.rej
patching file project_issue.module
Hunk #1 succeeded at 120 with fuzz 1 (offset 18 lines).
Hunk #2 FAILED at 722.
Hunk #3 FAILED at 844.
2 out of 3 hunks FAILED -- saving rejects to file project_issue.module.rej
The next patch would create the file includes/batch.inc,
which already exists! Assume -R? [n] y
patching file includes/batch.inc
#8
#9
This shouldn't be rerolled until #423304: Add a per-project default component setting to simplify the UI when submitting issues lands... That patch actually implements a fair bit of the changes this patch is trying to accomplish.
#10
#423304: Add a per-project default component setting to simplify the UI when submitting issues is now in, this can go forward again. The patch is going to need a pretty major reroll, but it'll hopefully result in a much smaller patch, since a lot of what this patch was doing is already done...
#11
@mr.baileys: Any chance you want to take a stab at re-rolling this?
#12
I did a major re-roll of the patch in #5 (rename logic is the same, but the UI has been removed from the patch since this was addressed in #423304: Add a per-project default component setting to simplify the UI when submitting issues, and the delete logic has been removed since this was added via the aforementioned issue)
I don't have CVS access from here, so I will upload the patch later today.
#13
Patch attached.
As I haven't gotten issue generation to work for 6.x-1.x-dev yet, I've only been able to test this patch with small numbers of projects/issues/components.
Also, my project*-knowledge is a bit rusty, so go easy on me :)
#14
Cool, thanks! Sorry I didn't review this sooner. Didn't actually look at the patch yet, just tried it out. A few notes based on using it:
A) I was not expecting the ability to just rename them all right there, but was imagining a "rename" link in the operations table with a confirm form (like we have for 'node/%project_node/edit/component/delete/%'). That would also ensure you're only renaming one at a time, which might be a feature or a bug. ;) From a brief skim of the batch code, it seems like there's no efficiency gain from renaming multiple components at the same time.
B) While the batch stuff worked to update issues and followups, the actual table reloads without all the changes you made. ;) I started with the 4 defaults. I renamed "Code" to "Source code" and "User interface" to "User interfaces". After the batch ran (which did the right things), the available components on my project were "Code", "Documentation", "Miscellaneous", and "User interfaces". ;) So, one change "took" but the other was lost.
I might take a quick stab at re-working this right now, to see if it's reasonable to get it out on d.o in the downtime that's scheduled in an hour. ;)
#15
Looks like I'm not going to have a chance to work on this, if anyone else wants to re-roll...
Also, it occurs to me that there might be cases when you rename components where you want to go through all the historical followups and rename those, too, and cases where you just want to rename the values available when people followup from here on out (and change any existing issues currently using that value). Since we just store the component as a string, you can have followups that use a component that's no longer considered valid... So, that'd be another nice thing that the confirm form could do -- it could just give people the option of only changing the current values, or retroactively renaming all values (via the batch stuff).
Thoughts?
#16
Somehow, I really start to question why PI doesn't simply use internal ids for the components. And. When considering that, then I start to question, why it's not simply re-using the taxonomy system (with a custom admin UI)...
At least, doing that would eliminate this entire issue (and probably more).
+++ includes/project_edit_issues.inc 27 Jul 2009 06:43:32 -0000@@ -226,6 +226,10 @@
function project_issue_project_edit_form_submit($form, &$form_state) {
@@ -287,4 +311,38 @@
+function project_issue_batch_rename_component_finished($success, $results, $operations) {
...
+ drupal_goto("node/$pid/edit/issues");
Instead of breaking the batch, this should be a $form_state['redirect'] in the form submit handler. Batch API takes over that value.
This review is powered by Dreditor.