[OCPADVISOR-6] Reset filters resets only filters #450

johnsonm325 · 2022-11-10T22:34:10Z

When you click on "Reset filters" values set for pagination and sorting get also reset. This is on every table in the app.

This PR ensures that on "Reset filters" click, only filters get reset and not pagination or sorting.

gkarat

I am afraid this feature is not relevant for all of the tables. Specifically, there is a scenario for the recs list table (/recommendations):

By default, you have two filters enabled: Clusters impacted: 1 or more, and Status: Enabled.
Remove the first filter, and you will get more recommendations available.
Navigate to for example page 3 and click Reset filters
You will get a blank table. This happens because the reset will show you the subset of rules that you saw with the removed default filter, and you don't reset pagination.

Can we make an exception for this table? All other tables seems to be working fine, since resetting to the default filters will always result in showing the same or larger number of items.

src/Services/Filters.js

src/Components/AffectedClustersTable/AffectedClustersTable.cy.js

src/Components/ClusterRules/ClusterRules.cy.js

johnsonm325 · 2022-11-17T22:57:55Z

@gkarat this is ready for another review

gkarat

LGTM! Just one minor comment regarding async + we need to fix the failing test: it looks like if you increase the number of items matching the filter "Name: o", then it will help. Otherwise, the next button is disabled.

gkarat · 2022-11-18T12:59:59Z

src/Components/RecsListTable/RecsListTable.js

@@ -110,8 +111,15 @@ const RecsListTable = ({ query }) => {
    filters.sortDirection,
  ]);

-  useEffect(() => {
-    setFilteredRows(buildFilteredRows(recs, filters));
+  useEffect(async () => {


Why we need to make this useEffect async here?

I added it and forgot to test it again later. It's not needed, so I removed it.

When you click on "Reset filters" values set for pagination and sorting get also reset. This is on every table in the app. This PR ensures that on "Reset filters" click, only filters get reset and not pagination or sorting.

codecov-commenter · 2022-11-18T18:14:52Z

Codecov Report

Base: 87.08% // Head: 86.91% // Decreases project coverage by -0.16% ⚠️

Coverage data is based on head (b9ea40e) compared to base (c0c1af8).
Patch coverage: 92.30% of modified lines in pull request are covered.

❗ Current head b9ea40e differs from pull request most recent head 71b7fa3. Consider uploading reports for the commit 71b7fa3 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #450      +/-   ##
==========================================
- Coverage   87.08%   86.91%   -0.17%     
==========================================
  Files          25       25              
  Lines        1154     1162       +8     
  Branches      442      445       +3     
==========================================
+ Hits         1005     1010       +5     
- Misses        149      152       +3

Impacted Files	Coverage Δ
src/Components/RecsListTable/RecsListTable.js	`80.09% <80.00%> (-0.19%)`	⬇️
...nts/AffectedClustersTable/AffectedClustersTable.js	`95.68% <100.00%> (ø)`
src/Components/ClusterRules/ClusterRules.js	`86.22% <100.00%> (ø)`
.../Components/ClustersListTable/ClustersListTable.js	`88.23% <100.00%> (ø)`
src/Services/Filters.js	`100.00% <100.00%> (ø)`
src/Components/Modals/DisableRule.js	`90.74% <0.00%> (-3.71%)`	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

gkarat · 2022-11-21T15:20:09Z

thanks Michael! looks good to me. and we can ignore the ci.int check: we need to update the test script in a separate PR

## [1.8.3](v1.8.2...v1.8.3) (2022-11-21) ### Bug Fixes * **Tables:** Reset filters resets only filters ([#450](#450)) ([593664d](593664d))

gkarat · 2022-11-21T15:33:21Z

🎉 This PR is included in version 1.8.3 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Fixes https://issues.redhat.com/browse/OCPADVISOR-6. When you click on "Reset filters" values set for pagination and sorting get also reset. This is on every table in the app. This PR ensures that on "Reset filters" click, only filters get reset and not pagination or sorting.

## [1.8.3](v1.8.2...v1.8.3) (2022-11-21) ### Bug Fixes * **Tables:** Reset filters resets only filters ([#450](#450)) ([593664d](593664d))

Fixes https://issues.redhat.com/browse/OCPADVISOR-6. When you click on "Reset filters" values set for pagination and sorting get also reset. This is on every table in the app. This PR ensures that on "Reset filters" click, only filters get reset and not pagination or sorting.

## [1.8.3](v1.8.2...v1.8.3) (2022-11-21) ### Bug Fixes * **Tables:** Reset filters resets only filters ([#450](#450)) ([593664d](593664d))

johnsonm325 requested a review from a team November 10, 2022 22:34

gkarat reviewed Nov 11, 2022

View reviewed changes

gkarat assigned johnsonm325 Nov 11, 2022

johnsonm325 force-pushed the ocpadvisor-6 branch 2 times, most recently from 1046011 to 66656e2 Compare November 17, 2022 22:57

gkarat requested a review from a team November 18, 2022 09:57

gkarat reviewed Nov 18, 2022

View reviewed changes

johnsonm325 force-pushed the ocpadvisor-6 branch 2 times, most recently from 7c5cecf to 8d5e5a1 Compare November 18, 2022 17:43

[OCPADVISOR-6] Reset filters resets only filters

71b7fa3

When you click on "Reset filters" values set for pagination and sorting get also reset. This is on every table in the app. This PR ensures that on "Reset filters" click, only filters get reset and not pagination or sorting.

johnsonm325 force-pushed the ocpadvisor-6 branch from 8d5e5a1 to 71b7fa3 Compare November 18, 2022 17:59

gkarat merged commit 593664d into RedHatInsights:master Nov 21, 2022

gkarat pushed a commit that referenced this pull request Nov 21, 2022

chore(release): 1.8.3 [skip ci]

51dae8a

## [1.8.3](v1.8.2...v1.8.3) (2022-11-21) ### Bug Fixes * **Tables:** Reset filters resets only filters ([#450](#450)) ([593664d](593664d))

gkarat added the released label Nov 21, 2022

gkarat pushed a commit that referenced this pull request Nov 28, 2022

chore(release): 1.8.3 [skip ci]

396c62d

## [1.8.3](v1.8.2...v1.8.3) (2022-11-21) ### Bug Fixes * **Tables:** Reset filters resets only filters ([#450](#450)) ([593664d](593664d))

gkarat pushed a commit that referenced this pull request Nov 28, 2022

chore(release): 1.8.3 [skip ci]

17f974f

## [1.8.3](v1.8.2...v1.8.3) (2022-11-21) ### Bug Fixes * **Tables:** Reset filters resets only filters ([#450](#450)) ([593664d](593664d))

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[OCPADVISOR-6] Reset filters resets only filters #450

[OCPADVISOR-6] Reset filters resets only filters #450

johnsonm325 commented Nov 10, 2022

gkarat left a comment

johnsonm325 commented Nov 17, 2022

gkarat left a comment

gkarat Nov 18, 2022

johnsonm325 Nov 18, 2022

codecov-commenter commented Nov 18, 2022

gkarat commented Nov 21, 2022

gkarat commented Nov 21, 2022

[OCPADVISOR-6] Reset filters resets only filters #450

[OCPADVISOR-6] Reset filters resets only filters #450

Conversation

johnsonm325 commented Nov 10, 2022

gkarat left a comment

Choose a reason for hiding this comment

johnsonm325 commented Nov 17, 2022

gkarat left a comment

Choose a reason for hiding this comment

gkarat Nov 18, 2022

Choose a reason for hiding this comment

johnsonm325 Nov 18, 2022

Choose a reason for hiding this comment

codecov-commenter commented Nov 18, 2022

Codecov Report

gkarat commented Nov 21, 2022

gkarat commented Nov 21, 2022