Skip to content

Require id_type for mark-id-verified#76

Merged
michaelmwu merged 5 commits intomainfrom
michaelmwu/mark-id-type
Mar 2, 2026
Merged

Require id_type for mark-id-verified#76
michaelmwu merged 5 commits intomainfrom
michaelmwu/mark-id-type

Conversation

@michaelmwu
Copy link
Copy Markdown
Member

@michaelmwu michaelmwu commented Mar 2, 2026

Description

  • Added required id_type to /mark-id-verified and wired it through the command handler, contact-selection flow, and CRM update payload.
  • The command now writes cVerifiedIdType (id_type) in addition to existing cIdVerifiedAt and cIdVerifiedBy fields.
  • Updated success feedback to include recorded ID type and added required-arg help text examples.
  • Documented the /mark-id-verified command and field mapping in README.md.

Related Issue

None.

How Has This Been Tested?

Local hooks ran via commit (ruff and mypy passed) and no additional manual tests were executed.

Summary by CodeRabbit

  • Documentation

    • Added comprehensive Discord Bot documentation covering all slash commands, role-based permissions, and command parameters
    • Consolidated Discord slash command documentation references
  • New Features

    • Enhanced ID verification workflow to capture and persist ID type information alongside verification records

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 2, 2026

Warning

Rate limit exceeded

@michaelmwu has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 18 minutes and 31 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 5ca9738 and 012ce77.

📒 Files selected for processing (2)
  • DISCORD_BOT.md
  • apps/discord_bot/src/five08/discord_bot/cogs/crm.py
📝 Walkthrough

Walkthrough

This pull request introduces comprehensive Discord bot documentation and extends the CRM cog's ID verification workflow to support an ID type field. The changes add documentation files for bot commands and permissions, while implementing a new id_type parameter that threads through the mark-ID-verified command pipeline, payload structures, and verification embeds.

Changes

Cohort / File(s) Summary
Documentation
DISCORD_BOT.md, DISCORD_SLASH_COMMANDS.md, README.md
New Discord bot documentation detailing bot overview, role-based permissions, and slash command specifications. Added deprecation notice in DISCORD_SLASH_COMMANDS.md directing to DISCORD_BOT.md. Cross-references added to README.md.
CRM ID Verification Flow
apps/discord_bot/src/five08/discord_bot/cogs/crm.py
Extended ID verification workflow with new id_type parameter. Modified mark_id_verified command, _show_mark_id_verified_contact_choices, _mark_id_verified_for_contact, and selection UI classes (MarkIdVerifiedSelectionButton, MarkIdVerifiedSelectionView) to accept and propagate id_type. Added ID_VERIFIED_TYPE_FIELD constant and updated payloads and embeds to include ID type field.

Sequence Diagram(s)

sequenceDiagram
    participant User as Discord User
    participant Cmd as mark_id_verified Command
    participant Show as _show_mark_id_verified_contact_choices
    participant View as Selection View
    participant Verify as _mark_id_verified_for_contact
    participant CRM as CRM Backend

    User->>Cmd: /mark-id-verified search_term verified_by id_type
    Cmd->>Show: Search for contact, pass id_type
    Show->>View: Create selection UI with id_type
    View-->>User: Display contact options
    User->>View: Select contact
    View->>Verify: Call with contact, verified_by, id_type, verified_at
    Verify->>Verify: Build payload with id_type
    Verify->>CRM: Update contact record with cVerifiedIdType
    CRM-->>Verify: Confirmation
    Verify->>View: Create embed with ID type display
    View-->>User: Show verification result
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A hop through the code, a new type takes flight,
ID verification flows with parameter might,
Through commands and selections it threads its way true,
From Discord to CRM, the id_type shines new! 🆔

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and accurately reflects the main change: requiring id_type as a parameter for the mark-id-verified command, which is the core functional change across the codebase.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch michaelmwu/mark-id-type

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
apps/discord_bot/src/five08/discord_bot/cogs/crm.py (1)

2366-2377: Include id_type in audit metadata for completeness.

The success audit event is missing id_type in its metadata, while the command-level audit events include it. For consistency and traceability, consider adding it here as well.

♻️ Suggested improvement
                 self._audit_command(
                     interaction=interaction,
                     action="crm.mark_id_verified",
                     result="success",
                     metadata={
                         "contact_id": str(contact_id),
                         "verified_by": verified_by,
                         "verified_at": verified_at,
+                        "id_type": id_type,
                     },
                     resource_type="crm_contact",
                     resource_id=str(contact_id),
                 )
🤖 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 2366 -
2377, The audit call for success in crm.mark_id_verified is missing the id_type
field; update the metadata passed to self._audit_command (the block that
currently contains "contact_id", "verified_by", "verified_at") to also include
"id_type": id_type so the success event matches the command-level audit entries
and uses the existing id_type variable in scope.
DISCORD_BOT.md (1)

16-21: Clarify Admin-only command placement in permissions list.

Line 19 mentions that mark-id-verified requires Admin, but it's listed under the Steering Committee bullet point, which could be confusing. Consider either moving it to the Admin section or restructuring for clarity.

📝 Suggested restructure
 - `Admin`:
   - `/mark-id-verified`
 - `Steering Committee`:
-  - `/kimai-status`, `/link-discord-user`, `/unlinked-discord-users`, `/set-github-username` (for others), `/upload-resume` (for others), `mark-id-verified` requires `Admin`.
+  - `/kimai-status`, `/link-discord-user`, `/unlinked-discord-users`, `/set-github-username` (for others), `/upload-resume` (for others)
 - `Member`:
   - `/search-members`, `/get-resume`, `/view-skills`
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@DISCORD_BOT.md` around lines 16 - 21, The documentation lists the command
"mark-id-verified" under the "Steering Committee" section while stating it
requires Admin, which is confusing; update DISCORD_BOT.md to either move
"mark-id-verified" into the "Admin" bullet or explicitly annotate it in the
"Steering Committee" list as "mark-id-verified (Admin only)" so the required
role is unambiguous—search for the headings "Admin" and "Steering Committee" and
the command name "mark-id-verified" to locate and correct the
placement/annotation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@apps/discord_bot/src/five08/discord_bot/cogs/crm.py`:
- Around line 2366-2377: The audit call for success in crm.mark_id_verified is
missing the id_type field; update the metadata passed to self._audit_command
(the block that currently contains "contact_id", "verified_by", "verified_at")
to also include "id_type": id_type so the success event matches the
command-level audit entries and uses the existing id_type variable in scope.

In `@DISCORD_BOT.md`:
- Around line 16-21: The documentation lists the command "mark-id-verified"
under the "Steering Committee" section while stating it requires Admin, which is
confusing; update DISCORD_BOT.md to either move "mark-id-verified" into the
"Admin" bullet or explicitly annotate it in the "Steering Committee" list as
"mark-id-verified (Admin only)" so the required role is unambiguous—search for
the headings "Admin" and "Steering Committee" and the command name
"mark-id-verified" to locate and correct the placement/annotation.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 48e6f50 and 5ca9738.

📒 Files selected for processing (4)
  • DISCORD_BOT.md
  • DISCORD_SLASH_COMMANDS.md
  • README.md
  • apps/discord_bot/src/five08/discord_bot/cogs/crm.py

@michaelmwu michaelmwu merged commit ec61b63 into main Mar 2, 2026
5 checks passed
@michaelmwu michaelmwu deleted the michaelmwu/mark-id-type branch March 2, 2026 10:50
michaelmwu added a commit that referenced this pull request Mar 2, 2026
* feat: require id_type for mark-id-verified

* docs: move slash command docs to linked command reference

* docs: add consolidated Discord bot documentation file

* docs: remove obsolete kimai slash commands

* Handle optional id_type for mark-id-verified compatibility
michaelmwu added a commit that referenced this pull request Mar 2, 2026
* Require id_type for mark-id-verified (#76)

* feat: require id_type for mark-id-verified

* docs: move slash command docs to linked command reference

* docs: add consolidated Discord bot documentation file

* docs: remove obsolete kimai slash commands

* Handle optional id_type for mark-id-verified compatibility

* Add job rerun endpoint and CLI tooling (#81)

* add rerun endpoint and jobsctl CLI

* Fix rerun payload validation and stabilize jobsctl tests

* fix: confirm overwrite for ID verified fields (#80)

* fix: confirm ID verification overwrite before update

* fix: require overwrite confirmation for single field conflicts

* feat(worker): add jobsctl recent jobs query

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant