69447 - Make CC changes not cause midairs.

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, enhancement, P4)

Description

[Matthew Tuck [:CodeMachine]]

It would be nice if CC changes didn't cause midairs, similar to the way voting
works.

Comment 1

[Jesse Ruderman]

But then I could read an entire bug and miss a new comment while CCing myself :(

Comment 2

[Matthew Tuck [:CodeMachine]]

Hmm, maybe.  But that's currently true of voting.

Also we should clash if either person is adding or removing someone who's not
themselves, so doing this would be quite difficult.

Comment 3

[Andreas Franke (gone)]

-> Bugzilla product, Changing Bugs component, reassigning.

Comment 4

[Jason Bassford]

> But then I could read an entire bug and miss a new comment while CCing myself

I didn't see mention of comments and CCs not causing midair collisions.  If you
CC yourself you shouldn't miss a comment (that *should* cause a collision), only
somebody else who's JUST CCing themself too...  (Do you care about missing
somebody else adding themselves to the CC list?)

Comment 5

[Mike Cowperthwaite]

(In reply to comment #2)
> Also we should clash if either person is adding or removing someone who's not
> themselves, so doing this would be quite difficult.

I don't agree with this; if the CC from the other person was added because the other person duped a bug over, or for any reason, I don't want that to cause a collision with my comment -- why should I care if that new person also gets my comment?  I *expect* that.

And if person A removes person B, why is that different from removing herself?  Barring vandalism, any removal from the list should be unimportant -- if I care whether a comment reaches a particular user, I scan the list of who got notified and, if necessary, drop a note.

The vandalism case should be handled by notification to the removee.
(Maybe that's the case now?)

Comment 9

[Jesse Ruderman]

CC changes should not cause mid-air collision interstitials.  Instead, the new CCs should get emails showing what happened to the bug as if they had been CCed a few moments earlier.

Comment 10

[Jeff Hammel]

I think this is being made more complicated by considering all edge cases.  CC is a list.  You can add to list.  You can remove from list.  There is a negligible actual conflict when both of those are nearly simultaneous, but that is conceptually ignorable.  Anything on top of that is another feature.  What is desired is, especially for new and active bugs, if I try to, e.g., add myself as a CC and get a collision...so I navigate back, refresh the page, and try again.  If the bug is really active, I rinse and repeat again. This is what is annoying.  The only possible conflict here is if someone adds me and I add myself, in which case should just not care -- I'm already added, job done.

Comment 11

[Josh Triplett]

Related to this, the mid-air collision message currently says that "the above changes" will be overridden (except for comments), but that "except" also includes the CC list, apparently.  (I had a mid-air collision and proceeded with my changes, and the CC list change got preserved as well, which seems like the correct behavior but which means the message needs changing.)

Comment 13

[David Lawrence [:dkl]]

(In reply to Josh Triplett from comment #11)
> Related to this, the mid-air collision message currently says that "the
> above changes" will be overridden (except for comments), but that "except"
> also includes the CC list, apparently.  (I had a mid-air collision and
> proceeded with my changes, and the CC list change got preserved as well,
> which seems like the correct behavior but which means the message needs
> changing.)

This part is being addressed by bug 303738 actually.

dkl

Comment 14

[David Lawrence [:dkl]]

Attachment: Patch to disallow midairs if only cc/comment/attachment made previously (v1)

This skips doing a midair collision of the previous user's changes were only one or more of comment/cc/attachment. It throws a note on the updated bug page saying that a prior comment/cc/attachment was detected.

dkl

Comment 15

[:glob ✱]

(In reply to David Lawrence [:dkl] from comment #14)
> Created attachment 652948 [details] [diff] [review]
> Patch to disallow midairs if only cc/comment/attachment made previously (v1)
> 
> This skips doing a midair collision of the previous user's changes were only
> one or more of comment/cc/attachment. It throws a note on the updated bug
> page saying that a prior comment/cc/attachment was detected.

i think we should still show the midair collision page even if the changes were only adding a comment or attachment.

in other words, only CC changes should be except from triggering midair changes.

Comment 16

[Frédéric Buclin]

Comment on attachment 652948 [details] [diff] [review]
Patch to disallow midairs if only cc/comment/attachment made previously (v1)

>+        # Do not midair if previous changes made were just cc/comments/attachemnts

>+                $do_midair = 1 if $change->{'fieldname'} ne 'cc';

I didn't look at the patch carefully at all. But at first glance, what the comment above and the text below say are not consistent with the check done above, as you only look at 'cc'. And I have to agree with glob: adding/editing an attachment and adding a comment should show the midair collision page. I often midaired with someone else writing a comment at the same time I was writing one, and in many cases, this last comment had an impact of what I wanted to say, which forced me to reword, agree or disagree with what was just said, or simply to cancel my comment because everything was already said in the previous comment.


>+    Your changes were submitted even though this bug had been updated since
>+    you last loaded it. The previous changes were one or more comments,
>+    changes to the cc list, or one or more attachments were added.

About CC changes, I don't think there is any need to mention that someone (un-)CC'ed himself. This information is really not important nor useful. So I wouldn't display any message at all.

For the next dev cycle (aka 4.5), I would like Bugzilla to either periodically or just when clicking the "Submit" button to look at the current state of the bug, and warn the user if the bug has been edited meanwhile. I guess this could be useful. But that's another story (and bug). :)

Comment 17

[David Lawrence [:dkl]]

Attachment: Patch to disallow midairs if only cc change made previously (v2)

New patch only skips midair if just changes to the bugs cc list were made. 

One thing I noticed (which I fill file sep. bug for) is that when no bug field changes were made, only comments, and then we include activity/table.html.tmpl in midair.html.tmpl. With that it says "No changes have been made to this bug yet" which may or may not be true. So we need to word it differently when used in midair.html.tmpl.

dkl

Comment 18

[Frédéric Buclin]

Comment on attachment 653484 [details] [diff] [review]
Patch to disallow midairs if only cc change made previously (v2)

>+        # Always sort midoair collision comments oldest to newest,

typo: s/midoair/midair/


>+        $do_midair = 1 if scalar @$comments > $start_at;

As this check is faster than looping over operations, I would do it first, and only loop over operations if $do_midair is still 0.


r=LpSolit

Comment 19

[David Lawrence [:dkl]]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk
modified process_bug.cgi
Committed revision 8386.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.4
modified process_bug.cgi
Committed revision 8384.

Comment 20

[Frédéric Buclin]

Added to relnotes for 4.4.