Skip to content

Comments

Fix formatting of DSpace item repository ids#1313

Merged
markpatton merged 2 commits intomainfrom
1190-dspace-url
Jul 9, 2025
Merged

Fix formatting of DSpace item repository ids#1313
markpatton merged 2 commits intomainfrom
1190-dspace-url

Conversation

@markpatton
Copy link
Contributor

Handles the case of item URLs (produced by REST API deposit) and handle URLs (produced by old SWORD deposit)

@markpatton markpatton requested review from Copilot and rpoet-jh July 8, 2025 12:47
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors how repository IDs are formatted by extracting the substring after known URL markers and consolidating the logic into a single helper.

  • Removes the hardcoded jscholarshipCheckString and inlined slicing logic
  • Adds a formatId method to centralize ID extraction for /handle/ and /items/ URLs
  • Updates the external ID mapping to use formatId
Comments suppressed due to low confidence (2)

app/components/submissions-repoid-cell/index.js:56

  • Consider adding JSDoc annotations (@param {string} id and @returns {string}) to formatId so callers and IDEs can better understand its expected input and output.
  formatId(id) {

app/components/submissions-repoid-cell/index.js:56

  • The new formatId method logic isn't covered by existing tests. Adding unit tests for URLs with /handle/, /items/, and strings without markers would ensure correct behavior.
  formatId(id) {

Copy link
Contributor

@rpoet-jh rpoet-jh left a comment

Choose a reason for hiding this comment

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

Looks good. Are there any tests that cover this code?

@markpatton
Copy link
Contributor Author

@rpoet-jh
The copilot review made the same comment! Will add.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 9, 2025

@markpatton markpatton requested a review from rpoet-jh July 9, 2025 14:56
@markpatton markpatton merged commit d551238 into main Jul 9, 2025
5 checks passed
@markpatton markpatton deleted the 1190-dspace-url branch July 9, 2025 18:01
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.

2 participants