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.

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

Scaffold (logic for) new AdSense setup account components #4759

Closed
felixarntz opened this issue Feb 1, 2022 · 4 comments
Closed

Scaffold (logic for) new AdSense setup account components #4759

felixarntz opened this issue Feb 1, 2022 · 4 comments
Labels
Module: AdSense Google AdSense module related issues P0 High priority QA: Eng Requires specialized QA by an engineer Rollover Issues which role over to the next sprint Type: Enhancement Improvement of an existing feature

Comments

@felixarntz
Copy link
Member

felixarntz commented Feb 1, 2022

Following the scaffolded main components for the enhanced AdSense setup flow from #4758, this issue is about scaffolding the next "hierarchical" level of components, around the more specific state of an (already created/selected) account.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • The following AdSense setup components should be created in assets/js/modules/adsense/components/setup/v2/:
  • The SetupAccount component introduced in Scaffold (logic for) new AdSense setup main components #4758 should be implemented with logic as follows (replacing the dummy content that is currently in there):
    • It needs to get the current client ID setting value (see getClientID selector).
    • It needs to call the getClients and getCurrentSite selectors of the module.
    • It should return one of the above components as follows:
      • If there is no client of type AFC in the list of clients from the API, it should return the SetupAccountNoClient component.
      • If there is no current site (i.e. the selector returns null, NOT undefined), it should return the SetupAccountCreateSite component.
      • If the pendingTasks list on the given account prop is not empty, it should return the SetupAccountPendingTasks component.
      • Otherwise it should return the SetupAccountSite component, passing the two props to it.
    • A number of useEffect hooks should be added:
      • A new useEffect to conditionally set the clientID setting on-the-fly should be added:
        • If there is an AFC client in the returned list from the API and no clientID is set yet.
        • Or if there is an AFC client in the returned list from the API but it is not the currently set clientID.
      • A new useEffect to conditionally set the accountStatus setting on-the-fly should be added:
        • If there is no AFC client in the API response, it should be set to ACCOUNT_STATUS_NO_CLIENT.
        • If the account's pendingTasks is not empty, it should be set to ACCOUNT_STATUS_PENDING_TASKS.
        • Otherwise, it should be set to ACCOUNT_STATUS_APPROVED.
      • A new useEffect to conditionally set the siteStatus setting on-the-fly should be added:
        • If there is no result for the "current site" (see above), it should be set to SITE_STATUS_NONE.
        • Otherwise, it should be set to SITE_STATUS_NEEDS_ATTENTION (for now).
  • A tiny but important tweak should be made to the V2 SetupMain component introduced in Scaffold (logic for) new AdSense setup main components #4758:
    • In the useEffect to update the accountStatus setting on-the-fly, the entire condition for setting it to ACCOUNT_STATUS_NO_CLIENT should be removed, as that was just temporary and is now surpassed by the above logic in SetupAccount.
  • The new SetupAccountNoClient should be scaffolded to simply return an untranslated string (e.g. in a div) TODO: UI for lack of AFC client in account {accountID} (using the _id from the account prop).
  • The new SetupAccountCreateSite should be scaffolded to simply return an untranslated string (e.g. in a div) TODO: UI to create a new site in account {accountID} (using the _id from the account prop).
  • The new SetupAccountPendingTasks should be scaffolded to simply return an untranslated string (e.g. in a div) TODO: UI for pending tasks in account {accountID} (using the _id from the account prop).
  • The new SetupAccountSite should be scaffolded to simply return an untranslated string (e.g. in a div) TODO: UI to proceed with next steps for account {accountID} and site {domain} (using the _id from the account prop and domain from site prop).
  • Storybook stories for SetupMain in the above four alternate variants should be added.

Implementation Brief

  • Create the following files in assets/js/modules/adsense/components/setup/v2/ which export a functional component having the same filename.
    • SetupAccountNoClient
    • SetupAccountCreateSite
    • SetupAccountPendingTasks
    • SetupAccountSite
  • For the above components, return the string as per the AC.
  • Using assets/js/modules/adsense/components/setup/v2/SetupAccount,
    • Get the clientID via the getClientID selector.
    • Get the clients via the getClients selector.
    • Get the currentSite via the getCurrentSite selector.
    • The following components should be rendered with the conditions below:
      • SetupAccountNoClient if there is no client of type AFC in the list of clients. This is done by checking if client.productCode === 'AFC'.
      • SetupAccountCreateSite if currentSite is null (not undefined).
      • SetupAccountPendingTasks if the pendingTasks list on the given account prop is not empty.
      • finally SetupAccountSite by default, proxying the account prop and the site prop, with currentSite as value.
    • A new useEffect to conditionally set the clientID (via dispatching the setClientID action) setting on-the-fly should be added,
      • If there is an AFC client in clients and clientID is not set.
      • Or if there is an AFC client in clients but it is not the currently set clientID.
    • A new useEffect to conditionally set the accountStatus (via dispatching the setAccountStatus action) setting on-the-fly should be added,
      • If there is no AFC client in clients, it should be set to ACCOUNT_STATUS_NO_CLIENT.
      • If the account's pendingTasks is not empty, it should be set to ACCOUNT_STATUS_PENDING_TASKS.
      • Otherwise, it should be set to ACCOUNT_STATUS_APPROVED.
      • The above constants should be imported from assets/js/modules/adsense/util/status.js.
    • A new useEffect to conditionally set the siteStatus (via dispatching the setSiteStatus action) setting on-the-fly should be added:
      • If currentSite is null, it should be set to SITE_STATUS_NONE.
      • Otherwise, it should be set to SITE_STATUS_NEEDS_ATTENTION.
  • Using assets/js/modules/adsense/components/setup/v2/SetupMain,
    • In the useEffect to update the accountStatus setting on-the-fly, the entire condition for setting it to ACCOUNT_STATUS_NO_CLIENT should be removed. Refer to AC for more details to as why.
  • Update assets/js/modules/adsense/components/setup/v2/SetupMain.stories.js to include stories for the above variants where SetupAccountNoClient, SetupAccountCreateSite, SetupAccountPendingTasks and SetupAccountSite are rendered.

Test Coverage

  • No new tests to be added.

QA Brief

  • Check stories for the SetupMain V2 component to make sure it contains new SetupAccount variants.
  • Check the SetupAccount component to verify that it implements useEffect hooks defined in AC.

Changelog entry

  • N/A
@felixarntz felixarntz added P0 High priority Type: Enhancement Improvement of an existing feature Module: AdSense Google AdSense module related issues labels Feb 1, 2022
@felixarntz felixarntz self-assigned this Feb 1, 2022
@felixarntz felixarntz removed their assignment Feb 2, 2022
@asvinb asvinb assigned asvinb and unassigned asvinb Feb 9, 2022
@eugene-manuilov eugene-manuilov self-assigned this Feb 10, 2022
@eugene-manuilov
Copy link
Collaborator

IB ✔️

@eugene-manuilov eugene-manuilov removed their assignment Feb 10, 2022
@eugene-manuilov eugene-manuilov self-assigned this Mar 21, 2022
@eugene-manuilov eugene-manuilov added the QA: Eng Requires specialized QA by an engineer label Mar 25, 2022
@eugene-manuilov eugene-manuilov removed their assignment Mar 25, 2022
@FlicHollis FlicHollis added the Rollover Issues which role over to the next sprint label Mar 28, 2022
@aaemnnosttv aaemnnosttv removed their assignment Mar 28, 2022
@asvinb asvinb self-assigned this Apr 1, 2022
@asvinb
Copy link
Collaborator

asvinb commented Apr 1, 2022

QA: ❗

@eugene-manuilov Just a note. The AC mentions that SetupAccountSite should accept 2 new props. account and site objects. However you're only passing the accountID. Is that intentional?

@eugene-manuilov
Copy link
Collaborator

The AC mentions that SetupAccountSite should accept 2 new props. account and site objects. However you're only passing the accountID. Is that intentional?

@asvinb Yes, i decided to use simplified properties for now instead of passing whole account and site objects to the component. If we see that we need more account/site data, we will update the component when we will be implementing it.

@asvinb
Copy link
Collaborator

asvinb commented Apr 1, 2022

QA: ✔️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Module: AdSense Google AdSense module related issues P0 High priority QA: Eng Requires specialized QA by an engineer Rollover Issues which role over to the next sprint Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

5 participants