Closed
Bug 303706
Opened 19 years ago
Closed 19 years ago
Eliminate deprecated Bugzilla::DB routines from editproducts.cgi
Categories
(Bugzilla :: Bugzilla-General, defect)
Tracking
()
RESOLVED
FIXED
Bugzilla 2.22
People
(Reporter: wicked, Assigned: LpSolit)
References
Details
Attachments
(1 file, 2 obsolete files)
|
31.25 KB,
patch
|
wicked
:
review+
|
Details | Diff | Splinter Review |
These lines need to be rewritten to use DBI:
editproducts.cgi:62: SendSQL("SELECT name
editproducts.cgi:64: WHERE name=" . SqlQuote($prod));
editproducts.cgi:65: return FetchOneColumn();
editproducts.cgi:94: SendSQL("SELECT name
editproducts.cgi:96: WHERE name=" . SqlQuote($cl));
editproducts.cgi:97: return FetchOneColumn();
editproducts.cgi:513: SendSQL("INSERT INTO products ( " .
editproducts.cgi:517: SqlQuote($product) . "," .
editproducts.cgi:518: SqlQuote($description) . "," .
editproducts.cgi:519: SqlQuote($milestoneurl) . "," .
editproducts.cgi:523: # and $votestoconfirm w/ SqlQuote()
editproducts.cgi:525: SqlQuote($disallownew) . "," .
editproducts.cgi:526: SqlQuote($votesperuser) . "," .
editproducts.cgi:527: SqlQuote($maxvotesperbug) . "," .
editproducts.cgi:528: SqlQuote($votestoconfirm) . "," .
editproducts.cgi:529: SqlQuote($defaultmilestone) . "," .
editproducts.cgi:530: SqlQuote($classification_id) . ")");
editproducts.cgi:533: SendSQL("INSERT INTO versions ( " .
editproducts.cgi:536: SqlQuote($version) . "," .
editproducts.cgi:539: SendSQL("INSERT INTO milestones (product_id, value)
VALUES (" .
editproducts.cgi:540: $product_id . ", " .
SqlQuote($defaultmilestone) . ")");
editproducts.cgi:550: SendSQL("INSERT INTO groups " .
editproducts.cgi:553: SqlQuote($productgroup) . ", " .
editproducts.cgi:554: SqlQuote("Access to bugs in the $product
product") . ", 1, NOW())");
editproducts.cgi:559: SendSQL("INSERT INTO group_group_map (member_id,
grantor_id, grant_type)
editproducts.cgi:561: SendSQL("INSERT INTO group_group_map (member_id,
grantor_id, grant_type)
editproducts.cgi:563: SendSQL("INSERT INTO group_group_map (member_id,
grantor_id, grant_type)
editproducts.cgi:567: SendSQL("INSERT INTO group_control_map " .
editproducts.cgi:801: SendSQL("SELECT classifications.description,
editproducts.cgi:805: WHERE products.name=" . SqlQuote($product) .
editproducts.cgi:806: " AND classifications.id=" .
SqlQuote($classification_id));
editproducts.cgi:809: FetchSQLData();
editproducts.cgi:821: SendSQL("SELECT name,description
editproducts.cgi:824: if (MoreSQLData()) {
editproducts.cgi:826: while ( MoreSQLData() ) {
editproducts.cgi:827: my ($component, $description) = FetchSQLData();
editproducts.cgi:841: SendSQL("SELECT value
editproducts.cgi:845: if (MoreSQLData()) {
editproducts.cgi:847: while ( MoreSQLData() ) {
editproducts.cgi:848: my ($version) = FetchSQLData();
editproducts.cgi:864: SendSQL("SELECT value
editproducts.cgi:868: if(MoreSQLData()) {
editproducts.cgi:870: while ( MoreSQLData() ) {
editproducts.cgi:871: my ($milestone) = FetchSQLData();
editproducts.cgi:884: SendSQL("SELECT id, name, isactive, entry,
membercontrol, othercontrol, canedit " .
editproducts.cgi:889: while (MoreSQLData()) {
editproducts.cgi:891: = FetchSQLData();
editproducts.cgi:905: SendSQL("SELECT count(bug_id), product_id
editproducts.cgi:910: $bugs = FetchOneColumn() if MoreSQLData();
editproducts.cgi:961: SendSQL("SELECT groups.name, COUNT(bugs.bug_id)
editproducts.cgi:968: while (MoreSQLData()) {
editproducts.cgi:969: my ($groupname, $bugcount) = FetchSQLData();
editproducts.cgi:979: SendSQL("SELECT groups.name, COUNT(bugs.bug_id)
editproducts.cgi:989: while (MoreSQLData()) {
editproducts.cgi:990: my ($groupname, $bugcount) = FetchSQLData();
editproducts.cgi:1008: SendSQL("SELECT id, name FROM groups " .
editproducts.cgi:1010: while (MoreSQLData()){
editproducts.cgi:1011: my ($groupid, $groupname) = FetchSQLData();
editproducts.cgi:1036: SendSQL("SELECT id, name, entry, membercontrol,
othercontrol, canedit " .
editproducts.cgi:1041: while (MoreSQLData()) {
editproducts.cgi:1043: $othercontrol, $canedit) = FetchSQLData();
editproducts.cgi:1059: PushGlobalSQLState();
editproducts.cgi:1060: SendSQL("INSERT INTO group_control_map " .
editproducts.cgi:1066: PopGlobalSQLState();
editproducts.cgi:1071: PushGlobalSQLState();
editproducts.cgi:1072: SendSQL("UPDATE group_control_map " .
editproducts.cgi:1079: PopGlobalSQLState();
editproducts.cgi:1084: PushGlobalSQLState();
editproducts.cgi:1085: SendSQL("DELETE FROM group_control_map " .
editproducts.cgi:1088: PopGlobalSQLState();
editproducts.cgi:1096: SendSQL("SELECT bugs.bug_id,
editproducts.cgi:1103: while (MoreSQLData()) {
editproducts.cgi:1104: my ($bugid, $mailiscurrent) = FetchSQLData();
editproducts.cgi:1105: PushGlobalSQLState();
editproducts.cgi:1106: SendSQL("DELETE FROM bug_group_map WHERE
editproducts.cgi:1108: SendSQL("SELECT name, NOW() FROM groups WHERE
id = $groupid");
editproducts.cgi:1109: my ($removed, $timestamp) = FetchSQLData();
editproducts.cgi:1114: $diffed = ", lastdiffed = " .
SqlQuote($timestamp);
editproducts.cgi:1116: SendSQL("UPDATE bugs SET delta_ts = " .
SqlQuote($timestamp) .
editproducts.cgi:1118: PopGlobalSQLState();
editproducts.cgi:1128: SendSQL("SELECT bugs.bug_id,
editproducts.cgi:1137: while (MoreSQLData()) {
editproducts.cgi:1138: my ($bugid, $mailiscurrent) = FetchSQLData();
editproducts.cgi:1139: PushGlobalSQLState();
editproducts.cgi:1140: SendSQL("INSERT INTO bug_group_map (bug_id,
group_id)
editproducts.cgi:1142: SendSQL("SELECT name, NOW() FROM groups WHERE
id = $groupid");
editproducts.cgi:1143: my ($added, $timestamp) = FetchSQLData();
editproducts.cgi:1148: $diffed = ", lastdiffed = " .
SqlQuote($timestamp);
editproducts.cgi:1150: SendSQL("UPDATE bugs SET delta_ts = " .
SqlQuote($timestamp) .
editproducts.cgi:1152: PopGlobalSQLState();
editproducts.cgi:1223: SendSQL("UPDATE products
editproducts.cgi:1236: SendSQL("UPDATE products
editproducts.cgi:1237: SET description=" .
SqlQuote($description) . "
editproducts.cgi:1243: SendSQL("UPDATE products
editproducts.cgi:1244: SET milestoneurl=" .
SqlQuote($milestoneurl) . "
editproducts.cgi:1251: SendSQL("UPDATE products
editproducts.cgi:1260: SendSQL("UPDATE products
editproducts.cgi:1269: SendSQL("UPDATE products
editproducts.cgi:1278: SendSQL("SELECT value FROM milestones " .
editproducts.cgi:1279: "WHERE value = " .
SqlQuote($defaultmilestone) .
editproducts.cgi:1281: if (!FetchOneColumn()) {
editproducts.cgi:1287: SendSQL("UPDATE products " .
editproducts.cgi:1288: "SET defaultmilestone = " .
SqlQuote($defaultmilestone) .
editproducts.cgi:1293: my $qp = SqlQuote($product);
editproducts.cgi:1294: my $qpold = SqlQuote($productold);
editproducts.cgi:1312: SendSQL("UPDATE products SET name=$qp WHERE
id=$product_id");
editproducts.cgi:1322: SendSQL("SELECT votes.who, votes.bug_id " .
editproducts.cgi:1328: while (MoreSQLData()) {
editproducts.cgi:1329: my ($who, $id) = (FetchSQLData());
editproducts.cgi:1345: SendSQL("SELECT votes.who, votes.vote_count FROM
votes, bugs " .
editproducts.cgi:1349: while (MoreSQLData()) {
editproducts.cgi:1350: my ($who, $count) = (FetchSQLData());
editproducts.cgi:1359: SendSQL("SELECT votes.bug_id FROM votes,
bugs " .
editproducts.cgi:1363: while (MoreSQLData()) {
editproducts.cgi:1364: my ($id) = FetchSQLData();
editproducts.cgi:1373: SendSQL("SELECT bug_id FROM bugs " .
editproducts.cgi:1377: if (MoreSQLData()) {
editproducts.cgi:1380: while (MoreSQLData()) {
editproducts.cgi:1382: CheckIfVotedConfirmed(FetchOneColumn(),
Bugzilla->user->id);
editproducts.cgi:1399: SendSQL("SELECT id, name, entry, membercontrol,
othercontrol, canedit, " .
editproducts.cgi:1416: while (MoreSQLData()) {
editproducts.cgi:1419: $canedit, $isactive, $bugcount) = FetchSQLData();
|
Assignee | |
Updated•19 years ago
|
Assignee: general → LpSolit
Target Milestone: --- → Bugzilla 2.22
|
|
Assignee | |
Comment 1•19 years ago
|
||
Attachment #199954 -
Flags: review?(wicked)
|
|
Assignee | |
Comment 2•19 years ago
|
||
unbitrotten patch
Attachment #199954 -
Attachment is obsolete: true
Attachment #200678 -
Flags: review?(wicked)
Attachment #199954 -
Flags: review?(wicked)
|
Reporter | |
Comment 3•19 years ago
|
||
Comment on attachment 200678 [details] [diff] [review] patch, v1.1 >Index: editproducts.cgi >=================================================================== >+ my $group_description = "Access to bugs in the " . $product->name . " product"; Nit: Just exceeds 80 chars so it would be nice if you move last part to second line. > my $gid = $dbh->bz_last_key('groups', 'id'); > my $admin = GroupNameToId('admin'); > # If we created a new group, give the "admin" group priviledges > # initially. Move this comment before these two variable definitions (or atleast $admin var) and add blank before but not after the comment to better associate it with the following code block. >+ my $sth_Select = $dbh->prepare('SELECT bugs.bug_id, >+ CASE WHEN (lastdiffed >= delta_ts) THEN 1 ELSE 0 END >+ FROM bugs >+ INNER JOIN bug_group_map >+ ON bug_group_map.bug_id = bugs.bug_id >+ WHERE group_id = ? >+ AND bugs.product_id = ? >+ ORDER BY bugs.bug_id'); Nit: This would probably look better if you start SELECT from the next line and decrease intendation so that CASE WHEN fits into one row. >+ $sth_Select->execute($groupid, $product->id); >+ my $bugs = $sth_Select->fetchall_arrayref; You should combine these two lines by using selectall_arrayref with first parameter as the prepared statement handle. >+ $sth_Select2->execute($groupid); >+ my ($removed, $timestamp) = $sth_Select2->fetchrow_array; Same here but with selectrow_array. >+ $sth_Select = $dbh->prepare('SELECT bugs.bug_id, >+ CASE WHEN (lastdiffed >= delta_ts) THEN 1 ELSE 0 END Nit: Fix intendation here too by moving SELECT to next line. >+ $sth_Select->execute($groupid, $product->id); >+ my $bugs = $sth_Select->fetchall_arrayref; Combine these two lines too. >+ $sth_Select2->execute($groupid); >+ my ($added, $timestamp) = $sth_Select2->fetchrow_array; And here too. >+ my $votes = $dbh->selectall_arrayref('SELECT votes.who, votes.bug_id >+ FROM votes >+ INNER JOIN bugs >+ ON bugs.bug_id = votes.bug_id >+ WHERE bugs.product_id = ? >+ AND votes.vote_count > ?', >+ undef, ($product->id, $maxvotesperbug)); Definedly reintent this one. >+ RemoveVotes($id, $who, "The rules for voting on this product has changed;\n" . >+ "you had too many votes for a single bug."); Nit: This string should also be moved to next lines to make it fit inside 80 chars. >+ my $bug_ids = $dbh->selectcol_arrayref('SELECT votes.bug_id >+ FROM votes >+ INNER JOIN bugs >+ ON bugs.bug_id = votes.bug_id >+ WHERE bugs.product_id = ? >+ AND votes.who = ?', >+ undef, ($product->id, $who)); Please, reintend these SQL lines. >+ RemoveVotes($bug_id, $who, "The rules for voting on this product " . >+ "has changed; you had too many\ntotal votes, so all " . >+ "votes have been removed."); Nit: And this string too.
Attachment #200678 -
Flags: review?(wicked) → review-
|
|
Assignee | |
Comment 4•19 years ago
|
||
Attachment #200678 -
Attachment is obsolete: true
Attachment #200910 -
Flags: review?(wicked)
|
|
Reporter | |
Comment 5•19 years ago
|
||
Comment on attachment 200910 [details] [diff] [review] patch, v2 >Index: editproducts.cgi >=================================================================== >+ $mandatory_groups = $dbh->selectall_arrayref( >+ 'SELECT groups.name, COUNT(bugs.bug_id) AS count > FROM bugs > LEFT JOIN bug_group_map > ON bug_group_map.bug_id = bugs.bug_id > INNER JOIN groups > ON bug_group_map.group_id = groups.id >+ WHERE groups.id IN (' . join(', ', @now_mandatory) . ') >+ AND bugs.product_id = ? >+ AND bug_group_map.bug_id IS NULL ' . >+ $dbh->sql_group_by('groups.name'), >+ {'Slice' => {}}, $product->id); >+ if (($na_groups && scalar(@$na_groups)) >+ || ($mandatory_groups && scalar(@$mandatory_groups))) >+ { For some reason, this doesn't show confirm screen when bugs will be added to mandatory groups. The addition itself works later in the code, though. Also, this happens without this patch so the problem was introduced earlier. > # 2. too many total votes for a single user. > # This part doesn't work in the general case because RemoveVotes > # doesn't enforce votesperuser (except per-bug when it's less >- # than maxvotesperbug). See RemoveVotes in globals.pl. >+ # than maxvotesperbug). See Bugzilla::Bug::RemoveVotes(). Couldn't test the changes for this logic. But looks like it's a known bug that this doesn't fully work so it should also be fixed in another bug. Otherwise, patch looks good and passes my tests so r=wicked.
Attachment #200910 -
Flags: review?(wicked) → review+
|
|
Reporter | |
Updated•19 years ago
|
Flags: approval?
|
||
Updated•19 years ago
|
Flags: approval? → approval+
|
|
Assignee | |
Comment 6•19 years ago
|
||
Checking in editproducts.cgi; /cvsroot/mozilla/webtools/bugzilla/editproducts.cgi,v <-- editproducts.cgi new revision: 1.105; previous revision: 1.104 done
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•