Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug 1918771 - Support pronouns such as %user% in "changed by" searches #2321

Merged
merged 5 commits into from
Sep 26, 2024
+50 −89
Merged

Bug 1918771 - Support pronouns such as %user% in "changed by" searches #2321

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a9c241a
Bug 1918771 - Support pronouns such as %user% in "changed by" searches
dklawren Sep 13, 2024
75123a8
Fixed failing test
dklawren Sep 17, 2024
90f736c
Fixed failing test
dklawren Sep 18, 2024
d52a6d5
More test fixing
dklawren Sep 19, 2024
732d62d
Added comment
dklawren Sep 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
125 changes: 42 additions & 83 deletions Bugzilla/Search.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,8 +366,8 @@ sub SPECIAL_PARSING {
# Pronoun Fields (Ones that can accept %user%, etc.)
assigned_to => \&_contact_pronoun,
'attachments.submitter' => \&_contact_pronoun,
cc => \&_cc_pronoun,
commenter => \&_commenter_pronoun,
cc => \&_contact_pronoun,
commenter => \&_contact_pronoun,
qa_contact => \&_contact_pronoun,
reporter => \&_contact_pronoun,
'requestees.login_name' => \&_contact_pronoun,
Expand Down Expand Up @@ -1772,8 +1772,9 @@ sub _special_parse_email {
my $id = $1;
my $email = trim($params->{"email$id"});
next if !$email;
my $type = $params->{"emailtype$id"} || 'anyexact';
$type = "anyexact" if $type eq "exact";
my $type = $params->{"emailtype$id"} || 'equals';
# for backward compatibility
$type = "equals" if $type eq "exact";

my $or_clause = new Bugzilla::Search::Clause('OR');
foreach my $field (qw(assigned_to reporter cc qa_contact bug_mentor)) {
Expand Down Expand Up @@ -2445,8 +2446,8 @@ sub _contact_pronoun {

sub _contact_exact_group {
my ($self, $args) = @_;
my ($value, $operator, $field, $chart_id, $joins)
= @$args{qw(value operator field chart_id joins)};
my ($value, $operator, $field, $chart_id, $joins, $sequence)
= @$args{qw(value operator field chart_id joins sequence)};
my $dbh = Bugzilla->dbh;
my $user = $self->_user;

Expand All @@ -2466,11 +2467,29 @@ sub _contact_exact_group {
$group->check_members_are_visible();

my $group_ids = Bugzilla::Group->flatten_group_membership($group->id);

if ($field eq 'cc' && $chart_id eq '') {
# This is for the email1, email2, email3 fields from query.cgi.
$chart_id = "CC$$sequence";
$args->{sequence}++;
}

my $from = $field;

# These fields need an additional table.
if ($field eq 'commenter' || $field eq 'cc') {
my $join_table = $field;
$join_table = 'longdescs' if $field eq 'commenter';
my $join_table_alias = "${field}_$chart_id";
push @$joins, {table => $join_table, as => $join_table_alias};
$from = "$join_table_alias.who";
}

my $table = "user_group_map_$chart_id";
my $join = {
table => 'user_group_map',
as => $table,
from => $field,
from => $from,
to => 'user_id',
extra => [$dbh->sql_in("$table.group_id", $group_ids), "$table.isbless = 0"],
};
Expand All @@ -2483,79 +2502,6 @@ sub _contact_exact_group {
}
}

sub _cc_pronoun {
my ($self, $args) = @_;
my ($full_field, $value) = @$args{qw(full_field value)};
my $user = $self->_user;

if ($value =~ /\%group/) {
return $self->_cc_exact_group($args);
}
elsif ($value =~ /^(%\w+%)$/) {
$args->{value} = pronoun($1, $user);
$args->{quoted} = $args->{value};
$args->{value_is_id} = 1;
}
}

sub _cc_exact_group {
my ($self, $args) = @_;
my ($chart_id, $sequence, $joins, $operator, $value)
= @$args{qw(chart_id sequence joins operator value)};
my $user = $self->_user;
my $dbh = Bugzilla->dbh;

$value =~ m/%group\.([^%]+)%/;
my $group
= Bugzilla::Group->check({name => $1, _error => 'invalid_group_name'});
$group->check_members_are_visible();
$user->in_group($group)
|| ThrowUserError('invalid_group_name', {name => $group->name});

my $all_groups = Bugzilla::Group->flatten_group_membership($group->id);

# This is for the email1, email2, email3 fields from query.cgi.
if ($chart_id eq "") {
$chart_id = "CC$$sequence";
$args->{sequence}++;
}

my $cc_table = "cc_$chart_id";
push(@$joins, {table => 'cc', as => $cc_table});
my $group_table = "user_group_map_$chart_id";
my $group_join = {
table => 'user_group_map',
as => $group_table,
from => "$cc_table.who",
to => 'user_id',
extra => [
$dbh->sql_in("$group_table.group_id", $all_groups),
"$group_table.isbless = 0"
],
};
push(@$joins, $group_join);

if ($operator =~ /^not/) {
$args->{term} = "$group_table.group_id IS NULL";
}
else {
$args->{term} = "$group_table.group_id IS NOT NULL";
}
}

# XXX This should probably be merged with cc_pronoun.
sub _commenter_pronoun {
my ($self, $args) = @_;
my $value = $args->{value};
my $user = $self->_user;

if ($value =~ /^(%\w+%)$/) {
$args->{value} = pronoun($1, $user);
$args->{quoted} = $args->{value};
$args->{value_is_id} = 1;
}
}

# XXX only works with %user% currently
sub _triage_owner_pronoun {
my ($self, $args) = @_;
Expand All @@ -2573,6 +2519,15 @@ sub _triage_owner_pronoun {
}
}

sub _get_user_id {
my ($self, $value) = @_;

if ($value =~ /^%\w+%$/) {
dklawren marked this conversation as resolved.
Show resolved Hide resolved
return pronoun($value, $self->_user);
}
return login_to_id($value, THROW_ERROR);
}

######################################
# "Special Parsing" Functions: Misc. #
######################################
Expand Down Expand Up @@ -2728,7 +2683,7 @@ sub _long_desc_changedby {

my $table = "longdescs_$chart_id";
push(@$joins, {table => 'longdescs', as => $table});
my $user_id = login_to_id($value, THROW_ERROR);
my $user_id = $self->_get_user_id($value);
$args->{term} = "$table.who = $user_id";
}

Expand Down Expand Up @@ -2896,7 +2851,7 @@ sub _work_time_changedby {

my $table = "longdescs_$chart_id";
push(@$joins, {table => 'longdescs', as => $table});
my $user_id = login_to_id($value, THROW_ERROR);
my $user_id = $self->_get_user_id($value);
$args->{term} = "$table.who = $user_id AND $table.work_time != 0";
}

Expand Down Expand Up @@ -3695,7 +3650,11 @@ sub _changedby {
push(@{$join->{extra}}, "$table.fieldid = $field_id");
}

my $user_id = login_to_id($value, THROW_ERROR);
# We may have already converted to user id if use of pronoun so skip
my $user_id = $value;
if ($value !~ /^\d+$/) {
$user_id = $self->_get_user_id($value);
}
push(@{$join->{extra}}, "$table.who = $user_id");

$args->{term} = "$table.bug_when IS NOT NULL";
Expand Down
10 changes: 5 additions & 5 deletions qa/t/1_test_bug_edit.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,17 +412,17 @@ $sel->remove_all_selections_ok("bug_status");
$sel->remove_all_selections_ok("resolution");
screenshot_page($sel, '/app/artifacts/line264.png');
$sel->is_checked_ok("emailassigned_to1");
$sel->select_ok("emailtype1", "value=exact");
$sel->select_ok("emailtype1", "value=equals");
$sel->type_ok("email1", $config->{admin_user_login});
$sel->check_ok("emailassigned_to2");
$sel->check_ok("emailqa_contact2");
$sel->check_ok("emailcc2");
$sel->select_ok("emailtype2", "value=exact");
$sel->select_ok("emailtype2", "value=equals");
$sel->type_ok("email2", $config->{QA_Selenium_TEST_user_login});
screenshot_page($sel, '/app/artifacts/line271.png');
$sel->click_ok("Search");
check_page_load($sel,
q{http://HOSTNAME/buglist.cgi?emailreporter2=1&order=Importance&emailtype2=exact&list_id=__LIST_ID__&emailtype1=exact&emailcc2=1&emailassigned_to1=1&query_format=advanced&emailqa_contact2=1&email2=QA-Selenium-TEST%40mozilla.test&emailassigned_to2=1&email1=admin%40mozilla.test&product=TestProduct}
q{http://HOSTNAME/buglist.cgi?emailreporter2=1&order=Importance&emailtype2=equals&list_id=__LIST_ID__&emailtype1=equals&emailcc2=1&emailassigned_to1=1&query_format=advanced&emailqa_contact2=1&email2=QA-Selenium-TEST%40mozilla.test&emailassigned_to2=1&email1=admin%40mozilla.test&product=TestProduct}
);
$sel->title_is("Bug List");
screenshot_page($sel, '/app/artifacts/line275.png');
Expand Down Expand Up @@ -507,7 +507,7 @@ $sel->is_text_present_ok("2 bugs found");
screenshot_page($sel, '/app/artifacts/line350.png');
$sel->click_ok('change-several');
check_page_load($sel,
q{http://HOSTNAME/buglist.cgi?email1=admin%40mozilla.test&email2=QA-Selenium-TEST%40mozilla.test&emailassigned_to1=1&emailassigned_to2=1&emailcc2=1&emailqa_contact2=1&emailreporter2=1&emailtype1=exact&emailtype2=exact&product=TestProduct&query_format=advanced&order=priority%2Cbug_severity&tweak=1&list_id=__LIST_ID__}
q{http://HOSTNAME/buglist.cgi?email1=admin%40mozilla.test&email2=QA-Selenium-TEST%40mozilla.test&emailassigned_to1=1&emailassigned_to2=1&emailcc2=1&emailqa_contact2=1&emailreporter2=1&emailtype1=equals&emailtype2=equals&product=TestProduct&query_format=advanced&order=priority%2Cbug_severity&tweak=1&list_id=__LIST_ID__}
);
$sel->title_is("Bug List");
$sel->click_ok("check_all");
Expand Down Expand Up @@ -639,7 +639,7 @@ $sel->title_is("Bug List: My bugs from QA_Selenium");
$sel->is_text_present_ok("2 bugs found");
$sel->click_ok('change-several', 'Change Several Bugs at Once');
check_page_load($sel,
q{http://HOSTNAME/buglist.cgi?email1=admin%40mozilla.test&email2=QA-Selenium-TEST%40mozilla.test&emailassigned_to1=1&emailassigned_to2=1&emailcc2=1&emailqa_contact2=1&emailreporter2=1&emailtype1=exact&emailtype2=exact&product=TestProduct&query_format=advanced&order=priority%2Cbug_severity&tweak=1&list_id=__LIST_ID__}
q{http://HOSTNAME/buglist.cgi?email1=admin%40mozilla.test&email2=QA-Selenium-TEST%40mozilla.test&emailassigned_to1=1&emailassigned_to2=1&emailcc2=1&emailqa_contact2=1&emailreporter2=1&emailtype1=equals&emailtype2=equals&product=TestProduct&query_format=advanced&order=priority%2Cbug_severity&tweak=1&list_id=__LIST_ID__}
);
$sel->title_is("Bug List");
$sel->click_ok("check_all");
Expand Down
2 changes: 1 addition & 1 deletion template/en/default/search/form.html.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ TUI_hide_default('custom_search_query');
[% FOREACH qv = [
{ name => "substring", description => "contains" },
{ name => "notsubstring", description => "doesn't contain" },
{ name => "exact", description => "is" },
{ name => "equals", description => "is" },
{ name => "notequals", description => "is not" },
{ name => "regexp", description => "matches regexp" },
{ name => "notregexp", description => "doesn't match regexp" } ] %]
Expand Down
Loading