Refactor preservation logic and include hidden metadata #6244
Conversation
BartChris
force-pushed
the
refactor_preserve_logic
branch
4 times, most recently
from
dc87dde
to
f274f0e
Compare
BartChris
force-pushed
the
refactor_preserve_logic
branch
from
f274f0e
to
bfbcf5c
Compare
BartChris
force-pushed
the
refactor_preserve_logic
branch
from
8f39a4f
to
533709a
Compare
Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessFieldedMetadata.java
Outdated
Show resolved
Hide resolved
| return ((ProcessTextMetadata) value).getValue(); | ||
| } else if (value instanceof ProcessSelectMetadata) { | ||
| return String.join(" ", ((ProcessSelectMetadata) value).getSelectedItems()); | ||
| } |
There was a problem hiding this comment.
Missing to handle ProcessBooleanMetadata
There was a problem hiding this comment.
I added boolean metadata.
| private String extractSimpleValue(ProcessDetail value) { | ||
| if (value instanceof ProcessTextMetadata) { | ||
| return ((ProcessTextMetadata) value).getValue(); | ||
| } else if (value instanceof ProcessSelectMetadata) { |
There was a problem hiding this comment.
In general, this refactoring is less object-oriented. It is not good practise to have lists of if (x instanceof A) { … } else if (x instanceof B) { … } else if (x instanceof C) { … } ...
There was a problem hiding this comment.
I tried to adress that remark by putting the necessary logic in the specific class and call it using polymorphism.
There was a problem hiding this comment.
Please add missing case for ProcessBooleanMetadata.
In general, I am only half-convineced about this refactoring, because is less object-oriented then. It is not good practise to have lists of if (x instanceof A) { … } else if (x instanceof B) { … } else if (x instanceof C) { … }, if that can be avoided. But I understand as well that the other variant is clumsy to read and understand.
BartChris
force-pushed
the
refactor_preserve_logic
branch
from
2847366
to
652e63a
Compare
fixes #5220
alternative to #6238
This pull request addresses an issue where the special metadata fields "LABEL", "ORDERLABEL", and "CONTENTIDS" are lost when they are set as excluded metadata. When opening a process in the metadata editor and saving it, this loss can occur. A special problem is, that an excluded "ORDERLABEL" will result in broken pagination (#5220).
Pull request #6238 specifically resolves the issue of preserving pagination even when the ORDERLABEL is excluded by not resetting the ORDERLABEL in case of a page. However, in order to fully address the broader problem of losing the special metada fields when they are excluded, we must also ensure that the division data is updated based on hidden/excluded metadata.
This pull request not only resolves the issue of losing excluded metadata but also refactors the existing logic. The goal of the refactoring is to make the logic more explicit, improving both readability and maintainability. The current BiConsumer logic is difficult to follow, so I’ve aimed to preserve the functionality while making the code more understandable.
If this approach is rejected or introduces new problems, we should seek an alternative solution to properly handle hidden metadata.