From e158078dc26220a69444ccea8e52c8dc7d61f0c0 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Fri, 11 Oct 2024 13:46:11 -0400 Subject: [PATCH 01/74] - Update xlsx generator to pin memory consumption via paginator --- tdrs-backend/tdpservice/data_files/util.py | 88 ++++++++++----------- tdrs-backend/tdpservice/data_files/views.py | 6 +- 2 files changed, 44 insertions(+), 50 deletions(-) diff --git a/tdrs-backend/tdpservice/data_files/util.py b/tdrs-backend/tdpservice/data_files/util.py index 0d2d7a941..b50a270c1 100644 --- a/tdrs-backend/tdpservice/data_files/util.py +++ b/tdrs-backend/tdpservice/data_files/util.py @@ -4,54 +4,32 @@ import xlsxwriter import calendar from tdpservice.parsers.models import ParserErrorCategoryChoices +from django.conf import settings +from django.core.paginator import Paginator +def format_error_msg(error_msg, fields_json): + """Format error message.""" + for key, value in fields_json['friendly_name'].items(): + error_msg = error_msg.replace(key, value) if value else error_msg + return error_msg -def get_xls_serialized_file(data): - """Return xls file created from the error.""" +def friendly_names(fields_json): + """Return comma separated string of friendly names.""" + return ','.join([i for i in fields_json['friendly_name'].values()]) + + +def internal_names(fields_json): + """Return comma separated string of internal names.""" + return ','.join([i for i in fields_json['friendly_name'].keys()]) - def chk(x): - """Check if fields_json is not None.""" - x['fields_json'] = x['fields_json'] if x.get('fields_json', None) else { - 'friendly_name': { - x['field_name']: x['field_name'] - }, - } - x['fields_json']['friendly_name'] = x['fields_json']['friendly_name'] if x['fields_json'].get( - 'friendly_name', None) else { - x['field_name']: x['field_name'] - } - if None in x['fields_json']['friendly_name'].keys(): - x['fields_json']['friendly_name'].pop(None) - if None in x['fields_json']['friendly_name'].values(): - x['fields_json']['friendly_name'].pop() - return x - - def format_error_msg(x): - """Format error message.""" - error_msg = x['error_message'] - for key, value in x['fields_json']['friendly_name'].items(): - error_msg = error_msg.replace(key, value) if value else error_msg - return error_msg +def get_xls_serialized_file(parser_errors): + """Return xls file created from the error.""" row, col = 0, 0 output = BytesIO() workbook = xlsxwriter.Workbook(output) worksheet = workbook.add_worksheet() - report_columns = [ - ('case_number', lambda x: x['case_number']), - ('year', lambda x: str(x['rpt_month_year'])[0:4] if x['rpt_month_year'] else None), - ('month', lambda x: calendar.month_name[ - int(str(x['rpt_month_year'])[4:]) - ] if x['rpt_month_year'] else None), - ('error_message', lambda x: format_error_msg(chk(x))), - ('item_number', lambda x: x['item_number']), - ('item_name', lambda x: ','.join([i for i in chk(x)['fields_json']['friendly_name'].values()])), - ('internal_variable_name', lambda x: ','.join([i for i in chk(x)['fields_json']['friendly_name'].keys()])), - ('row_number', lambda x: x['row_number']), - ('error_type', lambda x: str(ParserErrorCategoryChoices(x['error_type']).label)), - ] - # write beta banner worksheet.write( row, col, @@ -91,16 +69,34 @@ def format_header(header_list: list): return ' '.join([i.capitalize() for i in header_list.split('_')]) # We will write the headers in the first row - [worksheet.write(row, col, format_header(key[0]), bold) for col, key in enumerate(report_columns)] - - [ - worksheet.write(row + 6, col, key[1](data_i)) for col, key in enumerate(report_columns) - for row, data_i in enumerate(data) - ] + columns = ['case_number', 'year', 'month', + 'error_message', 'item_number', 'item_name', + 'internal_variable_name', 'row_number', 'error_type', + ] + for idx, col in enumerate(columns): + worksheet.write(row, idx, format_header(col), bold) + + paginator = Paginator(parser_errors, settings.BULK_CREATE_BATCH_SIZE) + row_idx = 6 + for page in paginator: + for record in page.object_list: + rpt_month_year = str(getattr(record, 'rpt_month_year', None)) + fields_json = getattr(record, 'fields_json', {}) + + worksheet.write(row_idx, 0, record.case_number) + worksheet.write(row_idx, 1, rpt_month_year[:4]) + worksheet.write(row_idx, 2, calendar.month_name[int(rpt_month_year[4:])] if rpt_month_year[4:] else None) + worksheet.write(row_idx, 3, format_error_msg(record.error_message, fields_json)) + worksheet.write(row_idx, 4, record.item_number) + worksheet.write(row_idx, 5, friendly_names(fields_json)) + worksheet.write(row_idx, 6, internal_names(fields_json)) + worksheet.write(row_idx, 7, record.row_number) + worksheet.write(row_idx, 8, str(ParserErrorCategoryChoices(record.error_type).label)) + row_idx += 1 # autofit all columns except for the first one worksheet.autofit() worksheet.set_column(0, 0, 20) workbook.close() - return {"data": data, "xls_report": base64.b64encode(output.getvalue()).decode("utf-8")} + return {"xls_report": base64.b64encode(output.getvalue()).decode("utf-8")} diff --git a/tdrs-backend/tdpservice/data_files/views.py b/tdrs-backend/tdpservice/data_files/views.py index 3f67d7cb3..adb9f37cf 100644 --- a/tdrs-backend/tdpservice/data_files/views.py +++ b/tdrs-backend/tdpservice/data_files/views.py @@ -21,7 +21,6 @@ from tdpservice.scheduling import parser_task from tdpservice.data_files.s3_client import S3Client from tdpservice.parsers.models import ParserError -from tdpservice.parsers.serializers import ParsingErrorSerializer logger = logging.getLogger(__name__) @@ -147,9 +146,8 @@ def download(self, request, pk=None): def download_error_report(self, request, pk=None): """Generate and return the parsing error report xlsx.""" datafile = self.get_object() - parser_errors = ParserError.objects.all().filter(file=datafile) - serializer = ParsingErrorSerializer(parser_errors, many=True, context=self.get_serializer_context()) - return Response(get_xls_serialized_file(serializer.data)) + parser_errors = ParserError.objects.all().filter(file=datafile).order_by('pk') + return Response(get_xls_serialized_file(parser_errors)) class GetYearList(APIView): From 428084577e83a295b467984f7f81a763fde7cc59 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Fri, 11 Oct 2024 13:59:44 -0400 Subject: [PATCH 02/74] - fix white space --- tdrs-backend/tdpservice/data_files/util.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tdrs-backend/tdpservice/data_files/util.py b/tdrs-backend/tdpservice/data_files/util.py index b50a270c1..5b52cb5f4 100644 --- a/tdrs-backend/tdpservice/data_files/util.py +++ b/tdrs-backend/tdpservice/data_files/util.py @@ -13,6 +13,7 @@ def format_error_msg(error_msg, fields_json): error_msg = error_msg.replace(key, value) if value else error_msg return error_msg + def friendly_names(fields_json): """Return comma separated string of friendly names.""" return ','.join([i for i in fields_json['friendly_name'].values()]) From 51cf19e690825d18ee985d05d20ac743a8591691 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Tue, 15 Oct 2024 10:40:10 -0400 Subject: [PATCH 03/74] - Add logic to narrow error report when stt is accessing it - Add admin property to user model - whitespace clean up --- tdrs-backend/tdpservice/data_files/util.py | 1 + tdrs-backend/tdpservice/data_files/views.py | 75 ++++++++++++++++++++- tdrs-backend/tdpservice/users/models.py | 5 ++ 3 files changed, 78 insertions(+), 3 deletions(-) diff --git a/tdrs-backend/tdpservice/data_files/util.py b/tdrs-backend/tdpservice/data_files/util.py index 5b52cb5f4..847e9d397 100644 --- a/tdrs-backend/tdpservice/data_files/util.py +++ b/tdrs-backend/tdpservice/data_files/util.py @@ -7,6 +7,7 @@ from django.conf import settings from django.core.paginator import Paginator + def format_error_msg(error_msg, fields_json): """Format error message.""" for key, value in fields_json['friendly_name'].items(): diff --git a/tdrs-backend/tdpservice/data_files/views.py b/tdrs-backend/tdpservice/data_files/views.py index adb9f37cf..3312b9da4 100644 --- a/tdrs-backend/tdpservice/data_files/views.py +++ b/tdrs-backend/tdpservice/data_files/views.py @@ -1,5 +1,6 @@ """Check if user is authorized.""" import logging +from django.db.models import Q from django.http import FileResponse from django_filters import rest_framework as filters from django.conf import settings @@ -20,7 +21,7 @@ from tdpservice.users.permissions import DataFilePermissions, IsApprovedPermission from tdpservice.scheduling import parser_task from tdpservice.data_files.s3_client import S3Client -from tdpservice.parsers.models import ParserError +from tdpservice.parsers.models import ParserError, ParserErrorCategoryChoices logger = logging.getLogger(__name__) @@ -146,8 +147,76 @@ def download(self, request, pk=None): def download_error_report(self, request, pk=None): """Generate and return the parsing error report xlsx.""" datafile = self.get_object() - parser_errors = ParserError.objects.all().filter(file=datafile).order_by('pk') - return Response(get_xls_serialized_file(parser_errors)) + all_errors = ParserError.objects.filter(file=datafile).order_by('pk') + filtered_errors = None + user = self.request.user + if not (user.is_ofa_sys_admin or user.is_ofa_admin): + error_type_query = Q(error_type=ParserErrorCategoryChoices.PRE_CHECK) | \ + Q(error_type=ParserErrorCategoryChoices.CASE_CONSISTENCY) + filtered_errors = all_errors.filter(error_type_query) + + # All cat2 errors associated with FAMILY_AFFILIATION or CITIZENSHIP_STATUS + filtered_errors = filtered_errors.union(all_errors.filter(fields_json__friendly_name__has_any_keys=[ + "FAMILY_AFFILIATION", + "CITIZENSHIP_STATUS" + ], + error_type=ParserErrorCategoryChoices.FIELD_VALUE)) + + # All cat3 errors associated with FAMILY_AFFILIATION and CITIZENSHIP_STATUS + filtered_errors = filtered_errors.union(all_errors.filter(fields_json__friendly_name__has_keys=[ + "FAMILY_AFFILIATION", + "CITIZENSHIP_STATUS" + ], + error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY)) + + # All cat3 errors associated with FAMILY_AFFILIATION and SSN + filtered_errors = filtered_errors.union(all_errors.filter(fields_json__friendly_name__has_keys=[ + "FAMILY_AFFILIATION", + "SSN" + ], + error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY)) + + if "Active" in datafile.section: + # All cat3 errors associated with FAMILY_AFFILIATION and SSN and CITIZENSHIP_STATUS + filtered_errors = filtered_errors.union(all_errors.filter(fields_json__friendly_name__has_keys=[ + "FAMILY_AFFILIATION", + "SSN", + "CITIZENSHIP_STATUS" + ], + error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY)) + + # All cat3 errors associated with FAMILY_AFFILIATION and EDUCATION_LEVEL + filtered_errors = filtered_errors.union(all_errors.filter(fields_json__friendly_name__has_keys=[ + "FAMILY_AFFILIATION", + "EDUCATION_LEVEL", + ], + error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY)) + + # All cat3 errors associated with FAMILY_AFFILIATION and WORK_ELIGIBLE_INDICATOR + filtered_errors = filtered_errors.union(all_errors.filter(fields_json__friendly_name__has_keys=[ + "FAMILY_AFFILIATION", + "WORK_ELIGIBLE_INDICATOR", + ], + error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY)) + + # All cat3 errors associated with CITIZENSHIP_STATUS and WORK_ELIGIBLE_INDICATOR + filtered_errors = filtered_errors.union(all_errors.filter(fields_json__friendly_name__has_keys=[ + "CITIZENSHIP_STATUS", + "WORK_ELIGIBLE_INDICATOR", + ], + error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY)) + + # All cat3 errors associated with summed fields: AMT_FOOD_STAMP_ASSISTANCE, AMT_SUB_CC, CASH_AMOUNT, + # CC_AMOUNT, TRANSP_AMOUNT + filtered_errors = filtered_errors.union(all_errors.filter(fields_json__friendly_name__has_keys=[ + "AMT_FOOD_STAMP_ASSISTANCE", "AMT_SUB_CC", "CASH_AMOUNT", "CC_AMOUNT", "TRANSP_AMOUNT" + ], + error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY)) + + else: + filtered_errors = all_errors + + return Response(get_xls_serialized_file(filtered_errors)) class GetYearList(APIView): diff --git a/tdrs-backend/tdpservice/users/models.py b/tdrs-backend/tdpservice/users/models.py index 40f8dc900..40db93a0c 100644 --- a/tdrs-backend/tdpservice/users/models.py +++ b/tdrs-backend/tdpservice/users/models.py @@ -180,6 +180,11 @@ def is_ocio_staff(self) -> bool: """Return whether or not the user is in the ACF OCIO Group.""" return self.is_in_group("ACF OCIO") + @property + def is_ofa_admin(self) -> bool: + """Return whether or not the user is in the OFA Admin Group.""" + return self.is_in_group("OFA Admin") + @property def is_ofa_sys_admin(self) -> bool: """Return whether or not the user is in the OFA System Admin Group.""" From bf4c4a9d57d5d7630e38a205357f7173b5ef197d Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Wed, 16 Oct 2024 08:50:30 -0400 Subject: [PATCH 04/74] - Add explainer --- tdrs-backend/tdpservice/data_files/views.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tdrs-backend/tdpservice/data_files/views.py b/tdrs-backend/tdpservice/data_files/views.py index 3312b9da4..9365b80bc 100644 --- a/tdrs-backend/tdpservice/data_files/views.py +++ b/tdrs-backend/tdpservice/data_files/views.py @@ -151,6 +151,7 @@ def download_error_report(self, request, pk=None): filtered_errors = None user = self.request.user if not (user.is_ofa_sys_admin or user.is_ofa_admin): + # All cat1/4 errors error_type_query = Q(error_type=ParserErrorCategoryChoices.PRE_CHECK) | \ Q(error_type=ParserErrorCategoryChoices.CASE_CONSISTENCY) filtered_errors = all_errors.filter(error_type_query) From 50cda342b2a49fce19e7be1c4077b731ac3249a0 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Wed, 16 Oct 2024 10:30:24 -0400 Subject: [PATCH 05/74] - add final query --- tdrs-backend/tdpservice/data_files/views.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tdrs-backend/tdpservice/data_files/views.py b/tdrs-backend/tdpservice/data_files/views.py index 9365b80bc..878701426 100644 --- a/tdrs-backend/tdpservice/data_files/views.py +++ b/tdrs-backend/tdpservice/data_files/views.py @@ -186,6 +186,13 @@ def download_error_report(self, request, pk=None): ], error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY)) + # All cat3 errors associated with FAMILY_AFFILIATION and PARENT_MINOR_CHILD + filtered_errors = filtered_errors.union(all_errors.filter(fields_json__friendly_name__has_keys=[ + "FAMILY_AFFILIATION", + "PARENT_MINOR_CHILD", + ], + error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY)) + # All cat3 errors associated with FAMILY_AFFILIATION and EDUCATION_LEVEL filtered_errors = filtered_errors.union(all_errors.filter(fields_json__friendly_name__has_keys=[ "FAMILY_AFFILIATION", @@ -213,7 +220,6 @@ def download_error_report(self, request, pk=None): "AMT_FOOD_STAMP_ASSISTANCE", "AMT_SUB_CC", "CASH_AMOUNT", "CC_AMOUNT", "TRANSP_AMOUNT" ], error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY)) - else: filtered_errors = all_errors From ddbc5318399f4dc140831befd7258dfb110a5f1a Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Fri, 18 Oct 2024 10:30:23 -0400 Subject: [PATCH 06/74] - add logs to ignore --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 6be3a5017..abd6af9f2 100644 --- a/.gitignore +++ b/.gitignore @@ -110,3 +110,6 @@ cypress.env.json # Patches *.patch tdrs-backend/*.pg + +# Log files +*.log From 3f0279347bdfab2e67e7690b8f6cc8acf05cb5a8 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Fri, 18 Oct 2024 11:26:14 -0400 Subject: [PATCH 07/74] - Updated queries - Ordered based on OneNote --- tdrs-backend/tdpservice/data_files/views.py | 43 ++++++++++++--------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/tdrs-backend/tdpservice/data_files/views.py b/tdrs-backend/tdpservice/data_files/views.py index 878701426..6821abcf1 100644 --- a/tdrs-backend/tdpservice/data_files/views.py +++ b/tdrs-backend/tdpservice/data_files/views.py @@ -150,34 +150,46 @@ def download_error_report(self, request, pk=None): all_errors = ParserError.objects.filter(file=datafile).order_by('pk') filtered_errors = None user = self.request.user - if not (user.is_ofa_sys_admin or user.is_ofa_admin): + is_active = "Active" in datafile.section + is_closed = "Closed" in datafile.section + + # We only filter Active and Closed submissions. Aggregate and Stratum return all errors. + if not (user.is_ofa_sys_admin or user.is_ofa_admin) and (is_active or is_closed): # All cat1/4 errors error_type_query = Q(error_type=ParserErrorCategoryChoices.PRE_CHECK) | \ Q(error_type=ParserErrorCategoryChoices.CASE_CONSISTENCY) filtered_errors = all_errors.filter(error_type_query) - # All cat2 errors associated with FAMILY_AFFILIATION or CITIZENSHIP_STATUS - filtered_errors = filtered_errors.union(all_errors.filter(fields_json__friendly_name__has_any_keys=[ - "FAMILY_AFFILIATION", - "CITIZENSHIP_STATUS" - ], - error_type=ParserErrorCategoryChoices.FIELD_VALUE)) + # All cat2 errors associated with FAMILY_AFFILIATION and (CITIZENSHIP_STATUS or CLOSURE_REASON) + second_field = "CITIZENSHIP_STATUS" if is_active else "CLOSURE_REASON" + field_query = Q(field_name="FAMILY_AFFILIATION") | Q(field_name=second_field) + filtered_errors = filtered_errors.union(all_errors.filter( + field_query, + error_type=ParserErrorCategoryChoices.FIELD_VALUE + )) - # All cat3 errors associated with FAMILY_AFFILIATION and CITIZENSHIP_STATUS + # All cat3 errors associated with FAMILY_AFFILIATION and SSN filtered_errors = filtered_errors.union(all_errors.filter(fields_json__friendly_name__has_keys=[ "FAMILY_AFFILIATION", - "CITIZENSHIP_STATUS" + "SSN" ], error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY)) - # All cat3 errors associated with FAMILY_AFFILIATION and SSN + # All cat3 errors associated with FAMILY_AFFILIATION and CITIZENSHIP_STATUS filtered_errors = filtered_errors.union(all_errors.filter(fields_json__friendly_name__has_keys=[ "FAMILY_AFFILIATION", - "SSN" + "CITIZENSHIP_STATUS" ], error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY)) - if "Active" in datafile.section: + if is_active: + # All cat3 errors associated with summed fields: AMT_FOOD_STAMP_ASSISTANCE, AMT_SUB_CC, CASH_AMOUNT, + # CC_AMOUNT, TRANSP_AMOUNT + filtered_errors = filtered_errors.union(all_errors.filter(fields_json__friendly_name__has_keys=[ + "AMT_FOOD_STAMP_ASSISTANCE", "AMT_SUB_CC", "CASH_AMOUNT", "CC_AMOUNT", "TRANSP_AMOUNT" + ], + error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY)) + # All cat3 errors associated with FAMILY_AFFILIATION and SSN and CITIZENSHIP_STATUS filtered_errors = filtered_errors.union(all_errors.filter(fields_json__friendly_name__has_keys=[ "FAMILY_AFFILIATION", @@ -213,13 +225,6 @@ def download_error_report(self, request, pk=None): "WORK_ELIGIBLE_INDICATOR", ], error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY)) - - # All cat3 errors associated with summed fields: AMT_FOOD_STAMP_ASSISTANCE, AMT_SUB_CC, CASH_AMOUNT, - # CC_AMOUNT, TRANSP_AMOUNT - filtered_errors = filtered_errors.union(all_errors.filter(fields_json__friendly_name__has_keys=[ - "AMT_FOOD_STAMP_ASSISTANCE", "AMT_SUB_CC", "CASH_AMOUNT", "CC_AMOUNT", "TRANSP_AMOUNT" - ], - error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY)) else: filtered_errors = all_errors From 451064febbab8a28af3f328c669a14936a71fc9b Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Sun, 20 Oct 2024 13:46:13 -0400 Subject: [PATCH 08/74] - Added tech memo outlining prioritized errors work --- .../priotitized-errors/prioritized-errors.md | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md diff --git a/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md b/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md new file mode 100644 index 000000000..1a7024b09 --- /dev/null +++ b/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md @@ -0,0 +1,101 @@ +# TDP Prioritized Parser Errors + +**Audience**: TDP Software Engineers
+**Subject**: Prioritized Errors
+**Date**: October 20, 2024
+ +## Summary +This technical memorandum provides a suggested path to implement a set of new requirements OFA has generated to alleviate the sheer number of parser errors generated during a STT's data submission. OFA has indicated, while still necessary for admins to see, some errors are of a lower priority for STTs to view and even prevent them from using the submission history reports to correct their submissions. Thus, the OFA team has requested that a "priority" be assigned to parser errors so that the report STTs receive is filtered down to only the highest priority errors that must be fixed. Regardless of how errors are prioritized, OFA admins will still retain the ability to generate the unfiltered error report for further, more in-depth investigation if a necessary situation presents itself. + +## Background +Currently, error reports are generated in the TDP backend via the `get_xls_serialized_file` function. This function accepts the serialized queryset of the appropriate `ParserError`s queryset. This function the writes an XLSX file and returns it to the user. Apart from the lack of priotization in the report generated from this function, it also introduces the possibility to cause an out of memory (OOM) error. This can occur because, the Django model serializer brings the entire queryset into memory to serialize it into JSON. Because these ParserError querysets can be very large (hundreds of thousands), we will also alleviate the memory pressure `get_xls_serialized_file` introduces by removing the Django model serializer and make use of queryset pagination. + +## Out of Scope +Current requirements from OFA do not require category two errors to be queryable by value and expected value. That feature is out of scope within the tech memo and would require more design and implementation work. + +## Method/Design +Given the current OFA requirements, we can implement prioritized errors, and memory efficient report generation without too much work. OFA has provided [this OneNote](https://gorafttech.sharepoint.com/:o:/s/TDRSResearchDesign/EnIa1Mn4v7pOskW7BLomXhIBxUMlYLRU_f1C0dxemW7dWw?e=m0rNyI) document which outlines the error types, errors, and fields that are most important/prioritized for STTs to see. + +### Memory Efficient Report Generation +As previously mentioned in the #background section, the `get_xls_serialized_file` introduces a method to serialize parser errors into a XLSX that requires the entire queryset of parser errors to be brought into memory. Because these querysets can be very large, having them in memory regularly kills Celery workers with an OOM error. To remedy the issue, this tech memo suggests updating `get_xls_serialized_file` to not use Django model serializers and instead leverage the power of Django querysets and pagination. To accomplish this, instead of passing a JSON serialized querset to `get_xls_serialized_file`, a standard (un-evaluated) queryset should be passed. Then, the body of the `get_xls_serialized_file` function should be updated appropriately to use a queryset object instead of a JSON object to generate the XLSX spreadsheet. The updates should also include paginating the queryset to avoid bringing the entirety of the queryset into memory at any one time. The code snippet below provides an example of paginating the queryset and writing the appropriate fields of each entry to the XLSX report. + +```python +paginator = Paginator(parser_errors, settings.BULK_CREATE_BATCH_SIZE) +row_idx = 6 +for page in paginator: + for record in page.object_list: + rpt_month_year = str(getattr(record, 'rpt_month_year', None)) + fields_json = getattr(record, 'fields_json', {}) + + worksheet.write(row_idx, 0, record.case_number) + worksheet.write(row_idx, 1, rpt_month_year[:4]) + worksheet.write(row_idx, 2, calendar.month_name[int(rpt_month_year[4:])] if rpt_month_year[4:] else None) + worksheet.write(row_idx, 3, format_error_msg(record.error_message, fields_json)) + worksheet.write(row_idx, 4, record.item_number) + worksheet.write(row_idx, 5, friendly_names(fields_json)) + worksheet.write(row_idx, 6, internal_names(fields_json)) + worksheet.write(row_idx, 7, record.row_number) + worksheet.write(row_idx, 8, str(ParserErrorCategoryChoices(record.error_type).label)) +``` + +The three helper functions: `format_error_msg`, `friendly_names`, `internal_names` used to write the appropriate fields can be seen below. + +```python +def format_error_msg(error_msg, fields_json): + """Format error message.""" + for key, value in fields_json['friendly_name'].items(): + error_msg = error_msg.replace(key, value) if value else error_msg + return error_msg + + +def friendly_names(fields_json): + """Return comma separated string of friendly names.""" + return ','.join([i for i in fields_json['friendly_name'].values()]) + + +def internal_names(fields_json): + """Return comma separated string of internal names.""" + return ','.join([i for i in fields_json['friendly_name'].keys()]) +``` + +### Prioritized Errors +[This OneNote](https://gorafttech.sharepoint.com/:o:/s/TDRSResearchDesign/EnIa1Mn4v7pOskW7BLomXhIBxUMlYLRU_f1C0dxemW7dWw?e=m0rNyI) is invaluable to the implementation of prioritized errors. Prioritizing errors could be a very large and technically challenging feature involving new migrations, validation/validator refactors, etc... However, this can all be avoided by making a key insight for each of the category two and category three validators by way of OFA's requirements for them. For the category two case, the OneNote document generically specifies category two validation surrounding: Family Affiliation, Citizenship and Closure reason. Further discussion with OFA indicated that it is important/a priority for a STT to see all category two errors encompassing these fields in their entirety. That makes prioritizing these category two errors extremely easy because the need to query those fields by specific values and expected values is not required. The queries below provide a complete implementation to query all category two errors encompassing those fields. + +```python +# All cat2 errors associated with FAMILY_AFFILIATION and (CITIZENSHIP_STATUS or CLOSURE_REASON) +second_field = "CITIZENSHIP_STATUS" if is_active else "CLOSURE_REASON" +field_query = Q(field_name="FAMILY_AFFILIATION") | Q(field_name=second_field) +filtered_errors = filtered_errors.union(all_errors.filter( + field_query, + error_type=ParserErrorCategoryChoices.FIELD_VALUE + )) +``` + +The key insight for the category three case is less obvious. Looking at the OneNote, it seems as though we might need to query errors based on field name(s), expected value and actual value. However, for category three errors that information is encoded into the error by its existence. For example, the OneNote indicates that a high priority error a STT should have included in their report is `If fam affil = 1 then SSN must be valid `. This exact error and it's values (expected and given) can be uniquely found in any of the active or closed case record schemas. E.g.: + +```python +category3.ifThenAlso( + condition_field_name='FAMILY_AFFILIATION', + condition_function=category3.isEqual(1), + result_field_name='SSN', + result_function=category3.validateSSN(), +) +``` + +The existence of this error, with these fields, is uniquely defined in the appropriate schemas. The same can be said for the remaining high priority category three errors. Thus, to define the high priority errors we need only know the required field(s) and their error type. Given those pieces of information, queries of the form below can be used to filter STT error reports to only show the highest priority errors. + +```python +errors.filter(fields_json__friendly_name__has_keys=[FIELD_NAME, FIELD_NAME, ETC...], + error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY) +``` + +By unioning the category two queries from above with the remainder of the category three queries, a queryset containing only the highest priority errors can be generated and subsequently passed to `get_xls_serialized_file` generate and return the prioritized error report to the requesting STT. + +## Affected Systems +- TDP backend +- TDP frontend: latency time incurred while generating report + +## Use and Test cases to consider +- Admin can get both prioritized and un-prioritized report +- STT only recieves prioritized report +- Existing tests leveraging ParserError querysets are updated and re-validated for correctness From eeb406f7e25c6f96c40273728837bf3497e810f6 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Sun, 20 Oct 2024 14:19:26 -0400 Subject: [PATCH 09/74] - fix tests --- tdrs-backend/tdpservice/data_files/test/test_api.py | 13 ++++++------- tdrs-backend/tdpservice/data_files/util.py | 9 ++++++++- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/tdrs-backend/tdpservice/data_files/test/test_api.py b/tdrs-backend/tdpservice/data_files/test/test_api.py index 78685b075..e69a5f9a7 100644 --- a/tdrs-backend/tdpservice/data_files/test/test_api.py +++ b/tdrs-backend/tdpservice/data_files/test/test_api.py @@ -101,9 +101,8 @@ def assert_error_report_tanf_file_content_matches_with_friendly_names(response): assert ws.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + "instructions (linked below) when looking up items and allowable values during the data revision process" assert ws.cell(row=8, column=COL_ERROR_MESSAGE).value == ( - "Since Item 21A (Cash Amount) is 873, then Item 21B " - "(Cash and Cash Equivalents: Number of Months) 0 must be greater than 0" - ) + "Every T1 record should have at least one corresponding T2 or T3 record with the same Item 4 " + "(Reporting Year and Month) and Item 6 (Case Number).") @staticmethod def assert_error_report_ssp_file_content_matches_with_friendly_names(response): @@ -114,8 +113,8 @@ def assert_error_report_ssp_file_content_matches_with_friendly_names(response): assert ws.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + "instructions (linked below) when looking up items and allowable values during the data revision process" - assert ws.cell(row=7, column=COL_ERROR_MESSAGE).value == ("M1 Item 11 (Receives Subsidized Housing): 3 is " - "not in range [1, 2].") + assert ws.cell(row=7, column=COL_ERROR_MESSAGE).value == ("TRAILER: record length is 15 characters " + "but must be 23.") @staticmethod def assert_error_report_file_content_matches_without_friendly_names(response): @@ -135,8 +134,8 @@ def assert_error_report_file_content_matches_without_friendly_names(response): assert ws.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + "instructions (linked below) when looking up items and allowable values during the data revision process" assert ws.cell(row=8, column=COL_ERROR_MESSAGE).value == ( - "Since Item 21A (Cash Amount) is 873, then Item 21B " - "(Cash and Cash Equivalents: Number of Months) 0 must be greater than 0" + "Every T1 record should have at least one corresponding T2 or T3 record with the same Item 4 " + "(Reporting Year and Month) and Item 6 (Case Number)." ) @staticmethod diff --git a/tdrs-backend/tdpservice/data_files/util.py b/tdrs-backend/tdpservice/data_files/util.py index 847e9d397..82d4927e4 100644 --- a/tdrs-backend/tdpservice/data_files/util.py +++ b/tdrs-backend/tdpservice/data_files/util.py @@ -24,6 +24,12 @@ def internal_names(fields_json): """Return comma separated string of internal names.""" return ','.join([i for i in fields_json['friendly_name'].keys()]) +def check_fields_json(fields_json, field_name): + """If fields_json is None, impute field name to avoid NoneType errors.""" + if not fields_json: + child_dict = {field_name: field_name} if field_name else {} + fields_json = {'friendly_name': child_dict} + return fields_json def get_xls_serialized_file(parser_errors): """Return xls file created from the error.""" @@ -83,7 +89,8 @@ def format_header(header_list: list): for page in paginator: for record in page.object_list: rpt_month_year = str(getattr(record, 'rpt_month_year', None)) - fields_json = getattr(record, 'fields_json', {}) + + fields_json = check_fields_json(getattr(record, 'fields_json', {}), record.field_name) worksheet.write(row_idx, 0, record.case_number) worksheet.write(row_idx, 1, rpt_month_year[:4]) From de90f489fd2bf6c7685ae9aa0d9bb0f4073847f5 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Mon, 21 Oct 2024 08:46:25 -0400 Subject: [PATCH 10/74] - Fixed None string included in report --- tdrs-backend/tdpservice/data_files/util.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tdrs-backend/tdpservice/data_files/util.py b/tdrs-backend/tdpservice/data_files/util.py index 82d4927e4..a9b63d326 100644 --- a/tdrs-backend/tdpservice/data_files/util.py +++ b/tdrs-backend/tdpservice/data_files/util.py @@ -88,7 +88,8 @@ def format_header(header_list: list): row_idx = 6 for page in paginator: for record in page.object_list: - rpt_month_year = str(getattr(record, 'rpt_month_year', None)) + rpt_month_year = getattr(record, 'rpt_month_year', None) + rpt_month_year = str(rpt_month_year) if rpt_month_year else "" fields_json = check_fields_json(getattr(record, 'fields_json', {}), record.field_name) From 1687fd15e3870457ff5ea6bd548d9bb74e9febf5 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Mon, 21 Oct 2024 08:53:38 -0400 Subject: [PATCH 11/74] - Change queryset ordering --- tdrs-backend/tdpservice/data_files/test/test_api.py | 6 ++---- tdrs-backend/tdpservice/data_files/views.py | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/tdrs-backend/tdpservice/data_files/test/test_api.py b/tdrs-backend/tdpservice/data_files/test/test_api.py index e69a5f9a7..a0a7fdc99 100644 --- a/tdrs-backend/tdpservice/data_files/test/test_api.py +++ b/tdrs-backend/tdpservice/data_files/test/test_api.py @@ -101,8 +101,7 @@ def assert_error_report_tanf_file_content_matches_with_friendly_names(response): assert ws.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + "instructions (linked below) when looking up items and allowable values during the data revision process" assert ws.cell(row=8, column=COL_ERROR_MESSAGE).value == ( - "Every T1 record should have at least one corresponding T2 or T3 record with the same Item 4 " - "(Reporting Year and Month) and Item 6 (Case Number).") + "No records created.") @staticmethod def assert_error_report_ssp_file_content_matches_with_friendly_names(response): @@ -134,8 +133,7 @@ def assert_error_report_file_content_matches_without_friendly_names(response): assert ws.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + "instructions (linked below) when looking up items and allowable values during the data revision process" assert ws.cell(row=8, column=COL_ERROR_MESSAGE).value == ( - "Every T1 record should have at least one corresponding T2 or T3 record with the same Item 4 " - "(Reporting Year and Month) and Item 6 (Case Number)." + "No records created." ) @staticmethod diff --git a/tdrs-backend/tdpservice/data_files/views.py b/tdrs-backend/tdpservice/data_files/views.py index 6821abcf1..c03a69ce6 100644 --- a/tdrs-backend/tdpservice/data_files/views.py +++ b/tdrs-backend/tdpservice/data_files/views.py @@ -147,7 +147,7 @@ def download(self, request, pk=None): def download_error_report(self, request, pk=None): """Generate and return the parsing error report xlsx.""" datafile = self.get_object() - all_errors = ParserError.objects.filter(file=datafile).order_by('pk') + all_errors = ParserError.objects.filter(file=datafile).order_by('-pk') filtered_errors = None user = self.request.user is_active = "Active" in datafile.section From d6042bd1b4e65f1f44f2bb85c0b451dab1046d8a Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Mon, 21 Oct 2024 09:14:16 -0400 Subject: [PATCH 12/74] - Adding debug code --- tdrs-backend/tdpservice/data_files/views.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tdrs-backend/tdpservice/data_files/views.py b/tdrs-backend/tdpservice/data_files/views.py index c03a69ce6..01955757f 100644 --- a/tdrs-backend/tdpservice/data_files/views.py +++ b/tdrs-backend/tdpservice/data_files/views.py @@ -148,6 +148,8 @@ def download_error_report(self, request, pk=None): """Generate and return the parsing error report xlsx.""" datafile = self.get_object() all_errors = ParserError.objects.filter(file=datafile).order_by('-pk') + for e in all_errors: + print(e.pk, e.error_message) filtered_errors = None user = self.request.user is_active = "Active" in datafile.section From 8a25b39babc6ba5e11339d73fc3cfb3f7611350d Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Mon, 21 Oct 2024 09:21:26 -0400 Subject: [PATCH 13/74] - enable layer caching --- .circleci/base_config.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.circleci/base_config.yml b/.circleci/base_config.yml index 02d3c8f53..1eb5f0b14 100644 --- a/.circleci/base_config.yml +++ b/.circleci/base_config.yml @@ -12,11 +12,11 @@ executors: user: root machine-executor: machine: - docker_layer_caching: false + docker_layer_caching: true image: ubuntu-2204:2024.05.1 large-machine-executor: machine: - docker_layer_caching: false + docker_layer_caching: true image: ubuntu-2204:2024.05.1 resource_class: large From 46eabe17582925a6b59f0afa4a975ca120685919 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Mon, 21 Oct 2024 09:36:22 -0400 Subject: [PATCH 14/74] - use same method to get worksheet throughout tests - delete workbook if it exists before creating a new one - Remove debug code --- .../tdpservice/data_files/test/test_api.py | 18 +++++++----------- tdrs-backend/tdpservice/data_files/views.py | 2 -- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/tdrs-backend/tdpservice/data_files/test/test_api.py b/tdrs-backend/tdpservice/data_files/test/test_api.py index a0a7fdc99..36d48a317 100644 --- a/tdrs-backend/tdpservice/data_files/test/test_api.py +++ b/tdrs-backend/tdpservice/data_files/test/test_api.py @@ -1,4 +1,5 @@ """Tests for DataFiles Application.""" +import os from rest_framework import status import pytest import base64 @@ -82,6 +83,9 @@ def get_spreadsheet(response): """Return error report.""" decoded_response = base64.b64decode(response.data['xls_report']) + if os.path.exists('mycls.xlsx'): + os.remove('mycls.xlsx') + # write the excel file to disk with open('mycls.xlsx', 'wb') as f: f.write(decoded_response) @@ -100,7 +104,7 @@ def assert_error_report_tanf_file_content_matches_with_friendly_names(response): assert ws.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + "instructions (linked below) when looking up items and allowable values during the data revision process" - assert ws.cell(row=8, column=COL_ERROR_MESSAGE).value == ( + assert ws.cell(row=7, column=COL_ERROR_MESSAGE).value == ( "No records created.") @staticmethod @@ -118,21 +122,13 @@ def assert_error_report_ssp_file_content_matches_with_friendly_names(response): @staticmethod def assert_error_report_file_content_matches_without_friendly_names(response): """Assert the error report file contents match expected without friendly names.""" - decoded_response = base64.b64decode(response.data['xls_report']) - - # write the excel file to disk - with open('mycls.xlsx', 'wb') as f: - f.write(decoded_response) - - # read the excel file from disk - wb = openpyxl.load_workbook('mycls.xlsx') - ws = wb.get_sheet_by_name('Sheet1') + ws = DataFileAPITestBase.get_spreadsheet(response) COL_ERROR_MESSAGE = 4 assert ws.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + "instructions (linked below) when looking up items and allowable values during the data revision process" - assert ws.cell(row=8, column=COL_ERROR_MESSAGE).value == ( + assert ws.cell(row=7, column=COL_ERROR_MESSAGE).value == ( "No records created." ) diff --git a/tdrs-backend/tdpservice/data_files/views.py b/tdrs-backend/tdpservice/data_files/views.py index 01955757f..c03a69ce6 100644 --- a/tdrs-backend/tdpservice/data_files/views.py +++ b/tdrs-backend/tdpservice/data_files/views.py @@ -148,8 +148,6 @@ def download_error_report(self, request, pk=None): """Generate and return the parsing error report xlsx.""" datafile = self.get_object() all_errors = ParserError.objects.filter(file=datafile).order_by('-pk') - for e in all_errors: - print(e.pk, e.error_message) filtered_errors = None user = self.request.user is_active = "Active" in datafile.section From 1173de51f79a6c3b237f4de8e4ed59e42a6c3540 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Mon, 21 Oct 2024 09:55:41 -0400 Subject: [PATCH 15/74] - Update error messages --- tdrs-backend/tdpservice/data_files/test/test_api.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tdrs-backend/tdpservice/data_files/test/test_api.py b/tdrs-backend/tdpservice/data_files/test/test_api.py index 36d48a317..8aa1c1392 100644 --- a/tdrs-backend/tdpservice/data_files/test/test_api.py +++ b/tdrs-backend/tdpservice/data_files/test/test_api.py @@ -105,7 +105,8 @@ def assert_error_report_tanf_file_content_matches_with_friendly_names(response): assert ws.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + "instructions (linked below) when looking up items and allowable values during the data revision process" assert ws.cell(row=7, column=COL_ERROR_MESSAGE).value == ( - "No records created.") + "Every T1 record should have at least one corresponding T2 or T3 record with the same Item 4 " + "(Reporting Year and Month) and Item 6 (Case Number).") @staticmethod def assert_error_report_ssp_file_content_matches_with_friendly_names(response): @@ -129,7 +130,8 @@ def assert_error_report_file_content_matches_without_friendly_names(response): assert ws.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + "instructions (linked below) when looking up items and allowable values during the data revision process" assert ws.cell(row=7, column=COL_ERROR_MESSAGE).value == ( - "No records created." + "Every T1 record should have at least one corresponding T2 or T3 record with the same Item 4 " + "(Reporting Year and Month) and Item 6 (Case Number)." ) @staticmethod From cca2967c94e19d88f16aebfe56d09429ee7265a1 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Mon, 21 Oct 2024 10:22:48 -0400 Subject: [PATCH 16/74] - move order_by to be the last query - Update row error message number --- tdrs-backend/tdpservice/data_files/test/test_api.py | 4 ++-- tdrs-backend/tdpservice/data_files/views.py | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/tdrs-backend/tdpservice/data_files/test/test_api.py b/tdrs-backend/tdpservice/data_files/test/test_api.py index 8aa1c1392..b07d035d5 100644 --- a/tdrs-backend/tdpservice/data_files/test/test_api.py +++ b/tdrs-backend/tdpservice/data_files/test/test_api.py @@ -104,7 +104,7 @@ def assert_error_report_tanf_file_content_matches_with_friendly_names(response): assert ws.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + "instructions (linked below) when looking up items and allowable values during the data revision process" - assert ws.cell(row=7, column=COL_ERROR_MESSAGE).value == ( + assert ws.cell(row=8, column=COL_ERROR_MESSAGE).value == ( "Every T1 record should have at least one corresponding T2 or T3 record with the same Item 4 " "(Reporting Year and Month) and Item 6 (Case Number).") @@ -129,7 +129,7 @@ def assert_error_report_file_content_matches_without_friendly_names(response): assert ws.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + "instructions (linked below) when looking up items and allowable values during the data revision process" - assert ws.cell(row=7, column=COL_ERROR_MESSAGE).value == ( + assert ws.cell(row=8, column=COL_ERROR_MESSAGE).value == ( "Every T1 record should have at least one corresponding T2 or T3 record with the same Item 4 " "(Reporting Year and Month) and Item 6 (Case Number)." ) diff --git a/tdrs-backend/tdpservice/data_files/views.py b/tdrs-backend/tdpservice/data_files/views.py index c03a69ce6..b9214b8c2 100644 --- a/tdrs-backend/tdpservice/data_files/views.py +++ b/tdrs-backend/tdpservice/data_files/views.py @@ -147,7 +147,7 @@ def download(self, request, pk=None): def download_error_report(self, request, pk=None): """Generate and return the parsing error report xlsx.""" datafile = self.get_object() - all_errors = ParserError.objects.filter(file=datafile).order_by('-pk') + all_errors = ParserError.objects.filter(file=datafile) filtered_errors = None user = self.request.user is_active = "Active" in datafile.section @@ -228,6 +228,7 @@ def download_error_report(self, request, pk=None): else: filtered_errors = all_errors + filtered_errors.order_by('pk') return Response(get_xls_serialized_file(filtered_errors)) From da2835683393955ea2c6a1cb60c73aa0d6b7d2bf Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Mon, 21 Oct 2024 11:05:38 -0400 Subject: [PATCH 17/74] - Update order by clause - add debug code --- tdrs-backend/tdpservice/data_files/test/test_api.py | 6 ++++++ tdrs-backend/tdpservice/data_files/views.py | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/tdrs-backend/tdpservice/data_files/test/test_api.py b/tdrs-backend/tdpservice/data_files/test/test_api.py index b07d035d5..dd53e49b7 100644 --- a/tdrs-backend/tdpservice/data_files/test/test_api.py +++ b/tdrs-backend/tdpservice/data_files/test/test_api.py @@ -104,9 +104,12 @@ def assert_error_report_tanf_file_content_matches_with_friendly_names(response): assert ws.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + "instructions (linked below) when looking up items and allowable values during the data revision process" + for i in range(6, 10): + print(ws.cell(row=i, column=COL_ERROR_MESSAGE).value) assert ws.cell(row=8, column=COL_ERROR_MESSAGE).value == ( "Every T1 record should have at least one corresponding T2 or T3 record with the same Item 4 " "(Reporting Year and Month) and Item 6 (Case Number).") + assert False @staticmethod def assert_error_report_ssp_file_content_matches_with_friendly_names(response): @@ -129,10 +132,13 @@ def assert_error_report_file_content_matches_without_friendly_names(response): assert ws.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + "instructions (linked below) when looking up items and allowable values during the data revision process" + for i in range(6, 10): + print(ws.cell(row=i, column=COL_ERROR_MESSAGE).value) assert ws.cell(row=8, column=COL_ERROR_MESSAGE).value == ( "Every T1 record should have at least one corresponding T2 or T3 record with the same Item 4 " "(Reporting Year and Month) and Item 6 (Case Number)." ) + assert False @staticmethod def assert_data_file_exists(data_file_data, version, user): diff --git a/tdrs-backend/tdpservice/data_files/views.py b/tdrs-backend/tdpservice/data_files/views.py index b9214b8c2..20a30ea34 100644 --- a/tdrs-backend/tdpservice/data_files/views.py +++ b/tdrs-backend/tdpservice/data_files/views.py @@ -228,7 +228,7 @@ def download_error_report(self, request, pk=None): else: filtered_errors = all_errors - filtered_errors.order_by('pk') + filtered_errors.order_by('error_message', 'pk') return Response(get_xls_serialized_file(filtered_errors)) From d907a344304fd29b68baa4f5c4e8fe428fa5cc7a Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Mon, 21 Oct 2024 11:16:07 -0400 Subject: [PATCH 18/74] - remove debug code --- tdrs-backend/tdpservice/data_files/test/test_api.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tdrs-backend/tdpservice/data_files/test/test_api.py b/tdrs-backend/tdpservice/data_files/test/test_api.py index dd53e49b7..b07d035d5 100644 --- a/tdrs-backend/tdpservice/data_files/test/test_api.py +++ b/tdrs-backend/tdpservice/data_files/test/test_api.py @@ -104,12 +104,9 @@ def assert_error_report_tanf_file_content_matches_with_friendly_names(response): assert ws.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + "instructions (linked below) when looking up items and allowable values during the data revision process" - for i in range(6, 10): - print(ws.cell(row=i, column=COL_ERROR_MESSAGE).value) assert ws.cell(row=8, column=COL_ERROR_MESSAGE).value == ( "Every T1 record should have at least one corresponding T2 or T3 record with the same Item 4 " "(Reporting Year and Month) and Item 6 (Case Number).") - assert False @staticmethod def assert_error_report_ssp_file_content_matches_with_friendly_names(response): @@ -132,13 +129,10 @@ def assert_error_report_file_content_matches_without_friendly_names(response): assert ws.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + "instructions (linked below) when looking up items and allowable values during the data revision process" - for i in range(6, 10): - print(ws.cell(row=i, column=COL_ERROR_MESSAGE).value) assert ws.cell(row=8, column=COL_ERROR_MESSAGE).value == ( "Every T1 record should have at least one corresponding T2 or T3 record with the same Item 4 " "(Reporting Year and Month) and Item 6 (Case Number)." ) - assert False @staticmethod def assert_data_file_exists(data_file_data, version, user): From fba33f3377d669c90d9920bb48278a4ce6c8ecd3 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Mon, 21 Oct 2024 12:13:06 -0400 Subject: [PATCH 19/74] - Assign order by clause --- tdrs-backend/tdpservice/data_files/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tdrs-backend/tdpservice/data_files/views.py b/tdrs-backend/tdpservice/data_files/views.py index 20a30ea34..04c6a80cb 100644 --- a/tdrs-backend/tdpservice/data_files/views.py +++ b/tdrs-backend/tdpservice/data_files/views.py @@ -228,7 +228,7 @@ def download_error_report(self, request, pk=None): else: filtered_errors = all_errors - filtered_errors.order_by('error_message', 'pk') + filtered_errors = filtered_errors.order_by('-pk') return Response(get_xls_serialized_file(filtered_errors)) From bf33db11f1a3c7b9e754adfa1e1bbb3272f4e562 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Wed, 23 Oct 2024 15:54:24 -0400 Subject: [PATCH 20/74] - Correct document - functionize prioritized queries - Update group check function to allow multiple groups - add new admin property to user class --- .../priotitized-errors/prioritized-errors.md | 2 +- tdrs-backend/tdpservice/data_files/views.py | 112 ++++++------------ tdrs-backend/tdpservice/users/models.py | 14 ++- 3 files changed, 48 insertions(+), 80 deletions(-) diff --git a/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md b/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md index 1a7024b09..36f937c01 100644 --- a/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md +++ b/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md @@ -17,7 +17,7 @@ Current requirements from OFA do not require category two errors to be queryable Given the current OFA requirements, we can implement prioritized errors, and memory efficient report generation without too much work. OFA has provided [this OneNote](https://gorafttech.sharepoint.com/:o:/s/TDRSResearchDesign/EnIa1Mn4v7pOskW7BLomXhIBxUMlYLRU_f1C0dxemW7dWw?e=m0rNyI) document which outlines the error types, errors, and fields that are most important/prioritized for STTs to see. ### Memory Efficient Report Generation -As previously mentioned in the #background section, the `get_xls_serialized_file` introduces a method to serialize parser errors into a XLSX that requires the entire queryset of parser errors to be brought into memory. Because these querysets can be very large, having them in memory regularly kills Celery workers with an OOM error. To remedy the issue, this tech memo suggests updating `get_xls_serialized_file` to not use Django model serializers and instead leverage the power of Django querysets and pagination. To accomplish this, instead of passing a JSON serialized querset to `get_xls_serialized_file`, a standard (un-evaluated) queryset should be passed. Then, the body of the `get_xls_serialized_file` function should be updated appropriately to use a queryset object instead of a JSON object to generate the XLSX spreadsheet. The updates should also include paginating the queryset to avoid bringing the entirety of the queryset into memory at any one time. The code snippet below provides an example of paginating the queryset and writing the appropriate fields of each entry to the XLSX report. +As previously mentioned in the #background section, the `get_xls_serialized_file` introduces a method to serialize parser errors into a XLSX that requires the entire queryset of parser errors to be brought into memory. Because these querysets can be very large, having them in memory regularly kills Gunicorn workers with an OOM error. To remedy the issue, this tech memo suggests updating `get_xls_serialized_file` to not use Django model serializers and instead leverage the power of Django querysets and pagination. To accomplish this, instead of passing a JSON serialized querset to `get_xls_serialized_file`, a standard (un-evaluated) queryset should be passed. Then, the body of the `get_xls_serialized_file` function should be updated appropriately to use a queryset object instead of a JSON object to generate the XLSX spreadsheet. The updates should also include paginating the queryset to avoid bringing the entirety of the queryset into memory at any one time. The code snippet below provides an example of paginating the queryset and writing the appropriate fields of each entry to the XLSX report. ```python paginator = Paginator(parser_errors, settings.BULK_CREATE_BATCH_SIZE) diff --git a/tdrs-backend/tdpservice/data_files/views.py b/tdrs-backend/tdpservice/data_files/views.py index 04c6a80cb..9d6a50158 100644 --- a/tdrs-backend/tdpservice/data_files/views.py +++ b/tdrs-backend/tdpservice/data_files/views.py @@ -52,6 +52,21 @@ class DataFileViewSet(ModelViewSet): # Ref: https://github.com/raft-tech/TANF-app/issues/1007 queryset = DataFile.objects.all() + PRIORITIZED_CAT2 = ( + ("FAMILY_AFFILIATION", "CITIZENSHIP_STATUS", "CLOSURE_REASON"), + ) + + PRIORITIZED_CAT3 = ( + ("FAMILY_AFFILIATION", "SSN"), + ("FAMILY_AFFILIATION", "CITIZENSHIP_STATUS"), + ("AMT_FOOD_STAMP_ASSISTANCE", "AMT_SUB_CC", "CASH_AMOUNT", "CC_AMOUNT", "TRANSP_AMOUNT"), + ("FAMILY_AFFILIATION", "SSN", "CITIZENSHIP_STATUS"), + ("FAMILY_AFFILIATION", "PARENT_MINOR_CHILD"), + ("FAMILY_AFFILIATION", "EDUCATION_LEVEL"), + ("FAMILY_AFFILIATION", "WORK_ELIGIBLE_INDICATOR"), + ("CITIZENSHIP_STATUS", "WORK_ELIGIBLE_INDICATOR"), + ) + def create(self, request, *args, **kwargs): """Override create to upload in case of successful scan.""" logger.debug(f"{self.__class__.__name__}: {request}") @@ -143,6 +158,27 @@ def download(self, request, pk=None): ) return response + def __prioritize_queryset(self, filtered_errors, all_errors): + """Generate prioritized queryset of ParserErrors.""" + # All cat1/4 errors + error_type_query = Q(error_type=ParserErrorCategoryChoices.PRE_CHECK) | \ + Q(error_type=ParserErrorCategoryChoices.CASE_CONSISTENCY) + filtered_errors = all_errors.filter(error_type_query) + + for fields in self.PRIORITIZED_CAT2: + filtered_errors = filtered_errors.union(all_errors.filter( + field_name__in=fields, + error_type=ParserErrorCategoryChoices.FIELD_VALUE + )) + + for fields in self.PRIORITIZED_CAT3: + filtered_errors = filtered_errors.union(all_errors.filter( + fields_json__friendly_name__has_keys=fields, + error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY + )) + + return filtered_errors + @action(methods=["get"], detail=True) def download_error_report(self, request, pk=None): """Generate and return the parsing error report xlsx.""" @@ -150,81 +186,11 @@ def download_error_report(self, request, pk=None): all_errors = ParserError.objects.filter(file=datafile) filtered_errors = None user = self.request.user - is_active = "Active" in datafile.section - is_closed = "Closed" in datafile.section + is_s1_s2 = "Active" in datafile.section or "Closed" in datafile.section # We only filter Active and Closed submissions. Aggregate and Stratum return all errors. - if not (user.is_ofa_sys_admin or user.is_ofa_admin) and (is_active or is_closed): - # All cat1/4 errors - error_type_query = Q(error_type=ParserErrorCategoryChoices.PRE_CHECK) | \ - Q(error_type=ParserErrorCategoryChoices.CASE_CONSISTENCY) - filtered_errors = all_errors.filter(error_type_query) - - # All cat2 errors associated with FAMILY_AFFILIATION and (CITIZENSHIP_STATUS or CLOSURE_REASON) - second_field = "CITIZENSHIP_STATUS" if is_active else "CLOSURE_REASON" - field_query = Q(field_name="FAMILY_AFFILIATION") | Q(field_name=second_field) - filtered_errors = filtered_errors.union(all_errors.filter( - field_query, - error_type=ParserErrorCategoryChoices.FIELD_VALUE - )) - - # All cat3 errors associated with FAMILY_AFFILIATION and SSN - filtered_errors = filtered_errors.union(all_errors.filter(fields_json__friendly_name__has_keys=[ - "FAMILY_AFFILIATION", - "SSN" - ], - error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY)) - - # All cat3 errors associated with FAMILY_AFFILIATION and CITIZENSHIP_STATUS - filtered_errors = filtered_errors.union(all_errors.filter(fields_json__friendly_name__has_keys=[ - "FAMILY_AFFILIATION", - "CITIZENSHIP_STATUS" - ], - error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY)) - - if is_active: - # All cat3 errors associated with summed fields: AMT_FOOD_STAMP_ASSISTANCE, AMT_SUB_CC, CASH_AMOUNT, - # CC_AMOUNT, TRANSP_AMOUNT - filtered_errors = filtered_errors.union(all_errors.filter(fields_json__friendly_name__has_keys=[ - "AMT_FOOD_STAMP_ASSISTANCE", "AMT_SUB_CC", "CASH_AMOUNT", "CC_AMOUNT", "TRANSP_AMOUNT" - ], - error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY)) - - # All cat3 errors associated with FAMILY_AFFILIATION and SSN and CITIZENSHIP_STATUS - filtered_errors = filtered_errors.union(all_errors.filter(fields_json__friendly_name__has_keys=[ - "FAMILY_AFFILIATION", - "SSN", - "CITIZENSHIP_STATUS" - ], - error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY)) - - # All cat3 errors associated with FAMILY_AFFILIATION and PARENT_MINOR_CHILD - filtered_errors = filtered_errors.union(all_errors.filter(fields_json__friendly_name__has_keys=[ - "FAMILY_AFFILIATION", - "PARENT_MINOR_CHILD", - ], - error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY)) - - # All cat3 errors associated with FAMILY_AFFILIATION and EDUCATION_LEVEL - filtered_errors = filtered_errors.union(all_errors.filter(fields_json__friendly_name__has_keys=[ - "FAMILY_AFFILIATION", - "EDUCATION_LEVEL", - ], - error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY)) - - # All cat3 errors associated with FAMILY_AFFILIATION and WORK_ELIGIBLE_INDICATOR - filtered_errors = filtered_errors.union(all_errors.filter(fields_json__friendly_name__has_keys=[ - "FAMILY_AFFILIATION", - "WORK_ELIGIBLE_INDICATOR", - ], - error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY)) - - # All cat3 errors associated with CITIZENSHIP_STATUS and WORK_ELIGIBLE_INDICATOR - filtered_errors = filtered_errors.union(all_errors.filter(fields_json__friendly_name__has_keys=[ - "CITIZENSHIP_STATUS", - "WORK_ELIGIBLE_INDICATOR", - ], - error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY)) + if not user.is_an_admin and is_s1_s2: + filtered_errors = self.__prioritize_queryset(filtered_errors, all_errors) else: filtered_errors = all_errors diff --git a/tdrs-backend/tdpservice/users/models.py b/tdrs-backend/tdpservice/users/models.py index 40db93a0c..3cf094264 100644 --- a/tdrs-backend/tdpservice/users/models.py +++ b/tdrs-backend/tdpservice/users/models.py @@ -118,9 +118,11 @@ def __str__(self): """Return the username as the string representation of the object.""" return self.username - def is_in_group(self, group_name: str) -> bool: - """Return whether or not the user is a member of the specified Group.""" - return self.groups.filter(name=group_name).exists() + def is_in_group(self, group_names: list) -> bool: + """Return whether or not the user is a member of the specified Group(s).""" + if type(group_names) == str: + group_names = [group_names] + return self.groups.filter(name__in=group_names).exists() def validate_location(self): """Throw a validation error if a user has a location type incompatable with their role.""" @@ -181,9 +183,9 @@ def is_ocio_staff(self) -> bool: return self.is_in_group("ACF OCIO") @property - def is_ofa_admin(self) -> bool: - """Return whether or not the user is in the OFA Admin Group.""" - return self.is_in_group("OFA Admin") + def is_an_admin(self) -> bool: + """Return whether or not the user is in the OFA Admin Group or OFA System Admin.""" + return self.is_in_group(["OFA Admin", "OFA System Admin"]) @property def is_ofa_sys_admin(self) -> bool: From 600c48e93fe7306b1b346cba64891cf5db0558b6 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Wed, 23 Oct 2024 15:58:48 -0400 Subject: [PATCH 21/74] - revert layer cachhing --- .circleci/base_config.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.circleci/base_config.yml b/.circleci/base_config.yml index 1eb5f0b14..02d3c8f53 100644 --- a/.circleci/base_config.yml +++ b/.circleci/base_config.yml @@ -12,11 +12,11 @@ executors: user: root machine-executor: machine: - docker_layer_caching: true + docker_layer_caching: false image: ubuntu-2204:2024.05.1 large-machine-executor: machine: - docker_layer_caching: true + docker_layer_caching: false image: ubuntu-2204:2024.05.1 resource_class: large From 4a627c981c66ba2967c661037645c5751decd866 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Tue, 12 Nov 2024 10:29:41 -0500 Subject: [PATCH 22/74] - Update report to include aggregates tab along with prioritized tab - functionized util.py - updated views.py to pass both querysets --- tdrs-backend/tdpservice/data_files/util.py | 77 +++++++++++++++++---- tdrs-backend/tdpservice/data_files/views.py | 17 ++--- 2 files changed, 68 insertions(+), 26 deletions(-) diff --git a/tdrs-backend/tdpservice/data_files/util.py b/tdrs-backend/tdpservice/data_files/util.py index a9b63d326..eb44ede23 100644 --- a/tdrs-backend/tdpservice/data_files/util.py +++ b/tdrs-backend/tdpservice/data_files/util.py @@ -6,6 +6,7 @@ from tdpservice.parsers.models import ParserErrorCategoryChoices from django.conf import settings from django.core.paginator import Paginator +from django.db.models import Count def format_error_msg(error_msg, fields_json): @@ -24,6 +25,7 @@ def internal_names(fields_json): """Return comma separated string of internal names.""" return ','.join([i for i in fields_json['friendly_name'].keys()]) + def check_fields_json(fields_json, field_name): """If fields_json is None, impute field name to avoid NoneType errors.""" if not fields_json: @@ -31,12 +33,10 @@ def check_fields_json(fields_json, field_name): fields_json = {'friendly_name': child_dict} return fields_json -def get_xls_serialized_file(parser_errors): - """Return xls file created from the error.""" + +def write_worksheet_banner(worksheet): + """Convenience function for banner writing.""" row, col = 0, 0 - output = BytesIO() - workbook = xlsxwriter.Workbook(output) - worksheet = workbook.add_worksheet() # write beta banner worksheet.write( @@ -67,15 +67,16 @@ def get_xls_serialized_file(parser_errors): string='Visit the Knowledge Center for further guidance on reviewing error reports' ) - row, col = 5, 0 - - # write csv header - bold = workbook.add_format({'bold': True}) - def format_header(header_list: list): +def format_header(header_list: list): """Format header.""" return ' '.join([i.capitalize() for i in header_list.split('_')]) + +def write_prioritized_errors(worksheet, prioritized_errors, bold): + """Write prioritized errors to spreadsheet.""" + row, col = 5, 0 + # We will write the headers in the first row columns = ['case_number', 'year', 'month', 'error_message', 'item_number', 'item_name', @@ -84,7 +85,7 @@ def format_header(header_list: list): for idx, col in enumerate(columns): worksheet.write(row, idx, format_header(col), bold) - paginator = Paginator(parser_errors, settings.BULK_CREATE_BATCH_SIZE) + paginator = Paginator(prioritized_errors.order_by('-pk'), settings.BULK_CREATE_BATCH_SIZE) row_idx = 6 for page in paginator: for record in page.object_list: @@ -104,9 +105,59 @@ def format_header(header_list: list): worksheet.write(row_idx, 8, str(ParserErrorCategoryChoices(record.error_type).label)) row_idx += 1 + +def write_aggregate_errors(worksheet, all_errors, bold): + """Aggregate by error message and write.""" + row, col = 5, 0 + + # We will write the headers in the first row + columns = ['year', 'month', 'error_message', 'item_number', 'item_name', + 'internal_variable_name', 'error_type', 'number_of_occurrences' + ] + for idx, col in enumerate(columns): + worksheet.write(row, idx, format_header(col), bold) + + aggregates = all_errors.values('rpt_month_year', 'error_message', 'item_number', 'field_name', 'fields_json', 'error_type').annotate(num_occurrences=Count('error_message')) + + paginator = Paginator(aggregates.order_by('-num_occurrences'), settings.BULK_CREATE_BATCH_SIZE) + row_idx = 6 + for page in paginator: + for record in page.object_list: + rpt_month_year = record['rpt_month_year'] + rpt_month_year = str(rpt_month_year) if rpt_month_year else "" + + fields_json = check_fields_json(record['fields_json'], record['field_name']) + + worksheet.write(row_idx, 0, rpt_month_year[:4]) + worksheet.write(row_idx, 1, calendar.month_name[int(rpt_month_year[4:])] if rpt_month_year[4:] else None) + worksheet.write(row_idx, 2, format_error_msg(record['error_message'], fields_json)) + worksheet.write(row_idx, 3, record['item_number']) + worksheet.write(row_idx, 4, friendly_names(fields_json)) + worksheet.write(row_idx, 5, internal_names(fields_json)) + worksheet.write(row_idx, 6, str(ParserErrorCategoryChoices(record['error_type']).label)) + worksheet.write(row_idx, 7, record['num_occurrences']) + row_idx += 1 + + +def get_xls_serialized_file(all_errors, prioritized_errors): + """Return xls file created from the error.""" + output = BytesIO() + workbook = xlsxwriter.Workbook(output) + prioritized_sheet = workbook.add_worksheet(name="Priorities") + aggregate_sheet = workbook.add_worksheet(name="Aggregates") + + write_worksheet_banner(prioritized_sheet) + write_worksheet_banner(aggregate_sheet) + + bold = workbook.add_format({'bold': True}) + write_prioritized_errors(prioritized_sheet, prioritized_errors, bold) + write_aggregate_errors(aggregate_sheet, all_errors, bold) + # autofit all columns except for the first one - worksheet.autofit() - worksheet.set_column(0, 0, 20) + prioritized_sheet.autofit() + prioritized_sheet.set_column(0, 0, 20) + aggregate_sheet.autofit() + aggregate_sheet.set_column(0, 0, 20) workbook.close() return {"xls_report": base64.b64encode(output.getvalue()).decode("utf-8")} diff --git a/tdrs-backend/tdpservice/data_files/views.py b/tdrs-backend/tdpservice/data_files/views.py index 9d6a50158..9dd7097ce 100644 --- a/tdrs-backend/tdpservice/data_files/views.py +++ b/tdrs-backend/tdpservice/data_files/views.py @@ -158,7 +158,7 @@ def download(self, request, pk=None): ) return response - def __prioritize_queryset(self, filtered_errors, all_errors): + def __prioritize_queryset(self, all_errors): """Generate prioritized queryset of ParserErrors.""" # All cat1/4 errors error_type_query = Q(error_type=ParserErrorCategoryChoices.PRE_CHECK) | \ @@ -183,19 +183,10 @@ def __prioritize_queryset(self, filtered_errors, all_errors): def download_error_report(self, request, pk=None): """Generate and return the parsing error report xlsx.""" datafile = self.get_object() - all_errors = ParserError.objects.filter(file=datafile) - filtered_errors = None - user = self.request.user - is_s1_s2 = "Active" in datafile.section or "Closed" in datafile.section - - # We only filter Active and Closed submissions. Aggregate and Stratum return all errors. - if not user.is_an_admin and is_s1_s2: - filtered_errors = self.__prioritize_queryset(filtered_errors, all_errors) - else: - filtered_errors = all_errors + all_errors = ParserError.objects.filter(file=datafile).order_by('-pk') + filtered_errors = self.__prioritize_queryset(all_errors) - filtered_errors = filtered_errors.order_by('-pk') - return Response(get_xls_serialized_file(filtered_errors)) + return Response(get_xls_serialized_file(all_errors, filtered_errors)) class GetYearList(APIView): From f06ceec879d558bae9fdf7fc5b1b56811c851fce Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Tue, 12 Nov 2024 10:30:40 -0500 Subject: [PATCH 23/74] - linting --- tdrs-backend/tdpservice/data_files/util.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tdrs-backend/tdpservice/data_files/util.py b/tdrs-backend/tdpservice/data_files/util.py index eb44ede23..99008a32f 100644 --- a/tdrs-backend/tdpservice/data_files/util.py +++ b/tdrs-backend/tdpservice/data_files/util.py @@ -35,7 +35,7 @@ def check_fields_json(fields_json, field_name): def write_worksheet_banner(worksheet): - """Convenience function for banner writing.""" + """Write worksheet banner.""" row, col = 0, 0 # write beta banner @@ -69,8 +69,8 @@ def write_worksheet_banner(worksheet): def format_header(header_list: list): - """Format header.""" - return ' '.join([i.capitalize() for i in header_list.split('_')]) + """Format header.""" + return ' '.join([i.capitalize() for i in header_list.split('_')]) def write_prioritized_errors(worksheet, prioritized_errors, bold): @@ -117,7 +117,9 @@ def write_aggregate_errors(worksheet, all_errors, bold): for idx, col in enumerate(columns): worksheet.write(row, idx, format_header(col), bold) - aggregates = all_errors.values('rpt_month_year', 'error_message', 'item_number', 'field_name', 'fields_json', 'error_type').annotate(num_occurrences=Count('error_message')) + aggregates = all_errors.values('rpt_month_year', 'error_message', + 'item_number', 'field_name', + 'fields_json', 'error_type').annotate(num_occurrences=Count('error_message')) paginator = Paginator(aggregates.order_by('-num_occurrences'), settings.BULK_CREATE_BATCH_SIZE) row_idx = 6 From 6dffd4b5c8d79d141efc37d16be533327a6f8dfb Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Tue, 12 Nov 2024 10:44:48 -0500 Subject: [PATCH 24/74] - Added tests for aggregates page --- .../tdpservice/data_files/test/test_api.py | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/tdrs-backend/tdpservice/data_files/test/test_api.py b/tdrs-backend/tdpservice/data_files/test/test_api.py index b07d035d5..1fe08469e 100644 --- a/tdrs-backend/tdpservice/data_files/test/test_api.py +++ b/tdrs-backend/tdpservice/data_files/test/test_api.py @@ -92,47 +92,54 @@ def get_spreadsheet(response): # read the excel file from disk wb = openpyxl.load_workbook('mycls.xlsx') - ws = wb.get_sheet_by_name('Sheet1') - return ws + priorities = wb['Priorities'] + aggregates = wb['Aggregates'] + return priorities, aggregates @staticmethod def assert_error_report_tanf_file_content_matches_with_friendly_names(response): """Assert the error report file contents match expected with friendly names.""" - ws = DataFileAPITestBase.get_spreadsheet(response) + priorities, aggregates = DataFileAPITestBase.get_spreadsheet(response) COL_ERROR_MESSAGE = 4 + COL_NUM_OCCURRENCES = 8 - assert ws.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + assert priorities.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + "instructions (linked below) when looking up items and allowable values during the data revision process" - assert ws.cell(row=8, column=COL_ERROR_MESSAGE).value == ( + assert priorities.cell(row=8, column=COL_ERROR_MESSAGE).value == ( "Every T1 record should have at least one corresponding T2 or T3 record with the same Item 4 " "(Reporting Year and Month) and Item 6 (Case Number).") + assert aggregates.cell(row=7, column=COL_NUM_OCCURRENCES).value == 1 @staticmethod def assert_error_report_ssp_file_content_matches_with_friendly_names(response): """Assert the error report file contents match expected with friendly names.""" - ws = DataFileAPITestBase.get_spreadsheet(response) + priorities, aggregates = DataFileAPITestBase.get_spreadsheet(response) COL_ERROR_MESSAGE = 4 + COL_NUM_OCCURRENCES = 8 - assert ws.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + assert priorities.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + "instructions (linked below) when looking up items and allowable values during the data revision process" - assert ws.cell(row=7, column=COL_ERROR_MESSAGE).value == ("TRAILER: record length is 15 characters " + assert priorities.cell(row=7, column=COL_ERROR_MESSAGE).value == ("TRAILER: record length is 15 characters " "but must be 23.") + assert aggregates.cell(row=7, column=COL_NUM_OCCURRENCES).value == 5 @staticmethod def assert_error_report_file_content_matches_without_friendly_names(response): """Assert the error report file contents match expected without friendly names.""" - ws = DataFileAPITestBase.get_spreadsheet(response) + priorities, aggregates = DataFileAPITestBase.get_spreadsheet(response) COL_ERROR_MESSAGE = 4 + COL_NUM_OCCURRENCES = 8 - assert ws.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + assert priorities.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + "instructions (linked below) when looking up items and allowable values during the data revision process" - assert ws.cell(row=8, column=COL_ERROR_MESSAGE).value == ( + assert priorities.cell(row=8, column=COL_ERROR_MESSAGE).value == ( "Every T1 record should have at least one corresponding T2 or T3 record with the same Item 4 " "(Reporting Year and Month) and Item 6 (Case Number)." ) + assert aggregates.cell(row=7, column=COL_NUM_OCCURRENCES).value == 1 @staticmethod def assert_data_file_exists(data_file_data, version, user): From 65cf52ba8fd30c5c5ea0c0779d3ae07d67b3ac26 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Tue, 12 Nov 2024 11:02:16 -0500 Subject: [PATCH 25/74] - linting --- tdrs-backend/tdpservice/data_files/test/test_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tdrs-backend/tdpservice/data_files/test/test_api.py b/tdrs-backend/tdpservice/data_files/test/test_api.py index 1fe08469e..b06179e2c 100644 --- a/tdrs-backend/tdpservice/data_files/test/test_api.py +++ b/tdrs-backend/tdpservice/data_files/test/test_api.py @@ -122,7 +122,7 @@ def assert_error_report_ssp_file_content_matches_with_friendly_names(response): assert priorities.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + "instructions (linked below) when looking up items and allowable values during the data revision process" assert priorities.cell(row=7, column=COL_ERROR_MESSAGE).value == ("TRAILER: record length is 15 characters " - "but must be 23.") + "but must be 23.") assert aggregates.cell(row=7, column=COL_NUM_OCCURRENCES).value == 5 @staticmethod From 5a5ca1bcc4f4324127ca6bad475e7b34756ebde6 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Wed, 13 Nov 2024 09:58:22 -0500 Subject: [PATCH 26/74] - moved prioritized querset func to util to be used in different locations - moved ParserErrorCategoryChoices to util.py to avoid circular import - updated get_status to reject file if more than 500 priority errors are generated. --- tdrs-backend/tdpservice/data_files/util.py | 52 ++++++++++++++++++++- tdrs-backend/tdpservice/data_files/views.py | 43 ++--------------- tdrs-backend/tdpservice/parsers/models.py | 17 +++---- 3 files changed, 59 insertions(+), 53 deletions(-) diff --git a/tdrs-backend/tdpservice/data_files/util.py b/tdrs-backend/tdpservice/data_files/util.py index 99008a32f..6851bbec2 100644 --- a/tdrs-backend/tdpservice/data_files/util.py +++ b/tdrs-backend/tdpservice/data_files/util.py @@ -3,10 +3,58 @@ from io import BytesIO import xlsxwriter import calendar -from tdpservice.parsers.models import ParserErrorCategoryChoices from django.conf import settings from django.core.paginator import Paginator -from django.db.models import Count +from django.db import models +from django.db.models import Count, Q +from django.utils.translation import gettext_lazy as _ + + +class ParserErrorCategoryChoices(models.TextChoices): + """Enum of ParserError error_type.""" + + PRE_CHECK = "1", _("File pre-check") + FIELD_VALUE = "2", _("Record value invalid") + VALUE_CONSISTENCY = "3", _("Record value consistency") + CASE_CONSISTENCY = "4", _("Case consistency") + SECTION_CONSISTENCY = "5", _("Section consistency") + HISTORICAL_CONSISTENCY = "6", _("Historical consistency") + + +def get_prioritized_queryset(parser_errors): + """Generate a prioritized queryset of ParserErrors.""" + PRIORITIZED_CAT2 = ( + ("FAMILY_AFFILIATION", "CITIZENSHIP_STATUS", "CLOSURE_REASON"), + ) + PRIORITIZED_CAT3 = ( + ("FAMILY_AFFILIATION", "SSN"), + ("FAMILY_AFFILIATION", "CITIZENSHIP_STATUS"), + ("AMT_FOOD_STAMP_ASSISTANCE", "AMT_SUB_CC", "CASH_AMOUNT", "CC_AMOUNT", "TRANSP_AMOUNT"), + ("FAMILY_AFFILIATION", "SSN", "CITIZENSHIP_STATUS"), + ("FAMILY_AFFILIATION", "PARENT_MINOR_CHILD"), + ("FAMILY_AFFILIATION", "EDUCATION_LEVEL"), + ("FAMILY_AFFILIATION", "WORK_ELIGIBLE_INDICATOR"), + ("CITIZENSHIP_STATUS", "WORK_ELIGIBLE_INDICATOR"), + ) + + # All cat1/4 errors + error_type_query = Q(error_type=ParserErrorCategoryChoices.PRE_CHECK) | \ + Q(error_type=ParserErrorCategoryChoices.CASE_CONSISTENCY) + filtered_errors = parser_errors.filter(error_type_query) + + for fields in PRIORITIZED_CAT2: + filtered_errors = filtered_errors.union(parser_errors.filter( + field_name__in=fields, + error_type=ParserErrorCategoryChoices.FIELD_VALUE + )) + + for fields in PRIORITIZED_CAT3: + filtered_errors = filtered_errors.union(parser_errors.filter( + fields_json__friendly_name__has_keys=fields, + error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY + )) + + return filtered_errors def format_error_msg(error_msg, fields_json): diff --git a/tdrs-backend/tdpservice/data_files/views.py b/tdrs-backend/tdpservice/data_files/views.py index 9dd7097ce..c67d701d7 100644 --- a/tdrs-backend/tdpservice/data_files/views.py +++ b/tdrs-backend/tdpservice/data_files/views.py @@ -1,6 +1,5 @@ """Check if user is authorized.""" import logging -from django.db.models import Q from django.http import FileResponse from django_filters import rest_framework as filters from django.conf import settings @@ -16,12 +15,12 @@ from rest_framework import status from tdpservice.data_files.serializers import DataFileSerializer -from tdpservice.data_files.util import get_xls_serialized_file +from tdpservice.data_files.util import get_xls_serialized_file, get_prioritized_queryset from tdpservice.data_files.models import DataFile, get_s3_upload_path from tdpservice.users.permissions import DataFilePermissions, IsApprovedPermission from tdpservice.scheduling import parser_task from tdpservice.data_files.s3_client import S3Client -from tdpservice.parsers.models import ParserError, ParserErrorCategoryChoices +from tdpservice.parsers.models import ParserError logger = logging.getLogger(__name__) @@ -52,21 +51,6 @@ class DataFileViewSet(ModelViewSet): # Ref: https://github.com/raft-tech/TANF-app/issues/1007 queryset = DataFile.objects.all() - PRIORITIZED_CAT2 = ( - ("FAMILY_AFFILIATION", "CITIZENSHIP_STATUS", "CLOSURE_REASON"), - ) - - PRIORITIZED_CAT3 = ( - ("FAMILY_AFFILIATION", "SSN"), - ("FAMILY_AFFILIATION", "CITIZENSHIP_STATUS"), - ("AMT_FOOD_STAMP_ASSISTANCE", "AMT_SUB_CC", "CASH_AMOUNT", "CC_AMOUNT", "TRANSP_AMOUNT"), - ("FAMILY_AFFILIATION", "SSN", "CITIZENSHIP_STATUS"), - ("FAMILY_AFFILIATION", "PARENT_MINOR_CHILD"), - ("FAMILY_AFFILIATION", "EDUCATION_LEVEL"), - ("FAMILY_AFFILIATION", "WORK_ELIGIBLE_INDICATOR"), - ("CITIZENSHIP_STATUS", "WORK_ELIGIBLE_INDICATOR"), - ) - def create(self, request, *args, **kwargs): """Override create to upload in case of successful scan.""" logger.debug(f"{self.__class__.__name__}: {request}") @@ -158,33 +142,12 @@ def download(self, request, pk=None): ) return response - def __prioritize_queryset(self, all_errors): - """Generate prioritized queryset of ParserErrors.""" - # All cat1/4 errors - error_type_query = Q(error_type=ParserErrorCategoryChoices.PRE_CHECK) | \ - Q(error_type=ParserErrorCategoryChoices.CASE_CONSISTENCY) - filtered_errors = all_errors.filter(error_type_query) - - for fields in self.PRIORITIZED_CAT2: - filtered_errors = filtered_errors.union(all_errors.filter( - field_name__in=fields, - error_type=ParserErrorCategoryChoices.FIELD_VALUE - )) - - for fields in self.PRIORITIZED_CAT3: - filtered_errors = filtered_errors.union(all_errors.filter( - fields_json__friendly_name__has_keys=fields, - error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY - )) - - return filtered_errors - @action(methods=["get"], detail=True) def download_error_report(self, request, pk=None): """Generate and return the parsing error report xlsx.""" datafile = self.get_object() all_errors = ParserError.objects.filter(file=datafile).order_by('-pk') - filtered_errors = self.__prioritize_queryset(all_errors) + filtered_errors = get_prioritized_queryset(all_errors) return Response(get_xls_serialized_file(all_errors, filtered_errors)) diff --git a/tdrs-backend/tdpservice/parsers/models.py b/tdrs-backend/tdpservice/parsers/models.py index f9c5f3c63..cc0f91d4d 100644 --- a/tdrs-backend/tdpservice/parsers/models.py +++ b/tdrs-backend/tdpservice/parsers/models.py @@ -6,20 +6,12 @@ from django.contrib.contenttypes.fields import GenericForeignKey from django.contrib.contenttypes.models import ContentType from tdpservice.data_files.models import DataFile +from tdpservice.data_files.util import ParserErrorCategoryChoices, get_prioritized_queryset + import logging logger = logging.getLogger(__name__) -class ParserErrorCategoryChoices(models.TextChoices): - """Enum of ParserError error_type.""" - - PRE_CHECK = "1", _("File pre-check") - FIELD_VALUE = "2", _("Record value invalid") - VALUE_CONSISTENCY = "3", _("Record value consistency") - CASE_CONSISTENCY = "4", _("Case consistency") - SECTION_CONSISTENCY = "5", _("Section consistency") - HISTORICAL_CONSISTENCY = "6", _("Historical consistency") - class ParserError(models.Model): """Model representing a parser error.""" @@ -133,9 +125,12 @@ def get_status(self): .filter(field_name="Record_Type")\ .exclude(error_message__icontains="trailer") + prioritized_errors = get_prioritized_queryset(errors) + if errors is None: return DataFileSummary.Status.PENDING - elif precheck_errors.count() > 0 or self.total_number_of_records_created == 0: + elif (precheck_errors.count() > 0 or self.total_number_of_records_created == 0 or + prioritized_errors.count() > 500): return DataFileSummary.Status.REJECTED elif errors.count() == 0: return DataFileSummary.Status.ACCEPTED From 8d0d1905e60397886a7e9344412ea4ca9c38f26c Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Wed, 13 Nov 2024 10:04:07 -0500 Subject: [PATCH 27/74] - Updated test to conform to priority errors update --- tdrs-backend/tdpservice/parsers/test/test_parse.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tdrs-backend/tdpservice/parsers/test/test_parse.py b/tdrs-backend/tdpservice/parsers/test/test_parse.py index d01a44030..d7f918421 100644 --- a/tdrs-backend/tdpservice/parsers/test/test_parse.py +++ b/tdrs-backend/tdpservice/parsers/test/test_parse.py @@ -1098,10 +1098,10 @@ def test_parse_ssp_section2_file(ssp_section2_file, dfs): dfs.case_aggregates = aggregates.case_aggregates_by_month( dfs.datafile, dfs.status) for dfs_case_aggregate in dfs.case_aggregates['months']: - assert dfs_case_aggregate['accepted_without_errors'] == 0 - assert dfs_case_aggregate['accepted_with_errors'] in [75, 78] + assert dfs_case_aggregate['accepted_without_errors'] == "N/A" + assert dfs_case_aggregate['accepted_with_errors'] == "N/A" assert dfs_case_aggregate['month'] in ['Oct', 'Nov', 'Dec'] - assert dfs.get_status() == DataFileSummary.Status.PARTIALLY_ACCEPTED + assert dfs.get_status() == DataFileSummary.Status.REJECTED m4_objs = SSP_M4.objects.all().order_by('id') m5_objs = SSP_M5.objects.all().order_by('AMOUNT_EARNED_INCOME') From f7d00cf32bb0d2a0cde35c9d6017bc86c92de9ff Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Wed, 13 Nov 2024 10:04:27 -0500 Subject: [PATCH 28/74] - linting --- tdrs-backend/tdpservice/parsers/models.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tdrs-backend/tdpservice/parsers/models.py b/tdrs-backend/tdpservice/parsers/models.py index cc0f91d4d..6a0ef71c7 100644 --- a/tdrs-backend/tdpservice/parsers/models.py +++ b/tdrs-backend/tdpservice/parsers/models.py @@ -2,7 +2,6 @@ import datetime from django.db import models -from django.utils.translation import gettext_lazy as _ from django.contrib.contenttypes.fields import GenericForeignKey from django.contrib.contenttypes.models import ContentType from tdpservice.data_files.models import DataFile From af3be1222cf0d3b9cce3a1efeb699daa7af1cc28 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Thu, 14 Nov 2024 08:59:00 -0500 Subject: [PATCH 29/74] - Updated to make status partially accepted - Updated xlsx tab names - Updated tests --- .../tdpservice/data_files/test/test_api.py | 35 ++++++++----------- tdrs-backend/tdpservice/data_files/util.py | 6 ++-- tdrs-backend/tdpservice/data_files/views.py | 2 +- tdrs-backend/tdpservice/parsers/models.py | 6 ++-- .../tdpservice/parsers/test/test_parse.py | 6 ++-- 5 files changed, 25 insertions(+), 30 deletions(-) diff --git a/tdrs-backend/tdpservice/data_files/test/test_api.py b/tdrs-backend/tdpservice/data_files/test/test_api.py index b06179e2c..ada702444 100644 --- a/tdrs-backend/tdpservice/data_files/test/test_api.py +++ b/tdrs-backend/tdpservice/data_files/test/test_api.py @@ -92,54 +92,49 @@ def get_spreadsheet(response): # read the excel file from disk wb = openpyxl.load_workbook('mycls.xlsx') - priorities = wb['Priorities'] - aggregates = wb['Aggregates'] - return priorities, aggregates + critical = wb['Critical'] + summary = wb['Summary'] + return critical, summary @staticmethod def assert_error_report_tanf_file_content_matches_with_friendly_names(response): """Assert the error report file contents match expected with friendly names.""" - priorities, aggregates = DataFileAPITestBase.get_spreadsheet(response) + critical, summary = DataFileAPITestBase.get_spreadsheet(response) COL_ERROR_MESSAGE = 4 COL_NUM_OCCURRENCES = 8 - assert priorities.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + assert critical.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + "instructions (linked below) when looking up items and allowable values during the data revision process" - assert priorities.cell(row=8, column=COL_ERROR_MESSAGE).value == ( - "Every T1 record should have at least one corresponding T2 or T3 record with the same Item 4 " - "(Reporting Year and Month) and Item 6 (Case Number).") - assert aggregates.cell(row=7, column=COL_NUM_OCCURRENCES).value == 1 + assert critical.cell(row=8, column=COL_ERROR_MESSAGE).value == "No records created." + assert summary.cell(row=7, column=COL_NUM_OCCURRENCES).value == 1 @staticmethod def assert_error_report_ssp_file_content_matches_with_friendly_names(response): """Assert the error report file contents match expected with friendly names.""" - priorities, aggregates = DataFileAPITestBase.get_spreadsheet(response) + critical, summary = DataFileAPITestBase.get_spreadsheet(response) COL_ERROR_MESSAGE = 4 COL_NUM_OCCURRENCES = 8 - assert priorities.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + assert critical.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + "instructions (linked below) when looking up items and allowable values during the data revision process" - assert priorities.cell(row=7, column=COL_ERROR_MESSAGE).value == ("TRAILER: record length is 15 characters " + assert critical.cell(row=7, column=COL_ERROR_MESSAGE).value == ("TRAILER: record length is 15 characters " "but must be 23.") - assert aggregates.cell(row=7, column=COL_NUM_OCCURRENCES).value == 5 + assert summary.cell(row=7, column=COL_NUM_OCCURRENCES).value == 5 @staticmethod def assert_error_report_file_content_matches_without_friendly_names(response): """Assert the error report file contents match expected without friendly names.""" - priorities, aggregates = DataFileAPITestBase.get_spreadsheet(response) + critical, summary = DataFileAPITestBase.get_spreadsheet(response) COL_ERROR_MESSAGE = 4 COL_NUM_OCCURRENCES = 8 - assert priorities.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + assert critical.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + "instructions (linked below) when looking up items and allowable values during the data revision process" - assert priorities.cell(row=8, column=COL_ERROR_MESSAGE).value == ( - "Every T1 record should have at least one corresponding T2 or T3 record with the same Item 4 " - "(Reporting Year and Month) and Item 6 (Case Number)." - ) - assert aggregates.cell(row=7, column=COL_NUM_OCCURRENCES).value == 1 + assert critical.cell(row=8, column=COL_ERROR_MESSAGE).value == "No records created." + assert summary.cell(row=7, column=COL_NUM_OCCURRENCES).value == 1 @staticmethod def assert_data_file_exists(data_file_data, version, user): diff --git a/tdrs-backend/tdpservice/data_files/util.py b/tdrs-backend/tdpservice/data_files/util.py index 6851bbec2..b7cc836b0 100644 --- a/tdrs-backend/tdpservice/data_files/util.py +++ b/tdrs-backend/tdpservice/data_files/util.py @@ -133,7 +133,7 @@ def write_prioritized_errors(worksheet, prioritized_errors, bold): for idx, col in enumerate(columns): worksheet.write(row, idx, format_header(col), bold) - paginator = Paginator(prioritized_errors.order_by('-pk'), settings.BULK_CREATE_BATCH_SIZE) + paginator = Paginator(prioritized_errors.order_by('pk'), settings.BULK_CREATE_BATCH_SIZE) row_idx = 6 for page in paginator: for record in page.object_list: @@ -193,8 +193,8 @@ def get_xls_serialized_file(all_errors, prioritized_errors): """Return xls file created from the error.""" output = BytesIO() workbook = xlsxwriter.Workbook(output) - prioritized_sheet = workbook.add_worksheet(name="Priorities") - aggregate_sheet = workbook.add_worksheet(name="Aggregates") + prioritized_sheet = workbook.add_worksheet(name="Critical") + aggregate_sheet = workbook.add_worksheet(name="Summary") write_worksheet_banner(prioritized_sheet) write_worksheet_banner(aggregate_sheet) diff --git a/tdrs-backend/tdpservice/data_files/views.py b/tdrs-backend/tdpservice/data_files/views.py index c67d701d7..8263fe62b 100644 --- a/tdrs-backend/tdpservice/data_files/views.py +++ b/tdrs-backend/tdpservice/data_files/views.py @@ -146,7 +146,7 @@ def download(self, request, pk=None): def download_error_report(self, request, pk=None): """Generate and return the parsing error report xlsx.""" datafile = self.get_object() - all_errors = ParserError.objects.filter(file=datafile).order_by('-pk') + all_errors = ParserError.objects.filter(file=datafile) filtered_errors = get_prioritized_queryset(all_errors) return Response(get_xls_serialized_file(all_errors, filtered_errors)) diff --git a/tdrs-backend/tdpservice/parsers/models.py b/tdrs-backend/tdpservice/parsers/models.py index 6a0ef71c7..f30a4f61a 100644 --- a/tdrs-backend/tdpservice/parsers/models.py +++ b/tdrs-backend/tdpservice/parsers/models.py @@ -128,12 +128,12 @@ def get_status(self): if errors is None: return DataFileSummary.Status.PENDING - elif (precheck_errors.count() > 0 or self.total_number_of_records_created == 0 or - prioritized_errors.count() > 500): + elif precheck_errors.count() > 0 or self.total_number_of_records_created == 0: return DataFileSummary.Status.REJECTED elif errors.count() == 0: return DataFileSummary.Status.ACCEPTED - elif row_precheck_errors.count() > 0 or case_consistency_errors.count() > 0: + elif (row_precheck_errors.count() > 0 or case_consistency_errors.count() > 0 or + prioritized_errors.count() > 500): return DataFileSummary.Status.PARTIALLY_ACCEPTED else: return DataFileSummary.Status.ACCEPTED_WITH_ERRORS diff --git a/tdrs-backend/tdpservice/parsers/test/test_parse.py b/tdrs-backend/tdpservice/parsers/test/test_parse.py index d7f918421..d01a44030 100644 --- a/tdrs-backend/tdpservice/parsers/test/test_parse.py +++ b/tdrs-backend/tdpservice/parsers/test/test_parse.py @@ -1098,10 +1098,10 @@ def test_parse_ssp_section2_file(ssp_section2_file, dfs): dfs.case_aggregates = aggregates.case_aggregates_by_month( dfs.datafile, dfs.status) for dfs_case_aggregate in dfs.case_aggregates['months']: - assert dfs_case_aggregate['accepted_without_errors'] == "N/A" - assert dfs_case_aggregate['accepted_with_errors'] == "N/A" + assert dfs_case_aggregate['accepted_without_errors'] == 0 + assert dfs_case_aggregate['accepted_with_errors'] in [75, 78] assert dfs_case_aggregate['month'] in ['Oct', 'Nov', 'Dec'] - assert dfs.get_status() == DataFileSummary.Status.REJECTED + assert dfs.get_status() == DataFileSummary.Status.PARTIALLY_ACCEPTED m4_objs = SSP_M4.objects.all().order_by('id') m5_objs = SSP_M5.objects.all().order_by('AMOUNT_EARNED_INCOME') From 50074662fd1dc2ba96de3d9a550deac2aca91e95 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Thu, 14 Nov 2024 08:59:18 -0500 Subject: [PATCH 30/74] - Linting --- tdrs-backend/tdpservice/data_files/test/test_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tdrs-backend/tdpservice/data_files/test/test_api.py b/tdrs-backend/tdpservice/data_files/test/test_api.py index ada702444..5fb3a0a5c 100644 --- a/tdrs-backend/tdpservice/data_files/test/test_api.py +++ b/tdrs-backend/tdpservice/data_files/test/test_api.py @@ -120,7 +120,7 @@ def assert_error_report_ssp_file_content_matches_with_friendly_names(response): assert critical.cell(row=1, column=1).value == "Please refer to the most recent versions of the coding " \ + "instructions (linked below) when looking up items and allowable values during the data revision process" assert critical.cell(row=7, column=COL_ERROR_MESSAGE).value == ("TRAILER: record length is 15 characters " - "but must be 23.") + "but must be 23.") assert summary.cell(row=7, column=COL_NUM_OCCURRENCES).value == 5 @staticmethod From a3a6919bcc07f7224fe17417d556c711c8956c60 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Wed, 20 Nov 2024 10:27:01 -0500 Subject: [PATCH 31/74] - Updated validators based on new coding instructions - Updated test --- tdrs-backend/tdpservice/parsers/schema_defs/ssp/m2.py | 2 +- tdrs-backend/tdpservice/parsers/schema_defs/tanf/t1.py | 6 +++--- tdrs-backend/tdpservice/parsers/test/test_parse.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tdrs-backend/tdpservice/parsers/schema_defs/ssp/m2.py b/tdrs-backend/tdpservice/parsers/schema_defs/ssp/m2.py index 82d5c2c46..20edf6fdb 100644 --- a/tdrs-backend/tdpservice/parsers/schema_defs/ssp/m2.py +++ b/tdrs-backend/tdpservice/parsers/schema_defs/ssp/m2.py @@ -317,7 +317,7 @@ startIndex=48, endIndex=49, required=False, - validators=[category2.isGreaterThan(0)] + validators=[category2.isGreaterThan(0, inclusive=True)] ), Field( item="32E", diff --git a/tdrs-backend/tdpservice/parsers/schema_defs/tanf/t1.py b/tdrs-backend/tdpservice/parsers/schema_defs/tanf/t1.py index 8f9aba575..3bd642c03 100644 --- a/tdrs-backend/tdpservice/parsers/schema_defs/tanf/t1.py +++ b/tdrs-backend/tdpservice/parsers/schema_defs/tanf/t1.py @@ -68,7 +68,7 @@ ), category3.ifThenAlso( condition_field_name="SANC_REDUCTION_AMT", - condition_function=category3.isGreaterThan(0), + condition_function=category3.isGreaterThan(0, inclusive=True), result_field_name="FAMILY_SANC_ADULT", result_function=category3.isOneOf((1, 2)), ), @@ -635,7 +635,7 @@ endIndex=114, required=False, validators=[ - category2.isOneOf(["9", " "]), + category2.isOneOf(["9", "0", " "]), category2.isAlphaNumeric(), ], ), @@ -658,7 +658,7 @@ endIndex=117, required=False, validators=[ - category2.isOneOf([1, 2]), + category2.isOneOf([0, 1, 2]), ], ), Field( diff --git a/tdrs-backend/tdpservice/parsers/test/test_parse.py b/tdrs-backend/tdpservice/parsers/test/test_parse.py index d01a44030..cc4f758a8 100644 --- a/tdrs-backend/tdpservice/parsers/test/test_parse.py +++ b/tdrs-backend/tdpservice/parsers/test/test_parse.py @@ -950,7 +950,7 @@ def test_parse_tanf_section1_blanks_file(tanf_section1_file_with_blanks, dfs): parser_errors = ParserError.objects.filter(file=tanf_section1_file_with_blanks) - assert parser_errors.count() == 22 + assert parser_errors.count() == 23 # Should only be cat3 validator errors for error in parser_errors: From d796870d77e6845c0501ddaf989d888240db12b5 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Wed, 20 Nov 2024 14:43:55 -0500 Subject: [PATCH 32/74] - Updated STTApiView to STTApiViewSet --- tdrs-backend/tdpservice/stts/urls.py | 8 +++++++- tdrs-backend/tdpservice/stts/views.py | 6 ++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/tdrs-backend/tdpservice/stts/urls.py b/tdrs-backend/tdpservice/stts/urls.py index 9fb01c24e..ae1edbb1a 100644 --- a/tdrs-backend/tdpservice/stts/urls.py +++ b/tdrs-backend/tdpservice/stts/urls.py @@ -1,10 +1,16 @@ """Routing for STTs.""" +from rest_framework.routers import DefaultRouter from django.urls import path from . import views +router = DefaultRouter() + +router.register("", views.STTApiViewSet) + urlpatterns = [ path("by_region", views.RegionAPIView.as_view(), name="stts-by-region"), path("alpha", views.STTApiAlphaView.as_view(), name="stts-alpha"), - path("", views.STTApiView.as_view(), name="stts"), ] + +urlpatterns += router.urls diff --git a/tdrs-backend/tdpservice/stts/views.py b/tdrs-backend/tdpservice/stts/views.py index 83a589d3c..5b1986089 100644 --- a/tdrs-backend/tdpservice/stts/views.py +++ b/tdrs-backend/tdpservice/stts/views.py @@ -2,7 +2,7 @@ import logging from django.db.models import Prefetch -from rest_framework import generics +from rest_framework import generics, mixins, viewsets from rest_framework.permissions import IsAuthenticated from tdpservice.stts.models import Region, STT from .serializers import RegionSerializer, STTSerializer @@ -30,7 +30,9 @@ class STTApiAlphaView(generics.ListAPIView): serializer_class = STTSerializer -class STTApiView(generics.ListAPIView): +class STTApiViewSet(mixins.ListModelMixin, + mixins.RetrieveModelMixin, + viewsets.GenericViewSet): """Simple view to get all STTs.""" pagination_class = None From 237e167e0e226d1ed5e9a76db464f420fa88f7aa Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Wed, 20 Nov 2024 15:16:51 -0500 Subject: [PATCH 33/74] - overriding retrieve method to key off of stt name instead of pk --- tdrs-backend/tdpservice/stts/views.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tdrs-backend/tdpservice/stts/views.py b/tdrs-backend/tdpservice/stts/views.py index 5b1986089..bc0607e62 100644 --- a/tdrs-backend/tdpservice/stts/views.py +++ b/tdrs-backend/tdpservice/stts/views.py @@ -2,8 +2,9 @@ import logging from django.db.models import Prefetch -from rest_framework import generics, mixins, viewsets +from rest_framework import generics, mixins, status, viewsets from rest_framework.permissions import IsAuthenticated +from rest_framework.response import Response from tdpservice.stts.models import Region, STT from .serializers import RegionSerializer, STTSerializer @@ -39,3 +40,14 @@ class STTApiViewSet(mixins.ListModelMixin, permission_classes = [IsAuthenticated] queryset = STT.objects serializer_class = STTSerializer + + def retrieve(self, request, pk=None): + """Return a specific user.""" + try: + stt = self.queryset.get(name=pk) + self.check_object_permissions(request, stt) + serializer = self.get_serializer_class()(stt) + return Response(serializer.data) + except Exception: + logger.exception(f"Caught exception trying to get STT with name {pk}.") + return Response(status=status.HTTP_404_NOT_FOUND) From ac7236c04d1b66fbd29e98a508d5c6bd5309cbc3 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Wed, 20 Nov 2024 15:47:27 -0500 Subject: [PATCH 34/74] - MVP of limiting submission boxes based on what an stt should be submitting --- tdrs-frontend/src/actions/reports.js | 9 +++++++-- .../SubmissionHistory/SubmissionHistory.jsx | 5 +++-- .../src/components/UploadReport/UploadReport.jsx | 7 ++++++- tdrs-frontend/src/reducers/reports.js | 11 ++++++----- 4 files changed, 22 insertions(+), 10 deletions(-) diff --git a/tdrs-frontend/src/actions/reports.js b/tdrs-frontend/src/actions/reports.js index 8ecb8839e..dcb459e89 100644 --- a/tdrs-frontend/src/actions/reports.js +++ b/tdrs-frontend/src/actions/reports.js @@ -269,8 +269,13 @@ export const SET_SELECTED_YEAR = 'SET_SELECTED_YEAR' export const SET_SELECTED_QUARTER = 'SET_SELECTED_QUARTER' export const SET_FILE_TYPE = 'SET_FILE_TYPE' -export const setStt = (stt) => (dispatch) => { - dispatch({ type: SET_SELECTED_STT, payload: { stt } }) +export const setStt = (stt) => async (dispatch) => { + const URL = `${process.env.REACT_APP_BACKEND_URL}/stts/${stt}` + const { data } = await axiosInstance.get(URL, { + withCredentials: true, + }) + const newUploadSections = Object.keys(data.filenames) + dispatch({ type: SET_SELECTED_STT, payload: { stt, newUploadSections } }) } export const setYear = (year) => (dispatch) => { dispatch({ type: SET_SELECTED_YEAR, payload: { year } }) diff --git a/tdrs-frontend/src/components/SubmissionHistory/SubmissionHistory.jsx b/tdrs-frontend/src/components/SubmissionHistory/SubmissionHistory.jsx index a1e28b7c0..5ddfb770f 100644 --- a/tdrs-frontend/src/components/SubmissionHistory/SubmissionHistory.jsx +++ b/tdrs-frontend/src/components/SubmissionHistory/SubmissionHistory.jsx @@ -1,8 +1,6 @@ import React from 'react' import PropTypes from 'prop-types' -import classNames from 'classnames' import { useDispatch, useSelector } from 'react-redux' -import { fileUploadSections } from '../../reducers/reports' import Paginator from '../Paginator' import { getAvailableFileList } from '../../actions/reports' import { useEffect } from 'react' @@ -64,6 +62,9 @@ const SubmissionHistory = ({ filterValues }) => { const dispatch = useDispatch() const [hasFetchedFiles, setHasFetchedFiles] = useState(false) const { files } = useSelector((state) => state.reports) + const fileUploadSections = useSelector( + (state) => state.reports.fileUploadSections + ) useEffect(() => { if (!hasFetchedFiles) { diff --git a/tdrs-frontend/src/components/UploadReport/UploadReport.jsx b/tdrs-frontend/src/components/UploadReport/UploadReport.jsx index 9e51c11a7..fa5b085a5 100644 --- a/tdrs-frontend/src/components/UploadReport/UploadReport.jsx +++ b/tdrs-frontend/src/components/UploadReport/UploadReport.jsx @@ -8,7 +8,6 @@ import Button from '../Button' import FileUpload from '../FileUpload' import { submit } from '../../actions/reports' import { useEventLogger } from '../../utils/eventLogger' -import { fileUploadSections } from '../../reducers/reports' function UploadReport({ handleCancel, stt }) { // The currently selected year from the reportingYears dropdown @@ -20,6 +19,12 @@ function UploadReport({ handleCancel, stt }) { // The set of uploaded files in our Redux state const files = useSelector((state) => state.reports.submittedFiles) + + // The set of sections the STT can report for + const fileUploadSections = useSelector( + (state) => state.reports.fileUploadSections + ) + // The logged in user in our Redux state const user = useSelector((state) => state.auth.user) diff --git a/tdrs-frontend/src/reducers/reports.js b/tdrs-frontend/src/reducers/reports.js index 9de986716..9f83843c1 100644 --- a/tdrs-frontend/src/reducers/reports.js +++ b/tdrs-frontend/src/reducers/reports.js @@ -23,7 +23,7 @@ const getFile = (files, section) => .sort((a, b) => b.id - a.id) .find((currentFile) => currentFile.section.includes(section)) -export const fileUploadSections = [ +export const defaultFileUploadSections = [ 'Active Case Data', 'Closed Case Data', 'Aggregate Data', @@ -73,7 +73,8 @@ export const serializeApiDataFile = (dataFile) => ({ const initialState = { files: [], - submittedFiles: fileUploadSections.map((section) => ({ + fileUploadSections: defaultFileUploadSections, + submittedFiles: defaultFileUploadSections.map((section) => ({ section, fileName: null, error: null, @@ -116,7 +117,7 @@ const reports = (state = initialState, action) => { ...state, isLoadingCurrentSubmission: false, currentSubmissionError: null, - submittedFiles: fileUploadSections.map((section) => { + submittedFiles: state.fileUploadSections.map((section) => { const file = getFile(data, section) if (file) { return serializeApiDataFile(file) @@ -201,8 +202,8 @@ const reports = (state = initialState, action) => { return { ...state, year } } case SET_SELECTED_STT: { - const { stt } = payload - return { ...state, stt } + const { stt, newUploadSections } = payload + return { ...state, stt, fileUploadSections: newUploadSections } } case SET_SELECTED_QUARTER: { const { quarter } = payload From 93f128c67763afcc9d0529abc617c7fd91aaf3d2 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Wed, 20 Nov 2024 16:28:40 -0500 Subject: [PATCH 35/74] - Fix logic to use correct section index. - need to sort uploads by section index --- .../SubmissionHistory/SubmissionHistory.jsx | 5 +++-- .../src/components/UploadReport/UploadReport.jsx | 12 ++++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/tdrs-frontend/src/components/SubmissionHistory/SubmissionHistory.jsx b/tdrs-frontend/src/components/SubmissionHistory/SubmissionHistory.jsx index 5ddfb770f..abddd326d 100644 --- a/tdrs-frontend/src/components/SubmissionHistory/SubmissionHistory.jsx +++ b/tdrs-frontend/src/components/SubmissionHistory/SubmissionHistory.jsx @@ -3,6 +3,7 @@ import PropTypes from 'prop-types' import { useDispatch, useSelector } from 'react-redux' import Paginator from '../Paginator' import { getAvailableFileList } from '../../actions/reports' +import { defaultFileUploadSections } from '../../reducers/reports' import { useEffect } from 'react' import { useState } from 'react' import { CaseAggregatesTable } from './CaseAggregatesTable' @@ -88,10 +89,10 @@ const SubmissionHistory = ({ filterValues }) => {
- {fileUploadSections.map((section, index) => ( + {fileUploadSections.map((section) => ( f.section.includes(section))} diff --git a/tdrs-frontend/src/components/UploadReport/UploadReport.jsx b/tdrs-frontend/src/components/UploadReport/UploadReport.jsx index fa5b085a5..451c1ae26 100644 --- a/tdrs-frontend/src/components/UploadReport/UploadReport.jsx +++ b/tdrs-frontend/src/components/UploadReport/UploadReport.jsx @@ -7,6 +7,7 @@ import Button from '../Button' import FileUpload from '../FileUpload' import { submit } from '../../actions/reports' +import { defaultFileUploadSections } from '../../reducers/reports' import { useEventLogger } from '../../utils/eventLogger' function UploadReport({ handleCancel, stt }) { @@ -44,7 +45,7 @@ function UploadReport({ handleCancel, stt }) { const uploadedFiles = files?.filter((file) => file.fileName && !file.id) const uploadedSections = uploadedFiles ? uploadedFiles - .map((file) => fileUploadSections.indexOf(file.section) + 1) + .map((file) => defaultFileUploadSections.indexOf(file.section) + 1) .join(', ') .split(' ') : [] @@ -110,10 +111,13 @@ function UploadReport({ handleCancel, stt }) {
)}
- {fileUploadSections.map((name, index) => ( + {fileUploadSections.map((section) => ( ))} From 2ab340f006942d1c9a3dbcadada91836e0ea35c3 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Thu, 21 Nov 2024 08:47:39 -0500 Subject: [PATCH 36/74] - Updated map logic to correctly order the sections --- .../SubmissionHistory/SubmissionHistory.jsx | 22 +++++++++++-------- .../components/UploadReport/UploadReport.jsx | 21 +++++++++--------- 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/tdrs-frontend/src/components/SubmissionHistory/SubmissionHistory.jsx b/tdrs-frontend/src/components/SubmissionHistory/SubmissionHistory.jsx index abddd326d..0dbab2937 100644 --- a/tdrs-frontend/src/components/SubmissionHistory/SubmissionHistory.jsx +++ b/tdrs-frontend/src/components/SubmissionHistory/SubmissionHistory.jsx @@ -89,15 +89,19 @@ const SubmissionHistory = ({ filterValues }) => {
- {fileUploadSections.map((section) => ( - f.section.includes(section))} - /> - ))} + {defaultFileUploadSections.map((section, index) => { + if (fileUploadSections.includes(section)) { + return ( + f.section.includes(section))} + /> + ) + } + })}
) diff --git a/tdrs-frontend/src/components/UploadReport/UploadReport.jsx b/tdrs-frontend/src/components/UploadReport/UploadReport.jsx index 451c1ae26..62e07f3ab 100644 --- a/tdrs-frontend/src/components/UploadReport/UploadReport.jsx +++ b/tdrs-frontend/src/components/UploadReport/UploadReport.jsx @@ -111,16 +111,17 @@ function UploadReport({ handleCancel, stt }) { )} - {fileUploadSections.map((section) => ( - - ))} + {defaultFileUploadSections.map((section, index) => { + if (fileUploadSections.includes(section)) { + return ( + + ) + } + })}
{defaultFileUploadSections.map((section, index) => { - if (fileUploadSections.includes(section)) { + if (fileUploadSections?.includes(section)) { return ( {defaultFileUploadSections.map((section, index) => { - if (fileUploadSections.includes(section)) { + if (fileUploadSections?.includes(section)) { return ( Date: Thu, 21 Nov 2024 10:13:36 -0500 Subject: [PATCH 41/74] - add missing expected values --- tdrs-frontend/src/actions/reports.test.js | 1 + tdrs-frontend/src/reducers/reports.test.js | 1 + 2 files changed, 2 insertions(+) diff --git a/tdrs-frontend/src/actions/reports.test.js b/tdrs-frontend/src/actions/reports.test.js index 40593f3bb..cb111f9a6 100644 --- a/tdrs-frontend/src/actions/reports.test.js +++ b/tdrs-frontend/src/actions/reports.test.js @@ -238,6 +238,7 @@ describe('actions/reports', () => { expect(actions[0].type).toBe(SET_SELECTED_STT) expect(actions[0].payload).toStrictEqual({ stt: 'florida', + newUploadSections: [], }) }) diff --git a/tdrs-frontend/src/reducers/reports.test.js b/tdrs-frontend/src/reducers/reports.test.js index c96ca88e2..ab8369b35 100644 --- a/tdrs-frontend/src/reducers/reports.test.js +++ b/tdrs-frontend/src/reducers/reports.test.js @@ -411,6 +411,7 @@ describe('reducers/reports', () => { type: SET_SELECTED_STT, payload: { stt: 'florida', + newUploadSections: [], }, }) ).toEqual({ From ba5c8906b90aff99996ab73849933dfcda6052ce Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Thu, 21 Nov 2024 10:32:35 -0500 Subject: [PATCH 42/74] - add default to missing state in tests --- tdrs-frontend/src/reducers/reports.test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tdrs-frontend/src/reducers/reports.test.js b/tdrs-frontend/src/reducers/reports.test.js index ab8369b35..362deb5fa 100644 --- a/tdrs-frontend/src/reducers/reports.test.js +++ b/tdrs-frontend/src/reducers/reports.test.js @@ -1,5 +1,5 @@ import { v4 as uuidv4 } from 'uuid' -import reducer, { getUpdatedFiles } from './reports' +import reducer, { defaultFileUploadSections, getUpdatedFiles } from './reports' import { CLEAR_ERROR, SET_FILE, @@ -14,6 +14,7 @@ import { const initialState = { files: [], + fileUploadSections: defaultFileUploadSections, submittedFiles: [ { section: 'Active Case Data', From fdf88a0b345b788bf43c657c7780f8835df3d3d9 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Thu, 21 Nov 2024 11:11:09 -0500 Subject: [PATCH 43/74] - fix reducer tests --- tdrs-frontend/src/reducers/reports.test.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tdrs-frontend/src/reducers/reports.test.js b/tdrs-frontend/src/reducers/reports.test.js index 362deb5fa..bbedc5360 100644 --- a/tdrs-frontend/src/reducers/reports.test.js +++ b/tdrs-frontend/src/reducers/reports.test.js @@ -82,6 +82,7 @@ describe('reducers/reports', () => { submittedFiles: initialState.submittedFiles, isLoadingCurrentSubmission: initialState.isLoadingCurrentSubmission, currentSubmissionError: initialState.currentSubmissionError, + fileUploadSections: initialState.fileUploadSections, files: [ { fileName: 'test.txt', @@ -118,6 +119,7 @@ describe('reducers/reports', () => { files: initialState.files, isLoadingCurrentSubmission: initialState.isLoadingCurrentSubmission, currentSubmissionError: initialState.currentSubmissionError, + fileUploadSections: initialState.fileUploadSections, submittedFiles: [ { section: 'Active Case Data', @@ -178,6 +180,7 @@ describe('reducers/reports', () => { files: initialState.files, isLoadingCurrentSubmission: initialState.isLoadingCurrentSubmission, currentSubmissionError: initialState.currentSubmissionError, + fileUploadSections: initialState.fileUploadSections, submittedFiles: [ { section: 'Active Case Data', @@ -229,6 +232,7 @@ describe('reducers/reports', () => { files: initialState.files, isLoadingCurrentSubmission: initialState.isLoadingCurrentSubmission, currentSubmissionError: initialState.currentSubmissionError, + fileUploadSections: initialState.fileUploadSections, submittedFiles: [ { section: 'Active Case Data', @@ -282,6 +286,7 @@ describe('reducers/reports', () => { files: initialState.files, isLoadingCurrentSubmission: initialState.isLoadingCurrentSubmission, currentSubmissionError: initialState.currentSubmissionError, + fileUploadSections: initialState.fileUploadSections, submittedFiles: [ { section: 'Active Case Data', @@ -420,6 +425,7 @@ describe('reducers/reports', () => { isLoadingCurrentSubmission: initialState.isLoadingCurrentSubmission, currentSubmissionError: initialState.currentSubmissionError, submittedFiles: initialState.submittedFiles, + fileUploadSections: [], year: '', stt: 'florida', quarter: '', @@ -440,6 +446,7 @@ describe('reducers/reports', () => { isLoadingCurrentSubmission: initialState.isLoadingCurrentSubmission, currentSubmissionError: initialState.currentSubmissionError, submittedFiles: initialState.submittedFiles, + fileUploadSections: initialState.fileUploadSections, year: '', stt: '', quarter: 'Q1', @@ -458,6 +465,7 @@ describe('reducers/reports', () => { isLoadingCurrentSubmission: initialState.isLoadingCurrentSubmission, currentSubmissionError: initialState.currentSubmissionError, submittedFiles: initialState.submittedFiles, + fileUploadSections: initialState.fileUploadSections, year: '', stt: '', quarter: 'Q2', @@ -476,6 +484,7 @@ describe('reducers/reports', () => { isLoadingCurrentSubmission: initialState.isLoadingCurrentSubmission, currentSubmissionError: initialState.currentSubmissionError, submittedFiles: initialState.submittedFiles, + fileUploadSections: initialState.fileUploadSections, year: '', stt: '', quarter: 'Q3', @@ -493,6 +502,7 @@ describe('reducers/reports', () => { isLoadingCurrentSubmission: initialState.isLoadingCurrentSubmission, currentSubmissionError: initialState.currentSubmissionError, submittedFiles: initialState.submittedFiles, + fileUploadSections: initialState.fileUploadSections, year: '', stt: '', quarter: 'Q4', @@ -513,6 +523,7 @@ describe('reducers/reports', () => { isLoadingCurrentSubmission: initialState.isLoadingCurrentSubmission, currentSubmissionError: initialState.currentSubmissionError, submittedFiles: initialState.submittedFiles, + fileUploadSections: initialState.fileUploadSections, year: '2021', stt: '', quarter: '', From 8dd7723b2705f5bc519fa9e8f2c7519af541fa11 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Thu, 21 Nov 2024 11:33:46 -0500 Subject: [PATCH 44/74] - Updated submission history tests with correct state --- .../SubmissionHistory/SubmissionHistory.jsx | 2 +- .../SubmissionHistory/SubmissionHistory.test.js | 11 +++++++++-- .../src/components/UploadReport/UploadReport.jsx | 2 +- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/tdrs-frontend/src/components/SubmissionHistory/SubmissionHistory.jsx b/tdrs-frontend/src/components/SubmissionHistory/SubmissionHistory.jsx index 4b69132c7..0dbab2937 100644 --- a/tdrs-frontend/src/components/SubmissionHistory/SubmissionHistory.jsx +++ b/tdrs-frontend/src/components/SubmissionHistory/SubmissionHistory.jsx @@ -90,7 +90,7 @@ const SubmissionHistory = ({ filterValues }) => {
{defaultFileUploadSections.map((section, index) => { - if (fileUploadSections?.includes(section)) { + if (fileUploadSections.includes(section)) { return ( { const initialState = { reports: { files: [], + fileUploadSections: defaultFileUploadSections, }, } @@ -18,7 +20,7 @@ describe('SubmissionHistory', () => { const defaultFilterValues = { quarter: 'Q1', year: '2023', - stt: { id: 4 }, + stt: { id: 5 }, file_type: 'TANF', } @@ -51,6 +53,7 @@ describe('SubmissionHistory', () => { it('Shows first five results on first page', () => { const state = { reports: { + fileUploadSections: defaultFileUploadSections, files: [ { id: '123', @@ -145,6 +148,7 @@ describe('SubmissionHistory', () => { it('Shows next five results on next page', () => { const state = { reports: { + fileUploadSections: defaultFileUploadSections, files: [ { id: '123', @@ -247,6 +251,7 @@ describe('SubmissionHistory', () => { it('Shows SSP results when SSP-MOE file type selected', () => { const state = { reports: { + fileUploadSections: defaultFileUploadSections, files: [ { id: '123', @@ -324,7 +329,7 @@ describe('SubmissionHistory', () => { setup(store, { ...defaultFilterValues, - stt: { id: 48 }, + stt: { id: 5 }, file_type: 'SSP', }) @@ -354,6 +359,7 @@ describe('SubmissionHistory', () => { (status, section) => { const state = { reports: { + fileUploadSections: defaultFileUploadSections, files: [ { id: '123', @@ -430,6 +436,7 @@ describe('SubmissionHistory', () => { (status, section) => { const state = { reports: { + fileUploadSections: defaultFileUploadSections, files: [ { id: '123', diff --git a/tdrs-frontend/src/components/UploadReport/UploadReport.jsx b/tdrs-frontend/src/components/UploadReport/UploadReport.jsx index 1b66dec12..62e07f3ab 100644 --- a/tdrs-frontend/src/components/UploadReport/UploadReport.jsx +++ b/tdrs-frontend/src/components/UploadReport/UploadReport.jsx @@ -112,7 +112,7 @@ function UploadReport({ handleCancel, stt }) { )} {defaultFileUploadSections.map((section, index) => { - if (fileUploadSections?.includes(section)) { + if (fileUploadSections.includes(section)) { return ( Date: Thu, 21 Nov 2024 11:36:48 -0500 Subject: [PATCH 45/74] - Fix upload report tests --- tdrs-frontend/src/components/UploadReport/UploadReport.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/tdrs-frontend/src/components/UploadReport/UploadReport.test.js b/tdrs-frontend/src/components/UploadReport/UploadReport.test.js index b8cc66efa..2167847ad 100644 --- a/tdrs-frontend/src/components/UploadReport/UploadReport.test.js +++ b/tdrs-frontend/src/components/UploadReport/UploadReport.test.js @@ -169,6 +169,7 @@ describe('UploadReport', () => { const store = mockStore({ ...initialState, reports: { + fileUploadSections: defaultFileUploadSections, submittedFiles: [ { section: 'Active Case Data', From 8700e5e5ef7726b8b94bcd0eb3a9c7bcd33c6e1b Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Thu, 21 Nov 2024 11:58:14 -0500 Subject: [PATCH 46/74] - Correct logic if reponse is null --- tdrs-frontend/src/actions/reports.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tdrs-frontend/src/actions/reports.js b/tdrs-frontend/src/actions/reports.js index 0fda9fb2a..1c55f14eb 100644 --- a/tdrs-frontend/src/actions/reports.js +++ b/tdrs-frontend/src/actions/reports.js @@ -271,11 +271,12 @@ export const SET_FILE_TYPE = 'SET_FILE_TYPE' export const setStt = (stt) => async (dispatch) => { const URL = `${process.env.REACT_APP_BACKEND_URL}/stts/${stt}` - const data = await axiosInstance.get(URL, { + const response = await axiosInstance.get(URL, { withCredentials: true, }) - const newUploadSections = - typeof data !== 'undefined' ? Object.keys(data.filenames) : [] + const data = + typeof response !== 'undefined' ? response.data : { filenames: [] } + const newUploadSections = Object.keys(data.filenames) dispatch({ type: SET_SELECTED_STT, payload: { stt, newUploadSections } }) } export const setYear = (year) => (dispatch) => { From 640dd9b46c3165b922d4fa77b934d73fe51cf64b Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Thu, 21 Nov 2024 13:57:30 -0500 Subject: [PATCH 47/74] - Updated action to use correct return type when api request fails - Updated tests --- tdrs-frontend/src/actions/reports.js | 14 ++++++++++++-- tdrs-frontend/src/actions/reports.test.js | 3 ++- .../src/components/Reports/Reports.test.js | 2 ++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/tdrs-frontend/src/actions/reports.js b/tdrs-frontend/src/actions/reports.js index 1c55f14eb..e4ed33838 100644 --- a/tdrs-frontend/src/actions/reports.js +++ b/tdrs-frontend/src/actions/reports.js @@ -4,6 +4,7 @@ import axios from 'axios' import axiosInstance from '../axios-instance' import { logErrorToServer } from '../utils/eventLogger' import removeFileInputErrorState from '../utils/removeFileInputErrorState' +import { defaultFileUploadSections } from '../reducers/reports' const BACKEND_URL = process.env.REACT_APP_BACKEND_URL @@ -274,8 +275,17 @@ export const setStt = (stt) => async (dispatch) => { const response = await axiosInstance.get(URL, { withCredentials: true, }) - const data = - typeof response !== 'undefined' ? response.data : { filenames: [] } + + var data = {} + if (typeof response !== 'undefined') { + data = response.data + } else { + var sectionMap = {} + defaultFileUploadSections.forEach((section, index) => { + return (sectionMap[section] = defaultFileUploadSections.length - index) + }) + data = { filenames: sectionMap } + } const newUploadSections = Object.keys(data.filenames) dispatch({ type: SET_SELECTED_STT, payload: { stt, newUploadSections } }) } diff --git a/tdrs-frontend/src/actions/reports.test.js b/tdrs-frontend/src/actions/reports.test.js index cb111f9a6..38782094e 100644 --- a/tdrs-frontend/src/actions/reports.test.js +++ b/tdrs-frontend/src/actions/reports.test.js @@ -22,6 +22,7 @@ import { submit, SET_FILE_SUBMITTED, } from './reports' +import { defaultFileUploadSections } from '../reducers/reports' describe('actions/reports', () => { const mockStore = configureStore([thunk]) @@ -238,7 +239,7 @@ describe('actions/reports', () => { expect(actions[0].type).toBe(SET_SELECTED_STT) expect(actions[0].payload).toStrictEqual({ stt: 'florida', - newUploadSections: [], + newUploadSections: defaultFileUploadSections, }) }) diff --git a/tdrs-frontend/src/components/Reports/Reports.test.js b/tdrs-frontend/src/components/Reports/Reports.test.js index 2c97a1b5c..4b3b742dd 100644 --- a/tdrs-frontend/src/components/Reports/Reports.test.js +++ b/tdrs-frontend/src/components/Reports/Reports.test.js @@ -7,10 +7,12 @@ import configureStore from 'redux-mock-store' import appConfigureStore from '../../configureStore' import Reports from './Reports' import { SET_FILE, upload } from '../../actions/reports' +import { defaultFileUploadSections } from '../../reducers/reports' describe('Reports', () => { const initialState = { reports: { + fileUploadSections: defaultFileUploadSections, files: [ { section: 'Active Case Data', From 35ad65659933fe8be075a35bad0e0a29230a429e Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Thu, 21 Nov 2024 14:12:35 -0500 Subject: [PATCH 48/74] - remove unnecessary branch --- tdrs-frontend/src/actions/reports.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tdrs-frontend/src/actions/reports.js b/tdrs-frontend/src/actions/reports.js index e4ed33838..9fbe7b2a8 100644 --- a/tdrs-frontend/src/actions/reports.js +++ b/tdrs-frontend/src/actions/reports.js @@ -276,10 +276,8 @@ export const setStt = (stt) => async (dispatch) => { withCredentials: true, }) - var data = {} - if (typeof response !== 'undefined') { - data = response.data - } else { + var data = response?.data + if (typeof data === 'undefined') { var sectionMap = {} defaultFileUploadSections.forEach((section, index) => { return (sectionMap[section] = defaultFileUploadSections.length - index) From 32c42d4d0b2866816dc1c852d2cdccfddf1b98ec Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Thu, 21 Nov 2024 14:27:31 -0500 Subject: [PATCH 49/74] - Update tests to reverse based on viewset --- tdrs-backend/tdpservice/stts/test/test_api.py | 8 ++++---- tdrs-backend/tdpservice/stts/urls.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tdrs-backend/tdpservice/stts/test/test_api.py b/tdrs-backend/tdpservice/stts/test/test_api.py index 38fc2d9b6..761e9b36e 100644 --- a/tdrs-backend/tdpservice/stts/test/test_api.py +++ b/tdrs-backend/tdpservice/stts/test/test_api.py @@ -12,14 +12,14 @@ def test_stts_is_valid_endpoint(api_client, stt_user): """Test an authorized user can successfully query the STT endpoint.""" api_client.login(username=stt_user.username, password="test_password") - response = api_client.get(reverse("stts")) + response = api_client.get(reverse("stts-list")) assert response.status_code == status.HTTP_200_OK @pytest.mark.django_db def test_stts_blocks_unauthorized(api_client, stt_user): """Test an unauthorized user cannot successfully query the STT endpoint.""" - response = api_client.get(reverse("stts")) + response = api_client.get(reverse("stts-list")) assert response.status_code == status.HTTP_403_FORBIDDEN @@ -57,7 +57,7 @@ def test_stts_by_region_blocks_unauthorized(api_client, stt_user): def test_can_get_stts(api_client, stt_user, stts): """Test endpoint returns a listing of states, tribes and territories.""" api_client.login(username=stt_user.username, password="test_password") - response = api_client.get(reverse("stts")) + response = api_client.get(reverse("stts-list")) assert response.status_code == status.HTTP_200_OK assert len(response.data) == STT.objects.count() @@ -117,5 +117,5 @@ def test_stts_and_stts_alpha_are_dissimilar(api_client, stt_user, stts): """The default STTs endpoint is not sorted the same as the alpha.""" api_client.login(username=stt_user.username, password="test_password") alpha_response = api_client.get(reverse("stts-alpha")) - default_response = api_client.get(reverse("stts")) + default_response = api_client.get(reverse("stts-list")) assert not alpha_response.data == default_response.data diff --git a/tdrs-backend/tdpservice/stts/urls.py b/tdrs-backend/tdpservice/stts/urls.py index ae1edbb1a..f51ef3f66 100644 --- a/tdrs-backend/tdpservice/stts/urls.py +++ b/tdrs-backend/tdpservice/stts/urls.py @@ -6,7 +6,7 @@ router = DefaultRouter() -router.register("", views.STTApiViewSet) +router.register("", views.STTApiViewSet, basename="stts") urlpatterns = [ path("by_region", views.RegionAPIView.as_view(), name="stts-by-region"), From eb5d532c147ad7283aad9329aaa6dfda9ddbfa69 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Thu, 21 Nov 2024 15:05:55 -0500 Subject: [PATCH 50/74] - add test for file extenstion reducer --- tdrs-frontend/src/reducers/reports.test.js | 47 ++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tdrs-frontend/src/reducers/reports.test.js b/tdrs-frontend/src/reducers/reports.test.js index bbedc5360..965886a19 100644 --- a/tdrs-frontend/src/reducers/reports.test.js +++ b/tdrs-frontend/src/reducers/reports.test.js @@ -4,6 +4,7 @@ import { CLEAR_ERROR, SET_FILE, CLEAR_FILE, + FILE_EXT_ERROR, SET_FILE_ERROR, SET_SELECTED_YEAR, SET_SELECTED_STT, @@ -272,6 +273,52 @@ describe('reducers/reports', () => { }) }) + it('should handle FILE_EXT_ERROR', () => { + expect( + reducer(undefined, { + type: FILE_EXT_ERROR, + payload: { + error: { message: "Test invalid ext." }, + section: "Active Case Data", + }, + }) + ).toEqual({ + ...initialState, + submittedFiles: [ + { + id: null, + file: null, + section: 'Active Case Data', + fileName: undefined, + error: { message: "Test invalid ext." }, + uuid: null, + fileType: null + }, + { + section: 'Closed Case Data', + fileName: null, + error: null, + uuid: null, + fileType: null + }, + { + section: 'Aggregate Data', + fileName: null, + error: null, + uuid: null, + fileType: null + }, + { + section: 'Stratum Data', + fileName: null, + error: null, + uuid: null, + fileType: null + } + ], + }) + }) + it('should handle SET_FILE_ERROR', () => { const fakeError = new Error({ message: 'something went wrong' }) expect( From 0cdd029b718e02e4900fd73a77ffc057ff789318 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Thu, 21 Nov 2024 15:12:34 -0500 Subject: [PATCH 51/74] - linting --- tdrs-frontend/src/reducers/reports.test.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tdrs-frontend/src/reducers/reports.test.js b/tdrs-frontend/src/reducers/reports.test.js index 965886a19..2e1ce4dd6 100644 --- a/tdrs-frontend/src/reducers/reports.test.js +++ b/tdrs-frontend/src/reducers/reports.test.js @@ -278,8 +278,8 @@ describe('reducers/reports', () => { reducer(undefined, { type: FILE_EXT_ERROR, payload: { - error: { message: "Test invalid ext." }, - section: "Active Case Data", + error: { message: 'Test invalid ext.' }, + section: 'Active Case Data', }, }) ).toEqual({ @@ -290,31 +290,31 @@ describe('reducers/reports', () => { file: null, section: 'Active Case Data', fileName: undefined, - error: { message: "Test invalid ext." }, + error: { message: 'Test invalid ext.' }, uuid: null, - fileType: null + fileType: null, }, { section: 'Closed Case Data', fileName: null, error: null, uuid: null, - fileType: null + fileType: null, }, { section: 'Aggregate Data', fileName: null, error: null, uuid: null, - fileType: null + fileType: null, }, { section: 'Stratum Data', fileName: null, error: null, uuid: null, - fileType: null - } + fileType: null, + }, ], }) }) From 078f8c57af41b7c62290a30b519447f2dcfe4ae7 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Fri, 22 Nov 2024 11:03:50 -0500 Subject: [PATCH 52/74] - Added error handling for empty stt --- tdrs-frontend/src/actions/reports.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tdrs-frontend/src/actions/reports.js b/tdrs-frontend/src/actions/reports.js index 9fbe7b2a8..a6c84852f 100644 --- a/tdrs-frontend/src/actions/reports.js +++ b/tdrs-frontend/src/actions/reports.js @@ -276,8 +276,8 @@ export const setStt = (stt) => async (dispatch) => { withCredentials: true, }) - var data = response?.data - if (typeof data === 'undefined') { + var data = typeof response === 'undefined' ? undefined : response.data + if (typeof data === 'undefined' || stt === '') { var sectionMap = {} defaultFileUploadSections.forEach((section, index) => { return (sectionMap[section] = defaultFileUploadSections.length - index) From d84f055899a8ed9f095682512796ccfab1ee8b08 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Fri, 22 Nov 2024 12:01:20 -0500 Subject: [PATCH 53/74] - add test for empty stt --- tdrs-frontend/src/actions/reports.test.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tdrs-frontend/src/actions/reports.test.js b/tdrs-frontend/src/actions/reports.test.js index 38782094e..8b042e27c 100644 --- a/tdrs-frontend/src/actions/reports.test.js +++ b/tdrs-frontend/src/actions/reports.test.js @@ -243,6 +243,19 @@ describe('actions/reports', () => { }) }) + it('should dispatch SET_SELECTED_STT with empty stt', async () => { + const store = mockStore() + + await store.dispatch(setStt('')) + + const actions = store.getActions() + expect(actions[0].type).toBe(SET_SELECTED_STT) + expect(actions[0].payload).toStrictEqual({ + stt: '', + newUploadSections: defaultFileUploadSections, + }) + }) + it('should dispatch SET_SELECTED_QUARTER', async () => { const store = mockStore() From 3e8b6c1534b02d75f05bf615bf62e80bb08ef8fb Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Fri, 22 Nov 2024 12:05:25 -0500 Subject: [PATCH 54/74] - Update logic to aviod extra conditional --- tdrs-frontend/src/actions/reports.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tdrs-frontend/src/actions/reports.js b/tdrs-frontend/src/actions/reports.js index a6c84852f..ab114e12e 100644 --- a/tdrs-frontend/src/actions/reports.js +++ b/tdrs-frontend/src/actions/reports.js @@ -276,7 +276,7 @@ export const setStt = (stt) => async (dispatch) => { withCredentials: true, }) - var data = typeof response === 'undefined' ? undefined : response.data + var data = response?.data if (typeof data === 'undefined' || stt === '') { var sectionMap = {} defaultFileUploadSections.forEach((section, index) => { From d74edb3c49e203c3288d854da58abe675bb370f9 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Mon, 25 Nov 2024 09:10:14 -0500 Subject: [PATCH 55/74] - Moved section logic mostly to the backend - Updated the frontend to leverage backend work --- tdrs-backend/tdpservice/stts/views.py | 16 +++++++++------- tdrs-frontend/src/actions/reports.js | 13 ++++--------- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/tdrs-backend/tdpservice/stts/views.py b/tdrs-backend/tdpservice/stts/views.py index f5d86e7eb..6b4d414b4 100644 --- a/tdrs-backend/tdpservice/stts/views.py +++ b/tdrs-backend/tdpservice/stts/views.py @@ -3,6 +3,7 @@ from django.db.models import Prefetch from rest_framework import generics, mixins, status, viewsets +from rest_framework.decorators import action from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from tdpservice.stts.models import Region, STT @@ -32,7 +33,6 @@ class STTApiAlphaView(generics.ListAPIView): class STTApiViewSet(mixins.ListModelMixin, - mixins.RetrieveModelMixin, viewsets.GenericViewSet): """Simple view to get all STTs.""" @@ -40,14 +40,16 @@ class STTApiViewSet(mixins.ListModelMixin, permission_classes = [IsAuthenticated] queryset = STT.objects serializer_class = STTSerializer + lookup_field = "name" - def retrieve(self, request, pk=None): - """Return a specific stt based on stt name.""" + @action(methods=["get"], detail=True) + def num_sections(self, request, name=None): + """Return number of sections an stt submits based on stt name.""" try: - stt = self.queryset.get(name=pk) + stt = self.queryset.get(name=name) self.check_object_permissions(request, stt) - serializer = self.get_serializer_class()(stt) - return Response(serializer.data) + divisor = int(stt.ssp) + 1 + return Response({"num_sections": len(stt.filenames) // divisor}) except Exception: - logger.exception(f"Caught exception trying to get STT with name {pk}.") + logger.exception(f"Caught exception trying to get STT with name {stt}.") return Response(status=status.HTTP_404_NOT_FOUND) diff --git a/tdrs-frontend/src/actions/reports.js b/tdrs-frontend/src/actions/reports.js index ab114e12e..5ac897dc0 100644 --- a/tdrs-frontend/src/actions/reports.js +++ b/tdrs-frontend/src/actions/reports.js @@ -271,20 +271,15 @@ export const SET_SELECTED_QUARTER = 'SET_SELECTED_QUARTER' export const SET_FILE_TYPE = 'SET_FILE_TYPE' export const setStt = (stt) => async (dispatch) => { - const URL = `${process.env.REACT_APP_BACKEND_URL}/stts/${stt}` + const URL = `${process.env.REACT_APP_BACKEND_URL}/stts/${stt}/num_sections` const response = await axiosInstance.get(URL, { withCredentials: true, }) - var data = response?.data - if (typeof data === 'undefined' || stt === '') { - var sectionMap = {} - defaultFileUploadSections.forEach((section, index) => { - return (sectionMap[section] = defaultFileUploadSections.length - index) - }) - data = { filenames: sectionMap } + var newUploadSections = defaultFileUploadSections + if (typeof response !== 'undefined') { + newUploadSections = newUploadSections.slice(0, response.data.num_sections) } - const newUploadSections = Object.keys(data.filenames) dispatch({ type: SET_SELECTED_STT, payload: { stt, newUploadSections } }) } export const setYear = (year) => (dispatch) => { From 33087d297db08dfabe952f5eede8a7f674afff00 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Mon, 25 Nov 2024 09:15:52 -0500 Subject: [PATCH 56/74] - Updated logic on components --- .../SubmissionHistory/SubmissionHistory.jsx | 22 +++++++++---------- .../components/UploadReport/UploadReport.jsx | 18 +++++++-------- 2 files changed, 18 insertions(+), 22 deletions(-) diff --git a/tdrs-frontend/src/components/SubmissionHistory/SubmissionHistory.jsx b/tdrs-frontend/src/components/SubmissionHistory/SubmissionHistory.jsx index 0dbab2937..8002cb4a1 100644 --- a/tdrs-frontend/src/components/SubmissionHistory/SubmissionHistory.jsx +++ b/tdrs-frontend/src/components/SubmissionHistory/SubmissionHistory.jsx @@ -89,18 +89,16 @@ const SubmissionHistory = ({ filterValues }) => {
- {defaultFileUploadSections.map((section, index) => { - if (fileUploadSections.includes(section)) { - return ( - f.section.includes(section))} - /> - ) - } + {fileUploadSections.map((section, index) => { + return ( + f.section.includes(section))} + /> + ) })}
diff --git a/tdrs-frontend/src/components/UploadReport/UploadReport.jsx b/tdrs-frontend/src/components/UploadReport/UploadReport.jsx index 62e07f3ab..0126ddf75 100644 --- a/tdrs-frontend/src/components/UploadReport/UploadReport.jsx +++ b/tdrs-frontend/src/components/UploadReport/UploadReport.jsx @@ -111,16 +111,14 @@ function UploadReport({ handleCancel, stt }) { )} - {defaultFileUploadSections.map((section, index) => { - if (fileUploadSections.includes(section)) { - return ( - - ) - } + {fileUploadSections.map((section, index) => { + return ( + + ) })}
From 97f277f87786fc2baa0b5a697364d35d20a655da Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Mon, 25 Nov 2024 10:12:07 -0500 Subject: [PATCH 57/74] - add empty stt check --- tdrs-frontend/src/actions/reports.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tdrs-frontend/src/actions/reports.js b/tdrs-frontend/src/actions/reports.js index 5ac897dc0..545cd640e 100644 --- a/tdrs-frontend/src/actions/reports.js +++ b/tdrs-frontend/src/actions/reports.js @@ -271,10 +271,13 @@ export const SET_SELECTED_QUARTER = 'SET_SELECTED_QUARTER' export const SET_FILE_TYPE = 'SET_FILE_TYPE' export const setStt = (stt) => async (dispatch) => { - const URL = `${process.env.REACT_APP_BACKEND_URL}/stts/${stt}/num_sections` - const response = await axiosInstance.get(URL, { - withCredentials: true, - }) + var response = undefined + if (stt !== '') { + const URL = `${process.env.REACT_APP_BACKEND_URL}/stts/${stt}/num_sections` + response = await axiosInstance.get(URL, { + withCredentials: true, + }) + } var newUploadSections = defaultFileUploadSections if (typeof response !== 'undefined') { From bf8b7a0d8374fbd9521694e10a15f5dfa935aa9a Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Mon, 25 Nov 2024 11:09:11 -0500 Subject: [PATCH 58/74] - remove status change based on critical errors --- tdrs-backend/tdpservice/parsers/models.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tdrs-backend/tdpservice/parsers/models.py b/tdrs-backend/tdpservice/parsers/models.py index f30a4f61a..583521385 100644 --- a/tdrs-backend/tdpservice/parsers/models.py +++ b/tdrs-backend/tdpservice/parsers/models.py @@ -124,16 +124,13 @@ def get_status(self): .filter(field_name="Record_Type")\ .exclude(error_message__icontains="trailer") - prioritized_errors = get_prioritized_queryset(errors) - if errors is None: return DataFileSummary.Status.PENDING elif precheck_errors.count() > 0 or self.total_number_of_records_created == 0: return DataFileSummary.Status.REJECTED elif errors.count() == 0: return DataFileSummary.Status.ACCEPTED - elif (row_precheck_errors.count() > 0 or case_consistency_errors.count() > 0 or - prioritized_errors.count() > 500): + elif (row_precheck_errors.count() > 0 or case_consistency_errors.count()): return DataFileSummary.Status.PARTIALLY_ACCEPTED else: return DataFileSummary.Status.ACCEPTED_WITH_ERRORS From 9c23ad506645dc814c85cc532cac1b3c7929c6ae Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Mon, 25 Nov 2024 11:36:26 -0500 Subject: [PATCH 59/74] - linting --- tdrs-backend/tdpservice/parsers/models.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tdrs-backend/tdpservice/parsers/models.py b/tdrs-backend/tdpservice/parsers/models.py index 583521385..5af51bec1 100644 --- a/tdrs-backend/tdpservice/parsers/models.py +++ b/tdrs-backend/tdpservice/parsers/models.py @@ -5,7 +5,6 @@ from django.contrib.contenttypes.fields import GenericForeignKey from django.contrib.contenttypes.models import ContentType from tdpservice.data_files.models import DataFile -from tdpservice.data_files.util import ParserErrorCategoryChoices, get_prioritized_queryset import logging From c8050b15d2ad3aa9f805381a4d5c1a1718c8e5a9 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Mon, 25 Nov 2024 11:41:30 -0500 Subject: [PATCH 60/74] - fix errro --- tdrs-backend/tdpservice/parsers/models.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tdrs-backend/tdpservice/parsers/models.py b/tdrs-backend/tdpservice/parsers/models.py index 5af51bec1..f1e470e6e 100644 --- a/tdrs-backend/tdpservice/parsers/models.py +++ b/tdrs-backend/tdpservice/parsers/models.py @@ -5,6 +5,7 @@ from django.contrib.contenttypes.fields import GenericForeignKey from django.contrib.contenttypes.models import ContentType from tdpservice.data_files.models import DataFile +from tdpservice.data_files.util import ParserErrorCategoryChoices import logging From 2e5100dfdb8cbd500d735579f5e491ed8e1af9fb Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Wed, 4 Dec 2024 11:13:02 -0500 Subject: [PATCH 61/74] - Updated stt serializer to include the number of sections an stt submits - Reverted code to hit `num_sections` endpoint - Reverted tests - Updated components to use new field in stt state --- tdrs-backend/tdpservice/stts/models.py | 8 ++++++++ tdrs-backend/tdpservice/stts/serializers.py | 2 +- tdrs-backend/tdpservice/stts/test/test_api.py | 8 ++++---- tdrs-backend/tdpservice/stts/urls.py | 8 +------- tdrs-backend/tdpservice/stts/views.py | 20 ++----------------- tdrs-frontend/src/actions/reports.js | 16 ++------------- tdrs-frontend/src/actions/reports.test.js | 3 --- .../src/components/Reports/Reports.jsx | 2 +- .../src/components/Reports/Reports.test.js | 2 -- .../SubmissionHistory/SubmissionHistory.jsx | 8 +++----- .../SubmissionHistory.test.js | 7 ------- .../components/UploadReport/UploadReport.jsx | 19 +++++++++--------- .../UploadReport/UploadReport.test.js | 3 --- tdrs-frontend/src/reducers/reports.js | 11 +++++----- tdrs-frontend/src/reducers/reports.test.js | 15 +------------- 15 files changed, 37 insertions(+), 95 deletions(-) diff --git a/tdrs-backend/tdpservice/stts/models.py b/tdrs-backend/tdpservice/stts/models.py index b883ded74..f26b1a6b0 100644 --- a/tdrs-backend/tdpservice/stts/models.py +++ b/tdrs-backend/tdpservice/stts/models.py @@ -39,6 +39,14 @@ class EntityType(models.TextChoices): ssp = models.BooleanField(default=False, null=True) sample = models.BooleanField(default=False, null=True) + @property + def num_sections(self): + """The number of sections this STT submits.""" + if self.filenames is None: + return 4 + divisor = int(self.ssp) + 1 + return len(self.filenames) // divisor + class Meta: """Metadata.""" diff --git a/tdrs-backend/tdpservice/stts/serializers.py b/tdrs-backend/tdpservice/stts/serializers.py index be2ec88b6..c683784aa 100644 --- a/tdrs-backend/tdpservice/stts/serializers.py +++ b/tdrs-backend/tdpservice/stts/serializers.py @@ -14,7 +14,7 @@ class Meta: """Metadata.""" model = STT - fields = ["id", "type", "postal_code", "name", "region", "filenames", "stt_code", "ssp",] + fields = ["id", "type", "postal_code", "name", "region", "filenames", "stt_code", "ssp", 'num_sections'] def get_postal_code(self, obj): """Return the state postal_code.""" diff --git a/tdrs-backend/tdpservice/stts/test/test_api.py b/tdrs-backend/tdpservice/stts/test/test_api.py index 761e9b36e..38fc2d9b6 100644 --- a/tdrs-backend/tdpservice/stts/test/test_api.py +++ b/tdrs-backend/tdpservice/stts/test/test_api.py @@ -12,14 +12,14 @@ def test_stts_is_valid_endpoint(api_client, stt_user): """Test an authorized user can successfully query the STT endpoint.""" api_client.login(username=stt_user.username, password="test_password") - response = api_client.get(reverse("stts-list")) + response = api_client.get(reverse("stts")) assert response.status_code == status.HTTP_200_OK @pytest.mark.django_db def test_stts_blocks_unauthorized(api_client, stt_user): """Test an unauthorized user cannot successfully query the STT endpoint.""" - response = api_client.get(reverse("stts-list")) + response = api_client.get(reverse("stts")) assert response.status_code == status.HTTP_403_FORBIDDEN @@ -57,7 +57,7 @@ def test_stts_by_region_blocks_unauthorized(api_client, stt_user): def test_can_get_stts(api_client, stt_user, stts): """Test endpoint returns a listing of states, tribes and territories.""" api_client.login(username=stt_user.username, password="test_password") - response = api_client.get(reverse("stts-list")) + response = api_client.get(reverse("stts")) assert response.status_code == status.HTTP_200_OK assert len(response.data) == STT.objects.count() @@ -117,5 +117,5 @@ def test_stts_and_stts_alpha_are_dissimilar(api_client, stt_user, stts): """The default STTs endpoint is not sorted the same as the alpha.""" api_client.login(username=stt_user.username, password="test_password") alpha_response = api_client.get(reverse("stts-alpha")) - default_response = api_client.get(reverse("stts-list")) + default_response = api_client.get(reverse("stts")) assert not alpha_response.data == default_response.data diff --git a/tdrs-backend/tdpservice/stts/urls.py b/tdrs-backend/tdpservice/stts/urls.py index f51ef3f66..9fb01c24e 100644 --- a/tdrs-backend/tdpservice/stts/urls.py +++ b/tdrs-backend/tdpservice/stts/urls.py @@ -1,16 +1,10 @@ """Routing for STTs.""" -from rest_framework.routers import DefaultRouter from django.urls import path from . import views -router = DefaultRouter() - -router.register("", views.STTApiViewSet, basename="stts") - urlpatterns = [ path("by_region", views.RegionAPIView.as_view(), name="stts-by-region"), path("alpha", views.STTApiAlphaView.as_view(), name="stts-alpha"), + path("", views.STTApiView.as_view(), name="stts"), ] - -urlpatterns += router.urls diff --git a/tdrs-backend/tdpservice/stts/views.py b/tdrs-backend/tdpservice/stts/views.py index 6b4d414b4..83a589d3c 100644 --- a/tdrs-backend/tdpservice/stts/views.py +++ b/tdrs-backend/tdpservice/stts/views.py @@ -2,10 +2,8 @@ import logging from django.db.models import Prefetch -from rest_framework import generics, mixins, status, viewsets -from rest_framework.decorators import action +from rest_framework import generics from rest_framework.permissions import IsAuthenticated -from rest_framework.response import Response from tdpservice.stts.models import Region, STT from .serializers import RegionSerializer, STTSerializer @@ -32,24 +30,10 @@ class STTApiAlphaView(generics.ListAPIView): serializer_class = STTSerializer -class STTApiViewSet(mixins.ListModelMixin, - viewsets.GenericViewSet): +class STTApiView(generics.ListAPIView): """Simple view to get all STTs.""" pagination_class = None permission_classes = [IsAuthenticated] queryset = STT.objects serializer_class = STTSerializer - lookup_field = "name" - - @action(methods=["get"], detail=True) - def num_sections(self, request, name=None): - """Return number of sections an stt submits based on stt name.""" - try: - stt = self.queryset.get(name=name) - self.check_object_permissions(request, stt) - divisor = int(stt.ssp) + 1 - return Response({"num_sections": len(stt.filenames) // divisor}) - except Exception: - logger.exception(f"Caught exception trying to get STT with name {stt}.") - return Response(status=status.HTTP_404_NOT_FOUND) diff --git a/tdrs-frontend/src/actions/reports.js b/tdrs-frontend/src/actions/reports.js index 545cd640e..9e0844cb1 100644 --- a/tdrs-frontend/src/actions/reports.js +++ b/tdrs-frontend/src/actions/reports.js @@ -4,7 +4,7 @@ import axios from 'axios' import axiosInstance from '../axios-instance' import { logErrorToServer } from '../utils/eventLogger' import removeFileInputErrorState from '../utils/removeFileInputErrorState' -import { defaultFileUploadSections } from '../reducers/reports' +import { fileUploadSections } from '../reducers/reports' const BACKEND_URL = process.env.REACT_APP_BACKEND_URL @@ -271,19 +271,7 @@ export const SET_SELECTED_QUARTER = 'SET_SELECTED_QUARTER' export const SET_FILE_TYPE = 'SET_FILE_TYPE' export const setStt = (stt) => async (dispatch) => { - var response = undefined - if (stt !== '') { - const URL = `${process.env.REACT_APP_BACKEND_URL}/stts/${stt}/num_sections` - response = await axiosInstance.get(URL, { - withCredentials: true, - }) - } - - var newUploadSections = defaultFileUploadSections - if (typeof response !== 'undefined') { - newUploadSections = newUploadSections.slice(0, response.data.num_sections) - } - dispatch({ type: SET_SELECTED_STT, payload: { stt, newUploadSections } }) + dispatch({ type: SET_SELECTED_STT, payload: { stt } }) } export const setYear = (year) => (dispatch) => { dispatch({ type: SET_SELECTED_YEAR, payload: { year } }) diff --git a/tdrs-frontend/src/actions/reports.test.js b/tdrs-frontend/src/actions/reports.test.js index 8b042e27c..294e31c9a 100644 --- a/tdrs-frontend/src/actions/reports.test.js +++ b/tdrs-frontend/src/actions/reports.test.js @@ -22,7 +22,6 @@ import { submit, SET_FILE_SUBMITTED, } from './reports' -import { defaultFileUploadSections } from '../reducers/reports' describe('actions/reports', () => { const mockStore = configureStore([thunk]) @@ -239,7 +238,6 @@ describe('actions/reports', () => { expect(actions[0].type).toBe(SET_SELECTED_STT) expect(actions[0].payload).toStrictEqual({ stt: 'florida', - newUploadSections: defaultFileUploadSections, }) }) @@ -252,7 +250,6 @@ describe('actions/reports', () => { expect(actions[0].type).toBe(SET_SELECTED_STT) expect(actions[0].payload).toStrictEqual({ stt: '', - newUploadSections: defaultFileUploadSections, }) }) diff --git a/tdrs-frontend/src/components/Reports/Reports.jsx b/tdrs-frontend/src/components/Reports/Reports.jsx index a22ae4fb1..0ac0f3d98 100644 --- a/tdrs-frontend/src/components/Reports/Reports.jsx +++ b/tdrs-frontend/src/components/Reports/Reports.jsx @@ -455,7 +455,7 @@ function Reports() { {selectedSubmissionTab === 1 && ( { setIsToggled(false) resetPreviousValues() diff --git a/tdrs-frontend/src/components/Reports/Reports.test.js b/tdrs-frontend/src/components/Reports/Reports.test.js index 4b3b742dd..2c97a1b5c 100644 --- a/tdrs-frontend/src/components/Reports/Reports.test.js +++ b/tdrs-frontend/src/components/Reports/Reports.test.js @@ -7,12 +7,10 @@ import configureStore from 'redux-mock-store' import appConfigureStore from '../../configureStore' import Reports from './Reports' import { SET_FILE, upload } from '../../actions/reports' -import { defaultFileUploadSections } from '../../reducers/reports' describe('Reports', () => { const initialState = { reports: { - fileUploadSections: defaultFileUploadSections, files: [ { section: 'Active Case Data', diff --git a/tdrs-frontend/src/components/SubmissionHistory/SubmissionHistory.jsx b/tdrs-frontend/src/components/SubmissionHistory/SubmissionHistory.jsx index 8002cb4a1..b768fb7cd 100644 --- a/tdrs-frontend/src/components/SubmissionHistory/SubmissionHistory.jsx +++ b/tdrs-frontend/src/components/SubmissionHistory/SubmissionHistory.jsx @@ -3,7 +3,7 @@ import PropTypes from 'prop-types' import { useDispatch, useSelector } from 'react-redux' import Paginator from '../Paginator' import { getAvailableFileList } from '../../actions/reports' -import { defaultFileUploadSections } from '../../reducers/reports' +import { fileUploadSections } from '../../reducers/reports' import { useEffect } from 'react' import { useState } from 'react' import { CaseAggregatesTable } from './CaseAggregatesTable' @@ -63,9 +63,7 @@ const SubmissionHistory = ({ filterValues }) => { const dispatch = useDispatch() const [hasFetchedFiles, setHasFetchedFiles] = useState(false) const { files } = useSelector((state) => state.reports) - const fileUploadSections = useSelector( - (state) => state.reports.fileUploadSections - ) + const num_sections = filterValues.stt.num_sections useEffect(() => { if (!hasFetchedFiles) { @@ -89,7 +87,7 @@ const SubmissionHistory = ({ filterValues }) => {
- {fileUploadSections.map((section, index) => { + {fileUploadSections.slice(0, num_sections).map((section, index) => { return ( { const initialState = { reports: { files: [], - fileUploadSections: defaultFileUploadSections, }, } @@ -53,7 +51,6 @@ describe('SubmissionHistory', () => { it('Shows first five results on first page', () => { const state = { reports: { - fileUploadSections: defaultFileUploadSections, files: [ { id: '123', @@ -148,7 +145,6 @@ describe('SubmissionHistory', () => { it('Shows next five results on next page', () => { const state = { reports: { - fileUploadSections: defaultFileUploadSections, files: [ { id: '123', @@ -251,7 +247,6 @@ describe('SubmissionHistory', () => { it('Shows SSP results when SSP-MOE file type selected', () => { const state = { reports: { - fileUploadSections: defaultFileUploadSections, files: [ { id: '123', @@ -359,7 +354,6 @@ describe('SubmissionHistory', () => { (status, section) => { const state = { reports: { - fileUploadSections: defaultFileUploadSections, files: [ { id: '123', @@ -436,7 +430,6 @@ describe('SubmissionHistory', () => { (status, section) => { const state = { reports: { - fileUploadSections: defaultFileUploadSections, files: [ { id: '123', diff --git a/tdrs-frontend/src/components/UploadReport/UploadReport.jsx b/tdrs-frontend/src/components/UploadReport/UploadReport.jsx index 0126ddf75..bac32a9b9 100644 --- a/tdrs-frontend/src/components/UploadReport/UploadReport.jsx +++ b/tdrs-frontend/src/components/UploadReport/UploadReport.jsx @@ -7,7 +7,7 @@ import Button from '../Button' import FileUpload from '../FileUpload' import { submit } from '../../actions/reports' -import { defaultFileUploadSections } from '../../reducers/reports' +import { fileUploadSections } from '../../reducers/reports' import { useEventLogger } from '../../utils/eventLogger' function UploadReport({ handleCancel, stt }) { @@ -21,14 +21,13 @@ function UploadReport({ handleCancel, stt }) { // The set of uploaded files in our Redux state const files = useSelector((state) => state.reports.submittedFiles) - // The set of sections the STT can report for - const fileUploadSections = useSelector( - (state) => state.reports.fileUploadSections - ) - // The logged in user in our Redux state const user = useSelector((state) => state.auth.user) + // The number of sections this stt submits data for and it's ID + const stt_id = stt === undefined ? undefined : stt.id + const num_sections = stt === undefined ? 4 : stt.num_sections + // TODO: Move this to Redux state so we can modify this value outside of // this component without having to pass the setter function around const [localAlert, setLocalAlertState] = useState({ @@ -45,7 +44,7 @@ function UploadReport({ handleCancel, stt }) { const uploadedFiles = files?.filter((file) => file.fileName && !file.id) const uploadedSections = uploadedFiles ? uploadedFiles - .map((file) => defaultFileUploadSections.indexOf(file.section) + 1) + .map((file) => fileUploadSections.indexOf(file.section) + 1) .join(', ') .split(' ') : [] @@ -76,7 +75,7 @@ function UploadReport({ handleCancel, stt }) { formattedSections, logger, setLocalAlertState, - stt, + stt_id, uploadedFiles, user, ssp: selectedFileType === 'ssp-moe', @@ -111,7 +110,7 @@ function UploadReport({ handleCancel, stt }) {
)} - {fileUploadSections.map((section, index) => { + {fileUploadSections.slice(0, num_sections).map((section, index) => { return ( { const initialState = { auth: { user: { email: 'test@test.com' }, authenticated: true }, reports: { - fileUploadSections: defaultFileUploadSections, submittedFiles: [ { fileName: 'test.txt', @@ -169,7 +167,6 @@ describe('UploadReport', () => { const store = mockStore({ ...initialState, reports: { - fileUploadSections: defaultFileUploadSections, submittedFiles: [ { section: 'Active Case Data', diff --git a/tdrs-frontend/src/reducers/reports.js b/tdrs-frontend/src/reducers/reports.js index 9f83843c1..9de986716 100644 --- a/tdrs-frontend/src/reducers/reports.js +++ b/tdrs-frontend/src/reducers/reports.js @@ -23,7 +23,7 @@ const getFile = (files, section) => .sort((a, b) => b.id - a.id) .find((currentFile) => currentFile.section.includes(section)) -export const defaultFileUploadSections = [ +export const fileUploadSections = [ 'Active Case Data', 'Closed Case Data', 'Aggregate Data', @@ -73,8 +73,7 @@ export const serializeApiDataFile = (dataFile) => ({ const initialState = { files: [], - fileUploadSections: defaultFileUploadSections, - submittedFiles: defaultFileUploadSections.map((section) => ({ + submittedFiles: fileUploadSections.map((section) => ({ section, fileName: null, error: null, @@ -117,7 +116,7 @@ const reports = (state = initialState, action) => { ...state, isLoadingCurrentSubmission: false, currentSubmissionError: null, - submittedFiles: state.fileUploadSections.map((section) => { + submittedFiles: fileUploadSections.map((section) => { const file = getFile(data, section) if (file) { return serializeApiDataFile(file) @@ -202,8 +201,8 @@ const reports = (state = initialState, action) => { return { ...state, year } } case SET_SELECTED_STT: { - const { stt, newUploadSections } = payload - return { ...state, stt, fileUploadSections: newUploadSections } + const { stt } = payload + return { ...state, stt } } case SET_SELECTED_QUARTER: { const { quarter } = payload diff --git a/tdrs-frontend/src/reducers/reports.test.js b/tdrs-frontend/src/reducers/reports.test.js index 2e1ce4dd6..6e41b21f7 100644 --- a/tdrs-frontend/src/reducers/reports.test.js +++ b/tdrs-frontend/src/reducers/reports.test.js @@ -1,5 +1,5 @@ import { v4 as uuidv4 } from 'uuid' -import reducer, { defaultFileUploadSections, getUpdatedFiles } from './reports' +import reducer, { getUpdatedFiles } from './reports' import { CLEAR_ERROR, SET_FILE, @@ -15,7 +15,6 @@ import { const initialState = { files: [], - fileUploadSections: defaultFileUploadSections, submittedFiles: [ { section: 'Active Case Data', @@ -83,7 +82,6 @@ describe('reducers/reports', () => { submittedFiles: initialState.submittedFiles, isLoadingCurrentSubmission: initialState.isLoadingCurrentSubmission, currentSubmissionError: initialState.currentSubmissionError, - fileUploadSections: initialState.fileUploadSections, files: [ { fileName: 'test.txt', @@ -120,7 +118,6 @@ describe('reducers/reports', () => { files: initialState.files, isLoadingCurrentSubmission: initialState.isLoadingCurrentSubmission, currentSubmissionError: initialState.currentSubmissionError, - fileUploadSections: initialState.fileUploadSections, submittedFiles: [ { section: 'Active Case Data', @@ -181,7 +178,6 @@ describe('reducers/reports', () => { files: initialState.files, isLoadingCurrentSubmission: initialState.isLoadingCurrentSubmission, currentSubmissionError: initialState.currentSubmissionError, - fileUploadSections: initialState.fileUploadSections, submittedFiles: [ { section: 'Active Case Data', @@ -233,7 +229,6 @@ describe('reducers/reports', () => { files: initialState.files, isLoadingCurrentSubmission: initialState.isLoadingCurrentSubmission, currentSubmissionError: initialState.currentSubmissionError, - fileUploadSections: initialState.fileUploadSections, submittedFiles: [ { section: 'Active Case Data', @@ -333,7 +328,6 @@ describe('reducers/reports', () => { files: initialState.files, isLoadingCurrentSubmission: initialState.isLoadingCurrentSubmission, currentSubmissionError: initialState.currentSubmissionError, - fileUploadSections: initialState.fileUploadSections, submittedFiles: [ { section: 'Active Case Data', @@ -464,7 +458,6 @@ describe('reducers/reports', () => { type: SET_SELECTED_STT, payload: { stt: 'florida', - newUploadSections: [], }, }) ).toEqual({ @@ -472,7 +465,6 @@ describe('reducers/reports', () => { isLoadingCurrentSubmission: initialState.isLoadingCurrentSubmission, currentSubmissionError: initialState.currentSubmissionError, submittedFiles: initialState.submittedFiles, - fileUploadSections: [], year: '', stt: 'florida', quarter: '', @@ -493,7 +485,6 @@ describe('reducers/reports', () => { isLoadingCurrentSubmission: initialState.isLoadingCurrentSubmission, currentSubmissionError: initialState.currentSubmissionError, submittedFiles: initialState.submittedFiles, - fileUploadSections: initialState.fileUploadSections, year: '', stt: '', quarter: 'Q1', @@ -512,7 +503,6 @@ describe('reducers/reports', () => { isLoadingCurrentSubmission: initialState.isLoadingCurrentSubmission, currentSubmissionError: initialState.currentSubmissionError, submittedFiles: initialState.submittedFiles, - fileUploadSections: initialState.fileUploadSections, year: '', stt: '', quarter: 'Q2', @@ -531,7 +521,6 @@ describe('reducers/reports', () => { isLoadingCurrentSubmission: initialState.isLoadingCurrentSubmission, currentSubmissionError: initialState.currentSubmissionError, submittedFiles: initialState.submittedFiles, - fileUploadSections: initialState.fileUploadSections, year: '', stt: '', quarter: 'Q3', @@ -549,7 +538,6 @@ describe('reducers/reports', () => { isLoadingCurrentSubmission: initialState.isLoadingCurrentSubmission, currentSubmissionError: initialState.currentSubmissionError, submittedFiles: initialState.submittedFiles, - fileUploadSections: initialState.fileUploadSections, year: '', stt: '', quarter: 'Q4', @@ -570,7 +558,6 @@ describe('reducers/reports', () => { isLoadingCurrentSubmission: initialState.isLoadingCurrentSubmission, currentSubmissionError: initialState.currentSubmissionError, submittedFiles: initialState.submittedFiles, - fileUploadSections: initialState.fileUploadSections, year: '2021', stt: '', quarter: '', From 0eedadf4514dfd20746699ae81ee879122339eb0 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Wed, 4 Dec 2024 12:31:50 -0500 Subject: [PATCH 62/74] - remove async - correct error --- tdrs-frontend/src/actions/reports.js | 2 +- tdrs-frontend/src/components/UploadReport/UploadReport.jsx | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tdrs-frontend/src/actions/reports.js b/tdrs-frontend/src/actions/reports.js index 9e0844cb1..766aafc7f 100644 --- a/tdrs-frontend/src/actions/reports.js +++ b/tdrs-frontend/src/actions/reports.js @@ -270,7 +270,7 @@ export const SET_SELECTED_YEAR = 'SET_SELECTED_YEAR' export const SET_SELECTED_QUARTER = 'SET_SELECTED_QUARTER' export const SET_FILE_TYPE = 'SET_FILE_TYPE' -export const setStt = (stt) => async (dispatch) => { +export const setStt = (stt) => (dispatch) => { dispatch({ type: SET_SELECTED_STT, payload: { stt } }) } export const setYear = (year) => (dispatch) => { diff --git a/tdrs-frontend/src/components/UploadReport/UploadReport.jsx b/tdrs-frontend/src/components/UploadReport/UploadReport.jsx index bac32a9b9..a2348fe65 100644 --- a/tdrs-frontend/src/components/UploadReport/UploadReport.jsx +++ b/tdrs-frontend/src/components/UploadReport/UploadReport.jsx @@ -25,7 +25,7 @@ function UploadReport({ handleCancel, stt }) { const user = useSelector((state) => state.auth.user) // The number of sections this stt submits data for and it's ID - const stt_id = stt === undefined ? undefined : stt.id + const stt_id = stt?.id const num_sections = stt === undefined ? 4 : stt.num_sections // TODO: Move this to Redux state so we can modify this value outside of @@ -75,7 +75,7 @@ function UploadReport({ handleCancel, stt }) { formattedSections, logger, setLocalAlertState, - stt_id, + stt: stt_id, uploadedFiles, user, ssp: selectedFileType === 'ssp-moe', From 1ddf0b19e879cd47affcfc658fc115c322877357 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Thu, 5 Dec 2024 09:28:20 -0500 Subject: [PATCH 63/74] - use constant - use double quotes --- tdrs-backend/tdpservice/stts/models.py | 5 ++++- tdrs-backend/tdpservice/stts/serializers.py | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/tdrs-backend/tdpservice/stts/models.py b/tdrs-backend/tdpservice/stts/models.py index f26b1a6b0..b960d0e55 100644 --- a/tdrs-backend/tdpservice/stts/models.py +++ b/tdrs-backend/tdpservice/stts/models.py @@ -4,6 +4,9 @@ from django.db.models import constraints +DEFAULT_NUMBER_OF_SECTIONS = 4 + + class Region(models.Model): """A model representing a US region.""" @@ -43,7 +46,7 @@ class EntityType(models.TextChoices): def num_sections(self): """The number of sections this STT submits.""" if self.filenames is None: - return 4 + return DEFAULT_NUMBER_OF_SECTIONS divisor = int(self.ssp) + 1 return len(self.filenames) // divisor diff --git a/tdrs-backend/tdpservice/stts/serializers.py b/tdrs-backend/tdpservice/stts/serializers.py index c683784aa..7774e87ab 100644 --- a/tdrs-backend/tdpservice/stts/serializers.py +++ b/tdrs-backend/tdpservice/stts/serializers.py @@ -14,7 +14,7 @@ class Meta: """Metadata.""" model = STT - fields = ["id", "type", "postal_code", "name", "region", "filenames", "stt_code", "ssp", 'num_sections'] + fields = ["id", "type", "postal_code", "name", "region", "filenames", "stt_code", "ssp", "num_sections"] def get_postal_code(self, obj): """Return the state postal_code.""" From ff241a8c9502811229536b3077d3bf94a951213c Mon Sep 17 00:00:00 2001 From: Eric Lipe <125676261+elipe17@users.noreply.github.com> Date: Thu, 5 Dec 2024 10:08:54 -0500 Subject: [PATCH 64/74] Update docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md Co-authored-by: Alex P. <63075587+ADPennington@users.noreply.github.com> --- .../tech-memos/priotitized-errors/prioritized-errors.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md b/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md index 36f937c01..c37528d4e 100644 --- a/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md +++ b/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md @@ -5,7 +5,7 @@ **Date**: October 20, 2024
## Summary -This technical memorandum provides a suggested path to implement a set of new requirements OFA has generated to alleviate the sheer number of parser errors generated during a STT's data submission. OFA has indicated, while still necessary for admins to see, some errors are of a lower priority for STTs to view and even prevent them from using the submission history reports to correct their submissions. Thus, the OFA team has requested that a "priority" be assigned to parser errors so that the report STTs receive is filtered down to only the highest priority errors that must be fixed. Regardless of how errors are prioritized, OFA admins will still retain the ability to generate the unfiltered error report for further, more in-depth investigation if a necessary situation presents itself. +This technical memorandum provides a suggested path to implement a set of new requirements OFA has generated to alleviate the sheer number of parser errors generated during a STT's data submission. OFA has indicated that some errors are of a lower priority for STTs to review and correct. Thus, the OFA team has requested that "critical" be assigned to parser errors so that the report STTs receive is filtered down to only the critical errors that must be reviewed and fixed. Regardless of how errors are prioritized, STTs will still retain the ability to see a summary of all errors detected in the error report. ## Background Currently, error reports are generated in the TDP backend via the `get_xls_serialized_file` function. This function accepts the serialized queryset of the appropriate `ParserError`s queryset. This function the writes an XLSX file and returns it to the user. Apart from the lack of priotization in the report generated from this function, it also introduces the possibility to cause an out of memory (OOM) error. This can occur because, the Django model serializer brings the entire queryset into memory to serialize it into JSON. Because these ParserError querysets can be very large (hundreds of thousands), we will also alleviate the memory pressure `get_xls_serialized_file` introduces by removing the Django model serializer and make use of queryset pagination. From be2a74ce841d2c047089200e9554f27837ba196e Mon Sep 17 00:00:00 2001 From: Eric Lipe <125676261+elipe17@users.noreply.github.com> Date: Thu, 5 Dec 2024 10:15:23 -0500 Subject: [PATCH 65/74] Update docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md Co-authored-by: Alex P. <63075587+ADPennington@users.noreply.github.com> --- .../tech-memos/priotitized-errors/prioritized-errors.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md b/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md index c37528d4e..7f104104c 100644 --- a/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md +++ b/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md @@ -14,7 +14,7 @@ Currently, error reports are generated in the TDP backend via the `get_xls_seria Current requirements from OFA do not require category two errors to be queryable by value and expected value. That feature is out of scope within the tech memo and would require more design and implementation work. ## Method/Design -Given the current OFA requirements, we can implement prioritized errors, and memory efficient report generation without too much work. OFA has provided [this OneNote](https://gorafttech.sharepoint.com/:o:/s/TDRSResearchDesign/EnIa1Mn4v7pOskW7BLomXhIBxUMlYLRU_f1C0dxemW7dWw?e=m0rNyI) document which outlines the error types, errors, and fields that are most important/prioritized for STTs to see. +Given the current OFA requirements, we can implement prioritized/critical errors, and memory efficient report generation without too much work. OFA has provided [this OneNote](https://gorafttech.sharepoint.com/:o:/s/TDRSResearchDesign/EnIa1Mn4v7pOskW7BLomXhIBxUMlYLRU_f1C0dxemW7dWw?e=m0rNyI) document which outlines the error types, errors, and fields that are most important/prioritized for STTs to see. ### Memory Efficient Report Generation As previously mentioned in the #background section, the `get_xls_serialized_file` introduces a method to serialize parser errors into a XLSX that requires the entire queryset of parser errors to be brought into memory. Because these querysets can be very large, having them in memory regularly kills Gunicorn workers with an OOM error. To remedy the issue, this tech memo suggests updating `get_xls_serialized_file` to not use Django model serializers and instead leverage the power of Django querysets and pagination. To accomplish this, instead of passing a JSON serialized querset to `get_xls_serialized_file`, a standard (un-evaluated) queryset should be passed. Then, the body of the `get_xls_serialized_file` function should be updated appropriately to use a queryset object instead of a JSON object to generate the XLSX spreadsheet. The updates should also include paginating the queryset to avoid bringing the entirety of the queryset into memory at any one time. The code snippet below provides an example of paginating the queryset and writing the appropriate fields of each entry to the XLSX report. From 25a7d8d6edf3a1c34e8f422035038469a3a49475 Mon Sep 17 00:00:00 2001 From: Eric Lipe <125676261+elipe17@users.noreply.github.com> Date: Thu, 5 Dec 2024 10:15:39 -0500 Subject: [PATCH 66/74] Update docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md Co-authored-by: Alex P. <63075587+ADPennington@users.noreply.github.com> --- .../tech-memos/priotitized-errors/prioritized-errors.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md b/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md index 7f104104c..13a0b30a8 100644 --- a/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md +++ b/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md @@ -58,7 +58,7 @@ def internal_names(fields_json): return ','.join([i for i in fields_json['friendly_name'].keys()]) ``` -### Prioritized Errors +### Prioritized/Critical Errors [This OneNote](https://gorafttech.sharepoint.com/:o:/s/TDRSResearchDesign/EnIa1Mn4v7pOskW7BLomXhIBxUMlYLRU_f1C0dxemW7dWw?e=m0rNyI) is invaluable to the implementation of prioritized errors. Prioritizing errors could be a very large and technically challenging feature involving new migrations, validation/validator refactors, etc... However, this can all be avoided by making a key insight for each of the category two and category three validators by way of OFA's requirements for them. For the category two case, the OneNote document generically specifies category two validation surrounding: Family Affiliation, Citizenship and Closure reason. Further discussion with OFA indicated that it is important/a priority for a STT to see all category two errors encompassing these fields in their entirety. That makes prioritizing these category two errors extremely easy because the need to query those fields by specific values and expected values is not required. The queries below provide a complete implementation to query all category two errors encompassing those fields. ```python From e054283dca4bdd3b6067d2d97424c341cac5c004 Mon Sep 17 00:00:00 2001 From: Eric Lipe <125676261+elipe17@users.noreply.github.com> Date: Thu, 5 Dec 2024 10:37:04 -0500 Subject: [PATCH 67/74] Update docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md Co-authored-by: Alex P. <63075587+ADPennington@users.noreply.github.com> --- .../tech-memos/priotitized-errors/prioritized-errors.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md b/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md index 13a0b30a8..a282971be 100644 --- a/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md +++ b/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md @@ -82,7 +82,7 @@ category3.ifThenAlso( ) ``` -The existence of this error, with these fields, is uniquely defined in the appropriate schemas. The same can be said for the remaining high priority category three errors. Thus, to define the high priority errors we need only know the required field(s) and their error type. Given those pieces of information, queries of the form below can be used to filter STT error reports to only show the highest priority errors. +The existence of this error, with these fields, is uniquely defined in the appropriate schemas. The same can be said for the remaining critical category three errors. Thus, to define the high priority errors we need only know the required field(s) and their error type. Given those pieces of information, queries of the form below can be used to filter STT error reports to only show the highest priority errors. ```python errors.filter(fields_json__friendly_name__has_keys=[FIELD_NAME, FIELD_NAME, ETC...], From 0528d4d9e8f4da6c3d0c7385debd05b2cf5fca3a Mon Sep 17 00:00:00 2001 From: Eric Lipe <125676261+elipe17@users.noreply.github.com> Date: Thu, 5 Dec 2024 10:37:22 -0500 Subject: [PATCH 68/74] Update docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md Co-authored-by: Alex P. <63075587+ADPennington@users.noreply.github.com> --- .../tech-memos/priotitized-errors/prioritized-errors.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md b/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md index a282971be..1d148ce6c 100644 --- a/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md +++ b/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md @@ -89,7 +89,7 @@ errors.filter(fields_json__friendly_name__has_keys=[FIELD_NAME, FIELD_NAME, ETC. error_type=ParserErrorCategoryChoices.VALUE_CONSISTENCY) ``` -By unioning the category two queries from above with the remainder of the category three queries, a queryset containing only the highest priority errors can be generated and subsequently passed to `get_xls_serialized_file` generate and return the prioritized error report to the requesting STT. +By unioning the category two queries from above with the remainder of the category three queries, a queryset containing only the critical errors can be generated and subsequently passed to `get_xls_serialized_file` generate and return the prioritized error report to the requesting STT. ## Affected Systems - TDP backend From 1ea6dd70d50ed2152892f0ec740b9ea1f49fc03b Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Thu, 5 Dec 2024 10:39:23 -0500 Subject: [PATCH 69/74] - updated based on suggestion --- .../tech-memos/priotitized-errors/prioritized-errors.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md b/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md index 1d148ce6c..931bceb47 100644 --- a/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md +++ b/docs/Technical-Documentation/tech-memos/priotitized-errors/prioritized-errors.md @@ -96,6 +96,5 @@ By unioning the category two queries from above with the remainder of the catego - TDP frontend: latency time incurred while generating report ## Use and Test cases to consider -- Admin can get both prioritized and un-prioritized report -- STT only recieves prioritized report +- Admin and STT receive the same report - Existing tests leveraging ParserError querysets are updated and re-validated for correctness From 581f050dae89a9854030603a7acf615094e1b6dd Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Thu, 5 Dec 2024 11:09:26 -0500 Subject: [PATCH 70/74] - Comment out sources,list --- tdrs-backend/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tdrs-backend/Dockerfile b/tdrs-backend/Dockerfile index 34ef5dd9b..6b908eee6 100644 --- a/tdrs-backend/Dockerfile +++ b/tdrs-backend/Dockerfile @@ -9,7 +9,7 @@ ENV DJANGO_SETTINGS_MODULE=tdpservice.settings.local ENV DJANGO_CONFIGURATION=Local # Allows docker to cache installed dependencies between builds COPY Pipfile Pipfile.lock /tdpapp/ -COPY sources.list /etc/apt/sources.list +# COPY sources.list /etc/apt/sources.list WORKDIR /tdpapp/ # Download latest listing of available packages: RUN apt-get -y update From cb403d4b74ed080ab86b8182830f7e08f38815d5 Mon Sep 17 00:00:00 2001 From: Andrew <84722778+andrew-jameson@users.noreply.github.com> Date: Fri, 6 Dec 2024 09:32:04 -0500 Subject: [PATCH 71/74] Feat/1786 precommit hooks (#3273) * testing precommit withnew flag * testing precommit withnew flag * gitcfg task and precommit updates * secrets-checking scripts changes * lint failure * finding breaking lint issues for backend * finding breaking lint issues for backend * resolving frontend linting * many config changes * trying with alertmanager w/o pass * set pre-push +x * explicit install to maybe resolve Jan's issue * updating install logic to also work on circleci * cleanup for testing * force install, removing caught key in AM.yml * For Jan, switching how to invoke docker compose * Updating git-secrets for reusability * another 'docker compose' mixup * removing commented out code * reverting trufflehog code * accounting for deleted files --- .gitconfig | 15 ++++--- .githooks/pre-commit | 4 ++ .githooks/pre-push | 14 ++++++ README.md | 2 +- Taskfile.yml | 83 +++++++++++++++++++----------------- scripts/git-secrets-check.sh | 55 ++++++++++++++++++------ 6 files changed, 115 insertions(+), 58 deletions(-) create mode 100755 .githooks/pre-commit create mode 100755 .githooks/pre-push diff --git a/.gitconfig b/.gitconfig index f70bcd581..b3cc6696c 100644 --- a/.gitconfig +++ b/.gitconfig @@ -1,17 +1,20 @@ [secrets] providers = git secrets --aws-provider - patterns = (A3T[A-Z0-9]|AKIA|AGPA|AIDA|AROA|AIPA|ANPA|ANVA|ASIA)[A-Z0-9]{16} - patterns = (\"|')?(AWS|aws|Aws)?_?(SECRET|secret|Secret)?_?(ACCESS|access|Access)?_?(KEY|key|Key)(\"|')?\\s*(:|=>|=)\\s*(\"|')?[A-Za-z0-9/\\+=]{40}(\"|')? - patterns = (\"|')?(AWS|aws|Aws)?_?(ACCOUNT|account|Account)_?(ID|id|Id)?(\"|')?\\s*(:|=>|=)\\s*(\"|')?[0-9]{4}\\-?[0-9]{4}\\-?[0-9]{4}(\"|')? - patterns = .+_KEY=.+ allowed = [A-Z]+_KEY=..echo \".{S3_CREDENTIALS}\" [|] jq -r .+ allowed = ./tdrs-backend/.env.example:.* allowed = ./tdrs-backend/docker-compose.yml:57:.* - allowed = ./tdrs-backend/manifest.proxy.yml:* + + allowed = ./tdrs-frontend/node_modules* allowed = regexes.json:.* allowed = ./scripts/copy-login-gov-keypair.sh:14:JWT_KEY=.* allowed = scripts/deploy-backend.sh:.+:DJANGO_SECRET_KEY=..python -c .from secrets import token_urlsafe. print.token_urlsafe..* allowed = .git/config:.* allowed = .gitconfig:.* - allowed = .*DJANGO_SECRET_KEY=.* + allowed = .*DJANGO_SECRET_KEY=.* #this is auto-generated in deployed environments + allowed = ./tdrs-backend/manifest.proxy.yml:* allowed = ./tdrs-backend/plg/loki/manifest.yml:* + patterns = (A3T[A-Z0-9]|AKIA|AGPA|AIDA|AROA|AIPA|ANPA|ANVA|ASIA)[A-Z0-9]{16} + patterns = (\"|')?(AWS|aws|Aws)?_?(SECRET|secret|Secret)?_?(ACCESS|access|Access)?_?(KEY|key|Key)(\"|')?\\s*(:|=>|=)\\s*(\"|')?[A-Za-z0-9/\\+=]{40}(\"|')? + patterns = (\"|')?(AWS|aws|Aws)?_?(ACCOUNT|account|Account)_?(ID|id|Id)?(\"|')?\\s*(:|=>|=)\\s*(\"|')?[0-9]{4}\\-?[0-9]{4}\\-?[0-9]{4}(\"|')? + patterns = .+_KEY=.+ + patterns = .+smtp_auth_password: .[^{]+ diff --git a/.githooks/pre-commit b/.githooks/pre-commit new file mode 100755 index 000000000..7da1e7bb0 --- /dev/null +++ b/.githooks/pre-commit @@ -0,0 +1,4 @@ +#!/bin/bash +set -e + +zsh ./scripts/git-secrets-check.sh local diff --git a/.githooks/pre-push b/.githooks/pre-push new file mode 100755 index 000000000..51e4e28ff --- /dev/null +++ b/.githooks/pre-push @@ -0,0 +1,14 @@ +#!/bin/bash +set -e + +task frontend-lint 2>/dev/null +if [ $? != "0" ]; then + echo "Frontend lint failed" + exit 1 +fi + +task backend-lint 2>/dev/null +if [ $? != "0" ]; then + echo "Backend lint failed" + exit 1 +fi \ No newline at end of file diff --git a/README.md b/README.md index c7ed080a9..ce86a895b 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# Temporary Assistance for Needy Families (TANF) Data Portal - TDP + # Temporary Assistance for Needy Families (TANF) Data Portal - TDP Welcome to the project for the New TANF Data Portal, which will replace the legacy TANF Data Reporting System! diff --git a/Taskfile.yml b/Taskfile.yml index 9f2488455..61bc997a2 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -2,6 +2,11 @@ version: '3' tasks: + gitcfg: + desc: Configure git + cmds: + - git config core.hooksPath .githooks + create-network: desc: Create the external network cmds: @@ -12,10 +17,10 @@ tasks: dir: tdrs-backend cmds: - task: create-network - - docker-compose -f docker-compose.yml up -d --build - - docker-compose -f docker-compose.yml exec web sh -c "python ./manage.py makemigrations" - - docker-compose -f docker-compose.yml exec web sh -c "python ./manage.py migrate" - - docker-compose -f docker-compose.yml down + - docker compose -f docker-compose.yml up -d --build + - docker compose -f docker-compose.yml exec web sh -c "python ./manage.py makemigrations" + - docker compose -f docker-compose.yml exec web sh -c "python ./manage.py migrate" + - docker compose -f docker-compose.yml down - task: sentry-down clone-sentry-repo: @@ -43,7 +48,7 @@ tasks: - docker cp .env sentry:/self-hosted/.env - docker exec sentry bash -c "cd self-hosted && ./install.sh --skip-user-creation --no-report-self-hosted-issues" # create a new user - - docker exec sentry bash -c "cd self-hosted && docker-compose run --rm web createuser --email admin@tanf.com --password admin --superuser" + - docker exec sentry bash -c "cd self-hosted && docker compose run --rm web createuser --email admin@tanf.com --password admin --superuser" # copy backup.json file to sentry - docker cp backup.json sentry:/self-hosted/sentry/backup.json # restore backup @@ -58,56 +63,56 @@ tasks: desc: Start sentry service dir: sentry cmds: - - docker exec sentry bash -c "cd self-hosted && docker-compose up -d" + - docker exec sentry bash -c "cd self-hosted && docker compose up -d" sentry-down: desc: Stop sentry service dir: sentry cmds: - - docker exec sentry bash -c "cd self-hosted && docker-compose down" + - docker exec sentry bash -c "cd self-hosted && docker compose down" drop-db: desc: Drop the backend database dir: tdrs-backend cmds: - - docker-compose -f docker-compose.yml down + - docker compose -f docker-compose.yml down - docker volume rm tdrs-backend_postgres_data backend-up: desc: Start backend web server dir: tdrs-backend cmds: - - docker-compose -f docker-compose.yml up -d + - docker compose -f docker-compose.yml up -d backend-down: desc: Stop backend web server dir: tdrs-backend cmds: - - docker-compose -f docker-compose.yml down + - docker compose -f docker-compose.yml down backend-logs: desc: Show and follow backend web server logs dir: tdrs-backend cmds: - - docker-compose -f docker-compose.yml logs -f + - docker compose -f docker-compose.yml logs -f backend-restart: desc: Restart backend web server dir: tdrs-backend cmds: - - docker-compose -f docker-compose.yml restart + - docker compose -f docker-compose.yml restart backend-bash: desc: Open a shell in the backend container dir: tdrs-backend cmds: - - docker-compose -f docker-compose.yml exec web sh + - docker compose -f docker-compose.yml exec web sh backend-shell: desc: Open a Django shell in the backend container dir: tdrs-backend cmds: - - docker-compose -f docker-compose.yml exec web sh -c "python ./manage.py shell" + - docker compose -f docker-compose.yml exec web sh -c "python ./manage.py shell" backend-exec: desc: Execute a command in the backend container @@ -115,7 +120,7 @@ tasks: vars: CMD: '{{.CMD}}' cmds: - - docker-compose -f docker-compose.yml exec web sh -c "python manage.py {{.CMD}}" + - docker compose -f docker-compose.yml exec web sh -c "python manage.py {{.CMD}}" backend-exec-seed-db: desc: Execute seed_db command in the backend container @@ -123,8 +128,8 @@ tasks: vars: CMD: '{{.CMD}}' cmds: - - docker-compose -f docker-compose.yml up -d - - docker-compose -f docker-compose.yml exec web sh -c "python manage.py populate_stts; python ./manage.py seed_db" + - docker compose -f docker-compose.yml up -d + - docker compose -f docker-compose.yml exec web sh -c "python manage.py populate_stts; python ./manage.py seed_db" backend-pytest: desc: 'Run pytest in the backend container E.g: task backend-pytest PYTEST_ARGS="tdpservice/test/ -s -vv"' @@ -133,20 +138,20 @@ tasks: PYTEST_ARGS: '{{.PYTEST_ARGS | default "."}}' cmds: - task backend-up - - docker-compose -f docker-compose.yml exec web sh -c "pytest {{.PYTEST_ARGS}}" + - docker compose -f docker-compose.yml exec web sh -c "pytest {{.PYTEST_ARGS}}" backend-remove-volumes: desc: Remove the backend volumes dir: tdrs-backend cmds: - - docker-compose -f docker-compose.yml down -v + - docker compose -f docker-compose.yml down -v backend-lint: desc: Run flake8 in the backend container dir: tdrs-backend cmds: - task backend-up - - docker-compose -f docker-compose.yml exec web sh -c "flake8 . && if [ $? -eq 0 ]; then echo 'Flake8 linter found no issues'; fi" + - docker compose -f docker-compose.yml exec -T web sh -c "flake8 . && if [ $? -eq 0 ]; then echo 'Flake8 linter found no issues'; fi" backend-pip-lock: #TODO: Add a task to lock the pip dependencies @@ -154,16 +159,16 @@ tasks: dir: tdrs-backend cmds: - task: backend-up - - docker-compose -f docker-compose.yml exec web sh -c "pipenv lock" + - docker compose -f docker-compose.yml exec web sh -c "pipenv lock" psql: desc: Open a psql shell in the backend container dir: tdrs-backend cmds: - task create-network || true - - docker-compose -f docker-compose.yml up -d postgres + - docker compose -f docker-compose.yml up -d postgres - sleep 5 - - docker-compose -f docker-compose.yml exec postgres sh -c "psql -U tdpuser -d tdrs_test" + - docker compose -f docker-compose.yml exec postgres sh -c "psql -U tdpuser -d tdrs_test" clean: desc: Remove all containers, networks, and volumes @@ -177,25 +182,25 @@ tasks: desc: Start clamav service dir: tdrs-backend cmds: - - docker-compose -f docker-compose.yml up -d clamav-rest + - docker compose -f docker-compose.yml up -d clamav-rest frontend-up: desc: Start frontend web server dir: tdrs-frontend cmds: - - docker-compose -f docker-compose.yml up -d + - docker compose -f docker-compose.yml up -d frontend-down: desc: Stop frontend web server dir: tdrs-frontend cmds: - - docker-compose -f docker-compose.yml down + - docker compose -f docker-compose.yml down frontend-restart: desc: Restart frontend web server dir: tdrs-frontend cmds: - - docker-compose -f docker-compose.yml restart + - docker compose -f docker-compose.yml restart frontend-av: desc: Start frontend with optional clamav service @@ -210,43 +215,43 @@ tasks: desc: Initialize the frontend project dir: tdrs-frontend cmds: - - docker-compose -f docker-compose.yml up -d --build - - docker-compose -f docker-compose.yml exec tdp-frontend sh -c "apk add nodejs npm" - - docker-compose -f docker-compose.yml exec tdp-frontend sh -c "npm install" - - docker-compose -f docker-compose.yml down + - docker compose -f docker-compose.yml up -d --build + - docker compose -f docker-compose.yml exec tdp-frontend sh -c "apk add nodejs npm" + - docker compose -f docker-compose.yml exec tdp-frontend sh -c "npm install" + - docker compose -f docker-compose.yml down frontend-test: desc: Run frontend tests dir: tdrs-frontend cmds: - - docker-compose -f docker-compose.local.yml up tdp-frontend-test -d - - docker-compose -f docker-compose.local.yml exec tdp-frontend-test sh -c "npm run test" + - docker compose -f docker-compose.local.yml up tdp-frontend-test -d + - docker compose -f docker-compose.local.yml exec tdp-frontend-test sh -c "npm run test" frontend-test-cov: desc: Run frontend tests with coverage dir: tdrs-frontend cmds: - - docker-compose -f docker-compose.local.yml up tdp-frontend-test -d - - docker-compose -f docker-compose.local.yml exec tdp-frontend-test sh -c "npm run test:cov" + - docker compose -f docker-compose.local.yml up tdp-frontend-test -d + - docker compose -f docker-compose.local.yml exec tdp-frontend-test sh -c "npm run test:cov" frontend-lint: desc: Run eslint in the frontend container dir: tdrs-frontend cmds: - - docker-compose -f docker-compose.local.yml up -d tdp-frontend-test --quiet-pull - - docker-compose -f docker-compose.yml exec tdp-frontend-test sh -c "npm run lint" + - docker compose -f docker-compose.local.yml up -d tdp-frontend-test --quiet-pull + - docker compose -f docker-compose.yml exec -T tdp-frontend-test sh -c "npm run lint" frontend-logs: desc: Show and follow frontend web server logs dir: tdrs-frontend cmds: - - docker-compose -f docker-compose.yml logs -f + - docker compose -f docker-compose.yml logs -f frontend-bash: desc: Open a shell in the frontend container dir: tdrs-frontend cmds: - - docker-compose -f docker-compose.yml exec tdp-frontend bash + - docker compose -f docker-compose.yml exec tdp-frontend bash up: desc: Start both frontend and backend web servers diff --git a/scripts/git-secrets-check.sh b/scripts/git-secrets-check.sh index f371f303e..6f6c85f8b 100755 --- a/scripts/git-secrets-check.sh +++ b/scripts/git-secrets-check.sh @@ -1,29 +1,57 @@ #!/bin/bash set -e +islocal=$1 -if [ -d /tmp/git-secrets ]; then +if [[ $(uname -s) == "Darwin" ]]; then # Mac OSX check + gs_path="/usr/local/bin" +else # Linux, we're likely running in CircleCI + gs_path="/usr/sbin" +fi + +if [ -f "$gs_path/git-secrets" ]; then echo The command git-secrets is available else echo The command git-secrets is not available, cloning... git clone git@github.com:awslabs/git-secrets.git /tmp/git-secrets/ if [ -f /tmp/git-secrets/git-secrets ]; then - echo "Moving git secrets into PATH" - sudo cp /tmp/git-secrets/git-secrets /usr/sbin/ + + echo "Moving git secrets into PATH" + sudo cp /tmp/git-secrets/git-secrets $gs_path/ + $gs_path/git-secrets --install -f + rm -rf /tmp/git-secrets #cleanup of clone dir else - echo "Git clone failed for git-secrets" + echo "Git clone failed for git-secrets" fi fi # ensure we have correct configs in place -[ -f ../.gitconfig ] -cat .gitconfig >> .git/config -echo "Git-Secrets Config loaded:" -grep -A10 secrets .git/config -# grep will return non-zero code if nothing found, failing the build +if [ -f .gitconfig ]; then + cat .gitconfig >> .git/config + echo "Git-Secrets Config loaded:" + grep -A10 secrets .git/config + # grep will return non-zero code if nothing found, failing the build +fi -echo "git-secrets-check.sh: Scanning repo ..." -git secrets --scan -r ../ -retVal=$? +if [ $islocal ]; then + echo "git-secrets-check.sh: Scanning files staged for commit ..." + setopt shwordsplit + staged_files=$(git diff --cached --name-status | grep -vE "D|^R[0-9]+"| cut -f2 | xargs) + + for filename in $staged_files; do + echo "git-secrets-check.sh: Scanning $filename ..." + git secrets --scan $filename + retVal=$? + if [[ $retVal -ne 0 ]]; then + echo "git-secrets found issues, prevented commit." + return 1 + fi + done + +else + echo "git-secrets-check.sh: Scanning repo ..." + git secrets --scan -r ../ + retVal=$? +fi # if there are issues, they will be listed then script will abort here if [[ $retVal -eq 0 ]]; then @@ -33,3 +61,6 @@ else return 1 fi +#cleanup for testing +rm -rf /tmp/git-secrets +sudo rm -f $gs_path/git-secrets \ No newline at end of file From 5a09073553f3f096c1683c0b1cccf4c630ee0304 Mon Sep 17 00:00:00 2001 From: Andrew <84722778+andrew-jameson@users.noreply.github.com> Date: Fri, 6 Dec 2024 13:41:06 -0500 Subject: [PATCH 72/74] redundant removal commands from debugging (#3344) --- scripts/git-secrets-check.sh | 4 ---- 1 file changed, 4 deletions(-) diff --git a/scripts/git-secrets-check.sh b/scripts/git-secrets-check.sh index 6f6c85f8b..dcfcd7821 100755 --- a/scripts/git-secrets-check.sh +++ b/scripts/git-secrets-check.sh @@ -60,7 +60,3 @@ else echo "git-secrets-check.sh: Issues found with return code $retVal, please remediate." return 1 fi - -#cleanup for testing -rm -rf /tmp/git-secrets -sudo rm -f $gs_path/git-secrets \ No newline at end of file From cdaa7fcce1fc412ef5d56df87e1faae7556d4db4 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Fri, 6 Dec 2024 14:04:20 -0500 Subject: [PATCH 73/74] - remove validator --- tdrs-backend/tdpservice/parsers/schema_defs/tanf/t1.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tdrs-backend/tdpservice/parsers/schema_defs/tanf/t1.py b/tdrs-backend/tdpservice/parsers/schema_defs/tanf/t1.py index 3bd642c03..9dc92acd1 100644 --- a/tdrs-backend/tdpservice/parsers/schema_defs/tanf/t1.py +++ b/tdrs-backend/tdpservice/parsers/schema_defs/tanf/t1.py @@ -66,12 +66,6 @@ result_field_name="WORK_REQ_SANCTION", result_function=category3.isOneOf((1, 2)), ), - category3.ifThenAlso( - condition_field_name="SANC_REDUCTION_AMT", - condition_function=category3.isGreaterThan(0, inclusive=True), - result_field_name="FAMILY_SANC_ADULT", - result_function=category3.isOneOf((1, 2)), - ), category3.ifThenAlso( condition_field_name="SANC_REDUCTION_AMT", condition_function=category3.isGreaterThan(0), From c7778e5d6c610686b11e88dd0861b0a0e371e092 Mon Sep 17 00:00:00 2001 From: Eric Lipe Date: Fri, 6 Dec 2024 14:25:29 -0500 Subject: [PATCH 74/74] - Correct num errors --- tdrs-backend/tdpservice/parsers/test/test_parse.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tdrs-backend/tdpservice/parsers/test/test_parse.py b/tdrs-backend/tdpservice/parsers/test/test_parse.py index cc4f758a8..d01a44030 100644 --- a/tdrs-backend/tdpservice/parsers/test/test_parse.py +++ b/tdrs-backend/tdpservice/parsers/test/test_parse.py @@ -950,7 +950,7 @@ def test_parse_tanf_section1_blanks_file(tanf_section1_file_with_blanks, dfs): parser_errors = ParserError.objects.filter(file=tanf_section1_file_with_blanks) - assert parser_errors.count() == 23 + assert parser_errors.count() == 22 # Should only be cat3 validator errors for error in parser_errors: