Skip to content

Commit

Permalink
Revert "[Justice Counts] Create child_agency_name_to_agency with `c…
Browse files Browse the repository at this point in the history
…ustom_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
  • Loading branch information
nichelle-hall authored and Helper Bot committed May 25, 2024
1 parent 52f1792 commit c5c8edf
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 105 deletions.
28 changes: 15 additions & 13 deletions recidiviz/justice_counts/spreadsheet.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
14 changes: 14 additions & 0 deletions recidiviz/justice_counts/utils/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
12 changes: 6 additions & 6 deletions recidiviz/tests/justice_counts/bulk_upload/bulk_upload_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand 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()
Expand All @@ -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."""
Expand Down
10 changes: 5 additions & 5 deletions recidiviz/tests/justice_counts/control_panel/server_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions recidiviz/tests/justice_counts/email_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 1 addition & 8 deletions recidiviz/tests/justice_counts/spreadsheet_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
76 changes: 9 additions & 67 deletions recidiviz/tests/justice_counts/spreadsheet_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand All @@ -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)
8 changes: 4 additions & 4 deletions recidiviz/tests/justice_counts/utils/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down

0 comments on commit c5c8edf

Please sign in to comment.