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.

Sign up for GitHub

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

Jump to bottom

MWPW-143143: Improvements for Upgrade modal #2323

Merged
merged 1 commit into from
Jun 3, 2024
Merged

MWPW-143143: Improvements for Upgrade modal #2323

merged 1 commit into from
Jun 3, 2024

Conversation

3ch023
Copy link
Contributor

@3ch023 3ch023 commented May 16, 2024

Links

Resolves https://jira.corp.adobe.com/browse/MWPW-143143
Tacocat PR: https://git.corp.adobe.com/wcms/tacocat.js/pull/592

Summary

  • pass promo code, set on Upgrade Offer CTA to the Upgrade Modal
  • don't show the Upgrade modal, unless 'upgrade=true' flag is present on Merch Link
  • Upgrade modal to not depend on 'entitlement' flag (to enable the case when GWP needs a merch link to have Upgrade feature but not show the TWP/D2P modals).

How To Test

  • login on https://www.stage.adobe.com/ with an eligible test user
  • open test page
  • verify CTAs with 'upgrade=true' flag show modal
  • re-login with not eligible test user and verify all links lead to Checkout Page

image

Test URLs:

@3ch023 3ch023 requested a review from a team as a code owner May 16, 2024 13:41
@aem-code-sync AEM Code Sync
Copy link
Contributor

aem-code-sync bot commented May 16, 2024 •
edited
Loading

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

@aem-code-sync AEM Code Sync
Copy link
Contributor

aem-code-sync bot commented May 16, 2024 •
edited
Loading

Page Scores Audits Google
/drafts/mariia/pr/price-cta?georouting=off&martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@3ch023 3ch023 added run-nala Run Nala Test Automation against PR commerce labels May 16, 2024
@3ch023 3ch023 changed the title Upromo MWPW-143143: Improvements for Upgrade modal May 16, 2024
yesil
yesil reviewed May 16, 2024
View reviewed changes
libs/blocks/merch/merch.js
@@ -226,7 +226,7 @@ export async function getUpgradeAction(
imsSignedInPromise,
[{ productArrangement: { productFamily: offerFamily } = {} }],
) {
if (options.entitlement === false) return undefined;
if (!options.upgrade) 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.

don't we need both flags here?

Copy link
Contributor Author

@3ch023 3ch023 May 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

i was thinking about it, but then decided to remove 'entitlement' to have more flexibility.

  • entitlement=true&upgrade=true: users will have both Download button and upgrade modal experience, depending on their bought products
  • entitlement=true&upgrade=false: users will have only Download button flow
  • entitlement=false&upgrade=true: users will have only the Upgrade modal, but no Download flow
  • entitlement=false&upgrade=false: only checkout page

Copy link
Contributor

@yesil yesil May 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

entitlement=true&upgrade=true: users will have both TWP modal and upgrade modal experience

actually this is not correct, you need the modal=true

@codecov Codecov
Copy link

codecov bot commented May 16, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.66%. Comparing base (2ebb018) to head (4b881b4).

Additional details and impacted files 
@@            Coverage Diff             @@
##            stage    #2323      +/-   ##
==========================================
- Coverage   95.67%   95.66%   -0.01%     
==========================================
  Files         173      173              
  Lines       45204    45209       +5     
==========================================
+ Hits        43248    43251       +3     
- Misses       1956     1958       +2

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

npeltier
npeltier approved these changes May 16, 2024
View reviewed changes
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.

lgtm

@Roycethan Roycethan added the verified PR has been E2E tested by a reviewer label May 17, 2024
@Roycethan
Copy link

@3ch023 Verified

@github-actions GitHub Actions
Copy link
Contributor

Reminder to set the Ready for Stage label - to queue this to get merged to stage & production.

@3ch023
Copy link
Contributor Author

3ch023 commented May 17, 2024

@3ch023 Verified

thank you @Roycethan !

the process changed a bit now, so only the QA should be setting the 'Ready for Stage' label (i guess instead of 'verified' or together with 'verified')
i will break the rules a bit today and set it myself, since you verified it :)

@3ch023 3ch023 added Ready for Stage do not merge PR should not be merged yet and removed Ready for Stage labels May 17, 2024
@3ch023
Copy link
Contributor Author

3ch023 commented May 17, 2024

Adding 'do not merge', since this PR also is blocked by Optical Price audit

@github-actions GitHub Actions
Copy link
Contributor

This PR has not been updated recently and will be closed in 7 days if no action is taken. Please ensure all checks are passing, https://github.com/orgs/adobecom/discussions/997 provides instructions. If the PR is ready to be merged, please mark it with the "Ready for Stage" label.

@github-actions github-actions bot added the Stale label May 25, 2024
@3ch023 3ch023 removed the Stale label May 27, 2024
@3ch023 3ch023 removed the do not merge PR should not be merged yet label May 27, 2024
@milo-pr-merge milo-pr-merge bot merged commit f5b2b48 into stage Jun 3, 2024
11 checks passed
@milo-pr-merge milo-pr-merge bot deleted the upromo branch June 3, 2024 16:12
@milo-pr-merge milo-pr-merge bot mentioned this pull request Jun 3, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mokimo mokimo mokimo approved these changes

@npeltier npeltier npeltier approved these changes

@yesil yesil yesil approved these changes

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@3ch023 @Roycethan @yesil @npeltier @mokimo @afmicka