From 7b744e3d091de8bffe99110b1aaf0c14c4397c02 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Tue, 12 Nov 2024 08:41:34 -0500 Subject: [PATCH] feat(projectHistoryLogs): record project history logs for imports TASK-944 (#5230) --- kobo/apps/audit_log/audit_actions.py | 1 + kobo/apps/audit_log/models.py | 39 ++++- kobo/apps/audit_log/signals.py | 11 +- kobo/apps/audit_log/tests/test_models.py | 90 +++++++++++- .../tests/test_project_history_logs.py | 135 ++++++++++++++++-- kpi/fixtures/asset_with_settings_and_qa.json | 10 +- kpi/models/import_export_task.py | 31 ++-- kpi/tasks.py | 1 + kpi/tests/api/v2/test_api_imports.py | 88 ++++++++++-- kpi/views/v1/import_task.py | 6 + kpi/views/v2/import_task.py | 12 +- 11 files changed, 387 insertions(+), 37 deletions(-) diff --git a/kobo/apps/audit_log/audit_actions.py b/kobo/apps/audit_log/audit_actions.py index 107caef0cb..a85b3ef563 100644 --- a/kobo/apps/audit_log/audit_actions.py +++ b/kobo/apps/audit_log/audit_actions.py @@ -22,6 +22,7 @@ class AuditAction(models.TextChoices): REDEPLOY = 'redeploy' REGISTER_SERVICE = 'register-service' REMOVE = 'remove' + REPLACE_FORM = 'replace-form' UNARCHIVE = 'unarchive' UPDATE = 'update' UPDATE_CONTENT = 'update-content' diff --git a/kobo/apps/audit_log/models.py b/kobo/apps/audit_log/models.py index 21a07a8240..2bc70292cb 100644 --- a/kobo/apps/audit_log/models.py +++ b/kobo/apps/audit_log/models.py @@ -22,7 +22,7 @@ PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, ) from kpi.fields.kpi_uid import UUID_LENGTH -from kpi.models import Asset +from kpi.models import Asset, ImportTask from kpi.utils.log import logging NEW = 'new' @@ -561,3 +561,40 @@ def create_from_related_request( ProjectHistoryLog.objects.create( user=request.user, object_id=object_id, action=action, metadata=metadata ) + + @classmethod + def create_from_import_task(cls, task: ImportTask): + # this will probably only ever be a list of size 1 or 0, + # sent as a list because of how ImportTask is implemented + # if somehow a task updates multiple assets, this should handle it + audit_log_blocks = task.messages.get('audit_logs', []) + for audit_log_info in audit_log_blocks: + metadata = { + 'asset_uid': audit_log_info['asset_uid'], + 'latest_version_uid': audit_log_info['latest_version_uid'], + 'ip_address': audit_log_info['ip_address'], + 'source': audit_log_info['source'], + 'log_subtype': PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, + } + ProjectHistoryLog.objects.create( + user=task.user, + object_id=audit_log_info['asset_id'], + action=AuditAction.REPLACE_FORM, + metadata=metadata, + ) + # imports may change the name of an asset, log that too + if audit_log_info['old_name'] != audit_log_info['new_name']: + metadata.update( + { + 'name': { + OLD: audit_log_info['old_name'], + NEW: audit_log_info['new_name'], + } + } + ) + ProjectHistoryLog.objects.create( + user=task.user, + object_id=audit_log_info['asset_id'], + action=AuditAction.UPDATE_NAME, + metadata=metadata, + ) diff --git a/kobo/apps/audit_log/signals.py b/kobo/apps/audit_log/signals.py index 046ea518eb..cd143bf208 100644 --- a/kobo/apps/audit_log/signals.py +++ b/kobo/apps/audit_log/signals.py @@ -1,8 +1,11 @@ +from celery.signals import task_success from django.contrib.auth.signals import user_logged_in from django.dispatch import receiver +from kpi.models import ImportTask +from kpi.tasks import import_in_background from kpi.utils.log import logging -from .models import AccessLog +from .models import AccessLog, ProjectHistoryLog @receiver(user_logged_in) @@ -14,3 +17,9 @@ def create_access_log(sender, user, **kwargs): AccessLog.create_from_request(request, user) else: AccessLog.create_from_request(request) + + +@receiver(task_success, sender=import_in_background) +def create_ph_log_for_import(sender, result, **kwargs): + task = ImportTask.objects.get(uid=result) + ProjectHistoryLog.create_from_import_task(task) diff --git a/kobo/apps/audit_log/tests/test_models.py b/kobo/apps/audit_log/tests/test_models.py index 6acaeb1b63..ae98bc484f 100644 --- a/kobo/apps/audit_log/tests/test_models.py +++ b/kobo/apps/audit_log/tests/test_models.py @@ -22,8 +22,9 @@ from kpi.constants import ( ACCESS_LOG_SUBMISSION_AUTH_TYPE, ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE, + PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, ) -from kpi.models import Asset +from kpi.models import Asset, ImportTask from kpi.tests.base_test_case import BaseTestCase @@ -556,3 +557,90 @@ def test_create_from_related_request_no_log_created_if_no_data(self): modify_action=AuditAction.UPDATE, ) self.assertEqual(ProjectHistoryLog.objects.count(), 0) + + def test_create_from_import_task_no_name_change(self): + asset = Asset.objects.get(pk=1) + task = ImportTask.objects.create( + user=User.objects.get(username='someuser'), data={} + ) + task.messages = { + 'audit_logs': [ + { + 'asset_uid': asset.uid, + 'latest_version_uid': 'av12345', + 'ip_address': '1.2.3.4', + 'source': 'source', + 'asset_id': asset.id, + 'old_name': asset.name, + 'new_name': asset.name, + } + ] + } + ProjectHistoryLog.create_from_import_task(task) + self.assertEqual(ProjectHistoryLog.objects.count(), 1) + log = ProjectHistoryLog.objects.first() + self.assertEqual(log.action, AuditAction.REPLACE_FORM) + self.assertEqual(log.object_id, asset.id) + + # data from 'messages' should be copied to the log + self.assertDictEqual( + log.metadata, + { + 'ip_address': '1.2.3.4', + 'asset_uid': asset.uid, + 'source': 'source', + 'latest_version_uid': 'av12345', + 'log_subtype': PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, + }, + ) + + def test_create_from_import_task_with_name_change(self): + asset = Asset.objects.get(pk=1) + old_name = asset.name + task = ImportTask.objects.create( + user=User.objects.get(username='someuser'), data={} + ) + task.messages = { + 'audit_logs': [ + { + 'asset_uid': asset.uid, + 'latest_version_uid': 'av12345', + 'ip_address': '1.2.3.4', + 'source': 'source', + 'asset_id': asset.id, + 'old_name': old_name, + 'new_name': 'new_name', + } + ] + } + ProjectHistoryLog.create_from_import_task(task) + self.assertEqual(ProjectHistoryLog.objects.count(), 2) + log = ProjectHistoryLog.objects.filter(action=AuditAction.REPLACE_FORM).first() + self.assertEqual(log.object_id, asset.id) + + self.assertDictEqual( + log.metadata, + { + 'ip_address': '1.2.3.4', + 'asset_uid': asset.uid, + 'source': 'source', + 'latest_version_uid': 'av12345', + 'log_subtype': PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, + }, + ) + name_log = ProjectHistoryLog.objects.filter( + action=AuditAction.UPDATE_NAME + ).first() + self.assertEqual(log.object_id, asset.id) + + self.assertDictEqual( + name_log.metadata, + { + 'ip_address': '1.2.3.4', + 'asset_uid': asset.uid, + 'source': 'source', + 'latest_version_uid': 'av12345', + 'log_subtype': PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, + 'name': {'old': old_name, 'new': 'new_name'}, + }, + ) 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 ae0eac6cb8..e386bc4733 100644 --- a/kobo/apps/audit_log/tests/test_project_history_logs.py +++ b/kobo/apps/audit_log/tests/test_project_history_logs.py @@ -1,8 +1,11 @@ +import base64 import copy import json +from unittest.mock import patch import jsonschema.exceptions -from ddt import data, ddt +import responses +from ddt import data, ddt, unpack from django.test import override_settings from django.urls import reverse from rest_framework.reverse import reverse as drf_reverse @@ -12,13 +15,18 @@ from kobo.apps.audit_log.tests.test_models import BaseAuditLogTestCase from kobo.apps.hook.models import Hook from kobo.apps.kobo_auth.shortcuts import User +from kpi.constants import PROJECT_HISTORY_LOG_PROJECT_SUBTYPE from kpi.models import Asset, AssetFile, PairedData from kpi.models.asset import AssetSetting +from kpi.utils.strings import to_str @ddt @override_settings(DEFAULT_DEPLOYMENT_BACKEND='mock') class TestProjectHistoryLogs(BaseAuditLogTestCase): + """ + Integration tests for flows that create ProjectHistoryLogs + """ fixtures = ['test_data', 'asset_with_settings_and_qa'] @@ -36,10 +44,11 @@ def setUp(self): self.detail_url = 'asset-detail' self.deployment_url = 'asset-deployment' - def _check_common_metadata(self, metadata_dict): + def _check_common_metadata(self, metadata_dict, expected_subtype): self.assertEqual(metadata_dict['asset_uid'], self.asset.uid) self.assertEqual(metadata_dict['ip_address'], '127.0.0.1') self.assertEqual(metadata_dict['source'], 'source') + self.assertEqual(metadata_dict['log_subtype'], expected_subtype) def _base_asset_detail_endpoint_test( self, patch, url_name, request_data, expected_action, use_v2=True @@ -48,7 +57,11 @@ def _base_asset_detail_endpoint_test( url = reverse(f'{url_name_prefix}{url_name}', kwargs={'uid': self.asset.uid}) method = self.client.patch if patch else self.client.post log_metadata = self._base_project_history_log_test( - method, url, request_data, expected_action + method, + url, + request_data, + expected_action, + PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, ) self.assertEqual( log_metadata['latest_version_uid'], self.asset.latest_version.uid @@ -56,7 +69,7 @@ def _base_asset_detail_endpoint_test( return log_metadata def _base_project_history_log_test( - self, method, url, request_data, expected_action + self, method, url, request_data, expected_action, expected_subtype ): # requests are either patches or posts # hit the endpoint with the correct data @@ -72,9 +85,9 @@ def _base_project_history_log_test( self.assertEqual(logs.count(), 1) log = logs.first() # check the log has the expected fields and metadata - self._check_common_metadata(log.metadata) self.assertEqual(log.object_id, self.asset.id) self.assertEqual(log.action, expected_action) + self._check_common_metadata(log.metadata, expected_subtype) return log.metadata def test_first_time_deployment_creates_log(self): @@ -395,17 +408,21 @@ def test_falsy_field_creates_sharing_disabled_log(self): def test_modify_sharing_creates_log(self): self.asset.data_sharing = { 'enabled': True, - 'fields': ['q1'], + 'fields': ['settings_fixture_q1'], } self.asset.save() log_metadata = self._base_asset_detail_endpoint_test( patch=True, url_name=self.detail_url, - request_data={'data_sharing': {'enabled': True, 'fields': ['q2']}}, + request_data={ + 'data_sharing': {'enabled': True, 'fields': ['settings_fixture_q2']} + }, expected_action=AuditAction.MODIFY_SHARING, ) - self.assertEqual(log_metadata['shared_fields'][ADDED], ['q2']) - self.assertEqual(log_metadata['shared_fields'][REMOVED], ['q1']) + self.assertEqual(log_metadata['shared_fields'][ADDED], ['settings_fixture_q2']) + self.assertEqual( + log_metadata['shared_fields'][REMOVED], ['settings_fixture_q1'] + ) @data(True, False) def test_update_content_creates_log(self, use_v2): @@ -481,6 +498,7 @@ def test_register_service_creates_log(self, use_v2): url=url, request_data=request_data, expected_action=AuditAction.REGISTER_SERVICE, + expected_subtype=PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, ) new_hook = Hook.objects.get(name='test') self.assertEqual(log_metadata['hook']['uid'], new_hook.uid) @@ -510,6 +528,7 @@ def test_modify_service_creates_log(self, use_v2): ), request_data=request_data, expected_action=AuditAction.MODIFY_SERVICE, + expected_subtype=PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, ) self.assertEqual(log_metadata['hook']['uid'], new_hook.uid) self.assertEqual(log_metadata['hook']['active'], False) @@ -537,6 +556,7 @@ def test_delete_service_creates_log(self, use_v2): ), request_data=request_data, expected_action=AuditAction.DELETE_SERVICE, + expected_subtype=PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, ) self.assertEqual(log_metadata['hook']['uid'], new_hook.uid) self.assertEqual(log_metadata['hook']['active'], True) @@ -565,6 +585,7 @@ def test_connect_project_creates_log(self): url=url, request_data=request_data, expected_action=AuditAction.CONNECT_PROJECT, + expected_subtype=PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, ) self.assertEqual(log_metadata['paired-data']['source_name'], source.name) self.assertEqual(log_metadata['paired-data']['source_uid'], source.uid) @@ -598,6 +619,7 @@ def test_disconnect_project_creates_log(self): ), expected_action=AuditAction.DISCONNECT_PROJECT, request_data=None, + expected_subtype=PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, ) self.assertEqual(log_metadata['paired-data']['source_name'], source.name) self.assertEqual(log_metadata['paired-data']['source_uid'], source.uid) @@ -630,6 +652,7 @@ def test_modify_imported_fields_creates_log(self): ), expected_action=AuditAction.MODIFY_IMPORTED_FIELDS, request_data={'fields': ['q2']}, + expected_subtype=PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, ) self.assertEqual(log_metadata['paired-data']['source_name'], source.name) self.assertEqual(log_metadata['paired-data']['source_uid'], source.uid) @@ -657,6 +680,7 @@ def test_add_media_creates_log(self, use_v2): url=url, request_data=request_data, expected_action=AuditAction.ADD_MEDIA, + expected_subtype=PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, ) file = AssetFile.objects.filter(asset=self.asset).first() self.assertEqual(log_metadata['asset-file']['uid'], file.uid) @@ -685,8 +709,101 @@ def test_delete_media_creates_log(self, use_v2): ), expected_action=AuditAction.DELETE_MEDIA, request_data=None, + expected_subtype=PROJECT_HISTORY_LOG_PROJECT_SUBTYPE, ) self.assertEqual(log_metadata['asset-file']['uid'], media.uid) self.assertEqual(log_metadata['asset-file']['filename'], media.filename) self.assertEqual(log_metadata['asset-file']['download_url'], media.download_url) self.assertEqual(log_metadata['asset-file']['md5_hash'], media.md5_hash) + + @responses.activate + @data( + # File or url, change asset name?, use v2? + ('file', True, True), + ('file', False, True), + ('url', True, True), + ('url', False, True), + ('file', True, False), + ('file', False, False), + ('url', True, False), + ('url', False, False), + ) + @unpack + def test_create_from_import_task(self, file_or_url, change_name, use_v2): + task_data = { + 'destination': reverse( + 'api_v2:asset-detail', kwargs={'uid': self.asset.uid} + ), + 'name': 'name', + } + old_name = self.asset.name + + # create an xlsx file to import + # if changing the name of the asset, create a file from an asset with + # a different name and versioned=True, which will add the name to the + # 'settings' sheet (only works for deployed assets) + if change_name: + new_asset = Asset.objects.get(pk=1) + new_asset.save() + new_asset.deploy(backend='mock') + xlsx_io = new_asset.to_xlsx_io(versioned=True).read() + else: + xlsx_io = self.asset.to_xlsx_io().read() + + if file_or_url == 'url': + # pretend to host the file somewhere + mock_xls_url = 'http://mock.kbtdev.org/form.xls' + responses.add( + responses.GET, + mock_xls_url, + content_type='application/xls', + body=xlsx_io, + ) + task_data['url'] = mock_xls_url + else: + encoded_xls = base64.b64encode(xlsx_io) + task_data['base64Encoded'] = ('base64:{}'.format(to_str(encoded_xls)),) + + # hit the endpoint that creates and runs the ImportTask + # Task should complete right away due to `CELERY_TASK_ALWAYS_EAGER` + version = 'v2' if use_v2 else 'v1' + url_prefix = 'api_v2:' if use_v2 else '' + with patch( + f'kpi.views.{version}.import_task.get_client_ip', return_value='127.0.0.1' + ): + with patch( + f'kpi.views.{version}.import_task.get_human_readable_client_user_agent', + return_value='source', + ): + self.client.post(reverse(f'{url_prefix}importtask-list'), task_data) + expected_logs_count = 2 if change_name else 1 + log_query = ProjectHistoryLog.objects.filter(metadata__asset_uid=self.asset.uid) + self.assertEqual(log_query.count(), expected_logs_count) + form_replace_log = log_query.filter(action=AuditAction.REPLACE_FORM).first() + self.assertEqual(form_replace_log.object_id, self.asset.id) + self._check_common_metadata( + form_replace_log.metadata, PROJECT_HISTORY_LOG_PROJECT_SUBTYPE + ) + self.assertEqual( + form_replace_log.metadata['latest_version_uid'], + self.asset.latest_version.uid, + ) + + if change_name: + # if the import also changed the name of the asset, + # check that was logged as well + change_name_log = log_query.filter(action=AuditAction.UPDATE_NAME).first() + self._check_common_metadata( + change_name_log.metadata, PROJECT_HISTORY_LOG_PROJECT_SUBTYPE + ) + self.assertEqual( + change_name_log.metadata['latest_version_uid'], + self.asset.latest_version.uid, + ) + self.assertDictEqual( + change_name_log.metadata['name'], + { + OLD: old_name, + NEW: new_asset.name, + }, + ) diff --git a/kpi/fixtures/asset_with_settings_and_qa.json b/kpi/fixtures/asset_with_settings_and_qa.json index ae407f71e5..2dd294471b 100644 --- a/kpi/fixtures/asset_with_settings_and_qa.json +++ b/kpi/fixtures/asset_with_settings_and_qa.json @@ -9,14 +9,14 @@ "survey":[ { "type":"text", - "label":"fixture q1", - "name":"q1", + "label":"settings fixture q1", + "name":"settings_fixture_q1", "kuid":"abc" }, { "type":"text", - "label":"fixture q2", - "name":"q2", + "label":"settings fixture q2", + "name":"settings_fixture_q2", "kuid":"def" } ] @@ -61,4 +61,4 @@ "uid":"aNp9yMt4zKpUtTeZUnozYG" } } -] + ] diff --git a/kpi/models/import_export_task.py b/kpi/models/import_export_task.py index 322f2cad76..532586b5dd 100644 --- a/kpi/models/import_export_task.py +++ b/kpi/models/import_export_task.py @@ -18,6 +18,7 @@ from backports.zoneinfo import ZoneInfo import constance +import formpack import requests from django.conf import settings from django.contrib.postgres.indexes import BTreeIndex, HashIndex @@ -26,13 +27,6 @@ from django.db.models import F from django.urls import reverse from django.utils.translation import gettext as t -from openpyxl.utils.exceptions import InvalidFileException -from private_storage.fields import PrivateFileField -from pyxform.xls2json_backends import xls_to_dict, xlsx_to_dict -from rest_framework import exceptions -from werkzeug.http import parse_options_header - -import formpack from formpack.constants import KOBO_LOCK_SHEET from formpack.schema.fields import ( IdCopyField, @@ -43,6 +37,12 @@ ) from formpack.utils.kobo_locking import get_kobo_locking_profiles from formpack.utils.string import ellipsize +from openpyxl.utils.exceptions import InvalidFileException +from private_storage.fields import PrivateFileField +from pyxform.xls2json_backends import xls_to_dict, xlsx_to_dict +from rest_framework import exceptions +from werkzeug.http import parse_options_header + from kobo.apps.reports.report_data import build_formpack from kobo.apps.subsequences.utils import stream_with_extras from kpi.constants import ( @@ -291,7 +291,6 @@ def _run_task(self, messages): # When a file is uploaded as base64, # no name is provided in the encoded string # We should rely on self.data.get(:filename:) - self._parse_b64_upload( base64_encoded_upload=self.data['base64Encoded'], filename=filename, @@ -354,7 +353,8 @@ def _load_assets_from_url(self, url, messages, **kwargs): 'uid': asset.uid, 'kind': 'asset', 'owner__username': self.user.username, - }) + } + ) if item.parent: collections_to_assign.append([ @@ -444,8 +444,21 @@ def _parse_b64_upload(self, base64_encoded_upload, messages, **kwargs): base64_encoded_upload, survey_dict ) asset.content = survey_dict + old_name = asset.name + # saving sometimes changes the name asset.save() msg_key = 'updated' + messages['audit_logs'].append( + { + 'asset_uid': asset.uid, + 'asset_id': asset.id, + 'latest_version_uid': asset.latest_version.uid, + 'ip_address': self.data.get('ip_address', None), + 'source': self.data.get('source', None), + 'old_name': old_name, + 'new_name': asset.name, + } + ) messages[msg_key].append({ 'uid': asset.uid, diff --git a/kpi/tasks.py b/kpi/tasks.py index 050b73108a..4ba283d8c3 100644 --- a/kpi/tasks.py +++ b/kpi/tasks.py @@ -21,6 +21,7 @@ def import_in_background(import_task_uid): import_task = ImportTask.objects.get(uid=import_task_uid) import_task.run() + return import_task.uid @celery_app.task diff --git a/kpi/tests/api/v2/test_api_imports.py b/kpi/tests/api/v2/test_api_imports.py index dca4323b5c..2109a33de1 100644 --- a/kpi/tests/api/v2/test_api_imports.py +++ b/kpi/tests/api/v2/test_api_imports.py @@ -1,10 +1,10 @@ # coding: utf-8 import base64 +import unittest from io import BytesIO -import responses -import unittest import openpyxl +import responses import xlwt from django.db import transaction from rest_framework import status @@ -12,7 +12,7 @@ from kobo.apps.kobo_auth.shortcuts import User from kpi.constants import ASSET_TYPE_BLOCK, ASSET_TYPE_QUESTION -from kpi.models import Asset +from kpi.models import Asset, ImportTask from kpi.tests.base_test_case import BaseTestCase from kpi.urls.router_api_v2 import URL_NAMESPACE as ROUTER_URL_NAMESPACE from kpi.utils.strings import to_str @@ -949,7 +949,9 @@ def test_import_invalid_host_url(self): self.assertEqual(detail_response.status_code, status.HTTP_200_OK) def test_import_xls_with_default_language_but_no_translations(self): - xlsx_io = self.asset.to_xlsx_io(append={"settings": {"default_language": "English (en)"}}) + xlsx_io = self.asset.to_xlsx_io( + append={'settings': {'default_language': 'English (en)'}} + ) task_data = { 'file': xlsx_io, 'name': 'I was imported via XLS!', @@ -963,9 +965,9 @@ def test_import_xls_with_default_language_but_no_translations(self): def test_import_xls_with_default_language_not_in_translations(self): asset = Asset.objects.get(pk=2) - xlsx_io = asset.to_xlsx_io(append={ - "settings": {"default_language": "English (en)"} - }) + xlsx_io = asset.to_xlsx_io( + append={'settings': {'default_language': 'English (en)'}} + ) task_data = { 'file': xlsx_io, 'name': 'I was imported via XLS!', @@ -979,8 +981,8 @@ def test_import_xls_with_default_language_not_in_translations(self): self.assertEqual(detail_response.data['status'], 'error') self.assertTrue( detail_response.data['messages']['error'].startswith( - "`English (en)` is specified as the default language, " - "but only these translations are present in the form:" + '`English (en)` is specified as the default language, ' + 'but only these translations are present in the form:' ) ) @@ -1011,3 +1013,71 @@ def test_import_strip_newline_from_form_title_setting(self): ) expected_name = 'A project with a new line' assert asset_response.data['name'] == expected_name + + @responses.activate + def test_import_to_existing_asset_from_xls_url_records_audit_log_info(self): + self.asset.owner = self.user + self.asset.save() + # Host the XLS on a mock HTTP server + mock_xls_url = 'http://mock.kbtdev.org/form.xls' + responses.add( + responses.GET, + mock_xls_url, + content_type='application/xls', + body=self.asset.to_xlsx_io().read(), + ) + task_data = { + 'url': mock_xls_url, + 'name': 'I was imported via URL!', + 'destination': reverse( + 'api_v2:asset-detail', kwargs={'uid': self.asset.uid} + ), + } + post_url = reverse(self._get_endpoint('importtask-list')) + response = self.client.post(post_url, task_data) + task = ImportTask.objects.get(uid=response.data['uid']) + audit_logs = task.messages['audit_logs'] + self.assertEqual(len(audit_logs), 1) + audit_log_info = audit_logs[0] + self.assertDictEqual( + audit_log_info, + { + 'asset_uid': self.asset.uid, + 'asset_id': self.asset.id, + 'latest_version_uid': self.asset.latest_version.uid, + 'ip_address': task.data['ip_address'], + 'source': task.data['source'], + 'old_name': self.asset.name, + 'new_name': self.asset.name, + }, + ) + + def test_import_to_existing_asset_base64_xls_records_audit_log_info(self): + self.asset.owner = self.user + self.asset.save() + encoded_xls = base64.b64encode(self.asset.to_xlsx_io().read()) + task_data = { + 'base64Encoded': 'base64:{}'.format(to_str(encoded_xls)), + 'name': 'I was imported via base64-encoded XLS!', + 'destination': reverse( + 'api_v2:asset-detail', kwargs={'uid': self.asset.uid} + ), + } + post_url = reverse(self._get_endpoint('importtask-list')) + response = self.client.post(post_url, task_data) + task = ImportTask.objects.get(uid=response.data['uid']) + audit_logs = task.messages['audit_logs'] + self.assertEqual(len(audit_logs), 1) + audit_log_info = audit_logs[0] + self.assertDictEqual( + audit_log_info, + { + 'asset_uid': self.asset.uid, + 'asset_id': self.asset.id, + 'latest_version_uid': self.asset.latest_version.uid, + 'ip_address': task.data['ip_address'], + 'source': task.data['source'], + 'old_name': self.asset.name, + 'new_name': self.asset.name, + }, + ) diff --git a/kpi/views/v1/import_task.py b/kpi/views/v1/import_task.py index 7532dace2f..9c841f4c86 100644 --- a/kpi/views/v1/import_task.py +++ b/kpi/views/v1/import_task.py @@ -5,6 +5,10 @@ from rest_framework.response import Response from rest_framework.reverse import reverse +from kobo.apps.openrosa.libs.utils.viewer_tools import ( + get_client_ip, + get_human_readable_client_user_agent, +) from kpi.models import ImportTask from kpi.serializers import ImportTaskListSerializer, ImportTaskSerializer from kpi.tasks import import_in_background @@ -37,6 +41,8 @@ def create(self, request, *args, **kwargs): # NOTE: 'filename' here comes from 'name' (!) in the POST data 'filename': request.POST.get('name', None), 'destination': request.POST.get('destination', None), + 'ip_address': get_client_ip(request), + 'source': get_human_readable_client_user_agent(request), } if 'base64Encoded' in request.POST: encoded_str = request.POST['base64Encoded'] diff --git a/kpi/views/v2/import_task.py b/kpi/views/v2/import_task.py index 0c8024a853..58ec09f7a1 100644 --- a/kpi/views/v2/import_task.py +++ b/kpi/views/v2/import_task.py @@ -3,11 +3,17 @@ from rest_framework import exceptions, status, viewsets from rest_framework.response import Response -from rest_framework.request import Request from rest_framework.reverse import reverse +from kobo.apps.openrosa.libs.utils.viewer_tools import ( + get_client_ip, + get_human_readable_client_user_agent, +) from kpi.models import ImportTask -from kpi.serializers.v2.import_task import ImportTaskListSerializer, ImportTaskSerializer +from kpi.serializers.v2.import_task import ( + ImportTaskListSerializer, + ImportTaskSerializer, +) from kpi.tasks import import_in_background from kpi.utils.strings import to_str @@ -84,6 +90,8 @@ def create(self, request, *args, **kwargs): 'filename': request.POST.get('name', None), 'destination': request.POST.get('destination', None), 'desired_type': request.POST.get('desired_type', None), + 'ip_address': get_client_ip(request), + 'source': get_human_readable_client_user_agent(request), } if 'base64Encoded' in request.POST: encoded_str = request.POST['base64Encoded']