From d4216a1268db7d2514d4525fd98c53b081c657cd Mon Sep 17 00:00:00 2001 From: Mason Ballengee Date: Thu, 7 Nov 2024 16:54:55 -0500 Subject: [PATCH 1/5] Populate section title with filename when there is one section We have completely reworked how media object, master file, and playlist item titles are generated/displayed so limiting the creation of a display title from filename to only when there are multiple sections is no longer necessary. --- app/models/concerns/master_file_behavior.rb | 2 +- app/presenters/speedy_af/proxy/master_file.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/concerns/master_file_behavior.rb b/app/models/concerns/master_file_behavior.rb index d878514d6c..80fed4a8ee 100644 --- a/app/models/concerns/master_file_behavior.rb +++ b/app/models/concerns/master_file_behavior.rb @@ -108,7 +108,7 @@ def display_title structuralMetadata.section_title elsif title.present? title - elsif file_location.present? && (media_object.section_ids.size > 1) + elsif file_location.present? file_location.split("/").last end mf_title.blank? ? nil : mf_title diff --git a/app/presenters/speedy_af/proxy/master_file.rb b/app/presenters/speedy_af/proxy/master_file.rb index 1584b2d9e5..2cfa08650f 100644 --- a/app/presenters/speedy_af/proxy/master_file.rb +++ b/app/presenters/speedy_af/proxy/master_file.rb @@ -40,7 +40,7 @@ def display_title structuralMetadata.section_title elsif title.present? title - elsif file_location.present? && (media_object.section_ids.size > 1) + elsif file_location.present? file_location.split("/").last end mf_title.blank? ? nil : mf_title From c265dea30ccd15b6e17e5810b614ce7bd0bd12eb Mon Sep 17 00:00:00 2001 From: Mason Ballengee Date: Fri, 8 Nov 2024 09:16:42 -0500 Subject: [PATCH 2/5] Fix failing tests --- spec/jobs/media_object_indexing_job_spec.rb | 2 +- spec/models/master_file_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/jobs/media_object_indexing_job_spec.rb b/spec/jobs/media_object_indexing_job_spec.rb index cfa79b97d1..3a39592a01 100644 --- a/spec/jobs/media_object_indexing_job_spec.rb +++ b/spec/jobs/media_object_indexing_job_spec.rb @@ -27,7 +27,7 @@ expect(before_doc["all_comments_ssim"]).to be_blank job.perform(media_object.id) after_doc = ActiveFedora::SolrService.query("id:#{media_object.id}").first - expect(after_doc["all_comments_ssim"]).to eq [comment] + expect(after_doc["all_comments_ssim"]).to eq ["[#{master_file.display_title}] #{comment}"] end end end diff --git a/spec/models/master_file_spec.rb b/spec/models/master_file_spec.rb index 9c00267597..aa354d3261 100644 --- a/spec/models/master_file_spec.rb +++ b/spec/models/master_file_spec.rb @@ -508,7 +508,7 @@ end it "should have an appropriate title for the embed code with no label (only one section)" do - expect( subject.embed_title ).to eq( "test" ) + expect( subject.embed_title ).to eq( "test - video.mp4" ) end it 'should have an appropriate title for the embed code with no label (more than 1 section)' do From 31d2ebbd8d5fd4c08861033e382bc3a1ae4043fe Mon Sep 17 00:00:00 2001 From: Mason Ballengee Date: Fri, 8 Nov 2024 13:19:56 -0500 Subject: [PATCH 3/5] Revert "Populate section title with filename when there is one section" This reverts commit d4216a1268db7d2514d4525fd98c53b081c657cd. --- app/models/concerns/master_file_behavior.rb | 2 +- app/presenters/speedy_af/proxy/master_file.rb | 2 +- spec/jobs/media_object_indexing_job_spec.rb | 2 +- spec/models/master_file_spec.rb | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/models/concerns/master_file_behavior.rb b/app/models/concerns/master_file_behavior.rb index 80fed4a8ee..d878514d6c 100644 --- a/app/models/concerns/master_file_behavior.rb +++ b/app/models/concerns/master_file_behavior.rb @@ -108,7 +108,7 @@ def display_title structuralMetadata.section_title elsif title.present? title - elsif file_location.present? + elsif file_location.present? && (media_object.section_ids.size > 1) file_location.split("/").last end mf_title.blank? ? nil : mf_title diff --git a/app/presenters/speedy_af/proxy/master_file.rb b/app/presenters/speedy_af/proxy/master_file.rb index 2cfa08650f..1584b2d9e5 100644 --- a/app/presenters/speedy_af/proxy/master_file.rb +++ b/app/presenters/speedy_af/proxy/master_file.rb @@ -40,7 +40,7 @@ def display_title structuralMetadata.section_title elsif title.present? title - elsif file_location.present? + elsif file_location.present? && (media_object.section_ids.size > 1) file_location.split("/").last end mf_title.blank? ? nil : mf_title diff --git a/spec/jobs/media_object_indexing_job_spec.rb b/spec/jobs/media_object_indexing_job_spec.rb index 3a39592a01..cfa79b97d1 100644 --- a/spec/jobs/media_object_indexing_job_spec.rb +++ b/spec/jobs/media_object_indexing_job_spec.rb @@ -27,7 +27,7 @@ expect(before_doc["all_comments_ssim"]).to be_blank job.perform(media_object.id) after_doc = ActiveFedora::SolrService.query("id:#{media_object.id}").first - expect(after_doc["all_comments_ssim"]).to eq ["[#{master_file.display_title}] #{comment}"] + expect(after_doc["all_comments_ssim"]).to eq [comment] end end end diff --git a/spec/models/master_file_spec.rb b/spec/models/master_file_spec.rb index aa354d3261..9c00267597 100644 --- a/spec/models/master_file_spec.rb +++ b/spec/models/master_file_spec.rb @@ -508,7 +508,7 @@ end it "should have an appropriate title for the embed code with no label (only one section)" do - expect( subject.embed_title ).to eq( "test - video.mp4" ) + expect( subject.embed_title ).to eq( "test" ) end it 'should have an appropriate title for the embed code with no label (more than 1 section)' do From 677cae9d140096d94c8d6f7785c61d691e7f8b14 Mon Sep 17 00:00:00 2001 From: Mason Ballengee Date: Fri, 8 Nov 2024 13:25:35 -0500 Subject: [PATCH 4/5] Use structure_title for populating structure list --- app/models/iiif_canvas_presenter.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/iiif_canvas_presenter.rb b/app/models/iiif_canvas_presenter.rb index f2c481b1f1..d0245cadf1 100644 --- a/app/models/iiif_canvas_presenter.rb +++ b/app/models/iiif_canvas_presenter.rb @@ -27,7 +27,7 @@ def initialize(master_file:, stream_info:, media_fragment: nil) delegate :derivative_ids, :id, to: :master_file def to_s - master_file.display_title + master_file.structure_title end def range From 35cd919d39f0cf5ba90610c35b8b7ad68649bbf4 Mon Sep 17 00:00:00 2001 From: Mason Ballengee Date: Mon, 11 Nov 2024 10:54:22 -0500 Subject: [PATCH 5/5] Add test of title generation --- spec/models/iiif_canvas_presenter_spec.rb | 27 +++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/spec/models/iiif_canvas_presenter_spec.rb b/spec/models/iiif_canvas_presenter_spec.rb index fb85b8c8dd..b8694deafe 100644 --- a/spec/models/iiif_canvas_presenter_spec.rb +++ b/spec/models/iiif_canvas_presenter_spec.rb @@ -47,6 +47,33 @@ end end + describe "#to_s" do + subject { presenter.to_s } + + context 'single-section item' do + it 'prioritizes MediaObject#title for section title' do + expect(subject).to eq master_file.structure_title + expect(subject).to eq media_object.title + expect(subject).to_not eq master_file.display_title + end + end + + context 'multi-section item' do + let(:second) { FactoryBot.build(:master_file, media_object: media_object, derivatives: [derivative]) } + + before :each do + media_object.sections = [master_file, second] + media_object.save! + end + + it 'prioritizes MasterFile#display_title for section titles' do + expect(subject).to eq master_file.structure_title + expect(subject).to eq master_file.display_title + expect(subject).to_not eq media_object.title + end + end + end + describe '#display_content' do subject { presenter.display_content.first }