Merge branch 'reduce-observers' into 'master'

Remove some observers

27 files changed, 214 insertions, 335 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
             &nbsp;
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/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 }