diff options
Diffstat (limited to 'spec/controllers')
38 files changed, 789 insertions, 1230 deletions
diff --git a/spec/controllers/admin/application_settings_controller_spec.rb b/spec/controllers/admin/application_settings_controller_spec.rb index 537424093fb..60343c822af 100644 --- a/spec/controllers/admin/application_settings_controller_spec.rb +++ b/spec/controllers/admin/application_settings_controller_spec.rb @@ -487,6 +487,43 @@ RSpec.describe Admin::ApplicationSettingsController, :do_not_mock_admin_mode_set end end + describe 'GET #slack_app_manifest_download', feature_category: :integrations do + before do + sign_in(admin) + end + + subject { get :slack_app_manifest_download } + + it 'downloads the GitLab for Slack app manifest' do + allow(Slack::Manifest).to receive(:to_h).and_return({ foo: 'bar' }) + + subject + + expect(response.body).to eq('{"foo":"bar"}') + expect(response.headers['Content-Disposition']).to eq( + 'attachment; filename="slack_manifest.json"; filename*=UTF-8\'\'slack_manifest.json' + ) + end + end + + describe 'GET #slack_app_manifest_share', feature_category: :integrations do + before do + sign_in(admin) + end + + subject { get :slack_app_manifest_share } + + it 'redirects the user to the Slack Manifest share URL' do + allow(Slack::Manifest).to receive(:to_h).and_return({ foo: 'bar' }) + + subject + + expect(response).to redirect_to( + "https://api.slack.com/apps?new_app=1&manifest_json=%7B%22foo%22%3A%22bar%22%7D" + ) + end + end + describe 'GET #service_usage_data', feature_category: :service_ping do before do stub_usage_data_connections diff --git a/spec/controllers/admin/runners_controller_spec.rb b/spec/controllers/admin/runners_controller_spec.rb index b1a2d90589a..6fa8d2c61c1 100644 --- a/spec/controllers/admin/runners_controller_spec.rb +++ b/spec/controllers/admin/runners_controller_spec.rb @@ -41,18 +41,6 @@ RSpec.describe Admin::RunnersController, feature_category: :runner_fleet do expect(response).to have_gitlab_http_status(:ok) expect(response).to render_template(:new) end - - context 'when create_runner_workflow_for_admin is disabled' do - before do - stub_feature_flags(create_runner_workflow_for_admin: false) - end - - it 'returns :not_found' do - get :new - - expect(response).to have_gitlab_http_status(:not_found) - end - end end describe '#register' do @@ -78,20 +66,6 @@ RSpec.describe Admin::RunnersController, feature_category: :runner_fleet do expect(response).to have_gitlab_http_status(:not_found) end end - - context 'when create_runner_workflow_for_admin is disabled' do - let_it_be(:new_runner) { create(:ci_runner, registration_type: :authenticated_user) } - - before do - stub_feature_flags(create_runner_workflow_for_admin: false) - end - - it 'returns :not_found' do - register - - expect(response).to have_gitlab_http_status(:not_found) - end - end end describe '#edit' do diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 58125f3a831..0beaae7a2d7 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -567,20 +567,6 @@ RSpec.describe ApplicationController, feature_category: :shared do expect(controller.last_payload[:response_bytes]).to eq('authenticated'.bytesize) end - - context 'with log_response_length disabled' do - before do - stub_feature_flags(log_response_length: false) - end - - it 'logs response length' do - sign_in user - - get :index - - expect(controller.last_payload).not_to include(:response_bytes) - end - end end describe '#access_denied' do diff --git a/spec/controllers/concerns/issuable_collections_spec.rb b/spec/controllers/concerns/issuable_collections_spec.rb index 6fa273bf3d7..9eb0f36cb37 100644 --- a/spec/controllers/concerns/issuable_collections_spec.rb +++ b/spec/controllers/concerns/issuable_collections_spec.rb @@ -92,7 +92,7 @@ RSpec.describe IssuableCollections do } end - it 'only allows whitelisted params' do + it 'only allows allowlisted params' do is_expected.to include({ 'assignee_id' => '1', 'assignee_username' => 'user1', @@ -123,7 +123,7 @@ RSpec.describe IssuableCollections do } end - it 'only allows whitelisted params' do + it 'only allows allowlisted params' do is_expected.to include({ 'label_name' => %w[label1 label2], 'assignee_username' => %w[user1 user2] diff --git a/spec/controllers/concerns/kas_cookie_spec.rb b/spec/controllers/concerns/kas_cookie_spec.rb index 7ab48f12d83..c9490508690 100644 --- a/spec/controllers/concerns/kas_cookie_spec.rb +++ b/spec/controllers/concerns/kas_cookie_spec.rb @@ -42,14 +42,6 @@ RSpec.describe KasCookie, feature_category: :deployment_management do expect(kas_cookie).to eq('foobar') expect(::Gitlab::Kas::UserAccess).to have_received(:cookie_data) end - - context 'when feature flag is disabled' do - before do - stub_feature_flags(kas_user_access: false) - end - - it { is_expected.to be_blank } - end end end @@ -88,60 +80,42 @@ RSpec.describe KasCookie, feature_category: :deployment_management do request.env['action_dispatch.content_security_policy'].directives['connect-src'] end - context "when feature flag is disabled" do + context 'when KAS is on same domain as rails' do let_it_be(:kas_tunnel_url) { 'ws://gitlab.example.com/-/k8s-proxy/' } - before do - stub_feature_flags(kas_user_access: false) - end - - it 'does not add KAS url to connect-src directives' do + it 'does not add KAS url to CSP connect-src directive' do expect(kas_csp_connect_src).not_to include(::Gitlab::Kas.tunnel_url) end end - context 'when feature flag is enabled' do - before do - stub_feature_flags(kas_user_access: true) + context 'when KAS is on subdomain' do + let_it_be(:kas_tunnel_url) { 'ws://kas.gitlab.example.com/k8s-proxy/' } + + it 'adds KAS url to CSP connect-src directive' do + expect(kas_csp_connect_src).to include(::Gitlab::Kas.tunnel_url) end - context 'when KAS is on same domain as rails' do - let_it_be(:kas_tunnel_url) { 'ws://gitlab.example.com/-/k8s-proxy/' } + context 'when content_security_policy is disabled' do + let(:content_security_policy_enabled) { false } it 'does not add KAS url to CSP connect-src directive' do expect(kas_csp_connect_src).not_to include(::Gitlab::Kas.tunnel_url) end end + end - context 'when KAS is on subdomain' do - let_it_be(:kas_tunnel_url) { 'ws://kas.gitlab.example.com/k8s-proxy/' } - - it 'adds KAS url to CSP connect-src directive' do - expect(kas_csp_connect_src).to include(::Gitlab::Kas.tunnel_url) - end - - context 'when content_security_policy is disabled' do - let(:content_security_policy_enabled) { false } + context 'when KAS tunnel url is configured without trailing slash' do + let_it_be(:kas_tunnel_url) { 'ws://kas.gitlab.example.com/k8s-proxy' } - it 'does not add KAS url to CSP connect-src directive' do - expect(kas_csp_connect_src).not_to include(::Gitlab::Kas.tunnel_url) - end - end + it 'adds KAS url to CSP connect-src directive with trailing slash' do + expect(kas_csp_connect_src).to include("#{::Gitlab::Kas.tunnel_url}/") end - context 'when KAS tunnel url is configured without trailing slash' do - let_it_be(:kas_tunnel_url) { 'ws://kas.gitlab.example.com/k8s-proxy' } + context 'when content_security_policy is disabled' do + let(:content_security_policy_enabled) { false } - it 'adds KAS url to CSP connect-src directive with trailing slash' do - expect(kas_csp_connect_src).to include("#{::Gitlab::Kas.tunnel_url}/") - end - - context 'when content_security_policy is disabled' do - let(:content_security_policy_enabled) { false } - - it 'does not add KAS url to CSP connect-src directive' do - expect(kas_csp_connect_src).not_to include("#{::Gitlab::Kas.tunnel_url}/") - end + it 'does not add KAS url to CSP connect-src directive' do + expect(kas_csp_connect_src).not_to include("#{::Gitlab::Kas.tunnel_url}/") end end end diff --git a/spec/controllers/concerns/metrics_dashboard_spec.rb b/spec/controllers/concerns/metrics_dashboard_spec.rb deleted file mode 100644 index 4a9c7c493a7..00000000000 --- a/spec/controllers/concerns/metrics_dashboard_spec.rb +++ /dev/null @@ -1,195 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe MetricsDashboard, feature_category: :metrics do - include MetricsDashboardHelpers - - describe 'GET #metrics_dashboard' do - let_it_be(:user) { create(:user) } - let_it_be(:project) { project_with_dashboard('.gitlab/dashboards/test.yml') } - let_it_be(:environment) { create(:environment, project: project) } - - before do - stub_feature_flags(remove_monitor_metrics: false) - sign_in(user) - project.add_maintainer(user) - end - - controller(::ApplicationController) do - include MetricsDashboard - end - - let(:json_response) do - routes.draw { get "metrics_dashboard" => "anonymous#metrics_dashboard" } - response = get :metrics_dashboard, format: :json - - response.parsed_body - end - - context 'when no parameters are provided' do - it 'returns an error json_response' do - expect(json_response['status']).to eq('error') - end - end - - context 'when params are provided' do - let(:params) { { environment: environment } } - - before do - allow(controller).to receive(:project).and_return(project) - allow(controller) - .to receive(:metrics_dashboard_params) - .and_return(params) - end - - it 'returns the specified dashboard' do - expect(json_response['dashboard']['dashboard']).to eq('Environment metrics') - expect(json_response).not_to have_key('all_dashboards') - expect(json_response).to have_key('metrics_data') - end - - context 'when the params are in an alternate format' do - let(:params) { ActionController::Parameters.new({ environment: environment }).permit! } - - it 'returns the specified dashboard' do - expect(json_response['dashboard']['dashboard']).to eq('Environment metrics') - expect(json_response).not_to have_key('all_dashboards') - expect(json_response).to have_key('metrics_data') - end - end - - context 'when environment for dashboard is available' do - let(:params) { { environment: environment } } - - before do - allow(controller).to receive(:project).and_return(project) - allow(controller).to receive(:environment).and_return(environment) - allow(controller) - .to receive(:metrics_dashboard_params) - .and_return(params) - end - - it 'returns the specified dashboard' do - expect(json_response['dashboard']['dashboard']).to eq('Environment metrics') - expect(json_response).not_to have_key('all_dashboards') - expect(json_response).to have_key('metrics_data') - end - end - - context 'when dashboard path includes encoded characters' do - let(:params) { { dashboard_path: 'dashboard%26copy.yml' } } - - before do - allow(controller) - .to receive(:metrics_dashboard_params) - .and_return(params) - end - - it 'decodes dashboard path' do - expect(::Gitlab::Metrics::Dashboard::Finder).to receive(:find).with(anything, anything, hash_including(dashboard_path: 'dashboard©.yml')) - - json_response - end - end - - context 'when parameters are provided and the list of all dashboards is required' do - before do - allow(controller).to receive(:include_all_dashboards?).and_return(true) - end - - it 'returns a dashboard in addition to the list of dashboards' do - expect(json_response['dashboard']['dashboard']).to eq('Environment metrics') - expect(json_response).to have_key('all_dashboards') - end - - context 'in all_dashboard list' do - let(:system_dashboard) { json_response['all_dashboards'].find { |dashboard| dashboard["system_dashboard"] == true } } - - let(:project_dashboard) do - json_response['all_dashboards'].find do |dashboard| - dashboard['path'] == '.gitlab/dashboards/test.yml' - end - end - - it 'includes project_blob_path only for project dashboards' do - expect(system_dashboard['project_blob_path']).to be_nil - expect(project_dashboard['project_blob_path']).to eq("/#{project.namespace.path}/#{project.path}/-/blob/master/.gitlab/dashboards/test.yml") - end - - it 'allows editing only for project dashboards' do - expect(system_dashboard['can_edit']).to be(false) - expect(project_dashboard['can_edit']).to be(true) - end - - it 'includes out_of_the_box_dashboard key' do - expect(system_dashboard['out_of_the_box_dashboard']).to be(true) - expect(project_dashboard['out_of_the_box_dashboard']).to be(false) - end - - describe 'project permissions' do - using RSpec::Parameterized::TableSyntax - - where(:can_collaborate, :system_can_edit, :project_can_edit) do - false | false | false - true | false | true - end - - with_them do - before do - allow(controller).to receive(:can_collaborate_with_project?).and_return(can_collaborate) - end - - it "sets can_edit appropriately" do - expect(system_dashboard["can_edit"]).to eq(system_can_edit) - expect(project_dashboard["can_edit"]).to eq(project_can_edit) - end - end - end - - context 'starred dashboards' do - let_it_be(:dashboard_yml) { fixture_file('lib/gitlab/metrics/dashboard/sample_dashboard.yml') } - let_it_be(:dashboards) do - { - '.gitlab/dashboards/test.yml' => dashboard_yml, - '.gitlab/dashboards/anomaly.yml' => dashboard_yml, - '.gitlab/dashboards/errors.yml' => dashboard_yml - } - end - - let_it_be(:project) { create(:project, :custom_repo, files: dashboards) } - - before do - create(:metrics_users_starred_dashboard, user: user, project: project, dashboard_path: '.gitlab/dashboards/errors.yml') - create(:metrics_users_starred_dashboard, user: user, project: project, dashboard_path: '.gitlab/dashboards/test.yml') - end - - it 'adds starred dashboard information and sorts the list' do - all_dashboards = json_response['all_dashboards'].map { |dashboard| dashboard.slice('display_name', 'starred', 'user_starred_path') } - expected_response = [ - { "display_name" => "anomaly.yml", "starred" => false, 'user_starred_path' => api_v4_projects_metrics_user_starred_dashboards_path(id: project.id, params: { dashboard_path: '.gitlab/dashboards/anomaly.yml' }) }, - { "display_name" => "errors.yml", "starred" => true, 'user_starred_path' => api_v4_projects_metrics_user_starred_dashboards_path(id: project.id, params: { dashboard_path: '.gitlab/dashboards/errors.yml' }) }, - { "display_name" => "K8s pod health", "starred" => false, 'user_starred_path' => api_v4_projects_metrics_user_starred_dashboards_path(id: project.id, params: { dashboard_path: 'config/prometheus/pod_metrics.yml' }) }, - { "display_name" => "Overview", "starred" => false, 'user_starred_path' => api_v4_projects_metrics_user_starred_dashboards_path(id: project.id, params: { dashboard_path: 'config/prometheus/common_metrics.yml' }) }, - { "display_name" => "test.yml", "starred" => true, 'user_starred_path' => api_v4_projects_metrics_user_starred_dashboards_path(id: project.id, params: { dashboard_path: '.gitlab/dashboards/test.yml' }) } - ] - - expect(all_dashboards).to eq(expected_response) - end - end - end - end - end - - context 'when metrics dashboard feature is unavailable' do - it 'returns 404 not found' do - stub_feature_flags(remove_monitor_metrics: true) - - routes.draw { get "metrics_dashboard" => "anonymous#metrics_dashboard" } - response = get :metrics_dashboard, format: :json - - expect(response).to have_gitlab_http_status(:not_found) - end - end - end -end diff --git a/spec/controllers/concerns/onboarding/status_spec.rb b/spec/controllers/concerns/onboarding/status_spec.rb new file mode 100644 index 00000000000..3f6e597a235 --- /dev/null +++ b/spec/controllers/concerns/onboarding/status_spec.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Onboarding::Status, feature_category: :onboarding do + let_it_be(:member) { create(:group_member) } + let_it_be(:user) { member.user } + let_it_be(:tasks_to_be_done) { %w[ci code] } + let_it_be(:source) { member.group } + + describe '#continue_full_onboarding?' do + subject { described_class.new(nil).continue_full_onboarding? } + + it { is_expected.to eq(false) } + end + + describe '#single_invite?' do + subject { described_class.new(user).single_invite? } + + context 'when there is only one member for the user' do + context 'when the member source exists' do + it { is_expected.to eq(true) } + end + end + + context 'when there is more than one member for the user' do + before do + create(:group_member, user: user) + end + + it { is_expected.to eq(false) } + end + + context 'when there are no members for the user' do + let(:user) { build_stubbed(:user) } + + it { is_expected.to eq(false) } + end + end + + describe '#last_invited_member' do + subject { described_class.new(user).last_invited_member } + + it { is_expected.to eq(member) } + + context 'when another member exists and is most recent' do + let!(:last_member) { create(:group_member, user: user) } + + it { is_expected.to eq(last_member) } + end + + context 'when there are no members' do + let_it_be(:user) { build_stubbed(:user) } + + it { is_expected.to be_nil } + end + end + + describe '#last_invited_member_source' do + subject { described_class.new(user).last_invited_member_source } + + context 'when a member exists' do + it { is_expected.to eq(source) } + end + + context 'when no members exist' do + let_it_be(:user) { build_stubbed(:user) } + + it { is_expected.to be_nil } + end + + context 'when another member exists and is most recent' do + let!(:last_member_source) { create(:group_member, user: user).group } + + it { is_expected.to eq(last_member_source) } + end + end + + describe '#invite_with_tasks_to_be_done?' do + subject { described_class.new(user).invite_with_tasks_to_be_done? } + + context 'when there are tasks_to_be_done with one member' do + let_it_be(:member) { create(:group_member, user: user, tasks_to_be_done: tasks_to_be_done) } + + it { is_expected.to eq(true) } + end + + context 'when there are multiple members and the tasks_to_be_done is on only one of them' do + before do + create(:group_member, user: user, tasks_to_be_done: tasks_to_be_done) + end + + it { is_expected.to eq(true) } + end + + context 'when there are no tasks_to_be_done' do + it { is_expected.to eq(false) } + end + + context 'when there are no members' do + let_it_be(:user) { build_stubbed(:user) } + + it { is_expected.to eq(false) } + end + end +end diff --git a/spec/controllers/dashboard_controller_spec.rb b/spec/controllers/dashboard_controller_spec.rb index 304e08f40bd..52f6f08e17a 100644 --- a/spec/controllers/dashboard_controller_spec.rb +++ b/spec/controllers/dashboard_controller_spec.rb @@ -16,22 +16,7 @@ RSpec.describe DashboardController, feature_category: :code_review_workflow do end describe 'GET issues' do - context 'when issues_full_text_search is disabled' do - before do - stub_feature_flags(issues_full_text_search: false) - end - - it_behaves_like 'issuables list meta-data', :issue, :issues - end - - context 'when issues_full_text_search is enabled' do - before do - stub_feature_flags(issues_full_text_search: true) - end - - it_behaves_like 'issuables list meta-data', :issue, :issues - end - + it_behaves_like 'issuables list meta-data', :issue, :issues it_behaves_like 'issuables requiring filter', :issues it 'includes tasks in issue list' do diff --git a/spec/controllers/explore/projects_controller_spec.rb b/spec/controllers/explore/projects_controller_spec.rb index e73e115b77d..68ae1ca218b 100644 --- a/spec/controllers/explore/projects_controller_spec.rb +++ b/spec/controllers/explore/projects_controller_spec.rb @@ -122,6 +122,59 @@ RSpec.describe Explore::ProjectsController, feature_category: :groups_and_projec end end end + + describe 'GET #topic.atom' do + context 'when topic does not exist' do + it 'renders a 404 error' do + get :topic, format: :atom, params: { topic_name: 'topic1' } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when topic exists' do + let(:topic) { create(:topic, name: 'topic1') } + let_it_be(:older_project) { create(:project, :public, updated_at: 1.day.ago) } + let_it_be(:newer_project) { create(:project, :public, updated_at: 2.days.ago) } + + before do + create(:project_topic, project: older_project, topic: topic) + create(:project_topic, project: newer_project, topic: topic) + end + + it 'renders the template' do + get :topic, format: :atom, params: { topic_name: 'topic1' } + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template('topic', layout: :xml) + end + + it 'sorts repos by descending creation date' do + get :topic, format: :atom, params: { topic_name: 'topic1' } + + expect(assigns(:projects)).to match_array [newer_project, older_project] + end + + it 'finds topic by case insensitive name' do + get :topic, format: :atom, params: { topic_name: 'TOPIC1' } + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template('topic', layout: :xml) + end + + describe 'when topic contains more than 20 projects' do + before do + create_list(:project, 22, :public, topics: [topic]) + end + + it 'does not assigns more than 20 projects' do + get :topic, format: :atom, params: { topic_name: 'topic1' } + + expect(assigns(:projects).count).to be(20) + end + end + end + end end shared_examples "blocks high page numbers" do diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb index b4a7e41ccd2..be47b32ec4f 100644 --- a/spec/controllers/graphql_controller_spec.rb +++ b/spec/controllers/graphql_controller_spec.rb @@ -431,6 +431,13 @@ RSpec.describe GraphqlController, feature_category: :integrations do context 'when querying an IntrospectionQuery', :use_clean_rails_memory_store_caching do let_it_be(:query) { File.read(Rails.root.join('spec/fixtures/api/graphql/introspection.graphql')) } + it 'caches IntrospectionQuery even when operationName is not given' do + expect(GitlabSchema).to receive(:execute).exactly(:once) + + post :execute, params: { query: query } + post :execute, params: { query: query } + end + it 'caches the IntrospectionQuery' do expect(GitlabSchema).to receive(:execute).exactly(:once) @@ -461,35 +468,9 @@ RSpec.describe GraphqlController, feature_category: :integrations do post :execute, params: { query: query, operationName: 'IntrospectionQuery' } end - it 'logs that it will try to hit the cache' do - expect(Gitlab::AppLogger).to receive(:info).with(message: "IntrospectionQueryCache hit") - expect(Gitlab::AppLogger).to receive(:info).with( - message: "IntrospectionQueryCache", - can_use_introspection_query_cache: "true", - query: query.to_s, - variables: "{}", - introspection_query_cache_key: "[\"introspection-query-cache\", \"#{Gitlab.revision}\", false]" - ) - - post :execute, params: { query: query, operationName: 'IntrospectionQuery' } - end - context 'when there is an unknown introspection query' do let(:query) { File.read(Rails.root.join('spec/fixtures/api/graphql/fake_introspection.graphql')) } - it 'logs that it did not try to hit the cache' do - expect(Gitlab::AppLogger).to receive(:info).with(message: "IntrospectionQueryCache miss") - expect(Gitlab::AppLogger).to receive(:info).with( - message: "IntrospectionQueryCache", - can_use_introspection_query_cache: "false", - query: query.to_s, - variables: "{}", - introspection_query_cache_key: "[\"introspection-query-cache\", \"#{Gitlab.revision}\", false]" - ) - - post :execute, params: { query: query, operationName: 'IntrospectionQuery' } - end - it 'does not cache an unknown introspection query' do expect(GitlabSchema).to receive(:execute).exactly(:twice) diff --git a/spec/controllers/groups/children_controller_spec.rb b/spec/controllers/groups/children_controller_spec.rb index ee8b2dce298..82dd8c18cfd 100644 --- a/spec/controllers/groups/children_controller_spec.rb +++ b/spec/controllers/groups/children_controller_spec.rb @@ -222,13 +222,13 @@ RSpec.describe Groups::ChildrenController, feature_category: :groups_and_project control = ActiveRecord::QueryRecorder.new { get_list } _new_project = create(:project, :public, namespace: group) - expect { get_list }.not_to exceed_query_limit(control).with_threshold(expected_queries_per_project) + expect { get_list }.not_to exceed_query_limit(control).with_threshold(expected_queries_per_project + 1) end context 'when rendering hierarchies' do # When loading hierarchies we load the all the ancestors for matched projects - # in 2 separate queries - let(:extra_queries_for_hierarchies) { 2 } + # in 3 separate queries + let(:extra_queries_for_hierarchies) { 3 } def get_filtered_list get :index, params: { group_id: group.to_param, filter: 'filter' }, format: :json diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index fe4b80e12fe..feebdd972aa 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -234,7 +234,7 @@ RSpec.describe Groups::GroupMembersController do it 'returns correct json response' do expect(json_response).to eq({ "expires_soon" => false, - "expires_at_formatted" => expiry_date.to_time.in_time_zone.to_s(:medium) + "expires_at_formatted" => expiry_date.to_time.in_time_zone.to_fs(:medium) }) end end diff --git a/spec/controllers/groups/runners_controller_spec.rb b/spec/controllers/groups/runners_controller_spec.rb index 9ae5cb6f87c..37242bce6bf 100644 --- a/spec/controllers/groups/runners_controller_spec.rb +++ b/spec/controllers/groups/runners_controller_spec.rb @@ -65,52 +65,28 @@ RSpec.describe Groups::RunnersController, feature_category: :runner_fleet do end describe '#new' do - context 'when create_runner_workflow_for_namespace is enabled' do + context 'when user is owner' do before do - stub_feature_flags(create_runner_workflow_for_namespace: [group]) - end - - context 'when user is owner' do - before do - group.add_owner(user) - end - - it 'renders new with 200 status code' do - get :new, params: { group_id: group } - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:new) - end + group.add_owner(user) end - context 'when user is not owner' do - before do - group.add_maintainer(user) - end - - it 'renders a 404' do - get :new, params: { group_id: group } + it 'renders new with 200 status code' do + get :new, params: { group_id: group } - expect(response).to have_gitlab_http_status(:not_found) - end + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:new) end end - context 'when create_runner_workflow_for_namespace is disabled' do + context 'when user is not owner' do before do - stub_feature_flags(create_runner_workflow_for_namespace: false) + group.add_maintainer(user) end - context 'when user is owner' do - before do - group.add_owner(user) - end - - it 'renders a 404' do - get :new, params: { group_id: group } + it 'renders a 404' do + get :new, params: { group_id: group } - expect(response).to have_gitlab_http_status(:not_found) - end + expect(response).to have_gitlab_http_status(:not_found) end end end @@ -118,66 +94,40 @@ RSpec.describe Groups::RunnersController, feature_category: :runner_fleet do describe '#register' do subject(:register) { get :register, params: { group_id: group, id: new_runner } } - context 'when create_runner_workflow_for_namespace is enabled' do + context 'when user is owner' do before do - stub_feature_flags(create_runner_workflow_for_namespace: [group]) + group.add_owner(user) end - context 'when user is owner' do - before do - group.add_owner(user) - end - - context 'when runner can be registered after creation' do - let_it_be(:new_runner) { create(:ci_runner, :group, groups: [group], registration_type: :authenticated_user) } + context 'when runner can be registered after creation' do + let_it_be(:new_runner) { create(:ci_runner, :group, groups: [group], registration_type: :authenticated_user) } - it 'renders a :register template' do - register - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:register) - end - end - - context 'when runner cannot be registered after creation' do - let_it_be(:new_runner) { runner } - - it 'returns :not_found' do - register + it 'renders a :register template' do + register - expect(response).to have_gitlab_http_status(:not_found) - end + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:register) end end - context 'when user is not owner' do - before do - group.add_maintainer(user) - end - - context 'when runner can be registered after creation' do - let_it_be(:new_runner) { create(:ci_runner, :group, groups: [group], registration_type: :authenticated_user) } + context 'when runner cannot be registered after creation' do + let_it_be(:new_runner) { runner } - it 'returns :not_found' do - register + it 'returns :not_found' do + register - expect(response).to have_gitlab_http_status(:not_found) - end + expect(response).to have_gitlab_http_status(:not_found) end end end - context 'when create_runner_workflow_for_namespace is disabled' do - let_it_be(:new_runner) { create(:ci_runner, :group, groups: [group], registration_type: :authenticated_user) } - + context 'when user is not owner' do before do - stub_feature_flags(create_runner_workflow_for_namespace: false) + group.add_maintainer(user) end - context 'when user is owner' do - before do - group.add_owner(user) - end + context 'when runner can be registered after creation' do + let_it_be(:new_runner) { create(:ci_runner, :group, groups: [group], registration_type: :authenticated_user) } it 'returns :not_found' do register diff --git a/spec/controllers/groups/uploads_controller_spec.rb b/spec/controllers/groups/uploads_controller_spec.rb index 6649e8f057c..7795fff5541 100644 --- a/spec/controllers/groups/uploads_controller_spec.rb +++ b/spec/controllers/groups/uploads_controller_spec.rb @@ -76,31 +76,17 @@ RSpec.describe Groups::UploadsController do context 'when uploader class does not match the upload' do let(:uploader_class) { FileUploader } - it 'responds with status 200 but logs a deprecation message' do - expect(Gitlab::AppJsonLogger).to receive(:info).with( - message: 'Deprecated usage of build_uploader_from_params', - uploader_class: uploader_class.name, - path: filename, - exists: true - ) - + it 'responds with status 404' do show_upload - expect(response).to have_gitlab_http_status(:ok) + expect(response).to have_gitlab_http_status(:not_found) end end context 'when filename does not match' do let(:invalid_filename) { 'invalid_filename.jpg' } - it 'responds with status 404 and logs a deprecation message' do - expect(Gitlab::AppJsonLogger).to receive(:info).with( - message: 'Deprecated usage of build_uploader_from_params', - uploader_class: uploader_class.name, - path: invalid_filename, - exists: false - ) - + it 'responds with status 404' do get :show, params: params.merge(secret: secret, filename: invalid_filename) expect(response).to have_gitlab_http_status(:not_found) diff --git a/spec/controllers/import/fogbugz_controller_spec.rb b/spec/controllers/import/fogbugz_controller_spec.rb index 3b099ba2613..273dfd6a9c7 100644 --- a/spec/controllers/import/fogbugz_controller_spec.rb +++ b/spec/controllers/import/fogbugz_controller_spec.rb @@ -17,7 +17,7 @@ RSpec.describe Import::FogbugzController, feature_category: :importers do end describe 'POST #callback' do - let(:xml_response) { %Q(<?xml version=\"1.0\" encoding=\"UTF-8\"?><response><token><![CDATA[#{token}]]></token></response>) } + let(:xml_response) { %(<?xml version=\"1.0\" encoding=\"UTF-8\"?><response><token><![CDATA[#{token}]]></token></response>) } before do stub_request(:post, "https://example.com/api.asp").to_return(status: 200, body: xml_response, headers: {}) diff --git a/spec/controllers/oauth/authorizations_controller_spec.rb b/spec/controllers/oauth/authorizations_controller_spec.rb index 3476c7b8465..4772c3f3487 100644 --- a/spec/controllers/oauth/authorizations_controller_spec.rb +++ b/spec/controllers/oauth/authorizations_controller_spec.rb @@ -298,4 +298,15 @@ RSpec.describe Oauth::AuthorizationsController do it 'includes Two-factor enforcement concern' do expect(described_class.included_modules.include?(EnforcesTwoFactorAuthentication)).to eq(true) end + + describe 'Gon variables' do + it 'adds Gon variables' do + expect(controller).to receive(:add_gon_variables) + get :new, params: params + end + + it 'includes GonHelper module' do + expect(controller).to be_a(Gitlab::GonHelper) + end + end end diff --git a/spec/controllers/projects/alert_management_controller_spec.rb b/spec/controllers/projects/alert_management_controller_spec.rb deleted file mode 100644 index d80147b5c59..00000000000 --- a/spec/controllers/projects/alert_management_controller_spec.rb +++ /dev/null @@ -1,59 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Projects::AlertManagementController do - let_it_be(:project) { create(:project) } - let_it_be(:role) { :developer } - let_it_be(:user) { create(:user) } - let_it_be(:id) { 1 } - - before do - project.add_role(user, role) - sign_in(user) - end - - describe 'GET #index' do - it 'shows the page' do - get :index, params: { namespace_id: project.namespace, project_id: project } - - expect(response).to have_gitlab_http_status(:ok) - end - - context 'when user is unauthorized' do - let(:role) { :reporter } - - it 'shows 404' do - get :index, params: { namespace_id: project.namespace, project_id: project } - - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - - describe 'GET #details' do - it 'shows the page' do - get :details, params: { namespace_id: project.namespace, project_id: project, id: id } - - expect(response).to have_gitlab_http_status(:ok) - end - - context 'when user is unauthorized' do - let(:role) { :reporter } - - it 'shows 404' do - get :details, params: { namespace_id: project.namespace, project_id: project, id: id } - - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - - describe 'set_alert_id' do - it 'sets alert id from the route' do - get :details, params: { namespace_id: project.namespace, project_id: project, id: id } - - expect(assigns(:alert_id)).to eq(id.to_s) - end - end -end diff --git a/spec/controllers/projects/artifacts_controller_spec.rb b/spec/controllers/projects/artifacts_controller_spec.rb index c7b74b5cf68..44615506e5d 100644 --- a/spec/controllers/projects/artifacts_controller_spec.rb +++ b/spec/controllers/projects/artifacts_controller_spec.rb @@ -104,7 +104,7 @@ RSpec.describe Projects::ArtifactsController, feature_category: :build_artifacts download_artifact - expect(response.headers['Content-Disposition']).to eq(%Q(attachment; filename="#{filename}"; filename*=UTF-8''#{filename})) + expect(response.headers['Content-Disposition']).to eq(%(attachment; filename="#{filename}"; filename*=UTF-8''#{filename})) end end @@ -135,7 +135,7 @@ RSpec.describe Projects::ArtifactsController, feature_category: :build_artifacts download_artifact(file_type: 'archive') expect(response).to have_gitlab_http_status(:ok) - expect(response.headers['Content-Disposition']).to eq(%Q(attachment; filename="#{filename}"; filename*=UTF-8''#{filename})) + expect(response.headers['Content-Disposition']).to eq(%(attachment; filename="#{filename}"; filename*=UTF-8''#{filename})) end end end @@ -168,7 +168,7 @@ RSpec.describe Projects::ArtifactsController, feature_category: :build_artifacts download_artifact(file_type: file_type) - expect(response.headers['Content-Disposition']).to eq(%Q(attachment; filename="#{filename}"; filename*=UTF-8''#{filename})) + expect(response.headers['Content-Disposition']).to eq(%(attachment; filename="#{filename}"; filename*=UTF-8''#{filename})) end end @@ -264,7 +264,7 @@ RSpec.describe Projects::ArtifactsController, feature_category: :build_artifacts expect(response).to have_gitlab_http_status(:ok) expect(response.headers['Content-Disposition']) - .to eq(%Q(attachment; filename="#{filename}"; filename*=UTF-8''#{filename})) + .to eq(%(attachment; filename="#{filename}"; filename*=UTF-8''#{filename})) end end end diff --git a/spec/controllers/projects/autocomplete_sources_controller_spec.rb b/spec/controllers/projects/autocomplete_sources_controller_spec.rb index 70178083e71..d0bfbeae78f 100644 --- a/spec/controllers/projects/autocomplete_sources_controller_spec.rb +++ b/spec/controllers/projects/autocomplete_sources_controller_spec.rb @@ -131,6 +131,10 @@ RSpec.describe Projects::AutocompleteSourcesController do end shared_examples 'all members are returned' do + before do + stub_feature_flags(disable_all_mention: false) + end + it 'returns an array of member object' do get :members, format: :json, params: { namespace_id: group.path, project_id: public_project.path, type: issuable_type } @@ -155,6 +159,19 @@ RSpec.describe Projects::AutocompleteSourcesController do name: invited_private_member.name, avatar_url: invited_private_member.avatar_url) end + + context 'when `disable_all_mention` FF is enabled' do + before do + stub_feature_flags(disable_all_mention: true) + end + + it 'does not return the all mention user' do + get :members, format: :json, params: { namespace_id: group.path, project_id: public_project.path, type: issuable_type } + + expect(json_response).not_to include(a_hash_including( + { username: 'all', name: 'All Project and Group Members' })) + end + end end context 'with issue' do @@ -180,6 +197,10 @@ RSpec.describe Projects::AutocompleteSourcesController do end shared_examples 'only public members are returned for public project' do + before do + stub_feature_flags(disable_all_mention: false) + end + it 'only returns public members' do get :members, format: :json, params: { namespace_id: group.path, project_id: public_project.path, type: issuable_type } @@ -193,6 +214,19 @@ RSpec.describe Projects::AutocompleteSourcesController do name: user.name, avatar_url: user.avatar_url) end + + context 'when `disable_all_mention` FF is enabled' do + before do + stub_feature_flags(disable_all_mention: true) + end + + it 'does not return the all mention user' do + get :members, format: :json, params: { namespace_id: group.path, project_id: public_project.path, type: issuable_type } + + expect(json_response).not_to include(a_hash_including( + { username: 'all', name: 'All Project and Group Members' })) + end + end end context 'with issue' do diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb index b07cb7a228d..49c1935c4a3 100644 --- a/spec/controllers/projects/blob_controller_spec.rb +++ b/spec/controllers/projects/blob_controller_spec.rb @@ -5,15 +5,21 @@ require 'spec_helper' RSpec.describe Projects::BlobController, feature_category: :source_code_management do include ProjectForksHelper - let(:project) { create(:project, :public, :repository, previous_default_branch: previous_default_branch) } - let(:previous_default_branch) { nil } + let_it_be(:project) { create(:project, :public, :repository) } describe "GET show" do - let(:params) { { namespace_id: project.namespace, project_id: project, id: id } } + let(:params) { { namespace_id: project.namespace, project_id: project, id: id, ref_type: ref_type } } + let(:ref_type) { nil } let(:request) do get(:show, params: params) end + let(:redirect_with_ref_type) { true } + + before do + stub_feature_flags(redirect_with_ref_type: redirect_with_ref_type) + end + render_views context 'with file path' do @@ -24,25 +30,43 @@ RSpec.describe Projects::BlobController, feature_category: :source_code_manageme request end + after do + project.repository.rm_tag(project.creator, 'ambiguous_ref') + project.repository.rm_branch(project.creator, 'ambiguous_ref') + end + context 'when the ref is ambiguous' do let(:ref) { 'ambiguous_ref' } let(:path) { 'README.md' } let(:id) { "#{ref}/#{path}" } - let(:params) { { namespace_id: project.namespace, project_id: project, id: id, ref_type: ref_type } } - context 'and explicitly requesting a branch' do - let(:ref_type) { 'heads' } + context 'and the redirect_with_ref_type flag is disabled' do + let(:redirect_with_ref_type) { false } + + context 'and explicitly requesting a branch' do + let(:ref_type) { 'heads' } + + it 'redirects to blob#show with sha for the branch' do + expect(response).to redirect_to(project_blob_path(project, "#{RepoHelpers.another_sample_commit.id}/#{path}")) + end + end + + context 'and explicitly requesting a tag' do + let(:ref_type) { 'tags' } - it 'redirects to blob#show with sha for the branch' do - expect(response).to redirect_to(project_blob_path(project, "#{RepoHelpers.another_sample_commit.id}/#{path}")) + it 'responds with success' do + expect(response).to be_ok + end end end - context 'and explicitly requesting a tag' do - let(:ref_type) { 'tags' } + context 'and the redirect_with_ref_type flag is enabled' do + context 'when the ref_type is nil' do + let(:ref_type) { nil } - it 'responds with success' do - expect(response).to be_ok + it 'redirects to the tag' do + expect(response).to redirect_to(project_blob_path(project, id, ref_type: 'tags')) + end end end end @@ -68,18 +92,20 @@ RSpec.describe Projects::BlobController, feature_category: :source_code_manageme it { is_expected.to respond_with(:not_found) } end - context "renamed default branch, valid file" do - let(:id) { 'old-default-branch/README.md' } - let(:previous_default_branch) { 'old-default-branch' } + context 'when default branch was renamed' do + let_it_be_with_reload(:project) { create(:project, :public, :repository, previous_default_branch: 'old-default-branch') } - it { is_expected.to redirect_to("/#{project.full_path}/-/blob/#{project.default_branch}/README.md") } - end + context "renamed default branch, valid file" do + let(:id) { 'old-default-branch/README.md' } + + it { is_expected.to redirect_to("/#{project.full_path}/-/blob/#{project.default_branch}/README.md") } + end - context "renamed default branch, invalid file" do - let(:id) { 'old-default-branch/invalid-path.rb' } - let(:previous_default_branch) { 'old-default-branch' } + context "renamed default branch, invalid file" do + let(:id) { 'old-default-branch/invalid-path.rb' } - it { is_expected.to redirect_to("/#{project.full_path}/-/blob/#{project.default_branch}/invalid-path.rb") } + it { is_expected.to redirect_to("/#{project.full_path}/-/blob/#{project.default_branch}/invalid-path.rb") } + end end context "binary file" do @@ -100,7 +126,7 @@ RSpec.describe Projects::BlobController, feature_category: :source_code_manageme let(:id) { 'master/README.md' } before do - get :show, params: { namespace_id: project.namespace, project_id: project, id: id }, format: :json + get :show, params: params, format: :json end it do @@ -114,7 +140,7 @@ RSpec.describe Projects::BlobController, feature_category: :source_code_manageme let(:id) { 'master/README.md' } before do - get :show, params: { namespace_id: project.namespace, project_id: project, id: id, viewer: 'none' }, format: :json + get :show, params: { namespace_id: project.namespace, project_id: project, id: id, ref_type: 'heads', viewer: 'none' }, format: :json end it do @@ -127,7 +153,7 @@ RSpec.describe Projects::BlobController, feature_category: :source_code_manageme context 'with tree path' do before do - get :show, params: { namespace_id: project.namespace, project_id: project, id: id } + get :show, params: params controller.instance_variable_set(:@blob, nil) end @@ -414,6 +440,10 @@ RSpec.describe Projects::BlobController, feature_category: :source_code_manageme let(:after_delete_path) { project_tree_path(project, 'master/files') } it 'redirects to the sub directory' do + expect_next_instance_of(Files::DeleteService) do |instance| + expect(instance).to receive(:execute).and_return({ status: :success }) + end + delete :destroy, params: default_params expect(response).to redirect_to(after_delete_path) diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index bface886674..15b7ddd85ea 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -113,18 +113,6 @@ RSpec.describe Projects::ClustersController, feature_category: :deployment_manag end end - it_behaves_like 'GET #metrics_dashboard for dashboard', 'Cluster health' do - let(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } - - let(:metrics_dashboard_req_params) do - { - id: cluster.id, - namespace_id: project.namespace.full_path, - project_id: project.path - } - end - end - describe 'POST create for existing cluster' do let(:params) do { diff --git a/spec/controllers/projects/design_management/designs/raw_images_controller_spec.rb b/spec/controllers/projects/design_management/designs/raw_images_controller_spec.rb index a7f3212a6f9..eaef455837f 100644 --- a/spec/controllers/projects/design_management/designs/raw_images_controller_spec.rb +++ b/spec/controllers/projects/design_management/designs/raw_images_controller_spec.rb @@ -129,7 +129,7 @@ RSpec.describe Projects::DesignManagement::Designs::RawImagesController do it 'serves files with `Content-Disposition: attachment`' do subject - expect(response.header['Content-Disposition']).to eq(%Q(attachment; filename=\"#{filename}\"; filename*=UTF-8''#{filename})) + expect(response.header['Content-Disposition']).to eq(%(attachment; filename=\"#{filename}\"; filename*=UTF-8''#{filename})) end it 'sets appropriate caching headers' do diff --git a/spec/controllers/projects/design_management/designs/resized_image_controller_spec.rb b/spec/controllers/projects/design_management/designs/resized_image_controller_spec.rb index 1bb5112681c..b4667b4568f 100644 --- a/spec/controllers/projects/design_management/designs/resized_image_controller_spec.rb +++ b/spec/controllers/projects/design_management/designs/resized_image_controller_spec.rb @@ -51,7 +51,7 @@ RSpec.describe Projects::DesignManagement::Designs::ResizedImageController, feat it 'sets Content-Disposition as attachment' do filename = design.filename - expect(response.header['Content-Disposition']).to eq(%Q(attachment; filename=\"#{filename}\"; filename*=UTF-8''#{filename})) + expect(response.header['Content-Disposition']).to eq(%(attachment; filename=\"#{filename}\"; filename*=UTF-8''#{filename})) end it 'serves files with Workhorse' do @@ -59,7 +59,7 @@ RSpec.describe Projects::DesignManagement::Designs::ResizedImageController, feat end it 'sets appropriate caching headers' do - expect(response.header['Cache-Control']).to eq('private') + expect(response.header['Cache-Control']).to eq('max-age=0, private, must-revalidate') expect(response.header['ETag']).to be_present end end diff --git a/spec/controllers/projects/grafana_api_controller_spec.rb b/spec/controllers/projects/grafana_api_controller_spec.rb deleted file mode 100644 index 9bc4a83030e..00000000000 --- a/spec/controllers/projects/grafana_api_controller_spec.rb +++ /dev/null @@ -1,268 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Projects::GrafanaApiController, feature_category: :metrics do - let_it_be(:project) { create(:project, :public) } - let_it_be(:reporter) { create(:user) } - let_it_be(:guest) { create(:user) } - let(:anonymous) { nil } - let(:user) { reporter } - - before_all do - project.add_reporter(reporter) - project.add_guest(guest) - end - - before do - stub_feature_flags(remove_monitor_metrics: false) - sign_in(user) if user - end - - describe 'GET #proxy' do - let(:proxy_service) { instance_double(Grafana::ProxyService) } - let(:params) do - { - namespace_id: project.namespace.full_path, - project_id: project.path, - proxy_path: 'api/v1/query_range', - datasource_id: '1', - query: 'rate(relevant_metric)', - start_time: '1570441248', - end_time: '1570444848', - step: '900' - } - end - - before do - allow(Grafana::ProxyService).to receive(:new).and_return(proxy_service) - allow(proxy_service).to receive(:execute).and_return(service_result) - end - - shared_examples_for 'error response' do |http_status| - it "returns #{http_status}" do - get :proxy, params: params - - expect(response).to have_gitlab_http_status(http_status) - expect(json_response['status']).to eq('error') - expect(json_response['message']).to eq('error message') - end - end - - shared_examples_for 'accessible' do - let(:service_result) { nil } - - it 'returns non erroneous response' do - get :proxy, params: params - - # We don't care about the specific code as long it's not an error. - expect(response).to have_gitlab_http_status(:no_content) - end - end - - shared_examples_for 'not accessible' do - let(:service_result) { nil } - - it 'returns 404 Not found' do - get :proxy, params: params - - expect(response).to have_gitlab_http_status(:not_found) - expect(Grafana::ProxyService).not_to have_received(:new) - end - end - - shared_examples_for 'login required' do - let(:service_result) { nil } - - it 'redirects to login page' do - get :proxy, params: params - - expect(response).to redirect_to(new_user_session_path) - expect(Grafana::ProxyService).not_to have_received(:new) - end - end - - context 'with a successful result' do - let(:service_result) { { status: :success, body: '{}' } } - - it 'returns a grafana datasource response' do - get :proxy, params: params - - expect(Grafana::ProxyService).to have_received(:new).with( - project, '1', 'api/v1/query_range', - { - 'query' => params[:query], - 'start' => params[:start_time], - 'end' => params[:end_time], - 'step' => params[:step] - } - ) - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to eq({}) - end - end - - context 'when the request is still unavailable' do - let(:service_result) { nil } - - it 'returns 204 no content' do - get :proxy, params: params - - expect(response).to have_gitlab_http_status(:no_content) - expect(json_response['status']).to eq('processing') - expect(json_response['message']).to eq('Not ready yet. Try again later.') - end - end - - context 'when an error has occurred' do - context 'with an error accessing grafana' do - let(:service_result) do - { - http_status: :service_unavailable, - status: :error, - message: 'error message' - } - end - - it_behaves_like 'error response', :service_unavailable - end - - context 'with a processing error' do - let(:service_result) do - { - status: :error, - message: 'error message' - } - end - - it_behaves_like 'error response', :bad_request - end - end - - context 'as guest' do - let(:user) { guest } - - it_behaves_like 'not accessible' - end - - context 'as anonymous' do - let(:user) { anonymous } - - it_behaves_like 'not accessible' - end - - context 'on a private project' do - let_it_be(:project) { create(:project, :private) } - - before_all do - project.add_guest(guest) - end - - context 'as anonymous' do - let(:user) { anonymous } - - it_behaves_like 'login required' - end - - context 'as guest' do - let(:user) { guest } - - it_behaves_like 'accessible' - end - end - - context 'when metrics dashboard feature is unavailable' do - before do - stub_feature_flags(remove_monitor_metrics: true) - end - - it_behaves_like 'not accessible' - end - end - - describe 'GET #metrics_dashboard' do - let(:service_result) { { status: :success, dashboard: '{}' } } - let(:params) do - { - format: :json, - embedded: true, - grafana_url: 'https://grafana.example.com', - namespace_id: project.namespace.full_path, - project_id: project.path - } - end - - before do - allow(Gitlab::Metrics::Dashboard::Finder) - .to receive(:find) - .and_return(service_result) - end - - context 'when the result is still processing' do - let(:service_result) { nil } - - it 'returns 204 no content' do - get :metrics_dashboard, params: params - - expect(response).to have_gitlab_http_status(:no_content) - end - end - - context 'when the result was successful' do - it 'returns the dashboard response' do - get :metrics_dashboard, params: params - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to include({ - 'dashboard' => '{}', - 'status' => 'success' - }) - expect(json_response).to include('metrics_data') - end - end - - context 'when an error has occurred' do - shared_examples_for 'error response' do |http_status| - it "returns #{http_status}" do - get :metrics_dashboard, params: params - - expect(response).to have_gitlab_http_status(http_status) - expect(json_response['status']).to eq('error') - expect(json_response['message']).to eq('error message') - end - end - - context 'with an error accessing grafana' do - let(:service_result) do - { - http_status: :service_unavailable, - status: :error, - message: 'error message' - } - end - - it_behaves_like 'error response', :service_unavailable - end - - context 'with a processing error' do - let(:service_result) { { status: :error, message: 'error message' } } - - it_behaves_like 'error response', :bad_request - end - - context 'when metrics dashboard feature is unavailable' do - before do - stub_feature_flags(remove_monitor_metrics: true) - end - - it 'returns 404 Not found' do - get :metrics_dashboard, params: params - - expect(response).to have_gitlab_http_status(:not_found) - expect(response.body).to be_empty - end - end - end - end -end diff --git a/spec/controllers/projects/incidents_controller_spec.rb b/spec/controllers/projects/incidents_controller_spec.rb deleted file mode 100644 index 460821634b0..00000000000 --- a/spec/controllers/projects/incidents_controller_spec.rb +++ /dev/null @@ -1,122 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Projects::IncidentsController do - let_it_be_with_refind(:project) { create(:project) } - let_it_be(:developer) { create(:user) } - let_it_be(:guest) { create(:user) } - let_it_be(:anonymous) { nil } - - before_all do - project.add_guest(guest) - project.add_developer(developer) - end - - before do - sign_in(user) if user - end - - subject { make_request } - - shared_examples 'not found' do - include_examples 'returning response status', :not_found - end - - shared_examples 'login required' do - it 'redirects to the login page' do - subject - - expect(response).to redirect_to(new_user_session_path) - end - end - - describe 'GET #index' do - def make_request - get :index, params: project_params - end - - let(:user) { developer } - - it 'shows the page' do - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:index) - end - - context 'when user is unauthorized' do - let(:user) { anonymous } - - it_behaves_like 'login required' - end - - context 'when user is a guest' do - let(:user) { guest } - - it 'shows the page' do - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:index) - end - end - end - - describe 'GET #show' do - def make_request - get :show, params: project_params(id: resource) - end - - let_it_be(:resource) { create(:incident, project: project) } - - let(:user) { developer } - - it 'renders incident page' do - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:show) - - expect(assigns(:incident)).to be_present - expect(assigns(:incident).author.association(:status)).to be_loaded - expect(assigns(:issue)).to be_present - expect(assigns(:noteable)).to eq(assigns(:incident)) - end - - context 'with non existing id' do - let(:resource) { non_existing_record_id } - - it_behaves_like 'not found' - end - - context 'for issue' do - let_it_be(:resource) { create(:issue, project: project) } - - it_behaves_like 'not found' - end - - context 'when user is a guest' do - let(:user) { guest } - - it 'shows the page' do - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:show) - end - end - - context 'when unauthorized' do - let(:user) { anonymous } - - it_behaves_like 'login required' - end - end - - private - - def project_params(opts = {}) - opts.reverse_merge(namespace_id: project.namespace, project_id: project) - end -end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 5e9135c00e3..f9ce77a44ba 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -1809,7 +1809,7 @@ RSpec.describe Projects::IssuesController, :request_store, feature_category: :te create(:user_status, user: second_discussion.author) expect { get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } } - .not_to exceed_query_limit(control) + .not_to exceed_query_limit(control).with_threshold(9) end context 'when user is setting notes filters' do diff --git a/spec/controllers/projects/merge_requests/drafts_controller_spec.rb b/spec/controllers/projects/merge_requests/drafts_controller_spec.rb index c3a5255b584..68fbeb00b67 100644 --- a/spec/controllers/projects/merge_requests/drafts_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/drafts_controller_spec.rb @@ -54,7 +54,7 @@ RSpec.describe Projects::MergeRequests::DraftsController, feature_category: :cod end it 'does not allow draft note creation' do - expect { create_draft_note }.to change { DraftNote.count }.by(0) + expect { create_draft_note }.not_to change { DraftNote.count } expect(response).to have_gitlab_http_status(:not_found) end end @@ -172,6 +172,33 @@ RSpec.describe Projects::MergeRequests::DraftsController, feature_category: :cod end end end + + context 'when the draft note is invalid' do + let_it_be(:draft_note) { DraftNote.new } + + before do + errors = ActiveModel::Errors.new(draft_note) + errors.add(:base, 'Error 1') + errors.add(:base, 'Error 2') + + allow(draft_note).to receive(:errors).and_return(errors) + + allow_next_instance_of(DraftNotes::CreateService) do |service| + allow(service).to receive(:execute).and_return(draft_note) + end + end + + it 'does not allow draft note creation' do + expect { create_draft_note }.not_to change { DraftNote.count } + end + + it "returns status 422", :aggregate_failures do + create_draft_note + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(response.body).to eq('{"errors":"Error 1 and Error 2"}') + end + end end describe 'PUT #update' do @@ -212,6 +239,30 @@ RSpec.describe Projects::MergeRequests::DraftsController, feature_category: :cod expect(draft.note).to eq('This is an updated unpublished comment') expect(json_response['note_html']).not_to be_empty end + + context 'when the draft note is invalid' do + before do + errors = ActiveModel::Errors.new(draft) + errors.add(:base, 'Error 1') + errors.add(:base, 'Error 2') + + allow_next_found_instance_of(DraftNote) do |instance| + allow(instance).to receive(:update).and_return(false) + allow(instance).to receive(:errors).and_return(errors) + end + end + + it 'does not update the draft' do + expect { update_draft_note }.not_to change { draft.reload.note } + end + + it 'returns status 422', :aggregate_failures do + update_draft_note + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(response.body).to eq('{"errors":"Error 1 and Error 2"}') + end + end end describe 'POST #publish' do diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index 4a5283f1127..940f6fed906 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -39,6 +39,12 @@ RSpec.describe Projects::NotesController, type: :controller, feature_category: : specify { expect(get(:index, params: request_params)).to have_request_urgency(:medium) } + it 'sets the correct feature category' do + get :index, params: request_params + + expect(::Gitlab::ApplicationContext.current_context_attribute(:feature_category)).to eq('team_planning') + end + it 'passes last_fetched_at from headers to NotesFinder and MergeIntoNotesService' do last_fetched_at = Time.zone.at(3.hours.ago.to_i) # remove nanoseconds @@ -149,6 +155,12 @@ RSpec.describe Projects::NotesController, type: :controller, feature_category: : expect(note_json[:discussion_line_code]).to be_nil end + it 'sets the correct feature category' do + get :index, params: params + + expect(::Gitlab::ApplicationContext.current_context_attribute(:feature_category)).to eq('source_code_management') + end + context 'when user cannot read commit' do before do allow(Ability).to receive(:allowed?).and_call_original @@ -164,7 +176,29 @@ RSpec.describe Projects::NotesController, type: :controller, feature_category: : end end - context 'for a regular note' do + context 'for a snippet note' do + let(:project_snippet) { create(:project_snippet, project: project) } + let!(:note) { create(:note_on_project_snippet, project: project, noteable: project_snippet) } + + let(:params) { request_params.merge(target_type: 'project_snippet', target_id: project_snippet.id, html: true) } + + it 'responds with the expected attributes' do + get :index, params: params + + expect(note_json[:id]).to eq(note.id) + expect(note_json[:discussion_html]).to be_nil + expect(note_json[:diff_discussion_html]).to be_nil + expect(note_json[:discussion_line_code]).to be_nil + end + + it 'sets the correct feature category' do + get :index, params: params + + expect(::Gitlab::ApplicationContext.current_context_attribute(:feature_category)).to eq('source_code_management') + end + end + + context 'for a merge request note' do let!(:note) { create(:note_on_merge_request, project: project) } let(:params) { request_params.merge(target_type: 'merge_request', target_id: note.noteable_id, html: true) } @@ -178,6 +212,12 @@ RSpec.describe Projects::NotesController, type: :controller, feature_category: : expect(note_json[:diff_discussion_html]).to be_nil expect(note_json[:discussion_line_code]).to be_nil end + + it 'sets the correct feature category' do + get :index, params: params + + expect(::Gitlab::ApplicationContext.current_context_attribute(:feature_category)).to eq('code_review_workflow') + end end context 'with cross-reference system note', :request_store do @@ -253,6 +293,68 @@ RSpec.describe Projects::NotesController, type: :controller, feature_category: : create! end + it 'sets the correct feature category' do + create! + + expect(::Gitlab::ApplicationContext.current_context_attribute(:feature_category)).to eq('code_review_workflow') + end + + context 'on an issue' do + let(:request_params) do + { + note: { note: note_text, noteable_id: issue.id, noteable_type: 'Issue' }, + namespace_id: project.namespace, + project_id: project, + target_type: 'issue', + target_id: issue.id + } + end + + it 'sets the correct feature category' do + create! + + expect(::Gitlab::ApplicationContext.current_context_attribute(:feature_category)).to eq('team_planning') + end + end + + context 'on a commit' do + let(:commit_id) { RepoHelpers.sample_commit.id } + let(:request_params) do + { + note: { note: note_text, commit_id: commit_id, noteable_type: 'Commit' }, + namespace_id: project.namespace, + project_id: project, + target_type: 'commit', + target_id: commit_id + } + end + + it 'sets the correct feature category' do + create! + + expect(::Gitlab::ApplicationContext.current_context_attribute(:feature_category)).to eq('source_code_management') + end + end + + context 'on a project snippet' do + let(:project_snippet) { create(:project_snippet, project: project) } + let(:request_params) do + { + note: { note: note_text, noteable_id: project_snippet.id, noteable_type: 'ProjectSnippet' }, + namespace_id: project.namespace, + project_id: project, + target_type: 'project_snippet', + target_id: project_snippet.id + } + end + + it 'sets the correct feature category' do + create! + + expect(::Gitlab::ApplicationContext.current_context_attribute(:feature_category)).to eq('source_code_management') + end + end + context 'the project is publically available' do context 'for HTML' do it "returns status 302" do diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb index 6d810fdcd51..486062fe52b 100644 --- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb +++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb @@ -106,7 +106,8 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu end end - describe 'POST #create' do + # Move this from `shared_context` to `describe` when `ci_refactoring_pipeline_schedule_create_service` is removed. + shared_context 'POST #create' do # rubocop:disable RSpec/ContextWording describe 'functionality' do before do project.add_developer(user) @@ -184,6 +185,16 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu end end + it_behaves_like 'POST #create' + + context 'when the FF ci_refactoring_pipeline_schedule_create_service is disabled' do + before do + stub_feature_flags(ci_refactoring_pipeline_schedule_create_service: false) + end + + it_behaves_like 'POST #create' + end + describe 'PUT #update' do describe 'functionality' do let!(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: user) } diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index 8c5f8fc6259..a5542a2b825 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -328,7 +328,7 @@ RSpec.describe Projects::PipelinesController, feature_category: :continuous_inte expect do get_pipeline_html expect(response).to have_gitlab_http_status(:ok) - end.not_to exceed_all_query_limit(control) + end.not_to exceed_all_query_limit(control).with_threshold(3) end end diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index ad49529b426..9657cf33afd 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -320,7 +320,7 @@ RSpec.describe Projects::ProjectMembersController do it 'returns correct json response' do expect(json_response).to eq({ "expires_soon" => false, - "expires_at_formatted" => expiry_date.to_time.in_time_zone.to_s(:medium) + "expires_at_formatted" => expiry_date.to_time.in_time_zone.to_fs(:medium) }) end end diff --git a/spec/controllers/projects/runners_controller_spec.rb b/spec/controllers/projects/runners_controller_spec.rb index e0e4d0f7bc5..d6816bd49af 100644 --- a/spec/controllers/projects/runners_controller_spec.rb +++ b/spec/controllers/projects/runners_controller_spec.rb @@ -28,52 +28,28 @@ RSpec.describe Projects::RunnersController, feature_category: :runner_fleet do } end - context 'when create_runner_workflow_for_namespace is enabled' do + context 'when user is maintainer' do before do - stub_feature_flags(create_runner_workflow_for_namespace: [project.namespace]) + project.add_maintainer(user) end - context 'when user is maintainer' do - before do - project.add_maintainer(user) - end - - it 'renders new with 200 status code' do - get :new, params: params + it 'renders new with 200 status code' do + get :new, params: params - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:new) - end - end - - context 'when user is not maintainer' do - before do - project.add_developer(user) - end - - it 'renders a 404' do - get :new, params: params - - expect(response).to have_gitlab_http_status(:not_found) - end + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:new) end end - context 'when create_runner_workflow_for_namespace is disabled' do + context 'when user is not maintainer' do before do - stub_feature_flags(create_runner_workflow_for_namespace: false) + project.add_developer(user) end - context 'when user is maintainer' do - before do - project.add_maintainer(user) - end + it 'renders a 404' do + get :new, params: params - it 'renders a 404' do - get :new, params: params - - expect(response).to have_gitlab_http_status(:not_found) - end + expect(response).to have_gitlab_http_status(:not_found) end end end @@ -81,66 +57,40 @@ RSpec.describe Projects::RunnersController, feature_category: :runner_fleet do describe '#register' do subject(:register) { get :register, params: { namespace_id: project.namespace, project_id: project, id: new_runner } } - context 'when create_runner_workflow_for_namespace is enabled' do + context 'when user is maintainer' do before do - stub_feature_flags(create_runner_workflow_for_namespace: [project.namespace]) + project.add_maintainer(user) end - context 'when user is maintainer' do - before do - project.add_maintainer(user) - end - - context 'when runner can be registered after creation' do - let_it_be(:new_runner) { create(:ci_runner, :project, projects: [project], registration_type: :authenticated_user) } - - it 'renders a :register template' do - register - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:register) - end - end - - context 'when runner cannot be registered after creation' do - let_it_be(:new_runner) { runner } + context 'when runner can be registered after creation' do + let_it_be(:new_runner) { create(:ci_runner, :project, projects: [project], registration_type: :authenticated_user) } - it 'returns :not_found' do - register + it 'renders a :register template' do + register - expect(response).to have_gitlab_http_status(:not_found) - end + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:register) end end - context 'when user is not maintainer' do - before do - project.add_developer(user) - end - - context 'when runner can be registered after creation' do - let_it_be(:new_runner) { create(:ci_runner, :project, projects: [project], registration_type: :authenticated_user) } + context 'when runner cannot be registered after creation' do + let_it_be(:new_runner) { runner } - it 'returns :not_found' do - register + it 'returns :not_found' do + register - expect(response).to have_gitlab_http_status(:not_found) - end + expect(response).to have_gitlab_http_status(:not_found) end end end - context 'when create_runner_workflow_for_namespace is disabled' do - let_it_be(:new_runner) { create(:ci_runner, :project, projects: [project], registration_type: :authenticated_user) } - + context 'when user is not maintainer' do before do - stub_feature_flags(create_runner_workflow_for_namespace: false) + project.add_developer(user) end - context 'when user is maintainer' do - before do - project.add_maintainer(user) - end + context 'when runner can be registered after creation' do + let_it_be(:new_runner) { create(:ci_runner, :project, projects: [project], registration_type: :authenticated_user) } it 'returns :not_found' do register diff --git a/spec/controllers/projects/service_desk_controller_spec.rb b/spec/controllers/projects/service_desk_controller_spec.rb deleted file mode 100644 index 6b914ac8f19..00000000000 --- a/spec/controllers/projects/service_desk_controller_spec.rb +++ /dev/null @@ -1,112 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Projects::ServiceDeskController do - let_it_be(:project) do - create(:project, :private, :custom_repo, - service_desk_enabled: true, - files: { '.gitlab/issue_templates/service_desk.md' => 'template' }) - end - - let_it_be(:user) { create(:user) } - - before do - allow(Gitlab::Email::IncomingEmail).to receive(:enabled?) { true } - allow(Gitlab::Email::IncomingEmail).to receive(:supports_wildcard?) { true } - - project.add_maintainer(user) - sign_in(user) - end - - describe 'GET service desk properties' do - it 'returns service_desk JSON data' do - get :show, params: { namespace_id: project.namespace.to_param, project_id: project }, format: :json - - expect(json_response["service_desk_address"]).to match(/\A[^@]+@[^@]+\z/) - expect(json_response["service_desk_enabled"]).to be_truthy - expect(response).to have_gitlab_http_status(:ok) - end - - context 'when user is not project maintainer' do - let(:guest) { create(:user) } - - it 'renders 404' do - project.add_guest(guest) - sign_in(guest) - - get :show, params: { namespace_id: project.namespace.to_param, project_id: project }, format: :json - - expect(response).to have_gitlab_http_status(:not_found) - end - end - - context 'when issue template is present' do - it 'returns template_file_missing as false' do - create(:service_desk_setting, project: project, issue_template_key: 'service_desk') - - get :show, params: { namespace_id: project.namespace.to_param, project_id: project }, format: :json - - response_hash = Gitlab::Json.parse(response.body) - expect(response_hash['template_file_missing']).to eq(false) - end - end - - context 'when issue template file becomes outdated' do - it 'returns template_file_missing as true' do - service = ServiceDeskSetting.new(project_id: project.id, issue_template_key: 'deleted') - service.save!(validate: false) - - get :show, params: { namespace_id: project.namespace.to_param, project_id: project }, format: :json - - expect(json_response['template_file_missing']).to eq(true) - end - end - end - - describe 'PUT service desk properties' do - it 'toggles services desk incoming email' do - project.update!(service_desk_enabled: false) - - put :update, params: { namespace_id: project.namespace.to_param, - project_id: project, - service_desk_enabled: true }, format: :json - - expect(json_response["service_desk_address"]).to be_present - expect(json_response["service_desk_enabled"]).to be_truthy - expect(response).to have_gitlab_http_status(:ok) - end - - it 'sets issue_template_key' do - put :update, params: { namespace_id: project.namespace.to_param, - project_id: project, - issue_template_key: 'service_desk' }, format: :json - - settings = project.service_desk_setting - expect(settings).to be_present - expect(settings.issue_template_key).to eq('service_desk') - expect(json_response['template_file_missing']).to eq(false) - expect(json_response['issue_template_key']).to eq('service_desk') - end - - it 'returns an error when update of service desk settings fails' do - put :update, params: { namespace_id: project.namespace.to_param, - project_id: project, - issue_template_key: 'invalid key' }, format: :json - - expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to eq('Issue template key is empty or does not exist') - end - - context 'when user cannot admin the project' do - let(:other_user) { create(:user) } - - it 'renders 404' do - sign_in(other_user) - put :update, params: { namespace_id: project.namespace.to_param, project_id: project, service_desk_enabled: true }, format: :json - - expect(response).to have_gitlab_http_status(:not_found) - end - end - end -end diff --git a/spec/controllers/projects/settings/ci_cd_controller_spec.rb b/spec/controllers/projects/settings/ci_cd_controller_spec.rb index 1c332eadc42..a1dbd27f49a 100644 --- a/spec/controllers/projects/settings/ci_cd_controller_spec.rb +++ b/spec/controllers/projects/settings/ci_cd_controller_spec.rb @@ -65,9 +65,8 @@ RSpec.describe Projects::Settings::CiCdController, feature_category: :continuous end context 'with group runners' do - let_it_be(:group) { create :group } - let_it_be(:project) { create :project, group: group } - let_it_be(:group_runner) { create(:ci_runner, :group, groups: [group]) } + let(:project) { other_project } + let!(:group_runner) { create(:ci_runner, :group, groups: [group]) } it 'sets group runners' do subject diff --git a/spec/controllers/projects/tree_controller_spec.rb b/spec/controllers/projects/tree_controller_spec.rb index 61998d516e8..ffec670e97d 100644 --- a/spec/controllers/projects/tree_controller_spec.rb +++ b/spec/controllers/projects/tree_controller_spec.rb @@ -3,9 +3,9 @@ require 'spec_helper' RSpec.describe Projects::TreeController, feature_category: :source_code_management do - let(:project) { create(:project, :repository, previous_default_branch: previous_default_branch) } - let(:previous_default_branch) { nil } + let_it_be(:project) { create(:project, :repository) } let(:user) { create(:user) } + let(:redirect_with_ref_type) { true } before do sign_in(user) @@ -17,10 +17,14 @@ RSpec.describe Projects::TreeController, feature_category: :source_code_manageme describe "GET show" do let(:params) do { - namespace_id: project.namespace.to_param, project_id: project, id: id + namespace_id: project.namespace.to_param, project_id: project, id: id, ref_type: ref_type } end + let(:request) { get :show, params: params } + + let(:ref_type) { nil } + # Make sure any errors accessing the tree in our views bubble up to this spec render_views @@ -28,26 +32,79 @@ RSpec.describe Projects::TreeController, feature_category: :source_code_manageme expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original project.repository.add_tag(project.creator, 'ambiguous_ref', RepoHelpers.sample_commit.id) project.repository.add_branch(project.creator, 'ambiguous_ref', RepoHelpers.another_sample_commit.id) - get :show, params: params + + stub_feature_flags(redirect_with_ref_type: redirect_with_ref_type) + end + + after do + project.repository.rm_tag(project.creator, 'ambiguous_ref') + project.repository.rm_branch(project.creator, 'ambiguous_ref') end - context 'when the ref is ambiguous' do - let(:id) { 'ambiguous_ref' } - let(:params) { { namespace_id: project.namespace, project_id: project, id: id, ref_type: ref_type } } + context 'when the redirect_with_ref_type flag is disabled' do + let(:redirect_with_ref_type) { false } - context 'and explicitly requesting a branch' do - let(:ref_type) { 'heads' } + context 'when there is a ref and tag with the same name' do + let(:id) { 'ambiguous_ref' } + let(:params) { { namespace_id: project.namespace, project_id: project, id: id, ref_type: ref_type } } - it 'redirects to blob#show with sha for the branch' do - expect(response).to redirect_to(project_tree_path(project, RepoHelpers.another_sample_commit.id)) + context 'and explicitly requesting a branch' do + let(:ref_type) { 'heads' } + + it 'redirects to blob#show with sha for the branch' do + request + expect(response).to redirect_to(project_tree_path(project, RepoHelpers.another_sample_commit.id)) + end + end + + context 'and explicitly requesting a tag' do + let(:ref_type) { 'tags' } + + it 'responds with success' do + request + expect(response).to be_ok + end end end + end - context 'and explicitly requesting a tag' do - let(:ref_type) { 'tags' } + describe 'delegating to ExtractsRef::RequestedRef' do + context 'when there is a ref and tag with the same name' do + let(:id) { 'ambiguous_ref' } + let(:params) { { namespace_id: project.namespace, project_id: project, id: id, ref_type: ref_type } } - it 'responds with success' do - expect(response).to be_ok + let(:requested_ref_double) { ExtractsRef::RequestedRef.new(project.repository, ref_type: ref_type, ref: id) } + + before do + allow(ExtractsRef::RequestedRef).to receive(:new).with(kind_of(Repository), ref_type: ref_type, ref: id).and_return(requested_ref_double) + end + + context 'and not specifying a ref_type' do + it 'finds the tags and redirects' do + expect(requested_ref_double).to receive(:find).and_call_original + request + expect(subject).to redirect_to("/#{project.full_path}/-/tree/#{id}/?ref_type=tags") + end + end + + context 'and explicitly requesting a branch' do + let(:ref_type) { 'heads' } + + it 'finds the branch' do + expect(requested_ref_double).not_to receive(:find) + request + expect(response).to be_ok + end + end + + context 'and explicitly requesting a tag' do + let(:ref_type) { 'tags' } + + it 'finds the tag' do + expect(requested_ref_double).not_to receive(:find) + request + expect(response).to be_ok + end end end end @@ -55,19 +112,26 @@ RSpec.describe Projects::TreeController, feature_category: :source_code_manageme context "valid branch, no path" do let(:id) { 'master' } - it { is_expected.to respond_with(:success) } + it 'responds with success' do + request + expect(response).to be_ok + end end context "valid branch, valid path" do let(:id) { 'master/encoding/' } - it { is_expected.to respond_with(:success) } + it 'responds with success' do + request + expect(response).to be_ok + end end context "valid branch, invalid path" do let(:id) { 'master/invalid-path/' } it 'redirects' do + request expect(subject) .to redirect_to("/#{project.full_path}/-/tree/master") end @@ -76,54 +140,91 @@ RSpec.describe Projects::TreeController, feature_category: :source_code_manageme context "invalid branch, valid path" do let(:id) { 'invalid-branch/encoding/' } - it { is_expected.to respond_with(:not_found) } + it 'responds with not_found' do + request + expect(subject).to respond_with(:not_found) + end end - context "renamed default branch, valid file" do - let(:id) { 'old-default-branch/encoding/' } - let(:previous_default_branch) { 'old-default-branch' } + context 'when default branch was renamed' do + let_it_be_with_reload(:project) { create(:project, :repository, previous_default_branch: 'old-default-branch') } - it { is_expected.to redirect_to("/#{project.full_path}/-/tree/#{project.default_branch}/encoding/") } - end + context "and the file is valid" do + let(:id) { 'old-default-branch/encoding/' } + + it 'redirects' do + request + expect(subject).to redirect_to("/#{project.full_path}/-/tree/#{project.default_branch}/encoding/") + end + end - context "renamed default branch, invalid file" do - let(:id) { 'old-default-branch/invalid-path/' } - let(:previous_default_branch) { 'old-default-branch' } + context "and the file is invalid" do + let(:id) { 'old-default-branch/invalid-path/' } - it { is_expected.to redirect_to("/#{project.full_path}/-/tree/#{project.default_branch}/invalid-path/") } + it 'redirects' do + request + expect(subject).to redirect_to("/#{project.full_path}/-/tree/#{project.default_branch}/invalid-path/") + end + end end context "valid empty branch, invalid path" do let(:id) { 'empty-branch/invalid-path/' } it 'redirects' do - expect(subject) - .to redirect_to("/#{project.full_path}/-/tree/empty-branch") + request + expect(subject).to redirect_to("/#{project.full_path}/-/tree/empty-branch") end end context "valid empty branch" do let(:id) { 'empty-branch' } - it { is_expected.to respond_with(:success) } + it 'responds with success' do + request + expect(response).to be_ok + end end context "invalid SHA commit ID" do let(:id) { 'ff39438/.gitignore' } - it { is_expected.to respond_with(:not_found) } + it 'responds with not_found' do + request + expect(subject).to respond_with(:not_found) + end end context "valid SHA commit ID" do let(:id) { '6d39438' } - it { is_expected.to respond_with(:success) } + it 'responds with success' do + request + expect(response).to be_ok + end + + context 'and there is a tag with the same name' do + before do + project.repository.add_tag(project.creator, id, RepoHelpers.sample_commit.id) + end + + it 'responds with success' do + request + + # This uses the tag + # TODO: Should we redirect in this case? + expect(response).to be_ok + end + end end context "valid SHA commit ID with path" do let(:id) { '6d39438/.gitignore' } - it { expect(response).to have_gitlab_http_status(:found) } + it 'responds with found' do + request + expect(response).to have_gitlab_http_status(:found) + end end end @@ -149,7 +250,7 @@ RSpec.describe Projects::TreeController, feature_category: :source_code_manageme before do get :show, params: { - namespace_id: project.namespace.to_param, project_id: project, id: id + namespace_id: project.namespace.to_param, project_id: project, id: id, ref_type: 'heads' } end @@ -157,7 +258,7 @@ RSpec.describe Projects::TreeController, feature_category: :source_code_manageme let(:id) { 'master/README.md' } it 'redirects' do - redirect_url = "/#{project.full_path}/-/blob/master/README.md" + redirect_url = "/#{project.full_path}/-/blob/master/README.md?ref_type=heads" expect(subject).to redirect_to(redirect_url) end end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 6adddccfda7..46913cfa649 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -164,107 +164,113 @@ RSpec.describe ProjectsController, feature_category: :groups_and_projects do end end - context 'when there is a tag with the same name as the default branch' do - let_it_be(:tagged_project) { create(:project, :public, :custom_repo, files: ['somefile']) } - let(:tree_with_default_branch) do - branch = tagged_project.repository.find_branch(tagged_project.default_branch) - project_tree_path(tagged_project, branch.target) - end - + context 'when redirect_with_ref_type is disabled' do before do - tagged_project.repository.create_file( - tagged_project.creator, - 'file_for_tag', - 'content for file', - message: "Automatically created file", - branch_name: 'branch-to-tag' - ) - - tagged_project.repository.add_tag( - tagged_project.creator, - tagged_project.default_branch, # tag name - 'branch-to-tag' # target - ) - end - - it 'redirects to tree view for the default branch' do - get :show, params: { namespace_id: tagged_project.namespace, id: tagged_project } - expect(response).to redirect_to(tree_with_default_branch) - end - end - - context 'when the default branch name is ambiguous' do - let_it_be(:project_with_default_branch) do - create(:project, :public, :custom_repo, files: ['somefile']) + stub_feature_flags(redirect_with_ref_type: false) end - shared_examples 'ambiguous ref redirects' do - let(:project) { project_with_default_branch } - let(:branch_ref) { "refs/heads/#{ref}" } - let(:repo) { project.repository } + context 'when there is a tag with the same name as the default branch' do + let_it_be(:tagged_project) { create(:project, :public, :custom_repo, files: ['somefile']) } + let(:tree_with_default_branch) do + branch = tagged_project.repository.find_branch(tagged_project.default_branch) + project_tree_path(tagged_project, branch.target) + end before do - repo.create_branch(branch_ref, 'master') - repo.change_head(ref) + tagged_project.repository.create_file( + tagged_project.creator, + 'file_for_tag', + 'content for file', + message: "Automatically created file", + branch_name: 'branch-to-tag' + ) + + tagged_project.repository.add_tag( + tagged_project.creator, + tagged_project.default_branch, # tag name + 'branch-to-tag' # target + ) end - after do - repo.change_head('master') - repo.delete_branch(branch_ref) + it 'redirects to tree view for the default branch' do + get :show, params: { namespace_id: tagged_project.namespace, id: tagged_project } + expect(response).to redirect_to(tree_with_default_branch) end + end - subject do - get( - :show, - params: { - namespace_id: project.namespace, - id: project - } - ) + context 'when the default branch name is ambiguous' do + let_it_be(:project_with_default_branch) do + create(:project, :public, :custom_repo, files: ['somefile']) end - context 'when there is no conflicting ref' do - let(:other_ref) { 'non-existent-ref' } + shared_examples 'ambiguous ref redirects' do + let(:project) { project_with_default_branch } + let(:branch_ref) { "refs/heads/#{ref}" } + let(:repo) { project.repository } - it { is_expected.to have_gitlab_http_status(:ok) } - end + before do + repo.create_branch(branch_ref, 'master') + repo.change_head(ref) + end + + after do + repo.change_head('master') + repo.delete_branch(branch_ref) + end - context 'and that other ref exists' do - let(:other_ref) { 'master' } + subject do + get( + :show, + params: { + namespace_id: project.namespace, + id: project + } + ) + end + + context 'when there is no conflicting ref' do + let(:other_ref) { 'non-existent-ref' } - let(:project_default_root_tree_path) do - sha = repo.find_branch(project.default_branch).target - project_tree_path(project, sha) + it { is_expected.to have_gitlab_http_status(:ok) } end - it 'redirects to tree view for the default branch' do - is_expected.to redirect_to(project_default_root_tree_path) + context 'and that other ref exists' do + let(:other_ref) { 'master' } + + let(:project_default_root_tree_path) do + sha = repo.find_branch(project.default_branch).target + project_tree_path(project, sha) + end + + it 'redirects to tree view for the default branch' do + is_expected.to redirect_to(project_default_root_tree_path) + end end end - end - context 'when ref starts with ref/heads/' do - let(:ref) { "refs/heads/#{other_ref}" } + context 'when ref starts with ref/heads/' do + let(:ref) { "refs/heads/#{other_ref}" } - include_examples 'ambiguous ref redirects' - end + include_examples 'ambiguous ref redirects' + end - context 'when ref starts with ref/tags/' do - let(:ref) { "refs/tags/#{other_ref}" } + context 'when ref starts with ref/tags/' do + let(:ref) { "refs/tags/#{other_ref}" } - include_examples 'ambiguous ref redirects' - end + include_examples 'ambiguous ref redirects' + end - context 'when ref starts with heads/' do - let(:ref) { "heads/#{other_ref}" } + context 'when ref starts with heads/' do + let(:ref) { "heads/#{other_ref}" } - include_examples 'ambiguous ref redirects' - end + include_examples 'ambiguous ref redirects' + end - context 'when ref starts with tags/' do - let(:ref) { "tags/#{other_ref}" } + context 'when ref starts with tags/' do + let(:ref) { "tags/#{other_ref}" } - include_examples 'ambiguous ref redirects' + include_examples 'ambiguous ref redirects' + end end end end diff --git a/spec/controllers/registrations/welcome_controller_spec.rb b/spec/controllers/registrations/welcome_controller_spec.rb index 4118754144c..5a3feefc1ba 100644 --- a/spec/controllers/registrations/welcome_controller_spec.rb +++ b/spec/controllers/registrations/welcome_controller_spec.rb @@ -117,7 +117,7 @@ RSpec.describe Registrations::WelcomeController, feature_category: :system_acces end context 'when the new user already has more than 1 accepted group membership' do - it 'redirects to the most recent membership group activty page' do + it 'redirects to the most recent membership group activity page' do member2 = create(:group_member, user: user) expect(subject).to redirect_to(activity_group_path(member2.source)) diff --git a/spec/controllers/repositories/git_http_controller_spec.rb b/spec/controllers/repositories/git_http_controller_spec.rb index 276bd9b65b9..88af7d1fe45 100644 --- a/spec/controllers/repositories/git_http_controller_spec.rb +++ b/spec/controllers/repositories/git_http_controller_spec.rb @@ -79,7 +79,7 @@ RSpec.describe Repositories::GitHttpController, feature_category: :source_code_m end context 'when repository container is a project' do - it_behaves_like Repositories::GitHttpController do + it_behaves_like described_class do let(:container) { project } let(:user) { project.first_owner } let(:access_checker_class) { Gitlab::GitAccess } @@ -133,7 +133,7 @@ RSpec.describe Repositories::GitHttpController, feature_category: :source_code_m end context 'when the user is a deploy token' do - it_behaves_like Repositories::GitHttpController do + it_behaves_like described_class do let(:container) { project } let(:user) { create(:deploy_token, :project, projects: [project]) } let(:access_checker_class) { Gitlab::GitAccess } @@ -144,7 +144,7 @@ RSpec.describe Repositories::GitHttpController, feature_category: :source_code_m end context 'when repository container is a project wiki' do - it_behaves_like Repositories::GitHttpController do + it_behaves_like described_class do let(:container) { create(:project_wiki, :empty_repo, project: project) } let(:user) { project.first_owner } let(:access_checker_class) { Gitlab::GitAccessWiki } @@ -155,7 +155,7 @@ RSpec.describe Repositories::GitHttpController, feature_category: :source_code_m end context 'when repository container is a personal snippet' do - it_behaves_like Repositories::GitHttpController do + it_behaves_like described_class do let(:container) { personal_snippet } let(:user) { personal_snippet.author } let(:access_checker_class) { Gitlab::GitAccessSnippet } @@ -167,7 +167,7 @@ RSpec.describe Repositories::GitHttpController, feature_category: :source_code_m end context 'when repository container is a project snippet' do - it_behaves_like Repositories::GitHttpController do + it_behaves_like described_class do let(:container) { project_snippet } let(:user) { project_snippet.author } let(:access_checker_class) { Gitlab::GitAccessSnippet } |