#732: easy fix, but lossing pages overlap

misago/threads/paginator.py

@@ -1,4 +1,4 @@
-from math import ceil
+from math import floor, ceil
 
 from django.core.paginator import Paginator
 from django.utils.functional import cached_property
@@ -9,23 +9,13 @@ class PostsPaginator(Paginator):
     Paginator that returns that makes last item on page
     repeat as first item on next page.
     """
-    @cached_property
-    def num_pages(self):
-        """
-        Returns the total number of pages.
-        """
-        if self.count == 0 and not self.allow_empty_first_page:
-            return 0
-        hits = max(1, self.count - self.orphans)
-        return int(ceil(hits / float(self.per_page - 1)))
-
     def page(self, number):
         """
         Returns a Page object for the given 1-based page number.
         """
         number = self.validate_number(number)
-        bottom = (number - 1) * (self.per_page - 1)
-        top = bottom + self.per_page
+        bottom = (number - 1) * self.per_page
+        top = bottom + self.per_page + 1
         if top + self.orphans >= self.count:
             top = self.count
         return self._get_page(self.object_list[bottom:top], number, self)

misago/threads/tests/test_paginator.py

@@ -9,54 +9,59 @@ class PostsPaginatorTests(TestCase):
         items = [i + 1 for i in range(30)]
 
         paginator = PostsPaginator(items, 5)
-        self.assertEqual(paginator.num_pages, 8)
-
-        self.assertEqual(paginator.page(1).object_list, [1, 2, 3, 4, 5])
-        self.assertEqual(paginator.page(2).object_list, [5, 6, 7, 8, 9])
-        self.assertEqual(paginator.page(3).object_list, [9, 10, 11, 12, 13])
-        self.assertEqual(paginator.page(4).object_list, [13, 14, 15, 16, 17])
-        self.assertEqual(paginator.page(5).object_list, [17, 18, 19, 20, 21])
-        self.assertEqual(paginator.page(6).object_list, [21, 22, 23, 24, 25])
-        self.assertEqual(paginator.page(7).object_list, [25, 26, 27, 28, 29])
-        self.assertEqual(paginator.page(8).object_list, [29, 30])
+        self.assertEqual(self.get_paginator_items_list(paginator), [
+            [1, 2, 3, 4, 5, 6],
+            [6, 7, 8, 9, 10, 11],
+            [11, 12, 13, 14, 15, 16],
+            [16, 17, 18, 19, 20, 21],
+            [21, 22, 23, 24, 25, 26],
+            [26, 27, 28, 29, 30],
+        ])
 
     def test_paginator_orphans(self):
         """paginator handles orphans"""
         items = [i + 1 for i in range(20)]
 
         paginator = PostsPaginator(items, 8, 6)
-        self.assertEqual(paginator.num_pages, 2)
+        self.assertEqual(self.get_paginator_items_list(paginator), [
+            [1, 2, 3, 4, 5, 6, 7, 8, 9],
+            [9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20],
+        ])
 
-        self.assertEqual(
-            paginator.page(1).object_list, [1, 2, 3, 4, 5, 6, 7, 8])
-        self.assertEqual(
-            paginator.page(2).object_list,
-            [8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20])
+        paginator = PostsPaginator(items, 8, 5)
+        self.assertEqual(paginator.num_pages, 2)
+        self.assertEqual(self.get_paginator_items_list(paginator), [
+            [1, 2, 3, 4, 5, 6, 7, 8, 9],
+            [9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20],
+        ])
 
-        paginator = PostsPaginator(items, 8, 4)
-        self.assertEqual(paginator.num_pages, 3)
+        paginator = PostsPaginator(items, 9, 3)
+        self.assertEqual(paginator.num_pages, 2)
+        self.assertEqual(self.get_paginator_items_list(paginator), [
+            [1, 2, 3, 4, 5, 6, 7, 8, 9, 10],
+            [10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20],
+        ])
 
-        self.assertEqual(
-            paginator.page(1).object_list, [1, 2, 3, 4, 5, 6, 7, 8])
-        self.assertEqual(
-            paginator.page(2).object_list, [8, 9, 10, 11, 12, 13, 14, 15])
-        self.assertEqual(
-            paginator.page(3).object_list, [15, 16, 17, 18, 19, 20])
+        # regression test for #732
+        items = [i + 1 for i in range(24)]
 
-        paginator = PostsPaginator(items, 8, 6)
-        self.assertEqual(paginator.num_pages, 2)
+        paginator = PostsPaginator(items, 18, 6)
+        self.assertEqual(paginator.num_pages, 1)
+        self.assertEqual(self.get_paginator_items_list(paginator), [
+            [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24],
+        ])
 
-        self.assertEqual(
-            paginator.page(1).object_list, [1, 2, 3, 4, 5, 6, 7, 8])
-        self.assertEqual(
-            paginator.page(2).object_list,
-            [8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20])
+        # extra tests for catching issues in excessively long datasets
+        paginator = PostsPaginator([i + 1 for i in range(144)], 14, 6)
+        last_page = self.get_paginator_items_list(paginator)[-1]
+        self.assertEqual(last_page[-4:], [141, 142, 143, 144])
 
-        paginator = PostsPaginator(items, 6, 5)
-        self.assertEqual(paginator.num_pages, 3)
+        paginator = PostsPaginator([i + 1 for i in range(321)], 14, 6)
+        last_page = self.get_paginator_items_list(paginator)[-1]
+        self.assertEqual(last_page[-4:], [318, 319, 320, 321])
 
-        self.assertEqual(paginator.page(1).object_list, [1, 2, 3, 4, 5, 6])
-        self.assertEqual(paginator.page(2).object_list, [6, 7, 8, 9, 10, 11])
-        self.assertEqual(
-            paginator.page(3).object_list,
-            [11, 12, 13, 14, 15, 16, 17, 18, 19, 20])
+    def get_paginator_items_list(self, paginator):
+        items_list = []
+        for page in paginator.page_range:
+            items_list.append(paginator.page(page).object_list)
+        return items_list

misago/threads/viewmodels/posts.py

@@ -30,6 +30,10 @@ class ViewModel(object):
         posts = list(list_page.object_list)
         posters = []
 
+        next_page_post = None
+        if list_page.has_next():
+            next_page_post, posts = posts[-1], next_page_post[:-1]
+
         for post in posts:
             post.category = thread.category
             post.thread = thread_model
@@ -44,16 +48,9 @@ class ViewModel(object):
 
         # add events to posts
         if thread_model.has_events:
-            first_post = None
-            if list_page.has_previous():
-                first_post = posts[0]
-            last_post = None
-            if list_page.has_next():
-                last_post = posts[-1]
-
             events_limit = settings.MISAGO_EVENTS_PER_PAGE
             posts += self.get_events_queryset(
-                request, thread_model, events_limit, first_post, last_post)
+                request, thread_model, events_limit, posts[0], next_page_post)
 
             # sort both by pk
             posts.sort(key=lambda p: p.pk)