Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

5.0/cleanup lifecycles #385

Closed
wants to merge 11 commits into from
Closed

5.0/cleanup lifecycles #385

wants to merge 11 commits into from

Conversation

sunnavy
Copy link
Member

@sunnavy sunnavy commented Feb 9, 2024

No description provided.

Here we just clean up obvious errors like nonexistent lifecycles,
statuses, etc. This is initially to sync __maps__ when statuses are
deleted from lifecycles.
This is initially to fix broken lifecycle links because the URL with
Name=... doesn't retain the trailing space.
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.
This gives admins hints of the issue of %Lifecycles configuration.
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").
@cbrandtbuffalo cbrandtbuffalo self-requested a review March 22, 2024 19:22
Copy link
Member

@cbrandtbuffalo cbrandtbuffalo left a comment

Choose a reason for hiding this comment

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

  • Need to add a minimum version for List::MoreUtils:

Undefined subroutine &List::MoreUtils::duplicates called at /Users/jbrandt/work/bps/rt/share/html/Admin/Lifecycles/Mappings.html line 103.

  • For the new "Map same names if unset", let's add a gearbox and make it the first menu item.

  • On the advanced page, I created some invalid JSON and clicked Validate Mappings and got the below. 2 possible fixes: 1) Add a validate button like the block above to validate the JSON and 2) eval the JSON call.

error: malformed JSON string, neither tag, array, object, number, string or atom, at character offset 676 (before "},\r\n "Testing ->...") at /Users/jbrandt/perl5/perlbrew/perls/perl-5.20.3/lib/site_perl/5.20.3/JSON.pm line 190.
context:
...
186: $json->$method( $opt->{$method} );
187: }
188: }
189:
190: return $json->decode( $_[0] );
191: }

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 };
Copy link
Member

Choose a reason for hiding this comment

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

I think we can drop this unless since it's added above?


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} ) {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a comment here to indicate the typical thing "Cleanup" will do?

$m->text_contains('Lifecycle foobar deleted');
$m->follow_link_ok( { text => 'Select', url_regex => qr{/Admin/Lifecycles} } );
$m->text_lacks( 'foobar', 'foobar is gone' );

Copy link
Member

Choose a reason for hiding this comment

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

Add tests for the leading/trailing space in lifecycle name fix also?

Copy link
Member Author

Choose a reason for hiding this comment

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

The lifecycle name was initially " foobar " on create page and has been auto-corrected to "foobar". I'll add a bit more tests to make sure the name is corrected.

@bestpractical-mirror bestpractical-mirror deleted the 5.0/cleanup-lifecycles branch March 26, 2024 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants