Schedule report tasks and add report preview view #1620
Conversation
GregKaleka
force-pushed
the
gk/issue-1603
branch
from
e64f108 to
d529a7a
Compare
| email = person_data.pop("email") or generate_fake_email( | ||
| f"{person_data['first_name']} {person_data['last_name']}" | ||
| ) | ||
| if not (user := User.objects.filter(email=email).first()): |
There was a problem hiding this comment.
nitpick: the brackets here probably aren't needed?
There was a problem hiding this comment.
They are actually needed. You can try it yourself:
>>> if not foo := 0:
File "<stdin>", line 1
if not foo := 0:
^^^^^^^
SyntaxError: cannot use assignment expressions with expression
>>> if not (foo := 0):
... print("hello")
...
hello
|
|
||
| return handled_commits | ||
|
|
||
|
|
||
| def check_credentials() -> list[str]: |
There was a problem hiding this comment.
Small thing to think about rather than a blocker. I'm not sure about the naming of this. I think it could be something more intuitive maybe. The first impression from the use later would be that check_credentials would return a bool. Maybe something like list_missing_credentials would be more intuitive?
There was a problem hiding this comment.
I'm not set on that name either if you can think of something better.
versions/views.py
Outdated
|
|
||
|
|
||
| @method_decorator(staff_member_required, name="get") | ||
| class ReportPreviewView(View): |
There was a problem hiding this comment.
Is it worth using the BoostVersionMixin here to have centralized handling of that for consistency.
There was a problem hiding this comment.
It does appear to add two extra queries, but the page is so fast and is a preview for staff only anyway. Probably worth it to keep things a bit DRYer.
/releases/<version_slug>/reportNote: I have not implemented PDF rendering.
I can do so, but wanted to have some discussion about what library to use. I've used WeasyPrint in the past, and the results have been good, though I recall the dev experience being annoying (someone on the team was always having to install some supporting thing on their machine).Update: I attempted to get PDF rendering working with weasyprint. The results were close but I could not get some of the styling to work. In agreement with Rob, we're going to move forward with this html-only version for now, since it takes a lot of the manual work out already. We can revisit programmatic PDF rendering in the future.
Fixes #1603