From d5d7e9b32e6ccbdc2c0e85321e3c7d05803da711 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Mon, 5 Oct 2020 16:42:31 +0200 Subject: [PATCH 1/2] Specify that only users with account can comment Signed-off-by: Thomas Citharel --- js/src/i18n/en_US.json | 4 ++-- js/src/i18n/fr_FR.json | 3 ++- js/src/views/Event/Edit.vue | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/js/src/i18n/en_US.json b/js/src/i18n/en_US.json index 6aea966ef..9feaa3a7a 100644 --- a/js/src/i18n/en_US.json +++ b/js/src/i18n/en_US.json @@ -20,7 +20,6 @@ "Admin": "Admin", "Administration": "Administration", "All the places have already been taken": "All the places have been taken|One place is still available|{places} places are still available", - "Allow all comments": "Allow all comments", "Allow registrations": "Allow registrations", "Anonymous participant": "Anonymous participant", "Anonymous participants will be asked to confirm their participation through e-mail.": "Anonymous participants will be asked to confirm their participation through e-mail.", @@ -785,5 +784,6 @@ "Accessible only to members": "Accessible only to members", "Created by {name}": "Created by {name}", "View all posts": "View all posts", - "Didn't receive the instructions?": "Didn't receive the instructions?" + "Didn't receive the instructions?": "Didn't receive the instructions?", + "Allow all comments from users with accounts": "Allow all comments from logged-in users" } diff --git a/js/src/i18n/fr_FR.json b/js/src/i18n/fr_FR.json index aec91ba3c..484d9352f 100644 --- a/js/src/i18n/fr_FR.json +++ b/js/src/i18n/fr_FR.json @@ -823,5 +823,6 @@ "{number} posts": "Aucun billet|Un billet|{number} billets", "{profile} (by default)": "{profile} (par défault)", "{title} ({count} todos)": "{title} ({count} todos)", - "© The OpenStreetMap Contributors": "© Les Contributeur⋅ices OpenStreetMap" + "© The OpenStreetMap Contributors": "© Les Contributeur⋅ices OpenStreetMap", + "Allow all comments from users with accounts": "Autoriser tous les commentaires d'utilisateur·ices avec des comptes" } diff --git a/js/src/views/Event/Edit.vue b/js/src/views/Event/Edit.vue index 950a435c2..a22f8dc80 100644 --- a/js/src/views/Event/Edit.vue +++ b/js/src/views/Event/Edit.vue @@ -169,7 +169,7 @@ v-model="event.options.commentModeration" name="commentModeration" :native-value="CommentModeration.ALLOW_ALL" - >{{ $t("Allow all comments") }}{{ $t("Allow all comments from users with accounts") }} From af867cde1799f9cf30c4a90375ed82f8bfe69b19 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Mon, 5 Oct 2020 17:42:53 +0200 Subject: [PATCH 2/2] Make sure only organizer actor can comment if event comments are disabled Signed-off-by: Thomas Citharel --- js/src/components/Comment/CommentTree.vue | 35 +++-- lib/graphql/resolvers/comment.ex | 18 ++- lib/graphql/schema/discussions/comment.ex | 2 +- test/graphql/resolvers/comment_test.exs | 158 ++++++++++++---------- 4 files changed, 130 insertions(+), 83 deletions(-) diff --git a/js/src/components/Comment/CommentTree.vue b/js/src/components/Comment/CommentTree.vue index 32f726682..de4aa8a6b 100644 --- a/js/src/components/Comment/CommentTree.vue +++ b/js/src/components/Comment/CommentTree.vue @@ -2,7 +2,7 @@
@@ -22,11 +22,7 @@
- {{ $t("Comments have been closed.") }} + {{ $t("Comments have been closed.") }} 0) { + this.$notifier.error(error.graphQLErrors[0].message); + } } } @@ -260,8 +258,11 @@ export default class CommentTree extends Vue { }, }); // this.comments = this.comments.filter(commentItem => commentItem.id !== comment.id); - } catch (e) { - Snackbar.open({ message: e.message, type: "is-danger", position: "is-bottom" }); + } catch (error) { + console.error(error); + if (error.graphQLErrors && error.graphQLErrors.length > 0) { + this.$notifier.error(error.graphQLErrors[0].message); + } } } @@ -279,6 +280,18 @@ export default class CommentTree extends Vue { get filteredOrderedComments(): IComment[] { return this.orderedComments.filter((comment) => !comment.deletedAt || comment.totalReplies > 0); } + + get isAbleToComment(): boolean { + if (this.currentActor.id) { + if ( + this.event.options.commentModeration !== CommentModeration.CLOSED || + (this.event.organizerActor && this.currentActor.id === this.event.organizerActor.id) + ) { + return true; + } + } + return false; + } } diff --git a/lib/graphql/resolvers/comment.ex b/lib/graphql/resolvers/comment.ex index 49c491543..d374f054a 100644 --- a/lib/graphql/resolvers/comment.ex +++ b/lib/graphql/resolvers/comment.ex @@ -3,9 +3,10 @@ defmodule Mobilizon.GraphQL.Resolvers.Comment do Handles the comment-related GraphQL calls. """ - alias Mobilizon.{Actors, Admin, Discussions} + alias Mobilizon.{Actors, Admin, Discussions, Events} alias Mobilizon.Actors.Actor alias Mobilizon.Discussions.Comment, as: CommentModel + alias Mobilizon.Events.{Event, EventOptions} alias Mobilizon.Users alias Mobilizon.Users.User import Mobilizon.Web.Gettext @@ -20,7 +21,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Comment do def create_comment( _parent, - %{actor_id: actor_id} = args, + %{actor_id: actor_id, event_id: event_id} = args, %{ context: %{ current_user: %User{} = user @@ -28,10 +29,23 @@ defmodule Mobilizon.GraphQL.Resolvers.Comment do } ) do with {:is_owned, %Actor{} = _organizer_actor} <- User.owns_actor(user, actor_id), + {:find_event, + {:ok, + %Event{ + options: %EventOptions{comment_moderation: comment_moderation}, + organizer_actor_id: organizer_actor_id + }}} <- + {:find_event, Events.get_event(event_id)}, + {actor_id, ""} <- Integer.parse(actor_id), + {:allowed, true} <- + {:allowed, comment_moderation != :closed || actor_id == organizer_actor_id}, {:ok, _, %CommentModel{} = comment} <- Comments.create_comment(args) do {:ok, comment} else + {:allowed, false} -> + {:error, :unauthorized} + {:is_owned, nil} -> {:error, dgettext("errors", "Profile is not owned by authenticated user")} end diff --git a/lib/graphql/schema/discussions/comment.ex b/lib/graphql/schema/discussions/comment.ex index 6103076b0..c5b77969e 100644 --- a/lib/graphql/schema/discussions/comment.ex +++ b/lib/graphql/schema/discussions/comment.ex @@ -66,7 +66,7 @@ defmodule Mobilizon.GraphQL.Schema.Discussions.CommentType do @desc "Create a comment" field :create_comment, type: :comment do arg(:text, non_null(:string)) - arg(:event_id, :id) + arg(:event_id, non_null(:id)) arg(:in_reply_to_comment_id, :id) arg(:actor_id, non_null(:id)) diff --git a/test/graphql/resolvers/comment_test.exs b/test/graphql/resolvers/comment_test.exs index 95af0de91..f07282db1 100644 --- a/test/graphql/resolvers/comment_test.exs +++ b/test/graphql/resolvers/comment_test.exs @@ -3,9 +3,11 @@ defmodule Mobilizon.GraphQL.Resolvers.CommentTest do import Mobilizon.Factory + alias Mobilizon.Events + alias Mobilizon.Events.{Event, EventOptions} alias Mobilizon.GraphQL.AbsintheHelpers - @comment %{text: "I love this event"} + @comment_text "I love this event" setup %{conn: conn} do user = insert(:user) @@ -16,75 +18,104 @@ defmodule Mobilizon.GraphQL.Resolvers.CommentTest do end describe "Comment Resolver" do + @create_comment_mutation """ + mutation CreateComment($text: String!, $actorId: ID, $eventId: ID, $inReplyToCommentId: ID) { + createComment(text: $text, actorId: $actorId, eventId: $eventId, inReplyToCommentId: $inReplyToCommentId) { + id, + text, + uuid, + inReplyToComment { + id, + text + } + } + } + """ + test "create_comment/3 creates a comment", %{ conn: conn, actor: actor, user: user, event: event } do - mutation = """ - mutation { - createComment( - text: "#{@comment.text}", - actor_id: "#{actor.id}", - event_id: "#{event.id}" - ) { - text, - uuid - } - } - """ - res = conn |> auth_conn(user) - |> AbsintheHelpers.graphql_query(query: mutation, variables: %{}) + |> AbsintheHelpers.graphql_query( + query: @create_comment_mutation, + variables: %{text: @comment_text, actorId: actor.id, eventId: event.id} + ) - assert res["data"]["createComment"]["text"] == @comment.text + assert res["data"]["createComment"]["text"] == @comment_text end - test "create_comment/3 checks that user owns actor", %{conn: conn, user: user} do + test "create_comment/3 checks that user owns actor", %{conn: conn, user: user, event: event} do actor = insert(:actor) - mutation = """ - mutation { - createComment( - text: "#{@comment.text}", - actor_id: "#{actor.id}" - ) { - text, - uuid - } - } - """ - res = conn |> auth_conn(user) - |> AbsintheHelpers.graphql_query(query: mutation, variables: %{}) + |> AbsintheHelpers.graphql_query( + query: @create_comment_mutation, + variables: %{text: @comment_text, actorId: actor.id, eventId: event.id} + ) assert hd(res["errors"])["message"] == "Profile is not owned by authenticated user" end - test "create_comment/3 requires that the user needs to be authenticated", %{conn: conn} do - actor = insert(:actor) - - mutation = """ - mutation { - createComment( - text: "#{@comment.text}", - actor_id: "#{actor.id}" - ) { - text, - uuid - } - } - """ + test "create_comment/3 doesn't allow creating events if it's disabled", %{ + conn: conn, + actor: actor, + user: user, + event: event + } do + {:ok, %Event{options: %EventOptions{comment_moderation: :closed}}} = + Events.update_event(event, %{options: %{comment_moderation: :closed}}) res = conn - |> AbsintheHelpers.graphql_query(query: mutation, variables: %{}) + |> auth_conn(user) + |> AbsintheHelpers.graphql_query( + query: @create_comment_mutation, + variables: %{text: @comment_text, actorId: actor.id, eventId: event.id} + ) + + assert hd(res["errors"])["message"] == + "You don't have permission to do this" + end + + test "create_comment/3 allows creating events if it's disabled but we're the organizer", %{ + conn: conn, + actor: actor, + user: user + } do + event = insert(:event, organizer_actor: actor, options: %{comment_moderation: :closed}) + + res = + conn + |> auth_conn(user) + |> AbsintheHelpers.graphql_query( + query: @create_comment_mutation, + variables: %{text: @comment_text, actorId: actor.id, eventId: event.id} + ) + + assert is_nil(res["errors"]) + assert res["data"]["createComment"]["text"] == @comment_text + end + + test "create_comment/3 requires that the user needs to be authenticated", %{ + conn: conn, + event: event + } do + actor = insert(:actor) + + res = + conn + |> AbsintheHelpers.graphql_query( + query: @create_comment_mutation, + variables: %{text: @comment_text, actorId: actor.id, eventId: event.id} + ) assert hd(res["errors"])["message"] == "You are not allowed to create a comment if not connected" @@ -98,35 +129,24 @@ defmodule Mobilizon.GraphQL.Resolvers.CommentTest do } do comment = insert(:comment) - mutation = """ - mutation { - createComment( - text: "#{@comment.text}", - actor_id: "#{actor.id}", - event_id: "#{event.id}", - in_reply_to_comment_id: "#{comment.id}" - ) { - id, - text, - uuid, - in_reply_to_comment { - id, - text - } - } - } - """ - res = conn |> auth_conn(user) - |> AbsintheHelpers.graphql_query(query: mutation, variables: %{}) + |> AbsintheHelpers.graphql_query( + query: @create_comment_mutation, + variables: %{ + text: @comment_text, + actorId: actor.id, + eventId: event.id, + inReplyToCommentId: comment.id + } + ) - assert res["errors"] == nil - assert res["data"]["createComment"]["text"] == @comment.text + assert is_nil(res["errors"]) + assert res["data"]["createComment"]["text"] == @comment_text uuid = res["data"]["createComment"]["uuid"] - assert res["data"]["createComment"]["in_reply_to_comment"]["id"] == + assert res["data"]["createComment"]["inReplyToComment"]["id"] == to_string(comment.id) query = """ @@ -144,7 +164,7 @@ defmodule Mobilizon.GraphQL.Resolvers.CommentTest do |> AbsintheHelpers.graphql_query(query: query, variables: %{}) assert res["errors"] == nil - assert res["data"]["thread"] == [%{"uuid" => uuid, "text" => @comment.text}] + assert res["data"]["thread"] == [%{"uuid" => uuid, "text" => @comment_text}] end @delete_comment """