pagination not working with SingleTableMixin

[fkdeboer]

Hi,
in v1.2.3 my tables aren't paginated anymore with the SingleTableMixin.
A default like table_pagination = 20 should be added.

[jieter]

Hi @fkdeboer, thanks for reporting.

To clarify: are you reporting it doesn't work, or that it doesn't work by default?

[fkdeboer]

Hi Jieter,
that's actually a good question:)
Where until version 1.2.2 all tables where paginated (without specification), in the current version all tables have no pagination anymore.
I see that table_pagination = None is removed in class SingleTableMixin(object)
which does change the behaviour, see:

def get_table_pagination(self, table):
        """
        Returns pagination options: True for standard pagination (default),
        False for no pagination, and a dictionary for custom pagination.
        """
        return self.table_pagination

[utapyngo]

It does not work even if Table.Meta.per_page is set. Worked in 1.2.2.
I had to use View.paginate_by instead to make pagination work in 1.2.3.

[graup]

I think I have found the bug (regression).

RequestConfig.paginate defaults to True, which enabled calling Table.paginate with per_page=None, which in turn uses TableOptions.per_page which defaults to 25.

Now, with the rewrite of the Mixins, RequestConfig always gets passed paginate=self.get_table_pagination(table) excplicitly, which defaults to None, which disables pagination altogether.

While in the old code, it would pass **options which would be an empty dict by default, leaving RequestConfig.paginate at its default value True, enabling default pagination.

[prydie]

The change in default behavior (from paginating by default to no pagination by default) should probably be documented in the changelog.

[fkdeboer]

But is this desirable behavior? under the new default we encountered a gateway timeout due to tables2 trying gettings thousands of records from the DB one by one.

[prydie]

@fkdeboer We caught the bug prior to updating our production environment but the same would have occurred in our case too had we not done so. According to SEMVER this kind of change should warrant a major version bump (certainly not a patch version bump).

[jieter]

Sorry guys, this is certainly not intended, and should be reverted. Furthermore, these kinds of behaviours should be covered by tests. @graup, seems like you dove in, are you probably willing to make a pull request?

[graup]

@jieter yes, I can do it in a few days. I would add a regression test confirming the default and then fix the code that I mentioned, ok?

[jieter]

@graup, thanks, looking forward to it.

[jieter]

released 1.2.4.