Skip to content

Commit

Permalink
Allow deletion of government responses (#888)
Browse files Browse the repository at this point in the history
Sometimes a response is posted and the department then changes its mind.
  • Loading branch information
thatkevin authored Jan 12, 2024
1 parent 49f553a commit 4be6049
Show file tree
Hide file tree
Showing 13 changed files with 221 additions and 12 deletions.
10 changes: 10 additions & 0 deletions app/controllers/admin/archived/government_response_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,16 @@ def update
end
end

def destroy
@government_response.destroy

if @government_response.destroyed?
redirect_to admin_archived_petition_path(@petition), notice: :government_response_deleted
else
redirect_to admin_archived_petition_government_response_path(@petition), alert: :government_response_not_deleted
end
end

private

def fetch_petition
Expand Down
10 changes: 10 additions & 0 deletions app/controllers/admin/government_response_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,16 @@ def update
end
end

def destroy
@government_response.destroy

if @government_response.destroyed?
redirect_to admin_petition_path(@petition), notice: :government_response_deleted
else
redirect_to admin_petition_government_response_path(@petition), alert: :government_response_not_deleted
end
end

private

def fetch_petition
Expand Down
8 changes: 8 additions & 0 deletions app/models/archived/government_response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@ class GovernmentResponse < ActiveRecord::Base
petition.touch(:government_response_at) unless petition.government_response_at?
end

after_destroy do
# This prevents any enqueued email jobs from being sent
petition.set_email_requested_at_for("government_response")

# This removes the petition from the 'Government response' list
petition.update_columns(government_response_at: nil)
end

def responded_on
super || default_responded_on
end
Expand Down
12 changes: 12 additions & 0 deletions app/models/government_response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,18 @@ class GovernmentResponse < ActiveRecord::Base
petition.touch(:government_response_at) unless petition.government_response_at?
end

after_destroy do
unless petition.archived?
Appsignal.increment_counter("petition.responded", -1)

# This prevents any enqueued email jobs from being sent
petition.set_email_requested_at_for("government_response")

# This removes the petition from the 'Government response' list
petition.update_columns(government_response_at: nil)
end
end

def responded_on
super || default_responded_on
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,11 @@
<% end %>
<%= email_petitioners_with_count_submit_button(f, petition) %>
<%= f.submit "Save", name: 'save', class: 'button-secondary' %>
<%= link_to 'Cancel', admin_archived_petition_path(@petition), class: 'button-secondary' %>
<% if @government_response.persisted? %>
<%= link_to "Delete", admin_archived_petition_government_response_path(@petition), class: "button-warning", method: :delete, data: { confirm: "Are you sure you want to delete the government response?" } %>
<% end %>
<%= f.submit "Save", name: "save", class: "button-secondary" %>
<%= link_to "Cancel", admin_archived_petition_path(@petition), class: "button-secondary" %>
<% end %>
<%= javascript_include_tag 'character-counter' %>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,11 @@
<% end %>
<%= email_petitioners_with_count_submit_button(f, petition, disabled: @petition.editing_disabled?) %>
<%= f.submit "Save", name: 'save', class: 'button-secondary', disabled: @petition.editing_disabled? %>
<%= link_to 'Cancel', admin_petition_path(@petition), class: 'button-secondary' %>
<% if @government_response.persisted? %>
<%= link_to "Delete", admin_petition_government_response_path(@petition), class: "button-warning", method: :delete, data: { confirm: "Are you sure you want to delete the government response?" } %>
<% end %>
<%= f.submit "Save", name: "save", class: "button-secondary", disabled: @petition.editing_disabled? %>
<%= link_to "Cancel", admin_petition_path(@petition), class: "button-secondary" %>
<% end %>
<%= javascript_include_tag 'character-counter' %>
Expand Down
2 changes: 2 additions & 0 deletions config/locales/admin.en-GB.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ en-GB:
email_resent_to_creator: "Resent the email to the petition creator and forwarded a copy to the feedback address"
email_sent_overnight: "Email will be sent overnight"
enqueued_petition_statistics_update: "Updating the petition statistics - please wait a few minutes and then refresh this page"
government_response_deleted: "Deleted government response successfully"
government_response_not_deleted: "Unable to delete government response - please contact support"
government_response_updated: "Updated government response successfully"
failed_tasks: "There was a problem starting the tasks - please contact support"
invalidation_cant_be_cancelled: "Can't cancel invalidations that have completed"
Expand Down
10 changes: 8 additions & 2 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@

scope only: %i[show update] do
resource :debate_outcome, path: 'debate-outcome'
resource :government_response, path: 'government-response', controller: 'government_response'
resource :notes
resource :details, controller: 'petition_details'
resource :schedule_debate, path: 'schedule-debate', controller: 'schedule_debate'
Expand All @@ -163,6 +162,10 @@
resource :removal, controller: 'petition_removals'
end

scope only: %i[show update destroy] do
resource :government_response, path: 'government-response', controller: 'government_response'
end

resources :signatures, only: %i[index destroy] do
post :validate, :invalidate, on: :member
post :subscribe, :unsubscribe, on: :member
Expand Down Expand Up @@ -213,7 +216,6 @@

scope only: %i[show update] do
resource :debate_outcome, path: 'debate-outcome'
resource :government_response, path: 'government-response', controller: 'government_response'
resource :notes
resource :details, controller: 'petition_details'
resource :schedule_debate, path: 'schedule-debate', controller: 'schedule_debate'
Expand All @@ -222,6 +224,10 @@
resource :topics, controller: 'petition_topics'
resource :removal, controller: 'petition_removals'
end

scope only: %i[show update destroy] do
resource :government_response, path: 'government-response', controller: 'government_response'
end
end

resources :signatures, only: %i[index destroy] do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -610,5 +610,78 @@ def do_patch(overrides = {})
end
end
end

describe 'DELETE /destroy' do
before do
expect(Archived::Petition).to receive_message_chain(:moderated, :find).with(petition.to_param).and_return(petition)
end

context "when the petition has a government response" do
let!(:petition) { FactoryBot.create(:archived_petition, :response) }

before do
expect(petition).to receive(:government_response).and_return(government_response)
end

context "when the government response is succcessfully deleted" do
it "redirects to the petition page and displays a notice" do
delete :destroy, params: { petition_id: petition.id }

expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/archived/petitions/#{petition.id}")
expect(controller).to set_flash[:notice].to("Deleted government response successfully")
end

it "updates the email_requested_at timestamp for 'government_response'" do
email_requested_at = 1.hour.ago
petition.set_email_requested_at_for("government_response", to: email_requested_at)

expect {
delete :destroy, params: { petition_id: petition.id }
}.to change {
petition.get_email_requested_at_for("government_response")
}.from(email_requested_at).to(be_within(1.second).of(Time.current))
end

it "updates the government_response_at timestamp to be nil" do
expect {
delete :destroy, params: { petition_id: petition.id }
}.to change {
petition.government_response_at
}.from(be_present).to(be_nil)
end
end

context "when the government response is not successfully deleted" do
before do
expect(government_response).to receive(:destroy).and_return(false)
expect(government_response).to receive(:destroyed?).and_return(false)
end

it "redirects to the government response page and displays an alert" do
delete :destroy, params: { petition_id: petition.id }

expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/archived/petitions/#{petition.id}/government-response")
expect(controller).to set_flash[:alert].to("Unable to delete government response - please contact support")
end
end
end

context "when the petition has no government response" do
let!(:petition) { FactoryBot.create(:archived_petition) }
let(:new_government_response) { petition.build_government_response }

before do
expect(petition).to receive(:government_response).and_return(nil)
expect(petition).to receive(:build_government_response).and_return(new_government_response)
end

it "redirects to the petition page and displays a notice" do
delete :destroy, params: { petition_id: petition.id }

expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/archived/petitions/#{petition.id}")
expect(controller).to set_flash[:notice].to("Deleted government response successfully")
end
end
end
end
end
77 changes: 77 additions & 0 deletions spec/controllers/admin/government_response_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -642,5 +642,82 @@ def do_patch(overrides = {})
end
end
end

describe 'DELETE /destroy' do
before do
expect(Petition).to receive_message_chain(:moderated, :find).with(petition.to_param).and_return(petition)
end

context "when the petition has a government response" do
let!(:petition) { FactoryBot.create(:responded_petition) }

before do
expect(petition).to receive(:government_response).and_return(government_response)
end

context "when the government response is succcessfully deleted" do
before do
expect(Appsignal).to receive(:increment_counter).with("petition.responded", -1)
end

it "redirects to the petition page and displays a notice" do
delete :destroy, params: { petition_id: petition.id }

expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/petitions/#{petition.id}")
expect(controller).to set_flash[:notice].to("Deleted government response successfully")
end

it "updates the email_requested_at timestamp for 'government_response'" do
email_requested_at = 1.hour.ago
petition.set_email_requested_at_for("government_response", to: email_requested_at)

expect {
delete :destroy, params: { petition_id: petition.id }
}.to change {
petition.get_email_requested_at_for("government_response")
}.from(email_requested_at).to(be_within(1.second).of(Time.current))
end

it "updates the government_response_at timestamp to be nil" do
expect {
delete :destroy, params: { petition_id: petition.id }
}.to change {
petition.government_response_at
}.from(be_present).to(be_nil)
end
end

context "when the government response is not successfully deleted" do
before do
expect(government_response).to receive(:destroy).and_return(false)
expect(government_response).to receive(:destroyed?).and_return(false)
end

it "redirects to the government response page and displays an alert" do
delete :destroy, params: { petition_id: petition.id }

expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/petitions/#{petition.id}/government-response")
expect(controller).to set_flash[:alert].to("Unable to delete government response - please contact support")
end
end
end

context "when the petition has no government response" do
let!(:petition) { FactoryBot.create(:open_petition) }
let(:new_government_response) { petition.build_government_response }

before do
expect(petition).to receive(:government_response).and_return(nil)
expect(petition).to receive(:build_government_response).and_return(new_government_response)
end

it "redirects to the petition page and displays a notice" do
delete :destroy, params: { petition_id: petition.id }

expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/petitions/#{petition.id}")
expect(controller).to set_flash[:notice].to("Deleted government response successfully")
end
end
end
end
end
5 changes: 5 additions & 0 deletions spec/factories.rb
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,11 @@
end
end

trait :archived do
archived_at { 3.day.ago }
archiving_started_at { 4.days.ago }
end

trait :with_additional_details do
additional_details { "Petition additional details" }
end
Expand Down
8 changes: 4 additions & 4 deletions spec/jobs/delete_petition_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
end

context "with a stopped petition" do
let!(:petition) { FactoryBot.create(:stopped_petition) }
let!(:petition) { FactoryBot.create(:stopped_petition, :archived) }

it "destroys the petition" do
expect {
Expand All @@ -19,7 +19,7 @@
end

context "with a closed petition" do
let!(:petition) { FactoryBot.create(:validated_petition, sponsors_signed: true, state: "closed", closed_at: 4.weeks.ago) }
let!(:petition) { FactoryBot.create(:closed_petition, :archived, sponsors_signed: true, closed_at: 4.weeks.ago) }
let!(:country_petition_journal) { FactoryBot.create(:country_petition_journal, petition: petition) }
let!(:constituency_petition_journal) { FactoryBot.create(:constituency_petition_journal, petition: petition) }

Expand Down Expand Up @@ -136,7 +136,7 @@
end

context "with a rejected petition" do
let!(:petition) { FactoryBot.create(:rejected_petition) }
let!(:petition) { FactoryBot.create(:rejected_petition, :archived) }

it "destroys the petition" do
expect {
Expand All @@ -157,7 +157,7 @@

context "with a hidden petition" do
let!(:user) { FactoryBot.create(:moderator_user) }
let!(:petition) { FactoryBot.create(:rejected_petition, rejection_code: "libellous", moderated_by: user) }
let!(:petition) { FactoryBot.create(:rejected_petition, :archived, rejection_code: "libellous", moderated_by: user) }

it "destroys the petition" do
expect {
Expand Down
4 changes: 2 additions & 2 deletions spec/routing/admin/petitions/government_response_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
expect(patch("/admin/petitions/1/government-response")).to route_to('admin/government_response#update', petition_id: '1')
end

it "doesn't route DELETE /admin/petitions/1/government-response" do
expect(delete("/admin/petitions/1/government-response")).not_to be_routable
it "routes DELETE /admin/petitions/1/government-response to admin/government_response#destroy" do
expect(delete("/admin/petitions/1/government-response")).to route_to('admin/government_response#destroy', petition_id: '1')
end
end

0 comments on commit 4be6049

Please sign in to comment.