From 3e69afdde3c9781e6097a6ee179253e0774ced63 Mon Sep 17 00:00:00 2001 From: ryanmiller-1 <40173609+ryanmiller-1@users.noreply.github.com> Date: Wed, 8 Jul 2020 14:51:10 -0400 Subject: [PATCH 01/34] MMT-2231 Add revisions page to published UMM-T page (#610) * MMT-2231 Adding revisions page to Tool Records * MMT-2231 updating display_header_subtitle to work with drafts again --- app/controllers/tools_controller.rb | 2 +- app/helpers/manage_metadata_helper.rb | 2 +- app/views/tools/revisions.html.erb | 119 +++++++++++++++++++ app/views/tools/show.html.erb | 3 +- config/routes.rb | 1 + spec/features/services/revision_list_spec.rb | 2 - spec/features/tools/revision_list_spec.rb | 90 ++++++++++++++ 7 files changed, 213 insertions(+), 6 deletions(-) create mode 100644 app/views/tools/revisions.html.erb create mode 100644 spec/features/tools/revision_list_spec.rb diff --git a/app/controllers/tools_controller.rb b/app/controllers/tools_controller.rb index acfdbda29..d92f31a8e 100644 --- a/app/controllers/tools_controller.rb +++ b/app/controllers/tools_controller.rb @@ -2,7 +2,7 @@ class ToolsController < BasePublishedRecordController include ManageMetadataHelper - before_action :set_tool, only: [:show, :edit] #, :clone, :destroy, :revisions, :revert, :download_json] + before_action :set_tool, only: [:show, :edit, :revisions] #, :clone, :destroy, :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] diff --git a/app/helpers/manage_metadata_helper.rb b/app/helpers/manage_metadata_helper.rb index 43f752208..c420b2c47 100644 --- a/app/helpers/manage_metadata_helper.rb +++ b/app/helpers/manage_metadata_helper.rb @@ -80,7 +80,7 @@ def resource_type end def display_header_subtitle(metadata, type) - return unless type.downcase.include?('variable') || type.downcase.include?('service') + return unless ['variable', 'service', 'tool'].any? { |type_fragment| type.downcase.include?(type_fragment) } metadata['LongName'] || 'Long Name Not Provided' end diff --git a/app/views/tools/revisions.html.erb b/app/views/tools/revisions.html.erb new file mode 100644 index 000000000..9c4f2a277 --- /dev/null +++ b/app/views/tools/revisions.html.erb @@ -0,0 +1,119 @@ +<% content_for :header_title do %> +

<%= fetch_entry_id(@tool, 'tools') %>

+

<%= display_header_subtitle(@tool, 'tool') %>

+<% end %> + +<% if @errors && !@errors.empty? %> +
+
+
+
    + <% @errors.each do |error| %> +
  • + <%= "#{error[:field]}, " if error[:field] %> + <%= error[:error] %> + <% if error[:request_id] %> + Click here to submit feedback + <% end %> +
  • + <% end %> +
+
+
+
+<% end %> + +
+
+

Revision History

+
+ <% if @error %> +
+
+

+ This tool could not be updated. You may <%= link_to 'edit', edit_tool_path(revision_id: @revision_id) %> the tool to resolve these issues. +

+
+
+ <% end %> +
+
+ + + + + + + + + + + <% @revisions.each_with_index do |revision, index| %> + <% revision_id = revision['meta']['revision-id'] %> + + + + + + + <% end %> + +
DescriptionRevision DateAction byActions
+ <% title = "View revision #{revision_id}" %> + <%= revision_id %> - + <% if revision['meta']['deleted'] == true %> + Deleted + <% elsif index == 0 %> + Published + <%# Uncomment in MMT-2232 %> + <%#= link_to 'View', tool_path(revision_id: revision_id), title: title %> + <% else %> + Revision + <%# Uncomment in MMT-2232 %> + <%#= link_to 'View', tool_path(revision_id: revision_id), title: title %> + <% end %> + + <%= revision['meta']['revision-date'] %> + + <%= revision['meta']['user-id'] %> + + <% if @revisions.first['meta']['deleted'] == true %> + <% phrase = 'Reinstate' %> + <% confirm_phrase = 'Are you sure you want to reinstate this record?' %> + <% action = 'reinstate-tool' %> + <% else %> + <% phrase = 'Revert to this Revision' %> + <% confirm_phrase = 'Are you sure you want to revert to this revision?' %> + <% action = 'revert-tool' %> + <% end %> + + <% unless index == 0 || revision['meta']['deleted'] == true %> + <%# Uncomment in MMT-2233 %> + <%# if current_provider?(@provider_id) %> + <%#= link_to phrase, "#revert-revisions-modal-#{revision_id}", class: 'display-modal' %> + <%# elsif available_provider?(@provider_id) %> + <%#= link_to phrase, "#not-current-provider-modal-#{revision_id}", class: 'display-modal not-current-provider', data: { 'provider': @provider_id, record_action: action } %> + <%# end %> + + <% end %> +
+
+
+
diff --git a/app/views/tools/show.html.erb b/app/views/tools/show.html.erb index 9cf647aed..6665cf2bb 100644 --- a/app/views/tools/show.html.erb +++ b/app/views/tools/show.html.erb @@ -133,8 +133,7 @@ } %>

- <%= link_to 'Revisions', '#', class: 'eui-btn--link disabled' %> - <%#= link_to "Revisions (#{@revisions.size})", tool_revisions_path, class: 'eui-btn--link disabled' %> + <%= link_to "Revisions (#{@revisions.size})", tool_revisions_path, class: 'eui-btn--link' %>

diff --git a/config/routes.rb b/config/routes.rb index 4b61880ee..6e8cc5c8a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -104,6 +104,7 @@ get '/services/:id/download_json(/:revision_id)' => 'services#download_json', as: 'download_json_service' resources :tools, only: [:show, :create, :edit] + get '/tools/:id/revisions' => 'tools#revisions', as: 'tool_revisions' resources :variable_drafts, controller: 'variable_drafts', draft_type: 'VariableDraft' do member do diff --git a/spec/features/services/revision_list_spec.rb b/spec/features/services/revision_list_spec.rb index 80a1a2c18..6fbe57f1d 100644 --- a/spec/features/services/revision_list_spec.rb +++ b/spec/features/services/revision_list_spec.rb @@ -1,5 +1,3 @@ -require 'rails_helper' - describe 'Service revision list', reset_provider: true, js: true do context 'when viewing a published service' do before do diff --git a/spec/features/tools/revision_list_spec.rb b/spec/features/tools/revision_list_spec.rb new file mode 100644 index 000000000..b33ba78f1 --- /dev/null +++ b/spec/features/tools/revision_list_spec.rb @@ -0,0 +1,90 @@ +describe 'Tool revision list', reset_provider: true, js: true do + context 'when viewing a published tool' do + before :all do + @ingest_response, @concept_response, @native_id = publish_tool_draft(revision_count: 2) + end + + # TODO: remove after CMR-6332 + after :all do + delete_response = cmr_client.delete_tool('MMT_2', @native_id, 'token') + + raise unless delete_response.success? + end + + before do + login + + visit tool_path(@ingest_response['concept-id']) + end + + it 'displays the number of revisions' do + expect(page).to have_content('Revisions (2)') + end + + context 'when clicking on the revision link' do + before do + wait_for_cmr + click_on 'Revisions' + end + + it 'displays the revision page' do + expect(page).to have_content('Revision History') + end + + it 'displays the tool long name' do + expect(page).to have_content(@concept_response.body['LongName']) + end + + it 'displays when the revision was made' do + expect(page).to have_content(today_string, count: 2) + end + + it 'displays what user made the revision' do + expect(page).to have_content('typical', count: 2) + end + +# TODO: Uncomment in MMT-2233 +# it 'displays the correct phrasing for reverting records' do +# expect(page).to have_content('Revert to this Revision', count: 1) +# end + +# TODO: Uncomment in MMT-2232 +# context 'when viewing an old revision' do +# link_text = 'You are viewing an older revision of this tool. Click here to view the latest published version.' +# before do +# all('a', text: 'View').last.click +# end +# +# it 'displays a message that the revision is old' do +# expect(page).to have_link(link_text) +# end +# +# it 'does not display a link to manage collection associations' do +# expect(page).to have_no_link('Manage Collection Associations') +# end +# +# context 'when clicking the message' do +# before do +# click_on link_text +# end +# +# it 'displays the latest revision to the user' do +# expect(page).to have_no_link(link_text) +# end +# end +# end + end + + context 'when searching for the tool' do + before do + full_search(record_type: 'Tools', keyword: @concept_response.body['LongName'], provider: 'MMT_2') + end + + it 'only displays the latest revision' do + within '#tool-search-results' do + expect(page).to have_content(@concept_response.body['LongName'], count: 1) + end + end + end + end +end From fe0543a75c40e00d673a9c9b4ce964cdf0801a11 Mon Sep 17 00:00:00 2001 From: Christian Trummer Date: Wed, 8 Jul 2020 11:40:11 -0400 Subject: [PATCH 02/34] MMT-2311: Added a translate_collections.rake file and added nokogiri/diff to the gemfile --- Gemfile | 2 + Gemfile.lock | 5 + lib/tasks/local_cmr.rake | 8 +- lib/tasks/translate_collections.rake | 275 +++++++++++++++++++++++++++ package-lock.json | 14 ++ 5 files changed, 300 insertions(+), 4 deletions(-) create mode 100644 lib/tasks/translate_collections.rake create mode 100644 package-lock.json diff --git a/Gemfile b/Gemfile index f7b917df1..f7c4b2a05 100644 --- a/Gemfile +++ b/Gemfile @@ -59,6 +59,8 @@ gem 'kaminari' gem 'momentjs-rails' # js lib for dates gem 'pundit' +gem 'nokogiri-diff', '~> 0.2.0' # for comparing xml documents + gem 'activerecord-import' # bulk insertion of data gem 'activerecord-session_store' diff --git a/Gemfile.lock b/Gemfile.lock index 61ce28ff4..493f42aa4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -195,6 +195,9 @@ GEM nio4r (2.5.2) nokogiri (1.10.9) mini_portile2 (~> 2.4.0) + nokogiri-diff (0.2.0) + nokogiri (~> 1.5) + tdiff (~> 0.3, >= 0.3.2) parallel (1.19.1) parser (2.7.1.2) ast (~> 2.4.0) @@ -297,6 +300,7 @@ GEM activesupport (>= 4.0) sprockets (>= 3.0.0) sqlite3 (1.4.2) + tdiff (0.3.4) thor (1.0.1) thread_safe (0.3.6) tilt (2.0.10) @@ -365,6 +369,7 @@ DEPENDENCIES mini_racer momentjs-rails multi_xml + nokogiri-diff (~> 0.2.0) pg pundit rack_session_access diff --git a/lib/tasks/local_cmr.rake b/lib/tasks/local_cmr.rake index 4e5689b9b..1930a95e2 100644 --- a/lib/tasks/local_cmr.rake +++ b/lib/tasks/local_cmr.rake @@ -11,8 +11,8 @@ namespace :cmr do # use puts %x() to run commands and not Process.spawn(), which returns # imediatly, because curl can take a while and we want to see the output # from standard out. - - # make a backup copy of the old jar if it exists, and a backup of the backup + + # make a backup copy of the old jar if it exists, and a backup of the backup cmd_backup = 'cd cmr ; '\ "if [ -a \"#{jar_name}\" ] ; then "\ 'echo Backup jar... ; '\ @@ -20,7 +20,7 @@ namespace :cmr do "mv -fv #{jar_name} #{jar_name}.last ; "\ 'fi' puts %x( #{cmd_backup} ) - + # download a new jar to a temp location and rename it if successfull puts 'Download jar...' cmd_fetch = 'cd cmr ; '\ @@ -247,4 +247,4 @@ namespace :cmr do puts "Done!" end end -end \ No newline at end of file +end diff --git a/lib/tasks/translate_collections.rake b/lib/tasks/translate_collections.rake new file mode 100644 index 000000000..3b73529dc --- /dev/null +++ b/lib/tasks/translate_collections.rake @@ -0,0 +1,275 @@ +require 'libxml_to_hash' + +namespace :collection do + desc 'Translate a collection from native format to UMM JSON and back to native format' + task :translate, [:file, :format, :version] => :environment do |_task, args| + args.with_defaults(version: '1.15.3') + + puts 'INVALID FORMAT' if !args.format.eql?('echo10' && 'dif10' && 'iso19115') + + filename = args.file.split('/')[-1] + puts "\nTranslating #{filename} to UMM JSON..." + + native_original_xml = File.read(args.file) + native_original_hash = Hash.from_xml(native_original_xml) + + #translate to UMM + umm = cmr_client.translate_collection(native_original_xml, "application/#{args.format}+xml", "application/vnd.nasa.cmr.umm+json;version=#{args.version}", skip_validation=true ) + umm_json = umm.body.to_json + umm.success? ? puts("\nsuccessful translation to UMM") : puts("\nUMM translation failure") + + # translate back to native + back_to_native = cmr_client.translate_collection(umm_json, "application/vnd.nasa.cmr.umm+json;version=#{args.version}", "application/#{args.format}+xml", skip_validation=true ) + native_converted_hash = Hash.from_xml(back_to_native.body) + native_converted_xml = back_to_native.body + back_to_native.success? ? puts("successful translation to native format \n\n") : puts("Native format translation failure \n\n") + + # nokogiri output + o = Nokogiri::XML(native_original_xml) { |config| config.strict.noblanks } + c = Nokogiri::XML(native_converted_xml) { |config| config.strict.noblanks } + + o.diff(c, {:added => true, :removed => true}) do |change,node| + next if path_leads_to_list?(node.parent.path, native_original_hash, native_converted_hash) + puts "#{change}: " + node.parent.path + end + + # find differences + dif_hash = find_difference_bt_hashes(native_original_hash, native_converted_hash) + compare_arrays(dif_hash, native_original_hash, native_converted_hash) + + end + + def path_leads_to_list?(path, org_hash, conv_hash) + org_hash_path = hash_navigation(path, org_hash) + conv_hash_path = hash_navigation(path, conv_hash) + + if org_hash_path == 'flag' || conv_hash_path == 'flag' + return false + end + + if path.include?("[") && path.include?("]") + bool = true + elsif org_hash_path.is_a?(Hash) && conv_hash_path.is_a?(Hash) + org_hash_path.keys.each { |key| bool = true; break if org_hash_path[key].is_a?(Array) } + conv_hash_path.keys.each { |key| bool = true; break if conv_hash_path[key].is_a?(Array) } + elsif org_hash_path.is_a?(Array) || conv_hash_path.is_a?(Array) + bool = true + else + bool = false + end + bool + end + + def hash_navigation(dir, hash) + dir = dir.split("/") + if dir.is_a? Array + dir.each do |key| + if !key.empty? && hash.is_a?(Hash) + hash = hash[key] + elsif hash.is_a? Array + return 'flag' + end + end + else + hash = hash[dir] + end + hash + end + + def get_list_paths(dif_hash, original, converted) + values_list = hash_to_list_of_values(dif_hash) + paths = Array.new + + for item in values_list + org_path = get_dir(item, original) + conv_path = get_dir(item, converted) + + if org_path.include? "[]" + path = org_path + elsif conv_path.include? "[]" + path = conv_path + else + path = org_path #arbitrary + end + + if path.include? "[]" + path = path.split "[]" + paths << path[0] unless paths.any? { |p| p.eql? path[0] } + elsif path_leads_to_list?(path, original, converted) + paths << path unless paths.any? { |p| p.eql? path } + end + end + paths + end + + def compare_arrays(diff_hash, original_hash, converted_hash) + dif_hash = diff_hash.clone + original = original_hash.clone + converted = converted_hash.clone + paths = get_list_paths(dif_hash, original, converted) + + paths.each do |path| + org_array = hash_navigation(path, original) + org_arr = org_array.clone + conv_array = hash_navigation(path, converted) + conv_arr = conv_array.clone + + org_arr = Array.wrap(org_arr) unless org_arr.is_a?(Array) + org_array = Array.wrap(org_array) unless org_array.is_a?(Array) + conv_arr = Array.wrap(conv_arr) unless conv_arr.is_a?(Array) + conv_array = Array.wrap(conv_array) unless conv_array.is_a?(Array) + + for conv_item in conv_array + for org_item in org_array + if org_item.eql? conv_item + org_arr.delete(org_item) + break + end + end + end + + for org_item in org_array + for conv_item in conv_array + if org_item.eql? conv_item + conv_arr.delete(conv_item) + break + end + end + end + + org_arr.each do |item| + path_with_index = path + "[#{org_array.index(item)}]" + puts "-: ". + path_with_index + end + + conv_arr.each do |item| + path_with_index = path + "[#{conv_array.index(item)}]" + puts "+: " + path_with_index #THIS INDEX DOESN'T MAKE SENSE + end + end + end + + def find_difference_bt_hash_arrays(org_arr, conv_arr) + org = org_arr.clone + conv = conv_arr.clone + missing = Array.new + if org.eql? conv + return missing + else + for conv_item in conv + for org_item in org + if org_item.eql? conv_item + org.delete(conv_item) + break + end + end + end + missing += org + end + missing + end + + def find_difference_bt_hashes(org, conv) + missing = Hash.new + if org.eql? conv + return missing + else + org.each do |org_key,org_value| + conv_value = conv[org_key] + if conv.key? org_key + if conv_value.eql? org_value + next + elsif org_value.is_a?(Hash) && conv_value.is_a?(Hash) + missing_value = find_difference_bt_hashes(org_value, conv_value) + unless missing_value.empty? + missing[org_key] = missing_value + end + elsif org_value.is_a?(Array) && conv_value.is_a?(Array) + missing_value = find_difference_bt_hash_arrays(org_value, conv_value) + unless missing_value.empty? + missing[org_key] = missing_value + end + else + missing[org_key] = org_value + end + else + missing[org_key] = org_value + end + end + end + missing + end + + def get_dir(value, hash_or_arr) + iterable = hash_or_arr.clone + dir = String.new + if iterable.is_a? Hash + unless iterable.key(value).nil? + matching_key = iterable.key(value) + dir += '/' + matching_key + iterable.delete(matching_key) + return dir + else + iterable.each do |key,val| + if val.is_a?(Hash) && hash_to_list_of_values(val).include?(value) + dir += '/' + key + dir += get_dir(value, val) + return dir + elsif val.is_a?(Array) && array_to_list_of_values(val).include?(value) + dir += '/' + key + "[]" + dir += get_dir(value, val) + return dir + elsif val.eql? value + dir += '/' + key + iterable.delete(key) + return dir + end + end + end + elsif iterable.is_a? Array + iterable.each do |item| + if item.is_a?(Hash) && hash_to_list_of_values(item).include?(value) + dir += get_dir(value,item) + return dir + elsif item.is_a?(Array) && array_to_list_of_values(item).include?(value) + puts "\n\n\n\n\n\n\n\n USED THIS SECTION \n\n\n\n\n\n\n\n\n\n\n\n" + dir += get_dir(value,item) + "[]" + return dir + end + end + end + dir + end + + def hash_to_list_of_values(hash) + list = Array.new + for val in hash.values + if val.is_a? Hash + list += hash_to_list_of_values(val) + elsif val.is_a? Array + list += array_to_list_of_values(val) + else + list << val + end + end + list + end + + def array_to_list_of_values(array) + ls = Array.new + for item in array + if item.is_a? Hash + ls += hash_to_list_of_values(item) + elsif item.is_a? Array + ls += array_to_list_of_values(item) + else + ls << item + end + end + ls + end + + def cmr_client + @cmr_client ||= Cmr::Client.client_for_environment(Rails.configuration.cmr_env, Rails.configuration.services) + end +end diff --git a/package-lock.json b/package-lock.json new file mode 100644 index 000000000..eec54849c --- /dev/null +++ b/package-lock.json @@ -0,0 +1,14 @@ +{ + "name": "mmt", + "version": "1.0.0", + "lockfileVersion": 1, + "requires": true, + "dependencies": { + "coffee-script": { + "version": "1.12.7", + "resolved": "https://registry.npmjs.org/coffee-script/-/coffee-script-1.12.7.tgz", + "integrity": "sha512-fLeEhqwymYat/MpTPUjSKHVYYl0ec2mOyALEMLmzr5i1isuG+6jfI2j2d5oBO3VIzgUXgBVIcOT9uH1TFxBckw==", + "dev": true + } + } +} From 5e6a0a8af7c9dcf56013d704c34469c8d86a14e4 Mon Sep 17 00:00:00 2001 From: ryanmiller-1 <40173609+ryanmiller-1@users.noreply.github.com> Date: Thu, 9 Jul 2020 12:09:14 -0400 Subject: [PATCH 03/34] MMT-2288 Update jquery version that is being package for the preview gem (#611) * MMT-2288 updated version for preview gem * MMT-2288 updating preview gem commit ref. --- Gemfile | 2 +- Gemfile.lock | 6 +++--- lib/tasks/local_cmr.rake | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Gemfile b/Gemfile index f7b917df1..3c70e72a4 100644 --- a/Gemfile +++ b/Gemfile @@ -78,7 +78,7 @@ gem 'browser' # bundle config local.cmr_metadata_preview /path/to/local/git/repository # make sure to delete the local config when done making changes to merge into master # bundle config --delete local.cmr_metadata_preview -gem 'cmr_metadata_preview', git: 'https://git.earthdata.nasa.gov/scm/cmr/cmr_metadata_preview.git', ref: 'fff65949cc6' +gem 'cmr_metadata_preview', git: 'https://git.earthdata.nasa.gov/scm/cmr/cmr_metadata_preview.git', ref: '1f6ffd54d65' group :development, :test do # Call 'byebug' anywhere in the code to stop execution and get a debugger console diff --git a/Gemfile.lock b/Gemfile.lock index 61ce28ff4..e2d476f3d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,9 +1,9 @@ GIT remote: https://git.earthdata.nasa.gov/scm/cmr/cmr_metadata_preview.git - revision: fff65949cc62d397b0675e9fe46e962b8cf43228 - ref: fff65949cc6 + revision: 1f6ffd54d6570f9f920078a84fd51750db3c21ab + ref: 1f6ffd54d65 specs: - cmr_metadata_preview (0.2.2) + cmr_metadata_preview (0.2.3) georuby rails (~> 5.2.0) sprockets (< 4.0) diff --git a/lib/tasks/local_cmr.rake b/lib/tasks/local_cmr.rake index 4e5689b9b..ed0ff1f54 100644 --- a/lib/tasks/local_cmr.rake +++ b/lib/tasks/local_cmr.rake @@ -153,11 +153,11 @@ namespace :cmr do File.join(Rails.root.to_s, 'vendor', 'assets', 'javascripts', 'eui-1.0.0', 'eui.js') ] - # TODO: move to version 3 of jquery - # it is not currently understood how this section works to select jquery - # currently the preview gem is not running with version 3, but 1 + # Find the path to jquery jquery = Rails.application.config.assets.paths.select { |p| p.to_s.include?('jquery-rails') } - dependencies.unshift(File.join(jquery.first, 'jquery.js')) if jquery.any? + # Include a specific file. jquery-rails has files for each major version + # stored in the above location + dependencies.unshift(File.join(jquery.first, 'jquery3.js')) if jquery.any? js_to_uglify = dependencies.sort.map do |file| puts "- Reading #{file}" From 994cbb17c16c552bff3f23786cbad15d11bd4c96 Mon Sep 17 00:00:00 2001 From: ryanmiller-1 <40173609+ryanmiller-1@users.noreply.github.com> Date: Thu, 9 Jul 2020 12:09:32 -0400 Subject: [PATCH 04/34] MMT-2230 (#608) --- .../change_current_provider.coffee | 2 + app/controllers/tools_controller.rb | 6 +- app/helpers/tools_helper.rb | 12 + app/models/tool_draft.rb | 10 +- .../_not_current_provider_modal.html.erb | 2 +- app/views/tools/show.html.erb | 17 +- config/locales/en.yml | 3 + config/routes.rb | 1 + .../create_tool_draft_from_cloning_spec.rb | 62 +++++ spec/features/tools/tool_permissions_spec.rb | 238 ++++++++++++++++++ 10 files changed, 333 insertions(+), 20 deletions(-) create mode 100644 app/helpers/tools_helper.rb create mode 100644 spec/features/tool_drafts/create_tool_draft_from_cloning_spec.rb create mode 100644 spec/features/tools/tool_permissions_spec.rb diff --git a/app/assets/javascripts/change_current_provider.coffee b/app/assets/javascripts/change_current_provider.coffee index 540b4124c..321d72a24 100644 --- a/app/assets/javascripts/change_current_provider.coffee +++ b/app/assets/javascripts/change_current_provider.coffee @@ -74,6 +74,8 @@ $(document).ready -> "Managing this service's collection associations" when 'edit-tool' 'Editing this tool' + when 'clone-tool' + 'Cloning 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 d92f31a8e..2c5686dbe 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, :revisions] #, :clone, :destroy, :revert, :download_json] - before_action :set_schema, only: [:show, :edit] #, :clone, :destroy] + before_action :set_tool, only: [:show, :edit, :clone, :revisions] #, :destroy, :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/helpers/tools_helper.rb b/app/helpers/tools_helper.rb new file mode 100644 index 000000000..19a5d5e6c --- /dev/null +++ b/app/helpers/tools_helper.rb @@ -0,0 +1,12 @@ +module ToolsHelper + def render_change_provider_tool_action_link(tool_action, concept_id, revision_id = nil) + case tool_action + when 'edit' + link_to('Edit Service', edit_tool_path(concept_id, revision_id: revision_id), class: 'is-invisible', id: 'change-provider-tool-edit') + when 'clone' + link_to('Clone Service', clone_tool_path(concept_id, revision_id: revision_id), class: 'is-invisible', id: 'change-provider-tool-clone') + when 'delete' + link_to('Delete Service', tool_path(concept_id), method: :delete, class: 'is-invisible', id: 'change-provider-tool-delete') + end + end +end \ No newline at end of file diff --git a/app/models/tool_draft.rb b/app/models/tool_draft.rb index 7695bc77f..a5282947c 100644 --- a/app/models/tool_draft.rb +++ b/app/models/tool_draft.rb @@ -15,11 +15,11 @@ def create_from_tool(tool, user, native_id) draft = self.find_or_initialize_by(native_id: native_id) draft.entry_title = tool['LongName'] draft.short_name = tool['Name'] - # else - # # Cloned Record - # draft = self.new - # tool.delete('Name') - # tool.delete('LongName') + else + # Cloned Record + draft = self.new + tool.delete('Name') + tool.delete('LongName') end draft.set_user_and_provider(user) diff --git a/app/views/shared/_not_current_provider_modal.html.erb b/app/views/shared/_not_current_provider_modal.html.erb index 3843bc179..aeb5182bc 100644 --- a/app/views/shared/_not_current_provider_modal.html.erb +++ b/app/views/shared/_not_current_provider_modal.html.erb @@ -33,7 +33,7 @@ <%= link_to 'Yes', service_collection_associations_path(options[:concept_id]), class: 'eui-btn--blue spinner is-invisible', id: 'not-current-provider-manage-service-associations-link' %> <% 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', 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' %> <% end %> diff --git a/app/views/tools/show.html.erb b/app/views/tools/show.html.erb index 6665cf2bb..14240393e 100644 --- a/app/views/tools/show.html.erb +++ b/app/views/tools/show.html.erb @@ -23,9 +23,7 @@ "#", id: "change-current-provider-banner-link", data: { "provider": @provider_id, action_link: "change-provider-tool-#{@record_action}" }) %>

- <%# TODO: this method does not exist yet. It should be created and used when %> - <%# additional actions are added to published Tool records %> - <%#= render_change_provider_tool_action_link(@record_action, @concept_id, @revision_id) %> + <%= render_change_provider_tool_action_link(@record_action, @concept_id, @revision_id) %> <% end %> @@ -94,14 +92,11 @@ <% end %> <% end %> - <%# TODO: All links commented out and disabled links added for MMT-2238 %> - <%# links should be re-enabled with the appropriate ticket %> - <%= link_to 'Clone Tool Record', '#', class: 'eui-btn--link bar-after disabled' %> - <%# if current_provider?(@provider_id) %> - <%#= link_to 'Clone Tool Record', clone_tool_path(revision_id: @revision_id), class: 'eui-btn--link bar-after' %> - <%# elsif available_provider?(@provider_id) %> - <%#= link_to 'Clone Tool Record', '#not-current-provider-modal', class: 'display-modal not-current-provider eui-btn--link bar-after', data: { 'provider': @provider_id, record_action: 'clone-tool' } %> - <%# end %> + <% if current_provider?(@provider_id) %> + <%= link_to 'Clone Tool Record', clone_tool_path(revision_id: @revision_id), class: 'eui-btn--link bar-after' %> + <% elsif available_provider?(@provider_id) %> + <%= link_to 'Clone Tool Record', '#not-current-provider-modal', class: 'display-modal not-current-provider eui-btn--link bar-after', data: { 'provider': @provider_id, record_action: 'clone-tool' } %> + <% end %> <%= 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' %> diff --git a/config/locales/en.yml b/config/locales/en.yml index bf9d5b6cb..d420d9298 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -198,6 +198,9 @@ en: flash: success: 'Tool Draft Published Successfully!' error: 'Tool Draft was not published 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.' collection_associations: destroy: flash: diff --git a/config/routes.rb b/config/routes.rb index 6e8cc5c8a..ce35fbe4d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -104,6 +104,7 @@ get '/services/:id/download_json(/:revision_id)' => 'services#download_json', as: 'download_json_service' resources :tools, only: [:show, :create, :edit] + get '/tools/:id/clone' => 'tools#clone', as: 'clone_tool' get '/tools/:id/revisions' => 'tools#revisions', as: 'tool_revisions' resources :variable_drafts, controller: 'variable_drafts', draft_type: 'VariableDraft' do diff --git a/spec/features/tool_drafts/create_tool_draft_from_cloning_spec.rb b/spec/features/tool_drafts/create_tool_draft_from_cloning_spec.rb new file mode 100644 index 000000000..76a02c85c --- /dev/null +++ b/spec/features/tool_drafts/create_tool_draft_from_cloning_spec.rb @@ -0,0 +1,62 @@ +describe 'Creating a tool draft from cloning a tool', reset_provider: true, js: true do + before :all do + @ingest_response, _concept_response, @native_id = publish_tool_draft + end + + after :all do + delete_response = cmr_client.delete_tool('MMT_2', @native_id, 'token') + + raise unless delete_response.success? + end + + context 'when cloning a published tool' do + before do + login + + visit tool_path(@ingest_response['concept-id']) + + click_on 'Clone Tool Record' + end + + it 'displays the draft preview page' do + within '.eui-breadcrumbs' do + expect(page).to have_content('Tool Drafts') + end + + expect(page).to have_content('Publish Tool Draft') + expect(page).to have_content('Delete Tool Draft') + expect(page).to have_content('Metadata Fields') + expect(page).to have_content('Tool Information') + end + + it 'removes the Name and Long Name from the metadata' do + within '#tool_draft_draft_name_preview' do + expect(page).to have_css('p', text: 'No value for Name provided.') + end + + within '#tool_draft_draft_long_name_preview' do + expect(page).to have_css('p', text: 'No value for Long Name provided.') + end + end + + it 'creates a new native id for the draft' do + draft = ToolDraft.last + expect(draft.native_id).to eq("mmt_tool_#{draft.id}") + end + + it 'displays a message that the draft needs a unique Name' do + expect(page).to have_content('Records must have a unique Name and Long Name within a provider. Click here to enter a new Name and Long Name.') + end + + context 'when clicking the banner message to enter a new Name' do + before do + click_on 'Click here to enter a new Name and Long Name.' + end + + it 'displays the empty Name and Long Name fields' do + expect(page).to have_field('Name', with: '') + expect(page).to have_field('Long Name', with: '') + end + end + end +end diff --git a/spec/features/tools/tool_permissions_spec.rb b/spec/features/tools/tool_permissions_spec.rb new file mode 100644 index 000000000..f01413565 --- /dev/null +++ b/spec/features/tools/tool_permissions_spec.rb @@ -0,0 +1,238 @@ +describe 'Tools permissions', reset_provider: true, js: true do + let(:modal_text) { 'requires you change your provider context to MMT_2' } + + context 'when viewing a tool' do + before do + login + end + + 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 + + before do + login(provider: 'MMT_1', providers: %w(MMT_1 MMT_2)) + + visit tool_path(@ingested_tool['concept-id']) + end + + it 'displays the action links' do + expect(page).to have_link('Edit Tool Record') + expect(page).to have_link('Clone Tool Record') + expect(page).to have_link('Delete Tool Record') + end + + context 'when clicking the edit link' do + before do + click_on 'Edit Tool Record' + end + + it 'displays a modal informing the user they need to switch providers' do + expect(page).to have_content("Editing this tool #{modal_text}") + end + + context 'when clicking Yes' do + before do + # click_on 'Yes' + 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 'creates a draft from the tool' do + expect(page).to have_content('Tool Draft Created Successfully!') + expect(Draft.where(provider_id: 'MMT_2').size).to eq(1) + end + end + end + + context 'when clicking the clone link' do + before do + click_on 'Clone Tool Record' + end + + it 'displays a modal informing the user they need to switch providers' do + expect(page).to have_content("Cloning 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 'creates a draft from the tool' do + expect(page).to have_content('Records must have a unique Name and Long Name within a provider. Click here to enter a new Name and Long Name.') + expect(Draft.where(provider_id: 'MMT_2').size).to eq(1) + end + end + 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 trying to visit the action paths directly' do + context 'when visiting the edit path directly' do + before do + edit_link = page.current_path + '/edit' + visit edit_link + end + + it 'displays warning banner link to change provider' do + expect(page).to have_css('.eui-banner--warn') + expect(page).to have_content('You need to change your current provider to edit this tool') + end + + context 'when clicking the warning banner link' do + before do + click_link('You need to change your current provider to edit this tool') + wait_for_jQuery + end + + it 'switches the provider context' do + expect(User.first.provider_id).to eq('MMT_2') + end + + it 'creates a draft from the tool' do + expect(page).to have_content('Tool Draft Created Successfully!') + expect(Draft.where(provider_id: 'MMT_2').size).to eq(1) + end + end + end + + context 'when visiting the clone path directly' do + before do + clone_link = page.current_path + '/clone' + visit clone_link + end + + it 'displays warning banner link to change provider' do + expect(page).to have_css('.eui-banner--warn') + expect(page).to have_content('You need to change your current provider to clone this tool') + end + + context 'when clicking the warning banner link' do + before do + click_link('You need to change your current provider to clone this tool') + wait_for_jQuery + end + + it 'switches the provider context' do + expect(User.first.provider_id).to eq('MMT_2') + end + + it 'creates a draft from the tool' do + expect(page).to have_content('Records must have a unique Name and Long Name within a provider. Click here to enter a new Name and Long Name.') + expect(Draft.where(provider_id: 'MMT_2').size).to eq(1) + end + end + end + end + end + + 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') + + visit tool_path(@ingested_not_available_provider_tool['concept-id']) + 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') + end + + context 'when trying to visit the action paths directly' do + context 'when visiting the edit path directly' do + before do + edit_link = page.current_path + '/edit' + visit edit_link + end + + it 'displays the no permissions banner message' do + expect(page).to have_css('.eui-banner--danger') + expect(page).to have_content("You don't have the appropriate permissions to edit this tool") + end + + it 'displays the Access Denied message' do + expect(page).to have_content('Access Denied') + expect(page).to have_content('It appears you do not have access to edit this content.') + end + end + + context 'when visiting the clone path directly' do + before do + clone_link = page.current_path + '/clone' + visit clone_link + end + + it 'displays the no permissions banner message' do + expect(page).to have_css('.eui-banner--danger') + expect(page).to have_content("You don't have the appropriate permissions to clone this tool") + end + + it 'displays the Access Denied message' do + expect(page).to have_content('Access Denied') + expect(page).to have_content('It appears you do not have access to clone this content.') + end + end + end + end + end +end From e1394af65ad85069f9dfe186b3fdf0b723ee10c1 Mon Sep 17 00:00:00 2001 From: Christian Trummer Date: Thu, 9 Jul 2020 13:28:45 -0400 Subject: [PATCH 05/34] added display parameter to rake task --- lib/tasks/translate_collections.rake | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/tasks/translate_collections.rake b/lib/tasks/translate_collections.rake index 3b73529dc..bd6a1de55 100644 --- a/lib/tasks/translate_collections.rake +++ b/lib/tasks/translate_collections.rake @@ -2,10 +2,11 @@ require 'libxml_to_hash' namespace :collection do desc 'Translate a collection from native format to UMM JSON and back to native format' - task :translate, [:file, :format, :version] => :environment do |_task, args| - args.with_defaults(version: '1.15.3') + task :translate, [:file, :format, :disp, :version] => :environment do |_task, args| + args.with_defaults(:version => '1.15.3') + args.with_defaults(:disp => 'show') - puts 'INVALID FORMAT' if !args.format.eql?('echo10' && 'dif10' && 'iso19115') + puts 'FORMAT INVALID' unless args.format == ('echo10' || 'dif10' || 'iso19115') filename = args.file.split('/')[-1] puts "\nTranslating #{filename} to UMM JSON..." @@ -27,11 +28,12 @@ namespace :collection do # nokogiri output o = Nokogiri::XML(native_original_xml) { |config| config.strict.noblanks } c = Nokogiri::XML(native_converted_xml) { |config| config.strict.noblanks } - - o.diff(c, {:added => true, :removed => true}) do |change,node| - next if path_leads_to_list?(node.parent.path, native_original_hash, native_converted_hash) - puts "#{change}: " + node.parent.path - end + # puts args.display, args.display.class + o.diff(c, {:added => true, :removed => true}) do |change,node| + next if path_leads_to_list?(node.parent.path, native_original_hash, native_converted_hash) + puts("#{change}: #{node.to_xml}".ljust(60) + node.parent.path) if args.disp.eql? 'show' + puts("#{change}: ". + node.parent.path) if args.disp.eql? 'hide' + end # find differences dif_hash = find_difference_bt_hashes(native_original_hash, native_converted_hash) From 31b1c59fccbe3e383fb792ac013aade001011eb9 Mon Sep 17 00:00:00 2001 From: William Valencia Date: Fri, 10 Jul 2020 04:08:58 -0400 Subject: [PATCH 06/34] 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 ebb408b8e333cbf522030ffeb7033bff59f2adef Mon Sep 17 00:00:00 2001 From: Christian Trummer Date: Wed, 8 Jul 2020 11:40:11 -0400 Subject: [PATCH 07/34] MMT-2311: Added a translate_collections.rake file and added nokogiri/diff to the gemfile --- Gemfile | 2 + Gemfile.lock | 5 + lib/tasks/local_cmr.rake | 8 +- lib/tasks/translate_collections.rake | 275 +++++++++++++++++++++++++++ package-lock.json | 14 ++ 5 files changed, 300 insertions(+), 4 deletions(-) create mode 100644 lib/tasks/translate_collections.rake create mode 100644 package-lock.json diff --git a/Gemfile b/Gemfile index f7b917df1..f7c4b2a05 100644 --- a/Gemfile +++ b/Gemfile @@ -59,6 +59,8 @@ gem 'kaminari' gem 'momentjs-rails' # js lib for dates gem 'pundit' +gem 'nokogiri-diff', '~> 0.2.0' # for comparing xml documents + gem 'activerecord-import' # bulk insertion of data gem 'activerecord-session_store' diff --git a/Gemfile.lock b/Gemfile.lock index 61ce28ff4..493f42aa4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -195,6 +195,9 @@ GEM nio4r (2.5.2) nokogiri (1.10.9) mini_portile2 (~> 2.4.0) + nokogiri-diff (0.2.0) + nokogiri (~> 1.5) + tdiff (~> 0.3, >= 0.3.2) parallel (1.19.1) parser (2.7.1.2) ast (~> 2.4.0) @@ -297,6 +300,7 @@ GEM activesupport (>= 4.0) sprockets (>= 3.0.0) sqlite3 (1.4.2) + tdiff (0.3.4) thor (1.0.1) thread_safe (0.3.6) tilt (2.0.10) @@ -365,6 +369,7 @@ DEPENDENCIES mini_racer momentjs-rails multi_xml + nokogiri-diff (~> 0.2.0) pg pundit rack_session_access diff --git a/lib/tasks/local_cmr.rake b/lib/tasks/local_cmr.rake index 4e5689b9b..1930a95e2 100644 --- a/lib/tasks/local_cmr.rake +++ b/lib/tasks/local_cmr.rake @@ -11,8 +11,8 @@ namespace :cmr do # use puts %x() to run commands and not Process.spawn(), which returns # imediatly, because curl can take a while and we want to see the output # from standard out. - - # make a backup copy of the old jar if it exists, and a backup of the backup + + # make a backup copy of the old jar if it exists, and a backup of the backup cmd_backup = 'cd cmr ; '\ "if [ -a \"#{jar_name}\" ] ; then "\ 'echo Backup jar... ; '\ @@ -20,7 +20,7 @@ namespace :cmr do "mv -fv #{jar_name} #{jar_name}.last ; "\ 'fi' puts %x( #{cmd_backup} ) - + # download a new jar to a temp location and rename it if successfull puts 'Download jar...' cmd_fetch = 'cd cmr ; '\ @@ -247,4 +247,4 @@ namespace :cmr do puts "Done!" end end -end \ No newline at end of file +end diff --git a/lib/tasks/translate_collections.rake b/lib/tasks/translate_collections.rake new file mode 100644 index 000000000..3b73529dc --- /dev/null +++ b/lib/tasks/translate_collections.rake @@ -0,0 +1,275 @@ +require 'libxml_to_hash' + +namespace :collection do + desc 'Translate a collection from native format to UMM JSON and back to native format' + task :translate, [:file, :format, :version] => :environment do |_task, args| + args.with_defaults(version: '1.15.3') + + puts 'INVALID FORMAT' if !args.format.eql?('echo10' && 'dif10' && 'iso19115') + + filename = args.file.split('/')[-1] + puts "\nTranslating #{filename} to UMM JSON..." + + native_original_xml = File.read(args.file) + native_original_hash = Hash.from_xml(native_original_xml) + + #translate to UMM + umm = cmr_client.translate_collection(native_original_xml, "application/#{args.format}+xml", "application/vnd.nasa.cmr.umm+json;version=#{args.version}", skip_validation=true ) + umm_json = umm.body.to_json + umm.success? ? puts("\nsuccessful translation to UMM") : puts("\nUMM translation failure") + + # translate back to native + back_to_native = cmr_client.translate_collection(umm_json, "application/vnd.nasa.cmr.umm+json;version=#{args.version}", "application/#{args.format}+xml", skip_validation=true ) + native_converted_hash = Hash.from_xml(back_to_native.body) + native_converted_xml = back_to_native.body + back_to_native.success? ? puts("successful translation to native format \n\n") : puts("Native format translation failure \n\n") + + # nokogiri output + o = Nokogiri::XML(native_original_xml) { |config| config.strict.noblanks } + c = Nokogiri::XML(native_converted_xml) { |config| config.strict.noblanks } + + o.diff(c, {:added => true, :removed => true}) do |change,node| + next if path_leads_to_list?(node.parent.path, native_original_hash, native_converted_hash) + puts "#{change}: " + node.parent.path + end + + # find differences + dif_hash = find_difference_bt_hashes(native_original_hash, native_converted_hash) + compare_arrays(dif_hash, native_original_hash, native_converted_hash) + + end + + def path_leads_to_list?(path, org_hash, conv_hash) + org_hash_path = hash_navigation(path, org_hash) + conv_hash_path = hash_navigation(path, conv_hash) + + if org_hash_path == 'flag' || conv_hash_path == 'flag' + return false + end + + if path.include?("[") && path.include?("]") + bool = true + elsif org_hash_path.is_a?(Hash) && conv_hash_path.is_a?(Hash) + org_hash_path.keys.each { |key| bool = true; break if org_hash_path[key].is_a?(Array) } + conv_hash_path.keys.each { |key| bool = true; break if conv_hash_path[key].is_a?(Array) } + elsif org_hash_path.is_a?(Array) || conv_hash_path.is_a?(Array) + bool = true + else + bool = false + end + bool + end + + def hash_navigation(dir, hash) + dir = dir.split("/") + if dir.is_a? Array + dir.each do |key| + if !key.empty? && hash.is_a?(Hash) + hash = hash[key] + elsif hash.is_a? Array + return 'flag' + end + end + else + hash = hash[dir] + end + hash + end + + def get_list_paths(dif_hash, original, converted) + values_list = hash_to_list_of_values(dif_hash) + paths = Array.new + + for item in values_list + org_path = get_dir(item, original) + conv_path = get_dir(item, converted) + + if org_path.include? "[]" + path = org_path + elsif conv_path.include? "[]" + path = conv_path + else + path = org_path #arbitrary + end + + if path.include? "[]" + path = path.split "[]" + paths << path[0] unless paths.any? { |p| p.eql? path[0] } + elsif path_leads_to_list?(path, original, converted) + paths << path unless paths.any? { |p| p.eql? path } + end + end + paths + end + + def compare_arrays(diff_hash, original_hash, converted_hash) + dif_hash = diff_hash.clone + original = original_hash.clone + converted = converted_hash.clone + paths = get_list_paths(dif_hash, original, converted) + + paths.each do |path| + org_array = hash_navigation(path, original) + org_arr = org_array.clone + conv_array = hash_navigation(path, converted) + conv_arr = conv_array.clone + + org_arr = Array.wrap(org_arr) unless org_arr.is_a?(Array) + org_array = Array.wrap(org_array) unless org_array.is_a?(Array) + conv_arr = Array.wrap(conv_arr) unless conv_arr.is_a?(Array) + conv_array = Array.wrap(conv_array) unless conv_array.is_a?(Array) + + for conv_item in conv_array + for org_item in org_array + if org_item.eql? conv_item + org_arr.delete(org_item) + break + end + end + end + + for org_item in org_array + for conv_item in conv_array + if org_item.eql? conv_item + conv_arr.delete(conv_item) + break + end + end + end + + org_arr.each do |item| + path_with_index = path + "[#{org_array.index(item)}]" + puts "-: ". + path_with_index + end + + conv_arr.each do |item| + path_with_index = path + "[#{conv_array.index(item)}]" + puts "+: " + path_with_index #THIS INDEX DOESN'T MAKE SENSE + end + end + end + + def find_difference_bt_hash_arrays(org_arr, conv_arr) + org = org_arr.clone + conv = conv_arr.clone + missing = Array.new + if org.eql? conv + return missing + else + for conv_item in conv + for org_item in org + if org_item.eql? conv_item + org.delete(conv_item) + break + end + end + end + missing += org + end + missing + end + + def find_difference_bt_hashes(org, conv) + missing = Hash.new + if org.eql? conv + return missing + else + org.each do |org_key,org_value| + conv_value = conv[org_key] + if conv.key? org_key + if conv_value.eql? org_value + next + elsif org_value.is_a?(Hash) && conv_value.is_a?(Hash) + missing_value = find_difference_bt_hashes(org_value, conv_value) + unless missing_value.empty? + missing[org_key] = missing_value + end + elsif org_value.is_a?(Array) && conv_value.is_a?(Array) + missing_value = find_difference_bt_hash_arrays(org_value, conv_value) + unless missing_value.empty? + missing[org_key] = missing_value + end + else + missing[org_key] = org_value + end + else + missing[org_key] = org_value + end + end + end + missing + end + + def get_dir(value, hash_or_arr) + iterable = hash_or_arr.clone + dir = String.new + if iterable.is_a? Hash + unless iterable.key(value).nil? + matching_key = iterable.key(value) + dir += '/' + matching_key + iterable.delete(matching_key) + return dir + else + iterable.each do |key,val| + if val.is_a?(Hash) && hash_to_list_of_values(val).include?(value) + dir += '/' + key + dir += get_dir(value, val) + return dir + elsif val.is_a?(Array) && array_to_list_of_values(val).include?(value) + dir += '/' + key + "[]" + dir += get_dir(value, val) + return dir + elsif val.eql? value + dir += '/' + key + iterable.delete(key) + return dir + end + end + end + elsif iterable.is_a? Array + iterable.each do |item| + if item.is_a?(Hash) && hash_to_list_of_values(item).include?(value) + dir += get_dir(value,item) + return dir + elsif item.is_a?(Array) && array_to_list_of_values(item).include?(value) + puts "\n\n\n\n\n\n\n\n USED THIS SECTION \n\n\n\n\n\n\n\n\n\n\n\n" + dir += get_dir(value,item) + "[]" + return dir + end + end + end + dir + end + + def hash_to_list_of_values(hash) + list = Array.new + for val in hash.values + if val.is_a? Hash + list += hash_to_list_of_values(val) + elsif val.is_a? Array + list += array_to_list_of_values(val) + else + list << val + end + end + list + end + + def array_to_list_of_values(array) + ls = Array.new + for item in array + if item.is_a? Hash + ls += hash_to_list_of_values(item) + elsif item.is_a? Array + ls += array_to_list_of_values(item) + else + ls << item + end + end + ls + end + + def cmr_client + @cmr_client ||= Cmr::Client.client_for_environment(Rails.configuration.cmr_env, Rails.configuration.services) + end +end diff --git a/package-lock.json b/package-lock.json new file mode 100644 index 000000000..eec54849c --- /dev/null +++ b/package-lock.json @@ -0,0 +1,14 @@ +{ + "name": "mmt", + "version": "1.0.0", + "lockfileVersion": 1, + "requires": true, + "dependencies": { + "coffee-script": { + "version": "1.12.7", + "resolved": "https://registry.npmjs.org/coffee-script/-/coffee-script-1.12.7.tgz", + "integrity": "sha512-fLeEhqwymYat/MpTPUjSKHVYYl0ec2mOyALEMLmzr5i1isuG+6jfI2j2d5oBO3VIzgUXgBVIcOT9uH1TFxBckw==", + "dev": true + } + } +} From 6018f8e315d150e5fd4a01f3f9e11fb36f34be3b Mon Sep 17 00:00:00 2001 From: Christian Trummer Date: Thu, 9 Jul 2020 13:28:45 -0400 Subject: [PATCH 08/34] MMT-2311: added display parameter to rake task --- lib/tasks/translate_collections.rake | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/tasks/translate_collections.rake b/lib/tasks/translate_collections.rake index 3b73529dc..bd6a1de55 100644 --- a/lib/tasks/translate_collections.rake +++ b/lib/tasks/translate_collections.rake @@ -2,10 +2,11 @@ require 'libxml_to_hash' namespace :collection do desc 'Translate a collection from native format to UMM JSON and back to native format' - task :translate, [:file, :format, :version] => :environment do |_task, args| - args.with_defaults(version: '1.15.3') + task :translate, [:file, :format, :disp, :version] => :environment do |_task, args| + args.with_defaults(:version => '1.15.3') + args.with_defaults(:disp => 'show') - puts 'INVALID FORMAT' if !args.format.eql?('echo10' && 'dif10' && 'iso19115') + puts 'FORMAT INVALID' unless args.format == ('echo10' || 'dif10' || 'iso19115') filename = args.file.split('/')[-1] puts "\nTranslating #{filename} to UMM JSON..." @@ -27,11 +28,12 @@ namespace :collection do # nokogiri output o = Nokogiri::XML(native_original_xml) { |config| config.strict.noblanks } c = Nokogiri::XML(native_converted_xml) { |config| config.strict.noblanks } - - o.diff(c, {:added => true, :removed => true}) do |change,node| - next if path_leads_to_list?(node.parent.path, native_original_hash, native_converted_hash) - puts "#{change}: " + node.parent.path - end + # puts args.display, args.display.class + o.diff(c, {:added => true, :removed => true}) do |change,node| + next if path_leads_to_list?(node.parent.path, native_original_hash, native_converted_hash) + puts("#{change}: #{node.to_xml}".ljust(60) + node.parent.path) if args.disp.eql? 'show' + puts("#{change}: ". + node.parent.path) if args.disp.eql? 'hide' + end # find differences dif_hash = find_difference_bt_hashes(native_original_hash, native_converted_hash) From 22a9fb8a916127df1810197bce3778f0f1fbfd00 Mon Sep 17 00:00:00 2001 From: Ryan Miller Date: Fri, 10 Jul 2020 14:07:24 -0400 Subject: [PATCH 09/34] 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 10/34] 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 11/34] 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 f58c6ad91a9c592032ef994423e44b676a277f2e Mon Sep 17 00:00:00 2001 From: ryanmiller-1 <40173609+ryanmiller-1@users.noreply.github.com> Date: Mon, 13 Jul 2020 07:51:45 -0400 Subject: [PATCH 12/34] MMT-2234 (#607) adding download json for tools --- app/controllers/tools_controller.rb | 2 +- app/views/tools/show.html.erb | 3 +- config/routes.rb | 1 + .../collections/downloading_xml_spec.rb | 16 +++---- spec/features/services/download_json_spec.rb | 18 ++++++++ spec/features/tools/download_json_spec.rb | 42 +++++++++++++++++++ spec/features/variables/download_json_spec.rb | 18 ++++++++ 7 files changed, 90 insertions(+), 10 deletions(-) create mode 100644 spec/features/tools/download_json_spec.rb diff --git a/app/controllers/tools_controller.rb b/app/controllers/tools_controller.rb index 2c5686dbe..6402e8db0 100644 --- a/app/controllers/tools_controller.rb +++ b/app/controllers/tools_controller.rb @@ -2,7 +2,7 @@ class ToolsController < BasePublishedRecordController include ManageMetadataHelper - before_action :set_tool, only: [:show, :edit, :clone, :revisions] #, :destroy, :revert, :download_json] + before_action :set_tool, only: [:show, :edit, :clone, :revisions, :download_json] #, :destroy, :revert] 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] diff --git a/app/views/tools/show.html.erb b/app/views/tools/show.html.erb index 14240393e..72f91d48c 100644 --- a/app/views/tools/show.html.erb +++ b/app/views/tools/show.html.erb @@ -98,8 +98,7 @@ <%= link_to 'Clone Tool Record', '#not-current-provider-modal', class: 'display-modal not-current-provider eui-btn--link bar-after', data: { 'provider': @provider_id, record_action: 'clone-tool' } %> <% end %> - <%= 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 '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) %> diff --git a/config/routes.rb b/config/routes.rb index ce35fbe4d..619d95aff 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -106,6 +106,7 @@ resources :tools, only: [:show, :create, :edit] get '/tools/:id/clone' => 'tools#clone', as: 'clone_tool' get '/tools/:id/revisions' => 'tools#revisions', as: 'tool_revisions' + get '/tools/:id/download_json(/:revision_id)' => 'tools#download_json', as: 'download_json_tool' resources :variable_drafts, controller: 'variable_drafts', draft_type: 'VariableDraft' do member do diff --git a/spec/features/collections/downloading_xml_spec.rb b/spec/features/collections/downloading_xml_spec.rb index 81ed7e69b..60107c477 100644 --- a/spec/features/collections/downloading_xml_spec.rb +++ b/spec/features/collections/downloading_xml_spec.rb @@ -26,16 +26,17 @@ before do @file = "#{Rails.root}/#{@concept_id}.echo10" click_on 'ECHO 10' - end - after do # Seems to need a brief (>0.01) pause to actually find the file. sleep(0.1) + end + + after do FileUtils.rm @file if File.exist?(@file) end it 'downloads the file' do - expect(File.exist?(@file)) + expect(File.exist?(@file)).to eq(true) end end end @@ -45,7 +46,7 @@ before do login visit manage_collections_path - + short_name = 'SPL4SMAU' fill_in 'keyword', with: short_name click_on 'Search Collections' @@ -76,16 +77,17 @@ before do @file = "#{Rails.root}/#{@concept_id}.iso-smap" click_on 'ISO 19115 (SMAP) (Native)' - end - after do # Seems to need a brief (>0.01) pause to actually find the file. sleep(0.1) + end + + after do FileUtils.rm @file if File.exist?(@file) end it 'downloads the file' do - expect(File.exist?(@file)) + expect(File.exist?(@file)).to eq(true) end end end diff --git a/spec/features/services/download_json_spec.rb b/spec/features/services/download_json_spec.rb index 7f9f8bb42..52299efd0 100644 --- a/spec/features/services/download_json_spec.rb +++ b/spec/features/services/download_json_spec.rb @@ -13,5 +13,23 @@ it 'renders the download link' do expect(page).to have_link('Download JSON', href: download_json_service_path(@ingest_response['concept-id'])) end + + context 'when downloading the json' do + before do + @file = "#{Rails.root}/#{@ingest_response['concept-id']}.json" + click_on 'Download JSON' + + # Seems to need a brief (>0.01) pause to actually find the file. + sleep(0.1) + end + + after do + FileUtils.rm @file if File.exist?(@file) + end + + it 'downloads the file' do + expect(File.exist?(@file)).to eq(true) + end + end end end diff --git a/spec/features/tools/download_json_spec.rb b/spec/features/tools/download_json_spec.rb new file mode 100644 index 000000000..b52ee6006 --- /dev/null +++ b/spec/features/tools/download_json_spec.rb @@ -0,0 +1,42 @@ +describe 'Downloading Tool JSON', js: true do + before :all do + @ingest_response, _concept_response, @native_id = publish_tool_draft + end + + # TODO: remove after CMR-6332 + after :all do + delete_response = cmr_client.delete_tool('MMT_2', @native_id, 'token') + + raise unless delete_response.success? + end + + context 'when viewing the tool preview page' do + before do + login + + visit tool_path(@ingest_response['concept-id']) + end + + it 'renders the download link' do + expect(page).to have_link('Download JSON', href: download_json_tool_path(@ingest_response['concept-id'])) + end + + context 'when downloading the json' do + before do + @file = "#{Rails.root}/#{@ingest_response['concept-id']}.json" + click_on 'Download JSON' + + # Seems to need a brief (>0.01) pause to actually find the file. + sleep(0.1) + end + + after do + FileUtils.rm @file if File.exist?(@file) + end + + it 'downloads the file' do + expect(File.exist?(@file)).to eq(true) + end + end + end +end diff --git a/spec/features/variables/download_json_spec.rb b/spec/features/variables/download_json_spec.rb index 6ac90d28d..739d26e62 100644 --- a/spec/features/variables/download_json_spec.rb +++ b/spec/features/variables/download_json_spec.rb @@ -13,5 +13,23 @@ it 'renders the download link' do expect(page).to have_link('Download JSON', href: download_json_variable_path(@ingest_response['concept-id'])) end + + context 'when downloading the json' do + before do + @file = "#{Rails.root}/#{@ingest_response['concept-id']}.json" + click_on 'Download JSON' + + # Seems to need a brief (>0.01) pause to actually find the file. + sleep(0.1) + end + + after do + FileUtils.rm @file if File.exist?(@file) + end + + it 'downloads the file' do + expect(File.exist?(@file)).to eq(true) + end + end end end From 4fa3f5cfae2125dd3e99ffbba985747c5721c4d6 Mon Sep 17 00:00:00 2001 From: ryanmiller-1 <40173609+ryanmiller-1@users.noreply.github.com> Date: Mon, 13 Jul 2020 11:11:06 -0400 Subject: [PATCH 13/34] Hotfix/MMT-2231-1 Updating revisions helpers to sort by revision date (#615) * MMT-2231 updated revisions testing * MMT-2231 updating collection sorting and tests --- app/concerns/cmr_collections_helper.rb | 2 +- app/controllers/manage_metadata_controller.rb | 6 ++--- .../collections/revision_list_spec.rb | 18 +++++++------ spec/features/services/revision_list_spec.rb | 25 +++++++++++++------ spec/features/tools/revision_list_spec.rb | 16 +++++++++--- 5 files changed, 45 insertions(+), 22 deletions(-) diff --git a/app/concerns/cmr_collections_helper.rb b/app/concerns/cmr_collections_helper.rb index f7ea2ad96..4e8502eec 100644 --- a/app/concerns/cmr_collections_helper.rb +++ b/app/concerns/cmr_collections_helper.rb @@ -6,7 +6,7 @@ def get_revisions(concept_id, revision_id) # try again because CMR might be a little slow to index if it is a newly published revision attempts = 0 while attempts < 20 - revisions_response = cmr_client.get_collections({ concept_id: concept_id, all_revisions: true, include_granule_counts: true }, token) + revisions_response = cmr_client.get_collections({ concept_id: concept_id, all_revisions: true, include_granule_counts: true, sort_key: '-revision_date' }, token) revisions = if revisions_response.success? revisions_response.body.fetch('items', []) else diff --git a/app/controllers/manage_metadata_controller.rb b/app/controllers/manage_metadata_controller.rb index a7d98f700..cdb465b8a 100644 --- a/app/controllers/manage_metadata_controller.rb +++ b/app/controllers/manage_metadata_controller.rb @@ -110,7 +110,7 @@ def set_variable_information # if the variable is not found, try again because CMR might be a little slow to index if it is a newly published record attempts = 0 while attempts < 20 - variables_search_response = cmr_client.get_variables(concept_id: @concept_id, all_revisions: true) + variables_search_response = cmr_client.get_variables(concept_id: @concept_id, all_revisions: true, sort_key: '-revision_date') variable_data = if variables_search_response.success? variables_search_response.body.fetch('items', []) @@ -163,7 +163,7 @@ def set_service_information # if the service is not found, try again because CMR might be a little slow to index if it is a newly published record attempts = 0 while attempts < 20 - services_search_response = cmr_client.get_services(concept_id: @concept_id, all_revisions: true) + services_search_response = cmr_client.get_services(concept_id: @concept_id, all_revisions: true, sort_key: '-revision_date') service_data = if services_search_response.success? services_search_response.body.fetch('items', []) @@ -218,7 +218,7 @@ def set_tool_information # if the tool is not found, try again because CMR might be a little slow to index if it is a newly published record attempts = 0 while attempts < 20 - tools_search_response = cmr_client.get_tools(concept_id: @concept_id, all_revisions: true) + tools_search_response = cmr_client.get_tools(concept_id: @concept_id, all_revisions: true, sort_key: '-revision_date') tool_data = if tools_search_response.success? tools_search_response.body.fetch('items', []) diff --git a/spec/features/collections/revision_list_spec.rb b/spec/features/collections/revision_list_spec.rb index 9d3c57bb9..60ba78a42 100644 --- a/spec/features/collections/revision_list_spec.rb +++ b/spec/features/collections/revision_list_spec.rb @@ -1,15 +1,19 @@ describe 'Revision list', js: true do context 'when viewing a published collection' do + before :all do + native_id = 'collection_revision_native_id' + _ingest_response, _concept_response = publish_collection_draft(native_id: native_id, revision_count: 10, short_name: 'b_test_01') + @ingest_response, @concept_response = publish_collection_draft(native_id: native_id, short_name: 'c_test_01') + end + before do login - ingest_response, @concept_response = publish_collection_draft(revision_count: 2) - - visit collection_path(ingest_response['concept-id']) + visit collection_path(@ingest_response['concept-id']) end it 'displays the number of revisions' do - expect(page).to have_content('Revisions (2)') + expect(page).to have_content('Revisions (10)') end context 'when clicking on the revision link' do @@ -27,15 +31,15 @@ end it 'displays when the revision was made' do - expect(page).to have_content(today_string, count: 2) + expect(page).to have_content(today_string, count: 10) end it 'displays what user made the revision' do - expect(page).to have_content('typical', count: 2) + expect(page).to have_content('typical', count: 10) end it 'displays the correct phrasing for reverting records' do - expect(page).to have_content('Revert to this Revision', count: 1) + expect(page).to have_content('Revert to this Revision', count: 9) end context 'when viewing an old revision' do diff --git a/spec/features/services/revision_list_spec.rb b/spec/features/services/revision_list_spec.rb index 6fbe57f1d..b560aad7f 100644 --- a/spec/features/services/revision_list_spec.rb +++ b/spec/features/services/revision_list_spec.rb @@ -1,15 +1,22 @@ describe 'Service revision list', reset_provider: true, js: true do context 'when viewing a published service' do + before :all do + # CMR does not return revisions sorted by revision_id. It sorts + # by name first (and maybe other things). If the sort_key is working + # correctly, the last revision (c_test_01), should be visible on the page + native_id = 'service_revision_native_id' + _ingest_response, _concept_response = publish_service_draft(native_id: native_id, revision_count: 10, name: 'b_test_01') + @ingest_response, @concept_response = publish_service_draft(native_id: native_id, name: 'c_test_01') + end + before do login - ingest_response, @concept_response = publish_service_draft(revision_count: 2) - - visit service_path(ingest_response['concept-id']) + visit service_path(@ingest_response['concept-id']) end it 'displays the number of revisions' do - expect(page).to have_content('Revisions (2)') + expect(page).to have_content('Revisions (10)') end context 'when clicking on the revision link' do @@ -27,15 +34,19 @@ end it 'displays when the revision was made' do - expect(page).to have_content(today_string, count: 2) + expect(page).to have_content(today_string, count: 10) end it 'displays what user made the revision' do - expect(page).to have_content('typical', count: 2) + expect(page).to have_content('typical', count: 10) + end + + it 'displays the most recent revisions' do + expect(page).to have_content('11 - Published') end it 'displays the correct phrasing for reverting records' do - expect(page).to have_content('Revert to this Revision', count: 1) + expect(page).to have_content('Revert to this Revision', count: 9) end context 'when viewing an old revision' do diff --git a/spec/features/tools/revision_list_spec.rb b/spec/features/tools/revision_list_spec.rb index b33ba78f1..0ef91f2a9 100644 --- a/spec/features/tools/revision_list_spec.rb +++ b/spec/features/tools/revision_list_spec.rb @@ -1,7 +1,11 @@ describe 'Tool revision list', reset_provider: true, js: true do context 'when viewing a published tool' do before :all do - @ingest_response, @concept_response, @native_id = publish_tool_draft(revision_count: 2) + # CMR does not return revisions sorted by revision_id. It sorts + # by name first (and maybe other things). If the sort_key is working + # correctly, the last revision (c_test_01), should be visible on the page + _ingest_response, _concept_response, @native_id = publish_tool_draft(revision_count: 10, name: 'b_test_01') + @ingest_response, @concept_response, _native_id = publish_tool_draft(native_id: @native_id, name: 'c_test_01') end # TODO: remove after CMR-6332 @@ -18,7 +22,7 @@ end it 'displays the number of revisions' do - expect(page).to have_content('Revisions (2)') + expect(page).to have_content('Revisions (10)') end context 'when clicking on the revision link' do @@ -36,11 +40,15 @@ end it 'displays when the revision was made' do - expect(page).to have_content(today_string, count: 2) + expect(page).to have_content(today_string, count: 10) end it 'displays what user made the revision' do - expect(page).to have_content('typical', count: 2) + expect(page).to have_content('typical', count: 10) + end + + it 'displays the most recent revisions' do + expect(page).to have_content('11 - Published') end # TODO: Uncomment in MMT-2233 From 3d6788b679165150a668a31ff62cb123cd6019b6 Mon Sep 17 00:00:00 2001 From: Chris Trummer <42478387+ChristianTrummer99@users.noreply.github.com> Date: Mon, 13 Jul 2020 17:10:14 -0400 Subject: [PATCH 14/34] Delete package-lock.json --- package-lock.json | 14 -------------- 1 file changed, 14 deletions(-) delete mode 100644 package-lock.json diff --git a/package-lock.json b/package-lock.json deleted file mode 100644 index eec54849c..000000000 --- a/package-lock.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "name": "mmt", - "version": "1.0.0", - "lockfileVersion": 1, - "requires": true, - "dependencies": { - "coffee-script": { - "version": "1.12.7", - "resolved": "https://registry.npmjs.org/coffee-script/-/coffee-script-1.12.7.tgz", - "integrity": "sha512-fLeEhqwymYat/MpTPUjSKHVYYl0ec2mOyALEMLmzr5i1isuG+6jfI2j2d5oBO3VIzgUXgBVIcOT9uH1TFxBckw==", - "dev": true - } - } -} From c9ea01ded9f4d63b65206a76df9f25939faf738e Mon Sep 17 00:00:00 2001 From: Christian Trummer Date: Mon, 13 Jul 2020 17:47:13 -0400 Subject: [PATCH 15/34] MMT-2311: made changes as per PR change requests --- .gitignore | 3 +++ lib/tasks/translate_collections.rake | 32 +++++++++++++--------------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/.gitignore b/.gitignore index 79593f013..f83837939 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,9 @@ # Ignore bundler config. /.bundle +# Ignore package-lock.json +package-lock.json + # Ignore the default SQLite database. /db/*.sqlite3 /db/*.sqlite3-journal diff --git a/lib/tasks/translate_collections.rake b/lib/tasks/translate_collections.rake index bd6a1de55..ab341fedd 100644 --- a/lib/tasks/translate_collections.rake +++ b/lib/tasks/translate_collections.rake @@ -6,7 +6,7 @@ namespace :collection do args.with_defaults(:version => '1.15.3') args.with_defaults(:disp => 'show') - puts 'FORMAT INVALID' unless args.format == ('echo10' || 'dif10' || 'iso19115') + puts 'FORMAT INVALID' unless args.format.eql? ('echo10' || 'dif10' || 'iso19115') filename = args.file.split('/')[-1] puts "\nTranslating #{filename} to UMM JSON..." @@ -15,25 +15,25 @@ namespace :collection do native_original_hash = Hash.from_xml(native_original_xml) #translate to UMM - umm = cmr_client.translate_collection(native_original_xml, "application/#{args.format}+xml", "application/vnd.nasa.cmr.umm+json;version=#{args.version}", skip_validation=true ) - umm_json = umm.body.to_json - umm.success? ? puts("\nsuccessful translation to UMM") : puts("\nUMM translation failure") + umm_response = cmr_client.translate_collection(native_original_xml, "application/#{args.format}+xml", "application/vnd.nasa.cmr.umm+json;version=#{args.version}", skip_validation=true ) + umm_json = umm_response.body.to_json + umm_response.success? ? puts("\nsuccessful translation to UMM") : abort("\nUMM translation failure") # translate back to native back_to_native = cmr_client.translate_collection(umm_json, "application/vnd.nasa.cmr.umm+json;version=#{args.version}", "application/#{args.format}+xml", skip_validation=true ) native_converted_hash = Hash.from_xml(back_to_native.body) native_converted_xml = back_to_native.body - back_to_native.success? ? puts("successful translation to native format \n\n") : puts("Native format translation failure \n\n") + back_to_native.success? ? puts("successful translation to native format \n\n") : abort("Native format translation failure \n\n") # nokogiri output - o = Nokogiri::XML(native_original_xml) { |config| config.strict.noblanks } - c = Nokogiri::XML(native_converted_xml) { |config| config.strict.noblanks } - # puts args.display, args.display.class - o.diff(c, {:added => true, :removed => true}) do |change,node| - next if path_leads_to_list?(node.parent.path, native_original_hash, native_converted_hash) - puts("#{change}: #{node.to_xml}".ljust(60) + node.parent.path) if args.disp.eql? 'show' - puts("#{change}: ". + node.parent.path) if args.disp.eql? 'hide' - end + nokogiri_original = Nokogiri::XML(native_original_xml) { |config| config.strict.noblanks } + nokogiri_converted = Nokogiri::XML(native_converted_xml) { |config| config.strict.noblanks } + + nokogiri_original.diff(nokogiri_converted, {:added => true, :removed => true}) do |change,node| + next if path_leads_to_list?(node.parent.path, native_original_hash, native_converted_hash) + puts("#{change}: #{node.to_xml}".ljust(60) + node.parent.path) if args.disp.eql? 'show' + puts("#{change}: ". + node.parent.path) if args.disp.eql? 'hide' + end # find differences dif_hash = find_difference_bt_hashes(native_original_hash, native_converted_hash) @@ -45,9 +45,7 @@ namespace :collection do org_hash_path = hash_navigation(path, org_hash) conv_hash_path = hash_navigation(path, conv_hash) - if org_hash_path == 'flag' || conv_hash_path == 'flag' - return false - end + return false if org_hash_path == false || conv_hash_path == false if path.include?("[") && path.include?("]") bool = true @@ -69,7 +67,7 @@ namespace :collection do if !key.empty? && hash.is_a?(Hash) hash = hash[key] elsif hash.is_a? Array - return 'flag' + return false end end else From 2b2fa4c42089101dcdade08b6cb7ab21580199ad Mon Sep 17 00:00:00 2001 From: Christian Trummer Date: Mon, 13 Jul 2020 18:32:21 -0400 Subject: [PATCH 16/34] MMT-2311: made changes per PR requests, added package-lock to gitignore --- lib/tasks/translate_collections.rake | 33 ++++++++++------------------ 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/lib/tasks/translate_collections.rake b/lib/tasks/translate_collections.rake index ab341fedd..0e3ea1986 100644 --- a/lib/tasks/translate_collections.rake +++ b/lib/tasks/translate_collections.rake @@ -89,9 +89,13 @@ namespace :collection do elsif conv_path.include? "[]" path = conv_path else - path = org_path #arbitrary + path = org_path end + # the get_dir method includes a clause that 'tags' array-containing fields with '[]' + # eg. '/Collection/Contacts/Contact[]/OrganizationEmails/Email' + # the following lines show how this 'tagging' is used to identify an array in a given directory + if path.include? "[]" path = path.split "[]" paths << path[0] unless paths.any? { |p| p.eql? path[0] } @@ -102,35 +106,22 @@ namespace :collection do paths end - def compare_arrays(diff_hash, original_hash, converted_hash) - dif_hash = diff_hash.clone - original = original_hash.clone - converted = converted_hash.clone + def compare_arrays(dif_hash, original, converted) paths = get_list_paths(dif_hash, original, converted) paths.each do |path| org_array = hash_navigation(path, original) - org_arr = org_array.clone conv_array = hash_navigation(path, converted) - conv_arr = conv_array.clone - org_arr = Array.wrap(org_arr) unless org_arr.is_a?(Array) + org_array.is_a?(Array) ? org_arr = Array.wrap(org_array) : org_arr = org_array.clone org_array = Array.wrap(org_array) unless org_array.is_a?(Array) - conv_arr = Array.wrap(conv_arr) unless conv_arr.is_a?(Array) + conv_array.is_a?(Array) ? conv_arr = Array.wrap(conv_array) : conv_arr = conv_array.clone conv_array = Array.wrap(conv_array) unless conv_array.is_a?(Array) for conv_item in conv_array for org_item in org_array if org_item.eql? conv_item org_arr.delete(org_item) - break - end - end - end - - for org_item in org_array - for conv_item in conv_array - if org_item.eql? conv_item conv_arr.delete(conv_item) break end @@ -144,7 +135,7 @@ namespace :collection do conv_arr.each do |item| path_with_index = path + "[#{conv_array.index(item)}]" - puts "+: " + path_with_index #THIS INDEX DOESN'T MAKE SENSE + puts "+: " + path_with_index end end end @@ -175,8 +166,7 @@ namespace :collection do return missing else org.each do |org_key,org_value| - conv_value = conv[org_key] - if conv.key? org_key + if (conv_value = conv[org_key]) if conv_value.eql? org_value next elsif org_value.is_a?(Hash) && conv_value.is_a?(Hash) @@ -204,8 +194,7 @@ namespace :collection do iterable = hash_or_arr.clone dir = String.new if iterable.is_a? Hash - unless iterable.key(value).nil? - matching_key = iterable.key(value) + if (matching_key = iterable.key(value)) dir += '/' + matching_key iterable.delete(matching_key) return dir From 9eec788ef4d036fad022c5773a6b8440d470b9b6 Mon Sep 17 00:00:00 2001 From: William Valencia Date: Mon, 13 Jul 2020 21:17:17 -0400 Subject: [PATCH 17/34] 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 18/34] 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 19/34] 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 From 0885415601203a777d632cf5284ba6d56243d880 Mon Sep 17 00:00:00 2001 From: Christian Trummer Date: Tue, 14 Jul 2020 08:38:17 -0400 Subject: [PATCH 20/34] MMT-2311: added comments to each method definition --- lib/tasks/translate_collections.rake | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/tasks/translate_collections.rake b/lib/tasks/translate_collections.rake index 0e3ea1986..ec0ae90e8 100644 --- a/lib/tasks/translate_collections.rake +++ b/lib/tasks/translate_collections.rake @@ -6,7 +6,7 @@ namespace :collection do args.with_defaults(:version => '1.15.3') args.with_defaults(:disp => 'show') - puts 'FORMAT INVALID' unless args.format.eql? ('echo10' || 'dif10' || 'iso19115') + abort 'FORMAT INVALID' unless args.format.eql? ('echo10' || 'dif10' || 'iso19115') filename = args.file.split('/')[-1] puts "\nTranslating #{filename} to UMM JSON..." @@ -42,6 +42,7 @@ namespace :collection do end def path_leads_to_list?(path, org_hash, conv_hash) + # this method takes a path string (and the full original and converted hashes) and outputs true if the path string contains a list; else false org_hash_path = hash_navigation(path, org_hash) conv_hash_path = hash_navigation(path, conv_hash) @@ -61,6 +62,8 @@ namespace :collection do end def hash_navigation(dir, hash) + # Passed a path string and the hash being navigated. This method parses the path string and + # returns the hash at the end of the path dir = dir.split("/") if dir.is_a? Array dir.each do |key| @@ -77,6 +80,8 @@ namespace :collection do end def get_list_paths(dif_hash, original, converted) + # arguments: differences hash, the original hash, and converted hash + # Using these 3 hashses, all paths that lead to a list are returned as an array of path strings values_list = hash_to_list_of_values(dif_hash) paths = Array.new @@ -107,6 +112,10 @@ namespace :collection do end def compare_arrays(dif_hash, original, converted) + # arguments: differences hash, the original hash, and converted hash + # each path that leads to an array is used to navigate to that array and + # subsequently compare the arrays in the original and converted hashes. + # there is no usable ouput; there is printing to the terminal paths = get_list_paths(dif_hash, original, converted) paths.each do |path| @@ -141,6 +150,8 @@ namespace :collection do end def find_difference_bt_hash_arrays(org_arr, conv_arr) + # array inputs; the output is an array that contains the items in the original array + # that were not found in the converted array org = org_arr.clone conv = conv_arr.clone missing = Array.new @@ -161,6 +172,9 @@ namespace :collection do end def find_difference_bt_hashes(org, conv) + # input is the original hash and the converted hash; the output is the + # 'differences hash' which represents the items in the original hash that were + # not found in the converted hash missing = Hash.new if org.eql? conv return missing @@ -191,6 +205,8 @@ namespace :collection do end def get_dir(value, hash_or_arr) + # passing the sought-after value and the hash or array being parsed + # output: a single string representing the path to the value arg passed to this method iterable = hash_or_arr.clone dir = String.new if iterable.is_a? Hash @@ -221,7 +237,6 @@ namespace :collection do dir += get_dir(value,item) return dir elsif item.is_a?(Array) && array_to_list_of_values(item).include?(value) - puts "\n\n\n\n\n\n\n\n USED THIS SECTION \n\n\n\n\n\n\n\n\n\n\n\n" dir += get_dir(value,item) + "[]" return dir end @@ -231,6 +246,7 @@ namespace :collection do end def hash_to_list_of_values(hash) + # converts a highly nested hash to a list of all its values list = Array.new for val in hash.values if val.is_a? Hash @@ -245,6 +261,7 @@ namespace :collection do end def array_to_list_of_values(array) + #converts a highly nested array to a list of all its values ls = Array.new for item in array if item.is_a? Hash From ae1845c9cfe4b226ae1a51f060dd8b7b68d71043 Mon Sep 17 00:00:00 2001 From: Christian Trummer Date: Wed, 15 Jul 2020 14:47:06 -0400 Subject: [PATCH 21/34] MMT-2311: Added nokogiri array-path-parsing feature; removed unused functions from rake task --- lib/tasks/compare_xml_collections.rake | 121 +++++++++++ lib/tasks/translate_collections.rake | 281 ------------------------- 2 files changed, 121 insertions(+), 281 deletions(-) create mode 100644 lib/tasks/compare_xml_collections.rake delete mode 100644 lib/tasks/translate_collections.rake diff --git a/lib/tasks/compare_xml_collections.rake b/lib/tasks/compare_xml_collections.rake new file mode 100644 index 000000000..dadd330ba --- /dev/null +++ b/lib/tasks/compare_xml_collections.rake @@ -0,0 +1,121 @@ +require 'libxml_to_hash' + +namespace :collection do + desc 'Translate a collection from native format to UMM JSON and back to native format' + task :loss, [:file, :format, :disp, :version] => :environment do |_task, args| + args.with_defaults(:version => '1.15.3') + args.with_defaults(:disp => 'show') + + abort 'FORMAT INVALID' unless args.format.eql? ('echo10' || 'dif10' || 'iso19115') + + filename = args.file.split('/')[-1] + puts "\nTranslating #{filename} to UMM JSON..." + + native_original_xml = File.read(args.file) + native_original_hash = Hash.from_xml(native_original_xml) + + #translate to UMM + umm_response = cmr_client.translate_collection(native_original_xml, "application/#{args.format}+xml", "application/vnd.nasa.cmr.umm+json;version=#{args.version}", skip_validation=true ) + umm_json = umm_response.body.to_json + umm_response.success? ? puts("\nsuccessful translation to UMM") : abort("\nUMM translation failure") + + # translate back to native + back_to_native = cmr_client.translate_collection(umm_json, "application/vnd.nasa.cmr.umm+json;version=#{args.version}", "application/#{args.format}+xml", skip_validation=true ) + native_converted_hash = Hash.from_xml(back_to_native.body) + native_converted_xml = back_to_native.body + back_to_native.success? ? puts("successful translation to native format \n\n") : abort("Native format translation failure \n\n") + + # nokogiri output + nokogiri_original = Nokogiri::XML(native_original_xml) { |config| config.strict.noblanks } + nokogiri_converted = Nokogiri::XML(native_converted_xml) { |config| config.strict.noblanks } + + ignored_paths = Array.new + + nokogiri_original.diff(nokogiri_converted, {:added => true, :removed => true}) do |change,node| + if node.parent.path.include?('[') && !ignored_paths.include?(node.parent.path.split('[')[0]) + ignored_paths << node.parent.path.split('[')[0] + array_comparison(node.parent.path.split('[')[0], native_original_hash, native_converted_hash).each do |item| + puts("#{item[0]}: #{item[1]}".ljust(60) + item[2]) if args.disp.eql? 'show' + puts("#{item[0]}: ". + item[2]) if args.disp.eql? 'hide' + end + elsif !ignored_paths.include?(node.parent.path.split('[')[0]) && !path_leads_to_list?(node.parent.path, native_original_hash, native_converted_hash) + puts("#{change}: #{node.to_xml}".ljust(60) + node.parent.path) if args.disp.eql? 'show' + puts("#{change}: ". + node.parent.path) if args.disp.eql? 'hide' + end + end + end + + def path_leads_to_list?(path, org_hash, conv_hash) + # this method takes a path string (and the full original and converted hashes) and outputs true if the path string contains a list; else false + org_hash_path = hash_navigation(path, org_hash) + conv_hash_path = hash_navigation(path, conv_hash) + + if path.include?("[") && path.include?("]") + bool = true + elsif org_hash_path.is_a?(Hash) && conv_hash_path.is_a?(Hash) + bool = true if org_hash_path.keys.length == 1 && org_hash_path[org_hash_path.keys[0]].is_a?(Array) + bool = true if conv_hash_path.keys.length == 1 && conv_hash_path[conv_hash_path.keys[0]].is_a?(Array) + elsif org_hash_path.is_a?(Array) || conv_hash_path.is_a?(Array) + bool = true + else + bool = false + end + bool + end + + def hash_navigation(dir, hash) + # Passed a path string and the hash being navigated. This method parses the path string and + # returns the hash at the end of the path + dir = dir.split '/' + if dir.is_a? Array + dir.each do |key| + if !key.empty? && hash.is_a?(Hash) + hash = hash[key] + elsif hash.is_a? Array + return hash + end + end + else + hash = hash[dir] + end + hash + end + + def array_comparison(path, original_hash, converted_hash) + # this is a 'less iterative' version of compare_arrays. Args: a single path, the original hash, and the converted hash. + # Rather than finding all the array paths and using those to find the array differences, the array paths are individually + # supplied by the nokogiri gem; this reduces redundancy + org_array = hash_navigation(path, original_hash) + conv_array = hash_navigation(path, converted_hash) + + org_array.is_a?(Array) ? org_arr = org_array.clone : org_arr = Array.wrap(org_array) + org_array = Array.wrap(org_array) unless org_array.is_a?(Array) + conv_array.is_a?(Array) ? conv_arr = conv_array.clone : conv_arr = Array.wrap(conv_array) + conv_array = Array.wrap(conv_array) unless conv_array.is_a?(Array) + + for conv_item in conv_array + for org_item in org_array + if org_item.eql? conv_item + org_arr.delete(org_item) + conv_arr.delete(conv_item) + break + end + end + end + + output = Array.new + org_arr.each do |item| + path_with_index = path + "[#{org_array.index(item)}]" + loss_item = ['-', item, path_with_index] + output << loss_item + end + + + conv_arr.each do |item| + path_with_index = path + "[#{conv_array.index(item)}]" + loss_item = ['+', item, path_with_index] + output << loss_item + end + output + end +end diff --git a/lib/tasks/translate_collections.rake b/lib/tasks/translate_collections.rake deleted file mode 100644 index ec0ae90e8..000000000 --- a/lib/tasks/translate_collections.rake +++ /dev/null @@ -1,281 +0,0 @@ -require 'libxml_to_hash' - -namespace :collection do - desc 'Translate a collection from native format to UMM JSON and back to native format' - task :translate, [:file, :format, :disp, :version] => :environment do |_task, args| - args.with_defaults(:version => '1.15.3') - args.with_defaults(:disp => 'show') - - abort 'FORMAT INVALID' unless args.format.eql? ('echo10' || 'dif10' || 'iso19115') - - filename = args.file.split('/')[-1] - puts "\nTranslating #{filename} to UMM JSON..." - - native_original_xml = File.read(args.file) - native_original_hash = Hash.from_xml(native_original_xml) - - #translate to UMM - umm_response = cmr_client.translate_collection(native_original_xml, "application/#{args.format}+xml", "application/vnd.nasa.cmr.umm+json;version=#{args.version}", skip_validation=true ) - umm_json = umm_response.body.to_json - umm_response.success? ? puts("\nsuccessful translation to UMM") : abort("\nUMM translation failure") - - # translate back to native - back_to_native = cmr_client.translate_collection(umm_json, "application/vnd.nasa.cmr.umm+json;version=#{args.version}", "application/#{args.format}+xml", skip_validation=true ) - native_converted_hash = Hash.from_xml(back_to_native.body) - native_converted_xml = back_to_native.body - back_to_native.success? ? puts("successful translation to native format \n\n") : abort("Native format translation failure \n\n") - - # nokogiri output - nokogiri_original = Nokogiri::XML(native_original_xml) { |config| config.strict.noblanks } - nokogiri_converted = Nokogiri::XML(native_converted_xml) { |config| config.strict.noblanks } - - nokogiri_original.diff(nokogiri_converted, {:added => true, :removed => true}) do |change,node| - next if path_leads_to_list?(node.parent.path, native_original_hash, native_converted_hash) - puts("#{change}: #{node.to_xml}".ljust(60) + node.parent.path) if args.disp.eql? 'show' - puts("#{change}: ". + node.parent.path) if args.disp.eql? 'hide' - end - - # find differences - dif_hash = find_difference_bt_hashes(native_original_hash, native_converted_hash) - compare_arrays(dif_hash, native_original_hash, native_converted_hash) - - end - - def path_leads_to_list?(path, org_hash, conv_hash) - # this method takes a path string (and the full original and converted hashes) and outputs true if the path string contains a list; else false - org_hash_path = hash_navigation(path, org_hash) - conv_hash_path = hash_navigation(path, conv_hash) - - return false if org_hash_path == false || conv_hash_path == false - - if path.include?("[") && path.include?("]") - bool = true - elsif org_hash_path.is_a?(Hash) && conv_hash_path.is_a?(Hash) - org_hash_path.keys.each { |key| bool = true; break if org_hash_path[key].is_a?(Array) } - conv_hash_path.keys.each { |key| bool = true; break if conv_hash_path[key].is_a?(Array) } - elsif org_hash_path.is_a?(Array) || conv_hash_path.is_a?(Array) - bool = true - else - bool = false - end - bool - end - - def hash_navigation(dir, hash) - # Passed a path string and the hash being navigated. This method parses the path string and - # returns the hash at the end of the path - dir = dir.split("/") - if dir.is_a? Array - dir.each do |key| - if !key.empty? && hash.is_a?(Hash) - hash = hash[key] - elsif hash.is_a? Array - return false - end - end - else - hash = hash[dir] - end - hash - end - - def get_list_paths(dif_hash, original, converted) - # arguments: differences hash, the original hash, and converted hash - # Using these 3 hashses, all paths that lead to a list are returned as an array of path strings - values_list = hash_to_list_of_values(dif_hash) - paths = Array.new - - for item in values_list - org_path = get_dir(item, original) - conv_path = get_dir(item, converted) - - if org_path.include? "[]" - path = org_path - elsif conv_path.include? "[]" - path = conv_path - else - path = org_path - end - - # the get_dir method includes a clause that 'tags' array-containing fields with '[]' - # eg. '/Collection/Contacts/Contact[]/OrganizationEmails/Email' - # the following lines show how this 'tagging' is used to identify an array in a given directory - - if path.include? "[]" - path = path.split "[]" - paths << path[0] unless paths.any? { |p| p.eql? path[0] } - elsif path_leads_to_list?(path, original, converted) - paths << path unless paths.any? { |p| p.eql? path } - end - end - paths - end - - def compare_arrays(dif_hash, original, converted) - # arguments: differences hash, the original hash, and converted hash - # each path that leads to an array is used to navigate to that array and - # subsequently compare the arrays in the original and converted hashes. - # there is no usable ouput; there is printing to the terminal - paths = get_list_paths(dif_hash, original, converted) - - paths.each do |path| - org_array = hash_navigation(path, original) - conv_array = hash_navigation(path, converted) - - org_array.is_a?(Array) ? org_arr = Array.wrap(org_array) : org_arr = org_array.clone - org_array = Array.wrap(org_array) unless org_array.is_a?(Array) - conv_array.is_a?(Array) ? conv_arr = Array.wrap(conv_array) : conv_arr = conv_array.clone - conv_array = Array.wrap(conv_array) unless conv_array.is_a?(Array) - - for conv_item in conv_array - for org_item in org_array - if org_item.eql? conv_item - org_arr.delete(org_item) - conv_arr.delete(conv_item) - break - end - end - end - - org_arr.each do |item| - path_with_index = path + "[#{org_array.index(item)}]" - puts "-: ". + path_with_index - end - - conv_arr.each do |item| - path_with_index = path + "[#{conv_array.index(item)}]" - puts "+: " + path_with_index - end - end - end - - def find_difference_bt_hash_arrays(org_arr, conv_arr) - # array inputs; the output is an array that contains the items in the original array - # that were not found in the converted array - org = org_arr.clone - conv = conv_arr.clone - missing = Array.new - if org.eql? conv - return missing - else - for conv_item in conv - for org_item in org - if org_item.eql? conv_item - org.delete(conv_item) - break - end - end - end - missing += org - end - missing - end - - def find_difference_bt_hashes(org, conv) - # input is the original hash and the converted hash; the output is the - # 'differences hash' which represents the items in the original hash that were - # not found in the converted hash - missing = Hash.new - if org.eql? conv - return missing - else - org.each do |org_key,org_value| - if (conv_value = conv[org_key]) - if conv_value.eql? org_value - next - elsif org_value.is_a?(Hash) && conv_value.is_a?(Hash) - missing_value = find_difference_bt_hashes(org_value, conv_value) - unless missing_value.empty? - missing[org_key] = missing_value - end - elsif org_value.is_a?(Array) && conv_value.is_a?(Array) - missing_value = find_difference_bt_hash_arrays(org_value, conv_value) - unless missing_value.empty? - missing[org_key] = missing_value - end - else - missing[org_key] = org_value - end - else - missing[org_key] = org_value - end - end - end - missing - end - - def get_dir(value, hash_or_arr) - # passing the sought-after value and the hash or array being parsed - # output: a single string representing the path to the value arg passed to this method - iterable = hash_or_arr.clone - dir = String.new - if iterable.is_a? Hash - if (matching_key = iterable.key(value)) - dir += '/' + matching_key - iterable.delete(matching_key) - return dir - else - iterable.each do |key,val| - if val.is_a?(Hash) && hash_to_list_of_values(val).include?(value) - dir += '/' + key - dir += get_dir(value, val) - return dir - elsif val.is_a?(Array) && array_to_list_of_values(val).include?(value) - dir += '/' + key + "[]" - dir += get_dir(value, val) - return dir - elsif val.eql? value - dir += '/' + key - iterable.delete(key) - return dir - end - end - end - elsif iterable.is_a? Array - iterable.each do |item| - if item.is_a?(Hash) && hash_to_list_of_values(item).include?(value) - dir += get_dir(value,item) - return dir - elsif item.is_a?(Array) && array_to_list_of_values(item).include?(value) - dir += get_dir(value,item) + "[]" - return dir - end - end - end - dir - end - - def hash_to_list_of_values(hash) - # converts a highly nested hash to a list of all its values - list = Array.new - for val in hash.values - if val.is_a? Hash - list += hash_to_list_of_values(val) - elsif val.is_a? Array - list += array_to_list_of_values(val) - else - list << val - end - end - list - end - - def array_to_list_of_values(array) - #converts a highly nested array to a list of all its values - ls = Array.new - for item in array - if item.is_a? Hash - ls += hash_to_list_of_values(item) - elsif item.is_a? Array - ls += array_to_list_of_values(item) - else - ls << item - end - end - ls - end - - def cmr_client - @cmr_client ||= Cmr::Client.client_for_environment(Rails.configuration.cmr_env, Rails.configuration.services) - end -end From 42371bbc8e67754b5dfcf870bb8232b085496a28 Mon Sep 17 00:00:00 2001 From: Christian Trummer Date: Wed, 15 Jul 2020 16:01:11 -0400 Subject: [PATCH 22/34] MMT-2311: made changes according to change requests --- lib/tasks/compare_xml_collections.rake | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/tasks/compare_xml_collections.rake b/lib/tasks/compare_xml_collections.rake index dadd330ba..1a63b9446 100644 --- a/lib/tasks/compare_xml_collections.rake +++ b/lib/tasks/compare_xml_collections.rake @@ -32,13 +32,14 @@ namespace :collection do ignored_paths = Array.new nokogiri_original.diff(nokogiri_converted, {:added => true, :removed => true}) do |change,node| - if node.parent.path.include?('[') && !ignored_paths.include?(node.parent.path.split('[')[0]) - ignored_paths << node.parent.path.split('[')[0] - array_comparison(node.parent.path.split('[')[0], native_original_hash, native_converted_hash).each do |item| + split_path = node.parent.path.split('[') + if node.parent.path.include?('[') && !ignored_paths.include?(split_path[0]) + ignored_paths << split_path[0] + array_comparison(split_path[0], native_original_hash, native_converted_hash).each do |item| puts("#{item[0]}: #{item[1]}".ljust(60) + item[2]) if args.disp.eql? 'show' puts("#{item[0]}: ". + item[2]) if args.disp.eql? 'hide' end - elsif !ignored_paths.include?(node.parent.path.split('[')[0]) && !path_leads_to_list?(node.parent.path, native_original_hash, native_converted_hash) + elsif !ignored_paths.include?(split_path[0]) && !path_leads_to_list?(node.parent.path, native_original_hash, native_converted_hash) puts("#{change}: #{node.to_xml}".ljust(60) + node.parent.path) if args.disp.eql? 'show' puts("#{change}: ". + node.parent.path) if args.disp.eql? 'hide' end @@ -82,9 +83,7 @@ namespace :collection do end def array_comparison(path, original_hash, converted_hash) - # this is a 'less iterative' version of compare_arrays. Args: a single path, the original hash, and the converted hash. - # Rather than finding all the array paths and using those to find the array differences, the array paths are individually - # supplied by the nokogiri gem; this reduces redundancy + org_array = hash_navigation(path, original_hash) conv_array = hash_navigation(path, converted_hash) @@ -93,6 +92,10 @@ namespace :collection do conv_array.is_a?(Array) ? conv_arr = conv_array.clone : conv_arr = Array.wrap(conv_array) conv_array = Array.wrap(conv_array) unless conv_array.is_a?(Array) + # org_arr and conv_arr are copies of org_array and conv_array, respectively. + # The *_arr values are edited during the comparison between the org_array and conv_array arrays + # and so the *_array arrays are used to maintained a full version of each array for indexing the items in the following lines. + for conv_item in conv_array for org_item in org_array if org_item.eql? conv_item From c50d76762e8bff2fd28cfab522e112112c64ef3c Mon Sep 17 00:00:00 2001 From: Christian Trummer Date: Wed, 15 Jul 2020 16:24:13 -0400 Subject: [PATCH 23/34] MMT-2311: more changes as per PR change requests --- lib/tasks/compare_xml_collections.rake | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/lib/tasks/compare_xml_collections.rake b/lib/tasks/compare_xml_collections.rake index 1a63b9446..f1b8c9c88 100644 --- a/lib/tasks/compare_xml_collections.rake +++ b/lib/tasks/compare_xml_collections.rake @@ -54,6 +54,9 @@ namespace :collection do if path.include?("[") && path.include?("]") bool = true elsif org_hash_path.is_a?(Hash) && conv_hash_path.is_a?(Hash) + # the number of keys must be 1 because all arrays in echo10, dif10, and iso19115 are tagged similar to: + # contact and so all array-containing tags will be the plural + # of the array name. bool = true if org_hash_path.keys.length == 1 && org_hash_path[org_hash_path.keys[0]].is_a?(Array) bool = true if conv_hash_path.keys.length == 1 && conv_hash_path[conv_hash_path.keys[0]].is_a?(Array) elsif org_hash_path.is_a?(Array) || conv_hash_path.is_a?(Array) @@ -68,16 +71,12 @@ namespace :collection do # Passed a path string and the hash being navigated. This method parses the path string and # returns the hash at the end of the path dir = dir.split '/' - if dir.is_a? Array - dir.each do |key| - if !key.empty? && hash.is_a?(Hash) - hash = hash[key] - elsif hash.is_a? Array - return hash - end + dir.each do |key| + if !key.empty? && hash.is_a?(Hash) + hash = hash[key] + elsif hash.is_a? Array + return hash end - else - hash = hash[dir] end hash end From 23e5cb71d24e013cdc4fe88515d4b9c26a141c5e Mon Sep 17 00:00:00 2001 From: Christian Trummer Date: Thu, 16 Jul 2020 08:03:43 -0400 Subject: [PATCH 24/34] MMT-2311: added comment --- lib/tasks/compare_xml_collections.rake | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/tasks/compare_xml_collections.rake b/lib/tasks/compare_xml_collections.rake index f1b8c9c88..27ca858b3 100644 --- a/lib/tasks/compare_xml_collections.rake +++ b/lib/tasks/compare_xml_collections.rake @@ -56,7 +56,8 @@ namespace :collection do elsif org_hash_path.is_a?(Hash) && conv_hash_path.is_a?(Hash) # the number of keys must be 1 because all arrays in echo10, dif10, and iso19115 are tagged similar to: # contact and so all array-containing tags will be the plural - # of the array name. + # of the array name. This clause serves to idenitfy array-containing tags when their paths aren't properly + # displayed by nokogiri bool = true if org_hash_path.keys.length == 1 && org_hash_path[org_hash_path.keys[0]].is_a?(Array) bool = true if conv_hash_path.keys.length == 1 && conv_hash_path[conv_hash_path.keys[0]].is_a?(Array) elsif org_hash_path.is_a?(Array) || conv_hash_path.is_a?(Array) From 04ce818957be9be9aa6f65cd4fe5cec143c0d936 Mon Sep 17 00:00:00 2001 From: ryanmiller-1 <40173609+ryanmiller-1@users.noreply.github.com> Date: Fri, 17 Jul 2020 08:49:46 -0400 Subject: [PATCH 25/34] MMT-2232/2233 Revision viewing and reversion for tools (#617) * MMT-2232 MMT-2233 adding view, revert, and reinstate for tools * MMT-2232 updating a test that is causing intermittent failures. * MMT-2232 removing erroneous comment in test * MMT-2232 fixing response body in altered subscription test * MMT-2232 flipping feature toggles for UMM-T in UAT and prod * MMT-2232 Replacing 'Service' with 'Tool' in tests. * MMT-2232 updating test to clean up correctly * MMT-2232 VCRing the subscription failed delete test --- .../change_current_provider.coffee | 6 + app/controllers/tools_controller.rb | 2 +- .../_not_current_provider_modal.html.erb | 2 + app/views/tools/revisions.html.erb | 26 +- config/environments/production.rb | 2 +- config/environments/uat.rb | 2 +- config/locales/en.yml | 4 + config/routes.rb | 1 + .../delete_subscriptions_spec.rb | 21 +- .../services/reverting_service_spec.rb | 2 - spec/features/tools/reverting_tool_spec.rb | 169 ++++++++ spec/features/tools/revision_list_spec.rb | 58 ++- spec/vcr/subscriptions/failed_delete.yml | 398 ++++++++++++++++++ 13 files changed, 640 insertions(+), 53 deletions(-) create mode 100644 spec/features/tools/reverting_tool_spec.rb create mode 100644 spec/vcr/subscriptions/failed_delete.yml diff --git a/app/assets/javascripts/change_current_provider.coffee b/app/assets/javascripts/change_current_provider.coffee index 08298d76b..54255c96e 100644 --- a/app/assets/javascripts/change_current_provider.coffee +++ b/app/assets/javascripts/change_current_provider.coffee @@ -78,6 +78,12 @@ $(document).ready -> 'Cloning this tool' when 'delete-tool' 'Deleting this tool' + when 'reinstate-tool' + action = 'revert' + 'Reinstating this tool' + when 'revert-tool' + action = 'revert' + 'Reverting 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 65fe35c1e..450fd0a80 100644 --- a/app/controllers/tools_controller.rb +++ b/app/controllers/tools_controller.rb @@ -2,7 +2,7 @@ class ToolsController < BasePublishedRecordController include ManageMetadataHelper - before_action :set_tool, only: [:show, :edit, :clone, :destroy, :revisions, :download_json] #, :revert] + 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] diff --git a/app/views/shared/_not_current_provider_modal.html.erb b/app/views/shared/_not_current_provider_modal.html.erb index 6f828700c..5f71b9558 100644 --- a/app/views/shared/_not_current_provider_modal.html.erb +++ b/app/views/shared/_not_current_provider_modal.html.erb @@ -14,6 +14,8 @@ <%= link_to 'Yes', revert_variable_path(options[:concept_id], revision_id: options[:revision_id]), class: 'eui-btn--blue spinner is-invisible', id: "not-current-provider-revert-link#{'-' + modal_index.to_s if modal_index}" %> <% elsif options[:service] %> <%= link_to 'Yes', revert_service_path(options[:concept_id], revision_id: options[:revision_id]), class: 'eui-btn--blue spinner is-invisible', id: "not-current-provider-revert-link#{'-' + modal_index.to_s if modal_index}" %> + <% elsif options[:tool] %> + <%= link_to 'Yes', revert_tool_path(options[:concept_id], revision_id: options[:revision_id]), class: 'eui-btn--blue spinner is-invisible', id: "not-current-provider-revert-link#{'-' + modal_index.to_s if modal_index}" %> <% else %> <%= link_to 'Yes', revert_collection_path(options[:concept_id], revision_id: options[:revision_id]), class: 'eui-btn--blue spinner is-invisible', id: 'not-current-provider-revert-link' %> <% end %> diff --git a/app/views/tools/revisions.html.erb b/app/views/tools/revisions.html.erb index 9c4f2a277..8a861ac4c 100644 --- a/app/views/tools/revisions.html.erb +++ b/app/views/tools/revisions.html.erb @@ -58,12 +58,10 @@ Deleted <% elsif index == 0 %> Published - <%# Uncomment in MMT-2232 %> - <%#= link_to 'View', tool_path(revision_id: revision_id), title: title %> + <%= link_to 'View', tool_path(revision_id: revision_id), title: title %> <% else %> Revision - <%# Uncomment in MMT-2232 %> - <%#= link_to 'View', tool_path(revision_id: revision_id), title: title %> + <%= link_to 'View', tool_path(revision_id: revision_id), title: title %> <% end %> @@ -84,21 +82,20 @@ <% end %> <% unless index == 0 || revision['meta']['deleted'] == true %> - <%# Uncomment in MMT-2233 %> - <%# if current_provider?(@provider_id) %> - <%#= link_to phrase, "#revert-revisions-modal-#{revision_id}", class: 'display-modal' %> - <%# elsif available_provider?(@provider_id) %> - <%#= link_to phrase, "#not-current-provider-modal-#{revision_id}", class: 'display-modal not-current-provider', data: { 'provider': @provider_id, record_action: action } %> - <%# end %> - <% end %> diff --git a/config/environments/production.rb b/config/environments/production.rb index dc5511069..662e251d8 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -135,7 +135,7 @@ config.subscriptions_enabled = true # Feature toggle for UMM-T - config.umm_t_enabled = false + config.umm_t_enabled = true config.cmr_env = 'ops' config.echo_env = 'ops' diff --git a/config/environments/uat.rb b/config/environments/uat.rb index 009094283..43a54ba41 100644 --- a/config/environments/uat.rb +++ b/config/environments/uat.rb @@ -115,7 +115,7 @@ config.csplog_enabled = false # Feature toggle for UMM-T - config.umm_t_enabled = false + config.umm_t_enabled = true config.cmr_env = 'uat' config.echo_env = 'uat' diff --git a/config/locales/en.yml b/config/locales/en.yml index 3ef4ec0cb..3201d3cf6 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -205,6 +205,10 @@ en: 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.' + revert: + flash: + success: 'Tool Revision Created Successfully!' + error: 'Tool revision was not created successfully' collection_associations: destroy: flash: diff --git a/config/routes.rb b/config/routes.rb index 734198410..2ae58ed56 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -107,6 +107,7 @@ get '/tools/:id/clone' => 'tools#clone', as: 'clone_tool' get '/tools/:id/revisions' => 'tools#revisions', as: 'tool_revisions' get '/tools/:id/download_json(/:revision_id)' => 'tools#download_json', as: 'download_json_tool' + get '/tools/:id/revert/:revision_id' => 'tools#revert', as: 'revert_tool' resources :variable_drafts, controller: 'variable_drafts', draft_type: 'VariableDraft' do member do diff --git a/spec/features/manage_cmr/subscriptions/delete_subscriptions_spec.rb b/spec/features/manage_cmr/subscriptions/delete_subscriptions_spec.rb index 8c4f366fd..bb2599082 100644 --- a/spec/features/manage_cmr/subscriptions/delete_subscriptions_spec.rb +++ b/spec/features/manage_cmr/subscriptions/delete_subscriptions_spec.rb @@ -53,13 +53,28 @@ context 'when failing to delete a subscription' do before do - # Generate an error message by deleting it underneath the 'user' - cmr_client.delete_subscription('MMT_2', @native_id, 'token') click_on 'Delete' - VCR.use_cassette('urs/rarxd5taqea', record: :none) do + # Using 'allow_any_instance_of' causes the after delete to fail as well. + # Need localhost to mock the CMR delete response to be an error. + VCR.configure do |c| + c.ignore_localhost = false + end + + VCR.use_cassette('subscriptions/failed_delete', erb: { concept_id: @ingest_response['concept_id'] }) do click_on 'Yes' end + + VCR.configure do |c| + c.ignore_localhost = true + end + end + + # TODO: Remove after CMR-6332 + after do + delete_response = cmr_client.delete_subscription('MMT_2', @native_id, 'token') + + raise unless delete_response.success? end it 'fails to delete the record' do diff --git a/spec/features/services/reverting_service_spec.rb b/spec/features/services/reverting_service_spec.rb index 5f0510878..d8024aa28 100644 --- a/spec/features/services/reverting_service_spec.rb +++ b/spec/features/services/reverting_service_spec.rb @@ -1,5 +1,3 @@ -require 'rails_helper' - describe 'Reverting to previous services', reset_provider: true, js: true do before :all do # service for simple reverting service test diff --git a/spec/features/tools/reverting_tool_spec.rb b/spec/features/tools/reverting_tool_spec.rb new file mode 100644 index 000000000..a2ba6fa09 --- /dev/null +++ b/spec/features/tools/reverting_tool_spec.rb @@ -0,0 +1,169 @@ +describe 'Reverting to previous tools', reset_provider: true, js: true do + before :all do + # tool for simple reverting tool test + @simple_revert_ingest_response, @simple_revert_concept_response, @native_id = publish_tool_draft(revision_count: 2) + + # tool for reverting tool with many revisions + @multiple_revisions_ingest_response, @multiple_revisions_concept_response, @native_id2 = publish_tool_draft(revision_count: 4, long_name: 'Reverting Tools Test', number_revision_long_names: true) + end + + after :all do + delete_response = cmr_client.delete_tool('MMT_2', @native_id, 'token') + delete_response2 = cmr_client.delete_tool('MMT_2', @native_id2, 'token') + + raise unless delete_response.success? && delete_response2.success? + end + + before do + login + end + + context 'when the latest revision is a published tool' do + before do + visit tool_path(@simple_revert_ingest_response['concept-id']) + + click_on 'Revisions' + end + + it 'displays the correct phrasing for reverting records' do + expect(page).to have_content('Revert to this Revision', count: 1) + end + + context 'when reverting the tool' do + before do + click_on 'Revert to this Revision' + click_on 'Yes' + + wait_for_jQuery + wait_for_cmr + end + + it 'displays all the correct revision information' do + expect(page).to have_content('Revision Created Successfully!') + + expect(page).to have_content('Published', count: 1) + expect(page).to have_content('Revision View', count: 2) + expect(page).to have_content('Revert to this Revision', count: 2) + end + end + + context 'when reverting to a revision before the previous revision from a different provider context' do + context 'when visiting the revisions page from a different provider' do + before do + login(provider: 'MMT_1', providers: %w(MMT_1 MMT_2)) + + visit tool_revisions_path(@multiple_revisions_ingest_response['concept-id']) + end + + it 'displays all the correct revision information' do + within 'main header' do + expect(page).to have_content('Reverting Tools Test -- revision 04') + end + + expect(page).to have_content('Published', count: 1) + expect(page).to have_content('Revision View', count: 3) + expect(page).to have_content('Revert to this Revision', count: 3) + end + + context 'when reverting to the earliest revision' do + before do + visit tool_revisions_path(@multiple_revisions_ingest_response['concept-id']) + + within '#tool-revisions-table tbody tr:last-child' do + # make sure we are clicking on the correct link + expect(page).to have_content('1 - Revision') + + click_on 'Revert to this Revision' + end + end + + it 'displays a modal informing the user they need to switch providers' do + expect(page).to have_content('Reverting this tool requires you change your provider context to MMT_2') + end + + context 'when clicking Yes' do + before do + find('.not-current-provider-link').click + wait_for_jQuery + end + + it 'reverts the tool to the correct revision and displays the correct revision information and switches provider context' do + within 'main header' do + expect(page).to have_content('Reverting Tools Test -- revision 01') + end + + expect(page).to have_content('Published', count: 1) + expect(page).to have_content('Revision View', count: 4) + expect(page).to have_content('Revert to this Revision', count: 4) + + expect(User.first.provider_id).to eq('MMT_2') + end + end + end + end + end + + context 'when reverting the tool fails ingestion into CMR' do + before do + # Do something to the revision so it fails + # Add a new field to the metadata, similar to a field name changing + # and old metadata still having the old field name + new_concept = @simple_revert_concept_response.deep_dup + new_concept.body['BadField'] = 'Not going to work' + + allow_any_instance_of(Cmr::CmrClient).to receive(:get_concept).and_return(new_concept) + + click_on 'Revert to this Revision', match: :first + click_on 'Yes' + + wait_for_jQuery + wait_for_cmr + end + + it 'displays an error message' do + expect(page).to have_content('extraneous key [BadField] is not permitted') + end + end + end + + context 'when the latest revision is a deleted tool' do + before do + ingest_response, _concept_response, @native_id3 = publish_tool_draft + + cmr_client.delete_tool('MMT_2', @native_id3, 'token') + wait_for_cmr + + visit tool_revisions_path(ingest_response['concept-id']) + end + + it 'displays the correct phrasing for reverting records' do + expect(page).to have_content('Reinstate', count: 1) + end + + context 'when reverting the tool' do + before do + click_on 'Reinstate' + click_on 'Yes' + + wait_for_jQuery + wait_for_cmr + end + + # TODO: remove after CMR-6332 + after do + delete_response = cmr_client.delete_tool('MMT_2', @native_id3, 'token') + + raise unless delete_response.success? + end + + it 'displays all the correct revision information' do + expect(page).to have_content('Revision Created Successfully!') + + expect(page).to have_content('Published', count: 1) + expect(page).to have_content('Deleted', count: 1) + expect(page).to have_content('Revision View', count: 1) + expect(page).to have_content('Revert to this Revision', count: 1) + end + end + end +end diff --git a/spec/features/tools/revision_list_spec.rb b/spec/features/tools/revision_list_spec.rb index 0ef91f2a9..aca40ffef 100644 --- a/spec/features/tools/revision_list_spec.rb +++ b/spec/features/tools/revision_list_spec.rb @@ -51,36 +51,34 @@ expect(page).to have_content('11 - Published') end -# TODO: Uncomment in MMT-2233 -# it 'displays the correct phrasing for reverting records' do -# expect(page).to have_content('Revert to this Revision', count: 1) -# end - -# TODO: Uncomment in MMT-2232 -# context 'when viewing an old revision' do -# link_text = 'You are viewing an older revision of this tool. Click here to view the latest published version.' -# before do -# all('a', text: 'View').last.click -# end -# -# it 'displays a message that the revision is old' do -# expect(page).to have_link(link_text) -# end -# -# it 'does not display a link to manage collection associations' do -# expect(page).to have_no_link('Manage Collection Associations') -# end -# -# context 'when clicking the message' do -# before do -# click_on link_text -# end -# -# it 'displays the latest revision to the user' do -# expect(page).to have_no_link(link_text) -# end -# end -# end + it 'displays the correct phrasing for reverting records' do + expect(page).to have_content('Revert to this Revision', count: 9) + end + + context 'when viewing an old revision' do + link_text = 'You are viewing an older revision of this tool. Click here to view the latest published version.' + before do + all('a', text: 'View').last.click + end + + it 'displays a message that the revision is old' do + expect(page).to have_link(link_text) + end + + it 'does not display a link to manage collection associations' do + expect(page).to have_no_link('Manage Collection Associations') + end + + context 'when clicking the message' do + before do + click_on link_text + end + + it 'displays the latest revision to the user' do + expect(page).to have_no_link(link_text) + end + end + end end context 'when searching for the tool' do diff --git a/spec/vcr/subscriptions/failed_delete.yml b/spec/vcr/subscriptions/failed_delete.yml new file mode 100644 index 000000000..ba6eb1f5f --- /dev/null +++ b/spec/vcr/subscriptions/failed_delete.yml @@ -0,0 +1,398 @@ +--- +http_interactions: +- request: + method: get + uri: https://sit.urs.earthdata.nasa.gov/api/users?uids%5B%5D=rarxd5taqea + body: + encoding: US-ASCII + string: '' + headers: + User-Agent: + - Faraday v0.8.11 + Authorization: + - Bearer access_token + response: + status: + code: 200 + message: + headers: + server: + - nginx/1.10.2 + date: + - Tue, 02 May 2017 18:45:16 GMT + content-type: + - application/json; charset=utf-8 + transfer-encoding: + - chunked + connection: + - close + x-frame-options: + - SAMEORIGIN + x-xss-protection: + - 1; mode=block + x-content-type-options: + - nosniff + etag: + - W/"a0ea6aa58a4b79e873d6cb66d9d5bfb7" + cache-control: + - max-age=0, private, must-revalidate + x-request-id: + - 37c1c4e1-f4a7-4af5-a94b-0b51d798342b + x-runtime: + - '0.024303' + strict-transport-security: + - max-age=31536000 + body: + encoding: UTF-8 + string: '{"users":[{"uid":"rarxd5taqea","first_name":"Rvrhzxhtra","last_name":"Vetxvbpmxf","email_address":"uozydogeyyyujukey@tjbh.eyyy","country":"Macedonia","study_area":null,"registered_date":"2012-08-29T11:02:42.000Z","allow_auth_app_emails":true}]}' + http_version: + recorded_at: Tue, 02 May 2017 18:45:16 GMT +- request: + method: delete + uri: http://localhost:3002/providers/MMT_2/subscriptions/test_native_id + body: + encoding: US-ASCII + string: '' + headers: + User-Agent: + - Faraday v1.0.1 + Content-Type: + - application/vnd.nasa.cmr.umm+json + Client-Id: + - MMT + Echo-Token: + - ABC-2 + response: + status: + code: 500 + message: OK + headers: + date: + - Thu, 16 Jul 2020 17:19:54 GMT + content-type: + - application/xml + cmr-request-id: + - 88dd6d01-aa41-46fc-828d-705d7dec1bbb + x-request-id: + - 88dd6d01-aa41-46fc-828d-705d7dec1bbb + content-length: + - '148' + server: + - Jetty(9.4.z-SNAPSHOT) + body: + encoding: UTF-8 + string: '{"errors" => ["Concept with native-id [test_native_id] and concept-id + [<%= concept_id %>] is already deleted."] }' + http_version: + recorded_at: Thu, 16 Jul 2020 17:19:54 GMT +- request: + method: get + uri: http://localhost:3011/permissions?provider=MMT_2&target=NON_NASA_DRAFT_APPROVER&user_id=testuser + body: + encoding: US-ASCII + string: '' + headers: + User-Agent: + - Faraday v1.0.1 + Client-Id: + - MMT + Accept: + - application/json; charset=utf-8 + Echo-Token: + - ABC-2 + response: + status: + code: 200 + message: OK + headers: + date: + - Thu, 16 Jul 2020 17:30:49 GMT + cmr-request-id: + - 866be3e9-3242-45b5-9d0c-bd8df58c0ccf + x-request-id: + - 866be3e9-3242-45b5-9d0c-bd8df58c0ccf + vary: + - Accept-Encoding, User-Agent + content-length: + - '30' + server: + - Jetty(9.4.z-SNAPSHOT) + body: + encoding: UTF-8 + string: '{"NON_NASA_DRAFT_APPROVER":[]}' + http_version: + recorded_at: Thu, 16 Jul 2020 17:30:49 GMT +- request: + method: get + uri: http://localhost:3003/subscriptions.umm_json?concept_id=<%= concept_id %> + body: + encoding: US-ASCII + string: '' + headers: + User-Agent: + - Faraday v1.0.1 + Client-Id: + - MMT + Echo-Token: + - ABC-2 + response: + status: + code: 200 + message: OK + headers: + date: + - Thu, 16 Jul 2020 17:30:49 GMT + content-type: + - application/vnd.nasa.cmr.umm_results+json;version=1.0; charset=utf-8 + access-control-expose-headers: + - CMR-Hits, CMR-Request-Id, X-Request-Id, CMR-Scroll-Id, CMR-Timed-Out, CMR-Shapefile-Original-Point-Count, + CMR-Shapefile-Simplified-Point-Count + access-control-allow-origin: + - "*" + cmr-hits: + - '1' + cmr-took: + - '10' + cmr-request-id: + - ea5ee97c-0b1f-493d-87da-48a3816d4030 + x-request-id: + - ea5ee97c-0b1f-493d-87da-48a3816d4030 + content-length: + - '553' + server: + - Jetty(9.4.z-SNAPSHOT) + body: + encoding: UTF-8 + string: '{"hits":1,"took":10,"items":[{"meta":{"revision-id":14,"deleted":false,"format":"application/vnd.nasa.cmr.umm+json","provider-id":"MMT_2","user-id":"typical","native-id":"test_native_id","concept-id":"<%= concept_id %>","revision-date":"2020-07-16T17:30:48Z","concept-type":"subscription"},"umm":{"Name":"Test_Subscription_38f5c2b5-4192-4866-bcd4-413452e27b65","CollectionConceptId":"C520536-TEST","Query":"bounding_box=-10,-5,10,5&attribute\\[\\]=float,PERCENTAGE,25.5,30","SubscriberId":"rarxd5taqea","EmailAddress":"uozydogeyyyujukey@tjbh.eyyy"}}]}' + http_version: + recorded_at: Thu, 16 Jul 2020 17:30:49 GMT +- request: + method: get + uri: http://localhost:3011/permissions?provider=MMT_2&target=SUBSCRIPTION_MANAGEMENT&user_id=testuser + body: + encoding: US-ASCII + string: '' + headers: + User-Agent: + - Faraday v1.0.1 + Client-Id: + - MMT + Accept: + - application/json; charset=utf-8 + Echo-Token: + - ABC-2 + response: + status: + code: 200 + message: OK + headers: + date: + - Thu, 16 Jul 2020 17:30:49 GMT + cmr-request-id: + - 0daa280f-6586-46b6-b186-bffcb8409009 + x-request-id: + - 0daa280f-6586-46b6-b186-bffcb8409009 + vary: + - Accept-Encoding, User-Agent + content-length: + - '45' + server: + - Jetty(9.4.z-SNAPSHOT) + body: + encoding: UTF-8 + string: '{"SUBSCRIPTION_MANAGEMENT":["read","update"]}' + http_version: + recorded_at: Thu, 16 Jul 2020 17:30:49 GMT +- request: + method: get + uri: http://localhost:3011/permissions?provider=MMT_2&target=NON_NASA_DRAFT_APPROVER&user_id=testuser + body: + encoding: US-ASCII + string: '' + headers: + User-Agent: + - Faraday v1.0.1 + Client-Id: + - MMT + Accept: + - application/json; charset=utf-8 + Echo-Token: + - ABC-2 + response: + status: + code: 200 + message: OK + headers: + date: + - Thu, 16 Jul 2020 17:30:49 GMT + cmr-request-id: + - 380e3f0f-b47d-4b32-9317-f28a2141f249 + x-request-id: + - 380e3f0f-b47d-4b32-9317-f28a2141f249 + vary: + - Accept-Encoding, User-Agent + content-length: + - '30' + server: + - Jetty(9.4.z-SNAPSHOT) + body: + encoding: UTF-8 + string: '{"NON_NASA_DRAFT_APPROVER":[]}' + http_version: + recorded_at: Thu, 16 Jul 2020 17:30:49 GMT +- request: + method: get + uri: http://localhost:3011/permissions?system_object=ANY_ACL&user_id=testuser + body: + encoding: US-ASCII + string: '' + headers: + User-Agent: + - Faraday v1.0.1 + Client-Id: + - MMT + Accept: + - application/json; charset=utf-8 + Echo-Token: + - ABC-2 + response: + status: + code: 200 + message: OK + headers: + date: + - Thu, 16 Jul 2020 17:30:49 GMT + cmr-request-id: + - 2e2d7e7c-57f1-41e1-b8c6-7daedce0a6da + x-request-id: + - 2e2d7e7c-57f1-41e1-b8c6-7daedce0a6da + vary: + - Accept-Encoding, User-Agent + content-length: + - '20' + server: + - Jetty(9.4.z-SNAPSHOT) + body: + encoding: UTF-8 + string: '{"ANY_ACL":["read"]}' + http_version: + recorded_at: Thu, 16 Jul 2020 17:30:49 GMT +- request: + method: get + uri: http://localhost:3011/permissions?provider=MMT_2&target=PROVIDER_OBJECT_ACL&user_id=testuser + body: + encoding: US-ASCII + string: '' + headers: + User-Agent: + - Faraday v1.0.1 + Client-Id: + - MMT + Accept: + - application/json; charset=utf-8 + Echo-Token: + - ABC-2 + response: + status: + code: 200 + message: OK + headers: + date: + - Thu, 16 Jul 2020 17:30:49 GMT + cmr-request-id: + - 24edd338-d818-4bad-9c2f-26aa216b5d48 + x-request-id: + - 24edd338-d818-4bad-9c2f-26aa216b5d48 + vary: + - Accept-Encoding, User-Agent + content-length: + - '59' + server: + - Jetty(9.4.z-SNAPSHOT) + body: + encoding: UTF-8 + string: '{"PROVIDER_OBJECT_ACL":["read","create","update","delete"]}' + http_version: + recorded_at: Thu, 16 Jul 2020 17:30:49 GMT +- request: + method: get + uri: http://localhost:3003/subscriptions.umm_json?concept_id=<%= concept_id %> + body: + encoding: US-ASCII + string: '' + headers: + User-Agent: + - Faraday v1.0.1 + Client-Id: + - MMT + Echo-Token: + - ABC-2 + response: + status: + code: 200 + message: OK + headers: + date: + - Thu, 16 Jul 2020 17:30:49 GMT + content-type: + - application/vnd.nasa.cmr.umm_results+json;version=1.0; charset=utf-8 + access-control-expose-headers: + - CMR-Hits, CMR-Request-Id, X-Request-Id, CMR-Scroll-Id, CMR-Timed-Out, CMR-Shapefile-Original-Point-Count, + CMR-Shapefile-Simplified-Point-Count + access-control-allow-origin: + - "*" + cmr-hits: + - '1' + cmr-took: + - '11' + cmr-request-id: + - f64a50a3-8a1d-4bfc-be6e-23ed95ce6d73 + x-request-id: + - f64a50a3-8a1d-4bfc-be6e-23ed95ce6d73 + content-length: + - '553' + server: + - Jetty(9.4.z-SNAPSHOT) + body: + encoding: UTF-8 + string: '{"hits":1,"took":10,"items":[{"meta":{"revision-id":14,"deleted":false,"format":"application/vnd.nasa.cmr.umm+json","provider-id":"MMT_2","user-id":"typical","native-id":"test_native_id","concept-id":"<%= concept_id %>","revision-date":"2020-07-16T17:30:48Z","concept-type":"subscription"},"umm":{"Name":"Test_Subscription_38f5c2b5-4192-4866-bcd4-413452e27b65","CollectionConceptId":"C520536-TEST","Query":"bounding_box=-10,-5,10,5&attribute\\[\\]=float,PERCENTAGE,25.5,30","SubscriberId":"rarxd5taqea","EmailAddress":"uozydogeyyyujukey@tjbh.eyyy"}}]}' + http_version: + recorded_at: Thu, 16 Jul 2020 17:30:49 GMT +- request: + method: get + uri: http://localhost:3011/permissions?provider=MMT_2&target=SUBSCRIPTION_MANAGEMENT&user_id=testuser + body: + encoding: US-ASCII + string: '' + headers: + User-Agent: + - Faraday v1.0.1 + Client-Id: + - MMT + Accept: + - application/json; charset=utf-8 + Echo-Token: + - ABC-2 + response: + status: + code: 200 + message: OK + headers: + date: + - Thu, 16 Jul 2020 17:30:49 GMT + cmr-request-id: + - 9faa6495-6144-42e8-9624-beed6443069a + x-request-id: + - 9faa6495-6144-42e8-9624-beed6443069a + vary: + - Accept-Encoding, User-Agent + content-length: + - '45' + server: + - Jetty(9.4.z-SNAPSHOT) + body: + encoding: UTF-8 + string: '{"SUBSCRIPTION_MANAGEMENT":["read","update"]}' + http_version: + recorded_at: Thu, 16 Jul 2020 17:30:49 GMT +recorded_with: VCR 5.1.0 From 40af5e443344ebe6364f9a2eb0fe85c828136445 Mon Sep 17 00:00:00 2001 From: Christian Trummer Date: Fri, 17 Jul 2020 10:16:57 -0400 Subject: [PATCH 26/34] MMT-2311: made changes according to PR change requests --- lib/tasks/compare_xml_collections.rake | 61 ++++++++++++-------------- 1 file changed, 29 insertions(+), 32 deletions(-) diff --git a/lib/tasks/compare_xml_collections.rake b/lib/tasks/compare_xml_collections.rake index 27ca858b3..f2df14087 100644 --- a/lib/tasks/compare_xml_collections.rake +++ b/lib/tasks/compare_xml_collections.rake @@ -6,7 +6,7 @@ namespace :collection do args.with_defaults(:version => '1.15.3') args.with_defaults(:disp => 'show') - abort 'FORMAT INVALID' unless args.format.eql? ('echo10' || 'dif10' || 'iso19115') + abort 'FORMAT INVALID' unless args.format.eql?('echo10') || args.format.eql?('dif10') || args.format.eql?('iso19115') filename = args.file.split('/')[-1] puts "\nTranslating #{filename} to UMM JSON..." @@ -16,14 +16,14 @@ namespace :collection do #translate to UMM umm_response = cmr_client.translate_collection(native_original_xml, "application/#{args.format}+xml", "application/vnd.nasa.cmr.umm+json;version=#{args.version}", skip_validation=true ) - umm_json = umm_response.body.to_json umm_response.success? ? puts("\nsuccessful translation to UMM") : abort("\nUMM translation failure") + umm_json = umm_response.body.to_json # translate back to native back_to_native = cmr_client.translate_collection(umm_json, "application/vnd.nasa.cmr.umm+json;version=#{args.version}", "application/#{args.format}+xml", skip_validation=true ) + back_to_native.success? ? puts("successful translation to native format \n\n") : abort("Native format translation failure \n\n") native_converted_hash = Hash.from_xml(back_to_native.body) native_converted_xml = back_to_native.body - back_to_native.success? ? puts("successful translation to native format \n\n") : abort("Native format translation failure \n\n") # nokogiri output nokogiri_original = Nokogiri::XML(native_original_xml) { |config| config.strict.noblanks } @@ -48,19 +48,19 @@ namespace :collection do def path_leads_to_list?(path, org_hash, conv_hash) # this method takes a path string (and the full original and converted hashes) and outputs true if the path string contains a list; else false - org_hash_path = hash_navigation(path, org_hash) - conv_hash_path = hash_navigation(path, conv_hash) + org_hash = hash_navigation(path, org_hash) + conv_hash = hash_navigation(path, conv_hash) if path.include?("[") && path.include?("]") bool = true - elsif org_hash_path.is_a?(Hash) && conv_hash_path.is_a?(Hash) + elsif org_hash.is_a?(Hash) && conv_hash.is_a?(Hash) # the number of keys must be 1 because all arrays in echo10, dif10, and iso19115 are tagged similar to: # contact and so all array-containing tags will be the plural - # of the array name. This clause serves to idenitfy array-containing tags when their paths aren't properly + # of the array name. This clause serves to identify array-containing tags when their paths aren't properly # displayed by nokogiri - bool = true if org_hash_path.keys.length == 1 && org_hash_path[org_hash_path.keys[0]].is_a?(Array) - bool = true if conv_hash_path.keys.length == 1 && conv_hash_path[conv_hash_path.keys[0]].is_a?(Array) - elsif org_hash_path.is_a?(Array) || conv_hash_path.is_a?(Array) + bool = true if org_hash.keys.length == 1 && org_hash[org_hash.keys[0]].is_a?(Array) + bool = true if conv_hash.keys.length == 1 && conv_hash[conv_hash.keys[0]].is_a?(Array) + elsif org_hash.is_a?(Array) || conv_hash.is_a?(Array) bool = true else bool = false @@ -68,12 +68,11 @@ namespace :collection do bool end - def hash_navigation(dir, hash) + def hash_navigation(path, hash) # Passed a path string and the hash being navigated. This method parses the path string and # returns the hash at the end of the path - dir = dir.split '/' - dir.each do |key| - if !key.empty? && hash.is_a?(Hash) + path.split('/').each do |key| + if hash.key?(key) && hash.is_a?(Hash) hash = hash[key] elsif hash.is_a? Array return hash @@ -84,40 +83,38 @@ namespace :collection do def array_comparison(path, original_hash, converted_hash) - org_array = hash_navigation(path, original_hash) - conv_array = hash_navigation(path, converted_hash) + pretranslation_array = hash_navigation(path, original_hash) + post_translation_array = hash_navigation(path, converted_hash) - org_array.is_a?(Array) ? org_arr = org_array.clone : org_arr = Array.wrap(org_array) - org_array = Array.wrap(org_array) unless org_array.is_a?(Array) - conv_array.is_a?(Array) ? conv_arr = conv_array.clone : conv_arr = Array.wrap(conv_array) - conv_array = Array.wrap(conv_array) unless conv_array.is_a?(Array) + pretranslation_array.is_a?(Array) ? lost_items_arr = pretranslation_array.clone : lost_items_arr = Array.wrap(pretranslation_array) + pretranslation_array = Array.wrap(pretranslation_array) unless pretranslation_array.is_a?(Array) + post_translation_array.is_a?(Array) ? added_itmes_arr = post_translation_array.clone : added_itmes_arr = Array.wrap(post_translation_array) + post_translation_array = Array.wrap(post_translation_array) unless post_translation_array.is_a?(Array) # org_arr and conv_arr are copies of org_array and conv_array, respectively. # The *_arr values are edited during the comparison between the org_array and conv_array arrays # and so the *_array arrays are used to maintained a full version of each array for indexing the items in the following lines. - for conv_item in conv_array - for org_item in org_array + for conv_item in post_translation_array + for org_item in pretranslation_array if org_item.eql? conv_item - org_arr.delete(org_item) - conv_arr.delete(conv_item) + lost_items_arr.delete(org_item) + added_itmes_arr.delete(conv_item) break end end end output = Array.new - org_arr.each do |item| - path_with_index = path + "[#{org_array.index(item)}]" - loss_item = ['-', item, path_with_index] - output << loss_item + lost_items_arr.each do |item| + path_with_index = path + "[#{pretranslation_array.index(item)}]" + output << ['-', item, path_with_index] end - conv_arr.each do |item| - path_with_index = path + "[#{conv_array.index(item)}]" - loss_item = ['+', item, path_with_index] - output << loss_item + added_itmes_arr.each do |item| + path_with_index = path + "[#{post_translation_array.index(item)}]" + output << ['+', item, path_with_index] end output end From c479ee253aa4fcf6d9c1675619b57824da5a2568 Mon Sep 17 00:00:00 2001 From: Christian Trummer Date: Mon, 20 Jul 2020 11:16:50 -0400 Subject: [PATCH 27/34] MMT-2311: more changes per PR change requests --- lib/tasks/compare_xml_collections.rake | 32 ++++++++++++++------------ 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/lib/tasks/compare_xml_collections.rake b/lib/tasks/compare_xml_collections.rake index f2df14087..f0ba0e832 100644 --- a/lib/tasks/compare_xml_collections.rake +++ b/lib/tasks/compare_xml_collections.rake @@ -6,7 +6,7 @@ namespace :collection do args.with_defaults(:version => '1.15.3') args.with_defaults(:disp => 'show') - abort 'FORMAT INVALID' unless args.format.eql?('echo10') || args.format.eql?('dif10') || args.format.eql?('iso19115') + abort 'FORMAT INVALID' unless args.format == 'echo10' || args.format == 'dif10' || args.format == 'iso19115' filename = args.file.split('/')[-1] puts "\nTranslating #{filename} to UMM JSON..." @@ -70,12 +70,12 @@ namespace :collection do def hash_navigation(path, hash) # Passed a path string and the hash being navigated. This method parses the path string and - # returns the hash at the end of the path + # returns the array/value at the end of the path path.split('/').each do |key| - if hash.key?(key) && hash.is_a?(Hash) - hash = hash[key] - elsif hash.is_a? Array + if hash.is_a?(Array) return hash + elsif hash.key?(key) && hash.is_a?(Hash) + hash = hash[key] end end hash @@ -83,21 +83,23 @@ namespace :collection do def array_comparison(path, original_hash, converted_hash) - pretranslation_array = hash_navigation(path, original_hash) + pre_translation_array = hash_navigation(path, original_hash) post_translation_array = hash_navigation(path, converted_hash) - pretranslation_array.is_a?(Array) ? lost_items_arr = pretranslation_array.clone : lost_items_arr = Array.wrap(pretranslation_array) - pretranslation_array = Array.wrap(pretranslation_array) unless pretranslation_array.is_a?(Array) + # in the case that a one-item array is parsed as a regular key-value pair instead of an array, an Array wrapper is placed around key-val pair + # so that the following for loops can be executed without error + pre_translation_array.is_a?(Array) ? lost_items_arr = pre_translation_array.clone : lost_items_arr = Array.wrap(pre_translation_array) + pre_translation_array = Array.wrap(pre_translation_array) post_translation_array.is_a?(Array) ? added_itmes_arr = post_translation_array.clone : added_itmes_arr = Array.wrap(post_translation_array) - post_translation_array = Array.wrap(post_translation_array) unless post_translation_array.is_a?(Array) + post_translation_array = Array.wrap(post_translation_array) - # org_arr and conv_arr are copies of org_array and conv_array, respectively. - # The *_arr values are edited during the comparison between the org_array and conv_array arrays - # and so the *_array arrays are used to maintained a full version of each array for indexing the items in the following lines. + # as defined above, the lost_items_arr and added_itmes_arr are copies of pre_translation_array and post_translation_array, respectively. + # The *_arr values are edited during the comparison between the pre_translation_array and post_translation_array arrays + # and so the *_array arrays are used to maintain a full version of each array for indexing the items in the following lines. for conv_item in post_translation_array - for org_item in pretranslation_array - if org_item.eql? conv_item + for org_item in pre_translation_array + if org_item == conv_item lost_items_arr.delete(org_item) added_itmes_arr.delete(conv_item) break @@ -107,7 +109,7 @@ namespace :collection do output = Array.new lost_items_arr.each do |item| - path_with_index = path + "[#{pretranslation_array.index(item)}]" + path_with_index = path + "[#{pre_translation_array.index(item)}]" output << ['-', item, path_with_index] end From 52d585901fff7fa58c0a4d01d9f17928a6c81f45 Mon Sep 17 00:00:00 2001 From: ryanmiller-1 <40173609+ryanmiller-1@users.noreply.github.com> Date: Tue, 21 Jul 2020 10:26:29 -0400 Subject: [PATCH 28/34] Hotfix/MMT-2232-1 Fixing tests bamboo started failing (#619) * MMT-2232 fixing some tests that bamboo started failing. * MMT-2232 adding comment to add_data_center_with_retry --- .../forms/data_centers_form_spec.rb | 2 +- .../saving_data_centers_data_contacts_spec.rb | 2 +- .../collection_drafts/forms/validation_spec.rb | 6 ++++++ spec/support/draft_helpers.rb | 15 +++++++++++++++ 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/spec/features/collection_drafts/forms/data_centers_form_spec.rb b/spec/features/collection_drafts/forms/data_centers_form_spec.rb index 1d41ce3a5..00082826e 100644 --- a/spec/features/collection_drafts/forms/data_centers_form_spec.rb +++ b/spec/features/collection_drafts/forms/data_centers_form_spec.rb @@ -27,7 +27,7 @@ within '#draft_data_centers_1' do select 'Originator', from: 'Role' - add_data_center('ESA/ED') + add_data_center_with_retry('ESA/ED') add_contact_information(type: 'data_center', single: false, button_type: 'Data Center') end diff --git a/spec/features/collection_drafts/forms/saving_data_centers_data_contacts_spec.rb b/spec/features/collection_drafts/forms/saving_data_centers_data_contacts_spec.rb index 6a31955e2..5fa7a27f0 100644 --- a/spec/features/collection_drafts/forms/saving_data_centers_data_contacts_spec.rb +++ b/spec/features/collection_drafts/forms/saving_data_centers_data_contacts_spec.rb @@ -66,7 +66,7 @@ within '.multiple.data-centers > .multiple-item-1' do select 'Originator', from: 'Role' - add_data_center('ESA/ED') + add_data_center_with_retry('ESA/ED') end within '.nav-top' do diff --git a/spec/features/collection_drafts/forms/validation_spec.rb b/spec/features/collection_drafts/forms/validation_spec.rb index ac0c80194..800af7f4f 100644 --- a/spec/features/collection_drafts/forms/validation_spec.rb +++ b/spec/features/collection_drafts/forms/validation_spec.rb @@ -290,6 +290,12 @@ fill_in 'draft_temporal_extents_0_single_date_times_0', with: '2015-07-01T00:00:00Z' end + # Bamboo spontaneously started failling this test with the apparent + # cause being that 'done' was not being clicked. Clicking something + # outside of the datepicker widget allows the done click to be + # processed correctly. Previously, it looks like the click for done + # was only exiting the single date time field. + find('#draft_temporal_extents_0_precision_of_seconds').click within '.nav-top' do click_on 'Done' end diff --git a/spec/support/draft_helpers.rb b/spec/support/draft_helpers.rb index bdf313501..41f3d8d05 100644 --- a/spec/support/draft_helpers.rb +++ b/spec/support/draft_helpers.rb @@ -48,6 +48,21 @@ def add_data_center(value) end end + # Bamboo started failing some tests where it seemed that the select2 was not + # opening properly. This is notably slower, so it should only be used when + # necessary. + def add_data_center_with_retry(value) + ActiveSupport::Notifications.instrument 'mmt.performance', activity: 'Helpers::DraftHelpers#add_data_center_with_retry' do + find('.select2-container .select2-selection').click + begin + find(:xpath, '//body').find('.select2-dropdown li.select2-results__option', text: value) + rescue Capybara::ElementNotFound + find('.select2-container .select2-selection').click + end + find(:xpath, '//body').find('.select2-dropdown li.select2-results__option', text: value).click + end + end + def add_person ActiveSupport::Notifications.instrument 'mmt.performance', activity: 'Helpers::DraftHelpers#add_person' do fill_in 'First Name', with: 'First Name' From d8f7561b8e908d3cbc666cb4aa679ba30118911d Mon Sep 17 00:00:00 2001 From: Christian Trummer Date: Wed, 22 Jul 2020 09:43:29 -0400 Subject: [PATCH 29/34] MMT-2311: fixed dif10 asterisk issues --- lib/tasks/compare_xml_collections.rake | 40 ++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/lib/tasks/compare_xml_collections.rake b/lib/tasks/compare_xml_collections.rake index f0ba0e832..8f80c080d 100644 --- a/lib/tasks/compare_xml_collections.rake +++ b/lib/tasks/compare_xml_collections.rake @@ -26,8 +26,8 @@ namespace :collection do native_converted_xml = back_to_native.body # nokogiri output - nokogiri_original = Nokogiri::XML(native_original_xml) { |config| config.strict.noblanks } - nokogiri_converted = Nokogiri::XML(native_converted_xml) { |config| config.strict.noblanks } + nokogiri_original = Nokogiri::XML(native_original_xml) { |config| config.strict.noblanks } .remove_namespaces! + nokogiri_converted = Nokogiri::XML(native_converted_xml) { |config| config.strict.noblanks } .remove_namespaces! ignored_paths = Array.new @@ -36,12 +36,20 @@ namespace :collection do if node.parent.path.include?('[') && !ignored_paths.include?(split_path[0]) ignored_paths << split_path[0] array_comparison(split_path[0], native_original_hash, native_converted_hash).each do |item| - puts("#{item[0]}: #{item[1]}".ljust(60) + item[2]) if args.disp.eql? 'show' - puts("#{item[0]}: ". + item[2]) if args.disp.eql? 'hide' + puts("#{item[0]}: #{item[1]}".ljust(60) + item[2]) if args.disp == 'show' + puts("#{item[0]}: " + item[2]) if args.disp == 'hide' end elsif !ignored_paths.include?(split_path[0]) && !path_leads_to_list?(node.parent.path, native_original_hash, native_converted_hash) - puts("#{change}: #{node.to_xml}".ljust(60) + node.parent.path) if args.disp.eql? 'show' - puts("#{change}: ". + node.parent.path) if args.disp.eql? 'hide' + if is_xml?(node) + element = Hash.from_xml(node.to_xml) + hash_map(element).each do |item| + puts("#{change}: #{item['value']}".ljust(60) + node.parent.path+'/'+item['path']) if args.disp == 'show' + puts("#{change}: " + node.parent.path+'/'+item['path']) if args.disp == 'hide' + end + else + puts("#{change}: #{node.to_xml}".ljust(60) + node.parent.path) if args.disp == 'show' + puts("#{change}: " + node.parent.path) if args.disp == 'hide' + end end end end @@ -68,6 +76,11 @@ namespace :collection do bool end + def is_xml?(node) + if node.to_xml.include?('<' && '') then return true + else return false end + end + def hash_navigation(path, hash) # Passed a path string and the hash being navigated. This method parses the path string and # returns the array/value at the end of the path @@ -81,6 +94,21 @@ namespace :collection do hash end + def hash_map(hash) + buckets = Array.new + hash.each do |key,val| + if val.is_a? Hash + hash_map(val).each do |item| + item['path'] = key + '/' + item['path'] + buckets << item + end + else + buckets << {'path'=> key, 'value'=> val} + end + end + buckets + end + def array_comparison(path, original_hash, converted_hash) pre_translation_array = hash_navigation(path, original_hash) From d60994a78721f43f65e1486c6ec32736c281955f Mon Sep 17 00:00:00 2001 From: Christian Trummer Date: Wed, 22 Jul 2020 13:33:00 -0400 Subject: [PATCH 30/34] MMT-2311: Adjusted removal of namespaces --- lib/tasks/compare_xml_collections.rake | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/tasks/compare_xml_collections.rake b/lib/tasks/compare_xml_collections.rake index 8f80c080d..6e9e99898 100644 --- a/lib/tasks/compare_xml_collections.rake +++ b/lib/tasks/compare_xml_collections.rake @@ -25,9 +25,13 @@ namespace :collection do native_converted_hash = Hash.from_xml(back_to_native.body) native_converted_xml = back_to_native.body - # nokogiri output - nokogiri_original = Nokogiri::XML(native_original_xml) { |config| config.strict.noblanks } .remove_namespaces! - nokogiri_converted = Nokogiri::XML(native_converted_xml) { |config| config.strict.noblanks } .remove_namespaces! + if args.format == 'dif10' + nokogiri_original = Nokogiri::XML(native_original_xml) { |config| config.strict.noblanks } .remove_namespaces! + nokogiri_converted = Nokogiri::XML(native_converted_xml) { |config| config.strict.noblanks } .remove_namespaces! + else + nokogiri_original = Nokogiri::XML(native_original_xml) { |config| config.strict.noblanks } + nokogiri_converted = Nokogiri::XML(native_converted_xml) { |config| config.strict.noblanks } + end ignored_paths = Array.new From 55423ba4772ab10da40fdf38835df510cc9a16af Mon Sep 17 00:00:00 2001 From: Christian Trummer Date: Mon, 27 Jul 2020 10:12:58 -0400 Subject: [PATCH 31/34] MMT-2311: not to be pushed --- app/helpers/loss_report_helper.rb | 229 +++++++++++++++++++++++++ config/routes.rb | 1 + lib/tasks/compare_xml_collections.rake | 35 ++-- 3 files changed, 240 insertions(+), 25 deletions(-) create mode 100644 app/helpers/loss_report_helper.rb diff --git a/app/helpers/loss_report_helper.rb b/app/helpers/loss_report_helper.rb new file mode 100644 index 000000000..e79dfb052 --- /dev/null +++ b/app/helpers/loss_report_helper.rb @@ -0,0 +1,229 @@ +module LossReportHelper + + def loss_report_output(concept_id, hide_items=true, disp='text') + # depending on the input selection (json or text) a comparison string/hash is created and displayed in-browser + + orig_xml,conv_xml,orig_h,conv_h,content_type = prepare_collections(concept_id, '1.15.3') + + if content_type.include?('iso') || content_type.include?('dif') + orig = Nokogiri::XML(orig_xml) { |config| config.strict.noblanks } .remove_namespaces! + conv = Nokogiri::XML(conv_xml) { |config| config.strict.noblanks } .remove_namespaces! + else + orig = Nokogiri::XML(orig_xml) { |config| config.strict.noblanks } + conv = Nokogiri::XML(conv_xml) { |config| config.strict.noblanks } + end + + #write files to test that all changes are being found with opendiff + dir = '/Users/ctrummer/Documents/devtesting' + File.write("#{dir}/o_#{concept_id}.xml", orig.to_xml) + File.write("#{dir}/c_#{concept_id}.xml", conv.to_xml) + + arr_paths = Array.new # This array is used to keep track of the paths that lead to arrays that have already been mapped + text_output = String.new if disp == 'text' + json_output = Hash.new if disp == 'json' + + # json_output['orig'] = hash_map(orig_h) if disp == 'json' + # json_output['orig'] = orig_h if disp == 'json' + # json_output['conv'] = conv_h if disp == 'json' + # text_output += orig_xml if disp == 'text' + + json_output['format'] = content_type if disp == 'json' + text_output += (content_type + "\n\n") if disp == 'text' + + # text_output += top_level_arr_path('/Collection/OnlineResources/OnlineResource', orig_h, conv_h).to_s+"\n" + + orig.diff(conv, {:added => true, :removed => true}) do |change,node| + element = node.to_xml + path = node.parent.path.split('[')[0] + arr_path = top_level_arr_path(path, orig_h, conv_h) + + # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- + puts "---------------------------------------------------------------------------------" + puts "arr_path: #{arr_path} ... node.parent.path: #{node.parent.path} ... path: #{path}" + # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- + + if arr_path && path_not_checked?(arr_path, arr_paths) + + # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- + puts "** path 1" + puts "ar path_not_checked?(arr_path,arr_paths): #{path_not_checked?(arr_path,arr_paths).to_s}" + # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- + + arr_paths << arr_path + array_comparison(arr_path, orig_h, conv_h).each do |item| # all lists + add_to_report('ar'+item[0], item[1], item[2], hide_items, disp, json_output, text_output) + end + + # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- + puts "arr_paths: #{arr_paths}" + # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- + + elsif path_not_checked?(path, arr_paths) # nokogiri + + # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- + puts "** path 2" + puts "path_not_checked?(path,arr_paths): #{path_not_checked?(path,arr_paths).to_s}" + # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- + + if is_xml?(node) + element = Hash.from_xml(element) + hash_map(element).each do |item| + arr_path = top_level_arr_path("#{path}/#{item['path']}", orig_h, conv_h) + + # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- + puts "path_not_checked?('path/item['path']}, arr_paths): #{path_not_checked?("#{path}/#{item['path']}", arr_paths)}" + # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- + + if arr_path && path_not_checked?("#{path}/#{item['path']}", arr_paths) # all list + if path_not_checked?(arr_path, arr_paths) + + # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- + puts "na path_not_checked?(arr_path, arr_paths): #{path_not_checked?(arr_path, arr_paths)}" + # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- + + arr_paths << arr_path + array_comparison(arr_path, orig_h, conv_h).each do |item| + add_to_report('na'+item[0], item[1], item[2], hide_items, disp, json_output, text_output) + end + + # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- + puts "arr_paths: #{arr_paths}" + # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- + + end + elsif path_not_checked?("#{path}/#{item['path']}", arr_paths) + add_to_report('hn'+change, item['value'], "#{path}/#{item['path']}", hide_items, disp, json_output, text_output) + end + + # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- + puts "arr_paths: #{arr_paths}" + # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- + + end + else + add_to_report('ng'+change, element, path, hide_items, disp, json_output, text_output) + + # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- + puts "arr_paths: #{arr_paths}" + # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- + + end + end + end + if disp == 'text' then return text_output + elsif disp == 'json' then return json_output end + end + + def path_not_checked?(arr_path, arr_paths) + arr_paths.each do |path| + if arr_path.include?(path) + return false + end + end + true + end + + def top_level_arr_path(path, orig_h, conv_h) + pre_translation_array, pre_translation_path = hash_navigation(path, orig_h) + post_translation_array, post_translation_path = hash_navigation(path, conv_h) + + return false if pre_translation_array == false && post_translation_array == false + + return pre_translation_path if pre_translation_array.is_a?(Array) + return post_translation_path if post_translation_array.is_a?(Array) + + # the number of keys must be 1 because all arrays in echo10, dif10, and iso19115 are tagged similar to: + # contact and so all array-containing tags will be the plural + # of the array name. This clause serves to identify array-containing tags when their paths aren't properly + # displayed by nokogiri + if pre_translation_array.is_a?(Hash) && pre_translation_array.keys.length == 1 && pre_translation_array[pre_translation_array.keys[0]].is_a?(Array) + return pre_translation_path + "/#{pre_translation_array.keys[0]}" + elsif post_translation_array.is_a?(Hash) && post_translation_array.keys.length == 1 && post_translation_array[post_translation_array.keys[0]].is_a?(Array) + return post_translation_path + "/#{post_translation_array.keys[0]}" + end + + path_contains_array = false + end + + def add_to_report(change, element, path, hide_items, disp, json_output, text_output) + @counter ||= 0 and @counter += 1 + + # this function serves to preclude complex nests from forming in loss_report_output the + # following 'if' structure is intended to increase readability by eliminating nests + return text_output.concat("#{@counter}.".ljust(4)+"#{change}: #{element}".ljust(60) + path + "\n") if hide_items == false && disp == 'text' + puts "#{@counter}.".ljust(4)+"#{change}: ".ljust(3) + path; return text_output.concat("#{@counter}.".ljust(4)+"#{change}: ".ljust(3) + path + "\n") if hide_items == true && disp == 'text' + return json_output["#{@counter}. #{change}: #{path}"] = element if disp == 'json' + end + + def hash_map(hash) + buckets = Array.new + hash.each do |key,val| + if val.is_a? Hash then hash_map(val).each do |item| + item['path'] = key + '/' + item['path'] + buckets << item end + else + buckets << {'path'=> key, 'value'=> val} + end + end + buckets + end + + def is_xml?(node) + if node.to_xml.include?('<' && '') then return true + else return false end + end + + def prepare_collections(concept_id, umm_c_version) + # TODO: need to add exception handling for get_concept, translate_collection + original_collection_native_xml = cmr_client.get_concept(concept_id,token, {}) + content_type = original_collection_native_xml.headers.fetch('content-type').split(';')[0] + original_collection_native_hash = Hash.from_xml(original_collection_native_xml.body) + translated_collection_umm_json = cmr_client.translate_collection(original_collection_native_xml.body, content_type, "application/vnd.nasa.cmr.umm+json;version=#{umm_c_version}", skip_validation=true) + translated_collection_native_xml = cmr_client.translate_collection(translated_collection_umm_json.body.to_json, "application/vnd.nasa.cmr.umm+json;version=#{umm_c_version}", content_type, skip_validation=true) + translated_collection_native_hash = Hash.from_xml(translated_collection_native_xml.body) + return original_collection_native_xml.body, translated_collection_native_xml.body, original_collection_native_hash, translated_collection_native_hash, content_type + end + + + def hash_navigation(path, hash) + # Passed a path string and the hash being navigated. This method parses the path string and + # returns the array/value at the end of the path + current_path = String.new + path.split('/').each do |key| + if hash.is_a?(Array) + return hash, current_path + elsif hash.key?(key) && hash.is_a?(Hash) + current_path += "/#{key}" + hash = hash[key] + elsif !hash.key?(key) && key != '' + return path_exists = false, "#{current_path}/#{key}" + end + end + return hash, current_path + end + + def array_comparison(path, original_hash, converted_hash) + pre_translation_array = hash_navigation(path, original_hash)[0] + post_translation_array = hash_navigation(path, converted_hash)[0] + + pre_translation_array == false ? pre_translation_array = Array.new : pre_translation_array = Array.wrap(pre_translation_array) + post_translation_array == false ? post_translation_array = Array.new : post_translation_array = Array.wrap(post_translation_array) + + output = Array.new + (pre_translation_array - post_translation_array).each do |item| + path_with_index = path + "[#{pre_translation_array.index(item)}]" + # the following line is used to eliminate indexing confusion when there is more than one occurrence of a particular item in an array + pre_translation_array[pre_translation_array.index(item)] = item.to_s + 'item indexed' + output << ['-', item, path_with_index] + end + + (post_translation_array - pre_translation_array).each do |item| + path_with_index = path + "[#{post_translation_array.index(item)}]" + # the following line is used to eliminate indexing confusion when there is more than one occurrence of a particular item in an array + post_translation_array[post_translation_array.index(item)] = item.to_s + 'item indexed' + output << ['+', item, path_with_index] + end + output + end + +end diff --git a/config/routes.rb b/config/routes.rb index 2ae58ed56..faa4b52f8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -70,6 +70,7 @@ get '/collections/:id/create_delete_proposal' => 'collections#create_delete_proposal', as: 'create_delete_proposal_collection' get '/collections/:id/create_update_proposal' => 'collections#create_update_proposal', as: 'create_update_proposal_collection' + resource :variable_generation_processes_search, only: [:new] resource :variable_generation_process, only: [:create, :update] do diff --git a/lib/tasks/compare_xml_collections.rake b/lib/tasks/compare_xml_collections.rake index 6e9e99898..a1a300e66 100644 --- a/lib/tasks/compare_xml_collections.rake +++ b/lib/tasks/compare_xml_collections.rake @@ -25,7 +25,7 @@ namespace :collection do native_converted_hash = Hash.from_xml(back_to_native.body) native_converted_xml = back_to_native.body - if args.format == 'dif10' + if args.format == 'dif10' || args.format == 'iso19115' nokogiri_original = Nokogiri::XML(native_original_xml) { |config| config.strict.noblanks } .remove_namespaces! nokogiri_converted = Nokogiri::XML(native_converted_xml) { |config| config.strict.noblanks } .remove_namespaces! else @@ -114,42 +114,27 @@ namespace :collection do end def array_comparison(path, original_hash, converted_hash) - pre_translation_array = hash_navigation(path, original_hash) post_translation_array = hash_navigation(path, converted_hash) - # in the case that a one-item array is parsed as a regular key-value pair instead of an array, an Array wrapper is placed around key-val pair - # so that the following for loops can be executed without error - pre_translation_array.is_a?(Array) ? lost_items_arr = pre_translation_array.clone : lost_items_arr = Array.wrap(pre_translation_array) - pre_translation_array = Array.wrap(pre_translation_array) - post_translation_array.is_a?(Array) ? added_itmes_arr = post_translation_array.clone : added_itmes_arr = Array.wrap(post_translation_array) - post_translation_array = Array.wrap(post_translation_array) - - # as defined above, the lost_items_arr and added_itmes_arr are copies of pre_translation_array and post_translation_array, respectively. - # The *_arr values are edited during the comparison between the pre_translation_array and post_translation_array arrays - # and so the *_array arrays are used to maintain a full version of each array for indexing the items in the following lines. - - for conv_item in post_translation_array - for org_item in pre_translation_array - if org_item == conv_item - lost_items_arr.delete(org_item) - added_itmes_arr.delete(conv_item) - break - end - end - end + pre_translation_array == false ? pre_translation_array = Array.new : pre_translation_array = Array.wrap(pre_translation_array) + post_translation_array == false ? post_translation_array = Array.new : post_translation_array = Array.wrap(post_translation_array) output = Array.new - lost_items_arr.each do |item| + (pre_translation_array - post_translation_array).each do |item| path_with_index = path + "[#{pre_translation_array.index(item)}]" + # the following line is used to eliminate indexing confusion when there is more than one occurrence of a particular item in an array + pre_translation_array[pre_translation_array.index(item)] = item.to_s + 'item indexed' output << ['-', item, path_with_index] end - - added_itmes_arr.each do |item| + (post_translation_array - pre_translation_array).each do |item| path_with_index = path + "[#{post_translation_array.index(item)}]" + # the following line is used to eliminate indexing confusion when there is more than one occurrence of a particular item in an array + post_translation_array[post_translation_array.index(item)] = item.to_s + 'item indexed' output << ['+', item, path_with_index] end output end + end From 9030b9b267d5939f51393f2955cabb93ae9daf3c Mon Sep 17 00:00:00 2001 From: Christian Trummer Date: Mon, 3 Aug 2020 08:12:43 -0400 Subject: [PATCH 32/34] MMT-2311: updated the comparison methods/logic to be congruent with those of loss_report_helper in MMT-2313 --- lib/tasks/compare_xml_collections.rake | 161 +++++++++++++++---------- 1 file changed, 100 insertions(+), 61 deletions(-) diff --git a/lib/tasks/compare_xml_collections.rake b/lib/tasks/compare_xml_collections.rake index a1a300e66..43dc25f2c 100644 --- a/lib/tasks/compare_xml_collections.rake +++ b/lib/tasks/compare_xml_collections.rake @@ -25,7 +25,7 @@ namespace :collection do native_converted_hash = Hash.from_xml(back_to_native.body) native_converted_xml = back_to_native.body - if args.format == 'dif10' || args.format == 'iso19115' + if args.format.include?('dif') || args.format.include?('iso') nokogiri_original = Nokogiri::XML(native_original_xml) { |config| config.strict.noblanks } .remove_namespaces! nokogiri_converted = Nokogiri::XML(native_converted_xml) { |config| config.strict.noblanks } .remove_namespaces! else @@ -33,86 +33,125 @@ namespace :collection do nokogiri_converted = Nokogiri::XML(native_converted_xml) { |config| config.strict.noblanks } end - ignored_paths = Array.new + arr_paths = Array.new # This array is used to keep track of the paths that lead to arrays that have already been mapped + text_output = String.new nokogiri_original.diff(nokogiri_converted, {:added => true, :removed => true}) do |change,node| - split_path = node.parent.path.split('[') - if node.parent.path.include?('[') && !ignored_paths.include?(split_path[0]) - ignored_paths << split_path[0] - array_comparison(split_path[0], native_original_hash, native_converted_hash).each do |item| - puts("#{item[0]}: #{item[1]}".ljust(60) + item[2]) if args.disp == 'show' - puts("#{item[0]}: " + item[2]) if args.disp == 'hide' - end - elsif !ignored_paths.include?(split_path[0]) && !path_leads_to_list?(node.parent.path, native_original_hash, native_converted_hash) + element = node.to_xml + path = node.parent.path.split('[')[0] + arr_path = top_level_arr_path(path, native_original_hash, native_converted_hash) + + if arr_path && path_not_checked?(arr_path, arr_paths) + arr_paths << arr_path + array_comparison(arr_path, native_original_hash, native_converted_hash).each { |item| add_to_report(item[0], item[1], item[2], args.disp) } + elsif path_not_checked?(path, arr_paths) if is_xml?(node) - element = Hash.from_xml(node.to_xml) + element = Hash.from_xml(element) hash_map(element).each do |item| - puts("#{change}: #{item['value']}".ljust(60) + node.parent.path+'/'+item['path']) if args.disp == 'show' - puts("#{change}: " + node.parent.path+'/'+item['path']) if args.disp == 'hide' + arr_path = top_level_arr_path("#{path}/#{item['path']}", native_original_hash, native_converted_hash) + if arr_path && path_not_checked?("#{path}/#{item['path']}", arr_paths) + if path_not_checked?(arr_path, arr_paths) + arr_paths << arr_path + array_comparison(arr_path, native_original_hash, native_converted_hash).each { |item| add_to_report(item[0], item[1], item[2], args.disp) } + end + elsif path_not_checked?("#{path}/#{item['path']}", arr_paths) + add_to_report(change, item['value'], "#{path}/#{item['path']}", args.disp) + end end + elsif (attr,val = is_attribute?(node)) + add_to_report(change, val, "#{path}/#{attr}" , args.disp) else - puts("#{change}: #{node.to_xml}".ljust(60) + node.parent.path) if args.disp == 'show' - puts("#{change}: " + node.parent.path) if args.disp == 'hide' + add_to_report(change, element, path, args.disp) end end end end - def path_leads_to_list?(path, org_hash, conv_hash) - # this method takes a path string (and the full original and converted hashes) and outputs true if the path string contains a list; else false - org_hash = hash_navigation(path, org_hash) - conv_hash = hash_navigation(path, conv_hash) - - if path.include?("[") && path.include?("]") - bool = true - elsif org_hash.is_a?(Hash) && conv_hash.is_a?(Hash) - # the number of keys must be 1 because all arrays in echo10, dif10, and iso19115 are tagged similar to: - # contact and so all array-containing tags will be the plural - # of the array name. This clause serves to identify array-containing tags when their paths aren't properly - # displayed by nokogiri - bool = true if org_hash.keys.length == 1 && org_hash[org_hash.keys[0]].is_a?(Array) - bool = true if conv_hash.keys.length == 1 && conv_hash[conv_hash.keys[0]].is_a?(Array) - elsif org_hash.is_a?(Array) || conv_hash.is_a?(Array) - bool = true - else - bool = false - end - bool + def add_to_report(change, element, path, display) + @counter ||= 0 and @counter += 1 + # this function serves to preclude complex nests from forming in loss_report_output the + # following 'if' structure is intended to increase readability by eliminating nests + puts "#{@counter}.".ljust(4)+"#{change}: #{element}".ljust(60) + path + "\n" if display == 'show' + puts "#{@counter}.".ljust(4)+"#{change}: ".ljust(3) + path + "\n" if display == 'hide' end def is_xml?(node) - if node.to_xml.include?('<' && '') then return true - else return false end + # checks if the node being passed is xml + # may be beneficial to add more checks + node.to_xml.include?('<' && '') ? true : false end - def hash_navigation(path, hash) - # Passed a path string and the hash being navigated. This method parses the path string and - # returns the array/value at the end of the path - path.split('/').each do |key| - if hash.is_a?(Array) - return hash - elsif hash.key?(key) && hash.is_a?(Hash) - hash = hash[key] - end + def is_attribute?(node) + # this method checks if the node being passed is an attribute change; + # TODO: it may be beneficial to add more conditions to improve accuracy + if node.to_xml.include?('=') && !node.to_xml.include?(' = ') + attr_val = Array.new + node.to_xml.split('=').each {|item| attr_val << item.strip.delete('\\"')} + attr_val + else + false end - hash end - def hash_map(hash) - buckets = Array.new - hash.each do |key,val| - if val.is_a? Hash - hash_map(val).each do |item| - item['path'] = key + '/' + item['path'] - buckets << item - end - else - buckets << {'path'=> key, 'value'=> val} - end + def path_not_checked?(arr_path, arr_paths) + # this method checks the arr_paths array to see if the path being added to + # the report has already been previously evaluated and added + arr_paths.each { |path| return false if arr_path.include?(path) } + true + end + + def top_level_arr_path(path, orig_h, conv_h) + # if an array is passed that passes through an array ie. /Contacts/Contact[0]/Role/Name + # this method would return /Contacts/Contact because Contact is the outermost array (or false if the path doesn't contain an array) + pre_translation_array, pre_translation_path = hash_navigation(path, orig_h) + post_translation_array, post_translation_path = hash_navigation(path, conv_h) + + return false if pre_translation_array == false && post_translation_array == false + return pre_translation_path if pre_translation_array.is_a?(Array) + return post_translation_path if post_translation_array.is_a?(Array) + + # the number of keys must be 1 because all arrays in echo10, dif10, and iso19115 are tagged similar to: + # contact and so all array-containing tags will be the plural + # of the array name. This clause serves to identify array-containing tags when their paths aren't properly + # displayed by nokogiri + if pre_translation_array.is_a?(Hash) && pre_translation_array.keys.length == 1 && pre_translation_array[pre_translation_array.keys[0]].is_a?(Array) + return "#{pre_translation_path}/#{pre_translation_array.keys[0]}" + elsif post_translation_array.is_a?(Hash) && post_translation_array.keys.length == 1 && post_translation_array[post_translation_array.keys[0]].is_a?(Array) + return "#{post_translation_path}/#{post_translation_array.keys[0]}" end - buckets + path_contains_array = false end + def hash_navigation(path, hash) + # Passed a path string and the hash being navigated. This method parses the path string and + # returns the array/value at the end of the path + current_path = String.new + path.split('/').each do |key| + if hash.is_a?(Array) + return hash, current_path + elsif hash.key?(key) && hash.is_a?(Hash) + current_path += "/#{key}" + hash = hash[key] + elsif !hash.key?(key) && key != '' + return path_exists = false, "#{current_path}/#{key}" + end + end + return hash, current_path + end + + def hash_map(hash) + buckets = Array.new + hash.each do |key,val| + if val.is_a? Hash then hash_map(val).each do |item| + item['path'] = key + '/' + item['path'] + buckets << item end + else + buckets << {'path'=> key, 'value'=> val} + end + end + buckets + end + def array_comparison(path, original_hash, converted_hash) pre_translation_array = hash_navigation(path, original_hash) post_translation_array = hash_navigation(path, converted_hash) @@ -124,14 +163,14 @@ namespace :collection do (pre_translation_array - post_translation_array).each do |item| path_with_index = path + "[#{pre_translation_array.index(item)}]" # the following line is used to eliminate indexing confusion when there is more than one occurrence of a particular item in an array - pre_translation_array[pre_translation_array.index(item)] = item.to_s + 'item indexed' + pre_translation_array[pre_translation_array.index(item)] = item.to_s + ' item indexed' output << ['-', item, path_with_index] end (post_translation_array - pre_translation_array).each do |item| path_with_index = path + "[#{post_translation_array.index(item)}]" # the following line is used to eliminate indexing confusion when there is more than one occurrence of a particular item in an array - post_translation_array[post_translation_array.index(item)] = item.to_s + 'item indexed' + post_translation_array[post_translation_array.index(item)] = item.to_s + ' item indexed' output << ['+', item, path_with_index] end output From d5e920ddb7fdbe0842b3b5ac1d948662ed230872 Mon Sep 17 00:00:00 2001 From: Christian Trummer Date: Tue, 4 Aug 2020 09:48:40 -0400 Subject: [PATCH 33/34] MMT-2311-1: removed loss_report_helper file (it's only supposed to be in MMT-2313) --- app/helpers/loss_report_helper.rb | 229 ------------------------------ 1 file changed, 229 deletions(-) delete mode 100644 app/helpers/loss_report_helper.rb diff --git a/app/helpers/loss_report_helper.rb b/app/helpers/loss_report_helper.rb deleted file mode 100644 index e79dfb052..000000000 --- a/app/helpers/loss_report_helper.rb +++ /dev/null @@ -1,229 +0,0 @@ -module LossReportHelper - - def loss_report_output(concept_id, hide_items=true, disp='text') - # depending on the input selection (json or text) a comparison string/hash is created and displayed in-browser - - orig_xml,conv_xml,orig_h,conv_h,content_type = prepare_collections(concept_id, '1.15.3') - - if content_type.include?('iso') || content_type.include?('dif') - orig = Nokogiri::XML(orig_xml) { |config| config.strict.noblanks } .remove_namespaces! - conv = Nokogiri::XML(conv_xml) { |config| config.strict.noblanks } .remove_namespaces! - else - orig = Nokogiri::XML(orig_xml) { |config| config.strict.noblanks } - conv = Nokogiri::XML(conv_xml) { |config| config.strict.noblanks } - end - - #write files to test that all changes are being found with opendiff - dir = '/Users/ctrummer/Documents/devtesting' - File.write("#{dir}/o_#{concept_id}.xml", orig.to_xml) - File.write("#{dir}/c_#{concept_id}.xml", conv.to_xml) - - arr_paths = Array.new # This array is used to keep track of the paths that lead to arrays that have already been mapped - text_output = String.new if disp == 'text' - json_output = Hash.new if disp == 'json' - - # json_output['orig'] = hash_map(orig_h) if disp == 'json' - # json_output['orig'] = orig_h if disp == 'json' - # json_output['conv'] = conv_h if disp == 'json' - # text_output += orig_xml if disp == 'text' - - json_output['format'] = content_type if disp == 'json' - text_output += (content_type + "\n\n") if disp == 'text' - - # text_output += top_level_arr_path('/Collection/OnlineResources/OnlineResource', orig_h, conv_h).to_s+"\n" - - orig.diff(conv, {:added => true, :removed => true}) do |change,node| - element = node.to_xml - path = node.parent.path.split('[')[0] - arr_path = top_level_arr_path(path, orig_h, conv_h) - - # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- - puts "---------------------------------------------------------------------------------" - puts "arr_path: #{arr_path} ... node.parent.path: #{node.parent.path} ... path: #{path}" - # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- - - if arr_path && path_not_checked?(arr_path, arr_paths) - - # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- - puts "** path 1" - puts "ar path_not_checked?(arr_path,arr_paths): #{path_not_checked?(arr_path,arr_paths).to_s}" - # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- - - arr_paths << arr_path - array_comparison(arr_path, orig_h, conv_h).each do |item| # all lists - add_to_report('ar'+item[0], item[1], item[2], hide_items, disp, json_output, text_output) - end - - # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- - puts "arr_paths: #{arr_paths}" - # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- - - elsif path_not_checked?(path, arr_paths) # nokogiri - - # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- - puts "** path 2" - puts "path_not_checked?(path,arr_paths): #{path_not_checked?(path,arr_paths).to_s}" - # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- - - if is_xml?(node) - element = Hash.from_xml(element) - hash_map(element).each do |item| - arr_path = top_level_arr_path("#{path}/#{item['path']}", orig_h, conv_h) - - # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- - puts "path_not_checked?('path/item['path']}, arr_paths): #{path_not_checked?("#{path}/#{item['path']}", arr_paths)}" - # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- - - if arr_path && path_not_checked?("#{path}/#{item['path']}", arr_paths) # all list - if path_not_checked?(arr_path, arr_paths) - - # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- - puts "na path_not_checked?(arr_path, arr_paths): #{path_not_checked?(arr_path, arr_paths)}" - # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- - - arr_paths << arr_path - array_comparison(arr_path, orig_h, conv_h).each do |item| - add_to_report('na'+item[0], item[1], item[2], hide_items, disp, json_output, text_output) - end - - # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- - puts "arr_paths: #{arr_paths}" - # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- - - end - elsif path_not_checked?("#{path}/#{item['path']}", arr_paths) - add_to_report('hn'+change, item['value'], "#{path}/#{item['path']}", hide_items, disp, json_output, text_output) - end - - # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- - puts "arr_paths: #{arr_paths}" - # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- - - end - else - add_to_report('ng'+change, element, path, hide_items, disp, json_output, text_output) - - # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- - puts "arr_paths: #{arr_paths}" - # FOR TROUBLESHOOTING ------------------------------------------------------------------------------------- - - end - end - end - if disp == 'text' then return text_output - elsif disp == 'json' then return json_output end - end - - def path_not_checked?(arr_path, arr_paths) - arr_paths.each do |path| - if arr_path.include?(path) - return false - end - end - true - end - - def top_level_arr_path(path, orig_h, conv_h) - pre_translation_array, pre_translation_path = hash_navigation(path, orig_h) - post_translation_array, post_translation_path = hash_navigation(path, conv_h) - - return false if pre_translation_array == false && post_translation_array == false - - return pre_translation_path if pre_translation_array.is_a?(Array) - return post_translation_path if post_translation_array.is_a?(Array) - - # the number of keys must be 1 because all arrays in echo10, dif10, and iso19115 are tagged similar to: - # contact and so all array-containing tags will be the plural - # of the array name. This clause serves to identify array-containing tags when their paths aren't properly - # displayed by nokogiri - if pre_translation_array.is_a?(Hash) && pre_translation_array.keys.length == 1 && pre_translation_array[pre_translation_array.keys[0]].is_a?(Array) - return pre_translation_path + "/#{pre_translation_array.keys[0]}" - elsif post_translation_array.is_a?(Hash) && post_translation_array.keys.length == 1 && post_translation_array[post_translation_array.keys[0]].is_a?(Array) - return post_translation_path + "/#{post_translation_array.keys[0]}" - end - - path_contains_array = false - end - - def add_to_report(change, element, path, hide_items, disp, json_output, text_output) - @counter ||= 0 and @counter += 1 - - # this function serves to preclude complex nests from forming in loss_report_output the - # following 'if' structure is intended to increase readability by eliminating nests - return text_output.concat("#{@counter}.".ljust(4)+"#{change}: #{element}".ljust(60) + path + "\n") if hide_items == false && disp == 'text' - puts "#{@counter}.".ljust(4)+"#{change}: ".ljust(3) + path; return text_output.concat("#{@counter}.".ljust(4)+"#{change}: ".ljust(3) + path + "\n") if hide_items == true && disp == 'text' - return json_output["#{@counter}. #{change}: #{path}"] = element if disp == 'json' - end - - def hash_map(hash) - buckets = Array.new - hash.each do |key,val| - if val.is_a? Hash then hash_map(val).each do |item| - item['path'] = key + '/' + item['path'] - buckets << item end - else - buckets << {'path'=> key, 'value'=> val} - end - end - buckets - end - - def is_xml?(node) - if node.to_xml.include?('<' && '') then return true - else return false end - end - - def prepare_collections(concept_id, umm_c_version) - # TODO: need to add exception handling for get_concept, translate_collection - original_collection_native_xml = cmr_client.get_concept(concept_id,token, {}) - content_type = original_collection_native_xml.headers.fetch('content-type').split(';')[0] - original_collection_native_hash = Hash.from_xml(original_collection_native_xml.body) - translated_collection_umm_json = cmr_client.translate_collection(original_collection_native_xml.body, content_type, "application/vnd.nasa.cmr.umm+json;version=#{umm_c_version}", skip_validation=true) - translated_collection_native_xml = cmr_client.translate_collection(translated_collection_umm_json.body.to_json, "application/vnd.nasa.cmr.umm+json;version=#{umm_c_version}", content_type, skip_validation=true) - translated_collection_native_hash = Hash.from_xml(translated_collection_native_xml.body) - return original_collection_native_xml.body, translated_collection_native_xml.body, original_collection_native_hash, translated_collection_native_hash, content_type - end - - - def hash_navigation(path, hash) - # Passed a path string and the hash being navigated. This method parses the path string and - # returns the array/value at the end of the path - current_path = String.new - path.split('/').each do |key| - if hash.is_a?(Array) - return hash, current_path - elsif hash.key?(key) && hash.is_a?(Hash) - current_path += "/#{key}" - hash = hash[key] - elsif !hash.key?(key) && key != '' - return path_exists = false, "#{current_path}/#{key}" - end - end - return hash, current_path - end - - def array_comparison(path, original_hash, converted_hash) - pre_translation_array = hash_navigation(path, original_hash)[0] - post_translation_array = hash_navigation(path, converted_hash)[0] - - pre_translation_array == false ? pre_translation_array = Array.new : pre_translation_array = Array.wrap(pre_translation_array) - post_translation_array == false ? post_translation_array = Array.new : post_translation_array = Array.wrap(post_translation_array) - - output = Array.new - (pre_translation_array - post_translation_array).each do |item| - path_with_index = path + "[#{pre_translation_array.index(item)}]" - # the following line is used to eliminate indexing confusion when there is more than one occurrence of a particular item in an array - pre_translation_array[pre_translation_array.index(item)] = item.to_s + 'item indexed' - output << ['-', item, path_with_index] - end - - (post_translation_array - pre_translation_array).each do |item| - path_with_index = path + "[#{post_translation_array.index(item)}]" - # the following line is used to eliminate indexing confusion when there is more than one occurrence of a particular item in an array - post_translation_array[post_translation_array.index(item)] = item.to_s + 'item indexed' - output << ['+', item, path_with_index] - end - output - end - -end From 345bab452a4e0bd7d3a16d326d283a7ddb643b56 Mon Sep 17 00:00:00 2001 From: Christian Trummer Date: Tue, 4 Aug 2020 11:17:24 -0400 Subject: [PATCH 34/34] MMT-2311-1: removed unnecessary variable --- lib/tasks/compare_xml_collections.rake | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/tasks/compare_xml_collections.rake b/lib/tasks/compare_xml_collections.rake index 7c420b916..604d3bc83 100644 --- a/lib/tasks/compare_xml_collections.rake +++ b/lib/tasks/compare_xml_collections.rake @@ -35,7 +35,6 @@ namespace :collection do end arr_paths = Array.new # This array is used to keep track of the paths that lead to arrays that have already been mapped - text_output = String.new nokogiri_original.diff(nokogiri_converted, {:added => true, :removed => true}) do |change,node| element = node.to_xml