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

REST API: Create new /v4 of Orders Endpoint #950

Closed
psealock opened this Issue Nov 28, 2018 · 5 comments

Comments

6 participants
@psealock
Copy link
Collaborator

psealock commented Nov 28, 2018

Thus far, populating the orders table from the established /orders endpoint has been working great. Filtering by Advanced Filters in #943 will introduce query parameters which have no meaning in the context of the endpoint.

We can keep using /orders by filtering client side. For example, an advanced filter of ?status_is_not=pending can be turned into ?status=... using wcSettings.orderStatuses to explicitly ask for all but pending orders.

product_includes=65 can also be translated to the correct query argument (product), but only accepts a single value. We'll need to filter by multiple ids.

The limitations are clear with query argument &customers=new. We can get customer data via /customer and compare results with customer_id from the order, but that won't give an indication of new/returning.

Likewise, we can't filter by coupon in the /orders endpoint.

Seems to me like we'll need to write /reports/orders. Do you agree @peterfabian ?

@psealock psealock added the REST API label Nov 28, 2018

@peterfabian

This comment has been minimized.

Copy link
Contributor

peterfabian commented Nov 28, 2018

I think you're right. Maybe we could add required filters to /orders endpoint?

Because otherwise, /reports/orders would return the same data (or a subset of data from orders), and would just add support for extra filters, if I understand the problem correctly?

@claudiosanches what is your view on this? Do you have any preference regarding new endpoint/updating /orders?

@timmyc timmyc added this to Project Backlog 🔙 in Isotope Nov 29, 2018

@claudiosanches

This comment has been minimized.

Copy link
Member

claudiosanches commented Nov 30, 2018

@claudiosanches I happy to introduce new endpoints for reports.
When possible, it's always great to introduce new filters into the current endpoints, still most of filters here probably more related to reports, so I don't see any problem having a new endpoint, also probably better that we don't need to list so many data at once too, since our reports displays less data and having the IDs it's possible to query the regular endpoint for more details.

It's nice that we are going back to our initial idea, where we introduce all fields required by our new reporting system, and doing less requests 😃

@psealock

This comment has been minimized.

Copy link
Collaborator Author

psealock commented Dec 4, 2018

Maybe we could add required filters to /orders endpoint?

If we go this route, the work will be in core, right? In order to augment the /orders endpoint, we'll need to do the following:

  • Change product query argument to also accept multiple values, eg &product=22,47,89.
  • Add coupon query argument, accepting multiple values, eg &coupon=sale123,family_discount.
  • Add customer query argument, accepting values new and returning.

It seems to me this is the right way to proceed because its simply adding more capabilities to an existing endpoint with backwards compatibility.

@timmyc timmyc changed the title Investigate need for `/reports/orders` endpoint REST API: Create new /v4 of Orders Endpoint Dec 26, 2018

@timmyc

This comment has been minimized.

Copy link
Collaborator

timmyc commented Dec 26, 2018

Seems like if we want to fully support the advanced filter query args, we will likely need to version the orders endpoint to /v4 - or as suggested above we can try to use this filter to manipulate the requisite query args:

Advanced Filter Arg /wc/v3 Argument
status_is (string) status (string)
status_is_not (string) NA
product_includes (int or array) product (int)
product_excludes (in or array) NA
coupon_includes (int or array) NA
coupon_excludes (int or array) NA
customer (string) customer (int)

The most challenging of the above is customer due to the mis-match of data types - which we could either update the logic in the orders stats endpoint to accept an arg of customer_type perhaps - and match that in a new /orders endpoint or vice-versa.

Given the gaps of the required advanced filter args contrasted to the current options in the /v3 endpoint - it seems like creating a new version 4 here in wc-admin is the best route forward. As such I've updated the issue title to reflect that need. I also think that these arg additions to the base orders route could be useful for other developers and all make sense as good additions to the core endpoint ( though would still love to hear thoughts on this from @mikejolley @peterfabian and @claudiulodro )

@timmyc timmyc moved this from wc-admin Backlog 🔙 to 🛋Sprint 10 Backlog in Isotope Dec 26, 2018

@claudiulodro

This comment has been minimized.

Copy link

claudiulodro commented Jan 2, 2019

I recommend using v4 endpoints for everything in wc-admin for the following reasons:

  • We can't add any of the new endpoints into the v3 namespace.
  • It doesn't make sense to mix and match endpoint namespaces in a feature.
  • It also wouldn't really make sense to have a v4 namespace that doesn't contain all of the existing core products, orders, etc. endpoints.

Let me know if you have any questions or need clarification about anything.

@joshuatf joshuatf self-assigned this Jan 7, 2019

Isotope automation moved this from 🛋Sprint 10 Backlog to 🛋Sprint 10 Done Jan 15, 2019

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