Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switching over to MaIS APIs and removing registry harvester XML option #483

Merged
merged 6 commits into from
May 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ coverage
# Ignoring vendorized gems
/vendor/bundle

# Ignoring CourseWork XML and JSON
/lib/course_work_xml/*.xml
# Ignoring CourseWork JSON
/lib/course_work_content/*.json
config/settings.local.yml
config/settings/*.local.yml
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/reserves_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ def all_courses
def all_courses_response
items = []
if current_user.superuser?
courses = CourseWorkCourses.instance.all_courses
courses = CourseWorkCourses.new.all_courses
else
courses = CourseWorkCourses.instance.find_by_sunet(current_user.sunetid)
courses = CourseWorkCourses.new.find_by_sunet(current_user.sunetid)
end
courses.each do |course|
cl = course.cross_listings.blank? ? "" : "(#{course.cross_listings})"
Expand Down Expand Up @@ -108,7 +108,7 @@ def clone
protected

def course_for_compound_key(cid)
CourseWorkCourses.instance.find_by_compound_key(cid).first
CourseWorkCourses.new.find_by_compound_key(cid).first
end

def reserve_mail_address(reserve)
Expand Down
2 changes: 1 addition & 1 deletion app/models/reserve.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class Reserve < ActiveRecord::Base
serialize :sent_item_list, Array

def course
@course ||= CourseWorkCourses.instance.find_by_compound_key(compound_key).first
@course ||= CourseWorkCourses.new.find_by_compound_key(compound_key).first
end

def course_title
Expand Down
4 changes: 2 additions & 2 deletions config/schedule.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
every :day, :at => '4:30am', :roles => [:app] do
rake "fetch_xml"
every :day, :at => '3:30am', :roles => [:app] do
rake "fetch_courses"
end
113 changes: 23 additions & 90 deletions lib/course_work_courses.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,34 +34,7 @@ def cross_listings
end
end

def self.instance
@instance ||= CourseWorkCourses.new
end

def initialize(file = nil)
if Settings.use_course_json
initialize_json(file)
else
initialize_xml(file)
end
end

# These two initialize methods are broken out to enable both approaches
# until we switch over completely to JSON
def initialize_xml(xml)
if xml
@raw_xml = [Nokogiri::XML(xml)]
else
@raw_xml = load_xml_from_coursework
end
end

def raw_xml
@raw_xml ||= self.raw_xml
end

# Handling JSON
def initialize_json(json_file)
def initialize(json_file = nil)
if json_file
@json_files = [JSON.parse(json_file)]
else
Expand Down Expand Up @@ -107,17 +80,30 @@ def find_by_class_id_and_section_and_sunet(class_id, section, sunet)
end
end

# TODO: We have tests that are highly sensitive to the order of the courses; this unfortunate
# logic preserves the bottom-most course from the xml. it's unclear whether this is incidental
# or a feature.
# Breaking out into two approaches. JSON is picked only if the Setting is present
# We have tests that are highly sensitive to the order of the courses.
# We use the dedup_courses method to preserve the original order while removing duplicates
def all_courses
# if Settings.use_course_json == true
if Settings.use_course_json
@all_courses ||= process_all_courses(self.json_files).to_a.reverse.uniq(&:key).reverse.to_a
else
@all_courses ||= process_all_courses_xml(self.raw_xml).to_a.reverse.uniq(&:key).reverse.to_a
@all_courses ||= dedup_courses(process_all_courses(self.json_files).to_a)
end

# Given two sections with the same CIDs and instructor sunet ids, preserve only the lower numbered section
# Keep the original order of courses
def dedup_courses(courses)
course_hash = {}
key_order = []
courses.each do |course|
ckey = course.key
sid = course.sid.to_i
key_order << ckey unless course_hash.key?(ckey)
# If we have not encountered this course and instructor set before
# or if the section id that is saved is greater than the one being processed
# save this section as the one to save for this course and instructor set
if !course_hash.key?(ckey) ||
(course_hash.key?(ckey) && course_hash[ckey].sid.to_i > sid)
course_hash[ckey] = course
end
end
key_order.map { |k| course_hash[k] }
end

# Efficient lookup of course data by compound key (which is used in a few places around the app)
Expand All @@ -133,59 +119,6 @@ def course_map

private

def load_xml_from_coursework
if Rails.env.test?
return [Nokogiri::XML(File.open("#{Rails.root}/spec/fixtures/course_work.xml", 'r'))]
else
current = Terms.process_term_for_cw(Terms.current_term)
next_term = Terms.process_term_for_cw(Terms.future_terms.first)
xml = []
["#{Rails.root}/lib/course_work_xml/courseXML_#{current}.xml", "#{Rails.root}/lib/course_work_xml/courseXML_#{next_term}.xml"].each do |url|
xml << Nokogiri::XML(File.open(url, 'r')) if File.exist?(url)
end
return xml
end
end

def process_all_courses_xml(xml_files)
return to_enum(:process_all_courses_xml, xml_files) unless block_given?

xml_files.each do |xml|
xml.xpath("//courseclass").each_with_index do |course, idx_course|
course_title = course[:title]
term = course[:term]
cids = []
course.xpath("./class").each do |cl|
cids << cl[:id].gsub(/^\w{1,2}\d{2}-/, "")
end
course.xpath("./class").each_with_index do |cl, idx_cl|
class_id = cl[:id].gsub(/^\w{1,2}\d{2}-/, "")
cl.xpath("./section").each_with_index do |sec, idx_sec|
section_id = sec[:id]
instructors = []
sec.xpath("./instructors/instructor").each do |inst|
sunet = inst[:sunetid]
name = inst.text
name = sunet if inst.text.blank?
instructors << { sunet: sunet, name: name }
end

if instructors.present?
yield Course.new(
title: course_title,
term: term,
cid: class_id,
cids: cids,
sid: section_id,
instructors: instructors
)
end
end
end
end
end
end

# Adding JSON processing
# Will replace load_xml_from_coursework. Now loads JSON files generated from MAIS course term and course API requests
def load_from_coursework
Expand Down
4 changes: 0 additions & 4 deletions lib/course_work_xml/.gitignore

This file was deleted.

25 changes: 0 additions & 25 deletions lib/tasks/fetch_xml.rake

This file was deleted.

71 changes: 0 additions & 71 deletions spec/fixtures/course_work.xml

This file was deleted.

4 changes: 0 additions & 4 deletions spec/lib/course_work_courses_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,6 @@
require "course_work_courses"
# This has been updated to focus on JSON loading and not XML
RSpec.describe CourseWorkCourses do
before do
Settings.use_course_json = true
end

let(:courses) { CourseWorkCourses.new }

describe "loading JSON from fixture file" do
Expand Down