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

Sector Nord AG: New feature - Refactor of search function to allow multi-page search #534

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

ZTrotter
Copy link
Contributor

Proposed change

Currently, when searching for dynamic fields in the AdminDynamicField mask, the search functionality is limited to dynamic fields displayed on the current page. This is not ideal, as it means that searching for a dynamic field may involve searching several pages before finding a result.

A dynamic field can be found by searching on a page where it appears
image

But not on a page where it doesn't appear
image

The suggested change is to rebuild the search functionality to allow for a search across multiple pages.

Type of change

  • '1 - 🚀 feature'

Additional information

Replication

  • Navigate to AdminDynamicField
  • Enter a search query for an existing dynamic field that is not displayed on the current page

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 passed.(❕)
  • Local UnitTests / Selenium passed.(❕)
  • GitHub workflow CI (UnitTests / Selenium) passed.(❗)

@mbethke
Copy link

mbethke commented Jan 17, 2024

Nice :)
One suggestion for _SearchDynamicField: that would be a good case for pulling the OM call out of the loop. But if you do that already, you might as well boil it down all the way:

  sub _SearchDynamicField {
      my ( $Self, %Param ) = @_;
      my $DynamicFieldObject = $Kernel::OM->Get('Kernel::System::DynamicField');
      return grep { $DynamicFieldObject->DynamicFieldGet( ID => $DynamicFieldID )->{Name} =~ /\Q$Param{Search}\E/i } @{ $Param{DynamicFields} }
  }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants