Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable manipulation of child process links in metadata editor #5507

Merged
merged 7 commits into from
Feb 20, 2023

Conversation

solth
Copy link
Member

@solth solth commented Jan 16, 2023

Fixes #3275
Fixes #4738

Based on #4850 by @matthias-ronge

@BartChris
Copy link
Collaborator

BartChris commented Jan 16, 2023

@solth I played a little bit with this feature. One question which came to my mind: Do we need another layer of permissions here? It seems to me that everybody who is allowed to change structural data can easily delete processes from a superordinate unit. I would prefer if there is a special permission for that.
And how can a link be reestablished after deletion? Is this possible at all?

Edit: I am wondering wether the interface for adding "structural elements" to superordinate units should be different than the one for "normal" processes. What is needed is a way to add subprocesses, not new structural elements, to actually establish a link between processes. But maybe i am overlooking some feature.

grafik

Edit2: I just noticed that it is (also in current Master) possible to add images and strucural elements to a superordinate unit (e.g. a multi volume work). This leads me to the question: Is it actually desired that elements can be freely added to superordinate units, when the ruleset allows for that? Or are superordinate units more restricted in the sense that they only hold metadata and pointers to subordinate processes and should not contain e.g. images?

@solth
Copy link
Member Author

solth commented Jan 16, 2023

@BartChris thanks for the comments.

Yes, it is already possible to (re-)establish links to subordinate processes directly from the metadata editor, and in fact it is possible from the very dialog that can be seen in your screenshot. You just have to check the checkbox "Verlinkung" and the GUI of the dialog will change to one that allows you to add process links. This means it is quite easy to create links between parent and child processes, even if a link was accidentally removed.

Because of this I am not convinved that we need another, separate permission to control whether links between processes can be removed (and added, for that matter), but this is probably something that the users are better suited to decide than me.

Concerning the question of your second edit: that is how Kitodo 3 is designed: only the ruleset controls what structures can be contained in another structure or document. This is of course something which can be discussed, but it something which is currently at the core of Kitodo.Production.

@BartChris
Copy link
Collaborator

BartChris commented Jan 16, 2023

@solth Thanks a lot for your explanation, i forgot about that. While checking that i encountered a bug in the search functionality, for which i made another issue. #5509

You are right that the restoration of links to sub processes is quite easy (if the search function is repaired), but i still am afraid that the deletion of a process link might happen by accident and stays unnoticed. But maybe others have a different opinion on that. We already had a similar discussion here (#5406). The main problem here is for me, that i cannot really control who is able to call the destructive operation of deleting a link between processes (basically everybody who is allowed to edit the structure tree) while in case of the deletion of images i can give this right only to specific persons.

The general feature of adding images to a super-ordinate process, is probably better discussed in a GitHub discussion.

@solth solth force-pushed the process-hierarchy-manipulation branch from 64730aa to b4f3169 Compare January 20, 2023 08:20
Copy link
Collaborator

@andre-hohmann andre-hohmann left a comment

Choose a reason for hiding this comment

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

I have tested the following scenario to modify subordinate process titles in the metadata editor view of the superordinate process.

1 Change the order of subordinate process titles in the metadata editor

  • [The order of the subordinated process titles was changed in the metadata editor.
  • The order of the subordinated process titles was changed in the METS file of the superordinate process.]

2 Unlink subordinate process title in the metadata editor

  • An existing subordinate process title was unklinked in the metadata editor.
  • An existing subordinate process title was unklinked in the METS file of the superordinate process.

3 Add subordinate process title in the metadata editor

  • A subordinate process title was added in the metadata editor.
  • A subordinate process title was added in the METS file of the superordinate process.

The error message described in #5509 could not be reproduced.

Thus, from the user perspective the the PR is approved.

@solth
Copy link
Member Author

solth commented Jan 20, 2023

@andre-hohmann thank you for the review!

The error message described in #5509 could not be reproduced.

@BartChris already fixed that issue with PR #5514 which has been merged into the master branch. This branch has been rebased against the updated master.

Copy link
Collaborator

@henning-gerhardt henning-gerhardt left a comment

Choose a reason for hiding this comment

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

Changes look good but the white space changes made is hard to read.

@solth solth modified the milestone: Kitodo.Production 3.6.0 Feb 6, 2023
@solth solth force-pushed the process-hierarchy-manipulation branch 2 times, most recently from 0937a00 to 473ebb2 Compare February 8, 2023 18:26
@solth solth force-pushed the process-hierarchy-manipulation branch 3 times, most recently from c096814 to f3eb2fc Compare February 17, 2023 21:50
@solth solth force-pushed the process-hierarchy-manipulation branch from f3eb2fc to 291350c Compare February 20, 2023 09:43
@solth solth merged commit 5a959f3 into kitodo:master Feb 20, 2023
@solth solth deleted the process-hierarchy-manipulation branch February 20, 2023 10:56
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.

Enable corrections of volume-processes in the metadata editor Process link cannot be deleted
4 participants