diff --git a/.travis/docker-compose-travis.yml b/.travis/docker-compose-travis.yml index 4a86b322c..40447ae70 100644 --- a/.travis/docker-compose-travis.yml +++ b/.travis/docker-compose-travis.yml @@ -12,7 +12,7 @@ services: ELASTICSEARCH_LEARNERS_UPDATE_INDEX: 'index_update' command: /edx/app/analytics_api/venvs/analytics_api/bin/python /edx/app/analytics_api/analytics_api/manage.py runserver 0.0.0.0:80 insights: - image: edxops/insights:latest + image: edxops/insights:upgrade-django container_name: insights_testing volumes: - ..:/edx/app/insights/edx_analytics_dashboard diff --git a/Makefile b/Makefile index 316d280bf..7a16753bd 100644 --- a/Makefile +++ b/Makefile @@ -42,7 +42,7 @@ test_python: test_compress test_python_no_compress accept: ifeq ("${DISPLAY_LEARNER_ANALYTICS}", "True") - ./manage.py waffle_flag enable_learner_analytics on --create --everyone + ./manage.py waffle_flag enable_learner_analytics --create --everyone endif ifeq ("${ENABLE_COURSE_LIST_FILTERS}", "True") ./manage.py waffle_switch enable_course_filters on --create @@ -63,7 +63,7 @@ accept_local: a11y: ifeq ("${DISPLAY_LEARNER_ANALYTICS}", "True") - ./manage.py waffle_flag enable_learner_analytics on --create --everyone + ./manage.py waffle_flag enable_learner_analytics --create --everyone endif BOKCHOY_A11Y_CUSTOM_RULES_FILE=./node_modules/edx-custom-a11y-rules/lib/custom_a11y_rules.js SELENIUM_BROWSER=firefox nosetests -v a11y_tests -e NUM_PROCESSES=1 --exclude-dir=acceptance_tests/course_validation diff --git a/acceptance_tests/course_validation/report_generators.py b/acceptance_tests/course_validation/report_generators.py index 5ce51d04e..b8dc99914 100644 --- a/acceptance_tests/course_validation/report_generators.py +++ b/acceptance_tests/course_validation/report_generators.py @@ -223,7 +223,7 @@ def generate_report(self): path = 'performance/graded_content/{}'.format(slugify(assignment_type)) path = self.build_course_path(path) status, elapsed = self.get_http_status_and_load_time(self.get_dashboard_url(path)) - valid = (has_submissions and status == 200) or not has_submissions + valid = (status == 200 if has_submissions else True) course_valid &= valid data = { diff --git a/acceptance_tests/mixins.py b/acceptance_tests/mixins.py index ed5d253d5..f9c6eebf3 100644 --- a/acceptance_tests/mixins.py +++ b/acceptance_tests/mixins.py @@ -313,8 +313,7 @@ def format_number(value): """ Format the given value for the current locale (e.g. include decimal separator). """ if isinstance(value, int): return locale.format("%d", value, grouping=True) - else: - return locale.format("%.1f", value, grouping=True) + return locale.format("%.1f", value, grouping=True) def assertSummaryPointValueEquals(self, data_selector, value): """ @@ -390,8 +389,7 @@ def build_display_percentage(self, count, total, zero_percent_default='0.0%'): if total and count: percent = count / float(total) * 100.0 return '{:.1f}%'.format(percent) if percent >= 1.0 else '< 1%' - else: - return zero_percent_default + return zero_percent_default def _get_data_update_message(self): raise NotImplementedError diff --git a/acceptance_tests/pages.py b/acceptance_tests/pages.py index 84b732fe5..670a72654 100644 --- a/acceptance_tests/pages.py +++ b/acceptance_tests/pages.py @@ -32,9 +32,6 @@ def __init__(self, browser, path=None): class LandingPage(DashboardPage): path = '' - def __init__(self, browser): - super(LandingPage, self).__init__(browser) - def is_browser_on_page(self): return self.browser.current_url == self.page_url @@ -198,9 +195,6 @@ def is_browser_on_page(self): class CourseIndexPage(DashboardPage): path = 'courses/' - def __init__(self, browser): - super(CourseIndexPage, self).__init__(browser) - def is_browser_on_page(self): return self.browser.title.startswith('Courses') diff --git a/acceptance_tests/test_course_enrollment_demographics.py b/acceptance_tests/test_course_enrollment_demographics.py index 50742caff..2e992f9c9 100644 --- a/acceptance_tests/test_course_enrollment_demographics.py +++ b/acceptance_tests/test_course_enrollment_demographics.py @@ -53,8 +53,7 @@ def _calculate_median_age(self, current_year): if total_enrollment % 2 == 0: next_age = current_year - data[index + 1]['birth_year'] return (next_age + age) * 0.5 - else: - return age + return age return None diff --git a/acceptance_tests/test_course_performance.py b/acceptance_tests/test_course_performance.py index 2ea651817..17ea1935c 100644 --- a/acceptance_tests/test_course_performance.py +++ b/acceptance_tests/test_course_performance.py @@ -48,14 +48,12 @@ def _get_data_update_message(self): def _format_number_or_hyphen(self, value): if value: return self.format_number(value) - else: - return '-' + return '-' def _build_display_percentage_or_hyphen(self, correct, total): if correct: return self.build_display_percentage(correct, total) - else: - return '-' + return '-' def _get_problems_dict(self): # Retrieve the submissions from the Analytics Data API and create a lookup table. diff --git a/analytics_dashboard/core/views.py b/analytics_dashboard/core/views.py index 2579d419f..56c7857b3 100644 --- a/analytics_dashboard/core/views.py +++ b/analytics_dashboard/core/views.py @@ -2,9 +2,9 @@ import logging import uuid -import django from django.conf import settings -from django.contrib.auth import get_user_model, login, authenticate, REDIRECT_FIELD_NAME +from django.contrib.auth import get_user_model, login, authenticate +from django.contrib.auth.views import LogoutView, logout_then_login from django.db import connection, DatabaseError from django.http import HttpResponse, Http404 from django.shortcuts import redirect @@ -99,26 +99,24 @@ def get(self, request, *_args, **_kwargs): return redirect('/') -def logout(request, next_page='/', template_name=None, - redirect_field_name=REDIRECT_FIELD_NAME, extra_context=None): - """ - Revoke user permissions and logout - """ - - # Revoke permissions - permissions.revoke_user_course_permissions(request.user) +class InsightsLogoutView(LogoutView): + def dispatch(self, request, *args, **kwargs): + """ + Revoke user permissions and logout + """ + # Revoke permissions + permissions.revoke_user_course_permissions(request.user) - # Back to the standard logout flow - return django.contrib.auth.views.logout(request, next_page, template_name, redirect_field_name, - extra_context) + # Back to the standard logout flow + return super(InsightsLogoutView, self).dispatch(request, *args, **kwargs) -def logout_then_login(request, login_url=reverse_lazy('login'), extra_context=None): +def insights_logout_then_login(request, login_url=reverse_lazy('login')): """ Logout then login """ permissions.revoke_user_course_permissions(request.user) - return django.contrib.auth.views.logout_then_login(request, login_url, extra_context) + return logout_then_login(request, login_url=login_url) class ServiceUnavailableView(TemplateView): @@ -138,8 +136,7 @@ def dispatch(self, request, *args, **kwargs): """ Non logged in users will be directed to the landing page. """ if request.user.is_anonymous(): return super(LandingView, self).dispatch(request, *args, **kwargs) - else: - return redirect('courses:index') + return redirect('courses:index') def get_context_data(self, **kwargs): context = super(LandingView, self).get_context_data(**kwargs) diff --git a/analytics_dashboard/courses/presenters/__init__.py b/analytics_dashboard/courses/presenters/__init__.py index 89f2bc1a5..cde24094c 100644 --- a/analytics_dashboard/courses/presenters/__init__.py +++ b/analytics_dashboard/courses/presenters/__init__.py @@ -337,8 +337,7 @@ def sibling_block(self, block_id, sibling_offset): sibling_index = block_index + sibling_offset if sibling_index < 0: return None - else: - return siblings[sibling_index] + return siblings[sibling_index] except (StopIteration, IndexError): # StopIteration: requested video not found in the course structure # IndexError: No such video with the requested offset diff --git a/analytics_dashboard/courses/presenters/course_summaries.py b/analytics_dashboard/courses/presenters/course_summaries.py index 3f35abcf8..1693f04d0 100644 --- a/analytics_dashboard/courses/presenters/course_summaries.py +++ b/analytics_dashboard/courses/presenters/course_summaries.py @@ -17,8 +17,7 @@ def filter_summaries(all_summaries, course_ids=None): """Filter results to just the course IDs specified.""" if course_ids is None: return all_summaries - else: - return [summary for summary in all_summaries if summary['course_id'] in course_ids] + return [summary for summary in all_summaries if summary['course_id'] in course_ids] def _get_all_summaries(self): """ @@ -42,8 +41,7 @@ def _get_last_updated(self, summaries): if summaries: summary = summaries[0] return self.parse_api_datetime(summary['created']) - else: - return None + return None def get_course_summaries(self, course_ids=None): """ diff --git a/analytics_dashboard/courses/presenters/enrollment.py b/analytics_dashboard/courses/presenters/enrollment.py index 62c637961..59b306fe5 100644 --- a/analytics_dashboard/courses/presenters/enrollment.py +++ b/analytics_dashboard/courses/presenters/enrollment.py @@ -112,7 +112,7 @@ def _get_valid_enrollment_modes(self, trends): invalid_modes.remove(candidate) valid_modes.add(candidate) - if len(invalid_modes) == 0: + if not invalid_modes: break return valid_modes @@ -388,8 +388,7 @@ def _calculate_median_age(self, api_response): # be the case that at the loop can be advanced next_age = current_year - api_response[index + 1]['birth_year'] return (next_age + age) * 0.5 - else: - return age + return age return None diff --git a/analytics_dashboard/courses/presenters/performance.py b/analytics_dashboard/courses/presenters/performance.py index 458cd3db5..929f9259c 100644 --- a/analytics_dashboard/courses/presenters/performance.py +++ b/analytics_dashboard/courses/presenters/performance.py @@ -63,7 +63,7 @@ def get_answer_distribution(self, problem_id, problem_part_id): questions = self._build_questions(api_response) filtered_active_question = [i for i in questions if i['part_id'] == problem_part_id] - if len(filtered_active_question) == 0: + if not filtered_active_question: raise NotFoundError else: active_question = filtered_active_question[0]['question'] @@ -239,7 +239,7 @@ def attach_computed_data(self, problem): def post_process_adding_data_to_blocks(self, data, parent_block, child_block, url_func=None): # not all problems have submissions - if len(data['part_ids']) > 0: + if data['part_ids']: utils.sorting.natural_sort(data['part_ids']) if url_func: data['url'] = url_func(parent_block, child_block, data) @@ -368,8 +368,7 @@ def assignment(self, assignment_id): filtered = [assignment for assignment in self.assignments() if assignment['id'] == assignment_id] if filtered: return filtered[0] - else: - return None + return None @property def section_type_template(self): diff --git a/analytics_dashboard/courses/presenters/programs.py b/analytics_dashboard/courses/presenters/programs.py index b6a817c6c..8da4880c4 100644 --- a/analytics_dashboard/courses/presenters/programs.py +++ b/analytics_dashboard/courses/presenters/programs.py @@ -21,8 +21,7 @@ def filter_programs(all_programs, program_ids=None, course_ids=None): # Now apply course_ids filter if course_ids is None: return programs - else: - return [program for program in programs if not set(program['course_ids']).isdisjoint(course_ids)] + return [program for program in programs if not set(program['course_ids']).isdisjoint(course_ids)] def _get_all_programs(self): """ diff --git a/analytics_dashboard/courses/tests/factories.py b/analytics_dashboard/courses/tests/factories.py index c66aa1b94..97c1b688b 100644 --- a/analytics_dashboard/courses/tests/factories.py +++ b/analytics_dashboard/courses/tests/factories.py @@ -360,9 +360,8 @@ def __init__(self, tags_data_per_homework_assigment): def _get_block_id(self, block_type, block_format=None, display_name=None, graded=True, children=None): if display_name: return hashlib.md5(display_name).hexdigest() - else: - return super(TagsDistributionDataFactory, self)._get_block_id(block_type, block_format, display_name, - graded, children) + return super(TagsDistributionDataFactory, self)._get_block_id(block_type, block_format, display_name, + graded, children) def _generate_tags_structure(self): reg = re.compile(r'Homework (\d) Problem (\d)') @@ -406,8 +405,7 @@ def get_expected_learning_outcome_tags_content_nav(self, key): if key in expected_available_tags: return [{'id': v, 'name': v, 'url': url_template.format(self.course_id, slugify(v))} for v in expected_available_tags[key]] - else: - return [] + return [] def get_expected_tags_distribution(self, tag_key): index = 0 diff --git a/analytics_dashboard/courses/tests/test_presenters/test_presenters.py b/analytics_dashboard/courses/tests/test_presenters/test_presenters.py index d68ff150d..cfd0ad329 100644 --- a/analytics_dashboard/courses/tests/test_presenters/test_presenters.py +++ b/analytics_dashboard/courses/tests/test_presenters/test_presenters.py @@ -237,7 +237,7 @@ def _create_graded_and_ungraded_course_structure_fixtures(self): } for grade_status in ['graded', 'ungraded']: - sequential_fixture = SequentialFixture(graded=grade_status is 'graded').add_children( + sequential_fixture = SequentialFixture(graded=(grade_status == 'graded')).add_children( VerticalFixture().add_children( VideoFixture() ) diff --git a/analytics_dashboard/courses/tests/test_views/__init__.py b/analytics_dashboard/courses/tests/test_views/__init__.py index 7fe5a923d..fd12e6aa5 100644 --- a/analytics_dashboard/courses/tests/test_views/__init__.py +++ b/analytics_dashboard/courses/tests/test_views/__init__.py @@ -10,7 +10,8 @@ from django.utils.translation import ugettext_lazy as _ import httpretty import mock -from mock import patch, Mock, _is_started +from mock import patch, Mock +from mock.mock import _is_started from waffle.testutils import override_switch from core.tests.test_views import RedirectTestCaseMixin, UserTestCaseMixin diff --git a/analytics_dashboard/courses/tests/utils.py b/analytics_dashboard/courses/tests/utils.py index 6ea10e2de..6dbce42a5 100644 --- a/analytics_dashboard/courses/tests/utils.py +++ b/analytics_dashboard/courses/tests/utils.py @@ -843,7 +843,7 @@ def get_mock_course_summaries_csv(course_ids, has_programs=False, has_passing=Fa if len(associated_programs) > 1: first_program = associated_programs[0] second_program = associated_programs[1] - elif len(associated_programs) > 0: + elif associated_programs: first_program = associated_programs[0] for program_field in ['program_id', 'program_title']: program_data = program_data + '{}{}{}'.format( diff --git a/analytics_dashboard/courses/views/__init__.py b/analytics_dashboard/courses/views/__init__.py index 631c19a6b..0af94133f 100644 --- a/analytics_dashboard/courses/views/__init__.py +++ b/analytics_dashboard/courses/views/__init__.py @@ -155,8 +155,7 @@ def get_page_data(self, context): """ Returns JSON serialized data with lazy translations converted. """ if 'js_data' in context: return json.dumps(context['js_data'], cls=LazyEncoder) - else: - return None + return None class CourseContextMixin(CourseAPIMixin, TrackedViewMixin, LazyEncoderMixin): @@ -235,9 +234,8 @@ def is_valid_course(self): # pylint: disable=no-member return response.status_code == requests.codes.ok - else: - # all courses valid if LMS url isn't specified - return True + # all courses valid if LMS url isn't specified + return True def dispatch(self, request, *args, **kwargs): if self.is_valid_course(): @@ -453,8 +451,7 @@ class LastUpdatedView(object): def get_last_updated_message(self, last_updated): if last_updated: return self.update_message % self.format_last_updated_date_and_time(last_updated) - else: - return None + return None @staticmethod def format_last_updated_date_and_time(d): diff --git a/analytics_dashboard/courses/views/performance.py b/analytics_dashboard/courses/views/performance.py index 63f890777..4661adf21 100644 --- a/analytics_dashboard/courses/views/performance.py +++ b/analytics_dashboard/courses/views/performance.py @@ -433,7 +433,7 @@ def get_context_data(self, **kwargs): if self.has_part_id_param and self.part_id is None and self.problem_id: assignments = self.presenter.course_module_data() - if self.problem_id in assignments and len(assignments[self.problem_id]['part_ids']) > 0: + if self.problem_id in assignments and assignments[self.problem_id]['part_ids']: self.part_id = assignments[self.problem_id]['part_ids'][0] modules_marked_with_tag = self.tags_presenter.get_modules_marked_with_tag('learning_outcome', diff --git a/analytics_dashboard/settings/base.py b/analytics_dashboard/settings/base.py index 3cbfec392..ce4867cf4 100644 --- a/analytics_dashboard/settings/base.py +++ b/analytics_dashboard/settings/base.py @@ -1,4 +1,4 @@ -"""Common settings and globals.""" +next_page='/'"""Common settings and globals.""" import ConfigParser import os @@ -362,6 +362,7 @@ COURSE_PERMISSIONS_TIMEOUT = 900 LOGIN_REDIRECT_URL = '/courses/' +LOGOUT_REDIRECT_URL = '/' # Determines if course permissions should be checked before rendering course views. ENABLE_COURSE_PERMISSIONS = True diff --git a/analytics_dashboard/templates/base_dashboard.html b/analytics_dashboard/templates/base_dashboard.html index 14615b547..e383c1ba8 100644 --- a/analytics_dashboard/templates/base_dashboard.html +++ b/analytics_dashboard/templates/base_dashboard.html @@ -60,7 +60,7 @@