-
Notifications
You must be signed in to change notification settings - Fork 71
Allow disabling AI assistant #146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow disabling AI assistant #146
Conversation
WalkthroughThe changes implement functionality to disable AI through the user settings interface. This includes a destructive-styled button in the AI prompts settings view, controller logic to handle both enabling and disabling AI with appropriate notices, a translation key, and system tests verifying the disable workflow. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as AI Prompts View
participant Controller as UsersController
participant Model as User Model
participant DB as Database
User->>UI: Click "Disable AI Assistant" button
UI->>Controller: PATCH /users (ai_enabled: false)
Controller->>Model: update(ai_enabled: false)
Model->>DB: persist ai_enabled change
DB-->>Model: confirm
Controller->>Controller: Build notice: "AI Assistant has been disabled"
Controller-->>UI: Redirect to ai_prompts with notice
UI-->>User: Show settings page with success notice
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes The changes follow a clear, consistent pattern for adding a user-facing toggle feature. The logic is straightforward (button click → controller update → redirect), files are focused and cohesive, and modifications are primarily additive rather than complex rewrites. The system test is well-structured and validates the happy path clearly. Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
app/controllers/users_controller.rb(2 hunks)app/views/settings/ai_prompts/show.html.erb(1 hunks)config/locales/views/settings/en.yml(1 hunks)test/system/settings/ai_prompts_test.rb(1 hunks)
🧰 Additional context used
📓 Path-based instructions (23)
config/**
📄 CodeRabbit inference engine (AGENTS.md)
Store application and environment configuration under config/
Files:
config/locales/views/settings/en.yml
config/locales/**/*.{yml,yaml,json}
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
Ignore i18n files; do not add or modify locale files
Files:
config/locales/views/settings/en.yml
app/**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Place Rails application Ruby code (models, controllers, services, jobs, mailers, components) under app/
Files:
app/controllers/users_controller.rb
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby style: 2-space indentation; snake_case for methods/variables; CamelCase for classes/modules
Files:
app/controllers/users_controller.rbtest/system/settings/ai_prompts_test.rb
**/*.{rb,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{rb,js,jsx,ts,tsx}: Make changes atomic, testable, and explain their impact briefly in code suggestions.
Respect existing tests and add tests when changing critical logic.
Files:
app/controllers/users_controller.rbtest/system/settings/ai_prompts_test.rb
**/app/**/*.{rb,erb,js,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Push Rails to its limits before adding new dependencies; a strong technical/business reason is required for new dependencies.
Files:
app/controllers/users_controller.rbapp/views/settings/ai_prompts/show.html.erb
**/app/controllers/**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
Controllers should be skinny; business logic should go in models (
app/models/), not inapp/services/.
Files:
app/controllers/users_controller.rb
**/*.{rb,erb,haml,slim}
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
**/*.{rb,erb,haml,slim}: UseCurrent.userfor the current user; do not usecurrent_user
UseCurrent.familyfor the current family; do not usecurrent_family
Ignore i18n methods; hardcode strings in English for now (do not useI18n.t,t, or similar)
Files:
app/controllers/users_controller.rbapp/views/settings/ai_prompts/show.html.erbtest/system/settings/ai_prompts_test.rb
app/{models,controllers,views}/**/*.{rb,erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Avoid N+1 queries
Files:
app/controllers/users_controller.rbapp/views/settings/ai_prompts/show.html.erb
app/views/**/*.erb
📄 CodeRabbit inference engine (AGENTS.md)
app/views/**/*.erb: Keep heavy logic out of ERB views; prefer helpers/components instead
ERB templates are linted by erb-lint per .erb_lint.ymlAlways use the icon helper (icon) for icons; never call lucide_icon directly
Files:
app/views/settings/ai_prompts/show.html.erb
app/views/**/*.html.*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/views/**/*.html.*: Use partials only for simple, context-specific, mostly static HTML content.
Prefer semantic HTML; use Turbo Frames where possible instead of client-side solutions.
Use query params for state, not localStorage or sessionStorage.
Always perform server-side formatting for currencies, numbers, and dates.
Files:
app/views/settings/ai_prompts/show.html.erb
**/*.{html,erb,slim,js,jsx,ts,tsx,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Always use functional design tokens (e.g., text-primary, bg-container) from the design system; do not use raw colors or ad-hoc classes.
Files:
app/views/settings/ai_prompts/show.html.erb
**/app/**/*.erb
📄 CodeRabbit inference engine (CLAUDE.md)
Always use the
iconhelper for icons in templates; never uselucide_icondirectly.
Files:
app/views/settings/ai_prompts/show.html.erb
**/app/views/**/*.erb
📄 CodeRabbit inference engine (CLAUDE.md)
**/app/views/**/*.erb: Keep domain logic OUT of view templates; logic belongs in component files, not templates.
Use partials only for primarily static, simple HTML with minimal logic, and only when not likely to be reused.
Prefer native HTML over JavaScript components for UI elements (e.g., use<dialog>,<details><summary>).
Leverage Turbo frames for page sections over client-side JavaScript solutions.
Use query params (not localStorage/sessions) for client state management.
Perform server-side formatting for currencies, numbers, and dates in templates.
Files:
app/views/settings/ai_prompts/show.html.erb
app/views/**/*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
app/views/**/*.html.erb: Prefer native HTML elements (e.g., ,) over JS-based components
Leverage Turbo frames to break up pages instead of JS-driven client-side solutions
Prefer native client-side form validation when possible
Files:
app/views/settings/ai_prompts/show.html.erb
{app/javascript/controllers/**/*.{js,ts},app/views/**/*.erb,app/components/**/*.erb}
📄 CodeRabbit inference engine (.cursor/rules/stimulus_conventions.mdc)
Use declarative Stimulus actions in ERB (data-action) instead of imperative event listeners in controller lifecycle (e.g., avoid addEventListener in connect); controllers should just respond to actions
Files:
app/views/settings/ai_prompts/show.html.erb
{app/components/**/*.{js,ts,erb},app/views/**/*.erb}
📄 CodeRabbit inference engine (.cursor/rules/stimulus_conventions.mdc)
Component-scoped Stimulus controllers in app/components must be used only within their component views, not in app/views
Files:
app/views/settings/ai_prompts/show.html.erb
{app/views/**,app/helpers/**,app/javascript/controllers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
{app/views/**,app/helpers/**,app/javascript/controllers/**}: Style UI using TailwindCSS v4.x with the custom design system defined in app/assets/tailwind/maybe-design-system.css
Always start by referencing app/assets/tailwind/maybe-design-system.css to understand available primitives, functional tokens, and component tokens before styling
Prefer functional tokens from the design system over raw Tailwind values (e.g., use text-primary, bg-container, border border-primary instead of text-white, bg-white, border-gray-200)
Files:
app/views/settings/ai_prompts/show.html.erb
{app/views/**,app/helpers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
Always generate semantic HTML
Files:
app/views/settings/ai_prompts/show.html.erb
{app/views,app/components}/**/*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)
{app/views,app/components}/**/*.html.erb: Keep domain logic out of ERB templates; compute values in component/controller code and reference them in the template
Integrate Stimulus declaratively in ERB: templates declare data-controller/data-action; controllers respond to those declarations
Pass data from Rails to Stimulus via data-*-value attributes, not inline JavaScript
Files:
app/views/settings/ai_prompts/show.html.erb
test/**/*_test.rb
📄 CodeRabbit inference engine (AGENTS.md)
Name Minitest files with *_test.rb and mirror the app/ structure under test/
test/**/*_test.rb: ALWAYS use Minitest + fixtures + Mocha for tests; NEVER RSpec or FactoryBot.
Use Mocha for mocking in tests when necessary.
Use VCR for external API tests.
test/**/*_test.rb: Always use Minitest for tests; do not use RSpec
Do not use factories (e.g., FactoryBot) in tests; rely on fixtures instead
For tests needing many records, use Rails helpers to construct data and inline the creation in the test
Only write tests for critical and important code paths
Avoid tests that merely verify framework/ActiveRecord behavior
Test boundaries correctly: for queries, assert returned values; for commands, assert collaborators are called with correct params
Never test the implementation details of one class in another class’s test suite
Use the mocha gem for stubs and mocks
Prefer OpenStruct for mock instances; use a mock class for complex cases
Only mock what’s necessary; don’t mock return values unless they are under test
Files:
test/system/settings/ai_prompts_test.rb
**/test/**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
**/test/**/*.rb: ALWAYS use Minitest and fixtures for Ruby tests; NEVER RSpec or factories.
Only test critical and important code paths; system tests should be used sparingly for critical user flows.
Files:
test/system/settings/ai_prompts_test.rb
test/system/**/*_test.rb
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
Use system tests sparingly to keep the suite fast
Files:
test/system/settings/ai_prompts_test.rb
🧬 Code graph analysis (1)
test/system/settings/ai_prompts_test.rb (1)
test/application_system_test_case.rb (1)
login_as(24-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci / test
🔇 Additional comments (4)
app/controllers/users_controller.rb (2)
20-31: LGTM! Clean state tracking for AI toggle.The implementation correctly captures the previous AI state before update and provides appropriate notices for both enable and disable actions.
72-73: LGTM! Consistent redirect handling.The new redirect case follows the existing pattern and properly supports the AI prompts settings workflow.
app/views/settings/ai_prompts/show.html.erb (1)
10-10: Verify if disabling Turbo is necessary.Disabling Turbo causes a full page reload. Since the action redirects back to the same page, Turbo should handle this correctly by default. Is there a specific reason for disabling Turbo here, or can this line be removed to leverage Turbo's navigation?
test/system/settings/ai_prompts_test.rb (1)
1-19: LGTM! Well-structured system test.The test correctly follows Minitest conventions, uses fixtures as required, and appropriately tests the critical user flow for disabling the AI assistant. The assertions verify both the navigation and the database state change.
* Test disabling AI assistant * Leave "Maybe" out of it * It's "sure" now
Summary
Testing
npm run lintnpm run formatbin/rubocopbin/rails test(fails: Please check your database configuration and ensure there is a valid connection to your database.)https://chatgpt.com/codex/tasks/task_e_68b11c384c348332b707a554b9d85880
Summary by CodeRabbit