Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion backend/backend/application/context/base_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,12 @@ def get_project_lists(
)
# Only annotate user_tasks if scheduler app is installed
from django.apps import apps
if apps.is_installed("job_scheduler") and hasattr(ProjectDetails, "user_tasks"):
try:
apps.get_app_config("job_scheduler")
_scheduler_installed = True
except LookupError:
_scheduler_installed = False
Comment on lines 184 to +189
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Scheduler check recomputed on every request

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.

# module level, outside the class/method
from django.apps import apps as _django_apps
try:
    _django_apps.get_app_config("job_scheduler")
    _SCHEDULER_INSTALLED = True
except LookupError:
    _SCHEDULER_INSTALLED = False

Then replace the inline block with just:

if _SCHEDULER_INSTALLED and hasattr(ProjectDetails, "user_tasks"):
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/backend/application/context/base_context.py
Line: 184-189

Comment:
**Scheduler check recomputed on every request**

`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.

```python
# module level, outside the class/method
from django.apps import apps as _django_apps
try:
    _django_apps.get_app_config("job_scheduler")
    _SCHEDULER_INSTALLED = True
except LookupError:
    _SCHEDULER_INSTALLED = False
```

Then replace the inline block with just:
```python
if _SCHEDULER_INSTALLED and hasattr(ProjectDetails, "user_tasks"):
```

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code

Copy link
Copy Markdown
Contributor Author

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.

if _scheduler_installed and hasattr(ProjectDetails, "user_tasks"):
annotations["_total_scheduled_jobs"] = Count("user_tasks", distinct=True)
annotations["_total_active_jobs"] = Count(
"user_tasks__periodic_task",
Expand Down
12 changes: 12 additions & 0 deletions frontend/src/base/components/project-list/ProjectListCard.css
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,18 @@
font-size: 12px;
color: var(--circle-color);
opacity: 0.6;
min-height: 38px;
display: flex;
align-items: center;
}

.project-list-card-job-link {
cursor: pointer;
transition: opacity 0.15s;
}

.project-list-card-job-link:hover {
opacity: 0.7;
}

.project-list-card-status-icon.warning {
Expand Down
24 changes: 21 additions & 3 deletions frontend/src/base/components/project-list/ProjectListCard.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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&nbsp;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
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

Prompt To Fix With AI
This 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.

Fix in Claude Code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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 (?project=<id>) which the Jobs page doesn't support yet. Will address when project-scoped filtering is added to the Jobs page.

Expand Down
Loading