Skip to content

Remove order/product count from wcSettings #33064

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.

Sign up for GitHub

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

Jump to bottom
Merged
louwie17 merged 6 commits into from
Jun 1, 2022
Merged

Remove order/product count from wcSettings #33064

louwie17 merged 6 commits into from
Jun 1, 2022

Conversation

@louwie17
Copy link
Contributor

@louwie17 louwie17 commented May 16, 2022

All Submissions:

Changes proposed in this Pull Request:

This PR depends on #32964 and #33063 being merged first (I currently merged their changes into this branch).
The only changes as part of this PR can be found in this commit: 8c2e6e8 aside from the additional changelog.

Closes #32524

How to test the changes in this Pull Request:

  1. Load this branch and finish the onboarding, now create a couple products and orders.
  2. Hide the task list
  3. The Orders and Stock panel should show up, and notice how two request were made to the WC api (one to /wc/v3/products and one to /wc/v3/orders
  4. If you change an order to processing, the correct number should show up in the panel header
  5. If you enable stock management on a product and set stock to zero, the stock panel should show products.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?
  • Have you created a changelog file for each project being changed, ie pnpm nx changelog <project>?

FOR PR REVIEWER ONLY:

  • I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

@github-actions github-actions bot added focus: react admin package: @woocommerce/data issues related to @woocommerce/data plugin: woocommerce Issues related to the WooCommerce Core plugin. labels May 16, 2022
@louwie17 louwie17 requested a review from a team May 16, 2022 18:39
@botwoo
Copy link
Collaborator

botwoo commented May 16, 2022 •
edited
Loading

📊 Test reports for this pull request have been published and are accessible through the following links:

Latest commit referenced in the reports: Fix invalidateResolution call that was misspelled 7fa99a8
This comment will automatically be updated with the latest referenced commit when you push new changes to this pull request.

Visit the WooCommerce Test Reports homepage to view all published reports. See the FAQs page if you're having problems accessing them.

@louwie17 louwie17 force-pushed the update/32524_order_product_count_api_requests_use_data_store branch from 5b1b170 to d9cbfad Compare May 30, 2022 14:10
@github-actions github-actions bot removed the package: @woocommerce/data issues related to @woocommerce/data label May 30, 2022
louwie17
louwie17 commented May 30, 2022
View reviewed changes
plugins/woocommerce-admin/client/homescreen/activity-panel/orders/index.js
.map( ( order ) => order.customer_id )
.filter( ( id ) => id !== 0 ),
_fields: [ 'id', 'name', 'country', 'user_id' ],
} );
Copy link
Contributor Author

@louwie17 louwie17 May 30, 2022

Choose a reason for hiding this comment

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

customer data is not available through the orders API, so we retrieve them from the Analytics customers API, as we link to the customer analytics page.
I added the users param to this API so we can retrieve customers by the user id instead of customer id.

@octaedro octaedro requested review from octaedro and removed request for a team May 31, 2022 12:38
octaedro
octaedro previously approved these changes May 31, 2022
View reviewed changes
Copy link
Contributor

@octaedro octaedro left a comment

Choose a reason for hiding this comment

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

Good job @louwie17! This is testing well here and the code LGTM 🚢

I just noticed a bug related to the Stock panel (the bug wasn't added in this PR).
After updating the stock in the Stock panel, it never changes the loading state.

Screen Capture on 2022-05-31 at 13-18-03

@louwie17
Copy link
Contributor Author

louwie17 commented May 31, 2022

Good job @louwie17! This is testing well here and the code LGTM 🚢

I just noticed a bug related to the Stock panel (the bug wasn't added in this PR). After updating the stock in the Stock panel, it never changes the loading state.

Thanks for the approval @octaedro, I did end up fixing the bug you found as part of this PR as it was relatively simple, but will need a re-approval.
See this commit 7fa99a8 for the fix.

@louwie17 louwie17 requested a review from octaedro May 31, 2022 19:36
octaedro
octaedro approved these changes Jun 1, 2022
View reviewed changes
Copy link
Contributor

@octaedro octaedro left a comment

Choose a reason for hiding this comment

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

Thank you @louwie17 for the quick fix. LGTM 🚀

@louwie17 louwie17 merged commit e02bdd9 into trunk Jun 1, 2022
@louwie17 louwie17 deleted the update/32524_order_product_count_api_requests_use_data_store branch June 1, 2022 17:15
@github-actions github-actions bot added this to the 6.7.0 milestone Jun 1, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jun 1, 2022

Hi @louwie17, thanks for merging this pull request. Please take a look at these follow-up tasks you may need to perform:

  • Add the release: add testing instructions label

@github-actions github-actions bot mentioned this pull request Jul 13, 2022
Closed
@mikkamp mikkamp mentioned this pull request Jul 4, 2024
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@octaedro octaedro octaedro approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

plugin: woocommerce Issues related to the WooCommerce Core plugin.

Projects

None yet

Milestone

6.7.0

Development

Successfully merging this pull request may close these issues.

Use REST API to query orderCount and publishedProductCount

4 participants

@louwie17 @botwoo @octaedro