Support users.profile.get API#298
Support users.profile.get API#298james-lawrence merged 3 commits intoslack-go:masterfrom suzuki-shunsuke:support-users-profile-get-api
Conversation
|
as always, thanks @suzuki-shunsuke |
|
@james-lawrence |
| StatusText string `json:"status_text,omitempty"` | ||
| StatusEmoji string `json:"status_emoji,omitempty"` | ||
| Team string `json:"team"` | ||
| Fields map[string]UserProfileCustomField `json:"fields"` |
There was a problem hiding this comment.
There's a small issue with this Fields field: Slack is inconsistent with the type of this field in the profile object.
If there are custom fields present, then fields will be a JSON object, and all is well. If there are NO custom fields present, then fields is an empty array (on one particularly old Slack team, fields was actually null?!). This causes an unmarshalling_error for an otherwise valid profile object.
There was a problem hiding this comment.
Thank you for your comment and I'm sorry for this issue.
I agree.
I send the Pull Request to revert the change.
https://github.com/nlopes/slack/pull/304
We have to think how to support custom fields.
There was a problem hiding this comment.
The cleanest way would probably(?) be to have a custom unmarshaller for this struct that calls through to json.Unmarshal for everything but Fields (we'll have to set the struct field tag to json:"-"), and then check the datatype of the fields in the JSON response before unmarshalling that as well. The downside of this approach is we would have to write a custom marshaller as well.
An alternative approach is to compose some UserProfileWithCustomFields struct with an anonymous UserProfile field, but that would get messy, since you would have to deal with two types for essentially the same JSON blob, depending on whether or not you know there are custom fields in your Slack workspace (and GetUserProfile functions would have to return values accordingly).
A third approach (though I'm unsure if this would actually work) would be to make the type of Fields interface{} so json.Unmarshal could handle it, and then have client library code deal with making sense of it.
As I actually don't have much experience developing Golang libraries that wrap JSON APIs, I'd like to get the opinion of a project maintainer (@james-lawrence ?) as well.
It would also be nice to inform Slack of this small bug in their API, I suppose :)
There was a problem hiding this comment.
This is one of solutions.
There was a problem hiding this comment.
@suzuki-shunsuke your example code looks good, minus the pointer for the field. you don't need it to be a pointer the json library handles it correctly.
There was a problem hiding this comment.
@james-lawrence
Thank you for your review.
I will send a pull request within tomorrow.
There was a problem hiding this comment.
I send a Pull Request.
3 participants
https://api.slack.com/methods/users.profile.get