From 8d7c4672b02f25efcb40de0c7ced11f566104549 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Fri, 9 Aug 2019 14:45:21 -0400 Subject: [PATCH 01/13] Display file upload errors in form --- js/components/upload_input.js | 10 ++++++---- templates/components/upload_input.html | 10 +++++----- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/js/components/upload_input.js b/js/components/upload_input.js index 22a7c81c..4a6a057f 100644 --- a/js/components/upload_input.js +++ b/js/components/upload_input.js @@ -48,6 +48,7 @@ export default { attachment: this.initialData || null, showErrors: this.initialErrors, changed: false, + uploadError: null, } }, @@ -63,15 +64,16 @@ export default { methods: { addAttachment: async function(e) { const file = e.target.files[0] - try { - await this.uploader.upload(file, this.objectName) + + const response = await this.uploader.upload(file, this.objectName) + if (response.ok) { this.attachment = e.target.value this.showErrors = false this.$refs.attachmentFilename.value = file.name this.$refs.attachmentObjectName.value = this.objectName - } catch (err) { - console.log(err) + } else { this.showErrors = true + this.uploadError = 'Your file failed to upload. Please try again later.' } this.changed = true diff --git a/templates/components/upload_input.html b/templates/components/upload_input.html index 6e71d157..f98a9f6a 100644 --- a/templates/components/upload_input.html +++ b/templates/components/upload_input.html @@ -30,9 +30,6 @@ Browse - {% if field.errors %} - {{ Icon('alert',classes="icon-validation") }} - {% endif %} - {% for error, error_message in field.errors.items() %} - {{error_message}} + + {% for error, error_messages in field.errors.items() %} + {{error_messages[0]}}. {% endfor %} From 3ecb2cf84f69154b545cb215685bee3cd22e020e Mon Sep 17 00:00:00 2001 From: richard-dds Date: Fri, 9 Aug 2019 15:39:59 -0400 Subject: [PATCH 02/13] Update copy for file upload failure --- js/components/upload_input.js | 1 - templates/components/upload_input.html | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/js/components/upload_input.js b/js/components/upload_input.js index 4a6a057f..c387e057 100644 --- a/js/components/upload_input.js +++ b/js/components/upload_input.js @@ -68,7 +68,6 @@ export default { const response = await this.uploader.upload(file, this.objectName) if (response.ok) { this.attachment = e.target.value - this.showErrors = false this.$refs.attachmentFilename.value = file.name this.$refs.attachmentObjectName.value = this.objectName } else { diff --git a/templates/components/upload_input.html b/templates/components/upload_input.html index f98a9f6a..b18e4e9f 100644 --- a/templates/components/upload_input.html +++ b/templates/components/upload_input.html @@ -44,10 +44,10 @@ {% for error, error_messages in field.errors.items() %} - {{error_messages[0]}}. + {{error_messages[0]}} {% endfor %} From 71bb1be13098f1d6ab9627165acf8d525f94a864 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Fri, 9 Aug 2019 15:40:15 -0400 Subject: [PATCH 03/13] Validate filename and object_name for TO PDF upload --- atst/forms/task_order.py | 11 ++++++--- tests/routes/task_orders/test_new.py | 37 ++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/atst/forms/task_order.py b/atst/forms/task_order.py index c9c9e19b..cb1571b1 100644 --- a/atst/forms/task_order.py +++ b/atst/forms/task_order.py @@ -7,7 +7,7 @@ from wtforms.fields import ( HiddenField, ) from wtforms.fields.html5 import DateField -from wtforms.validators import Required, Optional +from wtforms.validators import Required, Optional, Length from flask_wtf import FlaskForm from .data import JEDI_CLIN_TYPES @@ -65,8 +65,13 @@ class CLINForm(FlaskForm): class AttachmentForm(BaseForm): - filename = HiddenField(id="attachment_filename") - object_name = HiddenField(id="attachment_object_name") + filename = HiddenField( + id="attachment_filename", + validators=[ + Length(max=100, message="Filename may be no longer than 100 characters.") + ], + ) + object_name = HiddenField(id="attachment_object_name", validators=[Length(max=40)]) accept = ".pdf,application/pdf" diff --git a/tests/routes/task_orders/test_new.py b/tests/routes/task_orders/test_new.py index 478baedf..29180c5c 100644 --- a/tests/routes/task_orders/test_new.py +++ b/tests/routes/task_orders/test_new.py @@ -7,6 +7,7 @@ from atst.models.task_order import Status as TaskOrderStatus from atst.models import TaskOrder from tests.factories import CLINFactory, PortfolioFactory, TaskOrderFactory, UserFactory +from tests.utils import captured_templates def build_pdf_form_data(filename="sample.pdf", object_name="object_name"): @@ -101,6 +102,42 @@ def test_task_orders_submit_form_step_one_add_pdf_delete_pdf( assert response.status_code == 302 +def test_task_orders_submit_form_step_one_validates_filename( + app, client, user_session, portfolio +): + user_session(portfolio.owner) + with captured_templates(app) as templates: + client.post( + url_for( + "task_orders.submit_form_step_one_add_pdf", portfolio_id=portfolio.id + ), + data={"pdf-filename": "a" * 1024}, + follow_redirects=True, + ) + + _, context = templates[-1] + + assert "filename" in context["form"].pdf.errors + + +def test_task_orders_submit_form_step_one_validates_object_name( + app, client, user_session, portfolio +): + user_session(portfolio.owner) + with captured_templates(app) as templates: + client.post( + url_for( + "task_orders.submit_form_step_one_add_pdf", portfolio_id=portfolio.id + ), + data={"pdf-object_name": "a" * 41}, + follow_redirects=True, + ) + + _, context = templates[-1] + + assert "object_name" in context["form"].pdf.errors + + def test_task_orders_form_step_two_add_number(client, user_session, task_order): user_session(task_order.creator) response = client.get( From bc0fd4900a58e05dd44b73bbf8bdc3591d7b5be5 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Fri, 9 Aug 2019 15:54:41 -0400 Subject: [PATCH 04/13] Prevent "there were some errors" from being flashed twice --- atst/forms/task_order.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/atst/forms/task_order.py b/atst/forms/task_order.py index cb1571b1..7c1aa84b 100644 --- a/atst/forms/task_order.py +++ b/atst/forms/task_order.py @@ -74,6 +74,9 @@ class AttachmentForm(BaseForm): object_name = HiddenField(id="attachment_object_name", validators=[Length(max=40)]) accept = ".pdf,application/pdf" + def validate(self, *args, **kwargs): + return super().validate(*args, **{**kwargs, "flash_invalid": False}) + class TaskOrderForm(BaseForm): number = StringField(label=translate("forms.task_order.number_description")) From 034645d0132e9a1b934d9c31a2920e06628dd3cb Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 13 Aug 2019 10:16:25 -0400 Subject: [PATCH 05/13] Fix seed script --- script/seed_sample.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/script/seed_sample.py b/script/seed_sample.py index 7bc9cb9d..8578a8d8 100644 --- a/script/seed_sample.py +++ b/script/seed_sample.py @@ -5,6 +5,7 @@ from datetime import timedelta, date, timedelta import random from faker import Faker from werkzeug.datastructures import FileStorage +from uuid import uuid4 parent_dir = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) sys.path.append(parent_dir) @@ -30,9 +31,9 @@ from atst.routes.dev import _DEV_USERS as DEV_USERS from tests.factories import ( random_service_branch, - random_task_order_number, TaskOrderFactory, CLINFactory, + AttachmentFactory ) fake = Faker() @@ -167,20 +168,19 @@ def add_task_orders_to_portfolio(portfolio): yesterday = today - timedelta(days=1) five_days = timedelta(days=5) - with open("tests/fixtures/sample.pdf", "rb") as fp: - pdf = FileStorage(fp, content_type="application/pdf") + pdf = {"filename": "sample_task_order.pdf", "object_name": str(uuid4())} - draft_to = TaskOrderFactory.build(portfolio=portfolio, pdf=None) - unsigned_to = TaskOrderFactory.build(portfolio=portfolio, pdf=pdf) - upcoming_to = TaskOrderFactory.build( - portfolio=portfolio, signed_at=yesterday, pdf=pdf - ) - expired_to = TaskOrderFactory.build( - portfolio=portfolio, signed_at=yesterday, pdf=pdf - ) - active_to = TaskOrderFactory.build( - portfolio=portfolio, signed_at=yesterday, pdf=pdf - ) + draft_to = TaskOrderFactory.build(portfolio=portfolio, pdf=None) + unsigned_to = TaskOrderFactory.build(portfolio=portfolio, pdf=pdf) + upcoming_to = TaskOrderFactory.build( + portfolio=portfolio, signed_at=yesterday, pdf=pdf + ) + expired_to = TaskOrderFactory.build( + portfolio=portfolio, signed_at=yesterday, pdf=pdf + ) + active_to = TaskOrderFactory.build( + portfolio=portfolio, signed_at=yesterday, pdf=pdf + ) clins = [ CLINFactory.build( From b48154f7380cbfd0f4926e527c2eeaf22404645b Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 13 Aug 2019 10:16:37 -0400 Subject: [PATCH 06/13] Fix MockUploader by returning ok response --- js/lib/upload.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/lib/upload.js b/js/lib/upload.js index f102c851..4a300b32 100644 --- a/js/lib/upload.js +++ b/js/lib/upload.js @@ -70,7 +70,7 @@ class MockUploader { } async upload(file, objectName) { - return Promise.resolve({}) + return Promise.resolve({ ok: true }) } } From ac7de6b4d2c1f2f7642914838c1659593ea2ded5 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 13 Aug 2019 13:49:49 -0400 Subject: [PATCH 07/13] Formatting --- script/seed_sample.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/seed_sample.py b/script/seed_sample.py index 8578a8d8..88986639 100644 --- a/script/seed_sample.py +++ b/script/seed_sample.py @@ -33,7 +33,7 @@ from tests.factories import ( random_service_branch, TaskOrderFactory, CLINFactory, - AttachmentFactory + AttachmentFactory, ) fake = Faker() From 6652d47104b840428e65c660ebb5094e58a57e4c Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 13 Aug 2019 13:58:36 -0400 Subject: [PATCH 08/13] Add custom message for object_name length validation --- atst/forms/task_order.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/atst/forms/task_order.py b/atst/forms/task_order.py index 7c1aa84b..db5c42e8 100644 --- a/atst/forms/task_order.py +++ b/atst/forms/task_order.py @@ -71,7 +71,12 @@ class AttachmentForm(BaseForm): Length(max=100, message="Filename may be no longer than 100 characters.") ], ) - object_name = HiddenField(id="attachment_object_name", validators=[Length(max=40)]) + object_name = HiddenField( + id="attachment_object_name", + validators=[ + Length(max=40, message="Object name may be no longer than 40 characters.") + ], + ) accept = ".pdf,application/pdf" def validate(self, *args, **kwargs): From 330c9ef3657a008505ea6b9a94beae6bf5cbc4a2 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 13 Aug 2019 14:08:53 -0400 Subject: [PATCH 09/13] Correctly set and unset uploadError --- js/components/upload_input.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/js/components/upload_input.js b/js/components/upload_input.js index c387e057..273c1d20 100644 --- a/js/components/upload_input.js +++ b/js/components/upload_input.js @@ -72,7 +72,7 @@ export default { this.$refs.attachmentObjectName.value = this.objectName } else { this.showErrors = true - this.uploadError = 'Your file failed to upload. Please try again later.' + this.uploadError = true } this.changed = true @@ -91,6 +91,7 @@ export default { this.$refs.attachmentInput.value = null } this.showErrors = false + this.uploadError = false this.changed = true emitEvent('field-change', this, { From 36d39dc949bd82c17c228baaf1304f4ecee16a1e Mon Sep 17 00:00:00 2001 From: richard-dds Date: Wed, 14 Aug 2019 10:39:58 -0400 Subject: [PATCH 10/13] Use unique attachment object names --- .secrets.baseline | 4 ++-- script/seed_sample.py | 13 +++++++------ tests/routes/task_orders/test_new.py | 5 +++-- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index 8926613d..717bf9b4 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -3,7 +3,7 @@ "files": null, "lines": null }, - "generated_at": "2019-08-13T09:59:21Z", + "generated_at": "2019-08-14T14:39:14Z", "plugins_used": [ { "base64_limit": 4.5, @@ -169,7 +169,7 @@ "hashed_secret": "e4f14805dfd1e6af030359090c535e149e6b4207", "is_secret": false, "is_verified": false, - "line_number": 30, + "line_number": 32, "type": "Hex High Entropy String" } ], diff --git a/script/seed_sample.py b/script/seed_sample.py index 88986639..c6c164a4 100644 --- a/script/seed_sample.py +++ b/script/seed_sample.py @@ -1,7 +1,7 @@ # Add root application dir to the python path import os import sys -from datetime import timedelta, date, timedelta +from datetime import timedelta, date import random from faker import Faker from werkzeug.datastructures import FileStorage @@ -168,18 +168,19 @@ def add_task_orders_to_portfolio(portfolio): yesterday = today - timedelta(days=1) five_days = timedelta(days=5) - pdf = {"filename": "sample_task_order.pdf", "object_name": str(uuid4())} + def build_pdf(): + return {"filename": "sample_task_order.pdf", "object_name": str(uuid4())} draft_to = TaskOrderFactory.build(portfolio=portfolio, pdf=None) - unsigned_to = TaskOrderFactory.build(portfolio=portfolio, pdf=pdf) + unsigned_to = TaskOrderFactory.build(portfolio=portfolio, pdf=build_pdf()) upcoming_to = TaskOrderFactory.build( - portfolio=portfolio, signed_at=yesterday, pdf=pdf + portfolio=portfolio, signed_at=yesterday, pdf=build_pdf() ) expired_to = TaskOrderFactory.build( - portfolio=portfolio, signed_at=yesterday, pdf=pdf + portfolio=portfolio, signed_at=yesterday, pdf=build_pdf() ) active_to = TaskOrderFactory.build( - portfolio=portfolio, signed_at=yesterday, pdf=pdf + portfolio=portfolio, signed_at=yesterday, pdf=build_pdf() ) clins = [ diff --git a/tests/routes/task_orders/test_new.py b/tests/routes/task_orders/test_new.py index 29180c5c..bdc3a76d 100644 --- a/tests/routes/task_orders/test_new.py +++ b/tests/routes/task_orders/test_new.py @@ -1,6 +1,7 @@ import pytest from flask import url_for, get_flashed_messages from datetime import timedelta, date +from uuid import uuid4 from atst.domain.task_orders import TaskOrders from atst.models.task_order import Status as TaskOrderStatus @@ -10,8 +11,8 @@ from tests.factories import CLINFactory, PortfolioFactory, TaskOrderFactory, Use from tests.utils import captured_templates -def build_pdf_form_data(filename="sample.pdf", object_name="object_name"): - return {"pdf-filename": filename, "pdf-object_name": object_name} +def build_pdf_form_data(filename="sample.pdf", object_name=None): + return {"pdf-filename": filename, "pdf-object_name": object_name or uuid4()} @pytest.fixture From a7417b4f3941e1f4189e9fa73ce846f28d6cabd1 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Wed, 14 Aug 2019 11:35:07 -0400 Subject: [PATCH 11/13] Add upload errors to translations file --- atst/forms/task_order.py | 4 ++-- templates/components/upload_input.html | 2 +- translations.yaml | 6 ++++++ 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/atst/forms/task_order.py b/atst/forms/task_order.py index db5c42e8..6422aee7 100644 --- a/atst/forms/task_order.py +++ b/atst/forms/task_order.py @@ -68,13 +68,13 @@ class AttachmentForm(BaseForm): filename = HiddenField( id="attachment_filename", validators=[ - Length(max=100, message="Filename may be no longer than 100 characters.") + Length(max=100, message=translate("forms.attachment.filename.length_error")) ], ) object_name = HiddenField( id="attachment_object_name", validators=[ - Length(max=40, message="Object name may be no longer than 40 characters.") + Length(max=40, message=translate("forms.attachment.object_name.length_error")) ], ) accept = ".pdf,application/pdf" diff --git a/templates/components/upload_input.html b/templates/components/upload_input.html index b18e4e9f..a56cb0bf 100644 --- a/templates/components/upload_input.html +++ b/templates/components/upload_input.html @@ -44,7 +44,7 @@ {% for error, error_messages in field.errors.items() %} {{error_messages[0]}} diff --git a/translations.yaml b/translations.yaml index fd97ae8f..2a50ab80 100644 --- a/translations.yaml +++ b/translations.yaml @@ -145,7 +145,13 @@ forms: portfolio: name_label: Portfolio name name_length_validation_message: Portfolio names can be between 4-100 characters + attachment: + object_name: + length_error: Object name may be no longer than 40 characters. + filename: + length_error: Filename may be no longer than 100 characters. task_order: + upload_error: There was an error uploading your file. Please try again. If you encounter repeated problems uploading this file, please contact CCPO. app_migration: both: 'Yes, migrating from both an on-premise data center and another cloud provider' cloud: 'Yes, migrating from another cloud provider' From 56cec0694f51e8a009b7d7c5917488fb29a46a26 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Wed, 14 Aug 2019 11:40:23 -0400 Subject: [PATCH 12/13] Formatting --- atst/forms/task_order.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/atst/forms/task_order.py b/atst/forms/task_order.py index 6422aee7..47323319 100644 --- a/atst/forms/task_order.py +++ b/atst/forms/task_order.py @@ -74,7 +74,9 @@ class AttachmentForm(BaseForm): object_name = HiddenField( id="attachment_object_name", validators=[ - Length(max=40, message=translate("forms.attachment.object_name.length_error")) + Length( + max=40, message=translate("forms.attachment.object_name.length_error") + ) ], ) accept = ".pdf,application/pdf" From 51272922bb0cdfadf0ea628cfd87d390ec274856 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Wed, 14 Aug 2019 11:54:56 -0400 Subject: [PATCH 13/13] Update Vue test templates --- js/test_templates/upload_input_error_template.html | 13 ++++--------- js/test_templates/upload_input_template.html | 4 +++- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/js/test_templates/upload_input_error_template.html b/js/test_templates/upload_input_error_template.html index efb83e29..02256f0d 100644 --- a/js/test_templates/upload_input_error_template.html +++ b/js/test_templates/upload_input_error_template.html @@ -25,14 +25,6 @@ Browse - - - - - + - ['Test Error Message'] + Test Error Message diff --git a/js/test_templates/upload_input_template.html b/js/test_templates/upload_input_template.html index 37412a66..70c73407 100644 --- a/js/test_templates/upload_input_template.html +++ b/js/test_templates/upload_input_template.html @@ -25,7 +25,6 @@ Browse - +