From 2311efd03e1edff86f14cf86b63fbced769cf3a0 Mon Sep 17 00:00:00 2001 From: "Stanko K.R." Date: Sat, 29 Nov 2025 12:36:00 +0100 Subject: [PATCH] Make sign up Magic Links work across devices Previously if someone started signing in on their phone, but finished it on their laptop, they'd end up on the menu screen with no account. Bu tracking the purpose of a Magic Link we can always direct the user to sign up if they requested a magic link through sign up. This also makes the logic for changing the copy in the email more robust. --- .../sessions/magic_links_controller.rb | 14 ++++++++--- app/models/identity.rb | 6 +++-- app/models/magic_link.rb | 4 ++- app/models/signup.rb | 2 +- .../sign_in_instructions.html.erb | 2 +- .../sign_in_instructions.text.erb | 2 +- ...251129110120_add_purpose_to_magic_links.rb | 11 ++++++++ db/schema.rb | 3 ++- .../sessions/magic_links_controller_test.rb | 25 +++++++++++++++---- test/models/magic_link_test.rb | 4 +-- 10 files changed, 56 insertions(+), 17 deletions(-) create mode 100644 db/migrate/20251129110120_add_purpose_to_magic_links.rb diff --git a/app/controllers/sessions/magic_links_controller.rb b/app/controllers/sessions/magic_links_controller.rb index 40de894df..dc55782af 100644 --- a/app/controllers/sessions/magic_links_controller.rb +++ b/app/controllers/sessions/magic_links_controller.rb @@ -9,9 +9,9 @@ class Sessions::MagicLinksController < ApplicationController end def create - if identity = MagicLink.consume(code) - start_new_session_for identity - redirect_to after_authentication_url + if magic_link = MagicLink.consume(code) + start_new_session_for magic_link.identity + redirect_to after_sign_in_url(magic_link) else redirect_to session_magic_link_path, alert: "Try another code." end @@ -21,4 +21,12 @@ class Sessions::MagicLinksController < ApplicationController def code params.expect(:code) end + + def after_sign_in_url(magic_link) + if magic_link.for_sign_up? + new_signup_completion_path + else + after_authentication_url + end + end end diff --git a/app/models/identity.rb b/app/models/identity.rb index e9ff507a1..135fdaae7 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -12,8 +12,10 @@ class Identity < ApplicationRecord normalizes :email_address, with: ->(value) { value.strip.downcase.presence } - def send_magic_link - magic_links.create!.tap do |magic_link| + def send_magic_link(**attributes) + attributes[:purpose] = attributes.delete(:for) if attributes.key?(:for) + + magic_links.create!(attributes).tap do |magic_link| MagicLinkMailer.sign_in_instructions(magic_link).deliver_later end end diff --git a/app/models/magic_link.rb b/app/models/magic_link.rb index 69961e9a5..987ca776a 100644 --- a/app/models/magic_link.rb +++ b/app/models/magic_link.rb @@ -4,6 +4,8 @@ class MagicLink < ApplicationRecord belongs_to :identity + enum :purpose, %w[ sign_in sign_up ], prefix: :for, default: :sign_in + scope :active, -> { where(expires_at: Time.current...) } scope :stale, -> { where(expires_at: ..Time.current) } @@ -24,7 +26,7 @@ class MagicLink < ApplicationRecord def consume destroy - identity + self end private diff --git a/app/models/signup.rb b/app/models/signup.rb index 230aafd0b..bdcf3490d 100644 --- a/app/models/signup.rb +++ b/app/models/signup.rb @@ -18,7 +18,7 @@ class Signup def create_identity @identity = Identity.find_or_create_by!(email_address: email_address) - @identity.send_magic_link + @identity.send_magic_link for: :sign_up end def complete diff --git a/app/views/mailers/magic_link_mailer/sign_in_instructions.html.erb b/app/views/mailers/magic_link_mailer/sign_in_instructions.html.erb index 68ddeb165..40303c39e 100644 --- a/app/views/mailers/magic_link_mailer/sign_in_instructions.html.erb +++ b/app/views/mailers/magic_link_mailer/sign_in_instructions.html.erb @@ -1,5 +1,5 @@ -<% if @identity.users.any? %> +<% if @magic_link.for_sign_in? %>

Fizzy verification code

Please enter this 6-character verification code on the Fizzy sign-in page:

<% else %> diff --git a/app/views/mailers/magic_link_mailer/sign_in_instructions.text.erb b/app/views/mailers/magic_link_mailer/sign_in_instructions.text.erb index 2d5ba8f8b..b21f00509 100644 --- a/app/views/mailers/magic_link_mailer/sign_in_instructions.text.erb +++ b/app/views/mailers/magic_link_mailer/sign_in_instructions.text.erb @@ -1,4 +1,4 @@ -<% if @identity.users.any? %> +<% if @magic_link.for_sign_in? %> Please enter this 6-character verification code on the Fizzy sign-in page: <% else %> Please enter this 6-character verification code on the Fizzy sign-up page to create your new account: diff --git a/db/migrate/20251129110120_add_purpose_to_magic_links.rb b/db/migrate/20251129110120_add_purpose_to_magic_links.rb new file mode 100644 index 000000000..52e332d72 --- /dev/null +++ b/db/migrate/20251129110120_add_purpose_to_magic_links.rb @@ -0,0 +1,11 @@ +class AddPurposeToMagicLinks < ActiveRecord::Migration[8.2] + def change + add_column :magic_links, :purpose, :integer, null: true + + execute <<-SQL + UPDATE magic_links SET purpose = 0 + SQL + + change_column_null :magic_links, :purpose, false + end +end diff --git a/db/schema.rb b/db/schema.rb index 236c61df0..ef0a4d166 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_27_000001) do +ActiveRecord::Schema[8.2].define(version: 2025_11_29_110120) 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 @@ -315,6 +315,7 @@ ActiveRecord::Schema[8.2].define(version: 2025_11_27_000001) do t.datetime "created_at", null: false t.datetime "expires_at", null: false t.uuid "identity_id" + t.integer "purpose", null: false t.datetime "updated_at", null: false t.index ["code"], name: "index_magic_links_on_code", unique: true t.index ["expires_at"], name: "index_magic_links_on_expires_at" diff --git a/test/controllers/sessions/magic_links_controller_test.rb b/test/controllers/sessions/magic_links_controller_test.rb index a79027556..dbde3723e 100644 --- a/test/controllers/sessions/magic_links_controller_test.rb +++ b/test/controllers/sessions/magic_links_controller_test.rb @@ -9,17 +9,32 @@ class Sessions::MagicLinksControllerTest < ActionDispatch::IntegrationTest end end - test "create" do + test "create with sign in code" do identity = identities(:kevin) magic_link = MagicLink.create!(identity: identity) untenanted do post session_magic_link_url, params: { code: magic_link.code } - end - assert_response :redirect - assert cookies[:session_token].present? - assert_not MagicLink.exists?(magic_link.id), "The magic link should be consumed" + assert_response :redirect + assert cookies[:session_token].present? + assert_redirected_to landing_path, "Should redirect to after authentication path" + assert_not MagicLink.exists?(magic_link.id), "The magic link should be consumed" + end + end + + test "create with sign up code" do + identity = identities(:kevin) + magic_link = MagicLink.create!(identity: identity, purpose: :sign_up) + + untenanted do + post session_magic_link_url, params: { code: magic_link.code } + + assert_response :redirect + assert cookies[:session_token].present? + assert_redirected_to new_signup_completion_path, "Should redirect to signup completion" + assert_not MagicLink.exists?(magic_link.id), "The magic link should be consumed" + end end test "create with invalid code" do diff --git a/test/models/magic_link_test.rb b/test/models/magic_link_test.rb index 4dec89737..d9cb1f73e 100644 --- a/test/models/magic_link_test.rb +++ b/test/models/magic_link_test.rb @@ -32,8 +32,8 @@ class MagicLinkTest < ActiveSupport::TestCase magic_link = MagicLink.create!(identity: identities(:kevin)) code_with_spaces = magic_link.code.downcase.chars.join(" ") - identity = MagicLink.consume(code_with_spaces) - assert_equal identities(:kevin), identity + consumed_magic_link = MagicLink.consume(code_with_spaces) + assert_equal magic_link, consumed_magic_link assert_not MagicLink.exists?(magic_link.id) expired_link = MagicLink.create!(identity: identities(:kevin))