Version 1.1.8. #72
Merged
Version 1.1.8. #72
Conversation
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
* 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
Contributor
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
| return redirect(url_for("dashboard.index", rtype=rtype, rstate=rstate, sort=sort_key, order=sort_order)) | ||
|
|
||
|
|
||
| ## Helper functions |
There was a problem hiding this comment.
Corrected comment header format - should use single # for consistency with Python conventions.
Suggested change
| ## Helper functions | |
| # Helper functions |
flowapp/views/dashboard.py
Outdated
Comment on lines
247
to
248
| class Pagination: | ||
| pass |
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
| full_text = " ".join("{}".format(c) for c in rule.values()) | ||
| if get_search_query.lower() in full_text.lower(): | ||
| # Create a full text string from all values | ||
| full_text = " ".join(str(c) for c in rule.values()) |
There was a problem hiding this comment.
[nitpick] The variable name 'c' is ambiguous in this context. Consider renaming to 'value' for better readability.
Suggested change
| 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.
Suggested change
| 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]) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.