-
Notifications
You must be signed in to change notification settings - Fork 2
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
[NM-66] Hot fix for state being rewritten on a project #1025
Conversation
@@ -332,7 +332,7 @@ export const actions: ActionTree<SurveyAndProgramState, RootState> & SurveyAndPr | |||
async setSurveyResponse(context: ActionContext<SurveyAndProgramState, RootState>, response: SurveyResponse) { | |||
const {commit, rootState} = context; | |||
const shapeData = rootState.baseline.shape; | |||
if (shapeData) { | |||
if (shapeData?.data) { |
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 was also occasionally getting errors from shapeData.data.features being null in the call to getDataWithAreaLevel. So making this check cover that case too.
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 is definitely the correct place to target this bug, i managed to recreate it however theres a bigger issue with all this save state into local storage stuff, local storage is shared between tabs so your goal of making saving state atomic means that you must block saving state at any point theres a change, not just when a project is loading, but also when it decided to save state every 2 seconds for example, so one thing i was able to do after a couple attempts is:
- create a DRC project, stop at review inputs
- create a MWI project, go to output plots
- load DRC and MWI on two separate tabs
- wait for state saving of MWI (2 sec interval thing)
- reload DRC (this is a bit flaky) and you can get into a state that DRC is actually on the output plots step because somewhere we are saving a partial state with just the stepper state i am guessing
like shown here, note the zomba city error (city in malawi) and DRC area filter
if you want to further avoid these issues, i would save full states in local storage only and i would also potentially change the structure of the saved states in localstorage to be an object with keys equal to sessionId and value their local storage, which means that you can actually have things open on multiple tabs without them interacting with each other,
note: you could even skip saving only full states with this because saving partial states makes sense if you have the correct sessionId but honestly i always prefer to wipe and reconstruct because it costs almost no extra time and is cleaner
but yh since local storage is shared between tabs, its tricky with stuff like this!
ps: with two tabs you can also create 2 projects with the same name :)
Thanks @M-Kusumgar chatted about this Follow ups which I will make a ticket for
|
Description
See ticket for more details, this stops mutations from updating local storage between when a project load replaces local storage with the new project to be loaded and when the browser refreshes.
You should be able to recreate the bug fairly easily on prod (and locally) by opening a project onto review input step with a large country e.g. Kenya. Then go back to projects and create a new project and fit the model. Then switch between the projects a few times in succession. Wait for the Kenya project to initiate a fetch for input plot data before swapping out. You should see (it might take a couple of attempts) that the completed projects later steps will become invalid.
This is deployed on preview at the moment and I cannot recreate it there.
Type of version change
Patches
Checklist