Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Saved payment methods main view. #2882
Saved payment methods main view. #2882
Changes from 8 commits
2af3f7a
2c505ae
b90fe86
e295112
4098a4c
6ac74aa
e294ac8
7c73950
b1d7f04
3a1c7fe
721a521
e586dbc
8eb9f2c
27234d2
6a35ae6
d3ea89e
133131c
229aa67
4b85318
a6d5603
b6a62ba
1b483cb
e00d30d
33677e8
71a4f64
0320f7f
59672a4
9634eed
ca5f94a
e2ab10c
7282748
cfef3f9
a6a7e18
a607400
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 a clever solution, just give the collection an identity. I would normally suggest that the
PaymentToken
type needs an identity, but that may not be necessary since there's no way to mutate it. I also don't like adding these hidden requirements to queries, not sure how Apollo reacts when the field for the key identity is missing.@sirugh - is there a technical reason we should do something like:
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.
The solution you are proposing would work on individual tokens, the one I have here is for the whole collection. I feel like adding
keyFields
to each token would be overkill unless it provides some use. I am not equipped enough to make the call about the apollo types we are using here, I am fine with what the team thinks is the right approach.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 from the docs. I wouldn't try to get much more creative that our use case. We only use this because we need a handle on the cache key in order to delete it. If we find that other properties are still cached and not garbage collected after the deletion of these other objects then we may need to identify PaymentTokens with keyFields and delete them too.