Skip to content

Commit 2748b1d

Browse files
committed
Extend benefits cookie expiry to 30 days
The cookie which tracks when to refresh benefits cookies is reduced to 1 day, like everywhere else. This is part of a wider change to benefits cookie expiration to fix an issue where signed-in users with ad free benefits who didn't visit the site for time period would sometimes see ads on their first returning pageview. This change makes this less likely to happen by extending the expiration of user benefits cookies. This is due to a race condition between the user benefits refresh and the ads code. However, we don't want to delay ads until after the user benefits have been refreshed as that would impact performance. So instead, extend the expiry of the cookie. See also: guardian/frontend#28352 guardian/dotcom-rendering#14810 guardian/gateway#3285
1 parent d7b1259 commit 2748b1d

File tree

2 files changed

+21
-16
lines changed

2 files changed

+21
-16
lines changed

support-frontend/app/controllers/SubscriptionProductCookiesCreator.scala

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,10 @@ import com.gu.support.workers._
88

99
case class SubscriptionProductCookiesCreator(domain: GuardianDomain) {
1010
private val oneDayInSeconds = 60 * 60 * 24;
11-
private val maxAgeInDays = 7
11+
private val benefitsCookieLengthInDays = 30
12+
private val benefitsExpiryCookieLengthInDays = 1
1213

13-
private def persistentCookieWithMaxAge(name: String, now: DateTime) = {
14+
private def persistentCookieWithMaxAge(name: String, now: DateTime, maxAgeInDays: Int) = {
1415
val expiryTime = now.plusDays(maxAgeInDays).withTimeAtStartOfDay().getMillis.toString
1516
Cookie(
1617
name = name,
@@ -22,16 +23,20 @@ case class SubscriptionProductCookiesCreator(domain: GuardianDomain) {
2223
)
2324
}
2425
private def adFreeCookie(now: DateTime) =
25-
persistentCookieWithMaxAge("GU_AF1", now)
26+
persistentCookieWithMaxAge(name = "GU_AF1", now = now, maxAgeInDays = benefitsCookieLengthInDays)
2627

2728
private def hideSupportMessagingCookie(now: DateTime) =
28-
persistentCookieWithMaxAge("gu_hide_support_messaging", now)
29+
persistentCookieWithMaxAge(name = "gu_hide_support_messaging", now = now, maxAgeInDays = benefitsCookieLengthInDays)
2930

3031
private def allowRejectAllCookie(now: DateTime) =
31-
persistentCookieWithMaxAge("gu_allow_reject_all", now)
32+
persistentCookieWithMaxAge(name = "gu_allow_reject_all", now = now, maxAgeInDays = benefitsCookieLengthInDays)
3233

3334
private def userBenefitsExpiryCookie(now: DateTime) =
34-
persistentCookieWithMaxAge("gu_user_benefits_expiry", now)
35+
persistentCookieWithMaxAge(
36+
name = "gu_user_benefits_expiry",
37+
now = now,
38+
maxAgeInDays = benefitsExpiryCookieLengthInDays,
39+
)
3540

3641
def createCookiesForProduct(product: ProductType, now: DateTime): List[Cookie] = {
3742
// Setting the user benefits cookies used by frontend. See:

support-frontend/test/controllers/SubscriptionProductCookiesCreatorTest.scala

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,12 @@ class SubscriptionProductCookiesCreatorTest extends AnyFlatSpec with Matchers {
1313
private val creator = SubscriptionProductCookiesCreator(GuardianDomain("thegulocal.com"))
1414

1515
private def expectedCookie(name: String, ageInDays: Int) = {
16-
val ageInSeconds = ageInDays * 24 * 60 * 60
16+
val ageInSeconds = BigInt(ageInDays * 24 * 60 * 60)
1717

1818
Cookie(
1919
name = name,
20-
value = s"${now.getMillis + (ageInSeconds * 1000)}",
21-
maxAge = Some(ageInSeconds),
20+
value = (now.getMillis + (ageInSeconds * 1000)).toString,
21+
maxAge = Some(ageInSeconds.toInt),
2222
secure = true,
2323
httpOnly = false,
2424
domain = Some("thegulocal.com"),
@@ -29,9 +29,9 @@ class SubscriptionProductCookiesCreatorTest extends AnyFlatSpec with Matchers {
2929
val guardianAdLite = GuardianAdLite(GBP)
3030
val cookies = creator.createCookiesForProduct(guardianAdLite, now)
3131

32-
cookies should contain(expectedCookie(name = "gu_allow_reject_all", ageInDays = 7))
32+
cookies should contain(expectedCookie(name = "gu_allow_reject_all", ageInDays = 30))
3333
// It should also contain the user features expiry cookie
34-
cookies should contain(expectedCookie(name = "gu_user_benefits_expiry", ageInDays = 7))
34+
cookies should contain(expectedCookie(name = "gu_user_benefits_expiry", ageInDays = 1))
3535
}
3636

3737
it should "not set the hide support messaging cookie for Guardian Ad Lite because it is not a supporter product" in {
@@ -40,18 +40,18 @@ class SubscriptionProductCookiesCreatorTest extends AnyFlatSpec with Matchers {
4040

4141
cookies.map(_.name) should not contain "gu_hide_support_messaging"
4242
// It should also contain the user features expiry cookie
43-
cookies should contain(expectedCookie(name = "gu_user_benefits_expiry", ageInDays = 7))
43+
cookies should contain(expectedCookie(name = "gu_user_benefits_expiry", ageInDays = 1))
4444
}
4545

4646
it should "set the ad-free cookie, the hide supporter revenue cookie and the allow reject all cookie for SupporterPlus" in {
4747
val supporterPlus = SupporterPlus(BigDecimal(12), GBP, Monthly)
4848
val cookies = creator.createCookiesForProduct(supporterPlus, now)
4949

50-
cookies should contain(expectedCookie(name = "GU_AF1", ageInDays = 7))
51-
cookies should contain(expectedCookie(name = "gu_hide_support_messaging", ageInDays = 7))
52-
cookies should contain(expectedCookie(name = "gu_allow_reject_all", ageInDays = 7))
50+
cookies should contain(expectedCookie(name = "GU_AF1", ageInDays = 30))
51+
cookies should contain(expectedCookie(name = "gu_hide_support_messaging", ageInDays = 30))
52+
cookies should contain(expectedCookie(name = "gu_allow_reject_all", ageInDays = 30))
5353
// It should also contain the user features expiry cookie
54-
cookies should contain(expectedCookie(name = "gu_user_benefits_expiry", ageInDays = 7))
54+
cookies should contain(expectedCookie(name = "gu_user_benefits_expiry", ageInDays = 1))
5555
}
5656

5757
}

0 commit comments

Comments
 (0)