From c2bc31326d016aefbe8dbf88e6b6989c0804e149 Mon Sep 17 00:00:00 2001 From: Blanco Date: Mon, 22 Apr 2019 13:47:13 -0400 Subject: [PATCH] treat title as a single value even though it is multivalue on form --- app/forms/hyrax/forms/admin_set_form.rb | 8 ++++ app/forms/hyrax/forms/collection_form.rb | 37 +++++++++++++++++-- app/forms/hyrax/forms/work_form.rb | 26 ++++++++++++- app/indexers/hyrax/basic_metadata_indexer.rb | 2 +- app/models/admin_set.rb | 5 +++ app/models/concerns/hyrax/basic_metadata.rb | 2 + .../concerns/hyrax/solr_document/metadata.rb | 1 + app/presenters/hyrax/work_show_presenter.rb | 2 +- .../hyrax/dashboard/collections/edit.html.erb | 4 +- .../records/edit_fields/_alt_title.html.erb | 3 ++ app/views/records/edit_fields/_title.html.erb | 1 + spec/features/dashboard/collection_spec.rb | 6 +-- .../hyrax/forms/batch_upload_form_spec.rb | 3 +- .../forms/hyrax/forms/collection_form_spec.rb | 7 +++- spec/forms/hyrax/forms/work_form_spec.rb | 6 +-- spec/forms/hyrax/generic_work_form_spec.rb | 2 +- .../edit_fields/_alt_title.html.erb_spec.rb | 26 +++++++++++++ .../edit_fields/_title.html.erb_spec.rb | 24 ++++++++++++ 18 files changed, 145 insertions(+), 20 deletions(-) create mode 100644 app/views/records/edit_fields/_alt_title.html.erb create mode 100644 app/views/records/edit_fields/_title.html.erb create mode 100644 spec/views/records/edit_fields/_alt_title.html.erb_spec.rb create mode 100644 spec/views/records/edit_fields/_title.html.erb_spec.rb diff --git a/app/forms/hyrax/forms/admin_set_form.rb b/app/forms/hyrax/forms/admin_set_form.rb index c4cc8e2517..4760025935 100644 --- a/app/forms/hyrax/forms/admin_set_form.rb +++ b/app/forms/hyrax/forms/admin_set_form.rb @@ -7,6 +7,14 @@ class AdminSetForm < Hyrax::Forms::CollectionForm # Cast any array values on the model to scalars. def [](key) return super if key == :thumbnail_id + if key == :title + @attributes["title"].each do |value| + @attributes["alt_title"] << value + end + @attributes["alt_title"].delete(@attributes["alt_title"].sort.first) unless @attributes["alt_title"].empty? + return @attributes["title"].sort.first unless @attributes["title"].empty? + return "" + end super.first end diff --git a/app/forms/hyrax/forms/collection_form.rb b/app/forms/hyrax/forms/collection_form.rb index 95c7ad8731..98d5a87f73 100644 --- a/app/forms/hyrax/forms/collection_form.rb +++ b/app/forms/hyrax/forms/collection_form.rb @@ -7,7 +7,7 @@ class CollectionForm # Used by the search builder attr_reader :scope - delegate :id, :depositor, :permissions, :human_readable_type, :member_ids, :nestable?, to: :model + delegate :id, :depositor, :permissions, :human_readable_type, :member_ids, :nestable?, :alt_title, to: :model class_attribute :membership_service_class @@ -20,7 +20,7 @@ class CollectionForm delegate :blacklight_config, to: Hyrax::CollectionsController - self.terms = [:resource_type, :title, :creator, :contributor, :description, + self.terms = [:alt_title, :resource_type, :title, :creator, :contributor, :description, :keyword, :license, :publisher, :date_created, :subject, :language, :representative_id, :thumbnail_id, :identifier, :based_near, :related_url, :visibility, :collection_type_gid] @@ -41,6 +41,36 @@ def initialize(model, current_ability, repository) @scope = ProxyScope.new(current_ability, repository, blacklight_config) end + # Cast back to multi-value when saving + # Reads from form + def self.model_attributes(attributes) + attrs = super + return attrs unless attributes[:title] + + attrs[:title] = Array(attributes[:title]) + return attrs if attributes[:alt_title].nil? + Array(attributes[:alt_title]).each do |value| + attrs["title"] << value if value != "" + end + attrs + end + + # @param [Symbol] key the field to read + # @return the value of the form field. + # For display in edit page + def [](key) + return model.member_of_collection_ids if key == :member_of_collection_ids + if key == :title + @attributes["title"].each do |value| + @attributes["alt_title"] << value + end + @attributes["alt_title"].delete(@attributes["alt_title"].sort.first) unless @attributes["alt_title"].empty? + return @attributes["title"].sort.first unless @attributes["title"].empty? + return "" + end + super + end + def permission_template @permission_template ||= begin template_model = PermissionTemplate.find_or_create_by(source_id: model.id) @@ -60,7 +90,8 @@ def primary_terms # Terms that appear within the accordion def secondary_terms - [:creator, + [:alt_title, + :creator, :contributor, :keyword, :license, diff --git a/app/forms/hyrax/forms/work_form.rb b/app/forms/hyrax/forms/work_form.rb index 75c711e710..fba6c18da5 100644 --- a/app/forms/hyrax/forms/work_form.rb +++ b/app/forms/hyrax/forms/work_form.rb @@ -17,11 +17,11 @@ class WorkForm :visibility_during_embargo, :embargo_release_date, :visibility_after_embargo, :visibility_during_lease, :lease_expiration_date, :visibility_after_lease, :visibility, :in_works_ids, :depositor, :on_behalf_of, :permissions, - :member_ids, to: :model + :member_ids, :alt_title, to: :model attr_reader :agreement_accepted - self.terms = [:title, :creator, :contributor, :description, :abstract, + self.terms = [:title, :alt_title, :creator, :contributor, :description, :abstract, :keyword, :license, :rights_statement, :access_right, :rights_notes, :publisher, :date_created, :subject, :language, :identifier, :based_near, :related_url, :representative_id, :thumbnail_id, :rendering_ids, :files, @@ -42,6 +42,20 @@ def initialize(model, current_ability, controller) super(model) end + # Cast back to multi-value when saving + # Reads from form + def self.model_attributes(attributes) + attrs = super + return attrs unless attributes[:title] + + attrs[:title] = Array(attributes[:title]) + return attrs if attributes[:alt_title].nil? + Array(attributes[:alt_title]).each do |value| + attrs["title"] << value if value != "" + end + attrs + end + # when the add_works_to_collection parameter is set, they mean to create # a new work and add it to that collection. def member_of_collections @@ -95,8 +109,16 @@ def initialize_field(key) # @param [Symbol] key the field to read # @return the value of the form field. + # For display in edit page def [](key) return model.member_of_collection_ids if key == :member_of_collection_ids + if key == :title + @attributes["title"].each do |value| + @attributes["alt_title"] << value + end + @attributes["alt_title"].delete(@attributes["alt_title"].sort.first) unless @attributes["alt_title"].empty? + return @attributes[key.to_s].sort.first + end super end diff --git a/app/indexers/hyrax/basic_metadata_indexer.rb b/app/indexers/hyrax/basic_metadata_indexer.rb index 3f43179a6f..5bcedd0dd9 100644 --- a/app/indexers/hyrax/basic_metadata_indexer.rb +++ b/app/indexers/hyrax/basic_metadata_indexer.rb @@ -3,7 +3,7 @@ module Hyrax class BasicMetadataIndexer < ActiveFedora::RDF::IndexingService class_attribute :stored_and_facetable_fields, :stored_fields, :symbol_fields self.stored_and_facetable_fields = %i[resource_type creator contributor keyword publisher subject language based_near] - self.stored_fields = %i[description abstract license rights_statement rights_notes access_right date_created identifier related_url bibliographic_citation source] + self.stored_fields = %i[alt_title description abstract license rights_statement rights_notes access_right date_created identifier related_url bibliographic_citation source] self.symbol_fields = %i[import_url] private diff --git a/app/models/admin_set.rb b/app/models/admin_set.rb index 64baf02de6..a4faafd427 100644 --- a/app/models/admin_set.rb +++ b/app/models/admin_set.rb @@ -31,6 +31,11 @@ class AdminSet < ActiveFedora::Base property :title, predicate: ::RDF::Vocab::DC.title do |index| index.as :stored_searchable, :facetable end + + property :alt_title, predicate: ::RDF::Vocab::DC.alternative do |index| + index.as :stored_searchable + end + property :description, predicate: ::RDF::Vocab::DC.description do |index| index.as :stored_searchable end diff --git a/app/models/concerns/hyrax/basic_metadata.rb b/app/models/concerns/hyrax/basic_metadata.rb index 47790852bd..542c8d56fe 100644 --- a/app/models/concerns/hyrax/basic_metadata.rb +++ b/app/models/concerns/hyrax/basic_metadata.rb @@ -6,6 +6,8 @@ module BasicMetadata extend ActiveSupport::Concern included do + property :alt_title, predicate: ::RDF::Vocab::DC.alternative + property :label, predicate: ActiveFedora::RDF::Fcrepo::Model.downloadFilename, multiple: false property :relative_path, predicate: ::RDF::URI.new('http://scholarsphere.psu.edu/ns#relativePath'), multiple: false diff --git a/app/models/concerns/hyrax/solr_document/metadata.rb b/app/models/concerns/hyrax/solr_document/metadata.rb index 31cabe1422..02216987ec 100644 --- a/app/models/concerns/hyrax/solr_document/metadata.rb +++ b/app/models/concerns/hyrax/solr_document/metadata.rb @@ -44,6 +44,7 @@ def self.coerce(input) end included do + attribute :alt_title, Solr::Array, solr_name('alt_title') attribute :identifier, Solr::Array, solr_name('identifier') attribute :based_near, Solr::Array, solr_name('based_near') attribute :based_near_label, Solr::Array, solr_name('based_near_label') diff --git a/app/presenters/hyrax/work_show_presenter.rb b/app/presenters/hyrax/work_show_presenter.rb index 3c81590513..874655c422 100644 --- a/app/presenters/hyrax/work_show_presenter.rb +++ b/app/presenters/hyrax/work_show_presenter.rb @@ -39,7 +39,7 @@ def page_title delegate :title, :date_created, :description, :creator, :contributor, :subject, :publisher, :language, :embargo_release_date, :lease_expiration_date, :license, :source, :rights_statement, :thumbnail_id, :representative_id, - :rendering_ids, :member_of_collection_ids, to: :solr_document + :rendering_ids, :member_of_collection_ids, :alt_title, to: :solr_document def workflow @workflow ||= WorkflowPresenter.new(solr_document, current_ability) diff --git a/app/views/hyrax/dashboard/collections/edit.html.erb b/app/views/hyrax/dashboard/collections/edit.html.erb index d242f6aeef..07dbb28881 100644 --- a/app/views/hyrax/dashboard/collections/edit.html.erb +++ b/app/views/hyrax/dashboard/collections/edit.html.erb @@ -1,7 +1,7 @@ -<% provide :page_title, construct_page_title( t('.header', type_title: @collection.collection_type.title, title: @form.title.first) ) %> +<% provide :page_title, construct_page_title( t('.header', type_title: @collection.collection_type.title, title: @form.title) ) %> <% provide :page_header do %> -

<%= t('.header', type_title: @collection.collection_type.title, title: @form.title.first) %>

+

<%= t('.header', type_title: @collection.collection_type.title, title: @form.title) %>

<% end %>
diff --git a/app/views/records/edit_fields/_alt_title.html.erb b/app/views/records/edit_fields/_alt_title.html.erb new file mode 100644 index 0000000000..11e040d941 --- /dev/null +++ b/app/views/records/edit_fields/_alt_title.html.erb @@ -0,0 +1,3 @@ +<% f.object.alt_title.each do |value| %> + <%= f.hidden_field :alt_title, multiple: true, value: value.to_s %> +<% end %> diff --git a/app/views/records/edit_fields/_title.html.erb b/app/views/records/edit_fields/_title.html.erb new file mode 100644 index 0000000000..cefb71391d --- /dev/null +++ b/app/views/records/edit_fields/_title.html.erb @@ -0,0 +1 @@ +<%= f.input :title, required: f.object.required?(:title) %> \ No newline at end of file diff --git a/spec/features/dashboard/collection_spec.rb b/spec/features/dashboard/collection_spec.rb index aa68029c24..f8dd8d0a43 100644 --- a/spec/features/dashboard/collection_spec.rb +++ b/spec/features/dashboard/collection_spec.rb @@ -244,7 +244,6 @@ click_on('Create collection') expect(page).to have_selector('h1', text: 'New User Collection') - expect(page).to have_selector "input.collection_title.multi_value" click_link('Additional fields') expect(page).to have_selector "input.collection_creator.multi_value" @@ -275,7 +274,6 @@ it 'makes a new collection' do click_link "New Collection" expect(page).to have_selector('h1', text: 'New User Collection') - expect(page).to have_selector "input.collection_title.multi_value" click_link('Additional fields') expect(page).to have_selector "input.collection_creator.multi_value" @@ -335,7 +333,7 @@ let!(:adminset) { create(:admin_set, title: ['Admin Set with Work'], creator: [admin_user.user_key], with_permission_template: true) } let!(:work) { create(:work, title: ["King Louie"], admin_set: adminset, member_of_collections: [collection], user: user) } - # check table row has appropriate data attributes added + # Check table row has appropriate data attributes added def check_tr_data_attributes(id, type) url_fragment = get_url_fragment(type) expect(page).to have_selector("tr[data-id='#{id}'][data-colls-hash]") @@ -343,7 +341,7 @@ def check_tr_data_attributes(id, type) expect(page).to have_selector("tr[data-post-delete-url='/#{url_fragment}/#{id}?locale=en']") end - # check data attributes have been transferred from table row to the modal + # Check data attributes have been transferred from table row to the modal def check_modal_data_attributes(id, type) url_fragment = get_url_fragment(type) expect(page).to have_selector("div[data-id='#{id}']") diff --git a/spec/forms/hyrax/forms/batch_upload_form_spec.rb b/spec/forms/hyrax/forms/batch_upload_form_spec.rb index a5d6d2bd6a..29437ef300 100644 --- a/spec/forms/hyrax/forms/batch_upload_form_spec.rb +++ b/spec/forms/hyrax/forms/batch_upload_form_spec.rb @@ -41,7 +41,8 @@ subject { form.terms } it do - is_expected.to eq [:creator, + is_expected.to eq [:alt_title, + :creator, :contributor, :description, :abstract, diff --git a/spec/forms/hyrax/forms/collection_form_spec.rb b/spec/forms/hyrax/forms/collection_form_spec.rb index c376028c44..799a788fa6 100644 --- a/spec/forms/hyrax/forms/collection_form_spec.rb +++ b/spec/forms/hyrax/forms/collection_form_spec.rb @@ -3,7 +3,8 @@ subject { described_class.terms } it do - is_expected.to eq [:resource_type, + is_expected.to eq [:alt_title, + :resource_type, :title, :creator, :contributor, @@ -40,6 +41,7 @@ it do is_expected.to eq [ + :alt_title, :creator, :contributor, :keyword, @@ -127,7 +129,8 @@ subject { described_class.build_permitted_params } it do - is_expected.to eq [{ resource_type: [] }, + is_expected.to eq [{ alt_title: [] }, + { resource_type: [] }, { title: [] }, { creator: [] }, { contributor: [] }, diff --git a/spec/forms/hyrax/forms/work_form_spec.rb b/spec/forms/hyrax/forms/work_form_spec.rb index 9353e09966..8a44978b43 100644 --- a/spec/forms/hyrax/forms/work_form_spec.rb +++ b/spec/forms/hyrax/forms/work_form_spec.rb @@ -100,7 +100,7 @@ let(:params) { ActionController::Parameters.new(attributes) } let(:attributes) do { - title: ['foo'], + title: ['aaa', 'bbb', 'ccc'], description: [''], abstract: [''], visibility: 'open', @@ -118,8 +118,8 @@ subject { described_class.model_attributes(params) } - it 'permits metadata parameters' do - expect(subject['title']).to eq ['foo'] + it 'permits parameters' do + expect(subject['title']).to eq ['aaa', 'bbb', 'ccc'] expect(subject['description']).to be_empty expect(subject['abstract']).to be_empty expect(subject['visibility']).to eq 'open' diff --git a/spec/forms/hyrax/generic_work_form_spec.rb b/spec/forms/hyrax/generic_work_form_spec.rb index 4d953e5893..f10a3ea963 100644 --- a/spec/forms/hyrax/generic_work_form_spec.rb +++ b/spec/forms/hyrax/generic_work_form_spec.rb @@ -90,7 +90,7 @@ end it 'removes blank parameters' do - expect(subject['title']).to be_empty + expect(subject['title']).to eq [''] expect(subject['description']).to be_empty expect(subject['license']).to be_empty expect(subject['keyword']).to be_empty diff --git a/spec/views/records/edit_fields/_alt_title.html.erb_spec.rb b/spec/views/records/edit_fields/_alt_title.html.erb_spec.rb new file mode 100644 index 0000000000..d4aee8afbd --- /dev/null +++ b/spec/views/records/edit_fields/_alt_title.html.erb_spec.rb @@ -0,0 +1,26 @@ +RSpec.describe 'records/edit_fields/_title.html.erb', type: :view do + let(:work) { GenericWork.new } + let(:form) { Hyrax::GenericWorkForm.new(work, nil, controller) } + + let(:form_template) do + %( + <%= simple_form_for [main_app, @form] do |f| %> + <%= render "records/edit_fields/alt_title", f: f, key: 'description' %> + <% end %> + ) + end + + before do + work.title = ["bbb", "aaa", "ccc"] + work.alt_title = [] + assign(:form, form) + end + + context "when there are 3 titles" do + it 'hides the last 2 after alphabetizing all 3 titles' do + render inline: form_template + expect(rendered).to have_selector('input[type="hidden"][value="bbb"]', visible: false) + expect(rendered).to have_selector('input[type="hidden"][value="ccc"]', visible: false) + end + end +end diff --git a/spec/views/records/edit_fields/_title.html.erb_spec.rb b/spec/views/records/edit_fields/_title.html.erb_spec.rb new file mode 100644 index 0000000000..03563786a4 --- /dev/null +++ b/spec/views/records/edit_fields/_title.html.erb_spec.rb @@ -0,0 +1,24 @@ +RSpec.describe 'records/edit_fields/_title.html.erb', type: :view do + let(:work) { GenericWork.new } + let(:form) { Hyrax::GenericWorkForm.new(work, nil, controller) } + + let(:form_template) do + %( + <%= simple_form_for [main_app, @form] do |f| %> + <%= render "records/edit_fields/title", f: f, key: 'description' %> + <% end %> + ) + end + + before do + work.title = ["ccc", "bbb", "aaa"] + assign(:form, form) + end + + context "when there are 3 titles" do + it 'displays the first after alphabetizing the list' do + render inline: form_template + expect(rendered).to have_selector('input[class="form-control string required"][value="aaa"]') + end + end +end