feat(ZMSKVR-1345): hide scope source and scope selection for non-superuser - #2414
Merged
ThomasAFink merged 35 commits intoJun 16, 2026
Merged
Conversation
…how-scope-sorce-only-for-superuser
…rusers in Twig Split scope source form into superuser-only and details-only React mounts. Superuser check is enforced server-side in the template only, not in JS.
…pdate Require superuser to create owners, organisations, departments, and scopes. Non-superusers may still update scopes but cannot change provider or source.
addData mutates in place; without a clone, existingScope was already updated before withProviderSourceFrom could restore provider and source.
github-actions
Bot
added
Type: Feature
Module: zmsadmin
Module: zmsapi
Module: zmsentities
labels
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR implements permission-based scope provider/source editing: superusers can modify sources via a form interface, while restricted users view read-only details. The backend preserves provider/source from existing scopes for non-superusers during updates; the frontend splits components by permission; and department email persistence switches from insert to update semantics. ChangesSuperuser permission control for scope provider/source
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
ThomasAFink
changed the title
Member
Author
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
zmsadmin/js/page/scopeEdit/index.js (1)
37-41:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReplace
console.login the AJAX failure branch.These debug logs should be removed or converted to proper error logging behavior.
As per coding guidelines: `**/*.{js,jsx,ts,tsx}`: Flag specific logging violations including `console.log()` usage.Suggested fix
}).fail((err) => { - if (err.status === 404) { - console.log('404 error, ignored') - } else { - console.log('error', err) - } + if (err.status !== 404) { + console.error('Failed to load provider list for source', err) + } })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@zmsadmin/js/page/scopeEdit/index.js` around lines 37 - 41, The AJAX failure branch in scopeEdit/index.js currently uses console.log for error handling; replace these debug prints with the app's proper logging/notification mechanism (e.g., call the centralized logger or error handler and/or show a user-facing message) in the failure callback that contains the if (err.status === 404) check so 404s are handled quietly via the chosen logger/flow and all other errors are logged with error-level details and context; update the branch that references err.status to use the standard logger API (or toast/notification utility) instead of console.log.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@zmsadmin/js/reactcomponents.js`:
- Line 21: Remove the startup debug console log by deleting the
console.log("Loaded react components...") call in reactcomponents.js (do not
leave stray debug prints); if you need non-runtime diagnostics, replace it with
a gated logger or development-only check (e.g., use a debug util or condition on
NODE_ENV === "development") so no console.log remains in production frontend
code.
In `@zmsadmin/src/Zmsadmin/Scope.php`:
- Around line 104-107: The authorization currently trusts a client-supplied
$entity->id to decide create vs update which can be spoofed; update the check in
the Scope class to determine whether this is a create or update by checking
server-side persistence instead of the raw request property (e.g. use the entity
repository/ORM to find an existing record by the provided identifier or a unique
business key, or call a canonical getter that reflects persisted state rather
than raw payload), then enforce getUseraccount()->isSuperUser() only when the
server-side lookup shows the operation would create a new record the caller is
not permitted to create; reference $workstation, $entity and
getUseraccount()->isSuperUser() in your changes and replace the
client-controlled $entity->id branching with a DB/ORM existence check or an
explicit operation flag derived from server state.
In `@zmsapi/src/Zmsapi/DepartmentAddScope.php`:
- Around line 25-31: The current flow loads the department via (new
\BO\Zmsdb\Department())->readEntity(...) before enforcing the superuser
requirement; reorder the checks so Helper\User->checkPermissions('superuser') is
called before calling readEntity, and then after successfully loading
$department run $user->checkRights('department', new
\BO\Zmsentities\Useraccount\EntityAccess($department)); this avoids unnecessary
DB reads and matches the pattern used in other create endpoints.
In `@zmsapi/src/Zmsapi/ScopeUpdate.php`:
- Around line 30-32: The code clones $existingScope and calls
$scope->addData($input), but addData can overwrite the entity id allowing a
body-provided id to bypass route authorization; before calling addData remove or
ignore any id from the incoming payload (e.g. unset $input['id'] or
validate/override $scope->id afterwards) so the id on $scope remains identical
to $existingScope->id; ensure this change is applied in the ScopeUpdate flow
around the clone/addData/testValid sequence (referencing $scope, $existingScope,
and addData).
In `@zmsentities/tests/Zmsentities/ScopeTest.php`:
- Around line 48-61: The test testWithProviderSourceFromAfterAddData currently
doesn't verify that addData() didn't mutate the original $existing via aliasing;
before calling $updated->addData([...]) capture $existing's original provider
and source (e.g. $origProvider = $existing->getProviderId(); $origSource =
$existing->getSource()) and after addData() assert that $existing still returns
those originals (using getProviderId() and getSource()), ensuring addData()
doesn't mutate $existing; keep the existing assertions about
withProviderSourceFrom() as they are.
---
Outside diff comments:
In `@zmsadmin/js/page/scopeEdit/index.js`:
- Around line 37-41: The AJAX failure branch in scopeEdit/index.js currently
uses console.log for error handling; replace these debug prints with the app's
proper logging/notification mechanism (e.g., call the centralized logger or
error handler and/or show a user-facing message) in the failure callback that
contains the if (err.status === 404) check so 404s are handled quietly via the
chosen logger/flow and all other errors are logged with error-level details and
context; update the branch that references err.status to use the standard logger
API (or toast/notification utility) instead of console.log.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b1a162bb-c3f2-4172-b0a2-063905a54bcc
📒 Files selected for processing (21)
zmsadmin/js/block/scope/sourcesDetailsForm.jszmsadmin/js/block/scope/sourcesSuperuserSelectForm.jszmsadmin/js/page/scopeEdit/detailsOnly.jszmsadmin/js/page/scopeEdit/index.jszmsadmin/js/reactcomponents.jszmsadmin/src/Zmsadmin/DepartmentAddScope.phpzmsadmin/src/Zmsadmin/OrganisationAddDepartment.phpzmsadmin/src/Zmsadmin/OwnerAdd.phpzmsadmin/src/Zmsadmin/OwnerAddOrganisation.phpzmsadmin/src/Zmsadmin/Scope.phpzmsadmin/templates/block/scope/form.twigzmsapi/src/Zmsapi/DepartmentAddScope.phpzmsapi/src/Zmsapi/OrganisationAddDepartment.phpzmsapi/src/Zmsapi/OwnerAddOrganisation.phpzmsapi/src/Zmsapi/ScopeUpdate.phpzmsapi/tests/Zmsapi/DepartmentAddScopeTest.phpzmsapi/tests/Zmsapi/OrganisationAddDepartmentTest.phpzmsapi/tests/Zmsapi/OwnerAddOrganisationTest.phpzmsapi/tests/Zmsapi/ScopeUpdateTest.phpzmsentities/src/Zmsentities/Scope.phpzmsentities/tests/Zmsentities/ScopeTest.php
Capture provider/source before addData and verify the reference scope is unchanged, so shallow-clone aliasing regressions are caught.
Restore route-authorized scope id after addData so updates cannot target a different scope than the one checked for access rights.
…create Reorder DepartmentAddScope authorization to match other create endpoints and avoid unnecessary DB reads for rejected callers.
Base scope create/update on loaded existingScope instead of client-supplied entity id so non-superusers cannot bypass create restrictions or miss updates.
MoDaae
reviewed
Jun 2, 2026
MoDaae
reviewed
Jun 2, 2026
MoDaae
reviewed
Jun 2, 2026
MoDaae
reviewed
Jun 2, 2026
MoDaae
reviewed
Jun 2, 2026
Use permissions.scope instead of superuser for DLDB source selection, scope create/update, and the scope form source block. Align add-child endpoints with organisation and department permissions. Rename React blocks to SourceSelectView and SourceReadView with ScopeRestrictedView entry for restrictedscope users.
Require sender email in the department form and schema validation to avoid oneOf null errors when the field is submitted empty.
Schema required email broke user account edits that only submit department ids. Keep Absender-Adresse required in the department form template.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
zmsentities/schema/department.json (1)
89-103:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
minLengthdoes not makeThis only validates
zmsdb/src/Zmsdb/Department.phpwill then skipupdateDepartmentMail()becausetoProperty()->email->isAvailable()is false. If the sender address is now required on save, add a top-level"required": ["email"]and mirror that inzmsentities/schema/dereferenced/department.json.As per coding guidelines, "Always find root cause. Always look for the root cause of a problem."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@zmsentities/schema/department.json` around lines 89 - 103, The schema currently validates "email" only when present, so make it mandatory by adding a top-level "required": ["email"] to department.json and add the same requirement in the dereferenced schema (zmsentities/schema/dereferenced/department.json) to ensure server-side validation; then re-run schema generation/validation so callers cannot omit the key and Department.php's updateDepartmentMail() (and the toProperty()->email->isAvailable() check) will always receive an email value on save.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@zmsentities/schema/department.json`:
- Around line 89-92: The "email" property in zmsentities/schema/department.json
uses an overly strict hand-rolled regex that rejects valid addresses (e.g., with
'+'); replace the pattern with a simpler, permissive validator: remove the
current "pattern" and either add a standard JSON Schema email validator
("format": "email") or a minimal regex such as "^[^@\\s]+@[^@\\s]+\\.[^@\\s]+$"
while keeping "type": "string" and "minLength": 1; update the "email" field
definition accordingly (look for the "email" property object) so common valid
sender addresses are accepted.
---
Outside diff comments:
In `@zmsentities/schema/department.json`:
- Around line 89-103: The schema currently validates "email" only when present,
so make it mandatory by adding a top-level "required": ["email"] to
department.json and add the same requirement in the dereferenced schema
(zmsentities/schema/dereferenced/department.json) to ensure server-side
validation; then re-run schema generation/validation so callers cannot omit the
key and Department.php's updateDepartmentMail() (and the
toProperty()->email->isAvailable() check) will always receive an email value on
save.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1ec5c283-503b-49e1-8aae-2cd3cf3a6649
📒 Files selected for processing (5)
zmsadmin/templates/block/department/form.twigzmsdb/src/Zmsdb/Department.phpzmsdb/src/Zmsdb/Query/Department.phpzmsentities/schema/department.jsonzmsentities/schema/dereferenced/department.json
💤 Files with no reviewable changes (1)
- zmsdb/src/Zmsdb/Query/Department.php
Align Twig mount points with reactcomponents selectors and use restrictedscope for the read-only scope source block.
Departments without a sender address are stored as null; validation must accept null while the admin form still requires email on save.
Align test user with organisation permission and owner access via department 96.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
zmsapi/tests/Zmsapi/DepartmentUpdateTest.php (1)
17-26:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAssert the returned email as well.
Lines 17-21 add
DepartmentUpdateserializeszmsapi/src/Zmsapi/DepartmentUpdate.phpreturns the updated entity, a regression dropping that field would still pass here.Suggested assertion
$this->assertStringContainsString('Test Department Update', (string)$response->getBody()); + $this->assertStringContainsString('"email":"test@example.com"', (string)$response->getBody()); $this->assertStringContainsString('department.json', (string)$response->getBody()); $this->assertTrue(200 == $response->getStatusCode());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@zmsapi/tests/Zmsapi/DepartmentUpdateTest.php` around lines 17 - 26, The test adds an "email" in the request but never verifies it's returned; update DepartmentUpdateTest (the test that calls render) to assert the response includes the email value from the update (e.g. assertStringContainsString('test@example.com', (string)$response->getBody()) or decode the response JSON and assertEquals('test@example.com', $json['email'])); locate the call to render and the subsequent assertions in DepartmentUpdateTest and add this email assertion so DepartmentUpdate's serialization of email is validated.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@zmsapi/tests/Zmsapi/DepartmentUpdateTest.php`:
- Around line 17-26: The test adds an "email" in the request but never verifies
it's returned; update DepartmentUpdateTest (the test that calls render) to
assert the response includes the email value from the update (e.g.
assertStringContainsString('test@example.com', (string)$response->getBody()) or
decode the response JSON and assertEquals('test@example.com', $json['email']));
locate the call to render and the subsequent assertions in DepartmentUpdateTest
and add this email assertion so DepartmentUpdate's serialization of email is
validated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 48ec290b-c45c-4bb8-a961-b0bf9fc933bd
⛔ Files ignored due to path filters (1)
zmsstatistic/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
zmsadmin/js/reactcomponents.jszmsadmin/templates/block/scope/form.twigzmsapi/tests/Zmsapi/DepartmentUpdateTest.phpzmsapi/tests/Zmsapi/OwnerAddOrganisationTest.phpzmsentities/schema/department.jsonzmsentities/schema/dereferenced/department.json
…ddScopeTest Grant department permission so checkRights passes alongside scope permission and department assignment.
Add cases for missing department, scope, or both permissions.
Introduce jurisdiction permission with DB migrations and grant it to system_admin. Gate owner CRUD in zmsadmin and zmsapi on jurisdiction instead of superuser or organisation.
Required after jurisdiction permission check was added to OwnerDelete.
MoDaae
reviewed
Jun 5, 2026
MoDaae
approved these changes
Jun 8, 2026
ThomasAFink
deleted the
feature-zmskvr-1345-show-scope-sorce-only-for-superuser
branch
July 31, 2026 13:15
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Checklist (Feature Branch to
next):nextBranch in meinen Feature-Branch gemergt.docshinzugefügt.cleans up js form seperation
adds owner permission called jurisdiction
Summary by CodeRabbit
Release Notes
New Features
Improvements