diff --git a/common/djangoapps/third_party_auth/management/commands/tests/test_saml.py b/common/djangoapps/third_party_auth/management/commands/tests/test_saml.py index 9e809e2ac4..92686f31c6 100644 --- a/common/djangoapps/third_party_auth/management/commands/tests/test_saml.py +++ b/common/djangoapps/third_party_auth/management/commands/tests/test_saml.py @@ -11,7 +11,6 @@ import mock from django.conf import settings from django.core.management import call_command from django.core.management.base import CommandError -from django.test import TestCase from django.utils.six import StringIO from requests import exceptions from requests.models import Response diff --git a/lms/djangoapps/edxnotes/decorators.py b/lms/djangoapps/edxnotes/decorators.py index d5f700508b..2fdb5da743 100644 --- a/lms/djangoapps/edxnotes/decorators.py +++ b/lms/djangoapps/edxnotes/decorators.py @@ -22,7 +22,9 @@ def edxnotes(cls): Returns raw html for the component. """ # Import is placed here to avoid model import at project startup. - from edxnotes.helpers import generate_uid, get_edxnotes_id_token, get_public_endpoint, get_token_url, is_feature_enabled + from edxnotes.helpers import ( + generate_uid, get_edxnotes_id_token, get_public_endpoint, get_token_url, is_feature_enabled + ) runtime = getattr(self, 'descriptor', self).runtime if not hasattr(runtime, 'modulestore'): diff --git a/lms/djangoapps/teams/csv.py b/lms/djangoapps/teams/csv.py index b03b7ae844..a5a399f489 100644 --- a/lms/djangoapps/teams/csv.py +++ b/lms/djangoapps/teams/csv.py @@ -70,7 +70,7 @@ def _lookup_team_membership_data(course): def _group_teamset_memberships_by_user(course_team_memberships): """ Parameters: - - course_team_memberships: a collection of CourseTeamMemberships + - course_team_memberships: a collection of CourseTeamMemberships. Returns: { @@ -192,6 +192,10 @@ class TeamMembershipImportManager(object): return True def load_user_ids_by_teamset_id(self): + """ + Get users associations with each teamset in a course and + save to `self.user_ids_by_teamset_id` + """ for teamset_id in self.teamset_ids: self.user_ids_by_teamset_id[teamset_id] = { membership.user_id for membership in @@ -216,7 +220,7 @@ class TeamMembershipImportManager(object): Ensures that username exists only once in an input file """ if username in usernames_found_so_far: - error_message = 'Username {} was found more than once in input file.'.format(username) + error_message = 'Username {} listed more than once in file.'.format(username) if self.add_error_and_check_if_max_exceeded(error_message): return False return True @@ -240,7 +244,7 @@ class TeamMembershipImportManager(object): except KeyError: # if a team doesn't exists, the validation doesn't apply to it. all_teamset_user_ids = self.user_ids_by_teamset_id[teamset_id] - error_message = 'The user {0} is already a member of a team inside teamset {1} in this course.'.format( + error_message = 'User {0} is already on a team in teamset {1}.'.format( user.username, teamset_id ) if user.id in all_teamset_user_ids and self.add_error_and_check_if_max_exceeded(error_message): @@ -250,11 +254,11 @@ class TeamMembershipImportManager(object): continue max_team_size = self.course.teams_configuration.default_max_team_size if max_team_size is not None and team.users.count() >= max_team_size: - if self.add_error_and_check_if_max_exceeded('Team ' + team.team_id + ' is already full.'): + if self.add_error_and_check_if_max_exceeded('Team ' + team.team_id + ' is full.'): return False if (user.id, team.topic_id) in self.existing_course_team_memberships: - error_message = 'The user {0} is already a member of a team inside teamset {1} in this course.'.format( + error_message = 'User {0} is already on a team in teamset {1}.'.format( user.username, team.topic_id ) if self.add_error_and_check_if_max_exceeded(error_message): @@ -318,6 +322,6 @@ class TeamMembershipImportManager(object): try: return User.objects.get(email=user_name) except User.DoesNotExist: - self.validation_errors.append('Username or email ' + user_name + ' does not exist.') + self.validation_errors.append('User ' + user_name + ' does not exist.') return None # TODO - handle user key case diff --git a/lms/djangoapps/teams/static/teams/js/views/manage.js b/lms/djangoapps/teams/static/teams/js/views/manage.js index 34f782eed3..37c08a13d7 100644 --- a/lms/djangoapps/teams/static/teams/js/views/manage.js +++ b/lms/djangoapps/teams/static/teams/js/views/manage.js @@ -58,13 +58,15 @@ ); }, - handleCsvUploadSuccess: function() { + handleCsvUploadSuccess: function(data) { + TeamUtils.showInfoBanner(data.message, false); + // This handler is currently unimplemented (TODO MST-44) this.teamEvents.trigger('teams:update', {}); }, - handleCsvUploadFailure: function() { - // This handler is currently unimplemented (TODO MST-44) + handleCsvUploadFailure: function(jqXHR) { + TeamUtils.showInfoBanner(jqXHR.responseJSON.errors, true); } }); return ManageView; diff --git a/lms/djangoapps/teams/static/teams/js/views/team_utils.js b/lms/djangoapps/teams/static/teams/js/views/team_utils.js index 723b583abd..5bd2511688 100644 --- a/lms/djangoapps/teams/static/teams/js/views/team_utils.js +++ b/lms/djangoapps/teams/static/teams/js/views/team_utils.js @@ -72,6 +72,38 @@ return false; } return topicType.toLowerCase() !== 'open'; + }, + + /** Shows info/error banner for team membership CSV upload + * @param: content - string or array for display + * @param: isError - true sets error styling, false/none uses info styling + */ + showInfoBanner: function(content, isError) { + // clear message + var $message = $('#team-management-assign .page-banner .message-content'); + $message.html(''); + + // set message + if (Array.isArray(content)) { + content.forEach(function(item) { + // xss-lint: disable=javascript-jquery-append + $message.append($('

').text(item)); + }); + } else { + $('#team-management-assign .page-banner .message-content').text(content); + } + + // set color sytling + $('#team-management-assign .page-banner .alert') + .toggleClass('alert-success', !isError) + .toggleClass('alert-danger', isError); + + // set icon styling + $('#team-management-assign .page-banner .icon') + .toggleClass('fa-check', !isError) + .toggleClass('fa-warning', isError); + + $('#team-management-assign .page-banner').show(); } }; }); diff --git a/lms/djangoapps/teams/static/teams/templates/manage.underscore b/lms/djangoapps/teams/static/teams/templates/manage.underscore index 6f3974f69f..460769a1cb 100644 --- a/lms/djangoapps/teams/static/teams/templates/manage.underscore +++ b/lms/djangoapps/teams/static/teams/templates/manage.underscore @@ -17,7 +17,7 @@ <%- gettext("Download Memberships") %> -

+

<%- gettext("Assign Team Memberships") %>

@@ -37,6 +37,12 @@ <%- gettext("Upload Memberships") %>
+
diff --git a/lms/djangoapps/teams/views.py b/lms/djangoapps/teams/views.py index 523331e0b8..e365410416 100644 --- a/lms/djangoapps/teams/views.py +++ b/lms/djangoapps/teams/views.py @@ -11,12 +11,11 @@ from django.contrib.auth.models import User from django.core.exceptions import PermissionDenied from django.db.models.signals import post_save from django.dispatch import receiver -from django.http import Http404, HttpResponse +from django.http import Http404, HttpResponse, JsonResponse from django.shortcuts import get_object_or_404, render_to_response from django.utils.functional import cached_property from django.utils.translation import ugettext as _ from django.utils.translation import ugettext_noop -from django.views import View from django_countries import countries from edx_rest_framework_extensions.paginators import DefaultPagination, paginate_search_results from opaque_keys import InvalidKeyError @@ -1387,13 +1386,16 @@ class MembershipBulkManagementView(GenericAPIView): Process uploaded CSV to modify team memberships for given course run. """ self.check_access() + inputfile_handle = request.FILES['csv'] team_import_manager = TeamMembershipImportManager(self.course) team_import_manager.set_team_membership_from_csv(inputfile_handle) + if team_import_manager.import_succeeded: - return Response(str(team_import_manager.number_of_records_added), status=status.HTTP_201_CREATED) + msg = "Successfully added {} students to teams".format(team_import_manager.number_of_records_added) + return JsonResponse({'message': msg}, status=status.HTTP_201_CREATED) else: - return Response({ + return JsonResponse({ 'errors': team_import_manager.validation_errors }, status=status.HTTP_400_BAD_REQUEST) diff --git a/openedx/core/djangoapps/catalog/tests/test_models.py b/openedx/core/djangoapps/catalog/tests/test_models.py index 496d81f64f..6207599459 100644 --- a/openedx/core/djangoapps/catalog/tests/test_models.py +++ b/openedx/core/djangoapps/catalog/tests/test_models.py @@ -3,7 +3,7 @@ import ddt -from django.test import TestCase, override_settings +from django.test import override_settings from openedx.core.djangoapps.catalog.tests import mixins from openedx.core.djangoapps.site_configuration.tests.test_util import with_site_configuration diff --git a/openedx/core/djangoapps/user_api/models.py b/openedx/core/djangoapps/user_api/models.py index f91cb9bcec..11a44f85ec 100644 --- a/openedx/core/djangoapps/user_api/models.py +++ b/openedx/core/djangoapps/user_api/models.py @@ -92,7 +92,7 @@ def pre_save_callback(sender, **kwargs): Event changes to user preferences. """ user_preference = kwargs["instance"] - user_preference._old_value = get_changed_fields_dict(user_preference, sender).get("value", None) + user_preference._old_value = get_changed_fields_dict(user_preference, sender).get("value", None) # pylint: disable=protected-access @receiver(post_save, sender=UserPreference) @@ -107,9 +107,9 @@ def post_save_callback(sender, **kwargs): username=user_preference.user.username, new=user_preference.value)) emit_setting_changed_event( user_preference.user, sender._meta.db_table, user_preference.key, - user_preference._old_value, user_preference.value + user_preference._old_value, user_preference.value # pylint: disable=protected-access ) - user_preference._old_value = None + user_preference._old_value = None # pylint: disable=protected-access @receiver(post_delete, sender=UserPreference) @@ -139,7 +139,7 @@ class UserCourseTag(models.Model): unique_together = ("user", "course_id", "key") -class UserOrgTag(TimeStampedModel, DeletableByUserValue): # pylint: disable=model-missing-unicode +class UserOrgTag(TimeStampedModel, DeletableByUserValue): """ Per-Organization user tags. @@ -172,7 +172,6 @@ class RetirementState(models.Model): required = models.BooleanField(default=False) def __str__(self): - # pylint: disable=unicode-format-string return '{} (step {})'.format(self.state_name, self.state_execution_order) class Meta(object): @@ -293,7 +292,9 @@ class UserRetirementStatus(TimeStampedModel): if new_state_index <= states.index(self.current_state.state_name): raise ValueError() except ValueError: - err = u'{} does not exist or is an eariler state than current state {}'.format(new_state, self.current_state) + err = u'{} does not exist or is an eariler state than current state {}'.format( + new_state, self.current_state + ) raise RetirementStateError(err) def _validate_update_data(self, data): @@ -306,7 +307,9 @@ class UserRetirementStatus(TimeStampedModel): for required_key in required_keys: if required_key not in data: - raise RetirementStateError(u'RetirementStatus: Required key {} missing from update'.format(required_key)) + raise RetirementStateError(u'RetirementStatus: Required key {} missing from update'.format( + required_key + )) for key in data: if key not in known_keys: