fix: group scopes granted nobody (#83); wizard tests were aiming at the list pagination (#86) - #88
Merged
Merged
Conversation
…he list pagination
All 8 createApplicationWizard scenarios failed, reproducibly, with the wizard's
Next button "visible, enabled and stable" and every click swallowed:
<div class="dialog__actions"> from <div ... data-testid-modal="cn-wizard-dialog">
subtree intercepts pointer events
That reads as a broken dialog. It is not. `getByRole('button', { name: /^next$/i })
.first()` was never finding the wizard's Next at all.
The applications list BEHIND the modal renders a pagination control whose
button is also labelled "Next" (`.cn-pagination__nav`). It comes first in DOM
order, so `.first()` took it. Measured at the moment of failure:
Next #1 y=1318 secondary .cn-pagination__nav <- picked, off-screen
Next #2 y= 616 PRIMARY .dialog__actions [cn-wizard-dialog]
Viewport is 720px tall. Playwright judged the pagination button visible and
enabled (it has a box and is not display:none), scrolled to it, and the modal
overlay then intercepted the click — so the error named the DIALOG while the
target was a page element underneath it.
It only started failing once the seeded fixture apps grew past one page and the
pagination appeared, which is why it looked like a wizard regression. It is not
version-related either: reproduced identically on @conduction/nextcloud-vue
2.1.0-vue3.7 and 2.1.0-vue3.15.
Every wizard action button is now looked up inside
`[data-testid-modal="cn-wizard-dialog"]` via a `wizard(page)` helper — Next,
Create and Back. "Add app" stays page-scoped; it genuinely lives on the page.
8/8 pass.
Closes #86.
…e granted nobody
Application permission buckets carry `user:<uid>`, `group:<gid>` or a bare gid
(useRole.js). `availableGroups` filtered out only the `user:` form and passed
`group:rbac-editors` straight through to the Access editor's dropdown — so the
value an admin picked was also the value written into the schema's read rule.
OpenRegister matches read rules against getUserGroupIds(), which returns BARE
gids. A prefixed rule therefore matched nobody: any group scope configured
through this UI silently denied everyone, including the person who set it.
Measured on a live instance against the same schema, restoring between runs:
read: ["authenticated"] -> a member of rbac-editors saw 22 objects
read: ["group:rbac-editors"] -> the same member saw 0
read: ["rbac-editors"] -> the same member saw 22
The same mismatch made `authorLockedOut` fire for members, because it compares
the selected values against getCurrentUserGroups() (also bare) — so the
REQ-OBDSA-004 "a member editor sees NO warning" scenario could never pass. That
is the symptom this was filed under; the dead scope underneath it is worse.
availableGroups now strips the prefix, leaves an already-bare gid alone, and
dedupes when a group appears in both forms.
Every pre-existing test in SchemaDesigner.access.spec.js seeded permissions
with bare gids, which is why 13 green tests sat over a scope that granted
nothing in production. Added 4 covering the prefixed form, including the
member-sees-no-warning case.
17/17 pass.
Closes #83.
Neither suite changes behaviour; both had blocker comments that were partly
wrong, and a wrong blocker is worse than none — it sends the next person to fix
something that is already fixed, or to retarget a selector when the fixture is
what is missing.
schema-access-scopes-rbac — two of the five blockers are now gone:
- the designer is reachable for non-admins (nc-vue 2.1.0-vue3.15; the earlier
#575 attempt was inert, guarded now by non-admin-access.spec.ts);
- prefixed gids are normalised (#83).
What actually blocks it is a DRAFT-VERSION SCHEMA. Measured: the version chain
carries exactly one schema, `pw-verchain-production-hello-message`, and no
`pw-verchain-staging-*`. Every remaining scenario needs a non-production copy —
004/007 because production is owner-only read-only by design, 006 because it
compares the two copies. versionChain.ts creates VERSIONS, not their schemas.
version-rollback — the file said retargeting two selectors would be enough. It
would not. Driven live against the full pw-verchain chain with the tab mounted:
.ob-versions-tab 1 (the tab IS there)
.version-history 1 (the component renders)
.version-history__empty 1 (and renders EMPTY)
.version-history__row 0
.version-history__btn--danger 0
VersionHistory lists PUBLISH SNAPSHOTS, not ApplicationVersions. The old note
had the dependency backwards — rows come from publishing, not from having
versions — so a three-version app still shows zero. What it needs is a fixture
that publishes at least twice, which is new work, not a retarget.
Also recorded there, since reading cannot reveal it: the detail route takes the
UUID not the slug, and the sidebar tabs render WITHOUT clicking
.app-sidebar__toggle — a toggle click times out at 5s because they are already
open, so the original "open the sidebar first" step actively hangs.
Contributor
Quality Report — ConductionNL/openbuild @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ❌ | ||||
| stylelint | ❌ | ||||
| composer | ✅ | ✅ 100/100 | |||
| npm | ❌ | ❌ | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-08-02 08:29 UTC
Download the full PDF report from the workflow artifacts.
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
2 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.
Closes #83. Closes #86.
#86 — the wizard was never broken; my locator was
All 8
createApplicationWizardscenarios failed with the wizard's Next "visible, enabled and stable" and every click swallowed bydialog__actions ... subtree intercepts pointer events. I filed that as a product defect: "the app-creation wizard is unusable." That was wrong.getByRole('button', { name: /^next$/i }).first()was never finding the wizard's Next. The applications list behind the modal renders a pagination control whose button is also "Next". Measured at the moment of failure:Viewport is 720px. Playwright judged the pagination button visible and enabled, scrolled to it, and the modal then intercepted the click — so the error named the dialog while the target was a page element underneath it.
It only began failing once the fixture apps grew past one page and pagination appeared. Not version-related either: identical on
2.1.0-vue3.7and2.1.0-vue3.15.Every wizard action button is now scoped to
[data-testid-modal="cn-wizard-dialog"]. 8/8 pass.#83 — a group scope granted nobody
Filed as a cosmetic warning bug. It is worse than that:
availableGroupspassedgroup:rbac-editorsto the dropdown, and that same value was written into the schema's read rule. OpenRegister matches read rules against bare gids, so any group scope set through this UI silently denied everyone, including the person who set it.Proven on a live instance against one schema, restoring between runs:
rbac-editorsseesread: ["authenticated"]read: ["group:rbac-editors"]read: ["rbac-editors"]Fixed by normalising in
availableGroups(strip the prefix, leave bare alone, dedupe both forms).All 13 pre-existing tests in
SchemaDesigner.access.spec.jsseeded permissions with bare gids — which is why they stayed green over a scope that granted nothing in production. Added 4 for the prefixed form, including the REQ-OBDSA-004 member-sees-no-warning case that could never have passed. 17/17.Two blocker notes replaced with measured ones
No behaviour change, but a wrong blocker sends the next person to fix what is already fixed.
pw-verchain-production-hello-message.versionChain.tscreates versions, not their schemas.VersionHistoryrenders.version-history__emptywith 0 rows on a full three-version chain: it lists publish snapshots, not versions. The dependency was backwards. Also recorded: the detail route takes the UUID not the slug, and the sidebar tabs render without the toggle — clicking it hangs for 5s because they are already open.Verification
createApplicationWizard.spec.ts— 8/8.SchemaDesigner.access.spec.js— 17/17 (run in a container; the host's glibc is too old for the native rollup binary).🤖 Generated with Claude Code