[MGNLSTK-677] Exception in STKPager::getPageItem() Created: 19/Aug/10 Updated: 11/Feb/11 Resolved: 08/Dec/10 |
|
| Status: | Closed |
| Project: | Magnolia Standard Templating Kit (closed) |
| Component/s: | paragraphs |
| Affects Version/s: | 1.3.4 |
| Fix Version/s: | 1.3.6, 1.4.2 |
| Type: | Bug | Priority: | Trivial |
| Reporter: | Stefan Baur | Assignee: | Ondrej Chytil |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | 0.25h | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | 0.25h | ||
| Attachments: |
|
||||||||
| Issue Links: |
|
||||||||
| Template: |
|
||||||||
| Acceptance criteria: |
Empty
|
||||||||
| Date of First Response: | |||||||||
| Description |
|
the following line generates an exception if offset is larger than limit. FIX: public Collection getPageItems() { Collection subList = items; if(offset < limit) { subList = ((List)items).subList(offset, limit); }else { subList = items; } } This error happens if the GET Parameter currentPage is set (for example from an old session or an old link), but there are not enought items in the current Collection. |
| Comments |
| Comment by Bert Leunis [ 27/Sep/10 ] |
|
I am not convinced that the suggested solution is the best one. The problem with the larger offset occurs when in an overview a paged page is requested that does not exist. So if you have 5 paged pages, and you request page 6 the calculation here goes wrong. (Example: http://tegenlicht.vpro.nl/nieuws/?currentPage=1000). The fix suggests that now the complete list is returned. That is fine if you have only 30 articles, but not if you have a much larger number. There's several other options: return 404, return the last paged page, return 'no results for this page'. |
| Comment by Stefan Baur [ 27/Sep/10 ] |
|
yes, you are right. |
| Comment by Bert Leunis [ 28/Sep/10 ] |
|
IMHO the last page would be the right choice. I'll see if I can write a small patch for that. |
| Comment by Bert Leunis [ 30/Sep/10 ] |
|
Voilà, a small patch. When the offset of the requested page is beyond the actual number of items, the offset is set to that of the last page of items. Also added some code to prevent unfortunate DivideByZero errors. |
| Comment by Stefan Baur [ 01/Oct/10 ] |
|
nice. will this update be included in further releases? |
| Comment by Philipp Bärfuss [ 14/Oct/10 ] |
|
Yes. Added it to the maintenance mindmap. |
| Comment by Ondrej Chytil [ 08/Dec/10 ] |
|
Thanks for the patch Bert. |