[MM-343] Updated meeting flow for new users.

mattermost / mattermost-plugin-zoom

Zoom plugin for Mattermost :electric_plug:

Apache License 2.0

107 stars 68 forks source link

[MM-343] Updated meeting flow for new users. #344

Closed raghavaggarwal2308 closed 4 months ago

raghavaggarwal2308 commented 5 months ago

Summary

New users will get a post to select the default meeting ID when they try to start a meeting for the first time.

Screenshots

post updated_post

What to test?

New users are getting a post to select the default meeting ID when they try to start a meeting for the first time.

Steps to test:

Create a new user on MM.
Connect their zoom account.
Start a zoom meeting.

Ticket Link

Fixes #343 Fixes #341

codecov[bot] commented 5 months ago

Codecov Report

Attention: Patch coverage is 0% with 38 lines in your changes are missing coverage. Please review.

Project coverage is 18.04%. Comparing base (009e61e) to head (b66daca).

Files	Patch %	Lines
server/http.go	0.00%	38 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #344 +/- ## ========================================== - Coverage 18.47% 18.04% -0.44% ========================================== Files 9 9 Lines 1510 1546 +36 ========================================== Hits 279 279 - Misses 1180 1216 +36 Partials 51 51 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mickmister commented 5 months ago

@raghavaggarwal2308 Can you please post some more screenshots to show the other possible states that we show the user?

asaadmahmood commented 5 months ago

@raghavaggarwal2308 Do we automatically create the zoom meeting and take the user to the zoom meeting page after they select an option?

raghavaggarwal2308 commented 5 months ago

@raghavaggarwal2308 Do we automatically create the zoom meeting and take the user to the zoom meeting page after they select an option?

@asaadmahmood Added logic to automatically redirect the user to the meeting page and yes, we automatically create the zoom meeting.

raghavaggarwal2308 commented 5 months ago

@raghavaggarwal2308 Can you please post some more screenshots to show the other possible states that we show the user?

@mickmister For new users: new_users.webm

For existing users: existing_users.webm

mickmister commented 5 months ago

It seems off to me to have the zoom settings message shown during the connection process, as this makes it emphasized twice in this flow:

Thoughts on only showing this on meeting creation, and not account connection? @asaadmahmood

raghavaggarwal2308 commented 5 months ago

@mickmister Fixed the review comments mentioned by you

AayushChaudhary0001 commented 5 months ago

The PR above has been tested for the following scenario:-

Every time new user gets a post to select which meeting ID before starting a meeting.

This PR is working fine for the above condition, LGTM. Approved.

mickmister commented 5 months ago

I'm still thinking we should only show the zoom settings message on meeting creation, and not account connection. @asaadmahmood What do you think https://github.com/mattermost/mattermost-plugin-zoom/pull/344#issuecomment-1986531322?

ayusht2810 commented 5 months ago

@mickmister @asaadmahmood Gentle reminder for the updates on this PR.

asaadmahmood commented 5 months ago

@mickmister @ayusht2810 Yes, I think we should only show it on meeting creation.

ayusht2810 commented 5 months ago

@mickmister @asaadmahmood updated the statement. Please re-review

asaadmahmood commented 5 months ago

@ayusht2810 Can you show a video.

ayusht2810 commented 5 months ago

@asaadmahmood Demo video for the above fix screen-capture.webm

asaadmahmood commented 5 months ago

LGTM

raghavaggarwal2308 commented 4 months ago

@mickmister Can we merge this PR and move forward with release v1.7.1?

ayusht2810 commented 4 months ago

@hanzei Can we merge this PR as it is Dev and QA approved for the release 1.7.1?