240250 - Editing groupset for a product that doesn't exist gives an unfriendly error

Status: RESOLVED FIXED

(Bugzilla :: Administration, task)

Description

[timeless]

Software error:

DBD::mysql::st execute failed: You have an error in your SQL syntax.  Check the
manual that corresponds to your MySQL server version for the right syntax to use
near 'JOIN bug_group_map ON bug_group_map.group_id = groups.id LEFT J [for
statement ``SELECT id, name, entry, membercontrol, othercontrol, canedit,
isactive, COUNT(bugs.bug_id) FROM groups LEFT JOIN group_control_map ON
group_control_map.group_id = id AND group_control_map.product_id =  LEFT JOIN
bug_group_map ON bug_group_map.group_id = groups.id LEFT JOIN bugs ON
bugs.bug_id = bug_group_map.bug_id AND bugs.product_id =  WHERE isbuggroup != 0
AND (isactive != 0 OR entry IS NOT NULL OR bugs.bug_id IS NOT NULL) GROUP BY
name'']) at Bugzilla/DB.pm line 66
	Bugzilla::DB::SendSQL('SELECT id, name, entry, membercontrol, othercontrol,
canedit, is...') called at /opt/webtools/bugzilla/editproducts.cgi line 1281

For help, please send mail to the webmaster ([e-mail]), giving this
error message and the time and date of the error.

Comment 1

[Frédéric Buclin]

*** Bug 271590 has been marked as a duplicate of this bug. ***

Comment 2

[Frédéric Buclin]

I already have a patch for this... taking!

Comment 3

[Frédéric Buclin]

*** Bug 276831 has been marked as a duplicate of this bug. ***

Comment 4

[Frédéric Buclin]

Attachment: Complain if the product does not exist, v1

trivial patch. It checks if the product exists and if not, it displays a
warning. Should we make a distinction (in the error message) if no product is
given? timeless will answer "yes" for sure, but do we really need to make a
distinction as this requires to hack the URL and remove (or forget) the
&product=XXX part of it?

Comment 5

[timeless]

well, in one case you omitted a required item.
in another you almost certainly tried to follow a link from what used to be a
valid item. the cases really are different.

note that in the former case, i'd rather be given a list from which to choose an
item.

Comment 6

[Max Kanat-Alexander]

Comment on attachment 170182 [details] [diff] [review]
Complain if the product does not exist, v1

>+    $product_id
>+      || ThrowUserError("invalid_product_name", { product => $product });

I would have written that:

unless($product_id) { 
    ThrowUserError 
}

That's how all our other checks like this work in our modern code, as far as I
know, and it's clearer. If you write another patch that uses that syntax
instead, I'll be happy.

I think it's fine to have one check for for invalid and nonexistent -- that's
how we do it for everything else, AFAIK.

You could always add an "IF product" in the template, but maybe that's another
bug.

This is a software error, so we really should get rid of it.

Comment 7

[Vlad Dascalu]

Checking in editproducts.cgi;
/cvsroot/mozilla/webtools/bugzilla/editproducts.cgi,v  <--  editproducts.cgi
new revision: 1.53.2.5; previous revision: 1.53.2.4
done

Checking in editproducts.cgi;
/cvsroot/mozilla/webtools/bugzilla/editproducts.cgi,v  <--  editproducts.cgi
new revision: 1.62; previous revision: 1.61
done