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

Remove some cookies set by the checkout #6714

Merged
merged 4 commits into from
Feb 12, 2025

Conversation

tjmw
Copy link
Member

@tjmw tjmw commented Jan 22, 2025

What are you doing in this PR?

I've removed the following as they're no longer needed:

  • gu_recurring_contributor
  • gu_digital_subscriber
  • gu.contributions.recurring.contrib-timestamp.<billing period>

I've refactored the cookie setting code a bit to consolidate things and make it clear where products set the same cookie(s).

Trello Card

Why are you doing this?

These cookies are no longer needed.

How to test

Put through a purchase, see that these cookies are no longer set.

@tjmw tjmw requested a review from rupertbates January 22, 2025 12:34
Copy link
Contributor

github-actions bot commented Jan 22, 2025

Size Change: 0 B

Total Size: 1.9 MB

ℹ️ View Unchanged
Filename Size
./public/compiled-assets/javascripts/[countryGroupId]/events/router.js 90.4 kB
./public/compiled-assets/javascripts/[countryGroupId]/router.js 93.6 kB
./public/compiled-assets/javascripts/ausMomentMap.js 108 kB
./public/compiled-assets/javascripts/contributionsRedirectStyles.js 20 B
./public/compiled-assets/javascripts/digitalSubscriptionLandingPage.js 223 kB
./public/compiled-assets/javascripts/downForMaintenancePage.js 67.3 kB
./public/compiled-assets/javascripts/error404Page.js 67.3 kB
./public/compiled-assets/javascripts/error500Page.js 67.2 kB
./public/compiled-assets/javascripts/favicons.js 617 B
./public/compiled-assets/javascripts/paperSubscriptionCheckoutPage.js 163 kB
./public/compiled-assets/javascripts/paperSubscriptionLandingPage.js 87.7 kB
./public/compiled-assets/javascripts/payPalErrorPage.js 65.9 kB
./public/compiled-assets/javascripts/payPalErrorPageStyles.js 20 B
./public/compiled-assets/javascripts/promotionTerms.js 73.7 kB
./public/compiled-assets/javascripts/subscriptionsLandingPage.js 78 kB
./public/compiled-assets/javascripts/subscriptionsRedemptionPage.js 118 kB
./public/compiled-assets/javascripts/unsupportedBrowserStyles.js 20 B
./public/compiled-assets/javascripts/weeklySubscriptionCheckoutPage.js 160 kB
./public/compiled-assets/javascripts/weeklySubscriptionLandingPage.js 88.3 kB
./public/compiled-assets/webpack/114.js 12.2 kB
./public/compiled-assets/webpack/127.js 3.53 kB
./public/compiled-assets/webpack/136.js 2.17 kB
./public/compiled-assets/webpack/163.js 8.9 kB
./public/compiled-assets/webpack/187.js 20.1 kB
./public/compiled-assets/webpack/192.js 5.69 kB
./public/compiled-assets/webpack/276.js 4.39 kB
./public/compiled-assets/webpack/344.js 2 kB
./public/compiled-assets/webpack/445.js 6.94 kB
./public/compiled-assets/webpack/492.js 7.58 kB
./public/compiled-assets/webpack/503.js 37.7 kB
./public/compiled-assets/webpack/706.js 107 kB
./public/compiled-assets/webpack/719.js 13.5 kB
./public/compiled-assets/webpack/847.js 26 kB
./public/compiled-assets/webpack/checkout.js 17 kB
./public/compiled-assets/webpack/GuardianAdLiteLanding.js 8.15 kB
./public/compiled-assets/webpack/LandingPage.js 15.5 kB
./public/compiled-assets/webpack/oneTimeCheckout.js 6.16 kB
./public/compiled-assets/webpack/ThankYou.js 44.5 kB

compressed-size-action

Copy link
Contributor

@GHaberis GHaberis left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

)
case _: SupporterPlus | _: TierThree | _: DigitalPack =>
List(SessionCookie("GU_AF1", now.plusDays(1).getMillis.toString))

case p: Paper if p.productOptions.hasDigitalSubscription =>
Copy link
Member

Choose a reason for hiding this comment

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

I know this is pre-existing but we give ad-free to all paper subscribers now, not just those with the digipack add on

Accepted(createSubscriptionResponse.asJson)
.withCookies(cookies: _*)
.discardingCookies(discardIncompleteCheckoutCookie),
result.value.flatMap {
Copy link
Member

Choose a reason for hiding this comment

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

I refactored this because there was a warning about nested stateful monads. I think it looks clearer this way as well.

tjmw and others added 3 commits February 12, 2025 10:19
I've removed the following as they're no longer used:
* gu_recurring_contributor
* gu_digital_subscriber
* gu.contributions.recurring.contrib-timestamp.<billing period>

I've refactored the cookie setting code a bit to consolidate things and
make it clear where products set the same cookie(s).
@rupertbates rupertbates force-pushed the tw/remove-cookies-set-by-checkout branch from 44f1c56 to 9f2357c Compare February 12, 2025 10:21
Copy link
Member Author

@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.

Your changes look good to me @rupertbates 👍

@rupertbates rupertbates merged commit 19220a1 into main Feb 12, 2025
19 checks passed
@rupertbates rupertbates deleted the tw/remove-cookies-set-by-checkout branch February 12, 2025 11:45
@prout-bot
Copy link

Seen on PROD (created by @tjmw and merged by @rupertbates 9 minutes and 51 seconds ago)

Sentry Release: support-client-side, support

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.

4 participants