Conversation
* Refactor get_ip_rules into type specific functions with optional pagination * Introduce pagination parameters page and per_page for Flowspec4, Flowspec6, RTBH, and Whitelist queries * Update dashboard view to handle pagination and per page selection * Add pagination controls to dashboard templates * Preserve backward compatibility for non paginated queries
Bump version to 1.1.8b1 Extract dashboard search form into shared template Implement unified search across all rule types with result counters Add in-memory pagination for search results via paginate_list() Introduce /clear-search route to reset stored queries Include search_helpers.js and enhance UI feedback for active searches Simplify layout templates and refactor redundant search form code
There was a problem hiding this comment.
Pull Request Overview
This PR implements version 1.1.8 with enhancements to dashboard pagination, search functionality, and user interface improvements. The main focus is on adding pagination support for "Expired" and "All" dashboard states, improving search capabilities with quick clear functionality, and fixing group operations.
- Added comprehensive pagination system with database-level and in-memory pagination for search results
- Enhanced search functionality with quick clear buttons and ESC key support
- Refactored template structure to use shared search form components and pagination macros
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Removed email addresses from some authors |
| flowapp/views/dashboard.py | Major pagination implementation with enhanced search handling and new clear_search endpoint |
| flowapp/utils/app_factory.py | Added locale parameter to datetime formatting |
| flowapp/templates/pagination_macro.html | New pagination component with Bootstrap styling and per-page options |
| flowapp/templates/pages/submenu_dashboard_view.html | Refactored to use shared search form component |
| flowapp/templates/pages/submenu_dashboard.html | Added search result indicators and shared search form |
| flowapp/templates/pages/dashboard_view.html | Integrated pagination macro |
| flowapp/templates/pages/dashboard_search_form.html | New shared search form component with clear functionality |
| flowapp/templates/pages/dashboard_admin.html | Added pagination support |
| flowapp/templates/layouts/default.html | Added search helper JavaScript |
| flowapp/static/js/search_helpers.js | New JavaScript for search clear functionality |
| flowapp/static/js/check_all.js | New JavaScript for checkbox toggle functionality |
| flowapp/models/utils.py | Refactored rule retrieval with pagination support |
| flowapp/about.py | Version bump to 1.1.8 |
| config.example.py | Added Babel locale configuration |
| CHANGELOG.md | Updated with version 1.1.8 changes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
flowapp/views/dashboard.py
Outdated
There was a problem hiding this comment.
Corrected comment header format - should use single # for consistency with Python conventions.
| ## Helper functions | |
| # Helper functions |
flowapp/views/dashboard.py
Outdated
There was a problem hiding this comment.
Using an empty class with dynamic attribute assignment is not a best practice. Consider using a dataclass or NamedTuple for better type safety and clarity.
flowapp/views/dashboard.py
Outdated
There was a problem hiding this comment.
[nitpick] The variable name 'c' is ambiguous in this context. Consider renaming to 'value' for better readability.
| full_text = " ".join(str(c) for c in rule.values()) | |
| full_text = " ".join(str(value) for value in rule.values()) |
| @@ -0,0 +1,96 @@ | |||
| {# | |||
| Pagination macro for dashboard tables | |||
| Usage: {{ render_pagination(pagination, rtype, rstate, sort_key, sort_order, search_query, per_page, per_page_options) }} | |||
There was a problem hiding this comment.
The macro documentation should include parameter descriptions for better maintainability, especially for the complex parameter list.
| Usage: {{ render_pagination(pagination, rtype, rstate, sort_key, sort_order, search_query, per_page, per_page_options) }} | |
| Usage: {{ render_pagination(pagination, rtype, rstate, sort_key, sort_order, search_query, per_page, per_page_options) }} | |
| Parameters: | |
| pagination - Pagination object containing page info (required) | |
| rtype - Rule type filter (string, required) | |
| rstate - Rule state filter (string, required) | |
| sort_key - Key to sort by (string, required) | |
| sort_order - Sort order, e.g. 'asc' or 'desc' (string, required) | |
| search_query - Search query string (string, optional, default: '') | |
| per_page - Number of items per page (integer, optional, default: 50) | |
| per_page_options - List of selectable items per page (list of integers, optional, default: [25, 50, 100, 200]) |
No description provided.