Skip to content

Commit

Permalink
Update logic to handle edit submissions with identical attachment nam…
Browse files Browse the repository at this point in the history
…es but different content
  • Loading branch information
rajpatel24 committed Sep 23, 2024
1 parent cc37b6c commit cdc71db
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,7 @@
<meta>
<instanceID>uuid:729f173c688e482486a48661700455ff</instanceID>
</meta>
<formhub>
<uuid>c2f940a2f1e8490d8d3c3030452ed401</uuid>
</formhub>
</tutorial>
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?xml version='1.0' ?>
<tutorial id="tutorial">
<name>Larry
Again
</name><!-- newline to make sure we can handle it -->
<age>23</age>
<picture>1335783522563.jpg</picture>
<has_children>0</has_children>
<gps>-1.2836198 36.8795437 0.0 1044.0</gps>
<web_browsers>firefox chrome safari</web_browsers>
<meta>
<instanceID>uuid:031c585a-88b7-4962-a566-849a7dd15891</instanceID>
<deprecatedID>uuid:729f173c688e482486a48661700455ff</deprecatedID>
</meta>
<formhub>
<uuid>c2f940a2f1e8490d8d3c3030452ed401</uuid>
</formhub>
</tutorial>
57 changes: 57 additions & 0 deletions kobo/apps/openrosa/apps/logger/tests/test_form_submission.py
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,63 @@ def test_duplicate_submission_with_same_content_but_with_different_attachment(se
Attachment.objects.filter(instance=initial_instance).count(), 1
)

def test_edit_submission_with_same_attachment_name_but_different_content(self):
"""
Test editing a submission with an attachment with the same name
"""
xml_submission_file_path = os.path.join(
os.path.dirname(__file__),
'../fixtures/tutorial/instances/tutorial_with_attachment',
'tutorial_2012-06-27_11-27-53_w_attachment.xml',
)
xml_edit_submission_file_path = os.path.join(
os.path.dirname(__file__),
'../fixtures/tutorial/instances/tutorial_with_attachment',
'tutorial_2012-06-27_11-27-53_w_attachment_edit.xml',
)
media_file_path1 = os.path.join(
os.path.dirname(__file__),
'../fixtures/tutorial/instances/tutorial_with_attachment',
'1335783522563.jpg',
)
media_file_path2 = os.path.join(
os.path.dirname(__file__),
'../fixtures/tutorial/instances/tutorial_with_attachment/'
'attachment_with_different_content',
'1335783522563.jpg',
)
initial_instance_count = Instance.objects.count()

# Test submission with attachment
with open(media_file_path1, 'rb') as media_file:
self._make_submission(
xml_submission_file_path, media_file=media_file
)
initial_instance = Instance.objects.last()
attachment_basename = Attachment.objects.last().media_file_basename
attachment_hash = Attachment.objects.last().file_hash
self.assertEqual(self.response.status_code, 201)
self.assertEqual(Instance.objects.count(), initial_instance_count + 1)
self.assertEqual(
Attachment.objects.filter(instance=initial_instance).count(), 1
)

# Test edit submission with same attachment name but different content
with open(media_file_path2, 'rb') as media_file:
self._make_submission(
xml_edit_submission_file_path, media_file=media_file
)
initial_instance = Instance.objects.last()
edit_attachment_basename = Attachment.objects.last().media_file_basename
edit_attachment_hash = Attachment.objects.last().file_hash
self.assertEqual(self.response.status_code, 201)
self.assertEqual(Instance.objects.count(), initial_instance_count + 1)
self.assertEqual(
Attachment.objects.filter(instance=initial_instance).count(), 1
)
self.assertEqual(attachment_basename, edit_attachment_basename)
self.assertNotEqual(attachment_hash, edit_attachment_hash)

def test_owner_can_edit_submissions(self):
xml_submission_file_path = os.path.join(
os.path.dirname(os.path.abspath(__file__)),
Expand Down
15 changes: 11 additions & 4 deletions kobo/apps/openrosa/libs/utils/logger_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -811,16 +811,23 @@ def get_soft_deleted_attachments(instance: Instance) -> list[Attachment]:
# Update Attachment objects to hide them if they are not used anymore.
# We do not want to delete them until the instance itself is deleted.

# If the new attachment has the same basename as an existing one but
# different content, update the existing one.

# FIXME Temporary hack to leave background-audio files and audit files alone
# Bug comes from `get_xform_media_question_xpaths()`
queryset = Attachment.objects.filter(instance=instance).exclude(
Q(media_file_basename__in=basenames)
| Q(media_file_basename__endswith='.enc')
Q(media_file_basename__endswith='.enc')
| Q(media_file_basename='audit.csv')
| Q(media_file_basename__regex=r'^\d{10,}\.(m4a|amr)$')
).order_by('-id')

latest_attachments = queryset[:len(basenames)]
remaining_attachments = queryset.exclude(
id__in=latest_attachments.values_list('id', flat=True)
)
soft_deleted_attachments = list(queryset.all())
queryset.update(deleted_at=dj_timezone.now())
soft_deleted_attachments = list(remaining_attachments.all())
remaining_attachments.update(deleted_at=dj_timezone.now())

return soft_deleted_attachments

Expand Down

0 comments on commit cdc71db

Please sign in to comment.