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

Fix issue of settings command working for non-connected users #384

Merged
merged 1 commit into from
Jul 12, 2024

Conversation

ayusht2810
Copy link
Contributor

Summary

  • Fixed issue of settings command working for non-connected users

Ticket Link

Fixes #346

What to test?

  • Test the /zoom settings command should not work for non-connected users.

Checklist

  • Completed dev testing
  • make test Ran test cases and ensured they are passing
  • make check-style Ran style check and ensured both webapp and server pass the checks

@ayusht2810 ayusht2810 requested a review from mickmister as a code owner July 2, 2024 14:52
@ayusht2810 ayusht2810 self-assigned this Jul 2, 2024
@ayusht2810 ayusht2810 added the 2: Dev Review Requires review by a core committer label Jul 2, 2024
Comment on lines +264 to +271
if _, authErr := p.authenticateAndFetchZoomUser(user); authErr != nil {
// the user state will be needed later while connecting the user to Zoom via OAuth
if appErr := p.storeOAuthUserState(user.Id, args.ChannelId, false); appErr != nil {
p.API.LogWarn("failed to store user state")
}
return authErr.Message, authErr.Err
}

Copy link
Contributor

Choose a reason for hiding this comment

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

What's the message that's shown here?

Copy link
Contributor Author

@ayusht2810 ayusht2810 Jul 2, 2024

Choose a reason for hiding this comment

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

@mickmister User is shown a link to connect the account if it is not connected. It is the same flow as when the user tries to start meeting while not being connected

func (p *Plugin) runStartCommand(args *model.CommandArgs, user *model.User, topic string) (string, error) {

@ayusht2810 ayusht2810 added 3: QA Review Requires review by a QA tester and removed 2: Dev Review Requires review by a core committer labels Jul 2, 2024
Copy link
Contributor

@AayushChaudhary0001 AayushChaudhary0001 left a comment

Choose a reason for hiding this comment

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

The above PR has been tested for the following scenarios:

  • Checked the zoom settings command for non-connected users
  • Checked the zoom settings command for connected users

The PR was working fine for both the conditions, LGTM. Approved

@raghavaggarwal2308 raghavaggarwal2308 enabled auto-merge (squash) July 4, 2024 12:30
Copy link
Member

@wiggin77 wiggin77 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@ayusht2810 ayusht2810 merged commit 8056f5a into master Jul 12, 2024
9 checks passed
@ayusht2810 ayusht2810 deleted the fix_issue_346 branch July 12, 2024 07:59
@raghavaggarwal2308 raghavaggarwal2308 added this to the v1.9.0 milestone Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3: QA Review Requires review by a QA tester
Projects
None yet
Development

Successfully merging this pull request may close these issues.

/zoom settings command working for non-connected users
5 participants