Skip to content
This repository has been archived by the owner on Mar 10, 2021. It is now read-only.

[#67] Refactor ApiController to return JSON so thesis can redirect to the proper page. #111

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
4 changes: 2 additions & 2 deletions apps/example/mix.lock
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@
"fs": {:hex, :fs, "0.9.2", "ed17036c26c3f70ac49781ed9220a50c36775c6ca2cf8182d123b6566e49ec59", [:rebar], [], "hexpm"},
"gettext": {:hex, :gettext, "0.12.1", "c0624f52763469ef7a3674919ae28b8286d88195b90fa1516180f31bbbd26d14", [:mix], [], "hexpm"},
"hackney": {:hex, :hackney, "1.7.1", "e238c52c5df3c3b16ce613d3a51c7220a784d734879b1e231c9babd433ac1cb4", [:rebar3], [{:certifi, "1.0.0", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "4.0.0", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "1.0.1", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "1.0.2", [hex: :mimerl, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "1.1.1", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}], "hexpm"},
"html_sanitize_ex": {:hex, :html_sanitize_ex, "1.0.1", "2572e7122c78ab7e57b613e7c7f5e42bf9b3c25e430e32f23f1413d86db8a0af", [:mix], [{:mochiweb, "~> 2.12.2", [hex: :mochiweb, repo: "hexpm", optional: false]}], "hexpm"},
"html_sanitize_ex": {:hex, :html_sanitize_ex, "1.3.0", "f005ad692b717691203f940c686208aa3d8ffd9dd4bb3699240096a51fa9564e", [:mix], [{:mochiweb, "~> 2.15", [hex: :mochiweb, repo: "hexpm", optional: false]}], "hexpm"},
"httpoison": {:hex, :httpoison, "0.11.1", "d06c571274c0e77b6cc50e548db3fd7779f611fbed6681fd60a331f66c143a0b", [:mix], [{:hackney, "~> 1.7.0", [hex: :hackney, repo: "hexpm", optional: false]}], "hexpm"},
"idna": {:hex, :idna, "4.0.0", "10aaa9f79d0b12cf0def53038547855b91144f1bfcc0ec73494f38bb7b9c4961", [:rebar3], [], "hexpm"},
"metrics": {:hex, :metrics, "1.0.1", "25f094dea2cda98213cecc3aeff09e940299d950904393b2a29d191c346a8486", [:rebar3], [], "hexpm"},
"mime": {:hex, :mime, "1.0.1", "05c393850524767d13a53627df71beeebb016205eb43bfbd92d14d24ec7a1b51", [:mix], [], "hexpm"},
"mimerl": {:hex, :mimerl, "1.0.2", "993f9b0e084083405ed8252b99460c4f0563e41729ab42d9074fd5e52439be88", [:rebar3], [], "hexpm"},
"mochiweb": {:hex, :mochiweb, "2.12.2", "80804ad342afa3d7f3524040d4eed66ce74b17a555de454ac85b07c479928e46", [:make, :rebar], [], "hexpm"},
"mochiweb": {:hex, :mochiweb, "2.15.0", "e1daac474df07651e5d17cc1e642c4069c7850dc4508d3db7263a0651330aacc", [:rebar3], [], "hexpm"},
"phoenix": {:hex, :phoenix, "1.2.1", "6dc592249ab73c67575769765b66ad164ad25d83defa3492dc6ae269bd2a68ab", [:mix], [{:cowboy, "~> 1.0", [hex: :cowboy, repo: "hexpm", optional: true]}, {:phoenix_pubsub, "~> 1.0", [hex: :phoenix_pubsub, repo: "hexpm", optional: false]}, {:plug, "~> 1.1", [hex: :plug, repo: "hexpm", optional: false]}, {:poison, "~> 1.5 or ~> 2.0", [hex: :poison, repo: "hexpm", optional: false]}], "hexpm"},
"phoenix_ecto": {:hex, :phoenix_ecto, "3.0.1", "42eb486ef732cf209d0a353e791806721f33ff40beab0a86f02070a5649ed00a", [:mix], [{:ecto, "~> 2.0", [hex: :ecto, repo: "hexpm", optional: false]}, {:phoenix_html, "~> 2.6", [hex: :phoenix_html, repo: "hexpm", optional: true]}, {:plug, "~> 1.0", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm"},
"phoenix_html": {:hex, :phoenix_html, "2.7.0", "19e12e2044340c2e43df206a06d059677c59ea1868bd1c35165438d592cd420b", [:mix], [{:plug, "~> 1.0", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm"},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
defmodule Example.Repo.Migrations.AddAUniqueIndexOnSlug do
@moduledoc false
use Ecto.Migration

def up do
# Rreate old non-unique index
drop index(:thesis_pages, [:slug])

# Create a Unique Index
create unique_index(:thesis_pages, [:slug])
end

def down do
# Remove unique index
drop unique_index(:thesis_pages, :slug)

# Create old non-unique index
create index(:thesis_pages, [:slug])
end
end
41 changes: 35 additions & 6 deletions apps/example/test/controllers/page_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,24 @@ defmodule Example.PageControllerTest do
assert html_response(conn, 200) =~ "<h4>Help</h4>"

payload = %{
"page" => %{"slug" => "/", "title" => "", "description" => "","redirect_url" => nil,"template" => nil},
"contents" => [%{"name" => "Help","content_type" => "html","content" => "<p>Updated content area</p>", "meta" => "{\"global\":null,\"classes\":\"\"}","global" => nil}]
"page" => %{"slug" => "/", "title" => "Home Page",
"description" => "Home Page Description",
"redirect_url" => nil,"template" => nil},
"contents" => [%{"name" => "Help","content_type" => "html",
"content" => "<p>Updated content area</p>",
"meta" => "{\"global\":null,\"classes\":\"\"}",
"global" => nil}]
}
conn = put conn, "/thesis/update", payload
response = conn
|> put("/thesis/update", payload)
|> json_response(200)
assert response["id"] != nil
assert response["slug"] == "/"
assert response["title"] == "Home Page"
assert response["description"] == "Home Page Description"
assert response["template"] == nil
assert response["redirect_url"] == nil
assert response["page_contents"] == []

conn = get conn, "/"
refute html_response(conn, 200) =~ "<h4>Help</h4>"
Expand All @@ -29,10 +43,25 @@ defmodule Example.PageControllerTest do

# Updated content
payload = %{
"page" => %{"slug" => "/about", "title" => "", "description" => "","redirect_url" => nil,"template" => nil},
"contents" => [%{"name" => "Resources","content_type" => "html","content" => "<p>Updated content area</p>", "meta" => "{\"global\":null,\"classes\":\"\"}","global" => nil}]
"page" => %{"slug" => "/about", "title" => "About Page",
"description" => "About Page Description",
"redirect_url" => nil,"template" => nil},
"contents" => [%{"name" => "Resources","content_type" => "html",
"content" => "<p>Updated content area</p>",
"meta" => "{\"global\":null,\"classes\":\"\"}",
"global" => nil}]
}
conn = put conn, "/thesis/update", payload
response = conn
|> put("/thesis/update", payload)
|> json_response(200)
assert response["id"] != nil
assert response["slug"] == "/about"
assert response["title"] == "About Page"
assert response["description"] == "About Page Description"
assert response["template"] == nil
assert response["redirect_url"] == nil
assert response["page_contents"] == []


# Verify content was updated and default is gone
conn = get conn, "/about"
Expand Down
31 changes: 14 additions & 17 deletions apps/example/test/ecto_store_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -16,33 +16,33 @@ defmodule EctoStoreTest do
slug = random_slug
last_pc_id = last_updated(PageContent) && last_updated(PageContent) || 0

:ok = @store.update(valid_static_page(slug), [valid_text_page_content])
{:ok, page} = @store.update(valid_static_page(slug), [valid_text_page_content])

assert last_updated(Page).slug == "/" <> slug
assert page.slug == "/" <> slug
assert last_updated(PageContent).id > last_pc_id
end

test "Save global content to a page not edited before" do
slug = random_slug

:ok = @store.update(valid_static_page(slug), [valid_global_content])
{:ok, page} = @store.update(valid_static_page(slug), [valid_global_content])

assert last_updated(Page).slug == "/" <> slug
assert page.slug == "/" <> slug
assert last_updated(PageContent).page_id == nil
end

test "Save more than one areas at the same time" do
:ok = @store.update(valid_static_page, [valid_text_page_content, valid_html_page_content, valid_global_content])
{:ok, page} = @store.update(valid_static_page, [valid_text_page_content, valid_html_page_content, valid_global_content])

assert last_updated(Page).slug == valid_static_page["slug"]
assert page.slug == valid_static_page["slug"]
last_3 = last_updated(PageContent, 3)
assert Enum.at(last_3, 0).content_type == "text"
assert Enum.at(last_3, 1).content_type == "html"
assert Enum.at(last_3, 2).content_type == "raw_html"
end

test "Save global area on one page; retrieve on a different page that's not yet in database" do
:ok = @store.update(valid_static_page, [valid_global_content])
{:ok, _page} = @store.update(valid_static_page, [valid_global_content])

records = @store.page_contents("/" <> random_slug)

Expand All @@ -52,8 +52,8 @@ defmodule EctoStoreTest do

test "Retrieves page content as well as well as global content saved on a different page" do
slug = random_slug
:ok = @store.update(valid_static_page(random_slug), [valid_global_content])
:ok = @store.update(valid_static_page(slug), [valid_html_page_content])
{:ok, _page} = @store.update(valid_static_page(slug), [valid_global_content])
{:ok, _page} = @store.update(valid_static_page(slug), [valid_html_page_content])

records = @store.page_contents("/" <> slug)

Expand All @@ -62,12 +62,11 @@ defmodule EctoStoreTest do

test "First adds page, then deletes page found by slug" do
slug = random_slug
:ok = @store.update(valid_static_page(slug), [valid_html_page_content])
{:ok, page} = @store.update(valid_static_page(slug), [valid_html_page_content])

assert @store.page("/" <> slug).slug == "/" <> slug
assert last_updated(Page).slug == "/" <> slug
assert page.slug == "/" <> slug

:ok = @store.delete(valid_static_page(slug))
{:ok, _} = @store.delete(valid_static_page(slug))

refute @store.page("/" <> slug)
end
Expand All @@ -79,19 +78,17 @@ defmodule EctoStoreTest do
pc2 = %{"name" => "B", "content_type" => "text", "content" => "Yo"}
pc3 = %{"name" => "C", "content_type" => "raw_html", "content" => "Wat", "global" => "true"}

:ok = store.update(%{"slug" => "/test"}, [pc1, pc2, pc3])
{:ok, page1} = store.update(%{"slug" => "/test"}, [pc1, pc2, pc3])

pc4 = %{"name" => "D", "content_type" => "text", "content" => "Other page"}
pc5 = %{"name" => "E", "content_type" => "html", "content" => "<p>Other global</p>", "global" => "true"}

:ok = store.update(%{"slug" => "/asdf"}, [pc4, pc5])
{:ok, page2} = store.update(%{"slug" => "/asdf"}, [pc4, pc5])

page1 = store.page("/test")
assert is_integer(page1.id)
assert page1.id > 0
assert page1.slug == "/test"

page2 = store.page("/asdf")
assert is_integer(page2.id)
assert page2.id > 0
assert page2.slug == "/asdf"
Expand Down
2 changes: 1 addition & 1 deletion apps/example/web/static/js/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
//
// If you no longer want to use a dependency, remember
// to also remove its path from "config.paths.watched".
import "phoenix_html"
// import "phoenix_html"

// Import local files
//
Expand Down
3 changes: 2 additions & 1 deletion lib/mix/tasks/thesis.install.ex
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ defmodule Mix.Tasks.Thesis.Install do
"add_indexes_to_tables",
"add_template_and_redirect_url_to_thesis_pages",
"change_content_default_for_page_content",
"create_thesis_files_table"
"create_thesis_files_table",
"add_a_unique_index_on_slug"
]
@template_files [
{"priv/templates/thesis.install/thesis_auth.exs", "lib/thesis_auth.ex"}
Expand Down
12 changes: 8 additions & 4 deletions lib/thesis/api_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,17 @@ defmodule Thesis.ApiController do
def assets(conn, _params), do: conn

def update(conn, %{"contents" => contents, "page" => page}) do
:ok = store.update(page, contents)
json conn, %{}
{:ok, page} = store.update(page, contents)
conn
|> assign(:thesis_page, page)
|> render "page.json"
end

def delete(conn, %{"path" => path}) do
:ok = store.delete(%{"slug" => path})
json conn, %{}
{:ok, page} = store.delete(%{"slug" => path})
conn
|> assign(:thesis_page, page)
|> render "page.json"
end

def import_file(conn, %{"image_url" => ""}), do: json conn, %{path: ""}
Expand Down
13 changes: 12 additions & 1 deletion lib/thesis/models/page.ex
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ defmodule Thesis.Page do
title, description, and redirect if necessary.
"""
use Ecto.Schema
import Ecto.Changeset, only: [cast: 3, validate_required: 2]
import Ecto.Changeset, only: [cast: 3, validate_required: 2, fetch_field: 2, put_change: 3, unique_constraint: 2]
import Thesis.Utilities, only: [slugify: 1]

@type t :: %Thesis.Page{
id: any,
Expand Down Expand Up @@ -52,7 +53,17 @@ defmodule Thesis.Page do
def changeset(page, params \\ %{}) do
page
|> cast(params, @valid_attributes)
|> unique_constraint(:slug)
|> validate_required(@required_attributes)
|> parameterize_slug
end

defp parameterize_slug(changeset) do
if {:data, slug} = fetch_field(changeset, :slug) do
Copy link
Member

Choose a reason for hiding this comment

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

Can you change this to a case statement? For some reason I like that better in this case, like you did in ecto_store.

put_change(changeset, :slug, slugify(slug))
else
changeset
end
end

end
13 changes: 8 additions & 5 deletions lib/thesis/stores/ecto_store.ex
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,11 @@ defmodule Thesis.EctoStore do
"""
def update(page_params, contents_params) do
page = save_page(page_params)
save_page_contents(page, contents_params)
:ok

case save_page_contents(page, contents_params) do
:ok -> {:ok, page}
:error -> {:error, page}
end
end

@doc """
Expand All @@ -94,7 +97,7 @@ defmodule Thesis.EctoStore do
def delete(%{"slug" => slug}) do
page = page(slug)
repo.delete!(page)
:ok
{:ok, page}
end

defp save_page(%{"slug" => slug} = page_params) do
Expand All @@ -108,8 +111,8 @@ defmodule Thesis.EctoStore do
preloaded_contents = page_contents(page)

contents_params
|> Enum.map(fn(x) -> content_changeset(x, page, preloaded_contents) end)
|> Enum.each(fn(x) -> repo.insert_or_update!(x) end)
|> Enum.map(&(content_changeset(&1, page, preloaded_contents)))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should use Ecto.Multi here (future suggestion).

Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

I agree...can you file an issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filed in #113

|> Enum.each(&(repo.insert_or_update!(&1)))

:ok
end
Expand Down
6 changes: 3 additions & 3 deletions lib/thesis/stores/store.ex
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ defmodule Thesis.Store do

@doc """
Updates the given page (identified by its slug) with the given map of
string keys and Thesis.PageContent structs. Returns `:ok`.
string keys and Thesis.PageContent structs. Returns tuple `{:ok, page}`.
Copy link
Member

Choose a reason for hiding this comment

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

These are breaking changes. But I think we should do it now, for sure. It'll be harder to change later.


update(%{"slug" => "/"}, %{"Heading" => "My Heading Content"})
"""
@callback update(%{String.t => String.t}, map) :: atom
@callback update(%{String.t => String.t}, map) :: {atom, Thesis.Page.t}

@doc """
Deletes the given page (identified by its slug). Returns `:ok`.
Deletes the given page (identified by its slug). Returns tuple `{:ok, page}`.

delete(%{"slug" => "/asdf"})
"""
Expand Down
26 changes: 20 additions & 6 deletions lib/thesis/utilities.ex
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,36 @@ defmodule Thesis.Utilities do


@doc """
Removes special characters, keeps dashes and underscores, and replaces spaces
with dashes. Also downcases the entire string.
Slugifies the url and then removes slashes

iex> import Thesis.Utilities
iex> parameterize("Jamon is so cool!")
"jamon-is-so-cool"
iex> parameterize("%#d50SDF dfsJ FDS lkdsf f dfka a")
iex> parameterize("%#d50SDF dfsJ FDS / lkdsf f dfka a")
"d50sdf-dfsj-fds--lkdsf-f-dfka---a"
"""
def parameterize(str) do
str = Regex.replace(~r/[^a-z0-9\-\s\.]/i, str, "")
Regex.split(~r/\%20|\s/, str)
|> Enum.join("-")
str
|> slugify
|> String.replace(~r/\//i, "")
|> String.downcase
end

@doc """
Removes special characters, keeps dashes and underscores, and replaces spaces
with dashes.

iex> import Thesis.Utilities
iex> slugify("Jamon is so cool!")
"Jamon-is-so-cool"
Copy link
Member

Choose a reason for hiding this comment

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

oh yeah

iex> slugify("%#d50SDF dfsJ FDS/lkdsf f dfka a")
"d50SDF-dfsJ-FDS/lkdsf-f-dfka---a"
"""
def slugify(str) do
str = Regex.replace(~r/[^A-z0-9\-\s\.\/]/i, str, "")
Regex.split(~r/\%20|\s/, str)
|> Enum.join("-")
end
@doc """
Generates a random string of letters of a given length.

Expand Down
38 changes: 38 additions & 0 deletions lib/thesis/views/api_view.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
defmodule Thesis.ApiView do
use Thesis.View, :view


def render("page.json", %{thesis_page: page} = assigns) do
%{
id: page.id,
slug: page.slug,
title: page.title,
description: page.description,
redirect_url: page.redirect_url,
page_contents: render("page_contents.json", assigns[:page_contents]),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that page_contents will always be an empty list here, as we aren't assigning :page_contents in the controller -- this is only here for possible future need.

inserted_at: page.inserted_at,
updated_at: page.updated_at
}
end
Copy link
Member

Choose a reason for hiding this comment

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

Are we missing the description here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like it -- note that the javascript only makes use of the slug in the response currently, at any rate.

def render("page.json", nil), do: %{}

def render("page_contents.json", nil), do: []
def render("page_contents.json", page_contents) when is_list(page_contents) do
Enum.map(page_contents, fn(page_content) ->
render("page_content.json", page_content)
end)
end

def render("page_content.json", page_content) do
%{
id: page_content.id,
page_id: page_content.page_id,
name: page_content.name,
content: page_content.content,
content_type: page_content.content_type,
meta: page_content.meta,
inserted_at: page_content.inserted_at,
updated_at: page_content.updated_at
}
end
end
3 changes: 2 additions & 1 deletion mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ defmodule Thesis.Mixfile do
end

def application do
[applications: [:logger, :phoenix, :phoenix_html, :plug, :httpoison]]
[applications: [:logger, :phoenix, :phoenix_html, :plug,
:httpoison, :html_sanitize_ex]]
end

defp deps do
Expand Down
Loading