feat(ESSNTL-4194): Show groups in a table #1767

gkarat · 2023-02-14T16:03:35Z

Implements https://issues.redhat.com/browse/ESSNTL-4194.
Mock https://www.sketch.com/s/c19f555b-7887-4423-b55d-575c8dd1dfb7/a/dl8zwyb.

Create the groups list table with three columns: name, total systems and last modified. Implement sorting by each column, filtering by name and pagination.

⚠️ This doesn't implement actions (rename, delete, create group, bulk select) yet (will be in a separate PR).

How to test

Deploy locally with mock server (npm run start:mock).
Navigate to /inventory/groups and check that the table is rendered with dynamically generated values.
You can also disable the mock server and check whether network errors are properly handled with the error state.
You can also tweak your mock server and generate empty response (to verify the empty state).
The table is API paginated and filtered. Any filter change, pagination or sorting you make must trigger a new API request.

Screenshots

No match state:

Failed request (any runtime error):

Pagination values:

Implements https://issues.redhat.com/browse/ESSNTL-4194.

src/components/GroupsTable/GroupsTable.cy.js

cypress/support/commands.js

mmenestr · 2023-02-14T17:07:32Z

Looks good, just a few comments:

Can we reposition the columns so that the "total systems" column is more centered in the table?
Can we update the empty state to look like this (just add icon and button)

Search should use the "Search input" component where the icon is on the left
For the error empty state, wonder if that action button is really useful (?) is that what we usually use in other places? If so then leave it, but just wondering!

Fewwy · 2023-02-15T12:21:50Z

When I tried to sort the "Total systems" or "Last modified" columns I got an error

Fewwy · 2023-02-15T12:24:10Z

First change of page worked fine but after that I encountered this error

gkarat · 2023-02-15T13:14:51Z

@Fewwy,

regarding sorting: this is expected, the table sends the order_by with the parameter which is not currently listed in the API schema and thus the mock server will throw you error ('name', 'updated_at', host_ids are not allowed currently for order_by parameter).
the same for pagination, the mock server generates crazy values for count and total fields, thus the requests will be broken too with very huge (forbidden) values.

All these problems will not be reproducible with the real API implementation soon.

Fewwy · 2023-02-15T13:34:03Z

@gkarat Other than that the PR look fine :)

gkarat · 2023-02-15T14:52:41Z

@mmenestr, thanks a lot for your review! I addressed your points in the last commits. For the button in the error state: this would require some work in our library (separate from the inventory frontend): I left the TODO note to fix it later.

gkarat · 2023-02-15T14:56:36Z

/retest

…s-table

src/components/GroupsTable/GroupsTable.js

mkholjuraev · 2023-02-16T10:08:39Z

src/components/GroupsTable/GroupsTable.js

+import { generateLoadingRows } from '../InventoryTable/helpers';
+import NoEntitiesFound from '../InventoryTable/NoEntitiesFound';
+
+const GROUPS_TABLE_INITIAL_STATE = {


How about extracting assets into separate file?

i'd like to keep it in the same file since these constants are tied to the component's state only

Hmm, maybe there will be a new component that can re-use them? But, not a big deal, this is minor :)

src/components/GroupsTable/GroupsTable.js

codecov-commenter · 2023-02-16T10:21:10Z

Codecov Report

Base: 73.36% // Head: 69.54% // Decreases project coverage by -3.83% ⚠️

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

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1767      +/-   ##
==========================================
- Coverage   73.36%   69.54%   -3.83%     
==========================================
  Files         105      108       +3     
  Lines        2219     2420     +201     
  Branches      838      869      +31     
==========================================
+ Hits         1628     1683      +55     
- Misses        591      737     +146

Impacted Files	Coverage Δ
src/components/GroupsTable/GroupsTable.cy.js	`0.00% <0.00%> (ø)`
...c/components/InventoryGroups/InventoryGroups.cy.js	`0.00% <0.00%> (ø)`
src/components/GroupsTable/GroupsTable.js	`94.54% <94.54%> (ø)`
src/components/InventoryGroups/InventoryGroups.js	`100.00% <100.00%> (ø)`
src/components/InventoryGroups/utils/api.js	`88.88% <100.00%> (+3.17%)`	⬆️
src/components/InventoryTable/EntityTable.js	`97.67% <100.00%> (ø)`
src/components/InventoryTable/NoEntitiesFound.js	`100.00% <100.00%> (ø)`
src/components/InventoryTable/helpers.js	`97.22% <100.00%> (+0.16%)`	⬆️
src/constants.js	`82.53% <100.00%> (+0.28%)`	⬆️
src/store/groups.js	`100.00% <100.00%> (+85.71%)`	⬆️
... and 4 more

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.

mkholjuraev

Codewise looks good!

mkholjuraev · 2023-02-16T11:13:31Z

src/components/GroupsTable/GroupsTable.js

+import { generateLoadingRows } from '../InventoryTable/helpers';
+import NoEntitiesFound from '../InventoryTable/NoEntitiesFound';
+
+const GROUPS_TABLE_INITIAL_STATE = {


Hmm, maybe there will be a new component that can re-use them? But, not a big deal, this is minor :)

# [1.3.0](v1.2.0...v1.3.0) (2023-02-16) ### Features * **ESSNTL-4194:** Show groups in a table ([#1767](#1767)) ([b80c00e](b80c00e))

gkarat · 2023-02-16T11:29:04Z

🎉 This PR is included in version 1.3.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

gkarat added 3 commits February 14, 2023 17:00

Distinguish the uninitialized state

8f85f8c

Support search and pagination in getGroups

46580af

Add the groups list table

40b8455

gkarat requested a review from a team as a code owner February 14, 2023 16:03

gkarat marked this pull request as draft February 14, 2023 16:04

gkarat added 2 commits February 14, 2023 17:30

Extract interceptors, add tests for the grous page

8073be1

Add groups table (reference ESSNTL-4194 commit)

ec5865f

Implements https://issues.redhat.com/browse/ESSNTL-4194.

gkarat marked this pull request as ready for review February 14, 2023 16:34

gkarat requested a review from Fewwy February 14, 2023 16:47

gkarat self-assigned this Feb 14, 2023

gkarat added the enhancement New feature or request label Feb 14, 2023

Fix lint error

0bd89b5

computercamplove reviewed Feb 14, 2023

View reviewed changes

src/components/GroupsTable/GroupsTable.cy.js Show resolved Hide resolved

Fewwy reviewed Feb 14, 2023

View reviewed changes

cypress/support/commands.js Outdated Show resolved Hide resolved

gkarat added 4 commits February 15, 2023 12:07

Remove ouiaId unnecessary definition

7e39c47

Make the no match state render icon and optional link

3a8aadd

Fix actions.test.js, update snapshots

78bb7e4

Center the total systems column

7649a3a

gkarat added 2 commits February 15, 2023 15:50

Use SearchInput PF component

60cb8cd

Leave TODO note

90e5d8d

Merge remote-tracking branch 'upstream/master' into essntl-4194-group…

cc91451

…s-table

gkarat force-pushed the essntl-4194-groups-table branch from d617d3d to cc91451 Compare February 16, 2023 09:58

Merge branch 'master' into essntl-4194-groups-table

b4d095e

mkholjuraev reviewed Feb 16, 2023

View reviewed changes

gkarat added 2 commits February 16, 2023 11:25

Wrap filter config items with useMemo

6508e52

Extract table rows to memoized function

f4ac110

Fewwy approved these changes Feb 16, 2023

View reviewed changes

mkholjuraev reviewed Feb 16, 2023

View reviewed changes

gkarat merged commit b80c00e into RedHatInsights:master Feb 16, 2023

gkarat pushed a commit that referenced this pull request Feb 16, 2023

chore(release): 1.3.0 [skip ci]

c716c74

# [1.3.0](v1.2.0...v1.3.0) (2023-02-16) ### Features * **ESSNTL-4194:** Show groups in a table ([#1767](#1767)) ([b80c00e](b80c00e))

gkarat added the released label Feb 16, 2023

gkarat mentioned this pull request May 29, 2023

fix(ESSNTL-4194): Minor improvements #1879

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ESSNTL-4194): Show groups in a table #1767

feat(ESSNTL-4194): Show groups in a table #1767

gkarat commented Feb 14, 2023 •

edited

Loading

mmenestr commented Feb 14, 2023 •

edited

Loading

Fewwy commented Feb 15, 2023

Fewwy commented Feb 15, 2023 •

edited

Loading

gkarat commented Feb 15, 2023

Fewwy commented Feb 15, 2023

gkarat commented Feb 15, 2023

gkarat commented Feb 15, 2023

mkholjuraev Feb 16, 2023

gkarat Feb 16, 2023

mkholjuraev Feb 16, 2023

codecov-commenter commented Feb 16, 2023

mkholjuraev left a comment

mkholjuraev Feb 16, 2023

gkarat commented Feb 16, 2023

feat(ESSNTL-4194): Show groups in a table #1767

feat(ESSNTL-4194): Show groups in a table #1767

Conversation

gkarat commented Feb 14, 2023 • edited Loading

How to test

Screenshots

mmenestr commented Feb 14, 2023 • edited Loading

Fewwy commented Feb 15, 2023

Fewwy commented Feb 15, 2023 • edited Loading

gkarat commented Feb 15, 2023

Fewwy commented Feb 15, 2023

gkarat commented Feb 15, 2023

gkarat commented Feb 15, 2023

mkholjuraev Feb 16, 2023

Choose a reason for hiding this comment

gkarat Feb 16, 2023

Choose a reason for hiding this comment

mkholjuraev Feb 16, 2023

Choose a reason for hiding this comment

codecov-commenter commented Feb 16, 2023

Codecov Report

mkholjuraev left a comment

Choose a reason for hiding this comment

mkholjuraev Feb 16, 2023

Choose a reason for hiding this comment

gkarat commented Feb 16, 2023

gkarat commented Feb 14, 2023 •

edited

Loading

mmenestr commented Feb 14, 2023 •

edited

Loading

Fewwy commented Feb 15, 2023 •

edited

Loading