Explicitly define ordering in models using default_scope

22 files changed, 67 insertions, 16 deletions

diff --git a/app/controllers/admin/dashboard_controller.rb b/app/controllers/admin/dashboard_controller.rb
index be19139c9b1..c491e5c7550 100644
--- a/app/controllers/admin/dashboard_controller.rb
+++ b/app/controllers/admin/dashboard_controller.rb
@@ -1,7 +1,7 @@
 class Admin::DashboardController < Admin::ApplicationController
   def index
-    @projects = Project.order("created_at DESC").limit(10)
-    @users = User.order("created_at DESC").limit(10)
-    @groups = Group.order("created_at DESC").limit(10)
+    @projects = Project.limit(10)
+    @users = User.limit(10)
+    @groups = Group.limit(10)
   end
 end
diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb
index 8c7d90a5d9f..ae610d4871c 100644
--- a/app/controllers/admin/groups_controller.rb
+++ b/app/controllers/admin/groups_controller.rb
@@ -2,7 +2,7 @@ class Admin::GroupsController < Admin::ApplicationController
   before_filter :group, only: [:edit, :show, :update, :destroy, :project_update, :project_teams_update]
 
   def index
-    @groups = Group.order('name ASC')
+    @groups = Group.order_name
     @groups = @groups.search(params[:name]) if params[:name].present?
     @groups = @groups.page(params[:page]).per(20)
   end
diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index aea8545d38e..932bfc777e6 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -5,13 +5,13 @@ class Admin::UsersController < Admin::ApplicationController
     @users = User.filter(params[:filter])
     @users = @users.search(params[:name]) if params[:name].present?
     @users = @users.sort(@sort = params[:sort])
-    @users = @users.alphabetically.page(params[:page])
+    @users = @users.order_name.page(params[:page])
   end
 
   def show
     @personal_projects = user.personal_projects
     @joined_projects = user.projects.joined(@user)
-    @keys = user.keys.order('id DESC')
+    @keys = user.keys
   end
 
   def new
diff --git a/app/controllers/profiles/keys_controller.rb b/app/controllers/profiles/keys_controller.rb
index 88414b13564..4e2bd0a9b4b 100644
--- a/app/controllers/profiles/keys_controller.rb
+++ b/app/controllers/profiles/keys_controller.rb
@@ -3,7 +3,7 @@ class Profiles::KeysController < ApplicationController
   skip_before_filter :authenticate_user!, only: [:get_keys]
 
   def index
-    @keys = current_user.keys.order('id DESC')
+    @keys = current_user.keys
   end
 
   def show
diff --git a/app/models/broadcast_message.rb b/app/models/broadcast_message.rb
index 4d0c04bcc3d..05f5e979695 100644
--- a/app/models/broadcast_message.rb
+++ b/app/models/broadcast_message.rb
@@ -14,6 +14,8 @@
 #
 
 class BroadcastMessage < ActiveRecord::Base
+  include Sortable
+
   validates :message, presence: true
   validates :starts_at, presence: true
   validates :ends_at, presence: true
diff --git a/app/models/concerns/internal_id.rb b/app/models/concerns/internal_id.rb
index 821ed54fb98..e86357e3dee 100644
--- a/app/models/concerns/internal_id.rb
+++ b/app/models/concerns/internal_id.rb
@@ -1,5 +1,6 @@
 module InternalId
   extend ActiveSupport::Concern
+  include Sortable
 
   included do
     validate :set_iid, on: :create
diff --git a/app/models/concerns/sortable.rb b/app/models/concerns/sortable.rb
new file mode 100644
index 00000000000..49001cabc7c
--- /dev/null
+++ b/app/models/concerns/sortable.rb
@@ -0,0 +1,32 @@
+# == Sortable concern
+#
+# Set default scope for ordering objects
+#
+module Sortable
+  extend ActiveSupport::Concern
+
+  included do
+    # By default all models should be ordered
+    # by created_at field starting from newest
+    default_scope { order(created_at: :desc, id: :desc) }
+    scope :order_name, -> { reorder(name: :asc) }
+    scope :order_recent, -> { reorder(created_at: :desc, id: :desc) }
+    scope :order_oldest, -> { reorder(created_at: :asc, id: :asc) }
+    scope :order_recent_updated, -> { reorder(updated_at: :desc, id: :desc) }
+    scope :order_oldest_updated, -> { reorder(updated_at: :asc, id: :asc) }
+  end
+
+  module ClassMethods
+    def sort(method)
+      case method.to_s
+      when 'name' then order_name_asc
+      when 'recent' then order_recent
+      when 'oldest' then order_oldest
+      when 'recent_updated' then order_recent_updated
+      when 'oldest_updated' then order_oldest_updated
+      else
+        self
+      end
+    end
+  end
+end
diff --git a/app/models/email.rb b/app/models/email.rb
index 57f476bd519..556b0e9586e 100644
--- a/app/models/email.rb
+++ b/app/models/email.rb
@@ -10,6 +10,8 @@
 #
 
 class Email < ActiveRecord::Base
+  include Sortable
+
   belongs_to :user
 
   validates :user_id, presence: true
diff --git a/app/models/event.rb b/app/models/event.rb
index 2a6c690ab91..9a42d380f87 100644
--- a/app/models/event.rb
+++ b/app/models/event.rb
@@ -15,6 +15,7 @@
 #
 
 class Event < ActiveRecord::Base
+  include Sortable
   default_scope { where.not(author_id: nil) }
 
   CREATED   = 1
diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb
index c8fa9c50918..defef7216f2 100644
--- a/app/models/hooks/web_hook.rb
+++ b/app/models/hooks/web_hook.rb
@@ -16,6 +16,7 @@
 #
 
 class WebHook < ActiveRecord::Base
+  include Sortable
   include HTTParty
 
   default_value_for :push_events, true
diff --git a/app/models/identity.rb b/app/models/identity.rb
index 80e0e3a8a23..b2c3792d1ce 100644
--- a/app/models/identity.rb
+++ b/app/models/identity.rb
@@ -9,6 +9,7 @@
 #
 
 class Identity < ActiveRecord::Base
+  include Sortable
   belongs_to :user
 
   validates :extern_uid, allow_blank: true, uniqueness: { scope: :provider }
diff --git a/app/models/key.rb b/app/models/key.rb
index d2d1af68822..e2e59296eed 100644
--- a/app/models/key.rb
+++ b/app/models/key.rb
@@ -15,6 +15,7 @@
 require 'digest/md5'
 
 class Key < ActiveRecord::Base
+  include Sortable
   include Gitlab::Popen
 
   belongs_to :user
diff --git a/app/models/label.rb b/app/models/label.rb
index 2b2b02e0645..c8f6a7cd48c 100644
--- a/app/models/label.rb
+++ b/app/models/label.rb
@@ -11,6 +11,8 @@
 #
 
 class Label < ActiveRecord::Base
+  include Sortable
+
   DEFAULT_COLOR = '#428BCA'
 
   belongs_to :project
diff --git a/app/models/member.rb b/app/models/member.rb
index 671ef466baa..fe3d2f40e87 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -14,6 +14,7 @@
 #
 
 class Member < ActiveRecord::Base
+  include Sortable
   include Notifiable
   include Gitlab::Access
 
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index a71122d5e07..acac1ca4cf7 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -14,6 +14,8 @@
 require Rails.root.join("app/models/commit")
 
 class MergeRequestDiff < ActiveRecord::Base
+  include Sortable
+
   # Prevent store of diff
   # if commits amount more then 200
   COMMITS_SAFE_SIZE = 200
diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index e7fd3024750..ba0b2b71cf9 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -14,6 +14,7 @@
 #
 
 class Namespace < ActiveRecord::Base
+  include Sortable
   include Gitlab::ShellAdapter
 
   has_many :projects, dependent: :destroy
diff --git a/app/models/note.rb b/app/models/note.rb
index 0b988cc3e0f..a3f2980cebd 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -23,6 +23,7 @@ require 'file_size_validator'
 class Note < ActiveRecord::Base
   include Mentionable
 
+  default_scope { order(created_at: :asc, id: :asc) }
   default_value_for :system, false
 
   attr_mentionable :note
diff --git a/app/models/project.rb b/app/models/project.rb
index 390e1457ca1..246479624ee 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -33,6 +33,7 @@ require 'carrierwave/orm/activerecord'
 require 'file_size_validator'
 
 class Project < ActiveRecord::Base
+  include Sortable
   include Gitlab::ShellAdapter
   include Gitlab::VisibilityLevel
   include Gitlab::ConfigHelper
@@ -53,7 +54,7 @@ class Project < ActiveRecord::Base
   attr_accessor :new_default_branch
 
   # Relations
-  belongs_to :creator,      foreign_key: 'creator_id', class_name: 'User'
+  belongs_to :creator, foreign_key: 'creator_id', class_name: 'User'
   belongs_to :group, -> { where(type: Group) }, foreign_key: 'namespace_id'
   belongs_to :namespace
 
@@ -86,7 +87,7 @@ class Project < ActiveRecord::Base
   has_many :merge_requests,     dependent: :destroy, foreign_key: 'target_project_id'
   # Merge requests from source project should be kept when source project was removed
   has_many :fork_merge_requests, foreign_key: 'source_project_id', class_name: MergeRequest
-  has_many :issues, -> { order 'issues.state DESC, issues.created_at DESC' }, dependent: :destroy
+  has_many :issues,             dependent: :destroy
   has_many :labels,             dependent: :destroy
   has_many :services,           dependent: :destroy
   has_many :events,             dependent: :destroy
@@ -139,14 +140,16 @@ class Project < ActiveRecord::Base
   mount_uploader :avatar, AttachmentUploader
 
   # Scopes
+  scope :sorted_by_activity, -> { reorder('projects.last_activity_at DESC') }
+  scope :sorted_by_stars, -> { reorder('projects.star_count DESC') }
+  scope :sorted_by_names, -> { joins(:namespace).reorder('namespaces.name ASC, projects.name ASC') }
+
   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 :in_group_namespace, -> { joins(:group) }
-  scope :sorted_by_activity, -> { reorder('projects.last_activity_at DESC') }
-  scope :sorted_by_stars, -> { reorder('projects.star_count DESC') }
   scope :personal, ->(user) { where(namespace_id: user.namespace_id) }
   scope :joined, ->(user) { where('namespace_id != ?', user.namespace_id) }
   scope :public_only, -> { where(visibility_level: Project::PUBLIC) }
diff --git a/app/models/service.rb b/app/models/service.rb
index 15948e63e41..caabe8e971d 100644
--- a/app/models/service.rb
+++ b/app/models/service.rb
@@ -15,6 +15,7 @@
 # To add new service you should build a class inherited from Service
 # and implement a set of methods
 class Service < ActiveRecord::Base
+  include Sortable
   serialize :properties, JSON
 
   default_value_for :active, false
diff --git a/app/models/snippet.rb b/app/models/snippet.rb
index a3222d29892..82c1ab94446 100644
--- a/app/models/snippet.rb
+++ b/app/models/snippet.rb
@@ -16,6 +16,7 @@
 #
 
 class Snippet < ActiveRecord::Base
+  include Sortable
   include Linguist::BlobHelper
   include Gitlab::VisibilityLevel
 
diff --git a/app/models/user.rb b/app/models/user.rb
index 552a37c9533..41c52440320 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -49,6 +49,7 @@ require 'carrierwave/orm/activerecord'
 require 'file_size_validator'
 
 class User < ActiveRecord::Base
+  include Sortable
   include Gitlab::ConfigHelper
   include TokenAuthenticatable
   extend Gitlab::ConfigHelper
@@ -176,7 +177,6 @@ class User < ActiveRecord::Base
   scope :admins, -> { where(admin:  true) }
   scope :blocked, -> { with_state(:blocked) }
   scope :active, -> { with_state(:active) }
-  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) ) : all }
@@ -290,7 +290,7 @@ class User < ActiveRecord::Base
   def authorized_groups
     @authorized_groups ||= begin
                              group_ids = (groups.pluck(:id) + authorized_projects.pluck(:namespace_id))
-                             Group.where(id: group_ids).order('namespaces.name ASC')
+                             Group.where(id: group_ids)
                            end
   end
 
@@ -301,7 +301,7 @@ class User < ActiveRecord::Base
                                project_ids = personal_projects.pluck(:id)
                                project_ids.push(*groups_projects.pluck(:id))
                                project_ids.push(*projects.pluck(:id).uniq)
-                               Project.where(id: project_ids).joins(:namespace).order('namespaces.name ASC')
+                               Project.where(id: project_ids)
                              end
   end
 
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index d2828b24c36..e2c2cd4c3da 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -39,7 +39,6 @@ module API
         issues = current_user.issues
         issues = filter_issues_state(issues, params[:state]) unless params[:state].nil?
         issues = filter_issues_labels(issues, params[:labels]) unless params[:labels].nil?
-        issues = issues.order('issues.id DESC')
 
         present paginate(issues), with: Entities::Issue
       end
@@ -70,7 +69,6 @@ module API
         unless params[:milestone].nil?
           issues = filter_issues_milestone(issues, params[:milestone])
         end
-        issues = issues.order('issues.id DESC')
 
         present paginate(issues), with: Entities::Issue
       end