Skip to content

Commit

Permalink
Merge pull request #4978 from kobotoolbox/fix-mismatched-labels-and-t…
Browse files Browse the repository at this point in the history
…ranslations-export-failure

Fix "mismatched labels and translations" export failure
  • Loading branch information
jnm committed Jun 17, 2024
2 parents 8e14964 + a4dfe91 commit be33572
Show file tree
Hide file tree
Showing 8 changed files with 91 additions and 19 deletions.
11 changes: 1 addition & 10 deletions dependencies/pip/dev_requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
# via -r dependencies/pip/requirements.in
-e git+https://github.com/trevoriancox/django-dont-vary-on.git@01a804122b7ddcdc22f50b40993f91c27b03bef6#egg=django-dont-vary-on
# via -r dependencies/pip/requirements.in
-e git+https://github.com/kobotoolbox/formpack.git@443a8e940756976a9f88fb577dbbc53510726536#egg=formpack
-e git+https://github.com/kobotoolbox/formpack.git@451df4cd2a0d614be69a3b3309259c67369f7efb#egg=formpack
# via -r dependencies/pip/requirements.in
-e git+https://github.com/kobotoolbox/kobo-service-account.git@8ee76730106ff8dc0ee2539c8e6a567aea4ed9ee#egg=kobo-service-account
# via -r dependencies/pip/requirements.in
Expand Down Expand Up @@ -530,15 +530,6 @@ six==1.16.0
# via
# asttokens
# azure-core
# bcrypt
# click-repl
# django-organizations
# google-auth
# google-auth-httplib2
# grpcio
# mongomock
# paramiko
# pathlib2
# isodate
# python-dateutil
smsapi-client==2.9.5
Expand Down
4 changes: 2 additions & 2 deletions dependencies/pip/requirements.in
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# https://github.com/bndr/pipreqs is a handy utility, too.

# formpack
-e git+https://github.com/kobotoolbox/formpack.git@443a8e940756976a9f88fb577dbbc53510726536#egg=formpack
-e git+https://github.com/kobotoolbox/formpack.git@451df4cd2a0d614be69a3b3309259c67369f7efb#egg=formpack

# service-account
-e git+https://github.com/kobotoolbox/kobo-service-account.git@8ee76730106ff8dc0ee2539c8e6a567aea4ed9ee#egg=kobo-service-account
Expand Down Expand Up @@ -30,7 +30,7 @@ celery[redis]
dict2xml
dj-static
dj-stripe
django-allauth<0.58 # Backwards incompatible changes
django-allauth
django-braces
django-celery-beat
django-constance
Expand Down
7 changes: 1 addition & 6 deletions dependencies/pip/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
# via -r dependencies/pip/requirements.in
-e git+https://github.com/trevoriancox/django-dont-vary-on.git@01a804122b7ddcdc22f50b40993f91c27b03bef6#egg=django-dont-vary-on
# via -r dependencies/pip/requirements.in
-e git+https://github.com/kobotoolbox/formpack.git@443a8e940756976a9f88fb577dbbc53510726536#egg=formpack
-e git+https://github.com/kobotoolbox/formpack.git@451df4cd2a0d614be69a3b3309259c67369f7efb#egg=formpack
# via -r dependencies/pip/requirements.in
-e git+https://github.com/kobotoolbox/kobo-service-account.git@8ee76730106ff8dc0ee2539c8e6a567aea4ed9ee#egg=kobo-service-account
# via -r dependencies/pip/requirements.in
Expand Down Expand Up @@ -443,11 +443,6 @@ shortuuid==1.0.13
six==1.16.0
# via
# azure-core
# click-repl
# django-organizations
# google-auth
# google-auth-httplib2
# grpcio
# isodate
# python-dateutil
smsapi-client==2.9.5
Expand Down
5 changes: 5 additions & 0 deletions kobo/apps/reports/report_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from rest_framework import serializers
from formpack import FormPack

from kpi.utils.bugfix import repair_file_column_content_and_save
from kpi.utils.log import logging
from .constants import (
FUZZY_VERSION_ID_KEY,
Expand All @@ -21,6 +22,10 @@ def build_formpack(asset, submission_stream=None, use_all_form_versions=True):
are assumed to have been collected with that version of the form.
"""

# Cope with kobotoolbox/formpack#322, which wrote invalid content into the
# database
repair_file_column_content_and_save(asset)

if asset.has_deployment:
if use_all_form_versions:
_versions = asset.deployed_versions
Expand Down
2 changes: 2 additions & 0 deletions kpi/models/asset_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ class AssetVersion(models.Model):
)
version_content = models.JSONField()
uid_aliases = models.JSONField(null=True)
# Tee hee, `deployed_content` is never written to the database!
# TODO: It should be changed to a property instead, no?
deployed_content = models.JSONField(null=True)
_deployment_data = models.JSONField(default=dict)
deployed = models.BooleanField(default=False)
Expand Down
1 change: 1 addition & 0 deletions kpi/serializers/v2/asset.py
Original file line number Diff line number Diff line change
Expand Up @@ -893,6 +893,7 @@ def validate_settings(self, settings: dict) -> dict:
return {**self.instance.settings, **settings}

def _content(self, obj):
# FIXME: Is this dead code?
return json.dumps(obj.content)

def _get_status(self, perm_assignments):
Expand Down
67 changes: 67 additions & 0 deletions kpi/utils/bugfix.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# coding: utf-8

import datetime

from django.db import transaction
from formpack.utils.bugfix import repair_file_column_content_in_place

from kpi.models import Asset, AssetVersion


def repair_file_column_content_and_save(asset, include_versions=True) -> bool:
"""
Given an `Asset`, repair any damage caused to its `content` and the
`version_content` of all related `AssetVersion`s by
kobotoolbox/formpack#322, which wrongly transformed the `file` column into
`media::file` and included it in the list of translated columns.
Writes only to the `content` (or `version_content`) field for each modified
model instance.
Returns `True` if any change was made
"""

# When was this bug introduced? See formpack commit
# 443a8e940756976a9f88fb577dbbc53510726536
BAD_TIME = datetime.datetime(
2024, 4, 30, 15, 27, 2, tzinfo=datetime.timezone.utc
)

any_change = False
with transaction.atomic():
# Lock and refetch the latest content to guard against clobbering
# concurrent updates
content = (
Asset.objects.filter(pk=asset.pk, date_modified__gte=BAD_TIME)
.select_for_update()
.values('content')
.first()
)
if not content:
# This asset was not modified recently enough to be affected by
# this bug
return False

if repair_file_column_content_in_place(content):
Asset.objects.filter(pk=asset.pk).update(content=content)
any_change = True
# Also update the in-memory asset instance passed to this function
asset.content = content

if not include_versions:
return any_change

# Previous versions of the content may need repair, regardless of
# whether or not the current content did
for pk, version_content in (
asset.asset_versions.filter(date_modified__gte=BAD_TIME)
.select_for_update()
.values_list('pk', 'version_content')
):
if repair_file_column_content_in_place(version_content):
AssetVersion.objects.filter(pk=pk).update(
version_content=version_content
)
any_change = True

return any_change
13 changes: 12 additions & 1 deletion kpi/views/v2/asset.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,9 @@
AssetListSerializer,
AssetSerializer,
)
from kpi.utils.hash import calculate_hash
from kpi.serializers.v2.reports import ReportsDetailSerializer
from kpi.utils.bugfix import repair_file_column_content_and_save
from kpi.utils.hash import calculate_hash
from kpi.utils.kobo_to_xlsform import to_xlsform_structure
from kpi.utils.ss_structure_to_mdtable import ss_structure_to_mdtable
from kpi.utils.object_permission import (
Expand Down Expand Up @@ -403,6 +404,16 @@ def get_object(self):
raise Http404

self.check_object_permissions(self.request, asset)

# Cope with kobotoolbox/formpack#322, which wrote invalid content
# into the database. For performance, consider only the current
# content, not previous versions. Previous versions are handled in
# `kobo.apps.reports.report_data.build_formpack()`
if self.request.method == 'GET':
repair_file_column_content_and_save(
asset, include_versions=False
)

return asset

return super().get_object()
Expand Down

0 comments on commit be33572

Please sign in to comment.