-
-
Notifications
You must be signed in to change notification settings - Fork 72
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
Implement soft validation for member codes in training requests #2549
Implement soft validation for member codes in training requests #2549
Conversation
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, minor comments left.
amy/extforms/forms.py
Outdated
@@ -167,6 +171,10 @@ def set_accordion(self, layout: Layout) -> None: | |||
), | |||
) | |||
|
|||
def set_display_member_code_override(self, visible: bool) -> None: |
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.
For functions that accept int or boolean arguments I prefer to make them required as keyword by using *
in the function definition:
https://realpython.com/python-asterisk-and-slash-special-parameters/#in-short-the-python-asterisk-and-slash-control-how-to-pass-values-to-functions
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.
Oh interesting, I've never come across this syntax before. I see the appeal
amy/extforms/forms.py
Outdated
# confirm that membership is currently active | ||
# grace period: 90 days before and after | ||
if not membership.active_on_date( | ||
date.today(), grace_before=90, grace_after=90 | ||
): | ||
errors["member_code"] = ValidationError(error_msg) | ||
return False |
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.
I'd put just the line membership = Membership.objects.get(registration_code=code)
inside of the try-except block. The conditional if not membership.active_on_date(...)
could be outside of that block.
Fixes #2535 by using option 2 - adding a checkbox to the form to override the code validation, and changing the visibility as necessary.
May also need to be applied to
TrainingRequestUpdateForm
, but I need to check this with the instructor training team, so I'll make it a separate issue.