From c843722de2d778b6ec8fef0656797fd5a8074666 Mon Sep 17 00:00:00 2001 From: Jeff Stubler Date: Mon, 20 Jul 2015 20:34:19 -0500 Subject: [PATCH 01/23] Add graphs showing commits ahead and behind default to branches page --- CHANGELOG | 1 + app/assets/stylesheets/pages/commits.scss | 59 +++++++++++++++++++ .../projects/branches_controller.rb | 6 ++ app/models/project.rb | 2 + app/models/repository.rb | 42 ++++++++++++- app/views/projects/branches/_branch.html.haml | 14 +++++ 6 files changed, 122 insertions(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 6bec4f606e7..98702088199 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -16,6 +16,7 @@ v 8.2.0 (unreleased) - Fix: 500 error returned if destroy request without HTTP referer (Kazuki Shimizu) - Remove deprecated CI events from project settings page - Use issue editor as cross reference comment author when issue is edited with a new mention. + - Add graphs of commits ahead and behind default branch (Jeff Stubler) v 8.1.1 - Fix cloning Wiki repositories via HTTP (Stan Hu) diff --git a/app/assets/stylesheets/pages/commits.scss b/app/assets/stylesheets/pages/commits.scss index 4e121b95d13..4a080db7464 100644 --- a/app/assets/stylesheets/pages/commits.scss +++ b/app/assets/stylesheets/pages/commits.scss @@ -113,3 +113,62 @@ li.commit { } } } + +.divergence-graph { + padding: 12px 12px 0 0; + float: right; + + .graph-side { + position: relative; + width: 80px; + height: 22px; + padding: 5px 0 13px; + float: left; + + .bar { + position: absolute; + height: 4px; + background-color: #ccc; + } + + .bar-behind { + right: 0; + border-radius: 3px 0 0 3px; + } + + .bar-ahead { + left: 0; + border-radius: 0 3px 3px 0; + } + + .count { + padding-top: 6px; + padding-bottom: 0px; + font-size: 12px; + color: #333; + display: block; + } + + .count-behind { + padding-right: 4px; + text-align: right; + } + + .count-ahead { + padding-left: 4px; + text-align: left; + } + } + + .graph-separator { + position: relative; + width: 1px; + height: 18px; + margin: 5px 0 0; + float: left; + background-color: #ccc; + } +} + + + diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb index 3ac0a75fa70..c3cd7642dd2 100644 --- a/app/controllers/projects/branches_controller.rb +++ b/app/controllers/projects/branches_controller.rb @@ -9,6 +9,12 @@ class Projects::BranchesController < Projects::ApplicationController @sort = params[:sort] || 'name' @branches = @repository.branches_sorted_by(@sort) @branches = Kaminari.paginate_array(@branches).page(params[:page]).per(PER_PAGE) + + @max_commits = @branches.reduce(0) do + |memo, branch| + diverging_commit_counts = repository.diverging_commit_counts(branch) + [memo, diverging_commit_counts[:behind], diverging_commit_counts[:ahead]].max + end end def recent diff --git a/app/models/project.rb b/app/models/project.rb index 74b89aad499..e73a856c289 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -714,6 +714,8 @@ class Project < ActiveRecord::Base end def change_head(branch) + # Cached divergent commit counts are based on repository head + repository.expire_branch_cache gitlab_shell.update_repository_head(self.path_with_namespace, branch) reload_default_branch end diff --git a/app/models/repository.rb b/app/models/repository.rb index c9b36bd8170..9b270bc9d18 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -146,10 +146,27 @@ class Repository def size cache.fetch(:size) { raw_repository.size } end + + def diverging_commit_counts(branch) + branch_cache_key = ('diverging_commit_counts_' + branch.name).to_sym + cache.fetch(branch_cache_key) do + number_commits_behind = commits_between(branch.name, root_ref).size + number_commits_ahead = commits_between(root_ref, branch.name).size + + { behind: number_commits_behind, ahead: number_commits_ahead } + end + end def cache_keys - %i(size branch_names tag_names commit_count - readme version contribution_guide changelog license) + %i(size branch_names tag_names commit_count readme + contribution_guide changelog license) + end + + def branch_cache_keys + branches.map do + |branch| + ('diverging_commit_counts_' + branch.name).to_sym + end end def build_cache @@ -158,12 +175,28 @@ class Repository send(key) end end + + branches.each do |branch| + unless cache.exist?(('diverging_commit_counts_' + branch.name).to_sym) + send(:diverging_commit_counts, branch) + end + end end def expire_cache cache_keys.each do |key| cache.expire(key) end + + branches.each do |branch| + cache.expire(('diverging_commit_counts_' + branch.name).to_sym) + end + end + + def expire_branch_cache + branches.each do |branch| + cache.expire(('diverging_commit_counts_' + branch.name).to_sym) + end end def rebuild_cache @@ -171,6 +204,11 @@ class Repository cache.expire(key) send(key) end + + branches.each do |branch| + cache.expire(('diverging_commit_counts_' + branch.name).to_sym) + send(:diverging_commit_counts, branch) + end end def lookup_cache diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml index cc0ec9483d2..9ddb10a1c74 100644 --- a/app/views/projects/branches/_branch.html.haml +++ b/app/views/projects/branches/_branch.html.haml @@ -1,4 +1,7 @@ - commit = @repository.commit(branch.target) +- bar_graph_width_factor = @max_commits > 0 ? 100.0/@max_commits : 0 +- number_commits_behind = @repository.diverging_commit_counts(branch)[:behind] +- number_commits_ahead = @repository.diverging_commit_counts(branch)[:ahead] %li(class="js-branch-#{branch.name}") %div = link_to namespace_project_tree_path(@project.namespace, @project, branch.name) do @@ -29,6 +32,17 @@ = link_to namespace_project_branch_path(@project.namespace, @project, branch.name), class: 'btn btn-grouped btn-xs btn-remove remove-row', method: :delete, data: { confirm: 'Removed branch cannot be restored. Are you sure?'}, remote: true do = icon("trash-o") + - if branch.name != @repository.root_ref + .divergence-graph{ :title => "#{number_commits_ahead} commits ahead, #{number_commits_behind} commits behind #{@repository.root_ref}" } + .graph-side + .bar.bar-behind{ :style => "width: #{number_commits_behind * bar_graph_width_factor}%" } + %span.count.count-behind= number_commits_behind + .graph-separator + .graph-side + .bar.bar-ahead{ :style => "width: #{number_commits_ahead * bar_graph_width_factor}%" } + %span.count.count-ahead= number_commits_ahead + + - if commit = render 'projects/branches/commit', commit: commit, project: @project - else From e0c64fac68b4b3acc48300956146b85e03b426ce Mon Sep 17 00:00:00 2001 From: Jeff Stubler Date: Wed, 11 Nov 2015 16:29:29 -0600 Subject: [PATCH 02/23] Refactor for style issues --- .../projects/branches_controller.rb | 3 +-- app/models/repository.rb | 24 ++++++++----------- app/views/projects/branches/_branch.html.haml | 11 +++++---- 3 files changed, 17 insertions(+), 21 deletions(-) diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb index c3cd7642dd2..87884420952 100644 --- a/app/controllers/projects/branches_controller.rb +++ b/app/controllers/projects/branches_controller.rb @@ -10,8 +10,7 @@ class Projects::BranchesController < Projects::ApplicationController @branches = @repository.branches_sorted_by(@sort) @branches = Kaminari.paginate_array(@branches).page(params[:page]).per(PER_PAGE) - @max_commits = @branches.reduce(0) do - |memo, branch| + @max_commits = @branches.reduce(0) do |memo, branch| diverging_commit_counts = repository.diverging_commit_counts(branch) [memo, diverging_commit_counts[:behind], diverging_commit_counts[:ahead]].max end diff --git a/app/models/repository.rb b/app/models/repository.rb index 9b270bc9d18..0e2d4ea1fb8 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -148,8 +148,7 @@ class Repository end def diverging_commit_counts(branch) - branch_cache_key = ('diverging_commit_counts_' + branch.name).to_sym - cache.fetch(branch_cache_key) do + cache.fetch(:"diverging_commit_counts_#{branch.name}") do number_commits_behind = commits_between(branch.name, root_ref).size number_commits_ahead = commits_between(root_ref, branch.name).size @@ -158,14 +157,13 @@ class Repository end def cache_keys - %i(size branch_names tag_names commit_count readme - contribution_guide changelog license) + %i(size branch_names tag_names commit_count + readme version contribution_guide changelog license) end def branch_cache_keys - branches.map do - |branch| - ('diverging_commit_counts_' + branch.name).to_sym + branches.map do |branch| + :"diverging_commit_counts_#{branch.name}" end end @@ -177,7 +175,7 @@ class Repository end branches.each do |branch| - unless cache.exist?(('diverging_commit_counts_' + branch.name).to_sym) + unless cache.exist?(:"diverging_commit_counts_#{branch.name}") send(:diverging_commit_counts, branch) end end @@ -188,14 +186,12 @@ class Repository cache.expire(key) end - branches.each do |branch| - cache.expire(('diverging_commit_counts_' + branch.name).to_sym) - end + expire_branch_cache end def expire_branch_cache branches.each do |branch| - cache.expire(('diverging_commit_counts_' + branch.name).to_sym) + cache.expire(:"diverging_commit_counts_#{branch.name}") end end @@ -206,8 +202,8 @@ class Repository end branches.each do |branch| - cache.expire(('diverging_commit_counts_' + branch.name).to_sym) - send(:diverging_commit_counts, branch) + cache.expire(:"diverging_commit_counts_#{branch.name}") + diverging_commit_counts(branch) end end diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml index 9ddb10a1c74..a4202d1120d 100644 --- a/app/views/projects/branches/_branch.html.haml +++ b/app/views/projects/branches/_branch.html.haml @@ -1,7 +1,8 @@ - commit = @repository.commit(branch.target) - bar_graph_width_factor = @max_commits > 0 ? 100.0/@max_commits : 0 -- number_commits_behind = @repository.diverging_commit_counts(branch)[:behind] -- number_commits_ahead = @repository.diverging_commit_counts(branch)[:ahead] +- diverging_commit_counts = @repository.diverging_commit_counts(branch) +- number_commits_behind = diverging_commit_counts[:behind] +- number_commits_ahead = diverging_commit_counts[:ahead] %li(class="js-branch-#{branch.name}") %div = link_to namespace_project_tree_path(@project.namespace, @project, branch.name) do @@ -33,13 +34,13 @@ = icon("trash-o") - if branch.name != @repository.root_ref - .divergence-graph{ :title => "#{number_commits_ahead} commits ahead, #{number_commits_behind} commits behind #{@repository.root_ref}" } + .divergence-graph{ title: "#{number_commits_ahead} commits ahead, #{number_commits_behind} commits behind #{@repository.root_ref}" } .graph-side - .bar.bar-behind{ :style => "width: #{number_commits_behind * bar_graph_width_factor}%" } + .bar.bar-behind{ style: "width: #{number_commits_behind * bar_graph_width_factor}%" } %span.count.count-behind= number_commits_behind .graph-separator .graph-side - .bar.bar-ahead{ :style => "width: #{number_commits_ahead * bar_graph_width_factor}%" } + .bar.bar-ahead{ style: "width: #{number_commits_ahead * bar_graph_width_factor}%" } %span.count.count-ahead= number_commits_ahead From c12514fc2d3996e7cfc3553d8e2bac04d0c5afec Mon Sep 17 00:00:00 2001 From: The rugged tests are fragile Date: Tue, 15 Dec 2015 15:05:57 +0800 Subject: [PATCH 03/23] Ignore config/sidekiq.yml [ci skip] --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index f5b6427ca03..91ea81bfc4e 100644 --- a/.gitignore +++ b/.gitignore @@ -26,6 +26,7 @@ config/initializers/smtp_settings.rb config/resque.yml config/unicorn.rb config/secrets.yml +config/sidekiq.yml coverage/* db/*.sqlite3 db/*.sqlite3-journal From 2e8ec7e7204b2876218db34439584204b1062265 Mon Sep 17 00:00:00 2001 From: Jeff Stubler Date: Tue, 15 Dec 2015 16:23:52 -0600 Subject: [PATCH 04/23] Fix diverging commit count calculation Rugged seemed to stop accepting branch names as valid refs, throwing `Rugged::ReferenceError` exceptions. Now the branch target rather than branch name is used, and the default branch's hash is also calculated, and these work properly. This used to work and might be something worth re-investigating in the future. --- app/models/repository.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/models/repository.rb b/app/models/repository.rb index 4186ef295c9..77e5bd975ec 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -157,9 +157,12 @@ class Repository end def diverging_commit_counts(branch) + root_ref_hash = raw_repository.rev_parse_target(root_ref).oid cache.fetch(:"diverging_commit_counts_#{branch.name}") do - number_commits_behind = commits_between(branch.name, root_ref).size - number_commits_ahead = commits_between(root_ref, branch.name).size + # Rugged seems to throw a `ReferenceError` when given branch_names rather + # than SHA-1 hashes + number_commits_behind = commits_between(branch.target, root_ref_hash).size + number_commits_ahead = commits_between(root_ref_hash, branch.target).size { behind: number_commits_behind, ahead: number_commits_ahead } end From b45ee2c314e2c26f4574f2e973dfa40204860c66 Mon Sep 17 00:00:00 2001 From: Mike Wyatt Date: Wed, 16 Dec 2015 10:08:05 -0400 Subject: [PATCH 05/23] better support for referencing and closing issues in asana_service.rb --- app/models/project_services/asana_service.rb | 28 +++++++++++++------ .../project_services/asana_service_spec.rb | 21 ++++++++++++++ 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/app/models/project_services/asana_service.rb b/app/models/project_services/asana_service.rb index e6e16058d41..bbc508e8f8e 100644 --- a/app/models/project_services/asana_service.rb +++ b/app/models/project_services/asana_service.rb @@ -98,17 +98,29 @@ automatically inspected. Leave blank to include all branches.' task_list = [] close_list = [] - message.split("\n").each do |line| - # look for a task ID or a full Asana url - task_list.concat(line.scan(/#(\d+)/)) - task_list.concat(line.scan(/https:\/\/app\.asana\.com\/\d+\/\d+\/(\d+)/)) - # look for a word starting with 'fix' followed by a task ID - close_list.concat(line.scan(/(fix\w*)\W*#(\d+)/i)) + # matches either: + # - #1234 + # - https://app.asana.com/0/0/1234 + # optionally preceded with: + # - fix/ed/es/ing + # - close/s/d + # - closing + issue_finder = /(fix\w*|clos[ei]\w*+)?\W*(?:https:\/\/app\.asana\.com\/\d+\/\d+\/(\d+)|#(\d+))/i + + message.scan(issue_finder).each do |tuple| + # tuple will be + # [ 'fix', 'id_from_url', 'id_from_pound' ] + taskid = tuple[2] || tuple[1] + task_list.push(taskid) + + if tuple[0] + close_list.push(taskid) + end end # post commit to every taskid found task_list.each do |taskid| - task = Asana::Task.find(taskid[0]) + task = Asana::Task.find(taskid) if task task.create_story(text: push_msg + ' ' + message) @@ -117,7 +129,7 @@ automatically inspected. Leave blank to include all branches.' # close all tasks that had 'fix(ed/es/ing) #:id' in them close_list.each do |taskid| - task = Asana::Task.find(taskid.last) + task = Asana::Task.find(taskid) if task task.modify(completed: true) diff --git a/spec/models/project_services/asana_service_spec.rb b/spec/models/project_services/asana_service_spec.rb index 64bb92fba95..e368b03206e 100644 --- a/spec/models/project_services/asana_service_spec.rb +++ b/spec/models/project_services/asana_service_spec.rb @@ -62,5 +62,26 @@ describe AsanaService, models: true do @asana.check_commit('fix #456789', 'pushed') end + + it 'should be able to close via url' do + expect(Asana::Task).to receive(:find).with('42').twice + + @asana.check_commit('closes https://app.asana.com/19292/956299/42', 'pushed') + end + + it 'should allow multiple matches per line' do + expect(Asana::Task).to receive(:find).with('123').twice + expect(Asana::Task).to receive(:find).with('456').twice + expect(Asana::Task).to receive(:find).with('789').once + + expect(Asana::Task).to receive(:find).with('42').once + expect(Asana::Task).to receive(:find).with('12').twice + + message = <<-EOF + minor bigfix, refactoring, fixed #123 and Closes #456 work on #789 + ref https://app.asana.com/19292/956299/42 and closing https://app.asana.com/19292/956299/12 + EOF + @asana.check_commit(message, 'pushed') + end end end From 141b8b67ff4cbe67778ff6815a51f49834e290b9 Mon Sep 17 00:00:00 2001 From: Michi302 Date: Mon, 28 Dec 2015 15:50:44 +0100 Subject: [PATCH 06/23] Make single user API endpoint return Entities::User instead of Entities::UserBasic --- doc/api/users.md | 7 +++++++ lib/api/users.rb | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/doc/api/users.md b/doc/api/users.md index 66d2fd52526..773fe36d277 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -123,6 +123,13 @@ Parameters: "name": "John Smith", "state": "active", "avatar_url": "http://localhost:3000/uploads/user/avatar/1/cd8.jpeg", + "created_at": "2012-05-23T08:00:58Z", + "is_admin": false, + "bio": null, + "skype": "", + "linkedin": "", + "twitter": "", + "website_url": "" } ``` diff --git a/lib/api/users.rb b/lib/api/users.rb index 3400f0713ef..0d7813428e2 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -39,7 +39,7 @@ module API if current_user.is_admin? present @user, with: Entities::UserFull else - present @user, with: Entities::UserBasic + present @user, with: Entities::User end end From 2cd2c54bd1c83f8545b5f903d7717c4848453f0c Mon Sep 17 00:00:00 2001 From: Mike Wyatt Date: Tue, 29 Dec 2015 15:37:48 -0400 Subject: [PATCH 07/23] Update Asana field descriptions --- app/models/project_services/asana_service.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/models/project_services/asana_service.rb b/app/models/project_services/asana_service.rb index bbc508e8f8e..a16adf10432 100644 --- a/app/models/project_services/asana_service.rb +++ b/app/models/project_services/asana_service.rb @@ -53,14 +53,12 @@ http://developer.asana.com/documentation/#api_keys' { type: 'text', name: 'api_key', - placeholder: 'User API token. User must have access to task, -all comments will be attributed to this user.' + placeholder: 'User Personal Access Token. User must have access to task, all comments will be attributed to this user.' }, { type: 'text', name: 'restrict_to_branch', - placeholder: 'Comma-separated list of branches which will be -automatically inspected. Leave blank to include all branches.' + placeholder: 'Comma-separated list of branches which will be automatically inspected. Leave blank to include all branches.' } ] end From 12ec1e3c407a4b72f670483a558a9cb95449c838 Mon Sep 17 00:00:00 2001 From: Mike Wyatt Date: Tue, 29 Dec 2015 15:39:58 -0400 Subject: [PATCH 08/23] Update Asana service to work with Personal Access Token, lessen number of requests to Asana API --- app/models/project_services/asana_service.rb | 58 ++++++++------------ 1 file changed, 24 insertions(+), 34 deletions(-) diff --git a/app/models/project_services/asana_service.rb b/app/models/project_services/asana_service.rb index a16adf10432..111a60431b1 100644 --- a/app/models/project_services/asana_service.rb +++ b/app/models/project_services/asana_service.rb @@ -67,35 +67,34 @@ http://developer.asana.com/documentation/#api_keys' %w(push) end + def client + @_client ||= begin + Asana::Client.new do |c| + c.authentication :access_token, api_key + end + end + end + def execute(data) return unless supported_events.include?(data[:object_kind]) - Asana.configure do |client| - client.api_key = api_key - end - - user = data[:user_name] - branch = Gitlab::Git.ref_name(data[:ref]) - - branch_restriction = restrict_to_branch.to_s - # check the branch restriction is poplulated and branch is not included + branch = Gitlab::Git.ref_name(data[:ref]) + branch_restriction = restrict_to_branch.to_s if branch_restriction.length > 0 && branch_restriction.index(branch).nil? return end + user = data[:user_name] project_name = project.name_with_namespace - push_msg = user + ' pushed to branch ' + branch + ' of ' + project_name + push_msg = "#{user} pushed to branch #{branch} of #{project_name} ( #{commit[:url]} )" data[:commits].each do |commit| - check_commit(' ( ' + commit[:url] + ' ): ' + commit[:message], push_msg) + check_commit(commit[:message], push_msg) end end def check_commit(message, push_msg) - task_list = [] - close_list = [] - # matches either: # - #1234 # - https://app.asana.com/0/0/1234 @@ -109,28 +108,19 @@ http://developer.asana.com/documentation/#api_keys' # tuple will be # [ 'fix', 'id_from_url', 'id_from_pound' ] taskid = tuple[2] || tuple[1] - task_list.push(taskid) + + begin + task = Asana::Task.find_by_id(client, taskid) + rescue Exception => e + puts e.message + puts e.backtrace.inspect + next + end + + task.add_comment(text: "#{push_msg} #{message}") if tuple[0] - close_list.push(taskid) - end - end - - # post commit to every taskid found - task_list.each do |taskid| - task = Asana::Task.find(taskid) - - if task - task.create_story(text: push_msg + ' ' + message) - end - end - - # close all tasks that had 'fix(ed/es/ing) #:id' in them - close_list.each do |taskid| - task = Asana::Task.find(taskid) - - if task - task.modify(completed: true) + task.update(completed: true) end end end From 4c1f1c2c91a1a43f0ee38fc0f5e8c92dea996f04 Mon Sep 17 00:00:00 2001 From: Mike Wyatt Date: Tue, 29 Dec 2015 15:40:50 -0400 Subject: [PATCH 09/23] Update Asana specs --- .../project_services/asana_service_spec.rb | 40 +++++++++++++++---- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/spec/models/project_services/asana_service_spec.rb b/spec/models/project_services/asana_service_spec.rb index e368b03206e..306d18171be 100644 --- a/spec/models/project_services/asana_service_spec.rb +++ b/spec/models/project_services/asana_service_spec.rb @@ -52,30 +52,54 @@ describe AsanaService, models: true do end it 'should call Asana service to created a story' do - expect(Asana::Task).to receive(:find).with('123456').once + d1 = double('Asana::Task', add_comment: true) + expect(d1).to receive(:add_comment) + expect(Asana::Task).to receive(:find_by_id).with(anything, '123456').once.and_return(d1) @asana.check_commit('related to #123456', 'pushed') end it 'should call Asana service to created a story and close a task' do - expect(Asana::Task).to receive(:find).with('456789').twice + d1 = double('Asana::Task', add_comment: true) + expect(d1).to receive(:add_comment) + expect(d1).to receive(:update).with(completed: true) + expect(Asana::Task).to receive(:find_by_id).with(anything, '456789').once.and_return(d1) @asana.check_commit('fix #456789', 'pushed') end it 'should be able to close via url' do - expect(Asana::Task).to receive(:find).with('42').twice + d1 = double('Asana::Task', add_comment: true) + expect(d1).to receive(:add_comment) + expect(d1).to receive(:update).with(completed: true) + expect(Asana::Task).to receive(:find_by_id).with(anything, '42').once.and_return(d1) @asana.check_commit('closes https://app.asana.com/19292/956299/42', 'pushed') end it 'should allow multiple matches per line' do - expect(Asana::Task).to receive(:find).with('123').twice - expect(Asana::Task).to receive(:find).with('456').twice - expect(Asana::Task).to receive(:find).with('789').once + d1 = double('Asana::Task', add_comment: true) + expect(d1).to receive(:add_comment) + expect(d1).to receive(:update).with(completed: true) + expect(Asana::Task).to receive(:find_by_id).with(anything, '123').once.and_return(d1) - expect(Asana::Task).to receive(:find).with('42').once - expect(Asana::Task).to receive(:find).with('12').twice + d2 = double('Asana::Task', add_comment: true) + expect(d2).to receive(:add_comment) + expect(d2).to receive(:update).with(completed: true) + expect(Asana::Task).to receive(:find_by_id).with(anything, '456').once.and_return(d2) + + d3 = double('Asana::Task', add_comment: true) + expect(d3).to receive(:add_comment) + expect(Asana::Task).to receive(:find_by_id).with(anything, '789').once.and_return(d3) + + d4 = double('Asana::Task', add_comment: true) + expect(d4).to receive(:add_comment) + expect(Asana::Task).to receive(:find_by_id).with(anything, '42').once.and_return(d4) + + d5 = double('Asana::Task', add_comment: true) + expect(d5).to receive(:add_comment) + expect(d5).to receive(:update).with(completed: true) + expect(Asana::Task).to receive(:find_by_id).with(anything, '12').once.and_return(d5) message = <<-EOF minor bigfix, refactoring, fixed #123 and Closes #456 work on #789 From e8080fc8fb216fb6da7d3f056e8b4417f807d09e Mon Sep 17 00:00:00 2001 From: Mike Wyatt Date: Tue, 29 Dec 2015 16:00:36 -0400 Subject: [PATCH 10/23] Fix error in Asana service --- app/models/project_services/asana_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/project_services/asana_service.rb b/app/models/project_services/asana_service.rb index 111a60431b1..80c56b9097b 100644 --- a/app/models/project_services/asana_service.rb +++ b/app/models/project_services/asana_service.rb @@ -87,9 +87,9 @@ http://developer.asana.com/documentation/#api_keys' user = data[:user_name] project_name = project.name_with_namespace - push_msg = "#{user} pushed to branch #{branch} of #{project_name} ( #{commit[:url]} )" data[:commits].each do |commit| + push_msg = "#{user} pushed to branch #{branch} of #{project_name} ( #{commit[:url]} )" check_commit(commit[:message], push_msg) end end From 6eb273a11cf7521448f4bcb5f9376df24c2f9f3f Mon Sep 17 00:00:00 2001 From: Mike Wyatt Date: Wed, 30 Dec 2015 00:50:44 -0400 Subject: [PATCH 11/23] Restore colon in Asana comment --- app/models/project_services/asana_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/project_services/asana_service.rb b/app/models/project_services/asana_service.rb index 80c56b9097b..183ce2df787 100644 --- a/app/models/project_services/asana_service.rb +++ b/app/models/project_services/asana_service.rb @@ -89,7 +89,7 @@ http://developer.asana.com/documentation/#api_keys' project_name = project.name_with_namespace data[:commits].each do |commit| - push_msg = "#{user} pushed to branch #{branch} of #{project_name} ( #{commit[:url]} )" + push_msg = "#{user} pushed to branch #{branch} of #{project_name} ( #{commit[:url]} ):" check_commit(commit[:message], push_msg) end end From 7b98d0e1a24645df802ad65911c290ad057d1422 Mon Sep 17 00:00:00 2001 From: Mike Wyatt Date: Wed, 30 Dec 2015 00:51:05 -0400 Subject: [PATCH 12/23] Update Asana service documentation --- app/models/project_services/asana_service.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/project_services/asana_service.rb b/app/models/project_services/asana_service.rb index 183ce2df787..ab5772356f1 100644 --- a/app/models/project_services/asana_service.rb +++ b/app/models/project_services/asana_service.rb @@ -40,8 +40,8 @@ get the commit comment added to it. You can also close a task with a message containing: `fix #123456`. -You can find your Api Keys here: -http://developer.asana.com/documentation/#api_keys' +You can create a Personal Access Token here: +http://app.asana.com/-/account_api' end def to_param From 9e7a88f089323964088945829523b798ea6b78b5 Mon Sep 17 00:00:00 2001 From: Mike Wyatt Date: Wed, 30 Dec 2015 00:52:56 -0400 Subject: [PATCH 13/23] Better handling of errors in Asana service [ci skip] --- app/models/project_services/asana_service.rb | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/app/models/project_services/asana_service.rb b/app/models/project_services/asana_service.rb index ab5772356f1..cb4f6ddb3a5 100644 --- a/app/models/project_services/asana_service.rb +++ b/app/models/project_services/asana_service.rb @@ -111,17 +111,16 @@ http://app.asana.com/-/account_api' begin task = Asana::Task.find_by_id(client, taskid) - rescue Exception => e - puts e.message - puts e.backtrace.inspect + task.add_comment(text: "#{push_msg} #{message}") + + if tuple[0] + task.update(completed: true) + end + rescue => e + Rails.logger.error(e.message) + Rails.logger.error(e.backtrace.join("\n")) next end - - task.add_comment(text: "#{push_msg} #{message}") - - if tuple[0] - task.update(completed: true) - end end end end From 90029a5caaef1fd9d41a8ac02a7e9840ce3ac7b5 Mon Sep 17 00:00:00 2001 From: Mike Wyatt Date: Thu, 31 Dec 2015 18:27:34 -0400 Subject: [PATCH 14/23] Actually test the posted comment in Asana service --- .../project_services/asana_service_spec.rb | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/spec/models/project_services/asana_service_spec.rb b/spec/models/project_services/asana_service_spec.rb index 306d18171be..a7b32ac07a9 100644 --- a/spec/models/project_services/asana_service_spec.rb +++ b/spec/models/project_services/asana_service_spec.rb @@ -40,6 +40,20 @@ describe AsanaService, models: true do let(:user) { create(:user) } let(:project) { create(:project) } + def create_data_for_commits(*messages) + data = { + object_kind: 'push', + ref: 'master', + user_name: user.name, + commits: messages.map do |m| + { + message: m, + url: 'https://gitlab.com/', + } + end + } + end + before do @asana = AsanaService.new allow(@asana).to receive_messages( @@ -51,12 +65,15 @@ describe AsanaService, models: true do ) end - it 'should call Asana service to created a story' do - d1 = double('Asana::Task', add_comment: true) - expect(d1).to receive(:add_comment) + it 'should call Asana service to create a story' do + data = create_data_for_commits('Message from commit. related to #123456') + expected_message = "#{data[:user_name]} pushed to branch #{data[:ref]} of #{project.name_with_namespace} ( #{data[:commits][0][:url]} ): #{data[:commits][0][:message]}" + + d1 = double('Asana::Task') + expect(d1).to receive(:add_comment).with(text: expected_message) expect(Asana::Task).to receive(:find_by_id).with(anything, '123456').once.and_return(d1) - @asana.check_commit('related to #123456', 'pushed') + @asana.execute(data) end it 'should call Asana service to created a story and close a task' do From 571df5f44bfec89b21bdce0f91f9acfdda6d7660 Mon Sep 17 00:00:00 2001 From: Mike Wyatt Date: Thu, 31 Dec 2015 18:29:00 -0400 Subject: [PATCH 15/23] Use `execute` in Asana specs --- .../project_services/asana_service_spec.rb | 61 ++++++++++--------- 1 file changed, 32 insertions(+), 29 deletions(-) diff --git a/spec/models/project_services/asana_service_spec.rb b/spec/models/project_services/asana_service_spec.rb index a7b32ac07a9..0db48c75d1d 100644 --- a/spec/models/project_services/asana_service_spec.rb +++ b/spec/models/project_services/asana_service_spec.rb @@ -76,53 +76,56 @@ describe AsanaService, models: true do @asana.execute(data) end - it 'should call Asana service to created a story and close a task' do - d1 = double('Asana::Task', add_comment: true) + it 'should call Asana service to create a story and close a task' do + data = create_data_for_commits('fix #456789') + d1 = double('Asana::Task') expect(d1).to receive(:add_comment) expect(d1).to receive(:update).with(completed: true) expect(Asana::Task).to receive(:find_by_id).with(anything, '456789').once.and_return(d1) - @asana.check_commit('fix #456789', 'pushed') + @asana.execute(data) end it 'should be able to close via url' do - d1 = double('Asana::Task', add_comment: true) + data = create_data_for_commits('closes https://app.asana.com/19292/956299/42') + d1 = double('Asana::Task') expect(d1).to receive(:add_comment) expect(d1).to receive(:update).with(completed: true) expect(Asana::Task).to receive(:find_by_id).with(anything, '42').once.and_return(d1) - @asana.check_commit('closes https://app.asana.com/19292/956299/42', 'pushed') + @asana.execute(data) end it 'should allow multiple matches per line' do - d1 = double('Asana::Task', add_comment: true) - expect(d1).to receive(:add_comment) - expect(d1).to receive(:update).with(completed: true) - expect(Asana::Task).to receive(:find_by_id).with(anything, '123').once.and_return(d1) - - d2 = double('Asana::Task', add_comment: true) - expect(d2).to receive(:add_comment) - expect(d2).to receive(:update).with(completed: true) - expect(Asana::Task).to receive(:find_by_id).with(anything, '456').once.and_return(d2) - - d3 = double('Asana::Task', add_comment: true) - expect(d3).to receive(:add_comment) - expect(Asana::Task).to receive(:find_by_id).with(anything, '789').once.and_return(d3) - - d4 = double('Asana::Task', add_comment: true) - expect(d4).to receive(:add_comment) - expect(Asana::Task).to receive(:find_by_id).with(anything, '42').once.and_return(d4) - - d5 = double('Asana::Task', add_comment: true) - expect(d5).to receive(:add_comment) - expect(d5).to receive(:update).with(completed: true) - expect(Asana::Task).to receive(:find_by_id).with(anything, '12').once.and_return(d5) - message = <<-EOF minor bigfix, refactoring, fixed #123 and Closes #456 work on #789 ref https://app.asana.com/19292/956299/42 and closing https://app.asana.com/19292/956299/12 EOF - @asana.check_commit(message, 'pushed') + data = create_data_for_commits(message) + d1 = double('Asana::Task') + expect(d1).to receive(:add_comment) + expect(d1).to receive(:update).with(completed: true) + expect(Asana::Task).to receive(:find_by_id).with(anything, '123').once.and_return(d1) + + d2 = double('Asana::Task') + expect(d2).to receive(:add_comment) + expect(d2).to receive(:update).with(completed: true) + expect(Asana::Task).to receive(:find_by_id).with(anything, '456').once.and_return(d2) + + d3 = double('Asana::Task') + expect(d3).to receive(:add_comment) + expect(Asana::Task).to receive(:find_by_id).with(anything, '789').once.and_return(d3) + + d4 = double('Asana::Task') + expect(d4).to receive(:add_comment) + expect(Asana::Task).to receive(:find_by_id).with(anything, '42').once.and_return(d4) + + d5 = double('Asana::Task') + expect(d5).to receive(:add_comment) + expect(d5).to receive(:update).with(completed: true) + expect(Asana::Task).to receive(:find_by_id).with(anything, '12').once.and_return(d5) + + @asana.execute(data) end end end From bb6b793c55150ecbe072456bc4b151191764b642 Mon Sep 17 00:00:00 2001 From: Mike Wyatt Date: Mon, 4 Jan 2016 11:19:39 -0400 Subject: [PATCH 16/23] Don't log backtrace in Asana service --- app/models/project_services/asana_service.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/models/project_services/asana_service.rb b/app/models/project_services/asana_service.rb index cb4f6ddb3a5..7d367e40037 100644 --- a/app/models/project_services/asana_service.rb +++ b/app/models/project_services/asana_service.rb @@ -118,7 +118,6 @@ http://app.asana.com/-/account_api' end rescue => e Rails.logger.error(e.message) - Rails.logger.error(e.backtrace.join("\n")) next end end From 6ce01ca3ece007f135c6b5a9bc258fdd9e8d71de Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 5 Jan 2016 11:00:10 -0200 Subject: [PATCH 17/23] Validate README format before displaying MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Take the first previewable README file as project’s README, otherwise if none file is available, or we can’t preview any of them, we assume that project doesn’t have a README file. --- app/models/tree.rb | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/app/models/tree.rb b/app/models/tree.rb index 93b3246a668..e0e04d8859f 100644 --- a/app/models/tree.rb +++ b/app/models/tree.rb @@ -17,17 +17,15 @@ class Tree def readme return @readme if defined?(@readme) - available_readmes = blobs.select(&:readme?) - - if available_readmes.count == 0 - return @readme = nil + # Take the first previewable readme, or return nil if none is available or + # we can't preview any of them + readme_tree = blobs.find do |blob| + blob.readme? && (previewable?(blob.name) || plain?(blob.name)) end - # Take the first previewable readme, or the first available readme, if we - # can't preview any of them - readme_tree = available_readmes.find do |readme| - previewable?(readme.name) - end || available_readmes.first + if readme_tree.nil? + return @readme = nil + end readme_path = path == '/' ? readme_tree.name : File.join(path, readme_tree.name) From 8e33ec1deea60a176cd43092c913200d28ea04fe Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 5 Jan 2016 11:30:30 -0200 Subject: [PATCH 18/23] [ci skip] Update CHANGELOG --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index 47ef06bee54..72698e4fe3f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -18,6 +18,7 @@ v 8.4.0 (unreleased) - Fix version check image in Safari - Show 'All' tab by default in the builds page - Fix API project lookups when querying with a namespace with dots (Stan Hu) + - Validate README format before displaying v 8.3.3 (unreleased) - Fix project transfer e-mail sending incorrect paths in e-mail notification (Stan Hu) From 0bab4788ef870945feccbb102834fd89433dfef2 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 5 Jan 2016 16:31:05 +0100 Subject: [PATCH 19/23] Satisfy Rubocop --- spec/models/project_services/asana_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/project_services/asana_service_spec.rb b/spec/models/project_services/asana_service_spec.rb index 0db48c75d1d..f3d15f3c1ea 100644 --- a/spec/models/project_services/asana_service_spec.rb +++ b/spec/models/project_services/asana_service_spec.rb @@ -41,7 +41,7 @@ describe AsanaService, models: true do let(:project) { create(:project) } def create_data_for_commits(*messages) - data = { + { object_kind: 'push', ref: 'master', user_name: user.name, From b541446389c5d450cc7220b949d29d70528a51f1 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 5 Jan 2016 18:17:55 +0100 Subject: [PATCH 20/23] Fix notification spec --- spec/services/notification_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index b5c7b01357a..6d219f35895 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -53,7 +53,7 @@ describe NotificationService, services: true do add_users_with_subscription(note.project, issue) # Ensure create SentNotification by noteable = issue 6 times, not noteable = note - expect(SentNotification).to receive(:record).with(issue, any_args).exactly(6).times + expect(SentNotification).to receive(:record).with(issue, any_args).exactly(7).times ActionMailer::Base.deliveries.clear From 8393e3e04b8b336ec80cb0da49d273f709043120 Mon Sep 17 00:00:00 2001 From: Michi302 Date: Tue, 5 Jan 2016 19:05:41 +0100 Subject: [PATCH 21/23] Add missing changelog entry --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index 57f0b9f30d5..698f4e5f1cf 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -12,6 +12,7 @@ v 8.4.0 (unreleased) - Add CAS support (tduehr) - Add link to merge request on build detail page. - Revert back upvote and downvote button to the issue and MR pages + - Change single user API endpoint to return more detailed data (Michael Potthoff) v 8.3.2 (unreleased) - Disable --follow in `git log` to avoid loading duplicate commit data in infinite scroll (Stan Hu) From 045e8cc38c608b46924ce1ef4de3e8c2adefb1ba Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 29 Dec 2015 20:23:07 -0500 Subject: [PATCH 22/23] Update version check images to use SVG --- CHANGELOG | 1 + lib/version_check.rb | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index b0972ceab68..ed45042eb13 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -18,6 +18,7 @@ v 8.4.0 (unreleased) - Fix version check image in Safari - Show 'All' tab by default in the builds page - Fix API project lookups when querying with a namespace with dots (Stan Hu) + - Update version check images to use SVG v 8.3.3 (unreleased) - Fix project transfer e-mail sending incorrect paths in e-mail notification (Stan Hu) diff --git a/lib/version_check.rb b/lib/version_check.rb index ea23344948c..91ad07feee5 100644 --- a/lib/version_check.rb +++ b/lib/version_check.rb @@ -13,6 +13,6 @@ class VersionCheck end def host - 'https://version.gitlab.com/check.png' + 'https://version.gitlab.com/check.svg' end end From 52e41dcac4d0f76de77029aae07fce60b61d86ef Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 5 Jan 2016 18:02:12 -0500 Subject: [PATCH 23/23] Fix the abuse report detail URL in the HTML email template --- app/views/abuse_report_mailer/notify.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/abuse_report_mailer/notify.html.haml b/app/views/abuse_report_mailer/notify.html.haml index 619533e09a7..2741eb44357 100644 --- a/app/views/abuse_report_mailer/notify.html.haml +++ b/app/views/abuse_report_mailer/notify.html.haml @@ -8,4 +8,4 @@ = @abuse_report.message %p - = link_to "View details", abuse_reports_url + = link_to "View details", admin_abuse_reports_url