Skip to content

Commit c161fdf

Browse files
mikechochmikechoch
authored
Fixed if checks related to external user id (#672)
* 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
1 parent 1ea798f commit c161fdf

File tree

2 files changed

+214
-27
lines changed

2 files changed

+214
-27
lines changed

‎iOS_SDK/OneSignalSDK/Source/OneSignal.m‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2577,12 +2577,12 @@ + (void)setExternalUserId:(NSString * _Nonnull)externalId withCompletion:(OSUpda
25772577
}
25782578

25792579
[OneSignalClient.sharedClient executeSimultaneousRequests:requests withCompletion:^(NSDictionary<NSString *,NSDictionary *> *results) {
2580-
if (results[@"push"] && results[@"push"][@"success"])
2580+
if (results[@"push"] && results[@"push"][@"success"] && [results[@"push"][@"success"] boolValue])
25812581
[OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EXTERNAL_USER_ID withValue:externalId];
2582-
2583-
if (results[@"email"] && results[@"email"][@"success"])
2584-
[OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EMAIL_EXTERNAL_USER_ID withValue:externalId];
25852582

2583+
if (results[@"email"] && results[@"email"][@"success"] && [results[@"email"][@"success"] boolValue])
2584+
[OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EMAIL_EXTERNAL_USER_ID withValue:externalId];
2585+
25862586
if (completionBlock)
25872587
completionBlock(results);
25882588
}];
@@ -2617,8 +2617,16 @@ + (BOOL)isEmailSetup {
26172617
}
26182618

26192619
+ (BOOL)shouldUpdateExternalUserId:(NSString*)externalId withRequests:(NSDictionary*)requests {
2620-
return (![self.existingPushExternalUserId isEqualToString:externalId] && !requests[@"email"])
2621-
|| (requests[@"email"] && ![self.existingEmailExternalUserId isEqualToString:externalId]);
2620+
// If we are not making a request to email user, no need to validate that external user id
2621+
bool updateExternalUserId = ![self.existingPushExternalUserId isEqualToString:externalId]
2622+
&& !requests[@"email"];
2623+
2624+
// If we are making a request to email user, we need validate both external user ids
2625+
bool updateEmailExternalUserId = (![self.existingPushExternalUserId isEqualToString:externalId]
2626+
&& requests[@"email"]
2627+
&& ![self.existingEmailExternalUserId isEqualToString:externalId]);
2628+
2629+
return updateExternalUserId || updateEmailExternalUserId;
26222630
}
26232631

26242632
+ (NSMutableDictionary*)getDuplicateExternalUserIdResponse:(NSString*)externalId withRequests:(NSDictionary*)requests {
@@ -2628,16 +2636,12 @@ + (NSMutableDictionary*)getDuplicateExternalUserIdResponse:(NSString*)externalId
26282636
results[@"push"] = @{
26292637
@"success" : @(true)
26302638
};
2631-
[OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EXTERNAL_USER_ID withValue:externalId];
2632-
2639+
26332640
// Make sure to only add email if email was attempted
26342641
if (requests[@"email"]) {
26352642
results[@"email"] = @{
26362643
@"success" : @(true)
26372644
};
2638-
[OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EMAIL_EXTERNAL_USER_ID withValue:externalId];
2639-
} else {
2640-
[OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EMAIL_EXTERNAL_USER_ID withValue:nil];
26412645
}
26422646

26432647
return results;

‎iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m‎

Lines changed: 199 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2382,10 +2382,10 @@ - (void)testSetExternalUserId_forPush_withCompletion {
23822382

23832383
// 2. Call setExternalUserId with callbacks
23842384
[OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID withCompletion:^(NSDictionary *results) {
2385-
if (results[@"push"] && results[@"push"][@"success"])
2385+
if (results[@"push"] && results[@"push"][@"success"] && [results[@"push"][@"success"] boolValue])
23862386
self.CALLBACK_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID;
23872387

2388-
if (results[@"email"] && results[@"email"][@"success"])
2388+
if (results[@"email"] && results[@"email"][@"success"] && [results[@"email"][@"success"] boolValue])
23892389
self.CALLBACK_EMAIL_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID;
23902390
}];
23912391
[UnitTestCommonMethods runBackgroundThreads];
@@ -2405,31 +2405,214 @@ - (void)testSetExternalUserId_forPushAndEmail_withCompletion {
24052405

24062406
// 3. Call setExternalUserId with callbacks
24072407
[OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID withCompletion:^(NSDictionary *results) {
2408-
if (results[@"push"] && results[@"push"][@"success"])
2408+
if (results[@"push"] && results[@"push"][@"success"] && [results[@"push"][@"success"] boolValue])
24092409
self.CALLBACK_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID;
24102410

2411-
if (results[@"email"] && results[@"email"][@"success"])
2411+
if (results[@"email"] && results[@"email"][@"success"] && [results[@"email"][@"success"] boolValue])
24122412
self.CALLBACK_EMAIL_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID;
24132413
}];
24142414
[UnitTestCommonMethods runBackgroundThreads];
24152415

2416-
// 3. Make sure push and email external id were updated in completion callback
2416+
// 4. Make sure push and email external id were updated in completion callback
24172417
XCTAssertEqual(self.CALLBACK_EXTERNAL_USER_ID, TEST_EXTERNAL_USER_ID);
24182418
XCTAssertEqual(self.CALLBACK_EMAIL_EXTERNAL_USER_ID, TEST_EXTERNAL_USER_ID);
24192419
}
24202420

2421-
- (void)testRemoveExternalUserId {
2421+
- (void)testSetExternalUserId_forPush_afterLogoutEmail_withCompletion {
2422+
// 1. Init OneSignal
24222423
[UnitTestCommonMethods initOneSignalAndThreadWait];
24232424

2425+
// 2. Set email
2426+
[OneSignal setEmail:TEST_EMAIL];
2427+
[UnitTestCommonMethods runBackgroundThreads];
2428+
2429+
// 3. Call setExternalUserId with completion callback
2430+
[OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID withCompletion:^(NSDictionary *results) {
2431+
if (results[@"push"] && results[@"push"][@"success"] && [results[@"push"][@"success"] boolValue])
2432+
self.CALLBACK_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID;
2433+
2434+
if (results[@"email"] && results[@"email"][@"success"] && [results[@"email"][@"success"] boolValue])
2435+
self.CALLBACK_EMAIL_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID;
2436+
}];
2437+
[UnitTestCommonMethods runBackgroundThreads];
2438+
2439+
// 4. Make sure push and email external id were updated in completion callback
2440+
XCTAssertEqual(self.CALLBACK_EXTERNAL_USER_ID, TEST_EXTERNAL_USER_ID);
2441+
XCTAssertEqual(self.CALLBACK_EMAIL_EXTERNAL_USER_ID, TEST_EXTERNAL_USER_ID);
2442+
2443+
// 5. Clear out external user id callback ids
2444+
self.CALLBACK_EXTERNAL_USER_ID = nil;
2445+
self.CALLBACK_EMAIL_EXTERNAL_USER_ID = nil;
2446+
2447+
// 6. Log out email
2448+
[OneSignal logoutEmail];
2449+
[UnitTestCommonMethods runBackgroundThreads];
2450+
2451+
// 7. Call setExternalUserId with completion callback
2452+
[OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID withCompletion:^(NSDictionary *results) {
2453+
if (results[@"push"] && results[@"push"][@"success"] && [results[@"push"][@"success"] boolValue])
2454+
self.CALLBACK_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID;
2455+
2456+
if (results[@"email"] && results[@"email"][@"success"] && [results[@"email"][@"success"] boolValue])
2457+
self.CALLBACK_EMAIL_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID;
2458+
}];
2459+
[UnitTestCommonMethods runBackgroundThreads];
2460+
2461+
// 8. Make sure push external id was updated in completion callback
2462+
XCTAssertEqual(self.CALLBACK_EXTERNAL_USER_ID, TEST_EXTERNAL_USER_ID);
2463+
XCTAssertNil(self.CALLBACK_EMAIL_EXTERNAL_USER_ID);
2464+
}
2465+
2466+
- (void)testOverwriteSameExternalUserId_forPushAndEmail_withCompletion {
2467+
// 1. Cache the same external user ids for push and email channel
2468+
[OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EXTERNAL_USER_ID withValue:TEST_EXTERNAL_USER_ID];
2469+
[OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EMAIL_EXTERNAL_USER_ID withValue:TEST_EXTERNAL_USER_ID];
2470+
2471+
// 2. Init OneSignal
2472+
[UnitTestCommonMethods initOneSignalAndThreadWait];
2473+
2474+
// 3. Set email
2475+
[OneSignal setEmail:TEST_EMAIL];
2476+
[UnitTestCommonMethods runBackgroundThreads];
2477+
2478+
// 4. Call setExternalUserId with callbacks
2479+
[OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID withCompletion:^(NSDictionary *results) {
2480+
if (results[@"push"] && results[@"push"][@"success"] && [results[@"push"][@"success"] boolValue])
2481+
self.CALLBACK_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID;
2482+
2483+
if (results[@"email"] && results[@"email"][@"success"] && [results[@"email"][@"success"] boolValue])
2484+
self.CALLBACK_EMAIL_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID;
2485+
}];
2486+
[UnitTestCommonMethods runBackgroundThreads];
2487+
2488+
// 5. Make sure only push external id was attempted to be set since no email was set yet
2489+
XCTAssertEqual(self.CALLBACK_EXTERNAL_USER_ID, TEST_EXTERNAL_USER_ID);
2490+
XCTAssertEqual(self.CALLBACK_EMAIL_EXTERNAL_USER_ID, TEST_EXTERNAL_USER_ID);
2491+
}
2492+
2493+
- (void)testOverwriteDifferentExternalUserId_forPushAndEmail_withCompletion {
2494+
// 1. Cache different same external user ids for push and email channel
2495+
[OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EXTERNAL_USER_ID withValue:@"12345"];
2496+
[OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EMAIL_EXTERNAL_USER_ID withValue:@"12345"];
2497+
2498+
// 2. Init OneSignal
2499+
[UnitTestCommonMethods initOneSignalAndThreadWait];
2500+
2501+
// 3. Set email
2502+
[OneSignal setEmail:TEST_EMAIL];
2503+
[UnitTestCommonMethods runBackgroundThreads];
2504+
2505+
// 4. Call setExternalUserId with callbacks
2506+
[OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID withCompletion:^(NSDictionary *results) {
2507+
if (results[@"push"] && results[@"push"][@"success"] && [results[@"push"][@"success"] boolValue])
2508+
self.CALLBACK_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID;
2509+
2510+
if (results[@"email"] && results[@"email"][@"success"] && [results[@"email"][@"success"] boolValue])
2511+
self.CALLBACK_EMAIL_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID;
2512+
}];
2513+
[UnitTestCommonMethods runBackgroundThreads];
2514+
2515+
// 5. Make sure only push external id was attempted to be set since no email was set yet
2516+
XCTAssertEqual(self.CALLBACK_EXTERNAL_USER_ID, TEST_EXTERNAL_USER_ID);
2517+
XCTAssertEqual(self.CALLBACK_EMAIL_EXTERNAL_USER_ID, TEST_EXTERNAL_USER_ID);
2518+
}
2519+
2520+
- (void)testOverwriteExternalUserId_forPushAndEmail_withCompletion {
2521+
// 1. Cache two different external user ids for push and email channel
2522+
[OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EXTERNAL_USER_ID withValue:@"12345"];
2523+
[OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EMAIL_EXTERNAL_USER_ID withValue:TEST_EXTERNAL_USER_ID];
2524+
2525+
// 2. Init OneSignal
2526+
[UnitTestCommonMethods initOneSignalAndThreadWait];
2527+
2528+
// 3. Set email
2529+
[OneSignal setEmail:TEST_EMAIL];
2530+
[UnitTestCommonMethods runBackgroundThreads];
2531+
2532+
// 4. Call setExternalUserId with callbacks
2533+
[OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID withCompletion:^(NSDictionary *results) {
2534+
if (results[@"push"] && results[@"push"][@"success"] && [results[@"push"][@"success"] boolValue])
2535+
self.CALLBACK_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID;
2536+
2537+
if (results[@"email"] && results[@"email"][@"success"] && [results[@"email"][@"success"] boolValue])
2538+
self.CALLBACK_EMAIL_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID;
2539+
}];
2540+
[UnitTestCommonMethods runBackgroundThreads];
2541+
2542+
// 5. Make sure only push external id was attempted to be set since no email was set yet
2543+
XCTAssertEqual(self.CALLBACK_EXTERNAL_USER_ID, TEST_EXTERNAL_USER_ID);
2544+
XCTAssertEqual(self.CALLBACK_EMAIL_EXTERNAL_USER_ID, TEST_EXTERNAL_USER_ID);
2545+
}
2546+
2547+
- (void)testOverwriteEmailExternalUserId_forPushAndEmail_withCompletion {
2548+
// 1. Cache two different external user ids for push and email channel
2549+
[OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EXTERNAL_USER_ID withValue:TEST_EXTERNAL_USER_ID];
2550+
[OneSignalUserDefaults.initStandard saveStringForKey:OSUD_EMAIL_EXTERNAL_USER_ID withValue:@"12345"];
2551+
2552+
// 2. Init OneSignal
2553+
[UnitTestCommonMethods initOneSignalAndThreadWait];
2554+
2555+
// 3. Set email
2556+
[OneSignal setEmail:TEST_EMAIL];
2557+
[UnitTestCommonMethods runBackgroundThreads];
2558+
2559+
// 4. Call setExternalUserId with callbacks
2560+
[OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID withCompletion:^(NSDictionary *results) {
2561+
if (results[@"push"] && results[@"push"][@"success"] && [results[@"push"][@"success"] boolValue])
2562+
self.CALLBACK_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID;
2563+
2564+
if (results[@"email"] && results[@"email"][@"success"] && [results[@"email"][@"success"] boolValue])
2565+
self.CALLBACK_EMAIL_EXTERNAL_USER_ID = TEST_EXTERNAL_USER_ID;
2566+
}];
2567+
[UnitTestCommonMethods runBackgroundThreads];
2568+
2569+
// 5. Make sure only push external id was attempted to be set since no email was set yet
2570+
XCTAssertEqual(self.CALLBACK_EXTERNAL_USER_ID, TEST_EXTERNAL_USER_ID);
2571+
XCTAssertEqual(self.CALLBACK_EMAIL_EXTERNAL_USER_ID, TEST_EXTERNAL_USER_ID);
2572+
}
2573+
2574+
- (void)testRemoveExternalUserId_forPush {
2575+
// 1. Init OneSignal
2576+
[UnitTestCommonMethods initOneSignalAndThreadWait];
2577+
2578+
// 2. Set external user id
24242579
[OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID];
24252580
[UnitTestCommonMethods runBackgroundThreads];
24262581

2582+
// 3. Make sure last request was external id and had the correct external id being used in the request payload
24272583
XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequestType, NSStringFromClass([OSRequestUpdateExternalUserId class]));
24282584
XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"], TEST_EXTERNAL_USER_ID);
24292585

2586+
// 4. Remove the external user id
24302587
[OneSignal removeExternalUserId];
24312588
[UnitTestCommonMethods runBackgroundThreads];
24322589

2590+
// 5. Make sure last request was external id and the external id being used was an empty string
2591+
XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequestType, NSStringFromClass([OSRequestUpdateExternalUserId class]));
2592+
XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"], @"");
2593+
}
2594+
2595+
- (void)testRemoveExternalUserId_forPushAndEmail {
2596+
// 1. Init OneSignal
2597+
[UnitTestCommonMethods initOneSignalAndThreadWait];
2598+
2599+
// 2. Set email
2600+
[OneSignal setEmail:TEST_EMAIL];
2601+
[UnitTestCommonMethods runBackgroundThreads];
2602+
2603+
// 3. Set external user id
2604+
[OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID];
2605+
[UnitTestCommonMethods runBackgroundThreads];
2606+
2607+
// 4. Make sure last request was external id and had the correct external id being used in the request payload
2608+
XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequestType, NSStringFromClass([OSRequestUpdateExternalUserId class]));
2609+
XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"], TEST_EXTERNAL_USER_ID);
2610+
2611+
// 5. Remove the external user id
2612+
[OneSignal removeExternalUserId];
2613+
[UnitTestCommonMethods runBackgroundThreads];
2614+
2615+
// 6. Make sure last request was external id and the external id being used was an empty string
24332616
XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequestType, NSStringFromClass([OSRequestUpdateExternalUserId class]));
24342617
XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"], @"");
24352618
}
@@ -2446,12 +2629,12 @@ - (void)testRemoveExternalUserId_forPush_withCompletion {
24462629
XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequestType, NSStringFromClass([OSRequestUpdateExternalUserId class]));
24472630
XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"], TEST_EXTERNAL_USER_ID);
24482631

2449-
// 4. Remove the external user id
2632+
// 4. Remove the external user id with a callack implemented
24502633
[OneSignal removeExternalUserId:^(NSDictionary *results) {
2451-
if (results[@"push"] && results[@"push"][@"success"])
2634+
if (results[@"push"] && results[@"push"][@"success"] && [results[@"push"][@"success"] boolValue])
24522635
self.CALLBACK_EXTERNAL_USER_ID = @"";
24532636

2454-
if (results[@"email"] && results[@"email"][@"success"])
2637+
if (results[@"email"] && results[@"email"][@"success"] && [results[@"email"][@"success"] boolValue])
24552638
self.CALLBACK_EMAIL_EXTERNAL_USER_ID = @"";
24562639
}];
24572640
[UnitTestCommonMethods runBackgroundThreads];
@@ -2473,29 +2656,29 @@ - (void)testRemoveExternalUserId_forPushAndEmail_withCompletion {
24732656
[OneSignal setEmail:TEST_EMAIL];
24742657
[UnitTestCommonMethods runBackgroundThreads];
24752658

2476-
// 2. Set external user id
2659+
// 3. Set external user id
24772660
[OneSignal setExternalUserId:TEST_EXTERNAL_USER_ID];
24782661
[UnitTestCommonMethods runBackgroundThreads];
24792662

2480-
// 3. Make sure last request was external id and had the correct external id being used in the request payload
2663+
// 4. Make sure last request was external id and had the correct external id being used in the request payload
24812664
XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequestType, NSStringFromClass([OSRequestUpdateExternalUserId class]));
24822665
XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"], TEST_EXTERNAL_USER_ID);
24832666

2484-
// 4. Remove the external user id
2667+
// 5. Remove the external user id
24852668
[OneSignal removeExternalUserId:^(NSDictionary *results) {
2486-
if (results[@"push"] && results[@"push"][@"success"])
2669+
if (results[@"push"] && results[@"push"][@"success"] && [results[@"push"][@"success"] boolValue])
24872670
self.CALLBACK_EXTERNAL_USER_ID = @"";
24882671

2489-
if (results[@"email"] && results[@"email"][@"success"])
2672+
if (results[@"email"] && results[@"email"][@"success"] && [results[@"email"][@"success"] boolValue])
24902673
self.CALLBACK_EMAIL_EXTERNAL_USER_ID = @"";
24912674
}];
24922675
[UnitTestCommonMethods runBackgroundThreads];
24932676

2494-
// 5. Make sure last request was external id and the external id being used was an empty string
2677+
// 6. Make sure last request was external id and the external id being used was an empty string
24952678
XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequestType, NSStringFromClass([OSRequestUpdateExternalUserId class]));
24962679
XCTAssertEqualObjects(OneSignalClientOverrider.lastHTTPRequest[@"external_user_id"], @"");
24972680

2498-
// 6. Make sure completion handler was called, push and email external ids are empty strings
2681+
// 7. Make sure completion handler was called, push and email external ids are empty strings
24992682
XCTAssertEqual(self.CALLBACK_EXTERNAL_USER_ID, @"");
25002683
XCTAssertEqual(self.CALLBACK_EMAIL_EXTERNAL_USER_ID, @"");
25012684
}

0 commit comments

Comments
 (0)