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

Navigation Node visibility with custom nodeAccessibilityResolver #67

Merged
merged 24 commits into from
Sep 12, 2018

Conversation

maxmarkus
Copy link
Contributor

Description

Implemented Luigi.config.navigation.nodeAccessibilityResolver for checking Node access restrictions

Related issue(s)
Fixes #44

# Conflicts:
#	core/examples/luigi-sample-angular/package-lock.json
#	core/package-lock.json
#	core/test/routing.spec.js
@maxmarkus maxmarkus added the enhancement New feature or request label Sep 10, 2018
@maxmarkus maxmarkus changed the title Feature/visibility restriction base Navigation Node access restriction Sep 10, 2018
@maxmarkus maxmarkus changed the title Navigation Node access restriction Navigation Node access restriction with nodeAccessibilityResolver Sep 10, 2018
@maxmarkus maxmarkus changed the title Navigation Node access restriction with nodeAccessibilityResolver Navigation Node visibility with custom nodeAccessibilityResolver Sep 10, 2018
@dariadomagala-sap dariadomagala-sap self-assigned this Sep 11, 2018
Copy link
Contributor

@dariadomagala-sap dariadomagala-sap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the issue description: "I can set the visibility of a node/microfrontend based on my constraints". Right now the node is not visible in the navigation which is good, but I can see the microfrontend if I type the path in the address bar myself.

Copy link
Contributor

@dariadomagala-sap dariadomagala-sap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Just added small suggestions :)

@@ -58,6 +59,10 @@ window.Luigi.setConfig({

### Navigation parameters

- **nodeAccessibilityResolver** allows you to define a permission checker function that gets executed on every Node. If it returns false, the Node and it's children are removed from the navigation structure. You may want to add custom values to Nodes to receive it in the **nodeAccessibilityResolver** function. See [angular sampleconfig.js](../core/examples/luigi-sample-angular/src/assets/sampleconfig.js) for our constraints example.
Copy link

@kazydek kazydek Sep 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • it's children -> its children
  • it returns false -> it returns `false`
  • preferably it would be great if you could replace the passive voice here with such a structure:If it returns `false`, {subject} removes the Node and its children from the navigation structure.
  • You may want to add custom values to a Node to receive it in the nodeAccessibilityResolver function. -> To receive a Node in the **nodeAccessibilityResolver** function, add custom values to it.(I hope the meaning is kept?)
  • for our constraints example. -> for the **constraints** example.

@@ -58,6 +59,10 @@ window.Luigi.setConfig({

### Navigation parameters

- **nodeAccessibilityResolver** allows you to define a permission checker function that gets executed on every Node. If it returns false, the Node and it's children are removed from the navigation structure. You may want to add custom values to Nodes to receive it in the **nodeAccessibilityResolver** function. See [angular sampleconfig.js](../core/examples/luigi-sample-angular/src/assets/sampleconfig.js) for our constraints example.

## Nodes
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is ## instead of###intentional here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, since this is a standalone feature while Nodes section is a collection of settings for a Node.

{
  navigation: {
    nodeAccessibilityResolver: someFunction,
    nodes: [node1, node2] // every nodeN consists of parts described in ## Nodes
  }
}

@maxmarkus maxmarkus merged commit cf93a24 into SAP:master Sep 12, 2018
@maxmarkus maxmarkus deleted the feature/visibility-restriction-base branch September 12, 2018 13:19
stanleychh pushed a commit to stanleychh/luigi that referenced this pull request Dec 30, 2021
)

* added sample navigationPermissionChecker and added functionality to navigation without parent tree
* implemented nodeAccessibilityResolver for permission checking, added sample and test
* fixed non-working root-level check, added component for restricted view
* documentation improvements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Visibility restriction base implementation
3 participants