Update to not fetch experience if it is empty #4149
Conversation
Passing run #4311 ↗︎
Details:
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. |
||||||||||||||||||||
| } else if (!isPrivacyExperience(experience)) { | ||
| // An empty experience (e.g. {}) is expected when 1. pre-fetch is enabled, and 2. the location has no associated | ||
| // experience. We should not fetch experiences again in this case. We only fetch experiences if it's undefined. | ||
| } else if (!experience) { |
There was a problem hiding this comment.
nit...
| } else if (!experience) { | |
| } else if (!effectiveExperience) { |
There was a problem hiding this comment.
TBH, this was clearer before using the utils method. I reworked that method to suit your purpose
|
@eastandwestwind I modified this to use the I also improved some test coverage to give us some confidence it behaves as expected 👍 |
Co-authored-by: Neville Samuell <[email protected]>
| * | ||
| * This includes the special case where the input is an empty object ({}), which | ||
| * is a valid response when the API does not find a PrivacyExperience configured | ||
| * for the given geolocation. | ||
| */ | ||
| export const isPrivacyExperience = ( | ||
| obj: PrivacyExperience | undefined | EmptyExperience | ||
| ): obj is PrivacyExperience => { |
There was a problem hiding this comment.
I happened to stumble on this PR and this change is a little dangerous type-wise—we claim that returning true means the obj passed in was of type PrivacyExperience, but this change makes it so it can be either PrivacyExperience or EmptyExperience. the type predicate will still say it's PrivacyExperience though!
Closes #4147
Description Of Changes
Update to not fetch experience client-side if pre-fetched experience was empty. Empty simply means the location has no relevant experiences, so we shouldn't try to fetch the experience again client-side.
Code Changes
Steps to Confirm
Pre-Merge Checklist
CHANGELOG.md