MM-15205: Group Rest API #18
Conversation
This introduces the /api/group endpoints, as well as new /api/installation endpoints for joining and leaving a group. The API supports changing the target version of the installation group, but defers the propagation of same to the corresponding installations to https://mattermost.atlassian.net/browse/MM-15369.
There was a problem hiding this comment.
This looks great!
Based on my understanding of the code, it looks like we retain the group ID setting on an installation when deleting a group if said installation was part of the group. I think this makes sense, but just wanted to make sure. We didn't want to remove the group ID value from the installation at that time as well, did we?
| return nil | ||
|
|
||
| default: | ||
| return errors.Errorf("failed with status code %d", resp.StatusCode) |
There was a problem hiding this comment.
super nit: this error message is used heavily throughout the code. Perhaps we may want a helper function to return it in a standard way to make updating it in the future a little easier. 0/5 on this.
There was a problem hiding this comment.
I agree that this needs some attention. I'd like to also expose the actual error message to the client vs. requiring the server logs to explain. I'll add this as a TODO to revisit holistically.
|
@gabrieljackson, your understanding of the |
lieut-data
added
4: Reviews Complete
This introduces the
/api/groupendpoints, as well as new/api/installationendpoints for joining and leaving a group.The API supports changing the target version of the installation group, but defers the propagation of same to the corresponding installations to https://mattermost.atlassian.net/browse/MM-15369.
I also fixed a minor caching issue in
internal/tools/k8swith the use ofTempDirthat precludesgo test ./...from successfully caching these tests.Fixes: https://mattermost.atlassian.net/browse/MM-15205