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.

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

feat(api-core): helper function for settings api #64

Merged
merged 2 commits into from
Aug 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion packages/api-angular/src/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ import angular from 'angular';

import { AvSettings } from '@availity/api-core';

export default ($http, $q, avApiOptions) =>
export default ($http, $q, avUsersApi, avApiOptions) =>
new AvSettings({
http: $http,
promise: $q,
merge: angular.merge,
avUsers: avUsersApi,
config: angular.copy(avApiOptions),
});
2 changes: 2 additions & 0 deletions packages/api-axios/src/settings.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import axios from 'axios';
import merge from 'deep-assign';
import { AvSettings } from '@availity/api-core';
import userApi from './user';

export default new AvSettings({
http: axios,
promise: Promise,
merge,
avUsers: userApi,
});
40 changes: 39 additions & 1 deletion packages/api-core/src/resources/settings.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import AvApi from '../api';

export default class AvSettings extends AvApi {
constructor({ http, promise, merge, config }) {
constructor({ http, promise, merge, avUsers, config }) {
const options = Object.assign(
{
path: 'api/utils',
name: 'settings',
sessionBust: false,
pageBust: true,
},
config
);
Expand All @@ -15,5 +17,41 @@ export default class AvSettings extends AvApi {
merge,
config: options,
});
this.avUsers = avUsers;
Copy link
Collaborator

@TheSharpieOne TheSharpieOne Aug 22, 2018

Choose a reason for hiding this comment

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

Probably should throw if avUser is not provided here.

Or throw in the methods where it is used so that the other methods can still be used without needing it (maybe a less breaking change).

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 added it to the other implementations, that pattern makes sense but might be worth saving for a refactor PR for all the other apis with similar dependencies.

Copy link
Contributor

Choose a reason for hiding this comment

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

@KaseyPowers changes LGTM. Can you open a ticket with the refactor suggestions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created #66

}

getApplication(applicationId, config) {
if (!applicationId) {
throw new Error('must define applicationId');
}
return this.avUsers.me().then(user => {
const queryConfig = this.addParams(
{ applicationId, userId: user.id },
config
);
return this.query(queryConfig);
});
}

setApplication(applicaitonId, data, config) {
if (
typeof applicaitonId !== 'string' &&
typeof applicaitonId !== 'number'
) {
config = data;
data = applicaitonId;
applicaitonId = '';
}

if (!applicaitonId && (!data.scope || !data.scope.applicationId)) {
throw new Error('must set applicationId in settings call');
}

return this.avUsers.me().then(user => {
data.scope = data.scope || {};
data.scope.applicationId = applicaitonId;
data.scope.userId = user.id;
return this.update(data, config);
});
}
}
109 changes: 58 additions & 51 deletions packages/api-core/src/resources/tests/settings.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,81 +3,88 @@ import AvSettings from '../settings';
const mockHttp = jest.fn(() => Promise.resolve({}));
const mockMerge = jest.fn((...args) => Object.assign(...args));

const mockConfig = {
scope: {
applicationId: '123',
userId: 'myUser',
},
const testAppId = 'testApplicationId';

const mockUser = {
id: 'mockUserId',
};
const mockAvUsers = {
me: jest.fn(() => Promise.resolve(mockUser)),
};

describe('AvSettings', () => {
let api;

test('should be defined', () => {
beforeEach(() => {
api = new AvSettings({
http: mockHttp,
promise: Promise,
merge: mockMerge,
avUsers: mockAvUsers,
config: {},
});
expect(api).toBeDefined();
});

test('should handle no config passed in', () => {
api = new AvSettings({
http: mockHttp,
promise: Promise,
merge: mockMerge,
});
test('should be defined', () => {
expect(api).toBeDefined();
});

test('url should be correct', () => {
api = new AvSettings({
http: mockHttp,
promise: Promise,
merge: mockMerge,
describe('getApplication', () => {
beforeEach(() => {
api.query = jest.fn();
});
test('should call avUsers.me and use in query', async () => {
const expectedQuery = {
params: {
applicationId: testAppId,
userId: mockUser.id,
},
};
await api.getApplication(testAppId);
expect(mockAvUsers.me).toHaveBeenCalled();
expect(api.query).toHaveBeenCalledWith(expectedQuery);
});

test('should throw error if no applicationId passed in', () => {
expect(() => api.getApplication()).toThrow();
});
expect(api.getUrl(api.config(mockConfig))).toBe('/api/utils/v1/settings');
});

test('query() should be called with params', () => {
api = new AvSettings({
http: mockHttp,
promise: Promise,
merge: mockMerge,
config: {},
describe('setApplication', () => {
beforeEach(() => {
api.update = jest.fn();
});

const data = {
params: {
applicationId: '123',
userId: 'myUser',
},
};
test('should add applicationId and user.me to scope', async () => {
const testData = { key: 'value' };
const testConfig = {};
const expectedUpdate = Object.assign(
{
scope: {
applicationId: testAppId,
userId: mockUser.id,
},
},
testData
);

api.query = jest.fn();
api.query(data);
expect(api.query).toHaveBeenLastCalledWith(data);
});
await api.setApplication(testAppId, testData, testConfig);
expect(mockAvUsers.me).toHaveBeenCalled();
expect(api.update).toHaveBeenCalledWith(expectedUpdate, testConfig);
});

test('update() should be called with scope', async () => {
api = new AvSettings({
http: mockHttp,
promise: Promise,
merge: mockMerge,
config: {},
test('should not throw error if application id passed in as arugment', () => {
expect(() => api.setApplication(testAppId, {})).not.toThrow();
});

test('should not throw error if applicationId in scope', () => {
expect(() =>
api.setApplication({ scope: { applicationId: testAppId } })
).not.toThrow();
});
const data = {
scope: {
applicationId: '123',
userId: 'myUser',
},
key: 'value',
};

api.update = jest.fn();
api.update(data);
expect(api.update).toHaveBeenLastCalledWith(data);
test('should throw error if no applicationId in argument or data', () => {
expect(() => api.setApplication()).toThrow();
});
});
});