Skip to content

Commit

Permalink
Automatically map all statuses that have the same name in different l…
Browse files Browse the repository at this point in the history
…ifecycles
  • Loading branch information
sunnavy committed Feb 6, 2024
1 parent 3679823 commit 758f89f
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 2 deletions.
18 changes: 17 additions & 1 deletion lib/RT/Lifecycle.pm
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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};
Expand Down Expand Up @@ -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 ];
Expand Down Expand Up @@ -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;
Expand Down
14 changes: 13 additions & 1 deletion t/web/lifecycle_mappings.t
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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(
{
Expand Down

4 comments on commit 758f89f

@elacour
Copy link
Contributor

@elacour elacour commented on 758f89f Feb 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's maybe not a good idea. Having an empty map between lifecycles allows us to prevent moving tickets between those lifecycle which cannot always be achieved using acls.

@cbrandtbuffalo
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the interest of helping the most common case, we're trying to make this easier for what we think most people want to do. We can still allow a user to unset the automatic mappings to remove a mapping.

@elacour
Copy link
Contributor

@elacour elacour commented on 758f89f Feb 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand, I have one RT using an empty mapping for this purpose. Having it configurable could be nice in such case, but RT has already so many configurables options :(

@sunnavy
Copy link
Member Author

@sunnavy sunnavy commented on 758f89f Feb 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree that automatic mapping might be surprising, so I'm going to drop it.

Instead, I'm considering adding a button on Mappings page to help admins map such statuses easily.

Please sign in to comment.