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

[MM-564]: Fixed the subscribe permissions config preventing system admins from creating subscriptions in DM or GM #82

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Kshitij-Katiyar
Copy link

Summary

Fixed the subscribe permissions config preventing system admins from creating subscriptions in DM or GM

How to test

  • Set the Mattermost Roles Allowed to Edit Jira Subscriptions config of the Jira plugin to Users who can manage channel settings or Users who can manage teams
  • Try to run /Jira subscribe command in a DM or GM

Current Behaviour

  • Error regarding permissions is received

Updated Behaviour

  • The plugin works as intended and the subscribe modal is visible

…mins from creating subscriptions in DM or GM
server/subscribe.go Outdated Show resolved Hide resolved
@@ -702,7 +702,9 @@ func (p *Plugin) hasPermissionToManageSubscription(instanceID types.ID, userID,
return errors.New("is not channel admin")
}
default:
return errors.New("can only subscribe in public and private channels")

Choose a reason for hiding this comment

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

Looking at this makes me feel that subs were not allowed in DM/GM and our changes overrides that

Copy link
Author

Choose a reason for hiding this comment

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

Yes, as requested in the issue, the current PR changes allow the system admin to create subscriptions in DM/GM. Let me know if we need to get the approval from Doug before doing these changes, I will revert the changes

Choose a reason for hiding this comment

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

@Kshitij-Katiyar Yes we should get a confirmation here, that the current implmentation does not allow subs in DM/GM, do we need to update that.

Also, it was mentioned in the issue that the error they are receiving is regarding permisssions and not "not allowed in DM/gM.....". Did we look into that?

Copy link
Author

Choose a reason for hiding this comment

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

Screenshot from 2024-09-17 10-05-07

This is the error mentioned in the issue, the error message is apt to the condition. Reverting the changes involving change with plugin functionality. Will add a confirmation from Doug once we create a PR on mm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants