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-54933] Add the ability to @ mention custom groups in group constrained teams and channels #24987

Merged
merged 10 commits into from
Nov 16, 2023

Conversation

BenCookie95
Copy link
Contributor

Summary

There was a bug where you could not @ mention custom groups in group constrained teams, this fixes that issue. I think the ldap and custom group codes needs a complete review, there is a lot going on.

Ticket Link

https://mattermost.atlassian.net/browse/MM-54933

Release Note

Fixed a bug where you couldn't @ mention custom groups in group constrained teams and channels

@mm-cloud-bot mm-cloud-bot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Oct 17, 2023
@BenCookie95
Copy link
Contributor Author

/update-branch

Copy link
Contributor

@hanzei hanzei left a comment

Choose a reason for hiding this comment

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

Nice work on the tests 👍

server/channels/app/notification.go Outdated Show resolved Hide resolved
@hanzei hanzei added 2: Dev Review Requires review by a developer 3: QA Review Requires review by a QA tester. May occur at the same time as Dev Review labels Oct 18, 2023
@mattermost-build
Copy link
Contributor

E2E tests not automatically triggered, because the PR is not in a mergeable state. Please update the branch with the base branch and resolve outstanding conflicts.

Copy link
Contributor

@hanzei hanzei left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Member

@JulienTant JulienTant left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @BenCookie95

@BenCookie95 BenCookie95 removed the 2: Dev Review Requires review by a developer label Oct 27, 2023
@hanzei hanzei added this to the v9.3.0 milestone Oct 31, 2023
@BenCookie95
Copy link
Contributor Author

/update-branch

@amyblais amyblais added the CherryPick/Approved Meant for the quality or patch release tracked in the milestone label Nov 13, 2023
@DHaussermann
Copy link

/update-branch

Copy link

@DHaussermann DHaussermann left a comment

Choose a reason for hiding this comment

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

Tested and passed

  • Custom groups are now shown even when LDPA sync is enforced for team membership as well as on channels when channel membership is enforced
  • When there are no group members present on the team for the custom group a system message gives UI feedback
  • When a member of the custom group is also in the LDPA group and Team sync is enforced, the mention works normally
  • Regression tested custom group CRUD operations and ensured mention options are removed when a group is archived
  • LDAP sync enforcement on/off works as expected to remove members when sync is run
  • Tested LDAP group sync for teams as well as channels
  • Enable Group Mentions for a specific LDAP groups (feature labeled as Beta in the UI) and it works as expected

LGTM!
Note: This issue was a bit specific to find. I'll open a PR shortly to add a Zephyr test case for this.

Thanks @BenCookie95 👍

@DHaussermann DHaussermann added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester. May occur at the same time as Dev Review labels Nov 16, 2023
@BenCookie95 BenCookie95 merged commit 2a896f8 into master Nov 16, 2023
43 checks passed
@BenCookie95 BenCookie95 deleted the MM-54933 branch November 16, 2023 16:24
@mattermost-build
Copy link
Contributor

Cherry pick is scheduled.

mattermost-build pushed a commit that referenced this pull request Nov 16, 2023
…ained teams and channels (#24987)

* add the ability to @ mention custom groups in group constrained teams
---------
Co-authored-by: Mattermost Build <build@mattermost.com>

(cherry picked from commit 2a896f8)
@mattermost-build mattermost-build added CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone and removed CherryPick/Approved Meant for the quality or patch release tracked in the milestone labels Nov 16, 2023
mattermost-build added a commit that referenced this pull request Nov 16, 2023
@amyblais amyblais added Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation labels Nov 16, 2023
@BenCookie95
Copy link
Contributor Author

/cherry-pick release-8.1

@mattermost-build
Copy link
Contributor

Cherry pick is scheduled.

@mattermost-build
Copy link
Contributor

mattermost-build commented Nov 28, 2023

Error trying doing the automated Cherry picking. Please do this manually

+++ Updating remotes...
Fetching upstream
hostfile_replace_entries: mkstemp: Read-only file system
update_known_hosts: hostfile_replace_entries failed for /app/.ssh/known_hosts: Read-only file system
From github.com:mattermost/mattermost
   6ef43761f9..7a75e0d81b  CLD-6537              -> upstream/CLD-6537
   e95a331f38..3e797f3c59  bumpMySQL             -> upstream/bumpMySQL
   30381835ea..ac1250d568  feat/MM-55031-oauth-connection-app-backend -> upstream/feat/MM-55031-oauth-connection-app-backend
   2e537ce074..f686be76b6  feature-wrangler-mono -> upstream/feature-wrangler-mono
 * [new branch]            gm_conversion_modal_narrow_fix -> upstream/gm_conversion_modal_narrow_fix
   6a021a29f9..95670abcea  master                -> upstream/master
   e0132cfcc5..5fba7e1427  mm-55094-improve-roles-by-name-endpoint -> upstream/mm-55094-improve-roles-by-name-endpoint
 * [new branch]            mm-55484-add-bookmark-permissions -> upstream/mm-55484-add-bookmark-permissions
   d98e52cd93..7bf5dcb8fb  release-9.3           -> upstream/release-9.3
 * [new tag]               v9.3.0-rc2            -> v9.3.0-rc2
Fetching upstream
hostfile_replace_entries: mkstemp: Read-only file system
update_known_hosts: hostfile_replace_entries failed for /app/.ssh/known_hosts: Read-only file system
+++ Updating remotes done...
+++ Creating local branch automated-cherry-pick-of-mattermost-#24987-upstream-release-8.1-1701199238
Switched to a new branch 'automated-cherry-pick-of-mattermost-#24987-upstream-release-8.1-1701199238'
Branch 'automated-cherry-pick-of-mattermost-#24987-upstream-release-8.1-1701199238' set up to track remote branch 'release-8.1' from 'upstream'.

+++ About to attempt cherry pick of PR #24987 with merge commit 2a896f8420fbd7a71ba146688f4cc63ecba8bcdf.

Auto-merging server/channels/app/notification.go
Auto-merging server/channels/app/notification_test.go
CONFLICT (content): Merge conflict in server/channels/app/notification_test.go
Auto-merging webapp/channels/src/packages/mattermost-redux/src/selectors/entities/groups.test.ts
Auto-merging webapp/channels/src/packages/mattermost-redux/src/selectors/entities/groups.ts
CONFLICT (content): Merge conflict in webapp/channels/src/packages/mattermost-redux/src/selectors/entities/groups.ts
error: could not apply 2a896f8420... [MM-54933] Add the ability to @ mention custom groups in group constrained teams and channels (#24987)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

+++ Conflicts detected:

UU server/channels/app/notification_test.go
UU webapp/channels/src/packages/mattermost-redux/src/selectors/entities/groups.ts
Aborting.

+++ Aborting in-progress git cherry-pick.

+++ Returning you to the master branch and cleaning up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Done Required changelog entry has been written CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone Docs/Not Needed Does not require documentation release-note Denotes a PR that will be considered when it comes time to generate release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants