From 3918661cef25c0100aff40b70d2738bd454c508c Mon Sep 17 00:00:00 2001 From: rgraber Date: Wed, 12 Nov 2025 09:21:34 -0500 Subject: [PATCH 01/20] feat(subsequences): add update_params method --- kobo/apps/subsequences/actions/base.py | 18 +++++++++++++++++ .../test_automatic_google_transcription.py | 18 +++++++++++++++++ .../test_automatic_google_translation.py | 20 ++++++++++++++++++- .../tests/test_manual_transcription.py | 18 +++++++++++++++++ .../tests/test_manual_translation.py | 18 ++++++++++++++++- 5 files changed, 90 insertions(+), 2 deletions(-) diff --git a/kobo/apps/subsequences/actions/base.py b/kobo/apps/subsequences/actions/base.py index 6ec6abd893..15f9e61c6e 100644 --- a/kobo/apps/subsequences/actions/base.py +++ b/kobo/apps/subsequences/actions/base.py @@ -322,6 +322,17 @@ def get_output_fields(self) -> list[dict]: # raise NotImplementedError() return [] + def update_params(self, incoming_params): + """ + Returns the result of updating current params with incoming ones from + a request. May be overridden, eg, to prevent deletion of existing lanugages + for transcriptions/translations + Defaults to replacing the existing params with the new ones. + Should raise an error if the incoming params are not well-formatted + """ + self.validate_params(incoming_params) + self.params = incoming_params + def validate_external_data(self, data): jsonschema.validate(data, self.external_data_schema) @@ -594,6 +605,13 @@ def languages(self) -> list[str]: languages.append(individual_params['language']) return languages + def update_params(self, incoming_params): + self.validate_params(incoming_params) + current_languages = self.languages + for language_obj in incoming_params: + if language_obj['language'] not in current_languages: + self.params.append(language_obj) + class BaseAutomaticNLPAction(BaseManualNLPAction): """ diff --git a/kobo/apps/subsequences/tests/test_automatic_google_transcription.py b/kobo/apps/subsequences/tests/test_automatic_google_transcription.py index 8b62ce3361..701281b341 100644 --- a/kobo/apps/subsequences/tests/test_automatic_google_transcription.py +++ b/kobo/apps/subsequences/tests/test_automatic_google_transcription.py @@ -336,3 +336,21 @@ def test_latest_version_is_first(): assert mock_sup_det['_versions'][0]['_data']['value'] == 'trois' assert mock_sup_det['_versions'][1]['_data']['value'] == 'deux' assert mock_sup_det['_versions'][2]['_data']['value'] == 'un' + + +def test_update_params_only_adds_new_languages(): + xpath = 'group_name/question_name' + params = [{'language': 'fr'}, {'language': 'en'}] + action = AutomaticGoogleTranscriptionAction(xpath, params) + incoming_params = [{'language': 'en'}, {'language': 'es'}] + action.update_params(incoming_params) + assert sorted(action.languages) == ['en', 'es', 'fr'] + + +def test_update_params_fails_if_new_params_invalid(): + xpath = 'group_name/question_name' + params = [{'language': 'fr'}, {'language': 'en'}] + action = AutomaticGoogleTranscriptionAction(xpath, params) + incoming_params = [{'bad': 'things'}] + with pytest.raises(jsonschema.exceptions.ValidationError): + action.update_params(incoming_params) diff --git a/kobo/apps/subsequences/tests/test_automatic_google_translation.py b/kobo/apps/subsequences/tests/test_automatic_google_translation.py index 8df7dc1540..39807fdec5 100644 --- a/kobo/apps/subsequences/tests/test_automatic_google_translation.py +++ b/kobo/apps/subsequences/tests/test_automatic_google_translation.py @@ -6,8 +6,8 @@ import pytest from ..actions.automatic_google_translation import AutomaticGoogleTranslationAction -from .constants import EMPTY_SUBMISSION, EMPTY_SUPPLEMENT, QUESTION_SUPPLEMENT from ..exceptions import TranscriptionNotFound +from .constants import EMPTY_SUBMISSION, EMPTY_SUPPLEMENT, QUESTION_SUPPLEMENT def test_valid_params_pass_validation(): @@ -414,6 +414,24 @@ def test_action_is_updated_in_background_if_in_progress(): task_mock.apply_async.assert_called_once() +def test_update_params_only_adds_new_languages(): + xpath = 'group_name/question_name' + params = [{'language': 'fr'}, {'language': 'en'}] + action = AutomaticGoogleTranslationAction(xpath, params) + incoming_params = [{'language': 'en'}, {'language': 'es'}] + action.update_params(incoming_params) + assert sorted(action.languages) == ['en', 'es', 'fr'] + + +def test_update_params_fails_if_new_params_invalid(): + xpath = 'group_name/question_name' + params = [{'language': 'fr'}, {'language': 'en'}] + action = AutomaticGoogleTranslationAction(xpath, params) + incoming_params = [{'bad': 'things'}] + with pytest.raises(jsonschema.exceptions.ValidationError): + action.update_params(incoming_params) + + def _get_action(fetch_action_dependencies=True): xpath = 'group_name/question_name' # irrelevant for this test params = [{'language': 'fr'}, {'language': 'es'}] diff --git a/kobo/apps/subsequences/tests/test_manual_transcription.py b/kobo/apps/subsequences/tests/test_manual_transcription.py index a04fb129eb..4016a67212 100644 --- a/kobo/apps/subsequences/tests/test_manual_transcription.py +++ b/kobo/apps/subsequences/tests/test_manual_transcription.py @@ -173,3 +173,21 @@ def test_latest_revision_is_first(): assert mock_sup_det['_versions'][0]['_data']['value'] == 'trois' assert mock_sup_det['_versions'][1]['_data']['value'] == 'deux' assert mock_sup_det['_versions'][2]['_data']['value'] == 'un' + + +def test_update_params_only_adds_new_languages(): + xpath = 'group_name/question_name' + params = [{'language': 'fr'}, {'language': 'en'}] + action = ManualTranscriptionAction(xpath, params) + incoming_params = [{'language': 'en'}, {'language': 'es'}] + action.update_params(incoming_params) + assert sorted(action.languages) == ['en', 'es', 'fr'] + + +def test_update_params_fails_if_new_params_invalid(): + xpath = 'group_name/question_name' + params = [{'language': 'fr'}, {'language': 'en'}] + action = ManualTranscriptionAction(xpath, params) + incoming_params = [{'bad': 'things'}] + with pytest.raises(jsonschema.exceptions.ValidationError): + action.update_params(incoming_params) diff --git a/kobo/apps/subsequences/tests/test_manual_translation.py b/kobo/apps/subsequences/tests/test_manual_translation.py index 5dbc9d5a6d..f62661512d 100644 --- a/kobo/apps/subsequences/tests/test_manual_translation.py +++ b/kobo/apps/subsequences/tests/test_manual_translation.py @@ -2,8 +2,8 @@ import jsonschema import pytest -from ..exceptions import TranscriptionNotFound from ..actions.manual_translation import ManualTranslationAction +from ..exceptions import TranscriptionNotFound from .constants import EMPTY_SUBMISSION, EMPTY_SUPPLEMENT, QUESTION_SUPPLEMENT @@ -177,6 +177,22 @@ def test_cannot_revise_data_without_transcription(): ) +def test_update_params_only_adds_new_languages(): + xpath = 'group_name/question_name' + params = [{'language': 'fr'}, {'language': 'en'}] + action = ManualTranslationAction(xpath, params) + incoming_params = [{'language': 'en'}, {'language': 'es'}] + action.update_params(incoming_params) + assert sorted(action.languages) == ['en', 'es', 'fr'] + + +def test_update_params_fails_if_new_params_invalid(): + xpath = 'group_name/question_name' + params = [{'language': 'fr'}, {'language': 'en'}] + action = ManualTranslationAction(xpath, params) + incoming_params = [{'bad': 'things'}] + with pytest.raises(jsonschema.exceptions.ValidationError): + action.update_params(incoming_params) def _get_action(fetch_action_dependencies=True): From d029d95d75eb6b3de46ddd09fb3cb86948e8b6bc Mon Sep 17 00:00:00 2001 From: rgraber Date: Fri, 7 Nov 2025 07:30:29 -0500 Subject: [PATCH 02/20] refactor(subsequences): separate advanced feature configs --- ...issionsupplement_questionadvancedaction.py | 76 +++++++++++++++++++ kobo/apps/subsequences/models.py | 66 +++++++++------- kobo/apps/subsequences/serializers.py | 24 ++++++ kobo/apps/subsequences/views.py | 20 +++++ ...ter_accesslogexporttask_status_and_more.py | 47 ++++++++++++ kpi/urls/router_api_v2.py | 3 + kpi/views/v2/asset_advanced_features.py | 43 +++++++++++ 7 files changed, 253 insertions(+), 26 deletions(-) create mode 100644 kobo/apps/subsequences/migrations/0005_submissionsupplement_questionadvancedaction.py create mode 100644 kobo/apps/subsequences/serializers.py create mode 100644 kobo/apps/subsequences/views.py create mode 100644 kpi/migrations/0070_alter_asset_options_alter_accesslogexporttask_status_and_more.py create mode 100644 kpi/views/v2/asset_advanced_features.py diff --git a/kobo/apps/subsequences/migrations/0005_submissionsupplement_questionadvancedaction.py b/kobo/apps/subsequences/migrations/0005_submissionsupplement_questionadvancedaction.py new file mode 100644 index 0000000000..de5efc608b --- /dev/null +++ b/kobo/apps/subsequences/migrations/0005_submissionsupplement_questionadvancedaction.py @@ -0,0 +1,76 @@ +# Generated by Django 4.2.24 on 2025-11-11 18:01 + +import django.db.models.deletion +from django.db import migrations, models + +import kpi.fields.kpi_uid +import kpi.fields.lazy_default_jsonb + + +class Migration(migrations.Migration): + + dependencies = [ + ('kpi', '0070_alter_asset_options_alter_accesslogexporttask_status_and_more'), + ('subsequences', '0004_increase_subsequences_submission_uuid'), + ] + + operations = [ + migrations.CreateModel( + name='SubmissionSupplement', + fields=[], + options={ + 'abstract': False, + 'proxy': True, + 'indexes': [], + 'constraints': [], + }, + bases=('subsequences.submissionextras',), + ), + migrations.CreateModel( + name='QuestionAdvancedAction', + fields=[ + ( + 'uid', + kpi.fields.kpi_uid.KpiUidField( + _null=False, primary_key=True, uid_prefix='qaa' + ), + ), + ( + 'action', + models.CharField( + choices=[ + ('manual_transcription', 'Manual Transcription'), + ('manual_translation', 'Manual Translation'), + ( + 'automatic_google_translation', + 'Automatic Google Translation', + ), + ( + 'automatic_google_transcription', + 'Automatic Google Transcription', + ), + ('qual', 'Qual'), + ], + db_index=True, + max_length=60, + ), + ), + ('question_xpath', models.CharField(max_length=2000)), + ( + 'config', + kpi.fields.lazy_default_jsonb.LazyDefaultJSONBField(default=dict), + ), + ( + 'asset', + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name='advanced_features_set', + to='kpi.asset', + ), + ), + ], + options={ + 'unique_together': {('asset_id', 'question_xpath', 'action')}, + }, + ), + ] diff --git a/kobo/apps/subsequences/models.py b/kobo/apps/subsequences/models.py index d66e97f25f..c417b90115 100644 --- a/kobo/apps/subsequences/models.py +++ b/kobo/apps/subsequences/models.py @@ -1,6 +1,7 @@ from django.db import models from kobo.apps.openrosa.apps.logger.xform_instance_parser import remove_uuid_prefix +from kpi.fields import LazyDefaultJSONBField, KpiUidField from kpi.models.abstract_models import AbstractTimeStampedModel from .actions import ACTION_IDS_TO_CLASSES from .constants import SUBMISSION_UUID_FIELD, SCHEMA_VERSIONS @@ -34,7 +35,7 @@ def __repr__(self): @staticmethod def revise_data(asset: 'kpi.Asset', submission: dict, incoming_data: dict) -> dict: - if not asset.advanced_features: + if not asset.advanced_features_set.objects.exists(): raise InvalidAction schema_version = incoming_data.get('_version') @@ -65,12 +66,10 @@ def revise_data(asset: 'kpi.Asset', submission: dict, incoming_data: dict) -> di # FIXME: what's a better way? skip all leading underscore keys? # pop off the known special keys first? continue - try: - action_configs_for_this_question = asset.advanced_features[ - '_actionConfigs' - ][question_xpath] - except KeyError as e: - raise InvalidXPath from e + + action_configs_for_this_question = asset.advanced_features_set.filter(question_xpath=question_xpath) + if not action_configs_for_this_question.exists(): + raise InvalidXPath for action_id, action_data in data_for_this_question.items(): try: @@ -78,8 +77,8 @@ def revise_data(asset: 'kpi.Asset', submission: dict, incoming_data: dict) -> di except KeyError as e: raise InvalidAction from e try: - action_params = action_configs_for_this_question[action_id] - except KeyError as e: + action_params = action_configs_for_this_question.get(action=action_id).config + except QuestionAdvancedAction.DoesNotExist as e: raise InvalidAction from e action = action_class(question_xpath, action_params, asset) @@ -172,21 +171,8 @@ def retrieve_data( processed_data_for_this_question = retrieved_supplemental_data.setdefault( question_xpath, {} ) - action_configs = asset.advanced_features['_actionConfigs'] - try: - action_configs_for_this_question = action_configs[question_xpath] - except KeyError: - # There's still supplemental data for this question at the - # submission level, but the question is no longer configured at the - # asset level. - # Allow this for now, but maybe forbid later and also forbid - # removing things from the asset-level action configuration? - # Actions could be disabled or hidden instead of being removed - - # FIXME: divergence between the asset-level configuration and - # submission-level supplemental data is going to cause schema - # validation failures! We defo need to forbid removal of actions - # and instead provide a way to mark them as deleted + action_configs_for_this_question = asset.advanced_features_set.filter(question_xpath=question_xpath) + if not action_configs_for_this_question.exists(): continue for action_id, action_data in data_for_this_question.items(): @@ -198,8 +184,8 @@ def retrieve_data( # TODO: log an error continue try: - action_params = action_configs_for_this_question[action_id] - except KeyError: + action_params = action_configs_for_this_question.get(action=action_id) + except QuestionAdvancedAction.DoesNotExist: # An action class present in the submission data is no longer # configured at the asset level for this question # Allow this for now, but maybe forbid later and also forbid @@ -237,3 +223,31 @@ def retrieve_data( return data_for_output return retrieved_supplemental_data + +class Action(models.TextChoices): + MANUAL_TRANSCRIPTION = 'manual_transcription' + MANUAL_TRANSLATION = 'manual_translation' + AUTOMATIC_GOOGLE_TRANSLATION = 'automatic_google_translation' + AUTOMATIC_GOOGLE_TRANSCRIPTION = 'automatic_google_transcription' + QUAL = 'qual' + +class QuestionAdvancedAction(models.Model): + uid = KpiUidField(uid_prefix='qaa', primary_key=True) + asset = models.ForeignKey('kpi.Asset', related_name='advanced_features_set', + null=False, blank=False, on_delete=models.CASCADE) + action = models.CharField( + max_length=60, + choices=Action.choices, + db_index=True, + null=False, + blank=False, + ) + question_xpath = models.CharField( + null=False, blank=False, max_length=2000 + ) + config = LazyDefaultJSONBField(default=dict) + + class Meta: + unique_together = ('asset_id', 'question_xpath', 'action') + + diff --git a/kobo/apps/subsequences/serializers.py b/kobo/apps/subsequences/serializers.py new file mode 100644 index 0000000000..a633af2d1f --- /dev/null +++ b/kobo/apps/subsequences/serializers.py @@ -0,0 +1,24 @@ +from rest_framework import serializers + +from kobo.apps.subsequences.models import QuestionAdvancedAction + + +class QuestionAdvancedActionUpdateSerializer(serializers.ModelSerializer): + class Meta: + model = QuestionAdvancedAction + fields = ['config'] + read_only_fields = ['question_xpath', 'action', 'asset'] + + def update(self, instance, validated_data): + config = instance.config + languages = [obj["language"] for obj in config] + request_config = validated_data['config'] + new = [obj for obj in request_config if obj["language"] not in languages] + instance.config =[*config, *new] + instance.save(update_fields=['config']) + return instance + +class QuestionAdvancedActionCreateSerializer(serializers.ModelSerializer): + class Meta: + model = QuestionAdvancedAction + fields = ['question_xpath', 'action', 'config'] diff --git a/kobo/apps/subsequences/views.py b/kobo/apps/subsequences/views.py new file mode 100644 index 0000000000..96e80530bd --- /dev/null +++ b/kobo/apps/subsequences/views.py @@ -0,0 +1,20 @@ +from rest_framework import viewsets +from rest_framework_extensions.mixins import NestedViewSetMixin + +from kobo.apps.subsequences.models import QuestionAdvancedAction +from kobo.apps.subsequences.serializers import QuestionAdvancedActionUpdateSerializer, \ + QuestionAdvancedActionCreateSerializer +from kpi.utils.viewset_mixins import AssetNestedObjectViewsetMixin + + +class QuestionAdvancedActionViewSet(viewsets.ModelViewSet, AssetNestedObjectViewsetMixin, NestedViewSetMixin): + serializer_class = QuestionAdvancedActionUpdateSerializer + def get_queryset(self): + return QuestionAdvancedAction.objects.filter(asset=self.asset) + def perform_create(self, serializer): + serializer.save(asset=self.asset) + def get_serializer_class(self): + if self.action == 'create': + return QuestionAdvancedActionCreateSerializer + else: + return QuestionAdvancedActionUpdateSerializer diff --git a/kpi/migrations/0070_alter_asset_options_alter_accesslogexporttask_status_and_more.py b/kpi/migrations/0070_alter_asset_options_alter_accesslogexporttask_status_and_more.py new file mode 100644 index 0000000000..894dd46c44 --- /dev/null +++ b/kpi/migrations/0070_alter_asset_options_alter_accesslogexporttask_status_and_more.py @@ -0,0 +1,47 @@ +# Generated by Django 4.2.24 on 2025-11-11 18:01 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('kpi', '0069_alter_assetversion_reversion_version'), + ] + + operations = [ + migrations.AlterModelOptions( + name='asset', + options={'default_permissions': ('add', 'change', 'delete'), 'ordering': ['-date_modified'], 'permissions': (('view_asset', 'Can view asset'), ('discover_asset', 'Can discover asset in public lists'), ('manage_asset', 'Can manage all aspects of asset'), ('add_submissions', 'Can submit data to asset'), ('view_submissions', 'Can view submitted data for asset'), ('partial_submissions', 'Can make partial actions on submitted data for asset for specific users'), ('change_submissions', 'Can modify submitted data for asset'), ('delete_submissions', 'Can delete submitted data for asset'), ('validate_submissions', 'Can validate submitted data asset'))}, + ), + migrations.AlterField( + model_name='accesslogexporttask', + name='status', + field=models.CharField(choices=[('created', 'created'), ('processing', 'processing'), ('complete', 'complete'), ('error', 'error')], default='created', max_length=32), + ), + migrations.AlterField( + model_name='importtask', + name='status', + field=models.CharField(choices=[('created', 'created'), ('processing', 'processing'), ('complete', 'complete'), ('error', 'error')], default='created', max_length=32), + ), + migrations.AlterField( + model_name='projecthistorylogexporttask', + name='status', + field=models.CharField(choices=[('created', 'created'), ('processing', 'processing'), ('complete', 'complete'), ('error', 'error')], default='created', max_length=32), + ), + migrations.AlterField( + model_name='projectviewexporttask', + name='status', + field=models.CharField(choices=[('created', 'created'), ('processing', 'processing'), ('complete', 'complete'), ('error', 'error')], default='created', max_length=32), + ), + migrations.AlterField( + model_name='submissionexporttask', + name='status', + field=models.CharField(choices=[('created', 'created'), ('processing', 'processing'), ('complete', 'complete'), ('error', 'error')], default='created', max_length=32), + ), + migrations.AlterField( + model_name='submissionsynchronousexport', + name='status', + field=models.CharField(choices=[('created', 'created'), ('processing', 'processing'), ('complete', 'complete'), ('error', 'error')], default='created', max_length=32), + ), + ] diff --git a/kpi/urls/router_api_v2.py b/kpi/urls/router_api_v2.py index 55e358815d..aa2a52d954 100644 --- a/kpi/urls/router_api_v2.py +++ b/kpi/urls/router_api_v2.py @@ -14,6 +14,7 @@ ) from kobo.apps.project_ownership.urls import router as project_ownership_router from kobo.apps.project_views.views import ProjectViewViewSet +from kobo.apps.subsequences.views import QuestionAdvancedActionViewSet from kpi.views.v2.asset import AssetViewSet from kpi.views.v2.asset_counts import AssetCountsViewSet from kpi.views.v2.asset_export_settings import AssetExportSettingsViewSet @@ -138,6 +139,8 @@ def get_urls(self, *args, **kwargs): parents_query_lookups=['asset'], ) +asset_routes.register(r'advanced-features', QuestionAdvancedActionViewSet, basename='advanced-features', parents_query_lookups=['asset']) + data_routes = asset_routes.register(r'data', DataViewSet, basename='submission', diff --git a/kpi/views/v2/asset_advanced_features.py b/kpi/views/v2/asset_advanced_features.py new file mode 100644 index 0000000000..a207f64cab --- /dev/null +++ b/kpi/views/v2/asset_advanced_features.py @@ -0,0 +1,43 @@ +from rest_framework import serializers +from rest_framework.fields import empty +from rest_framework.mixins import UpdateModelMixin +from rest_framework_extensions.mixins import NestedViewSetMixin + +from kobo.apps.audit_log.base_views import AuditLoggedModelViewSet +from kobo.apps.audit_log.models import AuditType +from kobo.apps.subsequences.models import QuestionAdvancedAction +from kpi.mixins.asset import AssetViewSetListMixin +from kpi.mixins.object_permission import ObjectPermissionViewSetMixin +from kpi.utils.viewset_mixins import AssetNestedObjectViewsetMixin + + +class AssetAdvancedFeaturesViewSet( + AssetViewSetListMixin, + ObjectPermissionViewSetMixin, + AssetNestedObjectViewsetMixin, + NestedViewSetMixin, + AuditLoggedModelViewSet, + UpdateModelMixin, +): + logged_fields = [ + + ] + log_type = AuditType.PROJECT_HISTORY + + def list(self): + pass + def partial_update(self, request, *args, **kwargs): + + pass + pass + +class AdvancedFeaturesSerializer(serializers.Serializer): + def __init__(self, instance=None, data=empty, **kwargs): + super().__init__(instance=instance, data=data, **kwargs) + self.asset = kwargs.get('context').get('asset') + + def validate(self, data): + pass + + def create(self, validated_data): + self.asset.advanced_features = validated_data From 7f9d0b91b1486349ed4a13a25ee6ff7200300b5b Mon Sep 17 00:00:00 2001 From: rgraber Date: Wed, 12 Nov 2025 12:58:30 -0500 Subject: [PATCH 03/20] fixup!: viewset and serializer --- kobo/apps/subsequences/actions/base.py | 1 - kobo/apps/subsequences/models.py | 13 ++- kobo/apps/subsequences/serializers.py | 21 +++-- .../subsequences/tests/api/v2/test_views.py | 83 +++++++++++++++++++ kobo/apps/subsequences/views.py | 25 ++++-- 5 files changed, 125 insertions(+), 18 deletions(-) create mode 100644 kobo/apps/subsequences/tests/api/v2/test_views.py diff --git a/kobo/apps/subsequences/actions/base.py b/kobo/apps/subsequences/actions/base.py index 15f9e61c6e..aed04d2834 100644 --- a/kobo/apps/subsequences/actions/base.py +++ b/kobo/apps/subsequences/actions/base.py @@ -606,7 +606,6 @@ def languages(self) -> list[str]: return languages def update_params(self, incoming_params): - self.validate_params(incoming_params) current_languages = self.languages for language_obj in incoming_params: if language_obj['language'] not in current_languages: diff --git a/kobo/apps/subsequences/models.py b/kobo/apps/subsequences/models.py index c417b90115..aa89d7db30 100644 --- a/kobo/apps/subsequences/models.py +++ b/kobo/apps/subsequences/models.py @@ -1,10 +1,11 @@ from django.db import models from kobo.apps.openrosa.apps.logger.xform_instance_parser import remove_uuid_prefix -from kpi.fields import LazyDefaultJSONBField, KpiUidField +from kpi.fields import KpiUidField, LazyDefaultJSONBField from kpi.models.abstract_models import AbstractTimeStampedModel from .actions import ACTION_IDS_TO_CLASSES -from .constants import SUBMISSION_UUID_FIELD, SCHEMA_VERSIONS +from .actions.base import BaseAction +from .constants import SCHEMA_VERSIONS, SUBMISSION_UUID_FIELD from .exceptions import InvalidAction, InvalidXPath from .schemas import validate_submission_supplement @@ -250,4 +251,10 @@ class QuestionAdvancedAction(models.Model): class Meta: unique_together = ('asset_id', 'question_xpath', 'action') - + def to_action(self) -> BaseAction: + action_class = ACTION_IDS_TO_CLASSES[self.action] + return action_class( + source_question_xpath=self.question_xpath, + params=self.config, + asset=self.asset, + ) diff --git a/kobo/apps/subsequences/serializers.py b/kobo/apps/subsequences/serializers.py index a633af2d1f..c90b8d3083 100644 --- a/kobo/apps/subsequences/serializers.py +++ b/kobo/apps/subsequences/serializers.py @@ -1,3 +1,4 @@ +import jsonschema.exceptions from rest_framework import serializers from kobo.apps.subsequences.models import QuestionAdvancedAction @@ -9,16 +10,24 @@ class Meta: fields = ['config'] read_only_fields = ['question_xpath', 'action', 'asset'] + def validate(self, attrs): + data = super().validate(attrs) + action = self.instance.to_action() + try: + action.__class__.validate_params(attrs['config']) + except jsonschema.exceptions.ValidationError as ve: + raise serializers.ValidationError(ve) + return data + def update(self, instance, validated_data): - config = instance.config - languages = [obj["language"] for obj in config] - request_config = validated_data['config'] - new = [obj for obj in request_config if obj["language"] not in languages] - instance.config =[*config, *new] + action = instance.to_action() + action.update_params(validated_data['config']) + instance.config = action.params instance.save(update_fields=['config']) return instance -class QuestionAdvancedActionCreateSerializer(serializers.ModelSerializer): + +class QuestionAdvancedActionSerializer(serializers.ModelSerializer): class Meta: model = QuestionAdvancedAction fields = ['question_xpath', 'action', 'config'] diff --git a/kobo/apps/subsequences/tests/api/v2/test_views.py b/kobo/apps/subsequences/tests/api/v2/test_views.py new file mode 100644 index 0000000000..96f382eaa2 --- /dev/null +++ b/kobo/apps/subsequences/tests/api/v2/test_views.py @@ -0,0 +1,83 @@ +import json + +from django.test import Client +from django.urls import reverse +from rest_framework import status + +from kobo.apps.kobo_auth.shortcuts import User +from kobo.apps.subsequences.models import QuestionAdvancedAction +from kpi.models import Asset +from kpi.tests.base_test_case import BaseTestCase + + +class QuestionAdvancedActionViewSetTestCase(BaseTestCase): + fixtures = ['test_data'] + + def setUp(self): + user = User.objects.get(username='someuser') + self.asset = Asset.objects.create( + owner=user, + content={'survey': [{'type': 'audio', 'label': 'q1', 'name': 'q1'}]}, + ) + self.action = QuestionAdvancedAction.objects.create( + asset=self.asset, + question_xpath='q1', + action='manual_transcription', + config=[{'language': 'en'}], + ) + self.list_actions_url = reverse( + 'api_v2:advanced-features-list', + kwargs={'parent_lookup_asset': self.asset.uid}, + ) + self.action_detail_url = reverse( + 'api_v2:advanced-features-detail', + kwargs={'parent_lookup_asset': self.asset.uid, 'pk': self.action.uid}, + ) + self.client = Client(raise_request_exception=False) + + def test_list_advanced_features(self): + res = self.client.get(self.list_actions_url) + assert res.status_code == status.HTTP_200_OK + + def test_update_action(self): + res = self.client.patch( + self.action_detail_url, + content_type='application/json', + data=json.dumps({'config': [{'language': 'es'}]}), + ) + assert res.status_code == status.HTTP_200_OK + self.action.refresh_from_db() + assert self.action.config == [{'language': 'en'}, {'language': 'es'}] + + def test_cannot_update_action_with_invalid_params(self): + res = self.client.patch( + self.action_detail_url, + content_type='application/json', + data=json.dumps({'config': [{'bad': 'stuff'}]}), + ) + assert res.status_code == status.HTTP_400_BAD_REQUEST + self.action.refresh_from_db() + assert self.action.config == [{'language': 'en'}] + + def test_create_action(self): + res = self.client.post( + self.list_actions_url, + data={ + 'action': 'manual_translation', + 'config': json.dumps([{'language': 'de'}]), + 'question_xpath': 'q1', + }, + ) + assert res.status_code == status.HTTP_201_CREATED + new_action = QuestionAdvancedAction.objects.get( + asset=self.asset, action='manual_translation' + ) + assert new_action.config == [{'language': 'de'}] + assert new_action.question_xpath == 'q1' + + def test_cannot_delete_actions(self): + res = self.client.delete(self.action_detail_url) + assert res.status_code == status.HTTP_405_METHOD_NOT_ALLOWED + assert QuestionAdvancedAction.objects.filter( + asset=self.asset, action=self.action.action + ).exists() diff --git a/kobo/apps/subsequences/views.py b/kobo/apps/subsequences/views.py index 96e80530bd..3bcdd46a5c 100644 --- a/kobo/apps/subsequences/views.py +++ b/kobo/apps/subsequences/views.py @@ -1,20 +1,29 @@ -from rest_framework import viewsets +from rest_framework import mixins, viewsets from rest_framework_extensions.mixins import NestedViewSetMixin from kobo.apps.subsequences.models import QuestionAdvancedAction -from kobo.apps.subsequences.serializers import QuestionAdvancedActionUpdateSerializer, \ - QuestionAdvancedActionCreateSerializer +from kobo.apps.subsequences.serializers import ( + QuestionAdvancedActionSerializer, + QuestionAdvancedActionUpdateSerializer, +) from kpi.utils.viewset_mixins import AssetNestedObjectViewsetMixin -class QuestionAdvancedActionViewSet(viewsets.ModelViewSet, AssetNestedObjectViewsetMixin, NestedViewSetMixin): - serializer_class = QuestionAdvancedActionUpdateSerializer +class QuestionAdvancedActionViewSet( + viewsets.GenericViewSet, + mixins.CreateModelMixin, + mixins.RetrieveModelMixin, + mixins.UpdateModelMixin, + mixins.ListModelMixin, + AssetNestedObjectViewsetMixin, + NestedViewSetMixin, +): def get_queryset(self): return QuestionAdvancedAction.objects.filter(asset=self.asset) def perform_create(self, serializer): serializer.save(asset=self.asset) def get_serializer_class(self): - if self.action == 'create': - return QuestionAdvancedActionCreateSerializer - else: + if self.action in ['update', 'partial_update']: return QuestionAdvancedActionUpdateSerializer + else: + return QuestionAdvancedActionSerializer From 6cc10d185003ae3f8aa84f907e92a18ee408372f Mon Sep 17 00:00:00 2001 From: rgraber Date: Mon, 17 Nov 2025 08:48:48 -0500 Subject: [PATCH 04/20] fixup!: half migration --- kobo/apps/audit_log/models.py | 29 ++++++ .../tests/test_project_history_logs.py | 56 +++++------ .../jobs/0011_migrate_advanced_features.py | 8 ++ kobo/apps/subsequences/actions/__init__.py | 2 + kobo/apps/subsequences/constants.py | 15 +++ kobo/apps/subsequences/models.py | 51 +++------- kobo/apps/subsequences/schemas.py | 3 - kobo/apps/subsequences/serializers.py | 20 ++-- kobo/apps/subsequences/tasks.py | 4 +- .../subsequences/tests/api/v2/test_views.py | 14 +-- .../subsequences/utils/action_conversion.py | 10 ++ .../subsequences/utils/supplement_data.py | 19 ++-- kobo/apps/subsequences/utils/versioning.py | 92 +++++++++++-------- kobo/apps/subsequences/views.py | 10 +- kpi/models/asset.py | 21 +---- kpi/serializers/v2/asset.py | 5 +- 16 files changed, 192 insertions(+), 167 deletions(-) create mode 100644 kobo/apps/long_running_migrations/jobs/0011_migrate_advanced_features.py create mode 100644 kobo/apps/subsequences/utils/action_conversion.py diff --git a/kobo/apps/audit_log/models.py b/kobo/apps/audit_log/models.py index c0827f5af2..7fa16d8f0b 100644 --- a/kobo/apps/audit_log/models.py +++ b/kobo/apps/audit_log/models.py @@ -19,6 +19,7 @@ get_client_ip, get_human_readable_client_user_agent, ) +from kobo.apps.subsequences.constants import Action from kpi.constants import ( ACCESS_LOG_LOGINAS_AUTH_TYPE, ACCESS_LOG_SUBMISSION_AUTH_TYPE, @@ -404,6 +405,8 @@ def create_from_request(cls, request: WSGIRequest): 'submissions-list': cls._create_from_submission_request, 'submission-detail': cls._create_from_submission_request, 'advanced-submission-post': cls._create_from_submission_extra_request, + 'advanced-features-list': cls._create_from_question_advanced_action_request, + 'advanced-features-detail': cls._create_from_question_advanced_action_request, } url_name = request.resolver_match.url_name method = url_name_to_action.get(url_name, None) @@ -787,6 +790,32 @@ def _create_from_permissions_request(cls, request): ) ProjectHistoryLog.objects.bulk_create(logs) + @classmethod + def _create_from_question_advanced_action_request(cls, request): + initial_data = getattr(request, 'initial_data', None) + updated_data = getattr(request, 'updated_data', None) + source_data = updated_data if updated_data else initial_data + asset_uid = request.resolver_match.kwargs['parent_lookup_asset'] + if not source_data: + return + if source_data.get('action') != Action.QUAL: + return + owner = source_data.pop('asset.owner.username') + object_id = source_data.pop('object_id') + metadata = { + 'asset_uid': asset_uid, + 'log_subtype': PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, + 'ip_address': get_client_ip(request), + 'source': get_human_readable_client_user_agent(request), + 'project_owner': owner, + } + action = AuditAction.UPDATE_QA + metadata['qa'] = {PROJECT_HISTORY_LOG_METADATA_FIELD_NEW: source_data['params']} + user = get_database_user(request.user) + ProjectHistoryLog.objects.create( + user=user, object_id=object_id, action=action, metadata=metadata + ) + @classmethod def _create_from_v1_export(cls, request): updated_data = getattr(request, 'updated_data', None) diff --git a/kobo/apps/audit_log/tests/test_project_history_logs.py b/kobo/apps/audit_log/tests/test_project_history_logs.py index b83292cda9..dcfce08e07 100644 --- a/kobo/apps/audit_log/tests/test_project_history_logs.py +++ b/kobo/apps/audit_log/tests/test_project_history_logs.py @@ -28,6 +28,8 @@ remove_uuid_prefix, ) from kobo.apps.openrosa.libs.utils.logger_tools import dict2xform +from kobo.apps.subsequences.models import QuestionAdvancedAction +from kobo.apps.subsequences.constants import Action from kpi.constants import ( ASSET_TYPE_TEMPLATE, CLONE_ARG_NAME, @@ -559,37 +561,35 @@ def test_update_content_creates_log(self, use_v2): use_v2=use_v2, ) - def test_update_qa_creates_log(self): - request_data = { - 'advanced_features': { - 'qual': { - 'qual_survey': [ - { - 'type': 'qual_note', - 'uuid': '12345', - 'scope': 'by_question#survey', - 'xpath': 'q1', - 'labels': {'_default': 'QA Question'}, - # requests to remove a question just add this - # option rather than actually deleting anything - 'options': {'deleted': True}, - } - ] - } - } - } - - log_metadata = self._base_asset_detail_endpoint_test( - patch=True, - url_name=self.detail_url, - request_data=request_data, - expected_action=AuditAction.UPDATE_QA, + def test_add_qa_creates_log(self): + request_data = {"action":"qual", "question_xpath":"Audio", "params": [{"labels": {"_default":"wherefore?"},"uuid":"12345", "type":"qualText"}]} + metadata = self._base_project_history_log_test( + self.client.post, + reverse('api_v2:advanced-features-list', args=[self.asset.uid]), + request_data = request_data, + expected_action = AuditAction.UPDATE_QA, + expected_subtype=PROJECT_HISTORY_LOG_PROJECT_SUBTYPE ) + self.assertEqual(metadata['qa']['new'], request_data['params']) - self.assertEqual( - log_metadata['qa'][PROJECT_HISTORY_LOG_METADATA_FIELD_NEW], - request_data['advanced_features']['qual']['qual_survey'], + def test_update_qa_creates_log(self): + question_qual_action = QuestionAdvancedAction.objects.create( + asset=self.asset, + action=Action.QUAL, + question_xpath='q1', + params=[{"labels": {"_default": "why?"}, "uuid":"12345", "type":"qualText"}] + ) + request_data = {"params": [{"labels": {"_default":"wherefore?"},"uuid":"12345", "type":"qualText"}]} + metadata = self._base_project_history_log_test( + self.client.patch, + reverse('api_v2:advanced-features-detail', args=[self.asset.uid, question_qual_action.uid]), + request_data = request_data, + expected_action = AuditAction.UPDATE_QA, + expected_subtype=PROJECT_HISTORY_LOG_PROJECT_SUBTYPE ) + self.assertEqual(metadata['qa']['new'], request_data['params']) + + def test_failed_qa_update_does_not_create_log(self): # badly formatted QA dict should result in an error before update diff --git a/kobo/apps/long_running_migrations/jobs/0011_migrate_advanced_features.py b/kobo/apps/long_running_migrations/jobs/0011_migrate_advanced_features.py new file mode 100644 index 0000000000..3d84c1e4f3 --- /dev/null +++ b/kobo/apps/long_running_migrations/jobs/0011_migrate_advanced_features.py @@ -0,0 +1,8 @@ +from kobo.apps.subsequences.utils.versioning import migrate_advanced_features +from kpi.models import Asset + + +def run(): + assets = Asset.objects.exclude(advanced_features__iexact='{}') + for asset in assets: + migrate_advanced_features(asset) diff --git a/kobo/apps/subsequences/actions/__init__.py b/kobo/apps/subsequences/actions/__init__.py index e514a954ff..79f339f559 100644 --- a/kobo/apps/subsequences/actions/__init__.py +++ b/kobo/apps/subsequences/actions/__init__.py @@ -2,6 +2,7 @@ from .automatic_google_translation import AutomaticGoogleTranslationAction from .manual_transcription import ManualTranscriptionAction from .manual_translation import ManualTranslationAction +from .qual import QualAction # TODO, what about using a loader for every class in "actions" folder (except base.py)? ACTIONS = ( @@ -9,6 +10,7 @@ AutomaticGoogleTranslationAction, ManualTranscriptionAction, ManualTranslationAction, + QualAction, ) ACTION_IDS_TO_CLASSES = {a.ID: a for a in ACTIONS} diff --git a/kobo/apps/subsequences/constants.py b/kobo/apps/subsequences/constants.py index 2445db6e01..e1dd518a0f 100644 --- a/kobo/apps/subsequences/constants.py +++ b/kobo/apps/subsequences/constants.py @@ -1,3 +1,5 @@ +from django.db import models + SUBMISSION_UUID_FIELD = 'meta/rootUuid' # FIXME: import from elsewhere SUPPLEMENT_KEY = '_supplementalDetails' # leave unchanged for backwards compatibility @@ -20,3 +22,16 @@ '20250820', None ] + + +class Action(models.TextChoices): + MANUAL_TRANSCRIPTION = 'manual_transcription' + MANUAL_TRANSLATION = 'manual_translation' + AUTOMATIC_GOOGLE_TRANSLATION = 'automatic_google_translation' + AUTOMATIC_GOOGLE_TRANSCRIPTION = 'automatic_google_transcription' + QUAL = 'qual' + + +def set_version(schema: dict) -> dict: + schema['_version'] = SCHEMA_VERSIONS[0] + return schema diff --git a/kobo/apps/subsequences/models.py b/kobo/apps/subsequences/models.py index aa89d7db30..5769be0e13 100644 --- a/kobo/apps/subsequences/models.py +++ b/kobo/apps/subsequences/models.py @@ -1,13 +1,13 @@ from django.db import models from kobo.apps.openrosa.apps.logger.xform_instance_parser import remove_uuid_prefix -from kpi.fields import KpiUidField, LazyDefaultJSONBField +from kpi.fields import LazyDefaultJSONBField, KpiUidField from kpi.models.abstract_models import AbstractTimeStampedModel -from .actions import ACTION_IDS_TO_CLASSES -from .actions.base import BaseAction -from .constants import SCHEMA_VERSIONS, SUBMISSION_UUID_FIELD + +from .constants import SCHEMA_VERSIONS, SUBMISSION_UUID_FIELD, Action from .exceptions import InvalidAction, InvalidXPath from .schemas import validate_submission_supplement +from .utils.action_conversion import question_advanced_action_to_action class SubmissionExtras(AbstractTimeStampedModel): @@ -36,7 +36,7 @@ def __repr__(self): @staticmethod def revise_data(asset: 'kpi.Asset', submission: dict, incoming_data: dict) -> dict: - if not asset.advanced_features_set.objects.exists(): + if not asset.advanced_features_set.exists(): raise InvalidAction schema_version = incoming_data.get('_version') @@ -49,10 +49,6 @@ def revise_data(asset: 'kpi.Asset', submission: dict, incoming_data: dict) -> di # TODO: migrate from old per-submission schema raise NotImplementedError - if asset.advanced_features.get('_version') != schema_version: - # TODO: migrate from old per-asset schema - raise NotImplementedError - submission_uuid = remove_uuid_prefix(submission[SUBMISSION_UUID_FIELD]) # constant? supplemental_data = SubmissionExtras.objects.get_or_create( asset=asset, submission_uuid=submission_uuid @@ -74,15 +70,11 @@ def revise_data(asset: 'kpi.Asset', submission: dict, incoming_data: dict) -> di for action_id, action_data in data_for_this_question.items(): try: - action_class = ACTION_IDS_TO_CLASSES[action_id] - except KeyError as e: - raise InvalidAction from e - try: - action_params = action_configs_for_this_question.get(action=action_id).config + question_advanced_action = action_configs_for_this_question.get(action=action_id) except QuestionAdvancedAction.DoesNotExist as e: raise InvalidAction from e - action = action_class(question_xpath, action_params, asset) + action = question_advanced_action_to_action(question_advanced_action) action.check_limits(asset.owner) question_supplemental_data = supplemental_data.setdefault( @@ -161,7 +153,7 @@ def retrieve_data( # TODO: migrate from old per-submission schema raise NotImplementedError - if asset.advanced_features.get('_version') != schema_version: + if not asset.advanced_features_set.exists(): # TODO: migrate from old per-asset schema raise NotImplementedError @@ -178,14 +170,7 @@ def retrieve_data( for action_id, action_data in data_for_this_question.items(): try: - action_class = ACTION_IDS_TO_CLASSES[action_id] - except KeyError: - # An action class present in the submission data no longer - # exists in the application code - # TODO: log an error - continue - try: - action_params = action_configs_for_this_question.get(action=action_id) + question_advanced_action = action_configs_for_this_question.get(action=action_id) except QuestionAdvancedAction.DoesNotExist: # An action class present in the submission data is no longer # configured at the asset level for this question @@ -194,7 +179,7 @@ def retrieve_data( # Actions could be disabled or hidden instead of being removed continue - action = action_class(question_xpath, action_params) + action = question_advanced_action_to_action(question_advanced_action) retrieved_data = action.retrieve_data(action_data) processed_data_for_this_question[action_id] = retrieved_data @@ -225,12 +210,6 @@ def retrieve_data( return retrieved_supplemental_data -class Action(models.TextChoices): - MANUAL_TRANSCRIPTION = 'manual_transcription' - MANUAL_TRANSLATION = 'manual_translation' - AUTOMATIC_GOOGLE_TRANSLATION = 'automatic_google_translation' - AUTOMATIC_GOOGLE_TRANSCRIPTION = 'automatic_google_transcription' - QUAL = 'qual' class QuestionAdvancedAction(models.Model): uid = KpiUidField(uid_prefix='qaa', primary_key=True) @@ -246,15 +225,7 @@ class QuestionAdvancedAction(models.Model): question_xpath = models.CharField( null=False, blank=False, max_length=2000 ) - config = LazyDefaultJSONBField(default=dict) + params = LazyDefaultJSONBField(default=dict) class Meta: unique_together = ('asset_id', 'question_xpath', 'action') - - def to_action(self) -> BaseAction: - action_class = ACTION_IDS_TO_CLASSES[self.action] - return action_class( - source_question_xpath=self.question_xpath, - params=self.config, - asset=self.asset, - ) diff --git a/kobo/apps/subsequences/schemas.py b/kobo/apps/subsequences/schemas.py index 333ff79c32..a2493b6afb 100644 --- a/kobo/apps/subsequences/schemas.py +++ b/kobo/apps/subsequences/schemas.py @@ -2,7 +2,6 @@ from .actions import ACTION_IDS_TO_CLASSES, ACTIONS from .constants import SCHEMA_VERSIONS -from .utils.versioning import migrate_advanced_features # not the full complexity of XPath, but a slash-delimited path of valid XML tag # names to convey group hierarchy @@ -34,8 +33,6 @@ def validate_submission_supplement(asset: 'kpi.models.Asset', supplement: dict): def get_submission_supplement_schema(asset: 'kpi.models.Asset') -> dict: - if migrated_schema := migrate_advanced_features(asset.advanced_features): - asset.advanced_features = migrated_schema submission_supplement_schema = { 'additionalProperties': False, diff --git a/kobo/apps/subsequences/serializers.py b/kobo/apps/subsequences/serializers.py index c90b8d3083..b0f4170578 100644 --- a/kobo/apps/subsequences/serializers.py +++ b/kobo/apps/subsequences/serializers.py @@ -2,32 +2,34 @@ from rest_framework import serializers from kobo.apps.subsequences.models import QuestionAdvancedAction +from kobo.apps.subsequences.utils.action_conversion import question_advanced_action_to_action class QuestionAdvancedActionUpdateSerializer(serializers.ModelSerializer): class Meta: model = QuestionAdvancedAction - fields = ['config'] - read_only_fields = ['question_xpath', 'action', 'asset'] + fields = ['params', 'question_xpath', 'action', 'asset', 'uid'] + read_only_fields = ['question_xpath', 'action', 'asset', 'uid'] def validate(self, attrs): data = super().validate(attrs) - action = self.instance.to_action() + action = question_advanced_action_to_action(instance) try: - action.__class__.validate_params(attrs['config']) + action.__class__.validate_params(attrs['params']) except jsonschema.exceptions.ValidationError as ve: raise serializers.ValidationError(ve) return data def update(self, instance, validated_data): - action = instance.to_action() - action.update_params(validated_data['config']) - instance.config = action.params - instance.save(update_fields=['config']) + action = question_advanced_action_to_action(instance) + action.update_params(validated_data['params']) + instance.params = action.params + instance.save(update_fields=['params']) return instance class QuestionAdvancedActionSerializer(serializers.ModelSerializer): class Meta: model = QuestionAdvancedAction - fields = ['question_xpath', 'action', 'config'] + fields = ['question_xpath', 'action', 'params', 'uid'] + read_only_fields = ['uid'] diff --git a/kobo/apps/subsequences/tasks.py b/kobo/apps/subsequences/tasks.py index 8086402cbd..5927183d56 100644 --- a/kobo/apps/subsequences/tasks.py +++ b/kobo/apps/subsequences/tasks.py @@ -4,9 +4,9 @@ from kobo.celery import celery_app from kpi.utils.django_orm_helper import UpdateJSONFieldAttributes from kobo.apps.subsequences.exceptions import SubsequenceTimeoutError -from .constants import SUBMISSION_UUID_FIELD +from .constants import SUBMISSION_UUID_FIELD, set_version from kobo.apps.openrosa.apps.logger.xform_instance_parser import remove_uuid_prefix -from .utils.versioning import set_version + # With retry_backoff=5 and retry_backoff_max=60, each retry waits: # min(5 * 2^(n-1), 60) seconds. diff --git a/kobo/apps/subsequences/tests/api/v2/test_views.py b/kobo/apps/subsequences/tests/api/v2/test_views.py index 96f382eaa2..26b0bd38d7 100644 --- a/kobo/apps/subsequences/tests/api/v2/test_views.py +++ b/kobo/apps/subsequences/tests/api/v2/test_views.py @@ -23,7 +23,7 @@ def setUp(self): asset=self.asset, question_xpath='q1', action='manual_transcription', - config=[{'language': 'en'}], + params=[{'language': 'en'}], ) self.list_actions_url = reverse( 'api_v2:advanced-features-list', @@ -43,28 +43,28 @@ def test_update_action(self): res = self.client.patch( self.action_detail_url, content_type='application/json', - data=json.dumps({'config': [{'language': 'es'}]}), + data=json.dumps({'question_xpath': 'bad'}), ) assert res.status_code == status.HTTP_200_OK self.action.refresh_from_db() - assert self.action.config == [{'language': 'en'}, {'language': 'es'}] + assert self.action.params == [{'language': 'en'}, {'language': 'es'}] def test_cannot_update_action_with_invalid_params(self): res = self.client.patch( self.action_detail_url, content_type='application/json', - data=json.dumps({'config': [{'bad': 'stuff'}]}), + data=json.dumps({'params': [{'bad': 'stuff'}]}), ) assert res.status_code == status.HTTP_400_BAD_REQUEST self.action.refresh_from_db() - assert self.action.config == [{'language': 'en'}] + assert self.action.params == [{'language': 'en'}] def test_create_action(self): res = self.client.post( self.list_actions_url, data={ 'action': 'manual_translation', - 'config': json.dumps([{'language': 'de'}]), + 'params': json.dumps([{'language': 'de'}]), 'question_xpath': 'q1', }, ) @@ -72,7 +72,7 @@ def test_create_action(self): new_action = QuestionAdvancedAction.objects.get( asset=self.asset, action='manual_translation' ) - assert new_action.config == [{'language': 'de'}] + assert new_action.params == [{'language': 'de'}] assert new_action.question_xpath == 'q1' def test_cannot_delete_actions(self): diff --git a/kobo/apps/subsequences/utils/action_conversion.py b/kobo/apps/subsequences/utils/action_conversion.py new file mode 100644 index 0000000000..2a8caf3de9 --- /dev/null +++ b/kobo/apps/subsequences/utils/action_conversion.py @@ -0,0 +1,10 @@ +from kobo.apps.subsequences.actions import ACTION_IDS_TO_CLASSES + + +def question_advanced_action_to_action(qaa) : + action_class = ACTION_IDS_TO_CLASSES[qaa.action] + return action_class( + source_question_xpath=qaa.question_xpath, + params=qaa.params, + asset=qaa.asset, + ) diff --git a/kobo/apps/subsequences/utils/supplement_data.py b/kobo/apps/subsequences/utils/supplement_data.py index 613bed6611..548216b74b 100644 --- a/kobo/apps/subsequences/utils/supplement_data.py +++ b/kobo/apps/subsequences/utils/supplement_data.py @@ -2,9 +2,9 @@ from kobo.apps.openrosa.apps.logger.xform_instance_parser import remove_uuid_prefix from kobo.apps.subsequences.actions import ACTION_IDS_TO_CLASSES -from kobo.apps.subsequences.constants import SUBMISSION_UUID_FIELD, SUPPLEMENT_KEY, \ - SCHEMA_VERSIONS +from kobo.apps.subsequences.constants import SUBMISSION_UUID_FIELD, SUPPLEMENT_KEY from kobo.apps.subsequences.models import SubmissionSupplement +from kobo.apps.subsequences.utils.action_conversion import question_advanced_action_to_action from kobo.apps.subsequences.utils.versioning import migrate_advanced_features @@ -37,21 +37,13 @@ def get_supplemental_output_fields(asset: 'kpi.models.Asset') -> list[dict]: submission. We'll do that by looking at the acceptance dates and letting the most recent win """ - advanced_features = asset.advanced_features - - if migrated_schema := migrate_advanced_features(advanced_features): - asset.advanced_features = migrated_schema + advanced_features = asset.advanced_features_set.all() output_fields_by_name = {} # FIXME: `_actionConfigs` is 👎 and should be dropped in favor of top-level configs, eh? # data already exists at the top level alongisde leading-underscore metadata like _version - for source_question_xpath, per_question_actions in advanced_features[ - '_actionConfigs' - ].items(): - for action_id, action_config in per_question_actions.items(): - action = ACTION_IDS_TO_CLASSES[action_id]( - source_question_xpath, action_config - ) + for question_advanced_action in advanced_features: + action = question_advanced_action_to_action(question_advanced_action) for field in action.get_output_fields(): try: existing = output_fields_by_name[field['name']] @@ -98,3 +90,4 @@ def stream_with_supplements( prefetched_supplement=extras.get(submission_uuid, {}), ) yield submission + diff --git a/kobo/apps/subsequences/utils/versioning.py b/kobo/apps/subsequences/utils/versioning.py index aba7b21852..038e351639 100644 --- a/kobo/apps/subsequences/utils/versioning.py +++ b/kobo/apps/subsequences/utils/versioning.py @@ -1,44 +1,58 @@ -from ..constants import SCHEMA_VERSIONS +from django.db import transaction +from ..constants import SCHEMA_VERSIONS, Action +from ..models import QuestionAdvancedAction +from ...openrosa.apps.logger.models import XForm +from ...openrosa.apps.logger.xform_instance_parser import get_abbreviated_xpath -def migrate_advanced_features(advanced_features: dict) -> dict | None: - if advanced_features.get('_version') == SCHEMA_VERSIONS[0]: +def migrate_advanced_features(asset: 'kpi.models.Asset') -> dict | None: + advanced_features = asset.advanced_features + if advanced_features == {}: return - migrated_advanced_features = { - '_version': SCHEMA_VERSIONS[0], - '_actionConfigs': {} - } - - actionConfigs = migrated_advanced_features['_actionConfigs'] - for key, value in advanced_features.items(): - if ( - key == 'transcript' - and value - and 'languages' in value - and value['languages'] - ): - actionConfigs['manual_transcription'] = [ - {'language': language} for language in value['languages'] - ] - - if ( - key == 'translation' - and value - and 'languages' in value - and value['languages'] - ): - actionConfigs['manual_translation'] = [ - {'language': language} for language in value['languages'] - ] - - if key == 'qual': - raise NotImplementedError - - return migrated_advanced_features - - -def set_version(schema: dict) -> dict: - schema['_version'] = SCHEMA_VERSIONS[0] - return schema + xform = XForm.objects.get(kpi_asset_uid=asset.uid) + data_dict = xform.data_dictionary() + + audio_questions = data_dict.get_survey_elements_of_type('audio') + with transaction.atomic(): + for key, value in advanced_features.items(): + if ( + key == 'transcript' + and value + and 'languages' in value + and value['languages'] + ): + for q in audio_questions: + QuestionAdvancedAction.objects.create( + question_xpath=get_abbreviated_xpath(q), + asset=asset, + action=Action.MANUAL_TRANSCRIPTION, + params=[ + {'language': language} for language in value['languages'] + ] + ) + + if ( + key == 'translation' + and value + and 'languages' in value + and value['languages'] + ): + for q in audio_questions: + QuestionAdvancedAction.objects.create( + question_xpath=get_abbreviated_xpath(q), + asset=asset, + action=Action.MANUAL_TRANSCRIPTION, + params=[ + {'language': language} for language in value['languages'] + ] + ) + if key == 'qual': + # TODO: DEV-1295 + pass + asset.advanced_features = {} + asset.save(update_fields=['advanced_features'], adjust_content=False) + + + diff --git a/kobo/apps/subsequences/views.py b/kobo/apps/subsequences/views.py index 3bcdd46a5c..16589c470f 100644 --- a/kobo/apps/subsequences/views.py +++ b/kobo/apps/subsequences/views.py @@ -1,6 +1,8 @@ from rest_framework import mixins, viewsets from rest_framework_extensions.mixins import NestedViewSetMixin +from kobo.apps.audit_log.base_views import AuditLoggedViewSet +from kobo.apps.audit_log.models import AuditType from kobo.apps.subsequences.models import QuestionAdvancedAction from kobo.apps.subsequences.serializers import ( QuestionAdvancedActionSerializer, @@ -10,7 +12,7 @@ class QuestionAdvancedActionViewSet( - viewsets.GenericViewSet, + AuditLoggedViewSet, mixins.CreateModelMixin, mixins.RetrieveModelMixin, mixins.UpdateModelMixin, @@ -18,9 +20,13 @@ class QuestionAdvancedActionViewSet( AssetNestedObjectViewsetMixin, NestedViewSetMixin, ): + log_type = AuditType.PROJECT_HISTORY + logged_fields = ['asset.owner.username', 'action', 'params',('object_id', 'asset.id'),] + pagination_class = None def get_queryset(self): return QuestionAdvancedAction.objects.filter(asset=self.asset) - def perform_create(self, serializer): + def perform_create_override(self, serializer): + serializer.save(asset=self.asset) def get_serializer_class(self): if self.action in ['update', 'partial_update']: diff --git a/kpi/models/asset.py b/kpi/models/asset.py index 87b0f35e50..51c3c9ec79 100644 --- a/kpi/models/asset.py +++ b/kpi/models/asset.py @@ -19,9 +19,7 @@ from kobo.apps.data_collectors.models import DataCollectorGroup from kobo.apps.reports.constants import DEFAULT_REPORTS_KEY, SPECIFIC_REPORTS_KEY -from kobo.apps.subsequences.schemas import ACTION_PARAMS_SCHEMA from kobo.apps.subsequences.utils.supplement_data import get_supplemental_output_fields -from kobo.apps.subsequences.utils.versioning import migrate_advanced_features from kpi.constants import ( ASSET_TYPE_BLOCK, ASSET_TYPE_COLLECTION, @@ -929,11 +927,6 @@ def save( if adjust_content: self.adjust_content_on_save() - if ( - not update_fields - or update_fields and 'advanced_features' in update_fields - ): - self.validate_advanced_features() # standardize settings (only when required) if ( @@ -1152,19 +1145,7 @@ def update_languages(self, children=None): self.save(update_fields=['summary']) def validate_advanced_features(self): - if self.advanced_features is None: - self.advanced_features = {} - - if migrated_schema := migrate_advanced_features(self.advanced_features): - self.advanced_features = migrated_schema - # We should save the new schema, but for debugging purposes, - # we don't yet! - # self.save(update_fields=['advanced_features']) - - jsonschema.validate( - instance=self.advanced_features, - schema=ACTION_PARAMS_SCHEMA, - ) + pass @property def version__content_hash(self): diff --git a/kpi/serializers/v2/asset.py b/kpi/serializers/v2/asset.py index bba8de95fb..9ba0b8172e 100644 --- a/kpi/serializers/v2/asset.py +++ b/kpi/serializers/v2/asset.py @@ -343,9 +343,6 @@ class AssetSerializer(serializers.HyperlinkedModelSerializer): map_custom = WriteableJsonWithSchemaField( schema_field=MapCustomField, required=False ) - advanced_features = WriteableJsonWithSchemaField( - schema_field=AdvancedFeatureField, required=False - ) files = serializers.SerializerMethodField() xls_link = serializers.SerializerMethodField() summary = ReadOnlyFieldWithSchemaField(schema_field=SummaryField) @@ -403,6 +400,7 @@ class AssetSerializer(serializers.HyperlinkedModelSerializer): paired_data = serializers.SerializerMethodField() project_ownership = serializers.SerializerMethodField() kind = serializers.SerializerMethodField() + advanced_features = '' class Meta: model = Asset @@ -435,7 +433,6 @@ class Meta: 'deployment_status', 'report_styles', 'report_custom', - 'advanced_features', 'supplemental_output_fields', 'map_styles', 'map_custom', From 50148abdd09cae4acc853bb14156f03cae44baa3 Mon Sep 17 00:00:00 2001 From: rgraber Date: Mon, 17 Nov 2025 08:52:12 -0500 Subject: [PATCH 05/20] fixup!: second migration file will be removed --- ...ame_config_questionadvancedaction_params.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 kobo/apps/subsequences/migrations/0006_rename_config_questionadvancedaction_params.py diff --git a/kobo/apps/subsequences/migrations/0006_rename_config_questionadvancedaction_params.py b/kobo/apps/subsequences/migrations/0006_rename_config_questionadvancedaction_params.py new file mode 100644 index 0000000000..d3297d1b90 --- /dev/null +++ b/kobo/apps/subsequences/migrations/0006_rename_config_questionadvancedaction_params.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.24 on 2025-11-14 14:46 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('subsequences', '0005_submissionsupplement_questionadvancedaction'), + ] + + operations = [ + migrations.RenameField( + model_name='questionadvancedaction', + old_name='config', + new_name='params', + ), + ] From 7eca86344733b66d52e5a6aef3e090e935d27ad5 Mon Sep 17 00:00:00 2001 From: rgraber Date: Mon, 17 Nov 2025 09:53:21 -0500 Subject: [PATCH 06/20] fixup!: stuff --- kobo/apps/subsequences/models.py | 10 +--------- kobo/apps/subsequences/utils/versioning.py | 8 ++++---- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/kobo/apps/subsequences/models.py b/kobo/apps/subsequences/models.py index 5769be0e13..9da494a37c 100644 --- a/kobo/apps/subsequences/models.py +++ b/kobo/apps/subsequences/models.py @@ -169,15 +169,7 @@ def retrieve_data( continue for action_id, action_data in data_for_this_question.items(): - try: - question_advanced_action = action_configs_for_this_question.get(action=action_id) - except QuestionAdvancedAction.DoesNotExist: - # An action class present in the submission data is no longer - # configured at the asset level for this question - # Allow this for now, but maybe forbid later and also forbid - # removing things from the asset-level action configuration? - # Actions could be disabled or hidden instead of being removed - continue + question_advanced_action = action_configs_for_this_question.get(action=action_id) action = question_advanced_action_to_action(question_advanced_action) diff --git a/kobo/apps/subsequences/utils/versioning.py b/kobo/apps/subsequences/utils/versioning.py index 038e351639..3f05f5b6eb 100644 --- a/kobo/apps/subsequences/utils/versioning.py +++ b/kobo/apps/subsequences/utils/versioning.py @@ -8,13 +8,13 @@ def migrate_advanced_features(asset: 'kpi.models.Asset') -> dict | None: advanced_features = asset.advanced_features + known_cols = set([col.split(":")[0] for col in asset.known_cols]) + if advanced_features == {}: return xform = XForm.objects.get(kpi_asset_uid=asset.uid) - data_dict = xform.data_dictionary() - audio_questions = data_dict.get_survey_elements_of_type('audio') with transaction.atomic(): for key, value in advanced_features.items(): if ( @@ -23,7 +23,7 @@ def migrate_advanced_features(asset: 'kpi.models.Asset') -> dict | None: and 'languages' in value and value['languages'] ): - for q in audio_questions: + for q in known_cols: QuestionAdvancedAction.objects.create( question_xpath=get_abbreviated_xpath(q), asset=asset, @@ -39,7 +39,7 @@ def migrate_advanced_features(asset: 'kpi.models.Asset') -> dict | None: and 'languages' in value and value['languages'] ): - for q in audio_questions: + for q in known_cols: QuestionAdvancedAction.objects.create( question_xpath=get_abbreviated_xpath(q), asset=asset, From b448f8cc0cbbbf1e51189e864dfe6432e5804621 Mon Sep 17 00:00:00 2001 From: rgraber Date: Mon, 17 Nov 2025 17:15:52 +0100 Subject: [PATCH 07/20] fixup!: stuff --- kobo/apps/subsequences/models.py | 49 +++++++++++++++- kobo/apps/subsequences/schemas.py | 23 +++----- .../subsequences/utils/supplement_data.py | 2 - kobo/apps/subsequences/utils/versioning.py | 58 ------------------- 4 files changed, 57 insertions(+), 75 deletions(-) diff --git a/kobo/apps/subsequences/models.py b/kobo/apps/subsequences/models.py index 9da494a37c..83b71f78d3 100644 --- a/kobo/apps/subsequences/models.py +++ b/kobo/apps/subsequences/models.py @@ -1,4 +1,4 @@ -from django.db import models +from django.db import models, transaction from kobo.apps.openrosa.apps.logger.xform_instance_parser import remove_uuid_prefix from kpi.fields import LazyDefaultJSONBField, KpiUidField @@ -221,3 +221,50 @@ class QuestionAdvancedAction(models.Model): class Meta: unique_together = ('asset_id', 'question_xpath', 'action') + +def migrate_advanced_features(asset: 'kpi.models.Asset') -> dict | None: + advanced_features = asset.advanced_features + known_cols = set([col.split(":")[0] for col in asset.known_cols]) + + if advanced_features == {}: + return + + with transaction.atomic(): + for key, value in advanced_features.items(): + if ( + key == 'transcript' + and value + and 'languages' in value + and value['languages'] + ): + for q in known_cols: + QuestionAdvancedAction.objects.create( + question_xpath=q, + asset=asset, + action=Action.MANUAL_TRANSCRIPTION, + params=[ + {'language': language} for language in value['languages'] + ] + ) + + if ( + key == 'translation' + and value + and 'languages' in value + and value['languages'] + ): + for q in known_cols: + QuestionAdvancedAction.objects.create( + question_xpath=q, + asset=asset, + action=Action.MANUAL_TRANSCRIPTION, + params=[ + {'language': language} for language in value['languages'] + ] + ) + if key == 'qual': + # TODO: DEV-1295 + pass + asset.advanced_features = {} + asset.save(update_fields=['advanced_features'], adjust_content=False) + diff --git a/kobo/apps/subsequences/schemas.py b/kobo/apps/subsequences/schemas.py index a2493b6afb..8b88c08b27 100644 --- a/kobo/apps/subsequences/schemas.py +++ b/kobo/apps/subsequences/schemas.py @@ -2,6 +2,8 @@ from .actions import ACTION_IDS_TO_CLASSES, ACTIONS from .constants import SCHEMA_VERSIONS +from .models import migrate_advanced_features +from .utils.action_conversion import question_advanced_action_to_action # not the full complexity of XPath, but a slash-delimited path of valid XML tag # names to convey group hierarchy @@ -32,25 +34,18 @@ def validate_submission_supplement(asset: 'kpi.models.Asset', supplement: dict): def get_submission_supplement_schema(asset: 'kpi.models.Asset') -> dict: - + if asset.advanced_features != {}: + migrate_advanced_features(asset) submission_supplement_schema = { 'additionalProperties': False, 'properties': {'_version': {'const': SCHEMA_VERSIONS[0]}}, 'type': 'object', } - - for ( - question_xpath, - action_configs_for_this_question, - ) in asset.advanced_features['_actionConfigs'].items(): - for ( - action_id, - action_params, - ) in action_configs_for_this_question.items(): - action = ACTION_IDS_TO_CLASSES[action_id](question_xpath, action_params) - submission_supplement_schema['properties'].setdefault(question_xpath, {})[ - action_id - ] = action.result_schema + for question_advanced_action in asset.advanced_features_set.all(): + action = question_advanced_action_to_action(question_advanced_action) + submission_supplement_schema['properties'].setdefault(question_advanced_action.question_xpath, {})[ + question_advanced_action.action + ] = action.result_schema return submission_supplement_schema diff --git a/kobo/apps/subsequences/utils/supplement_data.py b/kobo/apps/subsequences/utils/supplement_data.py index 548216b74b..0e6688ac42 100644 --- a/kobo/apps/subsequences/utils/supplement_data.py +++ b/kobo/apps/subsequences/utils/supplement_data.py @@ -1,11 +1,9 @@ from typing import Generator from kobo.apps.openrosa.apps.logger.xform_instance_parser import remove_uuid_prefix -from kobo.apps.subsequences.actions import ACTION_IDS_TO_CLASSES from kobo.apps.subsequences.constants import SUBMISSION_UUID_FIELD, SUPPLEMENT_KEY from kobo.apps.subsequences.models import SubmissionSupplement from kobo.apps.subsequences.utils.action_conversion import question_advanced_action_to_action -from kobo.apps.subsequences.utils.versioning import migrate_advanced_features def get_supplemental_output_fields(asset: 'kpi.models.Asset') -> list[dict]: diff --git a/kobo/apps/subsequences/utils/versioning.py b/kobo/apps/subsequences/utils/versioning.py index 3f05f5b6eb..e69de29bb2 100644 --- a/kobo/apps/subsequences/utils/versioning.py +++ b/kobo/apps/subsequences/utils/versioning.py @@ -1,58 +0,0 @@ -from django.db import transaction - -from ..constants import SCHEMA_VERSIONS, Action -from ..models import QuestionAdvancedAction -from ...openrosa.apps.logger.models import XForm -from ...openrosa.apps.logger.xform_instance_parser import get_abbreviated_xpath - - -def migrate_advanced_features(asset: 'kpi.models.Asset') -> dict | None: - advanced_features = asset.advanced_features - known_cols = set([col.split(":")[0] for col in asset.known_cols]) - - if advanced_features == {}: - return - - xform = XForm.objects.get(kpi_asset_uid=asset.uid) - - with transaction.atomic(): - for key, value in advanced_features.items(): - if ( - key == 'transcript' - and value - and 'languages' in value - and value['languages'] - ): - for q in known_cols: - QuestionAdvancedAction.objects.create( - question_xpath=get_abbreviated_xpath(q), - asset=asset, - action=Action.MANUAL_TRANSCRIPTION, - params=[ - {'language': language} for language in value['languages'] - ] - ) - - if ( - key == 'translation' - and value - and 'languages' in value - and value['languages'] - ): - for q in known_cols: - QuestionAdvancedAction.objects.create( - question_xpath=get_abbreviated_xpath(q), - asset=asset, - action=Action.MANUAL_TRANSCRIPTION, - params=[ - {'language': language} for language in value['languages'] - ] - ) - if key == 'qual': - # TODO: DEV-1295 - pass - asset.advanced_features = {} - asset.save(update_fields=['advanced_features'], adjust_content=False) - - - From 7ef6af36347af5ac880c47cf5abdbcc60677be8f Mon Sep 17 00:00:00 2001 From: rgraber Date: Tue, 18 Nov 2025 09:32:14 +0100 Subject: [PATCH 08/20] fixup!: tests and stuff --- .../jobs/0011_migrate_advanced_features.py | 8 -- kobo/apps/subsequences/actions/base.py | 1 + kobo/apps/subsequences/models.py | 33 ++++- kobo/apps/subsequences/schemas.py | 26 +--- kobo/apps/subsequences/serializers.py | 8 +- .../tests/api/v2/test_permissions.py | 23 ++- .../tests/api/v2/test_validation.py | 135 ++++++++---------- .../subsequences/tests/api/v2/test_views.py | 2 +- kobo/apps/subsequences/tests/test_models.py | 39 ++--- .../subsequences/utils/supplement_data.py | 11 +- kobo/apps/subsequences/utils/versioning.py | 0 kobo/apps/subsequences/views.py | 3 +- 12 files changed, 124 insertions(+), 165 deletions(-) delete mode 100644 kobo/apps/long_running_migrations/jobs/0011_migrate_advanced_features.py delete mode 100644 kobo/apps/subsequences/utils/versioning.py diff --git a/kobo/apps/long_running_migrations/jobs/0011_migrate_advanced_features.py b/kobo/apps/long_running_migrations/jobs/0011_migrate_advanced_features.py deleted file mode 100644 index 3d84c1e4f3..0000000000 --- a/kobo/apps/long_running_migrations/jobs/0011_migrate_advanced_features.py +++ /dev/null @@ -1,8 +0,0 @@ -from kobo.apps.subsequences.utils.versioning import migrate_advanced_features -from kpi.models import Asset - - -def run(): - assets = Asset.objects.exclude(advanced_features__iexact='{}') - for asset in assets: - migrate_advanced_features(asset) diff --git a/kobo/apps/subsequences/actions/base.py b/kobo/apps/subsequences/actions/base.py index aed04d2834..15f9e61c6e 100644 --- a/kobo/apps/subsequences/actions/base.py +++ b/kobo/apps/subsequences/actions/base.py @@ -606,6 +606,7 @@ def languages(self) -> list[str]: return languages def update_params(self, incoming_params): + self.validate_params(incoming_params) current_languages = self.languages for language_obj in incoming_params: if language_obj['language'] not in current_languages: diff --git a/kobo/apps/subsequences/models.py b/kobo/apps/subsequences/models.py index 83b71f78d3..c777a2c236 100644 --- a/kobo/apps/subsequences/models.py +++ b/kobo/apps/subsequences/models.py @@ -1,12 +1,11 @@ +import jsonschema from django.db import models, transaction from kobo.apps.openrosa.apps.logger.xform_instance_parser import remove_uuid_prefix -from kpi.fields import LazyDefaultJSONBField, KpiUidField +from kpi.fields import KpiUidField, LazyDefaultJSONBField from kpi.models.abstract_models import AbstractTimeStampedModel - from .constants import SCHEMA_VERSIONS, SUBMISSION_UUID_FIELD, Action from .exceptions import InvalidAction, InvalidXPath -from .schemas import validate_submission_supplement from .utils.action_conversion import question_advanced_action_to_action @@ -37,7 +36,7 @@ def __repr__(self): def revise_data(asset: 'kpi.Asset', submission: dict, incoming_data: dict) -> dict: if not asset.advanced_features_set.exists(): - raise InvalidAction + migrate_advanced_features(asset) schema_version = incoming_data.get('_version') @@ -63,7 +62,6 @@ def revise_data(asset: 'kpi.Asset', submission: dict, incoming_data: dict) -> di # FIXME: what's a better way? skip all leading underscore keys? # pop off the known special keys first? continue - action_configs_for_this_question = asset.advanced_features_set.filter(question_xpath=question_xpath) if not action_configs_for_this_question.exists(): raise InvalidXPath @@ -155,7 +153,7 @@ def retrieve_data( if not asset.advanced_features_set.exists(): # TODO: migrate from old per-asset schema - raise NotImplementedError + migrate_advanced_features(asset) retrieved_supplemental_data = {} data_for_output = {} @@ -224,7 +222,7 @@ class Meta: def migrate_advanced_features(asset: 'kpi.models.Asset') -> dict | None: advanced_features = asset.advanced_features - known_cols = set([col.split(":")[0] for col in asset.known_cols]) + known_cols = set([col.split(':')[0] for col in asset.known_cols]) if advanced_features == {}: return @@ -238,6 +236,7 @@ def migrate_advanced_features(asset: 'kpi.models.Asset') -> dict | None: and value['languages'] ): for q in known_cols: + print(f'{q=}') QuestionAdvancedAction.objects.create( question_xpath=q, asset=asset, @@ -268,3 +267,23 @@ def migrate_advanced_features(asset: 'kpi.models.Asset') -> dict | None: asset.advanced_features = {} asset.save(update_fields=['advanced_features'], adjust_content=False) +def validate_submission_supplement(asset: 'kpi.models.Asset', supplement: dict): + jsonschema.validate(get_submission_supplement_schema(asset), supplement) + + +def get_submission_supplement_schema(asset: 'kpi.models.Asset') -> dict: + if asset.advanced_features != {}: + migrate_advanced_features(asset) + + submission_supplement_schema = { + 'additionalProperties': False, + 'properties': {'_version': {'const': SCHEMA_VERSIONS[0]}}, + 'type': 'object', + } + for question_advanced_action in asset.advanced_features_set.all(): + action = question_advanced_action_to_action(question_advanced_action) + submission_supplement_schema['properties'].setdefault( + question_advanced_action.question_xpath, {} + )[question_advanced_action.action] = action.result_schema + + return submission_supplement_schema diff --git a/kobo/apps/subsequences/schemas.py b/kobo/apps/subsequences/schemas.py index 8b88c08b27..ad1108daf6 100644 --- a/kobo/apps/subsequences/schemas.py +++ b/kobo/apps/subsequences/schemas.py @@ -1,9 +1,5 @@ -import jsonschema -from .actions import ACTION_IDS_TO_CLASSES, ACTIONS -from .constants import SCHEMA_VERSIONS -from .models import migrate_advanced_features -from .utils.action_conversion import question_advanced_action_to_action +from .actions import ACTIONS # not the full complexity of XPath, but a slash-delimited path of valid XML tag # names to convey group hierarchy @@ -29,23 +25,3 @@ } -def validate_submission_supplement(asset: 'kpi.models.Asset', supplement: dict): - jsonschema.validate(get_submission_supplement_schema(asset), supplement) - - -def get_submission_supplement_schema(asset: 'kpi.models.Asset') -> dict: - if asset.advanced_features != {}: - migrate_advanced_features(asset) - - submission_supplement_schema = { - 'additionalProperties': False, - 'properties': {'_version': {'const': SCHEMA_VERSIONS[0]}}, - 'type': 'object', - } - for question_advanced_action in asset.advanced_features_set.all(): - action = question_advanced_action_to_action(question_advanced_action) - submission_supplement_schema['properties'].setdefault(question_advanced_action.question_xpath, {})[ - question_advanced_action.action - ] = action.result_schema - - return submission_supplement_schema diff --git a/kobo/apps/subsequences/serializers.py b/kobo/apps/subsequences/serializers.py index b0f4170578..d62b796497 100644 --- a/kobo/apps/subsequences/serializers.py +++ b/kobo/apps/subsequences/serializers.py @@ -2,7 +2,9 @@ from rest_framework import serializers from kobo.apps.subsequences.models import QuestionAdvancedAction -from kobo.apps.subsequences.utils.action_conversion import question_advanced_action_to_action +from kobo.apps.subsequences.utils.action_conversion import ( + question_advanced_action_to_action, +) class QuestionAdvancedActionUpdateSerializer(serializers.ModelSerializer): @@ -13,9 +15,9 @@ class Meta: def validate(self, attrs): data = super().validate(attrs) - action = question_advanced_action_to_action(instance) + action = question_advanced_action_to_action(self.instance) try: - action.__class__.validate_params(attrs['params']) + action.__class__.validate_params(attrs.get('params')) except jsonschema.exceptions.ValidationError as ve: raise serializers.ValidationError(ve) return data diff --git a/kobo/apps/subsequences/tests/api/v2/test_permissions.py b/kobo/apps/subsequences/tests/api/v2/test_permissions.py index 603a51128c..b035123ce0 100644 --- a/kobo/apps/subsequences/tests/api/v2/test_permissions.py +++ b/kobo/apps/subsequences/tests/api/v2/test_permissions.py @@ -8,6 +8,7 @@ from rest_framework import status from kobo.apps.kobo_auth.shortcuts import User +from kobo.apps.subsequences.models import QuestionAdvancedAction from kobo.apps.subsequences.tests.api.v2.base import SubsequenceBaseTestCase from kpi.constants import ( PERM_CHANGE_SUBMISSIONS, @@ -137,17 +138,11 @@ def test_can_write(self, username, shared, status_code): self.client.force_login(user) # Activate advanced features for the project - self.set_asset_advanced_features( - { - '_version': '20250820', - '_actionConfigs': { - 'q1': { - 'manual_transcription': [ - {'language': 'es'}, - ] - } - }, - } + QuestionAdvancedAction.objects.create( + question_xpath='q1', + action='manual_transcription', + params=[{'language': 'es'}], + asset=self.asset, ) if shared: @@ -172,10 +167,12 @@ def test_can_write(self, username, shared, status_code): '_dateModified': '2024-04-08T15:27:00Z', '_versions': [ { + '_data': { + 'language': 'es', + 'value': 'buenas noches', + }, '_dateCreated': '2024-04-08T15:27:00Z', '_dateAccepted': '2024-04-08T15:27:00Z', - 'language': 'es', - 'value': 'buenas noches', '_uuid': '11111111-2222-3333-4444-555555555555', } ], diff --git a/kobo/apps/subsequences/tests/api/v2/test_validation.py b/kobo/apps/subsequences/tests/api/v2/test_validation.py index f6a25eaa0a..21879802a4 100644 --- a/kobo/apps/subsequences/tests/api/v2/test_validation.py +++ b/kobo/apps/subsequences/tests/api/v2/test_validation.py @@ -2,7 +2,7 @@ from rest_framework import status -from kobo.apps.subsequences.models import SubmissionSupplement +from kobo.apps.subsequences.models import QuestionAdvancedAction, SubmissionSupplement from kobo.apps.subsequences.tests.api.v2.base import SubsequenceBaseTestCase from kobo.apps.subsequences.tests.constants import QUESTION_SUPPLEMENT @@ -25,21 +25,16 @@ def test_cannot_patch_if_action_is_invalid(self): self.supplement_details_url, data=payload, format='json' ) assert response.status_code == status.HTTP_400_BAD_REQUEST - assert 'Invalid action' in str(response.data) + assert 'Invalid question' in str(response.data) # Activate manual transcription (even if payload asks for translation) - self.set_asset_advanced_features( - { - '_version': '20250820', - '_actionConfigs': { - 'q1': { - 'manual_transcription': [ - {'language': 'es'}, - ] - } - }, - } + QuestionAdvancedAction.objects.create( + action='manual_transcription', + params=[{'language': 'es'}], + asset=self.asset, + question_xpath='q1', ) + response = self.client.patch( self.supplement_details_url, data=payload, format='json' ) @@ -47,17 +42,11 @@ def test_cannot_patch_if_action_is_invalid(self): assert 'Invalid action' in str(response.data) def test_cannot_patch_with_invalid_payload(self): - self.set_asset_advanced_features( - { - '_version': '20250820', - '_actionConfigs': { - 'q1': { - 'manual_transcription': [ - {'language': 'es'}, - ] - } - }, - } + QuestionAdvancedAction.objects.create( + action='manual_transcription', + params=[{'language': 'es'}], + asset=self.asset, + question_xpath='q1', ) payload = { @@ -79,20 +68,19 @@ def test_cannot_patch_with_invalid_payload(self): def test_cannot_set_value_with_automatic_actions(self): # First, set up the asset to allow automatic actions - advanced_features = { - '_version': '20250820', - '_actionConfigs': { - 'q1': { - 'automatic_google_transcription': [ - {'language': 'en'}, - ], - 'automatic_google_translation': [ - {'language': 'fr'}, - ] - } - }, - } - self.set_asset_advanced_features(advanced_features) + + QuestionAdvancedAction.objects.create( + action='automatic_google_transcription', + params=[{'language': 'en'}], + asset=self.asset, + question_xpath='q1', + ) + QuestionAdvancedAction.objects.create( + action='automatic_google_translation', + params=[{'language': 'fr'}], + asset=self.asset, + question_xpath='q1', + ) # Simulate a completed transcription, first. mock_submission_supplement = { @@ -105,7 +93,10 @@ def test_cannot_set_value_with_automatic_actions(self): content=mock_submission_supplement, asset=self.asset, ) - automatic_actions = advanced_features['_actionConfigs']['q1'].keys() + automatic_actions = [ + 'automatic_google_translation', + 'automatic_google_transcription', + ] for automatic_action in automatic_actions: payload = { '_version': '20250820', @@ -125,17 +116,11 @@ def test_cannot_set_value_with_automatic_actions(self): def test_cannot_accept_incomplete_automatic_transcription(self): # Set up the asset to allow automatic google transcription - self.set_asset_advanced_features( - { - '_version': '20250820', - '_actionConfigs': { - 'q1': { - 'automatic_google_transcription': [ - {'language': 'es'}, - ] - } - }, - } + QuestionAdvancedAction.objects.create( + action='automatic_google_transcription', + params=[{'language': 'en'}], + asset=self.asset, + question_xpath='q1', ) # Try to set 'accepted' status when translation is not complete @@ -165,20 +150,17 @@ def test_cannot_accept_incomplete_automatic_transcription(self): def test_cannot_accept_incomplete_automatic_translation(self): # Set up the asset to allow automatic google actions - self.set_asset_advanced_features( - { - '_version': '20250820', - '_actionConfigs': { - 'q1': { - 'automatic_google_transcription': [ - {'language': 'en'}, - ], - 'automatic_google_translation': [ - {'language': 'fr'}, - ] - } - }, - } + QuestionAdvancedAction.objects.create( + action='automatic_google_transcription', + params=[{'language': 'en'}], + asset=self.asset, + question_xpath='q1', + ) + QuestionAdvancedAction.objects.create( + action='automatic_google_translation', + params=[{'language': 'fr'}], + asset=self.asset, + question_xpath='q1', ) # Simulate a completed transcription, first. @@ -219,20 +201,17 @@ def test_cannot_accept_incomplete_automatic_translation(self): def test_cannot_request_translation_without_transcription(self): # Set up the asset to allow automatic google actions - self.set_asset_advanced_features( - { - '_version': '20250820', - '_actionConfigs': { - 'q1': { - 'automatic_google_transcription': [ - {'language': 'en'}, - ], - 'automatic_google_translation': [ - {'language': 'fr'}, - ] - } - }, - } + QuestionAdvancedAction.objects.create( + action='automatic_google_transcription', + params=[{'language': 'en'}], + asset=self.asset, + question_xpath='q1', + ) + QuestionAdvancedAction.objects.create( + action='automatic_google_translation', + params=[{'language': 'fr'}], + asset=self.asset, + question_xpath='q1', ) # Try to ask for translation diff --git a/kobo/apps/subsequences/tests/api/v2/test_views.py b/kobo/apps/subsequences/tests/api/v2/test_views.py index 26b0bd38d7..2f297b1bf6 100644 --- a/kobo/apps/subsequences/tests/api/v2/test_views.py +++ b/kobo/apps/subsequences/tests/api/v2/test_views.py @@ -43,7 +43,7 @@ def test_update_action(self): res = self.client.patch( self.action_detail_url, content_type='application/json', - data=json.dumps({'question_xpath': 'bad'}), + data=json.dumps({'params': [{'language': 'es'}]}), ) assert res.status_code == status.HTTP_200_OK self.action.refresh_from_db() diff --git a/kobo/apps/subsequences/tests/test_models.py b/kobo/apps/subsequences/tests/test_models.py index fc13c97f18..55f4a6bf35 100644 --- a/kobo/apps/subsequences/tests/test_models.py +++ b/kobo/apps/subsequences/tests/test_models.py @@ -1,5 +1,4 @@ import uuid -from copy import deepcopy from datetime import datetime from unittest.mock import patch from zoneinfo import ZoneInfo @@ -12,7 +11,7 @@ from kpi.models import Asset from ..constants import SUBMISSION_UUID_FIELD from ..exceptions import InvalidAction, InvalidXPath -from ..models import SubmissionSupplement +from ..models import QuestionAdvancedAction, SubmissionSupplement from .constants import EMPTY_SUPPLEMENT @@ -21,15 +20,6 @@ class SubmissionSupplementTestCase(TestCase): # Asset-level config. # - Allow manual transcription for Arabic # - Allow manual translation for English and Spanish - ADVANCED_FEATURES = { - '_version': '20250820', - '_actionConfigs': { - 'group_name/question_name': { - 'manual_transcription': [{'language': 'ar'}], - 'manual_translation': [{'language': 'en'}, {'language': 'es'}], - } - }, - } EXPECTED_SUBMISSION_SUPPLEMENT = { '_version': '20250820', @@ -124,7 +114,18 @@ def setUp(self): self.asset = Asset.objects.create( owner=self.owner, name='Test Asset', - advanced_features=self.ADVANCED_FEATURES, + ) + QuestionAdvancedAction.objects.create( + asset=self.asset, + question_xpath='group_name/question_name', + action='manual_transcription', + params=[{'language': 'ar'}], + ) + QuestionAdvancedAction.objects.create( + asset=self.asset, + question_xpath='group_name/question_name', + action='manual_translation', + params=[{'language': 'en'}, {'language': 'es'}], ) # Mock submission with minimal info needed for subsequence actions @@ -146,20 +147,6 @@ def test_retrieve_data_with_invalid_arguments(self): self.asset, submission_root_uuid=None, prefetched_supplement=None ) - def test_retrieve_data_with_stale_questions(self): - SubmissionSupplement.objects.create( - asset=self.asset, - submission_uuid=self.submission_root_uuid, - content=self.EXPECTED_SUBMISSION_SUPPLEMENT, - ) - advanced_features = deepcopy(self.ADVANCED_FEATURES) - config = advanced_features['_actionConfigs'].pop('group_name/question_name') - advanced_features['_actionConfigs']['group_name/renamed_question_name'] = config - submission_supplement = SubmissionSupplement.retrieve_data( - self.asset, self.submission_root_uuid - ) - assert submission_supplement == EMPTY_SUPPLEMENT - def test_retrieve_data_from_migrated_data(self): submission_supplement = { 'group_name/question_name': { diff --git a/kobo/apps/subsequences/utils/supplement_data.py b/kobo/apps/subsequences/utils/supplement_data.py index 0e6688ac42..be53dc1f05 100644 --- a/kobo/apps/subsequences/utils/supplement_data.py +++ b/kobo/apps/subsequences/utils/supplement_data.py @@ -2,8 +2,13 @@ from kobo.apps.openrosa.apps.logger.xform_instance_parser import remove_uuid_prefix from kobo.apps.subsequences.constants import SUBMISSION_UUID_FIELD, SUPPLEMENT_KEY -from kobo.apps.subsequences.models import SubmissionSupplement -from kobo.apps.subsequences.utils.action_conversion import question_advanced_action_to_action +from kobo.apps.subsequences.models import ( + SubmissionSupplement, + migrate_advanced_features, +) +from kobo.apps.subsequences.utils.action_conversion import ( + question_advanced_action_to_action, +) def get_supplemental_output_fields(asset: 'kpi.models.Asset') -> list[dict]: @@ -35,6 +40,8 @@ def get_supplemental_output_fields(asset: 'kpi.models.Asset') -> list[dict]: submission. We'll do that by looking at the acceptance dates and letting the most recent win """ + if asset.advanced_features != {}: + migrate_advanced_features(asset) advanced_features = asset.advanced_features_set.all() output_fields_by_name = {} diff --git a/kobo/apps/subsequences/utils/versioning.py b/kobo/apps/subsequences/utils/versioning.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/kobo/apps/subsequences/views.py b/kobo/apps/subsequences/views.py index 16589c470f..5555df757b 100644 --- a/kobo/apps/subsequences/views.py +++ b/kobo/apps/subsequences/views.py @@ -1,4 +1,4 @@ -from rest_framework import mixins, viewsets +from rest_framework import mixins from rest_framework_extensions.mixins import NestedViewSetMixin from kobo.apps.audit_log.base_views import AuditLoggedViewSet @@ -26,7 +26,6 @@ class QuestionAdvancedActionViewSet( def get_queryset(self): return QuestionAdvancedAction.objects.filter(asset=self.asset) def perform_create_override(self, serializer): - serializer.save(asset=self.asset) def get_serializer_class(self): if self.action in ['update', 'partial_update']: From 5fd334a01543f6ddf9e50b225f20720685d80b05 Mon Sep 17 00:00:00 2001 From: rgraber Date: Tue, 18 Nov 2025 16:09:12 +0100 Subject: [PATCH 09/20] fixup!: api docs --- .../docs/api/v2/subsequences/create.md | 74 +++++++++++++++++ .../docs/api/v2/subsequences/list.md | 3 + .../docs/api/v2/subsequences/retrieve.md | 3 + .../docs/api/v2/subsequences/update.md | 67 ++++++++++++++++ .../schema_extensions/__init__.py | 0 .../schema_extensions/v2/__init__.py | 0 .../v2/subsequences/extensions.py | 23 ++++++ .../v2/subsequences/fields.py | 5 ++ .../v2/subsequences/serializers.py | 14 ++++ kobo/apps/subsequences/views.py | 79 ++++++++++++++++++- 10 files changed, 267 insertions(+), 1 deletion(-) create mode 100644 kobo/apps/subsequences/docs/api/v2/subsequences/create.md create mode 100644 kobo/apps/subsequences/docs/api/v2/subsequences/list.md create mode 100644 kobo/apps/subsequences/docs/api/v2/subsequences/retrieve.md create mode 100644 kobo/apps/subsequences/docs/api/v2/subsequences/update.md create mode 100644 kobo/apps/subsequences/schema_extensions/__init__.py create mode 100644 kobo/apps/subsequences/schema_extensions/v2/__init__.py create mode 100644 kobo/apps/subsequences/schema_extensions/v2/subsequences/extensions.py create mode 100644 kobo/apps/subsequences/schema_extensions/v2/subsequences/fields.py create mode 100644 kobo/apps/subsequences/schema_extensions/v2/subsequences/serializers.py diff --git a/kobo/apps/subsequences/docs/api/v2/subsequences/create.md b/kobo/apps/subsequences/docs/api/v2/subsequences/create.md new file mode 100644 index 0000000000..bf56302976 --- /dev/null +++ b/kobo/apps/subsequences/docs/api/v2/subsequences/create.md @@ -0,0 +1,74 @@ +## Add an advanced action to an asset + +Enables a new type of advanced action on a question in the asset. +* `action`, `params`, and `question_xpath` are required +* `params` must match the expected param_schema of the `action` + +Accepted `action`s include: +* `manual_transcription` +* `automatic_google_transcription` +* `manual_translation` +* `automatic_google_translation` +* `qual` + +For all actions except `qual`, `params` must look like +> '[{"language": "es"}, {"language": "en"}, ...]' + +For `qual`, `params` must look like +``` + [ + { + 'type': 'qualInteger', + 'uuid': '1a2c8eb0-e2ec-4b3c-942a-c1a5410c081a', + 'labels': {'_default': 'How many characters appear in the story?'}, + }, + { + 'type': 'qualSelectMultiple', + 'uuid': '2e30bec7-4843-43c7-98bc-13114af230c5', + 'labels': {'_default': "What themes were present in the story?"}, + 'choices': [ + { + 'uuid': '2e24e6b4-bc3b-4e8e-b0cd-d8d3b9ca15b6', + 'labels': {'_default': 'Empathy'}, + }, + { + 'uuid': 'cb82919d-2948-4ccf-a488-359c5d5ee53a', + 'labels': {'_default': 'Competition'}, + }, + { + 'uuid': '8effe3b1-619e-4ada-be45-ebcea5af0aaf', + 'labels': {'_default': 'Apathy'}, + }, + ], + }, + { + 'type': 'qualSelectOne', + 'uuid': '1a8b748b-f470-4c40-bc09-ce2b1197f503', + 'labels': {'_default': 'Was this a first-hand account?'}, + 'choices': [ + { + 'uuid': '3c7aacdc-8971-482a-9528-68e64730fc99', + 'labels': {'_default': 'Yes'}, + }, + { + 'uuid': '7e31c6a5-5eac-464c-970c-62c383546a94', + 'labels': {'_default': 'No'}, + }, + ], + }, + { + 'type': 'qualTags', + 'uuid': 'e9b4e6d1-fdbb-4dc9-8b10-a9c3c388322f', + 'labels': {'_default': 'Tag any landmarks mentioned in the story'}, + }, + { + 'type': 'qualText', + 'uuid': '83acf2a7-8edc-4fd8-8b9f-f832ca3f18ad', + 'labels': {'_default': 'Add any further remarks'}, + }, + { + 'type': 'qualNote', + 'uuid': '5ef11d48-d7a3-432e-af83-8c2e9b1feb72', + 'labels': {'_default': 'Thanks for your diligence'}, + }, + ]``` diff --git a/kobo/apps/subsequences/docs/api/v2/subsequences/list.md b/kobo/apps/subsequences/docs/api/v2/subsequences/list.md new file mode 100644 index 0000000000..ddebff474a --- /dev/null +++ b/kobo/apps/subsequences/docs/api/v2/subsequences/list.md @@ -0,0 +1,3 @@ +## List all advanced features on an asset + +Lists all advanced features on all questions in an asset diff --git a/kobo/apps/subsequences/docs/api/v2/subsequences/retrieve.md b/kobo/apps/subsequences/docs/api/v2/subsequences/retrieve.md new file mode 100644 index 0000000000..369db0251c --- /dev/null +++ b/kobo/apps/subsequences/docs/api/v2/subsequences/retrieve.md @@ -0,0 +1,3 @@ +## Retrieve advanced feature configuration for a question on an asset + +Gets the params for one advanced action for one question in an asset diff --git a/kobo/apps/subsequences/docs/api/v2/subsequences/update.md b/kobo/apps/subsequences/docs/api/v2/subsequences/update.md new file mode 100644 index 0000000000..a514e4ce21 --- /dev/null +++ b/kobo/apps/subsequences/docs/api/v2/subsequences/update.md @@ -0,0 +1,67 @@ +## Update an advanced action on an asset + +Update the params of an advanced action on a question in the asset. +* `params` is required +* `params` must match the expected param_schema of the action being updated + +For all actions except `qual`, `params` must look like +> '[{"language": "es"}, {"language": "en"}, ...]' + +For `qual`, `params` must look like +``` + [ + { + 'type': 'qualInteger', + 'uuid': '1a2c8eb0-e2ec-4b3c-942a-c1a5410c081a', + 'labels': {'_default': 'How many characters appear in the story?'}, + }, + { + 'type': 'qualSelectMultiple', + 'uuid': '2e30bec7-4843-43c7-98bc-13114af230c5', + 'labels': {'_default': "What themes were present in the story?"}, + 'choices': [ + { + 'uuid': '2e24e6b4-bc3b-4e8e-b0cd-d8d3b9ca15b6', + 'labels': {'_default': 'Empathy'}, + }, + { + 'uuid': 'cb82919d-2948-4ccf-a488-359c5d5ee53a', + 'labels': {'_default': 'Competition'}, + }, + { + 'uuid': '8effe3b1-619e-4ada-be45-ebcea5af0aaf', + 'labels': {'_default': 'Apathy'}, + }, + ], + }, + { + 'type': 'qualSelectOne', + 'uuid': '1a8b748b-f470-4c40-bc09-ce2b1197f503', + 'labels': {'_default': 'Was this a first-hand account?'}, + 'choices': [ + { + 'uuid': '3c7aacdc-8971-482a-9528-68e64730fc99', + 'labels': {'_default': 'Yes'}, + }, + { + 'uuid': '7e31c6a5-5eac-464c-970c-62c383546a94', + 'labels': {'_default': 'No'}, + }, + ], + }, + { + 'type': 'qualTags', + 'uuid': 'e9b4e6d1-fdbb-4dc9-8b10-a9c3c388322f', + 'labels': {'_default': 'Tag any landmarks mentioned in the story'}, + }, + { + 'type': 'qualText', + 'uuid': '83acf2a7-8edc-4fd8-8b9f-f832ca3f18ad', + 'labels': {'_default': 'Add any further remarks'}, + }, + { + 'type': 'qualNote', + 'uuid': '5ef11d48-d7a3-432e-af83-8c2e9b1feb72', + 'labels': {'_default': 'Thanks for your diligence'}, + }, + ]``` diff --git a/kobo/apps/subsequences/schema_extensions/__init__.py b/kobo/apps/subsequences/schema_extensions/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/kobo/apps/subsequences/schema_extensions/v2/__init__.py b/kobo/apps/subsequences/schema_extensions/v2/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/kobo/apps/subsequences/schema_extensions/v2/subsequences/extensions.py b/kobo/apps/subsequences/schema_extensions/v2/subsequences/extensions.py new file mode 100644 index 0000000000..8a75ef77f1 --- /dev/null +++ b/kobo/apps/subsequences/schema_extensions/v2/subsequences/extensions.py @@ -0,0 +1,23 @@ +# This drf-extension made for the metadata field of AccessLog targets the external class +# and tells it what it should return when generating the schema. +from drf_spectacular.extensions import OpenApiSerializerFieldExtension +from drf_spectacular.plumbing import build_object_type + +from kpi.schema_extensions.v2.generic.schema import ( + GENERIC_STRING_SCHEMA, + USER_URL_SCHEMA, GENERIC_ARRAY_SCHEMA, +) +class SubsequenceParamsFieldExtension(OpenApiSerializerFieldExtension): + target_class = 'kobo.apps.subsequences.schema_extensions.v2.subsequences.fields.AdvancedFeatureParamsField' # noqa + + def map_serializer_field(self, auto_schema, direction): + return build_object_type( + properties={ + 'source': GENERIC_STRING_SCHEMA, + 'auth_type': GENERIC_STRING_SCHEMA, + 'ip_address': GENERIC_STRING_SCHEMA, + 'initial_user_uid': GENERIC_STRING_SCHEMA, + 'initial_user_username': GENERIC_STRING_SCHEMA, + 'authorized_app_name': GENERIC_STRING_SCHEMA, + } + ) diff --git a/kobo/apps/subsequences/schema_extensions/v2/subsequences/fields.py b/kobo/apps/subsequences/schema_extensions/v2/subsequences/fields.py new file mode 100644 index 0000000000..fbcbd0efe0 --- /dev/null +++ b/kobo/apps/subsequences/schema_extensions/v2/subsequences/fields.py @@ -0,0 +1,5 @@ +from rest_framework import serializers + + +class AdvancedFeatureParamsField(serializers.JSONField): + pass diff --git a/kobo/apps/subsequences/schema_extensions/v2/subsequences/serializers.py b/kobo/apps/subsequences/schema_extensions/v2/subsequences/serializers.py new file mode 100644 index 0000000000..1bbb167281 --- /dev/null +++ b/kobo/apps/subsequences/schema_extensions/v2/subsequences/serializers.py @@ -0,0 +1,14 @@ +from rest_framework import serializers + +from kobo.apps.subsequences.schema_extensions.v2.subsequences.fields import AdvancedFeatureParamsField +from kpi.utils.schema_extensions.serializers import inline_serializer_class + +AdvancedFeatureResponse = inline_serializer_class( + name='AdvancedFeatureResponse', + fields={ + 'question_xpath': serializers.CharField(), + 'action': serializers.CharField(), + 'params': AdvancedFeatureParamsField, + 'asset': serializers.CharField(), + }, +) diff --git a/kobo/apps/subsequences/views.py b/kobo/apps/subsequences/views.py index 5555df757b..8b30ca991d 100644 --- a/kobo/apps/subsequences/views.py +++ b/kobo/apps/subsequences/views.py @@ -1,16 +1,93 @@ +from drf_spectacular.utils import OpenApiParameter, extend_schema, extend_schema_view + from rest_framework import mixins from rest_framework_extensions.mixins import NestedViewSetMixin from kobo.apps.audit_log.base_views import AuditLoggedViewSet from kobo.apps.audit_log.models import AuditType from kobo.apps.subsequences.models import QuestionAdvancedAction +from kobo.apps.subsequences.schema_extensions.v2.subsequences.serializers import AdvancedFeatureResponse from kobo.apps.subsequences.serializers import ( QuestionAdvancedActionSerializer, QuestionAdvancedActionUpdateSerializer, ) from kpi.utils.viewset_mixins import AssetNestedObjectViewsetMixin +from kpi.utils.schema_extensions.markdown import read_md +from kpi.utils.schema_extensions.response import ( + open_api_200_ok_response, + open_api_201_created_response, + open_api_204_empty_response, +) - +@extend_schema( + tags=['Advanced Features'], + parameters=[ + OpenApiParameter( + name='parent_lookup_asset', + type=str, + location=OpenApiParameter.PATH, + required=True, + description='UID of the parent assets', + ), + ], +) +@extend_schema_view( + create=extend_schema( + description=read_md('subsequences', 'subsequences/create.md'), + responses=open_api_201_created_response( + AdvancedFeatureResponse, + require_auth=False, + raise_access_forbidden=False, + ), + ), + list=extend_schema( + description=read_md('subsequences', 'subsequences/list.md'), + responses=open_api_200_ok_response( + AdvancedFeatureResponse, + require_auth=False, + raise_access_forbidden=False, + validate_payload=False, + ), + ), + partial_update=extend_schema( + description=read_md('subsequences', 'subsequences/update.md'), + responses=open_api_200_ok_response( + QuestionAdvancedActionUpdateSerializer, + require_auth=False, + raise_access_forbidden=False, + ), + parameters=[ + OpenApiParameter( + name='uid', + type=str, + location=OpenApiParameter.PATH, + required=True, + description='UID of the action', + ), + ], + ), + retrieve=extend_schema( + description=read_md('subsequences', 'subsequences/retrieve.md'), + responses=open_api_200_ok_response( + QuestionAdvancedActionSerializer, + require_auth=False, + raise_access_forbidden=False, + validate_payload=False, + ), + parameters=[ + OpenApiParameter( + name='uid', + type=str, + location=OpenApiParameter.PATH, + required=True, + description='UID of the action', + ), + ], + ), + update=extend_schema( + exclude=True, + ), +) class QuestionAdvancedActionViewSet( AuditLoggedViewSet, mixins.CreateModelMixin, From 38057640fb5fc3c5e404aa2e2648b9322e96ed52 Mon Sep 17 00:00:00 2001 From: rgraber Date: Tue, 18 Nov 2025 16:10:56 +0100 Subject: [PATCH 10/20] fixup!: api docs --- .../schema_extensions/v2/access_logs/extensions.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/kobo/apps/audit_log/schema_extensions/v2/access_logs/extensions.py b/kobo/apps/audit_log/schema_extensions/v2/access_logs/extensions.py index 9d788563de..2679fc3404 100644 --- a/kobo/apps/audit_log/schema_extensions/v2/access_logs/extensions.py +++ b/kobo/apps/audit_log/schema_extensions/v2/access_logs/extensions.py @@ -15,12 +15,7 @@ class AccessLogMetadataFieldExtension(OpenApiSerializerFieldExtension): def map_serializer_field(self, auto_schema, direction): return build_object_type( properties={ - 'source': GENERIC_STRING_SCHEMA, - 'auth_type': GENERIC_STRING_SCHEMA, - 'ip_address': GENERIC_STRING_SCHEMA, - 'initial_user_uid': GENERIC_STRING_SCHEMA, - 'initial_user_username': GENERIC_STRING_SCHEMA, - 'authorized_app_name': GENERIC_STRING_SCHEMA, + 'test': GENERIC_STRING_SCHEMA, } ) From 225b79565b7ac6ae97570cf62d46cd76447a13f5 Mon Sep 17 00:00:00 2001 From: rgraber Date: Tue, 18 Nov 2025 17:28:45 +0100 Subject: [PATCH 11/20] fixup!: rm old file --- .../v2/subsequences/serializers.py | 2 +- kobo/apps/subsequences/views.py | 4 +- kpi/views/v2/asset_advanced_features.py | 43 ------------------- 3 files changed, 4 insertions(+), 45 deletions(-) delete mode 100644 kpi/views/v2/asset_advanced_features.py diff --git a/kobo/apps/subsequences/schema_extensions/v2/subsequences/serializers.py b/kobo/apps/subsequences/schema_extensions/v2/subsequences/serializers.py index 1bbb167281..c614eb9184 100644 --- a/kobo/apps/subsequences/schema_extensions/v2/subsequences/serializers.py +++ b/kobo/apps/subsequences/schema_extensions/v2/subsequences/serializers.py @@ -8,7 +8,7 @@ fields={ 'question_xpath': serializers.CharField(), 'action': serializers.CharField(), - 'params': AdvancedFeatureParamsField, + 'params': AdvancedFeatureParamsField(), 'asset': serializers.CharField(), }, ) diff --git a/kobo/apps/subsequences/views.py b/kobo/apps/subsequences/views.py index 8b30ca991d..135ab2acc3 100644 --- a/kobo/apps/subsequences/views.py +++ b/kobo/apps/subsequences/views.py @@ -11,6 +11,7 @@ QuestionAdvancedActionSerializer, QuestionAdvancedActionUpdateSerializer, ) +from kpi.permissions import AssetNestedObjectPermission, AssetPermission from kpi.utils.viewset_mixins import AssetNestedObjectViewsetMixin from kpi.utils.schema_extensions.markdown import read_md from kpi.utils.schema_extensions.response import ( @@ -69,7 +70,7 @@ retrieve=extend_schema( description=read_md('subsequences', 'subsequences/retrieve.md'), responses=open_api_200_ok_response( - QuestionAdvancedActionSerializer, + AdvancedFeatureResponse, require_auth=False, raise_access_forbidden=False, validate_payload=False, @@ -100,6 +101,7 @@ class QuestionAdvancedActionViewSet( log_type = AuditType.PROJECT_HISTORY logged_fields = ['asset.owner.username', 'action', 'params',('object_id', 'asset.id'),] pagination_class = None + permission_classes = (AssetPermission,) def get_queryset(self): return QuestionAdvancedAction.objects.filter(asset=self.asset) def perform_create_override(self, serializer): diff --git a/kpi/views/v2/asset_advanced_features.py b/kpi/views/v2/asset_advanced_features.py deleted file mode 100644 index a207f64cab..0000000000 --- a/kpi/views/v2/asset_advanced_features.py +++ /dev/null @@ -1,43 +0,0 @@ -from rest_framework import serializers -from rest_framework.fields import empty -from rest_framework.mixins import UpdateModelMixin -from rest_framework_extensions.mixins import NestedViewSetMixin - -from kobo.apps.audit_log.base_views import AuditLoggedModelViewSet -from kobo.apps.audit_log.models import AuditType -from kobo.apps.subsequences.models import QuestionAdvancedAction -from kpi.mixins.asset import AssetViewSetListMixin -from kpi.mixins.object_permission import ObjectPermissionViewSetMixin -from kpi.utils.viewset_mixins import AssetNestedObjectViewsetMixin - - -class AssetAdvancedFeaturesViewSet( - AssetViewSetListMixin, - ObjectPermissionViewSetMixin, - AssetNestedObjectViewsetMixin, - NestedViewSetMixin, - AuditLoggedModelViewSet, - UpdateModelMixin, -): - logged_fields = [ - - ] - log_type = AuditType.PROJECT_HISTORY - - def list(self): - pass - def partial_update(self, request, *args, **kwargs): - - pass - pass - -class AdvancedFeaturesSerializer(serializers.Serializer): - def __init__(self, instance=None, data=empty, **kwargs): - super().__init__(instance=instance, data=data, **kwargs) - self.asset = kwargs.get('context').get('asset') - - def validate(self, data): - pass - - def create(self, validated_data): - self.asset.advanced_features = validated_data From 510635166f2d5e4ab5f07ae85895d4fa443f6633 Mon Sep 17 00:00:00 2001 From: rgraber Date: Wed, 19 Nov 2025 11:29:11 +0100 Subject: [PATCH 12/20] fixup!: permissions and test --- .../tests/api/v2/test_permissions.py | 152 +++++++++++++++++- kobo/apps/subsequences/views.py | 4 +- kpi/permissions.py | 13 ++ 3 files changed, 166 insertions(+), 3 deletions(-) diff --git a/kobo/apps/subsequences/tests/api/v2/test_permissions.py b/kobo/apps/subsequences/tests/api/v2/test_permissions.py index b035123ce0..4b41d84657 100644 --- a/kobo/apps/subsequences/tests/api/v2/test_permissions.py +++ b/kobo/apps/subsequences/tests/api/v2/test_permissions.py @@ -4,6 +4,7 @@ from zoneinfo import ZoneInfo from ddt import data, ddt, unpack +from django.urls import reverse from freezegun import freeze_time from rest_framework import status @@ -13,7 +14,7 @@ from kpi.constants import ( PERM_CHANGE_SUBMISSIONS, PERM_PARTIAL_SUBMISSIONS, - PERM_VIEW_SUBMISSIONS, + PERM_VIEW_SUBMISSIONS, PERM_VIEW_ASSET, PERM_CHANGE_METADATA_ASSET, PERM_MANAGE_ASSET, ) from kpi.utils.object_permission import get_anonymous_user @@ -223,3 +224,152 @@ def test_cannot_read_data(self): self.client.force_login(anotheruser) response = self.client.get(self.supplement_details_url) assert response.status_code == status.HTTP_404_NOT_FOUND + +@ddt +class AdvancedFeaturesPermissionTestCase(SubsequenceBaseTestCase): + + + def setUp(self): + super().setUp() + self.advanced_features_url = reverse('api_v2:advanced-features-list', args=(self.asset.uid,)) + with patch('kobo.apps.subsequences.models.KpiUidField.generate_uid', return_value='12345'): + QuestionAdvancedAction.objects.create( + asset=self.asset, + action='manual_transcription', + question_xpath='q1', + params=[{'language': 'en'}] + ) + + @data( + # owner: Obviously, no need to share. + ( + 'someuser', + False, + status.HTTP_200_OK, + ), + # regular user with no permissions + ( + 'anotheruser', + False, + status.HTTP_404_NOT_FOUND, + ), + # regular user with view permission + ( + 'anotheruser', + True, + status.HTTP_200_OK, + ), + # admin user with no permissions + ( + 'adminuser', + False, + status.HTTP_200_OK, + ), + # admin user with view permissions + ( + 'adminuser', + True, + status.HTTP_200_OK, + ), + # anonymous user with no permissions + ( + 'anonymous', + False, + status.HTTP_404_NOT_FOUND, + ), + # anonymous user with view permissions + ( + 'anonymous', + True, + status.HTTP_200_OK, + ), + ) + @unpack + def test_can_read(self, username, shared, status_code): + user = get_anonymous_user() + self.client.logout() + if username != 'anonymous': + user = User.objects.get(username=username) + self.client.force_login(user) + + if shared: + self.asset.assign_perm(user, PERM_VIEW_ASSET) + + response = self.client.get(self.advanced_features_url) + assert response.status_code == status_code + if status_code == status.HTTP_200_OK: + assert response.data == [{ + 'question_xpath': 'q1', + 'uid': '12345', + 'params': [{'language': 'en'}], + 'action': 'manual_transcription', + }] + + @data( + # owner: Obviously, no need to share. + ( + 'someuser', + False, + status.HTTP_201_CREATED, + ), + # regular user with no permissions + ( + 'anotheruser', + False, + status.HTTP_404_NOT_FOUND, + ), + # regular user with view permission + ( + 'anotheruser', + True, + status.HTTP_201_CREATED, + ), + # admin user with no permissions + ( + 'adminuser', + False, + status.HTTP_201_CREATED, + ), + # admin user with view permissions + ( + 'adminuser', + True, + status.HTTP_201_CREATED, + ), + # anonymous user with no permissions + ( + 'anonymous', + False, + status.HTTP_404_NOT_FOUND, + ), + ) + @unpack + def test_can_write(self, username, shared, status_code): + payload = { + 'action': 'manual_translation', + 'question_xpath': 'q1', + 'params': [{'language':'es'}] + } + + user = get_anonymous_user() + self.client.logout() + if username != 'anonymous': + user = User.objects.get(username=username) + self.client.force_login(user) + + if shared: + self.asset.assign_perm(user, PERM_MANAGE_ASSET) + + frozen_datetime_now = datetime(2024, 4, 8, 15, 27, 0, tzinfo=ZoneInfo('UTC')) + with freeze_time(frozen_datetime_now): + response = self.client.post( + self.advanced_features_url, data=payload, format='json' + ) + + assert response.status_code == status_code + if response.status_code == status.HTTP_201_CREATED: + assert QuestionAdvancedAction.objects.filter( + asset=self.asset, + action='manual_translation' + ).exists() + diff --git a/kobo/apps/subsequences/views.py b/kobo/apps/subsequences/views.py index 135ab2acc3..14fe747b77 100644 --- a/kobo/apps/subsequences/views.py +++ b/kobo/apps/subsequences/views.py @@ -11,7 +11,7 @@ QuestionAdvancedActionSerializer, QuestionAdvancedActionUpdateSerializer, ) -from kpi.permissions import AssetNestedObjectPermission, AssetPermission +from kpi.permissions import AssetNestedObjectPermission, AssetPermission, AssetAdvancedFeaturesPermission from kpi.utils.viewset_mixins import AssetNestedObjectViewsetMixin from kpi.utils.schema_extensions.markdown import read_md from kpi.utils.schema_extensions.response import ( @@ -101,7 +101,7 @@ class QuestionAdvancedActionViewSet( log_type = AuditType.PROJECT_HISTORY logged_fields = ['asset.owner.username', 'action', 'params',('object_id', 'asset.id'),] pagination_class = None - permission_classes = (AssetPermission,) + permission_classes = (AssetAdvancedFeaturesPermission,) def get_queryset(self): return QuestionAdvancedAction.objects.filter(asset=self.asset) def perform_create_override(self, serializer): diff --git a/kpi/permissions.py b/kpi/permissions.py index f326c141e3..db6a2d700f 100644 --- a/kpi/permissions.py +++ b/kpi/permissions.py @@ -222,6 +222,19 @@ def has_permission(self, request, view): # permission to view it raise Http404 +class AssetAdvancedFeaturesPermission(AssetNestedObjectPermission): + """ + Owner, managers and editors can write. + i.e.: + - Reads need 'view_asset' permission + - Writes need 'change_asset' permission + """ + + perms_map = deepcopy(AssetNestedObjectPermission.perms_map) + perms_map['POST'] = ['%(app_label)s.change_metadata_asset'] + perms_map['PUT'] = perms_map['POST'] + perms_map['PATCH'] = perms_map['POST'] + perms_map['DELETE'] = perms_map['POST'] class AssetEditorPermission(AssetNestedObjectPermission): """ From 78fe2c3d159b63d43f5f57844d13f7514cf3d7bd Mon Sep 17 00:00:00 2001 From: rgraber Date: Wed, 19 Nov 2025 12:42:23 +0100 Subject: [PATCH 13/20] fixup!: use correct permission --- kobo/apps/subsequences/tests/api/v2/test_permissions.py | 2 +- kpi/permissions.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/kobo/apps/subsequences/tests/api/v2/test_permissions.py b/kobo/apps/subsequences/tests/api/v2/test_permissions.py index 4b41d84657..52fab2a8ff 100644 --- a/kobo/apps/subsequences/tests/api/v2/test_permissions.py +++ b/kobo/apps/subsequences/tests/api/v2/test_permissions.py @@ -358,7 +358,7 @@ def test_can_write(self, username, shared, status_code): self.client.force_login(user) if shared: - self.asset.assign_perm(user, PERM_MANAGE_ASSET) + self.asset.assign_perm(user, PERM_CHANGE_SUBMISSIONS) frozen_datetime_now = datetime(2024, 4, 8, 15, 27, 0, tzinfo=ZoneInfo('UTC')) with freeze_time(frozen_datetime_now): diff --git a/kpi/permissions.py b/kpi/permissions.py index db6a2d700f..906020a889 100644 --- a/kpi/permissions.py +++ b/kpi/permissions.py @@ -227,11 +227,11 @@ class AssetAdvancedFeaturesPermission(AssetNestedObjectPermission): Owner, managers and editors can write. i.e.: - Reads need 'view_asset' permission - - Writes need 'change_asset' permission + - Writes need 'change_submissions' permission """ perms_map = deepcopy(AssetNestedObjectPermission.perms_map) - perms_map['POST'] = ['%(app_label)s.change_metadata_asset'] + perms_map['POST'] = ['%(app_label)s.change_submissions'] perms_map['PUT'] = perms_map['POST'] perms_map['PATCH'] = perms_map['POST'] perms_map['DELETE'] = perms_map['POST'] From b72bbc9af096757b93b28e7f17e61b14cb74cd54 Mon Sep 17 00:00:00 2001 From: rgraber Date: Wed, 19 Nov 2025 12:52:03 +0100 Subject: [PATCH 14/20] fixup!: migration cleanup --- ...issionsupplement_questionadvancedaction.py | 19 ++------ ...me_config_questionadvancedaction_params.py | 18 ------- ...ter_accesslogexporttask_status_and_more.py | 47 ------------------- 3 files changed, 3 insertions(+), 81 deletions(-) delete mode 100644 kobo/apps/subsequences/migrations/0006_rename_config_questionadvancedaction_params.py delete mode 100644 kpi/migrations/0070_alter_asset_options_alter_accesslogexporttask_status_and_more.py diff --git a/kobo/apps/subsequences/migrations/0005_submissionsupplement_questionadvancedaction.py b/kobo/apps/subsequences/migrations/0005_submissionsupplement_questionadvancedaction.py index de5efc608b..b47321c0bb 100644 --- a/kobo/apps/subsequences/migrations/0005_submissionsupplement_questionadvancedaction.py +++ b/kobo/apps/subsequences/migrations/0005_submissionsupplement_questionadvancedaction.py @@ -1,8 +1,7 @@ -# Generated by Django 4.2.24 on 2025-11-11 18:01 +# Generated by Django 4.2.24 on 2025-11-19 11:45 -import django.db.models.deletion from django.db import migrations, models - +import django.db.models.deletion import kpi.fields.kpi_uid import kpi.fields.lazy_default_jsonb @@ -10,22 +9,10 @@ class Migration(migrations.Migration): dependencies = [ - ('kpi', '0070_alter_asset_options_alter_accesslogexporttask_status_and_more'), ('subsequences', '0004_increase_subsequences_submission_uuid'), ] operations = [ - migrations.CreateModel( - name='SubmissionSupplement', - fields=[], - options={ - 'abstract': False, - 'proxy': True, - 'indexes': [], - 'constraints': [], - }, - bases=('subsequences.submissionextras',), - ), migrations.CreateModel( name='QuestionAdvancedAction', fields=[ @@ -57,7 +44,7 @@ class Migration(migrations.Migration): ), ('question_xpath', models.CharField(max_length=2000)), ( - 'config', + 'params', kpi.fields.lazy_default_jsonb.LazyDefaultJSONBField(default=dict), ), ( diff --git a/kobo/apps/subsequences/migrations/0006_rename_config_questionadvancedaction_params.py b/kobo/apps/subsequences/migrations/0006_rename_config_questionadvancedaction_params.py deleted file mode 100644 index d3297d1b90..0000000000 --- a/kobo/apps/subsequences/migrations/0006_rename_config_questionadvancedaction_params.py +++ /dev/null @@ -1,18 +0,0 @@ -# Generated by Django 4.2.24 on 2025-11-14 14:46 - -from django.db import migrations - - -class Migration(migrations.Migration): - - dependencies = [ - ('subsequences', '0005_submissionsupplement_questionadvancedaction'), - ] - - operations = [ - migrations.RenameField( - model_name='questionadvancedaction', - old_name='config', - new_name='params', - ), - ] diff --git a/kpi/migrations/0070_alter_asset_options_alter_accesslogexporttask_status_and_more.py b/kpi/migrations/0070_alter_asset_options_alter_accesslogexporttask_status_and_more.py deleted file mode 100644 index 894dd46c44..0000000000 --- a/kpi/migrations/0070_alter_asset_options_alter_accesslogexporttask_status_and_more.py +++ /dev/null @@ -1,47 +0,0 @@ -# Generated by Django 4.2.24 on 2025-11-11 18:01 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('kpi', '0069_alter_assetversion_reversion_version'), - ] - - operations = [ - migrations.AlterModelOptions( - name='asset', - options={'default_permissions': ('add', 'change', 'delete'), 'ordering': ['-date_modified'], 'permissions': (('view_asset', 'Can view asset'), ('discover_asset', 'Can discover asset in public lists'), ('manage_asset', 'Can manage all aspects of asset'), ('add_submissions', 'Can submit data to asset'), ('view_submissions', 'Can view submitted data for asset'), ('partial_submissions', 'Can make partial actions on submitted data for asset for specific users'), ('change_submissions', 'Can modify submitted data for asset'), ('delete_submissions', 'Can delete submitted data for asset'), ('validate_submissions', 'Can validate submitted data asset'))}, - ), - migrations.AlterField( - model_name='accesslogexporttask', - name='status', - field=models.CharField(choices=[('created', 'created'), ('processing', 'processing'), ('complete', 'complete'), ('error', 'error')], default='created', max_length=32), - ), - migrations.AlterField( - model_name='importtask', - name='status', - field=models.CharField(choices=[('created', 'created'), ('processing', 'processing'), ('complete', 'complete'), ('error', 'error')], default='created', max_length=32), - ), - migrations.AlterField( - model_name='projecthistorylogexporttask', - name='status', - field=models.CharField(choices=[('created', 'created'), ('processing', 'processing'), ('complete', 'complete'), ('error', 'error')], default='created', max_length=32), - ), - migrations.AlterField( - model_name='projectviewexporttask', - name='status', - field=models.CharField(choices=[('created', 'created'), ('processing', 'processing'), ('complete', 'complete'), ('error', 'error')], default='created', max_length=32), - ), - migrations.AlterField( - model_name='submissionexporttask', - name='status', - field=models.CharField(choices=[('created', 'created'), ('processing', 'processing'), ('complete', 'complete'), ('error', 'error')], default='created', max_length=32), - ), - migrations.AlterField( - model_name='submissionsynchronousexport', - name='status', - field=models.CharField(choices=[('created', 'created'), ('processing', 'processing'), ('complete', 'complete'), ('error', 'error')], default='created', max_length=32), - ), - ] From c5a0e33a2ad46a6f824f9e2ee9705480b043af52 Mon Sep 17 00:00:00 2001 From: rgraber Date: Wed, 19 Nov 2025 12:57:10 +0100 Subject: [PATCH 15/20] fixup!: accidental change --- .../schema_extensions/v2/access_logs/extensions.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/kobo/apps/audit_log/schema_extensions/v2/access_logs/extensions.py b/kobo/apps/audit_log/schema_extensions/v2/access_logs/extensions.py index 2679fc3404..448238f315 100644 --- a/kobo/apps/audit_log/schema_extensions/v2/access_logs/extensions.py +++ b/kobo/apps/audit_log/schema_extensions/v2/access_logs/extensions.py @@ -15,8 +15,12 @@ class AccessLogMetadataFieldExtension(OpenApiSerializerFieldExtension): def map_serializer_field(self, auto_schema, direction): return build_object_type( properties={ - 'test': GENERIC_STRING_SCHEMA, - } + 'source': GENERIC_STRING_SCHEMA, + 'auth_type': GENERIC_STRING_SCHEMA, + 'ip_address': GENERIC_STRING_SCHEMA, + 'initial_user_uid': GENERIC_STRING_SCHEMA, + 'initial_user_username': GENERIC_STRING_SCHEMA, + 'authorized_app_name': GENERIC_STRING_SCHEMA, } ) From 792772793e189edaed3c3ec90b46c5e407ec2268 Mon Sep 17 00:00:00 2001 From: rgraber Date: Wed, 19 Nov 2025 13:05:37 +0100 Subject: [PATCH 16/20] fixup!: accidental change --- .../audit_log/schema_extensions/v2/access_logs/extensions.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kobo/apps/audit_log/schema_extensions/v2/access_logs/extensions.py b/kobo/apps/audit_log/schema_extensions/v2/access_logs/extensions.py index 448238f315..9d788563de 100644 --- a/kobo/apps/audit_log/schema_extensions/v2/access_logs/extensions.py +++ b/kobo/apps/audit_log/schema_extensions/v2/access_logs/extensions.py @@ -20,7 +20,8 @@ def map_serializer_field(self, auto_schema, direction): 'ip_address': GENERIC_STRING_SCHEMA, 'initial_user_uid': GENERIC_STRING_SCHEMA, 'initial_user_username': GENERIC_STRING_SCHEMA, - 'authorized_app_name': GENERIC_STRING_SCHEMA, } + 'authorized_app_name': GENERIC_STRING_SCHEMA, + } ) From 272b8f38c2899fd197b6d31dc6718410cd887229 Mon Sep 17 00:00:00 2001 From: rgraber Date: Wed, 19 Nov 2025 13:07:49 +0100 Subject: [PATCH 17/20] fixup!: comment --- kpi/models/asset.py | 1 + 1 file changed, 1 insertion(+) diff --git a/kpi/models/asset.py b/kpi/models/asset.py index 51c3c9ec79..7375061170 100644 --- a/kpi/models/asset.py +++ b/kpi/models/asset.py @@ -1145,6 +1145,7 @@ def update_languages(self, children=None): self.save(update_fields=['summary']) def validate_advanced_features(self): + # TODO: remove this once subsequences__old is deleted pass @property From 528fbfa02db4033b55919da5ab5e20f22cd1bb6c Mon Sep 17 00:00:00 2001 From: rgraber Date: Wed, 19 Nov 2025 13:34:22 +0100 Subject: [PATCH 18/20] fixup!: schema stuff --- kpi/schema_extensions/v2/assets/extensions.py | 9 ++++++--- kpi/schema_extensions/v2/assets/fields.py | 2 +- kpi/serializers/v2/asset.py | 15 +++++++++++---- kpi/utils/schema_extensions/url_builder.py | 2 ++ 4 files changed, 20 insertions(+), 8 deletions(-) diff --git a/kpi/schema_extensions/v2/assets/extensions.py b/kpi/schema_extensions/v2/assets/extensions.py index d2131cf76b..84da262786 100644 --- a/kpi/schema_extensions/v2/assets/extensions.py +++ b/kpi/schema_extensions/v2/assets/extensions.py @@ -38,11 +38,14 @@ def map_serializer_field(self, auto_schema, direction): return GENERIC_ARRAY_SCHEMA -class AdvancedFeatureFieldExtension(OpenApiSerializerFieldExtension): - target_class = 'kpi.schema_extensions.v2.assets.fields.AdvancedFeatureField' +class AdvancedFeaturesLinkFieldExtension(OpenApiSerializerFieldExtension): + target_class = 'kpi.schema_extensions.v2.assets.fields.AdvancedFeaturesLinkField' def map_serializer_field(self, auto_schema, direction): - return GENERIC_OBJECT_SCHEMA + return build_url_type( + 'api_v2:advanced-features-list', + parent_lookup_asset='aBeA23YCYjkGTFvYVHuAyU', + ) class AdvancedSubmissionSchemaFieldExtension(OpenApiSerializerFieldExtension): diff --git a/kpi/schema_extensions/v2/assets/fields.py b/kpi/schema_extensions/v2/assets/fields.py index 50f63b19ae..64c2c42d81 100644 --- a/kpi/schema_extensions/v2/assets/fields.py +++ b/kpi/schema_extensions/v2/assets/fields.py @@ -5,7 +5,7 @@ class AccessTypeField(serializers.ListField): pass -class AdvancedFeatureField(serializers.JSONField): +class AdvancedFeaturesLinkField(serializers.URLField): pass diff --git a/kpi/serializers/v2/asset.py b/kpi/serializers/v2/asset.py index 9ba0b8172e..c7f2f62246 100644 --- a/kpi/serializers/v2/asset.py +++ b/kpi/serializers/v2/asset.py @@ -62,7 +62,7 @@ ) from ...schema_extensions.v2.assets.fields import ( AccessTypeField, - AdvancedFeatureField, + AdvancedFeaturesLinkField, AdvancedSubmissionSchemaField, AnalysisFormJsonField, AssetHyperlinkedURLField, @@ -91,7 +91,7 @@ SummaryField, UserURLRelativeHyperlinkedRelatedField, XFormLinkField, - XLSLinkField, + XLSLinkField, AdvancedFeaturesLinkField, ) from .asset_export_settings import AssetExportSettingsSerializer from .asset_file import AssetFileSerializer @@ -400,7 +400,7 @@ class AssetSerializer(serializers.HyperlinkedModelSerializer): paired_data = serializers.SerializerMethodField() project_ownership = serializers.SerializerMethodField() kind = serializers.SerializerMethodField() - advanced_features = '' + advanced_features = serializers.SerializerMethodField() class Meta: model = Asset @@ -461,7 +461,8 @@ class Meta: 'paired_data', 'project_ownership', 'owner_label', - 'last_modified_by' + 'last_modified_by', + 'advanced_features', ) read_only_fields = ('last_modified_by', 'uid') extra_kwargs = { @@ -941,6 +942,12 @@ def get_owner_label(self, asset): return organization.name return asset.owner.username + @extend_schema_field(AdvancedFeaturesLinkField) + def get_advanced_features(self, obj): + return reverse('advanced-features-list', + args=(obj.uid,), + request=self.context.get('request', None)) + def validate_data_sharing(self, data_sharing: dict) -> dict: """ Validates `data_sharing`. It is really basic. diff --git a/kpi/utils/schema_extensions/url_builder.py b/kpi/utils/schema_extensions/url_builder.py index 89a03a5044..0069470cce 100644 --- a/kpi/utils/schema_extensions/url_builder.py +++ b/kpi/utils/schema_extensions/url_builder.py @@ -28,6 +28,8 @@ def build_url_type(viewname: str, **kwargs) -> dict: _, viewname = viewname.split(':') urls_pattern_mapping = { + 'advanced-features-list': '/api/v2/assets/{parent_lookup_asset}/advanced-features', + 'advanced-features-detail': '/api/v2/assets/{parent_lookup_asset}/advanced-features/{uid}', 'asset-detail': '/api/v2/assets/{uid}/', 'asset-permission-assignment-detail': '/api/v2/assets/{parent_lookup_asset}/permission-assignments/{uid}/', # noqa 'permission-detail': '/api/v2/permissions/{codename}/', From 0b3c9d5d2ab5906a8609aa1c2752062be7c5d88f Mon Sep 17 00:00:00 2001 From: rgraber Date: Wed, 19 Nov 2025 16:23:42 +0100 Subject: [PATCH 19/20] fixup!: stuff --- kobo/apps/subsequences/models.py | 1 - .../v2/subsequences/extensions.py | 20 ++++--------- .../subsequences/tests/api/v2/test_views.py | 30 +++++++++++++++++++ kobo/apps/subsequences/views.py | 18 +++++++---- kpi/schema_extensions/imports.py | 1 + 5 files changed, 48 insertions(+), 22 deletions(-) diff --git a/kobo/apps/subsequences/models.py b/kobo/apps/subsequences/models.py index c777a2c236..7c8552a9e2 100644 --- a/kobo/apps/subsequences/models.py +++ b/kobo/apps/subsequences/models.py @@ -236,7 +236,6 @@ def migrate_advanced_features(asset: 'kpi.models.Asset') -> dict | None: and value['languages'] ): for q in known_cols: - print(f'{q=}') QuestionAdvancedAction.objects.create( question_xpath=q, asset=asset, diff --git a/kobo/apps/subsequences/schema_extensions/v2/subsequences/extensions.py b/kobo/apps/subsequences/schema_extensions/v2/subsequences/extensions.py index 8a75ef77f1..6a1d7b3b74 100644 --- a/kobo/apps/subsequences/schema_extensions/v2/subsequences/extensions.py +++ b/kobo/apps/subsequences/schema_extensions/v2/subsequences/extensions.py @@ -1,23 +1,13 @@ # This drf-extension made for the metadata field of AccessLog targets the external class # and tells it what it should return when generating the schema. from drf_spectacular.extensions import OpenApiSerializerFieldExtension -from drf_spectacular.plumbing import build_object_type +from drf_spectacular.plumbing import build_array_type + +from kpi.schema_extensions.v2.generic.schema import GENERIC_OBJECT_SCHEMA + -from kpi.schema_extensions.v2.generic.schema import ( - GENERIC_STRING_SCHEMA, - USER_URL_SCHEMA, GENERIC_ARRAY_SCHEMA, -) class SubsequenceParamsFieldExtension(OpenApiSerializerFieldExtension): target_class = 'kobo.apps.subsequences.schema_extensions.v2.subsequences.fields.AdvancedFeatureParamsField' # noqa def map_serializer_field(self, auto_schema, direction): - return build_object_type( - properties={ - 'source': GENERIC_STRING_SCHEMA, - 'auth_type': GENERIC_STRING_SCHEMA, - 'ip_address': GENERIC_STRING_SCHEMA, - 'initial_user_uid': GENERIC_STRING_SCHEMA, - 'initial_user_username': GENERIC_STRING_SCHEMA, - 'authorized_app_name': GENERIC_STRING_SCHEMA, - } - ) + return build_array_type(schema=GENERIC_OBJECT_SCHEMA) diff --git a/kobo/apps/subsequences/tests/api/v2/test_views.py b/kobo/apps/subsequences/tests/api/v2/test_views.py index 2f297b1bf6..ce962f801f 100644 --- a/kobo/apps/subsequences/tests/api/v2/test_views.py +++ b/kobo/apps/subsequences/tests/api/v2/test_views.py @@ -34,10 +34,19 @@ def setUp(self): kwargs={'parent_lookup_asset': self.asset.uid, 'pk': self.action.uid}, ) self.client = Client(raise_request_exception=False) + self.client.force_login(user) def test_list_advanced_features(self): res = self.client.get(self.list_actions_url) assert res.status_code == status.HTTP_200_OK + assert res.json() == [ + { + 'action': 'manual_transcription', + 'question_xpath': 'q1', + 'params': [{'language': 'en'}], + 'uid': self.action.uid, + } + ] def test_update_action(self): res = self.client.patch( @@ -81,3 +90,24 @@ def test_cannot_delete_actions(self): assert QuestionAdvancedAction.objects.filter( asset=self.asset, action=self.action.action ).exists() + + def test_on_the_fly_migration(self): + self.action.delete() + self.asset.advanced_features = {'transcript': {'languages': ['en']}} + self.asset.known_cols = ['q1'] + self.asset.save() + res = self.client.get(self.list_actions_url) + assert QuestionAdvancedAction.objects.filter( + asset=self.asset, action=self.action.action + ).exists() + action = QuestionAdvancedAction.objects.get( + asset=self.asset, action=self.action.action, question_xpath='q1' + ) + assert res.json() == [ + { + 'action': 'manual_transcription', + 'question_xpath': 'q1', + 'params': [{'language': 'en'}], + 'uid': action.uid, + } + ] diff --git a/kobo/apps/subsequences/views.py b/kobo/apps/subsequences/views.py index 14fe747b77..cfa1701b91 100644 --- a/kobo/apps/subsequences/views.py +++ b/kobo/apps/subsequences/views.py @@ -1,24 +1,28 @@ from drf_spectacular.utils import OpenApiParameter, extend_schema, extend_schema_view - from rest_framework import mixins from rest_framework_extensions.mixins import NestedViewSetMixin from kobo.apps.audit_log.base_views import AuditLoggedViewSet from kobo.apps.audit_log.models import AuditType -from kobo.apps.subsequences.models import QuestionAdvancedAction -from kobo.apps.subsequences.schema_extensions.v2.subsequences.serializers import AdvancedFeatureResponse +from kobo.apps.subsequences.models import ( + QuestionAdvancedAction, + migrate_advanced_features, +) +from kobo.apps.subsequences.schema_extensions.v2.subsequences.serializers import ( + AdvancedFeatureResponse, +) from kobo.apps.subsequences.serializers import ( QuestionAdvancedActionSerializer, QuestionAdvancedActionUpdateSerializer, ) -from kpi.permissions import AssetNestedObjectPermission, AssetPermission, AssetAdvancedFeaturesPermission -from kpi.utils.viewset_mixins import AssetNestedObjectViewsetMixin +from kpi.permissions import AssetAdvancedFeaturesPermission from kpi.utils.schema_extensions.markdown import read_md from kpi.utils.schema_extensions.response import ( open_api_200_ok_response, open_api_201_created_response, - open_api_204_empty_response, ) +from kpi.utils.viewset_mixins import AssetNestedObjectViewsetMixin + @extend_schema( tags=['Advanced Features'], @@ -103,6 +107,8 @@ class QuestionAdvancedActionViewSet( pagination_class = None permission_classes = (AssetAdvancedFeaturesPermission,) def get_queryset(self): + if self.asset.advanced_features != {}: + migrate_advanced_features(self.asset) return QuestionAdvancedAction.objects.filter(asset=self.asset) def perform_create_override(self, serializer): serializer.save(asset=self.asset) diff --git a/kpi/schema_extensions/imports.py b/kpi/schema_extensions/imports.py index 8d6bef6fdd..f18bbdec6a 100644 --- a/kpi/schema_extensions/imports.py +++ b/kpi/schema_extensions/imports.py @@ -1,4 +1,5 @@ # flake8: noqa: F401 +import kobo.apps.subsequences.schema_extensions.v2.subsequences.extensions import kpi.schema_extensions.v2.asset_attachments.extensions import kpi.schema_extensions.v2.asset_permission_assignments.extensions import kpi.schema_extensions.v2.asset_snapshots.extensions From fbdd6531a13f100731bdf397e4130d73869963cf Mon Sep 17 00:00:00 2001 From: rgraber Date: Thu, 20 Nov 2025 07:56:18 +0100 Subject: [PATCH 20/20] fixup!: error if post to unmigrated asset --- kobo/apps/subsequences/tests/api/v2/test_views.py | 15 +++++++++++++++ kobo/apps/subsequences/views.py | 2 ++ 2 files changed, 17 insertions(+) diff --git a/kobo/apps/subsequences/tests/api/v2/test_views.py b/kobo/apps/subsequences/tests/api/v2/test_views.py index ce962f801f..45884f9e54 100644 --- a/kobo/apps/subsequences/tests/api/v2/test_views.py +++ b/kobo/apps/subsequences/tests/api/v2/test_views.py @@ -111,3 +111,18 @@ def test_on_the_fly_migration(self): 'uid': action.uid, } ] + + def test_cannot_post_to_unmigrated_asset(self): + self.action.delete() + self.asset.advanced_features = {'transcript': {'languages': ['en']}} + self.asset.known_cols = ['q1'] + self.asset.save() + res = self.client.post( + self.list_actions_url, + data={ + 'action': 'manual_translation', + 'params': json.dumps([{'language': 'de'}]), + 'question_xpath': 'q1', + }, + ) + assert res.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR diff --git a/kobo/apps/subsequences/views.py b/kobo/apps/subsequences/views.py index cfa1701b91..44761a8a8e 100644 --- a/kobo/apps/subsequences/views.py +++ b/kobo/apps/subsequences/views.py @@ -111,6 +111,8 @@ def get_queryset(self): migrate_advanced_features(self.asset) return QuestionAdvancedAction.objects.filter(asset=self.asset) def perform_create_override(self, serializer): + if self.asset.advanced_features != {}: + raise Exception('Migrate advanced features before creating new actions') serializer.save(asset=self.asset) def get_serializer_class(self): if self.action in ['update', 'partial_update']: