Skip to content

[66695] Refactor Restfulmodel #267

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

Merged
mrashed-dev merged 5 commits into from
Sep 3, 2021
Merged

[66695] Refactor Restfulmodel #267

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c57e74b
Initial commit of refactor
mrashed-dev Aug 18, 2021
70c1954
Only a couple of test case fixes
mrashed-dev Aug 18, 2021
5e2425e
Update typing for model and collection related classes
mrashed-dev Aug 20, 2021
77d7945
Remove private keyword for path
mrashed-dev Aug 20, 2021
892f3b5
Applied `private` and `protected` method modifiers
mrashed-dev Sep 2, 2021
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
2 changes: 1 addition & 1 deletion __tests__/message-spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ describe('Message', () => {
Message,
testContext.connection
);
testContext.collection._getModelCollection = jest.fn(() => {
testContext.collection.getModelCollection = jest.fn(() => {
return Promise.resolve([testContext.message]);
});
});
Expand Down
16 changes: 9 additions & 7 deletions __tests__/neural-spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ describe('Neural', () => {
},
];

testContext.connection.neural._request = jest.fn(() =>
testContext.connection.neural.request = jest.fn(() =>
Promise.resolve(serverResponse)
);
});
Expand Down Expand Up @@ -106,7 +106,7 @@ describe('Neural', () => {
text: 'This is some text',
};

testContext.connection.neural._request = jest.fn((path, body) => {
testContext.connection.neural.request = jest.fn((path, body) => {
if (body['message_id']) {
return Promise.resolve([sentiment]);
}
Expand All @@ -127,7 +127,7 @@ describe('Neural', () => {
.sentimentAnalysisMessage(['abc123'])
.then(convoList => {
const sentBody =
testContext.connection.neural._request.mock.calls[0][1];
testContext.connection.neural.request.mock.calls[0][1];
expect(sentBody).toEqual({ message_id: ['abc123'] });
expect(convoList.length).toEqual(1);
evaluateSentiment(convoList[0]);
Expand All @@ -140,7 +140,7 @@ describe('Neural', () => {
.sentimentAnalysisText('This is some text')
.then(convo => {
const sentBody =
testContext.connection.neural._request.mock.calls[0][1];
testContext.connection.neural.request.mock.calls[0][1];
expect(sentBody).toEqual({ text: 'This is some text' });
evaluateSentiment(convo);
done();
Expand Down Expand Up @@ -195,7 +195,7 @@ describe('Neural', () => {
},
];

testContext.connection.neural._request = jest.fn(() =>
testContext.connection.neural.request = jest.fn(() =>
Promise.resolve(serverResponse)
);
});
Expand Down Expand Up @@ -235,7 +235,9 @@ describe('Neural', () => {
return testContext.connection.neural
.extractSignature(['abc123'])
.then(sigList => {
const contact = sigList[0].contacts.toContactObject();
const contact = sigList[0].contacts.toContactObject(
testContext.connection
);

expect(contact.givenName).toEqual('Nylas');
expect(contact.surname).toEqual('Swag');
Expand Down Expand Up @@ -359,7 +361,7 @@ describe('Neural', () => {
size: 20,
};

testContext.connection.neural._request = jest.fn(() =>
testContext.connection.neural.request = jest.fn(() =>
Promise.resolve(serverResponse)
);
});
Expand Down
6 changes: 3 additions & 3 deletions __tests__/nylas-connection-spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,21 @@ describe('NylasConnection', () => {
});
describe('mismatched api version warnings', () => {
test('should not warn if Nylas API version matches SDK supported API version', () => {
const noWarning = testContext.connection._getWarningForVersion(
const noWarning = testContext.connection.getWarningForVersion(
'2.0',
'2.0'
);
expect(noWarning).toEqual('');

const warnSdk = testContext.connection._getWarningForVersion(
const warnSdk = testContext.connection.getWarningForVersion(
'1.0',
'2.0'
);
expect(warnSdk).toEqual(
`WARNING: SDK version may not support your Nylas API version. SDK supports version 1.0 of the API and your application is currently running on version 2.0 of the API. Please update the sdk to ensure it works properly.`
);

const warnApi = testContext.connection._getWarningForVersion(
const warnApi = testContext.connection.getWarningForVersion(
'2.0',
'1.0'
);
Expand Down
54 changes: 27 additions & 27 deletions __tests__/restful-model-collection-spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ describe('RestfulModelCollection', () => {
threadsResponses.push(response);
}

testContext.collection._getModelCollection = jest.fn((params, offset) =>
testContext.collection.getModelCollection = jest.fn((params, offset) =>
Promise.resolve(threadsResponses[offset / 100])
);
});
Expand All @@ -66,7 +66,7 @@ describe('RestfulModelCollection', () => {
() => {},
() => {}
);
expect(testContext.collection._getModelCollection).toHaveBeenCalledWith(
expect(testContext.collection.getModelCollection).toHaveBeenCalledWith(
params,
0,
100,
Expand All @@ -81,16 +81,16 @@ describe('RestfulModelCollection', () => {
() => {},
() => {
expect(
testContext.collection._getModelCollection.mock.calls[0]
testContext.collection.getModelCollection.mock.calls[0]
).toEqual([{ from: 'ben@nylas.com' }, 0, 100, '/threads']);
expect(
testContext.collection._getModelCollection.mock.calls[1]
testContext.collection.getModelCollection.mock.calls[1]
).toEqual([{ from: 'ben@nylas.com' }, 100, 100, '/threads']);
expect(
testContext.collection._getModelCollection.mock.calls[2]
testContext.collection.getModelCollection.mock.calls[2]
).toEqual([{ from: 'ben@nylas.com' }, 200, 100, '/threads']);
expect(
testContext.collection._getModelCollection.mock.calls[3]
testContext.collection.getModelCollection.mock.calls[3]
).toEqual([{ from: 'ben@nylas.com' }, 300, 100, '/threads']);
done();
}
Expand Down Expand Up @@ -202,7 +202,7 @@ describe('RestfulModelCollection', () => {
},
};
testContext.items = [testContext.item];
testContext.collection._getModelCollection = jest.fn(() => {
testContext.collection.getModelCollection = jest.fn(() => {
return Promise.resolve(testContext.items);
});
});
Expand All @@ -215,7 +215,7 @@ describe('RestfulModelCollection', () => {
})
.then(msg => {
expect(
testContext.collection._getModelCollection
testContext.collection.getModelCollection
).toHaveBeenCalledWith(
{ from: 'ben@nylas.com', view: 'expanded' },
0,
Expand Down Expand Up @@ -253,7 +253,7 @@ describe('RestfulModelCollection', () => {
describe('when the request fails', () => {
beforeEach(() => {
testContext.error = new Error('Network error');
testContext.collection._getModelCollection = jest.fn(() => {
testContext.collection.getModelCollection = jest.fn(() => {
return Promise.reject(testContext.error);
});
});
Expand All @@ -278,12 +278,12 @@ describe('RestfulModelCollection', () => {

describe('list', () => {
test('should call range() with an infinite range', () => {
testContext.collection._range = jest.fn();
testContext.collection.range = jest.fn();

const params = { from: 'ben@nylas.com' };
const callback = () => {};
testContext.collection.list(params, callback);
expect(testContext.collection._range).toHaveBeenCalledWith({
expect(testContext.collection.range).toHaveBeenCalledWith({
params,
limit: Infinity,
callback,
Expand Down Expand Up @@ -714,11 +714,11 @@ describe('RestfulModelCollection', () => {

describe('path', () => {
test("should return the modelClass' collectionName with no prefix", () => {
expect(testContext.collection.path()).toEqual('/threads');
expect(testContext.collection.path).toEqual('/threads');
});
});

describe('_range', () => {
describe('range', () => {
beforeEach(() => {
const threadsResponses = [];
for (let x = 0; x <= 3; x++) {
Expand All @@ -738,15 +738,15 @@ describe('RestfulModelCollection', () => {
threadsResponses.push(response);
}

testContext.collection._getModelCollection = jest.fn((params, offset) =>
testContext.collection.getModelCollection = jest.fn((params, offset) =>
Promise.resolve(threadsResponses[offset / 100])
);
});

test('should fetch once if fewer than one page of models are requested', () => {
const params = { from: 'ben@nylas.com' };
testContext.collection._range({ params, offset: 0, limit: 50 });
expect(testContext.collection._getModelCollection).toHaveBeenCalledWith(
testContext.collection.range({ params, offset: 0, limit: 50 });
expect(testContext.collection.getModelCollection).toHaveBeenCalledWith(
params,
0,
50,
Expand All @@ -758,16 +758,16 @@ describe('RestfulModelCollection', () => {
const params = { from: 'ben@nylas.com' };
const callback = () => {
expect(
testContext.collection._getModelCollection.mock.calls[0]
testContext.collection.getModelCollection.mock.calls[0]
).toEqual([{ from: 'ben@nylas.com' }, 0, 100, '/threads']);
expect(
testContext.collection._getModelCollection.mock.calls[1]
testContext.collection.getModelCollection.mock.calls[1]
).toEqual([{ from: 'ben@nylas.com' }, 100, 100, '/threads']);
expect(
testContext.collection._getModelCollection.mock.calls[2]
testContext.collection.getModelCollection.mock.calls[2]
).toEqual([{ from: 'ben@nylas.com' }, 200, 100, '/threads']);
};
testContext.collection._range({
testContext.collection.range({
params,
callback,
offset: 0,
Expand All @@ -779,19 +779,19 @@ describe('RestfulModelCollection', () => {
const params = { from: 'ben@nylas.com' };
const callback = () => {
expect(
testContext.collection._getModelCollection.mock.calls[0]
testContext.collection.getModelCollection.mock.calls[0]
).toEqual([{ from: 'ben@nylas.com' }, 0, 100, '/threads']);
expect(
testContext.collection._getModelCollection.mock.calls[1]
testContext.collection.getModelCollection.mock.calls[1]
).toEqual([{ from: 'ben@nylas.com' }, 100, 100, '/threads']);
expect(
testContext.collection._getModelCollection.mock.calls[2]
testContext.collection.getModelCollection.mock.calls[2]
).toEqual([{ from: 'ben@nylas.com' }, 200, 100, '/threads']);
expect(
testContext.collection._getModelCollection.mock.calls[3]
testContext.collection.getModelCollection.mock.calls[3]
).toEqual([{ from: 'ben@nylas.com' }, 300, 100, '/threads']);
};
testContext.collection._range({
testContext.collection.range({
params,
callback,
offset: 0,
Expand All @@ -801,7 +801,7 @@ describe('RestfulModelCollection', () => {

test('should call the callback with all of the loaded models', done => {
const params = { from: 'ben@nylas.com' };
testContext.collection._range({
testContext.collection.range({
params,
offset: 0,
limit: 10000,
Expand All @@ -815,7 +815,7 @@ describe('RestfulModelCollection', () => {
test('should resolve with the loaded models', done => {
const params = { from: 'ben@nylas.com' };
testContext.collection
._range({ params, offset: 0, limit: 10000 })
.range({ params, offset: 0, limit: 10000 })
.then(function(models) {
expect(models.length).toBe(313);
done();
Expand Down
22 changes: 16 additions & 6 deletions src/models/attributes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import Model from './model';
import RestfulModel from './restful-model';

// The Attribute class represents a single model attribute, like 'namespace_id'
Expand Down Expand Up @@ -33,7 +34,7 @@ export class Attribute {
}

class AttributeObject extends Attribute {
itemClass?: typeof RestfulModel;
itemClass?: typeof Model | typeof RestfulModel;

constructor({
modelKey,
Expand All @@ -43,7 +44,7 @@ class AttributeObject extends Attribute {
}: {
modelKey: string;
jsonKey?: string;
itemClass?: typeof RestfulModel;
itemClass?: typeof Model | typeof RestfulModel;
readOnly?: boolean;
}) {
super({ modelKey, jsonKey, readOnly });
Expand All @@ -64,7 +65,11 @@ class AttributeObject extends Attribute {
if (!val || !this.itemClass) {
return val;
}
return new this.itemClass(_parent.connection, val);

if (this.itemClass.prototype instanceof RestfulModel) {
return new this.itemClass(_parent.connection, val);
}
return new this.itemClass(val);
}
}

Expand Down Expand Up @@ -155,7 +160,7 @@ class AttributeDateTime extends Attribute {
}

class AttributeCollection extends Attribute {
itemClass: typeof RestfulModel;
itemClass: typeof Model | typeof RestfulModel;

constructor({
modelKey,
Expand All @@ -165,7 +170,7 @@ class AttributeCollection extends Attribute {
}: {
modelKey: string;
jsonKey?: string;
itemClass: typeof RestfulModel;
itemClass: typeof Model | typeof RestfulModel;
readOnly?: boolean;
}) {
super({ modelKey, jsonKey, readOnly });
Expand Down Expand Up @@ -193,7 +198,12 @@ class AttributeCollection extends Attribute {
}
const objs = [];
for (const objJSON of json) {
const obj = new this.itemClass(_parent.connection, objJSON);
let obj;
if (this.itemClass.prototype instanceof RestfulModel) {
Comment thread
mrashed-dev marked this conversation as resolved.
obj = new this.itemClass(_parent.connection, objJSON);
} else {
obj = new this.itemClass(objJSON);
}
objs.push(obj);
}
return objs;
Expand Down
4 changes: 2 additions & 2 deletions src/models/calendar.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ export default class Calendar extends RestfulModel {
isPrimary?: boolean;
jobStatusId?: string;

save(params: {} | SaveCallback = {}, callback?: SaveCallback) {
return this._save(params, callback);
protected save(params: {} | SaveCallback = {}, callback?: SaveCallback) {
return super.save(params, callback);
}

saveRequestBody() {
Expand Down
2 changes: 1 addition & 1 deletion src/models/contact-restful-model-collection.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export default class ContactRestfulModelCollection extends RestfulModelCollectio
})
.then(json => {
const groups = json.map((group: { [key: string]: any }) => {
return new Group(this.connection, group);
return new Group(group);
});
if (callback) {
callback(null, groups);
Expand Down
Loading