7.1.x Scaffolding Display Constraint Expansion

[codeconsole]

Modify display constraint so that

1. it can override blacklist behavior if set. (e.g. Date dateCreated currently will always not show up.
2. Can specify display behavior (input/output)

For instance, let's say you have String password you would want display: INPUT_ONLY
For Date dateCreated, perhaps ideal configuration is display: OUTPUT_ONLY

class User {
    String password
    Date dateCreated
    Date lastUpdated

    static constraints {
        dateCreated display: OUTPUT_ONLY // show/index override
        lastUpdated display: ALL //  equivalent to a display: true  override
        password display: INPUT_ONLY // create/edit
    }
}

This is backwards compatible with display: boolean

[matrei]

Looks useful!

I find the DisplayType values a bit unintuitive. Suggestion:

DisplayType.ALL -> DisplayType.ALWAYS
DisplayType.NONE -> DisplayType.NEVER
DisplayType.INPUT_ONLY -> DisplayType.WRITE_ONLY
DisplayType.OUTPUT_ONLY -> DisplayType.READ_ONLY

[codeconsole]

Looks useful!

I find the DisplayType values a bit unintuitive. Suggestion:

DisplayType.ALL -> DisplayType.ALWAYS
DisplayType.NONE -> DisplayType.NEVER
DisplayType.INPUT_ONLY -> DisplayType.WRITE_ONLY
DisplayType.OUTPUT_ONLY -> DisplayType.READ_ONLY

There already is an editable constraint that controls whether a field is read-only in forms. READ_ONLY as a display type might be confused with that concept.

"input" and "output" directly correspond to the view types:

• Input views = create/edit forms (where users input data)
• Output views = show/index pages (where data is output to users)

Using WRITE_ONLY and READ_ONLY introduces a different mental model (database operations) that doesn't quite match what's happening. The scaffolding is about display in views, not about read/write permissions to the database. A property marked OUTPUT_ONLY can still be written to programmatically - it's just not shown in input forms.

I chose NONE because it is similar to display: 'none' is a css rule which is pretty common.

ALL or ALWAYS I am indifferent on and I guess if we went with ALWAYS, NEVER would make more sense.

[jdaugherty]

So to date, everything has been an optional / minor breaking change. But changing constrained means that this will be a breaking change, no? @matrei what are your thoughts on this?

[codeconsole]

@jdaugherty is adding a constraint breaking?

[jdaugherty]

Adding the constraint isn't, but changing the base interface that's applied to every object I assume would be breaking. We need to test this.

[jdaugherty]

@sbglasius is going to test this change with his side project to ensure it's backwards compatible.

[matrei]

Yes, we should test this, we don't want to have to recompile/re-release plugins for 7.1 compatibility.

[sbglasius]

Turns out, I'm implementing grails.gorm.validation.Constraint and not grails.gorm.validation.Constrained

I think it would be best, if @codeconsole makes a Constrained implementation with 7.0.4 and run it with 7.1.0-SNAPSHOT

Or direct me on how to do so.

[jdaugherty]

@sbglasius I think the real concern is this being a breaking change across domain objects for 7.x.

If you have your plugin with a domain class on 7.0.4, and you have your application on 7.1, what happens when you call findConstrainedProperties(Holders.grailsApplication.mappingContext.getPersistentEntity(MyDomain.class)) in your application? If it works, this isnt' a breaking change, if it doesn't, it is.

[matrei]

There already is an editable constraint that controls whether a field is read-only in forms. READ_ONLY as a display type might be confused with that concept.

@codeconsole Yes, that's a valid point. I'm fine with the naming as it is.

[jdaugherty]

My only remaining concern is if this is a breaking change. If it is, we need to agree as a group that this is ok for a minor release since it will force all plugins to be recompiled again.

[codeconsole]

@jdaugherty what is the concern that this is breaking? This PR is for 7.1.x apps. Why would you run 7.1.x scaffolding plugin on 7.0.x? This isn't an AST transformation.

[jdaugherty]

@codeconsole This scenario: create a 7.0.x plugin with a domain, don't update the 7.0.x plugin, have your 7.1.x app pull in that plugin (doesn't rebuild). This PR has changed an interface that is injected via an AST so it may break. I haven't had time to put together a multiproject build to test this unfortunately. It's on my list to do it after the holidays. Ultimately, we're trying to avoid rebuilding every grails plugin in minor releases unless there's a good reason.

[codeconsole]

@codeconsole This scenario: create a 7.0.x plugin with a domain, don't update the 7.0.x plugin, have your 7.1.x app pull in that plugin (doesn't rebuild). This PR has changed an interface that is injected via an AST so it may break. I haven't had time to put together a multiproject build to test this unfortunately. It's on my list to do it after the holidays. Ultimately, we're trying to avoid rebuilding every grails plugin in minor releases unless there's a good reason.

@jdaugherty I don't think that is how AST transformations work. AST transformation that add an interface don't also inject the implementation of that interface. The implementation is external and part of the grails-datamapping-validation jar.

Therefore, If you have a 7.0.x plugin with a domain and grails is upgraded to 7.1.x, it is going to use the 7.1.x dependencies and there respective implementations.

[matrei]

@codeconsole @jdaugherty

I have tested this with a 7.1.x application of this branch, and a 7.0.x plugin with a domain and it is not breaking.

However, for:

// domain object in application (not plugin, as DisplayType is not available there yet)
package testcompat

import grails.gorm.validation.DisplayType

class Cat {

    String name
    String type
    String color

    Date dateCreated
    Date lastUpdated

    static constraints = {
        name(nullable: false, size: 0..20)
        type(display: DisplayType.INPUT_ONLY)
        dateCreated(display: DisplayType.OUTPUT_ONLY)
        lastUpdated(display: DisplayType.ALL)
    }
}

I would have expected to see dateCreated in the show view, and both dateCreated and lastUpdated in the list view:

[matrei]

@codeconsole Thanks, now the dateCreated and lastUpdated fields show up in the expected views.