-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feature/64 edit user profile #180
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/tuanthanh2067/seeksi/2Zdfxw18VC6sRRuWGDvJNhVypE9M |
"return type of editUser request" | ||
type EditUserResponse{ | ||
code: Int! | ||
success: Boolean! | ||
message: [String] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some notes here. Any mutation response should follow a general/standard one (ref: #172) unless they need to add new fields.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll fix this in another issue
bio: String, | ||
distance: Int, | ||
minAge: Int, | ||
maxAge: Int, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
longTerm
and shortTerm
are missing here, but that's ok. We can add them later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the #37 there is no longterm or shortTerm field but yeah we can add it later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, great work 👏 ! I haven't reviewed your validation logic, but the code looks good. I like your idea of returning messages of which field is invalid. Feel free to create a separate issue for refactoring and merge this PR.
Related issue
Fixes #64
Type of Change
Description
TODO:
Screenshot
Authorized user fetches their data
Successful attempt to edit user data
User data is updated
Unsuccessful attempt due to empty values, this needs more validation constraints as I stated above
Testing
Has this pull request been tested?
Please describe shortly how you tested it:
Note
The title of your PR should follow this format:
[Type](area): Title