From 8e9d37d404b94f59086a99b7701159d92887a956 Mon Sep 17 00:00:00 2001 From: Christian Hartmann Date: Wed, 17 Jun 2026 23:27:09 +0200 Subject: [PATCH 1/2] feat: implement localStorage management for results view Signed-off-by: Christian Hartmann --- src/Forms.vue | 63 +++++++++++++++++++++++++++++++++++++++++-- src/views/Results.vue | 49 +++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 2 deletions(-) diff --git a/src/Forms.vue b/src/Forms.vue index 457d44b1a..61be16a46 100644 --- a/src/Forms.vue +++ b/src/Forms.vue @@ -351,6 +351,53 @@ export default { loading.value = false } + /** + * Clean up stale localStorage entries for forms that are no longer available. + * Removes localStorage keys matching the pattern `nextcloud_forms_*_activeResponseView` + * where the form hash no longer exists in the current forms list. + */ + const cleanupStaleLocalStorageEntries = () => { + try { + // Get all current form hashes + const currentFormHashes = new Set( + [...forms.value, ...allSharedForms.value].map( + (form) => form.hash, + ), + ) + + // Iterate through all localStorage keys + const keysToRemove = [] + for (let i = 0; i < localStorage.length; i++) { + const key = localStorage.key(i) + if ( + key + && key.startsWith('nextcloud_forms_') + && key.endsWith('_activeResponseView') + ) { + // Extract hash from key: nextcloud_forms__activeResponseView + const hash = key.substring( + 'nextcloud_forms_'.length, + key.length - '_activeResponseView'.length, + ) + // If form hash is not in current forms, mark for removal + if (!currentFormHashes.has(hash)) { + keysToRemove.push(key) + } + } + } + + // Remove stale entries + keysToRemove.forEach((key) => { + localStorage.removeItem(key) + logger.debug(`Removed stale localStorage entry: ${key}`) + }) + } catch (err) { + logger.debug('Error cleaning up stale localStorage entries', { + error: err, + }) + } + } + /** * Fetch a partial form by its hash after initial load completes. * @@ -447,6 +494,17 @@ export default { forms.value.splice(formIndex, 1) deletedFormHash.value = deletedHash + // Remove localStorage entry for this form's active response view + try { + localStorage.removeItem( + `nextcloud_forms_${deletedHash}_activeResponseView`, + ) + } catch (err) { + logger.debug('Error removing localStorage entry for deleted form', { + error: err, + }) + } + if (deletedHash === routeHash.value && route.name !== 'root') { // Navigate to root without triggering route guards router.replace({ name: 'root' }) @@ -477,8 +535,9 @@ export default { } } - onMounted(() => { - loadForms() + onMounted(async () => { + await loadForms() + cleanupStaleLocalStorageEntries() subscribe('forms:last-updated:set', onLastUpdatedByEventBus) subscribe('forms:ownership-transfered', onDeleteForm) }) diff --git a/src/views/Results.vue b/src/views/Results.vue index cfc61775a..f27f16210 100644 --- a/src/views/Results.vue +++ b/src/views/Results.vue @@ -498,6 +498,7 @@ export default { // Reload results when form changes async hash() { await this.fetchFullForm(this.form.id) + this.loadActiveResponseViewFromLocalStorage() this.loadFormResults() SetWindowTitle(this.formTitle) }, @@ -521,15 +522,63 @@ export default { }) this.loadFormResults() }, INPUT_DEBOUNCE_MS), + + // Persist active response view to localStorage when it changes + activeResponseView(newView) { + this.saveActiveResponseViewToLocalStorage(newView.id) + }, }, async beforeMount() { await this.fetchFullForm(this.form.id) + this.loadActiveResponseViewFromLocalStorage() this.loadFormResults() SetWindowTitle(this.formTitle) }, methods: { + /** + * Load the active response view preference from localStorage for the current form. + * Applies stored value if available and otherwise resets to default (summary) + */ + loadActiveResponseViewFromLocalStorage() { + try { + const storedViewId = localStorage.getItem( + `nextcloud_forms_${this.form.hash}_activeResponseView`, + ) + if (storedViewId) { + const view = responseViews.find((v) => v.id === storedViewId) + if (view) { + this.activeResponseView = view + } + } else { + this.activeResponseView = responseViews[0] + } + } catch (err) { + logger.debug('Error loading activeResponseView from localStorage', { + error: err, + }) + } + }, + + /** + * Save the active response view preference to localStorage for the current form. + * + * @param {string} viewId - The ID of the view ('summary' or 'responses') + */ + saveActiveResponseViewToLocalStorage(viewId) { + try { + localStorage.setItem( + `nextcloud_forms_${this.form.hash}_activeResponseView`, + viewId, + ) + } catch (err) { + logger.debug('Error saving activeResponseView to localStorage', { + error: err, + }) + } + }, + async onUnlinkFile() { await axios.patch( generateOcsUrl('apps/forms/api/v3/forms/{formId}', { From 676191192415291b0c8562f3f8e50c94bfe98c5f Mon Sep 17 00:00:00 2001 From: Christian Hartmann Date: Tue, 23 Jun 2026 16:33:43 +0200 Subject: [PATCH 2/2] feat: reflect response view in URL Signed-off-by: Christian Hartmann --- playwright/e2e/results-view.spec.ts | 60 +++++++++- src/views/Results.vue | 174 ++++++++++++++++++++++++---- 2 files changed, 208 insertions(+), 26 deletions(-) diff --git a/playwright/e2e/results-view.spec.ts b/playwright/e2e/results-view.spec.ts index e4a22f523..ec6e5c086 100644 --- a/playwright/e2e/results-view.spec.ts +++ b/playwright/e2e/results-view.spec.ts @@ -55,7 +55,7 @@ test.describe('Results view', () => { // from submit → results after submission causes a brief redirect loop, // so we use direct navigation instead of clicking the TopBar. await page.goto(page.url().replace(/\/submit.*$/, '/results')) - await page.waitForURL(/\/results$/) + await page.waitForURL(/\/results(?:\?.*)?$/) }) test('Summary tab shows submitted data', async ({ resultsView }) => { @@ -80,20 +80,76 @@ test.describe('Results view', () => { // Should show the individual submission with the answers await expect(resultsView.responsesTab).toBeChecked() await expect(resultsView.responseCount).toBeVisible() + await expect(resultsView.page).toHaveURL(/\/results\?responses$/) }) - test('Tab switching between Summary and Responses', async ({ resultsView }) => { + test('Tab switching between Summary and Responses updates the URL', async ({ + resultsView, + }) => { // Start on Summary await expect(resultsView.summaryTab).toBeChecked() + await expect(resultsView.page).toHaveURL(/\/results\?summary$/) // Switch to Responses await resultsView.switchToResponses() await expect(resultsView.responsesTab).toBeChecked() await expect(resultsView.summaryTab).not.toBeChecked() + await expect(resultsView.page).toHaveURL(/\/results\?responses$/) // Switch back to Summary await resultsView.switchToSummary() await expect(resultsView.summaryTab).toBeChecked() await expect(resultsView.responsesTab).not.toBeChecked() + await expect(resultsView.page).toHaveURL(/\/results\?summary$/) + }) + + test('Explicit query route wins over remembered localStorage view', async ({ + page, + resultsView, + }) => { + await page.evaluate(() => { + const match = window.location.pathname.match( + /\/apps\/forms\/([^/]+)\/results$/, + ) + if (!match) { + throw new Error('Expected results route before setting localStorage') + } + + localStorage.setItem( + `nextcloud_forms_${match[1]}_activeResponseView`, + 'responses', + ) + }) + + await page.goto(page.url().replace(/\/results$/, '/results?summary')) + await page.waitForURL(/\/results\?summary$/) + + await expect(resultsView.summaryTab).toBeChecked() + await expect(resultsView.responsesTab).not.toBeChecked() + }) + + test('Query-less results route restores the remembered localStorage view', async ({ + page, + resultsView, + }) => { + await page.evaluate(() => { + const match = window.location.pathname.match( + /\/apps\/forms\/([^/]+)\/results$/, + ) + if (!match) { + throw new Error('Expected results route before setting localStorage') + } + + localStorage.setItem( + `nextcloud_forms_${match[1]}_activeResponseView`, + 'responses', + ) + }) + + await page.goto(page.url().replace(/\/results\?summary$/, '/results')) + await page.waitForURL(/\/results\?responses$/) + + await expect(resultsView.responsesTab).toBeChecked() + await expect(resultsView.summaryTab).not.toBeChecked() }) }) diff --git a/src/views/Results.vue b/src/views/Results.vue index f27f16210..856c54333 100644 --- a/src/views/Results.vue +++ b/src/views/Results.vue @@ -47,7 +47,7 @@ :options="responseViews" :groupLabel="t('forms', 'View mode')" class="response-actions__toggle" - @update:active="loadFormResults" /> + @update:active="onChangeResponseView" /> view.id)) export default { // eslint-disable-next-line vue/multi-word-component-names @@ -379,7 +380,7 @@ export default { data() { return { - activeResponseView: responseViews[0], + activeResponseView: {}, questions: [], submissions: [], @@ -498,11 +499,16 @@ export default { // Reload results when form changes async hash() { await this.fetchFullForm(this.form.id) - this.loadActiveResponseViewFromLocalStorage() - this.loadFormResults() + await this.syncActiveResponseViewFromRoute() SetWindowTitle(this.formTitle) }, + '$route.query': { + handler() { + this.syncActiveResponseViewFromRoute() + }, + }, + limit() { this.loadFormResults() }, @@ -525,40 +531,115 @@ export default { // Persist active response view to localStorage when it changes activeResponseView(newView) { - this.saveActiveResponseViewToLocalStorage(newView.id) + if (newView?.id) { + this.saveActiveResponseViewToLocalStorage(newView.id) + } }, }, async beforeMount() { await this.fetchFullForm(this.form.id) - this.loadActiveResponseViewFromLocalStorage() - this.loadFormResults() + await this.syncActiveResponseViewFromRoute() SetWindowTitle(this.formTitle) }, methods: { /** - * Load the active response view preference from localStorage for the current form. - * Applies stored value if available and otherwise resets to default (summary) + * Resolve a response view object by its ID. + * + * @param {string} viewId The requested response view ID + * @return {object} + */ + getResponseViewById(viewId) { + return ( + responseViews.find((view) => view.id === viewId) ?? responseViews[0] + ) + }, + + /** + * Read the explicit response view from the current route query. + * + * @return {string|null} + */ + getRouteResponseViewId() { + const matchingView = responseViews.find((view) => { + return Object.hasOwn(this.$route.query, view.id) + }) + + return matchingView?.id ?? null + }, + + /** + * Load the stored response view preference from localStorage for the current form. + * + * @return {string} */ - loadActiveResponseViewFromLocalStorage() { + loadStoredActiveResponseViewId() { try { - const storedViewId = localStorage.getItem( - `nextcloud_forms_${this.form.hash}_activeResponseView`, - ) - if (storedViewId) { - const view = responseViews.find((v) => v.id === storedViewId) - if (view) { - this.activeResponseView = view - } - } else { - this.activeResponseView = responseViews[0] + const storageKey = this.getActiveResponseViewStorageKey() + if (!storageKey) { + return responseViews[0].id + } + + const storedViewId = localStorage.getItem(storageKey) + if (storedViewId && responseViewIds.has(storedViewId)) { + return storedViewId } + + return responseViews[0].id } catch (err) { logger.debug('Error loading activeResponseView from localStorage', { error: err, }) + return responseViews[0].id + } + }, + + /** + * Resolve the effective response view using route state first and localStorage second. + * + * @return {string} + */ + resolveActiveResponseViewId() { + return ( + this.getRouteResponseViewId() + ?? this.loadStoredActiveResponseViewId() + ) + }, + + /** + * Apply the effective route/localStorage view and refresh results when needed. + */ + async syncActiveResponseViewFromRoute() { + const routeViewId = this.getRouteResponseViewId() + const nextView = this.getResponseViewById( + routeViewId ?? this.loadStoredActiveResponseViewId(), + ) + const currentViewId = this.activeResponseView?.id + + if (currentViewId !== nextView.id) { + this.activeResponseView = nextView + } + + if (!routeViewId) { + try { + await this.$router.replace({ + name: 'results', + params: { + hash: this.form.hash, + }, + query: { + ...this.$route.query, + [nextView.id]: null, + }, + }) + return + } catch (error) { + logger.debug('Navigation cancelled', { error }) + } } + + this.loadFormResults() }, /** @@ -568,10 +649,12 @@ export default { */ saveActiveResponseViewToLocalStorage(viewId) { try { - localStorage.setItem( - `nextcloud_forms_${this.form.hash}_activeResponseView`, - viewId, - ) + const storageKey = this.getActiveResponseViewStorageKey() + if (!storageKey) { + return + } + + localStorage.setItem(storageKey, viewId) } catch (err) { logger.debug('Error saving activeResponseView to localStorage', { error: err, @@ -579,6 +662,49 @@ export default { } }, + /** + * Build the localStorage key for the active response view. + * + * @return {string|null} + */ + getActiveResponseViewStorageKey() { + const formHash = this.form?.hash + if (!formHash) { + return null + } + + return `nextcloud_forms_${formHash}_activeResponseView` + }, + + /** + * Navigate to an explicit route query for the selected response view. + * + * @param {object} view The selected response view object + */ + async onChangeResponseView(view) { + if (!view?.id) { + return + } + if (this.getRouteResponseViewId() === view.id) { + this.loadFormResults() + return + } + + try { + await this.$router.push({ + name: 'results', + params: { + hash: this.form.hash, + }, + query: { + [view.id]: null, + }, + }) + } catch (error) { + logger.debug('Navigation cancelled', { error }) + } + }, + async onUnlinkFile() { await axios.patch( generateOcsUrl('apps/forms/api/v3/forms/{formId}', {