From b7f8efc8cd51474003315eb4b6cc4d4c6626e835 Mon Sep 17 00:00:00 2001 From: Kipchirchir Sigei Date: Thu, 9 Feb 2023 17:48:37 +0300 Subject: [PATCH] Track granular form updates Signed-off-by: Kipchirchir Sigei --- .../api/tests/viewsets/test_xform_viewset.py | 40 +++++++++++++++++-- onadata/apps/api/viewsets/project_viewset.py | 25 ++++-------- onadata/apps/api/viewsets/xform_viewset.py | 40 ++++++++++++++----- onadata/apps/messaging/constants.py | 8 +++- onadata/apps/messaging/serializers.py | 2 +- onadata/apps/messaging/tests/test_utils.py | 8 +++- 6 files changed, 89 insertions(+), 34 deletions(-) diff --git a/onadata/apps/api/tests/viewsets/test_xform_viewset.py b/onadata/apps/api/tests/viewsets/test_xform_viewset.py index 19dca8a2a6..8aec9a96d6 100644 --- a/onadata/apps/api/tests/viewsets/test_xform_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_xform_viewset.py @@ -60,7 +60,8 @@ from onadata.apps.logger.xform_instance_parser import XLSFormError from onadata.apps.main.models import MetaData from onadata.apps.messaging.constants import ( - FORM_UPDATED, XFORM, PROJECT, FORM_DELETED, FORM_CREATED + FORM_UPDATED, XFORM, PROJECT, FORM_DELETED, FORM_CREATED, + FORM_RENAMED, FORM_ACTIVE ) from onadata.apps.viewer.models import Export from onadata.apps.viewer.models.export import ExportTypeError @@ -1351,7 +1352,7 @@ def test_publish_xlsform(self, mock_send_message): self.assertTrue(mock_send_message.called) mock_send_message.assert_called_with( instance_id=xform.id, - target_id=xform.project.pk, + target_id=xform.pk, target_type=XFORM, user=request.user, message_verb=FORM_CREATED @@ -1789,7 +1790,8 @@ def test_publish_invalid_xls_form_no_choices(self): ) self.assertEqual(response.data.get("text"), error_msg) - def test_partial_update(self): + @patch("onadata.apps.api.viewsets.xform_viewset.send_message") + def test_partial_update(self, mock_send_message): with HTTMock(enketo_mock): self._publish_xls_form_to_project() view = XFormViewSet.as_view({"patch": "partial_update"}) @@ -1813,6 +1815,38 @@ def test_partial_update(self): request = self.factory.patch("/", data=data, **self.extra) response = view(request, pk=self.xform.id) self.assertEqual(response.status_code, 200) + + # send messages upon form update + self.assertTrue(mock_send_message.called) + + # check calls to send_message triggered by patch request + mock_calls = mock_send_message.call_args_list + args, kwargs = mock_calls[0] + # test form rename message + message_kwargs = { + "instance_id": self.xform.id, + "target_id": self.xform.id, + "target_type": XFORM, + "user": self.xform.user, + "message_verb": FORM_RENAMED, + "custom_message": { + "old_title": "transportation_2011_07_25", + "new_title": "Hello & World!" + } + } + self.assertEqual(kwargs, message_kwargs) + + # test form status message + args, kwargs = mock_calls[1] + message_kwargs = { + "instance_id": self.xform.id, + "target_id": self.xform.id, + "target_type": XFORM, + "user": self.xform.user, + "message_verb": FORM_ACTIVE, + } + self.assertEqual(kwargs, message_kwargs) + xform_old_hash = self.xform.hash self.xform.refresh_from_db() self.assertTrue(self.xform.downloadable) diff --git a/onadata/apps/api/viewsets/project_viewset.py b/onadata/apps/api/viewsets/project_viewset.py index 0b67802ba2..a83ce5a41d 100644 --- a/onadata/apps/api/viewsets/project_viewset.py +++ b/onadata/apps/api/viewsets/project_viewset.py @@ -229,14 +229,6 @@ def share(self, request, *args, **kwargs): DEFAULT_FROM_EMAIL, (user.email,), ) - # send notification upon sharing project with multiple users - send_message( - instance_id=self.object.pk, - target_id=self.object.pk, - target_type=PROJECT, - user=user, - message_verb=message_verb - ) else: if email_msg: # send email if email_msg is present in payload @@ -246,20 +238,19 @@ def share(self, request, *args, **kwargs): DEFAULT_FROM_EMAIL, (user.email,), ) - # send notification upon sharing project with single user - send_message( - instance_id=self.object.pk, - target_id=self.object.pk, - target_type=PROJECT, - user=user, - message_verb=message_verb, - ) - else: return Response(data=serializer.errors, status=status.HTTP_400_BAD_REQUEST) # clear cache safe_delete(f"{PROJ_OWNER_CACHE}{self.object.pk}") + # send message upon sharing/unsharing project with user + send_message( + instance_id=self.object.pk, + target_id=self.object.pk, + target_type=PROJECT, + user=user, + message_verb=message_verb, + ) return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/onadata/apps/api/viewsets/xform_viewset.py b/onadata/apps/api/viewsets/xform_viewset.py index 38df02c6fe..7209cc26f8 100644 --- a/onadata/apps/api/viewsets/xform_viewset.py +++ b/onadata/apps/api/viewsets/xform_viewset.py @@ -53,8 +53,15 @@ from onadata.apps.logger.models.xform_version import XFormVersion from onadata.apps.logger.xform_instance_parser import XLSFormError from onadata.apps.messaging.constants import ( - FORM_UPDATED, EXPORT_CREATED, XFORM, - FORM_DELETED, FORM_CREATED, PROJECT + FORM_UPDATED, + EXPORT_CREATED, + XFORM, + FORM_DELETED, + FORM_CREATED, + PROJECT, + FORM_ACTIVE, + FORM_INACTIVE, + FORM_RENAMED ) from onadata.apps.messaging.serializers import send_message from onadata.apps.viewer.models.export import Export @@ -84,7 +91,7 @@ response_for_format, ) from onadata.libs.utils.cache_tools import PROJ_OWNER_CACHE, safe_delete -from onadata.libs.utils.common_tools import json_stream +from onadata.libs.utils.common_tools import json_stream, str_to_bool from onadata.libs.utils.csv_import import ( get_async_csv_submission_status, submission_xls_to_csv, @@ -436,11 +443,7 @@ def create_async(self, request, *args, **kwargs): owner.id, {"name": fname, "path": xls_file_path}, ) - resp.update( - { - "job_uuid": survey.task_id - } - ) + resp.update({"job_uuid": survey.task_id}) resp_code = status.HTTP_202_ACCEPTED # send form creation notification @@ -844,14 +847,31 @@ def partial_update(self, request, *args, **kwargs): try: # send notification for each form activity - if request.POST.get("title") or request.POST.get("title"): + if request.POST.get("title"): # send form update notification send_message( instance_id=self.object.pk, target_id=self.object.pk, target_type=XFORM, user=request.user or owner, - message_verb=FORM_UPDATED, + message_verb=FORM_RENAMED, + custom_message={ + "old_title": self.object.title, + "new_title": request.POST.get("title"), + }, + ) + if request.POST.get("downloadable"): + downloadable = request.POST.get("downloadable") + message_verb = ( + FORM_ACTIVE if str_to_bool(downloadable) else FORM_INACTIVE + ) + # send form status notification + send_message( + instance_id=self.object.pk, + target_id=self.object.pk, + target_type=XFORM, + user=request.user or owner, + message_verb=message_verb, ) return super().partial_update(request, *args, **kwargs) except XLSFormError as e: diff --git a/onadata/apps/messaging/constants.py b/onadata/apps/messaging/constants.py index 181c764730..0f2f86cf2d 100644 --- a/onadata/apps/messaging/constants.py +++ b/onadata/apps/messaging/constants.py @@ -35,6 +35,9 @@ FORM_UPDATED = "form_updated" FORM_CREATED = "form_created" FORM_DELETED = "form_deleted" +FORM_INACTIVE = "form_inactive" +FORM_ACTIVE = "form_active" +FORM_RENAMED = "form_renamed" EXPORT_CREATED = "export_created" EXPORT_DELETED = "export_deleted" PERMISSION_GRANTED = "permission_granted" @@ -44,7 +47,7 @@ SUBMISSION_DELETED, FORM_UPDATED, FORM_CREATED, FORM_DELETED, EXPORT_CREATED, EXPORT_DELETED, PROJECT_EDITED, PROJECT_SHARED, PROJECT_CREATED, USER_ADDED_TO_PROJECT, USER_REMOVED_FROM_PROJECT, - PROJECT_DELETED + PROJECT_DELETED, FORM_INACTIVE, FORM_ACTIVE ] VERB_TOPIC_DICT = { SUBMISSION_CREATED: "submission/created", @@ -54,6 +57,9 @@ FORM_UPDATED: "form/updated", FORM_CREATED: "form/created", FORM_DELETED: "form/deleted", + FORM_ACTIVE: "form/active", + FORM_RENAMED: "form/renamed", + FORM_INACTIVE: "form/inactive", EXPORT_CREATED: "export/created", EXPORT_DELETED: "export/deleted", PROJECT_CREATED: "project/created", diff --git a/onadata/apps/messaging/serializers.py b/onadata/apps/messaging/serializers.py index ce6cbea91c..f8e2e5fda8 100644 --- a/onadata/apps/messaging/serializers.py +++ b/onadata/apps/messaging/serializers.py @@ -17,7 +17,6 @@ from django.utils.translation import gettext as _ from rest_framework import exceptions, serializers -from onadata.apps.logger.models import Instance from onadata.apps.messaging.constants import MESSAGE, MESSAGE_VERBS from onadata.apps.messaging.utils import TargetDoesNotExist, get_target @@ -146,6 +145,7 @@ def create(self, validated_data): return instance +# pylint: disable=too-many-arguments def send_message( instance_id: Union[list, int], target_id: int, diff --git a/onadata/apps/messaging/tests/test_utils.py b/onadata/apps/messaging/tests/test_utils.py index 29d51ec61b..49ea2b7d0f 100644 --- a/onadata/apps/messaging/tests/test_utils.py +++ b/onadata/apps/messaging/tests/test_utils.py @@ -2,9 +2,11 @@ Tests messaging app utils """ import json +from mock import patch + from django.http.request import HttpRequest from django.test.utils import override_settings -from unittest.mock import patch + from onadata.apps.main.tests.test_base import TestBase from onadata.apps.messaging.serializers import send_message @@ -39,6 +41,9 @@ def is_valid(): @patch('onadata.apps.messaging.serializers.MessageSerializer') def test_custom_message(self, message_serializer_mock): + """ + Test custom_message passed into send_message function + """ def is_valid(): return True message_serializer_mock.is_valid.side_effect = is_valid @@ -73,4 +78,3 @@ def is_valid(): message_serializer_mock.called_with( data=data, context={"request": request} ) -