From 4f74d8a16a1162d232185dcc98d5527516d3b318 Mon Sep 17 00:00:00 2001 From: jawad khan Date: Mon, 26 Sep 2022 14:27:23 +0500 Subject: [PATCH] feat: added mobile configs in enrollments api (#31036) * feat: added mobile configs in enrollments api Mobile team needs some cnfigs after user has logged in, enrollment api seems to be the best place to put this data. They can change these flags or values from admin side. LEARNER-9039 --- lms/djangoapps/mobile_api/admin.py | 13 +++- .../migrations/0004_mobileconfig.py | 25 +++++++ lms/djangoapps/mobile_api/models.py | 38 +++++++++- lms/djangoapps/mobile_api/tests/test_model.py | 37 +++++++++- lms/djangoapps/mobile_api/users/tests.py | 69 +++++++++++++++---- lms/djangoapps/mobile_api/users/views.py | 16 ++++- lms/djangoapps/mobile_api/utils.py | 1 + lms/urls.py | 2 +- 8 files changed, 183 insertions(+), 18 deletions(-) create mode 100644 lms/djangoapps/mobile_api/migrations/0004_mobileconfig.py diff --git a/lms/djangoapps/mobile_api/admin.py b/lms/djangoapps/mobile_api/admin.py index fde951f309..02cdbf8d8a 100644 --- a/lms/djangoapps/mobile_api/admin.py +++ b/lms/djangoapps/mobile_api/admin.py @@ -6,7 +6,7 @@ Django admin dashboard configuration for LMS XBlock infrastructure. from config_models.admin import ConfigurationModelAdmin from django.contrib import admin -from .models import AppVersionConfig, IgnoreMobileAvailableFlagConfig, MobileApiConfig +from .models import AppVersionConfig, IgnoreMobileAvailableFlagConfig, MobileApiConfig, MobileConfig admin.site.register(MobileApiConfig, ConfigurationModelAdmin) admin.site.register(IgnoreMobileAvailableFlagConfig, ConfigurationModelAdmin) @@ -24,4 +24,15 @@ class AppVersionConfigAdmin(admin.ModelAdmin): """ defines fields to display in list view """ return ['platform', 'version', 'expire_at', 'enabled', 'created_at', 'updated_at'] + +class MobileConfigAdmin(admin.ModelAdmin): + """ Admin class for MobileConfig model """ + fields = ('name', 'value') + readonly_fields = ['created', 'modified'] + + class Meta: + ordering = ['name'] + + admin.site.register(AppVersionConfig, AppVersionConfigAdmin) +admin.site.register(MobileConfig, MobileConfigAdmin) diff --git a/lms/djangoapps/mobile_api/migrations/0004_mobileconfig.py b/lms/djangoapps/mobile_api/migrations/0004_mobileconfig.py new file mode 100644 index 0000000000..f554a6716f --- /dev/null +++ b/lms/djangoapps/mobile_api/migrations/0004_mobileconfig.py @@ -0,0 +1,25 @@ +# Generated by Django 3.2.15 on 2022-09-23 14:17 + +from django.db import migrations, models +import django.utils.timezone +import model_utils.fields + + +class Migration(migrations.Migration): + + dependencies = [ + ('mobile_api', '0003_ignore_mobile_available_flag'), + ] + + operations = [ + migrations.CreateModel( + name='MobileConfig', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created', model_utils.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False, verbose_name='created')), + ('modified', model_utils.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False, verbose_name='modified')), + ('name', models.CharField(max_length=255)), + ('value', models.CharField(max_length=255)), + ], + ), + ] diff --git a/lms/djangoapps/mobile_api/models.py b/lms/djangoapps/mobile_api/models.py index d9e640e03c..f2a912e78c 100644 --- a/lms/djangoapps/mobile_api/models.py +++ b/lms/djangoapps/mobile_api/models.py @@ -5,7 +5,7 @@ ConfigurationModel for the mobile_api djangoapp. from config_models.models import ConfigurationModel from django.db import models - +from model_utils.models import TimeStampedModel from . import utils from .mobile_platform import PLATFORM_CLASSES @@ -101,3 +101,39 @@ class IgnoreMobileAvailableFlagConfig(ConfigurationModel): class Meta: app_label = "mobile_api" + + +class MobileConfig(TimeStampedModel): + """ + Mobile configs to add through admin panel. Config values can be added dynamically. + + .. no_pii: + """ + name = models.CharField(max_length=255) + value = models.CharField(max_length=255) + + class Meta: + app_label = "mobile_api" + + def __str__(self): + return self.name + + @classmethod + def get_structured_configs(cls): + """ + Add config values in the following manner: + - If flag name starts with `iap_`, add value to configs['iap_configs'] + - Else add values to configs{} + """ + configs = MobileConfig.objects.all().values('name', 'value') + structured_configs = {"iap_configs": {}} + for config in configs: + name = config.get('name') + value = config.get('value') + + if name.startswith('iap_'): + structured_configs['iap_configs'][name] = value + else: + structured_configs[name] = value + + return structured_configs diff --git a/lms/djangoapps/mobile_api/tests/test_model.py b/lms/djangoapps/mobile_api/tests/test_model.py index c63d0f271e..6c2b324817 100644 --- a/lms/djangoapps/mobile_api/tests/test_model.py +++ b/lms/djangoapps/mobile_api/tests/test_model.py @@ -9,7 +9,7 @@ import ddt from django.test import TestCase from pytz import UTC -from lms.djangoapps.mobile_api.models import AppVersionConfig, MobileApiConfig +from lms.djangoapps.mobile_api.models import AppVersionConfig, MobileApiConfig, MobileConfig @ddt.ddt @@ -109,3 +109,38 @@ class TestMobileApiConfig(TestCase): MobileApiConfig(video_profiles="").save() video_profile_list = MobileApiConfig.get_video_profiles() assert video_profile_list == [] + + +class TestMobileConfig(TestCase): + """ + Tests MobileConfig + """ + + def test_structured_configs(self): + """Check that configs are structured properly""" + MobileConfig(name="simple config", value="simple").save() + MobileConfig(name="iap config", value="false iap").save() + MobileConfig(name="iap_config", value="true").save() + MobileConfig(name="", value="empty").save() + configs = MobileConfig.get_structured_configs() + expected_result = { + 'iap_configs': {'iap_config': 'true'}, + 'simple config': 'simple', + 'iap config': 'false iap', + '': 'empty'} + + self.assertDictEqual(configs, expected_result) + + def test_structured_configs_without_iap_configs(self): + """Check that configs are structured properly without iap configs""" + MobileConfig(name="simple config", value="simple").save() + MobileConfig(name="iap config", value="false iap").save() + MobileConfig(name="", value="empty").save() + configs = MobileConfig.get_structured_configs() + expected_result = { + 'iap_configs': {}, + 'simple config': 'simple', + 'iap config': 'false iap', + '': 'empty'} + + self.assertDictEqual(configs, expected_result) diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index d1b8a38703..9358966468 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -27,13 +27,14 @@ from common.djangoapps.util.testing import UrlResetMixin from lms.djangoapps.certificates.data import CertificateStatuses from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory from lms.djangoapps.courseware.access_response import MilestoneAccessError, StartDateError, VisibilityError +from lms.djangoapps.mobile_api.models import MobileConfig from lms.djangoapps.mobile_api.testutils import ( MobileAPITestCase, MobileAuthTestMixin, MobileAuthUserTestMixin, MobileCourseAccessTestMixin ) -from lms.djangoapps.mobile_api.utils import API_V1, API_V05 +from lms.djangoapps.mobile_api.utils import API_V1, API_V05, API_V2 from openedx.core.lib.courses import course_image_url from openedx.features.course_duration_limits.models import CourseDurationLimitConfig from openedx.features.course_experience.tests.views.helpers import add_course_mode @@ -144,7 +145,7 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest assert len(courses) == 0 @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True}) - @ddt.data(API_V05, API_V1) + @ddt.data(API_V05, API_V1, API_V2) def test_sort_order(self, api_version): self.login() @@ -156,11 +157,13 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest # verify courses are returned in the order of enrollment, with most recently enrolled first. response = self.api_response(api_version=api_version) + enrollments = response.data['enrollments'] if api_version == API_V2 else response.data + for course_index in range(num_courses): - assert response.data[course_index]['course']['id'] ==\ + assert enrollments[course_index]['course']['id'] ==\ str(courses[((num_courses - course_index) - 1)].id) - @ddt.data(API_V05, API_V1) + @ddt.data(API_V05, API_V1, API_V2) @patch.dict(settings.FEATURES, { 'ENABLE_PREREQUISITE_COURSES': True, 'DISABLE_START_DATES': False, @@ -194,8 +197,10 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest # Verify courses have the correct response through error code. Last enrolled course is first course in response response = self.api_response(api_version=api_version) + enrollments = response.data['enrollments'] if api_version == API_V2 else response.data + for course_index in range(len(courses)): - result = response.data[course_index]['course']['courseware_access'] + result = enrollments[course_index]['course']['courseware_access'] assert result['error_code'] == expected_error_codes[::(- 1)][course_index] if result['error_code'] is not None: @@ -204,16 +209,22 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest @ddt.data( ('next_week', ADVERTISED_START, ADVERTISED_START, "string", API_V05), ('next_week', ADVERTISED_START, ADVERTISED_START, "string", API_V1), + ('next_week', ADVERTISED_START, ADVERTISED_START, "string", API_V2), ('next_week', None, defaultfilters.date(NEXT_WEEK, "DATE_FORMAT"), "timestamp", API_V05), ('next_week', None, defaultfilters.date(NEXT_WEEK, "DATE_FORMAT"), "timestamp", API_V1), + ('next_week', None, defaultfilters.date(NEXT_WEEK, "DATE_FORMAT"), "timestamp", API_V2), ('next_week', '', defaultfilters.date(NEXT_WEEK, "DATE_FORMAT"), "timestamp", API_V05), ('next_week', '', defaultfilters.date(NEXT_WEEK, "DATE_FORMAT"), "timestamp", API_V1), + ('next_week', '', defaultfilters.date(NEXT_WEEK, "DATE_FORMAT"), "timestamp", API_V2), ('default_start_date', ADVERTISED_START, ADVERTISED_START, "string", API_V05), ('default_start_date', ADVERTISED_START, ADVERTISED_START, "string", API_V1), + ('default_start_date', ADVERTISED_START, ADVERTISED_START, "string", API_V2), ('default_start_date', '', None, "empty", API_V05), ('default_start_date', '', None, "empty", API_V1), + ('default_start_date', '', None, "empty", API_V2), ('default_start_date', None, None, "empty", API_V05), ('default_start_date', None, None, "empty", API_V1), + ('default_start_date', None, None, "empty", API_V2), ) @ddt.unpack @patch.dict(settings.FEATURES, {'DISABLE_START_DATES': False, 'ENABLE_MKTG_SITE': True}) @@ -227,19 +238,21 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest self.enroll(course.id) response = self.api_response(api_version=api_version) - assert response.data[0]['course']['start_type'] == expected_type - assert response.data[0]['course']['start_display'] == expected_display + courses = response.data['enrollments'] if api_version == API_V2 else response.data + assert courses[0]['course']['start_type'] == expected_type + assert courses[0]['course']['start_display'] == expected_display - @ddt.data(API_V05, API_V1) + @ddt.data(API_V05, API_V1, API_V2) @patch.dict(settings.FEATURES, {"ENABLE_DISCUSSION_SERVICE": True, 'ENABLE_MKTG_SITE': True}) def test_discussion_url(self, api_version): self.login_and_enroll() response = self.api_response(api_version=api_version) - response_discussion_url = response.data[0]['course']['discussion_url'] + courses = response.data['enrollments'] if api_version == API_V2 else response.data + response_discussion_url = courses[0]['course']['discussion_url'] assert f'/api/discussion/v1/courses/{self.course.id}' in response_discussion_url - @ddt.data(API_V05, API_V1) + @ddt.data(API_V05, API_V1, API_V2) def test_org_query(self, api_version): self.login() @@ -258,12 +271,13 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest self.enroll(course.id) response = self.api_response(data={'org': 'edX'}, api_version=api_version) + courses = response.data['enrollments'] if api_version == API_V2 else response.data # Test for 3 expected courses - assert len(response.data) == 3 + assert len(courses) == 3 # Verify only edX courses are returned - for entry in response.data: + for entry in courses: assert entry['course']['org'] == 'edX' def create_enrollment(self, expired): @@ -286,9 +300,15 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest add_course_mode(course) def _get_enrollment_data(self, api_version, expired): + """ + Login, Create enrollments and get data through enrollments api. + """ self.login() self.create_enrollment(expired) - return self.api_response(api_version=api_version).data + response = self.api_response(api_version=api_version).data + result = response['enrollments'] if api_version == API_V2 else response + + return result def _assert_enrollment_results(self, api_version, courses, num_courses_returned, gating_enabled=True): # lint-amnesty, pylint: disable=missing-function-docstring assert len(courses) == num_courses_returned @@ -306,6 +326,8 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest (API_V05, False, 1), (API_V1, True, 1), (API_V1, False, 1), + (API_V2, True, 1), + (API_V2, False, 1), ) @ddt.unpack def test_enrollment_with_gating(self, api_version, expired, num_courses_returned): @@ -322,6 +344,8 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest (API_V05, False, 1), (API_V1, True, 1), (API_V1, False, 1), + (API_V2, True, 1), + (API_V2, False, 1), ) @ddt.unpack def test_enrollment_no_gating(self, api_version, expired, num_courses_returned): @@ -333,6 +357,25 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest courses = self._get_enrollment_data(api_version, expired) self._assert_enrollment_results(api_version, courses, num_courses_returned, False) + def test_enrollment_with_configs(self): + """ + Test that configs are returned in proper structure in enrollments api. + """ + self.login_and_enroll() + + MobileConfig(name='simple config', value='simple').save() + MobileConfig(name='iap_config', value='iap').save() + MobileConfig(name='iap config', value='false iap').save() + expected_result = { + 'iap_configs': {'iap_config': 'iap'}, + 'simple config': 'simple', + 'iap config': 'false iap', + } + + response = self.api_response(api_version=API_V2) + self.assertDictEqual(response.data['configs'], expected_result) + assert 'enrollments' in response.data + @override_settings(MKTG_URLS={'ROOT': 'dummy-root'}) class TestUserEnrollmentCertificates(UrlResetMixin, MobileAPITestCase, MilestonesTestCaseMixin): diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index c3272e8a1b..a0c00a98df 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -27,7 +27,8 @@ from lms.djangoapps.courseware.courses import get_current_child from lms.djangoapps.courseware.model_data import FieldDataCache from lms.djangoapps.courseware.module_render import get_module_for_descriptor from lms.djangoapps.courseware.views.index import save_positions_recursively_up -from lms.djangoapps.mobile_api.utils import API_V1, API_V05 +from lms.djangoapps.mobile_api.models import MobileConfig +from lms.djangoapps.mobile_api.utils import API_V1, API_V05, API_V2 from openedx.features.course_duration_limits.access import check_course_expired from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order @@ -360,6 +361,19 @@ class UserCourseEnrollmentsList(generics.ListAPIView): # return all courses, with associated expiration return list(mobile_available) + def list(self, request, *args, **kwargs): + response = super().list(request, *args, **kwargs) + api_version = self.kwargs.get('api_version') + + if api_version == API_V2: + enrollment_data = { + 'configs': MobileConfig.get_structured_configs(), + 'enrollments': response.data + } + return Response(enrollment_data) + + return response + @api_view(["GET"]) @mobile_view() diff --git a/lms/djangoapps/mobile_api/utils.py b/lms/djangoapps/mobile_api/utils.py index e674df8ec6..5dcdf9e04c 100644 --- a/lms/djangoapps/mobile_api/utils.py +++ b/lms/djangoapps/mobile_api/utils.py @@ -4,6 +4,7 @@ Common utility methods for Mobile APIs. API_V05 = 'v0.5' API_V1 = 'v1' +API_V2 = 'v2' def parsed_version(version): diff --git a/lms/urls.py b/lms/urls.py index 7526a310af..f02b1ecadb 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -215,7 +215,7 @@ urlpatterns = [ if settings.FEATURES.get('ENABLE_MOBILE_REST_API'): urlpatterns += [ - re_path(r'^api/mobile/(?Pv(1|0.5))/', include('lms.djangoapps.mobile_api.urls')), + re_path(r'^api/mobile/(?Pv(2|1|0.5))/', include('lms.djangoapps.mobile_api.urls')), ] if settings.FEATURES.get('ENABLE_OPENBADGES'):