ugh

.ds.baseline

@@ -133,7 +133,7 @@
         "filename": ".github/workflows/checks.yml",
         "hashed_secret": "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8",
         "is_verified": false,
-        "line_number": 68,
+        "line_number": 71,
         "is_secret": false
       }
     ],
@@ -555,7 +555,7 @@
         "filename": "tests/app/main/views/test_register.py",
         "hashed_secret": "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8",
         "is_verified": false,
-        "line_number": 200,
+        "line_number": 199,
         "is_secret": false
       },
       {
@@ -563,7 +563,7 @@
         "filename": "tests/app/main/views/test_register.py",
         "hashed_secret": "bb5b7caa27d005d38039e3797c3ddb9bcd22c3c8",
         "is_verified": false,
-        "line_number": 273,
+        "line_number": 272,
         "is_secret": false
       }
     ],
@@ -684,5 +684,5 @@
       }
     ]
   },
-  "generated_at": "2025-01-13T20:16:58Z"
+  "generated_at": "2025-01-16T21:38:03Z"
 }

.github/workflows/checks.yml

@@ -38,10 +38,13 @@ jobs:
           output: report-markdown
           annotations: failed-tests
           prnumber: ${{ steps.findPr.outputs.number }}
+      - name: Check imports alphabetized
+        run: poetry run isort ./app ./tests
+      - name: Check pep8
+        run: poetry run black ./app ./tests
       - name: Run style checks
         run: poetry run flake8 .
-      - name: Check imports alphabetized
-        run: poetry run isort --check-only ./app ./tests
+
       - name: Check dead code
         run: make dead-code
       - name: Run js tests

app/main/validators.py

@@ -1,3 +1,5 @@
+import csv
+from io import StringIO
 import re
 from abc import ABC, abstractmethod
 
@@ -28,6 +30,16 @@ class CsvFileValidator:
         self.message = message
 
     def __call__(self, form, field):
+
+        data = Spreadsheet.from_file_form(form).as_dict
+        data = data["data"]
+        if data is not None and data != "":
+            csv_file = StringIO(data)
+            reader = csv.reader(csv_file)
+            first_line = next(reader, None)
+            if first_line is None or all(cell.strip() == "" for cell in first_line):
+                raise ValidationError(f"No headers on row 1 in {field.data.filename}")
+
         if not Spreadsheet.can_handle(field.data.filename):
             raise ValidationError(
                 "{} is not a spreadsheet that Notify can read".format(

app/main/views/index.py

@@ -32,11 +32,7 @@ def check_feature_flags():
 @main.route("/test/feature-flags")
 def test_feature_flags():
     return jsonify(
-        {
-            "FEATURE_ABOUT_PAGE_ENABLED": current_app.config[
-                "FEATURE_ABOUT_PAGE_ENABLED"
-            ]
-        }
+        {"FEATURE_ABOUT_PAGE_ENABLED": current_app.config["FEATURE_ABOUT_PAGE_ENABLED"]}
     )
 
 
@@ -235,7 +231,6 @@ def contact():
     return render_template(
         "views/contact.html",
         navigation_links=about_notify_nav(),
-
     )
 
 
@@ -268,7 +263,6 @@ def join_notify():
     return render_template(
         "views/join-notify.html",
         navigation_links=about_notify_nav(),
-
     )
 
 

app/main/views/send.py

@@ -132,6 +132,7 @@ def send_messages(service_id, template_id):
     form = CsvUploadForm()
     if form.validate_on_submit():
         try:
+
             upload_id = s3upload(
                 service_id,
                 Spreadsheet.from_file_form(form).as_dict,

app/models/spreadsheet.py

@@ -42,7 +42,8 @@ class Spreadsheet:
 
     @staticmethod
     def normalise_newlines(file_content):
-        return "\r\n".join(file_content.read().decode("utf-8").splitlines())
+        rows = file_content.read().decode("utf-8").splitlines()
+        return "\r\n".join(rows)
 
     @classmethod
     def from_rows(cls, rows, filename=""):

tests/app/main/views/test_index.py

@@ -122,9 +122,7 @@ def test_static_pages(client_request, mock_get_organization_by_domain, view, moc
         session["user_id"] = None
     request(
         _expected_status=302,
-        _expected_redirect="/sign-in?next={}".format(
-            url_for("main.{}".format(view))
-        ),
+        _expected_redirect="/sign-in?next={}".format(url_for("main.{}".format(view))),
     )
 
 

tests/app/main/views/test_register.py

@@ -144,9 +144,8 @@ def test_should_return_200_when_email_is_not_gov_uk(
         _expected_status=200,
     )
 
-    assert (
-        "Enter a public sector email address."
-        in normalize_spaces(page.select_one(".usa-error-message").text)
+    assert "Enter a public sector email address." in normalize_spaces(
+        page.select_one(".usa-error-message").text
     )
 
 

tests/app/main/views/test_send.py

@@ -898,6 +898,67 @@ def test_upload_csv_invalid_extension(
     assert "invalid.txt is not a spreadsheet that Notify can read" in page.text
 
 
+@pytest.mark.parametrize(
+    ("file_contents", "expected_error"),
+    [
+        (
+            """
+
+            phone number, name
+            +12028675109, example
+            +12028675109,
+            +12028675109, example
+        """,
+            (
+                "There’s a problem with example.csv "
+                "You need to put the column headers in row 1."
+            ),
+        ),
+    ],
+)
+def test_upload_csv_file_with_extra_blank_lines_is_flagged(
+    client_request,
+    mocker,
+    mock_get_service_template_with_placeholders,
+    mock_get_users_by_service,
+    mock_get_service_statistics,
+    mock_get_job_doesnt_exist,
+    mock_get_jobs,
+    service_one,
+    fake_uuid,
+    file_contents,
+    expected_error,
+):
+
+    mocker.patch("app.main.views.send.set_metadata_on_csv_upload")
+
+    mocker.patch(
+        "app.main.views.send.get_csv_metadata",
+        return_value={"original_file_name": "example.csv"},
+    )
+    mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid())
+
+    mocker.patch("app.main.views.send.s3download", return_value=file_contents)
+
+    page = client_request.post(
+        "main.send_messages",
+        service_id=service_one["id"],
+        template_id=fake_uuid,
+        _data={"file": (BytesIO("".encode("utf-8")), "example.csv")},
+        _follow_redirects=True,
+    )
+
+    with client_request.session_transaction() as session:
+        assert "file_uploads" not in session
+
+    assert page.select_one("input[type=file]").has_attr("accept")
+    assert (
+        page.select_one("input[type=file]")["accept"]
+        == ".csv,.xlsx,.xls,.ods,.xlsm,.tsv"
+    )
+    assert normalize_spaces(page.select(".banner-dangerous")[0].text) == expected_error
+
+
 def test_upload_csv_size_too_big(
     client_request,
     mock_login,