diff --git a/Signal/src/ViewControllers/AddContactShareToExistingContactViewController.swift b/Signal/src/ViewControllers/AddContactShareToExistingContactViewController.swift index c53eba4e6..327de07ab 100644 --- a/Signal/src/ViewControllers/AddContactShareToExistingContactViewController.swift +++ b/Signal/src/ViewControllers/AddContactShareToExistingContactViewController.swift @@ -8,8 +8,12 @@ import ContactsUI class AddContactShareToExistingContactViewController: ContactsPicker, ContactsPickerDelegate, CNContactViewControllerDelegate { - // TODO actual protocol? - weak var addToExistingContactDelegate: UIViewController? + // TODO - there are some hard coded assumptions in this VC that assume we are *pushed* onto a + // navigation controller. That seems fine for now, but if we need to be presented as a modal, + // or need to notify our presenter about our dismisall or other contact actions, a delegate + // would be helpful. It seems like this would require some broad changes to the ContactShareViewHelper, + // so I've left it as is for now, since it happens to work. + // weak var addToExistingContactDelegate: AddContactShareToExistingContactViewControllerDelegate? let contactShare: ContactShareViewModel @@ -55,8 +59,7 @@ class AddContactShareToExistingContactViewController: ContactsPicker, ContactsPi Logger.debug("\(self.logTag) in \(#function)") guard let mergedContact: CNContact = self.contactShare.cnContact(mergedWithExistingContact: contact) else { - // TODO maybe this should not be optional and return a blank contact so we can still save the (not-actually merged) contact - owsFail("\(logTag) in \(#function) navigationController was unexpectedly nil") + owsFail("\(logTag) in \(#function) mergedContact was unexpectedly nil") return } @@ -104,7 +107,16 @@ class AddContactShareToExistingContactViewController: ContactsPicker, ContactsPi return } - // We want to pop *this* view and the still presented CNContactViewController in a single animation. + // TODO this is weird - ideally we'd do something like + // self.delegate?.didFinishAddingContact + // and the delegate, which knows about our presentation context could do the right thing. + // + // As it is, we happen to always be *pushing* this view controller onto a navcontroller, so the + // following works in all current cases. + // + // If we ever wanted to do something different, like present this in a modal, we'd have to rethink. + + // We want to pop *this* view *and* the still presented CNContactViewController in a single animation. // Note this happens for *cancel* and for *done*. Unfortunately, I don't know of a way to detect the difference // between the two, since both just call this method. guard let myIndex = navigationController.viewControllers.index(of: self) else { diff --git a/SignalMessaging/ViewModels/ContactShareViewModel.swift b/SignalMessaging/ViewModels/ContactShareViewModel.swift index 210c84117..8e2a744cd 100644 --- a/SignalMessaging/ViewModels/ContactShareViewModel.swift +++ b/SignalMessaging/ViewModels/ContactShareViewModel.swift @@ -111,61 +111,12 @@ public class ContactShareViewModel: NSObject { public func cnContact(mergedWithExistingContact existingContact: Contact) -> CNContact? { - guard let mergedCNContact: CNMutableContact = existingContact.cnContact?.mutableCopy() as? CNMutableContact else { - owsFail("\(logTag) in \(#function) mergedCNContact was unexpectedly nil") - return nil - } - guard let newCNContact = OWSContacts.systemContact(for: self.dbRecord) else { owsFail("\(logTag) in \(#function) newCNContact was unexpectedly nil") return nil } - let normalize = { (input: String) -> String in - return (input as NSString).ows_stripped() - } - - // TODO display name - but only if existing is blank - - var existingPhoneNumberSet: Set = Set() - mergedCNContact.phoneNumbers.map { $0.value }.forEach { (existingPhoneNumber: CNPhoneNumber) in - // compare e164? - let normalizedValue = normalize(existingPhoneNumber.stringValue as String) - existingPhoneNumberSet.insert(normalizedValue) - } - - newCNContact.phoneNumbers.forEach { (newLabeledPhoneNumber: CNLabeledValue) in - let normalizedValue = normalize(newLabeledPhoneNumber.value.stringValue as String) - guard !existingPhoneNumberSet.contains(normalizedValue) else { - Logger.debug("\(self.logTag) in \(#function) ignoring matching phone number: \(normalizedValue)") - return - } - - Logger.debug("\(self.logTag) in \(#function) adding new phone number: \(normalizedValue)") - mergedCNContact.phoneNumbers.append(newLabeledPhoneNumber) - } - -// // TODO emails -// var existingEmailSet: Set = Set() -// mergedCNContact.emailAddresses.forEach { (existingEmail: CNLabeledValue) in -// existingEmailSet.insert(normalize(existingEmail.value as String)) -// } - - // TODO address - -// newCNContact.emailAddresses.forEach { newEmail in -// let match = mergedCNContact.emailAddresses.first { existingEmail in -// return normalize(existingEmail) == normalize(newEmail) -// } -// if match == nil { -// mergedCNContact.emailAddresses.add -// } -// } -// -// -// - - return mergedCNContact.copy() as? CNContact + return existingContact.buildCNContact(mergedWithNewContact: newCNContact) } public func copy(withNamePrefix namePrefix: String?, diff --git a/SignalServiceKit/src/Contacts/Contact.h b/SignalServiceKit/src/Contacts/Contact.h index f0205c48d..20ce68c66 100644 --- a/SignalServiceKit/src/Contacts/Contact.h +++ b/SignalServiceKit/src/Contacts/Contact.h @@ -7,10 +7,7 @@ NS_ASSUME_NONNULL_BEGIN /** - * - * Contact represents relevant information related to a contact from the user's - * contact list. - * + * An adapter for the system contacts */ @class CNContact; @@ -49,6 +46,9 @@ NS_ASSUME_NONNULL_BEGIN #endif // TARGET_OS_IOS + (NSComparator)comparatorSortingNamesByFirstThenLast:(BOOL)firstNameOrdering; ++ (NSString *)formattedFullNameWithCNContact:(CNContact *)cnContact NS_SWIFT_NAME(formattedFullName(cnContact:)); + +- (CNContact *)buildCNContactMergedWithNewContact:(CNContact *)newCNContact NS_SWIFT_NAME(buildCNContact(mergedWithNewContact:)); @end diff --git a/SignalServiceKit/src/Contacts/Contact.m b/SignalServiceKit/src/Contacts/Contact.m index 74fd195dc..01055d81b 100644 --- a/SignalServiceKit/src/Contacts/Contact.m +++ b/SignalServiceKit/src/Contacts/Contact.m @@ -4,6 +4,7 @@ #import "Contact.h" #import "Cryptography.h" +#import "NSString+SSK.h" #import "OWSPrimaryStorage.h" #import "PhoneNumber.h" #import "SignalRecipient.h" @@ -38,9 +39,9 @@ NS_ASSUME_NONNULL_BEGIN } _cnContact = contact; - _firstName = [self trimName:contact.givenName]; - _lastName = [self trimName:contact.familyName]; - _fullName = [CNContactFormatter stringFromContact:contact style:CNContactFormatterStyleFullName]; + _firstName = contact.givenName.ows_stripped; + _lastName = contact.familyName.ows_stripped; + _fullName = [Contact formattedFullNameWithCNContact:contact]; _uniqueId = contact.identifier; NSMutableArray *phoneNumbers = [NSMutableArray new]; @@ -125,11 +126,6 @@ NS_ASSUME_NONNULL_BEGIN return _image; } -- (NSString *)trimName:(NSString *)name -{ - return [name stringByTrimmingCharactersInSet:[NSCharacterSet whitespaceAndNewlineCharacterSet]]; -} - + (MTLPropertyStorage)storageBehaviorForPropertyWithKey:(NSString *)propertyKey { if ([propertyKey isEqualToString:@"cnContact"] || [propertyKey isEqualToString:@"image"]) { @@ -250,6 +246,11 @@ NS_ASSUME_NONNULL_BEGIN }; } ++ (NSString *)formattedFullNameWithCNContact:(CNContact *)cnContact +{ + return [CNContactFormatter stringFromContact:cnContact style:CNContactFormatterStyleFullName].ows_stripped; +} + - (NSString *)nameForPhoneNumber:(NSString *)recipientId { OWSAssert(recipientId.length > 0); @@ -290,6 +291,62 @@ NS_ASSUME_NONNULL_BEGIN return hash; } +- (CNContact *)buildCNContactMergedWithNewContact:(CNContact *)newCNContact +{ + CNMutableContact *_Nullable mergedCNContact = [self.cnContact mutableCopy]; + if (!mergedCNContact) { + OWSFail(@"%@ in %s mergedCNContact was unexpectedly nil", self.logTag, __PRETTY_FUNCTION__); + return [CNContact new]; + } + + // Name + NSString *formattedFullName = [self.class formattedFullNameWithCNContact:mergedCNContact]; + + // merged all or nothing - do not try to piece-meal merge. + if (formattedFullName.length == 0) { + mergedCNContact.namePrefix = newCNContact.namePrefix.ows_stripped; + mergedCNContact.givenName = newCNContact.givenName.ows_stripped; + mergedCNContact.middleName = newCNContact.middleName.ows_stripped; + mergedCNContact.familyName = newCNContact.familyName.ows_stripped; + mergedCNContact.nameSuffix = newCNContact.nameSuffix.ows_stripped; + } + + // Phone Numbers + NSSet *existingPhoneNumberSet = [NSSet setWithArray:self.parsedPhoneNumbers]; + + NSMutableArray *> *mergedPhoneNumbers = [mergedCNContact.phoneNumbers mutableCopy]; + for (CNLabeledValue *labeledPhoneNumber in newCNContact.phoneNumbers) { + PhoneNumber *_Nullable parsedPhoneNumber = [PhoneNumber tryParsePhoneNumberFromUserSpecifiedText:labeledPhoneNumber.value.stringValue]; + if (parsedPhoneNumber && ![existingPhoneNumberSet containsObject:parsedPhoneNumber]) { + [mergedPhoneNumbers addObject:labeledPhoneNumber]; + } + } + mergedCNContact.phoneNumbers = mergedPhoneNumbers; + + // Emails + NSSet *existingEmailSet = [NSSet setWithArray:self.emails]; + NSMutableArray *> *mergedEmailAddresses = [mergedCNContact.emailAddresses mutableCopy]; + for (CNLabeledValue *labeledEmail in newCNContact.emailAddresses) { + NSString *normalizedValue = labeledEmail.value.ows_stripped; + if (![existingEmailSet containsObject:normalizedValue]) { + [mergedEmailAddresses addObject:labeledEmail]; + } + } + mergedCNContact.emailAddresses = mergedEmailAddresses; + + // Address + // merged all or nothing - do not try to piece-meal merge. + BOOL hasExistingAddress = NO; + for (CNLabeledValue *labeledPostalAddress in mergedCNContact.postalAddresses) { + hasExistingAddress = YES; + } + if (!hasExistingAddress) { + mergedCNContact.postalAddresses = newCNContact.postalAddresses; + } + + return [mergedCNContact copy]; +} + @end NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Messages/Interactions/OWSContact.m b/SignalServiceKit/src/Messages/Interactions/OWSContact.m index 6131fda32..bebb3aef2 100644 --- a/SignalServiceKit/src/Messages/Interactions/OWSContact.m +++ b/SignalServiceKit/src/Messages/Interactions/OWSContact.m @@ -3,6 +3,7 @@ // #import "OWSContact.h" +#import "Contact.h" #import "MimeTypeUtil.h" #import "NSString+SSK.h" #import "OWSContact+Private.h" @@ -371,8 +372,9 @@ NSString *NSStringForContactAddressType(OWSContactAddressType value) - (void)ensureDisplayName { if (_displayName.length < 1) { - CNContact *_Nullable systemContact = [OWSContacts systemContactForContact:self]; - _displayName = [CNContactFormatter stringFromContact:systemContact style:CNContactFormatterStyleFullName]; + + CNContact *_Nullable cnContact = [OWSContacts systemContactForContact:self]; + _displayName = [Contact formattedFullNameWithCNContact:cnContact]; } if (_displayName.length < 1) { // Fall back to using the organization name.