412836 - Clean up process_bug.cgi now that we have Bugzilla::Bug->update

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, enhancement)

Description

[Max Kanat-Alexander]

Now that we have Bugzilla::Bug->update, there's a lot of little cleanups that can be done in process_bug. There's also a lot of remaining code there that's now useless.

Comment 1

[Max Kanat-Alexander]

Attachment: v1

Okay, this patch does or fixes the following things:

* Removed comment_exists, because it's not used anymore.

* Removed @idlist and we only use @bug_objects, now.

* Use new_from_list to create @bug_objects.

* Made "dontchange" actually delete things from the $cgi variable, so
  we don't have to worry if things are set to dontchange anymore.

* Made keyword changes happen before moving happens, so that you can
  add a keyword to a bug before you move it.

* Dependency setting now happens after product setting, and not way
  up at the top of the file.

* Midair collision checks shouldn't happen in the update loop, they should
  happen before we do anything. There's also no reason to separate
  setting $vars->{title_tag} from checking for a midair. I also made
  a few small changes to the midair template.

* The product_edit_denied error was in a really bad place where it could
  happen after some bugs had already been updated. (It also wasn't protecting
  moves.)

* Groups weren't getting updated if "product" wasn't passed as a CGI var.

* Put the updating of custom fields into the same loop as updating of
  standard fields.

* Changed the name of the global $bug object to $first_bug to be clearer
  and to not conflict with any loop variables.

* Removed $::comma, $::query, $bug_changed, SnapshotBug and any direct updates of
  a bug from process_bug.cgi.

* Removed bugs_activity updating from process_bug, which also removes
  the duplicated check_can_change_field checks.

* Removed lots of now-useless code from the DB-updating loop, like $newhash
  and $oldhash stuff.

* $old_bug_obj and $new_bug_obj aren't needed anymore.

##############################
# Still TODO In Another Bug: #
##############################

* All of the various update_ functions need to be united into one.

* RemoveVotes and CheckIfVotedConfirmed probably need to happen
  inside of update().

* At some point, email-sending should be directly tied to update(),
  or at least somehow closely linked.

Comment 2

[Max Kanat-Alexander]

Attachment: v2

Okay, I fixed the bitrot on tip.

Comment 3

[Max Kanat-Alexander]

Attachment: v3

LpSolit pointed out on IRC that the $changes array held IDs instead of login names for assigned_to and qa_contact, which messed up bugmail.

Comment 4

[Frédéric Buclin]

Comment on attachment 298047 [details] [diff] [review]
v3

I did some basic testing, including mass-change and using user accounts with no privs, and everything seems to work fine. So a big r=LpSolit for this patch. Thanks a lot! :)

Comment 5

[Max Kanat-Alexander]

Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.399; previous revision: 1.398
done
Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.228; previous revision: 1.227
done
Checking in Bugzilla/User.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/User.pm,v  <--  User.pm
new revision: 1.164; previous revision: 1.163
done
Checking in template/en/default/bug/process/midair.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/process/midair.html.tmpl,v  <--  midair.html.tmpl
new revision: 1.21; previous revision: 1.20
done