[BP] Add SSN prefix search to client profile lookup#1996
Open
jaeeungracelee wants to merge 9 commits intomainfrom
Open
[BP] Add SSN prefix search to client profile lookup#1996jaeeungracelee wants to merge 9 commits intomainfrom
jaeeungracelee wants to merge 9 commits intomainfrom
Conversation
jaeeungracelee
had a problem deploying
to
preview
GitHub Actions
Failure
Reviewer's GuideAdds an indexed SSN field to ClientProfile and updates the client profile search logic to use numeric input for SSN/California ID prefix searches while keeping existing name-based searches and permissions intact, covered by new query tests. Class diagram for updated ClientProfile modelclassDiagram
class ClientProfile {
CharField nickname
ImageField profile_photo
TextChoicesField race
CharField ssn
TextChoicesField veteran_status
}
Flow diagram for updated ClientProfileFilter.search logicflowchart TD
A["Start search(value, queryset)"] --> B["Split value into search_terms"]
B --> C{search_terms is empty?}
C -->|Yes| D["Return queryset, empty Q"]
C -->|No| E["Check all terms: term.isdigit()"]
E --> F{is_numeric_input?}
F -->|Yes| G["Set searchable_fields = [california_id, ssn]"]
F -->|No| H["Set searchable_fields = [california_id, first_name, last_name, middle_name, nickname]"]
G --> I["Build direct_queries using searchable_fields"]
H --> I["Build direct_queries using searchable_fields"]
I --> J["Combine queries and filter queryset"]
J --> K["Return filtered queryset, built Q"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
jaeeungracelee
had a problem deploying
to
preview
GitHub Actions
Failure
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The numeric search path currently relies on
str.isdigit(), so inputs with common SSN formatting like123-45-6789or spaces will fall back to name search; consider normalizing the search string (e.g., stripping non-digits) before deciding whether to treat it as numeric and before queryingssn. - The new
ssnfield is a plain indexedCharField; if SSNs are considered sensitive in this context, consider adding at-rest protection (e.g., encryption field type or hashing with a separate lookup key) or at least server-side validation to constrain the allowed format and length.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The numeric search path currently relies on `str.isdigit()`, so inputs with common SSN formatting like `123-45-6789` or spaces will fall back to name search; consider normalizing the search string (e.g., stripping non-digits) before deciding whether to treat it as numeric and before querying `ssn`.
- The new `ssn` field is a plain indexed `CharField`; if SSNs are considered sensitive in this context, consider adding at-rest protection (e.g., encryption field type or hashing with a separate lookup key) or at least server-side validation to constrain the allowed format and length.
## Individual Comments
### Comment 1
<location path="apps/betterangels-backend/clients/models.py" line_range="151" />
<code_context>
nickname = models.CharField(max_length=50, blank=True, null=True)
profile_photo = models.ImageField(upload_to=get_client_profile_photo_file_path, blank=True, null=True)
race = TextChoicesField(choices_enum=RaceEnum, blank=True, null=True)
+ ssn = models.CharField(max_length=9, blank=True, null=True, db_index=True)
veteran_status = TextChoicesField(choices_enum=VeteranStatusEnum, blank=True, null=True)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** SSN is stored as a free-form CharField with only a max_length constraint, which could allow invalid or malformed values.
`CharField(max_length=9)` alone will accept any <=9-character string (including non-digits, too-short values, or obviously invalid patterns). If other code treats this as a real SSN (for matching/searching), that can introduce data-quality bugs. Please add a validator to enforce 9 numeric digits and, if relevant, reject clearly invalid/test values so the DB constraint aligns with the intended semantics.
Suggested implementation:
```python
race = TextChoicesField(choices_enum=RaceEnum, blank=True, null=True)
ssn = models.CharField(
max_length=9,
blank=True,
null=True,
db_index=True,
validators=[
RegexValidator(
regex=r'^\d{9}$',
message=_('SSN must be exactly 9 numeric digits.'),
code='invalid_ssn_format',
),
reject_obviously_invalid_ssn,
],
)
veteran_status = TextChoicesField(choices_enum=VeteranStatusEnum, blank=True, null=True)
```
To complete this change, you also need to:
1. Add the necessary imports near the top of `apps/betterangels-backend/clients/models.py`:
- `from django.core.exceptions import ValidationError`
- `from django.core.validators import RegexValidator`
- `from django.utils.translation import gettext_lazy as _`
2. Define the `reject_obviously_invalid_ssn` validator function in the same file (at module level), for example:
```python
def reject_obviously_invalid_ssn(value: str) -> None:
if not value:
return
# Normalize just in case future changes allow formatting characters
normalized = ''.join(ch for ch in value if ch.isdigit())
if normalized in {
'000000000',
'123456789',
'111111111',
'999999999',
}:
raise ValidationError(
_('This looks like a placeholder/test SSN and is not allowed.'),
code='invalid_ssn_value',
)
```
3. Ensure any migrations are generated and applied so the updated field constraints (validators) are reflected in your schema and admin/forms.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
jaeeungracelee
had a problem deploying
to
preview
GitHub Actions
Failure
jaeeungracelee
had a problem deploying
to
preview
GitHub Actions
Failure
jaeeungracelee
had a problem deploying
to
preview
GitHub Actions
Failure
jaeeungracelee
changed the title
frank731
requested changes
Mar 31, 2026
| nickname = models.CharField(max_length=50, blank=True, null=True) | ||
| profile_photo = models.ImageField(upload_to=get_client_profile_photo_file_path, blank=True, null=True) | ||
| race = TextChoicesField(choices_enum=RaceEnum, blank=True, null=True) | ||
| ssn = models.CharField( |
Contributor
There was a problem hiding this comment.
I think there exists a SSN field in the HMIS instead in field '"ssn1",
"ssn2",
"ssn3",' sorry for not mentioning this in tickets but would like to test that you can search off an existing client that has one of these attached. Can look at HmisClientProfileBaseType
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ssnfield toClientProfilemodelClientProfileFilter.searchto detect numeric input and search by SSN/California ID prefix instead of name fieldsview_clientprofile) are unchangedSummary by Sourcery
Add SSN support to client profile search, enabling numeric queries to match SSN or California ID prefixes while preserving existing name-based search behavior.
New Features:
Tests: