From bf7a780e899e84676daa0e430c5e1d5d0ff27e2b Mon Sep 17 00:00:00 2001 From: FrankApiyo Date: Wed, 8 Jan 2025 13:12:22 +0300 Subject: [PATCH 1/7] Add gejson attachment params to EXPORT_OPTIONS --- onadata/apps/viewer/models/export.py | 4 ++++ onadata/libs/utils/api_export_tools.py | 8 +++++++- onadata/libs/utils/export_tools.py | 20 ++++++++++++-------- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/onadata/apps/viewer/models/export.py b/onadata/apps/viewer/models/export.py index fd2623e93e..a2bfd8260a 100644 --- a/onadata/apps/viewer/models/export.py +++ b/onadata/apps/viewer/models/export.py @@ -111,6 +111,10 @@ class ExportBaseModel(models.Model): EXPORT_OPTION_FIELDS = [ "binary_select_multiples", "dataview_pk", + "title", + "fields", + "geo_fields", + "simple_style", "group_delimiter", "include_images", "include_labels", diff --git a/onadata/libs/utils/api_export_tools.py b/onadata/libs/utils/api_export_tools.py index a49a3cce43..3fa4988007 100644 --- a/onadata/libs/utils/api_export_tools.py +++ b/onadata/libs/utils/api_export_tools.py @@ -57,6 +57,7 @@ from onadata.libs.utils.export_tools import ( check_pending_export, get_latest_generic_export, + get_query_params_from_metadata, generate_attachments_zip_export, generate_entity_list_export, generate_export, @@ -151,7 +152,12 @@ def custom_response_handler( # noqa: C0901 ): export_type = Export.EXTERNAL_EXPORT - options = parse_request_export_options(request.query_params) + metadata_query_params = get_query_params_from_metadata(metadata) + + options = { + **parse_request_export_options(request.query_params), + **metadata_query_params, + } dataview_pk = hasattr(dataview, "pk") and dataview.pk options["dataview_pk"] = dataview_pk diff --git a/onadata/libs/utils/export_tools.py b/onadata/libs/utils/export_tools.py index bded181e99..cda0b4d59a 100644 --- a/onadata/libs/utils/export_tools.py +++ b/onadata/libs/utils/export_tools.py @@ -641,6 +641,17 @@ def generate_kml_export( return export +def get_query_params_from_metadata(metadata): + extra_data = metadata.extra_data + # build out query params to be used in GeoJsonSerializer + return { + "geo_field": extra_data.get("data_geo_field"), + "simple_style": extra_data.get("data_simple_style"), + "title": extra_data.get("data_title"), + "fields": extra_data.get("data_fields"), + } + + def generate_geojson_export( export_type, username, @@ -665,14 +676,7 @@ def generate_geojson_export( if xform is None: xform = XForm.objects.get(user__username=username, id_string=id_string) request = HttpRequest() - extra_data = metadata.extra_data - # build out query params to be used in GeoJsonSerializer - request.query_params = { - "geo_field": extra_data.get("data_geo_field"), - "simple_style": extra_data.get("data_simple_style"), - "title": extra_data.get("data_title"), - "fields": extra_data.get("data_fields"), - } + request.query_params = get_query_params_from_metadata(metadata) _context = {} _context["request"] = request # filter out deleted submissions From b31c692ede4efbd4e0e176dcb3cf587e4ba0ca90 Mon Sep 17 00:00:00 2001 From: FrankApiyo Date: Wed, 8 Jan 2025 13:45:54 +0300 Subject: [PATCH 2/7] Update GeoJSON export test cases --- onadata/libs/tests/utils/test_export_tools.py | 52 ++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/onadata/libs/tests/utils/test_export_tools.py b/onadata/libs/tests/utils/test_export_tools.py index 744bcd698a..26bf5169a6 100644 --- a/onadata/libs/tests/utils/test_export_tools.py +++ b/onadata/libs/tests/utils/test_export_tools.py @@ -261,7 +261,7 @@ def test_should_create_export_when_submission_deleted(self): self.assertTrue(will_create_new_export) - def test_should_not_create_new_export_when_old_exists(self): + def test_should_not_create_new_export_fn(self): export_type = "csv" self._publish_transportation_form_and_submit_instance() options = { @@ -310,6 +310,9 @@ def test_should_not_create_new_export_when_old_exists(self): self.assertEqual( { "dataview_pk": False, + "title": "start", + "fields": "", + "simple_style": True, "include_hxl": True, "include_images": True, "include_labels": False, @@ -336,6 +339,9 @@ def test_should_not_create_new_export_when_old_exists(self): self.assertEqual( { "dataview_pk": False, + "title": "start", + "fields": "", + "simple_style": True, "include_hxl": True, "include_images": True, "include_labels": False, @@ -349,6 +355,50 @@ def test_should_not_create_new_export_when_old_exists(self): Export.objects.get(xform=self.xform).options, ) + # New metadata will yield a new export + metadata.delete() + metadata = MetaData.objects.create( + content_type=ContentType.objects.get_for_model(XForm), + data_type="media", + data_value=f"xform_geojson {self.xform.id} testgeojson", + extra_data={ + "data_title": "end", + "data_fields": "", + "data_geo_field": "qn09", + "data_simple_style": True, + }, + object_id=self.xform.id, + ) + _response = custom_response_handler( + request, + self.xform, + {}, + export_type, + filename="testgeojson2", + dataview=False, + metadata=metadata, + ) + # we generated a new export since the extra_data has been updated + self.assertEqual(2, Export.objects.filter(xform=self.xform).count()) + self.assertEqual( + { + "dataview_pk": False, + "title": "end", + "fields": "", + "simple_style": True, + "include_hxl": True, + "include_images": True, + "include_labels": False, + "win_excel_utf8": False, + "group_delimiter": "/", + "include_reviews": False, + "remove_group_name": False, + "include_labels_only": False, + "split_select_multiples": True, + }, + Export.objects.filter(xform=self.xform).last().options, + ) + def test_should_create_new_export_when_filter_defined(self): export_type = "csv" options = { From 28d28e634c47ee479ff1f48d9330c8c235dddea7 Mon Sep 17 00:00:00 2001 From: FrankApiyo Date: Wed, 8 Jan 2025 14:31:30 +0300 Subject: [PATCH 3/7] Add test cases for export_tools --- onadata/libs/tests/utils/test_export_tools.py | 46 +++++++++++++++++++ onadata/libs/utils/api_export_tools.py | 2 +- onadata/libs/utils/export_tools.py | 24 ++++++++-- 3 files changed, 66 insertions(+), 6 deletions(-) diff --git a/onadata/libs/tests/utils/test_export_tools.py b/onadata/libs/tests/utils/test_export_tools.py index 26bf5169a6..2877600ad8 100644 --- a/onadata/libs/tests/utils/test_export_tools.py +++ b/onadata/libs/tests/utils/test_export_tools.py @@ -47,6 +47,7 @@ generate_geojson_export, generate_kml_export, generate_osm_export, + get_query_params_from_metadata, get_repeat_index_tags, kml_export_data, parse_request_export_options, @@ -277,6 +278,51 @@ def test_should_not_create_new_export_fn(self): self.assertFalse(will_create_new_export) + def test_get_query_params_from_metadata_fn(self): + self._publish_transportation_form_and_submit_instance() + metadata = MetaData.objects.create( + content_type=ContentType.objects.get_for_model(XForm), + data_type="media", + data_value=f"xform_geojson {self.xform.id} testgeojson", + extra_data={ + "data_title": "start", + "data_fields": "", + "data_geo_field": "qn09", + "data_simple_style": True, + }, + object_id=self.xform.id, + ) + self.assertEqual( + { + "title": "start", + "fields": "", + "geo_field": "qn09", + "simple_style": True, + }, + get_query_params_from_metadata(metadata), + ) + + metadata.delete() + metadata = MetaData.objects.create( + content_type=ContentType.objects.get_for_model(XForm), + data_type="media", + data_value=f"xform_geojson {self.xform.id} testgeojson", + extra_data={ + "data_title": "start", + "data_fields": "", + "data_geo_field": "qn09", + }, + object_id=self.xform.id, + ) + self.assertEqual( + { + "title": "start", + "fields": "", + "geo_field": "qn09", + }, + get_query_params_from_metadata(metadata), + ) + def test_should_not_create_new_export_when_old_exists(self): export_type = "geojson" self._publish_transportation_form_and_submit_instance() diff --git a/onadata/libs/utils/api_export_tools.py b/onadata/libs/utils/api_export_tools.py index 3fa4988007..a80daca5bf 100644 --- a/onadata/libs/utils/api_export_tools.py +++ b/onadata/libs/utils/api_export_tools.py @@ -152,7 +152,7 @@ def custom_response_handler( # noqa: C0901 ): export_type = Export.EXTERNAL_EXPORT - metadata_query_params = get_query_params_from_metadata(metadata) + metadata_query_params = get_query_params_from_metadata(metadata) or {} options = { **parse_request_export_options(request.query_params), diff --git a/onadata/libs/utils/export_tools.py b/onadata/libs/utils/export_tools.py index cda0b4d59a..31968c812f 100644 --- a/onadata/libs/utils/export_tools.py +++ b/onadata/libs/utils/export_tools.py @@ -642,13 +642,27 @@ def generate_kml_export( def get_query_params_from_metadata(metadata): + """ + Build out query params to be used in GeoJsonSerializer + """ + if metadata is None: + return None + extra_data = metadata.extra_data - # build out query params to be used in GeoJsonSerializer + if extra_data is None: + return None + + keys_mapping = { + "data_geo_field": "geo_field", + "data_simple_style": "simple_style", + "data_title": "title", + "data_fields": "fields", + } + return { - "geo_field": extra_data.get("data_geo_field"), - "simple_style": extra_data.get("data_simple_style"), - "title": extra_data.get("data_title"), - "fields": extra_data.get("data_fields"), + mapped_key: extra_data[original_key] + for original_key, mapped_key in keys_mapping.items() + if original_key in extra_data } From cba0f9791132d225280e641cfe74708e912aa459 Mon Sep 17 00:00:00 2001 From: FrankApiyo Date: Wed, 8 Jan 2025 17:15:24 +0300 Subject: [PATCH 4/7] Refactor: Use a single if statement to check if values are None --- onadata/libs/utils/export_tools.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/onadata/libs/utils/export_tools.py b/onadata/libs/utils/export_tools.py index 31968c812f..558ff2b239 100644 --- a/onadata/libs/utils/export_tools.py +++ b/onadata/libs/utils/export_tools.py @@ -645,13 +645,10 @@ def get_query_params_from_metadata(metadata): """ Build out query params to be used in GeoJsonSerializer """ - if metadata is None: + if metadata is None or metadata.extra_data is None: return None extra_data = metadata.extra_data - if extra_data is None: - return None - keys_mapping = { "data_geo_field": "geo_field", "data_simple_style": "simple_style", From d3842c4c500676b5912c9e1f56d3c595733ddefd Mon Sep 17 00:00:00 2001 From: FrankApiyo Date: Thu, 9 Jan 2025 09:44:19 +0300 Subject: [PATCH 5/7] test: Remove medata.delete, reuse & create metadata with different name --- onadata/libs/tests/utils/test_export_tools.py | 37 ++++++++++--------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/onadata/libs/tests/utils/test_export_tools.py b/onadata/libs/tests/utils/test_export_tools.py index 2877600ad8..a22a7bbf7f 100644 --- a/onadata/libs/tests/utils/test_export_tools.py +++ b/onadata/libs/tests/utils/test_export_tools.py @@ -283,12 +283,11 @@ def test_get_query_params_from_metadata_fn(self): metadata = MetaData.objects.create( content_type=ContentType.objects.get_for_model(XForm), data_type="media", - data_value=f"xform_geojson {self.xform.id} testgeojson", + data_value=f"xform_geojson {self.xform.id} testgeojson2", extra_data={ "data_title": "start", "data_fields": "", "data_geo_field": "qn09", - "data_simple_style": True, }, object_id=self.xform.id, ) @@ -297,32 +296,35 @@ def test_get_query_params_from_metadata_fn(self): "title": "start", "fields": "", "geo_field": "qn09", - "simple_style": True, }, get_query_params_from_metadata(metadata), ) - metadata.delete() - metadata = MetaData.objects.create( - content_type=ContentType.objects.get_for_model(XForm), - data_type="media", - data_value=f"xform_geojson {self.xform.id} testgeojson", - extra_data={ - "data_title": "start", - "data_fields": "", - "data_geo_field": "qn09", - }, - object_id=self.xform.id, - ) + metadata.extra_data = { + "data_title": "start", + "data_fields": "one,two", + "data_geo_field": "qn09", + } self.assertEqual( { "title": "start", - "fields": "", + "fields": "one,two", "geo_field": "qn09", }, get_query_params_from_metadata(metadata), ) + metadata.extra_data = { + "data_title": "start", + "data_fields": "", + "data_geo_field": "qn09", + "data_simple_style": True, + } + self.assertEqual( + {"title": "start", "fields": "", "geo_field": "qn09", "simple_style": True}, + get_query_params_from_metadata(metadata), + ) + def test_should_not_create_new_export_when_old_exists(self): export_type = "geojson" self._publish_transportation_form_and_submit_instance() @@ -402,11 +404,10 @@ def test_should_not_create_new_export_when_old_exists(self): ) # New metadata will yield a new export - metadata.delete() metadata = MetaData.objects.create( content_type=ContentType.objects.get_for_model(XForm), data_type="media", - data_value=f"xform_geojson {self.xform.id} testgeojson", + data_value=f"xform_geojson {self.xform.id} testgeojson2", extra_data={ "data_title": "end", "data_fields": "", From 38c89129c89b918bedb9d7587a013bc0e513c93f Mon Sep 17 00:00:00 2001 From: FrankApiyo Date: Thu, 9 Jan 2025 11:21:16 +0300 Subject: [PATCH 6/7] feat: Add workflows_dispatch trigger to arm runner --- .github/workflows/ecr-image-build-w-arm-runner.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ecr-image-build-w-arm-runner.yml b/.github/workflows/ecr-image-build-w-arm-runner.yml index d0f4a268fa..9276e9f495 100644 --- a/.github/workflows/ecr-image-build-w-arm-runner.yml +++ b/.github/workflows/ecr-image-build-w-arm-runner.yml @@ -11,6 +11,7 @@ on: # yamllint disable-line rule:truthy - "*-rc" tags: - "v*" + workflow_dispatch: jobs: build: From 99fcab48af021bebfac4a9ca711e2d67c5aa9821 Mon Sep 17 00:00:00 2001 From: FrankApiyo Date: Mon, 13 Jan 2025 09:54:59 +0300 Subject: [PATCH 7/7] chore(test): Remove unused _response variables --- onadata/libs/tests/utils/test_export_tools.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/onadata/libs/tests/utils/test_export_tools.py b/onadata/libs/tests/utils/test_export_tools.py index a22a7bbf7f..ace33bf42c 100644 --- a/onadata/libs/tests/utils/test_export_tools.py +++ b/onadata/libs/tests/utils/test_export_tools.py @@ -344,7 +344,7 @@ def test_should_not_create_new_export_when_old_exists(self): }, object_id=self.xform.id, ) - _response = custom_response_handler( + custom_response_handler( request, self.xform, {}, @@ -373,7 +373,7 @@ def test_should_not_create_new_export_when_old_exists(self): }, Export.objects.get(xform=self.xform).options, ) - _response = custom_response_handler( + custom_response_handler( request, self.xform, {}, @@ -416,7 +416,7 @@ def test_should_not_create_new_export_when_old_exists(self): }, object_id=self.xform.id, ) - _response = custom_response_handler( + custom_response_handler( request, self.xform, {},