Uh oh!
There was an error while loading. Please reload this page.
feat: add /view-skills CRM slash command - #56
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. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdded Discord bot CRM module enhancements for skill viewing: new JSON parsing recovery helper, Discord ID extraction from Changes
Sequence DiagramsequenceDiagram
participant User
participant CRMCog as CRM Cog
participant ContactDB as Contact Lookup
participant Discord as Discord API
User->>CRMCog: /view_skills [`@mention` or search_term]
alt By `@mention` or Direct ID
CRMCog->>ContactDB: Lookup by Discord ID
else By Name/Search Term
CRMCog->>ContactDB: Search contacts with fallback suffix
end
ContactDB-->>CRMCog: Contact data (or multiple matches)
alt Multiple Matches Found
CRMCog->>User: Display selection prompt
User->>CRMCog: Select contact
else No Match
CRMCog->>User: Send error embed
end
alt Contact Found
CRMCog->>CRMCog: Extract skills from cSkillAttrs or raw skills
CRMCog->>CRMCog: Build skill embed with strengths
CRMCog->>Discord: Send embed with CRM link
CRMCog->>CRMCog: Audit log success
else Missing Linkage
CRMCog->>User: Send linkage error
CRMCog->>CRMCog: Audit log error
end
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/unit/test_crm.py (1)
293-306: Avoid mocking the lookup helper in the multi-match behavior test.This patching pattern bypasses real lookup behavior, so regressions in contact-resolution logic won’t be caught. Prefer mocking
espo_api.requestand driving_search_contacts_for_view_skillsend-to-end for this case.🤖 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 293 - 306, The test currently patches crm_cog._search_contacts_for_view_skills which hides contact-resolution logic; instead remove the patch of _search_contacts_for_view_skills and simulate backend responses by mocking espo_api.request so the real _search_contacts_for_view_skills runs end-to-end, then call crm_cog.view_skills.callback with mock_interaction and "john" and assert the multi-match refine behavior; reference crm_cog, _search_contacts_for_view_skills, espo_api.request, view_skills.callback and mock_interaction when updating the test.
🤖 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 2407-2429: _search_contacts_for_view_skills currently delegates to
_search_contact_for_linking which forces maxSize=1 for queries with spaces,
allowing a single arbitrary contact to be returned for full-name queries; change
the behavior so full-name queries are not limited to a single result.
Specifically, update the call site in _search_contacts_for_view_skills to
request unrestricted (or a larger) result set from _search_contact_for_linking
(or pass an explicit parameter to override the maxSize=1 logic), and/or change
_search_contact_for_linking to stop applying maxSize=1 for queries that contain
spaces so that multiple matches are returned and the caller can prompt for
disambiguation.
---
Nitpick comments:
In `@tests/unit/test_crm.py`:
- Around line 293-306: The test currently patches
crm_cog._search_contacts_for_view_skills which hides contact-resolution logic;
instead remove the patch of _search_contacts_for_view_skills and simulate
backend responses by mocking espo_api.request so the real
_search_contacts_for_view_skills runs end-to-end, then call
crm_cog.view_skills.callback with mock_interaction and "john" and assert the
multi-match refine behavior; reference crm_cog,
_search_contacts_for_view_skills, espo_api.request, view_skills.callback and
mock_interaction when updating the test.
Uh oh!
There was an error while loading. Please reload this page.
76b7f30 to
922529eCompareUh oh!
There was an error while loading. Please reload this page.
Description
Adds a new
/view-skillsCRM slash command for Members that shows either your own skills or a targeted contact's skills via optional search term (@mention, email, username, name, or contact ID).The command prioritizes structured
cSkillAttrsstrengths, falls back toskillsMulti-Enum when needed, and handles malformedcSkillAttrsJSON with best-effort recovery before dropping invalid payloads.This also reuses existing skill-attribute parsing logic in
CRMCogto avoid duplicate parsers and keeps audit logging behavior consistent with other CRM commands.Related Issue
#46
How Has This Been Tested?
uv run pytest tests/unit/test_crm.py -quv run ruff check apps/discord_bot/src/five08/discord_bot/cogs/crm.py tests/unit/test_crm.pySummary by CodeRabbit
New Features
Bug Fixes
Tests