From dcb7eef3fcb7cffaa6ee33475330df3cec8764d5 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 4 Apr 2017 10:19:47 -0400 Subject: [PATCH] Respond to CR. // FREEBIE --- .../AddToBlockListViewController.m | 11 +++- Signal/src/ViewControllers/BlockListUIUtils.h | 6 +- Signal/src/ViewControllers/BlockListUIUtils.m | 26 +++++---- .../ViewControllers/BlockListViewController.m | 34 +---------- ...SConversationSettingsTableViewController.m | 4 +- Signal/src/contact/OWSContactsManager.h | 1 + Signal/src/contact/OWSContactsManager.m | 58 ++++++++++++++++--- 7 files changed, 84 insertions(+), 56 deletions(-) diff --git a/Signal/src/ViewControllers/AddToBlockListViewController.m b/Signal/src/ViewControllers/AddToBlockListViewController.m index b55727743..557cff5bb 100644 --- a/Signal/src/ViewControllers/AddToBlockListViewController.m +++ b/Signal/src/ViewControllers/AddToBlockListViewController.m @@ -276,13 +276,14 @@ NSString *const kContactsTable_CellReuseIdentifier = @"kContactsTable_CellReuseI __weak AddToBlockListViewController *weakSelf = self; [BlockListUIUtils showBlockPhoneNumberActionSheet:[parsedPhoneNumber toE164] - displayName:[parsedPhoneNumber toE164] fromViewController:self blockingManager:_blockingManager + contactsManager:_contactsManager completionBlock:^(BOOL isBlocked) { if (isBlocked) { // Clear phone number text field is block succeeds. weakSelf.phoneNumberTextField.text = nil; + [weakSelf.navigationController popViewControllerAnimated:YES]; } }]; } @@ -409,11 +410,17 @@ NSString *const kContactsTable_CellReuseIdentifier = @"kContactsTable_CellReuseI { [tableView deselectRowAtIndexPath:indexPath animated:YES]; + __weak AddToBlockListViewController *weakSelf = self; Contact *contact = self.contacts[(NSUInteger)indexPath.item]; [BlockListUIUtils showBlockContactActionSheet:contact fromViewController:self blockingManager:_blockingManager - completionBlock:nil]; + contactsManager:_contactsManager + completionBlock:^(BOOL isBlocked) { + if (isBlocked) { + [weakSelf.navigationController popViewControllerAnimated:YES]; + } + }]; } #pragma mark - UIScrollViewDelegate diff --git a/Signal/src/ViewControllers/BlockListUIUtils.h b/Signal/src/ViewControllers/BlockListUIUtils.h index 23fb34975..c26ca1c61 100644 --- a/Signal/src/ViewControllers/BlockListUIUtils.h +++ b/Signal/src/ViewControllers/BlockListUIUtils.h @@ -6,6 +6,7 @@ @class Contact; @class OWSBlockingManager; +@class OWSContactsManager; typedef void (^BlockActionCompletionBlock)(BOOL isBlocked); @@ -16,18 +17,19 @@ typedef void (^BlockActionCompletionBlock)(BOOL isBlocked); + (void)showBlockContactActionSheet:(Contact *)contact fromViewController:(UIViewController *)fromViewController blockingManager:(OWSBlockingManager *)blockingManager + contactsManager:(OWSContactsManager *)contactsManager completionBlock:(BlockActionCompletionBlock)completionBlock; + (void)showBlockPhoneNumberActionSheet:(NSString *)phoneNumber - displayName:(NSString *)displayName fromViewController:(UIViewController *)fromViewController blockingManager:(OWSBlockingManager *)blockingManager + contactsManager:(OWSContactsManager *)contactsManager completionBlock:(BlockActionCompletionBlock)completionBlock; + (void)showUnblockPhoneNumberActionSheet:(NSString *)phoneNumber - displayName:(NSString *)displayName fromViewController:(UIViewController *)fromViewController blockingManager:(OWSBlockingManager *)blockingManager + contactsManager:(OWSContactsManager *)contactsManager completionBlock:(BlockActionCompletionBlock)completionBlock; @end diff --git a/Signal/src/ViewControllers/BlockListUIUtils.m b/Signal/src/ViewControllers/BlockListUIUtils.m index 713cb0136..1200aa0df 100644 --- a/Signal/src/ViewControllers/BlockListUIUtils.m +++ b/Signal/src/ViewControllers/BlockListUIUtils.m @@ -3,6 +3,7 @@ // #import "BlockListUIUtils.h" +#import "OWSContactsManager.h" #import "PhoneNumber.h" #import #import @@ -14,6 +15,7 @@ NS_ASSUME_NONNULL_BEGIN + (void)showBlockContactActionSheet:(Contact *)contact fromViewController:(UIViewController *)fromViewController blockingManager:(OWSBlockingManager *)blockingManager + contactsManager:(OWSContactsManager *)contactsManager completionBlock:(BlockActionCompletionBlock)completionBlock { NSMutableArray *phoneNumbers = [NSMutableArray new]; @@ -29,19 +31,21 @@ NS_ASSUME_NONNULL_BEGIN } return; } + NSString *displayName = [contactsManager displayNameForContact:contact]; [self showBlockPhoneNumbersActionSheet:phoneNumbers - displayName:contact.fullName + displayName:displayName fromViewController:fromViewController blockingManager:blockingManager completionBlock:completionBlock]; } + (void)showBlockPhoneNumberActionSheet:(NSString *)phoneNumber - displayName:(NSString *)displayName fromViewController:(UIViewController *)fromViewController blockingManager:(OWSBlockingManager *)blockingManager + contactsManager:(OWSContactsManager *)contactsManager completionBlock:(BlockActionCompletionBlock)completionBlock { + NSString *displayName = [contactsManager displayNameForPhoneIdentifier:phoneNumber]; [self showBlockPhoneNumbersActionSheet:@[ phoneNumber ] displayName:displayName fromViewController:fromViewController @@ -61,7 +65,8 @@ NS_ASSUME_NONNULL_BEGIN OWSAssert(blockingManager); NSString *title = [NSString stringWithFormat:NSLocalizedString(@"BLOCK_LIST_BLOCK_TITLE_FORMAT", - @"A format for the 'block user' action sheet title."), + @"A format for the 'block user' action sheet title. Embeds {{the " + @"blocked user's name or phone number}}."), displayName]; UIAlertController *actionSheetController = @@ -69,7 +74,7 @@ NS_ASSUME_NONNULL_BEGIN UIAlertAction *unblockAction = [UIAlertAction actionWithTitle:NSLocalizedString(@"BLOCK_LIST_BLOCK_BUTTON", @"Button label for the 'block' button") - style:UIAlertActionStyleDefault + style:UIAlertActionStyleDestructive handler:^(UIAlertAction *_Nonnull action) { [self blockPhoneNumbers:phoneNumbers displayName:displayName @@ -119,18 +124,20 @@ NS_ASSUME_NONNULL_BEGIN } + (void)showUnblockPhoneNumberActionSheet:(NSString *)phoneNumber - displayName:(NSString *)displayName fromViewController:(UIViewController *)fromViewController blockingManager:(OWSBlockingManager *)blockingManager + contactsManager:(OWSContactsManager *)contactsManager completionBlock:(BlockActionCompletionBlock)completionBlock { OWSAssert(phoneNumber.length > 0); - OWSAssert(displayName.length > 0); OWSAssert(fromViewController); OWSAssert(blockingManager); + NSString *displayName = [contactsManager displayNameForPhoneIdentifier:phoneNumber]; + NSString *title = [NSString stringWithFormat:NSLocalizedString(@"BLOCK_LIST_UNBLOCK_TITLE_FORMAT", - @"A format for the 'unblock user' action sheet title."), + @"A format for the 'unblock user' action sheet title. Embeds " + @"{{the blocked user's name or phone number}}."), displayName]; UIAlertController *actionSheetController = @@ -138,7 +145,7 @@ NS_ASSUME_NONNULL_BEGIN UIAlertAction *unblockAction = [UIAlertAction actionWithTitle:NSLocalizedString(@"BLOCK_LIST_UNBLOCK_BUTTON", @"Button label for the 'unblock' button") - style:UIAlertActionStyleDefault + style:UIAlertActionStyleDestructive handler:^(UIAlertAction *_Nonnull action) { [BlockListUIUtils unblockPhoneNumber:phoneNumber displayName:displayName @@ -179,8 +186,7 @@ NS_ASSUME_NONNULL_BEGIN message:[NSString stringWithFormat:NSLocalizedString(@"BLOCK_LIST_VIEW_UNBLOCKED_ALERT_MESSAGE_FORMAT", @"The message format of the 'user unblocked' " - @"alert. It is populated with the " - @"blocked phone number."), + @"alert. Embeds {{the blocked user's name or phone number}}."), displayName] fromViewController:fromViewController]; } diff --git a/Signal/src/ViewControllers/BlockListViewController.m b/Signal/src/ViewControllers/BlockListViewController.m index 8732e0936..fd43f0e35 100644 --- a/Signal/src/ViewControllers/BlockListViewController.m +++ b/Signal/src/ViewControllers/BlockListViewController.m @@ -20,7 +20,6 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, readonly) OWSContactsManager *contactsManager; @property (nonatomic) NSArray *contacts; -@property (nonatomic) NSDictionary *contactMap; @end @@ -138,18 +137,7 @@ typedef NS_ENUM(NSInteger, BlockListViewControllerSection) { - (NSString *)displayNameForIndexPath:(NSIndexPath *)indexPath { NSString *phoneNumber = _blockedPhoneNumbers[(NSUInteger)indexPath.item]; - PhoneNumber *parsedPhoneNumber = [PhoneNumber tryParsePhoneNumberFromUserSpecifiedText:phoneNumber]; - - // Try to parse and present the phone number in E164. - // It should already be in E164, so this should always work. - // If an invalid or unparsable phone number is already in the block list, - // present it as-is. - NSString *displayName = (parsedPhoneNumber ? parsedPhoneNumber.toE164 : phoneNumber); - Contact *contact = self.contactMap[displayName]; - if (contact && [contact fullName].length > 0) { - displayName = [contact fullName]; - } - + NSString *displayName = [_contactsManager displayNameForPhoneIdentifier:phoneNumber]; return displayName; } @@ -168,11 +156,11 @@ typedef NS_ENUM(NSInteger, BlockListViewControllerSection) { } case BlockListViewControllerSection_BlockList: { NSString *phoneNumber = _blockedPhoneNumbers[(NSUInteger)indexPath.item]; - NSString *displayName = [self displayNameForIndexPath:indexPath]; [BlockListUIUtils showUnblockPhoneNumberActionSheet:phoneNumber - displayName:displayName fromViewController:self blockingManager:_blockingManager + + contactsManager:_contactsManager completionBlock:nil]; break; } @@ -203,22 +191,6 @@ typedef NS_ENUM(NSInteger, BlockListViewControllerSection) { }); } -- (void)setContacts:(NSArray *)contacts -{ - _contacts = contacts; - - NSMutableDictionary *contactMap = [NSMutableDictionary new]; - for (Contact *contact in contacts) { - for (PhoneNumber *phoneNumber in contact.parsedPhoneNumbers) { - NSString *phoneNumberE164 = phoneNumber.toE164; - if (phoneNumberE164.length > 0) { - contactMap[phoneNumberE164] = contact; - } - } - } - self.contactMap = contactMap; -} - #pragma mark - Logging + (NSString *)tag diff --git a/Signal/src/ViewControllers/OWSConversationSettingsTableViewController.m b/Signal/src/ViewControllers/OWSConversationSettingsTableViewController.m index 750d72f0f..1a5beb489 100644 --- a/Signal/src/ViewControllers/OWSConversationSettingsTableViewController.m +++ b/Signal/src/ViewControllers/OWSConversationSettingsTableViewController.m @@ -437,9 +437,9 @@ static NSString *const OWSConversationSettingsTableViewControllerSegueShowGroupM return; } [BlockListUIUtils showBlockPhoneNumberActionSheet:self.thread.contactIdentifier - displayName:self.thread.name fromViewController:self blockingManager:_blockingManager + contactsManager:_contactsManager completionBlock:^(BOOL isBlocked) { // Update switch state if user cancels action. blockUserSwitch.on = isBlocked; @@ -450,9 +450,9 @@ static NSString *const OWSConversationSettingsTableViewControllerSegueShowGroupM return; } [BlockListUIUtils showUnblockPhoneNumberActionSheet:self.thread.contactIdentifier - displayName:self.thread.name fromViewController:self blockingManager:_blockingManager + contactsManager:_contactsManager completionBlock:^(BOOL isBlocked) { // Update switch state if user cancels action. blockUserSwitch.on = isBlocked; diff --git a/Signal/src/contact/OWSContactsManager.h b/Signal/src/contact/OWSContactsManager.h index a2564dbbc..36cc0a5d8 100644 --- a/Signal/src/contact/OWSContactsManager.h +++ b/Signal/src/contact/OWSContactsManager.h @@ -39,6 +39,7 @@ extern NSString *const OWSContactsManagerSignalRecipientsDidChangeNotification; - (void)doAfterEnvironmentInitSetup; - (NSString *)displayNameForPhoneIdentifier:(nullable NSString *)identifier; +- (NSString *)displayNameForContact:(Contact *)contact; - (nullable UIImage *)imageForPhoneIdentifier:(nullable NSString *)identifier; - (NSAttributedString *)formattedFullNameForContact:(Contact *)contact font:(UIFont *)font; diff --git a/Signal/src/contact/OWSContactsManager.m b/Signal/src/contact/OWSContactsManager.m index ce7c32f86..93ccef4f0 100644 --- a/Signal/src/contact/OWSContactsManager.m +++ b/Signal/src/contact/OWSContactsManager.m @@ -20,12 +20,15 @@ NSString *const OWSContactsManagerSignalRecipientsDidChangeNotification = @property TOCFuture *futureAddressBook; @property ObservableValueController *observableContactsController; @property TOCCancelTokenSource *life; -@property(atomic, copy) NSDictionary *latestContactsById; +@property (atomic) NSDictionary *latestContactsById; +@property (atomic) NSDictionary *contactMap; @end @implementation OWSContactsManager +@synthesize latestContactsById = _latestContactsById; + - (void)dealloc { [_life cancel]; } @@ -46,6 +49,41 @@ NSString *const OWSContactsManagerSignalRecipientsDidChangeNotification = return self; } +- (NSDictionary *)latestContactsById +{ + @synchronized(self) + { + return _latestContactsById; + } +} + +- (void)setLatestContactsById:(NSDictionary *)latestContactsById +{ + @synchronized(self) + { + _latestContactsById = [latestContactsById copy]; + + NSMutableDictionary *contactMap = [NSMutableDictionary new]; + for (Contact *contact in _latestContactsById.allValues) { + // The allContacts method seems to protect against non-contact instances + // in latestContactsById, so I've done the same here. I'm not sure if + // this is a real issue. + OWSAssert([contact isKindOfClass:[Contact class]]); + if (![contact isKindOfClass:[Contact class]]) { + continue; + } + + for (PhoneNumber *phoneNumber in contact.parsedPhoneNumbers) { + NSString *phoneNumberE164 = phoneNumber.toE164; + if (phoneNumberE164.length > 0) { + contactMap[phoneNumberE164] = contact; + } + } + } + self.contactMap = contactMap; + } +} + - (void)doAfterEnvironmentInitSetup { if (SYSTEM_VERSION_GREATER_THAN_OR_EQUAL_TO(9, 0)) { self.contactStore = [[CNContactStore alloc] init]; @@ -419,6 +457,15 @@ void onAddressBookChanged(ABAddressBookRef notifyAddressBook, CFDictionaryRef in return displayName; } +- (NSString *_Nonnull)displayNameForContact:(Contact *)contact +{ + OWSAssert(contact); + + NSString *displayName = (contact.fullName.length > 0) ? contact.fullName : self.unknownContactName; + + return displayName; +} + - (NSAttributedString *_Nonnull)formattedFullNameForContact:(Contact *)contact font:(UIFont *_Nonnull)font { UIFont *boldFont = [UIFont ows_mediumFontWithSize:font.pointSize]; @@ -473,14 +520,7 @@ void onAddressBookChanged(ABAddressBookRef notifyAddressBook, CFDictionaryRef in if (!identifier) { return nil; } - for (Contact *contact in self.allContacts) { - for (PhoneNumber *phoneNumber in contact.parsedPhoneNumbers) { - if ([phoneNumber.toE164 isEqualToString:identifier]) { - return contact; - } - } - } - return nil; + return self.contactMap[identifier]; } - (Contact *)getOrBuildContactForPhoneIdentifier:(NSString *)identifier