typeorm-linq-repository
typeorm-linq-repository copied to clipboard
IRCraziestTaxi
OrderBy random()
Hello!
Your repo is awesome! Thanks!
Context: IQuery provides two functions regarding ordering: https://github.com/IRCraziestTaxi/typeorm-linq-repository/blob/9f1efb1e162e5203c83e78c457aed28eadf2b8a1/src/query/Query.ts#L364 and https://github.com/IRCraziestTaxi/typeorm-linq-repository/blob/9f1efb1e162e5203c83e78c457aed28eadf2b8a1/src/query/Query.ts#L375 But Probably we should have another one for Random. Or pass the ordering as a parameter.
here goes one suggestion. should i ask for a merge? https://github.com/sthormio/typeorm-linq-repository/commit/eec3ca77e3c445789fe51f3f7ecbccf7bbb54492
Hey @tolstenko, sorry for just now getting around to looking at this.
Sure, I welcome PRs for new features! Before you create the PR, though, I have one comment on the commit you linked.
The summary for the new interface method says: Orders the query on the specified property in random order.
However, looking at the usage, orderByExpression("RANDOM()"), it doesn't take a property to sort on. In fact, I suppose that is the entire point of random sorting is that it doesn't depend on a property.
So just change the summary to Orders the query in random order. and I'll consider it good to go.
As far as the test case goes, since there are not very many songs in the test seed data (so chances of failing are higher than if the data set was larger) and since the expression depends on the DB implementation anyway, I would say you can just omit the test case.
Thanks!
