search

Iterable / swift-sdk

Iterable's iOS SDK. Receive and track pushes to Iterable from your iOS app.
https://iterable.com/
MIT License
86 stars 71 forks source link

[MOB-6055] callback on setEmail/setUserId #629

Closed devcsomnicg closed 1 year ago

devcsomnicg commented 1 year ago

🔹 Jira Ticket(s)

✏️ Description

Add callbacks for setEmail/setUserId, and it returns success/failure based on device token is registered successfully or not. It will always return success if autoPushRegistration is false.

devcsomnicg commented 1 year ago

@roninopf Could you please start review for this ?

roninopf commented 1 year ago

@devcsomnicg Will do! Would you like to look over the tests as well?

devcsomnicg commented 1 year ago

@roninopf updated code and tests, could you please run the tests again

Ayyanchira commented 1 year ago

Will have to resolve the conflict. But looks like its replacing the existing method with one with additional parameter. As like for other SDKs, we should have additional methods for the ones with completionHandlers. This is to avoid breakages for developers who would just update to newer versions of SDK

devcsomnicg commented 1 year ago

@Ayyanchira done, added additional methods.

devcsomnicg commented 1 year ago

@Ayyanchira Updated this with suggestions on PR. please review

devcsomnicg commented 1 year ago

@Ayyanchira Updated this with suggestions on PR. please review

Ayyanchira commented 1 year ago

Checked and its working well. So lets go ahead with current changes as it is for now until other methods also start needing success and failure callback handlers.

Ayyanchira commented 1 year ago

Test methods are failing @devcsomnicg . It looks like setEmail leads to AppExtensionHelper.application?.registerForRemoteNotifications(), but that doesn't lead to registerDevice Flow.

devcsomnicg commented 1 year ago

Test methods are failing @devcsomnicg . It looks like setEmail leads to AppExtensionHelper.application?.registerForRemoteNotifications(), but that doesn't lead to registerDevice Flow.

I am not sure about where this code is AppExtensionHelper.application.registerForRemoteNotifications() but, to give you a example when you do registerForRemoteNotifications it calls this function didRegisterForRemoteNotificationsWithDeviceToken here https://github.com/Iterable/swift-sdk/blob/5c033ded01e200acee52c9bd1fca377dbdcbbbce/sample-apps/swift-sample-app/swift-sample-app/AppDelegate.swift#L105.

Ayyanchira commented 1 year ago

Added a call to registerDevice() with no success failure callback after setEmail -> here in this PR. This commit. Verified that it eventually triggers the callbacks. This means the test assumes that registerDevice will eventually get called. And it checks if the handlers set while setting the credentials still remain active or not.