diff --git a/app/components/button/button.css b/app/components/button/button.css index 5a0a95338..f6fd58d98 100644 --- a/app/components/button/button.css +++ b/app/components/button/button.css @@ -118,6 +118,16 @@ --text-color: var(--color-orange-darkest); } +.Button--destroy { + --background-color: var(--color-red-lightest); + --border-color: var(--color-red); + --text-color: var(--color-red-dark); +} + +.Button--destroy:focus { + --color-focus: var(--color-red-light); +} + .Button:disabled { background-color: var(--color-gray); pointer-events: none; diff --git a/app/components/destroy_planned_request_modal/destroy_planned_request_modal.css b/app/components/destroy_planned_request_modal/destroy_planned_request_modal.css new file mode 100644 index 000000000..2e7ce3214 --- /dev/null +++ b/app/components/destroy_planned_request_modal/destroy_planned_request_modal.css @@ -0,0 +1,12 @@ +.DestroyPlannedRequestModal-footer { + display: flex; + margin-top: var(--spacing-unit); +} + +.DestroyPlannedRequestModal-footer .Button:first-child { + margin-right: var(--spacing-unit-xs); +} + +.DestroyPlannedRequestModal-footer .Button:last-child { + margin-left: var(--spacing-unit-xs); +} diff --git a/app/components/destroy_planned_request_modal/destroy_planned_request_modal.html.erb b/app/components/destroy_planned_request_modal/destroy_planned_request_modal.html.erb new file mode 100644 index 000000000..e42ad23f9 --- /dev/null +++ b/app/components/destroy_planned_request_modal/destroy_planned_request_modal.html.erb @@ -0,0 +1,19 @@ +<%= c 'modal', **attrs do %> + <%= c 'heading', tag: :h2 do %> + <%= t('.heading', request_title: planned_request.title) %> + <% end %> + +<% end %> diff --git a/app/components/destroy_planned_request_modal/destroy_planned_request_modal.rb b/app/components/destroy_planned_request_modal/destroy_planned_request_modal.rb new file mode 100644 index 000000000..e22bd1538 --- /dev/null +++ b/app/components/destroy_planned_request_modal/destroy_planned_request_modal.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module DestroyPlannedRequestModal + class DestroyPlannedRequestModal < ApplicationComponent + def initialize(planned_request:, **) + super + + @planned_request = planned_request + end + + attr_reader :planned_request + end +end diff --git a/app/components/profile_header/profile_header.js b/app/components/profile_header/profile_header.js index db9205e98..f0773d92a 100644 --- a/app/components/profile_header/profile_header.js +++ b/app/components/profile_header/profile_header.js @@ -8,7 +8,6 @@ export default class extends Controller { } closeModal() { - console.log('im closing...'); this.modalTarget.close(); } } diff --git a/app/components/request_form/request_form.html.erb b/app/components/request_form/request_form.html.erb index f4d4e14a2..53851737c 100644 --- a/app/components/request_form/request_form.html.erb +++ b/app/components/request_form/request_form.html.erb @@ -79,6 +79,16 @@ <% end %> <% end %> + <% if @request.planned? %> + <%= c 'button', + type: 'button', + styles: [:block, :destroy], + data: { action: 'request-form#openModal' } do %> + <%= t('.planned_request.destroy.button_text') %> + <% end%> + <%= c 'destroy_planned_request_modal', planned_request: @request, data: { controller: 'modal', request_form_target: 'modal' } %> + <% end %> + <%= c 'button', type: 'submit', styles: [:block, :primary], diff --git a/app/components/request_form/request_form.js b/app/components/request_form/request_form.js index 7751af2b4..404414c63 100644 --- a/app/components/request_form/request_form.js +++ b/app/components/request_form/request_form.js @@ -13,6 +13,7 @@ export default class extends Controller { 'filenames', 'submitButton', 'characterCounter', + 'modal', ]; static values = { membersCountMessage: String, @@ -217,4 +218,12 @@ export default class extends Controller { ? this.submitButtonTarget.setAttribute('disabled', isInvalid) : this.submitButtonTarget.removeAttribute('disabled'); } + + openModal() { + this.modalTarget.showModal(); + } + + closeModal() { + this.modalTarget.close(); + } } diff --git a/app/components/request_row/request_row.css b/app/components/request_row/request_row.css index d45585af6..edcae325d 100644 --- a/app/components/request_row/request_row.css +++ b/app/components/request_row/request_row.css @@ -26,6 +26,11 @@ justify-content: space-between; } +.RequestRow-editableWrapper .Icon:first-of-type { + margin-left: auto; + margin-right: var(--spacing-unit-s); +} + .RequestRow:target { border-color: var(--color-focus); box-shadow: var(--input-focus-shadow); diff --git a/app/components/request_row/request_row.html.erb b/app/components/request_row/request_row.html.erb index 43e629e61..24e512503 100644 --- a/app/components/request_row/request_row.html.erb +++ b/app/components/request_row/request_row.html.erb @@ -10,6 +10,7 @@ <%= request.title %> <% end %> <%= c 'icon', icon: 'pen', styles: [:inline] %> + <%= c 'icon', icon: 'bin', styles: [:inline] %> <% else %> <%= c 'heading', tag: :h2 do %> diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index 5c903b865..e6da640b7 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -1,10 +1,11 @@ # frozen_string_literal: true class RequestsController < ApplicationController - before_action :set_request, only: %i[show show_contributor_messages edit update notifications] + before_action :set_request, only: %i[show show_contributor_messages edit update notifications destroy] before_action :set_contributor, only: %i[show_contributor_messages] before_action :notifications_params, only: :notifications before_action :disallow_edit, only: %i[edit update] + before_action :disallow_destroy, only: :destroy def index @filter = filter_param @@ -57,6 +58,14 @@ def update end end + def destroy + if @request.destroy + redirect_to requests_url(filter: :planned), notice: t('request.destroy.successful', request_title: @request.title) + else + render :edit, status: :unprocessable_entity + end + end + def show_contributor_messages @chat_messages = @contributor.conversation_about(@request) end @@ -102,6 +111,12 @@ def disallow_edit redirect_to requests_path, flash: { error: I18n.t('request.editing_disallowed') } end + def disallow_destroy + return if @request.planned? + + redirect_to requests_path, flash: { error: I18n.t('request.destroy.broadcasted_request_unallowed', request_title: @request.title) } + end + def filter_param value = params.permit(:filter)[:filter]&.to_sym diff --git a/app/jobs/broadcast_request_job.rb b/app/jobs/broadcast_request_job.rb index ad6f37e4b..3de2debda 100644 --- a/app/jobs/broadcast_request_job.rb +++ b/app/jobs/broadcast_request_job.rb @@ -4,7 +4,8 @@ class BroadcastRequestJob < ApplicationJob queue_as :broadcast_request def perform(request_id) - request = Request.find(request_id) + request = Request.where(id: request_id).first + return unless request return if request.broadcasted_at.present? if request.planned? # rescheduled for future after this job was created diff --git a/config/locales/de.yml b/config/locales/de.yml index 2e63c9d8a..3e871bfb8 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -135,6 +135,9 @@ de: insert_placeholder_button: Vornamen einfügen attach_image_to_message: Bilder anhängen attached_images: Angehängte Bilder + planned_request: + destroy: + button_text: Diese geplante Frage löschen chat_preview: status: jetzt online alt: > @@ -382,6 +385,10 @@ de: onboarding_channels_checkboxes: legend: Aktive Onboarding-Channel help_text: Achtung, hier deaktivierst du Kanäle. Mitglieder können sich nur noch auf aktiven Kanälen anmelden. + destroy_planned_request_modal: + heading: "Bist du sicher, dass du „%{request_title}” dauerhaft löschen möchtest?" + cancel: abbrechen + confirm: löschen mailer: unsubscribe: @@ -600,6 +607,9 @@ de: one: Deine Frage wurde erfolgreich an ein Mitglied in der Community gesendet other: Deine Frage wurde erfolgreich an %{count} Mitglieder in der Community gesendet editing_disallowed: Sie können eine bereits verschickte Frage nicht mehr bearbeiten. + destroy: + successful: "Deine Frage „%{request_title}” wurde erfolgreich gelöscht" + broadcasted_request_unallowed: "Frage %{request_title} wurde bereits gesendet und kann nicht gelöscht werden" replies: Alle Antworten created_at: Frage gestellt %{datetime} planned_for: Frage geplant für %{datetime} diff --git a/config/routes.rb b/config/routes.rb index fe4cf5af6..9c5b59267 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -54,7 +54,7 @@ telegram_webhook Telegram::WebhookController - resources :requests, only: %i[index show new create edit update], concerns: :paginatable do + resources :requests, only: %i[index show new create edit update destroy], concerns: :paginatable do member do get 'notifications', format: /json/ end diff --git a/public/icons.svg b/public/icons.svg index 96c949f44..9db10e4df 100644 --- a/public/icons.svg +++ b/public/icons.svg @@ -24,6 +24,7 @@ menu 8 filter tool + bin diff --git a/spec/components/request_form_spec.rb b/spec/components/request_form_spec.rb index 5c5a6b0a1..543f9fb35 100644 --- a/spec/components/request_form_spec.rb +++ b/spec/components/request_form_spec.rb @@ -7,4 +7,21 @@ let(:params) { { request: build(:request) } } it { should have_css('.RequestForm') } + it { + is_expected.not_to have_css('button[data-action="request-form#openModal"]', + text: I18n.t('components.request_form.planned_request.destroy.button_text')) + } + + context 'planned request' do + let(:params) { { request: create(:request, broadcasted_at: nil, schedule_send_for: 1.day.from_now) } } + + it 'renders a button to open a confirm destroy modal' do + expect(subject).to have_css('button[data-action="request-form#openModal"]', + text: I18n.t('components.request_form.planned_request.destroy.button_text')) + end + + it 'renders a destroy planned request modal' do + expect(subject).to have_css('.DestroyPlannedRequestModal') + end + end end diff --git a/spec/jobs/broadcast_request_job_spec.rb b/spec/jobs/broadcast_request_job_spec.rb new file mode 100644 index 000000000..7137f7fbf --- /dev/null +++ b/spec/jobs/broadcast_request_job_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe BroadcastRequestJob do + describe '#perform_later(request_id)' do + subject { -> { described_class.new.perform(request.id) } } + + let!(:contributor) { create(:contributor) } + let(:request) { create(:request, broadcasted_at: nil) } + + context 'given the request has been deleted' do + before { request.destroy } + + it 'does not raise an error' do + expect { subject.call }.not_to raise_error + end + + it 'does not create a Message instance' do + expect { subject.call }.not_to change(Message, :count) + end + end + + context 'given a request has been broadcast' do + before { request.update(broadcasted_at: 5.minutes.ago) } + + it 'does not create a Message instance' do + expect { subject.call }.not_to change(Message, :count) + end + + it 'does not update the broadcasted_at attr' do + expect { subject.call }.not_to(change { request.reload.broadcasted_at }) + end + end + + context 'given a request has been rescheduled for the future' do + before { request.update(schedule_send_for: 1.day.from_now) } + let(:expected_params) { { request_id: request.id } } + + it 'enqueues a job to broadcast the request, and broadcast it when called again' do + expect { subject.call }.to change(DelayedJob, :count).from(0).to(1) + expect(Delayed::Job.last.run_at).to be_within(1.second).of(request.schedule_send_for) + + Timecop.travel(1.day.from_now + 2.minutes) + + expect { subject.call }.to change { request.reload.broadcasted_at }.from(nil).to(kind_of(ActiveSupport::TimeWithZone)) + end + + it 'does not create a Message instance' do + expect { subject.call }.not_to change(Message, :count) + end + + it 'does not update the broadcasted_at attr' do + expect { subject.call }.not_to(change { request.reload.broadcasted_at }) + end + end + end +end diff --git a/spec/requests/requests_spec.rb b/spec/requests/requests_spec.rb index e8a529c09..cd309e86c 100644 --- a/spec/requests/requests_spec.rb +++ b/spec/requests/requests_spec.rb @@ -88,6 +88,52 @@ end end + describe 'DELETE /requests/:id' do + subject { -> { delete "/requests/#{request.id}?as=#{user.id}" } } + + let(:user) { create(:user) } + + context 'broadcasted request' do + let!(:request) { create(:request) } + + it 'does not delete the request' do + expect { subject.call }.not_to change(Request, :count) + end + + it 'redirects to requests path' do + subject.call + + expect(response).to redirect_to requests_path + end + + it 'shows error message' do + subject.call + + expect(flash[:error]).to eq(I18n.t('request.destroy.broadcasted_request_unallowed', request_title: request.title)) + end + end + + context 'planned request' do + let!(:request) { create(:request, broadcasted_at: nil, schedule_send_for: 1.day.from_now) } + + it 'deletes the request' do + expect { subject.call }.to change(Request, :count).from(1).to(0) + end + + it 'redirects to requests path with planned filter' do + subject.call + + expect(response).to redirect_to requests_path(filter: :planned) + end + + it 'shows a notice that it was successful' do + subject.call + + expect(flash[:notice]).to eq(I18n.t('request.destroy.successful', request_title: request.title)) + end + end + end + describe 'GET /notifications' do let(:request) { create(:request) } let!(:older_message) { create(:message, request_id: request.id, created_at: 2.minutes.ago) } diff --git a/spec/system/requests/deleting_requests_spec.rb b/spec/system/requests/deleting_requests_spec.rb new file mode 100644 index 000000000..35b3d1709 --- /dev/null +++ b/spec/system/requests/deleting_requests_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Deleting requests' do + let(:user) { create(:user) } + let!(:broadcasted_request) { create(:request) } + let!(:planned_request) { create(:request, schedule_send_for: 1.hour.from_now) } + let!(:another_planned_request) { create(:request, schedule_send_for: 5.minutes.from_now) } + + before do + allow(Request).to receive(:broadcast!).and_call_original + create(:contributor) + end + + it 'conditonally allows deleting' do + # Broadcasted request + + visit requests_path(as: user) + within("#request-#{broadcasted_request.id}") do + expect(page).to have_link(href: request_path(broadcasted_request)) + expect(page).not_to have_link(href: edit_request_path(broadcasted_request)) + + find_link(href: request_path(broadcasted_request)).click + end + + expect(page).to have_current_path(request_path(broadcasted_request)) + visit edit_request_path(broadcasted_request, as: user) + expect(page).to have_content('Sie können eine bereits verschickte Frage nicht mehr bearbeiten.') + expect(page).to have_current_path(requests_path) + + # Planned request + + visit requests_path(as: user, filter: :planned) + within("#request-#{planned_request.id}") do + find_link(href: edit_request_path(planned_request)).click + end + + expect(page).to have_current_path(edit_request_path(planned_request)) + + click_on I18n.t('components.request_form.planned_request.destroy.button_text') + expect(page).to have_content(I18n.t('components.destroy_planned_request_modal.heading', request_title: planned_request.title)) + click_on 'löschen' + + expect(page).to have_content(I18n.t('request.destroy.successful', request_title: planned_request.title)) + expect(page).to have_current_path(requests_path(filter: :planned)) + + # Planned request, that was then sent out + + visit requests_path(as: user, filter: :planned) + within("#request-#{another_planned_request.id}") do + find_link(href: edit_request_path(another_planned_request)).click + end + + expect(page).to have_current_path(edit_request_path(another_planned_request)) + Timecop.travel(10.minutes.from_now) + another_planned_request.update(broadcasted_at: 5.minutes.ago) + + click_on I18n.t('components.request_form.planned_request.destroy.button_text') + expect(page).to have_content(I18n.t('components.request_form.planned_request.destroy.button_text', + request_title: another_planned_request.title)) + click_on 'löschen' + + expect(page).to have_content(I18n.t('request.destroy.broadcasted_request_unallowed', request_title: another_planned_request.title)) + expect(page).to have_current_path(requests_path) + end +end