-
Notifications
You must be signed in to change notification settings - Fork 176
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
filter high level groups and action groups by cluster and index #1482
filter high level groups and action groups by cluster and index #1482
Conversation
Signed-off-by: Derek Ho <dxho@amazon.com>
Signed-off-by: Derek Ho <dxho@amazon.com>
Signed-off-by: Derek Ho <dxho@amazon.com>
Signed-off-by: Derek Ho <dxho@amazon.com>
Thank you for the PR @derek-ho ! Would you be able to add a test to verify the new behavior? |
Codecov Report
@@ Coverage Diff @@
## main #1482 +/- ##
==========================================
- Coverage 65.62% 65.51% -0.12%
==========================================
Files 93 93
Lines 2307 2314 +7
Branches 314 315 +1
==========================================
+ Hits 1514 1516 +2
- Misses 725 730 +5
Partials 68 68
|
@derek-ho Would you please fetch the latest changes from main. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @derek-ho, thanks for taking this on, and it is looking good to me. Just like Craig mentioned above, a testing case would be great.
@@ -106,12 +106,12 @@ export function RoleEdit(props: RoleEditDeps) { | |||
} | |||
}, [addToast, props.action, props.coreStart.http, props.sourceRoleName]); | |||
|
|||
const [actionGroups, setActionGroups] = useState<string[]>([]); | |||
const [actionGroups, setActionGroups] = useState<Array<[string, ActionGroupItem]>>([]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for my understanding, why are we needing this ActionGroupItem
, or can we use the original Action
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If i'm understanding your question correctly I need the whole item here in order to tell whether it is a cluster/index one later on in order to filter them accordingly
5b059d5
to
a8f27ce
Compare
Signed-off-by: Derek Ho <dxho@amazon.com>
public/apps/configuration/panels/role-edit/test/role-edit-filtering.test.tsx
Outdated
Show resolved
Hide resolved
public/apps/configuration/panels/role-edit/test/role-edit-filtering.test.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: Derek Ho <dxho@amazon.com>
60ed3c7
to
f66725a
Compare
}, | ||
{ | ||
label: 'Cluster permissions', | ||
options: CLUSTER_PERMISSIONS.map((x) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much more robust and less lines of code :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Thank you for solving this pesky issue @derek-ho !
This should fix both. It is both filtering action groups (which comes from the network call), and also creates a separate variable, so that only cluster/index respectively permissions from the constants file is included |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* filter high level groups and action groups by cluster and index Signed-off-by: Derek Ho <dxho@amazon.com> * remove unecessary console Signed-off-by: Derek Ho <dxho@amazon.com> * add semicolon back Signed-off-by: Derek Ho <dxho@amazon.com> * use map instead of flat map Signed-off-by: Derek Ho <dxho@amazon.com> * fix lint Signed-off-by: Derek Ho <dxho@amazon.com> * fix tests Signed-off-by: Derek Ho <dxho@amazon.com> * revert file Signed-off-by: Derek Ho <dxho@amazon.com> * fix up tests Signed-off-by: Derek Ho <dxho@amazon.com> * lint Signed-off-by: Derek Ho <dxho@amazon.com> --------- Signed-off-by: Derek Ho <dxho@amazon.com> (cherry picked from commit 5fd8018)
… (#1508) * filter high level groups and action groups by cluster and index Signed-off-by: Derek Ho <dxho@amazon.com> * remove unecessary console Signed-off-by: Derek Ho <dxho@amazon.com> * add semicolon back Signed-off-by: Derek Ho <dxho@amazon.com> * use map instead of flat map Signed-off-by: Derek Ho <dxho@amazon.com> * fix lint Signed-off-by: Derek Ho <dxho@amazon.com> * fix tests Signed-off-by: Derek Ho <dxho@amazon.com> * revert file Signed-off-by: Derek Ho <dxho@amazon.com> * fix up tests Signed-off-by: Derek Ho <dxho@amazon.com> * lint Signed-off-by: Derek Ho <dxho@amazon.com> --------- Signed-off-by: Derek Ho <dxho@amazon.com> (cherry picked from commit 5fd8018) Co-authored-by: Derek Ho <dxho@amazon.com>
…search-project#1482) * filter high level groups and action groups by cluster and index Signed-off-by: Derek Ho <dxho@amazon.com> * remove unecessary console Signed-off-by: Derek Ho <dxho@amazon.com> * add semicolon back Signed-off-by: Derek Ho <dxho@amazon.com> * use map instead of flat map Signed-off-by: Derek Ho <dxho@amazon.com> * fix lint Signed-off-by: Derek Ho <dxho@amazon.com> * fix tests Signed-off-by: Derek Ho <dxho@amazon.com> * revert file Signed-off-by: Derek Ho <dxho@amazon.com> * fix up tests Signed-off-by: Derek Ho <dxho@amazon.com> * lint Signed-off-by: Derek Ho <dxho@amazon.com> --------- Signed-off-by: Derek Ho <dxho@amazon.com> Signed-off-by: Sam <samuel.costa@eliatra.com>
Description
Filters high level groups based on cluster or index panel and also filters the actiongroups based on their associated type
Category
[Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation]
Why these changes are required?
What is the old behavior before changes and new behavior after changes?
Issues Resolved
fix: #1346
fix: #1347
Testing
[Please provide details of testing done: unit testing, integration testing and manual testing]
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.