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

fix: Fix bulk action in search command #528

Merged
merged 1 commit into from
Apr 5, 2024
Merged

fix: Fix bulk action in search command #528

merged 1 commit into from
Apr 5, 2024

Conversation

arjankowski
Copy link
Contributor

No description provided.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 8523388514

Details

  • 4 of 5 (80.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 86.597%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/commands/search.js 4 5 80.0%
Totals Coverage Status
Change from base Build 8171494392: 0.02%
Covered Lines: 4199
Relevant Lines: 4697

💛 - Coveralls

@arjankowski
Copy link
Contributor Author

The SearchCommand was not working correctly when executing bulk actions, whereas it worked fine for single executions.

When executing a bulk command, parameters are prepared separately for each data row, and for this purpose, the _addFlagToArgv function from box-command.js is called. In its logic, this function checks the type of the passed argument, this.constructor.flags[flag].type === 'boolean'. For it to work correctly, the checked flag must be added to the flag list of the given command.
However, in our case, SearchCommand.flags did not have the max-items flag, which is added manually into the body of the run function, and that's why we were getting the error:

TypeError: Cannot read property 'type' of undefined

To fix this, I've added the max-items flag to the SearchCommand so that the existing mechanism would work correctly. However, I set this flag to hidden so that it is not visible to the user.

Copy link
Contributor

@mwwoda mwwoda left a comment

Choose a reason for hiding this comment

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

LGTM!

@arjankowski arjankowski merged commit 782b0e6 into main Apr 5, 2024
13 checks passed
@arjankowski arjankowski deleted the aj/fix_search branch April 5, 2024 10:36
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