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

feat: add bulk upload of shapes #986

Merged
merged 6 commits into from
Jul 1, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions lib/arrow/shuttle.ex
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ defmodule Arrow.Shuttle do

import Ecto.Query, warn: false
alias Arrow.Repo
alias ArrowWeb.ErrorHelpers

alias Arrow.Shuttle.Shape

Expand Down Expand Up @@ -37,6 +38,29 @@ defmodule Arrow.Shuttle do
"""
def get_shape!(id), do: Repo.get!(Shape, id)

@doc """
Creates shapes.

"""
def create_shapes(shapes) do
changesets = Enum.map(shapes, fn shape -> create_shape(shape) end)

case Enum.all?(changesets, fn changeset -> Kernel.match?({:ok, _shape}, changeset) end) do
true ->
{:ok, changesets}

_ ->
errors =
changesets
|> Enum.filter(fn changeset -> Kernel.match?({:error, _}, changeset) end)
|> Enum.map(fn {_, changeset} ->
ErrorHelpers.changeset_error_messages(changeset)
end)

{:error, {"Failed to upload some shapes", errors}}
end
end

@doc """
Creates a shape.

Expand Down
16 changes: 13 additions & 3 deletions lib/arrow/shuttle/shape.ex
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,27 @@ defmodule Arrow.Shuttle.Shape do
use Ecto.Schema
import Ecto.Changeset

@type id :: integer
@type t :: %__MODULE__{
id: id,
name: String.t(),
coordinates: list(String.t()),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this need to be saved to the db as well as the kml file uploaded to s3?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The designs for bulk upload for shapes is why I chose to include this in the db after all. There's versatility in having both the S3 files and the db representation of the coordinates.

It makes sense to have the kml files in S3 in terms of using the format used by other map/transit partners like Remix and Google Map (and gtfs_creator itself).

Since the ticket spec ended up calling for a bulk upload, with shape selection, shape re-naming, and coordinate consolidation, it seemed like making the backend aware (and able to later show/present) that data was worthwhile.

I see the concerns around the duplication though. What do you think?
cc @nlwstein @Whoops

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is updated in #989 to drop the saving of coordinates to the database by using an embedded schema for the shapes file processing.

inserted_at: DateTime.t(),
updated_at: DateTime.t()
}

schema "shapes" do
field :name, :string
field :coordinates, {:array, :string}

timestamps()
timestamps(type: :utc_datetime)
end

@doc false
def changeset(shape, attrs) do
shape
|> cast(attrs, [:name])
|> validate_required([:name])
|> cast(attrs, [:name, :coordinates])
|> validate_required([:name, :coordinates])
|> unique_constraint(:name)
end
end
80 changes: 80 additions & 0 deletions lib/arrow/shuttle/shape_upload.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
defmodule Arrow.Shuttle.ShapeUpload do
@moduledoc "schema for shape upload"
use Ecto.Schema
import Ecto.Changeset

@type t :: %__MODULE__{
filename: String.t(),
shapes: list(Arrow.Shuttle.Shape.t())
}

embedded_schema do
field :filename, :string
embeds_many :shapes, Arrow.Shuttle.Shape
end

@doc false
def changeset(shape, attrs) do
shape
|> cast(attrs, [:filename])
|> cast_embed(:shapes)
end

def parse_kml_from_file(shape_upload) do
file = shape_upload["filename"]
filename = file.filename

with {:ok, shapes_kml} <- File.read(file.path),
{:ok, shapes} <- parse_kml(shapes_kml) do
{:ok, shapes}
else
{:error, exception} ->
message =
if is_atom(exception),
do: :file.format_error(exception),
else: Exception.message(exception)

{:error,
{"Failed to upload shapes from #{filename} because the provided xml was invalid",
[message]}}
end
end

@doc """
Parses a KML shape into a map
"""
@spec parse_kml(String.t()) :: {:ok, map} | {:error, any}
def parse_kml(kml) do
SAXMap.from_string(kml)
end

@doc """
Parses one or many Shapes from a map of the KML/XML
"""
@spec shapes_from_kml(map) :: {:ok, list(Arrow.Shuttle.Shape.t())} | {:error, any}
def shapes_from_kml(saxy_shapes) do
placemarks = saxy_shapes["kml"]["Folder"]["Placemark"]

case placemarks do
%{"LineString" => %{"coordinates" => coords}, "name" => name}
when is_binary(coords) ->
{:ok, [%{name: name, coordinates: String.split(coords)}]}

%{"LineString" => %{"coordinates" => nil}, "name" => _name} ->
error =
{"Failed to parse shape from kml, no coordinates were found. Check your whitespace.",
[inspect(placemarks)]}

{:error, error}

_ ->
# Multiple placemarks, only capture Placemarks with LineString
placemarks = Enum.filter(placemarks, fn pm -> pm["LineString"] end)

{:ok,
Enum.map(placemarks, fn pm ->
%{name: pm["name"], coordinates: String.split(pm["LineString"]["coordinates"])}
end)}
end
end
end
35 changes: 21 additions & 14 deletions lib/arrow_web/controllers/shape_controller.ex
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
defmodule ArrowWeb.ShapeController do
require Logger
alias Arrow.Shuttle.ShapeUpload
use ArrowWeb, :controller

alias Arrow.Shuttle
alias Arrow.Shuttle.Shape
alias ArrowWeb.Plug.Authorize

plug(Authorize, :view_disruption when action in [:index, :show])
Expand All @@ -15,23 +16,29 @@ defmodule ArrowWeb.ShapeController do
render(conn, :index, shapes: shapes)
end

def new(conn, _params) do
changeset = Shuttle.change_shape(%Shape{})
render(conn, :new, changeset: changeset)
def new(conn, %{}) do
changeset_map = ShapeUpload.changeset(%ShapeUpload{shapes: %{}}, %{})
render(conn, :new_bulk, shape_upload: changeset_map)
end

def create(conn, %{"shape" => shape_params}) do
case Shuttle.create_shape(shape_params) do
{:ok, shape} ->
def create(conn, %{"shape_upload" => shape_upload}) do
with {:ok, saxy_shapes} <- ShapeUpload.parse_kml_from_file(shape_upload),
{:ok, shapes} <- ShapeUpload.shapes_from_kml(saxy_shapes),
{:ok, changesets} <- Shuttle.create_shapes(shapes) do
conn
|> put_flash(
:info,
"Uploaded successfully #{inspect(changesets)}"
)
|> redirect(to: ~p"/shapes/")
else
{:error, reason} ->
conn
|> put_flash(
:info,
"Shape created successfully from #{shape_params["filename"].filename}"
:errors,
reason
)
|> redirect(to: ~p"/shapes/#{shape}")

{:error, %Ecto.Changeset{} = changeset} ->
render(conn, :new, changeset: changeset)
|> render(:new_bulk, shape_upload: shape_upload, errors: reason)
end
end

Expand All @@ -54,7 +61,7 @@ defmodule ArrowWeb.ShapeController do
conn
|> put_flash(
:info,
"Shape updated successfully from #{shape_params["filename"].filename}"
"Shape name updated successfully"
)
|> redirect(to: ~p"/shapes/#{shape}")

Expand Down
10 changes: 9 additions & 1 deletion lib/arrow_web/controllers/shape_html/edit.html.heex
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@
<:subtitle>Use this form to manage shape records in your database.</:subtitle>
</.header>

<.shape_form changeset={@changeset} action={~p"/shapes/#{@shape}"} />
<.simple_form :let={f} for={@changeset} action={~p"/shapes/#{@shape}"} multipart>
<.error :if={@changeset.action}>
Oops, something went wrong! Please check the errors below.
</.error>
<.input field={f[:name]} type="text" label="Name" />
<:actions>
<.button>Save Shape</.button>
</:actions>
</.simple_form>

<.back navigate={~p"/shapes"}>Back to shapes</.back>
2 changes: 1 addition & 1 deletion lib/arrow_web/controllers/shape_html/index.html.heex
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<.header>
Listing Shapes
<:actions>
<.link href={~p"/shapes/new"}>
<.link href={~p"/shapes_upload"}>
<.button>New Shape</.button>
</.link>
</:actions>
Expand Down
8 changes: 0 additions & 8 deletions lib/arrow_web/controllers/shape_html/new.html.heex

This file was deleted.

20 changes: 20 additions & 0 deletions lib/arrow_web/controllers/shape_html/new_bulk.html.heex
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<.header>
New Shapes
<:subtitle>Use this form to manage shape records in your database.</:subtitle>
</.header>

<%= if Map.get(@shape_upload, "filename") do %>
<.simple_form for={@shape_upload} action={~p"/shapes_upload"} multipart>
<:actions>
<.button>Back</.button>
</:actions>
</.simple_form>
<% else %>
<.simple_form :let={f} for={@shape_upload} action={~p"/shapes_upload"} multipart>
<.input field={f[:filename]} type="file" label="Filename" required="true"/>
<:actions>
<.button>Upload File</.button>
</:actions>
</.simple_form>
<% end %>
<.back navigate={~p"/shapes"}>Back to shapes</.back>
7 changes: 7 additions & 0 deletions lib/arrow_web/controllers/shape_html/show.html.heex
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,12 @@
<.list>
<:item title="Name"><%= @shape.name %></:item>
</.list>
<.list>
<:item title="Coordinates">
<%= for coord <- @shape.coordinates do %>
<div> <%= coord %> </div>
<% end %>
</:item>
</.list>

<.back navigate={~p"/shapes"}>Back to shapes</.back>
4 changes: 3 additions & 1 deletion lib/arrow_web/router.ex
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,10 @@ defmodule ArrowWeb.Router do
resources("/disruptions", DisruptionController, except: [:index])
put("/disruptions/:id/row_status", DisruptionController, :update_row_status)
post("/disruptions/:id/notes", NoteController, :create)
resources("/shapes", ShapeController)
resources("/stops", StopController, except: [:show, :delete])
resources("/shapes", ShapeController, except: [:new, :create])
get("/shapes_upload", ShapeController, :new)
post("/shapes_upload", ShapeController, :create)
end

scope "/", ArrowWeb do
Expand Down
3 changes: 2 additions & 1 deletion mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ defmodule Arrow.MixProject do
sparse: "optimized",
app: false,
compile: false,
depth: 1}
depth: 1},
{:sax_map, "~> 1.2"}
]
end

Expand Down
2 changes: 2 additions & 0 deletions mix.lock
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@
"postgrex": {:hex, :postgrex, "0.17.3", "c92cda8de2033a7585dae8c61b1d420a1a1322421df84da9a82a6764580c503d", [:mix], [{:db_connection, "~> 2.1", [hex: :db_connection, repo: "hexpm", optional: false]}, {:decimal, "~> 1.5 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:table, "~> 0.1.0", [hex: :table, repo: "hexpm", optional: true]}], "hexpm", "946cf46935a4fdca7a81448be76ba3503cff082df42c6ec1ff16a4bdfbfb098d"},
"ranch": {:hex, :ranch, "1.8.0", "8c7a100a139fd57f17327b6413e4167ac559fbc04ca7448e9be9057311597a1d", [:make, :rebar3], [], "hexpm", "49fbcfd3682fab1f5d109351b61257676da1a2fdbe295904176d5e521a2ddfe5"},
"react_phoenix": {:hex, :react_phoenix, "1.3.1", "b2abb625ce7304a3b2ac5eea3126c30ef1e1c860f9ef27290ee726ab1fa3a87a", [:mix], [{:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: false]}, {:phoenix_html, "~> 2.11 or ~> 3.0", [hex: :phoenix_html, repo: "hexpm", optional: false]}], "hexpm", "a10de67f02f6c5cf04f6987cef7413400d671936b4df97e0b9ac3c227ba27e6b"},
"sax_map": {:hex, :sax_map, "1.2.1", "e0278054268c70f429084700680bf01c77a99508ea73713c9ed6b7b216e0a5da", [:mix], [{:saxy, "~> 1.3", [hex: :saxy, repo: "hexpm", optional: false]}], "hexpm", "98348ea9777c0577e074cba2e2d769cdd842dce8f79c89d1d59616e428d9cacc"},
"saxy": {:hex, :saxy, "1.5.0", "0141127f2d042856f135fb2d94e0beecda7a2306f47546dbc6411fc5b07e28bf", [:mix], [], "hexpm", "ea7bb6328fbd1f2aceffa3ec6090bfb18c85aadf0f8e5030905e84235861cf89"},
"sentry": {:hex, :sentry, "8.0.6", "c8de1bf0523bc120ec37d596c55260901029ecb0994e7075b0973328779ceef7", [:mix], [{:hackney, "~> 1.8", [hex: :hackney, repo: "hexpm", optional: true]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: true]}, {:plug, "~> 1.6", [hex: :plug, repo: "hexpm", optional: true]}, {:plug_cowboy, "~> 2.3", [hex: :plug_cowboy, repo: "hexpm", optional: true]}], "hexpm", "051a2d0472162f3137787c7c9d6e6e4ef239de9329c8c45b1f1bf1e9379e1883"},
"ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.7", "354c321cf377240c7b8716899e182ce4890c5938111a1296add3ec74cf1715df", [:make, :mix, :rebar3], [], "hexpm", "fe4c190e8f37401d30167c8c405eda19469f34577987c76dde613e838bbc67f8"},
"tailwind": {:hex, :tailwind, "0.2.2", "9e27288b568ede1d88517e8c61259bc214a12d7eed271e102db4c93fcca9b2cd", [:mix], [{:castore, ">= 0.0.0", [hex: :castore, repo: "hexpm", optional: false]}], "hexpm", "ccfb5025179ea307f7f899d1bb3905cd0ac9f687ed77feebc8f67bdca78565c4"},
Expand Down
2 changes: 1 addition & 1 deletion priv/repo/migrations/20240605185923_create_shapes.exs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ defmodule Arrow.Repo.Migrations.CreateShapes do
create table(:shapes) do
add :name, :string

timestamps()
timestamps(type: :timestamptz)
end

create unique_index(:shapes, [:name])
Expand Down
15 changes: 15 additions & 0 deletions priv/repo/migrations/20240611001158_add_coordinates_to_shapes.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
defmodule Arrow.Repo.Migrations.AddCoordinatesToShapes do
use Ecto.Migration

def change do
alter table(:shapes) do
add :coordinates, {:array, :string}
end
end

def down do
alter table(:shapes) do
remove :coordinates
end
end
end
6 changes: 4 additions & 2 deletions priv/repo/structure.sql
Original file line number Diff line number Diff line change
Expand Up @@ -345,8 +345,9 @@ CREATE TABLE public.schema_migrations (
CREATE TABLE public.shapes (
id bigint NOT NULL,
name character varying(255),
inserted_at timestamp(0) without time zone NOT NULL,
updated_at timestamp(0) without time zone NOT NULL
inserted_at timestamp with time zone NOT NULL,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOTE: I need to add a new migration for this change since the original scaffolding was already merged.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated_at timestamp with time zone NOT NULL,
coordinates character varying(255)[]
);


Expand Down Expand Up @@ -753,3 +754,4 @@ INSERT INTO public."schema_migrations" (version) VALUES (20240207224211);
INSERT INTO public."schema_migrations" (version) VALUES (20240605185923);
INSERT INTO public."schema_migrations" (version) VALUES (20240606171008);
INSERT INTO public."schema_migrations" (version) VALUES (20240610185146);
INSERT INTO public."schema_migrations" (version) VALUES (20240611001158);
2 changes: 1 addition & 1 deletion test/arrow/shuttle_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ defmodule Arrow.ShuttleTest do
end

test "create_shape/1 with valid data creates a shape" do
valid_attrs = %{name: "some name"}
valid_attrs = %{name: "some name", coordinates: coords()}

assert {:ok, %Shape{} = shape} = Shuttle.create_shape(valid_attrs)
assert shape.name == "some name"
Expand Down
Loading
Loading