From d044ed28b5edd2d67255439c22761a8dce9e978c Mon Sep 17 00:00:00 2001 From: Alex Wang Date: Mon, 25 Nov 2019 19:59:51 -0500 Subject: [PATCH] Student can NOT modify membership status with team in instructor managed topic (#22286) * no student change to instructor managed team * Address comments & fix unit tests * fix jenkins test failure * Fix eslint errors * Fix xss warnings * Fix bokchoy test --- lms/djangoapps/teams/api.py | 13 +++ lms/djangoapps/teams/serializers.py | 1 + .../teams/static/teams/js/collections/base.js | 11 +-- .../teams/static/teams/js/collections/team.js | 4 +- .../static/teams/js/collections/topic.js | 2 +- .../static/teams/js/models/team_membership.js | 2 +- .../spec/collections/topic_collection_spec.js | 4 +- .../teams/js/spec/views/edit_team_spec.js | 38 ++++---- .../js/spec/views/instructor_tools_spec.js | 2 +- .../teams/js/spec/views/my_teams_spec.js | 3 +- .../teams/js/spec/views/team_card_spec.js | 2 +- .../views/team_profile_header_actions_spec.js | 87 ++++++++++++++----- .../teams/js/spec/views/team_profile_spec.js | 17 +++- .../static/teams/js/spec/views/teams_spec.js | 8 +- .../teams/js/spec/views/teams_tab_spec.js | 13 +-- .../teams/js/spec/views/topic_card_spec.js | 2 + .../teams/js/spec/views/topic_teams_spec.js | 18 ++-- .../static/teams/js/spec/views/topics_spec.js | 2 +- .../js/spec_helpers/team_spec_helpers.js | 28 ++++-- .../static/teams/js/utils/team_analytics.js | 8 +- .../teams/static/teams/js/views/edit_team.js | 17 ++-- .../teams/js/views/edit_team_members.js | 29 +++++-- .../static/teams/js/views/instructor_tools.js | 5 +- .../teams/static/teams/js/views/my_teams.js | 4 +- .../teams/static/teams/js/views/team_card.js | 19 ++-- .../static/teams/js/views/team_profile.js | 15 +++- .../js/views/team_profile_header_actions.js | 24 +++-- .../teams/static/teams/js/views/team_utils.js | 18 ++-- .../teams/static/teams/js/views/teams_tab.js | 16 +++- .../teams/static/teams/js/views/topic_card.js | 11 ++- .../static/teams/js/views/topic_teams.js | 15 ++-- .../teams/static/teams/js/views/topics.js | 5 +- .../teams/templates/team-profile.underscore | 2 +- .../teams/tests/test_serializers.py | 21 +++-- lms/djangoapps/teams/tests/test_views.py | 20 +++++ lms/djangoapps/teams/views.py | 19 +++- 36 files changed, 353 insertions(+), 152 deletions(-) diff --git a/lms/djangoapps/teams/api.py b/lms/djangoapps/teams/api.py index 84edc3368f..37ec7a8350 100644 --- a/lms/djangoapps/teams/api.py +++ b/lms/djangoapps/teams/api.py @@ -62,6 +62,15 @@ def is_team_discussion_private(team): return getattr(team, 'is_discussion_private', False) +def is_instructor_managed_team(team): # pylint: disable=unused-argument + """ + Return true if the team is managed by instructors. + For now always return false, will complete the logic later. + TODO MST-25 + """ + return False + + def user_is_a_team_member(user, team): """ Return if the user is a member of the team @@ -189,3 +198,7 @@ def add_team_count(topics, course_id, organization_protection_status): topics_to_team_count = {d['topic_id']: d['team_count'] for d in teams_per_topic} for topic in topics: 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) diff --git a/lms/djangoapps/teams/serializers.py b/lms/djangoapps/teams/serializers.py index 78efa17c29..67a760a119 100644 --- a/lms/djangoapps/teams/serializers.py +++ b/lms/djangoapps/teams/serializers.py @@ -175,6 +175,7 @@ class BaseTopicSerializer(serializers.Serializer): # pylint: disable=abstract-m description = serializers.CharField() name = serializers.CharField() id = serializers.CharField() # pylint: disable=invalid-name + type = serializers.CharField() class TopicSerializer(BaseTopicSerializer): # pylint: disable=abstract-method diff --git a/lms/djangoapps/teams/static/teams/js/collections/base.js b/lms/djangoapps/teams/static/teams/js/collections/base.js index 4533296690..37c5d4d0a4 100644 --- a/lms/djangoapps/teams/static/teams/js/collections/base.js +++ b/lms/djangoapps/teams/static/teams/js/collections/base.js @@ -21,17 +21,17 @@ parse: function(response, options) { if (!response) { - response = {}; + response = {}; // eslint-disable-line no-param-reassign } if (!response.results) { - response.results = []; + response.results = []; // eslint-disable-line no-param-reassign } return PagingCollection.prototype.parse.call(this, response, options); }, - onUpdate: function(event) { + onUpdate: function(event) { // eslint-disable-line no-unused-vars // Mark the collection as stale so that it knows to refresh when needed. this.isStale = true; }, @@ -40,10 +40,11 @@ // remove when backbone.paginator gets a new release sync: function(method, model, options) { // do not send total pages and total records in request + var params; if (method === 'read') { - var params = _.values(_.pick(this.queryParams, ['totalPages', 'totalRecords'])); + params = _.values(_.pick(this.queryParams, ['totalPages', 'totalRecords'])); _.each(params, function(param) { - delete options.data[param]; + delete options.data[param]; // eslint-disable-line no-param-reassign }); } diff --git a/lms/djangoapps/teams/static/teams/js/collections/team.js b/lms/djangoapps/teams/static/teams/js/collections/team.js index 0e675ac24a..0f8eb4c6e1 100644 --- a/lms/djangoapps/teams/static/teams/js/collections/team.js +++ b/lms/djangoapps/teams/static/teams/js/collections/team.js @@ -1,7 +1,7 @@ (function(define) { 'use strict'; - define(['teams/js/collections/base', 'teams/js/models/team', 'gettext'], - function(BaseCollection, TeamModel, gettext) { + define(['teams/js/collections/base', 'teams/js/models/team', 'gettext', 'underscore'], + function(BaseCollection, TeamModel, gettext, _) { var TeamCollection = BaseCollection.extend({ model: TeamModel, diff --git a/lms/djangoapps/teams/static/teams/js/collections/topic.js b/lms/djangoapps/teams/static/teams/js/collections/topic.js index 40d21f22fb..45d141930b 100644 --- a/lms/djangoapps/teams/static/teams/js/collections/topic.js +++ b/lms/djangoapps/teams/static/teams/js/collections/topic.js @@ -19,7 +19,7 @@ this.state.sortKey = topics.sort_order; } - options.perPage = topics.results.length; + options.perPage = topics.results.length; // eslint-disable-line no-param-reassign BaseCollection.prototype.constructor.call(this, topics, options); this.registerSortableField('name', gettext('name')); diff --git a/lms/djangoapps/teams/static/teams/js/models/team_membership.js b/lms/djangoapps/teams/static/teams/js/models/team_membership.js index 24f8462f96..dbaf78a879 100644 --- a/lms/djangoapps/teams/static/teams/js/models/team_membership.js +++ b/lms/djangoapps/teams/static/teams/js/models/team_membership.js @@ -13,7 +13,7 @@ }, parse: function(response) { - response.team = new TeamModel(response.team); + response.team = new TeamModel(response.team); // eslint-disable-line no-param-reassign return response; } }); diff --git a/lms/djangoapps/teams/static/teams/js/spec/collections/topic_collection_spec.js b/lms/djangoapps/teams/static/teams/js/spec/collections/topic_collection_spec.js index d396de5bac..06f56a548e 100644 --- a/lms/djangoapps/teams/static/teams/js/spec/collections/topic_collection_spec.js +++ b/lms/djangoapps/teams/static/teams/js/spec/collections/topic_collection_spec.js @@ -3,12 +3,12 @@ define(['backbone', 'URI', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/a function(Backbone, URI, _, AjaxHelpers, TeamSpecHelpers) { 'use strict'; describe('TopicCollection', function() { - var topicCollection; + var topicCollection, testRequestParam; beforeEach(function() { topicCollection = TeamSpecHelpers.createMockTopicCollection(); }); - var testRequestParam = function(self, param, value) { + testRequestParam = function(self, param, value) { var requests = AjaxHelpers.requests(self), request, url, 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 20930bae84..8b7e4adce1 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 @@ -9,6 +9,13 @@ define([ 'teams/js/spec_helpers/team_spec_helpers' ], function($, _, Backbone, AjaxHelpers, PageHelpers, TeamEditView, TeamModel, TeamSpecHelpers) { 'use strict'; + var assertFormRendersCorrectly, + assertTeamCreateUpdateInfo, + assertValidationMessagesWhenFieldsEmpty, + assertValidationMessagesWhenInvalidData, + assertShowMessageOnError, + assertRedirectsToCorrectUrlOnCancel, + requestMethod; describe('CreateEditTeam', function() { var teamsUrl = '/api/team/v0/teams/', @@ -31,17 +38,18 @@ define([ language: 'en' }, verifyValidation = function(requests, teamEditView, fieldsData) { + var message = teamEditView.$('.wrapper-msg'); + var actionMessage = ( + // eslint-disable-next-line no-use-before-define + teamAction === 'create' ? 'Your team could not be created.' : 'Your team could not be updated.' + ); _.each(fieldsData, function(fieldData) { teamEditView.$(fieldData[0]).val(fieldData[1]); }); teamEditView.$('.create-team.form-actions .action-primary').click(); - var message = teamEditView.$('.wrapper-msg'); expect(message.hasClass('is-hidden')).toBeFalsy(); - var actionMessage = ( - teamAction === 'create' ? 'Your team could not be created.' : 'Your team could not be updated.' - ); expect(message.find('.title').text().trim()).toBe(actionMessage); expect(message.find('.copy').text().trim()).toBe( 'Check the highlighted fields below and try again.' @@ -95,7 +103,7 @@ define([ spyOn(Backbone.history, 'navigate'); }); - var assertFormRendersCorrectly = function() { + assertFormRendersCorrectly = function() { var fieldClasses = [ '.u-field-name', '.u-field-description', @@ -120,11 +128,11 @@ define([ } }; - var requestMethod = function() { + requestMethod = function() { return teamAction === 'create' ? 'POST' : 'PATCH'; }; - var assertTeamCreateUpdateInfo = function(that, teamsData, teamsUrl, expectedUrl) { + assertTeamCreateUpdateInfo = function(that, teamsData, teamsUrlLink, expectedUrl) { var requests = AjaxHelpers.requests(that), teamEditView = createEditTeamView(); @@ -135,14 +143,14 @@ define([ teamEditView.$('.create-team.form-actions .action-primary').click(); - AjaxHelpers.expectJsonRequest(requests, requestMethod(), teamsUrl, teamsData); + AjaxHelpers.expectJsonRequest(requests, requestMethod(), teamsUrlLink, teamsData); AjaxHelpers.respondWithJson(requests, _.extend({}, teamsData, teamAction === 'create' ? {id: '123'} : {})); expect(teamEditView.$('.create-team.wrapper-msg .copy').text().trim().length).toBe(0); expect(Backbone.history.navigate.calls.mostRecent().args[0]).toBe(expectedUrl); }; - var assertValidationMessagesWhenFieldsEmpty = function(that) { + assertValidationMessagesWhenFieldsEmpty = function(that) { var requests = AjaxHelpers.requests(that), teamEditView = createEditTeamView(); @@ -163,7 +171,7 @@ define([ ]); }; - var assertValidationMessagesWhenInvalidData = function(that) { + assertValidationMessagesWhenInvalidData = function(that) { var requests = AjaxHelpers.requests(that), teamEditView = createEditTeamView(), teamName = new Array(500 + 1).join('$'), @@ -185,7 +193,7 @@ define([ ]); }; - var assertShowMessageOnError = function(that, teamsData, teamsUrl, errorCode) { + assertShowMessageOnError = function(that, teamsData, teamsUrlLink, errorCode) { var teamEditView = createEditTeamView(), requests = AjaxHelpers.requests(that); @@ -195,10 +203,10 @@ define([ teamEditView.$('.create-team.form-actions .action-primary').click(); if (teamAction === 'create') { - teamsData.country = ''; - teamsData.language = ''; + teamsData.country = ''; // eslint-disable-line no-param-reassign + teamsData.language = ''; // eslint-disable-line no-param-reassign } - AjaxHelpers.expectJsonRequest(requests, requestMethod(), teamsUrl, teamsData); + AjaxHelpers.expectJsonRequest(requests, requestMethod(), teamsUrlLink, teamsData); if (errorCode < 500) { AjaxHelpers.respondWithError( @@ -213,7 +221,7 @@ define([ } }; - var assertRedirectsToCorrectUrlOnCancel = function(expectedUrl) { + assertRedirectsToCorrectUrlOnCancel = function(expectedUrl) { var teamEditView = createEditTeamView(); teamEditView.$('.create-team.form-actions .action-cancel').click(); expect(Backbone.history.navigate.calls.mostRecent().args[0]).toBe(expectedUrl); diff --git a/lms/djangoapps/teams/static/teams/js/spec/views/instructor_tools_spec.js b/lms/djangoapps/teams/static/teams/js/spec/views/instructor_tools_spec.js index b866826769..65b72230ef 100644 --- a/lms/djangoapps/teams/static/teams/js/spec/views/instructor_tools_spec.js +++ b/lms/djangoapps/teams/static/teams/js/spec/views/instructor_tools_spec.js @@ -19,7 +19,7 @@ define([ teamEvents: TeamSpecHelpers.teamEvents }); }, - deleteTeam = function(view, confirm) { + deleteTeam = function(view, confirm) { // eslint-disable-line no-shadow view.$('.action-delete').click(); // Confirm delete dialog if (confirm) { diff --git a/lms/djangoapps/teams/static/teams/js/spec/views/my_teams_spec.js b/lms/djangoapps/teams/static/teams/js/spec/views/my_teams_spec.js index bf9059f6ac..95202c95dc 100644 --- a/lms/djangoapps/teams/static/teams/js/spec/views/my_teams_spec.js +++ b/lms/djangoapps/teams/static/teams/js/spec/views/my_teams_spec.js @@ -6,12 +6,13 @@ define([ 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers' ], function(Backbone, MyTeamsCollection, MyTeamsView, TeamSpecHelpers, AjaxHelpers) { 'use strict'; + var createMyTeamsView; describe('My Teams View', function() { beforeEach(function() { setFixtures('
'); }); - var createMyTeamsView = function(myTeams) { + createMyTeamsView = function(myTeams) { return new MyTeamsView({ el: '.teams-container', collection: myTeams, diff --git a/lms/djangoapps/teams/static/teams/js/spec/views/team_card_spec.js b/lms/djangoapps/teams/static/teams/js/spec/views/team_card_spec.js index 4902e7f3b7..6ba3186516 100644 --- a/lms/djangoapps/teams/static/teams/js/spec/views/team_card_spec.js +++ b/lms/djangoapps/teams/static/teams/js/spec/views/team_card_spec.js @@ -88,7 +88,7 @@ define(['jquery', expectThumbnailsOrder = function(members) { var thumbnails = view.$('.item-member-thumb img'); expect(thumbnails.length).toBe(members.length); - thumbnails.each(function(index, imgEl) { + thumbnails.each(function(index) { expect(thumbnails.eq(index).attr('alt')).toBe(members[index].username); expect(thumbnails.eq(index).attr('src')).toBe(members[index].image_url); }); diff --git a/lms/djangoapps/teams/static/teams/js/spec/views/team_profile_header_actions_spec.js b/lms/djangoapps/teams/static/teams/js/spec/views/team_profile_header_actions_spec.js index af8a510f46..442a3ce2cd 100644 --- a/lms/djangoapps/teams/static/teams/js/spec/views/team_profile_header_actions_spec.js +++ b/lms/djangoapps/teams/static/teams/js/spec/views/team_profile_header_actions_spec.js @@ -25,26 +25,29 @@ define([ }; }; - createHeaderActionsView = function(requests, maxTeamSize, currentUsername, teamModelData, showEditButton) { - var model = new TeamModel(teamModelData, {parse: true}), - context = TeamSpecHelpers.createMockContext({ - maxTeamSize: maxTeamSize, - userInfo: TeamSpecHelpers.createMockUserInfo({ - username: currentUsername - }) - }); + createHeaderActionsView = + function(requests, maxTeamSize, currentUsername, teamModelData, showEditButton, isInstructorManagedTopic) { + var model = new TeamModel(teamModelData, {parse: true}), + context = TeamSpecHelpers.createMockContext({ + maxTeamSize: maxTeamSize, + userInfo: TeamSpecHelpers.createMockUserInfo({ + username: currentUsername + }) + }); - return new TeamProfileHeaderActionsView( - { - courseID: TeamSpecHelpers.testCourseID, - teamEvents: TeamSpecHelpers.teamEvents, - context: context, - model: model, - topic: TeamSpecHelpers.createMockTopic(), - showEditButton: showEditButton - } - ).render(); - }; + return new TeamProfileHeaderActionsView( + { + courseID: TeamSpecHelpers.testCourseID, + teamEvents: TeamSpecHelpers.teamEvents, + context: context, + model: model, + topic: isInstructorManagedTopic ? + TeamSpecHelpers.createMockInstructorManagedTopic() : + TeamSpecHelpers.createMockTopic(), + showEditButton: showEditButton + } + ).render(); + }; createMembershipData = function(username) { return [ @@ -60,7 +63,12 @@ define([ describe('JoinButton', function() { beforeEach(function() { setFixtures( - '
' + '
\n' + + '
\n' + + '
\n' + + '
\n' + + '
\n' + + '
' ); }); @@ -134,7 +142,9 @@ define([ it('shows already member message', function() { var requests = AjaxHelpers.requests(this); var currentUsername = 'ma1'; - var view = createHeaderActionsView(requests, 1, currentUsername, createTeamModelData('teamA', 'teamAlpha', [])); + var view = + createHeaderActionsView( + requests, 1, currentUsername, createTeamModelData('teamA', 'teamAlpha', [])); // a get request will be sent to get user membership info // because current user is not member of current team @@ -170,6 +180,36 @@ define([ AjaxHelpers.expectNoRequests(requests); }); + it('shows not join instructor managed team message', function() { + var requests = AjaxHelpers.requests(this); + var currentUsername = 'ma1'; + var view = createHeaderActionsView( + requests, + 1, + currentUsername, + createTeamModelData('teamA', 'teamAlpha', []), + false, + true); + + // a get request will be sent to get user membership info + // because current user is not member of current team + AjaxHelpers.expectRequest( + requests, + 'GET', + TeamSpecHelpers.testContext.teamMembershipsUrl + '?' + $.param({ + username: currentUsername, course_id: TeamSpecHelpers.testCourseID + }) + ); + + // Mock the response so that current user is not a member of any team + AjaxHelpers.respondWithJson(requests, {count: 0}); + + // current user is a student and current team belogs to an instructor managed topic + // so the Join Team button is hidden and we should see the correct message + expect(view.$('.action.action-primary').length).toEqual(0); + expect(view.$('.join-team-message').text().trim()).toBe(view.notJoinInstructorManagedTeam); + }); + it('shows correct error message if user fails to join team', function() { var requests = AjaxHelpers.requests(this); @@ -241,10 +281,11 @@ define([ }); it('can navigate to correct url', function() { - var requests = AjaxHelpers.requests(this); + var requests = AjaxHelpers.requests(this), + editButton; spyOn(Backbone.history, 'navigate'); createAndAssertView(requests, true); - var editButton = view.$('.action-edit-team'); + editButton = view.$('.action-edit-team'); expect(editButton.length).toEqual(1); $(editButton).click(); diff --git a/lms/djangoapps/teams/static/teams/js/spec/views/team_profile_spec.js b/lms/djangoapps/teams/static/teams/js/spec/views/team_profile_spec.js index a969d624a7..2e766f46f9 100644 --- a/lms/djangoapps/teams/static/teams/js/spec/views/team_profile_spec.js +++ b/lms/djangoapps/teams/static/teams/js/spec/views/team_profile_spec.js @@ -42,7 +42,7 @@ define([ }; }; - createTeamProfileView = function(requests, options) { + createTeamProfileView = function(requests, options, isInstructorManagedTopic) { teamModel = new TeamModel(createTeamModelData(options), {parse: true}); profileView = new TeamProfileView({ el: $('.profile-view'), @@ -50,6 +50,9 @@ define([ courseID: TeamSpecHelpers.testCourseID, context: TeamSpecHelpers.testContext, model: teamModel, + topic: isInstructorManagedTopic ? + TeamSpecHelpers.createMockInstructorManagedTopic() : + TeamSpecHelpers.createMockTopic(), setFocusToHeaderFunc: function() { $('.teams-content').focus(); } @@ -58,7 +61,7 @@ define([ AjaxHelpers.expectRequest( requests, 'GET', - interpolate( + interpolate( // eslint-disable-line no-undef '/courses/%(courseID)s/discussion/forum/%(topicID)s/inline' + '?page=1&sort_key=activity&sort_order=desc&ajax=1', { @@ -186,6 +189,16 @@ define([ assertTeamDetails(view, 0, false); }); + it('student can not leave instructor managed team', function() { + var requests = AjaxHelpers.requests(this); + + var view = createTeamProfileView( + requests, {country: 'US', language: 'en', membership: DEFAULT_MEMBERSHIP}, true + ); + // When a student is in a team of an instructor-managed topic, he can't see the leave team button. + assertTeamDetails(view, 1, false); + }); + it("wouldn't do anything if user click on Cancel button on dialog", function() { var requests = AjaxHelpers.requests(this); diff --git a/lms/djangoapps/teams/static/teams/js/spec/views/teams_spec.js b/lms/djangoapps/teams/static/teams/js/spec/views/teams_spec.js index 15e6a9f935..192aeeb229 100644 --- a/lms/djangoapps/teams/static/teams/js/spec/views/teams_spec.js +++ b/lms/djangoapps/teams/static/teams/js/spec/views/teams_spec.js @@ -5,12 +5,13 @@ define([ 'teams/js/spec_helpers/team_spec_helpers' ], function(Backbone, TeamCollection, TeamsView, TeamSpecHelpers) { 'use strict'; + var createTeamsView; describe('Teams View', function() { beforeEach(function() { setFixtures('
'); }); - var createTeamsView = function(options) { + createTeamsView = function(options) { return new TeamsView({ el: '.teams-container', collection: options.teams || TeamSpecHelpers.createMockTeams(), @@ -26,11 +27,10 @@ define([ results: testTeamData }) }); + var footerEl = teamsView.$('.teams-paging-footer'); expect(teamsView.$('.teams-paging-header').text()).toMatch('Showing 1-5 out of 6 total'); - - var footerEl = teamsView.$('.teams-paging-footer'); - expect(footerEl.text()).toMatch('1\\s+out of\\s+\/\\s+2'); + expect(footerEl.text()).toMatch('1\\s+out of\\s+\/\\s+2'); // eslint-disable-line no-useless-escape expect(footerEl).not.toHaveClass('hidden'); TeamSpecHelpers.verifyCards(teamsView, testTeamData); 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 e21604752e..9f4fa79547 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 @@ -6,8 +6,9 @@ define([ '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' -], function($, Backbone, Logger, SpecHelpers, AjaxHelpers, PageHelpers, TeamsTabView, TeamSpecHelpers) { + 'teams/js/spec_helpers/team_spec_helpers', + 'underscore' +], function($, Backbone, Logger, SpecHelpers, AjaxHelpers, PageHelpers, TeamsTabView, TeamSpecHelpers, _) { 'use strict'; describe('TeamsTab', function() { @@ -170,7 +171,7 @@ define([ } ], 'fires a page view event for the edit team page': [ - 'teams/' + TeamSpecHelpers.testTopicID + '/' + 'test_team_id/edit-team', + 'teams/' + TeamSpecHelpers.testTopicID + '/test_team_id/edit-team', { page_name: 'edit-team', topic_id: TeamSpecHelpers.testTopicID, @@ -230,17 +231,17 @@ define([ }); describe('Search', function() { - var performSearch = function(requests, teamsTabView) { + var performSearch = function(reqs, teamsTabView) { teamsTabView.$('.search-field').val('foo'); teamsTabView.$('.action-search').click(); verifyTeamsRequest({ order_by: '', text_search: 'foo' }); - AjaxHelpers.respondWithJson(requests, TeamSpecHelpers.createMockTeamsResponse({results: []})); + AjaxHelpers.respondWithJson(reqs, TeamSpecHelpers.createMockTeamsResponse({results: []})); // Expect exactly one search request to be fired - AjaxHelpers.expectNoRequests(requests); + AjaxHelpers.expectNoRequests(reqs); }; it('can search teams', function() { diff --git a/lms/djangoapps/teams/static/teams/js/spec/views/topic_card_spec.js b/lms/djangoapps/teams/static/teams/js/spec/views/topic_card_spec.js index 91ac3202a8..de3d32f889 100644 --- a/lms/djangoapps/teams/static/teams/js/spec/views/topic_card_spec.js +++ b/lms/djangoapps/teams/static/teams/js/spec/views/topic_card_spec.js @@ -3,6 +3,8 @@ define(['jquery', 'teams/js/views/topic_card', 'teams/js/models/topic'], function($, _, TopicCardView, Topic) { + 'use strict'; + describe('Topic card view', function() { var createTopicCardView = function() { return new TopicCardView({ 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 05e47a82ca..d775ca4f62 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 @@ -8,8 +8,9 @@ define([ 'use strict'; describe('Topic Teams View', function() { var createTopicTeamsView = function(options) { - options = options || {}; - var myTeamsCollection = options.myTeamsCollection || TeamSpecHelpers.createMockTeams({results: []}); + var myTeamsCollection; + options = options || {}; // eslint-disable-line no-param-reassign + myTeamsCollection = options.myTeamsCollection || TeamSpecHelpers.createMockTeams({results: []}); return new TopicTeamsView({ el: '.teams-container', model: TeamSpecHelpers.createMockTopic(), @@ -21,14 +22,15 @@ define([ }; var verifyActions = function(teamsView, options) { - if (!options) { - options = {showActions: true}; - } var expectedTitle = 'Are you having trouble finding a team to join?', expectedMessage = 'Browse teams in other topics or search teams in this topic. ' + 'If you still can\'t find a team to join, create a new team in this topic.', title = teamsView.$('.title').text().trim(), message = teamsView.$('.copy').text().trim(); + if (!options) { + options = {showActions: true}; // eslint-disable-line no-param-reassign + } + if (options.showActions) { expect(title).toBe(expectedTitle); expect(message).toBe(expectedMessage); @@ -50,11 +52,9 @@ define([ results: testTeamData }) }); - - expect(teamsView.$('.teams-paging-header').text()).toMatch('Showing 1-5 out of 6 total'); - var footerEl = teamsView.$('.teams-paging-footer'); - expect(footerEl.text()).toMatch('1\\s+out of\\s+\/\\s+2'); + expect(teamsView.$('.teams-paging-header').text()).toMatch('Showing 1-5 out of 6 total'); + expect(footerEl.text()).toMatch('1\\s+out of\\s+\/\\s+2'); // eslint-disable-line no-useless-escape expect(footerEl).not.toHaveClass('hidden'); TeamSpecHelpers.verifyCards(teamsView, testTeamData); diff --git a/lms/djangoapps/teams/static/teams/js/spec/views/topics_spec.js b/lms/djangoapps/teams/static/teams/js/spec/views/topics_spec.js index 26da5fbcd0..b325aede2b 100644 --- a/lms/djangoapps/teams/static/teams/js/spec/views/topics_spec.js +++ b/lms/djangoapps/teams/static/teams/js/spec/views/topics_spec.js @@ -40,7 +40,7 @@ define([ expect(currentCard.text()).toMatch(topic.description); expect(currentCard.text()).toMatch(topic.team_count + ' Teams'); }); - expect(footerEl.text()).toMatch('1\\s+out of\\s+\/\\s+2'); + expect(footerEl.text()).toMatch('1\\s+out of\\s+\/\\s+2'); // eslint-disable-line no-useless-escape expect(footerEl).not.toHaveClass('hidden'); }); 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 3b393f2266..efcfefc8fb 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 @@ -7,10 +7,13 @@ define([ ], function(Backbone, _, TeamCollection, TopicCollection, TopicModel) { 'use strict'; var createMockPostResponse, createMockDiscussionResponse, createAnnotatedContentInfo, createMockThreadResponse, - createMockTopicData, createMockTopicCollection, createMockTopic, + createMockTopicData, createMockTopicCollection, createMockTopic, createMockInstructorManagedTopic, + createMockContext, + testContext, testCourseID = 'course/1', testUser = 'testUser', testTopicID = 'test-topic-1', + testInstructorManagedTopicID = 'test-instructor-managed-topic-1', testTeamDiscussionID = '12345', teamEvents = _.clone(Backbone.Events), testCountries = [ @@ -57,7 +60,7 @@ define([ var createMockTeams = function(responseOptions, options, collectionType) { if (_.isUndefined(collectionType)) { - collectionType = TeamCollection; + collectionType = TeamCollection; // eslint-disable-line no-param-reassign } return new collectionType( createMockTeamsResponse(responseOptions), @@ -243,13 +246,26 @@ define([ { id: testTopicID, name: 'Test Topic 1', - description: 'Test description 1' + description: 'Test description 1', + type: 'open' }, options )); }; - var testContext = { + createMockInstructorManagedTopic = function(options) { + return new TopicModel(_.extend( + { + id: testInstructorManagedTopicID, + name: 'Test Instructor Managed Topic 1', + description: 'Test instructor managed topic description 1', + type: 'public_managed' + }, + options + )); + }; + + testContext = { courseID: testCourseID, topics: { count: 5, @@ -270,11 +286,12 @@ define([ userInfo: createMockUserInfo() }; - var createMockContext = function(options) { + createMockContext = function(options) { return _.extend({}, testContext, options); }; createMockTopicCollection = function(topicData) { + // eslint-disable-next-line no-param-reassign topicData = topicData !== undefined ? topicData : createMockTopicData(1, 5); return new TopicCollection( @@ -310,6 +327,7 @@ define([ createMockUserInfo: createMockUserInfo, createMockContext: createMockContext, createMockTopic: createMockTopic, + createMockInstructorManagedTopic: createMockInstructorManagedTopic, createMockPostResponse: createMockPostResponse, createMockDiscussionResponse: createMockDiscussionResponse, createAnnotatedContentInfo: createAnnotatedContentInfo, diff --git a/lms/djangoapps/teams/static/teams/js/utils/team_analytics.js b/lms/djangoapps/teams/static/teams/js/utils/team_analytics.js index c95df739ad..06c4825aa0 100644 --- a/lms/djangoapps/teams/static/teams/js/utils/team_analytics.js +++ b/lms/djangoapps/teams/static/teams/js/utils/team_analytics.js @@ -9,11 +9,11 @@ 'logger' ], function(Logger) { var TeamAnalytics = { - emitPageViewed: function(page_name, topic_id, team_id) { + emitPageViewed: function(pageName, topicId, teamId) { Logger.log('edx.team.page_viewed', { - page_name: page_name, - topic_id: topic_id, - team_id: team_id + page_name: pageName, + topic_id: topicId, + team_id: teamId }); } }; 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 686c84d391..9dd906940f 100644 --- a/lms/djangoapps/teams/static/teams/js/views/edit_team.js +++ b/lms/djangoapps/teams/static/teams/js/views/edit_team.js @@ -51,7 +51,9 @@ valueAttribute: 'description', editable: 'always', showMessages: false, - helpMessage: gettext('A short description of the team to help other learners understand the goals or direction of the team (maximum 300 characters).') + helpMessage: gettext( + 'A short description of the team to help other learners understand the ' + + 'goals or direction of the team (maximum 300 characters).') }); this.teamLanguageField = new FieldViews.DropdownFieldView({ @@ -62,7 +64,8 @@ showMessages: false, titleIconName: 'fa-comment-o', options: this.context.languages, - helpMessage: gettext('The language that team members primarily use to communicate with each other.') + helpMessage: + gettext('The language that team members primarily use to communicate with each other.') }); this.teamCountryField = new FieldViews.DropdownFieldView({ @@ -78,7 +81,7 @@ }, render: function() { - this.$el.html(_.template(editTeamTemplate)({ + this.$el.html(_.template(editTeamTemplate)({ // xss-lint: disable=javascript-jquery-html primaryButtonTitle: this.primaryButtonTitle, action: this.action, totalMembers: _.isUndefined(this.teamModel) ? 0 : this.teamModel.get('membership').length @@ -101,7 +104,7 @@ createOrUpdateTeam: function(event) { event.preventDefault(); - var view = this, + var view = this, // eslint-disable-line vars-on-top teamLanguage = this.teamLanguageField.fieldValue(), teamCountry = this.teamCountryField.fieldValue(), data = { @@ -122,7 +125,7 @@ saveOptions.contentType = 'application/merge-patch+json'; } - var validationResult = this.validateTeamData(data); + var validationResult = this.validateTeamData(data); // eslint-disable-line vars-on-top if (validationResult.status === false) { this.showMessage(validationResult.message, validationResult.srMessage); return $().promise(); @@ -138,7 +141,7 @@ {trigger: true} ); }) - .fail(function(data) { + .fail(function(data) { // eslint-disable-line no-shadow var response = JSON.parse(data.responseText); var message = gettext('An error occurred. Please try again.'); if ('user_message' in response) { @@ -202,8 +205,8 @@ }, cancelAndGoBack: function(event) { - event.preventDefault(); var url; + event.preventDefault(); if (this.action === 'create') { url = 'topics/' + this.topic.id; } else if (this.action === 'edit') { diff --git a/lms/djangoapps/teams/static/teams/js/views/edit_team_members.js b/lms/djangoapps/teams/static/teams/js/views/edit_team_members.js index 6a79acc44d..425d82e477 100644 --- a/lms/djangoapps/teams/static/teams/js/views/edit_team_members.js +++ b/lms/djangoapps/teams/static/teams/js/views/edit_team_members.js @@ -11,7 +11,8 @@ 'text!teams/templates/edit-team-member.underscore', 'text!teams/templates/date.underscore' ], - function(Backbone, $, _, gettext, TeamModel, TeamUtils, ViewUtils, editTeamMemberTemplate, dateTemplate) { + function( + Backbone, $, _, gettext, TeamModel, TeamUtils, ViewUtils, editTeamMemberTemplate, dateTemplate) { return Backbone.View.extend({ dateTemplate: _.template(dateTemplate), teamMemberTemplate: _.template(editTeamMemberTemplate), @@ -35,9 +36,11 @@ render: function() { if (this.model.get('membership').length === 0) { - this.$el.html('

' + gettext('This team does not have any members.') + '

'); + this.$el.html( // xss-lint: disable=javascript-jquery-html + // eslint-disable-next-line max-len + '

' + gettext('This team does not have any members.') + '

'); // xss-lint: disable=javascript-concat-html } else { - this.$el.html(''); + this.$el.html(''); // xss-lint: disable=javascript-jquery-html this.renderTeamMembers(); } return this; @@ -48,22 +51,29 @@ dateJoined, lastActivity; _.each(this.model.get('membership'), function(membership) { - dateJoined = interpolate( - // Translators: 'date' is a placeholder for a fuzzy, relative timestamp (see: https://github.com/rmm5t/jquery-timeago) + // eslint-disable-next-line no-undef + dateJoined = interpolate( // xss-lint: disable=javascript-interpolate + /* Translators: 'date' is a placeholder for a fuzzy, + * relative timestamp (see: https://github.com/rmm5t/jquery-timeago) + */ gettext('Joined %(date)s'), {date: self.dateTemplate({date: membership.date_joined})}, true ); - lastActivity = interpolate( - // Translators: 'date' is a placeholder for a fuzzy, relative timestamp (see: https://github.com/rmm5t/jquery-timeago) + // eslint-disable-next-line no-undef + lastActivity = interpolate( // xss-lint: disable=javascript-interpolate + /* Translators: 'date' is a placeholder for a fuzzy, + * relative timestamp (see: https://github.com/rmm5t/jquery-timeago) + */ gettext('Last Activity %(date)s'), {date: self.dateTemplate({date: membership.last_activity_at})}, true ); // It is assumed that the team member array is automatically in the order of date joined. - self.$('.edit-members').append(self.teamMemberTemplate({ + // eslint-disable-next-line max-len + self.$('.edit-members').append(self.teamMemberTemplate({ // xss-lint: disable=javascript-jquery-append imageUrl: membership.user.profile_image.image_url_medium, username: membership.user.username, memberProfileUrl: '/u/' + membership.user.username, @@ -81,7 +91,8 @@ ViewUtils.confirmThenRunOperation( gettext('Remove this team member?'), - gettext('This learner will be removed from the team, allowing another learner to take the available spot.'), + gettext('This learner will be removed from the team,' + + 'allowing another learner to take the available spot.'), gettext('Remove'), function() { $.ajax({ diff --git a/lms/djangoapps/teams/static/teams/js/views/instructor_tools.js b/lms/djangoapps/teams/static/teams/js/views/instructor_tools.js index 002db500b7..27df329ae8 100644 --- a/lms/djangoapps/teams/static/teams/js/views/instructor_tools.js +++ b/lms/djangoapps/teams/static/teams/js/views/instructor_tools.js @@ -23,7 +23,7 @@ }, render: function() { - this.$el.html(this.template); + this.$el.html(this.template); // xss-lint: disable=javascript-jquery-html return this; }, @@ -31,7 +31,8 @@ event.preventDefault(); ViewUtils.confirmThenRunOperation( gettext('Delete this team?'), - gettext('Deleting a team is permanent and cannot be undone. All members are removed from the team, and team discussions can no longer be accessed.'), + gettext('Deleting a team is permanent and cannot be undone.' + + 'All members are removed from the team, and team discussions can no longer be accessed.'), gettext('Delete'), _.bind(this.handleDelete, this) ); diff --git a/lms/djangoapps/teams/static/teams/js/views/my_teams.js b/lms/djangoapps/teams/static/teams/js/views/my_teams.js index 8989c32b44..35b520f962 100644 --- a/lms/djangoapps/teams/static/teams/js/views/my_teams.js +++ b/lms/djangoapps/teams/static/teams/js/views/my_teams.js @@ -13,7 +13,9 @@ .done(function() { TeamsView.prototype.render.call(view); if (view.collection.length === 0) { - view.$el.append('

' + gettext('You are not currently a member of any team.') + '

'); + view.$el.append( // xss-lint: disable=javascript-jquery-append + // eslint-disable-next-line max-len + '

' + gettext('You are not currently a member of any team.') + '

'); // xss-lint: disable=javascript-concat-html } }); return this; diff --git a/lms/djangoapps/teams/static/teams/js/views/team_card.js b/lms/djangoapps/teams/static/teams/js/views/team_card.js index 65ceb5e7c5..65ca694189 100644 --- a/lms/djangoapps/teams/static/teams/js/views/team_card.js +++ b/lms/djangoapps/teams/static/teams/js/views/team_card.js @@ -41,11 +41,12 @@ }).reverse(), displayableMemberships = allMemberships.slice(0, 5), maxMemberCount = this.maxTeamSize; - this.$el.html(this.template({ + this.$el.html(this.template({ // xss-lint: disable=javascript-jquery-html membership_message: TeamUtils.teamCapacityText(allMemberships.length, maxMemberCount), memberships: displayableMemberships, has_additional_memberships: displayableMemberships.length < allMemberships.length, - // Translators: "and others" refers to fact that additional members of a team exist that are not displayed. + /* Translators: "and others" refers to fact that additional + * members of a team exist that are not displayed. */ sr_message: gettext('and others') })); return this; @@ -62,7 +63,7 @@ render: function() { // this.$el should be the card meta div - this.$el.append(this.template({ + this.$el.append(this.template({ // xss-lint: disable=javascript-jquery-append country: this.countries[this.model.get('country')], language: this.languages[this.model.get('language')] })); @@ -82,9 +83,12 @@ var lastActivity = moment(this.date), currentLanguage = $('html').attr('lang'); lastActivity.locale(currentLanguage); - this.$el.html( - interpolate( - // Translators: 'date' is a placeholder for a fuzzy, relative timestamp (see: http://momentjs.com/) + this.$el.html( // xss-lint: disable=javascript-jquery-html + // eslint-disable-next-line no-undef + interpolate( // xss-lint: disable=javascript-interpolate + /* Translators: 'date' is a placeholder for a fuzzy, + * relative timestamp (see: http://momentjs.com/) + */ gettext('Last activity %(date)s'), {date: this.template({date: lastActivity.format('MMMM Do YYYY, h:mm:ss a')})}, true @@ -119,7 +123,8 @@ details: function() { return this.detailViews; }, actionClass: 'action-view', actionContent: function() { - return interpolate( + // eslint-disable-next-line no-undef + return interpolate( // xss-lint: disable=javascript-interpolate gettext('View %(span_start)s %(team_name)s %(span_end)s'), {span_start: '', team_name: _.escape(this.model.get('name')), span_end: ''}, true diff --git a/lms/djangoapps/teams/static/teams/js/views/team_profile.js b/lms/djangoapps/teams/static/teams/js/views/team_profile.js index ac605791ac..13bd2fbed7 100644 --- a/lms/djangoapps/teams/static/teams/js/views/team_profile.js +++ b/lms/djangoapps/teams/static/teams/js/views/team_profile.js @@ -31,6 +31,7 @@ this.countries = TeamUtils.selectorOptionsArrayToHashWithBlank(this.context.countries); this.languages = TeamUtils.selectorOptionsArrayToHashWithBlank(this.context.languages); + this.topic = options.topic; this.listenTo(this.model, 'change', this.render); }, @@ -38,7 +39,11 @@ render: function() { var memberships = this.model.get('membership'), discussionTopicID = this.model.get('discussion_topic_id'), - isMember = TeamUtils.isUserMemberOfTeam(memberships, this.context.userInfo.username); + isMember = TeamUtils.isUserMemberOfTeam(memberships, this.context.userInfo.username), + isAdminOrStaff = this.context.userInfo.privileged || this.context.userInfo.staff, + isInstructorManagedTopic = TeamUtils.isInstructorManagedTopic(this.topic.attributes.type); + + var showLeaveLink = isMember && (isAdminOrStaff || !isInstructorManagedTopic); HtmlUtils.setHtml( this.$el, @@ -50,6 +55,7 @@ language: this.languages[this.model.get('language')], membershipText: TeamUtils.teamCapacityText(memberships.length, this.context.maxTeamSize), isMember: isMember, + showLeaveLink: showLeaveLink, hasCapacity: memberships.length < this.context.maxTeamSize, hasMembers: memberships.length >= 1 }) @@ -87,16 +93,17 @@ leaveTeam: function(event) { event.preventDefault(); - var view = this; + var view = this; // eslint-disable-line vars-on-top ViewUtils.confirmThenRunOperation( gettext('Leave this team?'), - gettext("If you leave, you can no longer post in this team's discussions. Your place will be available to another learner."), + gettext("If you leave, you can no longer post in this team's discussions." + + 'Your place will be available to another learner.'), gettext('Confirm'), function() { $.ajax({ type: 'DELETE', url: view.context.teamMembershipDetailUrl.replace('team_id', view.model.get('id')) - }).done(function(data) { + }).done(function() { view.model.fetch() .done(function() { view.teamEvents.trigger('teams:update', { diff --git a/lms/djangoapps/teams/static/teams/js/views/team_profile_header_actions.js b/lms/djangoapps/teams/static/teams/js/views/team_profile_header_actions.js index c2107858d3..1a4bfbbeff 100644 --- a/lms/djangoapps/teams/static/teams/js/views/team_profile_header_actions.js +++ b/lms/djangoapps/teams/static/teams/js/views/team_profile_header_actions.js @@ -13,6 +13,7 @@ errorMessage: gettext('An error occurred. Try again.'), alreadyMemberMessage: gettext('You already belong to another team.'), teamFullMessage: gettext('This team is full.'), + notJoinInstructorManagedTeam: gettext('Cannot join instructor managed team'), events: { 'click .action-primary': 'joinTeam', @@ -39,16 +40,21 @@ // if user is the member of current team then we wouldn't show anything if (!info.memberOfCurrentTeam) { - showJoinButton = !info.alreadyMember && teamHasSpace; - if (info.alreadyMember) { + showJoinButton = false; message = info.memberOfCurrentTeam ? '' : view.alreadyMemberMessage; } else if (!teamHasSpace) { + showJoinButton = false; message = view.teamFullMessage; + } else if (!info.isAdminOrStaff && info.isInstructorManagedTopic) { + showJoinButton = false; + message = view.notJoinInstructorManagedTeam; + } else { + showJoinButton = true; } } - view.$el.html(view.template({ + view.$el.html(view.template({ // xss-lint: disable=javascript-jquery-html showJoinButton: showJoinButton, message: message, showEditButton: view.showEditButton @@ -65,7 +71,7 @@ type: 'POST', url: view.context.teamMembershipsUrl, data: {username: view.context.userInfo.username, team_id: view.model.get('id')} - }).done(function(data) { + }).done(function() { view.model.fetch() .done(function() { view.teamEvents.trigger('teams:update', { @@ -83,11 +89,15 @@ var info = { alreadyMember: false, memberOfCurrentTeam: false, - teamHasSpace: false + teamHasSpace: false, + isAdminOrStaff: false, + isInstructorManagedTopic: false }; + var teamHasSpace = this.model.get('membership').length < maxTeamSize; info.memberOfCurrentTeam = TeamUtils.isUserMemberOfTeam(this.model.get('membership'), username); - var teamHasSpace = this.model.get('membership').length < maxTeamSize; + info.isAdminOrStaff = this.context.userInfo.privileged || this.context.userInfo.staff; + info.isInstructorManagedTopic = TeamUtils.isInstructorManagedTopic(this.topic.attributes.type); if (info.memberOfCurrentTeam) { info.alreadyMember = true; @@ -95,7 +105,7 @@ deferred.resolve(info); } else { if (teamHasSpace) { - var view = this; + var view = this; // eslint-disable-line vars-on-top $.ajax({ type: 'GET', url: view.context.teamMembershipsUrl, diff --git a/lms/djangoapps/teams/static/teams/js/views/team_utils.js b/lms/djangoapps/teams/static/teams/js/views/team_utils.js index 49ee02970e..4c3e09ca8b 100644 --- a/lms/djangoapps/teams/static/teams/js/views/team_utils.js +++ b/lms/djangoapps/teams/static/teams/js/views/team_utils.js @@ -1,8 +1,8 @@ /* Team utility methods*/ (function(define) { 'use strict'; - define(['jquery', 'underscore' - ], function($, _) { + define(['jquery', 'underscore'], + function($, _) { return { /** @@ -20,7 +20,8 @@ }, teamCapacityText: function(memberCount, maxMemberCount) { - return interpolate( + // eslint-disable-next-line no-undef + return interpolate( // xss-lint: disable=javascript-interpolate // Translators: The following message displays the number of members on a team. ngettext( '%(memberCount)s / %(maxMemberCount)s Member', @@ -46,7 +47,7 @@ showMessage: function(message, type) { var $messageElement = $('#teams-message'); if (_.isUndefined(type)) { - type = 'warning'; + type = 'warning'; // eslint-disable-line no-param-reassign } $messageElement.removeClass('is-hidden').addClass(type); $('.teams-content .msg-content .copy').text(message); @@ -58,13 +59,20 @@ */ parseAndShowMessage: function(data, genericErrorMessage, type) { try { - var errors = JSON.parse(data.responseText); + var errors = JSON.parse(data.responseText); // eslint-disable-line vars-on-top this.showMessage( _.isUndefined(errors.user_message) ? genericErrorMessage : errors.user_message, type ); } catch (error) { this.showMessage(genericErrorMessage, type); } + }, + + isInstructorManagedTopic: function(topicType) { + if (topicType === undefined) { + return false; + } + return topicType.toLowerCase() !== 'open'; } }; }); 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 7c1cc6072d..dea3f100f9 100644 --- a/lms/djangoapps/teams/static/teams/js/views/teams_tab.js +++ b/lms/djangoapps/teams/static/teams/js/views/teams_tab.js @@ -78,7 +78,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)] ], function(route) { router.route.apply(router, route); @@ -133,7 +135,9 @@ 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'), @@ -235,7 +239,8 @@ 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', @@ -269,7 +274,8 @@ }); 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, @@ -298,7 +304,8 @@ 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 } @@ -422,6 +429,7 @@ router: self.router, context: self.context, model: team, + topic: topic, setFocusToHeaderFunc: self.setFocusToHeader }); diff --git a/lms/djangoapps/teams/static/teams/js/views/topic_card.js b/lms/djangoapps/teams/static/teams/js/views/topic_card.js index 54b21da047..a65ffe1b45 100644 --- a/lms/djangoapps/teams/static/teams/js/views/topic_card.js +++ b/lms/djangoapps/teams/static/teams/js/views/topic_card.js @@ -14,8 +14,9 @@ }, render: function() { - var team_count = this.model.get('team_count'); - this.$el.html(_.escape(interpolate( + var team_count = this.model.get('team_count'); // eslint-disable-line camelcase + // eslint-disable-next-line no-undef, max-len + this.$el.html(_.escape(interpolate( // xss-lint: disable=javascript-jquery-html,javascript-interpolate ngettext('%(team_count)s Team', '%(team_count)s Teams', team_count), {team_count: team_count}, true @@ -42,11 +43,13 @@ details: function() { return this.detailViews; }, actionClass: 'action-view', actionContent: function() { - var screenReaderText = _.escape(interpolate( + // eslint-disable-next-line no-undef + var screenReaderText = _.escape(interpolate( // xss-lint: disable=javascript-interpolate gettext('View Teams in the %(topic_name)s Topic'), {topic_name: this.model.get('name')}, true )); - return '' + screenReaderText + ''; // eslint-disable-line max-len + // eslint-disable-next-line max-len + return '' + screenReaderText + ''; // xss-lint: disable=javascript-concat-html } }); 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 b513579379..a389dfb01e 100644 --- a/lms/djangoapps/teams/static/teams/js/views/topic_teams.js +++ b/lms/djangoapps/teams/static/teams/js/views/topic_teams.js @@ -5,8 +5,9 @@ 'gettext', 'teams/js/views/teams', 'common/js/components/views/paging_header', - 'text!teams/templates/team-actions.underscore' - ], function(Backbone, gettext, TeamsView, PagingHeader, teamActionsTemplate) { + 'text!teams/templates/team-actions.underscore', + 'underscore' + ], function(Backbone, gettext, TeamsView, PagingHeader, teamActionsTemplate, _) { var TopicTeamsView = TeamsView.extend({ events: { 'click a.browse-teams': 'browseTeams', @@ -35,7 +36,7 @@ this.collection.refresh().done(function() { TeamsView.prototype.render.call(self); if (self.canUserCreateTeam()) { - var message = interpolate_text( + var message = interpolate_text( // eslint-disable-line vars-on-top, 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: @@ -44,7 +45,10 @@ // 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: '', search_span_start: '', @@ -52,7 +56,8 @@ span_end: '' } ); - self.$el.append(_.template(teamActionsTemplate)({message: message})); + // eslint-disable-next-line max-len + self.$el.append(_.template(teamActionsTemplate)({message: message})); // xss-lint: disable=javascript-jquery-append } }); 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 4f9e8aa850..85fb750f45 100644 --- a/lms/djangoapps/teams/static/teams/js/views/topics.js +++ b/lms/djangoapps/teams/static/teams/js/views/topics.js @@ -5,8 +5,9 @@ 'teams/js/views/topic_card', 'teams/js/views/team_utils', 'common/js/components/views/paging_header', - 'common/js/components/views/paginated_view' - ], function(gettext, TopicCardView, TeamUtils, PagingHeader, PaginatedView) { + 'common/js/components/views/paginated_view', + 'underscore' + ], function(gettext, TopicCardView, TeamUtils, PagingHeader, PaginatedView, _) { var TopicsView = PaginatedView.extend({ type: 'topics', diff --git a/lms/djangoapps/teams/static/teams/templates/team-profile.underscore b/lms/djangoapps/teams/static/teams/templates/team-profile.underscore index 959b5800e6..6bb3621c03 100644 --- a/lms/djangoapps/teams/static/teams/templates/team-profile.underscore +++ b/lms/djangoapps/teams/static/teams/templates/team-profile.underscore @@ -45,7 +45,7 @@ <% } %> - <% if (isMember) { %> + <% if (showLeaveLink) { %>
diff --git a/lms/djangoapps/teams/tests/test_serializers.py b/lms/djangoapps/teams/tests/test_serializers.py index 27ee6445ef..8bce6a8ec5 100644 --- a/lms/djangoapps/teams/tests/test_serializers.py +++ b/lms/djangoapps/teams/tests/test_serializers.py @@ -83,12 +83,12 @@ class TopicSerializerTestCase(SerializerTestCase): """ with self.assertNumQueries(1): serializer = TopicSerializer( - self.course.teamsets[0].cleaned_data_old_format, + self.course.teamsets[0].cleaned_data, context={'course_id': self.course.id}, ) self.assertEqual( serializer.data, - {u'name': u'Tøpic', u'description': u'The bést topic!', u'id': u'0', u'team_count': 0} + {u'name': u'Tøpic', u'description': u'The bést topic!', u'id': u'0', u'team_count': 0, u'type': u'open'} ) def test_topic_with_team_count(self): @@ -101,17 +101,17 @@ class TopicSerializerTestCase(SerializerTestCase): ) with self.assertNumQueries(1): serializer = TopicSerializer( - self.course.teamsets[0].cleaned_data_old_format, + self.course.teamsets[0].cleaned_data, context={'course_id': self.course.id}, ) self.assertEqual( serializer.data, - {u'name': u'Tøpic', u'description': u'The bést topic!', u'id': u'0', u'team_count': 1} + {u'name': u'Tøpic', u'description': u'The bést topic!', u'id': u'0', u'team_count': 1, u'type': u'open'} ) def test_scoped_within_course(self): """Verify that team count is scoped within a course.""" - duplicate_topic = self.course.teamsets[0].cleaned_data_old_format + duplicate_topic = self.course.teamsets[0].cleaned_data second_course = CourseFactory.create( teams_configuration=TeamsConfig({ "max_team_size": 10, @@ -122,12 +122,12 @@ class TopicSerializerTestCase(SerializerTestCase): CourseTeamFactory.create(course_id=second_course.id, topic_id=duplicate_topic[u'id']) with self.assertNumQueries(1): serializer = TopicSerializer( - self.course.teamsets[0].cleaned_data_old_format, + self.course.teamsets[0].cleaned_data, context={'course_id': self.course.id}, ) self.assertEqual( serializer.data, - {u'name': u'Tøpic', u'description': u'The bést topic!', u'id': u'0', u'team_count': 1} + {u'name': u'Tøpic', u'description': u'The bést topic!', u'id': u'0', u'team_count': 1, u'type': u'open'} ) @@ -153,7 +153,12 @@ class BaseTopicSerializerTestCase(SerializerTestCase): created topics. """ topics = [ - {u'name': u'Tøpic {}'.format(i), u'description': u'The bést topic! {}'.format(i), u'id': six.text_type(i)} + { + 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' + } for i in six.moves.range(num_topics) ] for topic in topics: diff --git a/lms/djangoapps/teams/tests/test_views.py b/lms/djangoapps/teams/tests/test_views.py index ca9546a89b..2e98115c60 100644 --- a/lms/djangoapps/teams/tests/test_views.py +++ b/lms/djangoapps/teams/tests/test_views.py @@ -1416,6 +1416,21 @@ class TestCreateMembershipAPI(EventTestMixin, TeamAPITestCase): user='staff' ) + @patch('lms.djangoapps.teams.api.is_instructor_managed_team', return_value=True) + def test_staff_join_instructor_managed_team(self, *args): # pylint: disable=unused-argument + self.post_create_membership( + 200, + self.build_membership_data_raw(self.users['staff'].username, self.solar_team.team_id), + user='staff' + ) + + @patch('lms.djangoapps.teams.api.is_instructor_managed_team', return_value=True) + def test_student_join_instructor_managed_team(self, *args): # pylint: disable=unused-argument + self.post_create_membership( + 403, + self.build_membership_data_raw(self.users['student_enrolled_not_on_team'].username, self.solar_team.team_id) + ) + @ddt.data('student_enrolled', 'staff', 'course_staff') def test_join_twice(self, user): response = self.post_create_membership( @@ -1577,6 +1592,11 @@ class TestDeleteMembershipAPI(EventTestMixin, TeamAPITestCase): def test_missing_membership(self): self.delete_membership(self.wind_team.team_id, self.users['student_enrolled'].username, 404) + @patch('lms.djangoapps.teams.api.is_instructor_managed_team', return_value=True) + def test_student_leave_instructor_managed_team(self, *args): # pylint: disable=unused-argument + self.delete_membership( + self.solar_team.team_id, self.users['student_enrolled'].username, 403, user='student_enrolled') + class TestElasticSearchErrors(TeamAPITestCase): """Test that the Team API is robust to Elasticsearch connection errors.""" diff --git a/lms/djangoapps/teams/views.py b/lms/djangoapps/teams/views.py index 2c5d80c5e5..529caebf81 100644 --- a/lms/djangoapps/teams/views.py +++ b/lms/djangoapps/teams/views.py @@ -34,6 +34,7 @@ from lms.djangoapps.teams.api import ( 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 @@ -362,7 +363,7 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView): permission_classes = (permissions.IsAuthenticated,) serializer_class = CourseTeamSerializer - def get(self, request): + def get(self, request): # pylint: disable=too-many-statements """GET /api/team/v0/teams/""" result_filter = {} @@ -852,7 +853,7 @@ def get_alphabetical_topics(course_module): list: a list of sorted team topics """ return sorted( - course_module.teams_configuration.cleaned_data_old_format['topics'], + course_module.teams_configuration.cleaned_data['team_sets'], key=lambda t: t['name'].lower(), ) @@ -926,7 +927,7 @@ class TopicDetailView(APIView): organization_protection_status = user_organization_protection_status(request.user, course_id) serializer = TopicSerializer( - topic.cleaned_data_old_format, + topic.cleaned_data, context={ 'course_id': course_id, 'organization_protection_status': organization_protection_status @@ -1167,6 +1168,12 @@ class MembershipListView(ExpandableFieldViewMixin, GenericAPIView): status=status.HTTP_400_BAD_REQUEST ) + if not can_user_modify_team(request.user, team.course_id, team): + return Response( + build_api_error(ugettext_noop("You can't join an instructor managed team.")), + status=status.HTTP_403_FORBIDDEN + ) + try: membership = team.add_user(user) emit_team_event( @@ -1310,6 +1317,12 @@ 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): + return Response( + build_api_error(ugettext_noop("You can't leave an instructor managed team.")), + status=status.HTTP_403_FORBIDDEN + ) + membership = self.get_membership(username, team) removal_method = 'self_removal' if 'admin' in request.query_params: