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

Fixed Inventory checks table filters by stats #4999

Merged
merged 5 commits into from
Dec 19, 2022

Conversation

Machi3mfl
Copy link
Member

@Machi3mfl Machi3mfl commented Dec 14, 2022

Description

Hi Team, This PR fixes the behavior when the Inventory Stat is clicked.
Closes #4952

Test case

  • When the Inventory Stat is clicked the Check table must be filtered
Peek.2022-12-14.13-36.mp4

Tests

  1. Scenario The user can add a filter by clicking on the result count indicator (Pass) of the Security configuration assessment
    policy checks
    Given SCA data is available
    When the user is seeing the table of checks of a Security configuration assessment
    policy
    And the user clicks on any result count indicator
    Then a filter must be added to the search bar
    And the table should be filtered by the added filter

  2. Scenario The user can add a filter by clicking on the result count indicator (Fail) of the Security configuration assessment
    policy checks
    Given SCA data is available
    When the user is seeing the table of checks of a Security configuration assessment
    policy
    And the user clicks on any result count indicator
    Then a filter must be added to the search bar
    And the table should be filtered by the added filter

  3. Scenario The user can add a filter by clicking on the result count indicator (Not applicable) of the Security configuration assessment
    policy checks
    Given SCA data is available
    When the user is seeing the table of checks of a Security configuration assessment
    policy
    And the user clicks on any result count indicator
    Then a filter must be added to the search bar
    And the table should be filtered by the added filter

Check List

  • All tests pass
    • yarn test:jest
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

@Machi3mfl Machi3mfl requested a review from a team as a code owner December 14, 2022 16:34
@Machi3mfl Machi3mfl changed the title Fixed checks table filters by stats Fixed Inventory checks table filters by stats Dec 14, 2022
@Machi3mfl Machi3mfl linked an issue Dec 14, 2022 that may be closed by this pull request
Copy link
Member

@Desvelao Desvelao left a comment

Choose a reason for hiding this comment

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

review

Tests

Legend:
🟢 Pass
🟡 Warning
🔴 Fail

Results:

Scenario Result
1 🟢
2 🟢
3 🔴

Details:

Scenario 1 - 🟢

image

Scenario 2 - 🟢

image

Scenario 3 - 🔴

I found a problem by clicking on the Not applicable result count indicator, the table doesn't display any results.

image

I reviewed the request done when adding this filter and it uses the result query paramater with not applicable value. This value is not the correct one to get the expected checks.
image

If we review the value for any check that the UI displays as Not applicable, we see this is an empty string.
image

So, we expected if we change the value of the result query parameter to an empty string, we could get the expected results. By my tests, it is not working, so I guess the Wazuh API could not be expecting this case. We should consult it with the @wazuh/framework team.

In previous versions (ex: 4.3.10), this worked correctly and this was because in that version, all the checks are obtained and the filter are managed in the frontend.

@Desvelao
Copy link
Member

review

Tests

Legend: green_circle Pass yellow_circle Warning red_circle Fail

Results:

Scenario Result
1
2
3
Details:

Scenario 1 -
Scenario 2 -
Scenario 3 -
I found a problem by clicking on the Not applicable result count indicator, the table doesn't display any results.

image

I reviewed the request done when adding this filter and it uses the result query paramater with not applicable value. This value is not the correct one to get the expected checks. image

If we review the value for any check that the UI displays as Not applicable, we see this is an empty string. image

So, we expected if we change the value of the result query parameter to an empty string, we could get the expected results. By my tests, it is not working, so I guess the Wazuh API could not be expecting this case. We should consult it with the @wazuh/framework team.

In previous versions (ex: 4.3.10), this worked correctly and this was because in that version, all the checks are obtained and the filter are managed in the frontend.

We had a meeting with @davidjiglesias and maybe we could get a workaround:
Use status query parameter instead of result.

For another hand, we could remove the empty string as a possible value of result filter so the users can't filter and get unexpected results.

We should review the suggestions for status too.

@Machi3mfl
Copy link
Member Author

Machi3mfl commented Dec 16, 2022

Fixed Not Applicable filter using the status field

Peek.2022-12-16.10-28.mp4

The solution applied was mentioned in comment

@Machi3mfl Machi3mfl requested a review from a team December 16, 2022 15:43
@chantal-kelm chantal-kelm self-requested a review December 16, 2022 15:58
Copy link
Member

@yenienserrano yenienserrano left a comment

Choose a reason for hiding this comment

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

LGTM

Pass

image

Fail

image

Not applicable

image

@github-actions
Copy link
Contributor

Code coverage (Jest) % values
Statements 8.68% ( 3210 / 36962 )
Branches 4.43% ( 1271 / 28689 )
Functions 7.59% ( 696 / 9167 )
Lines 8.75% ( 3097 / 35382 )

Copy link
Member

@chantal-kelm chantal-kelm left a comment

Choose a reason for hiding this comment

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

CR: ✅
Test: ✅

Copy link
Member

@Desvelao Desvelao left a comment

Choose a reason for hiding this comment

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

review

✔️ code
✔️ test

Tests

Legend:
🟢 Pass
🟡 Warning
🔴 Fail

Results:

Scenario Result
1 🟢
2 🟢
3 🟢

Details:

Scenario 1 - 🟢

image

Scenario 2 - 🟢

image

Scenario 3 - 🟢

image

@Desvelao Desvelao merged commit 6e8c942 into 4.4-7.10 Dec 19, 2022
@Desvelao Desvelao deleted the bugfix/4952-checks-table-stats-filter branch December 19, 2022 08:01
@github-actions
Copy link
Contributor

The backport to 4.4-7.16 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-4.4-7.16 4.4-7.16
# Navigate to the new working tree
cd .worktrees/backport-4.4-7.16
# Create a new branch
git switch --create backport-4999-to-4.4-7.16
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 6e8c94263c5faeb0d02b2b38786d4026b20fb137
# Push it to GitHub
git push --set-upstream origin backport-4999-to-4.4-7.16
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-4.4-7.16

Then, create a pull request where the base branch is 4.4-7.16 and the compare/head branch is backport-4999-to-4.4-7.16.

@github-actions
Copy link
Contributor

The backport to 4.4-2.4-wzd failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-4.4-2.4-wzd 4.4-2.4-wzd
# Navigate to the new working tree
cd .worktrees/backport-4.4-2.4-wzd
# Create a new branch
git switch --create backport-4999-to-4.4-2.4-wzd
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 6e8c94263c5faeb0d02b2b38786d4026b20fb137
# Push it to GitHub
git push --set-upstream origin backport-4999-to-4.4-2.4-wzd
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-4.4-2.4-wzd

Then, create a pull request where the base branch is 4.4-2.4-wzd and the compare/head branch is backport-4999-to-4.4-2.4-wzd.

Desvelao pushed a commit that referenced this pull request Dec 19, 2022
* Fixed checks table filters by stats

* Updated CHANGELOG

* Fixed no applicable filter

(cherry picked from commit 6e8c942)
Desvelao pushed a commit that referenced this pull request Dec 19, 2022
* Fixed checks table filters by stats

* Updated CHANGELOG

* Fixed no applicable filter

(cherry picked from commit 6e8c942)
Desvelao pushed a commit that referenced this pull request Dec 19, 2022
* Fixed checks table filters by stats

* Updated CHANGELOG

* Fixed no applicable filter

(cherry picked from commit 6e8c942)
@Desvelao Desvelao added the type/bug Bug issue label Dec 19, 2022
Desvelao added a commit that referenced this pull request Dec 20, 2022
) (#5004)

Fixed Inventory checks table filters by stats (#4999)

* Fixed checks table filters by stats

* Updated CHANGELOG

* Fixed no applicable filter

(cherry picked from commit 6e8c942)

Co-authored-by: Maximiliano Ibarra <[email protected]>
asteriscos pushed a commit that referenced this pull request Dec 20, 2022
…4999) (#5005)

Fixed Inventory checks table filters by stats (#4999)

* Fixed checks table filters by stats

* Updated CHANGELOG

* Fixed no applicable filter

(cherry picked from commit 6e8c942)

Co-authored-by: Maximiliano Ibarra <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Bug issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SCA result filters do not work
4 participants