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

Added member's subscription cancellation helper {{cancel_link}} #11434

Merged
merged 37 commits into from Dec 12, 2019

Conversation

naz
Copy link
Member

@naz naz commented Dec 4, 2019

refs TryGhost/Members#107

  • Adds membership cancellation handling for elements with data-members-cancel-subscription attribute.
  • Adds new members endpoint DELETE /subscriptions/:id which allows canceling membership by subscription_id

@peterzimon the naming of the data-members-cancel-subscription needs confirmation. Also, currently reloading the page with window.location.reload(); after the request succeeds, this flow needs some design input as well 😉

@allouis would be great 🙌 if you could give a second pair of 👀 but we discussed this in the pitch enough I think 😃

Copy link
Contributor

@allouis allouis 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!

The only thing it might be missing is the errorEl logic that you see in some of the other handlers in public/members.js

@naz
Copy link
Member Author

naz commented Dec 5, 2019

might be missing is the errorEl

@allouis good point on this one!

Have added handling and added a text which needs a copy review: There was an error canceling the subscription, please try again. @peterzimon lmk if this makes sense? Note: using data-members-subscription-error attribute to identify where the error should be displayed. It's this way because we are not inside of 'form' like in case of the signup (this is something I invented and also needs design input)

Also, have seen a console.error(err); in data-members-plan handling. Is it supposed to still be there? My assumption is it was left for debugging purposes.

@allouis
Copy link
Contributor

allouis commented Dec 5, 2019

@gargol I think you can still nest the error element for now, just like we do with the data-members-plan button - it's not great for now, but at leats it's consistent - and then we can improve it in future?

@naz
Copy link
Member Author

naz commented Dec 5, 2019

you can still nest the error element for now

We are listening to a button, so there's not much to be nested in I think. Will adjust as clearer wireframes are confirmed. For now, it's a rough prototype so is fine like this with a thought to be changed.

@allouis
Copy link
Contributor

allouis commented Dec 5, 2019

This is how we currently document it for the data-members-plan buttons https://ghost.org/docs/members/checkout-buttons/#error-messages

So if we have this one as different then IMO we should maybe update the other stuff too

@naz
Copy link
Member Author

naz commented Dec 6, 2019

This is how we currently document it for the data-members-plan buttons

Oh 😮 it makes sense! Good point!

@naz naz force-pushed the members-cancel-subscription branch from 4a72f32 to b5486fc Compare December 6, 2019 07:44
@naz naz force-pushed the members-cancel-subscription branch from 4f1bdbc to ea3e6ed Compare December 9, 2019 11:41
@@ -0,0 +1,25 @@
const commands = require('../../../schema').commands;
Copy link
Member Author

Choose a reason for hiding this comment

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

@allouis & @rishabhgrg this commit has a tiny migration and some schema changes. Would need your 👍 for this as usual 😉

Copy link
Contributor

Choose a reason for hiding this comment

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

@gargol Looks good 👍

@naz naz force-pushed the members-cancel-subscription branch from 9bdf1ce to 78b7d47 Compare December 10, 2019 07:07
naz added a commit to TryGhost/Members that referenced this pull request Dec 12, 2019
refs #TryGhost/Ghost#11434

- Added method to allow updating single subscription. Only `cancel_at_period_end` field can be updated. 
- Middleware is needed to allow Ghost Core to cancel/uncancel member's subscription. 
- Relies on the request containing identity information to be able to verify if subscription belongs to the user
- When member could not be identified by the identity information present in the request we should throw instead of continuing processing
- Handling and messaging inspired by https://github.com/TryGhost/Ghost/blob/3.1.1/core/server/services/mega/mega.js#L132
- When the user initiates subscription cancellation we can safely mark the subscription as canceled so that it's not shown in the interface on subsequent request. Otherwise, we end up in a situation where we still return the subscription in the period until Stripe triggers the webhook.
- Added boolean coercion for cancel_at_period_end parameter. If anything but boolean is passed to Stripe API it throws an error.  Coercing the value on our side is a gives a better dev experience
naz added a commit to TryGhost/gscan that referenced this pull request Dec 12, 2019
refs TryGhost/Ghost#11434

- Added {{cancel_link}} to know helper in canary spec
- Updated compile test structure to test version-specific changes
- The previous structure only tested v1 spec and didn't take into account any new 'allowed helpers'
- The new structure resembles one in deprecations and can be easily extended with new helpers on a per-version basis
- Allows to create elements with `data-members-cancel-subscription` attribute which would call subscription canellation
- This is more apporpriate palce for this endpoint.
…or' element

- We are not inside the form, so there needs to be a separate element which would define where the error should be shown
- We need to store auto-renewal/cancellation state on our side to be able to display it to the member
- We need 2 states handled for subscription, for that reason PUT with a flag makes more sense than a DELETE
- As discussed we only add single column to  subscriptions table to avoid preoptimizing for future cases
naz and others added 23 commits December 12, 2019 17:21
…continue-subscription

- This corresponds better to the intention of the action and matches parameter names for the {{cancel_link}} helper
- This is needed so that the check is also performed when the member flag in settings is toggled during runtime
- Needed to use `updateSubscription` middleware
- Needed to recognize new {{cancel_link}} helper
@naz naz force-pushed the members-cancel-subscription branch from 550866c to bbaf0ba Compare December 12, 2019 10:22
@naz naz changed the title Added member's subscription cancellation attribute Added member's subscription cancellation helper {{cancel_link}} Dec 12, 2019
@naz naz merged commit e277c6b into TryGhost:master Dec 12, 2019
@naz naz deleted the members-cancel-subscription branch December 12, 2019 12:59
naz added a commit to TryGhost/Lyra that referenced this pull request Dec 12, 2019
refs TryGhost/Ghost#11434

- Adds {{cancel_link}} use to the `account.hbs` page which allows to cancel or continue member's subscription.
- Adds some subscription specific data on the account page to give more context to the member.
daniellockyer pushed a commit that referenced this pull request Jul 20, 2022
refs ##11434

- Added method to allow updating single subscription. Only `cancel_at_period_end` field can be updated. 
- Middleware is needed to allow Ghost Core to cancel/uncancel member's subscription. 
- Relies on the request containing identity information to be able to verify if subscription belongs to the user
- When member could not be identified by the identity information present in the request we should throw instead of continuing processing
- Handling and messaging inspired by https://github.com/TryGhost/Ghost/blob/3.1.1/core/server/services/mega/mega.js#L132
- When the user initiates subscription cancellation we can safely mark the subscription as canceled so that it's not shown in the interface on subsequent request. Otherwise, we end up in a situation where we still return the subscription in the period until Stripe triggers the webhook.
- Added boolean coercion for cancel_at_period_end parameter. If anything but boolean is passed to Stripe API it throws an error.  Coercing the value on our side is a gives a better dev experience
Eros720 added a commit to Eros720/Lyra that referenced this pull request Jun 6, 2023
refs TryGhost/Ghost#11434

- Adds {{cancel_link}} use to the `account.hbs` page which allows to cancel or continue member's subscription.
- Adds some subscription specific data on the account page to give more context to the member.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants