Summary

Add dynamic site configuration to Review Board

Review Request #480 — Created July 30, 2008 and submitted

Information

Owner

chipx86

Repository

Review Board SVN (deprecated)

Branch

trunk

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

Up until now users have had to edit settings_local.py to customize their Review Board install. While we documented many of the options, it was still a pain for users as they'd have to log in and edit the file and then restart the server.

With the new djblets.siteconfig app, we can now move in a more modern direction for customization of Review Board. Aside from a few basic essential settings in settings_local.py (such as database configuration) the admin UI will now be the place to go to customize Review Board.

A migration script is provided to automatically migrate all settings into the new siteconfig database entry. The next time users run ./manage.py syncdb, the script will detect that the settings need to be migrated and handle it all. Everything we care about should be preserved, including authentication information.

Testing Done

I've tested this with brand new installs and with a few different migrated databases. I haven't hit any problems yet but I haven't actually tested the resulting settings for authentication. Users can specify NIS or LDAP servers and I know we save the information but before this goes in, I'll be testing on an actual install.

Screenshots

General Settings

LDAP

NIS

E-Mail Settings

Diff Viewer Settings

This looks awesome.  Do you have some screenshots of the UI?

chipx86 July 31, 2008, 8:19 p.m.
```
Will add some.
```

/trunk/reviewboard/accounts/views.py (Diff revision 1)
```
I don't think this line belongs here in the standard style for these.
```
1. chipx86 July 31, 2008, 8:18 p.m.
  Fixed.

/trunk/reviewboard/admin/forms.py (Diff revision 1)

Seems like the urlparse module is appropriate.  Note that if you do this, we need to use tuple indices instead of names, because the names were only added in python 2.5.

chipx86 July 31, 2008, 8:18 p.m.
```
Fixed.
```

/trunk/reviewboard/manage.py (Diff revision 1)

Instead of killing these, could you move them down and make them dependency_warning()s? This should let you remove your TODO by the forms for these too.

chipx86 July 31, 2008, 8:18 p.m.

It won't really because I want something that admins will see on Review Board itself, maybe at the top of the page when they log in.

It's also too soon to check for this because the database and settings aren't set up properly yet.

I'll put a change together soon that addresses this in another way. For now, people will just see that it doesn't work and will see the messages in the settings UI.

david Aug. 1, 2008, 2:50 p.m.

I don't mean you shouldn't keep the one in the settings stuff, just have a dependency warning here so people can run the checks and see everything at once.

chipx86 Aug. 1, 2008, 3:09 p.m.

I get that, but they wouldn't be dependent on the actual  settings anymore. We'd have to always show the warning if it doesn't exist, unlike the old code that checks first. Is that alright with you? I think it can be annoying since I don't use search on my local install and I really don't want to see it every time, so I think it should be setting-dependent but again we can't query for that at this stage.

david Aug. 1, 2008, 3:10 p.m.

Yeah.  We show similar warnings for things like perforce, even though not everyone will use it.

/trunk/reviewboard/accounts/urls.py (Diff revision 3)
```
Need to add file to accounts/Makefile.am
```

/trunk/reviewboard/admin/forms.py (Diff revision 3)

Need to add file to admin/Makefile.am

Same goes for other new files in this directory:

 - siteconfig.pc
 - management sub directory

/trunk/reviewboard/templates/admin/general_settings.html (Diff revision 3)

Needs to be added to templates/Makefile.am

Same goes for templates/admin/general_settings/html