diff --git a/openedx/core/djangoapps/user_api/accounts/serializers.py b/openedx/core/djangoapps/user_api/accounts/serializers.py index 1110bfad9c..7f197cb8b6 100644 --- a/openedx/core/djangoapps/user_api/accounts/serializers.py +++ b/openedx/core/djangoapps/user_api/accounts/serializers.py @@ -475,12 +475,10 @@ class UserRetirementPartnerReportSerializer(serializers.Serializer): Perform serialization for the UserRetirementPartnerReportingStatus model """ user_id = serializers.IntegerField() - student_id = serializers.CharField(required=False) original_username = serializers.CharField() original_email = serializers.EmailField() original_name = serializers.CharField() orgs = serializers.ListField(child=serializers.CharField()) - orgs_config = serializers.ListField(required=False) created = serializers.DateTimeField() # Required overrides of abstract base class methods, but we don't use them diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py index ab44772a3b..b9e17df87c 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py @@ -46,7 +46,6 @@ from openedx.core.djangoapps.credit.models import ( CreditRequirement, CreditRequirementStatus ) -from openedx.core.djangoapps.external_user_ids.models import ExternalId, ExternalIdType from openedx.core.djangoapps.oauth_dispatch.jwt import create_jwt_for_user from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory from openedx.core.djangoapps.user_api.accounts.views import AccountRetirementPartnerReportView @@ -288,7 +287,7 @@ class TestPartnerReportingCleanup(ModuleStoreTestCase): def create_partner_reporting_statuses(self, is_being_processed=True, num=2): """ - Creates and returns the given number of test users and UserRetirementPartnerReportingStatuses + Creates and returnes the given number of test users and UserRetirementPartnerReportingStatuses with the given is_being_processed value. """ statuses = [] @@ -483,17 +482,6 @@ class TestPartnerReportingList(ModuleStoreTestCase): """ Tests the partner reporting list endpoint """ - EXPECTED_MB_ORGS_CONFIG = [ - { - AccountRetirementPartnerReportView.ORGS_CONFIG_ORG_KEY: 'mb_coaching', - AccountRetirementPartnerReportView.ORGS_CONFIG_FIELD_HEADINGS_KEY: [ - AccountRetirementPartnerReportView.STUDENT_ID_KEY, - AccountRetirementPartnerReportView.ORIGINAL_EMAIL_KEY, - AccountRetirementPartnerReportView.ORIGINAL_NAME_KEY, - AccountRetirementPartnerReportView.DELETION_COMPLETED_KEY - ] - } - ] def setUp(self): super(TestPartnerReportingList, self).setUp() @@ -505,7 +493,6 @@ class TestPartnerReportingList(ModuleStoreTestCase): self.url = reverse('accounts_retirement_partner_report') self.maxDiff = None self.test_created_datetime = datetime.datetime(2018, 1, 1, tzinfo=pytz.UTC) - ExternalIdType.objects.get_or_create(name=ExternalIdType.MICROBACHELORS_COACHING) def get_user_dict(self, user, enrollments): """ @@ -530,10 +517,9 @@ class TestPartnerReportingList(ModuleStoreTestCase): their processing state to "is_being_processed". Returns a list of user dicts representing what we would expect back from the - endpoint for the given user / enrollment, and a list of the users themselves. + endpoint for the given user / enrollment. """ user_dicts = [] - users = [] courses = self.courses if courses is None else courses for _ in range(num): @@ -554,9 +540,8 @@ class TestPartnerReportingList(ModuleStoreTestCase): user_dicts.append( self.get_user_dict(user, enrollments) ) - users.append(user) - return user_dicts, users + return user_dicts def assert_status_and_user_list(self, expected_users, expected_status=status.HTTP_200_OK): """ @@ -576,19 +561,9 @@ class TestPartnerReportingList(ModuleStoreTestCase): # These sub-lists will fail assertCountEqual if they're out of order for expected_user in expected_users: expected_user['orgs'].sort() - if AccountRetirementPartnerReportView.ORGS_CONFIG_KEY in expected_user: - orgs_config = expected_user[AccountRetirementPartnerReportView.ORGS_CONFIG_KEY] - orgs_config.sort() - for config in orgs_config: - config[AccountRetirementPartnerReportView.ORGS_CONFIG_FIELD_HEADINGS_KEY].sort() for returned_user in returned_users: returned_user['orgs'].sort() - if AccountRetirementPartnerReportView.ORGS_CONFIG_KEY in returned_user: - orgs_config = returned_user[AccountRetirementPartnerReportView.ORGS_CONFIG_KEY] - orgs_config.sort() - for config in orgs_config: - config[AccountRetirementPartnerReportView.ORGS_CONFIG_FIELD_HEADINGS_KEY].sort() self.assertCountEqual(returned_users, expected_users) @@ -596,72 +571,21 @@ class TestPartnerReportingList(ModuleStoreTestCase): """ Basic test to make sure that users in two different orgs are returned. """ - user_dicts, users = self.create_partner_reporting_statuses() - additional_dicts, additional_users = self.create_partner_reporting_statuses(courses=(self.course_awesome_org,)) - user_dicts += additional_dicts + users = self.create_partner_reporting_statuses() + users += self.create_partner_reporting_statuses(courses=(self.course_awesome_org,)) - self.assert_status_and_user_list(user_dicts) + self.assert_status_and_user_list(users) def test_success_multiple_statuses(self): """ Checks that only users in the correct is_being_processed state (False) are returned. """ - user_dicts, users = self.create_partner_reporting_statuses() + users = self.create_partner_reporting_statuses() # These should not come back self.create_partner_reporting_statuses(courses=(self.course_awesome_org,), is_being_processed=True) - self.assert_status_and_user_list(user_dicts) - - def test_success_mb_coaching(self): - """ - Check that MicroBachelors users who have consented to coaching have the proper info - included for the partner report. - """ - path = 'openedx.core.djangoapps.user_api.accounts.views.has_ever_consented_to_coaching' - with mock.patch(path, return_value=True) as mock_has_ever_consented: - user_dicts, users = self.create_partner_reporting_statuses(num=1) - external_id, created = ExternalId.add_new_user_id( - user=users[0], - type_name=ExternalIdType.MICROBACHELORS_COACHING - ) - - expected_user = user_dicts[0] - expected_users = [expected_user] - expected_user[AccountRetirementPartnerReportView.STUDENT_ID_KEY] = str(external_id.external_user_id) - expected_user[ - AccountRetirementPartnerReportView.ORGS_CONFIG_KEY] = TestPartnerReportingList.EXPECTED_MB_ORGS_CONFIG - - self.assert_status_and_user_list(expected_users) - mock_has_ever_consented.assert_called_once() - - def test_success_mb_coaching_no_external_id(self): - """ - Check that MicroBachelors users who have consented to coaching, but who do not have an external id, have the - proper info included for the partner report. - """ - path = 'openedx.core.djangoapps.user_api.accounts.views.has_ever_consented_to_coaching' - with mock.patch(path, return_value=True) as mock_has_ever_consented: - user_dicts, users = self.create_partner_reporting_statuses(num=1) - - self.assert_status_and_user_list(user_dicts) - mock_has_ever_consented.assert_called_once() - - def test_success_mb_coaching_no_consent(self): - """ - Check that MicroBachelors users who have not consented to coaching have the proper info - included for the partner report. - """ - path = 'openedx.core.djangoapps.user_api.accounts.views.has_ever_consented_to_coaching' - with mock.patch(path, return_value=False) as mock_has_ever_consented: - user_dicts, users = self.create_partner_reporting_statuses(num=1) - ExternalId.add_new_user_id( - user=users[0], - type_name=ExternalIdType.MICROBACHELORS_COACHING - ) - - self.assert_status_and_user_list(user_dicts) - mock_has_ever_consented.assert_called_once() + self.assert_status_and_user_list(users) def test_no_users(self): """ @@ -682,10 +606,10 @@ class TestPartnerReportingList(ModuleStoreTestCase): Checks that users are progressed to "is_being_processed" True upon being returned from this call. """ - user_dicts, users = self.create_partner_reporting_statuses() + users = self.create_partner_reporting_statuses() # First time through we should get the users - self.assert_status_and_user_list(user_dicts) + self.assert_status_and_user_list(users) # Second time they should be updated to is_being_processed=True self.assert_status_and_user_list([]) @@ -1181,7 +1105,7 @@ class TestAccountRetirementUpdate(RetirementTestCase): data = {'new_state': 'LOCKING_ACCOUNT', 'response': 'this should succeed'} self.update_and_assert_status(data) - # Refresh the retirement object and confirm the messages and state are correct + # Refresh the retirment object and confirm the messages and state are correct retirement = UserRetirementStatus.objects.get(id=self.retirement.id) self.assertEqual(retirement.current_state, RetirementState.objects.get(state_name='LOCKING_ACCOUNT')) self.assertEqual(retirement.last_state, RetirementState.objects.get(state_name='PENDING')) @@ -1203,7 +1127,7 @@ class TestAccountRetirementUpdate(RetirementTestCase): for update_data in fake_retire_process: self.update_and_assert_status(update_data) - # Refresh the retirement object and confirm the messages and state are correct + # Refresh the retirment object and confirm the messages and state are correct retirement = UserRetirementStatus.objects.get(id=self.retirement.id) self.assertEqual(retirement.current_state, RetirementState.objects.get(state_name='COMPLETE')) self.assertEqual(retirement.last_state, RetirementState.objects.get(state_name='CREDENTIALS_COMPLETE')) diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index 0719ebe217..c319af66d6 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -44,7 +44,6 @@ from openedx.core.djangoapps.ace_common.template_context import get_base_templat from openedx.core.djangoapps.api_admin.models import ApiAccessRequest from openedx.core.djangoapps.course_groups.models import UnregisteredLearnerCohortAssignments from openedx.core.djangoapps.credit.models import CreditRequest, CreditRequirementStatus -from openedx.core.djangoapps.external_user_ids.models import ExternalId, ExternalIdType from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY from openedx.core.djangoapps.profile_images.images import remove_profile_images from openedx.core.djangoapps.user_api.accounts.image_helpers import get_profile_image_names, set_has_profile_image @@ -83,11 +82,6 @@ from .permissions import CanDeactivateUser, CanReplaceUsername, CanRetireUser from .serializers import UserRetirementPartnerReportSerializer, UserRetirementStatusSerializer from .signals import USER_RETIRE_LMS_CRITICAL, USER_RETIRE_LMS_MISC, USER_RETIRE_MAILINGS -try: - from coaching.api import has_ever_consented_to_coaching -except ImportError: - has_ever_consented_to_coaching = None - log = logging.getLogger(__name__) USER_PROFILE_PII = { @@ -536,14 +530,6 @@ class AccountRetirementPartnerReportView(ViewSet): Provides API endpoints for managing partner reporting of retired users. """ - DELETION_COMPLETED_KEY = 'deletion_completed' - ORGS_CONFIG_KEY = 'orgs_config' - ORGS_CONFIG_ORG_KEY = 'org' - ORGS_CONFIG_FIELD_HEADINGS_KEY = 'field_headings' - ORIGINAL_EMAIL_KEY = 'original_email' - ORIGINAL_NAME_KEY = 'original_name' - STUDENT_ID_KEY = 'student_id' - authentication_classes = (JwtAuthentication,) permission_classes = (permissions.IsAuthenticated, CanRetireUser,) parser_classes = (JSONParser,) @@ -558,7 +544,7 @@ class AccountRetirementPartnerReportView(ViewSet): for enrollment in user.courseenrollment_set.all(): org = enrollment.course_id.org - # Org can conceivably be blank or this bogus default value + # Org can concievably be blank or this bogus default value if org and org != 'outdated_entry': orgs.add(org) try: @@ -583,9 +569,17 @@ class AccountRetirementPartnerReportView(ViewSet): is_being_processed=False ).order_by('id') - retirements = [] - for retirement_status in retirement_statuses: - retirements.append(self._get_retirement_for_partner_report(retirement_status)) + retirements = [ + { + 'user_id': retirement.user.pk, + 'original_username': retirement.original_username, + 'original_email': retirement.original_email, + 'original_name': retirement.original_name, + 'orgs': self._get_orgs_for_user(retirement.user), + 'created': retirement.created, + } + for retirement in retirement_statuses + ] serializer = UserRetirementPartnerReportSerializer(retirements, many=True) @@ -593,62 +587,6 @@ class AccountRetirementPartnerReportView(ViewSet): return Response(serializer.data) - def _get_retirement_for_partner_report(self, retirement_status): - """ - Get the retirement for this retirement_status. The retirement info will be included in the partner report. - """ - retirement = { - 'user_id': retirement_status.user.pk, - 'original_username': retirement_status.original_username, - AccountRetirementPartnerReportView.ORIGINAL_EMAIL_KEY: retirement_status.original_email, - AccountRetirementPartnerReportView.ORIGINAL_NAME_KEY: retirement_status.original_name, - 'orgs': self._get_orgs_for_user(retirement_status.user), - 'created': retirement_status.created, - } - - # Some orgs have a custom list of headings and content for the partner report. Add this, if applicable. - self._add_orgs_config_for_user(retirement, retirement_status.user) - - return retirement - - def _add_orgs_config_for_user(self, retirement, user): - """ - Check to see if the user's info was sent to any partners (orgs) that have a a custom list of headings and - content for the partner report. If so, add this. - """ - # See if the MicroBachelors coaching provider needs to be notified of this user's retirement - if has_ever_consented_to_coaching is not None and has_ever_consented_to_coaching(user): - # See if the user has a MicroBachelors external id. If not, they were never sent to the - # coaching provider. - external_ids = ExternalId.objects.filter( - user=user, - external_id_type__name=ExternalIdType.MICROBACHELORS_COACHING - ) - if external_ids.exists(): - # User has an external id. Add the additional info. - external_id = str(external_ids[0].external_user_id) - self._add_coaching_orgs_config(retirement, external_id) - - def _add_coaching_orgs_config(self, retirement, external_id): - """ - Add the orgs configuration for MicroBachelors coaching - """ - # Add the custom field headings - retirement[AccountRetirementPartnerReportView.ORGS_CONFIG_KEY] = [ - { - AccountRetirementPartnerReportView.ORGS_CONFIG_ORG_KEY: 'mb_coaching', - AccountRetirementPartnerReportView.ORGS_CONFIG_FIELD_HEADINGS_KEY: [ - AccountRetirementPartnerReportView.STUDENT_ID_KEY, - AccountRetirementPartnerReportView.ORIGINAL_EMAIL_KEY, - AccountRetirementPartnerReportView.ORIGINAL_NAME_KEY, - AccountRetirementPartnerReportView.DELETION_COMPLETED_KEY - ] - } - ] - - # Add the custom field value - retirement[AccountRetirementPartnerReportView.STUDENT_ID_KEY] = external_id - @request_requires_username def retirement_partner_status_create(self, request): """ @@ -769,7 +707,7 @@ class AccountRetirementStatusView(ViewSet): ) serializer = UserRetirementStatusSerializer(retirements, many=True) return Response(serializer.data) - # This should only occur on the int() conversion of cool_off_days at this point + # This should only occur on the int() converstion of cool_off_days at this point except ValueError: return Response('Invalid cool_off_days, should be integer.', status=status.HTTP_400_BAD_REQUEST) except KeyError as exc: @@ -1118,7 +1056,7 @@ class UsernameReplacementView(APIView): updates usernames across all services. DO NOT run this alone or users will not match across the system and things will be broken. - API will receive a list of current usernames and their requested new + API will recieve a list of current usernames and their requested new username. If their new username is taken, it will randomly assign a new username. This API will be called first, before calling the APIs in other services as this @@ -1228,7 +1166,7 @@ class UsernameReplacementView(APIView): """ Generates a unique username. If the desired username is available, that will be returned. - Otherwise it will generate unique suffixes to the desired username until it is an available username. + Otherwise it will generate unique suffixs to the desired username until it is an available username. """ new_username = desired_username # Keep checking usernames in case desired_username + random suffix is already taken