[TAP5-1689] Grid pagination details should be overridable via a Grid parameter to support Grids that render in loops

• Type: Bug
• Status: Closed
• Priority: Minor
• Resolution: Fixed
• Affects Version/s: 5.2
• Fix Version/s: 5.4
• Component/s: tapestry-core
• Labels: bulk-close-candidate
• Assignee: Howard Lewis Ship
• Reporter: Wulf Wechsung
• Created: 11/Oct/11 15:21
• Updated: 06/Jan/15 09:04
• Resolved: 27/Aug/14 18:52

Description

Intro:
The T5 grid components stores the column that is currently being sorted in the @Persist field sortColumnId. This works perfectly fine in the usual case where the Grid always gets the same beanmodel for a given page.

Bug:
On a page that renders a Grid with a dynamic beanmodel ( for example a generic List page that decides from it's activation context what kind of list to display) the Grid will throw and exception in the following circumstances:

1) Page activation with parameter value A leads to a beanmodel with property name A
2) Grid is sorted on property name A
3) Page activation with parameter value B that leads to a beanmodel with property name B

-> Grid will throw an exception on SortModel.getSortConstraints() because property name A (which is still stored in sortColumnId) does not exist in the current beanmodel.

Solution:

in SortModel.getSortConstraints the following lines will solve the problem:

BeanModel dataModel = getDataModel();
if( !dataModel.getPropertyNames().contains(sortColumnId)) {
return Collections.emptyList();
}

General notes:

• many of the methods in BeanModel throw exception when a requested property doesn't exist. While that's probably appropriate for most cases it would be nice if all the beanmodel would support the if( exists) {} else{} pattern for all it's methods that yield PropertyModels. For example, in the above code I'm not sure if it's completely correct to check the columnId against a list of names because the beanmodel does make a distinction but doesn't provide a getPropertyIds() method.

• more generally, I feel that there is a small gap in the attribute persist model of tapestry. Whenever a component needs to keep some state over requests (such as AJAX requests) the methods of keeping that state all seem to have drawbacks. @Persist will do the job of course but it doesn't (so it seems to me) have good semantics for initializing, cleaning up and associating that state. Ideally there would be some sort of abstraction that automatically maps field persistence to how page requests flow for the user. For example, when a user sees a partial page response or refreshes a page, it's still the same page for him. I guess something that's tied to the page activation context and is cleared once the same user requests a different page or context would do.

Issue Links

• breaks: TAP5-2437 NullPointerException when trying to access (read or write) Grid's currentPage