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

feat(customer): list customer #6206

Merged
merged 4 commits into from
Jan 29, 2024
Merged

feat(customer): list customer #6206

merged 4 commits into from
Jan 29, 2024

Conversation

srindom
Copy link
Collaborator

@srindom srindom commented Jan 24, 2024

What

  • GET /admin/customers
  • GET /admin/customer-groups

Copy link

changeset-bot bot commented Jan 24, 2024

⚠️ No Changeset found

Latest commit: a63119d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

vercel bot commented Jan 24, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
medusa-dashboard ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 29, 2024 4:36pm
3 Ignored Deployments
Name Status Preview Comments Updated (UTC)
api-reference ⬜️ Ignored (Inspect) Jan 29, 2024 4:36pm
docs-ui ⬜️ Ignored (Inspect) Visit Preview Jan 29, 2024 4:36pm
medusa-docs ⬜️ Ignored (Inspect) Visit Preview Jan 29, 2024 4:36pm

@srindom srindom force-pushed the feat/customer-endpoints branch from baeccb9 to c37b269 Compare January 26, 2024 20:35
@srindom srindom marked this pull request as ready for review January 26, 2024 20:36
@srindom srindom requested review from a team as code owners January 26, 2024 20:36
import { MedusaRequest, MedusaResponse } from "../../../types/routing"

export const GET = async (req: MedusaRequest, res: MedusaResponse) => {
const customerModuleService = req.scope.resolve<ICustomerModuleService>(
Copy link
Contributor

Choose a reason for hiding this comment

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

We can align how we want to build the GET endpoints.
I believe we should use the remoteQuery to be able to easily expand relations from here.

We have 2 utils that can help on that:
remoteQueryObjectFromString and remoteQueryObjectToString

we can discuss it on Monday and come up with a pattern to follow in all the api-v2 endpoints. cc: @adrien2p @olivermrbl

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agree remote query should be the way to go - do we have an example implementation of the approach somewhere?

Copy link
Contributor

Choose a reason for hiding this comment

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

We have mixed examples that we need to normalize from now on.
I believe this test is a good starting point, where we have a list of all default fields and relations to be listed. We need to think about and implement how to have a list of "allowed" relations to be expanded, and how to customize this list, adding custom relations to it.

from there it is used like this:

const remoteQuery = container.resolve("remoteQuery")

const query = remoteQueryObjectFromString({
  entryPoint: "product",
  variables: {},
  fields,
})

const results = await remoteQuery(query)

variables are handled here before calling module.list(filters, options)
We can send all the filters and options needed to the service from here.

As this will be replicate in most GET endpoints, I think it worth spending some time in it, even if in another PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think I will need to see an example with filters and expands applied - I wasn't able to get this to work for these.

I suggest that we merge PRs without RemoteQuery until we have its feature set more standardized.

Copy link
Contributor

@carlos-r-l-rodrigues carlos-r-l-rodrigues left a comment

Choose a reason for hiding this comment

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

LGTM

@kodiakhq kodiakhq bot merged commit 8739070 into develop Jan 29, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants