From 2ccbfbf372b29ba48bf079681491c4c46cb348ff Mon Sep 17 00:00:00 2001
From: Danny van Kooten <dannyvankooten@gmail.com>
Date: Thu, 9 Aug 2018 14:43:42 +0200
Subject: [PATCH] add tests for aggregating site-wide stats

---
 pkg/aggregator/aggregator.go  |  7 +++--
 pkg/aggregator/siteviews.go   | 37 ----------------------
 pkg/models/site_stats.go      | 23 ++++++++++++++
 pkg/models/site_stats_test.go | 58 +++++++++++++++++++++++++++++++++++
 4 files changed, 86 insertions(+), 39 deletions(-)
 delete mode 100644 pkg/aggregator/siteviews.go

diff --git a/pkg/aggregator/aggregator.go b/pkg/aggregator/aggregator.go
index c9e63cc..d8fa300 100644
--- a/pkg/aggregator/aggregator.go
+++ b/pkg/aggregator/aggregator.go
@@ -1,9 +1,10 @@
 package aggregator
 
 import (
+	"net/url"
+
 	"github.com/usefathom/fathom/pkg/datastore"
 	"github.com/usefathom/fathom/pkg/models"
-	"net/url"
 
 	log "github.com/sirupsen/logrus"
 )
@@ -73,10 +74,12 @@ func (agg *aggregator) Process(pageviews []*models.Pageview) *results {
 	results := newResults()
 
 	for _, p := range pageviews {
-		err := agg.handleSiteview(results, p)
+		site, err := agg.getSiteStats(results, p.Timestamp)
 		if err != nil {
+			log.Error(err)
 			continue
 		}
+		site.HandlePageview(p)
 
 		err = agg.handlePageview(results, p)
 		if err != nil {
diff --git a/pkg/aggregator/siteviews.go b/pkg/aggregator/siteviews.go
deleted file mode 100644
index 207a67f..0000000
--- a/pkg/aggregator/siteviews.go
+++ /dev/null
@@ -1,37 +0,0 @@
-package aggregator
-
-import (
-	log "github.com/sirupsen/logrus"
-	"github.com/usefathom/fathom/pkg/models"
-)
-
-func (agg *aggregator) handleSiteview(results *results, p *models.Pageview) error {
-	site, err := agg.getSiteStats(results, p.Timestamp)
-	if err != nil {
-		log.Error(err)
-		return err
-	}
-
-	site.Pageviews += 1
-
-	if p.Duration > 0.00 {
-		site.KnownDurations += 1
-		site.AvgDuration = site.AvgDuration + ((float64(p.Duration) - site.AvgDuration) * 1 / float64(site.KnownDurations))
-	}
-
-	if p.IsNewVisitor {
-		site.Visitors += 1
-	}
-
-	if p.IsNewSession {
-		site.Sessions += 1
-
-		if p.IsBounce {
-			site.BounceRate = ((float64(site.Sessions-1) * site.BounceRate) + 1) / (float64(site.Sessions))
-		} else {
-			site.BounceRate = ((float64(site.Sessions-1) * site.BounceRate) + 0) / (float64(site.Sessions))
-		}
-	}
-
-	return nil
-}
diff --git a/pkg/models/site_stats.go b/pkg/models/site_stats.go
index ff2cdfc..cd92326 100644
--- a/pkg/models/site_stats.go
+++ b/pkg/models/site_stats.go
@@ -18,3 +18,26 @@ type SiteStats struct {
 func (s *SiteStats) FormattedDuration() string {
 	return fmt.Sprintf("%d:%d", int(s.AvgDuration/60.00), (int(s.AvgDuration) % 60))
 }
+
+func (s *SiteStats) HandlePageview(p *Pageview) {
+	s.Pageviews += 1
+
+	if p.Duration > 0.00 {
+		s.KnownDurations += 1
+		s.AvgDuration = s.AvgDuration + ((float64(p.Duration) - s.AvgDuration) * 1 / float64(s.KnownDurations))
+	}
+
+	if p.IsNewVisitor {
+		s.Visitors += 1
+	}
+
+	if p.IsNewSession {
+		s.Sessions += 1
+
+		if p.IsBounce {
+			s.BounceRate = ((float64(s.Sessions-1) * s.BounceRate) + 1) / (float64(s.Sessions))
+		} else {
+			s.BounceRate = ((float64(s.Sessions-1) * s.BounceRate) + 0) / (float64(s.Sessions))
+		}
+	}
+}
diff --git a/pkg/models/site_stats_test.go b/pkg/models/site_stats_test.go
index 4a15e8f..d56824e 100644
--- a/pkg/models/site_stats_test.go
+++ b/pkg/models/site_stats_test.go
@@ -19,3 +19,61 @@ func TestSiteStatsFormattedDuration(t *testing.T) {
 		t.Errorf("FormattedDuration: expected %s, got %s", e, v)
 	}
 }
+
+func TestSiteStatsHandlePageview(t *testing.T) {
+	s := SiteStats{}
+	p1 := &Pageview{
+		Duration:     100,
+		IsBounce:     false,
+		IsNewVisitor: true,
+		IsNewSession: true,
+	}
+	p2 := &Pageview{
+		Duration:     60,
+		IsNewVisitor: false,
+		IsNewSession: false,
+		IsBounce:     true, // should have no effect because only new sessions can bounce
+	}
+	p3 := &Pageview{
+		IsNewSession: true,
+		IsBounce:     true,
+	}
+
+	// add first pageview & test
+	s.HandlePageview(p1)
+	if s.Pageviews != 1 {
+		t.Errorf("Pageviews: expected %d, got %d", 1, s.Pageviews)
+	}
+	if s.Visitors != 1 {
+		t.Errorf("Visitors: expected %d, got %d", 1, s.Visitors)
+	}
+	if s.AvgDuration != 100 {
+		t.Errorf("AvgDuration: expected %.2f, got %.2f", 100.00, s.AvgDuration)
+	}
+	if s.BounceRate != 0.00 {
+		t.Errorf("BounceRate: expected %.2f, got %.2f", 0.00, s.BounceRate)
+	}
+
+	// add second pageview
+	s.HandlePageview(p2)
+	if s.Pageviews != 2 {
+		t.Errorf("Pageviews: expected %d, got %d", 2, s.Pageviews)
+	}
+	if s.Visitors != 1 {
+		t.Errorf("Visitors: expected %d, got %d", 1, s.Visitors)
+	}
+	if s.AvgDuration != 80 {
+		t.Errorf("AvgDuration: expected %.2f, got %.2f", 80.00, s.AvgDuration)
+	}
+	// should still be 0.00 because p2 was not a new session
+	if s.BounceRate != 0.00 {
+		t.Errorf("BounceRate: expected %.2f, got %.2f", 0.00, s.BounceRate)
+	}
+
+	// add third pageview
+	s.HandlePageview(p3)
+	if s.BounceRate != 0.50 {
+		t.Errorf("BounceRate: expected %.2f, got %.2f", 0.50, s.BounceRate)
+	}
+
+}