Fix suggestion for #329 / #388

[MrWook]

Added parameter to the sort function which is only be used for the sortDefault behaviour for the first call. This is to fix the problem for #329 and #388 and stop the confusion that this issue created.

It worked for me but i think i'm missing something because it was too easy... Any objection for this PR?

[lorenzofox3]

mmh not sure. This one was due to race conditions so it might change from setup to the other. Here you simply asking to bypass the asynchronous nature of the sort operation. Could work to fix part of the issues. However your modification breaks a lot of tests (see https://travis-ci.org/lorenzofox3/Smart-Table/builds/249633235?utm_source=github_status&utm_medium=notification) so you might have introduced a regression

[MrWook]

Do you mean rare conditon? Because it's not really rare it's always happening if you combine the pipe with stSortDefault. If you really mean race condition, the only way this could change is when someone set the pipe delay after the sort delay. But if someone new start with SmartTable i don't think someone will do this without a reason. As you can see in the two issues many people are confused because of this problem.

So i think to bypass the asynchronous nature for the initialization of the stSortDefault is a good deal.

I understand that the test can't set a default sort but in this plunker and in my application it works just fine, so i dont understand why the test can't do it.

The plunker is the pipe example from the documentation with these little changes. In line 542 i added a console.log to see how often the pipe is triggered. If you delete the "true" in line 416 as a parameter you will see the pipe will be triggered twice

[lorenzofox3]

I meant race condition: meaning sometimes it happens, sometimes it does not depending whether the pagination is loaded faster than an other directive for example.

Ok for bypassing the default asynchronous nature.

I understand that the test can't set a default sort but in this plunker and in my application it works just fine, so i dont understand why the test can't do it.

Well you will probably need to fix some tests as you changed somehow the behavior introducing a new one which is not taken into account in the tests, but I definitely can not merge safely a PR which breaks the build even if it "works" in a plunker: your plunker may not consider other cases the tests check

[MrWook]

Okey, but i don't think this is a race condition because you can modify the delay. So it depend on your own configuration. If you don't set the delay like in both of the issues described the pipe will be called twice. I never had the experience that it's not called twice without the config adjustments.

Unfortunately I'm not yet familiar with automatic testing. So i think it's better to ask you here over my opinion. What i read about the test script is that the $timeout.flush() for both tests which failed with an error, can't be executed because there is no pending task. I think this is because the data is directly sorted and finished before the test script reach the line for $timeout.flush().

Maybe if these two errors can be eliminate the remaining failed test will be solved too?