Merge branch 'master' into ghatighroias/fix_test

config/config.exs

@@ -34,4 +34,9 @@ config :canary,
   repo: CoursePlanner.Repo,
   unauthorized_handler: {CoursePlanner.Helper, :handle_unauthorized}
 
+config :course_planner, CoursePlanner.NotifierScheduler,
+  jobs: [
+    {"0 18 * * *", {CoursePlanner.Notifications, :send_all_notifications, []}}
+  ]
+
 import_config "#{Mix.env}.exs"

lib/course_planner.ex

@@ -3,7 +3,7 @@ defmodule CoursePlanner do
   This is the main module of the app
   """
   use Application
-  alias CoursePlanner.{Endpoint, Repo, Notifier}
+  alias CoursePlanner.{Endpoint, Repo, Notifier, NotifierScheduler}
 
   def start(_type, _args) do
     import Supervisor.Spec
@@ -12,6 +12,7 @@ defmodule CoursePlanner do
       supervisor(Repo, []),
       supervisor(Endpoint, []),
       worker(Notifier, []),
+      worker(NotifierScheduler, []),
     ]
 
     opts = [strategy: :one_for_one, name: CoursePlanner.Supervisor]

lib/course_planner/mailer/user_email.ex

@@ -60,6 +60,7 @@ defmodule CoursePlanner.Mailer.UserEmail do
     end
   end
 
+  def build_summary(%{notifications: []}), do: {:error, :empty_notifications}
   def build_summary(%{name: name, email: email, notifications: notifications}) do
     new()
     |> from("admin@courseplanner.com")

lib/course_planner/notifications.ex

@@ -3,7 +3,8 @@ defmodule CoursePlanner.Notifications do
   Contains notification logic
   """
 
-  alias CoursePlanner.{User, Notification}
+  alias CoursePlanner.{User, Notification, Notifier, Repo}
+  import Ecto.Query
 
   def new, do: %Notification{}
 
@@ -16,4 +17,20 @@ defmodule CoursePlanner.Notifications do
   def to(%Notification{} = notification, %User{} = user),
     do: %{notification | user: user}
 
+  def send_all_notifications do
+    IO.puts "cron call"
+    Timex.today()
+    |> get_notifiable_users()
+    |> Enum.each(&Notifier.notify_all/1)
+  end
+
+  def get_notifiable_users(date) do
+    User
+    |> where([u],
+      fragment("?::date + ?", u.notified_at, u.notification_period_days) <= type(^date, Ecto.Date)
+      or is_nil(u.notified_at))
+    |> Repo.all()
+    |> Repo.preload(:notifications)
+  end
+
 end

lib/course_planner/notifier.ex

@@ -3,73 +3,28 @@ defmodule CoursePlanner.Notifier do
   Module responsible for notifying users through e-mail with changes
   """
   use GenServer
-  alias CoursePlanner.{Mailer, Mailer.UserEmail, Notification, Repo, User}
+  alias CoursePlanner.{Notification, User, Notifier.Server}
   require Logger
 
   @spec start_link :: GenServer.start_link
   def start_link do
-    GenServer.start_link(__MODULE__, [], name: __MODULE__)
+    GenServer.start_link(Server, [], name: Server)
   end
 
   @spec notify_user(Notification.t) :: GenServer.cast
   def notify_user(%Notification{} = notification) do
-    GenServer.cast(__MODULE__, {:send_email, notification})
+    GenServer.cast(Server, {:send_email, notification})
   end
 
   @spec notify_later(Notification.t) :: GenServer.cast
   def notify_later(%Notification{} = notification) do
-    GenServer.cast(__MODULE__, {:save_email, notification})
+    GenServer.cast(Server, {:save_email, notification})
   end
 
   @spec notify_all(User.t) :: GenServer.cast
   def notify_all(%User{notifications: []} = _user), do: :nothing
   def notify_all(%User{} = user) do
-    GenServer.cast(__MODULE__, {:notify_all, user})
+    GenServer.cast(Server, {:notify_all, user})
   end
 
-  @spec handle_cast({atom(), Notification.t} | atom(), any()) :: {:noreply, any()}
-  def handle_cast({:send_email, notification}, state) do
-    email = UserEmail.build_email(notification)
-    case Mailer.deliver(email) do
-      {:ok, _} ->
-        {:noreply, state}
-      {:error, reason} ->
-        Logger.error("Email delivery failed: #{Kernel.inspect reason}")
-        {:noreply, [{:error, reason, email} | state]}
-    end
-  end
-  def handle_cast({:save_email, notification}, state) do
-    changeset = Notification.changeset(notification)
-    case Repo.insert(changeset) do
-      {:ok, _} ->
-        {:noreply, state}
-      {:error, %{errors: errors, data: email}} ->
-        Logger.error("Email saving failed: #{Kernel.inspect errors}")
-        {:noreply, [{:error, errors, email} | state]}
-    end
-  end
-  def handle_cast({:notify_all, user}, state) do
-    email = UserEmail.build_summary(user)
-    case Mailer.deliver(email) do
-      {:ok, _} ->
-        delete_notifications(user)
-        {:noreply, state}
-      {:error, reason} ->
-        Logger.error("Email delivery failed: #{Kernel.inspect reason}")
-        {:noreply, [{:error, reason, email} | state]}
-    end
-  end
-  def handle_cast(_, state), do: {:noreply, state}
-
-  defp delete_notifications(user) do
-    user.notifications
-    |> Enum.each(&Repo.delete/1)
-  end
-
-  @doc """
-  This function is used to suppress unhandled message warnings from `Swoosh.Adapters.Test` during
-  unit tests
-  """
-  @spec handle_info(any(), any()) :: {:noreply, any()}
-  def handle_info(_, state), do: {:noreply, state}
 end

lib/course_planner/notifier/server.ex

@@ -0,0 +1,47 @@
+defmodule CoursePlanner.Notifier.Server do
+  @moduledoc false
+  use GenServer
+  alias CoursePlanner.{Mailer, Mailer.UserEmail, Notification, Repo, Users}
+  require Logger
+
+  @spec handle_cast({atom(), Notification.t} | atom(), any()) :: {:noreply, any()}
+  def handle_cast({:send_email, notification}, state) do
+    email = UserEmail.build_email(notification)
+    case Mailer.deliver(email) do
+      {:ok, _} ->
+        {:noreply, state}
+      {:error, reason} ->
+        Logger.error("Email delivery failed: #{Kernel.inspect reason}")
+        {:noreply, [{:error, reason, email} | state]}
+    end
+  end
+  def handle_cast({:save_email, notification}, state) do
+    changeset = Notification.changeset(notification)
+    case Repo.insert(changeset) do
+      {:ok, _} ->
+        {:noreply, state}
+      {:error, %{errors: errors, data: email}} ->
+        Logger.error("Email saving failed: #{Kernel.inspect errors}")
+        {:noreply, [{:error, errors, email} | state]}
+    end
+  end
+  def handle_cast({:notify_all, user}, state) do
+    email = UserEmail.build_summary(user)
+    case Mailer.deliver(email) do
+      {:ok, _} ->
+        Users.update_notifications(user)
+        {:noreply, state}
+      {:error, reason} ->
+        Logger.error("Email delivery failed: #{Kernel.inspect reason}")
+        {:noreply, [{:error, reason, email} | state]}
+    end
+  end
+  def handle_cast(_, state), do: {:noreply, state}
+
+  @doc """
+  This function is used to suppress unhandled message warnings from `Swoosh.Adapters.Test` during
+  unit tests
+  """
+  @spec handle_info(any(), any()) :: {:noreply, any()}
+  def handle_info(_, state), do: {:noreply, state}
+end

lib/course_planner/notifier_scheduler.ex

@@ -0,0 +1,5 @@
+defmodule CoursePlanner.NotifierScheduler do
+  @moduledoc false
+  use Quantum.Scheduler,
+    otp_app: :course_planner
+end

lib/course_planner/users.ex

@@ -2,8 +2,11 @@ defmodule CoursePlanner.Users do
   @moduledoc """
     Handle all interactions with Users, create, list, fetch, edit, and delete
   """
-  alias CoursePlanner.{Repo, User, Notifications}
-  alias Ecto.DateTime
+  alias CoursePlanner.{Repo, User, Notification, Notifications}
+  alias Ecto.{DateTime, Changeset, Multi}
+
+  import Ecto.Query
+
   @notifier Application.get_env(:course_planner, :notifier, CoursePlanner.Notifier)
 
   def all do
@@ -44,6 +47,23 @@ defmodule CoursePlanner.Users do
     |> @notifier.notify_later()
   end
 
+  def update_notifications(user) do
+    Multi.new()
+    |> delete_notifications(user)
+    |> mark_user_as_notified(user)
+    |> Repo.transaction()
+  end
+
+  defp delete_notifications(multi, user) do
+    notification_ids = Enum.map(user.notifications, &(&1.id))
+    q = from n in Notification, where: n.id in ^notification_ids
+    Multi.delete_all(multi, Notification, q)
+  end
+
+  defp mark_user_as_notified(multi, user) do
+    Multi.update(multi, User, Changeset.change(user, notified_at: Ecto.DateTime.utc()))
+  end
+
   def notify_all do
     User
     |> Repo.all()

mix.exs

@@ -28,7 +28,7 @@ defmodule CoursePlanner.Mixfile do
   def application do
     [mod: {CoursePlanner, []},
      applications: [:phoenix, :phoenix_pubsub, :phoenix_html, :cowboy, :logger, :gettext,
-                    :phoenix_ecto, :postgrex, :coherence, :swoosh]]
+                    :phoenix_ecto, :postgrex, :coherence, :swoosh, :quantum]]
   end
 
   # Specifies which paths to compile per environment.
@@ -54,7 +54,8 @@ defmodule CoursePlanner.Mixfile do
      {:dogma, "~> 0.1.0", only: [:dev, :test]},
      {:credo, "~> 0.7", only: [:dev, :test]},
      {:ex_machina, "~> 2.0", only: :test},
-     {:excoveralls, "~> 0.6", only: :test}
+     {:excoveralls, "~> 0.6", only: :test},
+     {:quantum, ">= 2.0.0-beta.1"},
    ]
   end
 

mix.lock

@@ -9,6 +9,7 @@
   "cowboy": {:hex, :cowboy, "1.1.2", "61ac29ea970389a88eca5a65601460162d370a70018afe6f949a29dca91f3bb0", [:rebar3], [{:cowlib, "~> 1.0.2", [hex: :cowlib, repo: "hexpm", optional: false]}, {:ranch, "~> 1.3.2", [hex: :ranch, repo: "hexpm", optional: false]}], "hexpm"},
   "cowlib": {:hex, :cowlib, "1.0.2", "9d769a1d062c9c3ac753096f868ca121e2730b9a377de23dec0f7e08b1df84ee", [:make], [], "hexpm"},
   "credo": {:hex, :credo, "0.8.4", "4e50acac058cf6292d6066e5b0d03da5e1483702e1ccde39abba385c9f03ead4", [:mix], [{:bunt, "~> 0.2.0", [hex: :bunt, repo: "hexpm", optional: false]}], "hexpm"},
+  "crontab": {:hex, :crontab, "1.1.1", "aed5d595a654d87e26cb43b4f95a02ef9771da96db3b24dcf7ab288c8a1e007a", [:mix], [{:ecto, "~> 1.0 or ~> 2.0 or ~> 2.1", [hex: :ecto, repo: "hexpm", optional: true]}], "hexpm"},
   "db_connection": {:hex, :db_connection, "1.1.2", "2865c2a4bae0714e2213a0ce60a1b12d76a6efba0c51fbda59c9ab8d1accc7a8", [:mix], [{:connection, "~> 1.0.2", [hex: :connection, repo: "hexpm", optional: false]}, {:poolboy, "~> 1.5", [hex: :poolboy, repo: "hexpm", optional: true]}, {:sbroker, "~> 1.0", [hex: :sbroker, repo: "hexpm", optional: true]}], "hexpm"},
   "decimal": {:hex, :decimal, "1.3.1", "157b3cedb2bfcb5359372a7766dd7a41091ad34578296e951f58a946fcab49c6", [:mix], [], "hexpm"},
   "dogma": {:hex, :dogma, "0.1.15", "5bceba9054b2b97a4adcb2ab4948ca9245e5258b883946e82d32f785340fd411", [:mix], [{:poison, ">= 2.0.0", [hex: :poison, repo: "hexpm", optional: false]}], "hexpm"},
@@ -35,6 +36,7 @@
   "poison": {:hex, :poison, "2.2.0", "4763b69a8a77bd77d26f477d196428b741261a761257ff1cf92753a0d4d24a63", [:mix], [], "hexpm"},
   "poolboy": {:hex, :poolboy, "1.5.1", "6b46163901cfd0a1b43d692657ed9d7e599853b3b21b95ae5ae0a777cf9b6ca8", [:rebar], [], "hexpm"},
   "postgrex": {:hex, :postgrex, "0.13.3", "c277cfb2a9c5034d445a722494c13359e361d344ef6f25d604c2353185682bfc", [:mix], [{:connection, "~> 1.0", [hex: :connection, repo: "hexpm", optional: false]}, {:db_connection, "~> 1.1", [hex: :db_connection, repo: "hexpm", optional: false]}, {:decimal, "~> 1.0", [hex: :decimal, repo: "hexpm", optional: false]}], "hexpm"},
+  "quantum": {:hex, :quantum, "2.0.0", "e09848e5ad702810942a585385ee59b35ff7780e0d05e992139d4920108f53da", [:mix], [{:calendar, "~> 0.17", [hex: :calendar, repo: "hexpm", optional: true]}, {:crontab, "~> 1.1", [hex: :crontab, repo: "hexpm", optional: false]}, {:timex, "~> 3.1.13", [hex: :timex, repo: "hexpm", optional: true]}], "hexpm"},
   "ranch": {:hex, :ranch, "1.3.2", "e4965a144dc9fbe70e5c077c65e73c57165416a901bd02ea899cfd95aa890986", [:rebar3], [], "hexpm"},
   "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.1", "28a4d65b7f59893bc2c7de786dec1e1555bd742d336043fe644ae956c3497fbe", [:make, :rebar], [], "hexpm"},
   "swoosh": {:hex, :swoosh, "0.7.0", "bd29a06d95ee70e1ae44d5db76ba5920ae7a60fd418fe559b6b1a24c179521af", [:mix], [{:cowboy, "~> 1.0", [hex: :cowboy, repo: "hexpm", optional: true]}, {:gen_smtp, "~> 0.11", [hex: :gen_smtp, repo: "hexpm", optional: true]}, {:hackney, "~> 1.6", [hex: :hackney, repo: "hexpm", optional: false]}, {:plug, "~> 1.1", [hex: :plug, repo: "hexpm", optional: true]}, {:poison, "~> 1.5 or ~> 2.0 or ~> 3.0", [hex: :poison, repo: "hexpm", optional: false]}], "hexpm"},

priv/repo/migrations/20170803180536_add_notification_configuration.exs

@@ -0,0 +1,17 @@
+defmodule CoursePlanner.Repo.Migrations.AddNotificationConfiguration do
+  use Ecto.Migration
+
+  def up do
+    alter table(:users) do
+      add :notified_at, :naive_datetime
+      add :notification_period_days, :integer, null: false, default: 1
+    end
+  end
+
+  def down do
+    alter table(:users) do
+      remove :notified_at
+      remove :notification_period_days
+    end
+  end
+end

priv/repo/seeds.exs

@@ -38,18 +38,6 @@ CoursePlanner.Repo.delete_all CoursePlanner.User
     })
 |> CoursePlanner.Repo.insert!
 
-%CoursePlanner.SystemVariable{}
-|> CoursePlanner.SystemVariable.changeset(
-    %{
-      key: "NOTIFICATION_FREQUENCY",
-      value: "1",
-      type: "integer",
-      editable: true,
-      visible: true,
-      required: true
-    })
-|> CoursePlanner.Repo.insert!
-
 %CoursePlanner.SystemVariable{}
 |> CoursePlanner.SystemVariable.changeset(
     %{

test/lib/course_planner/notifications_test.exs

@@ -0,0 +1,27 @@
+defmodule CoursePlanner.NotificationsTest do
+  use CoursePlanner.ModelCase
+  doctest CoursePlanner.Notifications
+
+  import CoursePlanner.Factory
+
+  alias CoursePlanner.Notifications
+
+  test "send notification when it's day after" do
+    now = Timex.now()
+
+    user1 = insert(:user, %{notified_at: Timex.shift(now, days: -1), notification_period_days: 1})
+    insert(:notification, %{user_id: user1.id})
+
+    user2 = insert(:user, %{notified_at: Timex.shift(now, days: -1), notification_period_days: 2})
+    insert(:notification, %{user_id: user2.id})
+
+    user3 = insert(:user, %{notified_at: nil})
+    insert(:notification, %{user_id: user3.id})
+
+    [user_to_notiy1, user_to_notiy2] = Notifications.get_notifiable_users(now) |> Enum.sort(&(&1.id < &2.id))
+    assert user_to_notiy1.id == user1.id
+    assert user_to_notiy2.id == user3.id
+  end
+
+
+end

test/lib/course_planner/notifier/server_test.exs

@@ -0,0 +1,62 @@
+defmodule CoursePlanner.Notifier.ServerTest do
+  use CoursePlanner.ModelCase
+  doctest CoursePlanner.Notifier.Server
+
+  import ExUnit.CaptureLog
+  import CoursePlanner.Factory
+  import Swoosh.TestAssertions
+
+  alias CoursePlanner.{Notifier.Server, Notifications, User}
+
+  test "save notification to send later" do
+    user = insert(:user)
+    notification = Notifications.new()
+    |> Notifications.type(:user_modified)
+    |> Notifications.resource_path("/")
+    |> Notifications.to(user)
+
+    Server.handle_cast({:save_email, notification}, [])
+    saved_user = Repo.get(User, user.id) |> Repo.preload(:notifications)
+    [saved_notification] = saved_user.notifications
+    assert saved_notification.type == "user_modified"
+  end
+
+  test "send saved notifications" do
+    user = insert(:user)
+    insert(:notification, %{user_id: user.id})
+    insert(:notification, %{user_id: user.id})
+    insert(:notification, %{user_id: user.id})
+
+    saved_user = Repo.get(User, user.id) |> Repo.preload(:notifications)
+
+    Server.handle_cast({:notify_all, saved_user}, [])
+    assert_email_sent subject: "Activity Summary"
+    sent_user = Repo.get(User, user.id) |> Repo.preload(:notifications)
+    assert sent_user.notifications == []
+    assert Timex.to_date(sent_user.notified_at) == Timex.today()
+  end
+
+  test "send email" do
+    user = insert(:user)
+    notification = Notifications.new()
+    |> Notifications.type(:user_modified)
+    |> Notifications.resource_path("/")
+    |> Notifications.to(user)
+
+    Server.handle_cast({:send_email, notification}, [])
+    assert_email_sent subject: "Your profile is updated"
+  end
+
+  test "do not save notification without type" do
+    user = insert(:user)
+    notification = Notifications.new()
+    |> Notifications.resource_path("/")
+    |> Notifications.to(user)
+
+    assert capture_log(fn -> Server.handle_cast({:save_email, notification}, []) end) =~ "Email saving failed"
+
+    saved_user = Repo.get(User, user.id) |> Repo.preload(:notifications)
+    assert [] == saved_user.notifications
+  end
+
+end