Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add update-by-domain Action and Enforce JSON Field Validations in TenantConfig API #237

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion eox_tenant/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""
Init for eox-tenant.
"""
__version__ = '13.0.0'
__version__ = '13.1.0'
24 changes: 24 additions & 0 deletions eox_tenant/api/v1/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down
128 changes: 119 additions & 9 deletions eox_tenant/api/v1/tests/test_tenant_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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'},
Expand All @@ -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, _):
Expand Down Expand Up @@ -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"})
56 changes: 55 additions & 1 deletion eox_tenant/api/v1/viewsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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."""
Expand Down
2 changes: 1 addition & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[bumpversion]
current_version = 13.0.0
current_version = 13.1.0
commit = False
tag = False

Expand Down