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

Incomplete validation by creation of new processes #3403

Closed
andre-hohmann opened this issue Apr 8, 2020 · 7 comments · Fixed by #4949 or #5519
Closed

Incomplete validation by creation of new processes #3403

andre-hohmann opened this issue Apr 8, 2020 · 7 comments · Fixed by #4949 or #5519
Labels

Comments

@andre-hohmann
Copy link
Collaborator

Problem

If a new process is created, the metadata is not validated for all mandatory metadata, according to the ruleset. The following metadata, however, is validated:

  • TSL/ATS (all documenttyps)
  • BandNr-Sortierung (multivolume)

Solution

Kitodo.Production 2.x validated all shown metadata. That should be possible in Kitodo.Production 3.x, too. Probably, this is a question of configuration.

@andre-hohmann
Copy link
Collaborator Author

It seems that there is no validation against the ruleset anymore. It might have the same cause as the problems described in the following issues:

@BartChris
Copy link
Collaborator

BartChris commented Sep 7, 2022

I think this has to do with some changes to the template done in the following PR: #5026

Maybe @solth can help here because the PR seems to have introduced a change which sets the mandatory metadata fields to required fields only in the case of the metadata editor (#{request.requestURI.contains('metadataEditor'). So that they are not required anymore during the creation of the process.

 <p:inputText id="inputText"
    value="#{item.value}"
    disabled="#{not item.editable or readOnly}"
    required="#{request.requestURI.contains('metadataEditor') and item.required and (not empty  
    param['editForm:save'] or not empty param['editForm:saveContinue'])}"
    styleClass="#{not item.editable or readOnly ? 'read-only disabled' : ''}">

</p:inputText>

relevant file: https://github.com/effective-webwork/kitodo-production/blob/master/Kitodo/src/main/webapp/WEB-INF/templates/includes/metadataTreeTable.xhtml

@BartChris
Copy link
Collaborator

@solth can you check why you defined the metadata fields in a way that the required attribute is only set in the editor? And not in the process creation form. I have some elements which are only editable during process creation, but i cannot enforce them as required there.

@BartChris
Copy link
Collaborator

BartChris commented Dec 8, 2022

@andre-hohmann @solth I just looked into this again and the situation right now is the following. An input element in the creation form of a process or in the metadata editor is (apart from the process title) never marked as required, because the condition

"#{request.requestURI.contains('metadataEditor') and item.required and (not empty  
    param['editForm:save'] or not empty param['editForm:saveContinue'])

basically says:

Only mark an element as required if

  • we are on the metadataeditor page (metadataeditor is in the URL)
  • the element is required (MinOccurence=1) by the ruleset and is not excluded by the display rules
  • this form has a 'save' Button
  • this form has a 'save and Continue' button

This constellation never appears (because the metadata editor only has a "Save and Exit" (not "Save and Continue Button") and the process creation form's url does not contain metadataeditor . So i am quite uncertain what is actually desired here.

The main question is: Should there be situations where a user is disallowed to save when required values are not set. (By making certain fields required.) One situation where this might be necessary would be, when a value is required and can only be set during process creation. It is then necessary to actually enforce that in the application. And i have scenarios where i definitely want this.

The problem would of course be that the way things are implemented right now, this would be true for every required key (as defined by ruleset) displayed on the process creation form. And there might of course be situations where a generally required key is not known during process creation. So the user has to enter a placeholder value here.
What do you think?

@andre-hohmann
Copy link
Collaborator Author

@BartChris : Thanks for the investigation. I support your point of view.

It should be possible that users are disallowed to save the process creation form when required values are not set. If required fields are not set, the users will not be able to validate the process in the metadata editor without warnings and the export will be aborted.
However, the user who works in the metadata editor might not know the values of the required keys as for example: rights information, digital collection, ... Especially when service provider are involved.

I cannot imagine a scenario in which the value of a required keys are not known in the process creation form. It is rather that way: The required keys are explicitly set in the process creation form to enable a validation.
What must definitely not happen: the process creation form cannot be saved, because a required key which is not shown in the form is not set.

@IkramMaalej
Copy link
Collaborator

@andre-hohmann Is this issue about validating only the required metadata or validating all metadata against the ruleset (number of occurrences, valid values, ...) ?

@andre-hohmann
Copy link
Collaborator Author

@IkramMaalej : I hope, we have the same understanding. For my understanding, this issue is about validating only the required metadata.
That is the behavior of mandatory elements marked with "*" in commons forms, for the fields, which are necessary to send a request, ...

If "TSL/ATS" "digital collections" or "rights information" are mandatory fields, but empty, the process should not be created, but an warning/error message should be shown as for example:

  • Fehlende Eingabe für Volume "Neunzehntes Jahrhundert": TSL/ATS.

Or the lines of the mandatory field becomes another color - as it was with "TSL/ATS" in the first versions of Kitodo.Production 3. See:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants