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

andrew-the-drawer · 2020-10-31T14:05:31Z

Summary

This PR supports sending updates and notifications to host if a participant joins before host in a Zoom meeting

To recognize whether host is already in the meeting, this PR utilize Participant joined webhook

Ticket Link

Fixes #109

mattermod · 2020-10-31T14:05:35Z

Hello @lantrungseo,

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.

mattermod · 2020-11-13T01:01:29Z

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

/cc @jasonblais @jfrerich @emilyacook

larkox · 2020-11-13T08:32:11Z

@lantrungseo Is this ready to review, or are you still working on this? I see some lint errors in the build pipeline. Could you take a look at those?

andrew-the-drawer · 2020-11-13T15:59:27Z

@larkox this is still in progress. It's gonna be finished soon.

…t-plugin-zoom into GH-109

codecov · 2020-11-22T15:29:32Z

Codecov Report

Merging #185 (274a9b0) into master (45747e2) will decrease coverage by 1.05%.
The diff coverage is 10.52%.

@@            Coverage Diff             @@
##           master     #185      +/-   ##
==========================================
- Coverage   20.85%   19.79%   -1.06%     
==========================================
  Files           7        7              
  Lines         729      778      +49     
==========================================
+ Hits          152      154       +2     
- Misses        536      582      +46     
- Partials       41       42       +1

Impacted Files	Coverage Δ
server/http.go	`23.25% <10.52%> (-2.68%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 45747e2...274a9b0. Read the comment docs.

andrew-the-drawer · 2020-11-23T00:54:13Z

Ready for review!

larkox

In general, looks pretty good to me, just two small details. Could you add screenshots of the final result?

larkox · 2020-11-23T14:29:53Z

server/http.go

+	post.Props["meeting_host_joined"] = isHostJoined
+	post.Props["meeting_waiting_count"] = waitingCnt
+	_, updatePostErr := p.API.UpdatePost(post)
+	_, err := w.Write([]byte(post.ToJson()))


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

Yeah, I could fix that =))

larkox · 2020-11-23T14:31:06Z

server/http.go

+	_, updatePostErr := p.API.UpdatePost(post)
+	_, err := w.Write([]byte(post.ToJson()))
+	if err != nil || updatePostErr != nil {
+		p.API.LogWarn("failed to write response", "error", err.Error())


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.

andrew-the-drawer · 2020-11-23T15:58:32Z

In general, looks pretty good to me, just two small details. Could you add screenshots of the final result?

Yup, I will post the screen record of the whole flow here.

mickmister

Thanks for implementing this useful feature @lantrungseo! Functionally LGTM 👍 I have just a few requests regarding style.

mickmister · 2020-11-23T20:48:19Z

server/http.go

@@ -219,18 +219,79 @@ func (p *Plugin) handleWebhook(w http.ResponseWriter, r *http.Request) {
 		return


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

defer r.Body.Close()

mickmister · 2020-11-23T20:50:36Z

server/http.go

+		p.handleParticipantJoinedWebhook(w, r, &meetingWebhook)
+		return
+	}
+	if webhook.Event == zoom.EventTypeMeetingEnded {


Can we add an extra line between these if blocks?

mickmister · 2020-11-23T20:51:17Z

server/http.go

 		return
 	}
-	p.handleMeetingEnded(w, r, &meetingWebhook)
+	post, appErr := p.API.GetPost(postID)


Can we add an extra line after each if block in this function?

mickmister · 2020-11-23T20:59:56Z

webapp/src/components/post_type_zoom/post_type_zoom.jsx

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


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) {

mickmister · 2020-11-23T21:00:03Z

webapp/src/components/post_type_zoom/post_type_zoom.jsx

+                }
+                if (props.meeting_waiting_count > 1) {


Can we do this for better readability?

Suggested change

}

if (props.meeting_waiting_count > 1) {

} else if (props.meeting_waiting_count > 1) {

mattermod · 2020-12-06T01:01:25Z

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

/cc @jasonblais @jfrerich @emilyacook

jasonblais · 2020-12-09T19:25:36Z

@lantrungseo thank you for your contribution! Let us know if you have any questions about the feedback above, we'd be happy to help :)

larkox · 2021-01-13T09:41:32Z

@lantrungseo are you able to continue working on this issue?

lindy65 · 2021-05-27T08:56:40Z

Hi @lantrungseo - I'm helping to review stale PRs and, since we haven't heard from you in 6 months, I'll close this PR for now.
Please feel free to re-open if you're able to prioritize. Thanks!

I've re-added the "Up for Grabs" label on #109

//cc @larkox @mickmister

add participant join before host webhook event

da011ea

andrew-the-drawer requested a review from larkox as a code owner October 31, 2020 14:05

mattermod added Hacktoberfest hacktoberfest-accepted labels Oct 31, 2020

andrew-the-drawer marked this pull request as draft October 31, 2020 14:08

hanzei added the Work In Progress Not yet ready for review label Nov 2, 2020

mattermod added the Lifecycle/1:stale label Nov 13, 2020

andrew-the-drawer added 8 commits November 14, 2020 08:49

update from upstream

a9e47cb

when participant jbh, send ephemeral msg to host

adae1b6

Merge branch 'master' into GH-109

97cf15e

add participant join before host webhook event

02aaea9

when participant jbh, send ephemeral msg to host

7d4510e

Merge branch 'GH-109' of github.com-lantrungseo:lantrungseo/mattermos…

da91f9a

…t-plugin-zoom into GH-109

finish feature

c45f259

tidy go module

7b7889c

andrew-the-drawer marked this pull request as ready for review November 22, 2020 15:27

jasonblais added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester and removed Lifecycle/1:stale Work In Progress Not yet ready for review labels Nov 23, 2020

jasonblais requested a review from mickmister November 23, 2020 12:30

larkox requested changes Nov 23, 2020

View reviewed changes

mickmister suggested changes Nov 23, 2020

View reviewed changes

Merge branch 'master' into GH-109

274a9b0

mattermod added the Lifecycle/1:stale label Dec 6, 2020

lindy65 closed this May 27, 2021

hanzei added Lifecycle/3:orphaned and removed 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester Lifecycle/1:stale labels May 28, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

andrew-the-drawer commented Oct 31, 2020 •

edited

Loading

mattermod commented Oct 31, 2020

mattermod commented Nov 13, 2020

larkox commented Nov 13, 2020

andrew-the-drawer commented Nov 13, 2020

codecov bot commented Nov 22, 2020 •

edited

Loading

andrew-the-drawer commented Nov 23, 2020

larkox left a comment

larkox Nov 23, 2020

andrew-the-drawer Nov 23, 2020

larkox Nov 23, 2020

andrew-the-drawer Nov 23, 2020

andrew-the-drawer commented Nov 23, 2020

mickmister left a comment

mickmister Nov 23, 2020

mickmister Nov 23, 2020

mickmister Nov 23, 2020

mickmister Nov 23, 2020

mickmister Nov 23, 2020

mattermod commented Dec 6, 2020

jasonblais commented Dec 9, 2020

larkox commented Jan 13, 2021

lindy65 commented May 27, 2021

		@@ -219,18 +219,79 @@ func (p Plugin) handleWebhook(w http.ResponseWriter, r http.Request) {
		return

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

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

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

Conversation

andrew-the-drawer commented Oct 31, 2020 • edited Loading

Summary

Ticket Link

mattermod commented Oct 31, 2020

mattermod commented Nov 13, 2020

larkox commented Nov 13, 2020

andrew-the-drawer commented Nov 13, 2020

codecov bot commented Nov 22, 2020 • edited Loading

Codecov Report

andrew-the-drawer commented Nov 23, 2020

larkox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrew-the-drawer commented Nov 23, 2020

mickmister left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattermod commented Dec 6, 2020

jasonblais commented Dec 9, 2020

larkox commented Jan 13, 2021

lindy65 commented May 27, 2021

andrew-the-drawer commented Oct 31, 2020 •

edited

Loading

codecov bot commented Nov 22, 2020 •

edited

Loading