First cut of a simplified "intro" UI layout #265
Conversation
WalkthroughIntroduces a new UI layout system with "intro" and "dashboard" modes on the User model. Adds enum, database persistence, controller/view conditionals for layout switching, AI configuration customization for intro mode, and supporting database migration. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Suggested labels
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
app/views/pages/intro.html.erb (1)
1-6: Consider using margin instead of<br/>.While functional, the
<br/>at Line 4 could be replaced with a margin utility class on the heading for more semantic spacing control.Apply this diff if you prefer utility-based spacing:
<% content_for :page_header do %> - <div class="space-y-2"> - <h1 class="text-2xl font-semibold text-primary">Welcome!</h1> - <br/> + <div> + <h1 class="text-2xl font-semibold text-primary mb-4">Welcome!</h1> </div> <% end %>app/views/layouts/application.html.erb (1)
129-142: Consider minimal navigation in intro mode.While hiding the full breadcrumbs and panel controls simplifies the intro experience, users may benefit from some minimal navigation cues or a way to orient themselves.
Consider adding a simplified navigation element or progress indicator for intro mode.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
app/controllers/pages_controller.rb(2 hunks)app/controllers/settings/profiles_controller.rb(2 hunks)app/models/assistant/configurable.rb(1 hunks)app/models/user.rb(3 hunks)app/views/layouts/application.html.erb(5 hunks)app/views/pages/intro.html.erb(1 hunks)app/views/settings/profiles/show.html.erb(1 hunks)app/views/users/_user_menu.html.erb(2 hunks)config/application.rb(1 hunks)config/routes.rb(1 hunks)db/migrate/20251030140000_add_ui_layout_to_users.rb(1 hunks)db/schema.rb(3 hunks)test/controllers/settings/profiles_controller_test.rb(2 hunks)test/fixtures/chats.yml(1 hunks)test/fixtures/users.yml(1 hunks)test/models/assistant/configurable_test.rb(1 hunks)test/models/user_test.rb(1 hunks)
🧰 Additional context used
📓 Path-based instructions (25)
app/**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Place Rails application Ruby code (models, controllers, services, jobs, mailers, components) under app/
Files:
app/controllers/pages_controller.rbapp/controllers/settings/profiles_controller.rbapp/models/assistant/configurable.rbapp/models/user.rb
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby style: 2-space indentation; snake_case for methods/variables; CamelCase for classes/modules
Files:
app/controllers/pages_controller.rbtest/models/user_test.rbtest/models/assistant/configurable_test.rbapp/controllers/settings/profiles_controller.rbapp/models/assistant/configurable.rbdb/schema.rbconfig/routes.rbtest/controllers/settings/profiles_controller_test.rbapp/models/user.rbdb/migrate/20251030140000_add_ui_layout_to_users.rbconfig/application.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/pages_controller.rbtest/models/user_test.rbtest/models/assistant/configurable_test.rbapp/controllers/settings/profiles_controller.rbapp/models/assistant/configurable.rbdb/schema.rbconfig/routes.rbtest/controllers/settings/profiles_controller_test.rbapp/models/user.rbdb/migrate/20251030140000_add_ui_layout_to_users.rbconfig/application.rb
app/{models,controllers,views}/**/*.{rb,erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Avoid N+1 queries
Files:
app/controllers/pages_controller.rbapp/views/layouts/application.html.erbapp/views/pages/intro.html.erbapp/controllers/settings/profiles_controller.rbapp/models/assistant/configurable.rbapp/models/user.rbapp/views/users/_user_menu.html.erbapp/views/settings/profiles/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/models/user_test.rbtest/models/assistant/configurable_test.rbtest/controllers/settings/profiles_controller_test.rb
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/layouts/application.html.erbapp/views/pages/intro.html.erbapp/views/users/_user_menu.html.erbapp/views/settings/profiles/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/layouts/application.html.erbapp/views/pages/intro.html.erbapp/views/users/_user_menu.html.erbapp/views/settings/profiles/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/layouts/application.html.erbapp/views/pages/intro.html.erbapp/views/users/_user_menu.html.erbapp/views/settings/profiles/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/layouts/application.html.erbapp/views/pages/intro.html.erbapp/views/users/_user_menu.html.erbapp/views/settings/profiles/show.html.erb
app/views/layouts/application.html.erb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Use Turbo frames in the application layout to load controller actions as demonstrated
Files:
app/views/layouts/application.html.erb
app/views/layouts/**/*.erb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Be mindful of performance in global layouts (e.g., avoid loading large data payloads)
Files:
app/views/layouts/application.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/layouts/application.html.erbapp/views/pages/intro.html.erbapp/views/users/_user_menu.html.erbapp/views/settings/profiles/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/layouts/application.html.erbapp/views/pages/intro.html.erbapp/views/users/_user_menu.html.erbapp/views/settings/profiles/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/layouts/application.html.erbapp/views/pages/intro.html.erbapp/views/users/_user_menu.html.erbapp/views/settings/profiles/show.html.erb
{app/views/**,app/helpers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
Always generate semantic HTML
Files:
app/views/layouts/application.html.erbapp/views/pages/intro.html.erbapp/views/users/_user_menu.html.erbapp/views/settings/profiles/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/layouts/application.html.erbapp/views/pages/intro.html.erbapp/views/users/_user_menu.html.erbapp/views/settings/profiles/show.html.erb
app/models/**/*.rb
📄 CodeRabbit inference engine (.cursor/rules/project-design.mdc)
Domain models should not call Provider::Registry directly; use a Provided concern within the model’s namespace to select providers and expose convenience methods
Use ActiveRecord validations for forms and complex domain constraints.
app/models/**/*.rb: Place business logic in POROs and model classes under app/models
Models should answer questions about themselves (e.g., prefer account.balance_series over service objects)
Implement complex validations and business logic with ActiveRecord validations
Model-level validations may mirror DB constraints but are not strictly required
Files:
app/models/assistant/configurable.rbapp/models/user.rb
app/{helpers,models}/**/*.rb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Format currencies, numbers, and dates on the server side (Ruby) before sending to the client
Files:
app/models/assistant/configurable.rbapp/models/user.rb
db/schema.rb
📄 CodeRabbit inference engine (.cursor/rules/project-design.mdc)
Treat db/schema.rb as the authoritative source of truth for model relationships
Files:
db/schema.rb
config/**
📄 CodeRabbit inference engine (AGENTS.md)
Store application and environment configuration under config/
Files:
config/routes.rbconfig/application.rb
test/fixtures/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Use test/fixtures for fixtures (and existing VCR cassettes if present)
Files:
test/fixtures/users.ymltest/fixtures/chats.yml
test/fixtures/**/*.yml
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Keep fixtures minimal: 2–3 per model for base cases.
test/fixtures/**/*.yml: Use fixtures for test data rather than factories
Keep fixtures minimal: 2–3 per model for base cases; create edge cases inline in tests
Files:
test/fixtures/users.ymltest/fixtures/chats.yml
app/views/**/_*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)
Name partials with a leading underscore (e.g., _trend_change.html.erb, _form_errors.html.erb)
Files:
app/views/users/_user_menu.html.erb
db/migrate/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Place database migrations under db/migrate/
Simple validations like null checks and unique constraints should be handled by database constraints in migration files.
Files:
db/migrate/20251030140000_add_ui_layout_to_users.rb
db/migrate/**/*.rb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Enforce simple validations in the database (NULL constraints, unique indexes, etc.)
Files:
db/migrate/20251030140000_add_ui_layout_to_users.rb
🧠 Learnings (18)
📚 Learning: 2025-09-23T22:21:06.637Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-09-23T22:21:06.637Z
Learning: Applies to app/views/layouts/**/*.erb : Be mindful of performance in global layouts (e.g., avoid loading large data payloads)
Applied to files:
app/views/layouts/application.html.erbapp/controllers/settings/profiles_controller.rbconfig/application.rb
📚 Learning: 2025-09-23T22:24:15.106Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:24:15.106Z
Learning: Place context-specific partials within the relevant controller view directory (e.g., accounts/_account_sidebar_tabs.html.erb)
Applied to files:
app/views/layouts/application.html.erb
📚 Learning: 2025-09-23T22:21:06.637Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-09-23T22:21:06.637Z
Learning: Applies to app/views/layouts/application.html.erb : Use Turbo frames in the application layout to load controller actions as demonstrated
Applied to files:
app/views/layouts/application.html.erbapp/controllers/settings/profiles_controller.rb
📚 Learning: 2025-09-23T22:21:06.637Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-09-23T22:21:06.637Z
Learning: Applies to app/views/**/*.html.erb : Leverage Turbo frames to break up pages instead of JS-driven client-side solutions
Applied to files:
app/views/layouts/application.html.erbapp/views/pages/intro.html.erb
📚 Learning: 2025-09-23T22:23:22.963Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:23:22.963Z
Learning: Applies to app/views/**/*.html.erb : Use partials when content is primarily static, simple, context-specific, and without variants or complex configuration
Applied to files:
app/views/layouts/application.html.erbapp/views/pages/intro.html.erb
📚 Learning: 2025-09-23T22:23:22.963Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:23:22.963Z
Learning: Applies to app/{views,components}/**/*.html.erb : Integrate Stimulus declaratively in ERB: templates declare data-controller/actions/targets; controllers respond to those declarations
Applied to files:
app/views/pages/intro.html.erb
📚 Learning: 2025-09-23T22:23:22.963Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:23:22.963Z
Learning: Applies to app/views/**/*.html.erb : Use ViewComponents when elements have complex logic/styling, are reused across contexts, need variants/slots/accessibility, or require Stimulus interactivity
Applied to files:
app/views/pages/intro.html.erb
📚 Learning: 2025-09-23T22:24:15.106Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:24:15.106Z
Learning: Applies to {app/views,app/components}/**/*.html.erb : Integrate Stimulus declaratively in ERB: templates declare data-controller/data-action; controllers respond to those declarations
Applied to files:
app/views/pages/intro.html.erb
📚 Learning: 2025-09-23T22:21:06.637Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-09-23T22:21:06.637Z
Learning: Applies to app/views/**/*.html.erb : Prefer native HTML elements (e.g., <dialog>, <details><summary>) over JS-based components
Applied to files:
app/views/pages/intro.html.erb
📚 Learning: 2025-09-23T22:24:15.106Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:24:15.106Z
Learning: Applies to app/views/**/_*.html.erb : Name partials with a leading underscore (e.g., _trend_change.html.erb, _form_errors.html.erb)
Applied to files:
app/views/pages/intro.html.erb
📚 Learning: 2025-09-23T22:21:06.637Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-09-23T22:21:06.637Z
Learning: Applies to app/{helpers,models}/**/*.rb : Format currencies, numbers, and dates on the server side (Ruby) before sending to the client
Applied to files:
app/models/assistant/configurable.rb
📚 Learning: 2025-09-16T13:15:56.406Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-design.mdc:0-0
Timestamp: 2025-09-16T13:15:56.406Z
Learning: Applies to db/schema.rb : Treat db/schema.rb as the authoritative source of truth for model relationships
Applied to files:
db/schema.rb
📚 Learning: 2025-09-16T13:17:53.155Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-16T13:17:53.155Z
Learning: Applies to db/seeds.rb : Keep seed data definitions in db/seeds.rb
Applied to files:
db/schema.rb
📚 Learning: 2025-09-23T22:21:06.637Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-09-23T22:21:06.637Z
Learning: Applies to db/migrate/**/*.rb : Enforce simple validations in the database (NULL constraints, unique indexes, etc.)
Applied to files:
db/schema.rb
📚 Learning: 2025-09-23T22:21:06.637Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-09-23T22:21:06.637Z
Learning: Applies to app/models/**/*.rb : Implement complex validations and business logic with ActiveRecord validations
Applied to files:
app/models/user.rb
📚 Learning: 2025-09-20T08:37:48.022Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-20T08:37:48.022Z
Learning: Applies to app/models/**/*.rb : Use ActiveRecord validations for forms and complex domain constraints.
Applied to files:
app/models/user.rb
📚 Learning: 2025-09-23T22:22:48.511Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-09-23T22:22:48.511Z
Learning: Applies to test/fixtures/**/*.yml : Keep fixtures minimal: 2–3 per model for base cases; create edge cases inline in tests
Applied to files:
test/fixtures/users.yml
📚 Learning: 2025-09-20T08:37:48.022Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-20T08:37:48.022Z
Learning: Applies to test/fixtures/**/*.yml : Keep fixtures minimal: 2–3 per model for base cases.
Applied to files:
test/fixtures/users.yml
🧬 Code graph analysis (6)
app/controllers/pages_controller.rb (1)
app/models/current.rb (1)
user(8-10)
test/models/user_test.rb (1)
app/models/user.rb (2)
show_ai_sidebar?(90-92)ai_enabled?(98-100)
test/models/assistant/configurable_test.rb (1)
app/models/assistant/configurable.rb (1)
config_for(5-20)
app/controllers/settings/profiles_controller.rb (4)
app/controllers/settings/hostings_controller.rb (1)
layout(1-80)app/controllers/onboardings_controller.rb (1)
layout(1-24)app/controllers/settings/preferences_controller.rb (1)
layout(1-7)app/models/current.rb (1)
user(8-10)
test/controllers/settings/profiles_controller_test.rb (1)
test/test_helper.rb (1)
sign_in(64-66)
app/models/user.rb (2)
app/controllers/settings/profiles_controller.rb (1)
layout(1-45)config/application.rb (1)
config(10-48)
⏰ 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 (25)
config/application.rb (1)
45-47: LGTM! Configuration setup is clean and well-validated.The UI layout configuration is properly structured with good defaults and validation. The use of
in?()ensures only allowed values are accepted with a safe fallback.app/models/assistant/configurable.rb (2)
9-19: LGTM! Clean conditional configuration.The branching logic appropriately separates intro onboarding (no function calls) from dashboard mode (full function access). The consistent hash structure makes this easy to consume.
23-52: LGTM! Well-structured intro instructions.The intro prompt is thoughtfully designed for onboarding with clear sections and appropriate guidance. Embedding the user's currency and date preferences at Line 49 ensures personalized formatting, and the explicit function-calling prohibition at Line 50 aligns with the empty functions array.
test/models/assistant/configurable_test.rb (2)
4-11: LGTM! Test correctly verifies dashboard configuration.The test appropriately validates that the default dashboard mode returns non-empty functions and the expected instruction content.
13-20: LGTM! Test correctly verifies intro configuration.The test appropriately validates that intro mode returns an empty functions array and intro-specific instruction content.
config/routes.rb (1)
307-307: LGTM! Clean route addition.The intro route is properly defined and maps to the new pages#intro action.
app/controllers/pages_controller.rb (2)
7-9: LGTM! Proper flow guard for intro mode.The guard correctly redirects intro layout users to the chat interface, preventing premature dashboard access during onboarding.
52-54: LGTM! Clean intro action.The action appropriately sets breadcrumbs for the intro page with sensible navigation context.
test/fixtures/chats.yml (1)
9-11: LGTM! Fixture supports intro testing.The new intro chat fixture appropriately references the intro_user and supports intro mode test scenarios.
test/models/user_test.rb (1)
156-170: LGTM! Comprehensive test of intro layout behavior.The test thoroughly validates that intro layout users have the correct UI state: collapsed sidebars and AI enabled. Creating the user inline is appropriate for testing the initialization behavior.
app/views/pages/intro.html.erb (1)
8-21: LGTM! Clean implementation with design system tokens.The view properly uses functional design tokens (
bg-container,text-primary,text-secondary) and semantic HTML. The centered card layout with clear CTA is appropriate for an onboarding placeholder.db/schema.rb (2)
28-28: Good naming improvement.The index rename to include
_idsuffix improves clarity and follows Rails naming conventions for foreign key columns.
948-948: Database-level constraints are missing; app-level protections are present.The migration adds
ui_layoutas a nullable string without database constraints, but app-level protections are in place: the model defines an enum withvalidate: true, populates existing records with "dashboard" in the migration, and enforces defaults viabefore_validation. While this approach prevents invalid data at the application layer, the project guidelines recommend enforcing simple validations at the database level (NULL constraints, default values, CHECK constraints).Consider whether aligning with the project's convention of database-level constraints is desired, or if the current app-level-only approach is acceptable for this case.
app/views/users/_user_menu.html.erb (4)
1-3: LGTM! Proper parameter handling.The intro_mode parameter is correctly declared with a default value and defensively initialized using
local_assigns.fetch.
13-17: Intro mode settings button integration looks good.The conditional settings button for intro mode is properly integrated with the design system and menu controller.
46-51: Verify Settings link behavior in non-intro mode.In non-intro mode, the Settings menu item redirects to
accounts_path(return_to: request.fullpath)rather than a settings path. This may be intentional, but it could be confusing for users expecting to go to settings.Please confirm this is the desired behavior.
54-56: Good simplification for intro mode.Hiding the Feedback menu item during intro mode appropriately simplifies the onboarding experience.
app/views/layouts/application.html.erb (5)
1-2: LGTM! Efficient intro mode setup.The intro_mode flag and home_path computation are efficient and use safe navigation appropriately.
3-15: Well-structured conditional navigation.The mobile navigation items are properly structured with clear intro/normal mode separation and appropriate active state handling.
47-50: Consistent intro mode sidebar handling.Hiding the panel-left button in intro mode is consistent with the simplified navigation approach.
52-56: Proper parameter threading.The home_path is used consistently for logo links, and intro_mode is correctly passed to the user_menu partial.
31-35: No issues found - review comment is incorrect.The mobile sidebar behavior is properly gated in intro mode. The open button (
openMobileSidebar) is already wrapped in the mobile nav's<% if intro_mode %>...<% else %>conditional and only renders when NOT in intro mode. Since both the open and close buttons are hidden in intro mode, users cannot open the sidebar and therefore cannot be trapped. The code is correct.Likely an incorrect or invalid review comment.
app/views/settings/profiles/show.html.erb (3)
28-29: Appropriate intro mode simplification.Hiding the household section during intro mode reduces complexity during onboarding.
66-113: Invitation management section looks good.The pending invitations loop is well-structured with appropriate guards for admin actions and self-hosted features.
58-61: Route verified as correctly configured and functional.The
settings_profile_path(user_id: user)route is correctly wired. The singular resource:profilewith only[:show, :destroy]maps to/settings/profile(DELETE for destroy), and the controller's destroy action correctly retrieves the user viaparams[:user_id]passed as a query parameter. The implementation is valid, though it uses an unconventional pattern for user deletion compared to typical Railsresources :usersconventions.
| <% @users.each do |user| %> | ||
| <div class="flex gap-2 mt-2 items-center bg-container p-4 shadow-border-xs rounded-lg"> | ||
| <div class="w-9 h-9 shrink-0"> | ||
| <%= render "settings/user_avatar", avatar_url: user.profile_image&.variant(:small)&.url, initials: user.initials %> | ||
| </div> | ||
| <div class="flex items-center gap-4"> | ||
| <% if self_hosted? %> | ||
| <div class="flex items-center gap-2" data-controller="clipboard"> | ||
| <p class="text-secondary text-sm"><%= t(".invitation_link") %></p> | ||
| <span data-clipboard-target="source" class="hidden"><%= accept_invitation_url(invitation.token) %></span> | ||
| <input type="text" | ||
| readonly | ||
| autocomplete="off" | ||
| value="<%= accept_invitation_url(invitation.token) %>" | ||
| class="text-sm bg-gray-50 px-2 py-1 rounded border border-secondary w-72"> | ||
| <button data-action="clipboard#copy" class="text-secondary hover:text-gray-700"> | ||
| <span data-clipboard-target="iconDefault"> | ||
| <%= icon "copy" %> | ||
| </span> | ||
| <span class="hidden" data-clipboard-target="iconSuccess"> | ||
| <%= icon "check" %> | ||
| </span> | ||
| </button> | ||
| </div> | ||
| <% end %> | ||
|
|
||
| <% if Current.user.admin? %> | ||
| <p class="text-primary font-medium text-sm"><%= user.display_name %></p> | ||
| <div class="rounded-md bg-surface px-1.5 py-0.5"> | ||
| <p class="uppercase text-secondary font-medium text-xs"><%= user.role %></p> | ||
| </div> | ||
| <% if Current.user.admin? && user != Current.user %> | ||
| <div class="ml-auto"> | ||
| <%= render DS::Button.new( | ||
| variant: "icon", | ||
| icon: "x", | ||
| href: invitation_path(invitation), | ||
| href: settings_profile_path(user_id: user), | ||
| method: :delete, | ||
| confirm: CustomConfirm.for_resource_deletion(invitation.email, high_severity: true) | ||
| confirm: CustomConfirm.for_resource_deletion(user.display_name, high_severity: true) | ||
| ) %> | ||
| <% end %> | ||
| </div> | ||
| </div> | ||
| <% end %> | ||
| </div> | ||
| <% end %> |
There was a problem hiding this comment.
🧩 Analysis chain
Verify eager loading for users loop.
The loop over @users accesses user.profile_image which could cause N+1 queries if not eager-loaded.
As per coding guidelines (avoid N+1 queries).
🏁 Script executed:
#!/bin/bash
# Check how @users is loaded in the controller
rg -A 3 "@users\s*=" app/controllers/settings/profiles_controller.rb
# Check if profile_image is eager-loaded
rg "includes.*profile_image|eager_load.*profile_image" app/controllers/settings/profiles_controller.rbLength of output: 164
🏁 Script executed:
#!/bin/bash
# Check User model to understand profile_image
rg -A 2 "profile_image" app/models/user.rb
# Check if display_name, role, initials are database columns or computed
rg -n "def (display_name|role|initials)" app/models/user.rb
# Check for other eager_load patterns in profiles controller
rg -B 5 -A 5 "def show" app/controllers/settings/profiles_controller.rbLength of output: 1358
🏁 Script executed:
#!/bin/bash
# Check if there are any related associations on User model
rg "belongs_to.*profile_image|has_one.*profile_image|has_attachment.*profile_image" app/models/user.rbLength of output: 49
🏁 Script executed:
#!/bin/bash
# Check if role is a column, method, or association
rg -n "def role|column.*role|:role" app/models/user.rb | head -20Length of output: 97
Fix N+1 query: eager-load profile_image attachment.
The @users query in app/controllers/settings/profiles_controller.rb (show action) does not eager-load the profile_image attachment. Since the view accesses user.profile_image for each user in the loop (line 47), this causes an N+1 query.
Update the query to:
@users = Current.family.users.includes(:profile_image_attachment).order(:created_at)🤖 Prompt for AI Agents
In app/views/settings/profiles/show.html.erb around lines 44 to 65, the view
causes an N+1 by calling user.profile_image inside the users loop; fix this in
the controller's show action (app/controllers/settings/profiles_controller.rb)
by updating the @users query to eager-load the attachment (add
includes(:profile_image_attachment)) and keep the existing ordering so the view
no longer triggers per-user attachment queries.
| def up | ||
| add_column :users, :ui_layout, :string, if_not_exists: true | ||
|
|
||
| MigrationUser.reset_column_information | ||
| MigrationUser.where(ui_layout: [ nil, "" ]).update_all(ui_layout: "dashboard") | ||
| end | ||
|
|
||
| def down | ||
| remove_column :users, :ui_layout | ||
| end |
There was a problem hiding this comment.
Add DB constraints for users.ui_layout.
Without a NOT NULL requirement and check constraint, rows can be persisted with ui_layout set to NULL or an unexpected value, undermining the enum and layout selection logic. Please enforce both constraints at the database layer and clean up in the down migration.
As per coding guidelines
def up
add_column :users, :ui_layout, :string, if_not_exists: true
MigrationUser.reset_column_information
MigrationUser.where(ui_layout: [ nil, "" ]).update_all(ui_layout: "dashboard")
+ change_column_null :users, :ui_layout, false
+ add_check_constraint :users, "ui_layout IN ('dashboard','intro')", name: "users_ui_layout_allowed_values"
end
def down
- remove_column :users, :ui_layout
+ remove_check_constraint :users, name: "users_ui_layout_allowed_values", if_exists: true
+ remove_column :users, :ui_layout
end
Summary
/dashboardexperience until a later timeTesting
https://chatgpt.com/codex/tasks/task_e_69037a1a51e08332a62f1949ee747e59
Summary by CodeRabbit
Release Notes