thereadme/hapi-sequelize-crud
1
0
Fork 0
Code Issues 4 Pull Requests 1 Projects Releases Wiki Activity

Feat ordering by associated models now works #32

Merged
joeybaker merged 2 commits from more-order-integration into master 2016-10-31 21:23:43 +00:00
Conversation 2 Commits 2 Files Changed 5
+131 -33
joeybaker commented 2016-10-31 19:48:52 +00:00 (Migrated from github.com)
Copy Link
Copy Source

It's now possible to order by associated models. This technically might
have worked before b/c we were parsing JSON sent to order, but I'm pretty
sure it wouldn't actually work b/c we never grabbed the actual model to
associate by. Regardless, this actually enables things and adds tests to
prove it.

Note: there's a sequelize bug that's poorly reported but definitely known
where order with associated models can fail because the sql generated
doesn't include a join. So, I added docs noting that and a
test.failing so that we'll be notified when that bug is fixed and can
remove the note.

This is a minor release

It's now possible to order by associated models. This technically might have worked before b/c we were parsing JSON sent to `order`, but I'm pretty sure it wouldn't actually work b/c we never grabbed the actual model to associate by. Regardless, this actually enables things and adds tests to prove it. Note: there's a sequelize bug that's poorly reported but definitely known where `order` with associated models can fail because the sql generated doesn't include a join. So, I added docs noting that and a `test.failing` so that we'll be notified when that bug is fixed and can remove the note. This is a minor release
👍 1
codecov-io commented 2016-10-31 19:53:09 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Current coverage is 63.29% (diff: 100%)

Merging #32 into master will increase coverage by 1.07%

@@             master        #32   diff @@
==========================================
  Files             9          9          
  Lines           442        455    +13   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits            275        288    +13   
  Misses          167        167          
  Partials          0          0

Powered by Codecov. Last update 34e3721...c289fb2

## [Current coverage](https://codecov.io/gh/mdibaiee/hapi-sequelize-crud/pull/32?src=pr) is 63.29% (diff: 100%) > Merging [#32](https://codecov.io/gh/mdibaiee/hapi-sequelize-crud/pull/32?src=pr) into [master](https://codecov.io/gh/mdibaiee/hapi-sequelize-crud/branch/master?src=pr) will increase coverage by **1.07%** ``` diff @@ master #32 diff @@ ========================================== Files 9 9 Lines 442 455 +13 Methods 0 0 Messages 0 0 Branches 0 0 ========================================== + Hits 275 288 +13 Misses 167 167 Partials 0 0 ``` > Powered by [Codecov](https://codecov.io?src=pr). Last update [34e3721...c289fb2](https://codecov.io/gh/mdibaiee/hapi-sequelize-crud/compare/34e37217f167591a8858ae5a28338e24db68b009...c289fb2ed4183ffffa0d3c27e7b3beb0c02a46d7?src=pr)
joeybaker commented 2016-10-31 21:24:28 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Released as 2.9.0

Released as 2.9.0
🎉 1
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
thereadme
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: thereadme/hapi-sequelize-crud#32