From 31b1c59fccbe3e383fb792ac013aade001011eb9 Mon Sep 17 00:00:00 2001 From: William Valencia Date: Fri, 10 Jul 2020 04:08:58 -0400 Subject: [PATCH 1/7] MMT-2229 Adding delete for published tools --- .../change_current_provider.coffee | 2 + app/controllers/tools_controller.rb | 6 +- app/views/tools/show.html.erb | 17 ++-- config/locales/en.yml | 4 + config/routes.rb | 2 +- spec/features/tools/delete_tool_spec.rb | 96 +++++++++++++++++++ spec/features/tools/tool_permissions_spec.rb | 88 +++++++++-------- 7 files changed, 162 insertions(+), 53 deletions(-) create mode 100644 spec/features/tools/delete_tool_spec.rb diff --git a/app/assets/javascripts/change_current_provider.coffee b/app/assets/javascripts/change_current_provider.coffee index 321d72a24..08298d76b 100644 --- a/app/assets/javascripts/change_current_provider.coffee +++ b/app/assets/javascripts/change_current_provider.coffee @@ -76,6 +76,8 @@ $(document).ready -> 'Editing this tool' when 'clone-tool' 'Cloning this tool' + when 'delete-tool' + 'Deleting this tool' $link.data('type', action) $modal.find('span.provider').text(provider) diff --git a/app/controllers/tools_controller.rb b/app/controllers/tools_controller.rb index 2c5686dbe..5819480be 100644 --- a/app/controllers/tools_controller.rb +++ b/app/controllers/tools_controller.rb @@ -2,10 +2,10 @@ class ToolsController < BasePublishedRecordController include ManageMetadataHelper - before_action :set_tool, only: [:show, :edit, :clone, :revisions] #, :destroy, :revert, :download_json] - before_action :set_schema, only: [:show, :edit, :clone] #, :destroy] + before_action :set_tool, only: [:show, :edit, :clone, :destroy, :revisions] #, :revert, :download_json] + before_action :set_schema, only: [:show, :edit, :clone, :destroy] before_action :ensure_supported_version, only: [:show, :edit] - before_action :ensure_correct_provider, only: [:edit, :clone] #, :destroy] + before_action :ensure_correct_provider, only: [:edit, :clone, :destroy] before_action :set_preview, only: [:show] # If clone is not defined like this performing the clone action leads to a `action not found error` diff --git a/app/views/tools/show.html.erb b/app/views/tools/show.html.erb index 14240393e..19ed43cde 100644 --- a/app/views/tools/show.html.erb +++ b/app/views/tools/show.html.erb @@ -101,22 +101,21 @@ <%= link_to 'Download JSON', '#', class: 'eui-btn--link disabled' %> <%#= link_to 'Download JSON', download_json_tool_path(@concept_id, revision_id: @revision_id), class: 'eui-btn--link', target: '_blank' %> - <%= link_to 'Delete Tool Record', '#', class: 'display-modal delete-tool eui-btn--link bar-before disabled' %> - <%# if current_provider?(@provider_id) %> - <%#= link_to 'Delete Tool Record', "#delete-record-modal", class: 'display-modal delete-tool eui-btn--link bar-before' %> - <%# elsif available_provider?(@provider_id) %> - <%#= link_to 'Delete Tool Record', '#not-current-provider-modal', class: 'display-modal not-current-provider eui-btn--link bar-before', data: { 'provider': @provider_id, record_action: 'delete-tool', num_associated_collections: "#{@num_associated_collections}" } %> - <%# end %> + <% if current_provider?(@provider_id) %> + <%= link_to 'Delete Tool Record', "#delete-record-modal", class: 'display-modal delete-tool eui-btn--link bar-before' %> + <% elsif available_provider?(@provider_id) %> + <%= link_to 'Delete Tool Record', '#not-current-provider-modal', class: 'display-modal not-current-provider eui-btn--link bar-before', data: { 'provider': @provider_id, record_action: 'delete-tool' } %> + <% end %> - + <%= render partial: 'shared/not_current_provider_modal', locals: { options: { tool: @tool, diff --git a/config/locales/en.yml b/config/locales/en.yml index d420d9298..3ef4ec0cb 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -198,6 +198,10 @@ en: flash: success: 'Tool Draft Published Successfully!' error: 'Tool Draft was not published successfully' + destroy: + flash: + success: 'Tool Deleted Successfully!' + error: 'Tool was not deleted successfully' clone: flash: notice: 'Records must have a unique Name and Long Name within a provider. Click here to enter a new Name and Long Name.' diff --git a/config/routes.rb b/config/routes.rb index ce35fbe4d..ab8ae9839 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -103,7 +103,7 @@ get '/services/:id/clone' => 'services#clone', as: 'clone_service' get '/services/:id/download_json(/:revision_id)' => 'services#download_json', as: 'download_json_service' - resources :tools, only: [:show, :create, :edit] + resources :tools, only: [:show, :create, :edit, :destroy] get '/tools/:id/clone' => 'tools#clone', as: 'clone_tool' get '/tools/:id/revisions' => 'tools#revisions', as: 'tool_revisions' diff --git a/spec/features/tools/delete_tool_spec.rb b/spec/features/tools/delete_tool_spec.rb new file mode 100644 index 000000000..50c152752 --- /dev/null +++ b/spec/features/tools/delete_tool_spec.rb @@ -0,0 +1,96 @@ +require 'rails_helper' + +describe 'Delete tool', reset_provider: true, js: true do + before :all do + @ingested_tool, _concept_response, _native_id_1 = publish_tool_draft + + @ingested_tool_for_delete_messages, _concept_response, @native_id_2 = publish_tool_draft + end + + after :all do + delete_response = cmr_client.delete_tool('MMT_2', @native_id_2, 'token') + # First tool should be deleted in the delete test + + raise unless delete_response.success? + end + + before do + login + end + + context 'when viewing a published tool' do + before do + visit tool_path(@ingested_tool['concept-id']) + end + + it 'displays a delete link' do + expect(page).to have_content('Delete Tool Record') + end + + context 'when clicking the delete link' do + before do + click_on 'Delete Tool Record' + end + + it 'displays a confirmation modal' do + expect(page).to have_content('Are you sure you want to delete this tool record?') + end + + context 'when clicking Yes' do + before do + within '#delete-record-modal' do + click_on 'Yes' + end + end + + it 'redirects to the revisions page and displays a confirmation message' do + expect(page).to have_content('Revision History') + + expect(page).to have_content('Tool Deleted Successfully!') + end + end + end + end + + context 'when deleting the tool will fail' do + before do + visit tool_path(@ingested_tool_for_delete_messages['concept-id']) + end + + context 'when CMR provides a message' do + before do + error_body = '{"errors": ["You do not have permission to perform that action."]}' + error_response = Cmr::Response.new(Faraday::Response.new(status: 401, body: JSON.parse(error_body), response_headers: {})) + allow_any_instance_of(Cmr::CmrClient).to receive(:delete_tool).and_return(error_response) + + click_on 'Delete Tool Record' + + within '#delete-record-modal' do + click_on 'Yes' + end + end + + it 'displays the CMR error message' do + expect(page).to have_css('.eui-banner--danger', text: 'You do not have permission to perform that action.') + end + end + + context 'when CMR does not provide a message' do + before do + error_body = '{"message": "useless message"}' + error_response = Cmr::Response.new(Faraday::Response.new(status: 401, body: JSON.parse(error_body), response_headers: {})) + allow_any_instance_of(Cmr::CmrClient).to receive(:delete_tool).and_return(error_response) + + click_on 'Delete Tool Record' + + within '#delete-record-modal' do + click_on 'Yes' + end + end + + it 'displays the CMR error message' do + expect(page).to have_css('.eui-banner--danger', text: 'Tool was not deleted successfully') + end + end + end +end diff --git a/spec/features/tools/tool_permissions_spec.rb b/spec/features/tools/tool_permissions_spec.rb index f01413565..0075c67f4 100644 --- a/spec/features/tools/tool_permissions_spec.rb +++ b/spec/features/tools/tool_permissions_spec.rb @@ -9,7 +9,7 @@ context "when the tool's provider is in the users available providers" do before :all do @ingested_tool, _concept_response, @native_id_1 = publish_tool_draft -# @ingested_tool_for_delete_modal, _concept_response, _native_id_2 = publish_tool_draft + @ingested_tool_for_delete_modal, _concept_response, _native_id_2 = publish_tool_draft end after :all do @@ -85,45 +85,53 @@ end # TODO: Uncomment in MMT-2229 -# context 'when clicking the delete link' do -# context 'when the tool has no associated collections' do -# before do -# visit tool_path(@ingested_tool_for_delete_modal['concept-id']) -# -# click_on 'Delete Tool Record' -# end -# -# it 'displays a modal informing the user they need to switch providers' do -# expect(page).to have_content("Deleting this tool #{modal_text}") -# end -# -# it 'does not display a message about collection associations that will also be deleted' do -# expect(page).to have_no_content('This tool is associated with') -# expect(page).to have_no_content('collections. Deleting this tool will also delete the collection associations') -# end -# end -# -# context 'when deleting the tool' do -# before do -# ingested_tool_to_delete, _concept_response = publish_tool_draft -# -# visit tool_path(ingested_tool_to_delete['concept-id']) -# -# click_on 'Delete Tool Record' -# -# find('.not-current-provider-link').click -# wait_for_jQuery -# end -# -# it 'switches the provider context' do -# expect(User.first.provider_id).to eq('MMT_2') -# end -# -# it 'deletes the record' do -# expect(page).to have_content('Tool Deleted Successfully!') -# end -# end -# end + context 'when clicking the delete link' do + + before do + login(provider: 'MMT_1', providers: %w(MMT_1 MMT_2)) + visit tool_path(@ingested_tool_for_delete_modal['concept-id']) + + click_on 'Delete Tool Record' + end + + it 'displays a modal informing the user they need to switch providers' do + expect(page).to have_content("Deleting this tool #{modal_text}") + end + + context 'when clicking Yes' do + before do + find('.not-current-provider-link').click + wait_for_jQuery + end + + it 'switches the provider context' do + expect(User.first.provider_id).to eq('MMT_2') + end + + it 'deletes the record' do + expect(page).to have_content('Tool Deleted Successfully!') + end + end + + #context 'when deleting the tool' do + # before do + # visit tool_path(@ingested_tool_for_delete_modal['concept-id']) + + # click_on 'Delete Tool Record' + + # find('.not-current-provider-link').click + # wait_for_jQuery + # end + + # it 'switches the provider context' do + # expect(User.first.provider_id).to eq('MMT_2') + # end + + # it 'deletes the record' do + # expect(page).to have_content('Tool Deleted Successfully!') + # end + #end + end context 'when trying to visit the action paths directly' do context 'when visiting the edit path directly' do From 22a9fb8a916127df1810197bce3778f0f1fbfd00 Mon Sep 17 00:00:00 2001 From: Ryan Miller Date: Fri, 10 Jul 2020 14:07:24 -0400 Subject: [PATCH 2/7] MMT-2229 Updating not current provider modal --- .../_not_current_provider_modal.html.erb | 2 +- spec/features/tools/tool_permissions_spec.rb | 34 +++++++++---------- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/app/views/shared/_not_current_provider_modal.html.erb b/app/views/shared/_not_current_provider_modal.html.erb index aeb5182bc..6f828700c 100644 --- a/app/views/shared/_not_current_provider_modal.html.erb +++ b/app/views/shared/_not_current_provider_modal.html.erb @@ -34,7 +34,7 @@ <% elsif options[:tool] %> <%= link_to 'Yes', edit_tool_path(options[:concept_id], revision_id: options[:revision_id]), class: 'eui-btn--blue spinner is-invisible', id: 'not-current-provider-edit-tool-link' %> <%= link_to 'Yes', clone_tool_path(options[:concept_id], revision_id: options[:revision_id]), class: 'eui-btn--blue spinner is-invisible', id: 'not-current-provider-clone-tool-link' %> - <%#= link_to 'Yes', tool_path(options[:concept_id]), method: :delete, class: 'eui-btn--blue spinner is-invisible', id: 'not-current-provider-delete-tool-link' %> + <%= link_to 'Yes', tool_path(options[:concept_id]), method: :delete, class: 'eui-btn--blue spinner is-invisible', id: 'not-current-provider-delete-tool-link' %> <% end %> <% if options[:draft] %> diff --git a/spec/features/tools/tool_permissions_spec.rb b/spec/features/tools/tool_permissions_spec.rb index 0075c67f4..993d6fe8c 100644 --- a/spec/features/tools/tool_permissions_spec.rb +++ b/spec/features/tools/tool_permissions_spec.rb @@ -9,12 +9,10 @@ context "when the tool's provider is in the users available providers" do before :all do @ingested_tool, _concept_response, @native_id_1 = publish_tool_draft - @ingested_tool_for_delete_modal, _concept_response, _native_id_2 = publish_tool_draft end after :all do delete_response = cmr_client.delete_tool('MMT_2', @native_id_1, 'token') - # Second tool should be deleted in the delete test raise unless delete_response.success? end @@ -86,8 +84,8 @@ # TODO: Uncomment in MMT-2229 context 'when clicking the delete link' do - before do + @ingested_tool_for_delete_modal, _concept_response, @native_id_2 = publish_tool_draft login(provider: 'MMT_1', providers: %w(MMT_1 MMT_2)) visit tool_path(@ingested_tool_for_delete_modal['concept-id']) @@ -107,30 +105,30 @@ it 'switches the provider context' do expect(User.first.provider_id).to eq('MMT_2') end - + it 'deletes the record' do expect(page).to have_content('Tool Deleted Successfully!') end end - #context 'when deleting the tool' do - # before do - # visit tool_path(@ingested_tool_for_delete_modal['concept-id']) + context 'when deleting the tool' do + before do + visit tool_path(@ingested_tool_for_delete_modal['concept-id']) - # click_on 'Delete Tool Record' + click_on 'Delete Tool Record' - # find('.not-current-provider-link').click - # wait_for_jQuery - # end + find('.not-current-provider-link').click + wait_for_jQuery + end - # it 'switches the provider context' do - # expect(User.first.provider_id).to eq('MMT_2') - # end + it 'switches the provider context' do + expect(User.first.provider_id).to eq('MMT_2') + end - # it 'deletes the record' do - # expect(page).to have_content('Tool Deleted Successfully!') - # end - #end + it 'deletes the record' do + expect(page).to have_content('Tool Deleted Successfully!') + end + end end context 'when trying to visit the action paths directly' do From db5af9598498f3a903f81bd0ef5053b2f392d48a Mon Sep 17 00:00:00 2001 From: William Valencia Date: Sun, 12 Jul 2020 22:24:15 -0400 Subject: [PATCH 3/7] MMT-2229 Fixing the tool_permissions_spec --- .../_not_current_provider_modal.html.erb | 2 +- spec/features/tools/tool_permissions_spec.rb | 26 +------------------ 2 files changed, 2 insertions(+), 26 deletions(-) diff --git a/app/views/shared/_not_current_provider_modal.html.erb b/app/views/shared/_not_current_provider_modal.html.erb index aeb5182bc..6f828700c 100644 --- a/app/views/shared/_not_current_provider_modal.html.erb +++ b/app/views/shared/_not_current_provider_modal.html.erb @@ -34,7 +34,7 @@ <% elsif options[:tool] %> <%= link_to 'Yes', edit_tool_path(options[:concept_id], revision_id: options[:revision_id]), class: 'eui-btn--blue spinner is-invisible', id: 'not-current-provider-edit-tool-link' %> <%= link_to 'Yes', clone_tool_path(options[:concept_id], revision_id: options[:revision_id]), class: 'eui-btn--blue spinner is-invisible', id: 'not-current-provider-clone-tool-link' %> - <%#= link_to 'Yes', tool_path(options[:concept_id]), method: :delete, class: 'eui-btn--blue spinner is-invisible', id: 'not-current-provider-delete-tool-link' %> + <%= link_to 'Yes', tool_path(options[:concept_id]), method: :delete, class: 'eui-btn--blue spinner is-invisible', id: 'not-current-provider-delete-tool-link' %> <% end %> <% if options[:draft] %> diff --git a/spec/features/tools/tool_permissions_spec.rb b/spec/features/tools/tool_permissions_spec.rb index 0075c67f4..1ccd1edf8 100644 --- a/spec/features/tools/tool_permissions_spec.rb +++ b/spec/features/tools/tool_permissions_spec.rb @@ -84,9 +84,7 @@ end end -# TODO: Uncomment in MMT-2229 context 'when clicking the delete link' do - before do login(provider: 'MMT_1', providers: %w(MMT_1 MMT_2)) visit tool_path(@ingested_tool_for_delete_modal['concept-id']) @@ -104,33 +102,11 @@ wait_for_jQuery end - it 'switches the provider context' do + it 'switches the provider context and deletes the record' do expect(User.first.provider_id).to eq('MMT_2') - end - - it 'deletes the record' do expect(page).to have_content('Tool Deleted Successfully!') end end - - #context 'when deleting the tool' do - # before do - # visit tool_path(@ingested_tool_for_delete_modal['concept-id']) - - # click_on 'Delete Tool Record' - - # find('.not-current-provider-link').click - # wait_for_jQuery - # end - - # it 'switches the provider context' do - # expect(User.first.provider_id).to eq('MMT_2') - # end - - # it 'deletes the record' do - # expect(page).to have_content('Tool Deleted Successfully!') - # end - #end end context 'when trying to visit the action paths directly' do From daaa4cfd232cd14459383f30a5840a27d62926f1 Mon Sep 17 00:00:00 2001 From: William Valencia Date: Sun, 12 Jul 2020 22:45:32 -0400 Subject: [PATCH 4/7] Fixing spacing for tools_permissions_spec.rb --- spec/features/tools/tool_permissions_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/tools/tool_permissions_spec.rb b/spec/features/tools/tool_permissions_spec.rb index cc77606de..dde06078b 100644 --- a/spec/features/tools/tool_permissions_spec.rb +++ b/spec/features/tools/tool_permissions_spec.rb @@ -221,4 +221,4 @@ end end end -end \ No newline at end of file +end From 9eec788ef4d036fad022c5773a6b8440d470b9b6 Mon Sep 17 00:00:00 2001 From: William Valencia Date: Mon, 13 Jul 2020 21:17:17 -0400 Subject: [PATCH 5/7] MMT-2229 Modifying delete_tool_spec due to comments and fixing tool_premissions_spec because of CMR-6332 --- spec/features/tools/delete_tool_spec.rb | 3 +-- spec/features/tools/tool_permissions_spec.rb | 16 +++++++++++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/spec/features/tools/delete_tool_spec.rb b/spec/features/tools/delete_tool_spec.rb index 50c152752..d81a0bf79 100644 --- a/spec/features/tools/delete_tool_spec.rb +++ b/spec/features/tools/delete_tool_spec.rb @@ -1,5 +1,3 @@ -require 'rails_helper' - describe 'Delete tool', reset_provider: true, js: true do before :all do @ingested_tool, _concept_response, _native_id_1 = publish_tool_draft @@ -7,6 +5,7 @@ @ingested_tool_for_delete_messages, _concept_response, @native_id_2 = publish_tool_draft end + # Remove this section after CMR-6332 is resolved after :all do delete_response = cmr_client.delete_tool('MMT_2', @native_id_2, 'token') # First tool should be deleted in the delete test diff --git a/spec/features/tools/tool_permissions_spec.rb b/spec/features/tools/tool_permissions_spec.rb index dde06078b..c68e986f6 100644 --- a/spec/features/tools/tool_permissions_spec.rb +++ b/spec/features/tools/tool_permissions_spec.rb @@ -93,6 +93,10 @@ it 'displays a modal informing the user they need to switch providers' do expect(page).to have_content("Deleting this tool #{modal_text}") + + # Remove this section after CMR-6332 is resolved + delete_response = cmr_client.delete_tool('MMT_2', @native_id_2, 'token') + raise unless delete_response.success? end context 'when clicking Yes' do @@ -172,16 +176,22 @@ context 'when the tools provider is not in the users available providers' do before do - @ingested_not_available_provider_tool, _concept_response = publish_tool_draft(provider_id: 'SEDAC') + @ingested_not_available_provider_tool, _concept_response, @native_id_3 = publish_tool_draft(provider_id: 'SEDAC') visit tool_path(@ingested_not_available_provider_tool['concept-id']) end + # Remove this section after CMR-6332 is resolved + after do + delete_response = cmr_client.delete_tool('SEDAC', @native_id_3, 'token') + + raise unless delete_response.success? + end + it 'does not display the action links' do expect(page).to have_no_link('Edit Tool Record') expect(page).to have_no_link('Clone Tool Record') -# TODO: Uncomment in MMT-2229 -# expect(page).to have_no_link('Delete Tool Record') + expect(page).to have_no_link('Delete Tool Record') end context 'when trying to visit the action paths directly' do From e1eedaac36b246dc398c4c2b5e7f62974a31316d Mon Sep 17 00:00:00 2001 From: William Valencia Date: Mon, 13 Jul 2020 21:23:55 -0400 Subject: [PATCH 6/7] MMT-2229 Removing unneeded delete --- spec/features/tools/tool_permissions_spec.rb | 7 ------- 1 file changed, 7 deletions(-) diff --git a/spec/features/tools/tool_permissions_spec.rb b/spec/features/tools/tool_permissions_spec.rb index c68e986f6..1d7d8bd80 100644 --- a/spec/features/tools/tool_permissions_spec.rb +++ b/spec/features/tools/tool_permissions_spec.rb @@ -181,13 +181,6 @@ visit tool_path(@ingested_not_available_provider_tool['concept-id']) end - # Remove this section after CMR-6332 is resolved - after do - delete_response = cmr_client.delete_tool('SEDAC', @native_id_3, 'token') - - raise unless delete_response.success? - end - it 'does not display the action links' do expect(page).to have_no_link('Edit Tool Record') expect(page).to have_no_link('Clone Tool Record') From 55a52ae6a38c631defc2800a64c364d1046f2543 Mon Sep 17 00:00:00 2001 From: William Valencia Date: Mon, 13 Jul 2020 21:25:09 -0400 Subject: [PATCH 7/7] MMT-2229 Removing unneeded native_id_3 --- spec/features/tools/tool_permissions_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/tools/tool_permissions_spec.rb b/spec/features/tools/tool_permissions_spec.rb index 1d7d8bd80..1c617a181 100644 --- a/spec/features/tools/tool_permissions_spec.rb +++ b/spec/features/tools/tool_permissions_spec.rb @@ -176,7 +176,7 @@ context 'when the tools provider is not in the users available providers' do before do - @ingested_not_available_provider_tool, _concept_response, @native_id_3 = publish_tool_draft(provider_id: 'SEDAC') + @ingested_not_available_provider_tool, _concept_response = publish_tool_draft(provider_id: 'SEDAC') visit tool_path(@ingested_not_available_provider_tool['concept-id']) end