Multiple people editing any object in Launchpad may overwrite the other persons changes (via mid-air collisions)

Use case taken from bug 45304:

If a bug has been reassigned from:

  https://launchpad.net/distros/ubuntu/+bug/45240

while somebody else is also using the editstatus dialogue then the submit is made to:

  https://launchpad.net/distros/ubuntu/+bug/45240/+editstatus

which fails because the path is no longer valid.

  OOPS-137A366

shows a incident of this.

I believe I have encountered this as well when reassigning milestones last week. I would make a change to a milestone, refresh and the change was not applied. I had to reassign/retarget the milestone a second time and then it would take. For me it happened about 1 in every 50 instances.

(From bug 123059:)

USE CASE
Mike opens a few bug reports to triage them; one of those is bug #11111. While Mike is triaging another bug, Daniel modifies bug #11111. After that, Mike finished with the current bug and decides to work on bug #11111 (which he already has loaded on a tab) and makes some changes too; when he submits them, Daniel's changes get silently overridden.

IDEAL CASE
When Mike submits his changes, those fields that didn't got changed by Daniel get applied, but he gets a warning about the conflicting ones, asking him to confirm those.

IMPLEMENTATION
A hidden input field whit the timestamp of when the page was loaded as value. This gets submitted with the other changes and is used to check if there were newer changes.

The launchpad-answers counterpart of this bug is bug 192173

These cases are so rare, and solving them is so hard, that I'm not sure keeping this bug in Triaged/Low is the right thing to do.

I've been hitting this a bit recently, twice in two days, for example in <https://bugs.edge.launchpad.net/bzr/+bug/388675>. It's true that it's not unrecoverable but it does sometimes cause confusion along the lines of "hey, why did you reopen my bug?!?"

I don't think this needs to be very hard to solve, either. Store in hidden fields the original values of the assignee, status, importance, milestone, etc. Then when the form is submitted, only set the fields that were changed relative to the origin.

This should mostly go away once all of the edits are AJAX, shouldn't it? Then only the updated fields will be overwritten, unless you have no JS.

I don't think it goes away wgrant, it actually becomes worse :). What you need is to propogate other peoples edits to the people with the bug open [which could be done a number of different ways].

lifeless: It handles the case that poolie was talking about - changing one field clobbers changes in any of the rest. In the bug he pointed out, spiv probably accidentally unmilestoned the bug because somebody milestoned it while he had the bug page open. If the edits were done via AJAX, only the status field would have been saved, not the stale milestone.

This bug is easy to fix. Store a hash of all the changeable fields in a hidden form field, and send it back with any submission. The server can re-calculate the hash from what it thinks the current values are and if it agrees with the submission, great, if not, you report an edit collision and re-load the form with the current data. If you want to be fancy, you can do a hash per-field to be able to show which fields collided.

I would really prefer seeing what someone else did instead of blindly editing over the top.

Julian Edwards wrote:
[...]
> collision and re-load the form with the current data. If you want to be
> fancy, you can do a hash per-field to be able to show which fields
> collided.

Really fancy would be including a hidden timestamp field, and then if the last
update is newer than that use the BugActivity table to figure out what changed
since the user loaded the page...

On Thu, Jun 18, 2009 at 09:23:10AM -0000, Andrew Bennetts wrote:
> Julian Edwards wrote:
> [...]
> > collision and re-load the form with the current data. If you want to be
> > fancy, you can do a hash per-field to be able to show which fields
> > collided.
>
> Really fancy would be including a hidden timestamp field, and then if the last
> update is newer than that use the BugActivity table to figure out what changed
> since the user loaded the page...

Since we now track the changes to the bug a lot we could actually do
that... Unfortunately the changelog isn't structured enough to be
reliable enough for collision detection (well, maybe, but probably not).

The hidden fields thing is probably the easiest way to do this, though
it feels kinda hackish.

 status triaged
 importance medium

On Thursday 18 June 2009 10:34:44 Graham Binns wrote:
> The hidden fields thing is probably the easiest way to do this, though
> it feels kinda hackish.

It's not hackish at all IMO, it's a pretty standard way of working with
stateless clients.

On Thu, Jun 18, 2009 at 09:48:58AM -0000, Julian Edwards wrote:
> On Thursday 18 June 2009 10:34:44 Graham Binns wrote:
> > The hidden fields thing is probably the easiest way to do this, though
> > it feels kinda hackish.
>
> It's not hackish at all IMO, it's a pretty standard way of working with
> stateless clients.
>

To clarify, I feel that it's hackish compared to using the changelog
(were it reliable enough, which I'm not certain that it is right now)
plus a simple timestamp (marking when a user's interaction with the bug
began) to work out what happened when and which changes collide with
each other. Keeping a hash of the old state of the bug when we should
have a good record of it anyway (or at least a cumulative one) in the
database.

2009/6/18 Graham Binns <email address hidden>:

>> It's not hackish at all IMO, it's a pretty standard way of working with
>> stateless clients.
>>
>
> To clarify, I feel that it's hackish compared to using the changelog
> (were it reliable enough, which I'm not certain that it is right now)
> plus a simple timestamp (marking when a user's interaction with the bug
> began) to work out what happened when and which changes collide with
> each other. Keeping a hash of the old state of the bug when we should
> have a good record of it anyway (or at least a cumulative one) in the
> database.

There were two different proposals using hidden fields (at least).
One was to store a hash. Mine, which I still think is simple and
non-hacky, is to keep a second copy representing the original state of
each field:

  old_importance=medium importance=medium ---> no change
  old_target=None target=1.16 ---> change target to 1.16

Then you can see which fields they intended to change. If there was
no delta, don't change that field. If there was, do change it. For
bonus points, if the current value at the time of submission is not
the old value, you can tell the user about the conflict (maybe by
showing a notification bubble) - I'd probably apply the change anyhow.

I don't see how indirecting through the database to get the old values
helps because the amount of data is pretty small.

--
Martin <http://launchpad.net/~mbp/>

On Thu, Jun 18, 2009 at 11:29:58PM -0000, Martin Pool wrote:
> I don't see how indirecting through the database to get the old values
> helps because the amount of data is pretty small.

I was thinking that it would help for scenarios where people change
things via the API, which wouldn't have the second copy of the fields to
compare the changes against.

However, I've come to the conclusion that neither approach deals
perfectly with changes made via the API (or via AJAX elements, since
they use the API too), which is something of a problem. Whilst we could
mitigate that to some extent by making AJAX elements check for
collisions we can't really force other API users to do so.

Perhaps it would be sufficient to apply this for the whole bugtask / bug
edit forms, but as we move to an all-AJAX bug page that's going to
become less and less relevant for the majority of users.

On Thu, Jun 18, 2009 at 08:12:06AM -0000, Julian Edwards wrote:
> This bug is easy to fix.

Great, thanks for volunteering to fix it! ;)

I wouldn't call this exactly easy. Sure, we know what to do, but it's
still quite a lot of work involved actually fixing it.

On Tuesday 23 June 2009 09:39:53 Björn Tillenius wrote:
> On Thu, Jun 18, 2009 at 08:12:06AM -0000, Julian Edwards wrote:
> > This bug is easy to fix.
>
> Great, thanks for volunteering to fix it! ;)
>
> I wouldn't call this exactly easy. Sure, we know what to do, but it's
> still quite a lot of work involved actually fixing it.

Want me to swap you some Soyuz bugs? ;)