diff options
author | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2013-07-30 14:35:33 +0400 |
---|---|---|
committer | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2013-07-30 14:35:33 +0400 |
commit | 0d715bcd812ca6c99884e117f28a400669aa8e57 (patch) | |
tree | 5690fa43c790af40c51441425b9d8be068d4139d | |
parent | 4f07a6a99cab8f8ae3ad0a786a6cc9a837955c08 (diff) | |
parent | 4d373005968b8269a8d2fe56b7776820396127a4 (diff) |
Merge branch 'mr-on-fork' of https://github.com/karlhungus/gitlabhq into karlhungus-mr-on-fork
Conflicts:
app/views/projects/commit/show.html.haml
app/views/projects/compare/show.html.haml
app/views/projects/merge_requests/branch_from.js.haml
102 files changed, 1646 insertions, 456 deletions
diff --git a/app/assets/stylesheets/common.scss b/app/assets/stylesheets/common.scss index abbd6871008..2b3382dd8bb 100644 --- a/app/assets/stylesheets/common.scss +++ b/app/assets/stylesheets/common.scss @@ -400,6 +400,18 @@ img.emoji { margin-bottom: 10px; } +.label-project { + @include border-radius(4px); + padding: 2px 4px; + border: none; + font-size: 14px; + background: #474D57; + color: #fff; + font-family: $monospace_font; + text-shadow: 0 1px 1px #111; + font-weight: normal; +} + .group-name { font-size: 14px; line-height: 24px; diff --git a/app/contexts/filter_context.rb b/app/contexts/filter_context.rb index 401d19b31c8..e7f80e1a4e4 100644 --- a/app/contexts/filter_context.rb +++ b/app/contexts/filter_context.rb @@ -12,7 +12,7 @@ class FilterContext def apply_filter items if params[:project_id] - items = items.where(project_id: params[:project_id]) + items = items.by_project(params[:project_id]) end if params[:search].present? diff --git a/app/contexts/merge_requests_load_context.rb b/app/contexts/merge_requests_load_context.rb index fd44572c0eb..9eba81295a9 100644 --- a/app/contexts/merge_requests_load_context.rb +++ b/app/contexts/merge_requests_load_context.rb @@ -14,7 +14,7 @@ class MergeRequestsLoadContext < BaseContext end merge_requests = merge_requests.page(params[:page]).per(20) - merge_requests = merge_requests.includes(:author, :project).order("created_at desc") + merge_requests = merge_requests.includes(:author, :source_project, :target_project).order("created_at desc") # Filter by specific assignee_id (or lack thereof)? if params[:assignee_id].present? diff --git a/app/contexts/search_context.rb b/app/contexts/search_context.rb index 22cda709f69..4b1be84a2e1 100644 --- a/app/contexts/search_context.rb +++ b/app/contexts/search_context.rb @@ -19,7 +19,7 @@ class SearchContext if params[:search_code].present? result[:blobs] = project.repository.search_files(query, params[:repository_ref]) unless project.empty_repo? else - result[:merge_requests] = MergeRequest.where(project_id: project_ids).search(query).limit(10) + result[:merge_requests] = MergeRequest.in_projects(project_ids).search(query).limit(10) result[:issues] = Issue.where(project_id: project_ids).search(query).limit(10) result[:wiki_pages] = [] end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 33c1a1feff7..dd1f8cee873 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -24,8 +24,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController format.html format.js - format.diff { render text: @merge_request.to_diff } - format.patch { render text: @merge_request.to_patch } + format.diff { render text: @merge_request.to_diff(current_user) } + format.patch { render text: @merge_request.to_patch(current_user) } end end @@ -33,27 +33,37 @@ class Projects::MergeRequestsController < Projects::ApplicationController @commit = @merge_request.last_commit @comments_allowed = @reply_allowed = true - @comments_target = { noteable_type: 'MergeRequest', - noteable_id: @merge_request.id } + @comments_target = {noteable_type: 'MergeRequest', + noteable_id: @merge_request.id} @line_notes = @merge_request.notes.where("line_code is not null") end def new - @merge_request = @project.merge_requests.new(params[:merge_request]) + @merge_request = MergeRequest.new(params[:merge_request]) + @merge_request.source_project = @project unless @merge_request.source_project + @merge_request.target_project = @project unless @merge_request.target_project + @target_branches = @merge_request.target_project.nil? ? [] : @merge_request.target_project.repository.branch_names + @source_project = @merge_request.source_project + @merge_request end def edit + @source_project = @merge_request.source_project + @target_project = @merge_request.target_project + @target_branches = @merge_request.target_project.repository.branch_names end def create - @merge_request = @project.merge_requests.new(params[:merge_request]) + @merge_request = MergeRequest.new(params[:merge_request]) @merge_request.author = current_user - + @target_branches ||= [] if @merge_request.save @merge_request.reload_code - redirect_to [@project, @merge_request], notice: 'Merge request was successfully created.' + redirect_to [@merge_request.target_project, @merge_request], notice: 'Merge request was successfully created.' else - render "new" + @source_project = @merge_request.source_project + @target_project = @merge_request.target_project + render action: "new" end end @@ -61,7 +71,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController if @merge_request.update_attributes(params[:merge_request].merge(author_id_of_changes: current_user.id)) @merge_request.reload_code @merge_request.mark_as_unchecked - redirect_to [@project, @merge_request], notice: 'Merge request was successfully updated.' + redirect_to [@merge_request.target_project, @merge_request], notice: 'Merge request was successfully updated.' else render "edit" end @@ -89,22 +99,35 @@ class Projects::MergeRequestsController < Projects::ApplicationController end def branch_from + #This is always source + @source_project = @merge_request.nil? ? @project : @merge_request.source_project @commit = @repository.commit(params[:ref]) end def branch_to - @commit = @repository.commit(params[:ref]) + @target_project = selected_target_project + @commit = @target_project.repository.commit(params[:ref]) + end + + def update_branches + @target_project = selected_target_project + @target_branches = @target_project.repository.branch_names + @target_branches end def ci_status status = project.gitlab_ci_service.commit_status(merge_request.last_commit.sha) - response = { status: status } + response = {status: status} render json: response end protected + def selected_target_project + ((@project.id.to_s == params[:target_project_id]) || @project.forked_project_link.nil?) ? @project : @project.forked_project_link.forked_from_project + end + def merge_request @merge_request ||= @project.merge_requests.find(params[:id]) end @@ -123,11 +146,11 @@ class Projects::MergeRequestsController < Projects::ApplicationController def validates_merge_request # Show git not found page if target branch doesn't exist - return invalid_mr unless @project.repository.branch_names.include?(@merge_request.target_branch) + return invalid_mr unless @merge_request.target_project.repository.branch_names.include?(@merge_request.target_branch) # Show git not found page if source branch doesn't exist # and there is no saved commits between source & target branch - return invalid_mr if !@project.repository.branch_names.include?(@merge_request.source_branch) && @merge_request.commits.blank? + return invalid_mr if !@merge_request.source_project.repository.branch_names.include?(@merge_request.source_branch) && @merge_request.commits.blank? end def define_show_vars diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index 111e1cda19c..20a2443f523 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -108,8 +108,8 @@ module CommitsHelper end end - def commit_to_html commit - escape_javascript(render 'projects/commits/commit', commit: commit) + def commit_to_html commit, project + escape_javascript(render 'projects/commits/commit', commit: commit, project: project) unless commit.nil? end def diff_line_content(line) diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index 05ffec066f8..4ba48aa4339 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -2,14 +2,27 @@ module MergeRequestsHelper def new_mr_path_from_push_event(event) new_project_merge_request_path( event.project, - merge_request: { - source_branch: event.branch_name, - target_branch: event.project.repository.root_ref, - title: event.branch_name.titleize - } + new_mr_from_push_event(event, event.project) ) end + def new_mr_path_for_fork_from_push_event(event) + new_project_merge_request_path( + event.project, + new_mr_from_push_event(event, event.project.forked_from_project) + ) + end + + def new_mr_from_push_event(event, target_project) + return :merge_request => { + source_project_id: event.project.id, + target_project_id: target_project.id, + source_branch: event.branch_name, + target_branch: target_project.repository.root_ref, + title: event.branch_name.titleize + } + end + def mr_css_classes mr classes = "merge-request" classes << " closed" if mr.closed? @@ -18,6 +31,14 @@ module MergeRequestsHelper end def ci_build_details_path merge_request - merge_request.project.gitlab_ci_service.build_page(merge_request.last_commit.sha) + merge_request.source_project.gitlab_ci_service.build_page(merge_request.last_commit.sha) + end + + def merge_path_description(merge_request, separator) + if merge_request.for_fork? + "Project:Branches: #{@merge_request.source_project.path_with_namespace}:#{@merge_request.source_branch} #{separator} #{@merge_request.target_project.path_with_namespace}:#{@merge_request.target_branch}" + else + "Branches: #{@merge_request.source_branch} #{separator} #{@merge_request.target_branch}" + end end end diff --git a/app/mailers/emails/merge_requests.rb b/app/mailers/emails/merge_requests.rb index de47903c0d4..cf6950966e1 100644 --- a/app/mailers/emails/merge_requests.rb +++ b/app/mailers/emails/merge_requests.rb @@ -2,28 +2,65 @@ module Emails module MergeRequests def new_merge_request_email(recipient_id, merge_request_id) @merge_request = MergeRequest.find(merge_request_id) - @project = @merge_request.project mail(to: recipient(recipient_id), subject: subject("new merge request !#{@merge_request.id}", @merge_request.title)) end def reassigned_merge_request_email(recipient_id, merge_request_id, previous_assignee_id) @merge_request = MergeRequest.find(merge_request_id) @previous_assignee = User.find_by_id(previous_assignee_id) if previous_assignee_id - @project = @merge_request.project mail(to: recipient(recipient_id), subject: subject("changed merge request !#{@merge_request.id}", @merge_request.title)) end def closed_merge_request_email(recipient_id, merge_request_id, updated_by_user_id) @merge_request = MergeRequest.find(merge_request_id) - @project = @merge_request.project @updated_by = User.find updated_by_user_id mail(to: recipient(recipient_id), subject: subject("Closed merge request !#{@merge_request.id}", @merge_request.title)) end def merged_merge_request_email(recipient_id, merge_request_id) @merge_request = MergeRequest.find(merge_request_id) - @project = @merge_request.project mail(to: recipient(recipient_id), subject: subject("Accepted merge request !#{@merge_request.id}", @merge_request.title)) end end + + # Over rides default behavour to show source/target + # Formats arguments into a String suitable for use as an email subject + # + # extra - Extra Strings to be inserted into the subject + # + # Examples + # + # >> subject('Lorem ipsum') + # => "GitLab Merge Request | Lorem ipsum" + # + # # Automatically inserts Project name: + # Forked MR + # => source project => <Project id: 1, name: "Ruby on Rails", path: "ruby_on_rails", ...> + # => target project => <Project id: 2, name: "My Ror", path: "ruby_on_rails", ...> + # => source branch => source + # => target branch => target + # >> subject('Lorem ipsum') + # => "GitLab Merge Request | Ruby on Rails:source >> My Ror:target | Lorem ipsum " + # + # Non Forked MR + # => source project => <Project id: 1, name: "Ruby on Rails", path: "ruby_on_rails", ...> + # => target project => <Project id: 1, name: "Ruby on Rails", path: "ruby_on_rails", ...> + # => source branch => source + # => target branch => target + # >> subject('Lorem ipsum') + # => "GitLab Merge Request | Ruby on Rails | source >> target | Lorem ipsum " + # # Accepts multiple arguments + # >> subject('Lorem ipsum', 'Dolor sit amet') + # => "GitLab Merge Request | Lorem ipsum | Dolor sit amet" + def subject(*extra) + subject = "GitLab Merge Request |" + if @merge_request.for_fork? + subject << "#{@merge_request.source_project.name_with_namespace}:#{merge_request.source_branch} >> #{@merge_request.target_project.name_with_namespace}:#{merge_request.target_branch}" + else + subject << "#{@merge_request.source_project.name_with_namespace} | #{merge_request.source_branch} >> #{merge_request.target_branch}" + end + subject << " | " + extra.join(' | ') if extra.present? + subject + end + end diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index a5aa97ab13d..7890ca7793b 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -6,6 +6,7 @@ class Notify < ActionMailer::Base add_template_helper ApplicationHelper add_template_helper GitlabMarkdownHelper + add_template_helper MergeRequestsHelper default_url_options[:host] = Gitlab.config.gitlab.host default_url_options[:protocol] = Gitlab.config.gitlab.protocol diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 8868e818daa..88de1a037aa 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -9,19 +9,14 @@ module Issuable include Mentionable included do - belongs_to :project belongs_to :author, class_name: "User" belongs_to :assignee, class_name: "User" belongs_to :milestone has_many :notes, as: :noteable, dependent: :destroy - validates :project, presence: true validates :author, presence: true validates :title, presence: true, length: { within: 0..255 } - scope :opened, -> { with_state(:opened) } - scope :closed, -> { with_state(:closed) } - scope :of_group, ->(group) { where(project_id: group.project_ids) } scope :assigned_to, ->(u) { where(assignee_id: u.id)} scope :recent, -> { order("created_at DESC") } scope :assigned, -> { where("assignee_id IS NOT NULL") } diff --git a/app/models/issue.rb b/app/models/issue.rb index f56928891dc..c171941928c 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -17,8 +17,18 @@ # class Issue < ActiveRecord::Base + include Issuable + belongs_to :project + validates :project, presence: true + + scope :of_group, ->(group) { where(project_id: group.project_ids) } + scope :of_user_team, ->(team) { where(project_id: team.project_ids, assignee_id: team.member_ids) } + scope :opened, -> { with_state(:opened) } + scope :closed, -> { with_state(:closed) } + scope :by_project, ->(project_id) {where(project_id:project_id)} + attr_accessible :title, :assignee_id, :position, :description, :milestone_id, :label_list, :author_id_of_changes, :state_event diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 2a476355404..703032649b9 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -2,30 +2,35 @@ # # Table name: merge_requests # -# id :integer not null, primary key -# target_branch :string(255) not null -# source_branch :string(255) not null -# project_id :integer not null -# author_id :integer -# assignee_id :integer -# title :string(255) -# created_at :datetime -# updated_at :datetime -# st_commits :text(2147483647) -# st_diffs :text(2147483647) -# milestone_id :integer -# state :string(255) -# merge_status :string(255) +# id :integer not null, primary key +# target_project_id :integer not null +# target_branch :string(255) not null +# source_project_id :integer not null +# source_branch :string(255) not null +# author_id :integer +# assignee_id :integer +# title :string(255) +# created_at :datetime +# updated_at :datetime +# st_commits :text(2147483647) +# st_diffs :text(2147483647) +# milestone_id :integer +# state :string(255) +# merge_status :string(255) # require Rails.root.join("app/models/commit") require Rails.root.join("lib/static_model") class MergeRequest < ActiveRecord::Base + include Issuable - attr_accessible :title, :assignee_id, :target_branch, :source_branch, :milestone_id, - :author_id_of_changes, :state_event + belongs_to :target_project, foreign_key: :target_project_id, class_name: "Project" + belongs_to :source_project, foreign_key: :source_project_id, class_name: "Project" + + attr_accessible :title, :assignee_id, :source_project_id, :source_branch, :target_project_id, :target_branch, :milestone_id, :author_id_of_changes, :state_event + attr_accessor :should_remove_source_branch @@ -74,30 +79,37 @@ class MergeRequest < ActiveRecord::Base serialize :st_commits serialize :st_diffs + validates :source_project, presence: true validates :source_branch, presence: true + validates :target_project, presence: true validates :target_branch, presence: true - validate :validate_branches + validate :validate_branches + scope :of_group, ->(group) { where("source_project_id in (:group_project_ids) OR target_project_id in (:group_project_ids)", group_project_ids: group.project_ids) } + scope :of_user_team, ->(team) { where("(source_project_id in (:team_project_ids) OR target_project_id in (:team_project_ids) AND assignee_id in (:team_member_ids))", team_project_ids: team.project_ids, team_member_ids: team.member_ids) } + scope :opened, -> { with_state(:opened) } + scope :closed, -> { with_state(:closed) } scope :merged, -> { with_state(:merged) } - scope :by_branch, ->(branch_name) { where("source_branch LIKE :branch OR target_branch LIKE :branch", branch: branch_name) } + scope :by_branch, ->(branch_name) { where("(source_branch LIKE :branch) OR (target_branch LIKE :branch)", branch: branch_name) } scope :cared, ->(user) { where('assignee_id = :user OR author_id = :user', user: user.id) } scope :by_milestone, ->(milestone) { where(milestone_id: milestone) } - + scope :by_project, ->(project_id) { where("source_project_id = :project_id OR target_project_id = :project_id", project_id: project_id) } + scope :in_projects, ->(project_ids) { where("source_project_id in (:project_ids) OR target_project_id in (:project_ids)", project_ids: project_ids) } # Closed scope for merge request should return # both merged and closed mr's scope :closed, -> { with_states(:closed, :merged) } def validate_branches - if target_branch == source_branch - errors.add :branch_conflict, "You can not use same branch for source and target branches" + 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 if opened? || reopened? - similar_mrs = self.project.merge_requests.where(source_branch: source_branch, target_branch: target_branch).opened + similar_mrs = self.target_project.merge_requests.where(source_branch: source_branch, target_branch: target_branch, source_project_id: source_project.id).opened similar_mrs = similar_mrs.where('id not in (?)', self.id) if self.id if similar_mrs.any? - errors.add :base, "There is already an open merge request for this branches" + errors.add :base, "Cannot Create: This merge request already exists: #{similar_mrs.pluck(:title)}" end end end @@ -137,7 +149,13 @@ class MergeRequest < ActiveRecord::Base end def unmerged_diffs - project.repository.diffs_between(source_branch, target_branch) + if for_fork? + diffs = Gitlab::Satellite::MergeAction.new(author, self).diffs_between_satellite + else + diffs = target_project.repository.diffs_between(source_branch, target_branch) + end + diffs ||= [] + diffs end def last_commit @@ -145,11 +163,11 @@ class MergeRequest < ActiveRecord::Base end def merge_event - self.project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::MERGED).last + self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::MERGED).last end def closed_event - self.project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::CLOSED).last + self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::CLOSED).last end def commits @@ -165,15 +183,23 @@ class MergeRequest < ActiveRecord::Base if opened? && unmerged_commits.any? self.st_commits = dump_commits(unmerged_commits) save + end commits end def unmerged_commits - self.project.repository. - commits_between(self.target_branch, self.source_branch). + 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) @@ -199,21 +225,29 @@ class MergeRequest < ActiveRecord::Base # Returns the raw diff for this merge request # # see "git diff" - def to_diff - project.repo.git.native(:diff, {timeout: 30, raise: true}, "#{target_branch}...#{source_branch}") + def to_diff(current_user) + Gitlab::Satellite::MergeAction.new(current_user, self).diff_in_satellite end # Returns the commit as a series of email patches. # # see "git format-patch" - def to_patch - project.repo.git.format_patch({timeout: 30, raise: true, stdout: true}, "#{target_branch}..#{source_branch}") + def to_patch(current_user) + 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 + + def disallow_source_branch_removal? + (source_project.root_ref? source_branch) || for_fork? + end + private def dump_commits(commits) diff --git a/app/models/note.rb b/app/models/note.rb index c23aab03bcc..fecee950713 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -42,18 +42,18 @@ class Note < ActiveRecord::Base # Scopes scope :for_commit_id, ->(commit_id) { where(noteable_type: "Commit", commit_id: commit_id) } - scope :inline, -> { where("line_code IS NOT NULL") } - scope :not_inline, -> { where(line_code: [nil, '']) } + scope :inline, ->{ where("line_code IS NOT NULL") } + scope :not_inline, ->{ where(line_code: [nil, '']) } scope :common, ->{ where(noteable_type: ["", nil]) } scope :fresh, ->{ order("created_at ASC, id ASC") } scope :inc_author_project, ->{ includes(:project, :author) } scope :inc_author, ->{ includes(:author) } - def self.create_status_change_note(noteable, author, status) + def self.create_status_change_note(noteable, project, author, status) create({ noteable: noteable, - project: noteable.project, + project: project, author: author, note: "_Status changed to #{status}_" }, without_protection: true) @@ -62,7 +62,7 @@ class Note < ActiveRecord::Base def commit_author @commit_author ||= project.users.find_by_email(noteable.author_email) || - project.users.find_by_name(noteable.author_name) + project.users.find_by_name(noteable.author_name) rescue nil end diff --git a/app/models/project.rb b/app/models/project.rb index 497ed1a218c..59c6d212f48 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -53,7 +53,7 @@ class Project < ActiveRecord::Base has_many :services, dependent: :destroy has_many :events, dependent: :destroy - has_many :merge_requests, dependent: :destroy + has_many :merge_requests, dependent: :destroy, foreign_key: "target_project_id" has_many :issues, dependent: :destroy, order: "state DESC, created_at DESC" has_many :milestones, dependent: :destroy has_many :notes, dependent: :destroy diff --git a/app/observers/activity_observer.rb b/app/observers/activity_observer.rb index 4f4eb36a188..91f097154d8 100644 --- a/app/observers/activity_observer.rb +++ b/app/observers/activity_observer.rb @@ -1,5 +1,5 @@ class ActivityObserver < BaseObserver - observe :issue, :merge_request, :note, :milestone + observe :issue, :note, :milestone def after_create(record) event_author_id = record.author_id @@ -13,47 +13,27 @@ class ActivityObserver < BaseObserver end if event_author_id - Event.create( - project: record.project, - target_id: record.id, - target_type: record.class.name, - action: Event.determine_action(record), - author_id: event_author_id - ) + create_event(record, Event.determine_action(record)) end end def after_close(record, transition) - Event.create( - project: record.project, - target_id: record.id, - target_type: record.class.name, - action: Event::CLOSED, - author_id: record.author_id_of_changes - ) + create_event(record, Event::CLOSED) end def after_reopen(record, transition) - Event.create( - project: record.project, - target_id: record.id, - target_type: record.class.name, - action: Event::REOPENED, - author_id: record.author_id_of_changes - ) + create_event(record, Event::REOPENED) end - def after_merge(record, transition) - # Since MR can be merged via sidekiq - # to prevent event duplication do this check - return true if record.merge_event + protected + def create_event(record, status) Event.create( - project: record.project, - target_id: record.id, - target_type: record.class.name, - action: Event::MERGED, - author_id: record.author_id_of_changes + project: record.project, + target_id: record.id, + target_type: record.class.name, + action: status, + author_id: record.author_id ) end end diff --git a/app/observers/issue_observer.rb b/app/observers/issue_observer.rb index 888fa7f6b73..50538419776 100644 --- a/app/observers/issue_observer.rb +++ b/app/observers/issue_observer.rb @@ -23,6 +23,6 @@ class IssueObserver < BaseObserver # Create issue note with service comment like 'Status changed to closed' def create_note(issue) - Note.create_status_change_note(issue, current_user, issue.state) + Note.create_status_change_note(issue, issue.project, current_user, issue.state) end end diff --git a/app/observers/merge_request_observer.rb b/app/observers/merge_request_observer.rb index 03d4a22c1e6..436645a376f 100644 --- a/app/observers/merge_request_observer.rb +++ b/app/observers/merge_request_observer.rb @@ -1,23 +1,53 @@ -class MergeRequestObserver < BaseObserver +class MergeRequestObserver < ActivityObserver + observe :merge_request + def after_create(merge_request) + if merge_request.author_id + create_event(merge_request, Event.determine_action(merge_request)) + end + notification.new_merge_request(merge_request, current_user) end def after_close(merge_request, transition) - Note.create_status_change_note(merge_request, current_user, merge_request.state) + create_event(merge_request, Event::CLOSED) + Note.create_status_change_note(merge_request, merge_request.target_project, current_user, merge_request.state) notification.close_mr(merge_request, current_user) end def after_merge(merge_request, transition) notification.merge_mr(merge_request) + # Since MR can be merged via sidekiq + # to prevent event duplication do this check + return true if merge_request.merge_event + + Event.create( + project: merge_request.target_project, + target_id: merge_request.id, + target_type: merge_request.class.name, + action: Event::MERGED, + author_id: merge_request.author_id_of_changes + ) end def after_reopen(merge_request, transition) - Note.create_status_change_note(merge_request, current_user, merge_request.state) + create_event(merge_request, Event::REOPENED) + Note.create_status_change_note(merge_request, merge_request.target_project, current_user, merge_request.state) end def after_update(merge_request) notification.reassigned_merge_request(merge_request, current_user) if merge_request.is_being_reassigned? end + + def create_event(record, status) + Event.create( + project: record.target_project, + target_id: record.id, + target_type: record.class.name, + action: status, + author_id: record.author_id + ) + end + end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index b5cf5cedd6b..5548c18bfe1 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -23,7 +23,7 @@ class NotificationService # * project team members with notification level higher then Participating # def new_issue(issue, current_user) - new_resource_email(issue, 'new_issue_email') + new_resource_email(issue, issue.project, 'new_issue_email') end # When we close an issue we should send next emails: @@ -33,7 +33,7 @@ class NotificationService # * project team members with notification level higher then Participating # def close_issue(issue, current_user) - close_resource_email(issue, current_user, 'closed_issue_email') + close_resource_email(issue, issue.project, current_user, 'closed_issue_email') end # When we reassign an issue we should send next emails: @@ -42,7 +42,7 @@ class NotificationService # * issue new assignee if his notification level is not Disabled # def reassigned_issue(issue, current_user) - reassign_resource_email(issue, current_user, 'reassigned_issue_email') + reassign_resource_email(issue, issue.project, current_user, 'reassigned_issue_email') end @@ -51,7 +51,7 @@ class NotificationService # * mr assignee if his notification level is not Disabled # def new_merge_request(merge_request, current_user) - new_resource_email(merge_request, 'new_merge_request_email') + new_resource_email(merge_request, merge_request.target_project, 'new_merge_request_email') end # When we reassign a merge_request we should send next emails: @@ -60,7 +60,7 @@ class NotificationService # * merge_request assignee if his notification level is not Disabled # def reassigned_merge_request(merge_request, current_user) - reassign_resource_email(merge_request, current_user, 'reassigned_merge_request_email') + reassign_resource_email(merge_request, merge_request.target_project, current_user, 'reassigned_merge_request_email') end # When we close a merge request we should send next emails: @@ -70,7 +70,7 @@ class NotificationService # * project team members with notification level higher then Participating # def close_mr(merge_request, current_user) - close_resource_email(merge_request, current_user, 'closed_merge_request_email') + close_resource_email(merge_request, merge_request.target_project, current_user, 'closed_merge_request_email') end # When we merge a merge request we should send next emails: @@ -80,8 +80,8 @@ class NotificationService # * project team members with notification level higher then Participating # def merge_mr(merge_request) - recipients = reject_muted_users([merge_request.author, merge_request.assignee], merge_request.project) - recipients = recipients.concat(project_watchers(merge_request.project)).uniq + recipients = reject_muted_users([merge_request.author, merge_request.assignee], merge_request.target_project) + recipients = recipients.concat(project_watchers(merge_request.target_project)).uniq recipients.each do |recipient| mailer.merged_merge_request_email(recipient.id, merge_request.id) @@ -191,14 +191,14 @@ class NotificationService end end - def new_resource_email(target, method) + def new_resource_email(target, project, method) if target.respond_to?(:participants) recipients = target.participants else recipients = [] end - recipients = reject_muted_users(recipients, target.project) - recipients = recipients.concat(project_watchers(target.project)).uniq + recipients = reject_muted_users(recipients, project) + recipients = recipients.concat(project_watchers(project)).uniq recipients.delete(target.author) recipients.each do |recipient| @@ -206,9 +206,9 @@ class NotificationService end end - def close_resource_email(target, current_user, method) - recipients = reject_muted_users([target.author, target.assignee], target.project) - recipients = recipients.concat(project_watchers(target.project)).uniq + def close_resource_email(target, project, current_user, method) + recipients = reject_muted_users([target.author, target.assignee], project) + recipients = recipients.concat(project_watchers(project)).uniq recipients.delete(current_user) recipients.each do |recipient| @@ -216,14 +216,14 @@ class NotificationService end end - def reassign_resource_email(target, current_user, method) + def reassign_resource_email(target, project, current_user, method) recipients = User.where(id: [target.assignee_id, target.assignee_id_was]) # Add watchers to email list - recipients = recipients.concat(project_watchers(target.project)) + recipients = recipients.concat(project_watchers(project)) # reject users with disabled notifications - recipients = reject_muted_users(recipients, target.project) + recipients = reject_muted_users(recipients, project) # Reject me from recipients if I reassign an item recipients.delete(current_user) diff --git a/app/views/events/_event_last_push.html.haml b/app/views/events/_event_last_push.html.haml index de5634d3c55..a634365ff3e 100644 --- a/app/views/events/_event_last_push.html.haml +++ b/app/views/events/_event_last_push.html.haml @@ -9,6 +9,9 @@ = time_ago_in_words(event.created_at) ago. .pull-right - = link_to new_mr_path_from_push_event(event), title: "New Merge Request", class: "btn btn-create btn-small" do + = link_to new_mr_path_from_push_event(event), title: "New Merge Request", class: "btn btn-new-mr" do Create Merge Request + - if !event.project.nil? && event.project.forked? + = link_to new_mr_path_for_fork_from_push_event(event), title: "New Merge Request", class: "btn btn-create btn-small" do + Create Merge Request on fork %hr diff --git a/app/views/notify/closed_merge_request_email.html.haml b/app/views/notify/closed_merge_request_email.html.haml index 0c6c79e097a..c6b45b1710a 100644 --- a/app/views/notify/closed_merge_request_email.html.haml +++ b/app/views/notify/closed_merge_request_email.html.haml @@ -1,9 +1,9 @@ %p = "Merge Request #{@merge_request.id} was closed by #{@updated_by.name}" %p - = link_to_gfm truncate(@merge_request.title, length: 40), project_merge_request_url(@merge_request.project, @merge_request) + = link_to_gfm truncate(@merge_request.title, length: 40), project_merge_request_url(@merge_request.target_project, @merge_request) %p - Branches: #{@merge_request.source_branch} → #{@merge_request.target_branch} + != merge_path_description(@merge_request, '→') %p Assignee: #{@merge_request.author_name} → #{@merge_request.assignee_name} diff --git a/app/views/notify/closed_merge_request_email.text.haml b/app/views/notify/closed_merge_request_email.text.haml index ee4648e3d09..9a5bce5915a 100644 --- a/app/views/notify/closed_merge_request_email.text.haml +++ b/app/views/notify/closed_merge_request_email.text.haml @@ -1,8 +1,8 @@ = "Merge Request #{@merge_request.id} was closed by #{@updated_by.name}" -Merge Request url: #{project_merge_request_url(@merge_request.project, @merge_request)} +Merge Request url: #{project_merge_request_url(@merge_request.target_project, @merge_request)} -Branches: #{@merge_request.source_branch} - #{@merge_request.target_branch} += merge_path_description(@merge_request, 'to') Author: #{@merge_request.author_name} Assignee: #{@merge_request.assignee_name} diff --git a/app/views/notify/merged_merge_request_email.html.haml b/app/views/notify/merged_merge_request_email.html.haml index 2b8cc030b23..e3ae41819f8 100644 --- a/app/views/notify/merged_merge_request_email.html.haml +++ b/app/views/notify/merged_merge_request_email.html.haml @@ -1,9 +1,9 @@ %p = "Merge Request #{@merge_request.id} was merged" %p - = link_to_gfm truncate(@merge_request.title, length: 40), project_merge_request_url(@merge_request.project, @merge_request) + = link_to_gfm truncate(@merge_request.title, length: 40), project_merge_request_url(@merge_request.target_project, @merge_request) %p - Branches: #{@merge_request.source_branch} → #{@merge_request.target_branch} + != merge_path_description(@merge_request, '→') %p Assignee: #{@merge_request.author_name} → #{@merge_request.assignee_name} diff --git a/app/views/notify/merged_merge_request_email.text.haml b/app/views/notify/merged_merge_request_email.text.haml index 91c23360195..fa9ef0a4b32 100644 --- a/app/views/notify/merged_merge_request_email.text.haml +++ b/app/views/notify/merged_merge_request_email.text.haml @@ -1,8 +1,8 @@ = "Merge Request #{@merge_request.id} was merged" -Merge Request Url: #{project_merge_request_url(@merge_request.project, @merge_request)} +Merge Request Url: #{project_merge_request_url(@merge_request.target_project, @merge_request)} -Branches: #{@merge_request.source_branch} - #{@merge_request.target_branch} += merge_path_description(@merge_request, 'to') Author: #{@merge_request.author_name} Assignee: #{@merge_request.assignee_name} diff --git a/app/views/notify/new_merge_request_email.html.haml b/app/views/notify/new_merge_request_email.html.haml index 0f1cfff5831..3b060001e6b 100644 --- a/app/views/notify/new_merge_request_email.html.haml +++ b/app/views/notify/new_merge_request_email.html.haml @@ -1,9 +1,9 @@ %p = "New Merge Request !#{@merge_request.id}" %p - = link_to_gfm truncate(@merge_request.title, length: 40), project_merge_request_url(@merge_request.project, @merge_request) + = link_to_gfm truncate(@merge_request.title, length: 40), project_merge_request_url(@merge_request.target_project, @merge_request) %p - Branches: #{@merge_request.source_branch} → #{@merge_request.target_branch} + != merge_path_description(@merge_request, '→') %p Assignee: #{@merge_request.author_name} → #{@merge_request.assignee_name} diff --git a/app/views/notify/new_merge_request_email.text.erb b/app/views/notify/new_merge_request_email.text.erb index 3393d8384f1..3f711840a51 100644 --- a/app/views/notify/new_merge_request_email.text.erb +++ b/app/views/notify/new_merge_request_email.text.erb @@ -1,9 +1,8 @@ New Merge Request <%= @merge_request.id %> -<%= url_for(project_merge_request_url(@merge_request.project, @merge_request)) %> - +<%= url_for(project_merge_request_url(@merge_request.target_project, @merge_request)) %> -Branches: <%= @merge_request.source_branch %> to <%= @merge_request.target_branch %> +<%= merge_path_description(@merge_request, 'to') %> Author: <%= @merge_request.author_name %> Asignee: <%= @merge_request.assignee_name %> diff --git a/app/views/notify/note_merge_request_email.html.haml b/app/views/notify/note_merge_request_email.html.haml index 4f97867e49d..14abcca6070 100644 --- a/app/views/notify/note_merge_request_email.html.haml +++ b/app/views/notify/note_merge_request_email.html.haml @@ -1,8 +1,8 @@ %p - if @note.for_diff_line? - = link_to "New comment on diff", diffs_project_merge_request_url(@merge_request.project, @merge_request, anchor: "note_#{@note.id}") + = link_to "New comment on diff", diffs_project_merge_request_url(@merge_request.target_project, @merge_request, anchor: "note_#{@note.id}") - else - = link_to "New comment", project_merge_request_url(@merge_request.project, @merge_request, anchor: "note_#{@note.id}") + = link_to "New comment", project_merge_request_url(@merge_request.target_project, @merge_request, anchor: "note_#{@note.id}") for Merge Request ##{@merge_request.id} %cite "#{truncate(@merge_request.title, length: 20)}" = render 'note_message' diff --git a/app/views/notify/note_merge_request_email.text.erb b/app/views/notify/note_merge_request_email.text.erb index 26c73bdaa38..3daa091db81 100644 --- a/app/views/notify/note_merge_request_email.text.erb +++ b/app/views/notify/note_merge_request_email.text.erb @@ -1,6 +1,6 @@ New comment for Merge Request <%= @merge_request.id %> -<%= url_for(project_merge_request_url(@merge_request.project, @merge_request, anchor: "note_#{@note.id}")) %> +<%= url_for(project_merge_request_url(@merge_request.target_project, @merge_request, anchor: "note_#{@note.id}")) %> <%= @note.author_name %> diff --git a/app/views/notify/reassigned_merge_request_email.html.haml b/app/views/notify/reassigned_merge_request_email.html.haml index 5ad72764e38..314b2882672 100644 --- a/app/views/notify/reassigned_merge_request_email.html.haml +++ b/app/views/notify/reassigned_merge_request_email.html.haml @@ -1,6 +1,6 @@ %p = "Reassigned Merge Request !#{@merge_request.id}" - = link_to_gfm truncate(@merge_request.title, length: 30), project_merge_request_url(@merge_request.project, @merge_request) + = link_to_gfm truncate(@merge_request.title, length: 30), project_merge_request_url(@merge_request.target_project, @merge_request) %p Assignee changed - if @previous_assignee diff --git a/app/views/notify/reassigned_merge_request_email.text.erb b/app/views/notify/reassigned_merge_request_email.text.erb index 25b2a43fcdd..05a9797165d 100644 --- a/app/views/notify/reassigned_merge_request_email.text.erb +++ b/app/views/notify/reassigned_merge_request_email.text.erb @@ -1,6 +1,6 @@ Reassigned Merge Request <%= @merge_request.id %> -<%= url_for(project_merge_request_url(@merge_request.project, @merge_request)) %> +<%= url_for(project_merge_request_url(@merge_request.target_project, @merge_request)) %> Assignee changed <%= "from #{@previous_assignee.name}" if @previous_assignee %> to <%= @merge_request.assignee_name %> diff --git a/app/views/projects/commit/show.html.haml b/app/views/projects/commit/show.html.haml index 38c98d81b5c..da1b4c10f87 100644 --- a/app/views/projects/commit/show.html.haml +++ b/app/views/projects/commit/show.html.haml @@ -1,3 +1,3 @@ = render "commit_box" -= render "projects/commits/diffs", diffs: @commit.diffs += render "projects/commits/diffs", diffs: @commit.diffs, project: @project = render "projects/notes/notes_with_form" diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index eba6c206c46..eb5bdb886e2 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -1,12 +1,12 @@ %li.commit .browse_code_link_holder %p - %strong= link_to "Browse Code »", project_tree_path(@project, commit), class: "right" + %strong= link_to "Browse Code »", project_tree_path(project, commit), class: "right" %p - = link_to commit.short_id(8), project_commit_path(@project, commit), class: "commit_short_id" + = link_to commit.short_id(8), project_commit_path(project, commit), class: "commit_short_id" = commit_author_link(commit, avatar: true, size: 24) - = link_to_gfm truncate(commit.title, length: 70), project_commit_path(@project, commit.id), class: "row_title" + = link_to_gfm truncate(commit.title, length: 70), project_commit_path(project, commit.id), class: "row_title" %time.committed_ago{ datetime: commit.committed_date, title: commit.committed_date.stamp("Aug 21, 2011 9:23pm") } = time_ago_in_words(commit.committed_date) @@ -14,7 +14,7 @@ %span.notes_count - - notes = @project.notes.for_commit_id(commit.id) + - notes = project.notes.for_commit_id(commit.id) - if notes.any? %span.badge.badge-info %i.icon-comment diff --git a/app/views/projects/commits/_commits.html.haml b/app/views/projects/commits/_commits.html.haml index acdb8891344..243299d028c 100644 --- a/app/views/projects/commits/_commits.html.haml +++ b/app/views/projects/commits/_commits.html.haml @@ -3,7 +3,6 @@ .title %i.icon-calendar %span= day.stamp("28 Aug, 2010") - .pull-right %small= pluralize(commits.count, 'commit') - %ul.well-list= render commits + %ul.well-list= render commits, project: @project diff --git a/app/views/projects/commits/_diffs.html.haml b/app/views/projects/commits/_diffs.html.haml index a60bf75175d..ca025731258 100644 --- a/app/views/projects/commits/_diffs.html.haml +++ b/app/views/projects/commits/_diffs.html.haml @@ -5,7 +5,7 @@ %p To prevent performance issue we rejected diff information. %p But if you still want to see diff - = link_to "click this link", project_commit_path(@project, @commit, force_show_diff: true), class: "underlined_link" + = link_to "click this link", project_commit_path(project, @commit, force_show_diff: true), class: "underlined_link" %p.commit-stat-summary Showing @@ -23,8 +23,8 @@ - unless @suppress_diff - diffs.each_with_index do |diff, i| - next if diff.diff.empty? - - file = Gitlab::Git::Blob.new(@repository, @commit.id, @ref, diff.new_path) - - file = Gitlab::Git::Blob.new(@repository, @commit.parent_id, @ref, diff.old_path) unless file.exists? + - file = Gitlab::Git::Blob.new(project.repository, @commit.id, @ref, diff.new_path) + - file = Gitlab::Git::Blob.new(project.repository, @commit.parent_id, @ref, diff.old_path) unless file.exists? - next unless file.exists? .file{id: "diff-#{i}"} .header @@ -32,7 +32,7 @@ %span= diff.old_path - if @commit.parent_ids.present? - = link_to project_blob_path(@project, tree_join(@commit.parent_id, diff.new_path)), {:class => 'btn btn-tiny pull-right view-file'} do + = link_to project_blob_path(project, tree_join(@commit.parent_id, diff.new_path)), {:class => 'btn btn-tiny pull-right view-file'} do View file @ %span.commit-short-id= @commit.short_id(6) - else @@ -40,7 +40,7 @@ - if diff.a_mode && diff.b_mode && diff.a_mode != diff.b_mode %span.file-mode= "#{diff.a_mode} → #{diff.b_mode}" - = link_to project_blob_path(@project, tree_join(@commit.id, diff.new_path)), {:class => 'btn btn-tiny pull-right view-file'} do + = link_to project_blob_path(project, tree_join(@commit.id, diff.new_path)), {:class => 'btn btn-tiny pull-right view-file'} do View file @ %span.commit-short-id= @commit.short_id(6) @@ -50,7 +50,7 @@ - if file.text? = render "projects/commits/text_file", diff: diff, index: i - elsif file.image? - - old_file = Gitlab::Git::Blob.new(@repository, @commit.parent_id, @ref, diff.old_path) if @commit.parent_id + - old_file = Gitlab::Git::Blob.new(project.repository, @commit.parent_id, @ref, diff.old_path) if @commit.parent_id = render "projects/commits/image", diff: diff, old_file: old_file, file: file, index: i - else %p.nothing_here_message No preview for this file type diff --git a/app/views/projects/compare/show.html.haml b/app/views/projects/compare/show.html.haml index f2e65f68da6..266c14b358a 100644 --- a/app/views/projects/compare/show.html.haml +++ b/app/views/projects/compare/show.html.haml @@ -15,8 +15,8 @@ %div.ui-box .title Commits (#{@commits.count}) - %ul.well-list= render Commit.decorate(@commits) + %ul.well-list= render Commit.decorate(@commits), project: @project - unless @diffs.empty? %h4 Diff - = render "projects/commits/diffs", diffs: @diffs + = render "projects/commits/diffs", diffs: @diffs, project: @project diff --git a/app/views/projects/merge_requests/_form.html.haml b/app/views/projects/merge_requests/_form.html.haml index 86c442142bf..54697706198 100644 --- a/app/views/projects/merge_requests/_form.html.haml +++ b/app/views/projects/merge_requests/_form.html.haml @@ -12,18 +12,20 @@ .row .span5 .light-well - %h5.cgray From (Head Branch) - = f.select(:source_branch, @repository.branch_names, { include_blank: "Select branch" }, {class: 'chosen span4'}) + %h5.cgray From + .padded= f.select(:source_project_id,[[@merge_request.source_project.path_with_namespace,@merge_request.source_project.id]] , {}, {class: 'source_project chosen span4'}) + .padded= f.select(:source_branch, @merge_request.source_project.repository.branch_names, { include_blank: "Select branch" }, {class: 'source_branch chosen span4'}) .mr_source_commit.prepend-top-10 - .span2 %h1.merge-request-angle %i.icon-angle-right .span5 .light-well - %h5.cgray To (Base Branch) - = f.select(:target_branch, @repository.branch_names, { include_blank: "Select branch" }, {class: 'chosen span4'}) - .mr_target_commit.prepend-top-10 + %h5.cgray To + - projects = @project.forked_from_project.nil? ? [@project] : [ @project,@project.forked_from_project] + .padded= f.select(:target_project_id, projects.map { |proj| [proj.path_with_namespace,proj.id] }, {include_blank: "Select Target Project" }, {class: 'target_project chosen span4'}) + .padded= f.select(:target_branch, @target_branches, { include_blank: "Select branch" }, {class: 'target_branch chosen span4'}) + .mr_target_commit.prepend-top-10 %hr @@ -47,32 +49,34 @@ Milestone .input= f.select(:milestone_id, @project.milestones.active.all.map {|p| [ p.title, p.id ] }, { include_blank: "Select milestone" }, {class: 'chosen'}) - .form-actions - if @merge_request.new_record? = f.submit 'Submit merge request', class: "btn btn-create" -else = f.submit 'Save changes', class: "btn btn-save" - if @merge_request.new_record? - = link_to project_merge_requests_path(@project), class: "btn btn-cancel" do + = link_to project_merge_requests_path(@source_project), class: "btn btn-cancel" do Cancel - else - = link_to project_merge_request_path(@project, @merge_request), class: "btn btn-cancel" do + = link_to project_merge_request_path(@target_project, @merge_request), class: "btn btn-cancel" do Cancel :javascript disableButtonIfEmptyField("#merge_request_title", ".btn-save"); var source_branch = $("#merge_request_source_branch") - , target_branch = $("#merge_request_target_branch"); + , target_branch = $("#merge_request_target_branch") + , target_project = $("#merge_request_target_project_id"); - $.get("#{branch_from_project_merge_requests_path(@project)}", {ref: source_branch.val() }); - $.get("#{branch_to_project_merge_requests_path(@project)}", {ref: target_branch.val() }); + $.get("#{branch_from_project_merge_requests_path(@source_project)}", {ref: source_branch.val() }); + $.get("#{branch_to_project_merge_requests_path(@source_project)}", {target_project_id: target_project.val(),ref: target_branch.val() }); - source_branch.live("change", function() { - $.get("#{branch_from_project_merge_requests_path(@project)}", {ref: $(this).val() }); + target_project.on("change", function() { + $.get("#{update_branches_project_merge_requests_path(@source_project)}", {target_project_id: $(this).val() }); }); - - target_branch.live("change", function() { - $.get("#{branch_to_project_merge_requests_path(@project)}", {ref: $(this).val() }); + source_branch.on("change", function() { + $.get("#{branch_from_project_merge_requests_path(@source_project)}", {ref: $(this).val() }); + }); + target_branch.on("change", function() { + $.get("#{branch_to_project_merge_requests_path(@source_project)}", {target_project_id: target_project.val(),ref: $(this).val() }); }); diff --git a/app/views/projects/merge_requests/_merge_request.html.haml b/app/views/projects/merge_requests/_merge_request.html.haml index ffc6b8fda1e..a10ed02a077 100644 --- a/app/views/projects/merge_requests/_merge_request.html.haml +++ b/app/views/projects/merge_requests/_merge_request.html.haml @@ -1,18 +1,24 @@ %li{ class: mr_css_classes(merge_request) } .merge-request-title %span.light= "##{merge_request.id}" - = link_to_gfm truncate(merge_request.title, length: 80), project_merge_request_path(merge_request.project, merge_request), class: "row_title" + = link_to_gfm truncate(merge_request.title, length: 80), project_merge_request_path(merge_request.target_project, merge_request), class: "row_title" - if merge_request.merged? %small.pull-right %i.icon-ok = "MERGED" - else %span.pull-right - %i.icon-angle-right - = merge_request.target_branch + - if merge_request.for_fork? + = "#{merge_request.source_project.path_with_namespace}/#{merge_request.source_branch}" + %i.icon-angle-right + = "#{merge_request.target_project.path_with_namespace}/#{merge_request.target_branch}" + - else + = "#{merge_request.source_branch}" + %i.icon-angle-right + = "#{merge_request.target_branch}" .merge-request-info - if merge_request.author - authored by #{link_to_member(@project, merge_request.author)} + authored by #{link_to_member(merge_request.source_project, merge_request.author)} - if merge_request.votes_count > 0 = render 'votes/votes_inline', votable: merge_request - if merge_request.notes.any? diff --git a/app/views/projects/merge_requests/branch_from.js.haml b/app/views/projects/merge_requests/branch_from.js.haml index 1d6b10f03ae..d0a148575dc 100644 --- a/app/views/projects/merge_requests/branch_from.js.haml +++ b/app/views/projects/merge_requests/branch_from.js.haml @@ -1,5 +1,5 @@ :plain - $(".mr_source_commit").html("#{commit_to_html(@commit)}"); + $(".mr_source_commit").html("#{commit_to_html(@commit, @source_project)}"); var mrTitle = $('#merge_request_title'); if(mrTitle.is(":empty")) { diff --git a/app/views/projects/merge_requests/branch_to.js.haml b/app/views/projects/merge_requests/branch_to.js.haml index 974100d1ba7..f4e2886ee44 100644 --- a/app/views/projects/merge_requests/branch_to.js.haml +++ b/app/views/projects/merge_requests/branch_to.js.haml @@ -1,2 +1,2 @@ :plain - $(".mr_target_commit").html("#{commit_to_html(@commit)}"); + $(".mr_target_commit").html("#{commit_to_html(@commit, @target_project)}"); diff --git a/app/views/projects/merge_requests/show/_commits.html.haml b/app/views/projects/merge_requests/show/_commits.html.haml index 40876d16ddd..7b0e67053a5 100644 --- a/app/views/projects/merge_requests/show/_commits.html.haml +++ b/app/views/projects/merge_requests/show/_commits.html.haml @@ -7,19 +7,19 @@ - if @commits.count > 8 %ul.first-commits.well-list - @commits.first(8).each do |commit| - = render "projects/commits/commit", commit: commit + = render "projects/commits/commit", commit: commit, project: @merge_request.source_project %li.bottom 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 + = render "projects/commits/commit", commit: commit, project: @merge_request.source_project - else %ul.well-list - @commits.each do |commit| - = render "projects/commits/commit", commit: commit + = render "projects/commits/commit", commit: commit, project: @merge_request.source_project - else %h4.nothing_here_message diff --git a/app/views/projects/merge_requests/show/_diffs.html.haml b/app/views/projects/merge_requests/show/_diffs.html.haml index 3c1d14572aa..25f63804858 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? - = render "projects/commits/diffs", diffs: @merge_request.diffs + = render "projects/commits/diffs", diffs: @merge_request.diffs, project: @merge_request.source_project - elsif @merge_request.broken_diffs? %h4.nothing_here_message Can't load diff. You can - = link_to "download it", project_merge_request_path(@project, @merge_request, format: :diff), class: "vlink" + = 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 diff --git a/app/views/projects/merge_requests/show/_how_to_merge.html.haml b/app/views/projects/merge_requests/show/_how_to_merge.html.haml index 7f1e33418de..a0eb2309585 100644 --- a/app/views/projects/merge_requests/show/_how_to_merge.html.haml +++ b/app/views/projects/merge_requests/show/_how_to_merge.html.haml @@ -3,17 +3,49 @@ %a.close{href: "#"} × %h3 How To Merge .modal-body - %p - %strong Step 1. - Checkout target branch and get recent objects from GitLab - %pre.dark - :preserve - git checkout #{@merge_request.target_branch} - git fetch origin - %p - %strong Step 2. - Merge source branch into target branch and push changes to GitLab - %pre.dark - :preserve - git merge origin/#{@merge_request.source_branch} - git push origin #{@merge_request.target_branch} + - if @merge_request.for_fork? + - source_remote = @merge_request.source_project.namespace.nil? ? "source" :@merge_request.source_project.namespace.path + - target_remote = @merge_request.target_project.namespace.nil? ? "target" :@merge_request.target_project.namespace.path + %p + %strong Step 1. + Checkout target branch and get recent objects from GitLab + Assuming remote for #{@merge_request.target_project.path_with_namespace} is called #{target_remote} + remote for #{@merge_request.source_project.path_with_namespace} is called #{source_remote} + %pre.dark + :preserve + git checkout #{target_remote} #{@merge_request.target_branch} + git fetch #{source_remote} + %p + %strong Step 2. + Merge source branch into target branch and push changes to GitLab + %pre.dark + :preserve + git merge #{source_remote}/#{@merge_request.source_branch} + git push #{target_remote} #{@merge_request.target_branch} + - else + %p + %strong Step 1. + Checkout target branch and get recent objects from GitLab + %pre.dark + :preserve + git checkout #{@merge_request.target_branch} + git fetch origin + %p + %strong Step 2. + Merge source branch into target branch and push changes to GitLab + %pre.dark + :preserve + git merge origin/#{@merge_request.source_branch} + git push origin #{@merge_request.target_branch} + + +:javascript + $(function(){ + var modal = $('#modal_merge_info').modal({modal: true, show:false}); + $('.how_to_merge_link').bind("click", function(){ + modal.show(); + }); + $('.modal-header .close').bind("click", function(){ + modal.hide(); + }) + }) diff --git a/app/views/projects/merge_requests/show/_mr_accept.html.haml b/app/views/projects/merge_requests/show/_mr_accept.html.haml index 01378d99c99..47db8cdc8d2 100644 --- a/app/views/projects/merge_requests/show/_mr_accept.html.haml +++ b/app/views/projects/merge_requests/show/_mr_accept.html.haml @@ -15,7 +15,7 @@ for instructions .accept_group = f.submit "Accept Merge Request", class: "btn btn-create accept_merge_request" - - unless @project.root_ref? @merge_request.source_branch + - unless @merge_request.disallow_source_branch_removal? .remove_branch_holder = label_tag :should_remove_source_branch, class: "checkbox" do = check_box_tag :should_remove_source_branch diff --git a/app/views/projects/merge_requests/show/_mr_title.html.haml b/app/views/projects/merge_requests/show/_mr_title.html.haml index 5a07258f91c..c36ac2ed55b 100644 --- a/app/views/projects/merge_requests/show/_mr_title.html.haml +++ b/app/views/projects/merge_requests/show/_mr_title.html.haml @@ -1,9 +1,16 @@ %h3.page-title = "Merge Request ##{@merge_request.id}:" - %span.label-branch= @merge_request.source_branch - → - %span.label-branch= @merge_request.target_branch + -if @merge_request.for_fork? + %span.label-project= @merge_request.source_project.path_with_namespace + %span.label-branch= @merge_request.source_branch + → + %span.label-project= @merge_request.target_project.path_with_namespace + %span.label-branch= @merge_request.target_branch + - else + %span.label-branch= @merge_request.source_branch + → + %span.label-branch= @merge_request.target_branch %span.pull-right - if can?(current_user, :modify_merge_request, @merge_request) @@ -19,7 +26,7 @@ = link_to 'Close', project_merge_request_path(@project, @merge_request, merge_request: {state_event: :close }), method: :put, class: "btn grouped btn-close", title: "Close merge request" - = link_to edit_project_merge_request_path(@project, @merge_request), class: "btn grouped" do + = link_to edit_project_merge_request_path(@project, @merge_request), class: "btn grouped", id:"edit_merge_request" do %i.icon-edit Edit diff --git a/app/views/projects/merge_requests/update_branches.js.haml b/app/views/projects/merge_requests/update_branches.js.haml new file mode 100644 index 00000000000..db25ae3b918 --- /dev/null +++ b/app/views/projects/merge_requests/update_branches.js.haml @@ -0,0 +1,5 @@ +:plain + $(".target_branch").html("#{escape_javascript(options_for_select(@target_branches))}"); + $(".target_branch").trigger("liszt:updated"); + $(".mr_target_commit").html(""); + $(".target_branch").trigger("change");
\ No newline at end of file diff --git a/app/views/projects/notes/_discussion.html.haml b/app/views/projects/notes/_discussion.html.haml index 14d81bbb5ce..8c79b5ad53e 100644 --- a/app/views/projects/notes/_discussion.html.haml +++ b/app/views/projects/notes/_discussion.html.haml @@ -23,7 +23,7 @@ discussion on this merge request diff - elsif note.for_commit? started a discussion on commit - #{link_to note.noteable.short_id, project_commit_path(@project, note.noteable)} + #{link_to note.noteable.short_id, project_commit_path(note.project, note.noteable)} = link_to_commit_diff_line_note(note) if note.for_diff_line? - else %cite.cgray started a discussion diff --git a/app/views/search/_result.html.haml b/app/views/search/_result.html.haml index 4e56eea084e..59c7738019b 100644 --- a/app/views/search/_result.html.haml +++ b/app/views/search/_result.html.haml @@ -22,11 +22,14 @@ - @merge_requests.each do |merge_request| %li merge request: - = link_to [merge_request.project, merge_request] do + = link_to [merge_request.target_project, merge_request] do %span ##{merge_request.id} %strong.term = truncate merge_request.title, length: 50 - %span.light (#{merge_request.project.name_with_namespace}) + - if merge_request.for_fork? + %span.light (#{merge_request.source_project.name_with_namespace}:#{merge_request.source_branch} → #{merge_request.target_project.name_with_namespace}:#{merge_request.target_branch}) + - else + %span.light (#{merge_request.source_branch} → #{merge_request.target_branch}) - @issues.each do |issue| %li issue: diff --git a/app/views/shared/_merge_requests.html.haml b/app/views/shared/_merge_requests.html.haml index 935a7a7f7c0..5276f4bae31 100644 --- a/app/views/shared/_merge_requests.html.haml +++ b/app/views/shared/_merge_requests.html.haml @@ -1,5 +1,5 @@ - if @merge_requests.any? - - @merge_requests.group_by(&:project).each do |group| + - @merge_requests.group_by(&:target_project).each do |group| .ui-box - project = group[0] .title diff --git a/config/routes.rb b/config/routes.rb index d303a57d304..4fe15b1feef 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -257,6 +257,7 @@ Gitlab::Application.routes.draw do collection do get :branch_from get :branch_to + get :update_branches end end diff --git a/db/fixtures/development/10_merge_requests.rb b/db/fixtures/development/10_merge_requests.rb index 0a8d67d4461..bf247adb416 100644 --- a/db/fixtures/development/10_merge_requests.rb +++ b/db/fixtures/development/10_merge_requests.rb @@ -23,7 +23,8 @@ Gitlab::Seeder.quiet do id: i, source_branch: branches.first, target_branch: branches.last, - project_id: project.id, + source_project_id: project.id, + target_project_id: project.id, author_id: user_id, assignee_id: user_id, milestone: project.milestones.sample, diff --git a/db/fixtures/test/001_repo.rb b/db/fixtures/test/001_repo.rb index 18fc37cde0c..281e3476df1 100644 --- a/db/fixtures/test/001_repo.rb +++ b/db/fixtures/test/001_repo.rb @@ -19,5 +19,18 @@ FileUtils.cd(REPO_PATH) do # Remove the copy FileUtils.rm(SEED_REPO) end +puts ' done.' +print "Creating seed satellite..." + +SATELLITE_PATH = Rails.root.join('tmp', 'satellite') +# Make directory +FileUtils.mkdir_p(SATELLITE_PATH) +# Clear any potential directory +FileUtils.rm_rf("#{SATELLITE_PATH}/gitlabhq") +# Chdir, clone from the seed +FileUtils.cd(SATELLITE_PATH) do + # Clone the satellite + `git clone --quiet #{REPO_PATH}/gitlabhq #{SATELLITE_PATH}/gitlabhq` +end puts ' done.' diff --git a/db/migrate/20130419190306_allow_merges_for_forks.rb b/db/migrate/20130419190306_allow_merges_for_forks.rb new file mode 100644 index 00000000000..13135cbafcf --- /dev/null +++ b/db/migrate/20130419190306_allow_merges_for_forks.rb @@ -0,0 +1,14 @@ +class AllowMergesForForks < ActiveRecord::Migration + + def self.up + add_column :merge_requests, :target_project_id, :integer, :null => false + MergeRequest.update_all("target_project_id = project_id") + rename_column :merge_requests, :project_id, :source_project_id + end + + def self.down + remove_column :merge_requests, :target_project_id + rename_column :merge_requests, :source_project_id,:project_id + end + +end diff --git a/db/schema.rb b/db/schema.rb index 4ada3d07263..10191c6923a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -84,9 +84,9 @@ ActiveRecord::Schema.define(:version => 20130624162710) do add_index "keys", ["user_id"], :name => "index_keys_on_user_id" create_table "merge_requests", :force => true do |t| - t.string "target_branch", :null => false - t.string "source_branch", :null => false - t.integer "project_id", :null => false + t.string "target_branch", :null => false + t.string "source_branch", :null => false + t.integer "source_project_id", :null => false t.integer "author_id" t.integer "assignee_id" t.string "title" @@ -97,14 +97,15 @@ ActiveRecord::Schema.define(:version => 20130624162710) do t.integer "milestone_id" t.string "state" t.string "merge_status" + t.integer "target_project_id", :null => false end add_index "merge_requests", ["assignee_id"], :name => "index_merge_requests_on_assignee_id" add_index "merge_requests", ["author_id"], :name => "index_merge_requests_on_author_id" add_index "merge_requests", ["created_at"], :name => "index_merge_requests_on_created_at" add_index "merge_requests", ["milestone_id"], :name => "index_merge_requests_on_milestone_id" - add_index "merge_requests", ["project_id"], :name => "index_merge_requests_on_project_id" add_index "merge_requests", ["source_branch"], :name => "index_merge_requests_on_source_branch" + add_index "merge_requests", ["source_project_id"], :name => "index_merge_requests_on_project_id" add_index "merge_requests", ["target_branch"], :name => "index_merge_requests_on_target_branch" add_index "merge_requests", ["title"], :name => "index_merge_requests_on_title" diff --git a/features/dashboard/dashboard.feature b/features/dashboard/dashboard.feature index 695148b5cdf..e249f392de7 100644 --- a/features/dashboard/dashboard.feature +++ b/features/dashboard/dashboard.feature @@ -16,6 +16,7 @@ Feature: Dashboard And I visit dashboard page Then I should see groups list + @javascript Scenario: I should see last push widget Then I should see last push widget And I click "Create Merge Request" link diff --git a/features/project/forked_merge_requests.feature b/features/project/forked_merge_requests.feature new file mode 100644 index 00000000000..245515c33ad --- /dev/null +++ b/features/project/forked_merge_requests.feature @@ -0,0 +1,50 @@ +Feature: Project Forked Merge Requests + Background: + Given I sign in as a user + And I am a member of project "Shop" + And I have a project forked off of "Shop" called "Forked Shop" + + @javascript + Scenario: I can visit the target projects commit for a forked merge request + Given I visit project "Forked Shop" merge requests page + And I click link "New Merge Request" + And I fill out a "Merge Request On Forked Project" merge request + And I follow the target commit link + Then I should see the commit under the forked from project + + @javascript + Scenario: I submit new unassigned merge request to a forked project + Given I visit project "Forked Shop" merge requests page + And I click link "New Merge Request" + And I fill out a "Merge Request On Forked Project" merge request + And I submit the merge request + Then I should see merge request "Merge Request On Forked Project" + + @javascript + Scenario: I should see a push widget for forked merge requests + Given project "Forked Shop" has push event + And I visit dashboard page + Then I should see last push widget + And I click "Create Merge Request on fork" link + Then I see prefilled new Merge Request page for the forked project + + @javascript + Scenario: I can edit a forked merge request + Given I visit project "Forked Shop" merge requests page + And I click link "New Merge Request" + And I fill out a "Merge Request On Forked Project" merge request + And I submit the merge request + And I should see merge request "Merge Request On Forked Project" + And I click link edit "Merge Request On Forked Project" + Then I see the edit page prefilled for "Merge Request On Forked Project" + And I update the merge request title + And I save the merge request + Then I should see the edited merge request + + @javascript + Scenario: I cannot submit an invalid merge request + Given I visit project "Forked Shop" merge requests page + And I click link "New Merge Request" + And I fill out an invalid "Merge Request On Forked Project" merge request + And I submit the merge request + Then I should see validation errors
\ No newline at end of file diff --git a/features/project/merge_requests.feature b/features/project/merge_requests.feature index 5b8becbb6c9..63f27c3acc3 100644 --- a/features/project/merge_requests.feature +++ b/features/project/merge_requests.feature @@ -29,6 +29,7 @@ Feature: Project Merge Requests And I click link "Close" Then I should see closed merge request "Bug NS-04" + @javascript Scenario: I submit new unassigned merge request Given I click link "New Merge Request" And I submit new merge request "Wiki Feature" diff --git a/features/steps/dashboard/dashboard.rb b/features/steps/dashboard/dashboard.rb index 329571ac6ef..596b5a78170 100644 --- a/features/steps/dashboard/dashboard.rb +++ b/features/steps/dashboard/dashboard.rb @@ -22,6 +22,7 @@ class Dashboard < Spinach::FeatureSteps Then 'I see prefilled new Merge Request page' do current_path.should == new_project_merge_request_path(@project) + find("#merge_request_target_project_id").value.should == @project.id.to_s find("#merge_request_source_branch").value.should == "new_design" find("#merge_request_target_branch").value.should == "master" find("#merge_request_title").value.should == "New Design" diff --git a/features/steps/dashboard/dashboard_event_filters.rb b/features/steps/dashboard/dashboard_event_filters.rb index 09da4e6756f..d0fe5c9b64b 100644 --- a/features/steps/dashboard/dashboard_event_filters.rb +++ b/features/steps/dashboard/dashboard_event_filters.rb @@ -61,7 +61,7 @@ class EventFilters < Spinach::FeatureSteps end And 'this project has merge request event' do - merge_request = create :merge_request, author: @user, project: @project + merge_request = create :merge_request, author: @user, source_project: @project, target_project: @project Event.create( project: @project, action: Event::MERGED, diff --git a/features/steps/dashboard/dashboard_merge_requests.rb b/features/steps/dashboard/dashboard_merge_requests.rb index 7cfa8a13ff8..6c1fa39f081 100644 --- a/features/steps/dashboard/dashboard_merge_requests.rb +++ b/features/steps/dashboard/dashboard_merge_requests.rb @@ -6,18 +6,24 @@ class DashboardMergeRequests < Spinach::FeatureSteps merge_requests = @user.merge_requests merge_requests.each do |mr| page.should have_content(mr.title[0..10]) - page.should have_content(mr.project.name) + page.should have_content(mr.target_project.name) + page.should have_content(mr.source_project.name) end end And 'I have authored merge requests' do - project1 = create :project - project2 = create :project + project1_source = create :project + project1_target= create :project + project2_source = create :project + project2_target = create :project - project1.team << [@user, :master] - project2.team << [@user, :master] - merge_request1 = create :merge_request, author: @user, project: project1 - merge_request2 = create :merge_request, author: @user, project: project2 + project1_source.team << [@user, :master] + project1_target.team << [@user, :master] + project2_source.team << [@user, :master] + project2_target.team << [@user, :master] + + merge_request1 = create :merge_request, author: @user, source_project: project1_source, target_project: project1_target + merge_request2 = create :merge_request, author: @user, source_project: project2_source, target_project: project2_target end end diff --git a/features/steps/group/group.rb b/features/steps/group/group.rb index a6b9aa606c6..6f18f6185f5 100644 --- a/features/steps/group/group.rb +++ b/features/steps/group/group.rb @@ -60,7 +60,8 @@ class Groups < Spinach::FeatureSteps Given 'project from group has merge requests assigned to me' do create :merge_request, - project: project, + source_project: project, + target_project: project, assignee: current_user, author: current_user end diff --git a/features/steps/project/project_fork.rb b/features/steps/project/project_fork.rb index 775af0c5c58..858c7d11b32 100644 --- a/features/steps/project/project_fork.rb +++ b/features/steps/project/project_fork.rb @@ -4,6 +4,8 @@ class ForkProject < Spinach::FeatureSteps include SharedProject step 'I click link "Fork"' do + page.should have_content "Shop" + page.should have_content "Fork" Gitlab::Shell.any_instance.stub(:fork_repository).and_return(true) click_link "Fork" end @@ -17,9 +19,13 @@ class ForkProject < Spinach::FeatureSteps step 'I should see the forked project page' do page.should have_content "Project was successfully forked." current_path.should include current_user.namespace.path + @forked_project = Project.find_by_namespace_id(current_user.namespace.path) end step 'I already have a project named "Shop" in my namespace' do + 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) end diff --git a/features/steps/project/project_forked_merge_requests.rb b/features/steps/project/project_forked_merge_requests.rb new file mode 100644 index 00000000000..d6cad1ba2ec --- /dev/null +++ b/features/steps/project/project_forked_merge_requests.rb @@ -0,0 +1,198 @@ +class ProjectForkedMergeRequests < Spinach::FeatureSteps + include SharedAuthentication + include SharedProject + include SharedNote + include SharedPaths + + Given 'I am a member of project "Shop"' do + @project = Project.find_by_name "Shop" + @project ||= create(:project_with_code, name: "Shop") + @project.team << [@user, :reporter] + end + + And 'I have a project forked off of "Shop" called "Forked Shop"' do + @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_link.forked_from_project = @project + forked_project_link.forked_to_project = @forked_project + @forked_project.team << [@forking_user , :master] + forked_project_link.save! + end + + Given 'I click link "New Merge Request"' do + click_link "New Merge Request" + end + + Then 'I should see merge request "Merge Request On Forked Project"' do + page.should have_content "Merge Request On Forked Project" + @project.merge_requests.size.should >= 1 + @merge_request = @project.merge_requests.last + current_path.should == project_merge_request_path(@project, @merge_request) + @merge_request.title.should == "Merge Request On Forked Project" + @merge_request.source_project.should == @forked_project + @merge_request.source_branch.should == "master" + @merge_request.target_branch.should == "stable" + page.should have_content @forked_project.path_with_namespace + page.should have_content @project.path_with_namespace + page.should have_content @merge_request.source_branch + page.should have_content @merge_request.target_branch + end + + And 'I fill out a "Merge Request On Forked Project" merge request' do + #The ordering here is a bit whacky on purpose: + #Select the target right away, to give update_branches time to run and clean up the target_branches + find(:select, "merge_request_target_project_id", {}).value.should == @forked_project.id.to_s + select @project.path_with_namespace, from: "merge_request_target_project_id" + + + fill_in "merge_request_title", with: "Merge Request On Forked Project" + find(:select, "merge_request_source_project_id", {}).value.should == @forked_project.id.to_s + + find(:select, "merge_request_target_project_id", {}).value.should == @project.id.to_s + + #Ensure the option exists in the select + find(:select, "merge_request_source_branch", {}).should have_content "master" + select "master", from: "merge_request_source_branch" + #Ensure the option is selected + find(:select, "merge_request_source_branch", {}).value.should have_content "master" + verify_commit_link(".mr_source_commit",@forked_project) + + + #This could fail if the javascript hasn't run yet, there is a timing issue here -- this is why we do the select at the top + #Ensure the option exists in the select + find(:select, "merge_request_target_branch", {}).should have_content "stable" + #We must give apparently lots of time for update branches to finish + + (find(:select, "merge_request_target_branch", {}).find(:option, "stable",{}).select_option).should be_true + #Ensure the option is selected + find(:select, "merge_request_target_branch", {}).value.should have_content "stable" + verify_commit_link(".mr_target_commit",@project) + end + + And 'I submit the merge request' do + click_button "Submit merge request" + end + + And 'I follow the target commit link' do + commit = @project.repository.commit + click_link commit.short_id(8) + end + + Then 'I should see the commit under the forked from project' do + commit = @project.repository.commit + page.should have_content(commit.message) + end + + And 'I click "Create Merge Request on fork" link' do + click_link "Create Merge Request on fork" + end + + Then 'I see prefilled new Merge Request page for the forked project' do + current_path.should == new_project_merge_request_path(@forked_project) + find("#merge_request_source_project_id").value.should == @forked_project.id.to_s + find("#merge_request_target_project_id").value.should == @project.id.to_s + find("#merge_request_source_branch").value.should have_content "new_design" + find("#merge_request_target_branch").value.should have_content "master" + find("#merge_request_title").value.should == "New Design" + verify_commit_link(".mr_target_commit",@project) + verify_commit_link(".mr_source_commit",@forked_project) + end + + And 'I update the merge request title' do + fill_in "merge_request_title", with: "An Edited Forked Merge Request" + end + + And 'I save the merge request' do + click_button "Save changes" + end + + Then 'I should see the edited merge request' do + page.should have_content "An Edited Forked Merge Request" + @project.merge_requests.size.should >= 1 + @merge_request = @project.merge_requests.last + current_path.should == project_merge_request_path(@project, @merge_request) + @merge_request.source_project.should == @forked_project + @merge_request.source_branch.should == "master" + @merge_request.target_branch.should == "stable" + page.should have_content @forked_project.path_with_namespace + page.should have_content @project.path_with_namespace + page.should have_content @merge_request.source_branch + page.should have_content @merge_request.target_branch + end + + Then 'I should see last push widget' do + page.should have_content "You pushed to new_design" + page.should have_link "Create Merge Request" + end + + Given 'project "Forked Shop" has push event' do + @forked_project = Project.find_by_name("Forked Shop") + + data = { + before: "0000000000000000000000000000000000000000", + after: "0220c11b9a3e6c69dc8fd35321254ca9a7b98f7e", + ref: "refs/heads/new_design", + user_id: @user.id, + user_name: @user.name, + repository: { + name: @forked_project.name, + url: "localhost/rubinius", + description: "", + homepage: "localhost/rubinius", + private: true + } + } + + @event = Event.create( + project: @forked_project, + action: Event::PUSHED, + data: data, + author_id: @user.id + ) + end + + + Then 'I click link edit "Merge Request On Forked Project"' do + find("#edit_merge_request").click + end + + Then 'I see the edit page prefilled for "Merge Request On Forked Project"' do + current_path.should == edit_project_merge_request_path(@project, @merge_request) + page.should have_content "Edit merge request #{@merge_request.id}" + find("#merge_request_title").value.should == "Merge Request On Forked Project" + find("#merge_request_source_project_id").value.should == @forked_project.id.to_s + find("#merge_request_target_project_id").value.should == @project.id.to_s + find("#merge_request_source_branch").value.should have_content "master" + verify_commit_link(".mr_source_commit",@forked_project) + find("#merge_request_target_branch").value.should have_content "stable" + verify_commit_link(".mr_target_commit",@project) + end + + And 'I fill out an invalid "Merge Request On Forked Project" merge request' do + #If this isn't filled in the rest of the validations won't be triggered + fill_in "merge_request_title", with: "Merge Request On Forked Project" + find(:select, "merge_request_source_project_id", {}).value.should == @forked_project.id.to_s + find(:select, "merge_request_target_project_id", {}).value.should == @forked_project.id.to_s + find(:select, "merge_request_source_branch", {}).value.should == "" + find(:select, "merge_request_target_branch", {}).value.should == "" + end + + Then 'I should see validation errors' do + page.should have_content "Source branch can't be blank" + page.should have_content "Target branch can't be blank" + page.should have_content "Branch conflict You can not use same project/branch for source and target" + end + + def project + @project ||= Project.find_by_name!("Shop") + end + + #Verify a link is generated against the correct project + def verify_commit_link(container_div, container_project) + #This should force a wait for the javascript to execute + find(:div,container_div).should have_css ".browse_code_link_holder" + find(:div,container_div).find(".commit_short_id")['href'].should have_content "#{container_project.path_with_namespace}/commit" + end +end diff --git a/features/steps/project/project_merge_requests.rb b/features/steps/project/project_merge_requests.rb index ea434412bb2..7c70482deb5 100644 --- a/features/steps/project/project_merge_requests.rb +++ b/features/steps/project/project_merge_requests.rb @@ -56,30 +56,41 @@ class ProjectMergeRequests < Spinach::FeatureSteps end And 'I submit new merge request "Wiki Feature"' do - fill_in "merge_request_title", with: "Wiki Feature" - select "bootstrap", from: "merge_request_source_branch" - select "master", from: "merge_request_target_branch" + #this must come first, so that the target branch is set by the time the "select" for "notes_refactoring" is executed + select project.path_with_namespace, :from => "merge_request_target_project_id" + fill_in "merge_request_title", :with => "Wiki Feature" + select "master", :from => "merge_request_source_branch" + find(:select, "merge_request_target_project_id", {}).value.should == project.id.to_s + find(:select, "merge_request_source_project_id", {}).value.should == project.id.to_s + + #using "notes_refactoring" because "Bug NS-04" uses master/stable, this will fail merge_request validation if the branches are the same + find(:select, "merge_request_target_branch", {}).find(:option, "notes_refactoring", {}).value.should == "notes_refactoring" + select "notes_refactoring", :from => "merge_request_target_branch" + click_button "Submit merge request" end And 'project "Shop" have "Bug NS-04" open merge request' do create(:merge_request, title: "Bug NS-04", - project: project, + source_project: project, + target_project: project, author: project.users.first) end And 'project "Shop" have "Bug NS-05" open merge request with diffs inside' do create(:merge_request_with_diffs, title: "Bug NS-05", - project: project, + source_project: project, + target_project: project, author: project.users.first) end And 'project "Shop" have "Feature NS-03" closed merge request' do create(:closed_merge_request, title: "Feature NS-03", - project: project, + source_project: project, + target_project: project, author: project.users.first) end diff --git a/features/steps/shared/paths.rb b/features/steps/shared/paths.rb index eb91ed23737..c30eccce1c5 100644 --- a/features/steps/shared/paths.rb +++ b/features/steps/shared/paths.rb @@ -184,6 +184,10 @@ module SharedPaths visit project_path(project) end + step 'I visit project "Forked Shop" merge requests page' do + visit project_merge_requests_path(@forked_project) + end + step 'I visit edit project "Shop" page' do visit edit_project_path(project) end @@ -239,18 +243,22 @@ module SharedPaths step 'I visit merge request page "Bug NS-04"' do mr = MergeRequest.find_by_title("Bug NS-04") - visit project_merge_request_path(mr.project, mr) + visit project_merge_request_path(mr.target_project, mr) end step 'I visit merge request page "Bug NS-05"' do mr = MergeRequest.find_by_title("Bug NS-05") - visit project_merge_request_path(mr.project, mr) + visit project_merge_request_path(mr.target_project, mr) end step 'I visit project "Shop" merge requests page' do visit project_merge_requests_path(project) end + step 'I visit forked project "Shop" merge requests page' do + visit project_merge_requests_path(project) + end + step 'I visit project "Shop" milestones page' do visit project_milestones_path(project) end diff --git a/features/support/env.rb b/features/support/env.rb index 1693a588993..5dce3402083 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -35,8 +35,7 @@ Capybara.ignore_hidden_elements = false DatabaseCleaner.strategy = :truncation Spinach.hooks.before_scenario do - TestEnv.init(mailer: false) - + TestEnv.setup_stubs DatabaseCleaner.start end @@ -45,6 +44,7 @@ Spinach.hooks.after_scenario do end Spinach.hooks.before_run do + TestEnv.init(mailer: false, init_repos: true, repos: false) RSpec::Mocks::setup self include FactoryGirl::Syntax::Methods diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 861a4f4d159..d690f1d07e7 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -14,6 +14,14 @@ module API end not_found! end + + def not_fork?(target_project_id, user_project) + target_project_id.nil? || target_project_id == user_project.id.to_s + end + + def target_matches_fork(target_project_id,user_project) + user_project.forked? && user_project.forked_from_project.id.to_s == target_project_id + end end # List merge requests @@ -51,9 +59,10 @@ module API # # Parameters: # - # id (required) - The ID of a project + # id (required) - The ID of a project - this will be the source of the merge request # source_branch (required) - The source branch # target_branch (required) - The target branch + # target_project - The target project of the merge request defaults to the :id of the project # assignee_id - Assignee user ID # title (required) - Title of MR # @@ -63,10 +72,20 @@ module API post ":id/merge_requests" do authorize! :write_merge_request, user_project required_attributes! [:source_branch, :target_branch, :title] - - attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title] + attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :target_project_id] merge_request = user_project.merge_requests.new(attrs) merge_request.author = current_user + merge_request.source_project = user_project + target_project_id = attrs[:target_project_id] + if not_fork?(target_project_id, user_project) + merge_request.target_project = user_project + else + if target_matches_fork(target_project_id,user_project) + merge_request.target_project = Project.find_by_id(attrs[:target_project_id]) + else + render_api_error!('(Bad Request) Specified target project that is not the source project, or the source fork of the project.', 400) + end + end if merge_request.save merge_request.reload_code diff --git a/lib/gitlab/satellite/action.rb b/lib/gitlab/satellite/action.rb index 63303ca3de1..5ea6f956765 100644 --- a/lib/gitlab/satellite/action.rb +++ b/lib/gitlab/satellite/action.rb @@ -25,25 +25,31 @@ module Gitlab end end rescue Errno::ENOMEM => ex - Gitlab::GitLogger.error(ex.message) - return false + return handle_exception(ex) rescue Grit::Git::GitTimeout => ex - Gitlab::GitLogger.error(ex.message) - return false + return handle_exception(ex) ensure Gitlab::ShellEnv.reset_env end - # * Clears the satellite - # * Updates the satellite from Gitolite + # * Recreates the satellite # * Sets up Git variables for the user # # Note: use this within #in_locked_and_timed_satellite def prepare_satellite!(repo) project.satellite.clear_and_update! - repo.git.config({}, "user.name", user.name) - repo.git.config({}, "user.email", user.email) + repo.config['user.name'] = user.name + repo.config['user.email'] = user.email + end + + def default_options(options = {}) + {raise: true, timeout: true}.merge(options) + end + + def handle_exception(exception) + Gitlab::GitLogger.error(exception.message) + false end end end diff --git a/lib/gitlab/satellite/merge_action.rb b/lib/gitlab/satellite/merge_action.rb index 832db6621c4..6f402e80a63 100644 --- a/lib/gitlab/satellite/merge_action.rb +++ b/lib/gitlab/satellite/merge_action.rb @@ -5,48 +5,118 @@ module Gitlab attr_accessor :merge_request def initialize(user, merge_request) - super user, merge_request.project + super user, merge_request.target_project @merge_request = merge_request end # Checks if a merge request can be executed without user interaction def can_be_merged? in_locked_and_timed_satellite do |merge_repo| + prepare_satellite!(merge_repo) merge_in_satellite!(merge_repo) end end # Merges the source branch into the target branch in the satellite and - # pushes it back to Gitolite. - # It also removes the source branch if requested in the merge request. + # pushes it back to the repository. + # It also removes the source branch if requested in the merge request (and this is permitted by the merge request). # # Returns false if the merge produced conflicts - # Returns false if pushing from the satellite to Gitolite failed or was rejected + # Returns false if pushing from the satellite to the repository failed or was rejected # Returns true otherwise def merge! in_locked_and_timed_satellite do |merge_repo| + prepare_satellite!(merge_repo) if merge_in_satellite!(merge_repo) # push merge back to Gitolite # will raise CommandFailed when push fails - merge_repo.git.push({raise: true, timeout: true}, :origin, merge_request.target_branch) - + merge_repo.git.push(default_options, :origin, merge_request.target_branch) # remove source branch if merge_request.should_remove_source_branch && !project.root_ref?(merge_request.source_branch) # will raise CommandFailed when push fails - merge_repo.git.push({raise: true, timeout: true}, :origin, ":#{merge_request.source_branch}") + merge_repo.git.push(default_options, :origin, ":#{merge_request.source_branch}") end - # merge, push and branch removal successful true end end rescue Grit::Git::CommandFailed => ex - Gitlab::GitLogger.error(ex.message) - false + handle_exception(ex) end - private + # Get a raw diff of the source to the target + def diff_in_satellite + in_locked_and_timed_satellite do |merge_repo| + prepare_satellite!(merge_repo) + + update_satellite_source_and_target!(merge_repo) + if merge_request.for_fork? + diff = merge_repo.git.native(:diff, default_options, "origin/#{merge_request.target_branch}", "source/#{merge_request.source_branch}") + else + diff = merge_repo.git.native(:diff, default_options, "#{merge_request.target_branch}", "#{merge_request.source_branch}") + + end + return diff + end + rescue Grit::Git::CommandFailed => ex + handle_exception(ex) + end + + # Only show what is new in the source branch compared to the target branch, not the other way around. + # The line below with merge_base is equivalent to diff with three dots (git diff branch1...branch2) + # From the git documentation: "git diff A...B" is equivalent to "git diff $(git-merge-base A B) B" + def diffs_between_satellite + in_locked_and_timed_satellite do |merge_repo| + prepare_satellite!(merge_repo) + update_satellite_source_and_target!(merge_repo) + if merge_request.for_fork? + common_commit = merge_repo.git.native(:merge_base, default_options, ["origin/#{merge_request.target_branch}", "source/#{merge_request.source_branch}"]).strip + #this method doesn't take default options + diffs = merge_repo.diff(common_commit, "source/#{merge_request.source_branch}") + else + raise "Attempt to determine diffs between for a non forked merge request in satellite MergeRequest.id:[#{merge_request.id}]" + end + diffs = diffs.map { |diff| Gitlab::Git::Diff.new(diff) } + return diffs + end + rescue Grit::Git::CommandFailed => ex + handle_exception(ex) + end + + # Get commit as an email patch + def format_patch + in_locked_and_timed_satellite do |merge_repo| + prepare_satellite!(merge_repo) + update_satellite_source_and_target!(merge_repo) + if (merge_request.for_fork?) + patch = merge_repo.git.format_patch(default_options({stdout: true}), "origin/#{merge_request.target_branch}...source/#{merge_request.source_branch}") + else + patch = merge_repo.git.format_patch(default_options({stdout: true}), "#{merge_request.target_branch}...#{merge_request.source_branch}") + end + return patch + end + rescue Grit::Git::CommandFailed => ex + handle_exception(ex) + end + + # Retrieve an array of commits between the source and the target + def commits_between + in_locked_and_timed_satellite do |merge_repo| + prepare_satellite!(merge_repo) + update_satellite_source_and_target!(merge_repo) + if (merge_request.for_fork?) + commits = merge_repo.commits_between("origin/#{merge_request.target_branch}", "source/#{merge_request.source_branch}") + else + raise "Attempt to determine commits between for a non forked merge request in satellite MergeRequest.id:[#{merge_request.id}]" + end + commits = commits.map { |commit| Gitlab::Git::Commit.new(commit, nil) } + return commits + end + rescue Grit::Git::CommandFailed => ex + handle_exception(ex) + end + private # Merges the source_branch into the target_branch in the satellite. # # Note: it will clear out the satellite before doing anything @@ -54,18 +124,35 @@ module Gitlab # Returns false if the merge produced conflicts # Returns true otherwise def merge_in_satellite!(repo) - prepare_satellite!(repo) - - # create target branch in satellite at the corresponding commit from Gitolite - repo.git.checkout({raise: true, timeout: true, b: true}, merge_request.target_branch, "origin/#{merge_request.target_branch}") + update_satellite_source_and_target!(repo) - # merge the source branch from Gitolite into the satellite + # merge the source branch into the satellite # will raise CommandFailed when merge fails - repo.git.pull({raise: true, timeout: true, no_ff: true}, :origin, merge_request.source_branch) + if merge_request.for_fork? + repo.git.pull(default_options({no_ff: true}), 'source', merge_request.source_branch) + else + repo.git.pull(default_options({no_ff: true}), 'origin', merge_request.source_branch) + end rescue Grit::Git::CommandFailed => ex - Gitlab::GitLogger.error(ex.message) - false + handle_exception(ex) end + + # Assumes a satellite exists that is a fresh clone of the projects repo, prepares satellite for merges, diffs etc + def update_satellite_source_and_target!(repo) + if merge_request.for_fork? + repo.remote_add('source', merge_request.source_project.repository.path_to_repo) + repo.remote_fetch('source') + repo.git.checkout(default_options({b: true}), merge_request.target_branch, "origin/#{merge_request.target_branch}") + else + # We can't trust the input here being branch names, we can't always check it out because it could be a relative ref i.e. HEAD~3 + # we could actually remove the if true, because it should never ever happen (as long as the satellite has been prepared) + repo.git.checkout(default_options, "#{merge_request.source_branch}") + repo.git.checkout(default_options, "#{merge_request.target_branch}") + end + rescue Grit::Git::CommandFailed => ex + handle_exception(ex) + end + end end end diff --git a/lib/gitlab/satellite/satellite.rb b/lib/gitlab/satellite/satellite.rb index 69f8551661b..6cb7814fae5 100644 --- a/lib/gitlab/satellite/satellite.rb +++ b/lib/gitlab/satellite/satellite.rb @@ -1,5 +1,5 @@ module Gitlab - class SatelliteNotExistError < StandardError; end + class SatelliteNotExistError < StandardError; end module Satellite class Satellite @@ -24,8 +24,11 @@ module Gitlab def clear_and_update! raise_no_satellite unless exists? + File.exists? path + @repo = nil clear_working_dir! delete_heads! + remove_remotes! update_from_source! end @@ -55,10 +58,11 @@ module Gitlab raise_no_satellite unless exists? File.open(lock_file, "w+") do |f| - f.flock(File::LOCK_EX) - - Dir.chdir(path) do - return yield + begin + f.flock File::LOCK_EX + Dir.chdir(path) { return yield } + ensure + f.flock File::LOCK_UN end end end @@ -100,20 +104,34 @@ module Gitlab if heads.include? PARKING_BRANCH repo.git.checkout({}, PARKING_BRANCH) else - repo.git.checkout({b: true}, PARKING_BRANCH) + repo.git.checkout(default_options({b: true}), PARKING_BRANCH) end # remove the parking branch from the list of heads ... heads.delete(PARKING_BRANCH) # ... and delete all others - heads.each { |head| repo.git.branch({D: true}, head) } + heads.each { |head| repo.git.branch(default_options({D: true}), head) } + end + + # Deletes all remotes except origin + # + # This ensures we have no remote name clashes or issues updating branches when + # working with the satellite. + def remove_remotes! + remotes = repo.git.remote.split(' ') + remotes.delete('origin') + remotes.each { |name| repo.git.remote(default_options,'rm', name)} end # Updates the satellite from Gitolite # # Note: this will only update remote branches (i.e. origin/*) def update_from_source! - repo.git.fetch({timeout: true}, :origin) + repo.git.fetch(default_options, :origin) + end + + def default_options(options = {}) + {raise: true, timeout: true}.merge(options) end # Create directory for storing diff --git a/spec/contexts/filter_context_spec.rb b/spec/contexts/filter_context_spec.rb new file mode 100644 index 00000000000..0ecdb7416e5 --- /dev/null +++ b/spec/contexts/filter_context_spec.rb @@ -0,0 +1,57 @@ +require 'spec_helper' + +describe FilterContext do + + let(:user) { create :user } + let(:user2) { create :user } + let(:project1) { create(:project, creator_id: user.id) } + let(:project2) { create(:project, creator_id: user.id) } + let(:merge_request1) { create(:merge_request, author_id: user.id, source_project: project1, target_project: project2) } + let(:merge_request2) { create(:merge_request, author_id: user.id, source_project: project2, target_project: project1) } + let(:merge_request3) { create(:merge_request, author_id: user.id, source_project: project2, target_project: project2) } + let(:merge_request4) { create(:merge_request, author_id: user2.id, source_project: project2, target_project: project2, target_branch:"notes_refactoring") } + let(:issue1) { create(:issue, assignee_id: user.id, project: project1) } + let(:issue2) { create(:issue, assignee_id: user.id, project: project2) } + let(:issue3) { create(:issue, assignee_id: user2.id, project: project2) } + + describe 'merge requests' do + before :each do + merge_request1 + merge_request2 + merge_request3 + merge_request4 + end + it 'should by default filter properly' do + merge_requests = user.cared_merge_requests + params ={} + merge_requests = FilterContext.new(merge_requests, params).execute + merge_requests.size.should == 3 + end + it 'should apply blocks passed in on creation to the filters' do + merge_requests = user.cared_merge_requests + params = {:project_id => project1.id} + merge_requests = FilterContext.new(merge_requests, params).execute + merge_requests.size.should == 2 + end + end + + describe 'issues' do + before :each do + issue1 + issue2 + issue3 + end + it 'should by default filter projects properly' do + issues = user.assigned_issues + params = {} + issues = FilterContext.new(issues, params).execute + issues.size.should == 2 + end + it 'should apply blocks passed in on creation to the filters' do + issues = user.assigned_issues + params = {:project_id => project1.id} + issues = FilterContext.new(issues, params).execute + issues.size.should == 1 + end + end +end
\ No newline at end of file diff --git a/spec/controllers/commit_controller_spec.rb b/spec/controllers/commit_controller_spec.rb index 87c54143a05..8e98b0a3ce3 100644 --- a/spec/controllers/commit_controller_spec.rb +++ b/spec/controllers/commit_controller_spec.rb @@ -2,12 +2,11 @@ require 'spec_helper' describe Projects::CommitController do let(:project) { create(:project_with_code) } - let(:user) { create(:user) } - let(:commit) { project.repository.last_commit_for("master") } + let(:user) { create(:user) } + let(:commit) { project.repository.last_commit_for("master") } before do sign_in(user) - project.team << [user, :master] end diff --git a/spec/controllers/commits_controller_spec.rb b/spec/controllers/commits_controller_spec.rb index c9931a19622..facae17a0ad 100644 --- a/spec/controllers/commits_controller_spec.rb +++ b/spec/controllers/commits_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Projects::CommitsController do let(:project) { create(:project_with_code) } - let(:user) { create(:user) } + let(:user) { create(:user) } before do sign_in(user) diff --git a/spec/controllers/merge_requests_controller_spec.rb b/spec/controllers/merge_requests_controller_spec.rb index 51ba6ca5945..d344e3eb14c 100644 --- a/spec/controllers/merge_requests_controller_spec.rb +++ b/spec/controllers/merge_requests_controller_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Projects::MergeRequestsController do let(:project) { create(:project_with_code) } let(:user) { create(:user) } - let(:merge_request) { create(:merge_request_with_diffs, project: project, target_branch: "bcf03b5d~3", source_branch: "bcf03b5d") } + let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project, target_branch: "bcf03b5d~3", source_branch: "bcf03b5d") } before do sign_in(user) @@ -28,7 +28,7 @@ describe Projects::MergeRequestsController do it "should render it" do get :show, project_id: project.code, id: merge_request.id, format: format - expect(response.body).to eq(merge_request.send(:"to_#{format}")) + expect(response.body).to eq((merge_request.send(:"to_#{format}",user)).to_s) end it "should not escape Html" do diff --git a/spec/factories.rb b/spec/factories.rb index 793bd2434e8..e657321c294 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -29,8 +29,19 @@ FactoryGirl.define do sequence(:name) { |n| "project#{n}" } path { name.downcase.gsub(/\s/, '_') } 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" } @@ -39,11 +50,20 @@ FactoryGirl.define do factory :project_with_code, parent: :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| - repos_path = Rails.root.join('tmp', 'test-git-base-path') - seed_repo = Rails.root.join('tmp', 'repositories', 'gitlabhq') - target_repo = File.join(repos_path, project.path_with_namespace + '.git') - system("ln -s #{seed_repo} #{target_repo}") + TestEnv.clear_repo_dir(project.namespace, project.path) + TestEnv.create_repo(project.namespace, project.path) end end @@ -86,7 +106,8 @@ FactoryGirl.define do factory :merge_request do title author - project factory: :project_with_code + source_project factory: :source_project_with_code + target_project factory: :target_project_with_code source_branch "master" target_branch "stable" @@ -96,13 +117,13 @@ FactoryGirl.define do source_branch "stable" # pretend bcf03b5d st_commits do [ - project.repository.commit('bcf03b5d').to_hash, - project.repository.commit('bcf03b5d~1').to_hash, - project.repository.commit('bcf03b5d~2').to_hash + 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 - project.repo.diff("bcf03b5d~3", "bcf03b5d") + source_project.repo.diff("bcf03b5d~3", "bcf03b5d") end end @@ -133,7 +154,7 @@ FactoryGirl.define do trait :on_commit do project factory: :project_with_code - commit_id "bcf03b5de6c33f3869ef70d68cf06e679d1d7f9a" + commit_id "bcf03b5de6c33f3869ef70d68cf06e679d1d7f9a" noteable_type "Commit" end @@ -143,12 +164,12 @@ FactoryGirl.define do trait :on_merge_request do project factory: :project_with_code - noteable_id 1 + noteable_id 1 noteable_type "MergeRequest" end trait :on_issue do - noteable_id 1 + noteable_id 1 noteable_type "Issue" end diff --git a/spec/factories_spec.rb b/spec/factories_spec.rb index 8360477d8fe..66bef0761c7 100644 --- a/spec/factories_spec.rb +++ b/spec/factories_spec.rb @@ -1,8 +1,8 @@ require 'spec_helper' INVALID_FACTORIES = [ - :key_with_a_space_in_the_middle, - :invalid_key, + :key_with_a_space_in_the_middle, + :invalid_key, ] FactoryGirl.factories.map(&:name).each do |factory_name| diff --git a/spec/features/gitlab_flavored_markdown_spec.rb b/spec/features/gitlab_flavored_markdown_spec.rb index e67df7c1fb0..349d68399fc 100644 --- a/spec/features/gitlab_flavored_markdown_spec.rb +++ b/spec/features/gitlab_flavored_markdown_spec.rb @@ -3,11 +3,11 @@ require 'spec_helper' describe "GitLab Flavored Markdown" do let(:project) { create(:project_with_code) } let(:issue) { create(:issue, project: project) } - let(:merge_request) { create(:merge_request, project: project) } + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } let(:fred) do - u = create(:user, name: "fred") - project.team << [u, :master] - u + u = create(:user, name: "fred") + project.team << [u, :master] + u end before do @@ -83,9 +83,7 @@ describe "GitLab Flavored Markdown" do describe "for merge requests" do before do - @merge_request = create(:merge_request, - project: project, - title: "fix ##{issue.id}") + @merge_request = create(:merge_request, source_project: project, target_project: project, title: "fix ##{issue.id}") end it "should render title in merge_requests#index" do diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb index 4aa8937926c..16bdaf0f83c 100644 --- a/spec/features/notes_on_merge_requests_spec.rb +++ b/spec/features/notes_on_merge_requests_spec.rb @@ -2,8 +2,8 @@ require 'spec_helper' describe "On a merge request", js: true do let!(:project) { create(:project_with_code) } - let!(:merge_request) { create(:merge_request, project: project) } - let!(:note) { create(:note_on_merge_request_with_attachment, project: project) } + let!(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + let!(:note) { create(:note_on_merge_request_with_attachment, project: project) } before do login_as :user @@ -62,7 +62,7 @@ describe "On a merge request", js: true do it 'should be added and form reset' do should have_content("This is awsome!") - within(".js-main-target-form") { should have_no_field("note[note]", with: "This is awesome!") } + within(".js-main-target-form") { should have_no_field("note[note]", with: "This is awesome!") } within(".js-main-target-form") { should have_css(".js-note-preview", visible: false) } within(".js-main-target-form") { should have_css(".js-note-text", visible: true) } end @@ -135,8 +135,8 @@ describe "On a merge request", js: true do end describe "On a merge request diff", js: true, focus: true do - let!(:project) { create(:project_with_code) } - let!(:merge_request) { create(:merge_request_with_diffs, project: project) } + let!(:project) { create(:source_project_with_code) } + let!(:merge_request) { create(:merge_request_with_diffs, source_project: project, target_project: project) } before do login_as :user @@ -144,6 +144,7 @@ describe "On a merge request diff", js: true, focus: true do visit diffs_project_merge_request_path(project, merge_request) end + subject { page } describe "when adding a note" do @@ -183,6 +184,9 @@ 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!(: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="342e16cbbd482ac2047dc679b2749d248cc1428f_18_17"]').click @@ -205,13 +209,13 @@ describe "On a merge request diff", js: true, focus: true do # TODO: fix #it 'should check if previews were rendered separately' do - #within("tr[id='4735dfc552ad7bf15ca468adc3cad9d05b624490_185_185'] + .js-temp-notes-holder") do - #should have_css(".js-note-preview", text: "One comment on line 185") - #end + #within("tr[id='4735dfc552ad7bf15ca468adc3cad9d05b624490_185_185'] + .js-temp-notes-holder") do + #should have_css(".js-note-preview", text: "One comment on line 185") + #end - #within("tr[id='342e16cbbd482ac2047dc679b2749d248cc1428f_18_17'] + .js-temp-notes-holder") do - #should have_css(".js-note-preview", text: "Another comment on line 17") - #end + #within("tr[id='342e16cbbd482ac2047dc679b2749d248cc1428f_18_17'] + .js-temp-notes-holder") do + #should have_css(".js-note-preview", text: "Another comment on line 17") + #end #end end @@ -238,39 +242,38 @@ describe "On a merge request diff", js: true, focus: true do # TODO: fix #it "should remove last note of a discussion" do - #within("tr[id='342e16cbbd482ac2047dc679b2749d248cc1428f_18_17'] + .notes-holder") do - #find(".js-note-delete").click - #end - - #should_not have_css(".note_holder") + # within("tr[id='342e16cbbd482ac2047dc679b2749d248cc1428f_18_17'] + .notes-holder") do + # find(".js-note-delete").click + # end + # should_not have_css(".note_holder") #end end end # TODO: fix #describe "when replying to a note" do - #before do - ## create first note - #find('a[data-line-code="4735dfc552ad7bf15ca468adc3cad9d05b624490_184_184"]').click + #before do + ## create first note + # find('a[data-line-code="4735dfc552ad7bf15ca468adc3cad9d05b624490_184_184"]').click - #within(".js-temp-notes-holder") do - #fill_in "note[note]", with: "One comment on line 184" - #click_button("Add Comment") - #end + # within(".js-temp-notes-holder") do + # fill_in "note[note]", with: "One comment on line 184" + # click_button("Add Comment") + #end - #within(".js-temp-notes-holder") do - #find(".js-discussion-reply-button").click - #fill_in "note[note]", with: "An additional comment in reply" - #click_button("Add Comment") - #end - #end - - #it 'should be inserted and form removed from reply' do - #should have_content("An additional comment in reply") - #within(".notes_holder") { should have_css(".note", count: 2) } - #within(".notes_holder") { should have_no_css("form") } - #within(".notes_holder") { should have_link("Reply") } - #end + # within(".js-temp-notes-holder") do + # find(".js-discussion-reply-button").click + # fill_in "note[note]", with: "An additional comment in reply" + # click_button("Add Comment") + # end + #end + + #it 'should be inserted and form removed from reply' do + # should have_content("An additional comment in reply") + # within(".notes_holder") { should have_css(".note", count: 2) } + # within(".notes_holder") { should have_no_css("form") } + # within(".notes_holder") { should have_link("Reply") } + # end #end end diff --git a/spec/features/profile_spec.rb b/spec/features/profile_spec.rb index d46882d4e42..7fa474d0ea1 100644 --- a/spec/features/profile_spec.rb +++ b/spec/features/profile_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe "Profile account page" do before(:each) { enable_observers } + after(:each) {disable_observers} let(:user) { create(:user) } before do diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb index f0a1f75e1e0..9d5f9d5a2e2 100644 --- a/spec/features/projects_spec.rb +++ b/spec/features/projects_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe "Projects" do before(:each) { enable_observers } + after(:each) {disable_observers} before { login_as :user } describe "DELETE /projects/:id" do diff --git a/spec/features/security/project_access_spec.rb b/spec/features/security/project_access_spec.rb index 6bc04726a9d..2071a014be6 100644 --- a/spec/features/security/project_access_spec.rb +++ b/spec/features/security/project_access_spec.rb @@ -14,10 +14,10 @@ describe "Application access" do end describe "Project" do - let(:project) { create(:project_with_code) } + let(:project) { create(:project_with_code) } - let(:master) { create(:user) } - let(:guest) { create(:user) } + let(:master) { create(:user) } + let(:guest) { create(:user) } let(:reporter) { create(:user) } before do @@ -108,7 +108,7 @@ describe "Application access" do describe "GET /project_code/blob" do before do commit = project.repository.commit - path = commit.tree.contents.select { |i| i.is_a?(Grit::Blob)}.first.name + path = commit.tree.contents.select { |i| i.is_a?(Grit::Blob) }.first.name @blob_path = project_blob_path(project, File.join(commit.id, path)) end @@ -232,13 +232,13 @@ describe "Application access" do describe "PublicProject" do - let(:project) { create(:project_with_code) } + let(:project) { create(:project_with_code) } - let(:master) { create(:user) } - let(:guest) { create(:user) } + let(:master) { create(:user) } + let(:guest) { create(:user) } let(:reporter) { create(:user) } - let(:admin) { create(:user) } + let(:admin) { create(:user) } before do # public project @@ -339,7 +339,7 @@ describe "Application access" do describe "GET /project_code/blob" do before do commit = project.repository.commit - path = commit.tree.contents.select { |i| i.is_a?(Grit::Blob)}.first.name + path = commit.tree.contents.select { |i| i.is_a?(Grit::Blob) }.first.name @blob_path = project_blob_path(project, File.join(commit.id, path)) end diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb index a52606bb536..2dcc61e9560 100644 --- a/spec/helpers/gitlab_markdown_helper_spec.rb +++ b/spec/helpers/gitlab_markdown_helper_spec.rb @@ -9,7 +9,7 @@ describe GitlabMarkdownHelper do let(:user) { create(:user, username: 'gfm') } let(:commit) { project.repository.commit } let(:issue) { create(:issue, project: project) } - let(:merge_request) { create(:merge_request, project: project) } + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } let(:snippet) { create(:project_snippet, project: project) } let(:member) { project.users_projects.where(user_id: user).first } diff --git a/spec/lib/gitlab/satellite/action_spec.rb b/spec/lib/gitlab/satellite/action_spec.rb new file mode 100644 index 00000000000..5e0a825c3c3 --- /dev/null +++ b/spec/lib/gitlab/satellite/action_spec.rb @@ -0,0 +1,116 @@ +require 'spec_helper' + +describe 'Gitlab::Satellite::Action' do + let(:project) { create(:project_with_code) } + let(:user) { create(:user) } + + describe '#prepare_satellite!' do + + it 'create a repository with a parking branch and one remote: origin' do + repo = project.satellite.repo + + #now lets dirty it up + + starting_remote_count = repo.git.list_remotes.size + starting_remote_count.should >= 1 + #kind of hookey way to add a second remote + origin_uri = repo.git.remote({v: true}).split(" ")[1] + begin + repo.git.remote({raise: true}, 'add', 'another-remote', origin_uri) + repo.git.branch({raise: true}, 'a-new-branch') + + repo.heads.size.should > (starting_remote_count) + repo.git.remote().split(" ").size.should > (starting_remote_count) + rescue + end + + repo.git.config({}, "user.name", "#{user.name} -- foo") + repo.git.config({}, "user.email", "#{user.email} -- foo") + repo.config['user.name'].should =="#{user.name} -- foo" + repo.config['user.email'].should =="#{user.email} -- foo" + + + #These must happen in the context of the satellite directory... + satellite_action = Gitlab::Satellite::Action.new(user, project) + project.satellite.lock { + #Now clean it up, use send to get around prepare_satellite! being protected + satellite_action.send(:prepare_satellite!, repo) + } + + #verify it's clean + heads = repo.heads.map(&:name) + heads.size.should == 1 + heads.include?(Gitlab::Satellite::Satellite::PARKING_BRANCH).should == true + remotes = repo.git.remote().split(' ') + remotes.size.should == 1 + remotes.include?('origin').should == true + repo.config['user.name'].should ==user.name + repo.config['user.email'].should ==user.email + end + end + + describe '#in_locked_and_timed_satellite' do + + it 'should make use of a lockfile' do + repo = project.satellite.repo + called = false + + #set assumptions + File.rm(project.satellite.lock_file) unless !File.exists? project.satellite.lock_file + + File.exists?(project.satellite.lock_file).should be_false + + satellite_action = Gitlab::Satellite::Action.new(user, project) + satellite_action.send(:in_locked_and_timed_satellite) do |sat_repo| + repo.should == sat_repo + (File.exists? project.satellite.lock_file).should be_true + called = true + end + + called.should be_true + + end + + it 'should be able to use the satellite after locking' do + repo = project.satellite.repo + called = false + + # Set base assumptions + if File.exists? project.satellite.lock_file + FileLockStatusChecker.new(project.satellite.lock_file).flocked?.should be_false + end + + satellite_action = Gitlab::Satellite::Action.new(user, project) + satellite_action.send(:in_locked_and_timed_satellite) do |sat_repo| + called = true + repo.should == sat_repo + (File.exists? project.satellite.lock_file).should be_true + FileLockStatusChecker.new(project.satellite.lock_file).flocked?.should be_true + end + + called.should be_true + FileLockStatusChecker.new(project.satellite.lock_file).flocked?.should be_false + + end + + class FileLockStatusChecker < File + def flocked? &block + status = flock LOCK_EX|LOCK_NB + case status + when false + return true + when 0 + begin + block ? block.call : false + ensure + flock LOCK_UN + end + else + raise SystemCallError, status + end + end + end + + end +end + diff --git a/spec/lib/gitlab/satellite/merge_action_spec.rb b/spec/lib/gitlab/satellite/merge_action_spec.rb new file mode 100644 index 00000000000..98e475e715d --- /dev/null +++ b/spec/lib/gitlab/satellite/merge_action_spec.rb @@ -0,0 +1,135 @@ +require 'spec_helper' + +describe 'Gitlab::Satellite::MergeAction' do + before(:each) do +# TestEnv.init(mailer: false, init_repos: true, repos: true) + @master = ['master', 'bcf03b5de6c33f3869ef70d68cf06e679d1d7f9a'] + @one_after_stable = ['stable', '6ea87c47f0f8a24ae031c3fff17bc913889ecd00'] #this commit sha is one after stable + @wiki_branch = ['wiki', '635d3e09b72232b6e92a38de6cc184147e5bcb41'] #this is the commit sha where the wiki branch goes off from master + @conflicting_metior = ['metior', '313d96e42b313a0af5ab50fa233bf43e27118b3f'] #this branch conflicts with the wiki branch + + #these commits are quite close together, itended to make string diffs/format patches small + @close_commit1 = ['2_3_notes_fix', '8470d70da67355c9c009e4401746b1d5410af2e3'] + @close_commit2 = ['scss_refactoring', 'f0f14c8eaba69ebddd766498a9d0b0e79becd633'] + end + + let(:project) { create(:project_with_code) } + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + let(:merge_request_fork) { create(:merge_request) } + describe '#commits_between' do + def verify_commits(commits, first_commit_sha, last_commit_sha) + commits.each { |commit| commit.class.should == Gitlab::Git::Commit } + commits.first.id.should == first_commit_sha + commits.last.id.should == last_commit_sha + end + + context 'on fork' do + it 'should get proper commits between' do + merge_request_fork.target_branch = @one_after_stable[0] + merge_request_fork.source_branch = @master[0] + commits = Gitlab::Satellite::MergeAction.new(merge_request_fork.author, merge_request_fork).commits_between + verify_commits(commits, @one_after_stable[1], @master[1]) + + merge_request_fork.target_branch = @wiki_branch[0] + merge_request_fork.source_branch = @master[0] + commits = Gitlab::Satellite::MergeAction.new(merge_request_fork.author, merge_request_fork).commits_between + verify_commits(commits, @wiki_branch[1], @master[1]) + end + end + + context 'between branches' do + it 'should raise exception -- not expected to be used by non forks' do + merge_request.target_branch = @one_after_stable[0] + merge_request.source_branch = @master[0] + expect {Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request).commits_between}.to raise_error + + merge_request.target_branch = @wiki_branch[0] + merge_request.source_branch = @master[0] + expect {Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request).commits_between}.to raise_error + end + end + end + + describe '#format_patch' do + context 'on fork' do + it 'should build a format patch' do + merge_request_fork.target_branch = @close_commit1[0] + merge_request_fork.source_branch = @close_commit2[0] + patch = Gitlab::Satellite::MergeAction.new(merge_request_fork.author, merge_request_fork).format_patch + (patch.include? "From #{@close_commit2[1]}").should be_true + (patch.include? "From #{@close_commit1[1]}").should be_true + end + end + + context 'between branches' do + it 'should build a format patch' do + merge_request.target_branch = @close_commit1[0] + merge_request.source_branch = @close_commit2[0] + patch = Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request).format_patch + (patch.include? "From #{@close_commit2[1]}").should be_true + (patch.include? "From #{@close_commit1[1]}").should be_true + end + end + end + + describe '#diffs_between_satellite tested against diff_in_satellite' do + + def is_a_matching_diff(diff, diffs) + diff_count = diff.scan('diff --git').size + diff_count.should >= 1 + diffs.size.should == diff_count + diffs.each do |a_diff| + a_diff.class.should == Gitlab::Git::Diff + (diff.include? a_diff.diff).should be_true + end + end + + context 'on fork' do + it 'should get proper diffs' do + merge_request_fork.target_branch = @close_commit1[0] + merge_request_fork.source_branch = @master[0] + diffs = Gitlab::Satellite::MergeAction.new(merge_request_fork.author, merge_request_fork).diffs_between_satellite + + merge_request_fork.target_branch = @close_commit1[0] + merge_request_fork.source_branch = @master[0] + diff = Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request_fork).diff_in_satellite + + is_a_matching_diff(diff, diffs) + end + end + + context 'between branches' do + it 'should get proper diffs' do + merge_request.target_branch = @close_commit1[0] + merge_request.source_branch = @master[0] + expect{Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request).diffs_between_satellite}.to raise_error + end + end + end + + describe '#can_be_merged?' do + context 'on fork' do + it 'return true or false depending on if something is mergable' do + merge_request_fork.target_branch = @one_after_stable[0] + merge_request_fork.source_branch = @master[0] + Gitlab::Satellite::MergeAction.new(merge_request_fork.author, merge_request_fork).can_be_merged?.should be_true + + merge_request_fork.target_branch = @conflicting_metior[0] + merge_request_fork.source_branch = @wiki_branch[0] + Gitlab::Satellite::MergeAction.new(merge_request_fork.author, merge_request_fork).can_be_merged?.should be_false + end + end + + context 'between branches' do + it 'return true or false depending on if something is mergable' do + merge_request.target_branch = @one_after_stable[0] + merge_request.source_branch = @master[0] + Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request).can_be_merged?.should be_true + + merge_request.target_branch = @conflicting_metior[0] + merge_request.source_branch = @wiki_branch[0] + Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request).can_be_merged?.should be_false + end + end + end +end
\ No newline at end of file diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index e7e8bc5b028..d89029276d8 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -167,7 +167,7 @@ describe Notify do end context 'for merge requests' do - let(:merge_request) { create(:merge_request, assignee: assignee, project: project) } + let(:merge_request) { create(:merge_request, assignee: assignee, source_project: project, target_project: project) } describe 'that are new' do subject { Notify.new_merge_request_email(merge_request.assignee_id, merge_request.id) } @@ -311,7 +311,7 @@ describe Notify do end describe 'on a merge request' do - let(:merge_request) { create(:merge_request, project: project) } + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } let(:note_on_merge_request_path) { project_merge_request_path(project, merge_request, anchor: "note_#{note.id}") } before(:each) { note.stub(:noteable).and_return(merge_request) } diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index ad99d8a390b..b84f24bf3ad 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -3,7 +3,6 @@ require 'spec_helper' describe Commit do let(:commit) { create(:project_with_code).repository.commit } - describe '#title' do it "returns no_commit_message when safe_message is blank" do commit.stub(:safe_message).and_return('') diff --git a/spec/models/forked_project_link_spec.rb b/spec/models/forked_project_link_spec.rb index 5f25e2abd23..44b8c6155be 100644 --- a/spec/models/forked_project_link_spec.rb +++ b/spec/models/forked_project_link_spec.rb @@ -12,9 +12,9 @@ require 'spec_helper' describe ForkedProjectLink, "add link on fork" do - let(:project_from) {create(:project)} - let(:namespace) {create(:namespace)} - let(:user) {create(:user, namespace: namespace)} + let(:project_from) { create(:project) } + let(:namespace) { create(:namespace) } + let(:user) { create(:user, namespace: namespace) } before do @project_to = fork_project(project_from, user) @@ -30,9 +30,9 @@ describe ForkedProjectLink, "add link on fork" do end describe :forked_from_project do - let(:forked_project_link) {build(:forked_project_link)} - let(:project_from) {create(:project)} - let(:project_to) {create(:project, forked_project_link: forked_project_link)} + let(:forked_project_link) { build(:forked_project_link) } + let(:project_from) { create(:project) } + let(:project_to) { create(:project, forked_project_link: forked_project_link) } before :each do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index a0a43fbb815..68f3f920e06 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -41,15 +41,12 @@ describe MergeRequest do it { should include_module(Issuable) } end - describe "#mr_and_commit_notes" do - - end describe "#mr_and_commit_notes" do let!(:merge_request) { create(:merge_request) } before do - merge_request.stub(:commits) { [merge_request.project.repository.commit] } + merge_request.stub(:commits) { [merge_request.source_project.repository.commit] } create(:note, commit_id: merge_request.commits.first.id, noteable_type: 'Commit') create(:note, noteable: merge_request) end @@ -71,4 +68,38 @@ describe MergeRequest do subject.is_being_reassigned?.should be_false end end + + 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.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 + + describe '#allow_source_branch_removal?' do + it 'should not allow removal when mr is a fork' do + + subject.disallow_source_branch_removal?.should be_true + end + it 'should not allow removal when the mr is not a fork, but the source branch is the root reference' do + subject.target_project = subject.source_project + subject.source_branch = subject.source_project.repository.root_ref + subject.disallow_source_branch_removal?.should be_true + end + + it 'should not disallow removal when the mr is not a fork, and but source branch is not the root reference' do + subject.target_project = subject.source_project + subject.source_branch = "Something Different #{subject.source_project.repository.root_ref}" + subject.for_fork?.should be_false + subject.disallow_source_branch_removal?.should be_false + end + end + end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index ba94f940dc8..0f3af588457 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -144,12 +144,12 @@ describe Note do end describe '#create_status_change_note' do - let(:project) { create(:project) } - let(:thing) { create(:issue, project: project) } - let(:author) { create(:user) } - let(:status) { 'new_status' } + let(:project) { create(:project) } + let(:thing) { create(:issue, project: project) } + let(:author) { create(:user) } + let(:status) { 'new_status' } - subject { Note.create_status_change_note(thing, author, status) } + subject { Note.create_status_change_note(thing, project, author, status) } it 'creates and saves a Note' do should be_a Note @@ -157,9 +157,9 @@ describe Note do end its(:noteable) { should == thing } - its(:project) { should == thing.project } - its(:author) { should == author } - its(:note) { should =~ /Status changed to #{status}/ } + its(:project) { should == thing.project } + its(:author) { should == author } + its(:note) { should =~ /Status changed to #{status}/ } end describe :authorization do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 65b951c9619..65acf0c2498 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -26,6 +26,9 @@ require 'spec_helper' describe Project do + before(:each) { enable_observers } + after(:each) { disable_observers } + describe "Associations" do it { should belong_to(:group) } it { should belong_to(:namespace) } @@ -95,12 +98,11 @@ describe Project do end describe "last_activity methods" do - before { enable_observers } - let(:project) { create(:project) } + let(:project) { create(:project) } let(:last_event) { double(created_at: Time.now) } describe "last_activity" do - it "should alias last_activity to last_event"do + it "should alias last_activity to last_event" do project.stub(last_event: last_event) project.last_activity.should == last_event end @@ -122,7 +124,7 @@ describe Project do let(:project) { create(:project_with_code) } before do - @merge_request = create(:merge_request, project: project) + @merge_request = create(:merge_request, source_project: project, target_project: project) @key = create(:key, user_id: project.owner.id) end diff --git a/spec/observers/activity_observer_spec.rb b/spec/observers/activity_observer_spec.rb index 3d503027360..e93c969d5fe 100644 --- a/spec/observers/activity_observer_spec.rb +++ b/spec/observers/activity_observer_spec.rb @@ -8,18 +8,6 @@ describe ActivityObserver do it { @event.project.should == project } end - describe "Merge Request created" do - before do - MergeRequest.observers.enable :activity_observer do - @merge_request = create(:merge_request, project: project) - @event = Event.last - end - end - - it_should_be_valid_event - it { @event.action.should == Event::CREATED } - it { @event.target.should == @merge_request } - end describe "Issue created" do before do diff --git a/spec/observers/issue_observer_spec.rb b/spec/observers/issue_observer_spec.rb index 3cc621013d2..c9e88aee1a5 100644 --- a/spec/observers/issue_observer_spec.rb +++ b/spec/observers/issue_observer_spec.rb @@ -26,14 +26,13 @@ describe IssueObserver do before { mock_issue.stub(state: 'closed') } it 'note is created if the issue is being closed' do - Note.should_receive(:create_status_change_note).with(mock_issue, some_user, 'closed') + Note.should_receive(:create_status_change_note).with(mock_issue, mock_issue.project, some_user, 'closed') subject.after_close(mock_issue, nil) end it 'trigger notification to send emails' do subject.notification.should_receive(:close_issue).with(mock_issue, some_user) - subject.after_close(mock_issue, nil) end end @@ -42,8 +41,7 @@ describe IssueObserver do before { mock_issue.stub(state: 'reopened') } it 'note is created if the issue is being reopened' do - Note.should_receive(:create_status_change_note).with(mock_issue, some_user, 'reopened') - + Note.should_receive(:create_status_change_note).with(mock_issue, mock_issue.project, some_user, 'reopened') subject.after_reopen(mock_issue, nil) end end diff --git a/spec/observers/merge_request_observer_spec.rb b/spec/observers/merge_request_observer_spec.rb index 82c3fbc8a30..3d6fff9c24e 100644 --- a/spec/observers/merge_request_observer_spec.rb +++ b/spec/observers/merge_request_observer_spec.rb @@ -1,19 +1,21 @@ require 'spec_helper' describe MergeRequestObserver do - let(:some_user) { create :user } - let(:assignee) { create :user } - let(:author) { create :user } - let(:mr_mock) { double(:merge_request, id: 42, assignee: assignee, author: author) } - let(:assigned_mr) { create(:merge_request, assignee: assignee, author: author) } + let(:some_user) { create :user } + let(:assignee) { create :user } + let(:author) { create :user } + let(:mr_mock) { double(:merge_request, id: 42, assignee: assignee, author: author) } + let(:assigned_mr) { create(:merge_request, assignee: assignee, author: author) } let(:unassigned_mr) { create(:merge_request, author: author) } - let(:closed_assigned_mr) { create(:closed_merge_request, assignee: assignee, author: author) } + let(:closed_assigned_mr) { create(:closed_merge_request, assignee: assignee, author: author) } let(:closed_unassigned_mr) { create(:closed_merge_request, author: author) } before { subject.stub(:current_user).and_return(some_user) } before { subject.stub(notification: mock('NotificationService').as_null_object) } + before { mr_mock.stub(:author_id) } + before { mr_mock.stub(:target_project) } before(:each) { enable_observers } - + after(:each) { disable_observers } subject { MergeRequestObserver.instance } @@ -30,7 +32,7 @@ describe MergeRequestObserver do end it 'is called when a merge request is changed' do - changed = create(:merge_request, project: create(:project)) + changed = create(:merge_request, source_project: create(:project)) subject.should_receive(:after_update) MergeRequest.observers.enable :merge_request_observer do @@ -59,13 +61,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, some_user, 'closed') + Note.should_receive(:create_status_change_note).with(assigned_mr, assigned_mr.target_project, some_user, 'closed') 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, some_user, 'closed') + Note.should_receive(:create_status_change_note).with(unassigned_mr, unassigned_mr.target_project, some_user, 'closed') unassigned_mr.close end @@ -75,16 +77,41 @@ 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, some_user, 'reopened') + Note.should_receive(:create_status_change_note).with(closed_assigned_mr, closed_assigned_mr.target_project, some_user, 'reopened') 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, some_user, 'reopened') + Note.should_receive(:create_status_change_note).with(closed_unassigned_mr, closed_unassigned_mr.target_project, some_user, 'reopened') closed_unassigned_mr.reopen end end end + + describe "Merge Request created" do + def self.it_should_be_valid_event + it { @event.should_not be_nil } + it { @event.should_not be_nil } + it { @event.project.should == project } + 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) + @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/observers/user_observer_spec.rb b/spec/observers/user_observer_spec.rb index e0902b7eaf3..b74fceb98b1 100644 --- a/spec/observers/user_observer_spec.rb +++ b/spec/observers/user_observer_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe UserObserver do before(:each) { enable_observers } + after(:each) {disable_observers} subject { UserObserver.instance } before { subject.stub(notification: mock('NotificationService').as_null_object) } diff --git a/spec/observers/users_project_observer_spec.rb b/spec/observers/users_project_observer_spec.rb index b5f72946492..e33d8cc50fd 100644 --- a/spec/observers/users_project_observer_spec.rb +++ b/spec/observers/users_project_observer_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe UsersProjectObserver do before(:each) { enable_observers } + after(:each) { disable_observers } let(:user) { create(:user) } let(:project) { create(:project) } diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 5bf228a7448..7b893c43379 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -3,10 +3,12 @@ require "spec_helper" describe API::API do include ApiHelpers - let(:user) { create(:user ) } - let!(:project) { create(:project_with_code, creator_id: user.id) } - let!(:merge_request) { create(:merge_request, author: user, assignee: user, project: project, title: "Test") } - before { project.team << [user, :reporters] } + let(:user) { create(:user) } + let!(:project) {create(:project_with_code, creator_id: user.id) } + let!(:merge_request) { create(:merge_request, author: user, assignee: user, source_project: project, target_project: project, title: "Test") } + before { + project.team << [user, :reporters] + } describe "GET /projects/:id/merge_requests" do context "when unauthenticated" do @@ -40,35 +42,104 @@ describe API::API do end describe "POST /projects/:id/merge_requests" 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 - response.status.should == 201 - json_response['title'].should == 'Test merge_request' - end + 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 + 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 - response.status.should == 422 - 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 + 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 - response.status.should == 400 - 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 + 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 - response.status.should == 400 + 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 + 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' + response.status.should == 400 + end end - it "should return 400 when title is missing" do - post api("/projects/#{project.id}/merge_requests", user), - target_branch: 'master', source_branch: 'stable' - response.status.should == 400 + 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) } + + before :each do |each| + fork_project.team << [user2, :reporters] + forked_project_link.forked_from_project = project + forked_project_link.forked_to_project = fork_project + forked_project_link.save! + end + + 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 + response.status.should == 201 + json_response['title'].should == 'Test merge_request' + end + + it "should not return 422 when source_branch equals target_branch" do + project.id.should_not == fork_project.id + 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 + 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 + 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 + 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 + 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 + 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 + 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 + response.status.should == 201 + end end end @@ -97,14 +168,14 @@ 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 it "should return merge_request with renamed target_branch" do - put api("/projects/#{project.id}/merge_request/#{merge_request.id}", user), target_branch: "test" + put api("/projects/#{project.id}/merge_request/#{merge_request.id}", user), target_branch: "wiki" response.status.should == 200 - json_response['target_branch'].should == 'test' + json_response['target_branch'].should == 'wiki' end end diff --git a/spec/requests/api/milestones_spec.rb b/spec/requests/api/milestones_spec.rb index af12c088c8f..246fe262ce8 100644 --- a/spec/requests/api/milestones_spec.rb +++ b/spec/requests/api/milestones_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' describe API::API do include ApiHelpers before(:each) { enable_observers } + after(:each) {disable_observers} let(:user) { create(:user) } let!(:project) { create(:project, namespace: user.namespace ) } diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index 11296aea73e..48a2d111f8c 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -6,7 +6,7 @@ describe API::API do let(:user) { create(:user) } let!(:project) { create(:project, namespace: user.namespace ) } let!(:issue) { create(:issue, project: project, author: user) } - let!(:merge_request) { create(:merge_request, project: project, author: user) } + let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: user) } let!(:snippet) { create(:project_snippet, project: project, author: user) } let!(:issue_note) { create(:note, noteable: issue, project: project, author: user) } let!(:merge_request_note) { create(:note, noteable: merge_request, project: project, author: user) } diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 863ecc61bbb..2de3dc55e40 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' describe API::API do include ApiHelpers before(:each) { enable_observers } + after(:each) { disable_observers } let(:user) { create(:user) } let(:user2) { create(:user) } @@ -475,7 +476,6 @@ describe API::API do end end - describe "GET /projects/:id/snippets" do it "should return an array of project snippets" do get api("/projects/#{project.id}/snippets", user) diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index 13e6627840e..afa67d1ecbe 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' describe API::API do include ApiHelpers before(:each) { enable_observers } + after(:each) {disable_observers} let(:user) { create(:user) } let(:user2) { create(:user) } diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 76501482303..8428326e018 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -2,7 +2,6 @@ require 'spec_helper' describe NotificationService do let(:notification) { NotificationService.new } - describe 'Keys' do describe :new_key do let(:key) { create(:personal_key) } @@ -156,7 +155,8 @@ describe NotificationService do let(:merge_request) { create :merge_request, assignee: create(:user) } before do - build_team(merge_request.project) + build_team(merge_request.source_project) + build_team(merge_request.target_project) end describe :new_merge_request do @@ -232,11 +232,16 @@ describe NotificationService do end end + let(:u_watcher) { create(:user, notification_level: Notification::N_WATCH) } + let(:u_participating) { create(:user, notification_level: Notification::N_PARTICIPATING) } + let(:u_disabled) { create(:user, notification_level: Notification::N_DISABLED) } + let(:u_mentioned) { create(:user, username: 'mention', notification_level: Notification::N_WATCH) } + def build_team(project) - @u_watcher = create(:user, notification_level: Notification::N_WATCH) - @u_participating = create(:user, notification_level: Notification::N_PARTICIPATING) - @u_disabled = create(:user, notification_level: Notification::N_DISABLED) - @u_mentioned = create(:user, username: 'mention', notification_level: Notification::N_WATCH) + @u_watcher = u_watcher + @u_participating = u_participating + @u_disabled = u_disabled + @u_mentioned = u_mentioned project.team << [@u_watcher, :master] project.team << [@u_participating, :master] diff --git a/spec/services/project_transfer_service_spec.rb b/spec/services/project_transfer_service_spec.rb index 7b11c9785a9..bc26403b7f4 100644 --- a/spec/services/project_transfer_service_spec.rb +++ b/spec/services/project_transfer_service_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe ProjectTransferService do before(:each) { enable_observers } + after(:each) {disable_observers} context 'namespace -> namespace' do let(:user) { create(:user) } @@ -24,6 +25,7 @@ describe ProjectTransferService do @result = service.transfer(project, nil) end + it { @result.should be_true } it { project.namespace.should == nil } end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 3c318a4be82..dd008ed02ad 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -48,8 +48,11 @@ Spork.prefork do # instead of true. config.use_transactional_fixtures = false - config.before do - TestEnv.init(observers: false) + config.before(:suite) do + TestEnv.init(observers: false, init_repos: true, repos: false) + end + config.before(:each) do + TestEnv.setup_stubs end end end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 3230301eead..cae7ff88513 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -27,17 +27,41 @@ module TestEnv # Disable mailer for spinach tests disable_mailer if opts[:mailer] == false + setup_stubs - # Use tmp dir for FS manipulations - repos_path = Rails.root.join('tmp', 'test-git-base-path') - Gitlab.config.gitlab_shell.stub(repos_path: repos_path) - Gitlab::Git::Repository.stub(repos_path: repos_path) + clear_test_repo_dir if opts[:init_repos] == true + setup_test_repos(opts) if opts[:repos] == true + end + + def enable_observers + ActiveRecord::Base.observers.enable(:all) + end + + def disable_observers + ActiveRecord::Base.observers.disable(:all) + end + + def disable_mailer + NotificationService.any_instance.stub(mailer: double.as_null_object) + end + def enable_mailer + NotificationService.any_instance.unstub(:mailer) + end + def setup_stubs() + # Use tmp dir for FS manipulations + repos_path = testing_path() GollumWiki.any_instance.stub(:init_repo) do |path| create_temp_repo(File.join(repos_path, "#{path}.git")) end + Gitlab.config.gitlab_shell.stub(repos_path: repos_path) + + Gitlab.config.satellites.stub(path: satellite_path) + + Gitlab::Git::Repository.stub(repos_path: repos_path) + Gitlab::Shell.any_instance.stub( add_repository: true, mv_repository: true, @@ -50,22 +74,95 @@ module TestEnv Gitlab::Satellite::Satellite.any_instance.stub( exists?: true, destroy: true, - create: true + create: true, + lock_files_dir: repos_path ) MergeRequest.any_instance.stub( check_if_can_be_merged: true ) - Repository.any_instance.stub( size: 12.45 ) + end + + def clear_repo_dir(namespace, name) + setup_stubs + #Clean any .wiki.git that may have been created + FileUtils.rm_rf File.join(testing_path(), "#{name}.wiki.git") + end + + #Create a repo and it's satellite + def create_repo(namespace, name) + setup_stubs + repo = repo(namespace, name) + # Symlink tmp/repositories/gitlabhq to tmp/test-git-base-path/gitlabhq + system("ln -s -f #{seed_repo_path()} #{repo}") + create_satellite(repo, namespace, name) + end + + private + + def testing_path + Rails.root.join('tmp', 'test-git-base-path') + end + + def seed_repo_path + Rails.root.join('tmp', 'repositories', 'gitlabhq') + end + + def seed_satellite_path + Rails.root.join('tmp', 'satellite', 'gitlabhq') + end + + def satellite_path + "#{testing_path()}/satellite" + end + + def repo(namespace, name) + unless (namespace.nil? || namespace.path.nil? || namespace.path.strip.empty?) + repo = File.join(testing_path(), "#{namespace.path}/#{name}.git") + else + repo = File.join(testing_path(), "#{name}.git") + end + end + + def satellite(namespace, name) + unless (namespace.nil? || namespace.path.nil? || namespace.path.strip.empty?) + satellite_repo = File.join(satellite_path, namespace.path, name) + else + satellite_repo = File.join(satellite_path, name) + end + end + + def setup_test_repos(opts ={}) + create_repo(nil, 'gitlabhq') #unless opts[:repo].nil? || !opts[:repo].include?('') + create_repo(nil, 'source_gitlabhq') #unless opts[:repo].nil? || !opts[:repo].include?('source_') + create_repo(nil, 'target_gitlabhq') #unless opts[:repo].nil? || !opts[:repo].include?('target_') + end + + def clear_test_repo_dir + setup_stubs + # Use tmp dir for FS manipulations + repos_path = testing_path() # Remove tmp/test-git-base-path FileUtils.rm_rf Gitlab.config.gitlab_shell.repos_path # Recreate tmp/test-git-base-path FileUtils.mkdir_p Gitlab.config.gitlab_shell.repos_path + + #Since much more is happening in satellites + FileUtils.mkdir_p Gitlab.config.satellites.path + end + + # Create a testing satellite, and clone the source repo into it + def create_satellite(source_repo, namespace, satellite_name) + satellite_repo = satellite(namespace, satellite_name) + # Symlink tmp/satellite/gitlabhq to tmp/test-git-base-path/satellite/gitlabhq, create the directory if it doesn't exist already + satellite_dir = File.dirname(satellite_repo) + FileUtils.mkdir_p satellite_dir unless File.exists?(satellite_dir) + system("ln -s -f #{seed_satellite_path()} #{satellite_repo}") end def create_temp_repo(path) @@ -73,16 +170,4 @@ module TestEnv command = "git init --quiet --bare #{path};" system(command) end - - def enable_observers - ActiveRecord::Base.observers.enable(:all) - end - - def disable_observers - ActiveRecord::Base.observers.disable(:all) - end - - def disable_mailer - NotificationService.any_instance.stub(mailer: double.as_null_object) - end end |