search

vt-elixir / ja_resource

A behaviour to reduce boilerplate code in your JSON-API compliant Phoenix controllers without sacrificing flexibility.
Other
113 stars 33 forks source link

Clean up/determine what filter and sort callbacks should really look like. #10

Closed alanpeabody closed 8 years ago

alanpeabody commented 8 years ago

Should they include the conn? What argument order really makes sense?

alanpeabody commented 8 years ago

@beerlington @Dangeranger currently the index action has some not very well documented hooks for sorting and filtering based on the sort and filter query params.

The function signature for them is:

  @callback filter(String.t, JaResource.records, String.t) :: JaResource.records
  @callback sort(String.t, JaResource.records, String.t) :: JaResource.records

Where the first string is the key and the second the value. For example to filter based on type:

When executing: GET /v1/articles?filter[status]=draft

   def filter("status", query, status), do: where(query, status: status)

This all works, but it feels weird and inconsistent to me for the following reasons:

  1. It does not get conn at all.
  2. Every other callback and function on a controller takes conn as the first arguments.
  3. Why are key and value separated with the query in the middle?
  4. The query is the primary thing being operated on here... should it be first? second?

Sort is the same way, and I could foresee handling include in a similar fashion where we add preload statements on as needed.

What do you guys think?

Dangeranger commented 8 years ago

@alanpeabody I'll try to look at this later tonight. Thanks for the heads up.

beerlington commented 8 years ago

it seems like it should be:

def filter(query, key, value), do: ...
alanpeabody commented 8 years ago

@beerlington you don't think conn needs to be in there?

I have been thinking about filters where allowing the filter would be dependent on the user.

beerlington commented 8 years ago

Oh I missed that part. we'll be doing some filtering on a project soon and might have more feedback later this week.

Dangeranger commented 8 years ago

@alanpeabody If conn is passed in everything else I would expect it to be first like:

def filter(conn, query, [key: value]), do: where(query, key: value)

This query could get passed on to the next call to filter if there were query params like: GET /v1/articles?filter[status]=published&filter[author]=12

alanpeabody commented 8 years ago

Yup, each key/value pair is passed to filter and with the query the last returned.

I think filter(conn, query, key, value) might be the way to go. Matching keyword lists is discouraged and a pain, it would have to be filter(conn, query, [{key, value}]) but we can just pass them positionally.