Folders API (for caseworker UI project) #115
Conversation
- SQL queries - types - folders manager - routing
katerina-kossler
requested review from
lgarofalo,
richardxia,
jjfreund,
schroerbrian and
waynekoepcke
as code owners
| func (m *Manager) Get(w http.ResponseWriter, r *http.Request) { | ||
| userId, err := strconv.Atoi(chi.URLParam(r, "user_id")) | ||
| if err != nil { | ||
| fmt.Println("error:", err) |
There was a problem hiding this comment.
I know that this is just a placeholder for now, but what's our general strategy for returning useful error messages to the API client? I guess we might want two separate error messages for the following scenarios related to part of the function:
user_idshould probably be mandatory, at least for now, so if it's missing, we should let the client know it's mandatory- if it's an invalid number, then we should also report that
Thinking a little bit ahead, with the user auth stuff that I worked on, we'll also want to report an error message if there's an authentication failure (JWT failed to validate) or an authorization failure (the logged in user doesn't match the user_id query parameter here).
There was a problem hiding this comment.
Let me see if I can add an example of such on one of the existing endpoints
|
having issues testing my endpoints locally... maybe due to auth or no folders being in the db |
There was a problem hiding this comment.
Overall, this looks good, thanks for working on this, @katerina-kossler! In the interest of time, I'm going to go make the changes for the things I suggested on the PR and push back to this branch, just so that we can get things merged and ready for frontend integration in time.
I'm going to leave off the auth part for now, so that's something that can happen in a separate ticket.
| err = m.DbClient.UpdateFolder(dBFolder) | ||
| if err != nil { | ||
| log.Print(err) | ||
| writeStatus(w, http.StatusInternalServerError) |
There was a problem hiding this comment.
We probably need some more specific error handling here, since one of the possible failure modes is trying to PUT to an ID that doesn't exist, and this line will cause it to return a 500 error, even though the issue is on the caller's side for attempting to modify a non-existent object.
Ideally, we'd want to distinguish between server-side errors (5xx errors) and client-side errors (4xx errors), but I'm not sure what the best way of doing that in Go is.
It looks like newerish Go has an errors.Is function for checking an error of a known type, but I'm not exactly sure how to use it, so I may punt on this.
- Changed the get by users route from `/api/folders/user/{user_id}` to
`/api/folders` with a `user_id` query parameter
- Fixed `createFolder` query to have correct number of parameters
- Fixed SQL syntax errors due to `order` being a keyword by ensuring
that every occurrence is quoted
- Do error handling for `GetByID` on nonexistent ID
- Change `Post` to also return the newly created folder
- Various error handling improvements
- go fmt
|
I'm going to merge this in now, but if @lgarofalo has any post-merge comments, definitely let us know, since the rest of us are all still learning Go. |
note - draft PR
includes:
completed:
notes: