feat(dashboard): add pagination and filters to the jobs page#35
Open
knutties wants to merge 1 commit into
Open
feat(dashboard): add pagination and filters to the jobs page#35knutties wants to merge 1 commit into
knutties wants to merge 1 commit into
Conversation
WalkthroughThis PR adds job-list filtering and cursor-based pagination across the full stack. The API contract extends to accept optional filters; handlers validate and forward them to the database layer, which builds dynamic SQL. The frontend client safely encodes filters into query strings and returns paginated responses; the workspace detail page adds filter inputs and pagination UI controls. ChangesJob Listing with Filters and Pagination
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/common/src/db/jobs.rs`:
- Around line 181-215: The pagination uses only created_at, which is unstable;
update the cursor logic and ORDER BY to use a deterministic tie-breaker
(job_id). Replace the single-row cursor condition "created_at < (SELECT
created_at FROM {t} WHERE job_id = ${n})" with a composite condition like
"(created_at < (SELECT created_at FROM {t} WHERE job_id = ${n}) OR (created_at =
(SELECT created_at FROM {t} WHERE job_id = ${n}) AND job_id < ${m}))" and add
the job_id value to binds (note the new bind ${m}); then change the ORDER BY in
the sql string from "ORDER BY created_at DESC" to "ORDER BY created_at DESC,
job_id DESC" so results are stably ordered for cursor pagination (update the
binds vector where the cursor variable c is handled and the final sql variable).
- Around line 202-205: The endpoint filter currently treats user input percent
and underscore as SQL wildcards; update the block that handles filters.endpoint
so you escape '%' and '_' (and backslashes) in the endpoint value before pushing
it to binds, then use an ILIKE with an explicit ESCAPE clause; specifically,
transform filters.endpoint (e.g. replace '\' with '\\', '%' with '\%', '_' with
'\_'), push the escaped string to binds, change the condition pushed into
conditions from "endpoint ILIKE '%' || ${n} || '%'" to include "ESCAPE '\\'"
(e.g. "endpoint ILIKE '%' || ${n} || '%' ESCAPE '\\'") and leave n increment
logic as-is so the query matches literals correctly.
In `@crates/dashboard/src/pages/workspace_detail.rs`:
- Around line 1195-1204: The on_next closure can push the same next_cursor
repeatedly if clicked before data updates; modify on_next (the closure using
next_cursor, page_index, set_page_cursors, set_page_index) to first read the
current idx and cursors inside the set_page_cursors.update closure and only
truncate/push when the cursor at position idx+1 is not already equal to
Some(nc); if the existing entry equals Some(nc) skip pushing (but still
set_page_index to idx+1), otherwise perform the truncate + push and then
set_page_index; this prevents double-advancing/appending duplicates when
next_cursor is stale.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8de77983-74dd-4671-a7f1-0a2550e0338d
📒 Files selected for processing (6)
crates/api/src/handlers/jobs.rscrates/common/src/db/jobs.rscrates/dashboard/src/api/client.rscrates/dashboard/src/api/models.rscrates/dashboard/src/pages/workspace_detail.rssmithy/model/jobs.smithy
Comment on lines
+181
to
+215
| conditions.push(format!( | ||
| "created_at < (SELECT created_at FROM {t} WHERE job_id = ${n})" | ||
| )); | ||
| binds.push(c.to_string()); | ||
| n += 1; | ||
| } | ||
| if let Some(status) = &filters.status { | ||
| conditions.push(format!("status = ${n}")); | ||
| binds.push(status.clone()); | ||
| n += 1; | ||
| } | ||
| if let Some(trigger) = &filters.trigger { | ||
| conditions.push(format!("trigger_type = ${n}")); | ||
| binds.push(trigger.clone()); | ||
| n += 1; | ||
| } | ||
| if let Some(endpoint_type) = &filters.endpoint_type { | ||
| conditions.push(format!("endpoint_type = ${n}")); | ||
| binds.push(endpoint_type.clone()); | ||
| n += 1; | ||
| } | ||
| if let Some(endpoint) = &filters.endpoint { | ||
| conditions.push(format!("endpoint ILIKE '%' || ${n} || '%'")); | ||
| binds.push(endpoint.clone()); | ||
| n += 1; | ||
| } | ||
|
|
||
| let where_clause = if conditions.is_empty() { | ||
| String::new() | ||
| } else { | ||
| format!(" WHERE {}", conditions.join(" AND ")) | ||
| }; | ||
|
|
||
| let sql = format!("SELECT * FROM {t}{where_clause} ORDER BY created_at DESC LIMIT ${n}"); | ||
| (sql, binds) |
There was a problem hiding this comment.
Use a stable sort key for cursor pagination.
Line 182 and Line 214 paginate/order by created_at only, so rows sharing the same timestamp can be skipped or repeated across pages.
Suggested fix
- conditions.push(format!(
- "created_at < (SELECT created_at FROM {t} WHERE job_id = ${n})"
- ));
+ conditions.push(format!(
+ "(created_at, job_id) < (SELECT created_at, job_id FROM {t} WHERE job_id = ${n})"
+ ));
@@
- let sql = format!("SELECT * FROM {t}{where_clause} ORDER BY created_at DESC LIMIT ${n}");
+ let sql = format!(
+ "SELECT * FROM {t}{where_clause} ORDER BY created_at DESC, job_id DESC LIMIT ${n}"
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| conditions.push(format!( | |
| "created_at < (SELECT created_at FROM {t} WHERE job_id = ${n})" | |
| )); | |
| binds.push(c.to_string()); | |
| n += 1; | |
| } | |
| if let Some(status) = &filters.status { | |
| conditions.push(format!("status = ${n}")); | |
| binds.push(status.clone()); | |
| n += 1; | |
| } | |
| if let Some(trigger) = &filters.trigger { | |
| conditions.push(format!("trigger_type = ${n}")); | |
| binds.push(trigger.clone()); | |
| n += 1; | |
| } | |
| if let Some(endpoint_type) = &filters.endpoint_type { | |
| conditions.push(format!("endpoint_type = ${n}")); | |
| binds.push(endpoint_type.clone()); | |
| n += 1; | |
| } | |
| if let Some(endpoint) = &filters.endpoint { | |
| conditions.push(format!("endpoint ILIKE '%' || ${n} || '%'")); | |
| binds.push(endpoint.clone()); | |
| n += 1; | |
| } | |
| let where_clause = if conditions.is_empty() { | |
| String::new() | |
| } else { | |
| format!(" WHERE {}", conditions.join(" AND ")) | |
| }; | |
| let sql = format!("SELECT * FROM {t}{where_clause} ORDER BY created_at DESC LIMIT ${n}"); | |
| (sql, binds) | |
| conditions.push(format!( | |
| "(created_at, job_id) < (SELECT created_at, job_id FROM {t} WHERE job_id = ${n})" | |
| )); | |
| binds.push(c.to_string()); | |
| n += 1; | |
| } | |
| if let Some(status) = &filters.status { | |
| conditions.push(format!("status = ${n}")); | |
| binds.push(status.clone()); | |
| n += 1; | |
| } | |
| if let Some(trigger) = &filters.trigger { | |
| conditions.push(format!("trigger_type = ${n}")); | |
| binds.push(trigger.clone()); | |
| n += 1; | |
| } | |
| if let Some(endpoint_type) = &filters.endpoint_type { | |
| conditions.push(format!("endpoint_type = ${n}")); | |
| binds.push(endpoint_type.clone()); | |
| n += 1; | |
| } | |
| if let Some(endpoint) = &filters.endpoint { | |
| conditions.push(format!("endpoint ILIKE '%' || ${n} || '%'")); | |
| binds.push(endpoint.clone()); | |
| n += 1; | |
| } | |
| let where_clause = if conditions.is_empty() { | |
| String::new() | |
| } else { | |
| format!(" WHERE {}", conditions.join(" AND ")) | |
| }; | |
| let sql = format!( | |
| "SELECT * FROM {t}{where_clause} ORDER BY created_at DESC, job_id DESC LIMIT ${n}" | |
| ); | |
| (sql, binds) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/common/src/db/jobs.rs` around lines 181 - 215, The pagination uses
only created_at, which is unstable; update the cursor logic and ORDER BY to use
a deterministic tie-breaker (job_id). Replace the single-row cursor condition
"created_at < (SELECT created_at FROM {t} WHERE job_id = ${n})" with a composite
condition like "(created_at < (SELECT created_at FROM {t} WHERE job_id = ${n})
OR (created_at = (SELECT created_at FROM {t} WHERE job_id = ${n}) AND job_id <
${m}))" and add the job_id value to binds (note the new bind ${m}); then change
the ORDER BY in the sql string from "ORDER BY created_at DESC" to "ORDER BY
created_at DESC, job_id DESC" so results are stably ordered for cursor
pagination (update the binds vector where the cursor variable c is handled and
the final sql variable).
Comment on lines
+202
to
+205
| if let Some(endpoint) = &filters.endpoint { | ||
| conditions.push(format!("endpoint ILIKE '%' || ${n} || '%'")); | ||
| binds.push(endpoint.clone()); | ||
| n += 1; |
There was a problem hiding this comment.
Escape wildcard characters in endpoint search.
Line 203 treats % and _ inside user input as SQL wildcards, so endpoint-name filtering can return incorrect matches for literal names containing those characters.
Suggested fix
+fn escape_like(value: &str) -> String {
+ value
+ .replace('\\', "\\\\")
+ .replace('%', "\\%")
+ .replace('_', "\\_")
+}
@@
- if let Some(endpoint) = &filters.endpoint {
- conditions.push(format!("endpoint ILIKE '%' || ${n} || '%'"));
- binds.push(endpoint.clone());
+ if let Some(endpoint) = &filters.endpoint {
+ conditions.push(format!("endpoint ILIKE '%' || ${n} || '%' ESCAPE '\\'"));
+ binds.push(escape_like(endpoint));
n += 1;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if let Some(endpoint) = &filters.endpoint { | |
| conditions.push(format!("endpoint ILIKE '%' || ${n} || '%'")); | |
| binds.push(endpoint.clone()); | |
| n += 1; | |
| if let Some(endpoint) = &filters.endpoint { | |
| conditions.push(format!("endpoint ILIKE '%' || ${n} || '%' ESCAPE '\\'")); | |
| binds.push(escape_like(endpoint)); | |
| n += 1; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/common/src/db/jobs.rs` around lines 202 - 205, The endpoint filter
currently treats user input percent and underscore as SQL wildcards; update the
block that handles filters.endpoint so you escape '%' and '_' (and backslashes)
in the endpoint value before pushing it to binds, then use an ILIKE with an
explicit ESCAPE clause; specifically, transform filters.endpoint (e.g. replace
'\' with '\\', '%' with '\%', '_' with '\_'), push the escaped string to binds,
change the condition pushed into conditions from "endpoint ILIKE '%' || ${n} ||
'%'" to include "ESCAPE '\\'" (e.g. "endpoint ILIKE '%' || ${n} || '%' ESCAPE
'\\'") and leave n increment logic as-is so the query matches literals
correctly.
Comment on lines
+1195
to
+1204
| let on_next = move |_| { | ||
| if let Some(nc) = next_cursor.clone() { | ||
| let idx = page_index.get_untracked(); | ||
| set_page_cursors.update(|cursors| { | ||
| // Drop any forward history, then record the next page's cursor. | ||
| cursors.truncate(idx + 1); | ||
| cursors.push(Some(nc)); | ||
| }); | ||
| set_page_index.set(idx + 1); | ||
| } |
There was a problem hiding this comment.
Guard against stale next_cursor double-advance.
Line 1196 can run multiple times before new data arrives, appending the same cursor repeatedly and navigating to duplicate pages.
Suggested fix
let on_next = move |_| {
if let Some(nc) = next_cursor.clone() {
let idx = page_index.get_untracked();
- set_page_cursors.update(|cursors| {
+ let mut advanced = false;
+ set_page_cursors.update(|cursors| {
+ if cursors.get(idx).and_then(|c| c.as_ref()) == Some(&nc) {
+ return;
+ }
// Drop any forward history, then record the next page's cursor.
cursors.truncate(idx + 1);
cursors.push(Some(nc));
+ advanced = true;
});
- set_page_index.set(idx + 1);
+ if advanced {
+ set_page_index.set(idx + 1);
+ }
}
};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/dashboard/src/pages/workspace_detail.rs` around lines 1195 - 1204, The
on_next closure can push the same next_cursor repeatedly if clicked before data
updates; modify on_next (the closure using next_cursor, page_index,
set_page_cursors, set_page_index) to first read the current idx and cursors
inside the set_page_cursors.update closure and only truncate/push when the
cursor at position idx+1 is not already equal to Some(nc); if the existing entry
equals Some(nc) skip pushing (but still set_page_index to idx+1), otherwise
perform the truncate + push and then set_page_index; this prevents
double-advancing/appending duplicates when next_cursor is stale.
Wire up cursor-based pagination and server-side filtering for the Jobs tab in the workspace dashboard. Backend: - common/db/jobs::list now accepts a JobFilters struct (status, trigger, endpoint substring, endpoint_type), built via a pure, unit-tested query builder so the cursor + filter placeholder bookkeeping is verifiable. - api jobs::list parses a JobListFilters query struct alongside the existing pagination params, validating enum-like values up front so a typo returns 400 rather than silently empty results. - smithy ListJobsInput gains the endpoint_type query param (endpoint, trigger_type, status were already declared). Dashboard: - list_jobs sends cursor/limit/status/trigger_type/endpoint/endpoint_type via JobListQueryParams and returns the full PaginatedResponse (the cursor was previously discarded, capping the view at the first 50 jobs). - JobsTab gains a filter bar (status, trigger, endpoint type, endpoint search + clear) and pagination controls (Prev/Next with a cursor stack for back-navigation, plus a per-page size selector). Changing a filter or page size resets pagination.
knutties
force-pushed
the
claude/gifted-bardeen-n11mlb
branch
from
2ef502a to
9b0a56c
Compare
knutties
changed the title
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
2 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.
Summary
This PR adds comprehensive filtering and cursor-based pagination to the jobs list in the workspace detail page. Users can now filter jobs by status, trigger type, endpoint type, and endpoint name, with support for navigating through large result sets.
Key Changes
Dashboard Frontend (
crates/dashboard/src/)JobsPaginationcomponent with per-page size selector (25/50/100) and Previous/Next buttonslist_jobscall to passJobListParamswith cursor, limit, and filter optionsAPI Backend (
crates/api/src/handlers/jobs.rs)JobListFiltersstruct to parse filter query parameters from requests?status=) are treated as absent, allowing the dashboard to send empty params for "All" selectionslisthandler to convert API filters to database layer filters and pass them throughDatabase Layer (
crates/common/src/db/jobs.rs)build_list_queryfunction that constructs SQL with proper placeholder numbering for cursor and filtersAPI Client (
crates/dashboard/src/api/client.rs)encode_query_valuehelper for RFC 3986 compliant query parameter encodingpush_query_paramto safely construct query strings with filterslist_jobsreturn type fromVec<Job>toPaginatedResponse<Job>to include cursor for paginationModels (
crates/dashboard/src/api/models.rs)Smithy Schema (
smithy/model/jobs.smithy)endpoint_typequery parameter toListJobsInputfor API schema consistencyImplementation Details
Noneat multiple layers to ensure consistent behaviorhttps://claude.ai/code/session_01KEhs81eTYAfB64LRGeDiFw
Summary by CodeRabbit
Release Notes
New Features
Improvements