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: Implement /reports/customers endpoint #916

Closed
timmyc opened this Issue Nov 23, 2018 · 8 comments

Comments

3 participants
@timmyc
Copy link
Collaborator

timmyc commented Nov 23, 2018

p2: p7bje6-16h-p2
swagger: https://app.swaggerhub.com/apis/peterfabian/wc-v3-api/1.0.0#/default/get_reports_customers

Currently the /reports/customers endpoint has yet to be fully implemented. It appears the new endpoint will require another reporting table to be built, so for this issue we will need the required table to be built, along with the endpoint logic.

@timmyc timmyc added the REST API label Nov 23, 2018

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

@timmyc

This comment has been minimized.

Copy link
Collaborator Author

timmyc commented Dec 7, 2018

Noting that during our planning session today we discussed that support for customers registration filter does not exist in the swagger hub implementation.

@timmyc timmyc moved this from wc-admin Backlog 🔙 to 💪Sprint 9 Backlog in Isotope Dec 7, 2018

@jeffstieler jeffstieler self-assigned this Dec 10, 2018

@jeffstieler

This comment has been minimized.

Copy link
Contributor

jeffstieler commented Dec 19, 2018

@timmyc - question regarding the initial lookup table backfill..

It seems obvious that we want to backfill the customer lookup table with existing (registered) customers.. but what about "existing" guest customers? Should we pull all the unique email addresses from guest orders and backfill them?

@timmyc

This comment has been minimized.

Copy link
Collaborator Author

timmyc commented Dec 19, 2018

Wonder if we can hook some logic into the existing back-fill operation of the orders lookup table to also preform the requisite tasks regarding customers. @peterfabian any thoughts on that approach?

@jeffstieler

This comment has been minimized.

Copy link
Contributor

jeffstieler commented Dec 19, 2018

More questions:

  • The API spec doesn't have parameters for "last order" (we agreed to add on p2) - any objections to adding that?
  • The API spec uses "before" and "after" for customer registration date.. how about we rename to "registered_before" and "registered_after" ?
  • How should the "name" parameter work? We currently have separate columns for first and last name.
  • The API spec has a "username" parameter, but we don't have that in the lookup table. Should we remove the parameter, or add username to the lookup table?
  • The item schema for /reports/customers should match the lookup table right? (It currently doesn't)
@timmyc

This comment has been minimized.

Copy link
Collaborator Author

timmyc commented Dec 19, 2018

The API spec doesn't have parameters for "last order" (we agreed to add on p2) - any objections to adding that?

Nope sounds reasonable to me.

The API spec uses "before" and "after" for customer registration date.. how about we rename to "registered_before" and "registered_after" ?

I believe we have standardized on before and after in the context of all other stat endpoints for date range queries - but given we have multiple timestamp attributes on the object, I think being explicit with the registered_ prefix makes sense.

How should the "name" parameter work? We currently have separate columns for first and last name.

I would imagine we would want to search against both fields when this argument is sent along

The API spec has a "username" parameter, but we don't have that in the lookup table. Should we remove the parameter, or add username to the lookup table?

I think adding username to the lookup table makes sense, or not supporting it since we have the name option.

The item schema for /reports/customers should match the lookup table right? (It currently doesn't)

The schema returned by the endpoint should describe the shape/attributes of the return values from the API - so likely won't fully match the lookup table.

@jeffstieler

This comment has been minimized.

Copy link
Contributor

jeffstieler commented Dec 19, 2018

How should the "name" parameter work? We currently have separate columns for first and last name.

I would imagine we would want to search against both fields when this argument is sent along

Ok so are we matching on either the first or last name, or a concatenation of the two?

I think adding username to the lookup table makes sense, or not supporting it since we have the name option.

I'll add it.

The schema returned by the endpoint should describe the shape/attributes of the return values from the API - so likely won't fully match the lookup table.

It should at least contain the values displayed in the report table, right?

@timmyc

This comment has been minimized.

Copy link
Collaborator Author

timmyc commented Dec 19, 2018

Ok so are we matching on either the first or last name, or a concatenation of the two?

To me it seems like just basing this off the last name might simplify things, and be much more likely to happen then searching for first name. @LevinMedia ?

It should at least contain the values displayed in the report table, right?

It should really be a representation of the shape/types of values returned by the API. Here are some docs around what it should represent.

@LevinMedia

This comment has been minimized.

Copy link
Contributor

LevinMedia commented Dec 19, 2018

A concatenation of the two seems like it would be ideal. @timmyc I could see most people starting to search by last name, but I think we want the first as well, as there will inevitably be folks with the same last name.

@timmyc timmyc moved this from 💪Sprint 9 Backlog to 🛋Sprint 10 Backlog in Isotope Dec 28, 2018

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

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