From e2560a14d1f1d6ced0320110117cf68c13267a8e Mon Sep 17 00:00:00 2001 From: AyakorK Date: Fri, 13 Dec 2024 11:27:32 +0100 Subject: [PATCH] fix: Fix the editing of comments to put the same chars limit than it has on its creation --- config/application.rb | 2 + .../decidim/comments/comments_controller.rb | 61 +++++ .../decidim/comments/comment_form_extends.rb | 15 ++ .../comments/comments_controller_spec.rb | 237 ++++++++++++++++++ spec/forms/comment_form_spec.rb | 115 +++++++++ 5 files changed, 430 insertions(+) create mode 100644 lib/extends/controllers/decidim/comments/comments_controller.rb create mode 100644 lib/extends/forms/decidim/comments/comment_form_extends.rb create mode 100644 spec/controllers/decidim/comments/comments_controller_spec.rb create mode 100644 spec/forms/comment_form_spec.rb diff --git a/config/application.rb b/config/application.rb index 26a54c15b8..759eb48b0d 100644 --- a/config/application.rb +++ b/config/application.rb @@ -54,6 +54,7 @@ class Application < Rails::Application require "extends/controllers/decidim/admin/scopes_controller_extends" require "extends/controllers/decidim/scopes_controller_extends" require "extends/controllers/decidim/initiatives/committee_requests_controller_extends" + require "extends/controllers/decidim/comments/comments_controller" # Models require "extends/models/decidim/budgets/project_extends" require "extends/models/decidim/authorization_extends" @@ -67,6 +68,7 @@ class Application < Rails::Application # Forms require "extends/forms/decidim/initiatives/initiative_form_extends" require "extends/forms/decidim/initiatives/admin/initiative_form_extends" + require "extends/forms/decidim/comments/comment_form_extends" # Commands require "extends/commands/decidim/initiatives/admin/update_initiative_answer_extends" require "extends/commands/decidim/budgets/admin/import_proposals_to_budgets_extends" diff --git a/lib/extends/controllers/decidim/comments/comments_controller.rb b/lib/extends/controllers/decidim/comments/comments_controller.rb new file mode 100644 index 0000000000..5758d5eaf4 --- /dev/null +++ b/lib/extends/controllers/decidim/comments/comments_controller.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +require "active_support/concern" +module CommentsControllerExtends + extend ActiveSupport::Concern + included do + def update + set_comment + @commentable = comment.commentable + enforce_permission_to :update, :comment, comment: comment + + form = Decidim::Comments::CommentForm.from_params( + params.merge(commentable: comment.commentable, current_component: current_component) + ).with_context( + current_organization: current_organization + ) + + Decidim::Comments::UpdateComment.call(comment, current_user, form) do + on(:ok) do + respond_to do |format| + format.js { render :update } + end + end + + on(:invalid) do + respond_to do |format| + format.js { render :update_error } + end + end + end + end + + def create + enforce_permission_to :create, :comment, commentable: commentable + + form = Decidim::Comments::CommentForm.from_params( + params.merge(commentable: commentable, current_component: current_component) + ).with_context( + current_organization: current_organization, + current_component: current_component + ) + Decidim::Comments::CreateComment.call(form, current_user) do + on(:ok) do |comment| + handle_success(comment) + respond_to do |format| + format.js { render :create } + end + end + + on(:invalid) do + @error = t("create.error", scope: "decidim.comments.comments") + respond_to do |format| + format.js { render :error } + end + end + end + end + end +end + +Decidim::Comments::CommentsController.include(CommentsControllerExtends) diff --git a/lib/extends/forms/decidim/comments/comment_form_extends.rb b/lib/extends/forms/decidim/comments/comment_form_extends.rb new file mode 100644 index 0000000000..e8621043fe --- /dev/null +++ b/lib/extends/forms/decidim/comments/comment_form_extends.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require "active_support/concern" + +module CommentFormExtends + extend ActiveSupport::Concern + + included do + attribute :current_component, Decidim::Component + + validates :current_component, presence: true + end +end + +Decidim::Comments::CommentForm.include(CommentFormExtends) diff --git a/spec/controllers/decidim/comments/comments_controller_spec.rb b/spec/controllers/decidim/comments/comments_controller_spec.rb new file mode 100644 index 0000000000..0920d705bd --- /dev/null +++ b/spec/controllers/decidim/comments/comments_controller_spec.rb @@ -0,0 +1,237 @@ +# frozen_string_literal: true + +require "spec_helper" + +module Decidim + module Comments + describe CommentsController, type: :controller do + routes { Decidim::Comments::Engine.routes } + + let(:organization) { create(:organization) } + let(:participatory_process) { create :participatory_process, organization: organization } + let(:component) { create(:component, participatory_space: participatory_process) } + let(:commentable) { create(:dummy_resource, component: component) } + + before do + request.env["decidim.current_organization"] = organization + end + + describe "GET index" do + it "renders the index template" do + get :index, xhr: true, params: { commentable_gid: commentable.to_signed_global_id.to_s } + expect(subject).to render_template(:index) + end + + it "tells devise not to reset timeout counter" do + expect(request.env["devise.skip_timeoutable"]).to be_nil + get :index, xhr: true, params: { commentable_gid: commentable.to_signed_global_id.to_s } + expect(request.env["devise.skip_timeoutable"]).to be(true) + end + + context "when requested without an XHR request" do + it "redirects to the commentable" do + get :index, params: { commentable_gid: commentable.to_signed_global_id.to_s } + expect(subject).to redirect_to( + Decidim::ResourceLocatorPresenter.new(commentable).path + ) + end + end + + context "when the reload parameter is given" do + it "renders the reload template" do + get :index, xhr: true, params: { commentable_gid: commentable.to_signed_global_id.to_s, reload: 1 } + expect(subject).to render_template(:reload) + end + end + + context "when comments are disabled for the component" do + let(:component) { create(:component, :with_comments_disabled, participatory_space: participatory_process) } + + it "redirects with a flash alert" do + get :index, xhr: true, params: { commentable_gid: commentable.to_signed_global_id.to_s } + expect(flash[:alert]).to be_present + expect(response).to have_http_status(:redirect) + end + end + end + + describe "POST create" do + let(:comment_alignment) { 0 } + let(:comment_params) do + { + commentable_gid: commentable.to_signed_global_id.to_s, + body: "This is a new comment", + alignment: comment_alignment + } + end + + it "responds with unauthorized status" do + post :create, xhr: true, params: { comment: comment_params } + expect(response).to have_http_status(:unauthorized) + end + + context "when the user is signed in" do + let(:user) { create(:user, :confirmed, locale: "en", organization: organization) } + let(:comment) { Decidim::Comments::Comment.last } + + before do + sign_in user, scope: :user + end + + it "creates the comment" do + expect do + post :create, xhr: true, params: { comment: comment_params } + end.to change(Decidim::Comments::Comment, :count).by(1) + + expect(comment.body.values.first).to eq("This is a new comment") + expect(comment.alignment).to eq(comment_alignment) + expect(subject).to render_template(:create) + end + + context "when requested without an XHR request" do + it "throws an unknown format exception" do + expect do + post :create, params: { comment: comment_params } + end.to raise_error(ActionController::UnknownFormat) + end + end + + context "when comments are disabled for the component" do + let(:component) { create(:component, :with_comments_disabled, participatory_space: participatory_process) } + + it "redirects with a flash alert" do + post :create, xhr: true, params: { comment: comment_params } + expect(flash[:alert]).to be_present + expect(response).to have_http_status(:redirect) + end + end + + context "when trying to comment on a private space where the user is not assigned to" do + let(:participatory_process) { create :participatory_process, :private, organization: organization } + + it "redirects with a flash alert" do + post :create, xhr: true, params: { comment: comment_params } + expect(flash[:alert]).to be_present + expect(response).to have_http_status(:redirect) + end + end + + context "when comment alignment is positive" do + let(:comment_alignment) { 1 } + + it "creates the comment with the alignment defined as 1" do + expect do + post :create, xhr: true, params: { comment: comment_params } + end.to change(Decidim::Comments::Comment, :count).by(1) + + expect(comment.alignment).to eq(comment_alignment) + expect(subject).to render_template(:create) + end + end + + context "when comment alignment is negative" do + let(:comment_alignment) { -1 } + + it "creates the comment with the alignment defined as -1" do + expect do + post :create, xhr: true, params: { comment: comment_params } + end.to change(Decidim::Comments::Comment, :count).by(1) + + expect(comment.alignment).to eq(comment_alignment) + expect(subject).to render_template(:create) + end + end + + context "when comment body is missing" do + let(:comment_params) do + { + commentable_gid: commentable.to_signed_global_id.to_s, + alignment: comment_alignment + } + end + + it "renders the error template" do + post :create, xhr: true, params: { comment: comment_params } + expect(subject).to render_template(:error) + end + + context "when requested without an XHR request" do + it "throws an unknown format exception" do + expect do + post :create, params: { comment: comment_params } + end.to raise_error(ActionController::UnknownFormat) + end + end + end + + context "when comment alignment is invalid" do + let(:comment_alignment) { 2 } + + it "renders the error template" do + post :create, xhr: true, params: { comment: comment_params } + expect(subject).to render_template(:error) + end + end + + context "when the comment does not exist" do + let(:comment_params) do + { + commentable_gid: "unexisting", + body: "This is a new comment", + alignment: 0 + } + end + + it "raises a routing error" do + expect do + post :create, xhr: true, params: { comment: comment_params } + end.to raise_error(ActionController::RoutingError) + end + end + end + end + + describe "DELETE destroy" do + let(:user) { create(:user, :confirmed, locale: "en", organization: organization) } + let(:comment_author) { create(:user, :confirmed, locale: "en", organization: organization) } + let!(:comment) { create(:comment, commentable: commentable, author: comment_author) } + + it "redirects to sign in path" do + expect do + delete :destroy, xhr: true, params: { id: comment.id } + end.not_to(change { Decidim::Comments::Comment.not_deleted.count }) + + expect(response).to redirect_to("/users/sign_in") + end + + context "when a user different of the author is signed in" do + before do + sign_in user, scope: :user + end + + it "doesn't delete the comment" do + expect do + delete :destroy, xhr: true, params: { id: comment.id } + end.not_to(change { Decidim::Comments::Comment.not_deleted.count }) + + expect(response).not_to have_http_status(:success) + end + end + + context "when the author is signed in" do + before do + sign_in comment_author, scope: :user + end + + it "deletes the comment" do + expect do + delete :destroy, xhr: true, params: { id: comment.id } + end.to change { Decidim::Comments::Comment.not_deleted.count }.by(-1) + + expect(response).to have_http_status(:success) + end + end + end + end + end +end diff --git a/spec/forms/comment_form_spec.rb b/spec/forms/comment_form_spec.rb new file mode 100644 index 0000000000..f68a5c85c4 --- /dev/null +++ b/spec/forms/comment_form_spec.rb @@ -0,0 +1,115 @@ +# frozen_string_literal: true + +require "spec_helper" + +module Decidim + module Comments + describe CommentForm do + subject do + described_class.from_params( + attributes + ).with_context( + current_organization: organization, + current_component: component + ) + end + + let(:organization) { create(:organization) } + let!(:component) { create(:component, organization: organization) } + let(:body) { "This is a new comment" } + let(:alignment) { 1 } + let(:user_group) { create(:user_group, :verified) } + let(:user_group_id) { user_group.id } + + let(:commentable) { create :dummy_resource } + + let(:attributes) do + { + "comment" => { + "body" => body, + "alignment" => alignment, + "user_group_id" => user_group_id, + "commentable" => commentable, + "current_component" => component + } + } + end + + context "when everything is OK" do + it { is_expected.to be_valid } + end + + context "when body is blank" do + let(:body) { "" } + + it { is_expected.not_to be_valid } + end + + context "when body is too long" do + let(:body) { "c" * 1001 } + + it { is_expected.not_to be_valid } + + context "with carriage return characters that cause it to exceed" do + let(:body) { "#{"c" * 500}\r\n#{"c" * 499}" } + + it { is_expected.to be_valid } + end + end + + context "when alignment is not present" do + let(:alignment) { nil } + + it { is_expected.to be_valid } + end + + context "when alignment is present and it is different from 0, 1 and -1" do + let(:alignment) { 2 } + + it { is_expected.not_to be_valid } + end + + context "when current_component is missing" do + let!(:component) { nil } + + it { is_expected.not_to be_valid } + end + + describe "#max_length" do + context "when organization has a max length > 0" do + let(:body) { "c" * 1001 } + let(:organization) { create(:organization, comments_max_length: 1001) } + + it { is_expected.to be_valid } + end + + context "when component has a max length > 0" do + let(:body) { "c" * 1001 } + + before do + component.update!(settings: { comments_max_length: 1001 }) + end + + it { is_expected.to be_valid } + end + + context "when component is missing" do + let!(:component) { nil } + let(:body) { "c" * 1000 } + + it { is_expected.not_to be_valid } + end + + context "when the component settings do not define comments_max_length" do + let(:organization) { create(:organization, comments_max_length: 3549) } + let(:settings) { double } + + it "returns the organization comments_max_length" do + allow(component).to receive(:settings).and_return(settings) + expect(subject.max_length).to eq(organization.comments_max_length) + end + end + end + end + end +end