Bug 510996 - Renamed flagtype doesn't reflect in bugs' activity history record
Summary: Renamed flagtype doesn't reflect in bugs' activity history record
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Bugzilla
Classification: Community
Component: Bugzilla General
Version: 3.2
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Tony Fu
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-07-13 05:17 UTC by Tony Fu
Modified: 2009-07-20 04:47 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-07-20 04:47:21 UTC
Embargoed:


Attachments (Terms of Use)
update bugs_activity table aftering changing a flagtype's name (3.31 KB, patch)
2009-07-13 05:47 UTC, Tony Fu
dkl: review-
Details | Diff
update bugs_activity table aftering changing a flagtype's name (2.59 KB, patch)
2009-07-15 03:03 UTC, Tony Fu
dkl: review+
Details | Diff

Description Tony Fu 2009-07-13 05:17:37 UTC
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.

Comment 1 Tony Fu 2009-07-13 05:45:59 UTC
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

Comment 2 Tony Fu 2009-07-13 05:47:31 UTC
Created attachment 351432 [details]
update bugs_activity table aftering changing a flagtype's name

Comment 3 David Lawrence 2009-07-13 21:01:59 UTC
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.

Comment 4 David Lawrence 2009-07-13 21:23:47 UTC
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

Comment 5 Tony Fu 2009-07-14 00:28:56 UTC
(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

Comment 6 Tony Fu 2009-07-15 03:03:30 UTC
Created attachment 352051 [details]
update bugs_activity table aftering changing a flagtype's name

Comment 7 David Lawrence 2009-07-15 21:11:19 UTC
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

Comment 8 Tony Fu 2009-07-20 04:47:21 UTC
committed the patch to svn repository.


Note You need to log in before you can comment on or make changes to this bug.