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.

Sign up for GitHub

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

GH-109: Participant join before host webhook event #185

Closed
wants to merge 10 commits into from
Closed

GH-109: Participant join before host webhook event #185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
da011ea
add participant join before host webhook event
andrew-the-drawer Oct 31, 2020
a9e47cb
update from upstream
andrew-the-drawer Nov 14, 2020
adae1b6
when participant jbh, send ephemeral msg to host
andrew-the-drawer Nov 14, 2020
97cf15e
Merge branch 'master' into GH-109
andrew-the-drawer Nov 22, 2020
02aaea9
add participant join before host webhook event
andrew-the-drawer Oct 31, 2020
7d4510e
when participant jbh, send ephemeral msg to host
andrew-the-drawer Nov 14, 2020
da91f9a
Merge branch 'GH-109' of github.com-lantrungseo:lantrungseo/mattermos…
andrew-the-drawer Nov 22, 2020
c45f259
finish feature
andrew-the-drawer Nov 22, 2020
7b7889c
tidy go module
andrew-the-drawer Nov 22, 2020
274a9b0
Merge branch 'master' into GH-109
andrew-the-drawer Nov 25, 2020
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
80 changes: 72 additions & 8 deletions server/http.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,18 +219,79 @@ func (p *Plugin) handleWebhook(w http.ResponseWriter, r *http.Request) {
return
Copy link
Contributor

Choose a reason for hiding this comment

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

This PR doesn't touch this part, but do you mind adding this to line 214?

defer r.Body.Close()

}

if webhook.Event != zoom.EventTypeMeetingEnded {
w.WriteHeader(http.StatusNotImplemented)
if webhook.Event == zoom.EventTypeMeetingParticipantJoined {
var meetingWebhook zoom.MeetingParticipantsJoinedWebhook
if err = json.Unmarshal(b, &meetingWebhook); err != nil {
p.API.LogError("Error unmarshaling meeting webhook", "err", err.Error())
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
p.handleParticipantJoinedWebhook(w, r, &meetingWebhook)
return
}
if webhook.Event == zoom.EventTypeMeetingEnded {
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 add an extra line between these if blocks?

var meetingWebhook zoom.MeetingWebhook
if err = json.Unmarshal(b, &meetingWebhook); err != nil {
p.API.LogError("Error unmarshaling meeting webhook", "err", err.Error())
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
p.handleMeetingEnded(w, r, &meetingWebhook)
return
}
http.Error(w, "Not implemented", http.StatusNotImplemented)
}

var meetingWebhook zoom.MeetingWebhook
if err = json.Unmarshal(b, &meetingWebhook); err != nil {
p.API.LogError("Error unmarshaling meeting webhook", "err", err.Error())
http.Error(w, err.Error(), http.StatusBadRequest)
// handle webhook participant join before host in the meeting
func (p *Plugin) handleParticipantJoinedWebhook(
w http.ResponseWriter, r *http.Request, webhook *zoom.MeetingParticipantsJoinedWebhook,
) {
meetingID := webhook.Payload.Object.ID
postID, appErr := p.fetchMeetingPostID(meetingID)
if appErr != nil {
http.Error(w, appErr.Error(), appErr.StatusCode)
return
}
p.handleMeetingEnded(w, r, &meetingWebhook)
post, appErr := p.API.GetPost(postID)
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 add an extra line after each if block in this function?

if appErr != nil {
p.API.LogWarn("Could not get meeting post by id", "err", appErr)
http.Error(w, appErr.Error(), appErr.StatusCode)
return
}
meetingCreator := post.UserId
participant := webhook.Payload.Object.Participant
isHostJoined, isBool := post.Props["meeting_host_joined"].(bool)
if !isBool {
isHostJoined = false
}
waitingCnt, isNum := post.Props["meeting_waiting_count"].(float64)
if !isNum {
waitingCnt = 0
}
// if host has joined, then no need to proceed further
if isHostJoined {
return
}
// check whether participant is host
if participant.RegistrantID == webhook.Payload.Object.HostID {
isHostJoined = true
} else {
waitingCnt++
p.API.SendEphemeralPost(meetingCreator, &model.Post{
UserId: p.botUserID,
ChannelId: post.ChannelId,
Message: fmt.Sprintf(
"User %s has joined the meeting before you", participant.UserName,
),
})
}
post.Props["meeting_host_joined"] = isHostJoined
post.Props["meeting_waiting_count"] = waitingCnt
_, updatePostErr := p.API.UpdatePost(post)
_, err := w.Write([]byte(post.ToJson()))
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if there is any need to write the post to the zoom. Maybe just an OK is enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I could fix that =))

if err != nil || updatePostErr != nil {
p.API.LogWarn("failed to write response", "error", err.Error())
Copy link
Contributor

Choose a reason for hiding this comment

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

If err == nil and updatePostErr != nil, this will create a panic. Use two separate checks. That way you can introduce better errors for each case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay.

}
}

func (p *Plugin) handleMeetingEnded(w http.ResponseWriter, r *http.Request, webhook *zoom.MeetingWebhook) {
Expand Down Expand Up @@ -321,12 +382,15 @@ func (p *Plugin) postMeeting(creator *model.User, meetingID int, channelID strin
"meeting_topic": topic,
"meeting_creator_username": creator.Username,
"meeting_provider": zoomProviderName,
"meeting_waiting_count": 0,
"meeting_host_joined": false,
},
}

createdPost, appErr := p.API.CreatePost(post)
if appErr != nil {
return appErr
p.API.LogError(appErr.Error())
return errors.New(appErr.Error())
}

if appErr = p.storeMeetingPostID(meetingID, createdPost.Id); appErr != nil {
Expand Down
20 changes: 18 additions & 2 deletions server/zoom/webhook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ const (
RecordingWebhookTypeComplete = "RECORDING_MEETING_COMPLETED"
RecentlyCreated = "RECENTLY_CREATED"

EventTypeMeetingStarted EventType = "meeting.started"
EventTypeMeetingEnded EventType = "meeting.ended"
EventTypeMeetingStarted EventType = "meeting.started"
EventTypeMeetingEnded EventType = "meeting.ended"
EventTypeMeetingParticipantJoined EventType = "meeting.participant_joined"
)

type MeetingWebhookObject struct {
Expand All @@ -36,6 +37,21 @@ type MeetingWebhookPayload struct {
Object MeetingWebhookObject `json:"object"`
}

type MeetingParticipantsJoinedWebhook struct {
Event EventType `json:"event"`
Payload struct {
AccountID string `json:"account_id"`
Object struct {
MeetingWebhookObject
Participant struct {
ParticipantID string `json:"user_id"`
UserName string `json:"user_name"`
RegistrantID string `json:"id"`
}
} `json:"object"`
} `json:"payload"`
}

type MeetingWebhook struct {
Event EventType `json:"event"`
Payload MeetingWebhookPayload `json:"payload"`
Expand Down
42 changes: 29 additions & 13 deletions webapp/src/components/post_type_zoom/post_type_zoom.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,20 +83,36 @@ export default class PostTypeZoom extends React.PureComponent {
if (this.props.fromBot) {
preText = `${this.props.creatorName} has started a meeting`;
}
let announcementText = '';
if (props.meeting_host_joined) {
announcementText = 'Host already in the meeting';
} else {
Comment on lines +86 to +89
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about making this less lines like this?

Suggested change
let announcementText = '';
if (props.meeting_host_joined) {
announcementText = 'Host already in the meeting';
} else {
let announcementText = 'Host already in the meeting';
if (!props.meeting_host_joined) {

if (props.meeting_waiting_count === 1) {
announcementText = '1 participant waiting for host';
}
if (props.meeting_waiting_count > 1) {
Comment on lines +92 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.

Can we do this for better readability?

Suggested change
}
if (props.meeting_waiting_count > 1) {
} else if (props.meeting_waiting_count > 1) {

announcementText = `${props.meeting_waiting_count} participants waiting for host`;
}
}
content = (
<a
className='btn btn-lg btn-primary'
style={style.button}
rel='noopener noreferrer'
target='_blank'
href={props.meeting_link}
>
<i
style={style.buttonIcon}
dangerouslySetInnerHTML={{__html: Svgs.VIDEO_CAMERA_3}}
/>
{'JOIN MEETING'}
</a>
<>
<div>
<a
className='btn btn-lg btn-primary'
style={style.button}
rel='noopener noreferrer'
target='_blank'
href={props.meeting_link}
>
<i
style={style.buttonIcon}
dangerouslySetInnerHTML={{__html: Svgs.VIDEO_CAMERA_3}}
/>
{'JOIN MEETING'}
</a>
</div>
<p>{announcementText}</p>
</>
);

if (props.meeting_personal) {
Expand Down