+
diff --git a/lms/djangoapps/teams/tests/test_views.py b/lms/djangoapps/teams/tests/test_views.py
index 832bfa8832..45fb2512b8 100644
--- a/lms/djangoapps/teams/tests/test_views.py
+++ b/lms/djangoapps/teams/tests/test_views.py
@@ -21,6 +21,7 @@ from util.testing import EventTestMixin
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
from .factories import CourseTeamFactory, LAST_ACTIVITY_AT
+from ..models import CourseTeamMembership
from ..search_indexes import CourseTeamIndexer, CourseTeam, course_team_post_save_callback
from django_comment_common.models import Role, FORUM_ROLE_COMMUNITY_TA
@@ -339,6 +340,10 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase):
"""Gets detailed team information for team_id. Verifies expected_status."""
return self.make_call(reverse('teams_detail', args=[team_id]), expected_status, 'get', data, **kwargs)
+ def delete_team(self, team_id, expected_status, **kwargs):
+ """Delete the given team. Verifies expected_status."""
+ return self.make_call(reverse('teams_detail', args=[team_id]), expected_status, 'delete', **kwargs)
+
def patch_team_detail(self, team_id, expected_status, data=None, **kwargs):
"""Patches the team with team_id using data. Verifies expected_status."""
return self.make_call(
@@ -384,12 +389,8 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase):
def delete_membership(self, team_id, username, expected_status=200, **kwargs):
"""Deletes an individual membership record. Verifies expected_status."""
- return self.make_call(
- reverse('team_membership_detail', args=[team_id, username]),
- expected_status,
- 'delete',
- **kwargs
- )
+ url = reverse('team_membership_detail', args=[team_id, username]) + '?admin=true'
+ return self.make_call(url, expected_status, 'delete', **kwargs)
def verify_expanded_public_user(self, user):
"""Verifies that fields exist on the returned user json indicating that it is expanded."""
@@ -437,8 +438,9 @@ class TestListTeamsAPI(EventTestMixin, TeamAPITestCase):
def verify_names(self, data, status, names=None, **kwargs):
"""Gets a team listing with data as query params, verifies status, and then verifies team names if specified."""
teams = self.get_teams_list(data=data, expected_status=status, **kwargs)
- if names:
- self.assertEqual(names, [team['name'] for team in teams['results']])
+ if names is not None and 200 <= status < 300:
+ results = teams['results']
+ self.assertEqual(names, [team['name'] for team in results])
def test_filter_invalid_course_id(self):
self.verify_names({'course_id': 'no_such_course'}, 400)
@@ -562,6 +564,26 @@ class TestListTeamsAPI(EventTestMixin, TeamAPITestCase):
user='student_enrolled_public_profile'
)
+ def test_delete_removed_from_search(self):
+ team = CourseTeamFactory.create(
+ name=u'zoinks',
+ course_id=self.test_course_1.id,
+ topic_id='topic_0'
+ )
+ self.verify_names(
+ {'course_id': self.test_course_1.id, 'text_search': 'zoinks'},
+ 200,
+ [team.name],
+ user='staff'
+ )
+ team.delete()
+ self.verify_names(
+ {'course_id': self.test_course_1.id, 'text_search': 'zoinks'},
+ 200,
+ [],
+ user='staff'
+ )
+
@ddt.ddt
class TestCreateTeamAPI(EventTestMixin, TeamAPITestCase):
@@ -776,6 +798,60 @@ class TestDetailTeamAPI(TeamAPITestCase):
self.verify_expanded_public_user(result['membership'][0]['user'])
+@ddt.ddt
+class TestDeleteTeamAPI(EventTestMixin, TeamAPITestCase):
+ """Test cases for the team delete endpoint."""
+
+ def setUp(self): # pylint: disable=arguments-differ
+ super(TestDeleteTeamAPI, self).setUp('teams.views.tracker')
+
+ @ddt.data(
+ (None, 401),
+ ('student_inactive', 401),
+ ('student_unenrolled', 403),
+ ('student_enrolled', 403),
+ ('staff', 204),
+ ('course_staff', 204),
+ ('community_ta', 204)
+ )
+ @ddt.unpack
+ def test_access(self, user, status):
+ self.delete_team(self.solar_team.team_id, status, user=user)
+ if status == 204:
+ self.assert_event_emitted(
+ 'edx.team.deleted',
+ team_id=self.solar_team.team_id,
+ course_id=unicode(self.test_course_1.id)
+ )
+ self.assert_event_emitted(
+ 'edx.team.learner_removed',
+ team_id=self.solar_team.team_id,
+ course_id=unicode(self.test_course_1.id),
+ remove_method='team_deleted',
+ user_id=self.users['student_enrolled'].id
+ )
+
+ def test_does_not_exist(self):
+ self.delete_team('nonexistent', 404)
+
+ def test_memberships_deleted(self):
+ self.assertEqual(CourseTeamMembership.objects.filter(team=self.solar_team).count(), 1)
+ self.delete_team(self.solar_team.team_id, 204, user='staff')
+ self.assert_event_emitted(
+ 'edx.team.deleted',
+ team_id=self.solar_team.team_id,
+ course_id=unicode(self.test_course_1.id)
+ )
+ self.assert_event_emitted(
+ 'edx.team.learner_removed',
+ team_id=self.solar_team.team_id,
+ course_id=unicode(self.test_course_1.id),
+ remove_method='team_deleted',
+ user_id=self.users['student_enrolled'].id
+ )
+ self.assertEqual(CourseTeamMembership.objects.filter(team=self.solar_team).count(), 0)
+
+
@ddt.ddt
class TestUpdateTeamAPI(EventTestMixin, TeamAPITestCase):
"""Test cases for the team update endpoint."""
@@ -1299,17 +1375,32 @@ class TestDeleteMembershipAPI(EventTestMixin, TeamAPITestCase):
)
if status == 204:
- remove_method = 'self_removal' if user == 'student_enrolled' else 'removed_by_admin'
self.assert_event_emitted(
'edx.team.learner_removed',
team_id=self.solar_team.team_id,
course_id=unicode(self.solar_team.course_id),
user_id=self.users['student_enrolled'].id,
- remove_method=remove_method
+ remove_method='removed_by_admin'
)
else:
self.assert_no_events_were_emitted()
+ def test_leave_team(self):
+ """
+ The key difference between this test and test_access above is that
+ removal via "Edit Membership" and "Leave Team" emit different events
+ despite hitting the same API endpoint, due to the 'admin' query string.
+ """
+ url = reverse('team_membership_detail', args=[self.solar_team.team_id, self.users['student_enrolled'].username])
+ self.make_call(url, 204, 'delete', user='student_enrolled')
+ self.assert_event_emitted(
+ 'edx.team.learner_removed',
+ team_id=self.solar_team.team_id,
+ course_id=unicode(self.solar_team.course_id),
+ user_id=self.users['student_enrolled'].id,
+ remove_method='self_removal'
+ )
+
def test_bad_team(self):
self.delete_membership('no_such_team', self.users['student_enrolled'].username, 404)
diff --git a/lms/djangoapps/teams/views.py b/lms/djangoapps/teams/views.py
index 9b159a1977..ff9efc23a4 100644
--- a/lms/djangoapps/teams/views.py
+++ b/lms/djangoapps/teams/views.py
@@ -1,6 +1,8 @@
"""HTTP endpoints for the Teams API."""
-from django.shortcuts import render_to_response
+import logging
+
+from django.shortcuts import get_object_or_404, render_to_response
from django.http import Http404
from django.conf import settings
from django.core.paginator import Paginator
@@ -61,6 +63,8 @@ TEAM_MEMBERSHIPS_PER_PAGE = 2
TOPICS_PER_PAGE = 12
MAXIMUM_SEARCH_SIZE = 100000
+log = logging.getLogger(__name__)
+
@receiver(post_save, sender=CourseTeam)
def team_post_save_callback(sender, instance, **kwargs): # pylint: disable=unused-argument
@@ -504,7 +508,7 @@ class TeamsDetailView(ExpandableFieldViewMixin, RetrievePatchAPIView):
"""
**Use Cases**
- Get or update a course team's information. Updates are supported
+ Get, update, or delete a course team's information. Updates are supported
only through merge patch.
**Example Requests**:
@@ -513,6 +517,8 @@ class TeamsDetailView(ExpandableFieldViewMixin, RetrievePatchAPIView):
PATCH /api/team/v0/teams/{team_id} "application/merge-patch+json"
+ DELETE /api/team/v0/teams/{team_id}
+
**Query Parameters for GET**
* expand: Comma separated list of types for which to return
@@ -577,6 +583,20 @@ class TeamsDetailView(ExpandableFieldViewMixin, RetrievePatchAPIView):
If the update could not be completed due to validation errors, this
method returns a 400 error with all error messages in the
"field_errors" field of the returned JSON.
+
+ **Response Values for DELETE**
+
+ Only staff can delete teams. When a team is deleted, all
+ team memberships associated with that team are also
+ deleted. Returns 204 on successful deletion.
+
+ If the user is anonymous or inactive, a 401 is returned.
+
+ If the user is not course or global staff and does not
+ have discussion privileges, a 403 is returned.
+
+ If the user is logged in and the team does not exist, a 404 is returned.
+
"""
authentication_classes = (OAuth2Authentication, SessionAuthentication)
permission_classes = (permissions.IsAuthenticated, IsStaffOrPrivilegedOrReadOnly, IsEnrolledOrIsStaff,)
@@ -588,6 +608,29 @@ class TeamsDetailView(ExpandableFieldViewMixin, RetrievePatchAPIView):
"""Returns the queryset used to access the given team."""
return CourseTeam.objects.all()
+ def delete(self, request, team_id):
+ """DELETE /api/team/v0/teams/{team_id}"""
+ team = get_object_or_404(CourseTeam, team_id=team_id)
+ self.check_object_permissions(request, team)
+ # Note: list() forces the queryset to be evualuated before delete()
+ memberships = list(CourseTeamMembership.get_memberships(team_id=team_id))
+
+ # Note: also deletes all team memberships associated with this team
+ team.delete()
+ log.info('user %d deleted team %s', request.user.id, team_id)
+ tracker.emit('edx.team.deleted', {
+ 'team_id': team_id,
+ 'course_id': unicode(team.course_id),
+ })
+ for member in memberships:
+ tracker.emit('edx.team.learner_removed', {
+ 'team_id': team_id,
+ 'course_id': unicode(team.course_id),
+ 'remove_method': 'team_deleted',
+ 'user_id': member.user_id
+ })
+ return Response(status=status.HTTP_204_NO_CONTENT)
+
class TopicListView(GenericAPIView):
"""
@@ -1155,6 +1198,9 @@ class MembershipDetailView(ExpandableFieldViewMixin, GenericAPIView):
team = self.get_team(team_id)
if has_team_api_access(request.user, team.course_id, access_username=username):
membership = self.get_membership(username, team)
+ removal_method = 'self_removal'
+ if 'admin' in request.QUERY_PARAMS:
+ removal_method = 'removed_by_admin'
membership.delete()
tracker.emit(
'edx.team.learner_removed',
@@ -1162,7 +1208,7 @@ class MembershipDetailView(ExpandableFieldViewMixin, GenericAPIView):
'team_id': team.team_id,
'course_id': unicode(team.course_id),
'user_id': membership.user.id,
- 'remove_method': 'self_removal' if membership.user == request.user else 'removed_by_admin'
+ 'remove_method': removal_method
}
)
return Response(status=status.HTTP_204_NO_CONTENT)
diff --git a/lms/static/js/spec/main.js b/lms/static/js/spec/main.js
index 6d7c06af9d..9fac876357 100644
--- a/lms/static/js/spec/main.js
+++ b/lms/static/js/spec/main.js
@@ -704,6 +704,8 @@
'lms/include/teams/js/spec/collections/topic_collection_spec.js',
'lms/include/teams/js/spec/teams_tab_factory_spec.js',
'lms/include/teams/js/spec/views/edit_team_spec.js',
+ 'lms/include/teams/js/spec/views/edit_team_members_spec.js',
+ 'lms/include/teams/js/spec/views/instructor_tools_spec.js',
'lms/include/teams/js/spec/views/my_teams_spec.js',
'lms/include/teams/js/spec/views/team_card_spec.js',
'lms/include/teams/js/spec/views/team_discussion_spec.js',
diff --git a/lms/static/sass/_developer.scss b/lms/static/sass/_developer.scss
index ded26c5fd7..e31d9747e7 100644
--- a/lms/static/sass/_developer.scss
+++ b/lms/static/sass/_developer.scss
@@ -232,3 +232,85 @@
}
}
}
+
+//efischer; scratch space for teams instructor tools
+.view-teams {
+ .wrapper-msg {
+ @include clearfix();
+ max-width: grid-width(12);
+ margin: 0 auto;
+ border-top: 3px solid $orange;
+
+ .left-floater {
+ @include float(left);
+ text-transform: uppercase;
+ font-weight: $font-semibold;
+ color: $white;
+ line-height: $body-line-height;
+ }
+
+ .right-floater {
+ @include float(right);
+ line-height: $body-line-height;
+
+ button {
+ background: transparent;
+ border: 1px solid transparent;
+ color: $white;
+ box-shadow: 0 0 0 0;
+ font-weight: $font-regular;
+ text-shadow: 0 0;
+
+ &:hover {
+ color: $orange;
+ background: transparent;
+ border: 1px solid $orange;
+ box-shadow: 0 0 0 0;
+ }
+
+ &:focus {
+ box-shadow: 0 0 0 0;
+ }
+ }
+ }
+ }
+
+ .edit-members {
+ @extend %ui-no-list;
+
+ .team-member {
+ line-height: $body-line-height;
+ padding: 10px;
+ }
+
+ .member-info-container {
+ display: inline-block;
+ vertical-align: middle;
+ @include margin-left($baseline/2);
+
+ .primary {
+ font-size: 120%;
+ }
+
+ .secondary {
+ color: $lighter-base-font-color;
+ font-size: 80%;
+ display: block;
+ }
+ }
+
+ .member-profile {
+ img {
+ border: 1px solid $gray;
+ }
+ }
+
+ .action-remove-member {
+ color: $blue;
+ background: transparent;
+ border: 1px solid transparent;
+ font: inherit;
+ }
+ }
+}
+