Merge branch 'feature/refactoring_scopes_pr' of https://github.com/Undev/gitlabh…

…q into Undev-feature/refactoring_scopes_pr Conflicts: db/schema.rb

Merge branch 'feature/refactoring_scopes_pr' of https://github.com/Undev/gitlabh…
…q into Undev-feature/refactoring_scopes_pr Conflicts: db/schema.rb
Dmitriy Zaporozhets
2 parents 52cd655f 85e03904
Showing 12 changed files with 73 additions and 112 deletions Show diff stats
app/controllers/admin/users_controller.rb
app/models/group.rb
app/models/issue.rb
app/models/key.rb
app/models/milestone.rb
app/models/project.rb
app/models/user.rb
app/observers/project_activity_cache_observer.rb
config/application.rb
db/migrate/20130403003950_add_last_activity_column_into_project.rb
db/schema.rb
spec/models/project_spec.rb
@@ -14,7 +14,7 @@ class Admin::UsersController &lt; Admin::ApplicationController
     @not_in_projects = @not_in_projects.without_user(admin_user) if admin_user.authorized_projects.present?
     # Projects he already own or joined
-    @projects = admin_user.authorized_projects.where('projects.id in (?)', admin_user.authorized_projects.map(&:id))
+    @projects = admin_user.authorized_projects
   end
   def team_update
@@ -13,6 +13,7 @@
 #
 class Group < Namespace
+
   def add_users_to_project_teams(user_ids, project_access)
     UsersProject.add_users_into_projects(
       projects.map(&:id),
@@ -25,19 +25,9 @@ class Issue &lt; ActiveRecord::Base
   acts_as_taggable_on :labels
-  class << self
-    def cared(user)
-      where('assignee_id = :user', user: user.id)
-    end
-
-    def authored(user)
-      where('author_id = :user', user: user.id)
-    end
-
-    def open_for(user)
-      opened.assigned(user)
-    end
-  end
+  scope :cared, ->(user) { where(assignee_id: user) }
+  scope :authored, ->(user) { where(author_id: user) }
+  scope :open_for, ->(user) { opened.assigned(user) }
   state_machine :state, initial: :opened do
     event :close do
@@ -23,7 +23,7 @@ class Key &lt; ActiveRecord::Base
   before_validation :strip_white_space
   validates :title, presence: true, length: { within: 0..255 }
-  validates :key, presence: true, length: { within: 0..5000 }, format: { :with => /ssh-.{3} / }, uniqueness: true
+  validates :key, presence: true, length: { within: 0..5000 }, format: { with: /ssh-.{3} / }, uniqueness: true
   validate :fingerprintable_key
   delegate :name, :email, to: :user, prefix: true
@@ -48,7 +48,7 @@ class Key &lt; ActiveRecord::Base
   end
   def is_deploy_key
-    !!project_id
+    project.present?
   end
   # projects that has this key
@@ -19,6 +19,7 @@ class Milestone &lt; ActiveRecord::Base
   belongs_to :project
   has_many :issues
   has_many :merge_requests
+  has_many :participants, through: :issues, source: :assignee
   scope :active, -> { with_state(:active) }
   scope :closed, -> { with_state(:closed) }
@@ -48,10 +49,6 @@ class Milestone &lt; ActiveRecord::Base
     end
   end
-  def participants
-    User.where(id: issues.pluck(:assignee_id))
-  end
-
   def open_items_count
     self.issues.opened.count + self.merge_requests.opened.count
   end
@@ -30,7 +30,7 @@ class Project &lt; ActiveRecord::Base
   attr_accessible :name, :path, :description, :default_branch, :issues_tracker,
     :issues_enabled, :wall_enabled, :merge_requests_enabled, :snippets_enabled, :issues_tracker_id,
-    :wiki_enabled, :public, :import_url, as: [:default, :admin]
+    :wiki_enabled, :public, :import_url, :last_activity_at, as: [:default, :admin]
   attr_accessible :namespace_id, :creator_id, as: :admin
@@ -87,17 +87,18 @@ class Project &lt; ActiveRecord::Base
   validate :check_limit, :repo_name
   # Scopes
-  scope :without_user, ->(user)  { where("id NOT IN (:ids)", ids: user.authorized_projects.map(&:id) ) }
-  scope :not_in_group, ->(group) { where("id NOT IN (:ids)", ids: group.project_ids ) }
-  scope :without_team, ->(team) { team.projects.present? ? where("id NOT IN (:ids)", ids: team.projects.map(&:id)) : scoped  }
-  scope :in_team, ->(team) { where("id IN (:ids)", ids: team.projects.map(&:id)) }
+  scope :without_user, ->(user)  { where("projects.id NOT IN (:ids)", ids: user.authorized_projects.map(&:id) ) }
+  scope :without_team, ->(team) { team.projects.present? ? where("projects.id NOT IN (:ids)", ids: team.projects.map(&:id)) : scoped  }
+  scope :not_in_group, ->(group) { where("projects.id NOT IN (:ids)", ids: group.project_ids ) }
+  scope :in_team, ->(team) { where("projects.id IN (:ids)", ids: team.projects.map(&:id)) }
   scope :in_namespace, ->(namespace) { where(namespace_id: namespace.id) }
-  scope :sorted_by_activity, ->() { order("(SELECT max(events.created_at) FROM events WHERE events.project_id = projects.id) DESC") }
+  scope :in_group_namespace, -> { joins(:group) }
+  scope :sorted_by_activity, -> { order("projects.last_activity_at DESC") }
   scope :personal, ->(user) { where(namespace_id: user.namespace_id) }
   scope :joined, ->(user) { where("namespace_id != ?", user.namespace_id) }
   scope :public_only, -> { where(public: true) }
-  enumerize :issues_tracker, :in => (Gitlab.config.issues_tracker.keys).append(:gitlab), :default => :gitlab
+  enumerize :issues_tracker, in: (Gitlab.config.issues_tracker.keys).append(:gitlab), default: :gitlab
   class << self
     def abandoned
@@ -190,7 +191,7 @@ class Project &lt; ActiveRecord::Base
   end
   def last_activity_date
-    last_event.try(:created_at) || updated_at
+    last_activity_at || updated_at
   end
   def project_id
@@ -59,11 +59,10 @@ class User &lt; ActiveRecord::Base
   #
   # Namespace for personal projects
-  has_one :namespace,
-    dependent: :destroy,
-    foreign_key: :owner_id,
-    class_name: "Namespace",
-    conditions: 'type IS NULL'
+  has_one :namespace, dependent: :destroy, foreign_key: :owner_id, class_name: "Namespace", conditions: 'type IS NULL'
+
+  # Namespaces (owned groups and own namespace)
+  has_many :namespaces, foreign_key: :owner_id
   # Profile
   has_many :keys, dependent: :destroy
@@ -72,15 +71,11 @@ class User &lt; ActiveRecord::Base
   has_many :groups, class_name: "Group", foreign_key: :owner_id
   # Teams
-  has_many :own_teams,
-    class_name: "UserTeam",
-    foreign_key: :owner_id,
-    dependent: :destroy
-
-  has_many :user_team_user_relationships, dependent: :destroy
-  has_many :user_teams, through: :user_team_user_relationships
+  has_many :own_teams,                       dependent: :destroy, class_name: "UserTeam", foreign_key: :owner_id
+  has_many :user_team_user_relationships,    dependent: :destroy
+  has_many :user_teams,                      through: :user_team_user_relationships
   has_many :user_team_project_relationships, through: :user_teams
-  has_many :team_projects, through: :user_team_project_relationships
+  has_many :team_projects,                   through: :user_team_project_relationships
   # Projects
   has_many :users_projects,           dependent: :destroy
@@ -88,14 +83,14 @@ class User &lt; ActiveRecord::Base
   has_many :notes,                    dependent: :destroy, foreign_key: :author_id
   has_many :merge_requests,           dependent: :destroy, foreign_key: :author_id
   has_many :events,                   dependent: :destroy, foreign_key: :author_id,   class_name: "Event"
+  has_many :recent_events,                                 foreign_key: :author_id,   class_name: "Event", order: "id DESC"
   has_many :assigned_issues,          dependent: :destroy, foreign_key: :assignee_id, class_name: "Issue"
   has_many :assigned_merge_requests,  dependent: :destroy, foreign_key: :assignee_id, class_name: "MergeRequest"
-  has_many :projects, through: :users_projects
-  has_many :recent_events,
-    class_name: "Event",
-    foreign_key: :author_id,
-    order: "id DESC"
+  has_many :personal_projects,        through: :namespace, source: :projects
+  has_many :projects,                 through: :users_projects
+  has_many :own_projects,             foreign_key: :creator_id
+  has_many :owned_projects,           through: :namespaces, source: :projects
   #
   # Validations
@@ -109,9 +104,7 @@ class User &lt; ActiveRecord::Base
             format: { with: Gitlab::Regex.username_regex,
                       message: "only letters, digits & '_' '-' '.' allowed. Letter should be first" }
-  validates :notification_level,
-    inclusion: { in: Notification.notification_levels },
-    presence: true
+  validates :notification_level, inclusion: { in: Notification.notification_levels }, presence: true
   validate :namespace_uniq, if: ->(user) { user.username_changed? }
@@ -145,6 +138,9 @@ class User &lt; ActiveRecord::Base
   scope :alphabetically, -> { order('name ASC') }
   scope :in_team, ->(team){ where(id: team.member_ids) }
   scope :not_in_team, ->(team){ where('users.id NOT IN (:ids)', ids: team.member_ids) }
+  scope :not_in_project, ->(project) { project.users.present? ? where("id not in (:ids)", ids: project.users.map(&:id) ) : scoped }
+  scope :without_projects, -> { where('id NOT IN (SELECT DISTINCT(user_id) FROM users_projects)') }
+
   scope :potential_team_members, ->(team) { team.members.any? ? active.not_in_team(team) : active  }
   #
@@ -171,18 +167,6 @@ class User &lt; ActiveRecord::Base
       end
     end
-    def not_in_project(project)
-      if project.users.present?
-        where("id not in (:ids)", ids: project.users.map(&:id) )
-      else
-        scoped
-      end
-    end
-
-    def without_projects
-      where('id NOT IN (SELECT DISTINCT(user_id) FROM users_projects)')
-    end
-
     def create_from_omniauth(auth, ldap = false)
       gitlab_auth.create_from_omniauth(auth, ldap)
     end
@@ -229,56 +213,36 @@ class User &lt; ActiveRecord::Base
     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 += groups.all
-
-    namespaces
-  end
-
   # Groups where user is an owner
   def owned_groups
     groups
   end
+  def owned_teams
+    own_teams
+  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
+    @group_ids ||= (groups.pluck(:id) + authorized_projects.pluck(:namespace_id))
+    Group.where(id: @group_ids)
   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)
+    @project_ids ||= (owned_projects.pluck(:id) + projects.pluck(:id)).uniq
+    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)
+  def authorized_teams
+    @team_ids ||= (user_teams.pluck(:id) + own_teams.pluck(:id)).uniq
+    UserTeam.where(id: @team_ids)
   end
   # Team membership in authorized projects
   def tm_in_authorized_projects
-    UsersProject.where(project_id:  authorized_projects.map(&:id), user_id: self.id)
+    UsersProject.where(project_id: authorized_projects.map(&:id), user_id: self.id)
   end
   def is_admin?
@@ -348,28 +312,13 @@ class User &lt; ActiveRecord::Base
   end
   def several_namespaces?
-    namespaces.size > 1
+    namespaces.many?
   end
   def namespace_id
     namespace.try :id
   end
-  def authorized_teams
-    @authorized_teams ||= begin
-                            ids = []
-                            ids << UserTeam.with_member(self).pluck('user_teams.id')
-                            ids << UserTeam.created_by(self).pluck('user_teams.id')
-                            ids.flatten
-
-                            UserTeam.where(id: ids)
-                          end
-  end
-
-  def owned_teams
-    UserTeam.where(owner_id: self.id)
-  end
-
   def name_with_username
     "#{name} (#{username})"
   end
@@ -0,0 +1,8 @@
+class ProjectActivityCacheObserver < BaseObserver
+  observe :event
+
+  def after_create(event)
+    event.project.update_attribute(:last_activity_at, event.created_at) if event.project
+  end
+end
+
@@ -24,6 +24,7 @@ module Gitlab
     # Activate observers that should always be running.
     config.active_record.observers = :activity_observer,
+                                     :project_activity_cache_observer,
                                      :issue_observer,
                                      :key_observer,
                                      :merge_request_observer,
@@ -0,0 +1,15 @@
+class AddLastActivityColumnIntoProject < ActiveRecord::Migration
+  def up
+    add_column :projects, :last_activity_at, :datetime
+    add_index :projects, :last_activity_at
+
+    Project.find_each do |project|
+      project.update_attribute(:last_activity_at, project.last_activity_date)
+    end
+  end
+
+  def down
+    remove_index :projects, :last_activity_at
+    remove_column :projects, :last_activity_at
+  end
+end
@@ -12,7 +12,6 @@
 # It's strongly recommended to check this file into your version control system.
 ActiveRecord::Schema.define(:version => 20130404164628) do
-
   create_table "events", :force => true do |t|
     t.string   "target_type"
     t.integer  "target_id"
@@ -109,8 +109,8 @@ describe Project do
     describe 'last_activity_date' do
       it 'returns the creation date of the project\'s last event if present' do
-        project.stub(last_event: last_event)
-        project.last_activity_date.should == last_event.created_at
+        last_activity_event = create(:event, project: project)
+        project.last_activity_date.to_s(:db).should == last_event.created_at.to_s(:db)
       end
       it 'returns the project\'s last update date if it has no events' do
	@@ -14,7 +14,7 @@ class Admin::UsersController < Admin::ApplicationController		@@ -14,7 +14,7 @@ class Admin::UsersController < Admin::ApplicationController
14	@not_in_projects = @not_in_projects.without_user(admin_user) if admin_user.authorized_projects.present?	14	@not_in_projects = @not_in_projects.without_user(admin_user) if admin_user.authorized_projects.present?
15		15
16	# Projects he already own or joined	16	# Projects he already own or joined
17	- @projects = admin_user.authorized_projects.where('projects.id in (?)', admin_user.authorized_projects.map(&:id))	17	+ @projects = admin_user.authorized_projects
18	end	18	end
19		19
20	def team_update	20	def team_update
	@@ -13,6 +13,7 @@		@@ -13,6 +13,7 @@
13	#	13	#
14		14
15	class Group < Namespace	15	class Group < Namespace
		16	+
16	def add_users_to_project_teams(user_ids, project_access)	17	def add_users_to_project_teams(user_ids, project_access)
17	UsersProject.add_users_into_projects(	18	UsersProject.add_users_into_projects(
18	projects.map(&:id),	19	projects.map(&:id),
	@@ -25,19 +25,9 @@ class Issue < ActiveRecord::Base		@@ -25,19 +25,9 @@ class Issue < ActiveRecord::Base
25		25
26	acts_as_taggable_on :labels	26	acts_as_taggable_on :labels
27		27
28	- class << self
29	- def cared(user)
30	- where('assignee_id = :user', user: user.id)
31	- end
32	-
33	- def authored(user)
34	- where('author_id = :user', user: user.id)
35	- end
36	-
37	- def open_for(user)
38	- opened.assigned(user)
39	- end
40	- end	28	+ scope :cared, ->(user) { where(assignee_id: user) }
		29	+ scope :authored, ->(user) { where(author_id: user) }
		30	+ scope :open_for, ->(user) { opened.assigned(user) }
41		31
42	state_machine :state, initial: :opened do	32	state_machine :state, initial: :opened do
43	event :close do	33	event :close do
	@@ -23,7 +23,7 @@ class Key < ActiveRecord::Base		@@ -23,7 +23,7 @@ class Key < ActiveRecord::Base
23	before_validation :strip_white_space	23	before_validation :strip_white_space
24		24
25	validates :title, presence: true, length: { within: 0..255 }	25	validates :title, presence: true, length: { within: 0..255 }
26	- validates :key, presence: true, length: { within: 0..5000 }, format: { :with => /ssh-.{3} / }, uniqueness: true	26	+ validates :key, presence: true, length: { within: 0..5000 }, format: { with: /ssh-.{3} / }, uniqueness: true
27	validate :fingerprintable_key	27	validate :fingerprintable_key
28		28
29	delegate :name, :email, to: :user, prefix: true	29	delegate :name, :email, to: :user, prefix: true
	@@ -48,7 +48,7 @@ class Key < ActiveRecord::Base		@@ -48,7 +48,7 @@ class Key < ActiveRecord::Base
48	end	48	end
49		49
50	def is_deploy_key	50	def is_deploy_key
51	- !!project_id	51	+ project.present?
52	end	52	end
53		53
54	# projects that has this key	54	# projects that has this key
@@ -0,0 +1,8 @@		@@ -0,0 +1,8 @@
	1	+class ProjectActivityCacheObserver < BaseObserver
	2	+ observe :event
	3	+
	4	+ def after_create(event)
	5	+ event.project.update_attribute(:last_activity_at, event.created_at) if event.project
	6	+ end
	7	+end
	8	+
	@@ -24,6 +24,7 @@ module Gitlab		@@ -24,6 +24,7 @@ module Gitlab
24		24
25	# Activate observers that should always be running.	25	# Activate observers that should always be running.
26	config.active_record.observers = :activity_observer,	26	config.active_record.observers = :activity_observer,
		27	+ :project_activity_cache_observer,
27	:issue_observer,	28	:issue_observer,
28	:key_observer,	29	:key_observer,
29	:merge_request_observer,	30	:merge_request_observer,
@@ -0,0 +1,15 @@		@@ -0,0 +1,15 @@
	1	+class AddLastActivityColumnIntoProject < ActiveRecord::Migration
	2	+ def up
	3	+ add_column :projects, :last_activity_at, :datetime
	4	+ add_index :projects, :last_activity_at
	5	+
	6	+ Project.find_each do \|project\|
	7	+ project.update_attribute(:last_activity_at, project.last_activity_date)
	8	+ end
	9	+ end
	10	+
	11	+ def down
	12	+ remove_index :projects, :last_activity_at
	13	+ remove_column :projects, :last_activity_at
	14	+ end
	15	+end
	@@ -12,7 +12,6 @@		@@ -12,7 +12,6 @@
12	# It's strongly recommended to check this file into your version control system.	12	# It's strongly recommended to check this file into your version control system.
13		13
14	ActiveRecord::Schema.define(:version => 20130404164628) do	14	ActiveRecord::Schema.define(:version => 20130404164628) do
15	-
16	create_table "events", :force => true do \|t\|	15	create_table "events", :force => true do \|t\|
17	t.string "target_type"	16	t.string "target_type"
18	t.integer "target_id"	17	t.integer "target_id"