Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(accessLogExport)!: create new AccessLogExportTask to generate a csv of access logs TASK-871 #5258

Merged
merged 17 commits into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion jsapp/js/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ export const META_QUESTION_TYPES = createEnum([
// 1. https://github.com/kobotoolbox/kobocat/blob/78133d519f7b7674636c871e3ba5670cd64a7227/onadata/apps/viewer/models/parsed_instance.py#L242-L260
// 2. https://github.com/kobotoolbox/kpi/blob/7db39015866c905edc645677d72b9c1ea16067b1/jsapp/js/constants.es6#L284-L294
export const ADDITIONAL_SUBMISSION_PROPS = createEnum([
// match the ordering of (Python) kpi.models.import_export_task.ExportTask.COPY_FIELDS
// match the ordering of (Python) kpi.models.import_export_task.SubmissionExportTask.COPY_FIELDS
'_id',
'_uuid',
'_submission_time',
Expand Down
4 changes: 2 additions & 2 deletions kobo/apps/audit_log/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ def create_from_request(cls, request):
'asset-file-detail': cls.create_from_file_request,
'asset-file-list': cls.create_from_file_request,
'asset-export-list': cls.create_from_export_request,
'exporttask-list': cls.create_from_v1_export,
'submissionexporttask-list': cls.create_from_v1_export,
}
url_name = request.resolver_match.url_name
method = url_name_to_action.get(url_name, None)
Expand Down Expand Up @@ -567,7 +567,7 @@ def create_from_related_request(
action = modify_action
if action:
# some actions on related objects do not need to be logged,
# eg deleting an ExportTask
# eg deleting an SubmissionExportTask
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# eg deleting an SubmissionExportTask
# eg deleting a SubmissionExportTask

nit, definitely non-blocking

ProjectHistoryLog.objects.create(
user=request.user, object_id=object_id, action=action, metadata=metadata
)
Expand Down
2 changes: 1 addition & 1 deletion kobo/apps/audit_log/tests/test_project_history_logs.py
Original file line number Diff line number Diff line change
Expand Up @@ -851,7 +851,7 @@ def test_export_v1_creates_log(self):
# can't use _base_project_history_log_test because
# the old endpoint doesn't like format=json
self.client.post(
path=reverse('exporttask-list'),
path=reverse('submissionexporttask-list'),
data=request_data,
)

Expand Down
4 changes: 2 additions & 2 deletions kobo/apps/trash_bin/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from kobo.apps.audit_log.audit_actions import AuditAction
from kobo.apps.audit_log.models import AuditLog, AuditType
from kpi.exceptions import InvalidXFormException, MissingXFormException
from kpi.models import Asset, ExportTask, ImportTask
from kpi.models import Asset, SubmissionExportTask, ImportTask
from kpi.utils.mongo_helper import MongoHelper
from kpi.utils.storage import rmdir
from .constants import DELETE_PROJECT_STR_PREFIX, DELETE_USER_STR_PREFIX
Expand All @@ -45,7 +45,7 @@ def delete_asset(request_author: settings.AUTH_USER_MODEL, asset: 'kpi.Asset'):
if asset.has_deployment:
_delete_submissions(request_author, asset)
asset.deployment.delete()
project_exports = ExportTask.objects.filter(
project_exports = SubmissionExportTask.objects.filter(
Q(data__source=f'{host}/api/v2/assets/{asset.uid}/')
| Q(data__source=f'{host}/assets/{asset.uid}/')
)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
# Generated by Django 4.2.15 on 2024-11-05 15:47

import django.db.models.deletion
import private_storage.fields
import private_storage.storage.files
from django.conf import settings
from django.db import migrations, models

import kpi.fields.file
import kpi.fields.kpi_uid
import kpi.models.asset_file
import kpi.models.import_export_task


def populate_common_export_tasks(apps, schema_editor):
CommonExportTask = apps.get_model('kpi', 'CommonExportTask')
ProjectViewExportTask = apps.get_model('kpi', 'ProjectViewExportTask')
for project_view_task in ProjectViewExportTask.objects.all():
common_task = CommonExportTask.objects.create(
data=project_view_task.data,
messages=project_view_task.messages,
status=project_view_task.status,
date_created=project_view_task.date_created,
result=project_view_task.result,
user=project_view_task.user,
)
project_view_task.commonexporttask_ptr = common_task
project_view_task.save()


class Migration(migrations.Migration):

dependencies = [
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
('kpi', '0059_assetexportsettings_date_created_and_more'),
]

operations = [
migrations.CreateModel(
name='CommonExportTask',
fields=[
(
'id',
models.AutoField(
auto_created=True,
primary_key=True,
serialize=False,
verbose_name='ID',
),
),
('data', models.JSONField()),
('messages', models.JSONField(default=dict)),
(
'status',
models.CharField(
choices=[
('created', 'created'),
('processing', 'processing'),
('error', 'error'),
('complete', 'complete'),
],
default='created',
max_length=32,
),
),
('date_created', models.DateTimeField(auto_now_add=True)),
(
'result',
private_storage.fields.PrivateFileField(
max_length=380,
storage=(
private_storage.storage.files.PrivateFileSystemStorage()
),
upload_to=kpi.models.import_export_task.export_upload_to,
),
),
(
'user',
models.ForeignKey(
on_delete=django.db.models.deletion.CASCADE,
to=settings.AUTH_USER_MODEL,
),
),
],
options={
'abstract': False,
},
),
migrations.RunPython(populate_common_export_tasks),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot: if we end up keeping the migration, this needs to be reversible, even if the reverse is RunPython.noop. Otherwise unapplying migrations breaks

migrations.AddField(
model_name='projectviewexporttask',
name='commonexporttask_ptr',
field=models.OneToOneField(
null=True,
on_delete=django.db.models.deletion.CASCADE,
parent_link=True,
to='kpi.CommonExportTask',
),
preserve_default=False,
),
migrations.RemoveField(
model_name='projectviewexporttask',
name='data',
),
migrations.RemoveField(
model_name='projectviewexporttask',
name='date_created',
),
migrations.RemoveField(
model_name='projectviewexporttask',
name='id',
),
migrations.RemoveField(
model_name='projectviewexporttask',
name='messages',
),
migrations.RemoveField(
model_name='projectviewexporttask',
name='result',
),
migrations.RemoveField(
model_name='projectviewexporttask',
name='status',
),
migrations.RemoveField(
model_name='projectviewexporttask',
name='user',
),
migrations.AlterField(
model_name='assetfile',
name='content',
field=kpi.fields.file.PrivateExtendedFileField(
max_length=380, null=True, upload_to=kpi.models.asset_file.upload_to
),
),
migrations.CreateModel(
name='AccessLogExportTask',
fields=[
(
'commonexporttask_ptr',
models.OneToOneField(
auto_created=True,
on_delete=django.db.models.deletion.CASCADE,
parent_link=True,
primary_key=True,
serialize=False,
to='kpi.commonexporttask',
),
),
('uid', kpi.fields.kpi_uid.KpiUidField(_null=False, uid_prefix='ale')),
('get_all_logs', models.BooleanField(default=False)),
],
options={
'abstract': False,
},
bases=('kpi.commonexporttask',),
),
]

migrations.AddField(
model_name='projectviewexporttask',
name='commonexporttask_ptr',
field=models.OneToOneField(
null=True,
on_delete=django.db.models.deletion.CASCADE,
parent_link=True,
to='kpi.CommonExportTask',
),
),
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Generated by Django 4.2.15 on 2024-11-14 22:46

from django.conf import settings
from django.db import migrations


class Migration(migrations.Migration):

dependencies = [
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
('kpi', '0060_commonexporttask_remove_projectviewexporttask_data_and_more'),
]

operations = [
migrations.RenameModel(
old_name='SynchronousExport',
new_name='SubmissionSynchronousExport',
),
]
19 changes: 19 additions & 0 deletions kpi/migrations/0062_rename_exporttask_submissionexporttask.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Generated by Django 4.2.15 on 2024-11-14 22:50

from django.conf import settings
from django.db import migrations


class Migration(migrations.Migration):

dependencies = [
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
('kpi', '0061_rename_synchronousexport_submissionsynchronousexport'),
]

operations = [
migrations.RenameModel(
old_name='ExportTask',
new_name='SubmissionExportTask',
),
]
6 changes: 3 additions & 3 deletions kpi/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@
from .asset_snapshot import AssetSnapshot
from .asset_user_partial_permission import AssetUserPartialPermission
from .object_permission import ObjectPermission
from .import_export_task import (
ExportTask,
from .import_export_task import ( # noqa F401
SubmissionExportTask,
ImportTask,
ProjectViewExportTask,
SynchronousExport,
SubmissionSynchronousExport,
)
from .tag_uid import TagUid
from .authorized_application import AuthorizedApplication
Expand Down
52 changes: 36 additions & 16 deletions kpi/models/import_export_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@
from kpi.exceptions import XlsFormatException
from kpi.fields import KpiUidField
from kpi.models import Asset
from kpi.utils.data_exports import create_data_export
from kpi.utils.log import logging
from kpi.utils.models import _load_library_content, create_assets, resolve_url_to_asset
from kpi.utils.project_view_exports import create_project_view_export
from kpi.utils.rename_xls_sheet import (
ConflictSheetError,
NoFromSheetError,
Expand Down Expand Up @@ -134,7 +134,7 @@ def run(self):
# This method must be implemented by a subclass
self._run_task(msgs)
self.status = self.COMPLETE
except ExportTaskBase.InaccessibleData as e:
except SubmissionExportTaskBase.InaccessibleData as e:
msgs['error_type'] = t('Cannot access data')
msgs['error'] = str(e)
self.status = self.ERROR
Expand Down Expand Up @@ -482,27 +482,23 @@ def export_upload_to(self, filename):
return posixpath.join(self.user.username, 'exports', filename)


class ProjectViewExportTask(ImportExportTask):
uid = KpiUidField(uid_prefix='pve')
class CommonExportTask(ImportExportTask):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should have caught this before, but I think we may be able to avoid the complicated migration if we make this a mixin rather than a separate model.

result = PrivateFileField(upload_to=export_upload_to, max_length=380)

def _get_export_details(self) -> tuple:
return self.data['type'], self.data['view']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should account for data not having a view key since we don't need one for access logs exports. the filename can just be export_type-username-time for tasks without a view.


def _build_export_filename(
self, export_type: str, username: str, view: str
) -> str:
time = datetime.datetime.now().strftime('%Y-%m-%dT%H:%M:%SZ')
return f'{export_type}-{username}-view_{view}-{time}.csv'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as previous comment. This should be updated to allow an empty/None view parameter


def _run_task(self, messages: list) -> None:
export_type = self.data['type']
view = self.data['view']

filename = self._build_export_filename(
export_type, self.user.username, view
)
def _run_task_base(self, messages: list, buff) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def _run_task_base(self, messages: list, buff) -> None:
def _export_data_to_file(self, messages: list, buff) -> None:

since it's no longer doing the whole task

export_type, view = self._get_export_details()
filename = self._build_export_filename(export_type, self.user.username, view)
absolute_filepath = self.get_absolute_filepath(filename)

buff = create_project_view_export(export_type, self.user.username, view)

with self.result.storage.open(absolute_filepath, 'wb') as output_file:
output_file.write(buff.read().encode())

Expand All @@ -515,7 +511,31 @@ def delete(self, *args, **kwargs) -> None:
super().delete(*args, **kwargs)


class ExportTaskBase(ImportExportTask):
class AccessLogExportTask(CommonExportTask):
uid = KpiUidField(uid_prefix='ale')
get_all_logs = models.BooleanField(default=False)

def _run_task(self, messages: list) -> None:
if self.get_all_logs and not self.user.is_superuser:
raise PermissionError('Only superusers can export all access logs.')

export_type, view = self._get_export_details()
buff = create_data_export(
export_type, self.user.username, self.uid, self.get_all_logs
)
self._run_task_base(messages, buff)


class ProjectViewExportTask(CommonExportTask):
uid = KpiUidField(uid_prefix='pve')

def _run_task(self, messages: list) -> None:
export_type, view = self._get_export_details()
buff = create_data_export(export_type, self.user.username, view, False)
self._run_task_base(messages, buff)


class SubmissionExportTaskBase(ImportExportTask):
"""
An (asynchronous) submission data export job. The instantiator must set the
`data` attribute to a dictionary with the following keys:
Expand Down Expand Up @@ -945,7 +965,7 @@ def remove_excess(cls, user, source):
export.delete()


class ExportTask(ExportTaskBase):
class SubmissionExportTask(SubmissionExportTaskBase):
"""
An asynchronous export task, to be run with Celery
"""
Expand All @@ -966,7 +986,7 @@ def _run_task(self, messages):
self.remove_excess(self.user, source_url)


class SynchronousExport(ExportTaskBase):
class SubmissionSynchronousExport(SubmissionExportTaskBase):
"""
A synchronous export, with significant limitations on processing time, but
offered for user convenience
Expand Down
Loading
Loading