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

Update to not fetch experience if it is empty #4149

Merged
merged 10 commits into from
Sep 25, 2023
1 change: 1 addition & 0 deletions CHANGELOG.md
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
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
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
74 changes: 61 additions & 13 deletions clients/privacy-center/cypress/e2e/consent-banner.cy.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import {
ComponentType,
CONSENT_COOKIE_NAME,
ConsentMethod,
ConsentMechanism,
UserConsentPreference,
ConsentMethod,
ConsentOptionCreate,
FidesCookie,
LastServedNoticeSchema,
ConsentOptionCreate,
UserConsentPreference,
} from "fides-js";

import { mockPrivacyNotice } from "../support/mocks";
Expand Down Expand Up @@ -49,7 +49,7 @@ describe("Consent banner", () => {
options: {
isOverlayEnabled: false,
},
experience: OVERRIDE.EMPTY,
experience: OVERRIDE.UNDEFINED,
},
{},
{}
Expand Down Expand Up @@ -681,7 +681,7 @@ describe("Consent banner", () => {
describe("when experience is not provided, and valid geolocation is provided", () => {
beforeEach(() => {
stubConfig({
experience: OVERRIDE.EMPTY,
experience: OVERRIDE.UNDEFINED,
geolocation: {
country: "US",
location: "US-CA",
Expand Down Expand Up @@ -712,7 +712,7 @@ describe("Consent banner", () => {
describe("when experience is provided, and geolocation is not provided", () => {
beforeEach(() => {
stubConfig({
geolocation: OVERRIDE.EMPTY,
geolocation: OVERRIDE.UNDEFINED,
options: {
isGeolocationEnabled: true,
geolocationApiUrl: "https://some-geolocation-url.com",
Expand All @@ -738,13 +738,61 @@ describe("Consent banner", () => {
});
});

describe("when experience is empty, and geolocation is not provided", () => {
beforeEach(() => {
stubConfig({
geolocation: OVERRIDE.UNDEFINED,
experience: OVERRIDE.EMPTY,
options: {
isGeolocationEnabled: true,
geolocationApiUrl: "https://some-geolocation-url.com",
},
});
});

it("does fetches geolocation and does not render the banner", () => {
// we still need geolocation because it is needed to save consent preference
cy.wait("@getGeolocation");
cy.get("div#fides-banner").should("not.exist");
cy.contains("button", "Accept Test").should("not.exist");
});
it("does not render modal link", () => {
cy.get("#fides-modal-link").should("not.be.visible");
});
});

describe("when experience is empty, and geolocation is provided", () => {
beforeEach(() => {
stubConfig({
geolocation: {
country: "US",
location: "US-CA",
region: "CA",
},
experience: OVERRIDE.EMPTY,
options: {
isGeolocationEnabled: true,
geolocationApiUrl: "https://some-geolocation-url.com",
},
});
});

it("does not geolocate and does not render the banner", () => {
eastandwestwind marked this conversation as resolved.
Show resolved Hide resolved
cy.get("div#fides-banner").should("not.exist");
cy.contains("button", "Accept Test").should("not.exist");
});
it("does not render modal link", () => {
cy.get("#fides-modal-link").should("not.be.visible");
});
});

describe("when neither experience nor geolocation is provided, but geolocationApiUrl is defined", () => {
describe("when geolocation is successful", () => {
beforeEach(() => {
const geoLocationUrl = "https://some-geolocation-api.com";
stubConfig({
experience: OVERRIDE.EMPTY,
geolocation: OVERRIDE.EMPTY,
experience: OVERRIDE.UNDEFINED,
geolocation: OVERRIDE.UNDEFINED,
options: {
isGeolocationEnabled: true,
geolocationApiUrl: geoLocationUrl,
Expand Down Expand Up @@ -780,8 +828,8 @@ describe("Consent banner", () => {
};
stubConfig(
{
experience: OVERRIDE.EMPTY,
geolocation: OVERRIDE.EMPTY,
experience: OVERRIDE.UNDEFINED,
geolocation: OVERRIDE.UNDEFINED,
options: {
isGeolocationEnabled: true,
geolocationApiUrl: "https://some-geolocation-api.com",
Expand All @@ -805,7 +853,7 @@ describe("Consent banner", () => {
describe("when experience is not provided, and geolocation is invalid", () => {
beforeEach(() => {
stubConfig({
experience: OVERRIDE.EMPTY,
experience: OVERRIDE.UNDEFINED,
geolocation: {
country: "US",
location: "",
Expand Down Expand Up @@ -842,8 +890,8 @@ describe("Consent banner", () => {
describe("when experience is not provided, and geolocation is not provided, but geolocation is disabled", () => {
beforeEach(() => {
stubConfig({
experience: OVERRIDE.EMPTY,
geolocation: OVERRIDE.EMPTY,
experience: OVERRIDE.UNDEFINED,
geolocation: OVERRIDE.UNDEFINED,
options: {
isGeolocationEnabled: false,
},
Expand Down
31 changes: 15 additions & 16 deletions clients/privacy-center/cypress/support/stubs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,19 @@ export const stubIdVerification = () => {
export enum OVERRIDE {
// signals that we should override entire prop with undefined
EMPTY = "Empty",
UNDEFINED = "Undefined",
}

const setNewConfig = (baseConfigObj: any, newConfig: any): any => {
if (newConfig === OVERRIDE.EMPTY) {
return {};
}
if (newConfig === OVERRIDE.UNDEFINED) {
return undefined;
}
return Object.assign(baseConfigObj, newConfig);
};

interface FidesConfigTesting {
// We don't need all required props to override the default config
consent?: Partial<LegacyConsentConfig> | OVERRIDE;
Expand All @@ -39,22 +50,10 @@ export const stubConfig = (
) => {
cy.fixture("consent/test_banner_options.json").then((config) => {
const updatedConfig = {
consent:
consent === OVERRIDE.EMPTY
? undefined
: Object.assign(config.consent, consent),
experience:
experience === OVERRIDE.EMPTY
? {}
: Object.assign(config.experience, experience),
geolocation:
geolocation === OVERRIDE.EMPTY
? undefined
: Object.assign(config.geolocation, geolocation),
options:
options === OVERRIDE.EMPTY
? undefined
: Object.assign(config.options, options),
consent: setNewConfig(config.consent, consent),
experience: setNewConfig(config.experience, experience),
geolocation: setNewConfig(config.geolocation, geolocation),
options: setNewConfig(config.options, options),
};
// We conditionally stub these APIs because we need the exact API urls, which can change or not even exist
// depending on the specific test case.
Expand Down