[Rt-commit] rt branch 5.0/autocomplete-article-inclusion created. rt-5.0.2-55-gbe833e6a36
BPS Git Server
git at git.bestpractical.com
Wed Jan 19 19:32:16 UTC 2022
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "rt".
The branch, 5.0/autocomplete-article-inclusion has been created
at be833e6a360be5bc610f9a9a6e8105d7d6f575db (commit)
- Log -----------------------------------------------------------------
commit be833e6a360be5bc610f9a9a6e8105d7d6f575db
Author: Brian Conry <bconry at bestpractical.com>
Date: Wed Jan 19 12:53:30 2022 -0600
Allow Article autocomplete search on more fields
The autocomplete box for Articles in RT 4.x looked at more fields than
just the name, but in the updates to RT 5.0 the rework of that section
of code removed that feature.
This commit adds the config option $ArticleSearchFields, patterned after
$UserSearchFields, which defaults to Name and Summary (like RT 4.x),
and the changes to make use of it.
diff --git a/etc/RT_Config.pm.in b/etc/RT_Config.pm.in
index b0b0c5cc65..8b80bd7433 100644
--- a/etc/RT_Config.pm.in
+++ b/etc/RT_Config.pm.in
@@ -2541,6 +2541,23 @@ preference.
Set($AutocompleteQueues, 0);
+=item C<$ArticleSearchFields>
+
+Used by the Include Article as well as the Article Search.
+
+Specifies which fields of L<RT::Article> to match against and how to match
+each field when autocompleting articles. Valid match methods are LIKE,
+STARTSWITH, ENDSWITH, =, and !=. Valid search fields are the core Article
+fields, as well as custom fields, including Content, which are specified as
+"CF.1234" or "CF.Name"
+
+=cut
+
+Set($ArticleSearchFields, {
+ Name => 'STARTSWITH',
+ Summary => 'LIKE',
+});
+
=item C<$UserSearchFields>
Used by the User Autocompleter as well as the User Search.
diff --git a/lib/RT/Articles.pm b/lib/RT/Articles.pm
index 0e569d201e..e87ae4d3c6 100644
--- a/lib/RT/Articles.pm
+++ b/lib/RT/Articles.pm
@@ -875,6 +875,99 @@ sub Search {
return 1;
}
+=head2 SimpleSearch
+
+Does a 'simple' search of Articles against a specified Term.
+
+This Term is compared to a number of fields using various types of SQL
+comparison operators.
+
+Ensures that the returned collection of Articles will have a value for Return.
+
+This method is passed the following. You must specify a Term and a Return.
+
+ Queue - Limit the search to classes applied to this queue
+ Fields - Hashref of data - defaults to C<$ArticleSearchFields> emulate that if you want to override
+ Term - String that is in the fields specified by Fields
+ Return - What field on the Article you want to be sure isn't empty
+ Max - What to limit this collection to
+
+=cut
+
+sub SimpleSearch {
+ my $self = shift;
+ my %args = (
+ Queue => undef,
+ Fields => RT->Config->Get('ArticleSearchFields'),
+ Term => undef,
+ Return => undef,
+ Max => 10,
+ @_
+ );
+
+ return $self unless defined $args{Return}
+ and defined $args{Term}
+ and length $args{Term};
+
+ $self->RowsPerPage( $args{Max} );
+
+ if ($args{Queue}) {
+ $self->LimitAppliedClasses( Queue => $args{Queue} );
+ }
+
+ while (my ($name, $op) = each %{$args{Fields}}) {
+ $op = 'STARTSWITH'
+ unless $op =~ /^(?:LIKE|(?:START|END)SWITH|=|!=)$/i;
+
+ if ($name =~ /^CF\.(?:\{(.*)}|(.*))$/) {
+ my $cfname = $1 || $2;
+ my $cf = RT::CustomField->new(RT->SystemUser);
+ my ($ok, $msg) = $cf->LoadByName(
+ Name => $cfname,
+ LookupType => RT::Article->new( $self->CurrentUser )->CustomFieldLookupType,
+ );
+ if ( $ok ) {
+ $self->LimitCustomField(
+ CUSTOMFIELD => $cf->Id,
+ OPERATOR => $op,
+ VALUE => $args{Term},
+ ENTRYAGGREGATOR => 'OR',
+ SUBCLAUSE => 'autocomplete',
+ );
+ } else {
+ RT->Logger->warning("Asked to search custom field $name but unable to load an Article CF with the name $cfname: $msg");
+ }
+ } else {
+ $self->Limit(
+ FIELD => $name,
+ OPERATOR => $op,
+ VALUE => $args{Term},
+ ENTRYAGGREGATOR => 'OR',
+ SUBCLAUSE => 'autocomplete',
+ );
+ }
+ }
+
+ if ( RT->Config->Get('DatabaseType') eq 'Oracle' ) {
+ $self->Limit(
+ FIELD => $args{Return},
+ OPERATOR => 'IS NOT',
+ VALUE => 'NULL',
+ );
+ }
+ elsif ( $args{Return} !~ /^id$/i ) {
+ $self->Limit( FIELD => $args{Return}, OPERATOR => '!=', VALUE => '' );
+ $self->Limit(
+ FIELD => $args{Return},
+ OPERATOR => 'IS NOT',
+ VALUE => 'NULL',
+ ENTRYAGGREGATOR => 'AND'
+ );
+ }
+
+ return $self;
+}
+
RT::Base->_ImportOverlays();
1;
diff --git a/share/html/Helpers/Autocomplete/Articles b/share/html/Helpers/Autocomplete/Articles
index cd9e6e8d54..1d2fbcd437 100644
--- a/share/html/Helpers/Autocomplete/Articles
+++ b/share/html/Helpers/Autocomplete/Articles
@@ -50,10 +50,10 @@
% $m->abort;
<%ARGS>
$term => undef
-$max => 10
-$op => 'STARTSWITH'
+$max => undef
+$op => undef
$right => undef
-$return => 'Name'
+$return => ''
$queue => undef
</%ARGS>
<%INIT>
@@ -65,24 +65,18 @@ $m->abort unless defined $return
and defined $term
and length $term;
-# Sanity check the operator
-$op = 'STARTSWITH' unless $op =~ /^(?:LIKE|(?:START|END)SWITH|=|!=)$/i;
-
$m->callback( CallbackName => 'ModifyMaxResults', max => \$max );
+$max //= 10;
-my $articles = RT::Articles->new( $session{CurrentUser} );
-if( $queue ) {
- $articles->LimitAppliedClasses( Queue => $queue );
-}
-
-$articles->RowsPerPage( $max );
-$articles->Limit(
- FIELD => 'Name',
- OPERATOR => $op,
- VALUE => $term,
- ENTRYAGGREGATOR => 'OR',
- CASESENSITIVE => 0,
+my $articles = RT::Articles->new($session{'CurrentUser'});
+$articles->SimpleSearch(
+ Queue => $queue,
+ Return => $return,
+ Term => $term,
+ Max => $max,
+ $op ? ( Fields => { $return => $op } ) : (),
);
+$m->callback( CallbackName => "ModifyArticlesLimit", Articles => $articles, Term => $term, ARGSRef => \%ARGS );
my @suggestions;
while (my $a = $articles->Next) {
commit 0b1a5223c10d105690a7cdd1cc8d6f763ccb6a89
Author: Brian Conry <bconry at bestpractical.com>
Date: Wed Jan 19 12:37:34 2022 -0600
Fix ModifySuggestion callback invocation
Passing an array variable, without taking a reference to it, was causing
warnings in the logs about odd numbers of parameters in hash assignment
in RT::Interface::Web::Request::callback whenever @suggestions contained
an even number of elements (or zero).
This could have been addressed by passing \@suggestions, but then it
probably should be moved out of the loop and only invoked once after all
suggestions have been found. The callback with the same name in
/Helpers/Autocomplete/Users works on a single suggestion at a time, so
it would probably also need be renamed to be plural.
diff --git a/share/html/Helpers/Autocomplete/Articles b/share/html/Helpers/Autocomplete/Articles
index 825900b7ca..cd9e6e8d54 100644
--- a/share/html/Helpers/Autocomplete/Articles
+++ b/share/html/Helpers/Autocomplete/Articles
@@ -88,7 +88,8 @@ my @suggestions;
while (my $a = $articles->Next) {
next if $right and not $a->CurrentUserHasRight($right);
my $value = $a->$return;
- push @suggestions, { label => $a->Name, value => $value };
- $m->callback( CallbackName => "ModifySuggestion", suggestions => @suggestions, label => $a );
+ my $suggestion = { label => $a->Name, value => $value };
+ $m->callback( CallbackName => "ModifySuggestion", suggestion => $suggestion, label => $a );
+ push @suggestions, $suggestion;
}
</%INIT>
commit e2c5f7888da4a8f6a55cc3d2eadbc4e0a9b2e0d7
Author: Brian Conry <bconry at bestpractical.com>
Date: Wed Jan 19 09:52:18 2022 -0600
Sanity-check IncludeArticleId
Prevents ugly errors from some DBDs when IncludeArticleId has a
non-numeric value, as can happen when there are enough articles that the
autocomplete text box is used instead of the dropdown selectbox.
Also notifies the user and prevents the commit whenever it can't load
the specified IncludeArticleId so that the user has a chance to correct
the issue.
diff --git a/share/html/Articles/Elements/CheckSkipCreate b/share/html/Articles/Elements/CheckSkipCreate
index 14c8d57547..ff712f9c8c 100644
--- a/share/html/Articles/Elements/CheckSkipCreate
+++ b/share/html/Articles/Elements/CheckSkipCreate
@@ -50,11 +50,19 @@
return if $checks_failure; # we're already skipping Create
return unless RT->Config->Get('ArticleOnTicketCreate');
-
-if ( $ARGSRef->{IncludeArticleId} ) {
- my $article = RT::Article->new($session{'CurrentUser'});
- $article->Load($ARGSRef->{IncludeArticleId});
- $$skip_create = 1 if $article->id;
+if ( $ARGSRef->{IncludeArticleId} or $DECODED_ARGS->{IncludedArticleId} ) {
+ # there are three scenarios here, all of which should set $$skip_create
+ # 1. an IncludeArticleId was submitted, in the proper format, matching the
+ # ID of an existing article, which has been loaded and added to the
+ # messagebox, and the user should be given a chance to tweak/add to
+ # the message after the inclusion of the article
+ # 2. an IncludeArticleId was submitted, in the proper format, but it doesn't
+ # match the ID of an existing article, in which case the user should be
+ # given the chance to correct it before the ticket is created.
+ # 3. an IncludeArticleId was submitted but not in the proper format,
+ # in which case the user should be given a chance to correct it before
+ # the ticket is created.
+ $$skip_create = 1;
}
return;
diff --git a/share/html/Ticket/Create.html b/share/html/Ticket/Create.html
index a55e2c6fa8..003c855bb6 100644
--- a/share/html/Ticket/Create.html
+++ b/share/html/Ticket/Create.html
@@ -368,6 +368,35 @@ $session{DefaultQueue} = $Queue;
my $current_user = $session{'CurrentUser'};
+my $checks_failure = 0;
+my @results;
+
+if ($ARGS{IncludeArticleId}) {
+ my ($ret, $msg);
+
+ if ($ARGS{IncludeArticleId} !~ /^\d+$/ ) {
+ $ARGS{IncludeArticleId} = undef;
+ }
+ else {
+ my $article = RT::Article->new($session{'CurrentUser'});
+ ($ret, $msg) = $article->Load( $ARGS{IncludeArticleId} );
+ }
+
+ if ($ret) {
+ # This means that it will be able to be included, which means that we won't want
+ # the select/search box to pick this back up again. there is a risk. if something
+ # happens that causes the inclusion to fail or not happen, then the user won't get
+ # another chance without reselecting/re-entering their input
+ # The select/search box is rendered before the inclusion is attempted, so it can't
+ # simply rely on a result from the inclusion.
+ delete $DECODED_ARGS->{IncludeArticleId};
+ }
+ else {
+ $checks_failure = 1;
+ push @results, loc( 'Unable to include article "[_1]"', $DECODED_ARGS->{IncludeArticleId} );
+ }
+}
+
if ($CloneTicket) {
my $CloneTicketObj = RT::Ticket->new( $session{CurrentUser} );
$CloneTicketObj->Load($CloneTicket)
@@ -440,8 +469,6 @@ if ($CloneTicket) {
}
-my @results;
-
my $QueueObj = RT::Queue->new($current_user);
$QueueObj->Load($Queue) || Abort(loc("Queue [_1] could not be loaded.", $Queue||''), Code => HTTP::Status::HTTP_BAD_REQUEST);
@@ -457,8 +484,6 @@ my $ticket = RT::Ticket->new($current_user); # empty ticket object
ProcessAttachments(ARGSRef => \%ARGS);
-my $checks_failure = 0;
-
{
my ($status, @msg) = $m->comp(
'/Elements/ValidateCustomFields',
diff --git a/share/html/Ticket/Update.html b/share/html/Ticket/Update.html
index 859b59ebf6..b7c3cdd7df 100644
--- a/share/html/Ticket/Update.html
+++ b/share/html/Ticket/Update.html
@@ -341,6 +341,32 @@ my $TicketObj = LoadTicket($id);
my @results;
+if ($ARGS{IncludeArticleId}) {
+ my ($ret, $msg);
+
+ if ($ARGS{IncludeArticleId} !~ /^\d+$/ ) {
+ $ARGS{IncludeArticleId} = undef;
+ }
+ else {
+ my $article = RT::Article->new($session{'CurrentUser'});
+ ($ret, $msg) = $article->Load( $ARGS{IncludeArticleId} );
+ }
+
+ if ($ret) {
+ # This means that it will be able to be included, which means that we won't want
+ # the select/search box to pick this back up again. there is a risk. if something
+ # happens that causes the inclusion to fail or not happen, then the user won't get
+ # another chance without reselecting/re-entering their input
+ # The select/search box is rendered before the inclusion is attempted, so it can't
+ # simply rely on a result from the inclusion.
+ delete $DECODED_ARGS->{IncludeArticleId};
+ }
+ else {
+ $checks_failure = 1;
+ push @results, loc( 'Unable to include article "[_1]"', $DECODED_ARGS->{IncludeArticleId} );
+ }
+}
+
$m->callback( Ticket => $TicketObj, ARGSRef => \%ARGS, checks_failure => \$checks_failure, results => \@results, CallbackName => 'Initial' );
$m->scomp( '/Articles/Elements/SubjectOverride', Ticket => $TicketObj, ARGSRef => \%ARGS, results => \@results );
commit ab4711cc627949873383a32d7e178365db08a162
Author: Brian Conry <bconry at bestpractical.com>
Date: Wed Jan 19 09:43:10 2022 -0600
Fix broken argument chain for IncludeArticleId
Ensure that IncludeArticleId is passed in %ARGS to all of the subpages
that need it so that they don't need to pull that data out of
$DECODED_ARGS. This will allow us to add sanity checks to the
IncludeArticleId and clear it if necessary.
diff --git a/share/html/Articles/Elements/IncludeArticle b/share/html/Articles/Elements/IncludeArticle
index c5eb459167..01f4b99573 100644
--- a/share/html/Articles/Elements/IncludeArticle
+++ b/share/html/Articles/Elements/IncludeArticle
@@ -47,7 +47,7 @@
%# END BPS TAGGED BLOCK }}}
<%INIT>
# Nothing to do if we don't get an article id
-$IncludeArticleId //= $DECODED_ARGS->{'IncludeArticleId'};
+$IncludeArticleId //= $ARGS{'IncludeArticleId'};
($IncludeArticleId) = grep defined && length, @$IncludeArticleId if ref $IncludeArticleId eq 'ARRAY';
return unless $IncludeArticleId;
diff --git a/share/html/Elements/MessageBox b/share/html/Elements/MessageBox
index ce985aa51b..5362b47c7b 100644
--- a/share/html/Elements/MessageBox
+++ b/share/html/Elements/MessageBox
@@ -102,9 +102,9 @@ if ( $IncludeSignature and $signature =~ /\S/ ) {
}
my $article_id;
-if ( $IncludeDefaultArticle && defined $QueueObj && $QueueObj->Id ) {
+if ( $IncludeDefaultArticle && defined $QueueObj && $QueueObj->Id && $QueueObj->DefaultValue('Article') ) {
# Load a default article
- $article_id = $QueueObj->DefaultValue('Article') if $QueueObj->DefaultValue('Article');
+ $article_id = $QueueObj->DefaultValue('Article');
}
else {
# Load from the page, if provided
diff --git a/share/html/Elements/SelectArticle b/share/html/Elements/SelectArticle
index 5779aaab66..cb5ec38e5a 100644
--- a/share/html/Elements/SelectArticle
+++ b/share/html/Elements/SelectArticle
@@ -74,7 +74,7 @@ $Default //= '';
<%ARGS>
$QueueObj
$Name => 'IncludeArticleId'
-$Default => ''
+$Default => $DECODED_ARGS->{IncludeArticleId} // ''
$AutoSubmit => 0
$IncludeSummary => 1
</%ARGS>
diff --git a/share/html/Ticket/Create.html b/share/html/Ticket/Create.html
index bded7f6e57..a55e2c6fa8 100644
--- a/share/html/Ticket/Create.html
+++ b/share/html/Ticket/Create.html
@@ -226,11 +226,11 @@
<div class="form-group">
% $m->callback( %ARGS, QueueObj => $QueueObj, CallbackName => 'BeforeMessageBox' );
% if (exists $ARGS{Content}) {
-<& /Elements/MessageBox, QueueObj => $QueueObj, Default => $ARGS{Content}, IncludeSignature => 0, IncludeDefaultArticle => 0 &>
+<& /Elements/MessageBox, QueueObj => $QueueObj, IncludeArticleId => $ARGS{IncludeArticleId}, Default => $ARGS{Content}, IncludeSignature => 0, IncludeDefaultArticle => 0 &>
% } elsif ( $QuoteTransaction ) {
-<& /Elements/MessageBox, QueueObj => $QueueObj, QuoteTransaction => $QuoteTransaction, IncludeDefaultArticle => 0 &>
+<& /Elements/MessageBox, QueueObj => $QueueObj, IncludeArticleId => $ARGS{IncludeArticleId}, QuoteTransaction => $QuoteTransaction, IncludeDefaultArticle => 0 &>
% } else {
-<& /Elements/MessageBox, QueueObj => $QueueObj, IncludeDefaultArticle => 1 &>
+<& /Elements/MessageBox, QueueObj => $QueueObj, IncludeArticleId => $ARGS{IncludeArticleId}, IncludeDefaultArticle => 1 &>
%}
% $m->callback( %ARGS, QueueObj => $QueueObj, CallbackName => 'AfterMessageBox' );
</div>
diff --git a/share/html/Ticket/Update.html b/share/html/Ticket/Update.html
index 21bb8ed522..859b59ebf6 100644
--- a/share/html/Ticket/Update.html
+++ b/share/html/Ticket/Update.html
@@ -182,12 +182,12 @@
% # preserve QuoteTransaction so we can use it to set up sane references/in/reply to
% my $temp = $ARGS{'QuoteTransaction'};
% delete $ARGS{'QuoteTransaction'};
- <& /Elements/MessageBox, Name=>"UpdateContent", Default=>$ARGS{UpdateContent}, IncludeSignature => 0, %ARGS&>
+ <& /Elements/MessageBox, IncludeArticleId => $ARGS{IncludeArticleId}, Name=>"UpdateContent", Default=>$ARGS{UpdateContent}, IncludeSignature => 0, %ARGS&>
% $ARGS{'QuoteTransaction'} = $temp;
% } else {
% my $IncludeSignature = 1;
% $IncludeSignature = 0 if $Action ne 'Respond' && !RT->Config->Get('MessageBoxIncludeSignatureOnComment');
- <& /Elements/MessageBox, Name=>"UpdateContent", IncludeSignature => $IncludeSignature, %ARGS &>
+ <& /Elements/MessageBox, IncludeArticleId => $ARGS{IncludeArticleId}, Name=>"UpdateContent", IncludeSignature => $IncludeSignature, %ARGS &>
% }
% $m->callback( %ARGS, CallbackName => 'AfterMessageBox' );
</div>
commit 62028edabe4b48095cc4eae07055f2f6ec076507
Author: Brian Conry <bconry at bestpractical.com>
Date: Wed Jan 19 09:38:33 2022 -0600
Identify default article in select box by Id
The article name does not have to be unique, so identifying the selected
article by name may lead to multiple matches within the list and the
inclusion of the wrong article.
diff --git a/share/html/Elements/SelectArticle b/share/html/Elements/SelectArticle
index e503da7124..5779aaab66 100644
--- a/share/html/Elements/SelectArticle
+++ b/share/html/Elements/SelectArticle
@@ -51,7 +51,7 @@
<select name="<% $Name %>" <% $AutoSubmit ? 'onchange="this.form.submit()"' : '' |n%> class="form-control selectpicker">
<option value="">-</option>
% while (my $article = $articles->Next) {
-<option <% ( $article->Name eq $Default) ? qq[ selected="selected"] : '' |n %>
+<option <% ( $article->Id eq $Default) ? qq[ selected="selected"] : '' |n %>
value="<%$article->Id%>">
<%$article->Name || loc('(no name)')%><% $IncludeSummary ? ': ' . ($article->Summary || '') : '' %>
</option>
@@ -68,19 +68,6 @@ $m->callback( CallbackName => 'ModifyDropdownLimit', DropdownLimit => \$dropdown
my $autocomplete = $articles->Count > $dropdown_limit ? 1 : 0;
-if ( $Default and $Default =~ /^\d+$/ ){
- # We got an id, look up the name
- my $default_article = RT::Article->new($session{'CurrentUser'});
- my ($ret, $msg) = $default_article->Load( $Default );
-
- if ($ret) {
- $Default = $default_article->Name;
- }
- else {
- RT::Logger->error("Unable to load article $Default: $msg");
- }
-}
-
$Default //= '';
</%INIT>
-----------------------------------------------------------------------
hooks/post-receive
--
rt
More information about the rt-commit
mailing list