[DEV-14527] - Fix Account Download Agency Filter Validation

[DavidMikolaKC]

Description:

The validation for the /api/v2/download/accounts endpoint has been updated to prevent users from submitting toptier_agency_id's that don't exist. Additionally, users are now able to submit agency abbreviations within the agency field. Invalid agency abbreviations are similarly prevented from submitting abbreviations that do not exist.

Requirements for PR Merge:

1. Unit & integration tests updated
2. API documentation updated (examples listed below)
   1. API Contracts
   2. Comments
3. Data validation completed (examples listed below)
   1. Does this work well with the current frontend? Or is the frontend aware of a needed change?
   2. Is performance impacted in the changes (e.g., API, pipeline, downloads, etc.)?
   3. Is the expected data returned with the expected format?
4. Appropriate Operations ticket(s) created
5. Jira Ticket(s)
   1. DEV-14527

Explain N/A in above checklist:

[zachflanders-frb]

Generally, we try to avoid branching if/else statements because they can be a little hard to read. I included a few ideas for patterns that avoid the branching if/else pattern. Feel free to include these, but I am not blocking over these since they are more style/readability comments and this passes the style checks.

[zachflanders-frb]

You could isolate the items that are changing to make this a little more DRY.

Suggested change

	if filters.get("agency") and filters["agency"] != "all":
	if filters["agency"].isdigit():
	if not ToptierAgency.objects.filter(toptier_agency_id=filters["agency"]).exists():
	raise NotFound(f"No agency was found with id {filters['agency']}")
	else:
	query_filters[f"{tas_id}__funding_toptier_agency_id"] = filters["agency"]
	else:
	try:
	ToptierAgency.objects.get(abbreviation=filters["agency"])
	query_filters[f"{tas_id}__funding_toptier_agency__abbreviation"] = filters["agency"]
	except ToptierAgency.DoesNotExist as e:
	raise NotFound(f"No agency was found with abbreviation {filters['agency']}") from e
	filter_param, query_filter_suffix = (
	("top_tier_agency_id", "id")
	if filters["agency"].isdigit()
	else ("abbreviation", "abbreviation")
	)
	if ToptierAgency.objects.filter(**{filter_param: filters["agency"]}).exists():
	query_filters[f"{tas_id}__funding_toptier_agency_{query_filter_suffix}"] = filters["agency"]
	else:
	raise NotFound(f"No agency was found with {query_filter_suffix} {filters['agency']}")

[zachflanders-frb]

Here you could name the conditionals and use them together instead of the branching if/elses

Suggested change

	if agency_filter and agency_filter.lower() != "all":
	if agency_filter.isdigit():
	if not ToptierAgency.objects.filter(toptier_agency_id=agency_filter).exists():
	raise NotFound(f"No agency was found with id {agency_filter}")
	else:
	try:
	ToptierAgency.objects.get(abbreviation=agency_filter)
	except ToptierAgency.DoesNotExist as e:
	raise NotFound(f"No agency was found with abbreviation {agency_filter}") from e
	has_agency_filter = agency_filter and agency_filter.lower() != "all"
	is_valid_id = (
	agency_filter.isdigit()
	and ToptierAgency.objects.filter(toptier_agency_id=agency_filter).exists()
	)
	is_valid_abbr = ToptierAgency.objects.filter(abbreviation=agency_filter).exists()
	if has_agency_filter and not (is_valid_id or is_valid_abbr):
	raise NotFound(
	f"No agency was found with {'id' if agency_filter.isdigit() else 'abbreviation'} {agency_filter}"
	)

[zachflanders-frb]

Here's another idea to avoid the branching if/else statements, you could try a match/case statement:

Suggested change

	field = ""
	result = "All"
	if request_agency and request_agency != "all":
	toptier_agency_filter = ToptierAgency.objects.filter(toptier_agency_id=request_agency).first()
	if type(request_agency) is int or request_agency.isdigit():
	toptier_agency_filter = ToptierAgency.objects.filter(toptier_agency_id=request_agency).first()
	field = "toptier_code"
	else:
	try:
	toptier_agency_filter = ToptierAgency.objects.get(abbreviation=request_agency)
	field = "abbreviation"
	except ToptierAgency.DoesNotExist:
	toptier_agency_filter = None
	if toptier_agency_filter:
	result = toptier_agency_filter.toptier_code
	result = getattr(toptier_agency_filter, field)
	return result
	match request_agency:
	case None | "" | "all":
	result = "All"
	case int() | str() if isinstance(request_agency, int) or request_agency.isdigit():
	toptier_agency = ToptierAgency.objects.filter(toptier_agency_id=request_agency).first()
	result = toptier_agency.toptier_code if top_tier_agency else "All"
	case str():
	toptier_agency = ToptierAgency.objects.get(abbreviation=request_agency)
	result = toptier_agency.abbreviation if top_tier_agency else "All"
	case _:
	result = "All"
	return result