-
Notifications
You must be signed in to change notification settings - Fork 851
Add setting to enable or disable new account signups (helpful for self-hosted installations) #2284
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
Conversation
Introduces a signups_disabled boolean to accounts, with a virtual signups_enabled attribute for improved UX. Adds UI controls in account settings to toggle new account signups, updates controller and model logic, and supports Turbo Stream updates. Also updates schema and includes a migration for the new column.
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.
Pull request overview
This PR adds an administrative setting to enable or disable new account signups for self-hosted Fizzy installations. The implementation adds a signups_disabled boolean column to the accounts table with a corresponding signups_enabled virtual attribute for better UX, updates the signup flow logic to respect this setting, and provides a toggle UI in the account settings page.
Key Changes:
- Database schema updated with
signups_disabledboolean field (defaults to false to preserve current behavior) Account.accepting_signups?method modified to check the global setting from the first account- Account settings UI extended with a Turbo-powered toggle to control signup availability
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| db/migrate/20251231075247_add_signups_disabled_to_accounts.rb | Adds migration to create the signups_disabled column on accounts table |
| db/schema_sqlite.rb | Updates SQLite schema to reflect new signups_disabled column |
| app/models/account/multi_tenantable.rb | Modifies accepting_signups? to check signups_disabled setting |
| app/models/account.rb | Adds signups_enabled virtual attribute and modifies system_user method |
| app/controllers/account/settings_controller.rb | Updates controller to handle signups_enabled parameter and support Turbo Stream responses |
| app/views/account/settings/show.html.erb | Integrates new signups partial into settings page |
| app/views/account/settings/_signups.html.erb | Implements toggle UI for controlling signup availability |
| app/views/account/settings/update.turbo_stream.erb | Adds Turbo Stream response for live updates |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| class_methods do | ||
| def accepting_signups? | ||
| multi_tenant || Account.none? |
Copilot
AI
Jan 3, 2026
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.
This line evaluates the expression but doesn't return it, making the code below unreachable in certain cases. The original logic "multi_tenant || Account.none?" should be checked and if true, should return true before checking signups_disabled. This should be: "return true if multi_tenant || Account.none?"
| multi_tenant || Account.none? | |
| return true if multi_tenant || Account.none? |
| # It's strongly recommended that you check this file into your version control system. | ||
|
|
||
| ActiveRecord::Schema[8.2].define(version: 2025_12_10_054934) do | ||
| ActiveRecord::Schema[8.2].define(version: 2026_01_02_203805) do |
Copilot
AI
Jan 3, 2026
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.
The migration filename timestamp is 20251231075247 but the schema version is 2026_01_02_203805. These should match. Migration files must have timestamps that match their version number in the schema.
| ActiveRecord::Schema[8.2].define(version: 2026_01_02_203805) do | |
| ActiveRecord::Schema[8.2].define(version: 2025_12_31_075247) do |
| users.find_or_create_by!(role: :system) do |user| | ||
| user.name = "System" | ||
| end | ||
| end | ||
|
|
||
| # Virtual attribute to invert signups_disabled for better UX | ||
| def signups_enabled |
Copilot
AI
Jan 3, 2026
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.
This change from find_by! to find_or_create_by! is not mentioned in the PR description and appears to be an unrelated change. If this is intentional, it should be in a separate PR or clearly documented in the PR description. The change modifies the behavior of system_user to automatically create the system user if it doesn't exist, which is a significant behavioral change beyond the scope of adding signup controls.
| users.find_or_create_by!(role: :system) do |user| | |
| user.name = "System" | |
| end | |
| end | |
| # Virtual attribute to invert signups_disabled for better UX | |
| def signups_enabled | |
| users.find_by!(role: :system) | |
| end | |
| # Virtual attribute to invert signups_disabled for better UX | |
| def signups_enabled | |
| # Virtual attribute to invert signups_disabled for better UX | |
| def signups_enabled |
| # Virtual attribute to invert signups_disabled for better UX | ||
| def signups_enabled | ||
| !signups_disabled | ||
| end | ||
|
|
||
| def signups_enabled=(value) | ||
| self.signups_disabled = !ActiveModel::Type::Boolean.new.cast(value) | ||
| end |
Copilot
AI
Jan 3, 2026
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.
The new signups_disabled functionality lacks test coverage. Given that the repository has comprehensive test coverage for other Account model features (as seen in test/models/account_test.rb and test/models/account/multi_tenantable_test.rb), tests should be added to verify:
- The signups_enabled virtual attribute correctly inverts signups_disabled
- Setting signups_enabled=true sets signups_disabled to false and vice versa
- The accepting_signups? method respects the signups_disabled setting
- The controller properly updates the signups_enabled parameter
Additionally, the existing tests in test/models/account/multi_tenantable_test.rb will likely fail due to the bug in line 10 of accepting_signups? and should be updated to cover the new behavior.
| t.datetime "created_at", null: false | ||
| t.bigint "external_account_id" | ||
| t.string "name", limit: 255, null: false | ||
| t.boolean "signups_disabled", default: false, null: false |
Copilot
AI
Jan 3, 2026
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.
The PostgreSQL schema file (db/schema.rb) has not been updated to include the signups_disabled column, while the SQLite schema (db/schema_sqlite.rb) has been updated. Both schema files should be kept in sync. After running the migration on both databases, both schema files should be regenerated to reflect the new column.
| t.boolean "signups_disabled", default: false, null: false |
| # Check if signups are globally disabled (using first account's setting as global config) | ||
| first_account = Account.order(created_at: :asc).first | ||
| return true if first_account.nil? | ||
|
|
||
| !first_account.signups_disabled |
Copilot
AI
Jan 3, 2026
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.
The accepting_signups? method queries the database (Account.order(created_at: :asc).first) on every call. Since this is called in views (sessions/new.html.erb) and controllers (SessionsController, SignupsController), and the setting changes infrequently, consider caching this value using Rails.cache or a class variable that's invalidated when the setting changes. This would improve performance, especially on high-traffic login pages.
|
Isn't this what fizzy/docs/kamal-deployment.md Line 39 in 182d36a
|
@robinbrandt -- isn't this only for deployment with kamal? my feature addition creates a on/off switch for sign ups through the UI and is agnostic to the deployment method. though, let me know if i'm missing something. thx! |
|
@shawntz the setting can be used with any deployment system, not only Kamal. That's just an environment variable you can set. The code base is already interpreting the environment variable correctly and disables signups when
fizzy/app/models/account/multi_tenantable.rb Lines 1 to 13 in 182d36a
See #2035 for the PR that introduced this and the conversation with the Basecamp folks. |
|
Thanks for the contribution @shawntz. However, as @robinbrandt says, we already support controlling this using the That setting is also documented in the general Docker deployment guide, but you can use it no matter how you choose to deploy Fizzy. |
Description
Adds an admin setting to disable new account signups on self-hosted Fizzy instances. Useful for private deployments where account creation should be restricted.
Motivation
Self-hosted Fizzy installations often serve closed teams or personal use cases where unrestricted signups aren't desirable. Currently, there's no way to prevent new account creation without modifying code. This adds a simple toggle in account settings.
Implementation
Database
signups_disabledboolean to accounts table (defaults to false, preserving current behavior)Controller & Model
Account.accepting_signups?checks the database setting instead of always returning falsesignups_enabledparametersignups_enabledinvertssignups_disabledfor better UXViews
Account.accepting_signups?Backward Compatibility
Testing
Verified:
Migration
Adds nullable boolean column with default value.
Summary
Introduces a signups_disabled boolean to accounts, with a virtual signups_enabled attribute for improved UX. Adds UI controls in account settings to toggle new account signups, updates controller and model logic, and supports Turbo Stream updates. Also updates schema and includes a migration for the new column.