feat: add per-inbox signature management#226
Merged
gabrieljablonski merged 3 commits intomainfrom Feb 26, 2026
Merged
Conversation
- Introduced `InboxSignature` model to manage signatures specific to each inbox. - Added API endpoints for fetching, creating, updating, and deleting inbox signatures. - Updated UI components to support inbox-specific signatures, including overrides for signature position and separator. - Implemented a new composable `useInboxSignatures` for managing inbox signatures in the frontend. - Enhanced existing components to utilize inbox signatures, including the reply box and message signature settings. - Added tests for the new inbox signatures functionality, ensuring proper behavior of the API and model validations. - Updated translations for new UI elements related to inbox signatures.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughIntroduces per-inbox message signatures: new InboxSignature model, API endpoints and views, frontend composable and API client, editor/component wiring to fetch/apply inbox-scoped signatures, settings UI to manage inbox overrides, migrations, and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ReplyBox as ReplyBox (Component)
participant Composable as useInboxSignatures (Composable)
participant API as InboxSignatures API Client
participant Backend as Backend (API)
User->>ReplyBox: mounts / selects inbox
ReplyBox->>Composable: fetchInboxSignatures()
Composable->>API: getAll(accountId)
API->>Backend: GET /api/v1/profile/inbox_signatures
Backend-->>API: inbox signatures list
API-->>Composable: response
Composable->>Composable: build map by inbox_id
User->>ReplyBox: request signature for inbox
ReplyBox->>Composable: getSignatureForInbox(inboxId)
Composable-->>ReplyBox: inbox-specific signature or global fallback
ReplyBox->>ReplyBox: render Editor with overrides / append signature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
db/schema.rb (1)
860-869: Consider adding an index oninbox_idfor query performance.The composite unique index on
[user_id, inbox_id]only optimizes queries that filter byuser_idfirst. Queries filtering byinbox_idalone (e.g., when destroying an inbox and its associated signatures viadependent: :destroy_async) won't benefit from this index.Looking at similar patterns like
inbox_members(Line 857), an individual index oninbox_idis included.Suggested migration to add index
add_index :inbox_signatures, :inbox_idBased on learnings: "Always validate presence and uniqueness in Rails models, and add proper database indexes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@db/schema.rb` around lines 860 - 869, Add a single-column index on inbox_id for the inbox_signatures table to speed queries that filter by inbox_id (e.g., dependent: :destroy_async); create a migration that adds an index on :inbox_id for inbox_signatures (ensure it does not duplicate the existing unique composite index index_inbox_signatures_on_user_id_and_inbox_id) and run migrations to apply it.spec/controllers/api/v1/profile/inbox_signatures_controller_spec.rb (1)
82-128: Consider adding validation error tests.The upsert tests cover happy paths well. Consider adding a test for validation failures (e.g., invalid
signature_positionvalue like'invalid') to ensure the API returns appropriate error responses.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/controllers/api/v1/profile/inbox_signatures_controller_spec.rb` around lines 82 - 128, Add a negative spec that sends invalid params (e.g., signature_position: 'invalid') to the PUT /api/v1/profile/inbox_signatures/:inbox_id endpoint and assert the controller returns a 422 (or appropriate error status) and a JSON error payload; locate the test alongside the existing upsert examples in spec/controllers/api/v1/profile/inbox_signatures_controller_spec.rb, reuse the same request pattern (put ..., params: signature_params, headers: agent.create_new_auth_token, as: :json), and verify that InboxSignature count does not change and that the response.parsed_body contains the model validation errors for signature_position coming from the InboxSignature validations or Api::V1::Profile::InboxSignaturesController.app/javascript/dashboard/routes/dashboard/settings/profile/Index.vue (1)
50-53: Consider awaitingfetchInboxSignaturesor handling potential race conditions.The
fetchInboxSignatures()call is fire-and-forget during setup. While this is acceptable for preloading, if the user interacts with inbox signatures before the fetch completes, there could be stale state issues. Consider whether this timing is acceptable for the UX.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/dashboard/routes/dashboard/settings/profile/Index.vue` around lines 50 - 53, fetchInboxSignatures() is called fire-and-forget which can cause race conditions if the user interacts with inbox signatures before the fetch completes; update the setup logic to await fetchInboxSignatures() (or capture and act on its returned Promise) so the component state is populated before user actions, or add/loading state gating around the UI and ensure upsertInboxSignature and deleteInboxSignature check for pending/loaded state; specifically modify the initialization where useInboxSignatures() is used to either await fetchInboxSignatures() or set a "loadingSignatures" flag that the template and upsertInboxSignature/deleteInboxSignature respect.app/javascript/dashboard/routes/dashboard/settings/profile/MessageSignature.vue (1)
316-316: Minor: Consider using Tailwind utility class instead of customh-[10rem].The
h-[10rem]uses Tailwind's arbitrary value syntax which works, but consider using a standard Tailwind height class (e.g.,h-40= 10rem) for consistency.♻️ Suggested change
<WootMessageEditor id="message-signature-input" v-model="signature" - class="message-editor h-[10rem] !px-3" + class="message-editor h-40 !px-3" is-format-mode🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/dashboard/routes/dashboard/settings/profile/MessageSignature.vue` at line 316, The class on the message editor uses an arbitrary Tailwind value "h-[10rem]"—replace it with the equivalent standard utility "h-40" for consistency; update the element that currently has class "message-editor h-[10rem] !px-3" in MessageSignature.vue to use "message-editor h-40 !px-3".app/javascript/dashboard/components/widgets/conversation/ReplyBox.vue (1)
101-129: Consider removing unusedinboxSignaturesFetchedfrom setup return.
inboxSignaturesFetchedis destructured and returned from setup but is never used elsewhere in the component. Removing it will keep the code clean and reduce unnecessary clutter.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/dashboard/components/widgets/conversation/ReplyBox.vue` around lines 101 - 129, The variable inboxSignaturesFetched is unused; remove it to clean up the setup function by deleting the hasFetched: inboxSignaturesFetched alias from the useInboxSignatures() destructuring and removing inboxSignaturesFetched from the returned object, and verify no other references to inboxSignaturesFetched remain (adjust any remaining logic to use hasFetched or fetch status from useInboxSignatures() directly if needed).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/controllers/api/v1/profile/inbox_signatures_controller.rb`:
- Around line 5-12: The index action currently trusts params[:account_id] and
uses Account.find(...) which allows users to access other accounts' signatures;
update the logic in index to authorize the requested account before using its
inbox_ids — e.g. find the account scoped to the current user or use your
authorization policy (authorize Account) to ensure the user belongs to that
account, then use that account's inbox_ids for `@user.inbox_signatures`; if the
account is not accessible, fall back to `@user.inbox_signatures` or return a
forbidden response.
- Around line 18-26: In update action, before creating a new InboxSignature via
`@user.inbox_signatures.create`! (when `@inbox_signature` is nil), verify the
current user is a member of the target inbox (params[:inbox_id]) using the
existing membership check (e.g., InboxMember.exists?(user: `@user`, inbox_id:
params[:inbox_id]) or the controller's authorize/membership helper); if the user
is not a member, halt and respond with forbidden/unauthorized (raise
Pundit::NotAuthorizedError or render head :forbidden) instead of creating the
signature; keep the membership check path for both update and create branches so
`@inbox_signature.update`! still requires membership.
In `@db/migrate/20260226173647_create_inbox_signatures.rb`:
- Around line 4-5: The migration CreateInboxSignatures currently adds
t.references :user and t.references :inbox without DB-level foreign keys; update
the migration so these references include foreign_key constraints (or add
explicit add_foreign_key calls for user_id and inbox_id) to enforce referential
integrity and prevent orphaned inbox_signatures rows when users or inboxes are
deleted; modify the t.references lines in the CreateInboxSignatures migration or
append add_foreign_key :inbox_signatures, :users and add_foreign_key
:inbox_signatures, :inboxes accordingly.
---
Nitpick comments:
In `@app/javascript/dashboard/components/widgets/conversation/ReplyBox.vue`:
- Around line 101-129: The variable inboxSignaturesFetched is unused; remove it
to clean up the setup function by deleting the hasFetched:
inboxSignaturesFetched alias from the useInboxSignatures() destructuring and
removing inboxSignaturesFetched from the returned object, and verify no other
references to inboxSignaturesFetched remain (adjust any remaining logic to use
hasFetched or fetch status from useInboxSignatures() directly if needed).
In `@app/javascript/dashboard/routes/dashboard/settings/profile/Index.vue`:
- Around line 50-53: fetchInboxSignatures() is called fire-and-forget which can
cause race conditions if the user interacts with inbox signatures before the
fetch completes; update the setup logic to await fetchInboxSignatures() (or
capture and act on its returned Promise) so the component state is populated
before user actions, or add/loading state gating around the UI and ensure
upsertInboxSignature and deleteInboxSignature check for pending/loaded state;
specifically modify the initialization where useInboxSignatures() is used to
either await fetchInboxSignatures() or set a "loadingSignatures" flag that the
template and upsertInboxSignature/deleteInboxSignature respect.
In
`@app/javascript/dashboard/routes/dashboard/settings/profile/MessageSignature.vue`:
- Line 316: The class on the message editor uses an arbitrary Tailwind value
"h-[10rem]"—replace it with the equivalent standard utility "h-40" for
consistency; update the element that currently has class "message-editor
h-[10rem] !px-3" in MessageSignature.vue to use "message-editor h-40 !px-3".
In `@db/schema.rb`:
- Around line 860-869: Add a single-column index on inbox_id for the
inbox_signatures table to speed queries that filter by inbox_id (e.g.,
dependent: :destroy_async); create a migration that adds an index on :inbox_id
for inbox_signatures (ensure it does not duplicate the existing unique composite
index index_inbox_signatures_on_user_id_and_inbox_id) and run migrations to
apply it.
In `@spec/controllers/api/v1/profile/inbox_signatures_controller_spec.rb`:
- Around line 82-128: Add a negative spec that sends invalid params (e.g.,
signature_position: 'invalid') to the PUT
/api/v1/profile/inbox_signatures/:inbox_id endpoint and assert the controller
returns a 422 (or appropriate error status) and a JSON error payload; locate the
test alongside the existing upsert examples in
spec/controllers/api/v1/profile/inbox_signatures_controller_spec.rb, reuse the
same request pattern (put ..., params: signature_params, headers:
agent.create_new_auth_token, as: :json), and verify that InboxSignature count
does not change and that the response.parsed_body contains the model validation
errors for signature_position coming from the InboxSignature validations or
Api::V1::Profile::InboxSignaturesController.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
app/controllers/api/v1/profile/inbox_signatures_controller.rbapp/javascript/dashboard/api/inboxSignatures.jsapp/javascript/dashboard/components-next/NewConversation/ComposeConversation.vueapp/javascript/dashboard/components-next/NewConversation/components/ComposeNewConversationForm.vueapp/javascript/dashboard/components-next/NewConversation/helpers/composeConversationHelper.jsapp/javascript/dashboard/components/widgets/WootWriter/Editor.vueapp/javascript/dashboard/components/widgets/conversation/ReplyBox.vueapp/javascript/dashboard/composables/spec/useInboxSignatures.spec.jsapp/javascript/dashboard/composables/useInboxSignatures.jsapp/javascript/dashboard/i18n/locale/en/conversation.jsonapp/javascript/dashboard/i18n/locale/en/settings.jsonapp/javascript/dashboard/i18n/locale/pt_BR/conversation.jsonapp/javascript/dashboard/i18n/locale/pt_BR/settings.jsonapp/javascript/dashboard/routes/dashboard/settings/profile/Index.vueapp/javascript/dashboard/routes/dashboard/settings/profile/MessageSignature.vueapp/models/inbox.rbapp/models/inbox_signature.rbapp/models/user.rbapp/views/api/v1/profile/inbox_signatures/_inbox_signature.json.jbuilderapp/views/api/v1/profile/inbox_signatures/index.json.jbuilderapp/views/api/v1/profile/inbox_signatures/show.json.jbuilderapp/views/api/v1/profile/inbox_signatures/update.json.jbuilderconfig/routes.rbdb/migrate/20260226173647_create_inbox_signatures.rbdb/schema.rblib/captain/reply_suggestion_service.rbspec/controllers/api/v1/profile/inbox_signatures_controller_spec.rbspec/factories/inbox_signatures.rbspec/models/inbox_signature_spec.rb
There was a problem hiding this comment.
Pull request overview
This pull request introduces a comprehensive per-inbox signature management feature that allows users to configure unique email/message signatures for each inbox they manage, with fallback to a global default signature. The feature is well-implemented across both backend (Ruby on Rails) and frontend (Vue.js) with appropriate test coverage.
Changes:
- Backend: Added
InboxSignaturemodel with database migration, API endpoints, and business logic integration - Frontend: Implemented UI components for managing inbox-specific signatures with a composable pattern for state management
- Translations: Added complete English and Portuguese (pt-BR) translations for all new UI elements
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
db/migrate/20260226173647_create_inbox_signatures.rb |
Migration to create inbox_signatures table with unique constraint on user_id and inbox_id |
db/schema.rb |
Updated schema to reflect new inbox_signatures table |
app/models/inbox_signature.rb |
Model with validations for signature content, position, and separator |
app/models/user.rb |
Added has_many association to inbox_signatures |
app/models/inbox.rb |
Added has_many association to inbox_signatures |
app/controllers/api/v1/profile/inbox_signatures_controller.rb |
RESTful API endpoints for managing inbox signatures |
app/views/api/v1/profile/inbox_signatures/*.json.jbuilder |
JSON view templates for API responses |
config/routes.rb |
Added routes for inbox signatures API |
lib/captain/reply_suggestion_service.rb |
Integrated inbox-specific signatures into AI reply suggestions |
app/javascript/dashboard/api/inboxSignatures.js |
API client for inbox signatures endpoints |
app/javascript/dashboard/composables/useInboxSignatures.js |
Vue composable for managing inbox signatures state |
app/javascript/dashboard/composables/spec/useInboxSignatures.spec.js |
Comprehensive unit tests for the composable |
app/javascript/dashboard/routes/dashboard/settings/profile/MessageSignature.vue |
Enhanced UI component with inbox selector and signature management |
app/javascript/dashboard/routes/dashboard/settings/profile/Index.vue |
Integration of inbox signature handlers |
app/javascript/dashboard/components/widgets/conversation/ReplyBox.vue |
Updated to use inbox-specific signatures |
app/javascript/dashboard/components/widgets/WootWriter/Editor.vue |
Added support for signature position/separator overrides with tooltips |
app/javascript/dashboard/components-next/NewConversation/*.vue |
Updated compose conversation components to support inbox signatures |
app/javascript/dashboard/i18n/locale/en/*.json |
English translations for new UI elements |
app/javascript/dashboard/i18n/locale/pt_BR/*.json |
Portuguese translations for new UI elements |
spec/models/inbox_signature_spec.rb |
Model tests for validations and associations |
spec/factories/inbox_signatures.rb |
Factory for creating test inbox signatures |
spec/controllers/api/v1/profile/inbox_signatures_controller_spec.rb |
API endpoint tests covering CRUD operations |
app/javascript/dashboard/routes/dashboard/settings/profile/MessageSignature.vue
Show resolved
Hide resolved
app/javascript/dashboard/routes/dashboard/settings/profile/MessageSignature.vue
Show resolved
Hide resolved
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/controllers/api/v1/profile/inbox_signatures_controller.rb`:
- Around line 56-61: The authorization check in validate_account_access!
currently calls head :unauthorized but does not halt execution, so the
controller action (e.g., index) can continue; update validate_account_access!
(and mirror the same fix used for validate_inbox_access) to immediately return
after sending the unauthorized response — for example, change the unauthorized
branch to use return head :unauthorized or add an explicit return after head
:unauthorized so the action stops executing when the account check fails.
- Around line 49-54: The authorization check in validate_inbox_access uses head
:unauthorized but does not halt execution; update the validate_inbox_access
method so that when InboxMember.exists?(user_id: `@user.id`, inbox_id: inbox_id)
is false you call head :unauthorized and return to stop the action from
continuing. Locate the validate_inbox_access method and append an explicit
return after head :unauthorized (e.g., head :unauthorized and return) to ensure
the controller action is not executed for unauthorized requests.
In
`@app/javascript/dashboard/routes/dashboard/settings/profile/MessageSignature.vue`:
- Around line 234-243: The label uses for="inboxSelector" but the SingleSelect
component (v-model="selectedInbox") has no matching id, breaking label-control
association; update the SingleSelect instance to include an id="inboxSelector"
prop (or pass the id via the component's prop name if it uses a different prop
like inputId) so the label is explicitly associated with the control, or
alternatively add an aria-labelledby/aria-describedby that references the
label—ensure the identifier exactly matches "inboxSelector" and adjust the
SingleSelect prop name if the component expects a different prop.
- Around line 162-164: fetchInboxSignatures() is called async but the component
never refreshes the selected inbox form when that fetch completes; ensure the
selected inbox signature/state is reloaded after the fetch resolves by making
fetchInboxSignatures() return a Promise (or await it) and then calling the
existing selected-inbox loader (e.g., the method that populates selectedInbox or
the reactive state used for the form) to rehydrate the form values so you don't
fallback to global values and accidentally overwrite overrides; update the call
site in MessageSignature.vue (where fetchInboxSignatures() is invoked on mount
and/or on inbox change) to await the fetch and then invoke the routine that sets
selectedInbox/selectedInboxSignature so the correct override values are applied.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/controllers/api/v1/profile/inbox_signatures_controller.rbapp/javascript/dashboard/components/widgets/conversation/ReplyBox.vueapp/javascript/dashboard/routes/dashboard/settings/profile/MessageSignature.vueapp/models/inbox_signature.rbdb/migrate/20260226194714_add_inbox_id_index_to_inbox_signatures.rbdb/schema.rbspec/controllers/api/v1/profile/inbox_signatures_controller_spec.rbspec/models/inbox_signature_spec.rb
🚧 Files skipped from review as they are similar to previous changes (5)
- app/javascript/dashboard/components/widgets/conversation/ReplyBox.vue
- spec/models/inbox_signature_spec.rb
- spec/controllers/api/v1/profile/inbox_signatures_controller_spec.rb
- db/schema.rb
- app/models/inbox_signature.rb
app/javascript/dashboard/routes/dashboard/settings/profile/MessageSignature.vue
Outdated
Show resolved
Hide resolved
app/javascript/dashboard/routes/dashboard/settings/profile/MessageSignature.vue
Outdated
Show resolved
Hide resolved
app/javascript/dashboard/routes/dashboard/settings/profile/MessageSignature.vue
Outdated
Show resolved
Hide resolved
gabrieljablonski
deleted the
gabrieljablonski/feat-signature-per-inbox
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
InboxSignaturemodel to manage signatures specific to each inbox.useInboxSignaturesfor managing inbox signatures in the frontend.Pull Request Template
Description
Please include a summary of the change and issue(s) fixed. Also, mention relevant motivation, context, and any dependencies that this change requires.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.
Checklist:
This change is
Summary by CodeRabbit