Feat: Pagespeed Graph Pagination Backend #3045
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Client: hook & network client/src/Hooks/v1/monitorHooks.js, client/src/Utils/NetworkService.js |
useFetchStatsByMonitorId signature adds page; hook passes page to networkService.getStatsByMonitorId; NetworkService appends page to URLSearchParams. |
API layer: controller & validation server/src/controllers/v1/monitorController.js, server/src/validation/joi.js |
Controller extracts page from query and forwards it to service; Joi validation adds page: joi.number().integer().min(0). |
Business & DB logic server/src/service/v1/business/monitorService.js, server/src/db/v1/modules/monitorModule.js |
getMonitorStatsById signatures extended to accept page and forward it; new offsetDatesByPage(dates, dateRange, page) added; DB applies offset dates when page provided, adjusts date filtering (including checksSkipped) and returns remainingChecks in monitorStats. |
Sequence Diagram(s)
sequenceDiagram
participant Client
participant Hook as useFetchStatsByMonitorId
participant Network as NetworkService
participant Controller as monitorController
participant Service as monitorService
participant DB as monitorModule
Client->>Hook: set/use page (N)
Hook->>Network: getStatsByMonitorId(monitorId,..., page=N)
Network->>Controller: GET /monitors/{id}/stats?page=N
Controller->>Service: getMonitorStatsById(..., page=N)
Service->>DB: getMonitorStatsById(..., page=N)
rect rgb(235,245,255)
Note over DB: Pagination-aware date computation
DB->>DB: getDateRange(dateRange)
DB->>DB: offsetDatesByPage(dates, dateRange, page)
DB->>DB: filter checksForDateRange and checksSkipped
DB->>DB: compute remainingChecks
end
DB-->>Service: monitorStats + remainingChecks
Service-->>Controller: monitorStats + remainingChecks
Controller-->>Network: response
Network-->>Hook: data
Hook-->>Client: paginated stats
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
- Review
offsetDatesByPage()for correctness acrossdateRangebranches (recent/day/week/month/all). - Verify
checksSkippedandremainingCheckscalculations and boundary conditions. - Confirm
pageis validated and correctly threaded controller → service → DB and that behavior is unchanged whenpageis absent.
Poem
🐰 I hop through pages, one by one,
Offsetting days in morning sun.
Skipped and shown, the counts align,
Chunks arrive—smooth scroll, just fine.
📊✨
Pre-merge checks and finishing touches
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title 'Feat: Pagespeed Graph Pagination Backend' clearly and concisely describes the main change: adding backend pagination support for pagespeed graph. |
| Description check | ✅ Passed | The PR description includes a clear explanation of changes, covers the new pageOffset parameter, mentions the remaining checks addition, and includes the issue number #606 with several checklist items completed. |
| Linked Issues check | ✅ Passed | The changes implement pagination for PageSpeed checks via a pageOffset parameter that shifts date ranges, allowing paginated data fetching as required by issue #606, though preloading logic appears frontend-focused. |
| Out of Scope Changes check | ✅ Passed | All changes are narrowly focused on adding pagination support to monitor stats retrieval across client/server layers; no unrelated modifications detected. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
📜 Recent review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
client/src/Hooks/v1/monitorHooks.js(3 hunks)client/src/Utils/NetworkService.js(1 hunks)server/src/controllers/v1/monitorController.js(2 hunks)server/src/db/v1/modules/monitorModule.js(4 hunks)server/src/service/v1/business/monitorService.js(2 hunks)server/src/validation/joi.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- server/src/service/v1/business/monitorService.js
- client/src/Utils/NetworkService.js
- server/src/db/v1/modules/monitorModule.js
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-24T17:52:55.506Z
Learnt from: Jesulayomy
Repo: bluewave-labs/Checkmate PR: 2664
File: client/src/Pages/Uptime/Create/index.jsx:92-96
Timestamp: 2025-07-24T17:52:55.506Z
Learning: In the Uptime monitor components, the `formatAndSet` function was deprecated due to rendering issues caused by state mutations. The current approach stores intervals internally in milliseconds (API format) but converts for display using `const displayInterval = monitor?.interval / MS_PER_MINUTE || 1;` and converts user input back to milliseconds using `value = value * MS_PER_MINUTE` in the onChange handler.
Applied to files:
server/src/controllers/v1/monitorController.jsclient/src/Hooks/v1/monitorHooks.js
🧬 Code graph analysis (1)
server/src/controllers/v1/monitorController.js (1)
server/src/db/v1/modules/monitorModuleQueries.js (1)
req(1079-1079)
🔇 Additional comments (6)
server/src/validation/joi.js (1)
147-147: LGTM! Validation correctly implemented.The
pageparameter validation properly constrains the value to non-negative integers and addresses previous feedback. Making it optional ensures backward compatibility.server/src/controllers/v1/monitorController.js (2)
85-85: LGTM! Parameter extraction follows established patterns.The
pageparameter is correctly extracted from the validated query object alongside other parameters.
101-101: LGTM! Parameter correctly threaded to service layer.The
pageparameter is properly passed through to the monitor service for processing.client/src/Hooks/v1/monitorHooks.js (3)
173-173: LGTM! Parameter correctly added to hook signature.The
pageparameter is properly included in the hook's destructured parameters, following the established pattern.
191-191: LGTM! Parameter correctly passed to network service.The
pageparameter is properly threaded through to the network service call for fetching paginated statistics.
203-212: LGTM! Dependency array correctly updated.The
pageparameter is properly included in the dependency array to trigger refetching when pagination changes. The reformatting to one dependency per line improves readability and makes future diffs clearer.
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 @coderabbitai help to get the list of available commands and usage tips.
mospell
requested review from
Br0wnHammer,
Owaiseimdad,
ajhollid and
karenvicent
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
client/src/Utils/NetworkService.js (1)
169-195: Update JSDoc to document the pageOffset parameter.The JSDoc comment for
getStatsByMonitorIdis missing documentation for the newpageOffsetparameter.Apply this diff to update the JSDoc:
/** * ************************************ * Get stats for a monitor * ************************************ * * @async * @param {Object} config - The configuration object. * @param {string} config.monitorId - The ID of the monitor whose statistics are to be retrieved. * @param {string} config.sortOrder - The order in which to sort the retrieved statistics. * @param {number} config.limit - The maximum number of statistics to retrieve. * @param {string} config.dateRange - The date range for which to retrieve statistics. * @param {number} config.numToDisplay - The number of checks to display. * @param {boolean} config.normalize - Whether to normalize the retrieved statistics. + * @param {number} [config.pageOffset] - The page offset for pagination (shifts date range backward). * @returns {Promise<AxiosResponse>} The response from the axios GET request. */
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
client/src/Hooks/v1/monitorHooks.js(3 hunks)client/src/Utils/NetworkService.js(1 hunks)server/src/controllers/v1/monitorController.js(2 hunks)server/src/db/v1/modules/monitorModule.js(4 hunks)server/src/service/v1/business/monitorService.js(2 hunks)server/src/validation/joi.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
server/src/controllers/v1/monitorController.js (1)
server/src/db/v1/modules/monitorModuleQueries.js (1)
req(1079-1079)
🔇 Additional comments (4)
client/src/Hooks/v1/monitorHooks.js (1)
166-205: LGTM! Pagination parameter correctly integrated.The
pageOffsetparameter is properly threaded through the hook signature, passed to the network service call, and included in the dependency array to trigger refetching when the offset changes.server/src/controllers/v1/monitorController.js (1)
80-111: LGTM! Controller properly extracts and forwards pagination parameter.The
pageOffsetquery parameter is correctly extracted and passed to the service layer, consistent with the existing parameter handling pattern.server/src/service/v1/business/monitorService.js (1)
46-59: LGTM! Service layer correctly propagates pagination parameter.The
pageOffsetparameter is properly added to the service signature and forwarded to the database module without additional processing, which is appropriate for the service layer.server/src/db/v1/modules/monitorModule.js (1)
276-323: LGTM! Pagination logic correctly implemented.The pagination implementation properly:
- Applies date offset when
pageOffsetis provided (lines 289-291)- Calculates
checksSkippedto track checks newer than the current page (line 295)- Computes
remainingChecksto indicate how many older checks exist for future pages (line 305)The
remainingCheckscalculation is correct: it represents checks that are older than the current page by subtracting both the current page checks and future page checks from the total.
![llamapreview[bot]](https://avatars.githubusercontent.com/in/1023541?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Code Review by LlamaPReview
🎯 TL;DR & Recommendation
Recommendation: Request Changes
This PR adds pagination support but introduces critical bugs in date range handling and remaining checks calculation, and the main frontend component is not updated, making the feature unusable.
📄 Documentation Diagram
This diagram documents the paginated data fetching workflow for the PageSpeed graph.
sequenceDiagram
participant U as User
participant F as Frontend
participant B as Backend
participant D as Database
U->>F: Navigate to PageSpeed graph
F->>B: getMonitorStatsById(monitorId, pageOffset, ...)
note over B: PR #35;3045: pageOffset shifts date range
B->>B: offsetDatesByPage(dates, dateRange, pageOffset)
B->>D: Query checks within date range
D-->>B: Return checks
B->>B: Calculate remainingChecks
B-->>F: Return monitor stats with remainingChecks
F-->>U: Display paginated data
🌟 Strengths
- Adds requested pagination feature for PageSpeed graph with remaining checks count.
- Well-structured backend changes with validation updates.
| Priority | File | Category | Impact Summary | Anchors |
|---|---|---|---|---|
| P1 | server/src/db/v1/modules/monitorModule.js | Bug | Missing month/hour dateRange support causing runtime errors | symbol:offsetDatesByPage |
| P1 | server/src/db/v1/modules/monitorModule.js | Bug | Incorrect remainingChecks calculation breaking pagination | symbol:getMonitorStatsById |
| P1 | client/src/Pages/v1/PageSpeed/.../index.jsx | Architecture | Frontend not updated for pagination, feature unusable | symbol:useFetchStatsByMonitorId |
| P2 | server/src/db/v1/modules/monitorModule.js | Bug | pageOffset=0 not handled, inconsistent behavior | symbol:getMonitorStatsById |
| P2 | server/src/validation/joi.js | Maintainability | Negative pageOffset allowed, confusing API | symbol:getMonitorStatsByIdQueryValidation |
| P2 | server/src/db/v1/modules/monitorModule.js | Performance | Potential performance issue from loading all checks | symbol:getMonitorStatsById |
🔍 Notable Themes
- Inconsistent Date Range Handling: Gaps in date range support across functions risk runtime errors and broken queries.
- Frontend-Backend Contract Mismatch: Pagination implemented backend-side but not consumed by primary frontend component, hindering feature usability.
📈 Risk Diagram
This diagram illustrates the risks in date range handling and remaining checks calculation.
sequenceDiagram
participant F as Frontend
participant B as Backend
F->>B: getMonitorStatsById with pageOffset
B->>B: offsetDatesByPage
note over B: R1(P1): Missing month/hour dateRange support
B->>B: getMonitorChecks
B->>B: Calculate remainingChecks
note over B: R2(P1): Incorrect checksSkipped filter
B-->>F: Return data with remainingChecks
note over F: R3(P1): Frontend may not pass pageOffset
⚠️ **Unanchored Suggestions (Manual Review Recommended)**
The following suggestions could not be precisely anchored to a specific line in the diff. This can happen if the code is outside the changed lines, has been significantly refactored, or if the suggestion is a general observation. Please review them carefully in the context of the full file.
📁 File: server/src/db/v1/modules/monitorModule.js
The remainingChecks calculation is flawed. checksSkipped filters checks after dates.end, but dates.end is already offset into the past, so this incorrectly counts future checks. The intended behavior should count checks before the current page's date range. This will cause incorrect pagination state on the frontend.
Suggestion:
const checksBeforeCurrentPage = checksAll.filter((check) => check.createdAt < dates.start);
remainingChecks: checksBeforeCurrentPage.lengthRelated Code:
const checksSkipped = checksAll.filter((check) => check.createdAt > dates.end);
remainingChecks: checksAll.length - checksForDateRange.length - checksSkipped.length📁 File: client/src/Pages/v1/PageSpeed/Details/index.jsx
The primary consumer of useFetchStatsByMonitorId (PageSpeed Details page) is not updated to pass the new pageOffset parameter. This creates an API contract break where the backend expects pagination support but the main frontend component doesn't implement it. The feature will be unusable without updating this component.
Suggestion:
const [pageOffset, setPageOffset] = useState(0);
const [monitor, audits, isLoading, networkError] = useFetchStatsByMonitorId({
monitorId,
sortOrder: "desc",
limit: 50,
pageOffset,
});
Related Code:
const [monitor, audits, isLoading, networkError] = useFetchStatsByMonitorId({
monitorId,
sortOrder: "desc",
limit: 50,
});
💡 Have feedback? We'd love to hear it in our GitHub Discussions.
✨ This review was generated by LlamaPReview Advanced, which is free for all open-source projects. Learn more.
| }; | ||
|
|
||
| // Helper | ||
| offsetDatesByPage = (dates, dateRange, pageOffset) => { |
There was a problem hiding this comment.
P1 | Confidence: High
The date offset calculation is missing support for "month" dateRange and incorrectly handles "hour" dateRange. When dateRange is "month", dateOffsets[dateRange] will be undefined, resulting in NaN date calculations that will break the query. The "hour" dateRange is supported by getDateRange() but not handled here, creating inconsistency. This will cause runtime errors for these date ranges.
| offsetDatesByPage = (dates, dateRange, pageOffset) => { | |
| offsetDatesByPage = (dates, dateRange, pageOffset) => { | |
| const dateOffsets = { | |
| hour: 60 * 60 * pageOffset * 1000, | |
| recent: 60 * 60 * 2 * pageOffset * 1000, | |
| day: 60 * 60 * 24 * pageOffset * 1000, | |
| week: 60 * 60 * 24 * 7 * pageOffset * 1000, | |
| month: 60 * 60 * 24 * 30 * pageOffset * 1000, // Approximate month as 30 days | |
| all: 0, | |
| }; | |
| return { | |
| start: new Date(dates.start.getTime() - dateOffsets[dateRange]), | |
| end: new Date(new Date().getTime() - dateOffsets[dateRange]), | |
| }; | |
| }; |
Evidence: symbol:offsetDatesByPage, method:getMonitorStatsById
| let dates = this.getDateRange(dateRange); | ||
|
|
||
| if (pageOffset) { | ||
| dates = this.offsetDatesByPage(dates, dateRange, pageOffset); |
There was a problem hiding this comment.
P2 | Confidence: Medium
The condition if (pageOffset) treats 0 as falsy, preventing the first page (pageOffset=0) from applying date offsets. This creates inconsistent behavior where page 0 uses current dates but subsequent pages use offset dates. The pagination should work consistently starting from page 0.
| let dates = this.getDateRange(dateRange); | |
| if (pageOffset) { | |
| dates = this.offsetDatesByPage(dates, dateRange, pageOffset); | |
| if (pageOffset !== undefined && pageOffset !== null) { | |
| dates = this.offsetDatesByPage(dates, dateRange, pageOffset); | |
| } |
Evidence: symbol:getMonitorStatsById
server/src/validation/joi.js
Outdated
| dateRange: joi.string().valid("hour", "day", "week", "month", "all"), | ||
| numToDisplay: joi.number(), | ||
| normalize: joi.boolean(), | ||
| pageOffset: joi.number().integer(), |
There was a problem hiding this comment.
P2 | Confidence: Medium
The validation allows negative pageOffset values which don't make logical sense for pagination (can't page into the future). This could lead to confusing API behavior. Adding a minimum constraint would make the API more robust.
| pageOffset: joi.number().integer(), | |
| pageOffset: joi.number().integer().min(0), |
Evidence: symbol:getMonitorStatsByIdQueryValidation
|
|
||
| // Get Checks for monitor in date range requested | ||
| const dates = this.getDateRange(dateRange); | ||
| const { checksAll, checksForDateRange } = await this.getMonitorChecks(monitorId, dates, sort); |
There was a problem hiding this comment.
P2 | Confidence: Medium
Speculative: The current implementation loads all checks (checksAll) into memory for every paginated request. For monitors with extensive history, this could cause performance issues. Consider implementing database-level pagination or counting instead of loading all records.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
server/src/validation/joi.js (1)
147-147: Previous validation concerns have been addressed.The
pageOffsetvalidation now correctly includes the.min(0)constraint, preventing negative integers that would be semantically invalid for pagination. The implementation is sound.As an optional enhancement, consider adding an upper bound constraint (e.g.,
.max(365)) to prevent excessively large offset values that might impact performance or result in empty datasets. However, if this is already handled at the business logic layer, the current validation is sufficient.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
server/src/db/v1/modules/monitorModule.js (1)
296-298: Verify that pageOffset=0 is handled correctly.The condition
if (pageOffset)treats 0 as falsy. IfpageOffset=0represents the first page, the current code won't apply date offsets for it, creating inconsistent pagination behavior. Ensure this aligns with the intended pagination logic.As noted in past reviews, if page 0 should apply offsets, use:
-if (pageOffset) { +if (pageOffset !== undefined && pageOffset !== null) { dates = this.offsetDatesByPage(dates, dateRange, pageOffset); }
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
server/src/db/v1/modules/monitorModule.js(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-24T17:52:55.506Z
Learnt from: Jesulayomy
Repo: bluewave-labs/Checkmate PR: 2664
File: client/src/Pages/Uptime/Create/index.jsx:92-96
Timestamp: 2025-07-24T17:52:55.506Z
Learning: In the Uptime monitor components, the `formatAndSet` function was deprecated due to rendering issues caused by state mutations. The current approach stores intervals internally in milliseconds (API format) but converts for display using `const displayInterval = monitor?.interval / MS_PER_MINUTE || 1;` and converts user input back to milliseconds using `value = value * MS_PER_MINUTE` in the onChange handler.
Applied to files:
server/src/db/v1/modules/monitorModule.js
🔇 Additional comments (1)
server/src/db/v1/modules/monitorModule.js (1)
302-313: LGTM: Pagination metadata is correctly calculated.The logic for
checksSkippedandremainingCheckscorrectly supports backward pagination:
checksSkippedcounts checks newer than the current page window (afterdates.end)remainingChecksrepresents older checks available for further paginationThis aligns with the PR objective of implementing scrollable graph pagination.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
server/src/db/v1/modules/monitorModule.js (1)
296-298: Consider pageOffset=0 edge case.The condition
if (pageOffset)treats0as falsy, meaning whenpageOffsetis0, the date offset won't be applied. This could be intentional if page 0 represents "current data with no offset," but it creates an inconsistency where page 0 behaves differently from page 1+.If
pageOffset: 0should apply the same logic (i.e., no shift, which the math handles correctly), consider:-if (pageOffset) { +if (pageOffset !== undefined && pageOffset !== null) { dates = this.offsetDatesByPage(dates, dateRange, pageOffset); }This was previously flagged in past reviews.
🧹 Nitpick comments (1)
server/src/db/v1/modules/monitorModule.js (1)
124-144: Month handling correctly avoids mutation.The use of
new Date(dates.start)andnew Date(dates.end)at lines 135-136 properly prevents mutation of the original dates object, addressing the concern from previous reviews.However, note that if an unexpected
dateRangevalue is passed (not indateOffsetsand not "month"),dateOffsets[dateRange]will beundefined, leading toNaNdate values. SincedateRangeis validated by the Joi schema, this is acceptable defensive programming, but consider adding a fallback or validation within this helper for robustness.If you want to add defensive validation:
offsetDatesByPage = (dates, dateRange, pageOffset) => { const dateOffsets = { recent: 60 * 60 * 2 * pageOffset * 1000, day: 60 * 60 * 24 * pageOffset * 1000, week: 60 * 60 * 24 * 7 * pageOffset * 1000, all: 0, }; if (dateRange == "month") { return { start: new Date(new Date(dates.start).setMonth(dates.start.getMonth() - pageOffset)), end: new Date(new Date(dates.end).setMonth(dates.end.getMonth() - pageOffset)), }; } else { + const offset = dateOffsets[dateRange]; + if (offset === undefined) { + throw new Error(`Invalid dateRange: ${dateRange}`); + } return { - start: new Date(dates.start.getTime() - dateOffsets[dateRange]), - end: new Date(dates.end.getTime() - dateOffsets[dateRange]), + start: new Date(dates.start.getTime() - offset), + end: new Date(dates.end.getTime() - offset), }; } };
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
server/src/db/v1/modules/monitorModule.js(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-24T17:52:55.506Z
Learnt from: Jesulayomy
Repo: bluewave-labs/Checkmate PR: 2664
File: client/src/Pages/Uptime/Create/index.jsx:92-96
Timestamp: 2025-07-24T17:52:55.506Z
Learning: In the Uptime monitor components, the `formatAndSet` function was deprecated due to rendering issues caused by state mutations. The current approach stores intervals internally in milliseconds (API format) but converts for display using `const displayInterval = monitor?.interval / MS_PER_MINUTE || 1;` and converts user input back to milliseconds using `value = value * MS_PER_MINUTE` in the onChange handler.
Applied to files:
server/src/db/v1/modules/monitorModule.js
🔇 Additional comments (2)
server/src/db/v1/modules/monitorModule.js (2)
283-283: LGTM: Method signature updated for pagination.The addition of
pageOffsetto the method signature is clean and maintains backward compatibility since it's optional.
302-302: Pagination logic correctly computes remaining checks.The calculations for
checksSkipped(line 302) andremainingChecks(line 312) properly account for the paginated date window:
checksSkipped: Checks after the current page window (created afterdates.end)remainingChecks: Checks before the current page window (older data available for pagination)This provides the frontend with the necessary information for pagination UI.
Note: Loading all checks into memory (line 301 via
getMonitorChecks) may have performance implications for monitors with extensive history. This was flagged in previous reviews and remains a consideration for future optimization, but is acceptable for the current implementation scope.Also applies to: 312-312
ajhollid
left a comment
ajhollid
left a comment
There was a problem hiding this comment.
Hey @mospell ,
Thanks for your contribution here.
If the backend only takes a page offset that leaves calculating the page offset to the frontend. I believe that this should be a backend operation, and the frontend shouldn't know or care what the page offset is.
The frontend should only care about what page it is on, and how many rows per page to display.
In light of that, the backend should take two variables, the page and rowsPerPage, and multiplying the two together will yield the page offset.
You can see how pagination is handled for other monitor types on both the frontend and the backend for reference.
|
Hi @ajhollid,
My initial idea was time-based pagination, as querying an x amount of checks doesn't really make sense on a graph. How would rowsPerPage apply here? Could you give an example? |
|
@mospell you have a CI/CD check issue here. |
|
Hi @ajhollid, just pinging you again The way it currently works is that there is no pageOffset calculation in the frontend. All offset calculations happen in the backend as you requested. To clarify this I have renamed the I believe it would make more sense to have the user scroll through previous dates instead of displaying a number of rows per page. I think the way it currently works should satisfy your requirements. If you'd like me to change it anyway, please let me know. |
|
@gorkem-bwl this should be resolved now, thanks for the notice! |
|
@mospell perhaps I'm misunderstanding somethig here, pagination of results should depend on a ie on the frontend, the user selects "5 rows per page" and then paginates thorugh results, starting from page 0. Offset is then calcualted by multiplying Example: |
|
@ajhollid Usually yes, but I decided to go for time-based pagination here. In this case dateRange would kind of act as rowsPerPage here. On the frontend the user would pick a date range and then paginate through results that way, starting from page 0 (today). Instead of calculating an offset based on rowsPerPage and page, the backend shifts back the selected dateRange by the page number. So for example: This is done by the |
3 participants
Describe your changes
This PR adds pagination support for the PageSpeed chart on the backend by introducing an optional variable to FetchStatsByMonitorId/GetMonitorStatsById called
page, which shifts back the date range depending on it's value.For example: If
dateRangeis set to "day" andpageis 4, then it will query data from 4 days ago.The returned
monitorobject now also includes the number of remaining checks to be used for the frontend implementation.Write your issue number after "Fixes "
Fixes #606
Please ensure all items are checked off before requesting a review. "Checked off" means you need to add an "x" character between brackets so they turn into checkmarks.
<div>Add</div>, use):npm run formatin server and client directories, which automatically formats your code.Summary by CodeRabbit
New Features
Validation