Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

trackVerified() behavior by platform #121

Closed
wants to merge 15 commits into from
Closed

trackVerified() behavior by platform #121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
4cae05e
trackVerified() behavior by platform
nickpatrick Aug 28, 2023
91939eb
trackVerified() behavior by platform
nickpatrick Aug 28, 2023
1e1f2b0
bump version
nickpatrick Aug 28, 2023
ccdbdf6
bump version
nickpatrick Aug 28, 2023
6744b14
DESKTOP_MAC
nickpatrick Aug 28, 2023
6689d13
bump version
nickpatrick Aug 28, 2023
848c4ca
localhost
nickpatrick Aug 28, 2023
e900ce9
bump version
nickpatrick Aug 29, 2023
ceb102e
fix tests
nickpatrick Aug 29, 2023
1c4261c
fix tests
nickpatrick Aug 29, 2023
05afcb1
bump version
nickpatrick Aug 29, 2023
002f7ee
bump version
nickpatrick Aug 29, 2023
6a62840
handle error headers from desktop app
nickpatrick Aug 30, 2023
5710756
handle error codes from desktop app
nickpatrick Aug 30, 2023
c91f046
error handling
nickpatrick Sep 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ Radar.initialize('prj_test_pk_...', { /* options */ });

Add the following script in your `html` file
```html
<script src="https://js.radar.com/v4.1.5/radar.min.js"></script>
<script src="https://js.radar.com/v4.1.6-beta.1/radar.min.js"></script>
```

Then initialize the Radar SDK
Expand All @@ -73,8 +73,8 @@ To create a map, first initialize the Radar SDK with your publishable key. Then
```html
<html>
<head>
<link href="https://js.radar.com/v4.1.5/radar.css" rel="stylesheet">
<script src="https://js.radar.com/v4.1.5/radar.min.js"></script>
<link href="https://js.radar.com/v4.1.6-beta.1/radar.css" rel="stylesheet">
<script src="https://js.radar.com/v4.1.6-beta.1/radar.min.js"></script>
</head>

<body>
Expand All @@ -98,8 +98,8 @@ To create an autocomplete input, first initialize the Radar SDK with your publis
```html
<html>
<head>
<link href="https://js.radar.com/v4.1.5/radar.css" rel="stylesheet">
<script src="https://js.radar.com/v4.1.5/radar.min.js"></script>
<link href="https://js.radar.com/v4.1.6-beta.1/radar.css" rel="stylesheet">
<script src="https://js.radar.com/v4.1.6-beta.1/radar.min.js"></script>
</head>

<body>
Expand Down Expand Up @@ -130,8 +130,8 @@ To power [geofencing](https://radar.com/documentation/geofencing/overview) exper
```html
<html>
<head>
<link href="https://js.radar.com/v4.1.5/radar.css" rel="stylesheet">
<script src="https://js.radar.com/v4.1.5/radar.min.js"></script>
<link href="https://js.radar.com/v4.1.6-beta.1/radar.css" rel="stylesheet">
<script src="https://js.radar.com/v4.1.6-beta.1/radar.min.js"></script>
</head>

<body>
Expand Down
7 changes: 7 additions & 0 deletions demo/views/track-verified.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,13 @@
console.log(user);
console.log(events);

if (!location) {
$('#map').hide();
$('#response-details').hide();

return;
}

$('#map').show();
if (!map) {
map = Radar.ui.map({
Expand Down
4 changes: 2 additions & 2 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "radar-sdk-js",
"version": "4.1.5",
"version": "4.1.6-beta.1",
"description": "Web Javascript SDK for Radar, location infrastructure for mobile and web apps.",
"homepage": "https://radar.com",
"type": "module",
Expand Down
21 changes: 18 additions & 3 deletions src/api/track.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ import Storage from '../storage';

import TripsAPI from './trips';

import type { RadarTrackParams, RadarTrackResponse } from '../types';
import type { RadarTrackParams, RadarTrackResponse, RadarTrackTokenResponse } from '../types';

class TrackAPI {
static async trackOnce(params: RadarTrackParams) {
static async trackOnce(params: RadarTrackParams, verified: boolean = false, encrypted: boolean = false, host: string | undefined = undefined) {
Copy link
Collaborator

@kochis kochis Aug 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets just roll these into RadarTrackParams (maybe except for host since it's not send in the track request)

Copy link
Collaborator

@kochis kochis Aug 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or add another object that is like verifiedOptions or something

static async trackOnce(params: RadarTrackParams, verifiedOptions?: RadarVerifiedOptions) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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',
});

const options = Config.get();

let { latitude, longitude, accuracy, desiredAccuracy } = params;
Expand Down Expand Up @@ -75,17 +75,32 @@ class TrackAPI {
stopped: true,
userId,
tripOptions,
verified,
encrypted,
};

const response: any = await Http.request({
method: 'POST',
path: 'track',
data: body,
host,
});

const { user, events } = response;
const { user, events, token } = response;
const location = { latitude, longitude, accuracy };

if (encrypted) {
const trackTokenRes = {
token,
} as RadarTrackTokenResponse;

if (options.debug) {
trackTokenRes.response = response;
}

return trackTokenRes;
}

const trackRes = {
user,
events,
Expand Down
4 changes: 2 additions & 2 deletions src/api/verify.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, lets just make encrypted part of track params

const options = Config.get();

// user indentification fields
Expand Down Expand Up @@ -47,7 +47,7 @@ class VerifyAPI {
method: 'GET',
path: 'verify',
data: body,
host: 'https://radar-verify.com:52516',
host,
});

const { user, events, token } = response;
Expand Down
1 change: 1 addition & 0 deletions src/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ class Config {
live: false,
logLevel: 'error',
host: 'https://api.radar.io',
verifiedHost: 'https://api-verified.radar.io',
version: 'v1',
debug: false,
};
Expand Down
18 changes: 18 additions & 0 deletions src/device.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import Config from './config';
import Storage from './storage';

const generateUUID = (): string => {
Expand All @@ -11,6 +12,23 @@ const generateUUID = (): string => {
};

class Device {
static getPlatform(): string {
let userAgent = navigator.userAgent;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any idea how reliable this is? UserAgent parsing can have a lot of edge cases

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (userAgent) {
userAgent = userAgent.toLowerCase();
if (userAgent.includes('macintosh')) {
return 'DESKTOP_MAC';
} else if (userAgent.includes('windows')) {
return 'DESKTOP_WINDOWS';
} else if (userAgent.includes('iphone') || userAgent.includes('ipad') || userAgent.includes('ipod')) {
return 'MOBILE_IOS';
} else if (userAgent.includes('android')) {
return 'MOBILE_ANDROID';
}
}
return 'OTHER';
}

static getDeviceId(): string {
// use existing deviceId if present
const deviceId = Storage.getItem(Storage.DEVICE_ID);
Expand Down
11 changes: 10 additions & 1 deletion src/http.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {
RadarBadRequestError,
RadarDesktopAppError,
RadarForbiddenError,
RadarLocationError,
RadarLocationPermissionsError,
RadarNotFoundError,
RadarPaymentRequiredError,
RadarPublishableKeyError,
Expand Down Expand Up @@ -96,6 +98,13 @@ class Http {
return reject(new RadarServerError(response));
}

const error = response?.meta?.error;
if (error === 'ERROR_PERMISSIONS') {
return reject(new RadarLocationPermissionsError('Location permissions not granted'));
} else if (error === 'ERROR_LOCATION') {
return reject(new RadarLocationError('Could not determine location'));
}

if (xhr.status == 200) {
return resolve(response);
}
Expand Down Expand Up @@ -132,7 +141,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')) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

if (host && host.includes('localhost:52516')) {

reject(new RadarDesktopAppError());
} else {
reject(new RadarServerError());
Expand Down
35 changes: 30 additions & 5 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Config from './config';
import Device from './device';
import Logger from './logger';
import Storage from './storage';
import Navigator from './navigator';
Expand Down Expand Up @@ -33,6 +34,8 @@ import type {
RadarSearchGeofencesParams,
RadarSearchPlacesParams,
RadarTrackParams,
RadarTrackResponse,
RadarTrackTokenResponse,
RadarTripOptions,
RadarValidateAddressParams,
} from './types';
Expand Down Expand Up @@ -136,20 +139,42 @@ class Radar {
return Navigator.getCurrentPosition();
}

public static trackOnce(params: RadarTrackParams = {}) {
public static trackOnce(params: RadarTrackParams = {}): Promise<RadarTrackResponse> {
try {
return TrackAPI.trackOnce(params);
} finally {
ConfigAPI.getConfig(params); // call with updated permissions
}
}

public static trackVerified(params: RadarTrackParams = {}) {
return VerifyAPI.trackVerified(params);
public static trackVerified(params: RadarTrackParams = {}): Promise<RadarTrackResponse> {
const platform = Device.getPlatform();
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
}
}
Comment on lines +152 to +162
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can update

}

public static trackVerifiedToken(params: RadarTrackParams = {}) {
return VerifyAPI.trackVerified(params, true);
public static trackVerifiedToken(params: RadarTrackParams = {}): Promise<RadarTrackTokenResponse> {
const platform = Device.getPlatform();
if (platform === 'DESKTOP_MAC') { // use Mac app
return VerifyAPI.trackVerified(params, true, 'https://radar-verify.com:52516');
} else if (platform === 'DESKTOP_WINDOWS') { // use Windows app
return VerifyAPI.trackVerified(params, true, 'http://localhost:52516');
} else { // bypass desktop app
try {
return TrackAPI.trackOnce(params, true, true, 'https://api-verified.radar.io');
} finally {
ConfigAPI.getConfig(params); // call with updated permissions
}
}
}

public static getContext(params: Location) {
Expand Down
2 changes: 1 addition & 1 deletion src/version.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export default '4.1.5';
export default '4.1.6-beta.1';
2 changes: 1 addition & 1 deletion test/api/track.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ describe('Track', () => {
});

it('should call track with the users location and get a response', async () => {
const trackRes = await Track.trackOnce({
const trackRes: RadarTrackResponse = await Track.trackOnce({
userId: 'test-user',
tripOptions: {
externalId: 'test-trip',
Expand Down