Skip to content

fix(router): resolve deep links in both URL forms #788

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rubenvdlinde merged 1 commit into from
Sep 1, 2026
Merged

fix(router): resolve deep links in both URL forms #788

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion src/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,35 @@ const fragments = fragmentCtx
.map((key) => fragmentCtx(key))
const mergedManifest = buildManifest(bundledManifest, fragments, menuLayout)

/**
* The router base for THIS page load.
*
* ⚠️ `generateUrl('/apps/learniq')` alone is not enough. Nextcloud serves the
* app under BOTH `/apps/learniq/...` and `/index.php/apps/learniq/...`, but
* `generateUrl()` returns only the form the instance is configured for. A
* visitor arriving on the other form — a bookmark, an emailed deep link, an
* integration that hardcodes `/index.php` — 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: the deep link is silently
* swallowed.
*
* Measured on a live instance: `/apps/learniq/courses` resolves to Courses,
* while `/index.php/apps/learniq/courses` resolved to the dashboard. Every
* route behaved the same way, so this was not one broken page but every deep
* link in that URL form.
*
* Deriving the base from the pathname makes both forms resolve, because the
* base then always matches the URL the visitor actually arrived on.
*
* @return {string} The base path vue-router should strip from the URL.
*/
function routerBase() {
const match = window.location.pathname.match(/^(.*\/apps\/learniq)(?:\/|$)/)
return match ? match[1] : generateUrl('/apps/learniq')
}

const router = createRouter({
history: createWebHistory(generateUrl('/apps/learniq')),
history: createWebHistory(routerBase()),
routes: routesFromManifest(mergedManifest),
})

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

const APP_BASE = '/index.php/apps/learniq'

const ROUTES: { name: string; path: string }[] = [
{ name: 'Dashboard', path: '#/' },
{ name: 'Learning', path: '#/learning' },
{ name: 'People', path: '#/people' },
{ name: 'Courses', path: '#/courses' },
{ name: 'Enrolments', path: '#/enrolments' },
{ name: 'Credentials', path: '#/credentials' },
{ name: 'Compliance', path: '#/compliance' },
{ name: 'CourseDetail', path: '#/courses/test-id' },
{ name: 'LessonIndex', path: '#/courses/test-id/lessons' },
{ name: 'LessonDetail', path: '#/courses/test-id/lessons/test-lesson-id' },
{ name: 'LessonPlayer', path: '#/courses/test-id/lessons/test-lesson-id/play' },
{ name: 'Settings', path: '#/settings' },
{ name: 'EnrolmentDetail', path: '#/enrolments/test-id' },
{ name: 'BulkEnrol', path: '#/enrolments/bulk' },
{ name: 'Regulations', path: '#/compliance/regulations' },
{ name: 'RegulationDetail', path: '#/compliance/regulations/test-slug' },
{ name: 'Attestations', path: '#/compliance/attestations' },
{ name: 'AttestationDetail', path: '#/compliance/attestations/test-id' },
{ name: 'AuditPackExport', path: '#/compliance/export' },
{ name: 'CredentialDetail', path: '#/credentials/test-id' },
{ name: 'CredentialVerify', path: '#/credentials/test-id/verify' },
{ name: 'LearnerHome', path: '#/learner' },
{ name: 'Dashboard', path: '/' },
{ name: 'Learning', path: '/learning' },
{ name: 'People', path: '/people' },
{ name: 'Courses', path: '/courses' },
{ name: 'Enrolments', path: '/enrolments' },
{ name: 'Credentials', path: '/credentials' },
{ name: 'Compliance', path: '/compliance' },
{ name: 'CourseDetail', path: '/courses/test-id' },
{ name: 'LessonIndex', path: '/courses/test-id/lessons' },
{ name: 'LessonDetail', path: '/courses/test-id/lessons/test-lesson-id' },
{ name: 'LessonPlayer', path: '/courses/test-id/lessons/test-lesson-id/play' },
{ name: 'EnrolmentDetail', path: '/enrolments/test-id' },
{ name: 'BulkEnrol', path: '/enrolments/bulk' },
{ name: 'Regulations', path: '/compliance/regulations' },
{ name: 'RegulationDetail', path: '/compliance/regulations/test-slug' },
{ name: 'Attestations', path: '/compliance/attestations' },
{ name: 'AttestationDetail', path: '/compliance/attestations/test-id' },
{ name: 'AuditPackExport', path: '/compliance/export' },
{ name: 'CredentialDetail', path: '/credentials/test-id' },
{ name: 'CredentialVerify', path: '/credentials/test-id/verify' },
{ name: 'LearnerHome', path: '/learner' },
// @e2e data-exchange::data-exchange-page-remains-routable-via-deep-link
// Nav entry moved to Admin Settings (relocate-dataexchange-remove-assistant); pages stay routable.
{ name: 'DataExchangeJobs', path: '#/data-exchange/jobs' },
{ name: 'DataMappingProfiles', path: '#/data-exchange/mapping-profiles' },
{ name: 'DataExchangeJobs', path: '/data-exchange/jobs' },
{ name: 'DataMappingProfiles', path: '/data-exchange/mapping-profiles' },
]

test.describe('Learniq page routes', () => {
Expand Down Expand Up @@ -67,11 +79,31 @@ test.describe('Learniq page routes', () => {
}
})

await page.goto(`/index.php/apps/learniq/${path}`)
await page.goto(`${APP_BASE}${path}`)

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

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

// The page body must not be blank
const bodyText = await page.innerText('body')
expect(
Expand Down
Loading