Apply contact deletion when trying to hide a system contact

Co-authored-by: Marissa Le Coz <marissa@signal.org>

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 }

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";
 

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,

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,

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)