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-153611] [Gray Box] environment aware links #2622

Merged
merged 2 commits into from
Jul 29, 2024

Conversation

robert-bogos
Copy link
Contributor

@robert-bogos robert-bogos commented Jul 24, 2024

Description

Links on Graybox pages need to be environment aware.

Examples of desired links conversions:
When on https://pocone.graybox.adobe.com/some-path:
https://www.adobe.com/ -> https://pocone.graybox.adobe.com/
When on https://max24.business-graybox.adobe.com/some-path
https://business.adobe.com/ -> https://max24.business-graybox.adobe.com/

where
pocone / max are graybox environment names. These can be anything.
graybox / business-graybox are the names of the graybox environments sharepoint container folder. Each supported Milo consumer folder will have a –graybox variant.

Related Issue

Resolves: MWPW-153611

Testing instructions

1. go to the test URL
2. using the developer tools, add a breakpoint in scripts.js, where LIBS is defined.

Screenshot 2024-07-24 at 20 37 23

3. refresh the page, when the execution hits the break point. Overwrite, using the console, the setLibs function with () => 'https://mwpw-153611-convert-graybox-urls--milo--robert-bogos.hlx.live/libs'

image

4. resume the execution
5. scroll down on the page and inspect the Link to be converted link. It should send to https://pocone.business-graybox.adobe.com/...

Screenshot 2024-07-24 at 20 45 27

if you don't overwrite the LIBS, the Link to be converted link will send to https://business.adobe.com/...

Test URL

https://pocone.business-graybox.adobe.com/customer-success-stories/graybox-links-conversion.html

URL for PSI check: (irrelevant for testing the changes)

https://mwpw-153611-convert-graybox-urls--milo--robert-bogos.hlx.page/drafts/rbogos/page-default?martech=off

@robert-bogos robert-bogos requested review from arshadparwaiz, sukamat and a team July 24, 2024 17:49
@robert-bogos robert-bogos self-assigned this Jul 24, 2024
Copy link
Contributor

aem-code-sync bot commented Jul 24, 2024

Page Scores Audits Google
/libs PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
/drafts/rbogos/page-default?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@adobecom adobecom deleted a comment from codecov bot Jul 24, 2024
Copy link

codecov bot commented Jul 24, 2024

Codecov Report

Attention: Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Project coverage is 95.74%. Comparing base (543d9a8) to head (ecfe9e5).
Report is 5 commits behind head on stage.

Files Patch % Lines
libs/blocks/graybox/graybox.js 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            stage    #2622      +/-   ##
==========================================
+ Coverage   95.73%   95.74%   +0.01%     
==========================================
  Files         172      172              
  Lines       45392    45401       +9     
==========================================
+ Hits        43457    43471      +14     
+ Misses       1935     1930       -5     

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

Copy link
Contributor

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.

@robert-bogos
Copy link
Contributor Author

Could not find a proper way to unit test this as window.location cannot be mocked

Copy link
Contributor

@overmyheadandbody overmyheadandbody left a comment

Choose a reason for hiding this comment

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

I'd guess my questions are outside the scope of this story, but async sections might become a topic if not already considered.

libs/blocks/graybox/graybox.js Show resolved Hide resolved
libs/blocks/graybox/graybox.js Show resolved Hide resolved
@SilviuLCF SilviuLCF self-requested a review July 29, 2024 07:13
Copy link

@SilviuLCF SilviuLCF left a comment

Choose a reason for hiding this comment

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

@SilviuLCF SilviuLCF added verified PR has been E2E tested by a reviewer Ready for Stage labels Jul 29, 2024
@milo-pr-merge
Copy link
Contributor

milo-pr-merge bot commented Jul 29, 2024

Skipped merging 2622: [MWPW-153611] [Gray Box] environment aware links due to failing checks

@mokimo mokimo merged commit c00c777 into adobecom:stage Jul 29, 2024
19 of 20 checks passed
@mokimo mokimo mentioned this pull request Jul 29, 2024
rohitsahu pushed a commit to rohitsahu/milo that referenced this pull request Jul 30, 2024
* stage:
  [MWPW-153611] [Gray Box] environment aware links (adobecom#2622)
  MWPW-153580: Add Opt-In Feature for CaaS Badge Display (adobecom#2625)
  [MWPW-154335] [callout] Spacing issue encountered when the call-out section is added (adobecom#2628)
  MWPW-150557 - Split Marquee CLS issues on consuming sites (adobecom#2636)
  Mwpw 147034: Custom border color + badge/border color decoupling [merch card] (adobecom#2613)
  [MWPW-151517] - Remove condition for promobar hidden on mobile from gnav (adobecom#2538)
  MWPW-154998 [MEP][MILO] Manifests do not execute in the right order when there is a disabled manifest (adobecom#2632)
  mwpw-154965: Fetch federal stage content from hlx.page instead of stage.adobe.com (adobecom#2618)
  Correct error messages for duplicate files on the stage to main workflow (adobecom#2621)
  MWPW-153245 [merch][analytics] dispatch wcomp events, and let default lh (adobecom#2610)
  Revert "MWPW-146528[MILO][MEP][ANALYTICS] Add attribute to content changed by Target for analytics and MWPW-152274" (adobecom#2627)
  MWPW-128600 Locale Tool: Langstore points to langstore/en (adobecom#2615)
  Fix for errors in dynamically loaded scripts in test cases (adobecom#2619)
  MWPW-146528[MILO][MEP][ANALYTICS] Add attribute to content changed by Target for analytics and MWPW-152274 (adobecom#2593)
  Bootstrapper script for milo feds blocks (adobecom#2560)
  Revert "[MWPW-152968] mWeb - Passing ECID to Branch.io banner - Implementation" (adobecom#2612)

# Conflicts:
#	libs/deps/merch-card.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready for Stage verified PR has been E2E tested by a reviewer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants