I'm filing this since I'm under the impression that Bugzilla is no longer designed to handle open bugs with a resolution. It seems that mass reassignment of a bug that is RESOLVED-LATER can cause the bug to change to NEW without clearing the resolution. See diffs below. (BTW, it would be nice to be able to reassign LATER/REMIND bugs without reopening them.) http://bugzilla.mozilla.org/show_bug.cgi?id=10713 *** shadow/10713 Thu Jan 20 14:41:41 2000 --- shadow/10713.tmp.3292 Mon Mar 6 11:50:25 2000 *************** *** 3,14 **** Version: other Platform: All OS/Version: All ! Status: RESOLVED Resolution: LATER Severity: normal Priority: P5 Component: Layout ! AssignedTo: kipp@netscape.com ReportedBy: tom@dizoglio.com QAContact: vidur@netscape.com TargetMilestone: M15 --- 3,14 ---- Version: other Platform: All OS/Version: All ! Status: NEW Resolution: LATER Severity: normal Priority: P5 Component: Layout ! AssignedTo: buster@netscape.com ReportedBy: tom@dizoglio.com QAContact: vidur@netscape.com TargetMilestone: M15 *************** *** 42,44 **** --- 42,47 ---- Migrating from {css2} to css2 keyword. The {css1}, {css2}, {css3} and {css-moz} radars should now be considered deprecated in favour of keywords. I am *really* sorry about the spam... + + ------- Additional Comments From buster@netscape.com 2000-03-06 11:50 -------+ mine now

Terry does the underlying MySQL bug table have row restrictions? If so you might want to add this in case there are any more situations you missed.

MySQL doesn't have a concept of "row restrictions".

tara@tequilarista.org is the new owner of Bugzilla and Bonsai. (For details, see my posting in netscape.public.mozilla.webtools, news://news.mozilla.org/38F5D90D.F40E8C1A%40geocast.com .)

Huh. Okay, well, changing owner should definitely clear the resolution, however, the second point doesn't really fly with me. Why bother reassinging a bug if it's going to retain the later status? Especially as the dafault query is New|Assigned|Reopened, so if you reassigned a bug, but it retained it's Resolved/Later state, it's entirely possible that the new person would never see it. If that's the case, just close the bug out as wontfix and save some effort.

Reassigning would generate a notification. I assume people who use REMIND/LATER have queries and periodically check for them, so it shouldn't drop away. Of course, REMIND and LATER should be nuked from orbit, it's the only way to be sure.

go don

moving to real milestones...

-> New Bugzilla Product

Good lord.... This is still here... I just did it to 151 bugs. I did a mass-move on INVALID/WONTFIX/DUPLICATE/WORKSFORME from Webtools/Bugzilla to Bugzilla/Bugzilla-General, and did reassign to default owner so that if any of them got reopened someone who still cared would get the email. We need this on b.m.o's next update before we continue to move bugs. Expected behavior is that there is no change in bug_status if the bug_status is already a state that's considered closed, unless the status is also changing for some other reason as a result of the change (i.e. you clicked the reopen button).

*** Bug 77136 has been marked as a duplicate of this bug. ***

AIUI you can't reopen and reassign ...

Comment on attachment 47991 [details] [diff] [review] patch >Index: process_bug.cgi >+ if ( ($::FORM{knob} eq 'reassign' || $::FORM eq 'reassignbycomponent') && ^^^^^^^ There is no $::FORM... it's %::FORM unless you're providing a key.

-> Me

Comment on attachment 47993 [details] [diff] [review] patch v2 -- add missing {knob} Jake says he tested it and it works. My visual inspection says it's good this time. Since we're dealing with process_bug ickyness, and it's after 2am where I am, I suggest a second review.

One note on this: not all bugs get sent to NEW. There is one bug from tonight's mass move by dave that got sent to UNCO. Bug 66191.

yeah, someone just "fixed" the unconfirmed one for us.

Comment on attachment 47993 [details] [diff] [review] patch v2 -- add missing {knob} s/itslef/itself/

Comment on attachment 47993 [details] [diff] [review] patch v2 -- add missing {knob} test

Comment on attachment 48108 [details] [diff] [review] patch v3 - s/itslef/itself/ in comment r= justdave

Comment on attachment 48108 [details] [diff] [review] patch v3 - s/itslef/itself/ in comment The code works on my test installation, however... >Index: process_bug.cgi ... >+ if ( ($::FORM{knob} eq 'reassign' || $::FORM{knob} eq 'reassignbycomponent') && ... >+ $::query .= "bug_status = IF(bug_status IN($open_state), '$str', bug_status)"; >+ return; >+ } > if (IsOpenedState($str)) { Returning within the first if statement means the following line of code in this script never gets called when the first if conditional is true: $::FORM{'bug_status'} = $str; # Used later for call to # CheckCanChangeField to make sure this # is really kosher. That's a security bug. I suggest removing the return and making the second if into an elsif to ensure it doesn't get called when the first if conditional is true.

Myk, not setting |$::FORM{bug_status} = $str;| isn't a security issue as we (more likely than not) aren't really changing the status. Unfortunately, there is no way in the code to know if we are really changing the status or not (as this is done before we even enter the loop that determines what bug(s) we are changing). Also, reassigning a bug requires the same permission as changing its status. While this may not always be true, it is at the moment and I really can't think of another solution.

>Myk, not setting |$::FORM{bug_status} = $str;| isn't a security issue as we >(more likely than not) aren't really changing the status. If I read the SQL query correctly, however, it is possible for the statuses of some of the bugs being reassigned to get changed. >Also, reassigning a bug requires the same permission as changing its status. >While this may not always be true, it is at the moment and I really can't think >of another solution. The current solution is fine, it should just include the last line of the function so the appropriate security check gets done.

Comment on attachment 48798 [details] [diff] [review] patch v4 >+ $::query .= "bug_status = IF(bug_status IN($open_state), '$str', bug_status)"; >+ $::FORM{bug_status} = $str; # To check permissions later on >+ return; >+ } > if (IsOpenedState($str)) { If you make this "elsif", then you don't need "$::FORM{bug_status} = $str;" or "return;" here, saving you two lines of code.

Comment on attachment 49353 [details] [diff] [review] patch v5 works on my test install and looks good. 1st r=myk. do i have a second?

Comment on attachment 49353 [details] [diff] [review] patch v5 r= zach@zachlipton.com in IRC

Checked in.