Refactor Usage frontend data fetching #4639
Conversation
There was a problem hiding this comment.
I no longer see duplicated calls 👍 From the little testing in the UI I did, it seems everything works fine. I was overriding kpi/serializers/v2/service_usage.py to force the API to return exceeding/almost exceeding amounts - so I'm not 100% sure how the code behaves when the limits change while the app is being used, but I think this will be tested thoroughly during QA.
| @@ -1,5 +1,6 @@ | |||
| import {fetchGet, fetchPost} from 'jsapp/js/api'; | |||
| import {getOrganization} from 'js/account/stripe.api'; | |||
| import {createContext} from 'react'; | |||
There was a problem hiding this comment.
I'm guessing not needed :)
| submissions: number; | ||
| transcriptionMinutes: number; | ||
| translationChars: number; | ||
| interface LimitState { | ||
| storageByteLimit: number | 'unlimited'; | ||
| nlpCharacterLimit: number | 'unlimited'; | ||
| nlpMinuteLimit: number | 'unlimited'; | ||
| submissionLimit: number | 'unlimited'; |
There was a problem hiding this comment.
Would it make sense to create some type UsageLimit = number | 'unlimited';?
| let interval; | ||
| if (state.subscribedProduct?.length > 0) { | ||
| interval = state.subscribedProduct?.[0].items[0].price.recurring.interval; | ||
| } | ||
| isOverLimit(subscribedStorageLimit, usage.storage, 'storage'); |
There was a problem hiding this comment.
Given it's not a boolean-returning function, I find this name confusing. Could this be something like checkIfOverLimit or verifyOverLimit, something that sounds like an action that will cause the data to be rebuilt ;-)
| limits={limits.warningList} | ||
| usagePage={usagePage} | ||
| usagePage={Boolean(usagePage)} | ||
| /> | ||
| )} | ||
| {useModal && ( | ||
| <LimitModal |
There was a problem hiding this comment.
A nitpick (and more of a problem with the modal styles/decisions) - the buttons overlap in an ugly way in a small viewport:
| @@ -23,8 +21,8 @@ const LimitNotifications = ({ | |||
| const [dismissed, setDismissed] = useState(!useModal); | |||
There was a problem hiding this comment.
We could go with export default const LimitNotifications and remove that closing line :)
There was a problem hiding this comment.
Also please add some short /** comment description of this ❤️
| import {ACTIVE_STRIPE_STATUSES} from 'js/constants'; | ||
| import envStore from 'js/envStore'; | ||
| import subscriptionStore from 'js/account/subscriptionStore'; | ||
|
|
||
| // check if the currently logged-in user has a paid subscription in an active status | ||
| // promise returns a boolean, or `null` if Stripe is not active - we check for the existence of `stripe_public_key` |
There was a problem hiding this comment.
Please turn this into /** comment, as it will be showing up in code editors as a helpful hint :)
| @@ -1,5 +1,6 @@ | |||
| import {fetchGet, fetchPost} from 'jsapp/js/api'; | |||
| import {getOrganization} from 'js/account/stripe.api'; | |||
| import {createContext} from 'react'; | |||
|
|
|||
There was a problem hiding this comment.
Similarly to stripe.api.ts - some short /** comments for funfctions would be great.
| isUsageLoaded: boolean; | ||
| isLimitsLoaded: boolean; | ||
| isSubscriptionLoaded: boolean; | ||
| isLoaded: boolean; | ||
| } | ||
|
|
||
| export default function Usage() { |
There was a problem hiding this comment.
Please add a short /** comment ❤️ Having one would make it easier to understand why we render this in MyProjectsRoute etc.
| isLoaded: boolean; | ||
| } | ||
|
|
||
| export function useUsage() { |
There was a problem hiding this comment.
Please add a short /** comment ❤️
| @@ -30,17 +21,9 @@ function subscriptionReducer(state: SubscribedState, action: {prodData: any}) { | |||
| return {...state, subscribedProduct: action.prodData}; | |||
| } | |||
|
|
|||
| export const getAllExceedingLimits = () => { | |||
| export const useExceedingLimits = () => { | |||
There was a problem hiding this comment.
Please add a short /** comment ❤️
Checklist
Description
Prevent some extra API calls that were slowing down the Project list/Usage page.
Notes
api/v2/stripe/subscriptions/.Usagecomponent andgetAllExceedingLimitsand put it into a hook. Usage data is now passed down via context instead of getting fetched twice on the Usage page.getAllExceedingLimitstouseExceedingLimitsto make it more obvious that it's a hook.