aem-core-wcm-components icon indicating copy to clipboard operation
aem-core-wcm-components copied to clipboard

[List] Incorrect sorting of shadowed list items

Open ky940819 opened this issue 2 years ago • 0 comments

Bug Report

Current Behavior In V2 / V3 List components. when a list item is "shadowed" ( i.e. the list item is a redirect to a different page), the original page is used in sorting whereas the target page is presented. This causes issues with sorting by either title or date.

Expected behavior/code The list should be sorted based on the effective page (i.e the page that will be shown the user) and not the original page which is a redirect.

Environment

  • AEM 6.5.13.0
  • Core Components version 2.20.6
  • JRE 11

Possible Solution This occurs because the sorting is done in v1/ListImpl#getPages(), which is the raw list of pages. This worked fine in the V1 model because the LinkHandler had not yet been introduced.

Once the LinkHandler was introduces the PageListItemImpl was updated to change the effective page for the ListItem to be the page referenced by the link; which, in the case of a redirect page, is not the original page as returned by v1/ListImpl#getPages().

There are two fairly straight forward solutions to this:

  1. The cleanest solution is to pull v2/ListImpl#getListItems() up to the v1 model implementation, and apply sorting and max items to the ListItems and then derive the pages from the list items for backwards compatibility with the v1 model. Deriving the effective page from the list item is trivial because the list item always has a non-null page which only needs to be exposed internally (for PageListItemImpl anyway). This is clean because it only requires sorting and capping in one place. However, the downside to this solution is that it will also apply link shadowing by default to the V1 list component, which may be considered a breaking change?
  2. Another solution is to apply the sorting and capping in v1/ListImpl#getItems() (deprecated). Stop referencing this deprecated method in the V2 model, and apply a new ListItemComparator in v2/ListImpl#getListItems(). The downside to this is that there are two different comparators: the one for the old model, and one that applies to any ListItem for the new model.

In either case, there is benefit to comparing ListItems instead of Pages because it abstracts sorting of list items from the Page implementation and would allow sorting of ListItems that are not derived from pages - so long as those ListItems implement ListItem#getTitle() and ListItem#getLastModified().

ky940819 avatar Jul 20 '22 03:07 ky940819