From fd9cca2233621e1661bb35e5b272ae3eb4464298 Mon Sep 17 00:00:00 2001 From: Douglas Lutz Date: Wed, 4 Mar 2020 13:12:32 -0300 Subject: [PATCH 1/7] Change url companies param on :show route to name attribute --- lib/companies/companies.ex | 24 +++++++++++++++ .../controllers/company_controller.ex | 4 +-- lib/companies_web/router.ex | 3 +- test/companies/companies_test.exs | 29 +++++++++++++++++++ 4 files changed, 57 insertions(+), 3 deletions(-) diff --git a/lib/companies/companies.ex b/lib/companies/companies.ex index 51acda13..4b8ec85d 100644 --- a/lib/companies/companies.ex +++ b/lib/companies/companies.ex @@ -100,6 +100,30 @@ defmodule Companies.Companies do |> Repo.get!(id) end + @doc """ + Gets a single company. + + Raises `Ecto.NoResultsError` if the Company does not exist. + + ## Examples + + iex> get!("Valid name") + %Company{} + + iex> get!("Invalid name") + ** (Ecto.NoResultsError) + + """ + def get_by_name!(name, opts \\ []) do + preloads = Keyword.get(opts, :preloads, []) + + from(c in Company) + |> preload(^preloads) + |> from() + |> where([c], is_nil(c.removed_pending_change_id)) + |> Repo.get_by!(name: name) + end + @doc """ Submits a new company for approval. diff --git a/lib/companies_web/controllers/company_controller.ex b/lib/companies_web/controllers/company_controller.ex index 62718f5b..f0e3e870 100644 --- a/lib/companies_web/controllers/company_controller.ex +++ b/lib/companies_web/controllers/company_controller.ex @@ -37,8 +37,8 @@ defmodule CompaniesWeb.CompanyController do end end - def show(conn, %{"id" => id}) do - company = Companies.get!(id, preloads: [:jobs, :industry]) + def show(conn, %{"name" => name}) do + company = Companies.get_by_name!(name, preloads: [:jobs, :industry]) render(conn, "show.html", company: company) end diff --git a/lib/companies_web/router.ex b/lib/companies_web/router.ex index a5ba2919..260f6463 100644 --- a/lib/companies_web/router.ex +++ b/lib/companies_web/router.ex @@ -59,7 +59,8 @@ defmodule CompaniesWeb.Router do end get "/", CompanyController, :recent - resources "/companies", CompanyController, only: [:index, :show] + resources "/companies", CompanyController, only: [:index] + resources "/companies", CompanyController, only: [:show], param: "name" get "/jobs", JobController, :index get "/profile", UserController, :profile get "/for_hire", UserController, :for_hire diff --git a/test/companies/companies_test.exs b/test/companies/companies_test.exs index 44180704..6c7c9fba 100644 --- a/test/companies/companies_test.exs +++ b/test/companies/companies_test.exs @@ -85,6 +85,35 @@ defmodule Companies.CompaniesTest do end end + describe "get_by_name!/2" do + test "retrieves a company by it's name" do + %{name: name} = insert(:company, name: "ZULU") + + assert %{name: ^name} = Companies.get_by_name!(name) + end + + test "preloads given associations" do + company = insert(:company, name: "ZULU") + + assert %{jobs: []} = Companies.get_by_name!(company.name, preloads: [:jobs]) + end + + test "raises for unknown name" do + assert_raise Ecto.NoResultsError, fn -> + Companies.get_by_name!("NONAME", preloads: [:jobs]) + end + end + + test "raises for multiple companies with same name" do + insert(:company, name: "ZULU") + insert(:company, name: "ZULU") + + assert_raise Ecto.MultipleResultsError, fn -> + Companies.get_by_name!("ZULU", preloads: [:jobs]) + end + end + end + describe "get!/1" do test "retrieves by id" do %{id: company_id} = insert(:company) From b4d66f07ab293c767fb2814efdbea50df953aef7 Mon Sep 17 00:00:00 2001 From: Douglas Lutz Date: Wed, 4 Mar 2020 17:49:31 -0300 Subject: [PATCH 2/7] Change template link to new url --- lib/companies_web/templates/company/card.html.eex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/companies_web/templates/company/card.html.eex b/lib/companies_web/templates/company/card.html.eex index ad8cd070..17ad00ba 100644 --- a/lib/companies_web/templates/company/card.html.eex +++ b/lib/companies_web/templates/company/card.html.eex @@ -7,7 +7,7 @@ <% end %>
-

<%= link to: Routes.company_path(@conn, :show, locale(@conn), @company) do %><%= @company.name %><% end %>

+

<%= link to: Routes.company_path(@conn, :show, locale(@conn), @company.name) do %><%= @company.name %><% end %>

<%= if signed_in?(@conn) do %>
From 87168dd62f557a4a3eeb1a8b947917933c565653 Mon Sep 17 00:00:00 2001 From: Douglas Lutz Date: Thu, 5 Mar 2020 18:12:12 -0300 Subject: [PATCH 3/7] Add slug field to company --- lib/companies/companies.ex | 8 +++---- lib/companies/schema/company.ex | 19 ++++++++++++++- .../controllers/company_controller.ex | 16 ++++++------- lib/companies_web/router.ex | 5 ++-- .../templates/company/card.html.eex | 2 +- .../20200305171000_add_slug_to_companies.exs | 11 +++++++++ ..._add_slug_value_for_existing_companies.exs | 23 +++++++++++++++++++ priv/repo/seeds.exs | 1 + test/companies/companies_test.exs | 21 +++++------------ test/support/factories.ex | 1 + 10 files changed, 75 insertions(+), 32 deletions(-) create mode 100644 priv/repo/migrations/20200305171000_add_slug_to_companies.exs create mode 100644 priv/repo/migrations/20200305174544_add_slug_value_for_existing_companies.exs diff --git a/lib/companies/companies.ex b/lib/companies/companies.ex index 4b8ec85d..407ea9c0 100644 --- a/lib/companies/companies.ex +++ b/lib/companies/companies.ex @@ -107,21 +107,21 @@ defmodule Companies.Companies do ## Examples - iex> get!("Valid name") + iex> get!("Valid slug") %Company{} - iex> get!("Invalid name") + iex> get!("Invalid slug") ** (Ecto.NoResultsError) """ - def get_by_name!(name, opts \\ []) do + def get_by_slug!(slug, opts \\ []) do preloads = Keyword.get(opts, :preloads, []) from(c in Company) |> preload(^preloads) |> from() |> where([c], is_nil(c.removed_pending_change_id)) - |> Repo.get_by!(name: name) + |> Repo.get_by!(slug: slug) end @doc """ diff --git a/lib/companies/schema/company.ex b/lib/companies/schema/company.ex index 7cceb2c9..d6d2bc61 100644 --- a/lib/companies/schema/company.ex +++ b/lib/companies/schema/company.ex @@ -6,6 +6,7 @@ defmodule Companies.Schema.Company do alias Companies.Schema.{Industry, Job, PendingChange} + @derive {Phoenix.Param, key: :slug} schema "companies" do field :blog, :string field :description, :string @@ -13,6 +14,7 @@ defmodule Companies.Schema.Company do field :location, :string field :name, :string field :url, :string + field :slug, :string belongs_to :industry, Industry has_many :jobs, Job, defaults: [removed_pending_change_id: nil] @@ -24,8 +26,23 @@ defmodule Companies.Schema.Company do @doc false def changeset(company, attrs) do + attrs = Map.merge(attrs, slug_map(attrs)) + company - |> cast(attrs, [:name, :description, :url, :github, :location, :blog, :industry_id]) + |> cast(attrs, [:name, :description, :url, :github, :location, :blog, :industry_id, :slug]) |> validate_required([:name, :description, :url, :industry_id]) end + + defp slug_map(%{"name" => name}) do + slug = + name + |> String.replace(~r/['’]s/u, "s") + |> String.downcase() + |> String.replace(~r/([^a-z0-9가-힣])+/, "-") + |> String.replace(" ", "-") + + %{"slug" => slug} + end + + defp slug_map(_attrs), do: %{} end diff --git a/lib/companies_web/controllers/company_controller.ex b/lib/companies_web/controllers/company_controller.ex index f0e3e870..de4ddc88 100644 --- a/lib/companies_web/controllers/company_controller.ex +++ b/lib/companies_web/controllers/company_controller.ex @@ -37,20 +37,20 @@ defmodule CompaniesWeb.CompanyController do end end - def show(conn, %{"name" => name}) do - company = Companies.get_by_name!(name, preloads: [:jobs, :industry]) + def show(conn, %{"slug" => slug}) do + company = Companies.get_by_slug!(slug, preloads: [:jobs, :industry]) render(conn, "show.html", company: company) end - def edit(conn, %{"id" => id}) do - company = Companies.get!(id) + def edit(conn, %{"slug" => slug}) do + company = Companies.get_by_slug!(slug) changeset = Companies.change(company) industries = Industries.all() render(conn, "edit.html", company: company, changeset: changeset, industries: industries) end - def update(conn, %{"id" => id, "company" => company_params}) do - company = Companies.get!(id) + def update(conn, %{"slug" => slug, "company" => company_params}) do + company = Companies.get_by_slug!(slug) case Companies.update(company, company_params, current_user(conn)) do {:ok, _company} -> @@ -64,8 +64,8 @@ defmodule CompaniesWeb.CompanyController do end end - def delete(conn, %{"id" => id}) do - company = Companies.get!(id) + def delete(conn, %{"slug" => slug}) do + company = Companies.get_by_slug!(slug) {:ok, _company} = Companies.delete(company, current_user(conn)) conn diff --git a/lib/companies_web/router.ex b/lib/companies_web/router.ex index 260f6463..e2205d82 100644 --- a/lib/companies_web/router.ex +++ b/lib/companies_web/router.ex @@ -53,14 +53,13 @@ defmodule CompaniesWeb.Router do scope "/" do pipe_through [:auth] - resources "/companies", CompanyController, except: [:index, :show] + resources "/companies", CompanyController, except: [:index, :show], param: "slug" resources "/jobs", JobController, except: [:index, :show] resources "/users", UserController, only: [:edit, :update] end get "/", CompanyController, :recent - resources "/companies", CompanyController, only: [:index] - resources "/companies", CompanyController, only: [:show], param: "name" + resources "/companies", CompanyController, only: [:index, :show], param: "slug" get "/jobs", JobController, :index get "/profile", UserController, :profile get "/for_hire", UserController, :for_hire diff --git a/lib/companies_web/templates/company/card.html.eex b/lib/companies_web/templates/company/card.html.eex index 17ad00ba..ad8cd070 100644 --- a/lib/companies_web/templates/company/card.html.eex +++ b/lib/companies_web/templates/company/card.html.eex @@ -7,7 +7,7 @@
<% end %>
-

<%= link to: Routes.company_path(@conn, :show, locale(@conn), @company.name) do %><%= @company.name %><% end %>

+

<%= link to: Routes.company_path(@conn, :show, locale(@conn), @company) do %><%= @company.name %><% end %>

<%= if signed_in?(@conn) do %>
diff --git a/priv/repo/migrations/20200305171000_add_slug_to_companies.exs b/priv/repo/migrations/20200305171000_add_slug_to_companies.exs new file mode 100644 index 00000000..bbb57349 --- /dev/null +++ b/priv/repo/migrations/20200305171000_add_slug_to_companies.exs @@ -0,0 +1,11 @@ +defmodule Companies.Repo.Migrations.AddSlugToCompanies do + use Ecto.Migration + + def change do + alter table(:companies) do + add :slug, :string + end + + create index(:companies, [:slug], unique: true) + end +end diff --git a/priv/repo/migrations/20200305174544_add_slug_value_for_existing_companies.exs b/priv/repo/migrations/20200305174544_add_slug_value_for_existing_companies.exs new file mode 100644 index 00000000..2e4b1c1e --- /dev/null +++ b/priv/repo/migrations/20200305174544_add_slug_value_for_existing_companies.exs @@ -0,0 +1,23 @@ +defmodule Companies.Repo.Migrations.AddSlugValueForExistingCompanies do + use Ecto.Migration + + alias Companies.Repo + alias Companies.Schema.Company + + def change do + Repo.all(Company) + |> Enum.map(fn company -> + slug = + company.name + |> String.replace(~r/['’]s/u, "s") + |> String.downcase() + |> String.replace(~r/([^a-z0-9가-힣])+/, "-") + |> String.replace(" ", "-") + + changeset = Company.changeset(company, %{"slug" => slug}) + changeset = unless changeset.valid?, do: Company.changeset(company, %{"slug" => "#{slug}-2"}) + + Repo.update(changeset) + end) + end +end diff --git a/priv/repo/seeds.exs b/priv/repo/seeds.exs index ab45d596..4374a531 100644 --- a/priv/repo/seeds.exs +++ b/priv/repo/seeds.exs @@ -174,6 +174,7 @@ technology_consulting = Repo.insert!(%Industry{name: "Technology Consulting"}) plataformatec = Repo.insert!(%Company{ name: "Plataformatec", + slug: "plataformatec", description: """ Project inception, coaching, tailored projects, general consulting. Sponsor of Elixir, employer to Elixir's BDFL. """, diff --git a/test/companies/companies_test.exs b/test/companies/companies_test.exs index 6c7c9fba..e6d1b422 100644 --- a/test/companies/companies_test.exs +++ b/test/companies/companies_test.exs @@ -85,31 +85,22 @@ defmodule Companies.CompaniesTest do end end - describe "get_by_name!/2" do + describe "get_by_slug!/2" do test "retrieves a company by it's name" do - %{name: name} = insert(:company, name: "ZULU") + %{id: id, slug: slug} = insert(:company, name: "ZULU") - assert %{name: ^name} = Companies.get_by_name!(name) + assert %{id: ^id} = Companies.get_by_slug!(slug) end test "preloads given associations" do company = insert(:company, name: "ZULU") - assert %{jobs: []} = Companies.get_by_name!(company.name, preloads: [:jobs]) + assert %{jobs: []} = Companies.get_by_slug!(company.slug, preloads: [:jobs]) end - test "raises for unknown name" do + test "raises for unknown slug" do assert_raise Ecto.NoResultsError, fn -> - Companies.get_by_name!("NONAME", preloads: [:jobs]) - end - end - - test "raises for multiple companies with same name" do - insert(:company, name: "ZULU") - insert(:company, name: "ZULU") - - assert_raise Ecto.MultipleResultsError, fn -> - Companies.get_by_name!("ZULU", preloads: [:jobs]) + Companies.get_by_slug!("invalid-slug", preloads: [:jobs]) end end end diff --git a/test/support/factories.ex b/test/support/factories.ex index 317052db..cf158369 100644 --- a/test/support/factories.ex +++ b/test/support/factories.ex @@ -10,6 +10,7 @@ defmodule Companies.Factory do description: "A test company", industry: insert(:industry), name: sequence(:name, &"Test Company #{&1}"), + slug: sequence(:slug, &"test-company-#{&1}"), url: "www.example.com" } end From d093ae049ac4096ba836a1af7dec57dad03d2eb2 Mon Sep 17 00:00:00 2001 From: Douglas Lutz Date: Mon, 9 Mar 2020 13:51:25 -0300 Subject: [PATCH 4/7] Remove data migration --- ..._add_slug_value_for_existing_companies.exs | 23 ------------------- 1 file changed, 23 deletions(-) delete mode 100644 priv/repo/migrations/20200305174544_add_slug_value_for_existing_companies.exs diff --git a/priv/repo/migrations/20200305174544_add_slug_value_for_existing_companies.exs b/priv/repo/migrations/20200305174544_add_slug_value_for_existing_companies.exs deleted file mode 100644 index 2e4b1c1e..00000000 --- a/priv/repo/migrations/20200305174544_add_slug_value_for_existing_companies.exs +++ /dev/null @@ -1,23 +0,0 @@ -defmodule Companies.Repo.Migrations.AddSlugValueForExistingCompanies do - use Ecto.Migration - - alias Companies.Repo - alias Companies.Schema.Company - - def change do - Repo.all(Company) - |> Enum.map(fn company -> - slug = - company.name - |> String.replace(~r/['’]s/u, "s") - |> String.downcase() - |> String.replace(~r/([^a-z0-9가-힣])+/, "-") - |> String.replace(" ", "-") - - changeset = Company.changeset(company, %{"slug" => slug}) - changeset = unless changeset.valid?, do: Company.changeset(company, %{"slug" => "#{slug}-2"}) - - Repo.update(changeset) - end) - end -end From 30a35a02ce079dcd7411d6deb166d41e37b3123a Mon Sep 17 00:00:00 2001 From: Douglas Lutz Date: Wed, 18 Mar 2020 13:14:23 -0300 Subject: [PATCH 5/7] Change index to unique index --- priv/repo/migrations/20200305171000_add_slug_to_companies.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/priv/repo/migrations/20200305171000_add_slug_to_companies.exs b/priv/repo/migrations/20200305171000_add_slug_to_companies.exs index bbb57349..f88363e8 100644 --- a/priv/repo/migrations/20200305171000_add_slug_to_companies.exs +++ b/priv/repo/migrations/20200305171000_add_slug_to_companies.exs @@ -6,6 +6,6 @@ defmodule Companies.Repo.Migrations.AddSlugToCompanies do add :slug, :string end - create index(:companies, [:slug], unique: true) + create unique_index(:companies, [:slug]) end end From 014a304b54f0d6f9aee3b5b6154c70ec9469d415 Mon Sep 17 00:00:00 2001 From: Douglas Lutz Date: Wed, 18 Mar 2020 13:27:42 -0300 Subject: [PATCH 6/7] Change slug generation --- lib/companies/schema/company.ex | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/companies/schema/company.ex b/lib/companies/schema/company.ex index d6d2bc61..324a4643 100644 --- a/lib/companies/schema/company.ex +++ b/lib/companies/schema/company.ex @@ -26,14 +26,18 @@ defmodule Companies.Schema.Company do @doc false def changeset(company, attrs) do - attrs = Map.merge(attrs, slug_map(attrs)) - company |> cast(attrs, [:name, :description, :url, :github, :location, :blog, :industry_id, :slug]) |> validate_required([:name, :description, :url, :industry_id]) + |> generate_slug() + |> unique_constraint(:slug, message: "Company slug already exists") + end + + defp generate_slug(%Ecto.Changeset{data: %{id: id}} = changeset) when not is_nil(id) do + changeset end - defp slug_map(%{"name" => name}) do + defp generate_slug(%Ecto.Changeset{changes: %{name: name}} = changeset) do slug = name |> String.replace(~r/['’]s/u, "s") @@ -41,8 +45,8 @@ defmodule Companies.Schema.Company do |> String.replace(~r/([^a-z0-9가-힣])+/, "-") |> String.replace(" ", "-") - %{"slug" => slug} + put_change(changeset, :slug, slug) end - defp slug_map(_attrs), do: %{} + defp generate_slug(changeset), do: changeset end From 6288667d0a68872759f15f73f99e3e7c15fac413 Mon Sep 17 00:00:00 2001 From: Douglas Lutz Date: Wed, 18 Mar 2020 13:50:05 -0300 Subject: [PATCH 7/7] Remove get_by_slug! methot/change get! method to accept slugs --- lib/companies/companies.ex | 36 +++++-------------- .../controllers/company_controller.ex | 8 ++--- test/companies/companies_test.exs | 35 +++++------------- 3 files changed, 20 insertions(+), 59 deletions(-) diff --git a/lib/companies/companies.ex b/lib/companies/companies.ex index 407ea9c0..f7984649 100644 --- a/lib/companies/companies.ex +++ b/lib/companies/companies.ex @@ -90,38 +90,18 @@ defmodule Companies.Companies do ** (Ecto.NoResultsError) """ - def get!(id, opts \\ []) do + def get!(key, opts \\ []) do preloads = Keyword.get(opts, :preloads, []) - from(c in Company) - |> preload(^preloads) - |> from() - |> where([c], is_nil(c.removed_pending_change_id)) - |> Repo.get!(id) - end - - @doc """ - Gets a single company. - - Raises `Ecto.NoResultsError` if the Company does not exist. - - ## Examples - - iex> get!("Valid slug") - %Company{} - - iex> get!("Invalid slug") - ** (Ecto.NoResultsError) + query = from(c in Company) |> preload(^preloads) |> from() |> where([c], is_nil(c.removed_pending_change_id)) - """ - def get_by_slug!(slug, opts \\ []) do - preloads = Keyword.get(opts, :preloads, []) + final_query = + case Integer.parse(key) do + :error -> where(query, [c], c.slug == ^key) + {int_id, _remainder} -> where(query, [c], c.id == ^int_id or c.slug == ^key) + end - from(c in Company) - |> preload(^preloads) - |> from() - |> where([c], is_nil(c.removed_pending_change_id)) - |> Repo.get_by!(slug: slug) + Repo.one!(final_query) end @doc """ diff --git a/lib/companies_web/controllers/company_controller.ex b/lib/companies_web/controllers/company_controller.ex index de4ddc88..bc5bb539 100644 --- a/lib/companies_web/controllers/company_controller.ex +++ b/lib/companies_web/controllers/company_controller.ex @@ -38,19 +38,19 @@ defmodule CompaniesWeb.CompanyController do end def show(conn, %{"slug" => slug}) do - company = Companies.get_by_slug!(slug, preloads: [:jobs, :industry]) + company = Companies.get!(slug, preloads: [:jobs, :industry]) render(conn, "show.html", company: company) end def edit(conn, %{"slug" => slug}) do - company = Companies.get_by_slug!(slug) + company = Companies.get!(slug) changeset = Companies.change(company) industries = Industries.all() render(conn, "edit.html", company: company, changeset: changeset, industries: industries) end def update(conn, %{"slug" => slug, "company" => company_params}) do - company = Companies.get_by_slug!(slug) + company = Companies.get!(slug) case Companies.update(company, company_params, current_user(conn)) do {:ok, _company} -> @@ -65,7 +65,7 @@ defmodule CompaniesWeb.CompanyController do end def delete(conn, %{"slug" => slug}) do - company = Companies.get_by_slug!(slug) + company = Companies.get!(slug) {:ok, _company} = Companies.delete(company, current_user(conn)) conn diff --git a/test/companies/companies_test.exs b/test/companies/companies_test.exs index e6d1b422..99218f72 100644 --- a/test/companies/companies_test.exs +++ b/test/companies/companies_test.exs @@ -66,41 +66,22 @@ defmodule Companies.CompaniesTest do end describe "get!/2" do - test "retrieves a company by id" do - %{id: id} = insert(:company, name: "ZULU") - - assert %{id: ^id} = Companies.get!(id) - end - - test "preloads given associations" do - company = insert(:company, name: "ZULU") - - assert %{jobs: []} = Companies.get!(company.id, preloads: [:jobs]) - end - - test "raises for unknown id" do - assert_raise Ecto.NoResultsError, fn -> - Companies.get!(1000, preloads: [:jobs]) - end - end - end - - describe "get_by_slug!/2" do - test "retrieves a company by it's name" do + test "retrieves a company by given key" do %{id: id, slug: slug} = insert(:company, name: "ZULU") - assert %{id: ^id} = Companies.get_by_slug!(slug) + assert %{id: ^id} = Companies.get!("#{id}") + assert %{id: ^id} = Companies.get!(slug) end test "preloads given associations" do company = insert(:company, name: "ZULU") - assert %{jobs: []} = Companies.get_by_slug!(company.slug, preloads: [:jobs]) + assert %{jobs: []} = Companies.get!("#{company.id}", preloads: [:jobs]) end - test "raises for unknown slug" do + test "raises for unknown id" do assert_raise Ecto.NoResultsError, fn -> - Companies.get_by_slug!("invalid-slug", preloads: [:jobs]) + Companies.get!("#{1000}", preloads: [:jobs]) end end end @@ -109,13 +90,13 @@ defmodule Companies.CompaniesTest do test "retrieves by id" do %{id: company_id} = insert(:company) - assert %{id: ^company_id} = Companies.get!(company_id) + assert %{id: ^company_id} = Companies.get!("#{company_id}") end test "does not retrieve deleted record" do company = insert(:company, %{removed_pending_change: build(:pending_change)}) - assert_raise Ecto.NoResultsError, fn -> Companies.get!(company.id) end + assert_raise Ecto.NoResultsError, fn -> Companies.get!("#{company.id}") end end end