grails / scaffolding-old Goto Github PK

Normally if a grails domain class attribute is not nullable, then it is required by scaffolded views. In other words, the field implementing that field has required="true" set.

However, this doesn't work for booleans because in the DomainPropertyImpl.groovy glass, it has this:

    boolean isRequired() {
        if (type in [Boolean, boolean]) {
            false
        } else if (type == String) {
            // if the property prohibits nulls and blanks are converted to nulls, then blanks will be prohibited even if a blank
            // constraint does not exist
            boolean hasBlankConstraint = constrained?.hasAppliedConstraint(BLANK_CONSTRAINT)
            boolean blanksImplicityProhibited = !hasBlankConstraint && !constrained?.nullable && convertEmptyStringsToNull && trimStrings
            !constrained?.nullable && (!constrained?.blank || blanksImplicityProhibited)
        } else {
            !constrained?.nullable
        }
    }

Thus preventing booleans from participating with this odd special case code. Now sure, we could get into a philosophical discussion about whether a nullable field is truly "boolean", but the reality is, many boolean database fields are nullable,
the grails system allowed both nullable true and nullable false on booleans, and if you wanted purely yes and no outcomes, you would put nullable: false. The fact you put nullable: true in the constraints means that you want to cater for nulls, or no outcome.

However, it's also a problem if you don't want to cater for nulls, because UIs look at "required" for validation. And if you want to model a boolean with a drop down that starts out as blank, then there is a problem because it is marked not required, even though it is required, thus validation code doesn't work.

So the net result is that if say, you're using the fields plugin and you do something like this...

                <f:field bean="booking" property="male" label="gender"/>

where "male" is a boolean nullable: false field on the booking domain object, then unlike every other type of field marked nullable: false, the template won't get passed the "required" attribute, thus the UI won't mark that field as required and the validation won't enforce it as required.

Now sure, the default field implementation of booleans is a checkbox, which doesn't allow null, but I want to model it as a drop down... (using this in views/_fields/booking/male/_wrapper.gsp is automatically inserted by the fields plugin)

<div class="fieldcontain${required?' required':''}">
    <label for='${property}'>${label}
        <g:if test="${required}"><span class='required-indicator'>*</span></g:if>
    </label>
    <g:if test="${required}">
        <g:select name="${property}" from="${[[key: true, value: 'MALE'], [key: false, value: 'FEMALE']]}" noSelection="${['': '']}" optionKey="key" optionValue="value" value="${value}" required="${required}"/>
    </g:if>
    <g:else>
        <g:select name="${property}" from="${[[key: true, value: 'MALE'], [key: false, value: 'FEMALE']]}" noSelection="${['': '']}" optionKey="key" optionValue="value" value="${value}"/>
    </g:else>
</div>

A drop down that starts out showing blank or null, but requires you to select an outcome.

The net result of required not being passed to the template is firstly, it can't mark the field as mandatory, whether that be with an "*" asterisk, or red or whatever way the UI has designated. The other thing is the UI can't enforce it being mandatory when required, leading to bad data.

Steps to Reproduce

The way it manifests for me is using the fields plugin to automatically generate scaffolding, so having a domain class with a Boolean that has a constraint nullable: false, though no doubt it manifests other ways that wrongly assume booleans can't be required fields.

Expected Behaviour

There's no reason that Boolean fields should not allow both required and not required as possible attributes, manifesting from nullable: false and nullable: true constraints.

In my opinion the correct solution is to change DomainPropertyImpl.isRequired() thus:

    boolean isRequired() {
        if (type == String) {
            // if the property prohibits nulls and blanks are converted to nulls, then blanks will be prohibited even if a blank
            // constraint does not exist
            boolean hasBlankConstraint = constrained?.hasAppliedConstraint(BLANK_CONSTRAINT)
            boolean blanksImplicityProhibited = !hasBlankConstraint && !constrained?.nullable && convertEmptyStringsToNull && trimStrings
            !constrained?.nullable && (!constrained?.blank || blanksImplicityProhibited)
        } else {
            !constrained?.nullable
        }
    }

I thought about should boolean, aka the non-nullable java type with lower case "b" be treated specially, since it can't hold a null.
It's a debatable point, but the old behavior is that it was never "required" always returning false, whereas I would argue it is
always required since it can't be null. At the end of the day, since UI views use this attribute, and since we don't know how it
will be modelled, whether the UI starts by showing some kind of blank or not, we should leave it to the programmer
to decide whether the UI should be regarding it as required or not in the case. But either way, the Boolean type with big "B" which clearly allows null, but be allowed to be marked required or not required through the nullable: attribute.

Environment Information

• Grails Version: 4.0.10
• JDK Version: 1.8
• Container Version (If Applicable): TODO

Example Application

• TODO: link to github repository with example that reproduces the issue