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

fix(medusa): Implement listAndCount for UserService and update list endpoint #6190

Merged
merged 15 commits into from
Jan 24, 2024

Conversation

kasperkristensen
Copy link
Contributor

@kasperkristensen kasperkristensen commented Jan 23, 2024

What

  • Adds a listAndCount method to the UserService.
  • Updates the list method to use listAndCount and simply return the users only.
  • Updates GET /admin/users to use the new method, and implement the required query params.
  • Makes required changes to medusa-js and medusa-react.

Testing

  • Adds two new tests and updates existing test to not use snapshots.

Closes #6182

@kasperkristensen kasperkristensen requested review from a team as code owners January 23, 2024 16:03
Copy link

changeset-bot bot commented Jan 23, 2024

🦋 Changeset detected

Latest commit: 2f6767f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@medusajs/medusa Patch
@medusajs/medusa-js Patch
medusa-react Patch

Not sure what this means? Click here to learn what changesets are.

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

Copy link

vercel bot commented Jan 23, 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 24, 2024 9:20am
3 Ignored Deployments
Name Status Preview Comments Updated (UTC)
api-reference ⬜️ Ignored (Inspect) Jan 24, 2024 9:20am
docs-ui ⬜️ Ignored (Inspect) Visit Preview Jan 24, 2024 9:20am
medusa-docs ⬜️ Ignored (Inspect) Visit Preview Jan 24, 2024 9:20am

"first_name",
"last_name",
"role",
"api_token",
Copy link
Collaborator

Choose a reason for hiding this comment

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

thought(non-blocking): maybe we don't want this to be exposed to everyone, but only the logged in user? Don't know how it's been in the past though

Copy link
Contributor Author

@kasperkristensen kasperkristensen Jan 23, 2024

Choose a reason for hiding this comment

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

@srindom I thought so to, but the integration tests failed when I initially made it so the listAndCount endpoint didn't return it. I am down to removing it, as I also think it is weird that this very "private" thing is available to all users, but it would be a breaking change in some sense.

It seems like quite a few of the user endpoints expect being able to work with api_token so removing it would be a breaking change for multiple endpoints. I think its best to just leave it for now, and consider not making it part of the User module, or at least maybe only making it accessible to admin users.

@kasperkristensen
Copy link
Contributor Author

/snapshot-this

Copy link
Contributor

🚀 A snapshot release has been made for this PR

Test the snapshots by updating your package.json with the newly published versions:

yarn add @medusajs/authentication@0.0.2-snapshot-20240123175817
yarn add @medusajs/core-flows@0.0.5-snapshot-20240123175817
yarn add @medusajs/customer@0.0.1
yarn add @medusajs/icons@1.2.0-snapshot-20240123175817
yarn add @medusajs/ui@2.3.0-snapshot-20240123175817
yarn add @medusajs/ui-preset@1.1.0-snapshot-20240123175817
yarn add @medusajs/inventory@1.11.5-snapshot-20240123175817
yarn add @medusajs/link-modules@0.2.7-snapshot-20240123175817
yarn add @medusajs/medusa@1.20.1-snapshot-20240123175817
yarn add @medusajs/file-local@1.0.3-snapshot-20240123175817
yarn add medusa-file-minio@1.3.1-snapshot-20240123175817
yarn add medusa-file-s3@1.4.1-snapshot-20240123175817
yarn add @medusajs/medusa-js@6.1.6-snapshot-20240123175817
yarn add medusa-react@9.0.14-snapshot-20240123175817
yarn add @medusajs/modules-sdk@1.12.7-snapshot-20240123175817
yarn add @medusajs/orchestration@0.5.3-snapshot-20240123175817
yarn add @medusajs/payment@0.0.1
yarn add @medusajs/pricing@0.1.8-snapshot-20240123175817
yarn add @medusajs/product@0.3.8-snapshot-20240123175817
yarn add @medusajs/stock-location@1.11.4-snapshot-20240123175817
yarn add @medusajs/types@1.11.11-snapshot-20240123175817
yarn add @medusajs/utils@1.11.4-snapshot-20240123175817
yarn add @medusajs/workflow-engine-inmemory@0.0.2-snapshot-20240123175817
yarn add @medusajs/workflow-engine-redis@0.0.2-snapshot-20240123175817
yarn add @medusajs/workflows-sdk@0.1.2-snapshot-20240123175817

Latest commit: 3023239

@kasperkristensen kasperkristensen merged commit d68089b into develop Jan 24, 2024
16 checks passed
@kasperkristensen kasperkristensen deleted the fix/users-list-impl branch January 24, 2024 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(medusa): User service does not have a listAndCount method
3 participants