Process parent link

[solth]

Fixes #5626

Builds upon #5663 by @BartChris

As discussed in that pull request, processes in projects that are not assigned to the current user cannot be chosen as parent process in the TitleRecordLinkTab by default. Instead the corresponding option will be deactivated and a note will be displayed to inform the user why he cannot link to that specific parent process:

This PR adds a new permission, though, that allows a user to link processes to parent processes in other projects which are not assigned to the current user. The option in the TitleRecordLinkTab will still contain the hint about the parent process belonging to a project to which the current user does not have regular access, but the option to select said parent process will now be activated:

@andre-hohmann & @BartChris does this pull request meet your requirements for the discussed functionality?

[andre-hohmann]

From the (SLUB Dresden) users' perspective it works fine, in general.

It is possible to import volumes of multivolume works and link them with the superordinate process, even if the superordinate process is located in a project to which a person does not have permissions.
The user is informed that the process is linked, but that the user cannot access the superordinate process via metadata editor, ...

This is the main use in the SLUB Dresden and it works very fine - thanks a lot!

I have tried to apply to periodical volumes, but this does not work as expected.
If i enter the identifier of a superordinate process, only one process of a volume is shown.

I would expect the superordinate process, like:

As we do not apply the functionality in this way for periodicals, it does not bother us. I just want to give a hint, that this might need some examination. Maybe, that is the expected behavior?

I have assigned "Request change" because i am not certain, if the functionality with the periodicals works as expected.
I cannot give any precise requirements, but you can contact me, if there are further questions.

[BartChris]

@solth Thanks a lot for working on that again. I am again trying to work myself through the code. The general functionality seems to work good. But the way the code is written right now leads to a behavior where always only one possible parent is shown to the user. So the user does not even see all possible parents (independent of the fact wether he is able to actually select a specific process)

Imagine a situation where you have one possible parent process in the same project (as the to be created child record), and another possible parent process in a different project. Right now, only one of those is available for selection by the user.

Scenario a) Using the search button in the title record link tab:

We search for multiple possible parent processes, but after we set one of them as the parent process using setParentAsTitleRecord we break out of the for-loop so no other possible parent gets placed into the select menu.

kitodo-production/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/TitleRecordLinkTab.java

Lines 319 to 332 in a9ea218

	possibleParentProcesses = ServiceManager.getImportService()
	.getPotentialParentProcesses(processes, MAXIMUM_NUMBER_OF_HITS);
	} catch (DataException | DAOException e) {
	Helper.setErrorMessage("createProcessForm.titleRecordLinkTab.searchButtonClick.error", e.getMessage(),
	logger, e);
	indicationOfMoreHitsVisible = false;
	possibleParentProcesses = Collections.emptyList();
	}
	for (SelectItem selectItem : possibleParentProcesses) {
	if (!selectItem.isDisabled()) {
	int processId = Integer.parseInt(selectItem.getValue().toString());
	try {
	setParentAsTitleRecord(ServiceManager.getProcessService().getById(processId));
	break;

This could mean, that a parent process in the same project gets skipped because a possible parent process in the other project is chosen. (Given the case that the user has the ability to link to processes in other projects). This could happen also because the found processes are not sorted in that case of a manual search as done in the case of the automatic search for a parent record after the catalog import (

kitodo-production/Kitodo/src/main/java/org/kitodo/production/services/data/ImportService.java

Line 986 in a9ea218

public List<ProcessDTO> sortProcessesByProjectID(List<ProcessDTO> processDTOs, int projectId) {

), see b)

We should always show all possible parents when a user uses the search functionality.

Scenario b) Automatic search for parent after catalog import.

During a search for a title using catalog import Kitodo triggers an automatic search for parent records in the Kitodo database using the method loadParentProcess. This method only returns a single parent:

kitodo-production/Kitodo/src/main/java/org/kitodo/production/services/data/ImportService.java

Lines 967 to 970 in a9ea218

	if (process.getRuleset().getId().equals(ruleset.getId())) {
	parentProcess = process;
	break;
	}

So when calling setParentAsTitleRecord we again end up with only one selectable possible parent in our select box even if there is more than one possible parent process.

So the question would be, wether we want to show the user all possible parents in all projects or wether we want to automatically select one, as it is implicitly done with the current implementation.

[BartChris]

This is the main use in the SLUB Dresden and it works very fine - thanks a lot!

I have tried to apply to periodical volumes, but this does not work as expected. If i enter the identifier of a superordinate process, only one process of a volume is shown.

@andre-hohmann The problem is that the search implemented in the Title Record Link tab is not actually a search for a parent, but a search for every process whose ID or title matches the given input. Since every volume of the peridocial has the same base value as part of the identifier Kitodo just finds randomly one of those entries that match. (So in the example a single volume of that periodical)

kitodo-production/Kitodo/src/main/java/org/kitodo/production/services/data/ProcessService.java

Line 819 in a9ea218

public List<ProcessDTO> findLinkableParentProcesses(String searchInput, int projectId, int rulesetId)

You would have to specify the precise ID of the parent in order to find only the parent process.

[andre-hohmann]

@BartChris : Thanks a lot for the investigation and explanation!

This could become more obvious, if all processes are shown, which contain the search criteria.
With regard to our current use cases, it is not important but good to know.

[BartChris]

@andre-hohmann The problem is that the search implemented in the Title Record Link tab is not actually a search for a parent, but a search for every process whose ID or title matches the given input. Since every volume of the peridocial has the same base value as part of the identifier Kitodo just finds randomly one of those entries that match. (So in the example a single volume of that periodical)

The comment of the findLinkableParentProcesses method already indicates that returning only possible parents would require to look up the rulesets of the parent candidate. But it seems like this is not happening.

Searches for linkable processes based on user input. A process can be linked if it has the same rule set, belongs to the same client, and the topmost element of the logical outline below the selected parent element is an allowed child. For the latter, the data file must be read at the moment. This will be aborted after a timeout so that the user gets an answer (which may be incomplete) in finite time.

We could use ProcessService.canCreateChildProcess for that. But i am not sure about the performance implications if we apply that here (it would be good, if Elasticsearch would allow us to filter out all possible parents):

kitodo-production/Kitodo/src/main/java/org/kitodo/production/services/data/ImportService.java

Line 1431 in a9ea218

public ArrayList<SelectItem> getPotentialParentProcesses(List<ProcessDTO> parentCandidates, int maxNumber)

and extend the check with that condition:

 public ArrayList<SelectItem> getPotentialParentProcesses(List<ProcessDTO> parentCandidates, int maxNumber)
            throws DAOException, IOException {
        ArrayList<SelectItem> possibleParentProcesses = new ArrayList<>();
        for (ProcessDTO process : parentCandidates.subList(0, Math.min(parentCandidates.size(), maxNumber))) {
            if (ProcessService.canCreateChildProcess(process)) {
                SelectItem selectItem = new SelectItem(process.getId().toString(), process.getTitle());
                selectItem.setDisabled(!userMayLinkToParent(process.getId()));
                if (!processInAssignedProject(process.getId())) {
                    String problem = Helper.getTranslation("projectNotAssignedToCurrentUser", process.getProject().getTitle());
                    selectItem.setDescription(problem);
                    selectItem.setLabel(selectItem.getLabel() + " (" + problem + ")");
                }
                possibleParentProcesses.add(selectItem);
            }
        }
        return possibleParentProcesses;
    }

or filter the records directly in the search service

  public List<ProcessDTO> findLinkableParentProcesses(String searchInput, int rulesetId)
            throws DataException, DAOException, IOException {

        BoolQueryBuilder processQuery = new BoolQueryBuilder()
                .should(createSimpleWildcardQuery(ProcessTypeField.TITLE.getKey(), searchInput));
        if (searchInput.matches("\\d*")) {
            processQuery.should(new MatchQueryBuilder(ProcessTypeField.ID.getKey(), searchInput));
        }
        BoolQueryBuilder query = new BoolQueryBuilder().must(processQuery)
                .must(new MatchQueryBuilder(ProcessTypeField.RULESET.getKey(), rulesetId));
        List<ProcessDTO> filteredProcesses = new ArrayList<>();
        for (ProcessDTO process : findByQueryInAllProjects(query, false)) {
                if (ProcessService.canCreateChildProcess(process)) {
                    filteredProcesses.add(process);
                }
        }
        return filteredProcesses;
    }

[andre-hohmann]

@solth and @BartChris :
i am not sure, how to proceed with @BartChris last proposal.

On the one hand, i think there is no need to solve the problem for periodical-processes. Maybe, this is only a problem of the SLUB Dresden due to the the local configuration to create the process title - and we have no disadvantage with the current workflows.
On the other hand, i feel a bit uncomfortable to approve the pull request despite i am not fully convinced about the functionality.

As i do not see a use case which affects the SLUB Dresden (or other institutions) and i cannot estimate the resources and effects of the last proposal to improve the functionality, i would approve the current version of the PR. Then it can be merged.
We will then find out whether the behavior of periodical process leads to specific problems in other institutions and we could pick up the discussion again.

Is this procedure OK for you?

If yes, @henning-gerhardt could review the technical aspects of this PR.

[BartChris]

@andre-hohmann
I also want to move forward here, so i agree that we should strive for a merge. Regarding my different comments from above:

• The association to a parent process can always be changed later on. So yes, there might be situations, where we have multiple instances of a parent catalog id and Kitodo might (if one process is chosen automatically during import) associate to the wrong "process", but the user can change that manually. So i would be on board with the behaviour of the PR, that Kitodo (when automatically searching for a parent after import) picks one. Even if it picks the "wrong" process. The user can then correct this manually (e.g. by searching again in the Title Link tab or later in the metadata editor). I think we can optimize here (not so easy since this effects mass import), but maybe we can do that at a later point.
• The fact that the search in the Title Record Link Tab is not a search for possible parents only (but for everything) might be corrected in a later Pull Request, so that we longer delay the main changes here.
• The behaviour that the search in the Title Link tab only returns one result should probably be changed since in Kitodo 3.6 the software returns more than one result.

So the main suggested change to this PR would be that we should display multiple results in the search in the Title Record Link tab since there is no real reason why that worked in Kitodo 3.6 but does not work in 3.7 anymore. (And it is also required to enable the user tho choose between multiple possible parents).

[henning-gerhardt]

@BartChris

we should display multiple results in the search in the Title Record Link tab since there is no real reason why that worked in Kitodo 3.6 but does not work in 3.7 anymore

Is this issue introduced by this pull request or is this issue not related to this pull request and is existing already in 3.7 / master branch? If this issue is not related to this pull request then it should be solved by a separate pull request.

[henning-gerhardt]

The Map can be simplified regarding it used types from class types to primitive types as there is no real need for classes to be used here.

[henning-gerhardt]

Return value is false or true but never null so return type of method can be changed to primitive boolean.

[andre-hohmann]

Thanks!
I will open a new issue to describe the missing functionality, which has been summarized by @BartChris.
It is then possible to create an own PR for that issue with the necessary discussion.

[andre-hohmann]

As described in the discussions, i approve this pull requests because the main aspect has been solved:

• Subordinate processes cannot be linked to superordinate process in another project #5626

A new problem is described in a separate issue and should be solved by an own pull request:

• Display multiple results in the search in the Title Record Link tab #5963

[BartChris]

@BartChris

we should display multiple results in the search in the Title Record Link tab since there is no real reason why that worked in Kitodo 3.6 but does not work in 3.7 anymore

Is this issue introduced by this pull request or is this issue not related to this pull request and is existing already in 3.7 / master branch? If this issue is not related to this pull request then it should be solved by a separate pull request.

This is introduced by the current pull request. In master you get multiple hits.

[henning-gerhardt]

This is introduced by the current pull request. In master you get multiple hits.

Then this must be fixed in current pull request.

[solth]

@andre-hohmann , @BartChris , @henning-gerhardt sorry for not responding earlier, I didn't find the time to work on this pull request in the last few weeks. I will try to incorporate the changes in the functionality (show multiple potential parent processes in the pulldown menu as options, maybe check/filter for "canCreateChildren") and implement the change requests from the code review.

[solth]

We could use ProcessService.canCreateChildProcess for that. But i am not sure about the performance implications if we apply that here (it would be good, if Elasticsearch would allow us to filter out all possible parents):
...
or filter the records directly in the search service
...

I think we need to do the check in ImportService.getPotentialParentProcesses instead of ProcessService.findLinkableParentProcesses, because the former is also used in TitleRecordLinkTab.setParentAsTitleRecord (for automatically determining the parent process during import) and thus would not be covered if we added the filter to the later function. @BartChris do you agree?

[solth]

Also, wouldn't it be necessary to also check for createChildrenWithCalendar in addition to createChildrenFromParent, to be able to manually link newspaper issues?

[solth]

@BartChris can you formally approve this PR or make change requests if you see something that should be adjusted before merging?

[henning-gerhardt]

Can this new test class written with Junit5 usage instead of Junit4? Thank you.

[solth]

Done.

[BartChris]

@solth
I approve the Pull Request in the current form.

We probably need to document the behavior (in the current or in an adapted form) before releasing 3.7. Outstanding issues which can be adressed later:

• during Mass import there is no check wether the user triggering the Mass import has the right to attach a process to a parent process in a project he or she does not have access to. as The check wether this is allowed is only reached during "normal" import via the UI (

  kitodo-production/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/TitleRecordLinkTab.java

  Line 449 in d40c2fd

  public void setParentAsTitleRecord(Process parentProcess) {

  )
• When somebody imports a record using the UI, which has a parent record, Kitodo will automatically find the parent record. But if the user does not have the necessary right the method setParentAsTitleRecord checks the rights (

  kitodo-production/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/TitleRecordLinkTab.java

  Line 449 in d40c2fd

  public void setParentAsTitleRecord(Process parentProcess) {

  ) so that the process is not shown to the user. The dropdown is just empty. I suggest that in that case, the parent record is shown, but greyed out, so that the user has an idea of what is actually going on. (Kitodo behinds the scenes gets the parent record from the UI, creates a parentTempProcess, but discards it while constructing the TitleRecordLinkTab). Calling setChosenParentProcess(null);

[henning-gerhardt]

@BartChris : maybe by opening two issues with references to this issue? Both issues can be solved independent of each other (or maybe even together) so maybe one of the issues could be solved earlier than the other. Especially the second sound for me more important then the first one but maybe only as we did not use the mass import feature.

[BartChris]

I will add those issues, when the current PR gets merged and will also inspect wether there are more potential features to consider and to be adressed in issues. (e.g. topics outlined in this comment #5869 (comment), section on automatic import)

[solth]

I approve the Pull Request in the current form.

@BartChris Sorry, I thought you had the "Approve" button as in other pull requests when reviewing the changes in the "Files" tab, but apparently that was not the case.