Improve global team conversation handling and self conversation creation error. #2862
Conversation
zebot
added
the
ok-to-test
| E.getUserTeams (tUnqualified lusr) >>= \tids -> | ||
| runError @InternalError $ | ||
| runError @(Tagged 'NotATeamMember ()) | ||
| (for_ tids $ \tid -> getGlobalTeamConversation lusr tid) |
There was a problem hiding this comment.
Why leave a side effect in listing conversations; which, as a sideeffect of getting global team conversations, creates one?
Why not create the global team conversation during the creation of a team?
And if you'd like to enable this functionality for existing teams, you could add a one-off data migration as done in https://github.com/wireapp/wire-server/tree/develop/services/galley/migrate-data/src - read the list of teams, insert one record into conversations, and from then on all teams will have this.
There was a problem hiding this comment.
I think it would also be possible to use the approach you suggest, but after some discussion with other backend people, I'm not sure it's really worth it.
Ultimately, the "side effect" of creating the global team conversation is not really a side effect, since it's not observable from the outside. The small extra lookup needed when listing conversations doesn't seem like a huge problem, considering that listing conversations is already a slow operation.
On the other hand, preemptively creating all GTCs (and for consistency, all MLS self conversations) forces the creation of all these extra conversations for inactive teams and users. I admit that this is probably not a big concern.
Therefore, we decided to stick with the current approach for now. I'm happy to discuss this further if you still think it's a mistake.
elland
force-pushed
the
elland/global-team-conv-refine
branch
from
6992bbf
to
81fb3f8
Compare
| convMetadata = | ||
| ConversationMetadata | ||
| { cnvmType = GlobalTeamConv, | ||
| -- FUTUREWORK: Make this a qualified user ID. |
There was a problem hiding this comment.
That FUTUREWORK is proably better placed at the definition of ConversationMetadata
There was a problem hiding this comment.
It is already there. Copy paste remnants :)
elland
changed the title
* Revert "[FS-1267] Patch issue with initial commit bundle throwing a global team conversation error (#2887)" This reverts commit 1cc2bd2. * Revert "Commented out GTC for release. (#2879)" This reverts commit 49da310. * Revert "Improve global team conversation handling and self conversation creation error. (#2862)" This reverts commit 381bf7b. * Revert "[FS-926] Create new conversation type for global team conversation (#2753)" This reverts commit c4c9ea2.
Removed global team conversation creator, improved tests, improve adding users.
Improves error thrown when creating the self conversation, 500 bad.