From 63da1907eb6e9c580d95d85e35177ba3811a3bbd Mon Sep 17 00:00:00 2001 From: Andy Armstrong Date: Tue, 21 Jul 2015 17:39:44 -0400 Subject: [PATCH 1/5] Show inline discussion component on Team view TNL-2515 --- .../discussion/discussion_module_view.coffee | 5 +- .../coffee/src/discussion/templates.coffee | 0 .../views/discussion_thread_edit_view.js | 26 +- .../views/discussion_thread_view.coffee | 2 + .../src/discussion/views/new_post_view.coffee | 1 - .../test/acceptance/pages/lms/discussion.py | 6 +- common/test/acceptance/pages/lms/teams.py | 48 ++ .../test/acceptance/tests/lms/test_teams.py | 132 ++-- .../django_comment_client/forum/views.py | 5 + .../teams/static/teams/js/models/team.js | 4 + .../teams/static/teams/js/models/topic.js | 4 + .../topic_collection_spec.js | 0 .../static/teams/js/spec/edit_team_spec.js | 4 +- .../teams/js/spec/teams_factory_spec.js | 8 +- .../static/teams/js/spec/teams_tab_spec.js | 80 --- .../js/spec/views/team_discussion_spec.js | 214 ++++++ .../teams/js/spec/views/team_profile_spec.js | 50 ++ .../teams/js/spec/{ => views}/teams_spec.js | 2 +- .../teams/js/spec/views/teams_tab_spec.js | 92 +++ .../js/spec/{ => views}/topic_card_spec.js | 0 .../teams/js/spec/{ => views}/topics_spec.js | 0 .../spec_helpers/team_discussion_helpers.js | 133 ++++ .../teams/static/teams/js/views/edit_team.js | 325 +++++---- .../teams/static/teams/js/views/team_card.js | 5 + .../static/teams/js/views/team_discussion.js | 27 + .../static/teams/js/views/team_profile.js | 33 + .../teams/static/teams/js/views/teams.js | 2 + .../teams/static/teams/js/views/teams_tab.js | 629 ++++++++++-------- .../teams/templates/team-profile.underscore | 7 + .../teams/templates/teams/teams.html | 22 +- lms/static/js/spec/main.js | 225 ++++++- lms/static/js_test.yml | 3 + lms/static/lms/js/build.js | 1 + lms/static/lms/js/require-config.js | 1 + 34 files changed, 1492 insertions(+), 604 deletions(-) delete mode 100644 common/static/coffee/src/discussion/templates.coffee rename lms/djangoapps/teams/static/teams/js/spec/{ => collections}/topic_collection_spec.js (100%) delete mode 100644 lms/djangoapps/teams/static/teams/js/spec/teams_tab_spec.js create mode 100644 lms/djangoapps/teams/static/teams/js/spec/views/team_discussion_spec.js create mode 100644 lms/djangoapps/teams/static/teams/js/spec/views/team_profile_spec.js rename lms/djangoapps/teams/static/teams/js/spec/{ => views}/teams_spec.js (98%) create mode 100644 lms/djangoapps/teams/static/teams/js/spec/views/teams_tab_spec.js rename lms/djangoapps/teams/static/teams/js/spec/{ => views}/topic_card_spec.js (100%) rename lms/djangoapps/teams/static/teams/js/spec/{ => views}/topics_spec.js (100%) create mode 100644 lms/djangoapps/teams/static/teams/js/spec_helpers/team_discussion_helpers.js create mode 100644 lms/djangoapps/teams/static/teams/js/views/team_discussion.js create mode 100644 lms/djangoapps/teams/static/teams/js/views/team_profile.js create mode 100644 lms/djangoapps/teams/static/teams/templates/team-profile.underscore diff --git a/common/static/coffee/src/discussion/discussion_module_view.coffee b/common/static/coffee/src/discussion/discussion_module_view.coffee index 9733e6658f..0fd883d6c7 100644 --- a/common/static/coffee/src/discussion/discussion_module_view.coffee +++ b/common/static/coffee/src/discussion/discussion_module_view.coffee @@ -10,10 +10,11 @@ if Backbone? "click .discussion-paginator a": "navigateToPage" page_re: /\?discussion_page=(\d+)/ - initialize: -> + initialize: (options) -> @toggleDiscussionBtn = @$(".discussion-show") # Set the page if it was set in the URL. This is used to allow deep linking to pages match = @page_re.exec(window.location.href) + @context = options.context or "course" # allowed values are "course" or "standalone" if match @page = parseInt(match[1]) else @@ -105,6 +106,7 @@ if Backbone? el: @$("article#thread_#{thread.id}"), model: thread, mode: "inline", + context: @context, course_settings: @course_settings, topicId: discussionId ) @@ -141,6 +143,7 @@ if Backbone? el: article, model: thread, mode: "inline", + context: @context, course_settings: @course_settings, topicId: @$el.data("discussion-id") ) diff --git a/common/static/coffee/src/discussion/templates.coffee b/common/static/coffee/src/discussion/templates.coffee deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/common/static/coffee/src/discussion/views/discussion_thread_edit_view.js b/common/static/coffee/src/discussion/views/discussion_thread_edit_view.js index 316c3aff4f..bd82bad796 100644 --- a/common/static/coffee/src/discussion/views/discussion_thread_edit_view.js +++ b/common/static/coffee/src/discussion/views/discussion_thread_edit_view.js @@ -18,6 +18,7 @@ this.course_settings = options.course_settings; this.threadType = this.model.get('thread_type'); this.topicId = this.model.get('commentable_id'); + this.context = options.context || 'course'; _.bindAll(this); return this; }, @@ -31,11 +32,15 @@ threadTypeTemplate = _.template($("#thread-type-template").html()); this.addField(threadTypeTemplate({form_id: formId})); this.$("#" + formId + "-post-type-" + this.threadType).attr('checked', true); - this.topicView = new DiscussionTopicMenuView({ - topicId: this.topicId, - course_settings: this.course_settings - }); - this.addField(this.topicView.render()); + // Only allow the topic field for course threads, as standalone threads + // cannot be moved. + if (this.context === 'course') { + this.topicView = new DiscussionTopicMenuView({ + topicId: this.topicId, + course_settings: this.course_settings + }); + this.addField(this.topicView.render()); + } DiscussionUtil.makeWmdEditor(this.$el, $.proxy(this.$, this), 'edit-post-body'); return this; }, @@ -53,13 +58,14 @@ var title = this.$('.edit-post-title').val(), threadType = this.$(".post-type-input:checked").val(), body = this.$('.edit-post-body textarea').val(), - commentableId = this.topicView.getCurrentTopicId(), postData = { title: title, thread_type: threadType, - body: body, - commentable_id: commentableId + body: body }; + if (this.topicView) { + postData.commentable_id = this.topicView.getCurrentTopicId(); + } return DiscussionUtil.safeAjax({ $elem: this.submitBtn, @@ -75,7 +81,9 @@ this.$('.edit-post-title').val('').attr('prev-text', ''); this.$('.edit-post-body textarea').val('').attr('prev-text', ''); this.$('.wmd-preview p').html(''); - postData.courseware_title = this.topicView.getFullTopicName(); + if (this.topicView) { + postData.courseware_title = this.topicView.getFullTopicName(); + } this.model.set(postData).unset('abbreviatedBody'); this.trigger('thread:updated'); if (this.threadType !== threadType) { diff --git a/common/static/coffee/src/discussion/views/discussion_thread_view.coffee b/common/static/coffee/src/discussion/views/discussion_thread_view.coffee index f369cf6619..798a4022b6 100644 --- a/common/static/coffee/src/discussion/views/discussion_thread_view.coffee +++ b/common/static/coffee/src/discussion/views/discussion_thread_view.coffee @@ -19,6 +19,7 @@ if Backbone? initialize: (options) -> super() @mode = options.mode or "inline" # allowed values are "tab" or "inline" + @context = options.context or "course" # allowed values are "course" or "standalone" if @mode not in ["tab", "inline"] throw new Error("invalid mode: " + @mode) @@ -300,6 +301,7 @@ if Backbone? container: @$('.thread-content-wrapper') model: @model mode: @mode + context: @context course_settings: @options.course_settings ) @editView.bind "thread:updated thread:cancel_edit", @closeEditView diff --git a/common/static/coffee/src/discussion/views/new_post_view.coffee b/common/static/coffee/src/discussion/views/new_post_view.coffee index ccbb313874..75ad7a91bc 100644 --- a/common/static/coffee/src/discussion/views/new_post_view.coffee +++ b/common/static/coffee/src/discussion/views/new_post_view.coffee @@ -87,7 +87,6 @@ if Backbone? url: url type: "POST" dataType: 'json' - async: false # TODO when the rest of the stuff below is made to work properly.. data: thread_type: thread_type title: title diff --git a/common/test/acceptance/pages/lms/discussion.py b/common/test/acceptance/pages/lms/discussion.py index e92cc556a4..8a2c260e8a 100644 --- a/common/test/acceptance/pages/lms/discussion.py +++ b/common/test/acceptance/pages/lms/discussion.py @@ -389,7 +389,7 @@ class InlineDiscussionPage(PageObject): def __init__(self, browser, discussion_id): super(InlineDiscussionPage, self).__init__(browser) self._discussion_selector = ( - "body.courseware .discussion-module[data-discussion-id='{discussion_id}'] ".format( + ".discussion-module[data-discussion-id='{discussion_id}'] ".format( discussion_id=discussion_id ) ) @@ -418,6 +418,10 @@ class InlineDiscussionPage(PageObject): def get_num_displayed_threads(self): return len(self._find_within(".discussion-thread")) + def has_thread(self, thread_id): + """Returns true if this page is showing the thread with the specified id.""" + return self._find_within('.discussion-thread#thread_{}'.format(thread_id)).present + def element_exists(self, selector): return self.q(css=self._discussion_selector + " " + selector).present diff --git a/common/test/acceptance/pages/lms/teams.py b/common/test/acceptance/pages/lms/teams.py index 09f6f8d698..3eb3618578 100644 --- a/common/test/acceptance/pages/lms/teams.py +++ b/common/test/acceptance/pages/lms/teams.py @@ -4,6 +4,7 @@ Teams pages. """ from .course_page import CoursePage +from .discussion import InlineDiscussionPage from ..common.paging import PaginatedUIMixin from .fields import FieldsMixin @@ -179,3 +180,50 @@ class CreateTeamPage(CoursePage, FieldsMixin): """Click on cancel team button""" self.q(css='.create-team .action-cancel').first.click() self.wait_for_ajax() + + +class TeamPage(CoursePage, PaginatedUIMixin): + """ + The page for a specific Team within the Teams tab + """ + def __init__(self, browser, course_id, team=None): + """ + Set up `self.url_path` on instantiation, since it dynamically + reflects the current team. + """ + super(TeamPage, self).__init__(browser, course_id) + self.team = team + if self.team: + self.url_path = "teams/#teams/{topic_id}/{team_id}".format( + topic_id=self.team['topic_id'], team_id=self.team['id'] + ) + + def is_browser_on_page(self): + """Check if we're on the teams list page for a particular team.""" + if self.team: + if not self.url.endswith(self.url_path): + return False + return self.q(css='.team-profile').present + + @property + def discussion_id(self): + """Get the id of the discussion module on the page""" + return self.q(css='div.discussion-module').attrs('data-discussion-id')[0] + + @property + def discussion_page(self): + """Get the discussion as a bok_choy page object""" + if not hasattr(self, '_discussion_page'): + # pylint: disable=attribute-defined-outside-init + self._discussion_page = InlineDiscussionPage(self.browser, self.discussion_id) + return self._discussion_page + + @property + def team_name(self): + """Get the team's name as displayed in the page header""" + return self.q(css='.page-header .page-title')[0].text + + @property + def team_description(self): + """Get the team's description as displayed in the page header""" + return self.q(css=TEAMS_HEADER_CSS + ' .page-description')[0].text diff --git a/common/test/acceptance/tests/lms/test_teams.py b/common/test/acceptance/tests/lms/test_teams.py index 7e070693a8..1dff334ed5 100644 --- a/common/test/acceptance/tests/lms/test_teams.py +++ b/common/test/acceptance/tests/lms/test_teams.py @@ -4,14 +4,19 @@ Acceptance tests for the teams feature. import json from nose.plugins.attrib import attr +from uuid import uuid4 from ..helpers import UniqueCourseTest -from ...pages.lms.teams import TeamsPage, BrowseTopicsPage, BrowseTeamsPage, CreateTeamPage from ...fixtures import LMS_BASE_URL from ...fixtures.course import CourseFixture -from ...pages.lms.tab_nav import TabNavPage +from ...fixtures.discussion import ( + Thread, + MultipleThreadFixture +) from ...pages.lms.auto_auth import AutoAuthPage from ...pages.lms.course_info import CourseInfoPage +from ...pages.lms.tab_nav import TabNavPage +from ...pages.lms.teams import TeamsPage, BrowseTopicsPage, BrowseTeamsPage, CreateTeamPage, TeamPage class TeamsTabBase(UniqueCourseTest): @@ -26,6 +31,33 @@ class TeamsTabBase(UniqueCourseTest): """Create `num_topics` test topics.""" return [{u"description": str(i), u"name": str(i), u"id": i} for i in xrange(num_topics)] + def create_teams(self, topic, num_teams): + """Create `num_teams` teams belonging to `topic`.""" + teams = [] + for i in xrange(num_teams): + team = { + 'course_id': self.course_id, + 'topic_id': topic['id'], + 'name': 'Team {}'.format(i), + 'description': 'Description {}'.format(i) + } + response = self.course_fixture.session.post( + LMS_BASE_URL + '/api/team/v0/teams/', + data=json.dumps(team), + headers=self.course_fixture.headers + ) + teams.append(json.loads(response.text)) + return teams + + def create_membership(self, username, team_id): + """Assign `username` to `team_id`.""" + response = self.course_fixture.session.post( + LMS_BASE_URL + '/api/team/v0/team_membership/', + data=json.dumps({'username': username, 'team_id': team_id}), + headers=self.course_fixture.headers + ) + return json.loads(response.text) + def set_team_configuration(self, configuration, enroll_in_course=True, global_staff=False): """ Sets team configuration on the course and calls auto-auth on the user. @@ -272,33 +304,6 @@ class BrowseTeamsWithinTopicTest(TeamsTabBase): self.browse_teams_page = BrowseTeamsPage(self.browser, self.course_id, self.topic) self.topics_page = BrowseTopicsPage(self.browser, self.course_id) - def create_teams(self, num_teams): - """Create `num_teams` teams belonging to `self.topic`.""" - teams = [] - for i in xrange(num_teams): - team = { - 'course_id': self.course_id, - 'topic_id': self.topic['id'], - 'name': 'Team {}'.format(i), - 'description': 'Description {}'.format(i) - } - response = self.course_fixture.session.post( - LMS_BASE_URL + '/api/team/v0/teams/', - data=json.dumps(team), - headers=self.course_fixture.headers - ) - teams.append(json.loads(response.text)) - return teams - - def create_membership(self, username, team_id): - """Assign `username` to `team_id`.""" - response = self.course_fixture.session.post( - LMS_BASE_URL + '/api/team/v0/team_membership/', - data=json.dumps({'username': username, 'team_id': team_id}), - headers=self.course_fixture.headers - ) - return json.loads(response.text) - def verify_page_header(self): """Verify that the page header correctly reflects the current topic's name and description.""" self.assertEqual(self.browse_teams_page.header_topic_name, self.topic['name']) @@ -380,7 +385,7 @@ class BrowseTeamsWithinTopicTest(TeamsTabBase): And I should see a button to add a team And I should not see a pagination footer """ - teams = self.create_teams(self.TEAMS_PAGE_SIZE) + teams = self.create_teams(self.topic, self.TEAMS_PAGE_SIZE) self.browse_teams_page.visit() self.verify_page_header() self.assertEqual(self.browse_teams_page.get_pagination_header_text(), 'Showing 1-10 out of 10 total') @@ -403,7 +408,7 @@ class BrowseTeamsWithinTopicTest(TeamsTabBase): And when I click on the previous page button Then I should see that I am on the first page of results """ - teams = self.create_teams(self.TEAMS_PAGE_SIZE + 1) + teams = self.create_teams(self.topic, self.TEAMS_PAGE_SIZE + 1) self.browse_teams_page.visit() self.verify_page_header() self.verify_on_page(1, teams, 'Showing 1-10 out of 11 total', True) @@ -425,7 +430,7 @@ class BrowseTeamsWithinTopicTest(TeamsTabBase): When I input the first page Then I should see that I am on the first page of results """ - teams = self.create_teams(self.TEAMS_PAGE_SIZE + 10) + teams = self.create_teams(self.topic, self.TEAMS_PAGE_SIZE + 10) self.browse_teams_page.visit() self.verify_page_header() self.verify_on_page(1, teams, 'Showing 1-10 out of 20 total', True) @@ -445,7 +450,7 @@ class BrowseTeamsWithinTopicTest(TeamsTabBase): And I should see the team for that topic And I should see that the team card shows my membership """ - teams = self.create_teams(1) + teams = self.create_teams(self.topic, 1) self.browse_teams_page.visit() self.verify_page_header() self.verify_teams(teams) @@ -615,23 +620,18 @@ class CreateTeamTest(TeamsTabBase): Then I should see the Create Team header and form When I fill all the fields present with appropriate data And I click Create button - Then I should see teams list page with newly created team. + Then I should see the page for my team """ - self.assertEqual(self.browse_teams_page.get_pagination_header_text(), 'Showing 0 out of 0 total') self.verify_and_navigate_to_create_team_page() self.fill_create_form() self.create_team_page.submit_form() - self.assertTrue(self.browse_teams_page.is_browser_on_page()) - self.assertEqual(self.browse_teams_page.get_pagination_header_text(), 'Showing 1 out of 1 total') - # Verify the newly created team content. - team_card = self.browse_teams_page.team_cards.results[0] - self.assertEqual(team_card.find_element_by_css_selector('.card-title').text, self.team_name) - self.assertEqual( - team_card.find_element_by_css_selector('.card-description').text, - 'The Avengers are a fictional team of superheroes.' - ) + # Verify that the page is shown for the new team + team_page = TeamPage(self.browser, self.course_id) + team_page.wait_for_page() + self.assertEqual(team_page.team_name, self.team_name) + self.assertEqual(team_page.team_description, 'The Avengers are a fictional team of superheroes.') def test_user_can_cancel_the_team_creation(self): """ @@ -649,3 +649,47 @@ class CreateTeamTest(TeamsTabBase): self.assertTrue(self.browse_teams_page.is_browser_on_page()) self.assertEqual(self.browse_teams_page.get_pagination_header_text(), 'Showing 0 out of 0 total') + + +@attr('shard_5') +class TeamPageTest(TeamsTabBase): + """Tests for viewing a specific team""" + def setUp(self): + super(TeamPageTest, self).setUp() + self.topic = {u"name": u"Example Topic", u"id": "example_topic", u"description": "Description"} + self.set_team_configuration({'course_id': self.course_id, 'max_team_size': 10, 'topics': [self.topic]}) + self.team = self.create_teams(self.topic, 1)[0] + self.create_membership(self.user_info['username'], self.team['id']) + self.team_page = TeamPage(self.browser, self.course_id, self.team) + + def setup_thread(self): + """ + Set up the discussion thread for the team. + """ + thread = Thread( + id="test_thread_{}".format(uuid4().hex), + commentable_id=self.team['discussion_topic_id'], + body="Dummy text body." + ) + thread_fixture = MultipleThreadFixture([thread]) + thread_fixture.push() + return thread + + def test_discussion_on_team_page(self): + """ + Scenario: Team Page renders a team discussion. + Given I am enrolled in a course with a team configuration, a topic, + and a team belonging to that topic + When a thread exists in the team's discussion + And I visit the Team page for that team + Then I should see a discussion with the correct discussion_id + And I should see the existing thread + """ + thread = self.setup_thread() + self.team_page.visit() + self.assertEqual(self.team_page.discussion_id, self.team['discussion_topic_id']) + discussion = self.team_page.discussion_page + self.assertTrue(discussion.is_browser_on_page()) + self.assertTrue(discussion.is_discussion_expanded()) + self.assertEqual(discussion.get_num_displayed_threads(), 1) + self.assertTrue(discussion.has_thread(thread['id'])) diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index 2221db75f9..6d57b0f983 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -339,6 +339,11 @@ def single_thread(request, course_key, discussion_id, thread_id): raise Http404 raise + # Verify that the student has access to this thread if belongs to a course discussion module + thread_context = getattr(thread, "context", "course") + if thread_context == "course" and not utils.discussion_category_id_access(course, request.user, discussion_id): + raise Http404 + # verify that the thread belongs to the requesting student's cohort if is_commentable_cohorted(course_key, discussion_id) and not is_moderator: user_group_id = get_cohort_id(request.user, course_key) diff --git a/lms/djangoapps/teams/static/teams/js/models/team.js b/lms/djangoapps/teams/static/teams/js/models/team.js index 32a4531beb..be480904d3 100644 --- a/lms/djangoapps/teams/static/teams/js/models/team.js +++ b/lms/djangoapps/teams/static/teams/js/models/team.js @@ -16,6 +16,10 @@ country: '', language: '', membership: [] + }, + + initialize: function(options) { + this.url = options.url; } }); return Team; diff --git a/lms/djangoapps/teams/static/teams/js/models/topic.js b/lms/djangoapps/teams/static/teams/js/models/topic.js index 774e992f39..ffd576024a 100644 --- a/lms/djangoapps/teams/static/teams/js/models/topic.js +++ b/lms/djangoapps/teams/static/teams/js/models/topic.js @@ -10,6 +10,10 @@ description: '', team_count: 0, id: '' + }, + + initialize: function(options) { + this.url = options.url; } }); return Topic; diff --git a/lms/djangoapps/teams/static/teams/js/spec/topic_collection_spec.js b/lms/djangoapps/teams/static/teams/js/spec/collections/topic_collection_spec.js similarity index 100% rename from lms/djangoapps/teams/static/teams/js/spec/topic_collection_spec.js rename to lms/djangoapps/teams/static/teams/js/spec/collections/topic_collection_spec.js diff --git a/lms/djangoapps/teams/static/teams/js/spec/edit_team_spec.js b/lms/djangoapps/teams/static/teams/js/spec/edit_team_spec.js index 553389c778..0dd2563069 100644 --- a/lms/djangoapps/teams/static/teams/js/spec/edit_team_spec.js +++ b/lms/djangoapps/teams/static/teams/js/spec/edit_team_spec.js @@ -102,10 +102,10 @@ define([ teamEditView.$('.create-team.form-actions .action-primary').click(); AjaxHelpers.expectJsonRequest(requests, 'POST', teamsUrl, teamsData); - AjaxHelpers.respondWithJson(requests, teamsData); + AjaxHelpers.respondWithJson(requests, _.extend(_.extend({}, teamsData), { id: '123'})); expect(teamEditView.$('.create-team.wrapper-msg .copy').text().trim().length).toBe(0); - expect(Backbone.history.navigate.calls[0].args).toContain('topics/awesomeness'); + expect(Backbone.history.navigate.calls[0].args).toContain('teams/awesomeness/123'); }); it('shows validation error message when field is empty', function () { diff --git a/lms/djangoapps/teams/static/teams/js/spec/teams_factory_spec.js b/lms/djangoapps/teams/static/teams/js/spec/teams_factory_spec.js index dae5e029c1..911412ea88 100644 --- a/lms/djangoapps/teams/static/teams/js/spec/teams_factory_spec.js +++ b/lms/djangoapps/teams/static/teams/js/spec/teams_factory_spec.js @@ -2,17 +2,17 @@ define(["jquery", "backbone", "teams/js/teams_tab_factory"], function($, Backbone, TeamsTabFactory) { 'use strict'; - describe("Teams tab", function() { + describe("Teams Tab Factory", function() { var teamsTab; beforeEach(function() { setFixtures('
'); teamsTab = new TeamsTabFactory({ topics: {results: []}, - topics_url: '', - teams_url: '', + topicsUrl: '', + teamsUrl: '', maxTeamSize: 9999, - course_id: 'edX/DemoX/Demo_Course' + courseID: 'edX/DemoX/Demo_Course' }); }); diff --git a/lms/djangoapps/teams/static/teams/js/spec/teams_tab_spec.js b/lms/djangoapps/teams/static/teams/js/spec/teams_tab_spec.js deleted file mode 100644 index e2e182b777..0000000000 --- a/lms/djangoapps/teams/static/teams/js/spec/teams_tab_spec.js +++ /dev/null @@ -1,80 +0,0 @@ -define([ - 'jquery', - 'backbone', - 'common/js/spec_helpers/ajax_helpers', - 'teams/js/views/teams_tab' -], function ($, Backbone, AjaxHelpers, TeamsTabView) { - 'use strict'; - - describe('TeamsTab', function () { - var teamsTabView, - expectContent = function (text) { - expect(teamsTabView.$('.page-content-main').text()).toContain(text); - }, - expectHeader = function (text) { - expect(teamsTabView.$('.teams-header').text()).toContain(text); - }, - expectError = function (text) { - expect(teamsTabView.$('.warning').text()).toContain(text); - }, - expectFocus = function (element) { - expect(element.focus).toHaveBeenCalled(); - }; - - beforeEach(function () { - setFixtures('
'); - teamsTabView = new TeamsTabView({ - el: $('.teams-content'), - topics: { - count: 1, - num_pages: 1, - current_page: 1, - start: 0, - results: [{ - description: 'test description', - name: 'test topic', - id: 'test_id', - team_count: 0 - }] - }, - topic_url: 'api/topics/topic_id,course_id', - topics_url: 'topics_url', - teams_url: 'teams_url', - course_id: 'test/course/id' - }).render(); - Backbone.history.start(); - spyOn($.fn, 'focus'); - }); - - afterEach(function () { - Backbone.history.stop(); - }); - - it('shows the teams tab initially', function () { - expectHeader('See all teams in your course, organized by topic'); - expectContent('This is the new Teams tab.'); - }); - - it('can switch tabs', function () { - teamsTabView.$('a.nav-item[data-url="browse"]').click(); - expectContent('test description'); - teamsTabView.$('a.nav-item[data-url="teams"]').click(); - expectContent('This is the new Teams tab.'); - }); - - it('displays and focuses an error message when trying to navigate to a nonexistent route', function () { - teamsTabView.router.navigate('test', {trigger: true}); - expectError('The page "test" could not be found.'); - expectFocus(teamsTabView.$('.warning')); - }); - - it('displays and focuses an error message when trying to navigate to a nonexistent topic', function () { - var requests = AjaxHelpers.requests(this); - teamsTabView.router.navigate('topics/test', {trigger: true}); - AjaxHelpers.expectRequest(requests, 'GET', 'api/topics/test,course_id', null); - AjaxHelpers.respondWithError(requests, 404); - expectError('The topic "test" could not be found.'); - expectFocus(teamsTabView.$('.warning')); - }); - }); -}); diff --git a/lms/djangoapps/teams/static/teams/js/spec/views/team_discussion_spec.js b/lms/djangoapps/teams/static/teams/js/spec/views/team_discussion_spec.js new file mode 100644 index 0000000000..42045cf705 --- /dev/null +++ b/lms/djangoapps/teams/static/teams/js/spec/views/team_discussion_spec.js @@ -0,0 +1,214 @@ +define([ + 'underscore', 'common/js/spec_helpers/ajax_helpers', 'teams/js/views/team_discussion', + 'teams/js/spec_helpers/team_discussion_helpers', + 'xmodule_js/common_static/coffee/spec/discussion/discussion_spec_helper' +], function (_, AjaxHelpers, TeamDiscussionView, TeamDiscussionSpecHelper, DiscussionSpecHelper) { + 'use strict'; + describe('TeamDiscussionView', function() { + var discussionView, createDiscussionView, createPost, expandReplies, postReply; + + beforeEach(function() { + setFixtures('
'); + $('.discussion-module').data('course-id', TeamDiscussionSpecHelper.testCourseID); + $('.discussion-module').data('discussion-id', TeamDiscussionSpecHelper.testTeamDiscussionID); + $('.discussion-module').data('user-create-comment', true); + $('.discussion-module').data('user-create-subcomment', true); + DiscussionSpecHelper.setUnderscoreFixtures(); + }); + + createDiscussionView = function(requests, threads) { + discussionView = new TeamDiscussionView({ + el: '.discussion-module' + }); + discussionView.render(); + AjaxHelpers.expectRequest( + requests, 'GET', + interpolate( + '/courses/%(courseID)s/discussion/forum/%(discussionID)s/inline?page=1&ajax=1', + { + courseID: TeamDiscussionSpecHelper.testCourseID, + discussionID: TeamDiscussionSpecHelper.testTeamDiscussionID + }, + true + + ) + ); + AjaxHelpers.respondWithJson(requests, TeamDiscussionSpecHelper.createMockDiscussionResponse(threads)); + return discussionView; + }; + + createPost = function(requests, view, title, body, threadID) { + title = title || "Test title"; + body = body || "Test body"; + threadID = threadID || "999"; + view.$('.new-post-button').click(); + view.$('.js-post-title').val(title); + view.$('.js-post-body textarea').val(body); + view.$('.submit').click(); + AjaxHelpers.expectRequest( + requests, 'POST', + interpolate( + '/courses/%(courseID)s/discussion/%(discussionID)s/threads/create?ajax=1', + { + courseID: TeamDiscussionSpecHelper.testCourseID, + discussionID: TeamDiscussionSpecHelper.testTeamDiscussionID + }, + true + ), + interpolate( + 'thread_type=discussion&title=%(title)s&body=%(body)s&anonymous=false&anonymous_to_peers=false&auto_subscribe=true', + { + title: title.replace(/ /g, '+'), + body: body.replace(/ /g, '+') + }, + true + ) + ); + AjaxHelpers.respondWithJson(requests, { + content: TeamDiscussionSpecHelper.createMockPostResponse({ + id: threadID, + title: title, + body: body + }), + annotated_content_info: TeamDiscussionSpecHelper.createAnnotatedContentInfo() + }); + }; + + expandReplies = function(requests, view, threadID) { + view.$('.forum-thread-expand').first().click(); + AjaxHelpers.expectRequest( + requests, 'GET', + interpolate( + '/courses/%(courseID)s/discussion/forum/%(discussionID)s/threads/%(threadID)s?ajax=1&resp_skip=0&resp_limit=25', + { + courseID: TeamDiscussionSpecHelper.testCourseID, + discussionID: TeamDiscussionSpecHelper.testTeamDiscussionID, + threadID: threadID || "999" + }, + true + ) + ); + AjaxHelpers.respondWithJson(requests, { + content: TeamDiscussionSpecHelper.createMockThreadResponse(), + annotated_content_info: TeamDiscussionSpecHelper.createAnnotatedContentInfo() + }); + }; + + postReply = function(requests, view, reply, threadID) { + var replyForm = view.$('.discussion-reply-new').first(); + replyForm.find('.reply-body textarea').val(reply); + replyForm.find('.discussion-submit-post').click(); + AjaxHelpers.expectRequest( + requests, 'POST', + interpolate( + '/courses/%(courseID)s/discussion/threads/%(threadID)s/reply?ajax=1', + { + courseID: TeamDiscussionSpecHelper.testCourseID, + threadID: threadID || "999" + }, + true + ), + 'body=' + reply.replace(/ /g, '+') + ); + AjaxHelpers.respondWithJson(requests, { + content: TeamDiscussionSpecHelper.createMockThreadResponse({ + body: reply, + comments_count: 1 + }), + "annotated_content_info": TeamDiscussionSpecHelper.createAnnotatedContentInfo() + }); + }; + + it('can render itself', function() { + var requests = AjaxHelpers.requests(this), + view = createDiscussionView(requests); + expect(view.$('.discussion-thread').length).toEqual(3); + }); + + it('can create a new post', function() { + var requests = AjaxHelpers.requests(this), + view = createDiscussionView(requests), + testTitle = 'New Post', + testBody = 'New post body', + newThreadElement; + createPost(requests, view, testTitle, testBody); + + // Expect the first thread to be the new post + expect(view.$('.discussion-thread').length).toEqual(4); + newThreadElement = view.$('.discussion-thread').first(); + expect(newThreadElement.find('.post-header-content h1').text().trim()).toEqual(testTitle); + expect(newThreadElement.find('.post-body').text().trim()).toEqual(testBody); + }); + + it('can post a reply', function() { + var requests = AjaxHelpers.requests(this), + view = createDiscussionView(requests), + testReply = "Test reply", + testThreadID = "1"; + expandReplies(requests, view, testThreadID); + postReply(requests, view, testReply, testThreadID); + expect(view.$('.discussion-response .response-body').text().trim()).toBe(testReply); + }); + + it('can post a reply to a new post', function() { + var requests = AjaxHelpers.requests(this), + view = createDiscussionView(requests, []), + testReply = "Test reply"; + createPost(requests, view); + expandReplies(requests, view); + postReply(requests, view, testReply); + expect(view.$('.discussion-response .response-body').text().trim()).toBe(testReply); + }); + + it('cannot move an existing thread to a different topic', function() { + var requests = AjaxHelpers.requests(this), + view = createDiscussionView(requests), + postTopicButton, updatedThreadElement, + updatedTitle = 'Updated title', + updatedBody = 'Updated body', + testThreadID = "1"; + expandReplies(requests, view, testThreadID); + view.$('.action-more .icon').first().click(); + view.$('.action-edit').first().click(); + postTopicButton = view.$('.post-topic'); + expect(postTopicButton.length).toBe(0); + view.$('.js-post-post-title').val(updatedTitle); + view.$('.js-post-body textarea').val(updatedBody); + view.$('.submit').click(); + AjaxHelpers.expectRequest( + requests, 'POST', + interpolate( + '/courses/%(courseID)s/discussion/%(discussionID)s/threads/create?ajax=1', + { + courseID: TeamDiscussionSpecHelper.testCourseID, + discussionID: TeamDiscussionSpecHelper.testTeamDiscussionID + }, + true + ), + 'thread_type=discussion&title=&body=Updated+body&anonymous=false&anonymous_to_peers=false&auto_subscribe=true' + ); + AjaxHelpers.respondWithJson(requests, { + content: TeamDiscussionSpecHelper.createMockPostResponse({ + id: "999", title: updatedTitle, body: updatedBody + }), + annotated_content_info: TeamDiscussionSpecHelper.createAnnotatedContentInfo() + }); + + // Expect the thread to have been updated + updatedThreadElement = view.$('.discussion-thread').first(); + expect(updatedThreadElement.find('.post-header-content h1').text().trim()).toEqual(updatedTitle); + expect(updatedThreadElement.find('.post-body').text().trim()).toEqual(updatedBody); + }); + + it('cannot move a new thread to a different topic', function() { + var requests = AjaxHelpers.requests(this), + view = createDiscussionView(requests), + postTopicButton; + createPost(requests, view); + expandReplies(requests, view); + view.$('.action-more .icon').first().click(); + view.$('.action-edit').first().click(); + expect(view.$('.post-topic').length).toBe(0); + }); + }); +}); 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 new file mode 100644 index 0000000000..c61bc981d9 --- /dev/null +++ b/lms/djangoapps/teams/static/teams/js/spec/views/team_profile_spec.js @@ -0,0 +1,50 @@ +define([ + 'underscore', 'common/js/spec_helpers/ajax_helpers', 'teams/js/models/team', + 'teams/js/views/team_profile', 'teams/js/spec_helpers/team_discussion_helpers', + 'xmodule_js/common_static/coffee/spec/discussion/discussion_spec_helper' +], function (_, AjaxHelpers, TeamModel, TeamProfileView, TeamDiscussionSpecHelper, DiscussionSpecHelper) { + 'use strict'; + describe('TeamProfileView', function () { + var discussionView, createTeamProfileView; + + beforeEach(function () { + DiscussionSpecHelper.setUnderscoreFixtures(); + }); + + createTeamProfileView = function(requests) { + var model = new TeamModel( + { + id: "test-team", + name: "Test Team", + discussion_topic_id: TeamDiscussionSpecHelper.testTeamDiscussionID + }, + { parse: true } + ); + discussionView = new TeamProfileView({ + courseID: TeamDiscussionSpecHelper.testCourseID, + model: model + }); + discussionView.render(); + AjaxHelpers.expectRequest( + requests, + 'GET', + interpolate( + '/courses/%(courseID)s/discussion/forum/%(topicID)s/inline?page=1&ajax=1', + { + courseID: TeamDiscussionSpecHelper.testCourseID, + topicID: TeamDiscussionSpecHelper.testTeamDiscussionID + }, + true + ) + ); + AjaxHelpers.respondWithJson(requests, TeamDiscussionSpecHelper.createMockDiscussionResponse()); + return discussionView; + }; + + it('can render itself', function () { + var requests = AjaxHelpers.requests(this), + view = createTeamProfileView(requests); + expect(view.$('.discussion-thread').length).toEqual(3); + }); + }); +}); diff --git a/lms/djangoapps/teams/static/teams/js/spec/teams_spec.js b/lms/djangoapps/teams/static/teams/js/spec/views/teams_spec.js similarity index 98% rename from lms/djangoapps/teams/static/teams/js/spec/teams_spec.js rename to lms/djangoapps/teams/static/teams/js/spec/views/teams_spec.js index 8bea272a86..156bc87db2 100644 --- a/lms/djangoapps/teams/static/teams/js/spec/teams_spec.js +++ b/lms/djangoapps/teams/static/teams/js/spec/views/teams_spec.js @@ -2,7 +2,7 @@ define([ 'backbone', 'teams/js/collections/team', 'teams/js/views/teams' ], function (Backbone, TeamCollection, TeamsView) { 'use strict'; - describe('TeamsView', function () { + describe('Teams View', function () { var teamsView, teamCollection, initialTeams, createTeams = function (startIndex, stopIndex) { return _.map(_.range(startIndex, stopIndex + 1), function (i) { 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 new file mode 100644 index 0000000000..7b0169f706 --- /dev/null +++ b/lms/djangoapps/teams/static/teams/js/spec/views/teams_tab_spec.js @@ -0,0 +1,92 @@ +define([ + 'jquery', + 'backbone', + 'common/js/spec_helpers/ajax_helpers', + 'teams/js/views/teams_tab', + 'URI' +], function ($, Backbone, AjaxHelpers, TeamsTabView, URI) { + 'use strict'; + + describe('TeamsTab', function () { + var teamsTabView, + expectContent = function (text) { + expect(teamsTabView.$('.page-content-main').text()).toContain(text); + }, + expectHeader = function (text) { + expect(teamsTabView.$('.teams-header').text()).toContain(text); + }, + expectError = function (text) { + expect(teamsTabView.$('.warning').text()).toContain(text); + }, + expectFocus = function (element) { + expect(element.focus).toHaveBeenCalled(); + }; + + beforeEach(function () { + setFixtures('
'); + teamsTabView = new TeamsTabView({ + el: $('.teams-content'), + topics: { + count: 1, + num_pages: 1, + current_page: 1, + start: 0, + results: [{ + description: 'test description', + name: 'test topic', + id: 'test_topic', + team_count: 0 + }] + }, + topicsUrl: 'api/topics/', + topicUrl: 'api/topics/topic_id,test/course/id', + teamsUrl: 'api/teams/', + courseID: 'test/course/id' + }).render(); + Backbone.history.start(); + spyOn($.fn, 'focus'); + }); + + afterEach(function () { + Backbone.history.stop(); + }); + + it('shows the teams tab initially', function () { + expectHeader('See all teams in your course, organized by topic'); + expectContent('This is the new Teams tab.'); + }); + + describe('Navigation', function () { + it('can switch tabs', function () { + teamsTabView.$('a.nav-item[data-url="browse"]').click(); + expectContent('test description'); + teamsTabView.$('a.nav-item[data-url="teams"]').click(); + expectContent('This is the new Teams tab.'); + }); + + it('displays and focuses an error message when trying to navigate to a nonexistent page', function () { + teamsTabView.router.navigate('no_such_page', {trigger: true}); + expectError('The page "no_such_page" could not be found.'); + expectFocus(teamsTabView.$('.warning')); + }); + + it('displays and focuses an error message when trying to navigate to a nonexistent topic', function () { + var requests = AjaxHelpers.requests(this); + teamsTabView.router.navigate('topics/no_such_topic', {trigger: true}); + AjaxHelpers.expectRequest(requests, 'GET', 'api/topics/no_such_topic,test/course/id', null); + AjaxHelpers.respondWithError(requests, 404); + expectError('The topic "no_such_topic" could not be found.'); + expectFocus(teamsTabView.$('.warning')); + }); + + it('displays and focuses an error message when trying to navigate to a nonexistent team', function () { + var requests = AjaxHelpers.requests(this); + teamsTabView.router.navigate('teams/test_topic/no_such_team', {trigger: true}); + AjaxHelpers.expectRequest(requests, 'GET', 'api/teams/no_such_team', null); + AjaxHelpers.respondWithError(requests, 404); + expectError('The team "no_such_team" could not be found.'); + expectFocus(teamsTabView.$('.warning')); + }); + }); + }); +}); diff --git a/lms/djangoapps/teams/static/teams/js/spec/topic_card_spec.js b/lms/djangoapps/teams/static/teams/js/spec/views/topic_card_spec.js similarity index 100% rename from lms/djangoapps/teams/static/teams/js/spec/topic_card_spec.js rename to lms/djangoapps/teams/static/teams/js/spec/views/topic_card_spec.js diff --git a/lms/djangoapps/teams/static/teams/js/spec/topics_spec.js b/lms/djangoapps/teams/static/teams/js/spec/views/topics_spec.js similarity index 100% rename from lms/djangoapps/teams/static/teams/js/spec/topics_spec.js rename to lms/djangoapps/teams/static/teams/js/spec/views/topics_spec.js diff --git a/lms/djangoapps/teams/static/teams/js/spec_helpers/team_discussion_helpers.js b/lms/djangoapps/teams/static/teams/js/spec_helpers/team_discussion_helpers.js new file mode 100644 index 0000000000..665b48cbc7 --- /dev/null +++ b/lms/djangoapps/teams/static/teams/js/spec_helpers/team_discussion_helpers.js @@ -0,0 +1,133 @@ +define([ + 'underscore', 'common/js/spec_helpers/ajax_helpers' +], function (_, AjaxHelpers) { + 'use strict'; + var createMockPostResponse, createMockDiscussionResponse, createAnnotatedContentInfo, createMockThreadResponse, + testCourseID = 'course/1', + testUser = 'testUser', + testTeamDiscussionID = "12345"; + + createMockPostResponse = function(options) { + return _.extend( + { + username: testUser, + course_id: testCourseID, + commentable_id: testTeamDiscussionID, + type: 'thread', + body: "", + anonymous_to_peers: false, + unread_comments_count: 0, + updated_at: '2015-07-29T18:44:56Z', + group_name: 'Default Group', + pinned: false, + votes: {count: 0, down_count: 0, point: 0, up_count: 0}, + user_id: "9", + abuse_flaggers: [], + closed: false, + at_position_list: [], + read: false, + anonymous: false, + created_at: "2015-07-29T18:44:56Z", + thread_type: 'discussion', + comments_count: 0, + group_id: 1, + endorsed: false + }, + options || {} + ); + }; + + createMockDiscussionResponse = function(threads) { + if (_.isUndefined(threads)) { + threads = [ + createMockPostResponse({ id: "1", title: "First Post"}), + createMockPostResponse({ id: "2", title: "Second Post"}), + createMockPostResponse({ id: "3", title: "Third Post"}) + ]; + } + return { + "num_pages": 1, + "page": 1, + "discussion_data": threads, + "user_info": { + "username": testUser, + "follower_ids": [], + "default_sort_key": "date", + "downvoted_ids": [], + "subscribed_thread_ids": [], + "upvoted_ids": [], + "external_id": "9", + "id": "9", + "subscribed_user_ids": [], + "subscribed_commentable_ids": [] + }, + "annotated_content_info": { + }, + "roles": {"Moderator": [], "Administrator": [], "Community TA": []}, + "course_settings": { + "is_cohorted": false, + "allow_anonymous_to_peers": false, + "allow_anonymous": true, + "category_map": {"subcategories": {}, "children": [], "entries": {}}, + "cohorts": [] + } + }; + }; + + createAnnotatedContentInfo = function() { + return { + voted: '', + subscribed: true, + ability: { + can_reply: true, + editable: true, + can_openclose: true, + can_delete: true, + can_vote: true + } + }; + }; + + createMockThreadResponse = function(options) { + return _.extend( + { + username: testUser, + course_id: testCourseID, + commentable_id: testTeamDiscussionID, + children: [], + comments_count: 0, + anonymous_to_peers: false, + unread_comments_count: 0, + updated_at: "2015-08-04T21:44:28Z", + resp_skip: 0, + id: "55c1323c56c02ce921000001", + pinned: false, + votes: {"count": 0, "down_count": 0, "point": 0, "up_count": 0}, + resp_limit: 25, + abuse_flaggers: [], + closed: false, + resp_total: 1, + at_position_list: [], + type: "thread", + read: true, + anonymous: false, + user_id: "5", + created_at: "2015-08-04T21:44:28Z", + thread_type: "discussion", + context: "standalone", + endorsed: false + }, + options || {} + ); + }; + + return { + testCourseID: testCourseID, + testUser: testUser, + testTeamDiscussionID: testTeamDiscussionID, + createMockPostResponse: createMockPostResponse, + createMockDiscussionResponse: createMockDiscussionResponse, + createAnnotatedContentInfo: createAnnotatedContentInfo, + createMockThreadResponse: createMockThreadResponse + }; +}); 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 24019390fe..0776f7c90a 100644 --- a/lms/djangoapps/teams/static/teams/js/views/edit_team.js +++ b/lms/djangoapps/teams/static/teams/js/views/edit_team.js @@ -1,192 +1,191 @@ ;(function (define) { -'use strict'; + 'use strict'; -define(['backbone', - 'underscore', - 'gettext', - 'js/views/fields', - 'teams/js/models/team', - 'text!teams/templates/edit-team.underscore'], - function (Backbone, _, gettext, FieldViews, TeamModel, edit_team_template) { - return Backbone.View.extend({ + define(['backbone', + 'underscore', + 'gettext', + 'js/views/fields', + 'teams/js/models/team', + 'text!teams/templates/edit-team.underscore'], + function (Backbone, _, gettext, FieldViews, TeamModel, edit_team_template) { + return Backbone.View.extend({ - maxTeamNameLength: 255, - maxTeamDescriptionLength: 300, + maxTeamNameLength: 255, + maxTeamDescriptionLength: 300, - events: { - "click .action-primary": "createTeam", - "click .action-cancel": "goBackToTopic" - }, + events: { + 'click .action-primary': 'createTeam', + 'click .action-cancel': 'goBackToTopic' + }, - initialize: function(options) { - this.courseId = options.teamParams.courseId; - this.teamsUrl = options.teamParams.teamsUrl; - this.topicId = options.teamParams.topicId; - this.languages = options.teamParams.languages; - this.countries = options.teamParams.countries; - this.primaryButtonTitle = options.primaryButtonTitle || 'Submit'; + initialize: function(options) { + this.courseId = options.teamParams.courseId; + this.collection = options.collection; + this.teamsUrl = options.teamParams.teamsUrl; + this.topicId = options.teamParams.topicId; + this.languages = options.teamParams.languages; + this.countries = options.teamParams.countries; + this.primaryButtonTitle = options.primaryButtonTitle || 'Submit'; - _.bindAll(this, "goBackToTopic", "createTeam"); + _.bindAll(this, 'goBackToTopic', 'createTeam'); - this.teamModel = new TeamModel({}); - this.teamModel.url = this.teamsUrl; + this.teamModel = new TeamModel({}); + this.teamModel.url = this.teamsUrl; - this.teamNameField = new FieldViews.TextFieldView({ - model: this.teamModel, - title: gettext("Team Name (Required) *"), - valueAttribute: 'name', - helpMessage: gettext("A name that identifies your team (maximum 255 characters).") - }); + this.teamNameField = new FieldViews.TextFieldView({ + model: this.teamModel, + title: gettext('Team Name (Required) *'), + valueAttribute: 'name', + helpMessage: gettext('A name that identifies your team (maximum 255 characters).') + }); - this.teamDescriptionField = new FieldViews.TextareaFieldView({ - model: this.teamModel, - title: gettext("Team Description (Required) *"), - 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).") - }); + this.teamDescriptionField = new FieldViews.TextareaFieldView({ + model: this.teamModel, + title: gettext('Team Description (Required) *'), + 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).') + }); - this.optionalDescriptionField = new FieldViews.ReadonlyFieldView({ - model: this.teamModel, - title: gettext("Optional Characteristics"), - valueAttribute: 'optional_description', - helpMessage: gettext("Help other learners decide whether to join your team by specifying some characteristics for your team. Choose carefully, because fewer people might be interested in joining your team if it seems too restrictive.") - }); + this.optionalDescriptionField = new FieldViews.ReadonlyFieldView({ + model: this.teamModel, + title: gettext('Optional Characteristics'), + valueAttribute: 'optional_description', + helpMessage: gettext('Help other learners decide whether to join your team by specifying some characteristics for your team. Choose carefully, because fewer people might be interested in joining your team if it seems too restrictive.') + }); - this.teamLanguageField = new FieldViews.DropdownFieldView({ - model: this.teamModel, - title: gettext("Language"), - valueAttribute: 'language', - required: false, - showMessages: false, - titleIconName: 'fa-comment-o', - options: this.languages, - helpMessage: gettext("The language that team members primarily use to communicate with each other.") - }); + this.teamLanguageField = new FieldViews.DropdownFieldView({ + model: this.teamModel, + title: gettext('Language'), + valueAttribute: 'language', + required: false, + showMessages: false, + titleIconName: 'fa-comment-o', + options: this.languages, + helpMessage: gettext('The language that team members primarily use to communicate with each other.') + }); - this.teamCountryField = new FieldViews.DropdownFieldView({ - model: this.teamModel, - title: gettext('Country'), - valueAttribute: 'country', - required: false, - showMessages: false, - titleIconName: 'fa-globe', - options: this.countries, - helpMessage: gettext("The country that team members primarily identify with.") - }); - }, + this.teamCountryField = new FieldViews.DropdownFieldView({ + model: this.teamModel, + title: gettext('Country'), + valueAttribute: 'country', + required: false, + showMessages: false, + titleIconName: 'fa-globe', + options: this.countries, + helpMessage: gettext('The country that team members primarily identify with.') + }); + }, - render: function() { - this.$el.html(_.template(edit_team_template)({primaryButtonTitle: this.primaryButtonTitle})); - this.set(this.teamNameField, '.team-required-fields'); - this.set(this.teamDescriptionField, '.team-required-fields'); - this.set(this.optionalDescriptionField, '.team-optional-fields'); - this.set(this.teamLanguageField, '.team-optional-fields'); - this.set(this.teamCountryField, '.team-optional-fields'); - return this; - }, + render: function() { + this.$el.html(_.template(edit_team_template)({primaryButtonTitle: this.primaryButtonTitle})); + this.set(this.teamNameField, '.team-required-fields'); + this.set(this.teamDescriptionField, '.team-required-fields'); + this.set(this.optionalDescriptionField, '.team-optional-fields'); + this.set(this.teamLanguageField, '.team-optional-fields'); + this.set(this.teamCountryField, '.team-optional-fields'); + return this; + }, - set: function(view, selector) { - var viewEl = view.$el; - if (this.$(selector).has(viewEl).length) { - view.render().setElement(viewEl); - } else { - this.$(selector).append(view.render().$el); - } - }, + set: function(view, selector) { + var viewEl = view.$el; + if (this.$(selector).has(viewEl).length) { + view.render().setElement(viewEl); + } else { + this.$(selector).append(view.render().$el); + } + }, - createTeam: function () { - var teamName = this.teamNameField.fieldValue(); - var teamDescription = this.teamDescriptionField.fieldValue(); - var teamLanguage = this.teamLanguageField.fieldValue(); - var teamCountry = this.teamCountryField.fieldValue(); + createTeam: function () { + var view = this, + teamLanguage = this.teamLanguageField.fieldValue(), + teamCountry = this.teamCountryField.fieldValue(); - var data = { - course_id: this.courseId, - topic_id: this.topicId, - name: teamName, - description: teamDescription, - language: _.isNull(teamLanguage) ? '' : teamLanguage, - country: _.isNull(teamCountry) ? '' : teamCountry - }; + var data = { + course_id: this.courseId, + topic_id: this.topicId, + name: this.teamNameField.fieldValue(), + description: this.teamDescriptionField.fieldValue(), + language: _.isNull(teamLanguage) ? '' : teamLanguage, + country: _.isNull(teamCountry) ? '' : teamCountry + }; - var validationResult = this.validateTeamData(data); - if (validationResult.status === false) { - this.showMessage(validationResult.message, validationResult.srMessage); - return; - } + var validationResult = this.validateTeamData(data); + if (validationResult.status === false) { + this.showMessage(validationResult.message, validationResult.srMessage); + return; + } - var view = this; - var options = { - wait: true, - success: function () { - view.goBackToTopic(); - }, - error: function () { - var message = gettext('An error occurred. Please try again.'); - view.showMessage(message, message); - } - }; - this.teamModel.save(data, options); - }, + this.teamModel.save(data, { wait: true }) + .done(function(result) { + Backbone.history.navigate( + 'teams/' + view.topicId + '/' + view.teamModel.id, + {trigger: true} + ); + }) + .fail(function() { + var message = gettext('An error occurred. Please try again.'); + view.showMessage(message, message); + }); + }, - validateTeamData: function (data) { - var status = true, - message = gettext("Check the highlighted fields below and try again."); - var srMessages = []; + validateTeamData: function (data) { + var status = true, + message = gettext('Check the highlighted fields below and try again.'); + var srMessages = []; - this.teamNameField.unhighlightField(); - this.teamDescriptionField.unhighlightField(); + this.teamNameField.unhighlightField(); + this.teamDescriptionField.unhighlightField(); - if (_.isEmpty(data.name.trim()) ) { - status = false; - this.teamNameField.highlightFieldOnError(); - srMessages.push( - gettext("Enter team name.") - ); - } else if (data.name.length > this.maxTeamNameLength) { - status = false; - this.teamNameField.highlightFieldOnError(); - srMessages.push( - gettext("Team name cannot have more than 255 characters.") - ); - } + if (_.isEmpty(data.name.trim()) ) { + status = false; + this.teamNameField.highlightFieldOnError(); + srMessages.push( + gettext('Enter team name.') + ); + } else if (data.name.length > this.maxTeamNameLength) { + status = false; + this.teamNameField.highlightFieldOnError(); + srMessages.push( + gettext('Team name cannot have more than 255 characters.') + ); + } - if (_.isEmpty(data.description.trim()) ) { - status = false; - this.teamDescriptionField.highlightFieldOnError(); - srMessages.push( - gettext("Enter team description.") - ); - } else if (data.description.length > this.maxTeamDescriptionLength) { - status = false; - this.teamDescriptionField.highlightFieldOnError(); - srMessages.push( - gettext("Team description cannot have more than 300 characters.") - ); - } + if (_.isEmpty(data.description.trim()) ) { + status = false; + this.teamDescriptionField.highlightFieldOnError(); + srMessages.push( + gettext('Enter team description.') + ); + } else if (data.description.length > this.maxTeamDescriptionLength) { + status = false; + this.teamDescriptionField.highlightFieldOnError(); + srMessages.push( + gettext('Team description cannot have more than 300 characters.') + ); + } - return { - status: status, - message: message, - srMessage: srMessages.join(" ") - }; - }, + return { + status: status, + message: message, + srMessage: srMessages.join(' ') + }; + }, - showMessage: function (message, screenReaderMessage) { - this.$('.wrapper-msg').removeClass('is-hidden'); - this.$('.msg-content .copy p').text(message); - this.$('.wrapper-msg').focus(); + showMessage: function (message, screenReaderMessage) { + this.$('.wrapper-msg').removeClass('is-hidden'); + this.$('.msg-content .copy p').text(message); + this.$('.wrapper-msg').focus(); - if (screenReaderMessage) { - this.$('.screen-reader-message').text(screenReaderMessage); - } + if (screenReaderMessage) { + this.$('.screen-reader-message').text(screenReaderMessage); + } }, goBackToTopic: function () { - Backbone.history.navigate("topics/" + this.topicId, {trigger: true}); - } - }); - }); + Backbone.history.navigate('topics/' + this.topicId, {trigger: true}); + } + }); + }); }).call(this, define || RequireJS.define); 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 cdfbeda5cf..c13fa5cb96 100644 --- a/lms/djangoapps/teams/static/teams/js/views/team_card.js +++ b/lms/djangoapps/teams/static/teams/js/views/team_card.js @@ -78,6 +78,11 @@ {span_start: '', team_name: this.model.get('name'), span_end: ''}, true ); + }, + action: function (event) { + var url = 'teams/' + this.topic.get('id') + '/' + this.model.get('id'); + event.preventDefault(); + this.router.navigate(url, {trigger: true}); } }); return TeamCardView; diff --git a/lms/djangoapps/teams/static/teams/js/views/team_discussion.js b/lms/djangoapps/teams/static/teams/js/views/team_discussion.js new file mode 100644 index 0000000000..2549f17124 --- /dev/null +++ b/lms/djangoapps/teams/static/teams/js/views/team_discussion.js @@ -0,0 +1,27 @@ +/** + * View that shows the discussion for a team. + */ +;(function (define) { + 'use strict'; + define(['backbone', 'underscore', 'gettext', 'DiscussionModuleView'], + function (Backbone, _, gettext, DiscussionModuleView) { + var TeamDiscussionView = Backbone.View.extend({ + initialize: function () { + window.$$course_id = this.$el.data("course-id"); + this.render(); + }, + + render: function () { + var discussionModuleView = new DiscussionModuleView({ + el: this.$el, + context: 'standalone' + }); + discussionModuleView.render(); + discussionModuleView.loadPage(this.$el); + return this; + } + }); + + return TeamDiscussionView; + }); +}).call(this, define || RequireJS.define); diff --git a/lms/djangoapps/teams/static/teams/js/views/team_profile.js b/lms/djangoapps/teams/static/teams/js/views/team_profile.js new file mode 100644 index 0000000000..a0a0d7794a --- /dev/null +++ b/lms/djangoapps/teams/static/teams/js/views/team_profile.js @@ -0,0 +1,33 @@ +/** + * View for an individual team. + */ +;(function (define) { + 'use strict'; + define(['backbone', 'underscore', 'gettext', 'teams/js/views/team_discussion', + 'text!teams/templates/team-profile.underscore'], + function (Backbone, _, gettext, TeamDiscussionView, teamTemplate) { + var TeamProfileView = Backbone.View.extend({ + initialize: function (options) { + this.courseID = options.courseID; + this.discussionTopicID = this.model.get('discussion_topic_id'); + }, + + render: function () { + var canPostToTeam = true; // TODO: determine this permission correctly! + this.$el.html(_.template(teamTemplate, { + courseID: this.courseID, + discussionTopicID: this.discussionTopicID, + canCreateComment: canPostToTeam, + canCreateSubComment: canPostToTeam + })); + this.discussionView = new TeamDiscussionView({ + el: this.$('.discussion-module') + }); + this.discussionView.render(); + return this; + } + }); + + return TeamProfileView; + }); +}).call(this, define || RequireJS.define); diff --git a/lms/djangoapps/teams/static/teams/js/views/teams.js b/lms/djangoapps/teams/static/teams/js/views/teams.js index c509da128a..741ac255dd 100644 --- a/lms/djangoapps/teams/static/teams/js/views/teams.js +++ b/lms/djangoapps/teams/static/teams/js/views/teams.js @@ -10,8 +10,10 @@ type: 'teams', initialize: function (options) { + this.topic = options.topic; this.itemViewClass = TeamCardView.extend({ router: options.router, + topic: options.topic, maxTeamSize: options.maxTeamSize }); PaginatedView.prototype.initialize.call(this); 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 fd5bbe9e66..89de3e7c28 100644 --- a/lms/djangoapps/teams/static/teams/js/views/teams_tab.js +++ b/lms/djangoapps/teams/static/teams/js/views/teams_tab.js @@ -7,304 +7,379 @@ 'js/components/header/views/header', 'js/components/header/models/header', 'js/components/tabbed/views/tabbed_view', - 'teams/js/views/topics', 'teams/js/models/topic', 'teams/js/collections/topic', - 'teams/js/views/teams', + 'teams/js/models/team', 'teams/js/collections/team', + 'teams/js/views/topics', + 'teams/js/views/team_profile', + 'teams/js/views/teams', 'teams/js/views/edit_team', 'text!teams/templates/teams_tab.underscore'], - function (Backbone, _, gettext, HeaderView, HeaderModel, TabbedView, - TopicsView, TopicModel, TopicCollection, TeamsView, TeamCollection, - TeamEditView, teamsTemplate) { - var ViewWithHeader = Backbone.View.extend({ - initialize: function (options) { - this.header = options.header; - this.main = options.main; - }, + function (Backbone, _, gettext, HeaderView, HeaderModel, TabbedView, + TopicModel, TopicCollection, TeamModel, TeamCollection, + TopicsView, TeamProfileView, TeamsView, TeamEditView, + teamsTemplate) { + var ViewWithHeader = Backbone.View.extend({ + initialize: function (options) { + this.header = options.header; + this.main = options.main; + }, - render: function () { - this.$el.html(_.template(teamsTemplate)); - this.$('p.error').hide(); - this.header.setElement(this.$('.teams-header')).render(); - this.main.setElement(this.$('.page-content')).render(); - return this; - } - }); + render: function () { + this.$el.html(_.template(teamsTemplate)); + this.$('p.error').hide(); + this.header.setElement(this.$('.teams-header')).render(); + this.main.setElement(this.$('.page-content')).render(); + return this; + } + }); - var TeamTabView = Backbone.View.extend({ - initialize: function(options) { - var TempTabView, router; - this.course_id = options.course_id; - this.topics = options.topics; - this.topic_url = options.topic_url; - this.teams_url = options.teams_url; - this.maxTeamSize = options.maxTeamSize; - this.languages = options.languages; - this.countries = options.countries; - // This slightly tedious approach is necessary - // to use regular expressions within Backbone - // routes, allowing us to capture which tab - // name is being routed to - router = this.router = new Backbone.Router(); - _.each([ - [':default', _.bind(this.routeNotFound, this)], - ['topics/:topic_id(/)', _.bind(this.browseTopic, this)], - ['topics/:topic_id/create-team(/)', _.bind(this.newTeam, this)], - [new RegExp('^(browse)\/?$'), _.bind(this.goToTab, this)], - [new RegExp('^(teams)\/?$'), _.bind(this.goToTab, this)] - ], function (route) { - router.route.apply(router, route); - }); - // TODO replace this with actual views! - TempTabView = Backbone.View.extend({ - initialize: function (options) { - this.text = options.text; - }, + var TeamTabView = Backbone.View.extend({ + initialize: function(options) { + var TempTabView, router; + this.courseID = options.courseID; + this.topics = options.topics; + this.topicUrl = options.topicUrl; + this.teamsUrl = options.teamsUrl; + this.maxTeamSize = options.maxTeamSize; + this.languages = options.languages; + this.countries = options.countries; + // This slightly tedious approach is necessary + // to use regular expressions within Backbone + // routes, allowing us to capture which tab + // name is being routed to + router = this.router = new Backbone.Router(); + _.each([ + [':default', _.bind(this.routeNotFound, this)], + ['topics/:topic_id(/)', _.bind(this.browseTopic, this)], + ['topics/:topic_id/create-team(/)', _.bind(this.newTeam, this)], + ['teams/:topic_id/:team_id(/)', _.bind(this.browseTeam, this)], + [new RegExp('^(browse)\/?$'), _.bind(this.goToTab, this)], + [new RegExp('^(teams)\/?$'), _.bind(this.goToTab, this)] + ], function (route) { + router.route.apply(router, route); + }); + // TODO replace this with actual views! + TempTabView = Backbone.View.extend({ + initialize: function (options) { + this.text = options.text; + }, + render: function () { + this.$el.text(this.text); + } + }); + this.topicsCollection = new TopicCollection( + this.topics, + {url: options.topicsUrl, course_id: this.courseID, parse: true} + ).bootstrap(); + this.topicsView = new TopicsView({ + collection: this.topicsCollection, + router: this.router + }); + this.mainView = this.tabbedView = new ViewWithHeader({ + header: new HeaderView({ + model: new HeaderModel({ + 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."), + title: gettext("Teams") + }) + }), + main: new TabbedView({ + tabs: [{ + title: gettext('My Teams'), + url: 'teams', + view: new TempTabView({text: 'This is the new Teams tab.'}) + }, { + title: gettext('Browse'), + url: 'browse', + view: this.topicsView + }], + router: this.router + }) + }); + }, - render: function () { - this.$el.text(this.text); - } - }); - this.topicsCollection = new TopicCollection( - this.topics, - {url: options.topics_url, course_id: this.course_id, parse: true} - ).bootstrap(); - this.mainView = this.tabbedView = new ViewWithHeader({ - header: new HeaderView({ - model: new HeaderModel({ - 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."), - title: gettext("Teams") - }) - }), - main: new TabbedView({ - tabs: [{ - title: gettext('My Teams'), - url: 'teams', - view: new TempTabView({text: 'This is the new Teams tab.'}) - }, { - title: gettext('Browse'), - url: 'browse', - view: new TopicsView({ - collection: this.topicsCollection, - router: this.router - }) - }], - router: this.router - }) - }); - }, + render: function() { + this.mainView.setElement(this.$el).render(); + this.hideWarning(); + return this; + }, - render: function() { - this.mainView.setElement(this.$el).render(); - this.hideWarning(); - return this; - }, + /** + * Render the list of teams for the given topic ID. + */ + browseTopic: function (topicID) { + var self = this; + this.getTeamsView(topicID).done(function (teamsView) { + self.teamsView = self.mainView = teamsView; + self.render(); + }); + }, - /** - * Render the create new team form. - */ - newTeam: function (topicId) { - var self = this; - this.getTeamsView(topicId).done(function (teamsView) { - self.mainView = new ViewWithHeader({ - header: new HeaderView({ - model: new HeaderModel({ - description: gettext("Create a new team if you can't find existing teams to join, or if you would like to learn with friends you know."), - title: gettext("Create a New Team"), - breadcrumbs: [ - { - title: teamsView.main.teamParams.topicName, - url: '#topics/' + teamsView.main.teamParams.topicId - } - ] - }) - }), - main: new TeamEditView({ - tagName: 'create-new-team', - teamParams: teamsView.main.teamParams, - primaryButtonTitle: 'Create' - }) - }); - self.render(); - }); - }, + /** + * Render the create new team form. + */ + newTeam: function (topicId) { + var self = this; + this.getTeamsView(topicId).done(function (teamsView) { + self.mainView = new ViewWithHeader({ + header: new HeaderView({ + model: new HeaderModel({ + description: gettext("Create a new team if you can't find existing teams to join, or if you would like to learn with friends you know."), + title: gettext("Create a New Team"), + breadcrumbs: [ + { + title: teamsView.main.teamParams.topicName, + url: '#topics/' + teamsView.main.teamParams.topicId + } + ] + }) + }), + main: new TeamEditView({ + tagName: 'create-new-team', + teamParams: teamsView.main.teamParams, + primaryButtonTitle: 'Create' + }) + }); + self.render(); + }); + }, - /** - * Render the list of teams for the given topic ID. - */ - browseTopic: function (topicID) { - var self = this; - this.getTeamsView(topicID).done(function (teamsView) { - self.mainView = teamsView; - self.render(); - }); - }, + /** + * Return a promise for the TeamsView for the given topic ID. + */ + getTeamsView: function (topicID) { + // Lazily load the teams-for-topic view in + // order to avoid making an extra AJAX call. + var self = this, + router = this.router, + deferred = $.Deferred(); + if (this.teamsCollection && this.teamsCollection.topic_id === topicID) { + deferred.resolve(this.teamsView); + } else { + this.getTopic(topicID) + .done(function(topic) { + var collection = new TeamCollection([], { + course_id: self.courseID, + topic_id: topicID, + url: self.teamsUrl, + per_page: 10 + }); + this.teamsCollection = collection; + collection.goTo(1) + .done(function() { + var teamsView = new TeamsView({ + router: router, + topic: topic, + collection: collection, + maxTeamSize: self.maxTeamSize, + teamParams: { + courseId: self.courseID, + teamsUrl: self.teamsUrl, + topicId: topic.get('id'), + topicName: topic.get('name'), + languages: self.languages, + countries: self.countries + } + }); + deferred.resolve(self.createViewWithHeader(teamsView, topic)); + }); + }); + } + return deferred.promise(); + }, - /** - * Return a promise for the TeamsView for the given - * topic ID. - */ - getTeamsView: function (topicID) { - // Lazily load the teams-for-topic view in - // order to avoid making an extra AJAX call. - if (!_.isUndefined(this.teamsView) - && this.teamsView.main.collection.topic_id === topicID) { - return this.identityPromise(this.teamsView); - } - var self = this, - teamCollection = new TeamCollection([], { - course_id: this.course_id, - url: this.teams_url, - topic_id: topicID, - per_page: 10 - }), - teamPromise = teamCollection.goTo(1).fail(function (xhr) { - if (xhr.status === 400) { - self.topicNotFound(topicID); - } - }), - topicPromise = this.getTopic(topicID).fail(function (xhr) { - if (xhr.status === 404) { - self.topicNotFound(topicID); - } - }); - return $.when(topicPromise, teamPromise).pipe( - _.bind(this.constructTeamView, this) - ); - }, + /** + * Browse to the team with the specified team ID belonging to the specified topic. + */ + browseTeam: function (topicID, teamID) { + var self = this; + this.getBrowseTeamView(topicID, teamID).done(function (browseTeamView) { + self.mainView = browseTeamView; + self.render(); + }); + }, - /** - * Given a topic and the results of the team - * collection's fetch(), return the team list view. - */ - constructTeamView: function (topic, collectionResults) { - var self = this, - headerView = new HeaderView({ - model: new HeaderModel({ - description: _.escape(topic.get('description')), - title: _.escape(topic.get('name')), - breadcrumbs: [{ - title: 'All topics', - url: '#' - }] - }), - events: { - 'click nav.breadcrumbs a.nav-item': function (event) { - event.preventDefault(); - self.router.navigate('browse', {trigger: true}); - } - } - }); - return new ViewWithHeader({ - header: headerView, - main: new TeamsView({ - collection: new TeamCollection(collectionResults[0], { - course_id: this.course_id, - url: this.teams_url, - topic_id: topic.get('id'), - per_page: 10, - parse: true - }), - teamParams: { - courseId: this.course_id, - teamsUrl: this.teams_url, - topicId: topic.get('id'), - topicName: topic.get('name'), - languages: self.languages, - countries: self.countries - }, - maxTeamSize: this.maxTeamSize - }) - }); - }, + /** + * Return a promise for the team view for the given team ID. + */ + getBrowseTeamView: function (topicID, teamID) { + var self = this, + deferred = $.Deferred(), + courseID = this.courseID; + self.getTopic(topicID).done(function(topic) { + self.getTeam(teamID).done(function(team) { + var view = new TeamProfileView({ + courseID: courseID, + model: team + }); + deferred.resolve(self.createViewWithHeader(view, team, topic)); + }); + }); + return deferred.promise(); + }, - /** - * Get a topic given a topic ID. Returns a jQuery deferred - * promise, since the topic may need to be fetched from the - * server. - * @param topicID the string identifier for the requested topic - * @returns a jQuery deferred promise for the topic. - */ - getTopic: function (topicID) { - // Try finding topic in the current page of the - // topicCollection. Otherwise call the topic endpoint. - var topic = this.topicsCollection.findWhere({'id': topicID}), - self = this; - if (topic) { - return this.identityPromise(topic); - } else { - var TopicModelWithUrl = TopicModel.extend({ - url: function () { return self.topic_url.replace('topic_id', this.id); } - }); - return (new TopicModelWithUrl({id: topicID })).fetch(); - } - }, + createViewWithHeader: function (mainView, subject, parentTopic) { + var router = this.router, + breadcrumbs, headerView; + breadcrumbs = [{ + title: gettext('All Topics'), + url: '#browse' + }]; + if (parentTopic) { + breadcrumbs.push({ + title: parentTopic.get('name'), + url: '#topics/' + parentTopic.id + }); + } + headerView = new HeaderView({ + model: new HeaderModel({ + description: subject.get('description'), + title: subject.get('name'), + breadcrumbs: breadcrumbs + }), + events: { + 'click nav.breadcrumbs a.nav-item': function (event) { + var url = $(event.currentTarget).attr('href'); + event.preventDefault(); + router.navigate(url, {trigger: true}); + } + } + }); + return new ViewWithHeader({ + header: headerView, + main: mainView + }); + }, - /** - * Immediately return a promise for the given - * object. - */ - identityPromise: function (obj) { - return new $.Deferred().resolve(obj).promise(); - }, + /** + * Get a topic given a topic ID. Returns a jQuery deferred + * promise, since the topic may need to be fetched from the + * server. + * @param topicID the string identifier for the requested topic + * @returns a jQuery deferred promise for the topic. + */ + getTopic: function (topicID) { + // Try finding topic in the current page of the + // topicCollection. Otherwise call the topic endpoint. + var topic = this.topicsCollection.findWhere({'id': topicID}), + self = this, + deferred = $.Deferred(); + if (topic) { + deferred.resolve(topic); + } else { + topic = new TopicModel({ + id: topicID, + url: self.topicUrl.replace('topic_id', topicID) + }); + topic.fetch() + .done(function() { + deferred.resolve(topic); + }) + .fail(function() { + self.topicNotFound(topicID); + deferred.reject(); + }); + } + return deferred.promise(); + }, - /** - * Set up the tabbed view and switch tabs. - */ - goToTab: function (tab) { - this.mainView = this.tabbedView; - // Note that `render` should be called first so - // that the tabbed view's element is set - // correctly. - this.render(); - this.tabbedView.main.setActiveTab(tab); - }, + /** + * Get a team given a team ID. Returns a jQuery deferred + * promise, since the team may need to be fetched from the + * server. + * @param teamID the string identifier for the requested team + * @returns {promise} a jQuery deferred promise for the team. + */ + getTeam: function (teamID) { + var team = this.teamsCollection ? this.teamsCollection.get(teamID) : null, + self = this, + deferred = $.Deferred(); + if (team) { + deferred.resolve(team); + } else { + team = new TeamModel({ + id: teamID, + url: this.teamsUrl + teamID + }); + team.fetch() + .done(function() { + deferred.resolve(team); + }) + .fail(function() { + self.teamNotFound(teamID); + deferred.reject(); + }); + } + return deferred.promise(); + }, - // Error handling + /** + * Set up the tabbed view and switch tabs. + */ + goToTab: function (tab) { + this.mainView = this.tabbedView; + // Note that `render` should be called first so + // that the tabbed view's element is set + // correctly. + this.render(); + this.tabbedView.main.setActiveTab(tab); + }, - routeNotFound: function (route) { - this.notFoundError( - interpolate( - gettext('The page "%(route)s" could not be found.'), - {route: route}, - true - ) - ); - }, + // Error handling - topicNotFound: function (topicID) { - this.notFoundError( - interpolate( - gettext('The topic "%(topic)s" could not be found.'), - {topic: topicID}, - true - ) - ); - }, + routeNotFound: function (route) { + this.notFoundError( + interpolate( + gettext('The page "%(route)s" could not be found.'), + {route: route}, + true + ) + ); + }, - /** - * Called when the user attempts to navigate to a - * route that doesn't exist. "Redirects" back to - * the main teams tab, and adds an error message. - */ - notFoundError: function (message) { - this.router.navigate('teams', {trigger: true}); - this.showWarning(message); - }, + topicNotFound: function (topicID) { + this.notFoundError( + interpolate( + gettext('The topic "%(topic)s" could not be found.'), + {topic: topicID}, + true + ) + ); + }, - showWarning: function (message) { - var warningEl = this.$('.warning'); - warningEl.find('.copy').html('

' + message + '' + message + ' +

+ <%= gettext("New Post") %> +
+ diff --git a/lms/djangoapps/teams/templates/teams/teams.html b/lms/djangoapps/teams/templates/teams/teams.html index eb23a11d24..a267b96173 100644 --- a/lms/djangoapps/teams/templates/teams/teams.html +++ b/lms/djangoapps/teams/templates/teams/teams.html @@ -7,8 +7,11 @@ <%block name="bodyclass">view-teams is-in-course course <%block name="pagetitle">${_("Teams")} + <%block name="headextra"> +<%static:css group='style-course-vendor'/> <%static:css group='style-course'/> +<%include file="../discussion/_js_head_dependencies.html" /> <%include file="/courseware/course_navigation.html" args="active_page='teams'" /> @@ -21,16 +24,25 @@ <%block name="js_extra"> + +<%include file="../discussion/_js_body_dependencies.html" /> +<%static:js group='discussion'/> + + <%static:require_module module_name="teams/js/teams_tab_factory" class_name="TeamsTabFactory"> - new TeamsTabFactory({ + TeamsTabFactory({ + courseID: '${ unicode(course.id) }', topics: ${ json.dumps(topics, cls=EscapedEdxJSONEncoder) }, - topic_url: '${ topic_url }', - topics_url: '${ topics_url }', - teams_url: '${ teams_url }', + topicUrl: '${ topic_url }', + topicsUrl: '${ topics_url }', + teamsUrl: '${ teams_url }', maxTeamSize: ${ course.teams_max_size }, - course_id: '${ unicode(course.id) }', languages: ${ json.dumps(languages, cls=EscapedEdxJSONEncoder) }, countries: ${ json.dumps(countries, cls=EscapedEdxJSONEncoder) } }); + +<%include file="../discussion/_underscore_templates.html" /> diff --git a/lms/static/js/spec/main.js b/lms/static/js/spec/main.js index 7f688c473e..b6e3aa8bca 100644 --- a/lms/static/js/spec/main.js +++ b/lms/static/js/spec/main.js @@ -21,6 +21,7 @@ 'jquery.inputnumber': 'xmodule_js/common_static/js/vendor/html5-input-polyfills/number-polyfill', 'jquery.immediateDescendents': 'xmodule_js/common_static/coffee/src/jquery.immediateDescendents', 'jquery.simulate': 'xmodule_js/common_static/js/vendor/jquery.simulate', + 'jquery.timeago': 'xmodule_js/common_static/js/vendor/jquery.timeago', 'jquery.url': 'xmodule_js/common_static/js/vendor/url.min', 'datepair': 'xmodule_js/common_static/js/vendor/timepicker/datepair', 'date': 'xmodule_js/common_static/js/vendor/date', @@ -30,8 +31,8 @@ 'backbone': 'xmodule_js/common_static/js/vendor/backbone-min', 'backbone.associations': 'xmodule_js/common_static/js/vendor/backbone-associations-min', 'backbone.paginator': 'xmodule_js/common_static/js/vendor/backbone.paginator.min', + 'backbone-super': 'js/vendor/backbone-super', 'URI': 'xmodule_js/common_static/js/vendor/URI.min', - "backbone-super": "js/vendor/backbone-super", 'tinymce': 'xmodule_js/common_static/js/vendor/tinymce/js/tinymce/tinymce.full.min', 'jquery.tinymce': 'xmodule_js/common_static/js/vendor/tinymce/js/tinymce/jquery.tinymce', 'xmodule': 'xmodule_js/src/xmodule', @@ -57,6 +58,12 @@ 'capa/display': 'xmodule_js/src/capa/display', 'string_utils': 'xmodule_js/common_static/js/src/string_utils', 'logger': 'xmodule_js/common_static/js/src/logger', + 'Markdown.Converter': 'js/Markdown.Converter', + 'Markdown.Editor': 'js/Markdown.Editor', + 'Markdown.Sanitizer': 'js/Markdown.Sanitizer', + '_split': 'js/split', + 'mathjax_delay_renderer': 'coffee/src/mathjax_delay_renderer', + 'MathJaxProcessor': 'coffee/src/customwmd', // Manually specify LMS files that are not converted to RequireJS 'history': 'js/vendor/history', @@ -83,6 +90,9 @@ 'js/student_profile/views/learner_profile_view': 'js/student_profile/views/learner_profile_view', 'js/ccx/schedule': 'js/ccx/schedule', + // Discussion classes loaded explicitly until they are converted to use RequireJS + 'DiscussionModuleView': 'xmodule_js/common_static/coffee/src/discussion/discussion_module_view', + // edxnotes 'annotator_1.2.9': 'xmodule_js/common_static/js/vendor/edxnotes/annotator-full.min' }, @@ -149,6 +159,10 @@ deps: ['jquery'], exports: 'jQuery.fn.simulate' }, + 'jquery.timeago': { + deps: ['jquery'], + exports: 'jQuery.timeago' + }, 'jquery.tinymce': { deps: ['jquery', 'tinymce'], exports: 'jQuery.fn.tinymce' @@ -192,11 +206,32 @@ exports: 'Backbone.Paginator' }, "backbone-super": { - deps: ["backbone"], + deps: ["backbone"] }, 'youtube': { exports: 'YT' }, + 'Markdown.Converter': { + deps: ['mathjax'], + exports: 'Markdown.Converter' + }, + 'Markdown.Editor': { + deps: ['Markdown.Converter'], + exports: 'Markdown.Editor' + }, + 'Markdown.Sanitizer': { + deps: ['Markdown.Converter'], + exports: 'Markdown.Sanitizer' + }, + '_split': { + exports: '_split' + }, + 'MathJaxProcessor': { + deps: [ + 'Markdown.Converter', 'Markdown.Sanitizer', 'Markdown.Editor', '_split', 'mathjax_delay_renderer' + ], + exports: 'MathJaxProcessor' + }, 'codemirror': { exports: 'CodeMirror' }, @@ -417,7 +452,7 @@ exports: 'edx.verify_student.IntroStepView', deps: [ 'jquery', - 'js/verify_student/views/step_view', + 'js/verify_student/views/step_view' ] }, 'js/verify_student/views/make_payment_step_view': { @@ -429,7 +464,7 @@ 'jquery.cookie', 'jquery.url', 'string_utils', - 'js/verify_student/views/step_view', + 'js/verify_student/views/step_view' ] }, 'js/verify_student/views/payment_confirmation_step_view': { @@ -438,7 +473,7 @@ 'jquery', 'underscore', 'gettext', - 'js/verify_student/views/step_view', + 'js/verify_student/views/step_view' ] }, 'js/verify_student/views/face_photo_step_view': { @@ -475,14 +510,14 @@ exports: 'edx.verify_student.EnrollmentConfirmationStepView', deps: [ 'jquery', - 'js/verify_student/views/step_view', + 'js/verify_student/views/step_view' ] }, 'js/verify_student/views/reverify_success_step_view': { exports: 'edx.verify_student.ReverifySuccessStepView', deps: [ 'jquery', - 'js/verify_student/views/step_view', + 'js/verify_student/views/step_view' ] }, 'js/verify_student/views/pay_and_verify_view': { @@ -520,21 +555,169 @@ 'annotator_1.2.9': { exports: 'Annotator', deps: ['jquery'] + }, + // Discussions + 'xmodule_js/common_static/coffee/src/discussion/utils': { + deps: [ + 'jquery', + 'jquery.timeago', + 'underscore', + 'backbone', + 'gettext', + 'MathJaxProcessor', + 'URI' + ], + exports: 'DiscussionUtil', + init: function() { + // Set global variables that the discussion code is expecting to be defined + window.Backbone = require('backbone'); + window.URI = require('URI'); + } + }, + 'xmodule_js/common_static/coffee/src/discussion/content': { + deps: [ + 'xmodule_js/common_static/coffee/src/discussion/utils' + ], + exports: 'Content' + }, + 'xmodule_js/common_static/coffee/src/discussion/discussion': { + deps: [ + 'xmodule_js/common_static/coffee/src/discussion/utils', + 'xmodule_js/common_static/coffee/src/discussion/content' + ], + exports: 'Discussion' + }, + 'xmodule_js/common_static/coffee/src/discussion/discussion_filter': { + deps: [ + 'xmodule_js/common_static/coffee/src/discussion/utils' + ], + exports: 'DiscussionFilter' + }, + 'xmodule_js/common_static/coffee/src/discussion/models/discussion_course_settings': { + deps: [ + 'xmodule_js/common_static/coffee/src/discussion/utils' + ], + exports: 'DiscussionCourseSettings' + }, + 'xmodule_js/common_static/coffee/src/discussion/models/discussion_user': { + deps: [ + 'xmodule_js/common_static/coffee/src/discussion/utils' + ], + exports: 'DiscussionUser' + }, + 'xmodule_js/common_static/coffee/src/discussion/views/discussion_content_view': { + deps: [ + 'xmodule_js/common_static/coffee/src/discussion/utils' + ], + exports: 'DiscussionContentView' + }, + 'xmodule_js/common_static/coffee/src/discussion/views/discussion_thread_edit_view': { + deps: [ + 'xmodule_js/common_static/coffee/src/discussion/utils' + ], + exports: 'DiscussionThreadEditView' + }, + 'xmodule_js/common_static/coffee/src/discussion/views/discussion_thread_list_view': { + deps: [ + 'xmodule_js/common_static/coffee/src/discussion/utils' + ], + exports: 'DiscussionThreadListView' + }, + 'xmodule_js/common_static/coffee/src/discussion/views/discussion_thread_profile_view': { + deps: [ + 'xmodule_js/common_static/coffee/src/discussion/utils' + ], + exports: 'DiscussionThreadProfileView' + }, + 'xmodule_js/common_static/coffee/src/discussion/views/discussion_thread_show_view': { + deps: [ + 'xmodule_js/common_static/coffee/src/discussion/utils' + ], + exports: 'DiscussionThreadShowView' + }, + 'xmodule_js/common_static/coffee/src/discussion/views/discussion_thread_view': { + deps: [ + 'xmodule_js/common_static/coffee/src/discussion/utils' + ], + exports: 'DiscussionThreadView' + }, + 'xmodule_js/common_static/coffee/src/discussion/views/discussion_topic_menu_view': { + deps: [ + 'xmodule_js/common_static/coffee/src/discussion/utils' + ], + exports: 'DiscussionTopicMenuView' + }, + 'xmodule_js/common_static/coffee/src/discussion/views/discussion_user_profile_view': { + deps: [ + 'xmodule_js/common_static/coffee/src/discussion/utils' + ], + exports: 'DiscussionUserProfileView' + }, + 'xmodule_js/common_static/coffee/src/discussion/views/new_post_view': { + deps: [ + 'xmodule_js/common_static/coffee/src/discussion/utils' + ], + exports: 'NewPostView' + }, + 'xmodule_js/common_static/coffee/src/discussion/views/thread_response_edit_view': { + deps: [ + 'xmodule_js/common_static/coffee/src/discussion/utils' + ], + exports: 'ThreadResponseEditView' + }, + 'xmodule_js/common_static/coffee/src/discussion/views/thread_response_show_view': { + deps: [ + 'xmodule_js/common_static/coffee/src/discussion/utils' + ], + exports: 'ThreadResponseShowView' + }, + 'xmodule_js/common_static/coffee/src/discussion/views/thread_response_view': { + deps: [ + 'xmodule_js/common_static/coffee/src/discussion/utils' + ], + exports: 'ThreadResponseView' + }, + 'DiscussionModuleView': { + deps: [ + 'jquery', + 'underscore', + 'backbone', + 'gettext', + 'URI', + 'xmodule_js/common_static/coffee/src/discussion/content', + 'xmodule_js/common_static/coffee/src/discussion/discussion', + 'xmodule_js/common_static/coffee/src/discussion/discussion_filter', + 'xmodule_js/common_static/coffee/src/discussion/utils', + 'xmodule_js/common_static/coffee/src/discussion/models/discussion_course_settings', + 'xmodule_js/common_static/coffee/src/discussion/models/discussion_user', + 'xmodule_js/common_static/coffee/src/discussion/views/discussion_content_view', + 'xmodule_js/common_static/coffee/src/discussion/views/discussion_thread_edit_view', + 'xmodule_js/common_static/coffee/src/discussion/views/discussion_thread_list_view', + 'xmodule_js/common_static/coffee/src/discussion/views/discussion_thread_profile_view', + 'xmodule_js/common_static/coffee/src/discussion/views/discussion_thread_show_view', + 'xmodule_js/common_static/coffee/src/discussion/views/discussion_thread_view', + 'xmodule_js/common_static/coffee/src/discussion/views/discussion_topic_menu_view', + 'xmodule_js/common_static/coffee/src/discussion/views/discussion_user_profile_view', + 'xmodule_js/common_static/coffee/src/discussion/views/new_post_view', + 'xmodule_js/common_static/coffee/src/discussion/views/thread_response_edit_view', + 'xmodule_js/common_static/coffee/src/discussion/views/thread_response_show_view', + 'xmodule_js/common_static/coffee/src/discussion/views/thread_response_view' + ], + exports: 'DiscussionModuleView' + }, + 'xmodule_js/common_static/coffee/spec/discussion/discussion_spec_helper': { + deps: [ + 'xmodule_js/common_static/coffee/src/discussion/utils' + ], + exports: 'DiscussionSpecHelper' } + } }); // TODO: why do these need 'lms/include' at the front but the CMS equivalent logic doesn't? define([ // Run the LMS tests - 'lms/include/teams/js/spec/teams_factory_spec.js', - 'lms/include/teams/js/spec/topic_card_spec.js', - 'lms/include/teams/js/spec/topic_collection_spec.js', - 'lms/include/teams/js/spec/topics_spec.js', - 'lms/include/teams/js/spec/teams_spec.js', - 'lms/include/teams/js/spec/teams_tab_spec.js', - 'lms/include/teams/js/spec/team_actions_spec.js', - 'lms/include/teams/js/spec/edit_team_spec.js', 'lms/include/js/spec/components/header/header_spec.js', 'lms/include/js/spec/components/tabbed/tabbed_view_spec.js', 'lms/include/js/spec/components/card/card_spec.js', @@ -605,7 +788,17 @@ 'lms/include/js/spec/discovery/views/refine_sidebar_spec.js', 'lms/include/js/spec/discovery/views/search_form_spec.js', 'lms/include/js/spec/discovery/discovery_factory_spec.js', - 'lms/include/js/spec/ccx/schedule_spec.js' + 'lms/include/js/spec/ccx/schedule_spec.js', + 'lms/include/teams/js/spec/collections/topic_collection_spec.js', + 'lms/include/teams/js/spec/edit_team_spec.js', + 'lms/include/teams/js/spec/team_actions_spec.js', + 'lms/include/teams/js/spec/teams_factory_spec.js', + 'lms/include/teams/js/spec/views/team_discussion_spec.js', + 'lms/include/teams/js/spec/views/team_profile_spec.js', + 'lms/include/teams/js/spec/views/teams_spec.js', + 'lms/include/teams/js/spec/views/teams_tab_spec.js', + 'lms/include/teams/js/spec/views/topic_card_spec.js', + 'lms/include/teams/js/spec/views/topics_spec.js', ]); }).call(this, requirejs, define); diff --git a/lms/static/js_test.yml b/lms/static/js_test.yml index 82b875da50..4aa034d22f 100644 --- a/lms/static/js_test.yml +++ b/lms/static/js_test.yml @@ -39,6 +39,7 @@ lib_paths: - xmodule_js/common_static/js/vendor/jquery.min.js - xmodule_js/common_static/js/vendor/jquery-ui.min.js - xmodule_js/common_static/js/vendor/jquery.cookie.js + - xmodule_js/common_static/js/vendor/jquery.timeago.js - xmodule_js/common_static/js/vendor/flot/jquery.flot.js - xmodule_js/common_static/js/vendor/CodeMirror/codemirror.js - xmodule_js/common_static/js/vendor/URI.min.js @@ -66,8 +67,10 @@ lib_paths: # Paths to source JavaScript files src_paths: - js + - coffee/src - common/js - teams/js + - xmodule_js/common_static/coffee # Paths to spec (test) JavaScript files spec_paths: diff --git a/lms/static/lms/js/build.js b/lms/static/lms/js/build.js index 33b1903289..81d8568dd9 100644 --- a/lms/static/lms/js/build.js +++ b/lms/static/lms/js/build.js @@ -64,6 +64,7 @@ 'logger': 'empty:', 'utility': 'empty:', 'URI': 'empty:', + 'DiscussionModuleView': 'empty:' }, /** diff --git a/lms/static/lms/js/require-config.js b/lms/static/lms/js/require-config.js index 50c433affd..13270a1993 100644 --- a/lms/static/lms/js/require-config.js +++ b/lms/static/lms/js/require-config.js @@ -23,6 +23,7 @@ defineDependency("Logger", "logger"); defineDependency("URI", "URI"); defineDependency("Backbone", "backbone"); + // utility.js adds two functions to the window object, but does not return anything defineDependency("isExternal", "utility", true); } From 41173fefee1c188f9165a04506d0a33b9d28fe24 Mon Sep 17 00:00:00 2001 From: cahrens Date: Wed, 29 Jul 2015 13:50:17 -0400 Subject: [PATCH 2/5] Only verify course access for threads with course context. --- .../django_comment_client/forum/tests.py | 39 ++++++++++++++++++- .../django_comment_client/forum/views.py | 7 +--- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/lms/djangoapps/django_comment_client/forum/tests.py b/lms/djangoapps/django_comment_client/forum/tests.py index 2dc54201d1..54b5bb3114 100644 --- a/lms/djangoapps/django_comment_client/forum/tests.py +++ b/lms/djangoapps/django_comment_client/forum/tests.py @@ -176,6 +176,7 @@ def make_mock_request_impl( thread_id=thread_id, num_children=num_thread_responses, group_id=group_id, + commentable_id=commentable_id ) elif "/users/" in url: data = { @@ -336,8 +337,8 @@ class SingleThreadQueryCountTestCase(ModuleStoreTestCase): @ddt.data( # old mongo with cache - (ModuleStoreEnum.Type.mongo, 1, 7, 5, 14, 8), - (ModuleStoreEnum.Type.mongo, 50, 7, 5, 14, 8), + (ModuleStoreEnum.Type.mongo, 1, 6, 4, 14, 8), + (ModuleStoreEnum.Type.mongo, 50, 6, 4, 14, 8), # split mongo: 3 queries, regardless of thread response size. (ModuleStoreEnum.Type.split, 1, 3, 3, 14, 8), (ModuleStoreEnum.Type.split, 50, 3, 3, 14, 8), @@ -668,6 +669,40 @@ class SingleThreadContentGroupTestCase(ContentGroupTestCase): self.assert_can_access(self.non_cohorted_user, self.beta_module.discussion_id, thread_id, False) + def test_course_context_respected(self, mock_request): + """ + Verify that course threads go through discussion_category_id_access method. + """ + thread_id = "test_thread_id" + mock_request.side_effect = make_mock_request_impl( + course=self.course, text="dummy content", thread_id=thread_id + ) + + # Beta user does not have access to alpha_module. + self.assert_can_access(self.beta_user, self.alpha_module.discussion_id, thread_id, False) + + def test_standalone_context_respected(self, mock_request): + """ + Verify that standalone threads don't go through discussion_category_id_access method. + """ + # For this rather pathological test, we are assigning the alpha module discussion_id (commentable_id) + # to a team so that we can verify that standalone threads don't go through discussion_category_id_access. + thread_id = "test_thread_id" + CourseTeamFactory( + name="A team", + course_id=self.course.id, + topic_id='topic_id', + discussion_topic_id=self.alpha_module.discussion_id + ) + mock_request.side_effect = make_mock_request_impl( + course=self.course, text="dummy content", thread_id=thread_id, + commentable_id=self.alpha_module.discussion_id + ) + + # If a thread returns context other than "course", the access check is not done, and the beta user + # can see the alpha discussion module. + self.assert_can_access(self.beta_user, self.alpha_module.discussion_id, thread_id, True) + @patch('lms.lib.comment_client.utils.requests.request') class InlineDiscussionContextTestCase(ModuleStoreTestCase): diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index 6d57b0f983..3ff0a9a8c3 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -320,10 +320,6 @@ def single_thread(request, course_key, discussion_id, thread_id): user_info = cc_user.to_dict() is_moderator = has_permission(request.user, "see_all_cohorts", course_key) - # Verify that the student has access to this thread if belongs to a discussion module - if discussion_id not in utils.get_discussion_categories_ids(course, request.user): - raise Http404 - # Currently, the front end always loads responses via AJAX, even for this # page; it would be a nice optimization to avoid that extra round trip to # the comments service. @@ -340,8 +336,7 @@ def single_thread(request, course_key, discussion_id, thread_id): raise # Verify that the student has access to this thread if belongs to a course discussion module - thread_context = getattr(thread, "context", "course") - if thread_context == "course" and not utils.discussion_category_id_access(course, request.user, discussion_id): + if thread.context == "course" and not utils.discussion_category_id_access(course, request.user, discussion_id): raise Http404 # verify that the thread belongs to the requesting student's cohort From dd67f8aa72d64fc573f281459f1c12286b7d057d Mon Sep 17 00:00:00 2001 From: Marco Morales Date: Tue, 4 Aug 2015 11:19:47 -0400 Subject: [PATCH 3/5] updated inline discussion styling to account for lack of expand / show message (which was being relied on for layout) and also removed issues of generic pagination styling impacting the older inline discussion pagination pattern with a class renaming --- .../view/discussion_user_profile_view_spec.coffee | 4 ++-- .../coffee/src/discussion/discussion_module_view.coffee | 2 +- .../discussion/views/discussion_user_profile_view.coffee | 2 +- .../templates/discussion/inline-discussion.underscore | 2 +- .../common/templates/discussion/user-profile.underscore | 2 +- lms/static/sass/discussion/_discussion.scss | 6 +++--- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/common/static/coffee/spec/discussion/view/discussion_user_profile_view_spec.coffee b/common/static/coffee/spec/discussion/view/discussion_user_profile_view_spec.coffee index c3bb52fe41..989893d64e 100644 --- a/common/static/coffee/spec/discussion/view/discussion_user_profile_view_spec.coffee +++ b/common/static/coffee/spec/discussion/view/discussion_user_profile_view_spec.coffee @@ -205,7 +205,7 @@ describe "DiscussionUserProfileView", -> ) {always: ->} ) - @view.$(".pagination a").first().click() + @view.$(".discussion-pagination a").first().click() expect(@view.$(".current-page").text()).toEqual("42") expect(@view.$(".last-page").text()).toEqual("99") @@ -216,5 +216,5 @@ describe "DiscussionUserProfileView", -> params.error() {always: ->} ) - @view.$(".pagination a").first().click() + @view.$(".discussion-pagination a").first().click() expect(DiscussionUtil.discussionAlert).toHaveBeenCalled() diff --git a/common/static/coffee/src/discussion/discussion_module_view.coffee b/common/static/coffee/src/discussion/discussion_module_view.coffee index 0fd883d6c7..198d48d34c 100644 --- a/common/static/coffee/src/discussion/discussion_module_view.coffee +++ b/common/static/coffee/src/discussion/discussion_module_view.coffee @@ -155,7 +155,7 @@ if Backbone? "?discussion_page=#{number}" params = DiscussionUtil.getPaginationParams(@page, numPages, pageUrl) pagination = _.template($("#pagination-template").html())(params) - @$('section.pagination').html(pagination) + @$('section.discussion-pagination').html(pagination) navigateToPage: (event) => event.preventDefault() diff --git a/common/static/coffee/src/discussion/views/discussion_user_profile_view.coffee b/common/static/coffee/src/discussion/views/discussion_user_profile_view.coffee index 77cfd7df4f..6b03fca327 100644 --- a/common/static/coffee/src/discussion/views/discussion_user_profile_view.coffee +++ b/common/static/coffee/src/discussion/views/discussion_user_profile_view.coffee @@ -18,7 +18,7 @@ if Backbone? baseUri = URI(window.location).removeSearch("page") pageUrlFunc = (page) -> baseUri.clone().addSearch("page", page) paginationParams = DiscussionUtil.getPaginationParams(@page, @numPages, pageUrlFunc) - @$el.find(".pagination").html(_.template($("#pagination-template").html())(paginationParams)) + @$el.find(".discussion-pagination").html(_.template($("#pagination-template").html())(paginationParams)) changePage: (event) -> event.preventDefault() diff --git a/common/static/common/templates/discussion/inline-discussion.underscore b/common/static/common/templates/discussion/inline-discussion.underscore index 42f7f2f2f2..96dde1bed8 100644 --- a/common/static/common/templates/discussion/inline-discussion.underscore +++ b/common/static/common/templates/discussion/inline-discussion.underscore @@ -8,6 +8,6 @@ <% }); %> -
+
diff --git a/common/static/common/templates/discussion/user-profile.underscore b/common/static/common/templates/discussion/user-profile.underscore index 0eeda55723..b4e683ae72 100644 --- a/common/static/common/templates/discussion/user-profile.underscore +++ b/common/static/common/templates/discussion/user-profile.underscore @@ -4,4 +4,4 @@
<% }); %> -
+
diff --git a/lms/static/sass/discussion/_discussion.scss b/lms/static/sass/discussion/_discussion.scss index 7425a41618..09181d0012 100644 --- a/lms/static/sass/discussion/_discussion.scss +++ b/lms/static/sass/discussion/_discussion.scss @@ -753,10 +753,10 @@ body.discussion { } section.discussion { - margin-top: ($baseline*1.5); + clear: both; + padding-top: $baseline; .threads { - margin-top: $baseline; } .discussion-thread { @@ -936,7 +936,7 @@ body.discussion { color: $white; } - section.pagination { + section.discussion-pagination { margin-top: ($baseline*1.5); nav.discussion-paginator { From f5840489a0e708f13ccf7d334ce88514345a9660 Mon Sep 17 00:00:00 2001 From: Daniel Friedman Date: Tue, 4 Aug 2015 13:52:40 -0400 Subject: [PATCH 4/5] Allow editing of own post in team discussion --- lms/djangoapps/django_comment_client/base/tests.py | 8 ++++++-- lms/djangoapps/django_comment_client/base/views.py | 8 +++++--- lms/djangoapps/django_comment_client/forum/views.py | 3 ++- lms/djangoapps/django_comment_client/utils.py | 1 + 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/lms/djangoapps/django_comment_client/base/tests.py b/lms/djangoapps/django_comment_client/base/tests.py index 6cc18b361f..be3adc135c 100644 --- a/lms/djangoapps/django_comment_client/base/tests.py +++ b/lms/djangoapps/django_comment_client/base/tests.py @@ -1193,7 +1193,11 @@ class TeamsPermissionsTestCase(UrlResetMixin, ModuleStoreTestCase, MockRequestSe thread_author = getattr(self, thread_author) self._setup_mock( user, mock_request, # user is the person making the request. - {"user_id": str(thread_author.id), "closed": False, "commentable_id": commentable_id} + { + "user_id": str(thread_author.id), + "closed": False, "commentable_id": commentable_id, + "context": "standalone" + } ) response = self.client.post( reverse( @@ -1203,7 +1207,7 @@ class TeamsPermissionsTestCase(UrlResetMixin, ModuleStoreTestCase, MockRequestSe "thread_id": "dummy" } ), - data={"body": "foo", "title": "foo"} + data={"body": "foo", "title": "foo", "commentable_id": commentable_id} ) self.assertEqual(response.status_code, status_code) diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index 119bd59697..aacc1cc571 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -250,11 +250,13 @@ def update_thread(request, course_id, thread_id): if "thread_type" in request.POST: thread.thread_type = request.POST["thread_type"] if "commentable_id" in request.POST: + commentable_id = request.POST["commentable_id"] course = get_course_with_access(request.user, 'load', course_key) - if discussion_category_id_access(course, request.user, request.POST.get("commentable_id")): - thread.commentable_id = request.POST["commentable_id"] - else: + thread_context = getattr(thread, "context", "course") + if thread_context == "course" and not discussion_category_id_access(course, request.user, commentable_id): return JsonError(_("Topic doesn't exist")) + else: + thread.commentable_id = commentable_id thread.save() if request.is_ajax(): diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index 3ff0a9a8c3..484bccd7d8 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -336,7 +336,8 @@ def single_thread(request, course_key, discussion_id, thread_id): raise # Verify that the student has access to this thread if belongs to a course discussion module - if thread.context == "course" and not utils.discussion_category_id_access(course, request.user, discussion_id): + thread_context = getattr(thread, "context", "course") + if thread_context == "course" and not utils.discussion_category_id_access(course, request.user, discussion_id): raise Http404 # verify that the thread belongs to the requesting student's cohort diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index e50402e0df..e0c83e29ec 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -351,6 +351,7 @@ def get_discussion_category_map(course, user, cohorted_if_in_list=False, exclude def discussion_category_id_access(course, user, discussion_id): """ Returns True iff the given discussion_id is accessible for user in course. + Assumes that the commentable identified by discussion_id has a null or 'course' context. Uses the discussion id cache if available, falling back to get_discussion_categories_ids if there is no cache. """ From 2f7fa6fd8b41cfbde1ac76f6a41d8df5c7dcec90 Mon Sep 17 00:00:00 2001 From: Peter Fogg Date: Fri, 31 Jul 2015 11:40:53 -0400 Subject: [PATCH 5/5] Enforce discussions permissions in teams UI. Adds a read-only mode to the discussion module which disables any controls for updating the discussion (votes, follows, replies, etc). This mode is enabled for users who are not a member of the team, and are also not moderators, admins, or community TAs. --- .../discussion/discussion_spec_helper.coffee | 1 + .../views/discussion_thread_show_view.coffee | 3 +- .../views/discussion_thread_view.coffee | 9 ++- .../views/response_comment_show_view.coffee | 3 +- .../views/thread_response_show_view.coffee | 3 +- .../views/thread_response_view.coffee | 15 ++-- .../discussion/forum-actions.underscore | 32 +++++---- .../response-comment-show.underscore | 3 +- .../thread-response-show.underscore | 3 +- .../discussion/thread-response.underscore | 22 +++--- .../discussion/thread-show.underscore | 29 ++++---- .../templates/discussion/thread.underscore | 16 +++-- .../test/acceptance/tests/lms/test_teams.py | 70 ++++++++++++++++--- .../django_comment_client/tests/test_utils.py | 4 ++ .../teams/js/spec/views/teams_tab_spec.js | 30 ++++++++ .../static/teams/js/views/team_profile.js | 5 +- .../teams/static/teams/js/views/teams_tab.js | 27 +++++-- .../teams/templates/team-profile.underscore | 9 ++- .../teams/templates/teams/teams.html | 2 +- lms/djangoapps/teams/tests/test_views.py | 10 +-- lms/djangoapps/teams/views.py | 3 + .../discussion/_discussion_module.html | 2 +- lms/templates/discussion/index.html | 1 + openedx/core/lib/api/serializers.py | 8 +-- 24 files changed, 222 insertions(+), 88 deletions(-) diff --git a/common/static/coffee/spec/discussion/discussion_spec_helper.coffee b/common/static/coffee/spec/discussion/discussion_spec_helper.coffee index 0fee756658..0b052bc7ba 100644 --- a/common/static/coffee/spec/discussion/discussion_spec_helper.coffee +++ b/common/static/coffee/spec/discussion/discussion_spec_helper.coffee @@ -67,5 +67,6 @@ class @DiscussionSpecHelper data-course-name="Fake Course" data-user-create-comment="true" data-user-create-subcomment="true" + data-read-only="false" > """) diff --git a/common/static/coffee/src/discussion/views/discussion_thread_show_view.coffee b/common/static/coffee/src/discussion/views/discussion_thread_show_view.coffee index 5a1a0a5b29..75c6e9d5ac 100644 --- a/common/static/coffee/src/discussion/views/discussion_thread_show_view.coffee +++ b/common/static/coffee/src/discussion/views/discussion_thread_show_view.coffee @@ -13,7 +13,8 @@ if Backbone? mode: @mode, flagged: @model.isFlagged(), author_display: @getAuthorDisplay(), - cid: @model.cid + cid: @model.cid, + readOnly: $('.discussion-module').data('read-only') }, @model.attributes, ) diff --git a/common/static/coffee/src/discussion/views/discussion_thread_view.coffee b/common/static/coffee/src/discussion/views/discussion_thread_view.coffee index 798a4022b6..f9a6d21b00 100644 --- a/common/static/coffee/src/discussion/views/discussion_thread_view.coffee +++ b/common/static/coffee/src/discussion/views/discussion_thread_view.coffee @@ -23,6 +23,8 @@ if Backbone? if @mode not in ["tab", "inline"] throw new Error("invalid mode: " + @mode) + @readOnly = $(".discussion-module").data('read-only') + # Quick fix to have an actual model when we're receiving new models from # the server. @model.collection.on "reset", (collection) => @@ -51,12 +53,15 @@ if Backbone? renderTemplate: -> @template = _.template($("#thread-template").html()) - templateData = @model.toJSON() container = $("#discussion-container") if !container.length # inline discussion container = $(".discussion-module") - templateData.can_create_comment = container.data("user-create-comment") + templateData = _.extend( + @model.toJSON(), + readOnly: @readOnly, + can_create_comment: container.data("user-create-comment") + ) @template(templateData) render: -> diff --git a/common/static/coffee/src/discussion/views/response_comment_show_view.coffee b/common/static/coffee/src/discussion/views/response_comment_show_view.coffee index 51432770a2..059e63f65c 100644 --- a/common/static/coffee/src/discussion/views/response_comment_show_view.coffee +++ b/common/static/coffee/src/discussion/views/response_comment_show_view.coffee @@ -9,7 +9,8 @@ if Backbone? _.extend( { cid: @model.cid, - author_display: @getAuthorDisplay() + author_display: @getAuthorDisplay(), + readOnly: $('.discussion-module').data('read-only') }, @model.attributes ) diff --git a/common/static/coffee/src/discussion/views/thread_response_show_view.coffee b/common/static/coffee/src/discussion/views/thread_response_show_view.coffee index 6d634dd481..3e72cd56d9 100644 --- a/common/static/coffee/src/discussion/views/thread_response_show_view.coffee +++ b/common/static/coffee/src/discussion/views/thread_response_show_view.coffee @@ -10,7 +10,8 @@ if Backbone? { cid: @model.cid, author_display: @getAuthorDisplay(), - endorser_display: @getEndorserDisplay() + endorser_display: @getEndorserDisplay(), + readOnly: $('.discussion-module').data('read-only') }, @model.attributes ) diff --git a/common/static/coffee/src/discussion/views/thread_response_view.coffee b/common/static/coffee/src/discussion/views/thread_response_view.coffee index a299738d6f..9d91d1361c 100644 --- a/common/static/coffee/src/discussion/views/thread_response_view.coffee +++ b/common/static/coffee/src/discussion/views/thread_response_view.coffee @@ -13,17 +13,21 @@ if Backbone? initialize: (options) -> @collapseComments = options.collapseComments @createShowView() + @readOnly = $('.discussion-module').data('read-only') renderTemplate: -> @template = _.template($("#thread-response-template").html()) - templateData = @model.toJSON() - templateData.wmdId = @model.id ? (new Date()).getTime() container = $("#discussion-container") if !container.length # inline discussion container = $(".discussion-module") - templateData.create_sub_comment = container.data("user-create-subcomment") + templateData = _.extend( + @model.toJSON(), + wmdId: @model.id ? (new Date()).getTime(), + create_sub_comment: container.data("user-create-subcomment"), + readOnly: @readOnly + ) @template(templateData) render: -> @@ -88,7 +92,10 @@ if Backbone? comment.set('thread', @model.get('thread')) view = new ResponseCommentView(model: comment) view.render() - @$el.find(".comments .new-comment").before(view.el) + if @readOnly + @$el.find('.comments').append(view.el) + else + @$el.find(".comments .new-comment").before(view.el) view.bind "comment:edit", (event) => @cancelEdit(event) if @editView? @cancelCommentEdits() diff --git a/common/static/common/templates/discussion/forum-actions.underscore b/common/static/common/templates/discussion/forum-actions.underscore index 9fd9714a3e..5401e41180 100644 --- a/common/static/common/templates/discussion/forum-actions.underscore +++ b/common/static/common/templates/discussion/forum-actions.underscore @@ -1,16 +1,18 @@ -
    - <% _.each(primaryActions, function(action) { print(_.template($('#forum-action-' + action).html(), {})) }) %> -
  • -
    - - <%- gettext("More") %> - - - -
    • -
+ + +<% } %> diff --git a/common/static/common/templates/discussion/response-comment-show.underscore b/common/static/common/templates/discussion/response-comment-show.underscore index 5f15338f1e..5dbf579afb 100644 --- a/common/static/common/templates/discussion/response-comment-show.underscore +++ b/common/static/common/templates/discussion/response-comment-show.underscore @@ -7,7 +7,8 @@ contentId: cid, contentType: 'comment', primaryActions: [], - secondaryActions: ['edit', 'delete', 'report'] + secondaryActions: ['edit', 'delete', 'report'], + readOnly: readOnly } ) %> diff --git a/common/static/common/templates/discussion/thread-response-show.underscore b/common/static/common/templates/discussion/thread-response-show.underscore index d43fd2d8aa..60b6b82a4d 100644 --- a/common/static/common/templates/discussion/thread-response-show.underscore +++ b/common/static/common/templates/discussion/thread-response-show.underscore @@ -49,7 +49,8 @@ contentId: cid, contentType: 'response', primaryActions: ['vote', thread.get('thread_type') == 'question' ? 'answer' : 'endorse'], - secondaryActions: ['edit', 'delete', 'report'] + secondaryActions: ['edit', 'delete', 'report'], + readOnly: readOnly } ) %> diff --git a/common/static/common/templates/discussion/thread-response.underscore b/common/static/common/templates/discussion/thread-response.underscore index 7e3aeffde2..8032f473da 100644 --- a/common/static/common/templates/discussion/thread-response.underscore +++ b/common/static/common/templates/discussion/thread-response.underscore @@ -12,16 +12,16 @@
  1. - <% if (create_sub_comment) { %> -
    -
      - -
      -
      - <%- gettext("Submit") %> -
      -
      - <% } %> + <% if (create_sub_comment && !readOnly) { %> +
      +
        + +
        +
        + <%- gettext("Submit") %> +
        +
        + <% } %>
      diff --git a/common/static/common/templates/discussion/thread-show.underscore b/common/static/common/templates/discussion/thread-show.underscore index 0b46318bdd..c28e0859c4 100644 --- a/common/static/common/templates/discussion/thread-show.underscore +++ b/common/static/common/templates/discussion/thread-show.underscore @@ -40,19 +40,22 @@ <%- gettext("Closed") %> -
      - <%= - _.template( - $('#forum-actions').html(), - { - contentId: cid, - contentType: 'post', - primaryActions: ['vote', 'follow'], - secondaryActions: ['pin', 'edit', 'delete', 'report', 'close'] - } - ) - %> -
      + <% if (!readOnly) { %> +
      + <%= + _.template( + $('#forum-actions').html(), + { + contentId: cid, + contentType: 'post', + primaryActions: ['vote', 'follow'], + secondaryActions: ['pin', 'edit', 'delete', 'report', 'close'], + readOnly: readOnly + } + ) + %> +
      + <% } %>
      <%- body %>
      diff --git a/common/static/common/templates/discussion/thread.underscore b/common/static/common/templates/discussion/thread.underscore index 3212105475..8498c9ae8d 100644 --- a/common/static/common/templates/discussion/thread.underscore +++ b/common/static/common/templates/discussion/thread.underscore @@ -8,18 +8,20 @@
      -
      - -
      + <% if (!readOnly) { %> +
      + +
      + <% } %>
        - <% if (can_create_comment) { %> + <% if (can_create_comment && !readOnly) { %>

        <%- gettext("Post a response:") %>

          diff --git a/common/test/acceptance/tests/lms/test_teams.py b/common/test/acceptance/tests/lms/test_teams.py index 1dff334ed5..a0430b157e 100644 --- a/common/test/acceptance/tests/lms/test_teams.py +++ b/common/test/acceptance/tests/lms/test_teams.py @@ -3,6 +3,7 @@ Acceptance tests for the teams feature. """ import json +import ddt from nose.plugins.attrib import attr from uuid import uuid4 @@ -652,6 +653,7 @@ class CreateTeamTest(TeamsTabBase): @attr('shard_5') +@ddt.ddt class TeamPageTest(TeamsTabBase): """Tests for viewing a specific team""" def setUp(self): @@ -659,12 +661,11 @@ class TeamPageTest(TeamsTabBase): self.topic = {u"name": u"Example Topic", u"id": "example_topic", u"description": "Description"} self.set_team_configuration({'course_id': self.course_id, 'max_team_size': 10, 'topics': [self.topic]}) self.team = self.create_teams(self.topic, 1)[0] - self.create_membership(self.user_info['username'], self.team['id']) self.team_page = TeamPage(self.browser, self.course_id, self.team) def setup_thread(self): """ - Set up the discussion thread for the team. + Create and return a thread for this test's discussion topic. """ thread = Thread( id="test_thread_{}".format(uuid4().hex), @@ -675,15 +676,25 @@ class TeamPageTest(TeamsTabBase): thread_fixture.push() return thread - def test_discussion_on_team_page(self): + def setup_discussion_user(self, role=None, staff=False): + """Set this test's user to have the given role in its + discussions. Role is one of 'Community TA', 'Moderator', + 'Administrator', or 'Student'. """ - Scenario: Team Page renders a team discussion. - Given I am enrolled in a course with a team configuration, a topic, - and a team belonging to that topic - When a thread exists in the team's discussion - And I visit the Team page for that team - Then I should see a discussion with the correct discussion_id - And I should see the existing thread + kwargs = { + 'course_id': self.course_id, + 'staff': staff + } + if role is not None: + kwargs['roles'] = role + #pylint: disable=attribute-defined-outside-init + self.user_info = AutoAuthPage(self.browser, **kwargs).visit().user_info + + def verify_teams_discussion_permissions(self, should_have_permission): + """Verify that the teams discussion component is in the correct state + for the test user. If `should_have_permission` is True, assert that + the user can see controls for posting replies, voting, editing, and + deleting. Otherwise, assert that those controls are hidden. """ thread = self.setup_thread() self.team_page.visit() @@ -693,3 +704,42 @@ class TeamPageTest(TeamsTabBase): self.assertTrue(discussion.is_discussion_expanded()) self.assertEqual(discussion.get_num_displayed_threads(), 1) self.assertTrue(discussion.has_thread(thread['id'])) + assertion = self.assertTrue if should_have_permission else self.assertFalse + assertion(discussion.q(css='.post-header-actions').present) + assertion(discussion.q(css='.add-response').present) + assertion(discussion.q(css='.new-post-btn').present) + + def test_discussion_on_my_team_page(self): + """ + Scenario: Team Page renders a discussion for a team to which I belong. + Given I am enrolled in a course with a team configuration, a topic, + and a team belonging to that topic of which I am a member + When the team has a discussion with a thread + And I visit the Team page for that team + Then I should see a discussion with the correct discussion_id + And I should see the existing thread + And I should see controls to change the state of the discussion + """ + self.create_membership(self.user_info['username'], self.team['id']) + self.verify_teams_discussion_permissions(True) + + @ddt.data(True, False) + def test_discussion_on_other_team_page(self, is_staff): + """ + Scenario: Team Page renders a team discussion for a team to which I do + not belong. + Given I am enrolled in a course with a team configuration, a topic, + and a team belonging to that topic of which I am not a member + When the team has a discussion with a thread + And I visit the Team page for that team + Then I should see a discussion with the correct discussion_id + And I should see the team's thread + And I should not see controls to change the state of the discussion + """ + self.setup_discussion_user(staff=is_staff) + self.verify_teams_discussion_permissions(False) + + @ddt.data('Moderator', 'Community TA', 'Administrator') + def test_discussion_privileged(self, role): + self.setup_discussion_user(role=role) + self.verify_teams_discussion_permissions(True) diff --git a/lms/djangoapps/django_comment_client/tests/test_utils.py b/lms/djangoapps/django_comment_client/tests/test_utils.py index eff0e186d4..241ae24cf4 100644 --- a/lms/djangoapps/django_comment_client/tests/test_utils.py +++ b/lms/djangoapps/django_comment_client/tests/test_utils.py @@ -22,6 +22,7 @@ from openedx.core.djangoapps.course_groups.tests.helpers import config_course_co from student.tests.factories import UserFactory, AdminFactory, CourseEnrollmentFactory from openedx.core.djangoapps.content.course_structures.models import CourseStructure from openedx.core.djangoapps.util.testing import ContentGroupTestCase +from student.roles import CourseStaffRole from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, TEST_DATA_MIXED_TOY_MODULESTORE from xmodule.modulestore.django import modulestore @@ -80,6 +81,8 @@ class AccessUtilsTestCase(ModuleStoreTestCase): self.community_ta_role.users.add(self.community_ta1) self.community_ta2 = UserFactory(username='community_ta2', email='community_ta2@edx.org') self.community_ta_role.users.add(self.community_ta2) + self.course_staff = UserFactory(username='course_staff', email='course_staff@edx.org') + CourseStaffRole(self.course_id).add_users(self.course_staff) def test_get_role_ids(self): ret = utils.get_role_ids(self.course_id) @@ -89,6 +92,7 @@ class AccessUtilsTestCase(ModuleStoreTestCase): def test_has_discussion_privileges(self): self.assertFalse(utils.has_discussion_privileges(self.student1, self.course_id)) self.assertFalse(utils.has_discussion_privileges(self.student2, self.course_id)) + self.assertFalse(utils.has_discussion_privileges(self.course_staff, self.course_id)) self.assertTrue(utils.has_discussion_privileges(self.moderator, self.course_id)) self.assertTrue(utils.has_discussion_privileges(self.community_ta1, self.course_id)) self.assertTrue(utils.has_discussion_privileges(self.community_ta2, self.course_id)) 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 7b0169f706..8aee23a2d0 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 @@ -88,5 +88,35 @@ define([ expectFocus(teamsTabView.$('.warning')); }); }); + + describe('Discussion privileges', function () { + it('allows privileged access to any team', function () { + teamsTabView.$el.data('privileged', true); + // Note: using `undefined` here to ensure that we + // don't even look at the team when the user is + // privileged + expect(teamsTabView.readOnlyDiscussion(undefined)).toBe(false); + }); + + it('allows access to a team which an unprivileged user is a member of', function () { + teamsTabView.$el.data('privileged', false).data('username', 'test-user'); + expect(teamsTabView.readOnlyDiscussion({ + attributes: { + membership: [{ + user: { + username: 'test-user' + } + }] + } + })).toBe(false); + }); + + it('does not allow access if the user is neither privileged nor a team member', function () { + teamsTabView.$el.data('privileged', false).data('username', 'test-user'); + expect(teamsTabView.readOnlyDiscussion({ + attributes: { membership: [] } + })).toBe(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 a0a0d7794a..dddcafc916 100644 --- a/lms/djangoapps/teams/static/teams/js/views/team_profile.js +++ b/lms/djangoapps/teams/static/teams/js/views/team_profile.js @@ -10,15 +10,14 @@ initialize: function (options) { this.courseID = options.courseID; this.discussionTopicID = this.model.get('discussion_topic_id'); + this.readOnly = options.readOnly; }, render: function () { - var canPostToTeam = true; // TODO: determine this permission correctly! this.$el.html(_.template(teamTemplate, { courseID: this.courseID, discussionTopicID: this.discussionTopicID, - canCreateComment: canPostToTeam, - canCreateSubComment: canPostToTeam + readOnly: this.readOnly })); this.discussionView = new TeamDiscussionView({ el: this.$('.discussion-module') 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 89de3e7c28..131e8ad50e 100644 --- a/lms/djangoapps/teams/static/teams/js/views/teams_tab.js +++ b/lms/djangoapps/teams/static/teams/js/views/teams_tab.js @@ -209,10 +209,12 @@ courseID = this.courseID; self.getTopic(topicID).done(function(topic) { self.getTeam(teamID).done(function(team) { - var view = new TeamProfileView({ - courseID: courseID, - model: team - }); + var readOnly = self.readOnlyDiscussion(team), + view = new TeamProfileView({ + courseID: courseID, + model: team, + readOnly: readOnly + }); deferred.resolve(self.createViewWithHeader(view, team, topic)); }); }); @@ -377,6 +379,23 @@ hideWarning: function () { this.$('.warning').toggleClass('is-hidden', true); + }, + + /** + * Returns true if the discussion thread belonging to + * `team` is accessible to the user. This is the case + * if the user is privileged (i.e., a community TA, + * moderator, or administrator), or if the user + * belongs to the team. + */ + readOnlyDiscussion: function (team) { + var self = this; + return !( + this.$el.data('privileged') || + _.any(team.attributes.membership, function (membership) { + return membership.user.username === self.$el.data('username'); + }) + ); } }); diff --git a/lms/djangoapps/teams/static/teams/templates/team-profile.underscore b/lms/djangoapps/teams/static/teams/templates/team-profile.underscore index bb7ae65320..f524b7939d 100644 --- a/lms/djangoapps/teams/static/teams/templates/team-profile.underscore +++ b/lms/djangoapps/teams/static/teams/templates/team-profile.underscore @@ -1,7 +1,10 @@
          - <%= gettext("New Post") %> + data-read-only="<%= readOnly %>" + data-user-create-comment="<%= !readOnly %>" + data-user-create-subcomment="<%= !readOnly %>"> + <% if ( !readOnly) { %> + <%= gettext("New Post") %> + <% } %>
          diff --git a/lms/djangoapps/teams/templates/teams/teams.html b/lms/djangoapps/teams/templates/teams/teams.html index a267b96173..d2ac44343a 100644 --- a/lms/djangoapps/teams/templates/teams/teams.html +++ b/lms/djangoapps/teams/templates/teams/teams.html @@ -18,7 +18,7 @@
          -
          +
          diff --git a/lms/djangoapps/teams/tests/test_views.py b/lms/djangoapps/teams/tests/test_views.py index 5c12057ff8..b8c637a8f7 100644 --- a/lms/djangoapps/teams/tests/test_views.py +++ b/lms/djangoapps/teams/tests/test_views.py @@ -467,7 +467,7 @@ class TestCreateTeamAPI(TeamAPITestCase): # Verify that the creating user gets added to the team. self.assertEqual(len(team_membership), 1) member = team_membership[0]['user'] - self.assertEqual(member['id'], creator) + self.assertEqual(member['username'], creator) self.assertEqual(team, { 'name': 'Fully specified team', @@ -688,7 +688,7 @@ class TestListMembershipAPI(TeamAPITestCase): membership = self.get_membership_list(status, {'team_id': self.test_team_1.team_id}, user=user) if status == 200: self.assertEqual(membership['count'], 1) - self.assertEqual(membership['results'][0]['user']['id'], self.users['student_enrolled'].username) + self.assertEqual(membership['results'][0]['user']['username'], self.users['student_enrolled'].username) @ddt.data( (None, 401, False), @@ -705,7 +705,7 @@ class TestListMembershipAPI(TeamAPITestCase): if status == 200: if has_content: self.assertEqual(membership['count'], 1) - self.assertEqual(membership['results'][0]['team']['id'], self.test_team_1.team_id) + self.assertEqual(membership['results'][0]['team']['team_id'], self.test_team_1.team_id) else: self.assertEqual(membership['count'], 0) @@ -754,8 +754,8 @@ class TestCreateMembershipAPI(TeamAPITestCase): user=user ) if status == 200: - self.assertEqual(membership['user']['id'], self.users['student_enrolled_not_on_team'].username) - self.assertEqual(membership['team']['id'], self.test_team_1.team_id) + self.assertEqual(membership['user']['username'], self.users['student_enrolled_not_on_team'].username) + self.assertEqual(membership['team']['team_id'], self.test_team_1.team_id) memberships = self.get_membership_list(200, {'team_id': self.test_team_1.team_id}) self.assertEqual(memberships['count'], 2) diff --git a/lms/djangoapps/teams/views.py b/lms/djangoapps/teams/views.py index 313b433c9f..cdcd92c08b 100644 --- a/lms/djangoapps/teams/views.py +++ b/lms/djangoapps/teams/views.py @@ -90,6 +90,7 @@ class TeamsDashboardView(View): instance=topics_page, context={'course_id': course.id, 'sort_order': sort_order} ) + user = request.user context = { "course": course, "topics": topics_serializer.data, @@ -100,6 +101,8 @@ class TeamsDashboardView(View): "teams_url": reverse('teams_list', request=request), "languages": settings.ALL_LANGUAGES, "countries": list(countries), + "username": user.username, + "privileged": has_discussion_privileges(user, course_key) } return render_to_response("teams/teams.html", context) diff --git a/lms/templates/discussion/_discussion_module.html b/lms/templates/discussion/_discussion_module.html index aedf007343..ba6d1c88e9 100644 --- a/lms/templates/discussion/_discussion_module.html +++ b/lms/templates/discussion/_discussion_module.html @@ -3,7 +3,7 @@ from django.utils.translation import ugettext as _ %> -
          +
          ${_("Show Discussion")} % if can_create_thread: ${_("New Post")} diff --git a/lms/templates/discussion/index.html b/lms/templates/discussion/index.html index 8b074b8190..00fdfd773d 100644 --- a/lms/templates/discussion/index.html +++ b/lms/templates/discussion/index.html @@ -31,6 +31,7 @@ from django.core.urlresolvers import reverse data-user-info="${user_info}" data-user-create-comment="${can_create_comment}" data-user-create-subcomment="${can_create_subcomment}" + data-read-only="false" data-threads="${threads}" data-thread-pages="${thread_pages}" data-content-info="${annotated_content_info}" diff --git a/openedx/core/lib/api/serializers.py b/openedx/core/lib/api/serializers.py index 29c20a570b..03ec001137 100644 --- a/openedx/core/lib/api/serializers.py +++ b/openedx/core/lib/api/serializers.py @@ -31,7 +31,6 @@ class PaginationSerializer(pagination.PaginationSerializer): class CollapsedReferenceSerializer(serializers.HyperlinkedModelSerializer): """Serializes arbitrary models in a collapsed format, with just an id and url.""" - id = serializers.CharField(read_only=True) # pylint: disable=invalid-name url = serializers.HyperlinkedIdentityField(view_name='') def __init__(self, model_class, view_name, id_source='id', lookup_field=None, *args, **kwargs): @@ -42,7 +41,8 @@ class CollapsedReferenceSerializer(serializers.HyperlinkedModelSerializer): view_name (string): Name of the Django view used to lookup the model. id_source (string): Optional name of the id field on the model. - Defaults to 'id'. + Defaults to 'id'. Also used as the property name of the field + in the serialized representation. lookup_field (string): Optional name of the model field used to lookup the model in the view. Defaults to the value of id_source. @@ -54,7 +54,7 @@ class CollapsedReferenceSerializer(serializers.HyperlinkedModelSerializer): super(CollapsedReferenceSerializer, self).__init__(*args, **kwargs) - self.fields['id'].source = id_source + self.fields[id_source] = serializers.CharField(read_only=True, source=id_source) self.fields['url'].view_name = view_name self.fields['url'].lookup_field = lookup_field @@ -63,4 +63,4 @@ class CollapsedReferenceSerializer(serializers.HyperlinkedModelSerializer): model is set dynamically in __init__. """ - fields = ("id", "url") + fields = ("url",)