From 4e09352c09d40df866d52450b805b7a6f1b5ebdb Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 27 Nov 2025 12:57:36 +0100 Subject: [PATCH] Bring simple signup flow from the fizzy-saas gem We skip the QB code and we fill external account ids automatically on creation with a sequence See: https://github.com/basecamp/fizzy-saas/pull/7 --- Gemfile.saas | 2 +- Gemfile.saas.lock | 3 +- app/controllers/application_controller.rb | 1 - app/controllers/concerns/saas.rb | 11 -- app/controllers/sessions_controller.rb | 12 ++- .../signup/completions_controller.rb | 24 +++++ app/models/signup.rb | 100 ++++++++++++++++++ app/models/signup/account_name_generator.rb | 53 ++++++++++ app/views/sessions/menus/show.html.erb | 12 +-- app/views/signup/completions/new.html.erb | 30 ++++++ app/views/signup/new.html.erb | 28 +++++ config/routes.rb | 6 ++ ...01_create_account_external_id_sequences.rb | 2 +- db/schema.rb | 7 +- db/schema_sqlite.rb | 2 +- test/controllers/sessions_controller_test.rb | 26 +++-- .../signup/completions_controller_test.rb | 43 ++++++++ .../signup/account_name_generator_test.rb | 61 +++++++++++ test/models/signup_test.rb | 54 ++++++++++ 19 files changed, 443 insertions(+), 34 deletions(-) delete mode 100644 app/controllers/concerns/saas.rb create mode 100644 app/controllers/signup/completions_controller.rb create mode 100644 app/models/signup.rb create mode 100644 app/models/signup/account_name_generator.rb create mode 100644 app/views/signup/completions/new.html.erb create mode 100644 app/views/signup/new.html.erb create mode 100644 test/controllers/signup/completions_controller_test.rb create mode 100644 test/models/signup/account_name_generator_test.rb create mode 100644 test/models/signup_test.rb diff --git a/Gemfile.saas b/Gemfile.saas index 779c8c419..955a66303 100644 --- a/Gemfile.saas +++ b/Gemfile.saas @@ -6,6 +6,6 @@ git_source(:bc) { |repo| "https://github.com/basecamp/#{repo}" } gem "activeresource", require: "active_resource" gem "queenbee", bc: "queenbee-plugin" -gem "fizzy-saas", bc: "fizzy-saas" +gem "fizzy-saas", bc: "fizzy-saas", branch: "extract-signup" gem "rails_structured_logging", bc: "rails-structured-logging" diff --git a/Gemfile.saas.lock b/Gemfile.saas.lock index cc24a9147..b8f6dc0e7 100644 --- a/Gemfile.saas.lock +++ b/Gemfile.saas.lock @@ -1,6 +1,7 @@ GIT remote: https://github.com/basecamp/fizzy-saas - revision: 40f68d8604a1b410ba514f5793a93e9b11c00219 + revision: 3b012b9ffa7c8fdc1e4b9f5aa7d866ed4108a8ff + branch: extract-signup specs: fizzy-saas (0.1.0) queenbee diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 89b94f079..7f1cb6565 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -4,7 +4,6 @@ class ApplicationController < ActionController::Base include CurrentRequest, CurrentTimezone, SetPlatform include RequestForgeryProtection include TurboFlash, ViewTransitions - include Saas include RoutingHeaders etag { "v1" } diff --git a/app/controllers/concerns/saas.rb b/app/controllers/concerns/saas.rb deleted file mode 100644 index fd4ca8491..000000000 --- a/app/controllers/concerns/saas.rb +++ /dev/null @@ -1,11 +0,0 @@ -module Saas - extend ActiveSupport::Concern - - included do - helper_method :signups_allowed? - end - - def signups_allowed? - defined?(Signup) && defined?(saas) - end -end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 8c623b6ef..65cb443a0 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -22,10 +22,8 @@ class SessionsController < ApplicationController magic_link = identity.send_magic_link flash[:magic_link_code] = magic_link&.code if Rails.env.development? redirect_to session_magic_link_path - elsif signups_allowed? - Signup.new(email_address: email_address).create_identity - session[:return_to_after_authenticating] = saas.new_signup_completion_path - redirect_to session_magic_link_path + elsif + process_new_signup end end @@ -38,4 +36,10 @@ class SessionsController < ApplicationController def email_address params.expect(:email_address) end + + def process_new_signup + Signup.new(email_address: email_address).create_identity + session[:return_to_after_authenticating] = new_signup_completion_path + redirect_to session_magic_link_path + end end diff --git a/app/controllers/signup/completions_controller.rb b/app/controllers/signup/completions_controller.rb new file mode 100644 index 000000000..d7f09c086 --- /dev/null +++ b/app/controllers/signup/completions_controller.rb @@ -0,0 +1,24 @@ +class Signup::CompletionsController < ApplicationController + layout "public" + + disallow_account_scope + + def new + @signup = Signup.new(identity: Current.identity) + end + + def create + @signup = Signup.new(signup_params) + + if @signup.complete + redirect_to landing_url(script_name: @signup.account.slug) + else + render :new, status: :unprocessable_entity + end + end + + private + def signup_params + params.expect(signup: %i[ full_name ]).with_defaults(identity: Current.identity) + end +end diff --git a/app/models/signup.rb b/app/models/signup.rb new file mode 100644 index 000000000..230aafd0b --- /dev/null +++ b/app/models/signup.rb @@ -0,0 +1,100 @@ +class Signup + include ActiveModel::Model + include ActiveModel::Attributes + include ActiveModel::Validations + + attr_accessor :full_name, :email_address, :identity + attr_reader :account, :user + + with_options on: :completion do + validates_presence_of :full_name, :identity + end + + def initialize(...) + super + + @email_address = @identity.email_address if @identity + end + + def create_identity + @identity = Identity.find_or_create_by!(email_address: email_address) + @identity.send_magic_link + end + + def complete + if valid?(:completion) + begin + @tenant = create_tenant + create_account + true + rescue => error + destroy_account + handle_account_creation_error(error) + + errors.add(:base, "Something went wrong, and we couldn't create your account. Please give it another try.") + Rails.error.report(error, severity: :error) + Rails.logger.error error + Rails.logger.error error.backtrace.join("\n") + + false + end + else + false + end + end + + private + # Override to customize the handling of external accounts associated to the account. + def create_tenant + nil + end + + # Override to inject custom handling for account creation errors + def handle_account_creation_error(error) + end + + def create_account + @account = Account.create_with_admin_user( + account: { + external_account_id: @tenant, + name: generate_account_name + }, + owner: { + name: full_name, + identity: identity + } + ) + @user = @account.users.find_by!(role: :admin) + @account.setup_customer_template + end + + def generate_account_name + AccountNameGenerator.new(identity: identity, name: full_name).generate + end + + + def destroy_account + @account&.destroy! + + @user = nil + @account = nil + @tenant = nil + end + + def subscription_attributes + subscription = FreeV1Subscription + + {}.tap do |attributes| + attributes[:name] = subscription.to_param + attributes[:price] = subscription.price + end + end + + def request_attributes + {}.tap do |attributes| + attributes[:remote_address] = Current.ip_address + attributes[:user_agent] = Current.user_agent + attributes[:referrer] = Current.referrer + end + end +end diff --git a/app/models/signup/account_name_generator.rb b/app/models/signup/account_name_generator.rb new file mode 100644 index 000000000..a6844d3a3 --- /dev/null +++ b/app/models/signup/account_name_generator.rb @@ -0,0 +1,53 @@ +class Signup::AccountNameGenerator + SUFFIX = "Fizzy".freeze + + attr_reader :identity, :name + + def initialize(identity:, name:) + @identity = identity + @name = name + end + + def generate + next_index = current_index + 1 + + if next_index == 1 + "#{prefix} #{SUFFIX}" + else + "#{prefix} #{next_index.ordinalize} #{SUFFIX}" + end + end + + private + def current_index + existing_indices.max || 0 + end + + def existing_indices + Current.without_account do + identity.accounts.filter_map do |account| + if account.name.match?(first_account_name_regex) + 1 + elsif match = account.name.match(nth_account_name_regex) + match[1].to_i + end + end + end + end + + def first_account_name_regex + @first_account_name_regex ||= /\A#{prefix}\s+#{SUFFIX}\Z/i + end + + def nth_account_name_regex + @nth_account_name_regex ||= /\A#{prefix}\s+(1st|2nd|3rd|\d+th)\s+#{SUFFIX}/i + end + + def prefix + @prefix ||= "#{first_name}'s" + end + + def first_name + name.strip.split(" ", 2).first + end +end diff --git a/app/views/sessions/menus/show.html.erb b/app/views/sessions/menus/show.html.erb index 26b3d25bd..8a36261bf 100644 --- a/app/views/sessions/menus/show.html.erb +++ b/app/views/sessions/menus/show.html.erb @@ -24,13 +24,11 @@

You don’t have any Fizzy accounts.

<% end %> - <% if signups_allowed? %> -
- <%= link_to saas.new_signup_completion_path, class: "btn btn--plain txt-link center txt-small", data: { turbo_prefetch: false } do %> - Sign up for a new Fizzy account - <% end %> -
- <% end %> +
+ <%= link_to new_signup_completion_path, class: "btn btn--plain txt-link center txt-small", data: { turbo_prefetch: false } do %> + Sign up for a new Fizzy account + <% end %> +
<% end %> diff --git a/app/views/signup/completions/new.html.erb b/app/views/signup/completions/new.html.erb new file mode 100644 index 000000000..971c39b1d --- /dev/null +++ b/app/views/signup/completions/new.html.erb @@ -0,0 +1,30 @@ +<% @page_title = "Complete your sign-up" %> + +
"> +

<%= @page_title %>

+ + <%= form_with model: @signup, url: signup_completion_path, scope: "signup", class: "flex flex-column gap", data: { controller: "form" } do |form| %> + <%= form.text_field :full_name, class: "input txt-large", autocomplete: "name", placeholder: "Enter your full name…", autofocus: true, required: true %> + +

You're one step away. Just enter your name to get your own Fizzy account.

+ + <% if @signup.errors.any? %> +
+
    + <% @signup.errors.full_messages.each do |message| %> +
  • <%= message %>
    • + <% end %> +
+
+ <% end %> + + + <% end %> +
+ +<% content_for :footer do %> + <%= render "sessions/footer" %> +<% end %> diff --git a/app/views/signup/new.html.erb b/app/views/signup/new.html.erb new file mode 100644 index 000000000..14da6328e --- /dev/null +++ b/app/views/signup/new.html.erb @@ -0,0 +1,28 @@ +<% @page_title = "Sign up for Fizzy" %> + +
"> +

Sign up

+ + <%= form_with model: @signup, url: signup_path, scope: "signup", class: "flex flex-column gap", data: { turbo: false, controller: "form" } do |form| %> + <%= form.email_field :email_address, class: "input", autocomplete: "username", placeholder: "Email address", required: true %> + + <% if @signup.errors.any? %> +
+
    + <% @signup.errors.full_messages.each do |message| %> +
  • <%= message %>
    • + <% end %> +
+
+ <% end %> + + + <% end %> +
+ +<% content_for :footer do %> + <%= render "sessions/footer" %> +<% end %> diff --git a/config/routes.rb b/config/routes.rb index 3afb093a7..d4e4b17e6 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -155,6 +155,12 @@ Rails.application.routes.draw do end end + get "/signup/new", to: redirect("/session/new") + + namespace :signup do + resource :completion, only: %i[ new create ] + end + resource :landing namespace :my do diff --git a/db/migrate/20251127000001_create_account_external_id_sequences.rb b/db/migrate/20251127000001_create_account_external_id_sequences.rb index 834b80c5b..6f03cb27f 100644 --- a/db/migrate/20251127000001_create_account_external_id_sequences.rb +++ b/db/migrate/20251127000001_create_account_external_id_sequences.rb @@ -1,6 +1,6 @@ class CreateAccountExternalIdSequences < ActiveRecord::Migration[8.0] def change - create_table :account_external_id_sequences do |t| + create_table :account_external_id_sequences, id: :uuid do |t| t.bigint :value, null: false, default: 0 t.index :value, unique: true diff --git a/db/schema.rb b/db/schema.rb index b3a9f0503..236c61df0 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.2].define(version: 2025_11_25_130010) do +ActiveRecord::Schema[8.2].define(version: 2025_11_27_000001) do create_table "accesses", id: :uuid, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.datetime "accessed_at" t.uuid "account_id", null: false @@ -25,6 +25,11 @@ ActiveRecord::Schema[8.2].define(version: 2025_11_25_130010) do t.index ["user_id"], name: "index_accesses_on_user_id" end + create_table "account_external_id_sequences", id: :uuid, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| + t.bigint "value", default: 0, null: false + t.index ["value"], name: "index_account_external_id_sequences_on_value", unique: true + end + create_table "account_join_codes", id: :uuid, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.uuid "account_id", null: false t.string "code", null: false diff --git a/db/schema_sqlite.rb b/db/schema_sqlite.rb index e4475e7ea..fd3cedd44 100644 --- a/db/schema_sqlite.rb +++ b/db/schema_sqlite.rb @@ -25,7 +25,7 @@ ActiveRecord::Schema[8.2].define(version: 2025_11_27_000001) do t.index ["user_id"], name: "index_accesses_on_user_id" end - create_table "account_external_id_sequences", force: :cascade do |t| + create_table "account_external_id_sequences", id: :uuid, force: :cascade do |t| t.bigint "value", default: 0, null: false t.index ["value"], name: "index_account_external_id_sequences_on_value", unique: true end diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index 2f5b27b0d..e4a2163b1 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -21,14 +21,28 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest end end - test "destroy" do - sign_in_as :kevin - + test "create for a new user" do untenanted do - delete session_path + assert_difference -> { Identity.count }, +1 do + assert_difference -> { MagicLink.count }, +1 do + post session_path, + params: { email_address: "nonexistent-#{SecureRandom.hex(6)}@example.com" } + end + end - assert_redirected_to new_session_path - assert_not cookies[:session_token].present? + assert_redirected_to session_magic_link_path end end + + private + test "destroy" do + sign_in_as :kevin + + untenanted do + delete session_path + + assert_redirected_to new_session_path + assert_not cookies[:session_token].present? + end + end end diff --git a/test/controllers/signup/completions_controller_test.rb b/test/controllers/signup/completions_controller_test.rb new file mode 100644 index 000000000..1b788ce31 --- /dev/null +++ b/test/controllers/signup/completions_controller_test.rb @@ -0,0 +1,43 @@ +require "test_helper" + +class Signup::CompletionsControllerTest < ActionDispatch::IntegrationTest + setup do + @signup = Signup.new(email_address: "newuser@example.com", full_name: "New User") + + @signup.create_identity || raise("Failed to create identity") + + sign_in_as @signup.identity + end + + test "new" do + untenanted do + get new_signup_completion_path + end + + assert_response :success + end + + test "create" do + untenanted do + post signup_completion_path, params: { + signup: { + full_name: @signup.full_name + } + } + end + + assert_response :redirect, "Valid params should redirect" + end + + test "create with invalid params" do + untenanted do + post signup_completion_path, params: { + signup: { + full_name: "" + } + } + end + + assert_response :unprocessable_entity, "Invalid params should return unprocessable entity" + end +end diff --git a/test/models/signup/account_name_generator_test.rb b/test/models/signup/account_name_generator_test.rb new file mode 100644 index 000000000..d22922b29 --- /dev/null +++ b/test/models/signup/account_name_generator_test.rb @@ -0,0 +1,61 @@ +require "test_helper" + +class Signup::AccountNameGeneratorTest < ActiveSupport::TestCase + setup do + @identity = Identity.create!(email_address: "newart.userbaum@example.com") + @name = "Newart userbaum" + @generator = Signup::AccountNameGenerator.new(identity: @identity, name: @name) + end + + test "generate" do + account_name = @generator.generate + assert_equal "Newart's Fizzy", account_name, "The 1st account doesn't have 1st in the name" + + first_account = Account.create!(external_account_id: "1st", name: account_name) + Current.without_account do + @identity.users.create!(account: first_account, name: @name) + @identity.reload + end + + account_name = @generator.generate + assert_equal "Newart's 2nd Fizzy", account_name + + second_account = Account.create!(external_account_id: "2nd", name: account_name) + Current.without_account do + @identity.users.create!(account: second_account, name: @name) + @identity.reload + end + + account_name = @generator.generate + assert_equal "Newart's 3rd Fizzy", account_name + + third_account = Account.create!(external_account_id: "3rd", name: account_name) + Current.without_account do + @identity.users.create!(account: third_account, name: @name) + @identity.reload + end + + account_name = @generator.generate + assert_equal "Newart's 4th Fizzy", account_name + + fourth_account = Account.create!(external_account_id: "4th", name: account_name) + Current.without_account do + @identity.users.create!(account: fourth_account, name: @name) + @identity.reload + end + + account_name = @generator.generate + assert_equal "Newart's 5th Fizzy", account_name + end + + test "generate continues from the previous highest index" do + account = Account.create!(external_account_id: "12th", name: "Newart's 12th Fizzy") + Current.without_account do + @identity.users.create!(account: account, name: @name) + @identity.reload + end + + account_name = @generator.generate + assert_equal "Newart's 13th Fizzy", account_name + end +end diff --git a/test/models/signup_test.rb b/test/models/signup_test.rb new file mode 100644 index 000000000..bcdfc8a6c --- /dev/null +++ b/test/models/signup_test.rb @@ -0,0 +1,54 @@ +require "test_helper" + +class SignupTest < ActiveSupport::TestCase + test "#create_identity" do + signup = Signup.new(email_address: "brian@example.com") + + assert_difference -> { Identity.count }, 1 do + assert_difference -> { MagicLink.count }, 1 do + assert signup.create_identity + end + end + + assert_empty signup.errors + assert signup.identity + assert signup.identity.persisted? + + signup_existing = Signup.new(email_address: "brian@example.com") + + assert_no_difference -> { Identity.count } do + assert_difference -> { MagicLink.count }, 1 do + assert signup_existing.create_identity, "Should send magic link for existing identity" + end + end + + signup_invalid = Signup.new(email_address: "") + assert_raises do + signup_invalid.create_identity + end + end + + test "#complete" do + Account.any_instance.expects(:setup_customer_template).once + + Current.without_account do + signup = Signup.new( + full_name: "Kevin", + identity: identities(:kevin) + ) + + assert signup.complete + + assert signup.account + assert signup.user + assert_equal "Kevin", signup.user.name + + signup_invalid = Signup.new( + full_name: "", + identity: identities(:kevin) + ) + assert_not signup_invalid.complete + assert_not_empty signup_invalid.errors[:full_name] + end + end +end