-
Notifications
You must be signed in to change notification settings - Fork 0
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
Gear/joh 28 pet and like endpoints #14
base: dev
Are you sure you want to change the base?
Conversation
JOH-28 Pet and Like endpoints
FindAll - GET /pet FindOne - GET /pet/:id Create- POST /pet/create FindByUserId - GET /like |
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.
LGTM. iirc pet methods now return
Image structs instead of image urls. Also, please try updating protofiles
IsSterile *bool `json:"is_sterile"` | ||
IsVaccinated *bool `json:"is_vaccine"` | ||
IsVisible *bool `json:"is_visible"` | ||
IsClubPet *bool `json:"is_club_pet"` |
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 think we should discuss about the best method we can avoid "update non-zero 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.
from gorm documentation I see 3 solutions they suggested.
- update only one column
- use
map[string]interface{}
instead of struct - do select before update
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.
From my experience, I preferred the third option because it's very easy to implement it along fieldmask
// Update Pet
message UpdatePetRequest{
Pet Pet = 1;
google.protobuf.FieldMask field_mask = 2;
}
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.
feel free to state your opinion here, you won't be judged.
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.
Thank you for sharing your insights and great options, i'll take a look
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.
Have you fixed the boolean problem, or is it going to be a later incremental change?
true
tofalse
bool
to*bool
and create functionBoolAddr
that return address of boolean valuecodes.InvalidArgument
forDtoToRaw
andRawToDto
error