Issue #1292 link/unlink items with version relationship#1304
Issue #1292 link/unlink items with version relationship#1304
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements a new process script item-version-linker that allows administrators to manually link and unlink items in versioning relationships. The script provides two main actions: linking two items where the second becomes a new version of the first, and unlinking the last item from its versioning history.
Changes:
- Adds new
ItemVersionLinkerscript and configuration classes for managing item version relationships - Introduces
deleteVersion()method toVersioningServiceto support unlinking operations - Fixes a bug in
VersionHistoryServiceImpl.delete()that was deleting a new empty object instead of the passed parameter - Updates test configuration to include the new script and adjusts pagination in script repository tests
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| dspace/config/spring/rest/scripts.xml | Registers the new item-version-linker script configuration bean |
| dspace-api/src/test/data/dspaceFolder/config/spring/api/scripts.xml | Registers the script configuration for integration tests |
| dspace-server-webapp/src/test/java/org/dspace/app/rest/ScriptRestRepositoryIT.java | Adds pagination parameter to accommodate the additional script in test assertions |
| dspace-api/src/main/java/org/dspace/versioning/service/VersioningService.java | Adds new deleteVersion() method interface for removing versions without deleting items |
| dspace-api/src/main/java/org/dspace/versioning/VersioningServiceImpl.java | Implements the new deleteVersion() method |
| dspace-api/src/main/java/org/dspace/versioning/VersionHistoryServiceImpl.java | Fixes bug where wrong object was being deleted (was deleting new empty object instead of parameter) |
| dspace-api/src/main/java/org/dspace/administer/ItemVersionLinkerConfiguration.java | Defines configuration and CLI options for the item-version-linker script |
| dspace-api/src/main/java/org/dspace/administer/ItemVersionLinker.java | Implements the main script logic for linking/unlinking items in version relationships |
| dspace-api/src/test/java/org/dspace/administer/ItemVersionLinkerIT.java | Comprehensive integration tests covering link/unlink operations and error scenarios |
dspace-api/src/main/java/org/dspace/administer/ItemVersionLinker.java
Outdated
Show resolved
Hide resolved
dspace-api/src/main/java/org/dspace/administer/ItemVersionLinker.java
Outdated
Show resolved
Hide resolved
dspace-api/src/main/java/org/dspace/administer/ItemVersionLinker.java
Outdated
Show resolved
Hide resolved
dspace-api/src/main/java/org/dspace/administer/ItemVersionLinker.java
Outdated
Show resolved
Hide resolved
dspace-api/src/main/java/org/dspace/administer/ItemVersionLinker.java
Outdated
Show resolved
Hide resolved
dspace-api/src/main/java/org/dspace/administer/ItemVersionLinkerConfiguration.java
Outdated
Show resolved
Hide resolved
…set dc.relation.replaces and dc.relation.isreplacedby
kosarko
left a comment
There was a problem hiding this comment.
overall looks good, just a few questions in the code and one regarding the issue #1292 itself. Where it says:
11234/1-5537 & 11234/1-6033 should be connected. Are there others? How do we find out?
Is there some mechanism available to search for these (I guess these here means items that have dc.relation.replaces and the thing this points to is in our repository but doesn't have dc.relation.isreplacedby) - if there's nothing (no process, curation, cli or a reasonable solr query) this is a separate task.
| "and its version is not the latest version in that history.", item1.getID()), getErrorMessage()); | ||
| testDSpaceRunnableHandler.getErrorMessages().clear(); | ||
|
|
||
| // linking item3 with item2 should fail since item2 is already part of other versioning history |
There was a problem hiding this comment.
Based on the comment, I don't see why this should fail. Is it about the order of the parameters? (item2, item3, admin) would succeed, right?
There was a problem hiding this comment.
Right, the following version link item3(left) -> item2(right) cannot be establish as item2 is already connected to item1.
The (right) item to be linked cannot be part of any existing version history.
There was a problem hiding this comment.
can you improve the comment a bit then?
There was a problem hiding this comment.
Comment was changed (improved).
dspace-api/src/test/java/org/dspace/administer/ItemVersionLinkerIT.java
Outdated
Show resolved
Hide resolved
dspace-api/src/test/java/org/dspace/administer/ItemVersionLinkerIT.java
Outdated
Show resolved
Hide resolved
dspace-api/src/main/java/org/dspace/administer/ItemVersionLinker.java
Outdated
Show resolved
Hide resolved
|
With regards to checking items with "dc.relation.replaces" and "dc.relation.isreplacedby". Created new task: #1310 for this. |
tested with the production db dump on the items mentions in the issue (11234/1-5537). It was returning: ``` The script has started Item '11234/1-5537' has no handle assigned. ``` because it's dc.identifier.uri.*
|
Backport branch created but failed to create PR. Please create the PR manually: Or via GitHub CLI: gh pr create --repo dataquest-dev/dspace --base dtq-dev --head ufal:backport-1304-to-dtq-dev --title "[Port dtq-dev] Issue #1292 link/unlink items with version relationship" --body "Port of #1304 by @kuchtiak-ufal to `dtq-dev`."(see action log for full error response) |
* Issue 1292: script to allow link two items into version relationship * implement link and unlink actions * ItemVersionLinkerIT test * improve ItemVersionLinkerIT, fix ScriptRestRepositoryIT * resolve Copilot comments: Part 1 * resolve Copilot comments - part 2 * improve test to be more realistic * improve options description * better call of itemService.clearMetadata() * clear correctly dc.relation.replaces and dc.relation.isreplacedby * use dc.identifier.uri metadata value rather than item.getHandle() to set dc.relation.replaces and dc.relation.isreplacedby * code-cleanup * PR comments * resolve PR comments (O. Kosarko) * add also as a cli script * Use Item.ANY instead of null tested with the production db dump on the items mentions in the issue (11234/1-5537). It was returning: ``` The script has started Item '11234/1-5537' has no handle assigned. ``` because it's dc.identifier.uri.* --------- Co-authored-by: Ondřej Košarko <kosarko@ufal.mff.cuni.cz> Co-authored-by: Ondřej Košarko <ko_ok@centrum.cz> (cherry picked from commit 903b35a)
Implementation of the process script that implements two actions:
Limitations:
The item-version-linker process can only be started by admin user
Two items can be linked when
Item can be unlinked when:
Notes
In case there are only two items in a versioning history, and the last item is "unlinked", the entire versioning history is removed.
The script also sets/unsets the dc.relation.replaces and dc.relation.isreplacedby metadata on link/unlink actions.
Usage