Skip to content

feat: Contentful banner integration #15731

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

Merged
merged 6 commits into from
May 29, 2025
Merged

Conversation

mcbridemarci
Copy link
Contributor

Description

This PR integrates Contentful into the mobile carousel, allowing promotional banners to be managed remotely rather than hardcoded in the app. Previously, all banners were statically defined in code. Now, they are dynamically fetched from Contentful, filtered based on activity dates, and rendered alongside predefined banners.

To keep the experience streamlined, I’ve also introduced a limit of 15 visible banners. This is an aesthetic decision to prevent the UI from being overwhelmed if too many are returned from Contentful.

The goal is to align the mobile experience with the existing MetaMask Extension Promotional banner and improve flexibility for teams.

Manual testing steps

  1. Launch the app
  2. Make sure the LaunchDarkly flag is enabled.
  3. Observe banners fetched from Contentful are displayed. (Contentful Link)
  4. Confirm hardcoded banners are still there and contentful banners are appended after.
  5. Verify that banners beyond 15 are not displayed
  6. Confirm that startDate and endDate are respected (These are optional fields)

Screenshots/Recordings

Before

Screenshot 2025-05-27 at 5 02 46 PM

After

Screenshot 2025-05-27 at 5 03 34 PM

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Copy link
Contributor

github-actions bot commented May 27, 2025

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label May 27, 2025
@mcbridemarci
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

Copy link

socket-security bot commented May 27, 2025

All alerts resolved. Learn more about Socket for GitHub.

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

Ignoring alerts on:

  • npm/contentful@11.5.23

View full report

@mcbridemarci mcbridemarci force-pushed the contentful-banner-integration branch 2 times, most recently from 41c11a5 to e611b9a Compare May 27, 2025 17:33
@mcbridemarci mcbridemarci removed the INVALID-PR-TEMPLATE PR's body doesn't match template label May 27, 2025
@mcbridemarci mcbridemarci marked this pull request as ready for review May 28, 2025 13:15
@mcbridemarci mcbridemarci force-pushed the contentful-banner-integration branch from e611b9a to e80e9fb Compare May 29, 2025 10:04
@mcbridemarci mcbridemarci added type-enhancement New feature or request Spot Check on the Release Build If a ticket doesn't require feature QA, but does require some form of manual spot checking team-phosphor labels May 29, 2025
@mcbridemarci mcbridemarci force-pushed the contentful-banner-integration branch from e80e9fb to 607b12b Compare May 29, 2025 10:14
@Prithpal-Sooriya Prithpal-Sooriya requested a review from a team as a code owner May 29, 2025 12:46
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 73.26733% with 27 lines in your changes missing coverage. Please review.

Project coverage is 69.81%. Comparing base (858d71f) to head (86e81ef).
Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
...s/UI/Carousel/fetchCarouselSlidesFromContentful.ts 80.39% 2 Missing and 8 partials ⚠️
app/components/UI/Carousel/index.tsx 71.87% 7 Missing and 2 partials ⚠️
...onents/UI/Carousel/selectors/featureFlags/index.ts 50.00% 5 Missing and 2 partials ⚠️
app/components/UI/Carousel/constants.ts 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #15731      +/-   ##
==========================================
+ Coverage   69.64%   69.81%   +0.16%     
==========================================
  Files        2483     2519      +36     
  Lines       53212    53789     +577     
  Branches     8139     8251     +112     
==========================================
+ Hits        37058    37551     +493     
- Misses      13760    13801      +41     
- Partials     2394     2437      +43     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Prithpal-Sooriya Prithpal-Sooriya added the Run Smoke E2E Requires smoke E2E testing label May 29, 2025
Copy link
Contributor

github-actions bot commented May 29, 2025

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 4e51c82
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/caa77c20-f40b-4977-a451-be915e1a87ba

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

@Prithpal-Sooriya
Copy link
Contributor

@SocketSecurity ignore npm/contentful@11.5.23

We are using the contentful package for better fetching management of contentful content. If this is a blocker, we can fallback to just using the API

@Prithpal-Sooriya Prithpal-Sooriya changed the title Contentful banner integration feat: Contentful banner integration May 29, 2025
@metamaskbot metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label May 29, 2025
@Prithpal-Sooriya Prithpal-Sooriya added No E2E Smoke Needed If the PR does not need E2E smoke test run and removed Run Smoke E2E Requires smoke E2E testing labels May 29, 2025
@Prithpal-Sooriya
Copy link
Contributor

Looking at the e2e failures, it is a known issue with confirmation smoke tests.
Related failure here: #15815

I am confident that these changes do not touch the confirmation smoke tests.

@mcbridemarci mcbridemarci added the QA Passed QA testing has been completed and passed label May 29, 2025
@mcbridemarci mcbridemarci force-pushed the contentful-banner-integration branch from 4e51c82 to fd85e11 Compare May 29, 2025 17:17
@mcbridemarci mcbridemarci enabled auto-merge May 29, 2025 17:41
Copy link

@mcbridemarci mcbridemarci added this pull request to the merge queue May 29, 2025
Merged via the queue into main with commit 50f9bf9 May 29, 2025
41 checks passed
@mcbridemarci mcbridemarci deleted the contentful-banner-integration branch May 29, 2025 18:29
@github-actions github-actions bot locked and limited conversation to collaborators May 29, 2025
@metamaskbot metamaskbot added the release-7.48.0 Issue or pull request that will be included in release 7.48.0 label May 29, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
INVALID-PR-TEMPLATE PR's body doesn't match template No E2E Smoke Needed If the PR does not need E2E smoke test run QA Passed QA testing has been completed and passed release-7.48.0 Issue or pull request that will be included in release 7.48.0 Spot Check on the Release Build If a ticket doesn't require feature QA, but does require some form of manual spot checking team-phosphor type-enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants