Skip to content

Commit 07541cf

Browse files
revert: The blocked user no longer appears in the conversation list - WPB-24403 🍒 (#4915)
Co-authored-by: François Benaiteau <netbe@users.noreply.github.com> Co-authored-by: François Benaiteau <francois.benaiteau@wire.com>
1 parent cc24eee commit 07541cf

62 files changed

Lines changed: 48 additions & 942 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

WireDomain/Sources/WireDomain/Repositories/Connections/ConnectionsLocalStore.swift

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -48,20 +48,7 @@ final class ConnectionsLocalStore: ConnectionsLocalStoreProtocol {
4848
conversation.lastModifiedDate = connectionInfo.lastUpdate
4949
conversation.addParticipantAndUpdateConversationState(user: connection.to, role: nil)
5050

51-
// `ConnectionValidator` cleans up stale connections between users, so we normally (re)set this link here.
52-
// But when the two users already have an established MLS conversation, we keep it: overwriting it with the
53-
// Proteus connection conversation would break the link and hide the conversation from the list.
54-
//
55-
// `migratedToMLS` is only set on the proteus→MLS migration path, so it misses MLS one-on-ones that were
56-
// established directly. Relying on it alone lets the proteus connection conversation overwrite the MLS
57-
// link, which surfaces the proteus (read-only) conversation instead of the MLS one — including on the
58-
// side of a user who was blocked, since that side is never notified and should keep messaging. [WPB-24403]
59-
let existing = connection.to.oneOnOneConversation
60-
let isEstablishedMLS = existing?.messageProtocol == .mls
61-
&& (existing?.mlsStatus == .ready || existing?.migratedToMLS == true)
62-
if !isEstablishedMLS {
63-
connection.to.oneOnOneConversation = conversation
64-
}
51+
connection.to.oneOnOneConversation = conversation
6552
connection.status = connectionInfo.status
6653
connection.lastUpdateDateInGMT = connectionInfo.lastUpdate
6754

WireDomain/Sources/WireDomain/Repositories/Conversations/LocalStore/ConversationLocalStore+Group.swift

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -102,18 +102,9 @@ extension ConversationLocalStore {
102102
}
103103

104104
guard let otherUser = localConversation.localParticipantsExcludingSelf.first else {
105-
// The other participant is absent from the synced member list. This is exactly what the
106-
// backend returns to the party that was *blocked*: the blocker is omitted from the 1:1
107-
// member list. That side is never notified of the block and must keep its conversation
108-
// usable, so we must not tear down an established 1:1 — forcing it read-only and marking
109-
// the MLS group invalid (which wipes it and drops the user back to the read-only Proteus
110-
// conversation). Only do so when there is genuinely no user behind the conversation
111-
// (e.g. a malformed/empty 1:1 that was never linked). [WPB-24403]
112-
if localConversation.oneOnOneUser == nil {
113-
localConversation.isForcedReadOnly = true
114-
if localConversation.messageProtocol.isOne(of: .mls, .mixed) {
115-
localConversation.mlsStatus = .invalid
116-
}
105+
localConversation.isForcedReadOnly = true
106+
if localConversation.messageProtocol.isOne(of: .mls, .mixed) {
107+
localConversation.mlsStatus = .invalid
117108
}
118109
return
119110
}

WireDomain/Sources/WireDomain/Repositories/Conversations/Protocols/ConversationRepositoryProtocol.swift

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -90,17 +90,6 @@ public protocol ConversationRepositoryProtocol: Sendable {
9090
groupID: String
9191
) async -> ZMConversation?
9292

93-
/// Whether a conversation stored locally is a group.
94-
/// - Parameters:
95-
/// - id: The ID of the conversation.
96-
/// - domain: The domain of the conversation if any.
97-
/// - returns: `true` if a local conversation with this id exists and is of type `.group`.
98-
99-
func isGroupConversation(
100-
id: UUID,
101-
domain: String?
102-
) async -> Bool
103-
10493
/// Deletes a conversation locally.
10594
/// - Parameters:
10695
/// - id: The ID of the conversation.

WireDomain/Sources/WireDomain/Repositories/Conversations/Repository/ConversationRepository.swift

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -218,13 +218,6 @@ public final class ConversationRepository: ConversationRepositoryProtocol {
218218

219219
}
220220

221-
public func isGroupConversation(id: UUID, domain: String?) async -> Bool {
222-
guard let conversation = await fetchConversation(id: id, domain: domain) else {
223-
return false
224-
}
225-
return await conversationsLocalStore.isGroupConversation(conversation)
226-
}
227-
228221
public func deleteConversation(
229222
id: UUID,
230223
domain: String?

WireDomain/Sources/WireDomain/WorkAgent/WorkItem/UpdateConversationItem.swift

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -63,15 +63,7 @@ struct UpdateConversationItem: WorkItem {
6363
attributes: [.conversationId: conversationID.id.uuidString],
6464
.init(self)
6565
)
66-
// Only group conversations should be removed locally when the backend reports them
67-
// missing. A 1:1 whose connection has just transitioned to `.blocked` (and similarly
68-
// `.cancelled` / `.ignored`) still returns 404 from GET /conversations/{id} because
69-
// the backend only exposes the underlying connection, not a conversation resource.
70-
// Marking such conversations as `isDeletedRemotely` would hide them from the list
71-
// and prevent the user from unblocking from the cell long-press menu.
72-
if await repository.isGroupConversation(id: conversationID.id, domain: conversationID.domain) {
73-
try await repository.deleteConversation(id: conversationID.id, domain: conversationID.domain)
74-
}
66+
try await repository.deleteConversation(id: conversationID.id, domain: conversationID.domain)
7567

7668
} catch {
7769
// giving more context to the error

WireDomain/Sources/WireDomainSupport/Sourcery/generated/AutoMockable.generated.swift

Lines changed: 0 additions & 18 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

WireDomain/Tests/WireDomainTests/LocalStores/ConnectionsLocalStoreTests.swift

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -169,46 +169,6 @@ final class ConnectionsLocalStoreTests: XCTestCase {
169169
}
170170
}
171171

172-
func testStoreConnection_GivenLinkedToEstablishedMLSOneOnOne_ItPreservesTheMLSLink() async throws {
173-
// Given a stored connection whose user is linked to an established MLS one-on-one.
174-
// The conversation is created directly as MLS, so `migratedToMLS` stays `false` — the
175-
// case the previous `migratedToMLS`-only guard failed to protect.
176-
try await sut.storeConnection(Scaffolding.connection)
177-
178-
let mlsConversationID = UUID()
179-
try await context.perform { [context, modelHelper] in
180-
let storedConnection = try XCTUnwrap(ZMConnection.fetch(
181-
userID: Scaffolding.member2ID.uuid,
182-
domain: Scaffolding.member2ID.domain,
183-
in: context
184-
))
185-
let mlsConversation = modelHelper!.createMLSConversation(
186-
id: mlsConversationID,
187-
mlsStatus: .ready,
188-
conversationType: .oneOnOne,
189-
in: context
190-
)
191-
storedConnection.to.oneOnOneConversation = mlsConversation
192-
try context.save()
193-
}
194-
195-
// When the backend reports the proteus connection conversation again (e.g. on a connection
196-
// update such as a block, which the blocked side is never notified about).
197-
try await sut.storeConnection(Scaffolding.connection)
198-
199-
// Then the MLS link is preserved instead of being overwritten by the proteus conversation.
200-
try await context.perform { [context] in
201-
let storedConnection = try XCTUnwrap(ZMConnection.fetch(
202-
userID: Scaffolding.member2ID.uuid,
203-
domain: Scaffolding.member2ID.domain,
204-
in: context
205-
))
206-
let relatedConversation = try XCTUnwrap(storedConnection.to.oneOnOneConversation)
207-
XCTAssertEqual(relatedConversation.messageProtocol, .mls)
208-
XCTAssertEqual(relatedConversation.remoteIdentifier, mlsConversationID)
209-
}
210-
}
211-
212172
private enum Scaffolding {
213173
static let member1ID = WireDataModel.QualifiedID(
214174
uuid: .mockID1,

WireDomain/Tests/WireDomainTests/LocalStores/ConversationLocalStoreTests.swift

Lines changed: 0 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -545,57 +545,6 @@ final class ConversationLocalStoreTests: XCTestCase {
545545
}
546546
}
547547

548-
func testLinkOneOnOneUserIfNeeded_GivenLinkedUserButNoParticipants_ItKeepsMLSConversationUsable() async {
549-
// Given an established MLS 1:1 linked to its one-on-one user but with no participants — the
550-
// state the blocked party ends up in once the backend removes the blocker from the members.
551-
let conversation = await context.perform { [self] in
552-
_ = modelHelper.createSelfUser(in: context)
553-
let conversation = modelHelper.createMLSConversation(
554-
mlsStatus: .ready,
555-
conversationType: .oneOnOne,
556-
in: context
557-
)
558-
let otherUser = modelHelper.createUser(id: Scaffolding.otherUserID, domain: Scaffolding.domain, in: context)
559-
conversation.oneOnOneUser = otherUser
560-
return conversation
561-
}
562-
563-
// When
564-
await context.perform { [self] in
565-
sut.linkOneOnOneUserIfNeeded(for: conversation)
566-
}
567-
568-
// Then the MLS 1:1 is preserved and remains usable (not wiped, not read-only).
569-
await context.perform {
570-
XCTAssertEqual(conversation.mlsStatus, .ready)
571-
XCTAssertEqual(conversation.messageProtocol, .mls)
572-
XCTAssertFalse(conversation.isForcedReadOnly)
573-
}
574-
}
575-
576-
func testLinkOneOnOneUserIfNeeded_GivenNoLinkedUserAndNoParticipants_ItInvalidatesMLSConversation() async {
577-
// Given an MLS 1:1 with no linked one-on-one user (a genuinely empty/malformed 1:1).
578-
let conversation = await context.perform { [self] in
579-
_ = modelHelper.createSelfUser(in: context)
580-
return modelHelper.createMLSConversation(
581-
mlsStatus: .ready,
582-
conversationType: .oneOnOne,
583-
in: context
584-
)
585-
}
586-
587-
// When
588-
await context.perform { [self] in
589-
sut.linkOneOnOneUserIfNeeded(for: conversation)
590-
}
591-
592-
// Then the MLS group is invalidated and the conversation forced read-only.
593-
await context.perform {
594-
XCTAssertEqual(conversation.mlsStatus, .invalid)
595-
XCTAssertTrue(conversation.isForcedReadOnly)
596-
}
597-
}
598-
599548
private enum Scaffolding {
600549

601550
static let selfUserId = UUID.mockID1

WireDomain/Tests/WireDomainTests/WorkAgent/WorkItem/UpdateConversationItemTests.swift

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ struct UpdateConversationItemTests {
3434
self.repository = MockConversationRepositoryProtocol()
3535
self.conversationID = .init(id: UUID(), domain: String.randomDomain())
3636
self.sut = UpdateConversationItem(repository: repository, conversationID: conversationID)
37-
repository.isGroupConversationIdDomain_MockValue = true
3837
}
3938

4039
@Test("It calls repository pull conversation")
@@ -62,19 +61,6 @@ struct UpdateConversationItemTests {
6261
#expect(invocation?.domain == conversationID.domain && invocation?.id == conversationID.id)
6362
}
6463

65-
@Test("It does not delete 1:1 conversation if not found")
66-
func startsDeleteOneOnOneConversationIfNotFound() async throws {
67-
// Given
68-
repository.isGroupConversationIdDomain_MockValue = false
69-
repository.pullConversationIdDomain_MockError = ConversationRepositoryError.conversationNotFound
70-
repository.deleteConversationIdDomain_MockMethod = { _, _ in }
71-
// When
72-
try await sut.start()
73-
74-
// Then
75-
#expect(repository.deleteConversationIdDomain_Invocations.isEmpty)
76-
}
77-
7864
@Test("It throws an error in case of non supported error")
7965
func startThrowsInOtherError() async throws {
8066
// Given

WireUI/Sources/WireLocators/Locators.swift

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,7 @@ public enum Locators {
6363
case bottomBarDriveButton
6464
case createGroupOrSearchButton
6565
case conversationCell
66-
case blockOptionOnContextMenu
67-
case unblockOptionOnContextMenu
66+
case blockOptionOnContextMenu = "Block…"
6867
case clearOptionOnContextMenu = "Clear Content…"
6968
case clearButtonOnBottomSheet
7069
case leaveButtonOnBottomSheet

0 commit comments

Comments
 (0)