From 21b39ca4675ae2006d983e07fe84dd207bfbd8b5 Mon Sep 17 00:00:00 2001 From: Daniel Friedman Date: Wed, 8 Jul 2015 17:41:08 -0400 Subject: [PATCH] Add teams-for-topic list view to Teams page. TNL-1898 Also adds a generic paginated list view and Teams list view. The common PaginatedView takes a collection and a view class for its items and creates a header and footer with correct pagination. The topics list view is refactored to use this generic view. Authors: - Peter Fogg - Daniel Friedman --- .../js/components/views/paginated_view.js | 43 +++ .../js/components/views/paging_footer.js | 3 +- .../js/components/views/paging_header.js | 4 +- .../js/spec/components/paginated_view_spec.js | 184 +++++++++++++ .../components/paginated-view.underscore | 4 + common/static/js/spec/main_requirejs.js | 1 + common/test/acceptance/pages/lms/teams.py | 51 +++- .../test/acceptance/tests/lms/test_teams.py | 249 ++++++++++++++++- lms/djangoapps/teams/api_urls.py | 2 +- lms/djangoapps/teams/serializers.py | 47 +++- .../teams/static/teams/js/collections/team.js | 6 +- .../teams/js/spec/teams_factory_spec.js | 10 +- .../teams/static/teams/js/spec/teams_spec.js | 53 ++++ .../static/teams/js/spec/teams_tab_spec.js | 74 +++++ .../teams/static/teams/js/spec/topics_spec.js | 152 +--------- .../static/teams/js/teams_tab_factory.js | 16 +- .../teams/static/teams/js/views/team_card.js | 85 ++++++ .../teams/static/teams/js/views/teams.js | 33 +++ .../teams/static/teams/js/views/teams_tab.js | 260 ++++++++++++++++-- .../teams/static/teams/js/views/topic_card.js | 2 +- .../teams/static/teams/js/views/topics.js | 51 +--- .../team-country-language.underscore | 2 + .../static/teams/templates/teams.underscore | 4 + .../teams/templates/teams_tab.underscore | 12 + .../teams/templates/teams/teams.html | 9 +- .../teams/tests/test_serializers.py | 151 ++++++++++ lms/djangoapps/teams/tests/test_views.py | 44 ++- lms/djangoapps/teams/urls.py | 3 +- lms/djangoapps/teams/views.py | 45 ++- lms/static/js/components/card/views/card.js | 3 +- .../js/components/tabbed/views/tabbed_view.js | 52 +++- .../js/spec/components/card/card_spec.js | 2 +- .../components/tabbed/tabbed_view_spec.js | 72 +++-- lms/static/js/spec/main.js | 2 + lms/static/sass/_developer.scss | 3 - lms/templates/components/card/card.underscore | 19 ++ .../components/tabbed/tab.underscore | 2 +- .../components/tabbed/tabbed_view.underscore | 8 +- 38 files changed, 1449 insertions(+), 314 deletions(-) create mode 100644 common/static/common/js/components/views/paginated_view.js create mode 100644 common/static/common/js/spec/components/paginated_view_spec.js create mode 100644 common/static/common/templates/components/paginated-view.underscore create mode 100644 lms/djangoapps/teams/static/teams/js/spec/teams_spec.js create mode 100644 lms/djangoapps/teams/static/teams/js/spec/teams_tab_spec.js create mode 100644 lms/djangoapps/teams/static/teams/js/views/team_card.js create mode 100644 lms/djangoapps/teams/static/teams/js/views/teams.js create mode 100644 lms/djangoapps/teams/static/teams/templates/team-country-language.underscore create mode 100644 lms/djangoapps/teams/static/teams/templates/teams.underscore create mode 100644 lms/djangoapps/teams/static/teams/templates/teams_tab.underscore create mode 100644 lms/djangoapps/teams/tests/test_serializers.py diff --git a/common/static/common/js/components/views/paginated_view.js b/common/static/common/js/components/views/paginated_view.js new file mode 100644 index 0000000000..b90213ab59 --- /dev/null +++ b/common/static/common/js/components/views/paginated_view.js @@ -0,0 +1,43 @@ +;(function(define) { + 'use strict'; + define([ + 'backbone', + 'underscore', + 'common/js/components/views/paging_header', + 'common/js/components/views/paging_footer', + 'common/js/components/views/list', + 'text!common/templates/components/paginated-view.underscore' + ], function (Backbone, _, PagingHeader, PagingFooter, ListView, paginatedViewTemplate) { + var PaginatedView = Backbone.View.extend({ + initialize: function () { + var ItemListView = ListView.extend({ + tagName: 'div', + className: this.type + '-container', + itemViewClass: this.itemViewClass + }); + this.listView = new ItemListView({collection: this.options.collection}); + this.headerView = this.headerView = new PagingHeader({collection: this.options.collection}); + this.footerView = new PagingFooter({ + collection: this.options.collection, hideWhenOnePage: true + }); + this.collection.on('page_changed', function () { + this.$('.sr-is-focusable.sr-' + this.type + '-view').focus(); + }, this); + }, + + render: function () { + this.$el.html(_.template(paginatedViewTemplate, {type: this.type})); + this.assign(this.listView, '.' + this.type + '-list'); + this.assign(this.headerView, '.' + this.type + '-paging-header'); + this.assign(this.footerView, '.' + this.type + '-paging-footer'); + return this; + }, + + assign: function (view, selector) { + view.setElement(this.$(selector)).render(); + } + }); + + return PaginatedView; + }); +}).call(this, define || RequireJS.define); diff --git a/common/static/common/js/components/views/paging_footer.js b/common/static/common/js/components/views/paging_footer.js index 08f3257434..36d2c74f2a 100644 --- a/common/static/common/js/components/views/paging_footer.js +++ b/common/static/common/js/components/views/paging_footer.js @@ -23,7 +23,8 @@ var onFirstPage = !this.collection.hasPreviousPage(), onLastPage = !this.collection.hasNextPage(); if (this.hideWhenOnePage) { - if (this.collection.totalPages <= 1) { + if (_.isUndefined(this.collection.totalPages) + || this.collection.totalPages <= 1) { this.$el.addClass('hidden'); } else if (this.$el.hasClass('hidden')) { this.$el.removeClass('hidden'); diff --git a/common/static/common/js/components/views/paging_header.js b/common/static/common/js/components/views/paging_header.js index 3489b01b0f..3bed5147b1 100644 --- a/common/static/common/js/components/views/paging_header.js +++ b/common/static/common/js/components/views/paging_header.js @@ -16,9 +16,9 @@ render: function () { var message, - start = this.collection.start, + start = _.isUndefined(this.collection.start) ? 0 : this.collection.start, end = start + this.collection.length, - num_items = this.collection.totalCount, + num_items = _.isUndefined(this.collection.totalCount) ? 0 : this.collection.totalCount, context = {first_index: Math.min(start + 1, end), last_index: end, num_items: num_items}; if (end <= 1) { message = interpolate(gettext('Showing %(first_index)s out of %(num_items)s total'), context, true); diff --git a/common/static/common/js/spec/components/paginated_view_spec.js b/common/static/common/js/spec/components/paginated_view_spec.js new file mode 100644 index 0000000000..4e04ede8a6 --- /dev/null +++ b/common/static/common/js/spec/components/paginated_view_spec.js @@ -0,0 +1,184 @@ +define([ + 'backbone', + 'underscore', + 'common/js/spec_helpers/ajax_helpers', + 'common/js/components/views/paginated_view', + 'common/js/components/collections/paging_collection' +], function (Backbone, _, AjaxHelpers, PaginatedView, PagingCollection) { + 'use strict'; + describe('PaginatedView', function () { + var TestItemView = Backbone.View.extend({ + className: 'test-item', + tagName: 'div', + initialize: function () { + this.render(); + }, + render: function () { + this.$el.text(this.model.get('text')); + return this; + } + }), + TestPaginatedView = PaginatedView.extend({type: 'test', itemViewClass: TestItemView}), + testCollection, + testView, + initialItems, + nextPageButtonCss = '.next-page-link', + previousPageButtonCss = '.previous-page-link', + generateItems = function (numItems) { + return _.map(_.range(numItems), function (i) { + return { + text: 'item ' + i + }; + }); + }; + + beforeEach(function () { + setFixtures('
'); + initialItems = generateItems(5); + testCollection = new PagingCollection({ + count: 6, + num_pages: 2, + current_page: 1, + start: 0, + results: initialItems + }, {parse: true}); + testView = new TestPaginatedView({el: '.test-container', collection: testCollection}).render(); + }); + + /** + * Verify that the view's header reflects the page we're currently viewing. + * @param matchString the header we expect to see + */ + function expectHeader(matchString) { + expect(testView.$('.test-paging-header').text()).toMatch(matchString); + } + + /** + * Verify that the list view renders the expected items + * @param expectedItems an array of topic objects we expect to see + */ + function expectItems(expectedItems) { + var $items = testView.$('.test-item'); + _.each(expectedItems, function (item, index) { + var currentItem = $items.eq(index); + expect(currentItem.text()).toMatch(item.text); + }); + } + + /** + * Verify that the footer reflects the current pagination + * @param options a parameters hash containing: + * - currentPage: the one-indexed page we expect to be viewing + * - totalPages: the total number of pages to page through + * - isHidden: whether the footer is expected to be visible + */ + function expectFooter(options) { + var footerEl = testView.$('.test-paging-footer'); + expect(footerEl.text()) + .toMatch(new RegExp(options.currentPage + '\\s+out of\\s+\/\\s+' + testCollection.totalPages)); + expect(footerEl.hasClass('hidden')).toBe(options.isHidden); + } + + it('can render the first of many pages', function () { + expectHeader('Showing 1-5 out of 6 total'); + expectItems(initialItems); + expectFooter({currentPage: 1, totalPages: 2, isHidden: false}); + }); + + it('can render the only page', function () { + initialItems = generateItems(1); + testCollection.set( + { + "count": 1, + "num_pages": 1, + "current_page": 1, + "start": 0, + "results": initialItems + }, + {parse: true} + ); + expectHeader('Showing 1 out of 1 total'); + expectItems(initialItems); + expectFooter({currentPage: 1, totalPages: 1, isHidden: true}); + }); + + it('can change to the next page', function () { + var requests = AjaxHelpers.requests(this), + newItems = generateItems(1); + expectHeader('Showing 1-5 out of 6 total'); + expectItems(initialItems); + expectFooter({currentPage: 1, totalPages: 2, isHidden: false}); + expect(requests.length).toBe(0); + testView.$(nextPageButtonCss).click(); + expect(requests.length).toBe(1); + AjaxHelpers.respondWithJson(requests, { + "count": 6, + "num_pages": 2, + "current_page": 2, + "start": 5, + "results": newItems + }); + expectHeader('Showing 6-6 out of 6 total'); + expectItems(newItems); + expectFooter({currentPage: 2, totalPages: 2, isHidden: false}); + }); + + it('can change to the previous page', function () { + var requests = AjaxHelpers.requests(this), + previousPageItems; + initialItems = generateItems(1); + testCollection.set( + { + "count": 6, + "num_pages": 2, + "current_page": 2, + "start": 5, + "results": initialItems + }, + {parse: true} + ); + expectHeader('Showing 6-6 out of 6 total'); + expectItems(initialItems); + expectFooter({currentPage: 2, totalPages: 2, isHidden: false}); + testView.$(previousPageButtonCss).click(); + previousPageItems = generateItems(5); + AjaxHelpers.respondWithJson(requests, { + "count": 6, + "num_pages": 2, + "current_page": 1, + "start": 0, + "results": previousPageItems + }); + expectHeader('Showing 1-5 out of 6 total'); + expectItems(previousPageItems); + expectFooter({currentPage: 1, totalPages: 2, isHidden: false}); + }); + + it('sets focus for screen readers', function () { + var requests = AjaxHelpers.requests(this); + spyOn($.fn, 'focus'); + testView.$(nextPageButtonCss).click(); + AjaxHelpers.respondWithJson(requests, { + "count": 6, + "num_pages": 2, + "current_page": 2, + "start": 5, + "results": generateItems(1) + }); + expect(testView.$('.sr-is-focusable').focus).toHaveBeenCalled(); + }); + + it('does not change on server error', function () { + var requests = AjaxHelpers.requests(this), + expectInitialState = function () { + expectHeader('Showing 1-5 out of 6 total'); + expectItems(initialItems); + expectFooter({currentPage: 1, totalPages: 2, isHidden: false}); + }; + expectInitialState(); + testView.$(nextPageButtonCss).click(); + requests[0].respond(500); + expectInitialState(); + }); + }); +}); diff --git a/common/static/common/templates/components/paginated-view.underscore b/common/static/common/templates/components/paginated-view.underscore new file mode 100644 index 0000000000..09ea0a13b4 --- /dev/null +++ b/common/static/common/templates/components/paginated-view.underscore @@ -0,0 +1,4 @@ +
+
+
+
diff --git a/common/static/js/spec/main_requirejs.js b/common/static/js/spec/main_requirejs.js index 42a204e74a..ac83443fe6 100644 --- a/common/static/js/spec/main_requirejs.js +++ b/common/static/js/spec/main_requirejs.js @@ -156,6 +156,7 @@ define([ // Run the common tests that use RequireJS. 'common-requirejs/include/common/js/spec/components/list_spec.js', + 'common-requirejs/include/common/js/spec/components/paginated_view_spec.js', 'common-requirejs/include/common/js/spec/components/paging_collection_spec.js', 'common-requirejs/include/common/js/spec/components/paging_header_spec.js', 'common-requirejs/include/common/js/spec/components/paging_footer_spec.js' diff --git a/common/test/acceptance/pages/lms/teams.py b/common/test/acceptance/pages/lms/teams.py index de6b81c215..ce224ebb9c 100644 --- a/common/test/acceptance/pages/lms/teams.py +++ b/common/test/acceptance/pages/lms/teams.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- """ -Teams page. +Teams pages. """ from .course_page import CoursePage @@ -9,6 +9,8 @@ from ..common.paging import PaginatedUIMixin TOPIC_CARD_CSS = 'div.wrapper-card-core' BROWSE_BUTTON_CSS = 'a.nav-item[data-index="1"]' +TEAMS_LINK_CSS = '.action-view' +TEAMS_HEADER_CSS = '.teams-header' class TeamsPage(CoursePage): @@ -53,3 +55,50 @@ class BrowseTopicsPage(CoursePage, PaginatedUIMixin): def topic_cards(self): """Return a list of the topic cards present on the page.""" return self.q(css=TOPIC_CARD_CSS).results + + def browse_teams_for_topic(self, topic_name): + """ + Show the teams list for `topic_name`. + """ + self.q(css=TEAMS_LINK_CSS).filter( + text='View Teams in the {topic_name} Topic'.format(topic_name=topic_name) + )[0].click() + self.wait_for_ajax() + + +class BrowseTeamsPage(CoursePage, PaginatedUIMixin): + """ + The paginated UI for browsing teams within a Topic on the Teams + page. + """ + def __init__(self, browser, course_id, topic): + """ + Set up `self.url_path` on instantiation, since it dynamically + reflects the current topic. Note that `topic` is a dict + representation of a topic following the same convention as a + course module's topic. + """ + super(BrowseTeamsPage, self).__init__(browser, course_id) + self.topic = topic + self.url_path = "teams/#topics/{topic_id}".format(topic_id=self.topic['id']) + + def is_browser_on_page(self): + """Check if we're on the teams list page for a particular topic.""" + has_correct_url = self.url.endswith(self.url_path) + teams_list_view_present = self.q(css='.teams-main').present + return has_correct_url and teams_list_view_present + + @property + def header_topic_name(self): + """Get the topic name displayed by the page header""" + return self.q(css=TEAMS_HEADER_CSS + ' .page-title')[0].text + + @property + def header_topic_description(self): + """Get the topic description displayed by the page header""" + return self.q(css=TEAMS_HEADER_CSS + ' .page-description')[0].text + + @property + def team_cards(self): + """Get all the team cards on the page.""" + return self.q(css='.team-card') diff --git a/common/test/acceptance/tests/lms/test_teams.py b/common/test/acceptance/tests/lms/test_teams.py index a212dd30b4..ae1f9917cf 100644 --- a/common/test/acceptance/tests/lms/test_teams.py +++ b/common/test/acceptance/tests/lms/test_teams.py @@ -1,23 +1,23 @@ """ Acceptance tests for the teams feature. """ -from ..helpers import UniqueCourseTest -from ...pages.lms.teams import TeamsPage, BrowseTopicsPage +import json + from nose.plugins.attrib import attr + +from ..helpers import UniqueCourseTest +from ...pages.lms.teams import TeamsPage, BrowseTopicsPage, BrowseTeamsPage +from ...fixtures import LMS_BASE_URL from ...fixtures.course import CourseFixture from ...pages.lms.tab_nav import TabNavPage from ...pages.lms.auto_auth import AutoAuthPage from ...pages.lms.course_info import CourseInfoPage -@attr('shard_5') -class TeamsTabTest(UniqueCourseTest): - """ - Tests verifying when the Teams tab is present. - """ - +class TeamsTabBase(UniqueCourseTest): + """Base class for Teams Tab tests""" def setUp(self): - super(TeamsTabTest, self).setUp() + super(TeamsTabBase, self).setUp() self.tab_nav = TabNavPage(self.browser) self.course_info_page = CourseInfoPage(self.browser, self.course_id) self.teams_page = TeamsPage(self.browser, self.course_id) @@ -39,7 +39,8 @@ class TeamsTabTest(UniqueCourseTest): self.course_fixture.install() enroll_course_id = self.course_id if enroll_in_course else None - AutoAuthPage(self.browser, course_id=enroll_course_id, staff=global_staff).visit() + #pylint: disable=attribute-defined-outside-init + self.user_info = AutoAuthPage(self.browser, course_id=enroll_course_id, staff=global_staff).visit().user_info self.course_info_page.visit() def verify_teams_present(self, present): @@ -54,6 +55,12 @@ class TeamsTabTest(UniqueCourseTest): else: self.assertNotIn("Teams", self.tab_nav.tab_names) + +@attr('shard_5') +class TeamsTabTest(TeamsTabBase): + """ + Tests verifying when the Teams tab is present. + """ def test_teams_not_enabled(self): """ Scenario: teams tab should not be present if no team configuration is set @@ -118,7 +125,7 @@ class TeamsTabTest(UniqueCourseTest): @attr('shard_5') -class BrowseTopicsTest(TeamsTabTest): +class BrowseTopicsTest(TeamsTabBase): """ Tests for the Browse tab of the Teams page. """ @@ -228,3 +235,223 @@ class BrowseTopicsTest(TeamsTabTest): self.assertLess(len(truncated_description), len(initial_description)) self.assertTrue(truncated_description.endswith('...')) self.assertIn(truncated_description.split('...')[0], initial_description) + + def test_go_to_teams_list(self): + """ + Scenario: Clicking on a Topic Card should take you to the + teams list for that Topic. + Given I am enrolled in a course with a team configuration and a topic + When I visit the Teams page + And I browse topics + And I click on the arrow link to view teams for the first topic + Then I should be on the browse teams page + """ + topic = {u"name": u"Example Topic", u"id": u"example_topic", u"description": "Description"} + self.set_team_configuration( + {u"max_team_size": 1, u"topics": [topic]} + ) + self.topics_page.visit() + self.topics_page.browse_teams_for_topic('Example Topic') + browse_teams_page = BrowseTeamsPage(self.browser, self.course_id, topic) + self.assertTrue(browse_teams_page.is_browser_on_page()) + self.assertEqual(browse_teams_page.header_topic_name, 'Example Topic') + self.assertEqual(browse_teams_page.header_topic_description, 'Description') + + +@attr('shard_5') +class BrowseTeamsWithinTopicTest(TeamsTabBase): + """ + Tests for browsing Teams within a Topic on the Teams page. + """ + TEAMS_PAGE_SIZE = 10 + + def setUp(self): + super(BrowseTeamsWithinTopicTest, 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.browse_teams_page = BrowseTeamsPage(self.browser, self.course_id, self.topic) + + 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']) + self.assertEqual(self.browse_teams_page.header_topic_description, self.topic['description']) + + def verify_teams(self, expected_teams): + """Verify that the list of team cards on the current page match the expected teams in order.""" + + def assert_team_equal(expected_team, team_card_name, team_card_description): + """ + Helper to assert that a single team card has the expected name and + description. + """ + self.assertEqual(expected_team['name'], team_card_name) + self.assertEqual(expected_team['description'], team_card_description) + + team_cards = self.browse_teams_page.team_cards + team_card_names = [ + team_card.find_element_by_css_selector('.card-title').text + for team_card in team_cards.results + ] + team_card_descriptions = [ + team_card.find_element_by_css_selector('.card-description').text + for team_card in team_cards.results + ] + map(assert_team_equal, expected_teams, team_card_names, team_card_descriptions) + + def verify_on_page(self, page_num, total_teams, pagination_header_text, footer_visible): + """ + Verify that we are on the correct team list page. + + Arguments: + page_num (int): The one-indexed page we expect to be on + total_teams (list): An unsorted list of all the teams for the + current topic + pagination_header_text (str): Text we expect to see in the + pagination header. + footer_visible (bool): Whether we expect to see the pagination + footer controls. + """ + alphabetized_teams = sorted(total_teams, key=lambda team: team['name']) + self.assertEqual(self.browse_teams_page.get_pagination_header_text(), pagination_header_text) + self.verify_teams(alphabetized_teams[(page_num - 1) * self.TEAMS_PAGE_SIZE:page_num * self.TEAMS_PAGE_SIZE]) + self.assertEqual( + self.browse_teams_page.pagination_controls_visible(), + footer_visible, + msg='Expected paging footer to be ' + 'visible' if footer_visible else 'invisible' + ) + + def test_no_teams(self): + """ + Scenario: Visiting a topic with no teams should not display any teams. + Given I am enrolled in a course with a team configuration and a topic + When I visit the Teams page for that topic + Then I should see the correct page header + And I should see a pagination header showing no teams + And I should see no teams + And I should see a button to add a team + And I should not see a pagination footer + """ + self.browse_teams_page.visit() + self.verify_page_header() + self.assertEqual(self.browse_teams_page.get_pagination_header_text(), 'Showing 0 out of 0 total') + self.assertEqual(len(self.browse_teams_page.team_cards), 0, msg='Expected to see no team cards') + self.assertFalse( + self.browse_teams_page.pagination_controls_visible(), + msg='Expected paging footer to be invisible' + ) + + def test_teams_one_page(self): + """ + Scenario: Visiting a topic with fewer teams than the page size should + all those teams on one page. + Given I am enrolled in a course with a team configuration and a topic + When I visit the Teams page for that topic + Then I should see the correct page header + And I should see a pagination header showing the number of teams + And I should see all the expected team cards + 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) + 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') + self.verify_teams(teams) + self.assertFalse( + self.browse_teams_page.pagination_controls_visible(), + msg='Expected paging footer to be invisible' + ) + + def test_teams_navigation_buttons(self): + """ + Scenario: The user should be able to page through a topic's team list + using navigation buttons when it is longer than the page size. + Given I am enrolled in a course with a team configuration and a topic + When I visit the Teams page for that topic + Then I should see the correct page header + And I should see that I am on the first page of results + When I click on the next page button + Then I should see that I am on the second page of results + 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) + self.browse_teams_page.visit() + self.verify_page_header() + self.verify_on_page(1, teams, 'Showing 1-10 out of 11 total', True) + self.browse_teams_page.press_next_page_button() + self.verify_on_page(2, teams, 'Showing 11-11 out of 11 total', True) + self.browse_teams_page.press_previous_page_button() + self.verify_on_page(1, teams, 'Showing 1-10 out of 11 total', True) + + def test_teams_page_input(self): + """ + Scenario: The user should be able to page through a topic's team list + using the page input when it is longer than the page size. + Given I am enrolled in a course with a team configuration and a topic + When I visit the Teams page for that topic + Then I should see the correct page header + And I should see that I am on the first page of results + When I input the second page + Then I should see that I am on the second page of results + 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) + self.browse_teams_page.visit() + self.verify_page_header() + self.verify_on_page(1, teams, 'Showing 1-10 out of 20 total', True) + self.browse_teams_page.go_to_page(2) + self.verify_on_page(2, teams, 'Showing 11-20 out of 20 total', True) + self.browse_teams_page.go_to_page(1) + self.verify_on_page(1, teams, 'Showing 1-10 out of 20 total', True) + + def test_teams_membership(self): + """ + Scenario: Team cards correctly reflect membership of the team. + Given I am enrolled in a course with a team configuration and a topic + containing one team + And I add myself to the team + When I visit the Teams page for that topic + Then I should see the correct page header + 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) + self.browse_teams_page.visit() + self.verify_page_header() + self.verify_teams(teams) + self.create_membership(self.user_info['username'], teams[0]['id']) + self.browser.refresh() + self.browse_teams_page.wait_for_ajax() + self.assertEqual( + self.browse_teams_page.team_cards[0].find_element_by_css_selector('.member-count').text, + '1 / 10 Members' + ) diff --git a/lms/djangoapps/teams/api_urls.py b/lms/djangoapps/teams/api_urls.py index ab7556505f..f27f0af8af 100644 --- a/lms/djangoapps/teams/api_urls.py +++ b/lms/djangoapps/teams/api_urls.py @@ -39,7 +39,7 @@ urlpatterns = patterns( name="topics_detail" ), url( - r'^v0/team_membership$', + r'^v0/team_membership/$', MembershipListView.as_view(), name="team_membership_list" ), diff --git a/lms/djangoapps/teams/serializers.py b/lms/djangoapps/teams/serializers.py index 90d12f86de..486d6914c7 100644 --- a/lms/djangoapps/teams/serializers.py +++ b/lms/djangoapps/teams/serializers.py @@ -1,12 +1,16 @@ """Defines serializers used by the Team API.""" from django.contrib.auth.models import User +from django.db.models import Count + from rest_framework import serializers -from openedx.core.lib.api.serializers import CollapsedReferenceSerializer + +from openedx.core.lib.api.serializers import CollapsedReferenceSerializer, PaginationSerializer from openedx.core.lib.api.fields import ExpandableField -from .models import CourseTeam, CourseTeamMembership from openedx.core.djangoapps.user_api.serializers import UserSerializer +from .models import CourseTeam, CourseTeamMembership + class UserMembershipSerializer(serializers.ModelSerializer): """Serializes CourseTeamMemberships with only user and date_joined @@ -108,8 +112,43 @@ class MembershipSerializer(serializers.ModelSerializer): read_only_fields = ("date_joined",) -class TopicSerializer(serializers.Serializer): - """Serializes a topic.""" +class BaseTopicSerializer(serializers.Serializer): + """Serializes a topic without team_count.""" description = serializers.CharField() name = serializers.CharField() id = serializers.CharField() # pylint: disable=invalid-name + + +class TopicSerializer(BaseTopicSerializer): + """ + Adds team_count to the basic topic serializer. Use only when + serializing a single topic. When serializing many topics, use + `PaginatedTopicSerializer` to avoid O(N) SQL queries. + """ + team_count = serializers.SerializerMethodField('get_team_count') + + def get_team_count(self, topic): + """Get the number of teams associated with this topic""" + return CourseTeam.objects.filter(topic_id=topic['id']).count() + + +class PaginatedTopicSerializer(PaginationSerializer): + """Serializes a set of topics. Adds team_count field to each topic.""" + class Meta(object): + """Defines meta information for the PaginatedTopicSerializer.""" + object_serializer_class = BaseTopicSerializer + + def __init__(self, *args, **kwargs): + """Adds team_count to each topic.""" + super(PaginatedTopicSerializer, self).__init__(*args, **kwargs) + + # The following query gets all the team_counts for each topic + # and outputs the result as a list of dicts (one per topic). + topic_ids = [topic['id'] for topic in self.data['results']] + teams_per_topic = CourseTeam.objects.filter( + topic_id__in=topic_ids + ).values('topic_id').annotate(team_count=Count('topic_id')) + + topics_to_team_count = {d['topic_id']: d['team_count'] for d in teams_per_topic} + for topic in self.data['results']: + topic['team_count'] = topics_to_team_count.get(topic['id'], 0) diff --git a/lms/djangoapps/teams/static/teams/js/collections/team.js b/lms/djangoapps/teams/static/teams/js/collections/team.js index a07190be5b..2033d7e4f7 100644 --- a/lms/djangoapps/teams/static/teams/js/collections/team.js +++ b/lms/djangoapps/teams/static/teams/js/collections/team.js @@ -3,12 +3,14 @@ define(['common/js/components/collections/paging_collection', 'teams/js/models/team', 'gettext'], function(PagingCollection, TeamModel, gettext) { var TeamCollection = PagingCollection.extend({ - initialize: function(options) { + initialize: function(teams, options) { PagingCollection.prototype.initialize.call(this); this.course_id = options.course_id; + this.server_api['topic_id'] = this.topic_id = options.topic_id; + this.perPage = options.per_page; this.server_api['course_id'] = function () { return encodeURIComponent(this.course_id); }; - this.server_api['order_by'] = function () { return this.sortField; }; + this.server_api['order_by'] = function () { return 'name'; }; // TODO surface sort order in UI delete this.server_api['sort_order']; // Sort order is not specified for the Team API this.registerSortableField('name', gettext('name')); 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 eb5709354a..c81302167d 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 @@ -7,7 +7,13 @@ define(["jquery", "backbone", "teams/js/teams_tab_factory"], beforeEach(function() { setFixtures('
'); - teamsTab = new TeamsTabFactory($(".teams-content"), {results: []}, '', 'edX/DemoX/Demo_Course'); + teamsTab = new TeamsTabFactory({ + topics: {results: []}, + topics_url: '', + teams_url: '', + maxTeamSize: 9999 + course_id: 'edX/DemoX/Demo_Course' + }); }); afterEach(function() { @@ -19,7 +25,7 @@ define(["jquery", "backbone", "teams/js/teams_tab_factory"], }); it("displays a header", function() { - expect($("body").html()).toContain("Course teams are organized"); + expect($("body").html()).toContain("See all teams in your course, organized by topic"); }); }); } diff --git a/lms/djangoapps/teams/static/teams/js/spec/teams_spec.js b/lms/djangoapps/teams/static/teams/js/spec/teams_spec.js new file mode 100644 index 0000000000..355de9074a --- /dev/null +++ b/lms/djangoapps/teams/static/teams/js/spec/teams_spec.js @@ -0,0 +1,53 @@ +define([ + 'teams/js/collections/team', 'teams/js/views/teams' +], function (TeamCollection, TeamsView) { + 'use strict'; + describe('TeamsView', function () { + var teamsView, teamCollection, initialTeams, + createTeams = function (startIndex, stopIndex) { + return _.map(_.range(startIndex, stopIndex + 1), function (i) { + return { + name: "team " + i, + id: "id " + i, + language: "English", + country: "Sealand", + is_active: true, + membership: [] + }; + }); + }; + + beforeEach(function () { + setFixtures('
'); + initialTeams = createTeams(1, 5); + teamCollection = new TeamCollection( + { + count: 6, + num_pages: 2, + current_page: 1, + start: 0, + results: initialTeams + }, + {course_id: 'my/course/id', parse: true} + ); + teamsView = new TeamsView({ + el: '.teams-container', + collection: teamCollection + }).render(); + }); + + it('can render itself', function () { + var footerEl = teamsView.$('.teams-paging-footer'), + teamCards = teamsView.$('.team-card'); + expect(teamsView.$('.teams-paging-header').text()).toMatch('Showing 1-5 out of 6 total'); + _.each(initialTeams, function (team, index) { + var currentCard = teamCards.eq(index); + expect(currentCard.text()).toMatch(team.name); + expect(currentCard.text()).toMatch(team.language); + expect(currentCard.text()).toMatch(team.country); + }); + expect(footerEl.text()).toMatch('1\\s+out of\\s+\/\\s+2'); + expect(footerEl).not.toHaveClass('hidden'); + }); + }); +}); 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 new file mode 100644 index 0000000000..55cf4e7160 --- /dev/null +++ b/lms/djangoapps/teams/static/teams/js/spec/teams_tab_spec.js @@ -0,0 +1,74 @@ +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); + }; + + 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(); + }); + + 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 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.'); + }); + + it('displays 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.'); + }); + }); +}); diff --git a/lms/djangoapps/teams/static/teams/js/spec/topics_spec.js b/lms/djangoapps/teams/static/teams/js/spec/topics_spec.js index 34b3017372..3a4bade0aa 100644 --- a/lms/djangoapps/teams/static/teams/js/spec/topics_spec.js +++ b/lms/djangoapps/teams/static/teams/js/spec/topics_spec.js @@ -1,13 +1,10 @@ define([ - 'common/js/spec_helpers/ajax_helpers', 'teams/js/collections/topic', 'teams/js/views/topics' -], function (AjaxHelpers, TopicCollection, TopicsView) { + 'teams/js/collections/topic', 'teams/js/views/topics' +], function (TopicCollection, TopicsView) { 'use strict'; describe('TopicsView', function () { - var initialTopics, topicCollection, topicsView, nextPageButtonCss; - - nextPageButtonCss = '.next-page-link'; - - function generateTopics(startIndex, stopIndex) { + var initialTopics, topicCollection, topicsView, + generateTopics = function (startIndex, stopIndex) { return _.map(_.range(startIndex, stopIndex + 1), function (i) { return { "description": "description " + i, @@ -16,7 +13,7 @@ define([ "team_count": 0 }; }); - } + }; beforeEach(function () { setFixtures('
'); @@ -34,143 +31,18 @@ define([ topicsView = new TopicsView({el: '.topics-container', collection: topicCollection}).render(); }); - /** - * Verify that the topics view's header reflects the page we're currently viewing. - * @param matchString the header we expect to see - */ - function expectHeader(matchString) { - expect(topicsView.$('.topics-paging-header').text()).toMatch(matchString); - } - - /** - * Verify that the topics list view renders the expected topics - * @param expectedTopics an array of topic objects we expect to see - */ - function expectTopics(expectedTopics) { - var topicCards; - topicCards = topicsView.$('.topic-card'); - _.each(expectedTopics, function (topic, index) { + it('can render the first of many pages', function () { + var footerEl = topicsView.$('.topics-paging-footer'), + topicCards = topicsView.$('.topic-card'); + expect(topicsView.$('.topics-paging-header').text()).toMatch('Showing 1-5 out of 6 total'); + _.each(initialTopics, function (topic, index) { var currentCard = topicCards.eq(index); expect(currentCard.text()).toMatch(topic.name); expect(currentCard.text()).toMatch(topic.description); expect(currentCard.text()).toMatch(topic.team_count + ' Teams'); }); - } - - /** - * Verify that the topics footer reflects the current pagination - * @param options a parameters hash containing: - * - currentPage: the one-indexed page we expect to be viewing - * - totalPages: the total number of pages to page through - * - isHidden: whether the footer is expected to be visible - */ - function expectFooter(options) { - var footerEl = topicsView.$('.topics-paging-footer'); - expect(footerEl.text()) - .toMatch(new RegExp(options.currentPage + '\\s+out of\\s+\/\\s+' + topicCollection.totalPages)); - expect(footerEl.hasClass('hidden')).toBe(options.isHidden); - } - - it('can render the first of many pages', function () { - expectHeader('Showing 1-5 out of 6 total'); - expectTopics(initialTopics); - expectFooter({currentPage: 1, totalPages: 2, isHidden: false}); - }); - - it('can render the only page', function () { - initialTopics = generateTopics(1, 1); - topicCollection.set( - { - "count": 1, - "num_pages": 1, - "current_page": 1, - "start": 0, - "results": initialTopics - }, - {parse: true} - ); - expectHeader('Showing 1 out of 1 total'); - expectTopics(initialTopics); - expectFooter({currentPage: 1, totalPages: 1, isHidden: true}); - }); - - it('can change to the next page', function () { - var requests = AjaxHelpers.requests(this), - newTopics = generateTopics(1, 1); - expectHeader('Showing 1-5 out of 6 total'); - expectTopics(initialTopics); - expectFooter({currentPage: 1, totalPages: 2, isHidden: false}); - expect(requests.length).toBe(0); - topicsView.$(nextPageButtonCss).click(); - expect(requests.length).toBe(1); - AjaxHelpers.respondWithJson(requests, { - "count": 6, - "num_pages": 2, - "current_page": 2, - "start": 5, - "results": newTopics - }); - expectHeader('Showing 6-6 out of 6 total'); - expectTopics(newTopics); - expectFooter({currentPage: 2, totalPages: 2, isHidden: false}); - }); - - it('can change to the previous page', function () { - var requests = AjaxHelpers.requests(this), - previousPageTopics; - initialTopics = generateTopics(1, 1); - topicCollection.set( - { - "count": 6, - "num_pages": 2, - "current_page": 2, - "start": 5, - "results": initialTopics - }, - {parse: true} - ); - expectHeader('Showing 6-6 out of 6 total'); - expectTopics(initialTopics); - expectFooter({currentPage: 2, totalPages: 2, isHidden: false}); - topicsView.$('.previous-page-link').click(); - previousPageTopics = generateTopics(1, 5); - AjaxHelpers.respondWithJson(requests, { - "count": 6, - "num_pages": 2, - "current_page": 1, - "start": 0, - "results": previousPageTopics - }); - expectHeader('Showing 1-5 out of 6 total'); - expectTopics(previousPageTopics); - expectFooter({currentPage: 1, totalPages: 2, isHidden: false}); - }); - - it('sets focus for screen readers', function () { - var requests = AjaxHelpers.requests(this); - spyOn($.fn, 'focus'); - topicsView.$(nextPageButtonCss).click(); - AjaxHelpers.respondWithJson(requests, { - "count": 6, - "num_pages": 2, - "current_page": 2, - "start": 5, - "results": generateTopics(1, 1) - }); - expect(topicsView.$('.sr-is-focusable').focus).toHaveBeenCalled(); - }); - - it('does not change on server error', function () { - var requests = AjaxHelpers.requests(this), - expectInitialState = function () { - expectHeader('Showing 1-5 out of 6 total'); - expectTopics(initialTopics); - expectFooter({currentPage: 1, totalPages: 2, isHidden: false}); - }; - expectInitialState(); - topicsView.$(nextPageButtonCss).click(); - requests[0].respond(500); - expectInitialState(); + expect(footerEl.text()).toMatch('1\\s+out of\\s+\/\\s+2'); + expect(footerEl).not.toHaveClass('hidden'); }); }); }); diff --git a/lms/djangoapps/teams/static/teams/js/teams_tab_factory.js b/lms/djangoapps/teams/static/teams/js/teams_tab_factory.js index 8a0a6cc64a..4d66791542 100644 --- a/lms/djangoapps/teams/static/teams/js/teams_tab_factory.js +++ b/lms/djangoapps/teams/static/teams/js/teams_tab_factory.js @@ -1,16 +1,12 @@ ;(function (define) { 'use strict'; - define(['jquery', 'teams/js/views/teams_tab', 'teams/js/collections/topic'], - function ($, TeamsTabView, TopicCollection) { - return function (element, topics, topics_url, course_id) { - var topicCollection = new TopicCollection(topics, {url: topics_url, course_id: course_id, parse: true}); - topicCollection.bootstrap(); - var view = new TeamsTabView({ - el: element, - topicCollection: topicCollection - }); - view.render(); + define(['jquery', 'teams/js/views/teams_tab'], + function ($, TeamsTabView) { + return function (options) { + var teamsTab = new TeamsTabView(_.extend(options, {el: $('.teams-content')})); + teamsTab.render(); + Backbone.history.start(); }; }); }).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 new file mode 100644 index 0000000000..cdfbeda5cf --- /dev/null +++ b/lms/djangoapps/teams/static/teams/js/views/team_card.js @@ -0,0 +1,85 @@ +;(function (define) { + 'use strict'; + define([ + 'backbone', + 'underscore', + 'gettext', + 'js/components/card/views/card', + 'text!teams/templates/team-country-language.underscore' + ], function (Backbone, _, gettext, CardView, teamCountryLanguageTemplate) { + var TeamMembershipView, TeamCountryLanguageView, TeamCardView; + + TeamMembershipView = Backbone.View.extend({ + tagName: 'div', + className: 'team-members', + template: _.template( + '<%= membership_message %>' + + '
    ' + ), + + initialize: function (options) { + this.maxTeamSize = options.maxTeamSize; + }, + + render: function () { + var memberships = this.model.get('membership'), + maxMemberCount = this.maxTeamSize; + this.$el.html(this.template({ + membership_message: interpolate( + // Translators: The following message displays the number of members on a team. + ngettext( + '%(member_count)s / %(max_member_count)s Member', + '%(member_count)s / %(max_member_count)s Members', + maxMemberCount + ), + {member_count: memberships.length, max_member_count: maxMemberCount}, true + ) + })); + _.each(memberships, function (membership) { + this.$('list-member-thumbs').append( + '
  • ' + membership.user.username + '
    • ' + ); + }, this); + return this; + } + }); + + TeamCountryLanguageView = Backbone.View.extend({ + template: _.template(teamCountryLanguageTemplate), + + render: function() { + // this.$el should be the card meta div + this.$el.append(this.template({ + country: this.model.get('country'), + language: this.model.get('language') + })); + } + }); + + TeamCardView = CardView.extend({ + initialize: function () { + CardView.prototype.initialize.apply(this, arguments); + // TODO: show last activity detail view + this.detailViews = [ + new TeamMembershipView({model: this.model, maxTeamSize: this.maxTeamSize}), + new TeamCountryLanguageView({model: this.model}) + ]; + }, + + configuration: 'list_card', + cardClass: 'team-card', + title: function () { return this.model.get('name'); }, + description: function () { return this.model.get('description'); }, + details: function () { return this.detailViews; }, + actionClass: 'action-view', + actionContent: function() { + return interpolate( + gettext('View %(span_start)s %(team_name)s %(span_end)s'), + {span_start: '', team_name: this.model.get('name'), span_end: ''}, + true + ); + } + }); + return TeamCardView; + }); +}).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 new file mode 100644 index 0000000000..7d0d5a9670 --- /dev/null +++ b/lms/djangoapps/teams/static/teams/js/views/teams.js @@ -0,0 +1,33 @@ +;(function (define) { + 'use strict'; + define([ + 'teams/js/views/team_card', + 'common/js/components/views/paginated_view' + ], function (TeamCardView, PaginatedView) { + var TeamsView = PaginatedView.extend({ + type: 'teams', + + events: { + 'click button.action': '' // entry point for team creation + }, + + initialize: function (options) { + this.itemViewClass = TeamCardView.extend({ + router: options.router, + maxTeamSize: options.maxTeamSize + }); + PaginatedView.prototype.initialize.call(this); + }, + + render: function () { + PaginatedView.prototype.render.call(this); + + this.$el.append( + $('') + ); + return this; + } + }); + return TeamsView; + }); +}).call(this, define || RequireJS.define); diff --git a/lms/djangoapps/teams/static/teams/js/views/teams_tab.js b/lms/djangoapps/teams/static/teams/js/views/teams_tab.js index 8784f389b0..d320677425 100644 --- a/lms/djangoapps/teams/static/teams/js/views/teams_tab.js +++ b/lms/djangoapps/teams/static/teams/js/views/teams_tab.js @@ -7,19 +7,52 @@ 'js/components/header/views/header', 'js/components/header/models/header', 'js/components/tabbed/views/tabbed_view', - 'teams/js/views/topics'], - function (Backbone, _, gettext, HeaderView, HeaderModel, TabbedView, TopicsView) { + 'teams/js/views/topics', + 'teams/js/models/topic', + 'teams/js/collections/topic', + 'teams/js/views/teams', + 'teams/js/collections/team', + 'text!teams/templates/teams_tab.underscore'], + function (Backbone, _, gettext, HeaderView, HeaderModel, TabbedView, + TopicsView, TopicModel, TopicCollection, TeamsView, TeamCollection, 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; + } + }); + var TeamTabView = Backbone.View.extend({ initialize: function(options) { - this.headerModel = new HeaderModel({ - description: gettext("Course teams are organized into topics created by course instructors. Try to join others in an existing team before you decide to create a new team!"), - title: gettext("Teams") - }); - this.headerView = new HeaderView({ - model: this.headerModel + 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 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)], + [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! - var TempTabView = Backbone.View.extend({ + TempTabView = Backbone.View.extend({ initialize: function (options) { this.text = options.text; }, @@ -28,27 +61,204 @@ this.$el.text(this.text); } }); - this.tabbedView = 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: options.topicCollection + 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 + }) }); - Backbone.history.start(); }, render: function() { - this.$el.append(this.headerView.$el); - this.headerView.render(); - this.$el.append(this.tabbedView.$el); - this.tabbedView.render(); + 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.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. + 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) + ); + }, + + /** + * 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 + }), + maxTeamSize: this.maxTeamSize + }) + }); + }, + + /** + * 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(); + } + }, + + /** + * Immediately return a promise for the given + * object. + */ + identityPromise: function (obj) { + return new $.Deferred().resolve(obj).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); + }, + + // Error handling + + routeNotFound: function (route) { + this.notFoundError( + interpolate( + gettext('The page "%(route)s" could not be found.'), + {route: route}, + true + ) + ); + }, + + topicNotFound: function (topicID) { + this.notFoundError( + interpolate( + gettext('The topic "%(topic)s" could not be found.'), + {topic: topicID}, + 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); + }, + + showWarning: function (message) { + var warningEl = this.$('.warning'); + warningEl.find('.copy').html('

    ' + message + '' + country + '

    '); } %> +<% if (language) { print('

    ' + language + '

    '); } %> diff --git a/lms/djangoapps/teams/static/teams/templates/teams.underscore b/lms/djangoapps/teams/static/teams/templates/teams.underscore new file mode 100644 index 0000000000..e0a7a180c2 --- /dev/null +++ b/lms/djangoapps/teams/static/teams/templates/teams.underscore @@ -0,0 +1,4 @@ +
    +
    +
    +
    diff --git a/lms/djangoapps/teams/static/teams/templates/teams_tab.underscore b/lms/djangoapps/teams/static/teams/templates/teams_tab.underscore new file mode 100644 index 0000000000..239eb2b443 --- /dev/null +++ b/lms/djangoapps/teams/static/teams/templates/teams_tab.underscore @@ -0,0 +1,12 @@ + +
    +
    +
    +
    diff --git a/lms/djangoapps/teams/templates/teams/teams.html b/lms/djangoapps/teams/templates/teams/teams.html index 912d3d5b68..b53ce4a1dd 100644 --- a/lms/djangoapps/teams/templates/teams/teams.html +++ b/lms/djangoapps/teams/templates/teams/teams.html @@ -22,6 +22,13 @@ <%block name="js_extra"> <%static:require_module module_name="teams/js/teams_tab_factory" class_name="TeamsTabFactory"> - TeamsTabFactory($('.teams-content'), ${ json.dumps(topics, cls=EscapedEdxJSONEncoder) }, '${ topics_url }', '${ unicode(course.id) }'); + new TeamsTabFactory({ + topics: ${ json.dumps(topics, cls=EscapedEdxJSONEncoder) }, + topic_url: '${ topic_url }', + topics_url: '${ topics_url }', + teams_url: '${ teams_url }', + maxTeamSize: ${ course.teams_max_size }, + course_id: '${ unicode(course.id) }' + }); diff --git a/lms/djangoapps/teams/tests/test_serializers.py b/lms/djangoapps/teams/tests/test_serializers.py new file mode 100644 index 0000000000..56c3c82cb3 --- /dev/null +++ b/lms/djangoapps/teams/tests/test_serializers.py @@ -0,0 +1,151 @@ +# -*- coding: utf-8 -*- +""" +Tests for custom Teams Serializers. +""" +from django.core.paginator import Paginator + +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + +from lms.djangoapps.teams.tests.factories import CourseTeamFactory +from lms.djangoapps.teams.serializers import BaseTopicSerializer, PaginatedTopicSerializer, TopicSerializer + + +class TopicTestCase(ModuleStoreTestCase): + """ + Base test class to set up a course with topics + """ + def setUp(self): + """ + Set up a course with a teams configuration. + """ + super(TopicTestCase, self).setUp() + self.course = CourseFactory.create( + teams_configuration={ + "max_team_size": 10, + "topics": [{u'name': u'Tøpic', u'description': u'The bést topic!', u'id': u'0'}] + } + ) + + +class BaseTopicSerializerTestCase(TopicTestCase): + """ + Tests for the `BaseTopicSerializer`, which should not serialize team count + data. + """ + def test_team_count_not_included(self): + """Verifies that the `BaseTopicSerializer` does not include team count""" + with self.assertNumQueries(0): + serializer = BaseTopicSerializer(self.course.teams_topics[0]) + self.assertEqual( + serializer.data, + {u'name': u'Tøpic', u'description': u'The bést topic!', u'id': u'0'} + ) + + +class TopicSerializerTestCase(TopicTestCase): + """ + Tests for the `TopicSerializer`, which should serialize team count data for + a single topic. + """ + def test_topic_with_no_team_count(self): + """ + Verifies that the `TopicSerializer` correctly displays a topic with a + team count of 0, and that it only takes one SQL query. + """ + with self.assertNumQueries(1): + serializer = TopicSerializer(self.course.teams_topics[0]) + self.assertEqual( + serializer.data, + {u'name': u'Tøpic', u'description': u'The bést topic!', u'id': u'0', u'team_count': 0} + ) + + def test_topic_with_team_count(self): + """ + Verifies that the `TopicSerializer` correctly displays a topic with a + positive team count, and that it only takes one SQL query. + """ + CourseTeamFactory.create(course_id=self.course.id, topic_id=self.course.teams_topics[0]['id']) + with self.assertNumQueries(1): + serializer = TopicSerializer(self.course.teams_topics[0]) + self.assertEqual( + serializer.data, + {u'name': u'Tøpic', u'description': u'The bést topic!', u'id': u'0', u'team_count': 1} + ) + + +class PaginatedTopicSerializerTestCase(TopicTestCase): + """ + Tests for the `PaginatedTopicSerializer`, which should serialize team count + data for many topics with constant time SQL queries. + """ + PAGE_SIZE = 5 + + def _merge_dicts(self, first, second): + """Convenience method to merge two dicts in a single expression""" + result = first.copy() + result.update(second) + return result + + def setup_topics(self, num_topics=5, teams_per_topic=0): + """ + Helper method to set up topics on the course. Returns a list of + created topics. + """ + self.course.teams_configuration['topics'] = [] + topics = [ + {u'name': u'Tøpic {}'.format(i), u'description': u'The bést topic! {}'.format(i), u'id': unicode(i)} + for i in xrange(num_topics) + ] + for i in xrange(num_topics): + topic_id = unicode(i) + self.course.teams_configuration['topics'].append(topics[i]) + for _ in xrange(teams_per_topic): + CourseTeamFactory.create(course_id=self.course.id, topic_id=topic_id) + return topics + + def assert_serializer_output(self, topics, num_teams_per_topic, num_queries): + """ + Verify that the serializer produced the expected topics. + """ + with self.assertNumQueries(num_queries): + page = Paginator(self.course.teams_topics, self.PAGE_SIZE).page(1) + serializer = PaginatedTopicSerializer(instance=page) + self.assertEqual( + serializer.data['results'], + [self._merge_dicts(topic, {u'team_count': num_teams_per_topic}) for topic in topics] + ) + + def test_no_topics(self): + """ + Verify that we return no results and make no SQL queries for a page + with no topics. + """ + self.course.teams_configuration['topics'] = [] + self.assert_serializer_output([], num_teams_per_topic=0, num_queries=0) + + def test_topics_with_no_team_counts(self): + """ + Verify that we serialize topics with no team count, making only one SQL + query. + """ + topics = self.setup_topics(teams_per_topic=0) + self.assert_serializer_output(topics, num_teams_per_topic=0, num_queries=1) + + def test_topics_with_team_counts(self): + """ + Verify that we serialize topics with a positive team count, making only + one SQL query. + """ + teams_per_topic = 10 + topics = self.setup_topics(teams_per_topic=teams_per_topic) + self.assert_serializer_output(topics, num_teams_per_topic=teams_per_topic, num_queries=1) + + def test_subset_of_topics(self): + """ + Verify that we serialize a subset of the course's topics, making only + one SQL query. + """ + teams_per_topic = 10 + topics = self.setup_topics(num_topics=self.PAGE_SIZE + 1, teams_per_topic=teams_per_topic) + self.assert_serializer_output(topics[:self.PAGE_SIZE], num_teams_per_topic=teams_per_topic, num_queries=1) diff --git a/lms/djangoapps/teams/tests/test_views.py b/lms/djangoapps/teams/tests/test_views.py index 15e0c1e0ff..177c558d4c 100644 --- a/lms/djangoapps/teams/tests/test_views.py +++ b/lms/djangoapps/teams/tests/test_views.py @@ -6,6 +6,7 @@ import json import ddt from django.core.urlresolvers import reverse +from django.conf import settings from nose.plugins.attrib import attr from rest_framework.test import APITestCase, APIClient @@ -35,13 +36,16 @@ class TestDashboard(ModuleStoreTestCase): self.teams_url = reverse('teams_dashboard', args=[self.course.id]) def test_anonymous(self): - """ Verifies that an anonymous client cannot access the team dashboard. """ + """Verifies that an anonymous client cannot access the team + dashboard, and is redirected to the login page.""" anonymous_client = APIClient() response = anonymous_client.get(self.teams_url) - self.assertEqual(404, response.status_code) + redirect_url = '{0}?next={1}'.format(settings.LOGIN_URL, self.teams_url) + self.assertRedirects(response, redirect_url) def test_not_enrolled_not_staff(self): """ Verifies that a student who is not enrolled cannot access the team dashboard. """ + self.client.login(username=self.user.username, password=self.test_password) response = self.client.get(self.teams_url) self.assertEqual(404, response.status_code) @@ -82,6 +86,8 @@ class TestDashboard(ModuleStoreTestCase): """ bad_org = "badorgxxx" bad_team_url = self.teams_url.replace(self.course.id.org, bad_org) + CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id) + self.client.login(username=self.user.username, password=self.test_password) response = self.client.get(bad_team_url) self.assertEqual(404, response.status_code) @@ -134,12 +140,12 @@ class TeamAPITestCase(APITestCase, ModuleStoreTestCase): self.test_team_1 = CourseTeamFactory.create( name=u'sólar team', course_id=self.test_course_1.id, - topic_id='renewable' + topic_id='topic_0' ) self.test_team_2 = CourseTeamFactory.create(name='Wind Team', course_id=self.test_course_1.id) self.test_team_3 = CourseTeamFactory.create(name='Nuclear Team', course_id=self.test_course_1.id) self.test_team_4 = CourseTeamFactory.create(name='Coal Team', course_id=self.test_course_1.id, is_active=False) - self.test_team_4 = CourseTeamFactory.create(name='Another Team', course_id=self.test_course_2.id) + self.test_team_5 = CourseTeamFactory.create(name='Another Team', course_id=self.test_course_2.id) for user, course in [ ('student_enrolled', self.test_course_1), @@ -153,7 +159,7 @@ class TeamAPITestCase(APITestCase, ModuleStoreTestCase): self.test_team_1.add_user(self.users['student_enrolled']) self.test_team_3.add_user(self.users['student_enrolled_both_courses_other_team']) - self.test_team_4.add_user(self.users['student_enrolled_both_courses_other_team']) + self.test_team_5.add_user(self.users['student_enrolled_both_courses_other_team']) def login(self, user): """Given a user string, logs the given user in. @@ -312,7 +318,7 @@ class TestListTeamsAPI(TeamAPITestCase): self.verify_names({'course_id': self.test_course_2.id}, 200, ['Another Team'], user='staff') def test_filter_topic_id(self): - self.verify_names({'course_id': self.test_course_1.id, 'topic_id': 'renewable'}, 200, [u'sólar team']) + self.verify_names({'course_id': self.test_course_1.id, 'topic_id': 'topic_0'}, 200, [u'sólar team']) def test_filter_include_inactive(self): self.verify_names({'include_inactive': True}, 200, ['Coal Team', 'Nuclear Team', u'sólar team', 'Wind Team']) @@ -333,9 +339,10 @@ class TestListTeamsAPI(TeamAPITestCase): data = {'order_by': field} if field else {} self.verify_names(data, status, names) - @ddt.data({'course_id': 'no/such/course'}, {'topic_id': 'no_such_topic'}) - def test_no_results(self, data): - self.get_teams_list(404, data) + @ddt.data((404, {'course_id': 'no/such/course'}), (400, {'topic_id': 'no_such_topic'})) + @ddt.unpack + def test_no_results(self, status, data): + self.get_teams_list(status, data) def test_page_size(self): result = self.get_teams_list(200, {'page_size': 2}) @@ -348,7 +355,7 @@ class TestListTeamsAPI(TeamAPITestCase): self.assertIsNotNone(result['previous']) def test_expand_user(self): - result = self.get_teams_list(200, {'expand': 'user', 'topic_id': 'renewable'}) + result = self.get_teams_list(200, {'expand': 'user', 'topic_id': 'topic_0'}) self.verify_expanded_user(result['results'][0]['membership'][0]['user']) @@ -561,6 +568,16 @@ class TestListTopicsAPI(TeamAPITestCase): response = self.get_topics_list(data={'course_id': self.test_course_1.id}) self.assertEqual(response['sort_order'], 'name') + def test_team_count(self): + """Test that team_count is included for each topic""" + response = self.get_topics_list(data={'course_id': self.test_course_1.id}) + for topic in response['results']: + self.assertIn('team_count', topic) + if topic['id'] == u'topic_0': + self.assertEqual(topic['team_count'], 1) + else: + self.assertEqual(topic['team_count'], 0) + @ddt.ddt class TestDetailTopicAPI(TeamAPITestCase): @@ -588,6 +605,13 @@ class TestDetailTopicAPI(TeamAPITestCase): def test_invalid_topic_id(self): self.get_topic_detail('no_such_topic', self.test_course_1.id, 404) + def test_team_count(self): + """Test that team_count is included with a topic""" + topic = self.get_topic_detail(topic_id='topic_0', course_id=self.test_course_1.id) + self.assertEqual(topic['team_count'], 1) + topic = self.get_topic_detail(topic_id='topic_1', course_id=self.test_course_1.id) + self.assertEqual(topic['team_count'], 0) + @ddt.ddt class TestListMembershipAPI(TeamAPITestCase): diff --git a/lms/djangoapps/teams/urls.py b/lms/djangoapps/teams/urls.py index 5f8d9aa941..ed3d4e5868 100644 --- a/lms/djangoapps/teams/urls.py +++ b/lms/djangoapps/teams/urls.py @@ -1,10 +1,11 @@ """Defines the URL routes for this app.""" from django.conf.urls import patterns, url +from django.contrib.auth.decorators import login_required from .views import TeamsDashboardView urlpatterns = patterns( 'teams.views', - url(r"^/$", TeamsDashboardView.as_view(), name="teams_dashboard") + url(r"^/$", login_required(TeamsDashboardView.as_view()), name="teams_dashboard") ) diff --git a/lms/djangoapps/teams/views.py b/lms/djangoapps/teams/views.py index 045a4584b3..c2efe1d87e 100644 --- a/lms/djangoapps/teams/views.py +++ b/lms/djangoapps/teams/views.py @@ -42,7 +42,14 @@ from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from .models import CourseTeam, CourseTeamMembership -from .serializers import CourseTeamSerializer, CourseTeamCreationSerializer, TopicSerializer, MembershipSerializer +from .serializers import ( + CourseTeamSerializer, + CourseTeamCreationSerializer, + BaseTopicSerializer, + TopicSerializer, + PaginatedTopicSerializer, + MembershipSerializer +) from .errors import AlreadyOnTeamInCourse, NotEnrolledInCourseForTeam @@ -75,9 +82,15 @@ class TeamsDashboardView(View): sort_order = 'name' topics = get_ordered_topics(course, sort_order) topics_page = Paginator(topics, TOPICS_PER_PAGE).page(1) - topics_serializer = PaginationSerializer(instance=topics_page, context={'sort_order': sort_order}) + topics_serializer = PaginatedTopicSerializer(instance=topics_page, context={'sort_order': sort_order}) context = { - "course": course, "topics": topics_serializer.data, "topics_url": reverse('topics_list', request=request) + "course": course, + "topics": topics_serializer.data, + "topic_url": reverse( + 'topics_detail', kwargs={'topic_id': 'topic_id', 'course_id': str(course_id)}, request=request + ), + "topics_url": reverse('topics_list', request=request), + "teams_url": reverse('teams_list', request=request) } return render_to_response("teams/teams.html", context) @@ -248,7 +261,8 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView): try: course_key = CourseKey.from_string(course_id_string) # Ensure the course exists - if not modulestore().has_course(course_key): + course_module = modulestore().get_course(course_key) + if course_module is None: return Response(status=status.HTTP_404_NOT_FOUND) result_filter.update({'course_id': course_key}) except InvalidKeyError: @@ -267,6 +281,13 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView): ) if 'topic_id' in request.QUERY_PARAMS: + topic_id = request.QUERY_PARAMS['topic_id'] + if topic_id not in [topic['id'] for topic in course_module.teams_configuration['topics']]: + error = build_api_error( + ugettext_noop('The supplied topic id {topic_id} is not valid'), + topic_id=topic_id + ) + return Response(error, status=status.HTTP_400_BAD_REQUEST) result_filter.update({'topic_id': request.QUERY_PARAMS['topic_id']}) if 'include_inactive' in request.QUERY_PARAMS and request.QUERY_PARAMS['include_inactive'].lower() == 'true': del result_filter['is_active'] @@ -290,14 +311,17 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView): build_api_error(ugettext_noop("last_activity is not yet supported")), status=status.HTTP_400_BAD_REQUEST ) + else: + return Response({ + 'developer_message': "unsupported order_by value {}".format(order_by_input), + 'user_message': _(u"The ordering {} is not supported").format(order_by_input), + }, status=status.HTTP_400_BAD_REQUEST) queryset = queryset.order_by(order_by_field) - if not queryset: - return Response(status=status.HTTP_404_NOT_FOUND) - page = self.paginate_queryset(queryset) serializer = self.get_pagination_serializer(page) + serializer.context.update({'sort_order': order_by_input}) # pylint: disable=maybe-no-member return Response(serializer.data) # pylint: disable=maybe-no-member def post(self, request): @@ -492,8 +516,8 @@ class TopicListView(GenericAPIView): paginate_by = TOPICS_PER_PAGE paginate_by_param = 'page_size' - pagination_serializer_class = PaginationSerializer - serializer_class = TopicSerializer + pagination_serializer_class = PaginatedTopicSerializer + serializer_class = BaseTopicSerializer def get(self, request): """GET /api/team/v0/topics/?course_id={course_id}""" @@ -531,8 +555,7 @@ class TopicListView(GenericAPIView): }, status=status.HTTP_400_BAD_REQUEST) page = self.paginate_queryset(topics) - serializer = self.get_pagination_serializer(page) - serializer.context = {'sort_order': ordering} + serializer = self.pagination_serializer_class(page, context={'sort_order': ordering}) return Response(serializer.data) # pylint: disable=maybe-no-member diff --git a/lms/static/js/components/card/views/card.js b/lms/static/js/components/card/views/card.js index 4dfb680748..e236b6b3b1 100644 --- a/lms/static/js/components/card/views/card.js +++ b/lms/static/js/components/card/views/card.js @@ -81,7 +81,8 @@ description: description, action_class: this.callIfFunction(this.actionClass), action_url: this.callIfFunction(this.actionUrl), - action_content: this.callIfFunction(this.actionContent) + action_content: this.callIfFunction(this.actionContent), + configuration: this.callIfFunction(this.configuration) })); var detailsEl = this.$el.find('.card-meta'); _.each(this.callIfFunction(this.details), function (detail) { diff --git a/lms/static/js/components/tabbed/views/tabbed_view.js b/lms/static/js/components/tabbed/views/tabbed_view.js index 4689e60447..5d5223b0ff 100644 --- a/lms/static/js/components/tabbed/views/tabbed_view.js +++ b/lms/static/js/components/tabbed/views/tabbed_view.js @@ -19,35 +19,59 @@ * following properties: * view (Backbone.View): the view to render for this tab. * title (string): The title to display for this tab. - * url (string): The URL fragment which will navigate to this tab. + * url (string): The URL fragment which will + * navigate to this tab when a router is + * provided. + * If a router is passed in (via options.router), + * use that router to keep track of history between + * tabs. Backbone.history.start() must be called + * by the router's instatiator after this view is + * initialized. */ initialize: function (options) { - this.router = new Backbone.Router(); - this.$el.html(this.template({})); - var self = this; + this.router = options.router || null; this.tabs = options.tabs; + this.urlMap = _.reduce(this.tabs, function (map, value) { + map[value.url] = value; + return map; + }, {}); + }, + + render: function () { + var self = this; + this.$el.html(this.template({})); _.each(this.tabs, function(tabInfo, index) { var tabEl = $(_.template(tabTemplate, { index: index, - title: tabInfo.title + title: tabInfo.title, + url: tabInfo.url })); self.$('.page-content-nav').append(tabEl); - - self.router.route(tabInfo.url, function () { - self.setActiveTab(index); - }); }); - this.setActiveTab(0); + if(Backbone.history.getHash() === "") { + this.setActiveTab(0); + } + return this; }, setActiveTab: function (index) { - var tab = this.tabs[index], - view = tab.view; + var tab, tabEl, view; + if (typeof index === 'string') { + tab = this.urlMap[index]; + tabEl = this.$('a[data-url='+index+']'); + } + else { + tab = this.tabs[index]; + tabEl = this.$('a[data-index='+index+']'); + } + view = tab.view; this.$('a.is-active').removeClass('is-active').attr('aria-selected', 'false'); - this.$('a[data-index='+index+']').addClass('is-active').attr('aria-selected', 'true'); + tabEl.addClass('is-active').attr('aria-selected', 'true'); view.setElement(this.$('.page-content-main')).render(); this.$('.sr-is-focusable.sr-tab').focus(); - this.router.navigate(tab.url, {replace: true}); + if (this.router) { + this.router.navigate(tab.url, {replace: true, trigger: true}); + } }, switchTab: function (event) { diff --git a/lms/static/js/spec/components/card/card_spec.js b/lms/static/js/spec/components/card/card_spec.js index fbb6f1f375..4145f9ee83 100644 --- a/lms/static/js/spec/components/card/card_spec.js +++ b/lms/static/js/spec/components/card/card_spec.js @@ -18,7 +18,7 @@ it('can render itself as a list card', function () { var view = new CardView({ configuration: 'list_card' }); expect(view.$el).toHaveClass('list-card'); - expect(view.$el.find('.wrapper-card-meta .action').length).toBe(1); + expect(view.$el.find('.wrapper-card-core .action').length).toBe(1); }); it('renders a pennant only if the pennant value is truthy', function () { diff --git a/lms/static/js/spec/components/tabbed/tabbed_view_spec.js b/lms/static/js/spec/components/tabbed/tabbed_view_spec.js index 618b340d7b..ae8245a1d8 100644 --- a/lms/static/js/spec/components/tabbed/tabbed_view_spec.js +++ b/lms/static/js/spec/components/tabbed/tabbed_view_spec.js @@ -20,23 +20,15 @@ describe('TabbedView component', function () { beforeEach(function () { - spyOn(Backbone.history, 'navigate').andCallThrough(); - Backbone.history.start(); view = new TabbedView({ tabs: [{ - url: 'test 1', title: 'Test 1', view: new TestSubview({text: 'this is test text'}) }, { - url: 'test 2', title: 'Test 2', view: new TestSubview({text: 'other text'}) }] - }); - }); - - afterEach(function () { - Backbone.history.stop(); + }).render(); }); it('can render itself', function () { @@ -59,12 +51,6 @@ expect(view.$el.text()).toContain('other text'); }); - it('changes tabs on navigation', function () { - expect(view.$('.nav-item.is-active').data('index')).toEqual(0); - Backbone.history.navigate('test 2', {trigger: true}); - expect(view.$('.nav-item.is-active').data('index')).toEqual(1); - }); - it('marks the active tab as selected using aria attributes', function () { expect(view.$('.nav-item[data-index=0]')).toHaveAttr('aria-selected', 'true'); expect(view.$('.nav-item[data-index=1]')).toHaveAttr('aria-selected', 'false'); @@ -73,17 +59,59 @@ expect(view.$('.nav-item[data-index=1]')).toHaveAttr('aria-selected', 'true'); }); - it('updates the page URL on tab switches without adding to browser history', function () { - view.$('.nav-item[data-index=1]').click(); - expect(Backbone.history.navigate).toHaveBeenCalledWith('test 2', {replace: true}); - }); - it('sets focus for screen readers', function () { spyOn($.fn, 'focus'); view.$('.nav-item[data-index=1]').click(); expect(view.$('.sr-is-focusable.sr-tab').focus).toHaveBeenCalled(); }); + + describe('history', function() { + beforeEach(function () { + spyOn(Backbone.history, 'navigate').andCallThrough(); + view = new TabbedView({ + tabs: [{ + url: 'test 1', + title: 'Test 1', + view: new TestSubview({text: 'this is test text'}) + }, { + url: 'test 2', + title: 'Test 2', + view: new TestSubview({text: 'other text'}) + }], + router: new Backbone.Router({ + routes: { + 'test 1': function () { + view.setActiveTab(0); + }, + 'test 2': function () { + view.setActiveTab(1); + } + } + }) + }).render(); + Backbone.history.start(); + }); + + afterEach(function () { + view.router.navigate(''); + Backbone.history.stop(); + }); + + it('updates the page URL on tab switches without adding to browser history', function () { + view.$('.nav-item[data-index=1]').click(); + expect(Backbone.history.navigate).toHaveBeenCalledWith( + 'test 2', + {replace: true, trigger: true} + ); + }); + + it('changes tabs on URL navigation', function () { + expect(view.$('.nav-item.is-active').data('index')).toEqual(0); + Backbone.history.navigate('test 2', {trigger: true}); + expect(view.$('.nav-item.is-active').data('index')).toEqual(1); + }); + }); + }); - } - ); + }); }).call(this, define || RequireJS.define); diff --git a/lms/static/js/spec/main.js b/lms/static/js/spec/main.js index e196515555..f842a41e60 100644 --- a/lms/static/js/spec/main.js +++ b/lms/static/js/spec/main.js @@ -531,6 +531,8 @@ '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/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', diff --git a/lms/static/sass/_developer.scss b/lms/static/sass/_developer.scss index c7b347be08..55191cd0fa 100644 --- a/lms/static/sass/_developer.scss +++ b/lms/static/sass/_developer.scss @@ -76,9 +76,6 @@ // teams temporary .view-teams { - .global-new, #global-navigation { - display: none; - } // Copied from _pagination.scss in cms .pagination { diff --git a/lms/templates/components/card/card.underscore b/lms/templates/components/card/card.underscore index 022a6b5327..0fa32b0939 100644 --- a/lms/templates/components/card/card.underscore +++ b/lms/templates/components/card/card.underscore @@ -1,3 +1,4 @@ +<% if (configuration === 'square_card') { %>
    <% if (pennant) { %> @@ -14,3 +15,21 @@ <%= action_content %>
    +<% } else { %> +
    +
    + <% if (pennant) { %> + <%- pennant %> + <% } %> +

    <%- title %>

    +

    <%- description %>

    +
    +
    + <%= action_content %> +
    +
    +
    +
    +
    +
    +<% } %> diff --git a/lms/templates/components/tabbed/tab.underscore b/lms/templates/components/tabbed/tab.underscore index bb53565f93..c14527032a 100644 --- a/lms/templates/components/tabbed/tab.underscore +++ b/lms/templates/components/tabbed/tab.underscore @@ -1 +1 @@ -<%- title %> +<%- title %> diff --git a/lms/templates/components/tabbed/tabbed_view.underscore b/lms/templates/components/tabbed/tabbed_view.underscore index 1bae6435ab..80a39c2655 100644 --- a/lms/templates/components/tabbed/tabbed_view.underscore +++ b/lms/templates/components/tabbed/tabbed_view.underscore @@ -1,5 +1,3 @@ -
    - -
    -
    -
    + +
    +