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
SubmissionController bug fix #3118
Conversation
@@ -283,9 +283,9 @@ export class SubmissionController { | |||
) { | |||
delete declaration.data[attachmentSectionKey] | |||
} | |||
this.store.dispatch(writeDeclaration(declaration)) | |||
await this.store.dispatch(writeDeclaration(declaration)) |
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.
What was happening before adding these awaits? Did some changes get overwritten by some action creator? writeDeclaration
is quite widely used throughout the app. Do we need to await use await elsewhere too? If I recall right, action dispatching should be fire-and-forget, and waiting for the result is a bit of an anti-pattern
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.
yes, submissionStatus was getting overridden when there were multiple declarations.
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.
Where exactly did it get overwritten? I'm thinking if we could fix it so writeDeclaration
wouldn't overwrite the status of other declarations?
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.
Yes I agree. Instead of replacing all the declarations
in the successActionCreator
of WRITE_DECLARATION
, only replacing the declaration
we're writing should solve our issues.
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.
We debugged this with Yeasin and it turns out, it's workqueue auto-update that causes this issue. Basically, it can happen that while the offline queue is processing, the logic meant to keep workqueues up-to-date triggers, messing the local state of the applications. That logic is also currently built into SubmissionController
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.
I really want to get rid of the work-queue auto update in the next release. The user experience isnt great and if it is causing side effects like this, then it is another good reason to get rid
Codecov Report
@@ Coverage Diff @@
## develop #3118 +/- ##
===========================================
- Coverage 79.91% 79.88% -0.03%
===========================================
Files 598 602 +4
Lines 27690 27721 +31
Branches 8797 8815 +18
===========================================
+ Hits 22129 22146 +17
- Misses 5506 5520 +14
Partials 55 55
Continue to review full report at Codecov.
|
Test summaryRun details
View run in Cypress Dashboard ➡️ Flakiness
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
No description provided.