[MAGNOLIA-6213] Standardize TemplatingFunctions Created: 20/May/15  Updated: 12/Apr/17  Resolved: 22/Jun/15

Status: Closed
Project: Magnolia
Component/s: templating
Affects Version/s: None
Fix Version/s: 5.4

Type: Improvement Priority: Neutral
Reporter: Christopher Zimmermann Assignee: Christopher Zimmermann
Resolution: Fixed Votes: 0
Labels: templating-refactor
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: PNG File javadoc-inherit-after.png     PNG File javadoc-inherit-before.png    
Issue Links:
Relates
relates to MAGNOLIA-6230 Reorder methods in TemplatingFunctions Closed
dependency
is depended upon by MGNLCAT-158 Update uses of renamed TemplatingFunc... Closed
is depended upon by MGNLRES-157 Update uses of renamed TemplatingFunc... Closed
is depended upon by MGNLRSSAGG-200 Update uses of renamed TemplatingFunc... Closed
is depended upon by MGNLSTK-1483 Update uses of renamed TemplatingFunc... Closed
is depended upon by MGNLDEMO-47 Update uses of renamed TemplatingFunc... Closed
is depended upon by MGNLSITE-21 Update uses of renamed TemplatingFunc... Closed
is depended upon by MTE-30 Update uses of renamed TemplatingFunc... Closed
relation
is related to MAGNOLIA-7007 Counterpart methods accepting content... Closed
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Date of First Response:

 Description   

TemplatingFunctions should be updated to adhere to the new standard:
https://wiki.magnolia-cms.com/display/PDNOTES/TemplatingFunctions+Style+Guide

For 5.4, in this ticket - all new functions should have names standardized, work with both nodes & contentMaps, and have good comments.

Older functions deprecated and new versions created:

getQueryStringAndFragment

New functions in 5.4 renamed:

getLinkPrefix
getReferencedContent
findParentWithTemplateType
getTemplateType
getTemplateSubType
getContentListByTemplateTypeAndSubType
getContentListByTemplateId
getContentListByTemplateIds
getFileExtension
getDisplayFileSize

Additionally

TemplateTypeHelper#getContentListByTemplateTypeAndSubType

method is renamed to be in synch with the version in TemplatingFunctions:
"andSubType" is removed from the name.



 Comments   
Comment by Philip Mundt [ 29/May/15 ]

Doesn't compile due to checkstyle (Missing dots):

  • Preferred solution, change
    /**
     * See {@link #root(Node, String)}
     */
    

    to

    /**
     * @see #root(Node, String)
     */
    

    for lines:

    • info/magnolia/templating/functions/TemplatingFunctions.java:165
    • info/magnolia/templating/functions/TemplatingFunctions.java:158
    • info/magnolia/templating/functions/TemplatingFunctions.java:200
    • info/magnolia/templating/functions/TemplatingFunctions.java:207
    • info/magnolia/templating/functions/TemplatingFunctions.java:273
    • info/magnolia/templating/functions/TemplatingFunctions.java:280
    • info/magnolia/templating/functions/TemplatingFunctions.java:399
    • info/magnolia/templating/functions/TemplatingFunctions.java:406
    • info/magnolia/templating/functions/TemplatingFunctions.java:417
    • info/magnolia/templating/functions/TemplatingFunctions.java:455
    • info/magnolia/templating/functions/TemplatingFunctions.java:506
    • info/magnolia/templating/functions/TemplatingFunctions.java:521
  • No dot missing, adjust to above Javadoc anyway:
    • info/magnolia/templating/functions/TemplatingFunctions.java:572
    • info/magnolia/templating/functions/TemplatingFunctions.java:593
    • info/magnolia/templating/functions/TemplatingFunctions.java:894
  • Typo: specifc -> specific
    • info/magnolia/templating/functions/TemplatingFunctions.java:137
    • info/magnolia/templating/functions/TemplatingFunctions.java:151
  • info/magnolia/templating/functions/TemplatingFunctions.java:1061 and info/magnolia/templating/functions/TemplatingFunctions.java:1077
    • unresolvable class (and link in javadoc), because it's in another module where we can't "link" to. In this case I would go from:
      @deprecated since 5.4 - use {@link info.magnolia.templating.functions.SearchTemplatingFunctions#searchContent(String, String, String, String, long, long) instead.
      

      (btw: closing bracket is missing)
      to

      @deprecated since 5.4 - use <code>SearchTemplatingFunctions#searchContent(String, String, String, String, long, long)</code> from <code>info.magnolia.templating:magnolia-templating-essentials-models</code> instead.
      
  • Method info.magnolia.templating.functions.TemplatingFunctions#contentListByTemplateType(javax.jcr.Node, java.lang.String, java.lang.String, int, java.lang.String, java.lang.String)
    • Two parameters (@param) wrongly named
      • searchRoot
      • orderByClause
      • Maybe adjust parameters of functions instead of changing javadoc
  • Unresolvable link at info/magnolia/templating/functions/TemplatingFunctions.java:1257
  • Method info.magnolia.templating.functions.TemplatingFunctions#contentListByTemplateId(javax.jcr.Node, java.lang.String, int, java.lang.String, java.lang.String)
    • line
      * @param templateIds a {@link java.util.Set} of template IDs to search for

      is wrong

  • Method info.magnolia.templating.functions.TemplatingFunctions#getQueryStringAndFragment
    • Why do you duplicate the code instead of delegating to the new one?
  • For long javadoc make sure to have a short and precise description in the first sentence. Following parts of the javadoc should be warpped in a <p></p> in order to achieve a nicer layout. Wrapping at 160 characters also makes for a nicer reading experience (and better looks).
  • Single
    See {@link info.magnolia.rendering.template.configured.ConfiguredInheritance}

    change to

  • @see info.magnolia.rendering.template.configured.ConfiguredInheritance

    at the end of javadoc

/**
 * Wraps the provided node with an inheritance wrapper so that the node appears to
 * contain the components and/or properties that exist in the equivalent nodes of the
 * ancestors of the provided node's "anchor".
 * (The anchor is the first parent of type mgnl:content - typically the page node of the provided node).
 * The inheritance is subject to the inheritance configuration of the node, but can be overridden with the parameters of this method.
 * (See {@link info.magnolia.rendering.template.configured.ConfiguredInheritance}.)
 * For more details, see {@link info.magnolia.templating.inheritance.DefaultInheritanceContentDecorator} Note: The inherit methods are seldom necessary because areas already support inheritance natively through configuration.
 * 
 * @param relPath If not blank, the node will inherit the node specified by this relative path rather than the actual provided node.
 * This could be used to grab a specific property from a subnode.
 * @param nodeTypes If not blank, only ancestors of the anchor with one of the nodeTypes in this comma delimited string will be evaluated.
 * @param nodeInheritance Specify which child components of the ancestors to inherit.
 * Accepts [ConfiguredInheritance.COMPONENTS_ALL | ConfiguredInheritance.COMPONENTS_NONE | ConfiguredInheritance.COMPONENTS_FILTERED]
 * "all": Inherit all components. "filtered": Inherit only components with property inheritable=true. "none": Do not inherit components.
 * @param propertyInheritanct Specify which properties of the ancestors to inherit.
 * Accepts [ConfiguredInheritance.PROPERTIES_ALL | ConfiguredInheritance.PROPERTIES_NONE]
 * "all": Inherit all properties of ancestors, if multiple ancestors provide the same property - that from the nearest ancestor is used.
 * "none": Do not inherit properties.
 */

See: javadoc-inherit-before.png
Becomes

/**
 * Wraps the provided node with an inheritance wrapper so that the node appears to contain the components and/or
 * properties that exist in the equivalent nodes of the ancestors of the provided node's "anchor" (The anchor is
 * the first parent of type mgnl:content - typically the page node of the provided node).
 *
 * <p>The inheritance is subject to the inheritance configuration of the node, but can be overridden with the
 * parameters of this method.</p>
 * <p><b>Note</b>: The inherit methods are seldom necessary because areas already support inheritance natively
 * through configuration.</p>
 *
 * <p>Node inheritance accepts:
 * <dl><dt>{@link ConfiguredInheritance#COMPONENTS_ALL}</dt><dd>Inherit all components</dd>
 * <dt>{@link ConfiguredInheritance#COMPONENTS_FILTERED}</dt><dd>Inherit only components with property
 * <code>inheritable=true</code></dd>
 * <dt>{@link ConfiguredInheritance#COMPONENTS_NONE}</dt><dd>Do not inherit components</dd></dl></p>
 *
 * <p>Property inheritance accepts:
 * <dl><dt>{@link ConfiguredInheritance#PROPERTIES_ALL}</dt><dd>Inherit all properties of ancestors, if multiple
 * ancestors provide the same property the one from the nearest ancestor is used</dd>
 * <dt>{@link ConfiguredInheritance#PROPERTIES_NONE}</dt><dd>Do not inherit properties</dd></dl></p>
 *
 * @param relPath If not blank, the node will inherit the node specified by this relative path rather than the
 * actual provided node. This can be used to grab a specific property from a subnode.
 * @param nodeTypes If not blank, only ancestors of the anchor with one of the nodeTypes in this comma delimited
 * string will be evaluated.
 * @param nodeInheritance Specify which child components of the ancestors to inherit.
 * @param propertyInheritance Specify which properties of the ancestors to inherit.
 *
 * @see info.magnolia.templating.inheritance.DefaultInheritanceContentDecorator
 * @see info.magnolia.rendering.template.configured.ConfiguredInheritance
 */

See: javadoc-inherit-after.png (Once this is deployed as a site, I believe the linking to ConfiguredInheritance should also work)
Committed suggested change to branch

Comment by Philip Mundt [ 29/May/15 ]

Missing adjustments due to refactoring of method name getLinkPrefix():

  • site
  • multisite
Comment by Christopher Zimmermann [ 22/Jun/15 ]

Cleaned up javadoc and changed parameter names on a few functions.

Decided to stick with the See syntax for many functions because it is the most important part of the javadoc and I felt it was more helpful to have it be emphasized and be the first thing one rather than down in the "see also" section. Added " for details." to make the statement a sentance: "See

{link}

for details."

Comment by Christopher Zimmermann [ 23/Jun/15 ]

Created and linked tickets for Site and Multisite

Comment by Espen Jervidalo [ 24/Jun/15 ]

pushed a squashed and cleaned up version to MAGNOLIA-6213-review. Use it for integration! The other branch is broken

Generated at Mon Feb 12 04:12:22 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.