From e03f8a7c17192f5ec2876f0075fe81c227690669 Mon Sep 17 00:00:00 2001 From: Muhammad Afaq Shuaib Date: Tue, 23 Jul 2024 21:16:29 +0500 Subject: [PATCH] feat: add CRUD operations for taxi form (#4385) --- course_discovery/apps/api/serializers.py | 75 +++++++-- .../apps/api/tests/test_serializers.py | 1 + .../api/v1/tests/test_views/test_courses.py | 154 +++++++++++++++++- .../apps/course_metadata/admin.py | 1 + .../apps/course_metadata/models.py | 2 +- 5 files changed, 214 insertions(+), 19 deletions(-) diff --git a/course_discovery/apps/api/serializers.py b/course_discovery/apps/api/serializers.py index 69ec718c3c..11a54f8eee 100644 --- a/course_discovery/apps/api/serializers.py +++ b/course_discovery/apps/api/serializers.py @@ -688,6 +688,13 @@ class Meta: fields = ('title', 'description', 'keywords') +class TaxiFormSerializer(BaseModelSerializer): + """Serializer for the ``TaxiForm`` model.""" + class Meta: + model = TaxiForm + fields = ('form_id', 'grouping', 'title', 'subtitle', 'post_submit_url') + + class AdditionalMetadataSerializer(BaseModelSerializer): """Serializer for the ``AdditionalMetadata`` model.""" @@ -698,10 +705,11 @@ class AdditionalMetadataSerializer(BaseModelSerializer): end_date = serializers.DateTimeField() registration_deadline = serializers.DateTimeField(allow_null=True) variant_id = serializers.UUIDField(allow_null=True) + taxi_form = TaxiFormSerializer(required=False, allow_null=True) @classmethod def prefetch_queryset(cls): - return AdditionalMetadata.objects.select_related('facts', 'certificate_info', 'product_meta') + return AdditionalMetadata.objects.select_related('facts', 'certificate_info', 'product_meta', 'taxi_form') class Meta: model = AdditionalMetadata @@ -709,9 +717,20 @@ class Meta: 'external_identifier', 'external_url', 'lead_capture_form_url', 'facts', 'certificate_info', 'organic_url', 'start_date', 'end_date', 'registration_deadline', 'variant_id', 'course_term_override', 'product_status', - 'product_meta', 'external_course_marketing_type', 'display_on_org_page', + 'product_meta', 'external_course_marketing_type', 'display_on_org_page', 'taxi_form', ) + def update_taxi_form(self, instance, taxi_form): + if instance.taxi_form: + if not taxi_form: + instance.taxi_form = None + else: + TaxiForm.objects.filter( + id=instance.taxi_form.id + ).update(**taxi_form) + elif taxi_form: + instance.taxi_form = TaxiForm.objects.create(**taxi_form) + def _update_product_meta(self, instance, product_meta): if instance.product_meta: ProductMeta.objects.filter( @@ -721,22 +740,32 @@ def _update_product_meta(self, instance, product_meta): **product_meta) def update(self, instance, validated_data): - # Handle writing nested fields separately - if 'product_meta' in validated_data: + """ + Update the AdditionalMetadata instance with the validated data along with handling + of the nested fields separately. + """ + def handle_product_meta(instance, validated_data): # Handle product metadata only for 2U Executive Education courses else just pop - product_meta_data = validated_data.pop( - 'product_meta') - if instance: - self._update_product_meta( - instance, product_meta_data) - return super().update(instance, validated_data) + if 'product_meta' in validated_data: + product_meta_data = validated_data.pop('product_meta') + if instance: + self._update_product_meta(instance, product_meta_data) -class TaxiFormSerializer(BaseModelSerializer): - """Serializer for the ``TaxiForm`` model.""" - class Meta: - model = TaxiForm - fields = ('form_id', 'grouping', 'title', 'subtitle', 'post_submit_url') + return validated_data + + def handle_taxi_form(instance, validated_data): + if 'taxi_form' in validated_data: + taxi_form_data = validated_data.pop('taxi_form') + + if instance: + self.update_taxi_form(instance, taxi_form_data) + + return validated_data + + validated_data = handle_product_meta(instance, validated_data) + validated_data = handle_taxi_form(instance, validated_data) + return super().update(instance, validated_data) class DegreeAdditionalMetadataSerializer(BaseModelSerializer): @@ -1488,12 +1517,24 @@ def update_product_meta(self, instance, product_meta_data): return instance.product_meta, changed + def update_taxi_form(self, instance, taxi_form): + changed = False + if instance.taxi_form: + _, changed = update_instance(instance.taxi_form, taxi_form, True) + elif taxi_form: + instance.taxi_form = TaxiForm.objects.create(**taxi_form) + instance.save() + changed = True + + return instance.taxi_form, changed + def update_additional_metadata(self, instance, additional_metadata): changed = False facts = additional_metadata.pop('facts', None) certificate_info = additional_metadata.pop('certificate_info', None) product_meta = additional_metadata.pop('product_meta', None) + taxi_form = additional_metadata.pop('taxi_form', None) if instance.additional_metadata: _, additional_metadata_changed = update_instance(instance.additional_metadata, additional_metadata, True) @@ -1513,6 +1554,10 @@ def update_additional_metadata(self, instance, additional_metadata): _, certification_info_changed = self.update_certificate_info(instance.additional_metadata, certificate_info) changed = changed or certification_info_changed + if taxi_form: + _, taxi_form_changed = self.update_taxi_form(instance.additional_metadata, taxi_form) + changed = changed or taxi_form_changed + return changed # save() will be called by main update() diff --git a/course_discovery/apps/api/tests/test_serializers.py b/course_discovery/apps/api/tests/test_serializers.py index ec023e9adf..903623f2a9 100644 --- a/course_discovery/apps/api/tests/test_serializers.py +++ b/course_discovery/apps/api/tests/test_serializers.py @@ -2059,6 +2059,7 @@ def test_data(self): 'product_status': additional_metadata.product_status, 'external_course_marketing_type': additional_metadata.external_course_marketing_type, 'display_on_org_page': additional_metadata.display_on_org_page, + 'taxi_form': TaxiFormSerializer(additional_metadata.taxi_form).data, } assert serializer.data == expected diff --git a/course_discovery/apps/api/v1/tests/test_views/test_courses.py b/course_discovery/apps/api/v1/tests/test_views/test_courses.py index 8217afeb8a..4349d7fea9 100644 --- a/course_discovery/apps/api/v1/tests/test_views/test_courses.py +++ b/course_discovery/apps/api/v1/tests/test_views/test_courses.py @@ -26,7 +26,7 @@ from course_discovery.apps.course_metadata.models import ( AbstractLocationRestrictionModel, AdditionalMetadata, CertificateInfo, Course, CourseEditor, CourseEntitlement, CourseLocationRestriction, CourseRun, CourseRunType, CourseType, Fact, GeoLocation, ProductMeta, ProductValue, - RestrictedCourseRun, Seat, Source + RestrictedCourseRun, Seat, Source, TaxiForm ) from course_discovery.apps.course_metadata.signals import ( additional_metadata_facts_changed, connect_course_data_modified_timestamp_signal_handlers, @@ -1306,6 +1306,7 @@ def test_update_with_additional_metadata_if_type_2U(self, type_slug): 'product_meta': None, 'external_course_marketing_type': None, 'display_on_org_page': True, + 'taxi_form': None, } url = reverse('api:v1:course-detail', kwargs={'key': course.uuid}) course_data = { @@ -1418,6 +1419,7 @@ def test_update_facts_with_additional_metadata(self): # pylint: disable=too-man 'product_meta': None, 'external_course_marketing_type': None, 'display_on_org_page': True, + 'taxi_form': None, } additional_metadata_1 = { **additional_metadata, @@ -1583,6 +1585,7 @@ def test_update_product_meta_with_additional_metadata(self): 'product_meta': product_meta, 'external_course_marketing_type': None, 'display_on_org_page': True, + 'taxi_form': None, } url = reverse('api:v1:course-detail', kwargs={'key': course.uuid}) @@ -1596,7 +1599,6 @@ def test_update_product_meta_with_additional_metadata(self): assert ProductMeta.objects.count() == 2 course = Course.everything.get(uuid=course.uuid, draft=True) - self.assertDictEqual(self.serialize_course(course)['additional_metadata'], additional_metadata) self.assertDictEqual(self.serialize_course(course)['additional_metadata']['product_meta'], product_meta) assert previous_data_modified_timestamp < course.data_modified_timestamp @@ -1609,7 +1611,7 @@ def test_update_product_meta_with_additional_metadata(self): response = self.client.patch( url, {'additional_metadata': {'product_meta': product_meta}}, format='json' ) - additional_metadata['product_meta'] = product_meta + assert additional_metadata['product_meta'] == product_meta assert response.status_code == 200 course = Course.everything.get(uuid=course.uuid, draft=True) @@ -1632,6 +1634,152 @@ def test_update_product_meta_with_additional_metadata(self): pre_save.disconnect(data_modified_timestamp_update, ProductMeta) m2m_changed.disconnect(product_meta_taggable_changed, ProductMeta.keywords.through) + @responses.activate + def test_update_taxi_form_with_additional_metadata(self): + """ Verify that the taxi_form is updated when additional_metadata is updated. """ + pre_save.connect(data_modified_timestamp_update, TaxiForm) + current = datetime.datetime.now(pytz.UTC) + EE_type_2U = CourseTypeFactory(slug=CourseType.EXECUTIVE_EDUCATION_2U) + course = CourseFactory(additional_metadata=None, type=EE_type_2U) + previous_data_modified_timestamp = course.data_modified_timestamp + + taxi_form = { + "form_id": "12344", + "grouping": "test-grouping", + "title": course.title, + "subtitle": "hey you", + "post_submit_url": "http://www.edx.org", + } + + additional_metadata = { + 'external_url': 'https://example.com/456', + 'external_identifier': '456', + 'lead_capture_form_url': 'https://example.com/lead-capture', + 'organic_url': 'https://example.com/organic', + 'facts': [{'heading': 'Fact1 heading', 'blurb': '

Fact1 blurb

'}], + 'certificate_info': { + 'heading': 'Certificate heading', + 'blurb': '

Certificate blurb

', + }, + 'start_date': serialize_datetime(current), + 'end_date': serialize_datetime(current + datetime.timedelta(days=10)), + 'registration_deadline': serialize_datetime(current), + 'variant_id': str(uuid4()), + 'course_term_override': 'Example Program', + 'product_status': 'published', + 'product_meta': None, + 'external_course_marketing_type': None, + 'display_on_org_page': True, + 'taxi_form': taxi_form, + } + + url = reverse('api:v1:course-detail', kwargs={'key': course.uuid}) + response = self.client.patch(url, {'additional_metadata': additional_metadata}, format='json') + assert response.status_code == 200 + + assert TaxiForm.objects.get(form_id=taxi_form['form_id']) is not None + + course = Course.everything.get(uuid=course.uuid, draft=True) + self.assertDictEqual(self.serialize_course(course)['additional_metadata'], additional_metadata) + self.assertDictEqual(self.serialize_course(course)['additional_metadata']['taxi_form'], taxi_form) + assert previous_data_modified_timestamp < course.data_modified_timestamp + + previous_data_modified_timestamp = course.data_modified_timestamp + taxi_form['title'] = 'New title' + response = self.client.patch( + url, {'additional_metadata': {'taxi_form': taxi_form}}, format='json' + ) + additional_metadata['taxi_form'] = taxi_form + + assert response.status_code == 200 + course = Course.everything.get(uuid=course.uuid, draft=True) + + self.assertDictEqual(self.serialize_course(course)['additional_metadata'], additional_metadata) + self.assertDictEqual(self.serialize_course(course)['additional_metadata']['taxi_form'], taxi_form) + course.refresh_from_db(fields=('data_modified_timestamp',)) + + # If there is no taxi form change, the timestamp won't be updated. + previous_data_modified_timestamp = course.data_modified_timestamp + response = self.client.patch( + url, {'additional_metadata': {'taxi_form': taxi_form}}, format='json' + ) + additional_metadata['taxi_form'] = taxi_form + assert response.status_code == 200 + course = Course.everything.get(uuid=course.uuid, draft=True) + assert previous_data_modified_timestamp == course.data_modified_timestamp + + pre_save.disconnect(data_modified_timestamp_update, TaxiForm) + + @responses.activate + def test_update_additional_metadata__taxi_form_removal(self): + """ Verify that the taxi_form removed when additional_metadata is updated. """ + pre_save.connect(data_modified_timestamp_update, TaxiForm) + current = datetime.datetime.now(pytz.UTC) + EE_type_2U = CourseTypeFactory(slug=CourseType.EXECUTIVE_EDUCATION_2U) + course = CourseFactory(additional_metadata=None, type=EE_type_2U) + previous_data_modified_timestamp = course.data_modified_timestamp + + taxi_form = { + "form_id": "12344", + "grouping": "test-grouping", + "title": course.title, + "subtitle": "hey you", + "post_submit_url": "http://www.edx.org", + } + + additional_metadata = { + 'external_url': 'https://example.com/456', + 'external_identifier': '456', + 'lead_capture_form_url': 'https://example.com/lead-capture', + 'organic_url': 'https://example.com/organic', + 'facts': [{'heading': 'Fact1 heading', 'blurb': '

Fact1 blurb

'}], + 'certificate_info': { + 'heading': 'Certificate heading', + 'blurb': '

Certificate blurb

', + }, + 'start_date': serialize_datetime(current), + 'end_date': serialize_datetime(current + datetime.timedelta(days=10)), + 'registration_deadline': serialize_datetime(current), + 'variant_id': str(uuid4()), + 'course_term_override': 'Example Program', + 'product_status': 'published', + 'product_meta': None, + 'external_course_marketing_type': None, + 'display_on_org_page': True, + 'taxi_form': taxi_form, + } + + url = reverse('api:v1:course-detail', kwargs={'key': course.uuid}) + response = self.client.patch(url, {'additional_metadata': additional_metadata}, format='json') + assert response.status_code == 200 + assert TaxiForm.objects.get(form_id=taxi_form['form_id']) is not None + + course = Course.everything.get(uuid=course.uuid, draft=True) + self.assertDictEqual(self.serialize_course(course)['additional_metadata'], additional_metadata) + self.assertDictEqual(self.serialize_course(course)['additional_metadata']['taxi_form'], taxi_form) + assert previous_data_modified_timestamp < course.data_modified_timestamp + + previous_data_modified_timestamp = course.data_modified_timestamp + taxi_form = { + "form_id": "", + "grouping": "", + "title": "", + "subtitle": "", + "post_submit_url": "", + } + additional_metadata['taxi_form'] = taxi_form + + response = self.client.patch( + url, {'additional_metadata': {'taxi_form': taxi_form}}, format='json' + ) + + assert response.status_code == 200 + course = Course.everything.get(uuid=course.uuid, draft=True) + + self.assertDictEqual(self.serialize_course(course)['additional_metadata'], additional_metadata) + assert self.serialize_course(course)['additional_metadata']['taxi_form'] == taxi_form + pre_save.disconnect(data_modified_timestamp_update, TaxiForm) + @responses.activate def test_update_success_with_course_type_verified(self): verified_mode = SeatTypeFactory.verified() diff --git a/course_discovery/apps/course_metadata/admin.py b/course_discovery/apps/course_metadata/admin.py index f82113efcb..dca19692c7 100644 --- a/course_discovery/apps/course_metadata/admin.py +++ b/course_discovery/apps/course_metadata/admin.py @@ -577,6 +577,7 @@ def _get_course_keys(additional_metadata_object): class AdditionalMetadataAdmin(admin.ModelAdmin): list_display = ( 'id', 'external_identifier', 'external_url', 'courses', 'facts_list', 'external_course_marketing_type', + 'taxi_form', ) search_fields = ('external_identifier', 'external_url') list_filter = ('product_status', ) diff --git a/course_discovery/apps/course_metadata/models.py b/course_discovery/apps/course_metadata/models.py index 4921c6a35d..11c2a651b4 100644 --- a/course_discovery/apps/course_metadata/models.py +++ b/course_discovery/apps/course_metadata/models.py @@ -971,7 +971,7 @@ class AdditionalMetadata(ManageHistoryMixin, TimeStampedModel): def has_changed(self): if not self.pk: return False - external_keys = [self.product_meta,] + external_keys = [self.product_meta, self.taxi_form] return self.has_model_changed(external_keys=external_keys) def update_product_data_modified_timestamp(self, bypass_has_changed=False):