From 32c74ca90011bfa5cfc781ca0879529abb3c5d30 Mon Sep 17 00:00:00 2001 From: "jake%acutex.net" <> Date: Fri, 20 Jul 2001 22:18:25 +0000 Subject: Fix for bugs 55161 and 12819. The activity log now stores only what's changed in multi-value fields. r= justdave@syndicomm.com --- CGI.pl | 26 +++++------- buglist.cgi | 15 ++++++- checksetup.pl | 70 ++++++++++++++++++++++++++++++- globals.pl | 31 ++++++++++++++ move.pl | 4 +- process_bug.cgi | 127 +++++++++++++++++++++++++++++++++----------------------- processmail | 10 ++--- 7 files changed, 203 insertions(+), 80 deletions(-) diff --git a/CGI.pl b/CGI.pl index f248b2599..06d32d05f 100644 --- a/CGI.pl +++ b/CGI.pl @@ -1105,12 +1105,12 @@ sub CheckIfVotedConfirmed { "WHERE bug_id = $id"); my $fieldid = GetFieldID("bug_status"); SendSQL("INSERT INTO bugs_activity " . - "(bug_id,who,bug_when,fieldid,oldvalue,newvalue) VALUES " . + "(bug_id,who,bug_when,fieldid,removed,added) VALUES " . "($id,$who,now(),$fieldid,'$::unconfirmedstate','NEW')"); if (!$everconfirmed) { $fieldid = GetFieldID("everconfirmed"); SendSQL("INSERT INTO bugs_activity " . - "(bug_id,who,bug_when,fieldid,oldvalue,newvalue) VALUES " . + "(bug_id,who,bug_when,fieldid,removed,added) VALUES " . "($id,$who,now(),$fieldid,'0','1')"); } AppendComment($id, DBID_to_name($who), @@ -1136,7 +1136,7 @@ sub DumpBugActivity { my $query = " SELECT IFNULL(fielddefs.name, bugs_activity.fieldid), bugs_activity.bug_when, - bugs_activity.oldvalue, bugs_activity.newvalue, + bugs_activity.removed, bugs_activity.added, profiles.login_name FROM bugs_activity LEFT JOIN fielddefs ON bugs_activity.fieldid = fielddefs.fieldid, @@ -1149,25 +1149,21 @@ sub DumpBugActivity { print "\n"; print "\n"; - print " \n"; + print " \n"; print "\n"; my @row; while (@row = FetchSQLData()) { - my ($field,$when,$old,$new,$who) = (@row); - $old = value_quote($old); - $new = value_quote($new); - if ($old eq "") { - $old = " "; - } - if ($new eq "") { - $new = " "; - } + my ($field,$when,$removed,$added,$who) = (@row); + $removed = html_quote($removed); + $added = html_quote($added); + $removed ||= " "; + $added ||= " "; print "\n"; print "\n"; print "\n"; - print "\n"; - print "\n"; + print "\n"; + print "\n"; print "\n"; print "\n"; } diff --git a/buglist.cgi b/buglist.cgi index 6c0ff1e7f..ea3ec2049 100755 --- a/buglist.cgi +++ b/buglist.cgi @@ -1426,6 +1426,17 @@ document.write(\" "; } + print qq{ + + + +}; + if (@::legal_keywords) { print qq{ @@ -1541,8 +1552,8 @@ if ($::usergroupset ne '0' && $buggroupset =~ /^\d+$/) { To make changes to a bunch of bugs at once:
  1. Put check boxes next to the bugs you want to change. -
  2. Adjust above form elements. (It's always a good idea to add some - comment explaining what you're doing.) +
  3. Adjust above form elements. (If the change you are making requires + an explanation, include it in the comments box).
  4. Click the below \"Commit\" button.
"; diff --git a/checksetup.pl b/checksetup.pl index 8ce8bf6f7..bcbebff6d 100755 --- a/checksetup.pl +++ b/checksetup.pl @@ -768,8 +768,8 @@ $table{bugs_activity} = who mediumint not null, bug_when datetime not null, fieldid mediumint not null, - oldvalue tinytext, - newvalue tinytext, + added tinytext, + removed tinytext, index (bug_id), index (bug_when), @@ -2307,6 +2307,72 @@ if (!defined GetIndexDef('longdescs','who')) { # truncates re http://bugzilla.mozilla.org/show_bug.cgi?id=9352 ChangeFieldType('bugs', 'version','varchar(64) not null'); +# 2001-07-20 jake@acutex.net - Change bugs_activity to only record changes +# http://bugzilla.mozilla.org/show_bug.cgi?id=55161 +if (GetFieldDef('bugs_activity', 'oldvalue')) { + AddField("bugs_activity", "removed", "tinytext"); + AddField("bugs_activity", "added", "tinytext"); + + # Need to get fieldid's for the fields that have multipule values + my @multi = (); + foreach my $f ("cc", "dependson", "blocked", "keywords") { + my $sth = $dbh->prepare("SELECT fieldid FROM fielddefs WHERE name = '$f'"); + $sth->execute(); + my ($fid) = $sth->fetchrow_array(); + push (@multi, $fid); + } + + # Now we need to process the bugs_activity table and reformat the data + my $i = 0; + print "Fixing activity log "; + my $sth = $dbh->prepare("SELECT bug_id, who, bug_when, fieldid, + oldvalue, newvalue FROM bugs_activity"); + $sth->execute; + while (my ($bug_id, $who, $bug_when, $fieldid, $oldvalue, $newvalue) = $sth->fetchrow_array()) { + # print a "." every 500 records so the user knows we didn't die + print "." if !($i++ % 500); + # Make sure (old|new)value isn't null (to suppress warnings) + $oldvalue ||= ""; + $newvalue ||= ""; + my ($added, $removed) = ""; + if (grep /^$fieldid$/, @multi) { + my (@add, @remove) = (); + my @old = split(/[ ,]/, $oldvalue); + my @new = split(/[ ,]/, $newvalue); + # Find values that were "added" + foreach my $value(@new) { + if (! grep /^$value$/, @old) { + push (@add, $value); + } + } + # Find values that were removed + foreach my $value(@old) { + if (! grep /^$value$/, @new) { + push (@remove, $value); + } + } + $added = join (", ", @add); + $removed = join (", ", @remove); + # If we can't determine what changed, put a ? in both fields + unless ($added || $removed) { + $added = "?"; + $removed = "?"; + } + } else { + $removed = $oldvalue; + $added = $newvalue; + } + $added = $dbh->quote($added); + $removed = $dbh->quote($removed); + $dbh->do("UPDATE bugs_activity SET removed = $removed, added = $added + WHERE bug_id = $bug_id AND who = $who + AND bug_when = '$bug_when' AND fieldid = $fieldid"); + } + print ". Done.\n"; + DropField("bugs_activity", "oldvalue"); + DropField("bugs_activity", "newvalue"); +} + # If you had to change the --TABLE-- definition in any way, then add your # differential change code *** A B O V E *** this comment. # diff --git a/globals.pl b/globals.pl index 302d9b8b7..51fe1ddee 100644 --- a/globals.pl +++ b/globals.pl @@ -1224,6 +1224,37 @@ sub Param ($) { die "Can't find param named $value"; } +# Take two comma or space separated strings and return what +# values were removed from or added to the new one. +sub DiffStrings { + my ($oldstr, $newstr) = @_; + + my (@remove, @add) = (); + my @old = split(/[ ,]/, $oldstr); + my @new = split(/[ ,]/, $newstr); + + # Find values that were removed + foreach my $value(@old) { + next if $value =~ /^\s*$/; + if (! grep /^$value$/, @new) { + push (@remove, $value); + } + } + + # Find values that were added + foreach my $value(@new) { + next if $value =~ /^\s*$/; + if (! grep /^$value$/, @old) { + push (@add, $value); + } + } + + my $removed = join (", ", @remove); + my $added = join (", ", @add); + + return ($removed, $added); +} + sub PerformSubsts { my ($str, $substs) = (@_); $str =~ s/%([a-z]*)%/(defined $substs->{$1} ? $substs->{$1} : Param($1))/eg; diff --git a/move.pl b/move.pl index 42eb96351..37b8cb7ef 100755 --- a/move.pl +++ b/move.pl @@ -107,12 +107,12 @@ foreach my $id (split(/:/, $::FORM{'buglist'})) { my $fieldid = GetFieldID("bug_status"); my $cur_status= $bug->bug_status; SendSQL("INSERT INTO bugs_activity " . - "(bug_id,who,bug_when,fieldid,oldvalue,newvalue) VALUES " . + "(bug_id,who,bug_when,fieldid,removed,added) VALUES " . "($id,$exporterid,now(),$fieldid,'$cur_status','RESOLVED')"); $fieldid = GetFieldID("resolution"); my $cur_res= $bug->resolution; SendSQL("INSERT INTO bugs_activity " . - "(bug_id,who,bug_when,fieldid,oldvalue,newvalue) VALUES " . + "(bug_id,who,bug_when,fieldid,removed,added) VALUES " . "($id,$exporterid,now(),$fieldid,'$cur_res','MOVED')"); SendSQL("UPDATE bugs SET bug_status =\"RESOLVED\" where bug_id=\"$id\""); diff --git a/process_bug.cgi b/process_bug.cgi index 26ea412c1..b5ea3c782 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -456,32 +456,9 @@ if (defined $::FORM{'qa_contact'}) { ConnectToDatabase(); -my $formCcSet = new RelationSet; -my $origCcSet = new RelationSet; -my $origCcString; my $removedCcString = ""; my $duplicate = 0; -# We make sure to check out the CC list before we actually start touching any -# bugs. mergeFromString() ultimately searches the database using a quoted -# form of the data it gets from $::FORM{'cc'}, so anything bogus from a -# security standpoint should trigger an abort there. -# -if (defined $::FORM{'newcc'} && defined $::FORM{'id'}) { - $origCcSet->mergeFromDB("select who from cc where bug_id = $::FORM{'id'}"); - $formCcSet->mergeFromDB("select who from cc where bug_id = $::FORM{'id'}"); - $origCcString = $origCcSet->toString(); # cache a copy of the string vers - if ((exists $::FORM{'removecc'}) && (exists $::FORM{'cc'})) { - - # save off the folks removed from the CC list so they can be given to - # the processmaill command line so they can be sent mail about it. - # - $removedCcString = join (',', @{$::MFORM{'cc'}}); - $formCcSet->removeItemsInArray(@{$::MFORM{'cc'}}); - } - $formCcSet->mergeFromString($::FORM{'newcc'}); -} - if ( Param('strictvaluechecks') ) { CheckFormFieldDefined(\%::FORM, 'knob'); } @@ -686,10 +663,14 @@ sub LogDependencyActivity { my ($i, $oldstr, $target, $me) = (@_); my $newstr = SnapShotDeps($i, $target, $me); if ($oldstr ne $newstr) { + # Figure out what's really different... + my ($removed, $added) = DiffStrings($oldstr, $newstr); + $added = SqlQuote($added); + $removed = SqlQuote($removed); my $fieldid = GetFieldID($target); SendSQL("INSERT INTO bugs_activity " . - "(bug_id,who,bug_when,fieldid,oldvalue,newvalue) VALUES " . - "($i,$whoid,$timestamp,$fieldid,'$oldstr','$newstr')"); + "(bug_id,who,bug_when,fieldid,removed,added) VALUES " . + "($i,$whoid,$timestamp,$fieldid,$removed,$added)"); return 1; } return 0; @@ -899,25 +880,67 @@ The changes made were: AppendComment($id, $::FORM{'who'}, $::FORM{'comment'}); } - if (defined $::FORM{'newcc'} && defined $::FORM{'id'} - && ! $origCcSet->isEqual($formCcSet) ) { - - # update the database to look like the form - # - my @CCDELTAS = $origCcSet->generateSqlDeltas($formCcSet, "cc", - "bug_id", $::FORM{'id'}, - "who"); - $CCDELTAS[0] eq "" || SendSQL($CCDELTAS[0]); - $CCDELTAS[1] eq "" || SendSQL($CCDELTAS[1]); - - my $col = GetFieldID('cc'); - my $origq = SqlQuote($origCcString); - my $newq = SqlQuote($formCcSet->toString()); - SendSQL("INSERT INTO bugs_activity " . - "(bug_id,who,bug_when,fieldid,oldvalue,newvalue) VALUES " . - "($id,$whoid,'$timestamp',$col,$origq,$newq)"); + if (defined $::FORM{newcc} || defined $::FORM{removecc} || defined $::FORM{masscc}) { + # Get the current CC list for this bug + my %oncc; + SendSQL("SELECT who FROM cc WHERE bug_id = $id"); + while (MoreSQLData()) { + $oncc{FetchOneColumn()} = 1; + } + + # If masscc is defined, then we came from buglist and need to either add or + # remove cc's... otherwise, we came from bugform and may need to do both. + my ($cc_add, $cc_remove) = ""; + if (defined $::FORM{masscc}) { + if ($::FORM{ccaction} eq 'add') { + $cc_add = $::FORM{masscc}; + } elsif ($::FORM{ccaction} eq 'remove') { + $cc_remove = $::FORM{masscc}; + } + } else { + $cc_add = $::FORM{newcc}; + # We came from bug_form which uses a select box to determine what cc's + # need to be removed... + if (defined $::FORM{removecc}) { + $cc_remove = join (",", @{$::MFORM{cc}}); + } + } + + my (@added, @removed) = (); + if ($cc_add) { + my @new = split(/[ ,]/, $cc_add); + foreach my $person (@new) { + my $pid = DBNameToIdAndCheck($person); + # If this person isn't already on the cc list, add them + if (! $oncc{$pid}) { + SendSQL("INSERT INTO cc (bug_id, who) VALUES ($id, $pid)"); + push (@added, $person); + } + } + } + if ($cc_remove) { + my @old = split (/[ ,]/, $cc_remove); + foreach my $person (@old) { + my $pid = DBNameToIdAndCheck($person); + # If the person is on the cc list, remove them + if ($oncc{$pid}) { + SendSQL("DELETE FROM cc WHERE bug_id = $id AND who = $pid"); + push (@removed, $person); + } + } + # Save off the removedCcString so it can be fed to processmail + $removedCcString = join (",", @removed); + } + # If any changes were found, record it in the activity log + if (scalar(@removed) || scalar(@added)) { + my $col = GetFieldID('cc'); + my $removed = SqlQuote(join(", ", @removed)); + my $added = SqlQuote(join(", ", @added)); + SendSQL("INSERT INTO bugs_activity " . + "(bug_id,who,bug_when,fieldid,removed,added) VALUES " . + "($id,$whoid,'$timestamp',$col,$removed,$added)"); + } } - if (defined $::FORM{'dependson'}) { my $me = "blocked"; @@ -1013,6 +1036,11 @@ The changes made were: $origQaContact = $old; } + # If this is the keyword field, only record the changes, not everything. + if ($col eq 'keywords') { + ($old, $new) = DiffStrings($old, $new); + } + if ($col eq 'product') { RemoveVotes($id, 0, "This bug has been moved to a different product"); @@ -1020,7 +1048,7 @@ The changes made were: $col = GetFieldID($col); $old = SqlQuote($old); $new = SqlQuote($new); - my $q = "insert into bugs_activity (bug_id,who,bug_when,fieldid,oldvalue,newvalue) values ($id,$whoid,'$timestamp',$col,$old,$new)"; + my $q = "insert into bugs_activity (bug_id,who,bug_when,fieldid,removed,added) values ($id,$whoid,'$timestamp',$col,$old,$new)"; # puts "
$q
" SendSQL($q); } @@ -1054,17 +1082,10 @@ The changes made were: my $isoncc = FetchOneColumn(); unless ($isreporter || $isoncc) { # The reporter is oblivious to the existance of the new bug... add 'em to the cc (and record activity) - SendSQL("SELECT who FROM cc WHERE bug_id = " . SqlQuote($duplicate)); - my @dupecc; - while (MoreSQLData()) { - push (@dupecc, DBID_to_name(FetchOneColumn())); - } - my @newdupecc = @dupecc; - push (@newdupecc, DBID_to_name($reporter)); my $ccid = GetFieldID("cc"); my $whochange = DBNameToIdAndCheck($::FORM{'who'}); - SendSQL("INSERT INTO bugs_activity (bug_id,who,bug_when,fieldid,oldvalue,newvalue) VALUES " . - "('$duplicate','$whochange',now(),$ccid,'" . join (",", sort @dupecc) . "','" . join (",", sort @newdupecc) . "')"); + SendSQL("INSERT INTO bugs_activity (bug_id,who,bug_when,fieldid,removed,added) VALUES " . + "('$duplicate','$whochange',now(),$ccid,'','" . DBID_to_name($reporter) . "')"); SendSQL("INSERT INTO cc (who, bug_id) VALUES ($reporter, " . SqlQuote($duplicate) . ")"); } AppendComment($duplicate, $::FORM{'who'}, "*** Bug $::FORM{'id'} has been marked as a duplicate of this bug. ***"); diff --git a/processmail b/processmail index 0fcdbbdde..0f2dac947 100755 --- a/processmail +++ b/processmail @@ -135,7 +135,7 @@ sub ProcessOneBug { SendSQL("SELECT profiles.login_name, fielddefs.description, " . - " bug_when, oldvalue, newvalue " . + " bug_when, removed, added " . "FROM bugs_activity, fielddefs, profiles " . "WHERE bug_id = $id " . " AND fielddefs.fieldid = bugs_activity.fieldid " . @@ -157,7 +157,7 @@ sub ProcessOneBug { if ($who ne $lastwho) { $lastwho = $who; $difftext .= "\n$who" . Param('emailsuffix') . " changed:\n\n"; - $difftext .= FormatTriple("What ", "Old Value", "New Value"); + $difftext .= FormatTriple("What ", "Removed", "Added"); $difftext .= ('-' x 76) . "\n"; } $difftext .= FormatTriple($what, $old, $new); @@ -171,7 +171,7 @@ sub ProcessOneBug { my $resid = SendSQL("SELECT bugs_activity.bug_id, fielddefs.name, " . - " oldvalue, newvalue " . + " removed, added " . "FROM bugs_activity, dependencies, fielddefs ". "WHERE bugs_activity.bug_id = dependencies.dependson " . " AND dependencies.blocked = $id " . @@ -414,9 +414,7 @@ sub getEmailAttributes ($@) { } elsif ($fieldName eq 'QAContact') { push (@{$force{'QAContact'}}, $new); } elsif ($fieldName eq 'CC') { - my @oldVal = split (/,/, $old); - my @newVal = split (/,/, $new); - my @added = filterExcludeList(\@newVal, \@oldVal); + my @added = split (/[ ,]/, $new); push (@{$force{'CClist'}}, @added); } } -- cgit v1.2.3-24-g4f1b
WhoWhatOld valueNew valueWhenWhoWhatRemovedAddedWhen
$who$field$old$new$removed$added$when
CC List: + +
Keywords: