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(ESSNTL-4194): Minor improvements #1879

Merged
merged 16 commits into from
Jun 21, 2023

Conversation

gkarat
Copy link
Contributor

@gkarat gkarat commented May 29, 2023

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

This PR follows up on #1767 and fixes the following bugs:

  1. Sorting by Last modified lead to an error because the sorting is not implemented on the back end => the column is now not sortable,
  2. Selecting and removing groups from a list was leaving the selected count the same => now the selected number is reset once the selected groups are removed,
  3. Success message when removing more groups was containing undefined => fixed this too according to mocks,
  4. When user is creating the very first group the empty state is not updated => now when the first group is created the empty message disappears and start to render the table.

How to test

  1. Go to /groups and make sure all of the bugs are resolved.

@gkarat gkarat added the bug Something isn't working label May 29, 2023
@gkarat gkarat self-assigned this May 29, 2023
@gkarat gkarat requested a review from a team as a code owner May 29, 2023 13:47
@codecov-commenter
Copy link

codecov-commenter commented May 31, 2023

Codecov Report

Patch coverage: 69.23% and project coverage change: -0.03 ⚠️

Comparison is base (6777bd3) 65.28% compared to head (d46bb9e) 65.25%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1879      +/-   ##
==========================================
- Coverage   65.28%   65.25%   -0.03%     
==========================================
  Files         145      145              
  Lines        3964     3978      +14     
  Branches     1125     1129       +4     
==========================================
+ Hits         2588     2596       +8     
- Misses       1376     1382       +6     
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 83.33% <57.14%> (-0.96%) ⬇️
src/components/InventoryGroups/InventoryGroups.js 100.00% <100.00%> (ø)
...ponents/InventoryGroups/Modals/DeleteGroupModal.js 100.00% <100.00%> (ø)
src/components/InventoryGroups/Modals/Modal.js 100.00% <100.00%> (ø)
...c/components/InventoryGroups/NoGroupsEmptyState.js 80.00% <100.00%> (+5.00%) ⬆️

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

@gkarat gkarat requested a review from a team May 31, 2023 14:04
@gkarat
Copy link
Contributor Author

gkarat commented Jun 2, 2023

There is now only one test failing which is not related to the PR changes (InventoryGroups). I will research on the cause, but other tests look OK.

@gkarat gkarat force-pushed the essntl-4194-fixes branch from 09dea95 to 6e2bbaf Compare June 20, 2023 10:41
@fpjrh
Copy link

fpjrh commented Jun 20, 2023

@RedHatInsights/team-interact can we get 1 approving review? Thank you!

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.

LGTM

@gkarat gkarat merged commit ddf029b into RedHatInsights:master Jun 21, 2023
@gkarat gkarat deleted the essntl-4194-fixes branch June 21, 2023 09:01
@gkarat
Copy link
Contributor Author

gkarat commented Jun 21, 2023

🎉 This PR is included in version 1.22.4 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants