Skip to content

feat: allow ban/mute first arg to be a user instead of repliedTo#88

Merged
toto04 merged 4 commits intomainfrom
muteban-args-2
Apr 11, 2026
Merged

feat: allow ban/mute first arg to be a user instead of repliedTo#88
toto04 merged 4 commits intomainfrom
muteban-args-2

Conversation

@lorenzocorallo
Copy link
Copy Markdown
Member

No description provided.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 11, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c2e3e7a3-59b5-4875-9b01-1b97422fcc3c

📥 Commits

Reviewing files that changed from the base of the PR and between 896fa7c and 82a44c4.

📒 Files selected for processing (3)
  • src/commands/moderation/ban.ts
  • src/commands/moderation/mute.ts
  • src/utils/users.ts

Walkthrough

The ban and mute command handlers were refactored to resolve target users from either a reply or an explicit argument via a new getOverloadUser utility; command configs now accept an optional reasonOrUser and reply is optional. Handlers use the resolved { user, reason } when invoking moderation actions.

Changes

Cohort / File(s) Summary
Moderation Commands
src/commands/moderation/ban.ts, src/commands/moderation/mute.ts
Added optional reasonOrUser arg and changed reply from required to optional. Replaced direct repliedTo.from checks with calls to getOverloadUser, handle Err cases with ephemeral replies/logging, and pass repliedTo ? [repliedTo] : undefined when invoking moderation.
User Resolution Utility
src/utils/users.ts
New exported getOverloadUser that returns Promise<Result<{ user: User; reason?: string }, string>>. Resolves a target User from a replied-to message or from firstArg (converting string IDs via getTelegramId), and derives reason from reply or secondArg; returns distinct error messages for failure cases.

Sequence Diagram(s)

sequenceDiagram
  participant User as User (actor)
  participant Cmd as CommandHandler
  participant Util as getOverloadUser
  participant IDSvc as getTelegramId
  participant Mod as ModerationService

  User->>Cmd: sends command (reply or args)
  Cmd->>Util: getOverloadUser(context, repliedTo, firstArg, secondArg)
  alt firstArg is string needing conversion
    Util->>IDSvc: getTelegramId(firstArg)
    IDSvc-->>Util: telegramId | error
  end
  Util-->>Cmd: Result.Ok({ user, reason }) or Result.Err(errorMsg)
  alt Ok
    Cmd->>Mod: Moderation.mute/ban(user, reason, maybe [repliedTo])
    Mod-->>Cmd: success|failure
    Cmd-->>User: public or ephemeral confirmation
  else Err
    Cmd-->>User: ephemeral error (message varies if reply existed)
  end
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: allowing ban/mute commands to accept a user as the first argument instead of requiring a reply context.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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
Contributor

@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.

Actionable comments posted: 2

🧹 Nitpick comments (1)
src/commands/moderation/mute.ts (1)

69-80: Consider extracting the duplicate error handling.

The error handling block is identical between ban.ts and mute.ts. If more commands adopt this pattern, consider extracting a helper function in getOverloadUser or a shared utility that handles the ephemeral error reply and logging.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/commands/moderation/mute.ts` around lines 69 - 80, The duplicate
error-handling block around calling getOverloadUser in mute.ts (and the similar
block in ban.ts) should be extracted into a shared helper to avoid repetition:
create a utility function (e.g., handleOverloadUserError) that accepts the
Result from getOverloadUser, the context, repliedTo flag, args, and logger,
performs the ephemeral reply (using ephemeral and fmt) and logger.error with the
provided args and error, and returns a boolean or throws to indicate handling;
replace the inline block in mute.ts (and update ban.ts) to call
handleOverloadUserError(getOverloadUser(...), context, repliedTo, args, logger)
and early-return when it reports an error. Ensure the helper preserves the
existing reply messages and logging format so behavior is unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/commands/moderation/ban.ts`:
- Around line 37-40: The user-facing string in the ternary that uses repliedTo
contains a typo ("a their message"); update the fmt call in the repliedTo branch
inside the ban command (the ternary using repliedTo in
src/commands/moderation/ban.ts) to a grammatically correct message such as
fmt(({ n }) => n`Target user not found, please try replying to their message`)
(or "please try replying to the user's message") so the displayed text is
correct.

In `@src/commands/moderation/mute.ts`:
- Around line 73-76: The user-facing error message in the ternary inside mute.ts
incorrectly says "a their message"; update the fmt call that uses repliedTo (the
branch producing `Target user not found, please try replying to a their
message`) to a grammatically correct string (e.g., "please try replying to their
message" or "please try replying to the user's message"). Locate the ternary
using the repliedTo variable and replace the bad string in that fmt(({ n }) =>
n`...`) invocation so it matches the corrected wording used in ban.ts.

---

Nitpick comments:
In `@src/commands/moderation/mute.ts`:
- Around line 69-80: The duplicate error-handling block around calling
getOverloadUser in mute.ts (and the similar block in ban.ts) should be extracted
into a shared helper to avoid repetition: create a utility function (e.g.,
handleOverloadUserError) that accepts the Result from getOverloadUser, the
context, repliedTo flag, args, and logger, performs the ephemeral reply (using
ephemeral and fmt) and logger.error with the provided args and error, and
returns a boolean or throws to indicate handling; replace the inline block in
mute.ts (and update ban.ts) to call
handleOverloadUserError(getOverloadUser(...), context, repliedTo, args, logger)
and early-return when it reports an error. Ensure the helper preserves the
existing reply messages and logging format so behavior is unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f8f61a04-d7e5-402b-96b7-f53d316a79d7

📥 Commits

Reviewing files that changed from the base of the PR and between 6f9b761 and 896fa7c.

📒 Files selected for processing (3)
  • src/commands/moderation/ban.ts
  • src/commands/moderation/mute.ts
  • src/utils/users.ts

Comment thread src/commands/moderation/ban.ts
Comment thread src/commands/moderation/mute.ts
@toto04 toto04 enabled auto-merge (squash) April 11, 2026 17:06
@toto04 toto04 merged commit aa05741 into main Apr 11, 2026
1 of 2 checks passed
@toto04 toto04 deleted the muteban-args-2 branch April 11, 2026 17:07
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.

2 participants