From 99ea156e1c68cabe2507687dfb6c1ddf55563a93 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jonne=20Ha=C3=9F?= <me@jhass.eu>
Date: Fri, 27 Feb 2015 16:30:28 +0100
Subject: [PATCH] Refactor User.total_users into a scope

---
 app/models/user.rb                           |  9 +++-----
 app/presenters/statistics_presenter.rb       |  5 +---
 spec/models/user_spec.rb                     | 24 +++++++++++---------
 spec/presenters/statistics_presenter_spec.rb |  2 +-
 4 files changed, 18 insertions(+), 22 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 74c8a6ffa..91bf3214d 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -15,6 +15,7 @@ class User < ActiveRecord::Base
   scope :daily_actives, ->(time = Time.now) { logged_in_since(time - 1.day) }
   scope :yearly_actives, ->(time = Time.now) { logged_in_since(time - 1.year) }
   scope :halfyear_actives, ->(time = Time.now) { logged_in_since(time - 6.month) }
+  scope :active, -> { joins(:person).where(people: {closed_account: false}).where.not(username: nil) }
 
   devise :token_authenticatable, :database_authenticatable, :registerable,
          :recoverable, :rememberable, :trackable, :validatable,
@@ -517,7 +518,7 @@ class User < ActiveRecord::Base
       self.save
     end
   end
-  
+
   def after_database_authentication
     # remove any possible remove_after timestamp flag set by maintenance.remove_old_users
     unless self.remove_after.nil?
@@ -526,12 +527,8 @@ class User < ActiveRecord::Base
     end
   end
 
-
-  def self.total_users
-    User.joins(:person).where(:people => {:closed_account => false}).where.not(:username => nil)
-  end
-
   private
+
   def clearable_fields
     self.attributes.keys - ["id", "username", "encrypted_password",
                             "created_at", "updated_at", "locked_at",
diff --git a/app/presenters/statistics_presenter.rb b/app/presenters/statistics_presenter.rb
index d07913e3d..51d2b367c 100644
--- a/app/presenters/statistics_presenter.rb
+++ b/app/presenters/statistics_presenter.rb
@@ -48,10 +48,7 @@ class StatisticsPresenter
   end
 
   def total_users
-    @total_users ||= User.joins(:person)
-                         .where(people: {closed_account: false})
-                         .where.not(username: nil)
-                         .count
+    @total_users ||= User.active.count
   end
 
   def monthly_users
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 80b024cc1..d766874be 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1064,43 +1064,45 @@ describe User, :type => :model do
       @user.sign_up
     end
   end
-  
+
   describe "maintenance" do
     before do
       @user = bob
       AppConfig.settings.maintenance.remove_old_users.enable = true
     end
-    
+
     it "#flags user for removal" do
       remove_at = Time.now+5.days
       @user.flag_for_removal(remove_at)
       expect(@user.remove_after).to eq(remove_at)
     end
   end
-  
+
   describe "#auth database auth maintenance" do
     before do
       @user = bob
       @user.remove_after = Time.now
       @user.save
     end
-    
+
     it "remove_after is cleared" do
       @user.after_database_authentication
       expect(@user.remove_after).to eq(nil)
     end
+  end
 
-  describe "total_users" do
+  describe "active" do
     before do
-      @user1 = FactoryGirl.build(:user, :username => nil)
-      @user1.save(:validate => false)
-      @user2 = FactoryGirl.create(:user)
-      @user2.person.closed_account = true
-      @user2.save
+      invited_user = FactoryGirl.build(:user, username: nil)
+      invited_user.save(validate: false)
+
+      closed_account = FactoryGirl.create(:user)
+      closed_account.person.closed_account = true
+      closed_account.save
     end
 
     it "returns total_users excluding closed accounts & users without usernames" do
-      expect(User.total_users.count).to eq 5     #5 users from fixtures
+      expect(User.active.count).to eq 6     # 6 users from fixtures
     end
   end
 end
diff --git a/spec/presenters/statistics_presenter_spec.rb b/spec/presenters/statistics_presenter_spec.rb
index d586fdff1..40661f123 100644
--- a/spec/presenters/statistics_presenter_spec.rb
+++ b/spec/presenters/statistics_presenter_spec.rb
@@ -52,7 +52,7 @@ describe StatisticsPresenter do
           "network" => "Diaspora",
           "version" => AppConfig.version_string,
           "registrations_open" => AppConfig.settings.enable_registrations?,
-          "total_users" => User.total_users.count,
+          "total_users" => User.active.count,
           "active_users_halfyear" => User.halfyear_actives.count,
           "active_users_monthly" => User.monthly_actives.count,
           "local_posts" => @presenter.local_posts,