300549 - Eliminate deprecated Bugzilla::DB routines from Flag.pm and FlagType.pm

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, enhancement)

Description

[Frédéric Buclin]

.

Comment 1

[Frédéric Buclin]

Deprecated DB routines have been removed from Attachment.pm thanks to myk's
patch in bug 302669. Updating the summary accordingly.

Comment 2

[Bradley Baetz (:bbaetz)]

Duping to the bug I have a patch for.... 

*** This bug has been marked as a duplicate of 317149 ***

Comment 3

[Frédéric Buclin]

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

Comment 4

[Frédéric Buclin]

Attachment: patch, v1

As future(?) improvements, we could try to:
- force sqlify_criteria() to separate the SQL fragment from values and to return them separately;
- convert the column list in FlagType.pm to be a constant, but this would involve a lot of work due to the way sqlify_criteria() works, i.e. altering this list depending on your requests.

Anyway, as I plan to rewrite Flag.pm and FlagType.pm in bug 304699 very soon now (to use real flag objects), I won't waste too much time on it here.

Comment 5

[Teemu Mannermaa (:wicked)]

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

Looks ok, passes tests and atleast seems to work on few simple flag related changes I did.

>Index: Bugzilla/Flag.pm
>===================================================================
>@@ -192,7 +149,7 @@
> =item C<count($criteria)>
>-Queries the database for flags matching the given criteria 
>+Queries the database for flags matching the given criteria
> (specified as a hash of field names and their matching values)
> and returns an array of matching records.

Nit: Correct rest of the comment while you are at it. Surely this doesn't return an array of matches..

>+        $dbh->do('UPDATE bugs SET delta_ts = ? WHERE bug_id = ?',
>+                  undef, $timestamp, $bug_id);

Nit: Multiple values, so add brackets like in others.

>Index: Bugzilla/FlagType.pm
>===================================================================
>-    my $select_clause = "SELECT " . join(", ", @base_columns);
>-    my $from_clause = "FROM " . join(" ", @base_tables);
>+    my $columns = join(", ", @base_columns);
>+    my @data = $dbh->selectrow_array("SELECT $columns FROM flagtypes

Nit: Since base_tables has not removed from other places, don't remove it from here either. This is for consistency in case the list is changed later.

>Index: sanitycheck.cgi
>===================================================================
>-my @open_states = map(SqlQuote($_), BUG_STATE_OPEN);
>+my @open_states = map($dbh->quote($_), BUG_STATE_OPEN);

Oops, sorry about letting that SqlQuote in there. :)

Comment 6

[Frédéric Buclin]

Checking in attachment.cgi;
/cvsroot/mozilla/webtools/bugzilla/attachment.cgi,v  <--  attachment.cgi
new revision: 1.109; previous revision: 1.108
done
Checking in editusers.cgi;
/cvsroot/mozilla/webtools/bugzilla/editusers.cgi,v  <--  editusers.cgi
new revision: 1.120; previous revision: 1.119
done
Checking in sanitycheck.cgi;
/cvsroot/mozilla/webtools/bugzilla/sanitycheck.cgi,v  <--  sanitycheck.cgi
new revision: 1.113; previous revision: 1.112
done
Checking in Bugzilla/Flag.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Flag.pm,v  <--  Flag.pm
new revision: 1.63; previous revision: 1.62
done
Checking in Bugzilla/FlagType.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/FlagType.pm,v  <--  FlagType.pm
new revision: 1.26; previous revision: 1.25
done