Cleanup onboarding tests, convert to ActionDispatch::IntegrationTest

app/controllers/organizations/onboarding/base_controller.rb

@@ -4,6 +4,8 @@ class Organizations::Onboarding::BaseController < ApplicationController
   before_action :find_or_initialize_onboarding
   before_action :set_breadcrumbs
 
+  layout "onboarding"
+
   def find_or_initialize_onboarding
     @organization_onboarding = OrganizationOnboarding.find_or_initialize_by(created_by: Current.user, status: :pending)
   end

app/controllers/organizations/onboarding/confirm_controller.rb

@@ -1,6 +1,4 @@
 class Organizations::Onboarding::ConfirmController < Organizations::Onboarding::BaseController
-  layout "onboarding"
-
   def edit
   end
 

app/controllers/organizations/onboarding/gems_controller.rb

@@ -1,6 +1,4 @@
 class Organizations::Onboarding::GemsController < Organizations::Onboarding::BaseController
-  layout "onboarding"
-
   def edit
   end
 

app/controllers/organizations/onboarding/name_controller.rb

@@ -1,6 +1,4 @@
 class Organizations::Onboarding::NameController < Organizations::Onboarding::BaseController
-  layout "onboarding"
-
   def new
   end
 

app/controllers/organizations/onboarding/users_controller.rb

@@ -1,6 +1,4 @@
 class Organizations::Onboarding::UsersController < Organizations::Onboarding::BaseController
-  layout "onboarding"
-
   def edit
   end
 

test/functional/organizations/onboarding/gems_controller_test.rb

@@ -1,6 +1,6 @@
 require "test_helper"
 
-class Organizations::Onboarding::GemsControllerTest < ActionController::TestCase
+class Organizations::Onboarding::GemsControllerTest < ActionDispatch::IntegrationTest
   setup do
     @user = create(:user, :mfa_enabled)
     @namesake_rubygem = create(:rubygem, owners: [@user])
@@ -13,35 +13,33 @@ class Organizations::Onboarding::GemsControllerTest < ActionController::TestCase
       organization_handle: @namesake_rubygem.name,
       organization_name: "Existing Name"
     )
-
-    sign_in_as(@user)
   end
 
   context "PATCH update" do
     should "save the selected gems and redirect to the next step" do
-      patch :update, params: { organization_onboarding: { rubygems: [@gem.id] } }
+      patch organization_onboarding_gems_path(as: @user), params: { organization_onboarding: { rubygems: [@gem.id] } }
 
       assert_redirected_to organization_onboarding_users_path
       assert_equal [@namesake_rubygem.id, @gem.id], @organization_onboarding.reload.rubygems
     end
 
     should "allow selecting no additional gems" do
-      patch :update
+      patch organization_onboarding_gems_path(as: @user)
 
       assert_redirected_to organization_onboarding_users_path
       assert_equal [@namesake_rubygem.id], @organization_onboarding.reload.rubygems
     end
 
     should "ignore empty params" do
-      patch :update, params: { organization_onboarding: { rubygems: [""] } }
+      patch organization_onboarding_gems_path(as: @user), params: { organization_onboarding: { rubygems: [""] } }
 
       assert_redirected_to organization_onboarding_users_path
       assert_equal [@namesake_rubygem.id], @organization_onboarding.reload.rubygems
     end
 
     should "invalidate unknown gems" do
       notmygem = create(:rubygem)
-      patch :update, params: { organization_onboarding: { rubygems: [notmygem.id] } }
+      patch organization_onboarding_gems_path(as: @user), params: { organization_onboarding: { rubygems: [notmygem.id] } }
 
       assert_response :unprocessable_entity
       assert_equal [@namesake_rubygem.id], @organization_onboarding.reload.rubygems

test/functional/organizations/onboarding/name_controller_test.rb

@@ -1,16 +1,14 @@
 require "test_helper"
 
-class Organizations::Onboarding::NameControllerTest < ActionController::TestCase
+class Organizations::Onboarding::NameControllerTest < ActionDispatch::IntegrationTest
   setup do
     @user = create(:user, :mfa_enabled)
     @gem = create(:rubygem, owners: [@user])
-
-    sign_in_as(@user)
   end
 
   context "GET new" do
     should "ask the user to start creating a new organization" do
-      get :new
+      get organization_onboarding_name_path(as: @user)
 
       assert_select "input[name=?]", "organization_onboarding[organization_name]"
     end
@@ -21,7 +19,7 @@ class Organizations::Onboarding::NameControllerTest < ActionController::TestCase
       end
 
       should "render with the in-progress onboarding" do
-        get :new
+        get organization_onboarding_name_path(as: @user)
 
         assert_select "input[name=?][value=?]", "organization_onboarding[organization_name]", @organization_onboarding.organization_name
       end
@@ -30,7 +28,7 @@ class Organizations::Onboarding::NameControllerTest < ActionController::TestCase
 
   context "POST create" do
     should "create a new onboarding and redirect to the next step" do
-      post :create, params: { organization_onboarding: { organization_name: "New Name", organization_handle: @gem.name, name_type: "gem" } }
+      post organization_onboarding_name_path(as: @user), params: { organization_onboarding: { organization_name: "New Name", organization_handle: @gem.name, name_type: "gem" } }
 
       assert OrganizationOnboarding.exists?(organization_name: "New Name", organization_handle: @gem.name, name_type: "gem")
       assert_redirected_to organization_onboarding_gems_path
@@ -42,7 +40,7 @@ class Organizations::Onboarding::NameControllerTest < ActionController::TestCase
       end
 
       should "update the existing onboarding and redirect to the next step" do
-        post :create, params: { organization_onboarding: { organization_name: "Updated Name" } }
+        post organization_onboarding_name_path(as: @user), params: { organization_onboarding: { organization_name: "Updated Name" } }
 
         assert_redirected_to organization_onboarding_gems_path
         assert_equal "Updated Name", @organization_onboarding.reload.organization_name
@@ -51,7 +49,7 @@ class Organizations::Onboarding::NameControllerTest < ActionController::TestCase
 
     context "when the onboarding is invalid" do
       should "render the form with an error" do
-        post :create, params: { organization_onboarding: { organization_name: "" } }
+        post organization_onboarding_name_path(as: @user), params: { organization_onboarding: { organization_name: "" } }
 
         assert_response :unprocessable_entity
       end

test/functional/organizations/onboarding/users_controller_test.rb

@@ -1,13 +1,11 @@
 require "test_helper"
 
-class Organizations::Onboarding::UsersControllerTest < ActionController::TestCase
+class Organizations::Onboarding::UsersControllerTest < ActionDispatch::IntegrationTest
   setup do
     @user = create(:user, :mfa_enabled)
     @other_users = create_list(:user, 2)
     @rubygem = create(:rubygem, owners: [@user, *@other_users])
 
-    sign_in_as(@user)
-
     @organization_onboarding = create(
       :organization_onboarding,
       created_by: @user,
@@ -17,107 +15,110 @@ class Organizations::Onboarding::UsersControllerTest < ActionController::TestCas
     @invites = @organization_onboarding.invites.to_a
   end
 
-  test "render the list of users to invite" do
-    get :edit
+  context "on GET /organizations/onboarding/users" do
+    should "render the list of users to invite" do
+      get organization_onboarding_users_path(as: @user)
 
-    assert_response :ok
-    # assert a text field has has the handle
-    @invites.each_with_index do |invite, idx|
-      assert_select "input[name='organization_onboarding[invites_attributes][#{idx}][id]'][value='#{invite.id}']"
-      assert_select "select[name='organization_onboarding[invites_attributes][#{idx}][role]']"
+      assert_response :ok
+      # assert a text field has the handle
+      @invites.each_with_index do |invite, idx|
+        assert_select "input[name='organization_onboarding[invites_attributes][#{idx}][id]'][value='#{invite.id}']"
+        assert_select "select[name='organization_onboarding[invites_attributes][#{idx}][role]']"
+      end
     end
-  end
 
-  test "should update invites ignoring blank rows" do
-    patch :update, params: {
-      organization_onboarding: {
-        invites_attributes: {
-          "0" => { id: @invites[0].id, role: "maintainer" },
-          "1" => { id: @invites[1].id, role: "" }
-        }
-      }
-    }
+    context "when there are already users added" do
+      should "render the list of users with their roles" do
+        @organization_onboarding.invites.where(user_id: @other_users[0].id).update!(role: "admin")
+        @organization_onboarding.invites.where(user_id: @other_users[1].id).update!(role: "maintainer")
 
-    assert_redirected_to organization_onboarding_confirm_path
+        get organization_onboarding_users_path(as: @user)
 
-    @organization_onboarding.reload
+        assert_response :ok
 
-    assert_equal "maintainer", @organization_onboarding.invites.find_by(user_id: @other_users[0].id).role
-    assert_nil @organization_onboarding.invites.find_by(user_id: @other_users[1].id).role
+        assert_select "input[name='organization_onboarding[invites_attributes][0][id]'][value='#{@invites[0].id}']"
+        assert_select "select[name='organization_onboarding[invites_attributes][0][role]'] option[selected][value='admin']"
+        assert_select "input[name='organization_onboarding[invites_attributes][1][id]'][value='#{@invites[1].id}']"
+        assert_select "select[name='organization_onboarding[invites_attributes][1][role]'] option[selected][value='maintainer']"
+      end
+    end
   end
 
-  test "should update invites ignoring user_id in invites_attributes" do
-    patch :update, params: {
-      organization_onboarding: {
-        invites_attributes: {
-          "0" => { id: @invites[0].id, role: "maintainer" },
-          "1" => { user_id: @invites[1].user.id, role: "owner" }
+  context "on PATCH /organizations/onboarding/users" do
+    should "update invites ignoring blank rows" do
+      patch organization_onboarding_users_path(as: @user), params: {
+        organization_onboarding: {
+          invites_attributes: {
+            "0" => { id: @invites[0].id, role: "maintainer" },
+            "1" => { id: @invites[1].id, role: "" }
+          }
         }
       }
-    }
 
-    assert_redirected_to organization_onboarding_confirm_path
+      assert_redirected_to organization_onboarding_confirm_path
 
-    @organization_onboarding.reload
+      @organization_onboarding.reload
 
-    assert_equal "maintainer", @organization_onboarding.invites.find_by(user_id: @other_users[0].id).role
-    assert_nil @organization_onboarding.invites.find_by(user_id: @other_users[1].id).role
-    assert_equal 1, @organization_onboarding.approved_invites.count
-  end
+      assert_equal "maintainer", @organization_onboarding.invites.find_by(user_id: @other_users[0].id).role
+      assert_nil @organization_onboarding.invites.find_by(user_id: @other_users[1].id).role
+    end
 
-  test "should update multiple users" do
-    patch :update, params: {
-      organization_onboarding: {
-        invites_attributes: {
-          "0" => { id: @invites[0].id, role: "maintainer" },
-          "1" => { id: @invites[1].id, role: "admin" }
+    should "update invites ignoring user_id in invites_attributes" do
+      patch organization_onboarding_users_path(as: @user), params: {
+        organization_onboarding: {
+          invites_attributes: {
+            "0" => { id: @invites[0].id, role: "maintainer" },
+            "1" => { user_id: @invites[1].user.id, role: "owner" }
+          }
         }
       }
-    }
 
-    assert_redirected_to organization_onboarding_confirm_path
+      assert_redirected_to organization_onboarding_confirm_path
 
-    @organization_onboarding.reload
+      @organization_onboarding.reload
 
-    assert_equal "maintainer", @organization_onboarding.invites.find_by(user_id: @other_users[0].id).role
-    assert_equal "admin", @organization_onboarding.invites.find_by(user_id: @other_users[1].id).role
-  end
+      assert_equal "maintainer", @organization_onboarding.invites.find_by(user_id: @other_users[0].id).role
+      assert_nil @organization_onboarding.invites.find_by(user_id: @other_users[1].id).role
+      assert_equal 1, @organization_onboarding.approved_invites.count
+    end
 
-  test "should update users including existing invites" do
-    patch :update, params: {
-      organization_onboarding: {
-        invites_attributes: {
-          "0" => { id: @invites[0].id, role: "admin" },
-          "1" => { id: @invites[1].id, role: "maintainer" }
+    should "update multiple users" do
+      patch organization_onboarding_users_path(as: @user), params: {
+        organization_onboarding: {
+          invites_attributes: {
+            "0" => { id: @invites[0].id, role: "maintainer" },
+            "1" => { id: @invites[1].id, role: "admin" }
+          }
         }
       }
-    }
-
-    @organization_onboarding.reload
 
-    assert_redirected_to organization_onboarding_confirm_path
-    assert_equal "admin", @organization_onboarding.invites.find_by(user_id: @other_users[0].id).role
-    assert_equal "maintainer", @organization_onboarding.invites.find_by(user_id: @other_users[1].id).role
+      assert_redirected_to organization_onboarding_confirm_path
 
-    get :edit
+      @organization_onboarding.reload
 
-    assert_select "input[name='organization_onboarding[invites_attributes][0][id]'][value='#{@invites[0].id}']"
-    assert_select "select[name='organization_onboarding[invites_attributes][0][role]'] option[selected][value='admin']"
-    assert_select "input[name='organization_onboarding[invites_attributes][1][id]'][value='#{@invites[1].id}']"
-    assert_select "select[name='organization_onboarding[invites_attributes][1][role]'] option[selected][value='maintainer']"
+      assert_equal "maintainer", @organization_onboarding.invites.find_by(user_id: @other_users[0].id).role
+      assert_equal "admin", @organization_onboarding.invites.find_by(user_id: @other_users[1].id).role
+    end
 
-    patch :update, params: {
-      organization_onboarding: {
-        invites_attributes: {
-          "0" => { id: @invites[0].id, role: "maintainer" },
-          "1" => { id: @invites[1].id, role: "" }
+    context "when already invited users" do
+      should "update roles and/or uninvite" do
+        @organization_onboarding.invites.create(user_id: @other_users[0].id, role: "admin")
+        @organization_onboarding.invites.create(user_id: @other_users[1].id, role: "maintainer")
+
+        patch organization_onboarding_users_path(as: @user), params: {
+          organization_onboarding: {
+            invites_attributes: {
+              "0" => { id: @invites[0].id, role: "maintainer" },
+              "1" => { id: @invites[1].id, role: "" }
+            }
+          }
         }
-      }
-    }
 
-    @organization_onboarding.reload
+        @organization_onboarding.reload
 
-    assert_equal "maintainer", @organization_onboarding.invites.find_by(user_id: @other_users[0].id).role
-    assert_nil @organization_onboarding.invites.find_by(user_id: @other_users[1].id).role
+        assert_equal "maintainer", @organization_onboarding.invites.find_by(user_id: @other_users[0].id).role
+        assert_nil @organization_onboarding.invites.find_by(user_id: @other_users[1].id).role
+      end
+    end
   end
 end