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

Prepare DCR for an AB test between user benefits api & members data api #13113

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

rupertbates
Copy link
Member

@rupertbates rupertbates commented Jan 9, 2025

What does this change?

Extracts the logic relating to the members-data-api from the user features code

Why?

We want to run an AB test where we use the new user-benefits api instead of members-data-api to work out whether a user is entitled to ad-free and hidden support messaging.

The point of the AB test is just to roll out the new service in a gradual way so that we can manage the scale effectively.

This PR does some final refactoring of the existing code to make it easier to run this test, but because the changes are relatively extensive I'm putting it in as a stand alone PR.

Screenshots

Before After
before after

@rupertbates rupertbates requested review from a team as code owners January 9, 2025 12:03
Copy link

github-actions bot commented Jan 9, 2025

Hello 👋! When you're ready to run Chromatic, please apply the run_chromatic label to this PR.

You will need to reapply the label each time you want to run Chromatic.

Click here to see the Chromatic project.

@rupertbates rupertbates marked this pull request as draft January 9, 2025 12:05
Copy link

github-actions bot commented Jan 9, 2025

Size Change: -79 B (-0.01%)

Total Size: 870 kB

ℹ️ View Unchanged
Filename Size Change
dotcom-rendering/dist/1076.client.web.********************.js 3.41 kB 0 B
dotcom-rendering/dist/1262.client.web.********************.js 4.49 kB 0 B
dotcom-rendering/dist/1301.client.web.********************.js 4.82 kB 0 B
dotcom-rendering/dist/1401.client.web.********************.js 441 B 0 B
dotcom-rendering/dist/1477.client.web.********************.js 3.52 kB 0 B
dotcom-rendering/dist/1679.client.web.********************.js 2.49 kB 0 B
dotcom-rendering/dist/1714.client.web.********************.js 2.87 kB 0 B
dotcom-rendering/dist/1891.client.web.********************.js 3.83 kB 0 B
dotcom-rendering/dist/2188.client.web.********************.js 6.52 kB 0 B
dotcom-rendering/dist/2444.client.web.********************.js 2.67 kB 0 B
dotcom-rendering/dist/2482.client.web.********************.js 44.8 kB 0 B
dotcom-rendering/dist/280.client.web.********************.js 531 B 0 B
dotcom-rendering/dist/3213.client.web.********************.js 5.42 kB 0 B
dotcom-rendering/dist/342.client.web.********************.js 4.18 kB 0 B
dotcom-rendering/dist/3524.client.web.********************.js 3.51 kB 0 B
dotcom-rendering/dist/3769.client.web.********************.js 22.7 kB 0 B
dotcom-rendering/dist/3789.client.web.********************.js 3.58 kB 0 B
dotcom-rendering/dist/39.client.web.********************.js 3.06 kB 0 B
dotcom-rendering/dist/3937.client.web.********************.js 3.85 kB 0 B
dotcom-rendering/dist/4020.client.web.********************.js 14.1 kB 0 B
dotcom-rendering/dist/4170.client.web.********************.js 16.3 kB 0 B
dotcom-rendering/dist/4237.client.web.********************.js 3.22 kB 0 B
dotcom-rendering/dist/4285.client.web.********************.js 6.12 kB 0 B
dotcom-rendering/dist/4501.client.web.********************.js 4.29 kB 0 B
dotcom-rendering/dist/4684.client.web.********************.js 3.17 kB 0 B
dotcom-rendering/dist/4791.client.web.********************.js 4.94 kB 0 B
dotcom-rendering/dist/5095.client.web.********************.js 4.17 kB 0 B
dotcom-rendering/dist/5598.client.web.********************.js 4.49 kB 0 B
dotcom-rendering/dist/5721.client.web.********************.js 3.64 kB 0 B
dotcom-rendering/dist/5922.client.web.********************.js 8.08 kB 0 B
dotcom-rendering/dist/6021.client.web.********************.js 11 kB 0 B
dotcom-rendering/dist/6061.client.web.********************.js 3.63 kB 0 B
dotcom-rendering/dist/6073.client.web.********************.js 3.53 kB 0 B
dotcom-rendering/dist/6080.client.web.********************.js 3.27 kB 0 B
dotcom-rendering/dist/6627.client.web.********************.js 10.3 kB 0 B
dotcom-rendering/dist/6659.client.web.********************.js 3.58 kB 0 B
dotcom-rendering/dist/6876.client.web.********************.js 2.67 kB 0 B
dotcom-rendering/dist/6882.client.web.********************.js 12.8 kB 0 B
dotcom-rendering/dist/6903.client.web.********************.js 3.21 kB 0 B
dotcom-rendering/dist/6931.client.web.********************.js 2.56 kB 0 B
dotcom-rendering/dist/6940.client.web.********************.js 526 B 0 B
dotcom-rendering/dist/7026.client.web.********************.js 5.41 kB 0 B
dotcom-rendering/dist/7116.client.web.********************.js 23 kB 0 B
dotcom-rendering/dist/719.client.web.********************.js 3.48 kB 0 B
dotcom-rendering/dist/7350.client.web.********************.js 3.32 kB 0 B
dotcom-rendering/dist/7513.client.web.********************.js 157 B 0 B
dotcom-rendering/dist/7540.client.web.********************.js 2.72 kB 0 B
dotcom-rendering/dist/7546.client.web.********************.js 7.36 kB 0 B
dotcom-rendering/dist/7861.client.web.********************.js 617 B 0 B
dotcom-rendering/dist/8030.client.web.********************.js 4.18 kB 0 B
dotcom-rendering/dist/8067.client.web.********************.js 3.39 kB 0 B
dotcom-rendering/dist/895.client.web.********************.js 5.14 kB 0 B
dotcom-rendering/dist/9242.client.web.********************.js 3.75 kB 0 B
dotcom-rendering/dist/9288.client.web.********************.js 2.51 kB 0 B
dotcom-rendering/dist/9558.client.web.********************.js 3.53 kB 0 B
dotcom-rendering/dist/9665.client.web.********************.js 4.04 kB 0 B
dotcom-rendering/dist/9735.client.web.********************.js 4.46 kB 0 B
dotcom-rendering/dist/9766.client.web.********************.js 3.4 kB 0 B
dotcom-rendering/dist/9771.client.web.********************.js 3.69 kB 0 B
dotcom-rendering/dist/9995.client.web.********************.js 20.3 kB 0 B
dotcom-rendering/dist/Accessibility-importable.client.web.********************.js 6.7 kB 0 B
dotcom-rendering/dist/AdBlockAsk-importable.client.web.********************.js 2.85 kB -1 B (-0.04%)
dotcom-rendering/dist/AdPortals-importable.client.web.********************.js 4.85 kB 0 B
dotcom-rendering/dist/AlreadyVisited-importable.client.web.********************.js 423 B 0 B
dotcom-rendering/dist/AppsEpic-importable.client.web.********************.js 3.63 kB 0 B
dotcom-rendering/dist/AppsFooter-importable.client.web.********************.js 2.7 kB 0 B
dotcom-rendering/dist/AppsLightboxImage-importable.client.web.********************.js 2.66 kB 0 B
dotcom-rendering/dist/AppsLightboxImageStore-importable.client.web.********************.js 2.55 kB 0 B
dotcom-rendering/dist/AudioAtomWrapper-importable.client.web.********************.js 2.59 kB 0 B
dotcom-rendering/dist/AudioPlayerWrapper-importable.client.web.********************.js 6.32 kB 0 B
dotcom-rendering/dist/AustralianTerritorySwitcher-importable.client.web.********************.js 2 kB 0 B
dotcom-rendering/dist/Branding-importable.client.web.********************.js 2.88 kB 0 B
dotcom-rendering/dist/braze-web-sdk-core.client.web.********************.js 37.2 kB 0 B
dotcom-rendering/dist/BrazeMessaging-importable.client.web.********************.js 1.96 kB 0 B
dotcom-rendering/dist/CalloutBlockComponent-importable.client.web.********************.js 6.75 kB 0 B
dotcom-rendering/dist/CalloutEmbedBlockComponent-importable.client.web.********************.js 5.77 kB 0 B
dotcom-rendering/dist/CardCommentCount-importable.client.web.********************.js 2.66 kB 0 B
dotcom-rendering/dist/Carousel-importable.client.web.********************.js 7.03 kB 0 B
dotcom-rendering/dist/CarouselForNewsletters-importable.client.web.********************.js 5.14 kB 0 B
dotcom-rendering/dist/ChartAtom-importable.client.web.********************.js 538 B 0 B
dotcom-rendering/dist/CommentCount-importable.client.web.********************.js 2.29 kB 0 B
dotcom-rendering/dist/DiscussionApps-importable.client.web.********************.js 1.93 kB 0 B
dotcom-rendering/dist/DiscussionMeta-importable.client.web.********************.js 2.44 kB 0 B
dotcom-rendering/dist/DiscussionWeb-importable.client.web.********************.js 1.74 kB 0 B
dotcom-rendering/dist/DocumentBlockComponent-importable.client.web.********************.js 2.82 kB 0 B
dotcom-rendering/dist/Dropdown-importable.client.web.********************.js 1.72 kB 0 B
dotcom-rendering/dist/EditionSwitcherBanner-importable.client.web.********************.js 3.49 kB 0 B
dotcom-rendering/dist/EmbedBlockComponent-importable.client.web.********************.js 3.94 kB 0 B
dotcom-rendering/dist/EnhancePinnedPost-importable.client.web.********************.js 2.02 kB 0 B
dotcom-rendering/dist/FetchOnwardsData-importable.client.web.********************.js 1.93 kB 0 B
dotcom-rendering/dist/FilterKeyEventsToggle-importable.client.web.********************.js 3.8 kB 0 B
dotcom-rendering/dist/FocusStyles-importable.client.web.********************.js 617 B 0 B
dotcom-rendering/dist/FollowWrapper-importable.client.web.********************.js 2.52 kB 0 B
dotcom-rendering/dist/FooterLabel-importable.client.web.********************.js 343 B 0 B
dotcom-rendering/dist/FooterReaderRevenueLinks-importable.client.web.********************.js 3.5 kB 0 B
dotcom-rendering/dist/frameworks.client.web.********************.js 20.9 kB 0 B
dotcom-rendering/dist/FrontSubNav-importable.client.web.********************.js 7.37 kB 0 B
dotcom-rendering/dist/GetCricketScoreboard-importable.client.web.********************.js 6.26 kB 0 B
dotcom-rendering/dist/GetMatchNav-importable.client.web.********************.js 11.4 kB 0 B
dotcom-rendering/dist/GetMatchStats-importable.client.web.********************.js 7.97 kB 0 B
dotcom-rendering/dist/GetMatchTabs-importable.client.web.********************.js 2.58 kB 0 B
dotcom-rendering/dist/guardian-braze-components-banner.client.web.********************.js 15.8 kB 0 B
dotcom-rendering/dist/guardian-braze-components-end-of-article.client.web.********************.js 10.2 kB 0 B
dotcom-rendering/dist/GuideAtomWrapper-importable.client.web.********************.js 783 B 0 B
dotcom-rendering/dist/index.client.web.********************.js 44.7 kB -78 B (-0.17%)
dotcom-rendering/dist/InstagramBlockComponent-importable.client.web.********************.js 2.9 kB 0 B
dotcom-rendering/dist/InteractiveAtomMessenger-importable.client.web.********************.js 851 B 0 B
dotcom-rendering/dist/InteractiveBlockComponent-importable.client.web.********************.js 8.49 kB 0 B
dotcom-rendering/dist/InteractiveContentsBlockComponent-importable.client.web.********************.js 3.74 kB 0 B
dotcom-rendering/dist/KeyEventsCarousel-importable.client.web.********************.js 5.68 kB 0 B
dotcom-rendering/dist/KnowledgeQuizAtom-importable.client.web.********************.js 3.48 kB 0 B
dotcom-rendering/dist/LatestLinks-importable.client.web.********************.js 6.38 kB 0 B
dotcom-rendering/dist/LightboxHash-importable.client.web.********************.js 434 B 0 B
dotcom-rendering/dist/LightboxLayout-importable.client.web.********************.js 6.53 kB 0 B
dotcom-rendering/dist/LiveBlogEpic-importable.client.web.********************.js 3.55 kB 0 B
dotcom-rendering/dist/LiveblogNotifications-importable.client.web.********************.js 4.82 kB 0 B
dotcom-rendering/dist/Liveness-importable.client.web.********************.js 4.72 kB 0 B
dotcom-rendering/dist/ManyNewsletterSignUp-importable.client.web.********************.js 7.61 kB 0 B
dotcom-rendering/dist/MapEmbedBlockComponent-importable.client.web.********************.js 5.89 kB 0 B
dotcom-rendering/dist/Metrics-importable.client.web.********************.js 2.69 kB 0 B
dotcom-rendering/dist/MostViewedFooter-importable.client.web.********************.js 3.83 kB 0 B
dotcom-rendering/dist/MostViewedFooterData-importable.client.web.********************.js 6.07 kB 0 B
dotcom-rendering/dist/MostViewedRightWithAd-importable.client.web.********************.js 5 kB 0 B
dotcom-rendering/dist/OnwardsUpper-importable.client.web.********************.js 5.32 kB 0 B
dotcom-rendering/dist/PersonalityQuizAtom-importable.client.web.********************.js 3.65 kB 0 B
dotcom-rendering/dist/ProfileAtom-importable.client.web.********************.js 543 B 0 B
dotcom-rendering/dist/ProfileAtomWrapper-importable.client.web.********************.js 802 B 0 B
dotcom-rendering/dist/PulsingDot-importable.client.web.********************.js 750 B 0 B
dotcom-rendering/dist/QandaAtom-importable.client.web.********************.js 543 B 0 B
dotcom-rendering/dist/ReaderRevenueDev-importable.client.web.********************.js 468 B 0 B
dotcom-rendering/dist/readerRevenueDevUtils.client.web.********************.js 1.74 kB 0 B
dotcom-rendering/dist/RelativeTime-importable.client.web.********************.js 2.53 kB 0 B
dotcom-rendering/dist/RichLinkComponent-importable.client.web.********************.js 6.01 kB 0 B
dotcom-rendering/dist/ScrollableFeature-importable.client.web.********************.js 7.07 kB 0 B
dotcom-rendering/dist/ScrollableHighlights-importable.client.web.********************.js 6.19 kB 0 B
dotcom-rendering/dist/ScrollableMedium-importable.client.web.********************.js 4.4 kB 0 B
dotcom-rendering/dist/ScrollableSmall-importable.client.web.********************.js 4.37 kB 0 B
dotcom-rendering/dist/SecureSignup-importable.client.web.********************.js 4.1 kB 0 B
dotcom-rendering/dist/SendTargetingParams-importable.client.web.********************.js 2.22 kB 0 B
dotcom-rendering/dist/sentry.client.web.********************.js 794 B 0 B
dotcom-rendering/dist/SetABTests-importable.client.web.********************.js 3.69 kB 0 B
dotcom-rendering/dist/SetAdTargeting-importable.client.web.********************.js 485 B 0 B
dotcom-rendering/dist/ShareButton-importable.client.web.********************.js 919 B 0 B
dotcom-rendering/dist/shimport.client.web.********************.js 2.8 kB 0 B
dotcom-rendering/dist/ShowHideContainers-importable.client.web.********************.js 730 B 0 B
dotcom-rendering/dist/ShowMore-importable.client.web.********************.js 2.1 kB 0 B
dotcom-rendering/dist/SignInGateMain.client.web.********************.js 4.46 kB 0 B
dotcom-rendering/dist/SignInGateMainCheckoutComplete.client.web.********************.js 5.56 kB 0 B
dotcom-rendering/dist/SignInGateSelector-importable.client.web.********************.js 5.83 kB 0 B
dotcom-rendering/dist/SlideshowCarousel-importable.client.web.********************.js 4.37 kB 0 B
dotcom-rendering/dist/SlotBodyEnd-importable.client.web.********************.js 4.84 kB 0 B
dotcom-rendering/dist/SpotifyBlockComponent-importable.client.web.********************.js 5.71 kB 0 B
dotcom-rendering/dist/StickyBottomBanner-importable.client.web.********************.js 6.15 kB 0 B
dotcom-rendering/dist/StickyLiveblogAskWrapper-importable.client.web.********************.js 8.13 kB 0 B
dotcom-rendering/dist/SubNav-importable.client.web.********************.js 2.41 kB 0 B
dotcom-rendering/dist/TableOfContents-importable.client.web.********************.js 3.48 kB 0 B
dotcom-rendering/dist/TimelineAtom-importable.client.web.********************.js 1.23 kB 0 B
dotcom-rendering/dist/Titlepiece-importable.client.web.********************.js 13.5 kB 0 B
dotcom-rendering/dist/TopBar-importable.client.web.********************.js 9.29 kB 0 B
dotcom-rendering/dist/TopBarSupport-importable.client.web.********************.js 2.49 kB 0 B
dotcom-rendering/dist/TweetBlockComponent-importable.client.web.********************.js 1.13 kB 0 B
dotcom-rendering/dist/UnsafeEmbedBlockComponent-importable.client.web.********************.js 2.91 kB 0 B
dotcom-rendering/dist/VideoFacebookBlockComponent-importable.client.web.********************.js 5.9 kB 0 B
dotcom-rendering/dist/VineBlockComponent-importable.client.web.********************.js 2.78 kB 0 B
dotcom-rendering/dist/YoutubeBlockComponent-importable.client.web.********************.js 4.38 kB 0 B

compressed-size-action

@rupertbates rupertbates force-pushed the use-user-benefits-api branch 2 times, most recently from e9c81aa to 2ac27fb Compare January 9, 2025 15:58
@rupertbates rupertbates force-pushed the use-user-benefits-api branch from 2ac27fb to 9122d78 Compare January 9, 2025 16:02
@rupertbates rupertbates added the run_chromatic Runs chromatic when label is applied label Jan 9, 2025
@github-actions github-actions bot removed the run_chromatic Runs chromatic when label is applied label Jan 9, 2025
@rupertbates rupertbates changed the title Use user benefits api Prepare DCR for an Use user benefits api Jan 10, 2025
@rupertbates rupertbates changed the title Prepare DCR for an Use user benefits api Prepare DCR for an AB test between user benefits api & members data api Jan 10, 2025
@rupertbates rupertbates marked this pull request as ready for review January 10, 2025 11:49
expires.setMonth(expires.getMonth() + 6);
setCookie({
name: AD_FREE_USER_COOKIE,
value: expires.getTime().toString(),
Copy link
Contributor

Choose a reason for hiding this comment

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

What does the expires value mean if the cookie is valid for daysToLive?

Copy link
Member Author

Choose a reason for hiding this comment

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

It does seem a bit odd doesn't it? This logic was already there so I left it in place. I don't think it will hurt although it may well be redundant

Copy link
Member

@tjmw tjmw left a comment

Choose a reason for hiding this comment

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

👍 Nice

removeCookie({ name: AD_FREE_USER_COOKIE });
removeCookie({ name: USER_FEATURES_EXPIRY_COOKIE });
removeCookie({ name: HIDE_SUPPORT_MESSAGING_COOKIE });
return Promise.resolve();
Copy link
Member

Choose a reason for hiding this comment

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

I think because this function is async the return value will always be a promise, so this could just be

Suggested change
return Promise.resolve();
return;

(or could even be omitted completely).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants