Merge pull request #3071 from fantasticfears/visits-again

FIX: use utc time when generate reports; set boundary

app/models/post_action.rb

@@ -39,6 +39,13 @@ class PostAction < ActiveRecord::Base
     nil
   end
 
+  def self.flag_count_by_date(start_date, end_date)
+    where('created_at >= ? and created_at <= ?', start_date, end_date)
+      .where(post_action_type_id: PostActionType.flag_types.values)
+      .group('date(created_at)').order('date(created_at)')
+      .count
+  end
+
   def self.update_flagged_posts_count
     posts_flagged_count = PostAction.active
                                     .flags
@@ -92,7 +99,7 @@ class PostAction < ActiveRecord::Base
 
   def self.count_per_day_for_type(post_action_type, since_days_ago=30)
     unscoped.where(post_action_type_id: post_action_type)
-            .where('created_at > ?', since_days_ago.days.ago)
+            .where('created_at >= ?', since_days_ago.days.ago)
             .group('date(created_at)')
             .order('date(created_at)')
             .count

app/models/report.rb

@@ -13,8 +13,8 @@ class Report
     @data = nil
     @total = nil
     @prev30Days = nil
-    @start_date ||= 1.month.ago
-    @end_date ||= Time.now
+    @start_date ||= 1.month.ago.utc.beginning_of_day
+    @end_date ||= Time.now.utc.end_of_day
   end
 
   def as_json(options = nil)
@@ -46,7 +46,7 @@ class Report
   end
 
   def self.report_visits(report)
-    basic_report_about report, UserVisit, :by_day, report.start_date, report.end_date.change(hour: 24)
+    basic_report_about report, UserVisit, :by_day, report.start_date, report.end_date
   end
 
   def self.report_signups(report)
@@ -55,14 +55,12 @@ class Report
 
   def self.report_topics(report)
     basic_report_about report, Topic, :listable_count_per_day, report.start_date, report.end_date
-    report.total = Topic.listable_topics.count
-    report.prev30Days = Topic.listable_topics.where('created_at > ? and created_at < ?', report.start_date - 30.days, report.start_date).count
+    add_counts report, Topic.listable_topics, 'topics.created_at'
   end
 
   def self.report_posts(report)
     basic_report_about report, Post, :public_posts_count_per_day, report.start_date, report.end_date
-    report.total = Post.public_posts.count
-    report.prev30Days = Post.public_posts.where('posts.created_at > ? and posts.created_at < ?', report.start_date - 30.days, report.start_date).count
+    add_counts report, Post.public_posts, 'posts.created_at'
   end
 
   def self.report_emails(report)
@@ -71,7 +69,7 @@ class Report
 
   def self.report_about(report, subject_class, report_method = :count_per_day)
     basic_report_about report, subject_class, report_method, report.start_date, report.end_date
-    add_counts(report, subject_class)
+    add_counts report, subject_class
   end
 
   def self.basic_report_about(report, subject_class, report_method, *args)
@@ -81,9 +79,9 @@ class Report
     end
   end
 
-  def self.add_counts(report, subject_class)
+  def self.add_counts(report, subject_class, query_column = 'created_at')
     report.total      = subject_class.count
-    report.prev30Days = subject_class.where('created_at > ? and created_at < ?', report.start_date - 30.days, report.start_date).count
+    report.prev30Days = subject_class.where("#{query_column} >= ? and #{query_column} < ?", report.start_date - 30.days, report.start_date).count
   end
 
   def self.report_users_by_trust_level(report)
@@ -95,26 +93,14 @@ class Report
 
   def self.report_starred(report)
     basic_report_about report, Topic, :starred_counts_per_day, default_days
-    query = TopicUser.where(starred: true)
-    report.total = query.count
-    report.prev30Days = query.where('starred_at > ? and starred_at < ?', report.start_date - 30.days, report.start_date).count
+    add_counts report, TopicUser.where(starred: true), 'topic_users.starred_at'
   end
 
   # Post action counts:
 
   def self.report_flags(report)
-    report.data = []
-    (0..30).to_a.reverse.each do |i|
-      count = PostAction.where('date(created_at) = ?', i.days.ago.utc.to_date)
-        .where(post_action_type_id: PostActionType.flag_types.values)
-        .count
-      if count > 0
-        report.data << {x: i.days.ago.utc.to_date.to_s, y: count}
-      end
-    end
-    flagsQuery = PostAction.where(post_action_type_id: PostActionType.flag_types.values)
-    report.total = flagsQuery.count
-    report.prev30Days = flagsQuery.where('created_at > ? and created_at < ?', report.start_date - 30.days, report.start_date).count
+    basic_report_about report, PostAction, :flag_count_by_date, report.start_date, report.end_date
+    add_counts report, PostAction.where(post_action_type_id: PostActionType.flag_types.values), 'post_actions.created_at'
   end
 
   def self.report_likes(report)
@@ -130,17 +116,14 @@ class Report
     PostAction.count_per_day_for_type(post_action_type).each do |date, count|
       report.data << { x: date, y: count }
     end
-    query = PostAction.unscoped.where(post_action_type_id: post_action_type)
-    report.total = query.count
-    report.prev30Days = query.where('created_at > ? and created_at < ?', report.start_date - 30.days, report.start_date).count
+    add_counts report, PostAction.unscoped.where(post_action_type_id: post_action_type), 'post_actions.created_at'
   end
 
   # Private messages counts:
 
   def self.private_messages_report(report, topic_subtype)
     basic_report_about report, Post, :private_messages_count_per_day, default_days, topic_subtype
-    report.total = Post.private_posts.with_topic_subtype(topic_subtype).count
-    report.prev30Days = Post.private_posts.with_topic_subtype(topic_subtype).where('posts.created_at > ? and posts.created_at < ?', report.start_date - 30.days, report.start_date).count
+    add_counts report, Post.private_posts.with_topic_subtype(topic_subtype), 'posts.created_at'
   end
 
   def self.report_user_to_user_private_messages(report)

app/models/topic.rb

@@ -352,7 +352,7 @@ class Topic < ActiveRecord::Base
   end
 
   def self.listable_count_per_day(start_date, end_date)
-    listable_topics.where('created_at >= ? and created_at < ?', start_date, end_date).group('date(created_at)').order('date(created_at)').count
+    listable_topics.where('created_at >= ? and created_at <= ?', start_date, end_date).group('date(created_at)').order('date(created_at)').count
   end
 
   def private_message?

app/models/user.rb

@@ -531,7 +531,7 @@ class User < ActiveRecord::Base
   end
 
   def self.count_by_signup_date(start_date, end_date)
-    where('created_at >= ? and created_at < ?', start_date, end_date).group('date(created_at)').order('date(created_at)').count
+    where('created_at >= ? and created_at <= ?', start_date, end_date).group('date(created_at)').order('date(created_at)').count
   end
 
 

app/models/user_visit.rb

@@ -2,7 +2,7 @@ class UserVisit < ActiveRecord::Base
 
   # A count of visits in the last month by day
   def self.by_day(start_date, end_date)
-    where("visited_at >= ? and visited_at < ?", start_date, end_date).group(:visited_at).order(:visited_at).count
+    where('visited_at >= ? and visited_at <= ?', start_date.to_date, end_date.to_date).group(:visited_at).order(:visited_at).count
   end
 
   def self.ensure_consistency!

spec/models/report_spec.rb

@@ -14,7 +14,7 @@ describe Report do
     context "with visits" do
       let(:user) { Fabricate(:user) }
 
-      before do
+      before(:each) do
         user.user_visits.create(visited_at: 1.hour.ago)
         user.user_visits.create(visited_at: 1.day.ago)
         user.user_visits.create(visited_at: 2.days.ago)
@@ -24,7 +24,7 @@ describe Report do
         report.data.should be_present
       end
 
-      it "return today's visit" do
+      it "returns today's visit" do
         report.data.select { |v| v[:x].today? }.should be_present
       end
     end
@@ -43,7 +43,8 @@ describe Report do
       end
 
       context "with #{pluralized}" do
-        before do
+        before(:each) do
+          Timecop.freeze
           fabricator = case arg
           when :signup
             :user
@@ -52,18 +53,42 @@ describe Report do
           else
             arg
           end
-          Fabricate(fabricator, created_at: 25.hours.ago)
+          Fabricate(fabricator)
           Fabricate(fabricator, created_at: 1.hours.ago)
           Fabricate(fabricator, created_at: 1.hours.ago)
+          Fabricate(fabricator, created_at: 1.day.ago)
+          Fabricate(fabricator, created_at: 2.days.ago)
+          Fabricate(fabricator, created_at: 30.days.ago)
+          Fabricate(fabricator, created_at: 35.days.ago)
         end
+        after(:each) { Timecop.return }
 
-        it 'returns correct data' do
-          report.data[0][:y].should == 1
-          report.data[1][:y].should == 2
+        context 'returns a report with data'
+          it 'with 30 days data' do
+            report.data.count.should == 4
+          end
+
+          it 'has correct data sorted as asc' do
+            report.data[0][:y].should == 1 # 30.days.ago
+            report.data[1][:y].should == 1 # 2.days.ago
+            report.data[2][:y].should == 1 # 1.day.ago
+            report.data[3][:y].should == 3 # today
+          end
+
+          it "returns today's data" do
+            report.data.select { |v| v[:x].today? }.should be_present
+          end
+
+          it 'returns total data' do
+            report.total.should == 7
+          end
+
+          it "returns previous 30 day's data" do
+            report.prev30Days.should == 1
+          end
         end
       end
     end
-  end
 
   describe 'private messages' do
     let(:report) { Report.find('user_to_user_private_messages') }
@@ -149,5 +174,4 @@ describe Report do
       end
     end
   end
-
 end

spec/models/topic_spec.rb

@@ -1232,6 +1232,26 @@ describe Topic do
     end
   end
 
+  describe '#listable_count_per_day' do
+    before(:each) do
+      Timecop.freeze
+      Fabricate(:topic)
+      Fabricate(:topic, created_at: 1.day.ago)
+      Fabricate(:topic, created_at: 1.day.ago)
+      Fabricate(:topic, created_at: 2.days.ago)
+      Fabricate(:topic, created_at: 4.days.ago)
+    end
+    after(:each) do
+      Timecop.return
+    end
+    let(:listable_topics_count_per_day) { {1.day.ago.to_date => 2, 2.days.ago.to_date => 1, Time.now.utc.to_date => 1 } }
+
+    it 'collect closed interval listable topics count' do
+      Topic.listable_count_per_day(2.days.ago, Time.now).should include(listable_topics_count_per_day)
+      Topic.listable_count_per_day(2.days.ago, Time.now).should_not include({4.days.ago.to_date => 1})
+    end
+  end
+
   describe '#secure_category?' do
     let(:category){ Category.new }
 

spec/models/user_spec.rb

@@ -6,6 +6,25 @@ describe User do
   it { should validate_presence_of :username }
   it { should validate_presence_of :email }
 
+  describe '#count_by_signup_date' do
+    before(:each) do
+      User.destroy_all
+      Timecop.freeze
+      Fabricate(:user)
+      Fabricate(:user, created_at: 1.day.ago)
+      Fabricate(:user, created_at: 1.day.ago)
+      Fabricate(:user, created_at: 2.days.ago)
+      Fabricate(:user, created_at: 4.days.ago)
+    end
+    after(:each) { Timecop.return }
+    let(:signups_by_day) { {1.day.ago.to_date => 2, 2.days.ago.to_date => 1, Time.now.utc.to_date => 1} }
+
+    it 'collect closed interval signups' do
+      User.count_by_signup_date(2.days.ago, Time.now).should include(signups_by_day)
+      User.count_by_signup_date(2.days.ago, Time.now).should_not include({4.days.ago.to_date => 1})
+    end
+  end
+
   context '.enqueue_welcome_message' do
     let(:user) { Fabricate(:user) }
 

spec/models/user_visit_spec.rb

@@ -1,21 +1,41 @@
 require 'spec_helper'
 
 describe UserVisit do
+  let(:user) { Fabricate(:user) }
+  let(:other_user) { Fabricate(:user) }
+
   it 'can ensure consistency' do
-    u = Fabricate(:user)
-    u.update_visit_record!(2.weeks.ago.to_date)
-    u.last_seen_at = 2.weeks.ago
-    u.save
-    u.update_visit_record!(1.day.ago.to_date)
+    user.update_visit_record!(2.weeks.ago.to_date)
+    user.last_seen_at = 2.weeks.ago
+    user.save
+    user.update_visit_record!(1.day.ago.to_date)
 
-    u.reload
-    u.user_stat.days_visited.should == 2
+    user.reload
+    user.user_stat.days_visited.should == 2
 
-    u.user_stat.days_visited = 1
-    u.save
+    user.user_stat.days_visited = 1
+    user.save
     UserVisit.ensure_consistency!
 
-    u.reload
-    u.user_stat.days_visited.should == 2
+    user.reload
+    user.user_stat.days_visited.should == 2
+  end
+
+  describe '#by_day' do
+    before(:each) do
+      Timecop.freeze
+      user.user_visits.create(visited_at: Time.now)
+      user.user_visits.create(visited_at: 1.day.ago)
+      other_user.user_visits.create(visited_at: 1.day.ago)
+      user.user_visits.create(visited_at: 2.days.ago)
+      user.user_visits.create(visited_at: 4.days.ago)
+    end
+    after(:each) { Timecop.return }
+    let(:visits_by_day) { {1.day.ago.to_date => 2, 2.days.ago.to_date => 1, Time.now.to_date => 1 } }
+
+    it 'collect closed interval visits' do
+      UserVisit.by_day(2.days.ago, Time.now).should include(visits_by_day)
+      UserVisit.by_day(2.days.ago, Time.now).should_not include({4.days.ago.to_date => 1})
+    end
   end
 end