From c62fdf96a4218d3db404c22c9a895c375cf9f565 Mon Sep 17 00:00:00 2001 From: Harry <109690906+harry-signal@users.noreply.github.com> Date: Fri, 25 Aug 2023 09:22:16 -0700 Subject: [PATCH] Apply contact deletion when trying to hide a system contact Co-authored-by: Marissa Le Coz --- .../ComposeViewController.swift | 14 +- .../translations/en.lproj/Localizable.strings | 3 + .../src/Contacts/SignalAccountFinder.swift | 50 ++++- .../DeleteSystemContactViewController.swift | 202 +++++++++++++++--- .../RecipientContextMenuHelper.swift | 79 ++++--- 5 files changed, 284 insertions(+), 64 deletions(-) diff --git a/Signal/src/ViewControllers/RecipientPicker/ComposeViewController.swift b/Signal/src/ViewControllers/RecipientPicker/ComposeViewController.swift index ea71f6654a..6f85751db5 100644 --- a/Signal/src/ViewControllers/RecipientPicker/ComposeViewController.swift +++ b/Signal/src/ViewControllers/RecipientPicker/ComposeViewController.swift @@ -125,19 +125,7 @@ extension ComposeViewController: RecipientPickerDelegate { transaction: SDSAnyReadTransaction ) -> String? { switch recipient.identifier { - case .address(let address): - #if DEBUG - let isBlocked = blockingManager.isAddressBlocked(address, transaction: transaction) - owsAssert(!isBlocked, "It should be impossible to see a blocked connection in this view") - - if FeatureFlags.recipientHiding { - let isHidden = DependenciesBridge.shared.recipientHidingManager.isHiddenAddress( - address, - tx: transaction.asV2Read - ) - owsAssert(!isHidden, "It should be impossible to see a hidden recipient in this view") - } - #endif + case .address: return nil case .group(let thread): guard blockingManager.isThreadBlocked(thread, transaction: transaction) else { return nil } diff --git a/Signal/translations/en.lproj/Localizable.strings b/Signal/translations/en.lproj/Localizable.strings index 01378b54df..d28d61933f 100644 --- a/Signal/translations/en.lproj/Localizable.strings +++ b/Signal/translations/en.lproj/Localizable.strings @@ -3250,6 +3250,9 @@ /* A format for the 'unable to remove user' action sheet title. Embeds {{the removed user's name or phone number}}. */ "HIDE_RECIPIENT_IMPASS_BECAUSE_SYSTEM_CONTACT_ACTION_SHEET_TITLE" = "Unable to remove %@"; +/* An explanation of why the user cannot be removed on a linked device. */ +"HIDE_RECIPIENT_IMPOSSIBLE_BECAUSE_SYSTEM_CONTACT_ACTION_SHEET_EXPLANATION" = "This person is saved to your phone’s Contacts. Delete them from your Contacts on your phone and try again."; + /* Label for 'archived conversations' button. */ "HOME_VIEW_ARCHIVED_CONVERSATIONS" = "Archived Chats"; diff --git a/SignalServiceKit/src/Contacts/SignalAccountFinder.swift b/SignalServiceKit/src/Contacts/SignalAccountFinder.swift index 846981be62..1c93b6cba1 100644 --- a/SignalServiceKit/src/Contacts/SignalAccountFinder.swift +++ b/SignalServiceKit/src/Contacts/SignalAccountFinder.swift @@ -11,7 +11,37 @@ public class SignalAccountFinder: NSObject { for address: SignalServiceAddress, tx: SDSAnyReadTransaction ) -> SignalAccount? { - return signalAccounts(for: [address], tx: tx)[0] + if + let serviceId = address.serviceId, + let uuidMatch = signalAccountWhere( + column: SignalAccount.columnName(.recipientServiceId), + matches: serviceId.serviceIdUppercaseString, + tx: tx + ) + { + return uuidMatch + } else if + let phoneNumber = address.phoneNumber, + let phoneNumberMatch = signalAccountWhere( + column: SignalAccount.columnName(.recipientPhoneNumber), + matches: phoneNumber, + tx: tx + ) + { + return phoneNumberMatch + } + return nil + } + + public func signalAccount( + for e164: E164, + tx: SDSAnyReadTransaction + ) -> SignalAccount? { + return signalAccountWhere( + column: SignalAccount.columnName(.recipientPhoneNumber), + matches: e164.stringValue, + tx: tx + ) } func signalAccounts( @@ -101,6 +131,24 @@ public class SignalAccountFinder: NSObject { ) } + private func signalAccountWhere( + column: String, + matches matchString: String, + tx: SDSAnyReadTransaction + ) -> SignalAccount? { + let sql = "SELECT * FROM \(SignalAccount.databaseTableName) WHERE \(column) = ? LIMIT 1" + + /// Why did we use `allSignalAccounts` instead of `SignalAccount.anyFetchAll`? + /// The reason is that the `SignalAccountReadCache` needs to have + /// `didReadSignalAccount` called on it for each record we enumerate, and + /// `SignalAccount.anyEnumerate` has this built in. + return allSignalAccounts( + tx: tx, + sql: sql, + arguments: [matchString] + ).first + } + private func allSignalAccounts( tx: SDSAnyReadTransaction, sql: String, diff --git a/SignalUI/Context Menus/DeleteSystemContactViewController.swift b/SignalUI/Context Menus/DeleteSystemContactViewController.swift index eda99e5a96..7ff6952599 100644 --- a/SignalUI/Context Menus/DeleteSystemContactViewController.swift +++ b/SignalUI/Context Menus/DeleteSystemContactViewController.swift @@ -3,8 +3,15 @@ // SPDX-License-Identifier: AGPL-3.0-only // +import Contacts +import LibSignalClient import SignalMessaging +/// If we try and hide a recipient but fail because they correspond to +/// a system contact, we show this controller which provides a hook +/// to delete the system contact (which, if successful, then triggers a hide). +/// +/// SHOULD ONLY BE DISPLAYED ON THE PRIMARY DEVICE class DeleteSystemContactViewController: OWSTableViewController2 { /// Dependencies needed by this view controller. /// Note that these dependencies can be accessed @@ -14,10 +21,13 @@ class DeleteSystemContactViewController: OWSTableViewController2 { private struct Dependencies { let contactsManager: ContactsManagerProtocol let databaseStorage: SDSDatabaseStorage + let recipientHidingManager: RecipientHidingManager + let tsAccountManager: TSAccountManager } - /// The address of the contact represented by this contact card. - private let address: SignalServiceAddress + /// The e164 of the contact represented by this contact card. + private let e164: E164 + private let serviceId: ServiceId? /// The view controller that should present the toast /// confirming successful contact deletion. Note that @@ -26,22 +36,46 @@ class DeleteSystemContactViewController: OWSTableViewController2 { private let viewControllerPresentingToast: UIViewController init( - address: SignalServiceAddress, + e164: E164, + serviceId: ServiceId?, viewControllerPresentingToast: UIViewController, contactsManager: ContactsManagerProtocol, - databaseStorage: SDSDatabaseStorage + databaseStorage: SDSDatabaseStorage, + recipientHidingManager: RecipientHidingManager, + tsAccountManager: TSAccountManager ) { - self.address = address + self.e164 = e164 + self.serviceId = serviceId self.viewControllerPresentingToast = viewControllerPresentingToast self.dependencies = Dependencies( contactsManager: contactsManager, - databaseStorage: databaseStorage + databaseStorage: databaseStorage, + recipientHidingManager: recipientHidingManager, + tsAccountManager: tsAccountManager ) super.init() } + private lazy var spinnerContainer: UIView = { + let view = UIView() + self.view.addSubview(view) + view.autoPinEdgesToSuperviewEdges() + view.backgroundColor = .black.withAlphaComponent(0.15) + view.addSubview(spinnerView) + spinnerView.autoCenterInSuperview() + view.isHidden = true + return view + }() + + private lazy var spinnerView = UIActivityIndicatorView() + override func viewDidLoad() { super.viewDidLoad() + + // This screen is for primary devices only. If a non primary + // manages to get here bad things could happen. + owsAssert(tsAccountManager.isPrimaryDevice) + self.navigationItem.leftBarButtonItem = UIBarButtonItem( barButtonSystemItem: .cancel, target: self, @@ -115,24 +149,38 @@ class DeleteSystemContactViewController: OWSTableViewController2 { private func updateTableContents() { let contents = OWSTableContents() - let (image, nameComponents, phoneNumber) = dependencies.databaseStorage.read { tx in + let addressForProfileLookup = SignalServiceAddress(serviceId: serviceId, e164: e164) + let ( + image, + nameComponents, + displayNameForToast, + phoneNumber + ) = dependencies.databaseStorage.read { tx in let image = avatarBuilder.avatarImage( - forAddress: address, + forAddress: addressForProfileLookup, diameterPixels: Constants.avatarDiameter, localUserDisplayMode: .asUser, transaction: tx ) let nameComponents = dependencies.contactsManager.nameComponents( - for: address, + for: addressForProfileLookup, transaction: tx ) - + let displayNameForToast = dependencies.contactsManager.displayName( + for: addressForProfileLookup, + transaction: tx + ).formattedForActionSheetTitle() let phoneNumber = SignalRecipient.fetchRecipient( - for: address, + for: addressForProfileLookup, onlyIfRegistered: false, tx: tx )?.phoneNumber - return (image, nameComponents, phoneNumber) + return ( + image, + nameComponents, + displayNameForToast, + phoneNumber + ) } // Avatar @@ -194,7 +242,7 @@ class DeleteSystemContactViewController: OWSTableViewController2 { }, actionBlock: { [weak self] in guard let self else { return } - self.displayDeleteContactActionSheet() + self.displayDeleteContactActionSheet(phoneNumber: phoneNumber, displayNameForToast: displayNameForToast) } ) ) @@ -205,7 +253,7 @@ class DeleteSystemContactViewController: OWSTableViewController2 { /// Displays the action sheet confirming that the user really /// wants to delete this contact from their system contacts. - private func displayDeleteContactActionSheet() { + private func displayDeleteContactActionSheet(phoneNumber: String?, displayNameForToast: String) { let actionSheet = ActionSheetController( title: OWSLocalizedString( "DELETE_CONTACT_ACTION_SHEET_TITLE", @@ -224,10 +272,7 @@ class DeleteSystemContactViewController: OWSTableViewController2 { ), style: .destructive, handler: { [weak self] _ in - guard let self else { return } - // TODO: Delete contact and hide recipient. Ensure cell disappears when we return to the recipient picker list. - self.dismiss(animated: true) - self.displayDeletedContactToast() + self?.handleContactDelete(displayNameForToast: displayNameForToast) } )) actionSheet.addAction(ActionSheetAction( @@ -237,21 +282,126 @@ class DeleteSystemContactViewController: OWSTableViewController2 { self.presentActionSheet(actionSheet) } + private func handleContactDelete(displayNameForToast: String) { + switch CNContactStore.authorizationStatus(for: .contacts) { + case .authorized: + break + case .notDetermined, .denied, .restricted: + fallthrough + @unknown default: + Logger.info("No contact permissions") + showGenericErrorToastAndDismiss() + } + + let signalAccount = self.dependencies.databaseStorage.read { tx in + return SignalAccountFinder().signalAccount( + for: self.e164, + tx: tx + ) + } + // In the case where we have more than one contact with the e164, + // prefer the one with this id. Otherwise, choice is arbitrary. + let preferredContactIdForDeletion = signalAccount?.contact?.cnContactId + + // Go to CNContacts as the source of truth for contacts. + let contactStore = CNContactStore() + let phoneNumPredicate = CNContact.predicateForContacts(matching: CNPhoneNumber(stringValue: e164.stringValue)) + guard + let contacts = try? contactStore.unifiedContacts( + matching: phoneNumPredicate, + keysToFetch: [CNContactIdentifierKey as CNKeyDescriptor] + ) + else { + Logger.error("Failed to fetch CNContacts!") + showGenericErrorToastAndDismiss() + return + } + var contactToDelete: CNContact? + if let preferredContactIdForDeletion { + contactToDelete = contacts.first(where: { $0.identifier == preferredContactIdForDeletion }) + } + if contactToDelete == nil { + contactToDelete = contacts.first + } + + guard let contactToDelete = contactToDelete?.mutableCopy() as? CNMutableContact else { + // No contact to delete! Done. + Logger.warn("No contact to delete, exiting early.") + showGenericErrorToastAndDismiss() + return + } + + var didFail = false + // Set up the observer _before_ deleting, so its around + // when the deletion happens. + NotificationCenter.default.observe(once: .OWSContactsManagerSignalAccountsDidChange) + .asVoid() + .timeout(on: DispatchQueue.main, seconds: 5, substituteValue: ()) + .observe(on: DispatchQueue.main) { [weak self] _ in + guard let self, !didFail else { return } + + defer { + self.dismiss(animated: true) + self.displayDeletedContactToast(displayNameForToast: displayNameForToast) + } + + // Check that the contact got deleted from our db. + let isStillSystemContact = self.dependencies.databaseStorage.read { tx in + return self.contactsManager.isSystemContact(phoneNumber: self.e164.stringValue, transaction: tx) + } + guard !isStillSystemContact else { + // Can't hide; likely there was another contact with the same number. + // Just exit. + Logger.warn("Address still a system contact after deletion; possibly duplicate system contact") + return + } + self.dependencies.databaseStorage.write { tx in + do { + try self.dependencies.recipientHidingManager.addHiddenRecipient( + SignalServiceAddress(serviceId: self.serviceId, e164: self.e164), + wasLocallyInitiated: true, + tx: tx.asV2Write + ) + } catch { + owsFailDebug("Failed to hide recipient") + } + } + } + + // Delete + let saveRequest = CNSaveRequest() + saveRequest.delete(contactToDelete) + do { + try contactStore.execute(saveRequest) + } catch { + didFail = true + Logger.error("Failed to delete CNContact!") + showGenericErrorToastAndDismiss() + } + + spinnerContainer.isHidden = false + spinnerView.startAnimating() + self.view.isUserInteractionEnabled = false + } + + private func showGenericErrorToastAndDismiss() { + ToastController(text: CommonStrings.somethingWentWrongError).presentToastView( + from: .bottom, + of: self.viewControllerPresentingToast.view, + inset: self.view.safeAreaInsets.bottom + Constants.toastInset + ) + self.dismiss(animated: true) + } + /// Displays a toast confirming that a contact was /// successfully deleted. - private func displayDeletedContactToast() { - let displayName = dependencies.databaseStorage.read { tx in - return self.dependencies.contactsManager.displayName( - for: self.address, - transaction: tx - ).formattedForActionSheetTitle() - } + private func displayDeletedContactToast(displayNameForToast: String) { let toastMessage = String( format: OWSLocalizedString( "DELETE_CONTACT_CONFIRMATION_TOAST", comment: "Toast message confirming the system contact was deleted. Embeds {{The name of the user who was deleted.}}." ), - displayName + displayNameForToast ) ToastController(text: toastMessage).presentToastView( from: .bottom, diff --git a/SignalUI/Context Menus/RecipientContextMenuHelper.swift b/SignalUI/Context Menus/RecipientContextMenuHelper.swift index 3f02c5e258..e8ea136991 100644 --- a/SignalUI/Context Menus/RecipientContextMenuHelper.swift +++ b/SignalUI/Context Menus/RecipientContextMenuHelper.swift @@ -3,6 +3,7 @@ // SPDX-License-Identifier: AGPL-3.0-only // +import LibSignalClient import SignalMessaging import SignalServiceKit @@ -86,9 +87,10 @@ class RecipientContextMenuHelper { image: UIImage(named: "minus-circle") ) { [weak self] _ in guard let self else { return } - if self.isSystemContact(address: address) { + if let e164 = address.e164, self.isSystemContact(e164: e164) { self.displayViewContactActionSheet( address: address, + e164: e164, fromViewController: fromViewController ) } else { @@ -100,10 +102,10 @@ class RecipientContextMenuHelper { } } - /// Whether the given `address` corresponds with a system contact. - private func isSystemContact(address: SignalServiceAddress) -> Bool { + /// Whether the given `e164` corresponds with a system contact. + private func isSystemContact(e164: E164) -> Bool { return databaseStorage.read { tx in - contactsManager.isSystemContact(address: address, transaction: tx) + contactsManager.isSystemContact(phoneNumber: e164.stringValue, transaction: tx) } } @@ -232,22 +234,28 @@ class RecipientContextMenuHelper { /// first removing them from their system contacts. /// /// - Parameter address: Address of the recipient to hide. + /// - Parameter e164: Phone number of the recipient to hide. /// - Parameter fromViewController: The view controller from which to present the action sheet. private func displayViewContactActionSheet( address: SignalServiceAddress, + e164: E164, fromViewController: UIViewController ) { - guard address.isValid else { - owsFailDebug("Invalid address: \(address).") - return - } - let (localAddress, recipientDisplayName) = databaseStorage.read { tx in + let ( + isPrimaryDevice, + localAddress, + recipientDisplayName + ) = databaseStorage.read { tx in let localAddress = accountManager.localAddress(with: tx) let recipientDisplayName = contactsManager.displayName( for: address, transaction: tx ).formattedForActionSheetTitle() - return (localAddress, recipientDisplayName) + return ( + accountManager.isPrimaryDevice(transaction: tx), + localAddress, + recipientDisplayName + ) } guard let localAddress, @@ -264,21 +272,40 @@ class RecipientContextMenuHelper { recipientDisplayName ) - let actionSheet = ActionSheetController( - title: actionSheetTitle, - message: OWSLocalizedString( + let actionSheetMessage: String + let removeAction: ActionSheetAction? + if isPrimaryDevice { + actionSheetMessage = OWSLocalizedString( "HIDE_RECIPIENT_IMPASS_BECAUSE_SYSTEM_CONTACT_ACTION_SHEET_EXPLANATION", comment: "An explanation of why the user cannot be removed." ) + removeAction = ActionSheetAction( + title: OWSLocalizedString("VIEW_CONTACT_BUTTON", comment: "Button label for the 'View Contact' button"), + handler: { [weak self] _ in + guard let self else { return } + self.displayDeleteContactViewController( + e164: e164, + serviceId: address.serviceId, + fromViewController: fromViewController + ) + } + ) + } else { + actionSheetMessage = OWSLocalizedString( + "HIDE_RECIPIENT_IMPOSSIBLE_BECAUSE_SYSTEM_CONTACT_ACTION_SHEET_EXPLANATION", + comment: "An explanation of why the user cannot be removed on a linked device." + ) + removeAction = nil + } + + let actionSheet = ActionSheetController( + title: actionSheetTitle, + message: actionSheetMessage ) - actionSheet.addAction(ActionSheetAction( - title: OWSLocalizedString("VIEW_CONTACT_BUTTON", comment: "Button label for the 'View Contact' button"), - handler: { [weak self] _ in - guard let self else { return } - self.displayDeleteContactViewController(address: address, fromViewController: fromViewController) - } - )) + if let removeAction { + actionSheet.addAction(removeAction) + } actionSheet.addAction(ActionSheetAction( title: CommonStrings.okayButton, style: .cancel @@ -289,20 +316,24 @@ class RecipientContextMenuHelper { /// Displays a view controller with a simplified contact /// view and the option to delete this contact. /// - /// - Parameter address: The address of the contact to + /// - Parameter e164: The phone number of the contact to /// potentially be deleted. /// - Parameter fromViewController: The view controller /// from which to present this contact deletion view /// controller. private func displayDeleteContactViewController( - address: SignalServiceAddress, + e164: E164, + serviceId: ServiceId?, fromViewController: UIViewController ) { let deleteContactViewController = DeleteSystemContactViewController( - address: address, + e164: e164, + serviceId: serviceId, viewControllerPresentingToast: fromViewController, contactsManager: contactsManager, - databaseStorage: databaseStorage + databaseStorage: databaseStorage, + recipientHidingManager: recipientHidingManager, + tsAccountManager: accountManager ) let navigationController = OWSNavigationController() navigationController.setViewControllers([deleteContactViewController], animated: false)