fields icon indicating copy to clipboard operation
fields copied to clipboard

Bug corrections and enhancement proposal for f:display for Grails 3

Open namgang opened this issue 8 years ago • 5 comments

This issue only applies to the grails3 branch. I don't know how to handle a proper pull request, sorry.

  • Currently it's not possible to exclude persistent properties in f:display
  • Currently it's not possible to include non-persistent properties in f:display

In FormFieldsTagLib.groovy, in the closure defining the display tag, replace def properties = domainClass.persistentProperties.sort(new DomainClassPropertyComparator(domainClass)) by def properties = resolvePersistentProperties(domainClass, attrs)

The latter method handles exclusions. Note that render following this line passes domainProperties into the GSP. However, _list.gsp does not pick up that variable but invokes domainClass.persistentProperties again, wasting the work done to select properties. Change to ${domainProperties} in _list.gsp. That makes 2 changes.

So far only bug corrections. The documentation does not mention it, but the fields plugin picks up a static scaffold attribute from the domain class, if present. The value of the attribute must be a map. The resolvePersistentProperties() method checks that map and uses its exclude key, if defined, to exclude properties from the view.

It is quite easy to add logic to check an include key of the same scaffold static property. It may be used to include non-persistent properties in the view. Add the following code after the three properties.removeAll calls in the resolvePersistentProperties method.

if (scaffoldProp?.include) {
    def inclusion = domainClass.properties.findAll {prop ->
        scaffoldProp.include.contains(prop.name) && !properties.find {it.name == prop.name}
    }
    properties.addAll(inclusion)
}

The condition makes sure we don't duplicate a property that's already included.

namgang avatar Jun 02 '16 19:06 namgang

@namgang I suggest, that you try to do a Fork, clone the git repository, make a branch, and commit and push that branch. Check out this guide: https://gist.github.com/Chaser324/ce0505fbed06b947d962 and become a contributor to the Fields Plugin 👍

sbglasius avatar Feb 24 '17 08:02 sbglasius

Can we really include the non persistent properties ? as of now, it needs GrailsDomainClassProperty, and there will not be GrailsDomainClassProperty instance for a non persistent property.

May be, we can consider support for inclusion of non persistent properties when we implement #115 Currently, when the property is not specified explicitely the code expects the bean to be a domain class, when we remove this dependency on domain class we may support including any arbitrary properties.

snimavat avatar Apr 01 '17 17:04 snimavat

Sure you get a GrailsDomainClassProperty for a non-persistent property. It will flag isPersistent() = false, but it's there ok.

namgang avatar Apr 01 '17 17:04 namgang

Ah ok, will check that, so when domainClass.persistentProperties is called, doesnt it return the non persistent properties ? --- by non persistent properties, do u mean transients ?

I will check this and see if i can give get a pr over weekend..

snimavat avatar Apr 01 '17 17:04 snimavat

If we support the 'include' wouldnt users expect to work some thing like how "include" works in data binding.. eg if include is specified, include only those properties and no others, if exclude is specified include all other then excluded

May be it will just need good docs..

snimavat avatar Apr 01 '17 18:04 snimavat