From 9ecb85a4f36669fa05c961eef84cf46d7bf7f39c Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 5 Jun 2017 23:38:06 +0800 Subject: Forbid creating pipeline if it's protected and cannot create the tag if it's a tag, and cannot merge the branch if it's a branch. --- app/services/ci/create_pipeline_service.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'app') diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 13baa63220d..a54af4749ac 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -27,6 +27,12 @@ module Ci return error('Reference not found') end + if tag? + return error("#{ref} is protected") unless access.can_create_tag?(ref) + else + return error("#{ref} is protected") unless access.can_merge_to_branch?(ref) + end + unless commit return error('Commit not found') end @@ -94,6 +100,10 @@ module Ci @commit ||= project.commit(origin_sha || origin_ref) end + def access + @access ||= Gitlab::UserAccess.new(current_user, project: project) + end + def sha commit.try(:id) end -- cgit v1.2.1 From 4408da47b8462055612548b8d43a679c861595e8 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 6 Jun 2017 00:56:38 +0800 Subject: Move the check to Pipeline.allowed_to_create? So that we could use it for the schedule before trying to use CreatePipelineService --- app/models/ci/pipeline.rb | 14 ++++++++++++++ app/models/ci/pipeline_schedule.rb | 2 +- app/services/ci/create_pipeline_service.rb | 28 ++++++++++++++++------------ 3 files changed, 31 insertions(+), 13 deletions(-) (limited to 'app') diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 425ca9278eb..e2caeda2289 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -162,6 +162,20 @@ module Ci where.not(duration: nil).sum(:duration) end + def self.allowed_to_create?(user, project, ref) + repo = project.repository + access = Gitlab::UserAccess.new(user, project: project) + + Ability.allowed?(user, :create_pipeline, project) && + if repo.ref_exists?("#{Gitlab::Git::BRANCH_REF_PREFIX}#{ref}") + access.can_merge_to_branch?(ref) + elsif repo.ref_exists?("#{Gitlab::Git::TAG_REF_PREFIX}#{ref}") + access.can_create_tag?(ref) + else + false + end + end + def stage(name) stage = Ci::Stage.new(self, name: name) stage unless stage.statuses_count.zero? diff --git a/app/models/ci/pipeline_schedule.rb b/app/models/ci/pipeline_schedule.rb index 45d8cd34359..eaca2774bf9 100644 --- a/app/models/ci/pipeline_schedule.rb +++ b/app/models/ci/pipeline_schedule.rb @@ -37,7 +37,7 @@ module Ci end def runnable_by_owner? - Ability.allowed?(owner, :create_pipeline, project) + Ci::Pipeline.allowed_to_create?(owner, project, ref) end def set_next_run_at diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index a54af4749ac..5ed9d1aa517 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -27,10 +27,8 @@ module Ci return error('Reference not found') end - if tag? - return error("#{ref} is protected") unless access.can_create_tag?(ref) - else - return error("#{ref} is protected") unless access.can_merge_to_branch?(ref) + unless Ci::Pipeline.allowed_to_create?(current_user, project, ref) + return error("Insufficient permissions for protected #{ref}") end unless commit @@ -53,6 +51,12 @@ module Ci return error('No builds for this pipeline.') end + process! + end + + private + + def process! Ci::Pipeline.transaction do update_merge_requests_head_pipeline if pipeline.save @@ -66,8 +70,6 @@ module Ci pipeline.tap(&:process!) end - private - def update_merge_requests_head_pipeline return unless pipeline.latest? @@ -100,10 +102,6 @@ module Ci @commit ||= project.commit(origin_sha || origin_ref) end - def access - @access ||= Gitlab::UserAccess.new(current_user, project: project) - end - def sha commit.try(:id) end @@ -121,11 +119,17 @@ module Ci end def branch? - project.repository.ref_exists?(Gitlab::Git::BRANCH_REF_PREFIX + ref) + return @is_branch if defined?(@is_branch) + + @is_branch = + project.repository.ref_exists?(Gitlab::Git::BRANCH_REF_PREFIX + ref) end def tag? - project.repository.ref_exists?(Gitlab::Git::TAG_REF_PREFIX + ref) + return @is_tag if defined?(@is_tag) + + @is_tag = + project.repository.ref_exists?(Gitlab::Git::TAG_REF_PREFIX + ref) end def ref -- cgit v1.2.1 From 47b93fd76138ce24ec78926647497e52c5101dd8 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 6 Jun 2017 02:19:47 +0800 Subject: Don't check permission, only protected ref if no user --- app/services/ci/create_pipeline_service.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'app') diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 5ed9d1aa517..7efea564ba6 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -27,7 +27,7 @@ module Ci return error('Reference not found') end - unless Ci::Pipeline.allowed_to_create?(current_user, project, ref) + unless triggering_user_allowed_for_ref?(trigger_request, ref) return error("Insufficient permissions for protected #{ref}") end @@ -56,6 +56,14 @@ module Ci private + def triggering_user_allowed_for_ref?(trigger_request, ref) + triggering_user = current_user || trigger_request.trigger.owner + + (triggering_user && + Ci::Pipeline.allowed_to_create?(triggering_user, project, ref)) || + !project.protected_for?(ref) + end + def process! Ci::Pipeline.transaction do update_merge_requests_head_pipeline if pipeline.save -- cgit v1.2.1 From 9984f07a28273035d6c989913cb76c9c371965d0 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 6 Jun 2017 18:00:34 +0800 Subject: Disallow legacy trigger without a owner Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11910#note_31594492 https://gitlab.com/gitlab-org/gitlab-ce/issues/30634#note_31601001 --- app/services/ci/create_pipeline_service.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'app') diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 7efea564ba6..a51c52b3f91 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -23,6 +23,10 @@ module Ci return error('Insufficient permissions to create a new pipeline') end + unless trigger_request && trigger_request.trigger.owner + return error('Legacy trigger without a owner is not allowed') + end + unless branch? || tag? return error('Reference not found') end @@ -59,9 +63,7 @@ module Ci def triggering_user_allowed_for_ref?(trigger_request, ref) triggering_user = current_user || trigger_request.trigger.owner - (triggering_user && - Ci::Pipeline.allowed_to_create?(triggering_user, project, ref)) || - !project.protected_for?(ref) + Ci::Pipeline.allowed_to_create?(triggering_user, project, ref) end def process! -- cgit v1.2.1 From e86e1e515a7a4e4e1ee53d3d33bdfebfddd226a6 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 6 Jun 2017 20:23:19 +0800 Subject: Try to report why it's failing and fix tests --- app/services/ci/create_pipeline_service.rb | 2 +- app/services/ci/create_trigger_request_service.rb | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) (limited to 'app') diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index a51c52b3f91..b3dbb548454 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -23,7 +23,7 @@ module Ci return error('Insufficient permissions to create a new pipeline') end - unless trigger_request && trigger_request.trigger.owner + if trigger_request && !trigger_request.trigger.owner return error('Legacy trigger without a owner is not allowed') end diff --git a/app/services/ci/create_trigger_request_service.rb b/app/services/ci/create_trigger_request_service.rb index beb27a5a597..e4f55c27f61 100644 --- a/app/services/ci/create_trigger_request_service.rb +++ b/app/services/ci/create_trigger_request_service.rb @@ -6,7 +6,8 @@ module Ci pipeline = Ci::CreatePipelineService.new(project, trigger.owner, ref: ref). execute(:trigger, ignore_skip_ci: true, trigger_request: trigger_request) - trigger_request if pipeline.persisted? + trigger_request.pipeline = pipeline + trigger_request end end end -- cgit v1.2.1 From 6d17ddac5aaf6c178a13c1e371b072780e7fd049 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 6 Jun 2017 23:52:57 +0800 Subject: Still allow legacy triggers, feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11910#note_31632911 --- app/services/ci/create_pipeline_service.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'app') diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index b3dbb548454..7efea564ba6 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -23,10 +23,6 @@ module Ci return error('Insufficient permissions to create a new pipeline') end - if trigger_request && !trigger_request.trigger.owner - return error('Legacy trigger without a owner is not allowed') - end - unless branch? || tag? return error('Reference not found') end @@ -63,7 +59,9 @@ module Ci def triggering_user_allowed_for_ref?(trigger_request, ref) triggering_user = current_user || trigger_request.trigger.owner - Ci::Pipeline.allowed_to_create?(triggering_user, project, ref) + (triggering_user && + Ci::Pipeline.allowed_to_create?(triggering_user, project, ref)) || + !project.protected_for?(ref) end def process! -- cgit v1.2.1 From 23bfd8c13c803f4efdb9eaf8e6e3c1ffd17640e8 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 4 Jul 2017 05:01:05 +0800 Subject: Consistently check permission for creating pipelines, updating builds and updating pipelines. We check against being able to merge or push if the ref is protected. --- app/models/ci/pipeline.rb | 2 +- app/policies/ci/build_policy.rb | 11 ++++++----- app/policies/ci/pipeline_policy.rb | 19 ++++++++++++++++++- 3 files changed, 25 insertions(+), 7 deletions(-) (limited to 'app') diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index a46c1304667..06ce01095ea 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -169,7 +169,7 @@ module Ci Ability.allowed?(user, :create_pipeline, project) && if repo.ref_exists?("#{Gitlab::Git::BRANCH_REF_PREFIX}#{ref}") - access.can_merge_to_branch?(ref) + access.can_push_or_merge_to_branch?(ref) elsif repo.ref_exists?("#{Gitlab::Git::TAG_REF_PREFIX}#{ref}") access.can_create_tag?(ref) else diff --git a/app/policies/ci/build_policy.rb b/app/policies/ci/build_policy.rb index 2d7405dc240..85245528602 100644 --- a/app/policies/ci/build_policy.rb +++ b/app/policies/ci/build_policy.rb @@ -11,19 +11,20 @@ module Ci cannot! :"#{rule}_commit_status" unless can? :"#{rule}_build" end - if can?(:update_build) && protected_action? + if can?(:update_build) && !can_user_update? cannot! :update_build end end private - def protected_action? - return false unless build.action? + def can_user_update? + user_access.can_push_or_merge_to_branch?(build.ref) + end - !::Gitlab::UserAccess + def user_access + @user_access ||= ::Gitlab::UserAccess .new(user, project: build.project) - .can_merge_to_branch?(build.ref) end end end diff --git a/app/policies/ci/pipeline_policy.rb b/app/policies/ci/pipeline_policy.rb index 10aa2d3e72a..e71cc358353 100644 --- a/app/policies/ci/pipeline_policy.rb +++ b/app/policies/ci/pipeline_policy.rb @@ -1,7 +1,24 @@ module Ci class PipelinePolicy < BasePolicy + alias_method :pipeline, :subject + def rules - delegate! @subject.project + delegate! pipeline.project + + if can?(:update_pipeline) && !can_user_update? + cannot! :update_pipeline + end + end + + private + + def can_user_update? + user_access.can_push_or_merge_to_branch?(pipeline.ref) + end + + def user_access + @user_access ||= ::Gitlab::UserAccess + .new(user, project: pipeline.project) end end end -- cgit v1.2.1 From 005870d5ce1a00b3405d0ae3a639d0c4befcb7a2 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 4 Jul 2017 05:20:44 +0800 Subject: Fix bad conflict resolution --- app/policies/ci/pipeline_policy.rb | 2 +- app/services/ci/create_pipeline_service.rb | 22 ++++++++++++---------- 2 files changed, 13 insertions(+), 11 deletions(-) (limited to 'app') diff --git a/app/policies/ci/pipeline_policy.rb b/app/policies/ci/pipeline_policy.rb index 73b5a40c7fc..8dba28b8d97 100644 --- a/app/policies/ci/pipeline_policy.rb +++ b/app/policies/ci/pipeline_policy.rb @@ -1,6 +1,6 @@ module Ci class PipelinePolicy < BasePolicy - delegate { pipeline.project } + delegate { @subject.project } condition(:user_cannot_update) do !::Gitlab::UserAccess diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index db12116b3ae..e487b7d5f30 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -51,19 +51,13 @@ module Ci return error('No stages / jobs for this pipeline.') end - process! + process! do + pipeline_created_counter.increment(source: source) + end end private - def triggering_user_allowed_for_ref?(trigger_request, ref) - triggering_user = current_user || trigger_request.trigger.owner - - (triggering_user && - Ci::Pipeline.allowed_to_create?(triggering_user, project, ref)) || - !project.protected_for?(ref) - end - def process! Ci::Pipeline.transaction do update_merge_requests_head_pipeline if pipeline.save @@ -75,11 +69,19 @@ module Ci cancel_pending_pipelines if project.auto_cancel_pending_pipelines? - pipeline_created_counter.increment(source: source) + yield pipeline.tap(&:process!) end + def triggering_user_allowed_for_ref?(trigger_request, ref) + triggering_user = current_user || trigger_request.trigger.owner + + (triggering_user && + Ci::Pipeline.allowed_to_create?(triggering_user, project, ref)) || + !project.protected_for?(ref) + end + def update_merge_requests_head_pipeline return unless pipeline.latest? -- cgit v1.2.1 From a4dd3ea168d19d2b65b7e55ed0043c7e7dcac77c Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 4 Jul 2017 18:00:39 +0800 Subject: Make sure that retryable_builds would preload project --- app/models/ci/pipeline.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app') diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index bea2ec1e18c..7963386bdb1 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -21,7 +21,7 @@ module Ci has_many :merge_requests, foreign_key: "head_pipeline_id" has_many :pending_builds, -> { pending }, foreign_key: :commit_id, class_name: 'Ci::Build' - has_many :retryable_builds, -> { latest.failed_or_canceled }, foreign_key: :commit_id, class_name: 'Ci::Build' + has_many :retryable_builds, -> { latest.failed_or_canceled.includes(:project) }, foreign_key: :commit_id, class_name: 'Ci::Build' has_many :cancelable_statuses, -> { cancelable }, foreign_key: :commit_id, class_name: 'CommitStatus' has_many :manual_actions, -> { latest.manual_actions.includes(:project) }, foreign_key: :commit_id, class_name: 'Ci::Build' has_many :artifacts, -> { latest.with_artifacts_not_expired.includes(:project) }, foreign_key: :commit_id, class_name: 'Ci::Build' -- cgit v1.2.1 From 56ea7a0cfe0fcdff33de80fd4602f463367914b2 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 5 Jul 2017 21:55:35 +0800 Subject: Merge allowed_to_create? into CreatePipelineService --- app/models/ci/pipeline.rb | 14 -------------- app/models/ci/pipeline_schedule.rb | 4 ---- app/services/ci/create_pipeline_service.rb | 22 +++++++++++++++++----- app/workers/pipeline_schedule_worker.rb | 13 +++++-------- 4 files changed, 22 insertions(+), 31 deletions(-) (limited to 'app') diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 7963386bdb1..8d1beca9771 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -164,20 +164,6 @@ module Ci where.not(duration: nil).sum(:duration) end - def self.allowed_to_create?(user, project, ref) - repo = project.repository - access = Gitlab::UserAccess.new(user, project: project) - - Ability.allowed?(user, :create_pipeline, project) && - if repo.ref_exists?("#{Gitlab::Git::BRANCH_REF_PREFIX}#{ref}") - access.can_push_or_merge_to_branch?(ref) - elsif repo.ref_exists?("#{Gitlab::Git::TAG_REF_PREFIX}#{ref}") - access.can_create_tag?(ref) - else - false - end - end - def self.internal_sources sources.reject { |source| source == "external" }.values end diff --git a/app/models/ci/pipeline_schedule.rb b/app/models/ci/pipeline_schedule.rb index eaca2774bf9..49455e79c15 100644 --- a/app/models/ci/pipeline_schedule.rb +++ b/app/models/ci/pipeline_schedule.rb @@ -36,10 +36,6 @@ module Ci update_attribute(:active, false) end - def runnable_by_owner? - Ci::Pipeline.allowed_to_create?(owner, project, ref) - end - def set_next_run_at self.next_run_at = Gitlab::Ci::CronParser.new(cron, cron_timezone).next_time_from(Time.now) end diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index e487b7d5f30..485161e5f3f 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -27,7 +27,7 @@ module Ci return error('Reference not found') end - unless triggering_user_allowed_for_ref?(trigger_request, ref) + unless triggering_user_allowed_for_ref?(trigger_request) return error("Insufficient permissions for protected #{ref}") end @@ -74,14 +74,26 @@ module Ci pipeline.tap(&:process!) end - def triggering_user_allowed_for_ref?(trigger_request, ref) + def triggering_user_allowed_for_ref?(trigger_request) triggering_user = current_user || trigger_request.trigger.owner - (triggering_user && - Ci::Pipeline.allowed_to_create?(triggering_user, project, ref)) || + (triggering_user && allowed_to_create?(triggering_user)) || !project.protected_for?(ref) end + def allowed_to_create?(triggering_user) + access = Gitlab::UserAccess.new(triggering_user, project: project) + + Ability.allowed?(triggering_user, :create_pipeline, project) && + if branch? + access.can_push_or_merge_to_branch?(ref) + elsif tag? + access.can_create_tag?(ref) + else + false + end + end + def update_merge_requests_head_pipeline return unless pipeline.latest? @@ -145,7 +157,7 @@ module Ci end def ref - Gitlab::Git.ref_name(origin_ref) + @ref ||= Gitlab::Git.ref_name(origin_ref) end def valid_sha? diff --git a/app/workers/pipeline_schedule_worker.rb b/app/workers/pipeline_schedule_worker.rb index 7b485b3363c..d7087f20dfc 100644 --- a/app/workers/pipeline_schedule_worker.rb +++ b/app/workers/pipeline_schedule_worker.rb @@ -6,15 +6,12 @@ class PipelineScheduleWorker Ci::PipelineSchedule.active.where("next_run_at < ?", Time.now) .preload(:owner, :project).find_each do |schedule| begin - unless schedule.runnable_by_owner? - schedule.deactivate! - next - end - - Ci::CreatePipelineService.new(schedule.project, - schedule.owner, - ref: schedule.ref) + pipeline = Ci::CreatePipelineService.new(schedule.project, + schedule.owner, + ref: schedule.ref) .execute(:schedule, save_on_errors: false, schedule: schedule) + + schedule.deactivate! unless pipeline.persisted? rescue => e Rails.logger.error "#{schedule.id}: Failed to create a scheduled pipeline: #{e.message}" ensure -- cgit v1.2.1 From 550ccf443059412a26adfcba15fbe9d05d39a5f9 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 6 Jul 2017 17:37:27 +0800 Subject: Make message and code more clear --- app/services/ci/create_pipeline_service.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'app') diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 485161e5f3f..a8034e30a85 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -28,7 +28,7 @@ module Ci end unless triggering_user_allowed_for_ref?(trigger_request) - return error("Insufficient permissions for protected #{ref}") + return error("Insufficient permissions for protected ref '#{ref}'") end unless commit @@ -77,8 +77,11 @@ module Ci def triggering_user_allowed_for_ref?(trigger_request) triggering_user = current_user || trigger_request.trigger.owner - (triggering_user && allowed_to_create?(triggering_user)) || + if triggering_user + allowed_to_create?(triggering_user) + else # legacy triggers don't have a corresponding user !project.protected_for?(ref) + end end def allowed_to_create?(triggering_user) -- cgit v1.2.1 From 679789ee93b0e5db3863bfcd539e20074c140984 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 18 Jul 2017 21:56:28 +0800 Subject: Rename can_push_or_merge_to_branch? to can_update_branch? Also make sure pipeline would also check against tag as well --- app/policies/ci/build_policy.rb | 2 +- app/policies/ci/pipeline_policy.rb | 10 +++++++--- app/services/ci/create_pipeline_service.rb | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) (limited to 'app') diff --git a/app/policies/ci/build_policy.rb b/app/policies/ci/build_policy.rb index 00adb51e7de..00f18d0155b 100644 --- a/app/policies/ci/build_policy.rb +++ b/app/policies/ci/build_policy.rb @@ -6,7 +6,7 @@ module Ci if @subject.tag? !access.can_create_tag?(@subject.ref) else - !access.can_push_or_merge_to_branch?(@subject.ref) + !access.can_update_branch?(@subject.ref) end end diff --git a/app/policies/ci/pipeline_policy.rb b/app/policies/ci/pipeline_policy.rb index 8dba28b8d97..07d724c9cfd 100644 --- a/app/policies/ci/pipeline_policy.rb +++ b/app/policies/ci/pipeline_policy.rb @@ -3,9 +3,13 @@ module Ci delegate { @subject.project } condition(:user_cannot_update) do - !::Gitlab::UserAccess - .new(@user, project: @subject.project) - .can_push_or_merge_to_branch?(@subject.ref) + access = ::Gitlab::UserAccess.new(@user, project: @subject.project) + + if @subject.tag? + !access.can_create_tag?(@subject.ref) + else + !access.can_update_branch?(@subject.ref) + end end rule { user_cannot_update }.prevent :update_pipeline diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 8e2184a1f19..8b689968895 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -89,7 +89,7 @@ module Ci Ability.allowed?(triggering_user, :create_pipeline, project) && if branch? - access.can_push_or_merge_to_branch?(ref) + access.can_update_branch?(ref) elsif tag? access.can_create_tag?(ref) else -- cgit v1.2.1 From 1ed6d1541c7973c08cdd4c1906ddcc0c3db893e3 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 18 Jul 2017 22:13:57 +0800 Subject: Rename :user_cannot_update to :protected_ref --- app/policies/ci/build_policy.rb | 4 ++-- app/policies/ci/pipeline_policy.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'app') diff --git a/app/policies/ci/build_policy.rb b/app/policies/ci/build_policy.rb index 00f18d0155b..984e5482288 100644 --- a/app/policies/ci/build_policy.rb +++ b/app/policies/ci/build_policy.rb @@ -1,6 +1,6 @@ module Ci class BuildPolicy < CommitStatusPolicy - condition(:user_cannot_update) do + condition(:protected_ref) do access = ::Gitlab::UserAccess.new(@user, project: @subject.project) if @subject.tag? @@ -10,6 +10,6 @@ module Ci end end - rule { user_cannot_update }.prevent :update_build + rule { protected_ref }.prevent :update_build end end diff --git a/app/policies/ci/pipeline_policy.rb b/app/policies/ci/pipeline_policy.rb index 07d724c9cfd..4e689a9efd5 100644 --- a/app/policies/ci/pipeline_policy.rb +++ b/app/policies/ci/pipeline_policy.rb @@ -2,7 +2,7 @@ module Ci class PipelinePolicy < BasePolicy delegate { @subject.project } - condition(:user_cannot_update) do + condition(:protected_ref) do access = ::Gitlab::UserAccess.new(@user, project: @subject.project) if @subject.tag? @@ -12,6 +12,6 @@ module Ci end end - rule { user_cannot_update }.prevent :update_pipeline + rule { protected_ref }.prevent :update_pipeline end end -- cgit v1.2.1 From b84eb3434d0493cd594eade68d344a9675d72b8a Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 19 Jul 2017 16:42:47 +0800 Subject: Try to merge permission checks into one --- app/services/ci/create_pipeline_service.rb | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'app') diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 8b689968895..f331f86e622 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -19,18 +19,20 @@ module Ci return error('Pipeline is disabled') end - unless trigger_request || can?(current_user, :create_pipeline, project) - return error('Insufficient permissions to create a new pipeline') + triggering_user = current_user || trigger_request.trigger.owner + + unless allowed_to_trigger_pipeline?(triggering_user) + if can?(triggering_user, :create_pipeline, project) + return error("Insufficient permissions for protected ref '#{ref}'") + else + return error('Insufficient permissions to create a new pipeline') + end end unless branch? || tag? return error('Reference not found') end - unless triggering_user_allowed_for_ref?(trigger_request) - return error("Insufficient permissions for protected ref '#{ref}'") - end - unless commit return error('Commit not found') end @@ -74,9 +76,7 @@ module Ci pipeline.tap(&:process!) end - def triggering_user_allowed_for_ref?(trigger_request) - triggering_user = current_user || trigger_request.trigger.owner - + def allowed_to_trigger_pipeline?(triggering_user) if triggering_user allowed_to_create?(triggering_user) else # legacy triggers don't have a corresponding user @@ -87,7 +87,7 @@ module Ci def allowed_to_create?(triggering_user) access = Gitlab::UserAccess.new(triggering_user, project: project) - Ability.allowed?(triggering_user, :create_pipeline, project) && + can?(triggering_user, :create_pipeline, project) && if branch? access.can_update_branch?(ref) elsif tag? -- cgit v1.2.1 From bab44bd99433a77fa45802647d767f0ca94a4a5e Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 19 Jul 2017 13:11:39 +0200 Subject: Fix job merge request link to a forked source project --- app/serializers/build_details_entity.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app') diff --git a/app/serializers/build_details_entity.rb b/app/serializers/build_details_entity.rb index 20f9938f038..8ad5af1987c 100644 --- a/app/serializers/build_details_entity.rb +++ b/app/serializers/build_details_entity.rb @@ -16,7 +16,7 @@ class BuildDetailsEntity < JobEntity end expose :path do |build| - project_merge_request_path(project, build.merge_request) + project_merge_request_path(build.project, build.merge_request) end end -- cgit v1.2.1 From a397a0eb1a4c34c27175e2c4e68e7ceb43a81f02 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 19 Jul 2017 19:12:11 +0800 Subject: Eliminate N+1 queries on checking different protected refs I realized where the N+1 queries were actually coming from project.protected_branches, but how come we cannot preload this, or cache this at all? Then I found that this is somehow a Rails limitation. What we're doing before, eventually come to: project.protected_branches.matching But why it's not cached? (project.protected_branches.loaded? is always false) It's because matching is a class method, which is called on the proxy. In this case, Rails cannot cache the result. I don't know if this is possible to implement or not, because clearly this would require some tricks to implement class methods on associations. So instead, we could just pass project.protected_branches to ProtectedRef.matching, then it would work regularly. With this change, there's no more N+1 queries. --- app/models/concerns/protected_ref.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'app') diff --git a/app/models/concerns/protected_ref.rb b/app/models/concerns/protected_ref.rb index fc6b840f7a8..ca9ef2b9375 100644 --- a/app/models/concerns/protected_ref.rb +++ b/app/models/concerns/protected_ref.rb @@ -25,8 +25,8 @@ module ProtectedRef end end - def protected_ref_accessible_to?(ref, user, action:) - access_levels_for_ref(ref, action: action).any? do |access_level| + def protected_ref_accessible_to?(ref, user, action:, protected_refs: nil) + access_levels_for_ref(ref, action: action, protected_refs: protected_refs).any? do |access_level| access_level.check_access(user) end end @@ -37,8 +37,9 @@ module ProtectedRef end end - def access_levels_for_ref(ref, action:) - self.matching(ref).map(&:"#{action}_access_levels").flatten + def access_levels_for_ref(ref, action:, protected_refs: nil) + self.matching(ref, protected_refs: protected_refs) + .map(&:"#{action}_access_levels").flatten end def matching(ref_name, protected_refs: nil) -- cgit v1.2.1 From d035d735242a47bee7cd5973c9daa7d984800700 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 19 Jul 2017 22:37:38 +0800 Subject: Fix tests and fine tweak permission error message --- app/services/ci/create_pipeline_service.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'app') diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index f331f86e622..700ac42d56e 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -23,16 +23,16 @@ module Ci unless allowed_to_trigger_pipeline?(triggering_user) if can?(triggering_user, :create_pipeline, project) - return error("Insufficient permissions for protected ref '#{ref}'") + if branch? || tag? + return error("Insufficient permissions for protected ref '#{ref}'") + else + return error('Reference not found') + end else return error('Insufficient permissions to create a new pipeline') end end - unless branch? || tag? - return error('Reference not found') - end - unless commit return error('Commit not found') end -- cgit v1.2.1 From a05bc477b99500fa919295e1086f7a8de903e3c4 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 20 Jul 2017 00:08:34 +0800 Subject: Use hash to return multiple objects --- app/services/ci/create_trigger_request_service.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'app') diff --git a/app/services/ci/create_trigger_request_service.rb b/app/services/ci/create_trigger_request_service.rb index 90f75606ddf..1674830a41a 100644 --- a/app/services/ci/create_trigger_request_service.rb +++ b/app/services/ci/create_trigger_request_service.rb @@ -1,13 +1,13 @@ module Ci - class CreateTriggerRequestService - def execute(project, trigger, ref, variables = nil) + module CreateTriggerRequestService + def self.execute(project, trigger, ref, variables = nil) trigger_request = trigger.trigger_requests.create(variables: variables) pipeline = Ci::CreatePipelineService.new(project, trigger.owner, ref: ref) .execute(:trigger, ignore_skip_ci: true, trigger_request: trigger_request) - trigger_request.pipeline = pipeline - trigger_request + { trigger_request: trigger_request, + pipeline: pipeline } end end end -- cgit v1.2.1 From c9c715cd5510456d83da5272f28b7ce7f248c77f Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 20 Jul 2017 01:31:20 +0800 Subject: Make permission checks easier to understand --- app/services/ci/create_pipeline_service.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'app') diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 700ac42d56e..5da70ba87e9 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -23,16 +23,16 @@ module Ci unless allowed_to_trigger_pipeline?(triggering_user) if can?(triggering_user, :create_pipeline, project) - if branch? || tag? - return error("Insufficient permissions for protected ref '#{ref}'") - else - return error('Reference not found') - end + return error("Insufficient permissions for protected ref '#{ref}'") else return error('Insufficient permissions to create a new pipeline') end end + unless branch? || tag? + return error('Reference not found') + end + unless commit return error('Commit not found') end @@ -93,7 +93,7 @@ module Ci elsif tag? access.can_create_tag?(ref) else - false + true # Allow it for now and we'll reject when we check ref existence end end -- cgit v1.2.1 From e9862a9900c6269a41b65ca543035e57b49fede3 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 20 Jul 2017 20:17:42 +0800 Subject: Use struct instead of hash --- app/services/ci/create_trigger_request_service.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'app') diff --git a/app/services/ci/create_trigger_request_service.rb b/app/services/ci/create_trigger_request_service.rb index 1674830a41a..a43d0e4593c 100644 --- a/app/services/ci/create_trigger_request_service.rb +++ b/app/services/ci/create_trigger_request_service.rb @@ -1,13 +1,14 @@ module Ci module CreateTriggerRequestService + Result = Struct.new(:trigger_request, :pipeline) + def self.execute(project, trigger, ref, variables = nil) trigger_request = trigger.trigger_requests.create(variables: variables) pipeline = Ci::CreatePipelineService.new(project, trigger.owner, ref: ref) .execute(:trigger, ignore_skip_ci: true, trigger_request: trigger_request) - { trigger_request: trigger_request, - pipeline: pipeline } + Result.new(trigger_request, pipeline) end end end -- cgit v1.2.1 From fd045bbee94faba08dee2e017fcf3718e9752d1b Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 20 Jul 2017 14:34:39 +0100 Subject: Fixed issue boards sidebar close button with new navigation Closes #35296 --- app/assets/stylesheets/pages/boards.scss | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'app') diff --git a/app/assets/stylesheets/pages/boards.scss b/app/assets/stylesheets/pages/boards.scss index df858cffe09..6039cda96d8 100644 --- a/app/assets/stylesheets/pages/boards.scss +++ b/app/assets/stylesheets/pages/boards.scss @@ -431,7 +431,10 @@ margin: 5px; } -.page-with-layout-nav.page-with-sub-nav .issue-boards-sidebar { +.page-with-layout-nav.page-with-sub-nav .issue-boards-sidebar, +.page-with-new-sidebar.page-with-sidebar .issue-boards-sidebar { + position: absolute; + &.right-sidebar { top: 0; bottom: 0; -- cgit v1.2.1 From 01c9488f4a559063eba77074ba2d369de87b8018 Mon Sep 17 00:00:00 2001 From: Ryan Scott Date: Thu, 30 Mar 2017 10:39:06 +0900 Subject: Added slash command to close an issue as a duplicate. Closes #26372 --- app/models/system_note_metadata.rb | 2 +- app/services/issuable_base_service.rb | 22 ++++++++++++++++++++++ app/services/quick_actions/interpret_service.rb | 14 ++++++++++++++ app/services/system_note_service.rb | 19 +++++++++++++++++++ 4 files changed, 56 insertions(+), 1 deletion(-) (limited to 'app') diff --git a/app/models/system_note_metadata.rb b/app/models/system_note_metadata.rb index 414c95f7705..1ffdd285b91 100644 --- a/app/models/system_note_metadata.rb +++ b/app/models/system_note_metadata.rb @@ -2,7 +2,7 @@ class SystemNoteMetadata < ActiveRecord::Base ICON_TYPES = %w[ commit description merge confidential visible label assignee cross_reference title time_tracking branch milestone discussion task moved opened closed merged - outdated + outdated duplicate ].freeze validates :note, presence: true diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 9078b1f0983..c7e646222bb 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -46,6 +46,14 @@ class IssuableBaseService < BaseService SystemNoteService.change_time_spent(issuable, issuable.project, current_user) end + def create_issue_duplicate_note(issuable, original_issue) + SystemNoteService.mark_duplicate_issue(issuable, issuable.project, current_user, original_issue) + end + + def create_cross_reference_note(noteable, mentioner) + SystemNoteService.cross_reference(noteable, mentioner, current_user) + end + def filter_params(issuable) ability_name = :"admin_#{issuable.to_ability_name}" @@ -58,6 +66,7 @@ class IssuableBaseService < BaseService params.delete(:assignee_ids) params.delete(:assignee_id) params.delete(:due_date) + params.delete(:original_issue_id) end filter_assignee(issuable) @@ -209,6 +218,7 @@ class IssuableBaseService < BaseService change_state(issuable) change_subscription(issuable) change_todo(issuable) + change_issue_duplicate(issuable) toggle_award(issuable) filter_params(issuable) old_labels = issuable.labels.to_a @@ -291,6 +301,18 @@ class IssuableBaseService < BaseService end end + def change_issue_duplicate(issuable) + original_issue_id = params.delete(:original_issue_id) + return if original_issue_id.nil? + + original_issue = IssuesFinder.new(current_user).find(original_issue_id) + if original_issue.present? + create_issue_duplicate_note(issuable, original_issue) + close_service.new(project, current_user, {}).execute(issuable) + create_cross_reference_note(original_issue, issuable) + end + end + def toggle_award(issuable) award = params.delete(:emoji_award) if award diff --git a/app/services/quick_actions/interpret_service.rb b/app/services/quick_actions/interpret_service.rb index 6f82159e6c7..3eecf0b5545 100644 --- a/app/services/quick_actions/interpret_service.rb +++ b/app/services/quick_actions/interpret_service.rb @@ -471,6 +471,20 @@ module QuickActions end end + desc 'Mark this issue as a duplicate of another issue' + params '#issue' + condition do + issuable.is_a?(Issue) && + issuable.persisted? && + current_user.can?(:"update_#{issuable.to_ability_name}", issuable) + end + command :duplicate do |duplicate_param| + original_issue = extract_references(duplicate_param, :issue).first + if original_issue.present? && original_issue != issuable + @updates[:original_issue_id] = original_issue.id + end + end + def extract_users(params) return [] if params.nil? diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index da0f21d449a..2e5e904c43d 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -552,6 +552,25 @@ module SystemNoteService create_note(NoteSummary.new(noteable, project, author, body, action: 'moved')) end + # Called when a Notable has been marked as a duplicate of another Issue + # + # noteable - Noteable object + # project - Project owning noteable + # author - User performing the change + # original_issue - Issue that this is a duplicate of + # + # Example Note text: + # + # "marked this issue as a duplicate of #1234" + # + # "marked this issue as a duplicate of other_project#5678" + # + # Returns the created Note object + def mark_duplicate_issue(noteable, project, author, original_issue) + body = "marked this issue as a duplicate of #{original_issue.to_reference(project)}" + create_note(NoteSummary.new(noteable, project, author, body, action: 'duplicate')) + end + private def notes_for_mentioner(mentioner, noteable, notes) -- cgit v1.2.1 From 7e3d34595c3e090fe505b4fbd49cde2a303b1b6f Mon Sep 17 00:00:00 2001 From: Ryan Scott Date: Wed, 5 Apr 2017 11:31:48 +0900 Subject: Changes based on MR feedback. Marking an issue as a duplicate will now also add an upvote on behalf of the author on the original issue. --- app/models/system_note_metadata.rb | 5 +++-- app/services/issuable_base_service.rb | 20 ++++++++++---------- app/services/system_note_service.rb | 8 ++++---- 3 files changed, 17 insertions(+), 16 deletions(-) (limited to 'app') diff --git a/app/models/system_note_metadata.rb b/app/models/system_note_metadata.rb index 1ffdd285b91..0b33e45473b 100644 --- a/app/models/system_note_metadata.rb +++ b/app/models/system_note_metadata.rb @@ -1,8 +1,9 @@ class SystemNoteMetadata < ActiveRecord::Base ICON_TYPES = %w[ commit description merge confidential visible label assignee cross_reference - title time_tracking branch milestone discussion task moved opened closed merged - outdated duplicate + title time_tracking branch milestone discussion task moved + opened closed merged duplicate + outdated ].freeze validates :note, presence: true diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index c7e646222bb..f57fbaca836 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -50,10 +50,6 @@ class IssuableBaseService < BaseService SystemNoteService.mark_duplicate_issue(issuable, issuable.project, current_user, original_issue) end - def create_cross_reference_note(noteable, mentioner) - SystemNoteService.cross_reference(noteable, mentioner, current_user) - end - def filter_params(issuable) ability_name = :"admin_#{issuable.to_ability_name}" @@ -303,14 +299,18 @@ class IssuableBaseService < BaseService def change_issue_duplicate(issuable) original_issue_id = params.delete(:original_issue_id) - return if original_issue_id.nil? + return unless original_issue_id - original_issue = IssuesFinder.new(current_user).find(original_issue_id) - if original_issue.present? - create_issue_duplicate_note(issuable, original_issue) - close_service.new(project, current_user, {}).execute(issuable) - create_cross_reference_note(original_issue, issuable) + begin + original_issue = IssuesFinder.new(current_user).find(original_issue_id) + rescue ActiveRecord::RecordNotFound + return end + + note = create_issue_duplicate_note(issuable, original_issue) + note.create_cross_references! + close_service.new(project, current_user, {}).execute(issuable) + original_issue.create_award_emoji(AwardEmoji::UPVOTE_NAME, issuable.author) end def toggle_award(issuable) diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index 2e5e904c43d..ed079f0e495 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -552,11 +552,11 @@ module SystemNoteService create_note(NoteSummary.new(noteable, project, author, body, action: 'moved')) end - # Called when a Notable has been marked as a duplicate of another Issue + # Called when a Noteable has been marked as a duplicate of another Issue # - # noteable - Noteable object - # project - Project owning noteable - # author - User performing the change + # noteable - Noteable object + # project - Project owning noteable + # author - User performing the change # original_issue - Issue that this is a duplicate of # # Example Note text: -- cgit v1.2.1 From 8a444484345806dcbc0312d770b185edde1edb67 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 21 Jul 2017 17:49:32 +0800 Subject: Extract validations --- app/services/ci/create_pipeline_service.rb | 48 +++++++++++++++--------------- 1 file changed, 24 insertions(+), 24 deletions(-) (limited to 'app') diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 3ff698b6437..21e2ef153de 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -15,12 +15,34 @@ module Ci pipeline_schedule: schedule ) + result = validate(current_user || trigger_request.trigger.owner, + ignore_skip_ci: ignore_skip_ci, + save_on_errors: save_on_errors) + + return result if result + + Ci::Pipeline.transaction do + update_merge_requests_head_pipeline if pipeline.save + + Ci::CreatePipelineStagesService + .new(project, current_user) + .execute(pipeline) + end + + cancel_pending_pipelines if project.auto_cancel_pending_pipelines? + + pipeline_created_counter.increment(source: source) + + pipeline.tap(&:process!) + end + + private + + def validate(triggering_user, ignore_skip_ci:, save_on_errors:) unless project.builds_enabled? return error('Pipeline is disabled') end - triggering_user = current_user || trigger_request.trigger.owner - unless allowed_to_trigger_pipeline?(triggering_user) if can?(triggering_user, :create_pipeline, project) return error("Insufficient permissions for protected ref '#{ref}'") @@ -52,28 +74,6 @@ module Ci unless pipeline.has_stage_seeds? return error('No stages / jobs for this pipeline.') end - - process! do - pipeline_created_counter.increment(source: source) - end - end - - private - - def process! - Ci::Pipeline.transaction do - update_merge_requests_head_pipeline if pipeline.save - - Ci::CreatePipelineStagesService - .new(project, current_user) - .execute(pipeline) - end - - cancel_pending_pipelines if project.auto_cancel_pending_pipelines? - - yield - - pipeline.tap(&:process!) end def allowed_to_trigger_pipeline?(triggering_user) -- cgit v1.2.1 From 53c5b6717ccfb4c9bc1f4faf008d084dd4f0cd96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Fri, 21 Jul 2017 12:43:04 +0200 Subject: Fix translations for Star/Unstar in JS file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- app/assets/javascripts/star.js | 6 ++++-- app/views/projects/buttons/_star.html.haml | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) (limited to 'app') diff --git a/app/assets/javascripts/star.js b/app/assets/javascripts/star.js index 6d38124f1c1..3a06b477d7c 100644 --- a/app/assets/javascripts/star.js +++ b/app/assets/javascripts/star.js @@ -1,6 +1,8 @@ /* eslint-disable func-names, space-before-function-paren, wrap-iife, no-unused-vars, one-var, no-var, one-var-declaration-per-line, prefer-arrow-callback, no-new, max-len */ /* global Flash */ +import { __, s__ } from './locale'; + export default class Star { constructor() { $('.project-home-panel .toggle-star').on('ajax:success', function(e, data, status, xhr) { @@ -11,10 +13,10 @@ export default class Star { toggleStar = function(isStarred) { $this.parent().find('.star-count').text(data.star_count); if (isStarred) { - $starSpan.removeClass('starred').text('Star'); + $starSpan.removeClass('starred').text(s__('StarProject|Star')); $starIcon.removeClass('fa-star').addClass('fa-star-o'); } else { - $starSpan.addClass('starred').text('Unstar'); + $starSpan.addClass('starred').text(__('Unstar')); $starIcon.removeClass('fa-star-o').addClass('fa-star'); } }; diff --git a/app/views/projects/buttons/_star.html.haml b/app/views/projects/buttons/_star.html.haml index e248676be0d..c82ae35a685 100644 --- a/app/views/projects/buttons/_star.html.haml +++ b/app/views/projects/buttons/_star.html.haml @@ -2,7 +2,7 @@ = link_to toggle_star_project_path(@project), { class: 'btn star-btn toggle-star', method: :post, remote: true } do - if current_user.starred?(@project) = icon('star') - %span.starred= _('Unstar') + %span.starred= _('Unstar') - else = icon('star-o') %span= s_('StarProject|Star') -- cgit v1.2.1 From eaa935d77b824510a141ab10e9471107c516f902 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 21 Jul 2017 13:09:13 +0200 Subject: Fix target project merge request link on build page --- app/serializers/build_details_entity.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'app') diff --git a/app/serializers/build_details_entity.rb b/app/serializers/build_details_entity.rb index 8ad5af1987c..743a08acefe 100644 --- a/app/serializers/build_details_entity.rb +++ b/app/serializers/build_details_entity.rb @@ -16,7 +16,8 @@ class BuildDetailsEntity < JobEntity end expose :path do |build| - project_merge_request_path(build.project, build.merge_request) + project_merge_request_path(build.merge_request.project, + build.merge_request) end end -- cgit v1.2.1 From 1df696f5a6836e03a6bf8d5139c2c7ce6d96e727 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Thu, 20 Jul 2017 15:42:33 +0100 Subject: Move duplicate issue management to a service --- app/helpers/system_note_helper.rb | 3 ++- app/services/issuable_base_service.rb | 23 +----------------- app/services/issues/base_service.rb | 8 +++++++ app/services/issues/duplicate_service.rb | 24 +++++++++++++++++++ app/services/issues/update_service.rb | 18 +++++++------- app/services/quick_actions/interpret_service.rb | 10 +++++--- app/services/system_note_service.rb | 31 ++++++++++++++++++++----- app/views/shared/icons/_icon_clone.svg | 3 +++ 8 files changed, 80 insertions(+), 40 deletions(-) create mode 100644 app/services/issues/duplicate_service.rb create mode 100644 app/views/shared/icons/_icon_clone.svg (limited to 'app') diff --git a/app/helpers/system_note_helper.rb b/app/helpers/system_note_helper.rb index 209bd56b78a..08fd97cd048 100644 --- a/app/helpers/system_note_helper.rb +++ b/app/helpers/system_note_helper.rb @@ -18,7 +18,8 @@ module SystemNoteHelper 'milestone' => 'icon_clock_o', 'discussion' => 'icon_comment_o', 'moved' => 'icon_arrow_circle_o_right', - 'outdated' => 'icon_edit' + 'outdated' => 'icon_edit', + 'duplicate' => 'icon_clone' }.freeze def icon_for_system_note(note) diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index f57fbaca836..ea497729115 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -46,10 +46,6 @@ class IssuableBaseService < BaseService SystemNoteService.change_time_spent(issuable, issuable.project, current_user) end - def create_issue_duplicate_note(issuable, original_issue) - SystemNoteService.mark_duplicate_issue(issuable, issuable.project, current_user, original_issue) - end - def filter_params(issuable) ability_name = :"admin_#{issuable.to_ability_name}" @@ -62,7 +58,7 @@ class IssuableBaseService < BaseService params.delete(:assignee_ids) params.delete(:assignee_id) params.delete(:due_date) - params.delete(:original_issue_id) + params.delete(:canonical_issue_id) end filter_assignee(issuable) @@ -214,7 +210,6 @@ class IssuableBaseService < BaseService change_state(issuable) change_subscription(issuable) change_todo(issuable) - change_issue_duplicate(issuable) toggle_award(issuable) filter_params(issuable) old_labels = issuable.labels.to_a @@ -297,22 +292,6 @@ class IssuableBaseService < BaseService end end - def change_issue_duplicate(issuable) - original_issue_id = params.delete(:original_issue_id) - return unless original_issue_id - - begin - original_issue = IssuesFinder.new(current_user).find(original_issue_id) - rescue ActiveRecord::RecordNotFound - return - end - - note = create_issue_duplicate_note(issuable, original_issue) - note.create_cross_references! - close_service.new(project, current_user, {}).execute(issuable) - original_issue.create_award_emoji(AwardEmoji::UPVOTE_NAME, issuable.author) - end - def toggle_award(issuable) award = params.delete(:emoji_award) if award diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index 34199eb5d13..4c198fc96ea 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -7,6 +7,14 @@ module Issues issue_data end + def reopen_service + Issues::ReopenService + end + + def close_service + Issues::CloseService + end + private def create_assignee_note(issue, old_assignees) diff --git a/app/services/issues/duplicate_service.rb b/app/services/issues/duplicate_service.rb new file mode 100644 index 00000000000..5c0854e664d --- /dev/null +++ b/app/services/issues/duplicate_service.rb @@ -0,0 +1,24 @@ +module Issues + class DuplicateService < Issues::BaseService + def execute(duplicate_issue, canonical_issue) + return if canonical_issue == duplicate_issue + return unless can?(current_user, :update_issue, duplicate_issue) + return unless can?(current_user, :create_note, canonical_issue) + + create_issue_duplicate_note(duplicate_issue, canonical_issue) + create_issue_canonical_note(canonical_issue, duplicate_issue) + + close_service.new(project, current_user, {}).execute(duplicate_issue) + end + + private + + def create_issue_duplicate_note(duplicate_issue, canonical_issue) + SystemNoteService.mark_duplicate_issue(duplicate_issue, duplicate_issue.project, current_user, canonical_issue) + end + + def create_issue_canonical_note(canonical_issue, duplicate_issue) + SystemNoteService.mark_canonical_issue_of_duplicate(canonical_issue, canonical_issue.project, current_user, duplicate_issue) + end + end +end diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index cd9f9a4a16e..8d918ccc635 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -5,6 +5,7 @@ module Issues def execute(issue) handle_move_between_iids(issue) filter_spam_check_params + change_issue_duplicate(issue) update(issue) end @@ -53,14 +54,6 @@ module Issues end end - def reopen_service - Issues::ReopenService - end - - def close_service - Issues::CloseService - end - def handle_move_between_iids(issue) return unless params[:move_between_iids] @@ -72,6 +65,15 @@ module Issues issue.move_between(issue_before, issue_after) end + def change_issue_duplicate(issue) + canonical_issue_id = params.delete(:canonical_issue_id) + canonical_issue = IssuesFinder.new(current_user).find_by(id: canonical_issue_id) + + if canonical_issue + Issues::DuplicateService.new(project, current_user).execute(issue, canonical_issue) + end + end + private def get_issue_if_allowed(project, iid) diff --git a/app/services/quick_actions/interpret_service.rb b/app/services/quick_actions/interpret_service.rb index 3eecf0b5545..5dc1b91d2c0 100644 --- a/app/services/quick_actions/interpret_service.rb +++ b/app/services/quick_actions/interpret_service.rb @@ -472,6 +472,9 @@ module QuickActions end desc 'Mark this issue as a duplicate of another issue' + explanation do |duplicate_reference| + "Marks this issue as a duplicate of #{duplicate_reference}." + end params '#issue' condition do issuable.is_a?(Issue) && @@ -479,9 +482,10 @@ module QuickActions current_user.can?(:"update_#{issuable.to_ability_name}", issuable) end command :duplicate do |duplicate_param| - original_issue = extract_references(duplicate_param, :issue).first - if original_issue.present? && original_issue != issuable - @updates[:original_issue_id] = original_issue.id + canonical_issue = extract_references(duplicate_param, :issue).first + + if canonical_issue.present? + @updates[:canonical_issue_id] = canonical_issue.id end end diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index ed079f0e495..2dbee9c246e 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -554,10 +554,10 @@ module SystemNoteService # Called when a Noteable has been marked as a duplicate of another Issue # - # noteable - Noteable object - # project - Project owning noteable - # author - User performing the change - # original_issue - Issue that this is a duplicate of + # noteable - Noteable object + # project - Project owning noteable + # author - User performing the change + # canonical_issue - Issue that this is a duplicate of # # Example Note text: # @@ -566,8 +566,27 @@ module SystemNoteService # "marked this issue as a duplicate of other_project#5678" # # Returns the created Note object - def mark_duplicate_issue(noteable, project, author, original_issue) - body = "marked this issue as a duplicate of #{original_issue.to_reference(project)}" + def mark_duplicate_issue(noteable, project, author, canonical_issue) + body = "marked this issue as a duplicate of #{canonical_issue.to_reference(project)}" + create_note(NoteSummary.new(noteable, project, author, body, action: 'duplicate')) + end + + # Called when a Noteable has been marked as the canonical Issue of a duplicate + # + # noteable - Noteable object + # project - Project owning noteable + # author - User performing the change + # duplicate_issue - Issue that was a duplicate of this + # + # Example Note text: + # + # "marked #1234 as a duplicate of this issue" + # + # "marked other_project#5678 as a duplicate of this issue" + # + # Returns the created Note object + def mark_canonical_issue_of_duplicate(noteable, project, author, duplicate_issue) + body = "marked #{duplicate_issue.to_reference(project)} as a duplicate of this issue" create_note(NoteSummary.new(noteable, project, author, body, action: 'duplicate')) end diff --git a/app/views/shared/icons/_icon_clone.svg b/app/views/shared/icons/_icon_clone.svg new file mode 100644 index 00000000000..ccc897aa98f --- /dev/null +++ b/app/views/shared/icons/_icon_clone.svg @@ -0,0 +1,3 @@ + + + -- cgit v1.2.1 From e4391c7190fcebd37e49db447b22b1081dca9741 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Fri, 21 Jul 2017 18:45:12 +0100 Subject: Backport changes from https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2328 --- app/controllers/admin/application_settings_controller.rb | 4 ++-- app/controllers/projects/application_controller.rb | 1 + app/controllers/projects_controller.rb | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) (limited to 'app') diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index c1bc4c0d675..4c0f7556894 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -76,11 +76,11 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController params.delete(:domain_blacklist_raw) if params[:domain_blacklist_file] params.require(:application_setting).permit( - application_setting_params_ce + application_setting_params_attributes ) end - def application_setting_params_ce + def application_setting_params_attributes [ :admin_notification_email, :after_sign_out_path, diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index 95de3a44641..221e01b415a 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -22,6 +22,7 @@ class Projects::ApplicationController < ApplicationController def project return @project if @project + return nil unless params[:project_id] || params[:id] path = File.join(params[:namespace_id], params[:project_id] || params[:id]) auth_proc = ->(project) { !project.pending_delete? } diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index c769693255c..2d7cbd4614e 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -296,10 +296,10 @@ class ProjectsController < Projects::ApplicationController def project_params params.require(:project) - .permit(project_params_ce) + .permit(project_params_attributes) end - def project_params_ce + def project_params_attributes [ :avatar, :build_allow_git_fetch, -- cgit v1.2.1 From 9408ed7f5a3750dcf589c071a008afce9af56dc6 Mon Sep 17 00:00:00 2001 From: Simon Knox Date: Mon, 17 Jul 2017 15:05:22 +1000 Subject: fix resize bug for title and collapsible nav menus --- app/assets/javascripts/main.js | 8 +------- app/assets/stylesheets/framework/header.scss | 29 ++++++++++++++++++++++++++++ app/views/layouts/header/_new.html.haml | 2 +- 3 files changed, 31 insertions(+), 8 deletions(-) (limited to 'app') diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index 26c67fb721c..5704625ed2b 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -284,13 +284,7 @@ $(function () { return $container.remove(); // Commit show suppressed diff }); - $('.navbar-toggle').on('click', function () { - $('.header-content .title, .header-content .navbar-sub-nav').toggle(); - $('.header-content .header-logo').toggle(); - $('.header-content .navbar-collapse').toggle(); - $('.js-navbar-toggle-left, .js-navbar-toggle-right, .title-container').toggle(); - return $('.navbar-toggle').toggleClass('active'); - }); + $('.navbar-toggle').on('click', () => $('.header-content').toggleClass('menu-expanded')); // Show/hide comments on diff $body.on('click', '.js-toggle-diff-comments', function (e) { var $this = $(this); diff --git a/app/assets/stylesheets/framework/header.scss b/app/assets/stylesheets/framework/header.scss index 20fb10c28d4..605f4284bb5 100644 --- a/app/assets/stylesheets/framework/header.scss +++ b/app/assets/stylesheets/framework/header.scss @@ -132,6 +132,22 @@ header { } } + &.navbar-gitlab-new { + .fa-times { + display: none; + } + + .menu-expanded { + .fa-ellipsis-v { + display: none; + } + + .fa-times { + display: block; + } + } + } + .global-dropdown { position: absolute; left: -10px; @@ -171,6 +187,19 @@ header { min-height: $header-height; padding-left: 30px; + &.menu-expanded { + @media (max-width: $screen-xs-max) { + .header-logo, + .title-container { + display: none; + } + + .navbar-collapse { + display: block; + } + } + } + .dropdown-menu { margin-top: -5px; } diff --git a/app/views/layouts/header/_new.html.haml b/app/views/layouts/header/_new.html.haml index 4697d91724b..c0d35c73063 100644 --- a/app/views/layouts/header/_new.html.haml +++ b/app/views/layouts/header/_new.html.haml @@ -81,6 +81,6 @@ %button.navbar-toggle.hidden-sm.hidden-md.hidden-lg{ type: 'button' } %span.sr-only Toggle navigation = icon('ellipsis-v', class: 'js-navbar-toggle-right') - = icon('times', class: 'js-navbar-toggle-left', style: 'display: none;') + = icon('times', class: 'js-navbar-toggle-left') = render 'shared/outdated_browser' -- cgit v1.2.1 From 2fa22a07296223c1239bfab94654487cca222097 Mon Sep 17 00:00:00 2001 From: Jarka Kadlecova Date: Tue, 13 Jun 2017 10:25:25 +0200 Subject: Associate Issues tab only with internal issues tracker --- app/controllers/projects/issues_controller.rb | 13 ++++++++----- app/policies/project_policy.rb | 3 --- 2 files changed, 8 insertions(+), 8 deletions(-) (limited to 'app') diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 0ac9da2ff0f..5b0eeac2477 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -8,7 +8,6 @@ class Projects::IssuesController < Projects::ApplicationController prepend_before_action :authenticate_user!, only: [:new] - before_action :redirect_to_external_issue_tracker, only: [:index, :new] before_action :check_issues_available! before_action :issue, except: [:index, :new, :create, :bulk_update] @@ -243,19 +242,19 @@ class Projects::IssuesController < Projects::ApplicationController end def authorize_update_issue! - return render_404 unless can?(current_user, :update_issue, @issue) + render_404 unless can?(current_user, :update_issue, @issue) end def authorize_admin_issues! - return render_404 unless can?(current_user, :admin_issue, @project) + render_404 unless can?(current_user, :admin_issue, @project) end def authorize_create_merge_request! - return render_404 unless can?(current_user, :push_code, @project) && @issue.can_be_worked_on?(current_user) + render_404 unless can?(current_user, :push_code, @project) && @issue.can_be_worked_on?(current_user) end def check_issues_available! - return render_404 unless @project.feature_available?(:issues, current_user) && @project.default_issues_tracker? + return render_404 unless @project.feature_available?(:issues, current_user) end def redirect_to_external_issue_tracker @@ -270,6 +269,10 @@ class Projects::IssuesController < Projects::ApplicationController end end + def module_enabled + render_404 unless @project.feature_available?(:issues, current_user) + end + def issue_params params.require(:issue).permit(*issue_params_attributes) end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 323131c0f7e..d27bbf2948c 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -287,9 +287,6 @@ class ProjectPolicy < BasePolicy prevent :create_issue prevent :update_issue prevent :admin_issue - end - - rule { issues_disabled & default_issues_tracker }.policy do prevent :read_issue end -- cgit v1.2.1 From 7bee7b848aab883a6869e1fd2fbb9e66182d2023 Mon Sep 17 00:00:00 2001 From: Jarka Kadlecova Date: Mon, 10 Jul 2017 09:38:42 +0200 Subject: Support both internal and external issue trackers --- app/controllers/projects/issues_controller.rb | 4 ---- app/helpers/issues_helper.rb | 26 +++++++++++++++++++--- app/models/merge_request.rb | 2 +- app/models/project.rb | 10 +++++---- .../project_services/issue_tracker_service.rb | 8 +++++-- app/models/project_services/jira_service.rb | 2 +- app/services/issues/close_service.rb | 4 ++-- .../layouts/nav/_new_project_sidebar.html.haml | 6 ++--- app/views/layouts/nav/_project.html.haml | 6 ++--- app/views/projects/merge_requests/index.html.haml | 2 +- app/views/shared/_mr_head.html.haml | 2 +- 11 files changed, 47 insertions(+), 25 deletions(-) (limited to 'app') diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 5b0eeac2477..e2ccabb22db 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -269,10 +269,6 @@ class Projects::IssuesController < Projects::ApplicationController end end - def module_enabled - render_404 unless @project.feature_available?(:issues, current_user) - end - def issue_params params.require(:issue).permit(*issue_params_attributes) end diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 42b6cfdf02f..7e1ccb23e9e 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -17,10 +17,10 @@ module IssuesHelper return '' if project.nil? url = - if options[:only_path] - project.issues_tracker.issue_path(issue_iid) + if options[:internal] + url_for_internal_issue(issue_iid, project, options) else - project.issues_tracker.issue_url(issue_iid) + url_for_tracker_issue(issue_iid, project, options) end # Ensure we return a valid URL to prevent possible XSS. @@ -29,6 +29,24 @@ module IssuesHelper '' end + def url_for_tracker_issue(issue_iid, project, options) + if options[:only_path] + project.issues_tracker.issue_path(issue_iid) + else + project.issues_tracker.issue_url(issue_iid) + end + end + + def url_for_internal_issue(issue_iid, project = @project, options = {}) + helpers = Gitlab::Routing.url_helpers + + if options[:only_path] + helpers.namespace_project_issue_path(namespace_id: project.namespace, project_id: project, id: issue_iid) + else + helpers.namespace_project_issue_url(namespace_id: project.namespace, project_id: project, id: issue_iid) + end + end + def bulk_update_milestone_options milestones = @project.milestones.active.reorder(due_date: :asc, title: :asc).to_a milestones.unshift(Milestone::None) @@ -158,4 +176,6 @@ module IssuesHelper # Required for Banzai::Filter::IssueReferenceFilter module_function :url_for_issue + module_function :url_for_internal_issue + module_function :url_for_tracker_issue end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index e4e7999d0f2..a910099b4c1 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -596,7 +596,7 @@ class MergeRequest < ActiveRecord::Base # running `ReferenceExtractor` on each of them separately. # This optimization does not apply to issues from external sources. def cache_merge_request_closes_issues!(current_user) - return if project.has_external_issue_tracker? + return unless project.issues_enabled? transaction do self.merge_requests_closing_issues.delete_all diff --git a/app/models/project.rb b/app/models/project.rb index 0b357d5d003..d827bfaa806 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -734,9 +734,11 @@ class Project < ActiveRecord::Base end def get_issue(issue_id, current_user) - if default_issues_tracker? - IssuesFinder.new(current_user, project_id: id).find_by(iid: issue_id) - else + issue = IssuesFinder.new(current_user, project_id: id).find_by(iid: issue_id) if issues_enabled? + + if issue + issue + elsif external_issue_tracker ExternalIssue.new(issue_id, self) end end @@ -758,7 +760,7 @@ class Project < ActiveRecord::Base end def external_issue_reference_pattern - external_issue_tracker.class.reference_pattern + external_issue_tracker.class.reference_pattern(only_long: issues_enabled?) end def default_issues_tracker? diff --git a/app/models/project_services/issue_tracker_service.rb b/app/models/project_services/issue_tracker_service.rb index 6d6a3ae3647..31984c5d7ed 100644 --- a/app/models/project_services/issue_tracker_service.rb +++ b/app/models/project_services/issue_tracker_service.rb @@ -8,8 +8,12 @@ class IssueTrackerService < Service # This pattern does not support cross-project references # The other code assumes that this pattern is a superset of all # overriden patterns. See ReferenceRegexes::EXTERNAL_PATTERN - def self.reference_pattern - @reference_pattern ||= %r{(\b[A-Z][A-Z0-9_]+-|#{Issue.reference_prefix})(?\d+)} + def self.reference_pattern(only_long: false) + if only_long + %r{(\b[A-Z][A-Z0-9_]+-)(?\d+)} + else + %r{(\b[A-Z][A-Z0-9_]+-|#{Issue.reference_prefix})(?\d+)} + end end def default? diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb index 5498a2e17b2..450027c2e57 100644 --- a/app/models/project_services/jira_service.rb +++ b/app/models/project_services/jira_service.rb @@ -18,7 +18,7 @@ class JiraService < IssueTrackerService end # {PROJECT-KEY}-{NUMBER} Examples: JIRA-1, PROJECT-1 - def self.reference_pattern + def self.reference_pattern(only_long: true) @reference_pattern ||= %r{(?\b([A-Z][A-Z0-9_]+-)\d+)} end diff --git a/app/services/issues/close_service.rb b/app/services/issues/close_service.rb index ddef5281498..74459c3342c 100644 --- a/app/services/issues/close_service.rb +++ b/app/services/issues/close_service.rb @@ -16,13 +16,13 @@ module Issues # 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 + if project.jira_tracker? && project.jira_service.active && issue.is_a?(ExternalIssue) project.jira_service.close_issue(commit, issue) todo_service.close_issue(issue, current_user) return issue end - if project.default_issues_tracker? && issue.close + if project.issues_enabled? && issue.close event_service.close_issue(issue, current_user) create_note(issue, commit) if system_note notification_service.close_issue(issue, current_user) if notifications diff --git a/app/views/layouts/nav/_new_project_sidebar.html.haml b/app/views/layouts/nav/_new_project_sidebar.html.haml index 21f175291fa..00395b222e4 100644 --- a/app/views/layouts/nav/_new_project_sidebar.html.haml +++ b/app/views/layouts/nav/_new_project_sidebar.html.haml @@ -75,10 +75,10 @@ Registry - if project_nav_tab? :issues - = nav_link(controller: @project.default_issues_tracker? ? [:issues, :labels, :milestones, :boards] : :issues) do + = nav_link(controller: @project.issues_enabled? ? [:issues, :labels, :milestones, :boards] : :issues) do = link_to project_issues_path(@project), title: 'Issues', class: 'shortcuts-issues' do %span - - if @project.default_issues_tracker? + - if @project.issues_enabled? %span.badge.count.issue_counter= number_with_delimiter(IssuesFinder.new(current_user, project_id: @project.id).execute.opened.count) Issues @@ -113,7 +113,7 @@ Milestones - if project_nav_tab? :merge_requests - = nav_link(controller: @project.default_issues_tracker? ? :merge_requests : [:merge_requests, :labels, :milestones]) do + = nav_link(controller: @project.issues_enabled? ? :merge_requests : [:merge_requests, :labels, :milestones]) do = link_to project_merge_requests_path(@project), title: 'Merge Requests', class: 'shortcuts-merge_requests' do %span %span.badge.count.merge_counter.js-merge-counter= number_with_delimiter(MergeRequestsFinder.new(current_user, project_id: @project.id).execute.opened.count) diff --git a/app/views/layouts/nav/_project.html.haml b/app/views/layouts/nav/_project.html.haml index fb90bb4b472..924cd2e9681 100644 --- a/app/views/layouts/nav/_project.html.haml +++ b/app/views/layouts/nav/_project.html.haml @@ -23,16 +23,16 @@ Registry - if project_nav_tab? :issues - = nav_link(controller: @project.default_issues_tracker? ? [:issues, :labels, :milestones, :boards] : :issues) do + = nav_link(controller: @project.issues_enabled? ? [:issues, :labels, :milestones, :boards] : :issues) do = link_to project_issues_path(@project), title: 'Issues', class: 'shortcuts-issues' do %span Issues - - if @project.default_issues_tracker? + - if @project.issues_enabled? %span.badge.count.issue_counter= number_with_delimiter(issuables_count_for_state(:issues, :opened, finder: IssuesFinder.new(current_user, project_id: @project.id))) - if project_nav_tab? :merge_requests - controllers = [:merge_requests, 'projects/merge_requests/conflicts'] - - controllers.push(:merge_requests, :labels, :milestones) unless @project.default_issues_tracker? + - controllers.push(:merge_requests, :labels, :milestones) unless @project.issues_enabled? = nav_link(controller: controllers) do = link_to project_merge_requests_path(@project), title: 'Merge Requests', class: 'shortcuts-merge_requests' do %span diff --git a/app/views/projects/merge_requests/index.html.haml b/app/views/projects/merge_requests/index.html.haml index bfeb746ee83..c020e7db380 100644 --- a/app/views/projects/merge_requests/index.html.haml +++ b/app/views/projects/merge_requests/index.html.haml @@ -4,7 +4,7 @@ - new_merge_request_path = project_new_merge_request_path(merge_project) if merge_project - page_title "Merge Requests" -- unless @project.default_issues_tracker? +- unless @project.issues_enabled? = content_for :sub_nav do = render "projects/merge_requests/head" diff --git a/app/views/shared/_mr_head.html.haml b/app/views/shared/_mr_head.html.haml index 4211ec6351d..e7355ae2eea 100644 --- a/app/views/shared/_mr_head.html.haml +++ b/app/views/shared/_mr_head.html.haml @@ -1,4 +1,4 @@ -- if @project.default_issues_tracker? +- if @project.issues_enabled? = render "projects/issues/head" - else = render "projects/merge_requests/head" -- cgit v1.2.1 From fee65beb70bb9f995fe701a9deb0fabdc7a0e142 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Mon, 24 Jul 2017 09:20:22 +0100 Subject: Fixed custom logo sizing in new navigation header Closes #35439 --- app/assets/stylesheets/new_nav.scss | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'app') diff --git a/app/assets/stylesheets/new_nav.scss b/app/assets/stylesheets/new_nav.scss index 9f3e278ebfc..360ffda8d71 100644 --- a/app/assets/stylesheets/new_nav.scss +++ b/app/assets/stylesheets/new_nav.scss @@ -21,6 +21,11 @@ header.navbar-gitlab-new { padding-right: 0; color: currentColor; + img { + height: 28px; + margin-right: 10px; + } + > a { display: flex; align-items: center; -- cgit v1.2.1 From 32c47aa348049a2714ab34f0040e516b7921b746 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Mon, 24 Jul 2017 08:55:39 +0100 Subject: Fixed duplicate new milestone buttons in new navigation Closes #35454 --- app/assets/stylesheets/framework/nav.scss | 6 ++++++ app/views/projects/milestones/index.html.haml | 6 +++--- 2 files changed, 9 insertions(+), 3 deletions(-) (limited to 'app') diff --git a/app/assets/stylesheets/framework/nav.scss b/app/assets/stylesheets/framework/nav.scss index 99eea97377c..35b4d77a5ab 100644 --- a/app/assets/stylesheets/framework/nav.scss +++ b/app/assets/stylesheets/framework/nav.scss @@ -182,6 +182,12 @@ } } + &.nav-controls-new-nav { + > .dropdown { + margin-right: 0; + } + } + > .btn-grouped { float: none; } diff --git a/app/views/projects/milestones/index.html.haml b/app/views/projects/milestones/index.html.haml index a89387bc8f1..e0b29b0c2e1 100644 --- a/app/views/projects/milestones/index.html.haml +++ b/app/views/projects/milestones/index.html.haml @@ -1,7 +1,7 @@ - @no_container = true - page_title 'Milestones' -- if show_new_nav? +- if show_new_nav? && can?(current_user, :admin_milestone, @project) - content_for :breadcrumbs_extra do = link_to "New milestone", new_namespace_project_milestone_path(@project.namespace, @project), class: 'btn btn-new', title: 'New milestone' @@ -11,10 +11,10 @@ .top-area = render 'shared/milestones_filter', counts: milestone_counts(@project.milestones) - .nav-controls + .nav-controls{ class: ("nav-controls-new-nav" if show_new_nav?) } = render 'shared/milestones_sort_dropdown' - if can?(current_user, :admin_milestone, @project) - = link_to new_project_milestone_path(@project), class: 'btn btn-new', title: 'New milestone' do + = link_to new_project_milestone_path(@project), class: "btn btn-new #{("visible-xs" if show_new_nav?)}", title: 'New milestone' do New milestone .milestones -- cgit v1.2.1 From 4ad8f12e44b81bb5a07a5365ec0fc5fdba19094e Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 24 Jul 2017 10:47:33 +0000 Subject: Fix editing project with container images present --- app/services/projects/update_service.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'app') diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index 30ca95eef7a..d81035e4eba 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -5,7 +5,7 @@ module Projects return error('New visibility level not allowed!') end - if project.has_container_registry_tags? + if renaming_project_with_container_registry_tags? return error('Cannot rename project because it contains container registry tags!') end @@ -44,6 +44,13 @@ module Projects true end + def renaming_project_with_container_registry_tags? + new_path = params[:path] + + new_path && new_path != project.path && + project.has_container_registry_tags? + end + def changing_default_branch? new_branch = params[:default_branch] -- cgit v1.2.1 From fab1b0f1d189094ade6be5c35f03a53eeff99872 Mon Sep 17 00:00:00 2001 From: Maxim Rydkin Date: Mon, 24 Jul 2017 10:54:16 +0000 Subject: Decrease ABC threshold to 56.96 --- app/controllers/admin/projects_controller.rb | 15 +++---------- app/finders/admin/projects_finder.rb | 33 ++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 12 deletions(-) create mode 100644 app/finders/admin/projects_finder.rb (limited to 'app') diff --git a/app/controllers/admin/projects_controller.rb b/app/controllers/admin/projects_controller.rb index 984d5398708..0b6cd71e651 100644 --- a/app/controllers/admin/projects_controller.rb +++ b/app/controllers/admin/projects_controller.rb @@ -3,18 +3,9 @@ class Admin::ProjectsController < Admin::ApplicationController before_action :group, only: [:show, :transfer] def index - params[:sort] ||= 'latest_activity_desc' - @projects = Project.with_statistics - @projects = @projects.in_namespace(params[:namespace_id]) if params[:namespace_id].present? - @projects = @projects.where(visibility_level: params[:visibility_level]) if params[:visibility_level].present? - @projects = @projects.with_push if params[:with_push].present? - @projects = @projects.abandoned if params[:abandoned].present? - @projects = @projects.where(last_repository_check_failed: true) if params[:last_repository_check_failed].present? - @projects = @projects.non_archived unless params[:archived].present? - @projects = @projects.personal(current_user) if params[:personal].present? - @projects = @projects.search(params[:name]) if params[:name].present? - @projects = @projects.sort(@sort = params[:sort]) - @projects = @projects.includes(:namespace).order("namespaces.path, projects.name ASC").page(params[:page]) + finder = Admin::ProjectsFinder.new(params: params, current_user: current_user) + @projects = finder.execute + @sort = finder.sort respond_to do |format| format.html diff --git a/app/finders/admin/projects_finder.rb b/app/finders/admin/projects_finder.rb new file mode 100644 index 00000000000..a5ba791a513 --- /dev/null +++ b/app/finders/admin/projects_finder.rb @@ -0,0 +1,33 @@ +class Admin::ProjectsFinder + attr_reader :sort, :namespace_id, :visibility_level, :with_push, + :abandoned, :last_repository_check_failed, :archived, + :personal, :name, :page, :current_user + + def initialize(params:, current_user:) + @current_user = current_user + @sort = params.fetch(:sort) { 'latest_activity_desc' } + @namespace_id = params[:namespace_id] + @visibility_level = params[:visibility_level] + @with_push = params[:with_push] + @abandoned = params[:abandoned] + @last_repository_check_failed = params[:last_repository_check_failed] + @archived = params[:archived] + @personal = params[:personal] + @name = params[:name] + @page = params[:page] + end + + def execute + items = Project.with_statistics + items = items.in_namespace(namespace_id) if namespace_id.present? + items = items.where(visibility_level: visibility_level) if visibility_level.present? + items = items.with_push if with_push.present? + items = items.abandoned if abandoned.present? + items = items.where(last_repository_check_failed: true) if last_repository_check_failed.present? + items = items.non_archived unless archived.present? + items = items.personal(current_user) if personal.present? + items = items.search(name) if name.present? + items = items.sort(sort) + items.includes(:namespace).order("namespaces.path, projects.name ASC").page(page) + end +end -- cgit v1.2.1 From ff3d3ccb3ff00b7801c8a506453ca3f9d8ccf2ec Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 24 Jul 2017 15:45:56 +0300 Subject: Fix today day highlight in calendar Signed-off-by: Dmitriy Zaporozhets --- app/assets/stylesheets/framework/calendar.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app') diff --git a/app/assets/stylesheets/framework/calendar.scss b/app/assets/stylesheets/framework/calendar.scss index 759401a7806..0ac095f7d8f 100644 --- a/app/assets/stylesheets/framework/calendar.scss +++ b/app/assets/stylesheets/framework/calendar.scss @@ -93,7 +93,7 @@ .is-selected .pika-day, .pika-day:hover, - .is-today .pika-day:hover { + .is-today .pika-day { background: $gl-primary; color: $white-light; box-shadow: none; -- cgit v1.2.1 From a8b33d7b5db99f47000316a8dc167106214ca4f8 Mon Sep 17 00:00:00 2001 From: Emilien Mottet Date: Mon, 24 Jul 2017 17:04:54 +0200 Subject: fix conflict pluralized --- app/assets/javascripts/merge_conflicts/merge_conflict_store.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app') diff --git a/app/assets/javascripts/merge_conflicts/merge_conflict_store.js b/app/assets/javascripts/merge_conflicts/merge_conflict_store.js index c4e379a4a0b..8be7314ded8 100644 --- a/app/assets/javascripts/merge_conflicts/merge_conflict_store.js +++ b/app/assets/javascripts/merge_conflicts/merge_conflict_store.js @@ -175,7 +175,7 @@ import Cookies from 'js-cookie'; getConflictsCountText() { const count = this.getConflictsCount(); - const text = count ? 'conflicts' : 'conflict'; + const text = count > 1 ? 'conflicts' : 'conflict'; return `${count} ${text}`; }, -- cgit v1.2.1 From fef5a4fddd6ba0d152c553ab2b1667497400c062 Mon Sep 17 00:00:00 2001 From: Tim Zallmann Date: Mon, 24 Jul 2017 17:21:05 +0200 Subject: How to Merge to external File --- app/assets/javascripts/how_to_merge.js | 12 ++++++++++++ app/views/projects/merge_requests/_how_to_merge.html.haml | 14 +++----------- 2 files changed, 15 insertions(+), 11 deletions(-) create mode 100644 app/assets/javascripts/how_to_merge.js (limited to 'app') diff --git a/app/assets/javascripts/how_to_merge.js b/app/assets/javascripts/how_to_merge.js new file mode 100644 index 00000000000..f739db751a6 --- /dev/null +++ b/app/assets/javascripts/how_to_merge.js @@ -0,0 +1,12 @@ +document.addEventListener('DOMContentLoaded', () => { + const modal = $('#modal_merge_info').modal({ + modal: true, + show: false, + }); + $('.how_to_merge_link').bind('click', () => { + modal.show(); + }); + $('.modal-header .close').bind('click', () => { + modal.hide(); + }); +}); diff --git a/app/views/projects/merge_requests/_how_to_merge.html.haml b/app/views/projects/merge_requests/_how_to_merge.html.haml index 766cb272bec..917ec7fdbda 100644 --- a/app/views/projects/merge_requests/_how_to_merge.html.haml +++ b/app/views/projects/merge_requests/_how_to_merge.html.haml @@ -1,3 +1,6 @@ +- content_for :page_specific_javascripts do + = webpack_bundle_tag('how_to_merge') + #modal_merge_info.modal .modal-dialog .modal-content @@ -50,14 +53,3 @@ = succeed '.' do You can also checkout merge requests locally by = link_to 'following these guidelines', help_page_path('user/project/merge_requests/index.md', anchor: "checkout-merge-requests-locally"), target: '_blank', rel: 'noopener noreferrer' - -:javascript - $(function(){ - var modal = $('#modal_merge_info').modal({modal: true, show:false}); - $('.how_to_merge_link').bind("click", function(){ - modal.show(); - }); - $('.modal-header .close').bind("click", function(){ - modal.hide(); - }) - }) -- cgit v1.2.1 From ccac2abeba419f16029c40f29063f1812c9e159c Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Mon, 24 Jul 2017 11:35:54 +0100 Subject: Don't treat anonymous users as owners when group has pending invites The `members` table can have entries where `user_id: nil`, because people can invite group members by email. We never want to include those as members, because it might cause confusion with the anonymous (logged out) user. --- app/models/group.rb | 6 +++++- app/policies/project_policy.rb | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) (limited to 'app') diff --git a/app/models/group.rb b/app/models/group.rb index dfa4e8adedd..bd5735ed82e 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -167,10 +167,14 @@ class Group < Namespace end def has_owner?(user) + return false unless user + members_with_parents.owners.where(user_id: user).any? end def has_master?(user) + return false unless user + members_with_parents.masters.where(user_id: user).any? end @@ -212,7 +216,7 @@ class Group < Namespace end def members_with_parents - GroupMember.non_request.where(source_id: ancestors.pluck(:id).push(id)) + GroupMember.active.where(source_id: ancestors.pluck(:id).push(id)).where.not(user_id: nil) end def users_with_parents diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index d27bbf2948c..0133091db57 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -10,7 +10,8 @@ class ProjectPolicy < BasePolicy desc "User is a project owner" condition :owner do - @user && project.owner == @user || (project.group && project.group.has_owner?(@user)) + (project.owner.present? && project.owner == @user) || + project.group&.has_owner?(@user) end desc "Project has public builds enabled" -- cgit v1.2.1 From 52b8a0db689c2df968776a1f369ea6a6db245d39 Mon Sep 17 00:00:00 2001 From: Tim Zallmann Date: Mon, 24 Jul 2017 17:36:52 +0000 Subject: Resolve "Lazy load images on the Frontend" --- app/assets/javascripts/copy_as_gfm.js | 10 ++- app/assets/javascripts/lazy_loader.js | 76 +++++++++++++++++++++++ app/assets/javascripts/main.js | 6 ++ app/assets/stylesheets/framework/avatar.scss | 2 + app/assets/stylesheets/framework/typography.scss | 11 +++- app/assets/stylesheets/framework/variables.scss | 4 +- app/helpers/avatars_helper.rb | 9 +-- app/helpers/emails_helper.rb | 4 +- app/helpers/lazy_image_tag_helper.rb | 24 +++++++ app/helpers/version_check_helper.rb | 2 +- app/models/concerns/cache_markdown_field.rb | 2 +- app/views/projects/blob/viewers/_image.html.haml | 2 +- app/views/projects/diffs/viewers/_image.html.haml | 14 ++--- 13 files changed, 144 insertions(+), 22 deletions(-) create mode 100644 app/assets/javascripts/lazy_loader.js create mode 100644 app/helpers/lazy_image_tag_helper.rb (limited to 'app') diff --git a/app/assets/javascripts/copy_as_gfm.js b/app/assets/javascripts/copy_as_gfm.js index ba9d9a3e1f7..54257531284 100644 --- a/app/assets/javascripts/copy_as_gfm.js +++ b/app/assets/javascripts/copy_as_gfm.js @@ -1,6 +1,7 @@ /* eslint-disable class-methods-use-this, object-shorthand, no-unused-vars, no-use-before-define, no-new, max-len, no-restricted-syntax, guard-for-in, no-continue */ import './lib/utils/common_utils'; +import { placeholderImage } from './lazy_loader'; const gfmRules = { // The filters referenced in lib/banzai/pipeline/gfm_pipeline.rb convert @@ -56,6 +57,11 @@ const gfmRules = { return text; }, }, + ImageLazyLoadFilter: { + 'img'(el, text) { + return `![${el.getAttribute('alt')}](${el.getAttribute('src')})`; + }, + }, VideoLinkFilter: { '.video-container'(el) { const videoEl = el.querySelector('video'); @@ -163,7 +169,9 @@ const gfmRules = { return text.trim().split('\n').map(s => `> ${s}`.trim()).join('\n'); }, 'img'(el) { - return `![${el.getAttribute('alt')}](${el.getAttribute('src')})`; + const imageSrc = el.src; + const imageUrl = imageSrc && imageSrc !== placeholderImage ? imageSrc : (el.dataset.src || ''); + return `![${el.getAttribute('alt')}](${imageUrl})`; }, 'a.anchor'(el, text) { // Don't render a Markdown link for the anchor link inside a heading diff --git a/app/assets/javascripts/lazy_loader.js b/app/assets/javascripts/lazy_loader.js new file mode 100644 index 00000000000..3d64b121fa7 --- /dev/null +++ b/app/assets/javascripts/lazy_loader.js @@ -0,0 +1,76 @@ +/* eslint-disable one-export, one-var, one-var-declaration-per-line */ + +import _ from 'underscore'; + +export const placeholderImage = 'data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw=='; +const SCROLL_THRESHOLD = 300; + +export default class LazyLoader { + constructor(options = {}) { + this.lazyImages = []; + this.observerNode = options.observerNode || '#content-body'; + + const throttledScrollCheck = _.throttle(() => this.scrollCheck(), 300); + const debouncedElementsInView = _.debounce(() => this.checkElementsInView(), 300); + + window.addEventListener('scroll', throttledScrollCheck); + window.addEventListener('resize', debouncedElementsInView); + + const scrollContainer = options.scrollContainer || window; + scrollContainer.addEventListener('load', () => this.loadCheck()); + } + searchLazyImages() { + this.lazyImages = [].slice.call(document.querySelectorAll('.lazy')); + this.checkElementsInView(); + } + startContentObserver() { + const contentNode = document.querySelector(this.observerNode) || document.querySelector('body'); + + if (contentNode) { + const observer = new MutationObserver(() => this.searchLazyImages()); + + observer.observe(contentNode, { + childList: true, + subtree: true, + }); + } + } + loadCheck() { + this.searchLazyImages(); + this.startContentObserver(); + } + scrollCheck() { + requestAnimationFrame(() => this.checkElementsInView()); + } + checkElementsInView() { + const scrollTop = pageYOffset; + const visHeight = scrollTop + innerHeight + SCROLL_THRESHOLD; + let imgBoundRect, imgTop, imgBound; + + // Loading Images which are in the current viewport or close to them + this.lazyImages = this.lazyImages.filter((selectedImage) => { + if (selectedImage.getAttribute('data-src')) { + imgBoundRect = selectedImage.getBoundingClientRect(); + + imgTop = scrollTop + imgBoundRect.top; + imgBound = imgTop + imgBoundRect.height; + + if (scrollTop < imgBound && visHeight > imgTop) { + LazyLoader.loadImage(selectedImage); + return false; + } + + return true; + } + return false; + }); + } + static loadImage(img) { + if (img.getAttribute('data-src')) { + img.setAttribute('src', img.getAttribute('data-src')); + img.removeAttribute('data-src'); + img.classList.remove('lazy'); + img.classList.add('js-lazy-loaded'); + } + } +} diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index 26c67fb721c..44b502cdab3 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -109,6 +109,7 @@ import './label_manager'; import './labels'; import './labels_select'; import './layout_nav'; +import LazyLoader from './lazy_loader'; import './line_highlighter'; import './logo'; import './member_expiration_date'; @@ -166,6 +167,11 @@ window.addEventListener('load', function onLoad() { gl.utils.handleLocationHash(); }, false); +gl.lazyLoader = new LazyLoader({ + scrollContainer: window, + observerNode: '#content-body' +}); + $(function () { var $body = $('body'); var $document = $(document); diff --git a/app/assets/stylesheets/framework/avatar.scss b/app/assets/stylesheets/framework/avatar.scss index 06f7af33f94..0dfa7a31d31 100644 --- a/app/assets/stylesheets/framework/avatar.scss +++ b/app/assets/stylesheets/framework/avatar.scss @@ -35,6 +35,8 @@ width: 40px; height: 40px; padding: 0; + background: $avatar-background; + overflow: hidden; &.avatar-inline { float: none; diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index 8a58c1ed567..befd8133be0 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -11,8 +11,17 @@ } img { - max-width: 100%; + /*max-width: 100%;*/ margin: 0 0 8px; + min-width: 200px; + min-height: 100px; + background-color: $gray-lightest; + } + + img.js-lazy-loaded { + min-width: none; + min-height: none; + background-color: none; } p a:not(.no-attachment-icon) img { diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 7016208f624..cf0a1ad57d0 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -379,7 +379,9 @@ $issue-boards-card-shadow: rgba(186, 186, 186, 0.5); * Avatar */ $avatar_radius: 50%; -$avatar-border: $border-color; +$avatar-border: $gray-normal; +$avatar-border-hover: $gray-darker; +$avatar-background: $gray-lightest; $gl-avatar-size: 40px; /* diff --git a/app/helpers/avatars_helper.rb b/app/helpers/avatars_helper.rb index bbe7f3c8fb4..0e068d4b51c 100644 --- a/app/helpers/avatars_helper.rb +++ b/app/helpers/avatars_helper.rb @@ -11,17 +11,12 @@ module AvatarsHelper def user_avatar_without_link(options = {}) avatar_size = options[:size] || 16 user_name = options[:user].try(:name) || options[:user_name] - css_class = options[:css_class] || '' avatar_url = options[:url] || avatar_icon(options[:user] || options[:user_email], avatar_size) data_attributes = { container: 'body' } - if options[:lazy] - data_attributes[:src] = avatar_url - end - image_tag( - options[:lazy] ? '' : avatar_url, - class: "avatar has-tooltip s#{avatar_size} #{css_class}", + avatar_url, + class: %W[avatar has-tooltip s#{avatar_size}].push(*options[:css_class]), alt: "#{user_name}'s avatar", title: user_name, data: data_attributes diff --git a/app/helpers/emails_helper.rb b/app/helpers/emails_helper.rb index fdbca789d21..5f11fe62030 100644 --- a/app/helpers/emails_helper.rb +++ b/app/helpers/emails_helper.rb @@ -61,8 +61,8 @@ module EmailsHelper else image_tag( image_url('mailers/gitlab_header_logo.gif'), - size: "55x50", - alt: "GitLab" + size: '55x50', + alt: 'GitLab' ) end end diff --git a/app/helpers/lazy_image_tag_helper.rb b/app/helpers/lazy_image_tag_helper.rb new file mode 100644 index 00000000000..2c5619ac41b --- /dev/null +++ b/app/helpers/lazy_image_tag_helper.rb @@ -0,0 +1,24 @@ +module LazyImageTagHelper + def placeholder_image + "data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==" + end + + # Override the default ActionView `image_tag` helper to support lazy-loading + def image_tag(source, options = {}) + options = options.symbolize_keys + + unless options.delete(:lazy) == false + options[:data] ||= {} + options[:data][:src] = path_to_image(source) + options[:class] ||= "" + options[:class] << " lazy" + + source = placeholder_image + end + + super(source, options) + end + + # Required for Banzai::Filter::ImageLazyLoadFilter + module_function :placeholder_image +end diff --git a/app/helpers/version_check_helper.rb b/app/helpers/version_check_helper.rb index 456598b4c28..3b175251446 100644 --- a/app/helpers/version_check_helper.rb +++ b/app/helpers/version_check_helper.rb @@ -2,7 +2,7 @@ module VersionCheckHelper def version_status_badge if Rails.env.production? && current_application_settings.version_check_enabled image_url = VersionCheck.new.url - image_tag image_url, class: 'js-version-status-badge' + image_tag image_url, class: 'js-version-status-badge', lazy: false end end end diff --git a/app/models/concerns/cache_markdown_field.rb b/app/models/concerns/cache_markdown_field.rb index 95152dcd68c..48547a938fc 100644 --- a/app/models/concerns/cache_markdown_field.rb +++ b/app/models/concerns/cache_markdown_field.rb @@ -11,7 +11,7 @@ module CacheMarkdownField extend ActiveSupport::Concern # Increment this number every time the renderer changes its output - CACHE_VERSION = 1 + CACHE_VERSION = 2 # changes to these attributes cause the cache to be invalidates INVALIDATED_BY = %w[author project].freeze diff --git a/app/views/projects/blob/viewers/_image.html.haml b/app/views/projects/blob/viewers/_image.html.haml index 640d59b3174..1650aa8197f 100644 --- a/app/views/projects/blob/viewers/_image.html.haml +++ b/app/views/projects/blob/viewers/_image.html.haml @@ -1,2 +1,2 @@ .file-content.image_file - %img{ src: blob_raw_url, alt: viewer.blob.name } + %img{ 'data-src': blob_raw_url, alt: viewer.blob.name } diff --git a/app/views/projects/diffs/viewers/_image.html.haml b/app/views/projects/diffs/viewers/_image.html.haml index 33d3dcbeafa..05877ceed3d 100644 --- a/app/views/projects/diffs/viewers/_image.html.haml +++ b/app/views/projects/diffs/viewers/_image.html.haml @@ -8,7 +8,7 @@ .image %span.wrap .frame{ class: (diff_file.deleted_file? ? 'deleted' : 'added') } - %img{ src: blob_raw_path, alt: diff_file.file_path } + %img{ 'data-src': blob_raw_path, alt: diff_file.file_path } %p.image-info= number_to_human_size(blob.size) - else .image @@ -16,7 +16,7 @@ %span.wrap .frame.deleted %a{ href: project_blob_path(@project, tree_join(diff_file.old_content_sha, diff_file.old_path)) } - %img{ src: old_blob_raw_path, alt: diff_file.old_path } + %img{ 'data-src': old_blob_raw_path, alt: diff_file.old_path } %p.image-info.hide %span.meta-filesize= number_to_human_size(old_blob.size) | @@ -28,7 +28,7 @@ %span.wrap .frame.added %a{ href: project_blob_path(@project, tree_join(diff_file.content_sha, diff_file.new_path)) } - %img{ src: blob_raw_path, alt: diff_file.new_path } + %img{ 'data-src': blob_raw_path, alt: diff_file.new_path } %p.image-info.hide %span.meta-filesize= number_to_human_size(blob.size) | @@ -41,10 +41,10 @@ .swipe.view.hide .swipe-frame .frame.deleted - %img{ src: old_blob_raw_path, alt: diff_file.old_path } + %img{ 'data-src': old_blob_raw_path, alt: diff_file.old_path } .swipe-wrap .frame.added - %img{ src: blob_raw_path, alt: diff_file.new_path } + %img{ 'data-src': blob_raw_path, alt: diff_file.new_path } %span.swipe-bar %span.top-handle %span.bottom-handle @@ -52,9 +52,9 @@ .onion-skin.view.hide .onion-skin-frame .frame.deleted - %img{ src: old_blob_raw_path, alt: diff_file.old_path } + %img{ 'data-src': old_blob_raw_path, alt: diff_file.old_path } .frame.added - %img{ src: blob_raw_path, alt: diff_file.new_path } + %img{ 'data-src': blob_raw_path, alt: diff_file.new_path } .controls .transparent .drag-track -- cgit v1.2.1 From fa9adb6599ae20c8522c92c9a0d670633fe3d5b0 Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Mon, 24 Jul 2017 14:53:30 +0200 Subject: Explicitly add `protect_from_forgery` action Otherwise the token might be cleared before authentication is done, causing the authentication itself to fail --- app/controllers/sessions_controller.rb | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'app') diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 0e8a57f8e03..69513f4dadc 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -5,6 +5,14 @@ class SessionsController < Devise::SessionsController skip_before_action :check_two_factor_requirement, only: [:destroy] + # Explicitly call protect from forgery before anything else. Otherwise the + # CSFR-token might be cleared before authentication is done. This was the case + # when LDAP was enabled and the `OmniauthCallbacksController` is loaded + # + # *Note:* `prepend: true` is the default for rails4, but this will be changed + # to `prepend: false` in rails5. + protect_from_forgery prepend: true, with: :exception + prepend_before_action :check_initial_setup, only: [:new] prepend_before_action :authenticate_with_two_factor, if: :two_factor_enabled?, only: [:create] -- cgit v1.2.1 From 3951eb62705046fb2de6c836a82c1cad043d3036 Mon Sep 17 00:00:00 2001 From: Takuya Noguchi Date: Mon, 17 Jul 2017 00:11:32 +0900 Subject: Use only CSS to truncate commit message in blame --- app/assets/stylesheets/framework/files.scss | 10 ++++++++++ app/views/projects/blame/show.html.haml | 4 ++-- 2 files changed, 12 insertions(+), 2 deletions(-) (limited to 'app') diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss index c7c2684d548..8ad082f7a65 100644 --- a/app/assets/stylesheets/framework/files.scss +++ b/app/assets/stylesheets/framework/files.scss @@ -163,8 +163,18 @@ td.blame-commit { padding: 5px 10px; min-width: 400px; + max-width: 400px; background: $gray-light; border-left: 3px solid; + + .commit-row-title { + display: flex; + } + + .item-title { + flex: 1; + margin-right: 0.5em; + } } @for $i from 0 through 5 { diff --git a/app/views/projects/blame/show.html.haml b/app/views/projects/blame/show.html.haml index f11afe8fc22..c7359d873d9 100644 --- a/app/views/projects/blame/show.html.haml +++ b/app/views/projects/blame/show.html.haml @@ -21,8 +21,8 @@ .commit = author_avatar(commit, size: 36) .commit-row-title - %strong - = link_to_gfm truncate(commit.title, length: 35), project_commit_path(@project, commit.id), class: "cdark" + %span.item-title.str-truncated-100 + = link_to_gfm commit.title, project_commit_path(@project, commit.id), class: "cdark", title: commit.title .pull-right = link_to commit.short_id, project_commit_path(@project, commit), class: "commit-sha"   -- cgit v1.2.1 From 25e44edc30b5ca61267487248db9330da3e48a6c Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 25 Jul 2017 16:44:02 +0800 Subject: Allow admin to read_users_list even if it's restricted --- app/policies/global_policy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app') diff --git a/app/policies/global_policy.rb b/app/policies/global_policy.rb index 55eefa76d3f..1c91425f589 100644 --- a/app/policies/global_policy.rb +++ b/app/policies/global_policy.rb @@ -44,7 +44,7 @@ class GlobalPolicy < BasePolicy prevent :log_in end - rule { ~restricted_public_level }.policy do + rule { admin | ~restricted_public_level }.policy do enable :read_users_list end end -- cgit v1.2.1 From e13d75c38a09fca98dfbb52ef94119770b7a445a Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Sun, 2 Jul 2017 17:02:59 +0200 Subject: Explicitly define inverse of acces_level relations --- app/models/concerns/protected_ref.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'app') diff --git a/app/models/concerns/protected_ref.rb b/app/models/concerns/protected_ref.rb index fc6b840f7a8..5dd43c36222 100644 --- a/app/models/concerns/protected_ref.rb +++ b/app/models/concerns/protected_ref.rb @@ -17,7 +17,13 @@ module ProtectedRef class_methods do def protected_ref_access_levels(*types) types.each do |type| - has_many :"#{type}_access_levels", dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent + # We need to set `inverse_of` to make sure the `belongs_to`-object is set + # when creating children using `accepts_nested_attributes_for`. + # + # If we don't `protected_branch` or `protected_tag` would be empty and + # `project` cannot be delegated to it, which in turn would cause validations + # to fail. + has_many :"#{type}_access_levels", dependent: :destroy, inverse_of: self.model_name.singular # rubocop:disable Cop/ActiveRecordDependent validates :"#{type}_access_levels", length: { is: 1, message: "are restricted to a single instance per #{self.model_name.human}." } -- cgit v1.2.1 From 531681c11d9e542fbd0d5ae5db8bc9a17cc0aefd Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Tue, 25 Jul 2017 11:28:30 +0100 Subject: Fix vertical alignment in firefox and safari for pipeline mini graph --- app/assets/stylesheets/pages/pipelines.scss | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'app') diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index 9637d26e56d..d3862df20d3 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -597,7 +597,7 @@ } // Dropdown button in mini pipeline graph -.mini-pipeline-graph-dropdown-toggle { +button.mini-pipeline-graph-dropdown-toggle { border-radius: 100px; background-color: $white-light; border-width: 1px; @@ -608,6 +608,7 @@ padding: 0; transition: all 0.2s linear; position: relative; + vertical-align: middle; > .fa.fa-caret-down { position: absolute; -- cgit v1.2.1 From 5fdef68f2bb35dc7a217c55cd6f1ed01ec3adff2 Mon Sep 17 00:00:00 2001 From: Jarka Kadlecova Date: Tue, 25 Jul 2017 14:14:28 +0200 Subject: Move relative_path to the element that is being clicked --- app/views/shared/_new_project_item_select.html.haml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'app') diff --git a/app/views/shared/_new_project_item_select.html.haml b/app/views/shared/_new_project_item_select.html.haml index c1acee1a211..5f3cdaefd54 100644 --- a/app/views/shared/_new_project_item_select.html.haml +++ b/app/views/shared/_new_project_item_select.html.haml @@ -1,6 +1,6 @@ - if @projects.any? .project-item-select-holder - = project_select_tag :project_path, class: "project-item-select", data: { include_groups: local_assigns[:include_groups], order_by: 'last_activity_at' }, with_feature_enabled: local_assigns[:with_feature_enabled] - %a.btn.btn-new.new-project-item-select-button{ data: { relative_path: local_assigns[:path] } } + = project_select_tag :project_path, class: "project-item-select", data: { include_groups: local_assigns[:include_groups], order_by: 'last_activity_at', relative_path: local_assigns[:path] }, with_feature_enabled: local_assigns[:with_feature_enabled] + %a.btn.btn-new.new-project-item-select-button = local_assigns[:label] = icon('caret-down') -- cgit v1.2.1 From 1c572994004acbd442c05537cb5062cd2e5d29e6 Mon Sep 17 00:00:00 2001 From: Jarka Kadlecova Date: Tue, 25 Jul 2017 17:25:41 +0200 Subject: Remove project_key from the Jira configuration --- app/models/project_services/jira_service.rb | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) (limited to 'app') diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb index 450027c2e57..37f2c96a22f 100644 --- a/app/models/project_services/jira_service.rb +++ b/app/models/project_services/jira_service.rb @@ -3,10 +3,8 @@ class JiraService < IssueTrackerService validates :url, url: true, presence: true, if: :activated? validates :api_url, url: true, allow_blank: true - validates :project_key, presence: true, if: :activated? - prop_accessor :username, :password, :url, :api_url, :project_key, - :jira_issue_transition_id, :title, :description + prop_accessor :username, :password, :url, :api_url, :jira_issue_transition_id, :title, :description before_update :reset_password @@ -54,10 +52,6 @@ class JiraService < IssueTrackerService @client ||= JIRA::Client.new(options) end - def jira_project - @jira_project ||= jira_request { client.Project.find(project_key) } - end - def help "You need to configure JIRA before enabling this service. For more details read the @@ -88,18 +82,12 @@ class JiraService < IssueTrackerService [ { type: 'text', name: 'url', title: 'Web URL', placeholder: 'https://jira.example.com', required: true }, { type: 'text', name: 'api_url', title: 'JIRA API URL', placeholder: 'If different from Web URL' }, - { type: 'text', name: 'project_key', placeholder: 'Project Key', required: true }, { type: 'text', name: 'username', placeholder: '', required: true }, { type: 'password', name: 'password', placeholder: '', required: true }, - { type: 'text', name: 'jira_issue_transition_id', placeholder: '' } + { type: 'text', name: 'jira_issue_transition_id', title: 'Transition ID', placeholder: '' } ] end - # URLs to redirect from Gitlab issues pages to jira issue tracker - def project_url - "#{url}/issues/?jql=project=#{project_key}" - end - def issues_url "#{url}/browse/:id" end @@ -184,7 +172,7 @@ class JiraService < IssueTrackerService def test_settings return unless client_url.present? # Test settings by getting the project - jira_request { jira_project.present? } + jira_request { client.ServerInfo.all.attrs } end private -- cgit v1.2.1 From 22d53f06076e52165af3ba04d0b703bed20cfb97 Mon Sep 17 00:00:00 2001 From: Tiago Botelho Date: Tue, 25 Jul 2017 10:09:21 +0100 Subject: Fixes 500 error caused by pending delete projects in admin dashboard --- app/controllers/admin/dashboard_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app') diff --git a/app/controllers/admin/dashboard_controller.rb b/app/controllers/admin/dashboard_controller.rb index 8360ce08bdc..05e749c00c0 100644 --- a/app/controllers/admin/dashboard_controller.rb +++ b/app/controllers/admin/dashboard_controller.rb @@ -1,6 +1,6 @@ class Admin::DashboardController < Admin::ApplicationController def index - @projects = Project.with_route.limit(10) + @projects = Project.without_deleted.with_route.limit(10) @users = User.limit(10) @groups = Group.with_route.limit(10) end -- cgit v1.2.1 From 250dbecd28473ce256e46f7233c14acb8c02a29d Mon Sep 17 00:00:00 2001 From: Tiago Botelho Date: Tue, 25 Jul 2017 18:15:45 +0100 Subject: Pending delete projects should not show in deploy keys --- app/serializers/deploy_key_entity.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app') diff --git a/app/serializers/deploy_key_entity.rb b/app/serializers/deploy_key_entity.rb index 068013c8829..c75431a79ae 100644 --- a/app/serializers/deploy_key_entity.rb +++ b/app/serializers/deploy_key_entity.rb @@ -9,7 +9,7 @@ class DeployKeyEntity < Grape::Entity expose :created_at expose :updated_at expose :projects, using: ProjectEntity do |deploy_key| - deploy_key.projects.select { |project| options[:user].can?(:read_project, project) } + deploy_key.projects.without_deleted.select { |project| options[:user].can?(:read_project, project) } end expose :can_edit -- cgit v1.2.1 From acf4a36b3ed81c952d3f2edbfb054118b1d9dfff Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Fri, 21 Jul 2017 09:36:31 +0200 Subject: Implement GRPC call to RepositoryService --- app/models/repository.rb | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) (limited to 'app') diff --git a/app/models/repository.rb b/app/models/repository.rb index 8663cf5e602..d27eeff9fb4 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -471,8 +471,17 @@ class Repository end cache_method :root_ref + # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/314 def exists? - refs_directory_exists? + return false unless path_with_namespace + + Gitlab::GitalyClient.migrate(:repository_exists) do |enabled| + if enabled + raw_repository.exists? + else + refs_directory_exists? + end + end end cache_method :exists? @@ -1095,8 +1104,6 @@ class Repository end def refs_directory_exists? - return false unless path_with_namespace - File.exist?(File.join(path_to_repo, 'refs')) end -- cgit v1.2.1 From 9aa2205a15c72394234892ef3babe94ce7eb1828 Mon Sep 17 00:00:00 2001 From: Tim Zallmann Date: Wed, 26 Jul 2017 09:31:17 +0000 Subject: Resolve "Memory usage notice doesn't link anywhere" --- .../components/mr_widget_deployment.js | 3 ++- .../components/mr_widget_memory_usage.js | 11 +++++++++-- app/controllers/projects/merge_requests_controller.rb | 6 ++++++ 3 files changed, 17 insertions(+), 3 deletions(-) (limited to 'app') diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_deployment.js b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_deployment.js index e8e22ad93a5..744a1cd24fa 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_deployment.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_deployment.js @@ -108,7 +108,8 @@ export default { diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_memory_usage.js b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_memory_usage.js index 76cb71b6c12..534e2a88eff 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_memory_usage.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_memory_usage.js @@ -7,7 +7,14 @@ import MRWidgetService from '../services/mr_widget_service'; export default { name: 'MemoryUsage', props: { - metricsUrl: { type: String, required: true }, + metricsUrl: { + type: String, + required: true, + }, + metricsMonitoringUrl: { + type: String, + required: true, + }, }, data() { return { @@ -124,7 +131,7 @@ export default {

- Memory usage {{memoryChangeType}} from {{memoryFrom}}MB to {{memoryTo}}MB + Memory usage {{memoryChangeType}} from {{memoryFrom}}MB to {{memoryTo}}MB

Date: Wed, 26 Jul 2017 07:22:57 +0000 Subject: Add missing colon --- app/views/admin/dashboard/index.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app') diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml index 128b5dc01ab..8e94e68bc11 100644 --- a/app/views/admin/dashboard/index.html.haml +++ b/app/views/admin/dashboard/index.html.haml @@ -150,7 +150,7 @@ .well-segment.well-centered = link_to admin_groups_path do %h3.text-center - Groups + Groups: = number_with_delimiter(Group.count) %hr = link_to 'New group', new_admin_group_path, class: "btn btn-new" -- cgit v1.2.1