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.

Sign up for GitHub

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

feat: add a billing loading action #21863

Merged
merged 3 commits into from
Apr 26, 2024
Merged

feat: add a billing loading action #21863

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0407c57
Add a billing loading action
zlwaterfield Apr 25, 2024
7503221
Merge remote-tracking branch 'origin/master' into zach/billing-activa…
zlwaterfield Apr 25, 2024
58e0790
Merge remote-tracking branch 'origin/master' into zach/billing-activa…
zlwaterfield Apr 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions frontend/src/scenes/billing/BillingProduct.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,10 @@ export const getTierDescription = (

export const BillingProductAddon = ({ addon }: { addon: BillingProductV2AddonType }): JSX.Element => {
const { billing, redirectPath } = useValues(billingLogic)
const { isPricingModalOpen, currentAndUpgradePlans, surveyID } = useValues(billingProductLogic({ product: addon }))
const { toggleIsPricingModalOpen, reportSurveyShown, setSurveyResponse } = useActions(
const { isPricingModalOpen, currentAndUpgradePlans, surveyID, billingProductLoading } = useValues(
billingProductLogic({ product: addon })
)
const { toggleIsPricingModalOpen, reportSurveyShown, setSurveyResponse, setBillingProductLoading } = useActions(
billingProductLogic({ product: addon })
)
const { featureFlags } = useValues(featureFlagLogic)
Expand Down Expand Up @@ -168,6 +170,10 @@ export const BillingProductAddon = ({ addon }: { addon: BillingProductV2AddonTyp
currentAndUpgradePlans?.upgradePlan?.plan_key
}${redirectPath && `&redirect_path=${redirectPath}`}`}
disableClientSideRouting
loading={billingProductLoading === addon.type}
onClick={() => {
setBillingProductLoading(addon.type)
}}
>
Add
</LemonButton>
Expand Down Expand Up @@ -201,6 +207,7 @@ export const BillingProduct = ({ product }: { product: BillingProductV2Type }):
isPlanComparisonModalOpen,
currentAndUpgradePlans,
surveyID,
billingProductLoading,
} = useValues(billingProductLogic({ product }))
const {
setIsEditingBillingLimit,
Expand All @@ -209,6 +216,7 @@ export const BillingProduct = ({ product }: { product: BillingProductV2Type }):
toggleIsPlanComparisonModalOpen,
reportSurveyShown,
setSurveyResponse,
setBillingProductLoading,
} = useActions(billingProductLogic({ product, productRef }))
const { reportBillingUpgradeClicked } = useActions(eventUsageLogic)

Expand Down Expand Up @@ -707,8 +715,10 @@ export const BillingProduct = ({ product }: { product: BillingProductV2Type }):
type="primary"
icon={<IconPlus />}
disableClientSideRouting
loading={billingProductLoading === product.type}
onClick={() => {
reportBillingUpgradeClicked(product.type)
setBillingProductLoading(product.type)
}}
className="grow"
center
Expand Down
7 changes: 7 additions & 0 deletions frontend/src/scenes/billing/billingProductLogic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export const billingProductLogic = kea<billingProductLogicType>([
}),
reportSurveyDismissed: (surveyID: string) => ({ surveyID }),
setSurveyID: (surveyID: string) => ({ surveyID }),
setBillingProductLoading: (productKey: string) => ({ productKey }),
}),
reducers({
billingLimitInput: [
Expand Down Expand Up @@ -109,6 +110,12 @@ export const billingProductLogic = kea<billingProductLogicType>([
setSurveyID: (_, { surveyID }) => surveyID,
},
],
billingProductLoading: [
null as string | null,
{
setBillingProductLoading: (_, { productKey }) => productKey,
},
],
Comment on lines +113 to +118
Copy link
Member

Choose a reason for hiding this comment

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

I feel like this should be getting reset somewhere when it's done loading? If the page refreshes then makes sense that it doesn't get manually reset, but the page doesn't always refresh.

Copy link
Contributor

Choose a reason for hiding this comment

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

It probably makes more sense to move the call on the subscribe / add buttons into a loader so the loading state can be handled by kea.

Copy link
Contributor

Choose a reason for hiding this comment

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

It can be the same as the deactivate loader currently on billingLogic.tsx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are all outgoing links, so that's why I did it this way. I only added these to buttons with to props.

I could move into kea but it would just be a window.location.href = and the loader wouldn't get reset anyways.

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 don't think it makes sense to move to kea since sometimes we will need to redirect to checkout but that won't be known at time of click, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The original question was:

I feel like this should be getting reset somewhere when it's done loading?

So I'm just saying that all these requests are requests to the server that will return a redirect so the loading doesn't need to be reset because there always be a page refresh.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it, thanks.

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 can look into moving these into kea as a fetch that gets redirected as well, that's just a bit of a different change.

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like keeping the redirect logic in the billing service is fine. It keeps the logic around when to interact with stripe in one place.

Copy link
Contributor Author

@zlwaterfield zlwaterfield Apr 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I just gave moving into kea a try. I did just something like:

                activateProduct: async ({ productKey, redirect }: { productKey: string, redirect?: string }) => {
                    let path = 'api/billing-v2/activation?products=' + productKey
                    if (redirect) {
                        path += '&redirect=' + redirect
                    }
                    return api.getResponse(path)
                }

but the API returns a redirect to the billing repo, and then the fetch goes there and gets a CORS issue. So it's definitely better to keep it as is.

comparisonModalHighlightedFeatureKey: [
null as string | null,
{
Expand Down
Loading