Skip to content

Commit 211f17d

Browse files
silverkszlosilverkszlo
authored
Merge pull request #1985 from nextcloud/1296-infinite-loading-when-visiting-non-existent-application
1296 infinite loading when visiting non existent application
2 parents 98961b4 + e9d736a commit 211f17d

8 files changed

Lines changed: 300 additions & 69 deletions

File tree

‎cypress/e2e/entity-not-found.cy.js‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/**
2+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
3+
* SPDX-License-Identifier: AGPL-3.0-or-later
4+
*/
5+
let localUser
6+
7+
describe('Entity not found error handling', () => {
8+
before(function() {
9+
cy.createRandomUser().then(user => {
10+
localUser = user
11+
cy.login(localUser)
12+
})
13+
})
14+
15+
beforeEach(function() {
16+
cy.login(localUser)
17+
18+
cy.intercept('GET', '**/tables/999', { statusCode: 404 }).as('getTable')
19+
cy.intercept('GET', '**/views/999', { statusCode: 404 }).as('getView')
20+
cy.intercept('GET', '**/contexts/999*', { statusCode: 404 }).as('getContext')
21+
22+
cy.visit('apps/tables')
23+
})
24+
25+
it('Shows error message when table is not found', () => {
26+
cy.visit('/apps/tables/#/table/999')
27+
28+
cy.get('.error-container', { timeout: 10000 })
29+
.should('contain.text', 'This table could not be found')
30+
})
31+
32+
it('Shows error message when view is not found', () => {
33+
cy.visit('/apps/tables/#/view/999')
34+
35+
cy.get('.error-container', { timeout: 10000 })
36+
.should('contain.text', 'This view could not be found')
37+
})
38+
39+
it('Shows error message when application is not found', () => {
40+
cy.visit('/apps/tables/#/application/999')
41+
42+
cy.get('.error-container', { timeout: 10000 })
43+
.should('contain.text', 'This application could not be found')
44+
})
45+
})

‎src/App.vue‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -88,15 +88,17 @@ export default {
8888
}
8989
if (currentRoute.path.startsWith('/table/')) {
9090
this.setActiveTableId(parseInt(currentRoute.params.tableId))
91-
this.setPageTitle(this.activeTable.title)
91+
const tableName = this.activeTable?.title || t('tables', 'Table')
92+
this.setPageTitle(tableName)
9293
if (!currentRoute.path.includes('/row/')) {
9394
const targetElement = document.querySelector(`header .header-start .app-menu a[href="${url}"]`)
9495
|| document.querySelector(`header .header-left .app-menu a[href="${url}"]`)
9596
this.switchActiveMenuEntry(targetElement)
9697
}
9798
} else if (currentRoute.path.startsWith('/view/')) {
9899
this.setActiveViewId(parseInt(currentRoute.params.viewId))
99-
this.setPageTitle(this.activeView.title)
100+
const viewName = this.activeView?.title || t('tables', 'View')
101+
this.setPageTitle(viewName)
100102
if (!currentRoute.path.includes('/row/')) {
101103
const targetElement = document.querySelector(`header .header-start .app-menu a[href="${url}"]`)
102104
|| document.querySelector(`header .header-left .app-menu a[href="${url}"]`)
@@ -105,11 +107,12 @@ export default {
105107
} else if (currentRoute.path.startsWith('/application/')) {
106108
const contextId = parseInt(currentRoute.params.contextId)
107109
this.setActiveContextId(contextId)
108-
this.setPageTitle(this.activeContext.name)
110+
const contextName = this.activeContext?.name || t('tables', 'Tables')
111+
this.setPageTitle(contextName)
109112
110113
// This breaks if there are multiple contexts with the same name or another app has the same name. We need a better way to identify the correct element.
111-
const targetElement = document.querySelector(`header .header-start .app-menu [title="${this.activeContext.name}"]`)
112-
|| document.querySelector(`header .header-left .app-menu [title="${this.activeContext.name}"]`)
114+
const targetElement = document.querySelector(`header .header-start .app-menu [title="${contextName}"]`)
115+
|| document.querySelector(`header .header-left .app-menu [title="${contextName}"]`)
113116
if (targetElement) {
114117
this.switchActiveMenuEntry(targetElement)
115118
}
@@ -127,8 +130,8 @@ export default {
127130
switchActiveMenuEntry(targetElement) {
128131
targetElement = targetElement?.tagName?.toLowerCase() === 'a' ? targetElement.parentElement : targetElement
129132
const currentlyActive = document.querySelector('header .header-start .app-menu li.app-menu-entry--active') || document.querySelector('header .header-left .app-menu li.app-menu-entry--active')
130-
currentlyActive.classList.remove('app-menu-entry--active')
131-
targetElement.classList.add('app-menu-entry--active')
133+
currentlyActive?.classList.remove('app-menu-entry--active')
134+
targetElement?.classList.add('app-menu-entry--active')
132135
},
133136
setPageTitle(title) {
134137
if (this.defaultPageTitle === false) {

‎src/modules/main/partials/ErrorMessage.vue‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
<!--
2+
- SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
3+
- SPDX-License-Identifier: AGPL-3.0-or-later
4+
-->
5+
6+
<template>
7+
<div class="error-container">
8+
<IconTables :size="64" style="margin-bottom: 1rem;" />
9+
<p>{{ message }}</p>
10+
</div>
11+
</template>
12+
13+
<script>
14+
import IconTables from '../../../shared/assets/icons/IconTables.vue'
15+
16+
export default {
17+
name: 'ErrorMessage',
18+
components: { IconTables },
19+
props: {
20+
message: {
21+
type: String,
22+
required: true,
23+
},
24+
},
25+
}
26+
</script>
27+
28+
<style lang="scss">
29+
.error-container {
30+
display: flex;
31+
flex-direction: column;
32+
align-items: center;
33+
justify-content: center;
34+
text-align: center;
35+
padding: 2rem;
36+
height: 100dvh;
37+
min-height: 100%;
38+
color: var(--color-text);
39+
opacity: 0.6;
40+
41+
p {
42+
font-size: clamp(1.2rem, 4vw, 2rem);
43+
font-weight: 600;
44+
max-width: 90%;
45+
word-wrap: break-word;
46+
}
47+
}
48+
</style>

‎src/pages/Context.vue‎

Lines changed: 72 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55
<template>
66
<div class="row">
77
<div v-if="loading" class="icon-loading" />
8-
<div v-if="!loading && context">
8+
9+
<div v-else-if="activeContext">
910
<div class="content context">
1011
<div class="row first-row">
1112
<h1 class="context__title" data-cy="context-title">
@@ -32,9 +33,11 @@
3233
</div>
3334
</div>
3435
</div>
35-
36-
<MainModals />
3736
</div>
37+
38+
<ErrorMessage v-else-if="errorMessage" :message="errorMessage" />
39+
40+
<MainModals />
3841
</div>
3942
</template>
4043

@@ -50,11 +53,14 @@ import exportTableMixin from '../shared/components/ncTable/mixins/exportTableMix
5053
import svgHelper from '../shared/components/ncIconPicker/mixins/svgHelper.js'
5154
import { useTablesStore } from '../store/store.js'
5255
import { useDataStore } from '../store/data.js'
56+
import ErrorMessage from '../modules/main/partials/ErrorMessage.vue'
57+
import displayError, { getNotFoundError, getGenericLoadError } from '../shared/utils/displayError.js'
5358
5459
export default {
5560
components: {
5661
MainModals,
5762
NcIconSvgWrapper,
63+
ErrorMessage,
5864
TableWrapper,
5965
CustomView,
6066
},
@@ -73,6 +79,7 @@ export default {
7379
viewSetting: {},
7480
context: null,
7581
contextResources: [],
82+
errorMessage: null,
7683
}
7784
},
7885
@@ -114,14 +121,12 @@ export default {
114121
},
115122
116123
watch: {
117-
// Watch for changes to active context to make page reactive
118124
async activeContext() {
119-
if (this.activeContextId && !this.activeContext) {
120-
// context does not exists, go to startpage
121-
this.$router.push('/').catch(err => err)
122-
} else {
123-
await this.reload()
125+
if (this.errorMessage) {
126+
// Already showing an error, don't redirect
127+
return
124128
}
129+
await this.reload()
125130
},
126131
'context.iconName': {
127132
async handler(value) {
@@ -146,49 +151,71 @@ export default {
146151
return
147152
}
148153
this.loading = true
149-
this.icon = await this.getContextIcon(this.activeContext.iconName)
150154
this.contextResources = []
151-
await this.loadContext({ id: this.activeContextId })
152-
const index = this.contexts.findIndex(c => parseInt(c.id) === parseInt(this.activeContextId))
153-
this.context = this.contexts[index]
154155
155-
if (this.context && this.context.nodes) {
156-
for (const [, node] of Object.entries(this.context.nodes)) {
157-
const nodeType = parseInt(node.node_type)
158-
if (nodeType === NODE_TYPE_TABLE) {
159-
const table = this.tables.find(table => table.id === node.node_id)
160-
if (table) {
161-
await this.loadColumnsFromBE({
162-
view: null,
163-
tableId: table.id,
164-
})
165-
await this.loadRowsFromBE({
166-
viewId: null,
167-
tableId: table.id,
168-
})
169-
table.key = (table.id).toString()
170-
table.isView = false
171-
this.contextResources.push(table)
172-
}
156+
try {
157+
await this.loadContext({ id: this.activeContextId })
158+
const index = this.contexts.findIndex(c => parseInt(c.id) === parseInt(this.activeContextId))
159+
this.context = this.contexts[index]
160+
161+
if (!this.context) {
162+
this.errorMessage = t('tables', 'This application could not be found')
163+
return
164+
}
173165
174-
} else if (nodeType === NODE_TYPE_VIEW) {
175-
const view = this.views.find(view => view.id === node.node_id)
176-
if (view) {
177-
await this.loadColumnsFromBE({
178-
view,
179-
})
180-
await this.loadRowsFromBE({
181-
viewId: view.id,
182-
tableId: view.tableId,
183-
})
184-
view.key = 'view-' + (view.id).toString()
185-
view.isView = true
186-
this.contextResources.push(view)
166+
this.icon = await this.getContextIcon(this.activeContext.iconName)
167+
168+
if (this.context && this.context.nodes) {
169+
for (const [, node] of Object.entries(this.context.nodes)) {
170+
try {
171+
const nodeType = parseInt(node.node_type)
172+
if (nodeType === NODE_TYPE_TABLE) {
173+
const table = this.tables.find(table => table.id === node.node_id)
174+
if (table) {
175+
await this.loadColumnsFromBE({
176+
view: null,
177+
tableId: table.id,
178+
})
179+
await this.loadRowsFromBE({
180+
viewId: null,
181+
tableId: table.id,
182+
})
183+
table.key = (table.id).toString()
184+
table.isView = false
185+
this.contextResources.push(table)
186+
}
187+
188+
} else if (nodeType === NODE_TYPE_VIEW) {
189+
const view = this.views.find(view => view.id === node.node_id)
190+
if (view) {
191+
await this.loadColumnsFromBE({
192+
view,
193+
})
194+
await this.loadRowsFromBE({
195+
viewId: view.id,
196+
tableId: view.tableId,
197+
})
198+
view.key = 'view-' + (view.id).toString()
199+
view.isView = true
200+
this.contextResources.push(view)
201+
}
202+
}
203+
} catch (err) {
204+
console.error(`Failed to load resource ${node.node_id}:`, err)
205+
this.errorMessage = t('tables', 'Some resources in this application could not be loaded')
187206
}
188207
}
189208
}
209+
} catch (e) {
210+
if (e.message === 'NOT_FOUND') {
211+
this.errorMessage = getNotFoundError('application')
212+
} else {
213+
this.errorMessage = getGenericLoadError('application')
214+
displayError(e, this.errorMessage)
215+
}
216+
} finally {
217+
this.loading = false
190218
}
191-
this.loading = false
192219
},
193220
createColumn(isView, element) {
194221
emit('tables:column:create', { isView, element })

0 commit comments

Comments
 (0)