From 711e6c16ebff0242af341b5e0a876b001310e094 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 5 Aug 2022 16:11:24 +0200 Subject: [PATCH 1/2] Load annotations through the draft version We were ignoring the draft version param when loading an annotation, which could result in a strange situation where we load an annotation and a draft version different than the one it belongs to. Thanks to this change, we can simplify the code a little bit. IMHO the `comments` and `new_comment` routes should have been added on member instead of on collection, which would further simplify the code. I'm leaving the routes untouched just in case changing the URL has side effects on existing installations. --- .../legislation/annotations_controller.rb | 10 ++++----- .../annotations_controller_spec.rb | 21 +++++++++++++++++++ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/app/controllers/legislation/annotations_controller.rb b/app/controllers/legislation/annotations_controller.rb index 0c38ac181..ba1b55f89 100644 --- a/app/controllers/legislation/annotations_controller.rb +++ b/app/controllers/legislation/annotations_controller.rb @@ -6,12 +6,11 @@ class Legislation::AnnotationsController < Legislation::BaseController load_and_authorize_resource :process load_and_authorize_resource :draft_version, through: :process - load_and_authorize_resource + load_and_authorize_resource through: :draft_version has_orders %w[most_voted newest oldest], only: :show def index - @annotations = @draft_version.annotations end def show @@ -61,13 +60,13 @@ class Legislation::AnnotationsController < Legislation::BaseController end def search - @annotations = @draft_version.annotations.order(Arel.sql("LENGTH(quote) DESC")) + @annotations = @annotations.order(Arel.sql("LENGTH(quote) DESC")) annotations_hash = { total: @annotations.size, rows: @annotations } render json: annotations_hash.to_json(methods: :weight) end def comments - @annotation = Legislation::Annotation.find(params[:annotation_id]) + @annotation = @annotations.find(params[:annotation_id]) @comment = @annotation.comments.new end @@ -78,8 +77,7 @@ class Legislation::AnnotationsController < Legislation::BaseController end def new_comment - @draft_version = Legislation::DraftVersion.find(params[:draft_version_id]) - @annotation = @draft_version.annotations.find(params[:annotation_id]) + @annotation = @annotations.find(params[:annotation_id]) @comment = @annotation.comments.new(body: params[:comment][:body], user: current_user) if @comment.save @comment = @annotation.comments.new diff --git a/spec/controllers/legislation/annotations_controller_spec.rb b/spec/controllers/legislation/annotations_controller_spec.rb index 1768b4a3d..3700c794c 100644 --- a/spec/controllers/legislation/annotations_controller_spec.rb +++ b/spec/controllers/legislation/annotations_controller_spec.rb @@ -1,6 +1,27 @@ require "rails_helper" describe Legislation::AnnotationsController do + describe "GET show" do + it "finds the annotation when it belongs to the draft version" do + version = create(:legislation_draft_version) + annotation = create(:legislation_annotation, draft_version: version) + + get :show, params: { process_id: version.process, draft_version_id: version, id: annotation } + + expect(response).to be_ok + end + + it "returns a 404 when the annotation belongs to a different draft version" do + version = create(:legislation_draft_version) + annotation = create(:legislation_annotation, draft_version: version) + other_version = create(:legislation_draft_version, process: version.process) + + expect do + get :show, params: { process_id: version.process, draft_version_id: other_version, id: annotation } + end.to raise_error ActiveRecord::RecordNotFound + end + end + describe "POST create" do let(:legal_process) do create(:legislation_process, allegations_start_date: Date.current - 3.days, From 5b97e85dd75cbb0e171ea1019ae97f7c81b7f6cf Mon Sep 17 00:00:00 2001 From: decabeza Date: Wed, 20 Jul 2022 17:23:41 +0200 Subject: [PATCH 2/2] Hide comments when allegations phase is closed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Julian Nicolas Herrero Co-Authored-By: Javi Martín --- app/models/legislation/annotation.rb | 4 ++++ spec/controllers/comments_controller_spec.rb | 22 +++++++++++++++++++ .../comments/legislation_annotations_spec.rb | 19 ++++++++++++++++ 3 files changed, 45 insertions(+) diff --git a/app/models/legislation/annotation.rb b/app/models/legislation/annotation.rb index ebe7e0c10..c9fb4d046 100644 --- a/app/models/legislation/annotation.rb +++ b/app/models/legislation/annotation.rb @@ -55,4 +55,8 @@ class Legislation::Annotation < ApplicationRecord def weight comments_count + comments.sum(:cached_votes_total) end + + def comments_closed? + !draft_version.process.allegations_phase.open? + end end diff --git a/spec/controllers/comments_controller_spec.rb b/spec/controllers/comments_controller_spec.rb index 6f9cfc9b2..33aaf7047 100644 --- a/spec/controllers/comments_controller_spec.rb +++ b/spec/controllers/comments_controller_spec.rb @@ -41,6 +41,28 @@ describe CommentsController do end.not_to change { question.reload.comments_count } end + it "does not create an annotation comment if the allegations phase is closed" do + process = create(:legislation_process, + allegations_start_date: 2.days.from_now, + allegations_end_date: 1.month.from_now) + + version = create(:legislation_draft_version, process: process) + annotation = create(:legislation_annotation, draft_version: version, text: "One annotation") + + sign_in user + + expect do + post :create, xhr: true, + params: { + comment: { + commentable_id: annotation.id, + commentable_type: "Legislation::Annotation", + body: "a comment" + } + } + end.not_to change { annotation.reload.comments_count } + end + it "does not create a comment for unverified users when the commentable requires it" do sign_in unverified_user diff --git a/spec/system/comments/legislation_annotations_spec.rb b/spec/system/comments/legislation_annotations_spec.rb index 07658eafb..a9966442b 100644 --- a/spec/system/comments/legislation_annotations_spec.rb +++ b/spec/system/comments/legislation_annotations_spec.rb @@ -254,6 +254,25 @@ describe "Commenting legislation questions" do expect(page).to have_content "Can't be blank" end + scenario "Comments are disabled when the allegations phase is closed" do + process = create(:legislation_process, + allegations_start_date: 1.month.ago, + allegations_end_date: Date.yesterday) + + version = create(:legislation_draft_version, process: process) + annotation = create(:legislation_annotation, draft_version: version, text: "One annotation") + + login_as(user) + + visit legislation_process_draft_version_annotation_path(version.process, version, annotation) + + within "#comments" do + expect(page).to have_content "Comments are closed" + expect(page).not_to have_content "Leave your comment" + expect(page).not_to have_button "Publish comment" + end + end + scenario "Reply" do citizen = create(:user, username: "Ana") manuela = create(:user, username: "Manuela")