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

Hiding messaging when external payments are provided #1425

Merged
merged 4 commits into from
Jan 23, 2025
Merged

Hiding messaging when external payments are provided #1425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
adf5571
Making function just set all the related data
travjenkins Jan 17, 2025
146b0d4
Setting some changes back as they are not needed
travjenkins Jan 22, 2025
70e1e71
Moving stuff into a hook to make it a bit simpler and sharable
travjenkins Jan 22, 2025
4c3145d
Merge branch 'main' into travjenkins/bug/fix-billing-content-for-exte…
travjenkins Jan 22, 2025
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
11 changes: 9 additions & 2 deletions src/components/admin/Billing/PricingTierDetails.tsx
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason why this component shouldn't return null when an external payment method is detected? The argument I would make against returning null in this scenario is to more easily accommodate external payment method messaging in the future.

The only minor oddity with the current approach is that the loading state will still be displayed when externalPaymentMethod is true despite the message not having any content.

Original file line number Diff line number Diff line change
@@ -1,20 +1,27 @@
import { Skeleton, Typography } from '@mui/material';
import { useTenantUsesExternalPayment } from 'context/fetcher/TenantBillingDetails';
import { useMemo } from 'react';
import { FormattedMessage } from 'react-intl';
import { useBillingStore } from 'stores/Billing/Store';
import { useTenantStore } from 'stores/Tenant/Store';

function PricingTierDetails() {
const selectedTenant = useTenantStore((state) => state.selectedTenant);
const externalPaymentMethod = useTenantUsesExternalPayment(selectedTenant);

const billingStoreHydrated = useBillingStore((state) => state.hydrated);
const paymentMethodExists = useBillingStore(
(state) => state.paymentMethodExists
);

const messageId = useMemo(
() =>
paymentMethodExists
externalPaymentMethod
? 'admin.billing.message.external'
: paymentMethodExists
? 'admin.billing.message.paidTier'
: 'admin.billing.message.freeTier',
[paymentMethodExists]
[externalPaymentMethod, paymentMethodExists]
);

if (!billingStoreHydrated || typeof paymentMethodExists !== 'boolean') {
Expand Down
21 changes: 17 additions & 4 deletions src/context/fetcher/TenantBillingDetails.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { MAX_TENANTS } from 'api/billing';
import FullPageError from 'components/fullPage/Error';
import { useUserInfoSummaryStore } from 'context/UserInfoSummary/useUserInfoSummaryStore';
import { useTenantsDetailsForPayment } from 'hooks/useTenants';
import { createContext, useContext } from 'react';
import { createContext, useContext, useMemo } from 'react';
import { FormattedMessage } from 'react-intl';
import { useEntitiesStore_tenantsWithAdmin } from 'stores/Entities/hooks';
import { BaseComponentProps, TenantPaymentDetails } from 'types';
Expand All @@ -14,7 +14,7 @@ const TenantContext = createContext<TenantContextData>({
tenantBillingDetails: null,
});

const TenantBillingDetailsContextProvider = ({
export const TenantBillingDetailsContextProvider = ({
children,
}: BaseComponentProps) => {
const hasSupportRole = useUserInfoSummaryStore(
Expand Down Expand Up @@ -62,8 +62,21 @@ const TenantBillingDetailsContextProvider = ({
// TODO (optimization): Consider defining a hook that returns an array of mapped tenant names.
// The majority of the components that call useTenantDetails do so to extract a memoized
// array of tenant names.
const useTenantBillingDetails = () => {
export const useTenantBillingDetails = () => {
return useContext(TenantContext);
};

export { TenantBillingDetailsContextProvider, useTenantBillingDetails };
export const useTenantUsesExternalPayment = (selectedTenant: string) => {
const { tenantBillingDetails } = useTenantBillingDetails();

return useMemo(() => {
return !tenantBillingDetails
? false
: tenantBillingDetails.some((tenantBillingDetail) => {
return (
tenantBillingDetail.tenant === selectedTenant &&
tenantBillingDetail.payment_provider === 'external'
);
});
Comment on lines +73 to +80
Copy link
Member

Choose a reason for hiding this comment

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

Is there a benefit to using a ternary as opposed to the nullish coalescing operator?

}, [selectedTenant, tenantBillingDetails]);
};
1 change: 1 addition & 0 deletions src/lang/en-US/AdminPage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export const AdminPage: Record<string, string> = {
'admin.billing.header': `Billing`,
'admin.billing.message.freeTier': `The free tier lets you try Flow with up to 2 tasks and 10GB per month without entering a credit card. Usage beyond these limits automatically starts a 30 day free trial.`,
'admin.billing.message.paidTier': `Cloud tier.`,
'admin.billing.message.external': ` `,
'admin.billing.error.details.header': `There was a network issue.`,
'admin.billing.error.details.message': `There was an error fetching your billing details. ${Errors['error.tryAgain']}`,
'admin.billing.error.paymentMethodsError': `There was an error connecting with our payment provider. Please try again later.`,
Expand Down
Loading