Allow hsh_to_spdx to accept rightsUri as well as rightsURI.#205
Open
Allow hsh_to_spdx to accept rightsUri as well as rightsURI.#205
Conversation
…was already doing that around line 1309
There was a problem hiding this comment.
Pull Request Overview
This PR fixes inconsistent handling of the "rightsUri" field in the hsh_to_spdx method by accepting both "rightsUri" and "rightsURI" variations. The change addresses a failing test in Lupo by ensuring the method can handle both camelCase and PascalCase variations of the rights URI field name.
Key Changes
- Modified the license lookup logic to check for both "rightsUri" and "rightsURI" field names
| def hsh_to_spdx(hsh) | ||
| spdx = resource_json(:spdx).fetch("licenses") | ||
| license = spdx.find { |l| l["licenseId"].casecmp?(hsh["rightsIdentifier"]) || l["seeAlso"].first == normalize_cc_url(hsh["rightsURI"]) || l["name"] == hsh["rights"] || l["seeAlso"].first == normalize_cc_url(hsh["rights"]) } | ||
| license = spdx.find { |l| l["licenseId"].casecmp?(hsh["rightsIdentifier"]) || l["seeAlso"].first == normalize_cc_url(hsh["rightsUri"]) || l["seeAlso"].first == normalize_cc_url(hsh["rightsURI"]) || l["name"] == hsh["rights"] || l["seeAlso"].first == normalize_cc_url(hsh["rights"]) } |
There was a problem hiding this comment.
The expression l["seeAlso"].first is called three times in this condition chain. Consider extracting it to a variable to avoid repeated array access and potential nil errors if seeAlso is empty.
Suggested change
| license = spdx.find { |l| l["licenseId"].casecmp?(hsh["rightsIdentifier"]) || l["seeAlso"].first == normalize_cc_url(hsh["rightsUri"]) || l["seeAlso"].first == normalize_cc_url(hsh["rightsURI"]) || l["name"] == hsh["rights"] || l["seeAlso"].first == normalize_cc_url(hsh["rights"]) } | |
| license = spdx.find do |l| | |
| see_also_first = l["seeAlso"].first | |
| l["licenseId"].casecmp?(hsh["rightsIdentifier"]) || | |
| see_also_first == normalize_cc_url(hsh["rightsUri"]) || | |
| see_also_first == normalize_cc_url(hsh["rightsURI"]) || | |
| l["name"] == hsh["rights"] || | |
| see_also_first == normalize_cc_url(hsh["rights"]) | |
| end |
Contributor
|
@svogt0511 and @codycooperross This looks pretty much complete. Should we update it and get it out soon? |
Contributor
|
Not sure I remember what this is addressing, but feel free, @svogt0511 |
This file contains hidden or 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
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.
Note: hsh_to_spdx was was already implementing that behavior that around line 1309 of lib/bolognese/utils.rb.
Purpose
related to : https://github.com/orgs/datacite/projects/28/views/21?pane=issue&itemId=111924271&issue=datacite%7Cproduct-backlog%7C335
Approach
It fixes a failing test in lupo.
Open Questions and Pre-Merge TODOs
Learning
Types of changes
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Reviewer, please remember our guidelines: