-
Notifications
You must be signed in to change notification settings - Fork 119
REST API: Migrate leaderboards endpoint #8546
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
Conversation
Generated by 🚫 dangerJS |
You can test the changes from this Pull Request by:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just had a question about whether we support leaderboard stats WC versions under 6.7 - I tried a JN site with WC version 6.3 using the WooCommerce Beta Tester plugin, and the leaderboard API request had a 404 error.
| siteID: siteID, | ||
| path: Constants.path, | ||
| parameters: parameters, | ||
| availableAsRESTRequest: true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reasons we don't allow availableAsRESTRequest: true for the deprecated leaderboards request loadLeaderboardsDeprecated below for sites with WC version under 6.7? is it because application passwords are supported for higher WC version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching - I missed this 😅 Application password only has requirement for WordPress 5.6 minimum, so we should support WC under 6.7 I believe. Updated in c76f163.
Part of #8544
Description
This PR migrates the leaderboards endpoint to enable REST API. Changes include:
LeaderboardsListMapperto parse contents without the data envelopeloadLeaderboardsrequest.Testing instructions
DefaultFeatureFlagService, enable the flag forapplicationPasswordAuthenticationForSiteCredentialLoginand build the app.🔵 Tracked dashboard_top_performers_loaded. This means top performers have been loaded successfully.Please feel free to test again by logging in with a WPCom account. The top performer fetch should still succeed.
Screenshots
N/A
RELEASE-NOTES.txtif necessary.