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.

Sign up for GitHub

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

Jump to bottom

feat: update apollo-client #1027

Merged
merged 17 commits into from
Feb 2, 2022
Merged

feat: update apollo-client #1027

merged 17 commits into from
Feb 2, 2022

Conversation

bchrobot
Copy link
Member

@bchrobot bchrobot commented Nov 3, 2021 •
edited
Loading

Description

Update apollo-client to v3.

Motivation and Context

Redoing #1018.

To-do:

  • Fix local resolvers used in interaction steps.
  • Remove changes to ci-cd.yml

How Has This Been Tested?

This has been (more thoroughly) tested locally.

Screenshots (if appropriate):

N/A

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Documentation Changes

N/A

Checklist:

  • My code follows the code style of this project.
  • My commit messages follow the Conventional Commits specification.
  • My change requires a change to the documentation.
  • I have included updates for the documentation accordingly.

@bchrobot
Copy link
Member Author

bchrobot commented Nov 3, 2021

The final problem with the v3 upgrade seems to be the local fields used for storing pending interaction step CrUD operations. At first this was presenting with an in-browser warning about a merge function being required. Adding that seemed to address the bug but it didn't make sense why it was necessary; the default behavior should be sufficient.

Additional debugging of local fields resulted in the Interaction Steps section closing whenever a script changed. Reverting all changes did not solve that issue. That was as far as I got before reverting the whole thing.

@bchrobot
fix: fix interaction step management
63d3b75 
Writing interaction steps to the cache caused the GET_EDIT_CAMPAIGN_DATA query to be re-fetched.
The results of re-fetch then overwrote the local changes for pending interaction step changes. This
re-fetch happened because in apollo-client@3 cache-and-network fetch policies re-run whenever there
is a cache update that affects the query.

For more information, see:
apollographql/apollo-client#6760 (comment)
@bchrobot
Copy link
Member Author

bchrobot commented Jan 7, 2022 •
edited
Loading

Writing interaction steps to the cache caused the GET_EDIT_CAMPAIGN_DATA query to be re-fetched. The results of re-fetch then overwrote the local changes for pending interaction step changes. This re-fetch happened because in apollo-client@3 cache-and-network fetch policies re-run whenever there is a cache update that affects the query.

For more information, see:
apollographql/apollo-client#6760 (comment)

@bchrobot bchrobot marked this pull request as ready for review January 7, 2022 10:54
@bchrobot bchrobot requested a review from ben-pr-p January 7, 2022 10:54
@bchrobot
Copy link
Member Author

bchrobot commented Jan 7, 2022

The merge function warning turns out to be unrelated. It is still an issue when saving a delete: following the mutation HTTP request, the local cache still contains the deleted step (marked isDeleted: true) but the server response does not include that step. The default behavior of overwriting the local cache is what we want (though it still warns).

@bchrobot
Copy link
Member Author

bchrobot commented Jan 7, 2022

@ben-pr-p the final commit and my last two comments are the only things that need to be reviewed. Everything else is a replay of #1018.

ben-pr-p
ben-pr-p previously approved these changes Jan 10, 2022
View reviewed changes
@bchrobot bchrobot mentioned this pull request Jan 27, 2022
Merged
7 tasks
@bchrobot
Merge branch 'main' into feat-update-apollo-client
1f3270b
@bchrobot
fix: update apollo imports
4283a9a
@bchrobot
fix: fix app import
41f082b
@bchrobot
fix: fix graphql variable type
e159675
@bchrobot
fix: restore functionality of message review actions
34ba7a6
@bchrobot
fix: fix interaction step local fields
d721ad8 
Adapts to two apollo client changes:
1. readFragment returns null for insufficient data instead of throwing MissingFieldError exceptions
2. writeFragment now only writes to fields present in the fragment itself
@bchrobot
fix: handle null contact location
c6a7701
@bchrobot bchrobot dismissed ben-pr-p’s stale review January 27, 2022 12:35

Additional changes made. Next round of QA

@bchrobot
Copy link
Member Author

bchrobot commented Jan 27, 2022 •
edited
Loading

New changes to review:
https://github.com/politics-rewired/Spoke/pull/1027/files/1f3270bd587e9c7318a9fa3c6cf7294ad19bb944..8ad2242ad30b03d3177bec4eac6778e6ccae7700

@bchrobot
Copy link
Member Author

bchrobot commented Feb 2, 2022

@ben-pr-p new changes to review:
https://github.com/politics-rewired/Spoke/pull/1027/files/1f3270bd587e9c7318a9fa3c6cf7294ad19bb944..a155a5244ce2c84151492a28250de4f76f06b1c5

@bchrobot
Copy link
Member Author

bchrobot commented Feb 2, 2022

all bugs fixed / no new ones :yay:
-- @ajohn25

@bchrobot bchrobot merged commit 7931f4c into main Feb 2, 2022
@bchrobot bchrobot deleted the feat-update-apollo-client branch February 2, 2022 22:13
@bchrobot bchrobot mentioned this pull request Feb 8, 2022
Merged
7 tasks
@ajohn25 ajohn25 mentioned this pull request May 14, 2022
Merged
2 tasks
@lediur lediur mentioned this pull request Jul 12, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ben-pr-p ben-pr-p ben-pr-p approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bchrobot @ben-pr-p