Merge pull request #1019 from dod-ccpo/upload-error-handling

Add validations and error states for TO upload form

.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"
       }
     ],

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,10 +65,25 @@ class CLINForm(FlaskForm):
 
 
 class AttachmentForm(BaseForm):
-    filename = HiddenField(id="attachment_filename")
+    filename = HiddenField(
-    object_name = HiddenField(id="attachment_object_name")
+        id="attachment_filename",
+        validators=[
+            Length(max=100, message=translate("forms.attachment.filename.length_error"))
+        ],
+    )
+    object_name = HiddenField(
+        id="attachment_object_name",
+        validators=[
+            Length(
+                max=40, message=translate("forms.attachment.object_name.length_error")
+            )
+        ],
+    )
     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"))

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,15 @@ 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) {
+      } else {
-        console.log(err)
         this.showErrors = true
+        this.uploadError = true
       }
 
       this.changed = true
@@ -90,6 +91,7 @@ export default {
         this.$refs.attachmentInput.value = null
       }
       this.showErrors = false
+      this.uploadError = false
       this.changed = true
 
       emitEvent('field-change', this, {

js/lib/upload.js

@@ -70,7 +70,7 @@ class MockUploader {
   }
 
   async upload(file, objectName) {
-    return Promise.resolve({})
+    return Promise.resolve({ ok: true })
   }
 }
 

js/test_templates/upload_input_error_template.html

@@ -25,14 +25,6 @@
           <span class="upload-button">
             Browse
           </span>
-          
-            <span v-show="showErrors">
-      <span class="icon icon--alert icon-validation" aria-hidden="true"><svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16" fill="#fdb81e">
-  <path d="M8 16c-4.411 0-8-3.589-8-8s3.589-8 8-8 8 3.589 8 8-3.589 8-8 8zM8 2C4.691 2 2 4.691 2 8s2.691 6 6 6 6-2.691 6-6-2.691-6-6-6zm0 8c-.552 0-1-.447-1-1V4c0-.552.448-1 1-1s1 .448 1 1v5c0 .553-.448 1-1 1zm0 3c-.26 0-.52-.11-.71-.29-.18-.19-.29-.45-.29-.71 0-.271.11-.521.29-.71.38-.37 1.05-.37 1.42 0 .18.189.29.45.29.71s-.11.52-.29.71c-.19.18-.45.29-.71.29z"/>
-</svg>
-</span>
-  </span>
-          
         </label>
         <input
           v-on:change="addAttachment"
@@ -46,8 +38,11 @@
           <input type="hidden" name="pdf-filename" id="pdf-filename" ref="attachmentFilename">
           <input type="hidden" name="pdf-object_name" id="pdf-object_name" ref="attachmentObjectName">
       </div>
+      <template v-if="uploadError">
+        <span class="usa-input__message">There was an error uploading your file. Please try again. If you encounter repeated problems uploading this file, please contact CCPO.</span>
+      </template>
       
-        <span v-show="showErrors" class="usa-input__message">[&#39;Test Error Message&#39;]</span>
+        <span class="usa-input__message">Test Error Message</span>
       
     </div>
   </div>

js/test_templates/upload_input_template.html

@@ -25,7 +25,6 @@
           <span class="upload-button">
             Browse
           </span>
-          
         </label>
         <input
           v-on:change="addAttachment"
@@ -39,6 +38,9 @@
           <input type="hidden" name="pdf-filename" id="pdf-filename" ref="attachmentFilename">
           <input type="hidden" name="pdf-object_name" id="pdf-object_name" ref="attachmentObjectName">
       </div>
+      <template v-if="uploadError">
+        <span class="usa-input__message">There was an error uploading your file. Please try again. If you encounter repeated problems uploading this file, please contact CCPO.</span>
+      </template>
       
     </div>
   </div>

script/seed_sample.py

@@ -1,10 +1,11 @@
 # 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
+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,20 @@ 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:
+    def build_pdf():
-        pdf = FileStorage(fp, content_type="application/pdf")
+        return {"filename": "sample_task_order.pdf", "object_name": str(uuid4())}
 
-        draft_to = TaskOrderFactory.build(portfolio=portfolio, pdf=None)
+    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(
+    upcoming_to = TaskOrderFactory.build(
-            portfolio=portfolio, signed_at=yesterday, pdf=pdf
+        portfolio=portfolio, signed_at=yesterday, pdf=build_pdf()
-        )
+    )
-        expired_to = TaskOrderFactory.build(
+    expired_to = TaskOrderFactory.build(
-            portfolio=portfolio, signed_at=yesterday, pdf=pdf
+        portfolio=portfolio, signed_at=yesterday, pdf=build_pdf()
-        )
+    )
-        active_to = TaskOrderFactory.build(
+    active_to = TaskOrderFactory.build(
-            portfolio=portfolio, signed_at=yesterday, pdf=pdf
+        portfolio=portfolio, signed_at=yesterday, pdf=build_pdf()
-        )
+    )
 
     clins = [
         CLINFactory.build(

templates/components/upload_input.html

@@ -30,9 +30,6 @@
           <span class="upload-button">
             Browse
           </span>
-          {% if field.errors %}
-            <span v-show="showErrors">{{ Icon('alert',classes="icon-validation") }}</span>
-          {% endif %}
         </label>
         <input
           v-on:change="addAttachment"
@@ -46,8 +43,11 @@
           <input type="hidden" name="{{ field.filename.name }}" id="{{ field.filename.name }}" ref="attachmentFilename">
           <input type="hidden" name="{{ field.object_name.name }}" id="{{ field.object_name.name }}" ref="attachmentObjectName">
       </div>
-      {% for error, error_message in field.errors.items() %}
+      <template v-if="uploadError">
-        <span v-show="showErrors" class="usa-input__message">{{error_message}}</span>
+        <span class="usa-input__message">{{ "forms.task_order.upload_error" | translate }}</span>
+      </template>
+      {% for error, error_messages in field.errors.items() %}
+        <span class="usa-input__message">{{error_messages[0]}}</span>
       {% endfor %}
     </div>
   </div>

tests/routes/task_orders/test_new.py

@@ -1,16 +1,18 @@
 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
 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"):
+def build_pdf_form_data(filename="sample.pdf", object_name=None):
-    return {"pdf-filename": filename, "pdf-object_name": object_name}
+    return {"pdf-filename": filename, "pdf-object_name": object_name or uuid4()}
 
 
 @pytest.fixture
@@ -101,6 +103,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(

translations.yaml

@@ -149,7 +149,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 <strong>and</strong> another cloud provider'
       cloud: 'Yes, migrating from another cloud provider'