Index location doesn't work for Dexterity content

[wesleybl]

BUG/PROBLEM REPORT (OR OTHER COMMON ISSUE)

What I did:

• I created a Dexterity type with the location field.
• I created a location index of type ZCTextIndex in portal_catalog.
• I created an object of the content type that had the location field.

What I expect to happen:

Hope the catalog indexing the location field of the object created in location index.

What actually happened:

The location field is not indexed.

What version of Plone/ Addons I am using:

Plone: 5.2.6

This is because Plone define an location index but uses the getField method, which is for Archetype:

https://github.com/plone/Products.CMFPlone/blob/f2d8813d0a1b82cd8ef3adfc0554609452b1e51c/Products/CMFPlone/CatalogTool.py#L258-L260

As Archetype is at the end of its life, I think this index can be removed, or set to Archetype only.

[wesleybl]

In Plone 5.2, is it still necessary to maintain support to Archetype for this index? Or can we just make it work for Dexterity?

@mauritsvanrees @jensens @ale-rt

[ale-rt]

Plone 6

That indexer can be totally removed in Plone 6.

Plone 5.2

It is still needed in Plone 5.2, it should not go away!

Of course it should be fixed, I give you some options

Option 1

It should be registered only if we have AT:

try:
     from Prod.AT.Whatever import IProperIface
except:
     IProperIface = None

if IProperIface:
     @indexer(IProperIface) 
     def location(obj): 
          return obj.getField('location').get(obj) 

Also a similar "zcml condition" should be applied here: https://github.com/plone/Products.CMFPlone/blob/f2d8813d0a1b82cd8ef3adfc0554609452b1e51c/Products/CMFPlone/catalog.zcml#L20

See an example of zcml-condition https://github.com/plone/plone.restapi/commit/22af70e64c324d33e150679df74cd4489a17a391#diff-e640b28f1dbd6ebc932d29ef51a80a30a20d143f70b64edf5b33d6f01a818f6bR23

Option 2

Another acceptable (and cheaper) fix I think it might be:

 @indexer(Interface) 
 def location(obj): 
     try:
          return obj.getField('location').get(obj) 
     except AttributeError:
          return obj.location

Option 3

Wait for other people suggestions or pick another way you see fit :p Of course I did not test any of the things above and they could prove to be very wrong

[mauritsvanrees]

Alessandro is a smart guy, you should listen to him. :-) I agree with him. Either of those options should be fine.

[wesleybl]

That indexer can be totally removed in Plone 6.

Plone 5.2 It is still needed in Plone 5.2, it should not go away! Of course it should be fixed, I give you some options

I wonder if in Plone 6 and in Plone 5.2 with Dexterity, if this index should be removed.

The Event type in Dexterity still has the location field:

https://github.com/plone/plone.app.event/blob/4.0.0a7/plone/app/event/dx/behaviors.py#L197

[ale-rt]

If no filter is present the catalog will look for obj.location, which is what we want for dx types :)

[wesleybl]

If no filter is present the catalog will look for obj.location, which is what we want for dx types :)

@ale-rt In fact, we don't have an index in the catalog for location. Just one metatada, which uses this adapter to set the value of metatada.

But the fact is that location metadata is not working with Dexterity today. So it is not making lack. It might be an opportunity to remove the adapter and metadata in Plone 6.

[wesleybl]

humm, contrary to what I imagined, the metadata location works with Dexterity, even with the adapter with problem.

So I think it's okay to remove the adapter in Plone 5.2 and Plone 6.

[wesleybl]

I also tested it with Archetype, without the adapter and the metadata worked too. In other words, as we don't have the index in the catalog, the adapter is completely unnecessary, either with Dexterity or Archetype.

[ale-rt]

Good analysis! This will work with AT when you store the value as an attribute (attribute storage). IIRC correctly AT has other storage methods (metadata storage an annotation storage come to my mind but it has been years since I am not dealing with AT schemas). This is why you have this weird obj.getField(field_name).get(obj) syntax.

In short the adapter should go away to fix the location metadata for dexterity types. No problem for Plone 6, but in 5.2 we should keep it for ATs because you cannot say that the location will be stored as an attribute.

[jensens]

+1 for removal of dead code in Plone 6