-
Notifications
You must be signed in to change notification settings - Fork 8
fix: project list job stats not showing + uniform card height + clickable links #77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -223,15 +223,33 @@ function ProjectListCard({ | |
| Jobs | ||
| </Typography> | ||
| <div className="project-list-card-progress-section"> | ||
| <Typography.Text> | ||
| <Typography.Text | ||
| className="project-list-card-job-link" | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| navigate("/project/job/list"); | ||
| }} | ||
| > | ||
| <ExclamationCircleFilled className="project-list-card-status-icon warning" /> | ||
| {details?.total_scheduled_jobs} Scheduled | ||
| </Typography.Text> | ||
| <Typography.Text> | ||
| <Typography.Text | ||
| className="project-list-card-job-link" | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| navigate("/project/job/history"); | ||
| }} | ||
| > | ||
| <InfoCircleFilled className="project-list-card-status-icon info" /> | ||
| {details?.total_active_jobs} In Progress | ||
| </Typography.Text> | ||
| <Typography.Text> | ||
| <Typography.Text | ||
| className="project-list-card-job-link" | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| navigate("/project/job/history"); | ||
| }} | ||
| > | ||
| <CheckCircleFilled className="project-list-card-status-icon failed" /> | ||
| {details?.total_failed_job} Failed | ||
| </Typography.Text> | ||
|
Comment on lines
+226
to
255
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Clicking any of the three job-stat links navigates to the global This matches the existing Prompt To Fix With AIThis is a comment left during a code review.
Path: frontend/src/base/components/project-list/ProjectListCard.jsx
Line: 226-255
Comment:
**Job link navigation missing project context**
Clicking any of the three job-stat links navigates to the global `/project/job/list` or `/project/job/history` route without carrying the current project's ID. A user clicking "3 Scheduled" on a specific project card will land on the full global jobs list with no filter applied, which can be disorienting. Other call sites (e.g. `no-code-model.jsx`) pass a `task=` or params query string; at minimum passing a project filter would make these links meaningfully project-scoped.
This matches the existing `NavigationTabs` behaviour, so it may be intentional, but it is worth confirming whether the Jobs/Run History pages support a project filter query param.
How can I resolve this? If you propose a fix, please make it concise.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Valid point — currently navigates to the global Jobs/Run History pages, consistent with how the sidebar navigation works (same routes, no project filter). Filtering by project would need a query param ( |
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
apps.get_app_config("job_scheduler")is called inside the request handler on every project-list fetch. Since the Django app registry is fixed at startup, this result never changes at runtime. Consider hoisting the check to module level (or caching it as a module-level constant) to avoid the repeated try/except overhead on every request.Then replace the inline block with just:
Prompt To Fix With AI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acknowledged —
apps.get_app_config()is a dict lookup internally (O(1), nanosecond overhead). Caching at module level would save the try/except but the gain is negligible for a page-load endpoint. Can optimize in a follow-up if profiling shows it matters.