Limit Skill controller index to return only 5 skills#208
Conversation
|
|
||
| assert json["data"] == [] | ||
| returned_skills_length = json["data"] |> length | ||
| assert returned_skills_length == 5 |
There was a problem hiding this comment.
I think we should be doing this limit only in the search case, so the |> limit(5) I think should be folded into that.
There was a problem hiding this comment.
We'll need to test here that it renders all of them and test in the search case that it renders no more than 5.
There was a problem hiding this comment.
Yeah, I was thinking about that but couldnt figure out how to discern between the two with only backend. This might be better tackled on the Ember side to just display 5 after it requests all matching skills?
There was a problem hiding this comment.
I think instead of relying on title_filter in the ModelHelpers I'd make a function specifically on Skill that includes the limit. Anytime you're doing a ?query on that endpoint you should only be returning five.
There was a problem hiding this comment.
Either that or we should explicitly add a limit filter and then the Ember app can add that to the query params.
There was a problem hiding this comment.
The latter seems more flexible now that I think about it, but we could always do the former temporarily, open an issue here and the Ember side, and circle back later if we'd like.
|
Just a suggestion to consider, but how about creating a limit filter, which has a default of 5 for now? We can then later, in ember, explicitly specify a different value if necessary. |
|
Yeah I like the idea of the limit filter. Will also make the necessary changes on the Ember side. |
begedin
force-pushed
the
np-limit-skills-query
branch
from
4dc9b6c to
0ed4925
Compare
3 participants
Closes #205