diff options
29 files changed, 217 insertions, 338 deletions
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index e6edceb1fbc..c090dd917c2 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -76,10 +76,10 @@ class Projects::MergeRequestsController < Projects::ApplicationController end def create - @merge_request = MergeRequest.new(params[:merge_request]) - @merge_request.author = current_user @target_branches ||= [] - if @merge_request.save + @merge_request = MergeRequests::CreateService.new(project, current_user, params[:merge_request]).execute + + if @merge_request.valid? redirect_to [@merge_request.target_project, @merge_request], notice: 'Merge request was successfully created.' else @source_project = @merge_request.source_project @@ -89,29 +89,9 @@ class Projects::MergeRequestsController < Projects::ApplicationController end def update - # If we close MergeRequest we want to ignore validation - # so we can close broken one (Ex. fork project removed) - if params[:merge_request] == {"state_event"=>"close"} - @merge_request.allow_broken = true - - if @merge_request.close - opts = { notice: 'Merge request was successfully closed.' } - else - opts = { alert: 'Failed to close merge request.' } - end - - redirect_to [@merge_request.target_project, @merge_request], opts - return - end - - # We dont allow change of source/target projects - # after merge request was created - params[:merge_request].delete(:source_project_id) - params[:merge_request].delete(:target_project_id) - - if @merge_request.update_attributes(params[:merge_request]) - @merge_request.reset_events_cache + @merge_request = MergeRequests::UpdateService.new(project, current_user, params[:merge_request]).execute(@merge_request) + if @merge_request.valid? respond_to do |format| format.js format.html do diff --git a/app/models/email.rb b/app/models/email.rb index 22e71e4f107..b92c1841063 100644 --- a/app/models/email.rb +++ b/app/models/email.rb @@ -13,14 +13,15 @@ class Email < ActiveRecord::Base # Relations # belongs_to :user - + # # Validations # validates :user_id, presence: true validates :email, presence: true, email: { strict_mode: true }, uniqueness: true validate :unique_email, if: ->(email) { email.email_changed? } - + + after_create :notify before_validation :cleanup_email def cleanup_email @@ -30,4 +31,8 @@ class Email < ActiveRecord::Base def unique_email self.errors.add(:email, 'has already been taken') if User.exists?(email: self.email) end -end
\ No newline at end of file + + def notify + NotificationService.new.new_email(self) + end +end diff --git a/app/models/key.rb b/app/models/key.rb index 29a76f53f3d..4202d79a956 100644 --- a/app/models/key.rb +++ b/app/models/key.rb @@ -29,6 +29,10 @@ class Key < ActiveRecord::Base delegate :name, :email, to: :user, prefix: true + after_create :add_to_shell + after_create :notify_user + after_destroy :remove_from_shell + def strip_white_space self.key = key.strip unless key.blank? end @@ -42,6 +46,26 @@ class Key < ActiveRecord::Base "key-#{id}" end + def add_to_shell + GitlabShellWorker.perform_async( + :add_key, + shell_id, + key + ) + end + + def notify_user + NotificationService.new.new_key(self) + end + + def remove_from_shell + GitlabShellWorker.perform_async( + :remove_key, + shell_id, + key, + ) + end + private def generate_fingerpint diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 5c2b0656d40..0decc7782ee 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -97,6 +97,7 @@ class MergeRequest < ActiveRecord::Base validates :target_project, presence: true validates :target_branch, presence: true validate :validate_branches + validate :validate_fork 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) } @@ -125,6 +126,22 @@ class MergeRequest < ActiveRecord::Base end end + def validate_fork + return true unless target_project && source_project + + if target_project == source_project + true + else + # If source and target projects are different + # we should check if source project is actually a fork of target project + if source_project.forked_from?(target_project) + true + else + errors.add :base, "Source project is not a fork of target project" + end + end + end + def update_merge_request_diff if source_branch_changed? || target_branch_changed? reload_code diff --git a/app/models/project.rb b/app/models/project.rb index 769ab217625..79066e1c54a 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -552,4 +552,8 @@ class Project < ActiveRecord::Base gitlab_shell.update_repository_head(self.path_with_namespace, branch) reload_default_branch end + + def forked_from?(project) + forked? && project == forked_from_project + end end diff --git a/app/observers/email_observer.rb b/app/observers/email_observer.rb deleted file mode 100644 index 026ad8b1d9a..00000000000 --- a/app/observers/email_observer.rb +++ /dev/null @@ -1,5 +0,0 @@ -class EmailObserver < BaseObserver - def after_create(email) - notification.new_email(email) - end -end diff --git a/app/observers/key_observer.rb b/app/observers/key_observer.rb deleted file mode 100644 index c013594e787..00000000000 --- a/app/observers/key_observer.rb +++ /dev/null @@ -1,19 +0,0 @@ -class KeyObserver < BaseObserver - def after_create(key) - GitlabShellWorker.perform_async( - :add_key, - key.shell_id, - key.key - ) - - notification.new_key(key) - end - - def after_destroy(key) - GitlabShellWorker.perform_async( - :remove_key, - key.shell_id, - key.key, - ) - end -end diff --git a/app/observers/merge_request_observer.rb b/app/observers/merge_request_observer.rb deleted file mode 100644 index 04ee30b4976..00000000000 --- a/app/observers/merge_request_observer.rb +++ /dev/null @@ -1,43 +0,0 @@ -class MergeRequestObserver < BaseObserver - def after_create(merge_request) - event_service.open_mr(merge_request, current_user) - notification.new_merge_request(merge_request, current_user) - merge_request.create_cross_references!(merge_request.project, current_user) - execute_hooks(merge_request) - end - - def after_close(merge_request, transition) - event_service.close_mr(merge_request, current_user) - notification.close_mr(merge_request, current_user) - create_note(merge_request) - execute_hooks(merge_request) - end - - def after_reopen(merge_request, transition) - event_service.reopen_mr(merge_request, current_user) - create_note(merge_request) - execute_hooks(merge_request) - merge_request.reload_code - merge_request.mark_as_unchecked - end - - def after_update(merge_request) - notification.reassigned_merge_request(merge_request, current_user) if merge_request.is_being_reassigned? - - merge_request.notice_added_references(merge_request.project, current_user) - execute_hooks(merge_request) - end - - private - - # Create merge request note with service comment like 'Status changed to closed' - def create_note(merge_request) - Note.create_status_change_note(merge_request, merge_request.target_project, current_user, merge_request.state, nil) - end - - def execute_hooks(merge_request) - if merge_request.project - merge_request.project.execute_hooks(merge_request.to_hook_data, :merge_request_hooks) - end - end -end diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb new file mode 100644 index 00000000000..a1261972157 --- /dev/null +++ b/app/services/merge_requests/base_service.rb @@ -0,0 +1,16 @@ +module MergeRequests + class BaseService < ::BaseService + + private + + def create_note(merge_request) + Note.create_status_change_note(merge_request, merge_request.target_project, current_user, merge_request.state, nil) + end + + def execute_hooks(merge_request) + if merge_request.project + merge_request.project.execute_hooks(merge_request.to_hook_data, :merge_request_hooks) + end + end + end +end diff --git a/app/services/merge_requests/close_service.rb b/app/services/merge_requests/close_service.rb new file mode 100644 index 00000000000..64e37a23e6b --- /dev/null +++ b/app/services/merge_requests/close_service.rb @@ -0,0 +1,18 @@ +module MergeRequests + class CloseService < MergeRequests::BaseService + def execute(merge_request, commit = nil) + # If we close MergeRequest we want to ignore validation + # so we can close broken one (Ex. fork project removed) + merge_request.allow_broken = true + + if merge_request.close + event_service.close_mr(merge_request, current_user) + notification_service.close_mr(merge_request, current_user) + create_note(merge_request) + execute_hooks(merge_request) + end + + merge_request + end + end +end diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb new file mode 100644 index 00000000000..d1bf827f3fc --- /dev/null +++ b/app/services/merge_requests/create_service.rb @@ -0,0 +1,19 @@ +module MergeRequests + class CreateService < MergeRequests::BaseService + def execute + merge_request = MergeRequest.new(params) + merge_request.source_project = project + merge_request.target_project ||= project + merge_request.author = current_user + + if merge_request.save + event_service.open_mr(merge_request, current_user) + notification_service.new_merge_request(merge_request, current_user) + merge_request.create_cross_references!(merge_request.project, current_user) + execute_hooks(merge_request) + end + + merge_request + end + end +end diff --git a/app/services/merge_requests/reopen_service.rb b/app/services/merge_requests/reopen_service.rb new file mode 100644 index 00000000000..2eb13d3e0e1 --- /dev/null +++ b/app/services/merge_requests/reopen_service.rb @@ -0,0 +1,15 @@ +module MergeRequests + class ReopenService < MergeRequests::BaseService + def execute(merge_request) + if merge_request.reopen + event_service.reopen_mr(merge_request, current_user) + create_note(merge_request) + execute_hooks(merge_request) + merge_request.reload_code + merge_request.mark_as_unchecked + end + + merge_request + end + end +end diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb new file mode 100644 index 00000000000..bbedca6ffd6 --- /dev/null +++ b/app/services/merge_requests/update_service.rb @@ -0,0 +1,37 @@ +require_relative 'base_service' +require_relative 'reopen_service' +require_relative 'close_service' + +module MergeRequests + class UpdateService < MergeRequests::BaseService + def execute(merge_request) + # We dont allow change of source/target projects + # after merge request was created + params.delete(:source_project_id) + params.delete(:target_project_id) + + state = params.delete('state_event') + + case state + when 'reopen' + MergeRequests::ReopenService.new(project, current_user, {}).execute(merge_request) + when 'close' + MergeRequests::CloseService.new(project, current_user, {}).execute(merge_request) + end + + if params.present? && merge_request.update_attributes(params) + merge_request.reset_events_cache + + if merge_request.previous_changes.include?('assignee_id') + notification_service.reassigned_merge_request(merge_request, current_user) + create_assignee_note(merge_request) + end + + merge_request.notice_added_references(merge_request.project, current_user) + execute_hooks(merge_request) + end + + merge_request + end + end +end diff --git a/app/views/projects/merge_requests/_form.html.haml b/app/views/projects/merge_requests/_form.html.haml index 22502760e50..0fe2d1d9801 100644 --- a/app/views/projects/merge_requests/_form.html.haml +++ b/app/views/projects/merge_requests/_form.html.haml @@ -33,7 +33,7 @@ .col-sm-10 .clearfix .pull-left - - projects = @project.forked_from_project.nil? ? [@project] : [ @project,@project.forked_from_project] + - projects = @project.forked_from_project.nil? ? [@project] : [@project, @project.forked_from_project] = f.select(:target_project_id, options_from_collection_for_select(projects, 'id', 'path_with_namespace', f.object.target_project_id), {}, { class: 'target_project select2 span3', disabled: @merge_request.persisted? }) .pull-left diff --git a/config/application.rb b/config/application.rb index 3933c046af0..76b19eeb529 100644 --- a/config/application.rb +++ b/config/application.rb @@ -21,8 +21,6 @@ module Gitlab # Activate observers that should always be running. config.active_record.observers = :milestone_observer, :project_activity_cache_observer, - :key_observer, - :merge_request_observer, :note_observer, :project_observer, :system_hook_observer, diff --git a/doc/release/monthly.md b/doc/release/monthly.md index 08149b4da86..2949e320f3c 100644 --- a/doc/release/monthly.md +++ b/doc/release/monthly.md @@ -61,9 +61,9 @@ After making the release branch new commits are cherry-picked from master. When * 1-7th: official merge window (see contributing guide) * 8-14th: work on bugfixes, sponsored features and GitLab EE * 15th: code freeze (stop merging into master except essential bugfixes) -* 18th: release candidate 1 (VERSION x.x.0.rc1, tag and tweet about x.x.0.rc1, release on GitLab Cloud) +* 18th: release candidate 1 (VERSION x.x.0.rc1, annotated tag and tweet about x.x.0.rc1, release on GitLab Cloud) * 20st: optional release candidate 2 (x.x.0.rc2, only if rc1 had problems) -* 22nd: release (VERSION x.x.0, create x-x-stable branch, tag, blog and tweet) +* 22nd: release (VERSION x.x.0, create x-x-stable branch, annotated tag tag, blog and tweet) * 23nd: optional patch releases (x.x.1, x.x.2, etc., only if there are serious problems) * 24-end of month: release GitLab EE and GitLab CI diff --git a/doc/release/security.md b/doc/release/security.md index 8e5a7e32099..56a44b5d1da 100644 --- a/doc/release/security.md +++ b/doc/release/security.md @@ -18,7 +18,7 @@ Please report suspected security vulnerabilities in private to support@gitlab.co 1. Create feature branches for the blog post on GitLab.com and link them from the code branch 1. Merge the code feature branch into master 1. Cherry-pick the code into the latest stable branch -1. Create a git tag vX.X.X for CE and another patch release for EE +1. Create an annotated tag vX.X.X for CE and another patch release for EE 1. Push the code and the tags to all the CE and EE repositories 1. Apply the patch to GitLab Cloud and the private GitLab development server 1. Merge and publish the blog posts diff --git a/features/steps/dashboard/merge_requests.rb b/features/steps/dashboard/merge_requests.rb index 62d84506c49..e198bc0cf9c 100644 --- a/features/steps/dashboard/merge_requests.rb +++ b/features/steps/dashboard/merge_requests.rb @@ -53,15 +53,15 @@ class DashboardMergeRequests < Spinach::FeatureSteps end def assigned_merge_request - @assigned_merge_request ||= create :merge_request, assignee: current_user, target_project: project + @assigned_merge_request ||= create :merge_request, assignee: current_user, target_project: project, source_project: project end def authored_merge_request - @authored_merge_request ||= create :merge_request, author: current_user, target_project: project + @authored_merge_request ||= create :merge_request, source_branch: 'simple_merge_request', author: current_user, target_project: project, source_project: project end def other_merge_request - @other_merge_request ||= create :merge_request, target_project: project + @other_merge_request ||= create :merge_request, source_branch: '2_3_notes_fix', target_project: project, source_project: project end def project diff --git a/features/support/env.rb b/features/support/env.rb index 7b11f5a7c6f..a5b297775db 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -52,6 +52,4 @@ Spinach.hooks.before_run do RSpec::Mocks::setup self include FactoryGirl::Syntax::Methods - MergeRequestObserver.any_instance.stub(current_user: create(:user)) end - diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 3a1a00d0719..e2d2d034444 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -13,14 +13,6 @@ 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 @@ -70,29 +62,15 @@ module API # POST /projects/:id/merge_requests # post ":id/merge_requests" do - set_current_user_for_thread 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, :target_project_id, :description] - 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 - present merge_request, with: Entities::MergeRequest - else - handle_merge_request_errors! merge_request.errors - end + authorize! :write_merge_request, user_project + required_attributes! [:source_branch, :target_branch, :title] + attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :target_project_id, :description] + merge_request = ::MergeRequests::CreateService.new(user_project, current_user, attrs).execute + + if merge_request.valid? + present merge_request, with: Entities::MergeRequest + else + handle_merge_request_errors! merge_request.errors end end @@ -111,17 +89,15 @@ module API # PUT /projects/:id/merge_request/:merge_request_id # put ":id/merge_request/:merge_request_id" do - set_current_user_for_thread do - attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :state_event, :description] - merge_request = user_project.merge_requests.find(params[:merge_request_id]) - - authorize! :modify_merge_request, merge_request + attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :state_event, :description] + merge_request = user_project.merge_requests.find(params[:merge_request_id]) + authorize! :modify_merge_request, merge_request + merge_request = ::MergeRequests::UpdateService.new(user_project, current_user, attrs).execute(merge_request) - if merge_request.update_attributes attrs - present merge_request, with: Entities::MergeRequest - else - handle_merge_request_errors! merge_request.errors - end + if merge_request.valid? + present merge_request, with: Entities::MergeRequest + else + handle_merge_request_errors! merge_request.errors end end diff --git a/spec/finders/merge_requests_finder_spec.rb b/spec/finders/merge_requests_finder_spec.rb index 0bd2ccafcc1..94b4d4c4ff4 100644 --- a/spec/finders/merge_requests_finder_spec.rb +++ b/spec/finders/merge_requests_finder_spec.rb @@ -5,10 +5,10 @@ describe MergeRequestsFinder do let(:user2) { create :user } let(:project1) { create(:project) } - let(:project2) { create(:project) } + let(:project2) { create(:project, forked_from_project: project1) } - let!(:merge_request1) { create(:merge_request, :simple, author: user, source_project: project1, target_project: project2) } - let!(:merge_request2) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project1) } + let!(:merge_request1) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project1) } + let!(:merge_request2) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project1, state: 'closed') } let!(:merge_request3) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project2) } before do @@ -21,7 +21,7 @@ describe MergeRequestsFinder do it 'should filter by scope' do params = { scope: 'authored', state: 'opened' } merge_requests = MergeRequestsFinder.new.execute(user, params) - merge_requests.size.should == 3 + merge_requests.size.should == 2 end it 'should filter by project' do diff --git a/spec/lib/gitlab/satellite/merge_action_spec.rb b/spec/lib/gitlab/satellite/merge_action_spec.rb index ef06c742846..41d3321b173 100644 --- a/spec/lib/gitlab/satellite/merge_action_spec.rb +++ b/spec/lib/gitlab/satellite/merge_action_spec.rb @@ -13,7 +13,7 @@ describe 'Gitlab::Satellite::MergeAction' do end let(:project) { create(:project, namespace: create(:group)) } - let(:fork_project) { create(:project, namespace: create(:group)) } + let(:fork_project) { create(:project, namespace: create(:group), forked_from_project: project) } let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } let(:merge_request_fork) { create(:merge_request, source_project: fork_project, target_project: project) } diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index 9c872c02a53..c1c6c2f31b7 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -68,4 +68,18 @@ describe Key do build(:invalid_key).should_not be_valid end end + + context 'callbacks' do + it 'should add new key to authorized_file' do + @key = build(:personal_key, id: 7) + GitlabShellWorker.should_receive(:perform_async).with(:add_key, @key.shell_id, @key.key) + @key.save + end + + it 'should remove key from authorized_file' do + @key = create(:personal_key) + GitlabShellWorker.should_receive(:perform_async).with(:remove_key, @key.shell_id, @key.key) + @key.destroy + end + end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 6be8a6a13f6..4cdda1feb31 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -209,7 +209,7 @@ describe Note do let(:project) { create(:project) } let(:author) { create(:user) } let(:issue) { create(:issue, project: project) } - let(:mergereq) { create(:merge_request, target_project: project) } + let(:mergereq) { create(:merge_request, :simple, target_project: project, source_project: project) } let(:commit) { project.repository.commit } # Test all of {issue, merge request, commit} in both the referenced and referencing diff --git a/spec/observers/email_observer_spec.rb b/spec/observers/email_observer_spec.rb deleted file mode 100644 index 599b9a6ffba..00000000000 --- a/spec/observers/email_observer_spec.rb +++ /dev/null @@ -1,17 +0,0 @@ -require 'spec_helper' - -describe EmailObserver do - let(:email) { create(:email) } - - before { subject.stub(notification: double('NotificationService').as_null_object) } - - subject { EmailObserver.instance } - - describe '#after_create' do - it 'trigger notification to send emails' do - subject.should_receive(:notification) - - subject.after_create(email) - end - end -end diff --git a/spec/observers/key_observer_spec.rb b/spec/observers/key_observer_spec.rb deleted file mode 100644 index b304bf1fcb7..00000000000 --- a/spec/observers/key_observer_spec.rb +++ /dev/null @@ -1,23 +0,0 @@ -require 'spec_helper' - -describe KeyObserver do - before do - @key = create(:personal_key) - - @observer = KeyObserver.instance - end - - context :after_create do - it do - GitlabShellWorker.should_receive(:perform_async).with(:add_key, @key.shell_id, @key.key) - @observer.after_create(@key) - end - end - - context :after_destroy do - it do - GitlabShellWorker.should_receive(:perform_async).with(:remove_key, @key.shell_id, @key.key) - @observer.after_destroy(@key) - end - end -end diff --git a/spec/observers/merge_request_observer_spec.rb b/spec/observers/merge_request_observer_spec.rb deleted file mode 100644 index 18df8b78513..00000000000 --- a/spec/observers/merge_request_observer_spec.rb +++ /dev/null @@ -1,131 +0,0 @@ -require 'spec_helper' - -describe MergeRequestObserver do - let(:some_user) { create :user } - let(:assignee) { create :user } - let(:author) { create :user } - let(:project) { create :project } - let(:mr_mock) { double(:merge_request, id: 42, assignee: assignee, author: author).as_null_object } - let(:assigned_mr) { create(:merge_request, assignee: assignee, author: author, source_project: project) } - let(:unassigned_mr) { create(:merge_request, author: author, source_project: project) } - let(:closed_assigned_mr) { create(:closed_merge_request, assignee: assignee, author: author, source_project: project) } - let(:closed_unassigned_mr) { create(:closed_merge_request, author: author, source_project: project) } - - before { subject.stub(:current_user).and_return(some_user) } - before { subject.stub(notification: double('NotificationService').as_null_object) } - before { mr_mock.stub(:author_id) } - before { mr_mock.stub(:source_project) } - before { mr_mock.stub(:source_project) } - before { mr_mock.stub(:project) } - before { mr_mock.stub(:create_cross_references!).and_return(true) } - before { Repository.any_instance.stub(commit: nil) } - - before(:each) { enable_observers } - after(:each) { disable_observers } - - subject { MergeRequestObserver.instance } - - describe '#after_create' do - it 'trigger notification service' do - subject.should_receive(:notification) - subject.after_create(mr_mock) - end - - it 'creates cross-reference notes' do - project = create :project - mr_mock.stub(title: "this mr references !#{assigned_mr.id}", project: project) - mr_mock.should_receive(:create_cross_references!).with(project, some_user) - - subject.after_create(mr_mock) - end - end - - context '#after_update' do - before(:each) do - mr_mock.stub(:is_being_reassigned?).and_return(false) - mr_mock.stub(:notice_added_references) - end - - it 'is called when a merge request is changed' do - changed = create(:merge_request, source_project: project) - subject.should_receive(:after_update) - - MergeRequest.observers.enable :merge_request_observer do - changed.title = 'I changed' - changed.save - end - end - - it 'checks for new references' do - mr_mock.should_receive(:notice_added_references) - - subject.after_update(mr_mock) - end - - context 'a notification' do - it 'is sent if the merge request is being reassigned' do - mr_mock.should_receive(:is_being_reassigned?).and_return(true) - subject.should_receive(:notification) - - subject.after_update(mr_mock) - end - - it 'is not sent if the merge request is not being reassigned' do - mr_mock.should_receive(:is_being_reassigned?).and_return(false) - subject.should_not_receive(:notification) - - subject.after_update(mr_mock) - end - end - end - - context '#after_close' do - context 'a status "closed"' do - it 'note is created if the merge request is being closed' do - Note.should_receive(:create_status_change_note).with(assigned_mr, assigned_mr.source_project, some_user, 'closed', nil) - - assigned_mr.close - end - - it 'notification is delivered only to author if the merge request is being closed' do - Note.should_receive(:create_status_change_note).with(unassigned_mr, unassigned_mr.source_project, some_user, 'closed', nil) - - unassigned_mr.close - end - end - end - - context '#after_reopen' do - context 'a status "reopened"' do - it 'note is created if the merge request is being reopened' do - Note.should_receive(:create_status_change_note).with(closed_assigned_mr, closed_assigned_mr.source_project, some_user, 'reopened', nil) - - closed_assigned_mr.reopen - end - - it 'notification is delivered only to author if the merge request is being reopened' do - Note.should_receive(:create_status_change_note).with(closed_unassigned_mr, closed_unassigned_mr.source_project, some_user, 'reopened', nil) - - 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 - - before do - @merge_request = create(:merge_request, source_project: project, target_project: project) - @event = Event.last - end - - it_should_be_valid_event - it { @event.action.should == Event::CREATED } - it { @event.target.should == @merge_request } - end -end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 138f218d46c..9530f7ceb04 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -6,7 +6,7 @@ describe API::API do after(:each) { ActiveRecord::Base.observers.disable(:user_observer) } let(:user) { create(:user) } let!(:project) {create(:project, creator_id: user.id, namespace: user.namespace) } - let!(:merge_request) { create(:merge_request, author: user, assignee: user, source_project: project, target_project: project, title: "Test") } + let!(:merge_request) { create(:merge_request, :simple, author: user, assignee: user, source_project: project, target_project: project, title: "Test") } let!(:note) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "a comment on a MR") } before { project.team << [user, :reporters] @@ -79,16 +79,12 @@ describe API::API do end context 'forked projects' do - let!(:user2) {create(:user)} - let!(:forked_project_link) { build(:forked_project_link) } - let!(:fork_project) { create(:project, forked_project_link: forked_project_link, namespace: user2.namespace, creator_id: user2.id) } - let!(:unrelated_project) { create(:project, namespace: create(:user).namespace, creator_id: user2.id) } + let!(:user2) { create(:user) } + let!(:fork_project) { create(:project, forked_from_project: project, namespace: user2.namespace, creator_id: user2.id) } + let!(:unrelated_project) { create(:project, namespace: create(:user).namespace, creator_id: user2.id) } before :each do |each| fork_project.team << [user2, :reporters] - 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 @@ -127,16 +123,16 @@ describe API::API do response.status.should == 400 end - it "should return 400 when target_branch is specified and not a forked project" do + it "should return 404 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 + response.status.should == 404 end - it "should return 400 when target_branch is specified and for a different fork" do + it "should return 404 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 + response.status.should == 404 end it "should return 201 when target_branch is specified and for the same project" do diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 0c25f7a71d0..57a4240f7a2 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -5,7 +5,7 @@ describe NotificationService do describe 'Keys' do describe :new_key do - let(:key) { create(:personal_key) } + let!(:key) { create(:personal_key) } it { notification.new_key(key).should be_true } @@ -18,7 +18,7 @@ describe NotificationService do describe 'Email' do describe :new_email do - let(:email) { create(:email) } + let!(:email) { create(:email) } it { notification.new_email(email).should be_true } |