From 306e3fa59f94558a3262a592f434b147586d29a4 Mon Sep 17 00:00:00 2001 From: Amaia Castro Date: Wed, 11 Jan 2017 19:44:41 +0100 Subject: [PATCH 1/4] =?UTF-8?q?Don=E2=80=99t=20show=20comments=20panel=20i?= =?UTF-8?q?f=20seeing=20final=20version?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../legislation_annotatable.js.coffee | 1 + .../draft_versions/_comments_panel.html.erb | 18 +++++++++ .../legislation/draft_versions/show.html.erb | 39 ++++++++----------- .../legislation/draft_versions_spec.rb | 12 ++++++ 4 files changed, 48 insertions(+), 22 deletions(-) create mode 100644 app/views/legislation/draft_versions/_comments_panel.html.erb diff --git a/app/assets/javascripts/legislation_annotatable.js.coffee b/app/assets/javascripts/legislation_annotatable.js.coffee index ce9e9b8d7..f1df2927a 100644 --- a/app/assets/javascripts/legislation_annotatable.js.coffee +++ b/app/assets/javascripts/legislation_annotatable.js.coffee @@ -12,6 +12,7 @@ App.LegislationAnnotatable = viewerExtension: (viewer) -> viewer._onHighlightMouseover = (event) -> App.LegislationAllegations.show_comments() + $("#comments-box").show() $.event.trigger type: "renderLegislationAnnotation" annotation_id: $(event.target).data("annotation-id") diff --git a/app/views/legislation/draft_versions/_comments_panel.html.erb b/app/views/legislation/draft_versions/_comments_panel.html.erb new file mode 100644 index 000000000..94523b557 --- /dev/null +++ b/app/views/legislation/draft_versions/_comments_panel.html.erb @@ -0,0 +1,18 @@ +
+
+
+ <%= t('legislation.draft_versions.show.text_comments') %> +
+
+ +
+ <%= t('legislation.draft_versions.show.text_comments') %> +
+ + +
diff --git a/app/views/legislation/draft_versions/show.html.erb b/app/views/legislation/draft_versions/show.html.erb index 6bee874c4..d89926929 100644 --- a/app/views/legislation/draft_versions/show.html.erb +++ b/app/views/legislation/draft_versions/show.html.erb @@ -18,13 +18,17 @@ <%= t('.updated_at', date: format_date(@draft_version.updated_at)) %> -
- <%= link_to t('.see_comments'), legislation_process_draft_version_annotations_path(@process, @draft_version), title: t('.see_comments'), class: "button strong" %> -
+ + <% unless @draft_version.final_version? %> +
+ <%= link_to t('.see_comments'), legislation_process_draft_version_annotations_path(@process, @draft_version), title: t('.see_comments'), class: "button strong" %> +
+ <% end %> +
-
+
">
<%= t('.text_toc') %> @@ -44,34 +48,25 @@
<%= t('.text_body') %>
+ <% if @draft_version.final_version? %> +
+ <% else %>
+ <% end %> <%= @draft_version.body_html.html_safe %>
-
-
-
- <%= t('.text_comments') %> -
-
+ <% if @draft_version.final_version? %> +
+ <% else %> + <%= render 'comments_panel', draft_version: @draft_version %> + <% end %> -
- <%= t('.text_comments') %> -
- -
-
- -
<%= t('.loading_comments') %>
-
-
- -
diff --git a/spec/features/legislation/draft_versions_spec.rb b/spec/features/legislation/draft_versions_spec.rb index c7f761669..cd655ae4a 100644 --- a/spec/features/legislation/draft_versions_spec.rb +++ b/spec/features/legislation/draft_versions_spec.rb @@ -61,6 +61,18 @@ feature 'Legislation Draft Versions' do expect(page).to_not have_content("Body of the first version") expect(page).to have_content("Body of the second version") end + + context "for final versions" do + it "does not show the comments panel" do + final_version = create(:legislation_draft_version, process: @process, title: "Final version", body: "Final body", status: "published", final_version: true) + + visit legislation_process_draft_version_path(@process, final_version) + + expect(page).to have_content("Final body") + expect(page).to_not have_content("See all comments") + expect(page).to_not have_content("Comments") + end + end end context "See changes page" do From a56898fed7423e18b5347b8ee164718de1bdf447 Mon Sep 17 00:00:00 2001 From: Amaia Castro Date: Wed, 11 Jan 2017 20:14:20 +0100 Subject: [PATCH 2/4] Fix version chooser for annotations page --- .../legislation/annotations_controller.rb | 1 + .../legislation/draft_versions_controller.rb | 2 + .../legislation/draft_versions_spec.rb | 40 +++++++++++++++++++ 3 files changed, 43 insertions(+) diff --git a/app/controllers/legislation/annotations_controller.rb b/app/controllers/legislation/annotations_controller.rb index ee63574f5..ff4ecf813 100644 --- a/app/controllers/legislation/annotations_controller.rb +++ b/app/controllers/legislation/annotations_controller.rb @@ -10,6 +10,7 @@ class Legislation::AnnotationsController < ApplicationController has_orders %w{most_voted newest oldest}, only: :show def index + @annotations = @draft_version.annotations end def show diff --git a/app/controllers/legislation/draft_versions_controller.rb b/app/controllers/legislation/draft_versions_controller.rb index 0b6af46a6..cdac2c9d5 100644 --- a/app/controllers/legislation/draft_versions_controller.rb +++ b/app/controllers/legislation/draft_versions_controller.rb @@ -20,6 +20,8 @@ class Legislation::DraftVersionsController < Legislation::BaseController if params[:redirect_action] == 'changes' redirect_to legislation_process_draft_version_changes_path(@process, version) + elsif params[:redirect_action] == 'annotations' + redirect_to legislation_process_draft_version_annotations_path(@process, version) else redirect_to legislation_process_draft_version_path(@process, version) end diff --git a/spec/features/legislation/draft_versions_spec.rb b/spec/features/legislation/draft_versions_spec.rb index cd655ae4a..6c39d11ce 100644 --- a/spec/features/legislation/draft_versions_spec.rb +++ b/spec/features/legislation/draft_versions_spec.rb @@ -178,6 +178,7 @@ feature 'Legislation Draft Versions' do @annotation_1 = create(:legislation_annotation, draft_version: @draft_version, text: "my annotation", quote: "first quote") @annotation_2 = create(:legislation_annotation, draft_version: @draft_version, text: "my other annotation", quote: "second quote") end + scenario "See all annotations for a draft version" do visit legislation_process_draft_version_annotations_path(@draft_version.process, @draft_version) @@ -185,6 +186,45 @@ feature 'Legislation Draft Versions' do expect(page).to have_content "second quote" end + context "switching versions" do + background do + @process = create(:legislation_process) + @draft_version_1 = create(:legislation_draft_version, :published, process: @process, title: "Version 1", body: Faker::Lorem.paragraph) + @annotation_1 = create(:legislation_annotation, draft_version: @draft_version_1, text: "annotation for version 1", quote: "quote for version 1") + @draft_version_2 = create(:legislation_draft_version, :published, process: @process, title: "Version 2", body: Faker::Lorem.paragraph) + @annotation_1 = create(:legislation_annotation, draft_version: @draft_version_2, text: "annotation for version 2", quote: "quote for version 2") + end + + scenario "without js" do + visit legislation_process_draft_version_annotations_path(@process, @draft_version_1) + expect(page).to have_content("quote for version 1") + + select("Version 2") + click_button "see" + + expect(page).to_not have_content("quote for version 1") + expect(page).to have_content("quote for version 2") + end + + scenario "with js", :js do + visit legislation_process_draft_version_annotations_path(@process, @draft_version_1) + expect(page).to have_content("quote for version 1") + + select("Version 2") + + expect(page).to_not have_content("quote for version 1") + expect(page).to have_content("quote for version 2") + end + end + end + + context "Annotation comments page" do + background do + @draft_version = create(:legislation_draft_version, :published, body: Faker::Lorem.paragraph) + @annotation_1 = create(:legislation_annotation, draft_version: @draft_version, text: "my annotation", quote: "first quote") + @annotation_2 = create(:legislation_annotation, draft_version: @draft_version, text: "my other annotation", quote: "second quote") + end + scenario "See one annotation with replies for a draft version" do visit legislation_process_draft_version_annotation_path(@draft_version.process, @draft_version, @annotation_2) From 49e66a431c12560b76418f8779a67967e429b461 Mon Sep 17 00:00:00 2001 From: Amaia Castro Date: Fri, 13 Jan 2017 17:57:03 +0100 Subject: [PATCH 3/4] Check phase dates and that draft version is not final before creating annotations --- .../legislation/annotations_controller.rb | 18 ++++- .../legislation/answers_controller.rb | 2 +- .../annotations_controller_spec.rb | 70 +++++++++++++++++++ .../legislation/answers_controller_spec.rb | 5 -- spec/factories.rb | 4 ++ 5 files changed, 90 insertions(+), 9 deletions(-) create mode 100644 spec/controllers/legislation/annotations_controller_spec.rb diff --git a/app/controllers/legislation/annotations_controller.rb b/app/controllers/legislation/annotations_controller.rb index ff4ecf813..f1c4e4401 100644 --- a/app/controllers/legislation/annotations_controller.rb +++ b/app/controllers/legislation/annotations_controller.rb @@ -20,15 +20,22 @@ class Legislation::AnnotationsController < ApplicationController end def create - @annotation = Legislation::Annotation.new(annotation_params) + if !@process.open_phase?(:allegations) || @draft_version.final_version? + render json: {}, status: :not_found and return + end + + @annotation = @draft_version.annotations.new(annotation_params) @annotation.author = current_user if @annotation.save + track_event render json: @annotation.to_json + else + render json: {}, status: :unprocessable_entity end end def search - @annotations = Legislation::Annotation.where(legislation_draft_version_id: params[:legislation_draft_version_id]) + @annotations = @draft_version.annotations annotations_hash = { total: @annotations.size, rows: @annotations } render json: annotations_hash.to_json end @@ -43,7 +50,12 @@ class Legislation::AnnotationsController < ApplicationController params .require(:annotation) .permit(:quote, :text, ranges: [:start, :startOffset, :end, :endOffset]) - .merge(legislation_draft_version_id: params[:legislation_draft_version_id]) + end + + def track_event + ahoy.track "legislation_annotation_created".to_sym, + "legislation_annotation_id": @annotation.id, + "legislation_draft_version_id": @draft_version.id end end diff --git a/app/controllers/legislation/answers_controller.rb b/app/controllers/legislation/answers_controller.rb index ac77ea5f1..372398e41 100644 --- a/app/controllers/legislation/answers_controller.rb +++ b/app/controllers/legislation/answers_controller.rb @@ -19,7 +19,7 @@ class Legislation::AnswersController < Legislation::BaseController end else respond_to do |format| - format.js + format.js { render json: {} , status: :not_found } format.html { redirect_to legislation_process_question_path(@process, @question), alert: t('legislation.questions.participation.phase_not_open') } end end diff --git a/spec/controllers/legislation/annotations_controller_spec.rb b/spec/controllers/legislation/annotations_controller_spec.rb new file mode 100644 index 000000000..2713fc273 --- /dev/null +++ b/spec/controllers/legislation/annotations_controller_spec.rb @@ -0,0 +1,70 @@ +require 'rails_helper' + +describe Legislation::AnnotationsController do + + describe 'POST create' do + before(:each) do + @process = create(:legislation_process, allegations_start_date: Date.current - 3.day, allegations_end_date: Date.current + 2.days) + @draft_version = create(:legislation_draft_version, :published, process: @process, title: "Version 1") + @final_version = create(:legislation_draft_version, :published, :final_version, process: @process, title: "Final version") + @user = create(:user, :level_two) + end + + it 'should create an ahoy event' do + sign_in @user + + post :create, process_id: @process.id, + draft_version_id: @draft_version.id, + annotation: { + "quote"=>"Ordenación Territorial", + "ranges"=>[{"start"=>"/p[1]", "startOffset"=>1, "end"=>"/p[1]", "endOffset"=>3}], + "text": "una anotacion" + } + expect(Ahoy::Event.where(name: :legislation_annotation_created).count).to eq 1 + expect(Ahoy::Event.last.properties['legislation_annotation_id']).to eq Legislation::Annotation.last.id + end + + it 'should not create an annotation if the draft version is a final version' do + sign_in @user + + post :create, process_id: @process.id, + draft_version_id: @final_version.id, + annotation: { + "quote"=>"Ordenación Territorial", + "ranges"=>[{"start"=>"/p[1]", "startOffset"=>1, "end"=>"/p[1]", "endOffset"=>3}], + "text": "una anotacion" + } + + expect(response).to have_http_status(:not_found) + end + + it 'should create an annotation if the process allegations phase is open' do + sign_in @user + + expect do + xhr :post, :create, process_id: @process.id, + draft_version_id: @draft_version.id, + annotation: { + "quote"=>"Ordenación Territorial", + "ranges"=>[{"start"=>"/p[1]", "startOffset"=>1, "end"=>"/p[1]", "endOffset"=>3}], + "text": "una anotacion" + } + end.to change { @draft_version.annotations.count }.by(1) + end + + it 'should not create an annotation if the process allegations phase is not open' do + sign_in @user + @process.update_attribute(:allegations_end_date, Date.current - 1.day) + + expect do + xhr :post, :create, process_id: @process.id, + draft_version_id: @draft_version.id, + annotation: { + "quote"=>"Ordenación Territorial", + "ranges"=>[{"start"=>"/p[1]", "startOffset"=>1, "end"=>"/p[1]", "endOffset"=>3}], + "text": "una anotacion" + } + end.to_not change { @draft_version.annotations.count } + end + end +end diff --git a/spec/controllers/legislation/answers_controller_spec.rb b/spec/controllers/legislation/answers_controller_spec.rb index 99efcef67..77d037478 100644 --- a/spec/controllers/legislation/answers_controller_spec.rb +++ b/spec/controllers/legislation/answers_controller_spec.rb @@ -4,17 +4,12 @@ describe Legislation::AnswersController do describe 'POST create' do before(:each) do - InvisibleCaptcha.timestamp_enabled = false @process = create(:legislation_process, debate_start_date: Date.current - 3.day, debate_end_date: Date.current + 2.days) @question = create(:legislation_question, process: @process, title: "Question 1") @question_option = create(:legislation_question_option, question: @question, value: "Yes") @user = create(:user, :level_two) end - after(:each) do - InvisibleCaptcha.timestamp_enabled = true - end - it 'should create an ahoy event' do sign_in @user diff --git a/spec/factories.rb b/spec/factories.rb index a7ee19610..5ed16fc30 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -499,6 +499,10 @@ FactoryGirl.define do trait :published do status "published" end + + trait :final_version do + final_version true + end end factory :legislation_annotation, class: 'Legislation::Annotation' do From 49f4bf5bc568027760d103da1ce2c1f0c039863a Mon Sep 17 00:00:00 2001 From: Amaia Castro Date: Fri, 13 Jan 2017 19:34:17 +0100 Subject: [PATCH 4/4] Destroy object when their parent object is destroyed --- app/models/legislation/annotation.rb | 2 +- app/models/legislation/draft_version.rb | 2 +- app/models/legislation/process.rb | 4 ++-- app/models/legislation/question.rb | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/models/legislation/annotation.rb b/app/models/legislation/annotation.rb index d62363946..2bdfb3fca 100644 --- a/app/models/legislation/annotation.rb +++ b/app/models/legislation/annotation.rb @@ -6,7 +6,7 @@ class Legislation::Annotation < ActiveRecord::Base belongs_to :draft_version, class_name: 'Legislation::DraftVersion', foreign_key: 'legislation_draft_version_id' belongs_to :author, -> { with_hidden }, class_name: 'User', foreign_key: 'author_id' - has_many :comments, as: :commentable + has_many :comments, as: :commentable, dependent: :destroy validates :text, presence: true validates :quote, presence: true diff --git a/app/models/legislation/draft_version.rb b/app/models/legislation/draft_version.rb index d0150f1ff..297cef236 100644 --- a/app/models/legislation/draft_version.rb +++ b/app/models/legislation/draft_version.rb @@ -5,7 +5,7 @@ class Legislation::DraftVersion < ActiveRecord::Base include ActsAsParanoidAliases belongs_to :process, class_name: 'Legislation::Process', foreign_key: 'legislation_process_id' - has_many :annotations, class_name: 'Legislation::Annotation', foreign_key: 'legislation_draft_version_id' + has_many :annotations, class_name: 'Legislation::Annotation', foreign_key: 'legislation_draft_version_id', dependent: :destroy validates :title, presence: true validates :body, presence: true diff --git a/app/models/legislation/process.rb b/app/models/legislation/process.rb index 291e85b5a..5c4fc391b 100644 --- a/app/models/legislation/process.rb +++ b/app/models/legislation/process.rb @@ -2,9 +2,9 @@ class Legislation::Process < ActiveRecord::Base acts_as_paranoid column: :hidden_at include ActsAsParanoidAliases - has_many :draft_versions, -> { order(:id) }, class_name: 'Legislation::DraftVersion', foreign_key: 'legislation_process_id' + has_many :draft_versions, -> { order(:id) }, class_name: 'Legislation::DraftVersion', foreign_key: 'legislation_process_id', dependent: :destroy has_one :final_draft_version, -> { where final_version: true, status: 'published' }, class_name: 'Legislation::DraftVersion', foreign_key: 'legislation_process_id' - has_many :questions, -> { order(:id) }, class_name: 'Legislation::Question', foreign_key: 'legislation_process_id' + has_many :questions, -> { order(:id) }, class_name: 'Legislation::Question', foreign_key: 'legislation_process_id', dependent: :destroy validates :title, presence: true validates :description, presence: true diff --git a/app/models/legislation/question.rb b/app/models/legislation/question.rb index c1dacd464..5ab0ea355 100644 --- a/app/models/legislation/question.rb +++ b/app/models/legislation/question.rb @@ -7,7 +7,7 @@ class Legislation::Question < ActiveRecord::Base has_many :question_options, -> { order(:id) }, class_name: 'Legislation::QuestionOption', foreign_key: 'legislation_question_id', dependent: :destroy, inverse_of: :question has_many :answers, class_name: 'Legislation::Answer', foreign_key: 'legislation_question_id', dependent: :destroy, inverse_of: :question - has_many :comments, as: :commentable + has_many :comments, as: :commentable, dependent: :destroy accepts_nested_attributes_for :question_options, :reject_if => proc { |attributes| attributes[:value].blank? }, allow_destroy: true