Downgrade unicorn. Fix project fork. Fix reserved_names validator. Add comments, cleanup code and refactor. Fix some bugs. Redo specs project URLs. Refs #263

Gemfile

@@ -35,7 +35,7 @@ gem 'rdiscount'
 gem 'RedCloth'
 gem 'wikicloth'
 
-gem 'unicorn', '~> 4.3.0', :platforms => [:mri, :rbx]
+gem 'unicorn', '~> 4.2.1', :platforms => [:mri, :rbx]
 gem 'trinidad', '~> 1.0.2', :platforms => :jruby
 gem 'newrelic_rpm', '~> 3.3.3', :platforms => [:mri, :rbx]
 gem 'whenever', '~> 0.7.3', :require => false
@@ -45,8 +45,8 @@ gem 'will_paginate', '~> 3.0.3'
 gem 'meta-tags', '~> 1.2.5', :require => 'meta_tags'
 gem "haml-rails", '~> 0.3.4'
 gem 'jquery-rails', '~> 2.0.2'
-gem 'ruby-haml-js'
-gem 'rails-backbone'
+gem 'ruby-haml-js', '~> 0.0.3'
+gem 'rails-backbone', '~> 0.7.2'
 
 group :assets do
   gem 'sass-rails', '~> 3.2.5'

Gemfile.lock

@@ -297,7 +297,7 @@ GEM
     uglifier (1.2.4)
       execjs (>= 0.3.0)
       multi_json (>= 1.0.2)
-    unicorn (4.3.0)
+    unicorn (4.2.1)
       kgio (~> 2.6)
       rack
       raindrops (~> 0.7)
@@ -348,7 +348,7 @@ DEPENDENCIES
   paperclip (~> 3.0.2)
   pg (~> 0.13.2)
   rails (= 3.2.3)
-  rails-backbone
+  rails-backbone (~> 0.7.2)
   rails3-generators
   rails3-jquery-autocomplete (~> 1.0.7)
   rdiscount
@@ -356,7 +356,7 @@ DEPENDENCIES
   redhillonrails_core!
   rr (~> 1.0.4)
   rspec-rails (~> 2.9.0)
-  ruby-haml-js
+  ruby-haml-js (~> 0.0.3)
   russian (~> 0.6.0)
   sass-rails (~> 3.2.5)
   shotgun
@@ -365,7 +365,7 @@ DEPENDENCIES
   therubyrhino (~> 1.73.1)
   trinidad (~> 1.0.2)
   uglifier (~> 1.2.4)
-  unicorn (~> 4.3.0)
+  unicorn (~> 4.2.1)
   whenever (~> 0.7.3)
   wikicloth
   will_paginate (~> 3.0.3)

app/controllers/projects_controller.rb

@@ -59,6 +59,7 @@ class ProjectsController < ApplicationController
   end
 
   def fork
+    authorize! :fork, @project # TODO WTF ?
     owner = (Group.find params[:group] if params[:group].present?) || current_user
     authorize! :update, owner if owner.class == Group
     if forked = @project.fork(owner) and forked.valid?

app/models/group.rb

@@ -36,7 +36,9 @@ class Group < ActiveRecord::Base
     (by_owner(user) | by_admin(user))
   end
 
-  def to_param; uname; end
+  def to_param
+    uname
+  end
 
   def name
     uname

app/models/user.rb

@@ -57,7 +57,9 @@ class User < ActiveRecord::Base
   after_create lambda { self.create_notifier }
   before_create :ensure_authentication_token
 
-  def to_param; uname; end
+  def to_param
+    uname
+  end
 
   def admin?
     role == 'admin'

app/views/layouts/application.html.haml

@@ -10,7 +10,7 @@
     = csrf_meta_tag
     = display_meta_tags :site => APP_CONFIG['project_name'], :reverse => true, :separator => '-'
     - if user_signed_in?
-      = auto_discovery_link_tag :atom, atom_activity_feeds_url(:format => 'atom', :token => current_user.authentication_token), :title => t("layout.atom_link_tag_title", :nickname => current_user.name, :app_name => APP_CONFIG['project_name'])
+      = auto_discovery_link_tag :atom, atom_activity_feeds_path(:format => 'atom', :token => current_user.authentication_token), :title => t("layout.atom_link_tag_title", :nickname => current_user.uname, :app_name => APP_CONFIG['project_name'])
 
   %body
     .wrap{:class => content_for?(:sidebar) ? 'columns' : ''}

lib/ext/rails/reserved_name_validator.rb

@@ -22,11 +22,16 @@ class ReservedNameValidator < ActiveModel::EachValidator
     unfollow unsubscribe url user
     widget widgets wiki
     xfn xmpp
-  } << Rails.application.routes.routes.map{|r| r.path.spec.to_s.match(/^\/([\w-]+)/)[1] rescue nil}.uniq.compact # current routes
+  }
+
+  def reserved_names
+    @reserved_names ||= RESERVED_NAMES +
+                        Rails.application.routes.routes.map{|r| r.path.spec.to_s.match(/^\/([\w-]+)/)[1] rescue nil}.uniq.compact # current routes
+  end
 
   def validate_each(record, attribute, value)
-    if RESERVED_NAMES.include?(value.downcase)
-      record.errors.add(attribute, :exclusion, options.merge!(:value => value))
+    if reserved_names.include?(value.downcase)
+      record.errors.add(attribute, :exclusion, options.merge(:value => value))
     end
   end
 end

lib/ext/rails/url_for.rb

@@ -3,12 +3,9 @@ module ActionDispatch
   module Routing
     module UrlFor
       def url_for_with_defaults(options = nil)
-        # raise options.inspect
         if options.kind_of?(Hash)
-          # if options[:controller] == 'projects' and options[:action] == 'show' and post = options[:_positional_args].try(:first) and post.blog
-          if project = options[:_positional_args].try(:first) and project.is_a?(Project)
-            options[:_positional_args].unshift(project.owner)
-            # options[:use_route] = 'blog_post'
+          if project = options[:_positional_args].try(:first) and project.is_a?(Project) # for project routes
+            options[:_positional_args].unshift(project.owner) # add owner to URL for correct generation
           end
         end
         url_for_without_defaults(options)

lib/modules/models/acts_like_member.rb

@@ -21,7 +21,7 @@ module Modules
 
         scope :search_order, order("CHAR_LENGTH(uname) ASC")
         scope :without, lambda {|a| where("#{klass.table_name}.id NOT IN (?)", a)}
-        scope :search, lambda {|q| where("#{klass.table_name}.uname ILIKE ?", "%#{q.strip}%")}
+        scope :search, lambda {|q| where("#{klass.table_name}.uname ILIKE ?", "%#{q.to_s.strip}%")}
 
       end
 

spec/controllers/build_lists_controller_spec.rb

@@ -10,7 +10,7 @@ describe BuildListsController do
     end
 
     it 'should be able to perform index action in project scope' do
-      get :index, :project_id => @project.id
+      get :index, :owner_name => @project.owner.uname, :project_name => @project.name
       response.should be_success
     end
   end
@@ -22,7 +22,7 @@ describe BuildListsController do
     end
 
     it 'should not be able to perform index action in project scope' do
-      get :index, :project_id => @project.id
+      get :index, :owner_name => @project.owner.uname, :project_name => @project.name
       response.should redirect_to(forbidden_url)
     end
   end
@@ -31,35 +31,35 @@ describe BuildListsController do
     before {test_git_commit(@project)}
 
     it 'should be able to perform new action' do
-      get :new, :project_id => @project.id
+      get :new, :owner_name => @project.owner.uname, :project_name => @project.name
       response.should render_template(:new)
     end
 
     it 'should be able to perform create action' do
-      post :create, {:project_id => @project.id}.merge(@create_params)
+      post :create, {:owner_name => @project.owner.uname, :project_name => @project.name}.merge(@create_params)
       response.should redirect_to project_build_lists_path(@project)
     end
 
     it 'should save correct commit_hash for branch based build' do
-      post :create, {:project_id => @project.id}.merge(@create_params).deep_merge(:build_list => {:project_version => "latest_master"})
+      post :create, {:owner_name => @project.owner.uname, :project_name => @project.name}.merge(@create_params).deep_merge(:build_list => {:project_version => "latest_master"})
       @project.build_lists.last.commit_hash.should == @project.git_repository.commits('master').last.id
     end
 
     it 'should save correct commit_hash for tag based build' do
       system("cd #{@project.git_repository.path} && git tag 4.7.5.3") # TODO REDO through grit
-      post :create, {:project_id => @project.id}.merge(@create_params).deep_merge(:build_list => {:project_version => "4.7.5.3"})
+      post :create, {:owner_name => @project.owner.uname, :project_name => @project.name}.merge(@create_params).deep_merge(:build_list => {:project_version => "4.7.5.3"})
       @project.build_lists.last.commit_hash.should == @project.git_repository.commits('4.7.5.3').last.id
     end
   end
 
   shared_examples_for 'not create build list' do
     it 'should not be able to perform new action' do
-      get :new, :project_id => @project.id
+      get :new, :owner_name => @project.owner.uname, :project_name => @project.name
       response.should redirect_to(forbidden_url)
     end
 
     it 'should not be able to perform create action' do
-      post :create, {:project_id => @project.id}.merge(@create_params)
+      post :create, {:owner_name => @project.owner.uname, :project_name => @project.name}.merge(@create_params)
       response.should redirect_to(forbidden_url)
     end
   end
@@ -107,7 +107,7 @@ describe BuildListsController do
         rel.save
         @user = FactoryGirl.create(:user)
         set_session_for(@user)
-        @show_params = {:project_id => @project.id, :id => @build_list.id}
+        @show_params = {:owner_name => @project.owner.uname, :project_name => @project.name, :id => @build_list.id}
       end
   
       context 'for all build lists' do
@@ -192,7 +192,7 @@ describe BuildListsController do
         @build_list = FactoryGirl.create(:build_list_core, :project => @project)
 
         set_session_for(@user)
-        @show_params = {:project_id => @project.id, :id => @build_list.id}
+        @show_params = {:owner_name => @project.owner.uname, :project_name => @project.name, :id => @build_list.id}
       end
   
       context 'for all build lists' do

spec/controllers/comments_controller_for_commit_spec.rb

@@ -54,23 +54,23 @@ end
 
 shared_examples_for 'user without destroy comment rights for commits' do
   it 'should not be able to perform destroy action' do
-    delete :destroy, :id => @stranger_comment.id, :commit_id => @commit.id, :project_id => @project.id
+    delete :destroy, :id => @stranger_comment.id, :commit_id => @commit.id, :owner_name => @project.owner.uname, :project_name => @project.name
     response.should redirect_to(forbidden_path)
   end
 
   it 'should not reduce comments count' do
-    lambda{ delete :destroy, :id => @stranger_comment.id, :commit_id => @commit.id, :project_id => @project.id }.should change{ Comment.count }.by(0)
+    lambda{ delete :destroy, :id => @stranger_comment.id, :commit_id => @commit.id, :owner_name => @project.owner.uname, :project_name => @project.name }.should change{ Comment.count }.by(0)
   end
 end
 
 #shared_examples_for 'user with destroy rights' do
 #  it 'should be able to perform destroy action' do
-#    delete :destroy, :id => @stranger_comment.id, :project_id => @project.id
+#    delete :destroy, :id => @stranger_comment.id, :owner_name => @project.owner.uname, :project_name => @project.name
 #    response.should redirect_to(commit_path(@project, @commit.id))
 #  end
 #
 #  it 'should reduce comments count' do
-#    lambda{ delete :destroy, :id => @stranger_comment.id, :issue_id => @issue.serial_id, :project_id => @project.id }.should change{ Comment.count }.by(-1)
+#    lambda{ delete :destroy, :id => @stranger_comment.id, :issue_id => @issue.serial_id, :owner_name => @project.owner.uname, :project_name => @project.name }.should change{ Comment.count }.by(-1)
 #  end
 #end
 
@@ -81,8 +81,8 @@ describe CommentsController do
     %x(cp -Rf #{Rails.root}/spec/tests.git/* #{@project.git_repository.path}) # maybe FIXME ?
     @commit = @project.git_repository.commits.first
 
-    @create_params = {:comment => {:body => 'I am a comment!'}, :project_id => @project.id, :commit_id => @commit.id}
-    @update_params = {:comment => {:body => 'updated'}, :project_id => @project.id, :commit_id => @commit.id}
+    @create_params = {:comment => {:body => 'I am a comment!'}, :owner_name => @project.owner.uname, :project_name => @project.name, :commit_id => @commit.id}
+    @update_params = {:comment => {:body => 'updated'}, :owner_name => @project.owner.uname, :project_name => @project.name, :commit_id => @commit.id}
 
     any_instance_of(Project, :versions => ['v1.0', 'v2.0'])
     @stranger_comment = create_comment FactoryGirl.create(:user)

spec/controllers/comments_controller_spec.rb

@@ -50,23 +50,23 @@ end
 
 shared_examples_for 'user without destroy comment rights' do
   it 'should not be able to perform destroy action' do
-    delete :destroy, :id => @comment.id, :issue_id => @issue.serial_id, :project_id => @project.id
+    delete :destroy, :id => @comment.id, :issue_id => @issue.serial_id, :owner_name => @project.owner.uname, :project_name => @project.name
     response.should redirect_to(forbidden_path)
   end
 
   it 'should not reduce comments count' do
-    lambda{ delete :destroy, :id => @comment.id, :issue_id => @issue.serial_id, :project_id => @project.id }.should change{ Issue.count }.by(0)
+    lambda{ delete :destroy, :id => @comment.id, :issue_id => @issue.serial_id, :owner_name => @project.owner.uname, :project_name => @project.name }.should change{ Issue.count }.by(0)
   end
 end
 
 #shared_examples_for 'user with destroy rights' do
 #  it 'should be able to perform destroy action' do
-#    delete :destroy, :id => @comment.id, :issue_id => @issue.id, :project_id => @project.id
+#    delete :destroy, :id => @comment.id, :issue_id => @issue.id, :owner_name => @project.owner.uname, :project_name => @project.name
 #    response.should redirect_to([@project, @issue])
 #  end
 #
 #  it 'should reduce comments count' do
-#    lambda{ delete :destroy, :id => @comment.id, :issue_id => @issue.id, :project_id => @project.id }.should change{ Comment.count }.by(-1)
+#    lambda{ delete :destroy, :id => @comment.id, :issue_id => @issue.id, :owner_name => @project.owner.uname, :project_name => @project.name }.should change{ Comment.count }.by(-1)
 #  end
 #end
 
@@ -78,8 +78,8 @@ describe CommentsController do
     @issue = FactoryGirl.create(:issue, :project_id => @project.id, :user => FactoryGirl.create(:user))
     @comment = FactoryGirl.create(:comment, :commentable => @issue, :project_id => @project.id)
 
-    @create_params = {:comment => {:body => 'I am a comment!'}, :project_id => @project.id, :issue_id => @issue.serial_id}
-    @update_params = {:comment => {:body => 'updated'}, :project_id => @project.id, :issue_id => @issue.serial_id}
+    @create_params = {:comment => {:body => 'I am a comment!'}, :owner_name => @project.owner.uname, :project_name => @project.name, :issue_id => @issue.serial_id}
+    @update_params = {:comment => {:body => 'updated'}, :owner_name => @project.owner.uname, :project_name => @project.name, :issue_id => @issue.serial_id}
 
     any_instance_of(Project, :versions => ['v1.0', 'v2.0'])
 
@@ -102,6 +102,7 @@ describe CommentsController do
   context 'for project owner user' do
     before(:each) do
       @project.update_attribute(:owner, @user)
+      @create_params[:owner_name] = @user.uname; @update_params[:owner_name] = @user.uname
     end
 
    it_should_behave_like 'user with create comment rights'

spec/controllers/issues_controller_spec.rb

@@ -3,12 +3,12 @@ require 'spec_helper'
 
 shared_examples_for 'issue user with project reader rights' do
   it 'should be able to perform index action' do
-    get :index, :project_id => @project.id
+    get :index, :owner_name => @project.owner.uname, :project_name => @project.name
     response.should render_template(:index)
   end
 
   it 'should be able to perform show action' do
-    get :show, :project_id => @project.id, :id => @issue.serial_id
+    get :show, :owner_name => @project.owner.uname, :project_name => @project.name, :id => @issue.serial_id
     response.should render_template(:show)
   end
 end
@@ -50,12 +50,12 @@ end
 
 shared_examples_for 'user without issue destroy rights' do
   it 'should not be able to perform destroy action' do
-    delete :destroy, :id => @issue.serial_id, :project_id => @project.id
+    delete :destroy, :id => @issue.serial_id, :owner_name => @project.owner.uname, :project_name => @project.name
     response.should redirect_to(controller.current_user ? forbidden_path : new_user_session_path)
   end
 
   it 'should not reduce issues count' do
-    lambda{ delete :destroy, :id => @issue.serial_id, :project_id => @project.id }.should_not change{ Issue.count }
+    lambda{ delete :destroy, :id => @issue.serial_id, :owner_name => @project.owner.uname, :project_name => @project.name }.should_not change{ Issue.count }
   end
 end
 
@@ -82,17 +82,16 @@ describe IssuesController do
 
     @issue = FactoryGirl.create(:issue, :project_id => @project.id, :assignee_id => @issue_user.id)
     @create_params = {
-      :project_id => @project.id,
+      :owner_name => @project.owner.uname, :project_name => @project.name,
       :issue => {
         :title => "issue1",
-        :body => "issue body",
-        :project_id => @project.id
+        :body => "issue body"
       },
       :assignee_id => @issue_user.id,
       :assignee_uname => @issue_user.uname
     }
     @update_params = {
-      :project_id => @project.id,
+      :owner_name => @project.owner.uname, :project_name => @project.name,
       :issue => {
         :title => "issue2"
       }
@@ -129,7 +128,7 @@ describe IssuesController do
     before(:each) do
       @user = FactoryGirl.create(:user)
       set_session_for(@user)
-      @project.update_attribute(:owner, @user)
+      @project.update_attribute(:owner, @user); @create_params[:owner_name] = @user.uname; @update_params[:owner_name] = @user.uname
       @project.relations.create!(:object_type => 'User', :object_id => @user.id, :role => 'admin')
     end
 
@@ -191,12 +190,12 @@ describe IssuesController do
       it_should_behave_like 'issue user with project reader rights'
     else
       it 'should not be able to perform index action' do
-        get :index, :project_id => @project.id
+        get :index, :owner_name => @project.owner.uname, :project_name => @project.name
         response.should redirect_to(new_user_session_path)
       end
 
       it 'should not be able to perform show action' do
-        get :show, :project_id => @project.id, :id => @issue.serial_id
+        get :show, :owner_name => @project.owner.uname, :project_name => @project.name, :id => @issue.serial_id
         response.should redirect_to(new_user_session_path)
       end
     end

spec/controllers/projects_controller_spec.rb

@@ -130,7 +130,7 @@ describe ProjectsController do
       set_session_for(@user)
       @project.update_attribute(:visibility, 'hidden')
       post :fork, :owner_name => @project.owner.uname, :project_name => @project.name
-      response.should redirect_to(@project)
+      response.should redirect_to(forbidden_path)
     end
   end
 end

spec/controllers/subscribes_controller_spec.rb

@@ -54,8 +54,8 @@ describe SubscribesController do
     @project = FactoryGirl.create(:project)
     @issue = FactoryGirl.create(:issue, :project_id => @project.id)
 
-    @create_params = {:issue_id => @issue.serial_id, :project_id => @project.id}
-    @destroy_params = {:issue_id => @issue.serial_id, :project_id => @project.id}
+    @create_params = {:issue_id => @issue.serial_id, :owner_name => @project.owner.uname, :project_name => @project.name}
+    @destroy_params = {:issue_id => @issue.serial_id, :owner_name => @project.owner.uname, :project_name => @project.name}
 
     any_instance_of(Project, :versions => ['v1.0', 'v2.0'])