#866 Set user as Busy in GitHub when status in Mattermost is OOO #940
Conversation
Adds a feature to automatically sync a user's Mattermost "Out of Office" status to their GitHub status. When a user sets their status to "Out of Office" in Mattermost, their GitHub status will be set to "Busy" with the message "Out of office". When the user's status is no longer "Out of Office", their original GitHub status will be restored. This is implemented using the `UserStatusHasChanged` hook in the Mattermost plugin API and the `changeUserStatus` GraphQL mutation in the GitHub API. The user's original GitHub status is stored in the plugin's key-value store.
|
Hello @akshat-khosya, Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here. Per the Mattermost Contribution Guide, we need to add you to the list of approved contributors for the Mattermost project. Please help complete the Mattermost contribution license agreement? This is a standard procedure for many open source projects. Please let us know if you have any questions. We are very happy to have you join our growing community! If you're not yet a member, please consider joining our Contributors community channel to meet other contributors and discuss new opportunities with the core team. |
|
/check-cla |
|
As a user I would absolutely NOT want this behavior. If this gets merged I hope it will be configurable and off by default. |
| p.client.Log.Error("failed to update user status", "error", err) | ||
| return | ||
| } | ||
| p.store.Delete(userInfo.UserID + "_github_status") |
There was a problem hiding this comment.
@akshat-khosya this is cool feature. One minor issue I see is that a user's status will be left in the KV store if they disconnect their account while in "ooo" status. Can you add something to clear a user's status in the KV store when their account is disconnected?
|
|
||
| if userStatus.Status == "ooo" { | ||
| graphQLClient := p.graphQLConnect(userInfo) | ||
| message, emoji, busy, err := graphQLClient.GetUserStatus(context.Background(), userInfo.GitHubUsername) |
There was a problem hiding this comment.
Can you check here if the Github status is already "out of office" and not overwrite it if true. I don't think we want to clobber a manually set OoO on the Github side.
server/plugin/plugin.go
Outdated
| return | ||
| } | ||
|
|
||
| p.CreateBotDMPost(userInfo.UserID, "Your GitHub status has been updated to Out of office.", "custom_git_ooo_ephemeral") |
There was a problem hiding this comment.
This post type is not actually ephemeral.
wiggin77
left a comment
wiggin77
left a comment
There was a problem hiding this comment.
Thanks @akshat-khosya! This is a nice feature. A few things to fix but overall looks great. Also you'll need to run "gofmt" on the files.
|
@akshat-khosya one other consideration. Can this be opted in for users? Not everyone will want their statuses sync'd. |
Thanks for the feedback @ThiefMaster. I agree, this will be opt-in. |
server/plugin/graphql/client.go
Outdated
| func (c *Client) UpdateUserStatus(ctx context.Context, emoji, message string, busy bool) (string, error) { | ||
| var mutation changeUserStatusMutation | ||
| input := githubv4.ChangeUserStatusInput{ | ||
| Emoji: githubv4.NewString(githubv4.String(emoji)), |
There was a problem hiding this comment.
The CI failure happened here earlier because the file wasn’t gofmt formatted. Please make sure this whole struct literal is formatted (run gofmt -w).
server/plugin/graphql/client.go
Outdated
|
|
||
| err := c.client.Mutate(ctx, &mutation, input, nil) | ||
| if err != nil { | ||
| return "", err |
There was a problem hiding this comment.
Consider wrapping for more traceability:
// return "", fmt.Errorf("UpdateUserStatus mutate failed: %w", err)
- Add EnableStatusSync config option (opt-in, default: false) - Check if GitHub status is already busy before overwriting - Clear stored GitHub status from KV store on account disconnect - Fix misleading post type name (remove "ephemeral") - Add error wrapping for better traceability in GraphQL client - Format code with gofmt Addresses feedback from reviewers: - System-wide opt-in configuration - Respects manually set GitHub OOO statuses - Proper cleanup on disconnect - Improved error messages
akshat-khosya
force-pushed
the
issues-866
branch
from
90e2d1d to
921e60d
Compare
|
/check-cla |
5 participants
Summary
Adds a feature to automatically sync a user's Mattermost "Out of Office" status to their GitHub status.
When a user sets their status to "Out of Office" in Mattermost, their GitHub status will be set to "Busy" with the message "Out of office". When the user's status is no longer "Out of Office", their original GitHub status will be restored.
This is implemented using the
UserStatusHasChangedhook in the Mattermost plugin API and thechangeUserStatusGraphQL mutation in the GitHub API. The user's original GitHub status is stored in the plugin's key-value store.Ticket Link
#866