diff --git a/.rubocop_fixme.yml b/.rubocop_fixme.yml index 446516ad6a..428ccde77a 100644 --- a/.rubocop_fixme.yml +++ b/.rubocop_fixme.yml @@ -9,6 +9,7 @@ Metrics/ClassLength: - 'app/controllers/hyrax/file_sets_controller.rb' - 'app/forms/hyrax/forms/permission_template_form.rb' - 'app/presenters/hyrax/work_show_presenter.rb' + - 'app/presenters/hyrax/collection_presenter.rb' - 'app/services/hyrax/user_stat_importer.rb' - 'lib/generators/hyrax/templates/catalog_controller.rb' - 'lib/generators/hyrax/install_generator.rb' diff --git a/app/assets/javascripts/hyrax/collections.js b/app/assets/javascripts/hyrax/collections.js index 3c6c0b5c09..eeca65d566 100644 --- a/app/assets/javascripts/hyrax/collections.js +++ b/app/assets/javascripts/hyrax/collections.js @@ -96,4 +96,16 @@ Blacklight.onLoad(function () { $('#collections-to-delete-deny-modal').modal('show'); } }); + + $('#show-more-parent-collections').on('click', function () { + $(this).hide(); + $("#more-parent-collections").show(); + $("#show-less-parent-collections").show(); + }); + + $('#show-less-parent-collections').on('click', function () { + $(this).hide(); + $("#more-parent-collections").hide(); + $("#show-more-parent-collections").show(); + }); }); diff --git a/app/assets/stylesheets/hyrax/_collections.scss b/app/assets/stylesheets/hyrax/_collections.scss index 356fd9e42d..d8bc71b834 100644 --- a/app/assets/stylesheets/hyrax/_collections.scss +++ b/app/assets/stylesheets/hyrax/_collections.scss @@ -624,3 +624,14 @@ button.branding-banner-remove:hover { margin-left: 0.5em; } + +.more-parent-collections-block { + display: none; +} + +.show-more-parent-collections-btn, +.show-less-parent-collections-btn { + color: #8598b7; + font-size: 0.8em; + margin-left: 10px; +} diff --git a/app/controllers/concerns/hyrax/collections_controller_behavior.rb b/app/controllers/concerns/hyrax/collections_controller_behavior.rb index d0b4f42247..96f7572d5d 100644 --- a/app/controllers/concerns/hyrax/collections_controller_behavior.rb +++ b/app/controllers/concerns/hyrax/collections_controller_behavior.rb @@ -80,10 +80,13 @@ def member_works end def parent_collections - col = @collection - col = Collection.find(collection.id) if col.blank? - @parent_collections = col.member_of_collections + @parent_collections = collection_object.member_of_collections @parent_collection_count = @parent_collections.size + collection.parent_collections = @parent_collections if action_name == 'show' + end + + def collection_object + action_name == 'show' ? Collection.find(collection.id) : collection end def member_subcollections diff --git a/app/controllers/hyrax/dashboard/collections_controller.rb b/app/controllers/hyrax/dashboard/collections_controller.rb index 52e54d84ef..af4fbe1e96 100644 --- a/app/controllers/hyrax/dashboard/collections_controller.rb +++ b/app/controllers/hyrax/dashboard/collections_controller.rb @@ -449,10 +449,13 @@ def member_subcollections end def parent_collections - col = @collection - col = Collection.find(collection.id) if col.blank? - @parent_collections = col.member_of_collections + @parent_collections = collection_object.member_of_collections @parent_collection_count = @parent_collections.size + collection.parent_collections = @parent_collections if action_name == 'show' + end + + def collection_object + action_name == 'show' ? Collection.find(collection.id) : collection end # You can override this method if you need to provide additional diff --git a/app/presenters/hyrax/collection_presenter.rb b/app/presenters/hyrax/collection_presenter.rb index c6a414f536..b21401277f 100644 --- a/app/presenters/hyrax/collection_presenter.rb +++ b/app/presenters/hyrax/collection_presenter.rb @@ -4,7 +4,9 @@ class CollectionPresenter include PresentsAttributes include ActionView::Helpers::NumberHelper attr_accessor :solr_document, :current_ability, :request - attr_writer :collection_type + attr_writer :collection_type, :parent_collections + + NUM_PARENTS_TO_SHOW = 3 class_attribute :create_work_presenter_class self.create_work_presenter_class = Hyrax::SelectTypeListPresenter @@ -29,19 +31,16 @@ def collection_type end # Metadata Methods - delegate :title, :description, :creator, :contributor, :subject, :publisher, :keyword, :language, - :embargo_release_date, :lease_expiration_date, :license, :date_created, - :resource_type, :based_near, :related_url, :identifier, :thumbnail_path, - :title_or_label, :collection_type_gid, :create_date, :modified_date, :visibility, :edit_groups, - :edit_people, + delegate :title, :description, :creator, :contributor, :subject, :publisher, :keyword, :language, :embargo_release_date, + :lease_expiration_date, :license, :date_created, :resource_type, :based_near, :related_url, :identifier, :thumbnail_path, + :title_or_label, :collection_type_gid, :create_date, :modified_date, :visibility, :edit_groups, :edit_people, to: :solr_document # Terms is the list of fields displayed by # app/views/collections/_show_descriptions.html.erb def self.terms - [:total_items, :size, :resource_type, :creator, :contributor, :keyword, - :license, :publisher, :date_created, :subject, :language, :identifier, - :based_near, :related_url] + [:total_items, :size, :resource_type, :creator, :contributor, :keyword, :license, :publisher, :date_created, :subject, + :language, :identifier, :based_near, :related_url] end def terms_with_values @@ -83,6 +82,24 @@ def collection_type_badge collection_type.title end + def parent_collection_count + @parent_collections.blank? ? 0 : @parent_collections.size + end + + def more_parent_collections? + parent_collection_count > NUM_PARENTS_TO_SHOW + end + + def visible_parent_collections + return @parent_collections[0..NUM_PARENTS_TO_SHOW - 1] if more_parent_collections? + @parent_collections || [] + end + + def more_parent_collections + return @parent_collections[NUM_PARENTS_TO_SHOW..parent_collection_count - 1] if more_parent_collections? + [] + end + def user_can_nest_collection? current_ability.can?(:deposit, solr_document) end diff --git a/app/views/hyrax/collections/_show_parent_collections.html.erb b/app/views/hyrax/collections/_show_parent_collections.html.erb index 8c3776f77d..aa70e39c75 100644 --- a/app/views/hyrax/collections/_show_parent_collections.html.erb +++ b/app/views/hyrax/collections/_show_parent_collections.html.erb @@ -1,14 +1,31 @@ -<% if @parent_collections.nil? || @parent_collections.empty? %> +<% if presenter.parent_collection_count <= 0 %> <% else %>

- <% @parent_collections.each do |document| %> +
+ <% presenter.visible_parent_collections.each do |document| %>
  • <%= link_to document, [hyrax, document], id: "src_copy_link_#{document.id}" %>
+ <% end %> + <% if presenter.more_parent_collections? %> + + <% end %> +
+ <% if presenter.more_parent_collections? %> +
+ <% presenter.more_parent_collections.each do |document| %> +
    +
  • + <%= link_to document, [hyrax, document], id: "src_copy_link_#{document.id}" %> +
    • +
+ + <% end %> +
<% end %>

diff --git a/app/views/hyrax/collections/show.html.erb b/app/views/hyrax/collections/show.html.erb index b92da34e9f..25ac3f2c83 100644 --- a/app/views/hyrax/collections/show.html.erb +++ b/app/views/hyrax/collections/show.html.erb @@ -56,18 +56,14 @@
<%= render 'collection_description', presenter: @presenter %> - <% if @presenter.collection_type_is_nestable? && @parent_collection_count > 0 %> -
-
-

- <%= t('.parent_collection_header') %> (<%= @parent_collection_count %>) -

-
+ <% if @presenter.collection_type_is_nestable? && @presenter.parent_collection_count > 0 %> +
+

+ <%= t('.parent_collection_header') %> (<%= @presenter.parent_collection_count %>) +

-
-
- <%= render 'show_parent_collections', collection: @parent_collections %> -
+
+ <%= render 'show_parent_collections', presenter: @presenter %>
<% end %> diff --git a/app/views/hyrax/dashboard/collections/_show_parent_collections.html.erb b/app/views/hyrax/dashboard/collections/_show_parent_collections.html.erb index 8917b2f5ea..456788b6e5 100644 --- a/app/views/hyrax/dashboard/collections/_show_parent_collections.html.erb +++ b/app/views/hyrax/dashboard/collections/_show_parent_collections.html.erb @@ -1,15 +1,32 @@ -<% if @parent_collections.nil? || @parent_collections.empty? %> - +<% if presenter.parent_collection_count <= 0 %> + <% else %> -
-

- <% @parent_collections.each do |document| %> -
    -
  • - <%= link_to document, [hyrax, :dashboard, document], id: "src_copy_link_#{document.id}" %> -
    • -
- <% end %> -

-
+
+

+
+ <% presenter.visible_parent_collections.each do |document| %> +
    +
  • + <%= link_to document, [hyrax, :dashboard, document], id: "src_copy_link_#{document.id}" %> +
    • +
+ <% end %> + <% if presenter.more_parent_collections? %> + + <% end %> +
+ <% if presenter.more_parent_collections? %> +
+ <% presenter.more_parent_collections.each do |document| %> +
    +
  • + <%= link_to document, [hyrax, :dashboard, document], id: "src_copy_link_#{document.id}" %> +
    • +
+ + <% end %> +
+ <% end %> +

+
<% end %> diff --git a/app/views/hyrax/dashboard/collections/show.html.erb b/app/views/hyrax/dashboard/collections/show.html.erb index 98fa2c7900..110e0eb31b 100644 --- a/app/views/hyrax/dashboard/collections/show.html.erb +++ b/app/views/hyrax/dashboard/collections/show.html.erb @@ -19,8 +19,8 @@
- <% if @presenter.collection_type_is_nestable? %> -

<%= t('.parent_collection_header') %> (<%= @parent_collection_count %>)

+ <% if @presenter.collection_type_is_nestable? && @presenter.parent_collection_count > 0 %> +

<%= t('.parent_collection_header') %> (<%= @presenter.parent_collection_count %>)

<%= render 'hyrax/dashboard/collections/show_parent_collections', presenter: @presenter %>
diff --git a/config/locales/hyrax.en.yml b/config/locales/hyrax.en.yml index a106f94b09..48fec3fce4 100644 --- a/config/locales/hyrax.en.yml +++ b/config/locales/hyrax.en.yml @@ -490,6 +490,8 @@ en: no_visible_parent_collections: "There are no visible parent collections." no_visible_subcollections: "There are no visible subcollections." parent_collection_header: "Parent Collections" + show_less_parent_collections: "...show less" + show_more_parent_collections: "show more..." subcollection_count: "Subcollections" works_in_collection: "Works" collection_type: @@ -631,6 +633,8 @@ en: parent_collection_header: "Parent Collections" public_view_label: "Public view of Collection" search_results: "Search Results within this Collection" + show_less_parent_collections: "...show less" + show_more_parent_collections: "show more..." subcollection_count: "Subcollections" collection_type_actions: close: "Close" diff --git a/spec/presenters/hyrax/collection_presenter_spec.rb b/spec/presenters/hyrax/collection_presenter_spec.rb index e027e573e6..53e00dc00f 100644 --- a/spec/presenters/hyrax/collection_presenter_spec.rb +++ b/spec/presenters/hyrax/collection_presenter_spec.rb @@ -274,6 +274,130 @@ it { is_expected.to eq '0 Bytes' } end + describe "#parent_collection_count" do + subject { presenter.parent_collection_count } + + context('when parent_collections is nil') do + before do + allow(presenter).to receive(:parent_collections).and_return(nil) + end + + it { is_expected.to eq 0 } + end + + context('when parent_collections is has no collections') do + before do + allow(presenter).to receive(:parent_collections).and_return([]) + end + + it { is_expected.to eq 0 } + end + + context('when parent_collections is has collections') do + let(:collection1) { build(:collection, title: ['col1']) } + let(:collection2) { build(:collection, title: ['col2']) } + let!(:parent_collections) { [collection1, collection2] } + + before do + presenter.parent_collections = parent_collections + end + + it { is_expected.to eq 2 } + end + end + + describe "#more_parent_collections?" do + subject { presenter.more_parent_collections? } + + context('when parent_collections is has no collections') do + before do + allow(presenter).to receive(:parent_collection_count).and_return(0) + end + + it { is_expected.to eq false } + end + + context('when parent_collections has less than or equal to show limit') do + before do + allow(presenter).to receive(:parent_collection_count).and_return(3) + end + + it { is_expected.to eq false } + end + + context('when parent_collections has more than show limit') do + before do + allow(presenter).to receive(:parent_collection_count).and_return(4) + end + + it { is_expected.to eq true } + end + end + + describe "#visible_parent_collections" do + subject { presenter.visible_parent_collections } + + let(:collection1) { build(:collection, title: ['col1']) } + let(:collection2) { build(:collection, title: ['col2']) } + let(:collection3) { build(:collection, title: ['col3']) } + let(:collection4) { build(:collection, title: ['col4']) } + let(:collection5) { build(:collection, title: ['col5']) } + + before do + presenter.parent_collections = parent_collections + end + + context('when parent_collections is nil') do + let(:parent_collections) { nil } + + it { is_expected.to eq [] } + end + + context('when parent_collections has less than or equal to show limit') do + let(:parent_collections) { [collection1, collection2, collection3] } + + it { is_expected.to include(collection1, collection2, collection3) } + end + + context('when parent_collections has more than show limit') do + let(:parent_collections) { [collection1, collection2, collection3, collection4, collection5] } + + it { is_expected.to include(collection1, collection2, collection3) } + end + end + + describe "#more_parent_collections" do + subject { presenter.more_parent_collections } + + let(:collection1) { build(:collection, title: ['col1']) } + let(:collection2) { build(:collection, title: ['col2']) } + let(:collection3) { build(:collection, title: ['col3']) } + let(:collection4) { build(:collection, title: ['col4']) } + let(:collection5) { build(:collection, title: ['col5']) } + + before do + presenter.parent_collections = parent_collections + end + + context('when parent_collections is nil') do + let(:parent_collections) { nil } + + it { is_expected.to eq [] } + end + + context('when parent_collections has less than or equal to show limit') do + let(:parent_collections) { [collection1, collection2, collection3] } + + it { is_expected.to eq [] } + end + + context('when parent_collections has more than show limit') do + let(:parent_collections) { [collection1, collection2, collection3, collection4, collection5] } + + it { is_expected.to include(collection4, collection5) } + end + end + describe "#user_can_nest_collection?" do before do allow(ability).to receive(:can?).with(:deposit, solr_doc).and_return(true) diff --git a/spec/views/hyrax/collections/_show_parent_collections.html.erb_spec.rb b/spec/views/hyrax/collections/_show_parent_collections.html.erb_spec.rb index 497c3040e5..11294e63b9 100644 --- a/spec/views/hyrax/collections/_show_parent_collections.html.erb_spec.rb +++ b/spec/views/hyrax/collections/_show_parent_collections.html.erb_spec.rb @@ -1,36 +1,68 @@ RSpec.describe 'hyrax/collections/_show_parent_collections.html.erb', type: :view do - let(:collection) { build(:named_collection, id: '123') } + let(:collection_doc) do + { + id: '999', + "has_model_ssim" => ["Collection"], + "title_tesim" => ["Title 1"], + 'date_created_tesim' => '2000-01-01' + } + end + let(:ability) { double } + let(:solr_document) { SolrDocument.new(collection_doc) } + let(:presenter) { Hyrax::CollectionPresenter.new(solr_document, ability) } + let(:collection1) { build(:collection, id: 'col1', title: ['col1']) } + let(:collection2) { build(:collection, id: 'col2', title: ['col2']) } + let(:collection3) { build(:collection, id: 'col3', title: ['col3']) } + let(:collection4) { build(:collection, id: 'col4', title: ['col4']) } + let(:collection5) { build(:collection, id: 'col5', title: ['col5']) } - context 'when parent collection list is empty' do - let(:parentcollection) { nil } + before do + assign(:presenter, presenter) + presenter.parent_collections = parent_collections - before do - assign(:parent_collections, parentcollection) - end + allow(collection1).to receive(:persisted?).and_return true + allow(collection2).to receive(:persisted?).and_return true + allow(collection3).to receive(:persisted?).and_return true + allow(collection4).to receive(:persisted?).and_return true + allow(collection5).to receive(:persisted?).and_return true + + render('show_parent_collections.html.erb', presenter: presenter) + end + + context 'when parent collection list is empty' do + let(:parent_collections) { nil } it "posts a warning message" do - render('show_parent_collections.html.erb', collection: parentcollection) expect(rendered).to have_text("There are no visible parent collections.") end end context 'when parent collection list is not empty' do - let(:parentcollection) { [collection] } - - before do - assign(:parent_collections, parentcollection) - assign(:document, collection) - allow(collection).to receive(:title_or_label).and_return(collection.title) - allow(collection).to receive(:persisted?).and_return true - render('show_parent_collections.html.erb', collection: parentcollection) - end + let(:parent_collections) { [collection1, collection2, collection3] } it "posts the collection's title with a link to the collection" do - expect(rendered).to have_link(collection.title.first) + expect(rendered).to have_link(collection1.title.first, visible: true) + expect(rendered).to have_link(collection2.title.first, visible: true) + expect(rendered).to have_link(collection3.title.first, visible: true) + expect(rendered).not_to have_button('show more...') end xit 'includes a count of the parent collections' do # TODO: add test when actual count is added to page end end + + context 'when parent collection list exceeds parents_to_show' do + let(:parent_collections) { [collection1, collection2, collection3, collection4, collection5] } + + it "posts the collection's title with a link to the collection" do + expect(rendered).to have_link(collection1.title.first, visible: true) + expect(rendered).to have_link(collection2.title.first, visible: true) + expect(rendered).to have_link(collection3.title.first, visible: true) + expect(rendered).to have_link(collection4.title.first, visible: false) + expect(rendered).to have_link(collection5.title.first, visible: false) + expect(rendered).to have_button('show more...', visible: true) + expect(rendered).to have_button('...show less', visible: false) + end + end end diff --git a/spec/views/hyrax/dashboard/collections/_show_parent_collections.html.erb_spec.rb b/spec/views/hyrax/dashboard/collections/_show_parent_collections.html.erb_spec.rb index 92fcecddcf..fd1b0598e8 100644 --- a/spec/views/hyrax/dashboard/collections/_show_parent_collections.html.erb_spec.rb +++ b/spec/views/hyrax/dashboard/collections/_show_parent_collections.html.erb_spec.rb @@ -1,36 +1,68 @@ RSpec.describe 'hyrax/dashboard/collections/_show_parent_collections.html.erb', type: :view do - let(:collection) { build(:named_collection, id: '123') } + let(:collection_doc) do + { + id: '999', + "has_model_ssim" => ["Collection"], + "title_tesim" => ["Title 1"], + 'date_created_tesim' => '2000-01-01' + } + end + let(:ability) { double } + let(:solr_document) { SolrDocument.new(collection_doc) } + let(:presenter) { Hyrax::CollectionPresenter.new(solr_document, ability) } + let(:collection1) { build(:collection, id: 'col1', title: ['col1']) } + let(:collection2) { build(:collection, id: 'col2', title: ['col2']) } + let(:collection3) { build(:collection, id: 'col3', title: ['col3']) } + let(:collection4) { build(:collection, id: 'col4', title: ['col4']) } + let(:collection5) { build(:collection, id: 'col5', title: ['col5']) } - context 'when parent collection list is empty' do - let(:parentcollection) { nil } + before do + assign(:presenter, presenter) + presenter.parent_collections = parent_collections - before do - assign(:parent_collections, parentcollection) - end + allow(collection1).to receive(:persisted?).and_return true + allow(collection2).to receive(:persisted?).and_return true + allow(collection3).to receive(:persisted?).and_return true + allow(collection4).to receive(:persisted?).and_return true + allow(collection5).to receive(:persisted?).and_return true + + render('show_parent_collections.html.erb', presenter: presenter) + end + + context 'when parent collection list is empty' do + let(:parent_collections) { nil } it "posts a warning message" do - render('show_parent_collections.html.erb', collection: parentcollection) expect(rendered).to have_text("There are no visible parent collections.") end end context 'when parent collection list is not empty' do - let(:parentcollection) { [collection] } - - before do - assign(:parent_collections, parentcollection) - assign(:document, collection) - allow(collection).to receive(:title_or_label).and_return(collection.title) - allow(collection).to receive(:persisted?).and_return true - render('show_parent_collections.html.erb', collection: parentcollection) - end + let(:parent_collections) { [collection1, collection2, collection3] } it "posts the collection's title with a link to the collection" do - expect(rendered).to have_link(collection.title.first) + expect(rendered).to have_link(collection1.title.first, visible: true) + expect(rendered).to have_link(collection2.title.first, visible: true) + expect(rendered).to have_link(collection3.title.first, visible: true) + expect(rendered).not_to have_button('show more...') end xit 'includes a count of the parent collections' do # TODO: add test when actual count is added to page end end + + context 'when parent collection list exceeds parents_to_show' do + let(:parent_collections) { [collection1, collection2, collection3, collection4, collection5] } + + it "posts the collection's title with a link to the collection" do + expect(rendered).to have_link(collection1.title.first, visible: true) + expect(rendered).to have_link(collection2.title.first, visible: true) + expect(rendered).to have_link(collection3.title.first, visible: true) + expect(rendered).to have_link(collection4.title.first, visible: false) + expect(rendered).to have_link(collection5.title.first, visible: false) + expect(rendered).to have_button('show more...', visible: true) + expect(rendered).to have_button('...show less', visible: false) + end + end end