From 509f2259956c2efb0a463b2db6a159d61c325192 Mon Sep 17 00:00:00 2001 From: Dolsy Smith Date: Thu, 5 Sep 2024 11:29:28 -0400 Subject: [PATCH] T458 license field display (#562) * Upgrade from Bulkrax 2.3.0 to 8.0.0, no configuration just yet * Fixes uploads-with-files issue by pointing to bulkrax branch * Work in Progress - tasks to ingest ProQuest ETD zips * WIP - next need to create CSV from array of metadata hashes * WIP - fixed problem creating header row * Fixed embargo logic; fixed CSV structure * Eliminated folder names from metadata csv FileSet entries; copy files to bulkrax zip staging directory, but will need to segregate files from each ETD into separate directoroes * Adds 'bulkrax_identifier' metadata; fixes imports of works w/files, using prerelease of next bulkrax release. * implemented parent work/child FileSet bulkrax_identifier, repaired embargo attributes * refactor file paths for extracted zip; parse creator/contributors * Repair attachment filenames with spaces (or else bulkrax will); fix author parsing * Add degree, advisors, committee members * Add gw_affiliation, date_created * Simplify embargo date; add rights statement; clean up * Fix truncated file; clarify configs, set default rights * Update bulkrax hash, now contains db migration fix * Code cleanup for PR * Add scholarspace-ingest directory and volume mapping * Add mapping for scholarspace-ingest directory * Add CI directive to create ingest folder * Upgrade Bulkrax to 8.1.0 * Allow admin user to visit /importers and /exporters even when there isn't yet an admin set for admin to deposit to * Add fixture zips for bulkrax rspec testing * Add sidekiq inline testing setting * Set testing queue for inline sidekiq * Modify ingest_bulkrax_prep when in test mode * Add bulkrax importer tests * Simplify bulkrax tests * Populates degree and resource_type. License is still WIP, pending input from ScholComm * Added resource_type field * updated Gemfile.lock * Re-apply bulkrax 5.5.1 changes * Upgrade bulkrax to 6.0.1 * Added display of license field; deprecated previous field; added new field * Fixed merge error in Gemfile * moved patch to folder that mirrors Hyrax repo location * Added rake task to update license field * Revised bulkrax spec test to use ENV variable for temp file path * Added tests * Fixed tests to use temp directory from environment * Fixed license & bulkrax tests* --------- Co-authored-by: Dan Kerchner Co-authored-by: Alex Boyd --- app/services/hyrax/qa_authorities_patch.rb | 12 +++++ app/views/hyrax/gw_etds/_attribute_rows.erb | 1 + app/views/hyrax/gw_works/_attribute_rows.erb | 1 + config/authorities/licenses.yml | 4 ++ lib/tasks/ingest_bulkrax_prep.rake | 7 +-- lib/tasks/replace_license_value.rake | 29 ++++++++++++ spec/factories/gw_works.rb | 6 ++- spec/features/bulkrax_upload_spec.rb | 33 +++++++------ spec/features/license_spec.rb | 50 ++++++++++++++++++++ 9 files changed, 121 insertions(+), 22 deletions(-) create mode 100644 app/services/hyrax/qa_authorities_patch.rb create mode 100644 lib/tasks/replace_license_value.rake create mode 100644 spec/features/license_spec.rb diff --git a/app/services/hyrax/qa_authorities_patch.rb b/app/services/hyrax/qa_authorities_patch.rb new file mode 100644 index 00000000..f70c3a94 --- /dev/null +++ b/app/services/hyrax/qa_authorities_patch.rb @@ -0,0 +1,12 @@ +module Hyrax + module QaSelectServicePatch + def include_current_value(value, _index, render_options, html_options) + unless value.blank? || active?(value) + html_options[:class][-1] += ' force-select' + render_options += [[label(value) { value }, value]] + end + [render_options, html_options] + end + end +end +Hyrax::QaSelectService.prepend Hyrax::QaSelectServicePatch \ No newline at end of file diff --git a/app/views/hyrax/gw_etds/_attribute_rows.erb b/app/views/hyrax/gw_etds/_attribute_rows.erb index a7dd78b5..c485958f 100644 --- a/app/views/hyrax/gw_etds/_attribute_rows.erb +++ b/app/views/hyrax/gw_etds/_attribute_rows.erb @@ -11,6 +11,7 @@ <%= presenter.attribute_to_html(:related_url, render_as: :external_link) %> <%= presenter.attribute_to_html(:resource_type, render_as: :faceted) %> <%= presenter.attribute_to_html(:source) %> +<%= presenter.attribute_to_html(:license, render_as: :license) %> <%= presenter.attribute_to_html(:rights_statement, render_as: :rights_statement) %> <%= presenter.attribute_to_html(:gw_affiliation, render_as: :faceted) %> <%= presenter.attribute_to_html(:degree, render_as: :faceted) %> diff --git a/app/views/hyrax/gw_works/_attribute_rows.erb b/app/views/hyrax/gw_works/_attribute_rows.erb index a7dd78b5..c485958f 100644 --- a/app/views/hyrax/gw_works/_attribute_rows.erb +++ b/app/views/hyrax/gw_works/_attribute_rows.erb @@ -11,6 +11,7 @@ <%= presenter.attribute_to_html(:related_url, render_as: :external_link) %> <%= presenter.attribute_to_html(:resource_type, render_as: :faceted) %> <%= presenter.attribute_to_html(:source) %> +<%= presenter.attribute_to_html(:license, render_as: :license) %> <%= presenter.attribute_to_html(:rights_statement, render_as: :rights_statement) %> <%= presenter.attribute_to_html(:gw_affiliation, render_as: :faceted) %> <%= presenter.attribute_to_html(:degree, render_as: :faceted) %> diff --git a/config/authorities/licenses.yml b/config/authorities/licenses.yml index 8b7ef40c..40c4c917 100644 --- a/config/authorities/licenses.yml +++ b/config/authorities/licenses.yml @@ -42,5 +42,9 @@ terms: term: CC0 1.0 Universal active: true - id: http://www.europeana.eu/portal/rights/rr-r.html + term: All rights reserved (deprecated) + active: false + - id: All rights reserved term: All rights reserved active: true + diff --git a/lib/tasks/ingest_bulkrax_prep.rake b/lib/tasks/ingest_bulkrax_prep.rake index b53082e5..7e1ffe03 100644 --- a/lib/tasks/ingest_bulkrax_prep.rake +++ b/lib/tasks/ingest_bulkrax_prep.rake @@ -144,11 +144,7 @@ namespace :gwss do work_metadata['date_created'] = etd_date_created unless etd_date_created.nil? work_metadata['committee_member'] = get_committee_members(doc).join(';') work_metadata['rights_statement'] = 'http://rightsstatements.org/vocab/InC/1.0/' - # Can't currently load this license because this Bulkrax code - # https://github.com/samvera/bulkrax/blob/v8.1.0/app/models/concerns/bulkrax/import_behavior.rb#L145-L146 - # will try to match it with http://www.europeana.eu/portal/rights/rr-r.html/ (slash-terminated) - # -- not finding a match, Bulkrax will throw an error. - # work_metadata['license'] = 'http://www.europeana.eu/portal/rights/rr-r.html' + work_metadata['license'] = 'All rights reserved' work_metadata end @@ -193,7 +189,6 @@ namespace :gwss do end bulkrax_files_path = "#{bulkrax_zip_path}/files" - puts "File.exists?(bulkrax_zip_path) = #{File.exists?(bulkrax_zip_path)}" FileUtils.makedirs("#{bulkrax_files_path}") unless File.exists?(bulkrax_zip_path) # get all ETD zip files in the args.filepath folder diff --git a/lib/tasks/replace_license_value.rake b/lib/tasks/replace_license_value.rake new file mode 100644 index 00000000..774e8b52 --- /dev/null +++ b/lib/tasks/replace_license_value.rake @@ -0,0 +1,29 @@ +require 'rake' + +namespace :gwss do + desc 'Replace license value for deprecated Europeana license' + + task :replace_license_value => :environment do + old_value = 'http://www.europeana.eu/portal/rights/rr-r.html' + new_value = 'All rights reserved' + + # Since the text field (tesim) doesn't allow exact searching we do an additional filter just to be safe + solr_results(old_value).each do |doc| + work = ActiveFedora::Base.find(doc['id']) # Retrieve the Fedora object + work.license = work.license.map { |license_value| license_value == old_value ? new_value : license_value } # Update the Fedora object + work.save + end + end +end + +def solr_results(old_value) + # Retrieve paginated results if necessary for records with the old value in the license field + params = {:q => "license_tesim:#{old_value}", fl:'id,license_tesim'} + docs = ActiveFedora::SolrService.instance.conn.paginate(1, 1000, 'select', :params => params).dig('response', 'docs') + p = 2 + while (next_page = ActiveFedora::SolrService.instance.conn.paginate(p, 1000, 'select', :params => params).dig('response', 'docs')) != [] + docs += next_page + p += 1 + end + docs +end diff --git a/spec/factories/gw_works.rb b/spec/factories/gw_works.rb index 92e5aad0..8127d592 100644 --- a/spec/factories/gw_works.rb +++ b/spec/factories/gw_works.rb @@ -20,7 +20,11 @@ end end - after(:create) do |work, _evaluator| + after(:create) do |work, options| + # Add the user's ability to the work -> necessary for enabling access to Edit pages + actor = Hyrax::CurationConcern.actor + actor_environment = Hyrax::Actors::Environment.new(work, Ability.new(options.user), {}) + actor.create(actor_environment) work.save! if work.try(:member_of_collections) && work.member_of_collections.present? end diff --git a/spec/features/bulkrax_upload_spec.rb b/spec/features/bulkrax_upload_spec.rb index 792871f1..17b48b41 100644 --- a/spec/features/bulkrax_upload_spec.rb +++ b/spec/features/bulkrax_upload_spec.rb @@ -7,34 +7,37 @@ before :all do # remove the folder so it doesn't repeatedly add new works when ingest task is run - FileUtils.rm_rf("#{Rails.root}/tmp/test/bulkrax_zip") + @temp_dir = "#{ENV['TEMP_FILE_BASE']}/test/bulkrax_zip" + if File.directory?(@temp_dir) + FileUtils.rm_rf(@temp_dir) + end Rake::Task["gwss:ingest_pq_etds"].invoke("#{Rails.root}/spec/fixtures/etd_zips") end it 'generates deposit file structure via gwss:ingest_pq_etds task' do - expect(File.directory?("#{Rails.root}/tmp/test/bulkrax_zip")).to be true - expect(File.directory?("#{Rails.root}/tmp/test/bulkrax_zip/files")).to be true + expect(File.directory?(@temp_dir)).to be true + expect(File.directory?("#{@temp_dir}/files")).to be true - expect(File.directory?("#{Rails.root}/tmp/test/bulkrax_zip/files/etdadmin_upload_1")).to be true - expect(File.file?("#{Rails.root}/tmp/test/bulkrax_zip/files/etdadmin_upload_1/Ab_gwu_0075A_16593_DATA.xml")).to be true - expect(File.file?("#{Rails.root}/tmp/test/bulkrax_zip/files/etdadmin_upload_1/Ab_gwu_0075A_16593.pdf")).to be true + expect(File.directory?("#{@temp_dir}/files/etdadmin_upload_1")).to be true + expect(File.file?("#{@temp_dir}/files/etdadmin_upload_1/Ab_gwu_0075A_16593_DATA.xml")).to be true + expect(File.file?("#{@temp_dir}/files/etdadmin_upload_1/Ab_gwu_0075A_16593.pdf")).to be true - expect(File.directory?("#{Rails.root}/tmp/test/bulkrax_zip/files/etdadmin_upload_2")).to be true - expect(File.file?("#{Rails.root}/tmp/test/bulkrax_zip/files/etdadmin_upload_2/Ab_gwu_0076A_12345_DATA.xml")).to be true - expect(File.file?("#{Rails.root}/tmp/test/bulkrax_zip/files/etdadmin_upload_2/Ab_gwu_0076A_12345.pdf")).to be true + expect(File.directory?("#{@temp_dir}/files/etdadmin_upload_2")).to be true + expect(File.file?("#{@temp_dir}/files/etdadmin_upload_2/Ab_gwu_0076A_12345_DATA.xml")).to be true + expect(File.file?("#{@temp_dir}/files/etdadmin_upload_2/Ab_gwu_0076A_12345.pdf")).to be true - expect(File.file?("#{Rails.root}/tmp/test/bulkrax_zip/metadata.csv")).to be true + expect(File.file?("#{@temp_dir}/metadata.csv")).to be true end it 'generates accurate CSV file for import' do - csv_rows = CSV.read("#{Rails.root}/tmp/test/bulkrax_zip/metadata.csv") + csv_rows = CSV.read("#{@temp_dir}/metadata.csv") headers_arr = csv_rows[0] - + print headers_arr expect(headers_arr).to eq(["model", "title", "creator", "contributor", "language", "description", "keyword", "degree", "resource_type", "advisor", "gw_affiliation", - "date_created", "committee_member", "rights_statement", "bulkrax_identifier", + "date_created", "committee_member", "rights_statement", "license", "bulkrax_identifier", "file", "parents", "visibility", "visibility_during_embargo", "visibility_after_embargo", "embargo_release_date"]) @@ -57,7 +60,7 @@ end it 'can deposit works via bulkrax import' do - admin_user = FactoryBot.create(:admin_user) + admin_user = FactoryBot.create(:admin) etds_admin_set = Hyrax::AdministrativeSet.new(title: ['ETDs']) etds_admin_set = Hyrax.persister.save(resource: etds_admin_set) Hyrax::AdminSetCreateService.call!(admin_set: etds_admin_set, creating_user: admin_user) @@ -74,7 +77,7 @@ import_parser_radio_button_elements.last.click import_parser_file_path_elements = page.all('//*[@id="importer_parser_fields_import_file_path"]') - import_parser_file_path_elements.last.fill_in with: "#{Rails.root}/tmp/test/bulkrax_zip/metadata.csv" + import_parser_file_path_elements.last.fill_in with: "#{@temp_dir}/metadata.csv" click_on("Create and Import") diff --git a/spec/features/license_spec.rb b/spec/features/license_spec.rb new file mode 100644 index 00000000..ffbd4968 --- /dev/null +++ b/spec/features/license_spec.rb @@ -0,0 +1,50 @@ +require 'rails_helper' + +RSpec.describe "View and edit license field" do + + let(:solr) { Blacklight.default_index.connection } + let(:admin_set) { FactoryBot.create(:admin_set) } + + let(:content_admin_user) { FactoryBot.create(:content_admin) } + + let(:work_with_license) { FactoryBot.create(:public_work, + admin_set: admin_set, + user: content_admin_user) } + + let(:another_license_value) { Hyrax::QaSelectService.new('licenses').select_active_options.first.first } + + before do + ActiveFedora::Cleaner.clean! + solr.delete_by_query("*:*") + + solr.add(work_with_license.to_solr) + + solr.commit + end + + after do + ActiveFedora::Cleaner.clean! + solr.delete_by_query("*:*") + solr.commit + end + + context 'as a user looking at a work bearing a license' do + it 'can view the license field' do + visit "/concern/gw_works/#{work_with_license.id}" + expect(page).to have_content(work_with_license.license.first) + end + end + context 'as a content-admin user' do + before :each do + sign_in_user(content_admin_user) + end + + it 'can edit the work containing the license field' do + visit "/concern/gw_works/#{work_with_license.id}/edit" + page.select another_license_value, :from => "gw_work_license" + page.click_on("Save changes") + expect(page).to have_content(another_license_value) + end + end + +end