From 14df74fed74497576281efe9c83340c302665b10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 16 Jul 2020 19:23:04 +0200 Subject: [PATCH] Add collaborative legislation summary again It was removed in commit 128a8164 because we hadn't reviewed it nor tested it properly. We're now adding it again, fixing the issues we've found while reviewing. --- .../stylesheets/legislation_process.scss | 81 +++++++++ app/assets/stylesheets/participation.scss | 76 ++++---- .../legislation/processes_controller.rb | 6 + app/models/abilities/administrator.rb | 4 +- app/models/abilities/everyone.rb | 2 + app/models/comment.rb | 5 + app/models/legislation/draft_version.rb | 4 + app/models/legislation/question.rb | 4 + .../legislation/processes/_key_dates.html.erb | 9 + .../processes/_summary_allegations.html.erb | 32 ++++ .../processes/_summary_comments.html.erb | 10 + .../processes/_summary_debate.html.erb | 28 +++ .../processes/_summary_proposals.html.erb | 23 +++ .../legislation/processes/summary.html.erb | 25 +++ config/locales/en/legislation.yml | 28 +++ config/locales/es/legislation.yml | 28 +++ config/routes/legislation.rb | 1 + spec/models/abilities/administrator_spec.rb | 8 + spec/models/abilities/everyone_spec.rb | 4 + spec/system/legislation/summary_spec.rb | 172 ++++++++++++++++++ 20 files changed, 513 insertions(+), 37 deletions(-) create mode 100644 app/views/legislation/processes/_summary_allegations.html.erb create mode 100644 app/views/legislation/processes/_summary_comments.html.erb create mode 100644 app/views/legislation/processes/_summary_debate.html.erb create mode 100644 app/views/legislation/processes/_summary_proposals.html.erb create mode 100644 app/views/legislation/processes/summary.html.erb create mode 100644 spec/system/legislation/summary_spec.rb diff --git a/app/assets/stylesheets/legislation_process.scss b/app/assets/stylesheets/legislation_process.scss index cb4488004..81c090a5f 100644 --- a/app/assets/stylesheets/legislation_process.scss +++ b/app/assets/stylesheets/legislation_process.scss @@ -1006,3 +1006,84 @@ font-weight: bold; } } + +// 10. Legislation summary +// ------------------------- + +.process-summary { + > section { + @include grid-row; + margin-top: $line-height * 1.5; + padding: 0 rem-calc(16); + + > header { + background: none; + border: 0; + margin: 0; + } + } + + h4, + p { + margin-bottom: 0; + } + + > section > header, + .question-title, + .annotation-title, + .comment-summary, + .proposal-summary { + @include breakpoint(medium) { + align-items: center; + display: flex; + + > :first-child { + $margin: rem-calc(map-get($grid-column-gutter, "medium")); + + margin-right: $margin; + width: calc(75% - #{$margin}); + } + } + } + + .debate-summary, + .proposal-summary, + .annotation-summary { + @extend %panel; + min-height: auto; + padding-bottom: rem-calc(16); + padding-top: rem-calc(16); + + &:not(:last-child) { + margin-bottom: $line-height / 2; + } + } + + .comments-count { + @include has-fa-icon(comments, r); + } + + .question-title:not(:only-child) { + margin-bottom: $line-height / 2; + } + + .annotation-title { + margin-bottom: $line-height / 2; + margin-top: $line-height / 2; + } + + .annotation-quote { + border: 1px solid $black; + padding: rem-calc(10); + } + + .comment-summary { + margin-bottom: $line-height / 2; + + > :first-child { + background-color: rgba(217, 216, 243, 0.2); + border-radius: rem-calc(10); + padding: rem-calc(12); + } + } +} diff --git a/app/assets/stylesheets/participation.scss b/app/assets/stylesheets/participation.scss index 895e4d2cc..a825c77bb 100644 --- a/app/assets/stylesheets/participation.scss +++ b/app/assets/stylesheets/participation.scss @@ -612,6 +612,45 @@ } } +%panel { + background: #fff; + border: 1px solid; + border-color: #e5e6e9 #dfe0e4 #d0d1d5; + border-radius: 0; + box-shadow: 0 1px 3px 0 $border; + margin-bottom: rem-calc(12); + min-height: rem-calc(192); + padding: rem-calc(12) rem-calc(12) 0; + + @include breakpoint(medium) { + margin-bottom: rem-calc(-1); + padding-bottom: rem-calc(12); + } + + @include breakpoint(medium) { + .divider { + display: inline-block; + } + } + + h3 { + font-weight: bold; + margin-top: $line-height / 2; + + a { + color: $text; + } + } + + &.past-budgets { + min-height: 0; + + .button ~ .button { + margin-left: $line-height / 2; + } + } +} + .debate, .proposal, .investment-project, @@ -621,42 +660,7 @@ margin: $line-height / 4 0; .panel { - background: #fff; - border: 1px solid; - border-color: #e5e6e9 #dfe0e4 #d0d1d5; - border-radius: 0; - box-shadow: 0 1px 3px 0 $border; - margin-bottom: rem-calc(12); - min-height: rem-calc(192); - padding: rem-calc(12) rem-calc(12) 0; - - @include breakpoint(medium) { - margin-bottom: rem-calc(-1); - padding-bottom: rem-calc(12); - } - - @include breakpoint(medium) { - .divider { - display: inline-block; - } - } - - h3 { - font-weight: bold; - margin-top: $line-height / 2; - - a { - color: $text; - } - } - - &.past-budgets { - min-height: 0; - - .button ~ .button { - margin-left: $line-height / 2; - } - } + @extend %panel; } .debate-content, diff --git a/app/controllers/legislation/processes_controller.rb b/app/controllers/legislation/processes_controller.rb index d6dd6d3e5..2c8f01076 100644 --- a/app/controllers/legislation/processes_controller.rb +++ b/app/controllers/legislation/processes_controller.rb @@ -97,6 +97,12 @@ class Legislation::ProcessesController < Legislation::BaseController @phase = :milestones end + def summary + @phase = :summary + @proposals = @process.proposals.selected + @comments = @process.draft_versions.published.last&.best_comments || Comment.none + end + def proposals set_process @phase = :proposals_phase diff --git a/app/models/abilities/administrator.rb b/app/models/abilities/administrator.rb index 25dcb60c7..24b70e453 100644 --- a/app/models/abilities/administrator.rb +++ b/app/models/abilities/administrator.rb @@ -90,7 +90,9 @@ module Abilities can :access, :ckeditor can :manage, Ckeditor::Picture - can [:manage], ::Legislation::Process + can [:read, :debate, :draft_publication, :allegations, :result_publication, + :milestones], Legislation::Process + can [:create, :update, :destroy], Legislation::Process can [:manage], ::Legislation::DraftVersion can [:manage], ::Legislation::Question can [:manage], ::Legislation::Proposal diff --git a/app/models/abilities/everyone.rb b/app/models/abilities/everyone.rb index 89a29f4b9..432099150 100644 --- a/app/models/abilities/everyone.rb +++ b/app/models/abilities/everyone.rb @@ -21,6 +21,8 @@ module Abilities can :new, DirectMessage can [:read, :debate, :draft_publication, :allegations, :result_publication, :proposals, :milestones], Legislation::Process, published: true + can :summary, Legislation::Process, + id: Legislation::Process.past.published.where(result_publication_enabled: true).ids can [:read, :changes, :go_to_version], Legislation::DraftVersion can [:read], Legislation::Question can [:read, :map, :share], Legislation::Proposal diff --git a/app/models/comment.rb b/app/models/comment.rb index 1552ce3a5..7af862a73 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -49,6 +49,7 @@ class Comment < ApplicationRecord scope :sort_by_most_voted, -> { order(confidence_score: :desc, created_at: :desc) } scope :sort_descendants_by_most_voted, -> { order(confidence_score: :desc, created_at: :asc) } + scope :sort_by_supports, -> { order("cached_votes_up - cached_votes_down DESC") } scope :sort_by_newest, -> { order(created_at: :desc) } scope :sort_descendants_by_newest, -> { order(created_at: :desc) } @@ -129,6 +130,10 @@ class Comment < ApplicationRecord cached_votes_up) end + def votes_score + cached_votes_up - cached_votes_down + end + private def validate_body_length diff --git a/app/models/legislation/draft_version.rb b/app/models/legislation/draft_version.rb index 93b65eb8f..9019d6cb8 100644 --- a/app/models/legislation/draft_version.rb +++ b/app/models/legislation/draft_version.rb @@ -40,4 +40,8 @@ class Legislation::DraftVersion < ApplicationRecord def total_comments annotations.sum(:comments_count) end + + def best_comments + Comment.where(commentable: annotations, ancestry: nil).sort_by_supports.limit(10) + end end diff --git a/app/models/legislation/question.rb b/app/models/legislation/question.rb index ff9e0ea94..73f1cc946 100644 --- a/app/models/legislation/question.rb +++ b/app/models/legislation/question.rb @@ -44,4 +44,8 @@ class Legislation::Question < ApplicationRecord def comments_open? process.debate_phase.open? end + + def best_comments + comments.sort_by_supports.limit(3) + end end diff --git a/app/views/legislation/processes/_key_dates.html.erb b/app/views/legislation/processes/_key_dates.html.erb index 4e8ab85c0..7e5842d1a 100644 --- a/app/views/legislation/processes/_key_dates.html.erb +++ b/app/views/legislation/processes/_key_dates.html.erb @@ -52,6 +52,15 @@ <% end %> <% end %> + + <% if can?(:summary, process) %> +
  • > + <%= link_to summary_legislation_process_path(process) do %> +

    <%= t("legislation.summary.title") %>

    + <%= format_date(process.result_publication_date) %> + <% end %> +
    • + <% end %> diff --git a/app/views/legislation/processes/_summary_allegations.html.erb b/app/views/legislation/processes/_summary_allegations.html.erb new file mode 100644 index 000000000..345542dc1 --- /dev/null +++ b/app/views/legislation/processes/_summary_allegations.html.erb @@ -0,0 +1,32 @@ +
    +
    +

    <%= t("legislation.summary.allegations_phase") %>

    +

    <%= t("legislation.summary.top_comments", count: comments.count) %>

    +
    + + <% if comments.any? %> +
    + <% comments.group_by(&:commentable).each do |annotation, annotation_comments| %> +
    + <%= t("legislation.annotations.index.comments_about") %> +
    +
    + <%= annotation.quote %> +
    + + + <%= link_to t("legislation.summary.comments", count: annotation.comments.count), + polymorphic_path(annotation, anchor: "comments") %> + +
    + + <%= render "summary_comments", comments: annotation_comments %> +
    + <% end %> +
    + <% else %> +
    +

    <%= t("legislation.summary.no_allegation") %>

    +
    + <% end %> +
    diff --git a/app/views/legislation/processes/_summary_comments.html.erb b/app/views/legislation/processes/_summary_comments.html.erb new file mode 100644 index 000000000..7e8b61d60 --- /dev/null +++ b/app/views/legislation/processes/_summary_comments.html.erb @@ -0,0 +1,10 @@ +<% if comments.any? %> + <%= t("legislation.summary.most_voted_comments") %> + + <% comments.each do |comment| %> +
    + <%= link_to simple_format(sanitize(comment.body, tags: [])), comment_path(comment) %> +

    <%= t("legislation.summary.votes", count: comment.votes_score) %>

    +
    + <% end %> +<% end %> diff --git a/app/views/legislation/processes/_summary_debate.html.erb b/app/views/legislation/processes/_summary_debate.html.erb new file mode 100644 index 000000000..8530f192d --- /dev/null +++ b/app/views/legislation/processes/_summary_debate.html.erb @@ -0,0 +1,28 @@ +
    +
    +

    <%= t("legislation.summary.debate_phase") %>

    +

    <%= t("legislation.summary.debates", count: questions.count) %>

    +
    + + <% if questions.any? %> +
    + <% questions.each do |question| %> +
    +
    +

    <%= link_to question.title, polymorphic_path(question) %>

    + + <%= link_to t("legislation.summary.comments", count: question.comments.count), + polymorphic_path(question, anchor: "comments") %> + +
    + + <%= render "summary_comments", comments: question.best_comments %> +
    + <% end %> +
    + <% else %> +
    +

    <%= t("legislation.processes.debate.empty_questions") %>

    +
    + <% end %> +
    diff --git a/app/views/legislation/processes/_summary_proposals.html.erb b/app/views/legislation/processes/_summary_proposals.html.erb new file mode 100644 index 000000000..09ccca777 --- /dev/null +++ b/app/views/legislation/processes/_summary_proposals.html.erb @@ -0,0 +1,23 @@ +
    +
    +

    <%= t("legislation.summary.proposals_phase") %>

    +

    <%= t("legislation.summary.proposals", count: proposals.count) %>

    +
    + + <% if proposals.any? %> +
    + <% proposals.sort_by_supports.each do |proposal| %> +
    +

    + <%= link_to proposal.title, polymorphic_path(proposal) %> +

    +

    <%= t("legislation.summary.votes", count: proposal.votes_score) %>

    +
    + <% end %> +
    + <% else %> +
    +

    <%= t("legislation.processes.proposals.empty_proposals") %>

    +
    + <% end %> +
    diff --git a/app/views/legislation/processes/summary.html.erb b/app/views/legislation/processes/summary.html.erb new file mode 100644 index 000000000..97c211d7a --- /dev/null +++ b/app/views/legislation/processes/summary.html.erb @@ -0,0 +1,25 @@ +<% provide(:title) { @process.title } %> + +<%= render "legislation/processes/header", process: @process, header: :full %> + +<%= render "key_dates", process: @process, phase: @phase %> + +<% if @process.debate_phase.enabled? || @process.proposals_phase.enabled? || @process.allegations_phase.enabled? %> +
    + <% if @process.debate_phase.enabled? %> + <%= render "summary_debate", questions: @process.questions %> + <% end %> + + <% if @process.proposals_phase.enabled? %> + <%= render "summary_proposals", proposals: @proposals %> + <% end %> + + <% if @process.allegations_phase.enabled? %> + <%= render "summary_allegations", comments: @comments %> + <% end %> +
    +<% else %> +
    +

    <%= t("legislation.summary.process_empty") %>

    +
    +<% end %> diff --git a/config/locales/en/legislation.yml b/config/locales/en/legislation.yml index 967f6d1d9..665cb35ee 100644 --- a/config/locales/en/legislation.yml +++ b/config/locales/en/legislation.yml @@ -118,3 +118,31 @@ en: tags_label: "Categories" not_verified: "For vote proposals %{verify_account}." process_title: Collaborative legislation process + summary: + title: "Summary" + votes: + zero: "%{count} votes" + one: "%{count} vote" + other: "%{count} votes" + debate_phase: "Debate phase" + proposals_phase: "Proposals phase" + allegations_phase: "Comments phase" + debates: + zero: "No debates" + one: "%{count} debate" + other: "%{count} debates" + proposals: + zero: "No proposals" + one: "%{count} proposal" + other: "%{count} proposals" + comments: + zero: "No comments" + one: "%{count} comment" + other: "%{count} comments" + top_comments: + zero: "No comments" + one: "%{count} comment" + other: "Top comments" + most_voted_comments: "Most voted comments" + no_allegation: "There are no comments" + process_empty: "This process didn't have any participation phases" diff --git a/config/locales/es/legislation.yml b/config/locales/es/legislation.yml index fe723e250..2c88e185b 100644 --- a/config/locales/es/legislation.yml +++ b/config/locales/es/legislation.yml @@ -118,3 +118,31 @@ es: tags_label: "Categorías" not_verified: "Para votar propuestas %{verify_account}." process_title: Proceso de legislación colaborativa + summary: + title: "Resumen" + votes: + zero: "%{count} votos" + one: "%{count} voto" + other: "%{count} votos" + debate_phase: "Fase de debate" + proposals_phase: "Fase de propuestas" + allegations_phase: "Fase de comentarios" + debates: + zero: "No hay debates" + one: "%{count} debate" + other: "%{count} debates" + proposals: + zero: "No hay propuestas" + one: "%{count} propuesta" + other: "%{count} propuestas" + comments: + zero: "No hay comentarios" + one: "%{count} comentario" + other: "%{count} comentarios" + top_comments: + zero: "No hay comentarios" + one: "%{count} comentario" + other: "Los más votados" + most_voted_comments: "Comentarios más votados" + no_allegation: "No hay comentarios" + process_empty: "Este proceso no ha tenido fases de participación" diff --git a/config/routes/legislation.rb b/config/routes/legislation.rb index 2458a2bae..ee92ce26c 100644 --- a/config/routes/legislation.rb +++ b/config/routes/legislation.rb @@ -7,6 +7,7 @@ namespace :legislation do get :result_publication get :proposals get :milestones + get :summary end resources :questions, only: [:show] do diff --git a/spec/models/abilities/administrator_spec.rb b/spec/models/abilities/administrator_spec.rb index bbf8f91fe..c87cc889d 100644 --- a/spec/models/abilities/administrator_spec.rb +++ b/spec/models/abilities/administrator_spec.rb @@ -18,6 +18,10 @@ describe Abilities::Administrator do let(:legislation_question) { create(:legislation_question) } let(:poll_question) { create(:poll_question) } + let(:past_process) { create(:legislation_process, :past) } + let(:past_draft_process) { create(:legislation_process, :past, :not_published) } + let(:open_process) { create(:legislation_process, :open) } + let(:proposal_document) { build(:document, documentable: proposal, user: proposal.author) } let(:budget_investment_document) { build(:document, documentable: budget_investment) } let(:poll_question_document) { build(:document, documentable: poll_question) } @@ -67,6 +71,10 @@ describe Abilities::Administrator do it { should be_able_to(:comment_as_administrator, legislation_question) } it { should_not be_able_to(:comment_as_moderator, legislation_question) } + it { should be_able_to(:summary, past_process) } + it { should_not be_able_to(:summary, past_draft_process) } + it { should_not be_able_to(:summary, open_process) } + it { should be_able_to(:create, Budget) } it { should be_able_to(:update, Budget) } it { should be_able_to(:read_results, Budget) } diff --git a/spec/models/abilities/everyone_spec.rb b/spec/models/abilities/everyone_spec.rb index cfdbbf932..fdc03c74d 100644 --- a/spec/models/abilities/everyone_spec.rb +++ b/spec/models/abilities/everyone_spec.rb @@ -48,4 +48,8 @@ describe Abilities::Everyone do it { should be_able_to(:read_stats, create(:budget, :valuating, stats_enabled: true)) } it { should_not be_able_to(:read_stats, create(:budget, :valuating, stats_enabled: false)) } it { should_not be_able_to(:read_stats, create(:budget, :selecting, stats_enabled: true)) } + + it { should be_able_to(:summary, create(:legislation_process, :past)) } + it { should_not be_able_to(:summary, create(:legislation_process, :open)) } + it { should_not be_able_to(:summary, create(:legislation_process, :past, :not_published)) } end diff --git a/spec/system/legislation/summary_spec.rb b/spec/system/legislation/summary_spec.rb new file mode 100644 index 000000000..0763c16ba --- /dev/null +++ b/spec/system/legislation/summary_spec.rb @@ -0,0 +1,172 @@ +require "rails_helper" + +describe "Legislation" do + context "process summary page" do + scenario "summary tab is not shown for open processes" do + process = create(:legislation_process, :open) + + visit legislation_process_path(process) + + expect(page).not_to have_content "Summary" + end + + scenario "summary tab is shown por past processes" do + process = create(:legislation_process, :past) + + visit legislation_process_path(process) + + expect(page).to have_content "Summary" + end + end + + scenario "empty process" do + process = create(:legislation_process, :empty, + result_publication_enabled: true, + end_date: Date.current - 1.day + ) + + visit summary_legislation_process_path(process) + + expect(page).to have_content "This process didn't have any participation phases" + end + + scenario "empty phases" do + process = create(:legislation_process, end_date: Date.current - 1.day) + visit summary_legislation_process_path(process) + + expect(page).to have_content "Debate phase" + expect(page).to have_content "No debates" + expect(page).to have_content "There aren't any questions" + + expect(page).to have_content "Proposals phase" + expect(page).to have_content "No proposals" + expect(page).to have_content "There are no proposals" + + expect(page).to have_content "Comments phase" + expect(page).to have_content "No comments" + expect(page).to have_content "There are no comments" + end + + context "only debates exist" do + let(:process) { create(:legislation_process, end_date: Date.current - 1.day) } + let(:user) { create(:user, :level_two) } + + before do + create(:legislation_question, process: process, title: "Question 1") do |question| + create(:comment, user: user, commentable: question, body: "Answer 1") + create(:comment, user: user, commentable: question, body: "Answer 2") + end + + create(:legislation_question, process: process, title: "Question 2") do |question| + create(:comment, user: user, commentable: question, body: "Answer 3") + create(:comment, user: user, commentable: question, body: "Answer 4") + end + end + + scenario "shows debates list" do + visit summary_legislation_process_path(process) + + expect(page).to have_content "Debate phase" + expect(page).to have_content "2 debates" + expect(page).to have_link "Question 1" + expect(page).to have_content "Answer 1" + expect(page).to have_content "Answer 2" + expect(page).to have_link "Question 2" + expect(page).to have_content "Answer 3" + expect(page).to have_content "Answer 4" + + expect(page).to have_content "Proposals phase" + expect(page).to have_content "No proposals" + expect(page).to have_content "There are no proposals" + + expect(page).to have_content "Comments phase" + expect(page).to have_content "No comments" + expect(page).to have_content "There are no comments" + end + end + + context "only proposals exist" do + let(:process) { create(:legislation_process, end_date: Date.current - 1.day) } + + before do + create(:legislation_proposal, legislation_process_id: process.id, + title: "Legislation proposal 1", selected: true) + create(:legislation_proposal, legislation_process_id: process.id, + title: "Legislation proposal 2", selected: false) + create(:legislation_proposal, legislation_process_id: process.id, + title: "Legislation proposal 3", selected: true) + create(:legislation_proposal, legislation_process_id: process.id, + title: "Legislation proposal 4", selected: false) + end + + scenario "shows proposals list" do + visit summary_legislation_process_path(process) + + expect(page).to have_content "Debate phase" + expect(page).to have_content "No debates" + expect(page).to have_content "There aren't any questions" + + expect(page).to have_content "Proposals phase" + expect(page).to have_content "2 proposals" + expect(page).to have_link "Legislation proposal 1" + expect(page).not_to have_content "Legislation proposal 2" + expect(page).to have_link "Legislation proposal 3" + expect(page).not_to have_content "Legislation proposal 4" + + expect(page).to have_content "Comments phase" + expect(page).to have_content "No comments" + expect(page).to have_content "There are no comments" + end + end + + context "only text comments exist" do + let(:process) { create(:legislation_process, end_date: Date.current - 1.day) } + + before do + user = create(:user, :level_two) + draft_version_1 = create(:legislation_draft_version, process: process, + title: "Version 1", body: "Body of the first version", + status: "published") + draft_version_2 = create(:legislation_draft_version, process: process, + title: "Version 2", body: "Body of the second version and that's it all of it", + status: "published") + annotation0 = create(:legislation_annotation, + draft_version: draft_version_1, text: "my annotation123", + ranges: annotation_ranges(5, 10)) + annotation1 = create(:legislation_annotation, + draft_version: draft_version_2, text: "hola", + ranges: annotation_ranges(5, 10)) + annotation2 = create(:legislation_annotation, + draft_version: draft_version_2, + ranges: annotation_ranges(12, 19)) + + create(:comment, user: user, commentable: annotation0, body: "Comment 0") + create(:comment, user: user, commentable: annotation1, body: "Comment 1") + create(:comment, user: user, commentable: annotation2, body: "Comment 2") + create(:comment, user: user, commentable: annotation2, body: "Comment 3") + end + + scenario "shows coments list" do + visit summary_legislation_process_path(process) + + expect(page).to have_content "Debate phase" + expect(page).to have_content "No debates" + expect(page).to have_content "There aren't any questions" + + expect(page).to have_content "Proposals phase" + expect(page).to have_content "No proposals" + expect(page).to have_content "There are no proposals" + + expect(page).to have_content "Comments phase" + expect(page).to have_content "Top comments" + expect(page).not_to have_content "Comment 0" + expect(page).to have_link "Comment 1" + expect(page).to have_link "Comment 2" + expect(page).to have_link "Comment 3" + end + end + + def annotation_ranges(start_offset, end_offset) + [{ "start" => "/p[1]", "startOffset" => start_offset, "end" => "/p[1]", "endOffset" => end_offset }] + end +end