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

Streamline campaign setup: Show campaign setup fields immediately during onboarding #2533

Merged
merged 10 commits into from
Sep 10, 2024
Merged

Streamline campaign setup: Show campaign setup fields immediately during onboarding #2533

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8774a7e
Show campaign creation fields by default.
asvinb Aug 16, 2024
0e22ee2
Remove associated tests.
asvinb Aug 16, 2024
cd2fcc3
Merge branch 'develop' into feature/2500-show-campaign-fields
asvinb Aug 20, 2024
977d9e1
Remove the opened_paid_ads_setup prop and no need to merge objects.
asvinb Aug 20, 2024
a8ac154
Add unknown as fallback value.
asvinb Sep 2, 2024
5284ff6
Use logical OR operator.
asvinb Sep 4, 2024
f6b0fd2
Merge branch 'feature/2459-campaign-creation-flow' into feature/2500-…
asvinb Sep 10, 2024
0d73263
Fix merge conflict.
asvinb Sep 10, 2024
10adcf0
Merge branch 'feature/2500-show-campaign-fields' of github.com:woocom…
asvinb Sep 10, 2024
068f4c4
Revert changes.
asvinb Sep 10, 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
18 changes: 0 additions & 18 deletions js/src/setup-mc/setup-stepper/setup-paid-ads/clientSession.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,29 +6,11 @@
* @property {Array<CountryCode>} countryCodes Audience country codes of the paid ads campaign. Example: 'US'.
*/

const KEY_SHOW_PAID_ADS_SETUP = 'gla-onboarding-show-paid-ads-setup';
const KEY_PAID_ADS = 'gla-onboarding-paid-ads';

const { sessionStorage } = window;

const clientSession = {
/**
* @param {boolean} isShowing Whether the paid ads setup is showing.
*/
setShowPaidAdsSetup( isShowing ) {
const showing = Boolean( isShowing ).toString();
sessionStorage.setItem( KEY_SHOW_PAID_ADS_SETUP, showing );
},

/**
* @param {boolean} defaultValue The default value to be returned if stored value is not available.
* @return {boolean} Returns the stored value. It will return `defaultValue` if stored value is not available.
*/
getShowPaidAdsSetup( defaultValue ) {
const showing = sessionStorage.getItem( KEY_SHOW_PAID_ADS_SETUP );
return showing === null ? defaultValue : showing === 'true';
},

/**
* @param {CampaignData} data Campaign data to be stored.
* @param {number|undefined} data.amount Daily average cost of the campaign.
Expand Down
14 changes: 1 addition & 13 deletions js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-features-section.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,8 @@ function FeatureList( { hideBudgetContent } ) {
*
* @param {Object} props React props.
* @param {boolean} props.hideBudgetContent Whether to hide the content about the ad budget.
* @param {boolean} props.hideFooterButtons Whether to hide the buttons at the card footer.
* @param {JSX.Element} props.skipButton Button to skip paid ads setup.
* @param {JSX.Element} props.continueButton Button to continue paid ads setup.
*/
export default function PaidAdsFeaturesSection( {
hideBudgetContent,
hideFooterButtons,
skipButton,
continueButton,
} ) {
export default function PaidAdsFeaturesSection( { hideBudgetContent } ) {
return (
<Section
className="gla-paid-ads-features-section"
Expand Down Expand Up @@ -130,10 +122,6 @@ export default function PaidAdsFeaturesSection( {
</FlexItem>
</Flex>
</Section.Card.Body>
<Section.Card.Footer hidden={ hideFooterButtons }>
{ skipButton }
{ continueButton }
</Section.Card.Footer>
</Section.Card>
</Section>
);
Expand Down
49 changes: 9 additions & 40 deletions js/src/setup-mc/setup-stepper/setup-paid-ads/setup-paid-ads.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import AppButton from '.~/components/app-button';
import PaidAdsFeaturesSection from './paid-ads-features-section';
import PaidAdsSetupSections from './paid-ads-setup-sections';
import { getProductFeedUrl } from '.~/utils/urls';
import clientSession from './clientSession';
import { API_NAMESPACE, STORE_KEY } from '.~/data/constants';
import { GUIDE_NAMES } from '.~/constants';

Expand Down Expand Up @@ -71,17 +70,9 @@ export default function SetupPaidAds() {
const { createNotice } = useDispatchCoreNotices();
const { googleAdsAccount, hasGoogleAdsConnection } = useGoogleAdsAccount();
const [ handleSetupComplete ] = useAdsSetupCompleteCallback();
const [ showPaidAdsSetup, setShowPaidAdsSetup ] = useState( () =>
clientSession.getShowPaidAdsSetup( false )
);
const [ paidAds, setPaidAds ] = useState( {} );
const [ completing, setCompleting ] = useState( null );

const handleContinuePaidAdsSetupClick = () => {
setShowPaidAdsSetup( true );
clientSession.setShowPaidAdsSetup( true );
};

const finishOnboardingSetup = async ( event, onBeforeFinish = noop ) => {
setCompleting( event.target.dataset.action );

Expand Down Expand Up @@ -129,16 +120,14 @@ export default function SetupPaidAds() {
campaign_form_validation: 'unknown',
};

if ( showPaidAdsSetup ) {
const selector = select( STORE_KEY );
const billing = selector.getGoogleAdsAccountBillingStatus();
const selector = select( STORE_KEY );
const billing = selector.getGoogleAdsAccountBillingStatus();

merge( eventProps, {
opened_paid_ads_setup: 'yes',
billing_method_status: billing?.status,
campaign_form_validation: paidAds.isValid ? 'valid' : 'invalid',
} );
}
merge( eventProps, {
opened_paid_ads_setup: 'yes',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Now that we're removing the possibility of skipping without opening the ads setup, this event prop should be removed from the whole component and removed from the inline docs for gla_onboarding_complete_button_click above.

I think we could also avoid the need to merge event props here and just set them directly when eventProps is initialized above.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch and makes sense @joemcgill . I've updated the PR. Can you kindly take a look again please?

billing_method_status: billing?.status,
campaign_form_validation: paidAds.isValid ? 'valid' : 'invalid',
} );

const disabledSkip =
completing === ACTION_COMPLETE || ! hasGoogleAdsConnection;
Expand Down Expand Up @@ -171,30 +160,10 @@ export default function SetupPaidAds() {
/>
<PaidAdsFeaturesSection
hideBudgetContent={ ! hasGoogleAdsConnection }
hideFooterButtons={
! hasGoogleAdsConnection || showPaidAdsSetup
}
skipButton={ createSkipButton(
__( 'Skip this step for now', 'google-listings-and-ads' )
) }
continueButton={
<AppButton
isPrimary
text={ __(
'Create campaign',
'google-listings-and-ads'
) }
disabled={ completing === ACTION_SKIP }
onClick={ handleContinuePaidAdsSetupClick }
eventName="gla_onboarding_open_paid_ads_setup_button_click"
/>
}
/>
{ showPaidAdsSetup && (
<PaidAdsSetupSections onStatesReceived={ setPaidAds } />
) }
<PaidAdsSetupSections onStatesReceived={ setPaidAds } />
<FaqsSection />
<StepContentFooter hidden={ ! showPaidAdsSetup }>
<StepContentFooter>
<Flex justify="right" gap={ 4 }>
{ createSkipButton(
__(
Expand Down
34 changes: 0 additions & 34 deletions tests/e2e/specs/setup-mc/step-4-complete-campaign.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,15 +159,6 @@ test.describe( 'Complete your campaign', () => {

test.describe( 'Set up paid ads', () => {
test.describe( 'Click "Create a paid ad campaign" button', () => {
test.beforeAll( async () => {
await completeCampaign.clickCreatePaidAdButton();
} );

test( 'should not see the "Create a paid ad campaign" button after this section is shown', async () => {
const button = completeCampaign.getCreatePaidAdButton();
await expect( button ).toBeHidden();
} );

test( 'should see "Complete setup" button is disabled', async () => {
const completeSetupButton =
completeCampaign.getCompleteSetupButton();
Expand Down Expand Up @@ -382,35 +373,10 @@ test.describe( 'Complete your campaign', () => {
} );
} );

test.describe( 'Complete onboarding by "Skip this step for now"', () => {
joemcgill marked this conversation as resolved.
Show resolved Hide resolved
test.beforeAll( async () => {
// Reset the showing status for the "Set up paid ads" section.
await page.evaluate( () => window.sessionStorage.clear() );
await setupAdsAccountPage.mockAdsAccountIncomplete();
await completeCampaign.goto();
await completeCampaign.clickSkipStepButton();
} );

test( 'should see the setup success modal', async () => {
const setupSuccessModal = page
.locator( '.components-modal__content' )
.filter( {
hasText:
'You’ve successfully set up Google for WooCommerce!',
} );
await expect( setupSuccessModal ).toBeVisible();
} );

test( 'should see the url contains product-feed', async () => {
expect( page.url() ).toMatch( /path=%2Fgoogle%2Fproduct-feed/ );
} );
} );

test.describe( 'Complete onboarding by "Skip paid ads creation"', () => {
test.beforeAll( async () => {
await setupAdsAccountPage.mockAdsAccountIncomplete();
await completeCampaign.goto();
await completeCampaign.clickCreatePaidAdButton();
await completeCampaign.clickSkipPaidAdsCreationButon();
} );

Expand Down
46 changes: 0 additions & 46 deletions tests/e2e/utils/pages/setup-mc/step-4-complete-campaign.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,30 +64,6 @@ export default class CompleteCampaign extends MockRequests {
return this.page.locator( '.gla-budget-section' ).nth( 0 );
}

/**
* Get skip this step for now button.
*
* @return {import('@playwright/test').Locator} Get skip this step for now button.
*/
getSkipStepButton() {
return this.page.getByRole( 'button', {
name: 'Skip this step for now',
joemcgill marked this conversation as resolved.
Show resolved Hide resolved
exact: true,
} );
}

/**
* Get create a paid ad button.
*
* @return {import('@playwright/test').Locator} Get create a paid ad button.
*/
getCreatePaidAdButton() {
return this.page.getByRole( 'button', {
name: 'Create campaign',
exact: true,
} );
}

/**
* Get complete setup button.
*
Expand All @@ -112,17 +88,6 @@ export default class CompleteCampaign extends MockRequests {
} );
}

/**
* Click skip this step for now button.
*
* @return {Promise<void>}
*/
async clickSkipStepButton() {
joemcgill marked this conversation as resolved.
Show resolved Hide resolved
const button = this.getSkipStepButton();
await button.click();
await this.page.waitForLoadState( LOAD_STATE.DOM_CONTENT_LOADED );
}

/**
* Click skip paid ads creation button.
*
Expand All @@ -134,17 +99,6 @@ export default class CompleteCampaign extends MockRequests {
await this.page.waitForLoadState( LOAD_STATE.DOM_CONTENT_LOADED );
}

/**
* Click create a paid ad campaign button.
*
* @return {Promise<void>}
*/
async clickCreatePaidAdButton() {
const button = this.getCreatePaidAdButton();
await button.click();
await this.page.waitForLoadState( LOAD_STATE.DOM_CONTENT_LOADED );
}

/**
* Click complete setup button.
*
Expand Down
Loading