Skip to content

[DEV-15452] Mythos finding - district_original location filter silently overwrites district_current - #4702

Merged
james-at-kc merged 5 commits into
qatfrom
ftr/dev-15452-district_original-overwrites
Jul 28, 2026
Merged

[DEV-15452] Mythos finding - district_original location filter silently overwrites district_current#4702
james-at-kc merged 5 commits into
qatfrom
ftr/dev-15452-district_original-overwrites

Conversation

@james-at-kc

@james-at-kc james-at-kc commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Description:

In location_filter_geocode.py a district_current filter silently overwrites a district_original filter when both present in multi-element filter.

Expected behavior is to combine in a django query using logical OR operator.

Technical Details:

location_filter_geocode.py:47-50 assigns district_qs = Q(...) for district_current then unconditionally reassigns (not |=) for district_original, so when both lists are non-empty the current-CD predicate is dropped.

The only guard, validate_location_keys at :75, rejects both keys within a single list element, but create_nested_object at :126-134 .extend()s per-state across multiple elements, so [{state:VA,district_current:08},{state:VA,district_original:11}] populates both lists for VA without tripping :75.

Upstream TinyShield schemas (common/validator/award_filter.py:141-178, download/v2/request_validations.py:171-223) validate shape/length per element only, with no cross-element district check.

Reachable from the public Postgres download path via download/lookups.py:86 → search.py:25 → search.py:226/241 → geocode_filter_locations.

Acceptance Criteria
The district_current filter is no longer overwritten.

Requirements for PR Merge:

  1. Unit & integration tests updated
  2. N/A - API documentation updated (examples listed below)
    1. API Contracts
    2. API UI
    3. Comments
  3. N/A - 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. N/A - Appropriate Operations ticket(s) created
  5. Jira Ticket(s)
    1. DEV-15452

Explain N/A in above checklist:

2 - NA (API documentation) - there was no change to the API
3 - NA (Data validation) - there was no change to the data
4 - NA (ops tickets) - there is no ops involvement beyond the standard qat merge and deployment

@aguest-kc aguest-kc added the ready for review [PR] ready to be reviewed label Jul 27, 2026

@sethstoudenmier sethstoudenmier left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM. Left one comment that I won't block over.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file seems so share similar logic with the "location_filter_geocode.py" file. Won't block on this, but could some of it be consolidated to avoid repeating?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will evaluate the possibility of consolidation and report back. Thanks!

@james-at-kc
james-at-kc merged commit 9e3668b into qat Jul 28, 2026
35 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review [PR] ready to be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants