[MOB-7218] impression tracking rework #650
Conversation
There was a problem hiding this comment.
Overall this PR looks good as it replaces the string with Long. But this might cause us some trouble in cases when data has to be passed to backend or typecasted. There could be loss in precision which might lead to incorrect placementIds being referenced. However, this is still good and we have to be mindful to use strings if at all such problems occur
| @@ -1,12 +1,12 @@ | |||
| { | |||
| "placements": [ | |||
| { | |||
| "placementId": "0", | |||
| "placementId": 411, | |||
There was a problem hiding this comment.
We should have a really long value here for testing purpose. I worry if there will be some data loss in json conversion or when requests are to be made and we loose the precision of the long
There was a problem hiding this comment.
Hm true since the json does not allow for longs. We will have to keep this in mind. Merging for now. Will revisit later.
| @@ -205,7 +205,7 @@ class IterableEmbeddedPlacementTest { | |||
|
|
|||
| val embeddedMessage = IterableEmbeddedMessage(embeddedMessageMetadata, embeddedMessageElements, customPayload) | |||
|
|
|||
| val placementId: String = "0" | |||
| val placementId: Long = 411L | |||
There was a problem hiding this comment.
I hope this 'L' doesnt cause much trouble in future. If it does, it will be wise to typecast all placementId into string.
🔹 Jira Ticket(s) if any
✏️ Description