From e565be241f39cf4119cb1d3e2668455ed11630fe Mon Sep 17 00:00:00 2001 From: KennyTM~ Date: Sat, 2 Feb 2013 15:25:57 +0800 Subject: [PATCH 01/51] =?UTF-8?q?Show=20only=20=E2=89=A416=20lines=20of=20?= =?UTF-8?q?code=20in=20a=20discussion=20(fix=20issue=20#2860).?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/helpers/commits_helper.rb | 25 ++++++++++++++++++++++ app/views/notes/_discussion_diff.html.haml | 3 +-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index 6d2ce2fe..acdd48e0 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -57,6 +57,31 @@ module CommitsHelper end end + def each_diff_line_near(diff, index, expected_line_code) + max_number_of_lines = 16 + + prev_match_line = nil + prev_lines = [] + + each_diff_line(diff, index) do |full_line, type, line_code, line_new, line_old| + line = [full_line, type, line_code, line_new, line_old] + if line_code != expected_line_code + if type == "match" + prev_lines.clear + prev_match_line = line + else + prev_lines.push(line) + prev_lines.shift if prev_lines.length >= max_number_of_lines + end + else + yield(prev_match_line) if !prev_match_line.nil? + prev_lines.each { |ln| yield(ln) } + yield(line) + break + end + end + end + def image_diff_class(diff) if diff.deleted_file "deleted" diff --git a/app/views/notes/_discussion_diff.html.haml b/app/views/notes/_discussion_diff.html.haml index 790b7733..20bdb3f3 100644 --- a/app/views/notes/_discussion_diff.html.haml +++ b/app/views/notes/_discussion_diff.html.haml @@ -9,7 +9,7 @@ %br/ .content %table - - each_diff_line(diff, note.diff_file_index) do |line, type, line_code, line_new, line_old| + - each_diff_line_near(diff, note.diff_file_index, note.line_code) do |line, type, line_code, line_new, line_old| %tr.line_holder{ id: line_code } - if type == "match" %td.old_line= "..." @@ -22,4 +22,3 @@ - if line_code == note.line_code = render "notes/diff_notes_with_reply", notes: discussion_notes - - break # cut off diff after notes From 468c8c5f0a66a9ebf1489926ba32c19db71d821a Mon Sep 17 00:00:00 2001 From: Andrew8xx8 Date: Wed, 20 Feb 2013 13:15:56 +0400 Subject: [PATCH 02/51] A little bit of codestyle improvments --- app/observers/system_hook_observer.rb | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/app/observers/system_hook_observer.rb b/app/observers/system_hook_observer.rb index 312cd2b3..195a6b12 100644 --- a/app/observers/system_hook_observer.rb +++ b/app/observers/system_hook_observer.rb @@ -1,8 +1,9 @@ class SystemHookObserver < ActiveRecord::Observer observe :user, :project, :users_project - + def after_create(model) - if model.kind_of? Project + case model + when Project SystemHook.all_hooks_fire({ event_name: "project_create", name: model.name, @@ -12,15 +13,14 @@ class SystemHookObserver < ActiveRecord::Observer owner_email: model.owner.email, created_at: model.created_at }) - elsif model.kind_of? User + when User SystemHook.all_hooks_fire({ event_name: "user_create", name: model.name, email: model.email, created_at: model.created_at }) - - elsif model.kind_of? UsersProject + when UsersProject SystemHook.all_hooks_fire({ event_name: "user_add_to_team", project_name: model.project.name, @@ -31,12 +31,12 @@ class SystemHookObserver < ActiveRecord::Observer project_access: model.repo_access_human, created_at: model.created_at }) - end end def after_destroy(model) - if model.kind_of? Project + case model + when Project SystemHook.all_hooks_fire({ event_name: "project_destroy", name: model.name, @@ -45,14 +45,13 @@ class SystemHookObserver < ActiveRecord::Observer owner_name: model.owner.name, owner_email: model.owner.email, }) - elsif model.kind_of? User + when User SystemHook.all_hooks_fire({ event_name: "user_destroy", name: model.name, email: model.email }) - - elsif model.kind_of? UsersProject + when UsersProject SystemHook.all_hooks_fire({ event_name: "user_remove_from_team", project_name: model.project.name, From aa1780d03c7ceb916c2e122b841d3d4ebc5ce597 Mon Sep 17 00:00:00 2001 From: Andrew8xx8 Date: Wed, 20 Feb 2013 14:53:15 +0400 Subject: [PATCH 03/51] System hooks execution moved to System hook service --- app/models/system_hook.rb | 6 --- app/observers/system_hook_observer.rb | 59 +--------------------- app/services/system_hooks_service.rb | 55 ++++++++++++++++++++ spec/services/system_hooks_service_spec.rb | 41 +++++++++++++++ 4 files changed, 98 insertions(+), 63 deletions(-) create mode 100644 app/services/system_hooks_service.rb create mode 100644 spec/services/system_hooks_service_spec.rb diff --git a/app/models/system_hook.rb b/app/models/system_hook.rb index 5f1bd647..3914a915 100644 --- a/app/models/system_hook.rb +++ b/app/models/system_hook.rb @@ -12,12 +12,6 @@ # class SystemHook < WebHook - def self.all_hooks_fire(data) - SystemHook.all.each do |sh| - sh.async_execute data - end - end - def async_execute(data) Sidekiq::Client.enqueue(SystemHookWorker, id, data) end diff --git a/app/observers/system_hook_observer.rb b/app/observers/system_hook_observer.rb index 195a6b12..be2594b4 100644 --- a/app/observers/system_hook_observer.rb +++ b/app/observers/system_hook_observer.rb @@ -2,65 +2,10 @@ class SystemHookObserver < ActiveRecord::Observer observe :user, :project, :users_project def after_create(model) - case model - when Project - SystemHook.all_hooks_fire({ - event_name: "project_create", - name: model.name, - path: model.path, - project_id: model.id, - owner_name: model.owner.name, - owner_email: model.owner.email, - created_at: model.created_at - }) - when User - SystemHook.all_hooks_fire({ - event_name: "user_create", - name: model.name, - email: model.email, - created_at: model.created_at - }) - when UsersProject - SystemHook.all_hooks_fire({ - event_name: "user_add_to_team", - project_name: model.project.name, - project_path: model.project.path, - project_id: model.project_id, - user_name: model.user.name, - user_email: model.user.email, - project_access: model.repo_access_human, - created_at: model.created_at - }) - end + SystemHooksService.execute_hooks_for(model, :create) end def after_destroy(model) - case model - when Project - SystemHook.all_hooks_fire({ - event_name: "project_destroy", - name: model.name, - path: model.path, - project_id: model.id, - owner_name: model.owner.name, - owner_email: model.owner.email, - }) - when User - SystemHook.all_hooks_fire({ - event_name: "user_destroy", - name: model.name, - email: model.email - }) - when UsersProject - SystemHook.all_hooks_fire({ - event_name: "user_remove_from_team", - project_name: model.project.name, - project_path: model.project.path, - project_id: model.project_id, - user_name: model.user.name, - user_email: model.user.email, - project_access: model.repo_access_human - }) - end + SystemHooksService.execute_hooks_for(model, :destroy) end end diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb new file mode 100644 index 00000000..1d53f3ba --- /dev/null +++ b/app/services/system_hooks_service.rb @@ -0,0 +1,55 @@ +class SystemHooksService + def self.execute_hooks_for(model, event) + execute_hooks(build_event_data(model, event)) + end + + private + + def self.execute_hooks(data) + SystemHook.all.each do |sh| + sh.async_execute data + end + end + + def self.build_event_data(model, event) + data = { + event_name: build_event_name(model, event), + created_at: model.created_at + } + + case model + when Project + data.merge!({ + name: model.name, + path: model.path, + project_id: model.id, + owner_name: model.owner.name, + owner_email: model.owner.email + }) + when User + data.merge!({ + name: model.name, + email: model.email + }) + when UsersProject + data.merge!({ + project_name: model.project.name, + project_path: model.project.path, + project_id: model.project_id, + user_name: model.user.name, + user_email: model.user.email, + project_access: model.repo_access_human + }) + end + end + + def self.build_event_name(model, event) + case model + when UsersProject + return "user_add_to_team" if event == :create + return "user_remove_from_team" if event == :destroy + else + "#{model.class.name.downcase}_#{event.to_s}" + end + end +end diff --git a/spec/services/system_hooks_service_spec.rb b/spec/services/system_hooks_service_spec.rb new file mode 100644 index 00000000..7f1590f5 --- /dev/null +++ b/spec/services/system_hooks_service_spec.rb @@ -0,0 +1,41 @@ +require 'spec_helper' + +describe SystemHooksService do + let (:user) { create :user } + let (:project) { create :project } + let (:users_project) { create :users_project } + + context 'it should build event data' do + it 'should build event data for user' do + SystemHooksService.build_event_data(user, :create).should include(:event_name, :name, :created_at, :email) + end + + it 'should build event data for project' do + SystemHooksService.build_event_data(project, :create).should include(:event_name, :name, :created_at, :path, :project_id, :owner_name, :owner_email) + end + + it 'should build event data for users project' do + SystemHooksService.build_event_data(users_project, :create).should include(:event_name, :created_at, :project_name, :project_path, :project_id, :user_name, :user_email, :project_access) + end + end + + context 'it should build event names' do + it 'should build event names for user' do + SystemHooksService.build_event_name(user, :create).should eq "user_create" + + SystemHooksService.build_event_name(user, :destroy).should eq "user_destroy" + end + + it 'should build event names for project' do + SystemHooksService.build_event_name(project, :create).should eq "project_create" + + SystemHooksService.build_event_name(project, :destroy).should eq "project_destroy" + end + + it 'should build event names for users project' do + SystemHooksService.build_event_name(users_project, :create).should eq "user_add_to_team" + + SystemHooksService.build_event_name(users_project, :destroy).should eq "user_remove_from_team" + end + end +end From 99760edc757b24796d5db1f5328d55f483e4c33c Mon Sep 17 00:00:00 2001 From: Andrew8xx8 Date: Wed, 20 Feb 2013 15:33:03 +0400 Subject: [PATCH 04/51] Method moved to service --- app/models/system_hook.rb | 3 --- app/services/system_hooks_service.rb | 6 +++++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/app/models/system_hook.rb b/app/models/system_hook.rb index 3914a915..5cdf0466 100644 --- a/app/models/system_hook.rb +++ b/app/models/system_hook.rb @@ -12,7 +12,4 @@ # class SystemHook < WebHook - def async_execute(data) - Sidekiq::Client.enqueue(SystemHookWorker, id, data) - end end diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb index 1d53f3ba..6043bac6 100644 --- a/app/services/system_hooks_service.rb +++ b/app/services/system_hooks_service.rb @@ -7,10 +7,14 @@ class SystemHooksService def self.execute_hooks(data) SystemHook.all.each do |sh| - sh.async_execute data + async_execute_hook sh, data end end + def self.async_execute_hook(hook, data) + Sidekiq::Client.enqueue(SystemHookWorker, hook, data) + end + def self.build_event_data(model, event) data = { event_name: build_event_name(model, event), From 8ba27b7b463e6691c75528748590a1cd9b9651c9 Mon Sep 17 00:00:00 2001 From: Andrew8xx8 Date: Wed, 20 Feb 2013 17:13:18 +0400 Subject: [PATCH 05/51] Migrations for convertion merge_status added --- ...204_add_new_merge_status_to_merge_request.rb | 5 +++++ ...544_convert_merge_status_in_merge_request.rb | 17 +++++++++++++++++ ...45_remove_merge_status_from_merge_request.rb | 9 +++++++++ ...merge_status_to_merge_status_in_milestone.rb | 5 +++++ 4 files changed, 36 insertions(+) create mode 100644 db/migrate/20130220124204_add_new_merge_status_to_merge_request.rb create mode 100644 db/migrate/20130220125544_convert_merge_status_in_merge_request.rb create mode 100644 db/migrate/20130220125545_remove_merge_status_from_merge_request.rb create mode 100644 db/migrate/20130220133245_rename_new_merge_status_to_merge_status_in_milestone.rb diff --git a/db/migrate/20130220124204_add_new_merge_status_to_merge_request.rb b/db/migrate/20130220124204_add_new_merge_status_to_merge_request.rb new file mode 100644 index 00000000..d78bd0ae --- /dev/null +++ b/db/migrate/20130220124204_add_new_merge_status_to_merge_request.rb @@ -0,0 +1,5 @@ +class AddNewMergeStatusToMergeRequest < ActiveRecord::Migration + def change + add_column :merge_requests, :new_merge_status, :string + end +end diff --git a/db/migrate/20130220125544_convert_merge_status_in_merge_request.rb b/db/migrate/20130220125544_convert_merge_status_in_merge_request.rb new file mode 100644 index 00000000..b310b35e --- /dev/null +++ b/db/migrate/20130220125544_convert_merge_status_in_merge_request.rb @@ -0,0 +1,17 @@ +class ConvertMergeStatusInMergeRequest < ActiveRecord::Migration + def up + MergeRequest.transaction do + MergeRequest.where(merge_status: 1).update_all("new_merge_status = 'unchecked'") + MergeRequest.where(merge_status: 2).update_all("new_merge_status = 'can_be_merged'") + MergeRequest.where(merge_status: 3).update_all("new_merge_status = 'cannot_be_merged'") + end + end + + def down + MergeRequest.transaction do + MergeRequest.where(new_merge_status: :unchecked).update_all("merge_status = 1") + MergeRequest.where(new_merge_status: :can_be_merged).update_all("merge_status = 2") + MergeRequest.where(new_merge_status: :cannot_be_merged).update_all("merge_status = 3") + end + end +end diff --git a/db/migrate/20130220125545_remove_merge_status_from_merge_request.rb b/db/migrate/20130220125545_remove_merge_status_from_merge_request.rb new file mode 100644 index 00000000..9083183b --- /dev/null +++ b/db/migrate/20130220125545_remove_merge_status_from_merge_request.rb @@ -0,0 +1,9 @@ +class RemoveMergeStatusFromMergeRequest < ActiveRecord::Migration + def up + remove_column :merge_requests, :merge_status + end + + def down + add_column :merge_requests, :merge_status, :integer + end +end diff --git a/db/migrate/20130220133245_rename_new_merge_status_to_merge_status_in_milestone.rb b/db/migrate/20130220133245_rename_new_merge_status_to_merge_status_in_milestone.rb new file mode 100644 index 00000000..3f8f38dc --- /dev/null +++ b/db/migrate/20130220133245_rename_new_merge_status_to_merge_status_in_milestone.rb @@ -0,0 +1,5 @@ +class RenameNewMergeStatusToMergeStatusInMilestone < ActiveRecord::Migration + def change + rename_column :merge_requests, :new_merge_status, :merge_status + end +end From 1b7b17d12a9f176dd305b9ec95bd50ba3f56f520 Mon Sep 17 00:00:00 2001 From: Andrew8xx8 Date: Wed, 20 Feb 2013 17:13:42 +0400 Subject: [PATCH 06/51] Database schema updated --- db/schema.rb | 46 +++++++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index f837e6ed..04ed7984 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20130218141554) do +ActiveRecord::Schema.define(:version => 20130220133245) do create_table "events", :force => true do |t| t.string "target_type" @@ -68,19 +68,19 @@ ActiveRecord::Schema.define(:version => 20130218141554) do add_index "keys", ["user_id"], :name => "index_keys_on_user_id" create_table "merge_requests", :force => true do |t| - t.string "target_branch", :null => false - t.string "source_branch", :null => false - t.integer "project_id", :null => false + t.string "target_branch", :null => false + t.string "source_branch", :null => false + t.integer "project_id", :null => false t.integer "author_id" t.integer "assignee_id" t.string "title" - t.datetime "created_at", :null => false - t.datetime "updated_at", :null => false + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.text "st_commits", :limit => 2147483647 t.text "st_diffs", :limit => 2147483647 - t.integer "merge_status", :default => 1, :null => false t.integer "milestone_id" t.string "state" + t.string "merge_status" end add_index "merge_requests", ["assignee_id"], :name => "index_merge_requests_on_assignee_id" @@ -106,12 +106,13 @@ ActiveRecord::Schema.define(:version => 20130218141554) do add_index "milestones", ["project_id"], :name => "index_milestones_on_project_id" create_table "namespaces", :force => true do |t| - t.string "name", :null => false - t.string "path", :null => false - t.integer "owner_id", :null => false - t.datetime "created_at", :null => false - t.datetime "updated_at", :null => false + t.string "name", :null => false + t.string "path", :null => false + t.integer "owner_id", :null => false + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.string "type" + t.string "description", :default => "", :null => false end add_index "namespaces", ["name"], :name => "index_namespaces_on_name" @@ -142,16 +143,18 @@ ActiveRecord::Schema.define(:version => 20130218141554) do t.string "name" t.string "path" t.text "description" - t.datetime "created_at", :null => false - t.datetime "updated_at", :null => false + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.integer "creator_id" t.string "default_branch" - t.boolean "issues_enabled", :default => true, :null => false - t.boolean "wall_enabled", :default => true, :null => false - t.boolean "merge_requests_enabled", :default => true, :null => false - t.boolean "wiki_enabled", :default => true, :null => false + t.boolean "issues_enabled", :default => true, :null => false + t.boolean "wall_enabled", :default => true, :null => false + t.boolean "merge_requests_enabled", :default => true, :null => false + t.boolean "wiki_enabled", :default => true, :null => false t.integer "namespace_id" - t.boolean "public", :default => false, :null => false + t.boolean "public", :default => false, :null => false + t.string "issues_tracker", :default => "gitlab", :null => false + t.string "issues_tracker_id" end add_index "projects", ["creator_id"], :name => "index_projects_on_owner_id" @@ -230,8 +233,9 @@ ActiveRecord::Schema.define(:version => 20130218141554) do t.string "name" t.string "path" t.integer "owner_id" - t.datetime "created_at", :null => false - t.datetime "updated_at", :null => false + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false + t.string "description", :default => "", :null => false end create_table "users", :force => true do |t| From e2d94e0719cb4eed4757f4cef946b1c29ef971f0 Mon Sep 17 00:00:00 2001 From: Andrew8xx8 Date: Wed, 20 Feb 2013 17:15:01 +0400 Subject: [PATCH 07/51] State machine added for merge_status field --- app/models/merge_request.rb | 63 +++++++++++------------- app/views/merge_requests/_show.html.haml | 2 +- spec/models/merge_request_spec.rb | 6 +++ 3 files changed, 35 insertions(+), 36 deletions(-) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 06aa9f3c..05ebf44c 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -53,16 +53,32 @@ class MergeRequest < ActiveRecord::Base BROKEN_DIFF = "--broken-diff" - UNCHECKED = 1 - CAN_BE_MERGED = 2 - CANNOT_BE_MERGED = 3 + state_machine :merge_status, initial: :unchecked do + event :mark_as_unchecked do + transition [:can_be_merged, :cannot_be_merged] => :unchecked + end + + event :mark_as_mergeable do + transition unchecked: :can_be_merged + end + + event :mark_as_unmergeable do + transition unchecked: :cannot_be_merged + end + + state :unchecked + + state :can_be_merged + + state :cannot_be_merged + end serialize :st_commits serialize :st_diffs validates :source_branch, presence: true validates :target_branch, presence: true - validate :validate_branches + validate :validate_branches scope :merged, -> { with_state(:merged) } @@ -84,13 +100,9 @@ class MergeRequest < ActiveRecord::Base end end + # DEPRECATED: Please use human_merge_status_name instead def human_merge_status - merge_statuses = { - CAN_BE_MERGED => "can_be_merged", - CANNOT_BE_MERGED => "cannot_be_merged", - UNCHECKED => "unchecked" - } - merge_statuses[self.merge_status] + human_merge_status_name end def validate_branches @@ -104,26 +116,12 @@ class MergeRequest < ActiveRecord::Base self.reloaded_diffs end - def unchecked? - merge_status == UNCHECKED - end - - def mark_as_unchecked - self.merge_status = UNCHECKED - self.save - end - - def can_be_merged? - merge_status == CAN_BE_MERGED - end - def check_if_can_be_merged - self.merge_status = if Gitlab::Satellite::MergeAction.new(self.author, self).can_be_merged? - CAN_BE_MERGED - else - CANNOT_BE_MERGED - end - self.save + if Gitlab::Satellite::MergeAction.new(self.author, self).can_be_merged? + mark_as_mergeable + else + mark_as_unmergeable + end end def diffs @@ -178,11 +176,6 @@ class MergeRequest < ActiveRecord::Base commits.any? && opened? end - def mark_as_unmergable - self.merge_status = CANNOT_BE_MERGED - self.save - end - def reloaded_commits if opened? && unmerged_commits.any? self.st_commits = unmerged_commits @@ -217,7 +210,7 @@ class MergeRequest < ActiveRecord::Base true end rescue - self.mark_as_unmergable + mark_as_unmergeable false end diff --git a/app/views/merge_requests/_show.html.haml b/app/views/merge_requests/_show.html.haml index ae2cfe92..b1e282a8 100644 --- a/app/views/merge_requests/_show.html.haml +++ b/app/views/merge_requests/_show.html.haml @@ -29,7 +29,7 @@ $(function(){ merge_request = new MergeRequest({ url_to_automerge_check: "#{automerge_check_project_merge_request_path(@project, @merge_request)}", - check_enable: #{@merge_request.merge_status == MergeRequest::UNCHECKED ? "true" : "false"}, + check_enable: #{@merge_request.unchecked? ? "true" : "false"}, url_to_ci_check: "#{ci_status_project_merge_request_path(@project, @merge_request)}", ci_enable: #{@project.gitlab_ci? ? "true" : "false"}, current_status: "#{@merge_request.human_merge_status}", diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index e61bf44c..dbae019e 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -32,6 +32,12 @@ describe MergeRequest do it { should_not allow_mass_assignment_of(:project_id) } end + describe "Respond to" do + it { should respond_to(:unchecked?) } + it { should respond_to(:can_be_merged?) } + it { should respond_to(:cannot_be_merged?) } + end + describe 'modules' do it { should include_module(Issuable) } end From 52e0df5c23b94cc8f2929a97bc2211fd51bc1de4 Mon Sep 17 00:00:00 2001 From: Andrew8xx8 Date: Wed, 20 Feb 2013 17:37:20 +0400 Subject: [PATCH 08/51] class.self methods moved to scopes --- app/models/merge_request.rb | 21 +++------------------ app/models/project.rb | 2 +- 2 files changed, 4 insertions(+), 19 deletions(-) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 05ebf44c..e8e15aec 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -81,24 +81,9 @@ class MergeRequest < ActiveRecord::Base validate :validate_branches scope :merged, -> { with_state(:merged) } - - class << self - def find_all_by_branch(branch_name) - where("source_branch LIKE :branch OR target_branch LIKE :branch", branch: branch_name) - end - - def cared(user) - where('assignee_id = :user OR author_id = :user', user: user.id) - end - - def find_all_by_branch(branch_name) - where("source_branch LIKE :branch OR target_branch LIKE :branch", branch: branch_name) - end - - def find_all_by_milestone(milestone) - where("milestone_id = :milestone_id", milestone_id: milestone) - end - end + scope :by_branch, ->(branch_name) { where("source_branch LIKE :branch OR target_branch LIKE :branch", branch: branch_name) } + scope :cared, ->(user) { where('assignee_id = :user OR author_id = :user', user: user.id) } + scope :by_milestone, ->(milestone) { where("milestone_id = :milestone_id", milestone_id: milestone) } # DEPRECATED: Please use human_merge_status_name instead def human_merge_status diff --git a/app/models/project.rb b/app/models/project.rb index b7d688bf..2120f53b 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -424,7 +424,7 @@ class Project < ActiveRecord::Base c_ids = self.repository.commits_between(oldrev, newrev).map(&:id) # Update code for merge requests - mrs = self.merge_requests.opened.find_all_by_branch(branch_name).all + mrs = self.merge_requests.opened.by_branch(branch_name).all mrs.each { |merge_request| merge_request.reload_code; merge_request.mark_as_unchecked } # Close merge requests From 6d68923edceb23ae6a5281e022a351a0d9c0d59b Mon Sep 17 00:00:00 2001 From: Andrew8xx8 Date: Wed, 20 Feb 2013 17:45:29 +0400 Subject: [PATCH 09/51] human_merge_status replaced by human_merge_status_name --- app/controllers/merge_requests_controller.rb | 2 +- app/models/merge_request.rb | 5 ----- app/views/merge_requests/_show.html.haml | 2 +- 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index bf665272..3ebef869 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -73,7 +73,7 @@ class MergeRequestsController < ProjectResourceController if @merge_request.unchecked? @merge_request.check_if_can_be_merged end - render json: {merge_status: @merge_request.human_merge_status} + render json: {merge_status: @merge_request.human_merge_status_name} rescue Gitlab::SatelliteNotExistError render json: {merge_status: :no_satellite} end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index e8e15aec..fad6f9c4 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -85,11 +85,6 @@ class MergeRequest < ActiveRecord::Base scope :cared, ->(user) { where('assignee_id = :user OR author_id = :user', user: user.id) } scope :by_milestone, ->(milestone) { where("milestone_id = :milestone_id", milestone_id: milestone) } - # DEPRECATED: Please use human_merge_status_name instead - def human_merge_status - human_merge_status_name - end - def validate_branches if target_branch == source_branch errors.add :base, "You can not use same branch for source and target branches" diff --git a/app/views/merge_requests/_show.html.haml b/app/views/merge_requests/_show.html.haml index b1e282a8..6b6100f5 100644 --- a/app/views/merge_requests/_show.html.haml +++ b/app/views/merge_requests/_show.html.haml @@ -32,7 +32,7 @@ check_enable: #{@merge_request.unchecked? ? "true" : "false"}, url_to_ci_check: "#{ci_status_project_merge_request_path(@project, @merge_request)}", ci_enable: #{@project.gitlab_ci? ? "true" : "false"}, - current_status: "#{@merge_request.human_merge_status}", + current_status: "#{@merge_request.human_merge_status_name}", action: "#{controller.action_name}" }); }); From a3bbc5956bdecc39846ede219fa27b3311e2192b Mon Sep 17 00:00:00 2001 From: Axilleas Pipinellis Date: Wed, 20 Feb 2013 17:36:09 +0200 Subject: [PATCH 10/51] We don't need to check .profile now that gitolite is replaced by gitlab-shell --- lib/tasks/gitlab/check.rake | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake index 6a138396..1384231f 100644 --- a/lib/tasks/gitlab/check.rake +++ b/lib/tasks/gitlab/check.rake @@ -255,7 +255,6 @@ namespace :gitlab do warn_user_is_not_gitlab start_checking "Environment" - check_issue_1059_shell_profile_error check_gitlab_git_config check_python2_exists check_python2_version @@ -294,30 +293,6 @@ namespace :gitlab do end end - # see https://github.com/gitlabhq/gitlabhq/issues/1059 - def check_issue_1059_shell_profile_error - gitlab_shell_ssh_user = Gitlab.config.gitlab_shell.ssh_user - print "Has no \"-e\" in ~#{gitlab_shell_ssh_user}/.profile ... " - - profile_file = File.join(gitlab_shell_user_home, ".profile") - - unless File.read(profile_file) =~ /^-e PATH/ - puts "yes".green - else - puts "no".red - try_fixing_it( - "Open #{profile_file}", - "Find the line starting with \"-e PATH\"", - "Remove \"-e \" so the line starts with PATH" - ) - for_more_information( - see_installation_guide_section("Gitlab Shell"), - "https://github.com/gitlabhq/gitlabhq/issues/1059" - ) - fix_and_rerun - end - end - def check_python2_exists print "Has python2? ... " From 585259b8370dd50e732aac188ee7887dc1a5f4f2 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 21 Feb 2013 09:25:07 +0200 Subject: [PATCH 11/51] Fix merge state detection --- app/assets/javascripts/merge_requests.js.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/merge_requests.js.coffee b/app/assets/javascripts/merge_requests.js.coffee index 496da731..890ca400 100644 --- a/app/assets/javascripts/merge_requests.js.coffee +++ b/app/assets/javascripts/merge_requests.js.coffee @@ -31,7 +31,7 @@ class MergeRequest if this.$('.automerge_widget').length and @opts.check_enable $.get @opts.url_to_automerge_check, (data) => - this.showState( data.state ) + this.showState( data.merge_status ) , 'json' if @opts.ci_enable From af3138e8013af5f38f3dae6a3e0eee68f382a0ee Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 21 Feb 2013 10:00:18 +0200 Subject: [PATCH 12/51] Update sidekiq --- Gemfile | 4 ++-- Gemfile.lock | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Gemfile b/Gemfile index 4a7db0eb..9f4a972b 100644 --- a/Gemfile +++ b/Gemfile @@ -81,8 +81,8 @@ gem "draper", "~> 0.18.0" # Background jobs gem 'slim' -gem 'sinatra', :require => nil -gem 'sidekiq', '2.6.4' +gem 'sinatra', require: nil +gem 'sidekiq', '2.7.3' # HTTP requests gem "httparty" diff --git a/Gemfile.lock b/Gemfile.lock index 1b8c5837..92cf8e1f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -393,7 +393,7 @@ GEM sexp_processor (4.1.3) shoulda-matchers (1.3.0) activesupport (>= 3.0.0) - sidekiq (2.6.4) + sidekiq (2.7.3) celluloid (~> 0.12.0) connection_pool (~> 1.0) multi_json (~> 1) @@ -436,7 +436,7 @@ GEM rack (>= 1.0.0) thor (0.17.0) tilt (1.3.3) - timers (1.0.2) + timers (1.1.0) treetop (1.4.12) polyglot polyglot (>= 0.3.1) @@ -530,7 +530,7 @@ DEPENDENCIES seed-fu settingslogic shoulda-matchers (= 1.3.0) - sidekiq (= 2.6.4) + sidekiq (= 2.7.3) simplecov sinatra six From 830da0c2181eee0b65d7e51d1e2854580cc9d76a Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 21 Feb 2013 10:28:13 +0200 Subject: [PATCH 13/51] Update capybara, poltergeist, spinach, rspec-rails --- Gemfile | 8 ++++---- Gemfile.lock | 58 ++++++++++++++++++++++------------------------------ 2 files changed, 28 insertions(+), 38 deletions(-) diff --git a/Gemfile b/Gemfile index 9f4a972b..ec200e17 100644 --- a/Gemfile +++ b/Gemfile @@ -134,9 +134,9 @@ end group :development, :test do gem 'rails-dev-tweaks' - gem 'spinach-rails' - gem "rspec-rails" - gem "capybara" + gem 'spinach-rails', '0.2.0' + gem "rspec-rails", '2.12.2' + gem "capybara", '2.0.2' gem "pry" gem "awesome_print" gem "database_cleaner", ref: "f89c34300e114be99532f14c115b2799a3380ac6", git: "https://github.com/bmabey/database_cleaner.git" @@ -153,7 +153,7 @@ group :development, :test do gem 'rb-inotify', require: linux_only('rb-inotify') # PhantomJS driver for Capybara - gem 'poltergeist', git: 'https://github.com/jonleighton/poltergeist.git', ref: '5c2e092001074a8cf09f332d3714e9ba150bc8ca' + gem 'poltergeist', '1.1.0' end group :test do diff --git a/Gemfile.lock b/Gemfile.lock index 92cf8e1f..93187cd5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -54,18 +54,6 @@ GIT specs: raphael-rails (2.1.0) -GIT - remote: https://github.com/jonleighton/poltergeist.git - revision: 5c2e092001074a8cf09f332d3714e9ba150bc8ca - ref: 5c2e092001074a8cf09f332d3714e9ba150bc8ca - specs: - poltergeist (1.0.2) - capybara (~> 1.1) - childprocess (~> 0.3) - faye-websocket (~> 0.4, >= 0.4.4) - http_parser.rb (~> 0.5.3) - multi_json (~> 1.0) - GEM remote: http://rubygems.org/ specs: @@ -110,13 +98,13 @@ GEM bootstrap-sass (2.2.1.1) sass (~> 3.2) builder (3.0.4) - capybara (1.1.3) + capybara (2.0.2) mime-types (>= 1.16) nokogiri (>= 1.3.3) rack (>= 1.0.0) rack-test (>= 0.5.4) selenium-webdriver (~> 2.0) - xpath (~> 0.1.4) + xpath (~> 1.0.0) carrierwave (0.7.1) activemodel (>= 3.2.0) activesupport (>= 3.2.0) @@ -124,8 +112,8 @@ GEM facter (>= 1.6.12) timers (>= 1.0.0) charlock_holmes (0.6.9) - childprocess (0.3.6) - ffi (~> 1.0, >= 1.0.6) + childprocess (0.3.8) + ffi (~> 1.0, >= 1.0.11) chosen-rails (0.9.8) railties (~> 3.0) thor (~> 0.14) @@ -169,10 +157,10 @@ GEM railties (>= 3.0.0) faraday (0.8.4) multipart-post (~> 1.1) - faye-websocket (0.4.6) + faye-websocket (0.4.7) eventmachine (>= 0.12.0) ffaker (1.15.0) - ffi (1.1.5) + ffi (1.4.0) font-awesome-sass-rails (3.0.0.1) railties (>= 3.1.1) sass-rails (>= 3.1.1) @@ -249,8 +237,6 @@ GEM letter_opener (1.0.0) launchy (>= 2.0.4) libv8 (3.3.10.4) - libwebsocket (0.1.6) - websocket listen (0.5.3) lumberjack (1.0.2) mail (2.4.4) @@ -261,12 +247,12 @@ GEM mime-types (1.21) modernizr (2.6.2) sprockets (~> 2.0) - multi_json (1.5.1) + multi_json (1.6.1) multi_xml (0.5.1) multipart-post (1.1.5) mysql2 (0.3.11) net-ldap (0.2.2) - nokogiri (1.5.5) + nokogiri (1.5.6) oauth (0.4.7) oauth2 (0.8.0) faraday (~> 0.8) @@ -294,6 +280,10 @@ GEM omniauth-oauth (~> 1.0) orm_adapter (0.4.0) pg (0.14.1) + poltergeist (1.1.0) + capybara (~> 2.0, >= 2.0.1) + faye-websocket (~> 0.4, >= 0.4.4) + http_parser.rb (~> 0.5.3) polyglot (0.3.3) posix-spawn (0.3.6) progressbar (0.12.0) @@ -364,7 +354,7 @@ GEM rspec-expectations (2.12.0) diff-lcs (~> 1.1.3) rspec-mocks (2.12.0) - rspec-rails (2.12.0) + rspec-rails (2.12.2) actionpack (>= 3.0) activesupport (>= 3.0) railties (>= 3.0) @@ -384,11 +374,11 @@ GEM seed-fu (2.2.0) activerecord (~> 3.1) activesupport (~> 3.1) - selenium-webdriver (2.26.0) + selenium-webdriver (2.30.0) childprocess (>= 0.2.5) - libwebsocket (~> 0.1.3) multi_json (~> 1.0) rubyzip + websocket (~> 1.0.4) settingslogic (2.0.8) sexp_processor (4.1.3) shoulda-matchers (1.3.0) @@ -412,11 +402,11 @@ GEM temple (~> 0.5.5) tilt (~> 1.3.3) slop (3.3.3) - spinach (0.5.2) + spinach (0.7.0) colorize gherkin-ruby (~> 0.2.0) - spinach-rails (0.1.8) - capybara (~> 1) + spinach-rails (0.2.0) + capybara (~> 2.0.0) railties (>= 3) spinach (>= 0.4) sprockets (2.2.2) @@ -455,8 +445,8 @@ GEM webmock (1.9.0) addressable (>= 2.2.7) crack (>= 0.1.7) - websocket (1.0.2) - xpath (0.1.4) + websocket (1.0.7) + xpath (1.0.0) nokogiri (~> 1.3) yajl-ruby (1.1.0) @@ -470,7 +460,7 @@ DEPENDENCIES better_errors binding_of_caller bootstrap-sass (= 2.2.1.1) - capybara + capybara (= 2.0.2) carrierwave (~> 0.7.1) chosen-rails (= 0.9.8) coffee-rails (~> 3.2.2) @@ -512,7 +502,7 @@ DEPENDENCIES omniauth-google-oauth2 omniauth-twitter pg - poltergeist! + poltergeist (= 1.1.0) pry pygments.rb! quiet_assets (~> 1.0.1) @@ -524,7 +514,7 @@ DEPENDENCIES rb-fsevent rb-inotify redcarpet (~> 2.2.2) - rspec-rails + rspec-rails (= 2.12.2) sass-rails (~> 3.2.5) sdoc seed-fu @@ -535,7 +525,7 @@ DEPENDENCIES sinatra six slim - spinach-rails + spinach-rails (= 0.2.0) stamp state_machine test_after_commit From 9f722427e5835e4aeb5c1dd6a9cc8720b40d87c0 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 21 Feb 2013 10:41:13 +0200 Subject: [PATCH 14/51] Add LoginHelpers to feature type --- spec/spec_helper.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 77497991..c138d9a6 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -23,6 +23,7 @@ WebMock.disable_net_connect!(allow_localhost: true) RSpec.configure do |config| config.mock_with :rspec + config.include LoginHelpers, type: :feature config.include LoginHelpers, type: :request config.include FactoryGirl::Syntax::Methods config.include Devise::TestHelpers, type: :controller From 03f6a28ec0dab308070e83ec422f12fa289aad9f Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 21 Feb 2013 10:41:37 +0200 Subject: [PATCH 15/51] move capybara scenarios to spec/features --- spec/{requests => features}/admin/admin_hooks_spec.rb | 0 spec/{requests => features}/admin/admin_projects_spec.rb | 0 spec/{requests => features}/admin/admin_users_spec.rb | 0 spec/{requests => features}/admin/security_spec.rb | 0 spec/{requests => features}/atom/dashboard_issues_spec.rb | 0 spec/{requests => features}/atom/dashboard_spec.rb | 0 spec/{requests => features}/atom/issues_spec.rb | 0 spec/{requests => features}/gitlab_flavored_markdown_spec.rb | 0 spec/{requests => features}/issues_spec.rb | 0 spec/{requests => features}/notes_on_merge_requests_spec.rb | 4 +++- spec/{requests => features}/notes_on_wall_spec.rb | 0 spec/{requests => features}/profile_spec.rb | 0 spec/{requests => features}/projects_deploy_keys_spec.rb | 0 spec/{requests => features}/projects_spec.rb | 0 spec/{requests => features}/search_spec.rb | 0 spec/{requests => features}/security/profile_access_spec.rb | 0 spec/{requests => features}/security/project_access_spec.rb | 0 spec/{requests => features}/snippets_spec.rb | 0 18 files changed, 3 insertions(+), 1 deletion(-) rename spec/{requests => features}/admin/admin_hooks_spec.rb (100%) rename spec/{requests => features}/admin/admin_projects_spec.rb (100%) rename spec/{requests => features}/admin/admin_users_spec.rb (100%) rename spec/{requests => features}/admin/security_spec.rb (100%) rename spec/{requests => features}/atom/dashboard_issues_spec.rb (100%) rename spec/{requests => features}/atom/dashboard_spec.rb (100%) rename spec/{requests => features}/atom/issues_spec.rb (100%) rename spec/{requests => features}/gitlab_flavored_markdown_spec.rb (100%) rename spec/{requests => features}/issues_spec.rb (100%) rename spec/{requests => features}/notes_on_merge_requests_spec.rb (99%) rename spec/{requests => features}/notes_on_wall_spec.rb (100%) rename spec/{requests => features}/profile_spec.rb (100%) rename spec/{requests => features}/projects_deploy_keys_spec.rb (100%) rename spec/{requests => features}/projects_spec.rb (100%) rename spec/{requests => features}/search_spec.rb (100%) rename spec/{requests => features}/security/profile_access_spec.rb (100%) rename spec/{requests => features}/security/project_access_spec.rb (100%) rename spec/{requests => features}/snippets_spec.rb (100%) diff --git a/spec/requests/admin/admin_hooks_spec.rb b/spec/features/admin/admin_hooks_spec.rb similarity index 100% rename from spec/requests/admin/admin_hooks_spec.rb rename to spec/features/admin/admin_hooks_spec.rb diff --git a/spec/requests/admin/admin_projects_spec.rb b/spec/features/admin/admin_projects_spec.rb similarity index 100% rename from spec/requests/admin/admin_projects_spec.rb rename to spec/features/admin/admin_projects_spec.rb diff --git a/spec/requests/admin/admin_users_spec.rb b/spec/features/admin/admin_users_spec.rb similarity index 100% rename from spec/requests/admin/admin_users_spec.rb rename to spec/features/admin/admin_users_spec.rb diff --git a/spec/requests/admin/security_spec.rb b/spec/features/admin/security_spec.rb similarity index 100% rename from spec/requests/admin/security_spec.rb rename to spec/features/admin/security_spec.rb diff --git a/spec/requests/atom/dashboard_issues_spec.rb b/spec/features/atom/dashboard_issues_spec.rb similarity index 100% rename from spec/requests/atom/dashboard_issues_spec.rb rename to spec/features/atom/dashboard_issues_spec.rb diff --git a/spec/requests/atom/dashboard_spec.rb b/spec/features/atom/dashboard_spec.rb similarity index 100% rename from spec/requests/atom/dashboard_spec.rb rename to spec/features/atom/dashboard_spec.rb diff --git a/spec/requests/atom/issues_spec.rb b/spec/features/atom/issues_spec.rb similarity index 100% rename from spec/requests/atom/issues_spec.rb rename to spec/features/atom/issues_spec.rb diff --git a/spec/requests/gitlab_flavored_markdown_spec.rb b/spec/features/gitlab_flavored_markdown_spec.rb similarity index 100% rename from spec/requests/gitlab_flavored_markdown_spec.rb rename to spec/features/gitlab_flavored_markdown_spec.rb diff --git a/spec/requests/issues_spec.rb b/spec/features/issues_spec.rb similarity index 100% rename from spec/requests/issues_spec.rb rename to spec/features/issues_spec.rb diff --git a/spec/requests/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb similarity index 99% rename from spec/requests/notes_on_merge_requests_spec.rb rename to spec/features/notes_on_merge_requests_spec.rb index 0111cf42..37fc85c8 100644 --- a/spec/requests/notes_on_merge_requests_spec.rb +++ b/spec/features/notes_on_merge_requests_spec.rb @@ -97,7 +97,9 @@ describe "On a merge request diff", js: true, focus: true do visit diffs_project_merge_request_path(project, merge_request) - click_link("Diff") + within '.diffs-tab' do + click_link("Diff") + end end subject { page } diff --git a/spec/requests/notes_on_wall_spec.rb b/spec/features/notes_on_wall_spec.rb similarity index 100% rename from spec/requests/notes_on_wall_spec.rb rename to spec/features/notes_on_wall_spec.rb diff --git a/spec/requests/profile_spec.rb b/spec/features/profile_spec.rb similarity index 100% rename from spec/requests/profile_spec.rb rename to spec/features/profile_spec.rb diff --git a/spec/requests/projects_deploy_keys_spec.rb b/spec/features/projects_deploy_keys_spec.rb similarity index 100% rename from spec/requests/projects_deploy_keys_spec.rb rename to spec/features/projects_deploy_keys_spec.rb diff --git a/spec/requests/projects_spec.rb b/spec/features/projects_spec.rb similarity index 100% rename from spec/requests/projects_spec.rb rename to spec/features/projects_spec.rb diff --git a/spec/requests/search_spec.rb b/spec/features/search_spec.rb similarity index 100% rename from spec/requests/search_spec.rb rename to spec/features/search_spec.rb diff --git a/spec/requests/security/profile_access_spec.rb b/spec/features/security/profile_access_spec.rb similarity index 100% rename from spec/requests/security/profile_access_spec.rb rename to spec/features/security/profile_access_spec.rb diff --git a/spec/requests/security/project_access_spec.rb b/spec/features/security/project_access_spec.rb similarity index 100% rename from spec/requests/security/project_access_spec.rb rename to spec/features/security/project_access_spec.rb diff --git a/spec/requests/snippets_spec.rb b/spec/features/snippets_spec.rb similarity index 100% rename from spec/requests/snippets_spec.rb rename to spec/features/snippets_spec.rb From 42ce2c108022f09e6c8e2fa9a90ea3f74f2b97b4 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 21 Feb 2013 11:27:22 +0200 Subject: [PATCH 16/51] improve selectors to pass capybara 2.0 --- app/views/profiles/account.html.haml | 4 ++-- features/steps/profile/profile.rb | 22 ++++++++++++++-------- features/steps/shared/diff_note.rb | 6 +++--- spec/spec_helper.rb | 5 ++--- 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/app/views/profiles/account.html.haml b/app/views/profiles/account.html.haml index 5b6c298d..b20d5c7a 100644 --- a/app/views/profiles/account.html.haml +++ b/app/views/profiles/account.html.haml @@ -9,7 +9,7 @@ -%fieldset +%fieldset.update-token %legend Private token %span.cred.pull-right @@ -29,7 +29,7 @@ %span You don`t have one yet. Click generate to fix it. = f.submit 'Generate', class: "btn success btn-build-token" -%fieldset +%fieldset.update-password %legend Password = form_for @user, url: update_password_profile_path, method: :put do |f| .padded diff --git a/features/steps/profile/profile.rb b/features/steps/profile/profile.rb index b6833f2b..e5a39abc 100644 --- a/features/steps/profile/profile.rb +++ b/features/steps/profile/profile.rb @@ -23,15 +23,19 @@ class Profile < Spinach::FeatureSteps end Then 'I change my password' do - fill_in "user_password", :with => "222333" - fill_in "user_password_confirmation", :with => "222333" - click_button "Save" + within '.update-password' do + fill_in "user_password", :with => "222333" + fill_in "user_password_confirmation", :with => "222333" + click_button "Save" + end end When 'I unsuccessfully change my password' do - fill_in "user_password", with: "password" - fill_in "user_password_confirmation", with: "confirmation" - click_button "Save" + within '.update-password' do + fill_in "user_password", with: "password" + fill_in "user_password_confirmation", with: "confirmation" + click_button "Save" + end end Then "I should see a password error message" do @@ -43,8 +47,10 @@ class Profile < Spinach::FeatureSteps end Then 'I reset my token' do - @old_token = @user.private_token - click_button "Reset" + within '.update-token' do + @old_token = @user.private_token + click_button "Reset" + end end And 'I should see new token' do diff --git a/features/steps/shared/diff_note.rb b/features/steps/shared/diff_note.rb index 04862338..9dbbc553 100644 --- a/features/steps/shared/diff_note.rb +++ b/features/steps/shared/diff_note.rb @@ -22,7 +22,7 @@ module SharedDiffNote Given 'I leave a diff comment like "Typo, please fix"' do find("#586fb7c4e1add2d4d24e27566ed7064680098646_29_14.line_holder .js-add-diff-note-button").trigger("click") - within(".file") do + within(".file form[rel$='586fb7c4e1add2d4d24e27566ed7064680098646_29_14']") do fill_in "note[note]", with: "Typo, please fix" #click_button("Add Comment") find(".js-comment-button").trigger("click") @@ -32,7 +32,7 @@ module SharedDiffNote Given 'I preview a diff comment text like "Should fix it :smile:"' do find("#586fb7c4e1add2d4d24e27566ed7064680098646_29_14.line_holder .js-add-diff-note-button").trigger("click") - within(".file") do + within(".file form[rel$='586fb7c4e1add2d4d24e27566ed7064680098646_29_14']") do fill_in "note[note]", with: "Should fix it :smile:" find(".js-note-preview-button").trigger("click") end @@ -40,7 +40,7 @@ module SharedDiffNote Given 'I preview another diff comment text like "DRY this up"' do find("#586fb7c4e1add2d4d24e27566ed7064680098646_57_41.line_holder .js-add-diff-note-button").trigger("click") - within(".file") do + within(".file form[rel$='586fb7c4e1add2d4d24e27566ed7064680098646_57_41']") do fill_in "note[note]", with: "DRY this up" find(".js-note-preview-button").trigger("click") end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index c138d9a6..f6f70cfd 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -10,14 +10,13 @@ require 'capybara/rspec' require 'webmock/rspec' require 'email_spec' require 'sidekiq/testing/inline' +require 'capybara/poltergeist' +Capybara.javascript_driver = :poltergeist # Requires supporting ruby files with custom matchers and macros, etc, # in spec/support/ and its subdirectories. Dir[Rails.root.join("spec/support/**/*.rb")].each {|f| require f} -require 'capybara/poltergeist' -Capybara.javascript_driver = :poltergeist - WebMock.disable_net_connect!(allow_localhost: true) RSpec.configure do |config| From 28da2a8bdc8fd5cbb05a32933c0ab1c56202481f Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 21 Feb 2013 11:44:33 +0200 Subject: [PATCH 17/51] Monkeypatch satellite call for merge request in tests --- app/controllers/merge_requests_controller.rb | 2 ++ spec/support/stubbed_repository.rb | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index bf665272..f92d9976 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -1,3 +1,5 @@ +require 'gitlab/satellite/satellite' + class MergeRequestsController < ProjectResourceController before_filter :module_enabled before_filter :merge_request, only: [:edit, :update, :show, :commits, :diffs, :automerge, :automerge_check, :ci_status] diff --git a/spec/support/stubbed_repository.rb b/spec/support/stubbed_repository.rb index 434cab65..34798963 100644 --- a/spec/support/stubbed_repository.rb +++ b/spec/support/stubbed_repository.rb @@ -1,5 +1,6 @@ require "repository" require "project" +require "merge_request" require "shell" # Stubs out all Git repository access done by models so that specs can run @@ -32,6 +33,12 @@ class Project end end +class MergeRequest + def can_be_merged + true + end +end + class GitLabTestRepo < Repository def repo @repo ||= Grit::Repo.new(Rails.root.join('tmp', 'repositories', 'gitlabhq')) From c77730dd71b31d70a19f1729c795a44d7e609eb0 Mon Sep 17 00:00:00 2001 From: Andrew8xx8 Date: Thu, 21 Feb 2013 14:04:06 +0400 Subject: [PATCH 18/51] An Id must be sended to queue --- app/services/system_hooks_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb index 6043bac6..132bb14a 100644 --- a/app/services/system_hooks_service.rb +++ b/app/services/system_hooks_service.rb @@ -12,7 +12,7 @@ class SystemHooksService end def self.async_execute_hook(hook, data) - Sidekiq::Client.enqueue(SystemHookWorker, hook, data) + Sidekiq::Client.enqueue(SystemHookWorker, hook.id, data) end def self.build_event_data(model, event) From 37187336b1ae92751d98f33d1f378aac069ecdaa Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 21 Feb 2013 12:27:52 +0200 Subject: [PATCH 19/51] Team features are green now --- features/project/source/browse_files.feature | 4 ++-- features/project/source/git_blame.feature | 2 +- features/steps/admin/admin_teams.rb | 14 +++++++------- features/steps/project/project_browse_files.rb | 6 +++--- features/steps/project/project_browse_git_repo.rb | 6 +++--- features/steps/userteams/userteams.rb | 8 ++++---- spec/support/stubbed_repository.rb | 2 +- 7 files changed, 21 insertions(+), 21 deletions(-) diff --git a/features/project/source/browse_files.feature b/features/project/source/browse_files.feature index 0b8495ff..ee26f537 100644 --- a/features/project/source/browse_files.feature +++ b/features/project/source/browse_files.feature @@ -12,7 +12,7 @@ Feature: Project Browse files Then I should see files from repository for "8470d70" Scenario: I browse file content - Given I click on "Gemfile" file in repo + Given I click on "Gemfile.lock" file in repo Then I should see it content Scenario: I browse raw file @@ -22,6 +22,6 @@ Feature: Project Browse files @javascript Scenario: I can edit file - Given I click on "Gemfile" file in repo + Given I click on "Gemfile.lock" file in repo And I click button "edit" Then I can edit code diff --git a/features/project/source/git_blame.feature b/features/project/source/git_blame.feature index 93ed20a8..3b20437a 100644 --- a/features/project/source/git_blame.feature +++ b/features/project/source/git_blame.feature @@ -5,6 +5,6 @@ Feature: Project Browse git repo Given I visit project source page Scenario: I blame file - Given I click on "Gemfile" file in repo + Given I click on "Gemfile.lock" file in repo And I click blame button Then I should see git file blame diff --git a/features/steps/admin/admin_teams.rb b/features/steps/admin/admin_teams.rb index 5c66b24b..637fc4e5 100644 --- a/features/steps/admin/admin_teams.rb +++ b/features/steps/admin/admin_teams.rb @@ -9,7 +9,7 @@ class AdminTeams < Spinach::FeatureSteps end And 'Create gitlab user "John"' do - @user = create(:user, :name => "John") + @user = create(:user, name: "John") end And 'I click new team link' do @@ -50,8 +50,8 @@ class AdminTeams < Spinach::FeatureSteps When 'I select user "John" from user list as "Developer"' do @user ||= User.find_by_name("John") within "#team_members" do - select @user.name, :from => "user_ids" - select "Developer", :from => "default_project_access" + select "#{@user.name} (#{@user.email})", from: "user_ids" + select "Developer", from: "default_project_access" end end @@ -89,8 +89,8 @@ class AdminTeams < Spinach::FeatureSteps When 'I select project "Shop" with max access "Reporter"' do @project ||= Project.find_by_name("Shop") within "#assign_projects" do - select @project.name, :from => "project_ids" - select "Reporter", :from => "greatest_project_access" + select @project.name, from: "project_ids" + select "Reporter", from: "greatest_project_access" end end @@ -127,8 +127,8 @@ class AdminTeams < Spinach::FeatureSteps When 'I select user "Jimm" ub team members list as "Master"' do user = User.find_by_name("Jimm") within "#team_members" do - select user.name, :from => "user_ids" - select "Developer", :from => "default_project_access" + select "#{user.name} (#{user.email})", from: "user_ids" + select "Developer", from: "default_project_access" end end diff --git a/features/steps/project/project_browse_files.rb b/features/steps/project/project_browse_files.rb index 4efce0dc..71360fb6 100644 --- a/features/steps/project/project_browse_files.rb +++ b/features/steps/project/project_browse_files.rb @@ -16,12 +16,12 @@ class ProjectBrowseFiles < Spinach::FeatureSteps page.should have_content "Gemfile" end - Given 'I click on "Gemfile" file in repo' do - click_link "Gemfile" + Given 'I click on "Gemfile.lock" file in repo' do + click_link "Gemfile.lock" end Then 'I should see it content' do - page.should have_content "rubygems.org" + page.should have_content "DEPENDENCIES" end And 'I click link "raw"' do diff --git a/features/steps/project/project_browse_git_repo.rb b/features/steps/project/project_browse_git_repo.rb index 19edfb07..cd9a60f4 100644 --- a/features/steps/project/project_browse_git_repo.rb +++ b/features/steps/project/project_browse_git_repo.rb @@ -3,8 +3,8 @@ class ProjectBrowseGitRepo < Spinach::FeatureSteps include SharedProject include SharedPaths - Given 'I click on "Gemfile" file in repo' do - click_link "Gemfile" + Given 'I click on "Gemfile.lock" file in repo' do + click_link "Gemfile.lock" end And 'I click blame button' do @@ -12,7 +12,7 @@ class ProjectBrowseGitRepo < Spinach::FeatureSteps end Then 'I should see git file blame' do - page.should have_content "rubygems.org" + page.should have_content "DEPENDENCIES" page.should have_content "Dmitriy Zaporozhets" page.should have_content "Moving to rails 3.2" end diff --git a/features/steps/userteams/userteams.rb b/features/steps/userteams/userteams.rb index be83b4ba..1abb0f49 100644 --- a/features/steps/userteams/userteams.rb +++ b/features/steps/userteams/userteams.rb @@ -177,8 +177,8 @@ class Userteams < Spinach::FeatureSteps And 'I select user "John" from list with role "Reporter"' do user = User.find_by_name("John") within "#team_members" do - select user.name, :from => "user_ids" - select "Reporter", :from => "default_project_access" + select "#{user.name} (#{user.email})", from: "user_ids" + select "Reporter", from: "default_project_access" end click_button "Add" end @@ -213,8 +213,8 @@ class Userteams < Spinach::FeatureSteps When 'I submit form with selected project and max access' do within "#assign_projects" do - select @project.name_with_namespace, :from => "project_ids" - select "Reporter", :from => "greatest_project_access" + select @project.name_with_namespace, from: "project_ids" + select "Reporter", from: "greatest_project_access" end click_button "Add" end diff --git a/spec/support/stubbed_repository.rb b/spec/support/stubbed_repository.rb index 34798963..106cfa6d 100644 --- a/spec/support/stubbed_repository.rb +++ b/spec/support/stubbed_repository.rb @@ -34,7 +34,7 @@ class Project end class MergeRequest - def can_be_merged + def check_if_can_be_merged true end end From 2d5096b6787a601740a59cf9b8d5d4cf53471a83 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 21 Feb 2013 12:39:09 +0200 Subject: [PATCH 20/51] Fix ambiguity for member link in test --- features/steps/project/project_team_management.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/steps/project/project_team_management.rb b/features/steps/project/project_team_management.rb index 19352fe0..43589413 100644 --- a/features/steps/project/project_team_management.rb +++ b/features/steps/project/project_team_management.rb @@ -53,7 +53,7 @@ class ProjectTeamManagement < Spinach::FeatureSteps end Given 'I click link "Sam"' do - click_link "Sam" + first(:link, "Sam").click end Then 'I should see "Sam" team profile' do From 5aeaf248f1730ba1698d9e98ec43920b172b9e0c Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 21 Feb 2013 13:09:47 +0200 Subject: [PATCH 21/51] Fixing rspec after upgrade to capybara pt1 --- app/views/snippets/show.html.haml | 2 +- .../features/gitlab_flavored_markdown_spec.rb | 24 +++++++---- spec/features/snippets_spec.rb | 2 +- spec/features/users_spec.rb | 19 ++++++++ spec/requests/api/users_spec.rb | 43 ++++++++----------- 5 files changed, 56 insertions(+), 34 deletions(-) create mode 100644 spec/features/users_spec.rb diff --git a/app/views/snippets/show.html.haml b/app/views/snippets/show.html.haml index e6bcd88f..64b7d933 100644 --- a/app/views/snippets/show.html.haml +++ b/app/views/snippets/show.html.haml @@ -4,7 +4,7 @@ = @snippet.title %small= @snippet.file_name - if can?(current_user, :admin_snippet, @project) || @snippet.author == current_user - = link_to "Edit", edit_project_snippet_path(@project, @snippet), class: "btn btn-small pull-right" + = link_to "Edit", edit_project_snippet_path(@project, @snippet), class: "btn btn-small pull-right", title: 'Edit Snippet' %br %div= render 'blob' diff --git a/spec/features/gitlab_flavored_markdown_spec.rb b/spec/features/gitlab_flavored_markdown_spec.rb index 9a568511..769fcd68 100644 --- a/spec/features/gitlab_flavored_markdown_spec.rb +++ b/spec/features/gitlab_flavored_markdown_spec.rb @@ -169,32 +169,40 @@ describe "Gitlab Flavored Markdown" do describe "for notes" do it "should render in commits#show", js: true do visit project_commit_path(project, commit) - fill_in "note_note", with: "see ##{issue.id}" - click_button "Add Comment" + within ".new_note.js-main-target-form" do + fill_in "note_note", with: "see ##{issue.id}" + click_button "Add Comment" + end page.should have_link("##{issue.id}") end it "should render in issue#show", js: true do visit project_issue_path(project, issue) - fill_in "note_note", with: "see ##{issue.id}" - click_button "Add Comment" + within ".new_note.js-main-target-form" do + fill_in "note_note", with: "see ##{issue.id}" + click_button "Add Comment" + end page.should have_link("##{issue.id}") end it "should render in merge_request#show", js: true do visit project_merge_request_path(project, merge_request) - fill_in "note_note", with: "see ##{issue.id}" - click_button "Add Comment" + within ".new_note.js-main-target-form" do + fill_in "note_note", with: "see ##{issue.id}" + click_button "Add Comment" + end page.should have_link("##{issue.id}") end it "should render in projects#wall", js: true do visit wall_project_path(project) - fill_in "note_note", with: "see ##{issue.id}" - click_button "Add Comment" + within ".new_note.js-main-target-form" do + fill_in "note_note", with: "see ##{issue.id}" + click_button "Add Comment" + end page.should have_link("##{issue.id}") end diff --git a/spec/features/snippets_spec.rb b/spec/features/snippets_spec.rb index 770e34dc..1a0f6eae 100644 --- a/spec/features/snippets_spec.rb +++ b/spec/features/snippets_spec.rb @@ -72,7 +72,7 @@ describe "Snippets" do author: @user, project: project) visit project_snippet_path(project, @snippet) - click_link "Edit" + click_link "Edit Snippet" end it "should open edit page" do diff --git a/spec/features/users_spec.rb b/spec/features/users_spec.rb new file mode 100644 index 00000000..ed9e44fb --- /dev/null +++ b/spec/features/users_spec.rb @@ -0,0 +1,19 @@ +require 'spec_helper' + +describe 'Users' do + describe "GET /users/sign_up" do + before do + Gitlab.config.gitlab.stub(:signup_enabled).and_return(true) + end + + it "should create a new user account" do + visit new_user_registration_path + fill_in "user_name", with: "Name Surname" + fill_in "user_username", with: "Great" + fill_in "user_email", with: "name@mail.com" + fill_in "user_password", with: "password1234" + fill_in "user_password_confirmation", with: "password1234" + expect { click_button "Sign up" }.to change {User.count}.by(1) + end + end +end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 1645117e..33254eed 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -54,32 +54,27 @@ describe Gitlab::API do end describe "GET /users/sign_up" do - before do - Gitlab.config.gitlab.stub(:signup_enabled).and_return(false) - end - it "should redirect to sign in page if signup is disabled" do - get "/users/sign_up" - response.status.should == 302 - response.should redirect_to(new_user_session_path) - end - end + context 'enabled' do + before do + Gitlab.config.gitlab.stub(:signup_enabled).and_return(true) + end - describe "GET /users/sign_up" do - before do - Gitlab.config.gitlab.stub(:signup_enabled).and_return(true) + it "should return sign up page if signup is enabled" do + get "/users/sign_up" + response.status.should == 200 + end end - it "should return sign up page if signup is enabled" do - get "/users/sign_up" - response.status.should == 200 - end - it "should create a new user account" do - visit new_user_registration_path - fill_in "user_name", with: "Name Surname" - fill_in "user_username", with: "Great" - fill_in "user_email", with: "name@mail.com" - fill_in "user_password", with: "password1234" - fill_in "user_password_confirmation", with: "password1234" - expect { click_button "Sign up" }.to change {User.count}.by(1) + + context 'disabled' do + before do + Gitlab.config.gitlab.stub(:signup_enabled).and_return(false) + end + + it "should redirect to sign in page if signup is disabled" do + get "/users/sign_up" + response.status.should == 302 + response.should redirect_to(new_user_session_path) + end end end From cce14e0b0133ec6e8f380a23a6309bb52e67cc20 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 21 Feb 2013 13:28:05 +0200 Subject: [PATCH 22/51] Removing ambiguity and non-working selectors --- spec/features/admin/admin_users_spec.rb | 1 - spec/features/notes_on_merge_requests_spec.rb | 4 ++-- spec/features/notes_on_wall_spec.rb | 2 +- spec/features/search_spec.rb | 7 +++++-- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/spec/features/admin/admin_users_spec.rb b/spec/features/admin/admin_users_spec.rb index 455caf4a..77d6e9e3 100644 --- a/spec/features/admin/admin_users_spec.rb +++ b/spec/features/admin/admin_users_spec.rb @@ -82,7 +82,6 @@ describe "Admin::Users" do it "should have user info" do page.should have_content(@user.email) page.should have_content(@user.name) - page.should have_content(@user.projects_limit) end end diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb index 37fc85c8..06e18195 100644 --- a/spec/features/notes_on_merge_requests_spec.rb +++ b/spec/features/notes_on_merge_requests_spec.rb @@ -18,7 +18,7 @@ describe "On a merge request", js: true do it { should have_css(".js-main-target-form", visible: true, count: 1) } # button initalization - it { within(".js-main-target-form") { should have_button("Add Comment") } } + it { find(".js-main-target-form input[type=submit]").value.should == "Add Comment" } it { within(".js-main-target-form") { should_not have_link("Cancel") } } # notifiactions @@ -136,7 +136,7 @@ describe "On a merge request diff", js: true, focus: true do end it "should be removed when canceled" do - find(".js-close-discussion-note-form").trigger("click") + first(".js-close-discussion-note-form").trigger("click") should have_no_css(".js-temp-notes-holder") end diff --git a/spec/features/notes_on_wall_spec.rb b/spec/features/notes_on_wall_spec.rb index 4adcf74e..1281f2b1 100644 --- a/spec/features/notes_on_wall_spec.rb +++ b/spec/features/notes_on_wall_spec.rb @@ -17,7 +17,7 @@ describe "On the project wall", js: true do it { should have_css(".js-main-target-form", visible: true, count: 1) } # button initalization - it { within(".js-main-target-form") { should have_button("Add Comment") } } + it { find(".js-main-target-form input[type=submit]").value.should == "Add Comment" } it { within(".js-main-target-form") { should_not have_link("Cancel") } } # notifiactions diff --git a/spec/features/search_spec.rb b/spec/features/search_spec.rb index e338f359..5ce4cae3 100644 --- a/spec/features/search_spec.rb +++ b/spec/features/search_spec.rb @@ -6,8 +6,11 @@ describe "Search" do @project = create(:project) @project.team << [@user, :reporter] visit search_path - fill_in "search", with: @project.name[0..3] - click_button "Search" + + within '.search-holder' do + fill_in "search", with: @project.name[0..3] + click_button "Search" + end end it "should show project in search results" do From be817c53c6fcf946bc149b2e603e3809e79d9d36 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 21 Feb 2013 14:11:04 +0200 Subject: [PATCH 23/51] Update database cleaner. Remove postgres from travis build list since it gives us deadlocks always --- .travis.yml | 1 - Gemfile | 2 +- Gemfile.lock | 4 ++-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 6d39488d..a1d0d49d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,5 @@ language: ruby env: - - DB=postgresql - DB=mysql before_install: - sudo apt-get install libicu-dev -y diff --git a/Gemfile b/Gemfile index ec200e17..daba4c6c 100644 --- a/Gemfile +++ b/Gemfile @@ -139,7 +139,7 @@ group :development, :test do gem "capybara", '2.0.2' gem "pry" gem "awesome_print" - gem "database_cleaner", ref: "f89c34300e114be99532f14c115b2799a3380ac6", git: "https://github.com/bmabey/database_cleaner.git" + gem "database_cleaner", ref: "9f898fc50d87a5d51760f9dcf374bf5ffda21baf", git: "https://github.com/bmabey/database_cleaner.git" gem "launchy" gem 'factory_girl_rails' diff --git a/Gemfile.lock b/Gemfile.lock index 93187cd5..f414ca9f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ GIT remote: https://github.com/bmabey/database_cleaner.git - revision: f89c34300e114be99532f14c115b2799a3380ac6 - ref: f89c34300e114be99532f14c115b2799a3380ac6 + revision: 9f898fc50d87a5d51760f9dcf374bf5ffda21baf + ref: 9f898fc50d87a5d51760f9dcf374bf5ffda21baf specs: database_cleaner (0.9.1) From 4a137651ec0408699b9ad137ba8794429f524f32 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 21 Feb 2013 14:11:24 +0200 Subject: [PATCH 24/51] Fix merge request closed filter. Fixed one more test --- app/models/merge_request.rb | 4 ++++ spec/features/notes_on_merge_requests_spec.rb | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 06aa9f3c..1bc34284 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -66,6 +66,10 @@ class MergeRequest < ActiveRecord::Base scope :merged, -> { with_state(:merged) } + # Closed scope for merge request should return + # both merged and closed mr's + scope :closed, -> { with_states(:closed, :merged) } + class << self def find_all_by_branch(branch_name) where("source_branch LIKE :branch OR target_branch LIKE :branch", branch: branch_name) diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb index 06e18195..059e65f8 100644 --- a/spec/features/notes_on_merge_requests_spec.rb +++ b/spec/features/notes_on_merge_requests_spec.rb @@ -136,7 +136,9 @@ describe "On a merge request diff", js: true, focus: true do end it "should be removed when canceled" do - first(".js-close-discussion-note-form").trigger("click") + within(".file form[rel$='4735dfc552ad7bf15ca468adc3cad9d05b624490_185_185']") do + find(".js-close-discussion-note-form").trigger("click") + end should have_no_css(".js-temp-notes-holder") end From 99b6750e1586d7d5195a064e9b5226db7c64d276 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 21 Feb 2013 16:26:09 +0200 Subject: [PATCH 25/51] Restore old order for MR lists. Fix failing tests --- app/contexts/merge_requests_load_context.rb | 2 +- spec/features/notes_on_merge_requests_spec.rb | 2 +- spec/features/notes_on_wall_spec.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/contexts/merge_requests_load_context.rb b/app/contexts/merge_requests_load_context.rb index 683f4c83..fde04085 100644 --- a/app/contexts/merge_requests_load_context.rb +++ b/app/contexts/merge_requests_load_context.rb @@ -14,7 +14,7 @@ class MergeRequestsLoadContext < BaseContext end merge_requests = merge_requests.page(params[:page]).per(20) - merge_requests = merge_requests.includes(:author, :project).order("state, created_at desc") + merge_requests = merge_requests.includes(:author, :project).order("created_at desc") # Filter by specific assignee_id (or lack thereof)? if params[:assignee_id].present? diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb index 059e65f8..9bef0186 100644 --- a/spec/features/notes_on_merge_requests_spec.rb +++ b/spec/features/notes_on_merge_requests_spec.rb @@ -67,7 +67,7 @@ describe "On a merge request", js: true do end # note added - it { within(".js-main-target-form") { should have_content("This is awsome!") } } + it { should have_content("This is awsome!") } # reset form it { within(".js-main-target-form") { should have_no_field("note[note]", with: "This is awesome!") } } diff --git a/spec/features/notes_on_wall_spec.rb b/spec/features/notes_on_wall_spec.rb index 1281f2b1..69f35bea 100644 --- a/spec/features/notes_on_wall_spec.rb +++ b/spec/features/notes_on_wall_spec.rb @@ -66,7 +66,7 @@ describe "On the project wall", js: true do end # note added - it { within(".js-main-target-form") { should have_content("This is awsome!") } } + it { should have_content("This is awsome!") } # reset form it { within(".js-main-target-form") { should have_no_field("note[note]", with: "This is awesome!") } } From 292dffc2286afbb2bcd16331a0be764140e97132 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 21 Feb 2013 17:01:02 +0200 Subject: [PATCH 26/51] Add new entries to CHANGELOG --- CHANGELOG | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index a692bbfa..cd0cdcf7 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,10 +1,33 @@ v 5.0.0 - Replaced gitolite with gitlab-shell + - Removed gitolite-related libraries + - State machine added + - Setup gitlab as git user + - Internal API + - Show team tab for empty projects + - Import repository feature + - Updated rails + - Use lambda for scopes + - Redesign admin area -> users + - Redesign admin area -> user + - Secure link to file attachments + - Add validations for Group and Team names + - Restyle team page for project + - Update capybara, rspec-rails, poltergeist to recent versions v 4.2.0 - Teams - User show page. Via /u/username - Show help contents on pages for better navigation + - Async gitolite calls + - added satellites logs + - can_create_group, can_create_team booleans for User + - Process web hooks async + - GFM: Fix images escaped inside links + - Network graph improved + - Switchable branches for network graph + - API: Groups + - Fixed project download v 4.1.0 - Optional Sign-Up From dbd9d8d4c3c6a27ebf8add4fc20e790b94ca56a6 Mon Sep 17 00:00:00 2001 From: Dmitry Medvinsky Date: Fri, 22 Feb 2013 20:03:59 +0400 Subject: [PATCH 27/51] Fix WebHook and special symbols in credentials When using web hook with credentials secured web resource, one needs to put the credentials in the hook URL. If the credentials contain special symbols (e.g. @ or #), it should be URL-quoted (e.g. %40 instead of @). But when Gitlab is making a request, it should unquote the symbols before base64-encoding them. --- app/models/web_hook.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/models/web_hook.rb b/app/models/web_hook.rb index efa27f31..3f22b108 100644 --- a/app/models/web_hook.rb +++ b/app/models/web_hook.rb @@ -28,10 +28,14 @@ class WebHook < ActiveRecord::Base WebHook.post(url, body: data.to_json, headers: { "Content-Type" => "application/json" }) else post_url = url.gsub("#{parsed_url.userinfo}@", "") + auth = { + username: URI.decode(parsed_url.user), + password: URI.decode(parsed_url.password), + } WebHook.post(post_url, body: data.to_json, headers: {"Content-Type" => "application/json"}, - basic_auth: {username: parsed_url.user, password: parsed_url.password}) + basic_auth: auth) end end From 30180ed82e3525e3d12c66f28e91ef87f8064257 Mon Sep 17 00:00:00 2001 From: Dmitry Medvinsky Date: Fri, 22 Feb 2013 20:26:33 +0400 Subject: [PATCH 28/51] Add title for "Remove from team" button --- app/views/teams/members/_show.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/teams/members/_show.html.haml b/app/views/teams/members/_show.html.haml index 94d2fd50..3aa2db86 100644 --- a/app/views/teams/members/_show.html.haml +++ b/app/views/teams/members/_show.html.haml @@ -26,5 +26,5 @@ - elsif user.blocked %span.btn.disabled.blocked Blocked - elsif allow_admin - = link_to team_member_path(@team, user), confirm: remove_from_user_team_message(@team, user), method: :delete, class: "btn-tiny btn btn-remove" do + = link_to team_member_path(@team, user), confirm: remove_from_user_team_message(@team, user), method: :delete, class: "btn-tiny btn btn-remove", title: "Remove from team" do %i.icon-minus.icon-white From b0fb68c191709c598ca6378914fd4f2da95dfb3a Mon Sep 17 00:00:00 2001 From: Dmitry Medvinsky Date: Fri, 22 Feb 2013 20:50:17 +0400 Subject: [PATCH 29/51] Add transition on search box It's kind of cool trend to use animated-expanding search box nowadays. E.g. see Github. --- app/assets/stylesheets/gitlab_bootstrap/mixins.scss | 8 ++++++++ app/assets/stylesheets/sections/header.scss | 1 + 2 files changed, 9 insertions(+) diff --git a/app/assets/stylesheets/gitlab_bootstrap/mixins.scss b/app/assets/stylesheets/gitlab_bootstrap/mixins.scss index c8cc9a70..f416be95 100644 --- a/app/assets/stylesheets/gitlab_bootstrap/mixins.scss +++ b/app/assets/stylesheets/gitlab_bootstrap/mixins.scss @@ -24,6 +24,14 @@ background-image: -o-linear-gradient($from, $to); } +@mixin transition($transition) { + -webkit-transition: $transition; + -moz-transition: $transition; + -ms-transition: $transition; + -o-transition: $transition; + transition: $transition; +} + /** * Prefilled mixins * Mixins with fixed values diff --git a/app/assets/stylesheets/sections/header.scss b/app/assets/stylesheets/sections/header.scss index 05c077a8..99c8275b 100644 --- a/app/assets/stylesheets/sections/header.scss +++ b/app/assets/stylesheets/sections/header.scss @@ -90,6 +90,7 @@ header { @include border-radius(3px); border: 1px solid #c6c6c6; box-shadow: none; + @include transition(all 0.15s ease-in 0s); &:focus { @extend .span3; } From c3659ef2e50ff238cecb62e5e4b89f6d94c54be2 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 25 Feb 2013 08:53:49 +0200 Subject: [PATCH 30/51] Fix merge request migration for postgres --- ...8141327_convert_closed_to_state_in_merge_request.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/db/migrate/20130218141327_convert_closed_to_state_in_merge_request.rb b/db/migrate/20130218141327_convert_closed_to_state_in_merge_request.rb index 4d5c6ee5..5e7477d8 100644 --- a/db/migrate/20130218141327_convert_closed_to_state_in_merge_request.rb +++ b/db/migrate/20130218141327_convert_closed_to_state_in_merge_request.rb @@ -1,16 +1,16 @@ class ConvertClosedToStateInMergeRequest < ActiveRecord::Migration def up MergeRequest.transaction do - MergeRequest.where("closed = 1 AND merged = 1").update_all("state = 'merged'") - MergeRequest.where("closed = 1 AND merged = 0").update_all("state = 'closed'") - MergeRequest.where("closed = 0").update_all("state = 'opened'") + MergeRequest.where(closed: true, merged: true).update_all("state = 'merged'") + MergeRequest.where(closed: true, merged: true).update_all("state = 'closed'") + MergeRequest.where(closed: false).update_all("state = 'opened'") end end def down MergeRequest.transaction do - MergeRequest.where(state: :closed).update_all("closed = 1") - MergeRequest.where(state: :merged).update_all("closed = 1, merged = 1") + MergeRequest.where(state: :closed).update_all(closed: true) + MergeRequest.where(state: :merged).update_all(closed: true, merged: true) end end end From 3cf814ff9d16d761ec37a8189e5192a378ef13bb Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 25 Feb 2013 10:28:59 +0200 Subject: [PATCH 31/51] remove duplicate finder in features/steps/path --- features/steps/shared/paths.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/features/steps/shared/paths.rb b/features/steps/shared/paths.rb index a8e68012..40786f6e 100644 --- a/features/steps/shared/paths.rb +++ b/features/steps/shared/paths.rb @@ -173,12 +173,10 @@ module SharedPaths # ---------------------------------------- And 'I visit project "Shop" page' do - project = Project.find_by_name("Shop") visit project_path(project) end When 'I visit edit project "Shop" page' do - project = Project.find_by_name("Shop") visit edit_project_path(project) end @@ -219,7 +217,7 @@ module SharedPaths end And 'I visit project "Shop" issues page' do - visit project_issues_path(Project.find_by_name("Shop")) + visit project_issues_path(project) end Given 'I visit issue page "Release 0.4"' do @@ -228,7 +226,7 @@ module SharedPaths end Given 'I visit project "Shop" labels page' do - visit project_labels_path(Project.find_by_name("Shop")) + visit project_labels_path(project) end Given 'I visit merge request page "Bug NS-04"' do @@ -242,20 +240,18 @@ module SharedPaths end And 'I visit project "Shop" merge requests page' do - visit project_merge_requests_path(Project.find_by_name("Shop")) + visit project_merge_requests_path(project) end Given 'I visit project "Shop" milestones page' do - @project = Project.find_by_name("Shop") - visit project_milestones_path(@project) + visit project_milestones_path(project) end Then 'I visit project "Shop" team page' do - visit project_team_index_path(Project.find_by_name("Shop")) + visit project_team_index_path(project) end Then 'I visit project "Shop" wall page' do - project = Project.find_by_name("Shop") visit wall_project_path(project) end @@ -266,4 +262,8 @@ module SharedPaths def root_ref @project.repository.root_ref end + + def project + project = Project.find_by_name!("Shop") + end end From 262f80a68ac2ed14be0bbf3666b91480fb56e47d Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 25 Feb 2013 19:22:46 +0200 Subject: [PATCH 32/51] Update grit for ruby 2.0 support --- Gemfile | 2 +- Gemfile.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index daba4c6c..2cb148e7 100644 --- a/Gemfile +++ b/Gemfile @@ -22,7 +22,7 @@ gem 'omniauth-twitter' gem 'omniauth-github' # GITLAB patched libs -gem "grit", git: "https://github.com/gitlabhq/grit.git", ref: '7f35cb98ff17d534a07e3ce6ec3d580f67402837' +gem "grit", git: "https://github.com/gitlabhq/grit.git", ref: '09918a3f3217eab7d3b6f5936d5ea2a07f886794' gem 'grack', git: "https://github.com/gitlabhq/grack.git", ref: 'ba46f3b0845c6a09d488ae6abdce6ede37e227e8' gem 'grit_ext', git: "https://github.com/gitlabhq/grit_ext.git", ref: '8e6afc2da821354774aa4d1ee8a1aa2082f84a3e' diff --git a/Gemfile.lock b/Gemfile.lock index f414ca9f..81f2d092 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -23,8 +23,8 @@ GIT GIT remote: https://github.com/gitlabhq/grit.git - revision: 7f35cb98ff17d534a07e3ce6ec3d580f67402837 - ref: 7f35cb98ff17d534a07e3ce6ec3d580f67402837 + revision: 09918a3f3217eab7d3b6f5936d5ea2a07f886794 + ref: 09918a3f3217eab7d3b6f5936d5ea2a07f886794 specs: grit (2.5.0) diff-lcs (~> 1.1) From 580a56b6ff2a32243e62f52df73eb58e9671cb5f Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 25 Feb 2013 19:44:37 +0200 Subject: [PATCH 33/51] Update grit to handle symlinks --- Gemfile | 2 +- Gemfile.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index 2cb148e7..0eba7eb2 100644 --- a/Gemfile +++ b/Gemfile @@ -22,7 +22,7 @@ gem 'omniauth-twitter' gem 'omniauth-github' # GITLAB patched libs -gem "grit", git: "https://github.com/gitlabhq/grit.git", ref: '09918a3f3217eab7d3b6f5936d5ea2a07f886794' +gem "grit", git: "https://github.com/gitlabhq/grit.git", ref: '9e98418ce2d654485b967003726aa2706a10060b' gem 'grack', git: "https://github.com/gitlabhq/grack.git", ref: 'ba46f3b0845c6a09d488ae6abdce6ede37e227e8' gem 'grit_ext', git: "https://github.com/gitlabhq/grit_ext.git", ref: '8e6afc2da821354774aa4d1ee8a1aa2082f84a3e' diff --git a/Gemfile.lock b/Gemfile.lock index 81f2d092..3ca39aea 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -23,8 +23,8 @@ GIT GIT remote: https://github.com/gitlabhq/grit.git - revision: 09918a3f3217eab7d3b6f5936d5ea2a07f886794 - ref: 09918a3f3217eab7d3b6f5936d5ea2a07f886794 + revision: 9e98418ce2d654485b967003726aa2706a10060b + ref: 9e98418ce2d654485b967003726aa2706a10060b specs: grit (2.5.0) diff-lcs (~> 1.1) From 7bab81b199936597e1c762278bd16167de073342 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 25 Feb 2013 21:21:38 +0200 Subject: [PATCH 34/51] Move git post push logic to service --- app/models/project.rb | 117 +++------------------------ app/services/git_push_service.rb | 114 ++++++++++++++++++++++++++ app/workers/post_receive.rb | 2 +- spec/models/project_hooks_spec.rb | 128 ------------------------------ spec/models/project_spec.rb | 3 - spec/services/git_push_service.rb | 111 ++++++++++++++++++++++++++ 6 files changed, 235 insertions(+), 240 deletions(-) create mode 100644 app/services/git_push_service.rb delete mode 100644 spec/models/project_hooks_spec.rb create mode 100644 spec/services/git_push_service.rb diff --git a/app/models/project.rb b/app/models/project.rb index b7d688bf..28f56416 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -295,53 +295,6 @@ class Project < ActiveRecord::Base end end - # This method will be called after each post receive and only if the provided - # user is present in GitLab. - # - # All callbacks for post receive should be placed here. - def trigger_post_receive(oldrev, newrev, ref, user) - data = post_receive_data(oldrev, newrev, ref, user) - - # Create satellite - self.satellite.create unless self.satellite.exists? - - # Create push event - self.observe_push(data) - - if push_to_branch? ref, oldrev - # Close merged MR - self.update_merge_requests(oldrev, newrev, ref, user) - - # Execute web hooks - self.execute_hooks(data.dup) - - # Execute project services - self.execute_services(data.dup) - end - - # Discover the default branch, but only if it hasn't already been set to - # something else - if repository && default_branch.nil? - update_attributes(default_branch: self.repository.discover_default_branch) - end - end - - def push_to_branch? ref, oldrev - ref_parts = ref.split('/') - - # Return if this is not a push to a branch (e.g. new commits) - !(ref_parts[1] !~ /heads/ || oldrev == "00000000000000000000000000000000") - end - - def observe_push(data) - Event.create( - project: self, - action: Event::PUSHED, - data: data, - author_id: data[:user_id] - ) - end - def execute_hooks(data) hooks.each { |hook| hook.async_execute(data) } end @@ -354,68 +307,12 @@ class Project < ActiveRecord::Base end end - # Produce a hash of post-receive data - # - # data = { - # before: String, - # after: String, - # ref: String, - # user_id: String, - # user_name: String, - # repository: { - # name: String, - # url: String, - # description: String, - # homepage: String, - # }, - # commits: Array, - # total_commits_count: Fixnum - # } - # - def post_receive_data(oldrev, newrev, ref, user) - - push_commits = repository.commits_between(oldrev, newrev) - - # Total commits count - push_commits_count = push_commits.size - - # Get latest 20 commits ASC - push_commits_limited = push_commits.last(20) - - # Hash to be passed as post_receive_data - data = { - before: oldrev, - after: newrev, - ref: ref, - user_id: user.id, - user_name: user.name, - repository: { - name: name, - url: url_to_repo, - description: description, - homepage: web_url, - }, - commits: [], - total_commits_count: push_commits_count - } - - # For perfomance purposes maximum 20 latest commits - # will be passed as post receive hook data. - # - push_commits_limited.each do |commit| - data[:commits] << { - id: commit.id, - message: commit.safe_message, - timestamp: commit.date.xmlschema, - url: "#{Gitlab.config.gitlab.url}/#{path_with_namespace}/commit/#{commit.id}", - author: { - name: commit.author_name, - email: commit.author_email - } - } + def discover_default_branch + # Discover the default branch, but only if it hasn't already been set to + # something else + if repository && default_branch.nil? + update_attributes(default_branch: self.repository.discover_default_branch) end - - data end def update_merge_requests(oldrev, newrev, ref, user) @@ -446,6 +343,10 @@ class Project < ActiveRecord::Base !repository || repository.empty? end + def ensure_satellite_exists + self.satellite.create unless self.satellite.exists? + end + def satellite @satellite ||= Gitlab::Satellite::Satellite.new(self) end diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb new file mode 100644 index 00000000..55cf31cd --- /dev/null +++ b/app/services/git_push_service.rb @@ -0,0 +1,114 @@ +class GitPushService + attr_accessor :project, :user, :push_data + + # This method will be called after each git update + # and only if the provided user and project is present in GitLab. + # + # All callbacks for post receive action should be placed here. + # + # Now this method do next: + # 1. Ensure project satellite exists + # 2. Update merge requests + # 3. Execute project web hooks + # 4. Execute project services + # 5. Create Push Event + # + def execute(project, user, oldrev, newrev, ref) + @project, @user = project, user + + # Collect data for this git push + @push_data = post_receive_data(oldrev, newrev, ref) + + project.ensure_satellite_exists + project.discover_default_branch + + if push_to_branch?(ref, oldrev) + project.update_merge_requests(oldrev, newrev, ref, @user) + project.execute_hooks(@push_data.dup) + project.execute_services(@push_data.dup) + end + + create_push_event + end + + protected + + def create_push_event + Event.create( + project: project, + action: Event::PUSHED, + data: push_data, + author_id: push_data[:user_id] + ) + end + + # Produce a hash of post-receive data + # + # data = { + # before: String, + # after: String, + # ref: String, + # user_id: String, + # user_name: String, + # repository: { + # name: String, + # url: String, + # description: String, + # homepage: String, + # }, + # commits: Array, + # total_commits_count: Fixnum + # } + # + def post_receive_data(oldrev, newrev, ref) + push_commits = project.repository.commits_between(oldrev, newrev) + + # Total commits count + push_commits_count = push_commits.size + + # Get latest 20 commits ASC + push_commits_limited = push_commits.last(20) + + # Hash to be passed as post_receive_data + data = { + before: oldrev, + after: newrev, + ref: ref, + user_id: user.id, + user_name: user.name, + repository: { + name: project.name, + url: project.url_to_repo, + description: project.description, + homepage: project.web_url, + }, + commits: [], + total_commits_count: push_commits_count + } + + # For perfomance purposes maximum 20 latest commits + # will be passed as post receive hook data. + # + push_commits_limited.each do |commit| + data[:commits] << { + id: commit.id, + message: commit.safe_message, + timestamp: commit.date.xmlschema, + url: "#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/commit/#{commit.id}", + author: { + name: commit.author_name, + email: commit.author_email + } + } + end + + data + end + + def push_to_branch? ref, oldrev + ref_parts = ref.split('/') + + # Return if this is not a push to a branch (e.g. new commits) + !(ref_parts[1] !~ /heads/ || oldrev == "00000000000000000000000000000000") + end +end diff --git a/app/workers/post_receive.rb b/app/workers/post_receive.rb index 3ef6d597..72cef0fd 100644 --- a/app/workers/post_receive.rb +++ b/app/workers/post_receive.rb @@ -42,6 +42,6 @@ class PostReceive return false end - project.trigger_post_receive(oldrev, newrev, ref, user) + GitPushService.new.execute(project, user, oldrev, newrev, ref) end end diff --git a/spec/models/project_hooks_spec.rb b/spec/models/project_hooks_spec.rb deleted file mode 100644 index 65205538..00000000 --- a/spec/models/project_hooks_spec.rb +++ /dev/null @@ -1,128 +0,0 @@ -require 'spec_helper' - -describe Project, "Hooks" do - let(:project) { create(:project) } - - before do - @key = create(:key, user: project.owner) - @user = @key.user - @key_id = @key.identifier - end - - describe "Post Receive Event" do - it "should create push event" do - oldrev, newrev, ref = '00000000000000000000000000000000', 'newrev', 'refs/heads/master' - data = project.post_receive_data(oldrev, newrev, ref, @user) - - project.observe_push(data) - event = Event.last - - event.should_not be_nil - event.project.should == project - event.action.should == Event::PUSHED - event.data.should == data - end - end - - describe "Project hooks" do - context "with no web hooks" do - it "raises no errors" do - lambda { - project.execute_hooks({}) - }.should_not raise_error - end - end - - context "with web hooks" do - before do - @project_hook = create(:project_hook) - @project_hook_2 = create(:project_hook) - project.hooks << [@project_hook, @project_hook_2] - - stub_request(:post, @project_hook.url) - stub_request(:post, @project_hook_2.url) - end - - it "executes multiple web hook" do - @project_hook.should_receive(:async_execute).once - @project_hook_2.should_receive(:async_execute).once - - project.trigger_post_receive('oldrev', 'newrev', 'refs/heads/master', @user) - end - end - - context "does not execute web hooks" do - before do - @project_hook = create(:project_hook) - project.hooks << [@project_hook] - end - - it "when pushing a branch for the first time" do - @project_hook.should_not_receive(:execute) - project.trigger_post_receive('00000000000000000000000000000000', 'newrev', 'refs/heads/master', @user) - end - - it "when pushing tags" do - @project_hook.should_not_receive(:execute) - project.trigger_post_receive('oldrev', 'newrev', 'refs/tags/v1.0.0', @user) - end - end - - context "when pushing new branches" do - - end - - context "when gathering commit data" do - before do - @oldrev, @newrev, @ref = project.repository.fresh_commits(2).last.sha, - project.repository.fresh_commits(2).first.sha, 'refs/heads/master' - @commit = project.repository.fresh_commits(2).first - - # Fill nil/empty attributes - project.description = "This is a description" - - @data = project.post_receive_data(@oldrev, @newrev, @ref, @user) - end - - subject { @data } - - it { should include(before: @oldrev) } - it { should include(after: @newrev) } - it { should include(ref: @ref) } - it { should include(user_id: project.owner.id) } - it { should include(user_name: project.owner.name) } - - context "with repository data" do - subject { @data[:repository] } - - it { should include(name: project.name) } - it { should include(url: project.url_to_repo) } - it { should include(description: project.description) } - it { should include(homepage: project.web_url) } - end - - context "with commits" do - subject { @data[:commits] } - - it { should be_an(Array) } - it { should have(1).element } - - context "the commit" do - subject { @data[:commits].first } - - it { should include(id: @commit.id) } - it { should include(message: @commit.safe_message) } - it { should include(timestamp: @commit.date.xmlschema) } - it { should include(url: "#{Gitlab.config.gitlab.url}/#{project.code}/commit/#{@commit.id}") } - - context "with a author" do - subject { @data[:commits].first[:author] } - - it { should include(name: @commit.author_name) } - it { should include(email: @commit.author_email) } - end - end - end - end - end -end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 5c27f363..48432eac 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -72,11 +72,8 @@ describe Project do it { should respond_to(:url_to_repo) } it { should respond_to(:repo_exists?) } it { should respond_to(:satellite) } - it { should respond_to(:observe_push) } it { should respond_to(:update_merge_requests) } it { should respond_to(:execute_hooks) } - it { should respond_to(:post_receive_data) } - it { should respond_to(:trigger_post_receive) } it { should respond_to(:transfer) } it { should respond_to(:name_with_namespace) } it { should respond_to(:namespace_owner) } diff --git a/spec/services/git_push_service.rb b/spec/services/git_push_service.rb new file mode 100644 index 00000000..9fc5fd62 --- /dev/null +++ b/spec/services/git_push_service.rb @@ -0,0 +1,111 @@ +require 'spec_helper' + +describe GitPushService do + let (:user) { create :user } + let (:project) { create :project } + let (:service) { GitPushService.new } + + before do + @oldrev = 'b98a310def241a6fd9c9a9a3e7934c48e498fe81' + @newrev = 'b19a04f53caeebf4fe5ec2327cb83e9253dc91bb' + @ref = 'refs/heads/master' + end + + describe "Git Push Data" do + before do + service.execute(project, user, @oldrev, @newrev, @ref) + @push_data = service.push_data + @commit = project.repository.commit(@newrev) + end + + subject { @push_data } + + it { should include(before: @oldrev) } + it { should include(after: @newrev) } + it { should include(ref: @ref) } + it { should include(user_id: user.id) } + it { should include(user_name: user.name) } + + context "with repository data" do + subject { @push_data[:repository] } + + it { should include(name: project.name) } + it { should include(url: project.url_to_repo) } + it { should include(description: project.description) } + it { should include(homepage: project.web_url) } + end + + context "with commits" do + subject { @push_data[:commits] } + + it { should be_an(Array) } + it { should have(1).element } + + context "the commit" do + subject { @push_data[:commits].first } + + it { should include(id: @commit.id) } + it { should include(message: @commit.safe_message) } + it { should include(timestamp: @commit.date.xmlschema) } + it { should include(url: "#{Gitlab.config.gitlab.url}/#{project.code}/commit/#{@commit.id}") } + + context "with a author" do + subject { @push_data[:commits].first[:author] } + + it { should include(name: @commit.author_name) } + it { should include(email: @commit.author_email) } + end + end + end + end + + describe "Push Event" do + before do + service.execute(project, user, @oldrev, @newrev, @ref) + @event = Event.last + end + + it { @event.should_not be_nil } + it { @event.project.should == project } + it { @event.action.should == Event::PUSHED } + it { @event.data.should == service.push_data } + end + + describe "Web Hooks" do + context "with web hooks" do + before do + @project_hook = create(:project_hook) + @project_hook_2 = create(:project_hook) + project.hooks << [@project_hook, @project_hook_2] + + stub_request(:post, @project_hook.url) + stub_request(:post, @project_hook_2.url) + end + + it "executes multiple web hook" do + @project_hook.should_receive(:async_execute).once + @project_hook_2.should_receive(:async_execute).once + + service.execute(project, user, @oldrev, @newrev, @ref) + end + end + + context "does not execute web hooks" do + before do + @project_hook = create(:project_hook) + project.hooks << [@project_hook] + end + + it "when pushing a branch for the first time" do + @project_hook.should_not_receive(:execute) + service.execute(project, user, '00000000000000000000000000000000', 'newrev', 'refs/heads/master') + end + + it "when pushing tags" do + @project_hook.should_not_receive(:execute) + service.execute(project, user, 'newrev', 'newrev', 'refs/tags/v1.0.0') + end + end + end +end + From f17fe7fff2809720e13e84c1aea61878b8559e9d Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 25 Feb 2013 21:26:15 +0200 Subject: [PATCH 35/51] correct indentation in activity observer --- app/observers/activity_observer.rb | 28 ++++++++++++++-------------- app/observers/note_observer.rb | 1 - 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/app/observers/activity_observer.rb b/app/observers/activity_observer.rb index 9c72a6d3..919a50f0 100644 --- a/app/observers/activity_observer.rb +++ b/app/observers/activity_observer.rb @@ -21,22 +21,22 @@ class ActivityObserver < ActiveRecord::Observer end def after_close(record, transition) - Event.create( - project: record.project, - target_id: record.id, - target_type: record.class.name, - action: Event::CLOSED, - author_id: record.author_id_of_changes - ) + Event.create( + project: record.project, + target_id: record.id, + target_type: record.class.name, + action: Event::CLOSED, + author_id: record.author_id_of_changes + ) end def after_reopen(record, transition) - Event.create( - project: record.project, - target_id: record.id, - target_type: record.class.name, - action: Event::REOPENED, - author_id: record.author_id_of_changes - ) + Event.create( + project: record.project, + target_id: record.id, + target_type: record.class.name, + action: Event::REOPENED, + author_id: record.author_id_of_changes + ) end end diff --git a/app/observers/note_observer.rb b/app/observers/note_observer.rb index 4ee9fadf..0f820a26 100644 --- a/app/observers/note_observer.rb +++ b/app/observers/note_observer.rb @@ -1,5 +1,4 @@ class NoteObserver < ActiveRecord::Observer - def after_create(note) send_notify_mails(note) end From d0646babdbb54c652ce19e6329b904193be8522c Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 25 Feb 2013 21:45:33 +0200 Subject: [PATCH 36/51] Remove gitolite mention from docs --- doc/raketasks/backup_restore.md | 2 -- doc/raketasks/cleanup.md | 8 +------- doc/raketasks/features.md | 6 +++--- doc/raketasks/maintenance.md | 34 +++++---------------------------- 4 files changed, 9 insertions(+), 41 deletions(-) diff --git a/doc/raketasks/backup_restore.md b/doc/raketasks/backup_restore.md index 9b42afa7..d2da64f3 100644 --- a/doc/raketasks/backup_restore.md +++ b/doc/raketasks/backup_restore.md @@ -31,7 +31,6 @@ Dumping database tables: - Dumping table wikis... [DONE] Dumping repositories: - Dumping repository abcd... [DONE] -- Dumping repository gitolite-admin.git... [DONE] Creating backup archive: $TIMESTAMP_gitlab_backup.tar [DONE] Deleting tmp directories...[DONE] Deleting old backups... [SKIPPING] @@ -77,6 +76,5 @@ Restoring database tables: - Loading fixture wikis...[SKIPPING] Restoring repositories: - Restoring repository abcd... [DONE] -- Restoring repository gitolite-admin.git... [DONE] Deleting tmp directories...[DONE] ``` diff --git a/doc/raketasks/cleanup.md b/doc/raketasks/cleanup.md index ad9e5a61..30a5d362 100644 --- a/doc/raketasks/cleanup.md +++ b/doc/raketasks/cleanup.md @@ -1,10 +1,4 @@ -### Remove grabage from gitolite config and filesystem. Important! Data loss! - -Remove projects from gitolite config if they dont exist in GitLab database - -``` -bundle exec rake gitlab:cleanup:config RAILS_ENV=production -``` +### Remove grabage from filesystem. Important! Data loss! Remove namespaces(dirs) from /home/git/repositories if they dont exist in GitLab database diff --git a/doc/raketasks/features.md b/doc/raketasks/features.md index 7f7daaf0..018817d2 100644 --- a/doc/raketasks/features.md +++ b/doc/raketasks/features.md @@ -17,12 +17,12 @@ bundle exec rake gitlab:enable_namespaces RAILS_ENV=production ``` -### Enable auto merge +### Rebuild project satellites -This command will enable the auto merge feature. After this you will be able to **merge a merge request** via GitLab and use the **online editor**. +This command will build missing satellites for projects. After this you will be able to **merge a merge request** via GitLab and use the **online editor**. ``` -bundle exec rake gitlab:enable_automerge RAILS_ENV=production +bundle exec rake gitlab:satellites:create RAILS_ENV=production ``` Example output: diff --git a/doc/raketasks/maintenance.md b/doc/raketasks/maintenance.md index 110dbd16..726cc083 100644 --- a/doc/raketasks/maintenance.md +++ b/doc/raketasks/maintenance.md @@ -31,12 +31,10 @@ SSH Clone URL: git@localhost:some-project.git Using LDAP: no Using Omniauth: no -Gitolite information -Version: v3.04-4-g4524f01 -Admin URI: git@localhost:gitolite-admin -Admin Key: gitlab +GitLab Shell +Version: 1.0.4 Repositories: /home/git/repositories/ -Hooks: /home/git/.gitolite/hooks/ +Hooks: /home/git/gitlab-shell/hooks/ Git: /usr/bin/git ``` @@ -46,8 +44,8 @@ Git: /usr/bin/git Runs the following rake tasks: * gitlab:env:check -* gitlab:gitolite:check -* gitlab:resque:check +* gitlab:gitlab_shell:check +* gitlab:sidekiq:check * gitlab:app:check It will check that each component was setup according to the installation guide and suggest fixes for issues found. @@ -74,16 +72,12 @@ Checking Environment ... Finished Checking Gitolite ... Using recommended version ... yes -Repo umask is 0007 in .gitolite.rc? ... yes -Allow all Git config keys in .gitolite.rc ... yes Config directory exists? ... yes Config directory owned by git:git? ... yes Config directory access is drwxr-x---? ... yes Repo base directory exists? ... yes Repo base owned by git:git? ... yes Repo base access is drwxrws---? ... yes -Can clone gitolite-admin? ... yes -Can commit to gitolite-admin? ... yes post-receive hook exists? ... yes post-receive hook up-to-date? ... yes post-receive hooks in repos are links: ... @@ -135,24 +129,6 @@ If necessary, remove the `tmp/repo_satellites` directory and rerun the command b bundle exec rake gitlab:satellites:create RAILS_ENV=production ``` - -### Rebuild each key at gitolite config - -This will send all users ssh public keys to gitolite and grant them access (based on their permission) to their projects. - -``` -bundle exec rake gitlab:gitolite:update_keys RAILS_ENV=production -``` - - -### Rebuild each project at gitolite config - -This makes sure that all projects are present in gitolite and can be accessed. - -``` -bundle exec rake gitlab:gitolite:update_repos RAILS_ENV=production -``` - ### Import bare repositories into GitLab project instance Notes: From db8baf2895f111652699c5b48d8cb2663eed6c3f Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 25 Feb 2013 22:12:11 +0200 Subject: [PATCH 37/51] Since search_autocomplete_source rendered with raw all human input should be sanitized to prevent XSS --- app/helpers/application_helper.rb | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index d02130c5..dad23471 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -72,7 +72,7 @@ module ApplicationHelper end def search_autocomplete_source - projects = current_user.authorized_projects.map { |p| { label: "project: #{p.name_with_namespace}", url: project_path(p) } } + projects = current_user.authorized_projects.map { |p| { label: "project: #{simple_sanitize(p.name_with_namespace)}", url: project_path(p) } } groups = current_user.authorized_groups.map { |group| { label: "group: #{simple_sanitize(group.name)}", url: group_path(group) } } teams = current_user.authorized_teams.map { |team| { label: "team: #{simple_sanitize(team.name)}", url: team_path(team) } } @@ -98,15 +98,15 @@ module ApplicationHelper project_nav = [] if @project && @project.repository && @project.repository.root_ref project_nav = [ - { label: "#{@project.name_with_namespace} - Issues", url: project_issues_path(@project) }, - { label: "#{@project.name_with_namespace} - Commits", url: project_commits_path(@project, @ref || @project.repository.root_ref) }, - { label: "#{@project.name_with_namespace} - Merge Requests", url: project_merge_requests_path(@project) }, - { label: "#{@project.name_with_namespace} - Milestones", url: project_milestones_path(@project) }, - { label: "#{@project.name_with_namespace} - Snippets", url: project_snippets_path(@project) }, - { label: "#{@project.name_with_namespace} - Team", url: project_team_index_path(@project) }, - { label: "#{@project.name_with_namespace} - Tree", url: project_tree_path(@project, @ref || @project.repository.root_ref) }, - { label: "#{@project.name_with_namespace} - Wall", url: wall_project_path(@project) }, - { label: "#{@project.name_with_namespace} - Wiki", url: project_wikis_path(@project) }, + { label: "#{simple_sanitize(@project.name_with_namespace)} - Issues", url: project_issues_path(@project) }, + { label: "#{simple_sanitize(@project.name_with_namespace)} - Commits", url: project_commits_path(@project, @ref || @project.repository.root_ref) }, + { label: "#{simple_sanitize(@project.name_with_namespace)} - Merge Requests", url: project_merge_requests_path(@project) }, + { label: "#{simple_sanitize(@project.name_with_namespace)} - Milestones", url: project_milestones_path(@project) }, + { label: "#{simple_sanitize(@project.name_with_namespace)} - Snippets", url: project_snippets_path(@project) }, + { label: "#{simple_sanitize(@project.name_with_namespace)} - Team", url: project_team_index_path(@project) }, + { label: "#{simple_sanitize(@project.name_with_namespace)} - Tree", url: project_tree_path(@project, @ref || @project.repository.root_ref) }, + { label: "#{simple_sanitize(@project.name_with_namespace)} - Wall", url: wall_project_path(@project) }, + { label: "#{simple_sanitize(@project.name_with_namespace)} - Wiki", url: project_wikis_path(@project) }, ] end From 5e69ad2ceae8d3619775695b7fcab62a7a32377a Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 25 Feb 2013 22:51:15 +0200 Subject: [PATCH 38/51] Sanitize user profile input --- app/controllers/profiles_controller.rb | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb index 051a6664..6fa114a4 100644 --- a/app/controllers/profiles_controller.rb +++ b/app/controllers/profiles_controller.rb @@ -1,4 +1,6 @@ class ProfilesController < ApplicationController + include ActionView::Helpers::SanitizeHelper + before_filter :user layout 'profile' @@ -12,7 +14,7 @@ class ProfilesController < ApplicationController end def update - if @user.update_attributes(params[:user]) + if @user.update_attributes(user_attributes) flash[:notice] = "Profile was successfully updated" else flash[:alert] = "Failed to update profile" @@ -65,4 +67,17 @@ class ProfilesController < ApplicationController def user @user = current_user end + + def user_attributes + user_attributes = params[:user] + + # Sanitize user input because we dont have strict + # validation for this fields + %w(name skype linkedin twitter bio).each do |attr| + value = user_attributes[attr] + user_attributes[attr] = sanitize(value) if value.present? + end + + user_attributes + end end From 8de19b259ec4f451852ffaaaf7f1010dc05a6c2b Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 25 Feb 2013 22:56:10 +0200 Subject: [PATCH 39/51] proper name for issue partial --- app/views/dashboard/issues.html.haml | 2 +- app/views/groups/issues.html.haml | 2 +- app/views/issues/{_show.html.haml => _issue.html.haml} | 0 app/views/issues/_issues.html.haml | 3 +-- app/views/teams/issues.html.haml | 2 +- 5 files changed, 4 insertions(+), 5 deletions(-) rename app/views/issues/{_show.html.haml => _issue.html.haml} (100%) diff --git a/app/views/dashboard/issues.html.haml b/app/views/dashboard/issues.html.haml index affe01a7..539c5765 100644 --- a/app/views/dashboard/issues.html.haml +++ b/app/views/dashboard/issues.html.haml @@ -17,7 +17,7 @@ = link_to_project project %ul.well-list.issues_table - group[1].each do |issue| - = render(partial: 'issues/show', locals: {issue: issue}) + = render issue %hr = paginate @issues, theme: "gitlab" - else diff --git a/app/views/groups/issues.html.haml b/app/views/groups/issues.html.haml index 94682bdd..96aa2a16 100644 --- a/app/views/groups/issues.html.haml +++ b/app/views/groups/issues.html.haml @@ -16,7 +16,7 @@ = link_to_project project %ul.well-list.issues_table - group[1].each do |issue| - = render(partial: 'issues/show', locals: {issue: issue}) + = render issue %hr = paginate @issues, theme: "gitlab" - else diff --git a/app/views/issues/_show.html.haml b/app/views/issues/_issue.html.haml similarity index 100% rename from app/views/issues/_show.html.haml rename to app/views/issues/_issue.html.haml diff --git a/app/views/issues/_issues.html.haml b/app/views/issues/_issues.html.haml index 3bbd293d..dc7db906 100644 --- a/app/views/issues/_issues.html.haml +++ b/app/views/issues/_issues.html.haml @@ -1,5 +1,4 @@ -- @issues.each do |issue| - = render(partial: 'issues/show', locals: {issue: issue}) += render @issues - if @issues.present? %li.bottom diff --git a/app/views/teams/issues.html.haml b/app/views/teams/issues.html.haml index c6a68c37..5b17c5d4 100644 --- a/app/views/teams/issues.html.haml +++ b/app/views/teams/issues.html.haml @@ -16,7 +16,7 @@ = link_to_project @project %ul.well-list.issues_table - group[1].each do |issue| - = render(partial: 'issues/show', locals: {issue: issue}) + = render issue %hr = paginate @issues, theme: "gitlab" - else From 39fe9b644f200d4eeee30d4bc43486d177fd9b03 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 25 Feb 2013 23:10:50 +0200 Subject: [PATCH 40/51] Add close issue to note actions bar --- app/views/issues/show.html.haml | 13 ++++++++++--- app/views/notes/_form.html.haml | 2 ++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/app/views/issues/show.html.haml b/app/views/issues/show.html.haml index f1a97e10..2997cde1 100644 --- a/app/views/issues/show.html.haml +++ b/app/views/issues/show.html.haml @@ -6,15 +6,16 @@ = @issue.created_at.stamp("Aug 21, 2011") %span.pull-right - - if can?(current_user, :admin_project, @project) || @issue.author == current_user + - if can?(current_user, :modify_issue, @issue) - if @issue.closed? = link_to 'Reopen', project_issue_path(@project, @issue, issue: {state_event: :reopen }, status_only: true), method: :put, class: "btn grouped reopen_issue" - else = link_to 'Close', project_issue_path(@project, @issue, issue: {state_event: :close }, status_only: true), method: :put, class: "btn grouped close_issue", title: "Close Issue" - - if can?(current_user, :admin_project, @project) || @issue.author == current_user + + - if can?(current_user, :admin_issue, @issue) = link_to edit_project_issue_path(@project, @issue), class: "btn grouped" do %i.icon-edit - Edit + Edit .pull-right .span3#votes= render 'votes/votes_block', votable: @issue @@ -55,5 +56,11 @@ = preserve do = markdown @issue.description +- content_for :note_actions do + - if can?(current_user, :modify_issue, @issue) + - if @issue.closed? + = link_to 'Reopen Issue', project_issue_path(@project, @issue, issue: {state_event: :reopen }, status_only: true), method: :put, class: "btn grouped reopen_issue" + - else + = link_to 'Close Issue', project_issue_path(@project, @issue, issue: {state_event: :close }, status_only: true), method: :put, class: "btn grouped close_issue", title: "Close Issue" .voting_notes#notes= render "notes/notes_with_form" diff --git a/app/views/notes/_form.html.haml b/app/views/notes/_form.html.haml index a154c31e..eadf5bc6 100644 --- a/app/views/notes/_form.html.haml +++ b/app/views/notes/_form.html.haml @@ -22,6 +22,8 @@ .note-form-actions .buttons = f.submit 'Add Comment', class: "btn comment-btn grouped js-comment-button" + = yield(:note_actions) + %a.btn.grouped.js-close-discussion-note-form Cancel .note-form-option From c9b1df1201dccded5fdfb4835209af9c0f258c55 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 25 Feb 2013 23:16:34 +0200 Subject: [PATCH 41/51] Fixing tests --- spec/workers/post_receive_spec.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index d38cd59e..a4751bd0 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -21,7 +21,6 @@ describe PostReceive do it "does not run if the author is not in the project" do Key.stub(find_by_id: nil) - project.should_not_receive(:observe_push) project.should_not_receive(:execute_hooks) PostReceive.new.perform(pwd(project), 'sha-old', 'sha-new', 'refs/heads/master', key_id).should be_false @@ -32,7 +31,6 @@ describe PostReceive do project.should_receive(:execute_hooks) project.should_receive(:execute_services) project.should_receive(:update_merge_requests) - project.should_receive(:observe_push) PostReceive.new.perform(pwd(project), 'sha-old', 'sha-new', 'refs/heads/master', key_id) end From 573942263adf806b6eb61d04ba6e748334a4a519 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 26 Feb 2013 10:17:09 +0200 Subject: [PATCH 42/51] Fix issue edit button showup --- app/views/issues/show.html.haml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/views/issues/show.html.haml b/app/views/issues/show.html.haml index 2997cde1..70f94e52 100644 --- a/app/views/issues/show.html.haml +++ b/app/views/issues/show.html.haml @@ -12,10 +12,9 @@ - else = link_to 'Close', project_issue_path(@project, @issue, issue: {state_event: :close }, status_only: true), method: :put, class: "btn grouped close_issue", title: "Close Issue" - - if can?(current_user, :admin_issue, @issue) = link_to edit_project_issue_path(@project, @issue), class: "btn grouped" do %i.icon-edit - Edit + Edit .pull-right .span3#votes= render 'votes/votes_block', votable: @issue From 5ac5510fd68fa060986733c07421d8411731ee24 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 26 Feb 2013 10:47:27 +0200 Subject: [PATCH 43/51] Fix automerge detection client-side since #3056 --- app/controllers/merge_requests_controller.rb | 2 +- app/views/merge_requests/_show.html.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index b9bc99ec..67f96178 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -75,7 +75,7 @@ class MergeRequestsController < ProjectResourceController if @merge_request.unchecked? @merge_request.check_if_can_be_merged end - render json: {merge_status: @merge_request.human_merge_status_name} + render json: {merge_status: @merge_request.merge_status_name} rescue Gitlab::SatelliteNotExistError render json: {merge_status: :no_satellite} end diff --git a/app/views/merge_requests/_show.html.haml b/app/views/merge_requests/_show.html.haml index 6b6100f5..08b80172 100644 --- a/app/views/merge_requests/_show.html.haml +++ b/app/views/merge_requests/_show.html.haml @@ -32,7 +32,7 @@ check_enable: #{@merge_request.unchecked? ? "true" : "false"}, url_to_ci_check: "#{ci_status_project_merge_request_path(@project, @merge_request)}", ci_enable: #{@project.gitlab_ci? ? "true" : "false"}, - current_status: "#{@merge_request.human_merge_status_name}", + current_status: "#{@merge_request.merge_status_name}", action: "#{controller.action_name}" }); }); From c08f19f275182a24fa675c31d630126c75b50af9 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 26 Feb 2013 11:09:32 +0200 Subject: [PATCH 44/51] email validation added --- app/models/user.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/user.rb b/app/models/user.rb index 4ed31c7e..b72349f8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -70,6 +70,7 @@ class User < ActiveRecord::Base has_many :team_projects, through: :user_team_project_relationships validates :name, presence: true + validates :email, presence: true, format: { with: /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/ } validates :bio, length: { within: 0..255 } validates :extern_uid, allow_blank: true, uniqueness: {scope: :provider} validates :projects_limit, presence: true, numericality: {greater_than_or_equal_to: 0} From 15c0e58a49d623a0f8747e1d7e74364324eeb79f Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 26 Feb 2013 11:11:43 +0200 Subject: [PATCH 45/51] remove unused code related to gitolite --- app/models/key.rb | 9 --------- app/models/project_team.rb | 22 ---------------------- app/models/user.rb | 11 ----------- 3 files changed, 42 deletions(-) diff --git a/app/models/key.rb b/app/models/key.rb index edb0bcd6..53eee511 100644 --- a/app/models/key.rb +++ b/app/models/key.rb @@ -21,7 +21,6 @@ class Key < ActiveRecord::Base attr_accessible :key, :title before_validation :strip_white_space - before_save :set_identifier validates :title, presence: true, length: { within: 0..255 } validates :key, presence: true, length: { within: 0..5000 }, format: { :with => /ssh-.{3} / }, uniqueness: true @@ -48,14 +47,6 @@ class Key < ActiveRecord::Base errors.add(:key, "can't be fingerprinted") if $?.exitstatus != 0 end - def set_identifier - if is_deploy_key - self.identifier = "deploy_#{Digest::MD5.hexdigest(key)}" - else - self.identifier = "#{user.identifier}_#{Time.now.to_i}" - end - end - def is_deploy_key !!project_id end diff --git a/app/models/project_team.rb b/app/models/project_team.rb index c2cf83c0..f3e5c0e5 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -66,28 +66,6 @@ class ProjectTeam members.masters.map(&:user) end - def repository_readers - repository_members[UsersProject::REPORTER] - end - - def repository_writers - repository_members[UsersProject::DEVELOPER] - end - - def repository_masters - repository_members[UsersProject::MASTER] - end - - def repository_members - keys = Hash.new {|h,k| h[k] = [] } - UsersProject.select("keys.identifier, project_access"). - joins(user: :keys).where(project_id: project.id). - each {|row| keys[row.project_access] << [row.identifier] } - - keys[UsersProject::REPORTER] += project.deploy_keys.pluck(:identifier) - keys - end - def import(source_project) target_project = project diff --git a/app/models/user.rb b/app/models/user.rb index b72349f8..cd0754d7 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -216,17 +216,6 @@ class User < ActiveRecord::Base UsersProject.where(project_id: authorized_projects.map(&:id), user_id: self.id) end - # Returns a string for use as a Gitolite user identifier - # - # Note that Gitolite 2.x requires the following pattern for users: - # - # ^@?[0-9a-zA-Z][0-9a-zA-Z._\@+-]*$ - def identifier - # Replace non-word chars with underscores, then make sure it starts with - # valid chars - email.gsub(/\W/, '_').gsub(/\A([\W\_])+/, '') - end - def is_admin? admin end From 9c252a60c195cb348c9776f40ca2c6b1f33723f9 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 26 Feb 2013 11:17:02 +0200 Subject: [PATCH 46/51] remove tests for unexisting methods --- spec/models/project_team_spec.rb | 3 --- spec/models/user_spec.rb | 18 ------------------ 2 files changed, 21 deletions(-) diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index 7803811f..3e3543e8 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -10,9 +10,6 @@ describe ProjectTeam do it { should respond_to(:masters) } it { should respond_to(:reporters) } it { should respond_to(:guests) } - it { should respond_to(:repository_writers) } - it { should respond_to(:repository_masters) } - it { should respond_to(:repository_readers) } end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 8ab0a034..40047b35 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -69,28 +69,10 @@ describe User do describe "Respond to" do it { should respond_to(:is_admin?) } - it { should respond_to(:identifier) } it { should respond_to(:name) } it { should respond_to(:private_token) } end - describe '#identifier' do - it "should return valid identifier" do - user = build(:user, email: "test@mail.com") - user.identifier.should == "test_mail_com" - end - - it "should return identifier without + sign" do - user = build(:user, email: "test+foo@mail.com") - user.identifier.should == "test_foo_mail_com" - end - - it "should conform to Gitolite's required identifier pattern" do - user = build(:user, email: "_test@example.com") - user.identifier.should == 'test_example_com' - end - end - describe '#generate_password' do it "should execute callback when force_random_password specified" do user = build(:user, force_random_password: true) From 2c5e4955c020eb8d5a28a48d6adc375c327523ac Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 26 Feb 2013 22:53:46 +0200 Subject: [PATCH 47/51] specs for Gitlab::Popen --- spec/lib/popen_spec.rb | 29 +++++++++++++++++++++++++++++ spec/support/db_cleaner.rb | 8 ++++++-- 2 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 spec/lib/popen_spec.rb diff --git a/spec/lib/popen_spec.rb b/spec/lib/popen_spec.rb new file mode 100644 index 00000000..f5b3f947 --- /dev/null +++ b/spec/lib/popen_spec.rb @@ -0,0 +1,29 @@ +require 'spec_helper' + +describe 'Gitlab::Popen', no_db: true do + let (:path) { Rails.root.join('tmp').to_s } + + before do + @klass = Class.new(Object) + @klass.send(:include, Gitlab::Popen) + end + + context 'zero status' do + before do + @output, @status = @klass.new.popen('ls', path) + end + + it { @status.should be_zero } + it { @output.should include('pids') } + end + + context 'non-zero status' do + before do + @output, @status = @klass.new.popen('cat NOTHING', path) + end + + it { @status.should == 1 } + it { @output.should include('No such file or directory') } + end +end + diff --git a/spec/support/db_cleaner.rb b/spec/support/db_cleaner.rb index f1e072aa..8c9c74f1 100644 --- a/spec/support/db_cleaner.rb +++ b/spec/support/db_cleaner.rb @@ -9,10 +9,14 @@ RSpec.configure do |config| DatabaseCleaner.strategy = :transaction end - DatabaseCleaner.start + unless example.metadata[:no_db] + DatabaseCleaner.start + end end config.after do - DatabaseCleaner.clean + unless example.metadata[:no_db] + DatabaseCleaner.clean + end end end From 4e5164338a77894c68816bc1e7eec018aea8301c Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 26 Feb 2013 22:53:59 +0200 Subject: [PATCH 48/51] specs for api/internal --- lib/api/internal.rb | 6 ++ spec/requests/api/internal_spec.rb | 103 +++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+) create mode 100644 spec/requests/api/internal_spec.rb diff --git a/lib/api/internal.rb b/lib/api/internal.rb index 5d74a761..d4f72d70 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -5,6 +5,12 @@ module Gitlab # # Check if ssh key has access to project code # + # Params: + # key_id - SSH Key id + # project - project path with namespace + # action - git action (git-upload-pack or git-receive-pack) + # ref - branch name + # get "/allowed" do key = Key.find(params[:key_id]) project = Project.find_with_namespace(params[:project]) diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb new file mode 100644 index 00000000..d63429df --- /dev/null +++ b/spec/requests/api/internal_spec.rb @@ -0,0 +1,103 @@ +require 'spec_helper' + +describe Gitlab::API do + include ApiHelpers + + let(:user) { create(:user) } + let(:key) { create(:key, user: user) } + let(:project) { create(:project) } + + describe "GET /internal/check", no_db: true do + it do + get api("/internal/check") + + response.status.should == 200 + json_response['api_version'].should == Gitlab::API.version + end + end + + describe "GET /internal/discover" do + it do + get(api("/internal/discover"), key_id: key.id) + + response.status.should == 200 + + json_response['email'].should == user.email + end + end + + describe "GET /internal/allowed" do + context "access granted" do + before do + project.team << [user, :developer] + end + + context "git pull" do + it do + get( + api("/internal/allowed"), + ref: 'master', + key_id: key.id, + project: project.path_with_namespace, + action: 'git-upload-pack' + ) + + response.status.should == 200 + response.body.should == 'true' + end + end + + context "git push" do + it do + get( + api("/internal/allowed"), + ref: 'master', + key_id: key.id, + project: project.path_with_namespace, + action: 'git-receive-pack' + ) + + response.status.should == 200 + response.body.should == 'true' + end + end + end + + context "access denied" do + before do + project.team << [user, :guest] + end + + context "git pull" do + it do + get( + api("/internal/allowed"), + ref: 'master', + key_id: key.id, + project: project.path_with_namespace, + action: 'git-upload-pack' + ) + + response.status.should == 200 + response.body.should == 'false' + end + end + + context "git push" do + it do + get( + api("/internal/allowed"), + ref: 'master', + key_id: key.id, + project: project.path_with_namespace, + action: 'git-receive-pack' + ) + + response.status.should == 200 + response.body.should == 'false' + end + end + end + + end +end From 0cf0487d65a482b1e9d8dfe69899e333216a387c Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 26 Feb 2013 23:04:14 +0200 Subject: [PATCH 49/51] Fix TestHookContext --- app/contexts/test_hook_context.rb | 3 +-- app/services/git_push_service.rb | 10 ++++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/app/contexts/test_hook_context.rb b/app/contexts/test_hook_context.rb index d2d82a52..63eda6c7 100644 --- a/app/contexts/test_hook_context.rb +++ b/app/contexts/test_hook_context.rb @@ -1,8 +1,7 @@ class TestHookContext < BaseContext def execute hook = project.hooks.find(params[:id]) - commits = project.repository.commits(project.default_branch, nil, 3) - data = project.post_receive_data(commits.last.id, commits.first.id, "refs/heads/#{project.default_branch}", current_user) + data = GitPushService.new.sample_data(project, current_user) hook.execute(data) end end diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index 55cf31cd..40d57c67 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -31,6 +31,16 @@ class GitPushService create_push_event end + # This method provide a sample data + # generated with post_receive_data method + # for given project + # + def sample_data(project, user) + @project, @user = project, user + commits = project.repository.commits(project.default_branch, nil, 3) + post_receive_data(commits.last.id, commits.first.id, "refs/heads/#{project.default_branch}") + end + protected def create_push_event From cba6e9243620e2ddcb15c749d626156c5ce1c063 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 26 Feb 2013 23:14:32 +0200 Subject: [PATCH 50/51] move transfer logic out of project to service --- app/models/project.rb | 30 +++------------------ app/services/project_transfer_service.rb | 34 ++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 26 deletions(-) create mode 100644 app/services/project_transfer_service.rb diff --git a/app/models/project.rb b/app/models/project.rb index 6da1b0b1..6ff2a369 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -247,32 +247,6 @@ class Project < ActiveRecord::Base users_projects.find_by_user_id(user_id) end - def transfer(new_namespace) - Project.transaction do - old_namespace = namespace - self.namespace = new_namespace - - old_dir = old_namespace.try(:path) || '' - new_dir = new_namespace.try(:path) || '' - - old_repo = if old_dir.present? - File.join(old_dir, self.path) - else - self.path - end - - if Project.where(path: self.path, namespace_id: new_namespace.try(:id)).present? - raise TransferError.new("Project with same path in target namespace already exists") - end - - Gitlab::ProjectMover.new(self, old_dir, new_dir).execute - - save! - end - rescue Gitlab::ProjectMover::ProjectMoveError => ex - raise Project::TransferError.new(ex.message) - end - def name_with_namespace @name_with_namespace ||= begin if namespace @@ -295,6 +269,10 @@ class Project < ActiveRecord::Base end end + def transfer(new_namespace) + ProjectTransferService.new.transfer(self, new_namespace) + end + def execute_hooks(data) hooks.each { |hook| hook.async_execute(data) } end diff --git a/app/services/project_transfer_service.rb b/app/services/project_transfer_service.rb new file mode 100644 index 00000000..f91a3cd1 --- /dev/null +++ b/app/services/project_transfer_service.rb @@ -0,0 +1,34 @@ +# ProjectTransferService class +# +# Used for transfer project to another namespace +# +class ProjectTransferService + attr_accessor :project + + def transfer(project, new_namespace) + Project.transaction do + old_namespace = project.namespace + project.namespace = new_namespace + + old_dir = old_namespace.try(:path) || '' + new_dir = new_namespace.try(:path) || '' + + old_repo = if old_dir.present? + File.join(old_dir, project.path) + else + project.path + end + + if Project.where(path: project.path, namespace_id: new_namespace.try(:id)).present? + raise TransferError.new("Project with same path in target namespace already exists") + end + + Gitlab::ProjectMover.new(project, old_dir, new_dir).execute + + save! + end + rescue Gitlab::ProjectMover::ProjectMoveError => ex + raise Project::TransferError.new(ex.message) + end +end + From 135418dcbf72d264a846649b95ea8e6d8a2aadcf Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 27 Feb 2013 08:24:12 +0200 Subject: [PATCH 51/51] fixed popen test --- spec/lib/popen_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/popen_spec.rb b/spec/lib/popen_spec.rb index f5b3f947..4791be41 100644 --- a/spec/lib/popen_spec.rb +++ b/spec/lib/popen_spec.rb @@ -14,7 +14,7 @@ describe 'Gitlab::Popen', no_db: true do end it { @status.should be_zero } - it { @output.should include('pids') } + it { @output.should include('cache') } end context 'non-zero status' do