From 3215f48b83c2c933636f4b543e28461c9cacdcde Mon Sep 17 00:00:00 2001 From: Christina Chortaria Date: Tue, 4 Mar 2025 14:44:46 -0500 Subject: [PATCH] Break subjectify into smaller methods. Helps with https://github.com/pulibrary/orangelight/pull/3386/files --- .reek.yml | 5 +- app/helpers/application_helper.rb | 82 +++++++++++++++++++++---------- spec/features/browsables_spec.rb | 31 ++++++++++++ 3 files changed, 90 insertions(+), 28 deletions(-) diff --git a/.reek.yml b/.reek.yml index f0737b6bc..272485298 100644 --- a/.reek.yml +++ b/.reek.yml @@ -247,6 +247,7 @@ detectors: - AdvancedHelper#advanced_search_fields - AdvancedHelper#param_for_field - ApplicationHelper#series_results + - ApplicationHelper#subjectify - ApplicationHelper#title_hierarchy - BlacklightHelper#isbn_resolve - BlacklightHelper#issn_resolve @@ -681,6 +682,8 @@ detectors: - Requests::FormController#submit - Users::OmniauthCallbacksController#alma - ApplicationHelper#action_notes_display + - ApplicationHelper#build_search_subject_links + - ApplicationHelper#build_subject_list - ApplicationHelper#location_has - ApplicationHelper#name_title_hierarchy - ApplicationHelper#render_location_code @@ -799,11 +802,11 @@ detectors: - AdvancedHelper#advanced_search_fields - AdvancedHelper#generate_solr_fq - ApplicationHelper#action_notes_display + - ApplicationHelper#build_search_subject_links - ApplicationHelper#html_safe - ApplicationHelper#location_has - ApplicationHelper#name_title_hierarchy - ApplicationHelper#same_series_result - - ApplicationHelper#subjectify - ApplicationHelper#title_hierarchy - Blacklight::Document::DublinCore#export_as_oai_dc_xml - Blacklight::Document::DublinCore#export_as_rdf_dc diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 0d32eb718..f67f16a61 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -86,42 +86,24 @@ def search_location_display(holding) location_display.html_safe end - SEPARATOR = '—'.freeze - QUERYSEP = '—'.freeze - # rubocop:disable Metrics/AbcSize def subjectify(args) + # all_subjects, sub_array = process_subjects(args[:document][args[:field]]) all_subjects = [] sub_array = [] - args[:document][args[:field]].each_with_index do |subject, i| + args[:document][args[:field]].each_with_index do |subject, index| spl_sub = subject.split(QUERYSEP) sub_array << [] subjectaccum = '' - spl_sub.each_with_index do |subsubject, j| - spl_sub[j] = subjectaccum + subsubject - subjectaccum = spl_sub[j] + QUERYSEP - sub_array[i] << spl_sub[j] + spl_sub.each_with_index do |subsubject, subsubject_index| + spl_sub[subsubject_index] = subjectaccum + subsubject + subjectaccum = spl_sub[subsubject_index] + QUERYSEP + sub_array[index] << spl_sub[subsubject_index] end - all_subjects[i] = subject.split(QUERYSEP) - end - subject_list = args[:document][args[:field]].each_with_index do |_subject, i| - lnk = '' - lnk_accum = '' - full_sub = '' - all_subjects[i].each_with_index do |subsubject, j| - lnk = lnk_accum + link_to(subsubject, - "/?f[subject_facet][]=#{CGI.escape StringFunctions.trim_punctuation(sub_array[i][j])}", class: 'search-subject', 'data-original-title' => "Search: #{sub_array[i][j]}") - lnk_accum = lnk + content_tag(:span, SEPARATOR, class: 'subject-level') - full_sub = sub_array[i][j] - end - lnk += ' ' - lnk += link_to('[Browse]', "/browse/subjects?q=#{CGI.escape full_sub}", class: 'browse-subject', 'data-original-title' => "Browse: #{full_sub}", 'aria-label' => "Browse: #{full_sub}", dir: full_sub.dir.to_s) unless fast_subjects_value?(args, i) - args[:document][args[:field]][i] = lnk.html_safe - end - content_tag :ul do - subject_list.each { |subject| concat(content_tag(:li, subject, dir: subject.dir)) } + all_subjects[index] = subject.split(QUERYSEP) end + subject_list = build_subject_list(args, all_subjects, sub_array) + build_subject_ul(subject_list) end - # rubocop:enable Metrics/AbcSize def title_hierarchy(args) titles = JSON.parse(args[:document][args[:field]]) @@ -314,9 +296,55 @@ def should_show_viewer? private + SEPARATOR = '—'.freeze + QUERYSEP = '—'.freeze + private_constant :SEPARATOR, :QUERYSEP + def fast_subjects_value?(args, i) fast_subject_display_field = args[:document]["fast_subject_display"] return false if fast_subject_display_field.nil? fast_subject_display_field.present? && fast_subject_display_field.include?(args[:document][args[:field]][i]) end + + def build_subject_ul(subject_list) + content_tag :ul do + subject_list.each { |subject| concat(content_tag(:li, subject, dir: subject.dir)) } + end + end + + def build_subject_list(args, all_subjects, sub_array) + args_document_field = args[:document][args[:field]] + args_document_field.each_with_index do |_subject, index| + sub_array_index = sub_array[index] + lnk = build_search_subject_links(all_subjects[index], sub_array_index) + lnk += build_browse_subject_link(args, index, sub_array_index.last) + args_document_field[index] = lnk.html_safe + end + end + + def build_search_subject_links(subjects, sub_array) + lnk = '' + lnk_accum = '' + + subjects.each_with_index do |subsubject, j| + sub_array_j = sub_array[j] + lnk = lnk_accum + link_to(subsubject, + "/?f[subject_facet][]=#{CGI.escape StringFunctions.trim_punctuation(sub_array_j)}", + class: 'search-subject', + 'data-original-title' => "Search: #{sub_array_j}") + lnk_accum = lnk + content_tag(:span, SEPARATOR, class: 'subject-level') + end + lnk + end + + def build_browse_subject_link(args, index, full_sub) + return ' ' if fast_subjects_value?(args, index) + + ' ' + link_to('[Browse]', + "/browse/subjects?q=#{CGI.escape full_sub}", + class: 'browse-subject', + 'data-original-title' => "Browse: #{full_sub}", + 'aria-label' => "Browse: #{full_sub}", + dir: full_sub.dir.to_s) + end end diff --git a/spec/features/browsables_spec.rb b/spec/features/browsables_spec.rb index 97181d0d1..3e693210c 100644 --- a/spec/features/browsables_spec.rb +++ b/spec/features/browsables_spec.rb @@ -50,4 +50,35 @@ expect(page).to have_link('[Browse]', href: "/browse/name_titles?q=#{CGI.escape browse_title_part}") end end + + describe 'Browse subject links' do + before do + stub_holding_locations + visit '/catalog/9982377783506421' + end + it "browses a subject heading link" do + subject_heading_lc1 = "Piano music" + expect(page).to have_link('[Browse]', href: "/browse/subjects?q=#{CGI.escape subject_heading_lc1}") + end + it "browses a subject heading with subdivision link" do + subject_heading_lc_with_subdivision = "Concertos (Piano)—Cadenzas" + expect(page).to have_link('[Browse]', href: "/browse/subjects?q=#{CGI.escape subject_heading_lc_with_subdivision}") + end + end + + describe 'Search subject links' do + before do + stub_holding_locations + visit '/catalog/9961398363506421' + end + it "searches a subject heading" do + subject_heading_lc1 = "Menz family—Art patronage—Exhibitions" + expect(page).to have_link('Exhibitions', href: "/?f[subject_facet][]=#{CGI.escape subject_heading_lc1}") + end + it "searches a subject heading with subdivision" do + subject_heading_lc_subdivision = "Art patronage" + subject_heading_lc_with_subdivision = "Menz family—Art patronage" + expect(page).to have_link(subject_heading_lc_subdivision, href: "/?f[subject_facet][]=#{CGI.escape subject_heading_lc_with_subdivision}") + end + end end