Skip to content
This repository was archived by the owner on Mar 28, 2025. It is now read-only.

feat: Admin improvements #14

Merged
merged 2 commits into from
Dec 5, 2024
Merged

feat: Admin improvements #14

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
21b2cec
feat: Admin improvements
khvn26 Nov 22, 2024
ae9b71f
fix imports, add field to list display
khvn26 Dec 4, 2024
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
18 changes: 18 additions & 0 deletions task_processor/admin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
from datetime import datetime

from django.contrib import admin
from django.db.models import QuerySet
from django.http import HttpRequest

from task_processor.models import RecurringTask

Expand All @@ -10,6 +14,7 @@ class RecurringTaskAdmin(admin.ModelAdmin):
"task_identifier",
"run_every",
"last_run_status",
"last_run_finished_at",
"is_locked",
)
readonly_fields = ("args", "kwargs")
Expand All @@ -18,3 +23,16 @@ def last_run_status(self, instance: RecurringTask) -> str | None:
if last_run := instance.task_runs.order_by("-started_at").first():
return last_run.result
return None

def last_run_finished_at(self, instance: RecurringTask) -> datetime | None:
if last_run := instance.task_runs.order_by("-started_at").first():
return last_run.finished_at
return None
Comment on lines +27 to +30
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the intention here? To add this to the list view, or just the detail view? At the moment, I'm not sure it's added to either? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd also be reluctant to add this to the list display as it probably won't perform that well being run for all the records in the view.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, added the field to list_display.

I'd also be reluctant to add this to the list display as it probably won't perform that well being run for all the records in the view.

A similiar query is already run for all the records in the view — see last_run_status. I wouldn't worry too much about performance as the recurring task list is fairly constant.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, fair enough. I guess there's an argument to say that we could cache the last run between the 2 methods here, but maybe that's over optimisation at this stage.


@admin.action(description="Unlock selected tasks")
def unlock(
self,
request: HttpRequest,
queryset: QuerySet[RecurringTask],
) -> None:
queryset.update(is_locked=False)
Loading