Grant admin rights to members of the course team #470
Conversation
| self.inst_groupname = get_course_groupname_for_role(self.course.location, "instructor") | ||
|
|
||
| def test_index(self): | ||
| resp = self.client.get(self.index_url) |
There was a problem hiding this comment.
What is this testing? I'm not clear what self.index_url is supposed to signify.
There was a problem hiding this comment.
It's the URL for the manage_users page. Is there a clearer name I can use?
There was a problem hiding this comment.
manage_users_url?
I was just confused because I thought you were trying to test the index page (ie, index.html, which is what our dashboard page is). I saw that you had it set to manage_users, but then I couldn't figure out what you were trying to test. You are checking that the user is not on the course initially?
There was a problem hiding this comment.
The "index/detail pages" pattern is very common in web development. An index page provides a list view of many resources, while a detail page goes into detail about one specific resource. I would have changed the name of the view, but that would have required changing it all over the site, and I wasn't sure that was a good idea.
The testing ambiguity is addressed in 0dbb458.
| return JsonResponse(msg, 400) | ||
|
|
||
| # remove leading/trailing whitespace if necessary | ||
| email = email.strip() |
There was a problem hiding this comment.
Don't lose Peter's bug fix. Having a leading space on an e-mail address doesn't seem to work on your branch.
Although I also can't add users in general on your branch (I get an error).
|
A few things--
|
Match other views better, saner URLs, more RESTful style, extensible for other roles
It was causing unit tests to fail, and it's a needless bit of abstraction that never should have existed in the first place.
A user with `is_staff=True` is treated as being in all groups. This is problematic when we care about the user's staff/instructor role for a course: you can't remove the instructor role. This commit changes the `is_user_in_course_group_role` function to allow the caller to specify that it should not check the `is_staff` attribute on the user.
| staff_group, __ = Group.objects.get_or_create(name=staff_groupname) | ||
| inst_groupname = get_course_groupname_for_role(location, "instructor") | ||
| inst_group, __ = Group.objects.get_or_create(name=inst_groupname) | ||
|
|
There was a problem hiding this comment.
Just curious, is there a reason double underscores are used here?
There was a problem hiding this comment.
Yes; because of the line from django.utils.translation import ugettext as _ at the top of this file. Single underscore is an alias for the ugettext function, so if we used a single underscore here, it would overwrite that alias.
|
If the filler text in manage_users.html is OK, 👍. |
| <h3 class="title">${_("Add a User to Your Course's Team")}</h3> | ||
|
|
||
| <fieldset class="form-fields"> | ||
| <legend class="sr">${_("Textbook information")}</legend> |
There was a problem hiding this comment.
What should this be? It probably shouldn't be "Textbook information".
There was a problem hiding this comment.
D'oh! That's an artifact from copying and pasting a design pattern from the Textbook UI. I'll fix that up quickly.
|
@singingwolfboy Don't forget to add a manual BDD spec for our manual test plans (see https://edx-wiki.atlassian.net/wiki/display/STU/Studio+BDD+Specs+index?src=search). In particular, highlight anything that is not covered by an automated test. If everything is covered, you can use your Lettuce BDD spec. |
Grant admin rights to members of the course team
ensuring pystache>=0.5.2
API call to display 'is_staff' attribute
…s-header-logo Enable to configure the header logo openedx#470
…nt_response_report_for_content_libs Add support for content lib problems in student response report.
fix(account-setting page): add a link to manual page
'make dev.provision' was trying to provision the 'o' service, due strangeness related to the fact that Makefiles were designed for compiling C programs (which have .o intermediate files). Fix the issue by adding all rules to PHONIES, which tells Make that none of the targets are real files. Co-authored-by: Tim McCormack <[email protected]>
https://edx-wiki.atlassian.net/browse/STUD-327