Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Path params added as query param in links #22

Open
scarfacedeb opened this issue Mar 27, 2017 · 3 comments
Open

Path params added as query param in links #22

scarfacedeb opened this issue Mar 27, 2017 · 3 comments

Comments

@scarfacedeb
Copy link
Contributor

When current path contains path params, kerosene produces urls with duplicated param keys in a query.

e.g. I have a path with brand_id param: /:brand_id/products/.
When I pass the whole params map to Repo.paginate, it generates incorrect page links, such as: /asus/products?brand_id=asus&page=2.

One way to resolve it is to drop those path keys from params when passing it to Repo.paginate:

Repo.paginate(query, Map.drop(params, ["brand_id"]))

But it makes the code more brittle and difficult to update later, because we'll need to keep track of such params.

I looked into source code and I understand why it's happening.
The question is - Is there a better way to avoid such issues?

Maybe we could introduce some option to drop such keys automatically.

use Kerosene, exclude_params: ~w(locale brand_id)

I'm not particularly fond of this solution, because it's global option that applies to all Repo.paginate calls. But it could work fine for global path params, such as locale.

What do you think?

@allyraza
Copy link
Contributor

Hm weird we could use phoenix's route helpers to generate the links, but I would like to avoid a dependency on phoenix

we could do something like exclude_params, let me think about it

@scarfacedeb
Copy link
Contributor Author

@allyraza Scrivener uses routes when Phoenix is present:
https://github.com/mgwidmann/scrivener_html/blob/master/lib/scrivener/html.ex#L126

But it looks like meta magic to me.

Other option would be to pass path directly, like this:

path: &product_path(Endpoint, :index, brand.slug, &1)

@syamilmj
Copy link

My current workaround is to pass the params explicitly, e.g.:

Repo.paginate(query, Map.take(params, ["page"]))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants