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

Billing notifications #1861

Merged
merged 45 commits into from
Jan 21, 2022
Merged

Billing notifications #1861

merged 45 commits into from
Jan 21, 2022

Conversation

FSM1
Copy link
Contributor

@FSM1 FSM1 commented Jan 18, 2022

closes #1819

Submission checklist:

Layout

  • Change looks good in the desktop web ui
  • Change looks good in the mobile web ui - Currently N/A as the mobile notification area still needs to be designed and implemented.

Theme

  • Components / elements inspected in light mode
  • Components / elements inspected in dark mode

FSM1 and others added 30 commits December 16, 2021 16:12
Co-authored-by: Thibaut Sardan <[email protected]>
Co-authored-by: Tanmoy Basak Anjan <[email protected]>
…Tab/ChangePlan/CryptoPayment.tsx

Co-authored-by: Thibaut Sardan <[email protected]>
@FSM1 FSM1 marked this pull request as ready for review January 19, 2022 12:42
@FSM1 FSM1 self-assigned this Jan 19, 2022
@FSM1 FSM1 added the Status: Review Needed 👀 Added to PRs when they need more review label Jan 19, 2022
Copy link
Contributor

@tanmoyAtb tanmoyAtb left a comment

Choose a reason for hiding this comment

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

Looks 🚀 to go !

@asnaith
Copy link
Member

asnaith commented Jan 20, 2022

Hey Mike, should we be able to verify this by modifying the responses on https://stage.imploy.site/api/v1/billing/subscriptions? Meaning, if we managed to intercept and stub out the expiry date to be within 7 days should we see one of these notifications?

That was the most logical way I could think of testing. However, I got the traffic routing through Charles Proxy and set a breakpoint on billing/subscription. I manually edited the expiration date to be next week but still couldn't see the notification in Files.

@FSM1 FSM1 enabled auto-merge (squash) January 20, 2022 10:47
@FSM1
Copy link
Contributor Author

FSM1 commented Jan 20, 2022

Which of the notifications were you trying to test? If it's the first one, then the best way to test this would be to intercept >the invoices response and add in an invoice with a status of open in which case a notification should be displayed.

Yeah! Thank you for the tips, it's looking good

Screen Shot 2022-01-20 at 1 36 57 PM

The account restricted one is a bit trickier as it is driven by the contents of the JWT. I will need to take a deeper look at >the code to see if this one is easily testable.

Ok 👍

The card expiry notification is the easiest to test. Just need to add a card with an expiry date in the current month and >choose a monthly plan, and the notification should pop up. Updating the card to one expiring after the next payment date >should make the notification disappear.

Awesome!
Screen Shot 2022-01-20 at 1 47 21 PM

@lgtm-com
Copy link

lgtm-com bot commented Jan 20, 2022

This pull request introduces 2 alerts when merging c07340d into 7cb726c - view on LGTM.com

new alerts:

  • 2 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Jan 20, 2022

This pull request introduces 2 alerts when merging 2cccf99 into 7cb726c - view on LGTM.com

new alerts:

  • 2 for Unused variable, import, function or class

@Tbaut Tbaut self-requested a review January 20, 2022 12:47
@Tbaut
Copy link
Collaborator

Tbaut commented Jan 20, 2022

I didn't play with it yet, but the code looks good.

@FSM1 FSM1 merged commit 7625ce3 into dev Jan 21, 2022
@FSM1 FSM1 deleted the feat/billing-notifications-1819 branch January 21, 2022 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Review Needed 👀 Added to PRs when they need more review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Billing notifications
5 participants