diff --git a/app/services/request_destroy_service.rb b/app/services/request_destroy_service.rb index 09e8b33f6a..2ed41f55ee 100644 --- a/app/services/request_destroy_service.rb +++ b/app/services/request_destroy_service.rb @@ -30,6 +30,8 @@ def valid? errors.add(:base, 'request_id is invalid') elsif request.discarded_at.present? errors.add(:base, 'request already cancelled') + elsif reason.blank? + errors.add(:base, 'a cancellation reason is required') end errors.none? diff --git a/app/views/requests/cancelation/new.html.erb b/app/views/requests/cancelation/new.html.erb index e5220aa6c1..79fad7e8a5 100644 --- a/app/views/requests/cancelation/new.html.erb +++ b/app/views/requests/cancelation/new.html.erb @@ -28,9 +28,9 @@
- <%= simple_form_for :cancelation, url: request_cancelation_path(organization: @organization, request_id: @request.id), method: :post do |f| %> + <%= simple_form_for :cancelation, url: request_cancelation_path(organization: @organization, request_id: @request.id), method: :post, html: { novalidate: false } do |f| %>
- <%= f.input :reason, label: "Cancellation reason" %> + <%= f.input :reason, label: "Cancellation reason", required: true %>
This will be included in the email notification we send to the partner
Note: cancellation emails will not be sent to deactivated partners
diff --git a/spec/requests/requests_requests_spec.rb b/spec/requests/requests_requests_spec.rb index af085e1e91..f0bbac5d24 100644 --- a/spec/requests/requests_requests_spec.rb +++ b/spec/requests/requests_requests_spec.rb @@ -261,5 +261,33 @@ end end end + + describe 'POST #create for cancelation' do + let(:request) { create(:request, organization: organization) } + + context 'when a cancellation reason is given' do + it 'cancels the request and redirects to the index', :aggregate_failures do + expect do + post request_cancelation_path(request_id: request.id), params: { cancelation: { reason: 'Partner closed for the season' } } + end.to change { request.reload.status }.from('pending').to('cancelled') + + expect(request.reload.discard_reason).to eq('Partner closed for the season') + expect(flash[:notice]).to eq("Request #{request.id} has been removed!") + expect(response).to redirect_to(requests_path) + end + end + + context 'when the cancellation reason is blank' do + it 'does not cancel the request and redirects back with an error', :aggregate_failures do + expect do + post request_cancelation_path(request_id: request.id), params: { cancelation: { reason: '' } } + end.not_to change { request.reload.status } + + expect(request.reload.discarded?).to be false + expect(flash[:error]).to eq("Request #{request.id} could not be removed because a cancellation reason is required") + expect(response).to redirect_to(new_request_cancelation_path(request_id: request.id)) + end + end + end end end diff --git a/spec/services/request_destroy_service_spec.rb b/spec/services/request_destroy_service_spec.rb index 7a033133b2..ecc04ba73c 100644 --- a/spec/services/request_destroy_service_spec.rb +++ b/spec/services/request_destroy_service_spec.rb @@ -1,8 +1,9 @@ RSpec.describe RequestDestroyService, type: :service do describe '#call' do - subject { described_class.new(request_id: request_id).call } + subject { described_class.new(request_id: request_id, reason: reason).call } let(:request_id) { request.id } let(:request) { create(:request) } + let(:reason) { 'Partner no longer needs these items' } it 'should return an instance of itself' do expect(subject).to be_a_kind_of(RequestDestroyService) @@ -26,6 +27,30 @@ end end + context 'when the cancellation reason is blank' do + let(:reason) { ' ' } + + it 'should not be successful and have errors indicating a reason is required' do + expect(subject.errors.full_messages).to eq(['a cancellation reason is required']) + end + + it 'should have the same errors when no reason is given at all' do + svc = described_class.new(request_id: request_id).call + + expect(svc.errors.full_messages).to eq(['a cancellation reason is required']) + end + + it 'should not cancel the request' do + expect { subject }.not_to change { request.reload.discarded? } + expect(request.reload).to be_status_pending + end + + it 'should not send a email notification to the partner' do + expect(RequestMailer).not_to receive(:request_cancel_partner_notification) + subject + end + end + context 'when there are no validation errors' do let(:fake_mailer) { double('fake_mailer', deliver_later: -> {}) } before do @@ -40,6 +65,10 @@ expect { subject }.to change { request.reload.status_cancelled? }.from(false).to(true) end + it 'should store the cancellation reason on the request' do + expect { subject }.to change { request.reload.discard_reason }.from(nil).to(reason) + end + it 'should send a email notification to the partner' do subject expect(fake_mailer).to have_received(:deliver_later) diff --git a/spec/system/request_system_spec.rb b/spec/system/request_system_spec.rb index c1d3c936b9..ed6d191791 100644 --- a/spec/system/request_system_spec.rb +++ b/spec/system/request_system_spec.rb @@ -264,6 +264,24 @@ expect(request.reload.discard_reason).to eq(reason) end + it 'should not submit the form until a reason is given' do + click_on 'Cancel' + + # the browser blocks the submission while the required reason is empty + expect(page).to have_field('Cancellation reason *', valid: false) + + click_on 'Yes. Cancel Request' + + expect(page).to have_field('Cancellation reason *') + expect(request.reload.discarded_at).to eq(nil) + + fill_in 'Cancellation reason *', with: reason + click_on 'Yes. Cancel Request' + + expect(page).to have_content("Request #{request.id} has been removed") + expect(request.reload.discard_reason).to eq(reason) + end + it 'should show the partners name, requesters email, request date, comments' do click_on 'Cancel'