From dcf5d70bc4b831e48d89d3394d51110d2212d56d Mon Sep 17 00:00:00 2001 From: Kyle McCormick <kmccormick@edx.org> Date: Mon, 2 Dec 2019 11:27:05 -0500 Subject: [PATCH] Create UI for CSV team management (#22310) Adds "Manage" sub-tab to course "Teams" tab with UI for downloading and uploading team membership CSVs. The upload and download function- ality are currently not implemented. The new tab only appears when the user is course staff and the course has at least one instructor-managed team-set, which is not the case for any existing courses, so not current course staff will see this change. This ticket will be followed-up upon in MST-44 and MST-49. MST-41 --- lms/djangoapps/teams/api.py | 30 ++- lms/djangoapps/teams/api_urls.py | 8 + lms/djangoapps/teams/csv.py | 21 ++ .../teams/static/teams/js/models/topic.js | 8 +- .../static/teams/js/spec/views/manage_spec.js | 47 +++++ .../teams/js/spec/views/teams_tab_spec.js | 53 ++++- .../teams/static/teams/js/views/manage.js | 72 +++++++ .../teams/static/teams/js/views/teams_tab.js | 99 ++++++--- .../static/teams/js/views/topic_teams.js | 26 ++- .../teams/static/teams/js/views/topics.js | 6 +- .../static/teams/templates/manage.underscore | 42 ++++ .../teams/templates/teams/teams.html | 1 + lms/djangoapps/teams/tests/test_api.py | 2 +- .../teams/tests/test_serializers.py | 8 +- lms/djangoapps/teams/tests/test_views.py | 49 +++++ lms/djangoapps/teams/views.py | 188 ++++++++++++------ lms/static/lms/js/spec/main.js | 1 + lms/static/sass/views/_teams.scss | 24 ++- 18 files changed, 571 insertions(+), 114 deletions(-) create mode 100644 lms/djangoapps/teams/csv.py create mode 100644 lms/djangoapps/teams/static/teams/js/spec/views/manage_spec.js create mode 100644 lms/djangoapps/teams/static/teams/js/views/manage.js create mode 100644 lms/djangoapps/teams/static/teams/templates/manage.underscore diff --git a/lms/djangoapps/teams/api.py b/lms/djangoapps/teams/api.py index 37ec7a83501..0d31aac9256 100644 --- a/lms/djangoapps/teams/api.py +++ b/lms/djangoapps/teams/api.py @@ -32,6 +32,14 @@ class OrganizationProtectionStatus(Enum): protection_exempt = 'org_protection_exempt' unprotected = 'org_unprotected' + @property + def is_protected(self): + return self == self.protected + + @property + def is_exempt(self): + return self == self.protection_exempt + ORGANIZATION_PROTECTED_MODES = ( CourseMode.MASTERS, @@ -93,13 +101,15 @@ def discussion_visible_by_user(discussion_id, user): return not is_team_discussion_private(team) or user_is_a_team_member(user, team) -def _has_course_staff_privileges(user, course_key): +def has_course_staff_privileges(user, course_key): """ Returns True if the user is an admin for the course, else returns False """ if user.is_staff: return True - if CourseStaffRole(course_key).has_user(user) or CourseInstructorRole(course_key).has_user(user): + if CourseStaffRole(course_key).has_user(user): + return True + if CourseInstructorRole(course_key).has_user(user): return True return False @@ -117,7 +127,7 @@ def has_team_api_access(user, course_key, access_username=None): Returns: bool: True if the user has access, False otherwise. """ - if _has_course_staff_privileges(user, course_key): + if has_course_staff_privileges(user, course_key): return True if has_discussion_privileges(user, course_key): return True @@ -133,7 +143,7 @@ def user_organization_protection_status(user, course_key): If the user is a staff of the course, we return the protection_exempt status else, we return the unprotected status """ - if _has_course_staff_privileges(user, course_key): + if has_course_staff_privileges(user, course_key): return OrganizationProtectionStatus.protection_exempt enrollment = CourseEnrollment.get_enrollment(user, course_key) if enrollment and enrollment.is_active: @@ -200,5 +210,13 @@ def add_team_count(topics, course_id, organization_protection_status): topic['team_count'] = topics_to_team_count.get(topic['id'], 0) -def can_user_modify_team(user, course_key, team): - return not is_instructor_managed_team(team) or _has_course_staff_privileges(user, course_key) +def can_user_modify_team(user, team): + """ + Returns whether a User has permission to modify the membership of a CourseTeam. + + Assumes that user is enrolled in course run. + """ + return ( + (not is_instructor_managed_team(team)) or + has_course_staff_privileges(user, team.course_id) + ) diff --git a/lms/djangoapps/teams/api_urls.py b/lms/djangoapps/teams/api_urls.py index 393bea388d3..025caf84b60 100644 --- a/lms/djangoapps/teams/api_urls.py +++ b/lms/djangoapps/teams/api_urls.py @@ -7,6 +7,7 @@ from django.conf import settings from django.conf.urls import url from .views import ( + MembershipBulkManagementView, MembershipDetailView, MembershipListView, TeamsDetailView, @@ -56,5 +57,12 @@ urlpatterns = [ ), MembershipDetailView.as_view(), name="team_membership_detail" + ), + url( + r'^v0/bulk_team_membership/{course_id_pattern}$'.format( + course_id_pattern=settings.COURSE_ID_PATTERN, + ), + MembershipBulkManagementView.as_view(), + name="team_membership_bulk_management" ) ] diff --git a/lms/djangoapps/teams/csv.py b/lms/djangoapps/teams/csv.py new file mode 100644 index 00000000000..1418a4dee49 --- /dev/null +++ b/lms/djangoapps/teams/csv.py @@ -0,0 +1,21 @@ +""" +CSV processing and generation utilities for Teams LMS app. +""" + + +def load_team_membership_csv(course, response): + """ + Load a CSV detailing course membership. + + Arguments: + course (CourseDescriptor): Course module for which CSV + download has been requested. + response (HttpResponse): Django response object to which + the CSV content will be written. + """ + # This function needs to be implemented (TODO MST-31). + _ = course + not_implemented_message = ( + "Team membership CSV download is not yet implemented." + ) + response.write(not_implemented_message + "\n") diff --git a/lms/djangoapps/teams/static/teams/js/models/topic.js b/lms/djangoapps/teams/static/teams/js/models/topic.js index 86286b728d7..718b21f0f90 100644 --- a/lms/djangoapps/teams/static/teams/js/models/topic.js +++ b/lms/djangoapps/teams/static/teams/js/models/topic.js @@ -9,11 +9,17 @@ name: '', description: '', team_count: 0, - id: '' + id: '', + type: 'open' }, initialize: function(options) { this.url = options.url; + }, + + isInstructorManaged: function() { + var topicType = this.get('type'); + return topicType === 'public_managed' || topicType === 'private_managed'; } }); return Topic; diff --git a/lms/djangoapps/teams/static/teams/js/spec/views/manage_spec.js b/lms/djangoapps/teams/static/teams/js/spec/views/manage_spec.js new file mode 100644 index 00000000000..3cba23dee60 --- /dev/null +++ b/lms/djangoapps/teams/static/teams/js/spec/views/manage_spec.js @@ -0,0 +1,47 @@ +define([ + 'jquery', + 'backbone', + 'underscore', + 'teams/js/views/manage', + 'teams/js/spec_helpers/team_spec_helpers', + 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers' +], function($, Backbone, _, ManageView, TeamSpecHelpers, AjaxHelpers) { + 'use strict'; + + describe('Team Management Dashboard', function() { + var view; + var uploadFile = new File([], 'empty-test-file.csv'); + var mockUploadClickEvent = {target: {files: [uploadFile]}}; + + beforeEach(function() { + setFixtures('<div class="teams-container"></div>'); + view = new ManageView({ + teamEvents: TeamSpecHelpers.teamEvents, + teamMembershipManagementUrl: '/manage-test-url' + }).render(); + spyOn(view, 'handleCsvUploadSuccess'); + spyOn(view, 'handleCsvUploadFailure'); + }); + + it('can render itself', function() { + expect(_.strip(view.$('.download-team-csv').text())).toEqual('Download Memberships'); + expect(_.strip(view.$('.upload-team-csv').text())).toEqual('Upload Memberships'); + }); + + it('can handle a successful file upload', function() { + var requests = AjaxHelpers.requests(this); + view.uploadCsv(mockUploadClickEvent); + AjaxHelpers.expectRequest(requests, 'POST', view.csvUploadUrl); + AjaxHelpers.respondWithJson(requests, {}); + expect(view.handleCsvUploadSuccess).toHaveBeenCalled(); + }); + + it('can handle a failed file upload', function() { + var requests = AjaxHelpers.requests(this); + view.uploadCsv(mockUploadClickEvent); + AjaxHelpers.expectRequest(requests, 'POST', view.csvUploadUrl); + AjaxHelpers.respondWithError(requests); + expect(view.handleCsvUploadFailure).toHaveBeenCalled(); + }); + }); +}); 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 9f4fa795479..0935f149b28 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 @@ -1,14 +1,14 @@ define([ 'jquery', + 'underscore', 'backbone', 'logger', 'edx-ui-toolkit/js/utils/spec-helpers/spec-helpers', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers', 'common/js/spec_helpers/page_helpers', 'teams/js/views/teams_tab', - 'teams/js/spec_helpers/team_spec_helpers', - 'underscore' -], function($, Backbone, Logger, SpecHelpers, AjaxHelpers, PageHelpers, TeamsTabView, TeamSpecHelpers, _) { + 'teams/js/spec_helpers/team_spec_helpers' +], function($, _, Backbone, Logger, SpecHelpers, AjaxHelpers, PageHelpers, TeamsTabView, TeamSpecHelpers) { 'use strict'; describe('TeamsTab', function() { @@ -230,6 +230,53 @@ define([ }); }); + describe('Manage Tab', function() { + var manageTabSelector = '.page-content-nav>.nav-item[data-url=manage]'; + var noManagedData = TeamSpecHelpers.createMockTopicData(1, 2); + var withManagedData = TeamSpecHelpers.createMockTopicData(1, 2); + var topicsNoManaged, topicsWithManaged; + + topicsNoManaged = { + count: 2, + num_pages: 1, + current_page: 1, + start: 0, + results: noManagedData + }; + withManagedData[1].type = 'public_managed'; + topicsWithManaged = { + count: 2, + num_pages: 1, + current_page: 1, + start: 0, + results: withManagedData + }; + + it('is not visible to unprivileged users', function() { + var teamsTabView = createTeamsTabView(this, { + userInfo: TeamSpecHelpers.createMockUserInfo({privileged: false}), + topics: topicsNoManaged + }); + expect(teamsTabView.$(manageTabSelector).length).toBe(0); + }); + + it('is not visible when there are no managed topics', function() { + var teamsTabView = createTeamsTabView(this, { + userInfo: TeamSpecHelpers.createMockUserInfo({privileged: true}), + topics: topicsNoManaged + }); + expect(teamsTabView.$(manageTabSelector).length).toBe(0); + }); + + it('is visible to privileged users when there is a managed topic', function() { + var teamsTabView = createTeamsTabView(this, { + userInfo: TeamSpecHelpers.createMockUserInfo({privileged: true}), + topics: topicsWithManaged + }); + expect(teamsTabView.$(manageTabSelector).length).toBe(1); + }); + }); + describe('Search', function() { var performSearch = function(reqs, teamsTabView) { teamsTabView.$('.search-field').val('foo'); diff --git a/lms/djangoapps/teams/static/teams/js/views/manage.js b/lms/djangoapps/teams/static/teams/js/views/manage.js new file mode 100644 index 00000000000..34f782eed38 --- /dev/null +++ b/lms/djangoapps/teams/static/teams/js/views/manage.js @@ -0,0 +1,72 @@ +(function(define) { + 'use strict'; + define([ + 'backbone', + 'underscore', + 'gettext', + 'edx-ui-toolkit/js/utils/html-utils', + 'common/js/components/utils/view_utils', + 'teams/js/views/team_utils', + 'text!teams/templates/manage.underscore' + ], function(Backbone, _, gettext, HtmlUtils, ViewUtils, TeamUtils, manageTemplate) { + var ManageView = Backbone.View.extend({ + + srInfo: { + id: 'heading-manage', + text: gettext('Manage') + }, + + events: { + 'click #download-team-csv-input': ViewUtils.withDisabledElement('downloadCsv'), + 'change #upload-team-csv-input': ViewUtils.withDisabledElement('uploadCsv') + }, + + initialize: function(options) { + this.teamEvents = options.teamEvents; + this.csvUploadUrl = options.teamMembershipManagementUrl; + this.csvDownloadUrl = options.teamMembershipManagementUrl; + }, + + render: function() { + HtmlUtils.setHtml( + this.$el, + HtmlUtils.template(manageTemplate)({}) + ); + return this; + }, + + downloadCsv: function() { + window.location.href = this.csvDownloadUrl; + }, + + uploadCsv: function(event) { + var file = event.target.files[0]; + var self = this; + var formData = new FormData(); + + formData.append('csv', file); // xss-lint: disable=javascript-jquery-append + return $.ajax({ + type: 'POST', + url: self.csvUploadUrl, + data: formData, + processData: false, // tell jQuery not to process the data + contentType: false // tell jQuery not to set contentType + }).done( + self.handleCsvUploadSuccess + ).fail( + self.handleCsvUploadFailure + ); + }, + + handleCsvUploadSuccess: function() { + // This handler is currently unimplemented (TODO MST-44) + this.teamEvents.trigger('teams:update', {}); + }, + + handleCsvUploadFailure: function() { + // This handler is currently unimplemented (TODO MST-44) + } + }); + return ManageView; + }); +}).call(this, define || RequireJS.define); 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 dea3f100f92..f4e34cf3b16 100644 --- a/lms/djangoapps/teams/static/teams/js/views/teams_tab.js +++ b/lms/djangoapps/teams/static/teams/js/views/teams_tab.js @@ -20,6 +20,7 @@ 'teams/js/views/topics', 'teams/js/views/team_profile', 'teams/js/views/my_teams', + 'teams/js/views/manage', 'teams/js/views/topic_teams', 'teams/js/views/edit_team', 'teams/js/views/edit_team_members', @@ -29,8 +30,9 @@ 'text!teams/templates/teams_tab.underscore'], function(Backbone, $, _, gettext, HtmlUtils, StringUtils, SearchFieldView, HeaderView, HeaderModel, TopicModel, TopicCollection, TeamModel, TeamCollection, MyTeamsCollection, TeamAnalytics, - TeamsTabbedView, TopicsView, TeamProfileView, MyTeamsView, TopicTeamsView, TeamEditView, - TeamMembersEditView, TeamProfileHeaderActionsView, TeamUtils, InstructorToolsView, teamsTemplate) { + TeamsTabbedView, TopicsView, TeamProfileView, MyTeamsView, ManageView, TopicTeamsView, + TeamEditView, TeamMembersEditView, TeamProfileHeaderActionsView, TeamUtils, InstructorToolsView, + teamsTemplate) { var TeamsHeaderModel = HeaderModel.extend({ initialize: function() { _.extend(this.defaults, {nav_aria_label: gettext('Topics')}); @@ -59,7 +61,7 @@ var TeamTabView = Backbone.View.extend({ initialize: function(options) { - var router; + var router, tabsList; this.context = options.context; // This slightly tedious approach is necessary // to use regular expressions within Backbone @@ -78,10 +80,9 @@ ['topics/:topic_id/search(/)', _.bind(this.searchTeams, this)], ['topics/:topic_id/create-team(/)', _.bind(this.newTeam, this)], ['teams/:topic_id/:team_id(/)', _.bind(this.browseTeam, this)], - // eslint-disable-next-line no-useless-escape - [new RegExp('^(browse)\/?$'), _.bind(this.goToTab, this)], - // eslint-disable-next-line no-useless-escape - [new RegExp('^(my-teams)\/?$'), _.bind(this.goToTab, this)] + [new RegExp('^(browse)/?$'), _.bind(this.goToTab, this)], + [new RegExp('^(my-teams)/?$'), _.bind(this.goToTab, this)], + [new RegExp('^(manage)/?$'), _.bind(this.goToTab, this)] ], function(route) { router.route.apply(router, route); }); @@ -133,21 +134,38 @@ collection: this.topicsCollection }); + this.manageView = new ManageView({ + router: this.router, + teamEvents: this.teamEvents, + teamMembershipManagementUrl: this.context.teamMembershipManagementUrl + }); + + tabsList = [{ + title: gettext('My Team'), + url: 'my-teams', + view: this.myTeamsView + }, { + title: gettext('Browse'), + url: 'browse', + view: this.topicsView + }]; + if (this.canViewManageTab()) { + tabsList.push({ + title: gettext('Manage'), + url: 'manage', + view: this.manageView + }); + } + this.mainView = this.tabbedView = this.createViewWithHeader({ title: gettext('Teams'), - description: gettext('See all teams in your course, organized by topic. ' + - 'Join a team to collaborate with other learners who are interested' + - 'in the same topic as you are.'), + description: gettext( + 'See all teams in your course, organized by topic. ' + + 'Join a team to collaborate with other learners who are ' + + 'interested in the same topic as you are.' + ), mainView: new TeamsTabbedView({ - tabs: [{ - title: gettext('My Team'), - url: 'my-teams', - view: this.myTeamsView - }, { - title: gettext('Browse'), - url: 'browse', - view: this.topicsView - }], + tabs: tabsList, router: this.router }) }); @@ -239,8 +257,10 @@ view.mainView = view.createViewWithHeader({ topic: topic, title: gettext('Create a New Team'), - description: gettext("Create a new team if you can't find an existing team to join, " + - 'or if you would like to learn with friends you know.'), + description: gettext( + 'Create a new team if you can\'t find an existing team to join, ' + + 'or if you would like to learn with friends you know.' + ), breadcrumbs: view.createBreadcrumbs(topic), mainView: new TeamEditView({ action: 'create', @@ -274,8 +294,10 @@ }); editViewWithHeader = self.createViewWithHeader({ title: gettext('Edit Team'), - description: gettext('If you make significant changes, ' + - 'make sure you notify members of the team before making these changes.'), + description: gettext( + 'If you make significant changes, ' + + 'make sure you notify members of the team before making these changes.' + ), breadcrumbs: self.createBreadcrumbs(topic, team), mainView: view, topic: topic, @@ -304,8 +326,10 @@ mainView: view, breadcrumbs: self.createBreadcrumbs(topic, team), title: gettext('Membership'), - description: gettext('You can remove members from this team, ' + - "especially if they have not participated in the team's activity."), + description: gettext( + 'You can remove members from this team, ' + + 'especially if they have not participated in the team\'s activity.' + ), topic: topic, team: team } @@ -459,6 +483,31 @@ return this.context.userInfo.privileged || this.context.userInfo.staff; }, + + /** + * Returns whether the "Manage" tab should be shown to the user. + */ + canViewManageTab: function() { + return this.canManageTeams() && this.anyInstructorManagedTopics(); + }, + + /** + * Returns whether a user has permission to manage teams through CSV + * upload & download in the "Manage" tab. + */ + canManageTeams: function() { + return this.canEditTeam(); + }, + + /** + * Returns whether _any_ of the topics are instructor-managed. + */ + anyInstructorManagedTopics: function() { + return this.topicsCollection.some( + function(topic) { return topic.isInstructorManaged(); } + ); + }, + createBreadcrumbs: function(topic, team) { var breadcrumbs = [{ title: gettext('All Topics'), diff --git a/lms/djangoapps/teams/static/teams/js/views/topic_teams.js b/lms/djangoapps/teams/static/teams/js/views/topic_teams.js index a389dfb01e5..38a3e9e5a9a 100644 --- a/lms/djangoapps/teams/static/teams/js/views/topic_teams.js +++ b/lms/djangoapps/teams/static/teams/js/views/topic_teams.js @@ -1,13 +1,14 @@ (function(define) { 'use strict'; define([ + 'underscore', 'backbone', 'gettext', + 'edx-ui-toolkit/js/utils/html-utils', 'teams/js/views/teams', 'common/js/components/views/paging_header', - 'text!teams/templates/team-actions.underscore', - 'underscore' - ], function(Backbone, gettext, TeamsView, PagingHeader, teamActionsTemplate, _) { + 'text!teams/templates/team-actions.underscore' + ], function(_, Backbone, gettext, HtmlUtils, TeamsView, PagingHeader, teamActionsTemplate) { var TopicTeamsView = TeamsView.extend({ events: { 'click a.browse-teams': 'browseTeams', @@ -34,9 +35,10 @@ render: function() { var self = this; this.collection.refresh().done(function() { + var message; TeamsView.prototype.render.call(self); if (self.canUserCreateTeam()) { - var message = interpolate_text( // eslint-disable-line vars-on-top, no-undef + message = interpolate_text( // eslint-disable-line no-undef // Translators: this string is shown at the bottom of the teams page // to find a team to join or else to create a new one. There are three // links that need to be included in the message: @@ -45,10 +47,12 @@ // 3. create a new team // Be careful to start each link with the appropriate start indicator // (e.g. {browse_span_start} for #1) and finish it with {span_end}. - _.escape(gettext('{browse_span_start}Browse teams in other topics{span_end} or ' + - '{search_span_start}search teams{span_end} in this topic. ' + - "If you still can't find a team to join, " + - '{create_span_start}create a new team in this topic{span_end}.')), + _.escape(gettext( + '{browse_span_start}Browse teams in other ' + + 'topics{span_end} or {search_span_start}search teams{span_end} ' + + 'in this topic. If you still can\'t find a team to join, ' + + '{create_span_start}create a new team in this topic{span_end}.' + )), { browse_span_start: '<a class="browse-teams" href="">', search_span_start: '<a class="search-teams" href="">', @@ -56,8 +60,10 @@ span_end: '</a>' } ); - // eslint-disable-next-line max-len - self.$el.append(_.template(teamActionsTemplate)({message: message})); // xss-lint: disable=javascript-jquery-append + HtmlUtils.append( + self.$el, + HtmlUtils.template(teamActionsTemplate)({message: message}) + ); } }); return this; diff --git a/lms/djangoapps/teams/static/teams/js/views/topics.js b/lms/djangoapps/teams/static/teams/js/views/topics.js index 85fb750f459..5413edbf6d8 100644 --- a/lms/djangoapps/teams/static/teams/js/views/topics.js +++ b/lms/djangoapps/teams/static/teams/js/views/topics.js @@ -1,13 +1,13 @@ (function(define) { 'use strict'; define([ + 'underscore', 'gettext', 'teams/js/views/topic_card', 'teams/js/views/team_utils', 'common/js/components/views/paging_header', - 'common/js/components/views/paginated_view', - 'underscore' - ], function(gettext, TopicCardView, TeamUtils, PagingHeader, PaginatedView, _) { + 'common/js/components/views/paginated_view' + ], function(_, gettext, TopicCardView, TeamUtils, PagingHeader, PaginatedView) { var TopicsView = PaginatedView.extend({ type: 'topics', diff --git a/lms/djangoapps/teams/static/teams/templates/manage.underscore b/lms/djangoapps/teams/static/teams/templates/manage.underscore new file mode 100644 index 00000000000..6f3974f69f4 --- /dev/null +++ b/lms/djangoapps/teams/static/teams/templates/manage.underscore @@ -0,0 +1,42 @@ +<div class="team-management"> + <div class="page-content-main"> + <div class="team-management-section"> + <h3 class="team-detail-header"> + <%- gettext("View Current Team Memberships") %> + </h3> + <p> + <%- gettext( + "Click to download a comma-separated values (CSV) file that, " + + "for each user, lists the team they are on for each topic." + ) %> + </p> + <button + id="download-team-csv-input" + class="download-team-csv action action-primary" + > + <%- gettext("Download Memberships") %> + </button> + </div> + <div class="team-management-section"> + <h3 class="team-detail-header"> + <%- gettext("Assign Team Memberships") %> + </h3> + <p> + <%- gettext( + "Click to upload a comma-separated values (CSV) file to assign " + + "users to teams." + ) %> + </p> + <div class="upload-team-csv btn action action-primary"> + <input + id="upload-team-csv-input" + type="file" + accept="text/csv" + class="input-overlay-hack" + /> + <%- gettext("Upload Memberships") %> + </div> + <!-- We need to describe the format of the CSV here (TODO MST-49) --> + </div> + </div> +</div> diff --git a/lms/djangoapps/teams/templates/teams/teams.html b/lms/djangoapps/teams/templates/teams/teams.html index 14278bc042e..d24b9d7ffbf 100644 --- a/lms/djangoapps/teams/templates/teams/teams.html +++ b/lms/djangoapps/teams/templates/teams/teams.html @@ -51,6 +51,7 @@ from openedx.core.djangolib.js_utils import ( teamMembershipsUrl: '${team_memberships_url | n, js_escaped_string}', teamMembershipDetailUrl: '${team_membership_detail_url | n, js_escaped_string}', myTeamsUrl: '${my_teams_url | n, js_escaped_string}', + teamMembershipManagementUrl: '${team_membership_management_url | n, js_escaped_string}', maxTeamSize: ${course.teams_configuration.default_max_team_size | n, dump_js_escaped_json}, languages: ${languages | n, dump_js_escaped_json}, countries: ${countries | n, dump_js_escaped_json}, diff --git a/lms/djangoapps/teams/tests/test_api.py b/lms/djangoapps/teams/tests/test_api.py index c1386148daa..58efde25573 100644 --- a/lms/djangoapps/teams/tests/test_api.py +++ b/lms/djangoapps/teams/tests/test_api.py @@ -13,9 +13,9 @@ from opaque_keys.edx.keys import CourseKey from course_modes.models import CourseMode from lms.djangoapps.teams import api as teams_api from lms.djangoapps.teams.tests.factories import CourseTeamFactory -from student.tests.factories import CourseEnrollmentFactory, UserFactory from student.models import CourseEnrollment from student.roles import CourseStaffRole +from student.tests.factories import CourseEnrollmentFactory, UserFactory from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase COURSE_KEY1 = CourseKey.from_string('edx/history/1') diff --git a/lms/djangoapps/teams/tests/test_serializers.py b/lms/djangoapps/teams/tests/test_serializers.py index 8bce6a8ec56..fde8125581f 100644 --- a/lms/djangoapps/teams/tests/test_serializers.py +++ b/lms/djangoapps/teams/tests/test_serializers.py @@ -154,10 +154,10 @@ class BaseTopicSerializerTestCase(SerializerTestCase): """ topics = [ { - u'name': u'Tøpic {}'.format(i), - u'description': u'The bést topic! {}'.format(i), - u'id': six.text_type(i), - u'type': u'open' + 'name': 'Tøpic {}'.format(i), + 'description': 'The bést topic! {}'.format(i), + 'id': six.text_type(i), + 'type': 'open', } for i in six.moves.range(num_topics) ] diff --git a/lms/djangoapps/teams/tests/test_views.py b/lms/djangoapps/teams/tests/test_views.py index 2e98115c608..c49a9cba012 100644 --- a/lms/djangoapps/teams/tests/test_views.py +++ b/lms/djangoapps/teams/tests/test_views.py @@ -1642,3 +1642,52 @@ class TestElasticSearchErrors(TeamAPITestCase): data={'description': 'new description'}, user='staff' ) + + +@ddt.ddt +class TestBulkMembershipManagement(TeamAPITestCase): + """ + Test that CSVs can be uploaded and downloaded to manage course membership. + + This test case will be expanded when the view is fully + implemented (TODO MST-31). + """ + good_course_id = 'TestX/TS101/Test_Course' + fake_course_id = 'TestX/TS101/Non_Existent_Course' + + allow_username = 'course_staff' + deny_username = 'student_enrolled' + + @ddt.data( + ('GET', good_course_id, deny_username, 403), + ('GET', fake_course_id, allow_username, 404), + ('GET', fake_course_id, deny_username, 404), + ('POST', good_course_id, allow_username, 501), # TODO MST-31 + ('POST', good_course_id, deny_username, 403), + ('POST', fake_course_id, allow_username, 404), + ('POST', fake_course_id, deny_username, 404), + ) + @ddt.unpack + def test_error_statuses(self, method, course_id, username, expected_status): + url = self.get_url(course_id) + self.login(username) + response = self.client.generic(method, url) + assert response.status_code == expected_status + + def test_download_csv(self): + url = self.get_url(self.good_course_id) + self.login(self.allow_username) + response = self.client.get(url) + assert response.status_code == 200 + assert response['Content-Type'] == 'text/csv' + assert response['Content-Disposition'] == ( + 'attachment; filename="team-membership_TestX_TS101_Test_Course.csv"' + ) + # For now, just assert that the file is non-empty. + # Eventually, we will test contents (TODO MST-31). + assert response.content + + @staticmethod + def get_url(course_id): + # This strategy allows us to test with invalid course IDs + return reverse('team_membership_bulk_management', args=[course_id]) diff --git a/lms/djangoapps/teams/views.py b/lms/djangoapps/teams/views.py index 529caebf816..3dd2b2e0865 100644 --- a/lms/djangoapps/teams/views.py +++ b/lms/djangoapps/teams/views.py @@ -8,12 +8,15 @@ import logging import six from django.conf import settings from django.contrib.auth.models import User +from django.core.exceptions import PermissionDenied from django.db.models.signals import post_save from django.dispatch import receiver -from django.http import Http404 +from django.http import Http404, HttpResponse from django.shortcuts import get_object_or_404, render_to_response +from django.utils.functional import cached_property from django.utils.translation import ugettext as _ from django.utils.translation import ugettext_noop +from django.views import View from django_countries import countries from edx_rest_framework_extensions.paginators import DefaultPagination, paginate_search_results from opaque_keys import InvalidKeyError @@ -28,14 +31,6 @@ from rest_framework_oauth.authentication import OAuth2Authentication from lms.djangoapps.courseware.courses import get_course_with_access, has_access from lms.djangoapps.discussion.django_comment_client.utils import has_discussion_privileges -from lms.djangoapps.teams.api import ( - OrganizationProtectionStatus, - has_team_api_access, - user_organization_protection_status, - has_specific_team_access, - add_team_count, - can_user_modify_team -) from lms.djangoapps.teams.models import CourseTeam, CourseTeamMembership from openedx.core.lib.api.parsers import MergePatchParser from openedx.core.lib.api.permissions import IsStaffOrReadOnly @@ -50,6 +45,16 @@ from util.model_utils import truncate_fields from xmodule.modulestore.django import modulestore from . import is_feature_enabled +from .api import ( + OrganizationProtectionStatus, + add_team_count, + can_user_modify_team, + has_course_staff_privileges, + has_specific_team_access, + has_team_api_access, + user_organization_protection_status +) +from .csv import load_team_membership_csv from .errors import AlreadyOnTeamInCourse, ElasticSearchConnectionError, NotEnrolledInCourseForTeam from .search_indexes import CourseTeamIndexer from .serializers import ( @@ -183,6 +188,9 @@ class TeamsDashboardView(GenericAPIView): "team_memberships_url": reverse('team_membership_list', request=request), "my_teams_url": reverse('teams_list', request=request), "team_membership_detail_url": reverse('team_membership_detail', args=['team_id', user.username]), + "team_membership_management_url": reverse( + 'team_membership_bulk_management', request=request, kwargs={'course_id': course_id} + ), "languages": [[lang[0], _(lang[1])] for lang in settings.ALL_LANGUAGES], # pylint: disable=translation-of-non-string "countries": list(countries), "disable_courseware_js": True, @@ -363,34 +371,35 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView): permission_classes = (permissions.IsAuthenticated,) serializer_class = CourseTeamSerializer - def get(self, request): # pylint: disable=too-many-statements + def get(self, request): """GET /api/team/v0/teams/""" result_filter = {} - if 'course_id' in request.query_params: - course_id_string = request.query_params['course_id'] - try: - course_key = CourseKey.from_string(course_id_string) - # Ensure the course exists - course_module = modulestore().get_course(course_key) - if course_module is None: - return Response(status=status.HTTP_404_NOT_FOUND) - result_filter.update({'course_id': course_key}) - except InvalidKeyError: - error = build_api_error( - ugettext_noop(u"The supplied course id {course_id} is not valid."), - course_id=course_id_string, - ) - return Response(error, status=status.HTTP_400_BAD_REQUEST) - - if not has_team_api_access(request.user, course_key): - return Response(status=status.HTTP_403_FORBIDDEN) - else: + if 'course_id' not in request.query_params: return Response( build_api_error(ugettext_noop("course_id must be provided")), status=status.HTTP_400_BAD_REQUEST ) + course_id_string = request.query_params['course_id'] + try: + course_key = CourseKey.from_string(course_id_string) + course_module = modulestore().get_course(course_key) + except InvalidKeyError: + error = build_api_error( + ugettext_noop(u"The supplied course id {course_id} is not valid."), + course_id=course_id_string, + ) + return Response(error, status=status.HTTP_400_BAD_REQUEST) + + # Ensure the course exists + if course_module is None: + return Response(status=status.HTTP_404_NOT_FOUND) + result_filter.update({'course_id': course_key}) + + if not has_team_api_access(request.user, course_key): + return Response(status=status.HTTP_403_FORBIDDEN) + text_search = request.query_params.get('text_search', None) if text_search and request.query_params.get('order_by', None): return Response( @@ -411,13 +420,13 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView): return Response(error, status=status.HTTP_400_BAD_REQUEST) result_filter.update({'topic_id': topic_id}) - organization_protection_status = user_organization_protection_status(request.user, course_key) - if organization_protection_status != OrganizationProtectionStatus.protection_exempt: - result_filter.update( - { - 'organization_protected': organization_protection_status == OrganizationProtectionStatus.protected - } - ) + organization_protection_status = user_organization_protection_status( + request.user, course_key + ) + if not organization_protection_status.is_exempt: + result_filter.update({ + 'organization_protected': organization_protection_status.is_protected + }) if text_search and CourseTeamIndexer.search_is_enabled(): try: @@ -450,33 +459,36 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView): page = self.paginate_queryset(paginated_results) serializer = self.get_serializer(page, many=True) - order_by_input = None - else: - queryset = CourseTeam.objects.filter(**result_filter) - order_by_input = request.query_params.get('order_by', 'name') - if order_by_input == 'name': - # MySQL does case-insensitive order_by. - queryset = queryset.order_by('name') - elif order_by_input == 'open_slots': - queryset = queryset.order_by('team_size', '-last_activity_at') - elif order_by_input == 'last_activity_at': - queryset = queryset.order_by('-last_activity_at', 'team_size') - else: - return Response({ - 'developer_message': u"unsupported order_by value {ordering}".format(ordering=order_by_input), + return self.get_paginated_response(serializer.data) + + ordering_schemes = { + 'name': ('name',), # MySQL does case-insensitive order_by + 'open_slots': ('team_size', '-last_activity_at'), + 'last_activity_at': ('-last_activity_at', 'team_size'), + } + queryset = CourseTeam.objects.filter(**result_filter) + order_by_input = request.query_params.get('order_by', 'name') + if order_by_input not in ordering_schemes: + return Response( + { + 'developer_message': u"unsupported order_by value {ordering}".format( + ordering=order_by_input, + ), # Translators: 'ordering' is a string describing a way # of ordering a list. For example, {ordering} may be # 'name', indicating that the user wants to sort the # list by lower case name. - 'user_message': _(u"The ordering {ordering} is not supported").format(ordering=order_by_input), - }, status=status.HTTP_400_BAD_REQUEST) - - page = self.paginate_queryset(queryset) - serializer = self.get_serializer(page, many=True) - + 'user_message': _(u"The ordering {ordering} is not supported").format( + ordering=order_by_input, + ), + }, + status=status.HTTP_400_BAD_REQUEST, + ) + queryset = queryset.order_by(*ordering_schemes[order_by_input]) + page = self.paginate_queryset(queryset) + serializer = self.get_serializer(page, many=True) response = self.get_paginated_response(serializer.data) - if order_by_input is not None: - response.data['sort_order'] = order_by_input + response.data['sort_order'] = order_by_input return response def post(self, request): @@ -1168,7 +1180,7 @@ class MembershipListView(ExpandableFieldViewMixin, GenericAPIView): status=status.HTTP_400_BAD_REQUEST ) - if not can_user_modify_team(request.user, team.course_id, team): + if not can_user_modify_team(request.user, team): return Response( build_api_error(ugettext_noop("You can't join an instructor managed team.")), status=status.HTTP_403_FORBIDDEN @@ -1317,7 +1329,7 @@ class MembershipDetailView(ExpandableFieldViewMixin, GenericAPIView): if not has_specific_team_access(request.user, team): return Response(status=status.HTTP_403_FORBIDDEN) - if not can_user_modify_team(request.user, team.course_id, team): + if not can_user_modify_team(request.user, team): return Response( build_api_error(ugettext_noop("You can't leave an instructor managed team.")), status=status.HTTP_403_FORBIDDEN @@ -1338,3 +1350,59 @@ class MembershipDetailView(ExpandableFieldViewMixin, GenericAPIView): } ) return Response(status=status.HTTP_204_NO_CONTENT) + + +class MembershipBulkManagementView(View): + """ + Partially-implemented view for uploading and downloading team membership CSVs. + + TODO MST-31 + """ + def get(self, request, **_kwargs): + """ + Download CSV with team membership data for given course run. + """ + self.check_access() + response = HttpResponse(content_type='text/csv') + filename = "team-membership_{}_{}_{}.csv".format( + self.course.id.org, self.course.id.course, self.course.id.run + ) + response['Content-Disposition'] = 'attachment; filename="{}"'.format(filename) + load_team_membership_csv(self.course, response) + return response + + def post(self, request, **_kwargs): + """ + Process uploaded CSV to modify team memberships for given course run. + """ + self.check_access() + return HttpResponse(status=status.HTTP_501_NOT_IMPLEMENTED) + + def check_access(self): + """ + Raises 403 if user does not have access to this endpoint. + """ + if not has_course_staff_privileges(self.request.user, self.course.id): + raise PermissionDenied( + "To manage team membership of {}, you must be course staff.".format( + self.course.id + ) + ) + + @cached_property + def course(self): + """ + Return a CourseDescriptor based on the `course_id` kwarg. + If invalid or not found, raise 404. + """ + course_id_string = self.kwargs.get('course_id') + if not course_id_string: + raise Http404('No course key provided.') + try: + course_id = CourseKey.from_string(course_id_string) + except InvalidKeyError: + raise Http404('Invalid course key: {}'.format(course_id_string)) + course_module = modulestore().get_course(course_id) + if not course_module: + raise Http404('Course not found: {}'.format(course_id)) + return course_module diff --git a/lms/static/lms/js/spec/main.js b/lms/static/lms/js/spec/main.js index c310def38da..8de036f33a2 100644 --- a/lms/static/lms/js/spec/main.js +++ b/lms/static/lms/js/spec/main.js @@ -819,6 +819,7 @@ 'teams/js/spec/views/edit_team_members_spec.js', 'teams/js/spec/views/edit_team_spec.js', 'teams/js/spec/views/instructor_tools_spec.js', + 'teams/js/spec/views/manage_spec.js', 'teams/js/spec/views/my_teams_spec.js', 'teams/js/spec/views/team_card_spec.js', 'teams/js/spec/views/team_discussion_spec.js', diff --git a/lms/static/sass/views/_teams.scss b/lms/static/sass/views/_teams.scss index 5ede35512a0..3d13841df84 100644 --- a/lms/static/sass/views/_teams.scss +++ b/lms/static/sass/views/_teams.scss @@ -353,7 +353,29 @@ } } - .team-profile { + .team-management { + padding: 1%; + + h3, p, .action, .team-management-section { + margin-bottom: 2%; + } + + .input-overlay-hack { + position: absolute; + height: 100%; + width: 100%; + opacity: 0; + top: 0; + left: 0; + cursor: pointer; + } + + .upload-team-csv { + position: relative; + } + } + + .team-profile, .team-management { .page-content-main { display: inline-block; width: flex-grid(8, 12); -- GitLab