BugFix: controlled vocab values validation

[ErykKul]

What this PR does / why we need it:
Bug fix.

Which issue(s) this PR closes:

• Closes Required controlledVocabulary metadata marked as valid while empty #11900

Special notes for your reviewer:

• see also the test; it illustrates how the bug can happen

Suggestions on how to test this:

• hard to reproduce the bug, the easiest way was to write a test for it and ensure no regression can happen.

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

• no

Is there a release notes update needed for this change?:

• probably not? Maybe one liner: "bugfix: controlled vocab values validation when empty"?

Additional documentation:

[coveralls]

coverage: 24.318% (+0.001%) from 24.317%
when pulling 08bfa30 on 11900-improved-cvoc-value-validation
into c4b4e82 on develop.

[qqmyers]

Do you know if these changes are needed (the ones in this class. I can see why the change in DatasetPage is important)? At a minimum, I think if isControlledVocabulary is true, there shouldn't ever be a regular value unless it's temporary, so you could always remove it in that case (possibly just adding || isControlledVocabulary to line 1795 instead of a second if statement.

Beyond that though - we are hopefully never adding a temporary value that is not blank or NA_VALUE, so I'd think that the original code should catch and remove the temp value without changes in this class (with your change to DatasetPage). (If not - if there are temp values getting added that are other strings, maybe we should find/fix those? Or catch them for normal fields too and not just controlled vocab?)

[qqmyers]

@ErykKul - just a ping - this is waiting on you for a response

[ErykKul]

I was mixing two things here, the code here was for removing no-longer-existing vocab values, but I am not sure if that was a good idea, and definitely should not be mixed in this PR with the bug solution for "valid" empty values (I first though it was the reason for them being empty and valid: an old controlled vocabulary value that was removed from the metadata-block shows empty but it is not empty and it is not NA value -> however, that was not the reason for the bug, I could reproduce it in a way as the issue describes).

[ErykKul]

I reverted the file.

[ErykKul]

I also removed the unit test; it was testing the not-in-vocabulary value in CV field:

        DatasetField subjectField = new DatasetField();
        subjectField.setDatasetFieldType(subjectType);
        subjectField.setDatasetFieldValues(new ArrayList<>());
        subjectField.setControlledVocabularyValues(new ArrayList<>());
        DatasetFieldValue orphanValue = new DatasetFieldValue(subjectField);
        orphanValue.setValue("__placeholder__");
        subjectField.getDatasetFieldValues().add(orphanValue);

This is not the cause of the bug, so it was testing the wrong thing (and started failing after cleanup of the file)

[ErykKul]

I also re-added workingVersion.validate(); in DatasetPage, it has side effects we still want to have (set validation messages on dataset fields, that are then showed in the UI).

[qqmyers]

FWIW: The previous build had a failure in the Harvest from DataCite test, but just rerunning the job fixed it, so presumably random/transient and not related to this PR.

[qqmyers]

Looks good. isValid() is checking that the version is valid after removal of temporary N/A values which makes sense. Not sure the underlying code is very efficient (cloning the version) but we use it elsewhere.

[stevenwinship]

@stevenferey @ErykKul
Unable to reproduce this bug. More info in the main issue

[github-actions]

📦 Pushed preview images as

ghcr.io/gdcc/dataverse:11900-improved-cvoc-value-validation

ghcr.io/gdcc/configbaker:11900-improved-cvoc-value-validation

🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name.