[Rt-commit] rt branch 5.0/optimize-ticket-display created. rt-5.0.3-64-g462624c0c7
BPS Git Server
git at git.bestpractical.com
Fri Jul 29 16:01:55 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/optimize-ticket-display has been created
at 462624c0c728521af3c2ffd70bcd48e34826a924 (commit)
- Log -----------------------------------------------------------------
commit 462624c0c728521af3c2ffd70bcd48e34826a924
Author: Jim Brandt <jbrandt at bestpractical.com>
Date: Fri Jul 29 10:23:07 2022 -0400
Skip processing when displaying ticket with no update args
If a request to Ticket/Display.html doesn't have update args to process,
skip running all processing code for these display-only requests.
With no update args, this processing makes no updates and is effectively
a no-op on most RTs. However, on some systems these processing steps can
slow down page display.
diff --git a/share/html/Ticket/Display.html b/share/html/Ticket/Display.html
index 2a37d47cf6..2cb6f87626 100644
--- a/share/html/Ticket/Display.html
+++ b/share/html/Ticket/Display.html
@@ -153,53 +153,56 @@ if ($ARGS{'id'} eq 'new') {
# fill ACL cache
$TicketObj->CurrentUser->PrincipalObj->HasRights( Object => $TicketObj );
- my $SkipProcessing;
-
- $TicketObj->Atomic(sub{
- $m->callback( CallbackName => 'BeforeProcessArguments',
- TicketObj => $TicketObj,
- ActionsRef => \@Actions, ARGSRef => \%ARGS,
- SkipProcessing => \$SkipProcessing );
-
- my ($status, @msg) = $m->comp(
- '/Elements/ValidateCustomFields',
- Object => $TicketObj,
- CustomFields => $TicketObj->CustomFields,
- ARGSRef => \%ARGS,
- );
- unless ($status) {
- push @Actions, @msg;
- $SkipProcessing = 1;
- }
- return if $SkipProcessing;
-
- if ( defined $ARGS{'Action'} ) {
- if ($ARGS{'Action'} =~ /^(Steal|Delete|Take|Untake|SetTold)$/) {
- my $action = $1;
- my ($res, $msg) = $TicketObj->$action();
- push(@Actions, $msg);
+ # Process updates only if there are any additional passed args
+ if ( grep { $_ !~ /^(?:id|results|MarkAsSeen)$/ } keys %ARGS ) {
+ my $SkipProcessing;
+
+ $TicketObj->Atomic(sub{
+ $m->callback( CallbackName => 'BeforeProcessArguments',
+ TicketObj => $TicketObj,
+ ActionsRef => \@Actions, ARGSRef => \%ARGS,
+ SkipProcessing => \$SkipProcessing );
+
+ my ($status, @msg) = $m->comp(
+ '/Elements/ValidateCustomFields',
+ Object => $TicketObj,
+ CustomFields => $TicketObj->CustomFields,
+ ARGSRef => \%ARGS,
+ );
+ unless ($status) {
+ push @Actions, @msg;
+ $SkipProcessing = 1;
+ }
+ return if $SkipProcessing;
+
+ if ( defined $ARGS{'Action'} ) {
+ if ($ARGS{'Action'} =~ /^(Steal|Delete|Take|Untake|SetTold)$/) {
+ my $action = $1;
+ my ($res, $msg) = $TicketObj->$action();
+ push(@Actions, $msg);
+ }
}
- }
-
- $m->callback(CallbackName => 'ProcessArguments',
- Ticket => $TicketObj,
- ARGSRef => \%ARGS,
- Actions => \@Actions);
-
- push @Actions, ProcessUpdateMessage(
- ARGSRef => \%ARGS,
- Actions => \@Actions,
- TicketObj => $TicketObj,
- );
- #Process status updates
- push @Actions, ProcessTicketWatchers(ARGSRef => \%ARGS, TicketObj => $TicketObj );
- push @Actions, ProcessTicketBasics( ARGSRef => \%ARGS, TicketObj => $TicketObj );
- push @Actions, ProcessTicketLinks( ARGSRef => \%ARGS, TicketObj => $TicketObj );
- push @Actions, ProcessTicketDates( ARGSRef => \%ARGS, TicketObj => $TicketObj );
- push @Actions, ProcessObjectCustomFieldUpdates(ARGSRef => \%ARGS, Object => $TicketObj );
- push @Actions, ProcessTicketReminders( ARGSRef => \%ARGS, TicketObj => $TicketObj );
- });
+ $m->callback(CallbackName => 'ProcessArguments',
+ Ticket => $TicketObj,
+ ARGSRef => \%ARGS,
+ Actions => \@Actions);
+
+ push @Actions, ProcessUpdateMessage(
+ ARGSRef => \%ARGS,
+ Actions => \@Actions,
+ TicketObj => $TicketObj,
+ );
+
+ #Process status updates
+ push @Actions, ProcessTicketWatchers(ARGSRef => \%ARGS, TicketObj => $TicketObj );
+ push @Actions, ProcessTicketBasics( ARGSRef => \%ARGS, TicketObj => $TicketObj );
+ push @Actions, ProcessTicketLinks( ARGSRef => \%ARGS, TicketObj => $TicketObj );
+ push @Actions, ProcessTicketDates( ARGSRef => \%ARGS, TicketObj => $TicketObj );
+ push @Actions, ProcessObjectCustomFieldUpdates(ARGSRef => \%ARGS, Object => $TicketObj );
+ push @Actions, ProcessTicketReminders( ARGSRef => \%ARGS, TicketObj => $TicketObj );
+ });
+ }
unless ($TicketObj->CurrentUserHasRight('ShowTicket')) {
if (@Actions) {
commit 76576ec385db34dc1e4d4fe9a173363fb97d43fb
Author: sunnavy <sunnavy at bestpractical.com>
Date: Fri Jul 29 22:57:13 2022 +0800
Use $SkipProcessing for ticket updates only
The $SkipProcessing is used to short-circuit the following ticket
updates in some cases like invalid custom field values are submitted.
No matter if $SkipProcessing is true or false, we shall always check
"ShowTicket" right and the "MarkAsSeen" can be processed too as it's
more like an update to current user instead of the ticket.
Note that it's not quite a security issue: even without the "ShowTicket"
check, ticket fields wouldn't show up on display page anyway, in which
case the page would look like a ticket without any data in it.
diff --git a/share/html/Ticket/Display.html b/share/html/Ticket/Display.html
index 78642411ba..2a37d47cf6 100644
--- a/share/html/Ticket/Display.html
+++ b/share/html/Ticket/Display.html
@@ -200,23 +200,22 @@ if ($ARGS{'id'} eq 'new') {
push @Actions, ProcessObjectCustomFieldUpdates(ARGSRef => \%ARGS, Object => $TicketObj );
push @Actions, ProcessTicketReminders( ARGSRef => \%ARGS, TicketObj => $TicketObj );
});
- if ( !$SkipProcessing ) {
- unless ($TicketObj->CurrentUserHasRight('ShowTicket')) {
- if (@Actions) {
- Abort("A change was applied successfully, but you no longer have permissions to view the ticket", Actions => \@Actions, Code => HTTP::Status::HTTP_FORBIDDEN);
- } else {
- Abort("No permission to view ticket", Code => HTTP::Status::HTTP_FORBIDDEN);
- }
- }
- if ( $ARGS{'MarkAsSeen'} ) {
- $TicketObj->SetAttribute(
- Name => 'User-'. $TicketObj->CurrentUser->id .'-SeenUpTo',
- Content => $TicketObj->LastUpdated,
- );
- push @Actions, loc('Marked all messages as seen');
+
+ unless ($TicketObj->CurrentUserHasRight('ShowTicket')) {
+ if (@Actions) {
+ Abort("A change was applied successfully, but you no longer have permissions to view the ticket", Actions => \@Actions, Code => HTTP::Status::HTTP_FORBIDDEN);
+ } else {
+ Abort("No permission to view ticket", Code => HTTP::Status::HTTP_FORBIDDEN);
}
- $TicketObj->CurrentUser->AddRecentlyViewedTicket($TicketObj);
}
+ if ( $ARGS{'MarkAsSeen'} ) {
+ $TicketObj->SetAttribute(
+ Name => 'User-'. $TicketObj->CurrentUser->id .'-SeenUpTo',
+ Content => $TicketObj->LastUpdated,
+ );
+ push @Actions, loc('Marked all messages as seen');
+ }
+ $TicketObj->CurrentUser->AddRecentlyViewedTicket($TicketObj);
}
-----------------------------------------------------------------------
hooks/post-receive
--
rt
More information about the rt-commit
mailing list