From 7fdc205f2511ce2f27eafca3ff9e166702e53781 Mon Sep 17 00:00:00 2001 From: Guillem Arias Date: Fri, 22 May 2026 15:01:57 +0200 Subject: [PATCH] feat(modules): v1 feature-module gating + Investments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Exploration spike following expert-panel synthesis. Codifies the 3-tier gating model (instance / per-user / per-family) without adding a framework. Investments is the first family-scoped module. - Family#disabled_modules: string[] column, opt-out semantics. No per-module DB column. Existing families default to [] = enabled. - Family::AVAILABLE_MODULES = %w[investments] (the registry). - ModuleGateable concern (auto-included in ApplicationController): require_module! class macro + module_enabled? helper. - Api::V1::BaseController#require_module!: 403 feature_disabled JSON, mirrors require_ai_enabled. - NavigationHelper extracts mobile/desktop nav into a single source with module: key support; mobile shrinks via justify-around, never auto-fills empty slots. - Settings → Preferences gains a family-scoped module toggle card. - 4 HTML controllers (Investments, Holdings, Trades, Securities) + 3 API controllers gated. Investment/Crypto account types hidden in the new-account modal when off. - docs/feature-gating.md codifies the rule for future modules. Background-job layer not wired (no Investments-specific scheduled job to gate; flagged as TODO in docs). Run db:migrate before bin/rails test. No PR yet — awaiting decisions in open-questions list. --- app/controllers/api/v1/base_controller.rb | 8 ++ app/controllers/api/v1/holdings_controller.rb | 1 + .../api/v1/securities_controller.rb | 1 + app/controllers/api/v1/trades_controller.rb | 1 + app/controllers/application_controller.rb | 2 +- app/controllers/concerns/module_gateable.rb | 25 ++++ app/controllers/holdings_controller.rb | 2 + app/controllers/investments_controller.rb | 2 + app/controllers/securities_controller.rb | 2 + .../settings/preferences_controller.rb | 25 +++- app/controllers/trades_controller.rb | 2 + app/helpers/navigation_helper.rb | 31 +++++ app/models/family.rb | 5 + app/views/accounts/new.html.erb | 6 +- app/views/layouts/application.html.erb | 18 +-- app/views/settings/preferences/show.html.erb | 31 +++++ config/locales/views/modules/en.yml | 6 + config/locales/views/settings/en.yml | 3 + ...120000_add_disabled_modules_to_families.rb | 5 + docs/feature-gating.md | 109 ++++++++++++++++++ test/models/family_test.rb | 19 +++ 21 files changed, 281 insertions(+), 23 deletions(-) create mode 100644 app/controllers/concerns/module_gateable.rb create mode 100644 app/helpers/navigation_helper.rb create mode 100644 config/locales/views/modules/en.yml create mode 100644 db/migrate/20260522120000_add_disabled_modules_to_families.rb create mode 100644 docs/feature-gating.md diff --git a/app/controllers/api/v1/base_controller.rb b/app/controllers/api/v1/base_controller.rb index f1631ec1c..1a8dff77f 100644 --- a/app/controllers/api/v1/base_controller.rb +++ b/app/controllers/api/v1/base_controller.rb @@ -335,4 +335,12 @@ class Api::V1::BaseController < ApplicationController render_json({ error: "feature_disabled", message: "AI features are not enabled for this user" }, status: :forbidden) end end + + # Check if a Family feature module is enabled. Mirrors `require_ai_enabled`. + # See docs/feature-gating.md for the three-tier gating model. + def require_module!(name) + family = Current.family || current_resource_owner&.family + return if family&.module_enabled?(name) + render_json({ error: "feature_disabled", message: "Feature module '#{name}' is disabled for this family" }, status: :forbidden) + end end diff --git a/app/controllers/api/v1/holdings_controller.rb b/app/controllers/api/v1/holdings_controller.rb index 7f08dfe3b..4be295c27 100644 --- a/app/controllers/api/v1/holdings_controller.rb +++ b/app/controllers/api/v1/holdings_controller.rb @@ -3,6 +3,7 @@ class Api::V1::HoldingsController < Api::V1::BaseController include Pagy::Backend + before_action -> { require_module!(:investments) } before_action :ensure_read_scope before_action :set_holding, only: [ :show ] diff --git a/app/controllers/api/v1/securities_controller.rb b/app/controllers/api/v1/securities_controller.rb index bc0715d10..f53845408 100644 --- a/app/controllers/api/v1/securities_controller.rb +++ b/app/controllers/api/v1/securities_controller.rb @@ -4,6 +4,7 @@ class Api::V1::SecuritiesController < Api::V1::BaseController include Pagy::Backend include Api::V1::SecurityResourceFiltering + before_action -> { require_module!(:investments) } before_action :ensure_read_scope before_action :set_security, only: :show diff --git a/app/controllers/api/v1/trades_controller.rb b/app/controllers/api/v1/trades_controller.rb index 0538fe321..33638ef6b 100644 --- a/app/controllers/api/v1/trades_controller.rb +++ b/app/controllers/api/v1/trades_controller.rb @@ -3,6 +3,7 @@ class Api::V1::TradesController < Api::V1::BaseController include Pagy::Backend + before_action -> { require_module!(:investments) } before_action :ensure_read_scope, only: [ :index, :show ] before_action :ensure_write_scope, only: [ :create, :update, :destroy ] before_action :set_trade, only: [ :show, :update, :destroy ] diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 7306d504f..c663df392 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -2,7 +2,7 @@ class ApplicationController < ActionController::Base include RestoreLayoutPreferences, Onboardable, Localize, AutoSync, Authentication, Invitable, SelfHostable, StoreLocation, Impersonatable, Breadcrumbable, FeatureGuardable, Notifiable, SafePagination, AccountAuthorizable, - PreviewGateable + PreviewGateable, ModuleGateable include Pundit::Authorization include Pagy::Backend diff --git a/app/controllers/concerns/module_gateable.rb b/app/controllers/concerns/module_gateable.rb new file mode 100644 index 000000000..347b5e3a0 --- /dev/null +++ b/app/controllers/concerns/module_gateable.rb @@ -0,0 +1,25 @@ +module ModuleGateable + extend ActiveSupport::Concern + + included do + helper_method :module_enabled? + end + + class_methods do + def require_module!(name) + before_action -> { enforce_module!(name) } + end + end + + def module_enabled?(name) + family = Current.family + return true if family.nil? + family.module_enabled?(name) + end + + private + def enforce_module!(name) + return if module_enabled?(name) + redirect_to root_path, alert: I18n.t("modules.not_enabled") + end +end diff --git a/app/controllers/holdings_controller.rb b/app/controllers/holdings_controller.rb index 45a89f938..158b6d717 100644 --- a/app/controllers/holdings_controller.rb +++ b/app/controllers/holdings_controller.rb @@ -1,6 +1,8 @@ class HoldingsController < ApplicationController include StreamExtensions + require_module! :investments + before_action :set_holding, only: %i[show update destroy unlock_cost_basis remap_security reset_security sync_prices] before_action :require_holding_write_permission!, only: %i[update destroy unlock_cost_basis remap_security reset_security sync_prices] diff --git a/app/controllers/investments_controller.rb b/app/controllers/investments_controller.rb index 5fa25f123..8615e58f8 100644 --- a/app/controllers/investments_controller.rb +++ b/app/controllers/investments_controller.rb @@ -1,5 +1,7 @@ class InvestmentsController < ApplicationController include AccountableResource + require_module! :investments + permitted_accountable_attributes :id, :subtype end diff --git a/app/controllers/securities_controller.rb b/app/controllers/securities_controller.rb index f369c33c5..ec5c98f98 100644 --- a/app/controllers/securities_controller.rb +++ b/app/controllers/securities_controller.rb @@ -1,4 +1,6 @@ class SecuritiesController < ApplicationController + require_module! :investments + def index @securities = Security.search_provider( params[:q], diff --git a/app/controllers/settings/preferences_controller.rb b/app/controllers/settings/preferences_controller.rb index 5798c573e..5863a3f18 100644 --- a/app/controllers/settings/preferences_controller.rb +++ b/app/controllers/settings/preferences_controller.rb @@ -3,17 +3,18 @@ class Settings::PreferencesController < ApplicationController def show @user = Current.user + @family = Current.family end # Writes per-user boolean preferences stored in the JSONB `users.preferences` - # column. Mirrors Settings::AppearancesController#update so the toggle card on - # the Preferences page can submit directly without going through the broader - # UsersController#update flow (which expects a full user form payload). + # column, plus per-family module toggles in `families.disabled_modules`. The + # auto-submit pattern matches Settings::AppearancesController#update. def update @user = Current.user user_params = params.permit(user: [ :preview_features_enabled ]).fetch(:user, {}) + module_params = params.permit(family: { modules: {} }).dig(:family, :modules) - @user.transaction do + ActiveRecord::Base.transaction do @user.lock! updated_prefs = (@user.preferences || {}).deep_dup if user_params.key?(:preview_features_enabled) @@ -21,6 +22,22 @@ class Settings::PreferencesController < ApplicationController ActiveModel::Type::Boolean.new.cast(user_params[:preview_features_enabled]) end @user.update!(preferences: updated_prefs) + + if module_params.present? + family = Current.family + family.lock! + disabled = Array(family.disabled_modules).map(&:to_s) + module_params.each do |name, enabled| + name = name.to_s + next unless Family::AVAILABLE_MODULES.include?(name) + if ActiveModel::Type::Boolean.new.cast(enabled) + disabled.delete(name) + else + disabled << name unless disabled.include?(name) + end + end + family.update!(disabled_modules: disabled) + end end redirect_to settings_preferences_path end diff --git a/app/controllers/trades_controller.rb b/app/controllers/trades_controller.rb index e3a90121e..e7da4e657 100644 --- a/app/controllers/trades_controller.rb +++ b/app/controllers/trades_controller.rb @@ -1,6 +1,8 @@ class TradesController < ApplicationController include EntryableResource + require_module! :investments + before_action :set_entry_for_unlock, only: :unlock # Defaults to a buy trade diff --git a/app/helpers/navigation_helper.rb b/app/helpers/navigation_helper.rb new file mode 100644 index 000000000..a2b5c7e2d --- /dev/null +++ b/app/helpers/navigation_helper.rb @@ -0,0 +1,31 @@ +module NavigationHelper + def main_nav_items(intro_mode:) + return intro_nav_items if intro_mode + + items = [ + { name: t("layouts.application.nav.home"), path: root_path, icon: "pie-chart", icon_custom: false, active: page_active?(root_path) }, + { name: t("layouts.application.nav.transactions"), path: transactions_path, icon: "credit-card", icon_custom: false, active: page_active?(transactions_path) }, + { name: t("layouts.application.nav.reports"), path: reports_path, icon: "chart-bar", icon_custom: false, active: page_active?(reports_path) }, + { name: t("layouts.application.nav.budgets"), path: budgets_path, icon: "map", icon_custom: false, active: page_active?(budgets_path) }, + { name: t("layouts.application.nav.assistant"), path: chats_path, icon: "icon-assistant", icon_custom: true, active: page_active?(chats_path), mobile_only: true } + ] + + items.reject { |item| item[:module] && !module_enabled?(item[:module]) } + end + + def mobile_nav_items(intro_mode:) + main_nav_items(intro_mode: intro_mode) + end + + def desktop_nav_items(intro_mode:) + main_nav_items(intro_mode: intro_mode).reject { |item| item[:mobile_only] } + end + + private + def intro_nav_items + [ + { name: t("layouts.application.nav.home"), path: chats_path, icon: "home", icon_custom: false, active: page_active?(chats_path) }, + { name: "Intro", path: intro_path, icon: "sparkles", icon_custom: false, active: page_active?(intro_path) } + ] + end +end diff --git a/app/models/family.rb b/app/models/family.rb index 3ebd72406..c652c682a 100644 --- a/app/models/family.rb +++ b/app/models/family.rb @@ -21,6 +21,7 @@ class Family < ApplicationRecord MONIKERS = [ "Family", "Group" ].freeze ASSISTANT_TYPES = %w[builtin external].freeze SHARING_DEFAULTS = %w[shared private].freeze + AVAILABLE_MODULES = %w[investments].freeze has_many :users, dependent: :destroy has_many :accounts, dependent: :destroy @@ -55,6 +56,10 @@ class Family < ApplicationRecord before_validation :normalize_enabled_currencies! + def module_enabled?(name) + Array(disabled_modules).exclude?(name.to_s) + end + def primary_currency_code normalize_currency_code(currency) || "USD" end diff --git a/app/views/accounts/new.html.erb b/app/views/accounts/new.html.erb index 7caa4dda3..07939edea 100644 --- a/app/views/accounts/new.html.erb +++ b/app/views/accounts/new.html.erb @@ -5,8 +5,10 @@ <% end %>> <% unless params[:classification] == "liability" %> <%= render "account_type", accountable: Depository.new %> - <%= render "account_type", accountable: Investment.new %> - <%= render "account_type", accountable: Crypto.new %> + <% if module_enabled?(:investments) %> + <%= render "account_type", accountable: Investment.new %> + <%= render "account_type", accountable: Crypto.new %> + <% end %> <%= render "account_type", accountable: Property.new %> <%= render "account_type", accountable: Vehicle.new %> <% end %> diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index cffcc2c7f..bbbecc09a 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -1,21 +1,7 @@ <% intro_mode = Current.user&.ui_layout_intro? %> <% home_path = intro_mode ? chats_path : root_path %> -<% mobile_nav_items = if intro_mode - [ - { name: t(".nav.home"), path: chats_path, icon: "home", icon_custom: false, active: page_active?(chats_path) }, - { name: "Intro", path: intro_path, icon: "sparkles", icon_custom: false, active: page_active?(intro_path) } - ] -else - [ - { name: t(".nav.home"), path: root_path, icon: "pie-chart", icon_custom: false, active: page_active?(root_path) }, - { name: t(".nav.transactions"), path: transactions_path, icon: "credit-card", icon_custom: false, active: page_active?(transactions_path) }, - { name: t(".nav.reports"), path: reports_path, icon: "chart-bar", icon_custom: false, active: page_active?(reports_path) }, - { name: t(".nav.budgets"), path: budgets_path, icon: "map", icon_custom: false, active: page_active?(budgets_path) }, - { name: t(".nav.assistant"), path: chats_path, icon: "icon-assistant", icon_custom: true, active: page_active?(chats_path), mobile_only: true } - ] -end %> - -<% desktop_nav_items = mobile_nav_items.reject { |item| item[:mobile_only] } %> +<% mobile_nav_items = mobile_nav_items(intro_mode: intro_mode) %> +<% desktop_nav_items = desktop_nav_items(intro_mode: intro_mode) %> <% expanded_sidebar_class = "w-full" %> <% collapsed_sidebar_class = "w-0 overflow-hidden" %> diff --git a/app/views/settings/preferences/show.html.erb b/app/views/settings/preferences/show.html.erb index 80b767bd1..c1f285051 100644 --- a/app/views/settings/preferences/show.html.erb +++ b/app/views/settings/preferences/show.html.erb @@ -227,3 +227,34 @@ <% end %> + +<%# Family feature modules. One toggle per entry in Family::AVAILABLE_MODULES. + Stored as opt-out strings in families.disabled_modules; default state is + enabled so existing families see no change. See docs/feature-gating.md. %> +
+
+

<%= t(".modules.title") %>

+

<%= t(".modules.description") %>

+
+ + <%= form_with url: settings_preferences_path, method: :patch, + data: { controller: "auto-submit-form" } do |f| %> +
+ <% Family::AVAILABLE_MODULES.each do |module_name| %> + + <% end %> +
+ <% end %> +
diff --git a/config/locales/views/modules/en.yml b/config/locales/views/modules/en.yml new file mode 100644 index 000000000..0b974b94e --- /dev/null +++ b/config/locales/views/modules/en.yml @@ -0,0 +1,6 @@ +en: + modules: + not_enabled: This feature module is disabled. Re-enable it in Settings → Preferences. + investments: + title: Investments + description: Track holdings, trades, and investment/crypto accounts. Disabling hides the entire investments surface; existing data is preserved. diff --git a/config/locales/views/settings/en.yml b/config/locales/views/settings/en.yml index e7abc768f..1aac2db39 100644 --- a/config/locales/views/settings/en.yml +++ b/config/locales/views/settings/en.yml @@ -157,6 +157,9 @@ en: preview: title: Enable preview features description: Opt in to in-progress features tagged preview or canary. + modules: + title: Feature modules + description: Turn whole feature verticals on or off. Existing data is preserved; toggling back on restores everything. profiles: destroy: cannot_remove_self: You cannot remove yourself from the account. diff --git a/db/migrate/20260522120000_add_disabled_modules_to_families.rb b/db/migrate/20260522120000_add_disabled_modules_to_families.rb new file mode 100644 index 000000000..8c72b3e5d --- /dev/null +++ b/db/migrate/20260522120000_add_disabled_modules_to_families.rb @@ -0,0 +1,5 @@ +class AddDisabledModulesToFamilies < ActiveRecord::Migration[7.2] + def change + add_column :families, :disabled_modules, :string, array: true, null: false, default: [] + end +end diff --git a/docs/feature-gating.md b/docs/feature-gating.md new file mode 100644 index 000000000..d793608ee --- /dev/null +++ b/docs/feature-gating.md @@ -0,0 +1,109 @@ +# Feature Gating + +Sure already has three layered mechanisms for turning features on and off. Use them. Do not introduce a new framework (no Flipper, no plugin loader, no `app/modules/*/` autoload tree). + +## The three gating tiers + +| Tier | Where | When to use | Precedent | +|------|-------|-------------|-----------| +| **Instance** | `Rails.application.config.app_mode` (`config/application.rb`) + env vars like `SIMPLEFIN_INCLUDE_PENDING` | Whole-deploy switches: managed vs self-hosted, opt-in integrations that require credentials. | `app_mode.self_hosted?`, `Rails.configuration.x.simplefin.*` | +| **Per-user** | `User#preferences` jsonb | Personal preferences that affect only that user's UI. | `User#preview_features_enabled?` (`app/models/user.rb`), `User#ai_enabled?` | +| **Per-family ("modules")** | `Family#disabled_modules` text[] + `Family#module_enabled?(:name)` | Toggling whole feature verticals (Investments, future: Budgets, Goals, AI). Affects everyone in the family. | `Family#recurring_transactions_disabled?` (existing), `Family#module_enabled?(:investments)` (this doc) | + +## The "module" rule + +A module is just a string in `families.disabled_modules`. It opts a feature vertical OUT — default is enabled, presence in the array means disabled. Backwards-compatible: existing families ship with `[]` and see no change. + +```ruby +family.module_enabled?(:investments) # => true (default) +family.update!(disabled_modules: ["investments"]) +family.module_enabled?(:investments) # => false +``` + +No migrations per module. Add an entry to `Family::AVAILABLE_MODULES` and wire the three enforcement layers below. + +## Three enforcement layers — required for every module + +The biggest risk with feature gating is **half-enforcement**: hiding the UI but leaving controllers, jobs, or model writes open. A user disables the module, the surface disappears, background jobs keep writing data, and re-enabling produces surprise state. **For every module, all three layers must call `Family#module_enabled?`.** + +### 1. View / nav + +Inside ERB and helpers, call `module_enabled?(:investments)`. The method is provided as a `helper_method` by `ModuleGateable` (auto-included in `ApplicationController`). + +```erb +<% if module_enabled?(:investments) %> + <%= render "account_type", accountable: Investment.new %> + <%= render "account_type", accountable: Crypto.new %> +<% end %> +``` + +For nav items, opt in via the `module:` key in `NavigationHelper#main_nav_items`; disabled modules are filtered out and the bottom-mobile nav redistributes via `justify-around`. **Never auto-fill empty slots** with promoted sidebar items — empty slot is the affordance that the module is off. + +### 2. Controller + +HTML controllers use the `require_module!` class macro from `ModuleGateable`: + +```ruby +class InvestmentsController < ApplicationController + require_module! :investments +end +``` + +This redirects to `root_path` with `flash[:alert] = t("modules.not_enabled")` when the module is off. + +API controllers (under `Api::V1`) use the instance method on `Api::V1::BaseController`: + +```ruby +class Api::V1::HoldingsController < Api::V1::BaseController + before_action -> { require_module!(:investments) } +end +``` + +This renders `{ error: "feature_disabled" }` with status `403`. Matches the existing `require_ai_enabled` pattern. + +### 3. Background jobs / model writes + +If the module has scheduled jobs or model callbacks that write data, guard the entry point: + +```ruby +class SomeInvestmentJob < ApplicationJob + def perform(family_id) + family = Family.find(family_id) + return unless family.module_enabled?(:investments) + # ... + end +end +``` + +Without this, toggling off creates silent data accumulation, and toggling back on produces a populated module the user never approved. + +## Choosing the right tier + +- **Instance**: needs a deploy/restart. Right for integrations bound to env keys (Plaid, SimpleFIN, OpenAI), and for self-hosters who want a feature off at the docker-compose layer. +- **Per-user**: right for opt-in/opt-out personal preferences that don't affect family-shared data (AI sidebar visibility, preview features, dark mode). +- **Per-family module**: right for "we don't want this vertical at all" — affects all family members, the toggle lives in Settings → Preferences, and surfaces across web + API + jobs. + +If you find yourself wanting all three for one feature, you probably want per-family with a `Rails.configuration` instance kill-switch. Don't build "the module system" — there isn't one. + +## Naming and conventions + +- Module names are snake_case strings: `"investments"`, `"goals"`. Plural for verticals, singular for single features. +- Add to `Family::AVAILABLE_MODULES` so the Settings UI picks them up. +- Add locale entries under `modules..title` and `modules..description` in `config/locales/views/modules/.yml`. +- Same string is used in `disabled_modules` array, controller `require_module!` arg, view `module_enabled?` arg, and locale key. + +## Not modules + +These are foundational primitives. They are not candidates for module gating because too much else FKs into them: + +- Account, Transaction, Entry, Trade, Valuation, Balance, Holding, Security +- Category, Merchant, Transfer + +If you think one of these should be a module, you are wrong; redesign the feature instead. + +## What about a "modules registry"? + +Don't build one. Two reasons: + +1. You don't have three modules yet, and premature abstraction here is fatal because the surface is forever (per Discourse/WordPress prior art). +2. `Family::AVAILABLE_MODULES` is the registry. It's an array. That's the framework. diff --git a/test/models/family_test.rb b/test/models/family_test.rb index f026ff6ab..0f9a9f736 100644 --- a/test/models/family_test.rb +++ b/test/models/family_test.rb @@ -234,4 +234,23 @@ class FamilyTest < ActiveSupport::TestCase assert_equal({ "type" => "financial_document" }, document.metadata) assert_equal "vs_test123", family.reload.vector_store_id end + + test "module_enabled? defaults to true and toggles via disabled_modules" do + family = families(:dylan_family) + + assert family.module_enabled?(:investments) + assert family.module_enabled?("investments") + + family.update!(disabled_modules: [ "investments" ]) + assert_not family.module_enabled?(:investments) + assert_not family.module_enabled?("investments") + + family.update!(disabled_modules: []) + assert family.module_enabled?(:investments) + end + + test "module_enabled? returns true for unknown modules (default-enabled)" do + family = families(:dylan_family) + assert family.module_enabled?(:nonexistent_module) + end end