From 5b1fed81cc4fb27bddfc3f4caa86a58b97c5fc60 Mon Sep 17 00:00:00 2001 From: magajh Date: Sun, 2 Mar 2025 09:50:33 -0400 Subject: [PATCH] feat(api): add `update-by-domain` action and enforce JSON field validation in TenantConfig --- CHANGELOG.md | 8 ++ eox_tenant/__init__.py | 2 +- eox_tenant/api/v1/serializers.py | 24 ++++ eox_tenant/api/v1/tests/test_tenant_config.py | 128 ++++++++++++++++-- eox_tenant/api/v1/viewsets.py | 56 +++++++- setup.cfg | 2 +- 6 files changed, 208 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a32774fe..e8ebc667 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [v13.1.0](https://github.com/eduNEXT/eox-tenant/compare/v13.0.0...v13.1.0) - (2025-02-27) + +### Features +- **API:** Added `update-by-domain` action in `TenantConfigViewSet` to allow updates using `route__domain` as a lookup field. This enhances flexibility for scenarios where only the domain is known. + +### Improvements +- **Validation:** Enforced dictionary validation for `lms_configs`, `studio_configs`, `theming_configs`, and `meta` fields in `TenantConfigSerializer`. These fields now strictly accept only dictionary values, preventing unexpected data types. + ## [v13.0.0](https://github.com/eduNEXT/eox-tenant/compare/v12.1.0...v13.0.0) - (2025-01-20) #### Features diff --git a/eox_tenant/__init__.py b/eox_tenant/__init__.py index b756481e..a0f2f768 100644 --- a/eox_tenant/__init__.py +++ b/eox_tenant/__init__.py @@ -1,4 +1,4 @@ """ Init for eox-tenant. """ -__version__ = '13.0.0' +__version__ = '13.1.0' diff --git a/eox_tenant/api/v1/serializers.py b/eox_tenant/api/v1/serializers.py index 38f4f00f..963a0c06 100644 --- a/eox_tenant/api/v1/serializers.py +++ b/eox_tenant/api/v1/serializers.py @@ -32,6 +32,30 @@ class Meta: model = TenantConfig fields = '__all__' + def validate_lms_configs(self, value): + """Ensure lms_configs is a dictionary.""" + if not isinstance(value, dict): + raise serializers.ValidationError("lms_configs must be a dictionary.") + return value + + def validate_studio_configs(self, value): + """Ensure studio_configs is a dictionary.""" + if not isinstance(value, dict): + raise serializers.ValidationError("studio_configs must be a dictionary.") + return value + + def validate_theming_configs(self, value): + """Ensure theming_configs is a dictionary.""" + if not isinstance(value, dict): + raise serializers.ValidationError("theming_configs must be a dictionary.") + return value + + def validate_meta(self, value): + """Ensure meta is a dictionary.""" + if not isinstance(value, dict): + raise serializers.ValidationError("meta must be a dictionary.") + return value + class RouteSerializer(serializers.ModelSerializer): """Serializer class for Route model.""" diff --git a/eox_tenant/api/v1/tests/test_tenant_config.py b/eox_tenant/api/v1/tests/test_tenant_config.py index 914e9e32..2bb27a36 100644 --- a/eox_tenant/api/v1/tests/test_tenant_config.py +++ b/eox_tenant/api/v1/tests/test_tenant_config.py @@ -7,10 +7,10 @@ from rest_framework import status from rest_framework.test import APIClient, APITestCase -from eox_tenant.models import TenantConfig +from eox_tenant.models import Route, TenantConfig -class TenantConfigAPITest(APITestCase): +class TenantConfigAPITest(APITestCase): # pylint: disable=too-many-instance-attributes """TenantConfig API TestCase.""" patch_permissions = patch('eox_tenant.api.v1.permissions.EoxTenantAPIPermission.has_permission', return_value=True) @@ -31,6 +31,16 @@ def setUp(self): theming_configs={'key': 'value'}, meta={'key': 'value'}, ) + self.tenant_config_with_route = TenantConfig.objects.create( + external_key='test_key_with_route', + lms_configs={'PLATFORM_NAME': 'Old Name'}, + studio_configs={'key': 'value'}, + theming_configs={'key': 'value'}, + meta={'key': 'value'}, + ) + self.domain = 'site3.localhost' + self.route = Route.objects.create(domain=self.domain, config=self.tenant_config_with_route) + self.update_by_domain_url = f'{self.url}update-by-domain/' self.url_detail = f'{self.url}{self.tenant_config_example.pk}/' @patch_permissions @@ -50,8 +60,10 @@ def test_get_valid_single_tenant_config(self, _): @patch_permissions def test_create_tenant_config(self, _): """Must create new TenantConfig.""" + tenant_config_objects_count = TenantConfig.objects.count() + external_key = 'test_key_3' data = { - 'external_key': 'test_key', + 'external_key': external_key, 'lms_configs': {'key': 'value'}, 'studio_configs': {'key': 'value'}, 'theming_configs': {'key': 'value'}, @@ -61,12 +73,11 @@ def test_create_tenant_config(self, _): response = self.client.post(self.url, data=data, format='json') self.assertEqual(response.status_code, status.HTTP_201_CREATED) - self.assertEqual(TenantConfig.objects.count(), 2) - self.assertEqual(TenantConfig.objects.get(pk=2).external_key, 'test_key') - self.assertEqual(TenantConfig.objects.get(pk=2).lms_configs, {'key': 'value'}) - self.assertEqual(TenantConfig.objects.get(pk=2).studio_configs, {'key': 'value'}) - self.assertEqual(TenantConfig.objects.get(pk=2).theming_configs, {'key': 'value'}) - self.assertEqual(TenantConfig.objects.get(pk=2).meta, {'key': 'value'}) + self.assertEqual(TenantConfig.objects.count(), tenant_config_objects_count + 1) + self.assertEqual(TenantConfig.objects.get(external_key=external_key).lms_configs, {'key': 'value'}) + self.assertEqual(TenantConfig.objects.get(external_key=external_key).studio_configs, {'key': 'value'}) + self.assertEqual(TenantConfig.objects.get(external_key=external_key).theming_configs, {'key': 'value'}) + self.assertEqual(TenantConfig.objects.get(external_key=external_key).meta, {'key': 'value'}) @patch_permissions def test_post_input_empty_data(self, _): @@ -134,3 +145,102 @@ def test_delete_tenant_config(self, _): response = self.client.delete(self.url_detail) self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + + @patch_permissions + def test_update_tenant_config_by_domain_success(self, _): + """Must successfully update a TenantConfig using `route__domain`.""" + data = { + "lms_configs": { + "PLATFORM_NAME": "Updated Name" + } + } + response = self.client.patch(f"{self.update_by_domain_url}?domain={self.domain}", data=data, format='json') + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["lms_configs"]["PLATFORM_NAME"], "Updated Name") + + @patch_permissions + def test_update_tenant_config_by_domain_missing_query_param(self, _): + """Must return 400 when domain query parameter is missing.""" + data = { + "lms_configs": { + "PLATFORM_NAME": "Updated Name" + } + } + response = self.client.patch(self.update_by_domain_url, data=data, format='json') + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.data["error"], "The 'domain' query parameter is required.") + + @patch_permissions + def test_update_tenant_config_by_domain_not_found(self, _): + """Must return 404 when no TenantConfig is found for the given domain.""" + data = { + "lms_configs": { + "PLATFORM_NAME": "Updated Name" + } + } + response = self.client.patch(f"{self.update_by_domain_url}?domain=unknown.localhost", data=data, format='json') + + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertEqual(response.data["error"], "No TenantConfig found for domain 'unknown.localhost'.") + + @patch_permissions + def test_update_tenant_config_by_domain_empty_payload(self, _): + """Must ensure that if an empty payload is sent, nothing gets changed.""" + external_key = self.tenant_config_with_route.external_key + lms_configs = self.tenant_config_with_route.lms_configs + studio_configs = self.tenant_config_with_route.studio_configs + theming_configs = self.tenant_config_with_route.theming_configs + meta = self.tenant_config_with_route.meta + + response = self.client.patch(f"{self.update_by_domain_url}?domain={self.domain}", data={}, format='json') + self.tenant_config_with_route.refresh_from_db() + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(self.tenant_config_with_route.external_key, external_key) + self.assertEqual(self.tenant_config_with_route.lms_configs, lms_configs) + self.assertEqual(self.tenant_config_with_route.studio_configs, studio_configs) + self.assertEqual(self.tenant_config_with_route.theming_configs, theming_configs) + self.assertEqual(self.tenant_config_with_route.meta, meta) + + @patch_permissions + def test_update_tenant_config_by_domain_invalid_data(self, _): + """Must return 400 when the payload contains invalid data.""" + data = { + "lms_configs": "Invalid structure" # Should be a dictionary + } + response = self.client.patch(f"{self.update_by_domain_url}?domain={self.domain}", data=data, format='json') + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + @patch_permissions + def test_partial_update_tenant_config_by_domain(self, _): + """Must allow partial updates without modifying other fields.""" + data = { + "lms_configs": { + "PLATFORM_NAME": "New Partial Update" + } + } + response = self.client.patch(f"{self.update_by_domain_url}?domain={self.domain}", data=data, format='json') + print(100 * "#") + print(response.content) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.tenant_config_with_route.refresh_from_db() + self.assertEqual(self.tenant_config_with_route.lms_configs["PLATFORM_NAME"], "New Partial Update") + + @patch_permissions + def test_update_tenant_config_by_domain_preserves_other_fields(self, _): + """Ensure updating one field does not erase other fields.""" + data = { + "lms_configs": { + "PLATFORM_NAME": "Updated Platform Name" + } + } + response = self.client.patch(f"{self.update_by_domain_url}?domain={self.domain}", data=data, format='json') + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.tenant_config_with_route.refresh_from_db() + self.assertEqual(self.tenant_config_with_route.lms_configs["PLATFORM_NAME"], "Updated Platform Name") + self.assertEqual(self.tenant_config_with_route.studio_configs, {"key": "value"}) diff --git a/eox_tenant/api/v1/viewsets.py b/eox_tenant/api/v1/viewsets.py index e82ac7b2..d8b5ea0b 100644 --- a/eox_tenant/api/v1/viewsets.py +++ b/eox_tenant/api/v1/viewsets.py @@ -4,9 +4,11 @@ import logging from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication -from rest_framework import viewsets +from rest_framework import status, viewsets from rest_framework.authentication import SessionAuthentication +from rest_framework.decorators import action from rest_framework.parsers import JSONParser +from rest_framework.response import Response from eox_tenant.api.v1.permissions import EoxTenantAPIPermission from eox_tenant.api.v1.serializers import MicrositeSerializer, RouteSerializer, TenantConfigSerializer @@ -447,6 +449,58 @@ class TenantConfigViewSet(AlternativeFieldLookupMixin, viewsets.ModelViewSet): alternative_lookup_field = 'external_key' queryset = TenantConfig.objects.all() + @action(detail=False, methods=['patch'], url_path='update-by-domain') + def update_by_domain(self, request): + """ + Custom endpoint to update a TenantConfig using `route__domain`. + + **Request Format:** + ``` + PATCH /eox-tenant/api/v1/configs/update-by-domain/?domain=site3.localhost + Content-Type: application/json + + { + "lms_configs": { + "PLATFORM_NAME": "Updated Name" + } + } + ``` + + **Response Format:** + ``` + { + "id": 2, + "external_key": "xAer5z6FEbW", + "lms_configs": { + "PLATFORM_NAME": "Updated Name" + }, + ... + } + ``` + """ + domain = request.query_params.get("domain") + + if not domain: + return Response( + {"error": "The 'domain' query parameter is required."}, + status=status.HTTP_400_BAD_REQUEST, + ) + + try: + tenant_config = TenantConfig.objects.get(route__domain=domain) + except TenantConfig.DoesNotExist: # pylint: disable=no-member + return Response( + {"error": f"No TenantConfig found for domain '{domain}'."}, + status=status.HTTP_404_NOT_FOUND, + ) + + serializer = self.get_serializer(tenant_config, data=request.data, partial=True) + if serializer.is_valid(): + serializer.save() + return Response(serializer.data, status=status.HTTP_200_OK) + + return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) + class RouteViewSet(viewsets.ModelViewSet): """RouteViewSet that allows the basic API actions.""" diff --git a/setup.cfg b/setup.cfg index 6f93cc21..0a7ed763 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 13.0.0 +current_version = 13.1.0 commit = False tag = False