Bug 577805: Make chart_fields into an accessor in Search.pm

r=mkanat, a=mkanat (module owner)

1 files changed, 29 insertions, 23 deletions

diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index daec751d2..ab6c38a1e 100644
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -434,6 +434,22 @@ sub REPORT_COLUMNS {
 # Internal Accessors #
 ######################
 
+sub _chart_fields {
+    my ($self) = @_;
+
+    if (!$self->{chart_fields}) {
+        my $chart_fields = Bugzilla->fields({ by_name => 1 });
+
+        if (!Bugzilla->user->is_timetracker) {
+            foreach my $tt_field (TIMETRACKING_FIELDS) {
+                delete $chart_fields->{$tt_field};
+            }
+        }
+        $self->{chart_fields} = $chart_fields;
+    }
+    return $self->{chart_fields};
+}
+
 sub _multi_select_fields {
     my ($self) = @_;
     $self->{multi_select_fields} ||= Bugzilla->fields({
@@ -753,7 +769,7 @@ sub init {
                     }
                     $bug_creation_clause = "(" . join(' AND ', @l) . ")";
                 } else {
-                    push(@actlist, get_field_id($f));
+                    push(@actlist, $self->_chart_fields->{$f}->id);
                 }
             }
 
@@ -956,15 +972,6 @@ sub init {
 #               chart to merge the ON sections of each.
 # $suppstring = String which is pasted into query containing all table names
 
-    # get a list of field names to verify the user-submitted chart fields against
-    my $chart_fields = Bugzilla->fields({ by_name => 1 });
-
-    if (!$user->is_timetracker) {
-        foreach my $tt_field (TIMETRACKING_FIELDS) {
-            delete $chart_fields->{$tt_field};
-        }
-    }
-
     my ($sequence, $chartid);
     $row = 0;
     for ($chart=-1 ;
@@ -992,12 +999,12 @@ sub init {
 
                 # chart -1 is generated by other code above, not from the user-
                 # submitted form, so we'll blindly accept any values in chart -1
-                if (!$chart_fields->{$field} and $chart != -1) {
+                if (!$self->_chart_fields->{$field} and $chart != -1) {
                     ThrowCodeError("invalid_field_name", { field => $field });
                 }
 
                 # This is either from the internal chart (in which case we
-                # already know about it), or it was in $chart_fields, so it is
+                # already know about it), or it was in _chart_fields, so it is
                 # a valid field name, which means that it's ok.
                 trick_taint($field);
                 my $quoted = $dbh->quote($value);
@@ -1017,7 +1024,6 @@ sub init {
                     having       => \@having,
                     group_by     => \@groupby,
                     fields       => \@fields,
-                    chart_fields => $chart_fields,
                 );
                 # This should add a "term" selement to %search_args.
                 $self->do_search_function(\%search_args);
@@ -2177,7 +2183,7 @@ sub _owner_idle_time_greater_less {
     push(@$joins, $comments_join);
 
     my $act_table = "activity_$table";
-    my $assigned_fieldid = get_field_id('assigned_to');
+    my $assigned_fieldid = $self->_chart_fields->{'assigned_to'}->id;
 
     # XXX Why are we joining using $assignedto_fieldid here? It shouldn't
     #     matter when or if the assignee changed.
@@ -2377,13 +2383,13 @@ sub _nowords {
 
 sub _changedbefore_changedafter {
     my ($self, $args) = @_;
-    my ($chart_id, $joins, $field, $operator, $value, $chart_fields) =
-        @$args{qw(chart_id joins field operator value chart_fields)};
+    my ($chart_id, $joins, $field, $operator, $value) =
+        @$args{qw(chart_id joins field operator value)};
     my $dbh = Bugzilla->dbh;
     
     my $sql_operator = ($operator =~ /before/) ? '<' : '>';
     my $table = "act_$chart_id";
-    my $field_object = $chart_fields->{$field}
+    my $field_object = $self->_chart_fields->{$field}
         || ThrowCodeError("invalid_field_name", { field => $field });
     my $field_id = $field_object->id;
     
@@ -2398,12 +2404,12 @@ sub _changedbefore_changedafter {
 
 sub _changedfrom_changedto {
     my ($self, $args) = @_;
-    my ($chart_id, $joins, $field, $operator, $quoted, $chart_fields) =
-        @$args{qw(chart_id joins field operator quoted chart_fields)};
+    my ($chart_id, $joins, $field, $operator, $quoted) =
+        @$args{qw(chart_id joins field operator quoted)};
     
     my $column = ($operator =~ /from/) ? 'removed' : 'added';
     my $table = "act_$chart_id";
-    my $field_object = $chart_fields->{$field}
+    my $field_object = $self->_chart_fields->{$field}
         || ThrowCodeError("invalid_field_name", { field => $field });
     my $field_id = $field_object->id;
     push(@$joins,
@@ -2416,11 +2422,11 @@ sub _changedfrom_changedto {
 
 sub _changedby {
     my ($self, $args) = @_;
-    my ($chart_id, $joins, $field, $operator, $value, $chart_fields) =
-        @$args{qw(chart_id joins field operator value chart_fields)};
+    my ($chart_id, $joins, $field, $operator, $value) =
+        @$args{qw(chart_id joins field operator value)};
     
     my $table = "act_$chart_id";
-    my $field_object = $chart_fields->{$field}
+    my $field_object = $self->_chart_fields->{$field}
         || ThrowCodeError("invalid_field_name", { field => $field });
     my $field_id = $field_object->id;
     my $user_id  = login_to_id($value, THROW_ERROR);