refactor: process conversation rename event - WPB-10177 #2175
Conversation
… up redundant messages code - WPB-10177
echoes-hq
bot
added
the
echoes: technical-roadmap/technical-debt
| message, | ||
| to: conversation | ||
| var systemMessageType: MessageType = switch reason { | ||
| case .userDeleted, .left: |
There was a problem hiding this comment.
| case .userDeleted, .left: | |
| case .userDeleted, .userLeft: |
| member: (senderID, senderDomain), | ||
| date: date | ||
| ) | ||
| case .removed: |
There was a problem hiding this comment.
| case .removed: | |
| case .userRemoved: |
| ) | ||
|
|
||
| guard let localConversation else { | ||
| return WireLogger.eventProcessing.error( |
There was a problem hiding this comment.
Personally I find this pattern (returning void by calling something that returns void) super confusing. If I look at WireLogger.eventProcessing.error without knowing about our WireLogger I assume we are actually throwing an error then see that there is no throw. Is this a common pattern? I've only seen it at Wire.
| to conversation: ZMConversation | ||
| func updateConversationName( | ||
| newName: String, | ||
| conversationID: UUID, |
There was a problem hiding this comment.
Optional: We do have a lot of ID (UUID), domain (String?) pairs. They are almost a QualifiedID but not quite. I wonder if there is an improvement we can make here to save some boilerplate.
I also wonder if it is worth representing a conversation id+domain differently from a sender id+domain to prevent accidentally passing incorrect values.
I would consider introducing the following kinds of types:
struct ConversationID {
id: UUID
domain: String?
}| ) async -> Set<ZMUser> { | ||
| await withTaskGroup(of: ZMUser.self) { taskGroup in | ||
| for userID in userIDs { | ||
| taskGroup.addTask { [self] in |
There was a problem hiding this comment.
Not a request for change but just a thought:
I think there is probably unnecessary overhead with awaiting, locking in core data etc here. To avoid this we I think we should have a UserLocalStore.fetchOrCreateUsers(userIDs: [(id: UUID, domain: String?)]) -> [ZMUser] and have everything done within a single perform.
When wrapping core data it is probably more performant that our methods accept arrays of values I guess.
WPB-10177
Key points
This PR is part of the quick sync refactoring plan and is related to processing the multiple events we receive from the backend or the push channel.
Specifically, this PR is about porting the existing implementation of the
ConversationRenameevent.In addition:
ConversationRepositorynow that we have a dedicatedMessageRepositoryMessageRepositorywhere we need to.Testing
Checklist
[WPB-XXX].