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

Update cancellation reasons for SupporterPlus and Recurring contributions #1415

Merged
merged 4 commits into from
Nov 18, 2024

Conversation

rBangay
Copy link
Contributor

@rBangay rBangay commented Nov 11, 2024

What does this PR change?

Updates the cancellation reasons for supporter plus and recurring contributions. Consolidates the list of reasons and add's 2 new options.

See the raw data in "Cancellation journeys" doc https://docs.google.com/spreadsheets/d/1rf0M-TYmXltJkLHUy7hIeVtezQMmfg4TOaJfMXD3ltk/edit?gid=1054852399#gid=1054852399

images

Contribution Before Contribution After
Supporter plus Before Supporter plus After

…ion journeys.

split the digital+print (tier three) cancallation reasons out into it's own fil in preperation for having it's own reasons (at the moment it shares the same list as Guardian weekly)
@rBangay rBangay requested a review from a team November 14, 2024 10:18
@johnduffell
Copy link
Member

@rBangay just to make it easier to double check - where's the original reference for what the changes are supposed to be
Also a screenshot or two might make it easier to review if you have one handy.

@@ -37,6 +37,7 @@ export type CancellationReasonId =
| 'mma_cost_of_living'
| 'mma_cutting_subscriptions'
| 'mma_payment_issue'
| 'mma_price_increase'
| 'mma_article'
Copy link
Member

Choose a reason for hiding this comment

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

we can remove the unused ones now? e.g. I don't think mma_article exists in this repo after this change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep good shout, I'll remove all the unused reasons 👍

saveBody: [
'In order to improve our journalism, we’d love to know more about why you are thinking of cancelling.',
],
alternateFeedbackIntro: standardAlternateFeedbackIntro,
Copy link
Member

@johnduffell johnduffell Nov 14, 2024

Choose a reason for hiding this comment

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

is this making the "custom feedback form prefix message" of "Please share any further thoughts you have about cancelling — you can help us improve. Thank you." ?

Copy link
Member

Choose a reason for hiding this comment

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

yes it is

export const standardAlternateFeedbackIntro =
'Please share any further thoughts you have about cancelling — you can help us improve. Thank you.';

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep

import type { CancellationReason } from '../cancellationReason';
import { BreakFromNewsWithAlternative } from '../GenericSaveBodyResponses';

export const tierThreeCancellationReasons: CancellationReason[] = [
Copy link
Member

@johnduffell johnduffell Nov 14, 2024

Choose a reason for hiding this comment

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

do we need to shuffle these in the same way as the others? and if so, do we need to make sure the mma_other option is always last?

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 checked with Alex and he agreed that all the products cancellation reasosn should be shuffled 👍


export const tierThreeCancellationReasons: CancellationReason[] = [
{
reasonId: 'mma_editorial',
Copy link
Member

Choose a reason for hiding this comment

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

the doc doesn't mention what the reasonIds should be. These all look reasonable though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this will come in a seperate pr (if they change at all) this change was just to move them out into a seperate file where they previously shared the Guardian Weekly reasons.

{
reasonId: 'mma_values',
linkLabel: 'I don’t feel that The Guardian values my support',
alternateFeedbackIntro: inOrderToImproveSubs,
Copy link
Member

Choose a reason for hiding this comment

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

looks good

export const inOrderToImproveSubs =
'In order to improve our subscription and supporter model, we’d love to know more about why you are thinking of cancelling.';

Copy link
Member

@johnduffell johnduffell left a comment

Choose a reason for hiding this comment

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

looks accurate, I've updated the description with the doc link.

@rBangay rBangay force-pushed the cancellation-reasons-update branch from 5fcb33a to 2b64963 Compare November 15, 2024 12:59
@johnduffell
Copy link
Member

small tip - try not to force push once it's been looked at by someone else as it makes it hard to see what's happened since the last review (and I ideally don't want to review the whole thing again)

@rBangay rBangay merged commit 602728f into main Nov 18, 2024
13 checks passed
@rBangay rBangay deleted the cancellation-reasons-update branch November 18, 2024 11:21
@prout-bot
Copy link
Collaborator

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

Comment on lines +9 to +10
export const shuffleArray = (array: unknown[]) =>
[...array].sort(() => 0.5 - Math.random());
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

@johnduffell johnduffell Nov 18, 2024

Choose a reason for hiding this comment

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

tried in dev console and it's true, this is the percentage of the time each element appears first

Object.entries([...Array(1000000).keys().map(() => [1,2,3,4,5,6,7,8,9,10].sort( () => .5 - Math.random() )[0])].sort().reduce((acc, num) => {const e = {...acc};e[num]=acc[num] + 1; return e;}, {1:0,2:0,3:0,4:0,5:0,6:0,7:0,8:0,9:0,10:0})).map(([num, count]) => `${num}: ${count/10000}%`).map((a) => console.log(a))
1: 19.5461%
2: 7.2563%
3: 9.6343%
4: 12.8512%
5: 8.3207%
6: 8.9094%
7: 9.9178%
8: 11.1279%
9: 6.0317%
10: 6.4046%

you can see they should all be 10% but they are repeatably (very) strange amounts (and that's just for the first element)

Copy link
Member

Choose a reason for hiding this comment

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

just scrolled up and realised that this was an existing function, so it actually means our randomisation hasn't been working correctly the whole time

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