-
Notifications
You must be signed in to change notification settings - Fork 2k
A4A: Create a site from the licenses page without leaving it #113355
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
base: trunk
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| import { Button } from '@automattic/components'; | ||
| import { Spinner } from '@wordpress/components'; | ||
| import { useTranslate } from 'i18n-calypso'; | ||
|
|
||
| import './style.scss'; | ||
|
|
||
| type Props = { | ||
| isProvisioning: boolean; | ||
| isLoading: boolean; | ||
| onCreateSite: () => void; | ||
| className?: string; | ||
| primary?: boolean; | ||
| borderless?: boolean; | ||
| }; | ||
|
|
||
| export default function CreateSiteButton( { | ||
| isProvisioning, | ||
| isLoading, | ||
| onCreateSite, | ||
| className, | ||
| primary, | ||
| borderless, | ||
| }: Props ) { | ||
| const translate = useTranslate(); | ||
|
|
||
| return ( | ||
| <Button | ||
| className={ className } | ||
| compact | ||
| primary={ primary } | ||
| borderless={ borderless } | ||
| busy={ isLoading } | ||
| disabled={ isLoading || isProvisioning } | ||
| onClick={ onCreateSite } | ||
| > | ||
| { isProvisioning ? ( | ||
| <span className="licenses-create-site-button-provisioning"> | ||
| <Spinner /> | ||
| { translate( 'Creating site…' ) } | ||
| </span> | ||
| ) : ( | ||
| translate( 'Create site' ) | ||
| ) } | ||
| </Button> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| .licenses-create-site-button-provisioning { | ||
| display: inline-flex; | ||
| align-items: center; | ||
| gap: 4px; | ||
|
|
||
| .components-spinner { | ||
| margin: 0; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| import page from '@automattic/calypso-router'; | ||
| import { useCallback, useState } from 'react'; | ||
| import { | ||
| A4A_LICENSES_LINK, | ||
| A4A_SITES_LINK_NEEDS_SETUP, | ||
| } from 'calypso/a8c-for-agencies/components/sidebar-menu/lib/constants'; | ||
| import useFetchPendingSites from 'calypso/a8c-for-agencies/data/sites/use-fetch-pending-sites'; | ||
| import { useDispatch } from 'calypso/state'; | ||
| import { recordTracksEvent } from 'calypso/state/analytics/actions'; | ||
| import { findPendingSiteIdByLicenseKey, hasProvisioningSite } from '../lib/pending-sites'; | ||
| import LicenseSiteConfigurationsModal from '../license-site-configurations-modal'; | ||
| import usePaymentMethodGate from './use-payment-method-gate'; | ||
|
|
||
| type CreateSiteFromLicense = { | ||
| onCreateSite: () => void; | ||
| isProvisioning: boolean; | ||
| isLoading: boolean; | ||
| modal: JSX.Element | null; | ||
| }; | ||
|
|
||
| /** | ||
| * Lets an unassigned WordPress.com license be turned into a site without | ||
| * leaving the licenses page, using the same configuration modal the Needs | ||
| * setup page opens. Falls back to that page when the license has no pending | ||
| * site to configure. | ||
| */ | ||
| export default function useCreateSiteFromLicense( | ||
| licenseKey: string, | ||
| isClientLicense?: boolean | ||
| ): CreateSiteFromLicense { | ||
| const dispatch = useDispatch(); | ||
| const [ isModalOpen, setIsModalOpen ] = useState( false ); | ||
|
|
||
| const { data: pendingSites, isLoading } = useFetchPendingSites(); | ||
| const isBlockedByMissingPaymentMethod = usePaymentMethodGate( isClientLicense ); | ||
|
|
||
| const pendingSiteId = findPendingSiteIdByLicenseKey( pendingSites, licenseKey ); | ||
|
|
||
| const onCreateSite = useCallback( () => { | ||
| if ( isBlockedByMissingPaymentMethod( A4A_LICENSES_LINK ) ) { | ||
| return; | ||
| } | ||
|
|
||
| if ( ! pendingSiteId ) { | ||
| dispatch( | ||
| recordTracksEvent( 'calypso_a4a_licenses_create_site_redirect_needs_setup', { | ||
| license_key: licenseKey, | ||
| } ) | ||
| ); | ||
| page( A4A_SITES_LINK_NEEDS_SETUP ); | ||
| return; | ||
| } | ||
|
|
||
| dispatch( | ||
| recordTracksEvent( 'calypso_a4a_licenses_create_site_modal_open', { | ||
| license_key: licenseKey, | ||
| } ) | ||
| ); | ||
| setIsModalOpen( true ); | ||
| }, [ dispatch, isBlockedByMissingPaymentMethod, licenseKey, pendingSiteId ] ); | ||
|
|
||
| return { | ||
| onCreateSite, | ||
| isProvisioning: hasProvisioningSite( pendingSites ), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While one site is provisioning, every unassigned WordPress.com license shows the spinner and “Creating site…”, not just the one being created. Disabling all buttons is fine (matches the Needs setup page), but the label should only change on the license that’s actually provisioning. Suggest splitting the two signals: // lib/pending-sites.ts
export function isLicenseProvisioning(
pendingSites: PendingSite[] | undefined,
licenseKey: string
): boolean {
return !! pendingSites?.some(
( { features } ) =>
features?.wpcom_atomic?.license_key === licenseKey &&
features?.wpcom_atomic?.state === 'provisioning'
);
}Then return both from the hook —
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is intentional. We need to disable this on both the Needs Setup and Licenses pages while provisioning is in progress. This follows the same reasoning as our current implementation: we only allow one site to be provisioned at a time to avoid confusion and prevent conflicting provisioning requests. |
||
| isLoading, | ||
| modal: | ||
| isModalOpen && pendingSiteId ? ( | ||
| <LicenseSiteConfigurationsModal | ||
| siteId={ pendingSiteId } | ||
| closeModal={ () => setIsModalOpen( false ) } | ||
| /> | ||
| ) : null, | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| import { useTranslate } from 'i18n-calypso'; | ||
| import { useCallback } from 'react'; | ||
| import { addQueryArgs } from 'calypso/lib/url'; | ||
| import { useDispatch } from 'calypso/state'; | ||
| import { errorNotice } from 'calypso/state/notices/actions'; | ||
| import usePaymentMethod from '../../payment-methods/hooks/use-payment-method'; | ||
|
|
||
| /** | ||
| * Guards license actions that cost money. Returns a predicate that reports | ||
| * whether the action can't proceed, having told the agency why and where to | ||
| * come back to. Client licenses are billed to the client, so they skip it. | ||
| */ | ||
| export default function usePaymentMethodGate( isClientLicense?: boolean ) { | ||
| const dispatch = useDispatch(); | ||
| const translate = useTranslate(); | ||
|
|
||
| const { paymentMethodRequired } = usePaymentMethod(); | ||
|
|
||
| return useCallback( | ||
| ( returnUrl: string ) => { | ||
| if ( ! paymentMethodRequired || isClientLicense ) { | ||
| return false; | ||
| } | ||
|
|
||
| const noticeLinkHref = addQueryArgs( | ||
| { | ||
| return: returnUrl, | ||
| }, | ||
| '/purchases/payment-methods/add' | ||
| ); | ||
| const errorMessage = translate( | ||
| 'A primary payment method is required.{{br/}} ' + | ||
| '{{a}}Try adding a new payment method{{/a}} or contact support.', | ||
| { | ||
| components: { | ||
| a: <a href={ noticeLinkHref } />, | ||
| br: <br />, | ||
| }, | ||
| } | ||
| ); | ||
|
|
||
| dispatch( errorNotice( errorMessage ) ); | ||
| return true; | ||
| }, | ||
| [ dispatch, isClientLicense, paymentMethodRequired, translate ] | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| export type PendingSite = { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: the type declares
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Made both fields optional in the type. The endpoint is untyped and non-WordPress.com pending sites do not carry that block, so the optional chaining was the correct half. |
||
| id: number; | ||
| features?: { | ||
| wpcom_atomic?: { | ||
| license_key: string; | ||
| state: string; | ||
| }; | ||
| }; | ||
| }; | ||
|
|
||
| /** | ||
| * The pending site a license will become once the agency configures it. Only | ||
| * sites still in the `pending` state can be created; anything further along is | ||
| * already on its way to the sites dashboard. | ||
| */ | ||
| export function findPendingSiteIdByLicenseKey( | ||
| pendingSites: PendingSite[] | undefined, | ||
| licenseKey: string | ||
| ): number | null { | ||
| return ( | ||
| pendingSites?.find( | ||
| ( { features }: PendingSite ) => | ||
| features?.wpcom_atomic?.license_key === licenseKey && | ||
| features?.wpcom_atomic?.state === 'pending' | ||
| )?.id ?? null | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Mirrors the Needs setup page: provisioning is treated as an account-wide | ||
| * state, so no other site can be created while one is being built. | ||
| */ | ||
| export function hasProvisioningSite( pendingSites: PendingSite[] | undefined ): boolean { | ||
| return !! pendingSites?.some( | ||
| ( { features }: PendingSite ) => | ||
| features?.wpcom_atomic?.state === 'provisioning' && !! features?.wpcom_atomic?.license_key | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| import { findPendingSiteIdByLicenseKey, hasProvisioningSite } from '../pending-sites'; | ||
| import type { PendingSite } from '../pending-sites'; | ||
|
|
||
| const pendingSite = ( id: number, license_key: string, state: string ): PendingSite => ( { | ||
| id, | ||
| features: { wpcom_atomic: { license_key, state } }, | ||
| } ); | ||
|
|
||
| describe( 'findPendingSiteIdByLicenseKey', () => { | ||
| it( 'returns the id of the pending site behind the license', () => { | ||
| const sites = [ | ||
| pendingSite( 1, 'wpcom-hosting-business_aaa', 'pending' ), | ||
| pendingSite( 2, 'wpcom-hosting-business_bbb', 'pending' ), | ||
| ]; | ||
|
|
||
| expect( findPendingSiteIdByLicenseKey( sites, 'wpcom-hosting-business_bbb' ) ).toBe( 2 ); | ||
| } ); | ||
|
|
||
| it( 'ignores sites that are no longer pending', () => { | ||
| const sites = [ pendingSite( 1, 'wpcom-hosting-business_aaa', 'provisioning' ) ]; | ||
|
|
||
| expect( findPendingSiteIdByLicenseKey( sites, 'wpcom-hosting-business_aaa' ) ).toBeNull(); | ||
| } ); | ||
|
|
||
| it( 'returns null when no site matches the license', () => { | ||
| const sites = [ pendingSite( 1, 'wpcom-hosting-business_aaa', 'pending' ) ]; | ||
|
|
||
| expect( findPendingSiteIdByLicenseKey( sites, 'wpcom-hosting-business_zzz' ) ).toBeNull(); | ||
| } ); | ||
|
|
||
| it( 'returns null while pending sites are still loading', () => { | ||
| expect( findPendingSiteIdByLicenseKey( undefined, 'wpcom-hosting-business_aaa' ) ).toBeNull(); | ||
| } ); | ||
| } ); | ||
|
|
||
| describe( 'hasProvisioningSite', () => { | ||
| it( 'reports provisioning across any site, not just the one being looked at', () => { | ||
| const sites = [ | ||
| pendingSite( 1, 'wpcom-hosting-business_aaa', 'pending' ), | ||
| pendingSite( 2, 'wpcom-hosting-business_bbb', 'provisioning' ), | ||
| ]; | ||
|
|
||
| expect( hasProvisioningSite( sites ) ).toBe( true ); | ||
| } ); | ||
|
|
||
| it( 'is false when every site is still pending', () => { | ||
| expect( | ||
| hasProvisioningSite( [ pendingSite( 1, 'wpcom-hosting-business_aaa', 'pending' ) ] ) | ||
| ).toBe( false ); | ||
| } ); | ||
|
|
||
| it( 'ignores provisioning sites without a license key', () => { | ||
| expect( hasProvisioningSite( [ pendingSite( 1, '', 'provisioning' ) ] ) ).toBe( false ); | ||
| } ); | ||
|
|
||
| it( 'is false while pending sites are still loading', () => { | ||
| expect( hasProvisioningSite( undefined ) ).toBe( false ); | ||
| } ); | ||
| } ); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,6 @@ import { | |
| A4A_MARKETPLACE_ASSIGN_LICENSE_LINK, | ||
| A4A_MARKETPLACE_HOSTING_PRESSABLE_LINK, | ||
| A4A_MARKETPLACE_HOSTING_WPCOM_LINK, | ||
| A4A_SITES_LINK_NEEDS_SETUP, | ||
| EXTERNAL_PRESSABLE_AUTH_URL, | ||
| } from 'calypso/a8c-for-agencies/components/sidebar-menu/lib/constants'; | ||
| import { | ||
|
|
@@ -23,6 +22,8 @@ import { hasAgencyCapability } from 'calypso/state/a8c-for-agencies/agency/selec | |
| import { A4AStore } from 'calypso/state/a8c-for-agencies/types'; | ||
| import { recordTracksEvent } from 'calypso/state/analytics/actions'; | ||
| import { errorNotice } from 'calypso/state/notices/actions'; | ||
| import CreateSiteButton from '../create-site-button'; | ||
| import useCreateSiteFromLicense from '../hooks/use-create-site-from-license'; | ||
| import useLicenseDownloadUrlMutation from '../revoke-license-dialog/hooks/use-license-download-url-mutation'; | ||
| import type { LicenseSubscription } from 'calypso/state/partner-portal/types'; | ||
|
|
||
|
|
@@ -71,9 +72,14 @@ export default function LicenseDetailsActions( { | |
| const debugUrl = siteUrl ? `https://jptools.wordpress.com/debug/?url=${ siteUrl }` : null; | ||
| const downloadUrl = useLicenseDownloadUrlMutation( licenseKey ); | ||
|
|
||
| const redirectUrl = isWPCOMHostingLicense | ||
| ? A4A_SITES_LINK_NEEDS_SETUP | ||
| : addQueryArgs( { key: licenseKey }, A4A_MARKETPLACE_ASSIGN_LICENSE_LINK ); | ||
| const assignLicenseUrl = addQueryArgs( { key: licenseKey }, A4A_MARKETPLACE_ASSIGN_LICENSE_LINK ); | ||
|
|
||
| const { | ||
| onCreateSite, | ||
| isProvisioning, | ||
| isLoading: isLoadingPendingSites, | ||
| modal: siteConfigurationsModal, | ||
| } = useCreateSiteFromLicense( licenseKey, isClientLicense ); | ||
|
|
||
| const openRevokeDialog = useCallback( () => { | ||
| setRevokeDialog( true ); | ||
|
|
@@ -182,11 +188,27 @@ export default function LicenseDetailsActions( { | |
|
|
||
| { ! isPressableAddonLicense && | ||
| licenseState === LicenseState.Detached && | ||
| licenseType === LicenseType.Partner && ( | ||
| <Button compact primary className="license-details__assign-button" href={ redirectUrl }> | ||
| { isWPCOMHostingLicense ? translate( 'Create site' ) : translate( 'Assign license' ) } | ||
| licenseType === LicenseType.Partner && | ||
| ( isWPCOMHostingLicense ? ( | ||
| <CreateSiteButton | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This button skips the payment-method check that
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch. Moved the payment method check into the shared hook, so both the row button and the expanded details are gated the same way now. |
||
| className="license-details__assign-button" | ||
| primary | ||
| isProvisioning={ isProvisioning } | ||
| isLoading={ isLoadingPendingSites } | ||
| onCreateSite={ onCreateSite } | ||
| /> | ||
| ) : ( | ||
| <Button | ||
| compact | ||
| primary | ||
| className="license-details__assign-button" | ||
| href={ assignLicenseUrl } | ||
| > | ||
| { translate( 'Assign license' ) } | ||
| </Button> | ||
| ) } | ||
| ) ) } | ||
|
|
||
| { siteConfigurationsModal } | ||
|
|
||
| { revokeDialog && ( | ||
| <CancelLicenseFeedbackModal | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.