Skip to content

Commit

Permalink
Fix error modal in user merge frontend
Browse files Browse the repository at this point in the history
As noted in #168, the issue is that things thrown asynchronously (e.g. in a callback) get handled in some other context, which doesn't appear to get picked up anywhere within Nuxt/Vue.

The main way this manifests is `withLoading`, which one has to wait on to have the error thrown within the app context, though it appears in other places (like the confirm dialog, as fixed here)

To sanity check this theory, I found another un-awaited `withLoading`, on RunAnalysis. I intentionally broke the endpoint and confirmed that no error dialog appears before the changes I made in this PR.

LMK if you want to comb the codebase for these, or if you'd like me to.

Fixes #168, in two cases
  • Loading branch information
bcspragu committed Jan 24, 2024
1 parent b4a5e27 commit dad5da2
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 27 deletions.
14 changes: 7 additions & 7 deletions frontend/components/analysis/RunButton.vue
Original file line number Diff line number Diff line change
Expand Up @@ -58,19 +58,19 @@ const request = computed<RunAnalysisReq>(() => {
}
})
const startRun = () => {
const startRun = async () => {
emit('started')
clicked.value = true
void withLoading(
() => pactaClient.runAnalysis(request.value)
.then((resp) => { analysisId.value = resp.analysisId })
.then(() => { void refreshAnalysisState() }),
const resp = await withLoading(
() => pactaClient.runAnalysis(request.value),
`${prefix}.runAnalysis`,
)
analysisId.value = resp.analysisId
void refreshAnalysisState()
}
const runAnalysis = () => {
const runAnalysis = async () => {
if (!props.warnForDuplicate) {
startRun()
await startRun()
return
}
confirm({
Expand Down
43 changes: 23 additions & 20 deletions frontend/pages/admin/merge.vue
Original file line number Diff line number Diff line change
Expand Up @@ -12,31 +12,34 @@ const fromUserId = useState<string>(`${prefix}.fromUserId`, () => '')
const toUserId = useState<string>(`${prefix}.toUserId`, () => '')
const done = useState<boolean>(`${prefix}.done`, () => false)
// TODO(#168) An example of the error here.
const doMerge = (): void => {
void withLoading(() => pactaClient.mergeUsers({
const doMerge = async () => {
await withLoading(() => pactaClient.mergeUsers({
fromUserId: fromUserId.value,
toUserId: toUserId.value,
}).then((resp) => {
done.value = true
}), `${prefix}.doMerge`)
done.value = true
}
const clickMerge = () => {
confirm({
header: 'Are you 100% sure?',
message: 'This will transfer all assets from the source user to the destination user, and then delete the source user. This cannot be undone. Only proceed if you have tripple checked the user IDs and are confident in this procedure.',
icon: 'pi pi-user-minus',
position: 'center',
blockScroll: true,
reject: () => { /* noop */ },
rejectLabel: 'Cancel',
rejectIcon: 'pi pi-times',
rejectClass: 'p-button-secondary p-button-text',
acceptClass: 'p-button-danger',
acceptLabel: 'Proceed',
accept: doMerge,
acceptIcon: 'pi pi-check',
const clickMerge = async () => {
await new Promise((resolve, reject) => {
confirm({
header: 'Are you 100% sure?',
message: 'This will transfer all assets from the source user to the destination user, and then delete the source user. This cannot be undone. Only proceed if you have tripple checked the user IDs and are confident in this procedure.',
icon: 'pi pi-user-minus',
position: 'center',
blockScroll: true,
reject: () => { /* noop */ },
rejectLabel: 'Cancel',
rejectIcon: 'pi pi-times',
rejectClass: 'p-button-secondary p-button-text',
acceptClass: 'p-button-danger',
acceptLabel: 'Proceed',
accept: () => {
doMerge().then(resolve).catch(reject)
},
acceptIcon: 'pi pi-check',
})
})
}
Expand Down

0 comments on commit dad5da2

Please sign in to comment.