UI make navbar options configurable and filter menu items according to user groups #3154

JBWatenbergScality · 2021-02-26T18:23:28Z

Component:

ui, shell-ui

Context:

Menu items displayed in the navbar should be configurable and filtered according to user groups (as provided as part of OIDC claims).

Acceptance criteria:

Unit tests

Closes : #3159

bert-e · 2021-03-10T15:44:37Z

Hello jbwatenbergscality,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Status report is not available.

bert-e · 2021-03-10T15:44:41Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

gdemonet

Some small code improvements, behaviour looks good to me 👍

shell-ui/src/NavBar.js

gdemonet · 2021-03-11T16:07:20Z

shell-ui/src/NavBar.js

+          try {
+            return {
+              ...renderer(path, translationAndGroup),
+              selected: normalizedLocation === normalizePath(path),


Could be interesting to warn somehow (likely in console) when not a single entry matches the current location.

Also, from what I can see, normalized paths are simply p.origin + p.pathname. Isn't this going to break when a path is only linking to the root of a single-page app, and this app then manipulates the URL for virtual routes?

You are right, I think I will add a regex in options to let solutions configure which routes should be considered as selected and maybe use this implementation as the default one if no regex is provided.

Indeed, maybe assume that path is always a regex (not sure how to handle escapes though).

shell-ui/src/NavBar.js

shell-ui/src/Navbar.spec.js

shell-ui/src/auth/permissionUtils.js

shell-ui/src/index.js

shell-ui/src/auth/permissionUtils.js

shell-ui/src/setupTests.js

bert-e · 2021-03-12T09:57:29Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

The following reviewers are expecting changes from the author, or must review again:

@gdemonet

ChengYanJin · 2021-03-11T21:05:31Z

shell-ui/src/Navbar.spec.js

@@ -62,7 +62,7 @@ describe('navbar', () => {
            />


Would be great to format the code^^

ChengYanJin · 2021-03-11T21:06:35Z

shell-ui/src/setupTests.js

@@ -2,6 +2,36 @@ import 'whatwg-fetch';
 import "regenerator-runtime/runtime"


Missing the auto-format here too.

ChengYanJin · 2021-03-12T11:03:48Z

shell-ui/src/auth/permissionUtils.spec.js

+    );
+  });
+
+  it('should normalize path throw if invalid url', () => {


Suggested change

it('should normalize path throw if invalid url', () => {

it('should throw if the path is an invalid url', () => {

…pages

… alert menu items

gdemonet

I'm OK with merging in this state 👌

For answering "which UI is active at the moment?", it will likely become easier once we switch tables and get the Shell to include other UIs instead of the opposite.

gdemonet · 2021-03-15T14:57:44Z

shell-ui/src/NavBar.js

+                ? new RegExp(translationAndGroup.activeIfMatches).test(
+                    location.href,
+                  )
+                : normalizedLocation === normalizePath(path),


I'm afraid this default isn't really useful, since it will never match on "sub-paths"...
Maybe normalizedLocation.startsWith(normalizePath(path)) instead?

I also thought about switching to startsWith but IMO it is not a correct approach because we can for example have a menu entry for / and for /alerts on the same UI where this default will make both entries selected. I think an exact match is the strongest default to avoid duplicated selected entries that could mislead the user.

Mmh, right. So we'd need to sort by specificity of configured paths to know "which Ingress is catching the current page", I guess.

bert-e · 2021-03-15T15:01:51Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

JBWatenbergScality · 2021-03-15T15:31:41Z

/approve

bert-e · 2021-03-15T16:44:53Z

In the queue

The changeset has received all authorizations and has been added to the
relevant queue(s). The queue(s) will be merged in the target development
branch(es) as soon as builds have passed.

The changeset will be merged in:

✔️ development/2.9

The following branches will NOT be impacted:

development/1.0
development/1.1
development/1.2
development/1.3
development/2.0
development/2.1
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6
development/2.7
development/2.8

There is no action required on your side. You will be notified here once
the changeset has been merged. In the unlikely event that the changeset
fails permanently on the queue, a member of the admin team will
contact you to help resolve the matter.

IMPORTANT

Please do not attempt to modify this pull request.

Any commit you add on the source branch will trigger a new cycle after the
current queue is merged.
Any commit you add on one of the integration branches will be lost.

If you need this pull request to be removed from the queue, please contact a
member of the admin team now.

The following options are set: approve

bert-e · 2021-03-15T18:27:20Z

I have successfully merged the changeset of this pull request
into targetted development branches:

✔️ development/2.9

The following branches have NOT changed:

development/1.0
development/1.1
development/1.2
development/1.3
development/2.0
development/2.1
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6
development/2.7
development/2.8

Please check the status of the associated issue None.

Goodbye jbwatenbergscality.

JBWatenbergScality changed the title ~~Improvement/shell UI make navbar options configurable~~ UI make navbar options configurable and filter menu items according to user groups Feb 26, 2021

JBWatenbergScality force-pushed the improvement/use-solutions-navbar-in-metalk8s-ui branch from 34ed4dc to d47f6cd Compare March 1, 2021 10:22

JBWatenbergScality force-pushed the improvement/shell-ui-make-navbar-options-configurable branch from 11b39d9 to 63d8189 Compare March 1, 2021 10:23

JBWatenbergScality force-pushed the improvement/use-solutions-navbar-in-metalk8s-ui branch from d47f6cd to 43581e4 Compare March 1, 2021 13:55

JBWatenbergScality force-pushed the improvement/shell-ui-make-navbar-options-configurable branch from 63d8189 to e626624 Compare March 1, 2021 16:59

JBWatenbergScality mentioned this pull request Mar 1, 2021

Use solutions navbar in metalk8s UI #3110

Merged

JBWatenbergScality force-pushed the improvement/shell-ui-make-navbar-options-configurable branch 3 times, most recently from 0b00a42 to 428203e Compare March 3, 2021 15:43

JBWatenbergScality marked this pull request as ready for review March 3, 2021 15:47

JBWatenbergScality requested a review from a team as a code owner March 3, 2021 15:47

JBWatenbergScality added the topic:ui UI-related issues label Mar 3, 2021

JBWatenbergScality force-pushed the improvement/use-solutions-navbar-in-metalk8s-ui branch 2 times, most recently from 9c96d6d to d695076 Compare March 5, 2021 15:42

JBWatenbergScality force-pushed the improvement/shell-ui-make-navbar-options-configurable branch 2 times, most recently from 9e70a43 to c7f33a8 Compare March 8, 2021 09:06

JBWatenbergScality force-pushed the improvement/use-solutions-navbar-in-metalk8s-ui branch 5 times, most recently from 0a7bd93 to d695076 Compare March 8, 2021 15:11

JBWatenbergScality force-pushed the improvement/shell-ui-make-navbar-options-configurable branch from c7f33a8 to 904782f Compare March 8, 2021 15:25

JBWatenbergScality force-pushed the improvement/use-solutions-navbar-in-metalk8s-ui branch from d695076 to 73e4a52 Compare March 9, 2021 18:12

JBWatenbergScality force-pushed the improvement/shell-ui-make-navbar-options-configurable branch from 904782f to d870add Compare March 9, 2021 18:13

Base automatically changed from improvement/use-solutions-navbar-in-metalk8s-ui to development/2.9 March 10, 2021 15:44

JBWatenbergScality mentioned this pull request Mar 12, 2021

shell-ui : Add users group mapping in shell UI config #3196

Merged

gdemonet suggested changes Mar 12, 2021

View reviewed changes

ChengYanJin reviewed Mar 12, 2021

View reviewed changes

JBWatenbergScality added 4 commits March 15, 2021 09:55

shell-ui: Make navbar options configurable

55abad5

shell-ui: Filter menu based on user groups

3e9040b

shell-ui: Preload links to minimize flashing when navigating between …

a51caed

…pages

ui: Configure navbar so that it displays documentation, plateform and…

c2efd12

… alert menu items

JBWatenbergScality force-pushed the improvement/shell-ui-make-navbar-options-configurable branch from 41af475 to 06a5496 Compare March 15, 2021 09:08

JBWatenbergScality added 2 commits March 15, 2021 12:31

shell-ui: Use regexp to matches active paths

472bb94

shell-ui: Format navbar tests

06e1b08

JBWatenbergScality force-pushed the improvement/shell-ui-make-navbar-options-configurable branch from 06a5496 to 06e1b08 Compare March 15, 2021 11:31

JBWatenbergScality requested review from gdemonet and ChengYanJin March 15, 2021 11:31

gdemonet approved these changes Mar 15, 2021

View reviewed changes

bert-e merged commit cd742b5 into development/2.9 Mar 15, 2021

bert-e deleted the improvement/shell-ui-make-navbar-options-configurable branch March 15, 2021 18:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UI make navbar options configurable and filter menu items according to user groups #3154

UI make navbar options configurable and filter menu items according to user groups #3154

JBWatenbergScality commented Feb 26, 2021 •

edited

Loading

bert-e commented Mar 10, 2021

bert-e commented Mar 10, 2021

gdemonet left a comment

gdemonet Mar 11, 2021

gdemonet Mar 12, 2021

JBWatenbergScality Mar 12, 2021

gdemonet Mar 12, 2021

bert-e commented Mar 12, 2021

ChengYanJin Mar 11, 2021

ChengYanJin Mar 11, 2021

ChengYanJin Mar 12, 2021

gdemonet left a comment

gdemonet Mar 15, 2021

JBWatenbergScality Mar 15, 2021

gdemonet Mar 15, 2021

bert-e commented Mar 15, 2021

JBWatenbergScality commented Mar 15, 2021

bert-e commented Mar 15, 2021

bert-e commented Mar 15, 2021

		@@ -2,6 +2,36 @@ import 'whatwg-fetch';
		import "regenerator-runtime/runtime"

	it('should normalize path throw if invalid url', () => {
	it('should throw if the path is an invalid url', () => {

UI make navbar options configurable and filter menu items according to user groups #3154

UI make navbar options configurable and filter menu items according to user groups #3154

Conversation

JBWatenbergScality commented Feb 26, 2021 • edited Loading

bert-e commented Mar 10, 2021

Hello jbwatenbergscality,

bert-e commented Mar 10, 2021

Waiting for approval

gdemonet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bert-e commented Mar 12, 2021

Waiting for approval

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gdemonet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bert-e commented Mar 15, 2021

Waiting for approval

JBWatenbergScality commented Mar 15, 2021

bert-e commented Mar 15, 2021

In the queue

bert-e commented Mar 15, 2021

JBWatenbergScality commented Feb 26, 2021 •

edited

Loading