Skip to content

Commit 90e2d1d

Browse files
Akshat Khosyaclaude
andcommitted
Address PR feedback: Make status sync opt-in and fix issues
- 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 8394af6 commit 90e2d1d

File tree

4 files changed

+30
-6
lines changed

4 files changed

+30
-6
lines changed

plugin.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,13 @@
134134
"type": "bool",
135135
"help_text": "When set to 'true' you will get a notification with less details when a draft pull request is created and a notification with complete details when they are marked as ready for review. When set to 'false' no notifications are delivered for draft pull requests.",
136136
"default": false
137+
},
138+
{
139+
"key": "EnableStatusSync",
140+
"display_name": "Enable Status Synchronization:",
141+
"type": "bool",
142+
"help_text": "(Optional) When enabled, users can opt-in to automatically sync their Mattermost 'Out of Office' status to GitHub. When a user sets their status to OOO in Mattermost, their GitHub status will be updated to 'Out of office' with busy indicator. The status is restored when they return. The feature respects manually set GitHub statuses and will not overwrite them.",
143+
"default": false
137144
}
138145
],
139146
"footer": "* To report an issue, make a suggestion or a contribution, [check the repository](https://github.com/mattermost/mattermost-plugin-github)."

server/plugin/configuration.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ type Configuration struct {
4343
UsePreregisteredApplication bool `json:"usepreregisteredapplication"`
4444
ShowAuthorInCommitNotification bool `json:"showauthorincommitnotification"`
4545
GetNotificationForDraftPRs bool `json:"getnotificationfordraftprs"`
46+
EnableStatusSync bool `json:"enablestatussync"`
4647
}
4748

4849
func (c *Configuration) ToMap() (map[string]interface{}, error) {

server/plugin/graphql/client.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,14 +77,14 @@ type changeUserStatusMutation struct {
7777
func (c *Client) UpdateUserStatus(ctx context.Context, emoji, message string, busy bool) (string, error) {
7878
var mutation changeUserStatusMutation
7979
input := githubv4.ChangeUserStatusInput{
80-
Emoji: githubv4.NewString(githubv4.String(emoji)),
81-
Message: githubv4.NewString(githubv4.String(message)),
80+
Emoji: githubv4.NewString(githubv4.String(emoji)),
81+
Message: githubv4.NewString(githubv4.String(message)),
8282
LimitedAvailability: githubv4.NewBoolean(githubv4.Boolean(busy)),
8383
}
8484

8585
err := c.client.Mutate(ctx, &mutation, input, nil)
8686
if err != nil {
87-
return "", err
87+
return "", errors.Wrap(err, "UpdateUserStatus mutate failed")
8888
}
8989

9090
return string(mutation.ChangeUserStatus.Status.Message), nil
@@ -108,7 +108,7 @@ func (c *Client) GetUserStatus(ctx context.Context, login string) (string, strin
108108

109109
err := c.client.Query(ctx, &query, variables)
110110
if err != nil {
111-
return "", "", false, err
111+
return "", "", false, errors.Wrap(err, "GetUserStatus query failed")
112112
}
113113

114114
return string(query.User.Status.Message), string(query.User.Status.Emoji), bool(query.User.Status.LimitedAvailability), nil

server/plugin/plugin.go

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -717,6 +717,10 @@ func (p *Plugin) disconnectGitHubAccount(userID string) {
717717
p.client.Log.Warn("Failed to delete github token from KV store", "userID", userID, "error", err.Error())
718718
}
719719

720+
if err := p.store.Delete(userID + "_github_status"); err != nil {
721+
p.client.Log.Warn("Failed to delete github status from KV store", "userID", userID, "error", err.Error())
722+
}
723+
720724
user, err := p.client.User.Get(userID)
721725
if err != nil {
722726
p.client.Log.Warn("Failed to get user props", "userID", userID, "error", err.Error())
@@ -1200,6 +1204,12 @@ func (p *Plugin) handleRevokedToken(info *GitHubUserInfo) {
12001204
}
12011205

12021206
func (p *Plugin) UserStatusHasChanged(c *plugin.Context, userStatus *model.Status) {
1207+
// Check if status sync is enabled in configuration
1208+
config := p.getConfiguration()
1209+
if !config.EnableStatusSync {
1210+
return
1211+
}
1212+
12031213
userInfo, apiErr := p.getGitHubUserInfo(userStatus.UserId)
12041214
if apiErr != nil {
12051215
return
@@ -1213,6 +1223,12 @@ func (p *Plugin) UserStatusHasChanged(c *plugin.Context, userStatus *model.Statu
12131223
return
12141224
}
12151225

1226+
// Don't overwrite if GitHub status is already set to "out of office" (busy)
1227+
if busy {
1228+
p.client.Log.Debug("GitHub status is already set to busy/OOO, skipping update")
1229+
return
1230+
}
1231+
12161232
githubStatus := &GithubStatus{
12171233
Message: message,
12181234
Emoji: emoji,
@@ -1233,7 +1249,7 @@ func (p *Plugin) UserStatusHasChanged(c *plugin.Context, userStatus *model.Statu
12331249
return
12341250
}
12351251

1236-
p.CreateBotDMPost(userInfo.UserID, "Your GitHub status has been updated to Out of office.", "custom_git_ooo_ephemeral")
1252+
p.CreateBotDMPost(userInfo.UserID, "Your GitHub status has been updated to Out of office.", "custom_git_status_sync")
12371253
} else {
12381254
var oldStatus []byte
12391255
if err := p.store.Get(userInfo.UserID+"_github_status", &oldStatus); err == nil && len(oldStatus) > 0 {
@@ -1250,7 +1266,7 @@ func (p *Plugin) UserStatusHasChanged(c *plugin.Context, userStatus *model.Statu
12501266
return
12511267
}
12521268
p.store.Delete(userInfo.UserID + "_github_status")
1253-
p.CreateBotDMPost(userInfo.UserID, "Your GitHub status has been restored.", "custom_git_ooo_ephemeral")
1269+
p.CreateBotDMPost(userInfo.UserID, "Your GitHub status has been restored.", "custom_git_status_sync")
12541270
}
12551271
}
12561272
}

0 commit comments

Comments
 (0)