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

feat(ESSNTL-4365): Select rows in the groups table #1775

Conversation

gkarat
Copy link
Contributor

@gkarat gkarat commented Feb 28, 2023

Implements https://issues.redhat.com/browse/ESSNTL-4365 (1/2, the actions will be implemented in a separate PR).

This makes possible to select rows in the groups table separately or in bulk. Component tests are implemented to verify the correct request URLs. The selection drop down shows three options according to the UX tables audit document (Stefan Kukla).

How to test

  1. Run npm run start:mock and npm run mock-server in separate terminals.
  2. Navigate to https://stage.foo.redhat.com:1337/insights/inventory/groups.
  3. Mock server sends not consistent responses, this is why the selection will be working with bugs, but you can still play with it on the page.
  4. Run npm run test:openct and find the new tests. You can play with the table there too.

Screenshots

image

@gkarat gkarat added the enhancement New feature or request label Feb 28, 2023
@gkarat gkarat requested a review from Fewwy February 28, 2023 10:58
@gkarat gkarat requested a review from a team as a code owner February 28, 2023 10:58
@gkarat gkarat self-assigned this Feb 28, 2023
Copy link
Contributor

@mkholjuraev mkholjuraev left a comment

Choose a reason for hiding this comment

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

Overall, looks good as always. Just a minor suggestions that I though deserving

src/Utilities/hooks/usePromiseQueue.js Outdated Show resolved Hide resolved
@@ -208,6 +246,47 @@ const GroupsTable = () => {
}}
filterConfig={{ items: filterConfigItems }}
activeFiltersConfig={activeFiltersConfig}
bulkSelect={{
items: [
Copy link
Contributor

Choose a reason for hiding this comment

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

A minor: I think if we move this bulk select config object outside of the JSX rendering section, we would have even cleaner code. We can also separate this into a callback as the function definition does not change often.

@Fewwy
Copy link
Contributor

Fewwy commented Mar 1, 2023

select none works correctly
select/deselect page works correctly
select all didn't work for me(possibly because of the crazy number)
image

Copy link
Contributor

@Fewwy Fewwy left a comment

Choose a reason for hiding this comment

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

Good job! looks great!

@codecov-commenter
Copy link

Codecov Report

Patch coverage: 47.57% and project coverage change: -1.76 ⚠️

Comparison is base (ca24e55) 70.45% compared to head (d36987d) 68.69%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1775      +/-   ##
==========================================
- Coverage   70.45%   68.69%   -1.76%     
==========================================
  Files         122      124       +2     
  Lines        2579     2677      +98     
  Branches      885      900      +15     
==========================================
+ Hits         1817     1839      +22     
- Misses        762      838      +76     
Impacted Files Coverage Δ
src/components/GroupsTable/GroupsTable.cy.js 0.00% <0.00%> (ø)
src/components/SystemDetails/Advisor.js 0.00% <0.00%> (ø)
src/components/GroupsTable/GroupsTable.js 89.47% <81.57%> (-5.27%) ⬇️
src/Utilities/hooks/useFetchBatched.js 100.00% <100.00%> (ø)
src/Utilities/hooks/usePromiseQueue.js 100.00% <100.00%> (ø)
src/api/api.js 45.78% <0.00%> (-18.08%) ⬇️
src/routes/InventoryTable.js 60.34% <0.00%> (-7.76%) ⬇️
src/components/InventoryTable/InventoryTable.js 94.73% <0.00%> (-5.27%) ⬇️

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 gkarat merged commit 2d81ac7 into RedHatInsights:master Mar 1, 2023
gkarat pushed a commit that referenced this pull request Mar 1, 2023
# [1.5.0](v1.4.0...v1.5.0) (2023-03-01)

### Bug Fixes

* **ESSNTL-4387:** Pass down inventoryId to Advisor ([#1773](#1773)) ([72e017e](72e017e))

### Features

* **ESSNTL-4365:** Select rows in the groups table ([#1775](#1775)) ([2d81ac7](2d81ac7))
@gkarat
Copy link
Contributor Author

gkarat commented Mar 1, 2023

🎉 This PR is included in version 1.5.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@gkarat gkarat added the released label Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants