Repository navigation
fix(e2e): assert create authorization directly, not through the empty state - #380
Merged
rubenvdlinde merged 1 commit intoAug 28, 2026
Conversation
… state The last red spec on launchpad's development. #362 fixme'd two empty-state tests when #361 (c9e5808) made that state unreachable; this third one used the same locator and was missed, so it kept failing: Error: expect(locator).toBeVisible() failed Locator: locator('.workspace-shell__empty-cta') Error: element(s) not found SeedDefaultDashboard now provisions an instance-wide, group-shared dashboard on install and after every upgrade, so no account resolves to zero dashboards and no user ever sees the empty-state CTA. The CTA was never the claim. The claim is REQ-ASET-003 — an ordinary user may create a dashboard when allowUserDashboards is on — and that is what actually regressed once: `dashboard.create` 403'd and the user was stranded on the empty state. The sibling fixmes say so themselves: "the underlying rule is still worth covering, but it needs an assertion that does not route through the empty state." This is that assertion. It POSTs to the endpoint as a throwaway non-admin and requires 200/201. Three details carried over from the file's own hard-won conventions, rather than reinvented: - Basic-Auth + OCS-APIRequest via the existing `userApi` helper. A cookie-authenticated POST with no `requesttoken` is rejected by Nextcloud's CSRF check before reaching the controller, which would fail this test for a reason unrelated to authorization. - `expect([200, 201]).toContain(status)`, NOT `not.toBe(403)`. The weaker form also passes on 401 and 404, neither of which proves an ordinary user may create anything. That exact false green is documented two tests above. - The route is `dashboardApi#create`, `POST /api/dashboard`, read from appinfo/routes.php. The same file records an earlier draft that used a made-up path, got 404, and would have passed a weaker check. `create()` takes only optional parameters and is `#[NoAdminRequired]`, so a bare POST reaches `denyCreate()` — which is precisely the gate under test. Verified: npm ci rc=0, npm run lint rc=0, prettier clean, and `playwright test --list` compiles all 8 tests in the file.
Contributor
Quality Report — ConductionNL/launchpad @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 104/104 | |||
| npm | ✅ | ✅ 538/538 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | 🚨 NO VERDICT — enabled but never ran | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-28 18:50 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
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.
The last red spec on launchpad's development. #362 fixme'd two empty-state
tests when #361 (c9e5808) made that state unreachable; this third one
used the same locator and was missed, so it kept failing:
SeedDefaultDashboard now provisions an instance-wide, group-shared
dashboard on install and after every upgrade, so no account resolves to
zero dashboards and no user ever sees the empty-state CTA.
The CTA was never the claim. The claim is REQ-ASET-003 — an ordinary user
may create a dashboard when allowUserDashboards is on — and that is what
actually regressed once:
dashboard.create403'd and the user wasstranded on the empty state. The sibling fixmes say so themselves: "the
underlying rule is still worth covering, but it needs an assertion that
does not route through the empty state."
This is that assertion. It POSTs to the endpoint as a throwaway
non-admin and requires 200/201.
Three details carried over from the file's own hard-won conventions,
rather than reinvented:
userApihelper. Acookie-authenticated POST with no
requesttokenis rejected byNextcloud's CSRF check before reaching the controller, which would
fail this test for a reason unrelated to authorization.
expect([200, 201]).toContain(status), NOTnot.toBe(403). Theweaker form also passes on 401 and 404, neither of which proves an
ordinary user may create anything. That exact false green is
documented two tests above.
dashboardApi#create,POST /api/dashboard, read fromappinfo/routes.php. The same file records an earlier draft that used
a made-up path, got 404, and would have passed a weaker check.
create()takes only optional parameters and is#[NoAdminRequired], soa bare POST reaches
denyCreate()— which is precisely the gate undertest.
Verified: npm ci rc=0, npm run lint rc=0, prettier clean, and
playwright test --listcompiles all 8 tests in the file.