From 7108e2e75362aec227765820e116e3d7bc1ac08d Mon Sep 17 00:00:00 2001
From: Sean McGivern <sean@gitlab.com>
Date: Fri, 4 Nov 2016 09:29:52 +0000
Subject: [PATCH 01/56] Fix new branch button spec
1. We can create the note directly on the issue, rather than attaching
it after creation.
2. We need to update the MergeRequestClosesIssues relation to ensure
that the issue know that it's closed by the MR.
3. We should also check that the unavailable button is displayed - not
just that the available button is displayed.
---
spec/features/issues/new_branch_button_spec.rb | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/spec/features/issues/new_branch_button_spec.rb b/spec/features/issues/new_branch_button_spec.rb
index fb0c47042857b..755f4eb1b0b9a 100644
--- a/spec/features/issues/new_branch_button_spec.rb
+++ b/spec/features/issues/new_branch_button_spec.rb
@@ -18,8 +18,8 @@
end
context "when there is a referenced merge request" do
- let(:note) do
- create(:note, :on_issue, :system, project: project,
+ let!(:note) do
+ create(:note, :on_issue, :system, project: project, noteable: issue,
note: "Mentioned in !#{referenced_mr.iid}")
end
let(:referenced_mr) do
@@ -28,12 +28,13 @@
end
before do
- issue.notes << note
+ referenced_mr.cache_merge_request_closes_issues!(user)
visit namespace_project_issue_path(project.namespace, project, issue)
end
it "hides the new branch button", js: true do
+ expect(page).to have_css('#new-branch .unavailable')
expect(page).not_to have_css('#new-branch .available')
expect(page).to have_content /1 Related Merge Request/
end
--
GitLab
From 6bcc52a53678ca68001189c801497862d3f6e758 Mon Sep 17 00:00:00 2001
From: Jacob Vosmaer <jacob@gitlab.com>
Date: Thu, 27 Oct 2016 14:59:52 +0200
Subject: [PATCH 02/56] Refine Git garbage collection
---
.../admin/application_settings_controller.rb | 5 +
app/models/application_setting.rb | 17 +++
app/services/projects/housekeeping_service.rb | 59 +++++++--
.../application_settings/_form.html.haml | 39 ++++++
app/workers/git_garbage_collect_worker.rb | 47 ++++++-
changelogs/unreleased/git-gc-improvements.yml | 4 +
...dd_housekeeping_to_application_settings.rb | 32 +++++
db/schema.rb | 5 +
doc/administration/housekeeping.md | 8 ++
lib/gitlab/backend/shell.rb | 13 --
lib/gitlab/exclusive_lease.rb | 66 ++++------
spec/lib/gitlab/backend/shell_spec.rb | 1 -
spec/lib/gitlab/exclusive_lease_spec.rb | 27 +++-
spec/models/application_setting_spec.rb | 18 +++
.../projects/housekeeping_service_spec.rb | 28 +++-
.../git_garbage_collect_worker_spec.rb | 122 ++++++++++++++++--
16 files changed, 409 insertions(+), 82 deletions(-)
create mode 100644 changelogs/unreleased/git-gc-improvements.yml
create mode 100644 db/migrate/20161031155516_add_housekeeping_to_application_settings.rb
diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb
index 86e808314f4a8..52e0256943acd 100644
--- a/app/controllers/admin/application_settings_controller.rb
+++ b/app/controllers/admin/application_settings_controller.rb
@@ -117,6 +117,11 @@ def application_setting_params
:send_user_confirmation_email,
:container_registry_token_expire_delay,
:enabled_git_access_protocol,
+ :housekeeping_enabled,
+ :housekeeping_bitmaps_enabled,
+ :housekeeping_incremental_repack_period,
+ :housekeeping_full_repack_period,
+ :housekeeping_gc_period,
repository_storages: [],
restricted_visibility_levels: [],
import_sources: [],
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 6e7a90e7d9c90..fa5188ca27b57 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -85,6 +85,18 @@ class ApplicationSetting < ActiveRecord::Base
presence: { message: 'Domain blacklist cannot be empty if Blacklist is enabled.' },
if: :domain_blacklist_enabled?
+ validates :housekeeping_incremental_repack_period,
+ presence: true,
+ numericality: { only_integer: true, greater_than: 0 }
+
+ validates :housekeeping_full_repack_period,
+ presence: true,
+ numericality: { only_integer: true, greater_than: :housekeeping_incremental_repack_period }
+
+ validates :housekeeping_gc_period,
+ presence: true,
+ numericality: { only_integer: true, greater_than: :housekeeping_full_repack_period }
+
validates_each :restricted_visibility_levels do |record, attr, value|
unless value.nil?
value.each do |level|
@@ -168,6 +180,11 @@ def self.create_from_defaults
container_registry_token_expire_delay: 5,
repository_storages: ['default'],
user_default_external: false,
+ housekeeping_enabled: true,
+ housekeeping_bitmaps_enabled: true,
+ housekeeping_incremental_repack_period: 10,
+ housekeeping_full_repack_period: 50,
+ housekeeping_gc_period: 200,
)
end
diff --git a/app/services/projects/housekeeping_service.rb b/app/services/projects/housekeeping_service.rb
index c3dfc8cfbe8c9..4b8946f8ee21f 100644
--- a/app/services/projects/housekeeping_service.rb
+++ b/app/services/projects/housekeeping_service.rb
@@ -7,6 +7,8 @@
#
module Projects
class HousekeepingService < BaseService
+ include Gitlab::CurrentSettings
+
LEASE_TIMEOUT = 3600
class LeaseTaken < StandardError
@@ -20,13 +22,14 @@ def initialize(project)
end
def execute
- raise LeaseTaken unless try_obtain_lease
+ lease_uuid = try_obtain_lease
+ raise LeaseTaken unless lease_uuid.present?
- execute_gitlab_shell_gc
+ execute_gitlab_shell_gc(lease_uuid)
end
def needed?
- @project.pushes_since_gc >= 10
+ pushes_since_gc > 0 && period_match? && housekeeping_enabled?
end
def increment!
@@ -37,19 +40,59 @@ def increment!
private
- def execute_gitlab_shell_gc
- GitGarbageCollectWorker.perform_async(@project.id)
+ def execute_gitlab_shell_gc(lease_uuid)
+ GitGarbageCollectWorker.perform_async(@project.id, task, lease_key, lease_uuid)
ensure
- Gitlab::Metrics.measure(:reset_pushes_since_gc) do
- @project.reset_pushes_since_gc
+ if pushes_since_gc >= gc_period
+ Gitlab::Metrics.measure(:reset_pushes_since_gc) do
+ @project.reset_pushes_since_gc
+ end
end
end
def try_obtain_lease
Gitlab::Metrics.measure(:obtain_housekeeping_lease) do
- lease = ::Gitlab::ExclusiveLease.new("project_housekeeping:#{@project.id}", timeout: LEASE_TIMEOUT)
+ lease = ::Gitlab::ExclusiveLease.new(lease_key, timeout: LEASE_TIMEOUT)
lease.try_obtain
end
end
+
+ def lease_key
+ "project_housekeeping:#{@project.id}"
+ end
+
+ def pushes_since_gc
+ @project.pushes_since_gc
+ end
+
+ def task
+ if pushes_since_gc % gc_period == 0
+ :gc
+ elsif pushes_since_gc % full_repack_period == 0
+ :full_repack
+ else
+ :incremental_repack
+ end
+ end
+
+ def period_match?
+ [gc_period, full_repack_period, repack_period].any? { |period| pushes_since_gc % period == 0 }
+ end
+
+ def housekeeping_enabled?
+ current_application_settings.housekeeping_enabled
+ end
+
+ def gc_period
+ current_application_settings.housekeeping_gc_period
+ end
+
+ def full_repack_period
+ current_application_settings.housekeeping_full_repack_period
+ end
+
+ def repack_period
+ current_application_settings.housekeeping_incremental_repack_period
+ end
end
end
diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml
index 28003e5f5091d..450ec322f2c3a 100644
--- a/app/views/admin/application_settings/_form.html.haml
+++ b/app/views/admin/application_settings/_form.html.haml
@@ -422,5 +422,44 @@
Enable this option to include the name of the author of the issue,
merge request or comment in the email body instead.
+ %fieldset
+ %legend Automatic Git repository housekeeping
+ .form-group
+ .col-sm-offset-2.col-sm-10
+ .checkbox
+ = f.label :housekeeping_enabled do
+ = f.check_box :housekeeping_enabled
+ Enable automatic repository housekeeping (git repack, git gc)
+ .help-block
+ If you keep automatic housekeeping disabled for a long time Git
+ repository access on your GitLab server will become slower and your
+ repositories will use more disk space. We recommend to always leave
+ this enabled.
+ .checkbox
+ = f.label :housekeeping_bitmaps_enabled do
+ = f.check_box :housekeeping_bitmaps_enabled
+ Enable Git pack file bitmap creation
+ .help-block
+ Creating pack file bitmaps makes housekeeping take a little longer but
+ bitmaps should accelerate 'git clone' performance.
+ .form-group
+ = f.label :housekeeping_incremental_repack_period, 'Incremental repack period', class: 'control-label col-sm-2'
+ .col-sm-10
+ = f.number_field :housekeeping_incremental_repack_period, class: 'form-control'
+ .help-block
+ Number of Git pushes after which an incremental 'git repack' is run.
+ .form-group
+ = f.label :housekeeping_full_repack_period, 'Full repack period', class: 'control-label col-sm-2'
+ .col-sm-10
+ = f.number_field :housekeeping_full_repack_period, class: 'form-control'
+ .help-block
+ Number of Git pushes after which a full 'git repack' is run.
+ .form-group
+ = f.label :housekeeping_gc_period, 'Git GC period', class: 'control-label col-sm-2'
+ .col-sm-10
+ = f.number_field :housekeeping_gc_period, class: 'form-control'
+ .help-block
+ Number of Git pushes after which 'git gc' is run.
+
.form-actions
= f.submit 'Save', class: 'btn btn-save'
diff --git a/app/workers/git_garbage_collect_worker.rb b/app/workers/git_garbage_collect_worker.rb
index 65f8093b5b092..d369b639ae994 100644
--- a/app/workers/git_garbage_collect_worker.rb
+++ b/app/workers/git_garbage_collect_worker.rb
@@ -1,17 +1,58 @@
class GitGarbageCollectWorker
include Sidekiq::Worker
- include Gitlab::ShellAdapter
include DedicatedSidekiqQueue
+ include Gitlab::CurrentSettings
sidekiq_options retry: false
- def perform(project_id)
+ def perform(project_id, task = :gc, lease_key = nil, lease_uuid = nil)
project = Project.find(project_id)
+ task = task.to_sym
+
+ cmd = command(task)
+ repo_path = project.repository.path_to_repo
+ description = "'#{cmd.join(' ')}' in #{repo_path}"
+
+ Gitlab::GitLogger.info(description)
+
+ output, status = Gitlab::Popen.popen(cmd, repo_path)
+ Gitlab::GitLogger.error("#{description} failed:\n#{output}") unless status.zero?
- gitlab_shell.gc(project.repository_storage_path, project.path_with_namespace)
# Refresh the branch cache in case garbage collection caused a ref lookup to fail
+ flush_ref_caches(project) if task == :gc
+ ensure
+ Gitlab::ExclusiveLease.cancel(lease_key, lease_uuid) if lease_key.present? && lease_uuid.present?
+ end
+
+ private
+
+ def command(task)
+ case task
+ when :gc
+ git(write_bitmaps: bitmaps_enabled?) + %w[gc]
+ when :full_repack
+ git(write_bitmaps: bitmaps_enabled?) + %w[repack -A -d --pack-kept-objects]
+ when :incremental_repack
+ # Normal git repack fails when bitmaps are enabled. It is impossible to
+ # create a bitmap here anyway.
+ git(write_bitmaps: false) + %w[repack -d]
+ else
+ raise "Invalid gc task: #{task.inspect}"
+ end
+ end
+
+ def flush_ref_caches(project)
project.repository.after_create_branch
project.repository.branch_names
project.repository.has_visible_content?
end
+
+ def bitmaps_enabled?
+ current_application_settings.housekeeping_bitmaps_enabled
+ end
+
+ def git(write_bitmaps:)
+ config_value = write_bitmaps ? 'true' : 'false'
+ %W[git -c repack.writeBitmaps=#{config_value}]
+ end
end
diff --git a/changelogs/unreleased/git-gc-improvements.yml b/changelogs/unreleased/git-gc-improvements.yml
new file mode 100644
index 0000000000000..f15e667ce8735
--- /dev/null
+++ b/changelogs/unreleased/git-gc-improvements.yml
@@ -0,0 +1,4 @@
+---
+title: Finer-grained Git gargage collection
+merge_request: 6588
+author:
diff --git a/db/migrate/20161031155516_add_housekeeping_to_application_settings.rb b/db/migrate/20161031155516_add_housekeeping_to_application_settings.rb
new file mode 100644
index 0000000000000..5a451fb575b8c
--- /dev/null
+++ b/db/migrate/20161031155516_add_housekeeping_to_application_settings.rb
@@ -0,0 +1,32 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddHousekeepingToApplicationSettings < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ # Set this constant to true if this migration requires downtime.
+ DOWNTIME = false
+
+ # When a migration requires downtime you **must** uncomment the following
+ # constant and define a short and easy to understand explanation as to why the
+ # migration requires downtime.
+ # DOWNTIME_REASON = ''
+
+ disable_ddl_transaction!
+
+ def up
+ add_column_with_default(:application_settings, :housekeeping_enabled, :boolean, default: true, allow_null: false)
+ add_column_with_default(:application_settings, :housekeeping_bitmaps_enabled, :boolean, default: true, allow_null: false)
+ add_column_with_default(:application_settings, :housekeeping_incremental_repack_period, :integer, default: 10, allow_null: false)
+ add_column_with_default(:application_settings, :housekeeping_full_repack_period, :integer, default: 50, allow_null: false)
+ add_column_with_default(:application_settings, :housekeeping_gc_period, :integer, default: 200, allow_null: false)
+ end
+
+ def down
+ remove_column(:application_settings, :housekeeping_enabled, :boolean, default: true, allow_null: false)
+ remove_column(:application_settings, :housekeeping_bitmaps_enabled, :boolean, default: true, allow_null: false)
+ remove_column(:application_settings, :housekeeping_incremental_repack_period, :integer, default: 10, allow_null: false)
+ remove_column(:application_settings, :housekeeping_full_repack_period, :integer, default: 50, allow_null: false)
+ remove_column(:application_settings, :housekeeping_gc_period, :integer, default: 200, allow_null: false)
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index dc088925d978b..48cb24ed20de4 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -98,6 +98,11 @@
t.text "help_page_text_html"
t.text "shared_runners_text_html"
t.text "after_sign_up_text_html"
+ t.boolean "housekeeping_enabled", default: true, null: false
+ t.boolean "housekeeping_bitmaps_enabled", default: true, null: false
+ t.integer "housekeeping_incremental_repack_period", default: 10, null: false
+ t.integer "housekeeping_full_repack_period", default: 50, null: false
+ t.integer "housekeeping_gc_period", default: 200, null: false
end
create_table "audit_events", force: :cascade do |t|
diff --git a/doc/administration/housekeeping.md b/doc/administration/housekeeping.md
index ad1fa98b63ba2..f846c06ca4224 100644
--- a/doc/administration/housekeeping.md
+++ b/doc/administration/housekeeping.md
@@ -3,6 +3,14 @@
> [Introduced][ce-2371] in GitLab 8.4.
---
+## Automatic housekeeping
+
+GitLab automatically runs `git gc` and `git repack` on repositories
+after Git pushes. If needed you can change how often this happens, or
+to turn it off, go to **Admin area > Settings**
+(`/admin/application_settings`).
+
+## Manual housekeeping
The housekeeping function runs `git gc` ([man page][man]) on the current
project Git repository.
diff --git a/lib/gitlab/backend/shell.rb b/lib/gitlab/backend/shell.rb
index 9cec71a32220a..82e194c1af12f 100644
--- a/lib/gitlab/backend/shell.rb
+++ b/lib/gitlab/backend/shell.rb
@@ -127,19 +127,6 @@ def remove_repository(storage, name)
'rm-project', storage, "#{name}.git"])
end
- # Gc repository
- #
- # storage - project storage path
- # path - project path with namespace
- #
- # Ex.
- # gc("/path/to/storage", "gitlab/gitlab-ci")
- #
- def gc(storage, path)
- Gitlab::Utils.system_silent([gitlab_shell_projects_path, 'gc',
- storage, "#{path}.git"])
- end
-
# Add new key to gitlab-shell
#
# Ex.
diff --git a/lib/gitlab/exclusive_lease.rb b/lib/gitlab/exclusive_lease.rb
index 7e8f35e9298cf..2dd4270439625 100644
--- a/lib/gitlab/exclusive_lease.rb
+++ b/lib/gitlab/exclusive_lease.rb
@@ -1,66 +1,52 @@
+require 'securerandom'
+
module Gitlab
# This class implements an 'exclusive lease'. We call it a 'lease'
# because it has a set expiry time. We call it 'exclusive' because only
# one caller may obtain a lease for a given key at a time. The
# implementation is intended to work across GitLab processes and across
- # servers. It is a 'cheap' alternative to using SQL queries and updates:
+ # servers. It is a cheap alternative to using SQL queries and updates:
# you do not need to change the SQL schema to start using
# ExclusiveLease.
#
- # It is important to choose the timeout wisely. If the timeout is very
- # high (1 hour) then the throughput of your operation gets very low (at
- # most once an hour). If the timeout is lower than how long your
- # operation may take then you cannot count on exclusivity. For example,
- # if the timeout is 10 seconds and you do an operation which may take 20
- # seconds then two overlapping operations may hold a lease for the same
- # key at the same time.
- #
- # This class has no 'cancel' method. I originally decided against adding
- # it because it would add complexity and a false sense of security. The
- # complexity: instead of setting '1' we would have to set a UUID, and to
- # delete it we would have to execute Lua on the Redis server to only
- # delete the key if the value was our own UUID. Otherwise there is a
- # chance that when you intend to cancel your lease you actually delete
- # someone else's. The false sense of security: you cannot design your
- # system to rely too much on the lease being cancelled after use because
- # the calling (Ruby) process may crash or be killed. You _cannot_ count
- # on begin/ensure blocks to cancel a lease, because the 'ensure' does
- # not always run. Think of 'kill -9' from the Unicorn master for
- # instance.
- #
- # If you find that leases are getting in your way, ask yourself: would
- # it be enough to lower the lease timeout? Another thing that might be
- # appropriate is to only use a lease for bulk/automated operations, and
- # to ignore the lease when you get a single 'manual' user request (a
- # button click).
- #
class ExclusiveLease
+ LUA_CANCEL_SCRIPT = <<-EOS
+ local key, uuid = KEYS[1], ARGV[1]
+ if redis.call("get", key) == uuid then
+ redis.call("del", key)
+ end
+ EOS
+
+ def self.cancel(key, uuid)
+ Gitlab::Redis.with do |redis|
+ redis.eval(LUA_CANCEL_SCRIPT, keys: [redis_key(key)], argv: [uuid])
+ end
+ end
+
+ def self.redis_key(key)
+ "gitlab:exclusive_lease:#{key}"
+ end
+
def initialize(key, timeout:)
- @key, @timeout = key, timeout
+ @redis_key = self.class.redis_key(key)
+ @timeout = timeout
+ @uuid = SecureRandom.uuid
end
- # Try to obtain the lease. Return true on success,
+ # Try to obtain the lease. Return lease UUID on success,
# false if the lease is already taken.
def try_obtain
# Performing a single SET is atomic
Gitlab::Redis.with do |redis|
- !!redis.set(redis_key, '1', nx: true, ex: @timeout)
+ redis.set(@redis_key, @uuid, nx: true, ex: @timeout) && @uuid
end
end
# Returns true if the key for this lease is set.
def exists?
Gitlab::Redis.with do |redis|
- redis.exists(redis_key)
+ redis.exists(@redis_key)
end
end
-
- # No #cancel method. See comments above!
-
- private
-
- def redis_key
- "gitlab:exclusive_lease:#{@key}"
- end
end
end
diff --git a/spec/lib/gitlab/backend/shell_spec.rb b/spec/lib/gitlab/backend/shell_spec.rb
index f826d0d1b04c6..4b08a02ec730f 100644
--- a/spec/lib/gitlab/backend/shell_spec.rb
+++ b/spec/lib/gitlab/backend/shell_spec.rb
@@ -14,7 +14,6 @@
it { is_expected.to respond_to :add_repository }
it { is_expected.to respond_to :remove_repository }
it { is_expected.to respond_to :fork_repository }
- it { is_expected.to respond_to :gc }
it { is_expected.to respond_to :add_namespace }
it { is_expected.to respond_to :rm_namespace }
it { is_expected.to respond_to :mv_namespace }
diff --git a/spec/lib/gitlab/exclusive_lease_spec.rb b/spec/lib/gitlab/exclusive_lease_spec.rb
index 6b3bd08b97845..a366d68a14607 100644
--- a/spec/lib/gitlab/exclusive_lease_spec.rb
+++ b/spec/lib/gitlab/exclusive_lease_spec.rb
@@ -5,32 +5,47 @@
describe '#try_obtain' do
it 'cannot obtain twice before the lease has expired' do
- lease = Gitlab::ExclusiveLease.new(unique_key, timeout: 3600)
- expect(lease.try_obtain).to eq(true)
+ lease = described_class.new(unique_key, timeout: 3600)
+ expect(lease.try_obtain).to be_present
expect(lease.try_obtain).to eq(false)
end
it 'can obtain after the lease has expired' do
timeout = 1
- lease = Gitlab::ExclusiveLease.new(unique_key, timeout: timeout)
+ lease = described_class.new(unique_key, timeout: timeout)
lease.try_obtain # start the lease
sleep(2 * timeout) # lease should have expired now
- expect(lease.try_obtain).to eq(true)
+ expect(lease.try_obtain).to be_present
end
end
describe '#exists?' do
it 'returns true for an existing lease' do
- lease = Gitlab::ExclusiveLease.new(unique_key, timeout: 3600)
+ lease = described_class.new(unique_key, timeout: 3600)
lease.try_obtain
expect(lease.exists?).to eq(true)
end
it 'returns false for a lease that does not exist' do
- lease = Gitlab::ExclusiveLease.new(unique_key, timeout: 3600)
+ lease = described_class.new(unique_key, timeout: 3600)
expect(lease.exists?).to eq(false)
end
end
+
+ describe '.cancel' do
+ it 'can cancel a lease' do
+ uuid = new_lease(unique_key)
+ expect(uuid).to be_present
+ expect(new_lease(unique_key)).to eq(false)
+
+ described_class.cancel(unique_key, uuid)
+ expect(new_lease(unique_key)).to be_present
+ end
+
+ def new_lease(key)
+ described_class.new(key, timeout: 3600).try_obtain
+ end
+ end
end
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index 2b76e056f3c01..b950fcdd81aae 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -98,6 +98,24 @@
end
end
end
+
+ context 'housekeeping settings' do
+ it { is_expected.not_to allow_value(0).for(:housekeeping_incremental_repack_period) }
+
+ it 'wants the full repack period to be longer than the incremental repack period' do
+ subject.housekeeping_incremental_repack_period = 2
+ subject.housekeeping_full_repack_period = 1
+
+ expect(subject).not_to be_valid
+ end
+
+ it 'wants the gc period to be longer than the full repack period' do
+ subject.housekeeping_full_repack_period = 2
+ subject.housekeeping_gc_period = 1
+
+ expect(subject).not_to be_valid
+ end
+ end
end
context 'restricted signup domains' do
diff --git a/spec/services/projects/housekeeping_service_spec.rb b/spec/services/projects/housekeeping_service_spec.rb
index cf90b33dfb43b..57a5aa5cedc11 100644
--- a/spec/services/projects/housekeeping_service_spec.rb
+++ b/spec/services/projects/housekeeping_service_spec.rb
@@ -14,8 +14,10 @@
describe '#execute' do
it 'enqueues a sidekiq job' do
- expect(subject).to receive(:try_obtain_lease).and_return(true)
- expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id)
+ expect(subject).to receive(:try_obtain_lease).and_return(:the_uuid)
+ expect(subject).to receive(:lease_key).and_return(:the_lease_key)
+ expect(subject).to receive(:task).and_return(:the_task)
+ expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :the_task, :the_lease_key, :the_uuid)
subject.execute
expect(project.reload.pushes_since_gc).to eq(0)
@@ -58,4 +60,26 @@
end.to change { project.pushes_since_gc }.from(0).to(1)
end
end
+
+ it 'uses all three kinds of housekeeping we offer' do
+ allow(subject).to receive(:try_obtain_lease).and_return(:the_uuid)
+ allow(subject).to receive(:lease_key).and_return(:the_lease_key)
+
+ # At push 200
+ expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :gc, :the_lease_key, :the_uuid).
+ exactly(1).times
+ # At push 50, 100, 150
+ expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :full_repack, :the_lease_key, :the_uuid).
+ exactly(3).times
+ # At push 10, 20, ... (except those above)
+ expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :incremental_repack, :the_lease_key, :the_uuid).
+ exactly(16).times
+
+ 201.times do
+ subject.increment!
+ subject.execute if subject.needed?
+ end
+
+ expect(project.pushes_since_gc).to eq(1)
+ end
end
diff --git a/spec/workers/git_garbage_collect_worker_spec.rb b/spec/workers/git_garbage_collect_worker_spec.rb
index c9f5aae0815b3..ae258bde26d33 100644
--- a/spec/workers/git_garbage_collect_worker_spec.rb
+++ b/spec/workers/git_garbage_collect_worker_spec.rb
@@ -1,3 +1,6 @@
+require 'digest'
+require 'fileutils'
+
require 'spec_helper'
describe GitGarbageCollectWorker do
@@ -6,16 +9,12 @@
subject { GitGarbageCollectWorker.new }
- before do
- allow(subject).to receive(:gitlab_shell).and_return(shell)
- end
-
describe "#perform" do
- it "runs `git gc`" do
- expect(shell).to receive(:gc).with(
- project.repository_storage_path,
- project.path_with_namespace).
- and_return(true)
+ it "flushes ref caches when the task is 'gc'" do
+ expect(subject).to receive(:command).with(:gc).and_return([:the, :command])
+ expect(Gitlab::Popen).to receive(:popen).
+ with([:the, :command], project.repository.path_to_repo).and_return(["", 0])
+
expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original
expect_any_instance_of(Repository).to receive(:branch_names).and_call_original
expect_any_instance_of(Repository).to receive(:branch_count).and_call_original
@@ -23,5 +22,110 @@
subject.perform(project.id)
end
+
+ shared_examples 'gc tasks' do
+ before { allow(subject).to receive(:bitmaps_enabled?).and_return(bitmaps_enabled) }
+
+ it 'incremental repack adds a new packfile' do
+ create_objects(project)
+ before_packs = packs(project)
+
+ expect(before_packs.count).to be >= 1
+
+ subject.perform(project.id, 'incremental_repack')
+ after_packs = packs(project)
+
+ # Exactly one new pack should have been created
+ expect(after_packs.count).to eq(before_packs.count + 1)
+
+ # Previously existing packs are still around
+ expect(before_packs & after_packs).to eq(before_packs)
+ end
+
+ it 'full repack consolidates into 1 packfile' do
+ create_objects(project)
+ subject.perform(project.id, 'incremental_repack')
+ before_packs = packs(project)
+
+ expect(before_packs.count).to be >= 2
+
+ subject.perform(project.id, 'full_repack')
+ after_packs = packs(project)
+
+ expect(after_packs.count).to eq(1)
+
+ # Previously existing packs should be gone now
+ expect(after_packs - before_packs).to eq(after_packs)
+
+ expect(File.exist?(bitmap_path(after_packs.first))).to eq(bitmaps_enabled)
+ end
+
+ it 'gc consolidates into 1 packfile and updates packed-refs' do
+ create_objects(project)
+ before_packs = packs(project)
+ before_packed_refs = packed_refs(project)
+
+ expect(before_packs.count).to be >= 1
+
+ subject.perform(project.id, 'gc')
+ after_packed_refs = packed_refs(project)
+ after_packs = packs(project)
+
+ expect(after_packs.count).to eq(1)
+
+ # Previously existing packs should be gone now
+ expect(after_packs - before_packs).to eq(after_packs)
+
+ # The packed-refs file should have been updated during 'git gc'
+ expect(before_packed_refs).not_to eq(after_packed_refs)
+
+ expect(File.exist?(bitmap_path(after_packs.first))).to eq(bitmaps_enabled)
+ end
+ end
+
+ context 'with bitmaps enabled' do
+ let(:bitmaps_enabled) { true }
+
+ include_examples 'gc tasks'
+ end
+
+ context 'with bitmaps disabled' do
+ let(:bitmaps_enabled) { false }
+
+ include_examples 'gc tasks'
+ end
+ end
+
+ # Create a new commit on a random new branch
+ def create_objects(project)
+ rugged = project.repository.rugged
+ old_commit = rugged.branches.first.target
+ new_commit_sha = Rugged::Commit.create(
+ rugged,
+ message: "hello world #{SecureRandom.hex(6)}",
+ author: Gitlab::Git::committer_hash(email: 'foo@bar', name: 'baz'),
+ committer: Gitlab::Git::committer_hash(email: 'foo@bar', name: 'baz'),
+ tree: old_commit.tree,
+ parents: [old_commit],
+ )
+ project.repository.update_ref!(
+ "refs/heads/#{SecureRandom.hex(6)}",
+ new_commit_sha,
+ Gitlab::Git::BLANK_SHA
+ )
+ end
+
+ def packs(project)
+ Dir["#{project.repository.path_to_repo}/objects/pack/*.pack"]
+ end
+
+ def packed_refs(project)
+ path = "#{project.repository.path_to_repo}/packed-refs"
+ FileUtils.touch(path)
+ File.read(path)
+ end
+
+ def bitmap_path(pack)
+ pack.sub(/\.pack\z/, '.bitmap')
end
end
--
GitLab
From c5eca4a632d3be058a0094d269fb0aac88e130d5 Mon Sep 17 00:00:00 2001
From: Jacob Vosmaer <jacob@gitlab.com>
Date: Fri, 4 Nov 2016 14:34:10 +0100
Subject: [PATCH 03/56] Remove unused 'require'
---
spec/workers/git_garbage_collect_worker_spec.rb | 1 -
1 file changed, 1 deletion(-)
diff --git a/spec/workers/git_garbage_collect_worker_spec.rb b/spec/workers/git_garbage_collect_worker_spec.rb
index ae258bde26d33..e471a68a49afe 100644
--- a/spec/workers/git_garbage_collect_worker_spec.rb
+++ b/spec/workers/git_garbage_collect_worker_spec.rb
@@ -1,4 +1,3 @@
-require 'digest'
require 'fileutils'
require 'spec_helper'
--
GitLab
From db8fed26af82f1fb6f143a9b15b6af27138953f8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Wed, 2 Nov 2016 18:52:04 +0100
Subject: [PATCH 04/56] Refactor template selector in issuable form
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Rémy Coutable <remy@rymai.me>
---
app/helpers/blob_helper.rb | 27 ----------
app/helpers/issuables_helper.rb | 51 +++++++++++++++++++
app/views/shared/issuable/_form.html.haml | 21 ++------
.../form/_template_selector.html.haml | 13 +++++
4 files changed, 68 insertions(+), 44 deletions(-)
create mode 100644 app/views/shared/issuable/form/_template_selector.html.haml
diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb
index e13b7cdd7077d..07ff6fb94889c 100644
--- a/app/helpers/blob_helper.rb
+++ b/app/helpers/blob_helper.rb
@@ -179,33 +179,6 @@ def licenses_for_select
}
end
- def selected_template(issuable)
- templates = issuable_templates(issuable)
- params[:issuable_template] if templates.include?(params[:issuable_template])
- end
-
- def can_add_template?(issuable)
- names = issuable_templates(issuable)
- names.empty? && can?(current_user, :push_code, @project) && !@project.private?
- end
-
- def merge_request_template_names
- @merge_request_templates ||= Gitlab::Template::MergeRequestTemplate.dropdown_names(ref_project)
- end
-
- def issue_template_names
- @issue_templates ||= Gitlab::Template::IssueTemplate.dropdown_names(ref_project)
- end
-
- def issuable_templates(issuable)
- @issuable_templates ||=
- if issuable.is_a?(Issue)
- issue_template_names
- elsif issuable.is_a?(MergeRequest)
- merge_request_template_names
- end
- end
-
def ref_project
@ref_project ||= @target_project || @project
end
diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb
index ef6cfb235a900..8127c3f3ee3e6 100644
--- a/app/helpers/issuables_helper.rb
+++ b/app/helpers/issuables_helper.rb
@@ -30,6 +30,33 @@ def issuable_json_path(issuable)
end
end
+ def can_add_template?(issuable)
+ names = issuable_templates(issuable)
+ names.empty? && can?(current_user, :push_code, @project) && !@project.private?
+ end
+
+ def template_dropdown_tag(issuable, &block)
+ title = selected_template(issuable) || "Choose a template"
+ options = {
+ toggle_class: 'js-issuable-selector',
+ title: title,
+ filter: true,
+ placeholder: 'Filter',
+ footer_content: true,
+ data: {
+ data: issuable_templates(issuable),
+ field_name: 'issuable_template',
+ selected: selected_template(issuable),
+ project_path: ref_project.path,
+ namespace_path: ref_project.namespace.path
+ }
+ }
+
+ dropdown_tag(title, options: options) do
+ capture(&block)
+ end
+ end
+
def user_dropdown_label(user_id, default_label)
return default_label if user_id.nil?
return "Unassigned" if user_id == "0"
@@ -153,4 +180,28 @@ def issuables_state_counter_cache_key(issuable_type, state)
hexdigest(['issuables_count', issuable_type, opts.sort].flatten.join('-'))
end
+
+ def issuable_templates(issuable)
+ @issuable_templates ||=
+ case issuable
+ when Issue
+ issue_template_names
+ when MergeRequest
+ merge_request_template_names
+ else
+ raise 'Unknown issuable type!'
+ end
+ end
+
+ def merge_request_template_names
+ @merge_request_templates ||= Gitlab::Template::MergeRequestTemplate.dropdown_names(ref_project)
+ end
+
+ def issue_template_names
+ @issue_templates ||= Gitlab::Template::IssueTemplate.dropdown_names(ref_project)
+ end
+
+ def selected_template(issuable)
+ params[:issuable_template] if issuable_templates(issuable).include?(params[:issuable_template])
+ end
end
diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml
index 0ace6be8f4e20..8d9769527812a 100644
--- a/app/views/shared/issuable/_form.html.haml
+++ b/app/views/shared/issuable/_form.html.haml
@@ -1,4 +1,5 @@
- project = @target_project || @project
+
= form_errors(issuable)
- if @conflict
@@ -11,23 +12,9 @@
.form-group
= f.label :title, class: 'control-label'
- - issuable_template_names = issuable_templates(issuable)
-
- - if issuable_template_names.any?
- .col-sm-3.col-lg-2
- .js-issuable-selector-wrap{ data: { issuable_type: issuable.class.to_s.underscore.downcase } }
- - title = selected_template(issuable) || "Choose a template"
-
- = dropdown_tag(title, options: { toggle_class: 'js-issuable-selector',
- title: title, filter: true, placeholder: 'Filter', footer_content: true,
- data: { data: issuable_template_names, field_name: 'issuable_template', selected: selected_template(issuable), project_path: ref_project.path, namespace_path: ref_project.namespace.path } } ) do
- %ul.dropdown-footer-list
- %li
- %a.no-template
- No template
- %a.reset-template
- Reset template
- %div{ class: issuable_template_names.any? ? 'col-sm-7 col-lg-8' : 'col-sm-10' }
+ = render 'shared/issuable/form/template_selector', issuable: issuable
+
+ %div{ class: issuable_templates(issuable).any? ? 'col-sm-7 col-lg-8' : 'col-sm-10' }
= f.text_field :title, maxlength: 255, autofocus: true, autocomplete: 'off',
class: 'form-control pad', required: true
diff --git a/app/views/shared/issuable/form/_template_selector.html.haml b/app/views/shared/issuable/form/_template_selector.html.haml
new file mode 100644
index 0000000000000..d613bd31d81f1
--- /dev/null
+++ b/app/views/shared/issuable/form/_template_selector.html.haml
@@ -0,0 +1,13 @@
+- issuable = local_assigns.fetch(:issuable, nil)
+
+- return unless issuable && issuable_templates(issuable).any?
+
+.col-sm-3.col-lg-2
+ .js-issuable-selector-wrap{ data: { issuable_type: issuable.to_ability_name } }
+ = template_dropdown_tag(issuable) do
+ %ul.dropdown-footer-list
+ %li
+ %a.no-template
+ No template
+ %a.reset-template
+ Reset template
--
GitLab
From 18a26c3367a56d3ebaed28c6e060eefefca19677 Mon Sep 17 00:00:00 2001
From: Connor Shea <connor.james.shea@gmail.com>
Date: Fri, 4 Nov 2016 12:48:05 -0600
Subject: [PATCH 05/56] Upgrade redis-rails from 4.0.0 to 5.0.1.
This update adds support for Rails 5. Also update redis-rails dependencies.
---
Gemfile | 2 +-
Gemfile.lock | 32 ++++++++++++++++----------------
2 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/Gemfile b/Gemfile
index de624256c16df..5286d2f6d6b8b 100644
--- a/Gemfile
+++ b/Gemfile
@@ -152,7 +152,7 @@ gem 'settingslogic', '~> 2.0.9'
gem 'version_sorter', '~> 2.1.0'
# Cache
-gem 'redis-rails', '~> 4.0.0'
+gem 'redis-rails', '~> 5.0.1'
# Redis
gem 'redis', '~> 3.2'
diff --git a/Gemfile.lock b/Gemfile.lock
index 6ea0578d9d297..165b25c170ed2 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -573,23 +573,23 @@ GEM
json
redcarpet (3.3.3)
redis (3.2.2)
- redis-actionpack (4.0.1)
- actionpack (~> 4)
- redis-rack (~> 1.5.0)
- redis-store (~> 1.1.0)
- redis-activesupport (4.1.5)
- activesupport (>= 3, < 5)
- redis-store (~> 1.1.0)
+ redis-actionpack (5.0.1)
+ actionpack (>= 4.0, < 6)
+ redis-rack (>= 1, < 3)
+ redis-store (>= 1.1.0, < 1.4.0)
+ redis-activesupport (5.0.1)
+ activesupport (>= 3, < 6)
+ redis-store (~> 1.2.0)
redis-namespace (1.5.2)
redis (~> 3.0, >= 3.0.4)
- redis-rack (1.5.0)
+ redis-rack (1.6.0)
rack (~> 1.5)
- redis-store (~> 1.1.0)
- redis-rails (4.0.0)
- redis-actionpack (~> 4)
- redis-activesupport (~> 4)
- redis-store (~> 1.1.0)
- redis-store (1.1.7)
+ redis-store (~> 1.2.0)
+ redis-rails (5.0.1)
+ redis-actionpack (~> 5.0.0)
+ redis-activesupport (~> 5.0.0)
+ redis-store (~> 1.2.0)
+ redis-store (1.2.0)
redis (>= 2.2)
request_store (1.3.1)
rerun (0.11.0)
@@ -938,7 +938,7 @@ DEPENDENCIES
redcarpet (~> 3.3.3)
redis (~> 3.2)
redis-namespace (~> 1.5.2)
- redis-rails (~> 4.0.0)
+ redis-rails (~> 5.0.1)
request_store (~> 1.3)
rerun (~> 0.11.0)
responders (~> 2.0)
@@ -994,4 +994,4 @@ DEPENDENCIES
wikicloth (= 0.8.1)
BUNDLED WITH
- 1.13.5
+ 1.13.6
--
GitLab
From d498ec98e0286efecd70040180c69c2002f9ed78 Mon Sep 17 00:00:00 2001
From: Drew Blessing <drew@gitlab.com>
Date: Fri, 4 Nov 2016 14:48:05 -0500
Subject: [PATCH 06/56] Set default Sidekiq retries to 3
By default, Sidekiq will retry 25 times with an exponential backoff.
This may result in jobs retrying for up to 21 days. Most Sidekiq
failures occur when attempting to connect to external services -
Project service hooks, web hooks, mailers, mirror updates, etc.
We should set a default retry of 3, and if that's not sufficient
individual workers can override this in the worker class.
---
changelogs/unreleased/sidekiq_default_retries.yml | 4 ++++
config/initializers/sidekiq.rb | 3 +++
2 files changed, 7 insertions(+)
create mode 100644 changelogs/unreleased/sidekiq_default_retries.yml
diff --git a/changelogs/unreleased/sidekiq_default_retries.yml b/changelogs/unreleased/sidekiq_default_retries.yml
new file mode 100644
index 0000000000000..3df2a415dbcb6
--- /dev/null
+++ b/changelogs/unreleased/sidekiq_default_retries.yml
@@ -0,0 +1,4 @@
+---
+title: Set default Sidekiq retries to 3
+merge_request: 7294
+author:
diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb
index 0455a98dbfe99..023af2af23caa 100644
--- a/config/initializers/sidekiq.rb
+++ b/config/initializers/sidekiq.rb
@@ -2,6 +2,9 @@
redis_config_hash = Gitlab::Redis.params
redis_config_hash[:namespace] = Gitlab::Redis::SIDEKIQ_NAMESPACE
+# Default is to retry 25 times with exponential backoff. That's too much.
+Sidekiq.default_worker_options = { retry: 3 }
+
Sidekiq.configure_server do |config|
config.redis = redis_config_hash
--
GitLab
From 80a2e3a9c89a34df06eb2996fd5bdbaa6223f98c Mon Sep 17 00:00:00 2001
From: the-undefined <joe@joejames.io>
Date: Wed, 2 Nov 2016 06:47:12 +0000
Subject: [PATCH 07/56] Add missing security specs for raw snippet access
Each project visibility type (Public, Internal, Private) has an access
feature spec to catch security regressions. This commit adds relevent
tests for the raw snippet path in each of these project access specs.
Refacotrings:
- Use an empty project factory for access specs
---
.../project/snippet/internal_access_spec.rb | 78 ++++++++----
.../project/snippet/private_access_spec.rb | 16 ++-
.../project/snippet/public_access_spec.rb | 116 ++++++++++++------
3 files changed, 151 insertions(+), 59 deletions(-)
diff --git a/spec/features/security/project/snippet/internal_access_spec.rb b/spec/features/security/project/snippet/internal_access_spec.rb
index db53a9cec9747..49deacc5c7437 100644
--- a/spec/features/security/project/snippet/internal_access_spec.rb
+++ b/spec/features/security/project/snippet/internal_access_spec.rb
@@ -3,7 +3,7 @@
describe "Internal Project Snippets Access", feature: true do
include AccessMatchers
- let(:project) { create(:project, :internal) }
+ let(:project) { create(:empty_project, :internal) }
let(:owner) { project.owner }
let(:master) { create(:user) }
@@ -48,31 +48,63 @@
it { is_expected.to be_denied_for :visitor }
end
- describe "GET /:project_path/snippets/:id for an internal snippet" do
- subject { namespace_project_snippet_path(project.namespace, project, internal_snippet) }
+ describe "GET /:project_path/snippets/:id" do
+ context "for an internal snippet" do
+ subject { namespace_project_snippet_path(project.namespace, project, internal_snippet) }
- it { is_expected.to be_allowed_for :admin }
- it { is_expected.to be_allowed_for owner }
- it { is_expected.to be_allowed_for master }
- it { is_expected.to be_allowed_for developer }
- it { is_expected.to be_allowed_for reporter }
- it { is_expected.to be_allowed_for guest }
- it { is_expected.to be_allowed_for :user }
- it { is_expected.to be_denied_for :external }
- it { is_expected.to be_denied_for :visitor }
+ it { is_expected.to be_allowed_for :admin }
+ it { is_expected.to be_allowed_for owner }
+ it { is_expected.to be_allowed_for master }
+ it { is_expected.to be_allowed_for developer }
+ it { is_expected.to be_allowed_for reporter }
+ it { is_expected.to be_allowed_for guest }
+ it { is_expected.to be_allowed_for :user }
+ it { is_expected.to be_denied_for :external }
+ it { is_expected.to be_denied_for :visitor }
+ end
+
+ context "for a private snippet" do
+ subject { namespace_project_snippet_path(project.namespace, project, private_snippet) }
+
+ it { is_expected.to be_allowed_for :admin }
+ it { is_expected.to be_allowed_for owner }
+ it { is_expected.to be_allowed_for master }
+ it { is_expected.to be_allowed_for developer }
+ it { is_expected.to be_allowed_for reporter }
+ it { is_expected.to be_allowed_for guest }
+ it { is_expected.to be_denied_for :user }
+ it { is_expected.to be_denied_for :external }
+ it { is_expected.to be_denied_for :visitor }
+ end
end
- describe "GET /:project_path/snippets/:id for a private snippet" do
- subject { namespace_project_snippet_path(project.namespace, project, private_snippet) }
+ describe "GET /:project_path/snippets/:id/raw" do
+ context "for an internal snippet" do
+ subject { raw_namespace_project_snippet_path(project.namespace, project, internal_snippet) }
- it { is_expected.to be_allowed_for :admin }
- it { is_expected.to be_allowed_for owner }
- it { is_expected.to be_allowed_for master }
- it { is_expected.to be_allowed_for developer }
- it { is_expected.to be_allowed_for reporter }
- it { is_expected.to be_allowed_for guest }
- it { is_expected.to be_denied_for :user }
- it { is_expected.to be_denied_for :external }
- it { is_expected.to be_denied_for :visitor }
+ it { is_expected.to be_allowed_for :admin }
+ it { is_expected.to be_allowed_for owner }
+ it { is_expected.to be_allowed_for master }
+ it { is_expected.to be_allowed_for developer }
+ it { is_expected.to be_allowed_for reporter }
+ it { is_expected.to be_allowed_for guest }
+ it { is_expected.to be_allowed_for :user }
+ it { is_expected.to be_denied_for :external }
+ it { is_expected.to be_denied_for :visitor }
+ end
+
+ context "for a private snippet" do
+ subject { raw_namespace_project_snippet_path(project.namespace, project, private_snippet) }
+
+ it { is_expected.to be_allowed_for :admin }
+ it { is_expected.to be_allowed_for owner }
+ it { is_expected.to be_allowed_for master }
+ it { is_expected.to be_allowed_for developer }
+ it { is_expected.to be_allowed_for reporter }
+ it { is_expected.to be_allowed_for guest }
+ it { is_expected.to be_denied_for :user }
+ it { is_expected.to be_denied_for :external }
+ it { is_expected.to be_denied_for :visitor }
+ end
end
end
diff --git a/spec/features/security/project/snippet/private_access_spec.rb b/spec/features/security/project/snippet/private_access_spec.rb
index d23d645c8e56f..a1bfc076d9952 100644
--- a/spec/features/security/project/snippet/private_access_spec.rb
+++ b/spec/features/security/project/snippet/private_access_spec.rb
@@ -3,7 +3,7 @@
describe "Private Project Snippets Access", feature: true do
include AccessMatchers
- let(:project) { create(:project, :private) }
+ let(:project) { create(:empty_project, :private) }
let(:owner) { project.owner }
let(:master) { create(:user) }
@@ -60,4 +60,18 @@
it { is_expected.to be_denied_for :external }
it { is_expected.to be_denied_for :visitor }
end
+
+ describe "GET /:project_path/snippets/:id/raw for a private snippet" do
+ subject { raw_namespace_project_snippet_path(project.namespace, project, private_snippet) }
+
+ it { is_expected.to be_allowed_for :admin }
+ it { is_expected.to be_allowed_for owner }
+ it { is_expected.to be_allowed_for master }
+ it { is_expected.to be_allowed_for developer }
+ it { is_expected.to be_allowed_for reporter }
+ it { is_expected.to be_allowed_for guest }
+ it { is_expected.to be_denied_for :user }
+ it { is_expected.to be_denied_for :external }
+ it { is_expected.to be_denied_for :visitor }
+ end
end
diff --git a/spec/features/security/project/snippet/public_access_spec.rb b/spec/features/security/project/snippet/public_access_spec.rb
index e3665b6116ab8..30bcd87ef0496 100644
--- a/spec/features/security/project/snippet/public_access_spec.rb
+++ b/spec/features/security/project/snippet/public_access_spec.rb
@@ -3,7 +3,7 @@
describe "Public Project Snippets Access", feature: true do
include AccessMatchers
- let(:project) { create(:project, :public) }
+ let(:project) { create(:empty_project, :public) }
let(:owner) { project.owner }
let(:master) { create(:user) }
@@ -49,45 +49,91 @@
it { is_expected.to be_denied_for :visitor }
end
- describe "GET /:project_path/snippets/:id for a public snippet" do
- subject { namespace_project_snippet_path(project.namespace, project, public_snippet) }
+ describe "GET /:project_path/snippets/:id" do
+ context "for a public snippet" do
+ subject { namespace_project_snippet_path(project.namespace, project, public_snippet) }
- it { is_expected.to be_allowed_for :admin }
- it { is_expected.to be_allowed_for owner }
- it { is_expected.to be_allowed_for master }
- it { is_expected.to be_allowed_for developer }
- it { is_expected.to be_allowed_for reporter }
- it { is_expected.to be_allowed_for guest }
- it { is_expected.to be_allowed_for :user }
- it { is_expected.to be_allowed_for :external }
- it { is_expected.to be_allowed_for :visitor }
- end
+ it { is_expected.to be_allowed_for :admin }
+ it { is_expected.to be_allowed_for owner }
+ it { is_expected.to be_allowed_for master }
+ it { is_expected.to be_allowed_for developer }
+ it { is_expected.to be_allowed_for reporter }
+ it { is_expected.to be_allowed_for guest }
+ it { is_expected.to be_allowed_for :user }
+ it { is_expected.to be_allowed_for :external }
+ it { is_expected.to be_allowed_for :visitor }
+ end
- describe "GET /:project_path/snippets/:id for an internal snippet" do
- subject { namespace_project_snippet_path(project.namespace, project, internal_snippet) }
+ context "for an internal snippet" do
+ subject { namespace_project_snippet_path(project.namespace, project, internal_snippet) }
- it { is_expected.to be_allowed_for :admin }
- it { is_expected.to be_allowed_for owner }
- it { is_expected.to be_allowed_for master }
- it { is_expected.to be_allowed_for developer }
- it { is_expected.to be_allowed_for reporter }
- it { is_expected.to be_allowed_for guest }
- it { is_expected.to be_allowed_for :user }
- it { is_expected.to be_denied_for :external }
- it { is_expected.to be_denied_for :visitor }
+ it { is_expected.to be_allowed_for :admin }
+ it { is_expected.to be_allowed_for owner }
+ it { is_expected.to be_allowed_for master }
+ it { is_expected.to be_allowed_for developer }
+ it { is_expected.to be_allowed_for reporter }
+ it { is_expected.to be_allowed_for guest }
+ it { is_expected.to be_allowed_for :user }
+ it { is_expected.to be_denied_for :external }
+ it { is_expected.to be_denied_for :visitor }
+ end
+
+ context "for a private snippet" do
+ subject { namespace_project_snippet_path(project.namespace, project, private_snippet) }
+
+ it { is_expected.to be_allowed_for :admin }
+ it { is_expected.to be_allowed_for owner }
+ it { is_expected.to be_allowed_for master }
+ it { is_expected.to be_allowed_for developer }
+ it { is_expected.to be_allowed_for reporter }
+ it { is_expected.to be_allowed_for guest }
+ it { is_expected.to be_denied_for :user }
+ it { is_expected.to be_denied_for :external }
+ it { is_expected.to be_denied_for :visitor }
+ end
end
- describe "GET /:project_path/snippets/:id for a private snippet" do
- subject { namespace_project_snippet_path(project.namespace, project, private_snippet) }
+ describe "GET /:project_path/snippets/:id/raw" do
+ context "for a public snippet" do
+ subject { raw_namespace_project_snippet_path(project.namespace, project, public_snippet) }
- it { is_expected.to be_allowed_for :admin }
- it { is_expected.to be_allowed_for owner }
- it { is_expected.to be_allowed_for master }
- it { is_expected.to be_allowed_for developer }
- it { is_expected.to be_allowed_for reporter }
- it { is_expected.to be_allowed_for guest }
- it { is_expected.to be_denied_for :user }
- it { is_expected.to be_denied_for :external }
- it { is_expected.to be_denied_for :visitor }
+ it { is_expected.to be_allowed_for :admin }
+ it { is_expected.to be_allowed_for owner }
+ it { is_expected.to be_allowed_for master }
+ it { is_expected.to be_allowed_for developer }
+ it { is_expected.to be_allowed_for reporter }
+ it { is_expected.to be_allowed_for guest }
+ it { is_expected.to be_allowed_for :user }
+ it { is_expected.to be_allowed_for :external }
+ it { is_expected.to be_allowed_for :visitor }
+ end
+
+ context "for an internal snippet" do
+ subject { raw_namespace_project_snippet_path(project.namespace, project, internal_snippet) }
+
+ it { is_expected.to be_allowed_for :admin }
+ it { is_expected.to be_allowed_for owner }
+ it { is_expected.to be_allowed_for master }
+ it { is_expected.to be_allowed_for developer }
+ it { is_expected.to be_allowed_for reporter }
+ it { is_expected.to be_allowed_for guest }
+ it { is_expected.to be_allowed_for :user }
+ it { is_expected.to be_denied_for :external }
+ it { is_expected.to be_denied_for :visitor }
+ end
+
+ context "for a private snippet" do
+ subject { raw_namespace_project_snippet_path(project.namespace, project, private_snippet) }
+
+ it { is_expected.to be_allowed_for :admin }
+ it { is_expected.to be_allowed_for owner }
+ it { is_expected.to be_allowed_for master }
+ it { is_expected.to be_allowed_for developer }
+ it { is_expected.to be_allowed_for reporter }
+ it { is_expected.to be_allowed_for guest }
+ it { is_expected.to be_denied_for :user }
+ it { is_expected.to be_denied_for :external }
+ it { is_expected.to be_denied_for :visitor }
+ end
end
end
--
GitLab
From c16d5bcd48bb03f0eb145da4be9460cca87d1f91 Mon Sep 17 00:00:00 2001
From: winniehell <git@winniehell.de>
Date: Sat, 5 Nov 2016 11:53:23 +0100
Subject: [PATCH 08/56] Move Ajax interceptor into describe block (!7304)
---
.../boards/boards_store_spec.js.es6 | 215 +++++++++---------
spec/javascripts/boards/list_spec.js.es6 | 5 +
spec/javascripts/boards/mock_data.js.es6 | 4 +-
3 files changed, 116 insertions(+), 108 deletions(-)
diff --git a/spec/javascripts/boards/boards_store_spec.js.es6 b/spec/javascripts/boards/boards_store_spec.js.es6
index 6208c2386b04a..b84dfc8197beb 100644
--- a/spec/javascripts/boards/boards_store_spec.js.es6
+++ b/spec/javascripts/boards/boards_store_spec.js.es6
@@ -13,8 +13,9 @@
//= require boards/stores/boards_store
//= require ./mock_data
-(() => {
+describe('Store', () => {
beforeEach(() => {
+ Vue.http.interceptors.push(boardsMockInterceptor);
gl.boardService = new BoardService('/test/issue-boards/board', '1');
gl.issueBoards.BoardsStore.create();
@@ -24,145 +25,147 @@
});
});
- describe('Store', () => {
- it('starts with a blank state', () => {
- expect(gl.issueBoards.BoardsStore.state.lists.length).toBe(0);
- });
+ afterEach(() => {
+ Vue.http.interceptors = _.without(Vue.http.interceptors, boardsMockInterceptor);
+ });
- describe('lists', () => {
- it('creates new list without persisting to DB', () => {
- gl.issueBoards.BoardsStore.addList(listObj);
+ it('starts with a blank state', () => {
+ expect(gl.issueBoards.BoardsStore.state.lists.length).toBe(0);
+ });
- expect(gl.issueBoards.BoardsStore.state.lists.length).toBe(1);
- });
+ describe('lists', () => {
+ it('creates new list without persisting to DB', () => {
+ gl.issueBoards.BoardsStore.addList(listObj);
- it('finds list by ID', () => {
- gl.issueBoards.BoardsStore.addList(listObj);
- const list = gl.issueBoards.BoardsStore.findList('id', 1);
+ expect(gl.issueBoards.BoardsStore.state.lists.length).toBe(1);
+ });
- expect(list.id).toBe(1);
- });
+ it('finds list by ID', () => {
+ gl.issueBoards.BoardsStore.addList(listObj);
+ const list = gl.issueBoards.BoardsStore.findList('id', 1);
- it('finds list by type', () => {
- gl.issueBoards.BoardsStore.addList(listObj);
- const list = gl.issueBoards.BoardsStore.findList('type', 'label');
+ expect(list.id).toBe(1);
+ });
- expect(list).toBeDefined();
- });
+ it('finds list by type', () => {
+ gl.issueBoards.BoardsStore.addList(listObj);
+ const list = gl.issueBoards.BoardsStore.findList('type', 'label');
- it('finds list limited by type', () => {
- gl.issueBoards.BoardsStore.addList({
- id: 1,
- position: 0,
- title: 'Test',
- list_type: 'backlog'
- });
- const list = gl.issueBoards.BoardsStore.findList('id', 1, 'backlog');
+ expect(list).toBeDefined();
+ });
- expect(list).toBeDefined();
+ it('finds list limited by type', () => {
+ gl.issueBoards.BoardsStore.addList({
+ id: 1,
+ position: 0,
+ title: 'Test',
+ list_type: 'backlog'
});
+ const list = gl.issueBoards.BoardsStore.findList('id', 1, 'backlog');
- it('gets issue when new list added', (done) => {
- gl.issueBoards.BoardsStore.addList(listObj);
- const list = gl.issueBoards.BoardsStore.findList('id', 1);
+ expect(list).toBeDefined();
+ });
- expect(gl.issueBoards.BoardsStore.state.lists.length).toBe(1);
+ it('gets issue when new list added', (done) => {
+ gl.issueBoards.BoardsStore.addList(listObj);
+ const list = gl.issueBoards.BoardsStore.findList('id', 1);
- setTimeout(() => {
- expect(list.issues.length).toBe(1);
- expect(list.issues[0].id).toBe(1);
- done();
- }, 0);
- });
+ expect(gl.issueBoards.BoardsStore.state.lists.length).toBe(1);
- it('persists new list', (done) => {
- gl.issueBoards.BoardsStore.new({
- title: 'Test',
- type: 'label',
- label: {
- id: 1,
- title: 'Testing',
- color: 'red',
- description: 'testing;'
- }
- });
- expect(gl.issueBoards.BoardsStore.state.lists.length).toBe(1);
-
- setTimeout(() => {
- const list = gl.issueBoards.BoardsStore.findList('id', 1);
- expect(list).toBeDefined();
- expect(list.id).toBe(1);
- expect(list.position).toBe(0);
- done();
- }, 0);
- });
+ setTimeout(() => {
+ expect(list.issues.length).toBe(1);
+ expect(list.issues[0].id).toBe(1);
+ done();
+ }, 0);
+ });
- it('check for blank state adding', () => {
- expect(gl.issueBoards.BoardsStore.shouldAddBlankState()).toBe(true);
+ it('persists new list', (done) => {
+ gl.issueBoards.BoardsStore.new({
+ title: 'Test',
+ type: 'label',
+ label: {
+ id: 1,
+ title: 'Testing',
+ color: 'red',
+ description: 'testing;'
+ }
});
+ expect(gl.issueBoards.BoardsStore.state.lists.length).toBe(1);
- it('check for blank state not adding', () => {
- gl.issueBoards.BoardsStore.addList(listObj);
- expect(gl.issueBoards.BoardsStore.shouldAddBlankState()).toBe(false);
- });
+ setTimeout(() => {
+ const list = gl.issueBoards.BoardsStore.findList('id', 1);
+ expect(list).toBeDefined();
+ expect(list.id).toBe(1);
+ expect(list.position).toBe(0);
+ done();
+ }, 0);
+ });
- it('check for blank state adding when backlog & done list exist', () => {
- gl.issueBoards.BoardsStore.addList({
- list_type: 'backlog'
- });
- gl.issueBoards.BoardsStore.addList({
- list_type: 'done'
- });
+ it('check for blank state adding', () => {
+ expect(gl.issueBoards.BoardsStore.shouldAddBlankState()).toBe(true);
+ });
+
+ it('check for blank state not adding', () => {
+ gl.issueBoards.BoardsStore.addList(listObj);
+ expect(gl.issueBoards.BoardsStore.shouldAddBlankState()).toBe(false);
+ });
- expect(gl.issueBoards.BoardsStore.shouldAddBlankState()).toBe(true);
+ it('check for blank state adding when backlog & done list exist', () => {
+ gl.issueBoards.BoardsStore.addList({
+ list_type: 'backlog'
});
+ gl.issueBoards.BoardsStore.addList({
+ list_type: 'done'
+ });
+
+ expect(gl.issueBoards.BoardsStore.shouldAddBlankState()).toBe(true);
+ });
- it('adds the blank state', () => {
- gl.issueBoards.BoardsStore.addBlankState();
+ it('adds the blank state', () => {
+ gl.issueBoards.BoardsStore.addBlankState();
- const list = gl.issueBoards.BoardsStore.findList('type', 'blank', 'blank');
- expect(list).toBeDefined();
- });
+ const list = gl.issueBoards.BoardsStore.findList('type', 'blank', 'blank');
+ expect(list).toBeDefined();
+ });
- it('removes list from state', () => {
- gl.issueBoards.BoardsStore.addList(listObj);
+ it('removes list from state', () => {
+ gl.issueBoards.BoardsStore.addList(listObj);
- expect(gl.issueBoards.BoardsStore.state.lists.length).toBe(1);
+ expect(gl.issueBoards.BoardsStore.state.lists.length).toBe(1);
- gl.issueBoards.BoardsStore.removeList(1, 'label');
+ gl.issueBoards.BoardsStore.removeList(1, 'label');
- expect(gl.issueBoards.BoardsStore.state.lists.length).toBe(0);
- });
+ expect(gl.issueBoards.BoardsStore.state.lists.length).toBe(0);
+ });
- it('moves the position of lists', () => {
- const listOne = gl.issueBoards.BoardsStore.addList(listObj),
- listTwo = gl.issueBoards.BoardsStore.addList(listObjDuplicate);
+ it('moves the position of lists', () => {
+ const listOne = gl.issueBoards.BoardsStore.addList(listObj),
+ listTwo = gl.issueBoards.BoardsStore.addList(listObjDuplicate);
- expect(gl.issueBoards.BoardsStore.state.lists.length).toBe(2);
+ expect(gl.issueBoards.BoardsStore.state.lists.length).toBe(2);
- gl.issueBoards.BoardsStore.moveList(listOne, ['2', '1']);
+ gl.issueBoards.BoardsStore.moveList(listOne, ['2', '1']);
- expect(listOne.position).toBe(1);
- });
+ expect(listOne.position).toBe(1);
+ });
- it('moves an issue from one list to another', (done) => {
- const listOne = gl.issueBoards.BoardsStore.addList(listObj),
- listTwo = gl.issueBoards.BoardsStore.addList(listObjDuplicate);
+ it('moves an issue from one list to another', (done) => {
+ const listOne = gl.issueBoards.BoardsStore.addList(listObj),
+ listTwo = gl.issueBoards.BoardsStore.addList(listObjDuplicate);
- expect(gl.issueBoards.BoardsStore.state.lists.length).toBe(2);
+ expect(gl.issueBoards.BoardsStore.state.lists.length).toBe(2);
- setTimeout(() => {
- expect(listOne.issues.length).toBe(1);
- expect(listTwo.issues.length).toBe(1);
+ setTimeout(() => {
+ expect(listOne.issues.length).toBe(1);
+ expect(listTwo.issues.length).toBe(1);
- gl.issueBoards.BoardsStore.moveIssueToList(listOne, listTwo, listOne.findIssue(1));
+ gl.issueBoards.BoardsStore.moveIssueToList(listOne, listTwo, listOne.findIssue(1));
- expect(listOne.issues.length).toBe(0);
- expect(listTwo.issues.length).toBe(1);
+ expect(listOne.issues.length).toBe(0);
+ expect(listTwo.issues.length).toBe(1);
- done();
- }, 0);
- });
+ done();
+ }, 0);
});
});
-})();
+});
diff --git a/spec/javascripts/boards/list_spec.js.es6 b/spec/javascripts/boards/list_spec.js.es6
index 1a0427fdd90e1..dfbcbe3a7c1db 100644
--- a/spec/javascripts/boards/list_spec.js.es6
+++ b/spec/javascripts/boards/list_spec.js.es6
@@ -17,12 +17,17 @@ describe('List model', () => {
let list;
beforeEach(() => {
+ Vue.http.interceptors.push(boardsMockInterceptor);
gl.boardService = new BoardService('/test/issue-boards/board', '1');
gl.issueBoards.BoardsStore.create();
list = new List(listObj);
});
+ afterEach(() => {
+ Vue.http.interceptors = _.without(Vue.http.interceptors, boardsMockInterceptor);
+ });
+
it('gets issues when created', (done) => {
setTimeout(() => {
expect(list.issues.length).toBe(1);
diff --git a/spec/javascripts/boards/mock_data.js.es6 b/spec/javascripts/boards/mock_data.js.es6
index 80d05e8a1a3f0..fcb3d8f17d8c6 100644
--- a/spec/javascripts/boards/mock_data.js.es6
+++ b/spec/javascripts/boards/mock_data.js.es6
@@ -48,10 +48,10 @@ const BoardsMockData = {
}
};
-Vue.http.interceptors.push((request, next) => {
+const boardsMockInterceptor = (request, next) => {
const body = BoardsMockData[request.method][request.url];
next(request.respondWith(JSON.stringify(body), {
status: 200
}));
-});
+};
--
GitLab
From 56264f35d40e3ebcebf2b88b9ff6f3a8f4f545e9 Mon Sep 17 00:00:00 2001
From: Hiroyuki Sato <h-sato@ruby-dev.jp>
Date: Fri, 28 Oct 2016 20:38:14 +0900
Subject: [PATCH 09/56] Network page appear with an error message when entering
nonexistent git revision
---
app/assets/javascripts/network/network_bundle.js | 2 ++
app/controllers/projects/network_controller.rb | 4 +++-
app/views/projects/network/show.html.haml | 5 +++--
lib/extracts_path.rb | 4 ++--
4 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/app/assets/javascripts/network/network_bundle.js b/app/assets/javascripts/network/network_bundle.js
index 42d6799c82fc9..a192273a18060 100644
--- a/app/assets/javascripts/network/network_bundle.js
+++ b/app/assets/javascripts/network/network_bundle.js
@@ -9,6 +9,8 @@
(function() {
$(function() {
+ if (!$(".network-graph").length) return;
+
var network_graph;
network_graph = new Network({
url: $(".network-graph").attr('data-url'),
diff --git a/app/controllers/projects/network_controller.rb b/app/controllers/projects/network_controller.rb
index 34318391dd909..54783bbc5bbec 100644
--- a/app/controllers/projects/network_controller.rb
+++ b/app/controllers/projects/network_controller.rb
@@ -11,7 +11,9 @@ def show
@commit_url = namespace_project_commit_path(@project.namespace, @project, 'ae45ca32').gsub("ae45ca32", "%s")
respond_to do |format|
- format.html
+ format.html do
+ flash.now[:alert] = "Git revision '#{params[:extended_sha1]}' does not exist." if params[:extended_sha1].present? && !@commit
+ end
format.json do
@graph = Network::Graph.new(project, @ref, @commit, @options[:filter_ref])
diff --git a/app/views/projects/network/show.html.haml b/app/views/projects/network/show.html.haml
index 29df1bab04eed..d8951e6924216 100644
--- a/app/views/projects/network/show.html.haml
+++ b/app/views/projects/network/show.html.haml
@@ -17,5 +17,6 @@
= check_box_tag :filter_ref, 1, @options[:filter_ref]
%span Begin with the selected commit
- .network-graph{ data: { url: @url, commit_url: @commit_url, ref: @ref, commit_id: @commit.id } }
- = spinner nil, true
+ - if @commit
+ .network-graph{ data: { url: @url, commit_url: @commit_url, ref: @ref, commit_id: @commit.id } }
+ = spinner nil, true
diff --git a/lib/extracts_path.rb b/lib/extracts_path.rb
index 9b74364849e5e..a8cc189a8c2a9 100644
--- a/lib/extracts_path.rb
+++ b/lib/extracts_path.rb
@@ -125,9 +125,9 @@ def assign_ref_vars
request.format = :atom if @commit
end
- end
- raise InvalidPathError unless @commit
+ raise InvalidPathError unless @commit
+ end
@hex_path = Digest::SHA1.hexdigest(@path)
@logs_path = logs_file_namespace_project_ref_path(@project.namespace,
--
GitLab
From ee7374a9fb16ca65603ef095de2856694612d68f Mon Sep 17 00:00:00 2001
From: Hiroyuki Sato <h-sato@ruby-dev.jp>
Date: Fri, 28 Oct 2016 21:48:40 +0900
Subject: [PATCH 10/56] Separete a very long line
---
app/controllers/projects/network_controller.rb | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/app/controllers/projects/network_controller.rb b/app/controllers/projects/network_controller.rb
index 54783bbc5bbec..3dbc7a492b522 100644
--- a/app/controllers/projects/network_controller.rb
+++ b/app/controllers/projects/network_controller.rb
@@ -12,7 +12,9 @@ def show
respond_to do |format|
format.html do
- flash.now[:alert] = "Git revision '#{params[:extended_sha1]}' does not exist." if params[:extended_sha1].present? && !@commit
+ if params[:extended_sha1].present? && !@commit
+ flash.now[:alert] = "Git revision '#{params[:extended_sha1]}' does not exist."
+ end
end
format.json do
--
GitLab
From b5bb98100a58a39a4982c28d868c195ead19a69e Mon Sep 17 00:00:00 2001
From: Hiroyuki Sato <h-sato@ruby-dev.jp>
Date: Fri, 28 Oct 2016 21:54:55 +0900
Subject: [PATCH 11/56] Add a CHANGELOG entry
---
CHANGELOG.md | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7ed09fb1db8a6..f1826e23c8931 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -97,6 +97,40 @@ entry.
- Fix and improve `Sortable.highest_label_priority`. !7165
- Fixed sticky merge request tabs when sidebar is pinned. !7167
- Only remove right connector of first build of last stage. !7179
+ - Backups do not fail anymore when using tar on annex and custom_hooks only. !5814
+ - Adds user project membership expired event to clarify why user was removed (Callum Dryden)
+ - Trim leading and trailing whitespace on project_path (Linus Thiel)
+ - Prevent award emoji via notes for issues/MRs authored by user (barthc)
+ - Adds an optional path parameter to the Commits API to filter commits by path (Luis HGO)
+ - Fix extra space on Build sidebar on Firefox !7060
+ - Fix mobile layout issues in admin user overview page !7087
+ - Fix HipChat notifications rendering (airatshigapov, eisnerd)
+ - Add hover to trash icon in notes !7008 (blackst0ne)
+ - Fix sidekiq stats in admin area (blackst0ne)
+ - Removed delete branch tooltip !6954
+ - Escape ref and path for relative links !6050 (winniehell)
+ - Fixed link typo on /help/ui to Alerts section. !6915 (Sam Rose)
+ - Fix filtering of milestones with quotes in title (airatshigapov)
+ - Refactor less readable existance checking code from CoffeeScript !6289 (jlogandavison)
+ - Update mail_room and enable sentinel support to Reply By Email (!7101)
+ - Simpler arguments passed to named_route on toggle_award_url helper method
+ - Fix typo in framework css class. !7086 (Daniel Voogsgerd)
+ - New issue board list dropdown stays open after adding a new list
+ - Fix: Backup restore doesn't clear cache
+ - API: Fix project deploy keys 400 and 500 errors when adding an existing key. !6784 (Joshua Welsh)
+ - Replace jquery.cookie plugin with js.cookie !7085
+ - Use MergeRequestsClosingIssues cache data on Issue#closed_by_merge_requests method
+ - Fix Sign in page 'Forgot your password?' link overlaps on medium-large screens
+ - Show full status link on MR & commit pipelines
+ - Fix documents and comments on Build API `scope`
+ - Fix applying labels for GitHub-imported MRs
+ - Fix importing MR comments from GitHub
+ - Refactor email, use setter method instead AR callbacks for email attribute (Semyon Pupkov)
+ - Fix 404 on network page when entering non-existent git revision !7172 (Hiroyuki Sato)
+ - Shortened merge request modal to let clipboard button not overlap
+
+## 8.13.2
+ - Fix builds dropdown overlapping bug !7124
## 8.13.1 (2016-10-25)
--
GitLab
From 8d0e84bcf49ccb136557302313bac0833b7d91e5 Mon Sep 17 00:00:00 2001
From: Hiroyuki Sato <h-sato@ruby-dev.jp>
Date: Sat, 29 Oct 2016 00:47:15 +0900
Subject: [PATCH 12/56] Fix spinach test
---
features/project/network_graph.feature | 2 +-
features/steps/project/network_graph.rb | 4 ++++
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/features/project/network_graph.feature b/features/project/network_graph.feature
index 89a02706bd283..93c884e23c58c 100644
--- a/features/project/network_graph.feature
+++ b/features/project/network_graph.feature
@@ -43,4 +43,4 @@ Feature: Project Network Graph
Scenario: I should fail to look for a commit
When I look for a commit by ";"
- Then page status code should be 404
+ Then I should see non-existent git revision error message
diff --git a/features/steps/project/network_graph.rb b/features/steps/project/network_graph.rb
index 019b3124a863d..ff9251615c930 100644
--- a/features/steps/project/network_graph.rb
+++ b/features/steps/project/network_graph.rb
@@ -109,4 +109,8 @@ class Spinach::Features::ProjectNetworkGraph < Spinach::FeatureSteps
find('button').click
end
end
+
+ step 'I should see non-existent git revision error message' do
+ expect(page).to have_selector '.flash-alert', text: "Git revision ';' does not exist."
+ end
end
--
GitLab
From d02f7d29c07632f219e103ba256636f49329067c Mon Sep 17 00:00:00 2001
From: Hiroyuki Sato <h-sato@ruby-dev.jp>
Date: Sun, 6 Nov 2016 12:07:04 +0900
Subject: [PATCH 13/56] Update changelog
---
CHANGELOG.md | 34 -------------------
...en-entering-a-nonexistent-git-revision.yml | 4 +++
2 files changed, 4 insertions(+), 34 deletions(-)
create mode 100644 changelogs/unreleased/fix-404-on-network-when-entering-a-nonexistent-git-revision.yml
diff --git a/CHANGELOG.md b/CHANGELOG.md
index f1826e23c8931..7ed09fb1db8a6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -97,40 +97,6 @@ entry.
- Fix and improve `Sortable.highest_label_priority`. !7165
- Fixed sticky merge request tabs when sidebar is pinned. !7167
- Only remove right connector of first build of last stage. !7179
- - Backups do not fail anymore when using tar on annex and custom_hooks only. !5814
- - Adds user project membership expired event to clarify why user was removed (Callum Dryden)
- - Trim leading and trailing whitespace on project_path (Linus Thiel)
- - Prevent award emoji via notes for issues/MRs authored by user (barthc)
- - Adds an optional path parameter to the Commits API to filter commits by path (Luis HGO)
- - Fix extra space on Build sidebar on Firefox !7060
- - Fix mobile layout issues in admin user overview page !7087
- - Fix HipChat notifications rendering (airatshigapov, eisnerd)
- - Add hover to trash icon in notes !7008 (blackst0ne)
- - Fix sidekiq stats in admin area (blackst0ne)
- - Removed delete branch tooltip !6954
- - Escape ref and path for relative links !6050 (winniehell)
- - Fixed link typo on /help/ui to Alerts section. !6915 (Sam Rose)
- - Fix filtering of milestones with quotes in title (airatshigapov)
- - Refactor less readable existance checking code from CoffeeScript !6289 (jlogandavison)
- - Update mail_room and enable sentinel support to Reply By Email (!7101)
- - Simpler arguments passed to named_route on toggle_award_url helper method
- - Fix typo in framework css class. !7086 (Daniel Voogsgerd)
- - New issue board list dropdown stays open after adding a new list
- - Fix: Backup restore doesn't clear cache
- - API: Fix project deploy keys 400 and 500 errors when adding an existing key. !6784 (Joshua Welsh)
- - Replace jquery.cookie plugin with js.cookie !7085
- - Use MergeRequestsClosingIssues cache data on Issue#closed_by_merge_requests method
- - Fix Sign in page 'Forgot your password?' link overlaps on medium-large screens
- - Show full status link on MR & commit pipelines
- - Fix documents and comments on Build API `scope`
- - Fix applying labels for GitHub-imported MRs
- - Fix importing MR comments from GitHub
- - Refactor email, use setter method instead AR callbacks for email attribute (Semyon Pupkov)
- - Fix 404 on network page when entering non-existent git revision !7172 (Hiroyuki Sato)
- - Shortened merge request modal to let clipboard button not overlap
-
-## 8.13.2
- - Fix builds dropdown overlapping bug !7124
## 8.13.1 (2016-10-25)
diff --git a/changelogs/unreleased/fix-404-on-network-when-entering-a-nonexistent-git-revision.yml b/changelogs/unreleased/fix-404-on-network-when-entering-a-nonexistent-git-revision.yml
new file mode 100644
index 0000000000000..d1bc8ea2eb154
--- /dev/null
+++ b/changelogs/unreleased/fix-404-on-network-when-entering-a-nonexistent-git-revision.yml
@@ -0,0 +1,4 @@
+---
+title: Fix 404 on network page when entering non-existent git revision
+merge_request: 7172
+author: Hiroyuki Sato
--
GitLab
From d8cc8d7adc4c50843ecacc77a30b2e6db4e3fd84 Mon Sep 17 00:00:00 2001
From: Hiroyuki Sato <h-sato@ruby-dev.jp>
Date: Sun, 6 Nov 2016 12:33:39 +0900
Subject: [PATCH 14/56] Remove 'extended_sha1' option from ExtractsPath module
---
.../projects/network_controller.rb | 1 +
app/views/projects/network/show.html.haml | 2 +-
lib/extracts_path.rb | 20 ++++++++-----------
3 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/app/controllers/projects/network_controller.rb b/app/controllers/projects/network_controller.rb
index 3dbc7a492b522..e29e5994c4b87 100644
--- a/app/controllers/projects/network_controller.rb
+++ b/app/controllers/projects/network_controller.rb
@@ -9,6 +9,7 @@ class Projects::NetworkController < Projects::ApplicationController
def show
@url = namespace_project_network_path(@project.namespace, @project, @ref, @options.merge(format: :json))
@commit_url = namespace_project_commit_path(@project.namespace, @project, 'ae45ca32').gsub("ae45ca32", "%s")
+ @commit = @repo.commit(params[:extended_sha1]) if params[:extended_sha1].present?
respond_to do |format|
format.html do
diff --git a/app/views/projects/network/show.html.haml b/app/views/projects/network/show.html.haml
index d8951e6924216..af56587344430 100644
--- a/app/views/projects/network/show.html.haml
+++ b/app/views/projects/network/show.html.haml
@@ -8,7 +8,7 @@
.project-network
.controls
= form_tag namespace_project_network_path(@project.namespace, @project, @id), method: :get, class: 'form-inline network-form' do |f|
- = text_field_tag :extended_sha1, @options[:extended_sha1], placeholder: "Git revision", class: 'search-input form-control input-mx-250 search-sha'
+ = text_field_tag :extended_sha1, params[:extended_sha1], placeholder: "Git revision", class: 'search-input form-control input-mx-250 search-sha'
= button_tag class: 'btn btn-success' do
= icon('search')
.inline.prepend-left-20
diff --git a/lib/extracts_path.rb b/lib/extracts_path.rb
index a8cc189a8c2a9..82551f1f22237 100644
--- a/lib/extracts_path.rb
+++ b/lib/extracts_path.rb
@@ -106,7 +106,7 @@ def extract_ref_without_atom(id)
# resolved (e.g., when a user inserts an invalid path or ref).
def assign_ref_vars
# assign allowed options
- allowed_options = ["filter_ref", "extended_sha1"]
+ allowed_options = ["filter_ref"]
@options = params.select {|key, value| allowed_options.include?(key) && !value.blank? }
@options = HashWithIndifferentAccess.new(@options)
@@ -114,21 +114,17 @@ def assign_ref_vars
@ref, @path = extract_ref(@id)
@repo = @project.repository
- if @options[:extended_sha1].present?
- @commit = @repo.commit(@options[:extended_sha1])
- else
- @commit = @repo.commit(@ref)
-
- if @path.empty? && !@commit && @id.ends_with?('.atom')
- @id = @ref = extract_ref_without_atom(@id)
- @commit = @repo.commit(@ref)
+ @commit = @repo.commit(@ref)
- request.format = :atom if @commit
- end
+ if @path.empty? && !@commit && @id.ends_with?('.atom')
+ @id = @ref = extract_ref_without_atom(@id)
+ @commit = @repo.commit(@ref)
- raise InvalidPathError unless @commit
+ request.format = :atom if @commit
end
+ raise InvalidPathError unless @commit
+
@hex_path = Digest::SHA1.hexdigest(@path)
@logs_path = logs_file_namespace_project_ref_path(@project.namespace,
@project, @ref, @path)
--
GitLab
From 2b4700b2bee77ef03db53aa06579fd73b242bbe7 Mon Sep 17 00:00:00 2001
From: bogdanvlviv <bogdanvlviv@gmail.com>
Date: Sun, 6 Nov 2016 21:27:51 +0200
Subject: [PATCH 15/56] Use method helper instead of add_template_helper
---
app/mailers/base_mailer.rb | 4 ++--
app/mailers/notify.rb | 12 ++++++------
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/app/mailers/base_mailer.rb b/app/mailers/base_mailer.rb
index 61a574d3dc0d1..79c3c2e62c5a9 100644
--- a/app/mailers/base_mailer.rb
+++ b/app/mailers/base_mailer.rb
@@ -1,6 +1,6 @@
class BaseMailer < ActionMailer::Base
- add_template_helper ApplicationHelper
- add_template_helper GitlabMarkdownHelper
+ helper ApplicationHelper
+ helper GitlabMarkdownHelper
attr_accessor :current_user
helper_method :current_user, :can?
diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb
index eca6ec2976718..0bc1c19e9cd3b 100644
--- a/app/mailers/notify.rb
+++ b/app/mailers/notify.rb
@@ -10,12 +10,12 @@ class Notify < BaseMailer
include Emails::Pipelines
include Emails::Members
- add_template_helper MergeRequestsHelper
- add_template_helper DiffHelper
- add_template_helper BlobHelper
- add_template_helper EmailsHelper
- add_template_helper MembersHelper
- add_template_helper GitlabRoutingHelper
+ helper MergeRequestsHelper
+ helper DiffHelper
+ helper BlobHelper
+ helper EmailsHelper
+ helper MembersHelper
+ helper GitlabRoutingHelper
def test_email(recipient_email, subject, body)
mail(to: recipient_email,
--
GitLab
From a8f2fceadd60db759522c5669b99e68029df912e Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Sun, 6 Nov 2016 13:54:54 -0800
Subject: [PATCH 16/56] Add an index for project_id in project_import_data to
improve performance
We see that many slow queries on GitLab.com are dominated by finding
the project import data for a specific project. Adding an index is
the most straightforward way of fixing this.
Closes #23748
---
.../unreleased/add-project-import-data-index.yml | 4 ++++
...06185620_add_project_import_data_project_index.rb | 12 ++++++++++++
db/schema.rb | 4 +++-
3 files changed, 19 insertions(+), 1 deletion(-)
create mode 100644 changelogs/unreleased/add-project-import-data-index.yml
create mode 100644 db/migrate/20161106185620_add_project_import_data_project_index.rb
diff --git a/changelogs/unreleased/add-project-import-data-index.yml b/changelogs/unreleased/add-project-import-data-index.yml
new file mode 100644
index 0000000000000..f5e4005f544fa
--- /dev/null
+++ b/changelogs/unreleased/add-project-import-data-index.yml
@@ -0,0 +1,4 @@
+---
+title: Add an index for project_id in project_import_data to improve performance
+merge_request:
+author:
diff --git a/db/migrate/20161106185620_add_project_import_data_project_index.rb b/db/migrate/20161106185620_add_project_import_data_project_index.rb
new file mode 100644
index 0000000000000..750a6a8c51eba
--- /dev/null
+++ b/db/migrate/20161106185620_add_project_import_data_project_index.rb
@@ -0,0 +1,12 @@
+class AddProjectImportDataProjectIndex < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ # Set this constant to true if this migration requires downtime.
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ def change
+ add_concurrent_index :project_import_data, :project_id
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 5476b0c93e5c7..212b0ed1c58e1 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20161103171205) do
+ActiveRecord::Schema.define(version: 20161106185620) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -867,6 +867,8 @@
t.string "encrypted_credentials_salt"
end
+ add_index "project_import_data", ["project_id"], name: "index_project_import_data_on_project_id", using: :btree
+
create_table "projects", force: :cascade do |t|
t.string "name"
t.string "path"
--
GitLab
From 0c36b810abede73b3cf59f951e7467dbac54ec98 Mon Sep 17 00:00:00 2001
From: Sam Rose <samrose3@gmail.com>
Date: Sun, 6 Nov 2016 13:26:44 -0500
Subject: [PATCH 17/56] Fix broken link to observatory cli on Frontend Dev
Guide
---
changelogs/unreleased/broken-link-frontend-dev-guide.yml | 4 ++++
doc/development/frontend.md | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
create mode 100644 changelogs/unreleased/broken-link-frontend-dev-guide.yml
diff --git a/changelogs/unreleased/broken-link-frontend-dev-guide.yml b/changelogs/unreleased/broken-link-frontend-dev-guide.yml
new file mode 100644
index 0000000000000..d7b6f4a77018a
--- /dev/null
+++ b/changelogs/unreleased/broken-link-frontend-dev-guide.yml
@@ -0,0 +1,4 @@
+---
+title: Fix broken link to observatory cli on Frontend Dev Guide
+merge_request:
+author: Sam Rose
diff --git a/doc/development/frontend.md b/doc/development/frontend.md
index 1d7d9127a6467..ec8f2d6531c9f 100644
--- a/doc/development/frontend.md
+++ b/doc/development/frontend.md
@@ -228,7 +228,7 @@ For our currently-supported browsers, see our [requirements][requirements].
[page-specific-js-example]: https://gitlab.com/gitlab-org/gitlab-ce/blob/13bb9ed77f405c5f6ee4fdbc964ecf635c9a223f/app/views/projects/graphs/_head.html.haml#L6-8
[chrome-accessibility-developer-tools]: https://github.com/GoogleChrome/accessibility-developer-tools
[audit-rules]: https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules
-[observatory-cli]: https://github.com/mozilla/http-observatory-cli)
+[observatory-cli]: https://github.com/mozilla/http-observatory-cli
[qualys-ssl]: https://www.ssllabs.com/ssltest/analyze.html
[secure_headers]: https://github.com/twitter/secureheaders
[mdn-csp]: https://developer.mozilla.org/en-US/docs/Web/Security/CSP
--
GitLab
From 76c237460a801050e68def5ff54c15e4e74b38a8 Mon Sep 17 00:00:00 2001
From: Robert Schilling <rschilling@student.tugraz.at>
Date: Thu, 29 Sep 2016 16:33:38 +0200
Subject: [PATCH 18/56] Ability to update labels priority via API
---
.../unreleased/api-label-priorities.yml | 4 +
lib/api/entities.rb | 3 +
lib/api/labels.rb | 41 ++++++---
spec/requests/api/labels_spec.rb | 88 +++++++++++++++++--
4 files changed, 117 insertions(+), 19 deletions(-)
create mode 100644 changelogs/unreleased/api-label-priorities.yml
diff --git a/changelogs/unreleased/api-label-priorities.yml b/changelogs/unreleased/api-label-priorities.yml
new file mode 100644
index 0000000000000..85b6c2761bb1a
--- /dev/null
+++ b/changelogs/unreleased/api-label-priorities.yml
@@ -0,0 +1,4 @@
+---
+title: API: Ability to retrieve version information
+merge_request: 7286
+author: Robert Schilling
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 01e31f6f7d143..9dd36ec969e93 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -438,6 +438,9 @@ class LabelBasic < Grape::Entity
class Label < LabelBasic
expose :open_issues_count, :closed_issues_count, :open_merge_requests_count
+ expose :priority do |label, options|
+ label.priority(options[:project])
+ end
expose :subscribed do |label, options|
label.subscribed?(options[:current_user])
diff --git a/lib/api/labels.rb b/lib/api/labels.rb
index 238cea00fbab9..97218054f3764 100644
--- a/lib/api/labels.rb
+++ b/lib/api/labels.rb
@@ -11,7 +11,7 @@ class Labels < Grape::API
success Entities::Label
end
get ':id/labels' do
- present available_labels, with: Entities::Label, current_user: current_user
+ present available_labels, with: Entities::Label, current_user: current_user, project: user_project
end
desc 'Create a new label' do
@@ -21,6 +21,7 @@ class Labels < Grape::API
requires :name, type: String, desc: 'The name of the label to be created'
requires :color, type: String, desc: "The color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB)"
optional :description, type: String, desc: 'The description of label to be created'
+ optional :priority, type: Integer, desc: 'The priority of the label', allow_blank: true
end
post ':id/labels' do
authorize! :admin_label, user_project
@@ -28,10 +29,15 @@ class Labels < Grape::API
label = available_labels.find_by(title: params[:name])
conflict!('Label already exists') if label
- label = user_project.labels.create(declared(params, include_parent_namespaces: false).to_h)
+ priority = params.delete(:priority)
+ label_params = declared(params,
+ include_parent_namespaces: false,
+ include_missing: false).to_h
+ label = user_project.labels.create(label_params)
if label.valid?
- present label, with: Entities::Label, current_user: current_user
+ label.prioritize!(user_project, priority) if priority
+ present label, with: Entities::Label, current_user: current_user, project: user_project
else
render_validation_error!(label)
end
@@ -49,7 +55,7 @@ class Labels < Grape::API
label = user_project.labels.find_by(title: params[:name])
not_found!('Label') unless label
- present label.destroy, with: Entities::Label, current_user: current_user
+ present label.destroy, with: Entities::Label, current_user: current_user, project: user_project
end
desc 'Update an existing label. At least one optional parameter is required.' do
@@ -60,7 +66,8 @@ class Labels < Grape::API
optional :new_name, type: String, desc: 'The new name of the label'
optional :color, type: String, desc: "The new color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB)"
optional :description, type: String, desc: 'The new description of label'
- at_least_one_of :new_name, :color, :description
+ optional :priority, type: Integer, desc: 'The priority of the label', allow_blank: true
+ at_least_one_of :new_name, :color, :description, :priority
end
put ':id/labels' do
authorize! :admin_label, user_project
@@ -68,17 +75,25 @@ class Labels < Grape::API
label = user_project.labels.find_by(title: params[:name])
not_found!('Label not found') unless label
- update_params = declared(params,
- include_parent_namespaces: false,
- include_missing: false).to_h
+ update_priority = params.key?(:priority)
+ priority = params.delete(:priority)
+ label_params = declared(params,
+ include_parent_namespaces: false,
+ include_missing: false).to_h
# Rename new name to the actual label attribute name
- update_params['name'] = update_params.delete('new_name') if update_params.key?('new_name')
+ label_params[:name] = label_params.delete('new_name') if label_params.key?('new_name')
- if label.update(update_params)
- present label, with: Entities::Label, current_user: current_user
- else
- render_validation_error!(label)
+ render_validation_error!(label) unless label.update(label_params)
+
+ if update_priority
+ if priority.nil?
+ label.unprioritize!(user_project)
+ else
+ label.prioritize!(user_project, priority)
+ end
end
+
+ present label, with: Entities::Label, current_user: current_user, project: user_project
end
end
end
diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb
index f702dfaaf53a9..7e532912d0810 100644
--- a/spec/requests/api/labels_spec.rb
+++ b/spec/requests/api/labels_spec.rb
@@ -6,6 +6,7 @@
let(:user) { create(:user) }
let(:project) { create(:project, creator_id: user.id, namespace: user.namespace) }
let!(:label1) { create(:label, title: 'label1', project: project) }
+ let!(:priority_label) { create(:label, title: 'bug', project: project, priority: 3) }
before do
project.team << [user, :master]
@@ -21,8 +22,16 @@
expect(response).to have_http_status(200)
expect(json_response).to be_an Array
- expect(json_response.size).to eq(2)
- expect(json_response.map { |l| l['name'] }).to match_array([group_label.name, label1.name])
+ expect(json_response.size).to eq(3)
+ expect(json_response.map { |l| l['name'] }).to match_array([group_label.name, priority_label.name, label1.name])
+ expect(json_response.last['name']).to eq(label1.name)
+ expect(json_response.last['color']).to be_present
+ expect(json_response.last['description']).to be_nil
+ expect(json_response.last['open_issues_count']).to eq(0)
+ expect(json_response.last['closed_issues_count']).to eq(0)
+ expect(json_response.last['open_merge_requests_count']).to eq(0)
+ expect(json_response.last['priority']).to be_nil
+ expect(json_response.last['subscribed']).to be_falsey
end
end
@@ -31,21 +40,39 @@
post api("/projects/#{project.id}/labels", user),
name: 'Foo',
color: '#FFAABB',
- description: 'test'
+ description: 'test',
+ priority: 2
+
expect(response).to have_http_status(201)
expect(json_response['name']).to eq('Foo')
expect(json_response['color']).to eq('#FFAABB')
expect(json_response['description']).to eq('test')
+ expect(json_response['priority']).to eq(2)
end
it 'returns created label when only required params' do
post api("/projects/#{project.id}/labels", user),
name: 'Foo & Bar',
color: '#FFAABB'
+
expect(response.status).to eq(201)
expect(json_response['name']).to eq('Foo & Bar')
expect(json_response['color']).to eq('#FFAABB')
expect(json_response['description']).to be_nil
+ expect(json_response['priority']).to be_nil
+ end
+
+ it 'creates a prioritized label' do
+ post api("/projects/#{project.id}/labels", user),
+ name: 'Foo & Bar',
+ color: '#FFAABB',
+ priority: 3
+
+ expect(response.status).to eq(201)
+ expect(json_response['name']).to eq('Foo & Bar')
+ expect(json_response['color']).to eq('#FFAABB')
+ expect(json_response['description']).to be_nil
+ expect(json_response['priority']).to eq(3)
end
it 'returns a 400 bad request if name not given' do
@@ -95,6 +122,15 @@
expect(json_response['message']).to eq('Label already exists')
end
+ it 'returns 400 for invalid priority' do
+ post api("/projects/#{project.id}/labels", user),
+ name: 'Foo',
+ color: '#FFAAFFFF',
+ priority: 'foo'
+
+ expect(response).to have_http_status(400)
+ end
+
it 'returns 409 if label already exists in project' do
post api("/projects/#{project.id}/labels", user),
name: 'label1',
@@ -155,11 +191,43 @@
it 'returns 200 if description is changed' do
put api("/projects/#{project.id}/labels", user),
- name: 'label1',
+ name: 'bug',
description: 'test'
+
expect(response).to have_http_status(200)
- expect(json_response['name']).to eq(label1.name)
+ expect(json_response['name']).to eq(priority_label.name)
expect(json_response['description']).to eq('test')
+ expect(json_response['priority']).to eq(3)
+ end
+
+ it 'returns 200 if priority is changed' do
+ put api("/projects/#{project.id}/labels", user),
+ name: 'bug',
+ priority: 10
+
+ expect(response.status).to eq(200)
+ expect(json_response['name']).to eq(priority_label.name)
+ expect(json_response['priority']).to eq(10)
+ end
+
+ it 'returns 200 if a priority is added' do
+ put api("/projects/#{project.id}/labels", user),
+ name: 'label1',
+ priority: 3
+
+ expect(response.status).to eq(200)
+ expect(json_response['name']).to eq(label1.name)
+ expect(json_response['priority']).to eq(3)
+ end
+
+ it 'returns 200 if the priority is removed' do
+ put api("/projects/#{project.id}/labels", user),
+ name: priority_label.name,
+ priority: nil
+
+ expect(response.status).to eq(200)
+ expect(json_response['name']).to eq(priority_label.name)
+ expect(json_response['priority']).to be_nil
end
it 'returns 404 if label does not exist' do
@@ -178,7 +246,7 @@
it 'returns 400 if no new parameters given' do
put api("/projects/#{project.id}/labels", user), name: 'label1'
expect(response).to have_http_status(400)
- expect(json_response['error']).to eq('new_name, color, description are missing, '\
+ expect(json_response['error']).to eq('new_name, color, description, priority are missing, '\
'at least one parameter must be provided')
end
@@ -206,6 +274,14 @@
expect(response).to have_http_status(400)
expect(json_response['message']['color']).to eq(['must be a valid color code'])
end
+
+ it 'returns 400 for invalid priority' do
+ post api("/projects/#{project.id}/labels", user),
+ name: 'Foo',
+ priority: 'foo'
+
+ expect(response).to have_http_status(400)
+ end
end
describe "POST /projects/:id/labels/:label_id/subscription" do
--
GitLab
From c7c3b771205e243edece746811095c71ae151fa1 Mon Sep 17 00:00:00 2001
From: Robert Schilling <rschilling@student.tugraz.at>
Date: Fri, 4 Nov 2016 11:13:17 +0100
Subject: [PATCH 19/56] Update label's API documentation
---
doc/api/labels.md | 44 ++++++++++++++++++++++++++++++++++----------
1 file changed, 34 insertions(+), 10 deletions(-)
diff --git a/doc/api/labels.md b/doc/api/labels.md
index 656232cc9403e..b524203794986 100644
--- a/doc/api/labels.md
+++ b/doc/api/labels.md
@@ -26,7 +26,9 @@ Example response:
"description": "Bug reported by user",
"open_issues_count": 1,
"closed_issues_count": 0,
- "open_merge_requests_count": 1
+ "open_merge_requests_count": 1,
+ "subscribed": false,
+ "priority": 10
},
{
"color" : "#d9534f",
@@ -34,7 +36,9 @@ Example response:
"description": "Confirmed issue",
"open_issues_count": 2,
"closed_issues_count": 5,
- "open_merge_requests_count": 0
+ "open_merge_requests_count": 0,
+ "subscribed": false,
+ "priority": null
},
{
"name" : "critical",
@@ -42,7 +46,9 @@ Example response:
"description": "Critical issue. Need fix ASAP",
"open_issues_count": 1,
"closed_issues_count": 3,
- "open_merge_requests_count": 1
+ "open_merge_requests_count": 1,
+ "subscribed": false,
+ "priority": null
},
{
"name" : "documentation",
@@ -50,7 +56,9 @@ Example response:
"description": "Issue about documentation",
"open_issues_count": 1,
"closed_issues_count": 0,
- "open_merge_requests_count": 2
+ "open_merge_requests_count": 2,
+ "subscribed": false,
+ "priority": null
},
{
"color" : "#5cb85c",
@@ -58,7 +66,9 @@ Example response:
"description": "Enhancement proposal",
"open_issues_count": 1,
"closed_issues_count": 0,
- "open_merge_requests_count": 1
+ "open_merge_requests_count": 1,
+ "subscribed": false,
+ "priority": null
}
]
```
@@ -80,6 +90,7 @@ POST /projects/:id/labels
| `name` | string | yes | The name of the label |
| `color` | string | yes | The color of the label in 6-digit hex notation with leading `#` sign |
| `description` | string | no | The description of the label |
+| `priority` | integer | no | The priority of the label. Must be greater or equal than zero or `null` to remove the priority. |
```bash
curl --data "name=feature&color=#5843AD" --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/labels"
@@ -91,7 +102,11 @@ Example response:
{
"name" : "feature",
"color" : "#5843AD",
- "description":null
+ "open_issues_count": 1,
+ "closed_issues_count": 0,
+ "open_merge_requests_count": 1,
+ "description": null,
+ "priority": null
}
```
@@ -127,7 +142,8 @@ Example response:
"template" : false,
"project_id" : 1,
"created_at" : "2015-11-03T21:22:30.737Z",
- "id" : 9
+ "id" : 9,
+ "priority": null
}
```
@@ -151,6 +167,8 @@ PUT /projects/:id/labels
| `new_name` | string | yes if `color` is not provided | The new name of the label |
| `color` | string | yes if `new_name` is not provided | The new color of the label in 6-digit hex notation with leading `#` sign |
| `description` | string | no | The new description of the label |
+| `priority` | integer | no | The new priority of the label. Must be greater or equal than zero or `null` to remove the priority. |
+
```bash
curl --request PUT --data "name=documentation&new_name=docs&color=#8E44AD&description=Documentation" --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/labels"
@@ -162,7 +180,11 @@ Example response:
{
"color" : "#8E44AD",
"name" : "docs",
- "description": "Documentation"
+ "description": "Documentation",
+ "open_issues_count": 1,
+ "closed_issues_count": 0,
+ "open_merge_requests_count": 1,
+ "priority": null
}
```
@@ -197,7 +219,8 @@ Example response:
"open_issues_count": 0,
"closed_issues_count": 0,
"open_merge_requests_count": 0,
- "subscribed": true
+ "subscribed": true,
+ "priority": null
}
```
@@ -232,6 +255,7 @@ Example response:
"open_issues_count": 0,
"closed_issues_count": 0,
"open_merge_requests_count": 0,
- "subscribed": false
+ "subscribed": false,
+ "priority": null
}
```
--
GitLab
From 630eb119cb99eb971a9e29d83293774b5b882490 Mon Sep 17 00:00:00 2001
From: Nick Thomas <nick@gitlab.com>
Date: Fri, 4 Nov 2016 12:55:15 +0000
Subject: [PATCH 20/56] Renaming columns requires downtime
---
.../20161103171205_rename_repository_storage_column.rb | 4 ++--
doc/development/what_requires_downtime.md | 6 ++++++
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/db/migrate/20161103171205_rename_repository_storage_column.rb b/db/migrate/20161103171205_rename_repository_storage_column.rb
index e9f992793b42c..932805739394b 100644
--- a/db/migrate/20161103171205_rename_repository_storage_column.rb
+++ b/db/migrate/20161103171205_rename_repository_storage_column.rb
@@ -5,12 +5,12 @@ class RenameRepositoryStorageColumn < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
- DOWNTIME = false
+ DOWNTIME = true
# When a migration requires downtime you **must** uncomment the following
# constant and define a short and easy to understand explanation as to why the
# migration requires downtime.
- # DOWNTIME_REASON = ''
+ DOWNTIME_REASON = 'Renaming the application_settings.repository_storage column'
# When using the methods "add_concurrent_index" or "add_column_with_default"
# you must disable the use of transactions as these methods can not run in an
diff --git a/doc/development/what_requires_downtime.md b/doc/development/what_requires_downtime.md
index 2574c2c04727c..bbcd26477f34f 100644
--- a/doc/development/what_requires_downtime.md
+++ b/doc/development/what_requires_downtime.md
@@ -66,6 +66,12 @@ producing errors whenever it tries to use the `dummy` column.
As a result of the above downtime _is_ required when removing a column, even
when using PostgreSQL.
+## Renaming Columns
+
+Renaming columns requires downtime as running GitLab instances will continue
+using the old column name until a new version is deployed. This can result
+in the instance producing errors, which in turn can impact the user experience.
+
## Changing Column Constraints
Generally changing column constraints requires checking all rows in the table to
--
GitLab
From 42b238a1148fe14e8c2485438fd97dd648e6b7e3 Mon Sep 17 00:00:00 2001
From: Nick Thomas <nick@gitlab.com>
Date: Fri, 4 Nov 2016 12:56:53 +0000
Subject: [PATCH 21/56] Simplify ApplicationSetting#repository_storages
---
app/models/application_setting.rb | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 6e7a90e7d9c90..9b02a68a202ca 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -202,11 +202,7 @@ def domain_blacklist_file=(file)
end
def repository_storages
- value = read_attribute(:repository_storages)
- value = [value] if value.is_a?(String)
- value = [] if value.nil?
-
- value
+ Array(read_attribute(:repository_storages))
end
# repository_storage is still required in the API. Remove in 9.0
--
GitLab
From 29668aec46f0c7479bf974e6d27ace866c800940 Mon Sep 17 00:00:00 2001
From: Nick Thomas <nick@gitlab.com>
Date: Fri, 4 Nov 2016 04:47:39 +0000
Subject: [PATCH 22/56] Use `git update-ref --stdin -z` to speed up
TestEnv.set_repo_refs
Previously, we were calling `git update-ref <ref> <sha>` about 30 times per
test using `create(:project)` or similar.
---
spec/support/test_env.rb | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb
index c79975d8667ef..778e665500d43 100644
--- a/spec/support/test_env.rb
+++ b/spec/support/test_env.rb
@@ -204,20 +204,18 @@ def git_env
end
def set_repo_refs(repo_path, branch_sha)
+ instructions = branch_sha.map {|branch, sha| "update refs/heads/#{branch}\x00#{sha}\x00" }.join("\x00") << "\x00"
+ update_refs = %W(#{Gitlab.config.git.bin_path} update-ref --stdin -z)
+ reset = proc do
+ IO.popen(update_refs, "w") {|io| io.write(instructions) }
+ $?.success?
+ end
+
Dir.chdir(repo_path) do
- branch_sha.each do |branch, sha|
- # Try to reset without fetching to avoid using the network.
- reset = %W(#{Gitlab.config.git.bin_path} update-ref refs/heads/#{branch} #{sha})
- unless system(*reset)
- if system(*%W(#{Gitlab.config.git.bin_path} fetch origin))
- unless system(*reset)
- raise 'The fetched test seed '\
- 'does not contain the required revision.'
- end
- else
- raise 'Could not fetch test seed repository.'
- end
- end
+ # Try to reset without fetching to avoid using the network.
+ unless reset.call
+ raise 'Could not fetch test seed repository.' unless system(*%W(#{Gitlab.config.git.bin_path} fetch origin))
+ raise 'The fetched test seed does not contain the required revision.' unless reset.call
end
end
end
--
GitLab
From 95a78fb53bbddfa10db7da3b12406332a7fafacc Mon Sep 17 00:00:00 2001
From: Hiroyuki Sato <h-sato@ruby-dev.jp>
Date: Mon, 7 Nov 2016 20:44:15 +0900
Subject: [PATCH 23/56] Fix bug of json request url
---
app/controllers/projects/network_controller.rb | 13 ++++++++++---
app/views/projects/network/show.html.haml | 2 +-
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/app/controllers/projects/network_controller.rb b/app/controllers/projects/network_controller.rb
index e29e5994c4b87..e808c9c734304 100644
--- a/app/controllers/projects/network_controller.rb
+++ b/app/controllers/projects/network_controller.rb
@@ -5,16 +5,16 @@ class Projects::NetworkController < Projects::ApplicationController
before_action :require_non_empty_project
before_action :assign_ref_vars
before_action :authorize_download_code!
+ before_action :assign_extended_sha1
def show
@url = namespace_project_network_path(@project.namespace, @project, @ref, @options.merge(format: :json))
@commit_url = namespace_project_commit_path(@project.namespace, @project, 'ae45ca32').gsub("ae45ca32", "%s")
- @commit = @repo.commit(params[:extended_sha1]) if params[:extended_sha1].present?
respond_to do |format|
format.html do
- if params[:extended_sha1].present? && !@commit
- flash.now[:alert] = "Git revision '#{params[:extended_sha1]}' does not exist."
+ if @options[:extended_sha1] && !@commit
+ flash.now[:alert] = "Git revision '#{@options[:extended_sha1]}' does not exist."
end
end
@@ -23,4 +23,11 @@ def show
end
end
end
+
+ def assign_extended_sha1
+ return if params[:extended_sha1].blank?
+
+ @options[:extended_sha1] = params[:extended_sha1]
+ @commit = @repo.commit(@options[:extended_sha1])
+ end
end
diff --git a/app/views/projects/network/show.html.haml b/app/views/projects/network/show.html.haml
index af56587344430..d8951e6924216 100644
--- a/app/views/projects/network/show.html.haml
+++ b/app/views/projects/network/show.html.haml
@@ -8,7 +8,7 @@
.project-network
.controls
= form_tag namespace_project_network_path(@project.namespace, @project, @id), method: :get, class: 'form-inline network-form' do |f|
- = text_field_tag :extended_sha1, params[:extended_sha1], placeholder: "Git revision", class: 'search-input form-control input-mx-250 search-sha'
+ = text_field_tag :extended_sha1, @options[:extended_sha1], placeholder: "Git revision", class: 'search-input form-control input-mx-250 search-sha'
= button_tag class: 'btn btn-success' do
= icon('search')
.inline.prepend-left-20
--
GitLab
From 3c957c006633d2df44f0d23a3131294f9b657d2b Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Thu, 13 Oct 2016 17:53:01 +0200
Subject: [PATCH 24/56] Added tests for IssuePolicy
---
spec/policies/issue_policy_spec.rb | 119 +++++++++++++++++++++++++++++
1 file changed, 119 insertions(+)
create mode 100644 spec/policies/issue_policy_spec.rb
diff --git a/spec/policies/issue_policy_spec.rb b/spec/policies/issue_policy_spec.rb
new file mode 100644
index 0000000000000..7591bfd147145
--- /dev/null
+++ b/spec/policies/issue_policy_spec.rb
@@ -0,0 +1,119 @@
+require 'spec_helper'
+
+describe IssuePolicy, models: true do
+ let(:user) { create(:user) }
+
+ describe '#rules' do
+ context 'using a regular issue' do
+ let(:project) { create(:project, :public) }
+ let(:issue) { create(:issue, project: project) }
+ let(:policies) { described_class.abilities(user, issue).to_set }
+
+ context 'with a regular user' do
+ it 'includes the read_issue permission' do
+ expect(policies).to include(:read_issue)
+ end
+
+ it 'does not include the admin_issue permission' do
+ expect(policies).not_to include(:admin_issue)
+ end
+
+ it 'does not include the update_issue permission' do
+ expect(policies).not_to include(:update_issue)
+ end
+ end
+
+ context 'with a user that is a project reporter' do
+ before do
+ project.team << [user, :reporter]
+ end
+
+ it 'includes the read_issue permission' do
+ expect(policies).to include(:read_issue)
+ end
+
+ it 'includes the admin_issue permission' do
+ expect(policies).to include(:admin_issue)
+ end
+
+ it 'includes the update_issue permission' do
+ expect(policies).to include(:update_issue)
+ end
+ end
+
+ context 'with a user that is a project guest' do
+ before do
+ project.team << [user, :guest]
+ end
+
+ it 'includes the read_issue permission' do
+ expect(policies).to include(:read_issue)
+ end
+
+ it 'does not include the admin_issue permission' do
+ expect(policies).not_to include(:admin_issue)
+ end
+
+ it 'does not include the update_issue permission' do
+ expect(policies).not_to include(:update_issue)
+ end
+ end
+ end
+
+ context 'using a confidential issue' do
+ let(:issue) { create(:issue, :confidential) }
+
+ context 'with a regular user' do
+ let(:policies) { described_class.abilities(user, issue).to_set }
+
+ it 'does not include the read_issue permission' do
+ expect(policies).not_to include(:read_issue)
+ end
+
+ it 'does not include the admin_issue permission' do
+ expect(policies).not_to include(:admin_issue)
+ end
+
+ it 'does not include the update_issue permission' do
+ expect(policies).not_to include(:update_issue)
+ end
+ end
+
+ context 'with a user that is a project member' do
+ let(:policies) { described_class.abilities(user, issue).to_set }
+
+ before do
+ issue.project.team << [user, :reporter]
+ end
+
+ it 'includes the read_issue permission' do
+ expect(policies).to include(:read_issue)
+ end
+
+ it 'includes the admin_issue permission' do
+ expect(policies).to include(:admin_issue)
+ end
+
+ it 'includes the update_issue permission' do
+ expect(policies).to include(:update_issue)
+ end
+ end
+
+ context 'without a user' do
+ let(:policies) { described_class.abilities(nil, issue).to_set }
+
+ it 'does not include the read_issue permission' do
+ expect(policies).not_to include(:read_issue)
+ end
+
+ it 'does not include the admin_issue permission' do
+ expect(policies).not_to include(:admin_issue)
+ end
+
+ it 'does not include the update_issue permission' do
+ expect(policies).not_to include(:update_issue)
+ end
+ end
+ end
+ end
+end
--
GitLab
From 467b346f0684053081d7e762df1a2b5df5888543 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Thu, 13 Oct 2016 17:53:47 +0200
Subject: [PATCH 25/56] Add User#projects_with_reporter_access_limited_to
This method can be used to retrieve a list of projects for a user that
said user has reporter access to. This list is then be reduced down to
a specific set of projects. This allows you to reduce a list of projects
to a list of projects you have reporter access to in an efficient
manner.
---
app/models/user.rb | 10 ++++++++++
spec/models/user_spec.rb | 36 ++++++++++++++++++++++++++++++++++++
2 files changed, 46 insertions(+)
diff --git a/app/models/user.rb b/app/models/user.rb
index 65e96ee6b2e4e..c0dffa7b6ea21 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -444,6 +444,16 @@ def authorized_projects(min_access_level = nil)
Project.where("projects.id IN (#{projects_union(min_access_level).to_sql})")
end
+ # Returns the projects this user has reporter (or greater) access to, limited
+ # to at most the given projects.
+ #
+ # This method is useful when you have a list of projects and want to
+ # efficiently check to which of these projects the user has at least reporter
+ # access.
+ def projects_with_reporter_access_limited_to(projects)
+ authorized_projects(Gitlab::Access::REPORTER).where(id: projects)
+ end
+
def viewable_starred_projects
starred_projects.where("projects.visibility_level IN (?) OR projects.id IN (#{projects_union.to_sql})",
[Project::PUBLIC, Project::INTERNAL])
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index ba47479a2e104..3b152e15b618e 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1205,4 +1205,40 @@ def add_user(access)
expect(user.viewable_starred_projects).not_to include(private_project)
end
end
+
+ describe '#projects_with_reporter_access_limited_to' do
+ let(:project1) { create(:project) }
+ let(:project2) { create(:project) }
+ let(:user) { create(:user) }
+
+ before do
+ project1.team << [user, :reporter]
+ project2.team << [user, :guest]
+ end
+
+ it 'returns the projects when using a single project ID' do
+ projects = user.projects_with_reporter_access_limited_to(project1.id)
+
+ expect(projects).to eq([project1])
+ end
+
+ it 'returns the projects when using an Array of project IDs' do
+ projects = user.projects_with_reporter_access_limited_to([project1.id])
+
+ expect(projects).to eq([project1])
+ end
+
+ it 'returns the projects when using an ActiveRecord relation' do
+ projects = user.
+ projects_with_reporter_access_limited_to(Project.select(:id))
+
+ expect(projects).to eq([project1])
+ end
+
+ it 'does not return projects you do not have reporter access to' do
+ projects = user.projects_with_reporter_access_limited_to(project2.id)
+
+ expect(projects).to be_empty
+ end
+ end
end
--
GitLab
From 24261f2dbd50583ac997c3f53f78104a96fa2cd3 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Tue, 11 Oct 2016 17:37:19 +0200
Subject: [PATCH 26/56] Add the method ExternalIssue#project_id
This method returns the project's ID, making ExternalIssue slightly more
compatible with Issue (which also defines the "project_id" method).
---
app/models/external_issue.rb | 9 +++++++++
spec/models/external_issue_spec.rb | 8 +++++++-
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/app/models/external_issue.rb b/app/models/external_issue.rb
index fd9a8c1b8b7fa..91b508eb3251d 100644
--- a/app/models/external_issue.rb
+++ b/app/models/external_issue.rb
@@ -29,6 +29,15 @@ def project
@project
end
+ def project_id
+ @project.id
+ end
+
+ # Pattern used to extract `JIRA-123` issue references from text
+ def self.reference_pattern
+ @reference_pattern ||= %r{(?<issue>\b([A-Z][A-Z0-9_]+-)\d+)}
+ end
+
def to_reference(_from_project = nil)
id
end
diff --git a/spec/models/external_issue_spec.rb b/spec/models/external_issue_spec.rb
index ebba6e14578ca..2debe1289a3f7 100644
--- a/spec/models/external_issue_spec.rb
+++ b/spec/models/external_issue_spec.rb
@@ -1,7 +1,7 @@
require 'spec_helper'
describe ExternalIssue, models: true do
- let(:project) { double('project', to_reference: 'namespace1/project1') }
+ let(:project) { double('project', id: 1, to_reference: 'namespace1/project1') }
let(:issue) { described_class.new('EXT-1234', project) }
describe 'modules' do
@@ -36,4 +36,10 @@
end
end
end
+
+ describe '#project_id' do
+ it 'returns the ID of the project' do
+ expect(issue.project_id).to eq(project.id)
+ end
+ end
end
--
GitLab
From 89bb29b247b57e3b4ba053a5fd17f2087ac4414f Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Tue, 11 Oct 2016 17:31:19 +0200
Subject: [PATCH 27/56] Flush Housekeeping data from Redis specs
These specs use raw Redis objects which can not use the memory based
caching mechanism used for tests. As such we have to explicitly flush
the data from Redis before/after each spec to ensure no data lingers on.
---
spec/services/git_push_service_spec.rb | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb
index 45bc44ba17200..1c7a3d04b09fd 100644
--- a/spec/services/git_push_service_spec.rb
+++ b/spec/services/git_push_service_spec.rb
@@ -538,9 +538,16 @@
let(:housekeeping) { Projects::HousekeepingService.new(project) }
before do
+ # Flush any raw Redis data stored by the housekeeping code.
+ Gitlab::Redis.with { |conn| conn.flushall }
+
allow(Projects::HousekeepingService).to receive(:new).and_return(housekeeping)
end
+ after do
+ Gitlab::Redis.with { |conn| conn.flushall }
+ end
+
it 'does not perform housekeeping when not needed' do
expect(housekeeping).not_to receive(:execute)
--
GitLab
From f694f94c491452a50035c2ff43c8ba595c0e73aa Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Wed, 12 Oct 2016 14:01:34 +0200
Subject: [PATCH 28/56] Added IssueCollection
This class can be used to reduce a list of issues down to a subset based
on user permissions. This class operates in such a way that it can
reduce issues using as few queries as possible, if any at all.
---
app/models/concerns/issuable.rb | 5 +++
app/models/issue_collection.rb | 40 ++++++++++++++++++
app/policies/issuable_policy.rb | 2 +-
app/policies/issue_policy.rb | 9 +----
spec/models/concerns/issuable_spec.rb | 21 ++++++++++
spec/models/issue_collection_spec.rb | 58 +++++++++++++++++++++++++++
6 files changed, 127 insertions(+), 8 deletions(-)
create mode 100644 app/models/issue_collection.rb
create mode 100644 spec/models/issue_collection_spec.rb
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 613444e0d704b..93a6b3122e07d 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -286,6 +286,11 @@ def can_move?(*)
false
end
+ def assignee_or_author?(user)
+ # We're comparing IDs here so we don't need to load any associations.
+ author_id == user.id || assignee_id == user.id
+ end
+
def record_metrics
metrics = self.metrics || create_metrics
metrics.record!
diff --git a/app/models/issue_collection.rb b/app/models/issue_collection.rb
new file mode 100644
index 0000000000000..df36252a5b195
--- /dev/null
+++ b/app/models/issue_collection.rb
@@ -0,0 +1,40 @@
+# IssueCollection can be used to reduce a list of issues down to a subset.
+#
+# IssueCollection is not meant to be some sort of Enumerable, instead it's meant
+# to take a list of issues and return a new list of issues based on some
+# criteria. For example, given a list of issues you may want to return a list of
+# issues that can be read or updated by a given user.
+class IssueCollection
+ attr_reader :collection
+
+ def initialize(collection)
+ @collection = collection
+ end
+
+ # Returns all the issues that can be updated by the user.
+ def updatable_by_user(user)
+ return collection if user.admin?
+
+ # Given all the issue projects we get a list of projects that the current
+ # user has at least reporter access to.
+ projects_with_reporter_access = user.
+ projects_with_reporter_access_limited_to(project_ids).
+ pluck(:id)
+
+ collection.select do |issue|
+ if projects_with_reporter_access.include?(issue.project_id)
+ true
+ elsif issue.is_a?(Issue)
+ issue.assignee_or_author?(user)
+ else
+ false
+ end
+ end
+ end
+
+ private
+
+ def project_ids
+ @project_ids ||= collection.map(&:project_id).uniq
+ end
+end
diff --git a/app/policies/issuable_policy.rb b/app/policies/issuable_policy.rb
index c253f9a93995e..9501e499507b9 100644
--- a/app/policies/issuable_policy.rb
+++ b/app/policies/issuable_policy.rb
@@ -4,7 +4,7 @@ def action_name
end
def rules
- if @user && (@subject.author == @user || @subject.assignee == @user)
+ if @user && @subject.assignee_or_author?(@user)
can! :"read_#{action_name}"
can! :"update_#{action_name}"
end
diff --git a/app/policies/issue_policy.rb b/app/policies/issue_policy.rb
index bd1811a3c54d5..f3ede58a00141 100644
--- a/app/policies/issue_policy.rb
+++ b/app/policies/issue_policy.rb
@@ -8,9 +8,8 @@ def rules
if @subject.confidential? && !can_read_confidential?
cannot! :read_issue
- cannot! :admin_issue
cannot! :update_issue
- cannot! :read_issue
+ cannot! :admin_issue
end
end
@@ -18,11 +17,7 @@ def rules
def can_read_confidential?
return false unless @user
- return true if @user.admin?
- return true if @subject.author == @user
- return true if @subject.assignee == @user
- return true if @subject.project.team.member?(@user, Gitlab::Access::REPORTER)
- false
+ IssueCollection.new([@subject]).updatable_by_user(@user).any?
end
end
diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb
index a59d30687f6ec..a9603074c3240 100644
--- a/spec/models/concerns/issuable_spec.rb
+++ b/spec/models/concerns/issuable_spec.rb
@@ -341,4 +341,25 @@
expect(Issue.with_label([bug.title, enhancement.title])).to match_array([issue2])
end
end
+
+ describe '#assignee_or_author?' do
+ let(:user) { build(:user, id: 1) }
+ let(:issue) { build(:issue) }
+
+ it 'returns true for a user that is assigned to an issue' do
+ issue.assignee = user
+
+ expect(issue.assignee_or_author?(user)).to eq(true)
+ end
+
+ it 'returns true for a user that is the author of an issue' do
+ issue.author = user
+
+ expect(issue.assignee_or_author?(user)).to eq(true)
+ end
+
+ it 'returns false for a user that is not the assignee or author' do
+ expect(issue.assignee_or_author?(user)).to eq(false)
+ end
+ end
end
diff --git a/spec/models/issue_collection_spec.rb b/spec/models/issue_collection_spec.rb
new file mode 100644
index 0000000000000..d9ab397c302da
--- /dev/null
+++ b/spec/models/issue_collection_spec.rb
@@ -0,0 +1,58 @@
+require 'spec_helper'
+
+describe IssueCollection do
+ let(:user) { create(:user) }
+ let(:project) { create(:project) }
+ let(:issue1) { create(:issue, project: project) }
+ let(:issue2) { create(:issue, project: project) }
+ let(:collection) { described_class.new([issue1, issue2]) }
+
+ describe '#collection' do
+ it 'returns the issues in the same order as the input Array' do
+ expect(collection.collection).to eq([issue1, issue2])
+ end
+ end
+
+ describe '#updatable_by_user' do
+ context 'using an admin user' do
+ it 'returns all issues' do
+ user = create(:admin)
+
+ expect(collection.updatable_by_user(user)).to eq([issue1, issue2])
+ end
+ end
+
+ context 'using a user that has no access to the project' do
+ it 'returns no issues when the user is not an assignee or author' do
+ expect(collection.updatable_by_user(user)).to be_empty
+ end
+
+ it 'returns the issues the user is assigned to' do
+ issue1.assignee = user
+
+ expect(collection.updatable_by_user(user)).to eq([issue1])
+ end
+
+ it 'returns the issues for which the user is the author' do
+ issue1.author = user
+
+ expect(collection.updatable_by_user(user)).to eq([issue1])
+ end
+ end
+
+ context 'using a user that has reporter access to the project' do
+ it 'returns the issues of the project' do
+ project.team << [user, :reporter]
+
+ expect(collection.updatable_by_user(user)).to eq([issue1, issue2])
+ end
+ end
+
+ context 'using a user that is the owner of a project' do
+ it 'returns the issues of the project' do
+ expect(collection.updatable_by_user(project.namespace.owner)).
+ to eq([issue1, issue2])
+ end
+ end
+ end
+end
--
GitLab
From 509910b89f636f95d2d5a9cd3f38ce8f7f4f47a6 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Fri, 7 Oct 2016 15:20:57 +0200
Subject: [PATCH 29/56] Process commits in a separate worker
This moves the code used for processing commits from GitPushService to
its own Sidekiq worker: ProcessCommitWorker.
Using a Sidekiq worker allows us to process multiple commits in
parallel. This in turn will lead to issues being closed faster and cross
references being created faster. Furthermore by isolating this code into
a separate class it's easier to test and maintain the code.
The new worker also ensures it can efficiently check which issues can be
closed, without having to run numerous SQL queries for every issue.
---
app/models/issue_collection.rb | 2 +
app/policies/issue_policy.rb | 2 +-
app/services/git_push_service.rb | 37 +-----
app/services/issues/close_service.rb | 13 +++
app/workers/process_commit_worker.rb | 67 +++++++++++
.../process-commits-using-sidekiq.yml | 4 +
config/sidekiq_queues.yml | 1 +
spec/models/issue_collection_spec.rb | 9 ++
spec/services/git_push_service_spec.rb | 9 ++
spec/services/issues/close_service_spec.rb | 49 +++++---
spec/workers/process_commit_worker_spec.rb | 109 ++++++++++++++++++
11 files changed, 251 insertions(+), 51 deletions(-)
create mode 100644 app/workers/process_commit_worker.rb
create mode 100644 changelogs/unreleased/process-commits-using-sidekiq.yml
create mode 100644 spec/workers/process_commit_worker_spec.rb
diff --git a/app/models/issue_collection.rb b/app/models/issue_collection.rb
index df36252a5b195..f0b7d9914c801 100644
--- a/app/models/issue_collection.rb
+++ b/app/models/issue_collection.rb
@@ -32,6 +32,8 @@ def updatable_by_user(user)
end
end
+ alias_method :visible_to, :updatable_by_user
+
private
def project_ids
diff --git a/app/policies/issue_policy.rb b/app/policies/issue_policy.rb
index f3ede58a00141..52fa33bc4b032 100644
--- a/app/policies/issue_policy.rb
+++ b/app/policies/issue_policy.rb
@@ -18,6 +18,6 @@ def rules
def can_read_confidential?
return false unless @user
- IssueCollection.new([@subject]).updatable_by_user(@user).any?
+ IssueCollection.new([@subject]).visible_to(@user).any?
end
end
diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb
index e8415862de51b..de313095bedb8 100644
--- a/app/services/git_push_service.rb
+++ b/app/services/git_push_service.rb
@@ -105,35 +105,11 @@ def process_default_branch
# Extract any GFM references from the pushed commit messages. If the configured issue-closing regex is matched,
# close the referenced Issue. Create cross-reference Notes corresponding to any other referenced Mentionables.
def process_commit_messages
- is_default_branch = is_default_branch?
-
- authors = Hash.new do |hash, commit|
- email = commit.author_email
- next hash[email] if hash.has_key?(email)
-
- hash[email] = commit_user(commit)
- end
+ default = is_default_branch?
@push_commits.each do |commit|
- # Keep track of the issues that will be actually closed because they are on a default branch.
- # Hence, when creating cross-reference notes, the not-closed issues (on non-default branches)
- # will also have cross-reference.
- closed_issues = []
-
- if is_default_branch
- # Close issues if these commits were pushed to the project's default branch and the commit message matches the
- # closing regex. Exclude any mentioned Issues from cross-referencing even if the commits are being pushed to
- # a different branch.
- closed_issues = commit.closes_issues(current_user)
- closed_issues.each do |issue|
- if can?(current_user, :update_issue, issue)
- Issues::CloseService.new(project, authors[commit], {}).execute(issue, commit: commit)
- end
- end
- end
-
- commit.create_cross_references!(authors[commit], closed_issues)
- update_issue_metrics(commit, authors)
+ ProcessCommitWorker.
+ perform_async(project.id, current_user.id, commit.id, default)
end
end
@@ -176,11 +152,4 @@ def commit_user(commit)
def branch_name
@branch_name ||= Gitlab::Git.ref_name(params[:ref])
end
-
- def update_issue_metrics(commit, authors)
- mentioned_issues = commit.all_references(authors[commit]).issues
-
- Issue::Metrics.where(issue_id: mentioned_issues.map(&:id), first_mentioned_in_commit_at: nil).
- update_all(first_mentioned_in_commit_at: commit.committed_date)
- end
end
diff --git a/app/services/issues/close_service.rb b/app/services/issues/close_service.rb
index 45cca216ccce6..ab4c51386a429 100644
--- a/app/services/issues/close_service.rb
+++ b/app/services/issues/close_service.rb
@@ -1,8 +1,21 @@
module Issues
class CloseService < Issues::BaseService
+ # Closes the supplied issue if the current user is able to do so.
def execute(issue, commit: nil, notifications: true, system_note: true)
return issue unless can?(current_user, :update_issue, issue)
+ close_issue(issue,
+ commit: commit,
+ notifications: notifications,
+ system_note: system_note)
+ end
+
+ # Closes the supplied issue without checking if the user is authorized to
+ # do so.
+ #
+ # The code calling this method is responsible for ensuring that a user is
+ # allowed to close the given issue.
+ def close_issue(issue, commit: nil, notifications: true, system_note: true)
if project.jira_tracker? && project.jira_service.active
project.jira_service.execute(commit, issue)
todo_service.close_issue(issue, current_user)
diff --git a/app/workers/process_commit_worker.rb b/app/workers/process_commit_worker.rb
new file mode 100644
index 0000000000000..071741fbacd2c
--- /dev/null
+++ b/app/workers/process_commit_worker.rb
@@ -0,0 +1,67 @@
+# Worker for processing individiual commit messages pushed to a repository.
+#
+# Jobs for this worker are scheduled for every commit that is being pushed. As a
+# result of this the workload of this worker should be kept to a bare minimum.
+# Consider using an extra worker if you need to add any extra (and potentially
+# slow) processing of commits.
+class ProcessCommitWorker
+ include Sidekiq::Worker
+ include DedicatedSidekiqQueue
+
+ # project_id - The ID of the project this commit belongs to.
+ # user_id - The ID of the user that pushed the commit.
+ # commit_sha - The SHA1 of the commit to process.
+ # default - The data was pushed to the default branch.
+ def perform(project_id, user_id, commit_sha, default = false)
+ project = Project.find_by(id: project_id)
+
+ return unless project
+
+ user = User.find_by(id: user_id)
+
+ return unless user
+
+ commit = find_commit(project, commit_sha)
+
+ return unless commit
+
+ author = commit.author || user
+
+ process_commit_message(project, commit, user, author, default)
+
+ update_issue_metrics(commit, author)
+ end
+
+ def process_commit_message(project, commit, user, author, default = false)
+ closed_issues = default ? commit.closes_issues(user) : []
+
+ unless closed_issues.empty?
+ close_issues(project, user, author, commit, closed_issues)
+ end
+
+ commit.create_cross_references!(author, closed_issues)
+ end
+
+ def close_issues(project, user, author, commit, issues)
+ # We don't want to run permission related queries for every single issue,
+ # therefor we use IssueCollection here and skip the authorization check in
+ # Issues::CloseService#execute.
+ IssueCollection.new(issues).updatable_by_user(user).each do |issue|
+ Issues::CloseService.new(project, author).
+ close_issue(issue, commit: commit)
+ end
+ end
+
+ def update_issue_metrics(commit, author)
+ mentioned_issues = commit.all_references(author).issues
+
+ Issue::Metrics.where(issue_id: mentioned_issues.map(&:id), first_mentioned_in_commit_at: nil).
+ update_all(first_mentioned_in_commit_at: commit.committed_date)
+ end
+
+ private
+
+ def find_commit(project, sha)
+ project.commit(sha)
+ end
+end
diff --git a/changelogs/unreleased/process-commits-using-sidekiq.yml b/changelogs/unreleased/process-commits-using-sidekiq.yml
new file mode 100644
index 0000000000000..9f596e6a584c3
--- /dev/null
+++ b/changelogs/unreleased/process-commits-using-sidekiq.yml
@@ -0,0 +1,4 @@
+---
+title: Process commits using a dedicated Sidekiq worker
+merge_request: 6802
+author:
diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml
index f36fe893fd0cf..0aec8aedf724f 100644
--- a/config/sidekiq_queues.yml
+++ b/config/sidekiq_queues.yml
@@ -21,6 +21,7 @@
- [post_receive, 5]
- [merge, 5]
- [update_merge_requests, 3]
+ - [process_commit, 2]
- [new_note, 2]
- [build, 2]
- [pipeline, 2]
diff --git a/spec/models/issue_collection_spec.rb b/spec/models/issue_collection_spec.rb
index d9ab397c302da..d742c8146802c 100644
--- a/spec/models/issue_collection_spec.rb
+++ b/spec/models/issue_collection_spec.rb
@@ -55,4 +55,13 @@
end
end
end
+
+ describe '#visible_to' do
+ it 'is an alias for updatable_by_user' do
+ updatable_by_user = described_class.instance_method(:updatable_by_user)
+ visible_to = described_class.instance_method(:visible_to)
+
+ expect(visible_to).to eq(updatable_by_user)
+ end
+ end
end
diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb
index 1c7a3d04b09fd..cea7e6429f953 100644
--- a/spec/services/git_push_service_spec.rb
+++ b/spec/services/git_push_service_spec.rb
@@ -302,6 +302,9 @@
author_email: commit_author.email
)
+ allow_any_instance_of(ProcessCommitWorker).to receive(:find_commit).
+ and_return(commit)
+
allow(project.repository).to receive(:commits_between).and_return([commit])
end
@@ -357,6 +360,9 @@
committed_date: commit_time
)
+ allow_any_instance_of(ProcessCommitWorker).to receive(:find_commit).
+ and_return(commit)
+
allow(project.repository).to receive(:commits_between).and_return([commit])
end
@@ -393,6 +399,9 @@
allow(project.repository).to receive(:commits_between).
and_return([closing_commit])
+ allow_any_instance_of(ProcessCommitWorker).to receive(:find_commit).
+ and_return(closing_commit)
+
project.team << [commit_author, :master]
end
diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb
index 5dfb33f4b28d5..4465f22a001ac 100644
--- a/spec/services/issues/close_service_spec.rb
+++ b/spec/services/issues/close_service_spec.rb
@@ -15,10 +15,39 @@
end
describe '#execute' do
+ let(:service) { described_class.new(project, user) }
+
+ it 'checks if the user is authorized to update the issue' do
+ expect(service).to receive(:can?).with(user, :update_issue, issue).
+ and_call_original
+
+ service.execute(issue)
+ end
+
+ it 'does not close the issue when the user is not authorized to do so' do
+ allow(service).to receive(:can?).with(user, :update_issue, issue).
+ and_return(false)
+
+ expect(service).not_to receive(:close_issue)
+ expect(service.execute(issue)).to eq(issue)
+ end
+
+ it 'closes the issue when the user is authorized to do so' do
+ allow(service).to receive(:can?).with(user, :update_issue, issue).
+ and_return(true)
+
+ expect(service).to receive(:close_issue).
+ with(issue, commit: nil, notifications: true, system_note: true)
+
+ service.execute(issue)
+ end
+ end
+
+ describe '#close_issue' do
context "valid params" do
before do
perform_enqueued_jobs do
- described_class.new(project, user).execute(issue)
+ described_class.new(project, user).close_issue(issue)
end
end
@@ -41,24 +70,12 @@
end
end
- context 'current user is not authorized to close issue' do
- before do
- perform_enqueued_jobs do
- described_class.new(project, guest).execute(issue)
- end
- end
-
- it 'does not close the issue' do
- expect(issue).to be_open
- end
- end
-
context 'when issue is not confidential' do
it 'executes issue hooks' do
expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :issue_hooks)
expect(project).to receive(:execute_services).with(an_instance_of(Hash), :issue_hooks)
- described_class.new(project, user).execute(issue)
+ described_class.new(project, user).close_issue(issue)
end
end
@@ -69,14 +86,14 @@
expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :confidential_issue_hooks)
expect(project).to receive(:execute_services).with(an_instance_of(Hash), :confidential_issue_hooks)
- described_class.new(project, user).execute(issue)
+ described_class.new(project, user).close_issue(issue)
end
end
context 'external issue tracker' do
before do
allow(project).to receive(:default_issues_tracker?).and_return(false)
- described_class.new(project, user).execute(issue)
+ described_class.new(project, user).close_issue(issue)
end
it { expect(issue).to be_valid }
diff --git a/spec/workers/process_commit_worker_spec.rb b/spec/workers/process_commit_worker_spec.rb
new file mode 100644
index 0000000000000..3e4fee422409e
--- /dev/null
+++ b/spec/workers/process_commit_worker_spec.rb
@@ -0,0 +1,109 @@
+require 'spec_helper'
+
+describe ProcessCommitWorker do
+ let(:worker) { described_class.new }
+ let(:user) { create(:user) }
+ let(:project) { create(:project, :public) }
+ let(:issue) { create(:issue, project: project, author: user) }
+ let(:commit) { project.commit }
+
+ describe '#perform' do
+ it 'does not process the commit when the project does not exist' do
+ expect(worker).not_to receive(:close_issues)
+
+ worker.perform(-1, user.id, commit.id)
+ end
+
+ it 'does not process the commit when the user does not exist' do
+ expect(worker).not_to receive(:close_issues)
+
+ worker.perform(project.id, -1, commit.id)
+ end
+
+ it 'does not process the commit when the commit no longer exists' do
+ expect(worker).not_to receive(:close_issues)
+
+ worker.perform(project.id, user.id, 'this-should-does-not-exist')
+ end
+
+ it 'processes the commit message' do
+ expect(worker).to receive(:process_commit_message).and_call_original
+
+ worker.perform(project.id, user.id, commit.id)
+ end
+
+ it 'updates the issue metrics' do
+ expect(worker).to receive(:update_issue_metrics).and_call_original
+
+ worker.perform(project.id, user.id, commit.id)
+ end
+ end
+
+ describe '#process_commit_message' do
+ context 'when pushing to the default branch' do
+ it 'closes issues that should be closed per the commit message' do
+ allow(commit).to receive(:safe_message).
+ and_return("Closes #{issue.to_reference}")
+
+ expect(worker).to receive(:close_issues).
+ with(project, user, user, commit, [issue])
+
+ worker.process_commit_message(project, commit, user, user, true)
+ end
+ end
+
+ context 'when pushing to a non-default branch' do
+ it 'does not close any issues' do
+ allow(commit).to receive(:safe_message).
+ and_return("Closes #{issue.to_reference}")
+
+ expect(worker).not_to receive(:close_issues)
+
+ worker.process_commit_message(project, commit, user, user, false)
+ end
+ end
+
+ it 'creates cross references' do
+ expect(commit).to receive(:create_cross_references!)
+
+ worker.process_commit_message(project, commit, user, user)
+ end
+ end
+
+ describe '#close_issues' do
+ context 'when the user can update the issues' do
+ it 'closes the issues' do
+ worker.close_issues(project, user, user, commit, [issue])
+
+ issue.reload
+
+ expect(issue.closed?).to eq(true)
+ end
+ end
+
+ context 'when the user can not update the issues' do
+ it 'does not close the issues' do
+ other_user = create(:user)
+
+ worker.close_issues(project, other_user, other_user, commit, [issue])
+
+ issue.reload
+
+ expect(issue.closed?).to eq(false)
+ end
+ end
+ end
+
+ describe '#update_issue_metrics' do
+ it 'updates any existing issue metrics' do
+ allow(commit).to receive(:safe_message).
+ and_return("Closes #{issue.to_reference}")
+
+ worker.update_issue_metrics(commit, user)
+
+ metric = Issue::Metrics.first
+
+ expect(metric.first_mentioned_in_commit_at).to eq(commit.committed_date)
+ end
+ end
+end
--
GitLab
From 82e551bdac7f2792e1d6aceb1b0b674dbd3dda81 Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Mon, 7 Nov 2016 15:16:04 +0200
Subject: [PATCH 30/56] Refactor routing constraints
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
lib/constraints/constrainer_helper.rb | 15 ++++++++
lib/constraints/group_url_constrainer.rb | 16 ++++++---
lib/constraints/namespace_url_constrainer.rb | 24 -------------
lib/constraints/user_url_constrainer.rb | 16 ++++++---
.../constraints/constrainer_helper_spec.rb | 20 +++++++++++
.../constraints/group_url_constrainer_spec.rb | 17 ++++++---
.../namespace_url_constrainer_spec.rb | 35 -------------------
.../constraints/user_url_constrainer_spec.rb | 12 +++++--
8 files changed, 81 insertions(+), 74 deletions(-)
create mode 100644 lib/constraints/constrainer_helper.rb
delete mode 100644 lib/constraints/namespace_url_constrainer.rb
create mode 100644 spec/lib/constraints/constrainer_helper_spec.rb
delete mode 100644 spec/lib/constraints/namespace_url_constrainer_spec.rb
diff --git a/lib/constraints/constrainer_helper.rb b/lib/constraints/constrainer_helper.rb
new file mode 100644
index 0000000000000..ab07a6793d90d
--- /dev/null
+++ b/lib/constraints/constrainer_helper.rb
@@ -0,0 +1,15 @@
+module ConstrainerHelper
+ def extract_resource_path(path)
+ id = path.dup
+ id.sub!(/\A#{relative_url_root}/, '') if relative_url_root
+ id.sub(/\A\/+/, '').sub(/\/+\z/, '').sub(/.atom\z/, '')
+ end
+
+ private
+
+ def relative_url_root
+ if defined?(Gitlab::Application.config.relative_url_root)
+ Gitlab::Application.config.relative_url_root
+ end
+ end
+end
diff --git a/lib/constraints/group_url_constrainer.rb b/lib/constraints/group_url_constrainer.rb
index ca39b1961ae0d..2bf973a73dafc 100644
--- a/lib/constraints/group_url_constrainer.rb
+++ b/lib/constraints/group_url_constrainer.rb
@@ -1,7 +1,15 @@
-require 'constraints/namespace_url_constrainer'
+require_relative 'constrainer_helper'
-class GroupUrlConstrainer < NamespaceUrlConstrainer
- def find_resource(id)
- Group.find_by_path(id)
+class GroupUrlConstrainer
+ include ConstrainerHelper
+
+ def matches?(request)
+ id = extract_resource_path(request.path)
+
+ if id =~ Gitlab::Regex.namespace_regex
+ !!Group.find_by_path(id)
+ else
+ false
+ end
end
end
diff --git a/lib/constraints/namespace_url_constrainer.rb b/lib/constraints/namespace_url_constrainer.rb
deleted file mode 100644
index 91b70143f1137..0000000000000
--- a/lib/constraints/namespace_url_constrainer.rb
+++ /dev/null
@@ -1,24 +0,0 @@
-class NamespaceUrlConstrainer
- def matches?(request)
- id = request.path
- id = id.sub(/\A#{relative_url_root}/, '') if relative_url_root
- id = id.sub(/\A\/+/, '').split('/').first
- id = id.sub(/.atom\z/, '') if id
-
- if id =~ Gitlab::Regex.namespace_regex
- find_resource(id)
- end
- end
-
- def find_resource(id)
- Namespace.find_by_path(id)
- end
-
- private
-
- def relative_url_root
- if defined?(Gitlab::Application.config.relative_url_root)
- Gitlab::Application.config.relative_url_root
- end
- end
-end
diff --git a/lib/constraints/user_url_constrainer.rb b/lib/constraints/user_url_constrainer.rb
index 504a0f5d93e0f..9583cace022eb 100644
--- a/lib/constraints/user_url_constrainer.rb
+++ b/lib/constraints/user_url_constrainer.rb
@@ -1,7 +1,15 @@
-require 'constraints/namespace_url_constrainer'
+require_relative 'constrainer_helper'
-class UserUrlConstrainer < NamespaceUrlConstrainer
- def find_resource(id)
- User.find_by('lower(username) = ?', id.downcase)
+class UserUrlConstrainer
+ include ConstrainerHelper
+
+ def matches?(request)
+ id = extract_resource_path(request.path)
+
+ if id =~ Gitlab::Regex.namespace_regex
+ !!User.find_by('lower(username) = ?', id.downcase)
+ else
+ false
+ end
end
end
diff --git a/spec/lib/constraints/constrainer_helper_spec.rb b/spec/lib/constraints/constrainer_helper_spec.rb
new file mode 100644
index 0000000000000..27c8d72aefc03
--- /dev/null
+++ b/spec/lib/constraints/constrainer_helper_spec.rb
@@ -0,0 +1,20 @@
+require 'spec_helper'
+
+describe ConstrainerHelper, lib: true do
+ include ConstrainerHelper
+
+ describe '#extract_resource_path' do
+ it { expect(extract_resource_path('/gitlab/')).to eq('gitlab') }
+ it { expect(extract_resource_path('///gitlab//')).to eq('gitlab') }
+ it { expect(extract_resource_path('/gitlab.atom')).to eq('gitlab') }
+
+ context 'relative url' do
+ before do
+ allow(Gitlab::Application.config).to receive(:relative_url_root) { '/gitlab' }
+ end
+
+ it { expect(extract_resource_path('/gitlab/foo')).to eq('foo') }
+ it { expect(extract_resource_path('/foo/bar')).to eq('foo/bar') }
+ end
+ end
+end
diff --git a/spec/lib/constraints/group_url_constrainer_spec.rb b/spec/lib/constraints/group_url_constrainer_spec.rb
index f0b75a664f27d..be69a36c2b67c 100644
--- a/spec/lib/constraints/group_url_constrainer_spec.rb
+++ b/spec/lib/constraints/group_url_constrainer_spec.rb
@@ -1,10 +1,19 @@
require 'spec_helper'
describe GroupUrlConstrainer, lib: true do
- let!(:username) { create(:group, path: 'gitlab-org') }
+ let!(:group) { create(:group, path: 'gitlab') }
- describe '#find_resource' do
- it { expect(!!subject.find_resource('gitlab-org')).to be_truthy }
- it { expect(!!subject.find_resource('gitlab-com')).to be_falsey }
+ describe '#matches?' do
+ context 'root group' do
+ it { expect(subject.matches?(request '/gitlab')).to be_truthy }
+ it { expect(subject.matches?(request '/gitlab.atom')).to be_truthy }
+ it { expect(subject.matches?(request '/gitlab/edit')).to be_falsey }
+ it { expect(subject.matches?(request '/gitlab-ce')).to be_falsey }
+ it { expect(subject.matches?(request '/.gitlab')).to be_falsey }
+ end
+ end
+
+ def request(path)
+ OpenStruct.new(path: path)
end
end
diff --git a/spec/lib/constraints/namespace_url_constrainer_spec.rb b/spec/lib/constraints/namespace_url_constrainer_spec.rb
deleted file mode 100644
index 7814711fe2786..0000000000000
--- a/spec/lib/constraints/namespace_url_constrainer_spec.rb
+++ /dev/null
@@ -1,35 +0,0 @@
-require 'spec_helper'
-
-describe NamespaceUrlConstrainer, lib: true do
- let!(:group) { create(:group, path: 'gitlab') }
-
- describe '#matches?' do
- context 'existing namespace' do
- it { expect(subject.matches?(request '/gitlab')).to be_truthy }
- it { expect(subject.matches?(request '/gitlab.atom')).to be_truthy }
- it { expect(subject.matches?(request '/gitlab/')).to be_truthy }
- it { expect(subject.matches?(request '//gitlab/')).to be_truthy }
- end
-
- context 'non-existing namespace' do
- it { expect(subject.matches?(request '/gitlab-ce')).to be_falsey }
- it { expect(subject.matches?(request '/gitlab.ce')).to be_falsey }
- it { expect(subject.matches?(request '/g/gitlab')).to be_falsey }
- it { expect(subject.matches?(request '/.gitlab')).to be_falsey }
- end
-
- context 'relative url' do
- before do
- allow(Gitlab::Application.config).to receive(:relative_url_root) { '/gitlab' }
- end
-
- it { expect(subject.matches?(request '/gitlab/gitlab')).to be_truthy }
- it { expect(subject.matches?(request '/gitlab/gitlab-ce')).to be_falsey }
- it { expect(subject.matches?(request '/gitlab/')).to be_falsey }
- end
- end
-
- def request(path)
- OpenStruct.new(path: path)
- end
-end
diff --git a/spec/lib/constraints/user_url_constrainer_spec.rb b/spec/lib/constraints/user_url_constrainer_spec.rb
index 4b26692672f0f..d7b7d5664ff8d 100644
--- a/spec/lib/constraints/user_url_constrainer_spec.rb
+++ b/spec/lib/constraints/user_url_constrainer_spec.rb
@@ -3,8 +3,14 @@
describe UserUrlConstrainer, lib: true do
let!(:username) { create(:user, username: 'dz') }
- describe '#find_resource' do
- it { expect(!!subject.find_resource('dz')).to be_truthy }
- it { expect(!!subject.find_resource('john')).to be_falsey }
+ describe '#matches?' do
+ it { expect(subject.matches?(request '/dz')).to be_truthy }
+ it { expect(subject.matches?(request '/dz.atom')).to be_truthy }
+ it { expect(subject.matches?(request '/dz/projects')).to be_falsey }
+ it { expect(subject.matches?(request '/gitlab')).to be_falsey }
+ end
+
+ def request(path)
+ OpenStruct.new(path: path)
end
end
--
GitLab
From 4ca3db3f9d3bc288fe6b535aca6fb33b00f5e040 Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Mon, 7 Nov 2016 15:34:25 +0200
Subject: [PATCH 31/56] Refactor group routing
* separate controller actions from nested resources
* prepare group routing for nested namespaces support
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
config/routes/group.rb | 38 +++++++++++++++++++-------------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/config/routes/group.rb b/config/routes/group.rb
index 826048ba196cf..faec3f0ad5646 100644
--- a/config/routes/group.rb
+++ b/config/routes/group.rb
@@ -12,26 +12,26 @@
end
end
-scope constraints: { id: /[a-zA-Z.0-9_\-]+(?<!\.atom)/ } do
- resources :groups, except: [:show] do
- member do
- get :issues
- get :merge_requests
- get :projects
- get :activity
- end
+resources :groups, only: [:index, :new, :create]
- scope module: :groups do
- resources :group_members, only: [:index, :create, :update, :destroy], concerns: :access_requestable do
- post :resend_invite, on: :member
- delete :leave, on: :collection
- end
-
- resource :avatar, only: [:destroy]
- resources :milestones, constraints: { id: /[^\/]+/ }, only: [:index, :show, :update, :new, :create]
+scope(path: 'groups/:id', controller: :groups) do
+ get :edit, as: :edit_group
+ get :issues, as: :issues_group
+ get :merge_requests, as: :merge_requests_group
+ get :projects, as: :projects_group
+ get :activity, as: :activity_group
+end
- resources :labels, except: [:show], constraints: { id: /\d+/ }
- end
+scope(path: 'groups/:group_id', module: :groups, as: :group) do
+ resources :group_members, only: [:index, :create, :update, :destroy], concerns: :access_requestable do
+ post :resend_invite, on: :member
+ delete :leave, on: :collection
end
- get 'groups/:id' => 'groups#show', as: :group_canonical
+
+ resource :avatar, only: [:destroy]
+ resources :milestones, constraints: { id: /[^\/]+/ }, only: [:index, :show, :update, :new, :create]
+ resources :labels, except: [:show], constraints: { id: /\d+/ }
end
+
+# Must be last route in this file
+get 'groups/:id' => 'groups#show', as: :group_canonical
--
GitLab
From 63f0b099744834424e6fef78c694beda9d8b16fe Mon Sep 17 00:00:00 2001
From: Rares Sfirlogea <rrr.junior@gmail.com>
Date: Fri, 4 Nov 2016 12:52:38 +0100
Subject: [PATCH 32/56] Expose Label id to API
[e44da1c] Add Label API expected keys to tests
[ac929c8] Update Label API documentation
---
changelogs/unreleased/add-api-label-id.yml | 4 +
doc/api/labels.md | 189 +++++++++++----------
lib/api/entities.rb | 2 +-
spec/requests/api/labels_spec.rb | 6 +
4 files changed, 111 insertions(+), 90 deletions(-)
create mode 100644 changelogs/unreleased/add-api-label-id.yml
diff --git a/changelogs/unreleased/add-api-label-id.yml b/changelogs/unreleased/add-api-label-id.yml
new file mode 100644
index 0000000000000..3af4f5e677d79
--- /dev/null
+++ b/changelogs/unreleased/add-api-label-id.yml
@@ -0,0 +1,4 @@
+---
+title: Expose label IDs in API
+merge_request: 7275
+author: Rares Sfirlogea
diff --git a/doc/api/labels.md b/doc/api/labels.md
index b524203794986..78686fdcad4d1 100644
--- a/doc/api/labels.md
+++ b/doc/api/labels.md
@@ -20,56 +20,61 @@ Example response:
```json
[
- {
- "name" : "bug",
- "color" : "#d9534f",
- "description": "Bug reported by user",
- "open_issues_count": 1,
- "closed_issues_count": 0,
- "open_merge_requests_count": 1,
- "subscribed": false,
- "priority": 10
- },
- {
- "color" : "#d9534f",
- "name" : "confirmed",
- "description": "Confirmed issue",
- "open_issues_count": 2,
- "closed_issues_count": 5,
- "open_merge_requests_count": 0,
- "subscribed": false,
- "priority": null
- },
- {
- "name" : "critical",
- "color" : "#d9534f",
- "description": "Critical issue. Need fix ASAP",
- "open_issues_count": 1,
- "closed_issues_count": 3,
- "open_merge_requests_count": 1,
- "subscribed": false,
- "priority": null
- },
- {
- "name" : "documentation",
- "color" : "#f0ad4e",
- "description": "Issue about documentation",
- "open_issues_count": 1,
- "closed_issues_count": 0,
- "open_merge_requests_count": 2,
- "subscribed": false,
- "priority": null
- },
- {
- "color" : "#5cb85c",
- "name" : "enhancement",
- "description": "Enhancement proposal",
- "open_issues_count": 1,
- "closed_issues_count": 0,
- "open_merge_requests_count": 1,
- "subscribed": false,
- "priority": null
- }
+ {
+ "id" : 1,
+ "name" : "bug",
+ "color" : "#d9534f",
+ "description": "Bug reported by user",
+ "open_issues_count": 1,
+ "closed_issues_count": 0,
+ "open_merge_requests_count": 1,
+ "subscribed": false,
+ "priority": 10
+ },
+ {
+ "id" : 4,
+ "color" : "#d9534f",
+ "name" : "confirmed",
+ "description": "Confirmed issue",
+ "open_issues_count": 2,
+ "closed_issues_count": 5,
+ "open_merge_requests_count": 0,
+ "subscribed": false,
+ "priority": null
+ },
+ {
+ "id" : 7,
+ "name" : "critical",
+ "color" : "#d9534f",
+ "description": "Critical issue. Need fix ASAP",
+ "open_issues_count": 1,
+ "closed_issues_count": 3,
+ "open_merge_requests_count": 1,
+ "subscribed": false,
+ "priority": null
+ },
+ {
+ "id" : 8,
+ "name" : "documentation",
+ "color" : "#f0ad4e",
+ "description": "Issue about documentation",
+ "open_issues_count": 1,
+ "closed_issues_count": 0,
+ "open_merge_requests_count": 2,
+ "subscribed": false,
+ "priority": null
+ },
+ {
+ "id" : 9,
+ "color" : "#5cb85c",
+ "name" : "enhancement",
+ "description": "Enhancement proposal",
+ "open_issues_count": 1,
+ "closed_issues_count": 0,
+ "open_merge_requests_count": 1,
+ "subscribed": true,
+ "priority": null
+ }
]
```
@@ -100,13 +105,15 @@ Example response:
```json
{
- "name" : "feature",
- "color" : "#5843AD",
- "open_issues_count": 1,
- "closed_issues_count": 0,
- "open_merge_requests_count": 1,
- "description": null,
- "priority": null
+ "id" : 10,
+ "name" : "feature",
+ "color" : "#5843AD",
+ "description":null,
+ "open_issues_count": 0,
+ "closed_issues_count": 0,
+ "open_merge_requests_count": 0,
+ "subscribed": false,
+ "priority": null
}
```
@@ -135,15 +142,15 @@ Example response:
```json
{
- "title" : "feature",
- "color" : "#5843AD",
- "description": "New feature proposal",
- "updated_at" : "2015-11-03T21:22:30.737Z",
- "template" : false,
- "project_id" : 1,
- "created_at" : "2015-11-03T21:22:30.737Z",
- "id" : 9,
- "priority": null
+ "id" : 1,
+ "name" : "bug",
+ "color" : "#d9534f",
+ "description": "Bug reported by user",
+ "open_issues_count": 1,
+ "closed_issues_count": 0,
+ "open_merge_requests_count": 1,
+ "subscribed": false,
+ "priority": null
}
```
@@ -178,13 +185,15 @@ Example response:
```json
{
- "color" : "#8E44AD",
- "name" : "docs",
- "description": "Documentation",
- "open_issues_count": 1,
- "closed_issues_count": 0,
- "open_merge_requests_count": 1,
- "priority": null
+ "id" : 8,
+ "name" : "docs",
+ "color" : "#8E44AD",
+ "description": "Documentation",
+ "open_issues_count": 1,
+ "closed_issues_count": 0,
+ "open_merge_requests_count": 2,
+ "subscribed": false,
+ "priority": null
}
```
@@ -213,14 +222,15 @@ Example response:
```json
{
- "name": "Docs",
- "color": "#cc0033",
- "description": "",
- "open_issues_count": 0,
- "closed_issues_count": 0,
- "open_merge_requests_count": 0,
- "subscribed": true,
- "priority": null
+ "id" : 1,
+ "name" : "bug",
+ "color" : "#d9534f",
+ "description": "Bug reported by user",
+ "open_issues_count": 1,
+ "closed_issues_count": 0,
+ "open_merge_requests_count": 1,
+ "subscribed": true,
+ "priority": null
}
```
@@ -249,13 +259,14 @@ Example response:
```json
{
- "name": "Docs",
- "color": "#cc0033",
- "description": "",
- "open_issues_count": 0,
- "closed_issues_count": 0,
- "open_merge_requests_count": 0,
- "subscribed": false,
- "priority": null
+ "id" : 1,
+ "name" : "bug",
+ "color" : "#d9534f",
+ "description": "Bug reported by user",
+ "open_issues_count": 1,
+ "closed_issues_count": 0,
+ "open_merge_requests_count": 1,
+ "subscribed": false,
+ "priority": null
}
```
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 9dd36ec969e93..1942aeea65662 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -433,7 +433,7 @@ class ProjectWithAccess < Project
end
class LabelBasic < Grape::Entity
- expose :name, :color, :description
+ expose :id, :name, :color, :description
end
class Label < LabelBasic
diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb
index 7e532912d0810..2ff90b6deac5c 100644
--- a/spec/requests/api/labels_spec.rb
+++ b/spec/requests/api/labels_spec.rb
@@ -17,12 +17,18 @@
group = create(:group)
group_label = create(:group_label, group: group)
project.update(group: group)
+ expected_keys = [
+ 'id', 'name', 'color', 'description',
+ 'open_issues_count', 'closed_issues_count', 'open_merge_requests_count',
+ 'subscribed', 'priority'
+ ]
get api("/projects/#{project.id}/labels", user)
expect(response).to have_http_status(200)
expect(json_response).to be_an Array
expect(json_response.size).to eq(3)
+ expect(json_response.first.keys).to match_array expected_keys
expect(json_response.map { |l| l['name'] }).to match_array([group_label.name, priority_label.name, label1.name])
expect(json_response.last['name']).to eq(label1.name)
expect(json_response.last['color']).to be_present
--
GitLab
From 314ef630148d55742ac6835c8a903190e88573d9 Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Mon, 7 Nov 2016 16:13:31 +0200
Subject: [PATCH 33/56] Fix project index
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
app/controllers/projects_controller.rb | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 6988527a3be49..8cd50480ec19d 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -2,9 +2,9 @@ class ProjectsController < Projects::ApplicationController
include IssuableCollections
include ExtractsPath
- before_action :authenticate_user!, except: [:show, :activity, :refs]
- before_action :project, except: [:new, :create]
- before_action :repository, except: [:new, :create]
+ before_action :authenticate_user!, except: [:index, :show, :activity, :refs]
+ before_action :project, except: [:index, :new, :create]
+ before_action :repository, except: [:index, :new, :create]
before_action :assign_ref_vars, only: [:show], if: :repo_exists?
before_action :tree, only: [:show], if: [:repo_exists?, :project_view_files?]
--
GitLab
From 591e18364a82fc8a9a906310e37566c05ad4f4ab Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Thu, 3 Nov 2016 13:01:23 +0200
Subject: [PATCH 34/56] Add tests for project#index routing
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
spec/controllers/projects_controller_spec.rb | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb
index 8eefa284ba067..a302d1c57f41e 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -7,6 +7,26 @@
let(:jpg) { fixture_file_upload(Rails.root + 'spec/fixtures/rails_sample.jpg', 'image/jpg') }
let(:txt) { fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain') }
+ describe 'GET index' do
+ context 'as a user' do
+ it 'redirects to root page' do
+ sign_in(user)
+
+ get :index
+
+ expect(response).to redirect_to(root_path)
+ end
+ end
+
+ context 'as a guest' do
+ it 'redirects to Explore page' do
+ get :index
+
+ expect(response).to redirect_to(explore_root_path)
+ end
+ end
+ end
+
describe "GET show" do
context "user not project member" do
before { sign_in(user) }
--
GitLab
From e6e4147880e831cdc6cc9ef31774297222f654c3 Mon Sep 17 00:00:00 2001
From: Lisanne Fellinger <lisanne.88@gmail.com>
Date: Thu, 27 Oct 2016 22:08:20 +0200
Subject: [PATCH 35/56] Rewritten spinach git_blame tests to rspec feature
tests
Fixing rubocop violations
Relocated git_blame spec and fixed styling issue
Rewritten spinach git_blame tests to rspec feature tests
Fixing rubocop violations
Relocated git_blame spec and fixed styling issue
Rewritten spinach git_blame tests to rspec feature tests
Fixing rubocop violations
Rewritten spinach git_blame tests to rspec feature tests
Fixing rubocop violations
Rewritten spinach git_blame tests to rspec feature tests
Fixing rubocop violations
Relocated git_blame spec and fixed styling issue
---
...spinach-tests-with-rspec-feature-tests.yml | 4 ++++
features/project/source/git_blame.feature | 10 ---------
features/steps/project/source/git_blame.rb | 19 -----------------
.../projects/files/browse_files_spec.rb | 21 +++++++++++++++++++
4 files changed, 25 insertions(+), 29 deletions(-)
create mode 100644 changelogs/unreleased/23036-replace-git-blame-spinach-tests-with-rspec-feature-tests.yml
delete mode 100644 features/project/source/git_blame.feature
delete mode 100644 features/steps/project/source/git_blame.rb
create mode 100644 spec/features/projects/files/browse_files_spec.rb
diff --git a/changelogs/unreleased/23036-replace-git-blame-spinach-tests-with-rspec-feature-tests.yml b/changelogs/unreleased/23036-replace-git-blame-spinach-tests-with-rspec-feature-tests.yml
new file mode 100644
index 0000000000000..7b54d3df56d2b
--- /dev/null
+++ b/changelogs/unreleased/23036-replace-git-blame-spinach-tests-with-rspec-feature-tests.yml
@@ -0,0 +1,4 @@
+---
+title: Rewrite git blame spinach feature tests to rspec feature tests
+merge_request: 7197
+author: Lisanne Fellinger
diff --git a/features/project/source/git_blame.feature b/features/project/source/git_blame.feature
deleted file mode 100644
index 48b1077dc6b8f..0000000000000
--- a/features/project/source/git_blame.feature
+++ /dev/null
@@ -1,10 +0,0 @@
-Feature: Project Source Git Blame
- Background:
- Given I sign in as a user
- And I own project "Shop"
- Given I visit project source page
-
- Scenario: I blame file
- Given I click on ".gitignore" file in repo
- And I click Blame button
- Then I should see git file blame
diff --git a/features/steps/project/source/git_blame.rb b/features/steps/project/source/git_blame.rb
deleted file mode 100644
index d0a27f47e2a47..0000000000000
--- a/features/steps/project/source/git_blame.rb
+++ /dev/null
@@ -1,19 +0,0 @@
-class Spinach::Features::ProjectSourceGitBlame < Spinach::FeatureSteps
- include SharedAuthentication
- include SharedProject
- include SharedPaths
-
- step 'I click on ".gitignore" file in repo' do
- click_link ".gitignore"
- end
-
- step 'I click Blame button' do
- click_link 'Blame'
- end
-
- step 'I should see git file blame' do
- expect(page).to have_content "*.rb"
- expect(page).to have_content "Dmitriy Zaporozhets"
- expect(page).to have_content "Initial commit"
- end
-end
diff --git a/spec/features/projects/files/browse_files_spec.rb b/spec/features/projects/files/browse_files_spec.rb
new file mode 100644
index 0000000000000..69295e450d016
--- /dev/null
+++ b/spec/features/projects/files/browse_files_spec.rb
@@ -0,0 +1,21 @@
+require 'spec_helper'
+
+feature 'user checks git blame', feature: true do
+ let(:project) { create(:project) }
+ let(:user) { create(:user) }
+
+ before do
+ project.team << [user, :master]
+ login_with(user)
+ visit namespace_project_tree_path(project.namespace, project, project.default_branch)
+ end
+
+ scenario "can see blame of '.gitignore'" do
+ click_link ".gitignore"
+ click_link 'Blame'
+
+ expect(page).to have_content "*.rb"
+ expect(page).to have_content "Dmitriy Zaporozhets"
+ expect(page).to have_content "Initial commit"
+ end
+end
--
GitLab
From 8357ae980a396cb0aa57ecd182a36c43821b548e Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Mon, 7 Nov 2016 16:20:06 +0200
Subject: [PATCH 36/56] Fix 404 when visit /projects page
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
CHANGELOG.md | 1 +
1 file changed, 1 insertion(+)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7ed09fb1db8a6..5b072ce9f6071 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -71,6 +71,7 @@ entry.
- Fix applying GitHub-imported labels when importing job is interrupted
- Allow to search for user by secondary email address in the admin interface(/admin/users) !7115 (YarNayar)
- Updated commit SHA styling on the branches page.
+- Fix 404 when visit /projects page
## 8.13.3 (2016-11-02)
--
GitLab
From 41990128a33c268f3af2db80f4ac224802ac8b76 Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Mon, 7 Nov 2016 17:14:34 +0200
Subject: [PATCH 37/56] Refactor project routing
* split on multiple files
* improve routing order
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
config/initializers/routing_draw.rb | 7 ++
config/routes.rb | 6 -
config/routes/git_http.rb | 37 ++++++
config/routes/project.rb | 183 +---------------------------
config/routes/repository.rb | 110 +++++++++++++++++
config/routes/wiki.rb | 16 +++
6 files changed, 176 insertions(+), 183 deletions(-)
create mode 100644 config/initializers/routing_draw.rb
create mode 100644 config/routes/git_http.rb
create mode 100644 config/routes/repository.rb
create mode 100644 config/routes/wiki.rb
diff --git a/config/initializers/routing_draw.rb b/config/initializers/routing_draw.rb
new file mode 100644
index 0000000000000..25003cf023904
--- /dev/null
+++ b/config/initializers/routing_draw.rb
@@ -0,0 +1,7 @@
+# Adds draw method into Rails routing
+# It allows us to keep routing splitted into files
+class ActionDispatch::Routing::Mapper
+ def draw(routes_name)
+ instance_eval(File.read(Rails.root.join("config/routes/#{routes_name}.rb")))
+ end
+end
diff --git a/config/routes.rb b/config/routes.rb
index 659ea51bc75b6..7bf6c03e69b4e 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -2,12 +2,6 @@
require 'sidekiq/cron/web'
require 'api/api'
-class ActionDispatch::Routing::Mapper
- def draw(routes_name)
- instance_eval(File.read(Rails.root.join("config/routes/#{routes_name}.rb")))
- end
-end
-
Rails.application.routes.draw do
concern :access_requestable do
post :request_access, on: :collection
diff --git a/config/routes/git_http.rb b/config/routes/git_http.rb
new file mode 100644
index 0000000000000..03adc4815f38b
--- /dev/null
+++ b/config/routes/git_http.rb
@@ -0,0 +1,37 @@
+scope constraints: { id: /.+\.git/, format: nil } do
+ # Git HTTP clients ('git clone' etc.)
+ get '/info/refs', to: 'git_http#info_refs'
+ post '/git-upload-pack', to: 'git_http#git_upload_pack'
+ post '/git-receive-pack', to: 'git_http#git_receive_pack'
+
+ # Git LFS API (metadata)
+ post '/info/lfs/objects/batch', to: 'lfs_api#batch'
+ post '/info/lfs/objects', to: 'lfs_api#deprecated'
+ get '/info/lfs/objects/*oid', to: 'lfs_api#deprecated'
+
+ # GitLab LFS object storage
+ scope constraints: { oid: /[a-f0-9]{64}/ } do
+ get '/gitlab-lfs/objects/*oid', to: 'lfs_storage#download'
+
+ scope constraints: { size: /[0-9]+/ } do
+ put '/gitlab-lfs/objects/*oid/*size/authorize', to: 'lfs_storage#upload_authorize'
+ put '/gitlab-lfs/objects/*oid/*size', to: 'lfs_storage#upload_finalize'
+ end
+ end
+end
+
+# Allow /info/refs, /info/refs?service=git-upload-pack, and
+# /info/refs?service=git-receive-pack, but nothing else.
+#
+git_http_handshake = lambda do |request|
+ request.query_string.blank? ||
+ request.query_string.match(/\Aservice=git-(upload|receive)-pack\z/)
+end
+
+ref_redirect = redirect do |params, request|
+ path = "#{params[:namespace_id]}/#{params[:project_id]}.git/info/refs"
+ path << "?#{request.query_string}" unless request.query_string.blank?
+ path
+end
+
+get '/info/refs', constraints: git_http_handshake, to: ref_redirect
diff --git a/config/routes/project.rb b/config/routes/project.rb
index 8142e231621e7..2f4a56ae5c912 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -21,149 +21,13 @@
end
scope module: :projects do
- scope constraints: { id: /.+\.git/, format: nil } do
- # Git HTTP clients ('git clone' etc.)
- get '/info/refs', to: 'git_http#info_refs'
- post '/git-upload-pack', to: 'git_http#git_upload_pack'
- post '/git-receive-pack', to: 'git_http#git_receive_pack'
-
- # Git LFS API (metadata)
- post '/info/lfs/objects/batch', to: 'lfs_api#batch'
- post '/info/lfs/objects', to: 'lfs_api#deprecated'
- get '/info/lfs/objects/*oid', to: 'lfs_api#deprecated'
-
- # GitLab LFS object storage
- scope constraints: { oid: /[a-f0-9]{64}/ } do
- get '/gitlab-lfs/objects/*oid', to: 'lfs_storage#download'
-
- scope constraints: { size: /[0-9]+/ } do
- put '/gitlab-lfs/objects/*oid/*size/authorize', to: 'lfs_storage#upload_authorize'
- put '/gitlab-lfs/objects/*oid/*size', to: 'lfs_storage#upload_finalize'
- end
- end
- end
-
- # Allow /info/refs, /info/refs?service=git-upload-pack, and
- # /info/refs?service=git-receive-pack, but nothing else.
- #
- git_http_handshake = lambda do |request|
- request.query_string.blank? ||
- request.query_string.match(/\Aservice=git-(upload|receive)-pack\z/)
- end
-
- ref_redirect = redirect do |params, request|
- path = "#{params[:namespace_id]}/#{params[:project_id]}.git/info/refs"
- path << "?#{request.query_string}" unless request.query_string.blank?
- path
- end
-
- get '/info/refs', constraints: git_http_handshake, to: ref_redirect
-
- # Blob routes:
- get '/new/*id', to: 'blob#new', constraints: { id: /.+/ }, as: 'new_blob'
- post '/create/*id', to: 'blob#create', constraints: { id: /.+/ }, as: 'create_blob'
- get '/edit/*id', to: 'blob#edit', constraints: { id: /.+/ }, as: 'edit_blob'
- put '/update/*id', to: 'blob#update', constraints: { id: /.+/ }, as: 'update_blob'
- post '/preview/*id', to: 'blob#preview', constraints: { id: /.+/ }, as: 'preview_blob'
+ draw :git_http
#
# Templates
#
get '/templates/:template_type/:key' => 'templates#show', as: :template
- scope do
- get(
- '/blob/*id/diff',
- to: 'blob#diff',
- constraints: { id: /.+/, format: false },
- as: :blob_diff
- )
- get(
- '/blob/*id',
- to: 'blob#show',
- constraints: { id: /.+/, format: false },
- as: :blob
- )
- delete(
- '/blob/*id',
- to: 'blob#destroy',
- constraints: { id: /.+/, format: false }
- )
- put(
- '/blob/*id',
- to: 'blob#update',
- constraints: { id: /.+/, format: false }
- )
- post(
- '/blob/*id',
- to: 'blob#create',
- constraints: { id: /.+/, format: false }
- )
- end
-
- scope do
- get(
- '/raw/*id',
- to: 'raw#show',
- constraints: { id: /.+/, format: /(html|js)/ },
- as: :raw
- )
- end
-
- scope do
- get(
- '/tree/*id',
- to: 'tree#show',
- constraints: { id: /.+/, format: /(html|js)/ },
- as: :tree
- )
- end
-
- scope do
- get(
- '/find_file/*id',
- to: 'find_file#show',
- constraints: { id: /.+/, format: /html/ },
- as: :find_file
- )
- end
-
- scope do
- get(
- '/files/*id',
- to: 'find_file#list',
- constraints: { id: /(?:[^.]|\.(?!json$))+/, format: /json/ },
- as: :files
- )
- end
-
- scope do
- post(
- '/create_dir/*id',
- to: 'tree#create_dir',
- constraints: { id: /.+/ },
- as: 'create_dir'
- )
- end
-
- scope do
- get(
- '/blame/*id',
- to: 'blame#show',
- constraints: { id: /.+/, format: /(html|js)/ },
- as: :blame
- )
- end
-
- scope do
- get(
- '/commits/*id',
- to: 'commits#show',
- constraints: { id: /.+/, format: false },
- as: :commits
- )
- end
-
resource :avatar, only: [:show, :destroy]
resources :commit, only: [:show], constraints: { id: /\h{7,40}/ } do
member do
@@ -206,29 +70,6 @@
end
end
- WIKI_SLUG_ID = { id: /\S+/ } unless defined? WIKI_SLUG_ID
-
- scope do
- # Order matters to give priority to these matches
- get '/wikis/git_access', to: 'wikis#git_access'
- get '/wikis/pages', to: 'wikis#pages', as: 'wiki_pages'
- post '/wikis', to: 'wikis#create'
-
- get '/wikis/*id/history', to: 'wikis#history', as: 'wiki_history', constraints: WIKI_SLUG_ID
- get '/wikis/*id/edit', to: 'wikis#edit', as: 'wiki_edit', constraints: WIKI_SLUG_ID
-
- get '/wikis/*id', to: 'wikis#show', as: 'wiki', constraints: WIKI_SLUG_ID
- delete '/wikis/*id', to: 'wikis#destroy', constraints: WIKI_SLUG_ID
- put '/wikis/*id', to: 'wikis#update', constraints: WIKI_SLUG_ID
- post '/wikis/*id/preview_markdown', to: 'wikis#preview_markdown', constraints: WIKI_SLUG_ID, as: 'wiki_preview_markdown'
- end
-
- resource :repository, only: [:create] do
- member do
- get 'archive', constraints: { format: Gitlab::Regex.archive_formats_regex }
- end
- end
-
resources :services, constraints: { id: /[^\/]+/ }, only: [:index, :edit, :update] do
member do
get :test
@@ -245,23 +86,6 @@
resources :forks, only: [:index, :new, :create]
resource :import, only: [:new, :create, :show]
- resources :refs, only: [] do
- collection do
- get 'switch'
- end
-
- member do
- # tree viewer logs
- get 'logs_tree', constraints: { id: Gitlab::Regex.git_reference_regex }
- # Directories with leading dots erroneously get rejected if git
- # ref regex used in constraints. Regex verification now done in controller.
- get 'logs_tree/*path' => 'refs#logs_tree', as: :logs_file, constraints: {
- id: /.*/,
- path: /.*/
- }
- end
- end
-
resources :merge_requests, concerns: :awardable, constraints: { id: /\d+/ } do
member do
get :commits
@@ -467,6 +291,11 @@
end
end
end
+
+ # Since both wiki and repository routing contains wildcard characters
+ # its preferable to keep it below all other project routes
+ draw :wiki
+ draw :repository
end
end
end
diff --git a/config/routes/repository.rb b/config/routes/repository.rb
new file mode 100644
index 0000000000000..76dcf113aea6d
--- /dev/null
+++ b/config/routes/repository.rb
@@ -0,0 +1,110 @@
+# All routing related to repositoty browsing
+
+resource :repository, only: [:create] do
+ member do
+ get 'archive', constraints: { format: Gitlab::Regex.archive_formats_regex }
+ end
+end
+
+resources :refs, only: [] do
+ collection do
+ get 'switch'
+ end
+
+ member do
+ # tree viewer logs
+ get 'logs_tree', constraints: { id: Gitlab::Regex.git_reference_regex }
+ # Directories with leading dots erroneously get rejected if git
+ # ref regex used in constraints. Regex verification now done in controller.
+ get 'logs_tree/*path' => 'refs#logs_tree', as: :logs_file, constraints: {
+ id: /.*/,
+ path: /.*/
+ }
+ end
+end
+
+get '/new/*id', to: 'blob#new', constraints: { id: /.+/ }, as: 'new_blob'
+post '/create/*id', to: 'blob#create', constraints: { id: /.+/ }, as: 'create_blob'
+get '/edit/*id', to: 'blob#edit', constraints: { id: /.+/ }, as: 'edit_blob'
+put '/update/*id', to: 'blob#update', constraints: { id: /.+/ }, as: 'update_blob'
+post '/preview/*id', to: 'blob#preview', constraints: { id: /.+/ }, as: 'preview_blob'
+
+scope do
+ get(
+ '/blob/*id/diff',
+ to: 'blob#diff',
+ constraints: { id: /.+/, format: false },
+ as: :blob_diff
+ )
+ get(
+ '/blob/*id',
+ to: 'blob#show',
+ constraints: { id: /.+/, format: false },
+ as: :blob
+ )
+ delete(
+ '/blob/*id',
+ to: 'blob#destroy',
+ constraints: { id: /.+/, format: false }
+ )
+ put(
+ '/blob/*id',
+ to: 'blob#update',
+ constraints: { id: /.+/, format: false }
+ )
+ post(
+ '/blob/*id',
+ to: 'blob#create',
+ constraints: { id: /.+/, format: false }
+ )
+
+ get(
+ '/raw/*id',
+ to: 'raw#show',
+ constraints: { id: /.+/, format: /(html|js)/ },
+ as: :raw
+ )
+
+ get(
+ '/tree/*id',
+ to: 'tree#show',
+ constraints: { id: /.+/, format: /(html|js)/ },
+ as: :tree
+ )
+
+ get(
+ '/find_file/*id',
+ to: 'find_file#show',
+ constraints: { id: /.+/, format: /html/ },
+ as: :find_file
+ )
+
+ get(
+ '/files/*id',
+ to: 'find_file#list',
+ constraints: { id: /(?:[^.]|\.(?!json$))+/, format: /json/ },
+ as: :files
+ )
+
+ post(
+ '/create_dir/*id',
+ to: 'tree#create_dir',
+ constraints: { id: /.+/ },
+ as: 'create_dir'
+ )
+
+ get(
+ '/blame/*id',
+ to: 'blame#show',
+ constraints: { id: /.+/, format: /(html|js)/ },
+ as: :blame
+ )
+
+ # File/dir history
+ get(
+ '/commits/*id',
+ to: 'commits#show',
+ constraints: { id: /.+/, format: false },
+ as: :commits
+ )
+end
diff --git a/config/routes/wiki.rb b/config/routes/wiki.rb
new file mode 100644
index 0000000000000..ecd4d395d6682
--- /dev/null
+++ b/config/routes/wiki.rb
@@ -0,0 +1,16 @@
+WIKI_SLUG_ID = { id: /\S+/ } unless defined? WIKI_SLUG_ID
+
+scope do
+ # Order matters to give priority to these matches
+ get '/wikis/git_access', to: 'wikis#git_access'
+ get '/wikis/pages', to: 'wikis#pages', as: 'wiki_pages'
+ post '/wikis', to: 'wikis#create'
+
+ get '/wikis/*id/history', to: 'wikis#history', as: 'wiki_history', constraints: WIKI_SLUG_ID
+ get '/wikis/*id/edit', to: 'wikis#edit', as: 'wiki_edit', constraints: WIKI_SLUG_ID
+
+ get '/wikis/*id', to: 'wikis#show', as: 'wiki', constraints: WIKI_SLUG_ID
+ delete '/wikis/*id', to: 'wikis#destroy', constraints: WIKI_SLUG_ID
+ put '/wikis/*id', to: 'wikis#update', constraints: WIKI_SLUG_ID
+ post '/wikis/*id/preview_markdown', to: 'wikis#preview_markdown', constraints: WIKI_SLUG_ID, as: 'wiki_preview_markdown'
+end
--
GitLab
From 16b0723a7ce709437084c12647e59b3a73479ddf Mon Sep 17 00:00:00 2001
From: Chris Wright <chris@dxw.com>
Date: Tue, 30 Aug 2016 21:44:30 +0100
Subject: [PATCH 38/56] Use the Gitlab Workhorse HTTP header in the admin
dashboard
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Rémy Coutable <remy@rymai.me>
---
app/helpers/components_helper.rb | 9 ++++++++
app/views/admin/dashboard/index.html.haml | 2 +-
...ect-workhorse-version-number-displayed.yml | 4 ++++
spec/helpers/components_helper_spec.rb | 21 +++++++++++++++++++
4 files changed, 35 insertions(+), 1 deletion(-)
create mode 100644 app/helpers/components_helper.rb
create mode 100644 changelogs/unreleased/21664-incorrect-workhorse-version-number-displayed.yml
create mode 100644 spec/helpers/components_helper_spec.rb
diff --git a/app/helpers/components_helper.rb b/app/helpers/components_helper.rb
new file mode 100644
index 0000000000000..8893209b3146f
--- /dev/null
+++ b/app/helpers/components_helper.rb
@@ -0,0 +1,9 @@
+module ComponentsHelper
+ def gitlab_workhorse_version
+ if request.headers['Gitlab-Workhorse'].present?
+ request.headers['Gitlab-Workhorse'].split('-').first
+ else
+ Gitlab::Workhorse.version
+ end
+ end
+end
diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml
index 90798c47d9745..1db2150f3366f 100644
--- a/app/views/admin/dashboard/index.html.haml
+++ b/app/views/admin/dashboard/index.html.haml
@@ -87,7 +87,7 @@
%p
GitLab Workhorse
%span.pull-right
- = Gitlab::Workhorse.version
+ = gitlab_workhorse_version
%p
GitLab API
%span.pull-right
diff --git a/changelogs/unreleased/21664-incorrect-workhorse-version-number-displayed.yml b/changelogs/unreleased/21664-incorrect-workhorse-version-number-displayed.yml
new file mode 100644
index 0000000000000..95d8fef109904
--- /dev/null
+++ b/changelogs/unreleased/21664-incorrect-workhorse-version-number-displayed.yml
@@ -0,0 +1,4 @@
+---
+title: Use the Gitlab Workhorse HTTP header in the admin dashboard
+merge_request:
+author: Chris Wright
diff --git a/spec/helpers/components_helper_spec.rb b/spec/helpers/components_helper_spec.rb
new file mode 100644
index 0000000000000..94a59193be846
--- /dev/null
+++ b/spec/helpers/components_helper_spec.rb
@@ -0,0 +1,21 @@
+require 'spec_helper'
+
+describe ComponentsHelper do
+ describe '#gitlab_workhorse_version' do
+ context 'without a Gitlab-Workhorse header' do
+ it 'shows the version from Gitlab::Workhorse.version' do
+ expect(helper.gitlab_workhorse_version).to eq Gitlab::Workhorse.version
+ end
+ end
+
+ context 'with a Gitlab-Workhorse header' do
+ before do
+ helper.request.headers['Gitlab-Workhorse'] = '42.42.0-rc3'
+ end
+
+ it 'shows the actual GitLab Workhorse version currently in use' do
+ expect(helper.gitlab_workhorse_version).to eq '42.42.0'
+ end
+ end
+ end
+end
--
GitLab
From 9d51421346178c9189ffb47189f51d573ab42822 Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@selenight.nl>
Date: Fri, 19 Aug 2016 18:33:46 -0500
Subject: [PATCH 39/56] Use separate email-friendly token for incoming email
and let incoming email token be reset
---
app/controllers/profiles_controller.rb | 10 +++++-
app/models/concerns/token_authenticatable.rb | 10 ++++--
app/models/project.rb | 11 +++---
app/models/user.rb | 12 ++++++-
app/views/profiles/accounts/show.html.haml | 35 +++++++++++--------
config/routes/profile.rb | 1 +
...32256_add_incoming_email_token_to_users.rb | 16 +++++++++
db/schema.rb | 4 ++-
.../email/handler/create_issue_handler.rb | 8 ++---
spec/features/projects/issues/issues_spec.rb | 0
...ken.eml => wrong_incoming_email_token.eml} | 0
.../handler/create_issue_handler_spec.rb | 6 ++--
12 files changed, 79 insertions(+), 34 deletions(-)
create mode 100644 db/migrate/20160819232256_add_incoming_email_token_to_users.rb
create mode 100644 spec/features/projects/issues/issues_spec.rb
rename spec/fixtures/emails/{wrong_authentication_token.eml => wrong_incoming_email_token.eml} (100%)
diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb
index f71e0a1302bd9..e4865642cd320 100644
--- a/app/controllers/profiles_controller.rb
+++ b/app/controllers/profiles_controller.rb
@@ -26,7 +26,15 @@ def update
def reset_private_token
if current_user.reset_authentication_token!
- flash[:notice] = "Token was successfully updated"
+ flash[:notice] = "Private token was successfully updated"
+ end
+
+ redirect_to profile_account_path
+ end
+
+ def reset_incoming_email_token
+ if current_user.reset_incoming_email_token!
+ flash[:notice] = "Incoming email token was successfully updated"
end
redirect_to profile_account_path
diff --git a/app/models/concerns/token_authenticatable.rb b/app/models/concerns/token_authenticatable.rb
index 24c7b26d223d2..04d30f462101b 100644
--- a/app/models/concerns/token_authenticatable.rb
+++ b/app/models/concerns/token_authenticatable.rb
@@ -4,17 +4,21 @@ module TokenAuthenticatable
private
def write_new_token(token_field)
- new_token = generate_token(token_field)
+ new_token = generate_available_token(token_field)
write_attribute(token_field, new_token)
end
- def generate_token(token_field)
+ def generate_available_token(token_field)
loop do
- token = Devise.friendly_token
+ token = generate_token(token_field)
break token unless self.class.unscoped.find_by(token_field => token)
end
end
+ def generate_token(token_field)
+ Devise.friendly_token
+ end
+
class_methods do
def authentication_token_fields
@token_fields || []
diff --git a/app/models/project.rb b/app/models/project.rb
index 686d285410be7..56b84b0aebbb0 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -624,13 +624,12 @@ def web_url_without_protocol
end
def new_issue_address(author)
- # This feature is disabled for the time being.
- return nil
+ return unless Gitlab::IncomingEmail.enabled? && author
- if Gitlab::IncomingEmail.enabled? && author # rubocop:disable Lint/UnreachableCode
- Gitlab::IncomingEmail.reply_address(
- "#{path_with_namespace}+#{author.authentication_token}")
- end
+ author.ensure_incoming_email_token!
+
+ Gitlab::IncomingEmail.reply_address(
+ "#{path_with_namespace}+#{author.incoming_email_token}")
end
def build_commit_note(commit)
diff --git a/app/models/user.rb b/app/models/user.rb
index 65e96ee6b2e4e..9a3619b0bc304 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -13,6 +13,7 @@ class User < ActiveRecord::Base
DEFAULT_NOTIFICATION_LEVEL = :participating
add_authentication_token_field :authentication_token
+ add_authentication_token_field :incoming_email_token
default_value_for :admin, false
default_value_for(:external) { current_application_settings.user_default_external }
@@ -119,7 +120,7 @@ class User < ActiveRecord::Base
before_validation :set_public_email, if: ->(user) { user.public_email_changed? }
after_update :update_emails_with_primary_email, if: ->(user) { user.email_changed? }
- before_save :ensure_authentication_token
+ before_save :ensure_authentication_token, :ensure_incoming_email_token
before_save :ensure_external_user_rights
after_save :ensure_namespace_correct
after_initialize :set_projects_limit
@@ -946,4 +947,13 @@ def domain_matches?(email_domains, email)
signup_domain =~ regexp
end
end
+
+ def generate_token(token_field)
+ if token_field == :incoming_email_token
+ # Needs to be all lowercase and alphanumeric because it's gonna be used in an email address.
+ SecureRandom.hex
+ else
+ super
+ end
+ end
end
diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml
index e2e974ba07219..2c256b1b2332f 100644
--- a/app/views/profiles/accounts/show.html.haml
+++ b/app/views/profiles/accounts/show.html.haml
@@ -8,24 +8,29 @@
.row.prepend-top-default
.col-lg-3.profile-settings-sidebar
%h4.prepend-top-0
- Private Token
+ Private Tokens
%p
- Your private token is used to access application resources without authentication.
+ Your private token is used to access the API and Atom feeds without
+ username/password authentication.
+ %p
+ Your incoming email token is used to create new issues by email, and is
+ included in your project-specific email addresses.
.col-lg-9
- = form_for @user, url: reset_private_token_profile_path, method: :put, html: { class: "private-token" } do |f|
- %p.cgray
- - if current_user.private_token
- = label_tag "token", "Private token", class: "label-light"
- = text_field_tag "token", current_user.private_token, class: "form-control"
- - else
- %span You don`t have one yet. Click generate to fix it.
+ %p.cgray
+ - if current_user.private_token
+ = label_tag "token", "Private token", class: "label-light"
+ = text_field_tag "token", current_user.private_token, class: "form-control"
+ - else
+ %span You don`t have one yet. Click generate to fix it.
%p.help-block
- It can be used for atom feeds or the API. Keep it secret!
- .prepend-top-default
- - if current_user.private_token
- = f.submit 'Reset private token', data: { confirm: "Are you sure?" }, class: "btn btn-default"
- - else
- = f.submit 'Generate', class: "btn btn-default"
+ Keep this token secret, anyone with access to it can interact with the GitLab API as if they were you.
+ .prepend-top-default
+ - if current_user.private_token
+ = link_to 'Reset private token', reset_private_token_profile_path, method: :put, data: { confirm: "Are you sure?" }, class: "btn btn-default"
+ - else
+ = f.submit 'Generate', class: "btn btn-default"
+ = link_to 'Reset incoming email token', reset_incoming_email_token_profile_path, method: :put, data: { confirm: "Are you sure?" }, class: "btn btn-default"
+
%hr
.row.prepend-top-default
.col-lg-3.profile-settings-sidebar
diff --git a/config/routes/profile.rb b/config/routes/profile.rb
index 4cb68c9b34a7c..52b9a565db865 100644
--- a/config/routes/profile.rb
+++ b/config/routes/profile.rb
@@ -4,6 +4,7 @@
get :applications, to: 'oauth/applications#index'
put :reset_private_token
+ put :reset_incoming_email_token
put :update_username
end
diff --git a/db/migrate/20160819232256_add_incoming_email_token_to_users.rb b/db/migrate/20160819232256_add_incoming_email_token_to_users.rb
new file mode 100644
index 0000000000000..f2cf956adc96f
--- /dev/null
+++ b/db/migrate/20160819232256_add_incoming_email_token_to_users.rb
@@ -0,0 +1,16 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddIncomingEmailTokenToUsers < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ # Set this constant to true if this migration requires downtime.
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ def change
+ add_column :users, :incoming_email_token, :string
+ add_concurrent_index :users, :incoming_email_token
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 5476b0c93e5c7..fd82fb326a65d 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20161103171205) do
+ActiveRecord::Schema.define(version: 20160819232256) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -1176,6 +1176,7 @@
t.boolean "ldap_email", default: false, null: false
t.boolean "external", default: false
t.string "organization"
+ t.string "incoming_email_token"
end
add_index "users", ["admin"], name: "index_users_on_admin", using: :btree
@@ -1185,6 +1186,7 @@
add_index "users", ["current_sign_in_at"], name: "index_users_on_current_sign_in_at", using: :btree
add_index "users", ["email"], name: "index_users_on_email", unique: true, using: :btree
add_index "users", ["email"], name: "index_users_on_email_trigram", using: :gin, opclasses: {"email"=>"gin_trgm_ops"}
+ add_index "users", ["incoming_email_token"], name: "index_users_on_incoming_email_token", using: :btree
add_index "users", ["name"], name: "index_users_on_name", using: :btree
add_index "users", ["name"], name: "index_users_on_name_trigram", using: :gin, opclasses: {"name"=>"gin_trgm_ops"}
add_index "users", ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true, using: :btree
diff --git a/lib/gitlab/email/handler/create_issue_handler.rb b/lib/gitlab/email/handler/create_issue_handler.rb
index 4e6566af8abed..9f90a3ec2b2f7 100644
--- a/lib/gitlab/email/handler/create_issue_handler.rb
+++ b/lib/gitlab/email/handler/create_issue_handler.rb
@@ -5,16 +5,16 @@ module Gitlab
module Email
module Handler
class CreateIssueHandler < BaseHandler
- attr_reader :project_path, :authentication_token
+ attr_reader :project_path, :incoming_email_token
def initialize(mail, mail_key)
super(mail, mail_key)
- @project_path, @authentication_token =
+ @project_path, @incoming_email_token =
mail_key && mail_key.split('+', 2)
end
def can_handle?
- !authentication_token.nil?
+ !incoming_email_token.nil?
end
def execute
@@ -29,7 +29,7 @@ def execute
end
def author
- @author ||= User.find_by(authentication_token: authentication_token)
+ @author ||= User.find_by(incoming_email_token: incoming_email_token)
end
def project
diff --git a/spec/features/projects/issues/issues_spec.rb b/spec/features/projects/issues/issues_spec.rb
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/spec/fixtures/emails/wrong_authentication_token.eml b/spec/fixtures/emails/wrong_incoming_email_token.eml
similarity index 100%
rename from spec/fixtures/emails/wrong_authentication_token.eml
rename to spec/fixtures/emails/wrong_incoming_email_token.eml
diff --git a/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb b/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb
index a5cc7b02936f3..939189a3bc04d 100644
--- a/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb
+++ b/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb
@@ -18,7 +18,7 @@
create(
:user,
email: 'jake@adventuretime.ooo',
- authentication_token: 'auth_token'
+ incoming_email_token: 'auth_token'
)
end
@@ -60,8 +60,8 @@
end
end
- context "when we can't find the authentication_token" do
- let(:email_raw) { fixture_file("emails/wrong_authentication_token.eml") }
+ context "when we can't find the incoming_email_token" do
+ let(:email_raw) { fixture_file("emails/wrong_incoming_email_token.eml") }
it "raises an UserNotFoundError" do
expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotFoundError)
--
GitLab
From 011e561bfa227f3ecbafe5b1ffd51700c680a15f Mon Sep 17 00:00:00 2001
From: tiagonbotelho <tiagonbotelho@hotmail.com>
Date: Tue, 18 Oct 2016 19:03:31 +0100
Subject: [PATCH 40/56] implements reset incoming email token on issues modal
and account page, reactivates all tests and writes more tests for it
---
app/assets/javascripts/issuable.js.es6 | 22 +++++++++
app/assets/stylesheets/pages/profile.scss | 4 ++
app/controllers/profiles_controller.rb | 4 +-
app/controllers/projects_controller.rb | 7 +++
app/helpers/accounts_helper.rb | 5 ++
app/models/project.rb | 2 +-
app/models/user.rb | 2 +-
app/views/profiles/accounts/show.html.haml | 49 +++++++++++--------
.../projects/issues/_issue_by_email.html.haml | 21 +++++---
.../use-separate-token-for-incoming-email.yml | 4 ++
config/routes/project.rb | 1 +
db/schema.rb | 4 +-
features/profile/profile.feature | 5 --
features/steps/profile/profile.rb | 12 -----
lib/gitlab/email/handler.rb | 3 +-
lib/gitlab/incoming_email.rb | 12 ++++-
spec/controllers/projects_controller_spec.rb | 27 ++++++++++
spec/features/issues_spec.rb | 23 ++++++++-
spec/features/profile_spec.rb | 29 +++++++++++
spec/features/projects/issues/issues_spec.rb | 0
.../handler/create_issue_handler_spec.rb | 2 +-
spec/models/project_spec.rb | 5 +-
22 files changed, 184 insertions(+), 59 deletions(-)
create mode 100644 app/helpers/accounts_helper.rb
create mode 100644 changelogs/unreleased/use-separate-token-for-incoming-email.yml
delete mode 100644 spec/features/projects/issues/issues_spec.rb
diff --git a/app/assets/javascripts/issuable.js.es6 b/app/assets/javascripts/issuable.js.es6
index 8fc498be27d27..46503c290aed9 100644
--- a/app/assets/javascripts/issuable.js.es6
+++ b/app/assets/javascripts/issuable.js.es6
@@ -10,6 +10,7 @@
Issuable.initSearch();
Issuable.initChecks();
Issuable.initResetFilters();
+ Issuable.resetIncomingEmailToken();
return Issuable.initLabelFilterRemove();
},
initTemplates: function() {
@@ -154,6 +155,27 @@
this.issuableBulkActions.willUpdateLabels = false;
}
return true;
+ },
+
+ resetIncomingEmailToken: function() {
+ $('.incoming-email-token-reset').on('click', function(e) {
+ e.preventDefault();
+
+ $.ajax({
+ type: 'PUT',
+ url: $('.incoming-email-token-reset').attr('href'),
+ dataType: 'json',
+ success: function(response) {
+ $('#issue_email').val(response.new_issue_address).focus();
+ },
+ beforeSend: function() {
+ $('.incoming-email-token-reset').text('resetting...');
+ },
+ complete: function() {
+ $('.incoming-email-token-reset').text('reset it');
+ }
+ });
+ });
}
};
diff --git a/app/assets/stylesheets/pages/profile.scss b/app/assets/stylesheets/pages/profile.scss
index ede29db1979aa..6fab97a71aacf 100644
--- a/app/assets/stylesheets/pages/profile.scss
+++ b/app/assets/stylesheets/pages/profile.scss
@@ -23,6 +23,10 @@
color: $md-link-color;
}
+.private-tokens-reset div.reset-action:not(:first-child) {
+ padding-top: 15px;
+}
+
.oauth-buttons {
.btn-group {
margin-right: 10px;
diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb
index e4865642cd320..f0c71725ea8c8 100644
--- a/app/controllers/profiles_controller.rb
+++ b/app/controllers/profiles_controller.rb
@@ -26,7 +26,7 @@ def update
def reset_private_token
if current_user.reset_authentication_token!
- flash[:notice] = "Private token was successfully updated"
+ flash[:notice] = "Private token was successfully reset"
end
redirect_to profile_account_path
@@ -34,7 +34,7 @@ def reset_private_token
def reset_incoming_email_token
if current_user.reset_incoming_email_token!
- flash[:notice] = "Incoming email token was successfully updated"
+ flash[:notice] = "Incoming email token was successfully reset"
end
redirect_to profile_account_path
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 6988527a3be49..4d5725448cd5c 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -160,6 +160,13 @@ def autocomplete_sources
end
end
+ def new_issue_address
+ return render_404 unless Gitlab::IncomingEmail.supports_issue_creation?
+
+ current_user.reset_incoming_email_token!
+ render json: { new_issue_address: @project.new_issue_address(current_user) }
+ end
+
def archive
return access_denied! unless can?(current_user, :archive_project, @project)
diff --git a/app/helpers/accounts_helper.rb b/app/helpers/accounts_helper.rb
new file mode 100644
index 0000000000000..5d27d30eaa353
--- /dev/null
+++ b/app/helpers/accounts_helper.rb
@@ -0,0 +1,5 @@
+module AccountsHelper
+ def incoming_email_token_enabled?
+ current_user.incoming_email_token && Gitlab::IncomingEmail.supports_issue_creation?
+ end
+end
diff --git a/app/models/project.rb b/app/models/project.rb
index 56b84b0aebbb0..4c9c7c001dd4a 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -624,7 +624,7 @@ def web_url_without_protocol
end
def new_issue_address(author)
- return unless Gitlab::IncomingEmail.enabled? && author
+ return unless Gitlab::IncomingEmail.supports_issue_creation? && author
author.ensure_incoming_email_token!
diff --git a/app/models/user.rb b/app/models/user.rb
index 9a3619b0bc304..d6aeda809da8d 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -951,7 +951,7 @@ def domain_matches?(email_domains, email)
def generate_token(token_field)
if token_field == :incoming_email_token
# Needs to be all lowercase and alphanumeric because it's gonna be used in an email address.
- SecureRandom.hex
+ SecureRandom.hex.to_i(16).to_s(36)
else
super
end
diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml
index 2c256b1b2332f..72f658d1b68d7 100644
--- a/app/views/profiles/accounts/show.html.haml
+++ b/app/views/profiles/accounts/show.html.haml
@@ -8,28 +8,35 @@
.row.prepend-top-default
.col-lg-3.profile-settings-sidebar
%h4.prepend-top-0
- Private Tokens
+ = incoming_email_token_enabled? ? "Private Tokens" : "Private Token"
%p
- Your private token is used to access the API and Atom feeds without
- username/password authentication.
- %p
- Your incoming email token is used to create new issues by email, and is
- included in your project-specific email addresses.
- .col-lg-9
- %p.cgray
- - if current_user.private_token
- = label_tag "token", "Private token", class: "label-light"
- = text_field_tag "token", current_user.private_token, class: "form-control"
- - else
- %span You don`t have one yet. Click generate to fix it.
- %p.help-block
- Keep this token secret, anyone with access to it can interact with the GitLab API as if they were you.
- .prepend-top-default
- - if current_user.private_token
- = link_to 'Reset private token', reset_private_token_profile_path, method: :put, data: { confirm: "Are you sure?" }, class: "btn btn-default"
- - else
- = f.submit 'Generate', class: "btn btn-default"
- = link_to 'Reset incoming email token', reset_incoming_email_token_profile_path, method: :put, data: { confirm: "Are you sure?" }, class: "btn btn-default"
+ Keep
+ = incoming_email_token_enabled? ? "these tokens" : "this token"
+ secret, anyone with access to them can interact with GitLab as if they were you.
+ .col-lg-9.private-tokens-reset
+ .reset-action
+ %p.cgray
+ - if current_user.private_token
+ = label_tag "private-token", "Private token", class: "label-light"
+ = text_field_tag "private-token", current_user.private_token, class: "form-control", readonly: true, onclick: "this.select()"
+ - else
+ %span You don't have one yet. Click generate to fix it.
+ %p.help-block
+ Your private token is used to access the API and Atom feeds without username/password authentication.
+ .prepend-top-default
+ - if current_user.private_token
+ = link_to 'Reset private token', reset_private_token_profile_path, method: :put, data: { confirm: "Are you sure?" }, class: "btn btn-default private-token"
+ - else
+ = f.submit 'Generate', class: "btn btn-default"
+ - if incoming_email_token_enabled?
+ .reset-action
+ %p.cgray
+ = label_tag "incoming-email-token", "Incoming Email Token", class: 'label-light'
+ = text_field_tag "incoming-email-token", current_user.incoming_email_token, class: "form-control", readonly: true, onclick: "this.select()"
+ %p.help-block
+ Your incoming email token is used to create new issues by email, and is included in your project-specific email addresses.
+ .prepend-top-default
+ = link_to 'Reset incoming email token', reset_incoming_email_token_profile_path, method: :put, data: { confirm: "Are you sure?" }, class: "btn btn-default incoming-email-token"
%hr
.row.prepend-top-default
diff --git a/app/views/projects/issues/_issue_by_email.html.haml b/app/views/projects/issues/_issue_by_email.html.haml
index 72669372497dc..d2038a2be68ae 100644
--- a/app/views/projects/issues/_issue_by_email.html.haml
+++ b/app/views/projects/issues/_issue_by_email.html.haml
@@ -12,16 +12,23 @@
Create new issue by email
.modal-body
%p
- Write an email to the below email address. (This is a private email address, so keep it secret.)
+ You can create a new issue inside this project by sending an email to the following email address:
.email-modal-input-group.input-group
= text_field_tag :issue_email, email, class: "monospace js-select-on-focus form-control", readonly: true
.input-group-btn
= clipboard_button(clipboard_target: '#issue_email')
%p
- Send an email to this address to create an issue.
- %p
- Use the subject line as the title of your issue.
+ The subject will be used as the title of the new issue, and the message will be the description.
+
+ = link_to 'Slash commands', help_page_path('user/project/slash_commands'), target: '_blank', tabindex: -1
+ and styling with
+ = link_to 'Markdown', help_page_path('user/markdown'), target: '_blank', tabindex: -1
+ are supported.
+
%p
- Use the message as the body of your issue (feel free to include some nice
- = succeed ")." do
- = link_to "Markdown", help_page_path('markdown', 'markdown')
+ This is a private email address, generated just for you.
+
+ Anyone who gets ahold of it can create issues as if they were you.
+ You should
+ = link_to 'reset it', new_issue_address_namespace_project_path(@project.namespace, @project), class: 'incoming-email-token-reset'
+ if that ever happens.
diff --git a/changelogs/unreleased/use-separate-token-for-incoming-email.yml b/changelogs/unreleased/use-separate-token-for-incoming-email.yml
new file mode 100644
index 0000000000000..e498f8dd0a6b7
--- /dev/null
+++ b/changelogs/unreleased/use-separate-token-for-incoming-email.yml
@@ -0,0 +1,4 @@
+---
+title: Use separate email-token for incoming email and revert back the inactive feature
+merge_request: 5914
+author:
diff --git a/config/routes/project.rb b/config/routes/project.rb
index 8142e231621e7..7a1bfa6a9f0b4 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -18,6 +18,7 @@
get :autocomplete_sources
get :activity
get :refs
+ put :new_issue_address
end
scope module: :projects do
diff --git a/db/schema.rb b/db/schema.rb
index fd82fb326a65d..a82a26c662308 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20160819232256) do
+ActiveRecord::Schema.define(version: 20161103171205) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -1176,7 +1176,7 @@
t.boolean "ldap_email", default: false, null: false
t.boolean "external", default: false
t.string "organization"
- t.string "incoming_email_token"
+ t.string "incoming_email_token"
end
add_index "users", ["admin"], name: "index_users_on_admin", using: :btree
diff --git a/features/profile/profile.feature b/features/profile/profile.feature
index 447dd92a458b2..dc1339deb4c5c 100644
--- a/features/profile/profile.feature
+++ b/features/profile/profile.feature
@@ -59,11 +59,6 @@ Feature: Profile
When I unsuccessfully change my password
Then I should see a password error message
- Scenario: I reset my token
- Given I visit profile account page
- Then I reset my token
- And I should see new token
-
Scenario: I visit history tab
Given I have activity
When I visit Audit Log page
diff --git a/features/steps/profile/profile.rb b/features/steps/profile/profile.rb
index 05ab2a7dc7342..ea480d2ad68d7 100644
--- a/features/steps/profile/profile.rb
+++ b/features/steps/profile/profile.rb
@@ -104,18 +104,6 @@ class Spinach::Features::Profile < Spinach::FeatureSteps
end
end
- step 'I reset my token' do
- page.within '.private-token' do
- @old_token = @user.private_token
- click_button "Reset private token"
- end
- end
-
- step 'I should see new token' do
- expect(find("#token").value).not_to eq @old_token
- expect(find("#token").value).to eq @user.reload.private_token
- end
-
step 'I have activity' do
create(:closed_issue_event, author: current_user)
end
diff --git a/lib/gitlab/email/handler.rb b/lib/gitlab/email/handler.rb
index 5cf9d5ebe28d6..bd3267e2a80ba 100644
--- a/lib/gitlab/email/handler.rb
+++ b/lib/gitlab/email/handler.rb
@@ -4,8 +4,7 @@
module Gitlab
module Email
module Handler
- # The `CreateIssueHandler` feature is disabled for the time being.
- HANDLERS = [CreateNoteHandler]
+ HANDLERS = [CreateNoteHandler, CreateIssueHandler]
def self.for(mail, mail_key)
HANDLERS.find do |klass|
diff --git a/lib/gitlab/incoming_email.rb b/lib/gitlab/incoming_email.rb
index d7be50bd43725..801dfde9a368f 100644
--- a/lib/gitlab/incoming_email.rb
+++ b/lib/gitlab/incoming_email.rb
@@ -1,5 +1,7 @@
module Gitlab
module IncomingEmail
+ WILDCARD_PLACEHOLDER = '%{key}'.freeze
+
class << self
FALLBACK_MESSAGE_ID_REGEX = /\Areply\-(.+)@#{Gitlab.config.gitlab.host}\Z/.freeze
@@ -7,8 +9,16 @@ def enabled?
config.enabled && config.address
end
+ def supports_wildcard?
+ config.address && config.address.include?(WILDCARD_PLACEHOLDER)
+ end
+
+ def supports_issue_creation?
+ enabled? && supports_wildcard?
+ end
+
def reply_address(key)
- config.address.gsub('%{key}', key)
+ config.address.gsub(WILDCARD_PLACEHOLDER, key)
end
def key_from_address(address)
diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb
index 8eefa284ba067..ca6bf17005c04 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -264,6 +264,33 @@
end
end
+ describe 'PUT #new_issue_address' do
+ subject do
+ put :new_issue_address,
+ namespace_id: project.namespace.to_param,
+ id: project.to_param
+ user.reload
+ end
+
+ before do
+ sign_in(user)
+ project.team << [user, :developer]
+ allow(Gitlab.config.incoming_email).to receive(:enabled).and_return(true)
+ end
+
+ it 'has http status 200' do
+ expect(response).to have_http_status(200)
+ end
+
+ it 'changes the user incoming email token' do
+ expect { subject }.to change { user.incoming_email_token }
+ end
+
+ it 'changes projects new issue address' do
+ expect { subject }.to change { project.new_issue_address(user) }
+ end
+ end
+
describe "POST #toggle_star" do
it "toggles star if user is signed in" do
sign_in(user)
diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb
index b504329656fc6..cdd02a8c8e36a 100644
--- a/spec/features/issues_spec.rb
+++ b/spec/features/issues_spec.rb
@@ -3,6 +3,7 @@
describe 'Issues', feature: true do
include IssueHelpers
include SortingHelper
+ include WaitForAjax
let(:project) { create(:project) }
@@ -368,6 +369,26 @@
end
end
+ describe 'when I want to reset my incoming email token' do
+ let(:project1) { create(:project, namespace: @user.namespace) }
+
+ before do
+ allow(Gitlab.config.incoming_email).to receive(:enabled).and_return(true)
+ project1.team << [@user, :master]
+ visit namespace_project_issues_path(@user.namespace, project1)
+ end
+
+ it 'changes incoming email address token', js: true do
+ find('.issue-email-modal-btn').click
+ previous_token = find('input#issue_email').value
+
+ find('.incoming-email-token-reset').click
+ wait_for_ajax
+
+ expect(find('input#issue_email').value).not_to eq(previous_token)
+ end
+ end
+
describe 'update labels from issue#show', js: true do
let(:issue) { create(:issue, project: project, author: @user, assignee: @user) }
let!(:label) { create(:label, project: project) }
@@ -553,7 +574,7 @@
end
end
- xdescribe 'new issue by email' do
+ describe 'new issue by email' do
shared_examples 'show the email in the modal' do
before do
stub_incoming_email_setting(enabled: true, address: "p+%{key}@gl.ab")
diff --git a/spec/features/profile_spec.rb b/spec/features/profile_spec.rb
index c3d8c349ca4c1..7a562b5e03d07 100644
--- a/spec/features/profile_spec.rb
+++ b/spec/features/profile_spec.rb
@@ -32,4 +32,33 @@
expect(current_path).to eq(profile_account_path)
end
end
+
+ describe 'when I reset private token' do
+ before do
+ visit profile_account_path
+ end
+
+ it 'resets private token' do
+ previous_token = find("#private-token").value
+
+ click_link('Reset private token')
+
+ expect(find('#private-token').value).not_to eq(previous_token)
+ end
+ end
+
+ describe 'when I reset incoming email token' do
+ before do
+ allow(Gitlab.config.incoming_email).to receive(:enabled).and_return(true)
+ visit profile_account_path
+ end
+
+ it 'resets incoming email token' do
+ previous_token = find('#incoming-email-token').value
+
+ click_link('Reset incoming email token')
+
+ expect(find('#incoming-email-token').value).not_to eq(previous_token)
+ end
+ end
end
diff --git a/spec/features/projects/issues/issues_spec.rb b/spec/features/projects/issues/issues_spec.rb
deleted file mode 100644
index e69de29bb2d1d..0000000000000
diff --git a/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb b/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb
index 939189a3bc04d..cb3651e3845be 100644
--- a/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb
+++ b/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb
@@ -1,7 +1,7 @@
require 'spec_helper'
require_relative '../email_shared_blocks'
-xdescribe Gitlab::Email::Handler::CreateIssueHandler, lib: true do
+describe Gitlab::Email::Handler::CreateIssueHandler, lib: true do
include_context :email_shared_context
it_behaves_like :email_shared_examples
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 0245897938c14..0810d06b50ff4 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -295,7 +295,7 @@
end
end
- xdescribe "#new_issue_address" do
+ describe "#new_issue_address" do
let(:project) { create(:empty_project, path: "somewhere") }
let(:user) { create(:user) }
@@ -305,8 +305,7 @@
end
it 'returns the address to create a new issue' do
- token = user.authentication_token
- address = "p+#{project.namespace.path}/#{project.path}+#{token}@gl.ab"
+ address = "p+#{project.path_with_namespace}+#{user.incoming_email_token}@gl.ab"
expect(project.new_issue_address(user)).to eq(address)
end
--
GitLab
From 706113167682d0b47970834ba218657624c0196e Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Mon, 7 Nov 2016 18:35:14 +0200
Subject: [PATCH 41/56] Refactor namespace regex
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
config/routes/group.rb | 2 +-
config/routes/user.rb | 45 +++++++++++++++++++++---------------------
lib/gitlab/regex.rb | 4 ++++
3 files changed, 28 insertions(+), 23 deletions(-)
diff --git a/config/routes/group.rb b/config/routes/group.rb
index 826048ba196cf..4518cd4015a53 100644
--- a/config/routes/group.rb
+++ b/config/routes/group.rb
@@ -3,7 +3,7 @@
constraints(GroupUrlConstrainer.new) do
scope(path: ':id',
as: :group,
- constraints: { id: /[a-zA-Z.0-9_\-]+(?<!\.atom)/ },
+ constraints: { id: Gitlab::Regex.namespace_route_regex },
controller: :groups) do
get '/', action: :show
patch '/', action: :update
diff --git a/config/routes/user.rb b/config/routes/user.rb
index 0a9c924863da8..dc1068af6f632 100644
--- a/config/routes/user.rb
+++ b/config/routes/user.rb
@@ -14,31 +14,32 @@
constraints(UserUrlConstrainer.new) do
scope(path: ':username',
as: :user,
- constraints: { username: /[a-zA-Z.0-9_\-]+(?<!\.atom)/ },
+ constraints: { username: Gitlab::Regex.namespace_route_regex },
controller: :users) do
get '/', action: :show
end
end
-scope(path: 'users/:username',
- as: :user,
- constraints: { username: /[a-zA-Z.0-9_\-]+(?<!\.atom)/ },
- controller: :users) do
- get :calendar
- get :calendar_activities
- get :groups
- get :projects
- get :contributed, as: :contributed_projects
- get :snippets
- get :exists
- get '/', to: redirect('/%{username}')
-end
+scope(constraints: { username: Gitlab::Regex.namespace_route_regex }) do
+ scope(path: 'users/:username',
+ as: :user,
+ controller: :users) do
+ get :calendar
+ get :calendar_activities
+ get :groups
+ get :projects
+ get :contributed, as: :contributed_projects
+ get :snippets
+ get :exists
+ get '/', to: redirect('/%{username}')
+ end
-# Compatibility with old routing
-# TODO (dzaporozhets): remove in 10.0
-get '/u/:username', to: redirect('/%{username}'), constraints: { username: /[a-zA-Z.0-9_\-]+(?<!\.atom)/ }
-# TODO (dzaporozhets): remove in 9.0
-get '/u/:username/groups', to: redirect('/users/%{username}/groups'), constraints: { username: /[a-zA-Z.0-9_\-]+/ }
-get '/u/:username/projects', to: redirect('/users/%{username}/projects'), constraints: { username: /[a-zA-Z.0-9_\-]+/ }
-get '/u/:username/snippets', to: redirect('/users/%{username}/snippets'), constraints: { username: /[a-zA-Z.0-9_\-]+/ }
-get '/u/:username/contributed', to: redirect('/users/%{username}/contributed'), constraints: { username: /[a-zA-Z.0-9_\-]+/ }
+ # Compatibility with old routing
+ # TODO (dzaporozhets): remove in 10.0
+ get '/u/:username', to: redirect('/%{username}')
+ # TODO (dzaporozhets): remove in 9.0
+ get '/u/:username/groups', to: redirect('/users/%{username}/groups')
+ get '/u/:username/projects', to: redirect('/users/%{username}/projects')
+ get '/u/:username/snippets', to: redirect('/users/%{username}/snippets')
+ get '/u/:username/contributed', to: redirect('/users/%{username}/contributed')
+end
diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb
index 0d30e1bb92ed3..cb1659f9cee9c 100644
--- a/lib/gitlab/regex.rb
+++ b/lib/gitlab/regex.rb
@@ -8,6 +8,10 @@ def namespace_regex
@namespace_regex ||= /\A#{NAMESPACE_REGEX_STR}\z/.freeze
end
+ def namespace_route_regex
+ @namespace_route_regex ||= /#{NAMESPACE_REGEX_STR}/.freeze
+ end
+
def namespace_regex_message
"can contain only letters, digits, '_', '-' and '.'. " \
"Cannot start with '-' or end in '.', '.git' or '.atom'." \
--
GitLab
From 0bc9008ef62277094534711238894b4e43aca7b0 Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Mon, 7 Nov 2016 16:12:05 +0000
Subject: [PATCH 42/56] Fixed todos empty state when filtering
Closes #24127
---
app/helpers/todos_helper.rb | 4 ++++
app/views/dashboard/todos/index.html.haml | 20 ++++++++++++--------
2 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb
index a9db8bb2b8235..a75a03b16d229 100644
--- a/app/helpers/todos_helper.rb
+++ b/app/helpers/todos_helper.rb
@@ -61,6 +61,10 @@ def todos_filter_params
}
end
+ def todos_filter_empty?
+ todos_filter_params.all? {|key, value| value.nil?}
+ end
+
def todos_filter_path(options = {})
without = options.delete(:without)
diff --git a/app/views/dashboard/todos/index.html.haml b/app/views/dashboard/todos/index.html.haml
index e247eebc3fcdc..16bdf284f8370 100644
--- a/app/views/dashboard/todos/index.html.haml
+++ b/app/views/dashboard/todos/index.html.haml
@@ -83,14 +83,18 @@
.todos-all-done
= render "shared/empty_states/todos_all_done.svg"
%h4.text-center
- Good job! Looks like you don't have any todos left.
- %p.text-center
- Are you looking for things to do? Take a look at
- = succeed "," do
- = link_to "the opened issues", issues_dashboard_path
- contribute to
- = link_to "merge requests", merge_requests_dashboard_path
- or mention someone in a comment to assign a new todo automatically.
+ - if todos_filter_empty?
+ Good job! Looks like you don't have any todos left.
+ - else
+ There are no Todos to show.
+ - if todos_filter_empty?
+ %p.text-center
+ Are you looking for things to do? Take a look at
+ = succeed "," do
+ = link_to "the opened issues", issues_dashboard_path
+ contribute to
+ = link_to "merge requests", merge_requests_dashboard_path
+ or mention someone in a comment to assign a new todo automatically.
- else
.todos-empty
.todos-empty-hero
--
GitLab
From bab98d29799deaa6b778a50bfdc44bdf7dc753f2 Mon Sep 17 00:00:00 2001
From: tauriedavis <taurie@gitlab.com>
Date: Mon, 7 Nov 2016 11:06:48 -0800
Subject: [PATCH 43/56] 17492 Update link color for more accessible contrast
---
app/assets/stylesheets/framework/selects.scss | 2 +-
app/assets/stylesheets/framework/variables.scss | 4 ++--
app/views/notify/pipeline_failed_email.html.haml | 14 +++++++-------
app/views/notify/pipeline_success_email.html.haml | 12 ++++++------
4 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/app/assets/stylesheets/framework/selects.scss b/app/assets/stylesheets/framework/selects.scss
index 13749f1b7bdbc..920ce249b9a35 100644
--- a/app/assets/stylesheets/framework/selects.scss
+++ b/app/assets/stylesheets/framework/selects.scss
@@ -63,7 +63,7 @@
}
.select2-highlighted {
- background: #3084bb !important;
+ background: $gl-link-color !important;
}
.select2-results li.select2-result-with-children > .select2-result-label {
diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss
index be2a7ceefff67..e0d00759c9cbb 100644
--- a/app/assets/stylesheets/framework/variables.scss
+++ b/app/assets/stylesheets/framework/variables.scss
@@ -103,7 +103,7 @@ $gl-text-color-light: #8c8c8c;
$gl-text-green: #4a2;
$gl-text-red: #d12f19;
$gl-text-orange: #d90;
-$gl-link-color: #3084bb;
+$gl-link-color: #3777b0;
$gl-dark-link-color: #333;
$gl-placeholder-color: #8f8f8f;
$gl-icon-color: $gl-placeholder-color;
@@ -197,7 +197,7 @@ $line-number-new: #ddfbe6;
$line-number-select: #fbf2da;
$match-line: $gray-light;
$table-border-gray: #f0f0f0;
-$line-target-blue: #eaf3fc;
+$line-target-blue: #f6faff;
$line-select-yellow: #fcf8e7;
$line-select-yellow-dark: #f0e2bd;
diff --git a/app/views/notify/pipeline_failed_email.html.haml b/app/views/notify/pipeline_failed_email.html.haml
index 0995826775ae9..38c852f0a3a1f 100644
--- a/app/views/notify/pipeline_failed_email.html.haml
+++ b/app/views/notify/pipeline_failed_email.html.haml
@@ -103,11 +103,11 @@
%td{style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;"}
%img{height: "13", src: image_url('mailers/ci_pipeline_notif_v1/icon-commit-gray.gif'), style: "display:block;", width: "13"}/
%td{style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;"}
- %a{href: commit_url(@pipeline), style: "color:#3084bb;text-decoration:none;"}
+ %a{href: commit_url(@pipeline), style: "color:#3777b0;text-decoration:none;"}
= @pipeline.short_sha
- if @merge_request
in
- %a{href: merge_request_url(@merge_request), style: "color:#3084bb;text-decoration:none;"}
+ %a{href: merge_request_url(@merge_request), style: "color:#3777b0;text-decoration:none;"}
= @merge_request.to_reference
.commit{style: "color:#5c5c5c;font-weight:300;"}
= @pipeline.git_commit_message.truncate(50)
@@ -134,7 +134,7 @@
%tr.pre-section
%td{style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;color:#333333;font-size:15px;font-weight:400;line-height:1.4;padding:15px 0;"}
Pipeline
- %a{href: pipeline_url(@pipeline), style: "color:#3084bb;text-decoration:none;"}
+ %a{href: pipeline_url(@pipeline), style: "color:#3777b0;text-decoration:none;"}
= "\##{@pipeline.id}"
had
= failed.size
@@ -158,7 +158,7 @@
%td{style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;color:#8c8c8c;font-weight:500;font-size:15px;vertical-align:middle;"}
= build.stage
%td{align: "right", style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;padding:20px 0;color:#8c8c8c;font-weight:500;font-size:15px;"}
- %a{href: pipeline_build_url(@pipeline, build), style: "color:#3084bb;text-decoration:none;"}
+ %a{href: pipeline_build_url(@pipeline, build), style: "color:#3777b0;text-decoration:none;"}
= build.name
%tr.build-log
%td{colspan: "2", style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;padding:0 0 15px;"}
@@ -168,10 +168,10 @@
%td{style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;padding:25px 0;font-size:13px;line-height:1.6;color:#5c5c5c;"}
%img{alt: "GitLab", height: "33", src: image_url('mailers/ci_pipeline_notif_v1/gitlab-logo-full-horizontal.gif'), style: "display:block;margin:0 auto 1em;", width: "90"}/
%div
- %a{href: profile_notifications_url, style: "color:#3084bb;text-decoration:none;"} Manage all notifications
+ %a{href: profile_notifications_url, style: "color:#3777b0;text-decoration:none;"} Manage all notifications
·
- %a{href: help_url, style: "color:#3084bb;text-decoration:none;"} Help
+ %a{href: help_url, style: "color:#3777b0;text-decoration:none;"} Help
%div
You're receiving this email because of your account on
= succeed "." do
- %a{href: root_url, style: "color:#3084bb;text-decoration:none;"}= Gitlab.config.gitlab.host
+ %a{href: root_url, style: "color:#3777b0;text-decoration:none;"}= Gitlab.config.gitlab.host
diff --git a/app/views/notify/pipeline_success_email.html.haml b/app/views/notify/pipeline_success_email.html.haml
index cf9c1d4d72c7c..697c8d19257cf 100644
--- a/app/views/notify/pipeline_success_email.html.haml
+++ b/app/views/notify/pipeline_success_email.html.haml
@@ -103,11 +103,11 @@
%td{style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;"}
%img{height: "13", src: image_url('mailers/ci_pipeline_notif_v1/icon-commit-gray.gif'), style: "display:block;", width: "13"}/
%td{style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;"}
- %a{href: commit_url(@pipeline), style: "color:#3084bb;text-decoration:none;"}
+ %a{href: commit_url(@pipeline), style: "color:#3777b0;text-decoration:none;"}
= @pipeline.short_sha
- if @merge_request
in
- %a{href: merge_request_url(@merge_request), style: "color:#3084bb;text-decoration:none;"}
+ %a{href: merge_request_url(@merge_request), style: "color:#3777b0;text-decoration:none;"}
= @merge_request.to_reference
.commit{style: "color:#5c5c5c;font-weight:300;"}
= @pipeline.git_commit_message.truncate(50)
@@ -135,7 +135,7 @@
- build_count = @pipeline.statuses.latest.size
- stage_count = @pipeline.stages.size
Pipeline
- %a{href: pipeline_url(@pipeline), style: "color:#3084bb;text-decoration:none;"}
+ %a{href: pipeline_url(@pipeline), style: "color:#3777b0;text-decoration:none;"}
= "\##{@pipeline.id}"
successfully completed
= "#{build_count} #{'build'.pluralize(build_count)}"
@@ -145,10 +145,10 @@
%td{style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;padding:25px 0;font-size:13px;line-height:1.6;color:#5c5c5c;"}
%img{alt: "GitLab", height: "33", src: image_url('mailers/ci_pipeline_notif_v1/gitlab-logo-full-horizontal.gif'), style: "display:block;margin:0 auto 1em;", width: "90"}/
%div
- %a{href: profile_notifications_url, style: "color:#3084bb;text-decoration:none;"} Manage all notifications
+ %a{href: profile_notifications_url, style: "color:#3777b0;text-decoration:none;"} Manage all notifications
·
- %a{href: help_url, style: "color:#3084bb;text-decoration:none;"} Help
+ %a{href: help_url, style: "color:#3777b0;text-decoration:none;"} Help
%div
You're receiving this email because of your account on
= succeed "." do
- %a{href: root_url, style: "color:#3084bb;text-decoration:none;"}= Gitlab.config.gitlab.host
+ %a{href: root_url, style: "color:#3777b0;text-decoration:none;"}= Gitlab.config.gitlab.host
--
GitLab
From 5569573a24efca76abede20370988a81d623e6a5 Mon Sep 17 00:00:00 2001
From: Hiroyuki Sato <h-sato@ruby-dev.jp>
Date: Tue, 8 Nov 2016 05:12:17 +0900
Subject: [PATCH 44/56] Refactor method name
---
app/controllers/projects/network_controller.rb | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/app/controllers/projects/network_controller.rb b/app/controllers/projects/network_controller.rb
index e808c9c734304..33a152ad34f86 100644
--- a/app/controllers/projects/network_controller.rb
+++ b/app/controllers/projects/network_controller.rb
@@ -5,7 +5,7 @@ class Projects::NetworkController < Projects::ApplicationController
before_action :require_non_empty_project
before_action :assign_ref_vars
before_action :authorize_download_code!
- before_action :assign_extended_sha1
+ before_action :assign_commit
def show
@url = namespace_project_network_path(@project.namespace, @project, @ref, @options.merge(format: :json))
@@ -24,7 +24,7 @@ def show
end
end
- def assign_extended_sha1
+ def assign_commit
return if params[:extended_sha1].blank?
@options[:extended_sha1] = params[:extended_sha1]
--
GitLab
From d92c0037e1ad9f538d2f42f78349abdae7c01d75 Mon Sep 17 00:00:00 2001
From: Achilleas Pipinellis <axilleas@axilleas.me>
Date: Mon, 7 Nov 2016 21:45:28 +0100
Subject: [PATCH 45/56] Replace trigger with the new ID of the docs project
[ci skip]
---
.gitlab-ci.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index d04069df885af..195783454f9c2 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -331,7 +331,7 @@ trigger_docs:
cache: {}
artifacts: {}
script:
- - "curl -X POST -F token=${DOCS_TRIGGER_TOKEN} -F ref=master -F variables[PROJECT]=ce https://gitlab.com/api/v3/projects/38069/trigger/builds"
+ - "curl -X POST -F token=${DOCS_TRIGGER_TOKEN} -F ref=master -F variables[PROJECT]=ce https://gitlab.com/api/v3/projects/1794617/trigger/builds"
only:
- master@gitlab-org/gitlab-ce
--
GitLab
From 38909dac1d8b5ded2dd94f5eddc21206934d2231 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Mon, 7 Nov 2016 21:39:38 -0800
Subject: [PATCH 46/56] Bump omniauth-gitlab to 1.0.2 to fix incompatibility
with omniauth-oauth2
Closes gitlab-com/support-forum#1270
---
Gemfile | 2 +-
Gemfile.lock | 4 ++--
changelogs/unreleased/sh-bump-omniauth-gitlab.yml | 4 ++++
3 files changed, 7 insertions(+), 3 deletions(-)
create mode 100644 changelogs/unreleased/sh-bump-omniauth-gitlab.yml
diff --git a/Gemfile b/Gemfile
index 5286d2f6d6b8b..cb2a847012651 100644
--- a/Gemfile
+++ b/Gemfile
@@ -26,7 +26,7 @@ gem 'omniauth-bitbucket', '~> 0.0.2'
gem 'omniauth-cas3', '~> 1.1.2'
gem 'omniauth-facebook', '~> 4.0.0'
gem 'omniauth-github', '~> 1.1.1'
-gem 'omniauth-gitlab', '~> 1.0.0'
+gem 'omniauth-gitlab', '~> 1.0.2'
gem 'omniauth-google-oauth2', '~> 0.4.1'
gem 'omniauth-kerberos', '~> 0.3.0', group: :kerberos
gem 'omniauth-saml', '~> 1.7.0'
diff --git a/Gemfile.lock b/Gemfile.lock
index 165b25c170ed2..290e4c3e1b32f 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -456,7 +456,7 @@ GEM
omniauth-github (1.1.2)
omniauth (~> 1.0)
omniauth-oauth2 (~> 1.1)
- omniauth-gitlab (1.0.1)
+ omniauth-gitlab (1.0.2)
omniauth (~> 1.0)
omniauth-oauth2 (~> 1.0)
omniauth-google-oauth2 (0.4.1)
@@ -913,7 +913,7 @@ DEPENDENCIES
omniauth-cas3 (~> 1.1.2)
omniauth-facebook (~> 4.0.0)
omniauth-github (~> 1.1.1)
- omniauth-gitlab (~> 1.0.0)
+ omniauth-gitlab (~> 1.0.2)
omniauth-google-oauth2 (~> 0.4.1)
omniauth-kerberos (~> 0.3.0)
omniauth-saml (~> 1.7.0)
diff --git a/changelogs/unreleased/sh-bump-omniauth-gitlab.yml b/changelogs/unreleased/sh-bump-omniauth-gitlab.yml
new file mode 100644
index 0000000000000..17cd5a993dd1b
--- /dev/null
+++ b/changelogs/unreleased/sh-bump-omniauth-gitlab.yml
@@ -0,0 +1,4 @@
+---
+title: Bump omniauth-gitlab to 1.0.2 to fix incompatibility with omniauth-oauth2
+merge_request:
+author:
--
GitLab
From 34962c7d0833f222aaaea9d6b042542801177699 Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Tue, 8 Nov 2016 10:13:13 +0200
Subject: [PATCH 47/56] Update non-exist group spinach test to match routing
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
features/steps/groups.rb | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/features/steps/groups.rb b/features/steps/groups.rb
index 0e81e99120bee..0c88838767cdc 100644
--- a/features/steps/groups.rb
+++ b/features/steps/groups.rb
@@ -117,7 +117,7 @@ class Spinach::Features::Groups < Spinach::FeatureSteps
end
step 'I visit group "NonExistentGroup" page' do
- visit group_path(-1)
+ visit group_path("NonExistentGroup")
end
step 'the archived project have some issues' do
--
GitLab
From 659ef54605e37ba750486f25957ec367264dffd4 Mon Sep 17 00:00:00 2001
From: Robert Schilling <rschilling@student.tugraz.at>
Date: Tue, 8 Nov 2016 10:05:19 +0100
Subject: [PATCH 48/56] API: Return 400 when creating a systemhook fails
---
.../unreleased/api-return-400-if-post-systemhook-fails.yml | 4 ++++
lib/api/system_hooks.rb | 2 +-
spec/requests/api/system_hooks_spec.rb | 6 ++++++
3 files changed, 11 insertions(+), 1 deletion(-)
create mode 100644 changelogs/unreleased/api-return-400-if-post-systemhook-fails.yml
diff --git a/changelogs/unreleased/api-return-400-if-post-systemhook-fails.yml b/changelogs/unreleased/api-return-400-if-post-systemhook-fails.yml
new file mode 100644
index 0000000000000..d132d7e79c3ff
--- /dev/null
+++ b/changelogs/unreleased/api-return-400-if-post-systemhook-fails.yml
@@ -0,0 +1,4 @@
+---
+title: Return 400 when creating a system hook fails
+merge_request: 7350
+author: Robert Schilling
diff --git a/lib/api/system_hooks.rb b/lib/api/system_hooks.rb
index 32f731c565221..b6bfff9f20f2d 100644
--- a/lib/api/system_hooks.rb
+++ b/lib/api/system_hooks.rb
@@ -32,7 +32,7 @@ class SystemHooks < Grape::API
if hook.save
present hook, with: Entities::Hook
else
- not_found!
+ render_validation_error!(hook)
end
end
diff --git a/spec/requests/api/system_hooks_spec.rb b/spec/requests/api/system_hooks_spec.rb
index f685a3685e696..6c9df21f59838 100644
--- a/spec/requests/api/system_hooks_spec.rb
+++ b/spec/requests/api/system_hooks_spec.rb
@@ -52,6 +52,12 @@
expect(response).to have_http_status(400)
end
+ it "responds with 400 if url is invalid" do
+ post api("/hooks", admin), url: 'hp://mep.mep'
+
+ expect(response).to have_http_status(400)
+ end
+
it "does not create new hook without url" do
expect do
post api("/hooks", admin)
--
GitLab
From dd93079a05d49922fdbe4c221781f39ce69684e8 Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Tue, 8 Nov 2016 09:31:08 +0000
Subject: [PATCH 49/56] Changed helper method to check for none on params Moved
if statements around in view
---
app/helpers/todos_helper.rb | 2 +-
app/views/dashboard/todos/index.html.haml | 10 +++++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb
index a75a03b16d229..09c6978679137 100644
--- a/app/helpers/todos_helper.rb
+++ b/app/helpers/todos_helper.rb
@@ -62,7 +62,7 @@ def todos_filter_params
end
def todos_filter_empty?
- todos_filter_params.all? {|key, value| value.nil?}
+ todos_filter_params.values.none?
end
def todos_filter_path(options = {})
diff --git a/app/views/dashboard/todos/index.html.haml b/app/views/dashboard/todos/index.html.haml
index 16bdf284f8370..5b2465e25ee75 100644
--- a/app/views/dashboard/todos/index.html.haml
+++ b/app/views/dashboard/todos/index.html.haml
@@ -82,12 +82,9 @@
- elsif current_user.todos.any?
.todos-all-done
= render "shared/empty_states/todos_all_done.svg"
- %h4.text-center
- - if todos_filter_empty?
- Good job! Looks like you don't have any todos left.
- - else
- There are no Todos to show.
- if todos_filter_empty?
+ %h4.text-center
+ Good job! Looks like you don't have any todos left.
%p.text-center
Are you looking for things to do? Take a look at
= succeed "," do
@@ -95,6 +92,9 @@
contribute to
= link_to "merge requests", merge_requests_dashboard_path
or mention someone in a comment to assign a new todo automatically.
+ - else
+ %h4.text-center
+ There are no todos to show.
- else
.todos-empty
.todos-empty-hero
--
GitLab
From ebc44befdc773b39a171d38dc13c38cd6630828a Mon Sep 17 00:00:00 2001
From: Valery Sizov <valery@gitlab.com>
Date: Mon, 7 Nov 2016 20:36:58 +0200
Subject: [PATCH 50/56] Fix broken commits search
---
app/controllers/search_controller.rb | 2 +-
app/views/search/results/_commit.html.haml | 2 +-
changelogs/unreleased/24255-search-fix.yml | 4 ++++
spec/features/global_search_spec.rb | 28 ++++++++++++++++++++++
spec/features/search_spec.rb | 26 ++++++++++++++++++++
spec/spec_helper.rb | 1 +
spec/support/search_helpers.rb | 5 ++++
7 files changed, 66 insertions(+), 2 deletions(-)
create mode 100644 changelogs/unreleased/24255-search-fix.yml
create mode 100644 spec/features/global_search_spec.rb
create mode 100644 spec/support/search_helpers.rb
diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb
index d01e0dedf52aa..b666aa01d6ba8 100644
--- a/app/controllers/search_controller.rb
+++ b/app/controllers/search_controller.rb
@@ -16,7 +16,7 @@ def show
@group = nil unless can?(current_user, :read_group, @group)
end
- return if params[:search].nil? || params[:search].blank?
+ return if params[:search].blank?
@search_term = params[:search]
diff --git a/app/views/search/results/_commit.html.haml b/app/views/search/results/_commit.html.haml
index 5b2d83d6b92ec..f34eaf8902796 100644
--- a/app/views/search/results/_commit.html.haml
+++ b/app/views/search/results/_commit.html.haml
@@ -1 +1 @@
-= render 'projects/commits/commit', project: @project, commit: commit
+= render 'projects/commits/commit', project: @project, commit: commit, ref: nil
diff --git a/changelogs/unreleased/24255-search-fix.yml b/changelogs/unreleased/24255-search-fix.yml
new file mode 100644
index 0000000000000..c0afade9bc89e
--- /dev/null
+++ b/changelogs/unreleased/24255-search-fix.yml
@@ -0,0 +1,4 @@
+---
+title: Fix broken commits search
+merge_request:
+author:
diff --git a/spec/features/global_search_spec.rb b/spec/features/global_search_spec.rb
new file mode 100644
index 0000000000000..f6409e00f22fb
--- /dev/null
+++ b/spec/features/global_search_spec.rb
@@ -0,0 +1,28 @@
+require 'spec_helper'
+
+feature 'Global search', feature: true do
+ let(:user) { create(:user) }
+ let(:project) { create(:project, namespace: user.namespace) }
+
+ before do
+ project.team << [user, :master]
+ login_with(user)
+ end
+
+ describe 'I search through the issues and I see pagination' do
+ before do
+ allow_any_instance_of(Gitlab::SearchResults).to receive(:per_page).and_return(1)
+ create_list(:issue, 2, project: project, title: 'initial')
+ end
+
+ it "has a pagination" do
+ visit dashboard_projects_path
+
+ fill_in "search", with: "initial"
+ click_button "Go"
+
+ select_filter("Issues")
+ expect(page).to have_selector('.gl-pagination .page', count: 2)
+ end
+ end
+end
diff --git a/spec/features/search_spec.rb b/spec/features/search_spec.rb
index 1806200c82cc5..caecd027aaa4b 100644
--- a/spec/features/search_spec.rb
+++ b/spec/features/search_spec.rb
@@ -100,6 +100,32 @@
expect(page).to have_link(snippet.title)
end
+
+ it 'finds a commit' do
+ visit namespace_project_path(project.namespace, project)
+
+ page.within '.search' do
+ fill_in 'search', with: 'add'
+ click_button 'Go'
+ end
+
+ click_link "Commits"
+
+ expect(page).to have_selector('.commit-row-description')
+ end
+
+ it 'finds a code' do
+ visit namespace_project_path(project.namespace, project)
+
+ page.within '.search' do
+ fill_in 'search', with: 'def'
+ click_button 'Go'
+ end
+
+ click_link "Code"
+
+ expect(page).to have_selector('.file-content .code')
+ end
end
describe 'Right header search field', feature: true do
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index b2ca856f89f57..73cf4c9a24cb1 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -29,6 +29,7 @@
config.include Devise::Test::ControllerHelpers, type: :controller
config.include Warden::Test::Helpers, type: :request
config.include LoginHelpers, type: :feature
+ config.include SearchHelpers, type: :feature
config.include StubConfiguration
config.include EmailHelpers
config.include TestEnv
diff --git a/spec/support/search_helpers.rb b/spec/support/search_helpers.rb
new file mode 100644
index 0000000000000..abbbb636d66c6
--- /dev/null
+++ b/spec/support/search_helpers.rb
@@ -0,0 +1,5 @@
+module SearchHelpers
+ def select_filter(name)
+ find(:xpath, "//ul[contains(@class, 'search-filter')]//a[contains(.,'#{name}')]").click
+ end
+end
--
GitLab
From 869696bca3d8ff72e2dbaa96744eb7a7d560f0cf Mon Sep 17 00:00:00 2001
From: Valery Sizov <valery@gitlab.com>
Date: Tue, 8 Nov 2016 14:21:19 +0200
Subject: [PATCH 51/56] Faster search
---
app/models/repository.rb | 4 +++
.../unreleased/faster_project_search.yml | 4 +++
lib/gitlab/project_search_results.rb | 30 ++++++++-----------
spec/models/repository_spec.rb | 13 ++++++++
4 files changed, 33 insertions(+), 18 deletions(-)
create mode 100644 changelogs/unreleased/faster_project_search.yml
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 30be726243849..7d06ce1e85bec 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -1064,6 +1064,10 @@ def is_ancestor?(ancestor_id, descendant_id)
end
def search_files(query, ref)
+ unless exists? && has_visible_content? && query.present?
+ return []
+ end
+
offset = 2
args = %W(#{Gitlab.config.git.bin_path} grep -i -I -n --before-context #{offset} --after-context #{offset} -E -e #{Regexp.escape(query)} #{ref || root_ref})
Gitlab::Popen.popen(args, path_to_repo).first.scrub.split(/^--$/)
diff --git a/changelogs/unreleased/faster_project_search.yml b/changelogs/unreleased/faster_project_search.yml
new file mode 100644
index 0000000000000..e29a9f34ed45e
--- /dev/null
+++ b/changelogs/unreleased/faster_project_search.yml
@@ -0,0 +1,4 @@
+---
+title: Faster search inside Project
+merge_request:
+author:
diff --git a/lib/gitlab/project_search_results.rb b/lib/gitlab/project_search_results.rb
index 24733435a5a86..b8326a64b2226 100644
--- a/lib/gitlab/project_search_results.rb
+++ b/lib/gitlab/project_search_results.rb
@@ -5,11 +5,7 @@ class ProjectSearchResults < SearchResults
def initialize(current_user, project, query, repository_ref = nil)
@current_user = current_user
@project = project
- @repository_ref = if repository_ref.present?
- repository_ref
- else
- nil
- end
+ @repository_ref = repository_ref.presence
@query = query
end
@@ -47,33 +43,31 @@ def commits_count
private
def blobs
- if project.empty_repo? || query.blank?
- []
- else
- project.repository.search_files(query, repository_ref)
- end
+ @blobs ||= project.repository.search_files(query, repository_ref)
end
def wiki_blobs
- if project.wiki_enabled? && query.present?
- project_wiki = ProjectWiki.new(project)
+ @wiki_blobs ||= begin
+ if project.wiki_enabled? && query.present?
+ project_wiki = ProjectWiki.new(project)
- unless project_wiki.empty?
- project_wiki.search_files(query)
+ unless project_wiki.empty?
+ project_wiki.search_files(query)
+ else
+ []
+ end
else
[]
end
- else
- []
end
end
def notes
- project.notes.user.search(query, as_user: @current_user).order('updated_at DESC')
+ @notes ||= project.notes.user.search(query, as_user: @current_user).order('updated_at DESC')
end
def commits
- project.repository.find_commits_by_message(query)
+ @commits ||= project.repository.find_commits_by_message(query)
end
def project_ids_relation
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 04b7d19d41496..12989d4db531b 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -362,6 +362,19 @@
expect(results.first).not_to start_with('fatal:')
end
+ it 'properly handles when query is not present' do
+ results = repository.search_files('', 'master')
+
+ expect(results).to match_array([])
+ end
+
+ it 'properly handles query when repo is empty' do
+ repository = create(:empty_project).repository
+ results = repository.search_files('test', 'master')
+
+ expect(results).to match_array([])
+ end
+
describe 'result' do
subject { results.first }
--
GitLab
From 08d21fe89927fa9ebd3695fde047a17d27893dbf Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Tue, 8 Nov 2016 14:32:42 +0200
Subject: [PATCH 52/56] Add small improvements to constrainers and specs
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
lib/constraints/group_url_constrainer.rb | 2 +-
lib/constraints/user_url_constrainer.rb | 2 +-
spec/lib/constraints/group_url_constrainer_spec.rb | 2 +-
spec/lib/constraints/user_url_constrainer_spec.rb | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/lib/constraints/group_url_constrainer.rb b/lib/constraints/group_url_constrainer.rb
index 2bf973a73dafc..2af6e1a11c861 100644
--- a/lib/constraints/group_url_constrainer.rb
+++ b/lib/constraints/group_url_constrainer.rb
@@ -7,7 +7,7 @@ def matches?(request)
id = extract_resource_path(request.path)
if id =~ Gitlab::Regex.namespace_regex
- !!Group.find_by_path(id)
+ Group.find_by(path: id).present?
else
false
end
diff --git a/lib/constraints/user_url_constrainer.rb b/lib/constraints/user_url_constrainer.rb
index 9583cace022eb..4d722ad5af248 100644
--- a/lib/constraints/user_url_constrainer.rb
+++ b/lib/constraints/user_url_constrainer.rb
@@ -7,7 +7,7 @@ def matches?(request)
id = extract_resource_path(request.path)
if id =~ Gitlab::Regex.namespace_regex
- !!User.find_by('lower(username) = ?', id.downcase)
+ User.find_by('lower(username) = ?', id.downcase).present?
else
false
end
diff --git a/spec/lib/constraints/group_url_constrainer_spec.rb b/spec/lib/constraints/group_url_constrainer_spec.rb
index be69a36c2b67c..42299b17c2bc4 100644
--- a/spec/lib/constraints/group_url_constrainer_spec.rb
+++ b/spec/lib/constraints/group_url_constrainer_spec.rb
@@ -14,6 +14,6 @@
end
def request(path)
- OpenStruct.new(path: path)
+ double(:request, path: path)
end
end
diff --git a/spec/lib/constraints/user_url_constrainer_spec.rb b/spec/lib/constraints/user_url_constrainer_spec.rb
index d7b7d5664ff8d..b3f8530c60979 100644
--- a/spec/lib/constraints/user_url_constrainer_spec.rb
+++ b/spec/lib/constraints/user_url_constrainer_spec.rb
@@ -11,6 +11,6 @@
end
def request(path)
- OpenStruct.new(path: path)
+ double(:request, path: path)
end
end
--
GitLab
From 5395f92e7ca93a907cdb2507b2ba2073863bc83a Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Tue, 8 Nov 2016 15:54:17 +0200
Subject: [PATCH 53/56] Fix routing spec for group controller
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
spec/routing/routing_spec.rb | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/spec/routing/routing_spec.rb b/spec/routing/routing_spec.rb
index c18a2d55e4386..61dca5d5a62c4 100644
--- a/spec/routing/routing_spec.rb
+++ b/spec/routing/routing_spec.rb
@@ -266,13 +266,13 @@
end
it "also display group#show on the short path" do
- allow(Group).to receive(:find_by_path).and_return(true)
+ allow(Group).to receive(:find_by).and_return(true)
expect(get('/1')).to route_to('groups#show', id: '1')
end
it "also display group#show with dot in the path" do
- allow(Group).to receive(:find_by_path).and_return(true)
+ allow(Group).to receive(:find_by).and_return(true)
expect(get('/group.with.dot')).to route_to('groups#show', id: 'group.with.dot')
end
--
GitLab
From 937fe659a1f26c2f88c802ba2c01e6a7bc5b9ede Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= <alejorro70@gmail.com>
Date: Tue, 8 Nov 2016 19:08:39 -0300
Subject: [PATCH 54/56] Fix merge conflicts on CE Upstream merge
---
.gitlab-ci.yml | 4 --
.../admin/application_settings_controller.rb | 6 +-
app/views/search/results/_commit.html.haml | 4 --
config/routes/group.rb | 64 +++++++------------
config/routes/project.rb | 4 +-
spec/features/global_search_spec.rb | 17 -----
spec/models/project_spec.rb | 4 --
spec/spec_helper.rb | 4 --
8 files changed, 26 insertions(+), 81 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 0c7b6ecb7ae82..ea4c6731a155f 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -325,11 +325,7 @@ trigger_docs:
cache: {}
artifacts: {}
script:
-<<<<<<< HEAD
- "curl -X POST -F token=${DOCS_TRIGGER_TOKEN} -F ref=master -F variables[PROJECT]=ee https://gitlab.com/api/v3/projects/1794617/trigger/builds"
-=======
- - "curl -X POST -F token=${DOCS_TRIGGER_TOKEN} -F ref=master -F variables[PROJECT]=ce https://gitlab.com/api/v3/projects/1794617/trigger/builds"
->>>>>>> ce/master
only:
- master@gitlab-org/gitlab-ee
diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb
index b0f5326498f94..8fd5907197c4f 100644
--- a/app/controllers/admin/application_settings_controller.rb
+++ b/app/controllers/admin/application_settings_controller.rb
@@ -133,20 +133,16 @@ def application_setting_params
:elasticsearch_port,
:usage_ping_enabled,
:enabled_git_access_protocol,
-<<<<<<< HEAD
:repository_size_limit,
-=======
:housekeeping_enabled,
:housekeeping_bitmaps_enabled,
:housekeeping_incremental_repack_period,
:housekeeping_full_repack_period,
:housekeeping_gc_period,
repository_storages: [],
->>>>>>> ce/master
restricted_visibility_levels: [],
import_sources: [],
- disabled_oauth_sign_in_sources: [],
- repository_storages: []
+ disabled_oauth_sign_in_sources: []
)
end
end
diff --git a/app/views/search/results/_commit.html.haml b/app/views/search/results/_commit.html.haml
index 50168ada54b81..78255d42243d3 100644
--- a/app/views/search/results/_commit.html.haml
+++ b/app/views/search/results/_commit.html.haml
@@ -1,5 +1 @@
-<<<<<<< HEAD
= render 'projects/commits/commit', project: commit.project, commit: commit, ref: nil
-=======
-= render 'projects/commits/commit', project: @project, commit: commit, ref: nil
->>>>>>> ce/master
diff --git a/config/routes/group.rb b/config/routes/group.rb
index 0b1076008c71e..ba43fbcb2433d 100644
--- a/config/routes/group.rb
+++ b/config/routes/group.rb
@@ -14,45 +14,6 @@
resources :groups, only: [:index, :new, :create]
-<<<<<<< HEAD
- scope module: :groups do
- ## EE-specific
- resource :analytics, only: [:show]
- resource :ldap, only: [] do
- member do
- put :sync
- end
- end
-
- resources :ldap_group_links, only: [:index, :create, :destroy]
- ## EE-specific
-
- resources :group_members, only: [:index, :create, :update, :destroy], concerns: :access_requestable do
- post :resend_invite, on: :member
- delete :leave, on: :collection
- end
-
- resource :avatar, only: [:destroy]
- resources :milestones, constraints: { id: /[^\/]+/ }, only: [:index, :show, :update, :new, :create]
- resources :labels, except: [:show], constraints: { id: /\d+/ }
-
- ## EE-specific
- resource :notification_setting, only: [:update]
- resources :audit_events, only: [:index]
- ## EE-specific
- end
-
- ## EE-specific
- resources :hooks, only: [:index, :create, :destroy], constraints: { id: /\d+/ }, module: :groups do
- member do
- get :test
- end
- end
- ## EE-specific
- end
-
- get 'groups/:id' => 'groups#show', as: :group_canonical
-=======
scope(path: 'groups/:id', controller: :groups) do
get :edit, as: :edit_group
get :issues, as: :issues_group
@@ -62,6 +23,17 @@
end
scope(path: 'groups/:group_id', module: :groups, as: :group) do
+ ## EE-specific
+ resource :analytics, only: [:show]
+ resource :ldap, only: [] do
+ member do
+ put :sync
+ end
+ end
+
+ resources :ldap_group_links, only: [:index, :create, :destroy]
+ ## EE-specific
+
resources :group_members, only: [:index, :create, :update, :destroy], concerns: :access_requestable do
post :resend_invite, on: :member
delete :leave, on: :collection
@@ -70,8 +42,20 @@
resource :avatar, only: [:destroy]
resources :milestones, constraints: { id: /[^\/]+/ }, only: [:index, :show, :update, :new, :create]
resources :labels, except: [:show], constraints: { id: /\d+/ }
->>>>>>> ce/master
+
+ ## EE-specific
+ resource :notification_setting, only: [:update]
+ resources :audit_events, only: [:index]
+ ## EE-specific
+end
+
+## EE-specific
+resources :hooks, only: [:index, :create, :destroy], constraints: { id: /\d+/ }, module: :groups do
+ member do
+ get :test
+ end
end
+## EE-specific
# Must be last route in this file
get 'groups/:id' => 'groups#show', as: :group_canonical
diff --git a/config/routes/project.rb b/config/routes/project.rb
index bfcd419a5ab5f..a57c18a065c37 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -338,16 +338,14 @@
end
end
-<<<<<<< HEAD
## EE-specific
resources :audit_events, only: [:index]
## EE-specific
-=======
+
# Since both wiki and repository routing contains wildcard characters
# its preferable to keep it below all other project routes
draw :wiki
draw :repository
->>>>>>> ce/master
end
end
end
diff --git a/spec/features/global_search_spec.rb b/spec/features/global_search_spec.rb
index 7224c91106759..9469e572c7c1e 100644
--- a/spec/features/global_search_spec.rb
+++ b/spec/features/global_search_spec.rb
@@ -1,25 +1,17 @@
require 'spec_helper'
-<<<<<<< HEAD
-feature 'Global elastic search', feature: true do
-=======
feature 'Global search', feature: true do
->>>>>>> ce/master
let(:user) { create(:user) }
let(:project) { create(:project, namespace: user.namespace) }
before do
-<<<<<<< HEAD
stub_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
Gitlab::Elastic::Helper.create_empty_index
-=======
->>>>>>> ce/master
project.team << [user, :master]
login_with(user)
end
-<<<<<<< HEAD
after do
Gitlab::Elastic::Helper.delete_index
stub_application_setting(elasticsearch_search: false, elasticsearch_indexing: false)
@@ -30,12 +22,6 @@
create_list(:issue, 21, project: project, title: 'initial')
Gitlab::Elastic::Helper.refresh_index
-=======
- describe 'I search through the issues and I see pagination' do
- before do
- allow_any_instance_of(Gitlab::SearchResults).to receive(:per_page).and_return(1)
- create_list(:issue, 2, project: project, title: 'initial')
->>>>>>> ce/master
end
it "has a pagination" do
@@ -48,7 +34,6 @@
expect(page).to have_selector('.gl-pagination .page', count: 2)
end
end
-<<<<<<< HEAD
describe 'I search through the blobs' do
before do
@@ -86,6 +71,4 @@
expect(page).to have_selector('.commit-row-description')
end
end
-=======
->>>>>>> ce/master
end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index c1b40aafe4c31..750d8d1f2b86c 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -296,7 +296,6 @@
end
end
-<<<<<<< HEAD
describe "#kerberos_url_to_repo" do
let(:project) { create(:empty_project, path: "somewhere") }
@@ -305,10 +304,7 @@
end
end
- xdescribe "#new_issue_address" do
-=======
describe "#new_issue_address" do
->>>>>>> ce/master
let(:project) { create(:empty_project, path: "somewhere") }
let(:user) { create(:user) }
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 15f7179fdfa71..d63c8f39eeef4 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -29,11 +29,7 @@
config.include Devise::Test::ControllerHelpers, type: :controller
config.include Warden::Test::Helpers, type: :request
config.include LoginHelpers, type: :feature
-<<<<<<< HEAD
- config.include SearchHelpers, type: :feature
-=======
config.include SearchHelpers, type: :feature
->>>>>>> ce/master
config.include StubConfiguration
config.include EmailHelpers
config.include TestEnv
--
GitLab
From dbaeb02572d73085bd5175efbc20204ea520be82 Mon Sep 17 00:00:00 2001
From: Valery Sizov <valery@gitlab.com>
Date: Wed, 9 Nov 2016 09:00:44 +0200
Subject: [PATCH 55/56] Resolve global_search_spec.rb
---
spec/features/es_global_search_spec.rb | 74 ++++++++++++++++++++++++++
spec/features/global_search_spec.rb | 50 +----------------
2 files changed, 76 insertions(+), 48 deletions(-)
create mode 100644 spec/features/es_global_search_spec.rb
diff --git a/spec/features/es_global_search_spec.rb b/spec/features/es_global_search_spec.rb
new file mode 100644
index 0000000000000..406d06a9a86e3
--- /dev/null
+++ b/spec/features/es_global_search_spec.rb
@@ -0,0 +1,74 @@
+require 'spec_helper'
+
+feature 'Global elastic search', feature: true do
+ let(:user) { create(:user) }
+ let(:project) { create(:project, namespace: user.namespace) }
+
+ before do
+ stub_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
+ Gitlab::Elastic::Helper.create_empty_index
+
+ project.team << [user, :master]
+ login_with(user)
+ end
+
+ after do
+ Gitlab::Elastic::Helper.delete_index
+ stub_application_setting(elasticsearch_search: false, elasticsearch_indexing: false)
+ end
+
+ describe 'I search through the issues and I see pagination' do
+ before do
+ create_list(:issue, 21, project: project, title: 'initial')
+
+ Gitlab::Elastic::Helper.refresh_index
+ end
+
+ it "has a pagination" do
+ visit dashboard_projects_path
+
+ fill_in "search", with: "initial"
+ click_button "Go"
+
+ select_filter("Issues")
+ expect(page).to have_selector('.gl-pagination .page', count: 2)
+ end
+ end
+
+ describe 'I search through the blobs' do
+ before do
+ project.repository.index_blobs
+
+ Gitlab::Elastic::Helper.refresh_index
+ end
+
+ it "finds files" do
+ visit dashboard_projects_path
+
+ fill_in "search", with: "def"
+ click_button "Go"
+
+ select_filter("Code")
+
+ expect(page).to have_selector('.file-content .code')
+ end
+ end
+
+ describe 'I search through the commits' do
+ before do
+ project.repository.index_commits
+ Gitlab::Elastic::Helper.refresh_index
+ end
+
+ it "finds commits" do
+ visit dashboard_projects_path
+
+ fill_in "search", with: "add"
+ click_button "Go"
+
+ select_filter("Commits")
+
+ expect(page).to have_selector('.commit-row-description')
+ end
+ end
+end
diff --git a/spec/features/global_search_spec.rb b/spec/features/global_search_spec.rb
index 9469e572c7c1e..f6409e00f22fb 100644
--- a/spec/features/global_search_spec.rb
+++ b/spec/features/global_search_spec.rb
@@ -5,23 +5,14 @@
let(:project) { create(:project, namespace: user.namespace) }
before do
- stub_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
- Gitlab::Elastic::Helper.create_empty_index
-
project.team << [user, :master]
login_with(user)
end
- after do
- Gitlab::Elastic::Helper.delete_index
- stub_application_setting(elasticsearch_search: false, elasticsearch_indexing: false)
- end
-
describe 'I search through the issues and I see pagination' do
before do
- create_list(:issue, 21, project: project, title: 'initial')
-
- Gitlab::Elastic::Helper.refresh_index
+ allow_any_instance_of(Gitlab::SearchResults).to receive(:per_page).and_return(1)
+ create_list(:issue, 2, project: project, title: 'initial')
end
it "has a pagination" do
@@ -34,41 +25,4 @@
expect(page).to have_selector('.gl-pagination .page', count: 2)
end
end
-
- describe 'I search through the blobs' do
- before do
- project.repository.index_blobs
-
- Gitlab::Elastic::Helper.refresh_index
- end
-
- it "finds files" do
- visit dashboard_projects_path
-
- fill_in "search", with: "def"
- click_button "Go"
-
- select_filter("Code")
-
- expect(page).to have_selector('.file-content .code')
- end
- end
-
- describe 'I search through the commits' do
- before do
- project.repository.index_commits
- Gitlab::Elastic::Helper.refresh_index
- end
-
- it "finds commits" do
- visit dashboard_projects_path
-
- fill_in "search", with: "add"
- click_button "Go"
-
- select_filter("Commits")
-
- expect(page).to have_selector('.commit-row-description')
- end
- end
end
--
GitLab
From 327f7ae88c9d33eb0988200c60bbff8178e76c3a Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Wed, 9 Nov 2016 16:18:48 +0200
Subject: [PATCH 56/56] Fix group hooks routing after merge conflicts
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
config/routes/group.rb | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/config/routes/group.rb b/config/routes/group.rb
index ba43fbcb2433d..5ceb861077c38 100644
--- a/config/routes/group.rb
+++ b/config/routes/group.rb
@@ -47,15 +47,15 @@
resource :notification_setting, only: [:update]
resources :audit_events, only: [:index]
## EE-specific
-end
-## EE-specific
-resources :hooks, only: [:index, :create, :destroy], constraints: { id: /\d+/ }, module: :groups do
- member do
- get :test
+ ## EE-specific
+ resources :hooks, only: [:index, :create, :destroy], constraints: { id: /\d+/ } do
+ member do
+ get :test
+ end
end
+ ## EE-specific
end
-## EE-specific
# Must be last route in this file
get 'groups/:id' => 'groups#show', as: :group_canonical
--
GitLab