-
Notifications
You must be signed in to change notification settings - Fork 0
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
[SC-25] and [SC-26] User model updates and form updates #24
base: main
Are you sure you want to change the base?
Conversation
@@ -17,6 +17,8 @@ class User(AbstractBaseUser, PermissionsMixin): | |||
) | |||
first_name = models.CharField(_("first name"), max_length=150, blank=True) | |||
last_name = models.CharField(_("last name"), max_length=150, blank=True) | |||
phone_number = models.CharField(max_length=15, blank=True, null=True) | |||
total_donations = models.DecimalField(max_digits=10, decimal_places=2, default=0.00) |
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 believe Joshua created a total_donations field already for one of his tickets so this might not be needed
|
||
def validate(self, attrs): | ||
if attrs.get("password1") != attrs.get("password2"): | ||
raise serializers.ValidationError({"password": "Passwords do not match"}) | ||
|
||
if not attrs.get("phone_number").isdigit() or len(attrs.get("phone_number")) < 10: |
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 have not used the react library here, but does the form validation in the frontend avoid dashes between numbers i.e 416-523-..... or can only raw numbers be put in?
superuser = User.objects.create_superuser( | ||
email="[email protected]", | ||
password="adminpassword", | ||
phone_number="+1987654321" |
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.
Shouldn't this test not be valid since it contains a non digit char (+) and also is longer than 10 chars? I'm more so asking because this is what would be restricted in practice with the existing serializer
}; | ||
|
||
useEffect(() => { | ||
axios.get("https://ipapi.co/json/") |
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.
what is this?
User model should be updated now to have phone numbers as mandatory.
Tests ran: (check test file) and phone number appears in shell.
Forms should check for a valid phonenumber, email, names, and strong password (i made up the requirements for that one)/
Charity signup form shows errors "onChange" because it was annoying otherwise.
Register shows errors after sumbit. I can make both these forms have a consistent error display mode (idk what its called) if necessary.
Signup page i think should be validated on the backend?
Donate I thought we were "repurposing" so no i did not do that form.