Summary

Added support for multiple admins

Review Request #2670 — Created Oct. 23, 2011 and discarded April 23, 2014, 3:54 p.m.

Information

Owner

Repository

Review Board

Branch

Bugs

Depends On

Reviewers

Groups

People

Description

* Admins names and emails can be added and removed through admin UI.
* New User registration emails will send to all admins.

This patch go with another patch in djblet:
http://reviews.reviewboard.org/r/2671/

Testing Done

Unit tested. The result looks good except some failure due to Windows environment.
Manually test:
* Manually added and removed admins in FF, Chrome, and IE. Worked as expected.
* Ran "prepare-dev.py", admins settings was migrated properly.
* Registered a new user, email notifications sent to all admins.

Screenshots

Files

Issues

Description	From	Last Updated
This would just be good as "Name."	chipx86	Nov. 6, 2011, 9:35 a.m.
"E-mail Address"	chipx86	Nov. 6, 2011, 9:35 a.m.
"Delete"	chipx86	Nov. 6, 2011, 9:35 a.m.
"Add" Actually, it'd be nice to have this be somewhat dynamic so that there's always one more row of blank …	chipx86	Nov. 6, 2011, 9:35 a.m.
Should derive from object.	chipx86	Nov. 6, 2011, 9:37 a.m.
Should be initial_data instead of initVals. No CamelCase.	chipx86	Nov. 6, 2011, 9:37 a.m.
This can instead be: initial_data = [ { 'site_admin_name': admin[0], 'site_admin_email': admin[1], } for admin in admins ]	chipx86	Nov. 6, 2011, 9:37 a.m.
All one line.	chipx86	Nov. 6, 2011, 9:37 a.m.
All one line.	chipx86	Nov. 6, 2011, 9:37 a.m.
Blank line between these. The setting of "admins" should probably be grouped with the save call.	chipx86	Nov. 6, 2011, 9:37 a.m.
Blank line between the block and apply_setting. Should group the two apply_settings together.	chipx86	Nov. 6, 2011, 9:37 a.m.
SSHSettingsForm should be indented along with AdminsFormSetManager.	chipx86	Nov. 6, 2011, 9:37 a.m.
Store site_config separately and pass it in. It'll make for nicer code.	chipx86	Nov. 6, 2011, 9:37 a.m.
Is this part of this change?	chipx86	Nov. 6, 2011, 9:38 a.m.
Should be localized.	chipx86	Nov. 6, 2011, 9:38 a.m.
No space in {{variables}}	chipx86	Nov. 6, 2011, 9:38 a.m.
the href should be "#". javascript: URLs are deprecated and not even supported in modern browsers. Also, localize the text.	chipx86	Nov. 6, 2011, 9:38 a.m.
Same comments about href and localization.	chipx86	Nov. 6, 2011, 9:38 a.m.

I'm going to be very happy when this change is in. :) Some style and text stuff first.

The issue has been resolved. Show all issues
```
This would just be good as "Name."
```
The issue has been resolved. Show all issues
```
"E-mail Address"
```
The issue has been resolved. Show all issues
```
"Delete"
```

The issue has been resolved. Show all issues

"Add"

Actually, it'd be nice to have this be somewhat dynamic so that there's always one more row of blank values. That'd remove the need for "Add" and instead there'd just be fields to type into. Much better user experience.

HO

hongbin Nov. 6, 2011, 9:42 a.m.

Did that, but the drawback is the delete. User need to delete the target admin as well as the blank form.....

reviewboard/admin/forms.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Should derive from object.
```
reviewboard/admin/forms.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Should be initial_data instead of initVals. No CamelCase.
```

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

The issue has been resolved. Show all issues

This can instead be:

initial_data = [
    {
        'site_admin_name': admin[0],
        'site_admin_email': admin[1],
    }
    for admin in admins
]

reviewboard/admin/forms.py (Diff revision 1)
The issue has been resolved. Show all issues
```
All one line.
```
reviewboard/admin/forms.py (Diff revision 1)
The issue has been resolved. Show all issues
```
All one line.
```

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

The issue has been resolved. Show all issues

Blank line between these.

The setting of "admins" should probably be grouped with the save call.

reviewboard/admin/siteconfig.py (Diff revision 1)

The issue has been resolved. Show all issues

Blank line between the block and apply_setting. Should group the two apply_settings together.

reviewboard/admin/views.py (Diff revision 1)
The issue has been resolved. Show all issues
```
SSHSettingsForm should be indented along with AdminsFormSetManager.
```
reviewboard/admin/views.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Store site_config separately and pass it in. It'll make for nicer code.
```
reviewboard/diffviewer/diffutils.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Is this part of this change?
```
reviewboard/templates/admin/site_admins_settings.html (Diff revision 1)
The issue has been resolved. Show all issues
```
Should be localized.
```
reviewboard/templates/admin/site_admins_settings.html (Diff revision 1)
The issue has been resolved. Show all issues
```
No space in {{variables}}
```

reviewboard/templates/admin/site_admins_settings.html (Diff revision 1)

The issue has been resolved. Show all issues

the href should be "#".

javascript: URLs are deprecated and not even supported in modern browsers.

Also, localize the text.

reviewboard/templates/admin/site_admins_settings.html (Diff revision 1)
The issue has been resolved. Show all issues
```
Same comments about href and localization.
```

HO

Change Summary:

Thanks for review. Changed according to comments of the review.

Description:

		Admins names and emails can be added and removed through admin UI.
		New User registration emails will send to all admins.
-		Imported "with" statement in order to fix an syntax error, when RB runs on python 2.5.

		This patch go with another patch in djblet:
		http://reviews.reviewboard.org/r/2671/

Diff:

Revision 2 (+190 -30)

	reviewboard/admin/forms.py
	reviewboard/admin/siteconfig.py
	reviewboard/admin/urls.py
	reviewboard/admin/views.py
	reviewboard/admin/management/sites.py
	reviewboard/cmdline/rbsite.py
	reviewboard/htdocs/media/rb/css/admin.css
	reviewboard/notifications/tests.py
	3 more

Screenshots:

HO

Status: Discarded

Loading...