Skip to content
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

Fix issue in deployed apps where StateSubmissions are shown as Draft after submission #176

Merged
merged 4 commits into from
Jun 28, 2021

Conversation

macrael
Copy link
Contributor

@macrael macrael commented Jun 26, 2021

Summary

Issue found in acceptance, it's not reproducing locally so I'm going to try and figure it out in the review app.

OMG What a rabbit hole.

SO. The bug was that in prod after submission the card would still show "DRAFT" instead of "SUBMITTED". This didn't happen locally.

To try and solve this, we were calling "refetchQueries" on our submit mutation, thinking that the query would be refetched and the fresh data would be in it when we redirected. This turned out not to work in prod because of a confluence of reasons:

  1. We wrap our entire app in <React.StrictMode> which tries to catch some errors in local dev and one of the ways it tries to catch those errors is by calling all your render functions twice, silently. (It literally swizzles console.log so that your log statements aren't printed twice)
  2. Apollo Client does not handle that correctly. This issue seems to be the best one tracking them fixing this, but there were several that led me there. This means that useQuery() is being called twice when Apollo doesn't expect it, which causes it to register more "active queries" than it should. Apollo tracks queries for the lifecycle of the component that called useQuery(), but because of this bug, they are outliving the components in local dev even though they are correctly discarded in prod.
  3. When we call "refetchQueries" on our submit mutation, it only refetches active queries. In dev, that fixed the issue because there were erroneous active queries lying around, but in prod it did nothing because the indexSubmissions query was gone. Then when we redirected to the dashboard, a new indexSubmissions query was created that simply read from the cache.

Phew.

The fix is to delete the data from the cache instead of calling refetchQueries. This more accurately maps to what we want to happen anyway so it's a good fix.

Related issues

Screenshots

Testing guidance

I updated the cypress test to catch this issue, but run through a full submission from scratch in the review app and make sure that the card says "Submitted" afterwards.

@macrael macrael marked this pull request as ready for review June 27, 2021 06:58
@macrael macrael requested a review from haworku June 27, 2021 06:58
Copy link
Contributor

@haworku haworku left a comment

Choose a reason for hiding this comment

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

Ran through everything. Things working from scratch ⭐ - though that GH issue you linked made me 🤔 . I need to understand apollo client caching more, might ask some Trussels about this too in terms of what they have seen.

Unrelated to the scope of this issue but shouldn't the most recently submitted submission be at the top of the list? thought that was the requirement from #168 - usually we list drafts then submitted submission ... but the special case is right when you submit (while that query in the url) the recently submitted submission only floats to the top.

@macrael
Copy link
Contributor Author

macrael commented Jun 27, 2021

I had to get a lot more in the weeds about how caching works, happy to talk about it with you.

And we should discuss stripping out StrictMode until apollo fixes that issue, I think it's not great that we're having this kind of client/server difference

I missed that requirement about just submitted. That should be an easy fix on this PR

@haworku
Copy link
Contributor

haworku commented Jun 28, 2021

@macrael Going to merge this since things look good and I think this influences cypress testing the submitted submission

@haworku haworku merged commit f2f4c3d into main Jun 28, 2021
@haworku haworku deleted the wml-list-ac branch June 28, 2021 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants