From c983e8eb3d9cac01090b8657735544f71f891576 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 22 Jan 2014 14:22:47 +0200 Subject: [PATCH 01/18] Create separate table to store Merge request commits/diffs Signed-off-by: Dmitriy Zaporozhets --- ...140122112253_create_merge_request_diffs.rb | 12 +++ db/migrate/20140122114406_migrate_mr_diffs.rb | 11 +++ db/schema.rb | 83 ++++++++++--------- 3 files changed, 67 insertions(+), 39 deletions(-) create mode 100644 db/migrate/20140122112253_create_merge_request_diffs.rb create mode 100644 db/migrate/20140122114406_migrate_mr_diffs.rb diff --git a/db/migrate/20140122112253_create_merge_request_diffs.rb b/db/migrate/20140122112253_create_merge_request_diffs.rb new file mode 100644 index 00000000000..f016ef0f061 --- /dev/null +++ b/db/migrate/20140122112253_create_merge_request_diffs.rb @@ -0,0 +1,12 @@ +class CreateMergeRequestDiffs < ActiveRecord::Migration + def change + create_table :merge_request_diffs do |t| + t.string :state, null: false, default: 'valid' + t.text :st_commits, null: true, limit: 2147483647 + t.text :st_diffs, null: true, limit: 2147483647 + t.integer :merge_request_id, null: false + + t.timestamps + end + end +end diff --git a/db/migrate/20140122114406_migrate_mr_diffs.rb b/db/migrate/20140122114406_migrate_mr_diffs.rb new file mode 100644 index 00000000000..2cc5faaa851 --- /dev/null +++ b/db/migrate/20140122114406_migrate_mr_diffs.rb @@ -0,0 +1,11 @@ +class MigrateMrDiffs < ActiveRecord::Migration + def self.up + execute "INSERT INTO merge_request_diffs ( merge_request_id ) SELECT id FROM merge_requests" + execute "UPDATE merge_requests mr, merge_request_diffs md SET md.st_commits = mr.st_commits WHERE md.merge_request_id = mr.id" + execute "UPDATE merge_requests mr, merge_request_diffs md SET md.st_diffs = mr.st_diffs WHERE md.merge_request_id = mr.id" + end + + def self.down + MergeRequestDiff.delete_all + end +end diff --git a/db/schema.rb b/db/schema.rb index 73d0a92e1cc..50170bf4a29 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20140116231608) do +ActiveRecord::Schema.define(version: 20140122114406) do create_table "broadcast_messages", force: true do |t| t.text "message", null: false @@ -66,8 +66,8 @@ ActiveRecord::Schema.define(version: 20140116231608) do t.integer "assignee_id" t.integer "author_id" t.integer "project_id" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.datetime "created_at" + t.datetime "updated_at" t.integer "position", default: 0 t.string "branch_name" t.text "description" @@ -85,8 +85,8 @@ ActiveRecord::Schema.define(version: 20140116231608) do create_table "keys", force: true do |t| t.integer "user_id" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.datetime "created_at" + t.datetime "updated_at" t.text "key" t.string "title" t.string "type" @@ -95,6 +95,15 @@ ActiveRecord::Schema.define(version: 20140116231608) do add_index "keys", ["user_id"], name: "index_keys_on_user_id", using: :btree + create_table "merge_request_diffs", force: true do |t| + t.string "state", default: "valid", null: false + t.text "st_commits", limit: 2147483647 + t.text "st_diffs", limit: 2147483647 + t.integer "merge_request_id", null: false + t.datetime "created_at" + t.datetime "updated_at" + end + create_table "merge_requests", force: true do |t| t.string "target_branch", null: false t.string "source_branch", null: false @@ -102,8 +111,8 @@ ActiveRecord::Schema.define(version: 20140116231608) do 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" + t.datetime "updated_at" t.text "st_commits", limit: 2147483647 t.text "st_diffs", limit: 2147483647 t.integer "milestone_id" @@ -156,8 +165,8 @@ ActiveRecord::Schema.define(version: 20140116231608) do t.text "note" t.string "noteable_type" t.integer "author_id" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.datetime "created_at" + t.datetime "updated_at" t.integer "project_id" t.string "attachment" t.string "line_code" @@ -179,8 +188,8 @@ ActiveRecord::Schema.define(version: 20140116231608) 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" + t.datetime "updated_at" t.integer "creator_id" t.boolean "issues_enabled", default: true, null: false t.boolean "wall_enabled", default: true, null: false @@ -231,8 +240,8 @@ ActiveRecord::Schema.define(version: 20140116231608) do t.text "content", limit: 2147483647 t.integer "author_id", null: false t.integer "project_id" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.datetime "created_at" + t.datetime "updated_at" t.string "file_name" t.datetime "expires_at" t.boolean "private", default: true, null: false @@ -254,45 +263,42 @@ ActiveRecord::Schema.define(version: 20140116231608) do t.datetime "created_at" end - add_index "taggings", ["tag_id"], name: "index_taggings_on_tag_id", using: :btree - add_index "taggings", ["taggable_id", "taggable_type", "context"], name: "index_taggings_on_taggable_id_and_taggable_type_and_context", using: :btree - create_table "tags", force: true do |t| t.string "name" end create_table "users", force: true do |t| - t.string "email", default: "", null: false - t.string "encrypted_password", default: "", null: false + t.string "email", default: "", null: false + t.string "encrypted_password", limit: 128, default: "", null: false t.string "reset_password_token" t.datetime "reset_password_sent_at" t.datetime "remember_created_at" - t.integer "sign_in_count", default: 0 + t.integer "sign_in_count", default: 0 t.datetime "current_sign_in_at" t.datetime "last_sign_in_at" t.string "current_sign_in_ip" t.string "last_sign_in_ip" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.datetime "created_at" + t.datetime "updated_at" t.string "name" - t.boolean "admin", default: false, null: false - t.integer "projects_limit", default: 10 - t.string "skype", default: "", null: false - t.string "linkedin", default: "", null: false - t.string "twitter", default: "", null: false + t.boolean "admin", default: false, null: false + t.integer "projects_limit", default: 10 + t.string "skype", default: "", null: false + t.string "linkedin", default: "", null: false + t.string "twitter", default: "", null: false t.string "authentication_token" - t.integer "theme_id", default: 1, null: false + t.integer "theme_id", default: 1, null: false t.string "bio" - t.integer "failed_attempts", default: 0 + t.integer "failed_attempts", default: 0 t.datetime "locked_at" t.string "extern_uid" t.string "provider" t.string "username" - t.boolean "can_create_group", default: true, null: false - t.boolean "can_create_team", default: true, null: false + t.boolean "can_create_group", default: true, null: false + t.boolean "can_create_team", default: true, null: false t.string "state" - t.integer "color_scheme_id", default: 1, null: false - t.integer "notification_level", default: 1, null: false + t.integer "color_scheme_id", default: 1, null: false + t.integer "notification_level", default: 1, null: false t.datetime "password_expires_at" t.integer "created_by_id" t.string "avatar" @@ -300,15 +306,14 @@ ActiveRecord::Schema.define(version: 20140116231608) do t.datetime "confirmed_at" t.datetime "confirmation_sent_at" t.string "unconfirmed_email" - t.boolean "hide_no_ssh_key", default: false - t.string "website_url", default: "", null: false + t.boolean "hide_no_ssh_key", default: false + t.string "website_url", default: "", null: false end add_index "users", ["admin"], name: "index_users_on_admin", using: :btree add_index "users", ["authentication_token"], name: "index_users_on_authentication_token", unique: true, using: :btree add_index "users", ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true, using: :btree add_index "users", ["email"], name: "index_users_on_email", unique: true, using: :btree - add_index "users", ["extern_uid", "provider"], name: "index_users_on_extern_uid_and_provider", unique: true, using: :btree add_index "users", ["name"], name: "index_users_on_name", using: :btree add_index "users", ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true, using: :btree add_index "users", ["username"], name: "index_users_on_username", using: :btree @@ -327,8 +332,8 @@ ActiveRecord::Schema.define(version: 20140116231608) do create_table "users_projects", force: true do |t| t.integer "user_id", null: false t.integer "project_id", null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.datetime "created_at" + t.datetime "updated_at" t.integer "project_access", default: 0, null: false t.integer "notification_level", default: 3, null: false end @@ -340,8 +345,8 @@ ActiveRecord::Schema.define(version: 20140116231608) do create_table "web_hooks", force: true do |t| t.string "url" t.integer "project_id" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.datetime "created_at" + t.datetime "updated_at" t.string "type", default: "ProjectHook" t.integer "service_id" t.boolean "push_events", default: true, null: false From de724a7a34a501234000a539e9ba6ce16857ad5d Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 22 Jan 2014 15:17:41 +0200 Subject: [PATCH 02/18] Improve db schema for merge request diffs Signed-off-by: Dmitriy Zaporozhets --- .../20140122112253_create_merge_request_diffs.rb | 2 +- .../20140122122549_remove_m_rdiff_fields.rb | 13 +++++++++++++ db/schema.rb | 16 +++++++--------- 3 files changed, 21 insertions(+), 10 deletions(-) create mode 100644 db/migrate/20140122122549_remove_m_rdiff_fields.rb diff --git a/db/migrate/20140122112253_create_merge_request_diffs.rb b/db/migrate/20140122112253_create_merge_request_diffs.rb index f016ef0f061..ef592305a23 100644 --- a/db/migrate/20140122112253_create_merge_request_diffs.rb +++ b/db/migrate/20140122112253_create_merge_request_diffs.rb @@ -1,7 +1,7 @@ class CreateMergeRequestDiffs < ActiveRecord::Migration def change create_table :merge_request_diffs do |t| - t.string :state, null: false, default: 'valid' + t.string :state, null: false, default: 'collected' t.text :st_commits, null: true, limit: 2147483647 t.text :st_diffs, null: true, limit: 2147483647 t.integer :merge_request_id, null: false diff --git a/db/migrate/20140122122549_remove_m_rdiff_fields.rb b/db/migrate/20140122122549_remove_m_rdiff_fields.rb new file mode 100644 index 00000000000..c27e649e9a3 --- /dev/null +++ b/db/migrate/20140122122549_remove_m_rdiff_fields.rb @@ -0,0 +1,13 @@ +class RemoveMRdiffFields < ActiveRecord::Migration + def up + remove_column :merge_requests, :st_commits + remove_column :merge_requests, :st_diffs + end + + def down + add_column :merge_requests, :st_commits, :text, null: true, limit: 2147483647 + add_column :merge_requests, :st_diffs, :text, null: true, limit: 2147483647 + execute "UPDATE merge_requests mr, merge_request_diffs md SET mr.st_commits = md.st_commits WHERE md.merge_request_id = mr.id" + execute "UPDATE merge_requests mr, merge_request_diffs md SET mr.st_diffs = md.st_diffs WHERE md.merge_request_id = mr.id" + end +end diff --git a/db/schema.rb b/db/schema.rb index 50170bf4a29..75a44d9aa04 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20140122114406) do +ActiveRecord::Schema.define(version: 20140122122549) do create_table "broadcast_messages", force: true do |t| t.text "message", null: false @@ -96,29 +96,27 @@ ActiveRecord::Schema.define(version: 20140122114406) do add_index "keys", ["user_id"], name: "index_keys_on_user_id", using: :btree create_table "merge_request_diffs", force: true do |t| - t.string "state", default: "valid", null: false + t.string "state", default: "collected", null: false t.text "st_commits", limit: 2147483647 t.text "st_diffs", limit: 2147483647 - t.integer "merge_request_id", null: false + t.integer "merge_request_id", null: false t.datetime "created_at" t.datetime "updated_at" end create_table "merge_requests", force: true do |t| - t.string "target_branch", null: false - t.string "source_branch", null: false - t.integer "source_project_id", null: false + t.string "target_branch", null: false + t.string "source_branch", null: false + t.integer "source_project_id", null: false t.integer "author_id" t.integer "assignee_id" t.string "title" t.datetime "created_at" t.datetime "updated_at" - t.text "st_commits", limit: 2147483647 - t.text "st_diffs", limit: 2147483647 t.integer "milestone_id" t.string "state" t.string "merge_status" - t.integer "target_project_id", null: false + t.integer "target_project_id", null: false t.integer "iid" t.text "description" end From d9be420ee603fef634107c576b41715de18b5385 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 22 Jan 2014 15:19:02 +0200 Subject: [PATCH 03/18] MergeRequestDiff model responsoible for storing commits/diff info Signed-off-by: Dmitriy Zaporozhets --- app/models/merge_request_diff.rb | 160 +++++++++++++++++++++++++++++++ 1 file changed, 160 insertions(+) create mode 100644 app/models/merge_request_diff.rb diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb new file mode 100644 index 00000000000..d06b95daa6a --- /dev/null +++ b/app/models/merge_request_diff.rb @@ -0,0 +1,160 @@ +require Rails.root.join("app/models/commit") + +class MergeRequestDiff < ActiveRecord::Base + # Prevent store of diff + # if commits amount more then 200 + COMMITS_SAFE_SIZE = 200 + + attr_reader :commits, :diffs + + belongs_to :merge_request + + attr_accessible :state, :st_commits, :st_diffs + + delegate :target_branch, :source_branch, to: :merge_request, prefix: nil + + state_machine :state, initial: :empty do + state :collected + state :timeout + state :overflow_commits_safe_size + state :overflow_diff_files_limit + state :overflow_diff_lines_limit + end + + serialize :st_commits + serialize :st_diffs + + after_create :reload_content + + def reload_content + reload_commits + reload_diffs + end + + def diffs + @diffs ||= (load_diffs(st_diffs) || []) + end + + def commits + @commits ||= load_commits(st_commits || []) + end + + def last_commit + commits.first + end + + def last_commit_short_sha + @last_commit_short_sha ||= last_commit.sha[0..10] + end + + private + + def dump_commits(commits) + commits.map(&:to_hash) + end + + def load_commits(array) + array.map { |hash| Commit.new(Gitlab::Git::Commit.new(hash)) } + end + + def dump_diffs(diffs) + if diffs.respond_to?(:map) + diffs.map(&:to_hash) + end + end + + def load_diffs(raw) + if raw.respond_to?(:map) + raw.map { |hash| Gitlab::Git::Diff.new(hash) } + end + end + + # When Git::Diff is not able to get diff + # because of git timeout it return this value + def broken_diffs + [Gitlab::Git::Diff::BROKEN_DIFF] + end + + # Collect array of Git::Commit objects + # between target and source branches + def unmerged_commits + commits = if merge_request.for_fork? + Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request).commits_between + else + repository.commits_between(target_branch, source_branch) + end + + if commits.present? + commits = Commit.decorate(commits). + sort_by(&:created_at). + reverse + end + + commits + end + + # Reload all commits related to current merge request from repo + # and save it as array of hashes in st_commits db field + def reload_commits + commit_objects = unmerged_commits + + if commit_objects.present? + self.st_commits = dump_commits(commit_objects) + end + + save + end + + # Reload diffs between branches related to current merge request from repo + # and save it as array of hashes in st_diffs db field + def reload_diffs + new_diffs = [] + + if commits.size.zero? + self.state = :empty + elsif commits.size > COMMITS_SAFE_SIZE + self.state = :overflow_commits_safe_size + else + new_diffs = unmerged_diffs + end + + if new_diffs.any? + if new_diffs.size > Commit::DIFF_HARD_LIMIT_FILES + self.state = :overflow_diff_files_limit + new_diffs = [] + end + + if new_diffs.sum { |diff| diff.diff.lines.count } > Commit::DIFF_HARD_LIMIT_LINES + self.state = :overflow_diff_lines_limit + new_diffs = [] + end + end + + new_diffs = dump_commits(new_diffs) if new_diffs.present? + + self.st_diffs = new_diffs + self.save + end + + # Collect array of Git::Diff objects + # between target and source branches + def unmerged_diffs + diffs = if merge_request.for_fork? + Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request).diffs_between_satellite + else + Gitlab::Git::Diff.between(repository, source_branch, target_branch) + end + + if diffs == broken_diffs + self.state = :timeout + diffs = [] + end + + diffs ||= [] + diffs + end + + def repository + merge_request.target_project.repository + end +end From 79bfbe5918b414cc44fbb506f0b350c04f49c1e9 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 22 Jan 2014 15:20:20 +0200 Subject: [PATCH 04/18] Remove commits/diff store login from MergeRequest model Signed-off-by: Dmitriy Zaporozhets --- app/models/merge_request.rb | 119 +----------------------- app/observers/merge_request_observer.rb | 1 + 2 files changed, 6 insertions(+), 114 deletions(-) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index da7aebd944f..01538cf8c12 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -30,6 +30,9 @@ class MergeRequest < ActiveRecord::Base belongs_to :target_project, foreign_key: :target_project_id, class_name: "Project" belongs_to :source_project, foreign_key: :source_project_id, class_name: "Project" + has_one :merge_request_diff, dependent: :destroy + + delegate :commits, :diffs, :last_commit, :last_commit_short_sha, to: :merge_request_diff, prefix: nil attr_accessible :title, :assignee_id, :source_project_id, :source_branch, :target_project_id, :target_branch, :milestone_id, :author_id_of_changes, :state_event, :description @@ -53,11 +56,8 @@ class MergeRequest < ActiveRecord::Base end state :opened - state :reopened - state :closed - state :merged end @@ -75,15 +75,10 @@ class MergeRequest < ActiveRecord::Base end state :unchecked - state :can_be_merged - state :cannot_be_merged end - serialize :st_commits - serialize :st_diffs - validates :source_project, presence: true, unless: :allow_broken validates :source_branch, presence: true validates :target_project, presence: true @@ -105,7 +100,7 @@ class MergeRequest < ActiveRecord::Base scope :closed, -> { with_states(:closed, :merged) } def validate_branches - if target_project==source_project && target_branch == source_branch + if target_project == source_project && target_branch == source_branch errors.add :branch_conflict, "You can not use same project/branch for source and target" end @@ -120,8 +115,7 @@ class MergeRequest < ActiveRecord::Base end def reload_code - self.reloaded_commits - self.reloaded_diffs + merge_request_diff.reload_content if opened? end def check_if_can_be_merged @@ -132,42 +126,6 @@ class MergeRequest < ActiveRecord::Base end end - def diffs - @diffs ||= (load_diffs(st_diffs) || []) - end - - def reloaded_diffs - if opened? && unmerged_diffs.any? - self.st_diffs = dump_diffs(unmerged_diffs) - self.save - end - end - - def broken_diffs? - diffs == broken_diffs - rescue - true - end - - def valid_diffs? - !broken_diffs? - end - - def unmerged_diffs - diffs = if for_fork? - Gitlab::Satellite::MergeAction.new(author, self).diffs_between_satellite - else - Gitlab::Git::Diff.between(target_project.repository, source_branch, target_branch) - end - - diffs ||= [] - diffs - end - - def last_commit - commits.first - end - def merge_event self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::MERGED).last end @@ -176,39 +134,6 @@ class MergeRequest < ActiveRecord::Base self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::CLOSED).last end - def commits - load_commits(st_commits || []) - end - - def probably_merged? - unmerged_commits.empty? && - commits.any? && opened? - end - - def reloaded_commits - if opened? && unmerged_commits.any? - self.st_commits = dump_commits(unmerged_commits) - save - - end - commits - end - - def unmerged_commits - if for_fork? - commits = Gitlab::Satellite::MergeAction.new(self.author, self).commits_between - else - commits = target_project.repository.commits_between(self.target_branch, self.source_branch) - end - - if commits.present? - commits = Commit.decorate(commits). - sort_by(&:created_at). - reverse - end - commits - end - def merge!(user_id) self.author_id_of_changes = user_id self.merge @@ -247,10 +172,6 @@ class MergeRequest < ActiveRecord::Base Gitlab::Satellite::MergeAction.new(current_user, self).format_patch end - def last_commit_short_sha - @last_commit_short_sha ||= last_commit.sha[0..10] - end - def for_fork? target_project != source_project end @@ -327,34 +248,4 @@ class MergeRequest < ActiveRecord::Base message << description.to_s message end - - private - - def dump_commits(commits) - commits.map(&:to_hash) - end - - def load_commits(array) - array.map { |hash| Commit.new(Gitlab::Git::Commit.new(hash)) } - end - - def dump_diffs(diffs) - if diffs == broken_diffs - broken_diffs - elsif diffs.respond_to?(:map) - diffs.map(&:to_hash) - end - end - - def load_diffs(raw) - if raw == broken_diffs - broken_diffs - elsif raw.respond_to?(:map) - raw.map { |hash| Gitlab::Git::Diff.new(hash) } - end - end - - def broken_diffs - [Gitlab::Git::Diff::BROKEN_DIFF] - end end diff --git a/app/observers/merge_request_observer.rb b/app/observers/merge_request_observer.rb index 0ac555fce7c..9a3e90d83e8 100644 --- a/app/observers/merge_request_observer.rb +++ b/app/observers/merge_request_observer.rb @@ -6,6 +6,7 @@ class MergeRequestObserver < ActivityObserver create_event(merge_request, Event.determine_action(merge_request)) end + merge_request.create_merge_request_diff notification.new_merge_request(merge_request, current_user) merge_request.create_cross_references!(merge_request.project, current_user) execute_hooks(merge_request) From 193a5ed3c3c352f043752c8ed3b0f3d1cb8c0ddc Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 22 Jan 2014 15:20:40 +0200 Subject: [PATCH 05/18] Render MR diff correctly Signed-off-by: Dmitriy Zaporozhets --- app/controllers/projects/merge_requests_controller.rb | 1 + app/views/projects/merge_requests/show/_diffs.html.haml | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 0792dbf041f..0c7a44274e3 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -213,6 +213,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController # or from cache if already merged @commits = @merge_request.commits + @merge_request_diff = @merge_request.merge_request_diff @allowed_to_merge = allowed_to_merge? @show_merge_controls = @merge_request.opened? && @commits.any? && @allowed_to_merge end diff --git a/app/views/projects/merge_requests/show/_diffs.html.haml b/app/views/projects/merge_requests/show/_diffs.html.haml index 25f63804858..2c7507cfb8b 100644 --- a/app/views/projects/merge_requests/show/_diffs.html.haml +++ b/app/views/projects/merge_requests/show/_diffs.html.haml @@ -1,10 +1,10 @@ -- if @merge_request.valid_diffs? +- if @merge_request_diff.collected? = render "projects/commits/diffs", diffs: @merge_request.diffs, project: @merge_request.source_project -- elsif @merge_request.broken_diffs? +- elsif @merge_request_diff.empty? + %h4.nothing_here_message Nothing to merge from #{@merge_request.source_branch} into #{@merge_request.target_branch} +- else %h4.nothing_here_message Can't load diff. You can = link_to "download it", project_merge_request_path(@merge_request.source_project, @merge_request), format: :diff, class: "vlink" instead. -- else - %h4.nothing_here_message Nothing to merge From 39a86963e93b4a24296ffe96e63c5a189f8d1d07 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 22 Jan 2014 15:30:14 +0200 Subject: [PATCH 06/18] Set mr_diff state to collected if diff is ok Signed-off-by: Dmitriy Zaporozhets --- app/models/merge_request_diff.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index d06b95daa6a..3ea610197e6 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -130,7 +130,10 @@ class MergeRequestDiff < ActiveRecord::Base end end - new_diffs = dump_commits(new_diffs) if new_diffs.present? + if new_diffs.present? + new_diffs = dump_commits(new_diffs) + self.state = :collected + end self.st_diffs = new_diffs self.save From 51ea42e2386f109e3fb4f3619dd33eb9a5398080 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 22 Jan 2014 15:54:53 +0200 Subject: [PATCH 07/18] Improve commit render performnace for MR show page Signed-off-by: Dmitriy Zaporozhets --- app/assets/stylesheets/sections/merge_requests.scss | 13 ------------- app/models/merge_request.rb | 5 ++++- app/views/projects/commits/_inline_commit.html.haml | 6 ++++-- .../projects/merge_requests/show/_commits.html.haml | 13 ++++++++++--- 4 files changed, 18 insertions(+), 19 deletions(-) diff --git a/app/assets/stylesheets/sections/merge_requests.scss b/app/assets/stylesheets/sections/merge_requests.scss index d3462d6aaa7..4388da00735 100644 --- a/app/assets/stylesheets/sections/merge_requests.scss +++ b/app/assets/stylesheets/sections/merge_requests.scss @@ -89,16 +89,3 @@ .merge-request-form-info { padding-top: 15px; } - -.merge-request-branches { - .commit-row-message { - font-weight: normal !important; - } - - .select2-container .select2-single { - span { - font-weight: bold; - color: #555; - } - } -} diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 01538cf8c12..076463f6c40 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -150,7 +150,10 @@ class MergeRequest < ActiveRecord::Base end def mr_and_commit_notes - commit_ids = commits.map(&:id) + # Fetch comments only from last 100 commits + commits_for_notes_limit = 100 + commit_ids = commits.last(commits_for_notes_limit).map(&:id) + project.notes.where( "(noteable_type = 'MergeRequest' AND noteable_id = :mr_id) OR (noteable_type = 'Commit' AND commit_id IN (:commit_ids))", mr_id: id, diff --git a/app/views/projects/commits/_inline_commit.html.haml b/app/views/projects/commits/_inline_commit.html.haml index f5863463fee..b36369b4285 100644 --- a/app/views/projects/commits/_inline_commit.html.haml +++ b/app/views/projects/commits/_inline_commit.html.haml @@ -2,5 +2,7 @@ .commit-row-title = link_to commit.short_id(8), project_commit_path(project, commit), class: "commit_short_id"   - = link_to_gfm truncate(commit.title, length: 40), project_commit_path(project, commit.id), class: "commit-row-message" - #{time_ago_with_tooltip(commit.committed_date)}   + %span.str-truncated + = link_to_gfm commit.title, project_commit_path(project, commit.id), class: "commit-row-message" + .pull-right + #{time_ago_with_tooltip(commit.committed_date)} diff --git a/app/views/projects/merge_requests/show/_commits.html.haml b/app/views/projects/merge_requests/show/_commits.html.haml index 7b0e67053a5..8ca1326c96a 100644 --- a/app/views/projects/merge_requests/show/_commits.html.haml +++ b/app/views/projects/merge_requests/show/_commits.html.haml @@ -12,9 +12,16 @@ 8 of #{@commits.count} commits displayed. %strong %a.show-all-commits Click here to show all - %ul.all-commits.hide.well-list - - @commits.each do |commit| - = render "projects/commits/commit", commit: commit, project: @merge_request.source_project + - if @commits.size > MergeRequestDiff::COMMITS_SAFE_SIZE + %ul.all-commits.hide.well-list + - @commits.first(MergeRequestDiff::COMMITS_SAFE_SIZE).each do |commit| + = render "projects/commits/inline_commit", commit: commit, project: @merge_request.source_project + %li + other #{@commits.size - MergeRequestDiff::COMMITS_SAFE_SIZE} commits hidden top prevent performance issues. + - else + %ul.all-commits.hide.well-list + - @commits.each do |commit| + = render "projects/commits/inline_commit", commit: commit, project: @merge_request.source_project - else %ul.well-list From 4b3afe2325e063943bdec123154a576f6a300bce Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 22 Jan 2014 20:22:20 +0200 Subject: [PATCH 08/18] Always create merge_request_diff if MR created Signed-off-by: Dmitriy Zaporozhets --- app/controllers/projects/merge_requests_controller.rb | 1 - app/models/merge_request.rb | 2 ++ app/observers/merge_request_observer.rb | 1 - lib/api/merge_requests.rb | 1 - 4 files changed, 2 insertions(+), 3 deletions(-) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 0c7a44274e3..baaa3c7ba0b 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -76,7 +76,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController @merge_request.author = current_user @target_branches ||= [] if @merge_request.save - @merge_request.reload_code redirect_to [@merge_request.target_project, @merge_request], notice: 'Merge request was successfully created.' else @source_project = @merge_request.source_project diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 076463f6c40..c53b24b817d 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -30,7 +30,9 @@ class MergeRequest < ActiveRecord::Base belongs_to :target_project, foreign_key: :target_project_id, class_name: "Project" belongs_to :source_project, foreign_key: :source_project_id, class_name: "Project" + has_one :merge_request_diff, dependent: :destroy + after_create :create_merge_request_diff delegate :commits, :diffs, :last_commit, :last_commit_short_sha, to: :merge_request_diff, prefix: nil diff --git a/app/observers/merge_request_observer.rb b/app/observers/merge_request_observer.rb index 9a3e90d83e8..0ac555fce7c 100644 --- a/app/observers/merge_request_observer.rb +++ b/app/observers/merge_request_observer.rb @@ -6,7 +6,6 @@ class MergeRequestObserver < ActivityObserver create_event(merge_request, Event.determine_action(merge_request)) end - merge_request.create_merge_request_diff notification.new_merge_request(merge_request, current_user) merge_request.create_cross_references!(merge_request.project, current_user) execute_hooks(merge_request) diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 3f4bec895bf..4bd57f516db 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -88,7 +88,6 @@ module API end if merge_request.save - merge_request.reload_code present merge_request, with: Entities::MergeRequest else handle_merge_request_errors! merge_request.errors From c227aa44f9c54dd88f2ac8de7a3463a23f516dff Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 22 Jan 2014 21:03:52 +0200 Subject: [PATCH 09/18] Make changes to tests * project_with_code -> project * project -> ermpty_project Signed-off-by: Dmitriy Zaporozhets --- features/steps/dashboard/dashboard_issues.rb | 2 +- .../dashboard/dashboard_merge_requests.rb | 2 +- features/steps/project/project_fork.rb | 4 +- .../project/project_forked_merge_requests.rb | 4 +- .../steps/project/project_issue_tracker.rb | 2 +- .../steps/project/project_markdown_render.rb | 2 +- features/steps/project/redirects.rb | 2 +- features/steps/public/projects_feature.rb | 4 +- features/steps/shared/project.rb | 6 +- spec/controllers/blob_controller_spec.rb | 2 +- spec/controllers/commit_controller_spec.rb | 2 +- spec/controllers/commits_controller_spec.rb | 2 +- .../merge_requests_controller_spec.rb | 2 +- spec/controllers/tree_controller_spec.rb | 2 +- spec/factories.rb | 57 ++++--------------- .../features/gitlab_flavored_markdown_spec.rb | 2 +- spec/features/notes_on_merge_requests_spec.rb | 6 +- .../security/project/internal_access_spec.rb | 2 +- .../security/project/private_access_spec.rb | 2 +- .../security/project/public_access_spec.rb | 2 +- spec/helpers/gitlab_markdown_helper_spec.rb | 2 +- spec/helpers/search_helper_spec.rb | 2 +- spec/lib/gitlab/reference_extractor_spec.rb | 2 +- spec/lib/gitlab/satellite/action_spec.rb | 2 +- .../lib/gitlab/satellite/merge_action_spec.rb | 2 +- spec/mailers/notify_spec.rb | 2 +- spec/models/assembla_service_spec.rb | 2 +- spec/models/commit_spec.rb | 2 +- spec/models/flowdock_service_spec.rb | 2 +- spec/models/note_spec.rb | 2 +- spec/models/project_spec.rb | 4 +- spec/models/service_spec.rb | 2 +- spec/observers/merge_request_observer_spec.rb | 30 ++++------ spec/requests/api/files_spec.rb | 2 +- spec/requests/api/merge_requests_spec.rb | 6 +- spec/requests/api/project_hooks_spec.rb | 2 +- spec/requests/api/projects_spec.rb | 53 +++++++++++------ spec/requests/api/repositories_spec.rb | 2 +- spec/requests/api/services_spec.rb | 2 +- spec/services/git_push_service_spec.rb | 2 +- spec/services/test_hook_service_spec.rb | 2 +- spec/support/mentionable_shared_examples.rb | 2 +- spec/workers/post_receive_spec.rb | 2 +- 43 files changed, 109 insertions(+), 131 deletions(-) diff --git a/features/steps/dashboard/dashboard_issues.rb b/features/steps/dashboard/dashboard_issues.rb index a458acdae07..1344edfa80b 100644 --- a/features/steps/dashboard/dashboard_issues.rb +++ b/features/steps/dashboard/dashboard_issues.rb @@ -66,7 +66,7 @@ class DashboardIssues < Spinach::FeatureSteps def project @project ||= begin - project =create :project_with_code + project =create :project project.team << [current_user, :master] project end diff --git a/features/steps/dashboard/dashboard_merge_requests.rb b/features/steps/dashboard/dashboard_merge_requests.rb index 73286532b93..62d84506c49 100644 --- a/features/steps/dashboard/dashboard_merge_requests.rb +++ b/features/steps/dashboard/dashboard_merge_requests.rb @@ -66,7 +66,7 @@ class DashboardMergeRequests < Spinach::FeatureSteps def project @project ||= begin - project =create :project_with_code + project =create :project project.team << [current_user, :master] project end diff --git a/features/steps/project/project_fork.rb b/features/steps/project/project_fork.rb index 858c7d11b32..b00903b3394 100644 --- a/features/steps/project/project_fork.rb +++ b/features/steps/project/project_fork.rb @@ -12,7 +12,7 @@ class ForkProject < Spinach::FeatureSteps step 'I am a member of project "Shop"' do @project = Project.find_by_name "Shop" - @project ||= create(:project_with_code, name: "Shop", group: create(:group)) + @project ||= create(:project, name: "Shop", group: create(:group)) @project.team << [@user, :reporter] end @@ -26,7 +26,7 @@ class ForkProject < Spinach::FeatureSteps current_user.namespace ||= create(:namespace) current_user.namespace.should_not be_nil current_user.namespace.path.should_not be_nil - @my_project = create(:project_with_code, name: "Shop", namespace: current_user.namespace) + @my_project = create(:project, name: "Shop", namespace: current_user.namespace) end step 'I should see a "Name has already been taken" warning' do diff --git a/features/steps/project/project_forked_merge_requests.rb b/features/steps/project/project_forked_merge_requests.rb index d34aeac30bb..57ab6439564 100644 --- a/features/steps/project/project_forked_merge_requests.rb +++ b/features/steps/project/project_forked_merge_requests.rb @@ -7,7 +7,7 @@ class ProjectForkedMergeRequests < Spinach::FeatureSteps step 'I am a member of project "Shop"' do @project = Project.find_by_name "Shop" - @project ||= create(:project_with_code, name: "Shop") + @project ||= create(:project, name: "Shop") @project.team << [@user, :reporter] end @@ -15,7 +15,7 @@ class ProjectForkedMergeRequests < Spinach::FeatureSteps @forking_user = @user forked_project_link = build(:forked_project_link) @forked_project = Project.find_by_name "Forked Shop" - @forked_project ||= create(:source_project_with_code, name: "Forked Shop", forked_project_link: forked_project_link, creator_id: @forking_user.id , namespace: @forking_user.namespace) + @forked_project ||= create(:source_project, name: "Forked Shop", forked_project_link: forked_project_link, creator_id: @forking_user.id , namespace: @forking_user.namespace) forked_project_link.forked_from_project = @project forked_project_link.forked_to_project = @forked_project diff --git a/features/steps/project/project_issue_tracker.rb b/features/steps/project/project_issue_tracker.rb index a05d7a0bc37..65e7e4de9df 100644 --- a/features/steps/project/project_issue_tracker.rb +++ b/features/steps/project/project_issue_tracker.rb @@ -5,7 +5,7 @@ class ProjectIssueTracker < Spinach::FeatureSteps step 'project "Shop" has issues enabled' do @project = Project.find_by_name "Shop" - @project ||= create(:project_with_code, name: "Shop", namespace: @user.namespace) + @project ||= create(:project, name: "Shop", namespace: @user.namespace) @project.issues_enabled = true end diff --git a/features/steps/project/project_markdown_render.rb b/features/steps/project/project_markdown_render.rb index da044e46ebd..bef62c61bc4 100644 --- a/features/steps/project/project_markdown_render.rb +++ b/features/steps/project/project_markdown_render.rb @@ -4,7 +4,7 @@ class Spinach::Features::ProjectMarkdownRender < Spinach::FeatureSteps And 'I own project "Delta"' do @project = Project.find_by_name "Delta" - @project ||= create(:project_with_code, name: "Delta", namespace: @user.namespace) + @project ||= create(:project, name: "Delta", namespace: @user.namespace) @project.team << [@user, :master] end diff --git a/features/steps/project/redirects.rb b/features/steps/project/redirects.rb index 4ac53075704..8015ae833df 100644 --- a/features/steps/project/redirects.rb +++ b/features/steps/project/redirects.rb @@ -4,7 +4,7 @@ class Spinach::Features::ProjectRedirects < Spinach::FeatureSteps include SharedProject step 'public project "Community"' do - create :project_with_code, name: 'Community', visibility_level: Gitlab::VisibilityLevel::PUBLIC + create :project, name: 'Community', visibility_level: Gitlab::VisibilityLevel::PUBLIC end step 'private project "Enterprise"' do diff --git a/features/steps/public/projects_feature.rb b/features/steps/public/projects_feature.rb index 47e52f47d07..2292fdad6fb 100644 --- a/features/steps/public/projects_feature.rb +++ b/features/steps/public/projects_feature.rb @@ -25,7 +25,7 @@ class Spinach::Features::PublicProjectsFeature < Spinach::FeatureSteps end step 'public project "Community"' do - create :project_with_code, name: 'Community', visibility_level: Gitlab::VisibilityLevel::PUBLIC + create :project, name: 'Community', visibility_level: Gitlab::VisibilityLevel::PUBLIC end step 'public empty project "Empty Public Project"' do @@ -76,7 +76,7 @@ class Spinach::Features::PublicProjectsFeature < Spinach::FeatureSteps end step 'internal project "Internal"' do - create :project_with_code, name: 'Internal', visibility_level: Gitlab::VisibilityLevel::INTERNAL + create :project, name: 'Internal', visibility_level: Gitlab::VisibilityLevel::INTERNAL end step 'I should see project "Internal"' do diff --git a/features/steps/shared/project.rb b/features/steps/shared/project.rb index 3dc4932a09a..97b036ae743 100644 --- a/features/steps/shared/project.rb +++ b/features/steps/shared/project.rb @@ -3,21 +3,21 @@ module SharedProject # Create a project without caring about what it's called And "I own a project" do - @project = create(:project_with_code, namespace: @user.namespace) + @project = create(:project, namespace: @user.namespace) @project.team << [@user, :master] end # Create a specific project called "Shop" And 'I own project "Shop"' do @project = Project.find_by_name "Shop" - @project ||= create(:project_with_code, name: "Shop", namespace: @user.namespace) + @project ||= create(:project, name: "Shop", namespace: @user.namespace) @project.team << [@user, :master] end # Create another specific project called "Forum" And 'I own project "Forum"' do @project = Project.find_by_name "Forum" - @project ||= create(:project_with_code, name: "Forum", namespace: @user.namespace, path: 'forum_project') + @project ||= create(:project, name: "Forum", namespace: @user.namespace, path: 'forum_project') @project.team << [@user, :master] end diff --git a/spec/controllers/blob_controller_spec.rb b/spec/controllers/blob_controller_spec.rb index 479d8fc1a1d..cea6922e1c3 100644 --- a/spec/controllers/blob_controller_spec.rb +++ b/spec/controllers/blob_controller_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Projects::BlobController do - let(:project) { create(:project_with_code) } + let(:project) { create(:project) } let(:user) { create(:user) } before do diff --git a/spec/controllers/commit_controller_spec.rb b/spec/controllers/commit_controller_spec.rb index fdf0884f4e2..f5822157ea4 100644 --- a/spec/controllers/commit_controller_spec.rb +++ b/spec/controllers/commit_controller_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Projects::CommitController do - let(:project) { create(:project_with_code) } + let(:project) { create(:project) } let(:user) { create(:user) } let(:commit) { project.repository.commit("master") } diff --git a/spec/controllers/commits_controller_spec.rb b/spec/controllers/commits_controller_spec.rb index 8263afc97a2..fbf4f29acfd 100644 --- a/spec/controllers/commits_controller_spec.rb +++ b/spec/controllers/commits_controller_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Projects::CommitsController do - let(:project) { create(:project_with_code) } + let(:project) { create(:project) } let(:user) { create(:user) } before do diff --git a/spec/controllers/merge_requests_controller_spec.rb b/spec/controllers/merge_requests_controller_spec.rb index f237f358452..1502bded97f 100644 --- a/spec/controllers/merge_requests_controller_spec.rb +++ b/spec/controllers/merge_requests_controller_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Projects::MergeRequestsController do - let(:project) { create(:project_with_code) } + let(:project) { create(:project) } let(:user) { create(:user) } let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project, target_branch: "stable", source_branch: "master") } diff --git a/spec/controllers/tree_controller_spec.rb b/spec/controllers/tree_controller_spec.rb index bb1232e6264..479118a3465 100644 --- a/spec/controllers/tree_controller_spec.rb +++ b/spec/controllers/tree_controller_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Projects::TreeController do - let(:project) { create(:project_with_code) } + let(:project) { create(:project) } let(:user) { create(:user) } before do diff --git a/spec/factories.rb b/spec/factories.rb index daf84173648..bf74063a30d 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -27,43 +27,16 @@ FactoryGirl.define do factory :admin, traits: [:admin] end - factory :project do + factory :empty_project, class: 'Project' do sequence(:name) { |n| "project#{n}" } path { name.downcase.gsub(/\s/, '_') } namespace creator - - trait :source do - sequence(:name) { |n| "source project#{n}" } - end - trait :target do - sequence(:name) { |n| "target project#{n}" } - end - - factory :source_project, traits: [:source] - factory :target_project, traits: [:target] end - - factory :redmine_project, parent: :project do - issues_tracker { "redmine" } - issues_tracker_id { "project_name_in_redmine" } - end - - factory :project_with_code, parent: :project do + factory :project, parent: :empty_project do path { 'gitlabhq' } - trait :source_path do - path { 'source_gitlabhq' } - end - - trait :target_path do - path { 'target_gitlabhq' } - end - - factory :source_project_with_code, traits: [:source, :source_path] - factory :target_project_with_code, traits: [:target, :target_path] - after :create do |project| TestEnv.clear_repo_dir(project.namespace, project.path) TestEnv.reset_satellite_dir @@ -71,6 +44,11 @@ FactoryGirl.define do end end + factory :redmine_project, parent: :project do + issues_tracker { "redmine" } + issues_tracker_id { "project_name_in_redmine" } + end + factory :group do sequence(:name) { |n| "group#{n}" } path { name.downcase.gsub(/\s/, '_') } @@ -109,25 +87,12 @@ FactoryGirl.define do factory :merge_request do title author - source_project factory: :source_project_with_code - target_project factory: :target_project_with_code + source_project factory: :project + target_project { source_project } source_branch "master" target_branch "stable" - # pick 3 commits "at random" (from bcf03b5d~3 to bcf03b5d) trait :with_diffs do - target_branch "master" # pretend bcf03b5d~3 - source_branch "stable" # pretend bcf03b5d - st_commits do - [ - source_project.repository.commit('bcf03b5d').to_hash, - source_project.repository.commit('bcf03b5d~1').to_hash, - source_project.repository.commit('bcf03b5d~2').to_hash - ] - end - st_diffs do - source_project.repo.diff("bcf03b5d~3", "bcf03b5d") - end end trait :closed do @@ -156,7 +121,7 @@ FactoryGirl.define do factory :note_on_merge_request_with_attachment, traits: [:on_merge_request, :with_attachment] trait :on_commit do - project factory: :project_with_code + project factory: :project commit_id "bcf03b5de6c33f3869ef70d68cf06e679d1d7f9a" noteable_type "Commit" end @@ -166,7 +131,7 @@ FactoryGirl.define do end trait :on_merge_request do - project factory: :project_with_code + project factory: :project noteable_id 1 noteable_type "MergeRequest" end diff --git a/spec/features/gitlab_flavored_markdown_spec.rb b/spec/features/gitlab_flavored_markdown_spec.rb index 2ea569a6208..a507f0314c6 100644 --- a/spec/features/gitlab_flavored_markdown_spec.rb +++ b/spec/features/gitlab_flavored_markdown_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe "GitLab Flavored Markdown" do - let(:project) { create(:project_with_code) } + let(:project) { create(:project) } let(:issue) { create(:issue, project: project) } let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } let(:fred) do diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb index b534548a122..7b91853b13c 100644 --- a/spec/features/notes_on_merge_requests_spec.rb +++ b/spec/features/notes_on_merge_requests_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe "On a merge request", js: true do - let!(:project) { create(:project_with_code) } + let!(:project) { create(:project) } let!(:merge_request) { create(:merge_request, source_project: project, target_project: project) } let!(:note) { create(:note_on_merge_request_with_attachment, project: project) } @@ -135,7 +135,7 @@ describe "On a merge request", js: true do end describe "On a merge request diff", js: true, focus: true do - let!(:project) { create(:source_project_with_code) } + let!(:project) { create(:source_project) } let!(:merge_request) { create(:merge_request_with_diffs, source_project: project, target_project: project) } before do @@ -185,7 +185,7 @@ describe "On a merge request diff", js: true, focus: true do end describe "with muliple note forms" do - let!(:project) { create(:source_project_with_code) } + let!(:project) { create(:source_project) } let!(:merge_request) { create(:merge_request_with_diffs, source_project: project, target_project: project) } before do diff --git a/spec/features/security/project/internal_access_spec.rb b/spec/features/security/project/internal_access_spec.rb index 5abccd259d4..8bb1e259efa 100644 --- a/spec/features/security/project/internal_access_spec.rb +++ b/spec/features/security/project/internal_access_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe "Internal Project Access" do - let(:project) { create(:project_with_code) } + let(:project) { create(:project) } let(:master) { create(:user) } let(:guest) { create(:user) } diff --git a/spec/features/security/project/private_access_spec.rb b/spec/features/security/project/private_access_spec.rb index 481d8cec416..0402ff39735 100644 --- a/spec/features/security/project/private_access_spec.rb +++ b/spec/features/security/project/private_access_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe "Private Project Access" do - let(:project) { create(:project_with_code) } + let(:project) { create(:project) } let(:master) { create(:user) } let(:guest) { create(:user) } diff --git a/spec/features/security/project/public_access_spec.rb b/spec/features/security/project/public_access_spec.rb index 3f1016473f5..7e6a39fad69 100644 --- a/spec/features/security/project/public_access_spec.rb +++ b/spec/features/security/project/public_access_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe "Public Project Access" do - let(:project) { create(:project_with_code) } + let(:project) { create(:project) } let(:master) { create(:user) } let(:guest) { create(:user) } diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb index 33e69d4326c..088a6a09bc8 100644 --- a/spec/helpers/gitlab_markdown_helper_spec.rb +++ b/spec/helpers/gitlab_markdown_helper_spec.rb @@ -4,7 +4,7 @@ describe GitlabMarkdownHelper do include ApplicationHelper include IssuesHelper - let!(:project) { create(:project_with_code) } + let!(:project) { create(:project) } let(:user) { create(:user, username: 'gfm') } let(:commit) { project.repository.commit } diff --git a/spec/helpers/search_helper_spec.rb b/spec/helpers/search_helper_spec.rb index d04945dfe35..733f2754727 100644 --- a/spec/helpers/search_helper_spec.rb +++ b/spec/helpers/search_helper_spec.rb @@ -43,7 +43,7 @@ describe SearchHelper do end context "with a current project" do - before { @project = create(:project_with_code) } + before { @project = create(:project) } it "includes project-specific sections" do search_autocomplete_opts("Files").size.should == 1 diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb index 7d805f8c72a..19259a8b79c 100644 --- a/spec/lib/gitlab/reference_extractor_spec.rb +++ b/spec/lib/gitlab/reference_extractor_spec.rb @@ -43,7 +43,7 @@ describe Gitlab::ReferenceExtractor do end context 'with a project' do - let(:project) { create(:project_with_code) } + let(:project) { create(:project) } it 'accesses valid user objects on the project team' do @u_foo = create(:user, username: 'foo') diff --git a/spec/lib/gitlab/satellite/action_spec.rb b/spec/lib/gitlab/satellite/action_spec.rb index 5e0a825c3c3..d65e7c42b7e 100644 --- a/spec/lib/gitlab/satellite/action_spec.rb +++ b/spec/lib/gitlab/satellite/action_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe 'Gitlab::Satellite::Action' do - let(:project) { create(:project_with_code) } + let(:project) { create(:project) } let(:user) { create(:user) } describe '#prepare_satellite!' do diff --git a/spec/lib/gitlab/satellite/merge_action_spec.rb b/spec/lib/gitlab/satellite/merge_action_spec.rb index d2f026f96e8..1808935728d 100644 --- a/spec/lib/gitlab/satellite/merge_action_spec.rb +++ b/spec/lib/gitlab/satellite/merge_action_spec.rb @@ -12,7 +12,7 @@ describe 'Gitlab::Satellite::MergeAction' do @close_commit2 = ['scss_refactoring', 'f0f14c8eaba69ebddd766498a9d0b0e79becd633'] end - let(:project) { create(:project_with_code) } + let(:project) { create(:project) } let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } let(:merge_request_fork) { create(:merge_request) } diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index b1e53486816..d53dc17d977 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -5,7 +5,7 @@ describe Notify do include EmailSpec::Matchers let(:recipient) { create(:user, email: 'recipient@example.com') } - let(:project) { create(:project_with_code) } + let(:project) { create(:project) } shared_examples 'a multiple recipients email' do it 'is sent to the given recipient' do diff --git a/spec/models/assembla_service_spec.rb b/spec/models/assembla_service_spec.rb index 0b961c81ac1..ded6d87c33e 100644 --- a/spec/models/assembla_service_spec.rb +++ b/spec/models/assembla_service_spec.rb @@ -25,7 +25,7 @@ describe AssemblaService do describe "Execute" do let(:user) { create(:user) } - let(:project) { create(:project_with_code) } + let(:project) { create(:project) } before do @assembla_service = AssemblaService.new diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index fa556f94a1d..d8ab171d3ee 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Commit do - let(:project) { create :project_with_code } + let(:project) { create :project } let(:commit) { project.repository.commit } describe '#title' do diff --git a/spec/models/flowdock_service_spec.rb b/spec/models/flowdock_service_spec.rb index 636aba2f012..cd553b33ad7 100644 --- a/spec/models/flowdock_service_spec.rb +++ b/spec/models/flowdock_service_spec.rb @@ -25,7 +25,7 @@ describe FlowdockService do describe "Execute" do let(:user) { create(:user) } - let(:project) { create(:project_with_code) } + let(:project) { create(:project) } before do @flowdock_service = FlowdockService.new diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index b86603dd4ac..7a00ee83ba4 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -206,7 +206,7 @@ describe Note do end describe '#create_cross_reference_note' do - let(:project) { create(:project_with_code) } + let(:project) { create(:project) } let(:author) { create(:user) } let(:issue) { create(:issue, project: project) } let(:mergereq) { create(:merge_request, target_project: project) } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 373accfe412..9449782425f 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -128,7 +128,7 @@ describe Project do end describe :update_merge_requests do - let(:project) { create(:project_with_code) } + let(:project) { create(:project) } before do @merge_request = create(:merge_request, source_project: project, target_project: project) @@ -237,7 +237,7 @@ describe Project do end describe :open_branches do - let(:project) { create(:project_with_code) } + let(:project) { create(:project) } before do project.protected_branches.create(name: 'master') diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 667c80bcf19..b1b837c2263 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -49,7 +49,7 @@ describe Service do end describe "With commits" do - let (:project) { create :project_with_code } + let (:project) { create :project } before do @service.stub( diff --git a/spec/observers/merge_request_observer_spec.rb b/spec/observers/merge_request_observer_spec.rb index a450b4d518e..6ad7c4d81da 100644 --- a/spec/observers/merge_request_observer_spec.rb +++ b/spec/observers/merge_request_observer_spec.rb @@ -4,16 +4,17 @@ describe MergeRequestObserver do let(:some_user) { create :user } let(:assignee) { create :user } let(:author) { create :user } + let(:project) { create :project } let(:mr_mock) { double(:merge_request, id: 42, assignee: assignee, author: author).as_null_object } - let(:assigned_mr) { create(:merge_request, assignee: assignee, author: author, target_project: create(:project)) } - let(:unassigned_mr) { create(:merge_request, author: author, target_project: create(:project)) } - let(:closed_assigned_mr) { create(:closed_merge_request, assignee: assignee, author: author, target_project: create(:project)) } - let(:closed_unassigned_mr) { create(:closed_merge_request, author: author, target_project: create(:project)) } + let(:assigned_mr) { create(:merge_request, assignee: assignee, author: author, source_project: project) } + let(:unassigned_mr) { create(:merge_request, author: author, source_project: project) } + let(:closed_assigned_mr) { create(:closed_merge_request, assignee: assignee, author: author, source_project: project) } + let(:closed_unassigned_mr) { create(:closed_merge_request, author: author, source_project: project) } before { subject.stub(:current_user).and_return(some_user) } before { subject.stub(notification: double('NotificationService').as_null_object) } before { mr_mock.stub(:author_id) } - before { mr_mock.stub(:target_project) } + before { mr_mock.stub(:source_project) } before { mr_mock.stub(:source_project) } before { mr_mock.stub(:project) } before { mr_mock.stub(:create_cross_references!).and_return(true) } @@ -46,7 +47,7 @@ describe MergeRequestObserver do end it 'is called when a merge request is changed' do - changed = create(:merge_request, source_project: create(:project)) + changed = create(:merge_request, source_project: project) subject.should_receive(:after_update) MergeRequest.observers.enable :merge_request_observer do @@ -81,13 +82,13 @@ describe MergeRequestObserver do context '#after_close' do context 'a status "closed"' do it 'note is created if the merge request is being closed' do - Note.should_receive(:create_status_change_note).with(assigned_mr, assigned_mr.target_project, some_user, 'closed', nil) + Note.should_receive(:create_status_change_note).with(assigned_mr, assigned_mr.source_project, some_user, 'closed', nil) assigned_mr.close end it 'notification is delivered only to author if the merge request is being closed' do - Note.should_receive(:create_status_change_note).with(unassigned_mr, unassigned_mr.target_project, some_user, 'closed', nil) + Note.should_receive(:create_status_change_note).with(unassigned_mr, unassigned_mr.source_project, some_user, 'closed', nil) unassigned_mr.close end @@ -97,13 +98,13 @@ describe MergeRequestObserver do context '#after_reopen' do context 'a status "reopened"' do it 'note is created if the merge request is being reopened' do - Note.should_receive(:create_status_change_note).with(closed_assigned_mr, closed_assigned_mr.target_project, some_user, 'reopened', nil) + Note.should_receive(:create_status_change_note).with(closed_assigned_mr, closed_assigned_mr.source_project, some_user, 'reopened', nil) closed_assigned_mr.reopen end it 'notification is delivered only to author if the merge request is being reopened' do - Note.should_receive(:create_status_change_note).with(closed_unassigned_mr, closed_unassigned_mr.target_project, some_user, 'reopened', nil) + Note.should_receive(:create_status_change_note).with(closed_unassigned_mr, closed_unassigned_mr.source_project, some_user, 'reopened', nil) closed_unassigned_mr.reopen end @@ -118,20 +119,13 @@ describe MergeRequestObserver do it { @event.project.should == project } end - let(:project) { create(:project) } before do - TestEnv.enable_observers - @merge_request = create(:merge_request, source_project: project, target_project: project) + @merge_request = create(:merge_request, source_project: project, source_project: project) @event = Event.last end - after do - TestEnv.disable_observers - end - it_should_be_valid_event it { @event.action.should == Event::CREATED } it { @event.target.should == @merge_request } end - end diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index 2d1f8df47dd..acef7df8777 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -6,7 +6,7 @@ describe API::API do after(:each) { ActiveRecord::Base.observers.disable(:user_observer) } let(:user) { create(:user) } - let!(:project) { create(:project_with_code, namespace: user.namespace ) } + let!(:project) { create(:project, namespace: user.namespace ) } before { project.team << [user, :developer] } describe "POST /projects/:id/repository/files" do diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index f31b4da90cd..908f73be854 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -5,7 +5,7 @@ describe API::API do before(:each) { ActiveRecord::Base.observers.enable(:user_observer) } after(:each) { ActiveRecord::Base.observers.disable(:user_observer) } let(:user) { create(:user) } - let!(:project) {create(:project_with_code, creator_id: user.id, namespace: user.namespace) } + let!(:project) {create(:project, creator_id: user.id, namespace: user.namespace) } let!(:merge_request) { create(:merge_request, author: user, assignee: user, source_project: project, target_project: project, title: "Test") } before { project.team << [user, :reporters] @@ -80,8 +80,8 @@ describe API::API do context 'forked projects' do let!(:user2) {create(:user)} let!(:forked_project_link) { build(:forked_project_link) } - let!(:fork_project) { create(:source_project_with_code, forked_project_link: forked_project_link, namespace: user2.namespace, creator_id: user2.id) } - let!(:unrelated_project) { create(:target_project_with_code, namespace: user2.namespace, creator_id: user2.id) } + let!(:fork_project) { create(:source_project, forked_project_link: forked_project_link, namespace: user2.namespace, creator_id: user2.id) } + let!(:unrelated_project) { create(:target_project, namespace: user2.namespace, creator_id: user2.id) } before :each do |each| fork_project.team << [user2, :reporters] diff --git a/spec/requests/api/project_hooks_spec.rb b/spec/requests/api/project_hooks_spec.rb index beccd61866e..c8ace0b9462 100644 --- a/spec/requests/api/project_hooks_spec.rb +++ b/spec/requests/api/project_hooks_spec.rb @@ -7,7 +7,7 @@ describe API::API, 'ProjectHooks' do let(:user) { create(:user) } let(:user3) { create(:user) } - let!(:project) { create(:project_with_code, creator_id: user.id, namespace: user.namespace) } + let!(:project) { create(:project, creator_id: user.id, namespace: user.namespace) } let!(:hook) { create(:project_hook, project: project, url: "http://example.com") } before do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 8e0b9067672..342587ba5d6 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -9,14 +9,14 @@ describe API::API do let(:user2) { create(:user) } let(:user3) { create(:user) } let(:admin) { create(:admin) } - let!(:project) { create(:project_with_code, creator_id: user.id, namespace: user.namespace) } - let!(:snippet) { create(:project_snippet, author: user, project: project, title: 'example') } - let!(:users_project) { create(:users_project, user: user, project: project, project_access: UsersProject::MASTER) } - let!(:users_project2) { create(:users_project, user: user3, project: project, project_access: UsersProject::DEVELOPER) } - - before { project.team << [user, :reporter] } + let(:project) { create(:project, creator_id: user.id, namespace: user.namespace) } + let(:snippet) { create(:project_snippet, author: user, project: project, title: 'example') } + let(:users_project) { create(:users_project, user: user, project: project, project_access: UsersProject::MASTER) } + let(:users_project2) { create(:users_project, user: user3, project: project, project_access: UsersProject::DEVELOPER) } describe "GET /projects" do + before { project } + context "when unauthenticated" do it "should return authentication error" do get api("/projects") @@ -36,6 +36,8 @@ describe API::API do end describe "GET /projects/all" do + before { project } + context "when unauthenticated" do it "should return authentication error" do get api("/projects/all") @@ -174,6 +176,7 @@ describe API::API do end describe "POST /projects/user/:id" do + before { project } before { admin } it "should create new project without path" do @@ -255,6 +258,8 @@ describe API::API do end describe "GET /projects/:id" do + before { project } + it "should return a project by id" do get api("/projects/#{project.id}", user) response.status.should == 200 @@ -282,6 +287,8 @@ describe API::API do end describe "GET /projects/:id/events" do + before { users_project } + it "should return a project events" do get api("/projects/#{project.id}/events", user) response.status.should == 200 @@ -305,6 +312,9 @@ describe API::API do end describe "GET /projects/:id/members" do + before { users_project } + before { users_project2 } + it "should return project team members" do get api("/projects/#{project.id}/members", user) response.status.should == 200 @@ -328,6 +338,8 @@ describe API::API do end describe "GET /projects/:id/members/:user_id" do + before { users_project } + it "should return project team member" do get api("/projects/#{project.id}/members/#{user.id}", user) response.status.should == 200 @@ -383,6 +395,8 @@ describe API::API do end describe "PUT /projects/:id/members/:user_id" do + before { users_project2 } + it "should update project team member" do put api("/projects/#{project.id}/members/#{user3.id}", user), access_level: UsersProject::MASTER response.status.should == 200 @@ -407,6 +421,9 @@ describe API::API do end describe "DELETE /projects/:id/members/:user_id" do + before { users_project } + before { users_project2 } + it "should remove user from project team" do expect { delete api("/projects/#{project.id}/members/#{user3.id}", user) @@ -425,9 +442,7 @@ describe API::API do delete api("/projects/#{project.id}/members/#{user3.id}", user) response.status.should == 200 end - end - describe "DELETE /projects/:id/members/:user_id" do it "should return 200 OK when the user was not member" do expect { delete api("/projects/#{project.id}/members/1000000", user) @@ -439,6 +454,8 @@ describe API::API do end describe "GET /projects/:id/snippets" do + before { snippet } + it "should return an array of project snippets" do get api("/projects/#{project.id}/snippets", user) response.status.should == 200 @@ -505,6 +522,8 @@ describe API::API do end describe "DELETE /projects/:id/snippets/:snippet_id" do + before { snippet } + it "should delete existing project snippet" do expect { delete api("/projects/#{project.id}/snippets/#{snippet.id}", user) @@ -657,15 +676,15 @@ describe API::API do describe "GET /projects/search/:query" do let!(:query) { 'query'} - let!(:search) { create(:project, name: query, creator_id: user.id, namespace: user.namespace) } - let!(:pre) { create(:project, name: "pre_#{query}", creator_id: user.id, namespace: user.namespace) } - let!(:post) { create(:project, name: "#{query}_post", creator_id: user.id, namespace: user.namespace) } - let!(:pre_post) { create(:project, name: "pre_#{query}_post", creator_id: user.id, namespace: user.namespace) } - let!(:unfound) { create(:project, name: 'unfound', creator_id: user.id, namespace: user.namespace) } - let!(:internal) { create(:project, name: "internal #{query}", visibility_level: Gitlab::VisibilityLevel::INTERNAL) } - let!(:unfound_internal) { create(:project, name: 'unfound internal', visibility_level: Gitlab::VisibilityLevel::INTERNAL) } - let!(:public) { create(:project, name: "public #{query}", visibility_level: Gitlab::VisibilityLevel::PUBLIC) } - let!(:unfound_public) { create(:project, name: 'unfound public', visibility_level: Gitlab::VisibilityLevel::PUBLIC) } + let!(:search) { create(:empty_project, name: query, creator_id: user.id, namespace: user.namespace) } + let!(:pre) { create(:empty_project, name: "pre_#{query}", creator_id: user.id, namespace: user.namespace) } + let!(:post) { create(:empty_project, name: "#{query}_post", creator_id: user.id, namespace: user.namespace) } + let!(:pre_post) { create(:empty_project, name: "pre_#{query}_post", creator_id: user.id, namespace: user.namespace) } + let!(:unfound) { create(:empty_project, name: 'unfound', creator_id: user.id, namespace: user.namespace) } + let!(:internal) { create(:empty_project, name: "internal #{query}", visibility_level: Gitlab::VisibilityLevel::INTERNAL) } + let!(:unfound_internal) { create(:empty_project, name: 'unfound internal', visibility_level: Gitlab::VisibilityLevel::INTERNAL) } + let!(:public) { create(:empty_project, name: "public #{query}", visibility_level: Gitlab::VisibilityLevel::PUBLIC) } + let!(:unfound_public) { create(:empty_project, name: 'unfound public', visibility_level: Gitlab::VisibilityLevel::PUBLIC) } context "when unauthenticated" do it "should return authentication error" do diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index f73ac4372b2..47008728252 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -8,7 +8,7 @@ describe API::API do let(:user) { create(:user) } let(:user2) { create(:user) } - let!(:project) { create(:project_with_code, creator_id: user.id) } + let!(:project) { create(:project, creator_id: user.id) } let!(:master) { create(:users_project, user: user, project: project, project_access: UsersProject::MASTER) } let!(:guest) { create(:users_project, user: user2, project: project, project_access: UsersProject::GUEST) } diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb index e2fd945bad3..aecd18bc14a 100644 --- a/spec/requests/api/services_spec.rb +++ b/spec/requests/api/services_spec.rb @@ -6,7 +6,7 @@ describe API::API do after(:each) { ActiveRecord::Base.observers.disable(:user_observer) } let(:user) { create(:user) } - let(:project) {create(:project_with_code, creator_id: user.id, namespace: user.namespace) } + let(:project) {create(:project, creator_id: user.id, namespace: user.namespace) } describe "POST /projects/:id/services/gitlab-ci" do it "should update gitlab-ci settings" do diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index b46022fb2da..90738c681fa 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe GitPushService do let (:user) { create :user } - let (:project) { create :project_with_code } + let (:project) { create :project } let (:service) { GitPushService.new } before do diff --git a/spec/services/test_hook_service_spec.rb b/spec/services/test_hook_service_spec.rb index fbe9066096d..76af5bf7b88 100644 --- a/spec/services/test_hook_service_spec.rb +++ b/spec/services/test_hook_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe TestHookService do let (:user) { create :user } - let (:project) { create :project_with_code } + let (:project) { create :project } let (:hook) { create :project_hook, project: project } describe :execute do diff --git a/spec/support/mentionable_shared_examples.rb b/spec/support/mentionable_shared_examples.rb index 948fff27b89..3802e94ecf0 100644 --- a/spec/support/mentionable_shared_examples.rb +++ b/spec/support/mentionable_shared_examples.rb @@ -11,7 +11,7 @@ def common_mentionable_setup let(:mentioned_issue) { create :issue, project: mproject } let(:other_issue) { create :issue, project: mproject } - let(:mentioned_mr) { create :merge_request, target_project: mproject, source_branch: 'different' } + let(:mentioned_mr) { create :merge_request, source_project: mproject, source_branch: 'different' } let(:mentioned_commit) { double('commit', sha: '1234567890abcdef').as_null_object } # Override to add known commits to the repository stub. diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index 46e86dbe00a..a4751bd0baf 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -9,7 +9,7 @@ describe PostReceive do end context "web hook" do - let(:project) { create(:project_with_code) } + let(:project) { create(:project) } let(:key) { create(:key, user: project.owner) } let(:key_id) { key.shell_id } From cee68dec8a9b9531690da0f81fbe1ebdc31a7eea Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 22 Jan 2014 21:59:09 +0200 Subject: [PATCH 10/18] Fix api merge request specs Signed-off-by: Dmitriy Zaporozhets --- spec/requests/api/merge_requests_spec.rb | 33 ++++++++++++------------ 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 908f73be854..412b6c95ffa 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -47,32 +47,32 @@ describe API::API do context 'between branches projects' do it "should return merge_request" do post api("/projects/#{project.id}/merge_requests", user), - title: 'Test merge_request', source_branch: "stable", target_branch: "master", author: user + title: 'Test merge_request', source_branch: "stable", target_branch: "master", author: user response.status.should == 201 json_response['title'].should == 'Test merge_request' end it "should return 422 when source_branch equals target_branch" do post api("/projects/#{project.id}/merge_requests", user), - title: "Test merge_request", source_branch: "master", target_branch: "master", author: user + title: "Test merge_request", source_branch: "master", target_branch: "master", author: user response.status.should == 422 end it "should return 400 when source_branch is missing" do post api("/projects/#{project.id}/merge_requests", user), - title: "Test merge_request", target_branch: "master", author: user + title: "Test merge_request", target_branch: "master", author: user response.status.should == 400 end it "should return 400 when target_branch is missing" do post api("/projects/#{project.id}/merge_requests", user), - title: "Test merge_request", source_branch: "stable", author: user + title: "Test merge_request", source_branch: "stable", author: user response.status.should == 400 end it "should return 400 when title is missing" do post api("/projects/#{project.id}/merge_requests", user), - target_branch: 'master', source_branch: 'stable' + target_branch: 'master', source_branch: 'stable' response.status.should == 400 end end @@ -80,8 +80,8 @@ describe API::API do context 'forked projects' do let!(:user2) {create(:user)} let!(:forked_project_link) { build(:forked_project_link) } - let!(:fork_project) { create(:source_project, forked_project_link: forked_project_link, namespace: user2.namespace, creator_id: user2.id) } - let!(:unrelated_project) { create(:target_project, namespace: user2.namespace, creator_id: user2.id) } + let!(:fork_project) { create(:project, forked_project_link: forked_project_link, namespace: user2.namespace, creator_id: user2.id) } + let!(:unrelated_project) { create(:project, namespace: create(:user).namespace, creator_id: user2.id) } before :each do |each| fork_project.team << [user2, :reporters] @@ -92,7 +92,7 @@ describe API::API do it "should return merge_request" do post api("/projects/#{fork_project.id}/merge_requests", user2), - title: 'Test merge_request', source_branch: "stable", target_branch: "master", author: user2, target_project_id: project.id + title: 'Test merge_request', source_branch: "stable", target_branch: "master", author: user2, target_project_id: project.id response.status.should == 201 json_response['title'].should == 'Test merge_request' end @@ -102,44 +102,44 @@ describe API::API do fork_project.forked?.should be_true fork_project.forked_from_project.should == project post api("/projects/#{fork_project.id}/merge_requests", user2), - title: 'Test merge_request', source_branch: "master", target_branch: "master", author: user2, target_project_id: project.id + title: 'Test merge_request', source_branch: "master", target_branch: "master", author: user2, target_project_id: project.id response.status.should == 201 json_response['title'].should == 'Test merge_request' end it "should return 400 when source_branch is missing" do post api("/projects/#{fork_project.id}/merge_requests", user2), - title: 'Test merge_request', target_branch: "master", author: user2, target_project_id: project.id + title: 'Test merge_request', target_branch: "master", author: user2, target_project_id: project.id response.status.should == 400 end it "should return 400 when target_branch is missing" do post api("/projects/#{fork_project.id}/merge_requests", user2), - title: 'Test merge_request', target_branch: "master", author: user2, target_project_id: project.id + title: 'Test merge_request', target_branch: "master", author: user2, target_project_id: project.id response.status.should == 400 end it "should return 400 when title is missing" do post api("/projects/#{fork_project.id}/merge_requests", user2), - target_branch: 'master', source_branch: 'stable', author: user2, target_project_id: project.id + target_branch: 'master', source_branch: 'stable', author: user2, target_project_id: project.id response.status.should == 400 end it "should return 400 when target_branch is specified and not a forked project" do post api("/projects/#{project.id}/merge_requests", user), - title: 'Test merge_request', target_branch: 'master', source_branch: 'stable', author: user, target_project_id: fork_project.id + title: 'Test merge_request', target_branch: 'master', source_branch: 'stable', author: user, target_project_id: fork_project.id response.status.should == 400 end it "should return 400 when target_branch is specified and for a different fork" do post api("/projects/#{fork_project.id}/merge_requests", user2), - title: 'Test merge_request', target_branch: 'master', source_branch: 'stable', author: user2, target_project_id: unrelated_project.id + title: 'Test merge_request', target_branch: 'master', source_branch: 'stable', author: user2, target_project_id: unrelated_project.id response.status.should == 400 end it "should return 201 when target_branch is specified and for the same project" do post api("/projects/#{fork_project.id}/merge_requests", user2), - title: 'Test merge_request', target_branch: 'master', source_branch: 'stable', author: user2, target_project_id: fork_project.id + title: 'Test merge_request', target_branch: 'master', source_branch: 'stable', author: user2, target_project_id: fork_project.id response.status.should == 201 end end @@ -170,7 +170,7 @@ describe API::API do it "should return 422 when source_branch and target_branch are renamed the same" do put api("/projects/#{project.id}/merge_request/#{merge_request.id}", user), - source_branch: "master", target_branch: "master" + source_branch: "master", target_branch: "master" response.status.should == 422 end @@ -198,5 +198,4 @@ describe API::API do response.status.should == 404 end end - end From 380342784d3efbecb98de81c9b2679e3d720f7c9 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 23 Jan 2014 12:55:43 +0200 Subject: [PATCH 11/18] Fix MR spinach tests Signed-off-by: Dmitriy Zaporozhets --- app/models/merge_request.rb | 2 +- features/project/merge_requests.feature | 8 +-- features/steps/project/deploy_keys.rb | 2 +- .../project/project_forked_merge_requests.rb | 2 +- .../steps/project/project_merge_requests.rb | 51 +++++++++++-------- spec/factories.rb | 33 ++++++++++++ spec/support/test_env.rb | 4 ++ 7 files changed, 74 insertions(+), 28 deletions(-) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index c53b24b817d..712d01626b5 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -142,7 +142,7 @@ class MergeRequest < ActiveRecord::Base end def automerge!(current_user, commit_message = nil) - if Gitlab::Satellite::MergeAction.new(current_user, self).merge!(commit_message) && self.unmerged_commits.empty? + if Gitlab::Satellite::MergeAction.new(current_user, self).merge!(commit_message) self.merge!(current_user.id) true end diff --git a/features/project/merge_requests.feature b/features/project/merge_requests.feature index 4e4ac1a68e4..946f6760126 100644 --- a/features/project/merge_requests.feature +++ b/features/project/merge_requests.feature @@ -55,18 +55,18 @@ Feature: Project Merge Requests Given project "Shop" have "Bug NS-05" open merge request with diffs inside And I visit merge request page "Bug NS-05" And I click on the first commit in the merge request - And I leave a comment like "Line is wrong" on line 185 of the first file + And I leave a comment like "Line is wrong" on line 185 of the first file in commit And I switch to the merge request's comments tab - Then I should see a discussion has started on commit bcf03b5de6c:L185 + Then I should see a discussion has started on commit b1e6a9dbf1:L185 @javascript Scenario: I comment on a commit in merge request Given project "Shop" have "Bug NS-05" open merge request with diffs inside And I visit merge request page "Bug NS-05" And I click on the first commit in the merge request - And I leave a comment on the diff page + And I leave a comment on the diff page in commit And I switch to the merge request's comments tab - Then I should see a discussion has started on commit bcf03b5de6c + Then I should see a discussion has started on commit b1e6a9dbf1 @javascript Scenario: I accept merge request with custom commit message diff --git a/features/steps/project/deploy_keys.rb b/features/steps/project/deploy_keys.rb index 7f7492bfd6d..914da31322f 100644 --- a/features/steps/project/deploy_keys.rb +++ b/features/steps/project/deploy_keys.rb @@ -34,7 +34,7 @@ class Spinach::Features::ProjectDeployKeys < Spinach::FeatureSteps end step 'other project has deploy key' do - @second_project = create :project, namespace: current_user.namespace + @second_project = create :project, namespace: create(:group) @second_project.team << [current_user, :master] create(:deploy_keys_project, project: @second_project) end diff --git a/features/steps/project/project_forked_merge_requests.rb b/features/steps/project/project_forked_merge_requests.rb index 57ab6439564..48e76277487 100644 --- a/features/steps/project/project_forked_merge_requests.rb +++ b/features/steps/project/project_forked_merge_requests.rb @@ -15,7 +15,7 @@ class ProjectForkedMergeRequests < Spinach::FeatureSteps @forking_user = @user forked_project_link = build(:forked_project_link) @forked_project = Project.find_by_name "Forked Shop" - @forked_project ||= create(:source_project, name: "Forked Shop", forked_project_link: forked_project_link, creator_id: @forking_user.id , namespace: @forking_user.namespace) + @forked_project ||= create(:project, name: "Forked Shop", forked_project_link: forked_project_link, creator_id: @forking_user.id , namespace: @forking_user.namespace) forked_project_link.forked_from_project = @project forked_project_link.forked_to_project = @forked_project diff --git a/features/steps/project/project_merge_requests.rb b/features/steps/project/project_merge_requests.rb index 8ef952cf3d1..e848a0b455b 100644 --- a/features/steps/project/project_merge_requests.rb +++ b/features/steps/project/project_merge_requests.rb @@ -81,6 +81,8 @@ class ProjectMergeRequests < Spinach::FeatureSteps title: "Bug NS-04", source_project: project, target_project: project, + source_branch: 'stable', + target_branch: 'master', author: project.users.first) end @@ -109,33 +111,29 @@ class ProjectMergeRequests < Spinach::FeatureSteps end step 'I click on the first commit in the merge request' do - click_link merge_request.commits.first.short_id(8) + within '.first-commits' do + click_link merge_request.commits.first.short_id(8) + end end step 'I leave a comment on the diff page' do init_diff_note + leave_comment "One comment to rule them all" + end - within('.js-discussion-note-form') do - fill_in "note_note", with: "One comment to rule them all" - click_button "Add Comment" - end - - within ".note-text" do - page.should have_content "One comment to rule them all" - end + step 'I leave a comment on the diff page in commit' do + find('a[data-line-code="4735dfc552ad7bf15ca468adc3cad9d05b624490_185_185"]').click + leave_comment "One comment to rule them all" end step 'I leave a comment like "Line is wrong" on line 185 of the first file' do init_diff_note + leave_comment "Line is wrong" + end - within(".js-discussion-note-form") do - fill_in "note_note", with: "Line is wrong" - click_button "Add Comment" - end - - within ".note-text" do - page.should have_content "Line is wrong" - end + step 'I leave a comment like "Line is wrong" on line 185 of the first file in commit' do + find('a[data-line-code="4735dfc552ad7bf15ca468adc3cad9d05b624490_185_185"]').click + leave_comment "Line is wrong" end step 'I should see a discussion has started on line 185' do @@ -144,14 +142,14 @@ class ProjectMergeRequests < Spinach::FeatureSteps page.should have_content "Line is wrong" end - step 'I should see a discussion has started on commit bcf03b5de6c:L185' do + step 'I should see a discussion has started on commit b1e6a9dbf1:L185' do page.should have_content "#{current_user.name} started a discussion on commit" page.should have_content "app/assets/stylesheets/tree.scss:L185" page.should have_content "Line is wrong" end - step 'I should see a discussion has started on commit bcf03b5de6c' do - page.should have_content "#{current_user.name} started a discussion on commit bcf03b5de6c" + step 'I should see a discussion has started on commit b1e6a9dbf1' do + page.should have_content "#{current_user.name} started a discussion on commit" page.should have_content "One comment to rule them all" page.should have_content "app/assets/stylesheets/tree.scss:L185" end @@ -188,6 +186,17 @@ class ProjectMergeRequests < Spinach::FeatureSteps end def init_diff_note - find('a[data-line-code="4735dfc552ad7bf15ca468adc3cad9d05b624490_185_185"]').click + find('a[data-line-code="4735dfc552ad7bf15ca468adc3cad9d05b624490_172_185"]').click + end + + def leave_comment(message) + within(".js-discussion-note-form") do + fill_in "note_note", with: message + click_button "Add Comment" + end + + within ".note-text" do + page.should have_content message + end end end diff --git a/spec/factories.rb b/spec/factories.rb index bf74063a30d..8c12c9b3e19 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -89,6 +89,39 @@ FactoryGirl.define do author source_project factory: :project target_project { source_project } + + # → git log stable..master --pretty=oneline + # b1e6a9dbf1c85e6616497a5e7bad9143a4bd0828 tree css fixes + # 8716fc78f3c65bbf7bcf7b574febd583bc5d2812 Added loading animation for notes + # cd5c4bac5042c5469dcdf7e7b2f768d3c6fd7088 notes count for wall + # 8470d70da67355c9c009e4401746b1d5410af2e3 notes controller refactored + # 1e689bfba39525ead225eaf611948cfbe8ac34cf fixed notes logic + # f0f14c8eaba69ebddd766498a9d0b0e79becd633 finished scss refactoring + # 3a4b4fb4cde7809f033822a171b9feae19d41fff Moving ui styles to one scss file, Added ui class to body + # 065c200c33f68c2bb781e35a43f9dc8138a893b5 removed unnecessary hr tags & titles + # 1e8b111be85df0db6c8000ef9a710bc0221eae83 Merge branch 'master' of github.com:gitlabhq/gitlabhq + # f403da73f5e62794a0447aca879360494b08f678 Fixed ajax loading image. Fixed wrong wording + # e6ea73c77600d413d370249b8e392734f7d1dbee Merge pull request #468 from bencevans/patch-1 + # 4a3c05b69355deee25767a74d0512ec4b510d4ef Merge pull request #470 from bgondy/patch-1 + # 0347fe2412eb51d3efeccc35210e9268bc765ac5 Update app/views/projects/team.html.haml + # 2b5c61bdece1f7eb2b901ceea7d364065cdf76ac Title for a link fixed + # 460eeb13b7560b40104044973ff933b1a6dbbcaa Increased count of notes loaded when visit wall page + # 21c141afb1c53a9180a99d2cca29ffa613eb7e3a Merge branch 'notes_refactoring' + # 292a41cbe295f16f7148913b31eb0fb91f3251c3 Fixed comments for snippets. Tests fixed + # d41d8ffb02fa74fd4571603548bd7e401ec99e0c Reply button, Comments for Merge Request diff + # b1a36b552be2a7a6bc57fbed6c52dc6ed82111f8 Merge pull request #466 from skroutz/no-rbenv + # db75dae913e8365453ca231f101b067314a7ea71 Merge pull request #465 from skroutz/branches_commit_link + # 75f040fbfe4b5af23ff004ad3207c3976df097a8 Don't enforce rbenv version + # e42fb4fda475370dcb0d8f8f1268bfdc7a0cc437 Fix broken commit link in branches page + # 215a01f63ccdc085f75a48f6f7ab6f2b15b5852c move notes login to one controller + # 81092c01984a481e312de10a28e3f1a6dda182a3 Status codes for errors, New error pages + # 7d279f9302151e3c8f4c5df9c5200a72799409b9 better error handling for not found resource, gitolite error + # 9e6d0710e927aa8ea834b8a9ae9f277be617ac7d Merge pull request #443 from CedricGatay/fix/incorrectLineNumberingInDiff + # 6ea87c47f0f8a24ae031c3fff17bc913889ecd00 Incorrect line numbering in diff + # + # → git log master..stable --pretty=oneline + # empty + source_branch "master" target_branch "stable" diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index e2bc2a5d7dd..43aec1cd43d 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -73,6 +73,10 @@ module TestEnv version: '6.3.0' ) + Gitlab::Satellite::MergeAction.any_instance.stub( + merge!: true, + ) + Gitlab::Satellite::Satellite.any_instance.stub( exists?: true, destroy: true, From c3270875edd4d381b11401545b100e7b0408d6ef Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 23 Jan 2014 13:02:43 +0200 Subject: [PATCH 12/18] Fix project team tests Signed-off-by: Dmitriy Zaporozhets --- 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 efebba1be24..9071b8cc6d4 100644 --- a/features/steps/project/project_team_management.rb +++ b/features/steps/project/project_team_management.rb @@ -79,7 +79,7 @@ class ProjectTeamManagement < Spinach::FeatureSteps end Given 'I own project "Website"' do - @project = create(:project, name: "Website", namespace: @user.namespace) + @project = create(:empty_project, name: "Website", namespace: @user.namespace) @project.team << [@user, :master] end From 418660b9713fbb6bad156dc15fafcc80ecd3e7c0 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 23 Jan 2014 13:05:34 +0200 Subject: [PATCH 13/18] Fix public tests Signed-off-by: Dmitriy Zaporozhets --- features/steps/public/projects_feature.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/steps/public/projects_feature.rb b/features/steps/public/projects_feature.rb index 2292fdad6fb..6872872271d 100644 --- a/features/steps/public/projects_feature.rb +++ b/features/steps/public/projects_feature.rb @@ -29,7 +29,7 @@ class Spinach::Features::PublicProjectsFeature < Spinach::FeatureSteps end step 'public empty project "Empty Public Project"' do - create :project, name: 'Empty Public Project', visibility_level: Gitlab::VisibilityLevel::PUBLIC + create :empty_project, name: 'Empty Public Project', visibility_level: Gitlab::VisibilityLevel::PUBLIC end step 'I visit empty project page' do From 9db8cb3fecca819c2d48fbff9ad69d969a336801 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 23 Jan 2014 13:17:31 +0200 Subject: [PATCH 14/18] fix tests notes_on_merge_requests Signed-off-by: Dmitriy Zaporozhets --- spec/features/notes_on_merge_requests_spec.rb | 25 ++++++------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb index 7b91853b13c..da723ae39bd 100644 --- a/spec/features/notes_on_merge_requests_spec.rb +++ b/spec/features/notes_on_merge_requests_spec.rb @@ -135,7 +135,7 @@ describe "On a merge request", js: true do end describe "On a merge request diff", js: true, focus: true do - let!(:project) { create(:source_project) } + let!(:project) { create(:project) } let!(:merge_request) { create(:merge_request_with_diffs, source_project: project, target_project: project) } before do @@ -149,7 +149,7 @@ describe "On a merge request diff", js: true, focus: true do describe "when adding a note" do before do - find('a[data-line-code="4735dfc552ad7bf15ca468adc3cad9d05b624490_185_185"]').click + find('a[data-line-code="4735dfc552ad7bf15ca468adc3cad9d05b624490_172_185"]').click end describe "the notes holder" do @@ -159,23 +159,14 @@ describe "On a merge request diff", js: true, focus: true do end describe "the note form" do - # FIXME - #it 'should be valid' do - #within(".js-temp-notes-holder") { find("#note_noteable_type").value.should == "MergeRequest" } - #within(".js-temp-notes-holder") { find("#note_noteable_id").value.should == merge_request.id.to_s } - #within(".js-temp-notes-holder") { find("#note_commit_id").value.should == "" } - #within(".js-temp-notes-holder") { find("#note_line_code").value.should == "4735dfc552ad7bf15ca468adc3cad9d05b624490_185_185" } - #should have_css(".js-close-discussion-note-form", text: "Cancel") - #end - it "shouldn't add a second form for same row" do - find('a[data-line-code="4735dfc552ad7bf15ca468adc3cad9d05b624490_185_185"]').click + find('a[data-line-code="4735dfc552ad7bf15ca468adc3cad9d05b624490_172_185"]').click - should have_css("tr[id='4735dfc552ad7bf15ca468adc3cad9d05b624490_185_185'] + .js-temp-notes-holder form", count: 1) + should have_css("tr[id='4735dfc552ad7bf15ca468adc3cad9d05b624490_172_185'] + .js-temp-notes-holder form", count: 1) end it "should be removed when canceled" do - within(".file form[rel$='4735dfc552ad7bf15ca468adc3cad9d05b624490_185_185']") do + within(".file form[rel$='4735dfc552ad7bf15ca468adc3cad9d05b624490_172_185']") do find(".js-close-discussion-note-form").trigger("click") end @@ -185,11 +176,11 @@ describe "On a merge request diff", js: true, focus: true do end describe "with muliple note forms" do - let!(:project) { create(:source_project) } + let!(:project) { create(:project) } let!(:merge_request) { create(:merge_request_with_diffs, source_project: project, target_project: project) } before do - find('a[data-line-code="4735dfc552ad7bf15ca468adc3cad9d05b624490_185_185"]').click + find('a[data-line-code="4735dfc552ad7bf15ca468adc3cad9d05b624490_172_185"]').click find('a[data-line-code="342e16cbbd482ac2047dc679b2749d248cc1428f_18_17"]').click end @@ -198,7 +189,7 @@ describe "On a merge request diff", js: true, focus: true do describe "previewing them separately" do before do # add two separate texts and trigger previews on both - within("tr[id='4735dfc552ad7bf15ca468adc3cad9d05b624490_185_185'] + .js-temp-notes-holder") do + within("tr[id='4735dfc552ad7bf15ca468adc3cad9d05b624490_172_185'] + .js-temp-notes-holder") do fill_in "note[note]", with: "One comment on line 185" find(".js-note-preview-button").trigger("click") end From 4cde3593b91b2aff4681bc8a3cece8be3d194b42 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 23 Jan 2014 13:21:58 +0200 Subject: [PATCH 15/18] Fix satellites specs Signed-off-by: Dmitriy Zaporozhets --- spec/lib/gitlab/satellite/merge_action_spec.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/spec/lib/gitlab/satellite/merge_action_spec.rb b/spec/lib/gitlab/satellite/merge_action_spec.rb index 1808935728d..25ddf87dc82 100644 --- a/spec/lib/gitlab/satellite/merge_action_spec.rb +++ b/spec/lib/gitlab/satellite/merge_action_spec.rb @@ -12,9 +12,10 @@ describe 'Gitlab::Satellite::MergeAction' do @close_commit2 = ['scss_refactoring', 'f0f14c8eaba69ebddd766498a9d0b0e79becd633'] end - let(:project) { create(:project) } + let(:project) { create(:project, namespace: create(:group)) } + let(:fork_project) { create(:project, namespace: create(:group)) } let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } - let(:merge_request_fork) { create(:merge_request) } + let(:merge_request_fork) { create(:merge_request, source_project: fork_project, target_project: project) } describe '#commits_between' do def verify_commits(commits, first_commit_sha, last_commit_sha) From e013cd44e0da544cd23e0cdb1eed44b2f8cdf494 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 23 Jan 2014 13:24:25 +0200 Subject: [PATCH 16/18] Fix mr specs Signed-off-by: Dmitriy Zaporozhets --- spec/models/merge_request_spec.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 039a0c087ba..f1ad679b658 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -73,14 +73,13 @@ describe MergeRequest do describe '#for_fork?' do it 'returns true if the merge request is for a fork' do - subject.source_project = create(:source_project) - subject.target_project = create(:target_project) + subject.source_project = create(:project, namespace: create(:group)) + subject.target_project = create(:project, namespace: create(:group)) subject.for_fork?.should be_true end + it 'returns false if is not for a fork' do - subject.source_project = create(:source_project) - subject.target_project = subject.source_project subject.for_fork?.should be_false end end From 71851a7e4e6784105b727ae7fde8fae72343ef3c Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 23 Jan 2014 13:31:16 +0200 Subject: [PATCH 17/18] Fix tests Signed-off-by: Dmitriy Zaporozhets --- spec/models/project_spec.rb | 7 +++---- spec/models/service_spec.rb | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 9449782425f..77467fb3890 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -136,7 +136,7 @@ describe Project do end it "should close merge request if last commit from source branch was pushed to target branch" do - @merge_request.reloaded_commits + @merge_request.reload_code @merge_request.last_commit.id.should == "b1e6a9dbf1c85e6616497a5e7bad9143a4bd0828" project.update_merge_requests("8716fc78f3c65bbf7bcf7b574febd583bc5d2812", "b1e6a9dbf1c85e6616497a5e7bad9143a4bd0828", "refs/heads/stable", @key.user) @merge_request.reload @@ -144,7 +144,6 @@ describe Project do end it "should update merge request commits with new one if pushed to source branch" do - @merge_request.last_commit.should == nil project.update_merge_requests("8716fc78f3c65bbf7bcf7b574febd583bc5d2812", "b1e6a9dbf1c85e6616497a5e7bad9143a4bd0828", "refs/heads/master", @key.user) @merge_request.reload @merge_request.last_commit.id.should == "b1e6a9dbf1c85e6616497a5e7bad9143a4bd0828" @@ -156,10 +155,10 @@ describe Project do context 'with namespace' do before do @group = create :group, name: 'gitlab' - @project = create(:project, name: 'gitlab-ci', namespace: @group) + @project = create(:project, name: 'gitlabhq', namespace: @group) end - it { Project.find_with_namespace('gitlab/gitlab-ci').should == @project } + it { Project.find_with_namespace('gitlab/gitlabhq').should == @project } it { Project.find_with_namespace('gitlab-ci').should be_nil } end end diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index b1b837c2263..46b3bf39aeb 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -44,7 +44,7 @@ describe Service do end describe :can_test do - it { @testable.should == false } + it { @testable.should == true } end end From 573f1f4ade80cc3b66723ad12594467e881b4510 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 23 Jan 2014 14:23:21 +0200 Subject: [PATCH 18/18] Fix project model spec Signed-off-by: Dmitriy Zaporozhets --- spec/models/project_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 77467fb3890..8fe669487f5 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -167,10 +167,10 @@ describe Project do context 'with namespace' do before do @group = create :group, name: 'gitlab' - @project = create(:project, name: 'gitlab-ci', namespace: @group) + @project = create(:project, name: 'gitlabhq', namespace: @group) end - it { @project.to_param.should == "gitlab/gitlab-ci" } + it { @project.to_param.should == "gitlab/gitlabhq" } end end