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

MWPW-156220: Support pa code and product code for #2712

Merged
merged 4 commits into from
Aug 12, 2024
Merged

Conversation

yesil
Copy link
Contributor

@yesil yesil commented Aug 8, 2024

checkout link configuration in addition to product family.
@yesil yesil requested a review from a team as a code owner August 8, 2024 10:58
Copy link
Contributor

aem-code-sync bot commented Aug 8, 2024

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

  • Re-run PSI checks
  • Re-sync branch
Commits

Copy link
Contributor

aem-code-sync bot commented Aug 8, 2024

Page Scores Audits Google
/drafts/ilyas/checkout-link-config?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@yesil yesil added run-nala Run Nala Test Automation against PR commerce labels Aug 8, 2024
expect(checkoutLinkConfig.DOWNLOAD_TEXT).to.equal('productCode');
checkoutLinkConfig = await getCheckoutLinkConfig('', 'testProductCode', '');
expect(checkoutLinkConfig.DOWNLOAD_TEXT).to.equal('productCode');
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add a test with paCode and family code and twice the same family code (which would look like ticket's use case ;) )

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

twice the same family code
Not sure if I understand it. Too specific test cases will make test unnecessarily complex and verbose.
Having said that, I have verified the use case manually, see https://mwpw-156220--milo--adobecom.hlx.page/drafts/ilyas/checkout-link-config?martech=off

Clicking on Buy Now opens the AI buy now modal (the iframe doesn't open due to CSP but you can verify the correctness of TWP url in the DOM)

const checkoutLinkConfig = productFamilyConfigs.find(
let productCheckoutLinkConfigs = [];
if (paCode) {
productCheckoutLinkConfigs = checkoutLinkConfigs.data?.filter(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would rather only use the filter function once, with 3 tests in there, no? only one traversal of the array, with quick test looks quicker to me

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, in that case, it will likely find first the entry with the product family rather than pa code unless we require GWP to author product code and pa codes entries first.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, but typical case with a family code line match implies traversal and test of 100 entries (l217) (no match), then another traversal of 100 entries (l221) (no match), then finally traversal of 100 entries to filter the right ones in l227.
my proposal was to do one traversal instead

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I can do it with a reducer logic, let me give it a try.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@3ch023 I'll update the column name and the code after https://jira.corp.adobe.com/browse/ODM-19213 is done.

Copy link

codecov bot commented Aug 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.75%. Comparing base (ccb9704) to head (aa6b71d).
Report is 5 commits behind head on stage.

Additional details and impacted files
@@           Coverage Diff           @@
##            stage    #2712   +/-   ##
=======================================
  Coverage   95.74%   95.75%           
=======================================
  Files         173      173           
  Lines       45600    45692   +92     
=======================================
+ Hits        43661    43752   +91     
- Misses       1939     1940    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Roycethan
Copy link

Roycethan commented Aug 9, 2024

@yesil Regression covered. Need 2 approvals for for making it ready for stage. also below descripancy needs to be reviwed..

AI Assistent for Acrobat modal renders right product now and also pre-selct to Business works fine can be reviwed here:
https://main--cc--adobecom.hlx.page/products/catalog?milolibs=mwpw-156220&mep=%2Fproducts%2Fcatalog.json--target-b2b_pzn---%2Fdrafts%2Frakaur%2Fdotcom-126722-catalog-test.json--default---%2Fdrafts%2Frakaur%2Fdotcom-126722-catalog-test-ste.json--default---%2Fdrafts%2Frakaur%2Fdotcom-126722-catalog-test-cct.json--default#search=acrobat

Issue/Observation:
However want to make one observation on Signed-in use case :
Upon login to above link with noronha+photo+acro@adobetest.com/Adobe123#
CTA for AI Assistent for Acrobat renders "Download" pointing to https://creativecloud.adobe.com/apps/download/acrobat
image

Expectation ( As per observed behaviour in con-a with same user credentials:
User will see "Buy now" with CRM modal pointed

image

@yesil
Copy link
Contributor Author

yesil commented Aug 9, 2024

@Roycethan the reported discrepancies are not real discrepancies. They are reproducible on main as well.
We need to address the entitlement related logic in a separate ticket/PR.

Copy link
Contributor

@npeltier npeltier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i guess we could make reduce bit slightly smarter and manage only one config per dimension + locale directly, but not sure it will make better readabilty

@yesil
Copy link
Contributor Author

yesil commented Aug 9, 2024

@npeltier feel free to suggest a code change directly here.

We can give a second touch once product code is enabled, and we can disable the paCode.

paCodeConfigs,
productCodeConfigs,
productFamilyConfigs,
} = checkoutLinkConfigs.data.reduce((acc, config) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
} = checkoutLinkConfigs.data.reduce((acc, config) => {
} = checkoutLinkConfigs.data?.reduce((acc, config) => {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The suggested change causing a strange eslint warning.
Since it was checkoutLinkConfigs.data before with no error, I'll keep the line as is.
Instead, I initialize it as checkoutLinkConfigs = { data: [] } in case of error.

...paCodeConfigs, ...productCodeConfigs, ...productFamilyConfigs,
];

if (productCheckoutLinkConfigs.length === 0) return undefined;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (productCheckoutLinkConfigs.length === 0) return undefined;
if (!productCheckoutLinkConfigs.length) return undefined;

@@ -198,22 +198,47 @@ export async function fetchCheckoutLinkConfigs(base = '') {
return fetchCheckoutLinkConfigs.promise;
}

export async function getCheckoutLinkConfig(productFamily) {
/**
* @param {*} productFamily product family code
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we needs the docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not really. I guess you want to save bytes :)

@Roycethan
Copy link

Expectation ( As per observed behaviour in con-a with same user credentials:
User will see "Buy now" with CRM modal pointed

Yes ok tracked here: https://jira.corp.adobe.com/browse/MWPW-156362

@Roycethan Roycethan added verified PR has been E2E tested by a reviewer Ready for Stage labels Aug 9, 2024
@milo-pr-merge milo-pr-merge bot merged commit b407c02 into stage Aug 12, 2024
20 checks passed
@milo-pr-merge milo-pr-merge bot deleted the MWPW-156220 branch August 12, 2024 09:15
@milo-pr-merge milo-pr-merge bot mentioned this pull request Aug 12, 2024
elan-tbx added a commit that referenced this pull request Aug 12, 2024
Revert "MWPW-147867 - [marquee] support RTL for loc" (#2662)

Revert "MWPW-147867 - [marquee] support RTL for loc (#2640)"

This reverts commit 49df5f7.

Revert "[MWPW-145131] Send locale and language code to analytics" (#2661)

Revert "[MWPW-145131] Send locale and language code to analytics (#2600)"

This reverts commit 04e687f.

MWPW-155079 [MILO][MEP] Manifests not changing domains (#2626)

* replace overridePersonalizationVariant function

* Fix chaining

* Move parseMepParam

* add additional optional chaining operator

Co-authored-by: Vivian A Goodrich <101133187+vgoodric@users.noreply.github.com>

* Refactor personalization.js

* Remove comment

* Add TargetManifestId

* Remove comment

* Add targetManifestId test

* Refactor addMepHighlightAndTargetManifestIdGnav

* Remove console.log from test

* Shorten function name

* Limits targetManifestId to target exp and adds missing name to  manifestId

* Fix test

* Handle default selectedVariant issue

* Mostly done

* Merging in prop display update

* Unit tests fixed

* Add targetmanifestid to useblockcode

* refactor to remove unnecessary import

* Remove commented code

* Removing unused functions

* Update tests, not including replacePage

* Shorten Function Name

* move insert inline back to fragment.js

* additional cleanup and unit tests

* Fix adobeTargetTestId issues

* add optional chaining

* Update condition for addAnalyticsForUseBlockCode

* Refactor to handle analytics for updatemetadata

* Fix useBlockCode test

* Fix merged merged test

* Remove applyPers from preload fragments test

* Fix martech init post-merge

* Fix tests and martech merge cleanup

* Dynamically import personalization.js

* add 2 ignore comments

* add conditional chaining

* Remove mwpw-146528 temp files

* MWPW-152275 Move manifest.json request up while waiting on sstats / etc. (#2608)

* add preload

* shorten if and and conditional chaining

* run loop even if Target is off

* Add nullish coalescing assignment suggestion

* MWPW-154998 [MEP][MILO] Manifests do not execute in the right order when there is a disabled manifest (#2616)

* add default execution order

* unit test

* normalizePath

* add normalizePath back into preload

* MWPW-155568 [MILO][MEP] If MEP is completely off, mep param should still show button (#2658)

don't return if param is present but empty

---------

Co-authored-by: markpadbe <markp@adobe.com>
Co-authored-by: Mark Perry <124626043+markpadbe@users.noreply.github.com>

MWPW-146030 and MWPW-143704 Add fresco-any and cc-paid-no-stock (#2657)

add entitlements

Attaching loadblock to window to provide alternative way of loading the script (#2660)

Attaching loadblock to window

MWPW-155316 - Fixing Pill Notification edge cases (#2639)

* handling pill edge cases

* pill edge case styles

* update tests to fix coverage

* correction to previous commit

* updates based on PR feedback

* more simplifying

* adding an additional filter

* style tweak

* change inline size for desktop pills

* revert of icon sizing

* revert of ribbon revert

Correctly pass on the dataset (#2651)

MWPW-155289: Fixed filtering and sorting in catalog sidenav (#2653)

handled empty hash case in deeplink

MWPW-154124: use preview index (#2654)

* MWPW-154124: use preview index

* should remove both links

[AUTOMATED-PR] Update imslib.min.js dependency (#2659)

Update self hosted dependency

Co-authored-by: GitHub Action <action@github.com>

[MWPW-152181] Tabbing gets "stuck" after second instance of merch-card CTAs, within a segment blade (#2650)

* Allow the default tab action to proceed if the current focused element is from the last merch card

* ran "npm run build"

* Updated readme

* fix vulnerability

* Revert "fix vulnerability"

This reverts commit e34d131.

* allow tabbing across section for merch cards

---------

Co-authored-by: Rohit Sahu <rosahu@adobe.com>

[MWPW-145131] Send locale and language code to analytics (#2670)

MWPW-147867 - [marquee] support RTL for loc - RePost (#2676)

added `support-rtl` selectors

MWPW-155691 [MILO][MEP] Also use region in placeholders decision  (#2680)

* use region for placeholders

* move region to it's own line

* move value to last and add other

MWPW-154901 Set debug=true on categories container (#2624)

* MWPW-154901: enables debug=true based on container type

* Update libs/blocks/caas/utils.js

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

---------

Co-authored-by: milo-pr-merge[bot] <169241390+milo-pr-merge[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

[MWPW-136406] Tabs block enhancement: deep linking (#2652)

* query param for focused tab

* addressed feedback

add lockup support

lockup improvements

updating button defaults

fix button size logic

update default btn size for banners

MWPW-154981: adds a new Created Date option to details text (#2685)

Co-authored-by: milo-pr-merge[bot] <169241390+milo-pr-merge[bot]@users.noreply.github.com>

MWPW-155781 [MILO] Geo Modal Tracking on BACOM - Missing custom link (#2694)

add analytic

MWPW-154898 No wait for modal fragment (#2607)

No wait for modal fragment

MWPW-154191: Ability to have green promo text on merch-card to bottom of the card (under description) (#2663)

added promo-text slot to all merch cards

MWPW-155654 Open TWP modal with preselected tab (#2691)

* MWPW-155654 Open TWP modal with preselected tab

* MWPW-155654 Open TWP modal with preselected tab

* MWPW-155654 Open TWP modal with preselected tab

* MWPW-155654 Open TWP modal with preselected tab

* MWPW-155654 Open TWP modal with preselected tab

* Trigger Build

* MWPW-155654 Open TWP modal with preselected tab

* MWPW-155654 Open TWP modal with preselected tab

* MWPW-155654 Open TWP modal with preselected tab

---------

Co-authored-by: Bozo Jovicic <bozo@hitthecode.com>

MWPW-154893 - [editorial-card] bugs (#2656)

* added .support-rtl variant for easier rtl loc support

* moved support-rtl to tablet up selector

* remove right

* removed margin-top bottom tablet styles

* added empty class and updated has-footer to equal-height to expose that as a variant

* s spacing gap default

* action column-gap

* no footer test coverage

* PR feedback

* revert marquee changes

* updated .footer class to .card-footer to prevent naming colision

* updated test ref

* linting whitespace, test label clarity

* revert icon block changes derp

ENB-6995 Query Params update & sendTargetResponse function update Resubmit for #2664 (#2682)

* Query Parms update & sendTargetResponse function update (#2664)

* Query Parms update & sendTargetResponse function update
- sendTargetResponse: Added Check for window._satellite and changed direct alloy call to _satellite call
- Added additional parameter as marketingtech to disable analytics calls

* Query Parms update & sendTargetResponse function update
- Updated conditional chaining

---------

Co-authored-by: milo-pr-merge[bot] <169241390+milo-pr-merge[bot]@users.noreply.github.com>

* Update martech.js

* not caching values

---------

Co-authored-by: theankit-sinha <ankitsinha@adobe.com>
Co-authored-by: milo-pr-merge[bot] <169241390+milo-pr-merge[bot]@users.noreply.github.com>

MWPW-153810 media image is seen loaded twice in product page urls in desktop (#2647)

* placeholderChanges

* placeholderChanges

* Update utils.js

* Update utils.js

* Update utils.js

* lint errors

* Update utils.js

* Update utils.js

* Adding promise.all

* Update utils.js

* Update utils.js

---------

Co-authored-by: Suhani <suhjain@suhanis-mbp.corp.adobe.com>

[MWPW-155627] Merch Callout feature: Missing font cascade (#2679)

add fallback to "Adobe Clean" font for callout text

Co-authored-by: Rohit Sahu <rosahu@adobe.com>

MWPW-153955 [PEP ] dismiss tool tip text is seen chopped in RTL locale in ipad and iphone in landscape mode (#2687)

* Fixed pep dismissal tooltip in rtl locales

* Update libs/features/webapp-prompt/webapp-prompt.css

Co-authored-by: Rares Munteanu <overmyheadandbody@gmail.com>

---------

Co-authored-by: Rares Munteanu <overmyheadandbody@gmail.com>

MWPW-154969 Log LCP and key user attributes (#2686)

* MWPW-154969 Log LCP and key user attributes

This adds the ability to log performance data back to Lana.

To enable the feature, metadata `pageperf = ‘on’ ` must be set for the page.  This feature only works in Chrome browsers.

Additionally `pageperf-rate` can be set to determine what percentage of pages will send data.  E.g `pageperf-rate=15` would have 15% of the pages send perf data.
The default rate is `50`%.

The sending of the data to Lana is delayed so as to not affect any parts of the page load.

Data sent:
* Chrome Version
* CLS Score
* Country (only if georouting is enabled via geo2)
* Downlink: Mbps - Chrome caps value at `10`
* LCP Score
* LCP Element Type
* LCP URL
  * If there is no identifying url, the element html will be used.
* Adobe IMS Logged In Status
* OS
* URL of page
* Window Width & Height
* MEP Manifest Data
  * Manifest(s) Used (only the ones that have been applied to the page)
  * Selected Target in the manifest

* Update logWebVitalsUtils.test.js

* Address PR feedback

---------

Co-authored-by: Okan Sahin <osahin@adobe.com>

[MWPW-150579] Update supported browser list (#2696)

MWPW-152952 Allow message for Marketo form success (#2545)

MWPW-146237: Repurpose seotech-structured-data feature for arbitrary JSON-LD (#2578)

* Add necessary helper functions

* Fix two checks

* Replace getStructuredData

* Move things around

* Fix regex?

* Add env

* Support localhost

* Update paths

* Fix test

* Use seotech api instead

* Update env/subdomain

* Fix test properly

* Update endoint

* Add homepage

* Update README

* Rework endpoint handling

* Add class to script tags

* More doc updates

revert  PR of placeholder function as placeholder value is seen displayed with additional nbsp text in locale product pages in stage (#2709)

Update utils.js

Standalone GNav for Non-Milo Consumers (#2669)

* Adding configuration for header component

* Adding test cases

* Adding css changes

* Updated for css

* Setting the origin to federal

* Removing promise array

* Adding redirect uri in meta

* Updating meta insert function

* Lint fix

* Making error message descriptive

---------

Co-authored-by: Snehal Sonawane <sonawane@Snehals-MacBook-Pro.local>

Making new GNAV experience default (#2689)

Fix: Load gnav search files only if it is authored (#2700)

[MWPW-154102] Accordion RTL alignment (#2704)

[MWPW-156163] Send additional Slack notifications for high impact PRs (#2713)

MWPW-156388 [MILO][MEP] fix for martech=off when page has manifest with enablements (#2720)

* initial fix

* Update libs/features/personalization/personalization.js

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* move where martech=off is used

* move logic to handleEntitlements function

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

MWPW-156220: Support pa code and product code for (#2712)

* MWPW-156220: Support pa code and product code for

checkout link configuration in addition to product family.

* optimize perf

* PR feedback

* PR feedback

Added download and open-in spectrum icons (#2595)

adde download and open-in spectrum icons

Co-authored-by: Dragana Trajkovic <dragana.trajkovic@lilly021.com>

MWPW-142166 One Page Gated (#2673)

* MWPW-142166 One Page Gated

* PR Changes

MWPW-153987 [MILO][MEP] looks in wrong location for MEP test block code on stage (#2693)

* calculate even on stage, find miloLibs and origin outside of loop

* MWPW-155314 [MEP] Enable Entitlements + Operator (#2638)

port over fixes

---------

Co-authored-by: Jingle Huang <32369333+JingleH@users.noreply.github.com>

[MWPW-154969] Performance consent (#2714)

* Require consent

* Improve the check

* Apply suggestions from code review

Co-authored-by: Narcis Radu <github@narcisradu.ro>

* Fix linting issues

* Fix linting and unit tests

* Return faster

* Track once consent is given

* Fix unit tests

* Remove defaults

* Only listen to privacy once.

---------

Co-authored-by: Narcis Radu <github@narcisradu.ro>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commerce Ready for Stage run-nala Run Nala Test Automation against PR verified PR has been E2E tested by a reviewer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants