diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb index 1fcadbfe..695e8cb8 100644 --- a/app/controllers/dashboard_controller.rb +++ b/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) diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 6646b10c..981adf06 100644 --- a/app/controllers/groups_controller.rb +++ b/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 diff --git a/app/models/project.rb b/app/models/project.rb index 5d0cdd64..f0c70f0d 100644 --- a/app/models/project.rb +++ b/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 diff --git a/app/models/user.rb b/app/models/user.rb index ae0680b7..cebbfcda 100644 --- a/app/models/user.rb +++ b/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 diff --git a/app/roles/account.rb b/app/roles/account.rb index 7596c833..42e3243d 100644 --- a/app/roles/account.rb +++ b/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 diff --git a/app/roles/namespaced_project.rb b/app/roles/namespaced_project.rb index 1c10c8f7..0f9fb051 100644 --- a/app/roles/namespaced_project.rb +++ b/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 diff --git a/app/views/admin/projects/show.html.haml b/app/views/admin/projects/show.html.haml index 634b1836..ca9b9d3d 100644 --- a/app/views/admin/projects/show.html.haml +++ b/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 diff --git a/app/views/dashboard/_groups.html.haml b/app/views/dashboard/_groups.html.haml index 9e3401e5..5a95ab3f 100644 --- a/app/views/dashboard/_groups.html.haml +++ 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 diff --git a/db/migrate/20130102143055_rename_owner_to_creator_for_project.rb b/db/migrate/20130102143055_rename_owner_to_creator_for_project.rb new file mode 100644 index 00000000..d0fca269 --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index 7de55932..b1cf0ccb 100644 --- a/db/schema.rb +++ b/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 diff --git a/spec/factories.rb b/spec/factories.rb index abc0d374..86c101cc 100644 --- a/spec/factories.rb +++ b/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 diff --git a/spec/models/project_security_spec.rb b/spec/models/project_security_spec.rb index 92c6bce0..1f2bd7a5 100644 --- a/spec/models/project_security_spec.rb +++ b/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 diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 83a76976..27e68ce1 100644 --- a/spec/models/project_spec.rb +++ b/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 diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index d09484f8..eb2717e3 100644 --- a/spec/models/user_spec.rb +++ b/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 diff --git a/spec/roles/account_role_spec.rb b/spec/roles/account_role_spec.rb index 4b214551..f7a128d0 100644 --- a/spec/roles/account_role_spec.rb +++ b/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