metadata101 / iso19139.ca.HNAP

ISO Harmonized North American Profile (HNAP)
GNU General Public License v2.0
4 stars 18 forks source link

allow other constraint to be filled #356

Open wangf1122 opened 10 months ago

wangf1122 commented 10 months ago

The current validation doesn't allow this other constraint field to be filled if the user didn't select "other restriction" for Access Constraint field or User Constraint field.

image

This pull request removes such barrier. Other validation will still stay (i.e. check other constraint's emptiness if the user select "Other restriction" for Access Constraint field or User Constraint field.)

ianwallen commented 9 months ago

@wangf1122 Could you explain the issue. - Based on my testing I don't see any issue.

The screenshot that you posted will be invalid because you did select other restriction for use constraint or access constraint Without your change, I can add other constraint using the following sample.

image

wangf1122 commented 9 months ago

@ianwallen @josegar74

We had discussed this issue during last community meeting session. The issue is this field doesn't allow any input if the user DOES NOT select "other restriction". So the error text is not so clear. I will ask the technical writer to compose another text and update the error itself.

wangf1122 commented 9 months ago

@josegar74 @ianwallen

There is a logic flaw in the current logic in this area. As we discussed from the last HNAP discussion session, if the user not select "other restriction" but decide to put some text to other constraint field. We should see error message saying this field needs to be empty. But it is pointed to the original none empty message. I added such scenario and message to handle this case. Here is the test result

1) If Access Constraints or Use Constraints field has no "other restriction", the error will be this image

2) If Access Constraints or Use Constraints field has "other restriction" but the other constraint is empty, then error will be 👍 image

The code is updated and you can do some general test based on this branch of code. The text update also went thru our technical writer

josegar74 commented 8 months ago

According to the HNAP 2.3.1 specification:


From the previous rules, I understand that otherConstraints is mandatory when accessConstraints or useConstraints are set to "otherRestrictions". In other cases, can be provided optionally.

The original validation, allowed only a value in otherConstraints, if accessConstraints or useConstraints are set to "otherRestrictions", reporting an error in other cases. This seems not correct according to the previous rules.


The pull request keeps the original validation, but provides clear messages. Please @ianwallen and @wangf1122 verify if my previous understanding is correct based on the specification text. If so I think the validation for OtherConstraintsNoteEmpty has to be removed.

In case that OtherConstraintsNoteEmpty should be kept, I noticed this issue with the validation (empty message for the multilingual validation):

validation-empty-message

ianwallen commented 8 months ago

From the previous rules, I understand that otherConstraints is mandatory when accessConstraints or useConstraints are set to "otherRestrictions". In other cases, can be provided optionally.

I agree that there is no specification indicating that otherConstraints cannot be used when accessConstraints or useConstraints are not set to otherRestrictions. So it does seem like it can be optional in this case.

My only concern is how FGP currently handles this situation. I just tested FGP and it fails with this use case so if we implement this change, then FGP also has to implement the change as well.

image

@bo-lu - could you provide your comments on this. Thank you