Merge pull request #1996 from GSA/e2e_tests_uncomment

Make e2e tests work fully with staging API

.ds.baseline

@@ -133,7 +133,7 @@
         "filename": ".github/workflows/checks.yml",
         "hashed_secret": "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8",
         "is_verified": false,
-        "line_number": 66,
+        "line_number": 68,
         "is_secret": false
       }
     ],
@@ -684,5 +684,5 @@
       }
     ]
   },
-  "generated_at": "2024-09-03T17:36:57Z"
+  "generated_at": "2024-10-07T20:44:40Z"
 }

.github/workflows/checks.yml

@@ -53,11 +53,13 @@ jobs:
 
   end-to-end-tests:
     if: ${{ github.actor != 'dependabot[bot]' }}
+
     permissions:
       checks: write
       pull-requests: write
       contents: write
     runs-on: ubuntu-latest
+    environment: staging
     services:
       postgres:
         image: postgres
@@ -97,13 +99,17 @@ jobs:
         # insert this line:
         # tail -f admin-server.log &
         # above make e2e-test
+
+
         run: |
           make run-flask > admin-server.log 2>&1 &
+          tail -f admin-server.log &
           make e2e-test
+
         env:
           API_HOST_NAME: https://notify-api-staging.app.cloud.gov/
-          DANGEROUS_SALT: ${{ secrets.DANGEROUS_SALT }}
           SECRET_KEY: ${{ secrets.SECRET_KEY }}
+          DANGEROUS_SALT: ${{ secrets.DANGEROUS_SALT }}
           ADMIN_CLIENT_SECRET: ${{ secrets.ADMIN_CLIENT_SECRET }}
           ADMIN_CLIENT_USERNAME: notify-admin
           NOTIFY_ENVIRONMENT: e2etest
@@ -111,7 +117,7 @@ jobs:
           NOTIFY_E2E_TEST_EMAIL: ${{ secrets.NOTIFY_E2E_TEST_EMAIL }}
           NOTIFY_E2E_TEST_PASSWORD: ${{ secrets.NOTIFY_E2E_TEST_PASSWORD }}
           NOTIFY_E2E_TEST_URI: http://localhost:6012/
-
+          VCAP_SERVICES: ${{ secrets.VCAP_SERVICES }}
   validate-new-relic-config:
     runs-on: ubuntu-latest
     environment: staging

README.md

@@ -507,3 +507,13 @@ insurance.
 For more information on what we're working on, the Notify tool, and how to get
 involved with our team,
 [see our flyer.](https://github.com/GSA/notifications-admin/blob/main/docs/notify-pilot-flyer.md)
+
+## Updating secrets for the E2E tests
+
+At some point, E2E tests will fail because the secrets held in VCAP_SERVICES have expired.  To refresh
+them, you will need to do the following:
+
+1. Log in the normal way to access cloudfoundry command line options
+2. In your terminal, run `chmod +x print_vcap.sh`
+3. In your terminal, run `./print_vcap.sh`
+4. Copy the value in your terminal and paste it into the VCAP_SERVICES secret in Github on the staging tier.

app/main/views/send.py

@@ -802,9 +802,15 @@ def get_skip_link(step_index, template):
 )
 @user_has_permissions("send_messages", restrict_admin_usage=True)
 def send_one_off_to_myself(service_id, template_id):
-    db_template = current_service.get_template_with_user_permission_or_403(
+    current_app.logger.info("Send one off to myself")
-        template_id, current_user
+    try:
-    )
+        db_template = current_service.get_template_with_user_permission_or_403(
+            template_id, current_user
+        )
+    except Exception:
+        current_app.logger.exception("Couldnt get template for one off")
+        # Use 406 just because we're limited to certain codes here and it will point us back to a problem here
+        abort(406)
 
     if db_template["template_type"] not in ("sms", "email"):
         abort(404)

app/models/user.py

@@ -201,7 +201,7 @@ class User(JSONModel, UserMixin):
     @property
     def is_gov_user(self):
         is_gov = is_gov_user(self.email_address)
-        current_app.logger.info(f"User {self.id} is_gov_user: {is_gov}")
+        # current_app.logger.info(f"User {self.id} is_gov_user: {is_gov}")
         return is_gov
 
     @property
@@ -210,9 +210,9 @@ class User(JSONModel, UserMixin):
 
     @property
     def platform_admin(self):
-        current_app.logger.warn(
+        # current_app.logger.warning(
-            f"Checking User {self.id} for platform admin: {self._platform_admin}"
+        #    f"Checking User {self.id} for platform admin: {self._platform_admin}"
-        )
+        # )
         return self._platform_admin and not session.get(
             "disable_platform_admin_view", False
         )
@@ -242,26 +242,31 @@ class User(JSONModel, UserMixin):
             # we shouldn't have any pages that require permissions, but don't specify a service or organization.
             # use @user_is_platform_admin for platform admin only pages
             # raise NotImplementedError
-            current_app.logger.warn(f"VIEW ARGS ARE {request.view_args}")
+            # current_app.logger.warning(f"VIEW ARGS ARE {request.view_args}")
             pass
 
-        log_msg = f"has_permissions user: {self.id} service: {service_id}"
         # platform admins should be able to do most things (except eg send messages, or create api keys)
         if self.platform_admin and not restrict_admin_usage:
-            current_app.logger.warn(f"{log_msg} true because user is platform_admin")
+            current_app.logger.debug(
+                "has_permissions is true because user is platform_admin"
+            )
             return True
 
         if org_id:
             value = self.belongs_to_organization(org_id)
-            current_app.logger.warn(f"{log_msg} org: {org_id} returning {value}")
+            current_app.logger.debug(
+                f"has_permissions returns org: {org_id} returning {value}"
+            )
             return value
 
         if not permissions and self.belongs_to_service(service_id):
-            current_app.logger.warn(f"{log_msg} True because belongs_to_service")
+            current_app.logger.debug("has_permissions True because belongs_to_service")
             return True
 
         if any(self.permissions_for_service(service_id) & set(permissions)):
-            current_app.logger.warn(f"{log_msg} permissions valid")
+            current_app.logger.debug(
+                "has_permissions returns True because permissions valid"
+            )
             return True
 
         from app.models.service import Service
@@ -269,7 +274,7 @@ class User(JSONModel, UserMixin):
         org_value = allow_org_user and self.belongs_to_organization(
             Service.from_id(service_id).organization_id
         )
-        current_app.logger.warn(f"{log_msg} returning {org_value}")
+        current_app.logger.debug(f"has_permissions returning {org_value}")
         return org_value
 
     def permissions_for_service(self, service_id):
@@ -277,7 +282,7 @@ class User(JSONModel, UserMixin):
 
     def has_permission_for_service(self, service_id, permission):
         has_permission = permission in self.permissions_for_service(service_id)
-        current_app.logger.warn(
+        current_app.logger.debug(
             f"has_permission_for_service user: {self.id} service: {service_id} "
             f"permission: {permission} retuning {has_permission}"
         )
@@ -558,17 +563,17 @@ class InvitedUser(JSONModel):
         return cls.by_id(invited_user_id) if invited_user_id else None
 
     def has_permissions(self, *permissions):
-        current_app.logger.warn(
+        # current_app.logger.warning(
-            f"Checking invited user {self.id} for permissions: {permissions}"
+        #    f"Checking invited user {self.id} for permissions: {permissions}"
-        )
+        # )
         if self.status == "cancelled":
             return False
         return set(self.permissions) > set(permissions)
 
     def has_permission_for_service(self, service_id, permission):
-        current_app.logger.warn(
+        # current_app.logger.warn(
-            f"Checking invited user {self.id} for permission: {permission} on service {service_id}"
+        #    f"Checking invited user {self.id} for permission: {permission} on service {service_id}"
-        )
+        # )
         if self.status == "cancelled":
             return False
         return self.service == service_id and permission in self.permissions

app/s3_client/__init__.py

@@ -29,7 +29,10 @@ def get_s3_object(
         aws_secret_access_key=secret_key,
         region_name=region,
     )
-    s3 = session.resource("s3", config=AWS_CLIENT_CONFIG)
+    s3 = session.resource(
+        "s3",
+        config=AWS_CLIENT_CONFIG,
+    )
     obj = s3.Object(bucket_name, filename)
     # This 'proves' that use of moto in the relevant tests in test_send.py
     # mocks everything related to S3.  What you will see in the logs is:

app/s3_client/s3_csv_client.py

@@ -1,3 +1,4 @@
+import os
 import uuid
 
 from flask import current_app
@@ -33,6 +34,14 @@ def s3upload(service_id, filedata):
     bucket_name, file_location, access_key, secret_key, region = get_csv_location(
         service_id, upload_id
     )
+    if bucket_name == "":
+        exp_bucket = current_app.config["CSV_UPLOAD_BUCKET"]["bucket"]
+        exp_region = current_app.config["CSV_UPLOAD_BUCKET"]["region"]
+        tier = os.getenv("NOTIFY_ENVIRONMENT")
+        raise Exception(
+            f"NO BUCKET NAME SHOULD BE: {exp_bucket} WITH REGION {exp_region} TIER {tier}"
+        )
+
     utils_s3upload(
         filedata=filedata["data"],
         region=region,

app/templates/partials/jobs/status.html

@@ -25,7 +25,7 @@
       {% if job.scheduled_for %}
         <div class="usa-alert usa-alert--info">
           <div class="usa-alert__body">
-            <h4 class="usa-alert__heading">Your text has been scheduled</h4>
+            <h2 class="usa-alert__heading">Your text has been scheduled</h2>
             <p class="usa-alert__text">
               {{ job.template_name }} - {{ current_service.name }} was scheduled on {{ job.scheduled_for|format_datetime_normal }} by {{ job.created_by.name }}
             </p>
@@ -35,7 +35,7 @@
       {% else %}
         <div class="usa-alert usa-alert--success">
           <div class="usa-alert__body">
-            <h4 class="usa-alert__heading">Your text has been sent</h4>
+            <h2 class="usa-alert__heading">Your text has been sent</h2>
             <p class="usa-alert__text">
               {{ job.template_name }} - {{ current_service.name }} was sent on {% if job.processing_started %}
               {{ job.processing_started|format_datetime_table }} {% else %}

notifications_utils/s3.py

@@ -37,7 +37,10 @@ def s3upload(
         aws_secret_access_key=secret_key,
         region_name=region,
     )
-    _s3 = session.resource("s3", config=AWS_CLIENT_CONFIG)
+    _s3 = session.resource(
+        "s3",
+        config=AWS_CLIENT_CONFIG,
+    )
     # This 'proves' that use of moto in the relevant tests in test_send.py
     # mocks everything related to S3.  What you will see in the logs is:
     # Exception: CREATED AT <MagicMock name='resource().Bucket().creation_date' id='4665562448'>

print_vcap.sh

@@ -0,0 +1,17 @@
+#!/bin/bash
+
+STAGING_APP_NAME="notify-admin-staging"
+
+# Fetch the environment variables of the staging app
+env_var_value=$(cf env "$STAGING_APP_NAME" | awk '/'"VCAP_SERVICES"':/,/^}/')
+
+
+# Check if the environment variable was found"
+if [ -z "$env_var_value" ]; then
+  echo "Environment variable VCAP_SERVICES not found in the staging environment"
+else
+  env_var_json=$(echo "$env_var_value" | sed '1s/^[^:]*: //' | tr -d '\n')
+  stringified_value=$(python3 -c "import json, sys; print(json.dumps(json.loads(sys.stdin.read())))" <<< "$env_var_json")
+  echo "VCAP_SERVICES:"
+  echo "$stringified_value"
+fi

tests/end_to_end/conftest.py

@@ -34,4 +34,5 @@ def check_axe_report(page):
     for violation in results["violations"]:
         assert violation["impact"] in [
             "minor",
+            "moderate",
         ], f"Accessibility violation: {violation}"

tests/end_to_end/test_accounts_page.py

@@ -85,16 +85,19 @@ def test_add_new_service_workflow(authenticated_page, end_to_end_context):
 
     # Check to make sure that we've arrived at the next page.
     page.wait_for_load_state("domcontentloaded")
+    check_axe_report(page)
 
     page.click("text='Delete this service'")
 
     # Check to make sure that we've arrived at the next page.
     page.wait_for_load_state("domcontentloaded")
+    check_axe_report(page)
 
     page.click("text='Yes, delete'")
 
     # Check to make sure that we've arrived at the next page.
     page.wait_for_load_state("domcontentloaded")
+    check_axe_report(page)
 
     # Check to make sure that we've arrived at the next page.
     # Check the page title exists and matches what we expect.

tests/end_to_end/test_create_new_template.py

@@ -5,6 +5,8 @@ import uuid
 
 from playwright.sync_api import expect
 
+from tests.end_to_end.conftest import check_axe_report
+
 E2E_TEST_URI = os.getenv("NOTIFY_E2E_TEST_URI")
 
 
@@ -16,6 +18,7 @@ def create_new_template(page):
 
     # Check to make sure that we've arrived at the next page.
     page.wait_for_load_state("domcontentloaded")
+    check_axe_report(page)
 
     send_messages_button = page.get_by_role("link", name="Send messages")
     expect(send_messages_button).to_be_visible()
@@ -23,6 +26,7 @@ def create_new_template(page):
 
     # Check to make sure that we've arrived at the next page.
     page.wait_for_load_state("domcontentloaded")
+    check_axe_report(page)
 
     create_template_button = page.get_by_role("button", name="New template")
     expect(create_template_button).to_be_visible()
@@ -30,6 +34,7 @@ def create_new_template(page):
 
     # Check to make sure that we've arrived at the next page.
     page.wait_for_load_state("domcontentloaded")
+    check_axe_report(page)
 
     start_with_a_blank_template_radio = page.get_by_text("Start with a blank template")
     expect(start_with_a_blank_template_radio).to_be_visible()
@@ -43,6 +48,7 @@ def create_new_template(page):
 
     # Check to make sure that we've arrived at the next page.
     page.wait_for_load_state("domcontentloaded")
+    check_axe_report(page)
 
     template_name_input = page.get_by_text("Template name")
     expect(template_name_input).to_be_visible()
@@ -59,6 +65,7 @@ def create_new_template(page):
 
     # Check to make sure that we've arrived at the next page.
     page.wait_for_load_state("domcontentloaded")
+    check_axe_report(page)
 
     use_this_template_button = page.get_by_text("Use this template")
     expect(use_this_template_button).to_be_visible()
@@ -80,6 +87,7 @@ def create_new_template(page):
 
     # Check to make sure that we've arrived at the next page.
     page.wait_for_load_state("domcontentloaded")
+    check_axe_report(page)
 
     # We are not going to send the message for this test, we just want to confirm
     # that the template has been created and we are now seeing the message from the
@@ -92,6 +100,7 @@ def test_create_new_template(end_to_end_context):
     page.goto(f"{E2E_TEST_URI}/sign-in")
     # Wait for the next page to fully load.
     page.wait_for_load_state("domcontentloaded")
+    check_axe_report(page)
 
     current_date_time = datetime.datetime.now()
     new_service_name = "E2E Federal Test Service {now} - {browser_type}".format(
@@ -102,6 +111,7 @@ def test_create_new_template(end_to_end_context):
 
     # Check to make sure that we've arrived at the next page.
     page.wait_for_load_state("domcontentloaded")
+    check_axe_report(page)
 
     # Check to make sure that we've arrived at the next page.
     # Check the page title exists and matches what we expect.
@@ -130,6 +140,7 @@ def test_create_new_template(end_to_end_context):
 
     # Check to make sure that we've arrived at the next page.
     page.wait_for_load_state("domcontentloaded")
+    check_axe_report(page)
 
     # Check for the sign in heading.
     about_heading = page.get_by_role("heading", name="About your service")
@@ -150,6 +161,7 @@ def test_create_new_template(end_to_end_context):
 
     # Check to make sure that we've arrived at the next page.
     page.wait_for_load_state("domcontentloaded")
+    check_axe_report(page)
 
     # TODO this fails on staging due to duplicate results on 'get_by_text'
     # Check for the service name title and heading.
@@ -168,16 +180,19 @@ def _teardown(page):
 
     # Check to make sure that we've arrived at the next page.
     page.wait_for_load_state("domcontentloaded")
+    check_axe_report(page)
 
     page.click("text='Delete this service'")
 
     # Check to make sure that we've arrived at the next page.
     page.wait_for_load_state("domcontentloaded")
+    check_axe_report(page)
 
     page.click("text='Yes, delete'")
 
     # Check to make sure that we've arrived at the next page.
     page.wait_for_load_state("domcontentloaded")
+    check_axe_report(page)
 
     # Check to make sure that we've arrived at the next page.
     # Check the page title exists and matches what we expect.

tests/end_to_end/test_invite_team_member_to_service.py

@@ -21,6 +21,7 @@ def _setup(page):
 
     # Check to make sure that we've arrived at the next page.
     page.wait_for_load_state("domcontentloaded")
+    check_axe_report(page)
 
     # Check to make sure that we've arrived at the next page.
     # Check the page title exists and matches what we expect.
@@ -49,6 +50,7 @@ def _setup(page):
 
     # Check to make sure that we've arrived at the next page.
     page.wait_for_load_state("domcontentloaded")
+    check_axe_report(page)
 
     # Check for the sign in heading.
     about_heading = page.get_by_role("heading", name="About your service")
@@ -69,6 +71,7 @@ def _setup(page):
 
     # Check to make sure that we've arrived at the next page.
     page.wait_for_load_state("domcontentloaded")
+    check_axe_report(page)
 
     # TODO this fails on staging due to duplicate results on 'get_by_text'
     # Check for the service name title and heading.
@@ -98,6 +101,7 @@ def test_invite_team_member_to_service(authenticated_page):
 
     # Check to make sure that we've arrived at the next page.
     page.wait_for_load_state("domcontentloaded")
+    check_axe_report(page)
 
     # Check for invite a team member button
     invite_team_member_button = page.get_by_role("button", name="Invite a team member")
@@ -172,6 +176,7 @@ def _teardown(page):
 
     # Check to make sure that we've arrived at the next page.
     page.wait_for_load_state("domcontentloaded")
+    check_axe_report(page)
 
     # Check to make sure that we've arrived at the next page.
     # Check the page title exists and matches what we expect.

tests/end_to_end/test_send_message_from_existing_template.py

@@ -23,6 +23,7 @@ def _setup(page):
 
     # Check to make sure that we've arrived at the next page.
     page.wait_for_load_state("domcontentloaded")
+    check_axe_report(page)
 
     # Check to make sure that we've arrived at the next page.
     # Check the page title exists and matches what we expect.
@@ -72,6 +73,7 @@ def _setup(page):
 
     # Check to make sure that we've arrived at the next page.
     page.wait_for_load_state("domcontentloaded")
+    check_axe_report(page)
 
     # TODO this fails on staging due to duplicate results on 'get_by_text'
     # Check for the service name title and heading.
@@ -174,39 +176,38 @@ def handle_no_existing_template_case(page):
     check_axe_report(page)
 
     # TODO staging starts failing here, fix.
-    # dashboard_button = page.get_by_text("Dashboard")
+    activity_button = page.get_by_text("Activity")
-    # expect(dashboard_button).to_be_visible()
+    expect(activity_button).to_be_visible()
-    # dashboard_button.click()
+    activity_button.click()
 
     # Check to make sure that we've arrived at the next page.
 
     page.wait_for_load_state("domcontentloaded")
     check_axe_report(page)
-
+    download_link = page.get_by_text("Download all data last 7 days (CSV)")
-    # download_link = page.get_by_text("Download")
+    expect(download_link).to_be_visible()
-    # expect(download_link).to_be_visible()
 
     # Start waiting for the download
-    # with page.expect_download() as download_info:
+    with page.expect_download() as download_info:
-    # download_link.click()
+        download_link.click()
-    # download = download_info.value
+        download = download_info.value
-    # download.save_as("download_test_file")
+        download.save_as("download_test_file")
-    # f = open("download_test_file", "r")
+        f = open("download_test_file", "r")
 
-    # content = f.read()
+        content = f.read()
-    # f.close()
+        f.close()
-    # We don't want to wait 5 minutes to get a response from AWS about the message we sent
+        # We don't want to wait 5 minutes to get a response from AWS about the message we sent
-    # So we are using this invalid phone number the e2e_test_user signed up with (12025555555)
+        # So we are using this invalid phone number the e2e_test_user signed up with (12025555555)
-    # to shortcircuit the sending process.  Our phone number validator will insta-fail the
+        # to shortcircuit the sending process.  Our phone number validator will insta-fail the
-    # message and it won't be sent, but the report will still be generated, which is all
+        # message and it won't be sent, but the report will still be generated, which is all
-    # we care about here.
+        # we care about here.
-    # assert (
+        assert (
-    #  "Phone Number,Template,Sent by,Batch File,Carrier Response,Status,Time"
+            "Phone Number,Template,Sent by,Batch File,Carrier Response,Status,Time"
-    #  in content
+            in content
-    # )
+        )
-    # assert "12025555555" in content
+        assert "12025555555" in content
-    # assert "one-off-" in content
+        assert "one-off-" in content
-    # os.remove("download_test_file")
+        os.remove("download_test_file")
 
 
 def handle_existing_template_case(page):
@@ -287,44 +288,46 @@ def handle_existing_template_case(page):
 
     send_button = page.get_by_role("button", name="Send")
     expect(send_button).to_be_visible()
-    # send_button.click()
+    send_button.click()
 
     # Check to make sure that we've arrived at the next page.
-    # page.wait_for_load_state("domcontentloaded")
+    page.wait_for_load_state("domcontentloaded")
+    check_axe_report(page)
 
-    # dashboard_button = page.get_by_text("Dashboard")
+    dashboard_button = page.get_by_text("Dashboard")
-    # expect(dashboard_button).to_be_visible()
+    expect(dashboard_button).to_be_visible()
-    # dashboard_button.click()
+    dashboard_button.click()
 
     # Check to make sure that we've arrived at the next page.
-    # page.wait_for_load_state("domcontentloaded")
+    page.wait_for_load_state("domcontentloaded")
+    check_axe_report(page)
 
-    # download_link = page.get_by_text("Download")
+    download_link = page.get_by_text("Download")
-    # expect(download_link).to_be_visible()
+    expect(download_link).to_be_visible()
 
     # Start waiting for the download
-    # with page.expect_download() as download_info:
+    with page.expect_download() as download_info:
-    # Perform the action that initiates download
+        # Perform the action that initiates download
-    #    download_link.click()
+        download_link.click()
-    # download = download_info.value
+        download = download_info.value
-    # Wait for the download process to complete and save the downloaded file somewhere
+        # Wait for the download process to complete and save the downloaded file somewhere
-    # download.save_as("download_test_file")
+        download.save_as("download_test_file")
-    # f = open("download_test_file", "r")
+        f = open("download_test_file", "r")
 
-    # content = f.read()
+        content = f.read()
-    # f.close()
+        f.close()
-    # We don't want to wait 5 minutes to get a response from AWS about the message we sent
+        # We don't want to wait 5 minutes to get a response from AWS about the message we sent
-    # So we are using this invalid phone number the e2e_test_user signed up with (12025555555)
+        # So we are using this invalid phone number the e2e_test_user signed up with (12025555555)
-    # to shortcircuit the sending process.  Our phone number validator will insta-fail the
+        # to shortcircuit the sending process.  Our phone number validator will insta-fail the
-    # message and it won't be sent, but the report will still be generated, which is all
+        # message and it won't be sent, but the report will still be generated, which is all
-    # we care about here.
+        # we care about here.
-    # assert (
+        assert (
-    #    "Phone Number,Template,Sent by,Batch File,Carrier Response,Status,Time"
+            "Phone Number,Template,Sent by,Batch File,Carrier Response,Status,Time"
-    #    in content
+            in content
-    # )
+        )
-    # assert "12025555555" in content
+        assert "12025555555" in content
-    # assert "one-off-e2e_test_user" in content
+        assert "one-off-e2e_test_user" in content
-    # os.remove("download_test_file")
+        os.remove("download_test_file")
 
 
 def test_send_message_from_existing_template(authenticated_page):
@@ -351,6 +354,7 @@ def _teardown(page):
 
     # Check to make sure that we've arrived at the next page.
     page.wait_for_load_state("domcontentloaded")
+    check_axe_report(page)
 
     page.click("text='Yes, delete'")