Merge pull request #1745 from basecamp/fix-join-codes-skipping-user-setup

Fix join codes skipping user setup

app/controllers/join_codes_controller.rb

@@ -13,9 +13,12 @@ class JoinCodesController < ApplicationController
     identity = Identity.find_or_create_by!(email_address: params.expect(:email_address))
 
     @join_code.redeem { |account| identity.join(account) } unless identity.member_of?(@join_code.account)
+    user = User.active.find_by!(account: @join_code.account, identity: identity)
 
-    if identity == Current.identity
+    if identity == Current.identity && user.setup?
       redirect_to landing_url(script_name: @join_code.account.slug)
+    elsif identity == Current.identity
+      redirect_to new_users_join_url(script_name: @join_code.account.slug)
     else
       terminate_session if Current.identity
 

app/models/user.rb

@@ -25,4 +25,8 @@ class User < ApplicationRecord
       update! active: false, identity: nil
     end
   end
+
+  def setup?
+    name != identity.email_address
+  end
 end

test/controllers/join_codes_controller_test.rb

@@ -43,6 +43,9 @@ class JoinCodesControllerTest < ActionDispatch::IntegrationTest
     identity = identities(:jz)
     sign_in_as :jz
 
+    assert identity.member_of?(@account), "JZ should be a member of 37s for this test"
+    assert identity.users.find_by!(account: @account).setup?, "JZ's user should be setup for this test"
+
     assert_no_difference -> { Identity.count } do
       assert_no_difference -> { User.count } do
         post join_path(code: @join_code.code, script_name: @account.slug), params: { email_address: identity.email_address }
@@ -51,4 +54,19 @@ class JoinCodesControllerTest < ActionDispatch::IntegrationTest
 
     assert_redirected_to landing_url(script_name: @account.slug)
   end
+
+  test "create for signed-in identity without a user in the account redirects to user setup" do
+    identity = identities(:mike)
+    sign_in_as :mike
+
+    assert_not identity.member_of?(@account), "Mike should not be a member of 37s for this test"
+
+    assert_no_difference -> { Identity.count } do
+      assert_difference -> { User.count }, 1 do
+        post join_path(code: @join_code.code, script_name: @account.slug), params: { email_address: identity.email_address }
+      end
+    end
+
+    assert_redirected_to new_users_join_url(script_name: @account.slug)
+  end
 end

test/models/identity/joinable_test.rb

@@ -0,0 +1,25 @@
+require "test_helper"
+
+class Identity::JoinableTest < ActiveSupport::TestCase
+  test "join" do
+    identity = identities(:david)
+
+    user = identity.join(accounts(:initech))
+    assert_kind_of User, user
+    assert_equal accounts(:initech), user.account
+    assert_equal identity.email_address, user.name
+
+    identity = identities(:mike)
+
+    user = identity.join(accounts("37s"), name: "Mike")
+    assert_kind_of User, user
+    assert_equal accounts("37s"), user.account
+    assert_equal "Mike", user.name
+  end
+
+  test "member_of?" do
+    identity = identities(:david)
+    assert identity.member_of?(accounts("37s"))
+    assert_not identity.member_of?(accounts(:initech))
+  end
+end

test/models/user_test.rb

@@ -35,4 +35,14 @@ class UserTest < ActiveSupport::TestCase
     assert_equal "DHH", User.new(name: "David Heinemeier Hansson").initials
     assert_equal "ÉLH", User.new(name: "Éva-Louise Hernández").initials
   end
+
+  test "setup?" do
+    user = users(:kevin)
+
+    user.update!(name: user.identity.email_address)
+    assert_not user.setup?
+
+    user.update!(name: "Kevin")
+    assert user.setup?
+  end
 end