fix: Data browser pagination is ignored when using browser navigation or page reload #3097
Conversation
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. |
📝 WalkthroughWalkthroughThis PR adds URL-driven pagination support to the Browser component. It introduces utilities to extract pagination parameters from query strings and update URLs while preserving filter state. Data fetching and navigation flows are adjusted to read and propagate pagination parameters. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Browser as Browser Component
participant URL as App Router/URL
participant DataFetch as Data Fetch
User->>Browser: Interact with pagination (next/prev page)
Browser->>Browser: updateURL(newSkip, newLimit)<br/>Preserve current filters
Browser->>URL: Push new URL<br/>(skip, limit, filters)
Browser->>Browser: prefetchData() triggered<br/>via componentDidMount/update
Browser->>Browser: extractPaginationFromQuery()<br/>Read skip/limit from URL
Browser->>DataFetch: Fetch data with new skip/limit
DataFetch-->>Browser: Return paginated data
Browser->>Browser: Update state & render<br/>with new page
Browser-->>User: Display new page
User->>Browser: Change filter/class
Browser->>Browser: updateURL(skip=0, currentLimit)<br/>Reset to first page
Browser->>URL: Push new URL<br/>(skip=0, filters changed)
Browser->>DataFetch: Fetch fresh data
DataFetch-->>Browser: Return data for new filter
Browser-->>User: Display results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
mtrezza
changed the title
mtrezza
changed the title
# [8.2.0-alpha.15](8.2.0-alpha.14...8.2.0-alpha.15) (2025-12-28) ### Bug Fixes * Data browser pagination is ignored when using browser navigation or page reload ([#3097](#3097)) ([bcc4d5f](bcc4d5f))
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/dashboard/Data/Browser/Browser.react.js (1)
2728-2746: Potential double data fetch when pagination changes.The current implementation triggers two data fetches when pagination changes:
updateOrdering(this.state.ordering)fetches data immediately (Line 2733, 2741)updateURL()callsnavigate(), which triggerscomponentWillReceiveProps→prefetchData→ another data fetchBoth fetches use the same parameters and retrieve identical data, making the second fetch redundant. This wastes resources and could cause race conditions or UI flickering if the responses arrive in an unexpected order.
Suggested approach: Remove the
updateOrderingcall and rely solely on the navigation flow. The immediatesetStateprovides UI responsiveness, whileprefetchData(triggered by URL change) handles the data fetch:🔎 Proposed refactor
setSkip={skip => { this.setState({ skip }, () => { this.updateURL(skip, null); - this.updateOrdering(this.state.ordering); }); }}setLimit={limit => { this.setState({ limit, skip: 0 }, () => { this.updateURL(0, limit); - this.updateOrdering(this.state.ordering); }); }}If immediate data fetching is required for UX (to avoid waiting for async navigation), consider adding a flag to prevent the second fetch when URL changes are triggered programmatically.
🧹 Nitpick comments (2)
src/dashboard/Data/Browser/Browser.react.js (2)
1423-1426: Consider removing redundant state update.The
setState({ skip: 0 })after navigation is redundant. Thenavigatecall triggerscomponentWillReceiveProps, which callsprefetchData, which readsskipfrom the URL (defaulting to 0) and updates state accordingly.🔎 Proposed refactor
// filters param change is making the fetch call this.props.navigate(generatePath(this.context, url)); } - -this.setState({ - skip: 0, -});
1399-1421: Consider refactoring URL building logic to reduce duplication.The URL construction logic here is similar to
updateURL(lines 659-696). If the URL building logic changes (e.g., adding new parameters to preserve), these two locations would need to be updated separately, risking inconsistency.Consider extracting the URL building logic into a shared helper method or using
updateURLdirectly where feasible.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/dashboard/Data/Browser/Browser.react.js
🧰 Additional context used
🧬 Code graph analysis (1)
src/dashboard/Data/Browser/Browser.react.js (1)
src/lib/generatePath.js (2)
generatePath(3-34)filters(8-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Node 18
- GitHub Check: Docker linux/amd64
🔇 Additional comments (4)
src/dashboard/Data/Browser/Browser.react.js (4)
206-206: LGTM!The method binding follows the standard React pattern and is consistent with other bindings in the constructor.
638-657: LGTM!The pagination extraction logic is well-implemented with proper validation:
- Safely handles missing or invalid query parameters
- Uses
parseIntwith radix 10 (good practice)- Validates skip >= 0 and limit > 0
- Returns sensible defaults
659-696: LGTM!The
updateURLmethod is well-designed:
- Preserves existing query parameters (filters, filterId, editFilter)
- Optimizes URLs by excluding default values (skip=0, limit=100)
- Uses
navigateto enable browser back/forward functionality- Correctly handles relation URLs by returning early
551-594: LGTM!The pagination integration into
prefetchDatais well-implemented:
- Extracts pagination parameters from the URL query string
- Provides sensible fallback to current state when URL parameters are absent
- Makes the URL the source of truth for pagination state
|
🎉 This change has been released in version 8.2.0-alpha.15 |
2 participants
New Pull Request Checklist
Issue Description
Data browser pagination is not reflected in the dashboard URL, hence when using browser navigation to go backwards, forwards the pagination information is lost.
Approach
Add pagination to browser URL, so that pagination information is preserved when using browser navigation. This also adds the benefit that pagination is now controllable through dashboard URL; for example it's now possible to jump to a specific page via the dashboard URL.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.