From fb6495e8d252e6be2fcbd91329a2fc71741fc433 Mon Sep 17 00:00:00 2001 From: Justin Coyne Date: Mon, 4 Mar 2024 07:33:47 -0600 Subject: [PATCH] Bound with redesign for child records --- .../access-panel-library-locations.scss | 11 +-- .../stylesheets/modules/availability.scss | 30 +++++--- app/assets/stylesheets/sul-variables.scss | 1 + .../library_location_component.html.erb | 25 +++---- .../library_location_component.rb | 8 +++ .../location_item_component.html.erb | 69 ++++++++++-------- .../access_panels/location_item_component.rb | 31 ++++++-- .../search_result/location_component.html.erb | 2 +- .../location_item_component.html.erb | 29 ++++++++ .../search_result/location_item_component.rb | 58 +++++++++++++++ lib/holdings/item.rb | 12 ++-- lib/holdings/location.rb | 7 +- .../at_the_library_component_spec.rb | 69 ++++-------------- .../search_result/location_component_spec.rb | 37 +++------- .../access_panels/library_location_spec.rb | 4 +- spec/lib/holdings/item_spec.rb | 63 +++++----------- spec/lib/holdings/location_spec.rb | 71 ++++++------------- 17 files changed, 268 insertions(+), 259 deletions(-) create mode 100644 app/components/search_result/location_item_component.html.erb create mode 100644 app/components/search_result/location_item_component.rb diff --git a/app/assets/stylesheets/modules/access-panel-library-locations.scss b/app/assets/stylesheets/modules/access-panel-library-locations.scss index d61ba6f14..3313374df 100644 --- a/app/assets/stylesheets/modules/access-panel-library-locations.scss +++ b/app/assets/stylesheets/modules/access-panel-library-locations.scss @@ -74,13 +74,4 @@ .bi { // for location request link margin-right: 4px; } -} - -.bound-with-note { - margin-left: 10px; - - ul { - list-style-type: none; - padding-left: 0; - } -} +} \ No newline at end of file diff --git a/app/assets/stylesheets/modules/availability.scss b/app/assets/stylesheets/modules/availability.scss index 80163d216..5ff906b65 100644 --- a/app/assets/stylesheets/modules/availability.scss +++ b/app/assets/stylesheets/modules/availability.scss @@ -75,22 +75,36 @@ } .availability-item { - border-top: 1px solid $sul-availability-table-cell-border; - display: flex; - flex-direction: row; - flex-wrap: wrap; - justify-content: space-between; + border-top: 1px solid $fog-light; - &>.callnumber { + .callnumber { flex-grow: 1; } - &>.public-note { + .public-note { order: 2; width: 100%; } - &>.item-availability { + .item-availability { text-align: right; } + + .bound-with { + --link-decoration-line: none; + + background-color: $fog-light; + + .bound-with-type { + color: $cardinal-red; + } + .bound-with-title { + display: -webkit-box; + text-overflow: ellipsis; + overflow: hidden; + padding-bottom: 2px; // So we can see the text-decoration on the link. + -webkit-line-clamp: 3; + -webkit-box-orient: vertical; + } + } } diff --git a/app/assets/stylesheets/sul-variables.scss b/app/assets/stylesheets/sul-variables.scss index 1a4d0798d..079db6b83 100644 --- a/app/assets/stylesheets/sul-variables.scss +++ b/app/assets/stylesheets/sul-variables.scss @@ -10,6 +10,7 @@ $clay: #5f574f; $cloud: #dad7cb; $cool-grey: #4d4f53; $driftwood: #b6b1a9; +$fog-light: #f4f4f4; $gray-80-percent: #ccc; $lagunita: #007C92; $light-grey: #f3f3f3; diff --git a/app/components/access_panels/library_location_component.html.erb b/app/components/access_panels/library_location_component.html.erb index 3ee62ab4d..9775eae6b 100644 --- a/app/components/access_panels/library_location_component.html.erb +++ b/app/components/access_panels/library_location_component.html.erb @@ -1,7 +1,5 @@ <%= render partial: "catalog/stackmap_link", locals: { document: document, location: location, location_name_class: "location-name" } %> -<% if location.bound_with? && document.bound_with_note? %> - <%= content_tag(:div, class: 'bound-with-note note-highlight') { render document.bound_with_note_for_access_panel } %> -<% else %> +<% if !location.bound_with? %> <%= render location_request_link %> <% end %> <% if location.mhld.present? %> @@ -17,16 +15,13 @@ <% end %> <% end %> <% end %> - - - - - - - - - - <%= render LongTableComponent.new(node_id: "location_#{location.code}", size: location.items.size) do %> - <%= render AccessPanels::LocationItemComponent.with_collection(location.items, document:, classes: 'availability-item', render_item_level_request_link: !location_request_link.render?) %> +<%= tag.ul class: "availability record-view list-unstyled", id: list_identifier, data: truncate? ? { controller: "long-list" } : nil do %> + <%= render AccessPanels::LocationItemComponent.with_collection(location.items, document:, classes: 'availability-item', render_item_level_request_link: !location_request_link.render?) %> + <% if truncate? %> +
  • + + +
    • <% end %> -
    Items in <%= location.name %>
    Call numberNoteStatus
    + +<% end %> \ No newline at end of file diff --git a/app/components/access_panels/library_location_component.rb b/app/components/access_panels/library_location_component.rb index 67a9e4ec5..bc754be1d 100644 --- a/app/components/access_panels/library_location_component.rb +++ b/app/components/access_panels/library_location_component.rb @@ -18,5 +18,13 @@ def initialize(location:, library:, document:) def location_request_link @location_request_link ||= LocationRequestLinkComponent.for(document:, library_code: library.code, location:) end + + def truncate? + location.items.size > 5 + end + + def list_identifier + "location_#{location.code}" + end end end diff --git a/app/components/access_panels/location_item_component.html.erb b/app/components/access_panels/location_item_component.html.erb index ae65382f8..eb3f658b0 100644 --- a/app/components/access_panels/location_item_component.html.erb +++ b/app/components/access_panels/location_item_component.html.erb @@ -1,35 +1,42 @@ -<%= tag.tr class: classes, data: { barcode: item.barcode } do %> - - <%= item.callnumber %> - - <% if render_item_note? %> - - <% if item.public_note.present? %> -
    Note: <%= inject_line_break_opportunities item.public_note %>
    - <% end %> - - <% end %> - <% if render_item_details? %> - > - - - - <%= availability_text %> +<%= tag.li class: classes, data: html_data do %> +
    +
    + <%= item.callnumber %> +
    + <% if render_item_details? %> +
    > + + + + <%= availability_text %> + - - - <%= temporary_location_text %> - - <% if item.on_reserve? %> - <%= item.loan_period %> - <% end %> - <% if render_item_level_request_link? %> - - <%= render item.request_link %> + + <%= temporary_location_text %> - <% end %> - - <% else %> - + <% if item.on_reserve? %> + <%= item.loan_period %> + <% end %> + <% if render_item_level_request_link? %> + + <%= render item.request_link %> + + <% end %> +
    + <% end %> +
    + +
    + <% if item.public_note.present? %> +
    Note: <%= inject_line_break_opportunities item.public_note %>
    + <% end %> +
    + + <% if bound_with_parent? %> +
    +
    Bound and shelved with
    +
    <%= link_to bound_with_title, solr_document_path(item.bound_with_hrid) %>
    +
    <%= bound_with_callnumber %>
    +
    <% end %> <% end %> diff --git a/app/components/access_panels/location_item_component.rb b/app/components/access_panels/location_item_component.rb index 4385e0b1d..1b84c0ea8 100644 --- a/app/components/access_panels/location_item_component.rb +++ b/app/components/access_panels/location_item_component.rb @@ -3,16 +3,37 @@ module AccessPanels class LocationItemComponent < ViewComponent::Base with_collection_parameter :item - attr_reader :item, :document, :classes + attr_reader :item, :document, :classes, :counter - def initialize(item:, document:, classes: nil, render_item_level_request_link: true, render_item_note: true) + def initialize(item:, document:, item_counter:, classes: nil, render_item_level_request_link: true) super @item = item @classes = classes @document = document @render_item_level_request_link = render_item_level_request_link - @render_item_note = render_item_note + @counter = item_counter + end + + delegate :bound_with_parent, to: :item + + def html_data + data = { barcode: item.barcode } + return data if counter < 5 + + data.merge(long_list_target: 'hideable') + end + + def bound_with_parent? + bound_with_parent.present? + end + + def bound_with_title + bound_with_parent['title'] + end + + def bound_with_callnumber + [bound_with_parent['call_number'], bound_with_parent['volume'], bound_with_parent['enumeration'], bound_with_parent['chronology']].compact.join(' ') end def availability_text @@ -36,10 +57,6 @@ def render_item_details? !item.suppressed? end - def render_item_note? - @render_item_note - end - def render_item_level_request_link? @render_item_level_request_link end diff --git a/app/components/search_result/location_component.html.erb b/app/components/search_result/location_component.html.erb index 1c913f158..f5ba69d1d 100644 --- a/app/components/search_result/location_component.html.erb +++ b/app/components/search_result/location_component.html.erb @@ -43,7 +43,7 @@ <% if show_items? %> - <%= render AccessPanels::LocationItemComponent.with_collection(location.items, document:, render_item_level_request_link: !location_request_link.render?, render_item_note: false) %> + <%= render SearchResult::LocationItemComponent.with_collection(location.items, document:, render_item_level_request_link: !location_request_link.render?) %> <% else %> <%= link_to_record %> diff --git a/app/components/search_result/location_item_component.html.erb b/app/components/search_result/location_item_component.html.erb new file mode 100644 index 000000000..53c4800f6 --- /dev/null +++ b/app/components/search_result/location_item_component.html.erb @@ -0,0 +1,29 @@ +<%= tag.tr class: classes, data: { barcode: item.barcode } do %> + + <%= item.callnumber %> + + + <% if render_item_details? %> + > + + + + <%= availability_text %> + + + + <%= temporary_location_text %> + + <% if item.on_reserve? %> + <%= item.loan_period %> + <% end %> + <% if render_item_level_request_link? %> + + <%= render item.request_link %> + + <% end %> + + <% else %> + + <% end %> +<% end %> diff --git a/app/components/search_result/location_item_component.rb b/app/components/search_result/location_item_component.rb new file mode 100644 index 000000000..442962e65 --- /dev/null +++ b/app/components/search_result/location_item_component.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +module SearchResult + class LocationItemComponent < ViewComponent::Base + with_collection_parameter :item + attr_reader :item, :document, :classes + + def initialize(item:, document:, classes: nil, render_item_level_request_link: true) + super + + @item = item + @classes = classes + @document = document + @render_item_level_request_link = render_item_level_request_link + end + + def availability_text + item.status.status_text unless temporary_location_text + end + + def temporary_location_text + return if item.effective_location&.details&.key?('availabilityClass') || + item.effective_location&.details&.key?('searchworksTreatTemporaryLocationAsPermanentLocation') || + item.effective_permanent_location_code == item.temporary_location_code + + item.temporary_location&.name + end + + def has_in_process_availability_class? + availability_class = item.effective_location&.details&.dig('availabilityClass') + availability_class.present? && availability_class == 'In_process' + end + + def render_item_details? + !item.suppressed? + end + + def render_item_level_request_link? + @render_item_level_request_link + end + + def render_real_time_availability_request_link? + # we're not rendering item-level request links (because e.g. there's already alocation level request link) + return false unless render_item_level_request_link? + + # don't render unless item is requestable + return false unless item.requestable? + + # non-circulating items don't need real time availability + return false unless item.circulates? + + # items that definitely have an item-level request link at render time don't need real time availability + return false if item.request_link.render? + + true + end + end +end diff --git a/lib/holdings/item.rb b/lib/holdings/item.rb index bb7c76617..a70207619 100644 --- a/lib/holdings/item.rb +++ b/lib/holdings/item.rb @@ -62,6 +62,10 @@ def type item_display[:type] end + def bound_with_hrid + bound_with_parent[:hrid] + end + def truncated_callnumber item_display[:lopped_callnumber] end @@ -107,14 +111,10 @@ def on_reserve? end def bound_with_parent - return nil unless document&.folio_holdings&.any? - - match = document.folio_holdings.find do |holding| - holding.bound_with_parent&.dig('item', 'id') == id - end - match&.bound_with_parent + item_display[:bound_with] end + # @return [Bool] true if this is a bound-with child def bound_with? bound_with_parent.present? end diff --git a/lib/holdings/location.rb b/lib/holdings/location.rb index 9c04a9942..c207fd92f 100644 --- a/lib/holdings/location.rb +++ b/lib/holdings/location.rb @@ -31,10 +31,15 @@ def details Folio::Locations.details(code: @code) || {} end + # @return [Bool] true if any of the items in this location bound-with children def bound_with? items.any?(&:bound_with?) end + def bound_with_parents + items.filter_map(&:bound_with_parent) + end + # Intentionally left blank def location_link; end @@ -52,7 +57,7 @@ def sort # This prevents logging too much data when there is an error. def inspect - "<##{this.class.class_name} @code=#{@code}>" + "<##{self.class.class_name} @code=#{@code}>" end private diff --git a/spec/components/access_panels/at_the_library_component_spec.rb b/spec/components/access_panels/at_the_library_component_spec.rb index 310482558..2388e82d4 100644 --- a/spec/components/access_panels/at_the_library_component_spec.rb +++ b/spec/components/access_panels/at_the_library_component_spec.rb @@ -70,47 +70,15 @@ SolrDocument.new( id: '1234', item_display_struct: [ - { barcode: '1234', library: 'SAL3', effective_permanent_location_code: 'SAL3-STACKS', callnumber: 'ABC 123' } - ], - marc_json_struct: linked_ckey_fixture, - holdings_json_struct: [ - { holdings: [ - { - id: 'holding1234', - location: { - effectiveLocation: { - id: "158168a3-ede4-4cc1-8c98-61f4feeb22ea", - code: "SAL3-SEE-OTHER", - name: "See linked record to request items bound together", - campus: { - id: "c365047a-51f2-45ce-8601-e421ca3615c5", - code: "SUL", - name: "Stanford Libraries" - }, - details: {}, - library: { - id: "ddd3bce1-9f8f-4448-8d6d-b6c1b3907ba9", - code: "SAL3", - name: "SAL3 (off-campus storage)" - }, - institution: { - id: "8d433cdd-4e8f-4dc1-aa24-8a4ddb7dc929", - code: "SU", - name: "Stanford University" - } - } - }, - holdingsType: { - id: "5b08b35d-aaa3-4806-998c-9cd85e5bc406", - name: "Bound-with" - }, - boundWith: { - item: { barcode: "1234" }, - instance: { id: "7e194e58-e134-56fe-a3c2-2c0494e04c5b" } - } + { + barcode: '1234', library: 'SAL3', effective_permanent_location_code: 'SAL3-STACKS', + callnumber: 'ABC 123', + bound_with: { + hrid: 'a999999' } - ] } - ] + } + ], + marc_json_struct: linked_ckey_fixture ) end @@ -118,18 +86,13 @@ render_inline(described_class.new(document:)) end - it 'displays the MARC 590 as a bound with note (excluding subfield $c)' do - expect(page).to have_css('.bound-with-note.note-highlight a', text: 'Copy 1 bound with v. 140') - expect(page).to have_no_css('.bound-with-note.note-highlight', text: '55523 (parent record’s ckey)') - end - it "does not display request links for requestable libraries" do expect(page).to have_no_content("Request") end it 'displays the callnumber with live lookup' do - expect(page).to have_css 'td', text: 'ABC 123' - expect(page).to have_css 'td[data-live-lookup-id]' + expect(page).to have_content 'ABC 123' + expect(page).to have_css '[data-live-lookup-id]' end end @@ -251,7 +214,7 @@ expect(page).to have_css('.panel-library-location .mhld', text: "public note") expect(page).to have_css('.panel-library-location .mhld.note-highlight', text: "Latest: latest received") expect(page).to have_css('.panel-library-location .mhld', text: "Library has: library has") - expect(page).to have_css('.panel-library-location td', text: "ABC 123") + expect(page).to have_css('.panel-library-location .callnumber', text: "ABC 123") end end @@ -353,12 +316,8 @@ render_inline(described_class.new(document:)) end - skip "should not have a request url stored in the data attribute" do - expect(page).to have_no_css('td[data-request-url]') - end - - it "should have a request link in the item" do - expect(page).to have_css('tbody a', text: 'Request') + it "has a request link in the item" do + expect(page).to have_link 'Request' end end @@ -400,7 +359,7 @@ expect(page).to have_css('.panel-library-location a', count: 1) end it "renders blank (i.e. on order) items in the zombie library" do - expect(page).to have_css('.panel-library-location td', text: 'GHI') + expect(page).to have_css('.panel-library-location .callnumber', text: 'GHI') end end diff --git a/spec/components/search_result/location_component_spec.rb b/spec/components/search_result/location_component_spec.rb index f44eb9806..5d9b9018b 100644 --- a/spec/components/search_result/location_component_spec.rb +++ b/spec/components/search_result/location_component_spec.rb @@ -123,34 +123,17 @@ SolrDocument.new( id: '123', item_display_struct: [ - { id: '66645303-add1-5d4f-ae33-7944f5d1cae2', barcode: '36105097469808', library: 'SPEC-COLL', - effective_permanent_location_code: 'SPEC-SAMSON', callnumber: 'PJ5204 .B6 1866' }, - { id: '086a8f9d-bece-5919-afe9-fdc65f970d36', barcode: '36105023721066', library: 'SAL3', - effective_permanent_location_code: 'SAL3-STACKS', callnumber: 'PJ5204 .B6 1838 2ND IN VOL' } - ], - holdings_json_struct: [ { - holdings: [ - { - id: 'd2777d47-2150-54aa-a5f8-50c2d4042338', - boundWith: { - item: { - id: '086a8f9d-bece-5919-afe9-fdc65f970d36' - }, - holding: {}, - instance: {} - }, - location: { - effectiveLocation: build(:location, code: 'SAL3-SEE-OTHER') - } - }, - { - id: '971f9a6d-7650-5793-898a-5927b9378570', - location: { - effectiveLocation: build(:location, code: 'SPEC-SAMSON') - } - } - ] + id: '66645303-add1-5d4f-ae33-7944f5d1cae2', barcode: '36105097469808', library: 'SPEC-COLL', + effective_permanent_location_code: 'SPEC-SAMSON', callnumber: 'PJ5204 .B6 1866', + bound_with: nil + }, + { + id: '086a8f9d-bece-5919-afe9-fdc65f970d36', barcode: '36105023721066', library: 'SAL3', + effective_permanent_location_code: 'SAL3-STACKS', callnumber: 'PJ5204 .B6 1838 2ND IN VOL', + bound_with: { + hrid: 'a5488051' + } } ] ) diff --git a/spec/features/access_panels/library_location_spec.rb b/spec/features/access_panels/library_location_spec.rb index 1c1c89170..afe6b6e20 100644 --- a/spec/features/access_panels/library_location_spec.rb +++ b/spec/features/access_panels/library_location_spec.rb @@ -38,9 +38,9 @@ describe 'long lists more than 5 callnumbers', :js do it 'is truncated with a more link' do visit solr_document_path '10' - expect(page).to have_no_css('td', text: 'MNO', visible: true) + expect(page).to have_no_css('.callnumber', text: 'MNO', visible: true) click_button 'show all' - expect(page).to have_css('td', text: 'MNO', visible: true) + expect(page).to have_css('.callnumber', text: 'MNO', visible: true) end end end diff --git a/spec/lib/holdings/item_spec.rb b/spec/lib/holdings/item_spec.rb index f719a32cd..c74fc54bc 100644 --- a/spec/lib/holdings/item_spec.rb +++ b/spec/lib/holdings/item_spec.rb @@ -188,56 +188,25 @@ end context 'with a FOLIO bound-with' do - let(:document) { - SolrDocument.new( - id: '1234', - holdings_json_struct: [ - { holdings: [ + let(:document) { SolrDocument.new } + + describe '#live_lookup_instance_id' do + context 'with a bound-with item' do + subject(:item) do + described_class.new( { - id: 'holding1234', - location: { - effectiveLocation: { - id: "158168a3-ede4-4cc1-8c98-61f4feeb22ea", - code: "SAL3-SEE-OTHER", - name: "See linked record to request items bound together", - campus: { - id: "c365047a-51f2-45ce-8601-e421ca3615c5", - code: "SUL", - name: "Stanford Libraries" - }, - details: {}, - library: { - id: "ddd3bce1-9f8f-4448-8d6d-b6c1b3907ba9", - code: "SAL3", - name: "SAL3 (off-campus storage)" - }, - institution: { - id: "8d433cdd-4e8f-4dc1-aa24-8a4ddb7dc929", - code: "SU", - name: "Stanford University" - } - } - }, - holdingsType: { - id: "5b08b35d-aaa3-4806-998c-9cd85e5bc406", - name: "Bound-with" - }, - boundWith: { - item: { barcode: "1234" }, + barcode: '1234', library: 'GREEN', effective_permanent_location_code: 'GRE-STACKS', + bound_with: { instance: { id: "7e194e58-e134-56fe-a3c2-2c0494e04c5b" } } - } - ] } - ] - ) - } - - subject(:item) { described_class.new({ barcode: '1234', library: 'GREEN', effective_permanent_location_code: 'GRE-STACKS' }, document:) } - - describe '#live_lookup_instance_id' do - context 'with a bound-with item' - it 'returns the parent instance id' do - expect(item.live_lookup_instance_id).to eq "7e194e58-e134-56fe-a3c2-2c0494e04c5b" + }, + document: + ) + end + + it 'returns the parent instance id' do + expect(item.live_lookup_instance_id).to eq "7e194e58-e134-56fe-a3c2-2c0494e04c5b" + end end end end diff --git a/spec/lib/holdings/location_spec.rb b/spec/lib/holdings/location_spec.rb index 63a165705..1fba0c002 100644 --- a/spec/lib/holdings/location_spec.rb +++ b/spec/lib/holdings/location_spec.rb @@ -86,62 +86,35 @@ end describe "#bound_with?" do + let(:document) { SolrDocument.new } + context 'with items that are bound with' do - let(:document) { - SolrDocument.new( - id: '1234', - holdings_json_struct: [ - { holdings: [ - { - id: 'holding1234', - location: { - effectiveLocation: { - id: "158168a3-ede4-4cc1-8c98-61f4feeb22ea", - code: "SAL3-SEE-OTHER", - name: "See linked record to request items bound together", - campus: { - id: "c365047a-51f2-45ce-8601-e421ca3615c5", - code: "SUL", - name: "Stanford Libraries" - }, - details: {}, - library: { - id: "ddd3bce1-9f8f-4448-8d6d-b6c1b3907ba9", - code: "SAL3", - name: "SAL3 (off-campus storage)" - }, - institution: { - id: "8d433cdd-4e8f-4dc1-aa24-8a4ddb7dc929", - code: "SU", - name: "Stanford University" - } - } - }, - holdingsType: { - id: "5b08b35d-aaa3-4806-998c-9cd85e5bc406", - name: "Bound-with" - }, - boundWith: { - item: { barcode: "1234" }, - instance: { id: "7e194e58-e134-56fe-a3c2-2c0494e04c5b" } - } + subject do + described_class.new("SAL3-STACKS", [ + Holdings::Item.new( + { + barcode: '1234', library: 'SAL3', effective_permanent_location_code: 'SAL3-STACKS', + bound_with: { + hrid: 'a5488051' } - ] } - ] - ) - } - - subject { described_class.new("SAL3-STACKS", [ - Holdings::Item.new({ barcode: '1234', library: 'SAL3', effective_permanent_location_code: 'SAL3-STACKS' }, document:) - ]) } + }, + document: + ) + ]) + end it { is_expected.to be_bound_with } end context 'with items that are not bound with' do - subject { described_class.new("GRE-STACKS", [ - Holdings::Item.new({ barcode: 'barcode2', library: 'GREEN', effective_permanent_location_code: 'SEE-OTHER' }, document: SolrDocument.new) - ]) } + subject do + described_class.new("GRE-STACKS", [ + Holdings::Item.new( + { barcode: 'barcode2', library: 'GREEN', effective_permanent_location_code: 'SEE-OTHER' }, + document: + ) + ]) + end it { is_expected.not_to be_bound_with } end