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

create tags and captures_tags tables #16

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
22 changes: 22 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
language: elixir
elixir:
- 1.9
otp_release:
- 22.1.8
env:
- MIX_ENV=test
cache:
directories:
- _build
- deps
services:
- postgresql
env:
global:
- MIX_ENV=test
before_script:
- mix do ecto.create, ecto.migrate
script:
- mix cover
after_success:
- bash <(curl -s https://codecov.io/bash)
11 changes: 10 additions & 1 deletion lib/app_api/captures.ex
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ defmodule AppApi.Captures do

alias AppApi.Captures.Capture
alias AppApi.Timers.Timer
alias AppApi.Tags.Tag

@doc """
Returns the list of captures.
Expand All @@ -20,6 +21,8 @@ defmodule AppApi.Captures do
"""
def list_captures do
Repo.all(Capture)
|> Repo.preload(timers: from(t in Timer, order_by: [desc: t.inserted_at]))
|> Repo.preload(tags: from(t in Tag, order_by: [asc: t.text]))
end

@doc """
Expand All @@ -39,14 +42,18 @@ defmodule AppApi.Captures do
def get_capture!(id) do
Repo.get!(Capture, id)
|> Repo.preload(timers: from(t in Timer, order_by: [desc: t.inserted_at]))
|> Repo.preload(tags: from(t in Tag, order_by: [asc: t.text]))
end

def get_capture_by_id_person(id_person) do
query =
from c in Capture,
where: c.id_person == ^id_person,
order_by: [desc: c.inserted_at],
preload: [timers: ^from(t in Timer, order_by: [desc: t.inserted_at])]
preload: [
timers: ^from(t in Timer, order_by: [desc: t.inserted_at]),
tags: ^from(t in Tag, order_by: [asc: t.text])
]

Repo.all(query)
end
Expand All @@ -68,6 +75,7 @@ defmodule AppApi.Captures do
|> Capture.changeset(attrs)
|> Repo.insert!()
|> Repo.preload(timers: from(t in Timer, order_by: [desc: t.inserted_at]))
|> Repo.preload(tags: from(t in Tag, order_by: [asc: t.text]))
end

@doc """
Expand All @@ -87,6 +95,7 @@ defmodule AppApi.Captures do
|> Capture.changeset(attrs)
|> Repo.update!()
|> Repo.preload(timers: from(t in Timer, order_by: [desc: t.inserted_at]))
|> Repo.preload(tags: from(t in Tag, order_by: [asc: t.text]))
end

@doc """
Expand Down
26 changes: 26 additions & 0 deletions lib/app_api/captures/capture.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,20 @@ defmodule AppApi.Captures.Capture do
use Ecto.Schema
import Ecto.Changeset
alias AppApi.Timers.Timer
alias AppApi.Tags.Tag
import Ecto.Query, warn: false
alias AppApi.Repo

schema "captures" do
field :completed, :boolean, default: false
field :id_person, :integer
field :text, :string
has_many :timers, Timer

many_to_many :tags, Tag,
join_through: "captures_tags",
on_replace: :delete

timestamps()
end

Expand All @@ -17,5 +24,24 @@ defmodule AppApi.Captures.Capture do
capture
|> cast(attrs, [:id_person, :text, :completed])
|> validate_required([:id_person, :text, :completed])
|> put_assoc(:tags, parse_tags(attrs))
end

defp parse_tags(params) do
Copy link
Member Author

Choose a reason for hiding this comment

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

Tags are passed as a string separated by a comma

(params["tags"] || "")
|> String.split(",")
|> Enum.map(&String.trim/1)
|> Enum.reject(&(&1 == ""))
|> insert_and_get_all()
end

defp insert_and_get_all([]) do
[]
end

defp insert_and_get_all(names) do
maps = Enum.map(names, &%{text: &1})
Repo.insert_all(Tag, maps, on_conflict: :nothing)
Copy link
Member Author

Choose a reason for hiding this comment

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

A conflict can appears when inserting tags, for example on a race condition. If mutiple tag are inserted at the same time with the same name the postgres index will report an error. The on_conflict catch this error and do nothing as at least one tag will be inserted with the correct name.

Copy link
Member

Choose a reason for hiding this comment

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

@SimonLab ideally the index should be based on the person_id and tag_id so that different people can have the same tag. But I think we need a dedicated issue for tag ownership. 💭

Repo.all(from t in Tag, where: t.name in ^names)
end
end
19 changes: 19 additions & 0 deletions lib/app_api/tags/tag.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
defmodule AppApi.Tags.Tag do
use Ecto.Schema
import Ecto.Changeset
alias AppApi.Captures.Capture

schema "tags" do
field :text, :string
field :id_person, :integer

timestamps()
end

@doc false
def changeset(tag, attrs) do
tag
|> cast(attrs, [:text, :id_person])
|> validate_required([:text])
end
end
16 changes: 13 additions & 3 deletions mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,14 @@ defmodule AppApi.MixProject do
compilers: [:phoenix, :gettext] ++ Mix.compilers(),
start_permanent: Mix.env() == :prod,
aliases: aliases(),
deps: deps()
deps: deps(),
test_coverage: [tool: ExCoveralls],
preferred_cli_env: [
coveralls: :test,
"coveralls.detail": :test,
"coveralls.post": :test,
"coveralls.html": :test
]
]
end

Expand Down Expand Up @@ -43,7 +50,8 @@ defmodule AppApi.MixProject do
{:plug_cowboy, "~> 2.0"},
{:cors_plug, "~> 2.0"},
{:httpoison, "~> 1.6"},
{:poison, "~> 4.0"}
{:poison, "~> 4.0"},
{:excoveralls, "~> 0.12.2", only: [:dev, :test]}
]
end

Expand All @@ -57,7 +65,9 @@ defmodule AppApi.MixProject do
[
"ecto.setup": ["ecto.create", "ecto.migrate", "run priv/repo/seeds.exs"],
"ecto.reset": ["ecto.drop", "ecto.setup"],
test: ["ecto.create --quiet", "ecto.migrate", "test"]
test: ["ecto.create --quiet", "ecto.migrate", "test"],
cover: ["coveralls.json"],
"cover.html": ["coveralls.html"]
]
end
end
1 change: 1 addition & 0 deletions mix.lock
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"decimal": {:hex, :decimal, "1.8.1", "a4ef3f5f3428bdbc0d35374029ffcf4ede8533536fa79896dd450168d9acdf3c", [:mix], [], "hexpm", "3cb154b00225ac687f6cbd4acc4b7960027c757a5152b369923ead9ddbca7aec"},
"ecto": {:hex, :ecto, "3.3.2", "002aa428c752a8ee4bb65baa9d1041f0514d7435d2e21d58cb6aa69a1076721d", [:mix], [{:decimal, "~> 1.6 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}], "hexpm", "c651e3ea0919241da314f518404b84449c8569525c4d4cf0f3d16f1d5d0a560c"},
"ecto_sql": {:hex, :ecto_sql, "3.3.3", "7d8962d39f16181c1df1bbd0f64aa392bd9ce0b9f8ff5ff21d43dca3d624eee7", [:mix], [{:db_connection, "~> 2.2", [hex: :db_connection, repo: "hexpm", optional: false]}, {:ecto, "~> 3.4 or ~> 3.3.2", [hex: :ecto, repo: "hexpm", optional: false]}, {:myxql, "~> 0.3.0", [hex: :myxql, repo: "hexpm", optional: true]}, {:postgrex, "~> 0.15.0", [hex: :postgrex, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "3d5b8e14836d930448d79ab6ac325501c3c62caed83c34791d5af77718766795"},
"excoveralls": {:hex, :excoveralls, "0.12.3", "2142be7cb978a3ae78385487edda6d1aff0e482ffc6123877bb7270a8ffbcfe0", [:mix], [{:hackney, "~> 1.0", [hex: :hackney, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "568a3e616c264283f5dea5b020783ae40eef3f7ee2163f7a67cbd7b35bcadada"},
"gettext": {:hex, :gettext, "0.17.4", "f13088e1ec10ce01665cf25f5ff779e7df3f2dc71b37084976cf89d1aa124d5c", [:mix], [], "hexpm", "3c75b5ea8288e2ee7ea503ff9e30dfe4d07ad3c054576a6e60040e79a801e14d"},
"hackney": {:hex, :hackney, "1.15.2", "07e33c794f8f8964ee86cebec1a8ed88db5070e52e904b8f12209773c1036085", [:rebar3], [{:certifi, "2.5.1", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "6.0.0", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "1.0.1", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "~>1.1", [hex: :mimerl, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "1.1.5", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}], "hexpm", "e0100f8ef7d1124222c11ad362c857d3df7cb5f4204054f9f0f4a728666591fc"},
"httpoison": {:hex, :httpoison, "1.6.2", "ace7c8d3a361cebccbed19c283c349b3d26991eff73a1eaaa8abae2e3c8089b6", [:mix], [{:hackney, "~> 1.15 and >= 1.15.2", [hex: :hackney, repo: "hexpm", optional: false]}], "hexpm", "aa2c74bd271af34239a3948779612f87df2422c2fdcfdbcec28d9c105f0773fe"},
Expand Down
15 changes: 15 additions & 0 deletions priv/repo/migrations/20200420133108_create_tags.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
defmodule AppApi.Repo.Migrations.CreateTags do
use Ecto.Migration

def change do
create table(:tags) do
add :text, :string
add :id_person, :integer

timestamps()
end

create unique_index(:tags, [:text])
Copy link
Member Author

Choose a reason for hiding this comment

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

This index makes sure the database doesn't contain tags with the same name

Copy link
Member

Choose a reason for hiding this comment

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

@SimonLab which issue requirement or acceptance criteria is this for?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've tried to follow the requirements from dwyl/app#245

Copy link
Member

Choose a reason for hiding this comment

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

@SimonLab thanks for clarifying. In which case it could be worth having that issue linked in the PR description and having it "in-progress" on the Kanban board (which makes it easier for the reviewer) 😉

end

end
19 changes: 19 additions & 0 deletions priv/repo/migrations/20200420133553_captures_tags.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
defmodule AppApi.Repo.Migrations.CapturesTags do
use Ecto.Migration

def change do
create table(:captures_tags, primary_key: false) do
add(:capture_id, references(:captures, on_delete: :delete_all), primary_key: true)
add(:tag_id, references(:tags, on_delete: :delete_all), primary_key: true)
timestamps()
end

create(index(:captures_tags, [:capture_id]))
create(index(:captures_tags, [:tag_id]))

create(
unique_index(:captures_tags, [:capture_id, :tag_id], name: :capture_id_tag_id_unique_index)
)

end
end
17 changes: 7 additions & 10 deletions test/app_api/captures_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,14 @@ defmodule AppApi.CapturesTest do
describe "captures" do
alias AppApi.Captures.Capture

@valid_attrs %{completed: true, id_person: 42, text: "some text"}
@update_attrs %{completed: false, id_person: 43, text: "some updated text"}
@invalid_attrs %{completed: nil, id_person: nil, text: nil}
@valid_attrs %{completed: true, id_person: 42, text: "some text", tags: "tag1, tag2"}
@update_attrs %{completed: false, id_person: 43, text: "some updated text", tags: "" }
@invalid_attrs %{completed: nil, id_person: nil, text: nil, tags: ""}

def capture_fixture(attrs \\ %{}) do
{:ok, capture} =
attrs
|> Enum.into(@valid_attrs)
|> Captures.create_capture()

capture
end

test "list_captures/0 returns all captures" do
Expand All @@ -30,27 +27,27 @@ defmodule AppApi.CapturesTest do
end

test "create_capture/1 with valid data creates a capture" do
assert {:ok, %Capture{} = capture} = Captures.create_capture(@valid_attrs)
assert %Capture{} = capture = Captures.create_capture(@valid_attrs)
assert capture.completed == true
assert capture.id_person == 42
assert capture.text == "some text"
end

test "create_capture/1 with invalid data returns error changeset" do
assert {:error, %Ecto.Changeset{}} = Captures.create_capture(@invalid_attrs)
assert catch_error(Captures.create_capture(@invalid_attrs))
end

test "update_capture/2 with valid data updates the capture" do
capture = capture_fixture()
assert {:ok, %Capture{} = capture} = Captures.update_capture(capture, @update_attrs)
assert %Capture{} = capture = Captures.update_capture(capture, @update_attrs)
assert capture.completed == false
assert capture.id_person == 43
assert capture.text == "some updated text"
end

test "update_capture/2 with invalid data returns error changeset" do
capture = capture_fixture()
assert {:error, %Ecto.Changeset{}} = Captures.update_capture(capture, @invalid_attrs)
assert catch_error(Captures.update_capture(capture, @invalid_attrs))
assert capture == Captures.get_capture!(capture.id)
end

Expand Down