-
Notifications
You must be signed in to change notification settings - Fork 3
fix: improve resume location parsing and editing #180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -895,6 +895,115 @@ async def on_submit(self, interaction: discord.Interaction) -> None: | |||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| class ResumeEditLocationModal(discord.ui.Modal, title="Edit Location"): | ||||||||||||||||||||||||||||||||||||||||
| """Modal for editing proposed location fields before confirmation.""" | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| city_input: discord.ui.TextInput = discord.ui.TextInput( | ||||||||||||||||||||||||||||||||||||||||
| label="City", | ||||||||||||||||||||||||||||||||||||||||
| required=False, | ||||||||||||||||||||||||||||||||||||||||
| max_length=100, | ||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||
| state_input: discord.ui.TextInput = discord.ui.TextInput( | ||||||||||||||||||||||||||||||||||||||||
| label="State / Region", | ||||||||||||||||||||||||||||||||||||||||
| required=False, | ||||||||||||||||||||||||||||||||||||||||
| max_length=100, | ||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||
| country_input: discord.ui.TextInput = discord.ui.TextInput( | ||||||||||||||||||||||||||||||||||||||||
| label="Country", | ||||||||||||||||||||||||||||||||||||||||
| required=False, | ||||||||||||||||||||||||||||||||||||||||
| max_length=100, | ||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||
| timezone_input: discord.ui.TextInput = discord.ui.TextInput( | ||||||||||||||||||||||||||||||||||||||||
| label="Timezone", | ||||||||||||||||||||||||||||||||||||||||
| required=False, | ||||||||||||||||||||||||||||||||||||||||
| max_length=100, | ||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| def __init__(self, *, confirmation_view: "ResumeUpdateConfirmationView") -> None: | ||||||||||||||||||||||||||||||||||||||||
| super().__init__() | ||||||||||||||||||||||||||||||||||||||||
| self.confirmation_view = confirmation_view | ||||||||||||||||||||||||||||||||||||||||
| self.city_input.default = str( | ||||||||||||||||||||||||||||||||||||||||
| confirmation_view.proposed_updates.get("addressCity") or "" | ||||||||||||||||||||||||||||||||||||||||
| ).strip() | ||||||||||||||||||||||||||||||||||||||||
| self.state_input.default = str( | ||||||||||||||||||||||||||||||||||||||||
| confirmation_view.proposed_updates.get("addressState") or "" | ||||||||||||||||||||||||||||||||||||||||
| ).strip() | ||||||||||||||||||||||||||||||||||||||||
| self.country_input.default = str( | ||||||||||||||||||||||||||||||||||||||||
| confirmation_view.proposed_updates.get("addressCountry") or "" | ||||||||||||||||||||||||||||||||||||||||
| ).strip() | ||||||||||||||||||||||||||||||||||||||||
| self.timezone_input.default = str( | ||||||||||||||||||||||||||||||||||||||||
| confirmation_view.proposed_updates.get("cTimezone") or "" | ||||||||||||||||||||||||||||||||||||||||
| ).strip() | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| async def on_submit(self, interaction: discord.Interaction) -> None: | ||||||||||||||||||||||||||||||||||||||||
| from five08.crm_normalization import ( | ||||||||||||||||||||||||||||||||||||||||
| normalize_city, | ||||||||||||||||||||||||||||||||||||||||
| normalize_country, | ||||||||||||||||||||||||||||||||||||||||
| normalize_state, | ||||||||||||||||||||||||||||||||||||||||
| normalize_timezone, | ||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| raw_city = (self.city_input.value or "").strip() | ||||||||||||||||||||||||||||||||||||||||
| raw_state = (self.state_input.value or "").strip() | ||||||||||||||||||||||||||||||||||||||||
| raw_country = (self.country_input.value or "").strip() | ||||||||||||||||||||||||||||||||||||||||
| raw_timezone = (self.timezone_input.value or "").strip() | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+946
to
+950
|
||||||||||||||||||||||||||||||||||||||||
| raw_city = (self.city_input.value or "").strip() | |
| raw_state = (self.state_input.value or "").strip() | |
| raw_country = (self.country_input.value or "").strip() | |
| raw_timezone = (self.timezone_input.value or "").strip() | |
| def _clean_location_input(value: str | None) -> str: | |
| """Normalize raw user location input before CRM normalization. | |
| Treats case-insensitive 'none'/'null' as empty so those values clear | |
| existing fields instead of being stored literally. | |
| """ | |
| cleaned = (value or "").strip() | |
| if cleaned.lower() in {"none", "null"}: | |
| return "" | |
| return cleaned | |
| raw_city = _clean_location_input(self.city_input.value) | |
| raw_state = _clean_location_input(self.state_input.value) | |
| raw_country = _clean_location_input(self.country_input.value) | |
| raw_timezone = _clean_location_input(self.timezone_input.value) |
Copilot
AI
Mar 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ResumeUpdateConfirmationView._has_location_updates is introduced but appears unused in the module. If it’s not needed, consider removing it to keep the view API minimal; otherwise, wire it into the call sites that need it (e.g., deciding whether to render grouped location output).
| @classmethod | |
| def _has_location_updates(cls, values: dict[str, Any]) -> bool: | |
| return any(values.get(field) for field in cls._LOCATION_FIELDS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The location modal allows up to 100 characters per field, but
normalize_city/normalize_statecurrently reject values over 40 chars (via_is_plausible_location_phrase(..., max_length=40)). This mismatch will cause the UI to accept input that the backend will always mark invalid. Consider aligningmax_lengthon theTextInputs with the normalization constraints (or loosening the normalization max_length if 100 is intentional).