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

Implement initial default adapter for upcoming adapter pattern #51

Merged
merged 23 commits into from
Nov 28, 2017
Merged

Implement initial default adapter for upcoming adapter pattern #51

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
c9c2a13
Add initial adapter pattern for collection.js
Nov 10, 2017
396fded
Add remaining adapter methods to existing actions
Nov 13, 2017
594737f
Fix linting
Nov 13, 2017
eb7f531
Move storageManager actions to shared adapter in in adapters/default.js
Nov 13, 2017
4dda476
Add missing adapters foldee to git
Nov 13, 2017
daf3c01
revert to basic function pattern
Nov 13, 2017
f3c7017
Make dispatch and getState available as options to all adapter actions
Nov 14, 2017
ab39813
Added collectionToIds and collectionToArray to the default adapter
Nov 15, 2017
5807638
Conditionally pass in adapter when its defined in typeOptions
Nov 15, 2017
3ee071f
Add the SYNC_COLLECTIONS reducer that merges new collections into the…
Nov 16, 2017
4fe6296
add additional makeBrainstemType actions that use adapter and add nee…
Nov 20, 2017
31f9d98
fix issues with validate + destroy specs
Nov 21, 2017
2656bf3
move option building to shared method
Nov 21, 2017
5e50c93
bump version to next beta
Nov 27, 2017
a0ea814
Prefer Object.assign with an empty object to prevent mutating the ori…
Nov 27, 2017
ab98fe0
Fix typo in spec assertion
Nov 27, 2017
201b48f
Add directions for running and testing example code in README
Nov 27, 2017
a1be14f
Use Object.assign with a new empty object in favor of mutating the ex…
Nov 28, 2017
2785ac7
Merge branch 'add_adapter_pattern' of https://github.com/mavenlink/br…
heyellieday Nov 28, 2017
b0d6aba
Merge branch 'add_adapter_pattern' of github.com:mavenlink/brainstem-…
shirish-pampoorickal Nov 28, 2017
424dd71
Bump version number to 0.0.35
Nov 28, 2017
1c56c06
Merge pull request #50 from mavenlink/make_brainstem_type_options
heyellieday Nov 28, 2017
e13303e
Merge branch 'master' into add_adapter_pattern
Nov 28, 2017
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 lib/actions/collection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const storageManagerAdapter = {
return storageManager.storage(brainstemKey)
.fetch(Object.assign(options.fetchOptions, { remove: false }));
},
}
};

module.exports = {
fetch(brainstemKey, options = {}) {
Expand Down
9 changes: 3 additions & 6 deletions lib/actions/model.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ const storageManagerAdapter = {
const storageManager = StorageManager.get();
const Model = storageManager.storage(brainstemKey).model;
return new Model().validate(attributes, options.validateOptions);
}
}
},
};

module.exports = {
fetch(brainstemKey, modelId, options = {}) {
Expand Down Expand Up @@ -62,8 +62,6 @@ module.exports = {
adapter = storageManagerAdapter,
} = options;

const storageManager = StorageManager.get();

if (xhrs[trackKey] && xhrs[trackKey].state() === 'pending') xhrs[trackKey].abort();

return (dispatch) => {
Expand Down Expand Up @@ -92,7 +90,6 @@ module.exports = {
},

destroy(brainstemKey, modelId, options = {}) {
const storageManager = StorageManager.get();
const {
deleteOptions = {},
trackKey,
Expand Down Expand Up @@ -121,7 +118,7 @@ module.exports = {
return (dispatch) => {
if (preValidateAction) dispatch(preValidateAction);

const validationErrors = adapter.validateModel(brainstemKey, attributes, { validateOptions })
const validationErrors = adapter.validateModel(brainstemKey, attributes, { validateOptions });

if (postValidateAction) { dispatch(postValidateAction(validationErrors)); }

Expand Down
6 changes: 3 additions & 3 deletions spec/actions/model-spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ describe('model action creators', () => {
}));

expect(stubAdapter.saveModel).toHaveBeenCalledWith('posts', '76', attributes, {
saveOptions
saveOptions,
});
});
});
Expand All @@ -205,7 +205,7 @@ describe('model action creators', () => {
this.destroy = modelActions.destroy;
});

it('calls destroy on the adapter', function () {
it('calls destroy on the adapter', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

uh, bad merge conflict? Let's not nest it blocks!

Copy link
Contributor

Choose a reason for hiding this comment

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

What is going on here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To reply to your past few comments re: "what is going on here", I'd suggest looking at the diff side by side as it will be easier to see that the existing specs were wrapped in an additional describe block. This causes the diff to appear a bit wonky, which you've likely seen before. I hope that clears things up!

Copy link
Contributor

Choose a reason for hiding this comment

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

There are nested it specs in it specs. Can we fix that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I see what you mean. Fixed it!

it('calls fetch on the adapter', function () {
const deferred = $.Deferred(); // eslint-disable-line new-cap
const stubAdapter = {
Expand All @@ -227,7 +227,7 @@ describe('model action creators', () => {
this.destroy = modelActions.destroy;
});

it('calls destroy on the adapter', function () {
it('calls destroy on the adapter', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is going on here.

Copy link
Contributor

Choose a reason for hiding this comment

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

There are nested it specs in it specs. Can we fix that?

it('calls fetch on the adapter', function () {
const deferred = $.Deferred(); // eslint-disable-line new-cap
const stubAdapter = {
Expand Down