Mailing List Archive

rt branch 5.0/cleanup-lifecycles created. rt-5.0.5-165-ga8cd5ac657
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/cleanup-lifecycles has been created
at a8cd5ac65757cb6512ba0c21d02765543b19538c (commit)

- Log -----------------------------------------------------------------
commit a8cd5ac65757cb6512ba0c21d02765543b19538c
Author: sunnavy <sunnavy@bestpractical.com>
Date: Thu Feb 8 09:15:01 2024 -0500

Add links to help map statuses that have the same name

diff --git a/etc/cpanfile b/etc/cpanfile
index 07ee82d182..e20d748af1 100644
--- a/etc/cpanfile
+++ b/etc/cpanfile
@@ -52,7 +52,7 @@ requires 'HTTP::Message', '>= 6.07';
requires 'IPC::Run3';
requires 'JavaScript::Minifier::XS';
requires 'JSON';
-requires 'List::MoreUtils';
+requires 'List::MoreUtils', '>= 0.420';
requires 'Locale::Maketext', '>= 1.06';
requires 'Locale::Maketext::Fuzzy', '>= 0.11';
requires 'Locale::Maketext::Lexicon', '>= 0.32';
diff --git a/share/html/Admin/Lifecycles/Mappings.html b/share/html/Admin/Lifecycles/Mappings.html
index 38f3426a23..a39a4d4529 100644
--- a/share/html/Admin/Lifecycles/Mappings.html
+++ b/share/html/Admin/Lifecycles/Mappings.html
@@ -64,7 +64,12 @@
% my $ToMapping = $LifecycleObj->MoveMap($Other);
% my @OtherStatuses = $Other->Valid;

-<&| /Widgets/TitleBox, title => $Other->Name &>
+% require List::MoreUtils;
+<&| /Widgets/TitleBox, title => $Other->Name,
+ List::MoreUtils::duplicates( map { lc } @MyStatuses, @OtherStatuses )
+ ? ( titleright_raw => $titleright )
+ : (),
+&>
<div class="form-row">
<div class="col-6">
<h6 class="text-center my-3"><&|/l, $Other->Name, $LifecycleObj->Name &>From [_1] to [_2]</&></h6>
@@ -104,6 +109,28 @@
</div>

</form>
+
+<script type="text/javascript">
+jQuery('.lifecycle-auto-map').click(function() {
+ jQuery(this).closest('div.titlebox').find('select').each(function() {
+ const select = jQuery(this);
+ if ( select.val() ) {
+ return;
+ }
+ else {
+ const from = select.attr('name').replace(/map-.*--(.*)--.*/, '$1').toLowerCase();
+ select.find('option').each(function() {
+ if ( jQuery(this).attr('value').toLowerCase() === from ) {
+ select.val(jQuery(this).attr('value'));
+ select.selectpicker('refresh');
+ return false;
+ }
+ });
+ }
+ });
+ return false;
+});
+</script>
% }
<%INIT>
my ($title, @results);
@@ -147,6 +174,20 @@ my @lifecycles = map { RT::Lifecycle->Load(Name => $_, Type => $Type) }
@lifecycle_names;

push @results, loc("You only need mappings once you have two more [_1] lifecycles", loc($Type)) unless @lifecycles;
+
+my $alt = loc('Options');
+my $map_same_name = loc('Map same names if unset');
+my $titleright = qq{
+<div class="btn-group dropdown">
+ <a id="history-dropdown" href="#" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false">
+ <span class="fas fa-cog icon-bordered fa-2x" alt="$alt" data-toggle="tooltip" data-placement="top" data-original-title="$alt"></span>
+ </a>
+ <ul class="dropdown-menu dropdown-menu-right">
+ <li class="dropdown-item"><a href="#" class="lifecycle-auto-map">$map_same_name</a></li>
+ </ul>
+</div>
+};
+
</%INIT>
<%ARGS>
$Name => undef

commit 21c6b467d93d0aeac81bd0ece176d7f3722210c4
Author: sunnavy <sunnavy@bestpractical.com>
Date: Fri Feb 2 16:42:48 2024 -0500

Support to update maps of a lifecycle via JSON on Advanced page

diff --git a/lib/RT/Lifecycle.pm b/lib/RT/Lifecycle.pm
index 85544535bf..6e48938ec7 100644
--- a/lib/RT/Lifecycle.pm
+++ b/lib/RT/Lifecycle.pm
@@ -1005,12 +1005,15 @@ sub UpdateLifecycle {
return (1, $CurrentUser->loc("Lifecycle [_1] updated", $name));
}

-=head2 UpdateMaps( CurrentUser => undef, Maps => undef )
+=head2 UpdateMaps( CurrentUser => undef, Maps => undef, Name => undef )

Update lifecycle maps.

Returns (STATUS, MESSAGE). STATUS is true if succeeded, otherwise false.

+Note that the Maps in argument will be merged into existing maps. To delete
+all existing items for a lifecycle before merging, pass its Name.
+
=cut

sub UpdateMaps {
@@ -1018,14 +1021,18 @@ sub UpdateMaps {
my %args = (
CurrentUser => undef,
Maps => undef,
+ Name => undef,
@_,
);

my $CurrentUser = $args{CurrentUser};
my $lifecycles = RT->Config->Get('Lifecycles');

+ my $all_maps = $lifecycles->{__maps__} || {};
+ my @keep_keys = grep { $args{Name} && !/^\Q$args{Name}\E -> | -> \Q$args{Name}\E$/ } keys %$all_maps;
+
%{ $lifecycles->{__maps__} } = (
- %{ $lifecycles->{__maps__} || {} },
+ map( { $_ => $all_maps->{$_} } @keep_keys ),
%{ $args{Maps} },
);

diff --git a/share/html/Admin/Lifecycles/Advanced.html b/share/html/Admin/Lifecycles/Advanced.html
index 09e0c1355e..ddc7452a64 100644
--- a/share/html/Admin/Lifecycles/Advanced.html
+++ b/share/html/Admin/Lifecycles/Advanced.html
@@ -54,10 +54,11 @@
</span>
</div>

-<form action="<%RT->Config->Get('WebPath')%>/Admin/Lifecycles/Advanced.html" name="ModifyLifecycleAdvanced" method="post" enctype="multipart/form-data" class="mx-auto max-width-sm">
+
+<form action="<%RT->Config->Get('WebPath')%>/Admin/Lifecycles/Advanced.html" name="ModifyLifecycleAdvanced" method="post" enctype="multipart/form-data" class="mx-auto max-width-lg">
<input type="hidden" class="hidden" name="Name" value="<% $LifecycleObj->Name %>" />
<input type="hidden" class="hidden" name="Type" value="<% $LifecycleObj->Type %>" />
-
+ <&| /Widgets/TitleBox, title => loc('Basics'), content_class => 'mx-auto width-sm' &>
<div class="form-row">
<span class="col-12">
<textarea class="form-control" rows="30" name="Config"><% $Config |n %></textarea>
@@ -83,8 +84,40 @@
<& /Elements/Submit, Label => loc('Delete'), Name => 'Delete' &>
</div>
</div>
+ </&>
</form>

+
+
+<form action="<%RT->Config->Get('WebPath')%>/Admin/Lifecycles/Advanced.html" name="ModifyLifecycleAdvancedMappings" method="post" enctype="multipart/form-data" class="mx-auto max-width-lg">
+ <input type="hidden" class="hidden" name="Name" value="<% $LifecycleObj->Name %>" />
+ <input type="hidden" class="hidden" name="Type" value="<% $LifecycleObj->Type %>" />
+ <&| /Widgets/TitleBox, title => loc('Mappings'), content_class => 'mx-auto width-sm' &>
+
+ <div class="form-row">
+ <span class="col-12">
+ <textarea class="form-control" rows="30" name="Maps"><% $Maps |n %></textarea>
+ </span>
+ </div>
+
+ <div class="form-row invalid-json hidden">
+ <div class="col-12">
+ <div class="alert alert-danger mb-0"><&|/l&>Invalid JSON</&></div>
+ </div>
+ </div>
+
+ <div class="form-row">
+ <div class="col-6 d-flex">
+ <& /Elements/Submit, Label => loc('Validate Mappings'), Name => 'ValidateMaps' &>
+ </div>
+ <div class="col-6">
+ <& /Elements/Submit, Label => loc('Save Mappings'), Name => 'UpdateMaps' &>
+ </div>
+ </div>
+ </&>
+</form>
+
+
<%INIT>
my ($title, @results);
my $LifecycleObj = RT::Lifecycle->new();
@@ -100,6 +133,13 @@ if ( $ARGS{RedirectedFromModify} && RT::Interface::Web->ClientIsIE() ) {
push @results, loc("The graphical lifecycle builder is not currently supported on IE 11. You can update the lifecycle configuration using the Advanced tab or access the lifecycle editor in a supported browser.");
}

+if ( !defined $Maps && ( my $all_maps = RT->Config->Get('Lifecycles')->{__maps__} ) ) {
+ for my $item ( grep {/^\Q$Name\E -> | -> \Q$Name\E$/} keys %$all_maps ) {
+ $Maps->{$item} = $all_maps->{$item};
+ }
+ $Maps = JSON::to_json( $Maps || {}, { canonical => 1, pretty => 1 } );
+}
+
my $redirect_to ='/Admin/Lifecycles/Advanced.html';
my %redirect_args;

@@ -157,6 +197,41 @@ elsif ( $Delete ) {
);
}
}
+elsif ( $ValidateMaps || $UpdateMaps ) {
+ my $maps = JSON::from_json($Maps || '{}');
+
+ my ( $valid, @warnings )
+ = $LifecycleObj->ValidateLifecycleMaps( Maps => $maps, CurrentUser => $session{CurrentUser} );
+
+ my $updated;
+ if ($valid) {
+ if ($ValidateMaps) {
+ push @results, loc('Mappings is valid');
+ }
+ else {
+ # Maps will be merged into existing value, here we remove existing values so admins can delete items
+
+ ( $updated, my $msg ) = RT::Lifecycle->UpdateMaps(
+ CurrentUser => $session{CurrentUser},
+ Maps => $maps,
+ Name => $Name,
+ );
+ push @results, $msg;
+ }
+
+ }
+ else {
+ push @results, @warnings;
+ }
+
+ %redirect_args = (
+ Name => $Name,
+ Type => $Type,
+
+ # Get the latest canonicalized data if updated successfully
+ $updated ? () : ( Maps => $Maps ),
+ );
+}

MaybeRedirectForResults(
Actions => \@results,
@@ -172,4 +247,7 @@ $Config => undef
$Validate => undef
$Update => undef
$Delete => undef
+$ValidateMaps => undef
+$UpdateMaps => undef
+$Maps => undef
</%ARGS>
diff --git a/share/static/js/util.js b/share/static/js/util.js
index bd42241256..00a5bd4b38 100644
--- a/share/static/js/util.js
+++ b/share/static/js/util.js
@@ -823,7 +823,7 @@ jQuery(function() {
return true;
};

- jQuery('[name=Config]').bind('input propertychange', function() {
+ jQuery('[name=Config], [name=Maps]').bind('input propertychange', function() {
var form = jQuery(this).closest('form');
if ( validate_json(jQuery(this).val()) ) {
form.find('input[type=submit]').prop('disabled', false);
diff --git a/t/web/lifecycle_mappings.t b/t/web/lifecycle_mappings.t
index f9efafa1b7..dcaec2b651 100644
--- a/t/web/lifecycle_mappings.t
+++ b/t/web/lifecycle_mappings.t
@@ -172,6 +172,62 @@ diag "Test updating sales-engineering mappings";
);
}

+diag "Test advanced mappings";
+{
+ $m->get_ok( $url . '/Admin/Lifecycles/Advanced.html?Type=ticket&Name=sales-engineering' );
+ my $form = $m->form_name('ModifyLifecycleAdvancedMappings');
+
+ require JSON;
+ my $maps = JSON::from_json( $form->value('Maps') );
+ is_deeply(
+ $maps,
+ {
+ 'sales-engineering -> default' => {
+ 'rejected' => 'rejected',
+ 'resolved' => 'resolved',
+ 'deleted' => 'deleted',
+ 'engineering' => 'open',
+ 'stalled' => 'stalled',
+ 'sales' => 'new'
+ }
+ },
+ 'Correct current value'
+ );
+
+ $maps->{'default -> sales-engineering'} = { 'new' => 'sales', };
+
+ $m->submit_form_ok(
+ {
+ fields => {
+ Maps => JSON::to_json($maps),
+ Name => "sales-engineering",
+ Type => "ticket",
+ },
+ button => 'UpdateMaps',
+ },
+ 'Update maps'
+ );
+ $m->content_contains('Lifecycle mappings updated');
+ $form = $m->form_name('ModifyLifecycleAdvancedMappings');
+ is_deeply( $maps, JSON::from_json( $form->value('Maps') ), 'Correct updated value' );
+
+ $m->submit_form_ok(
+ {
+ fields => {
+ Maps => '',
+ Name => "sales-engineering",
+ Type => "ticket",
+ },
+ button => 'UpdateMaps',
+ },
+ 'Clear maps'
+ );
+ $m->content_contains('Lifecycle mappings updated');
+ $form = $m->form_name('ModifyLifecycleAdvancedMappings');
+ $form->value( "Maps", "{}", 'Maps got cleared' );
+}
+
+
sub reload_lifecycle {
# to get rid of the warning of:
# you're changing config option in a test file when server is active

commit 4bfab68cf68a7db07aec297c06aae2d7d643a7db
Author: sunnavy <sunnavy@bestpractical.com>
Date: Mon Feb 5 10:17:51 2024 -0500

Test lifecycle warnings on admin web UI

diff --git a/t/lifecycles/basics.t b/t/lifecycles/basics.t
index f53f077cca..bee73b43bb 100644
--- a/t/lifecycles/basics.t
+++ b/t/lifecycles/basics.t
@@ -242,4 +242,16 @@ diag "'!inactive -> inactive' actions are shown even if ticket has unresolved de
);
}

+diag "Test lifecycle warnings on admin pages";
+{
+ no warnings 'redefine';
+ local *RT::Queue::ValidateLifecycle = sub {1};
+ my ( $ret, $msg ) = $general->SetLifecycle('foobar');
+ ok( $ret, $msg );
+ RT->System->LifecycleCacheNeedsUpdate(1);
+ $m->get_ok('/Admin/Lifecycles/');
+ $m->warning_like(qr/Lifecycle foobar is missing/);
+ $m->text_contains('Lifecycle foobar is missing');
+}
+
done_testing;

commit 9fafa620766ef74205b9994d80e8d3fa4e6c4734
Author: sunnavy <sunnavy@bestpractical.com>
Date: Mon Feb 5 10:09:24 2024 -0500

Add protective code in case of invalid lifecycles

This is initially to get rid of server errors for our tests that contain
other unsupported types(e.g. "racecar" type in "t/lifecycles/utils.pl").

diff --git a/share/html/Admin/Lifecycles/index.html b/share/html/Admin/Lifecycles/index.html
index dc1555c8ce..c9f97f4e31 100644
--- a/share/html/Admin/Lifecycles/index.html
+++ b/share/html/Admin/Lifecycles/index.html
@@ -157,6 +157,10 @@ my $get_applied = sub {
RT::Logger->error( 'Unable to load lifecycle ' . $key );
return;
}
+
+ # No applied objects if lifecycle is not a subclass, in case of invalid data
+ return if ref $lifecycle eq 'RT::Lifecycle';
+
my $list_method = $lifecycle->Type() eq 'asset' ? 'Catalogs' : 'Queues';
my $appliedobjs = $lifecycle->$list_method();
$appliedobjs->RowsPerPage($applied_list_limit);

commit 4af6f01af42e1cf965bed86ea184b5dc990fe5dd
Author: sunnavy <sunnavy@bestpractical.com>
Date: Fri Feb 2 15:14:07 2024 -0500

Show lifecycle warnings to admins who are accessing lifecycle pages

This gives admins hints of the issue of %Lifecycles configuration.

diff --git a/lib/RT/Lifecycle.pm b/lib/RT/Lifecycle.pm
index 09a3479eaf..85544535bf 100644
--- a/lib/RT/Lifecycle.pm
+++ b/lib/RT/Lifecycle.pm
@@ -625,9 +625,12 @@ sub CanonicalCase {
return($self->{data}{canonical_case}{lc $status} || lc $status);
}

+our @WARNINGS;
sub FillCache {
my $self = shift;

+ @WARNINGS = ();
+
my $map = RT->Config->Get('Lifecycles') or return;

{
@@ -643,6 +646,7 @@ sub FillCache {
for my $name ( @lifecycles ) {
unless ( $map->{$name} ) {
warn "Lifecycle $name is missing in %Lifecycles config";
+ push @WARNINGS, loc( "Lifecycle [_1] is missing in %Lifecycles config", $name );
}
}
}
@@ -667,6 +671,7 @@ sub FillCache {
my ( $ret, @warnings ) = $self->ValidateLifecycle(Lifecycle => $lifecycle, Name => $name);
unless ( $ret ) {
warn $_ for @warnings;
+ push @WARNINGS, @warnings;
}

my @statuses;
@@ -739,6 +744,7 @@ sub FillCache {
my ( $ret, @warnings ) = $self->ValidateLifecycleMaps();
unless ( $ret ) {
warn $_ for @warnings;
+ push @WARNINGS, @warnings;
}

# Lower-case the transition maps
diff --git a/share/html/Admin/Lifecycles/autohandler b/share/html/Admin/Lifecycles/autohandler
index db74cbe305..a7be91f9d6 100644
--- a/share/html/Admin/Lifecycles/autohandler
+++ b/share/html/Admin/Lifecycles/autohandler
@@ -46,9 +46,21 @@
<%init>
-return $m->call_next(%ARGS)
- if RT->Config->Get('ShowEditLifecycleConfig')
+$m->clear_and_abort(403)
+ unless RT->Config->Get('ShowEditLifecycleConfig')
&& $session{'CurrentUser'}->UserObj->HasRight( Right => 'SuperUser', Object => $RT::System, );

-$m->clear_and_abort(403);
+if ( RT::Interface::Web::RequestENV('REQUEST_METHOD') eq 'GET' && ( my @warnings = @RT::Lifecycle::WARNINGS ) ) {
+
+ # Filter by lifecycle name to show warnings only for that lifecycle
+ if ( $DECODED_ARGS->{Name} ) {
+ @warnings = grep { / \Q$DECODED_ARGS->{Name}\E / } @warnings;
+ }
+
+ if ( @warnings ) {
+ push @{ $session{'Actions'}{''} ||= []}, @warnings;
+ }
+}
+
+$m->call_next(%ARGS);
</%init>

commit b9d6a091171a2036acc980be1797f735e7462f18
Author: sunnavy <sunnavy@bestpractical.com>
Date: Mon Feb 5 10:56:06 2024 -0500

Test lifecycle deletions

diff --git a/t/web/lifecycle.t b/t/web/lifecycle.t
new file mode 100644
index 0000000000..24a7631245
--- /dev/null
+++ b/t/web/lifecycle.t
@@ -0,0 +1,51 @@
+use strict;
+use warnings;
+
+BEGIN { require './t/lifecycles/utils.pl' }
+
+my ( $url, $m ) = RT::Test->started_ok;
+ok( $m->login(), 'logged in' );
+
+diag "Test lifecycle creation";
+
+$m->get_ok('/Admin/Lifecycles/Create.html');
+$m->submit_form_ok(
+ {
+ form_name => 'CreateLifecycle',
+ fields => { Name => ' foobar ', }, # Intentially add spaces to test the auto cleanup.
+ button => 'Create',
+ },
+ 'Create lifecycle foobar'
+);
+
+$m->text_contains( 'foobar', 'Lifecycle foobar created' );
+
+# Test if index page has it too
+$m->follow_link_ok( { text => 'Select', url_regex => qr{/Admin/Lifecycles} } );
+$m->follow_link_ok( { text => 'foobar' } );
+
+my $lifecycle = RT::Lifecycle->new;
+$lifecycle->Load(' foobar ');
+ok( !$lifecycle->Name, 'Lifecycle " foo bar " does not exist' );
+$lifecycle->Load('foobar');
+is( $lifecycle->Name, 'foobar', 'Lifecycle name is corrected to "foobar"' );
+
+# Test more updates
+
+
+diag "Test lifecycle deletion";
+
+$m->follow_link_ok( { url_regex => qr{/Admin/Lifecycles/Advanced.html} } );
+$m->submit_form_ok(
+ {
+ form_name => 'ModifyLifecycleAdvanced',
+ button => 'Delete',
+ },
+ 'Delete lifecycle foobar'
+);
+
+$m->text_contains('Lifecycle foobar deleted');
+$m->follow_link_ok( { text => 'Select', url_regex => qr{/Admin/Lifecycles} } );
+$m->text_lacks( 'foobar', 'foobar is gone' );
+
+done_testing;

commit e19e0556be7d3e271e61ad2756d56a5c839c6d34
Author: sunnavy <sunnavy@bestpractical.com>
Date: Wed Jan 31 19:39:47 2024 -0500

Support to delete lifecycles

diff --git a/lib/RT/Lifecycle.pm b/lib/RT/Lifecycle.pm
index e04f67d5f8..09a3479eaf 100644
--- a/lib/RT/Lifecycle.pm
+++ b/lib/RT/Lifecycle.pm
@@ -905,6 +905,66 @@ sub CreateLifecycle {
return $class->_CreateLifecycle(%args);
}

+=head2 DeleteLifecycle( CurrentUser => undef, Name => undef )
+
+Delete a lifecycle.
+
+Returns (STATUS, MESSAGE). STATUS is true if succeeded, otherwise false.
+
+=cut
+
+sub DeleteLifecycle {
+ my $class = shift;
+ my %args = (
+ CurrentUser => undef,
+ Name => undef,
+ @_,
+ );
+
+ my $CurrentUser = $args{CurrentUser};
+ my $Name = $args{Name};
+
+ return ( 0, $CurrentUser->loc("Lifecycle Name required") ) unless length $Name;
+
+ my $lifecycles = RT->Config->Get('Lifecycles');
+ if ( $lifecycles->{$Name} ) {
+ my $dep_class = ( $lifecycles->{$Name}{'type'} // '' ) eq 'asset' ? 'RT::Catalogs' : 'RT::Queues';
+ my $dep_objects = $dep_class->new( $args{CurrentUser} );
+ $dep_objects->Limit( FIELD => 'Lifecycle', VALUE => $Name );
+ my @dep_names = map { $_->Name } @{ $dep_objects->ItemsArrayRef };
+
+ if (@dep_names) {
+ return (
+ 0,
+ $CurrentUser->loc(
+ "Could not delete lifecycle [_1]: it is used by [_2] [_3]",
+ $Name,
+ (
+ $dep_class eq 'RT::Queues'
+ ? ( @dep_names > 1 ? 'queues' : 'queue' )
+ : ( @dep_names > 1 ? 'catalogs' : 'catalog' )
+ ),
+ join( ', ', @dep_names ),
+ )
+ );
+ }
+ else {
+ delete $lifecycles->{$Name};
+ my $maps = $lifecycles->{__maps__};
+ for my $key ( keys %$maps ) {
+ if ( $key =~ m/^$Name -> / || $key =~ m/ -> $Name$/ ) {
+ delete $maps->{$key};
+ }
+ }
+ my ( $ok, $msg ) = $class->_SaveLifecycles( $lifecycles, $CurrentUser );
+ return ( $ok, $msg ) if !$ok;
+ }
+ }
+
+ return ( 1, $CurrentUser->loc( 'Lifecycle [_1] deleted', $args{Name} ) );
+}
+
+
=head2 UpdateLifecycle( CurrentUser => undef, LifecycleObj => undef, NewConfig => undef, Maps => undef )

Update passed lifecycle to the new configuration.
diff --git a/share/html/Admin/Lifecycles/Advanced.html b/share/html/Admin/Lifecycles/Advanced.html
index 0282c9e062..09e0c1355e 100644
--- a/share/html/Admin/Lifecycles/Advanced.html
+++ b/share/html/Admin/Lifecycles/Advanced.html
@@ -78,7 +78,11 @@
<& /Elements/Submit, Label => loc('Save Changes'), Name => 'Update' &>
</div>
</div>
-
+ <div class="form-row">
+ <div class="col-12">
+ <& /Elements/Submit, Label => loc('Delete'), Name => 'Delete' &>
+ </div>
+ </div>
</form>

<%INIT>
@@ -96,6 +100,9 @@ if ( $ARGS{RedirectedFromModify} && RT::Interface::Web->ClientIsIE() ) {
push @results, loc("The graphical lifecycle builder is not currently supported on IE 11. You can update the lifecycle configuration using the Advanced tab or access the lifecycle editor in a supported browser.");
}

+my $redirect_to ='/Admin/Lifecycles/Advanced.html';
+my %redirect_args;
+
if ( $Validate || $Update ) {
my $lifecycle = JSON::from_json($Config);
my ( $valid, @warnings )
@@ -126,17 +133,36 @@ if ( $Validate || $Update ) {
push @results, @warnings;
}

- MaybeRedirectForResults(
- Actions => \@results,
- Arguments => {
- Name => $LifecycleObj->Name,
- Type => $LifecycleObj->Type,
+ %redirect_args = (
+ Name => $Name,
+ Type => $Type,

- # Get the latest canonicalized data if updated successfully
- $updated ? () : ( Config => $Config ),
- },
+ # Get the latest canonicalized data if updated successfully
+ $updated ? () : ( Config => $Config ),
);
}
+elsif ( $Delete ) {
+ my ( $ret, $msg ) = RT::Lifecycle->DeleteLifecycle(
+ CurrentUser => $session{CurrentUser},
+ Name => $Name,
+ );
+ push @results, $msg;
+ if ( $ret ) {
+ $redirect_to = '/Admin/Lifecycles/';
+ }
+ else {
+ %redirect_args = (
+ Name => $Name,
+ Type => $Type,
+ );
+ }
+}
+
+MaybeRedirectForResults(
+ Actions => \@results,
+ Path => $redirect_to,
+ Arguments => \%redirect_args,
+);

</%INIT>
<%ARGS>
@@ -145,4 +171,5 @@ $Type => undef
$Config => undef
$Validate => undef
$Update => undef
+$Delete => undef
</%ARGS>

commit ce3adb04c2d0e82ecd66d0ed4445caa4f356e7d8
Author: sunnavy <sunnavy@bestpractical.com>
Date: Wed Jan 31 19:33:54 2024 -0500

Do not merge old values for hash configs in database

When admins update configs from web UI, the default values of inputs fully
contain current config values, so there is no need to merge old values after
update. By not merging old values, admins now are able to delete keys from
hash configs.

This is initially to support lifecycle deletions.

diff --git a/lib/RT/Config.pm b/lib/RT/Config.pm
index 96a7994338..e670f3ca45 100644
--- a/lib/RT/Config.pm
+++ b/lib/RT/Config.pm
@@ -3077,10 +3077,9 @@ sub LoadConfigFromDatabase {
: $type eq 'HASH' ? [ %$value ]
: [ $value ];

- # hashes combine, but by default previous config settings shadow
- # later changes, here we want database configs to shadow file ones.
+ # Unlike hashes in files that merge together, database configs are supposed to contain all the data, so no need
+ # to merge file configs. With it, admins are able to delete keys.
if ($type eq 'HASH') {
- $val = [ $self->Get($name), @$val ];
$self->Set($name, ());
}


commit 4f0700d3d3f1dd01c44e41e8dd326ac1ce24ccf3
Author: sunnavy <sunnavy@bestpractical.com>
Date: Wed Jan 31 16:02:11 2024 -0500

Trim any leading and trailing spaces from name on lifecycle create

This is initially to fix broken lifecycle links because the URL with
Name=... doesn't retain the trailing space.

diff --git a/share/html/Admin/Lifecycles/Create.html b/share/html/Admin/Lifecycles/Create.html
index e3a25973f0..8e02113b8b 100644
--- a/share/html/Admin/Lifecycles/Create.html
+++ b/share/html/Admin/Lifecycles/Create.html
@@ -126,6 +126,8 @@ for my $type (@types) {
}

if ($Create) {
+ $Name =~ s!^\s+!!;
+ $Name =~ s!\s+$!!;
my ($ok, $msg) = RT::Lifecycle->CreateLifecycle(
CurrentUser => $session{CurrentUser},
Name => $Name,

commit 9cdf37182900936c865c0ba605a7a7debccccf56
Author: sunnavy <sunnavy@bestpractical.com>
Date: Thu Jul 22 04:00:57 2021 +0800

Clean up lifecycles on save when possible

Here we just clean up obvious errors like nonexistent lifecycles,
statuses, etc. This is initially to sync __maps__ when statuses are
deleted from lifecycles.

diff --git a/lib/RT/Lifecycle.pm b/lib/RT/Lifecycle.pm
index f872404c00..e04f67d5f8 100644
--- a/lib/RT/Lifecycle.pm
+++ b/lib/RT/Lifecycle.pm
@@ -799,6 +799,22 @@ sub _SaveLifecycles {
my $lifecycles = shift;
my $CurrentUser = shift;

+ for my $key ( keys %$lifecycles ) {
+ next if $key eq '__maps__';
+ my ( $ret, @warnings )
+ = $class->ValidateLifecycle( Lifecycle => $lifecycles->{$key}, Name => $key, Cleanup => 1 );
+ unless ($ret) {
+ RT->Logger->debug($_) for @warnings;
+ }
+ }
+
+ if ( $lifecycles->{__maps__} ) {
+ my ( $ret, @warnings ) = $class->ValidateLifecycleMaps( Lifecycles => $lifecycles, Cleanup => 1 );
+ unless ($ret) {
+ RT->Logger->debug($_) for @warnings;
+ }
+ }
+
my $setting = RT::Configuration->new($CurrentUser);
$setting->LoadByCols(Name => 'Lifecycles', Disabled => 0);
if ($setting->Id) {
@@ -953,10 +969,13 @@ sub UpdateMaps {
return (1, $CurrentUser->loc("Lifecycle mappings updated"));
}

-=head2 ValidateLifecycle( CurrentUser => undef, Lifecycle => undef, Name => undef )
+=head2 ValidateLifecycle( CurrentUser => undef, Lifecycle => undef, Name => undef, Cleanup => undef )

Validate passed Lifecycle data structure.

+If Cleanup is true, clean up passed Lifecycle data structure, e.g. removing
+nonexistent statuses.
+
Returns (STATUS, MESSAGE). STATUS is true if succeeded, otherwise false.

=cut
@@ -967,6 +986,7 @@ sub ValidateLifecycle {
CurrentUser => undef,
Lifecycle => undef,
Name => undef,
+ Cleanup => undef,
@_,
);
my $current_user = $args{CurrentUser} || RT->SystemUser;
@@ -995,43 +1015,79 @@ sub ValidateLifecycle {
for my $state ( keys %{ $lifecycle->{defaults} || {} } ) {
my $status = $lifecycle->{defaults}{$state};
if ( $status ) {
- push @warnings, $current_user->loc( "Nonexistent default [_1] status [_2] in [_3] lifecycle", $state, lc $status, $name )
- unless $lifecycle->{canonical_case}{ lc $status };
+ unless ( $lifecycle->{canonical_case}{ lc $status } ) {
+ push @warnings, $current_user->loc( "Nonexistent default [_1] status [_2] in [_3] lifecycle", $state, lc $status, $name );
+ if ( $args{Cleanup} ) {
+ delete $lifecycle->{defaults}{$state};
+ }
+ }
}
else {
push @warnings, $current_user->loc( "Empty default [_1] status in [_2] Lifecycle", $state, $name );
}
}
for my $from ( keys %{ $lifecycle->{transitions} || {} } ) {
- push @warnings, $current_user->loc( "Nonexistent status [_1] in transitions in [_2] lifecycle", lc $from, $name )
- unless $from eq '' || $lifecycle->{canonical_case}{ lc $from };
+ unless ( $from eq '' || $lifecycle->{canonical_case}{ lc $from } ) {
+ push @warnings,
+ $current_user->loc( "Nonexistent status [_1] in transitions in [_2] lifecycle", lc $from, $name );
+ if ( $args{Cleanup} ) {
+ delete $lifecycle->{transitions}{$from};
+ next;
+ }
+ }

for my $status ( @{ ( $lifecycle->{transitions}{$from} ) || [] } ) {
push @warnings, $current_user->loc( "Nonexistent status [_1] in transitions in [_2] lifecycle", lc $status, $name )
unless $lifecycle->{canonical_case}{ lc $status };
}
+
+ # Remove nonexistent statuses
+ if ( $args{Cleanup} ) {
+ $lifecycle->{transitions}{$from}
+ = [. grep { $lifecycle->{canonical_case}{ lc $_ } } @{ ( $lifecycle->{transitions}{$from} ) || [] } ];
+ }
}

for my $schema ( keys %{ $lifecycle->{rights} || {} } ) {
my ( $from, $to ) = split /\s*->\s*/, $schema, 2;
unless ( $from and $to ) {
push @warnings, $current_user->loc( "Invalid right transition [_1] in [_2] lifecycle", $schema, $name );
+ if ( $args{Cleanup} ) {
+ delete $lifecycle->{rights}{$schema};
+ }
next;
}
- push @warnings, $current_user->loc( "Nonexistent status [_1] in right transition in [_2] lifecycle", lc $from, $name )
- unless $from eq '*'
- or $lifecycle->{canonical_case}{ lc $from };
- push @warnings, $current_user->loc( "Nonexistent status [_1] in right transition in [_2] lifecycle", lc $to, $name )
- unless $to eq '*' || $lifecycle->{canonical_case}{ lc $to };
-
- push @warnings,
- $current_user->loc( "Invalid right name ([_1]) in [_2] lifecycle; right names must be ASCII", $lifecycle->{rights}{$schema}, $name )
- if $lifecycle->{rights}{$schema} =~ /\P{ASCII}/;
-
- push @warnings,
- $current_user
- ->loc( "Invalid right name ([_1]) in [_2] lifecycle; right names must be <= 25 characters", $lifecycle->{rights}{$schema}, $name )
- if length( $lifecycle->{rights}{$schema} ) > 25;
+
+ my $invalid;
+ unless ( $from eq '*' or $lifecycle->{canonical_case}{ lc $from } ) {
+ push @warnings,
+ $current_user->loc( "Nonexistent status [_1] in right transition in [_2] lifecycle", lc $from, $name );
+ $invalid = 1;
+ }
+
+ unless ( $to eq '*' or $lifecycle->{canonical_case}{ lc $to } ) {
+ push @warnings,
+ $current_user->loc( "Nonexistent status [_1] in right transition in [_2] lifecycle", lc $to, $name );
+ $invalid = 1;
+ }
+
+ if ( $lifecycle->{rights}{$schema} =~ /\P{ASCII}/ ) {
+ push @warnings,
+ $current_user->loc( "Invalid right name ([_1]) in [_2] lifecycle; right names must be ASCII",
+ $lifecycle->{rights}{$schema}, $name );
+ $invalid = 1;
+ }
+
+ if ( length( $lifecycle->{rights}{$schema} ) > 25 ) {
+ push @warnings,
+ $current_user->loc( "Invalid right name ([_1]) in [_2] lifecycle; right names must be <= 25 characters",
+ $lifecycle->{rights}{$schema}, $name );
+ $invalid = 1;
+ }
+
+ if ( $args{Cleanup} && $invalid ) {
+ delete $lifecycle->{rights}{$schema};
+ }
}

my @actions;
@@ -1044,27 +1100,44 @@ sub ValidateLifecycle {
@actions = @{ $lifecycle->{'actions'} };
}

+ my @valid_actions;
while ( my ( $transition, $info ) = splice @actions, 0, 2 ) {
my ( $from, $to ) = split /\s*->\s*/, $transition, 2;
unless ( $from and $to ) {
push @warnings, $current_user->loc( "Invalid action status change [_1], in [_2] lifecycle", $transition, $name );
next;
}
- push @warnings, $current_user->loc( "Nonexistent status [_1] in actions in [_2] lifecycle", lc $from, $name )
- unless $from eq '*'
- or $lifecycle->{canonical_case}{ lc $from };
- push @warnings, $current_user->loc( "Nonexistent status [_1] in actions in [_2] lifecycle", lc $to, $name )
- unless $to eq '*'
- or $lifecycle->{canonical_case}{ lc $to };
+
+ my $invalid;
+ unless ( $from eq '*' or $lifecycle->{canonical_case}{ lc $from } ) {
+ push @warnings, $current_user->loc( "Nonexistent status [_1] in actions in [_2] lifecycle", lc $from, $name );
+ $invalid = 1;
+ }
+
+ unless ( $to eq '*' or $lifecycle->{canonical_case}{ lc $to } ) {
+ push @warnings, $current_user->loc( "Nonexistent status [_1] in actions in [_2] lifecycle", lc $to, $name );
+ $invalid = 1;
+ }
+
+ if ( $args{Cleanup} && !$invalid ) {
+ push @valid_actions, $transition, $info;
+ }
+ }
+
+ if ( $args{Cleanup} ) {
+ $lifecycle->{'actions'} = \@valid_actions;
}

return @warnings ? ( 0, uniq @warnings ) : 1;
}

-=head2 ValidateLifecycleMaps( CurrentUser => undef )
+=head2 ValidateLifecycleMaps( CurrentUser => undef, Lifecycles => undef, Maps => undef, Cleanup => undef )

Validate lifecycle Maps.

+If Cleanup is true, clean up maps structure, e.g. removing nonexistent
+statuses.
+
Returns (STATUS, MESSAGES). STATUS is true if succeeded, otherwise false.

=cut
@@ -1073,32 +1146,57 @@ sub ValidateLifecycleMaps {
my $self = shift;
my %args = (
CurrentUser => undef,
+ Lifecycles => undef,
+ Maps => undef,
+ Cleanup => undef,
@_,
);
my $current_user = $args{CurrentUser} || RT->SystemUser;

my @warnings;
- for my $mapname ( keys %{ $LIFECYCLES_CACHE{'__maps__'} || {} } ) {
+ my $lifecycles = $args{Lifecycles} || \%LIFECYCLES_CACHE;
+ my $maps = $args{Maps} || $lifecycles->{__maps__} || $LIFECYCLES_CACHE{__maps__} || {};
+
+ for my $mapname ( keys %$maps ) {
my ( $from, $to ) = split /\s*->\s*/, $mapname, 2;
unless ( $from and $to ) {
push @warnings, $current_user->loc( "Invalid lifecycle mapping [_1]", $mapname );
+ if ( $args{Cleanup} ) {
+ delete $maps->{$mapname};
+ }
next;
}
push @warnings, $current_user->loc( "Nonexistent lifecycle [_1] in [_2] lifecycle map", $from, $mapname )
- unless $LIFECYCLES_CACHE{$from};
+ unless $lifecycles->{$from};
push @warnings, $current_user->loc( "Nonexistent lifecycle [_1] in [_2] lifecycle map", $to, $mapname )
- unless $LIFECYCLES_CACHE{$to};
+ unless $lifecycles->{$to};

# Ignore mappings referring to disabled lifecycles
next if $LIFECYCLES_CACHE{$from} && $LIFECYCLES_CACHE{$from}{disabled};
next if $LIFECYCLES_CACHE{$to} && $LIFECYCLES_CACHE{$to}{disabled};

- my $map = $LIFECYCLES_CACHE{'__maps__'}{$mapname};
+ if ( $args{Cleanup} && ( !$lifecycles->{$from} || !$lifecycles->{$to} ) ) {
+ delete $maps->{$mapname};
+ next;
+ }
+
+ my $map = $maps->{$mapname};
+ my $new_map = {};
for my $status ( keys %{$map} ) {
push @warnings, $current_user->loc( "Nonexistent status [_1] in [_2] in [_3] lifecycle map", lc $status, $from, $mapname )
- if $LIFECYCLES_CACHE{$from} && !$LIFECYCLES_CACHE{$from}{canonical_case}{ lc $status };
+ if $lifecycles->{$from} && !$lifecycles->{$from}{canonical_case}{ lc $status };
push @warnings, $current_user->loc( "Nonexistent status [_1] in [_2] in [_3] lifecycle map", lc $map->{$status}, $to, $mapname )
- if $LIFECYCLES_CACHE{$to} && !$LIFECYCLES_CACHE{$to}{canonical_case}{ lc $map->{$status} };
+ if $lifecycles->{$to} && !$lifecycles->{$to}{canonical_case}{ lc $map->{$status} };
+ $new_map->{$status} = $map->{$status}
+ if $args{Cleanup}
+ && $lifecycles->{$from}
+ && $lifecycles->{$from}{canonical_case}{ lc $status }
+ && $lifecycles->{$to}
+ && $lifecycles->{$to}{canonical_case}{ lc $map->{$status} };
+ }
+
+ if ( $args{Cleanup} ) {
+ $maps->{$mapname} = $new_map;
}
}


commit b4531d64f0b7c1a6289767d1065e83ec209c5306
Author: sunnavy <sunnavy@bestpractical.com>
Date: Thu Jul 22 05:30:39 2021 +0800

Fix typo of "nonexistent"

diff --git a/lib/RT/Lifecycle.pm b/lib/RT/Lifecycle.pm
index 29382ef2a2..f872404c00 100644
--- a/lib/RT/Lifecycle.pm
+++ b/lib/RT/Lifecycle.pm
@@ -995,7 +995,7 @@ sub ValidateLifecycle {
for my $state ( keys %{ $lifecycle->{defaults} || {} } ) {
my $status = $lifecycle->{defaults}{$state};
if ( $status ) {
- push @warnings, $current_user->loc( "Nonexistant default [_1] status [_2] in [_3] lifecycle", $state, lc $status, $name )
+ push @warnings, $current_user->loc( "Nonexistent default [_1] status [_2] in [_3] lifecycle", $state, lc $status, $name )
unless $lifecycle->{canonical_case}{ lc $status };
}
else {
@@ -1003,11 +1003,11 @@ sub ValidateLifecycle {
}
}
for my $from ( keys %{ $lifecycle->{transitions} || {} } ) {
- push @warnings, $current_user->loc( "Nonexistant status [_1] in transitions in [_2] lifecycle", lc $from, $name )
+ push @warnings, $current_user->loc( "Nonexistent status [_1] in transitions in [_2] lifecycle", lc $from, $name )
unless $from eq '' || $lifecycle->{canonical_case}{ lc $from };

for my $status ( @{ ( $lifecycle->{transitions}{$from} ) || [] } ) {
- push @warnings, $current_user->loc( "Nonexistant status [_1] in transitions in [_2] lifecycle", lc $status, $name )
+ push @warnings, $current_user->loc( "Nonexistent status [_1] in transitions in [_2] lifecycle", lc $status, $name )
unless $lifecycle->{canonical_case}{ lc $status };
}
}
@@ -1018,10 +1018,10 @@ sub ValidateLifecycle {
push @warnings, $current_user->loc( "Invalid right transition [_1] in [_2] lifecycle", $schema, $name );
next;
}
- push @warnings, $current_user->loc( "Nonexistant status [_1] in right transition in [_2] lifecycle", lc $from, $name )
+ push @warnings, $current_user->loc( "Nonexistent status [_1] in right transition in [_2] lifecycle", lc $from, $name )
unless $from eq '*'
or $lifecycle->{canonical_case}{ lc $from };
- push @warnings, $current_user->loc( "Nonexistant status [_1] in right transition in [_2] lifecycle", lc $to, $name )
+ push @warnings, $current_user->loc( "Nonexistent status [_1] in right transition in [_2] lifecycle", lc $to, $name )
unless $to eq '*' || $lifecycle->{canonical_case}{ lc $to };

push @warnings,
@@ -1050,10 +1050,10 @@ sub ValidateLifecycle {
push @warnings, $current_user->loc( "Invalid action status change [_1], in [_2] lifecycle", $transition, $name );
next;
}
- push @warnings, $current_user->loc( "Nonexistant status [_1] in actions in [_2] lifecycle", lc $from, $name )
+ push @warnings, $current_user->loc( "Nonexistent status [_1] in actions in [_2] lifecycle", lc $from, $name )
unless $from eq '*'
or $lifecycle->{canonical_case}{ lc $from };
- push @warnings, $current_user->loc( "Nonexistant status [_1] in actions in [_2] lifecycle", lc $to, $name )
+ push @warnings, $current_user->loc( "Nonexistent status [_1] in actions in [_2] lifecycle", lc $to, $name )
unless $to eq '*'
or $lifecycle->{canonical_case}{ lc $to };
}
@@ -1084,9 +1084,9 @@ sub ValidateLifecycleMaps {
push @warnings, $current_user->loc( "Invalid lifecycle mapping [_1]", $mapname );
next;
}
- push @warnings, $current_user->loc( "Nonexistant lifecycle [_1] in [_2] lifecycle map", $from, $mapname )
+ push @warnings, $current_user->loc( "Nonexistent lifecycle [_1] in [_2] lifecycle map", $from, $mapname )
unless $LIFECYCLES_CACHE{$from};
- push @warnings, $current_user->loc( "Nonexistant lifecycle [_1] in [_2] lifecycle map", $to, $mapname )
+ push @warnings, $current_user->loc( "Nonexistent lifecycle [_1] in [_2] lifecycle map", $to, $mapname )
unless $LIFECYCLES_CACHE{$to};

# Ignore mappings referring to disabled lifecycles
@@ -1095,9 +1095,9 @@ sub ValidateLifecycleMaps {

my $map = $LIFECYCLES_CACHE{'__maps__'}{$mapname};
for my $status ( keys %{$map} ) {
- push @warnings, $current_user->loc( "Nonexistant status [_1] in [_2] in [_3] lifecycle map", lc $status, $from, $mapname )
+ push @warnings, $current_user->loc( "Nonexistent status [_1] in [_2] in [_3] lifecycle map", lc $status, $from, $mapname )
if $LIFECYCLES_CACHE{$from} && !$LIFECYCLES_CACHE{$from}{canonical_case}{ lc $status };
- push @warnings, $current_user->loc( "Nonexistant status [_1] in [_2] in [_3] lifecycle map", lc $map->{$status}, $to, $mapname )
+ push @warnings, $current_user->loc( "Nonexistent status [_1] in [_2] in [_3] lifecycle map", lc $map->{$status}, $to, $mapname )
if $LIFECYCLES_CACHE{$to} && !$LIFECYCLES_CACHE{$to}{canonical_case}{ lc $map->{$status} };
}
}
diff --git a/lib/RT/Record.pm b/lib/RT/Record.pm
index 3a87daf590..96729ed08b 100644
--- a/lib/RT/Record.pm
+++ b/lib/RT/Record.pm
@@ -1010,7 +1010,7 @@ sub _UpdateAttributes {
"The new value has been set.", # loc
"No column specified", # loc
"Immutable field", # loc
- "Nonexistant field?", # loc
+ "Nonexistent field?", # loc
"Invalid data", # loc
"Couldn't find row", # loc
"Missing a primary key?: [_1]", # loc

-----------------------------------------------------------------------


hooks/post-receive
--
rt
_______________________________________________
rt-commit mailing list
rt-commit@lists.bestpractical.com
https://lists.bestpractical.com/mailman/listinfo/rt-commit