-
Notifications
You must be signed in to change notification settings - Fork 1
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
Stop ID autocomplete #1057
base: master
Are you sure you want to change the base?
Stop ID autocomplete #1057
Changes from all commits
d5dfb6c
a68dd2b
a5c3a27
9bd5b46
9f8b809
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -429,4 +429,34 @@ defmodule Arrow.Shuttles do | |
stop -> stop | ||
end | ||
end | ||
|
||
@spec stops_or_gtfs_stops_by_search_string(String.t()) :: [Stop.t() | GtfsStop.t()] | ||
def stops_or_gtfs_stops_by_search_string(string) do | ||
# See https://github.blog/engineering/like-injection/ | ||
sanitized_string = Regex.replace(~r/([\%_])/, string, fn _, x -> "\\#{x}" end) <> "%" | ||
|
||
stops_query = | ||
from(s in Stop, | ||
where: | ||
ilike(s.stop_id, ^sanitized_string) or ilike(s.stop_desc, ^sanitized_string) or | ||
ilike(s.stop_name, ^sanitized_string) | ||
) | ||
|
||
gtfs_stops_query = | ||
from(g in GtfsStop, | ||
where: | ||
g.vehicle_type == :bus and g.location_type == :stop_platform and | ||
(ilike(g.id, ^sanitized_string) or ilike(g.desc, ^sanitized_string) or | ||
ilike(g.name, ^sanitized_string)) | ||
) | ||
|
||
matching_stops = Repo.all(stops_query) | ||
|
||
arrow_stop_ids = matching_stops |> Enum.map(& &1.stop_id) |> MapSet.new() | ||
|
||
matching_gtfs_stops = | ||
gtfs_stops_query |> Repo.all() |> Enum.filter(&(!MapSet.member?(arrow_stop_ids, &1.id))) | ||
|
||
Enum.concat(matching_stops, matching_gtfs_stops) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion Would it make sense to sort the results in some way? For example, alphabetically or by Jaro distance from the user-entered string. (Though since the search is over 3 different fields in 2 different tables, this could get complicated as you'd need to do something along the lines of matches
|> Enum.sort_by(fn stop ->
[stop.name, stop.id, stop.desc]
|> Enum.map(&String.jaro_distance(&1, string)
|> Enum.max()
end) so feel free to leave it as-is if that seems like too much) |
||
end | ||
end |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,76 @@ | ||||||
defmodule ArrowWeb.StopInput do | ||||||
@moduledoc """ | ||||||
LiveComponent providing a wrapper around live_select tailored for | ||||||
stop ID autocomplete. | ||||||
""" | ||||||
|
||||||
use ArrowWeb, :live_component | ||||||
|
||||||
alias Arrow.Gtfs.Stop, as: GtfsStop | ||||||
alias Arrow.Shuttles | ||||||
alias Arrow.Shuttles.Stop | ||||||
|
||||||
attr :id, :string, required: true | ||||||
attr :field, :any, required: true | ||||||
attr :label, :string, default: "Stop ID" | ||||||
attr :class, :string, default: nil | ||||||
|
||||||
def render(assigns) do | ||||||
assigns = | ||||||
assign( | ||||||
assigns, | ||||||
:stop, | ||||||
Phoenix.HTML.Form.input_value(assigns.field.form, :stop) || | ||||||
Phoenix.HTML.Form.input_value(assigns.field.form, :gtfs_stop) | ||||||
Comment on lines
+23
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion / question Does this mean this component is safe to use only in a form that also has My understanding of LiveView is still pretty tenuous, but this feels like a sort of hidden dependency for this component. Is there a way to pass these values directly and explicitly into |
||||||
) | ||||||
|
||||||
# This should only change if the selected value actually changes, | ||||||
# not just when a user is typing and options change. | ||||||
assigns = | ||||||
assign( | ||||||
assigns, | ||||||
:options, | ||||||
if is_nil(assigns.stop) || !Ecto.assoc_loaded?(assigns.stop) do | ||||||
[] | ||||||
else | ||||||
[option_for_stop(assigns.stop)] | ||||||
end | ||||||
) | ||||||
Comment on lines
+27
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question Bear with me as I continue to muddle through the live component lifecycle & data-flow models... So, is the idea here that this expression in Based on the diagram here, it seems like Would that cause the options list to be reset to the result of this expression, if it was previously updated by a |
||||||
|
||||||
~H""" | ||||||
<div id={@id} class={@class}> | ||||||
<.live_select | ||||||
field={@field} | ||||||
label={@label} | ||||||
allow_clear={true} | ||||||
target={@myself} | ||||||
options={@options} | ||||||
/> | ||||||
</div> | ||||||
""" | ||||||
end | ||||||
|
||||||
def handle_event("live_select_change", %{"id" => live_select_id, "text" => text}, socket) do | ||||||
new_opts = | ||||||
if String.length(text) < 2 do | ||||||
# We only start autocomplete at 2 characters, but there are some 1-character stop IDs | ||||||
case Shuttles.stop_or_gtfs_stop_for_stop_id(text) do | ||||||
nil -> [] | ||||||
stop -> [option_for_stop(stop)] | ||||||
end | ||||||
else | ||||||
text |> Shuttles.stops_or_gtfs_stops_by_search_string() |> Enum.map(&option_for_stop/1) | ||||||
end | ||||||
|
||||||
send_update(LiveSelect.Component, id: live_select_id, options: new_opts) | ||||||
|
||||||
{:noreply, socket} | ||||||
end | ||||||
|
||||||
@spec option_for_stop(Stop.t() | GtfsStop.t()) :: {String.t(), String.t()} | ||||||
defp option_for_stop(%Stop{stop_id: stop_id, stop_desc: stop_desc, stop_name: stop_name}), | ||||||
do: {"#{stop_id} - #{stop_desc || stop_name}", stop_id} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Issue The Arrow shuttle stops table's I'm not sure if we allow
Suggested change
|
||||||
|
||||||
defp option_for_stop(%GtfsStop{id: id, desc: desc, name: name}), | ||||||
do: {"#{id} - #{desc || name}", id} | ||||||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion / question
Maybe a silly question, but could we just remove all
%
's? And maybe even_
's too. I wouldn't expect either of those characters to ever show up in a stop name, ID, or description.Also, it looks like you anchor the search to the start of the string since
%
is only appended and not prepended to the search. I think that's ok but just wanted to check that it was an intentional choice.