-
Notifications
You must be signed in to change notification settings - Fork 6
[NAE-2201] Dashboard management missing permissions #353
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
Conversation
- add a default role reference with view permission to `dashboard_item.xml` - add a default role reference with view permission to `dashboard_management.xml`
WalkthroughParent POM versions updated to 7.0.0-RC6.1 across modules. Introduces a new Nullable utility, a nullable(...) helper in ActionDelegate, and adds related Groovy/script imports via configuration and application.yaml. Adds metadata Map to Plugin. Grants default role view permissions in two petri nets. Preserves ordering in UserService with LinkedHashSet. Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/plugin/domain/Plugin.java (1)
52-63: Prevent null maps from Lombok builder; add defensive copiesUsing
@Builderon the all-args ctor allowsnullforentryPointsandmetadata, bypassing the non-null defaults from the no-args ctor. Make the builder path null-safe and defensively copy inputs to avoid shared mutable state.Apply:
- public Plugin(String identifier, String name, String version, String description, String url, int restPort, int grpcPort, Map<String, EntryPoint> entryPoints, boolean active, Map<String, String> metadata) { + public Plugin(String identifier, String name, String version, String description, String url, int restPort, int grpcPort, Map<String, EntryPoint> entryPoints, boolean active, Map<String, String> metadata) { this.identifier = identifier; this.name = name; this.version = version; this.description = description; this.url = url; this.restPort = restPort; this.grpcPort = grpcPort; - this.entryPoints = entryPoints; + this.entryPoints = (entryPoints != null) ? new HashMap<>(entryPoints) : new HashMap<>(); this.active = active; - this.metadata = metadata; + this.metadata = (metadata != null) ? new LinkedHashMap<>(metadata) : new LinkedHashMap<>(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (14)
application-engine/pom.xml(1 hunks)application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy(3 hunks)application-engine/src/main/java/com/netgrif/application/engine/configuration/groovy/GroovyShellConfiguration.java(3 hunks)application-engine/src/main/resources/application.yaml(1 hunks)application-engine/src/main/resources/petriNets/engine-processes/dashboard_item.xml(1 hunks)application-engine/src/main/resources/petriNets/engine-processes/dashboard_management.xml(1 hunks)nae-object-library/pom.xml(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/plugin/domain/Plugin.java(3 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/utils/Nullable.java(1 hunks)nae-spring-core-adapter/pom.xml(1 hunks)nae-user-ce/pom.xml(1 hunks)nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/UserServiceImpl.java(1 hunks)nae-user-common/pom.xml(1 hunks)pom.xml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/plugin/domain/Plugin.java (1)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/menu/dashboard/DashboardManagementBody.java (1)
Builder(13-88)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: Build
- GitHub Check: task-list-completed
🔇 Additional comments (17)
nae-user-common/pom.xml (1)
9-9: Parent bump to 7.0.0-RC6.1 — LGTM.
Consistent with the rest of the PR.nae-spring-core-adapter/pom.xml (2)
10-10: RC6.1 parent update — OK.
No other POM changes; aligns with the multi-module bump.
10-10: Validate effective dependency graph against RC6.1 parent
A local grep only showsmapstruct.version(1.5.5.Final) andquerydsl.version(5.0.0). Please runmvn help:effective-pomormvn dependency:treeto confirm no transitive overrides of the parent BOM’s managed versions.nae-object-library/pom.xml (2)
10-10: Parent set to 7.0.0-RC6.1 — looks good.
Matches the intended release candidate.
10-10: Verify compatibility of jackson-datatype-jsr310 2.18.2 override
Root POM’s<jackson.version>is 2.17.1, but this module explicitly setsjackson-datatype-jsr310to 2.18.2. Confirm that upgrading to 2.18.2 won’t cause compatibility issues across modules.nae-user-ce/pom.xml (2)
9-9: Parent version bump to RC6.1 — fine.
No functional changes in this POM.
9-9: Confirm parent version alignment
All modules (application-engine, nae-object-library, nae-spring-core-adapter, nae-user-ce, nae-user-common) still referenceapplication-engine-parentversion7.0.0-RC6.1, but you’re on branchrelease/7.0.0-rev7. Ensure this mismatch is intentional for your release process—or bump all parent versions to7.0.0-rev7as needed.pom.xml (1)
9-9: Root parent version → 7.0.0-RC6.1 — approved.
Centralizes the RC across modules.application-engine/pom.xml (1)
9-9: Modules aligned to 7.0.0-RC6.1; no 7.0.0-SNAPSHOT occurrences found.application-engine/src/main/resources/petriNets/engine-processes/dashboard_management.xml (1)
10-15: No explicit default role definition found
Grep search returned no<role><id>default</id></role>in this net. Confirm that your Petriflow version supports referencing the built-in “default” role without an explicit definition; if it doesn’t, add under the<roles>section:<role> <id>default</id> <title>Default</title> </role>application-engine/src/main/resources/petriNets/engine-processes/dashboard_item.xml (1)
10-15: Confirm implicit default role declaration
No<role><id>default</id>found (only<defaultRole>true</defaultRole>and a<roleRef>). Verify your engine supports implicit default roles; if not, add:<role> <id>default</id> <title>Default</title> </role>application-engine/src/main/java/com/netgrif/application/engine/configuration/groovy/GroovyShellConfiguration.java (2)
11-12: No action.
Import changes are benign.
23-27: Default star imports for Groovy actions — good QoL improvement.
Keeps scripts concise while still allowing config-driven imports.application-engine/src/main/resources/application.yaml (1)
162-162: No override risk—custom imports append to defaults GroovyShellConfiguration.registers default star imports via getDefaultEngineImports() before applying actionsProperties imports, so addingNullablewon’t override existing defaults; switching to a YAML list for readability remains optional.application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy (3)
93-93: LGTM: Nullable importImport placement and scope are fine.
2769-2780: LGTM: clearer error messages in resolveStoragePathUsing String.formatted(...) is fine with Java 21; no behavior change detected.
911-914: No change needed: Nullable.of(null) is supported
Theof(T value)Javadoc explicitly allowsnulland simply wraps it (equivalent toempty()), sonullable(value)is already null-safe by construction.
Description
Fix dashboard permissions (add default role)
Fixes NAE-2201
Dependencies
Third party dependencies
Blocking Pull requests
There are no dependencies on other PR
How Has Been This Tested?
Test Configuration
<Please describe configuration for tests to run if applicable, like program parameters, host OS, VM configuration etc.>
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Chores