Validate ends_on being after begins_on

Closes #315

Signed-off-by: Thomas Citharel <tcit@tcit.fr>

lib/mobilizon/events/event.ex

@@ -128,7 +128,6 @@ defmodule Mobilizon.Events.Event do
     |> common_changeset(attrs)
     |> put_creator_if_published(:create)
     |> validate_required(@required_attrs)
-    |> validate_lengths()
   end
 
   @doc false
@@ -139,7 +138,6 @@ defmodule Mobilizon.Events.Event do
     |> common_changeset(attrs)
     |> put_creator_if_published(:update)
     |> validate_required(@update_required_attrs)
-    |> validate_lengths()
   end
 
   @spec common_changeset(Changeset.t(), map) :: Changeset.t()
@@ -149,6 +147,8 @@ defmodule Mobilizon.Events.Event do
     |> put_tags(attrs)
     |> put_address(attrs)
     |> put_picture(attrs)
+    |> validate_lengths()
+    |> validate_end_time()
   end
 
   @spec validate_lengths(Changeset.t()) :: Changeset.t()
@@ -162,6 +162,20 @@ defmodule Mobilizon.Events.Event do
     |> validate_length(:slug, min: 3, max: 200)
   end
 
+  defp validate_end_time(%Changeset{} = changeset) do
+    case fetch_field(changeset, :begins_on) do
+      {_, begins_on} ->
+        validate_change(changeset, :ends_on, fn :ends_on, ends_on ->
+          if begins_on > ends_on,
+            do: [ends_on: "ends_on cannot be set before begins_on"],
+            else: []
+        end)
+
+      :error ->
+        changeset
+    end
+  end
+
   @doc """
   Checks whether an event can be managed.
   """

lib/mobilizon_web/resolvers/event.ex

@@ -275,6 +275,9 @@ defmodule MobilizonWeb.Resolvers.Event do
       {:is_owned, nil} ->
         {:error, "Organizer actor id is not owned by the user"}
 
+      {:error, _, %Ecto.Changeset{} = error, _} ->
+        {:error, error}
+
       {:error, %Ecto.Changeset{} = error} ->
         {:error, error}
     end
@@ -292,15 +295,12 @@ defmodule MobilizonWeb.Resolvers.Event do
         %{event_id: event_id} = args,
         %{context: %{current_user: user}} = _resolution
       ) do
-    require Logger
-    Logger.error(inspect(args))
     # See https://github.com/absinthe-graphql/absinthe/issues/490
     with args <- Map.put(args, :options, args[:options] || %{}),
          {:ok, %Event{} = event} <- Events.get_event_with_preload(event_id),
          organizer_actor_id <- args |> Map.get(:organizer_actor_id, event.organizer_actor_id),
          {:is_owned, %Actor{} = organizer_actor} <-
            User.owns_actor(user, organizer_actor_id),
-         :ok <- Logger.error(inspect(organizer_actor)),
          args <- Map.put(args, :organizer_actor, organizer_actor),
          {:ok, %Activity{data: %{"object" => %{"type" => "Event"}}}, %Event{} = event} <-
            MobilizonWeb.API.Events.update_event(args, event) do
@@ -311,6 +311,9 @@ defmodule MobilizonWeb.Resolvers.Event do
 
       {:is_owned, nil} ->
         {:error, "User doesn't own actor"}
+
+      {:error, _, %Ecto.Changeset{} = error, _} ->
+        {:error, error}
     end
   end
 

lib/service/activity_pub/activity_pub.ex

@@ -93,8 +93,6 @@ defmodule Mobilizon.Service.ActivityPub do
         {:ok, Actors.get_actor_by_url!(actor_url, true)}
 
       e ->
-        require Logger
-        Logger.error(inspect(e))
         {:error, e}
     end
   end
@@ -135,14 +133,13 @@ defmodule Mobilizon.Service.ActivityPub do
     Logger.debug("creating an activity")
     Logger.debug(inspect(args))
 
-    {:ok, entity, create_data} =
+    with {:ok, entity, create_data} <-
-      case type do
+           (case type do
-        :event -> create_event(args, additional)
+              :event -> create_event(args, additional)
-        :comment -> create_comment(args, additional)
+              :comment -> create_comment(args, additional)
-        :group -> create_group(args, additional)
+              :group -> create_group(args, additional)
-      end
+            end),
-
+         {:ok, activity} <- create_activity(create_data, local),
-    with {:ok, activity} <- create_activity(create_data, local),
          :ok <- maybe_federate(activity) do
       {:ok, activity, entity}
     else
@@ -167,13 +164,12 @@ defmodule Mobilizon.Service.ActivityPub do
     Logger.debug("updating an activity")
     Logger.debug(inspect(args))
 
-    {:ok, entity, update_data} =
+    with {:ok, entity, update_data} <-
-      case type do
+           (case type do
-        :event -> update_event(old_entity, args, additional)
+              :event -> update_event(old_entity, args, additional)
-        :actor -> update_actor(old_entity, args, additional)
+              :actor -> update_actor(old_entity, args, additional)
-      end
+            end),
-
+         {:ok, activity} <- create_activity(update_data, local),
-    with {:ok, activity} <- create_activity(update_data, local),
          :ok <- maybe_federate(activity) do
       {:ok, activity, entity}
     else

lib/service/activity_pub/transmogrifier.ex

@@ -339,8 +339,7 @@ defmodule Mobilizon.Service.ActivityPub.Transmogrifier do
            ActivityPub.update(:event, old_event, object_data, false) do
       {:ok, activity, new_event}
     else
-      e ->
+      _e ->
-        Logger.error(inspect(e))
         :error
     end
   end
@@ -442,8 +441,7 @@ defmodule Mobilizon.Service.ActivityPub.Transmogrifier do
 
         :error
 
-      e ->
+      _e ->
-        Logger.error(inspect(e))
         :error
     end
   end

test/mobilizon_web/resolvers/event_resolver_test.exs

@@ -95,6 +95,40 @@ defmodule MobilizonWeb.Resolvers.EventResolverTest do
                "Organizer actor id is not owned by the user"
     end
 
+    test "create_event/3 should check that end time is after start time", %{
+      conn: conn,
+      actor: actor,
+      user: user
+    } do
+      begins_on = DateTime.utc_now() |> DateTime.truncate(:second)
+      ends_on = Timex.shift(begins_on, hours: -2)
+
+      mutation = """
+          mutation {
+              createEvent(
+                  title: "come to my event",
+                  description: "it will be fine",
+                  begins_on: "#{DateTime.to_iso8601(begins_on)}",
+                  ends_on: "#{DateTime.to_iso8601(ends_on)}",
+                  organizer_actor_id: "#{actor.id}",
+                  category: "birthday"
+              ) {
+                id,
+                title,
+                uuid
+              }
+            }
+      """
+
+      res =
+        conn
+        |> auth_conn(user)
+        |> post("/api", AbsintheHelpers.mutation_skeleton(mutation))
+
+      assert hd(json_response(res, 200)["errors"])["message"] ==
+               "ends_on cannot be set before begins_on"
+    end
+
     test "create_event/3 creates an event", %{conn: conn, actor: actor, user: user} do
       mutation = """
           mutation {
@@ -661,6 +695,39 @@ defmodule MobilizonWeb.Resolvers.EventResolverTest do
       assert hd(json_response(res, 200)["errors"])["message"] == "User doesn't own actor"
     end
 
+    test "update_event/3 should check end time is after the beginning time", %{
+      conn: conn,
+      actor: actor,
+      user: user
+    } do
+      event = insert(:event, organizer_actor: actor)
+
+      mutation = """
+          mutation {
+              updateEvent(
+                  title: "my event updated",
+                  ends_on: "#{Timex.shift(event.begins_on, hours: -2)}",
+                  event_id: #{event.id}
+              ) {
+                title,
+                uuid,
+                tags {
+                  title,
+                  slug
+                }
+              }
+            }
+      """
+
+      res =
+        conn
+        |> auth_conn(user)
+        |> post("/api", AbsintheHelpers.mutation_skeleton(mutation))
+
+      assert hd(json_response(res, 200)["errors"])["message"] ==
+               "ends_on cannot be set before begins_on"
+    end
+
     test "update_event/3 updates an event", %{conn: conn, actor: actor, user: user} do
       event = insert(:event, organizer_actor: actor)
       _creator = insert(:participant, event: event, actor: actor, role: :creator)