Skip to content
This repository has been archived by the owner on Aug 21, 2024. It is now read-only.

Commit

Permalink
Upgrade to Django 1.11.2 (#683)
Browse files Browse the repository at this point in the history
* Update edx-auth-backends and fix imports

* Update dependencies and fix new pylint errors

* Switch to alternative django-waffle fork

Current django-waffle master does not support management commands in Django
1.10+. The maintainer seems to have disappeared and hasn't merged any PRs in a
year.

This fork fixes the issues, but hopefully it will get merged into master in the
future and we can go back to installing django-waffle from pip.

* Pull freshly installed docker image for tests

* Don't use option "on" with waffle flag command

* Upgrade to Django 1.11.2

Will it just work?

* Use class based LogoutView and set redirect url

The function-based views are deprecated in Django 1.11. Except for
logout_then_login, which is still okay apparently.

* Use dispatch instead of get on InsightsLogoutView

* Address PR comments

* Use 0.25.1 of data api (also upgraded to 1.11)
  • Loading branch information
thallada authored Jun 8, 2017
1 parent 22f1ab4 commit 4144ed8
Show file tree
Hide file tree
Showing 31 changed files with 92 additions and 113 deletions.
2 changes: 1 addition & 1 deletion .travis/docker-compose-travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down
2 changes: 1 addition & 1 deletion acceptance_tests/course_validation/report_generators.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
6 changes: 2 additions & 4 deletions acceptance_tests/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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
Expand Down
6 changes: 0 additions & 6 deletions acceptance_tests/pages.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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')

Expand Down
3 changes: 1 addition & 2 deletions acceptance_tests/test_course_enrollment_demographics.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 2 additions & 4 deletions acceptance_tests/test_course_performance.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
31 changes: 14 additions & 17 deletions analytics_dashboard/core/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand All @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions analytics_dashboard/courses/presenters/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 2 additions & 4 deletions analytics_dashboard/courses/presenters/course_summaries.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand All @@ -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):
"""
Expand Down
5 changes: 2 additions & 3 deletions analytics_dashboard/courses/presenters/enrollment.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
7 changes: 3 additions & 4 deletions analytics_dashboard/courses/presenters/performance.py
Original file line number Diff line number Diff line change
Expand Up @@ -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']
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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):
Expand Down
3 changes: 1 addition & 2 deletions analytics_dashboard/courses/presenters/programs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand Down
8 changes: 3 additions & 5 deletions analytics_dashboard/courses/tests/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)')
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
)
Expand Down
3 changes: 2 additions & 1 deletion analytics_dashboard/courses/tests/test_views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion analytics_dashboard/courses/tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
11 changes: 4 additions & 7 deletions analytics_dashboard/courses/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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():
Expand Down Expand Up @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion analytics_dashboard/courses/views/performance.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
3 changes: 2 additions & 1 deletion analytics_dashboard/settings/base.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""Common settings and globals."""
next_page='/'"""Common settings and globals."""
import ConfigParser

import os
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion analytics_dashboard/templates/base_dashboard.html
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ <h1>{{ page_title }}</h1>
{% endcomment %}

{# Translation support for JavaScript strings. #}
<script type="text/javascript" src="{% url 'django.views.i18n.javascript_catalog' %}"></script>
<script type="text/javascript" src="{% url 'javascript-catalog' %}"></script>
<script type="text/javascript">
window.language = "{{ LANGUAGE_CODE|languade_code_to_cldr }}";
</script>
Expand Down
1 change: 0 additions & 1 deletion analytics_dashboard/templates/header.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
{% load i18n %}
{% load firstof from future %}
{% load dashboard_extras %}

{% comment %}
Expand Down
Loading

0 comments on commit 4144ed8

Please sign in to comment.