Skip to content

Commit

Permalink
Redirect to the petition page after success
Browse files Browse the repository at this point in the history
There's no need to show the government response page as it has no data.
  • Loading branch information
pixeltrix committed Jan 12, 2024
1 parent 99e861a commit de6b9b6
Show file tree
Hide file tree
Showing 11 changed files with 178 additions and 85 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ def destroy
@government_response.destroy

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

Expand Down
4 changes: 2 additions & 2 deletions app/controllers/admin/government_response_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ def destroy
@government_response.destroy

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

Expand Down
8 changes: 5 additions & 3 deletions app/models/archived/government_response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ class GovernmentResponse < ActiveRecord::Base
end

after_destroy do
# this will prevent EmailThresholdResponseJob from sending out emails for the deleted response
petition.set_email_requested_at_for('government_response')
# 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
Expand Down
10 changes: 7 additions & 3 deletions app/models/government_response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,13 @@ class GovernmentResponse < ActiveRecord::Base
end

after_destroy do
# this will prevent EmailThresholdResponseJob from sending out emails for the deleted response
unless petition.archived?
petition.set_email_requested_at_for('government_response')
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,11 @@
<% end %>
<%= email_petitioners_with_count_submit_button(f, petition) %>
<%= f.submit "Save", name: 'save', 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?" } %>
<% end %>
<%= link_to 'Cancel', admin_archived_petition_path(@petition), class: 'button-secondary' %>
<%= 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,12 +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? %>
<% 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?" } %>
<% end %>
<%= link_to 'Cancel', admin_petition_path(@petition), class: 'button-secondary' %>
<%= 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
4 changes: 2 additions & 2 deletions config/locales/admin.en-GB.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ 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"
government_response_destroyed: "Deleted government response successfully"
government_response_not_destroyed: "Government response was not deleted - please contact support"
failed_tasks: "There was a problem starting the tasks - please contact support"
invalidation_cant_be_cancelled: "Can't cancel invalidations that have completed"
invalidation_cant_be_counted: "Can't count invalidations that aren't pending"
Expand Down
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@
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
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
139 changes: 77 additions & 62 deletions spec/controllers/admin/government_response_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -116,68 +116,6 @@
end
end

describe 'DELETE /destroy' do
let(:government_response_attributes) do
{
responded_on: Date.civil(2022, 1, 9),
summary: 'The government disagrees',
details: 'Your petition lacks clarity and will not become law.'
}
end

def add_response(overrides = {})
login_as(user)

params = {
petition_id: petition.id,
government_response: government_response_attributes,
save: "Save"
}
patch :update, params: params.merge(overrides)
end

context 'when attempting to delete a government response that does not exist' do
it 'still responds with success' do
expect(government_response_id = petition.government_response).to be_nil
delete :destroy, params: { petition_id: petition.id }
expect(response).to have_http_status(302)
expect(flash[:notice]).to eq "Deleted government response successfully"
expect(response).to redirect_to "https://moderate.petition.parliament.uk/admin/petitions/#{petition.id}/government-response"
end
end

context 'when deleting a government response' do
it 'deletes the response and responds with success' do
add_response
government_response_id = petition.government_response.id
delete :destroy, params: { petition_id: petition.id }

expect(response).to have_http_status(302)
expect(GovernmentResponse.exists?(government_response_id)).to be_falsey
expect(flash[:notice]).to eq "Deleted government response successfully"
expect(petition.get_email_requested_at_for('government_response')).to be_within(1.second).of(Time.zone.now)
expect(response).to redirect_to "https://moderate.petition.parliament.uk/admin/petitions/#{petition.id}/government-response"
end
end

context 'when deleting a government response fails' do

let(:petition) { FactoryBot.create(:responded_petition) }
let(:government_response) { petition.government_response }
let(:petition_id) { petition.id.to_s }

before do
allow(Petition).to receive_message_chain(:moderated, :find).with(petition_id).and_return(petition)
allow(government_response).to receive(:destroy).and_return(false)
allow(government_response).to receive(:destroyed?).and_return(false)
end
it 'responds with a failure message' do
delete :destroy, params: { petition_id: petition.id }
expect(flash[:notice]).to eq "Government response was not deleted - please contact support"
end
end
end

describe 'PATCH /update' do
let(:government_response_attributes) do
{
Expand Down Expand Up @@ -704,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
2 changes: 1 addition & 1 deletion 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 "does route DELETE /admin/petitions/1/government-response" do
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 de6b9b6

Please sign in to comment.