[Rt-commit] rt branch 5.0/cleanup-lifecycles created. rt-5.0.5-165-g46fe0f380d

BPS Git Server git at git.bestpractical.com
Tue Feb 6 13:29:39 UTC 2024


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  46fe0f380dd354916514c0f566541bdf4426bf03 (commit)

- Log -----------------------------------------------------------------
commit 46fe0f380dd354916514c0f566541bdf4426bf03
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Fri Feb 2 18:09:12 2024 -0500

    Automatically map all statuses that have the same name in different lifecycles

diff --git a/lib/RT/Lifecycle.pm b/lib/RT/Lifecycle.pm
index b978a4c04e..9faaeea571 100644
--- a/lib/RT/Lifecycle.pm
+++ b/lib/RT/Lifecycle.pm
@@ -546,7 +546,7 @@ sub MoveMap {
     my $from = shift; # self
     my $to = shift;
     $to = RT::Lifecycle->Load( Name => $to, Type => $from->Type ) unless ref $to;
-    return $LIFECYCLES{'__maps__'}{ $from->Name .' -> '. $to->Name } || {};
+    return $LIFECYCLES_CACHE{'__maps__'}{ $from->Name .' -> '. $to->Name } || {};
 }
 
 =head3 HasMoveMap
@@ -655,6 +655,7 @@ sub FillCache {
     %LIFECYCLES_CACHE = %LIFECYCLES = %$map;
     $_ = { %$_ } foreach values %LIFECYCLES_CACHE;
 
+    my %all_statuses;
     foreach my $name ( keys %LIFECYCLES_CACHE ) {
         next if $name eq "__maps__";
         my $lifecycle = $LIFECYCLES_CACHE{$name};
@@ -714,6 +715,7 @@ sub FillCache {
         my %seen;
         @statuses = grep !$seen{ lc $_ }++, @statuses;
         $lifecycle->{''} = \@statuses;
+        push @{$all_statuses{$type}{lc $_} ||= []}, $name for @statuses;
 
         unless ( $lifecycle->{'transitions'}{''} ) {
             $lifecycle->{'transitions'}{''} = [ grep lc $_ ne 'deleted', @statuses ];
@@ -760,6 +762,20 @@ sub FillCache {
         }
     }
 
+
+    # Automatically map statuses with the same name
+    for my $type ( keys %all_statuses ) {
+        for my $status ( keys %{$all_statuses{$type}} ) {
+            my @lifecycles = @{ $all_statuses{$type}{$status} };
+            for my $from ( @lifecycles ) {
+                for my $to ( @lifecycles ) {
+                    next if $from eq $to;
+                    $LIFECYCLES_CACHE{'__maps__'}{"$from -> $to"}{$status} ||= $status;
+                }
+            }
+        }
+    }
+
     for my $type (keys %LIFECYCLES_TYPES) {
         for my $category ( qw(initial active inactive), '' ) {
             my %seen;
diff --git a/t/web/lifecycle_mappings.t b/t/web/lifecycle_mappings.t
index dcaec2b651..43303d3517 100644
--- a/t/web/lifecycle_mappings.t
+++ b/t/web/lifecycle_mappings.t
@@ -182,6 +182,18 @@ diag "Test advanced mappings";
     is_deeply(
         $maps,
         {
+            'triage -> sales-engineering' => {
+                'resolved' => 'resolved'
+            },
+            'default -> sales-engineering' => {
+                'rejected' => 'rejected',
+                'resolved' => 'resolved',
+                'deleted'  => 'deleted',
+                'stalled'  => 'stalled'
+            },
+            'sales-engineering -> triage' => {
+                'resolved' => 'resolved'
+            },
             'sales-engineering -> default' => {
                 'rejected'    => 'rejected',
                 'resolved'    => 'resolved',
@@ -194,7 +206,7 @@ diag "Test advanced mappings";
         'Correct current value'
     );
 
-    $maps->{'default -> sales-engineering'} = { 'new' => 'sales', };
+    $maps->{'default -> sales-engineering'}{new} = 'sales';
 
     $m->submit_form_ok(
         {

commit ce47e76e4620f0def301b956bf4967e652c0d087
Author: sunnavy <sunnavy at 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 0f7d08b257..b978a4c04e 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/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 b673bc75b23236da699295bb333b1d8827c1c193
Author: sunnavy <sunnavy at 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..7b763ebafc 100644
--- a/t/lifecycles/basics.t
+++ b/t/lifecycles/basics.t
@@ -242,4 +242,15 @@ diag "'!inactive -> inactive' actions are shown even if ticket has unresolved de
     );
 }
 
+diag "Test lifecycle warnings on admin pages";
+{
+    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 04b1c87d950971e01f901329b7ca7aa60a093d9a
Author: sunnavy <sunnavy at 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 d2543f35389aa1bda558788771b340a959a6da8c
Author: sunnavy <sunnavy at 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 cff0275ebc..0f7d08b257 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 @@
 %#
 %# END BPS TAGGED BLOCK }}}
 <%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 da8373268f81bc4aa7643205e474d522506d5889
Author: sunnavy <sunnavy at 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..a165253243
--- /dev/null
+++ b/t/web/lifecycle.t
@@ -0,0 +1,46 @@
+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' } );
+
+
+# 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 9fa6147b63f2807c842598ee089549ac04e3b474
Author: sunnavy <sunnavy at 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 f29c32aeda..cff0275ebc 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 dbfa304457251493c0fb6c4bb7753548582c6756
Author: sunnavy <sunnavy at 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 aa4426ade886ad53573601f3f4be09cb5b14a300
Author: sunnavy <sunnavy at 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 f7b3dc76cd7c7b2753fad43712cb19a0f2369bfd
Author: sunnavy <sunnavy at 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..f29c32aeda 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 )
+                    unless $lifecycle->{canonical_case}{ lc $status };
+                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 };
         }
+
+        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 at 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


More information about the rt-commit mailing list