Fix double loadData on page turns

[hbrysiewicz]

I found a bug with pagination where the loadData task was getting called twice. It looks like it is still happening even in later versions, even though we are still on v0.1.1.

Since didChangeAttrs is called when any of YetiTables attributes change, and didChangeAttrs triggers a loadData task refire, it is not necessary to manually call this.loadData() from the prevPage, nextPage, or goToPage methods on the YetiTable component.

In examples in documentation, yield timeout(250) is used on a restartable() task, so this behavior is hidden. Ideally, a page turn would be immediate and you should not need a yield timeout(250). This timeout was a bandaid over this issue.

[miguelcobain]

@hbrysiewicz what you're saying makes sense to me but for some reason this isn't passing the tests: https://travis-ci.org/github/miguelcobain/ember-yeti-table/jobs/727424729

🤔

[miguelcobain]

Also, it's weird because we have tests that make sure that load data is ran the correct number of times: https://github.com/miguelcobain/ember-yeti-table/blob/master/tests/integration/components/yeti-table/async-data-test.js#L486-L570

more specifically these lines:

assert.ok(this.loadData.calledOnce, 'loadData was called once');
// ...
assert.ok(this.loadData.calledTwice, 'loadData was called twice');

[hbrysiewicz]

Yeah I'll take look at those tests later today. If you're following the same pattern in your examples though, with the yield timeout(250) then yes, your tests would pass and my change would make them fail.

[hbrysiewicz]

@miguelcobain Tests should be fixed

[miguelcobain]

@hbrysiewicz I now understand the problem. Thank you.

Basically the problem was that, by adding @pageNumber={{this.pageNumber}} you're making the @pageNumber argument to trigger changes using the didChangeAttrs custom hook. So, that indeed triggers the loadData function twice, which is a problem.

However, we still need to support the case where no @pageNumber={{this.pageNumber}} is passed in. In that case, your changes will make the test fail. So we need to think about something else.

This would be solved with the args separation now present in octane. The problem here is that when we set the internal pageNumber property we are propagating it to the outside of the component.

[miguelcobain]

@hbrysiewicz can you please try 1.6.1? I believe I fixed the root problem of this issue.

Relevant commit: https://github.com/miguelcobain/ember-yeti-table/commit/75fe6183ebed894e3a0a5dac862a623dadca576b