438810 - Whines fail when a group or user that was receiving whines is manually deleted, and sanitycheck.cgi should report this problem

Status: RESOLVED FIXED

(Bugzilla :: Whining, defect)

Description

[Potri]

User-Agent:       Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 5.1; .NET CLR 1.1.4322; .NET CLR 2.0.50727; InfoPath.2; FDM)
Build Identifier: Version 3.0.4

Hi, 

        we are getting the below mentioned error when we try to run 
the whine.pl script and even we upgraded the version of bugzilla from 
3.0.1 to 3.0.4 and even then we are getting the same error. 


      " Can't use an undefined value as an ARRAY reference at ./ 
whine.pl line 253. 
DBI::db=HASH(0x1b73e10)->disconnect invalidates 1 active statement 
handle (either destroy statement handles or call finish on them before 
disconnecting) at Bugzilla.pm line 435 ". 


      Please let us know how resolve this issue. 


Regards, 
Potri Raaja.M. 




Reproducible: Always

Steps to Reproduce:
1.Run the whine.pl script.
2.
3.
Actual Results:  
Whining mail is not genereating and we are getting the error message 


      " Can't use an undefined value as an ARRAY reference at ./ 
whine.pl line 253. 
DBI::db=HASH(0x1b73e10)->disconnect invalidates 1 active statement 
handle (either destroy statement handles or call finish on them before 
disconnecting) at Bugzilla.pm line 435 ". 


Expected Results:  
It should have generated the whining mail.

Comment 1

[Frédéric Buclin]

What's your version of DBD::mysql?

Comment 2

[Potri]

Our DBD-mysql version is v3.0002.

Comment 3

[Frédéric Buclin]

This is weird. It looks like an old bug, see https://bugzilla.mozilla.org/show_bug.cgi?id=349210#c28.

If you upgrade DBD::mysql to 4.00, can you still reproduce the bug? What's the output of checksetup.pl?

Comment 4

[Max Kanat-Alexander]

The actual error is:

Can't use an undefined value as an ARRAY reference at ./ 
whine.pl line 253. 

And that line is:

 my $groupname = $sth->fetch->[0];

In the following block:

                elsif ($mailto_type == MAILTO_GROUP) {
                    my $sth = $dbh->prepare("SELECT name FROM groups " .
                                            "WHERE id=?");
                    $sth->execute($mailto);
                    my $groupname = $sth->fetch->[0];
                    my $group_id = Bugzilla::Group::ValidateGroupName(
                        $groupname, $owner);

In other words, somebody deleted a group without deleting the associated whine, and we don't handle that properly. (Really, we should just be deleting the associated whine event, I'd think.)

Comment 5

[Frédéric Buclin]

This problem cannot happen by deleting a group from the UI. Entries in the whine_schedules table about deleted groups have always been deleted at the same time as groups themselves. So this probably means you deleted a group manually. But our code is fine and does the right job.

All I will do is to add a check to sanitycheck.cgi to catch those (rare) situations.

Comment 6

[Frédéric Buclin]

Attachment: patch, v1

Fix both deleted users and groups. Tested, works fine.

Comment 7

[Max Kanat-Alexander]

Comment on attachment 325049 [details] [diff] [review]
patch, v1

Say "users/groups" instead of targets in the displayed messages.

Otherwise this is fine.

sanitycheck is starting to become a bit bloated--we should figure out some way to fix that.

Comment 8

[Frédéric Buclin]

(In reply to comment #7)
> sanitycheck is starting to become a bit bloated--we should figure out some way
> to fix that.

This shouldn't be too hard. Now that we have foreign keys and triggers implemented, most checks can go away as deleting e.g. a bug must remove (or do some other action) all entries in other tables which point to that bug. I think sanitycheck.cgi no longer needs to do these checks, in Bugzilla 4.0.

Comment 9

[Frédéric Buclin]

tip:

Checking in sanitycheck.cgi;
/cvsroot/mozilla/webtools/bugzilla/sanitycheck.cgi,v  <--  sanitycheck.cgi
new revision: 1.143; previous revision: 1.142
done
Checking in template/en/default/admin/sanitycheck/messages.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/sanitycheck/messages.html.tmpl,v  <--  messages.html.tmpl
new revision: 1.6; previous revision: 1.5
done
Checking in template/en/default/admin/sanitycheck/messages.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/sanitycheck/messages.html.tmpl,v  <--  messages.html.tmpl
new revision: 1.7; previous revision: 1.6 (with the fix on checkin)
done


3.1.4:

Checking in sanitycheck.cgi;
/cvsroot/mozilla/webtools/bugzilla/sanitycheck.cgi,v  <--  sanitycheck.cgi
new revision: 1.140.2.3; previous revision: 1.140.2.2
done
Checking in template/en/default/admin/sanitycheck/messages.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/sanitycheck/messages.html.tmpl,v  <--  messages.html.tmpl
new revision: 1.4.2.2; previous revision: 1.4.2.1
done

Comment 10

[Potri]

Can anyone tell me the procedure about how to apply this sanitycheck.cgi patch to the existing bugzilla installation.Please let me know.

Comment 11

[Frédéric Buclin]

Leave this bug as FIXED! I checked in the patch into the source code, so don't reopen it. The patch can be applied as:

  patch -p0 -i name_of_the_patch.diff

from the bugzilla root directory.

Comment 12

[Potri]

       That's fine but can you please let me know how to install this patch and where to get the patch file and how to update the sanitycheck.cgi script. Please let me know so that I can proceed and solve the issue. Waiting for your favorable reply.

Comment 13

[Potri]

After installing the patch, we got the below mentioned checks added in the Bugzilla Sanity check but still I am getting the same error given below, not sure why I am getting the same error.

CHECKS :
whines_obsolete_target_start 

whines_obsolete_target_alert 

whines_obsolete_target_fix 

ERROR :

HC-TEST001:/srv/www/htdocs/usergroup/bugzilla/1 # ./whine.pl 
Can't use an undefined value as an ARRAY reference at ./whine.pl line 253.
DBI::db=HASH(0x1b73290)->disconnect invalidates 1 active statement handle (either destroy statement handles or call finish on them before disconnecting) at Bugzilla.pm line 435.

Comment 14

[Frédéric Buclin]

As I said in comment 11, leave this bug as FIXED! The patch has been checked in. And as I explained in comment 5, my patch doesn't fix the error reported in comment 0, which seems to be a problem unrelated to Bugzilla (but a problem with your versions of MySQL/DBD::mysql). So if you still have problems with whine.pl, and you can still reproduce with a newer version of DBD::mysql, file a *new* bug. But PLEASE leave this one alone. (no offend)

Comment 15

[Vitaly Fedrushkov]

Just a note: r- from l10n for:

      Non-existent [% (type == constants.MAILTO_USER) ? "user" : "group" FILTER html %]

This example ignores possible grammatic gender of 'non-existent' and may create l10n issue when we move to Maketext.  Explicit IFs make code more localizable with  current technology.

Comment 16

[Max Kanat-Alexander]

(In reply to comment #15)
> Explicit IFs make code more localizable with current technology.

  We can fix those issues after we move to maketext.