Validate ends_on being after begins_on

Closes #315

Signed-off-by: Thomas Citharel <tcit@tcit.fr>
This commit is contained in:
Thomas Citharel 2019-11-18 18:40:03 +01:00
parent 4dc4524e71
commit 634a0b851e
No known key found for this signature in database
GPG Key ID: A061B9DDE0CA0773
5 changed files with 104 additions and 26 deletions

View File

@ -128,7 +128,6 @@ defmodule Mobilizon.Events.Event do
|> common_changeset(attrs) |> common_changeset(attrs)
|> put_creator_if_published(:create) |> put_creator_if_published(:create)
|> validate_required(@required_attrs) |> validate_required(@required_attrs)
|> validate_lengths()
end end
@doc false @doc false
@ -139,7 +138,6 @@ defmodule Mobilizon.Events.Event do
|> common_changeset(attrs) |> common_changeset(attrs)
|> put_creator_if_published(:update) |> put_creator_if_published(:update)
|> validate_required(@update_required_attrs) |> validate_required(@update_required_attrs)
|> validate_lengths()
end end
@spec common_changeset(Changeset.t(), map) :: Changeset.t() @spec common_changeset(Changeset.t(), map) :: Changeset.t()
@ -149,6 +147,8 @@ defmodule Mobilizon.Events.Event do
|> put_tags(attrs) |> put_tags(attrs)
|> put_address(attrs) |> put_address(attrs)
|> put_picture(attrs) |> put_picture(attrs)
|> validate_lengths()
|> validate_end_time()
end end
@spec validate_lengths(Changeset.t()) :: Changeset.t() @spec validate_lengths(Changeset.t()) :: Changeset.t()
@ -162,6 +162,20 @@ defmodule Mobilizon.Events.Event do
|> validate_length(:slug, min: 3, max: 200) |> validate_length(:slug, min: 3, max: 200)
end 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 """ @doc """
Checks whether an event can be managed. Checks whether an event can be managed.
""" """

View File

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

View File

@ -93,8 +93,6 @@ defmodule Mobilizon.Service.ActivityPub do
{:ok, Actors.get_actor_by_url!(actor_url, true)} {:ok, Actors.get_actor_by_url!(actor_url, true)}
e -> e ->
require Logger
Logger.error(inspect(e))
{:error, e} {:error, e}
end end
end end
@ -135,14 +133,13 @@ defmodule Mobilizon.Service.ActivityPub do
Logger.debug("creating an activity") Logger.debug("creating an activity")
Logger.debug(inspect(args)) 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 <- maybe_federate(activity) do
{:ok, activity, entity} {:ok, activity, entity}
else else
@ -167,13 +164,12 @@ defmodule Mobilizon.Service.ActivityPub do
Logger.debug("updating an activity") Logger.debug("updating an activity")
Logger.debug(inspect(args)) 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 <- maybe_federate(activity) do
{:ok, activity, entity} {:ok, activity, entity}
else else

View File

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

View File

@ -95,6 +95,40 @@ defmodule MobilizonWeb.Resolvers.EventResolverTest do
"Organizer actor id is not owned by the user" "Organizer actor id is not owned by the user"
end 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 test "create_event/3 creates an event", %{conn: conn, actor: actor, user: user} do
mutation = """ mutation = """
mutation { mutation {
@ -661,6 +695,39 @@ defmodule MobilizonWeb.Resolvers.EventResolverTest do
assert hd(json_response(res, 200)["errors"])["message"] == "User doesn't own actor" assert hd(json_response(res, 200)["errors"])["message"] == "User doesn't own actor"
end 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 test "update_event/3 updates an event", %{conn: conn, actor: actor, user: user} do
event = insert(:event, organizer_actor: actor) event = insert(:event, organizer_actor: actor)
_creator = insert(:participant, event: event, actor: actor, role: :creator) _creator = insert(:participant, event: event, actor: actor, role: :creator)