Skip to content

Add full-text options to relevant Alma records - #452

Open
jazairi wants to merge 2 commits into
mainfrom
use-663
Open

Add full-text options to relevant Alma records#452
jazairi wants to merge 2 commits into
mainfrom
use-663

Conversation

@jazairi

@jazairi jazairi commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Why these changes are being introduced:

Certain catalog records include an electronic
holding that is not included in the links
object of the PNX metadata. For these records, Primo displays a 'Full-text options' section with the e-resource link, but SML displays nothing because it only checks links` for this
information.

Relevant ticket(s):

How this addresses that need:

This adds 'Full-text options' to a result's
links object, provided the result meets the
following conditions:

  • pnx['links'] is nil
  • pnx['deliveryCategory'] includes Alma-E

The 'Full-text options' link resolves to the
corresponding section of the Primo record.

Side effects of this change:

This decision infers that all Primo records that
meet the two conditions above will have a
'Full-text options' section. If this assumption
is incorrect, it will result in a confusing user
experience.

Developer

Accessibility
  • ANDI or WAVE has been run in accordance to our guide.
  • This PR contains no changes to the view layer.
  • New issues flagged by ANDI or WAVE have been resolved.
  • New issues flagged by ANDI or WAVE have been ticketed (link in the Pull Request details above).
  • No new accessibility issues have been flagged.
New ENV
  • All new ENV is documented in README.
  • All new ENV has been added to Heroku Pipeline, Staging and Prod.
  • ENV has not changed.
Approval beyond code review
  • UXWS/stakeholder approval has been confirmed.
  • UXWS/stakeholder review will be completed retroactively.
  • UXWS/stakeholder review is not needed.
Additional context needed to review

Searching for "Sea otters: a history" in the PR build will return as the top result a record that should include the 'Full-text options' link.

Code Reviewer

Code
  • I have confirmed that the code works as intended.
  • Any CodeClimate issues have been fixed or confirmed as
    added technical debt.
Documentation
  • The commit message is clear and follows our guidelines
    (not just this pull request message).
  • The documentation has been updated or is unnecessary.
  • New dependencies are appropriate or there were no changes.
Testing
  • There are appropriate tests covering any new functionality.
  • No additional test coverage is required.

@coveralls

coveralls commented Aug 13, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 31750706282

Coverage increased (+0.003%) to 98.327%

Details

  • Coverage increased (+0.003%) from the base build.
  • Patch coverage: 5 of 5 lines across 1 file are fully covered (100%).
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 1554
Covered Lines: 1528
Line Coverage: 98.33%
Coverage Strength: 86.93 hits per line

💛 - Coveralls

@mitlib
mitlib temporarily deployed to timdex-ui-pi-use-663-mf7bsrvsh August 13, 2026 17:05 Inactive
@qltysh

qltysh Bot commented Aug 13, 2026

Copy link
Copy Markdown

❌ 5 blocking issues (5 total)

Tool Category Rule Count
rubocop Lint Class has too many lines. [257/100] 1
rubocop Lint Assignment Branch Condition size for links is too high. [<4, 44, 15> 46.66/17] 1
rubocop Lint Cyclomatic complexity for links is too high. [16/7] 1
rubocop Lint Method has too many lines. [27/10] 1
rubocop Lint Perceived complexity for links is too high. [17/8] 1

Comment thread app/models/normalize_primo_record.rb
Comment thread app/models/normalize_primo_record.rb Outdated
Comment thread app/models/normalize_primo_record.rb Outdated
Comment thread test/models/normalize_primo_record_test.rb Outdated
Comment thread test/models/normalize_primo_record_test.rb Outdated
Comment thread test/models/normalize_primo_record_test.rb Outdated
@jazairi
jazairi had a problem deploying to timdex-ui-pi-use-663-mf7bsrvsh August 13, 2026 17:42 Failure
Comment thread app/models/normalize_primo_record.rb
@jazairi
jazairi had a problem deploying to timdex-ui-pi-use-663-mf7bsrvsh August 13, 2026 17:46 Failure
Comment thread app/models/normalize_primo_record.rb
@jazairi
jazairi temporarily deployed to timdex-ui-pi-use-663-mf7bsrvsh August 13, 2026 20:47 Inactive
Comment thread app/models/normalize_primo_record.rb
Comment thread test/models/normalize_primo_record_test.rb Outdated
Why these changes are being introduced:

Certain catalog records include an electronic
holding that is not included in the `links`
`object of the PNX metadata. For these records,
Primo displays a 'Full-text options' section with
the e-resource link, but SML displays nothing
because it only checks `links` for this
information.

Relevant ticket(s):

- [USE-663](https://mitlibraries.atlassian.net/browse/USE-663)

How this addresses that need:

This adds 'Full-text options' to a result's
`links` object, provided the result meets the
following conditions:

- pnx['links'] is nil
- pnx['deliveryCategory'] includes Alma-E

The 'Full-text options' link resolves to the
corresponding section of the Primo record.

Side effects of this change:

This decision infers that all Primo records that
meet the two conditions above will have a
'Full-text options' section. If this assumption
is incorrect, it will result in a confusing user
experience.
@jazairi
jazairi temporarily deployed to timdex-ui-pi-use-663-mf7bsrvsh August 13, 2026 22:27 Inactive
Comment thread app/models/normalize_primo_record.rb
@jazairi
jazairi requested a lite review from Copilot August 13, 2026 22:29
@jazairi
jazairi marked this pull request as ready for review August 13, 2026 22:30

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds support for showing a “Full-text options” link for certain Alma Primo records that have electronic availability (Alma‑E) but don’t include the expected pnx.links metadata, aligning the UI behavior more closely with what Primo shows in its own full display.

Changes:

  • Extend NormalizePrimoRecord#links to append a “Full-text options” link (anchored to the Primo “View It” section) when pnx.links is nil and deliveryCategory includes Alma-E.
  • Add model tests covering inclusion/exclusion rules for the new link behavior.
  • Make frbrized? consistently return a boolean (false instead of nil) when FRBR facet data is missing.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
app/models/normalize_primo_record.rb Adds conditional “Full-text options” link generation and tightens frbrized? boolean behavior.
test/models/normalize_primo_record_test.rb Adds coverage for when the “Full-text options” link should and shouldn’t be included.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread app/models/normalize_primo_record.rb
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@jazairi
jazairi temporarily deployed to timdex-ui-pi-use-663-mf7bsrvsh August 13, 2026 22:37 Inactive
def frbrized?
return unless @record['pnx']['facets']
return unless @record['pnx']['facets']['frbrtype']
return false unless @record['pnx']['facets']

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes are a result of running rubocop. They are unrelated to this feature.

end

# Return links if we found any
links.any? ? links : []

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found 4 issues:

1. Assignment Branch Condition size for links is too high. [<4, 44, 15> 46.66/17] [rubocop:Metrics/AbcSize]


2. Cyclomatic complexity for links is too high. [16/7] [rubocop:Metrics/CyclomaticComplexity]


3. Method has too many lines. [27/10] [rubocop:Metrics/MethodLength]


4. Perceived complexity for links is too high. [17/8] [rubocop:Metrics/PerceivedComplexity]

@jazairi

jazairi commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

This has provisionally passed QA. The wording of the link might change pending feedback from UXWS, but nothing else should change that would affect code review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants