From c3721db3634e2721fc56698276d268baec2d47c6 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Wed, 4 Nov 2026 13:30:42 +0100 Subject: [PATCH 1/2] fix(store): guard findIndex(-1) before indexing in store actions Several store actions dereferenced or spliced a findIndex() result without guarding the -1 case: - favoriteView/removeFavoriteView/favoriteTable/removeFavoriteTable threw a TypeError at this.X[-1].favorite when toggling a favorite on a node absent from the local store. - removeView/removeTable/removeContext and the transfer actions called splice(index, 1) with index -1, which removes the LAST element instead of the intended (absent) one. - updateView/reloadViewsOfTable assigned to arr[-1], silently creating a bogus "-1" property instead of updating the view. - setTable spliced at -1, replacing the LAST table with the one it was asked to set; setView assigned to views[-1]. Both are reached by every caller, not only the favorite actions guarded above. - updateTable and updateContext assigned to tables[-1] and contexts[-1]. Return early (favorites) or skip the write, so a server success never corrupts the local store. An absent id means "nothing to update" rather than "insert": on a mutation echo it is missing because the node was removed while the request was in flight, and adding it back would resurrect it as a navigation entry that 404s on click. setContext is the exception and keeps its insert, which exists so that loading one application by its URL before the list is there stays visible, so updateContext guards at the call site instead of routing through it. updateView and reloadViewsOfTable now delegate to setView rather than repeating its body, and setTable assigns by index instead of splicing, as AGENTS.md requires for Vue 3. Assisted-by: ClaudeCode:claude-opus-4-8 Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger --- cypress/component/store/tablesStore.cy.js | 47 +++++++++++++++++ src/store/store.js | 63 ++++++++++++++--------- 2 files changed, 87 insertions(+), 23 deletions(-) create mode 100644 cypress/component/store/tablesStore.cy.js diff --git a/cypress/component/store/tablesStore.cy.js b/cypress/component/store/tablesStore.cy.js new file mode 100644 index 0000000000..024e49cd72 --- /dev/null +++ b/cypress/component/store/tablesStore.cy.js @@ -0,0 +1,47 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +import { createPinia, setActivePinia } from 'pinia' +import { useTablesStore } from '../../../src/store/store.js' + +const hasStrayIndex = collection => Object.hasOwn(collection, '-1') + +describe('tables store setters', () => { + beforeEach(() => { + setActivePinia(createPinia()) + }) + + it('setTable replaces in place and leaves an unlisted table out', () => { + const store = useTablesStore() + store.setTables([{ id: 1, title: 'A' }, { id: 3, title: 'C' }]) + + store.setTable({ id: 3, title: 'C renamed' }) + expect(store.tables.map(table => table.title)).to.deep.equal(['A', 'C renamed']) + + store.setTable({ id: 2, title: 'removed in the meantime' }) + expect(store.tables.map(table => table.id)).to.deep.equal([1, 3]) + expect(hasStrayIndex(store.tables)).to.equal(false) + }) + + it('setView replaces in place and leaves an unlisted view out', () => { + const store = useTablesStore() + store.setViews([{ id: 1, title: 'A' }, { id: 3, title: 'C' }]) + + store.setView({ id: 3, title: 'C renamed' }) + expect(store.views.map(view => view.title)).to.deep.equal(['A', 'C renamed']) + + store.setView({ id: 2, title: 'removed in the meantime' }) + expect(store.views.map(view => view.id)).to.deep.equal([1, 3]) + expect(hasStrayIndex(store.views)).to.equal(false) + }) + + it('setContext appends an unlisted application so a load by URL stays visible', () => { + const store = useTablesStore() + store.setContexts([{ id: 1, name: 'A' }]) + + store.setContext({ id: 2, name: 'B' }) + expect(store.contexts.map(context => context.id)).to.deep.equal([1, 2]) + expect(hasStrayIndex(store.contexts)).to.equal(false) + }) +}) diff --git a/src/store/store.js b/src/store/store.js index ce22251a6c..e40add7ca3 100644 --- a/src/store/store.js +++ b/src/store/store.js @@ -117,11 +117,15 @@ export const useTablesStore = defineStore('store', { }, setTable(table) { const index = this.tables.findIndex(t => t.id === table.id) - this.tables.splice(index, 1, table) + if (index !== -1) { + this.tables[index] = table + } }, setView(view) { const index = this.views.findIndex(v => v.id === view.id) - this.views[index] = view + if (index !== -1) { + this.views[index] = view + } }, setContext(context) { const index = this.contexts.findIndex(c => c.id === context.id) @@ -242,11 +246,8 @@ export const useTablesStore = defineStore('store', { return false } - const view = res.data - const views = this.views - const index = views.findIndex(v => v.id === view.id) - views[index] = view - this.setViews([...views]) + this.setView(res.data) + this.setViews([...this.views]) return true }, @@ -260,7 +261,9 @@ export const useTablesStore = defineStore('store', { const views = this.views const index = views.findIndex(v => v.id === viewId) - views.splice(index, 1) + if (index !== -1) { + views.splice(index, 1) + } this.setViews([...views]) return true }, @@ -269,12 +272,7 @@ export const useTablesStore = defineStore('store', { let res = null try { res = await axios.get(generateUrl('/apps/tables/view/table/' + tableId)) - // Set Views - const views = this.views - res.data.forEach(view => { - const index = views.findIndex(v => v.id === view.id) - this.views[index] = view - }) + res.data.forEach(view => this.setView(view)) } catch (e) { displayError(e, t('tables', 'Could not reload view.')) return false @@ -309,10 +307,7 @@ export const useTablesStore = defineStore('store', { return false } - const table = res.data - const tables = this.tables - const index = tables.findIndex(t => t.id === table.id) - this.tables[index] = table + this.setTable(res.data) return true }, @@ -325,6 +320,9 @@ export const useTablesStore = defineStore('store', { } const index = this.views.findIndex(v => v.id === id) + if (index === -1) { + return true + } const view = this.views[index] view.favorite = true this.setView(view) @@ -341,6 +339,9 @@ export const useTablesStore = defineStore('store', { } const index = this.views.findIndex(v => v.id === id) + if (index === -1) { + return true + } const view = this.views[index] view.favorite = false this.setView(view) @@ -357,6 +358,9 @@ export const useTablesStore = defineStore('store', { } const index = this.tables.findIndex(t => t.id === id) + if (index === -1) { + return true + } const table = this.tables[index] table.favorite = true this.setTable(table) @@ -373,6 +377,9 @@ export const useTablesStore = defineStore('store', { } const index = this.tables.findIndex(t => t.id === id) + if (index === -1) { + return true + } const table = this.tables[index] table.favorite = false this.setTable(table) @@ -467,7 +474,9 @@ export const useTablesStore = defineStore('store', { const context = res.data.ocs.data const index = this.contexts.findIndex(c => c.id === context.id) - this.contexts[index] = context + if (index !== -1) { + this.contexts[index] = context + } return true }, @@ -482,7 +491,9 @@ export const useTablesStore = defineStore('store', { const tables = this.tables const index = tables.findIndex(t => t.id === id) - tables.splice(index, 1) + if (index !== -1) { + tables.splice(index, 1) + } this.setTables([...tables]) return true }, @@ -631,7 +642,9 @@ export const useTablesStore = defineStore('store', { const contexts = this.contexts const index = contexts.findIndex(c => c.id === id) - contexts.splice(index, 1) + if (index !== -1) { + contexts.splice(index, 1) + } this.setContexts([...contexts]) return true }, @@ -645,7 +658,9 @@ export const useTablesStore = defineStore('store', { } const contexts = this.contexts const index = contexts.findIndex(c => c.id === context.id) - contexts.splice(index, 1) + if (index !== -1) { + contexts.splice(index, 1) + } this.setContexts([...contexts]) return true }, @@ -660,7 +675,9 @@ export const useTablesStore = defineStore('store', { const tables = this.tables const index = tables.findIndex(t => t.id === tableId) - tables.splice(index, 1) + if (index !== -1) { + tables.splice(index, 1) + } this.setTables([...tables]) return true }, From e8a82ca8283ae08cb163cab65d555ea650d562e1 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Fri, 6 Nov 2026 00:23:39 +0100 Subject: [PATCH 2/2] fix(store): do not throw in insertNewView when the table is unknown insertNewView looks the parent table up in this.tables and pushes the new view into it. The lookup is outside the try block, so when the table is not in the local store the push throws a TypeError and the action never returns the new view id. The table is absent whenever the view was created on a table the user only reaches through a view share: findAll returns the tables the user owns or has a table share on, so a directly shared view has no table entry. Cloning such a view then reports "Could not create new view" even though the server created it, leaving an orphan behind. Skip the push when the table is unknown; the view is already in this.views, which is what the navigation renders for shared views. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger --- src/store/store.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/store/store.js b/src/store/store.js index e40add7ca3..eea587f898 100644 --- a/src/store/store.js +++ b/src/store/store.js @@ -231,7 +231,7 @@ export const useTablesStore = defineStore('store', { views.push(res.data) this.setViews(views) const table = this.tables.find(t => t.id === res.data.tableId) - table.views.push(res.data) + table?.views.push(res.data) return res.data.id },