Description of problem: Currently we use the needinfo flag specifically for requesting information from a specific user or anyone in general. The needinfo flag should not ever be in either the <blank> or ? status. We need a sanity check that returns a list of bugs that have needinfo statuses set to either - or +. Some people mistakenly set these manually and they may not get cleared properly.
Created attachment 340859 [details] Add checking incorrect needinfo flag statuses to sanitycheck
Comment on attachment 340859 [details] Add checking incorrect needinfo flag statuses to sanitycheck Hey Tony, Patch looks good to me, I tested it and it works good , maybe good idea to add the check to the cron job. Thanks, Noura
Comment on attachment 340859 [details] Add checking incorrect needinfo flag statuses to sanitycheck >Index: sanitycheck.cgi >=================================================================== >--- sanitycheck.cgi (revision 1198) >+++ sanitycheck.cgi (working copy) >@@ -113,6 +113,7 @@ > control_values > unsent_emails > flag_check >+ flag_needinfo_status_check > bugs_components > bugs_activity_flag > whines_obsolete_target >@@ -962,7 +963,49 @@ > } > $cgi->param('flag_check_report', 'output'); > } Nit: add extra newline here >+# RED HAT ENTENSION START 497031 > ########################################################################### >+# Check for flags "needinfo" having incorrect status >+########################################################################### >+ >+if ($cgi->param('flag_needinfo_status_check')) { >+ >+ $cgi->param('output', ''); >+ >+ Status('flag_needinfo_status_check_start'); >+ >+ my $invalid_flags = $dbh->selectall_arrayref( >+ 'SELECT DISTINCT flags.status, flags.id, flags.attach_id, flags.bug_id >+ FROM flags >+ INNER JOIN flagtypes >+ ON flags.type_id = flagtypes.id >+ WHERE flagtypes.name = "needinfo" >+ AND flags.status = "+" OR flags.status = "-"'); >+ You need to add parenthesis ( ) around flags.status = "+" OR flags.status = "-". Otherwise you get more rows that you want. On bz-web2-test I got ~4000 rows where with the parens I got only 88. >+ my @invalid_flags = @$invalid_flags; >+ >+ if (scalar(@invalid_flags)) { >+ if ($cgi->param('remove_invalid_needinfo_status')) { >+ Status('flag_deletion_start'); >+ my @flag_ids = map {$_->[1]} @invalid_flags; >+ # Silently delete these flags, with no notification to requesters/setters. >+ $dbh->do('DELETE FROM flags WHERE id IN (' . join(',', @flag_ids) .')'); >+ Status('flag_deletion_end'); I would rather we add a log activity entry here and not just blindly delete the flags from the flags table. We need to retain proper bug history whenever possible. You have the entry recorded as being changed by bugzilla and of course send no email notification. The rest looks fine so far. Make sure to add to cronjob script like Noura mentioned so we can get eng-systems to update it later. Dave
Noura, Dave, Thank you for the review. I have changed my patch according to your review, please see attached file for details. Thanks, Tony
Created attachment 341047 [details] Add checking incorrect needinfo flag statuses to sanitycheck 1. corrected a sql query error 2. added bugs_activity log 3. added flag_needinfo_status_check check to cron jobs.
Comment on attachment 341047 [details] Add checking incorrect needinfo flag statuses to sanitycheck Looks good to me Tony.
committed patch into svn repo. Close this bug.