Commit 6ad56d89 authored by Jérome Perrin's avatar Jérome Perrin

core: rename misnamed Method_viewFilter

there is no "Method", but there is a property sheet "CatalogFilter"
parent 7fdfac56
......@@ -79,7 +79,7 @@
<dictionary>
<item>
<key> <string>text</string> </key>
<value> <string>string:${object_url}/Method_viewFilter</string> </value>
<value> <string>string:${object_url}/CatalogFilter_viewFilter</string> </value>
</item>
</dictionary>
</pickle>
......
......@@ -77,7 +77,7 @@
<dictionary>
<item>
<key> <string>text</string> </key>
<value> <string>string:${object_url}/Method_viewFilter</string> </value>
<value> <string>string:${object_url}/CatalogFilter_viewFilter</string> </value>
</item>
</dictionary>
</pickle>
......
......@@ -90,7 +90,7 @@
</item>
<item>
<key> <string>id</string> </key>
<value> <string>Method_viewFilter</string> </value>
<value> <string>CatalogFilter_viewFilter</string> </value>
</item>
<item>
<key> <string>method</string> </key>
......@@ -98,7 +98,7 @@
</item>
<item>
<key> <string>name</string> </key>
<value> <string>CatalogTool_viewFilter</string> </value>
<value> <string>CatalogFilter_viewFilter</string> </value>
</item>
<item>
<key> <string>pt</string> </key>
......
  • I disagree. The first part (before _) should describe what the object (on which the callable is used) is and not what is has. Usually, the portal type, class or subclass. Either the property sheet is badly named or property sheets shall not count is the list of possible subclasses.

    Here, the view applies on something on which catalog filters are defined. And not on something that is a catalog filter.

    Maybe Method does not exist but "Method" describes better the kind of object on which the view applies. Better that rather than something confusing.

    Apart from that, there is a redundancy in CatalogFilter_viewFilter: assuming that choosing a property sheet name is ok, the right part should simply be view if there's only a single view that is specific to the property sheet.

  • mentioned in commit fd4880ce

    Toggle commit list
  • The spec is https://www.erp5.com/documentation/developer/guideline/programming/erp5-Guideline.Programming.Naming.Conventions/#method-name-is-prefixed-with-portal-or-meta-type,-class-or-interface , which comes from wiki.erp5.org (where it's maybe easier to read).

    In the PortalType_methodName, PortalType can be:

    • a portal type name
    • a meta type name, without space
    • a class name ( eg. Movement_lookupPrice )
    • an interface name

    Initially, there was nothing about property sheets, but with portal type as classes, we have classes for property sheets. For this reason, _getTypeBasedMethod also accepts property sheets as prefix (see also 5b647c4a ).

    I still feel it's good to include property sheets in the list of bases and don't really see a difference between is and has in this context.

    Another possibility would be to make an interface, which could also be used in CatalogTool, instead of checking catalog methods by meta types or portal types, we could maybe check if they implement the interface.

    What do other think ?

    It would be interesting to know how many cases we have of skins using property sheet as prefix. I guess not so many, but that's probably not the only case.

Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment