From 1c9a68199ded8dad2f3a75406ee9ab9010b68527 Mon Sep 17 00:00:00 2001 From: Justin Coyne Date: Thu, 1 Feb 2024 13:53:55 -0600 Subject: [PATCH] Course reserves are now driven from the Folio data --- .../course_reserves_list_component.html.erb | 2 +- .../course_reserves_list_component.rb | 4 +- app/controllers/catalog_controller.rb | 7 ++- app/controllers/course_reserves_controller.rb | 16 +----- .../course_reserve_access_point_helper.rb | 18 ------- app/models/concerns/course_reserves.rb | 24 +-------- app/models/course_reserve.rb | 33 +++++++++++++ app/models/solr_document.rb | 1 + .../folio_course_facet_item_presenter.rb | 7 +++ ...accordion_section_course_reserves.html.erb | 8 +-- .../mastheads/_course_reserve.html.erb | 15 +++--- app/views/course_reserves/index.html.erb | 8 +-- config/solr_configs/schema.xml | 3 +- config/solr_configs/solrconfig.xml | 2 +- lib/page_location.rb | 2 +- lib/tasks/searchworks.rake | 2 +- .../course_reserves_component_spec.rb | 4 +- .../course_reserves_controller_spec.rb | 3 +- .../access_panels/course_reserve_spec.rb | 6 +-- .../access_points/course_reserve_spec.rb | 10 ++-- spec/fixtures/solr_documents/1.yml | 8 +-- spec/fixtures/solr_documents/2.yml | 4 +- ...course_reserve_access_point_helper_spec.rb | 38 -------------- spec/lib/page_location_spec.rb | 49 ++++++++++--------- spec/models/concerns/course_reserves_spec.rb | 43 ---------------- ...n_section_course_reserves.html.erb_spec.rb | 31 ++++++------ .../_course_reserve.html.erb_spec.rb | 10 ---- .../course_reserves/index.html.erb_spec.rb | 4 +- 28 files changed, 134 insertions(+), 228 deletions(-) delete mode 100644 app/helpers/course_reserve_access_point_helper.rb create mode 100644 app/models/course_reserve.rb create mode 100644 app/presenters/folio_course_facet_item_presenter.rb delete mode 100644 spec/helpers/course_reserve_access_point_helper_spec.rb delete mode 100644 spec/models/concerns/course_reserves_spec.rb delete mode 100644 spec/views/catalog/mastheads/_course_reserve.html.erb_spec.rb diff --git a/app/components/access_panels/course_reserves_list_component.html.erb b/app/components/access_panels/course_reserves_list_component.html.erb index e0e7ba234..a656c19aa 100644 --- a/app/components/access_panels/course_reserves_list_component.html.erb +++ b/app/components/access_panels/course_reserves_list_component.html.erb @@ -6,7 +6,7 @@
Instructor(s)
- <%= course.instructor %> + <%= safe_join course.instructor, ', ' %>
<% end %> <% end %> diff --git a/app/components/access_panels/course_reserves_list_component.rb b/app/components/access_panels/course_reserves_list_component.rb index cfb06a93e..82c55cd63 100644 --- a/app/components/access_panels/course_reserves_list_component.rb +++ b/app/components/access_panels/course_reserves_list_component.rb @@ -6,7 +6,7 @@ def initialize(document:, **html_attrs) end def courses - @document.course_reserves&.courses || [] + @document.course_reserves end def render? @@ -14,7 +14,7 @@ def render? end def link_to_course_reserve_course(course) - link_to("#{course.id} -- #{course.name}", search_catalog_path({ f: { course: [course.id], instructor: [course.instructor] } })) + link_to("#{course.course_number} -- #{course.name}", search_catalog_path({ f: { courses_folio_id_ssim: [course.id] } })) end end end diff --git a/app/controllers/catalog_controller.rb b/app/controllers/catalog_controller.rb index f1bdc1dfd..a72efb18e 100644 --- a/app/controllers/catalog_controller.rb +++ b/app/controllers/catalog_controller.rb @@ -178,10 +178,15 @@ class CatalogController < ApplicationController } config.add_facet_field "topic_facet", label: "Topic", limit: 20, component: Blacklight::FacetFieldListComponent config.add_facet_field "genre_ssim", label: "Genre", limit: 20, component: Blacklight::FacetFieldListComponent + # TODO: remove "course" and "instructor" after a time. These have been replaced by courses_folio_id_ssim. + # We just don't want to break any bookmarks that people may have. Perhaps wait for a semester or so. config.add_facet_field "course", label: "Course", show: false, component: Blacklight::FacetFieldListComponent config.add_facet_field "instructor", label: "Instructor", show: false, component: Blacklight::FacetFieldListComponent + config.add_facet_field "courses_folio_id_ssim", label: "Course", show: false, + component: Blacklight::FacetFieldListComponent, + item_presenter: FolioCourseFacetItemPresenter - # Should be shown under the "more..." section see https://github.com/sul-dlss/SearchWorks/issues/257 + # Should be shown under the "more..." section see https://github.com/sul-dlss/SearchWorks/issues/257 config.add_facet_field "geographic_facet", label: "Region", limit: 20, component: Blacklight::FacetFieldListComponent config.add_facet_field "era_facet", label: "Era", limit: 20, component: Blacklight::FacetFieldListComponent config.add_facet_field "author_other_facet", label: "Organization (as author)", limit: 20, component: Blacklight::FacetFieldListComponent diff --git a/app/controllers/course_reserves_controller.rb b/app/controllers/course_reserves_controller.rb index b64f2e4fb..5801062b6 100644 --- a/app/controllers/course_reserves_controller.rb +++ b/app/controllers/course_reserves_controller.rb @@ -1,19 +1,5 @@ class CourseReservesController < ApplicationController - include Blacklight::SearchContext - include Blacklight::Configurable - copy_blacklight_config_from(CatalogController) - def index - facet = "crez_course_info" - p = {} - p[:"facet.field"] = facet - p[:"f.#{facet}.facet.limit"] = "-1" # this implies lexical sort - p[:rows] = 0 - response = blacklight_config.repository.search(p) - course_reserves = [] - response.aggregations.values.first.items.each do |item| - course_reserves << CourseReserves.from_crez_info(item.value) - end - @course_reserves = course_reserves + @course_reserves = CourseReserve.all end end diff --git a/app/helpers/course_reserve_access_point_helper.rb b/app/helpers/course_reserve_access_point_helper.rb deleted file mode 100644 index ab59f7097..000000000 --- a/app/helpers/course_reserve_access_point_helper.rb +++ /dev/null @@ -1,18 +0,0 @@ -module CourseReserveAccessPointHelper - def create_course - if @response.docs.present? - # Find the document in the response that contains the requested course - # and return it's course reserve info that matches facet params. - @course_info = @response.docs.map do |document| - document[:crez_course_info].map do |course| - CourseReserves.from_crez_info(course) - end.find do |course| - course.id == params[:f][:course][0] && course.instructor == params[:f][:instructor][0] - end - end.compact.first - else - # If no docs match the search params, return some course info though it will be missing course name - @course_info = CourseReserves::CourseInfo.new(id: params[:f][:course][0], name: '', instructor: params[:f][:instructor][0]) - end - end -end diff --git a/app/models/concerns/course_reserves.rb b/app/models/concerns/course_reserves.rb index 33e5fb3b4..9aa9e5c1c 100644 --- a/app/models/concerns/course_reserves.rb +++ b/app/models/concerns/course_reserves.rb @@ -1,27 +1,5 @@ module CourseReserves def course_reserves - if self.respond_to?(:[]) - @course_reserves ||= CourseReserves::Processor.new(self) - end + @course_reserves ||= course_ids.present? ? Array(CourseReserve.find(*course_ids)).sort_by(&:course_number) : [] end - - def self.from_crez_info(course) - CourseInfo.new(*course.split('-|-').map { |c| c.strip }) - end - - private - - class Processor - def initialize(document) - @courses = Array(document[:crez_course_info]).map { |course| CourseReserves.from_crez_info(course) } - end - - def present? - true ? @courses : false - end - - attr_reader :courses - end - - CourseInfo = Data.define(:id, :name, :instructor) end diff --git a/app/models/course_reserve.rb b/app/models/course_reserve.rb new file mode 100644 index 000000000..0c58a9f39 --- /dev/null +++ b/app/models/course_reserve.rb @@ -0,0 +1,33 @@ +# The store of course reserve data from courses.json +class CourseReserve + FolioCourseInfo = Data.define(:id, :course_number, :name, :instructor) + + class Repository + include Singleton + + def all + @all ||= Folio::Types.courses.map do |id, v| + FolioCourseInfo.new(id:, course_number: v['courseNumber'], + name: v['name'], + instructor: v.dig('courseListingObject', 'instructorObjects').pluck('name')) + end + end + + def find(*ids) + if ids.length == 1 + id = ids.first + all.find { |course| course.id == id } + else + all.select { |course| ids.include?(course.id) } + end + end + end + + def self.all + Repository.instance.all + end + + def self.find(*id) + Repository.instance.find(*id) + end +end diff --git a/app/models/solr_document.rb b/app/models/solr_document.rb index 271cb016b..1f6173c05 100644 --- a/app/models/solr_document.rb +++ b/app/models/solr_document.rb @@ -113,6 +113,7 @@ def to_semantic_values # Recommendation: Use field names from Dublin Core use_extension(Blacklight::Document::DublinCore) + attribute :course_ids, :array, :courses_folio_id_ssim attribute :document_formats, :array, FORMAT_KEY attribute :live_lookup_id, :string, 'uuid_ssi' diff --git a/app/presenters/folio_course_facet_item_presenter.rb b/app/presenters/folio_course_facet_item_presenter.rb new file mode 100644 index 000000000..4710dae6b --- /dev/null +++ b/app/presenters/folio_course_facet_item_presenter.rb @@ -0,0 +1,7 @@ +# The facet value here is a UUID for a folio course. We overide to show a label that a user +# can read. +class FolioCourseFacetItemPresenter < Blacklight::FacetItemPresenter + def constraint_label + CourseReserve.find(value)&.course_number || value + end +end diff --git a/app/views/catalog/_accordion_section_course_reserves.html.erb b/app/views/catalog/_accordion_section_course_reserves.html.erb index c052b92c1..e2c3ef93b 100644 --- a/app/views/catalog/_accordion_section_course_reserves.html.erb +++ b/app/views/catalog/_accordion_section_course_reserves.html.erb @@ -3,18 +3,14 @@ id = document[:id] %> -<% if document.course_reserves&.courses.present? %> - <% - count = document.course_reserves.courses.length - snippet = document.course_reserves.courses.map(&:id).join(', ') - %> +<% if document.course_reserves.present? %>
- <%= snippet %> + <%= document.course_reserves.map(&:course_number).join(', ') %>