Description of problem: After renmaing a flagtype, the change doesn't appear in bugs' activity history rerord, which is still using the old flagtype name. It is because that the bugs_activity table records flagtype name rather than flag id which searches the flag name from flagtype table, while renaming the flagtype only updates the flagtype record in flagtype table. In order to fix this possible, we need to update editflagtypes.cgi to change the flagtype name in bugs_activity table after renaming a flagtype.
Dave, I worked out a patch to fix this problem. Can you please review it? The basic idea of this patch is to add a hidden field in template/en/default/admin/flag-type/edit.html.tmpl to record the old flagtype name, then pass it to editflagtypes.cgi, which will check this hidden field to see if the action is renaming the flagtype. If flagtype's name has been changed, then update bugs_activity table accordingly. Thanks, Tony
Created attachment 351432 [details] update bugs_activity table aftering changing a flagtype's name
Comment on attachment 351432 [details] update bugs_activity table aftering changing a flagtype's name >Index: editflagtypes.cgi >=================================================================== > > $dbh->bz_commit_transaction(); Please do the _update_bugs_activity() before the $dbh->bz_commit_transaction() so if something fails with the update the data is rolled back. > >+ # REDHAT EXTENSION START 510996 >+ # change the old flagtype name to new flagtype name in bugs_activity table >+ my $orig_flagname = $cgi->param('flagname-orig'); >+ if ( $orig_flagname && ($orig_flagname ne $name)) { >+ _update_bugs_activity($orig_flagname, $name); >+ } >+ # REDHAT EXTENSION END 510996 Nit-pick: You do not need to add an additional form field for the old flag name as you already have the $flag_type object which has the old flag name before the update. So you really just nedd to do: if ( $name && ($flag_type->name ne $name)) { _update_bugs_activity($flag_type->name, $name); } >+ # find the records that are using the old flagtype name in bugs_activity >+ my $name_orig_sql = $dbh->quote($name_orig); >+ my $name_new_sql = $dbh->quote($name_new); >+ my $query = >+ "SELECT bug_id, bug_when, added, removed" . >+ " FROM bugs_activity " . >+ " WHERE (added REGEXP $name_orig_sql " . >+ " OR removed REGEXP $name_orig_sql)" . >+ " AND fieldid=$fieldid"; Please use the $dbh methods for doing REGEXP instead so this is more portable across different databases. " WHERE (" . $dbh->sql_regexp('added', $name_orig_sql) . " OR " . $dbh->sql_regexp('removed', $name_orig_sql) . " >Index: template/en/default/admin/flag-type/edit.html.tmpl >+ [%# Add a hidden field to record the old flagtype name %] >+ <input type="hidden" name="flagname-orig" value="[% type.name FILTER html %]"> Nit: seem comment above about not needing this field.
Something I just though of and this is purely for discussion is whether we should be doing this in the first place. Normally the Bugzilla rule of thumb is to never remove history for a bug. Which is why we normally do not delete anything if we do not have to. For flags, maybe we should be leaving the names alone. It may be important to know that a bug originally had a rhev-2.1 flag set at a point in time instead of changing that to rhev-5.4-2.1 which is not accurately what happened at that time. Kevin, what is your opinion on this? Dave
(In reply to comment #4) > Something I just though of and this is purely for discussion is whether we > should be doing this in the first place. Normally the Bugzilla rule of thumb is > to never remove history for a bug. Which is why we normally do not delete > anything if we do not have to. > > For flags, maybe we should be leaving the names alone. It may be important to > know that a bug originally had a rhev-2.1 flag set at a point in time instead > of changing that to rhev-5.4-2.1 which is not accurately what happened at that > time. I agree with it. I think leaving the original flag name alone in bugs_activity table makes more sense in terms of recording the change history of a bug. Maybe we just need to remove the function of checking the flag name in bugs_activity from sanity check, so we won't receive this kind warnings from sanity check. And we can add an entry in bugs_activity table to record the change of flag name if someone renames the flagtype name. Tony
Created attachment 352051 [details] update bugs_activity table aftering changing a flagtype's name
Comment on attachment 352051 [details] update bugs_activity table aftering changing a flagtype's name + # REDHAT EXTENSION START 510996 + # change the old flagtype name to new flagtype name in bugs_activity table + #my $orig_flagname = $cgi->param('flagname-orig'); + if ( $name && ($flag_type->name ne $name)) { + _update_bugs_activity($flag_type->name, $name); + } + # REDHAT EXTENSION END 510996 Make sure you remove the commented line above. Other than that, looks good. Make sure you have tested it adequately on the test database then feel free to commit. Dave
committed the patch to svn repository.