diff --git a/CHANGELOG.rst b/CHANGELOG.rst index eeb04908c7..0c327c738e 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -15,6 +15,17 @@ Change Log Unreleased ---------- +[4.16.4] +-------- +* revert: fix: set default langauge for all learners linked with an enteprise customer + +[4.16.3] +-------- +* fix: fixing the removal logic for EnterpriseGroupMemberships, adding optional flag + +[4.16.2] +--------- +* fix: fix the PATCH method of ``EnterpriseCourseEnrollmentView``. [4.16.1] --------- @@ -40,7 +51,6 @@ Unreleased -------- * fix: return a 404 response for inactive CSOD customers while fetching courses - [4.15.8] -------- * fix: SSO self-serve tool invalid entityId parsing diff --git a/enterprise/__init__.py b/enterprise/__init__.py index c8a6de1876..0ccfbd5bf6 100644 --- a/enterprise/__init__.py +++ b/enterprise/__init__.py @@ -2,4 +2,4 @@ Your project description goes here. """ -__version__ = "4.16.1" +__version__ = "4.16.4" diff --git a/enterprise/api/v1/views/enterprise_group.py b/enterprise/api/v1/views/enterprise_group.py index f15b48c229..cd25b3e83d 100644 --- a/enterprise/api/v1/views/enterprise_group.py +++ b/enterprise/api/v1/views/enterprise_group.py @@ -112,6 +112,7 @@ def get_learners(self, request, *args, **kwargs): are `memberDetails`, `memberStatus`, and `recentAction`. Ordering can be reversed by supplying a `-` at the beginning of the sorting value ie `-memberStatus`. - ``page`` (int, optional): Which page of paginated data to return. + - ``show_removed`` (bool, optional): Include removed learners in the response. Returns: Paginated list of learners that are associated with the enterprise group uuid:: @@ -137,6 +138,7 @@ def get_learners(self, request, *args, **kwargs): """ query_params = self.request.query_params.copy() is_reversed = bool(query_params.get('is_reversed', False)) + show_removed = bool(query_params.get('show_removed', False)) param_serializers = serializers.EnterpriseGroupLearnersRequestQuerySerializer( data=query_params @@ -155,7 +157,8 @@ def get_learners(self, request, *args, **kwargs): members = group_object.get_all_learners(user_query, sort_by, desc_order=is_reversed, - pending_users_only=pending_users_only) + pending_users_only=pending_users_only, + fetch_removed=show_removed) page = self.paginate_queryset(members) serializer = serializers.EnterpriseGroupMembershipSerializer(page, many=True) diff --git a/enterprise/models.py b/enterprise/models.py index 3963d13115..fffff4a64b 100644 --- a/enterprise/models.py +++ b/enterprise/models.py @@ -4338,9 +4338,9 @@ def _get_explicit_group_members(self, user_query=None, fetch_removed=False, pend """ Fetch explicitly defined members of a group, indicated by an existing membership record """ - members = self.members.select_related( - 'enterprise_customer_user', 'pending_enterprise_customer_user' - ) + # note: self.members doesn't seem to surface soft deleted items + members = EnterpriseGroupMembership.all_objects.filter( + group__uuid=self.uuid).select_related('enterprise_customer_user', 'pending_enterprise_customer_user') if not fetch_removed: members = members.filter(is_removed=False) if user_query: diff --git a/enterprise/signals.py b/enterprise/signals.py index 438a8e74b2..24f19003c8 100644 --- a/enterprise/signals.py +++ b/enterprise/signals.py @@ -16,7 +16,7 @@ NotConnectedToOpenEdX, get_default_catalog_content_filter, localized_utcnow, - set_enterprise_learner_language, + unset_enterprise_learner_language, ) from integrated_channels.blackboard.models import BlackboardEnterpriseCustomerConfiguration from integrated_channels.canvas.models import CanvasEnterpriseCustomerConfiguration @@ -104,15 +104,10 @@ def update_lang_pref_of_all_learners(sender, instance, **kwargs): # pylint: dis if instance.default_language: prev_state = models.EnterpriseCustomer.objects.filter(uuid=instance.uuid).first() if prev_state and prev_state.default_language != instance.default_language: - default_language = instance.default_language # Unset the language preference of all the learners linked with the enterprise customer. # The middleware in the enterprise will handle the cases for setting a proper language for the learner. - logger.info( - '[SET_ENT_LANG] Task triggered to update user preference.. Enterprise: [%s], Language: [%s]', - instance.uuid, - default_language - ) - update_enterprise_learners_user_preference.delay(instance.uuid, default_language) + logger.info('Task triggered to update user preference for learners. Enterprise: [%s]', instance.uuid) + update_enterprise_learners_user_preference.delay(instance.uuid) @receiver(pre_save, sender=models.EnterpriseCustomerBrandingConfiguration) @@ -212,7 +207,7 @@ def update_learner_language_preference(sender, instance, created, **kwargs): # Unset the language preference when a new learner is linked with the enterprise customer. # The middleware in the enterprise will handle the cases for setting a proper language for the learner. if created and instance.enterprise_customer.default_language: - set_enterprise_learner_language(instance) + unset_enterprise_learner_language(instance) @receiver(post_save, sender=models.PendingEnterpriseCustomerAdminUser) diff --git a/enterprise/tasks.py b/enterprise/tasks.py index befe385e48..f6646ad1ec 100644 --- a/enterprise/tasks.py +++ b/enterprise/tasks.py @@ -19,7 +19,7 @@ from enterprise.utils import ( get_enterprise_customer, send_email_notification_message, - set_language_of_all_enterprise_learners, + unset_language_of_all_enterprise_learners, ) LOGGER = getLogger(__name__) @@ -342,12 +342,11 @@ def send_group_membership_removal_notification(enterprise_customer_uuid, members @shared_task @set_code_owner_attribute -def update_enterprise_learners_user_preference(enterprise_customer_uuid, default_language): +def update_enterprise_learners_user_preference(enterprise_customer_uuid): """ Update the user preference `pref-lang` attribute for all enterprise learners linked with an enterprise. Arguments: * enterprise_customer_uuid (UUID): uuid of an enterprise customer - * default_language (str): new language code to set for all enterprise learners """ - set_language_of_all_enterprise_learners(enterprise_customer_uuid, default_language) + unset_language_of_all_enterprise_learners(enterprise_customer_uuid) diff --git a/enterprise/utils.py b/enterprise/utils.py index 502c203767..dae60e6713 100644 --- a/enterprise/utils.py +++ b/enterprise/utils.py @@ -2219,43 +2219,34 @@ def get_platform_logo_url(): return urljoin(settings.LMS_ROOT_URL, logo_url) -def set_language_of_all_enterprise_learners(enterprise_customer_uuid, default_language): +def unset_language_of_all_enterprise_learners(enterprise_customer_uuid): """ - Set the language preference of all the learners belonging to the given enterprise customer. + Unset the language preference of all the learners belonging to the given enterprise customer. Arguments: enterprise_customer_uuid (UUI): uuid of an enterprise customer - default_language (str): default language to set for all learners """ if UserPreference: enterprise_customer = get_enterprise_customer(enterprise_customer_uuid) user_ids = list(enterprise_customer.enterprise_customer_users.values_list('user_id', flat=True)) - LOGGER.info( - '[SET_ENT_LANG] Update user preference started for learners. Enterprise: [%s], Language: [%s]', - enterprise_customer_uuid, - default_language - ) + LOGGER.info('Update user preference started for learners. Enterprise: [%s]', enterprise_customer_uuid) for chunk in batch(user_ids, batch_size=10000): UserPreference.objects.filter( key=LANGUAGE_KEY, user_id__in=chunk ).update( - value=default_language + value='' ) - LOGGER.info('[SET_ENT_LANG] Updated user preference for learners. Batch Size: [%s]', len(chunk)) + LOGGER.info('Updated user preference for learners. Batch Size: [%s]', len(chunk)) - LOGGER.info( - '[SET_ENT_LANG] Update user preference completed for learners. Enterprise: [%s], Language: [%s]', - enterprise_customer_uuid, - default_language - ) + LOGGER.info('Update user preference completed for learners. Enterprise: [%s]', enterprise_customer_uuid) -def set_enterprise_learner_language(enterprise_customer_user): +def unset_enterprise_learner_language(enterprise_customer_user): """ - Set the language preference of the given enterprise learners. + Unset the language preference of the given enterprise learners. Arguments: enterprise_customer_user (EnterpriseCustomerUser): Instance of the enterprise customer user. @@ -2264,7 +2255,7 @@ def set_enterprise_learner_language(enterprise_customer_user): UserPreference.objects.update_or_create( key=LANGUAGE_KEY, user_id=enterprise_customer_user.user_id, - defaults={'value': enterprise_customer_user.enterprise_customer.default_language} + defaults={'value': ''} ) diff --git a/enterprise_learner_portal/api/v1/views.py b/enterprise_learner_portal/api/v1/views.py index b962fa4733..78d9d5a578 100644 --- a/enterprise_learner_portal/api/v1/views.py +++ b/enterprise_learner_portal/api/v1/views.py @@ -96,6 +96,8 @@ def get(self, request): user, list(filtered_enterprise_enrollments) ) + else: + course_enrollments_resume_urls = {} data = EnterpriseCourseEnrollmentSerializer( filtered_enterprise_enrollments, @@ -103,9 +105,7 @@ def get(self, request): context={ 'request': request, 'course_overviews': course_overviews, - 'course_enrollments_resume_urls': ( - course_enrollments_resume_urls if course_enrollments_resume_urls else None - ) + 'course_enrollments_resume_urls': course_enrollments_resume_urls, }, ).data @@ -158,13 +158,22 @@ def patch(self, request): # TODO: For now, this makes the change backward compatible, we will change this to true boolean support enterprise_enrollment.saved_for_later = saved_for_later.lower() == 'true' - enterprise_enrollment.save() course_overviews = get_course_overviews([course_id]) + + if get_resume_urls_for_course_enrollments and enterprise_enrollment.course_enrollment: + course_enrollments_resume_urls = get_resume_urls_for_course_enrollments(user, [enterprise_enrollment]) + else: + course_enrollments_resume_urls = {} + data = EnterpriseCourseEnrollmentSerializer( enterprise_enrollment, - context={'request': request, 'course_overviews': course_overviews}, + context={ + 'request': request, + 'course_overviews': course_overviews, + 'course_enrollments_resume_urls': course_enrollments_resume_urls, + }, ).data return Response(data) diff --git a/pytest.local.ini b/pytest.local.ini index b254bf0713..5bc8773e9f 100644 --- a/pytest.local.ini +++ b/pytest.local.ini @@ -4,4 +4,4 @@ [pytest] DJANGO_SETTINGS_MODULE = enterprise.settings.test addopts = --cov-report term-missing -W ignore -norecursedirs = .* docs requirements node_modules +testpaths = tests diff --git a/setup.cfg b/setup.cfg index aab24d6646..f5452e73dd 100644 --- a/setup.cfg +++ b/setup.cfg @@ -4,3 +4,6 @@ # W503,W504: warnings added since pep8/pycodestyle 1.5.7 that we haven't cleaned up yet ignore=E501,W503,W504 exclude=.git,.tox,enterprise/settings,enterprise/migrations,enterprise/static + +[flake8] +max-line-length = 120 diff --git a/tests/test_enterprise/api/test_views.py b/tests/test_enterprise/api/test_views.py index cce0931388..1e26ac2e14 100644 --- a/tests/test_enterprise/api/test_views.py +++ b/tests/test_enterprise/api/test_views.py @@ -7629,6 +7629,44 @@ def test_list_learners_filtered(self): 'pending_learner_id' ) == pending_membership.pending_enterprise_customer_user.id + def test_list_removed_learners(self): + group = EnterpriseGroupFactory( + enterprise_customer=self.enterprise_customer, + applies_to_all_contexts=False, + ) + memberships_to_delete = [] + membership = EnterpriseGroupMembershipFactory( + group=group, + pending_enterprise_customer_user=None, + enterprise_customer_user__enterprise_customer=self.enterprise_customer, + ) + memberships_to_delete.append(membership.enterprise_customer_user.user.email) + + # first we remove the membership + remove_url = settings.TEST_SERVER + reverse( + 'enterprise-group-remove-learners', + kwargs={'group_uuid': group.uuid}, + ) + request_data = {'learner_emails': memberships_to_delete} + response = self.client.post(remove_url, data=request_data) + + # then we're checking if the filter works + removed_users_query_string = '?show_removed=true' + url = settings.TEST_SERVER + reverse( + 'enterprise-group-learners', + kwargs={'group_uuid': group.uuid}, + ) + removed_users_query_string + response = self.client.get(url) + assert response.json().get('count') == 1 + + # but we're doing an extra check to make sure its not fetched normally + url = settings.TEST_SERVER + reverse( + 'enterprise-group-learners', + kwargs={'group_uuid': group.uuid}, + ) + response = self.client.get(url) + assert response.json().get('count') == 0 + def test_list_learners_sort_by(self): """ Test that the list learners endpoint can be sorted by 'recentAction', 'status', 'memberDetails' @@ -7978,7 +8016,7 @@ def test_successful_assign_learners_to_group(self, mock_send_group_membership_in def test_remove_learners_404(self): """ - Test that the remove learners endpoint properly handles no finding the provided group + Test that the remove learners endpoint properly handles not finding the provided group """ url = settings.TEST_SERVER + reverse( 'enterprise-group-remove-learners', diff --git a/tests/test_enterprise_learner_portal/api/test_views.py b/tests/test_enterprise_learner_portal/api/test_views.py index f43a297fd5..731be00246 100644 --- a/tests/test_enterprise_learner_portal/api/test_views.py +++ b/tests/test_enterprise_learner_portal/api/test_views.py @@ -238,15 +238,45 @@ def test_view_requires_openedx_installation(self): ) ) - @mock.patch('enterprise_learner_portal.api.v1.views.EnterpriseCourseEnrollmentSerializer') + @mock.patch('enterprise_learner_portal.api.v1.views.get_resume_urls_for_course_enrollments') @mock.patch('enterprise_learner_portal.api.v1.views.get_course_overviews') - def test_patch_success(self, mock_get_overviews, mock_serializer): + @mock.patch('enterprise_learner_portal.api.v1.serializers.get_course_run_status') + @mock.patch('enterprise_learner_portal.api.v1.serializers.get_emails_enabled') + @mock.patch('enterprise_learner_portal.api.v1.serializers.get_course_run_url') + @mock.patch('enterprise_learner_portal.api.v1.serializers.get_certificate_for_user') + def test_patch_success( + self, + mock_get_cert, + mock_get_course_run_url, + mock_get_emails_enabled, + mock_get_course_run_status, + mock_get_overviews, + mock_get_resume_urls, + ): """ View should update the enrollment's saved_for_later field and return serialized data from EnterpriseCourseEnrollmentSerializer for the enrollment (which we mock in this case) """ - mock_get_overviews.return_value = {'overview_info': 'this would be a larger dict'} - mock_serializer.return_value = self.MockSerializer() + mock_get_overviews.return_value = [{ + 'id': self.course_run_id, + 'start': 'a datetime object', + 'end': 'a datetime object', + 'display_name_with_default': 'a default name', + 'pacing': 'instructor', + 'display_org_with_default': 'my university', + }] + mock_get_resume_urls.return_value = { + self.course_run_id: '/courses/course-v1:MITx+6.86x+2T2024' + } + mock_get_cert.return_value = { + 'download_url': 'example.com', + 'is_passing': True, + 'created': 'a datetime object', + } + mock_get_course_run_url.return_value = 'course-run-url' + mock_get_emails_enabled.return_value = True + mock_get_course_run_status.return_value = 'completed' + query_params = { 'enterprise_id': str(self.enterprise_customer.uuid), 'course_id': self.course_run_id, @@ -255,18 +285,25 @@ def test_patch_success(self, mock_get_overviews, mock_serializer): assert not self.enterprise_enrollment.saved_for_later - resp = self.client.patch( - '{host}{path}?{query_params}'.format( - host=settings.TEST_SERVER, - path=reverse('enterprise-learner-portal-course-enrollment-list'), - query_params=urlencode(query_params), + with mock.patch( + 'enterprise.models.EnterpriseCourseEnrollment.course_enrollment', + new_callable=mock.PropertyMock, + ) as mock_course_enrollment: + mock_course_enrollment.return_value = mock.Mock( + is_active=True, + mode='verified', + ) + resp = self.client.patch( + '{host}{path}?{query_params}'.format( + host=settings.TEST_SERVER, + path=reverse('enterprise-learner-portal-course-enrollment-list'), + query_params=urlencode(query_params), + ) ) - ) assert resp.status_code == 200 - assert resp.json() == [ - SERIALIZED_MOCK_ACTIVE_ENROLLMENT, - SERIALIZED_MOCK_INACTIVE_ENROLLMENT, - ] + updated_record = resp.json() + assert updated_record['course_run_id'] == self.course_run_id + assert '/courses/course-v1:MITx+6.86x+2T2024' in updated_record['resume_course_run_url'] self.enterprise_enrollment.refresh_from_db() assert self.enterprise_enrollment.saved_for_later diff --git a/tests/test_models.py b/tests/test_models.py index f0e8f15db7..558652b289 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -332,9 +332,9 @@ def test_catalog_contains_course_with_enterprise_customer_catalog(self, api_clie wraps=update_enterprise_learners_user_preference ) @mock.patch('enterprise.utils.UserPreference', return_value=mock.MagicMock()) - def test_set_language_of_all_enterprise_learners(self, user_preference_mock, mock_task): + def test_unset_language_of_all_enterprise_learners(self, user_preference_mock, mock_task): """ - Validate that set_language_of_all_enterprise_learners is called whenever default_language changes. + Validate that unset_language_of_all_enterprise_learners is called whenever default_language changes. """ user = factories.UserFactory(email='user123@example.com') enterprise_customer = factories.EnterpriseCustomerFactory() @@ -345,24 +345,22 @@ def test_set_language_of_all_enterprise_learners(self, user_preference_mock, moc enterprise_customer.default_language = 'es-419' enterprise_customer.save() mock_task.assert_called_once() - user_preference_mock.objects.filter.return_value.update.assert_called_with(value='es-419') user_preference_mock.objects.filter.assert_called_once() - # Make sure `set_language_of_all_enterprise_learners` is called each time `default_language` changes. + # Make sure `unset_language_of_all_enterprise_learners` is called each time `default_language` changes. enterprise_customer.default_language = 'es-417' enterprise_customer.save() assert mock_task.call_count == 2 - user_preference_mock.objects.filter.return_value.update.assert_called_with(value='es-417') assert user_preference_mock.objects.filter.call_count == 2 - # make sure `set_language_of_all_enterprise_learners` is not called if `default_language` is + # make sure `unset_language_of_all_enterprise_learners` is not called if `default_language` is # not changed. enterprise_customer.default_language = 'es-417' enterprise_customer.save() assert mock_task.call_count == 2 assert user_preference_mock.objects.filter.call_count == 2 - # Make sure `set_language_of_all_enterprise_learners` is not called if `default_language` is + # Make sure `unset_language_of_all_enterprise_learners` is not called if `default_language` is # set to `None`. enterprise_customer.default_language = None enterprise_customer.save() @@ -1008,20 +1006,15 @@ def test_soft_delete(self, method_name): ) @mock.patch('enterprise.utils.UserPreference', return_value=mock.MagicMock()) - def test_set_enterprise_learner_language(self, user_preference_mock): + def test_unset_enterprise_learner_language(self, user_preference_mock): """ - Validate that set_enterprise_learner_language is called whenever a nwe learner is linked to an enterprise + Validate that unset_enterprise_learner_language is called whenever a nwe learner is linked to an enterprise customer with Non-Null default language. """ enterprise_customer_user = factories.EnterpriseCustomerUserFactory() user_preference_mock.objects.update_or_create.assert_called_once() - user_preference_mock.objects.update_or_create.assert_called_with( - key='pref-lang', - user_id=enterprise_customer_user.user_id, - defaults={'value': 'en'} - ) - # Make sure `set_enterprise_learner_language` is not called if the enterprise customer does not have a + # Make sure `unset_enterprise_learner_language` is not called if the enterprise customer does not have a # default_language. enterprise_customer_user.enterprise_customer.default_language = None enterprise_customer_user.enterprise_customer.save()