diff --git a/lms/djangoapps/teams/static/teams/js/collections/team_membership.js b/lms/djangoapps/teams/static/teams/js/collections/team_membership.js index 9478d65d60..bfb29bffc4 100644 --- a/lms/djangoapps/teams/static/teams/js/collections/team_membership.js +++ b/lms/djangoapps/teams/static/teams/js/collections/team_membership.js @@ -10,6 +10,7 @@ this.perPage = options.per_page || 10; this.username = options.username; this.privileged = options.privileged; + this.staff = options.staff; this.server_api = _.extend( { @@ -26,11 +27,11 @@ model: TeamMembershipModel, canUserCreateTeam: function() { - // Note: non-privileged users are automatically added to any team + // Note: non-staff and non-privileged users are automatically added to any team // that they create. This means that if multiple team membership is // disabled that they cannot create a new team when they already // belong to one. - return this.privileged || this.length === 0; + return this.privileged || this.staff || this.length === 0; } }); return TeamMembershipCollection; diff --git a/lms/djangoapps/teams/static/teams/js/spec/teams_tab_factory_spec.js b/lms/djangoapps/teams/static/teams/js/spec/teams_tab_factory_spec.js index ee7337e75a..cab815b09a 100644 --- a/lms/djangoapps/teams/static/teams/js/spec/teams_tab_factory_spec.js +++ b/lms/djangoapps/teams/static/teams/js/spec/teams_tab_factory_spec.js @@ -15,6 +15,7 @@ define(["jquery", "backbone", "teams/js/teams_tab_factory"], userInfo: { username: 'test-user', privileged: false, + staff: false, team_memberships_data: null } }); diff --git a/lms/djangoapps/teams/static/teams/js/spec/views/edit_team_spec.js b/lms/djangoapps/teams/static/teams/js/spec/views/edit_team_spec.js index 505de68844..f10d4b41ca 100644 --- a/lms/djangoapps/teams/static/teams/js/spec/views/edit_team_spec.js +++ b/lms/djangoapps/teams/static/teams/js/spec/views/edit_team_spec.js @@ -173,7 +173,7 @@ define([ AjaxHelpers.respondWithError( requests, 400, - {'error_message': {'user_message': 'User message', 'developer_message': 'Developer message' }} + {'user_message': 'User message', 'developer_message': 'Developer message'} ); expect(teamEditView.$('.wrapper-msg .copy').text().trim()).toBe("User message"); diff --git a/lms/djangoapps/teams/static/teams/js/spec/views/teams_tab_spec.js b/lms/djangoapps/teams/static/teams/js/spec/views/teams_tab_spec.js index 2daf6fd0b8..83fccf20c5 100644 --- a/lms/djangoapps/teams/static/teams/js/spec/views/teams_tab_spec.js +++ b/lms/djangoapps/teams/static/teams/js/spec/views/teams_tab_spec.js @@ -130,7 +130,7 @@ define([ it('does not allow access if the user is neither privileged nor a team member', function () { var teamsTabView = createTeamsTabView({ - userInfo: TeamSpecHelpers.createMockUserInfo({ privileged: false }) + userInfo: TeamSpecHelpers.createMockUserInfo({ privileged: false, staff: true }) }); expect(teamsTabView.readOnlyDiscussion({ attributes: { membership: [] } diff --git a/lms/djangoapps/teams/static/teams/js/spec/views/topic_teams_spec.js b/lms/djangoapps/teams/static/teams/js/spec/views/topic_teams_spec.js index e923eb7280..415ada244c 100644 --- a/lms/djangoapps/teams/static/teams/js/spec/views/topic_teams_spec.js +++ b/lms/djangoapps/teams/static/teams/js/spec/views/topic_teams_spec.js @@ -100,6 +100,15 @@ define([ verifyActions(teamsView); }); + it('shows actions for a staff user already in a team', function () { + var staffMembership = TeamSpecHelpers.createMockTeamMemberships( + TeamSpecHelpers.createMockTeamMembershipsData(1, 5), + { privileged: false, staff: true } + ), + teamsView = createTopicTeamsView({ teamMemberships: staffMembership }); + verifyActions(teamsView); + }); + /* // TODO: make this ready for prime time it('refreshes when the team membership changes', function() { diff --git a/lms/djangoapps/teams/static/teams/js/spec_helpers/team_spec_helpers.js b/lms/djangoapps/teams/static/teams/js/spec_helpers/team_spec_helpers.js index f5b51c63fb..50a4f0841f 100644 --- a/lms/djangoapps/teams/static/teams/js/spec_helpers/team_spec_helpers.js +++ b/lms/djangoapps/teams/static/teams/js/spec_helpers/team_spec_helpers.js @@ -89,7 +89,8 @@ define([ parse: true, url: 'api/teams/team_memberships', username: testUser, - privileged: false + privileged: false, + staff: false }), options) ); @@ -100,6 +101,7 @@ define([ { username: testUser, privileged: false, + staff: false, team_memberships_data: createMockTeamMembershipsData(1, 5) }, options diff --git a/lms/djangoapps/teams/static/teams/js/views/edit_team.js b/lms/djangoapps/teams/static/teams/js/views/edit_team.js index acc2ad82d7..ff12c86d4a 100644 --- a/lms/djangoapps/teams/static/teams/js/views/edit_team.js +++ b/lms/djangoapps/teams/static/teams/js/views/edit_team.js @@ -125,9 +125,9 @@ }) .fail(function(data) { var response = JSON.parse(data.responseText); - var message = gettext("An error occurred. Please try again.") - if ('error_message' in response && 'user_message' in response['error_message']){ - message = response['error_message']['user_message']; + var message = gettext("An error occurred. Please try again."); + if ('user_message' in response){ + message = response.user_message; } view.showMessage(message, message); }); diff --git a/lms/djangoapps/teams/static/teams/js/views/teams_tab.js b/lms/djangoapps/teams/static/teams/js/views/teams_tab.js index 52d4830b7c..d8d56374c4 100644 --- a/lms/djangoapps/teams/static/teams/js/views/teams_tab.js +++ b/lms/djangoapps/teams/static/teams/js/views/teams_tab.js @@ -92,6 +92,7 @@ course_id: this.courseID, username: this.userInfo.username, privileged: this.userInfo.privileged, + staff: this.userInfo.staff, parse: true } ).bootstrap(); diff --git a/lms/djangoapps/teams/tests/test_views.py b/lms/djangoapps/teams/tests/test_views.py index df5d4a866d..088a3677f7 100644 --- a/lms/djangoapps/teams/tests/test_views.py +++ b/lms/djangoapps/teams/tests/test_views.py @@ -227,6 +227,14 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase): self.test_team_5.add_user(self.users['student_enrolled_both_courses_other_team']) self.test_team_6.add_user(self.users['student_enrolled_public_profile']) + def build_membership_data_raw(self, username, team): + """Assembles a membership creation payload based on the raw values provided.""" + return {'username': username, 'team_id': team} + + def build_membership_data(self, username, team): + """Assembles a membership creation payload based on the username and team model provided.""" + return self.build_membership_data_raw(self.users[username].username, team.team_id) + def create_and_enroll_student(self, courses=None, username=None): """ Creates a new student and enrolls that student in the course. @@ -515,14 +523,38 @@ class TestCreateTeamAPI(TeamAPITestCase): self.post_create_team(status, data) def test_student_in_team(self): - self.post_create_team( + response = self.post_create_team( 400, - { - 'course_id': str(self.test_course_1.id), - 'description': "You are already on a team in this course." - }, + data=self.build_team_data( + name="Doomed team", + course=self.test_course_1, + description="Overly ambitious student" + ), user='student_enrolled' ) + self.assertEqual( + "You are already in a team in this course.", + json.loads(response.content)["user_message"] + ) + + @ddt.data('staff', 'course_staff', 'community_ta') + def test_privileged_create_multiple_teams(self, user): + """ Privileged users can create multiple teams, even if they are already in one. """ + # First add the privileged user to a team. + self.post_create_membership( + 200, + self.build_membership_data(user, self.test_team_1), + user=user + ) + + self.post_create_team( + data=self.build_team_data( + name="Another team", + course=self.test_course_1, + description="Privileged users are the best" + ), + user=user + ) @ddt.data({'description': ''}, {'name': 'x' * 1000}, {'name': ''}) def test_bad_fields(self, kwargs): @@ -885,14 +917,6 @@ class TestListMembershipAPI(TeamAPITestCase): class TestCreateMembershipAPI(TeamAPITestCase): """Test cases for the membership creation endpoint.""" - def build_membership_data_raw(self, username, team): - """Assembles a membership creation payload based on the raw values provided.""" - return {'username': username, 'team_id': team} - - def build_membership_data(self, username, team): - """Assembles a membership creation payload based on the username and team model provided.""" - return self.build_membership_data_raw(self.users[username].username, team.team_id) - @ddt.data( (None, 401), ('student_inactive', 401), diff --git a/lms/djangoapps/teams/views.py b/lms/djangoapps/teams/views.py index 9e5c113983..59f1387706 100644 --- a/lms/djangoapps/teams/views.py +++ b/lms/djangoapps/teams/views.py @@ -96,9 +96,13 @@ class TeamsDashboardView(View): context = { "course": course, "topics": topics_serializer.data, + # It is necessary to pass both privileged and staff because only privileged users can + # administer discussion threads, but both privileged and staff users are allowed to create + # multiple teams (since they are not automatically added to teams upon creation). "user_info": { "username": user.username, "privileged": has_discussion_privileges(user, course_key), + "staff": bool(has_access(user, 'staff', course_key)), "team_memberships_data": team_memberships_serializer.data, }, "topic_url": reverse( @@ -372,14 +376,16 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView): 'field_errors': field_errors, }, status=status.HTTP_400_BAD_REQUEST) - if CourseTeamMembership.user_in_team_for_course(request.user, course_key): + # Course and global staff, as well as discussion "privileged" users, will not automatically + # be added to a team when they create it. They are allowed to create multiple teams. + team_administrator = (has_access(request.user, 'staff', course_key) + or has_discussion_privileges(request.user, course_key)) + if not team_administrator and CourseTeamMembership.user_in_team_for_course(request.user, course_key): error_message = build_api_error( ugettext_noop('You are already in a team in this course.'), course_id=course_id ) - return Response({ - 'error_message': error_message, - }, status=status.HTTP_400_BAD_REQUEST) + return Response(error_message, status=status.HTTP_400_BAD_REQUEST) if course_key and not has_team_api_access(request.user, course_key): return Response(status=status.HTTP_403_FORBIDDEN) @@ -396,8 +402,7 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView): }, status=status.HTTP_400_BAD_REQUEST) else: team = serializer.save() - if not (has_access(request.user, 'staff', course_key) - or has_discussion_privileges(request.user, course_key)): + if not team_administrator: # Add the creating user to the team. team.add_user(request.user) return Response(CourseTeamSerializer(team).data)