-
Notifications
You must be signed in to change notification settings - Fork 6
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
Application submission management clarity #781
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #781 +/- ##
==========================================
- Coverage 72.67% 72.64% -0.04%
==========================================
Files 32 32
Lines 7078 7091 +13
==========================================
+ Hits 5144 5151 +7
- Misses 1934 1940 +6 ☔ View full report in Codecov by Sentry. |
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.
LGTM, thanks for taking the initiative on this 🐐
if skip["pending"]: | ||
skip_msg += f" {skip['pending']} still marked as pending" | ||
if skip["no_reason"]: | ||
skip_msg += f" {skip['no_reason']} have no reason provided" | ||
return Response( | ||
{ | ||
"detail": f"{dry_run_msg} emails to {n} people, " | ||
f"skipping {skip} due to one of (already notified, no reason, no email)" | ||
f"{skip_msg if skip_msg else ''}", |
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.
Not sure how this message shows up on the frontend (guessing it's a toast), but are these going to render as on the same line? Might be good to add newlines (or some sort of delimiter) between the messages if so.
Some QoL improvements to the application submission management flow for club officers, aiming at reducing user error and increasing clarity for the application decision sending process:
/application
route returns a proper X not found screen instead of an empty one