-
Notifications
You must be signed in to change notification settings - Fork 77
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
Optimize TCF bundle with just-in-time GVL translations #5074
Merged
gilluminate
merged 22 commits into
main
from
gill/PROD-2281_mediavine-optimize-tcf-bundle
Jul 17, 2024
Merged
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
28a159d
fix extra long strings being rendered in demo pages
gilluminate e3f2bd8
update logs to be more helpful
gilluminate 3387920
remove circular dependency
gilluminate 8d1132a
Side load GVL translations to reduce payload size
gilluminate f28acc7
Add fetchGvlTranslations to call existing endpoint
gilluminate b1f9980
set available_locales on the experience
gilluminate f9db42e
mimic filtered GVL Language response
gilluminate 446dfcb
update cypress tests
gilluminate f875db2
fix more tests
gilluminate e03da81
offset GVL translation timing. stop `await`ing initOverlay
gilluminate 3791ad6
reduce flakiness of test. just check success
gilluminate d54555b
fix admin-ui language issue: supply locales to preveiw
gilluminate 08b5e4e
fix unit tests
gilluminate 309b1ac
Update api.ts
gilluminate 5a871a5
fix flaky test by not looking at data reliant on config
gilluminate 8bdc809
error handling for gvl translations
gilluminate bbf90ce
add Cypress test for failing translation API fallback
gilluminate 3a825ac
update Changelog. fix typo
gilluminate cf7d492
remove client-side parsing of available_locales
gilluminate b0b2456
update to use new version of translations API
gilluminate ae1c3ca
misc. clean up
gilluminate 384a8ae
remove all references to gvl_translations
gilluminate File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ import { | |
selectBestExperienceConfigTranslation, | ||
areLocalesEqual, | ||
localizeModalLinkText, | ||
loadMessagesFromGVLTranslations, | ||
} from "~/lib/i18n"; | ||
import { loadTcfMessagesFromFiles } from "~/lib/tcf/i18n/tcf-i18n-utils"; | ||
import messagesEn from "~/lib/i18n/locales/en/messages.json"; | ||
|
@@ -80,7 +81,6 @@ describe("i18n-utils", () => { | |
]; | ||
|
||
const mockI18n = { | ||
// eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
activate: jest.fn((locale: Locale): void => { | ||
mockCurrentLocale = locale; | ||
}), | ||
|
@@ -263,12 +263,8 @@ describe("i18n-utils", () => { | |
|
||
describe("loadMessagesFromExperience", () => { | ||
it("reads all messages from experience API response and loads into the i18n catalog", () => { | ||
const updatedLocales = loadMessagesFromExperience( | ||
mockI18n, | ||
mockExperience | ||
); | ||
loadMessagesFromExperience(mockI18n, mockExperience); | ||
const EXPECTED_NUM_TRANSLATIONS = 2; | ||
expect(updatedLocales).toEqual(["en", "es"]); | ||
expect(mockI18n.load).toHaveBeenCalledTimes(EXPECTED_NUM_TRANSLATIONS); | ||
expect(mockI18n.load).toHaveBeenCalledWith("en", mockI18nCatalogLoad[0]); | ||
expect(mockI18n.load).toHaveBeenCalledWith("es", mockI18nCatalogLoad[1]); | ||
|
@@ -301,15 +297,12 @@ describe("i18n-utils", () => { | |
|
||
// Edit the experience data to match the legacy format (w/o translations) | ||
delete mockExpNoTranslations.experience_config.translations; | ||
mockExpNoTranslations.available_locales = ["en"]; | ||
|
||
// Load the "no translations" version of the experience and run tests | ||
const updatedLocales = loadMessagesFromExperience( | ||
mockI18n, | ||
mockExpNoTranslations as any | ||
); | ||
loadMessagesFromExperience(mockI18n, mockExpNoTranslations as any); | ||
|
||
const EXPECTED_NUM_TRANSLATIONS = 1; | ||
expect(updatedLocales).toEqual(["en"]); | ||
expect(mockI18n.load).toHaveBeenCalledTimes(EXPECTED_NUM_TRANSLATIONS); | ||
expect(mockI18n.load).toHaveBeenCalledWith("en", mockI18nCatalogLoad[0]); | ||
}); | ||
|
@@ -322,13 +315,12 @@ describe("i18n-utils", () => { | |
privacy_policy_url: "https://example.com/privacy", | ||
override_language: "en", | ||
}; | ||
const updatedLocales = loadMessagesFromExperience( | ||
loadMessagesFromExperience( | ||
mockI18n, | ||
mockExperience, | ||
experienceTranslationOverrides | ||
); | ||
const EXPECTED_NUM_TRANSLATIONS = 2; | ||
expect(updatedLocales).toEqual(["en", "es"]); | ||
expect(mockI18n.load).toHaveBeenCalledTimes(EXPECTED_NUM_TRANSLATIONS); | ||
expect(mockI18n.load).toHaveBeenCalledWith("en", { | ||
...mockI18nCatalogLoad[0], | ||
|
@@ -350,47 +342,35 @@ describe("i18n-utils", () => { | |
privacy_policy_url: "https://example.com/privacy", | ||
override_language: "ja", | ||
}; | ||
const updatedLocales = loadMessagesFromExperience( | ||
loadMessagesFromExperience( | ||
mockI18n, | ||
mockExperience, | ||
experienceTranslationOverrides | ||
); | ||
const EXPECTED_NUM_TRANSLATIONS = 2; | ||
expect(updatedLocales).toEqual(["en", "es"]); | ||
expect(mockI18n.load).toHaveBeenCalledTimes(EXPECTED_NUM_TRANSLATIONS); | ||
expect(mockI18n.load).toHaveBeenCalledWith("en", mockI18nCatalogLoad[0]); | ||
expect(mockI18n.load).toHaveBeenCalledWith("es", mockI18nCatalogLoad[1]); | ||
}); | ||
|
||
describe("when loading from a tcf_overlay experience", () => { | ||
it("reads all messages from gvl_translations API response and loads into the i18n catalog", () => { | ||
it("reads all messages from gvl translations API response and loads into the i18n catalog", () => { | ||
// Mock out a partial response for a tcf_overlay including translations | ||
const mockExpWithGVL = JSON.parse(JSON.stringify(mockExperience)); | ||
mockExpWithGVL.experience_config.component = "tcf_overlay"; | ||
mockExpWithGVL.gvl_translations = mockGVLTranslationsJSON; | ||
|
||
// Load all the translations | ||
const updatedLocales = loadMessagesFromExperience( | ||
mockI18n, | ||
mockExpWithGVL | ||
); | ||
loadMessagesFromGVLTranslations(mockI18n, mockGVLTranslationsJSON, [ | ||
"en", | ||
"es", | ||
]); | ||
|
||
// First, confirm that the "regular" experience_config translations are loaded | ||
const EXPECTED_NUM_TRANSLATIONS = 2; | ||
expect(updatedLocales).toEqual(["en", "es"]); | ||
expect(mockI18n.load).toHaveBeenCalledTimes(EXPECTED_NUM_TRANSLATIONS); | ||
const [, loadedMessagesEn] = mockI18n.load.mock.calls[0]; | ||
const [, loadedMessagesEs] = mockI18n.load.mock.calls[1]; | ||
expect(loadedMessagesEn).toMatchObject({ | ||
"exp.accept_button_label": "Accept Test", | ||
"exp.acknowledge_button_label": "Acknowledge Test", | ||
}); | ||
expect(loadedMessagesEs).toMatchObject({ | ||
"exp.accept_button_label": "Aceptar Prueba", | ||
"exp.acknowledge_button_label": "Reconocer Prueba", | ||
}); | ||
|
||
// Confirm that the English gvl_translations are loaded | ||
// Confirm that the English GVL translations are loaded | ||
const expectedMessagesEn: Record<string, RegExp> = { | ||
// Example purposes | ||
"exp.tcf.purposes.1.name": /^Store and\/or access/, | ||
|
@@ -421,7 +401,7 @@ describe("i18n-utils", () => { | |
expect(loadedMessagesEn[id]).toMatch(regex); | ||
}); | ||
|
||
// Confirm that the Spanish gvl_translations are loaded | ||
// Confirm that the Spanish GVL translations are loaded | ||
const expectedMessagesEs: Record<string, RegExp> = { | ||
// Example purposes | ||
"exp.tcf.purposes.1.name": /^Almacenar la información/, | ||
|
@@ -483,49 +463,6 @@ describe("i18n-utils", () => { | |
expect(getRecordCounts(loadedMessagesEn)).toMatchObject(expectedCounts); | ||
expect(getRecordCounts(loadedMessagesEs)).toMatchObject(expectedCounts); | ||
}); | ||
|
||
it("handles a mismatch between the experience_config and gvl_translations APIs by returning only locales available in both", () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is all handled server-side now |
||
// Mock out a partial response for a tcf_overlay including translations | ||
const mockExpWithGVL = JSON.parse(JSON.stringify(mockExperience)); | ||
mockExpWithGVL.experience_config.component = "tcf_overlay"; | ||
mockExpWithGVL.gvl_translations = mockGVLTranslationsJSON; | ||
|
||
// Modify "en" to be "EN" (uppercased!), which shouldn't be treated as a mismatch! | ||
mockExpWithGVL.experience_config.translations[0].language = "EN"; | ||
|
||
// Replace "es" with "es-MX" in the experience_config.translations to force a mismatch | ||
mockExpWithGVL.experience_config.translations[1].language = "es-MX"; | ||
|
||
// Confirm our test setup shows a mismatch between experience_config & gvl_translations | ||
expect( | ||
mockExpWithGVL.experience_config.translations.map( | ||
(e: any) => e.language | ||
) | ||
).toEqual(["EN", "es-MX"]); | ||
expect(Object.keys(mockExpWithGVL.gvl_translations)).toEqual([ | ||
"en", | ||
"es", | ||
]); | ||
|
||
// Load all the translations | ||
const updatedLocales = loadMessagesFromExperience( | ||
mockI18n, | ||
mockExpWithGVL | ||
); | ||
|
||
// Confirm that only the overlapping locales are loaded | ||
expect(updatedLocales).toEqual(["EN"]); | ||
const [, loadedMessagesEn] = mockI18n.load.mock.calls[0]; | ||
expect(loadedMessagesEn).toMatchObject({ | ||
"exp.accept_button_label": "Accept Test", | ||
"exp.acknowledge_button_label": "Acknowledge Test", | ||
"exp.tcf.purposes.1.name": | ||
"Store and/or access information on a device", | ||
"exp.tcf.purposes.1.description": expect.stringMatching( | ||
/^Cookies, device or similar/ | ||
), | ||
}); | ||
}); | ||
}); | ||
}); | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -281,12 +281,16 @@ const PagedVendorData = ({ | |
otherVendors: VendorRecord[]; | ||
} = useMemo( | ||
() => ({ | ||
gvlVendors: activeChunk.filter((v) => v.isGvl), | ||
otherVendors: activeChunk.filter((v) => !v.isGvl), | ||
gvlVendors: activeChunk?.filter((v) => v.isGvl), | ||
otherVendors: activeChunk?.filter((v) => !v.isGvl), | ||
}), | ||
[activeChunk] | ||
); | ||
|
||
if (!activeChunk) { | ||
return null; | ||
} | ||
|
||
Comment on lines
+284
to
+293
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. handling (unlikely) scenario of having 0 vendors configured for a given experience |
||
return ( | ||
<Fragment> | ||
<RecordsList<VendorRecord> | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
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.
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.
resolves a Typescript complaint that these were missing