[refs #90] fixing pull methods

Gemfile

@@ -47,6 +47,7 @@ gem "haml-rails", '~> 0.3.4'
 gem 'ruby-haml-js'
 gem 'rails-backbone'
 gem 'jquery-rails', '~> 2.0.1'
+gem 'state_machine', :require => 'state_machine/core'
 
 group :assets do
   gem 'sass-rails', '~> 3.2.5'

Gemfile.lock

@@ -372,6 +372,7 @@ DEPENDENCIES
   sass-rails (~> 3.2.5)
   shotgun
   shoulda
+  state_machine
   therubyracer (~> 0.10.0)
   therubyrhino (~> 1.73.1)
   trinidad (~> 1.0.2)

app/models/activity_feed_observer.rb

@@ -90,6 +90,7 @@ class ActivityFeedObserver < ActiveRecord::Observer
                           :change_type => change_type, :user_email => record.project.git_repository.repo.log(branch_name, nil).first.author.email,
                           :project_owner => record.project.owner.uname}
         options.merge!({:user_id => first_commiter.id, :user_name => first_commiter.name}) if first_commiter
+        record.project.pull_requests.needed_checking.each {|pull| pull.check}
       end
 
       record.project.owner_and_admin_ids.each do |recipient|

app/models/issue.rb

@@ -21,8 +21,8 @@ class Issue < ActiveRecord::Base
   attr_accessible :labelings_attributes, :title, :body, :assignee_id
   accepts_nested_attributes_for :labelings, :allow_destroy => true
 
-  scope :opened, where(:status => 'open', :closed_by => nil, :closed_at => nil)
-  scope :closed, where(:status => 'closed').where("closed_by is not null and closed_at is not null")
+  scope :opened, where(:status => 'open')
+  scope :closed, where(:status => 'closed')
 
   def assign_uname
     assignee.uname if assignee

app/models/pull_request.rb

@@ -1,48 +1,78 @@
 class PullRequest < Issue
+  extend StateMachine::MacroMethods # no method state_machine WTF?!
   serialize :data
+  #TODO add validates to serialized data
+  scope :needed_checking, where(:state => ['open', 'blocked', 'ready'])
 
   state_machine :initial => :open do
+    #after_transition [:ready, :blocked] => [:merged, :closed] do |pull, transition|
+    #  FileUtils.rm_rf(pull.path) # What about diff?
+    #end
 
-    event :open do
-      transition :closed => :open
-      transition :blocked => :open, :if => lambda {|pull| pull.can_merge?}
+    event :ready do
+      transition [:open, :blocked] => :ready
     end
 
     event :block do
-      transition :open => :blocked
+      transition [:open, :ready] => :block
     end
 
-    event :merge do
-      transition :open => :merged,  :if => lambda {|pull|  pull.can_merge?}
-      transition :open => :blocked, :if => lambda {|pull| !pull.can_merge?}
+    event :merging do
+      transition :ready => :merged
     end
 
     event :close do
-      transition [:open, :blocked] => :closed
+      transition [:open, :ready, :blocked] => :closed
+    end
+
+    event :reopen do
+      transition :closed => :open
     end
   end
 
   def can_merge?
-    !merge
+    state == 'ready'
+  end
+
+  def check
+    if ret = merge
+      system("cd #{path} && git reset --hard HEAD^") # remove merge commit
+      ready
+    else
+      system("cd #{path} && git reset --hard HEAD")
+      block
+    end
+  end
+
+  def merge!(who)
+    return false unless can_merge?
+    Dir.chdir(path) do
+      system "git config user.name \"#{who.uname}\" && git config user.email \"#{who.email}\""
+      if merge
+        merging
+        system("git push origin HEAD")
+      end
+    end
   end
 
   protected
 
   def path
+    filename = [id, project.owner.uname, project.name].join('-')
     if Rails.env == "production"
-      File.join('/srv/rosa_build/shared/tmp', "pull_requests", [id, project.owner.uname, project.name].join('-'))
+      File.join('/srv/rosa_build/shared/tmp', "pull_requests", filename)
     else
-      File.join(Rails.root, "tmp", Rails.env, "pull_requests", [id, project.owner.uname, project.name].join('-'))
+      File.join(Rails.root, "tmp", Rails.env, "pull_requests", filename)
     end
   end
 
   def merge
     clone
-    system "cd #{path} && git checkout #{data.base_branch} && git merge #{data.head_branch} && git reset --hard #{data.head_branch}"
+    system("cd #{path} && git checkout #{data[:base_branch]} && git merge --no-ff #{data[:head_branch]}")
   end
 
   def clone
-    git = Git.new(path)
+    git = Grit::Git.new(path)
 
     unless git.exist?
       FileUtils.mkdir_p(path)
@@ -51,7 +81,7 @@ class PullRequest < Issue
       end
     else
       Dir.chdir(path) do
-        [data.base_branch, data.head_branch].each do |branch|
+        [data[:base_branch], data[:head_branch]].each do |branch|
           system "git checkout #{branch} && git pull origin #{branch}"
         end
       end
@@ -59,4 +89,7 @@ class PullRequest < Issue
     # TODO catch errors
   end
 
+  def set_serial_id
+    self.update_attribute :serial_id, self.project.pull_requests.count
+  end
 end

db/migrate/20120412173938_create_pull_requests.rb

@@ -1,6 +1,7 @@
 class CreatePullRequests < ActiveRecord::Migration
   def change
     add_column :issues, :type, :string
+    add_column :issues, :state, :string
     add_column :issues, :data, :text, :null => false, :default => 0
   end
 end

spec/factories/pull_request.rb

@@ -0,0 +1,10 @@
+# -*- encoding : utf-8 -*-
+FactoryGirl.define do
+  factory :pull_request do
+    title { FactoryGirl.generate(:string) }
+    body { FactoryGirl.generate(:string) }
+    association :project, :factory => :project
+    association :user, :factory => :user
+    association :assignee, :factory => :user
+  end
+end

spec/models/pull_request_spec.rb

@@ -1,5 +1,48 @@
+# -*- encoding : utf-8 -*-
 require 'spec_helper'
 
 describe PullRequest do
-  pending "add some examples to (or delete) #{__FILE__}"
+
+  #~ context 'when create with same owner that platform' do
+    #~ before (:each) do
+      #~ stub_rsync_methods
+      #~ @platform = FactoryGirl.create(:platform)
+      #~ @params = {:name => 'tst_platform', :description => 'test platform'}
+    #~ end
+
+    #~ it 'it should increase Repository.count by 1' do
+      #~ rep = Repository.create(@params) {|r| r.platform = @platform}
+      #~ @platform.repositories.count.should eql(1)
+    #~ end
+  #~ end
+  
+  before(:all) do
+    stub_rsync_methods
+    Platform.delete_all
+    User.delete_all
+    Repository.delete_all
+    FileUtils.rm_rf(APP_CONFIG['root_path'])
+    # Need for validate_uniqueness_of check
+    FactoryGirl.create(:pull_request)
+  end
+
+  it { should belong_to(:project) }
+  it { should have_many(:comments)}
+  
+  it { should validate_presence_of(:title)}
+  #it { should validate_uniqueness_of(:serial_id).scoped_to(:project_id) }
+  it { should validate_presence_of(:body) }
+
+  it { should_not allow_mass_assignment_of(:project) }
+  it { should_not allow_mass_assignment_of(:project_id) }
+  it { should_not allow_mass_assignment_of(:user) }
+  it { should_not allow_mass_assignment_of(:user_id) }
+
+  after(:all) do
+    Platform.delete_all
+    User.delete_all
+    Repository.delete_all
+    FileUtils.rm_rf(APP_CONFIG['root_path'])
+  end
+
 end