Skip to content

Conversation

@nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented Dec 18, 2020

Fix #1329

  • Use a shared method to handle add users of a group to a conversation
  • Store groups as attendees for further usage
  • Add users when they are added to a group
  • Remove users when they are removed from a group and don't have another group that is a member
  • Add integration tests for those things
  • Do the same tracking with circles (if possible)
  • Check all attendee places whether we should exclude groups (to gain performance)
  • Make sure the display name is set for "group attendees"
  • Make sure autocomplete doesn't suggest groups you already added
  • Always show groups but at the end of the participant list
  • 💭 groups should have 2 options or which one do we implement:
    • 1. Remove group only
    • 2. Remove group and also remove all members (that don't have another group)

@nickvergessen nickvergessen added 2. developing enhancement feature: api 🛠️ OCS API for conversations, chats and participants labels Dec 18, 2020
@nickvergessen nickvergessen added this to the 💖 Next Major (22) milestone Dec 18, 2020
@jancborchardt
Copy link
Member

Should groups be always listed as participants, or only for moderators (or even only when searching?)

I’d say yes they should always be listed. Only showing them for certain people or when searching it would be a bit restrictive.
We can make sure they are distinct. For example we can do (either one or multiple of these):

  • Have the list entries for groups on the bottom of the list – still visible but sort of out of the way (could also have an intermediate heading)
  • Have a more distinct icon (primary color e.g.)
  • A subline saying "23 participants" (or "members") which is informative and also separates it from the other entries

Should have 2 options on groups: 1. Remove, 2. Remove and also remove all members (that don't have another group)

I’d expect only one "Remove" action. If e.g. I added a dev, and also a group "Engineering" where the dev is in, and I remove the "Engineering" group, I would expect that the individually added dev should still be a member.

@nickvergessen nickvergessen force-pushed the feature/1329/sync-group-members-with-conversation-participants branch from 77c9ace to 70edd80 Compare March 25, 2021 11:00
@nickvergessen
Copy link
Member Author

I’d expect only one "Remove" action. If e.g. I added a dev, and also a group "Engineering" where the dev is in, and I remove the "Engineering" group, I would expect that the individually added dev should still be a member.

So let me draw a case:

  • Group Engineering is users A, B, C
  • I create a conversation
  • I add user A
  • I add group Engineering
  • Now I, A, B and C are in
  • I remove group Engineering
  • Now we can have the result:
    • I, A, B and C (group members are not removed) or:
    • I (group members are removed)

Your comment is not explicit, I read it as you would like to have "I and A" but that is not going to work unless we add tones of additional logic and database entries.

@nickvergessen nickvergessen force-pushed the feature/1329/sync-group-members-with-conversation-participants branch from 4aa512a to f8d5071 Compare March 25, 2021 15:08
@nickvergessen
Copy link
Member Author

I currently went for "Remove group and also remove all members (that don't have another group)" because that is more in line with the expected behaviour that people are removed from chats when they are removed from a group and don't have another group in the chat. I think at this point it's more a matter of documentation and clarifying it in the UI maybe?

@nickvergessen
Copy link
Member Author

But ready to review for now

@nickvergessen nickvergessen force-pushed the feature/1329/sync-group-members-with-conversation-participants branch from 59fa88f to 5730ef0 Compare March 26, 2021 13:27
@nickvergessen nickvergessen marked this pull request as ready for review March 26, 2021 13:27
Copy link
Member

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

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

Woooorks!

@nickvergessen
Copy link
Member Author

After a discussion with jan we came to the conclusion that owners and moderators should not be dropped from the chat when their groups are removed. I will add that soon.

@nickvergessen
Copy link
Member Author

run.sh: line 22: 533 Segmentation fault (core dumped) php -S localhost:8080 -t ${ROOT_DIR}**

Restarted the tests

@nickvergessen nickvergessen requested a review from danxuliu April 7, 2021 10:38
Copy link
Member

@danxuliu danxuliu 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 works 👍

I left some comments, but they are not blockers.

@nickvergessen
Copy link
Member Author

Updated

@nickvergessen nickvergessen requested a review from danxuliu April 8, 2021 07:54
Copy link
Member

@danxuliu danxuliu 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 works 👍

@nickvergessen nickvergessen merged commit 24bcefa into master Apr 8, 2021
@nickvergessen nickvergessen deleted the feature/1329/sync-group-members-with-conversation-participants branch April 8, 2021 12:27
@forlornidealist
Copy link

I know this is closed, but it does not seem to work with LDAP groups. If I add or remove a user to an LDAP group, they are not added/removed from the group chat like they are if I create the same group within Nextcloud manually. I will open a new issue for this, but for now I will create a script that creates a local group, named NC- that corresponds to each relevant AD group.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review enhancement feature: api 🛠️ OCS API for conversations, chats and participants

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Synchronize group members in group chat continuously

6 participants