diff --git a/app/controllers/account/join_codes_controller.rb b/app/controllers/account/join_codes_controller.rb index 562ffaa0d..fba2ccfe9 100644 --- a/app/controllers/account/join_codes_controller.rb +++ b/app/controllers/account/join_codes_controller.rb @@ -20,7 +20,7 @@ class Account::JoinCodesController < ApplicationController private def set_join_code - @join_code = Account::JoinCode.sole + @join_code = Current.account.join_code end def join_code_params diff --git a/app/controllers/concerns/authorization.rb b/app/controllers/concerns/authorization.rb index aa1d08e34..c18958536 100644 --- a/app/controllers/concerns/authorization.rb +++ b/app/controllers/concerns/authorization.rb @@ -30,8 +30,6 @@ module Authorization redirect_to session_menu_url(script_name: nil) elsif Current.membership.blank? redirect_to session_menu_url(script_name: nil) - elsif Current.user.nil? && Current.membership.join_code.present? - redirect_to new_users_join_path elsif !Current.user&.active? redirect_to unlink_membership_url(script_name: nil, membership_id: Current.membership.signed_id(purpose: :unlinking)) end diff --git a/app/controllers/join_codes_controller.rb b/app/controllers/join_codes_controller.rb index 2edf41860..62f3c1ca9 100644 --- a/app/controllers/join_codes_controller.rb +++ b/app/controllers/join_codes_controller.rb @@ -1,40 +1,46 @@ class JoinCodesController < ApplicationController disallow_account_scope allow_unauthenticated_access + before_action :set_join_code before_action :ensure_join_code_is_valid layout "public" def new - @account = Account.find_by_external_account_id!(tenant) - @account_name = @account.name end def create - Identity.transaction do - identity = Identity.find_or_create_by!(email_address: params.expect(:email_address)) - identity.memberships.find_or_create_by!(tenant: tenant) do |membership| - membership.join_code = code + identity = Identity.find_or_create_by!(email_address: params.expect(:email_address)) + + unless identity.member_of?(@join_code.account) + @join_code.redeem do |account| + identity.join(account) end + magic_link = identity.send_magic_link flash[:magic_link_code] = magic_link&.code if Rails.env.development? - end - session[:return_to_after_authenticating] = landing_url(script_name: "/#{tenant}") - redirect_to session_magic_link_path + session[:return_to_after_authenticating] = new_users_join_url(script_name: @join_code.account.slug) + redirect_to session_magic_link_path + else + redirect_to landing_url(script_name: @join_code.account.slug) + end end private - def ensure_join_code_is_valid - head :not_found unless @join_code&.active? - end - def set_join_code # TODO:PLANB: this find should be scoped by account + # 2025-11-12 [Stanko]: I think that we don't have to scope this by account as the code + # is globally unique and indexed. Except if we need to scope it + # for some other reason? @join_code ||= Account::JoinCode.active.find_by(code: code) end + def ensure_join_code_is_valid + head :not_found unless @join_code&.active? + end + def tenant params.expect(:tenant) end diff --git a/app/controllers/users/joins_controller.rb b/app/controllers/users/joins_controller.rb index 73a9ae296..6baa888ae 100644 --- a/app/controllers/users/joins_controller.rb +++ b/app/controllers/users/joins_controller.rb @@ -1,32 +1,15 @@ class Users::JoinsController < ApplicationController - require_access_without_a_user - - before_action :set_join_code, :ensure_join_code_is_valid - layout "public" def new end def create - @join_code.redeem do - User.create!(user_params.merge(membership: Current.membership)) - end - + Current.user.update!(user_params) redirect_to landing_path end private - def set_join_code - @join_code = Account::JoinCode.active.find_by(code: Current.membership.join_code) - end - - def ensure_join_code_is_valid - unless @join_code&.active? - redirect_to unlink_membership_url(script_name: nil, membership_id: Current.membership.signed_id(purpose: :unlinking)) - end - end - def user_params params.expect(user: [ :name, :avatar ]) end diff --git a/app/models/account/join_code.rb b/app/models/account/join_code.rb index b647c70ad..49fe9af22 100644 --- a/app/models/account/join_code.rb +++ b/app/models/account/join_code.rb @@ -10,7 +10,7 @@ class Account::JoinCode < ApplicationRecord def redeem transaction do increment!(:usage_count) - yield if block_given? + yield account if block_given? end end diff --git a/app/models/identity.rb b/app/models/identity.rb index 09d29d87e..9c00e999c 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -1,5 +1,5 @@ class Identity < ApplicationRecord - include Transferable + include Joinable, Transferable has_many :memberships, dependent: :destroy has_many :magic_links, dependent: :destroy diff --git a/app/models/identity/joinable.rb b/app/models/identity/joinable.rb new file mode 100644 index 000000000..6286485f6 --- /dev/null +++ b/app/models/identity/joinable.rb @@ -0,0 +1,14 @@ +module Identity::Joinable + extend ActiveSupport::Concern + + def join(account) + transaction do + membership = memberships.create!(tenant: account.external_account_id) + account.users.create!(membership: membership, name: email_address) + end + end + + def member_of?(account) + memberships.exists?(tenant: account.external_account_id.to_s) + end +end diff --git a/app/models/search/highlighter.rb b/app/models/search/highlighter.rb index 0680901c8..c62755e94 100644 --- a/app/models/search/highlighter.rb +++ b/app/models/search/highlighter.rb @@ -13,7 +13,7 @@ class Search::Highlighter result = text.dup terms.each do |term| - result.gsub!(/(#{Regexp.escape(term)})/i) do |match| + result.gsub!(/\b(#{Regexp.escape(term)}\w*)\b/i) do |match| "#{OPENING_MARK}#{match}#{CLOSING_MARK}" end end diff --git a/app/views/join_codes/new.html.erb b/app/views/join_codes/new.html.erb index c4d933b8b..6a9ed4e19 100644 --- a/app/views/join_codes/new.html.erb +++ b/app/views/join_codes/new.html.erb @@ -1,4 +1,4 @@ -<% @page_title = "Join #{@account_name} in Fizzy" %> +<% @page_title = "Join #{@join_code.account.name} in Fizzy" %>

<%= @page_title %>

diff --git a/db/migrate/20251112184932_remove_join_code_from_memberships.rb b/db/migrate/20251112184932_remove_join_code_from_memberships.rb new file mode 100644 index 000000000..75dee872e --- /dev/null +++ b/db/migrate/20251112184932_remove_join_code_from_memberships.rb @@ -0,0 +1,5 @@ +class RemoveJoinCodeFromMemberships < ActiveRecord::Migration[8.2] + def change + remove_column :memberships, :join_code, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 7b59e877b..a67645407 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_12_093037) do +ActiveRecord::Schema[8.2].define(version: 2025_11_12_184932) do create_table "accesses", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.datetime "accessed_at" t.bigint "board_id", null: false @@ -291,7 +291,6 @@ ActiveRecord::Schema[8.2].define(version: 2025_11_12_093037) do create_table "memberships", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.datetime "created_at", null: false t.bigint "identity_id", null: false - t.string "join_code" t.string "tenant", null: false t.datetime "updated_at", null: false t.index ["identity_id"], name: "index_memberships_on_identity_id" diff --git a/test/controllers/accounts/join_codes_controller_test.rb b/test/controllers/accounts/join_codes_controller_test.rb index ee80e4193..33c118386 100644 --- a/test/controllers/accounts/join_codes_controller_test.rb +++ b/test/controllers/accounts/join_codes_controller_test.rb @@ -9,7 +9,7 @@ class Account::JoinCodesControllerTest < ActionDispatch::IntegrationTest get account_join_code_path assert_response :success - assert_changes -> { Account::JoinCode.sole.code } do + assert_changes -> { Current.account.join_code.reload.code } do delete account_join_code_path assert_redirected_to account_join_code_path end @@ -20,7 +20,7 @@ class Account::JoinCodesControllerTest < ActionDispatch::IntegrationTest assert_response :success put account_join_code_path, params: { account_join_code: { usage_limit: 5 } } - assert_equal 5, Account::JoinCode.sole.usage_limit + assert_equal 5, Current.account.join_code.reload.usage_limit assert_redirected_to account_join_code_path end diff --git a/test/controllers/join_codes_controller_test.rb b/test/controllers/join_codes_controller_test.rb index f9ddbcf07..097d30ca1 100644 --- a/test/controllers/join_codes_controller_test.rb +++ b/test/controllers/join_codes_controller_test.rb @@ -3,7 +3,7 @@ require "test_helper" class JoinCodesControllerTest < ActionDispatch::IntegrationTest setup do @tenant = accounts("37s").external_account_id - @join_code = account_join_codes(:sole) + @join_code = account_join_codes(:"37s") end test "new" do @@ -41,10 +41,9 @@ class JoinCodesControllerTest < ActionDispatch::IntegrationTest end end - assert_equal @join_code.code, Membership.last.join_code - assert_redirected_to session_magic_link_path - assert_equal landing_url(script_name: "/#{@tenant}"), session[:return_to_after_authenticating] + account = accounts("37s") + assert_equal new_users_join_url(script_name: account.slug), session[:return_to_after_authenticating] end end @@ -58,7 +57,8 @@ class JoinCodesControllerTest < ActionDispatch::IntegrationTest end end - assert_redirected_to session_magic_link_path + account = accounts("37s") + assert_redirected_to landing_url(script_name: account.slug) end end end diff --git a/test/controllers/users/joins_controller_test.rb b/test/controllers/users/joins_controller_test.rb index 839458bc9..552707236 100644 --- a/test/controllers/users/joins_controller_test.rb +++ b/test/controllers/users/joins_controller_test.rb @@ -2,31 +2,21 @@ require "test_helper" class Users::JoinsControllerTest < ActionDispatch::IntegrationTest test "new" do - identity = Identity.create!(email_address: "new.user@example.com") - identity.memberships.create(tenant: Current.account.external_account_id, join_code: Account::JoinCode.sole.code) - sign_in_as identity + sign_in_as :david get new_users_join_path assert_response :ok end - test "new with invalid params" do - identity = Identity.create!(email_address: "new.user@example.com") - membership = identity.memberships.create(tenant: Current.account.external_account_id, join_code: "PHONY") - sign_in_as identity - - get new_users_join_path - assert_redirected_to unlink_membership_url(script_name: nil, membership_id: membership.signed_id(purpose: :unlinking)) - end - test "create" do - identity = Identity.create!(email_address: "newart.userbaum@example.com") - identity.memberships.create(tenant: Current.account.external_account_id, join_code: Account::JoinCode.sole.code) - sign_in_as identity + user = users(:david) + sign_in_as user - assert_difference -> { User.count }, +1 do - post users_joins_path, params: { user: { name: "Newart Userbaum" } } + assert_no_difference -> { User.count } do + post users_joins_path, params: { user: { name: "David Updated" } } assert_redirected_to landing_path end + + assert_equal "David Updated", user.reload.name end end diff --git a/test/fixtures/account/join_codes.yml b/test/fixtures/account/join_codes.yml index 855c67c85..1f4c2f876 100644 --- a/test/fixtures/account/join_codes.yml +++ b/test/fixtures/account/join_codes.yml @@ -1,5 +1,11 @@ -sole: - code: 1234-5678-9XYZ +37s: + code: 37S0-5678-9XYZ usage_count: 0 usage_limit: 10 account: 37s + +initech: + code: INIT-5678-9XYZ + usage_count: 10 + usage_limit: 10 + account: initech diff --git a/test/models/account/join_code_test.rb b/test/models/account/join_code_test.rb index ed8062e5a..4247ce7ec 100644 --- a/test/models/account/join_code_test.rb +++ b/test/models/account/join_code_test.rb @@ -11,7 +11,7 @@ class Account::JoinCodeTest < ActiveSupport::TestCase end test "redeem" do - join_code = account_join_codes(:sole) + join_code = account_join_codes(:"37s") assert_difference -> { join_code.reload.usage_count }, 1 do join_code.redeem @@ -19,7 +19,7 @@ class Account::JoinCodeTest < ActiveSupport::TestCase end test "reset" do - join_code = account_join_codes(:sole) + join_code = account_join_codes(:"37s") original_code = join_code.code join_code.reset diff --git a/test/models/identity_test.rb b/test/models/identity_test.rb index 26e84fd51..27fa2b73a 100644 --- a/test/models/identity_test.rb +++ b/test/models/identity_test.rb @@ -18,4 +18,20 @@ class IdentityTest < ActiveSupport::TestCase assert Identity.new(email_address: "test@basecamp.com").staff? assert_not Identity.new(email_address: "test@example.com").staff? end + + test "join creates membership and user for account" do + identity = identities(:david) + account = accounts(:initech) + + assert_difference ["Membership.count", "User.count"], 1 do + identity.join(account) + end + + membership = identity.memberships.last + assert_equal account.external_account_id.to_s, membership.tenant + + user = account.users.last + assert_equal membership, user.membership + assert_equal identity.email_address, user.name + end end