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

Add products and product stats analytics reports to API doc #249

Conversation

ianlin
Copy link
Member

@ianlin ianlin commented Aug 16, 2024

Part of #248. This PR intends to merge into a feature branch feature/add-wc-analytics-reports-api-doc.

This PR adds products and product stats APIs to the analytics doc:

  • Add a wc-analytics link in wc/v3 introduction.
  • Add introduction section
  • Add authentication section
  • Add index section
  • Add product reports section (/wc-analytics/reports/products)
  • Add product stats reports section (/wc-analytics/reports/products/stats)

@woocommerce/proton Can you help on reviewing the structure of the API doc? The content will be reviewed by @woocommerce/automata. Thank you.

Detailed test instruction

  1. Build the doc by running ./build.sh
  2. Open build/index.html
  3. Click Analytics docs
  4. Verify the structure and the content in the Analytics API doc.

@ianlin ianlin self-assigned this Aug 16, 2024
@ianlin
Copy link
Member Author

ianlin commented Aug 16, 2024

@woocommerce/proton
@woocommerce/automata

I couldn't find team proton/automata in the reviewer section, probably needs someone to add it in the repo settings.

@barryhughes barryhughes requested review from a team and vedanshujain and removed request for a team August 19, 2024 17:31
@ianlin ianlin requested a review from a team August 21, 2024 02:58
Copy link
Contributor

@vedanshujain vedanshujain left a comment

Choose a reason for hiding this comment

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

Awesome work, much needed. Added few questions, but looks great!

- **Internally Used**: Primarily intended for internal use within WooCommerce, such as powering the Analytics section in the admin dashboard.
- **Non-Versioned**: Currently, it does not use versioning, allowing WooCommerce developers to make changes as needed to support analytics features.

To use the `/wc-analytics` API, you must enable WooCommerce Analytics in your store's settings. Although it is not part of the public API like `/wc/v3`, it can still be accessed and used for custom development and integrations.
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose we are marking this public API if we allow it to be used for custom development.

```

```javascript
WooCommerce.get("reports")
Copy link
Contributor

Choose a reason for hiding this comment

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

are we upgrading the SDKs as part of this PR, or is this something that automatically happens?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we don't need to upgrade the SDKs, I've added in the doc that we need to use wc-analytics instead of wc/v3 when specifying version in the SDK settings. The code examples were also added for setting for all languages, here's the PHP one.

I tried locally using PHP, Python, and Node.js and they can get the results under /wc-analytics namespace.

```json
[
{
"slug": "performance-indicators",
Copy link
Contributor

Choose a reason for hiding this comment

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

should we document all properties as well for these reports?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the suggestion, I will add them in the upcoming PRs. Added as todo tasks it in the epic issue, along with the APIs for Import/Export as well: #248

Copy link
Member

@eason9487 eason9487 left a comment

Choose a reason for hiding this comment

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

Thanks for the work.

Found a few minor issues but I think their fixes don't need a new round.

source/wc-analytics.html.md Outdated Show resolved Hide resolved
source/wc-analytics.html.md Outdated Show resolved Hide resolved
source/includes/wc-analytics/_introduction.md Outdated Show resolved Hide resolved
source/includes/wc-analytics/_introduction.md Outdated Show resolved Hide resolved
@ianlin
Copy link
Member Author

ianlin commented Aug 23, 2024

Hi @vedanshujain, thanks for reviewing. I have updated the docs according to your suggestions, it's ready for another look.

@ianlin ianlin requested a review from vedanshujain August 23, 2024 04:39
Copy link
Contributor

@vedanshujain vedanshujain left a comment

Choose a reason for hiding this comment

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

LGTM! Although the 36k line change was quite hard to review, so we might be missing things there.

Copy link
Contributor

Choose a reason for hiding this comment

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

(optional) We seem to be repeating the pagination args, maybe we can move to it's own section

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the suggestion, I will work on it in the upcoming PRs.

@ianlin ianlin merged commit 3dc6fc6 into feature/add-wc-analytics-reports-api-doc Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants