Skip to content
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
2 changes: 1 addition & 1 deletion config/settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ settings:
ucpath_upload_path: "/alma/patron_employees/"
upload_host: "upload.lib.berkeley.edu"
upload_user: "ssullivan"
application_version: "1.6.16"
application_version: "1.6.18"

# TODO - flesh this out
# http://docopt.org/
Expand Down
39 changes: 30 additions & 9 deletions lib/sis/api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,23 +80,29 @@ def as_of_date
# rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity, Metrics/BlockLength
def fetch_by_term(term_id, as_of_date = nil)
raw_users = []

current_page = 0

# AP-827 : (2026-08-14)
# We've recently encountered duplicate Student_IDs (aka primary_ids) in the XML, local tests have failed
# to reproduce the error, so adding a "seen_ids" to log and skip any duplicates we encounter.
seen_ids = {}

loop do
current_page += 1

logger.info " Page: #{current_page}"
# current_page = 1000 if current_page >= 5

logger.info "Fetching page: #{current_page}"

req = create_request(term_id, current_page, as_of_date)

res = ''
response = ''
res = nil
response = nil

# We should give it 4 or 5 tries when hitting the API
# since they've sort of hosed their API in the past.
(1..5).each do |i|
logger.info " attempt: #{i}"
logger.info " attempt: #{i}" if i > 1

res = sis_fetch(req, 'json')

Expand All @@ -117,11 +123,11 @@ def fetch_by_term(term_id, as_of_date = nil)
break loop if status != '200'

# Extract the students array from the response
students = response['apiResponse']['response']['students'] || 0
students = response['apiResponse']['response']['students'] || []

errors = false
students.each do |student|
errors = false

students.each_with_index do |student, _idx|
# Bundle this student's data into a hash
s = {}

Expand All @@ -142,7 +148,22 @@ def fetch_by_term(term_id, as_of_date = nil)
end
end

raw_users.push(s) unless errors
next if errors

student_id = s['student_id']

# If we've seen this student already - log it and skip it!
if seen_ids[student_id]
logger.warn(
"DUPLICATE SIS STUDENT #{student_id}: " \
"first seen on page #{seen_ids[student_id]}, " \
"seen again on page #{current_page}; skipping duplicate"
)
next
end

seen_ids[student_id] = current_page
raw_users.push(s)
end
end

Expand Down
24 changes: 24 additions & 0 deletions spec/lib/sis_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,30 @@
expect(users.count).to eq(2)
end

it 'logs and skips duplicate student IDs' do
term_id = '2222'

stub_sis_data(term_id, 1)
stub_sis_data(term_id, 2, fixture: 'term_2222_1.json')

stub_get(
sis_data_url(term_id, 3),
status: 404
)

allow(SIS::API.logger).to receive(:warn)

users = SIS::API.fetch_by_term(term_id)

student_ids = users.map { |user| user['student_id'] }

expect(student_ids.count('10162050')).to eq(1)

expect(SIS::API.logger).to have_received(:warn).with(
/DUPLICATE SIS STUDENT 10162050: first seen on page 1, seen again on page 2; skipping duplicate/
)
end

it 'returns the correct term code for summer term' do
allow(Date).to receive(:today).and_return Date.new(2022, 6, 15)
expected_term = '2225'
Expand Down
10 changes: 8 additions & 2 deletions spec/stub_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,14 @@ def stub_past_sis_data(term_id, as_of_date, page_num)
stub_get(sis_past_data_url(term_id, as_of_date, page_num), status: 200, body: fixture_body('sis', "past_#{term_id}_#{page_num}.json"))
end

def stub_sis_data(term_id, page_num)
stub_get(sis_data_url(term_id, page_num), status: 200, body: fixture_body('sis', "term_#{term_id}_#{page_num}.json"))
def stub_sis_data(term_id, page_num, fixture: nil)
fixture ||= "term_#{term_id}_#{page_num}.json"

stub_get(
sis_data_url(term_id, page_num),
status: 200,
body: fixture_body('sis', fixture)
)
end

def stub_missing_sis_data(term_id, page_num)
Expand Down
Loading