Add active filter #1323
Add active filter #1323
Conversation
| def status_filter(queryable, %{"status" => status}) do | ||
| queryable | ||
| |> where([c], c.status == ^status) | ||
| end |
There was a problem hiding this comment.
This still needs a test
| Conversation | ||
| |> Messages.list_conversations(%{"status" => "any"}) | ||
| |> get_and_sort_ids() | ||
| # result_ids = |
There was a problem hiding this comment.
Should the default be Repo.all()....do we want to get all conversations?
There was a problem hiding this comment.
I think the test on L128 still stands: returns all records by default.
In this case we just want to test filtering by that status.
There was a problem hiding this comment.
And to expand on this, the client really should be responsible for passing the "defaults" in cases where it's not about authorization to see records.
| @spec status_filter(Queryable.t, map) :: Queryable.t | ||
| def status_filter(queryable, %{"status" => "active"}) do | ||
| @spec active_filter(Queryable.t, map) :: Queryable.t | ||
| def active_filter(queryable, %{"active" => true}) do |
There was a problem hiding this comment.
@joshsmith is this how active filter should work? What specifically filters out inactive records?
| |> join(:left, [c, _m], cp in ConversationPart, c.id == cp.conversation_id) | ||
| |> group_by([c, m, _cp], [c.id, m.initiated_by]) | ||
| |> having([_c, m, _cp], m.initiated_by == "user") | ||
| |> or_having([c, m, cp], m.initiated_by == "admin" and count(cp.id) > 0) |
There was a problem hiding this comment.
In response to your question above, this is the query that does it (specifically the having and or_having), and the documentation above expands on that.
| The status of `active` means that only those records are included which either | ||
| - belong to a `CodeCorps.Message` initiated by user | ||
| - belong to a `CodeCorps.Message` initiated by admin, with at least a single |
There was a problem hiding this comment.
We need to update this documentation.
Filters
CodeCorps.Conversationrecord queries to return only those considered to be active.Active conversations belong either:
And then update the bullets to remove "belong".
|
|
||
| @doc ~S""" | ||
| Narrows down a `CodeCorps.Conversation` query to return only those records | ||
| considered to have a specific status. |
There was a problem hiding this comment.
Would change this to:
Filters
CodeCorps.Conversationrecord queries by their status.
| Conversation | ||
| |> Messages.list_conversations(%{"status" => "any"}) | ||
| |> get_and_sort_ids() | ||
| # result_ids = |
There was a problem hiding this comment.
I think the test on L128 still stands: returns all records by default.
In this case we just want to test filtering by that status.
| Conversation | ||
| |> Messages.list_conversations(%{"status" => "any"}) | ||
| |> get_and_sort_ids() | ||
| # result_ids = |
There was a problem hiding this comment.
And to expand on this, the client really should be responsible for passing the "defaults" in cases where it's not about authorization to see records.
snewcomer
force-pushed
the
conv-query-active
branch
2 times, most recently
from
4efea63 to
5dfaf3f
Compare
joshsmith
force-pushed
the
conv-query-active
branch
from
5dfaf3f to
6b2e8fc
Compare
joshsmith
force-pushed
the
conv-query-active
branch
from
414a345 to
21e5aa6
Compare
Fixes #1312