From b63b64ef112b000763ed9bab2633a6489db30d11 Mon Sep 17 00:00:00 2001 From: Martin Emde Date: Tue, 3 Dec 2024 16:51:05 -0800 Subject: [PATCH] Cleanup onboarding tests, convert to ActionDispatch::IntegrationTest --- .../onboarding/base_controller.rb | 2 + .../onboarding/confirm_controller.rb | 2 - .../onboarding/gems_controller.rb | 2 - .../onboarding/name_controller.rb | 2 - .../onboarding/users_controller.rb | 2 - .../onboarding/gems_controller_test.rb | 12 +- .../onboarding/name_controller_test.rb | 14 +- .../onboarding/users_controller_test.rb | 153 +++++++++--------- .../onboarding_controller_test.rb | 51 +++--- 9 files changed, 120 insertions(+), 120 deletions(-) diff --git a/app/controllers/organizations/onboarding/base_controller.rb b/app/controllers/organizations/onboarding/base_controller.rb index 67eae35ded2..fbc31cc22fd 100644 --- a/app/controllers/organizations/onboarding/base_controller.rb +++ b/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 diff --git a/app/controllers/organizations/onboarding/confirm_controller.rb b/app/controllers/organizations/onboarding/confirm_controller.rb index bba2ae80f4f..2f5c955494b 100644 --- a/app/controllers/organizations/onboarding/confirm_controller.rb +++ b/app/controllers/organizations/onboarding/confirm_controller.rb @@ -1,6 +1,4 @@ class Organizations::Onboarding::ConfirmController < Organizations::Onboarding::BaseController - layout "onboarding" - def edit end diff --git a/app/controllers/organizations/onboarding/gems_controller.rb b/app/controllers/organizations/onboarding/gems_controller.rb index c80825ee2f8..9db652bb255 100644 --- a/app/controllers/organizations/onboarding/gems_controller.rb +++ b/app/controllers/organizations/onboarding/gems_controller.rb @@ -1,6 +1,4 @@ class Organizations::Onboarding::GemsController < Organizations::Onboarding::BaseController - layout "onboarding" - def edit end diff --git a/app/controllers/organizations/onboarding/name_controller.rb b/app/controllers/organizations/onboarding/name_controller.rb index e61ccf376fe..c9e4c2c1713 100644 --- a/app/controllers/organizations/onboarding/name_controller.rb +++ b/app/controllers/organizations/onboarding/name_controller.rb @@ -1,6 +1,4 @@ class Organizations::Onboarding::NameController < Organizations::Onboarding::BaseController - layout "onboarding" - def new end diff --git a/app/controllers/organizations/onboarding/users_controller.rb b/app/controllers/organizations/onboarding/users_controller.rb index 94dc7d3d571..a1431ee24bd 100644 --- a/app/controllers/organizations/onboarding/users_controller.rb +++ b/app/controllers/organizations/onboarding/users_controller.rb @@ -1,6 +1,4 @@ class Organizations::Onboarding::UsersController < Organizations::Onboarding::BaseController - layout "onboarding" - def edit end diff --git a/test/functional/organizations/onboarding/gems_controller_test.rb b/test/functional/organizations/onboarding/gems_controller_test.rb index a17bf7a5ab8..715e89ee870 100644 --- a/test/functional/organizations/onboarding/gems_controller_test.rb +++ b/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,27 +13,25 @@ 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 @@ -41,7 +39,7 @@ class Organizations::Onboarding::GemsControllerTest < ActionController::TestCase 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 diff --git a/test/functional/organizations/onboarding/name_controller_test.rb b/test/functional/organizations/onboarding/name_controller_test.rb index b7c091cd762..68033ad9af3 100644 --- a/test/functional/organizations/onboarding/name_controller_test.rb +++ b/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 diff --git a/test/functional/organizations/onboarding/users_controller_test.rb b/test/functional/organizations/onboarding/users_controller_test.rb index 97eaeb5f1c2..9dfef146030 100644 --- a/test/functional/organizations/onboarding/users_controller_test.rb +++ b/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 diff --git a/test/functional/organizations/onboarding_controller_test.rb b/test/functional/organizations/onboarding_controller_test.rb index a549d660992..1d6d34e4d7c 100644 --- a/test/functional/organizations/onboarding_controller_test.rb +++ b/test/functional/organizations/onboarding_controller_test.rb @@ -2,50 +2,59 @@ class Organizations::OnboardingControllerTest < ActionDispatch::IntegrationTest setup do - @user = create(:user, remember_token_expires_at: Gemcutter::REMEMBER_FOR.from_now) - post session_path(session: { who: @user.handle, password: PasswordHelpers::SECURE_TEST_PASSWORD }) + @user = create(:user) end context "GET /organizations/onboarding" do should "redirect to onboarding start page" do - get organization_onboarding_path + get organization_onboarding_path(as: @user) assert_redirected_to organization_onboarding_name_path end end context "DELETE /organizations/onboarding" do - should "not destroy an OrganizationOnboarding that is already completed" do - organization_onboarding = create(:organization_onboarding, :completed, created_by: @user) + 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 + delete organization_onboarding_path(as: @user) - assert_redirected_to dashboard_path - assert OrganizationOnboarding.exists?(id: organization_onboarding.id) + assert_redirected_to dashboard_path + assert OrganizationOnboarding.exists?(id: organization_onboarding.id) + end end - should "destroy a pending OrganizationOnboarding created by the current user" do - organization_onboarding = create(:organization_onboarding, created_by: @user) + 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 + delete organization_onboarding_path(as: @user) - assert_redirected_to dashboard_path - refute OrganizationOnboarding.exists?(id: organization_onboarding.id) + assert_redirected_to dashboard_path + refute OrganizationOnboarding.exists?(id: organization_onboarding.id) + end end - should "destroy a failed OrganizationOnboarding created by the current user" do - organization_onboarding = create(:organization_onboarding, :failed, created_by: @user) + 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 + delete organization_onboarding_path(as: @user) - assert_redirected_to dashboard_path - refute OrganizationOnboarding.exists?(id: organization_onboarding.id) + assert_redirected_to dashboard_path + refute OrganizationOnboarding.exists?(id: organization_onboarding.id) + end end - should "redirect to the dashboarding if the current user has not started organization onboarding" do - delete organization_onboarding_path + 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 + assert_redirected_to dashboard_path + end end end end