MWPW-158464: Switch Odin endpoint #2894
Conversation
|
Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## stage #2894 +/- ##
==========================================
+ Coverage 96.09% 96.24% +0.14%
==========================================
Files 215 237 +22
Lines 53966 54222 +256
==========================================
+ Hits 51861 52184 +323
+ Misses 2105 2038 -67 ☔ View full report in Codecov by Sentry. |
3ch023
requested review from
yesil,
npeltier and
Axelcureno
and removed request for
a team
| @@ -2,6 +2,7 @@ import { AEM } from './aem.js'; | |||
| import { createTag } from './utils.js'; | |||
|
|
|||
| const ATTR_AEM_BUCKET = 'aem-bucket'; | |||
| const AEM_BUCKET = 'publish-p22655-e155390'; | |||
There was a problem hiding this comment.
should we already have STAGE & QA overrides ready? considering this will be freyja prod?
There was a problem hiding this comment.
should we already have STAGE & QA overrides ready?
that will need some agreement in the team on how we do those overrides, so i'd say out of scope of this PR.
I just want us to start using the actual odin content modal thus this small change
JackySun9
added
caas-configurator
| @@ -3,12 +3,12 @@ export async function withAem(originalFetch) { | |||
| if (/cf\/fragments\/search/.test(pathname)) { | |||
| // TODO add conditional use case. | |||
| return originalFetch( | |||
| '/test/mocks/sites/cf/fragments/search/default.json', | |||
| '/test/mocks/sites/cf/fragments/search/authorPayload.json', | |||
There was a problem hiding this comment.
It is implied that the API and response is for author, since Freyja/publish doesn't support this.
This name might be misleading like one can think that there may be also a publishPayload for search api.
how about searchResponse.json ? Neutral and explicit.
There was a problem hiding this comment.
there will be 'edgePayload.json' or something like this to test Freya
| @@ -2,6 +2,7 @@ import { AEM } from './aem.js'; | |||
| import { createTag } from './utils.js'; | |||
|
|
|||
| const ATTR_AEM_BUCKET = 'aem-bucket'; | |||
| const AEM_BUCKET = 'publish-p22655-e155390'; | |||
There was a problem hiding this comment.
I think we should delete the default value and discuss how to pass it as a parameter in a clean way
There was a problem hiding this comment.
yes, agree here, i just didn't want to bloat this PR yet.
same as @npeltier mentioned above, we need mechanism to switch from prod to qa/stage etc
| const merchIcon = createTag('merch-icon', { | ||
| slot: 'icons', | ||
| src: icon, | ||
| alt: '', | ||
| href: '', | ||
| alt, |
There was a problem hiding this comment.
if alt and href are undefined, are they properly omitted or rendered as alt="undefined" ? I couldn't verify it.
There was a problem hiding this comment.
good point, will test it today
| @@ -44,6 +44,7 @@ runTests(async () => { | |||
| const [ccCard, photoshopCard] = getTemplateContent('cards'); | |||
| document.querySelector('main').append(ccCard, photoshopCard); | |||
| expect(aemMock.count).to.equal(0); | |||
| const card = document.querySelector('main merch-card:has(> merch-datasource[path="/content/dam/sandbox/mas/creative-cloud"])'); | |||
There was a problem hiding this comment.
we should stop using /content/dam/sandbox/mas and start using /content/dam/mas
There was a problem hiding this comment.
will address in next PR
Test Page:
https://mas-docs--milo--adobecom.hlx.live/libs/features/mas/docs/plans.html?martech=off&georouting=off
Resolves: https://jira.corp.adobe.com/browse/MWPW-158464
Test URLs: