ticket mods are lost on submit if ticket modified by someone else

[bboisvert@…]

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.

[eblot]

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 ?

• it cannot be the other user's CC field, as it would overwrite the current user's CC field without warning to the current user
• it cannot be the current user's CC field, as it would overwrite the other user's CC field changes
• it cannot be a merge of both field as the merge may fail in some cases

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.

[anonymous]

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.

[bboisvert@…]

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.

[mankoff@…]

This is a severe bug for Safari users, where the back button doesn't work for some reason, so all edits are 100% lost. :(

[bboisvert@…]

Any word on this? It's becoming more and more of a problem for us, as we expand.

[bboisvert@…]

patch for installed 0.9.5 source to add submitted fields to the HDF on a "mid air collision" error

[bboisvert@…]

patch for installed 0.9.5 templates to display the submitted fields on a "mid air collision" error

[bboisvert@…]

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.

[bboisvert@…]

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

[bboisvert@…]

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.

[bboisvert@…]

Patch for 0.10dev to display page parameters on all error messages

[bboisvert@…]

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.

[cboos]

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

[cboos]

Implements a Retry button on the "Mid Air Collision" error page.

[cboos]

See proof-of-concept implementation in attachment:mid_air_collision_retry-r3341.patch

[bboisvert@…]

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.

[bboisvert@…]

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

[bboisvert@…]

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.

[bboisvert@…]

futher tweaks to attachment:mid_air_collision_retry-bboisvert.patch to deal with some weird timestamp resolution conflicts and improve formatting

[anonymous]

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.

[bboisvert@…]

That last comment was by me, of course. Forgot to enter my email address before I submitted.

[cboos]

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.

[bboisvert@…]

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.

[anonymous]

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.

[cboos]

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

[cboos]

Post-poning to 0.11 (and probably after the WorkFlow merge)

[cboos]

I think the approach suggested in #4100 should be preferred, so I'd like to move the discussion for this topic there.