From 5986a963c7aee5b8e5ab2c720bd7d074c46ef84c Mon Sep 17 00:00:00 2001 From: mikechoch Date: Tue, 26 May 2020 11:25:16 -0400 Subject: [PATCH] Fixed if checks related to external user id * Make sure when we are updating email external user id we check both push and email channels separately * On completion only update the cached external user id if the success status is true (was only checking contains before) * Added various UnitTests that check overwriting several scenarios with push and email external user id --- iOS_SDK/OneSignalSDK/Source/OneSignal.m | 26 +-- iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m | 215 +++++++++++++++++++-- 2 files changed, 214 insertions(+), 27 deletions(-) diff --git a/iOS_SDK/OneSignalSDK/Source/OneSignal.m b/iOS_SDK/OneSignalSDK/Source/OneSignal.m index 1e7d051e1..0c4737821 100755 --- a/iOS_SDK/OneSignalSDK/Source/OneSignal.m +++ b/iOS_SDK/OneSignalSDK/Source/OneSignal.m @@ -2577,12 +2577,12 @@ + (void)setExternalUserId:(NSString * _Nonnull)externalId withCompletion:(OSUpda } [OneSignalClient.sharedClient executeSimultaneousRequests:requests withCompletion:^(NSDictionary *results) { - if (results[@"push"] && results[@"push"][@"success"]) + if (results[@"push"] && results[@"push"][@"success"] && [results[@"push"][@"success"] boolValue]) [OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EXTERNAL_USER_ID withValue:externalId]; - - if (results[@"email"] && results[@"email"][@"success"]) - [OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EMAIL_EXTERNAL_USER_ID withValue:externalId]; + if (results[@"email"] && results[@"email"][@"success"] && [results[@"email"][@"success"] boolValue]) + [OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EMAIL_EXTERNAL_USER_ID withValue:externalId]; + if (completionBlock) completionBlock(results); }]; @@ -2617,8 +2617,16 @@ + (BOOL)isEmailSetup { } + (BOOL)shouldUpdateExternalUserId:(NSString*)externalId withRequests:(NSDictionary*)requests { - return (![self.existingPushExternalUserId isEqualToString:externalId] && !requests[@"email"]) - || (requests[@"email"] && ![self.existingEmailExternalUserId isEqualToString:externalId]); + // If we are not making a request to email user, no need to validate that external user id + bool updateExternalUserId = ![self.existingPushExternalUserId isEqualToString:externalId] + && !requests[@"email"]; + + // If we are making a request to email user, we need validate both external user ids + bool updateEmailExternalUserId = (![self.existingPushExternalUserId isEqualToString:externalId] + && requests[@"email"] + && ![self.existingEmailExternalUserId isEqualToString:externalId]); + + return updateExternalUserId || updateEmailExternalUserId; } + (NSMutableDictionary*)getDuplicateExternalUserIdResponse:(NSString*)externalId withRequests:(NSDictionary*)requests { @@ -2628,16 +2636,12 @@ + (NSMutableDictionary*)getDuplicateExternalUserIdResponse:(NSString*)externalId results[@"push"] = @{ @"success" : @(true) }; - [OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EXTERNAL_USER_ID withValue:externalId]; - + // Make sure to only add email if email was attempted if (requests[@"email"]) { results[@"email"] = @{ @"success" : @(true) }; - [OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EMAIL_EXTERNAL_USER_ID withValue:externalId]; - } else { - [OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EMAIL_EXTERNAL_USER_ID withValue:nil]; } return results; diff --git a/iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m b/iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m index 36c4205a0..2636f687d 100644 --- a/iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m +++ b/iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m @@ -2373,10 +2373,10 @@ - (void)testSetExternalUserId_forPush_withCompletion { // 2. Call setExternalUserId with callbacks [OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID withCompletion:^(NSDictionary *results) { - if (results[@"push"] && results[@"push"][@"success"]) + if (results[@"push"] && results[@"push"][@"success"] && [results[@"push"][@"success"] boolValue]) self.CALLBACK_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID; - if (results[@"email"] && results[@"email"][@"success"]) + if (results[@"email"] && results[@"email"][@"success"] && [results[@"email"][@"success"] boolValue]) self.CALLBACK_EMAIL_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID; }]; [UnitTestCommonMethods runBackgroundThreads]; @@ -2396,31 +2396,214 @@ - (void)testSetExternalUserId_forPushAndEmail_withCompletion { // 3. Call setExternalUserId with callbacks [OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID withCompletion:^(NSDictionary *results) { - if (results[@"push"] && results[@"push"][@"success"]) + if (results[@"push"] && results[@"push"][@"success"] && [results[@"push"][@"success"] boolValue]) self.CALLBACK_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID; - if (results[@"email"] && results[@"email"][@"success"]) + if (results[@"email"] && results[@"email"][@"success"] && [results[@"email"][@"success"] boolValue]) self.CALLBACK_EMAIL_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID; }]; [UnitTestCommonMethods runBackgroundThreads]; - // 3. Make sure push and email external id were updated in completion callback + // 4. Make sure push and email external id were updated in completion callback XCTAssertEqual(self.CALLBACK_EXTERNAL_USER_ID, TEST_EXTERNAL_USER_ID); XCTAssertEqual(self.CALLBACK_EMAIL_EXTERNAL_USER_ID, TEST_EXTERNAL_USER_ID); } -- (void)testRemoveExternalUserId { +- (void)testSetExternalUserId_forPush_afterLogoutEmail_withCompletion { + // 1. Init OneSignal [UnitTestCommonMethods initOneSignalAndThreadWait]; + // 2. Set email + [OneSignal setEmail:TEST_EMAIL]; + [UnitTestCommonMethods runBackgroundThreads]; + + // 3. Call setExternalUserId with completion callback + [OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID withCompletion:^(NSDictionary *results) { + if (results[@"push"] && results[@"push"][@"success"] && [results[@"push"][@"success"] boolValue]) + self.CALLBACK_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID; + + if (results[@"email"] && results[@"email"][@"success"] && [results[@"email"][@"success"] boolValue]) + self.CALLBACK_EMAIL_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID; + }]; + [UnitTestCommonMethods runBackgroundThreads]; + + // 4. Make sure push and email external id were updated in completion callback + XCTAssertEqual(self.CALLBACK_EXTERNAL_USER_ID, TEST_EXTERNAL_USER_ID); + XCTAssertEqual(self.CALLBACK_EMAIL_EXTERNAL_USER_ID, TEST_EXTERNAL_USER_ID); + + // 5. Clear out external user id callback ids + self.CALLBACK_EXTERNAL_USER_ID = nil; + self.CALLBACK_EMAIL_EXTERNAL_USER_ID = nil; + + // 6. Log out email + [OneSignal logoutEmail]; + [UnitTestCommonMethods runBackgroundThreads]; + + // 7. Call setExternalUserId with completion callback + [OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID withCompletion:^(NSDictionary *results) { + if (results[@"push"] && results[@"push"][@"success"] && [results[@"push"][@"success"] boolValue]) + self.CALLBACK_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID; + + if (results[@"email"] && results[@"email"][@"success"] && [results[@"email"][@"success"] boolValue]) + self.CALLBACK_EMAIL_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID; + }]; + [UnitTestCommonMethods runBackgroundThreads]; + + // 8. Make sure push external id was updated in completion callback + XCTAssertEqual(self.CALLBACK_EXTERNAL_USER_ID, TEST_EXTERNAL_USER_ID); + XCTAssertNil(self.CALLBACK_EMAIL_EXTERNAL_USER_ID); +} + +- (void)testOverwriteSameExternalUserId_forPushAndEmail_withCompletion { + // 1. Cache the same external user ids for push and email channel + [OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EXTERNAL_USER_ID withValue:TEST_EXTERNAL_USER_ID]; + [OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EMAIL_EXTERNAL_USER_ID withValue:TEST_EXTERNAL_USER_ID]; + + // 2. Init OneSignal + [UnitTestCommonMethods initOneSignalAndThreadWait]; + + // 3. Set email + [OneSignal setEmail:TEST_EMAIL]; + [UnitTestCommonMethods runBackgroundThreads]; + + // 4. Call setExternalUserId with callbacks + [OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID withCompletion:^(NSDictionary *results) { + if (results[@"push"] && results[@"push"][@"success"] && [results[@"push"][@"success"] boolValue]) + self.CALLBACK_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID; + + if (results[@"email"] && results[@"email"][@"success"] && [results[@"email"][@"success"] boolValue]) + self.CALLBACK_EMAIL_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID; + }]; + [UnitTestCommonMethods runBackgroundThreads]; + + // 5. Make sure only push external id was attempted to be set since no email was set yet + XCTAssertEqual(self.CALLBACK_EXTERNAL_USER_ID, TEST_EXTERNAL_USER_ID); + XCTAssertEqual(self.CALLBACK_EMAIL_EXTERNAL_USER_ID, TEST_EXTERNAL_USER_ID); +} + +- (void)testOverwriteDifferentExternalUserId_forPushAndEmail_withCompletion { + // 1. Cache different same external user ids for push and email channel + [OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EXTERNAL_USER_ID withValue:@"12345"]; + [OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EMAIL_EXTERNAL_USER_ID withValue:@"12345"]; + + // 2. Init OneSignal + [UnitTestCommonMethods initOneSignalAndThreadWait]; + + // 3. Set email + [OneSignal setEmail:TEST_EMAIL]; + [UnitTestCommonMethods runBackgroundThreads]; + + // 4. Call setExternalUserId with callbacks + [OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID withCompletion:^(NSDictionary *results) { + if (results[@"push"] && results[@"push"][@"success"] && [results[@"push"][@"success"] boolValue]) + self.CALLBACK_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID; + + if (results[@"email"] && results[@"email"][@"success"] && [results[@"email"][@"success"] boolValue]) + self.CALLBACK_EMAIL_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID; + }]; + [UnitTestCommonMethods runBackgroundThreads]; + + // 5. Make sure only push external id was attempted to be set since no email was set yet + XCTAssertEqual(self.CALLBACK_EXTERNAL_USER_ID, TEST_EXTERNAL_USER_ID); + XCTAssertEqual(self.CALLBACK_EMAIL_EXTERNAL_USER_ID, TEST_EXTERNAL_USER_ID); +} + +- (void)testOverwriteExternalUserId_forPushAndEmail_withCompletion { + // 1. Cache two different external user ids for push and email channel + [OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EXTERNAL_USER_ID withValue:@"12345"]; + [OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EMAIL_EXTERNAL_USER_ID withValue:TEST_EXTERNAL_USER_ID]; + + // 2. Init OneSignal + [UnitTestCommonMethods initOneSignalAndThreadWait]; + + // 3. Set email + [OneSignal setEmail:TEST_EMAIL]; + [UnitTestCommonMethods runBackgroundThreads]; + + // 4. Call setExternalUserId with callbacks + [OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID withCompletion:^(NSDictionary *results) { + if (results[@"push"] && results[@"push"][@"success"] && [results[@"push"][@"success"] boolValue]) + self.CALLBACK_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID; + + if (results[@"email"] && results[@"email"][@"success"] && [results[@"email"][@"success"] boolValue]) + self.CALLBACK_EMAIL_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID; + }]; + [UnitTestCommonMethods runBackgroundThreads]; + + // 5. Make sure only push external id was attempted to be set since no email was set yet + XCTAssertEqual(self.CALLBACK_EXTERNAL_USER_ID, TEST_EXTERNAL_USER_ID); + XCTAssertEqual(self.CALLBACK_EMAIL_EXTERNAL_USER_ID, TEST_EXTERNAL_USER_ID); +} + +- (void)testOverwriteEmailExternalUserId_forPushAndEmail_withCompletion { + // 1. Cache two different external user ids for push and email channel + [OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EXTERNAL_USER_ID withValue:TEST_EXTERNAL_USER_ID]; + [OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EMAIL_EXTERNAL_USER_ID withValue:@"12345"]; + + // 2. Init OneSignal + [UnitTestCommonMethods initOneSignalAndThreadWait]; + + // 3. Set email + [OneSignal setEmail:TEST_EMAIL]; + [UnitTestCommonMethods runBackgroundThreads]; + + // 4. Call setExternalUserId with callbacks + [OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID withCompletion:^(NSDictionary *results) { + if (results[@"push"] && results[@"push"][@"success"] && [results[@"push"][@"success"] boolValue]) + self.CALLBACK_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID; + + if (results[@"email"] && results[@"email"][@"success"] && [results[@"email"][@"success"] boolValue]) + self.CALLBACK_EMAIL_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID; + }]; + [UnitTestCommonMethods runBackgroundThreads]; + + // 5. Make sure only push external id was attempted to be set since no email was set yet + XCTAssertEqual(self.CALLBACK_EXTERNAL_USER_ID, TEST_EXTERNAL_USER_ID); + XCTAssertEqual(self.CALLBACK_EMAIL_EXTERNAL_USER_ID, TEST_EXTERNAL_USER_ID); +} + +- (void)testRemoveExternalUserId_forPush { + // 1. Init OneSignal + [UnitTestCommonMethods initOneSignalAndThreadWait]; + + // 2. Set external user id [OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID]; [UnitTestCommonMethods runBackgroundThreads]; + // 3. Make sure last request was external id and had the correct external id being used in the request payload XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequestType, NSStringFromClass([OSRequestUpdateExternalUserId class])); XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"], TEST_EXTERNAL_USER_ID); + // 4. Remove the external user id [OneSignal removeExternalUserId]; [UnitTestCommonMethods runBackgroundThreads]; + // 5. Make sure last request was external id and the external id being used was an empty string + XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequestType, NSStringFromClass([OSRequestUpdateExternalUserId class])); + XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"], @""); +} + +- (void)testRemoveExternalUserId_forPushAndEmail { + // 1. Init OneSignal + [UnitTestCommonMethods initOneSignalAndThreadWait]; + + // 2. Set email + [OneSignal setEmail:TEST_EMAIL]; + [UnitTestCommonMethods runBackgroundThreads]; + + // 3. Set external user id + [OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID]; + [UnitTestCommonMethods runBackgroundThreads]; + + // 4. Make sure last request was external id and had the correct external id being used in the request payload + XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequestType, NSStringFromClass([OSRequestUpdateExternalUserId class])); + XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"], TEST_EXTERNAL_USER_ID); + + // 5. Remove the external user id + [OneSignal removeExternalUserId]; + [UnitTestCommonMethods runBackgroundThreads]; + + // 6. Make sure last request was external id and the external id being used was an empty string XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequestType, NSStringFromClass([OSRequestUpdateExternalUserId class])); XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"], @""); } @@ -2437,12 +2620,12 @@ - (void)testRemoveExternalUserId_forPush_withCompletion { XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequestType, NSStringFromClass([OSRequestUpdateExternalUserId class])); XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"], TEST_EXTERNAL_USER_ID); - // 4. Remove the external user id + // 4. Remove the external user id with a callack implemented [OneSignal removeExternalUserId:^(NSDictionary *results) { - if (results[@"push"] && results[@"push"][@"success"]) + if (results[@"push"] && results[@"push"][@"success"] && [results[@"push"][@"success"] boolValue]) self.CALLBACK_EXTERNAL_USER_ID = @""; - if (results[@"email"] && results[@"email"][@"success"]) + if (results[@"email"] && results[@"email"][@"success"] && [results[@"email"][@"success"] boolValue]) self.CALLBACK_EMAIL_EXTERNAL_USER_ID = @""; }]; [UnitTestCommonMethods runBackgroundThreads]; @@ -2464,29 +2647,29 @@ - (void)testRemoveExternalUserId_forPushAndEmail_withCompletion { [OneSignal setEmail:TEST_EMAIL]; [UnitTestCommonMethods runBackgroundThreads]; - // 2. Set external user id + // 3. Set external user id [OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID]; [UnitTestCommonMethods runBackgroundThreads]; - // 3. Make sure last request was external id and had the correct external id being used in the request payload + // 4. Make sure last request was external id and had the correct external id being used in the request payload XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequestType, NSStringFromClass([OSRequestUpdateExternalUserId class])); XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"], TEST_EXTERNAL_USER_ID); - // 4. Remove the external user id + // 5. Remove the external user id [OneSignal removeExternalUserId:^(NSDictionary *results) { - if (results[@"push"] && results[@"push"][@"success"]) + if (results[@"push"] && results[@"push"][@"success"] && [results[@"push"][@"success"] boolValue]) self.CALLBACK_EXTERNAL_USER_ID = @""; - if (results[@"email"] && results[@"email"][@"success"]) + if (results[@"email"] && results[@"email"][@"success"] && [results[@"email"][@"success"] boolValue]) self.CALLBACK_EMAIL_EXTERNAL_USER_ID = @""; }]; [UnitTestCommonMethods runBackgroundThreads]; - // 5. Make sure last request was external id and the external id being used was an empty string + // 6. Make sure last request was external id and the external id being used was an empty string XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequestType, NSStringFromClass([OSRequestUpdateExternalUserId class])); XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"], @""); - // 6. Make sure completion handler was called, push and email external ids are empty strings + // 7. Make sure completion handler was called, push and email external ids are empty strings XCTAssertEqual(self.CALLBACK_EXTERNAL_USER_ID, @""); XCTAssertEqual(self.CALLBACK_EMAIL_EXTERNAL_USER_ID, @""); }