Merge branch 'master' into 222-create-new-branch-on-web

app/controllers/api/v1/issues_controller.rb

@@ -34,6 +34,7 @@ class Api::V1::IssuesController < Api::V1::BaseController
   end
 
   def show
+    redirect_to api_v1_project_pull_request_path(@project.id, @issue.serial_id) if @issue.pull_request
   end
 
   def create

app/controllers/api/v1/pull_requests_controller.rb

@@ -39,6 +39,7 @@ class Api::V1::PullRequestsController < Api::V1::BaseController
   end
 
   def show
+    redirect_to api_v1_project_issue_path(@project.id, @issue.serial_id) if @pull.nil?
   end
 
   def create
@@ -73,7 +74,7 @@ class Api::V1::PullRequestsController < Api::V1::BaseController
 
     if pull_params.present?
       attrs = pull_params.slice(:title, :body)
-      attrs.merge(:assignee_id => pull_params[:assignee_id]) if can?(:write, @project)
+      attrs.merge!(:assignee_id => pull_params[:assignee_id]) if can?(:write, @project)
 
       if (action = pull_params[:status]) && %w(close reopen).include?(pull_params[:status])
         if @pull.send("can_#{action}?")

app/controllers/projects/pull_requests_controller.rb

@@ -79,8 +79,12 @@ class Projects::PullRequestsController < Projects::BaseController
   end
 
   def show
+    if @pull.nil?
+      redirect_to project_issue_path(@project, @issue)
+    else
       load_diff_commits_data
     end
+  end
 
   def index(status = 200)
     @issues_with_pull_request = @project.issues.joins(:pull_request)

spec/controllers/api/v1/issues_controller_spec.rb

@@ -7,8 +7,8 @@ describe Api::V1::IssuesController do
     stub_redis
     any_instance_of(Project, :versions => ['v1.0', 'v2.0'])
 
-    @issue = FactoryGirl.create(:issue)
+    @project = FactoryGirl.create(:project_with_commit)
-    @project = @issue.project
+    @issue = FactoryGirl.create(:issue, :project => @project)
 
     @membered_issue = FactoryGirl.create(:issue)
     @membered_project = @membered_issue.project
@@ -27,6 +27,12 @@ describe Api::V1::IssuesController do
 
     @create_params = {:issue => {:title => 'title', :body => 'body'}, :project_id => @project.id, :format => :json}
     @update_params = {:issue => {:title => 'new title'}, :project_id => @project.id, :id => @issue.serial_id, :format => :json}
+
+    @pull = @project.pull_requests.new :issue_attributes => {:title => 'test', :body => 'testing'}
+    @pull.issue.user, @pull.issue.project = @project.owner, @pull.to_project
+    @pull.to_ref = 'master'
+    @pull.from_project, @pull.from_ref = @project, 'non_conflicts'
+    @pull.save
   end
 
   context 'read and accessible abilities' do
@@ -82,6 +88,16 @@ describe Api::V1::IssuesController do
         get :user_index, :format => :json
         response.should render_template('api/v1/issues/index')
       end
+
+      it 'should return 404' do
+        get :show, :project_id => @project.id, :id => 999999, :format => :json
+        response.status.should == 404
+      end
+
+      it 'should redirect to pull request page' do
+        get :show, :project_id => @project.id, :id => @pull.serial_id, :format => :json
+        response.should redirect_to(api_v1_project_pull_request_path(@project.id, @pull.serial_id))
+      end
     end
 
     context 'for anonymous user' do

spec/controllers/api/v1/pull_requests_controller.rb

@@ -38,6 +38,8 @@ describe Api::V1::PullRequestsController do
 
     @update_params = {:pull_request => {:title => 'new title'},
                       :project_id => @project.id, :id => @pull.serial_id, :format => :json}
+
+    @issue = FactoryGirl.create(:issue, :project => @project)
   end
 
   context 'read and accessible abilities' do
@@ -110,6 +112,16 @@ describe Api::V1::PullRequestsController do
           response.should_not be_success
         end
       end
+
+      it 'should return 404' do
+        get :show, :project_id => @project.id, :id => 999999, :format => :json
+        response.status.should == 404
+      end
+
+      it 'should redirect to issue page' do
+        get :show, :project_id => @project.id, :id => @issue.serial_id, :format => :json
+        response.should redirect_to(api_v1_project_issue_path(@project.id, @issue.serial_id))
+      end
     end
 
     context 'for anonymous user' do
@@ -130,17 +142,17 @@ describe Api::V1::PullRequestsController do
       end
 
       %w(commits files).each do |action|
-        it "can show pull request #{action} in project" do
+        it "can show pull request #{action} in project", :anonymous_access => true do
           get action, :project_id => @project.id, :id => @pull.serial_id, :format => :json
           response.should be_success
         end
 
-        it "should render right template for commits action" do
+        it "should render right template for commits action", :anonymous_access => true do
           get action, :project_id => @project.id, :id => @pull.serial_id, :format => :json
           response.should render_template("api/v1/pull_requests/#{action}")
         end
 
-        it "can't show pull request #{action} in hidden project" do
+        it "can't show pull request #{action} in hidden project", :anonymous_access => true do
           get action, :project_id => @hidden_project.id, :id => @hidden_pull.serial_id, :format => :json
           response.should_not be_success
         end
@@ -290,6 +302,7 @@ describe Api::V1::PullRequestsController do
     end
 
     it 'should send email message to new assignee' do
+      http_login(@project_admin)
       put :update, @update_params.deep_merge(:pull_request => {:assignee_id => @project_reader.id})
       @project.pull_requests.last.issue.send(:send_assign_notifications)
       ActionMailer::Base.deliveries.count.should == 1

spec/controllers/projects/issues_controller_spec.rb

@@ -5,7 +5,7 @@ shared_context "issues controller" do
   before do
     stub_symlink_methods
 
-    @project = FactoryGirl.create(:project)
+    @project = FactoryGirl.create(:project_with_commit)
     @issue_user = FactoryGirl.create(:user)
 
     @issue = FactoryGirl.create(:issue, :project_id => @project.id, :assignee_id => @issue_user.id)
@@ -34,6 +34,11 @@ shared_context "issues controller" do
       }
     }
 
+    @pull = @project.pull_requests.new :issue_attributes => {:title => 'test', :body => 'testing'}
+    @pull.issue.user, @pull.issue.project = @project.owner, @pull.to_project
+    @pull.to_ref = 'master'
+    @pull.from_project, @pull.from_ref = @project, 'non_conflicts'
+    @pull.save
   end
 
 end
@@ -214,6 +219,16 @@ describe Projects::IssuesController do
     # it 'should not create issue object into db' do
     #   lambda{ post :create, @create_params }.should change{ Issue.count }.by(0)
     # end
+
+    it 'should return 404' do
+      get :show, :owner_name => @project.owner.uname, :project_name => @project.name, :id => 999999
+      render_template(:file => "#{Rails.root}/public/404.html")
+    end
+
+    it 'should redirect to pull request page' do
+      get :show, :owner_name => @project.owner.uname, :project_name => @project.name, :id => @pull.serial_id
+      response.should redirect_to(project_pull_request_path(@project, @pull))
+    end
   end
 
   context 'for project writer user' do

spec/controllers/projects/pull_requests_controller_spec.rb

@@ -31,6 +31,8 @@ shared_context "pull request controller" do
 
     @user = FactoryGirl.create(:user)
     set_session_for(@user)
+
+    @issue = FactoryGirl.create(:issue, :project => @project)
   end
 end
 
@@ -207,6 +209,16 @@ describe Projects::PullRequestsController do
     it_should_behave_like 'pull request user with project reader rights'
     it_should_behave_like 'user without pull request update rights'
     it_should_behave_like 'pull request when project with issues turned off'
+
+    it 'should return 404' do
+      get :show, :owner_name => @project.owner.uname, :project_name => @project.name, :id => 999999
+      render_template(:file => "#{Rails.root}/public/404.html")
+    end
+
+    it 'should redirect to issue page' do
+      get :show, :owner_name => @project.owner.uname, :project_name => @project.name, :id => @issue.serial_id
+      response.should redirect_to(project_issue_path(@project, @issue))
+    end
   end
 
   context 'for project writer user' do