MWPW-158407, 158404, 158406, 158408, 158461: Ignoring decorateMeta if the metadata is from promo. #2924
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## stage #2924 +/- ##
==========================================
+ Coverage 96.24% 96.37% +0.12%
==========================================
Files 236 243 +7
Lines 54278 55122 +844
==========================================
+ Hits 52241 53122 +881
+ Misses 2037 2000 -37 ☔ View full report in Codecov by Sentry. |
seanchoi-dev
changed the title
|
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. |
libs/utils/utils.js
Outdated
| @@ -1147,7 +1147,7 @@ function decorateMeta() { | |||
| const { origin } = window.location; | |||
| const contents = document.head.querySelectorAll(`[content*=".${SLD}."]`); | |||
| contents.forEach((meta) => { | |||
| if (meta.getAttribute('property') === 'hlx:proxyUrl') return; | |||
| if (meta.getAttribute('property') === 'hlx:proxyUrl' || meta.getAttribute('name').endsWith('schedule')) return; | |||
There was a problem hiding this comment.
Are there any other meta names with this issue, other than "schedule"? We might be in a situation where someone else has another schedule referencing a URL and they will be forced to update the meta name. If this is just <meta name="schedule"> we're interested in, could we:
| if (meta.getAttribute('property') === 'hlx:proxyUrl' || meta.getAttribute('name').endsWith('schedule')) return; | |
| if (meta.getAttribute('property') === 'hlx:proxyUrl' || meta.getAttribute('name') === 'schedule') return; |
There was a problem hiding this comment.
@overmyheadandbody The metadata can be like americas_schedule or apac_schedule according to @3ch023 .
(https://adobe-mwp.slack.com/archives/CCB79168Z/p1726589105876609?thread_ts=1726522536.260839&cid=CCB79168Z)
There was a problem hiding this comment.
yes it can be emea_schedule too :) endswith('schedule') seems good to me
There was a problem hiding this comment.
Should it possibly be .endsWith('_schedule') or make it more explicit with .endsWith('_promo_schedule')?
There was a problem hiding this comment.
I think the answer is NO since the issue case (US homepage) is showing just schedule w/o prefix.
There was a problem hiding this comment.
confirming that .endsWith('_schedule') will not be enough since we also need to ignore 'schedule' itself.
I updated the doc with all possible variations of this metadata: https://milo.adobe.com/docs/authoring/metadata (see end of table)
We might be in a situation where someone else has another schedule referencing a URL and they will be forced to update the meta name.
I understand, but as of now I am not able to find such a field (checked code and https://milo.adobe.com/docs/authoring/metadata). + It is rather unlikely that the field containing a 'pure' URL would end with a 'schedule' in the name.
If we find such a field we could switch to the exact check of === instead of endsWith, but it will cost more bytes in code.
From my side it's ok to leave it as .endsWith('schedule').
seanchoi-dev
changed the title
|
|
@3ch023 @seanchoi-dev replace fragment promo is breaking the page on this branch. There might be some stale blocks on the branch or there might be the real issue. The page loads blank: Expected (main): https://main--milo--adobecom.hlx.page/drafts/nala/features/promotions/promo-with-fragments As far as i can see, the error reported comes from the changed file |
I fixed the js error. |
Resolves:
Test URLs:
Consumer test w/ real case: