-
Notifications
You must be signed in to change notification settings - Fork 49
31672 Fixed redirect during save error dialog and blank page when OK clicked #665
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.
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
Conversation
- added updateErrorDialog property to end overloaded saveErrorDialog - do not hide app "main" component when there is a save error (to prevent blank page / router not refreshing) - deleted unused imports, etc in View Wrapper - check for save success before navigating to dashboard - return boolean to indicate save success or failure
| :warnings="saveWarnings" | ||
| @exit="goToDashboard()" | ||
| @okay="saveErrorDialog = false" | ||
| @okay="saveErrorDialog = false; updateErrorDialog = false" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This dialog is displayed for 2 separate error types.
| this.fetchErrorDialog || | ||
| this.paymentErrorDialog || | ||
| this.saveErrorDialog || | ||
| this.updateErrorDialog || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See line 114.
Do hide the main body if there is an update error (same as before).
Do not hide the main body if there is a save error, as that blanks out the body, which isn't re-rendered when the dialog is closed, and the user is left with a blank page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes issues with error dialog handling and navigation after save errors. The key improvement is differentiating between save errors and update errors by introducing a separate updateErrorDialog property, which prevents the application from showing a blank page when certain errors occur.
- Adds
updateErrorDialogto handle update-specific errors separately from save errors - Modifies
onClickSaveto return a boolean indicating save success/failure - Checks save success before navigating to dashboard in "Save & Resume Later" workflow
- Removes unused imports and commented-out code in ViewWrapper.vue
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/components/ViewWrapper.vue | Updated onClickSave to return boolean success indicator; added conditional navigation based on save result; removed unused imports and properties |
| src/App.vue | Added updateErrorDialog property; modified dialog visibility logic to keep main component visible during save errors; updated event handlers |
| package.json | Version bumped to 4.14.10 |
| package-lock.json | Version synchronized to 4.14.10 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { FilingDataIF, ConfirmDialogType, FlagsReviewCertifyIF, FlagsCompanyInfoIF, | ||
| AlterationFilingIF, ChgRegistrationFilingIF, ConversionFilingIF, RestorationFilingIF, | ||
| SpecialResolutionFilingIF } from '@/interfaces/' | ||
| import { SessionStorageKeys } from 'sbc-common-components/src/util/constants' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a lot of stuff in here that wasn't used. (It must have been copied over when this component was created.) So I deleted it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lot of clean up!
| } else { | ||
| const error = new Error('Missing Payment Token or Filing ID') | ||
| this.$root.$emit('save-error-event', error) | ||
| this.setIsSaving(false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not having this was actually a bug -- the isSaving state was left as True if this error occurred, leaving the user unable to client any buttons.
… as the code will fall through correctly (but Copilot complained)
|
/gcbrun |
|
Temporary Url for review: https://business-edit-dev--pr-665-x2dgqt4x.web.app SB says, try changing and saving this Special Resolution. There's already a draft in play so your new draft will get a save error. |
ketaki-deodhar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
meawong
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM

Issue #: bcgov/entity#31672
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the bcrs-entities-create-ui license (Apache 2.0).