Check for Kernel::System::Web::Request in the plugin OTOBO::Perl::ParamObject

[bschmalhofer]

The plugin OTOBO::Perl::ParamObject checks for occurrence of the string "ParamObject", probably intending to restrict the use of the param object. But modules don't have to use the name ! ParamObject". E.g in ./Kernel/System/SysConfig/ValueType/Entity/State.pm:

sub EntityLookupFromWebRequest {
    my ( $Self, %Param ) = @_;

    my $StateID = $Kernel::OM->Get('Kernel::System::Web::Request')->GetParam( Param => 'ID' ) // '';

    return if !$StateID;

    return $Kernel::OM->Get('Kernel::System::State')->StateLookup( StateID => $StateID );
}

I suggest to enhance the check to eally check for Kernel::System::Web::Request. It might also be sensible to turn that plugin into a Perl::Critic policy.

See also #44 .

[svenoe]

Actually it would make more sense to just search the files for Kernel::System::Web::Request, as I guess it should be allowed to use a ParamObject which was passed from a Kernel/Modules module e.g..

(Of course this is not totally fool proof, but it should do its job in 99% of the cases.)

[stefanhaerter]

Is there an opinion on which branches this should be implemented for?

[bschmalhofer]

In my opinon this could be implemented for all release series. Code changes can be done on a case by case basis. For new warnings I think that adding ## nofilter(TidyAll::Plugin::OTOBO::Perl::ParamObject) would be sufficient initially.

[stefanhaerter]

Changing the policy resulted in 10 Files not passing under Kernel/System/ and no hits in bin/ on rel-11_0. I'll have a look for about 10 or 15 minutes; suggest to briefly discuss this in one of the upcoming dev meetings.

[stefanhaerter]

As a decision has not been made yet, I un-assign myself from the issue.