2797: account preferences page fails silently when using an auth backend that does not support timezone change

kch***@gmai***** (Google Code) (Is this you? Claim this profile.)
david
david
Nov. 18, 2012
What version are you running?
1.7beta2

What's the URL of the page containing the problem?
account/preferences/

What steps will reproduce the problem?
1. Set up a review board server with activedirectory auth backend (or any other backend that does not have supports_change_timezone = True)
2. Login
3. Go to the account/preferences page
4. Change any setting and click "Save Preferences"
5. The save silently fails, and the form is displayed again with no visible errors and no hint that it failed.
6. Go to a different page (such as the dashboard)
7. return to account/preferences
   The setting(s) changed in step 4 have reverted.

What is the expected output? What do you see instead?
The changes should save.

What operating system are you using? What browser?
Chrome and IE8 on XP

Please provide any additional information below.

The timezone field of the account preferences form (accounts/forms.py:34) is marked as required, but the template (templates/accounts/prefs.html:39)does not display it if the auth backend does not have supports_change_timezone=True

If the user tries to save changes to the preference page, the validation fails because the timezone field is not present.  The error is not visible because the field is not rendered.

I worked around this by adding supports-change_timezone = True to the ActiveDirectoryBackend class. (not sure if this breaks anything, but timezone changes do appear to be working with this backend).

To fix for all backends regardless of capability, I suggest changing timezone to be an optional field in PreferencesForm.
#1 kch***@gmai***** (Google Code) (Is this you? Claim this profile.)
Forgot to mention that the line numbers in the report are based on the current master branch, not the tagged 1.7beta2
david
#2 david
The timezone is just part of the user profile and isn't connected to the auth backend, so there's really no reason for us to ever hide the field. I'll fix this.
  • +Started
  • +david
david
#3 david
Fixed in master (eb21d1f). Thanks!
  • -Started
    +Fixed