From eb9ab89f6299a98605f1ffe555cad9e87ffb53c8 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Mon, 18 Nov 2019 18:05:54 -0400 Subject: [PATCH 1/4] Stopped trying to update KoBoCat 'has_active_hooks' boolean on Asset deletion, pass silently if KoBoCat returns a 404 and 'has_active_hooks' equals 'False' --- kpi/deployment_backends/kobocat_backend.py | 18 ++++++++++++------ kpi/signals.py | 2 +- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/kpi/deployment_backends/kobocat_backend.py b/kpi/deployment_backends/kobocat_backend.py index abe19afcd4..91a60d5607 100644 --- a/kpi/deployment_backends/kobocat_backend.py +++ b/kpi/deployment_backends/kobocat_backend.py @@ -347,7 +347,7 @@ def set_asset_uid(self, force=False): def set_has_kpi_hooks(self): """ PATCH `has_kpi_hooks` boolean of survey. - It lets `kc` know whether it needs to ping `kpi` + It lets KoBoCat know whether it needs to ping KPI each time a submission comes in. Store results in self.asset._deployment_data @@ -359,11 +359,17 @@ def set_has_kpi_hooks(self): 'has_kpi_hooks': has_active_hooks, 'kpi_asset_uid': self.asset.uid } - json_response = self._kobocat_request('PATCH', url, data=payload) - assert(json_response['has_kpi_hooks'] == has_active_hooks) - self.store_data({ - 'backend_response': json_response, - }) + + try: + json_response = self._kobocat_request('PATCH', url, data=payload) + assert (json_response['has_kpi_hooks'] == has_active_hooks) + self.store_data({ + 'backend_response': json_response, + }) + except KobocatDeploymentException as e: + if not (has_active_hooks is False and hasattr(e, 'response') and + e.response.status_code != status.HTTP_404_NOT_FOUND): + raise e def delete(self): """ diff --git a/kpi/signals.py b/kpi/signals.py index 3b01e1ae14..c9a6f7497a 100644 --- a/kpi/signals.py +++ b/kpi/signals.py @@ -115,7 +115,7 @@ def tag_uid_post_save(sender, instance, created, raw, **kwargs): TagUid.objects.get_or_create(tag=instance) -@receiver([post_save, post_delete], sender=Hook) +@receiver(post_save, sender=Hook) def update_kc_xform_has_kpi_hooks(sender, instance, **kwargs): """ Updates `kc.XForm` instance as soon as Asset.Hook list is updated. From 2f4e58fd5f42b76695484917630b39be22fff644 Mon Sep 17 00:00:00 2001 From: "John N. Milner" Date: Fri, 17 Apr 2020 13:21:58 -0400 Subject: [PATCH 2/4] Capitalize KoBoCAT in docs and messages --- kobo/settings/testing.py | 2 +- kpi/deployment_backends/kobocat_backend.py | 10 +++++----- kpi/utils/mongo_helper.py | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/kobo/settings/testing.py b/kobo/settings/testing.py index cd04898229..8743ecaac0 100644 --- a/kobo/settings/testing.py +++ b/kobo/settings/testing.py @@ -1,7 +1,7 @@ # coding: utf-8 from .base import * -# For tests, don't use KoBoCat's DB +# For tests, don't use KoBoCAT's DB DATABASES = { 'default': dj_database_url.config(default='sqlite:///%s/db.sqlite3' % BASE_DIR), } diff --git a/kpi/deployment_backends/kobocat_backend.py b/kpi/deployment_backends/kobocat_backend.py index 91a60d5607..9a5ab36910 100644 --- a/kpi/deployment_backends/kobocat_backend.py +++ b/kpi/deployment_backends/kobocat_backend.py @@ -317,7 +317,7 @@ def set_active(self, active): def set_asset_uid(self, force=False): """ Link KoBoCAT `XForm` back to its corresponding KPI `Asset` by - populating the `kpi_asset_uid` field (use KoBoCat proxy to PATCH XForm). + populating the `kpi_asset_uid` field (use KoBoCAT proxy to PATCH XForm). Useful when a form is created from the legacy upload form. Store results in self.asset._deployment_data @@ -347,7 +347,7 @@ def set_asset_uid(self, force=False): def set_has_kpi_hooks(self): """ PATCH `has_kpi_hooks` boolean of survey. - It lets KoBoCat know whether it needs to ping KPI + It lets KoBoCAT know whether it needs to ping KPI each time a submission comes in. Store results in self.asset._deployment_data @@ -388,7 +388,7 @@ def delete(self): def delete_submission(self, pk, user): """ - Deletes submission through `KoBoCat` proxy + Deletes submission through KoBoCAT proxy :param pk: int :param user: User :return: dict @@ -401,7 +401,7 @@ def delete_submission(self, pk, user): def delete_submissions(self, data, user): """ - Deletes submissions through `KoBoCat` proxy + Deletes submissions through KoBoCAT proxy :param user: User :return: dict """ @@ -735,7 +735,7 @@ def __prepare_as_drf_response_signature(requests_response): except ValueError as e: if not requests_response.status_code == status.HTTP_204_NO_CONTENT: prepared_drf_response['data'] = { - 'detail': _('KoBoCat returned an unexpected response: {}'.format(str(e))) + 'detail': _('KoBoCAT returned an unexpected response: {}'.format(str(e))) } return prepared_drf_response diff --git a/kpi/utils/mongo_helper.py b/kpi/utils/mongo_helper.py index 92251c17eb..d367e8f59c 100644 --- a/kpi/utils/mongo_helper.py +++ b/kpi/utils/mongo_helper.py @@ -31,7 +31,7 @@ class MongoHelper: (re.compile(base64_encodestring('.').strip()), '.'), ] - # Match KoBoCat's variables of ParsedInstance class + # Match KoBoCAT's variables of ParsedInstance class USERFORM_ID = '_userform_id' DEFAULT_BATCHSIZE = 1000 From 65135297a65261a62a573043e4bb9a8045d8655e Mon Sep 17 00:00:00 2001 From: "John N. Milner" Date: Fri, 17 Apr 2020 13:39:55 -0400 Subject: [PATCH 3/4] Add explanatory comment and adjust formatting --- kpi/deployment_backends/kobocat_backend.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/kpi/deployment_backends/kobocat_backend.py b/kpi/deployment_backends/kobocat_backend.py index 9a5ab36910..2d018aad08 100644 --- a/kpi/deployment_backends/kobocat_backend.py +++ b/kpi/deployment_backends/kobocat_backend.py @@ -367,9 +367,16 @@ def set_has_kpi_hooks(self): 'backend_response': json_response, }) except KobocatDeploymentException as e: - if not (has_active_hooks is False and hasattr(e, 'response') and - e.response.status_code != status.HTTP_404_NOT_FOUND): - raise e + if ( + has_active_hooks is False + and hasattr(e, 'response') + and e.response.status_code == status.HTTP_404_NOT_FOUND + ): + # It's okay if we're trying to unset the active hooks flag and + # the KoBoCAT project is already gone. See #2497 + pass + else: + raise def delete(self): """ @@ -379,7 +386,10 @@ def delete(self): try: self._kobocat_request('DELETE', url) except KobocatDeploymentException as e: - if hasattr(e, 'response') and e.response.status_code == 404: + if ( + hasattr(e, 'response') + and e.response.status_code == status.HTTP_404_NOT_FOUND + ): # The KC project is already gone! pass else: From 92e7a794cf20fed0464b7bb6fed010fe5caf562a Mon Sep 17 00:00:00 2001 From: "John N. Milner" Date: Fri, 17 Apr 2020 13:42:31 -0400 Subject: [PATCH 4/4] `try` only the call that could raise the exception and `assert` without parentheses --- kpi/deployment_backends/kobocat_backend.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/kpi/deployment_backends/kobocat_backend.py b/kpi/deployment_backends/kobocat_backend.py index 2d018aad08..d69a7c3a48 100644 --- a/kpi/deployment_backends/kobocat_backend.py +++ b/kpi/deployment_backends/kobocat_backend.py @@ -308,7 +308,7 @@ def set_active(self, active): 'downloadable': bool(active) } json_response = self._kobocat_request('PATCH', url, data=payload) - assert(json_response['downloadable'] == bool(active)) + assert json_response['downloadable'] == bool(active) self.store_data({ 'active': json_response['downloadable'], 'backend_response': json_response, @@ -362,10 +362,6 @@ def set_has_kpi_hooks(self): try: json_response = self._kobocat_request('PATCH', url, data=payload) - assert (json_response['has_kpi_hooks'] == has_active_hooks) - self.store_data({ - 'backend_response': json_response, - }) except KobocatDeploymentException as e: if ( has_active_hooks is False @@ -377,6 +373,11 @@ def set_has_kpi_hooks(self): pass else: raise + else: + assert json_response['has_kpi_hooks'] == has_active_hooks + self.store_data({ + 'backend_response': json_response, + }) def delete(self): """