From c5c8edf9b4898147aaaaee4bf637b90379f25bae Mon Sep 17 00:00:00 2001 From: Nichelle Date: Fri, 10 May 2024 15:48:51 -0600 Subject: [PATCH] Revert "[Justice Counts] Create `child_agency_name_to_agency` with `custom_child_agency_name`" (Recidiviz/recidiviz-data#29657) Reverts Recidiviz/recidiviz-dataRecidiviz/recidiviz-data#29595 I tested this code in Prod and it didn't work as expected! Reverting so that the team doesn't have to worry about it during my week off. GitOrigin-RevId: 63e0e6052046208047b91dd18f19e9f40a8a446d --- recidiviz/justice_counts/spreadsheet.py | 28 +++---- recidiviz/justice_counts/utils/constants.py | 14 ++++ .../bulk_upload/bulk_upload_test.py | 12 +-- .../control_panel/server_test.py | 10 +-- recidiviz/tests/justice_counts/email_test.py | 4 +- .../justice_counts/spreadsheet_helpers.py | 9 +-- .../tests/justice_counts/spreadsheet_test.py | 76 +++---------------- recidiviz/tests/justice_counts/utils/utils.py | 8 +- 8 files changed, 56 insertions(+), 105 deletions(-) diff --git a/recidiviz/justice_counts/spreadsheet.py b/recidiviz/justice_counts/spreadsheet.py index 936ffbdc16..8dab92196b 100644 --- a/recidiviz/justice_counts/spreadsheet.py +++ b/recidiviz/justice_counts/spreadsheet.py @@ -55,6 +55,7 @@ from recidiviz.justice_counts.metrics.metric_interface import MetricInterface from recidiviz.justice_counts.types import BulkUploadFileType, DatapointJson from recidiviz.justice_counts.utils.constants import ( + CHILD_AGENCY_NAME_TO_UPLOAD_NAME, ERRORS_WARNINGS_JSON_BUCKET_PROD, ERRORS_WARNINGS_JSON_BUCKET_STAGING, UploadMethod, @@ -274,19 +275,20 @@ def ingest_spreadsheet( session=session, agency=agency ) - child_agency_name_to_agency = {} - for child_agency in child_agencies: - child_agency_name_to_agency[ - child_agency.name.strip().lower() - ] = child_agency - if child_agency.custom_child_agency_name is not None: - # Add the custom_child_agency_name of the agency as a key in - # child_agency_name_to_agency. That way, Bulk Upload will - # be successful if the user uploads with EITHER the name - # or the original name of the agency - child_agency_name_to_agency[ - child_agency.custom_child_agency_name.strip().lower() - ] = child_agency + if agency.id == 110: + # This is a special case. Ohio Office of Criminal Justice Services (OCJS) + # uploads shorthand names rather than full names of child agencies. This mapping + # should not be used for any other agency + child_agency_name_to_agency = { + CHILD_AGENCY_NAME_TO_UPLOAD_NAME.get( + a.name.strip().lower(), a.name.strip().lower() + ): a + for a in child_agencies + } + else: + child_agency_name_to_agency = { + a.name.strip().lower(): a for a in child_agencies + } uploader = WorkbookUploader( agency=agency, diff --git a/recidiviz/justice_counts/utils/constants.py b/recidiviz/justice_counts/utils/constants.py index 9ab5984fe3..32dd259239 100644 --- a/recidiviz/justice_counts/utils/constants.py +++ b/recidiviz/justice_counts/utils/constants.py @@ -33,6 +33,20 @@ # Unsubscribe group ID number for SendGrid for Justice Counts UNSUBSCRIBE_GROUP_ID = 26272 +# Maps the actual name of the child agency to +# a shorthand used in a the spreadsheet during +# Bulk Upload. We need this because some agencies +# only want to provide shorthands in their spreadsheets. +CHILD_AGENCY_NAME_TO_UPLOAD_NAME = { + "toledo police department": "toledo", + "newark division of police": "newark", + "cleveland police department": "cleveland", + "columbus police department (oh)": "columbus", + "franklin county sheriff's office (oh)": "franklin county sheriff's office", + "amberley village police department": "amberley village", + "hamilton county sheriff’s office": "hamilton county sheriff's office", +} + class DatapointGetRequestEntryPoint(enum.Enum): REPORT_PAGE = "REPORT_PAGE" diff --git a/recidiviz/tests/justice_counts/bulk_upload/bulk_upload_test.py b/recidiviz/tests/justice_counts/bulk_upload/bulk_upload_test.py index ea22cb8e11..bae599aeed 100644 --- a/recidiviz/tests/justice_counts/bulk_upload/bulk_upload_test.py +++ b/recidiviz/tests/justice_counts/bulk_upload/bulk_upload_test.py @@ -82,8 +82,8 @@ def setUp(self) -> None: prosecution_agency = self.test_schema_objects.test_agency_F supervision_agency = self.test_schema_objects.test_agency_E prison_super_agency = self.test_schema_objects.test_prison_super_agency - prison_child_agency_A = self.test_schema_objects.test_prison_child_agency_A - prison_child_agency_B = self.test_schema_objects.test_prison_child_agency_B + prison_affiliate_A = self.test_schema_objects.test_prison_affiliate_A + prison_affiliate_B = self.test_schema_objects.test_prison_affiliate_B with SessionFactory.using_database(self.database_key) as session: session.add_all( @@ -94,8 +94,8 @@ def setUp(self) -> None: supervision_agency, law_enforcement_agency, prison_super_agency, - prison_child_agency_A, - prison_child_agency_B, + prison_affiliate_A, + prison_affiliate_B, ] ) session.commit() @@ -106,8 +106,8 @@ def setUp(self) -> None: self.law_enforcement_agency_id = law_enforcement_agency.id self.supervision_agency_id = supervision_agency.id self.user_account_id = user_account.id - prison_child_agency_A.super_agency_id = self.prison_super_agency_id - prison_child_agency_B.super_agency_id = self.prison_super_agency_id + prison_affiliate_A.super_agency_id = self.prison_super_agency_id + prison_affiliate_B.super_agency_id = self.prison_super_agency_id def test_validation(self) -> None: """Test that errors are thrown on invalid inputs.""" diff --git a/recidiviz/tests/justice_counts/control_panel/server_test.py b/recidiviz/tests/justice_counts/control_panel/server_test.py index 3b3912c7b1..20312bc1d8 100644 --- a/recidiviz/tests/justice_counts/control_panel/server_test.py +++ b/recidiviz/tests/justice_counts/control_panel/server_test.py @@ -228,7 +228,7 @@ def test_get_home_metadata(self) -> None: def test_get_superagency_home_metadata(self) -> None: user = self.test_schema_objects.test_user_A super_agency = self.test_schema_objects.test_prison_super_agency - child_agency = self.test_schema_objects.test_prison_child_agency_A + child_agency = self.test_schema_objects.test_prison_affiliate_A self.session.add_all( [ user, @@ -3185,20 +3185,20 @@ def test_upload_superagency_spreadsheet(self) -> None: self.session.add_all( [ self.test_schema_objects.test_prison_super_agency, - self.test_schema_objects.test_prison_child_agency_A, + self.test_schema_objects.test_prison_affiliate_A, AgencyUserAccountAssociation( user_account=self.test_schema_objects.test_user_A, agency=self.test_schema_objects.test_prison_super_agency, ), AgencyUserAccountAssociation( user_account=self.test_schema_objects.test_user_A, - agency=self.test_schema_objects.test_prison_child_agency_A, + agency=self.test_schema_objects.test_prison_affiliate_A, ), ] ) self.session.commit() super_agency = self.test_schema_objects.test_prison_super_agency - child_agency = self.test_schema_objects.test_prison_child_agency_A + child_agency = self.test_schema_objects.test_prison_affiliate_A child_agency.super_agency_id = super_agency.id # Set superagency to have the following metric config @@ -3330,7 +3330,7 @@ def test_upload_superagency_spreadsheet(self) -> None: def test_get_child_agencies_for_superagency(self) -> None: user = self.test_schema_objects.test_user_A super_agency = self.test_schema_objects.test_prison_super_agency - child_agency = self.test_schema_objects.test_prison_child_agency_A + child_agency = self.test_schema_objects.test_prison_affiliate_A self.session.add_all( [ user, diff --git a/recidiviz/tests/justice_counts/email_test.py b/recidiviz/tests/justice_counts/email_test.py index 177efd6b45..5939335215 100644 --- a/recidiviz/tests/justice_counts/email_test.py +++ b/recidiviz/tests/justice_counts/email_test.py @@ -702,8 +702,8 @@ def test_get_missing_metrics_superagency(self) -> None: frozen_today = datetime.date.today() with SessionFactory.using_database(self.database_key) as session: agency = self.test_schema_objects.test_prison_super_agency - child_agency_A = self.test_schema_objects.test_prison_child_agency_A - child_agency_B = self.test_schema_objects.test_prison_child_agency_B + child_agency_A = self.test_schema_objects.test_prison_affiliate_A + child_agency_B = self.test_schema_objects.test_prison_affiliate_B agency.systems = [ schema.System.SUPERAGENCY.value, diff --git a/recidiviz/tests/justice_counts/spreadsheet_helpers.py b/recidiviz/tests/justice_counts/spreadsheet_helpers.py index 19449fb03b..92a2aa9cb4 100644 --- a/recidiviz/tests/justice_counts/spreadsheet_helpers.py +++ b/recidiviz/tests/justice_counts/spreadsheet_helpers.py @@ -79,14 +79,7 @@ def _get_dimension_columns( ] # [Person, Property, .. Unknown, Other] -> [Person, Person, Property, Property...] if child_agencies is not None: - agency_names = [ - ( - a.name - if a.custom_child_agency_name is None - else a.custom_child_agency_name - ) - for a in child_agencies - ] + agency_names = [a.name for a in child_agencies] month_col = [ mon for mon in month_col for _ in range(len(agency_names)) ] # [January, February] -> [January, January, February, February] diff --git a/recidiviz/tests/justice_counts/spreadsheet_test.py b/recidiviz/tests/justice_counts/spreadsheet_test.py index 9a1226a11b..f41e0d958b 100644 --- a/recidiviz/tests/justice_counts/spreadsheet_test.py +++ b/recidiviz/tests/justice_counts/spreadsheet_test.py @@ -16,7 +16,6 @@ # ============================================================================= """This class implements tests for the Justice Counts SpreadsheetInterface.""" import datetime -import itertools import os import tempfile from pathlib import Path @@ -286,20 +285,20 @@ def test_template_generator(self) -> None: # dissaggregations that have been disabled. with SessionFactory.using_database(self.database_key) as session: prison_super_agency = self.test_schema_objects.test_prison_super_agency - prison_child_agency_A = self.test_schema_objects.test_prison_child_agency_A - prison_child_agency_B = self.test_schema_objects.test_prison_child_agency_B + prison_affiliate_A = self.test_schema_objects.test_prison_affiliate_A + prison_affiliate_B = self.test_schema_objects.test_prison_affiliate_B test_user_A = self.test_schema_objects.test_user_A session.add_all( [ prison_super_agency, - prison_child_agency_A, - prison_child_agency_B, + prison_affiliate_A, + prison_affiliate_B, test_user_A, ] ) session.commit() - prison_child_agency_A.super_agency_id = prison_super_agency.id - prison_child_agency_B.super_agency_id = prison_super_agency.id + prison_affiliate_A.super_agency_id = prison_super_agency.id + prison_affiliate_B.super_agency_id = prison_super_agency.id session.refresh(prison_super_agency) super_agency_datapoints = ( self.test_schema_objects.get_test_agency_datapoints( @@ -366,10 +365,7 @@ def test_template_generator(self) -> None: self.assertTrue("agency" in row) self.assertTrue( row["agency"] - in { - prison_child_agency_A.name, - prison_child_agency_B.name, - } + in {prison_affiliate_A.name, prison_affiliate_B.name} ) # Single-page template @@ -387,59 +383,5 @@ def test_template_generator(self) -> None: metrics_in_sheet = df["metric"].unique() self.assertFalse({"admissions"} == (metrics_in_sheet)) agencies = df["agency"].unique() - self.assertTrue(prison_child_agency_A.name in agencies) - self.assertTrue(prison_child_agency_B.name in agencies) - - def test_custom_child_agency_name(self) -> None: - with SessionFactory.using_database(self.database_key) as session: - user = self.test_schema_objects.test_user_A - agency = self.test_schema_objects.test_prison_super_agency - child_agency = self.test_schema_objects.test_prison_child_agency_A - child_agency.custom_child_agency_name = "foobar" - session.add_all([user, agency, child_agency]) - session.commit() - child_agency.super_agency_id = agency.id - session.refresh(agency) - session.refresh(user) - - spreadsheet = self.test_schema_objects.get_test_spreadsheet( - system=schema.System.PRISONS, - user_id=user.auth0_user_id, - agency_id=agency.id, - ) - session.add(spreadsheet) - # Excel workbook will have an invalid sheet. - file_path = create_excel_file( - system=schema.System.PRISONS, - file_name="test_custom_child_agency_name.xlsx", - child_agencies=[child_agency], - ) - ( - metric_key_to_datapoint_jsons, - _, - _, - _, - _, - _, - ) = SpreadsheetInterface.ingest_spreadsheet( - session=session, - xls=pd.ExcelFile(file_path), - spreadsheet=spreadsheet, - auth0_user_id=user.auth0_user_id, - agency=agency, - metric_key_to_metric_interface={}, - metric_definitions=METRICS_BY_SYSTEM[schema.System.PRISONS.value], - filename=file_path, - upload_method=UploadMethod.BULK_UPLOAD, - upload_filetype=BulkUploadFileType.XLSX, - ) - - spreadsheet = session.query(schema.Spreadsheet).one() - spreadsheet = session.query(schema.Spreadsheet).one() - self.assertEqual(spreadsheet.status, schema.SpreadsheetStatus.INGESTED) - self.assertEqual(spreadsheet.ingested_by, user.auth0_user_id) - # Confirm that datapoints were ingested for the child agency - - self.assertTrue( - len(list(itertools.chain(*metric_key_to_datapoint_jsons.values()))) > 0 - ) + self.assertTrue(prison_affiliate_A.name in agencies) + self.assertTrue(prison_affiliate_B.name in agencies) diff --git a/recidiviz/tests/justice_counts/utils/utils.py b/recidiviz/tests/justice_counts/utils/utils.py index 60dc3a969e..b670c0f61d 100644 --- a/recidiviz/tests/justice_counts/utils/utils.py +++ b/recidiviz/tests/justice_counts/utils/utils.py @@ -190,14 +190,14 @@ def __init__(self) -> None: systems=[schema.System.PRISONS.value, schema.System.SUPERAGENCY.value], is_superagency=True, ) - self.test_prison_child_agency_A = schema.Agency( - name="Child Agency Prison A", + self.test_prison_affiliate_A = schema.Agency( + name="Affiliate Agency Prison A", state_code="US_PA", fips_county_code="us_ca_san_francisco", systems=[schema.System.PRISONS.value], ) - self.test_prison_child_agency_B = schema.Agency( - name="Child Agency Prison B", + self.test_prison_affiliate_B = schema.Agency( + name="Affiliate Agency Prison B", state_code="US_XX", fips_county_code="us_ca_san_francisco", systems=[schema.System.PRISONS.value],