kitodo-production icon indicating copy to clipboard operation
kitodo-production copied to clipboard

Do not reset the ORDERLABEL if division is a page

Open BartChris opened this issue 1 year ago • 2 comments

fixes https://github.com/kitodo/kitodo-production/issues/5220 alternative: https://github.com/kitodo/kitodo-production/pull/6244 We should not reset the ORDERLABEL of a page during metadata preservation since this breaks the pagination when the ORDERLABEL is set to excluded. While this fixes the pagination issue there are other potential problems. I did an alternative pull request which tries to adress them all: https://github.com/kitodo/kitodo-production/pull/6244

BartChris avatar Sep 27 '24 16:09 BartChris

Although i did not yet notice any problems when not resetting the ORDERLABEL for pages (as done in this PR), we can also extend the original logic. The root cause of the linked issue is, that the necessary logic for setting the LABEL and ORDERLABEL (which are set to NULL in the beginning) is not called for hidden metatdata.

https://github.com/kitodo/kitodo-production/blob/eded39f924588f71d3adb5c2bc36abe0bc815623/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessFieldedMetadata.java#L681-L683

To solve the issue at hand we could for hiddenMetadata call the setters directly to not lose the ORDERLABELs and therefor our pagination. We would by this also preserve hidden division LABEL's, which might be lost.

private void processHiddenMetadata() {
        for (Metadata metadatum : hiddenMetadata) {
            if (metadatum instanceof MetadataEntry) {
                MetadataEntry metadataEntry = (MetadataEntry) metadatum;
                String key = metadataEntry.getKey().toUpperCase();
                String value = metadataEntry.getValue();
                switch (key) {
                    case "LABEL":
                        division.setLabel(value);
                        break;
                    case "ORDERLABEL":
                        division.setOrderlabel(value);
                        break;
                    case "CONTENTIDS":
                        division.getContentIds().add(URI.create(value));
                        break;
                }
            }
        }
        metadata.addAll(hiddenMetadata);
    }

A more general remark: The necessary setters for the divisions of (non-hidden) metadata elements in the UI are called as a side effect of the (IMHO very complicated) BiConsumer logic, which does some transformations to the values from the tree, but mainly defines the setters for different division values, which are then applied. There may be a specific reason for using functional interfaces here, but I wonder if there might be opportunities to simplify the logic for better readability and maintainability.

BartChris avatar Sep 30 '24 09:09 BartChris

@BartChris Please rebase against current master to avoid failing builds caused by broken Selenium tests!

solth avatar Oct 01 '24 05:10 solth

Since https://github.com/kitodo/kitodo-production/pull/6244 is merged, this is not needed any more.

BartChris avatar Jan 22 '25 09:01 BartChris