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

Update to not fetch experience if it is empty #4149

Merged
merged 10 commits into from
Sep 25, 2023
Merged

Update to not fetch experience if it is empty #4149

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
087953c
update to not fetch experience if it is empty
eastandwestwind Sep 22, 2023
ad91b9b
update changelog
eastandwestwind Sep 22, 2023
b224989
update and add e2e tests
eastandwestwind Sep 23, 2023
e5c74ad
Refactor setting config overrides in e2e tests
eastandwestwind Sep 23, 2023
d25248a
lint
eastandwestwind Sep 23, 2023
9c8c749
Add unit tests for isPrivacyExperience
NevilleS Sep 23, 2023
cb35523
Use isPrivacyExperience to test prefetch experience
NevilleS Sep 24, 2023
58bb3f1
Improve whitespace in Cypress tests for consent banner
NevilleS Sep 24, 2023
40320e9
Cleanup & de-lint
NevilleS Sep 24, 2023
5797c6d
Fix formatting
NevilleS Sep 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ The types of changes are:
### Fixed
- Allows CDN to cache empty experience responses from fides.js API [#4113](https://github.com/ethyca/fides/pull/4113)
- added version_added, version_deprecated, and replaced_by to data use, data subject, and data category APIs [#4135](https://github.com/ethyca/fides/pull/4135)
- Update fides.js to not fetch experience client-side if pre-fetched experience is empty [#4149](https://github.com/ethyca/fides/pull/4149)

## [2.20.1](https://github.com/ethyca/fides/compare/2.20.0...2.20.1)

Expand Down
4 changes: 2 additions & 2 deletions clients/fides-js/src/lib/consent-types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ export interface FidesConfig {
// Set the consent defaults from a "legacy" Privacy Center config.json.
consent?: LegacyConsentConfig;
// Set the "experience" to be used for this Fides.js instance -- overrides the "legacy" config.
// If set, Fides.js will fetch neither experience config nor user geolocation.
// If not set or is empty, Fides.js will attempt to fetch its own experience config.
// If defined or is empty, Fides.js will not fetch experience config.
// If undefined, Fides.js will attempt to fetch its own experience config.
experience?: PrivacyExperience | EmptyExperience;
// Set the geolocation for this Fides.js instance. If *not* set, Fides.js will fetch its own geolocation.
geolocation?: UserGeolocation;
Expand Down
4 changes: 3 additions & 1 deletion clients/fides-js/src/lib/initialize.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,9 @@ export const initialize = async ({
`User location could not be obtained. Skipping overlay initialization.`
);
shouldInitOverlay = false;
} 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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit...

Suggested change
} else if (!experience) {
} else if (!effectiveExperience) {

Copy link
Contributor

Choose a reason for hiding this comment

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

TBH, this was clearer before using the utils method. I reworked that method to suit your purpose

effectiveExperience = await fetchExperience(
fidesRegionString,
options.fidesApiUrl,
Expand Down
Loading