Skip to content

Commit ce2bf81

Browse files
rubenvdlinderubenvdlinde
committed
fix(router): resolve deep links in both URL forms
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.
1 parent 86aa230 commit ce2bf81

2 files changed

Lines changed: 85 additions & 26 deletions

File tree

‎src/main.js‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,8 +182,35 @@ const fragments = fragmentCtx
182182
.map((key) => fragmentCtx(key))
183183
const mergedManifest = buildManifest(bundledManifest, fragments, menuLayout)
184184

185+
/**
186+
* The router base for THIS page load.
187+
*
188+
* ⚠️ `generateUrl('/apps/learniq')` alone is not enough. Nextcloud serves the
189+
* app under BOTH `/apps/learniq/...` and `/index.php/apps/learniq/...`, but
190+
* `generateUrl()` returns only the form the instance is configured for. A
191+
* visitor arriving on the other form — a bookmark, an emailed deep link, an
192+
* integration that hardcodes `/index.php` — has a pathname the router cannot
193+
* strip its base from. No route matches, the catch-all takes over, and they
194+
* land on the dashboard with no error at all: the deep link is silently
195+
* swallowed.
196+
*
197+
* Measured on a live instance: `/apps/learniq/courses` resolves to Courses,
198+
* while `/index.php/apps/learniq/courses` resolved to the dashboard. Every
199+
* route behaved the same way, so this was not one broken page but every deep
200+
* link in that URL form.
201+
*
202+
* Deriving the base from the pathname makes both forms resolve, because the
203+
* base then always matches the URL the visitor actually arrived on.
204+
*
205+
* @return {string} The base path vue-router should strip from the URL.
206+
*/
207+
function routerBase() {
208+
const match = window.location.pathname.match(/^(.*\/apps\/learniq)(?:\/|$)/)
209+
return match ? match[1] : generateUrl('/apps/learniq')
210+
}
211+
185212
const router = createRouter({
186-
history: createWebHistory(generateUrl('/apps/learniq')),
213+
history: createWebHistory(routerBase()),
187214
routes: routesFromManifest(mergedManifest),
188215
})
189216

‎tests/e2e/pages.spec.ts‎

Lines changed: 57 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -11,35 +11,47 @@ import { expect, test } from './fixtures.ts'
1111
* Routes taken from src/manifest.json pages[].route.
1212
* Dynamic segments are replaced with placeholder values that produce a valid
1313
* URL (the app should show an empty-state or a not-found message, not crash).
14+
*
15+
* ⚠️ NO `#` in these paths. The router is history mode (`createWebHistory` in
16+
* src/main.js), so `#/courses` is a fragment the router never reads, not a
17+
* route. Every entry here used to carry one, which meant every test in this
18+
* file loaded the dashboard and asserted the dashboard was fine. The same
19+
* warning is on course-evaluation.spec.ts; this file was missed because its
20+
* assertions are weak enough to pass on any page that renders at all.
21+
*
22+
* `/settings` was in this table and is not a route — the app has no such page.
23+
* It is dropped rather than kept as a known failure, because a table of routes
24+
* is only useful if every entry is one.
1425
*/
1526

27+
const APP_BASE = '/index.php/apps/learniq'
28+
1629
const ROUTES: { name: string; path: string }[] = [
17-
{ name: 'Dashboard', path: '#/' },
18-
{ name: 'Learning', path: '#/learning' },
19-
{ name: 'People', path: '#/people' },
20-
{ name: 'Courses', path: '#/courses' },
21-
{ name: 'Enrolments', path: '#/enrolments' },
22-
{ name: 'Credentials', path: '#/credentials' },
23-
{ name: 'Compliance', path: '#/compliance' },
24-
{ name: 'CourseDetail', path: '#/courses/test-id' },
25-
{ name: 'LessonIndex', path: '#/courses/test-id/lessons' },
26-
{ name: 'LessonDetail', path: '#/courses/test-id/lessons/test-lesson-id' },
27-
{ name: 'LessonPlayer', path: '#/courses/test-id/lessons/test-lesson-id/play' },
28-
{ name: 'Settings', path: '#/settings' },
29-
{ name: 'EnrolmentDetail', path: '#/enrolments/test-id' },
30-
{ name: 'BulkEnrol', path: '#/enrolments/bulk' },
31-
{ name: 'Regulations', path: '#/compliance/regulations' },
32-
{ name: 'RegulationDetail', path: '#/compliance/regulations/test-slug' },
33-
{ name: 'Attestations', path: '#/compliance/attestations' },
34-
{ name: 'AttestationDetail', path: '#/compliance/attestations/test-id' },
35-
{ name: 'AuditPackExport', path: '#/compliance/export' },
36-
{ name: 'CredentialDetail', path: '#/credentials/test-id' },
37-
{ name: 'CredentialVerify', path: '#/credentials/test-id/verify' },
38-
{ name: 'LearnerHome', path: '#/learner' },
30+
{ name: 'Dashboard', path: '/' },
31+
{ name: 'Learning', path: '/learning' },
32+
{ name: 'People', path: '/people' },
33+
{ name: 'Courses', path: '/courses' },
34+
{ name: 'Enrolments', path: '/enrolments' },
35+
{ name: 'Credentials', path: '/credentials' },
36+
{ name: 'Compliance', path: '/compliance' },
37+
{ name: 'CourseDetail', path: '/courses/test-id' },
38+
{ name: 'LessonIndex', path: '/courses/test-id/lessons' },
39+
{ name: 'LessonDetail', path: '/courses/test-id/lessons/test-lesson-id' },
40+
{ name: 'LessonPlayer', path: '/courses/test-id/lessons/test-lesson-id/play' },
41+
{ name: 'EnrolmentDetail', path: '/enrolments/test-id' },
42+
{ name: 'BulkEnrol', path: '/enrolments/bulk' },
43+
{ name: 'Regulations', path: '/compliance/regulations' },
44+
{ name: 'RegulationDetail', path: '/compliance/regulations/test-slug' },
45+
{ name: 'Attestations', path: '/compliance/attestations' },
46+
{ name: 'AttestationDetail', path: '/compliance/attestations/test-id' },
47+
{ name: 'AuditPackExport', path: '/compliance/export' },
48+
{ name: 'CredentialDetail', path: '/credentials/test-id' },
49+
{ name: 'CredentialVerify', path: '/credentials/test-id/verify' },
50+
{ name: 'LearnerHome', path: '/learner' },
3951
// @e2e data-exchange::data-exchange-page-remains-routable-via-deep-link
4052
// Nav entry moved to Admin Settings (relocate-dataexchange-remove-assistant); pages stay routable.
41-
{ name: 'DataExchangeJobs', path: '#/data-exchange/jobs' },
42-
{ name: 'DataMappingProfiles', path: '#/data-exchange/mapping-profiles' },
53+
{ name: 'DataExchangeJobs', path: '/data-exchange/jobs' },
54+
{ name: 'DataMappingProfiles', path: '/data-exchange/mapping-profiles' },
4355
]
4456

4557
test.describe('Learniq page routes', () => {
@@ -67,11 +79,31 @@ test.describe('Learniq page routes', () => {
6779
}
6880
})
6981

70-
await page.goto(`/index.php/apps/learniq/${path}`)
82+
await page.goto(`${APP_BASE}${path}`)
7183

7284
// Wait for the page to stabilise
7385
await page.waitForLoadState('domcontentloaded', { timeout: 15_000 })
7486

87+
// The route must still be the one we asked for.
88+
//
89+
// This is the assertion that makes the rest of the test mean
90+
// something. vue-router's catch-all rewrites a location it cannot
91+
// resolve to `/`, so a request that fails to route does not error —
92+
// it renders the dashboard. Both checks below (non-blank body, no
93+
// console errors) are then satisfied by the dashboard, for every
94+
// route in the table, whether or not the page under test exists.
95+
//
96+
// Matched WITHOUT the `/index.php` prefix on purpose: Nextcloud
97+
// serves the app under both forms and redirects to whichever the
98+
// instance is configured for, so pinning the requested form fails
99+
// on the redirect rather than on the route.
100+
await expect(page).toHaveURL(
101+
new RegExp(
102+
`${`/apps/learniq${path}`.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}/?$`,
103+
),
104+
{ timeout: 10_000 },
105+
)
106+
75107
// The page body must not be blank
76108
const bodyText = await page.innerText('body')
77109
expect(

0 commit comments

Comments
 (0)