fix session overlapped duration with other users

change calculation of MusicSessionUserHistory.duration_minutes to
exactly get the number of minutes overlapped with other user sessions.

previously it returned the entire music session time if a other user
joined in and left without staying compleyely within the session.

this commit also fixs an edge case of the query in
MusicSessionUserHistory.overlapping_connections

ruby/Gemfile

@@ -51,7 +51,8 @@ gem "activerecord-import", "~> 0.4.1"
 gem "auto_strip_attributes"
 gem "json", "1.8.6"
 gem 'uuidtools', '2.1.2'
-gem 'bcrypt-ruby', '3.0.1'
+gem 'bcrypt', '3.1.15' 
+gem 'bcrypt-ruby' #, '3.0.1'
 gem 'ruby-protocol-buffers', '1.2.2'
 gem 'eventmachine', '1.0.4'
 gem 'amqp', '1.0.2'

ruby/Gemfile.lock

@@ -76,8 +76,9 @@ GEM
     aws-sdk-v1 (1.67.0)
       json (~> 1.4)
       nokogiri (~> 1)
-    bcrypt (3.1.16)
-    bcrypt-ruby (3.0.1)
+    bcrypt (3.1.15)
+    bcrypt-ruby (3.1.5)
+      bcrypt (>= 3.1.3)
     builder (3.2.4)
     carrierwave (0.9.0)
       activemodel (>= 3.2.0)
@@ -493,7 +494,8 @@ DEPENDENCIES
   amqp (= 1.0.2)
   auto_strip_attributes
   aws-sdk (~> 1)
-  bcrypt-ruby (= 3.0.1)
+  bcrypt (= 3.1.15)
+  bcrypt-ruby
   builder
   carrierwave (= 0.9.0)
   database_cleaner (= 1.4.1)
@@ -555,7 +557,7 @@ DEPENDENCIES
   zip-codes
 
 RUBY VERSION
-   ruby 2.3.1p112
+   ruby 2.4.1p111
 
 BUNDLED WITH
-   1.17.2
+   1.17.3

ruby/lib/jam_ruby/models/music_session_user_history.rb

@@ -55,9 +55,28 @@ module JamRuby
       self.user ? self.user.email : '<user deleted>'
     end
 
+    # def duration_minutes
+    #   end_time = self.session_removed_at || Time.now
+    #   (end_time - self.created_at) / 60.0
+    # end
+
+    #take duration that this user session was with other
+    #
     def duration_minutes
-      end_time = self.session_removed_at || Time.now
-      (end_time - self.created_at) / 60.0
+        query = <<-SQL
+          select sum(upper(diff) - lower(diff)) as duration
+          FROM (
+           SELECT tsrange(created_at, session_removed_at, '[]') * tsrange('#{self.created_at.strftime '%Y-%m-%d %H:%M:%S'}', '#{self.session_removed_at.strftime '%Y-%m-%d %H:%M:%S'}', '[]') AS diff
+           FROM music_sessions_user_history 
+           WHERE music_session_id = '#{self.music_session_id}' 
+           AND id <> '#{self.id}'
+
+          ) AS derivedTable
+SQL
+        result = ActiveRecord::Base.connection.exec_query(query)
+        duration = result.cast_values[0]
+        return 0 unless duration
+        (duration.split(':').map(&:to_f).inject(0) { |a, b| a * 60 + b } / 60.0).round
     end
 
     def self.join_music_session(user_id, session_id, client_id)
@@ -89,10 +108,10 @@ module JamRuby
     def end_history
       if self.session_removed_at.nil?
         self.session_removed_at = Time.now
-        self.max_concurrent_connections = determine_max_concurrent
-        self.update_attributes(:session_removed_at => self.session_removed_at, :max_concurrent_connections => self.max_concurrent_connections)
-        self.update_remaining_play_time
       end
+      self.max_concurrent_connections = determine_max_concurrent
+      self.update_attributes(:session_removed_at => self.session_removed_at, :max_concurrent_connections => self.max_concurrent_connections)
+      self.update_remaining_play_time
     end
 
     # update the users monthly play time
@@ -116,20 +135,33 @@ module JamRuby
     # figures out what the peak amount of other clients the user saw while playing at any given time.
     # we use the database to get all other connections that occurred while their this user was connected,
     # and then sort through them using custom logic to increase/decrease the count as people come and go
+    def overlapping_connections
+      MusicSessionUserHistory.where("music_session_id = ? AND
+        ((created_at >= ? AND (session_removed_at is NULL OR session_removed_at <= ?)) OR
+        (created_at > ? AND created_at < ? AND (session_removed_at is NULL OR session_removed_at > ?)) OR
+        (created_at <= ? AND (session_removed_at is NULL OR session_removed_at >= ?)))",
+                                  self.music_session_id, 
+                                  self.created_at, self.session_removed_at,
+                                  self.created_at, self.session_removed_at, self.session_removed_at,
+                                  self.created_at, self.created_at).select('id, created_at, session_removed_at').order(:created_at)
+    end
+    
     def determine_max_concurrent
-        overlapping_connections = MusicSessionUserHistory.where("music_session_id = ? AND
-                                    ((created_at >= ? AND (session_removed_at is NULL OR session_removed_at <= ?)) OR
-                                    (created_at <= ? AND (session_removed_at is NULL OR session_removed_at >= ?)))",
-                                                              self.music_session_id, self.created_at, self.session_removed_at, self.created_at, self.created_at).select('id, created_at, session_removed_at').order(:created_at)
+        # overlapping_connections = MusicSessionUserHistory.where("music_session_id = ? AND
+        #                             ((created_at >= ? AND (session_removed_at is NULL OR session_removed_at <= ?)) OR
+        #                             (created_at <= ? AND (session_removed_at is NULL OR session_removed_at >= ?)))",
+        #                                                       self.music_session_id, self.created_at, self.session_removed_at, self.created_at, self.created_at).select('id, created_at, session_removed_at').order(:created_at)
       in_out_times = []
       overlapping_connections.each do |connection|
         in_out_times.push([connection.created_at.nil? ? Time.at(0) : connection.created_at, true])
         in_out_times.push([connection.session_removed_at.nil? ? Time.new(3000) : connection.session_removed_at, false]) #helpful to get rid of nulls for sorting
       end
-
+      #raise ">>>>>> #{in_out_times.inspect}"
       count_concurrent(in_out_times)
     end
 
+
+
     def count_concurrent(in_out_times)
       in_out_times.sort! { |a,b| a[0] <=> b[0] }
 

ruby/spec/jam_ruby/models/music_sessions_user_history_spec.rb

@@ -7,6 +7,7 @@ describe MusicSessionUserHistory do
   let(:user_history1) { FactoryGirl.create(:music_session_user_history, :history => music_session.music_session, :user => music_session.creator) }
   let(:user_history2) { FactoryGirl.create(:music_session_user_history, :history => music_session.music_session, :user => some_user) }
 
+
   after {
     Timecop.return
   }
@@ -19,6 +20,20 @@ describe MusicSessionUserHistory do
 
   describe "rating" do
 
+    def stub_app_config
+      #stubbing APP_CONFIG 
+      app_config = Class.new{
+        def rating_dialog_min_time
+          60
+        end
+
+        def rating_dialog_min_num
+          1
+        end
+      }
+      stub_const("APP_CONFIG", app_config.new)
+    end
+   
     it "success" do
       user_history1.update_attribute(:rating, 1)
       expect( user_history1.errors.any? ).to eq(false)
@@ -31,12 +46,15 @@ describe MusicSessionUserHistory do
     end
 
     it 'should rate success' do
+      stub_app_config
       users = [user_history1, user_history2]
-      Timecop.travel(Time.now + (MusicSessionUserHistory::MIN_SESSION_DURATION_RATING * 1.5).seconds)
+      #Timecop.travel(Time.now + (MusicSessionUserHistory::MIN_SESSION_DURATION_RATING * 1.5).seconds)
+      Timecop.travel(Time.now + (APP_CONFIG.rating_dialog_min_time * 1.5).seconds)
       expect( user_history1.should_rate_session? ).to eq(true)
     end
 
     it 'should rate fails' do
+      stub_app_config
       users = [user_history1]
       expect( user_history1.should_rate_session? ).to eq(false)
       users = [user_history1, user_history2]
@@ -54,6 +72,13 @@ describe MusicSessionUserHistory do
       user_history1.max_concurrent_connections.should == 2
     end
 
+    it "when history1 only" do
+      user_history1.save.should be_true
+      user_history1.reload
+      user_history1.end_history
+      user_history1.max_concurrent_connections.should == 1
+    end
+
     it "history2 ends before history1 starts" do
       user_history2.created_at = user_history1.created_at - 2
       user_history2.session_removed_at = user_history1.created_at - 1
@@ -74,7 +99,37 @@ describe MusicSessionUserHistory do
       user_history1.max_concurrent_connections.should == 1
     end
 
-    it "history2 is within bounds of history1 " do
+    it "history2 begin before history1 starts and terminates before history1 end" do
+      user_history1.session_removed_at = user_history1.created_at + 3
+      user_history2.created_at = user_history1.created_at - 1
+      user_history2.session_removed_at = user_history1.created_at + 2
+
+      user_history2.save.should be_true
+      
+      user_history1.reload
+      user_history2.reload
+      user_history1.end_history
+      
+      user_history1.max_concurrent_connections.should == 2
+    end
+
+    it "history2 begin after history1 starts and terminates after history1 end" do
+      user_history1.session_removed_at = user_history1.created_at + 3
+      user_history2.created_at = user_history1.created_at + 1
+      user_history2.session_removed_at = user_history1.created_at + 4
+
+      user_history1.save.should be_true
+      user_history2.save.should be_true
+      
+      user_history1.reload
+      user_history2.reload
+      user_history1.end_history
+      
+      user_history1.max_concurrent_connections.should == 2
+    end
+
+
+    it "history2 is within bounds of history1 " do 
       user_history1.session_removed_at = user_history1.created_at + 3
       user_history2.created_at = user_history1.created_at + 1
       user_history2.session_removed_at = user_history1.created_at + 2
@@ -147,6 +202,90 @@ describe MusicSessionUserHistory do
       user_history1.max_concurrent_connections.should == 3
     end
   end
+
+  describe "duration_minutes" do
+    it "returns zero when history2 ends before history1 starts" do
+      
+      user_history1.session_removed_at = user_history1.created_at + 5
+      user_history2.created_at = user_history1.created_at - 2
+      user_history2.session_removed_at = user_history1.created_at - 1
+      
+      user_history2.save.should be_true
+
+      user_history1.reload
+      user_history2.reload
+      user_history1.end_history
+      
+      expect(user_history1.duration_minutes).to eq 0
+
+    end
+
+
+    it "returns zero when history2 starts after history1 ends" do
+      
+      user_history1.session_removed_at = user_history1.created_at + 5
+      user_history2.created_at = user_history1.session_removed_at + 1
+      user_history2.session_removed_at = user_history1.session_removed_at + 2
+      
+      user_history2.save.should be_true
+
+      user_history1.reload
+      user_history2.reload
+      user_history1.end_history
+      
+      #user_history1.max_concurrent_connections.should == 1
+
+      expect(user_history1.duration_minutes).to eq 0
+
+    end
+
+
+    it "returns overlapped time when history2 is within bounds of history1" do
+      #pending
+      user_history1.session_removed_at = user_history1.created_at + 3*60
+      user_history2.created_at = user_history1.created_at + 1*60
+      user_history2.session_removed_at = user_history1.created_at + 3*60
+      
+      user_history1.save.should be_true
+      user_history2.save.should be_true
+      
+      user_history1.reload
+      user_history2.reload
+      user_history1.end_history
+      #user_history1.max_concurrent_connections.should == 2
+      expect(user_history1.duration_minutes).to eq 2
+    end
+
+    it "returns overlapped time when history2 begings before history1 start and terminates before history1 end" do
+      user_history1.session_removed_at = user_history1.created_at + 3*60
+      user_history2.created_at = user_history1.created_at - 1*60
+      user_history2.session_removed_at = user_history1.created_at + 2*60
+      
+      expect(user_history1.save).to eq true
+      expect(user_history2.save).to eq true
+
+      user_history1.reload
+      user_history2.reload
+      user_history1.end_history
+
+      expect(user_history1.duration_minutes).to eq 2
+    end
+
+
+    it "returns overlapped time when history2 begings after history1 start and terminates after history1 end" do
+      user_history1.session_removed_at = user_history1.created_at + 3*60
+      user_history2.created_at = user_history1.created_at + 1*60
+      user_history2.session_removed_at = user_history1.created_at + 4*60
+
+      expect(user_history1.save).to eq true
+      expect(user_history2.save).to eq true
+
+      user_history1.end_history
+
+      expect(user_history1.duration_minutes).to eq 2
+
+    end
+  end
 end