Skip to content

Commit

Permalink
Zone Schema and Model
Browse files Browse the repository at this point in the history
Schema
------
Zones are of two kinds (State, and Country), and this is another non-exclusive
subtype-supertype relationship.

I've chosen to not keep schemas for StateZone and CountryZone subtype, as
there's no table backing them. I considered making them embedded_schemas, but
we can't put associations in them (can't do state_zone_struct.members).

Model
-----

Conversely, there's no model for Zone, instead we have models for StateZone and
CountryZone.
update/3 has a gotcha: there's no way to add/remove single members from the
list.

* This is because the UI element used to update the member list will definitely
  have the "new" member list.
* update reduces the number of inserts and deletes to a minimum.

Summary
-------

* Added tests and factory for zone schema, model
* Tweaked credo config
  - Credo was complaining about code duplication, so instead of fixing my
code, I shut credo up. ヽ(`Д´)ノ︵ ┻━┻
* Tests use the state and country factory/fixture.

Misc
----

* Fix contribution guide
* Fix stock tests that depend on absence of records.
  • Loading branch information
oyeb authored and pkrawat1 committed Oct 4, 2018
1 parent deb0076 commit 12b5367
Show file tree
Hide file tree
Showing 20 changed files with 802 additions and 36 deletions.
2 changes: 1 addition & 1 deletion .credo.exs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
# or the `schema` macro in Ecto schemas to trigger DuplicatedCode, just
# set the `excluded_macros` parameter to `[:schema, :setup, :test]`.
#
{Credo.Check.Design.DuplicatedCode, excluded_macros: []},
{Credo.Check.Design.DuplicatedCode, excluded_macros: [], mass_threshold: 41},

# You can also customize the exit_status of each check.
# If you don't want TODO comments to cause `mix credo` to fail, just
Expand Down
4 changes: 2 additions & 2 deletions .scripts/post-commit
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@ RED='\033[1;31m'
LGRAY='\033[1;30m'
NC='\033[0m' # No Color

printf "${RED}Running 'mix credo --strict --format=oneline' on project...${NC}\n"
mix credo --strict --format=oneline
printf "${RED}Running 'mix credo --strict' on project...${NC}\n"
mix credo --strict
echo
5 changes: 3 additions & 2 deletions CONTIBUTING.md → CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ The repo includes some handy `git` hooks under `.scripts/`:

We strongly recommend that you set up these git hooks on your machine by:
```sh
ln -s .scripts/pre-commit .git/hooks/pre-commit
ln -s .scripts/post-commit .git/hooks/post-commit
# in the project root!
ln -sf ../../.scripts/pre-commit .git/hooks/pre-commit
ln -sf ../../.scripts/post-commit .git/hooks/post-commit
```

> Note that our CI will fail your PR if you dont run `mix format` in the project
Expand Down
131 changes: 131 additions & 0 deletions apps/snitch_core/lib/core/data/model/zone/country_zone.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
defmodule Snitch.Data.Model.CountryZone do
@moduledoc """
CountryZone API
"""
use Snitch.Data.Model

import Ecto.Query

alias Ecto.Multi
alias Snitch.Data.Schema.{CountryZoneMember, Zone, Country}

@doc """
Creates a new country `Zone` whose members are `country_ids`.
`country_ids` is a list of primary keys of the `Snitch.Data.Schema.Country`s that
make up this zone. Duplicate IDs are ignored.
"""
@spec create(String.t(), String.t(), [non_neg_integer]) :: term
def create(name, description, country_ids) do
zone_params = %{name: name, description: description, zone_type: "C"}
zone_changeset = Zone.changeset(%Zone{}, zone_params, :create)

Multi.new()
|> Multi.insert(:zone, zone_changeset)
|> Multi.run(:members, fn %{zone: zone} -> insert_members(country_ids, zone) end)
|> Repo.transaction()
|> case do
{:ok, %{zone: zone}} -> {:ok, zone}
error -> error
end
end

@spec delete(non_neg_integer | Zone.t()) ::
{:ok, Zone.t()} | {:error, Ecto.Changeset.t()} | {:error, :not_found}
def delete(id_or_instance) do
QH.delete(Zone, id_or_instance, Repo)
end

@spec get(map | non_neg_integer) :: Zone.t() | nil
def get(query_fields_or_primary_key) do
QH.get(Zone, query_fields_or_primary_key, Repo)
end

@spec get_all() :: [Zone.t()]
def get_all, do: Repo.all(from(z in Zone, where: z.zone_type == "C"))

@doc """
Returns the list of `Country` IDs that make up this zone.
"""
@spec member_ids(non_neg_integer) :: [non_neg_integer]
def member_ids(zone_id) do
query = from(m in CountryZoneMember, where: m.zone_id == ^zone_id, select: m.country_id)
Repo.all(query)
end

@doc """
Returns the list of `Country` structs that make up this zone.
"""
@spec members(non_neg_integer) :: [Country.t()]
def members(zone_id) do
query =
from(
c in Country,
join: m in CountryZoneMember,
on: m.country_id == c.id,
where: m.zone_id == ^zone_id
)

Repo.all(query)
end

@doc """
Updates Zone params and sets the members as per `new_country_ids`.
This replaces the old members with the new ones. Duplicate IDs in the list are
ignored.
"""
@spec update(String.t(), String.t(), [non_neg_integer]) ::
{:ok, Zone.t()} | {:error, Ecto.Changeset.t()}
def update(zone, zone_params, new_country_ids) do
zone_changeset = Zone.changeset(zone, zone_params, :update)

old_members = MapSet.new(member_ids(zone.id))
new_members = MapSet.new(new_country_ids)
added = set_difference(new_members, old_members)
removed = set_difference(old_members, new_members)

delete_query =
from(m in CountryZoneMember, where: m.country_id in ^removed and m.zone_id == ^zone.id)

deletions_multi =
if length(removed) > 0 do
Multi.delete_all(%Multi{}, :removed, delete_query)
else
Multi.new()
end

Multi.new()
|> Multi.update(:zone, zone_changeset)
|> Multi.run(:added, fn _ -> insert_members(added, zone) end)
|> Multi.append(deletions_multi)
|> Repo.transaction()
|> case do
{:ok, %{zone: zone}} -> {:ok, zone}
error -> error
end
end

defp insert_members(country_ids, zone) do
country_ids
|> Stream.uniq()
|> Stream.map(
&CountryZoneMember.changeset(
%CountryZoneMember{},
%{country_id: &1, zone_id: zone.id},
:create
)
)
|> Stream.map(&Repo.insert/1)
|> Enum.reduce_while({:ok, []}, fn
{:ok, member}, {:ok, acc} -> {:cont, {:ok, [member | acc]}}
changeset, _acc -> {:halt, changeset}
end)
end

defp set_difference(a, b) do
a
|> MapSet.difference(b)
|> MapSet.to_list()
end
end
131 changes: 131 additions & 0 deletions apps/snitch_core/lib/core/data/model/zone/state_zone.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
defmodule Snitch.Data.Model.StateZone do
@moduledoc """
StateZone API
"""
use Snitch.Data.Model

import Ecto.Query

alias Ecto.Multi
alias Snitch.Data.Schema.{StateZoneMember, Zone, State}

@doc """
Creates a new state `Zone` whose members are `state_ids`.
`state_ids` is a list of primary keys of the `Snitch.Data.Schema.State`s that
make up this zone. Duplicate IDs are ignored.
"""
@spec create(String.t(), String.t(), [non_neg_integer]) :: term
def create(name, description, state_ids) do
zone_params = %{name: name, description: description, zone_type: "S"}
zone_changeset = Zone.changeset(%Zone{}, zone_params, :create)

Multi.new()
|> Multi.insert(:zone, zone_changeset)
|> Multi.run(:members, fn %{zone: zone} -> insert_members(state_ids, zone) end)
|> Repo.transaction()
|> case do
{:ok, %{zone: zone}} -> {:ok, zone}
error -> error
end
end

@spec delete(non_neg_integer | Zone.t()) ::
{:ok, Zone.t()} | {:error, Ecto.Changeset.t()} | {:error, :not_found}
def delete(id_or_instance) do
QH.delete(Zone, id_or_instance, Repo)
end

@spec get(map | non_neg_integer) :: Zone.t() | nil
def get(query_fields_or_primary_key) do
QH.get(Zone, query_fields_or_primary_key, Repo)
end

@spec get_all() :: [Zone.t()]
def get_all, do: Repo.all(from(z in Zone, where: z.zone_type == "S"))

@doc """
Returns the list of `State` IDs that make up this zone.
"""
@spec member_ids(non_neg_integer) :: [non_neg_integer]
def member_ids(zone_id) do
query = from(s in StateZoneMember, where: s.zone_id == ^zone_id, select: s.state_id)
Repo.all(query)
end

@doc """
Returns the list of `State` structs that make up this zone.
"""
@spec members(non_neg_integer) :: [State.t()]
def members(zone_id) do
query =
from(
s in State,
join: m in StateZoneMember,
on: m.state_id == s.id,
where: m.zone_id == ^zone_id
)

Repo.all(query)
end

@doc """
Updates Zone params and sets the members as per `new_state_ids`.
This replaces the old members with the new ones. Duplicate IDs in the list are
ignored.
"""
@spec update(String.t(), String.t(), [non_neg_integer]) ::
{:ok, Zone.t()} | {:error, Ecto.Changeset.t()}
def update(zone, zone_params, new_state_ids) do
zone_changeset = Zone.changeset(zone, zone_params, :update)

old_members = MapSet.new(member_ids(zone.id))
new_members = MapSet.new(new_state_ids)
added = set_difference(new_members, old_members)
removed = set_difference(old_members, new_members)

delete_query =
from(m in StateZoneMember, where: m.state_id in ^removed and m.zone_id == ^zone.id)

deletions_multi =
if length(removed) > 0 do
Multi.delete_all(%Multi{}, :removed, delete_query)
else
Multi.new()
end

Multi.new()
|> Multi.update(:zone, zone_changeset)
|> Multi.run(:added, fn _ -> insert_members(added, zone) end)
|> Multi.append(deletions_multi)
|> Repo.transaction()
|> case do
{:ok, %{zone: zone}} -> {:ok, zone}
error -> error
end
end

defp insert_members(state_ids, zone) do
state_ids
|> Stream.uniq()
|> Stream.map(
&StateZoneMember.changeset(
%StateZoneMember{},
%{state_id: &1, zone_id: zone.id},
:create
)
)
|> Stream.map(&Repo.insert/1)
|> Enum.reduce_while({:ok, []}, fn
{:ok, member}, {:ok, acc} -> {:cont, {:ok, [member | acc]}}
changeset, _acc -> {:halt, changeset}
end)
end

defp set_difference(a, b) do
a
|> MapSet.difference(b)
|> MapSet.to_list()
end
end
50 changes: 50 additions & 0 deletions apps/snitch_core/lib/core/data/schema/zone/country_zone_member.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
defmodule Snitch.Data.Schema.CountryZoneMember do
@moduledoc """
Models a CountryZone member.
"""
use Snitch.Data.Schema
alias Snitch.Data.Schema.{Zone, Country}

@typedoc """
CountryZoneMember struct.
## Fields
* `zone_id` uniquely determines both `Zone` and `CountryZone`
"""
@type t :: %__MODULE__{}

schema "snitch_country_zone_members" do
belongs_to(:zone, Zone)
belongs_to(:country, Country)
timestamps()
end

@update_fields ~w(country_id)a
@create_fields [:zone_id | @update_fields]

@doc """
Returns a `Zone` changeset.
"""
@spec changeset(t, map, :create | :update) :: Ecto.Changeset.t()
def changeset(country_zone, params, :create) do
country_zone
|> cast(params, @create_fields)
|> validate_required(@create_fields)
|> foreign_key_constraint(:zone_id)
|> unique_constraint(:country_id, name: :snitch_country_zone_members_zone_id_country_id_index)
|> foreign_key_constraint(:country_id)
|> check_constraint(
:zone_id,
name: :country_zone_exclusivity,
message: "does not refer a country zone"
)
end

def changeset(country_zone, params, :update) do
country_zone
|> cast(params, @update_fields)
|> foreign_key_constraint(:country_id)
|> unique_constraint(:country_id, name: :snitch_country_zone_members_zone_id_country_id_index)
end
end
50 changes: 50 additions & 0 deletions apps/snitch_core/lib/core/data/schema/zone/state_zone_member.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
defmodule Snitch.Data.Schema.StateZoneMember do
@moduledoc """
Models a StateZone member.
"""
use Snitch.Data.Schema
alias Snitch.Data.Schema.{Zone, State}

@typedoc """
StateZoneMember struct.
## Fields
* `zone_id` uniquely determines both `Zone` and `StateZone`
"""
@type t :: %__MODULE__{}

schema "snitch_state_zone_members" do
belongs_to(:zone, Zone)
belongs_to(:state, State)
timestamps()
end

@update_fields ~w(state_id)a
@create_fields [:zone_id | @update_fields]

@doc """
Returns a `Zone` changeset.
"""
@spec changeset(t, map, :create | :update) :: Ecto.Changeset.t()
def changeset(state_zone, params, :create) do
state_zone
|> cast(params, @create_fields)
|> validate_required(@create_fields)
|> foreign_key_constraint(:zone_id)
|> unique_constraint(:state_id, name: :snitch_state_zone_members_zone_id_state_id_index)
|> foreign_key_constraint(:state_id)
|> check_constraint(
:zone_id,
name: :state_zone_exclusivity,
message: "does not refer a state zone"
)
end

def changeset(state_zone, params, :update) do
state_zone
|> cast(params, @update_fields)
|> foreign_key_constraint(:state_id)
|> unique_constraint(:state_id, name: :snitch_state_zone_members_zone_id_state_id_index)
end
end
Loading

0 comments on commit 12b5367

Please sign in to comment.