Update and move onboarding controller tests to test/functional (#5307)

* Move onboarding controller tests to test/functional

These tests fit more with the functional paradigm since they are named
after a controller and test one action at a time.

* 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/integration/organizations/onboarding/gems_controller_test.rb → 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/integration/organizations/onboarding/name_controller_test.rb → 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,9 @@ 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 +42,9 @@ 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 +53,9 @@ 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

@@ -0,0 +1,124 @@
+require "test_helper"
+
+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])
+
+    @organization_onboarding = create(
+      :organization_onboarding,
+      created_by: @user,
+      namesake_rubygem: @rubygem
+    )
+
+    @invites = @organization_onboarding.invites.to_a
+  end
+
+  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 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
+
+    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")
+
+        get organization_onboarding_users_path(as: @user)
+
+        assert_response :ok
+
+        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
+
+  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
+
+      @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
+    end
+
+    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
+
+      @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
+
+    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" }
+          }
+        }
+      }
+
+      assert_redirected_to organization_onboarding_confirm_path
+
+      @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
+
+    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
+
+        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

test/functional/organizations/onboarding_controller_test.rb

@@ -0,0 +1,60 @@
+require "test_helper"
+
+class Organizations::OnboardingControllerTest < ActionDispatch::IntegrationTest
+  setup do
+    @user = create(:user)
+  end
+
+  context "GET /organizations/onboarding" do
+    should "redirect to onboarding start page" do
+      get organization_onboarding_path(as: @user)
+
+      assert_redirected_to organization_onboarding_name_path
+    end
+  end
+
+  context "DELETE /organizations/onboarding" do
+    context "when onboarding is already completed" do
+      should "not destroy the OrganizationOnboarding" do
+        organization_onboarding = create(:organization_onboarding, :completed, created_by: @user)
+
+        delete organization_onboarding_path(as: @user)
+
+        assert_redirected_to dashboard_path
+        assert OrganizationOnboarding.exists?(id: organization_onboarding.id)
+      end
+    end
+
+    context "when user has a pending onboarding" do
+      should "destroy the OrganizationOnboarding" do
+        organization_onboarding = create(:organization_onboarding, created_by: @user)
+
+        delete organization_onboarding_path(as: @user)
+
+        assert_redirected_to dashboard_path
+        refute OrganizationOnboarding.exists?(id: organization_onboarding.id)
+      end
+    end
+
+    context "when user has a failed onboarding" do
+      should "destroy the OrganizationOnboarding" do
+        organization_onboarding = create(:organization_onboarding, :failed, created_by: @user)
+
+        delete organization_onboarding_path(as: @user)
+
+        assert_redirected_to dashboard_path
+        refute OrganizationOnboarding.exists?(id: organization_onboarding.id)
+      end
+    end
+
+    context "when the current user has not started onboarding" do
+      should "redirect to the dashboard" do
+        assert_no_difference -> { OrganizationOnboarding.count } do
+          delete organization_onboarding_path(as: @user)
+        end
+
+        assert_redirected_to dashboard_path
+      end
+    end
+  end
+end