Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Filter, Sort, and Limit methods on DataList change context within templates #9803

Closed
davejtoews opened this issue Dec 16, 2020 · 1 comment

Comments

@davejtoews
Copy link

davejtoews commented Dec 16, 2020

Affected Version

Silverstripe 4.6.1

(This issue came up in our Evans Hunt team Slack at least as early as Oct 2017, so it likely originated long before the current version.)

Description

When looping over a DataList within a template, making use of .Filter, .Sort, or .Limit methods will push the template context one level deeper per method use, making additional calls to Up within the loop necessary to reach the context outside the loop.

Expected behaviour

<h1>Children of '$Title'</h1>

<% loop $Children.Limit(2) %>
    <p>Page '$Title' is a child of '$Up.Title'</p>
<% end_loop %>

Given the following structure I would expect this output.

	My Page
	|
	+-+ Child 1
 	|
 	+-+ Child 2
 	|
 	+-+ Child 3

	Children of 'My Page'

	Page 'Child 1' is a child of 'My Page'
	Page 'Child 2' is a child of 'MyPage'

Actual behaviour

The code above will actually output the following

	Children of 'My Page'

	Page 'Child 1' is a child of ''
	Page 'Child 2' is a child of ''

If I throw an $Up.Me into the loop. I'll get this error:

[Emergency] Uncaught BadMethodCallException: Object->__call(): the method 'forTemplate' does not exist on 'SilverStripe\ORM\HasManyList'

The limit method apparently pushes the template context one level deeper, adding a level where the context is a list object itself. A list object is never an accessible context within a template without using Filter, Sort, or Limit so this is unexpected behaviour from my perspective.

Workaround

You can access the correct context by adding an extra Up

<h1>Children of '$Title'</h1>

<% loop $Children.Limit(2) %>
    <p>Page '$Title' is a child of '$Up.Up.Title'</p>
<% end_loop %>

Further examples

This nested context compounds itself if you use multiple modifiers to the DataList. You could find yourself doing the following.

<h1>Children of '$Title'</h1>

<% loop $Children.Limit(2).Sort('Title ASC') %>
    <p>Page '$Title' is a child of '$Up.Up.Up.Title'</p>
<% end_loop %>

Or go even deeper

<h1>Children of '$Title'</h1>

<% loop $Children.Limit(2).Sort('Title ASC').Filter('ImageID:GreaterThan', 0) %>
    <p>Page '$Title' is a child of '$Up.Up.Up.Up.Title'</p>
<% end_loop %>

Conclusion

This does not seem like it should be the intended behaviour. If the core team believes otherwise then I think this should be reflected in the docs.

I suspect the underlying cause is related to the following comments in the DataList and ArrayList source code respectively:

/**
 * DataLists are _immutable_ as far as the query they represent is concerned. When you call a method that
 * alters the query, a new DataList instance is returned, rather than modifying the existing instance

/**
 * Note that (like DataLists), the implementations of the methods from SS_Filterable, SS_Sortable and
 * SS_Limitable return a new instance of ArrayList, rather than modifying the existing instance.

If this is indeed a bug, the fix will be a breaking change as there are likely plenty of templates out there using an extra Up.

@kinglozzer
Copy link
Member

Thanks @davejtoews, this has been fixed for SS5 (#8148) and you’re right - unfortunately we can’t fix this in SS4 as it’d be a breaking change. We do have a small note about it in the docs, I think that’s about all we can do about this issue for now 😞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants