From 20fd4655ffff124af0b055590d1c1dba3f3bc0ef Mon Sep 17 00:00:00 2001 From: Phil Dominguez <142051477+phildominguez-gsa@users.noreply.github.com> Date: Fri, 13 Dec 2024 09:04:33 -0500 Subject: [PATCH 1/4] Requiring in_progress status for some endpoints (#4538) --- backend/audit/views/views.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/backend/audit/views/views.py b/backend/audit/views/views.py index b1f920db1..1d785128c 100644 --- a/backend/audit/views/views.py +++ b/backend/audit/views/views.py @@ -301,6 +301,7 @@ def post(self, request, *args, **kwargs): class CrossValidationView(SingleAuditChecklistAccessRequiredMixin, generic.View): + @verify_status(STATUS.IN_PROGRESS) def get(self, request, *args, **kwargs): report_id = kwargs["report_id"] @@ -317,6 +318,7 @@ def get(self, request, *args, **kwargs): except SingleAuditChecklist.DoesNotExist: raise PermissionDenied("You do not have access to this audit.") + @verify_status(STATUS.IN_PROGRESS) def post(self, request, *args, **kwargs): report_id = kwargs["report_id"] @@ -336,6 +338,7 @@ def post(self, request, *args, **kwargs): class ReadyForCertificationView(SingleAuditChecklistAccessRequiredMixin, generic.View): + @verify_status(STATUS.IN_PROGRESS) def get(self, request, *args, **kwargs): report_id = kwargs["report_id"] @@ -352,6 +355,7 @@ def get(self, request, *args, **kwargs): except SingleAuditChecklist.DoesNotExist: raise PermissionDenied("You do not have access to this audit.") + @verify_status(STATUS.IN_PROGRESS) def post(self, request, *args, **kwargs): report_id = kwargs["report_id"] From 73dd827f003a3cae5fb31b8ab54b7517c2ef8d09 Mon Sep 17 00:00:00 2001 From: Phil Dominguez <142051477+phildominguez-gsa@users.noreply.github.com> Date: Fri, 13 Dec 2024 09:05:16 -0500 Subject: [PATCH 2/4] Improving cog/over search results (#4528) * Improving cog/over search results * Restoring deleting log line * Accepting empty string or either --- backend/dissemination/search.py | 2 + .../searchlib/search_cog_or_oversight.py | 47 ++++++++++ .../dissemination/searchlib/search_general.py | 23 ----- backend/dissemination/test_search.py | 92 ++++++++++--------- 4 files changed, 97 insertions(+), 67 deletions(-) create mode 100644 backend/dissemination/searchlib/search_cog_or_oversight.py diff --git a/backend/dissemination/search.py b/backend/dissemination/search.py index 827dcb1a3..5659d62a6 100644 --- a/backend/dissemination/search.py +++ b/backend/dissemination/search.py @@ -4,6 +4,7 @@ from .searchlib.search_constants import ORDER_BY, DIRECTION, DAS_LIMIT from .searchlib.search_general import report_timing, search_general from .searchlib.search_alns import search_alns +from .searchlib.search_cog_or_oversight import search_cog_or_oversight from .searchlib.search_findings import search_findings from .searchlib.search_direct_funding import search_direct_funding from .searchlib.search_major_program import search_major_program @@ -64,6 +65,7 @@ def search(params): logger.info("search Searching `DisseminationCombined`") results = search_general(DisseminationCombined, params) results = search_alns(results, params) + results = search_cog_or_oversight(results, params) results = search_findings(results, params) results = search_direct_funding(results, params) results = search_major_program(results, params) diff --git a/backend/dissemination/searchlib/search_cog_or_oversight.py b/backend/dissemination/searchlib/search_cog_or_oversight.py new file mode 100644 index 000000000..1f41ca153 --- /dev/null +++ b/backend/dissemination/searchlib/search_cog_or_oversight.py @@ -0,0 +1,47 @@ +from .search_general import report_timing + +from django.db.models import Q + +import logging +import time + +logger = logging.getLogger(__name__) + + +def search_cog_or_oversight(general_results, params): + t0 = time.time() + + q_cogover = _get_cog_or_oversight_match_query( + params.get("agency_name", None), params.get("cog_or_oversight", "either") + ) + filtered_general_results = general_results.filter(q_cogover) + + t1 = time.time() + report_timing("search_cog_or_oversight", params, t0, t1) + + return filtered_general_results + + +def _get_cog_or_oversight_match_query(agency_name, cog_or_oversight): + if cog_or_oversight.lower() in ["", "either"]: + if agency_name: + return Q( + Q(cognizant_agency__in=[agency_name]) + | Q(oversight_agency__in=[agency_name]) + ) + else: + # Every submission should have a value for either cog or over, so + # nothing to do here + return Q() + elif cog_or_oversight.lower() == "cog": + if agency_name: + return Q(cognizant_agency__in=[agency_name]) + else: + # Submissions that have any cog + return Q(cognizant_agency__isnull=False) + elif cog_or_oversight.lower() == "oversight": + if agency_name: + return Q(oversight_agency__in=[agency_name]) + else: + # Submissions that have any over + return Q(oversight_agency__isnull=False) diff --git a/backend/dissemination/searchlib/search_general.py b/backend/dissemination/searchlib/search_general.py index 8c43ffc0d..4e1550b7e 100644 --- a/backend/dissemination/searchlib/search_general.py +++ b/backend/dissemination/searchlib/search_general.py @@ -44,13 +44,6 @@ def search_general(base_model, params=None): q_end_date = _get_end_date_match_query(params.get("end_date", None)) r_end_date = base_model.objects.filter(q_end_date) - ############## - # Cog/Over - q_cogover = _get_cog_or_oversight_match_query( - params.get("agency_name", None), params.get("cog_or_oversight", None) - ) - r_cogover = base_model.objects.filter(q_cogover) - ############## # Fiscal year end month q_fy_end_month = _get_fy_end_month_match_query(params.get("fy_end_month", None)) @@ -74,7 +67,6 @@ def search_general(base_model, params=None): & r_start_date & r_end_date & r_state - & r_cogover & r_names & r_uei & r_base @@ -130,21 +122,6 @@ def _get_end_date_match_query(end_date): return Q(fac_accepted_date__lte=end_date) -def _get_cog_or_oversight_match_query(agency_name, cog_or_oversight): - if not agency_name: - return Q() - - if cog_or_oversight.lower() == "either": - return Q( - Q(cognizant_agency__in=[agency_name]) - | Q(oversight_agency__in=[agency_name]) - ) - elif cog_or_oversight.lower() == "cog": - return Q(cognizant_agency__in=[agency_name]) - elif cog_or_oversight.lower() == "oversight": - return Q(oversight_agency__in=[agency_name]) - - def _get_audit_years_match_query(audit_years): if not audit_years: return Q() diff --git a/backend/dissemination/test_search.py b/backend/dissemination/test_search.py index 653421e0b..7e4171221 100644 --- a/backend/dissemination/test_search.py +++ b/backend/dissemination/test_search.py @@ -283,50 +283,6 @@ def test_date_range(self): self.assertGreaterEqual(r.fac_accepted_date, search_start_date) self.assertLessEqual(r.fac_accepted_date, search_end_date) - def test_cognizant_agency(self): - """ - Given a cognizant agency name, search_general should return only records with a matching cognizant agency name (not oversight) - """ - - baker.make(General, is_public=True, cognizant_agency="01") - baker.make(General, is_public=True, cognizant_agency="02") - - baker.make(General, is_public=True, oversight_agency="01") - - results = search_general( - General, - { - "cog_or_oversight": "cog", - "agency_name": "01", - }, - ) - - assert_all_results_public(self, results) - self.assertEqual(len(results), 1) - self.assertEqual(results[0].cognizant_agency, "01") - - def test_oversight_agency(self): - """ - Given an oversight agency name, search_general should return only records with a matching oversight agency name (not cognizant) - """ - - baker.make(General, is_public=True, cognizant_agency="01") - - baker.make(General, is_public=True, oversight_agency="01") - baker.make(General, is_public=True, oversight_agency="02") - - results = search_general( - General, - { - "cog_or_oversight": "oversight", - "agency_name": "01", - }, - ) - - assert_all_results_public(self, results) - self.assertEqual(len(results), 1) - self.assertEqual(results[0].oversight_agency, "01") - def test_audit_year(self): """ Given a list of audit years, search_general should return only records where @@ -671,6 +627,54 @@ def test_alns_no_findings(self): class SearchAdvancedFilterTests(TestMaterializedViewBuilder): + def test_search_cog_or_oversight(self): + """ + When making a search on major program, search_general should only return records with an award of that type. + """ + general_cog = baker.make( + General, + cognizant_agency=42, + oversight_agency=None, + ) + baker.make(FederalAward, report_id=general_cog) + general_over = baker.make( + General, + cognizant_agency=None, + oversight_agency=24, + ) + baker.make(FederalAward, report_id=general_over) + self.refresh_materialized_view() + + adv_params = {"advanced_search_flag": True} + + # Either cog or over with no agency should return all + params = {"agency_name": None, "cog_or_oversight": "either", **adv_params} + results = search(params) + self.assertEqual(len(results), 2) + + # Unused agency should return nothing + params = {"agency_name": 99, "cog_or_oversight": "either", **adv_params} + results = search(params) + self.assertEqual(len(results), 0) + + # Either cog or over with valid agency + params = {"agency_name": 42, "cog_or_oversight": "either", **adv_params} + results = search(params) + self.assertEqual(len(results), 1) + self.assertEqual(results[0].report_id, general_cog.report_id) + + # Cog with valid agency + params = {"agency_name": 42, "cog_or_oversight": "cog", **adv_params} + results = search(params) + self.assertEqual(len(results), 1) + self.assertEqual(results[0].report_id, general_cog.report_id) + + # Over with valid agency + params = {"agency_name": 24, "cog_or_oversight": "oversight", **adv_params} + results = search(params) + self.assertEqual(len(results), 1) + self.assertEqual(results[0].report_id, general_over.report_id) + def test_search_findings(self): """ When making a search on a particular type of finding, search_general should only return records with a finding of that type. From a9434a259ffe6124b209fe0839cb0b5e36132612 Mon Sep 17 00:00:00 2001 From: "Hassan D. M. Sambo" Date: Fri, 13 Dec 2024 14:15:05 -0500 Subject: [PATCH 3/4] #4434 Adding ADR for In-progress audit deletion workflow (#4530) --- ...041-in-progress-audit-deletion-workflow.md | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 docs/architecture/decisions/0041-in-progress-audit-deletion-workflow.md diff --git a/docs/architecture/decisions/0041-in-progress-audit-deletion-workflow.md b/docs/architecture/decisions/0041-in-progress-audit-deletion-workflow.md new file mode 100644 index 000000000..8903cb1f3 --- /dev/null +++ b/docs/architecture/decisions/0041-in-progress-audit-deletion-workflow.md @@ -0,0 +1,44 @@ +# 41. In-Progress Audit Deletion Workflow + +Date: 2024-12-11 + +## Status + +Accepted + +## Areas of impact + +* Design +* Engineering +* Process + + +## Related documents/links + +## Context: +**Problem Statement:** +Users frequently create new audit submissions but often abandon them before completion, resulting in a cluttered dashboard view. These incomplete audits create confusion among editors and certifying officials as they navigate numerous partially completed entries. A solution is needed to allow users to manage incomplete audits without risking the accidental deletion of valid or in-progress audits. +While allowing users to delete audits could help reduce dashboard clutter, it introduces the risk of accidental deletion of important or in-progress audits. Such actions could lead to increased user support requests for audit recovery, a task that we are not well-equipped to handle with current resources. + +**Objective:** +Develop a controlled workflow that enables users to remove incomplete audits only (not completed) while minimizing the risk of accidental deletion. The workflow should incorporate checks and balances, such as inactivity thresholds or dual approval mechanisms, to ensure that audits are removed only when appropriate. + +## Decision (Proposed Workflow Solution): + + **User-Initiated Deletion Process:** +- Access-Based Deletion: Any user with access to a specific audit can initiate a deletion request. +- Flagging as Deleted: Once deletion is initiated, the audit record is flagged as "deleted," removing it from the initiating user's audit list and making it inaccessible to all other users. However, the record itself remains in the database for future auditing purposes. +- Auto-Cleanup: A Django command or Django Admin process will periodically scan for flagged records and remove those marked "deleted" for more than a specified duration (to be determined by the team) from the database. + +## Consequences + +**- Reduced Dashboard Clutter:** The workflow will help maintain a cleaner dashboard view by allowing users to remove incomplete audits, reducing confusion for editors and certifying officials navigating audit entries. + +**- Minimized Risk of Accidental Deletion:** By flagging audits for deletion rather than immediately removing them, the system provides a buffer period, allowing users or administrators to intervene if an important audit is flagged in error. + +**- Improved User Experience:** Users will have more control over their audit list, leading to a streamlined experience, as they can manage abandoned audits without cluttering their workspace. + +**- Additional Database Management Requirement:** The system will need periodic monitoring to ensure flagged entries are appropriately purged. + +## Metrics +- Decrease in Zendesk Helpdesk tickets after the feature gets released as tracked by number of times macro gets used. From d231bda7c26caa53a2b863a3e6250bd026b37f98 Mon Sep 17 00:00:00 2001 From: Anastasia Gradova <108748167+anagradova@users.noreply.github.com> Date: Fri, 13 Dec 2024 12:45:38 -0700 Subject: [PATCH 4/4] Updates to test_views.py from old branch (#4546) * Updates to test_views.py from old branch 4213-improve-views-coverage * Updated to match new status for test and format * Updated for flake8 --- backend/audit/test_views.py | 595 ++++++++++++++++++++++++++++++++++-- 1 file changed, 565 insertions(+), 30 deletions(-) diff --git a/backend/audit/test_views.py b/backend/audit/test_views.py index 1c5b0dfcf..4ad864b08 100644 --- a/backend/audit/test_views.py +++ b/backend/audit/test_views.py @@ -1,44 +1,34 @@ -from datetime import datetime, timezone import json +from datetime import datetime, timezone from pathlib import Path from tempfile import NamedTemporaryFile from unittest.mock import patch -from django.test import Client, TestCase, TransactionTestCase -from django.contrib.auth import get_user_model -from django.core.files.uploadedfile import SimpleUploadedFile -from django.urls import reverse - -from faker import Faker - -from model_bakery import baker - -from openpyxl import load_workbook -from openpyxl.cell import Cell - +from audit.cross_validation.naming import SECTION_NAMES as SN from audit.fixtures.excel import ( - ADDITIONAL_UEIS_TEMPLATE, + ADDITIONAL_EINS_ENTRY_FIXTURES, ADDITIONAL_EINS_TEMPLATE, - FEDERAL_AWARDS_TEMPLATE, + ADDITIONAL_UEIS_ENTRY_FIXTURES, + ADDITIONAL_UEIS_TEMPLATE, + CORRECTIVE_ACTION_PLAN_ENTRY_FIXTURES, CORRECTIVE_ACTION_PLAN_TEMPLATE, + FEDERAL_AWARDS_ENTRY_FIXTURES, + FEDERAL_AWARDS_TEMPLATE, + FINDINGS_TEXT_ENTRY_FIXTURES, FINDINGS_TEXT_TEMPLATE, + FINDINGS_UNIFORM_GUIDANCE_ENTRY_FIXTURES, FINDINGS_UNIFORM_GUIDANCE_TEMPLATE, - SECONDARY_AUDITORS_TEMPLATE, + FORM_SECTIONS, + NOTES_TO_SEFA_ENTRY_FIXTURES, NOTES_TO_SEFA_TEMPLATE, - CORRECTIVE_ACTION_PLAN_ENTRY_FIXTURES, - FINDINGS_TEXT_ENTRY_FIXTURES, - FINDINGS_UNIFORM_GUIDANCE_ENTRY_FIXTURES, - FEDERAL_AWARDS_ENTRY_FIXTURES, - ADDITIONAL_UEIS_ENTRY_FIXTURES, - ADDITIONAL_EINS_ENTRY_FIXTURES, SECONDARY_AUDITORS_ENTRY_FIXTURES, - NOTES_TO_SEFA_ENTRY_FIXTURES, - FORM_SECTIONS, + SECONDARY_AUDITORS_TEMPLATE, ) from audit.fixtures.single_audit_checklist import ( - fake_auditor_certification, fake_auditee_certification, + fake_auditor_certification, ) +from audit.forms import AuditeeCertificationStep2Form, AuditorCertificationStep1Form from audit.models import ( Access, SingleAuditChecklist, @@ -47,9 +37,17 @@ generate_sac_report_id, ) from audit.models.models import STATUS -from audit.cross_validation.naming import SECTION_NAMES as SN -from audit.views import MySubmissions +from audit.views import AuditeeCertificationStep2View, MySubmissions from dissemination.models import FederalAward, General +from django.contrib.auth import get_user_model +from django.core.exceptions import PermissionDenied +from django.core.files.uploadedfile import SimpleUploadedFile +from django.test import Client, RequestFactory, TestCase, TransactionTestCase +from django.urls import reverse +from faker import Faker +from model_bakery import baker +from openpyxl import load_workbook +from openpyxl.cell import Cell User = get_user_model() @@ -113,6 +111,7 @@ def _authed_post(client, user, view_str, kwargs=None, data=None): def _make_user_and_sac(**kwargs): + """Helper function for to make a user and basic sac""" user = baker.make(User) sac = baker.make(SingleAuditChecklist, **kwargs) return user, sac @@ -125,6 +124,7 @@ def _load_json(target): def _mock_gen_report_id(): + """Helper function for generate a sac report id""" return generate_sac_report_id(end_date=datetime.now().date().isoformat()) @@ -134,6 +134,7 @@ def _merge_dict_seq(seq): def _build_auditor_cert_dict(certification: dict, signature: dict) -> dict: + """Helper function for building a dictionary for auditor certification""" return { "auditor_certification": certification, "auditor_signature": signature, @@ -141,6 +142,7 @@ def _build_auditor_cert_dict(certification: dict, signature: dict) -> dict: def _build_auditee_cert_dict(certification: dict, signature: dict) -> dict: + """Helper function for building a dictionary for auditee certification""" return { "auditee_certification": certification, "auditee_signature": signature, @@ -217,21 +219,25 @@ def test_no_robots(self): class MySubmissionsViewTests(TestCase): def setUp(self): + """Setup function for users and client""" self.user = baker.make(User) self.user2 = baker.make(User) self.client = Client() def test_redirect_if_not_logged_in(self): + """Test that accessing submission page redirects if user is not logged in""" result = self.client.get(SUBMISSIONS_PATH) self.assertAlmostEqual(result.status_code, 302) def test_no_submissions_returns_empty_list(self): + """Test that an authenticated user with no submissions gets empty list""" self.client.force_login(user=self.user) data = MySubmissions.fetch_my_submissions(self.user) self.assertEqual(len(data), 0) def test_user_with_submissions_should_return_expected_data_columns(self): + """Test that a user with submissions gets data with expected columns""" self.client.force_login(user=self.user) self.user.profile.entry_form_data = ( VALID_ELIGIBILITY_DATA | VALID_AUDITEE_INFO_DATA @@ -251,6 +257,7 @@ def test_user_with_submissions_should_return_expected_data_columns(self): self.assertTrue("fiscal_year_end_date" in keys) def test_user_with_no_submissions_should_return_no_data(self): + """Test that another user with no submissions gets no data""" self.client.force_login(user=self.user) self.user.profile.entry_form_data = ( VALID_ELIGIBILITY_DATA | VALID_AUDITEE_INFO_DATA @@ -264,9 +271,40 @@ def test_user_with_no_submissions_should_return_no_data(self): class EditSubmissionViewTests(TestCase): - def test_redirect_if_not_logged_in(self): - result = self.client.get(reverse(EDIT_PATH, args=["SOME_REPORT_ID"])) - self.assertAlmostEqual(result.status_code, 302) + def setUp(self): + """Setup test factory, client, user, and report ID""" + self.factory = RequestFactory() + self.client = Client() + self.user = baker.make(User) + self.sac = baker.make( + SingleAuditChecklist, submission_status=STATUS.READY_FOR_CERTIFICATION + ) + self.url_name = "audit:EditSubmission" + self.report_id = "TEST_REPORT_ID" + self.url = reverse( + "audit:EditSubmission", kwargs={"report_id": self.sac.report_id} + ) + self.client.force_login(self.user) + baker.make( + "audit.Access", + sac=self.sac, + user=self.user, + role="certifying_auditee_contact", + ) + self.session = self.client.session + + def test_redirect_not_logged_in(self): + """Test that accessing edit submission page redirects if not authenticated""" + url = reverse(self.url_name, args=[self.report_id]) + response = self.client.get(url) + self.assertEqual(response.status_code, 302) + + def test_redirects_to_singleauditchecklist(self): + """Test that accessing edit submission redirects to SAC view""" + response = self.client.get(self.url) + self.assertRedirects( + response, reverse("singleauditchecklist", args=[self.sac.report_id]) + ) class SubmissionViewTests(TestCase): @@ -274,6 +312,115 @@ class SubmissionViewTests(TestCase): Testing for the final step: submitting. """ + def setUp(self): + """Set up test client, user, SAC, and URL""" + self.client = Client() + self.user = baker.make(User) + self.sac = baker.make( + SingleAuditChecklist, submission_status=STATUS.AUDITEE_CERTIFIED + ) + self.url = reverse("audit:Submission", kwargs={"report_id": self.sac.report_id}) + self.client.force_login(self.user) + baker.make( + "audit.Access", + sac=self.sac, + user=self.user, + role="certifying_auditee_contact", + ) + + def test_get_renders_template(self): + """Test that GET renders the submission template with correct context""" + response = self.client.get(self.url) + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, "audit/submission.html") + self.assertIn("report_id", response.context) + self.assertIn("submission_status", response.context) + self.assertEqual(response.context["report_id"], self.sac.report_id) + self.assertEqual( + response.context["submission_status"], self.sac.submission_status + ) + + def test_get_permission_denied_if_no_sac(self): + """Test that GET returns 403 if SAC does not exist""" + invalid_url = reverse("audit:Submission", kwargs={"report_id": "INVALID"}) + response = self.client.get(invalid_url) + self.assertEqual(response.status_code, 403) + + def test_get_access_denied_for_unauthorized_user(self): + """Test that GET returns 403 if user is unauthorized""" + self.client.logout() + response = self.client.get(self.url) + self.assertEqual(response.status_code, 403) + + @patch("audit.models.SingleAuditChecklist.validate_full") + @patch("audit.views.views.sac_transition") + @patch("audit.views.views.remove_workbook_artifacts") + @patch("audit.views.views.SingleAuditChecklist.disseminate") + def test_post_successful( + self, mock_disseminate, mock_remove, mock_transition, mock_validate + ): + """Test that a valid submission transitions SAC to a disseminated state""" + mock_validate.return_value = [] + mock_disseminate.return_value = None + response = self.client.post(self.url) + + mock_validate.assert_called_once() + mock_disseminate.assert_called_once() + mock_transition.assert_called_with( + response.wsgi_request, self.sac, transition_to=STATUS.DISSEMINATED + ) + mock_remove.assert_called_once() + + self.assertEqual(response.status_code, 302) + self.assertRedirects(response, reverse("audit:MySubmissions")) + + @patch("audit.views.views.SingleAuditChecklist.validate_full") + @patch("audit.views.views.sac_transition") + @patch("audit.views.views.SingleAuditChecklist.disseminate") + def test_post_validation_errors( + self, mock_disseminate, mock_transition, mock_validate + ): + """Test that validation errors are displayed if submission is invalid""" + mock_validate.return_value = ["Error 1", "Error 2"] + + self.sac.submission_status = STATUS.AUDITEE_CERTIFIED + self.sac.save() + + response = self.client.post(self.url) + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed( + response, "audit/cross-validation/cross-validation-results.html" + ) + self.assertIn("errors", response.context) + self.assertListEqual(response.context["errors"], ["Error 1", "Error 2"]) + + mock_disseminate.assert_not_called() + mock_transition.assert_not_called() + + @patch("audit.views.views.General.objects.get") + @patch("audit.views.views.SingleAuditChecklist.validate_full") + def test_post_transaction_error(self, mock_validate, mock_general_get): + """Test that a transaction error during a submission is handled properly""" + self.sac.submission_status = STATUS.AUDITEE_CERTIFIED + self.sac.save() + + mock_validate.return_value = [] + mock_general_get.return_value = True + + response = self.client.post(self.url) + + self.assertEqual(response.status_code, 302) + self.assertRedirects(response, reverse("audit:MySubmissions")) + + self.assertEqual(response.status_code, 302) + self.assertRedirects(response, reverse("audit:MySubmissions")) + + def test_post_permission_denied_if_no_sac(self): + """Test that POST returns 403 if SAC does not exist""" + invalid_url = reverse("audit:Submission", kwargs={"report_id": "INVALID"}) + response = self.client.post(invalid_url) + self.assertEqual(response.status_code, 403) + def test_post_redirect(self): """ The status should be "disseminated" after the post. @@ -330,6 +477,26 @@ def test_post_redirect(self): self.assertEqual(sac_after.submission_status, STATUSES.DISSEMINATED) +class SubmissionGetTest(TestCase): + def setUp(self): + """Setup test user and client""" + self.user = baker.make(User) + self.client = Client() + self.url = reverse("audit:MySubmissions") + + def testValidSubmission(self): + """Test that a valid submission is displayed on the submissions page""" + self.client.force_login(self.user) + sac1 = baker.make(SingleAuditChecklist, submission_status=STATUS.IN_PROGRESS) + sac2 = baker.make(SingleAuditChecklist, submission_status=STATUS.DISSEMINATED) + baker.make(Access, user=self.user, sac=sac1) + baker.make(Access, user=self.user, sac=sac2) + + response = self.client.get(self.url) + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, "audit/my_submissions.html") + + class SubmissionStatusTests(TransactionTestCase): """ Tests the expected order of progression for submission_status. @@ -342,6 +509,7 @@ class SubmissionStatusTests(TransactionTestCase): """ def setUp(self): + """Setup user and client""" self.user = baker.make(User) self.client = Client() @@ -1348,6 +1516,34 @@ def test_late_file_upload(self, mock_scan_file): "no_late_changes", response.content.decode("utf-8") ) + def test_get_login_required(self): + """Test that uploading files requires user authentication""" + for form_section in FORM_SECTIONS: + response = self.client.get( + reverse( + f"audit:{form_section}", + kwargs={"report_id": "12345", "form_section": form_section}, + ) + ) + self.assertEqual(response.status_code, 403) + + def test_get_bad_report_id_returns_403(self): + """Test that uploading with a malformed or nonexistant report_id reutrns 403""" + user = baker.make(User) + self.client.force_login(user) + + for form_section in FORM_SECTIONS: + response = self.client.get( + reverse( + f"audit:{form_section}", + kwargs={ + "report_id": "this is not a report id", + "form_section": form_section, + }, + ) + ) + self.assertEqual(response.status_code, 403) + class SingleAuditReportFileHandlerViewTests(TestCase): def test_login_required(self): @@ -1411,6 +1607,7 @@ def test_no_file_attached_returns_400(self): @patch("audit.validators._scan_file") def test_valid_file_upload(self, mock_scan_file): + """Test that uploading a valid SAR update the SAC accordingly""" sac = _mock_login_and_scan( self.client, mock_scan_file, @@ -1656,3 +1853,341 @@ def test_valid_file_upload_for_notes_to_sefa(self, mock_scan_file): submission_events[event_count - 1].event, SubmissionEvent.EventType.NOTES_TO_SEFA_UPDATED, ) + + +class EditSubmissionTest(TestCase): + def setUp(self): + """Setup factory, client, user, SAC, and URL""" + self.factory = RequestFactory() + self.client = Client() + self.user = baker.make(User) + self.sac = baker.make( + SingleAuditChecklist, submission_status=STATUS.READY_FOR_CERTIFICATION + ) + self.url = reverse( + "audit:EditSubmission", kwargs={"report_id": self.sac.report_id} + ) + self.client.force_login(self.user) + baker.make( + "audit.Access", + sac=self.sac, + user=self.user, + role="certifying_auditee_contact", + ) + self.session = self.client.session + + def test_redirects_to_singleauditchecklist(self): + """Test that accessing edit submission redirects to SAC view""" + response = self.client.get(self.url) + self.assertRedirects( + response, reverse("singleauditchecklist", args=[self.sac.report_id]) + ) + + +class AuditorCertificationStep1ViewTests(TestCase): + def setUp(self): + """Setup client, user, SAC, and URL""" + self.client = Client() + self.user = baker.make(User) + self.sac = baker.make( + SingleAuditChecklist, submission_status=STATUS.READY_FOR_CERTIFICATION + ) + self.url = reverse( + "audit:AuditorCertification", kwargs={"report_id": self.sac.report_id} + ) + self.client.force_login(self.user) + baker.make( + "audit.Access", + sac=self.sac, + user=self.user, + role="certifying_auditor_contact", + ) + self.session = self.client.session + + def test_get_redirects_if_status_not_ready_for_certification(self): + """Test that GET redirects if SAC status is not READY_FOR_CERTIFICATION""" + self.sac.submission_status = STATUS.IN_PROGRESS + self.sac.save() + response = self.client.get(self.url) + self.assertRedirects( + response, f"/audit/submission-progress/{self.sac.report_id}" + ) + + def test_get_renders_template_if_valid_state(self): + """ + Test that GET renders the auditor certification setp 1 template when SAC + is in a valid state. + """ + response = self.client.get(self.url) + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, "audit/auditor-certification-step-1.html") + self.assertIn("form", response.context) + self.assertIsInstance(response.context["form"], AuditorCertificationStep1Form) + + def test_get_permission_denied_if_sac_not_found(self): + """Test that POST redirects if SAC report_id is not found""" + response = self.client.get( + reverse("audit:AuditorCertification", kwargs={"report_id": "INVALID"}) + ) + self.assertEqual(response.status_code, 403) + + def test_post_redirects_if_status_not_ready_for_certification(self): + """Test that POST redirects if SAC status is not READY_FOR_CERTIFICATION""" + self.sac.submission_status = STATUS.IN_PROGRESS + self.sac.save() + response = self.client.post(self.url, {"field": "value"}) + self.assertRedirects( + response, f"/audit/submission-progress/{self.sac.report_id}" + ) + + def test_post_valid_form(self): + """ + Test that submitting a valid form updates the session + and redirects correctly + """ + self.session["AuditorCertificationStep1Session"] = {"field": "value"} + self.session.save() + + form_data = { + "is_OMB_limited": True, + "is_auditee_responsible": True, + "has_used_auditors_report": True, + "has_no_auditee_procedures": True, + "is_FAC_releasable": True, + "is_accurate_and_complete": True, + } + + response = self.client.post(self.url, form_data) + + self.assertEqual(response.status_code, 302) + self.sac.refresh_from_db() + + self.assertRedirects( + response, + reverse("audit:AuditorCertificationConfirm", args=[self.sac.report_id]), + ) + self.assertIn("AuditorCertificationStep1Session", self.client.session) + self.assertEqual( + self.client.session["AuditorCertificationStep1Session"], form_data + ) + + def test_post_invalid_form(self): + """Test that submitting and invalid form renders an error template""" + response = self.client.post(self.url, {"invalid_field": ""}) + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, "audit/auditor-certification-step-1.html") + self.assertIn("form", response.context) + self.assertIsInstance(response.context["form"], AuditorCertificationStep1Form) + self.assertTrue(response.context["form"].errors) + + def test_post_permission_denied_if_sac_not_found(self): + """Test that results in 403 if SAC report_id is not found""" + response = self.client.post( + reverse("audit:AuditorCertification", kwargs={"report_id": "INVALID"}) + ) + self.assertEqual(response.status_code, 403) + + +class AuditeeCertificationStep2ViewTests(TestCase): + def setUp(self): + """Setup client, user, SAC, and URL""" + self.client = Client() + self.user = baker.make(User) + self.sac = baker.make( + SingleAuditChecklist, submission_status=STATUS.AUDITOR_CERTIFIED + ) + self.url = reverse( + "audit:AuditeeCertificationConfirm", + kwargs={"report_id": self.sac.report_id}, + ) + self.client.force_login(self.user) + baker.make( + "audit.Access", + sac=self.sac, + user=self.user, + role="certifying_auditee_contact", + ) + self.session = self.client.session + + def test_get_redirects_if_no_step_1(self): + """Test that GET redirects if AuditeeCertificationStep1Session is missing""" + response = self.client.get(self.url) + self.assertRedirects( + response, reverse("audit:AuditeeCertification", args=[self.sac.report_id]) + ) + + def test_get_renders_template_if_valid_session(self): + """ + Test that GET renders the Auditee certification step 2 + if session is valid + """ + self.session["AuditeeCertificationStep1Session"] = {"field": "value"} + self.session.save() + response = self.client.get(self.url) + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, "audit/auditee-certification-step-2.html") + self.assertIn("form", response.context) + self.assertIsInstance(response.context["form"], AuditeeCertificationStep2Form) + + def test_get_redirects_if_not_auditor_certified(self): + """Test that GET will redirect if SAC status is not AUDITOR_CERTIFIED""" + self.sac.submission_status = STATUS.IN_PROGRESS + self.sac.save() + self.session["AuditeeCertificationStep1Session"] = {"field": "value"} + self.session.save() + response = self.client.get(self.url) + self.assertRedirects( + response, f"/audit/submission-progress/{self.sac.report_id}" + ) + + @patch("audit.views.views.validate_auditee_certification_json") + @patch("audit.views.views.sac_transition") + def test_post_valid_form(self, mock_transition, mock_validate): + """ + Test that submitting a valid Auditee Certification Form + updates the SAC and redirects correctly + """ + mock_transition.return_value = True + mock_validate.return_value = {"auditee_certification": "validated_data"} + + self.session["AuditeeCertificationStep1Session"] = {"field": "value"} + self.session.save() + + self.sac.submission_status = STATUS.AUDITOR_CERTIFIED + self.sac.save() + + response = self.client.post( + self.url, + { + "auditee_certification_date_signed": "2024-01-01", + "auditee_name": "Test Auditee", + "auditee_title": "Auditor", + }, + ) + self.assertEqual(response.status_code, 302) + self.sac.refresh_from_db() + mock_transition.assert_called_once_with( + response.wsgi_request, self.sac, transition_to=STATUS.AUDITEE_CERTIFIED + ) + self.assertRedirects( + response, reverse("audit:SubmissionProgress", args=[self.sac.report_id]) + ) + + def test_post_invalid_form(self): + """ + Test that submitting an invalid Auditee Ceritifcation Form + renders an error template + """ + self.session["AuditeeCertificationStep1Session"] = None + self.session.save() + response = self.client.post(self.url, {"auditee_certification_date_signed": ""}) + + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, "audit/auditee-certification-step-2.html") + self.assertIn("form", response.context) + self.assertIsInstance(response.context["form"], AuditeeCertificationStep2Form) + self.assertTrue(response.context["form"].errors) + + def test_post_redirects_if_status_not_auditor_certified(self): + """Test that POST will redirect if SAC submission status is not AUDITOR_CERTIFIED""" + self.sac.submission_status = STATUS.IN_PROGRESS + self.sac.save() + self.session["AuditeeCertificationStep1Session"] = {"field": "value"} + self.session.save() + response = self.client.post( + self.url, {"auditee_certification_date_signed": "2024-01-01"} + ) + self.assertRedirects( + response, f"/audit/submission-progress/{self.sac.report_id}" + ) + + def test_post_permission_denied_if_sac_not_found(self): + """Test that POST will result in permission error is SAC report_id not found""" + response = self.client.post( + reverse( + "audit:AuditeeCertificationConfirm", kwargs={"report_id": "INVALID"} + ) + ) + self.assertEqual(response.status_code, 403) + + def test_single_audit_checklist_does_not_exist_exception(self): + """Test that SAC does not exist renders a unique exception.""" + factory = RequestFactory() + request = factory.get(reverse("audit:AuditeeCertification", args=["12345"])) + + with patch("audit.models.SingleAuditChecklist.objects.get") as mock_get: + mock_get.side_effect = SingleAuditChecklist.DoesNotExist + + view = AuditeeCertificationStep2View.as_view() + + with self.assertRaises(PermissionDenied) as context: + view(request, report_id="12345") + + self.assertEqual( + str(context.exception), "You do not have access to this audit." + ) + + +class CrossValidationViewTests(TestCase): + def setUp(self): + """Setup client, user, SAC, and URL""" + self.client = Client() + self.user = baker.make(User) + self.sac = baker.make( + SingleAuditChecklist, + report_id="test-report-id", + submission_status=STATUS.IN_PROGRESS, + general_information={"auditee_fiscal_period_end": "2024-12-31"}, + ) + self.url = reverse( + "audit:CrossValidation", kwargs={"report_id": self.sac.report_id} + ) + self.client.force_login(self.user) + baker.make( + "audit.Access", + sac=self.sac, + user=self.user, + role="certifying_auditee_contact", + ) + + def test_get_view_renders_correct_template(self): + """Test that GET renders cross-validation template with correct context""" + response = self.client.get(self.url) + + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed( + response, "audit/cross-validation/cross-validation.html" + ) + self.assertEqual(response.context["report_id"], self.sac.report_id) + self.assertEqual( + response.context["submission_status"], self.sac.submission_status + ) + + def test_get_view_permission_denied(self): + """Test that GET returns 403 if SAC report_id does not exist""" + url = reverse("audit:CrossValidation", args=["non-existent-id"]) + response = self.client.get(url) + + self.assertEqual(response.status_code, 403) # PermissionDenied results in 403 + + @patch("audit.models.SingleAuditChecklist.validate_full") + def test_post_view_renders_results_template(self, mock_validate_full): + """Test that POST with validation errors renders template with errors""" + mock_validate_full.return_value = ["Error 1", "Error 2"] + + response = self.client.post(self.url) + + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed( + response, "audit/cross-validation/cross-validation-results.html" + ) + self.assertEqual(response.context["report_id"], self.sac.report_id) + self.assertEqual(response.context["errors"], ["Error 1", "Error 2"]) + mock_validate_full.assert_called_once() + + def test_post_view_permission_denied(self): + """Test that POST returns 403 if SAC report_id does not exist""" + url = reverse("audit:CrossValidation", args=["non-existent-id"]) + response = self.client.post(url) + + self.assertEqual(response.status_code, 403)