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.

Sign up for GitHub

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

Jump to bottom

feat: create endpoint to get users #81

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open

feat: create endpoint to get users #81

wants to merge 31 commits into from

Conversation

Luana-ns
Copy link
Contributor

@Luana-ns Luana-ns commented Oct 18, 2023 •
edited by frattezi
Loading

Description

This PR fully implements the GET Users route, it includes:

  • Endpoint for GET /users
  • New fetchUsers DAO method
  • Create a shared pagination helper with basic validations that can be imported and merged with other Zod objects.
  • Defined a basic return interface for paginated calls, FE will need: data, total count and pagination info, WDYT?

TODO

I plan to finish this on my weekend

  • I want to add tests
  • Zod errors in the API params are not being automatically handled by nextjs API. Checking best way of doing that

Demo

@ogatalars @Luana-ns please also review this PR, comment doubts and suggestions.

@Luana-ns Luana-ns linked an issue Oct 18, 2023 that may be closed by this pull request
Create endpoint to get all Users #68
Open
@vercel
Copy link

vercel bot commented Oct 18, 2023 •
edited
Loading

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

Name Status Preview Comments Updated (UTC)
app ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 8, 2024 10:23pm

@Luana-ns Luana-ns added the enhancement New feature or request label Oct 18, 2023
@Luana-ns Luana-ns added this to the Admin milestone Oct 18, 2023
guilherme-bbsa
guilherme-bbsa approved these changes Oct 24, 2023
View reviewed changes
Copy link
Contributor

@guilherme-bbsa guilherme-bbsa left a comment

Choose a reason for hiding this comment

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

god job

@marco-souza
Copy link
Contributor

FYI, I think @Luana-ns have some changes to be pushed here to make this work mode "native" to nextjs

frattezi
frattezi reviewed Nov 22, 2023
View reviewed changes
src/app/api/users/user.dao.ts Outdated Show resolved Hide resolved
src/dao/user.dao.ts Outdated Show resolved Hide resolved
src/app/api/users/route.ts Outdated Show resolved Hide resolved
marco-souza
marco-souza reviewed Feb 15, 2024
View reviewed changes
src/app/api/users/route.ts Outdated Show resolved Hide resolved
src/app/api/users/route.ts Outdated Show resolved Hide resolved
marco-souza
marco-souza requested changes Mar 4, 2024
View reviewed changes
Copy link
Contributor

@marco-souza marco-souza left a comment •
edited
Loading

Choose a reason for hiding this comment

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

We need to be careful with the things we're changing, as they are also used on the App UI, not only on the admin.

Also, let's study a little more about:

  • next.js
  • zod
  • how raise works

I left a few links for reference

src/app/api/users/route.ts Outdated Show resolved Hide resolved
src/app/api/users/route.ts Outdated Show resolved Hide resolved
src/app/api/users/route.ts Outdated Show resolved Hide resolved
src/app/api/users/route.ts Outdated Show resolved Hide resolved
src/dao/user.dao.ts Outdated Show resolved Hide resolved
src/dao/user.dao.ts Outdated Show resolved Hide resolved
vzsoares
vzsoares reviewed Mar 4, 2024
View reviewed changes
src/app/api/users/route.ts Outdated Show resolved Hide resolved
frattezi
frattezi reviewed Mar 6, 2024
View reviewed changes
src/dao/user.dao.ts Outdated Show resolved Hide resolved
@frattezi
Copy link

frattezi commented Mar 6, 2024

Hey @marco-souza we solved most issues here, just need to enhance some final points, let you know when this is ready

marco-souza
marco-souza reviewed Mar 7, 2024
View reviewed changes
src/dao/user.dao.ts Show resolved Hide resolved
src/dao/user.dao.ts Show resolved Hide resolved
src/dao/user.dao.ts Outdated Show resolved Hide resolved
src/app/api/users/route.ts Show resolved Hide resolved
@ogatalars ogatalars marked this pull request as ready for review May 16, 2024 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marco-souza marco-souza marco-souza requested changes

@frattezi frattezi frattezi left review comments

@ogatalars ogatalars ogatalars left review comments

@vzsoares vzsoares vzsoares left review comments

@guilherme-bbsa guilherme-bbsa Awaiting requested review from guilherme-bbsa

Requested changes must be addressed to merge this pull request.

Assignees

@Luana-ns Luana-ns

@ogatalars ogatalars

Labels
enhancement New feature or request
Projects
None yet
Milestone
Admin
Development

Successfully merging this pull request may close these issues.

Create endpoint to get all Users
6 participants
@Luana-ns @marco-souza @frattezi @ogatalars @vzsoares @guilherme-bbsa