summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/assets/stylesheets/pages/experimental_separate_sign_up.scss2
-rw-r--r--app/controllers/application_controller.rb25
-rw-r--r--app/controllers/registrations_controller.rb14
-rw-r--r--app/finders/issuable_finder.rb3
-rw-r--r--app/models/lfs_object.rb5
-rw-r--r--app/models/user.rb1
-rw-r--r--app/services/projects/lfs_pointers/lfs_link_service.rb26
-rw-r--r--app/services/users/signup_service.rb34
-rw-r--r--app/views/registrations/welcome.html.haml15
-rw-r--r--changelogs/unreleased/35289-remove-existence-check-in-url-constrainer.yml5
-rw-r--r--changelogs/unreleased/35749-improve-performance-of-lfs_object-queries.yml5
-rw-r--r--changelogs/unreleased/48-add-company-question-to-profile-information.yml5
-rw-r--r--config/routes.rb2
-rw-r--r--config/routes/git_http.rb2
-rw-r--r--config/routes/project.rb62
-rw-r--r--db/migrate/20191028162543_add_setup_for_company_to_user_preferences.rb9
-rw-r--r--db/schema.rb1
-rw-r--r--lib/constraints/project_url_constrainer.rb9
-rw-r--r--lib/gitlab/patch/draw_route.rb2
-rw-r--r--locale/gitlab.pot7
-rw-r--r--spec/controllers/application_controller_spec.rb8
-rw-r--r--spec/controllers/projects/commits_controller_spec.rb4
-rw-r--r--spec/controllers/projects/error_tracking_controller_spec.rb2
-rw-r--r--spec/controllers/projects/issues_controller_spec.rb4
-rw-r--r--spec/controllers/projects/releases_controller_spec.rb4
-rw-r--r--spec/controllers/projects/tags_controller_spec.rb2
-rw-r--r--spec/controllers/projects_controller_spec.rb2
-rw-r--r--spec/controllers/registrations_controller_spec.rb4
-rw-r--r--spec/features/projects/pipelines/pipelines_spec.rb5
-rw-r--r--spec/features/projects/tags/user_views_tags_spec.rb2
-rw-r--r--spec/features/users/signup_spec.rb2
-rw-r--r--spec/lib/constraints/project_url_constrainer_spec.rb31
-rw-r--r--spec/models/lfs_object_spec.rb12
-rw-r--r--spec/requests/projects/blob_controller_spec.rb44
-rw-r--r--spec/routing/project_routing_spec.rb4
-rw-r--r--spec/services/projects/lfs_pointers/lfs_link_service_spec.rb30
-rw-r--r--spec/services/users/signup_service_spec.rb64
-rw-r--r--spec/support/controllers/sessionless_auth_controller_shared_examples.rb22
-rw-r--r--spec/support/shared_examples/controllers/todos_shared_examples.rb2
39 files changed, 364 insertions, 118 deletions
diff --git a/app/assets/stylesheets/pages/experimental_separate_sign_up.scss b/app/assets/stylesheets/pages/experimental_separate_sign_up.scss
index 53dfdd10788..5a80ea79600 100644
--- a/app/assets/stylesheets/pages/experimental_separate_sign_up.scss
+++ b/app/assets/stylesheets/pages/experimental_separate_sign_up.scss
@@ -23,7 +23,7 @@
.signup-heading h2 {
font-weight: $gl-font-weight-bold;
- padding: 0 10px;
+ padding: 0 $gl-padding;
@include media-breakpoint-down(md) {
font-size: $gl-font-size-large;
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 7329753ac54..c85b192b34a 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -17,7 +17,7 @@ class ApplicationController < ActionController::Base
include Gitlab::Tracking::ControllerConcern
include Gitlab::Experimentation::ControllerConcern
- before_action :authenticate_user!
+ before_action :authenticate_user!, except: [:route_not_found]
before_action :enforce_terms!, if: :should_enforce_terms?
before_action :validate_user_service_ticket!
before_action :check_password_expiration
@@ -30,7 +30,7 @@ class ApplicationController < ActionController::Base
before_action :active_user_check, unless: :devise_controller?
before_action :set_usage_stats_consent_flag
before_action :check_impersonation_availability
- before_action :require_role
+ before_action :required_signup_info
around_action :set_locale
around_action :set_session_storage
@@ -95,11 +95,13 @@ class ApplicationController < ActionController::Base
end
def route_not_found
- # We need to call #authenticate_user! here because sometimes this is called from another action
- # and not from our wildcard fallback route
- authenticate_user!
+ if current_user
+ not_found
+ else
+ store_location_for(:user, request.fullpath) unless request.xhr?
- not_found
+ redirect_to new_user_session_path, alert: I18n.t('devise.failure.unauthenticated')
+ end
end
def render(*args)
@@ -536,10 +538,13 @@ class ApplicationController < ActionController::Base
@current_user_mode ||= Gitlab::Auth::CurrentUserMode.new(current_user)
end
- # A user requires a role when they are part of the experimental signup flow (executed by the Growth team). Users
- # are redirected to the welcome page when their role is required and the experiment is enabled for the current user.
- def require_role
- return unless current_user && current_user.role_required? && experiment_enabled?(:signup_flow)
+ # A user requires a role and have the setup_for_company attribute set when they are part of the experimental signup
+ # flow (executed by the Growth team). Users are redirected to the welcome page when their role is required and the
+ # experiment is enabled for the current user.
+ def required_signup_info
+ return unless current_user
+ return unless current_user.role_required?
+ return unless experiment_enabled?(:signup_flow)
store_location_for :user, request.fullpath
diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb
index e55156c619b..84011e7643d 100644
--- a/app/controllers/registrations_controller.rb
+++ b/app/controllers/registrations_controller.rb
@@ -8,7 +8,7 @@ class RegistrationsController < Devise::RegistrationsController
layout :choose_layout
- skip_before_action :require_role, only: [:welcome, :update_role]
+ skip_before_action :required_signup_info, only: [:welcome, :update_registration]
prepend_before_action :check_captcha, only: :create
before_action :whitelist_query_limiting, only: [:destroy]
before_action :ensure_terms_accepted,
@@ -53,22 +53,22 @@ class RegistrationsController < Devise::RegistrationsController
def welcome
return redirect_to new_user_registration_path unless current_user
- return redirect_to stored_location_or_dashboard_or_almost_there_path(current_user) if current_user.role.present?
+ return redirect_to stored_location_or_dashboard_or_almost_there_path(current_user) if current_user.role.present? && !current_user.setup_for_company.nil?
- current_user.name = nil
+ current_user.name = nil if current_user.name == current_user.username
render layout: 'devise_experimental_separate_sign_up_flow'
end
- def update_role
- user_params = params.require(:user).permit(:name, :role)
- result = ::Users::UpdateService.new(current_user, user_params.merge(user: current_user)).execute
+ def update_registration
+ user_params = params.require(:user).permit(:name, :role, :setup_for_company)
+ result = ::Users::SignupService.new(current_user, user_params).execute
if result[:status] == :success
track_experiment_event(:signup_flow, 'end') # We want this event to be tracked when the user is _in_ the experimental group
set_flash_message! :notice, :signed_up
redirect_to stored_location_or_dashboard_or_almost_there_path(current_user)
else
- redirect_to users_sign_up_welcome_path, alert: result[:message]
+ render :welcome, layout: 'devise_experimental_separate_sign_up_flow'
end
end
diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb
index 0005206970b..dfddd32d7df 100644
--- a/app/finders/issuable_finder.rb
+++ b/app/finders/issuable_finder.rb
@@ -385,6 +385,9 @@ class IssuableFinder
end
def count_key(value)
+ # value may be an array if the finder used in `count_by_state` added an
+ # additional `group by`. Anyway we are sure that state will be always the
+ # last item because it's added as the last one to the query.
value = Array(value).last
klass.available_states.key(value)
end
diff --git a/app/models/lfs_object.rb b/app/models/lfs_object.rb
index 535c3cf2ba1..48c971194c6 100644
--- a/app/models/lfs_object.rb
+++ b/app/models/lfs_object.rb
@@ -18,6 +18,11 @@ class LfsObject < ApplicationRecord
after_save :update_file_store, if: :saved_change_to_file?
+ def self.not_linked_to_project(project)
+ where('NOT EXISTS (?)',
+ project.lfs_objects_projects.select(1).where('lfs_objects_projects.lfs_object_id = lfs_objects.id'))
+ end
+
def update_file_store
# The file.object_store is set during `uploader.store!`
# which happens after object is inserted/updated
diff --git a/app/models/user.rb b/app/models/user.rb
index 07cd8431d1a..f704589ad80 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -240,6 +240,7 @@ class User < ApplicationRecord
delegate :time_display_relative, :time_display_relative=, to: :user_preference
delegate :time_format_in_24h, :time_format_in_24h=, to: :user_preference
delegate :show_whitespace_in_diffs, :show_whitespace_in_diffs=, to: :user_preference
+ delegate :setup_for_company, :setup_for_company=, to: :user_preference
accepts_nested_attributes_for :user_preference, update_only: true
diff --git a/app/services/projects/lfs_pointers/lfs_link_service.rb b/app/services/projects/lfs_pointers/lfs_link_service.rb
index 38de2af9c1e..a05c76f5e85 100644
--- a/app/services/projects/lfs_pointers/lfs_link_service.rb
+++ b/app/services/projects/lfs_pointers/lfs_link_service.rb
@@ -4,6 +4,9 @@
module Projects
module LfsPointers
class LfsLinkService < BaseService
+ TooManyOidsError = Class.new(StandardError)
+
+ MAX_OIDS = 100_000
BATCH_SIZE = 1000
# Accept an array of oids to link
@@ -12,6 +15,10 @@ module Projects
def execute(oids)
return [] unless project&.lfs_enabled?
+ if oids.size > MAX_OIDS
+ raise TooManyOidsError, 'Too many LFS object ids to link, please push them manually'
+ end
+
# Search and link existing LFS Object
link_existing_lfs_objects(oids)
end
@@ -20,22 +27,27 @@ module Projects
# rubocop: disable CodeReuse/ActiveRecord
def link_existing_lfs_objects(oids)
- all_existing_objects = []
+ linked_existing_objects = []
iterations = 0
- LfsObject.where(oid: oids).each_batch(of: BATCH_SIZE) do |existent_lfs_objects|
+ oids.each_slice(BATCH_SIZE) do |oids_batch|
+ # Load all existing LFS Objects immediately so we don't issue an extra
+ # query for the `.any?`
+ existent_lfs_objects = LfsObject.where(oid: oids_batch).load
next unless existent_lfs_objects.any?
+ rows = existent_lfs_objects
+ .not_linked_to_project(project)
+ .map { |existing_lfs_object| { project_id: project.id, lfs_object_id: existing_lfs_object.id } }
+ Gitlab::Database.bulk_insert(:lfs_objects_projects, rows)
iterations += 1
- not_linked_lfs_objects = existent_lfs_objects.where.not(id: project.all_lfs_objects)
- project.all_lfs_objects << not_linked_lfs_objects
- all_existing_objects += existent_lfs_objects.pluck(:oid)
+ linked_existing_objects += existent_lfs_objects.map(&:oid)
end
- log_lfs_link_results(all_existing_objects.count, iterations)
+ log_lfs_link_results(linked_existing_objects.count, iterations)
- all_existing_objects
+ linked_existing_objects
end
# rubocop: enable CodeReuse/ActiveRecord
diff --git a/app/services/users/signup_service.rb b/app/services/users/signup_service.rb
new file mode 100644
index 00000000000..1031cec44cb
--- /dev/null
+++ b/app/services/users/signup_service.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+module Users
+ class SignupService < BaseService
+ def initialize(current_user, params = {})
+ @user = current_user
+ @params = params.dup
+ end
+
+ def execute
+ assign_attributes
+ inject_validators
+
+ if @user.save
+ success
+ else
+ error(@user.errors.full_messages.join('. '))
+ end
+ end
+
+ private
+
+ def assign_attributes
+ @user.assign_attributes(params) unless params.empty?
+ end
+
+ def inject_validators
+ class << @user
+ validates :role, presence: true
+ validates :setup_for_company, inclusion: { in: [true, false], message: :blank }
+ end
+ end
+ end
+end
diff --git a/app/views/registrations/welcome.html.haml b/app/views/registrations/welcome.html.haml
index 76c4a935584..7b92f5070df 100644
--- a/app/views/registrations/welcome.html.haml
+++ b/app/views/registrations/welcome.html.haml
@@ -1,10 +1,10 @@
-- content_for(:page_title, _('Welcome to GitLab %{username}!') % { username: current_user.username })
+- content_for(:page_title, _('Welcome to GitLab @%{username}!') % { username: current_user.username })
- max_name_length = 128
.text-center.mb-3
- = _('In order to tailor your experience with GitLab<br>we would like to know a bit more about you.').html_safe
+ = _('In order to tailor your experience with GitLab we<br>would like to know a bit more about you.').html_safe
.signup-box.p-3.mb-2
.signup-body
- = form_for(current_user, url: users_sign_up_update_role_path, html: { class: 'new_new_user gl-show-field-errors', 'aria-live' => 'assertive' }) do |f|
+ = form_for(current_user, url: users_sign_up_update_registration_path, html: { class: 'new_new_user gl-show-field-errors', 'aria-live' => 'assertive' }) do |f|
.devise-errors.mt-0
= render 'devise/shared/error_messages', resource: current_user
.name.form-group
@@ -13,5 +13,14 @@
.form-group
= f.label :role, _('Role'), class: 'label-bold'
= f.select :role, ::User.roles.keys.map { |role| [role.titleize, role] }, {}, class: 'form-control'
+ .form-group
+ = f.label :setup_for_company, _('Are you setting up GitLab for a company?'), class: 'label-bold'
+ .d-flex.justify-content-center
+ .w-25
+ = f.radio_button :setup_for_company, true
+ = f.label :setup_for_company, _('Yes'), value: 'true'
+ .w-25
+ = f.radio_button :setup_for_company, false
+ = f.label :setup_for_company, _('No'), value: 'false'
.submit-container.mt-3
= f.submit _('Get started!'), class: 'btn-register btn btn-block mb-0 p-2'
diff --git a/changelogs/unreleased/35289-remove-existence-check-in-url-constrainer.yml b/changelogs/unreleased/35289-remove-existence-check-in-url-constrainer.yml
deleted file mode 100644
index d33803cfdec..00000000000
--- a/changelogs/unreleased/35289-remove-existence-check-in-url-constrainer.yml
+++ /dev/null
@@ -1,5 +0,0 @@
----
-title: Fix JSON responses returning 302 instead of 401
-merge_request: 19412
-author:
-type: fixed
diff --git a/changelogs/unreleased/35749-improve-performance-of-lfs_object-queries.yml b/changelogs/unreleased/35749-improve-performance-of-lfs_object-queries.yml
new file mode 100644
index 00000000000..ea2582805c4
--- /dev/null
+++ b/changelogs/unreleased/35749-improve-performance-of-lfs_object-queries.yml
@@ -0,0 +1,5 @@
+---
+title: Improve performance of linking LFS objects during import
+merge_request: 19709
+author:
+type: performance
diff --git a/changelogs/unreleased/48-add-company-question-to-profile-information.yml b/changelogs/unreleased/48-add-company-question-to-profile-information.yml
new file mode 100644
index 00000000000..255f2289cdc
--- /dev/null
+++ b/changelogs/unreleased/48-add-company-question-to-profile-information.yml
@@ -0,0 +1,5 @@
+---
+title: Ask if the user is setting up GitLab for a company during signup
+merge_request: 17999
+author:
+type: changed
diff --git a/config/routes.rb b/config/routes.rb
index e32c4f7415b..23e97fe32dd 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -57,7 +57,7 @@ Rails.application.routes.draw do
# Sign up
get 'users/sign_up/welcome' => 'registrations#welcome'
- patch 'users/sign_up/update_role' => 'registrations#update_role'
+ patch 'users/sign_up/update_registration' => 'registrations#update_registration'
# Search
get 'search' => 'search#show'
diff --git a/config/routes/git_http.rb b/config/routes/git_http.rb
index 2e70fd9f1b6..aac6d418a92 100644
--- a/config/routes/git_http.rb
+++ b/config/routes/git_http.rb
@@ -52,7 +52,7 @@ scope(path: '*namespace_id/:project_id',
# /info/refs?service=git-receive-pack, but nothing else.
#
git_http_handshake = lambda do |request|
- ::Constraints::ProjectUrlConstrainer.new.matches?(request) &&
+ ::Constraints::ProjectUrlConstrainer.new.matches?(request, existence_check: false) &&
(request.query_string.blank? ||
request.query_string.match(/\Aservice=git-(upload|receive)-pack\z/))
end
diff --git a/config/routes/project.rb b/config/routes/project.rb
index 62a70b4655e..d49ba20ce84 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -245,6 +245,12 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
post :validate_query, on: :collection
end
end
+
+ Gitlab.ee do
+ resources :alerts, constraints: { id: /\d+/ }, only: [:index, :create, :show, :update, :destroy] do
+ post :notify, on: :collection
+ end
+ end
end
resources :merge_requests, concerns: :awardable, except: [:new, :create, :show], constraints: { id: /\d+/ } do
@@ -347,6 +353,17 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
end
end
+ Gitlab.ee do
+ resources :path_locks, only: [:index, :destroy] do
+ collection do
+ post :toggle
+ end
+ end
+
+ get '/service_desk' => 'service_desk#show', as: :service_desk
+ put '/service_desk' => 'service_desk#update', as: :service_desk_refresh
+ end
+
resource :variables, only: [:show, :update]
resources :triggers, only: [:index, :create, :edit, :update, :destroy]
@@ -380,6 +397,11 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
get :failures
get :status
get :test_report
+
+ Gitlab.ee do
+ get :security
+ get :licenses
+ end
end
member do
@@ -514,11 +536,24 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
get :realtime_changes
post :create_merge_request
get :discussions, format: :json
+
+ Gitlab.ee do
+ get 'designs(/*vueroute)', to: 'issues#designs', as: :designs, format: false
+ end
end
collection do
post :bulk_update
post :import_csv
+
+ Gitlab.ee do
+ post :export_csv
+ get :service_desk
+ end
+ end
+
+ Gitlab.ee do
+ resources :issue_links, only: [:index, :create, :destroy], as: 'links', path: 'links'
end
end
@@ -594,15 +629,6 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
Gitlab.ee do
resources :managed_licenses, only: [:index, :show, :new, :create, :edit, :update, :destroy]
end
-
- # Legacy routes.
- # Introduced in 12.0.
- # Should be removed after 12.1
- Gitlab::Routing.redirect_legacy_paths(self, :settings, :branches, :tags,
- :network, :graphs, :autocomplete_sources,
- :project_members, :deploy_keys, :deploy_tokens,
- :labels, :milestones, :services, :boards, :releases,
- :forks, :group_links, :import, :avatar)
end
resources(:projects,
@@ -627,4 +653,22 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
end
end
end
+
+ # Legacy routes.
+ # Introduced in 12.0.
+ # Should be removed after 12.1
+ scope(path: '*namespace_id',
+ as: :namespace,
+ namespace_id: Gitlab::PathRegex.full_namespace_route_regex) do
+ scope(path: ':project_id',
+ constraints: { project_id: Gitlab::PathRegex.project_route_regex },
+ module: :projects,
+ as: :project) do
+ Gitlab::Routing.redirect_legacy_paths(self, :settings, :branches, :tags,
+ :network, :graphs, :autocomplete_sources,
+ :project_members, :deploy_keys, :deploy_tokens,
+ :labels, :milestones, :services, :boards, :releases,
+ :forks, :group_links, :import, :avatar)
+ end
+ end
end
diff --git a/db/migrate/20191028162543_add_setup_for_company_to_user_preferences.rb b/db/migrate/20191028162543_add_setup_for_company_to_user_preferences.rb
new file mode 100644
index 00000000000..18a8a2306e2
--- /dev/null
+++ b/db/migrate/20191028162543_add_setup_for_company_to_user_preferences.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class AddSetupForCompanyToUserPreferences < ActiveRecord::Migration[5.2]
+ DOWNTIME = false
+
+ def change
+ add_column :user_preferences, :setup_for_company, :boolean
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 36f82f9913b..fa12e13f1b4 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -3754,6 +3754,7 @@ ActiveRecord::Schema.define(version: 2019_11_05_094625) do
t.boolean "time_format_in_24h"
t.string "projects_sort", limit: 64
t.boolean "show_whitespace_in_diffs", default: true, null: false
+ t.boolean "setup_for_company"
t.index ["user_id"], name: "index_user_preferences_on_user_id", unique: true
end
diff --git a/lib/constraints/project_url_constrainer.rb b/lib/constraints/project_url_constrainer.rb
index 64eefd67d81..d41490d2ebd 100644
--- a/lib/constraints/project_url_constrainer.rb
+++ b/lib/constraints/project_url_constrainer.rb
@@ -2,12 +2,17 @@
module Constraints
class ProjectUrlConstrainer
- def matches?(request)
+ def matches?(request, existence_check: true)
namespace_path = request.params[:namespace_id]
project_path = request.params[:project_id] || request.params[:id]
full_path = [namespace_path, project_path].join('/')
- ProjectPathValidator.valid_path?(full_path)
+ return false unless ProjectPathValidator.valid_path?(full_path)
+ return true unless existence_check
+
+ # We intentionally allow SELECT(*) here so result of this query can be used
+ # as cache for further Project.find_by_full_path calls within request
+ Project.find_by_full_path(full_path, follow_redirects: request.get?).present?
end
end
end
diff --git a/lib/gitlab/patch/draw_route.rb b/lib/gitlab/patch/draw_route.rb
index 4d1b57fbbbb..4c8ca015974 100644
--- a/lib/gitlab/patch/draw_route.rb
+++ b/lib/gitlab/patch/draw_route.rb
@@ -10,7 +10,7 @@ module Gitlab
RoutesNotFound = Class.new(StandardError)
def draw(routes_name)
- drawn_any = draw_ee(routes_name) | draw_ce(routes_name)
+ drawn_any = draw_ce(routes_name) | draw_ee(routes_name)
drawn_any || raise(RoutesNotFound.new("Cannot find #{routes_name}"))
end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index bcf7405a711..3f9e0e7805b 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -1947,6 +1947,9 @@ msgstr ""
msgid "Archiving the project will make it entirely read-only. It is hidden from the dashboard and doesn't show up in searches. <strong>The repository cannot be committed to, and no issues, comments or other entities can be created.</strong>"
msgstr ""
+msgid "Are you setting up GitLab for a company?"
+msgstr ""
+
msgid "Are you sure that you want to archive this project?"
msgstr ""
@@ -9143,7 +9146,7 @@ msgstr ""
msgid "In order to gather accurate feature usage data, it can take 1 to 2 weeks to see your index."
msgstr ""
-msgid "In order to tailor your experience with GitLab<br>we would like to know a bit more about you."
+msgid "In order to tailor your experience with GitLab we<br>would like to know a bit more about you."
msgstr ""
msgid "In the next step, you'll be able to select the projects you want to import."
@@ -19090,7 +19093,7 @@ msgstr ""
msgid "Welcome to GitLab"
msgstr ""
-msgid "Welcome to GitLab %{username}!"
+msgid "Welcome to GitLab @%{username}!"
msgstr ""
msgid "Welcome to the Guided GitLab Tour"
diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb
index 94afe741057..53896c7f5c7 100644
--- a/spec/controllers/application_controller_spec.rb
+++ b/spec/controllers/application_controller_spec.rb
@@ -186,7 +186,7 @@ describe ApplicationController do
expect(response).to have_gitlab_http_status(404)
end
- it 'redirects to login page via authenticate_user! if not authenticated' do
+ it 'redirects to login page if not authenticated' do
get :index
expect(response).to redirect_to new_user_session_path
@@ -827,7 +827,7 @@ describe ApplicationController do
end
end
- describe '#require_role' do
+ describe '#required_signup_info' do
controller(described_class) do
def index; end
end
@@ -849,7 +849,7 @@ describe ApplicationController do
it { is_expected.to redirect_to users_sign_up_welcome_path }
end
- context 'experiment enabled and user without a role' do
+ context 'experiment enabled and user without a required role' do
before do
sign_in(user)
get :index
@@ -858,7 +858,7 @@ describe ApplicationController do
it { is_expected.not_to redirect_to users_sign_up_welcome_path }
end
- context 'experiment disabled and user with required role' do
+ context 'experiment disabled' do
let(:experiment_enabled) { false }
before do
diff --git a/spec/controllers/projects/commits_controller_spec.rb b/spec/controllers/projects/commits_controller_spec.rb
index 9c4d6fdcb2a..1977e92e42b 100644
--- a/spec/controllers/projects/commits_controller_spec.rb
+++ b/spec/controllers/projects/commits_controller_spec.rb
@@ -142,7 +142,7 @@ describe Projects::CommitsController do
context 'token authentication' do
context 'public project' do
- it_behaves_like 'authenticates sessionless user', :show, :atom, public: true do
+ it_behaves_like 'authenticates sessionless user', :show, :atom, { public: true, ignore_incrementing: true } do
before do
public_project = create(:project, :repository, :public)
@@ -152,7 +152,7 @@ describe Projects::CommitsController do
end
context 'private project' do
- it_behaves_like 'authenticates sessionless user', :show, :atom, public: false do
+ it_behaves_like 'authenticates sessionless user', :show, :atom, { public: false, ignore_incrementing: true } do
before do
private_project = create(:project, :repository, :private)
private_project.add_maintainer(user)
diff --git a/spec/controllers/projects/error_tracking_controller_spec.rb b/spec/controllers/projects/error_tracking_controller_spec.rb
index 4c224e960a6..31868f5f717 100644
--- a/spec/controllers/projects/error_tracking_controller_spec.rb
+++ b/spec/controllers/projects/error_tracking_controller_spec.rb
@@ -146,7 +146,7 @@ describe Projects::ErrorTrackingController do
it 'redirects to sign-in page' do
post :list_projects, params: list_projects_params
- expect(response).to have_gitlab_http_status(:unauthorized)
+ expect(response).to have_gitlab_http_status(:redirect)
end
end
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 4c2b58551bf..8770a5ee303 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -1441,7 +1441,7 @@ describe Projects::IssuesController do
context 'private project with token authentication' do
let(:private_project) { create(:project, :private) }
- it_behaves_like 'authenticates sessionless user', :index, :atom do
+ it_behaves_like 'authenticates sessionless user', :index, :atom, ignore_incrementing: true do
before do
default_params.merge!(project_id: private_project, namespace_id: private_project.namespace)
@@ -1449,7 +1449,7 @@ describe Projects::IssuesController do
end
end
- it_behaves_like 'authenticates sessionless user', :calendar, :ics do
+ it_behaves_like 'authenticates sessionless user', :calendar, :ics, ignore_incrementing: true do
before do
default_params.merge!(project_id: private_project, namespace_id: private_project.namespace)
diff --git a/spec/controllers/projects/releases_controller_spec.rb b/spec/controllers/projects/releases_controller_spec.rb
index 28ca20d7dab..562119d967f 100644
--- a/spec/controllers/projects/releases_controller_spec.rb
+++ b/spec/controllers/projects/releases_controller_spec.rb
@@ -111,8 +111,8 @@ describe Projects::ReleasesController do
context 'when the project is private and the user is not logged in' do
let(:project) { private_project }
- it 'returns a 401' do
- expect(response).to have_gitlab_http_status(:unauthorized)
+ it 'returns a redirect' do
+ expect(response).to have_gitlab_http_status(:redirect)
end
end
end
diff --git a/spec/controllers/projects/tags_controller_spec.rb b/spec/controllers/projects/tags_controller_spec.rb
index b99b5d611fc..f077b4c99fc 100644
--- a/spec/controllers/projects/tags_controller_spec.rb
+++ b/spec/controllers/projects/tags_controller_spec.rb
@@ -41,7 +41,7 @@ describe Projects::TagsController do
context 'private project with token authentication' do
let(:private_project) { create(:project, :repository, :private) }
- it_behaves_like 'authenticates sessionless user', :index, :atom do
+ it_behaves_like 'authenticates sessionless user', :index, :atom, ignore_incrementing: true do
before do
default_params.merge!(project_id: private_project, namespace_id: private_project.namespace)
diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb
index 321f5ecdbc9..22538565698 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -1149,7 +1149,7 @@ describe ProjectsController do
context 'private project with token authentication' do
let(:private_project) { create(:project, :private) }
- it_behaves_like 'authenticates sessionless user', :show, :atom do
+ it_behaves_like 'authenticates sessionless user', :show, :atom, ignore_incrementing: true do
before do
default_params.merge!(id: private_project, namespace_id: private_project.namespace)
diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb
index 8f260aa8b43..c5cfdd32619 100644
--- a/spec/controllers/registrations_controller_spec.rb
+++ b/spec/controllers/registrations_controller_spec.rb
@@ -381,7 +381,7 @@ describe RegistrationsController do
end
end
- describe '#update_role' do
+ describe '#update_registration' do
before do
stub_experiment(signup_flow: true)
stub_experiment_for_user(signup_flow: true)
@@ -395,7 +395,7 @@ describe RegistrationsController do
label: anything,
property: 'experimental_group'
)
- patch :update_role, params: { user: { name: 'New name', role: 'software_developer' } }
+ patch :update_registration, params: { user: { name: 'New name', role: 'software_developer', setup_for_company: 'false' } }
end
end
end
diff --git a/spec/features/projects/pipelines/pipelines_spec.rb b/spec/features/projects/pipelines/pipelines_spec.rb
index a9a127da56f..f6eeb8d7065 100644
--- a/spec/features/projects/pipelines/pipelines_spec.rb
+++ b/spec/features/projects/pipelines/pipelines_spec.rb
@@ -819,7 +819,10 @@ describe 'Pipelines', :js do
context 'when project is private' do
let(:project) { create(:project, :private, :repository) }
- it { expect(page).to have_content 'You need to sign in' }
+ it 'redirects the user to sign_in and displays the flash alert' do
+ expect(page).to have_content 'You need to sign in'
+ expect(page.current_path).to eq("/users/sign_in")
+ end
end
end
diff --git a/spec/features/projects/tags/user_views_tags_spec.rb b/spec/features/projects/tags/user_views_tags_spec.rb
index f344b682715..bc570f502bf 100644
--- a/spec/features/projects/tags/user_views_tags_spec.rb
+++ b/spec/features/projects/tags/user_views_tags_spec.rb
@@ -15,7 +15,7 @@ describe 'User views tags', :feature do
it do
visit project_tags_path(project, format: :atom)
- expect(page).to have_gitlab_http_status(401)
+ expect(page.current_path).to eq("/users/sign_in")
end
end
diff --git a/spec/features/users/signup_spec.rb b/spec/features/users/signup_spec.rb
index 5d4c30b6e8e..29ff0c67dbd 100644
--- a/spec/features/users/signup_spec.rb
+++ b/spec/features/users/signup_spec.rb
@@ -441,11 +441,13 @@ describe 'With experimental flow' do
fill_in 'user_name', with: 'New name'
select 'Software Developer', from: 'user_role'
+ choose 'user_setup_for_company_true'
click_button 'Get started!'
new_user = User.find_by_username(new_user.username)
expect(new_user.name).to eq 'New name'
expect(new_user.software_developer_role?).to be_truthy
+ expect(new_user.setup_for_company).to be_truthy
expect(page).to have_current_path(new_project_path)
end
end
diff --git a/spec/lib/constraints/project_url_constrainer_spec.rb b/spec/lib/constraints/project_url_constrainer_spec.rb
index 27d70d562c1..ac3221ecab7 100644
--- a/spec/lib/constraints/project_url_constrainer_spec.rb
+++ b/spec/lib/constraints/project_url_constrainer_spec.rb
@@ -14,15 +14,42 @@ describe Constraints::ProjectUrlConstrainer do
end
context 'invalid request' do
+ context "non-existing project" do
+ let(:request) { build_request('foo', 'bar') }
+
+ it { expect(subject.matches?(request)).to be_falsey }
+
+ context 'existence_check is false' do
+ it { expect(subject.matches?(request, existence_check: false)).to be_truthy }
+ end
+ end
+
context "project id ending with .git" do
let(:request) { build_request(namespace.full_path, project.path + '.git') }
it { expect(subject.matches?(request)).to be_falsey }
end
end
+
+ context 'when the request matches a redirect route' do
+ let(:old_project_path) { 'old_project_path' }
+ let!(:redirect_route) { project.redirect_routes.create!(path: "#{namespace.full_path}/#{old_project_path}") }
+
+ context 'and is a GET request' do
+ let(:request) { build_request(namespace.full_path, old_project_path) }
+ it { expect(subject.matches?(request)).to be_truthy }
+ end
+
+ context 'and is NOT a GET request' do
+ let(:request) { build_request(namespace.full_path, old_project_path, 'POST') }
+ it { expect(subject.matches?(request)).to be_falsey }
+ end
+ end
end
- def build_request(namespace, project)
- double(:request, params: { namespace_id: namespace, id: project })
+ def build_request(namespace, project, method = 'GET')
+ double(:request,
+ 'get?': (method == 'GET'),
+ params: { namespace_id: namespace, id: project })
end
end
diff --git a/spec/models/lfs_object_spec.rb b/spec/models/lfs_object_spec.rb
index 47cae5cf197..44445429d3e 100644
--- a/spec/models/lfs_object_spec.rb
+++ b/spec/models/lfs_object_spec.rb
@@ -3,6 +3,18 @@
require 'spec_helper'
describe LfsObject do
+ context 'scopes' do
+ describe '.not_existing_in_project' do
+ it 'contains only lfs objects not linked to the project' do
+ project = create(:project)
+ create(:lfs_objects_project, project: project)
+ other_lfs_object = create(:lfs_object)
+
+ expect(described_class.not_linked_to_project(project)).to contain_exactly(other_lfs_object)
+ end
+ end
+ end
+
it 'has a distinct has_many :projects relation through lfs_objects_projects' do
lfs_object = create(:lfs_object)
project = create(:project)
diff --git a/spec/requests/projects/blob_controller_spec.rb b/spec/requests/projects/blob_controller_spec.rb
deleted file mode 100644
index b3321375ccc..00000000000
--- a/spec/requests/projects/blob_controller_spec.rb
+++ /dev/null
@@ -1,44 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe Projects::BlobController do
- let(:project) { create(:project, :private, :repository) }
- let(:namespace) { project.namespace }
-
- context 'anonymous user views blob in inaccessible project' do
- context 'with default HTML format' do
- before do
- get namespace_project_blob_path(namespace_id: namespace, project_id: project, id: 'master/README.md')
- end
-
- context 'when project is private' do
- it { expect(response).to have_gitlab_http_status(:redirect) }
- end
-
- context 'when project does not exist' do
- let(:namespace) { 'non_existent_namespace' }
- let(:project) { 'non_existent_project' }
-
- it { expect(response).to have_gitlab_http_status(:redirect) }
- end
- end
-
- context 'with JSON format' do
- before do
- get namespace_project_blob_path(namespace_id: namespace, project_id: project, id: 'master/README.md', format: :json)
- end
-
- context 'when project is private' do
- it { expect(response).to have_gitlab_http_status(:unauthorized) }
- end
-
- context 'when project does not exist' do
- let(:namespace) { 'non_existent_namespace' }
- let(:project) { 'non_existent_project' }
-
- it { expect(response).to have_gitlab_http_status(:unauthorized) }
- end
- end
- end
-end
diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb
index 61110790a43..561c2b572ec 100644
--- a/spec/routing/project_routing_spec.rb
+++ b/spec/routing/project_routing_spec.rb
@@ -776,6 +776,10 @@ describe 'project routing' do
it 'routes when :template_type is `issue`' do
expect(get(show_with_template_type('issue'))).to route_to('projects/templates#show', namespace_id: 'gitlab', project_id: 'gitlabhq', template_type: 'issue', key: 'template_name', format: 'json')
end
+
+ it 'routes to application#route_not_found when :template_type is unknown' do
+ expect(get(show_with_template_type('invalid'))).to route_to('application#route_not_found', unmatched_route: 'gitlab/gitlabhq/templates/invalid/template_name')
+ end
end
end
diff --git a/spec/services/projects/lfs_pointers/lfs_link_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_link_service_spec.rb
index 66233787d3a..aca59079b3c 100644
--- a/spec/services/projects/lfs_pointers/lfs_link_service_spec.rb
+++ b/spec/services/projects/lfs_pointers/lfs_link_service_spec.rb
@@ -16,6 +16,13 @@ describe Projects::LfsPointers::LfsLinkService do
end
describe '#execute' do
+ it 'raises an error when trying to link too many objects at once' do
+ oids = Array.new(described_class::MAX_OIDS) { |i| "oid-#{i}" }
+ oids << 'the straw'
+
+ expect { subject.execute(oids) }.to raise_error(described_class::TooManyOidsError)
+ end
+
it 'links existing lfs objects to the project' do
expect(project.all_lfs_objects.count).to eq 2
@@ -28,7 +35,7 @@ describe Projects::LfsPointers::LfsLinkService do
it 'returns linked oids' do
linked = lfs_objects_project.map(&:lfs_object).map(&:oid) << new_lfs_object.oid
- expect(subject.execute(new_oid_list.keys)).to eq linked
+ expect(subject.execute(new_oid_list.keys)).to contain_exactly(*linked)
end
it 'links in batches' do
@@ -48,5 +55,26 @@ describe Projects::LfsPointers::LfsLinkService do
expect(project.all_lfs_objects.count).to eq 9
expect(linked.size).to eq 7
end
+
+ it 'only queries for the batch that will be processed', :aggregate_failures do
+ stub_const("#{described_class}::BATCH_SIZE", 1)
+ oids = %w(one two)
+
+ expect(LfsObject).to receive(:where).with(oid: %w(one)).once.and_call_original
+ expect(LfsObject).to receive(:where).with(oid: %w(two)).once.and_call_original
+
+ subject.execute(oids)
+ end
+
+ it 'only queries 3 times' do
+ # make sure that we don't count the queries in the setup
+ new_oid_list
+
+ # These are repeated for each batch of oids: maximum (MAX_OIDS / BATCH_SIZE) times
+ # 1. Load the batch of lfs object ids that we might know already
+ # 2. Load the objects that have not been linked to the project yet
+ # 3. Insert the lfs_objects_projects for that batch
+ expect { subject.execute(new_oid_list.keys) }.not_to exceed_query_limit(3)
+ end
end
end
diff --git a/spec/services/users/signup_service_spec.rb b/spec/services/users/signup_service_spec.rb
new file mode 100644
index 00000000000..7d3cd614142
--- /dev/null
+++ b/spec/services/users/signup_service_spec.rb
@@ -0,0 +1,64 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Users::SignupService do
+ let(:user) { create(:user, setup_for_company: true) }
+
+ describe '#execute' do
+ context 'when updating name' do
+ it 'updates the name attribute' do
+ result = update_user(user, name: 'New Name')
+
+ expect(result).to eq(status: :success)
+ expect(user.reload.name).to eq('New Name')
+ end
+
+ it 'returns an error result when name is missing' do
+ result = update_user(user, name: '')
+
+ expect(user.reload.name).not_to be_blank
+ expect(result[:status]).to eq(:error)
+ expect(result[:message]).to include("Name can't be blank")
+ end
+ end
+
+ context 'when updating role' do
+ it 'updates the role attribute' do
+ result = update_user(user, role: 'development_team_lead')
+
+ expect(result).to eq(status: :success)
+ expect(user.reload.role).to eq('development_team_lead')
+ end
+
+ it 'returns an error result when role is missing' do
+ result = update_user(user, role: '')
+
+ expect(user.reload.role).not_to be_blank
+ expect(result[:status]).to eq(:error)
+ expect(result[:message]).to eq("Role can't be blank")
+ end
+ end
+
+ context 'when updating setup_for_company' do
+ it 'updates the setup_for_company attribute' do
+ result = update_user(user, setup_for_company: 'false')
+
+ expect(result).to eq(status: :success)
+ expect(user.reload.setup_for_company).to be(false)
+ end
+
+ it 'returns an error result when setup_for_company is missing' do
+ result = update_user(user, setup_for_company: '')
+
+ expect(user.reload.setup_for_company).not_to be_blank
+ expect(result[:status]).to eq(:error)
+ expect(result[:message]).to eq("Setup for company can't be blank")
+ end
+ end
+
+ def update_user(user, opts)
+ described_class.new(user, opts).execute
+ end
+ end
+end
diff --git a/spec/support/controllers/sessionless_auth_controller_shared_examples.rb b/spec/support/controllers/sessionless_auth_controller_shared_examples.rb
index b5149a0fcb1..bc95fcd6b88 100644
--- a/spec/support/controllers/sessionless_auth_controller_shared_examples.rb
+++ b/spec/support/controllers/sessionless_auth_controller_shared_examples.rb
@@ -34,8 +34,15 @@ shared_examples 'authenticates sessionless user' do |path, format, params|
context 'when the personal access token has no api scope', unless: params[:public] do
it 'does not log the user in' do
- expect(authentication_metrics)
- .to increment(:user_unauthenticated_counter)
+ # Several instances of where these specs are shared route the request
+ # through ApplicationController#route_not_found which does not involve
+ # the usual auth code from Devise, so does not increment the
+ # :user_unauthenticated_counter
+ #
+ unless params[:ignore_incrementing]
+ expect(authentication_metrics)
+ .to increment(:user_unauthenticated_counter)
+ end
personal_access_token.update(scopes: [:read_user])
@@ -84,8 +91,15 @@ shared_examples 'authenticates sessionless user' do |path, format, params|
end
it "doesn't log the user in otherwise", unless: params[:public] do
- expect(authentication_metrics)
- .to increment(:user_unauthenticated_counter)
+ # Several instances of where these specs are shared route the request
+ # through ApplicationController#route_not_found which does not involve
+ # the usual auth code from Devise, so does not increment the
+ # :user_unauthenticated_counter
+ #
+ unless params[:ignore_incrementing]
+ expect(authentication_metrics)
+ .to increment(:user_unauthenticated_counter)
+ end
get path, params: default_params.merge(private_token: 'token')
diff --git a/spec/support/shared_examples/controllers/todos_shared_examples.rb b/spec/support/shared_examples/controllers/todos_shared_examples.rb
index f3f9abb7da2..914bf506320 100644
--- a/spec/support/shared_examples/controllers/todos_shared_examples.rb
+++ b/spec/support/shared_examples/controllers/todos_shared_examples.rb
@@ -39,7 +39,7 @@ shared_examples 'todos actions' do
post_create
end.to change { user.todos.count }.by(0)
- expect(response).to have_gitlab_http_status(parent.is_a?(Group) ? 401 : 302)
+ expect(response).to have_gitlab_http_status(302)
end
end
end