Unable to send along extra options

[timohermans]

For my application I would like to send along extra options, like sorting / active / etc. filters. Now I implemented some extra logic for me in the route.js, but I guess creating an issue would be the right thing to do :).

[joshudev]

I had this issue today too. So I've forked and created a pull request. I handled this by adding a 'params' option which gets merged into the store.find request.

Pull request: #7
Fork: https://github.com/joshudev/ember-infinity.git

[timohermans]

@joshudev Nice solution. However, wouldn't it be better to not use a params object and just extend the object itself?

[joshudev]

I did think about doing it that way, but then wondered whether it's cleaner to use a params object in case in future there will be options that need to be set that you wouldn't want getting passed on to store.find. I don't mind so much either way, but it's more explicit with the params object.

[timohermans]

Haha I agree on that as well 😄 . Both has it's pros and cons. Let's see what @hhff has to say about it

[joshudev]

Yeah I'm just happy I got me an infinite scroll! It's funny that I went to log the same issue only 30 mins after you did!

[hhff]

Hey duders!

Thanks so much for the PR! This is definitely something we need. Ultimately, all of the params will be passed into the store.find call anyway, so IMO there's no need to separate them then re-merge them.

I think we just need to jimmy-jangle the code so that it passes through the full object to the store#find method, but just pulls out the startingPage and perPage and sets them correctly in the final params.

Thoughts?

[joshudev]

That sounds good to me! Also I noticed the travis build is failing on my fork - I think I should have used Ember.$.extend instead of $.extend.. anyways.. so will you add this feature quite soon? I'm using my fork on the project I'm working on for the moment.

[joshudev]

p.s If you want me to adjust the fork and test, I am happy to do so.

[hhff]

I'd love if you updated your PR @joshudev ! A test or two would be nice also.

Doesn't look like we'll need $.extend with this approach at all

[timohermans]

Nice, this all sounds like music to my ears, haha. Thank you guys!

[bruce]

Came to post the same issue. Glad to see it's already underway! 👍 😄

[bruce]

Since it's been a few days, I've submitted a PR (#9) along the same lines with passing acceptance and unit tests. Comments welcome.

[joshudev]

Thanks @bruce. Looks good! @hhff - it would be great to have that merged in as soon as possible.

[hhff]

Woo!

reviewing this now

[hhff]

closed via #9