Skip to content

feat: role menu templates (#135)#216

Merged
BillChirico merged 18 commits intomainfrom
feat/issue-135
Mar 2, 2026
Merged

feat: role menu templates (#135)#216
BillChirico merged 18 commits intomainfrom
feat/issue-135

Conversation

@BillChirico
Copy link
Collaborator

Summary

Implements reusable role menu templates as described in #135.

Features

Pre-defined built-in templates (auto-seeded on startup)

  • 🎨 color-roles — Red, Blue, Green, Yellow, Purple
  • 🏷️ pronouns — he/him, she/her, they/them, any pronouns, ask my pronouns
  • 🔔 notifications — Announcements, Events, Updates, Releases

Custom template creation

  • /rolemenu template create <name> <options-json> — create a guild-specific template
  • Options support label, description, and roleId fields
  • Full input validation (name, options count, label length)

One-click apply

  • /rolemenu template apply <name> [merge] — applies template to guild's welcome.roleMenu config
  • Optional merge flag preserves existing options (role IDs carried over by label matching)
  • Warns when applying built-in templates that need role IDs assigned

Template sharing between guilds

  • /rolemenu template share <name> <true|false> — toggle cross-guild visibility
  • Shared templates are queryable by all guilds (read-only; only the owning guild can edit/delete)

Additional commands

  • /rolemenu template list — categorised embed of all visible templates
  • /rolemenu template info <name> — details + option list
  • /rolemenu template delete <name> — delete owned template

Database

Migration 004_role_menu_templates.cjs adds role_menu_templates table with:

  • Unique index on (name, guild_id) (built-ins keyed to __builtin__ sentinel)
  • Indexes for guild lookup and shared-template queries

Tests

  • 36 unit tests for the module (roleMenuTemplates.test.js)
  • 19 command handler tests (rolemenu.test.js)
  • All 55 new tests pass; pre-existing failures (redis, triage coverage, ai-feedback) are unrelated to this PR

Closes #135

Copilot AI review requested due to automatic review settings March 2, 2026 04:40
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 2, 2026

Warning

Rate limit exceeded

@BillChirico has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 7 minutes and 12 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 2791afb and 2e3d6ce.

📒 Files selected for processing (6)
  • migrations/004_role_menu_templates.cjs
  • src/commands/rolemenu.js
  • src/index.js
  • src/modules/roleMenuTemplates.js
  • tests/commands/rolemenu.test.js
  • tests/modules/roleMenuTemplates.test.js
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/issue-135

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.

@greptile-apps
Copy link

greptile-apps bot commented Mar 2, 2026

Greptile Summary

Implements reusable role menu templates with 3 built-in templates (color-roles, pronouns, notifications) and custom template creation. Adds database migration for role_menu_templates table with proper indexing, template management module with validation and CRUD operations, and /rolemenu template command with 6 subcommands (list, info, apply, create, delete, share).

Key changes:

  • Migration creates table with unique case-insensitive index on (name, guild_id) and indexes for shared templates
  • Module uses parameterized queries throughout (SQL injection safe) and Winston logging per project standards
  • Built-in templates seed automatically on startup (idempotent)
  • Template application supports merge mode to preserve existing role IDs by label matching
  • 55 new tests (36 unit + 19 command handler) with comprehensive coverage

Issues found:

  • shouldEnable logic (line 217) disables role menu when applying built-in template with merge, even when valid options exist
  • Previous comments note ON CONFLICT syntax concern and filtering behavior that removes options before config save

Confidence Score: 4/5

  • Safe to merge with minor logic fix recommended
  • Well-structured implementation with comprehensive tests (55 new tests), proper validation, parameterized queries, and Winston logging. One logic issue found (shouldEnable flag) that affects merge behavior but doesn't break core functionality. Previous review comments about filtering and ON CONFLICT have been acknowledged.
  • src/commands/rolemenu.js (handleApply function line 217)

Important Files Changed

Filename Overview
migrations/004_role_menu_templates.cjs Adds role_menu_templates table with appropriate indexes for built-in and guild-specific templates
src/modules/roleMenuTemplates.js Implements core template management logic with proper validation, parameterized queries, and Winston logging
src/commands/rolemenu.js Implements /rolemenu command with template operations; has logic issue with shouldEnable flag when merging built-in templates
src/index.js Adds seedBuiltinTemplates call during startup with proper error handling

Sequence Diagram

sequenceDiagram
    participant User
    participant Discord
    participant rolemenu.js
    participant roleMenuTemplates.js
    participant Database
    participant config.js

    User->>Discord: /rolemenu template apply <name> [merge]
    Discord->>rolemenu.js: handleApply()
    rolemenu.js->>roleMenuTemplates.js: getTemplateByName(guildId, name)
    roleMenuTemplates.js->>Database: SELECT template WHERE visible to guild
    Database-->>roleMenuTemplates.js: template row
    roleMenuTemplates.js-->>rolemenu.js: template object
    
    rolemenu.js->>config.js: getConfig(guildId)
    config.js-->>rolemenu.js: existing roleMenu config
    
    rolemenu.js->>roleMenuTemplates.js: applyTemplateToOptions(tpl, existing)
    Note over roleMenuTemplates.js: Merge template options<br/>with existing roleIds<br/>by label matching
    roleMenuTemplates.js-->>rolemenu.js: merged options
    
    Note over rolemenu.js: Filter out options<br/>with empty roleIds
    
    rolemenu.js->>config.js: setConfigValue('welcome.roleMenu.enabled', shouldEnable)
    rolemenu.js->>config.js: setConfigValue('welcome.roleMenu.options', validOptions)
    config.js->>Database: UPDATE guild_configs
    
    rolemenu.js->>Discord: Reply with success message
    Discord->>User: Template applied confirmation
Loading

Last reviewed commit: 2e3d6ce

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

6 files reviewed, 5 comments

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

coderabbitai[bot]
coderabbitai bot previously approved these changes Mar 2, 2026
BillChirico and others added 3 commits March 2, 2026 04:57
Fixes three performance bottlenecks identified in code review of
recently merged features (PR #121 conversations viewer, PR #190 AI feedback).

## Changes

### migrations/004_performance_indexes.cjs (new)
Four new indexes targeting hot query paths:

- idx_ai_feedback_guild_created (guild_id, created_at DESC)
  getFeedbackTrend() and getRecentFeedback() filtered by guild_id
  AND created_at but only had a single-column guild_id index, forcing
  a full guild scan + sort on every trend/recent call.

- idx_conversations_content_trgm (GIN, pg_trgm)
  content ILIKE '%...%' search was a sequential scan. GIN/trgm index
  reduces this from O(n) to O(log n * trigram matches).
  Requires pg_trgm extension (added idempotently).

- idx_conversations_guild_created (guild_id, created_at DESC)
  Default 30-day listing query filters guild_id + created_at. The
  existing 3-column (guild_id, channel_id, created_at) composite is
  suboptimal when channel_id is not in the predicate.

- idx_flagged_messages_guild_message (guild_id, message_id)
  Conversation detail + flag endpoints query flagged_messages by
  guild_id AND message_id = ANY(...). Existing index only covers
  (guild_id, status).

### src/api/routes/conversations.js
**GET / — Replace in-memory pagination with SQL CTE grouping**

Before: fetched up to 10,000 message rows into Node memory, grouped
them in JavaScript (O(n) time + memory), then sliced for pagination.
Every page request loaded the full 10k row dataset.

After: single SQL query using window functions (LAG + SUM OVER) to
identify conversation boundaries and aggregate summaries directly.
COUNT(*) OVER() provides total count without a second query.
Pagination happens at the DB with LIMIT/OFFSET on summary rows.
Memory overhead is now proportional to page size (default 25), not
total conversation volume.

Removed now-unused buildConversationSummary() helper (logic inlined
into the SQL-side aggregation).

**POST /:conversationId/flag — Parallel verification queries**

Before: msgCheck and anchorCheck ran sequentially (~2× RTT).
After: both run in parallel via Promise.all (1× RTT for verification).

### tests/api/routes/conversations.test.js
Updated 'should return paginated conversations' test to mock the new
SQL CTE response shape (pre-aggregated summary rows) instead of raw
message rows. All 41 conversation tests pass.
* feat: quiet mode per-channel via bot mention (#173)

- Add quietMode.js module with Redis+memory storage
- Parse duration from natural language (30m, 1 hour, etc.)
- Permission gated via config.quietMode.allowedRoles
- Commands: quiet, unquiet, status
- Suppress AI responses during quiet mode in events.js
- Add quietMode section to config.json (disabled by default)
- Add quietMode to configAllowlist.js for dashboard editing

* test: add quiet mode tests (41 tests, all passing)

* style: fix biome formatting in quietMode.js, events.js, and test

* fix(web): fix ai-feedback-stats TypeScript and formatting errors

* fix: gate quiet mode checks on enabled flag, validate TTL, honor maxDurationMinutes config

- events.js: Wrap isQuietMode() calls in guildConfig.quietMode?.enabled check
  to avoid unnecessary Redis lookups and prevent stale records from suppressing
  AI responses when the feature is disabled (PRRT_kwDORICdSM5xdbmp, PRRT_kwDORICdSM5xdbmx)

- quietMode.js: Add TTL validation in setQuiet() to guard against 0, negative,
  or NaN values that would error in Redis (PRRT_kwDORICdSM5xdbm3)

- quietMode.js: Update parseDurationFromContent() to accept config parameter
  and honor guildConfig.quietMode.maxDurationMinutes. Also clamp defaultSeconds
  to the effective max (PRRT_kwDORICdSM5xdbm_)

- configValidation.js: Add quietMode schema entry with enabled, maxDurationMinutes,
  and allowedRoles properties (PRRT_kwDORICdSM5xdbnH)

* style: fix biome formatting in quietMode.js and ai-feedback-stats.tsx
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 8 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@BillChirico BillChirico enabled auto-merge (squash) March 2, 2026 10:16
@BillChirico BillChirico disabled auto-merge March 2, 2026 10:17
Copilot AI review requested due to automatic review settings March 2, 2026 10:18
@BillChirico BillChirico enabled auto-merge (squash) March 2, 2026 10:19
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

coderabbitai[bot]
coderabbitai bot previously approved these changes Mar 2, 2026
Bill added 2 commits March 2, 2026 05:40
- Add ORDER BY to getTemplateByName for deterministic results
- Fix roleId precedence to preserve existing roleIds during merge
- Truncate Discord embed field values to 1024 chars
The test expected template roleId to win, but the comment said existing
should take precedence. Fixed assertion to match documented behavior.
coderabbitai[bot]
coderabbitai bot previously approved these changes Mar 2, 2026
- Filter out options with empty roleIds before saving
- Only enable role menu for non-built-in templates with valid options
- Add user-facing note when options are filtered
Bill added 2 commits March 2, 2026 05:53
Use LOWER(name) in unique index to match case-insensitive queries
and prevent duplicate templates differing only by case.
coderabbitai[bot]
coderabbitai bot previously approved these changes Mar 2, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- validateTemplateOptions now validates that optional roleId and
  description fields are strings when present
- Update JSDoc @see reference from issue #135 (voice tracking) to
  issue #216 (role menu templates)
- Update ON CONFLICT clause to use constraint name for consistency
  with the new LOWER(name) index
@BillChirico BillChirico merged commit c3a1ac2 into main Mar 2, 2026
12 of 15 checks passed
@BillChirico BillChirico deleted the feat/issue-135 branch March 2, 2026 13:08
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.

feat: voice channel tracking — join/leave logs, time stats, temp channels

2 participants