Skip to content

Send bump stat requests to API endpoint - #988

Merged
fredrikekelund merged 2 commits into
trunkfrom
f26d/bump-stat-endpoint
Feb 28, 2025
Merged

Send bump stat requests to API endpoint#988
fredrikekelund merged 2 commits into
trunkfrom
f26d/bump-stat-endpoint

Conversation

@fredrikekelund

Copy link
Copy Markdown
Contributor

Related issues

Proposed Changes

  • Send bump stats to an intermediate public-api.wordpress.com API endpoint so that we can determine if the request originated from inside A8C's network. If so, mark the stat as a11n.

Testing Instructions

  1. Sandbox public-api.wordpress.com
  2. Apply 175123-ghe-Automattic/wpcom to your sandbox
  3. See the testing instructions in that PR for how to add and monitor log output on your sandbox
  4. Apply the following diff
diff --git a/src/lib/bump-stats.ts b/src/lib/bump-stats.ts
index 7c9f1f2..9cdac76 100644
--- a/src/lib/bump-stats.ts
+++ b/src/lib/bump-stats.ts
@@ -52,7 +52,6 @@ export function bumpAggregatedUniqueStat(
 export function bumpStat( group: string, stat: string, bumpInDev = false ) {
        if ( process.env.E2E || ( process.env.NODE_ENV === 'development' && ! bumpInDev ) ) {
                console.info( `Would have bumped stat: ${ group }=${ stat }` );
-               return false;
        }
 
        // Fire and forget POST request
  1. Start the Studio app
  2. See the testing instructions in the back-end PR for final steps

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?
@fredrikekelund
fredrikekelund requested a review from a team February 27, 2025 10:36
@fredrikekelund fredrikekelund self-assigned this Feb 27, 2025
Comment thread package.json
"follow-redirects": "1.15.6",
"fs-extra": "11.1.1",
"hpagent": "1.2.0",
"node-fetch": "^2.7.0",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is not the latest version of node-fetch. I chose it because it already exists in our dependency tree.

@ivan-ottinger ivan-ottinger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The proposed changes look good to me and work as expected. I shared more feedback in the related backend PR.

@fredrikekelund
fredrikekelund merged commit e4d9ce1 into trunk Feb 28, 2025
@fredrikekelund
fredrikekelund deleted the f26d/bump-stat-endpoint branch February 28, 2025 14:07
bcotrim pushed a commit that referenced this pull request Feb 28, 2025
* Send bump stat requests to API endpoint

* Fix tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants