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

Pass specific dates in AdSense deep links #2689

Closed
felixarntz opened this issue Jan 26, 2021 · 6 comments
Closed

Pass specific dates in AdSense deep links #2689

felixarntz opened this issue Jan 26, 2021 · 6 comments
Labels
Module: AdSense Google AdSense module related issues P1 Medium priority Rollover Issues which role over to the next sprint Type: Enhancement Improvement of an existing feature
Milestone

Comments

@felixarntz
Copy link
Member

felixarntz commented Jan 26, 2021

Similar to #2286 and #2287, we should include specific dates in AdSense deep links for all new datastore-driven components (i.e. those components using select( 'core/user' ).getDateRangeDates()).


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

Acceptance criteria

  • All AdSense frontend deep links (specifically "source links", not links in AdSense setup or settings) in JS components that rely on the datastore should be updated to include arguments indicating the dates from the current date range, so that the AdSense FE will show metrics for that specific date range.
  • Such links should include the date in a d query parameter, such as &d=2020%2F12%2F19-2021%2F01%2F17.

Implementation Brief

  • Using /assets/js/modules/analytics/components/dashboard/AnalyticsAdSenseDashboardWidgetLayout.js
    • Query the user store to get the date range:
       const dateRangeDates = useSelect( ( select ) => select( CORE_USER ).getDateRangeDates( {
       	offsetDays: DATE_RANGE_OFFSET,
       } ) );
    • The DATE_RANGE_OFFSET should be imported from /assets/js/modules/adsense/datastore/constants.js
  • Create new file /assets/js/modules/adsense/util/report-date-range-args and using newly created file,
    • Create a new function generateDateRangeArgs which accepts the returned object from the getDateRangeDates selector. The object contains the following keys:
      • startDate
      • endDate
    • The generateDateRangeArgs should return an object with the following key, d:
       d: `${ startDate.replace( /-/g, '/' ) }-${ endDate.replace( /-/g, '/' ) }`
    • For e.g { d: '2021/01/01-2021/01/27' }
  • Using /assets/js/modules/adsense/datastore/service.js, update the getServiceAccountSiteURL selector to include a parameter urlParams (object) which will be appended to the URL returned by the selector. In the query constant, spread the urlParams parameter.
  • Using /assets/js/modules/analytics/components/dashboard/AnalyticsAdSenseDashboardWidgetLayout.js
    • Call the generateDateRangeArgs function and pass the returned object as args to the getServiceAccountSiteURL selector.
  • The only other component which has AdSense deep links is the AdSenseDashboardWidget component. However we need to refactor this class based component into a functional one to avoid using legacy functions.
  • Using /assets/js/modules/adsense/components/dashboard/AdSenseDashboardWidget.js
    • Refactor component to be a functional one without using legacy functions such as:
      • getModulesData from which we get the homepage value. This can be refactored to use the getServiceAccountSiteURL like in /assets/js/modules/analytics/components/dashboard/AnalyticsAdSenseDashboardWidgetLayout.js
      • Instead of getting the dateRange via props, use the getDateRange from the user store, like in AnalyticsAdSenseDashboardWidgetLayout
      • To check if AdSense is connected, instead of using modulesData.adsense.setupComplete, query the modules store and use the isModuleConnected selector to get this information. For e.g
       const adSenseModuleConnected = useSelect( ( select ) => select( CORE_MODULES ).isModuleConnected( 'adsense' ) );

Test Coverage

  • Add new tests for the generateDateRangeArgs function.
  • Add new tests for the new urlParams parameter to the getServiceAccountSiteURL selector.

Visual Regression Changes

  • None expected

QA Brief

  • Go to the AdSense dashboard
  • Inspect each of the 'See full stats...' links:
    • the path should be https://www.google.com/adsense/new/u/0/{ACCOUNT_ID/reporting
    • the query parameters should contain the new d parameter to reflect the selected date range, e.g.:
      https://www.google.com/adsense/new/u/0/{ACCOUNT_ID}/reporting?d=2021/01/12-2021/02/08&authuser={EMAIL_ADDRESS}

Changelog entry

  • Include current dates in AdSense deep links so that the service frontend shows the same time period as Site Kit.
@felixarntz felixarntz added P1 Medium priority Type: Enhancement Improvement of an existing feature Next Up Module: AdSense Google AdSense module related issues labels Jan 26, 2021
@felixarntz felixarntz assigned felixarntz and unassigned felixarntz Jan 26, 2021
@asvinb asvinb assigned asvinb and unassigned asvinb Jan 27, 2021
@fhollis fhollis added this to the Sprint 42 milestone Jan 28, 2021
@tofumatt tofumatt self-assigned this Jan 28, 2021
@tofumatt
Copy link
Collaborator

IB ✅

@aaemnnosttv
Copy link
Collaborator

@johnPhillips would you please add a QA brief here?

@johnPhillips
Copy link
Contributor

QA brief added, let me know if there are any questions

@aaemnnosttv aaemnnosttv assigned felixarntz and unassigned aaemnnosttv Feb 8, 2021
@felixarntz felixarntz assigned johnPhillips and unassigned felixarntz Feb 8, 2021
@fhollis fhollis modified the milestones: Sprint 42, Sprint 43 Feb 15, 2021
@fhollis fhollis added the Rollover Issues which role over to the next sprint label Feb 15, 2021
@felixarntz felixarntz removed their assignment Feb 18, 2021
@wpdarren wpdarren self-assigned this Feb 18, 2021
@wpdarren
Copy link
Collaborator

QA Update: Engineer to confirm ⚠️

@johnPhillips I don't want to assume so would like to confirm the URL is correct:

When I click on the See full stats in AdSense link this is the URL that loads.

https://www.google.com/adsense/new/u/0/pub-7603478468516085/reporting?d=2021%2F01%2F21-2021%2F02%2F17&authuser=darren.cronian%40get10up.com

I am assuming that the %2F etc. is just the HTML code for the slashes / and spaces.

The URL in the QAB suggests that there should be slashes and a hyphen.

?d=2021/01/12-2021/02/08

Please could you confirm that this is the URL you would expect to see, and then I will continue the QA.

@johnPhillips
Copy link
Contributor

@wpdarren That's correct, the %2F is just escaped slashes - this is called percent encoding.

The proof in the pudding is that the link should work, i.e. you see the date selection in the AdSense stats.

@wpdarren
Copy link
Collaborator

QA Update: Pass ✅

Verified:

  • Can confirm the path is https://www.google.com/adsense/new/u/0/{ACCOUNT_ID/reporting
  • Can confirm that the URL works and redirects me to Google Adsense
  • Can confirm that the dates changes based on the date period, i.e. 7, 14,28 and 90 days.

For 7 days /reporting?d=2021%2F02%2F11-2021%2F02%2F17&authuser=darren.cronian%40get10up.com

For 14 days /reporting?d=2021%2F02%2F04-2021%2F02%2F17&authuser=darren.cronian%40get10up.com

For 28 days /reporting?d=2021%2F01%2F21-2021%2F02%2F17&authuser=darren.cronian%40get10up.com

For 90 days /reporting?d=2020%2F11%2F20-2021%2F02%2F17&authuser=darren.cronian%40get10up.com

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 P1 Medium priority 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

8 participants