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 2 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
39 changes: 39 additions & 0 deletions lib/thesis/views/api_view.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
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", page_contents)
when is_nil(page_contents), do: []
Copy link
Member

Choose a reason for hiding this comment

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

why not put this on 18?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yulolimum I like keeping line-length under 80 characters -- I have a guide in my editor that shows when a line is over that length.

Copy link
Member

Choose a reason for hiding this comment

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

gotcha - i have mine set to 9999, just like most of my z-indexes
¯_(ツ)_/¯

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