Skip to content

Commit

Permalink
combine opcode.joinWithTokenOrUrl with dynamicConfig.initByUser
Browse files Browse the repository at this point in the history 
Looking at these 2 functions I realized they can be combined and error handling unified, so I combined these to make a new function using async/await syntax.

I chose to put the new function in dynamicConfig because this avoids a circular dependency at the module level (which is allowed but not tidy)
Now, `dynamicConfig.ts` imports from `opcode.ts` but `opcode.ts` no longer imports anything from `dynamicConfig.ts`.

Updated the tests for dynamicConfig to validate that the correct error messages are shown.
  • Loading branch information
@JGreenlee
JGreenlee committed Oct 17, 2024
1 parent 8fbe0e4 commit 3752e2c
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 61 deletions.
80 changes: 56 additions & 24 deletions www/__tests__/dynamicConfig.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { getConfig, initByUser } from '../js/config/dynamicConfig';

import { getConfig, joinWithTokenOrUrl } from '../js/config/dynamicConfig';
import initializedI18next from '../js/i18nextInit';
import { storageClear } from '../js/plugin/storage';
import i18next from '../js/i18nextInit';

window['i18next'] = initializedI18next;

beforeEach(() => {
Expand Down Expand Up @@ -56,6 +57,8 @@ global.fetch = (url: string) => {
}) as any;
};

const windowAlert = jest.spyOn(window, 'alert').mockImplementation(() => {});

describe('dynamicConfig', () => {
const fakeStudyName = 'gotham-city-transit';
const validStudyNrelCommute = 'nrel-commute';
Expand All @@ -65,9 +68,9 @@ describe('dynamicConfig', () => {
it('should resolve with null since no config is set yet', async () => {
await expect(getConfig()).resolves.toBeNull();
});
it('should resolve with a valid config once initByUser is called for an nrel-commute token', async () => {
it('should resolve with a valid config once joinWithTokenOrUrl is called for an nrel-commute token', async () => {
const validToken = `nrelop_${validStudyNrelCommute}_user1`;
await initByUser({ token: validToken });
await joinWithTokenOrUrl(validToken);
const config = await getConfig();
expect(config!.server.connectUrl).toBe('https://nrel-commute-openpath.nrel.gov/api/');
expect(config!.joined).toEqual({
Expand All @@ -77,9 +80,9 @@ describe('dynamicConfig', () => {
});
});

it('should resolve with a valid config once initByUser is called for a denver-casr token', async () => {
it('should resolve with a valid config once joinWithTokenOrUrl is called for a denver-casr token', async () => {
const validToken = `nrelop_${validStudyDenverCasr}_test_user1`;
await initByUser({ token: validToken });
await joinWithTokenOrUrl(validToken);
const config = await getConfig();
expect(config!.server.connectUrl).toBe('https://denver-casr-openpath.nrel.gov/api/');
expect(config!.joined).toEqual({
Expand All @@ -90,39 +93,68 @@ describe('dynamicConfig', () => {
});
});

describe('initByUser', () => {
describe('joinWithTokenOrUrl', () => {
// fake study (gotham-city-transit)
it('should error if the study is nonexistent', async () => {
it('returns false if the study is nonexistent', async () => {
const fakeBatmanToken = `nrelop_${fakeStudyName}_batman`;
await expect(initByUser({ token: fakeBatmanToken })).rejects.toThrow();
await expect(joinWithTokenOrUrl(fakeBatmanToken)).resolves.toBe(false);
expect(windowAlert).toHaveBeenLastCalledWith(
expect.stringContaining(i18next.t('config.unable-download-config')),
);
});

// real study without subgroups (nrel-commute)
it('should error if the study exists but the token is invalid format', async () => {
const badToken1 = validStudyNrelCommute; // doesn't start with nrelop_
await expect(initByUser({ token: badToken1 })).rejects.toThrow();
const badToken2 = `nrelop_${validStudyNrelCommute}`; // doesn't have enough _
await expect(initByUser({ token: badToken2 })).rejects.toThrow();
const badToken3 = `nrelop_${validStudyNrelCommute}_`; // doesn't have user code after last _
await expect(initByUser({ token: badToken3 })).rejects.toThrow();
it('returns false if the study exists but the token is invalid format', async () => {
const badToken1 = `nrelop_${validStudyNrelCommute}`; // doesn't have enough _
await expect(joinWithTokenOrUrl(badToken1)).resolves.toBe(false);
expect(windowAlert).toHaveBeenLastCalledWith(
expect.stringContaining(
i18next.t('config.not-enough-parts-old-style', { token: badToken1 }),
),
);

const badToken2 = `nrelop_${validStudyNrelCommute}_`; // doesn't have user code after last _
await expect(joinWithTokenOrUrl(badToken2)).resolves.toBe(false);
expect(windowAlert).toHaveBeenLastCalledWith(
expect.stringContaining(
i18next.t('config.not-enough-parts-old-style', { token: badToken2 }),
),
);

const badToken3 = `invalid_${validStudyNrelCommute}_user3`; // doesn't start with nrelop_
await expect(joinWithTokenOrUrl(badToken3)).resolves.toBe(false);
expect(windowAlert).toHaveBeenLastCalledWith(
expect.stringContaining(i18next.t('config.no-nrelop-start', { token: badToken3 })),
);
});
it('should return true after successfully storing the config for a valid token', async () => {

it('returns true after successfully storing the config for a valid token', async () => {
const validToken = `nrelop_${validStudyNrelCommute}_user2`;
await expect(initByUser({ token: validToken })).resolves.toBe(true);
await expect(joinWithTokenOrUrl(validToken)).resolves.toBe(true);
});

// real study with subgroups (denver-casr)
it('should error if the study uses subgroups but the token has no subgroup', async () => {
it('returns false if the study uses subgroups but the token has no subgroup', async () => {
const tokenWithoutSubgroup = `nrelop_${validStudyDenverCasr}_user2`;
await expect(initByUser({ token: tokenWithoutSubgroup })).rejects.toThrow();
await expect(joinWithTokenOrUrl(tokenWithoutSubgroup)).resolves.toBe(false);
expect(windowAlert).toHaveBeenLastCalledWith(
expect.stringContaining(
i18next.t('config.not-enough-parts', { token: tokenWithoutSubgroup }),
),
);
});
it('should error if the study uses subgroups and the token is invalid format', async () => {
it('returns false if the study uses subgroups and the token is invalid format', async () => {
const badToken1 = `nrelop_${validStudyDenverCasr}_test_`; // doesn't have user code after last _
await expect(initByUser({ token: badToken1 })).rejects.toThrow();
await expect(joinWithTokenOrUrl(badToken1)).resolves.toBe(false);
expect(windowAlert).toHaveBeenLastCalledWith(
expect.stringContaining(
i18next.t('config.not-enough-parts-old-style', { token: badToken1 }),
),
);
});
it('should return true after successfully storing the config for a valid token with subgroup', async () => {
it('returns true after successfully storing the config for a valid token with subgroup', async () => {
const validToken = `nrelop_${validStudyDenverCasr}_test_user2`;
await expect(initByUser({ token: validToken })).resolves.toBe(true);
await expect(joinWithTokenOrUrl(validToken)).resolves.toBe(true);
});
});
});
31 changes: 19 additions & 12 deletions www/js/config/dynamicConfig.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@ import { displayError, logDebug, logWarn } from '../plugin/logger';
import { fetchUrlCached } from '../services/commHelper';
import { storageClear, storageGet, storageSet } from '../plugin/storage';
import { AppConfig } from '../types/appConfigTypes';
import { getStudyNameFromToken, getStudyNameFromUrl, getSubgroupFromToken } from './opcode';
import {
getStudyNameFromToken,
getStudyNameFromUrl,
getSubgroupFromToken,
getTokenFromUrl,
} from './opcode';

export const CONFIG_PHONE_UI = 'config/app_ui_config';
export const CONFIG_PHONE_UI_KVSTORE = 'CONFIG_PHONE_UI';
Expand Down Expand Up @@ -179,26 +184,28 @@ export function loadNewConfig(newToken: string, existingVersion?: number): Promi
})
.catch((storeError) => {
displayError(storeError, i18next.t('config.unable-to-store-config'));
return Promise.reject(storeError);
return Promise.resolve(false);
});
})
.catch((fetchErr) => {
displayError(fetchErr, i18next.t('config.unable-download-config'));
return Promise.reject(fetchErr);
return Promise.resolve(false);
});
}

// exported wrapper around loadNewConfig that includes error handling
export function initByUser(urlComponents: { token: string }) {
const { token } = urlComponents;
export async function joinWithTokenOrUrl(tokenOrUrl: string) {
try {
return loadNewConfig(token).catch((fetchErr) => {
displayError(fetchErr, i18next.t('config.unable-download-config'));
return Promise.reject(fetchErr);
});
} catch (error) {
displayError(error, i18next.t('config.invalid-opcode-format'));
return Promise.reject(error);
const token = tokenOrUrl.includes('://') ? getTokenFromUrl(tokenOrUrl) : tokenOrUrl;
try {
return await loadNewConfig(token);
} catch (err) {
displayError(err, i18next.t('config.invalid-opcode-format'));
return false;
}
} catch (err) {
displayError(err, 'Error parsing token or URL: ' + tokenOrUrl);
return false;
}
}

Expand Down
26 changes: 1 addition & 25 deletions www/js/config/opcode.ts
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import i18next from 'i18next';
import { displayError, logDebug } from '../plugin/logger';
import { logDebug } from '../plugin/logger';
import AppConfig from '../types/appConfigTypes';
import { initByUser } from './dynamicConfig';
import { AlertManager } from '../components/AlertBar';

/**
* Adapted from https://stackoverflow.com/a/63363662/4040267
Expand Down Expand Up @@ -141,25 +139,3 @@ function getTokenFromUrl(url: string) {
throw new Error(`URL ${url} had path ${path}, expected 'join' or 'login_token'`);
}
}

export async function joinWithTokenOrUrl(tokenOrUrl: string) {
try {
const token = tokenOrUrl.includes('://') ? getTokenFromUrl(tokenOrUrl) : tokenOrUrl;
try {
const result = await initByUser({ token });
if (result) {
AlertManager.addMessage({
text: i18next.t('join.proceeding-with-token', { token }),
style: { wordBreak: 'break-all' },
});
}
return result;
} catch (err) {
displayError(err, 'Error logging in with token: ' + token);
return false;
}
} catch (err) {
displayError(err, 'Error parsing token or URL: ' + tokenOrUrl);
return false;
}
}

0 comments on commit 3752e2c

Please sign in to comment.