fix(router): resolve deep links in both URL forms - #788
Merged
rubenvdlinde merged 1 commit intoSep 1, 2026
Conversation
Nextcloud serves an app under both /apps/learniq/... and /index.php/apps/learniq/..., but generateUrl() returns only the form the instance is configured for. Used as the vue-router base, that means a visitor arriving on the other form has a pathname the router cannot strip its base from. No route matches, the catch-all takes over, and they land on the dashboard with no error at all. Measured on a live instance, across all 282 routes: /apps/learniq/courses resolves to Courses, /index.php/apps/learniq/courses resolved to the dashboard. Not one broken page, every deep link in that URL form. routerBase() derives the base from the pathname, so it always matches the URL the visitor actually arrived on. openregister, opencatalogi, stackiq, larpinq, zaakafhandelapp, pipelinq and keepiq already do exactly this. pages.spec.ts could not have caught it. Its route table addressed every page as '#/courses', a fragment the history router never reads, so all 24 cases loaded the dashboard and asserted the dashboard was fine. Both of its assertions, a non-blank body and no console errors, hold there. The paths lose the '#' and the test now asserts the URL is the route it asked for, which is the check that keeps this fixed. /settings was in that table and is not a route; it is dropped. Verified live: 23 passed, against 1 passed and 23 failed before the router fix.
Contributor
Quality Report — ConductionNL/learniq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| format | ✅ | ||||
| check-l10n-js | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 104/104 | |||
| npm | ✅ | ✅ 637/637 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-09-01 16:24 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.
Nextcloud serves an app under both
/apps/learniq/...and/index.php/apps/learniq/..., butgenerateUrl()returns only the form the instance is configured for. Used as the vue-router base, a visitor arriving on the other form has a pathname the router cannot strip its base from: no route matches, the catch-all takes over, and they land on the dashboard with no error.Measured on a live instance, across all 282 routes:
/apps/learniq/courses/index.php/apps/learniq/coursesEvery route behaved the same way. This is not one broken page, it is every deep link in that URL form, and it fails silently, which is the part that makes it expensive: a bookmark, an emailed link or an integration that hardcodes
/index.phpjust quietly shows the wrong screen.routerBase()derives the base from the pathname, so it always matches the URL the visitor actually arrived on. This is not a new idea here: openregister, opencatalogi, stackiq, larpinq, zaakafhandelapp, pipelinq and keepiq already do exactly this, and learniq is one of ten apps still on the naive form. The others are integriq, filinq, shillinq, portaliq, decidiq, buildiq, hermiq, humaniq and planninq, and I will carry the same fix to them.Why the test suite did not catch it.
pages.spec.tsaddressed every page as'#/courses'. The router is history mode, so that is a fragment it never reads: all 24 cases loaded the dashboard, and both assertions (non-blank body, no console errors) are satisfied by the dashboard. 24 green tests that had never visited the pages they name. The same warning already sits oncourse-evaluation.spec.ts; this file was missed because its assertions are weak enough to pass anywhere.The paths lose the
#, and the test now asserts the URL is the route it asked for. That assertion is what keeps this fixed, since vue-router's catch-all rewrites an unresolved location to/and a navigation that goes nowhere otherwise passes everything downstream./settingswas in that table and is not a route, so it is dropped rather than kept as a known failure.Verified against a live instance: 23 passed. Before the router fix, the same file gave 1 passed and 23 failed.
Note that the E2E job runs on the push to development, not on this PR, so CI here will not exercise any of it.