Skip to content

Fix leaving conversation when fetching the conversation fails #4338

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

Closed
danxuliu wants to merge 3 commits into from
Closed

Fix leaving conversation when fetching the conversation fails #4338

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2edb3e0
Reset the page title and the token when redirecting to "not-found"
danxuliu Oct 9, 2020
91f0f9f
Do not redirect to "not-found" if fetching the conversation failed
danxuliu Oct 9, 2020
665d09c
Show an error notification if fetching the conversation failed
danxuliu Oct 9, 2020
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: 26 additions & 3 deletions src/App.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ import browserCheck from './mixins/browserCheck'
import duplicateSessionHandler from './mixins/duplicateSessionHandler'
import isInCall from './mixins/isInCall'
import talkHashCheck from './mixins/talkHashCheck'
import { showError } from '@nextcloud/dialogs'
import { generateUrl } from '@nextcloud/router'
import UploadEditor from './components/UploadEditor'
import SettingsDialog from './components/SettingsDialog/SettingsDialog'
Expand Down Expand Up @@ -92,6 +93,7 @@ export default {
defaultPageTitle: false,
loading: false,
isRefreshingCurrentConversation: false,
errorUpdatingConversationToast: null,
Copy link
Member

@PVince81 PVince81 Oct 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional: s/Toast/Message to not make this name implementation dependent ?

}
},

Expand Down Expand Up @@ -286,7 +288,11 @@ export default {
this.setPageTitle(nextConversationName)
// Update current token in the token store
this.$store.dispatch('updateToken', to.params.token)
} else if (to.name === 'notfound') {
this.setPageTitle('')
this.$store.dispatch('updateToken', '')
Copy link
Member

@PVince81 PVince81 Oct 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as someone not knowing what effect this code will have, would be nice to add a comment :-)

Copy link
Member

@nickvergessen nickvergessen Oct 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updating the token should automatically change the pagetitle?

Copy link
Member

@PVince81 PVince81 Nov 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somehow I think we should still keep the current URL, page title and token in place and only render a "not found" content. The switching will be done anyway by the user.

Copy link
Member

@nickvergessen nickvergessen Nov 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't work reliably with guests. They would re initialise the conversation immediately and continue chatting without being annoyed for a second, until we can really block people.

}

/**
* Fires a global event that tells the whole app that the route has changed. The event
* carries the from and to objects as payload
Expand Down Expand Up @@ -429,6 +435,11 @@ export default {
*/
const response = await fetchConversation(token)

if (this.errorUpdatingConversationToast) {
this.errorUpdatingConversationToast.hideToast()
this.errorUpdatingConversationToast = null
}

// this.$store.dispatch('purgeConversationsStore')
this.$store.dispatch('addConversation', response.data.ocs.data)
this.$store.dispatch('markConversationRead', token)
Expand All @@ -441,9 +452,21 @@ export default {
singleConversation: true,
})
} catch (exception) {
console.info('Conversation received, but the current conversation is not in the list. Redirecting to /apps/spreed')
this.$router.push('/apps/spreed/not-found')
this.$store.dispatch('hideSidebar')
if (exception.response && exception.response.status === 404) {
console.info('Conversation received, but the current conversation is not in the list. Redirecting to /apps/spreed')
this.$router.push('/apps/spreed/not-found')
this.$store.dispatch('hideSidebar')

return
}

// TODO Maybe progressively increase debouncing time to avoid
// hammering the server if it is already loaded?
this.debounceRefreshCurrentConversation()

if (!this.errorUpdatingConversationToast) {
this.errorUpdatingConversationToast = showError(t('spreed', 'An error occurred while updating the conversation'))
}
} finally {
this.isRefreshingCurrentConversation = false
}
Expand Down