Allow role mappings to be cloned by XavierM · Pull Request #118434 · elastic/kibana

XavierM · 2021-11-11T21:54:04Z

Summary

Adding functionality to clone a role mapping. I realized that there was some discrepancy between roles and roles mapping. I took the initiative to make it similar. It also matching what EUI is doing in their example

Resolves: #111744

Checklist

Unit or functional tests were updated or added to match the most common scenarios

elasticmachine · 2021-11-11T21:54:06Z

Pinging @elastic/kibana-security (Team:Security)

XavierM · 2021-11-15T21:23:21Z

x-pack/plugins/security/public/management/table_utils.tsx

+interface ActionsEuiTableFormattingProps {
+  children: ReactNode;
+}
+export const ActionsEuiTableFormatting = React.memo<ActionsEuiTableFormattingProps>(


Notes for reviewer: Why did Xavier decide to override EUI? is he crazy?
Answer: maybe he is, but the real reason is that I/we wanted to keep our icon working with onClick and href at the same time but the actions props in EuiTable does not let you do that and I still wanted the same style how EUI do it and that's my reason.
if you know/have a better way please let me know, I will love/do it.

There isn't a better way (that I'm aware of) to achieve this without changing some things in EUI 👍

XavierM · 2021-11-16T14:07:20Z

@elasticmachine merge upstream

thompsongl

Both comments should be taken as one. Not a blocker, just a suggestion if you don't need to maintain the styles in a variable as template string.

x-pack/plugins/security/public/management/table_utils.tsx

…lone_role_mapping

XavierM · 2021-11-16T17:59:17Z

@elasticmachine merge upstream

jportner

Nice job, just a couple of nits below

jportner · 2021-11-16T19:28:10Z

x-pack/test/functional/apps/security/role_mappings.ts


    it('allows a role mapping to be deleted', async () => {
-      await testSubjects.click(`deleteRoleMappingButton-new_role_mapping`);
+      await testSubjects.click('euiCollapsedItemActionsButton');


Maybe to future-proof the other tests against any EUI changes, we should change the other two action tests (clone, edit) to also use the collapsed item actions button. WDYT?

I do not think it is really needed because it is the same actions button on the row or in the popover menu item. In my opinion, it should already being handle by the EUI library ;) (We need to trust our tools)

x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.tsx

jportner · 2021-11-16T19:51:11Z

x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.tsx

  private onCancelDelete = () => {
-    this.setState({ showDeleteConfirmation: false });
+    this.setState({ showDeleteConfirmation: false, selection: [] });
+    this.tableRef.current?.setSelection([]);
  };


…lone_role_mapping

jportner

LGTM, one last nit below

x-pack/plugins/security/public/management/table_utils.tsx

kibana-ci · 2021-11-17T18:54:52Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`security`	400	401	+1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`security`	487.4KB	490.5KB	+3.1KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`security`	49.4KB	49.5KB	+123.0B

History

💚 Build #7132 succeeded 45447eb
💔 Build #7023 failed 335a779
💔 Build #7022 failed a2dfd21
💔 Build #6834 failed 290322f
💔 Build #6816 failed 290322f
💔 Build #6707 failed cb70574

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

* add clobe to role mapping and update functionalit in role to match UX * fix some I * fix jest test + fix table selection when canceling deletion * add tests around clone action in role mapping * fix i18n * remove i18n * review Greg I * fix styling + name * add explaination Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

kibanamachine · 2021-11-17T18:58:48Z

💚 Backport successful

Status	Branch	Result
✅	8.0

This backport PR will be merged automatically after passing CI.

* add clobe to role mapping and update functionalit in role to match UX * fix some I * fix jest test + fix table selection when canceling deletion * add tests around clone action in role mapping * fix i18n * remove i18n * review Greg I * fix styling + name * add explaination Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Xavier Mouligneau <xavier.mouligneau@elastic.co>

* add clobe to role mapping and update functionalit in role to match UX * fix some I * fix jest test + fix table selection when canceling deletion * add tests around clone action in role mapping * fix i18n * remove i18n * review Greg I * fix styling + name * add explaination Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

add clobe to role mapping and update functionalit in role to match UX

d733e58

XavierM added release_note:enhancement Team:Security Platform Security: Auth, Users, Roles, Spaces, Audit Logging, etc t// enhancement New value added to drive a business result v8.0.0 auto-backport Deprecated - use backport:version if exact versions are needed v8.1.0 labels Nov 11, 2021

XavierM requested a review from a team as a code owner November 11, 2021 21:54

XavierM marked this pull request as draft November 11, 2021 21:55

XavierM added 2 commits November 15, 2021 10:34

fix some I

916bc2a

Merge branch 'main' of github.com:elastic/kibana into clone_role_mapping

cb70574

elastic deleted a comment from kibanamachine Nov 15, 2021

XavierM added 4 commits November 15, 2021 14:54

fix jest test + fix table selection when canceling deletion

290322f

add tests around clone action in role mapping

5305afb

Merge branch 'main' of github.com:elastic/kibana into clone_role_mapping

a2dfd21

fix i18n

e27065f

XavierM marked this pull request as ready for review November 15, 2021 21:12

XavierM requested a review from thompsongl November 15, 2021 21:17

XavierM commented Nov 15, 2021

View reviewed changes

Merge branch 'main' into clone_role_mapping

335a779

jportner self-requested a review November 16, 2021 15:36

thompsongl reviewed Nov 16, 2021

View reviewed changes

x-pack/plugins/security/public/management/table_utils.tsx Outdated Show resolved Hide resolved

x-pack/plugins/security/public/management/table_utils.tsx Show resolved Hide resolved

XavierM added 4 commits November 16, 2021 12:14

remove i18n

3ac3456

Merge branch 'clone_role_mapping' of github.com:XavierM/kibana into c…

76969bc

…lone_role_mapping

Merge branch 'main' of github.com:elastic/kibana into clone_role_mapping

e826771

review Greg I

f7f4dc9

Merge branch 'main' into clone_role_mapping

45447eb

jportner reviewed Nov 16, 2021

View reviewed changes

XavierM added 3 commits November 17, 2021 11:43

fix styling + name

ed2877c

Merge branch 'main' of github.com:elastic/kibana into clone_role_mapping

c1e8dbd

Merge branch 'clone_role_mapping' of github.com:XavierM/kibana into c…

0429e88

…lone_role_mapping

XavierM requested a review from jportner November 17, 2021 16:45

jportner approved these changes Nov 17, 2021

View reviewed changes

x-pack/plugins/security/public/management/table_utils.tsx Show resolved Hide resolved

add explaination

820c1d7

XavierM enabled auto-merge (squash) November 17, 2021 17:27

XavierM merged commit 50ad304 into elastic:main Nov 17, 2021

kibanamachine mentioned this pull request Nov 17, 2021

[8.0] [SECURITY] add clone functionality to role mapping (#118434) #118923

Merged

legrego changed the title ~~[SECURITY] add clone functionality to role mapping~~ Allow role mappings to be cloned Nov 17, 2021

This was referenced Apr 10, 2024

[EuiBasicTable] Pass click events to actions onClick elastic/eui#7667

Merged

Upgrade EUI to v94.1.0 (major EuiTable refactors) #180514

Merged

Conversation

XavierM commented Nov 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

elasticmachine commented Nov 11, 2021

Uh oh!

XavierM Nov 15, 2021

Choose a reason for hiding this comment

Uh oh!

thompsongl Nov 16, 2021

Choose a reason for hiding this comment

Uh oh!

XavierM commented Nov 16, 2021

Uh oh!

thompsongl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

XavierM commented Nov 16, 2021

Uh oh!

jportner left a comment

Choose a reason for hiding this comment

Uh oh!

jportner Nov 16, 2021

Choose a reason for hiding this comment

Uh oh!

XavierM Nov 17, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jportner Nov 16, 2021

Choose a reason for hiding this comment

Uh oh!

jportner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kibana-ci commented Nov 17, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

Uh oh!

kibanamachine commented Nov 17, 2021

💚 Backport successful

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

XavierM commented Nov 11, 2021 •

edited

Loading