Update Magnes to 5.6.0 and getClientMetadataId to receive a callback
#1521
Conversation
Dependency Review✅ No vulnerabilities found.Scanned FilesNone |
| return result.paypalClientMetaDataId | ||
| ) { status, clientMetadataId -> | ||
| // Callback is invoked when device data collection and submit API completes | ||
| callback(clientMetadataId) |
There was a problem hiding this comment.
Should we be checking that the status is a success before returning the callback here? For the timeout and error status we can instead return an empty string or an error.
There was a problem hiding this comment.
I added the check for the status in b52b032, let me know if it is sufficient!
CHANGELOG.md
Outdated
| * PayPal | ||
| * Pass `funding_source` to the app switch url link and to analytics events. | ||
|
|
||
|
|
There was a problem hiding this comment.
Looks like we've got a double space here now
saralvasquez
left a comment
saralvasquez
left a comment
There was a problem hiding this comment.
Couple small nits, but looks good otherwise!
| } | ||
|
|
||
| Assert.assertTrue(result.isEmpty()) | ||
| Assert.assertNotNull(receivedClientMetadataId) |
There was a problem hiding this comment.
Do we need the extra NotNull assert here since you've got the !! below?
| } | ||
|
|
||
| Assert.assertTrue(result.isEmpty()) | ||
| Assert.assertNotNull(receivedClientMetadataId) |
There was a problem hiding this comment.
Same question as above
|
^Although I haven't tested on physical device yet... |
|
|
||
| val sut = MagnesInternalClient(magnesSDK) | ||
| val result = sut.getClientMetadataId(context, prodConfiguration, requestWithInvalidGUID) | ||
| var receivedClientMetadataId: String? = null |
There was a problem hiding this comment.
| var receivedClientMetadataId: String? = null | |
| var receivedClientMetadataId: String = "non-empty" |
would this work for your use-case? you could potentially remove the !! in the assert.
There was a problem hiding this comment.
thank you for this suggestion, I have updated the test not to use !! in 123ebec!
| hasUserLocationConsent: Boolean | ||
| ): String { | ||
| hasUserLocationConsent: Boolean, | ||
| callback: (String) -> Unit |
There was a problem hiding this comment.
Is either this or the below changing the public interface for merchants that use the data collector directly? I am not sure how the RestrictTo logic works.
There was a problem hiding this comment.
As far as I understand @RestrictTo means this function is internal to our library group - merchants shouldn't be calling it directly, the only function that merchants should call is collectDeviceData.
|
Ok I tested on emulator and physical device and paypal and local payments flows seem to be working as expected |
Summary of changes
getClientMetadataIdwhen the submit completesgetClientMetadataIdChecklist
Authors