From 5f4c2890f6ee22d39cc19d7c8819a61ce10c23f5 Mon Sep 17 00:00:00 2001 From: Mauro <34335419+Velin92@users.noreply.github.com> Date: Mon, 30 Sep 2024 14:32:35 +0200 Subject: [PATCH] Remove message pinning FF (#3318) --- .../Sources/Application/AppSettings.swift | 3 -- .../RoomDetailsScreenModels.swift | 1 - .../RoomDetailsScreenViewModel.swift | 9 ++---- .../View/RoomDetailsScreen.swift | 28 +++++++------------ .../Screens/RoomScreen/RoomScreenModels.swift | 3 +- .../RoomScreen/RoomScreenViewModel.swift | 11 ++------ .../DeveloperOptionsScreenModels.swift | 1 - .../View/DeveloperOptionsScreen.swift | 7 ----- .../Screens/Timeline/TimelineViewModel.swift | 7 ++--- .../Style/TimelineItemBubbledStylerView.swift | 6 ++-- .../Sources/Services/Client/ClientProxy.swift | 9 ++---- .../Sources/RoomScreenViewModelTests.swift | 2 -- .../Sources/TimelineViewModelTests.swift | 6 ---- 13 files changed, 21 insertions(+), 72 deletions(-) diff --git a/ElementX/Sources/Application/AppSettings.swift b/ElementX/Sources/Application/AppSettings.swift index d2985c8c7..3f8db7204 100644 --- a/ElementX/Sources/Application/AppSettings.swift +++ b/ElementX/Sources/Application/AppSettings.swift @@ -271,9 +271,6 @@ final class AppSettings { @UserPreference(key: UserDefaultsKeys.fuzzyRoomListSearchEnabled, defaultValue: false, storageType: .userDefaults(store)) var fuzzyRoomListSearchEnabled - @UserPreference(key: UserDefaultsKeys.pinningEnabled, defaultValue: true, storageType: .userDefaults(store)) - var pinningEnabled - enum SlidingSyncDiscovery: Codable { case proxy, native, forceNative } @UserPreference(key: UserDefaultsKeys.slidingSyncDiscovery, defaultValue: .native, storageType: .userDefaults(store)) var slidingSyncDiscovery: SlidingSyncDiscovery diff --git a/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenModels.swift b/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenModels.swift index 72ff876c9..e1854f8a4 100644 --- a/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenModels.swift +++ b/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenModels.swift @@ -44,7 +44,6 @@ struct RoomDetailsScreenViewState: BindableState { var canEditRolesOrPermissions = false var notificationSettingsState: RoomDetailsNotificationSettingsState = .loading var canJoinCall = false - var isPinningEnabled = false var pinnedEventsActionState = RoomDetailsScreenPinnedEventsActionState.loading var canEdit: Bool { diff --git a/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenViewModel.swift b/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenViewModel.swift index b435eeb23..55553d881 100644 --- a/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenViewModel.swift +++ b/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenViewModel.swift @@ -75,13 +75,8 @@ class RoomDetailsScreenViewModel: RoomDetailsScreenViewModelType, RoomDetailsScr bindings: .init()), mediaProvider: mediaProvider) - appSettings.$pinningEnabled - .weakAssign(to: \.state.isPinningEnabled, on: self) - .store(in: &cancellables) - - appSettings.$pinningEnabled - .combineLatest(appMediator.networkMonitor.reachabilityPublisher) - .filter { $0.0 && $0.1 == .reachable } + appMediator.networkMonitor.reachabilityPublisher + .filter { $0 == .reachable } .receive(on: DispatchQueue.main) .sink { [weak self] _ in self?.setupPinnedEventsTimelineProviderIfNeeded() diff --git a/ElementX/Sources/Screens/RoomDetailsScreen/View/RoomDetailsScreen.swift b/ElementX/Sources/Screens/RoomDetailsScreen/View/RoomDetailsScreen.swift index 3b682e8b2..919aa3aff 100644 --- a/ElementX/Sources/Screens/RoomDetailsScreen/View/RoomDetailsScreen.swift +++ b/ElementX/Sources/Screens/RoomDetailsScreen/View/RoomDetailsScreen.swift @@ -189,15 +189,13 @@ struct RoomDetailsScreen: View { context.send(viewAction: .toggleFavourite(isFavourite: newValue)) } - if context.viewState.isPinningEnabled { - ListRow(label: .default(title: L10n.screenRoomDetailsPinnedEventsRowTitle, - icon: \.pin), - details: context.viewState.pinnedEventsActionState.isLoading ? .isWaiting(true) : .title(context.viewState.pinnedEventsActionState.count), - kind: context.viewState.pinnedEventsActionState.isLoading ? .label : .navigationLink(action: { - context.send(viewAction: .processTapPinnedEvents) - })) - .disabled(context.viewState.pinnedEventsActionState.isLoading) - } + ListRow(label: .default(title: L10n.screenRoomDetailsPinnedEventsRowTitle, + icon: \.pin), + details: context.viewState.pinnedEventsActionState.isLoading ? .isWaiting(true) : .title(context.viewState.pinnedEventsActionState.count), + kind: context.viewState.pinnedEventsActionState.isLoading ? .label : .navigationLink(action: { + context.send(viewAction: .processTapPinnedEvents) + })) + .disabled(context.viewState.pinnedEventsActionState.isLoading) if context.viewState.canEditRolesOrPermissions, context.viewState.dmRecipient == nil { ListRow(label: .default(title: L10n.screenRoomDetailsRolesAndPermissions, @@ -320,8 +318,6 @@ struct RoomDetailsScreen_Previews: PreviewProvider, TestablePreview { var notificationSettingsProxyMockConfiguration = NotificationSettingsProxyMockConfiguration() notificationSettingsProxyMockConfiguration.roomMode.isDefault = false let notificationSettingsProxy = NotificationSettingsProxyMock(with: notificationSettingsProxyMockConfiguration) - let appSettings = AppSettings() - appSettings.pinningEnabled = true return RoomDetailsScreenViewModel(roomProxy: roomProxy, clientProxy: ClientProxyMock(.init()), @@ -331,7 +327,7 @@ struct RoomDetailsScreen_Previews: PreviewProvider, TestablePreview { notificationSettingsProxy: notificationSettingsProxy, attributedStringBuilder: AttributedStringBuilder(mentionBuilder: MentionBuilder()), appMediator: AppMediatorMock.default, - appSettings: appSettings) + appSettings: ServiceLocator.shared.settings) }() static let dmRoomViewModel = { @@ -348,8 +344,6 @@ struct RoomDetailsScreen_Previews: PreviewProvider, TestablePreview { canonicalAlias: "#alias:domain.com", members: members)) let notificationSettingsProxy = NotificationSettingsProxyMock(with: .init()) - let appSettings = AppSettings() - appSettings.pinningEnabled = true return RoomDetailsScreenViewModel(roomProxy: roomProxy, clientProxy: ClientProxyMock(.init()), @@ -359,7 +353,7 @@ struct RoomDetailsScreen_Previews: PreviewProvider, TestablePreview { notificationSettingsProxy: notificationSettingsProxy, attributedStringBuilder: AttributedStringBuilder(mentionBuilder: MentionBuilder()), appMediator: AppMediatorMock.default, - appSettings: appSettings) + appSettings: ServiceLocator.shared.settings) }() static let simpleRoomViewModel = { @@ -375,8 +369,6 @@ struct RoomDetailsScreen_Previews: PreviewProvider, TestablePreview { isEncrypted: false, members: members)) let notificationSettingsProxy = NotificationSettingsProxyMock(with: .init()) - let appSettings = AppSettings() - appSettings.pinningEnabled = true return RoomDetailsScreenViewModel(roomProxy: roomProxy, clientProxy: ClientProxyMock(.init()), @@ -386,7 +378,7 @@ struct RoomDetailsScreen_Previews: PreviewProvider, TestablePreview { notificationSettingsProxy: notificationSettingsProxy, attributedStringBuilder: AttributedStringBuilder(mentionBuilder: MentionBuilder()), appMediator: AppMediatorMock.default, - appSettings: appSettings) + appSettings: ServiceLocator.shared.settings) }() static var previews: some View { diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift index cd2d31f02..9f4bb68a2 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift @@ -28,11 +28,10 @@ struct RoomScreenViewState: BindableState { var roomAvatar: RoomAvatar var lastScrollDirection: ScrollDirection? - var isPinningEnabled = false // This is used to control the banner var pinnedEventsBannerState: PinnedEventsBannerState = .loading(numbersOfEvents: 0) var shouldShowPinnedEventsBanner: Bool { - isPinningEnabled && !pinnedEventsBannerState.isEmpty && lastScrollDirection != .top + !pinnedEventsBannerState.isEmpty && lastScrollDirection != .top } var canJoinCall = false diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift index 8c33e3e3a..990d30408 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift @@ -124,15 +124,8 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol } .store(in: &cancellables) - let pinningEnabledPublisher = appSettings.$pinningEnabled - - pinningEnabledPublisher - .weakAssign(to: \.state.isPinningEnabled, on: self) - .store(in: &cancellables) - - pinningEnabledPublisher - .combineLatest(appMediator.networkMonitor.reachabilityPublisher) - .filter { $0.0 && $0.1 == .reachable } + appMediator.networkMonitor.reachabilityPublisher + .filter { $0 == .reachable } .receive(on: DispatchQueue.main) .sink { [weak self] _ in self?.setupPinnedEventsTimelineProviderIfNeeded() diff --git a/ElementX/Sources/Screens/Settings/DeveloperOptionsScreen/DeveloperOptionsScreenModels.swift b/ElementX/Sources/Screens/Settings/DeveloperOptionsScreen/DeveloperOptionsScreenModels.swift index 8a3d11809..15b1f7d68 100644 --- a/ElementX/Sources/Screens/Settings/DeveloperOptionsScreen/DeveloperOptionsScreenModels.swift +++ b/ElementX/Sources/Screens/Settings/DeveloperOptionsScreen/DeveloperOptionsScreenModels.swift @@ -46,7 +46,6 @@ protocol DeveloperOptionsProtocol: AnyObject { var hideUnreadMessagesBadge: Bool { get set } var elementCallBaseURLOverride: URL? { get set } var fuzzyRoomListSearchEnabled: Bool { get set } - var pinningEnabled: Bool { get set } var enableOnlySignedDeviceIsolationMode: Bool { get set } } diff --git a/ElementX/Sources/Screens/Settings/DeveloperOptionsScreen/View/DeveloperOptionsScreen.swift b/ElementX/Sources/Screens/Settings/DeveloperOptionsScreen/View/DeveloperOptionsScreen.swift index 30f6e8ec8..723ae359d 100644 --- a/ElementX/Sources/Screens/Settings/DeveloperOptionsScreen/View/DeveloperOptionsScreen.swift +++ b/ElementX/Sources/Screens/Settings/DeveloperOptionsScreen/View/DeveloperOptionsScreen.swift @@ -35,13 +35,6 @@ struct DeveloperOptionsScreen: View { Text(context.viewState.slidingSyncFooter) } - Section("Message Pinning") { - Toggle(isOn: $context.pinningEnabled) { - Text("Enable message pinning") - Text("Requires app reboot") - } - } - Section("Room List") { Toggle(isOn: $context.hideUnreadMessagesBadge) { Text("Hide grey dots") diff --git a/ElementX/Sources/Screens/Timeline/TimelineViewModel.swift b/ElementX/Sources/Screens/Timeline/TimelineViewModel.swift index 623b5d412..ffc0386f2 100644 --- a/ElementX/Sources/Screens/Timeline/TimelineViewModel.swift +++ b/ElementX/Sources/Screens/Timeline/TimelineViewModel.swift @@ -336,8 +336,7 @@ class TimelineViewModel: TimelineViewModelType, TimelineViewModelProtocol { state.canCurrentUserRedactSelf = false } - if appSettings.pinningEnabled, - case let .success(value) = await roomProxy.canUserPinOrUnpin(userID: roomProxy.ownUserID) { + if case let .success(value) = await roomProxy.canUserPinOrUnpin(userID: roomProxy.ownUserID) { state.canCurrentUserPin = value } else { state.canCurrentUserPin = false @@ -444,9 +443,7 @@ class TimelineViewModel: TimelineViewModelType, TimelineViewModelProtocol { } private func updatePinnedEventIDs() async { - if appSettings.pinningEnabled { - state.pinnedEventIDs = await roomProxy.pinnedEventIDs - } + state.pinnedEventIDs = await roomProxy.pinnedEventIDs } private func setupDirectRoomSubscriptionsIfNeeded() { diff --git a/ElementX/Sources/Screens/Timeline/View/Style/TimelineItemBubbledStylerView.swift b/ElementX/Sources/Screens/Timeline/View/Style/TimelineItemBubbledStylerView.swift index 32b33c57a..49ca3828b 100644 --- a/ElementX/Sources/Screens/Timeline/View/Style/TimelineItemBubbledStylerView.swift +++ b/ElementX/Sources/Screens/Timeline/View/Style/TimelineItemBubbledStylerView.swift @@ -355,8 +355,6 @@ private extension View { struct TimelineItemBubbledStylerView_Previews: PreviewProvider, TestablePreview { static let viewModel = TimelineViewModel.mock static let viewModelWithPins: TimelineViewModel = { - var settings = AppSettings() - settings.pinningEnabled = true let roomProxy = JoinedRoomProxyMock(.init(name: "Preview Room", pinnedEventIDs: [""])) return TimelineViewModel(roomProxy: roomProxy, focussedEventID: nil, @@ -366,7 +364,7 @@ struct TimelineItemBubbledStylerView_Previews: PreviewProvider, TestablePreview voiceMessageMediaManager: VoiceMessageMediaManagerMock(), userIndicatorController: ServiceLocator.shared.userIndicatorController, appMediator: AppMediatorMock.default, - appSettings: settings, + appSettings: ServiceLocator.shared.settings, analyticsService: ServiceLocator.shared.analytics) }() @@ -384,7 +382,7 @@ struct TimelineItemBubbledStylerView_Previews: PreviewProvider, TestablePreview .previewDisplayName("Encryption Indicators") pinned .previewDisplayName("Pinned messages") - .snapshotPreferences(delay: 1.0) + .snapshotPreferences(delay: 2.0) } // These always include a reply diff --git a/ElementX/Sources/Services/Client/ClientProxy.swift b/ElementX/Sources/Services/Client/ClientProxy.swift index b06c3e0ce..9a0585444 100644 --- a/ElementX/Sources/Services/Client/ClientProxy.swift +++ b/ElementX/Sources/Services/Client/ClientProxy.swift @@ -816,12 +816,13 @@ class ClientProxy: ClientProxyProtocol { }) } - private lazy var eventFilters: TimelineEventTypeFilter = { + private let eventFilters: TimelineEventTypeFilter = { var stateEventFilters: [StateEventType] = [.roomAliases, .roomCanonicalAlias, .roomGuestAccess, .roomHistoryVisibility, .roomJoinRules, + .roomPinnedEvents, .roomPowerLevels, .roomServerAcl, .roomTombstone, @@ -830,12 +831,6 @@ class ClientProxy: ClientProxyProtocol { .policyRuleRoom, .policyRuleServer, .policyRuleUser] - - // Reminder: once the feature flag is not required anymore, change the lazy var back to a let - if !appSettings.pinningEnabled { - stateEventFilters.append(.roomPinnedEvents) - } - return .exclude(eventTypes: stateEventFilters.map { FilterTimelineEventType.state(eventType: $0) }) }() diff --git a/UnitTests/Sources/RoomScreenViewModelTests.swift b/UnitTests/Sources/RoomScreenViewModelTests.swift index a7998b4dc..ab52a3cd1 100644 --- a/UnitTests/Sources/RoomScreenViewModelTests.swift +++ b/UnitTests/Sources/RoomScreenViewModelTests.swift @@ -23,7 +23,6 @@ class RoomScreenViewModelTests: XCTestCase { } func testPinnedEventsBanner() async throws { - ServiceLocator.shared.settings.pinningEnabled = true let timelineSubject = PassthroughSubject() let updateSubject = PassthroughSubject() let roomProxyMock = JoinedRoomProxyMock(.init()) @@ -101,7 +100,6 @@ class RoomScreenViewModelTests: XCTestCase { } func testPinnedEventsBannerSelection() async throws { - ServiceLocator.shared.settings.pinningEnabled = true let roomProxyMock = JoinedRoomProxyMock(.init()) // setup a way to inject the mock of the pinned events timeline let pinnedTimelineMock = TimelineProxyMock() diff --git a/UnitTests/Sources/TimelineViewModelTests.swift b/UnitTests/Sources/TimelineViewModelTests.swift index 22e63a25a..1243d5291 100644 --- a/UnitTests/Sources/TimelineViewModelTests.swift +++ b/UnitTests/Sources/TimelineViewModelTests.swift @@ -346,9 +346,6 @@ class TimelineViewModelTests: XCTestCase { // MARK: - Pins func testPinnedEvents() async throws { - ServiceLocator.shared.settings.pinningEnabled = true - - // Note: We need to start the test with a non-default value so we know the view model has finished the Task. let roomProxyMock = JoinedRoomProxyMock(.init(name: "", pinnedEventIDs: .init(["test1"]))) let actionsSubject = PassthroughSubject() @@ -378,9 +375,6 @@ class TimelineViewModelTests: XCTestCase { } func testCanUserPinEvents() async throws { - ServiceLocator.shared.settings.pinningEnabled = true - - // Note: We need to start the test with the non-default value so we know the view model has finished the Task. let roomProxyMock = JoinedRoomProxyMock(.init(name: "", canUserPin: true)) let actionsSubject = PassthroughSubject() roomProxyMock.underlyingActionsPublisher = actionsSubject.eraseToAnyPublisher()