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

Top tier copy changes #1366

Merged
merged 5 commits into from
Aug 1, 2024
Merged

Top tier copy changes #1366

merged 5 commits into from
Aug 1, 2024

Conversation

rBangay
Copy link
Contributor

@rBangay rBangay commented Jul 29, 2024

What does this PR change?

Copy changed for the top tier (digital + print) product, which was modelled after the Guardian weekly product.

Minor refactor of the cancellation confirmation page to remove unnecessary product detail providing closure.

@@ -532,15 +532,15 @@ export function baseTierThree(): ProductDetail {
country: 'United Kingdom',
},
safeToUpdatePaymentMethod: true,
start: '2024-06-28',
end: '2025-06-13',
start: '2021-12-24',
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, why are these set to dates in the past?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved them into the past to line up with the mocked data for the potential holiday stop api response. I could have setup some new holiday stop dates specifically for top tier, or updated the dates for all products everywhere, but I thought this was the simplest thing to do.

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.

LGTM! 👍

@@ -183,19 +193,16 @@ const actuallyCancelled = (
export const isCancelled = (subscription: Subscription) =>
Object.keys(subscription).length === 0 || subscription.cancelledAt;

export const getCancellationSummary =
(productType: ProductType, cancelledProductDetail: ProductDetail) =>
(productDetail: ProductDetail) =>
Copy link
Member

Choose a reason for hiding this comment

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

Nice work getting rid of the extra layer of function application here 👍 It seems like it's definitely not needed - all of the call sites I can see immediately call the returned function, passing the same value as was passed in the initial call.

only call 'getMainPlan' function on the cancellation summary page if the required properties on the subscription object exist
@rBangay rBangay merged commit 9e998f3 into main Aug 1, 2024
13 checks passed
@rBangay rBangay deleted the top-tier-copy-changes branch August 1, 2024 13:08
@prout-bot
Copy link
Collaborator

Seen on PROD (merged by @rBangay 10 minutes and 33 seconds ago) Please check your changes!

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