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

Fix isUpdating for DS.AdapterPopulatedRecordArray#update() #4316

Merged
merged 2 commits into from
Apr 9, 2016
Merged

Fix isUpdating for DS.AdapterPopulatedRecordArray#update() #4316

merged 2 commits into from
Apr 9, 2016

Conversation

pangratz
Copy link
Member

@pangratz pangratz commented Apr 9, 2016

The isUpdating flag is not set to true when the update() method on
a DS.AdapterPopulatedRecordArray is invoked. As with the flag on
DS.RecordArray, this should be true until the update is finished and
the array contains the most reset result for the query, fetched from the
adapter.


The [FEATURE] commit DRY's up some code in the DS.AdapterPopulatedRecordArray and by this makes it easier to cherry-pick this bugfix into beta (where this feature is not yet available).

@@ -57,6 +57,42 @@ test("When a query is made, the adapter should receive a record array it can pop
}));
});

test("a query can be updated via `update()`", function(assert) {
let done = assert.async();
Copy link
Member

Choose a reason for hiding this comment

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

Is this done needed? I thought the run loop would remove the need for this?

Copy link
Member Author

Choose a reason for hiding this comment

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

You are absolutely right 👍 Removed the done and the amount of expected assertions, since this is handled by the run-loop...

@bmac
Copy link
Member

bmac commented Apr 9, 2016

Overall looks good.

The `isUpdating` flag is not set to `true` when the `update()` method on
a `DS.AdapterPopulatedRecordArray` is invoked. As with the flag on
`DS.RecordArray`, this should be `true` until the update is finished and
the array contains the most reset result for the query, fetched from the
adapter.
@bmac bmac merged commit c628d74 into emberjs:master Apr 9, 2016
@pangratz pangratz deleted the fix-adapter-populated-record-array-update branch April 10, 2016 08:00
bmac pushed a commit that referenced this pull request May 17, 2016
A regression for the `isUpdating` flag on the RecordArray returned for
`store.peekAll` has been introduced in #4316: the flag isn't set to true
anymore when records are reloaded or a background reloaded.

(cherry picked from commit 50b4c27)
bmac pushed a commit that referenced this pull request May 17, 2016
A regression for the `isUpdating` flag on the RecordArray returned for
`store.peekAll` has been introduced in #4316: the flag isn't set to true
anymore when records are reloaded or a background reloaded.

(cherry picked from commit 50b4c27)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants