Sector Nord AG: Added TransitionAction parameter placeholder

[jsinagowitz]

Proposed change

Additional feature that provides a placeholder for adding parameters to TransitionActions.
Provided by Sector Nord AG as discussed with Roy Kaldung (@rkaldung) and Henrik Vetter (@Tronsy).

Type of change

• '1 - 🚀 feature'

Additional information

The placeholders can be modified in the System Configuration. The required parameters are marked with the tag (* required).

Checklist

• The code change is tested and works locally.(❗)
• There is no commented out code in this PR.(❕)
• You improved or added new unit tests.(❕)
• Local ZnunyCodePolicy run passes successfully.(❕)
• Local unit tests pass.(❕)
• GitHub workflow ZnunyCodePolicy passes.(❗)
• GitHub workflow unit tests pass.(❗)

[dennykorsukewitz]

Hi @jsinagowitz ,

First of all, the positive statements. 🚀
Thank you for this really good idea. I have already tested it and it is definitely a MUST HAVE for the framework. 👍

Unfortunately, it is not possible to preview the current status correctly.

In the Kernel/Modules/AdminProcessManagementTransitionAction.pm, 1546 changes are displayed. In reality, the diff looks different.
I can't edit it like that.

145c145
<         # set entity sync state
---
>         # set entitty sync state
333c333
<         # set entity sync state
---
>         # set entitty sync state
414,436d413
<     # Add default parameter
<     # ------------------------------------------------------------ #
<     elsif ( $Self->{Subaction} eq 'GetDefaultConfigParameters' ) {
<
<         my $Module = $ParamObject->GetParam( Param => 'Module' );
<         my %ConfigParameter = $Self->_GetDefaultConfigParameters(
<             Module => $Module,
<         );
<
<         my $JSON = $LayoutObject->JSONEncode(
<             Data => {
<                 %ConfigParameter
<             }
<         );
<         return $LayoutObject->Attachment(
<             ContentType => 'application/json; charset=' . $LayoutObject->{Charset},
<             Content     => $JSON || '',
<             Type        => 'inline',
<             NoCache     => 1,
<         );
<     }
<
<     # ------------------------------------------------------------ #
773,796c750
< sub _GetDefaultConfigParameters {
<     my ( $Self, %Param ) = @_;
<
<     for my $Needed (qw(Module)) {
<         if ( !$Param{$Needed} ) {
<             $Kernel::OM->Get('Kernel::System::Log')->Log(
<                 Priority => 'error',
<                 Message  => 'Need '.$Needed.'!'
<             );
<             return;
<         }
<     }
<
<     # replace e.g. 'Kernel::System::ProcessManagement::TransitionAction::TicketCreate' to 'TicketCreate'
<     if ($Param{Module} =~ m/TransitionAction::(.+)$/) {
<         my $Settings = $Kernel::OM->Get('Kernel::Config')->Get('TransitionActionDefaultParameter::Settings');
<         if ( IsHashRefWithData($Settings) && IsHashRefWithData($Settings->{ $1 }) ) {
<             return %{ $Settings->{ $1 } };
<         }
<     }
<     return;
< }
<
< 1;
---
> 1;
\ No newline at end of file

The same applies to the following files:

• Kernel/Language/en.pm
• kernel/modules/AdminProcessManagementTransitionAction.pm
• scripts/test/ProcessManagement/AdminProcessManagementTransitionActionParameter.t
• var/httpd/htdocs/js/Core.Agent.Admin.ProcessManagement.js

Todo

Since I cannot go into detail, I will simply give you a few points to consider.

• Fix git (github) diff for a better review like Kernel/Config/Files/XML/ProcessManagement.xml
• SysConfig-Name should end with ###Framework, to loop over other settings (package). Please change to ProcessManagement::TransitionAction::DefaultParameters###Framework
• make sure you can use these functions GetDefaultConfigParameters also for e.g. Transitions or something else in process context

I look forward to your changes. 😃 🚀

[jsinagowitz]

Hi @dennykorsukewitz

thank you for your feedback. I just started to improve my pull request.

I already found the problem with the high number of changes in some files. Unfortunately, my code editor changed the EOL to CRLF. I fixed that and updated three files in my pull request.

My first question is about the Sysconfig. Your proposal is to name the config ProcessManagement::TransitionAction::DefaultParameters###Framework. I did not find other Sysconfig-Names with ...###Framework at the end, but ...###001-Framework. Shall I better change the Sysconfg-Name to ProcessManagement::TransitionAction::DefaultParameters###001-Framework?

My second question is about the functionality of GetDefaultConfigParameters. I am not sure about the benefit to use the function for transitions. There is not the same kind of key-value pair needed when creating a new transition. We did not have that use-case for our customers yet and I think there is more coding necessary to get this feature done, e.g. a new config structure. Maybe we can deliver that feature later if there is a demand.

Best regards 🚀

[dennykorsukewitz]

Hi @jsinagowitz

I already found the problem with the high number of changes in some files. Unfortunately, my code editor changed the EOL to CRLF. I fixed that and updated three files in my pull request.

👍🏼 nice. This is so much better.

My first question is about the Sysconfig. Your proposal is to name the config ProcessManagement::TransitionAction::DefaultParameters###Framework. I did not find other Sysconfig-Names with ...###Framework at the end, but ...###001-Framework. Shall I better change the Sysconfg-Name to ProcessManagement::TransitionAction::DefaultParameters###001-Framework?

👍🏼 001-Framework sounds also better.

My second question is about the functionality of GetDefaultConfigParameters. I am not sure about the benefit to use the function for transitions. There is not the same kind of key-value pair needed when creating a new transition. We did not have that use-case for our customers yet and I think there is more coding necessary to get this feature done, e.g. a new config structure. Maybe we can deliver that feature later if there is a demand.

I agree. That's how we'll do it for now.
Then please make the changes before I start the review. Thank you

🚀 💯

[jsinagowitz]

Hi @dennykorsukewitz

I just updated the sysconfig name. I hope now is everything fine.

Thank you & best regards 🚀

[dennykorsukewitz]

Hi @jsinagowitz,

here are my adjustments.
In general, everything looks good. 👍🏼

• AdminProcessManagementTransitionAction: Loop over SysConfig (for Packages)
• UnitTest: extended
• JS: Division into individual functions
• Tidied

If you have any questions you can reach me via Discord.

🚀 Thank you

[dennykorsukewitz]

Applied ZnunyCodePolicy

[dennykorsukewitz]

Hi @jsinagowitz,

so last changes.
NEEDED was missing, my fault.
Everything else fits then.

[jsinagowitz]

@dennykorsukewitz I just fixed the code policy problem in the JS too.