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

Update menu generator to better display existing schema IFC-834 #4716

Merged
merged 4 commits into from
Oct 25, 2024

Conversation

dgarros
Copy link
Collaborator

@dgarros dgarros commented Oct 24, 2024

fixes ifc-834

This PR improve the compatibility for the current schema format with the new menu. By default, if a node/generic is meant to the included in the schema it will be displayed at the top level.
Menu placement can still be used as well. In order to support the current format, the identifier for the schema is no longer using a :, instead the identifier is identical as the element of the schema <namespace><name>.

if a menu item with the same identifier already exist in the menu it will take precedence over what is in the schema.

This PR also adds the initial support for permissions to the menu.
For each CoreMenuItem it's now possible to defined a list of required_permissions, currently only global permissions are supported.
At least of the permission must match for the menu to be visible.
By default, the menu Admin will be only visible if the user has the global:super_admin permission

And finally, the structure of the menu has slightly changed

  • Credential is now under Unified Storage
  • Webhook is now part of a new Integrations menu item

@github-actions github-actions bot added the group/backend Issue related to the backend (API Server, Git Agent) label Oct 24, 2024
@dgarros dgarros marked this pull request as ready for review October 24, 2024 19:39
@dgarros dgarros requested review from a team October 24, 2024 19:39
Copy link

codspeed-hq bot commented Oct 25, 2024

CodSpeed Performance Report

Merging #4716 will degrade performances by 10.8%

Comparing dga-20241024-ifc-834 (e50d59f) with release-1.0 (81421bf)

Summary

❌ 1 (👁 1) regressions
✅ 9 untouched benchmarks

Benchmarks breakdown

Benchmark release-1.0 dga-20241024-ifc-834 Change
👁 test_get_menu 422.2 ms 473.3 ms -10.8%

Copy link
Contributor

@ogenstad ogenstad left a comment

Choose a reason for hiding this comment

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

Aside my minor comment this looks good to me. However it doesn't seem to work, it looks like the frontend isn't sending in an authorization header for the menu api:

Screenshot 2024-10-25 at 13 18 55


if not account:
return menu
Copy link
Contributor

Choose a reason for hiding this comment

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

This part should never come into play get_menu() will always send an object but it might be an unauthenticated object. Even if that happens I think it could make sense to run the code below so that we'd also hide the admin menu for anonymous users. The function signature here could change account: AccountSession | None = None -> account: AccountSession

@github-actions github-actions bot added the group/frontend Issue related to the frontend (React) label Oct 25, 2024
@dgarros dgarros merged commit a328399 into release-1.0 Oct 25, 2024
32 checks passed
@dgarros dgarros deleted the dga-20241024-ifc-834 branch October 25, 2024 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
group/backend Issue related to the backend (API Server, Git Agent) group/frontend Issue related to the frontend (React)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants