From 9d1eaee84e260f88dfd07ba053657c53caabfbb0 Mon Sep 17 00:00:00 2001 From: Arit Amana <32520970+msarit@users.noreply.github.com> Date: Mon, 4 Oct 2021 08:36:43 -0400 Subject: [PATCH] Heavy Confirmation Flow for DisplayAd-Destroy (#14862) * wham, bam, done! * refactor and remove switch statement * add guards to handleRecord function * make cypress spec more robust --- .../admin/display_ads_controller.rb | 6 +- .../confirmation_modal_controller.js | 36 +++++-- app/views/admin/display_ads/index.html.erb | 93 +++++++++++-------- .../adminFlows/displayAds/displayAds.spec.js | 87 +++++++++++++++++ spec/requests/admin/display_ads_spec.rb | 2 - spec/support/seeds/seeds_e2e.rb | 12 +++ 6 files changed, 182 insertions(+), 54 deletions(-) create mode 100644 cypress/integration/seededFlows/adminFlows/displayAds/displayAds.spec.js diff --git a/app/controllers/admin/display_ads_controller.rb b/app/controllers/admin/display_ads_controller.rb index 8eab6657c..efb4d09a5 100644 --- a/app/controllers/admin/display_ads_controller.rb +++ b/app/controllers/admin/display_ads_controller.rb @@ -51,11 +51,9 @@ module Admin @display_ad = DisplayAd.find(params[:id]) if @display_ad.destroy - flash[:success] = "Display Ad has been deleted!" - redirect_to admin_display_ads_path + render json: { message: "Display Ad has been deleted!" }, status: :ok else - flash[:danger] = "Something went wrong with deleting the Display Ad." - render :edit + render json: { error: "Something went wrong with deleting the Display Ad." }, status: :unprocessable_entity end end diff --git a/app/javascript/admin/controllers/confirmation_modal_controller.js b/app/javascript/admin/controllers/confirmation_modal_controller.js index 61348b67e..2c0f817bd 100644 --- a/app/javascript/admin/controllers/confirmation_modal_controller.js +++ b/app/javascript/admin/controllers/confirmation_modal_controller.js @@ -13,20 +13,38 @@ window.addEventListener('load', () => { } }); +const nonRedirectEndpoints = [ + '/admin/content_manager/badge_achievements', + '/admin/customization/display_ads', +]; + +const redirectEndpoints = ['/admin/advanced/broadcasts']; + export default class ConfirmationModalController extends ModalController { static targets = ['itemId', 'username', 'endpoint']; + removeRecordAsync({ id, outcome }) { + document.querySelector(`[data-row-id="${id}"]`).remove(); + displaySnackbar(outcome.message); + } + + redirectAfterDestroy({ endpoint, outcome }) { + localStorage.setItem('outcome', outcome.message); + window.location.replace(`${endpoint}?redirected`); + } + handleRecord({ endpoint, id, outcome }) { - switch (endpoint) { - case '/admin/content_manager/badge_achievements': - document.querySelector(`[data-row-id="${id}"]`).remove(); - displaySnackbar(outcome.message); - break; - case '/admin/advanced/broadcasts': - localStorage.setItem('outcome', outcome.message); - window.location.replace(`${endpoint}?redirected`); - break; + if (nonRedirectEndpoints.includes(endpoint)) { + this.removeRecordAsync({ id, outcome }); + return; } + + if (redirectEndpoints.includes(endpoint)) { + this.redirectAfterDestroy({ endpoint, outcome }); + return; + } + + displayErrorAlert('Something went wrong.'); } async sendToEndpoint({ itemId, endpoint }) { diff --git a/app/views/admin/display_ads/index.html.erb b/app/views/admin/display_ads/index.html.erb index 898418353..184c8e786 100644 --- a/app/views/admin/display_ads/index.html.erb +++ b/app/views/admin/display_ads/index.html.erb @@ -1,41 +1,56 @@ - - -<%= paginate @display_ads %> - - - - - - - - - - - - - - <% @display_ads.each do |display_ad| %> - - - - - - - - - - +
+
-
IDOrganizationPlacement AreaPublishedApprovedSuccess Rate
<%= link_to display_ad.id, edit_admin_display_ad_path(display_ad) %><%= link_to display_ad.organization.name, admin_organization_path(display_ad.organization) %><%= display_ad.human_readable_placement_area %><%= display_ad.published %><%= display_ad.approved %><%= display_ad.success_rate %><%= link_to "Edit", edit_admin_display_ad_path(display_ad), class: "crayons-btn" %><%= link_to "Destroy", admin_display_ad_path(display_ad), class: "crayons-btn crayons-btn--danger", method: :delete, data: { confirm: "Are you sure?" } %>
+
+
+ <%= link_to "Make A New Display Ad", new_admin_display_ad_path, class: "crayons-btn" %> +
+
+ -<%= paginate @display_ads %> + <%= paginate @display_ads %> + + + + + + + + + + + + + + <% @display_ads.each do |display_ad| %> + + + + + + + + + + + <% end %> + +
IDOrganizationPlacement AreaPublishedApprovedSuccess Rate
<%= link_to display_ad.id, edit_admin_display_ad_path(display_ad) %><%= link_to display_ad.organization.name, admin_organization_path(display_ad.organization) %><%= display_ad.human_readable_placement_area %><%= display_ad.published %><%= display_ad.approved %><%= display_ad.success_rate %><%= link_to "Edit", edit_admin_display_ad_path(display_ad), class: "crayons-btn" %> + +
+ <%= render partial: "admin/shared/destroy_confirmation_modal" %> + + <%= paginate @display_ads %> + diff --git a/cypress/integration/seededFlows/adminFlows/displayAds/displayAds.spec.js b/cypress/integration/seededFlows/adminFlows/displayAds/displayAds.spec.js new file mode 100644 index 000000000..979935c76 --- /dev/null +++ b/cypress/integration/seededFlows/adminFlows/displayAds/displayAds.spec.js @@ -0,0 +1,87 @@ +describe('Display Ads', () => { + beforeEach(() => { + cy.testSetup(); + cy.fixture('users/adminUser.json').as('user'); + + cy.get('@user').then((user) => { + cy.loginAndVisit(user, '/admin/customization/display_ads'); + + cy.findByRole('table').within(() => { + cy.findByRole('button', { name: 'Destroy' }).click(); + }); + }); + }); + + describe('delete a display ad', () => { + it('should display confirmation modal', () => { + cy.findByRole('dialog').contains('Confirm changes').should('be.visible'); + }); + + it('should display warning text if confirmation text does not match', () => { + cy.findByRole('dialog').within(() => { + cy.get('input').type('Text that does not match.'); + cy.findByRole('button', { name: 'Confirm changes' }).click(); + + cy.get('.crayons-notice') + .contains('The confirmation text did not match.') + .should('be.visible'); + + cy.findByRole('button', { name: 'Close' }).click(); + }); + + cy.findByRole('table').within(() => { + cy.findByRole('button', { name: 'Destroy' }).should('be.visible'); + }); + }); + + it('should remove display ad if confirmation text matches', () => { + cy.get('@user').then((user) => { + cy.findByRole('dialog').within(() => { + cy.get('input').type( + `My username is @${user.username} and this action is 100% safe and appropriate.`, + ); + cy.findByRole('button', { name: 'Confirm changes' }).click(); + }); + + cy.findByTestId('snackbar').within(() => { + cy.findByRole('alert').should( + 'have.text', + 'Display Ad has been deleted!', + ); + }); + + cy.findByRole('table').within(() => { + cy.findByRole('button', { name: 'Destroy' }).should('not.exist'); + }); + }); + }); + + it('generates error message when remove action fails', () => { + cy.intercept('DELETE', '/admin/customization/display_ads/**', { + statusCode: 422, + body: { + error: 'Something went wrong with deleting the Display Ad.', + }, + }); + + cy.get('@user').then((user) => { + cy.findByRole('dialog').within(() => { + cy.get('input').type( + `My username is @${user.username} and this action is 100% safe and appropriate.`, + ); + cy.findByRole('button', { name: 'Confirm changes' }).click(); + }); + + cy.findByTestId('alertzone').within(() => { + cy.findByRole('alert') + .contains('Something went wrong with deleting the Display Ad.') + .should('be.visible'); + }); + + cy.findByRole('table').within(() => { + cy.findByRole('button', { name: 'Destroy' }).should('be.visible'); + }); + }); + }); + }); +}); diff --git a/spec/requests/admin/display_ads_spec.rb b/spec/requests/admin/display_ads_spec.rb index 617e597b2..9b5e207e4 100644 --- a/spec/requests/admin/display_ads_spec.rb +++ b/spec/requests/admin/display_ads_spec.rb @@ -77,7 +77,6 @@ RSpec.describe "/admin/customization/display_ads", type: :request do expect do delete admin_display_ad_path(display_ad.id) end.to change { DisplayAd.all.count }.by(-1) - expect(response.body).to redirect_to admin_display_ads_path end end end @@ -121,7 +120,6 @@ RSpec.describe "/admin/customization/display_ads", type: :request do expect do delete admin_display_ad_path(display_ad.id) end.to change { DisplayAd.all.count }.by(-1) - expect(response.body).to redirect_to admin_display_ads_path end end end diff --git a/spec/support/seeds/seeds_e2e.rb b/spec/support/seeds/seeds_e2e.rb index 2de5362a2..67933c7dd 100644 --- a/spec/support/seeds/seeds_e2e.rb +++ b/spec/support/seeds/seeds_e2e.rb @@ -622,3 +622,15 @@ seeder.create_if_none(Broadcast) do active: true, ) end + +############################################################################## + +seeder.create_if_none(DisplayAd) do + DisplayAd.create!( + organization_id: 1, + body_markdown: "

This is an add

", + placement_area: "sidebar_left", + published: true, + approved: true, + ) +end