fix: app crash on identify method #458
Conversation
Sample app builds 📱Below you will find the list of the latest versions of the sample apps. It's recommended to always download the latest builds of the sample apps to accurately test the pull request.
|
| // (refer https://github.com/customerio/customerio-ios/blob/94cbf686c3c2a405534cfe908f7166558a3e0b5d/Sources/Tracking/CustomerIO.swift#L71-L75) | ||
| if let optionalValue = body as? Any?, optionalValue == nil { | ||
| implementation?.identify(identifier: identifier, body: EmptyRequestBody()) | ||
| return |
There was a problem hiding this comment.
personal preference opinion only,
if and else, over early return.
There was a problem hiding this comment.
Personal preference again but I would prefer guard which I think is more common in Swift and our codebase 😄
There was a problem hiding this comment.
i meant this, think this would have worked.
if let optionalValue = body as? Any?, optionalValue == nil {
implementation?.identify(identifier: identifier, body: EmptyRequestBody())
} else {
implementation?.identify(identifier: identifier, body: body)
}
There was a problem hiding this comment.
Yes, this would have worked. I understand it is a personal preference but does it affect the business logic and working of the fix ?
There was a problem hiding this comment.
No it doesn't and hence, didn't say so either. 😄
I was just responding to this reply, that the personal preference I was mentioning was utilizing the same logic for comparison. We choose to go with it or not thats another thing.
I do agree that there could be personal preferences but swift doesn't allow the conversion and hence the alternative approach has been followed here for comparison of a non-nil property with nil.
| // (refer https://github.com/customerio/customerio-ios/blob/94cbf686c3c2a405534cfe908f7166558a3e0b5d/Sources/Tracking/CustomerIO.swift#L71-L75) | ||
| if let optionalValue = body as? Any?, optionalValue == nil { | ||
| implementation?.identify(identifier: identifier, body: EmptyRequestBody()) | ||
| return |
There was a problem hiding this comment.
Personal preference again but I would prefer guard which I think is more common in Swift and our codebase 😄
|
@mrehan27 @Shahroz16 directly converting a non-optional property to optional isn't possible in Swift. This is one of the ways we can indirectly attain what's said above. I do agree that there could be personal preferences but swift doesn't allow the conversion and hence the alternative approach has been followed here for comparison of a non-nil property with |
|
I would like to request automated tests for this fix. Was there a reason for not adding them in the PR I am unaware of? |
We are updating the test cases separately as a part of clean up. |
## [3.0.0](2.12.5...3.0.0) (2024-03-20) ### ⚠ BREAKING CHANGES * iOS as a source for Data Pipelines (#659) ### Features * iOS as a source for Data Pipelines ([#659](#659)) ([0a68373](0a68373)) * migration module to cater to all migration tasks ([#530](#530)) ([2feb1d4](2feb1d4)) ### Bug Fixes * add attributes to properties ([#649](#649)) ([4b02e92](4b02e92)) * all sdk modules can only be initialized once ([ae46c7f](ae46c7f)) * app crash on identify method ([#458](#458)) ([13e9862](13e9862)) * compilation for test ([f14b773](f14b773)) * compilation issue ([420a61e](420a61e)) * eventbus handler ref ([#469](#469)) ([8c8ef91](8c8ef91)) * journey id in migration payload ([#653](#653)) ([3b649c9](3b649c9)) * prevent duplicate automatic screenview events from being tracked ([fea9ec5](fea9ec5)) * pushEventHandler test ([dc80fc2](dc80fc2)) * remove occurrence of autoTrackDeviceAttributes from all push modules ([#505](#505)) ([8dc6507](8dc6507)) * removed last_used from properties ([#477](#477)) ([b0b9631](b0b9631)) * sample app issues ([#551](#551)) ([05544b3](05544b3)) * use git commit instead of git branch for segment dependency ([d245015](d245015))
This PR fixes the following scenario:
Sample app was crashing in a scenario where the user sends
nilvalue for body inidentifymethod. The specific use case is when a user clicksGenerate Random Loginand attempts to login without a body.Complete each step to get your pull request merged in. Learn more about the workflow this project uses.