[16.0][ADD] website_portal_pager_limit#1201
Open
halbtonjazz wants to merge 5 commits into
Open
Conversation
Portal pages use a fixed page size hardcoded in controllers, which hurts usability when browsing large datasets. Let portal users pick the number of records per page via a selector next to the pager, applied through a whitelisted "limit" query parameter configurable in system parameters. Task: 0350
…ight Position the page size selector at the right edge of the pager row instead of inline after the pagination widget. Keeps the pager itself centered via the core justify-content-center wrapper while using absolute positioning for the selector. Adds a min-height guard so the row stays visible on single-page lists where the <ul> pager hides itself. Task: 0350
Cover controller logic with unit tests: options parsing fallbacks, whitelist validation on the _items_per_page property, portal layout context values, and template rendering guard for third-party callers. Add an HttpCase smoke test for the /my route with valid, invalid and missing limit values. Update readme/CONTRIBUTORS.rst to follow the XXP format: company entry with Project Manager and Developer roles, no email addresses. Task: 0350
OCA-git-bot
added
series:16.0
mod:website_portal_pager_limit
geomer198
reviewed
Jun 17, 2026
geomer198
left a comment
geomer198
left a comment
There was a problem hiding this comment.
Could you please check code review result.
| param = ( | ||
| request.env["ir.config_parameter"] | ||
| .sudo() | ||
| .get_param(OPTIONS_PARAM, default="10,20,40,80,100") |
There was a problem hiding this comment.
Maybe better use default value=False and then use DEFAULT_LIMIT_OPTIONS const without split methods?
param = request.env["ir.config_parameter].sudo().get_param(OPTIONS_PARAM, default=False)
if not param:
return DEFAULT_LIMIT_OPTIONS
return [int(x.strip()) for x in param.split(",") if x.strip().isdigit()] or DEFAULT_LIMIT_OPTIONS| _request_stack.pop() | ||
|
|
||
|
|
||
| class TestPagerLimitOptions(TestPortalPagerLimitCommon): |
There was a problem hiding this comment.
Please split this file.
Use for each test class separate file for tests.
Refactor the controller to handle empty pagination options by returning default values. Introduce new test cases for validating pagination limits, including handling of non-numeric and excessive values. Add common test utilities and ensure proper rendering of pagination templates. Task: 0350
geomer198
reviewed
Jun 18, 2026
| """ | ||
| param = request.env["ir.config_parameter"].sudo().get_param(OPTIONS_PARAM) | ||
| if not param: | ||
| return list(DEFAULT_LIMIT_OPTIONS) |
Author
There was a problem hiding this comment.
it was meant as a defensive copy against append, sort, etc. I just didn't notice the constant was already a list, not a tuple. Reworked the constant into a tuple, now list() actually makes sense (a fresh mutable list from an immutable constant)
| if not param: | ||
| return list(DEFAULT_LIMIT_OPTIONS) | ||
| options = [int(x.strip()) for x in param.split(",") if x.strip().isdigit()] | ||
| return options or list(DEFAULT_LIMIT_OPTIONS) |
|
|
||
| def _prepare_portal_layout_values(self): | ||
| values = super()._prepare_portal_layout_values() | ||
| values.update( |
There was a problem hiding this comment.
Please add comment this with describe about new functional.
| # honors the ``limit`` query parameter without overriding each route. | ||
| @property | ||
| def _items_per_page(self): | ||
| limit = request.httprequest.args.get("limit", "") |
Make DEFAULT_LIMIT_OPTIONS a tuple so the list() copies guard the module-level default against mutation. Add docstrings to the controller methods and replace the layout-values comment with a docstring. Task: 0350
Contributor
|
This PR has the |
|
@pedrobaeza Hello, could you please take a look? Thank you. |
Member
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
New module that adds a configurable page size selector to the portal
pager. Portal users can pick how many records are shown per page
(10/20/40/80/100 by default) via a dropdown next to the pagination
controls. The choice is applied through a
limitquery parameter andkept while paginating, sorting, filtering and searching.
Allowed values are whitelisted through the
website_portal_pager_limit.optionssystem parameter; any invalid orout-of-list value falls back to the standard portal page size (80), so a
misconfiguration can never break pagination.
How it works
_items_per_pageas a property reading thevalidated
limit, so every/my/*route benefits without overridingeach route.
portal.pageris inherited to render the selector (kept visible evenon single-page lists).
Task: 0350