MWPW-156866 [MILO][MEP] Create martech metadata table if placeholders are used in non EN page #2780
Conversation
|
Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
|
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## stage #2780 +/- ##
==========================================
+ Coverage 95.89% 95.90% +0.01%
==========================================
Files 173 173
Lines 45853 45870 +17
==========================================
+ Hits 43969 43992 +23
+ Misses 1884 1878 -6 ☔ View full report in Codecov by Sentry. |
|
This pull request is not passing all required checks. Please see this discussion for information on how to get all checks passing. Inconsistent checks can be manually retried. If a test absolutely can not pass for a good reason, please add a comment with an explanation to the PR. |
There was a problem hiding this comment.
Is this really necessary? Can't we re-use the existing placeholder entries?
They've recently been exposed https://github.com/adobecom/milo/pull/2737/files
@mokimo yes, because this is for MEP specific placeholders that are being introduced by the manifests. They are not part of the retreived placeholders. |
|
There are some unit tests / linting failing, other than that IMO this looks good and can be approved after that's fixed 👍 |
AdobeLinhart
added
verified
|
@mokimo can you review and lift request change if you are satisfied? |
| export async function createMartechMetadata(placeholders, config, column) { | ||
| if (config.locale.ietf === 'en-US') return; | ||
|
|
||
| await import('../../martech/attributes.js').then(({ processTrackingLabels }) => { |
There was a problem hiding this comment.
I'd be a bit careful with those imports as they block the pipeline and we aren't loading things in parallel.
There was a problem hiding this comment.
Here's a bit of a case-study I did on project unity on this: adobecom/unity#102
When we use MEP placeholders on a non EN page, the analytics don't stay in English. To combat this, we will add to the martech-metadata.
In libs/features/personalization/personalization.js parsePlaceholders function, we decide the column of values we will use.
We should check the value of config.locale.ietf. If it equals en-US we don't need to generate a table.
If it doesn't, we need to see if there is a good column for us to use for the original English text. Column names we want to use in order of preference: en-us, us, en.
I think you can make an array with those values, then reuse the code we use to build the placeholder object to build a translation object. Move it to a reusable function maybe.
Then add to config.mep and reference your new data in martech/attributes.js
Resolves: MWPW-156866
QA instructions
In the before and after links, inspect the buttons in the marquee. The analytics should now show en-us version of the placeholder instead of the fr.
Test URLs: