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

Conversation

magajh
Copy link
Contributor

@magajh magajh commented Mar 2, 2025

Summary

This PR introduces two key improvements to the TenantConfigViewSet:

  1. New update-by-domain action: Allows updating a TenantConfig by searching for it using its domain route__domain, rather than the default lookup fields (id or external_key).
  2. Stronger JSON field validation in TenantConfigSerializer: Ensures that the lms_configs, studio_configs, theming_configs, and meta fields must always be dictionaries.

Motivation

Why Add update-by-domain action?

In Control Center, an automated process is required to create new subscriptions and correctly link them to their respective Open edX sites. To achieve this, Control Center must be able to update the external_key of the site (TenantConfig) so that it matches the external_key assigned to the tenant in Control Center.

This alignment is crucial because Control Center relies on the external_key to establish connections with the Open edX site and enable the necessary functionalities for managing the site.

However, in this process, Control Center only has access to the site's domain (route__domain), not its id or external_key. Therefore, this new update-by-domain action allows Control Center to update the TenantConfig using the domain as the lookup field, ensuring that the correct site is modified while maintaining compatibility with existing API consumers.

Why Strengthen JSON Field Validation?

The existing serializer used serializers.JSONField() for storing configurations (lms_configs, studio_configs, theming_configs, and meta). However, DRF’s JSONField does not enforce any specific type, meaning these fields could accept any valid JSON value (e.g., strings, numbers, lists). This led to unexpected behavior where incorrect data types were stored without validation.

To address this, custom field validators were added to enforce that these fields must always be dictionaries. This ensures data integrity and prevents errors caused by unexpected input types.


Changes Introduced

update-by-domain Action

  • A PATCH request can now be made to:
    PATCH /eox-tenant/api/v1/configs/update-by-domain/?domain=<domain>
    
  • If a TenantConfig is found using route__domain, it is updated and returned.
  • If no TenantConfig is found, a 404 Not Found response is returned.
  • If the domain query parameter is missing, a 400 Bad Request response is returned.

How to Test

Update a TenantConfig by its domain

PATCH /eox-tenant/api/v1/configs/update-by-domain/?domain=test.domain.co
Content-Type: application/json

{
    "lms_configs": {
        "PLATFORM_NAME": "Updated Name"
        .... <all the other new config values that should go here>
    }
}

Expected: 200 OK with updated TenantConfig.

Backward Compatibility

  • The update-by-domain action does not modify existing API behavior.
  • The new validations only enforce expected data structures—they do not affect existing valid data.
  • No impact on existing clients consuming this API.

@magajh magajh marked this pull request as ready for review March 3, 2025 21:00
@magajh magajh requested a review from a team as a code owner March 3, 2025 21:00
@magajh magajh force-pushed the maga/allow-search-by-domain-tenantconfig-viewset branch from efcaa68 to 5b1fed8 Compare March 3, 2025 21:19
Copy link
Contributor

@luisfelipec95 luisfelipec95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants