Summary

Create extension for auto-assigning students to the students group

Review Request #7856 — Created Jan. 8, 2016 and discarded Jan. 15, 2016, 1:19 p.m.

Information

Owner

mike_conley

Repository

rb-extension-pack

Branch

master

Bugs

Depends On

Reviewers

Groups

rb-extension-pack

People

Description

A simple extension for auto-assigning students working on Review Board to
the "students" group.

I can imagine this maybe being useful for other projects as well, so I can
see this expanding out to allow multiple auto-assignments, but for now I've
just hard-coded the "students" group.

Testing Done

Created some test users and a "students" group. Entered the extension
configuration page and added some users in the comma-separated list to
auto-assign. Created review requests for the users in the list and saw
that they were correctly added to the "students" group. The review requests
from the other users were not.

Issues

Description	From	Last Updated
'RBAutoAssigner' imported but unused	reviewbot	Jan. 8, 2016, 1:03 a.m.
'json' imported but unused	reviewbot	Jan. 8, 2016, 12:56 a.m.
Single quotes.	brennie	Jan. 8, 2016, 12:57 a.m.
Blank line here.	brennie	Jan. 8, 2016, 1:02 a.m.
Same here.	brennie	Jan. 8, 2016, 1:03 a.m.
And here.	brennie	Jan. 8, 2016, 1:03 a.m.
logging.info, etc will do % interpolation with positional arguments, so you can do: logging.info('foo %s', bar)	brennie	Jan. 8, 2016, 1:02 a.m.
Col: 80 E501 line too long (100 > 79 characters)	reviewbot	Jan. 8, 2016, 12:56 a.m.
Single quotes	brennie	Jan. 8, 2016, 1:03 a.m.
Prefer MANIFEST.in over package_data	brennie	Jan. 8, 2016, 1:03 a.m.
Instead of doing this on publish, what about on create?	david	Jan. 15, 2016, 12:19 p.m.
I don't think we care if they've joined the group or not.	david	Jan. 15, 2016, 12:26 p.m.
Would be better to do Group.objects.get(name=self.auto_assign_group) and catch Group.DoesNotExist	david	Jan. 15, 2016, 12:28 p.m.
Not possible.	david	Jan. 15, 2016, 12:28 p.m.
Limiting to 40 characters means we won't be able to put in very many usernames.	david	Jan. 15, 2016, 1:11 p.m.

Tool: PEP8 Style Checker
Processed Files:
    rbauto_assigner/rbauto_assigner/admin_urls.py
    rbauto_assigner/rbauto_assigner/extension.py
    rbauto_assigner/setup.py

Ignored Files:
    rbauto_assigner/rbauto_assigner/__init__.py



Tool: Pyflakes
Processed Files:
    rbauto_assigner/rbauto_assigner/admin_urls.py
    rbauto_assigner/rbauto_assigner/extension.py
    rbauto_assigner/setup.py

Ignored Files:
    rbauto_assigner/rbauto_assigner/__init__.py

rbauto_assigner/rbauto_assigner/admin_urls.py (Diff revision 1)
The issue has been resolved. Show all issues
```
 'RBAutoAssigner' imported but unused
```
rbauto_assigner/rbauto_assigner/extension.py (Diff revision 1)
The issue has been resolved. Show all issues
```
 'json' imported but unused
```
rbauto_assigner/setup.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 80
 E501 line too long (100 > 79 characters)
```

rbauto_assigner/rbauto_assigner/extension.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Single quotes.
```
rbauto_assigner/rbauto_assigner/extension.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Blank line here.
```
rbauto_assigner/rbauto_assigner/extension.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Same here.
```
rbauto_assigner/rbauto_assigner/extension.py (Diff revision 1)
The issue has been resolved. Show all issues
```
And here.
```

rbauto_assigner/rbauto_assigner/extension.py (Diff revision 1)

The issue has been resolved. Show all issues

logging.info, etc will do % interpolation with positional arguments, so you can do:

logging.info('foo %s',
             bar)

mike_conley Jan. 8, 2016, 1:03 a.m.
```
TIL!
```

rbauto_assigner/setup.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Single quotes
```
rbauto_assigner/setup.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Prefer MANIFEST.in over package_data
```

Change Summary:

I've made it so that the extension can be configured with a
comma-separated list of student usernames. This way, we don't
end up auto-assigning review requests from everybody in the
students group (since that could include mentors).

Testing Done:

~		Created some test users and a "students" group, putting some of the users
~		in that group. Created review requests for the users in the group and saw
	~	Created some test users and a "students" group. Entered the extension
	~	configuration page and added some users in the comma-separated list to
	+	auto-assign. Created review requests for the users in the list and saw
		that they were correctly added to the "students" group. The review requests
		from the other users were not.

Commit:

4f5d1ace5176226d1b1b01e715e524aae56b0db0

78dafb1bdf3fcc2e316968001891edd68a7eea96

Diff:

Revision 2 (+128)

Show changes

	rbauto_assigner/setup.py
	rbauto_assigner/rbauto_assigner/__init__.py
	rbauto_assigner/rbauto_assigner/admin_urls.py
	rbauto_assigner/rbauto_assigner/extension.py
	rbauto_assigner/rbauto_assigner/forms.py

Tool: PEP8 Style Checker
Processed Files:
    rbauto_assigner/rbauto_assigner/admin_urls.py
    rbauto_assigner/rbauto_assigner/extension.py
    rbauto_assigner/setup.py
    rbauto_assigner/rbauto_assigner/forms.py

Ignored Files:
    rbauto_assigner/rbauto_assigner/__init__.py

Fix it!

Given that this is currently highly student-specific (right now), instead of putting it in rb-extension-pack, how about in the misc/ directory of the student-sonar repository? We can always move it when/if we make it more generic.

rbauto_assigner/rbauto_assigner/extension.py (Diff revision 2)

An issue was opened. Show all issues

Instead of doing this on publish, what about on create?

mike_conley Jan. 15, 2016, 1:17 p.m.

I don't see a signal for review request creation. Should I add one in a separate review request?

david Jan. 15, 2016, 1:43 p.m.

There's no explicit creation signal, but you can listen to django.db.models.signals.pre_save for ReviewRequest and see if the pk is falsy.

rbauto_assigner/rbauto_assigner/extension.py (Diff revision 2)
The issue has been resolved. Show all issues
```
I don't think we care if they've joined the group or not.
```

rbauto_assigner/rbauto_assigner/extension.py (Diff revision 2)

The issue has been resolved. Show all issues

Would be better to do Group.objects.get(name=self.auto_assign_group) and catch Group.DoesNotExist

rbauto_assigner/rbauto_assigner/extension.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Not possible.
```
rbauto_assigner/rbauto_assigner/forms.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Limiting to 40 characters means we won't be able to put in very many usernames.
```

Status:: Discarded
Change Summary:: Re-posted for the student-sonar repo at https://reviews.reviewboard.org/r/7872/