Bug 304745: Move GetFieldID() out of globals.pl - Patch by Frédéric Buclin <LpSolit@gmail.com> r=wicked a=justdave

9 files changed, 50 insertions, 30 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index dfa419316..3a80f4095 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -40,6 +40,7 @@ use Bugzilla::Attachment;
 use Bugzilla::BugMail;
 use Bugzilla::Config;
 use Bugzilla::Constants;
+use Bugzilla::Field;
 use Bugzilla::Flag;
 use Bugzilla::FlagType;
 use Bugzilla::User;
@@ -956,7 +957,7 @@ sub LogActivityEntry {
         }
         trick_taint($addstr);
         trick_taint($removestr);
-        my $fieldid = &::GetFieldID($col);
+        my $fieldid = get_field_id($col);
         $dbh->do("INSERT INTO bugs_activity
                   (bug_id, who, bug_when, fieldid, removed, added)
                   VALUES (?, ?, ?, ?, ?, ?)",
@@ -1106,7 +1107,7 @@ sub CheckIfVotedConfirmed {
     my $ret = 0;
     if ($votes >= $votestoconfirm && !$everconfirmed) {
         if ($status eq 'UNCONFIRMED') {
-            my $fieldid = &::GetFieldID("bug_status");
+            my $fieldid = get_field_id("bug_status");
             $dbh->do("UPDATE bugs SET bug_status = 'NEW', everconfirmed = 1, " .
                      "delta_ts = ? WHERE bug_id = ?",
                      undef, ($timestamp, $id));
@@ -1120,7 +1121,7 @@ sub CheckIfVotedConfirmed {
                      "WHERE bug_id = ?", undef, ($timestamp, $id));
         }
 
-        my $fieldid = &::GetFieldID("everconfirmed");
+        my $fieldid = get_field_id("everconfirmed");
         $dbh->do("INSERT INTO bugs_activity " .
                  "(bug_id, who, bug_when, fieldid, removed, added) " .
                  "VALUES (?, ?, ?, ?, ?, ?)",
diff --git a/Bugzilla/Field.pm b/Bugzilla/Field.pm
index 26265ac3d..09c4731ac 100644
--- a/Bugzilla/Field.pm
+++ b/Bugzilla/Field.pm
@@ -20,7 +20,8 @@ package Bugzilla::Field;
 use strict;
 
 use base qw(Exporter);
-@Bugzilla::Field::EXPORT = qw(check_form_field check_form_field_defined);
+@Bugzilla::Field::EXPORT = qw(check_form_field check_form_field_defined
+                              get_field_id);
 
 use Bugzilla::Util;
 use Bugzilla::Error;
@@ -52,6 +53,22 @@ sub check_form_field_defined {
     }
 }
 
+sub get_field_id {
+    my ($name) = @_;
+    my $dbh = Bugzilla->dbh;
+
+    trick_taint($name);
+    my $id = $dbh->selectrow_array('SELECT fieldid FROM fielddefs
+                                    WHERE name = ?', undef, $name);
+
+    ThrowCodeError('invalid_field_name', {field => $name}) unless $id;
+    return $id
+}
+
+1;
+
+__END__
+
 =head1 NAME
 
 Bugzilla::Field - Useful routines for fields manipulation
@@ -63,7 +80,7 @@ Bugzilla::Field - Useful routines for fields manipulation
   # Validation Routines
   check_form_field($cgi, $fieldname, \@legal_values);
   check_form_field_defined($cgi, $fieldname);
-
+  $fieldid = get_field_id($fieldname);
 
 =head1 DESCRIPTION
 
@@ -101,4 +118,14 @@ Params:      $cgi       - a CGI object
 
 Returns:     nothing
 
+=item C<get_field_id($fieldname)>
+
+Description: Returns the ID of the specified field name and throws
+             an error if this field does not exist.
+
+Params:      $fieldname - a field name
+
+Returns:     the corresponding field ID or an error if the field name
+             does not exist.
+
 =back
diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm
index 89dda08a5..34ded7dd2 100644
--- a/Bugzilla/Flag.pm
+++ b/Bugzilla/Flag.pm
@@ -72,6 +72,7 @@ use Bugzilla::Error;
 use Bugzilla::Attachment;
 use Bugzilla::BugMail;
 use Bugzilla::Constants;
+use Bugzilla::Field;
 
 # Note that this line doesn't actually import these variables for some reason,
 # so I have to use them as $::template and $::vars in the package code.
@@ -500,7 +501,7 @@ sub update_activity {
     if ($removed ne $added) {
         my $sql_removed = &::SqlQuote($removed);
         my $sql_added = &::SqlQuote($added);
-        my $field_id = &::GetFieldID('flagtypes.name');
+        my $field_id = get_field_id('flagtypes.name');
         $dbh->do("INSERT INTO bugs_activity
                   (bug_id, attach_id, who, bug_when, fieldid, removed, added)
                   VALUES ($bug_id, $attach_id, $::userid, $timestamp,
diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index 42f6da749..55113f641 100644
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -44,6 +44,7 @@ use Bugzilla::Util;
 use Bugzilla::Constants;
 use Bugzilla::Group;
 use Bugzilla::User;
+use Bugzilla::Field;
 
 use Date::Format;
 use Date::Parse;
@@ -312,7 +313,7 @@ sub init {
                     push(@l, "bugs.creation_ts <= $sql_chto") if($sql_chto);
                     $bug_creation_clause = "(" . join(' AND ', @l) . ")";
                 } else {
-                    push(@list, "\nactcheck.fieldid = " . &::GetFieldID($f));
+                    push(@list, "\nactcheck.fieldid = " . get_field_id($f));
                 }
             }
 
@@ -998,7 +999,7 @@ sub init {
                 }
                 my $cutoff = "NOW() - " .
                              $dbh->sql_interval("$quantity $unitinterval");
-                my $assigned_fieldid = &::GetFieldID('assigned_to');
+                my $assigned_fieldid = get_field_id('assigned_to');
                 push(@supptables, "LEFT JOIN longdescs AS comment_$table " .
                                   "ON comment_$table.who = bugs.assigned_to " .
                                   "AND comment_$table.bug_id = bugs.bug_id " .
diff --git a/attachment.cgi b/attachment.cgi
index c406516a3..67272ae50 100755
--- a/attachment.cgi
+++ b/attachment.cgi
@@ -48,6 +48,7 @@ use Bugzilla::FlagType;
 use Bugzilla::User;
 use Bugzilla::Util;
 use Bugzilla::Bug;
+use Bugzilla::Field;
 
 Bugzilla->login();
 
@@ -1008,7 +1009,7 @@ sub insert
   AppendComment($bugid, $userid, $comment, $isprivate, $timestamp);
 
   # Make existing attachments obsolete.
-  my $fieldid = GetFieldID('attachments.isobsolete');
+  my $fieldid = get_field_id('attachments.isobsolete');
   foreach my $obsolete_id (@obsolete_ids) {
       # If the obsolete attachment has request flags, cancel them.
       # This call must be done before updating the 'attachments' table.
@@ -1057,7 +1058,7 @@ sub insert
       # Add the changes to the bugs_activity table
       for (my $i = 0; $i < 3; $i++) {
           if ($oldvalues[$i] ne $newvalues[$i]) {
-              my $fieldid = GetFieldID($fields[$i]);
+              my $fieldid = get_field_id($fields[$i]);
               SendSQL("INSERT INTO bugs_activity " .
                       "(bug_id, who, bug_when, fieldid, removed, added) " .
                       "VALUES ($bugid, $userid, $sql_timestamp, " .
@@ -1233,7 +1234,7 @@ sub update
   my $sql_timestamp = SqlQuote($timestamp);
   if ($olddescription ne $cgi->param('description')) {
     my $quotedolddescription = SqlQuote($olddescription);
-    my $fieldid = GetFieldID('attachments.description');
+    my $fieldid = get_field_id('attachments.description');
     SendSQL("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when,
                                         fieldid, removed, added)
              VALUES ($bugid, $attach_id, $userid, $sql_timestamp, $fieldid,
@@ -1241,7 +1242,7 @@ sub update
   }
   if ($oldcontenttype ne $cgi->param('contenttype')) {
     my $quotedoldcontenttype = SqlQuote($oldcontenttype);
-    my $fieldid = GetFieldID('attachments.mimetype');
+    my $fieldid = get_field_id('attachments.mimetype');
     SendSQL("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when,
                                         fieldid, removed, added)
              VALUES ($bugid, $attach_id, $userid, $sql_timestamp, $fieldid,
@@ -1249,28 +1250,28 @@ sub update
   }
   if ($oldfilename ne $cgi->param('filename')) {
     my $quotedoldfilename = SqlQuote($oldfilename);
-    my $fieldid = GetFieldID('attachments.filename');
+    my $fieldid = get_field_id('attachments.filename');
     SendSQL("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when,
                                         fieldid, removed, added)
              VALUES ($bugid, $attach_id, $userid, $sql_timestamp, $fieldid,
                      $quotedoldfilename, $quotedfilename)");
   }
   if ($oldispatch ne $cgi->param('ispatch')) {
-    my $fieldid = GetFieldID('attachments.ispatch');
+    my $fieldid = get_field_id('attachments.ispatch');
     SendSQL("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when,
                                         fieldid, removed, added)
              VALUES ($bugid, $attach_id, $userid, $sql_timestamp, $fieldid,
                      $oldispatch, " . $cgi->param('ispatch') . ")");
   }
   if ($oldisobsolete ne $cgi->param('isobsolete')) {
-    my $fieldid = GetFieldID('attachments.isobsolete');
+    my $fieldid = get_field_id('attachments.isobsolete');
     SendSQL("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when,
                                         fieldid, removed, added)
              VALUES ($bugid, $attach_id, $userid, $sql_timestamp, $fieldid,
                      $oldisobsolete, " . $cgi->param('isobsolete') . ")");
   }
   if ($oldisprivate ne $cgi->param('isprivate')) {
-    my $fieldid = GetFieldID('attachments.isprivate');
+    my $fieldid = get_field_id('attachments.isprivate');
     SendSQL("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when,
                                         fieldid, removed, added)
              VALUES ($bugid, $attach_id, $userid, $sql_timestamp, $fieldid,
diff --git a/checksetup.pl b/checksetup.pl
index b37d6e414..13d5e0782 100755
--- a/checksetup.pl
+++ b/checksetup.pl
@@ -1677,7 +1677,7 @@ sub AddFDef {
 }
 
 
-# Note that all of these entries are unconditional, from when GetFieldID
+# Note that all of these entries are unconditional, from when get_field_id
 # used to create an entry if it wasn't found. New fielddef columns should
 # be created with their associated schema change.
 AddFDef("bug_id", "Bug \#", 1);
diff --git a/editusers.cgi b/editusers.cgi
index e6d4fffe8..ade2f0ca7 100755
--- a/editusers.cgi
+++ b/editusers.cgi
@@ -30,6 +30,7 @@ use Bugzilla::Flag;
 use Bugzilla::Config;
 use Bugzilla::Constants;
 use Bugzilla::Util;
+use Bugzilla::Field;
 
 Bugzilla->login(LOGIN_REQUIRED);
 
@@ -377,7 +378,7 @@ if ($action eq 'search') {
                    },
                  undef,
                  ($otherUserID, $userid,
-                  GetFieldID('bug_group'),
+                  get_field_id('bug_group'),
                   join(', ', @groupsRemovedFrom), join(', ', @groupsAddedTo)));
         $dbh->do('UPDATE profiles SET refreshed_when=? WHERE userid = ?',
                  undef, ('1900-01-01 00:00:00', $otherUserID));
diff --git a/globals.pl b/globals.pl
index 46ceebc74..7605ffe0f 100644
--- a/globals.pl
+++ b/globals.pl
@@ -108,14 +108,6 @@ $::SIG{PIPE} = 'IGNORE';
 #}
 #$::SIG{__DIE__} = \&die_with_dignity;
 
-sub GetFieldID {
-    my ($f) = (@_);
-    SendSQL("SELECT fieldid FROM fielddefs WHERE name = " . SqlQuote($f));
-    my $fieldid = FetchOneColumn();
-    die "Unknown field id: $f" if !$fieldid;
-    return $fieldid;
-}
-
 # XXXX - this needs to go away
 sub GenerateVersionTable {
     my $dbh = Bugzilla->dbh;
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 53fdcc59f..b339ccc85 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -641,10 +641,6 @@
     The context [% context FILTER html %] is invalid (must be a number,
     "file" or "patch").
 
-  [% ELSIF error == "invalid_field_name" %]
-    [% title = "Invalid Field Name" %]
-    The field "[% name FILTER html %]" is invalid.
-
   [% ELSIF error == "invalid_format" %]
     [% title = "Invalid Format" %]
     The format "[% format FILTER html %]" is invalid (must be one of