From 3dc07837d9716dff12ad9fa4918f470dd5e89c4d Mon Sep 17 00:00:00 2001 From: Vera Rojman Date: Thu, 10 Dec 2020 09:19:45 +0100 Subject: [PATCH] Archive Debates (#6940) * Archive and unarchive debates * Add empty message * Fix erb, rb and i18norm offenses * Archive debate from debate close form * Put back accidentally removed create_debates migration * Add spec for Archive command * Add spec for archived? method in debate * Add system spec for archiving debates * Enable unarchiving from close form * Add seeds for archived debates * Change icon class when debate is closed / archived * Allow admin to archive non-official debates * Allow open debates to be archived too --- .../decidim/debates/admin/archive_debate.rb | 50 +++++++++ .../decidim/debates/admin/close_debate.rb | 3 +- .../debates/admin/debates_controller.rb | 30 ++++- .../debates/admin/close_debate_form.rb | 5 + .../debates/admin/application_helper.rb | 9 ++ .../app/models/decidim/debates/debate.rb | 16 +++ .../decidim/debates/admin/permissions.rb | 2 + .../debates/admin/debate_closes/edit.html.erb | 6 + .../debates/admin/debates/index.html.erb | 105 ++++++++++-------- decidim-debates/config/locales/en.yml | 13 +++ .../migrate/20201126112752_archive_debates.rb | 8 ++ .../lib/decidim/debates/admin_engine.rb | 1 + .../lib/decidim/debates/component.rb | 29 +++-- .../debates/admin/archive_debate_spec.rb | 48 ++++++++ .../models/decidim/debates/debate_spec.rb | 16 +++ .../decidim/debates/admin/permissions_spec.rb | 30 +++++ .../spec/shared/manage_debates_examples.rb | 44 +++++++- 17 files changed, 355 insertions(+), 60 deletions(-) create mode 100644 decidim-debates/app/commands/decidim/debates/admin/archive_debate.rb create mode 100644 decidim-debates/db/migrate/20201126112752_archive_debates.rb create mode 100644 decidim-debates/spec/commands/decidim/debates/admin/archive_debate_spec.rb diff --git a/decidim-debates/app/commands/decidim/debates/admin/archive_debate.rb b/decidim-debates/app/commands/decidim/debates/admin/archive_debate.rb new file mode 100644 index 000000000000..39a519c2f79c --- /dev/null +++ b/decidim-debates/app/commands/decidim/debates/admin/archive_debate.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +module Decidim + module Debates + module Admin + # A command with all the business logic when an admin archives a debate. + class ArchiveDebate < Rectify::Command + # Public: Initializes the command. + # + # archive - Boolean, whether to archive (true) or unarchive (false) the debate. + # debate - The debate object to archive. + # user - The user performing the action. + def initialize(archive, debate, user) + @archive = archive + @debate = debate + @user = user + end + + # Executes the command. Broadcasts these events: + # + # - :ok when the debate is valid. + # - :invalid if the debate wasn't valid and we couldn't proceed. + # + # Returns nothing. + def call + archive_debate + broadcast(:ok) + rescue ActiveRecord::RecordInvalid + broadcast(:invalid) + end + + attr_reader :debate + + private + + def archive_debate + @debate = Decidim.traceability.perform_action!( + :close, + @debate, + @user + ) do + @debate.update!( + archived_at: @archive ? Time.zone.now : nil + ) + end + end + end + end + end +end diff --git a/decidim-debates/app/commands/decidim/debates/admin/close_debate.rb b/decidim-debates/app/commands/decidim/debates/admin/close_debate.rb index 8c60fbeaac36..4c7940b516b4 100644 --- a/decidim-debates/app/commands/decidim/debates/admin/close_debate.rb +++ b/decidim-debates/app/commands/decidim/debates/admin/close_debate.rb @@ -37,7 +37,8 @@ def close_debate ) do form.debate.update!( conclusions: form.conclusions, - closed_at: form.closed_at + closed_at: form.closed_at, + archived_at: (Time.zone.now if form.archive) ) end diff --git a/decidim-debates/app/controllers/decidim/debates/admin/debates_controller.rb b/decidim-debates/app/controllers/decidim/debates/admin/debates_controller.rb index ba2ae310cefa..bfe20506c6b3 100644 --- a/decidim-debates/app/controllers/decidim/debates/admin/debates_controller.rb +++ b/decidim-debates/app/controllers/decidim/debates/admin/debates_controller.rb @@ -61,6 +61,24 @@ def update end end + def archive + enforce_permission_to :archive, :debate, debate: debate + + archive = params[:archive] == "true" + + ArchiveDebate.call(archive, debate, current_user) do + on(:ok) do + flash[:notice] = I18n.t("debates.#{archive ? "archive" : "unarchive"}.success", scope: "decidim.debates.admin") + redirect_to debates_path(archive ? {} : { filter: "archive" }) + end + + on(:invalid) do + flash.now[:alert] = I18n.t("debates.#{archive ? "archive" : "unarchive"}.invalid", scope: "decidim.debates.admin") + redirect_to debates_path(archive ? {} : { filter: "archive" }) + end + end + end + def destroy enforce_permission_to :delete, :debate, debate: debate @@ -74,11 +92,19 @@ def destroy private def debates - @debates ||= Debate.where(component: current_component) + @debates ||= archive? ? all_debates.archived : all_debates.not_archived end def debate - @debate ||= debates.find(params[:id]) + @debate ||= all_debates.find(params[:id]) + end + + def all_debates + Debate.where(component: current_component) + end + + def archive? + params[:filter] == "archive" end end end diff --git a/decidim-debates/app/forms/decidim/debates/admin/close_debate_form.rb b/decidim-debates/app/forms/decidim/debates/admin/close_debate_form.rb index cdc518decd74..8ead2f9d065c 100644 --- a/decidim-debates/app/forms/decidim/debates/admin/close_debate_form.rb +++ b/decidim-debates/app/forms/decidim/debates/admin/close_debate_form.rb @@ -15,6 +15,7 @@ class CloseDebateForm < Decidim::Form end attribute :debate, Debate + attribute :archive, Boolean validates :debate, presence: true validate :user_can_close_debate @@ -23,6 +24,10 @@ def closed_at debate&.closed_at || Time.current end + def map_model(model) + self.archive = model.archived_at.present? + end + private def user_can_close_debate diff --git a/decidim-debates/app/helpers/decidim/debates/admin/application_helper.rb b/decidim-debates/app/helpers/decidim/debates/admin/application_helper.rb index 2dbfb7caeab7..30fb1c337eac 100644 --- a/decidim-debates/app/helpers/decidim/debates/admin/application_helper.rb +++ b/decidim-debates/app/helpers/decidim/debates/admin/application_helper.rb @@ -7,6 +7,15 @@ module Admin # module ApplicationHelper include Decidim::Admin::ResourceScopeHelper + + def link_to_filtered_debates + unfiltered = params[:filter].blank? + link_to( + t("actions.#{unfiltered ? "archived" : "active"}", scope: "decidim.debates", name: t("models.debate.name", scope: "decidim.debates.admin")), + debates_path(unfiltered ? { filter: "archive" } : {}), + class: "button tiny button--title" + ) + end end end end diff --git a/decidim-debates/app/models/decidim/debates/debate.rb b/decidim-debates/app/models/decidim/debates/debate.rb index 8179485dac8d..693a5711dc0f 100644 --- a/decidim-debates/app/models/decidim/debates/debate.rb +++ b/decidim-debates/app/models/decidim/debates/debate.rb @@ -42,6 +42,8 @@ class Debate < Debates::ApplicationRecord scope :open, -> { where(closed_at: nil) } scope :closed, -> { where.not(closed_at: nil) } + scope :archived, -> { where.not(archived_at: nil) } + scope :not_archived, -> { where(archived_at: nil) } scope :authored_by, ->(author) { where(author: author) } scope :commented_by, lambda { |author| joins(:comments).where( @@ -86,6 +88,13 @@ def open? (ama? && open_ama?) || !ama? end + # Public: Checks if the debate is archived or not. + # + # Returns a boolean. + def archived? + archived_at.present? + end + # Public: Overrides the `commentable?` Commentable concern method. def commentable? component.settings.comments_enabled? @@ -157,6 +166,13 @@ def closeable_by?(user) authored_by?(user) end + # Checks whether the user can archive the debate. + # + # user - the user to check for authorship + def archivable_by?(user) + authored_by?(user) + end + # Public: Updates the comments counter cache. We have to do it these # way in order to properly calculate the counter with hidden # comments. diff --git a/decidim-debates/app/permissions/decidim/debates/admin/permissions.rb b/decidim-debates/app/permissions/decidim/debates/admin/permissions.rb index be721bbc436e..958c23f0b15f 100644 --- a/decidim-debates/app/permissions/decidim/debates/admin/permissions.rb +++ b/decidim-debates/app/permissions/decidim/debates/admin/permissions.rb @@ -12,6 +12,8 @@ def permissions case permission_action.action when :create, :read allow! + when :archive + toggle_allow(debate.present?) when :update toggle_allow(debate && !debate.closed? && debate.official?) when :delete, :close diff --git a/decidim-debates/app/views/decidim/debates/admin/debate_closes/edit.html.erb b/decidim-debates/app/views/decidim/debates/admin/debate_closes/edit.html.erb index 1bcc8c8e68c3..f6744df3607b 100644 --- a/decidim-debates/app/views/decidim/debates/admin/debate_closes/edit.html.erb +++ b/decidim-debates/app/views/decidim/debates/admin/debate_closes/edit.html.erb @@ -9,6 +9,12 @@ <%= f.translated :editor, :conclusions, autofocus: true, rows: 15 %> +
+
+ <%= f.check_box :archive, label: t(".archive") %> +

<%= t(".archive_help") %>

+
+
diff --git a/decidim-debates/app/views/decidim/debates/admin/debates/index.html.erb b/decidim-debates/app/views/decidim/debates/admin/debates/index.html.erb index 4cea3cec6e23..d6c112d7542b 100644 --- a/decidim-debates/app/views/decidim/debates/admin/debates/index.html.erb +++ b/decidim-debates/app/views/decidim/debates/admin/debates/index.html.erb @@ -4,55 +4,72 @@ <%= t(".title") %> <%= link_to t("actions.new", scope: "decidim.debates", name: t("models.debate.name", scope: "decidim.debates.admin")), new_debate_path, class: "button tiny button--title" if allowed_to? :create, :debate %> + <%= link_to_filtered_debates if allowed_to? :read, :debate %>
-
- - - - - - - <%= th_resource_scope_label %> - - - - - <% debates.each do |debate| %> - - - - - <%= td_resource_scope_for(debate.scope) %> - + + <% end %> + +
<%= t("models.debate.fields.title", scope: "decidim.debates") %><%= t("models.debate.fields.start_time", scope: "decidim.debates") %><%= t("models.debate.fields.end_time", scope: "decidim.debates") %><%= t("actions.title", scope: "decidim.debates") %>
- <%= link_to present(debate).title, resource_locator(debate).path, target: :blank %>
- 		- <% if debate.start_time %> - <%= l debate.start_time, format: :long %> - <% end %> - - <% if debate.end_time %> - <%= l debate.end_time, format: :long %> - <% end %> - - <% if allowed_to? :update, :debate, debate: debate %> - <%= icon_link_to "pencil", edit_debate_path(debate), t("actions.edit", scope: "decidim.debates"), class: "action-icon--edit" %> - <% end %> + <% if debates.any? %> +
+ + + + + + + <%= th_resource_scope_label %> + + + + + <% debates.each do |debate| %> + + + + + <%= td_resource_scope_for(debate.scope) %> + - - <% end %> - -
<%= t("models.debate.fields.title", scope: "decidim.debates") %><%= t("models.debate.fields.start_time", scope: "decidim.debates") %><%= t("models.debate.fields.end_time", scope: "decidim.debates") %><%= t("actions.title", scope: "decidim.debates") %>
+ <%= link_to present(debate).title, resource_locator(debate).path, target: :blank %>
+ 		+ <% if debate.start_time %> + <%= l debate.start_time, format: :long %> + <% end %> + + <% if debate.end_time %> + <%= l debate.end_time, format: :long %> + <% end %> + + <% if allowed_to? :update, :debate, debate: debate %> + <%= icon_link_to "pencil", edit_debate_path(debate), t("actions.edit", scope: "decidim.debates"), class: "action-icon--edit" %> + <% else %> + + <% end %> - <% if allowed_to? :close, :debate, debate: debate %> - <%= icon_link_to "lock-locked", edit_debate_debate_close_path(debate_id: debate.id, id: debate.id), t("actions.close", scope: "decidim.debates"), class: "action-icon--close" %> - <% end %> + <% if allowed_to? :close, :debate, debate: debate %> + <%= icon_link_to "lock-locked", edit_debate_debate_close_path(debate_id: debate.id, id: debate.id), t("actions.close", scope: "decidim.debates"), class: "action-icon--close #{"action-icon--highlighted" if debate.closed?}" %> + <% else %> + + <% end %> - <% if allowed_to? :delete, :debate, debate: debate %> - <%= icon_link_to "circle-x", debate_path(debate), t("actions.destroy", scope: "decidim.debates"), method: :delete, class: "action-icon--remove", data: { confirm: t("actions.confirm_destroy", scope: "decidim.debates") } %> - <% end %> -
-
+ <% if allowed_to? :archive, :debate, debate: debate %> + <%= icon_link_to "folder", archive_debate_path(id: debate.id, archive: !debate.archived?), t("actions.#{ debate.archived? ? "unarchive" : "archive" }", scope: "decidim.debates"), class: "action-icon--archive #{"action-icon--highlighted" if debate.archived?}", method: :post %> + <% else %> + + <% end %> + + <% if allowed_to? :delete, :debate, debate: debate %> + <%= icon_link_to "circle-x", debate_path(debate), t("actions.destroy", scope: "decidim.debates"), method: :delete, class: "action-icon--remove", data: { confirm: t("actions.confirm_destroy", scope: "decidim.debates") } %> + <% else %> + + <% end %> +
+
+ <% else %> + <%= t("debates.empty", scope: "decidim.debates.admin") %> + <% end %>
diff --git a/decidim-debates/config/locales/en.yml b/decidim-debates/config/locales/en.yml index 937e97b39787..fe2fb1892695 100644 --- a/decidim-debates/config/locales/en.yml +++ b/decidim-debates/config/locales/en.yml @@ -46,18 +46,27 @@ en: endorsements_enabled: Endorsements enabled debates: actions: + active: Active debates + archive: Archive + archived: Archived debates close: Close confirm_destroy: Are you sure? destroy: Delete edit: Edit new: New %{name} title: Actions + unarchive: Unarchive admin: debate_closes: edit: + archive: Archive this debate + archive_help: Archiving this debate will hide it from the public debates section close: Close title: Close debate debates: + archive: + invalid: There was a problem archiving the debate. + success: Debate successfully archived. create: invalid: There was a problem creating the debate. success: Debate successfully created. @@ -66,11 +75,15 @@ en: edit: title: Edit debate update: Update debate + empty: There are no debates matching these criteria. index: title: Debates new: create: Create debate title: New debate + unarchive: + invalid: There was a problem unarchiving the debate. + success: Debate successfully unarchived. update: invalid: There was a problem updating this debate. success: Debate successfully updated. diff --git a/decidim-debates/db/migrate/20201126112752_archive_debates.rb b/decidim-debates/db/migrate/20201126112752_archive_debates.rb new file mode 100644 index 000000000000..586039d1e3ba --- /dev/null +++ b/decidim-debates/db/migrate/20201126112752_archive_debates.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class ArchiveDebates < ActiveRecord::Migration[5.2] + def change + add_column :decidim_debates_debates, :archived_at, :datetime + add_index :decidim_debates_debates, :archived_at + end +end diff --git a/decidim-debates/lib/decidim/debates/admin_engine.rb b/decidim-debates/lib/decidim/debates/admin_engine.rb index 82f049af6f6b..bb160e038dcf 100644 --- a/decidim-debates/lib/decidim/debates/admin_engine.rb +++ b/decidim-debates/lib/decidim/debates/admin_engine.rb @@ -13,6 +13,7 @@ class AdminEngine < ::Rails::Engine routes do resources :debates do + post :archive, on: :member resources :debate_closes, only: [:edit, :update] end root to: "debates#index" diff --git a/decidim-debates/lib/decidim/debates/component.rb b/decidim-debates/lib/decidim/debates/component.rb index e1f4cfc1220d..356e67edfd48 100644 --- a/decidim-debates/lib/decidim/debates/component.rb +++ b/decidim-debates/lib/decidim/debates/component.rb @@ -83,16 +83,16 @@ Decidim::Component.create!(params) end - 3.times do + 5.times do params = { component: component, category: participatory_space.categories.sample, - title: Decidim::Faker::Localized.sentence(2), + title: Decidim::Faker::Localized.sentence(word_count: 2), description: Decidim::Faker::Localized.wrapped("

", "

") do - Decidim::Faker::Localized.paragraph(3) + Decidim::Faker::Localized.paragraph(sentence_count: 3) end, instructions: Decidim::Faker::Localized.wrapped("

", "

") do - Decidim::Faker::Localized.paragraph(3) + Decidim::Faker::Localized.paragraph(sentence_count: 3) end, start_time: 3.weeks.from_now, end_time: 3.weeks.from_now + 4.hours, @@ -109,22 +109,27 @@ Decidim::Comments::Seed.comments_for(debate) end - closed_debate = Decidim::Debates::Debate.last - closed_debate.conclusions = Decidim::Faker::Localized.wrapped("

", "

") do - Decidim::Faker::Localized.paragraph(3) + Decidim::Debates::Debate.last(2).each do |debate| + debate.conclusions = Decidim::Faker::Localized.wrapped("

", "

") do + Decidim::Faker::Localized.paragraph(sentence_count: 3) + end + debate.closed_at = Time.current + debate.save! end - closed_debate.closed_at = Time.current - closed_debate.save! + + archived_debate = Decidim::Debates::Debate.last + archived_debate.archived_at = Time.current + archived_debate.save! params = { component: component, category: participatory_space.categories.sample, - title: Decidim::Faker::Localized.sentence(2), + title: Decidim::Faker::Localized.sentence(word_count: 2), description: Decidim::Faker::Localized.wrapped("

", "

") do - Decidim::Faker::Localized.paragraph(3) + Decidim::Faker::Localized.paragraph(sentence_count: 3) end, instructions: Decidim::Faker::Localized.wrapped("

", "

") do - Decidim::Faker::Localized.paragraph(3) + Decidim::Faker::Localized.paragraph(sentence_count: 3) end, author: user } diff --git a/decidim-debates/spec/commands/decidim/debates/admin/archive_debate_spec.rb b/decidim-debates/spec/commands/decidim/debates/admin/archive_debate_spec.rb new file mode 100644 index 000000000000..fc55ee80db4b --- /dev/null +++ b/decidim-debates/spec/commands/decidim/debates/admin/archive_debate_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require "spec_helper" + +describe Decidim::Debates::Admin::ArchiveDebate do + subject { described_class.new(archive, debate, user) } + + let(:organization) { create :organization, available_locales: [:en, :ca, :es], default_locale: :en } + let(:participatory_process) { create :participatory_process, organization: organization } + let(:current_component) { create :component, participatory_space: participatory_process, manifest_name: "debates" } + let(:category) { create :category, participatory_space: participatory_process } + let(:user) { create :user, :admin, :confirmed, organization: organization } + let(:debate) { create :debate, component: current_component, archived_at: archived_at } + let(:archive) { true } + let(:archived_at) { nil } + + context "when the debate object is not valid" do + before do + debate.title = nil + end + + it "is not valid" do + expect { subject.call }.to broadcast(:invalid) + end + end + + context "when everything is ok" do + context "when the archive parameter is true" do + let(:archived_at) { nil } + let(:archive) { true } + + it "archives the debate" do + subject.call + expect(debate.archived_at).to be_present + end + end + + context "when the archive parameter is false" do + let(:archived_at) { 1.day.ago } + let(:archive) { false } + + it "unarchives the debate" do + subject.call + expect(debate.archived_at).not_to be_present + end + end + end +end diff --git a/decidim-debates/spec/models/decidim/debates/debate_spec.rb b/decidim-debates/spec/models/decidim/debates/debate_spec.rb index 1e1ee727a443..dee941e077b7 100644 --- a/decidim-debates/spec/models/decidim/debates/debate_spec.rb +++ b/decidim-debates/spec/models/decidim/debates/debate_spec.rb @@ -125,4 +125,20 @@ it { is_expected.to be_falsey } end end + + describe "archived?" do + subject { debate.archived? } + + context "when the debate has an archived_at time" do + let(:debate) { build :debate, archived_at: 2.days.ago } + + it { is_expected.to be_truthy } + end + + context "when the debate does not have an archived_at time" do + let(:debate) { build :debate, archived_at: nil } + + it { is_expected.to be_falsey } + end + end end diff --git a/decidim-debates/spec/permissions/decidim/debates/admin/permissions_spec.rb b/decidim-debates/spec/permissions/decidim/debates/admin/permissions_spec.rb index e96cb68a8a5a..781efc0b1f94 100644 --- a/decidim-debates/spec/permissions/decidim/debates/admin/permissions_spec.rb +++ b/decidim-debates/spec/permissions/decidim/debates/admin/permissions_spec.rb @@ -79,4 +79,34 @@ it { is_expected.to eq false } end end + + describe "debate archive" do + let(:action) do + { scope: :admin, action: :archive, subject: :debate } + end + + context "when the debate is closed" do + let(:debate) { create :debate, :closed, component: debates_component } + + it { is_expected.to eq true } + + context "and it is not official" do + let(:debate) { create :debate, :closed, author: user, component: debates_component } + + it { is_expected.to eq true } + end + end + + context "when debate is open" do + let(:debate) { create :debate, component: debates_component } + + it { is_expected.to eq true } + + context "and it is not official" do + let(:debate) { create :debate, author: user, component: debates_component } + + it { is_expected.to eq true } + end + end + end end diff --git a/decidim-debates/spec/shared/manage_debates_examples.rb b/decidim-debates/spec/shared/manage_debates_examples.rb index 2c0fe4f5b72d..ae538fc12138 100644 --- a/decidim-debates/spec/shared/manage_debates_examples.rb +++ b/decidim-debates/spec/shared/manage_debates_examples.rb @@ -63,7 +63,7 @@ it "creates a new debate" do within ".card-title" do - page.find(".button.button--title").click + click_link "New Debate" end within ".new_debate" do @@ -191,4 +191,46 @@ end end end + + describe "archiving a debate" do + let!(:debate) { create(:debate, :closed, component: current_component) } + + it "archives a debate" do + within find("tr", text: translated(debate.title)) do + page.find(".action-icon--archive").click + end + + within ".callout-wrapper" do + expect(page).to have_content("successfully") + end + + expect(page).to have_no_content(translated(debate.title)) + + click_link "Archived debates" + expect(page).to have_content(translated(debate.title)) + end + end + + describe "unarchiving a debate" do + let!(:debate) { create(:debate, :closed, component: current_component, archived_at: 1.day.ago) } + + before do + click_link "Archived debates" + end + + it "unarchives a debate" do + within find("tr", text: translated(debate.title)) do + page.find(".action-icon--archive").click + end + + within ".callout-wrapper" do + expect(page).to have_content("successfully") + end + + expect(page).to have_no_content(translated(debate.title)) + + click_link "Active debates" + expect(page).to have_content(translated(debate.title)) + end + end end