-
Notifications
You must be signed in to change notification settings - Fork 0
Add support for Primo NDE links #367
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| # Builds Primo links with support for discovery and NDE UIs. | ||
| # | ||
| # @example Building a search link | ||
| # builder = PrimoLinkBuilder.new(query_term: "machine learning") | ||
| # builder.search_link | ||
| # # => "https://mit.primo.exlibrisgroup.com/discovery/search?query=any%2Ccontains%2Cmachine+learning&..." | ||
| # | ||
| # @example Building a full record link | ||
| # builder = PrimoLinkBuilder.new(record_id: "alma123", context: "L") | ||
| # builder.full_record_link | ||
| # # => "https://mit.primo.exlibrisgroup.com/discovery/fulldisplay?docid=alma123&..." | ||
| class PrimoLinkBuilder | ||
| # @param query_term [String, nil] The search query term (used for search_link) | ||
| # @param record_id [String, nil] The Primo record ID (used for full_record_link) | ||
| # @param context [String, nil] The Primo context code indicating record type: | ||
| # - 'L' for local catalog (Alma) records | ||
| # - 'PC' for CDI records | ||
| # - 'ALL' for all scopes | ||
| # See https://developers.exlibrisgroup.com/primo/apis/deep-links-new-ui/ and | ||
| # https://knowledge.exlibrisgroup.com/Primo/Product_Documentation/Primo/Back_Office_Guide/070Monitoring_and_Maintaining_Primo/Displaying_PNX_Records_from_Primo_Front_End | ||
| def initialize(query_term: nil, record_id: nil, context: nil) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be helpful to explain these input variables. While query_term and record_id are easily understandable, I'm genuinely not sure what context is.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Context is used for the |
||
| @query_term = query_term | ||
| @record_id = record_id | ||
| @context = context | ||
| end | ||
|
|
||
| # Build a Primo search results link | ||
| # @param tab [String] Determines which results tab to display. (default: PRIMO_TAB env var, or 'all'). | ||
| # For detailed documentation, see primo_search model. | ||
| # @param search_scope [String] Determines which Primo scope to search. (default: PRIMO_SCOPE env var, or 'all'). | ||
| # For detailed documentation, see primo_search model. | ||
| # @return [String] The complete search URL | ||
| def search_link(tab: ENV.fetch('PRIMO_TAB', 'all'), search_scope: ENV.fetch('PRIMO_SCOPE', 'all')) | ||
| return nil unless @query_term | ||
|
|
||
| base_url = "#{ENV.fetch('MIT_PRIMO_URL')}#{search_path}?" | ||
| params = { | ||
| query: search_query, | ||
| tab: tab, | ||
| search_scope: search_scope, | ||
| vid: vid | ||
| } | ||
| base_url + URI.encode_www_form(params) | ||
| end | ||
|
|
||
| # Build a Primo full record link | ||
| # @param tab [String] Determines which results tab to display. (default: PRIMO_TAB env var, or 'all'). | ||
| # For detailed documentation, see primo_search model. | ||
| # @param search_scope [String] Determines which Primo scope to search. (default: PRIMO_SCOPE env var, or 'all'). | ||
| # For detailed documentation, see primo_search model. | ||
| # @param lang [String] The language (default: 'en') | ||
| # @return [String] The complete full record URL, or nil if record_id or context is missing | ||
| def full_record_link(tab: ENV.fetch('PRIMO_TAB', 'all'), search_scope: ENV.fetch('PRIMO_SCOPE', 'all'), lang: 'en') | ||
| return nil unless @record_id && @context | ||
|
|
||
| base_url = "#{ENV.fetch('MIT_PRIMO_URL')}#{full_record_path}?" | ||
| params = { | ||
| docid: @record_id, | ||
| vid: vid, | ||
| context: @context, | ||
| search_scope: search_scope, | ||
| lang: lang, | ||
| tab: tab | ||
| } | ||
| base_url + URI.encode_www_form(params) | ||
qltysh[bot] marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| end | ||
|
|
||
| private | ||
|
|
||
| def search_path | ||
| Feature.enabled?(:primo_nde_links) ? '/nde/search' : '/discovery/search' | ||
| end | ||
|
|
||
| def full_record_path | ||
| Feature.enabled?(:primo_nde_links) ? '/nde/fulldisplay' : '/discovery/fulldisplay' | ||
| end | ||
|
|
||
| def vid | ||
| Feature.enabled?(:primo_nde_links) ? ENV.fetch('PRIMO_NDE_VID') : ENV.fetch('PRIMO_VID') | ||
| end | ||
|
|
||
| def search_query | ||
| if Feature.enabled?(:primo_nde_links) | ||
| @query_term | ||
| else | ||
| "any,contains,#{@query_term}" | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| # Test NDE link generation | ||
| require 'test_helper' | ||
|
|
||
| class PrimoLinkBuilderTest < ActiveSupport::TestCase | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suspect the intent of testing the way we are here is to avoid fragile tests, but it makes me nervous not to have any full link testing. I think most of these are fine as is, but maybe one more test for each type of link "search_link|full_record_link generates a full link as expected" or something? |
||
| test 'search_link generates Primo discovery URL by default' do | ||
| link = PrimoLinkBuilder.new(query_term: 'machine learning').search_link | ||
|
|
||
| assert_includes link, '/discovery/search?' | ||
| assert_includes link, 'query=any%2Ccontains%2Cmachine+learning' | ||
| assert_includes link, 'vid=01MIT_INST%3AMIT' | ||
| end | ||
|
|
||
| test 'search_link generates Primo NDE URL when feature flag us enabled' do | ||
| ClimateControl.modify(FEATURE_PRIMO_NDE_LINKS: 'true') do | ||
| link = PrimoLinkBuilder.new(query_term: 'machine learning').search_link | ||
|
|
||
| assert_includes link, '/nde/search?' | ||
| assert_includes link, 'query=machine+learning' | ||
| assert_includes link, 'vid=01MIT_INST%3ANDE' | ||
| assert_not_includes link, 'any%2Ccontains' | ||
| end | ||
| end | ||
|
|
||
| test 'full_record_link generates discovery URL by default' do | ||
| link = PrimoLinkBuilder.new(record_id: 'alma990003098710106761', context: 'L').full_record_link | ||
|
|
||
| assert_includes link, '/discovery/fulldisplay?' | ||
| assert_includes link, 'docid=alma990003098710106761' | ||
| assert_includes link, 'vid=01MIT_INST%3AMIT' | ||
| assert_includes link, 'context=L' | ||
| end | ||
|
|
||
| test 'full_record_link generates NDE URL when feature flag enabled' do | ||
| ClimateControl.modify(FEATURE_PRIMO_NDE_LINKS: 'true') do | ||
| link = PrimoLinkBuilder.new(record_id: 'alma990003098710106761', context: 'L').full_record_link | ||
|
|
||
| assert_includes link, '/nde/fulldisplay?' | ||
| assert_includes link, 'docid=alma990003098710106761' | ||
| assert_includes link, 'vid=01MIT_INST%3ANDE' | ||
| assert_includes link, 'context=L' | ||
| end | ||
| end | ||
|
|
||
| test 'search_link returns nil when query_term is nil' do | ||
| link = PrimoLinkBuilder.new(query_term: nil).search_link | ||
|
|
||
| assert_nil link | ||
| end | ||
|
|
||
| test 'full_record_link returns nil when record_id is missing' do | ||
| link = PrimoLinkBuilder.new(context: 'foo').full_record_link | ||
|
|
||
| assert_nil link | ||
| end | ||
|
|
||
| test 'full_record_link returns nil when context is missing' do | ||
| link = PrimoLinkBuilder.new(record_id: 'alma123').full_record_link | ||
|
|
||
| assert_nil link | ||
| end | ||
|
|
||
| test 'search_link generates complete URL for discovery' do | ||
| builder = PrimoLinkBuilder.new(query_term: 'database security') | ||
| link = builder.search_link | ||
|
|
||
| expected = 'https://mit.primo.exlibrisgroup.com/discovery/search?query=any%2Ccontains%2Cdatabase+security&tab=all&search_scope=cdi&vid=01MIT_INST%3AMIT' | ||
| assert_equal expected, link | ||
| end | ||
|
|
||
| test 'search_link generates complete URL for NDE' do | ||
| ClimateControl.modify(FEATURE_PRIMO_NDE_LINKS: 'true') do | ||
| builder = PrimoLinkBuilder.new(query_term: 'machine learning') | ||
| link = builder.search_link | ||
|
|
||
| expected = 'https://mit.primo.exlibrisgroup.com/nde/search?query=machine+learning&tab=all&search_scope=cdi&vid=01MIT_INST%3ANDE' | ||
| assert_equal expected, link | ||
| end | ||
| end | ||
|
|
||
| test 'full_record_link generates complete URL for discovery' do | ||
| builder = PrimoLinkBuilder.new(record_id: 'alma990003098710106761', context: 'L') | ||
| link = builder.full_record_link | ||
|
|
||
| assert link.start_with?('https://mit.primo.exlibrisgroup.com/discovery/fulldisplay?') | ||
| assert_includes link, 'docid=alma990003098710106761' | ||
| assert_includes link, 'context=L' | ||
| assert_includes link, 'vid=01MIT_INST%3AMIT' | ||
| assert_includes link, 'search_scope=cdi' | ||
| assert_includes link, 'lang=en' | ||
| end | ||
|
|
||
| test 'full_record_link generates complete URL for NDE' do | ||
| ClimateControl.modify(FEATURE_PRIMO_NDE_LINKS: 'true') do | ||
| builder = PrimoLinkBuilder.new(record_id: 'alma990003098710106761', context: 'P') | ||
| link = builder.full_record_link | ||
|
|
||
| assert link.start_with?('https://mit.primo.exlibrisgroup.com/nde/fulldisplay?') | ||
| assert_includes link, 'docid=alma990003098710106761' | ||
| assert_includes link, 'context=P' | ||
| assert_includes link, 'vid=01MIT_INST%3ANDE' | ||
| assert_includes link, 'search_scope=cdi' | ||
| assert_includes link, 'lang=en' | ||
| end | ||
| end | ||
| end | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice use of the builder pattern. It does a great job consolidating this logic and making it more testable.