Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GitHub Issue #170 - Update status when starting/ending a zoom meeting #171

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 29 additions & 4 deletions server/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ import (
)

const (
helpText = `* |/zoom start| - Start a zoom meeting`
oAuthHelpText = `* |/zoom disconnect| - Disconnect from zoom`
helpText = `* |/zoom start| - Start a zoom meeting`
oAuthHelpText = `* |/zoom disconnect| - Disconnect from zoom`
statusHelpText = `* |/zoom status yes/no| - Change automatic status change setting, yes/no`
)

func (p *Plugin) getCommand() (*model.Command, error) {
Expand Down Expand Up @@ -68,6 +69,8 @@ func (p *Plugin) executeCommand(c *plugin.Context, args *model.CommandArgs) (str
return p.runDisconnectCommand(userID)
case "help", "":
return p.runHelpCommand()
case "status":
return p.runStatusChange(userID, split[1:])
default:
return fmt.Sprintf("Unknown action %v", action), nil
}
Expand Down Expand Up @@ -126,20 +129,42 @@ func (p *Plugin) runDisconnectCommand(userID string) (string, error) {
// runHelpCommand runs command to display help text.
func (p *Plugin) runHelpCommand() (string, error) {
text := "###### Mattermost Zoom Plugin - Slash Command Help\n"
text += strings.ReplaceAll(helpText, "|", "`")
text += strings.Replace(helpText, "|", "`", -1)
text += "\n" + strings.Replace(statusHelpText, "|", "`", -1)

if p.configuration.EnableOAuth {
text += "\n" + strings.ReplaceAll(oAuthHelpText, "|", "`")
}
return text, nil
}

func (p *Plugin) runStatusChange(userID string, args []string) (string, error) {
errMsg := "Expecting yes/no"
if len(args) < 2 {
return errMsg, errors.New("unexpected argument")
}

action := args[1]
if action != yes && action != no {
return errMsg, errors.New("unexpected argument")
}

appErr := p.API.KVSet(fmt.Sprintf("%v_%v", changeStatusKey, userID), []byte(action))
if appErr != nil {
p.API.LogDebug("Could not save status change preference ", appErr)
return "Could not save status preference", appErr
}
return "Status preference updated successfully", nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this message convey the fact of "Your status will be updated" or "Your status will not be updated"?

}

// getAutocompleteData retrieves auto-complete data for the "/zoom" command
func (p *Plugin) getAutocompleteData() *model.AutocompleteData {
zoom := model.NewAutocompleteData("zoom", "[command]", "Available commands: start, disconnect, help")
zoom := model.NewAutocompleteData("zoom", "[command]", "Available commands: start, disconnect, status help")

start := model.NewAutocompleteData("start", "", "Starts a Zoom meeting")
status := model.NewAutocompleteData("status", "Takes yes/no", "Change automatic status change setting")
cvhariharan marked this conversation as resolved.
Show resolved Hide resolved
zoom.AddCommand(start)
zoom.AddCommand(status)

// no point in showing the 'disconnect' option if OAuth is not enabled
if p.configuration.EnableOAuth {
Expand Down
72 changes: 70 additions & 2 deletions server/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,12 @@ import (
"github.com/mattermost/mattermost-plugin-zoom/server/zoom"
)

const defaultMeetingTopic = "Zoom Meeting"
const (
defaultMeetingTopic = "Zoom Meeting"
postActionPath = "/action/status"
yes = "yes"
no = "no"
)

func (p *Plugin) ServeHTTP(c *plugin.Context, w http.ResponseWriter, r *http.Request) {
config := p.getConfiguration()
Expand All @@ -42,11 +47,52 @@ func (p *Plugin) ServeHTTP(c *plugin.Context, w http.ResponseWriter, r *http.Req
p.completeUserOAuthToZoom(w, r)
case "/deauthorization":
p.deauthorizeUser(w, r)
case postActionPath:
p.postActionConfirm(w, r)
default:
http.NotFound(w, r)
}
}

func (p *Plugin) postActionConfirm(w http.ResponseWriter, r *http.Request) {
userID := r.Header.Get("Mattermost-User-ID")
response := model.PostActionIntegrationResponse{}
request := model.PostActionIntegrationRequestFromJson(r.Body)
accepted := request.Context["accept"].(bool)
meetingID := request.Context["meetingId"].(float64)
cvhariharan marked this conversation as resolved.
Show resolved Hide resolved

post := &model.Post{}
key := fmt.Sprintf("%v_%v", changeStatusKey, userID)

message := "Ok, the status won't be updated automatically"
changeStatus := no
if accepted {
changeStatus = yes
message = "You have accepted automatic status change. Yay!"
}

appErr := p.API.KVSet(key, []byte(changeStatus))
if appErr != nil {
p.API.LogDebug("failed to set status change preference", "error", appErr.Error())
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should return here instead of proceeding after the if statement. Maybe have this function return an error, and call this function with a smaller function that handles the error. Any error that occurs in this function should cause the function to return early.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mickmister Here if we don't return early, the next call is to setUserStatus which would automatically send another slack attachment for user confirmation on automatic status change. Won't that be more convenient?

Also, if I understand this correctly, you are suggesting we remove the KVSet and setUserStatus snippets and put it in a separate function and return early if that function throws any error?


err := p.setUserStatus(userID, int(meetingID), false)
if appErr != nil {
p.API.LogDebug("failed to change user status", "error", err)
}
Comment on lines +90 to +93
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
err := p.setUserStatus(userID, int(meetingID), false)
if appErr != nil {
p.API.LogDebug("failed to change user status", "error", err)
}
err := p.setUserStatus(userID, int(meetingID), false)
if err != nil {
p.API.LogDebug("failed to change user status", "error", err)
}


sa := &model.SlackAttachment{
Title: "Status Change",
Text: message,
}
model.ParseSlackAttachment(post, []*model.SlackAttachment{sa})
response.Update = post
_, err = w.Write(response.ToJson())
if err != nil {
p.API.LogDebug("failed to write response")
}
}

func (p *Plugin) connectUserToZoom(w http.ResponseWriter, r *http.Request) {
userID := r.Header.Get("Mattermost-User-ID")
if userID == "" {
Expand Down Expand Up @@ -288,13 +334,18 @@ func (p *Plugin) handleMeetingEnded(w http.ResponseWriter, r *http.Request, webh
return
}

err := p.setUserStatus(post.UserId, int(meetingID), true)
if err != nil {
p.API.LogDebug("failed to change user status", "error", err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we return the error, and handle it on the caller's side? I think we will need to make a separate small function to do so.

}

appErr = p.API.KVDelete(key)
if appErr != nil {
p.API.LogWarn("failed to delete db entry", "error", appErr.Error())
return
}

_, err := w.Write([]byte(post.ToJson()))
_, err = w.Write([]byte(post.ToJson()))
if err != nil {
p.API.LogWarn("failed to write response", "error", err.Error())
}
Expand Down Expand Up @@ -341,6 +392,23 @@ func (p *Plugin) postMeeting(creator *model.User, meetingID int, channelID strin
return appErr
}

storedStatusPref, appErr := p.API.KVGet(fmt.Sprintf("%v_%v", changeStatusKey, creator.Id))
if appErr != nil {
p.API.LogDebug("Could not get stored status preference from KV ", appErr)
}

if storedStatusPref == nil {
err := p.sendStatusChangeAttachment(creator.Id, p.botUserID, meetingID)
if err != nil {
p.API.LogDebug("could not send status change attachment ", "error", err)
}
}
larkox marked this conversation as resolved.
Show resolved Hide resolved

err := p.setUserStatus(creator.Id, meetingID, false)
if err != nil {
p.API.LogDebug("failed to change user status", "error", err)
}

appErr = p.API.KVSetWithExpiry(fmt.Sprintf("%v%v", postMeetingKey, meetingID), []byte(createdPost.Id), meetingPostIDTTL)
if appErr != nil {
p.API.LogDebug("failed to store post id", "err", appErr)
Expand Down
114 changes: 113 additions & 1 deletion server/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ import (
)

const (
postMeetingKey = "post_meeting_"
postMeetingKey = "post_meeting_"
oldStatusKey = "old_status_"
changeStatusKey = "zoom_status_change"

botUserName = "zoom"
botDisplayName = "Zoom"
Expand All @@ -42,6 +44,7 @@ const (
meetingPostIDTTL = 60 * 60 * 24 // One day

zoomProviderName = "Zoom"
pluginURLPath = "/plugins/" + botUserName
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

)

type Plugin struct {
Expand Down Expand Up @@ -394,3 +397,112 @@ func (p *Plugin) dm(userID string, message string) error {
}
return nil
}

func (p *Plugin) setUserStatus(userID string, meetingID int, meetingEnd bool) error {
statusChangePrefkey := fmt.Sprintf("%v_%v", changeStatusKey, userID)
changeStatus, err := p.API.KVGet(statusChangePrefkey)
if err != nil {
p.API.LogDebug("Could not get old status from KVStore", "err", err.Error())
return err
}

if string(changeStatus) != yes {
return nil
}

statusKey := fmt.Sprintf("%v%v", oldStatusKey, meetingID)
if meetingEnd {
b, appErr := p.API.KVGet(statusKey)
cvhariharan marked this conversation as resolved.
Show resolved Hide resolved
if appErr != nil {
p.API.LogDebug("Could not get old status from KVStore", "err", appErr.Error())
return appErr
}

newStatus := string(b)
if newStatus == "" {
newStatus = model.STATUS_ONLINE
}

_, appErr = p.API.UpdateUserStatus(userID, newStatus)
if appErr != nil {
p.API.LogDebug("Could not get update status", "err", appErr.Error())
return appErr
}

return nil
}

currentStatus, appErr := p.API.GetUserStatus(userID)
if appErr != nil {
p.API.LogDebug("Failed to update user status", "err", appErr)
return appErr
}

oldStatus := ""
if currentStatus.Manual {
oldStatus = currentStatus.Status
}

appErr = p.API.KVSetWithExpiry(statusKey, []byte(oldStatus), meetingPostIDTTL)
if appErr != nil {
p.API.LogDebug("failed to store old status", "err", appErr)
return appErr
}

_, appErr = p.API.UpdateUserStatus(userID, model.STATUS_DND)
if appErr != nil {
p.API.LogDebug("Failed to update user status", "err", appErr)
return appErr
Comment on lines +272 to +273
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use errors.Wrap to decorate errors with relevant error messages in this function and have the parent log, instead of logging here?

}

return nil
}

func (p *Plugin) sendStatusChangeAttachment(userID, botUserID string, meetingID int) error {
url := pluginURLPath + postActionPath
actionYes := &model.PostAction{
Name: "Yes",
Integration: &model.PostActionIntegration{
URL: url,
Context: map[string]interface{}{
"accept": true,
"meetingId": meetingID,
},
},
}

actionNo := &model.PostAction{
Name: "No",
Integration: &model.PostActionIntegration{
URL: url,
Context: map[string]interface{}{
"accept": false,
"meetingId": meetingID,
},
},
}

sa := &model.SlackAttachment{
Title: "Status change",
Text: "Allow Zoom plugin to automatically change status",
Actions: []*model.PostAction{actionYes, actionNo},
}

attachmentPost := model.Post{}
model.ParseSlackAttachment(&attachmentPost, []*model.SlackAttachment{sa})
directChannel, appErr := p.API.GetDirectChannel(userID, botUserID)
larkox marked this conversation as resolved.
Show resolved Hide resolved
if appErr != nil {
p.API.LogDebug("Create Attachment: ", appErr)
return appErr
Comment on lines +313 to +314
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
p.API.LogDebug("Create Attachment: ", appErr)
return appErr
return errors.Wrap(appErr, "Failed to get bot DM channel")

}
attachmentPost.ChannelId = directChannel.Id
attachmentPost.UserId = botUserID

_, appErr = p.API.CreatePost(&attachmentPost)
if appErr != nil {
p.API.LogDebug("Create Attachment: ", appErr)
return appErr
Comment on lines +321 to +322
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
p.API.LogDebug("Create Attachment: ", appErr)
return appErr
return errors.Wrap(appErr, "Failed to create status change attachment post")

}

return nil
}
7 changes: 7 additions & 0 deletions server/plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,23 +88,30 @@ func TestPlugin(t *testing.T) {
}, nil)

api.On("GetChannelMember", "thechannelid", "theuserid").Return(&model.ChannelMember{}, nil)
api.On("GetUserStatus", "theuserid").Return(&model.Status{}, nil)

api.On("GetPost", "thepostid").Return(&model.Post{Props: map[string]interface{}{}}, nil)
api.On("CreatePost", mock.AnythingOfType("*model.Post")).Return(&model.Post{}, nil)
api.On("UpdatePost", mock.AnythingOfType("*model.Post")).Return(&model.Post{}, nil)
api.On("GetPostsSince", "thechannelid", mock.AnythingOfType("int64")).Return(&model.PostList{}, nil)

api.On("KVSetWithExpiry", "old_status_123", mock.AnythingOfType("[]uint8"), mock.AnythingOfType("int64")).Return(nil)
api.On("KVSetWithExpiry", fmt.Sprintf("%v%v", postMeetingKey, 234), mock.AnythingOfType("[]uint8"), mock.AnythingOfType("int64")).Return(nil)
api.On("KVSetWithExpiry", fmt.Sprintf("%v%v", postMeetingKey, 123), mock.AnythingOfType("[]uint8"), mock.AnythingOfType("int64")).Return(nil)

api.On("KVGet", fmt.Sprintf("%v%v", postMeetingKey, 234)).Return([]byte("thepostid"), nil)
api.On("KVGet", fmt.Sprintf("%v%v", postMeetingKey, 123)).Return([]byte("thepostid"), nil)
api.On("KVGet", "zoom_status_change_theuserid").Return([]byte("yes"), nil)
api.On("KVGet", "zoom_status_change_").Return([]byte("yes"), nil)
api.On("KVGet", "old_status_0").Return([]byte("online"), nil)

api.On("KVDelete", fmt.Sprintf("%v%v", postMeetingKey, 234)).Return(nil)

api.On("LogWarn", mock.AnythingOfType("string")).Return()
api.On("LogDebug", mock.AnythingOfType("string"), mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return()

api.On("UpdateUserStatus", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return(&model.Status{}, nil)

path, err := filepath.Abs("..")
require.Nil(t, err)
api.On("GetBundlePath").Return(path, nil)
Expand Down