-
Notifications
You must be signed in to change notification settings - Fork 8
Made changes suggested by Nina #29
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
base: develop
Are you sure you want to change the base?
Conversation
gitumarkk
commented
Jul 22, 2013
- Added google forms and removed registration code.
- Made html edits.
- Did south migrations.
- Changed string formatting in views.py and form.py - Removed spacing around = in registration.html and contact.html
- Made digits css class in RegistrationForm one word - Made changes to html indenting in contact.html and registration.html - Made changes to send_email_f() in views to have a key, value paring
umonya/requirements.txt
Outdated
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.
alphabetical order.
The South stuff isn't in here?
umonya/umonya/apps/main/admin.py
Outdated
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 Dynamic_Section being used for?
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.
@ninas to check if section is enabled, the menu and the registration page depends on it.
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.
Why can't this functionality be covered in Registration?
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.
@ninas now that the registration form depends only on one field in the django admin, will do that
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 where the comment disappeared to.
Ok, so looking closer at this code - your indentation is wrong (if is in the for)
Also, this data is filled in server side - i.e. user hasn't touched this yet, what field.errors are you expecting?
Any reason to use something more than:
{% csrf_token %} {{ form.as_p }}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.
@ninas I assume that javascript will not be working all the time for all users hence server side validation serves as a back up, I think it provides significant benefits.
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.
So this code is converted into html BEFORE the data is sent to the user's browser to be rendered. I.e. this becomes static html. The fields you're using can be used to display client-side validation errors.
https://docs.djangoproject.com/en/dev/topics/forms/#customizing-the-form-template
You should definitely have server side validation, but this isn't it.
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.
@ninas basically what is happening initially from what I understand when the form gets rendered initially, the errors defaults to false, then as the user is filling in data if any is wrong the javascript catches this error, however if for some reason the javascript is broken i.e. disabled in the browser, form validation is repeated in the views.py file i.e. form.validate() function, if this returns false, the form is re-rendered this time the error defaults to true and the required error message is given (but I could be wrong in that case I will remove it).
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.
Have you tested this functionality?
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.
@ninas it was what was validating the forms before the javascript was introduced, and it is also what is working in the unittests (https://github.com/gitumarkk/DjangoWebsite/blob/develop/umonya/umonya/apps/main/tests/test_forms.py)
a form object is not a valid URL and therefore gives an error
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.
form_url
Make this a URL field...
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.
Where is the ability to specify registration is open or closed?
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.
@ninas <iframe> causes validation errors and doesn't save in database as I think the URLField is meant for http//: urls If there is no form code in database registration is closed.
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.
All that's going to change is the url - you don't need to include the whole iframe html stuff as well - just change which url is used by the iframe, and store only that. Does that make sense?
Also: http://stackoverflow.com/questions/8778416/what-are-the-valid-values-for-a-django-url-field