Rename "Library ID" to "University ID" #1033
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mjgiarlo
force-pushed
the
university-id-SearchWorks#3773
branch
3 times, most recently
from
feb2bd5 to
373c73a
Compare
mjgiarlo
force-pushed
the
university-id-SearchWorks#3773
branch
2 times, most recently
from
989239a to
57aefe3
Compare
jcoyne
requested changes
Feb 28, 2024
mjgiarlo
changed the title
mjgiarlo
force-pushed
the
university-id-SearchWorks#3773
branch
from
57aefe3 to
1727350
Compare
mjgiarlo
force-pushed
the
university-id-SearchWorks#3773
branch
from
1727350 to
e129ae4
Compare
jcoyne
approved these changes
Feb 29, 2024
cbeer
changed the title
Comment on lines
-179
to
-184
| # Look up a patron by barcode and return a Patron object | ||
| # If 'patron_info' is false, don't run the full patron info GraphQL query | ||
| def find_patron_by_barcode(barcode, patron_info: true) | ||
| response = get_json('/users', params: { query: CqlQuery.new(barcode:).to_query }) | ||
| user = response.dig('users', 0) | ||
| raise ActiveRecord::RecordNotFound, "User with barcode #{barcode} not found" unless user | ||
|
|
||
| patron_info ? Folio::Patron.find(user['id']) : Folio::Patron.new({ 'user' => user }) | ||
| end | ||
|
|
There was a problem hiding this comment.
Could we keep this, make find_patron_by_university_id do what it says, and then have a new method that tries the other two methods in sequence? That's clearer to me than having find_patron_by_university_id the way it is in this PR
There was a problem hiding this comment.
(I can make this change if others agree)
There was a problem hiding this comment.
That's fine with me too.. I was trying to leave the code in a "just take these few lines away later" state, but since it sounds like later may be... much later... clarifying it is good too 🤷♂️
thatbudakguy
reviewed
Apr 1, 2024
Comment on lines
-56
to
-57
| def user_info(user_id) | ||
| get_json("/users/#{CGI.escape(user_id)}") |
There was a problem hiding this comment.
as far as I could tell, this was unused.
thatbudakguy
reviewed
Apr 1, 2024
Comment on lines
+248
to
+267
| # Find a user by barcode in FOLIO; raise an error if not found | ||
| def find_user_by_barcode(barcode) | ||
| user = get_json('/users', params: { query: CqlQuery.new(barcode:).to_query }).dig('users', 0) | ||
| raise ActiveRecord::RecordNotFound, "User with barcode '#{barcode}' not found" unless user | ||
|
|
||
| user | ||
| end | ||
|
|
||
| # Find a user by university ID (externalSystemId in FOLIO); raise an error if not found | ||
| def find_user_by_university_id(university_id) | ||
| user = get_json('/users', params: { query: CqlQuery.new(externalSystemId: university_id).to_query }).dig('users', 0) | ||
| raise ActiveRecord::RecordNotFound, "User with externalSystemId '#{university_id}' not found" unless user | ||
|
|
||
| user | ||
| end | ||
|
|
||
| # Find a user by sunetid (username in FOLIO); raise an error if not found | ||
| def find_user_by_sunetid(sunetid) | ||
| user = get_json('/users', params: { query: CqlQuery.new(username: sunetid).to_query }).dig('users', 0) | ||
| raise ActiveRecord::RecordNotFound, "User with username '#{sunetid}' not found" unless user | ||
|
|
||
| user | ||
| end | ||
|
|
There was a problem hiding this comment.
these were repeated across the login_by_* and find_patron_by_* methods, so I extracted them.
Connects to sul-dlss/SearchWorks#3773 This commit helps us move the access portfolio beyond using library barcode numbers, leaning instead on the university-assigned and -managed "university ID."
…e login/reset forms
thatbudakguy
force-pushed
the
university-id-SearchWorks#3773
branch
from
92e559f to
96946ff
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Connects to sul-dlss/SearchWorks#3773
This commit helps us move the access portfolio beyond using library barcode numbers, leaning instead on the university-assigned and -managed "university ID."