Project has now correct owner and creator. Increased test coverage

app/controllers/dashboard_controller.rb

@@ -20,7 +20,7 @@ class DashboardController < ApplicationController
 
     @projects = @projects.page(params[:page]).per(30)
 
-    @events = Event.in_projects(current_user.project_ids)
+    @events = Event.in_projects(current_user.authorized_projects.pluck(:id))
     @events = @event_filter.apply_filter(@events)
     @events = @events.limit(20).offset(params[:offset] || 0)
 

app/controllers/groups_controller.rb

@@ -70,7 +70,7 @@ class GroupsController < ApplicationController
   end
 
   def projects
-    @projects ||= group.projects.authorized_for(current_user).sorted_by_activity
+    @projects ||= current_user.authorized_projects.where(namespace_id: group.id).sorted_by_activity
   end
 
   def project_ids

app/models/project.rb

@@ -32,7 +32,7 @@ class Project < ActiveRecord::Base
   attr_accessible :name, :path, :description, :default_branch, :issues_enabled,
                   :wall_enabled, :merge_requests_enabled, :wiki_enabled, as: [:default, :admin]
 
-  attr_accessible :namespace_id, :owner_id, as: :admin
+  attr_accessible :namespace_id, :creator_id, as: :admin
 
   attr_accessor :error_code
 
@@ -40,10 +40,10 @@ class Project < ActiveRecord::Base
   belongs_to :group, foreign_key: "namespace_id", conditions: "type = 'Group'"
   belongs_to :namespace
 
-  # TODO: replace owner with creator.
-  # With namespaces a project owner will be a namespace owner
-  # so this field makes sense only for global projects
-  belongs_to :owner, class_name: "User"
+  belongs_to :creator,
+    class_name: "User",
+    foreign_key: "creator_id"
+
   has_many :users,          through: :users_projects
   has_many :events,         dependent: :destroy
   has_many :merge_requests, dependent: :destroy
@@ -62,7 +62,7 @@ class Project < ActiveRecord::Base
   delegate :name, to: :owner, allow_nil: true, prefix: true
 
   # Validations
-  validates :owner, presence: true
+  validates :creator, presence: true
   validates :description, length: { within: 0..2000 }
   validates :name, presence: true, length: { within: 0..255 },
             format: { with: Gitlab::Regex.project_name_regex,
@@ -89,8 +89,7 @@ class Project < ActiveRecord::Base
 
   class << self
     def authorized_for user
-      projects = includes(:users_projects, :namespace)
-      projects = projects.where("users_projects.user_id = :user_id or projects.owner_id = :user_id or namespaces.owner_id = :user_id", user_id: user.id)
+      raise "DERECATED"
     end
 
     def active
@@ -104,8 +103,10 @@ class Project < ActiveRecord::Base
     def find_with_namespace(id)
       if id.include?("/")
         id = id.split("/")
-        namespace_id = Namespace.find_by_path(id.first).id
-        where(namespace_id: namespace_id).find_by_path(id.second)
+        namespace = Namespace.find_by_path(id.first)
+        return nil unless namespace
+
+        where(namespace_id: namespace.id).find_by_path(id.second)
       else
         where(path: id, namespace_id: nil).last
       end
@@ -125,7 +126,7 @@ class Project < ActiveRecord::Base
         #
         project.path = project.name.dup.parameterize
 
-        project.owner = user
+        project.creator = user
 
         # Apply namespace if user has access to it
         # else fallback to user namespace
@@ -174,8 +175,8 @@ class Project < ActiveRecord::Base
   end
 
   def check_limit
-    unless owner.can_create_project?
-      errors[:base] << ("Your own projects limit is #{owner.projects_limit}! Please contact administrator to increase it")
+    unless creator.can_create_project?
+      errors[:base] << ("Your own projects limit is #{creator.projects_limit}! Please contact administrator to increase it")
     end
   rescue
     errors[:base] << ("Can't check your ability to create project")
@@ -268,4 +269,12 @@ class Project < ActiveRecord::Base
       Notify.project_was_moved_email(member.id).deliver
     end
   end
+
+  def owner
+    if namespace
+      namespace_owner
+    else
+      creator
+    end
+  end
 end

app/models/user.rb

@@ -51,7 +51,6 @@ class User < ActiveRecord::Base
   has_many :groups, class_name: "Group", foreign_key: :owner_id
 
   has_many :keys, dependent: :destroy
-  has_many :projects, through: :users_projects
   has_many :users_projects, dependent: :destroy
   has_many :issues, foreign_key: :author_id, dependent: :destroy
   has_many :notes, foreign_key: :author_id, dependent: :destroy
@@ -82,6 +81,9 @@ class User < ActiveRecord::Base
   scope :active, where(blocked:  false)
   scope :alphabetically, order('name ASC')
 
+  #
+  # Class methods
+  #
   class << self
     def filter filter_name
       case filter_name
@@ -126,9 +128,63 @@ class User < ActiveRecord::Base
     end
   end
 
+  #
+  # Instance methods
+  #
   def generate_password
     if self.force_random_password
       self.password = self.password_confirmation = Devise.friendly_token.first(8)
     end
   end
+
+
+  # Namespaces user has access to
+  def namespaces
+    namespaces = []
+
+    # Add user account namespace
+    namespaces << self.namespace if self.namespace
+
+    # Add groups you can manage
+    namespaces += if admin
+                    Group.all
+                  else
+                    groups.all
+                  end
+    namespaces
+  end
+
+  # Groups where user is an owner
+  def owned_groups
+    groups
+  end
+
+  # Groups user has access to
+  def authorized_groups
+    @authorized_groups ||= begin
+                           groups = Group.where(id: self.authorized_projects.pluck(:namespace_id)).all
+                           groups = groups + self.groups
+                           groups.uniq
+                         end
+  end
+
+
+  # Projects user has access to
+  def authorized_projects
+    project_ids = users_projects.pluck(:project_id)
+    project_ids = project_ids | owned_projects.pluck(:id)
+    Project.where(id: project_ids)
+  end
+
+  # Projects in user namespace
+  def personal_projects
+    Project.personal(self)
+  end
+
+  # Projects where user is an owner
+  def owned_projects
+    Project.where("(projects.namespace_id IN (:namespaces)) OR
+                  (projects.namespace_id IS NULL AND projects.creator_id = :user_id)",
+                  namespaces: namespaces.map(&:id), user_id: self.id)
+  end
 end

app/roles/account.rb

@@ -25,7 +25,7 @@ module Account
   end
 
   def can_create_project?
-    projects_limit > my_own_projects.count
+    projects_limit > personal_projects.count
   end
 
   def can_create_group?
@@ -56,10 +56,6 @@ module Account
     MergeRequest.where("author_id = :id or assignee_id = :id", id: self.id)
   end
 
-  def project_ids
-    projects.map(&:id)
-  end
-
   # Remove user from all projects and
   # set blocked attribute to true
   def block
@@ -86,22 +82,7 @@ module Account
   end
 
   def projects_sorted_by_activity
-    projects.sorted_by_activity
-  end
-
-  def namespaces
-    namespaces = []
-
-    # Add user account namespace
-    namespaces << self.namespace if self.namespace
-
-    # Add groups you can manage
-    namespaces += if admin
-                    Group.all
-                  else
-                    groups.all
-                  end
-    namespaces
+    authorized_projects.sorted_by_activity
   end
 
   def several_namespaces?
@@ -111,20 +92,4 @@ module Account
   def namespace_id
     namespace.try :id
   end
-
-  def authorized_groups
-    @authorized_groups ||= begin
-                           groups = Group.where(id: self.projects.pluck(:namespace_id)).all
-                           groups = groups + self.groups
-                           groups.uniq
-                         end
-  end
-
-  def authorized_projects
-    Project.authorized_for(self)
-  end
-
-  def my_own_projects
-    Project.personal(self)
-  end
 end

app/roles/namespaced_project.rb

@@ -50,14 +50,6 @@ module NamespacedProject
     namespace.try(:owner)
   end
 
-  def chief
-    if namespace
-      namespace_owner
-    else
-      owner
-    end
-  end
-
   def path_with_namespace
     if namespace
       namespace.path + '/' + path

app/views/admin/projects/show.html.haml

@@ -49,8 +49,8 @@
       %b
         Owned by:
     %td
-      - if @project.chief
-        = link_to @project.chief.name, admin_user_path(@project.chief)
+      - if @project.owner
+        = link_to @project.owner_name, admin_user_path(@project.owner)
       - else
         (deleted)
   %tr
@@ -58,7 +58,7 @@
       %b
         Created by:
     %td
-      = @project.owner_name || '(deleted)'
+      = @project.creator.try(:name) || '(deleted)'
   %tr
     %td
       %b

app/views/dashboard/_groups.html.haml

@@ -17,4 +17,4 @@
             →
           %span.last_activity
             %strong Projects:
-            %span= group.projects.authorized_for(current_user).count
+            %span= current_user.authorized_projects.where(namespace_id: group.id).count

db/migrate/20130102143055_rename_owner_to_creator_for_project.rb

@@ -0,0 +1,5 @@
+class RenameOwnerToCreatorForProject < ActiveRecord::Migration
+  def change
+    rename_column :projects, :owner_id, :creator_id
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended to check this file into your version control system.
 
-ActiveRecord::Schema.define(:version => 20121219095402) do
+ActiveRecord::Schema.define(:version => 20130102143055) do
 
   create_table "events", :force => true do |t|
     t.string   "target_type"
@@ -148,7 +148,7 @@ ActiveRecord::Schema.define(:version => 20121219095402) do
     t.datetime "created_at",                               :null => false
     t.datetime "updated_at",                               :null => false
     t.boolean  "private_flag",           :default => true, :null => false
-    t.integer  "owner_id"
+    t.integer  "creator_id"
     t.string   "default_branch"
     t.boolean  "issues_enabled",         :default => true, :null => false
     t.boolean  "wall_enabled",           :default => true, :null => false
@@ -157,8 +157,8 @@ ActiveRecord::Schema.define(:version => 20121219095402) do
     t.integer  "namespace_id"
   end
 
+  add_index "projects", ["creator_id"], :name => "index_projects_on_owner_id"
   add_index "projects", ["namespace_id"], :name => "index_projects_on_namespace_id"
-  add_index "projects", ["owner_id"], :name => "index_projects_on_owner_id"
 
   create_table "protected_branches", :force => true do |t|
     t.integer  "project_id", :null => false

spec/factories.rb

@@ -9,7 +9,7 @@ FactoryGirl.define do
 
   sequence(:url) { Faker::Internet.uri('http') }
 
-  factory :user, aliases: [:author, :assignee, :owner] do
+  factory :user, aliases: [:author, :assignee, :owner, :creator] do
     email { Faker::Internet.email }
     name
     username { Faker::Internet.user_name }
@@ -26,7 +26,7 @@ FactoryGirl.define do
   factory :project do
     sequence(:name) { |n| "project#{n}" }
     path { name.downcase.gsub(/\s/, '_') }
-    owner
+    creator
   end
 
   factory :group do

spec/models/project_security_spec.rb

@@ -8,7 +8,7 @@ describe Project do
       @u1 = create(:user)
       @u2 = create(:user)
       @u3 = create(:user)
-      @u4 = @p1.chief
+      @u4 = @p1.owner
 
       @abilities = Six.new
       @abilities << Ability

spec/models/project_spec.rb

@@ -24,7 +24,7 @@ describe Project do
   describe "Associations" do
     it { should belong_to(:group) }
     it { should belong_to(:namespace) }
-    it { should belong_to(:owner).class_name('User') }
+    it { should belong_to(:creator).class_name('User') }
     it { should have_many(:users) }
     it { should have_many(:events).dependent(:destroy) }
     it { should have_many(:merge_requests).dependent(:destroy) }
@@ -41,7 +41,7 @@ describe Project do
 
   describe "Mass assignment" do
     it { should_not allow_mass_assignment_of(:namespace_id) }
-    it { should_not allow_mass_assignment_of(:owner_id) }
+    it { should_not allow_mass_assignment_of(:creator_id) }
     it { should_not allow_mass_assignment_of(:private_flag) }
   end
 
@@ -55,20 +55,15 @@ describe Project do
     it { should validate_presence_of(:path) }
     it { should validate_uniqueness_of(:path) }
     it { should ensure_length_of(:path).is_within(0..255) }
-    # TODO: Formats
-
     it { should ensure_length_of(:description).is_within(0..2000) }
-
-    # TODO: Formats
-
-    it { should validate_presence_of(:owner) }
+    it { should validate_presence_of(:creator) }
     it { should ensure_inclusion_of(:issues_enabled).in_array([true, false]) }
     it { should ensure_inclusion_of(:wall_enabled).in_array([true, false]) }
     it { should ensure_inclusion_of(:merge_requests_enabled).in_array([true, false]) }
     it { should ensure_inclusion_of(:wiki_enabled).in_array([true, false]) }
 
     it "should not allow new projects beyond user limits" do
-      project.stub(:owner).and_return(double(can_create_project?: false, projects_limit: 1))
+      project.stub(:creator).and_return(double(can_create_project?: false, projects_limit: 1))
       project.should_not be_valid
       project.errors[:base].first.should match(/Your own projects limit is 1/)
     end
@@ -134,7 +129,7 @@ describe Project do
     it { should respond_to(:transfer) }
     it { should respond_to(:name_with_namespace) }
     it { should respond_to(:namespace_owner) }
-    it { should respond_to(:chief) }
+    it { should respond_to(:owner) }
     it { should respond_to(:path_with_namespace) }
   end
 
@@ -211,4 +206,75 @@ describe Project do
       @merge_request.last_commit.id.should == "bcf03b5de6c33f3869ef70d68cf06e679d1d7f9a"
     end
   end
+
+  describe :create_by_user do
+    before do
+      @user = create :user
+      @opts = {
+        name: "GitLab"
+      }
+    end
+
+    context 'user namespace' do
+      before do
+        @project = Project.create_by_user(@opts, @user)
+      end
+
+      it { @project.should be_valid }
+      it { @project.owner.should == @user }
+      it { @project.namespace.should == @user.namespace }
+    end
+
+    context 'user namespace' do
+      before do
+        @group = create :group, owner: @user
+        @opts.merge!(namespace_id: @group.id)
+        @project = Project.create_by_user(@opts, @user)
+      end
+
+      it { @project.should be_valid }
+      it { @project.owner.should == @user }
+      it { @project.namespace.should == @group }
+    end
+  end
+
+  describe :find_with_namespace do
+    context 'with namespace' do
+      before do
+        @group = create :group, name: 'gitlab'
+        @project = create(:project, name: 'gitlab-ci', namespace: @group)
+      end
+
+      it { Project.find_with_namespace('gitlab/gitlab-ci').should == @project }
+      it { Project.find_with_namespace('gitlab-ci').should be_nil }
+    end
+
+    context 'w/o namespace' do
+      before do
+        @project = create(:project, name: 'gitlab-ci')
+      end
+
+      it { Project.find_with_namespace('gitlab-ci').should == @project }
+      it { Project.find_with_namespace('gitlab/gitlab-ci').should be_nil }
+    end
+  end
+
+  describe :to_param do
+    context 'with namespace' do
+      before do
+        @group = create :group, name: 'gitlab'
+        @project = create(:project, name: 'gitlab-ci', namespace: @group)
+      end
+
+      it { @project.to_param.should == "gitlab/gitlab-ci" }
+    end
+
+    context 'w/o namespace' do
+      before do
+        @project = create(:project, name: 'gitlab-ci')
+      end
+
+      it { @project.to_param.should == "gitlab-ci" }
+    end
+  end
 end

spec/models/user_spec.rb

@@ -39,7 +39,6 @@ describe User do
   describe "Associations" do
     it { should have_one(:namespace) }
     it { should have_many(:users_projects).dependent(:destroy) }
-    it { should have_many(:projects) }
     it { should have_many(:groups) }
     it { should have_many(:keys).dependent(:destroy) }
     it { should have_many(:events).class_name('Event').dependent(:destroy) }
@@ -119,4 +118,71 @@ describe User do
       user.authentication_token.should_not be_blank
     end
   end
+
+  describe 'projects' do
+    before do
+      ActiveRecord::Base.observers.enable(:user_observer)
+      @user = create :user
+      @project = create :project, namespace: @user.namespace
+    end
+
+    it { @user.authorized_projects.should include(@project) }
+    it { @user.owned_projects.should include(@project) }
+    it { @user.personal_projects.should include(@project) }
+  end
+
+  describe 'groups' do
+    before do
+      ActiveRecord::Base.observers.enable(:user_observer)
+      @user = create :user
+      @group = create :group, owner: @user
+    end
+
+    it { @user.several_namespaces?.should be_true }
+    it { @user.namespaces.should == [@user.namespace, @group] }
+    it { @user.authorized_groups.should == [@group] }
+    it { @user.owned_groups.should == [@group] }
+  end
+
+  describe 'namespaced' do
+    before do
+      ActiveRecord::Base.observers.enable(:user_observer)
+      @user = create :user
+      @project = create :project, namespace: @user.namespace
+    end
+
+    it { @user.several_namespaces?.should be_false }
+    it { @user.namespaces.should == [@user.namespace] }
+  end
+
+  describe 'blocking user' do
+    let(:user) { create(:user, name: 'John Smith') }
+
+    it "should block user" do
+      user.block
+      user.blocked.should be_true
+    end
+  end
+
+  describe 'filter' do
+    before do
+      @user = create :user
+      @admin = create :user, admin: true
+      @blocked = create :user, blocked: true
+    end
+
+    it { User.filter("admins").should == [@admin] }
+    it { User.filter("blocked").should == [@blocked] }
+    it { User.filter("wop").should == [@user, @admin, @blocked] }
+    it { User.filter(nil).should == [@user, @admin] }
+  end
+
+  describe :not_in_project do
+    before do
+      @user = create :user
+      @project = create :project
+    end
+
+    it { User.not_in_project(@project).should == [@user, @project.owner] }
+  end
 end

spec/roles/account_role_spec.rb

@@ -10,35 +10,4 @@ describe User, "Account" do
     it { user.can_create_project?.should be_true }
     it { user.first_name.should == 'John' }
   end
-
-  describe 'blocking user' do
-    let(:user) { create(:user, name: 'John Smith') }
-
-    it "should block user" do
-      user.block
-      user.blocked.should be_true
-    end
-  end
-
-  describe 'projects' do
-    before do
-      ActiveRecord::Base.observers.enable(:user_observer)
-      @user = create :user
-      @project = create :project, namespace: @user.namespace
-    end
-
-    it { @user.authorized_projects.should include(@project) }
-    it { @user.my_own_projects.should include(@project) }
-  end
-
-  describe 'namespaced' do
-    before do
-      ActiveRecord::Base.observers.enable(:user_observer)
-      @user = create :user
-      @project = create :project, namespace: @user.namespace
-    end
-
-    it { @user.several_namespaces?.should be_false }
-    it { @user.namespaces.should == [@user.namespace] }
-  end
 end