fix: support LinkedIn URL contact lookup#214
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe pull request adds LinkedIn profile URL normalization and validation to the Discord bot's CRM cog. A new private helper method validates LinkedIn URLs, normalizes variants (with/without HTTPS scheme, www subdomain, trailing slashes), and returns deduplicated forms for exact matching. The search filter logic is updated to use these variants for precise LinkedIn lookups via equality filters. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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.
Pull request overview
Updates CRM contact lookup behavior so LinkedIn profile URLs are treated as exact cLinkedIn matches (with normalized URL variants), avoiding fallback to name-based searching and improving match reliability across common stored URL formats.
Changes:
- Add LinkedIn profile URL detection + generation of normalized exact-match URL variants for lookup.
- Update
_build_contact_search_filtersto prefercLinkedInequality filters when a LinkedIn profile URL is provided. - Add unit tests covering full and bare LinkedIn profile URL inputs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
apps/discord_bot/src/five08/discord_bot/cogs/crm.py |
Adds LinkedIn URL variant normalization and uses it to build cLinkedIn exact-match filters during lookup. |
tests/unit/test_crm.py |
Adds unit coverage validating LinkedIn URL inputs produce cLinkedIn filters (and avoid name filters). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/unit/test_crm.py (1)
3718-3728: Strengthen bare-URL assertions to fully enforcecLinkedInvariant behavior.At Line 3722 and Line 3727, the test checks one normalized value and no
namefilter, but it can still pass if other non-cLinkedInfilters leak in or if the bare input doesn’t produce thewwwvariant.Suggested test hardening
def test_build_contact_search_filters_bare_linkedin_profile_url(self, crm_cog): """Build shared search filters for bare LinkedIn profile URLs.""" filters = crm_cog._build_contact_search_filters("linkedin.com/in/hshidara") + assert all(filter_["attribute"] == "cLinkedIn" for filter_ in filters) assert { "type": "equals", "attribute": "cLinkedIn", "value": "https://linkedin.com/in/hshidara", } in filters + assert { + "type": "equals", + "attribute": "cLinkedIn", + "value": "https://www.linkedin.com/in/hshidara", + } in filters assert not any(filter_["attribute"] == "name" for filter_ in filters)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/test_crm.py` around lines 3718 - 3728, Update the test_build_contact_search_filters_bare_linkedin_profile_url to call crm_cog._build_contact_search_filters and assert that the returned filters include both normalized cLinkedIn variants ("https://linkedin.com/in/hshidara" and "https://www.linkedin.com/in/hshidara") and that every filter with attribute "cLinkedIn" uses only those normalized values, while also asserting there are no filters whose attribute is not "cLinkedIn" (to prevent leaks) and still asserting no filter has attribute "name".
🤖 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 4736-4745: The variants list creation (variables: variants,
variant, value, path) currently adds only the raw scheme-less input and
scheme-prefixed URLs; add scheme-less cross-variants for both www and non-www
forms so CRM lookups match stored entries without a scheme. Update the loop that
builds variants in crm.py (around the block that iterates over (value.strip(),
f"https://linkedin.com{path}", ...)) to also include f"www.linkedin.com{path}"
and f"linkedin.com{path}" (or their non-www equivalents) as separate scheme-less
entries, ensuring you still deduplicate via the existing if variant and variant
not in variants check. Ensure ordering and dedupe logic remain the same.
---
Nitpick comments:
In `@tests/unit/test_crm.py`:
- Around line 3718-3728: Update the
test_build_contact_search_filters_bare_linkedin_profile_url to call
crm_cog._build_contact_search_filters and assert that the returned filters
include both normalized cLinkedIn variants ("https://linkedin.com/in/hshidara"
and "https://www.linkedin.com/in/hshidara") and that every filter with attribute
"cLinkedIn" uses only those normalized values, while also asserting there are no
filters whose attribute is not "cLinkedIn" (to prevent leaks) and still
asserting no filter has attribute "name".
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7fd9ae6e-8da1-4a90-9874-b2bd121d81ce
📒 Files selected for processing (2)
apps/discord_bot/src/five08/discord_bot/cogs/crm.pytests/unit/test_crm.py
Description
Treat LinkedIn profile URLs in CRM
search_termlookups as exactcLinkedInmatches instead of falling back to name search.Normalize common LinkedIn profile URL variants so stored
wwwand non-wwwforms can both resolve the same contact.Add unit coverage for full and bare LinkedIn profile URL inputs.
Related Issue
N/A
How Has This Been Tested?
uv run pytest tests/unit/test_crm.py -k "build_contact_search_filters or search_contacts_for_lookup_includes_discord_username_filter_when_requested or search_contacts_by_field_includes_requested_field_and_excludes_default"Summary by CodeRabbit