From 6b0315b816c479f90117b69c9ff57d9c843b9397 Mon Sep 17 00:00:00 2001 From: Frédéric Buclin Date: Mon, 5 Dec 2011 18:04:47 +0100 Subject: Bug 550299: User fields are left blank in buglists and whines when local user accounts are used (i.e. they have no @company.com suffix) r/a=mkanat --- Bugzilla/DB.pm | 7 +++++-- Bugzilla/DB/Oracle.pm | 7 ------- Bugzilla/DB/Pg.pm | 12 ----------- Bugzilla/DB/Sqlite.pm | 6 ------ Bugzilla/Search.pm | 55 ++++++++++++++++++++++++++++----------------------- 5 files changed, 35 insertions(+), 52 deletions(-) (limited to 'Bugzilla') diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm index ccd86d781..6043aee00 100644 --- a/Bugzilla/DB.pm +++ b/Bugzilla/DB.pm @@ -397,8 +397,11 @@ sub sql_string_concat { sub sql_string_until { my ($self, $string, $substring) = @_; - return "SUBSTRING($string FROM 1 FOR " . - $self->sql_position($substring, $string) . " - 1)"; + + my $position = $self->sql_position($substring, $string); + return "CASE WHEN $position != 0" + . " THEN SUBSTR($string, 1, $position - 1)" + . " ELSE $string END"; } sub sql_in { diff --git a/Bugzilla/DB/Oracle.pm b/Bugzilla/DB/Oracle.pm index ceee1eb50..157eeca30 100644 --- a/Bugzilla/DB/Oracle.pm +++ b/Bugzilla/DB/Oracle.pm @@ -160,13 +160,6 @@ sub sql_string_concat { return 'CONCAT(' . join(', ', @params) . ')'; } -sub sql_string_until { - my ($self, $string, $substring) = @_; - return "SUBSTR($string, 1, " - . $self->sql_position($substring, $string) - . " - 1)"; -} - sub sql_to_days { my ($self, $date) = @_; diff --git a/Bugzilla/DB/Pg.pm b/Bugzilla/DB/Pg.pm index 386a67709..e59a638a4 100644 --- a/Bugzilla/DB/Pg.pm +++ b/Bugzilla/DB/Pg.pm @@ -192,18 +192,6 @@ sub sql_string_concat { return '(CAST(' . join(' AS text) || CAST(', @params) . ' AS text))'; } -sub sql_string_until { - my ($self, $string, $substring) = @_; - - # PostgreSQL does not permit a negative substring length; therefore we - # use CASE to only perform the SUBSTRING operation when $substring can - # be found withing $string. - my $position = $self->sql_position($substring, $string); - return "CASE WHEN $position != 0" - . " THEN SUBSTRING($string FROM 1 FOR $position - 1)" - . " ELSE $string END"; -} - # Tell us whether or not a particular sequence exists in the DB. sub bz_sequence_exists { my ($self, $seq_name) = @_; diff --git a/Bugzilla/DB/Sqlite.pm b/Bugzilla/DB/Sqlite.pm index fb6aaba97..e13fd18e1 100644 --- a/Bugzilla/DB/Sqlite.pm +++ b/Bugzilla/DB/Sqlite.pm @@ -237,12 +237,6 @@ sub sql_date_math { return "DATETIME($date, '$operator' || $interval || ' $units')"; } -sub sql_string_until { - my ($self, $string, $substring) = @_; - my $position = $self->sql_position($substring, $string); - return "SUBSTR($string, 1, $position - 1)" -} - ############### # bz_ methods # ############### diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm index e6682fcc4..e0bb2355d 100644 --- a/Bugzilla/Search.pm +++ b/Bugzilla/Search.pm @@ -38,7 +38,6 @@ use base qw(Exporter); @Bugzilla::Search::EXPORT = qw( IsValidQueryType split_order_term - translate_old_column ); use Bugzilla::Error; @@ -55,6 +54,7 @@ use Bugzilla::Keyword; use Data::Dumper; use Date::Format; use Date::Parse; +use Scalar::Util qw(blessed); use List::MoreUtils qw(all part uniq); use POSIX qw(INT_MAX); use Storable qw(dclone); @@ -509,9 +509,14 @@ use constant COLUMN_JOINS => { # and we don't want it to happen at compile time, so we have it as a # subroutine. sub COLUMNS { + my $invocant = shift; + my $user = blessed($invocant) ? $invocant->_user : Bugzilla->user; my $dbh = Bugzilla->dbh; my $cache = Bugzilla->request_cache; - return $cache->{search_columns} if defined $cache->{search_columns}; + + if (defined $cache->{search_columns}->{$user->id}) { + return $cache->{search_columns}->{$user->id}; + } # These are columns that don't exist in fielddefs, but are valid buglist # columns. (Also see near the bottom of this function for the definition @@ -549,10 +554,10 @@ sub COLUMNS { ); # Backward-compatibility for old field names. Goes new_name => old_name. - # These are here and not in translate_old_column because the rest of the + # These are here and not in _translate_old_column because the rest of the # code actually still uses the old names, while the fielddefs table uses - # the new names (which is not the case for the fields handled by - # translate_old_column). + # the new names (which is not the case for the fields handled by + # _translate_old_column). my %old_names = ( creation_ts => 'opendate', delta_ts => 'changeddate', @@ -567,10 +572,7 @@ sub COLUMNS { foreach my $col (@email_fields) { my $sql = "map_${col}.login_name"; - # XXX This needs to be generated inside an accessor instead, - # probably, because it should use $self->_user to determine - # this, not Bugzilla->user. - if (!Bugzilla->user->id) { + if (!$user->id) { $sql = $dbh->sql_string_until($sql, $dbh->quote('@')); } $special_sql{$col} = $sql; @@ -610,7 +612,10 @@ sub COLUMNS { } sub REPORT_COLUMNS { - my $columns = dclone(COLUMNS); + my $invocant = shift; + my $user = blessed($invocant) ? $invocant->_user : Bugzilla->user; + + my $columns = dclone(blessed($invocant) ? $invocant->COLUMNS : COLUMNS); # There's no reason to support reporting on unique fields. # Also, some other fields don't make very good reporting axises, # or simply don't work with the current reporting system. @@ -625,7 +630,7 @@ sub REPORT_COLUMNS { # If you're not a time-tracker, you can't use time-tracking # columns. - if (!Bugzilla->user->is_timetracker) { + if (!$user->is_timetracker) { push(@no_report_columns, TIMETRACKING_FIELDS); } @@ -856,7 +861,7 @@ sub _sql_select { my $alias = $column; # Aliases cannot contain dots in them. We convert them to underscores. $alias =~ s/\./_/g; - my $sql = COLUMNS->{$column}->{name} . " AS $alias"; + my $sql = $self->COLUMNS->{$column}->{name} . " AS $alias"; push(@sql_fields, $sql); } return @sql_fields; @@ -885,13 +890,13 @@ sub _validate_order_column { # Translate old column names my ($field, $direction) = split_order_term($order_item); - $field = translate_old_column($field); + $field = $self->_translate_old_column($field); # Only accept valid columns - return if (!exists COLUMNS->{$field}); + return if (!exists $self->COLUMNS->{$field}); # Relevance column can be used only with one or more fulltext searches - return if ($field eq 'relevance' && !COLUMNS->{$field}->{name}); + return if ($field eq 'relevance' && !$self->COLUMNS->{$field}->{name}); $direction = " $direction" if $direction; return "$field$direction"; @@ -1227,7 +1232,7 @@ sub _sql_group_by { my @extra_group_by; foreach my $column ($self->_select_columns) { next if $self->_skip_group_by->{$column}; - my $sql = COLUMNS->{$column}->{name}; + my $sql = $self->COLUMNS->{$column}->{name}; push(@extra_group_by, $sql); } @@ -2336,11 +2341,11 @@ sub _content_matches { # # We build the relevance SQL by modifying the COLUMNS list directly, # which is kind of a hack but works. - my $current = COLUMNS->{'relevance'}->{name}; + my $current = $self->COLUMNS->{'relevance'}->{name}; $current = $current ? "$current + " : ''; # For NOT searches, we just add 0 to the relevance. my $select_term = $operator =~ /not/ ? 0 : "($current$rterm1 + $rterm2)"; - COLUMNS->{'relevance'}->{name} = $select_term; + $self->COLUMNS->{'relevance'}->{name} = $select_term; } sub _long_descs_count { @@ -2390,13 +2395,13 @@ sub _work_time_changedbefore_after { sub _work_time { my ($self, $args) = @_; $self->_add_extra_column('actual_time'); - $args->{full_field} = COLUMNS->{actual_time}->{name}; + $args->{full_field} = $self->COLUMNS->{actual_time}->{name}; } sub _percentage_complete { my ($self, $args) = @_; - $args->{full_field} = COLUMNS->{percentage_complete}->{name}; + $args->{full_field} = $self->COLUMNS->{percentage_complete}->{name}; # We need actual_time in _select_columns, otherwise we can't use # it in the expression for searching percentage_complete. @@ -2844,8 +2849,8 @@ sub split_order_term { # Used to translate old SQL fragments from buglist.cgi's "order" argument # into our modern field IDs. -sub translate_old_column { - my ($column) = @_; +sub _translate_old_column { + my ($self, $column) = @_; # All old SQL fragments have a period in them somewhere. return $column if $column !~ /\./; @@ -2859,9 +2864,9 @@ sub translate_old_column { # If it doesn't match the regexps above, check to see if the old # SQL fragment matches the SQL of an existing column - foreach my $key (%{ COLUMNS() }) { - next unless exists COLUMNS->{$key}->{name}; - return $key if COLUMNS->{$key}->{name} eq $column; + foreach my $key (%{ $self->COLUMNS }) { + next unless exists $self->COLUMNS->{$key}->{name}; + return $key if $self->COLUMNS->{$key}->{name} eq $column; } return $column; -- cgit v1.2.3-24-g4f1b