Uh oh!
There was an error while loading. Please reload this page.
fix: confirm link_user contact creation - #88
Merged
Conversation
📝 WalkthroughWalkthroughThe changes extend the CRM cog to support payload override and custom target scope when creating contacts from resumes, add four helper methods for Discord user linkage integration, and enhance resume-based contact creation with Discord linkage support and richer error handling. Changes
Sequence DiagramsequenceDiagram
participant User as Discord User
participant Bot as Discord Bot
participant CRM as CRM Service
participant API as CRM API
participant View as Resume View
User->>Bot: Upload resume
Bot->>CRM: Check if user linked
alt User Linked
CRM->>View: Show confirmation
User->>View: Confirm create
View->>API: Create contact with payload
API-->>View: Contact created
View->>API: Upload resume attachment
API-->>View: Attachment uploaded
else User Not Linked
Bot->>CRM: Build contact payload for link
CRM->>View: Show creation prompt
User->>View: Confirm create
View->>API: Create contact with override payload
API-->>View: Contact created
View->>API: Upload resume attachment
API-->>View: Attachment uploaded
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/discord_bot/src/five08/discord_bot/cogs/crm.py`:
- Around line 874-881: The logger.exception call currently logs the entire
create_payload (via payload=%s) which may contain PII; update the logging in the
failure path that calls logger.exception (the call using self.filename,
self.target_scope, self.inferred_contact_meta, status_code, create_payload) to
avoid emitting full payload contents—instead log a redacted shape or just the
keys and any non-sensitive summary (e.g., list(create_payload.keys()) or a dict
with sensitive fields like "email" and "discord_id" replaced with "<redacted>")
so filename, target_scope, inferred_contact_meta and status_code remain logged
but actual PII from create_payload is not.
- Around line 838-840: The code currently treats falsy values like {} as "no
override"; change the conditional in the block that sets create_payload to
explicitly check create_payload_override is not None so empty dicts are
respected as valid overrides—locate the branch around the symbol
create_payload_override and the assignment to create_payload and replace the
truthy check with an explicit "is not None" check so that create_payload =
dict(self.create_payload_override) runs when an empty dict is provided.
Comment on lines
+838
to
+840
| if self.create_payload_override: | ||
| create_payload = dict(self.create_payload_override) | ||
| else: |
There was a problem hiding this comment.
Use an explicit None check for payload override.
Line 838 currently treats {} as “no override” and silently falls back to rebuilt payload. If override support is explicit, this should branch on is not None.
Suggested fix
- if self.create_payload_override:+ if self.create_payload_override is not None:
create_payload = dict(self.create_payload_override)
else:
create_payload = self.crm_cog._build_resume_create_contact_payload(
file_content=self.file_content
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ifself.create_payload_override: | |
| create_payload=dict(self.create_payload_override) | |
| else: | |
| ifself.create_payload_overrideisnotNone: | |
| create_payload=dict(self.create_payload_override) | |
| else: | |
| create_payload=self.crm_cog._build_resume_create_contact_payload( | |
| file_content=self.file_content | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/discord_bot/src/five08/discord_bot/cogs/crm.py` around lines 838 - 840,
The code currently treats falsy values like {} as "no override"; change the
conditional in the block that sets create_payload to explicitly check
create_payload_override is not None so empty dicts are respected as valid
overrides—locate the branch around the symbol create_payload_override and the
assignment to create_payload and replace the truthy check with an explicit "is
not None" check so that create_payload = dict(self.create_payload_override) runs
when an empty dict is provided.
Comment on lines
+874
to
+881
| logger.exception( | ||
| "Failed to create contact from resume filename=%s target_scope=%s inferred_meta=%s status_code=%s payload=%s", | ||
| self.filename, | ||
| self.target_scope, | ||
| self.inferred_contact_meta, | ||
| status_code, | ||
| create_payload, | ||
| ) |
There was a problem hiding this comment.
Avoid logging full contact payload on create failures.
Line 875 logs payload=%s, which can include email and Discord identifiers. Log keys (or a redacted shape) instead to reduce PII exposure in logs.
Suggested fix
- logger.exception(- "Failed to create contact from resume filename=%s target_scope=%s inferred_meta=%s status_code=%s payload=%s",- self.filename,- self.target_scope,- self.inferred_contact_meta,- status_code,- create_payload,- )+ payload_keys = sorted(create_payload.keys()) if create_payload else None+ logger.exception(+ "Failed to create contact from resume filename=%s target_scope=%s inferred_meta=%s status_code=%s payload_keys=%s",+ self.filename,+ self.target_scope,+ self.inferred_contact_meta,+ status_code,+ payload_keys,+ )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/discord_bot/src/five08/discord_bot/cogs/crm.py` around lines 874 - 881,
The logger.exception call currently logs the entire create_payload (via
payload=%s) which may contain PII; update the logging in the failure path that
calls logger.exception (the call using self.filename, self.target_scope,
self.inferred_contact_meta, status_code, create_payload) to avoid emitting full
payload contents—instead log a redacted shape or just the keys and any
non-sensitive summary (e.g., list(create_payload.keys()) or a dict with
sensitive fields like "email" and "discord_id" replaced with "<redacted>") so
filename, target_scope, inferred_contact_meta and status_code remain logged but
actual PII from create_payload is not.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Description
This updates
/upload-resumeso whenlink_useris provided but not linked to a CRM contact, the bot shows a confirmation button before creating a new contact from resume + Discord details.It also reuses the same create-contact view for this flow, adds payload override support for that view, and adds structured debug/audit metadata (
status_code, payload keys) when contact creation fails.Additionally, resume contact payload generation now maps email to either
emailAddressorc508Emailbased on domain and uses a safer fallback contact name.Related Issue
N/A
How Has This Been Tested?
uv run pytest tests/unit/test_crm.pyuv run ruff check apps/discord_bot/src/five08/discord_bot/cogs/crm.py tests/unit/test_crm.pySummary by CodeRabbit
Release Notes
New Features
Bug Fixes