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

Capture attachment file names whose name exceeds 100 chars #2003

Merged
merged 2 commits into from
Feb 22, 2021

Conversation

WinnyTroy
Copy link
Contributor

Changes / Features implemented

  • Include check to confirm attachment file name length before saving submission

Steps taken to verify this change does what is intended

  • Added Tests

Closes #1939

DavisRayM
DavisRayM previously approved these changes Jan 25, 2021
Copy link
Contributor

@ivermac ivermac left a comment

Choose a reason for hiding this comment

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

It generally looks good. I have a question though

@@ -336,6 +336,10 @@ def save_attachments(xform, instance, media_files, remove_deleted_media=False):
xform.instances_with_osm = True
xform.save()
filename = os.path.basename(f.name)
# Validate Attachment file name length
if len(filename) > 99:
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on the error message, should the number in the condition be 100 instead of 99? At the moment, if the size of the filename is 100, the error message will be triggerd, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made the change

ivermac
ivermac previously approved these changes Jan 29, 2021
Copy link
Contributor

@ivermac ivermac left a comment

Choose a reason for hiding this comment

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

lgtm

@WinnyTroy WinnyTroy force-pushed the user_friendly_attachment_error_name branch 2 times, most recently from db9e897 to 55e6720 Compare February 3, 2021 08:42
Comment on lines 341 to 342
raise AttachmentNameError(
_('Attachment file name exceeds 100 characters'))
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's ensure this exception is caught and an appropriate status code is returned with the reason.

@WinnyTroy WinnyTroy force-pushed the user_friendly_attachment_error_name branch from 55e6720 to 055d7c5 Compare February 3, 2021 13:50
@WinnyTroy WinnyTroy force-pushed the user_friendly_attachment_error_name branch from 055d7c5 to f068dfa Compare February 19, 2021 09:02
@WinnyTroy WinnyTroy force-pushed the user_friendly_attachment_error_name branch from f068dfa to f0fb07d Compare February 19, 2021 09:34
@DavisRayM DavisRayM merged commit 3d6b987 into master Feb 22, 2021
@DavisRayM DavisRayM deleted the user_friendly_attachment_error_name branch February 22, 2021 06:57
@DavisRayM DavisRayM mentioned this pull request Feb 23, 2021
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Return user friendly warning when an attachment name exceeds 100 characters
3 participants