From bbef8bb5896d8858bf91692bdd5aa9236472a885 Mon Sep 17 00:00:00 2001
From: Izaak Alpert <ialpert@blackberry.com>
Date: Sat, 8 Jun 2013 11:51:18 -0400
Subject: [PATCH] MR on fork validation error

Wasn't setting fields so validations would fail

Change-Id: I0363c57bf6c8b9937cc1b4140d09071d7c5168c2
---
 .../projects/merge_requests_controller.rb     |  6 ++--
 .../projects/merge_requests/_form.html.haml   | 28 +++++++++----------
 .../project/forked_merge_requests.feature     |  7 +++++
 .../project/project_forked_merge_requests.rb  | 17 +++++++++++
 4 files changed, 41 insertions(+), 17 deletions(-)

diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 5bbf52beed9..11f7f0bee9e 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -70,7 +70,9 @@ class Projects::MergeRequestsController < Projects::ApplicationController
       @merge_request.reload_code
       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
 
@@ -118,7 +120,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
 
   def update_branches
     @target_project = selected_target_project
-    @target_branches = (@target_project.repository.branch_names).unshift("Select branch")
+    @target_branches = @target_project.repository.branch_names
     @target_branches
   end
 
diff --git a/app/views/projects/merge_requests/_form.html.haml b/app/views/projects/merge_requests/_form.html.haml
index 9941a3718de..558c17eb0f6 100644
--- a/app/views/projects/merge_requests/_form.html.haml
+++ b/app/views/projects/merge_requests/_form.html.haml
@@ -64,21 +64,19 @@
 :javascript
   disableButtonIfEmptyField("#merge_request_title", ".btn-save");
 
-    var source_branch = $("#merge_request_source_branch")
-      , target_branch = $("#merge_request_target_branch")
-      , target_project = $("#merge_request_target_project_id");
+  var source_branch = $("#merge_request_source_branch")
+    , target_branch = $("#merge_request_target_branch")
+    , target_project = $("#merge_request_target_project_id");
 
-    $.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() });
-
-    target_project.live("change", function() {
-      $.get("#{update_branches_project_merge_requests_path(@source_project)}", {target_project_id:  $(this).val() });
-    });
-    source_branch.live("change", function() {
-      $.get("#{branch_from_project_merge_requests_path(@source_project)}", {ref: $(this).val() });
-    });
-    target_branch.live("change", function() {
-      $.get("#{branch_to_project_merge_requests_path(@source_project)}", {target_project_id: target_project.val(),ref: $(this).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() });
 
+  target_project.live("change", function() {
+    $.get("#{update_branches_project_merge_requests_path(@source_project)}", {target_project_id:  $(this).val() });
+  });
+  source_branch.live("change", function() {
+    $.get("#{branch_from_project_merge_requests_path(@source_project)}", {ref: $(this).val() });
+  });
+  target_branch.live("change", function() {
+    $.get("#{branch_to_project_merge_requests_path(@source_project)}", {target_project_id: target_project.val(),ref: $(this).val() });
   });
\ No newline at end of file
diff --git a/features/project/forked_merge_requests.feature b/features/project/forked_merge_requests.feature
index 45f4f1251a2..4312d5748b8 100644
--- a/features/project/forked_merge_requests.feature
+++ b/features/project/forked_merge_requests.feature
@@ -41,3 +41,10 @@ Feature: Project Forked Merge Requests
     Then I see the edit page prefilled for "Merge Request On Forked Project"
 
 
+  @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/steps/project/project_forked_merge_requests.rb b/features/steps/project/project_forked_merge_requests.rb
index 10d26a1498b..7579dce9268 100644
--- a/features/steps/project/project_forked_merge_requests.rb
+++ b/features/steps/project/project_forked_merge_requests.rb
@@ -152,6 +152,23 @@ class ProjectForkedMergeRequests < Spinach::FeatureSteps
   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