Administer Workflow (Abort WorkflowItem, Delete WorkflowItem) #2727
Conversation
…wAdmin bean This moves some workspace and workflow specific code from SolrServiceImpl to the new SolrServiceWorkspaceWorkflowRestrictionPlugin. This also updates the SolrServiceResourceRestrictionPlugin to restrict those records based on read rights as well.
|
@tdonohue we need your help on that to decide how to proceed |
There was a problem hiding this comment.
@benbosman : Honestly, this approach seems good to me. I only have minor comments after an initial code review. I have not tested this yet.
@abollini : I responded to your question in your thread above. This approach seems to be the same as the current "workflow" configuration in Discovery, so I don't see an issue here (though as I mentioned it might be nice to better document why these index tasks instead of WorkflowItems). That said, we could create a ticket to revisit both configurations at a later time...but I think it's better that the two remain consistent for now.
| } else if (idxObj instanceof IndexableInProgressSubmission) { | ||
| final InProgressSubmission inProgressSubmission | ||
| = ((IndexableInProgressSubmission) idxObj).getIndexedObject(); | ||
| dso = inProgressSubmission.getItem(); | ||
| } else if (idxObj instanceof IndexablePoolTask) { | ||
| final PoolTask poolTask = ((IndexablePoolTask) idxObj).getIndexedObject(); | ||
| dso = poolTask.getWorkflowItem().getItem(); | ||
| } else if (idxObj instanceof IndexableClaimedTask) { | ||
| final ClaimedTask claimedTask = ((IndexableClaimedTask) idxObj).getIndexedObject(); | ||
| dso = claimedTask.getWorkflowItem().getItem(); |
There was a problem hiding this comment.
Not necessarily required of this PR, but this big if/else if seems like something we should refactor at some point. It implies to me that IndexableInProgressSubmission, IndexablePoolTask and IndexableClaimedTask should all implement an interface that has a getDSO() (or getRelatedDSO()) method. That would make this easier to extend in the future.
There was a problem hiding this comment.
I agree
We didn't want to change the discovery indexable objects in this PR, but it would definitely improve the functionality here, and can perhaps be used in the future in other locations where the item of any workflow object should be retrieved
dspace-api/src/main/java/org/dspace/discovery/SolrServiceSearchPlugin.java
Show resolved
Hide resolved
|
@tdonohue it is not exactly the same scenario. The workflow configuration (maybe we should rename it workflowTasks) is about exposing the task that can be claimed or are already claimed by an user so it is natural that it returns tasks. The workflowAdmin configuration really should expose workflowItem because this is what the admin can abort. Let me to use an example, we have a custom workflow that assign all submission to two reviewers in parallel. This mean that after one submission we have two claimed tasks, if we use the proposed configuration in this PR we will end to show two "rows" to the admin that can be both aborted. |
|
@abollini : Please re-review. It looks (to me) like @benbosman and @antoine-atmire decided to update this PR to change the |
There was a problem hiding this comment.
it looks fine to me. Thanks @benbosman to have revisited it
There was a problem hiding this comment.
👍 Looks good to me now as well. Thanks @benbosman and @antoine-atmire !
References
Description
This includes support for building an administer workflow page and the corresponding actions. This is part of beta 3
Instructions for Reviewers
List of changes in this PR:
Checklist