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

Endpoint /store/customers/me is not expanding relations #5091

Open
pevey opened this issue Sep 16, 2023 · 8 comments
Open

Endpoint /store/customers/me is not expanding relations #5091

pevey opened this issue Sep 16, 2023 · 8 comments

Comments

@pevey
Copy link
Contributor

pevey commented Sep 16, 2023

Bug report

Describe the bug

The store/customers/me endpoint does not expand relations.

A request to

http://localhost:9000/store/customers/me?expand=groups

returns a 200 response that does not include groups as expected. Similarly, a request to

http://localhost:9000/stores/customers/me?expand=orders

returns a 200 response that does not include orders.

System information

Medusa version (including plugins): 1.15.0
Node.js version: 18.17.1
Database: postgres
Operating system: ubuntu
Browser (if relevant):

Link to relevant Discord thread: https://discord.com/channels/876835651130097704/1124319911745949787

@fxmb
Copy link

fxmb commented Sep 16, 2023

Thanks for flagging this here @pevey

@pevey
Copy link
Contributor Author

pevey commented Sep 16, 2023

Screenshot 2023-09-16 092620

A bit more info: This image is from @medusajs/medusa/src/api/routes/store/customers/get-customer.ts

This is to confirm that there is currently no logic on this route to include relations.

allowedStoreCustomersRelations is defined in index.ts, but never imported into get-customer.ts. Also, allowedStoreCustomersRelations appears to be incomplete. The documentation suggests more relations should be available.

Screenshot 2023-09-16 092841

@olivermrbl
Copy link
Contributor

Thanks for highlighting @pevey. Would you be up for contributing with a PR? 🙏

@pevey
Copy link
Contributor Author

pevey commented Sep 18, 2023

I did think about it, but I wasn't sure I could do it in the exact style preferred.

I will give it a shot using the parallel admin endpoint (https://github.com/medusajs/medusa/blob/develop/packages/medusa/src/api/routes/admin/customers/get-customer.ts) as a guide.

@pevey
Copy link
Contributor Author

pevey commented Sep 18, 2023

There's also the somewhat related issue that the customer object returned from a successful call to /store/auth includes the relations orders and shipping_addresses. This results in unnecessary work on the backend since an auth call to check the session cookie happens on every single request a user makes to the storefront app. This is also discussed in the Discord link in the OP.

This should really be removed at some point. However, doing this would be a breaking change for many people since that data is currently only coming from that endpoint (since order and address relations on the customer endpoint have not been working).

@pevey
Copy link
Contributor Author

pevey commented Sep 18, 2023

Sorry for the message spam, but one additional point to consider:

Looking into this more, and after referencing the API docs, I think this behavior on /store/auth is intended. it avoids a separate network call right after a successful auth call to get the customer object.

Perhaps the real issue is that having a /store/customers/me endpoint is redundant. Perhaps the endpoint should be removed and the docs should be updated to clarify that the customer object can only be obtained from /store routes in response to a successful auth request.

EDIT: And, based on the docs, it seems customer groups can be obtained by passing the expand parameter on the auth endpoint. I'm about to test.

EDIT2: Despite the documentation, /store/auth actually does not support expanding additional relations. The default relations are:

export const defaultRelations = ["orders", "orders.items", "shipping_addresses"]

PLAN: I will adjust both endpoints to support relations as documented. I will also try to adjust the auth endpoint to support 'select' to enable reducing unneeded data flow and db work in a non-breaking way.

@fxmb
Copy link

fxmb commented Oct 11, 2023

Sorry but asking again... When will medusa team approve this change? It is critical for us to offer customer group based pricing!!

@lacymorrow
Copy link

Testing... 1... 2... Could someone merge this PR?

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

No branches or pull requests

4 participants