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

416 implement organic session backend endpoint using the google analytics api #22023

Conversation

leonidasmi
Copy link
Contributor

@leonidasmi leonidasmi commented Feb 7, 2025

Context

Summary

This PR can be summarized in the following changelog entry:

  • Builds the endpoint route for the Organic Sessions widget.
  • Corrects the date ranges of existing endpoints for Top 5 pages and Top Queries

Relevant technical choices:

  • Right now, we have heavily coupled our parsing of Site Kit responses with what we request. I'm not sure if it's the best way to avoid breaking changes in Site Kit API and it feels like we're re-inventing the wheel with how we parse the responses, but I'm not sure I have a better answer right now so it would be nice if the reviewer had a critical view on that as well.
    • With this commit we safeguard our parsing of responses to be sure that eg. the dimensionValues[0]->value of the rows of a request that asked for daily data is in fact a day. To make sure of that, we check if our request had date as the only dimension, because otherwise our parsing of responses will break. This means that
      • When we add more kinds of Analytics requests, we will need to enhance that safeguard and our parsing of responses
      • If Site Kit changes something in their internal API (eg. if they automatically add a dimension in their Google API requests), our integration will break.
    • To improve a bit on the above, I added a @TODO for properly accessing the properties of the responses and created a task to take care of that. The task also describes the same for the Search Console responses that we have already implemented.
    • It will help if, we use this task to fully document how we interact with Site Kit APIs and external Google APIs. That way we will have a good overview ourselves and it will be shared to the Google team to avoid breaking changes in those areas.
  • For the order_by Analytics parameter: I have constructed its format based on how Site Kit does it and also how Google docs say. I've even compared a request coming from us with a request coming from Site Kit and it results in the same orderBy format reaching external Google APIs. HOWEVER, I couldn't see it working properly with other arguments than the current one, but I might be missing something. I'm mentioning this for the future, if we ever need to order in a different way. For now, the order of the response is how we want it.
  • The response parsers in the analytics adapter were created so that they can deal with multiple metrics and dimensions, even though we currently do requests with single metrics and dimensions. For that reason, I have added handling of requesting also the total users, so that we can have a POC of it working that way too.

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

Note: Wherever it's mentioned that you have to cross-reference the results you got with the ones you see in the Google Analytics/Search Console platforms, you can check the Google Search Console & Analytics tutorial Feb 11 2025 tutorial we have in our Google Drive.


To test the new endpoints:

  • For getting the data about each of the last 28 days (that will build the relevant graph), do a GET request to /wp-json/yoast/v1/time_based_seo_metrics?options[widget]=οrganicSessionsDaily
  • Confirm that the response you get is like:
[
    {
        "date": "20250117",
        "sessions": 2
    },
    {
        "date": "20250118",
        "sessions": 0
    },
...
    {
        "date": "20250213",
        "sessions": 0
    }
  • Make sure that the date range are the last 28 days without including today's date (so, the above example is for a request that happened on the 14/2/2025

  • Go to the actual Google Analytics platform and search for organic sessions of the last 28 days and confirm that they data you got are the same with the data you see in that platform.

  • One easy way to check that is to go to the Traffic Acquisition page of the analytics and click on the Organic sessions on the graph:
    image

  • For getting the complete data about the last 28 days and the complete data of the 28 days before that, do a GET request to /wp-json/yoast/v1/time_based_seo_metrics?options[widget]=οrganicSessionsCompare

  • Confirm that the response you get is like:

[
    {
        "current": {
            "sessions": 8
        },
        "previous": {
            "sessions": 0
        }
    }
]
  • Go to the actual Google Analytics platform and search for organic sessions of the last 28 days compared with the organic sessions of the 28 days before that and confirm you get the same result.
    • Specifically, you can do that by changing the date range at the top right corner of the Acquisition page in analytics and then enable Compare and select Preceding period:
      image
  • You can check now the sessions from the last 28 days compared with the 28 days before that:
    image

To test that Search Console results are actually corrected (prompted by the fix of date ranges for older implementations):

Note: Sometimes I see Search Console treating last 28 days as the 28 days BEFORE YESTERDAY for some reason, where we treat it as the 28 days before today, which seems more correct. If you see differences in the test below or in future tests, try to move the date range in Search Console a day forward.

  • Do the requests for the top 5 pages and top queries (/wp-json/yoast/v1/time_based_seo_metrics?options[widget]=page and /wp-json/yoast/v1/time_based_seo_metrics?options[widget]=query respectively) or actually look at the widgets if they have been implemented in the frontend
  • Check the results and compare them with the same results in Search Console
  • Confirm that they are the same results ⬇️
  • For the top queries, you can go to the Performance tab of the search console, select the Queries section and you can find the results there. To check all the metrics we also return, enable them in the section above the graph and you can look at each query under the graph:
    image
  • For the top page, it's similar though you just check the Pages section:
    image

To test requesting multiple metrics:

  • In src\dashboard\user-interface\time-based-seo-metrics\time-based-seo-metrics-route.php change the $request_parameters->set_metrics( [ 'sessions' ] ); lines to $request_parameters->set_metrics( [ 'sessions', 'totalUsers' ] ); in both lines.
  • That way you're requesting two metrics in one go
  • Do both requests and confirm that the response you get is enhanced with that metric, like so:
[
    {
        "date": "20250116",
        "sessions": 1,
        "total_users": 1
    },
    {
        "date": "20250117",
        "sessions": 2,
        "total_users": 2
    },
...

and

[
    {
        "current": {
            "sessions": 7,
            "total_users": 7
        },
        "previous": {
            "sessions": 0,
            "total_users": 0
        }
    }
]
  • Check both results similarly to how you check the sessions above. This time, for users you can look into the User acquisition tab:
    image

  • Because total users will probably be the same number with sessions, you can also request for a different metric, like Event Counts.

    • In order to do that, in src\dashboard\user-interface\time-based-seo-metrics\time-based-seo-metrics-route.php you will have to change the $request_parameters->set_metrics( [ 'sessions' ] ); line to $request_parameters->set_metrics( [ 'sessions', 'eventCount' ] );
    • But also, in src\dashboard\infrastructure\analytics-4\site-kit-analytics-4-adapter.php, you have to change elseif ( $metric->name === 'totalUsers' ) { to elseif ( $metric->name === 'eventCount' ) {
  • You can check again your response to look like this:

[
    {
        "date": "20250116",
        "sessions": 2,
        "total_users": 8
    },
    {
        "date": "20250117",
        "sessions": 2,
        "total_users": 3
    },
...

and

[
    {
        "current": {
            "sessions": 8,
            "total_users": 59
        },
        "previous": {
            "sessions": 0,
            "total_users": 0
        }
    }
]
  • This time, the total_users refer to the Event Count metric. You should confirm those results with Analytics ⬇️

  • For the first request, I was able to find the relevant graph by searching for event counts of organic search in the Analytics search. After going there, you should check the graph points with the total_users points of the first response.
    image

  • For the second request, you can see the Event Count column in the Traffic Acquisition tab (after you have added a comparison date range like described in previous steps):
    image


To test doing an Analytics request while not authorized:

  • Do a /wp-json/yoast/v1/time_based_seo_metrics?options[widget]=οrganicSessionsDaily or /wp-json/yoast/v1/time_based_seo_metrics?options[widget]=οrganicSessionsCompare request as a user that has no viewing rights of Analytics
  • Confirm you get a 403 response with a "error": "The Analytics 4 request failed: Site Kit can’t access the relevant data from Analytics because you haven’t granted all permissions requested during setup." body.

To test asking for unexpected requests:

  • In src\dashboard\user-interface\time-based-seo-metrics\time-based-seo-metrics-route.php change $request_parameters->set_dimensions( [ 'date' ] ); to $request_parameters->set_dimensions( [ 'platform' ] ); and $request_parameters->set_order_by( 'dimension', 'date' ); to $request_parameters->set_order_by( 'dimension', 'platform' );
  • That way we do a request to Site Kit that we haven't added support yet
  • Do a οrganicSessionsDaily request and confirm you get a 400 response with a "error": "The Analytics 4 request is invalid: Unexpected parameters for the request" body

Relevant test scenarios

  • Changes should be tested with the browser console open
  • Changes should be tested on different posts/pages/taxonomies/custom post types/custom taxonomies
  • Changes should be tested on different editors (Default Block/Gutenberg/Classic/Elementor/other)
  • Changes should be tested on different browsers
  • Changes should be tested on multisite

Test instructions for QA when the code is in the RC

  • QA should use the same steps as above.

Impact check

This PR affects the following parts of the plugin, which may require extra testing:

UI changes

  • This PR changes the UI in the plugin. I have added the 'UI change' label to this PR.

Other environments

  • This PR also affects Shopify. I have added a changelog entry starting with [shopify-seo], added test instructions for Shopify and attached the Shopify label to this PR.

Documentation

  • I have written documentation for this change. For example, comments in the Relevant technical choices, comments in the code, documentation on Confluence / shared Google Drive / Yoast developer portal, or other.

Quality assurance

  • I have tested this code to the best of my abilities.
  • During testing, I had activated all plugins that Yoast SEO provides integrations for.
  • I have added unit tests to verify the code works as intended.
  • If any part of the code is behind a feature flag, my test instructions also cover cases where the feature flag is switched off.
  • I have written this PR in accordance with my team's definition of done.
  • I have checked that the base branch is correctly set.

Innovation

  • No innovation project is applicable for this PR.
  • This PR falls under an innovation project. I have attached the innovation label.
  • I have added my hours to the WBSO document.

Fixes https://github.com/Yoast/reserved-tasks/issues/416

Copy link

A merge conflict has been detected for the proposed code changes in this PR. Please resolve the conflict by either rebasing the PR or merging in changes from the base branch.

Base automatically changed from timebasedtrafficroute-update to trunk February 12, 2025 11:41
@leonidasmi leonidasmi added the changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog label Feb 13, 2025
@coveralls
Copy link

coveralls commented Feb 13, 2025

Pull Request Test Coverage Report for Build 5e5b5cce9aa343cb367faeb387e1405c684f37a0

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 195 (0.0%) changed or added relevant lines in 13 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall first build on 416-implement-organic-session-backend-endpoint-using-the-google-analytics-api at 50.776%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/dashboard/application/search-rankings/top-page-repository.php 0 2 0.0%
src/dashboard/domain/analytics-4/failed-request-exception.php 0 2 0.0%
src/dashboard/domain/analytics-4/invalid-request-exception.php 0 2 0.0%
src/dashboard/domain/search-rankings/top-page-data.php 0 4 0.0%
src/dashboard/application/traffic/organic-sessions-repository.php 0 5 0.0%
src/dashboard/domain/traffic/daily-traffic-data.php 0 7 0.0%
src/dashboard/domain/data-provider/parameters.php 0 8 0.0%
src/dashboard/infrastructure/search-console/site-kit-search-console-adapter.php 0 8 0.0%
src/dashboard/domain/traffic/traffic-data.php 0 11 0.0%
src/dashboard/domain/traffic/comparison-traffic-data.php 0 12 0.0%
Totals Coverage Status
Change from base Build b656761056a1b4d001449e8f043bba2e40f4a6e7: 50.8%
Covered Lines: 16742
Relevant Lines: 32972

💛 - Coveralls

@leonidasmi leonidasmi marked this pull request as ready for review February 17, 2025 09:08
Copy link
Contributor

@thijsoo thijsoo left a comment

Choose a reason for hiding this comment

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

CR 🚧 some questions.

@thijsoo thijsoo modified the milestones: 24.6, 24.7 Feb 18, 2025
@thijsoo thijsoo merged commit c11c566 into trunk Feb 18, 2025
27 of 30 checks passed
@thijsoo thijsoo deleted the 416-implement-organic-session-backend-endpoint-using-the-google-analytics-api branch February 18, 2025 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants