#2618 closed defect (duplicate)
ticket mods are lost on submit if ticket modified by someone else
Reported by: | Owned by: | Christian Boos | |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | ticket system | Version: | 0.9 |
Severity: | major | Keywords: | conflict |
Cc: | bboisvert@…, mankoff@… | Branch: | |
Release Notes: | |||
API Changes: | |||
Internal Changes: |
Description
When you submit a modification to a ticket, Trac correctly disallows it if an update has happened since the time you opened the ticket page (because you aren't aware of the most current information). However, instead of throwing an uncaught error and abandoning the modification, it should treat the submission as a 'preview' request, rather than a real submission, rerender the page (with the now-current mods) and a message as to what it did, and allow the user to edit their message if needed, before resubmitting.
Attachments (7)
Change History (27)
comment:1 by , 19 years ago
comment:2 by , 19 years ago
At the very least, how about displaying back to the user a summary of the requested (but rejected) changes, with instructions about what to do.
I wouldn't care except that around here when we encounter this scenario, you hit the back button, and the form has reverted back to the original state - you've lost all your work.
comment:3 by , 19 years ago
Cc: | added |
---|
I'd say that it should use the committed data to populate the fields. It's no big deal to reenter you CC or keywords, but it's a big deal to have to reenter a longish comment on a ticket. Perhaps have a "your value" displayed above the mutable fields, if different, so you can easily get back what you entered.
More concisely: do the preview using the data from the DB where there's a conflict, and optionally [hopefully] display the just-submitted data above/below the field where it's changed.
comment:4 by , 19 years ago
Cc: | added |
---|---|
Severity: | normal → major |
This is a severe bug for Safari users, where the back button doesn't work for some reason, so all edits are 100% lost. :(
comment:5 by , 19 years ago
Any word on this? It's becoming more and more of a problem for us, as we expand.
by , 19 years ago
Attachment: | source_patch.txt added |
---|
patch for installed 0.9.5 source to add submitted fields to the HDF on a "mid air collision" error
by , 19 years ago
Attachment: | templates_patch.txt added |
---|
patch for installed 0.9.5 templates to display the submitted fields on a "mid air collision" error
comment:6 by , 19 years ago
I've implemented a very hackish solution that will simply rerender the submitted fields as a raw HDF dump on the "mid air collision" error page. There are two patches, one for the installed source (/usr/lib/python2.x/site-packages/trac), and one for the installed templates (/usr/share/trac). Note that if you're running the compiled bytecode (rather than from source), you'll need to recompile after applying the patch.
This patch is far from a good solution, as it is ugly, unusable, and leverages the exception handling subsystem in a rather poor way. But it prevents the loss of data.
by , 19 years ago
Attachment: | templates_patch.2.txt added |
---|
patch for installed 0.9.5 templates to display the submitted fields on a "mid air collision" error - formatted as an HTML table, rather than an HDF dump
comment:7 by , 19 years ago
Unfortunately, after upgrading to the trunk (i.e 0.10dev) these patches don't seem to work anymore, as the request dispatching stuff is quite a bit changed.
by , 19 years ago
Attachment: | 0.10dev-error.cs-patch.2.txt added |
---|
Patch for 0.10dev to display page parameters on all error messages
comment:8 by , 19 years ago
The patch for 0.10dev is of rather a different style. Rather than the kludgy hackery that was in the previous patches, this time the patch is only for templates/error.cs
, and causes the page parameters to be displayed on all error pages. Far simpler, no dependance on internal APIs and error handling mechanisms, and it's not constrained to just the mid air collision error. In particular, forgetting a summary on a new ticket.
comment:9 by , 19 years ago
Milestone: | → 0.10 |
---|---|
Owner: | changed from | to
I was thinking about doing something like this myself. What I had in mind was to add at the end of the error message a Retry button that would simply resubmit all the changed fields in order to get a new preview: in that preview, you could see what changed and decide to keep or adapt your own changes (i.e. a kind of manual merge).
by , 19 years ago
Attachment: | mid_air_collision_retry-r3341.patch added |
---|
Implements a Retry button on the "Mid Air Collision" error page.
comment:10 by , 19 years ago
Status: | new → assigned |
---|
See proof-of-concept implementation in attachment:mid_air_collision_retry-r3341.patch
comment:11 by , 19 years ago
Shouldn't the ClearSilver template be ticket_error_midair.cs, not ticket_error.cs? There are other ticket errors (such as forgetting the summary on a new ticket) that aren't handled by it, though it's probably desirable if they all are. You don't need the merge stuff for new tickets, but not having to manually copy and paste your ticket data back in would be nice.
One problem with it, however, is that the preview gives you a preview only of YOUR data, so if the collision was for anything except the addition of a comment/attachement, there's no way to view what the updated value is. E.g. if someone updates 'keywords', then you submit and get a mid-air, when you hit 'retry', it's going to show your keywords, with no indication of what the current commited keyword's value is (which you've never seen).
What I'm thinking is maybe on the error page, it can display the committed values and the submitted values in two columns, and you can run down the list and pick which to keep. Then when you hit 'retry' it submits the collection of values you picked. You get your ability to manually merge, and without much more fuss. Just need to load the committed data into the HDF for the template to use, and throw in a little JavaScript magic for dynamically valued form fields based on which side of the merge is requested for each field.
by , 19 years ago
Attachment: | mid_air_collision_retry-bboisvert.patch added |
---|
Implements a Retry button with a manual merge form on Mid-Air collision errros. Is an EXTENSION of attachment:mid_air_collision_retry-r3341.patch
comment:12 by , 19 years ago
cboos, I extended your patch to display a manual merge form above the retry button, if there were any fields with discrepancies in the ticket data (i.e. not the comment). It's not particularly attractive, but it's functional. It's a patch for a slightly modified source:/trunk#3335, though I don't think any of my custom mods interfere at all. They're not included in the patch in any case.
Note also that I named my template ticket_error_midair.cs.
by , 19 years ago
Attachment: | mid_air_collision_retry-bboisvert2.patch added |
---|
futher tweaks to attachment:mid_air_collision_retry-bboisvert.patch to deal with some weird timestamp resolution conflicts and improve formatting
comment:13 by , 19 years ago
This second patch is an immediate successor to my first page that deals with a weird timestamp issue that causes mid-air collisions repeatedly. There's probably a better way to handle it, but I don't know enough about Trac's guts to know what it is.
I also cleaned up the merge form to render nothing if there aren't any conflicts, rather than rendering the headers with no fields.
comment:14 by , 19 years ago
That last comment was by me, of course. Forgot to enter my email address before I submitted.
comment:15 by , 19 years ago
Thanks for your patches, I'll have a look at them. But before that, I wanted to mention that in my patch, the preview did show the other changes: it's simply the last comment added, which as usual contains a summary of the field changes.
comment:16 by , 19 years ago
You are correct, of course, excepting the description. Description changes are not recorded as comments (nor versioned anywhere else, except for notification emails), so dealing with collisions on the description requires some "extra" mechanism somewhere.
comment:17 by , 18 years ago
This issue just blew 1.5 hours of my life away ≥(
With or without a merge function, it would help to display the item's preview so that someone could collect their text, update if necessary, and re-submit.
comment:18 by , 18 years ago
I'm sorry, too many things changed lately in the ticket web_ui.py, so the patch(es) don't apply cleanly anymore. Could you help out and make an updated patch on top of a recent trunk (e.g. r3477)?
comment:19 by , 18 years ago
Keywords: | conflict added |
---|---|
Milestone: | 0.10 → 0.11 |
Post-poning to 0.11 (and probably after the WorkFlow merge)
comment:20 by , 18 years ago
Milestone: | 0.11 |
---|---|
Resolution: | → duplicate |
Status: | assigned → closed |
I think the approach suggested in #4100 should be preferred, so I'd like to move the discussion for this topic there.
I'm not sure to understand how this would actually work:
Imagine that the user who receives the middle-air collision warning has changed the CC field while another user has updated the same ticket with a different CC field content: what would be the content of the "preview" page with such an event ?
The trouble is that the ticket changes are not only about the "description" field which is added to the current ticket contents, but other fields that are overwritten.