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

feature(unlock-app, locksmith): Cancel card paid memberships #13123

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

julien51
Copy link
Member

Description

We did not have a good way to cancel recurring card payments.

Issue: #13111

Checklist:

  • 1 PR, 1 purpose: my Pull Request applies to a single purpose
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the docs to reflect my changes if applicable
  • I have added tests (and stories for frontend components) that prove my fix is effective or that my feature works
  • I have performed a self-review of my own code
  • If my code involves visual changes, I am adding applicable screenshots to this thread

Release Note Draft Snippet

lockAddress,
network,
userAddress,
await KeySubscription.update(
Copy link
Member Author

Choose a reason for hiding this comment

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

Instead of deleting we just move to 0 recurring.

Copy link
Member Author

@julien51 julien51 left a comment

Choose a reason for hiding this comment

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

@@ -25,7 +25,7 @@ export interface Subscription {
price: Amount
possibleRenewals: string
approvedRenewals: string
type: 'Crypto' | 'Stripe'
type: 'crypto' | 'stripe'
Copy link
Member Author

Choose a reason for hiding this comment

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

fixing the types... based on openapi

@@ -54,42 +54,63 @@ export const getSubscriptionsForLockByOwner = async ({
)

// If no key is found or not erc20 or version < 11 which we don't fully support, return nothing.
if (
!key ||
key.lock.tokenAddress === ethers.constants.AddressZero ||
Copy link
Member Author

Choose a reason for hiding this comment

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

We may actually have card recurring even if the lock itself does not support recurring thru an ERC20.

@@ -43,7 +44,7 @@ export const CancelAndRefundModal = ({
['getAmounts', lockAddress],
getAmounts,
{
enabled: isOpen, // execute query only when the modal is open
enabled: isOpen && subscription.type !== 'Stripe',
Copy link
Member Author

Choose a reason for hiding this comment

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

for cards there is no refund.

}

const cancelRefundMutation = useMutation(cancelAndRefund, {
onSuccess: () => {
ToastHelper.success('Key cancelled and successfully refunded.')
ToastHelper.success('Key cancelled.')
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed the mention of refund as it might not be applicable!

tokenId={tokenId}
network={network}
expiration={expiration}
imageURL={metadata.image}
/>
<CancelAndRefundModal
Copy link
Member Author

Choose a reason for hiding this comment

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

Move that into the drawer for simplicity

@@ -485,16 +456,6 @@ function Key({ ownedKey, account, network }: Props) {
<CopyLineIcon size={18} />
</button>
</div>
<a
Copy link
Member Author

Choose a reason for hiding this comment

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

There is already a lin to the block explorer in the "action" menu

Copy link
Member

@clemsos clemsos left a comment

Choose a reason for hiding this comment

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

lgtm

Comment on lines +66 to +93
let userBalance,
decimals,
userAllowance,
symbol,
numberOfRenewalsApprovedValue,
numberOfRenewalsApproved

if (
key.lock.tokenAddress &&
key.lock.tokenAddress !== ethers.constants.AddressZero
) {
;[userBalance, decimals, userAllowance, symbol] = await Promise.all([
getErc20BalanceForAddress(key.lock.tokenAddress, key.owner, provider),
getErc20Decimals(key.lock.tokenAddress, provider),
getAllowance(
key.lock.tokenAddress,
key.lock.address,
provider,
key.owner
),
getErc20TokenSymbol(key.lock.tokenAddress, provider),
])

// Approved renewals
numberOfRenewalsApprovedValue =
userAllowance.gt(0) && parseFloat(price) > 0
? userAllowance.div(price)
: ethers.BigNumber.from(0)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is hard to read. Can we split it down?

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