diff --git a/.github/ISSUE_TEMPLATE/api-access-template.yaml b/.github/ISSUE_TEMPLATE/api-access-template.yaml index 10207c818c..0a0e313118 100644 --- a/.github/ISSUE_TEMPLATE/api-access-template.yaml +++ b/.github/ISSUE_TEMPLATE/api-access-template.yaml @@ -27,6 +27,29 @@ body: default: 0 validations: required: true + - type: markdown + attributes: + value: | + What kind of key is being requested? + + A *personal* key is one that will be used by an individual developer + on agency-issued GFE. + + A *system* key is one that will be used to communicate with the FAC + as part of a FISMA-Moderate system that has an active ATO. + + - type: dropdown + id: accesstype + attributes: + label: Key type + description: | + Is a personal or system key being requested? + options: + - Personal key + - System key + default: 0 + validations: + required: true - type: input id: email attributes: diff --git a/.github/ISSUE_TEMPLATE/cross-validation.md b/.github/ISSUE_TEMPLATE/cross-validation.md deleted file mode 100644 index 6d5802c854..0000000000 --- a/.github/ISSUE_TEMPLATE/cross-validation.md +++ /dev/null @@ -1,71 +0,0 @@ ---- -name: Cross-validation template -about: Starting point for writing individual cross-validation functions. -title: 'Cross-validation: short summary' -labels: '' -assignees: '' ---- - -## Validation Description - -[//]: # (A description of the validalidation in plain language.) - -## Section(s): -- [ ] This validation touches ALL sections - -[//]: # (Spacing) - -  - -[//]: # (Spacing) - -- [ ] Pre-SAC -- [ ] General Information -- [ ] Audit Information -- [ ] Federal Awards -- [ ] Audit Findings -- [ ] Audit findings text -- [ ] CAP Text -- [ ] Additional UEIs -- [ ] Additional Auditors -- [ ] Audit package (PDF) -- [ ] Tribal data consent -- [ ] Auditee certification -- [ ] Auditor certification - -## Error(s) - -- [ ] This generates one error -- [ ] This possibly generates multiple errors - - -### Error text - -[//]: # (Use Python format string syntax. E.g. f"The section {general-information} is missing everything.") - -```python -f"Sad Mac" -``` - -### Error number - -[//]: # (Short identifier followed by timestamp. E.g. UEI_202307050927) - -``` -SADMAC_202307071352 -``` - -## Notes - -[//]: # (Any additional notes for implementation.) - - -```[tasklist] -### Tasks -- [ ] Error text is edited and approved by @lauraherring -- [ ] Validation is implemented -- [ ] When data is correct, validation produces no errors -- [ ] When data is incorrect, validation produces expected errors. -``` - - diff --git a/.github/ISSUE_TEMPLATE/design-issue-template.md b/.github/ISSUE_TEMPLATE/design-issue-template.md deleted file mode 100644 index 35560d6d37..0000000000 --- a/.github/ISSUE_TEMPLATE/design-issue-template.md +++ /dev/null @@ -1,35 +0,0 @@ ---- -name: Design issue template. -about: Starting point for new design issues. -title: '' -labels: 'design' -assignees: '' - ---- - -Breakout issue of... - -This issue covers the design of... - -### Background - -Example - -### Acceptance criteria (We'll know we're done when...) - -- [ ] Example - -### Tasks - -- [ ] Design -- [ ] Peer review -- [ ] Write implementation notes - -### Definition of Done - -- [ ] Screens in Figma are ready to be handed off to front end -- [x] ~All text has a contrast ratio of 4.5:1 with the background~ (Built into USWDS) -- [ ] Interaction are documented if they deviate from USWDS components -- [ ] The needs of someone navigating the site with the use of a screenreader or keyboard has been considered and documented -- [x] ~There is an option for multilingual support~ (We've established multilingual support in the header) -- [ ] There is supporting documentation for alt text, labels for links (anchor + target), error text in forms, tab order, focus order, and what assistive technologies should announce (if applicable) diff --git a/.github/ISSUE_TEMPLATE/epic-template.md b/.github/ISSUE_TEMPLATE/epic-template.md index 64b08d6c82..893c4950a1 100644 --- a/.github/ISSUE_TEMPLATE/epic-template.md +++ b/.github/ISSUE_TEMPLATE/epic-template.md @@ -11,3 +11,26 @@ ### Stories - [ ] ``` + + +# Acceptance Criteria + +### Scenario: + +**Given** +**when** +... + +[comment]: # "Each task should be a verifiable outcome" +```[tasklist] +### then... +- [ ] [a thing happens] +``` + +# Security Considerations + +Required per [CM-4](https://nvd.nist.gov/800-53/Rev4/control/CM-4). + +[comment]: # "Our SSP says 'The team ensures security implications are considered as part of the agile requirements refinement process by including a section in the issue template used as a basis for new work.'" +[comment]: # "Please do not remove this section without care." +[comment]: # "Note any security concerns that might be implicated in the change. 'None' is OK, but we must be explicit here." diff --git a/.github/ISSUE_TEMPLATE/snip-acceptance-criteria.md b/.github/ISSUE_TEMPLATE/snip-acceptance-criteria.md deleted file mode 100644 index 11fe918999..0000000000 --- a/.github/ISSUE_TEMPLATE/snip-acceptance-criteria.md +++ /dev/null @@ -1,32 +0,0 @@ ---- -name: "Snip: Acceptance Criteria" -about: Starting point for a story acceptance. -title: 'snip-acceptance-criteria' -labels: '' -assignees: '' ---- - -# Acceptance Criteria - -We use [DRY](https://docs.behat.org/en/latest/user_guide/writing_scenarios.html#backgrounds) [behavior-driven development](https://en.wikipedia.org/wiki/Behavior-driven_development#Behavioral_specifications) wherever possible. - -[comment]: # "ACs should be clearly demoable/verifiable whenever possible." -[comment]: # "Given: the initial context at the beginning of the scenario" -[comment]: # "when: the event that triggers the scenario" -[comment]: # "then: the expected outcome(s)" -[comment]: # "Repeat scenarios as needed, or repeat behaviors and lists within a scenario as needed." - -[comment]: # "The scenario should be a short, plain language description." -[comment]: # "Feeling repetative? Apply the DRY (Don't Repeat Yourself) principle!" - -### Scenario: - -**Given** -**when** -... - -[comment]: # "Each task should be a verifiable outcome" -```[tasklist] -### then... -- [ ] [a thing happens] -``` diff --git a/.github/ISSUE_TEMPLATE/snip-at-a-glance.md b/.github/ISSUE_TEMPLATE/snip-at-a-glance.md deleted file mode 100644 index 93e50f5618..0000000000 --- a/.github/ISSUE_TEMPLATE/snip-at-a-glance.md +++ /dev/null @@ -1,19 +0,0 @@ ---- -name: "Snip: At a glance" -about: Starting point for a story. -title: 'snip-at-a-glance' -labels: '' -assignees: '' ---- - -# At a glance - -[comment]: # "Begin with a short summary so intent can be understood at a glance." -[comment]: # "In order to: some objective or value to be achieved" -[comment]: # "as a: stakeholder" -[comment]: # "I want: some new feature" - -**In order to** -**as a** -**I want** - diff --git a/.github/ISSUE_TEMPLATE/snip-background.md b/.github/ISSUE_TEMPLATE/snip-background.md deleted file mode 100644 index c60bce9646..0000000000 --- a/.github/ISSUE_TEMPLATE/snip-background.md +++ /dev/null @@ -1,11 +0,0 @@ ---- -name: "Snip: Background" -about: Starting point for the background on a ticket. -title: 'snip-background' -labels: '' -assignees: '' ---- - -# Background - -[comment]: # "Any helpful contextual notes or links to artifacts/evidence, if needed" diff --git a/.github/ISSUE_TEMPLATE/snip-content-signoff.md b/.github/ISSUE_TEMPLATE/snip-content-signoff.md deleted file mode 100644 index 7ae69dc68b..0000000000 --- a/.github/ISSUE_TEMPLATE/snip-content-signoff.md +++ /dev/null @@ -1,20 +0,0 @@ ---- -name: "Snip: Content signoff" -about: Starting point for a content signoff checklist. -title: 'snip-content-signoff' -labels: 'content' -assignees: '' ---- - -### Content signoff - -[comment]: # "As each step is completed, assign the next team member to this ticket. At-mention (@-mention) them in a comment for visibility." - -```[tasklist] -### Signed off by... -- [ ] Author -- [ ] Content review -- [ ] Content lead -- [ ] Optional: Product -``` - diff --git a/.github/ISSUE_TEMPLATE/snip-scenario.md b/.github/ISSUE_TEMPLATE/snip-scenario.md deleted file mode 100644 index 573c0744c3..0000000000 --- a/.github/ISSUE_TEMPLATE/snip-scenario.md +++ /dev/null @@ -1,22 +0,0 @@ ---- -name: "Snip: Scenario" -about: Starting point for a scenario; sub-item to acceptance. -title: 'snip-scenario' -labels: '' -assignees: '' ---- - -[comment]: # "The scenario should be a short, plain language description." -[comment]: # "Feeling repetative? Apply the DRY (Don't Repeat Yourself) principle!" - -### Scenario: - -**Given** -**when** -... - -[comment]: # "Each task should be a verifiable outcome" -```[tasklist] -### then... -- [ ] [a thing happens] -``` diff --git a/.github/ISSUE_TEMPLATE/snip-security-considerations.md b/.github/ISSUE_TEMPLATE/snip-security-considerations.md deleted file mode 100644 index 68174bfa52..0000000000 --- a/.github/ISSUE_TEMPLATE/snip-security-considerations.md +++ /dev/null @@ -1,15 +0,0 @@ ---- -name: "Snip: Security Considerations" -about: Starting point for security considerations on a piece of work. -title: 'snip-security-considerations' -labels: '' -assignees: '' ---- - -# Security Considerations - -Required per [CM-4](https://nvd.nist.gov/800-53/Rev4/control/CM-4). - -[comment]: # "Our SSP says 'The team ensures security implications are considered as part of the agile requirements refinement process by including a section in the issue template used as a basis for new work.'" -[comment]: # "Please do not remove this section without care." -[comment]: # "Note any security concerns that might be implicated in the change. 'None' is OK, but we must be explicit here." diff --git a/.github/ISSUE_TEMPLATE/snip-shepherd.md b/.github/ISSUE_TEMPLATE/snip-shepherd.md deleted file mode 100644 index 3f45670f9e..0000000000 --- a/.github/ISSUE_TEMPLATE/snip-shepherd.md +++ /dev/null @@ -1,19 +0,0 @@ ---- -name: "Snip: Shepherds" -about: Starting point for who will shepherd work. -title: 'snip-shepherds' -labels: '' -assignees: '' ---- - -### Shepherds - -[comment]: # "@ mention shepherds as we move across the board." -[comment]: # "Add/remove as needed" - -* Content shepherd: -* Design shepherd: -* Data shepherd: -* Infrastructure shepherd: -* Engineering shepherd: - diff --git a/.github/ISSUE_TEMPLATE/snip-story-process.md b/.github/ISSUE_TEMPLATE/snip-story-process.md deleted file mode 100644 index 16d95a9584..0000000000 --- a/.github/ISSUE_TEMPLATE/snip-story-process.md +++ /dev/null @@ -1,64 +0,0 @@ ---- -name: "Snip: Process Checklist" -about: Starting point for a process checklist. -title: 'snip-process-checklist' -labels: '' -assignees: '' ---- - - -

Process Checklist

-

How it moves across the board...

- -
- Process checklist - -# Sketch - -[comment]: # "Notes or a checklist reflecting our understanding of the selected approach" - -Team members who will likely need to be involved in doing all the things: - -- [ ] Content -- [ ] Data -- [ ] Design -- [ ] Engineering -- [ ] Infrastructure -- [ ] Product - -# Definition of Done - -## Triage - -### If not likely to be important in the next quarter... -- [ ] Archived from the board - -### Otherwise... - -- [ ] Has a clear story statement -- [ ] Product team moves it to the appropriate backlog - -## Backlog - -- [ ] Has clearly stated/testable acceptance criteria -- [ ] One or more shepherds have been identified - -## In Progress - -- [ ] Meets the acceptance criteria -- [ ] (As appropriate) Is relabeled and triaged for movement from design to engineering, etc. - -## Review Needed - -- [ ] Necessary outside review/sign-off was provided - -## Done - -- [ ] Includes screenshots or references to artifacts - -### If there's UI... -- [ ] Screen reader - Listen to the experience with a screen reader extension, ensure the information presented in order -- [ ] Keyboard navigation - Run through acceptance criteria with keyboard tabs, ensure it works. -- [ ] Text scaling - Adjust viewport to 1280 pixels wide and zoom to 200%, ensure everything renders as expected. Document 400% zoom issues with USWDS if appropriate. - -
diff --git a/backend/audit/test_manage_submission_access_view.py b/backend/audit/test_manage_submission_access_view.py index d3f3400529..8770dacbdd 100644 --- a/backend/audit/test_manage_submission_access_view.py +++ b/backend/audit/test_manage_submission_access_view.py @@ -131,6 +131,7 @@ def test_basic_get(self): sac.general_information = {"auditee_uei": "YESIAMAREALUEI"} sac.save() current_cac = baker.make(Access, sac=sac, role=self.role) + current_role = str(current_cac.get_friendly_role()) self.client.force_login(user) url = reverse(self.view, kwargs={"report_id": sac.report_id}) @@ -139,6 +140,25 @@ def test_basic_get(self): self.assertEqual(response.status_code, 200) self.assertIn("YESIAMAREALUEI", response.content.decode("UTF-8")) self.assertIn(current_cac.email, response.content.decode("UTF-8")) + self.assertIn(current_role, response.content.decode("UTF-8")) + + def test_no_existing_role_get(self): + """ + A user should be able to access this page for a SAC they're associated with + even if there's no current assignment for the role. + """ + user, sac = _make_user_and_sac() + baker.make(Access, user=user, sac=sac, role="editor") + sac.general_information = {"auditee_uei": "YESIAMAREALUEI"} + sac.save() + + self.client.force_login(user) + url = reverse(self.view, kwargs={"report_id": sac.report_id}) + response = self.client.get(url) + + self.assertEqual(response.status_code, 200) + self.assertIn("YESIAMAREALUEI", response.content.decode("UTF-8")) + self.assertIn("UNASSIGNED ROLE", response.content.decode("UTF-8")) def test_basic_post(self): """ @@ -180,6 +200,41 @@ def test_basic_post(self): ) self.assertEqual(self.role, oldaccess.role) + def test_no_existing_role_post(self): + """ + Submitting the form with a new email address should delete the existing + Access, create a DeletedAccess, and create a new Access. + + However, if we have no existing Access, we should still create a new Access. + """ + user = baker.make(User, email="removing_user@example.com") + sac = baker.make(SingleAuditChecklist) + baker.make(Access, user=user, sac=sac, role="editor") + baker.make( + Access, + sac=sac, + role=self.other_role, + email="contact@example.com", + ) + sac.general_information = {"auditee_uei": "YESIAMAREALUEI"} + sac.save() + + self.client.force_login(user) + + data = { + "fullname": "The New CAC", + "email": "newcacuser@example.com", + } + + url = reverse(self.view, kwargs={"report_id": sac.report_id}) + response = self.client.post(url, data=data) + self.assertEqual(302, response.status_code) + + newaccess = Access.objects.get( + sac=sac, fullname=data["fullname"], email=data["email"] + ) + self.assertEqual(self.role, newaccess.role) + def test_bad_email_post(self): """ Submitting an email address that's already in use for the other role should diff --git a/backend/audit/views/manage_submission.py b/backend/audit/views/manage_submission.py index bc99188274..dd55a9aeab 100644 --- a/backend/audit/views/manage_submission.py +++ b/backend/audit/views/manage_submission.py @@ -6,11 +6,16 @@ SingleAuditChecklistAccessRequiredMixin, ) from audit.models import ( + ACCESS_ROLES, Access, SingleAuditChecklist, ) +def _get_friendly_role(role): + return dict(ACCESS_ROLES)[role] + + class ManageSubmissionView(SingleAuditChecklistAccessRequiredMixin, generic.View): """ View for managing a submission. @@ -43,7 +48,7 @@ def _user_entry(access: Access) -> dict: return { "name": access.fullname, "email": access.email, - "role": access.get_friendly_role(), + "role": _get_friendly_role(access.role), "user_exists": bool(access.user), "never_logged_in_flag": "" if bool(access.user) else "*", } @@ -55,11 +60,26 @@ def _get_url_from_viewname(viewname: str, report_id: str | None = None) -> str: report_id = kwargs["report_id"] sac = SingleAuditChecklist.objects.get(report_id=report_id) accesses = Access.objects.filter(sac=sac).order_by("role") - entries = map(_user_entry, accesses) + base_entries = list(map(_user_entry, accesses)) period_start = sac.general_information.get("auditee_fiscal_period_start") period_end = sac.general_information.get("auditee_fiscal_period_end") _url = partial(_get_url_from_viewname, report_id=report_id) + # Handle missing certifier roles: + entries_to_add = [] + for role in ("certifying_auditee_contact", "certifying_auditor_contact"): + frole = _get_friendly_role(role) + if not [row for row in base_entries if row["role"] == frole]: + entry = { + "name": "UNASSIGNED ROLE", + "email": "UNASSIGNED ROLE", + "role": frole, + "user_exists": False, + "never_logged_in_flag": "*", + } + entries_to_add.append(entry) + entries = base_entries + entries_to_add + context = { "auditee_uei": sac.general_information["auditee_uei"], "auditee_name": sac.general_information.get("auditee_name"), diff --git a/backend/audit/views/manage_submission_access.py b/backend/audit/views/manage_submission_access.py index 66f48af016..efbc455c65 100644 --- a/backend/audit/views/manage_submission_access.py +++ b/backend/audit/views/manage_submission_access.py @@ -1,3 +1,4 @@ +from types import SimpleNamespace from django import forms from django.db import transaction from django.shortcuts import redirect, render, reverse @@ -7,11 +8,20 @@ SingleAuditChecklistAccessRequiredMixin, ) from audit.models import ( + ACCESS_ROLES, Access, SingleAuditChecklist, ) +def _get_friendly_role(role): + return dict(ACCESS_ROLES)[role] + + +def _create_and_save_access(sac, role, fullname, email): + Access(sac=sac, role=role, fullname=fullname, email=email).save() + + class ChangeAccessForm(forms.Form): """ Form for changing access. The view class, not this class, has the responsibility for handling whether we’re deleting access (in the cases where only one user can have the role) or adding access (in the cases where multiple users can have the role). @@ -36,6 +46,7 @@ class ChangeOrAddRoleView(SingleAuditChecklistAccessRequiredMixin, generic.View) role = "editor" other_role = "" + template = "audit/manage-submission-change-access.html" def get(self, request, *args, **kwargs): """ @@ -54,14 +65,20 @@ def get(self, request, *args, **kwargs): "errors": [], } if self.role != "editor": - access = Access.objects.get(sac=sac, role=self.role) + try: + access = Access.objects.get(sac=sac, role=self.role) + except Access.DoesNotExist: + access = SimpleNamespace( + fullname="UNASSIGNED ROLE", email="UNASSIGNED ROLE", role=self.role + ) + context = context | { - "friendly_role": access.get_friendly_role(), + "friendly_role": _get_friendly_role(access.role), "certifier_name": access.fullname, "email": access.email, } - return render(request, "audit/manage-submission-change-access.html", context) + return render(request, self.template, context) def post(self, request, *args, **kwargs): """ @@ -73,22 +90,33 @@ def post(self, request, *args, **kwargs): form = ChangeAccessForm(request.POST) form.full_clean() url = reverse("audit:ManageSubmission", kwargs={"report_id": report_id}) + fullname = form.cleaned_data["fullname"] + email = form.cleaned_data["email"] + # Only if we have self.other_role do we need further checks: if not self.other_role: - Access( - sac=sac, - role=self.role, - fullname=form.cleaned_data["fullname"], - email=form.cleaned_data["email"], - ).save() + _create_and_save_access(sac, self.role, fullname, email) return redirect(url) - access = Access.objects.get(sac=sac, role=self.role) - other_access = Access.objects.get(sac=sac, role=self.other_role) - if form.cleaned_data["email"] == other_access.email: + # We need the existing role assignment, if any, to delete it: + try: + access = Access.objects.get(sac=sac, role=self.role) + except Access.DoesNotExist: + access = SimpleNamespace( + fullname="UNASSIGNED ROLE", email="UNASSIGNED ROLE", role=self.role + ) + + # We need the other role assignment, if any, to compare email addresses: + try: + other_access = Access.objects.get(sac=sac, role=self.other_role) + other_access_email = other_access.email + except Access.DoesNotExist: + other_access_email = None + + if email == other_access_email: context = { "role": self.role, - "friendly_role": access.get_friendly_role(), + "friendly_role": _get_friendly_role(self.role), "auditee_uei": sac.general_information["auditee_uei"], "auditee_name": sac.general_information.get("auditee_name"), "certifier_name": access.fullname, @@ -98,22 +126,18 @@ def post(self, request, *args, **kwargs): "Cannot use the same email address for both certifying officials." ], } - return render( - request, - "audit/manage-submission-change-access.html", - context, - status=400, - ) - - with transaction.atomic(): - access.delete(removing_user=request.user, removal_event="access-change") - - Access( - sac=sac, - role=self.role, - fullname=form.cleaned_data["fullname"], - email=form.cleaned_data["email"], - ).save() + return render(request, self.template, context, status=400) + + # Only Access, and not SimpleNamespace, has .delete: + if hasattr(access, "delete"): + with transaction.atomic(): + access.delete(removing_user=request.user, removal_event="access-change") + _create_and_save_access(sac, self.role, fullname, email) + # We know that submissions can get into a bad state where no + # certifying role(s) exist, so we should support cases where this + # happens: + else: + _create_and_save_access(sac, self.role, fullname, email) return redirect(url) diff --git a/backend/config/settings.py b/backend/config/settings.py index 9961a037a6..fae719f140 100644 --- a/backend/config/settings.py +++ b/backend/config/settings.py @@ -531,8 +531,10 @@ OMB_EXP_DATE = "09/30/2026" # APP-level constants -DOLLAR_THRESHOLD = 750000 CENSUS_DATA_SOURCE = "CENSUS" +GSA_MIGRATION = "GSA_MIGRATION" # There is a copy of `GSA_MIGRATION` in Base.libsonnet. If you change it here, change it there too. +DOLLAR_THRESHOLD = 750000 +SUMMARY_REPORT_DOWNLOAD_LIMIT = 1000 # A version of these regexes also exists in Base.libsonnet REGEX_ALN_PREFIX = r"^([0-9]{2})$" diff --git a/backend/dissemination/search.py b/backend/dissemination/search.py index 4ebfd8c6d6..2e7a3300d6 100644 --- a/backend/dissemination/search.py +++ b/backend/dissemination/search.py @@ -36,6 +36,10 @@ def search_general( order_by=ORDER_BY.fac_accepted_date, order_direction=DIRECTION.ascending, ): + """ + Given any (or no) search fields, build and execute a query on the General table and return the results. + Empty searches return everything. + """ if not order_by: order_by = ORDER_BY.fac_accepted_date if not order_direction: diff --git a/backend/dissemination/summary_reports.py b/backend/dissemination/summary_reports.py index 33bb5ab26f..6f7eea4234 100644 --- a/backend/dissemination/summary_reports.py +++ b/backend/dissemination/summary_reports.py @@ -281,6 +281,15 @@ def insert_precert_coversheet(workbook): def insert_dissem_coversheet(workbook): sheet = workbook.create_sheet("Coversheet", 0) sheet.append(["Time created", datetime.utcnow().strftime("%Y-%m-%d %H:%M:%S")]) + sheet.append( + [ + "Note", + f"This spreadsheet contains the first {settings.SUMMARY_REPORT_DOWNLOAD_LIMIT} results of your search. If you need to download more than {settings.SUMMARY_REPORT_DOWNLOAD_LIMIT} submissions, try limiting your search parameters to download in batches.", + ] + ) + # Uncomment if we want to link to the FAC API for larger data dumps. + # sheet.cell(row=3, column=2).value = "FAC API Link" + # sheet.cell(row=3, column=2).hyperlink = f"{settings.STATIC_SITE_URL}/developers/" set_column_widths(sheet) @@ -317,12 +326,11 @@ def gather_report_data_dissemination(report_ids): data[model_name] = {"field_names": field_names, "entries": []} - for report_id in report_ids: - objects = model.objects.all().filter(report_id=report_id) - for obj in objects: - data[model_name]["entries"].append( - [getattr(obj, field_name) for field_name in field_names] - ) + objects = model.objects.all().filter(report_id__in=report_ids) + for obj in objects: + data[model_name]["entries"].append( + [getattr(obj, field_name) for field_name in field_names] + ) return data diff --git a/backend/dissemination/templates/search-alert-info.html b/backend/dissemination/templates/search-alert-info.html new file mode 100644 index 0000000000..2c27829c95 --- /dev/null +++ b/backend/dissemination/templates/search-alert-info.html @@ -0,0 +1,21 @@ +{% load humanize %} +{% load sprite_helper %} +
+
+

Searching the FAC database

+

+ Learn more about how our search filters work on our Search Resources page. +

+ +

Sorting

+

+ Use the arrows at the top of each column to sort results. +

+ +

Summary reports

+

+ For search results of {{ summary_report_download_limit|intcomma }} submissions or less, you can download a combined spreadsheet of all data. + If you need to download more than {{ summary_report_download_limit|intcomma }} submissions, try limiting your search parameters to download in batches. +

+
+
diff --git a/backend/dissemination/templates/search.html b/backend/dissemination/templates/search.html index 2162f1324a..a7bc50e2f9 100644 --- a/backend/dissemination/templates/search.html +++ b/backend/dissemination/templates/search.html @@ -170,27 +170,36 @@

Filters

Search single audit reports

{% if results|length > 0 %} -
-
-

Sorting

-

- Use the arrows at the top of each column to sort results. -

-
+ {% include "search-alert-info.html"%} +
+

+ Results: {{ results_count }} + showing {{ limit }} per page +

+ {% if results_count <= summary_report_download_limit %} + + {% endif %}
-

- Results: {{ results_count }} - showing {{ limit }} per page -

+
- {% include "search-table-header.html" with friendly_title="Name" field_name="auditee_name"%} - {% include "search-table-header.html" with friendly_title="UEI or EIN" field_name="auditee_uei"%} - {% include "search-table-header.html" with friendly_title="Acc Date" field_name="fac_accepted_date"%} - {% include "search-table-header.html" with friendly_title="AY" field_name="audit_year"%} - {% include "search-table-header.html" with friendly_title="Cog or Over" field_name="cog_over"%} + {% include "search-table-header.html" with friendly_title="Name" field_name="auditee_name" %} + {% include "search-table-header.html" with friendly_title="UEI or EIN" field_name="auditee_uei" %} + {% include "search-table-header.html" with friendly_title="Acc Date" field_name="fac_accepted_date" %} + {% include "search-table-header.html" with friendly_title="AY" field_name="audit_year" %} + {% include "search-table-header.html" with friendly_title="Cog or Over" field_name="cog_over" %} {% if results.0.finding_my_aln is not None%} @@ -305,13 +314,8 @@

Sorting

{% elif results is not None %} + {% include "search-alert-info.html"%}
-
-

Searching the FAC database

-

- Learn more about how our search filters work on our Search Resources page. -

-
Searching the FAC database

{% else %} -
-
-

Searching the FAC database

-

- Learn more about how our search filters work on our Search Resources page. -

-
-
+ {% include "search-alert-info.html"%}
an arrow points left, toward the search form diff --git a/backend/dissemination/test_views.py b/backend/dissemination/test_views.py index 35f0e01561..012267a33b 100644 --- a/backend/dissemination/test_views.py +++ b/backend/dissemination/test_views.py @@ -448,3 +448,140 @@ def test_summary_context(self): response.context["data"]["Notes to SEFA"][0]["accounting_policies"], note.accounting_policies, ) + + +class SummaryReportDownloadViewTests(TestCase): + def setUp(self): + self.anon_client = Client() + self.perm_client = Client() + + self.perm_user = baker.make(User) + permission = Permission.objects.get(slug=Permission.PermissionType.READ_TRIBAL) + baker.make( + UserPermission, + email=self.perm_user.email, + user=self.perm_user, + permission=permission, + ) + self.perm_client.force_login(self.perm_user) + + def _make_general(self, is_public=True, **kwargs): + """ + Create a General object in dissemination with the keyword arguments passed in. + """ + general = baker.make( + General, + is_public=is_public, + **kwargs, + ) + return general + + def _summary_report_url(self): + return reverse("dissemination:MultipleSummaryReportDownload") + + def _mock_filename(self): + return "some-report-name.xlsx" + + def _mock_download_url(self): + return "http://example.com/gsa-fac-private-s3/temp/some-report-name.xlsx" + + @patch("dissemination.summary_reports.persist_workbook") + def test_bad_search_returns_400(self, mock_persist_workbook): + """ + Submitting a form with bad parameters should throw a BadRequest. + """ + response = self.anon_client.post( + self._summary_report_url(), {"start_date": "Not a date"} + ) + self.assertEquals(response.status_code, 400) + + @patch("dissemination.summary_reports.persist_workbook") + def test_empty_results_returns_404(self, mock_persist_workbook): + """ + Searches with no results should return a 404, not an empty excel file. + """ + self._make_general(is_public=False, auditee_uei="123456789012") + response = self.anon_client.post( + self._summary_report_url(), {"uei_or_ein": "NotTheOther1"} + ) + self.assertEquals(response.status_code, 404) + + @patch("dissemination.summary_reports.persist_workbook") + def test_no_permissions_returns_404_on_private(self, mock_persist_workbook): + """ + Non-permissioned users cannot access private audits through the summary report post. + """ + self._make_general(is_public=False) + response = self.anon_client.post(self._summary_report_url(), {}) + self.assertEquals(response.status_code, 404) + + @patch("dissemination.views.get_download_url") + @patch("dissemination.summary_reports.persist_workbook") + def test_permissions_returns_file_on_private( + self, mock_persist_workbook, mock_get_download_url + ): + """ + Permissioned users recieve a file if there are private results. + """ + mock_persist_workbook.return_value = self._mock_filename() + mock_get_download_url.return_value = self._mock_download_url() + + self._make_general(is_public=False) + + response = self.perm_client.post(self._summary_report_url(), {}) + self.assertRedirects( + response, + self._mock_download_url(), + status_code=302, + target_status_code=200, + fetch_redirect_response=False, + ) + + @patch("dissemination.views.get_download_url") + @patch("dissemination.summary_reports.persist_workbook") + def test_empty_search_params_returns_file( + self, mock_persist_workbook, mock_get_download_url + ): + """ + File should be generated on empty search parameters ("search all"). + """ + mock_persist_workbook.return_value = self._mock_filename() + mock_get_download_url.return_value = self._mock_download_url() + + self._make_general(is_public=True) + + response = self.anon_client.post(self._summary_report_url(), {}) + self.assertRedirects( + response, + self._mock_download_url(), + status_code=302, + target_status_code=200, + fetch_redirect_response=False, + ) + + @patch("dissemination.views.get_download_url") + @patch("dissemination.summary_reports.persist_workbook") + def test_many_results_returns_file( + self, mock_persist_workbook, mock_get_download_url + ): + """ + File should still be generated if there are above SUMMARY_REPORT_DOWNLOAD_LIMIT total results. + """ + mock_persist_workbook.return_value = self._mock_filename() + mock_get_download_url.return_value = self._mock_download_url() + + for i in range(4): + self._make_general( + is_public=True, + report_id=generate_sac_report_id(end_date="2023-12-31", count=str(i)), + ) + + with self.settings(SUMMARY_REPORT_DOWNLOAD_LIMIT=2): + response = self.anon_client.post(self._summary_report_url(), {}) + self.assertRedirects( + response, + self._mock_download_url(), + status_code=302, + target_status_code=200, + fetch_redirect_response=False, + ) diff --git a/backend/dissemination/urls.py b/backend/dissemination/urls.py index d6b5a631cf..cdd1793ce6 100644 --- a/backend/dissemination/urls.py +++ b/backend/dissemination/urls.py @@ -17,9 +17,14 @@ ), path( "summary-report/xlsx/", - views.SummaryReportDownloadView.as_view(), + views.SingleSummaryReportDownloadView.as_view(), name="SummaryReportDownload", ), + path( + "summary-report/xlsx", + views.MultipleSummaryReportDownloadView.as_view(), + name="MultipleSummaryReportDownload", + ), path("search/", views.Search.as_view(), name="Search"), path("summary/", views.AuditSummaryView.as_view(), name="Summary"), ] diff --git a/backend/dissemination/views.py b/backend/dissemination/views.py index 75c722a6f6..78988318f9 100644 --- a/backend/dissemination/views.py +++ b/backend/dissemination/views.py @@ -1,3 +1,5 @@ +from collections import namedtuple +import logging import math from django.core.exceptions import BadRequest @@ -8,7 +10,7 @@ from audit.models import SingleAuditChecklist -from config.settings import STATE_ABBREVS +from config.settings import STATE_ABBREVS, SUMMARY_REPORT_DOWNLOAD_LIMIT from dissemination.file_downloads import get_download_url, get_filename from dissemination.forms import SearchForm @@ -29,8 +31,13 @@ from users.permissions import can_read_tribal +logger = logging.getLogger(__name__) + def include_private_results(request): + """ + Determine if the user is authenicated to see private data. + """ if not request.user.is_authenticated: return False @@ -40,8 +47,83 @@ def include_private_results(request): return True +def clean_form_data(form): + """ + Given a SearchForm, return a namedtuple with its cleaned and formatted data. + Ideally, this makes accessing form data later a little more readable. + """ + FormData = namedtuple( + "FormData", + "uei_or_eins alns names start_date end_date cog_or_oversight agency_name audit_years auditee_state order_by order_direction limit page", + ) + + if form.is_valid(): + uei_or_eins = form.cleaned_data["uei_or_ein"].splitlines() + alns = form.cleaned_data["aln"].replace(", ", " ").split() + names = form.cleaned_data["entity_name"].splitlines() + start_date = form.cleaned_data["start_date"] + end_date = form.cleaned_data["end_date"] + cog_or_oversight = form.cleaned_data["cog_or_oversight"] + agency_name = form.cleaned_data["agency_name"] + audit_years = [ + int(year) for year in form.cleaned_data["audit_year"] + ] # Cast strings from HTML to int + auditee_state = form.cleaned_data["auditee_state"] + order_by = form.cleaned_data["order_by"] + order_direction = form.cleaned_data["order_direction"] + + # TODO: Add a limit choice field to the form + limit = form.cleaned_data["limit"] or 30 + page = int(form.cleaned_data["page"] or 1) + + form_data = FormData( + uei_or_eins, + alns, + names, + start_date, + end_date, + cog_or_oversight, + agency_name, + audit_years, + auditee_state, + order_by, + order_direction, + limit, + page, + ) + + else: + raise BadRequest("Form data validation error.", form.errors) + + return form_data + + +def run_search_general(form_data, include_private): + """ + Given cleaned form data and an 'include_private' boolean, run the search. + Returns the results QuerySet. + """ + return search_general( + names=form_data.names, + alns=form_data.alns, + uei_or_eins=form_data.uei_or_eins, + start_date=form_data.start_date, + end_date=form_data.end_date, + cog_or_oversight=form_data.cog_or_oversight, + agency_name=form_data.agency_name, + audit_years=form_data.audit_years, + auditee_state=form_data.auditee_state, + include_private=include_private, + order_by=form_data.order_by, + order_direction=form_data.order_direction, + ) + + class Search(View): def get(self, request, *args, **kwargs): + """ + When accessing the search page through get, return the blank search page. + """ form = SearchForm() return render( @@ -54,78 +136,50 @@ def get(self, request, *args, **kwargs): ) def post(self, request, *args, **kwargs): + """ + When accessing the search page through post, run a search and display the results. + """ form = SearchForm(request.POST) results = [] context = {} - if form.is_valid(): - uei_or_eins = form.cleaned_data["uei_or_ein"].splitlines() - alns = form.cleaned_data["aln"].replace(", ", " ").split() - names = form.cleaned_data["entity_name"].splitlines() - start_date = form.cleaned_data["start_date"] - end_date = form.cleaned_data["end_date"] - cog_or_oversight = form.cleaned_data["cog_or_oversight"] - agency_name = form.cleaned_data["agency_name"] - audit_years = [ - int(year) for year in form.cleaned_data["audit_year"] - ] # Cast strings from HTML to int - auditee_state = form.cleaned_data["auditee_state"] - order_by = form.cleaned_data["order_by"] - order_direction = form.cleaned_data["order_direction"] - - # TODO: Add a limit choice field to the form - limit = form.cleaned_data["limit"] or 30 - # Changed in the form via pagination links - page = int(form.cleaned_data["page"] or 1) - - # is the user authenticated? - include_private = include_private_results(request) + form_data = clean_form_data(form) + include_private = include_private_results(request) + results = run_search_general(form_data, include_private) + + results_count = len(results) + # Reset page to one if the page number surpasses how many pages there actually are + page = form_data.page + if page > math.ceil(results_count / form_data.limit): + page = 1 + + # The paginator object handles splicing the results to a one-page iterable and calculates which page numbers to show. + paginator = Paginator(object_list=results, per_page=form_data.limit) + results = paginator.get_page(form_data.page) + results.adjusted_elided_pages = paginator.get_elided_page_range( + form_data.page, on_each_side=1 + ) - results = search_general( - names=names, - alns=alns, - uei_or_eins=uei_or_eins, - start_date=start_date, - end_date=end_date, - cog_or_oversight=cog_or_oversight, - agency_name=agency_name, - audit_years=audit_years, - auditee_state=auditee_state, - include_private=include_private, - order_by=order_by, - order_direction=order_direction, + # Reformat these so the date-picker elements in HTML prepopulate + if form.cleaned_data["start_date"]: + form.cleaned_data["start_date"] = form.cleaned_data["start_date"].strftime( + "%Y-%m-%d" + ) + if form.cleaned_data["end_date"]: + form.cleaned_data["end_date"] = form.cleaned_data["end_date"].strftime( + "%Y-%m-%d" ) - results_count = len(results) - # Reset page to one if the page number surpasses how many pages there actually are - if page > math.ceil(results_count / limit): - page = 1 - - paginator = Paginator(object_list=results, per_page=limit) - results = paginator.get_page(page) # List of size objects - results.adjusted_elided_pages = paginator.get_elided_page_range( - page, on_each_side=1 - ) # Pagination buttons, adjust ellipses around the current page - - # Reformat these so the date-picker elements in HTML prepopulate - if form.cleaned_data["start_date"]: - form.cleaned_data["start_date"] = start_date.strftime("%Y-%m-%d") - if form.cleaned_data["end_date"]: - form.cleaned_data["end_date"] = end_date.strftime("%Y-%m-%d") - else: - raise BadRequest("Form data validation error.", form.errors) - - if order_by is None: - order_by = "acceptance_date" context = context | { "form": form, "state_abbrevs": STATE_ABBREVS, - "limit": limit, + "limit": form_data.limit, "results": results, "results_count": results_count, "page": page, - "order_by": order_by, - "order_direction": order_direction, + "order_by": form_data.order_by, + "order_direction": form_data.order_direction, + "summary_report_download_limit": SUMMARY_REPORT_DOWNLOAD_LIMIT, } return render(request, "search.html", context) @@ -134,10 +188,10 @@ def post(self, request, *args, **kwargs): class AuditSummaryView(ReportAccessRequiredMixin, View): def get(self, request, report_id): """ - Grab any information about the given report in the dissemination tables. - 1. See if this audit is available in the dissemination tables. If not, 404. - 2. Grab all relevant info from dissem tables. Some items may not exist if they had no findings. - 3. Wrap up the data all nice in a context object for display. + Display information about the given report in the dissemination tables. + 1. See if this audit is available. If not, 404. + 2. Grab all relevant info from the dissemination tables. + 3. Wrap the data into a context object for display. """ # Viewable audits __MUST__ be public. general = General.objects.filter(report_id=report_id) @@ -150,7 +204,7 @@ def get(self, request, report_id): data = self.get_audit_content(report_id) - # Add entity name and UEI to the context, for the footer bit. + # Add entity name and UEI to the context, for the footer. context = { "report_id": report_id, "auditee_name": general_data["auditee_name"], @@ -165,8 +219,6 @@ def get_audit_content(self, report_id): """ Grab everything relevant from the dissemination tables. Wrap that data into a dict, and return it. - We may want to define additional functions to squish this information down - further. I.e. remove DB ids or something. """ awards = FederalAward.objects.filter(report_id=report_id) audit_findings = Finding.objects.filter(report_id=report_id) @@ -179,10 +231,8 @@ def get_audit_content(self, report_id): data = {} - data["Awards"] = [ - x for x in awards.values() - ] # Take QuerySet to a list of objects - + # QuerySet values to an array of dicts + data["Awards"] = [x for x in awards.values()] if notes_to_sefa.exists(): data["Notes to SEFA"] = [x for x in notes_to_sefa.values()] if audit_findings.exists(): @@ -205,6 +255,10 @@ def get_audit_content(self, report_id): class PdfDownloadView(ReportAccessRequiredMixin, View): def get(self, request, report_id): + """ + Given a report_id in the URL, find the relevant PDF in S3 and + redirect to the download link. + """ # only allow PDF downloads for disseminated submissions get_object_or_404(General, report_id=report_id) @@ -216,6 +270,10 @@ def get(self, request, report_id): class XlsxDownloadView(ReportAccessRequiredMixin, View): def get(self, request, report_id, file_type): + """ + Given a report_id and workbook section (file_type) in the URL, + find the relevant XLSX file in S3 and redirect to its download link. + """ # only allow xlsx downloads from disseminated submissions get_object_or_404(General, report_id=report_id) @@ -225,10 +283,51 @@ def get(self, request, report_id, file_type): return redirect(get_download_url(filename)) -class SummaryReportDownloadView(ReportAccessRequiredMixin, View): +class SingleSummaryReportDownloadView(ReportAccessRequiredMixin, View): def get(self, request, report_id): + """ + Given a report_id in the URL, generate the summary report in S3 and + redirect to its download link. + """ sac = get_object_or_404(General, report_id=report_id) filename = generate_summary_report([sac.report_id]) download_url = get_download_url(filename) return redirect(download_url) + + +class MultipleSummaryReportDownloadView(View): + def post(self, request): + """ + 1. Run a fresh search with the provided search parameters + 2. Get the report_id's from the search + 3. Generate a summary report with the report_ids, which goes into into S3 + 4. Redirect to the download url of this new report + """ + form = SearchForm(request.POST) + + try: + cleaned_data = clean_form_data(form) + include_private = include_private_results(request) + results = run_search_general(cleaned_data, include_private) + results = results[:SUMMARY_REPORT_DOWNLOAD_LIMIT] # Hard limit XLSX size + + if len(results) == 0: + raise Http404("Cannot generate summary report. No results found.") + report_ids = [result.report_id for result in results] + + filename = generate_summary_report(report_ids) + download_url = get_download_url(filename) + + return redirect(download_url) + + except Http404 as err: + logger.info( + "No results found for MultipleSummaryReportDownloadView post. Suggests an improper or old form submission." + ) + raise Http404 from err + except Exception as err: + logger.info( + "Enexpected error in MultipleSummaryReportDownloadView post:\n%s", err + ) + raise BadRequest(err)
View PDF