trackVerified() behavior by platform #121
Conversation
|
|
||
| class TrackAPI { | ||
| static async trackOnce(params: RadarTrackParams) { | ||
| static async trackOnce(params: RadarTrackParams, verified: boolean = false, encrypted: boolean = false, host: string | undefined = undefined) { |
There was a problem hiding this comment.
Lets just roll these into RadarTrackParams (maybe except for host since it's not send in the track request)
There was a problem hiding this comment.
Or add another object that is like verifiedOptions or something
static async trackOnce(params: RadarTrackParams, verifiedOptions?: RadarVerifiedOptions) {There was a problem hiding this comment.
I thought about adding it to RadarTrackParams, but we don't want to allow developers to pass it explicitly. More an internal implementation detail. Whether we pass encrypted = true (which we will prob rename or remove, btw) is implicit in whether the developer is calling trackVerified() or `trackVerifiedToken()
There was a problem hiding this comment.
We should still wrap it in an optional object. Otherwise it's hard to tell what the arguments are at the callsite:
TrackAPI.trackOnce(params, true, false, 'https://api-verified.radar.io');As opposed to:
TrackAPI.trackOnce(params, {
verified: true,
encrypted: false,
host: 'https://api-verified.radar.io',
});
| @@ -9,7 +9,7 @@ import Storage from '../storage'; | |||
| import type { RadarTrackParams, RadarTrackResponse, RadarTrackTokenResponse } from '../types'; | |||
|
|
|||
| class VerifyAPI { | |||
| static async trackVerified(params: RadarTrackParams, encrypted: Boolean = false) { | |||
| static async trackVerified(params: RadarTrackParams, encrypted: Boolean = false, host: string = 'http://localhost:52516') { | |||
There was a problem hiding this comment.
Same as above, lets just make encrypted part of track params
| @@ -132,7 +132,7 @@ class Http { | |||
| } | |||
|
|
|||
| xhr.onerror = function() { | |||
| if (host && host === 'https://radar-verify.com:52516') { | |||
| if (host && (host === 'https://radar-verify.com:52516' || host === 'http://localhost:52516')) { | |||
There was a problem hiding this comment.
if (host && host.includes('localhost:52516')) {
| @@ -11,6 +12,23 @@ const generateUUID = (): string => { | |||
| }; | |||
|
|
|||
| class Device { | |||
| static getPlatform(): string { | |||
| let userAgent = navigator.userAgent; | |||
There was a problem hiding this comment.
Any idea how reliable this is? UserAgent parsing can have a lot of edge cases
There was a problem hiding this comment.
I think it's the best option for this use case. There's also navigator.platform, but that's deprecated. We could also let the developer decide (e.g., Radar.trackVerified({ useDesktopApp: boolean = false })), but that just shifts the problem to the developer
| if (platform === 'DESKTOP_MAC') { // use Mac app | ||
| return VerifyAPI.trackVerified(params, false, 'https://radar-verify.com:52516'); | ||
| } else if (platform === 'DESKTOP_WINDOWS') { // use Windows app | ||
| return VerifyAPI.trackVerified(params, false, 'http://localhost:52516'); | ||
| } else { // bypass desktop app | ||
| try { | ||
| return TrackAPI.trackOnce(params, true, false, 'https://api-verified.radar.io'); | ||
| } finally { | ||
| ConfigAPI.getConfig(params); // call with updated permissions | ||
| } | ||
| } |
There was a problem hiding this comment.
Small nit, if each branch returns technically these don't need to be else if. A bit cleaner to read without them
if (platform === 'DESKTOP_MAC') { // use Mac app
return VerifyAPI.trackVerified(params, false, 'https://radar-verify.com:52516');
}
if (platform === 'DESKTOP_WINDOWS') { // use Windows app
return VerifyAPI.trackVerified(params, false, 'http://localhost:52516');
}
// bypass desktop app
try {
return TrackAPI.trackOnce(params, true, false, 'https://api-verified.radar.io');
} finally {
ConfigAPI.getConfig(params); // call with updated permissions
}
|
Closing in favor of simpler #126 |
No description provided.