From 18a71c47603d703de73c46fef4889887f685bebe Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 7 Nov 2016 23:44:11 +0800 Subject: Show commit status from latest pipeline Rather than compound status from all pipelines. Closes #20560 --- app/models/commit.rb | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) (limited to 'app/models/commit.rb') diff --git a/app/models/commit.rb b/app/models/commit.rb index 9e7fde9503d..2134ba2d75f 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -232,13 +232,15 @@ class Commit def status(ref = nil) @statuses ||= {} - if @statuses.key?(ref) - @statuses[ref] - elsif ref - @statuses[ref] = pipelines.where(ref: ref).status - else - @statuses[ref] = pipelines.status - end + return @statuses[ref] if @statuses.key?(ref) + + latest_pipeline = if ref + pipelines.latest_for(ref) + else + pipelines.latest + end.first + + @statuses[ref] = latest_pipeline.try(:status) end def revert_branch_name -- cgit v1.2.1 From 721f2d3788ae5e8374f357014bd9e20d62de0a81 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 21 Nov 2016 22:19:16 +0800 Subject: Still use compound pipeline status, but group by ref and sha so that it would show latest pipeline if ref and sha are both specified, otherwise still the same as before. --- app/models/commit.rb | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) (limited to 'app/models/commit.rb') diff --git a/app/models/commit.rb b/app/models/commit.rb index 2134ba2d75f..b588b93b158 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -234,13 +234,7 @@ class Commit return @statuses[ref] if @statuses.key?(ref) - latest_pipeline = if ref - pipelines.latest_for(ref) - else - pipelines.latest - end.first - - @statuses[ref] = latest_pipeline.try(:status) + @statuses[ref] = pipelines.latest_for(ref).status end def revert_branch_name -- cgit v1.2.1 From 6192ea53fad0ea04e356e5a79a5a0e5359ba46ce Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 24 Nov 2016 16:50:37 +0800 Subject: Rename latest_for to latest, feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333/diffs#note_18819292 --- app/models/commit.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/models/commit.rb') diff --git a/app/models/commit.rb b/app/models/commit.rb index b588b93b158..946bfc4712c 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -234,7 +234,7 @@ class Commit return @statuses[ref] if @statuses.key?(ref) - @statuses[ref] = pipelines.latest_for(ref).status + @statuses[ref] = pipelines.latest(ref).status end def revert_branch_name -- cgit v1.2.1 From 6b4d33566f5f434cc86381a4a1347e42bbe348ee Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Thu, 24 Nov 2016 15:07:44 +0100 Subject: Pass commit data to ProcessCommitWorker By passing commit data to this worker we remove the need for querying the Git repository for every job. This in turn reduces the time spent processing each job. The migration included migrates jobs from the old format to the new format. For this to work properly it requires downtime as otherwise workers may start producing errors until they're using a newer version of the worker code. --- app/models/commit.rb | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'app/models/commit.rb') diff --git a/app/models/commit.rb b/app/models/commit.rb index 9e7fde9503d..176c524cf7b 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -48,6 +48,10 @@ class Commit max_lines: DIFF_HARD_LIMIT_LINES, } end + + def from_hash(hash, project) + new(Gitlab::Git::Commit.new(hash), project) + end end attr_accessor :raw -- cgit v1.2.1 From f272ee6eba37548cbd8919139d583a71ffdac8dc Mon Sep 17 00:00:00 2001 From: Oswaldo Ferreira Date: Wed, 2 Nov 2016 21:49:13 -0200 Subject: Add shorthand support to gitlab markdown references --- app/models/commit.rb | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) (limited to 'app/models/commit.rb') diff --git a/app/models/commit.rb b/app/models/commit.rb index 176c524cf7b..248140f421b 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -92,19 +92,11 @@ class Commit end def to_reference(from_project = nil) - if cross_project_reference?(from_project) - project.to_reference + self.class.reference_prefix + self.id - else - self.id - end + commit_reference(from_project, id) end def reference_link_text(from_project = nil) - if cross_project_reference?(from_project) - project.to_reference + self.class.reference_prefix + self.short_id - else - self.short_id - end + commit_reference(from_project, short_id) end def diff_line_count @@ -329,6 +321,16 @@ class Commit private + def commit_reference(from_project, referable_commit_id) + reference = project.to_reference(from_project) + + if reference.present? + "#{reference}#{self.class.reference_prefix}#{referable_commit_id}" + else + referable_commit_id + end + end + def find_author_by_any_email User.find_by_any_email(author_email.downcase) end -- cgit v1.2.1 From f23b1cb453deea2659c0cb9e9047c72d859bbf9d Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 29 Nov 2016 13:47:43 +0000 Subject: Merge branch 'jej-23867-use-mr-finder-instead-of-access-check' into 'security' Replace MR access checks with use of MergeRequestsFinder Split from !2024 to partially solve https://gitlab.com/gitlab-org/gitlab-ce/issues/23867 :warning: - Potentially untested :bomb: - No test coverage :traffic_light: - Test coverage of some sort exists (a test failed when error raised) :vertical_traffic_light: - Test coverage of return value (a test failed when nil used) :white_check_mark: - Permissions check tested - [x] :bomb: app/finders/notes_finder.rb:17 - [x] :warning: app/views/layouts/nav/_project.html.haml:80 [`.count`] - [x] :bomb: app/controllers/concerns/creates_commit.rb:84 - [x] :traffic_light: app/controllers/projects/commits_controller.rb:24 - [x] :traffic_light: app/controllers/projects/compare_controller.rb:56 - [x] :vertical_traffic_light: app/controllers/projects/discussions_controller.rb:29 - [x] :white_check_mark: app/controllers/projects/todos_controller.rb:27 - [x] :vertical_traffic_light: app/models/commit.rb:268 - [x] :white_check_mark: lib/gitlab/search_results.rb:71 - [x] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#d1c10892daedb4d4dd3d4b12b6d071091eea83df_267_266 Memoize ` merged_merge_request(current_user)` - [x] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#d1c10892daedb4d4dd3d4b12b6d071091eea83df_248_247 Expected side effect for `merged_merge_request!`, consider `skip_authorization: true`. - [x] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#d1c10892daedb4d4dd3d4b12b6d071091eea83df_269_269 Scary use of unchecked `merged_merge_request?` See merge request !2033 --- app/models/commit.rb | 41 ++++++++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 15 deletions(-) (limited to 'app/models/commit.rb') diff --git a/app/models/commit.rb b/app/models/commit.rb index 248140f421b..1831cc7e175 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -245,44 +245,47 @@ class Commit project.repository.next_branch("cherry-pick-#{short_id}", mild: true) end - def revert_description - if merged_merge_request - "This reverts merge request #{merged_merge_request.to_reference}" + def revert_description(user) + if merged_merge_request?(user) + "This reverts merge request #{merged_merge_request(user).to_reference}" else "This reverts commit #{sha}" end end - def revert_message - %Q{Revert "#{title.strip}"\n\n#{revert_description}} + def revert_message(user) + %Q{Revert "#{title.strip}"\n\n#{revert_description(user)}} end - def reverts_commit?(commit) - description? && description.include?(commit.revert_description) + def reverts_commit?(commit, user) + description? && description.include?(commit.revert_description(user)) end def merge_commit? parents.size > 1 end - def merged_merge_request - return @merged_merge_request if defined?(@merged_merge_request) - - @merged_merge_request = project.merge_requests.find_by(merge_commit_sha: id) if merge_commit? + def merged_merge_request(current_user) + # Memoize with per-user access check + @merged_merge_request_hash ||= Hash.new do |hash, user| + hash[user] = merged_merge_request_no_cache(user) + end + + @merged_merge_request_hash[current_user] end - def has_been_reverted?(current_user = nil, noteable = self) + def has_been_reverted?(current_user, noteable = self) ext = all_references(current_user) noteable.notes_with_associations.system.each do |note| note.all_references(current_user, extractor: ext) end - ext.commits.any? { |commit_ref| commit_ref.reverts_commit?(self) } + ext.commits.any? { |commit_ref| commit_ref.reverts_commit?(self, current_user) } end - def change_type_title - merged_merge_request ? 'merge request' : 'commit' + def change_type_title(user) + merged_merge_request?(user) ? 'merge request' : 'commit' end # Get the URI type of the given path @@ -350,4 +353,12 @@ class Commit changes end + + def merged_merge_request?(user) + !!merged_merge_request(user) + end + + def merged_merge_request_no_cache(user) + MergeRequestsFinder.new(user, project_id: project.id).find_by(merge_commit_sha: id) if merge_commit? + end end -- cgit v1.2.1 From 7cced60069c248156decf6ceabc4d1f447e47ff7 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 14 Dec 2016 21:00:06 +0800 Subject: Introduce latest_status and add a few tests Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_20003268 --- app/models/commit.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'app/models/commit.rb') diff --git a/app/models/commit.rb b/app/models/commit.rb index 91c7970fca6..69cfc47f5bf 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -230,7 +230,7 @@ class Commit return @statuses[ref] if @statuses.key?(ref) - @statuses[ref] = pipelines.latest(ref).status + @statuses[ref] = pipelines.latest_status(ref) end def revert_branch_name @@ -266,7 +266,7 @@ class Commit @merged_merge_request_hash ||= Hash.new do |hash, user| hash[user] = merged_merge_request_no_cache(user) end - + @merged_merge_request_hash[current_user] end -- cgit v1.2.1