Refactor campaign analytics to show unique / non-unique data.

The analytics page showed non-unique counts for views and clicks which was misleading and source of confusion: #522, #561, #571, #676, #680 This commit changes this behaviour to pull unique views and clicks when individual subscriber tracking is turned on in settings, and non-unique counts when it is turned off (as `subscriber_id` in `campaign_views` and `link_clicks` will be NULL, rendering unique queries dysfunctional). This commit changes the stats SQL queries to use string interpolation to either to SELECT `*` or `DISTINCT subscriber_id` on app boot based on the setting in the DB. This involves significant changes to how queries are read and prepared on init. - Refactor `initQueries()` to `readQueries()` and `prepareQueries()`. - Read queries first before preparing. - Load settings from the DB using the read settings query. - Prepare queries next. Use the privacy setting from the DB to apply string interpolation to the analytics queries to pull unique/non-unique before preparing the queries. On the UI: - Show a note on the analytics page about unique/non-unique counts. - Hide the % donut charts on the analytics page in non-unique mode. Closes #676, closes #680
knadh · Feb 1, 2022 · 2614b07 · 2614b07
1 parent d0b32b9
commit 2614b07
Show file tree

Hide file tree

Showing 21 changed files with 89 additions and 25 deletions.
diff --git a/cmd/init.go b/cmd/init.go
@@ -249,9 +249,8 @@ func initDB() *sqlx.DB {
 	return db
 }
 
-// initQueries loads named SQL queries from the queries file and optionally
-// prepares them.
-func initQueries(sqlFile string, db *sqlx.DB, fs stuffbin.FileSystem, prepareQueries bool) (goyesql.Queries, *Queries) {
+// readQueries reads named SQL queries from the SQL queries file into a query map.
+func readQueries(sqlFile string, db *sqlx.DB, fs stuffbin.FileSystem) goyesql.Queries {
 	// Load SQL queries.
 	qB, err := fs.Read(sqlFile)
 	if err != nil {
@@ -262,23 +261,38 @@ func initQueries(sqlFile string, db *sqlx.DB, fs stuffbin.FileSystem, prepareQue
 		lo.Fatalf("error parsing SQL queries: %v", err)
 	}
 
-	if !prepareQueries {
-		return qMap, nil
+	return qMap
+}
+
+// prepareQueries queries prepares a query map and returns a *Queries
+func prepareQueries(qMap goyesql.Queries, db *sqlx.DB, ko *koanf.Koanf) *Queries {
+	// The campaign view/click count queries have a COUNT(%s) placeholder that should either
+	// be substituted with * to pull non-unique rows when individual subscriber tracking is off
+	// as all subscriber_ids will be null, or with DISTINCT subscriber_id when tracking is on
+	// to only pull unique rows per subscriber.
+	sel := "*"
+	if ko.Bool("privacy.individual_tracking") {
+		sel = "DISTINCT subscriber_id"
+	}
+
+	keys := []string{"get-campaign-view-counts", "get-campaign-click-counts", "get-campaign-link-counts"}
+	for _, k := range keys {
+		qMap[k].Query = fmt.Sprintf(qMap[k].Query, sel)
 	}
 
-	// Prepare queries.
+	// Scan and prepare all queries.
 	var q Queries
 	if err := goyesqlx.ScanToStruct(&q, qMap, db.Unsafe()); err != nil {
 		lo.Fatalf("error preparing SQL queries: %v", err)
 	}
 
-	return qMap, &q
+	return &q
 }
 
-// initSettings loads settings from the DB.
-func initSettings(q *sqlx.Stmt) {
+// initSettings loads settings from the DB into the given Koanf map.
+func initSettings(query string, db *sqlx.DB, ko *koanf.Koanf) {
 	var s types.JSONText
-	if err := q.Get(&s); err != nil {
+	if err := db.Get(&s, query); err != nil {
 		lo.Fatalf("error reading settings from DB: %s", pqErrMsg(err))
 	}
 

diff --git a/cmd/install.go b/cmd/install.go
@@ -10,7 +10,6 @@ import (
 
 	"github.com/gofrs/uuid"
 	"github.com/jmoiron/sqlx"
-	goyesqlx "github.com/knadh/goyesql/v2/sqlx"
 	"github.com/knadh/listmonk/models"
 	"github.com/knadh/stuffbin"
 	"github.com/lib/pq"
@@ -19,7 +18,7 @@ import (
 // install runs the first time setup of creating and
 // migrating the database and creating the super user.
 func install(lastVer string, db *sqlx.DB, fs stuffbin.FileSystem, prompt, idempotent bool) {
-	qMap, _ := initQueries(queryFilePath, db, fs, false)
+	qMap := readQueries(queryFilePath, db, fs)
 
 	fmt.Println("")
 	if !idempotent {
@@ -62,10 +61,7 @@ func install(lastVer string, db *sqlx.DB, fs stuffbin.FileSystem, prompt, idempo
 	}
 
 	// Load the queries.
-	var q Queries
-	if err := goyesqlx.ScanToStruct(&q, qMap, db.Unsafe()); err != nil {
-		lo.Fatalf("error loading SQL queries: %v", err)
-	}
+	q := prepareQueries(qMap, db, ko)
 
 	// Sample list.
 	var (

diff --git a/cmd/main.go b/cmd/main.go
@@ -140,11 +140,16 @@ func init() {
 	// Before the queries are prepared, see if there are pending upgrades.
 	checkUpgrade(db)
 
-	// Load the SQL queries from the filesystem.
-	_, queries := initQueries(queryFilePath, db, fs, true)
+	// Read the SQL queries from the queries file.
+	qMap := readQueries(queryFilePath, db, fs)
 
 	// Load settings from DB.
-	initSettings(queries.GetSettings)
+	if q, ok := qMap["get-settings"]; ok {
+		initSettings(q.Query, db, ko)
+	}
+
+	// Prepare queries.
+	queries = prepareQueries(qMap, db, ko)
 }
 
 func main() {
@@ -163,7 +168,7 @@ func main() {
 	// Load i18n language map.
 	app.i18n = initI18n(app.constants.Lang, fs)
 
-	_, app.queries = initQueries(queryFilePath, db, fs, true)
+	app.queries = queries
 	app.manager = initCampaignManager(app.queries, app.constants, app)
 	app.importer = initImporter(app.queries, db, app)
 	app.notifTpls = initNotifTemplates("/email-templates/*.html", fs, app.i18n, app.constants)

diff --git a/cmd/queries.go b/cmd/queries.go
@@ -60,8 +60,8 @@ type Queries struct {
 	GetCampaignStatus        *sqlx.Stmt `query:"get-campaign-status"`
 	GetCampaignViewCounts    *sqlx.Stmt `query:"get-campaign-view-counts"`
 	GetCampaignClickCounts   *sqlx.Stmt `query:"get-campaign-click-counts"`
-	GetCampaignBounceCounts  *sqlx.Stmt `query:"get-campaign-bounce-counts"`
 	GetCampaignLinkCounts    *sqlx.Stmt `query:"get-campaign-link-counts"`
+	GetCampaignBounceCounts  *sqlx.Stmt `query:"get-campaign-bounce-counts"`
 	NextCampaigns            *sqlx.Stmt `query:"next-campaigns"`
 	NextCampaignSubscribers  *sqlx.Stmt `query:"next-campaign-subscribers"`
 	GetOneCampaignSubscriber *sqlx.Stmt `query:"get-one-campaign-subscriber"`

diff --git a/frontend/src/views/CampaignAnalytics.vue b/frontend/src/views/CampaignAnalytics.vue
@@ -44,6 +44,14 @@
       </div><!-- columns -->
     </form>
 
+    <p class="is-size-7 mt-2 has-text-grey-light">
+      <template v-if="settings['privacy.individual_tracking']">
+        {{ $t('analytics.isUnique') }}
+      </template>
+      <template v-else>{{ $t('analytics.nonUnique') }}</template>
+    </p>
+
+
     <section class="charts mt-5">
       <div class="chart columns" v-for="(v, k) in charts" :key="k">
         <div class="column is-9">
@@ -68,6 +76,7 @@
 
 <script>
 import Vue from 'vue';
+import { mapState } from 'vuex';
 import dayjs from 'dayjs';
 import c3 from 'c3';
 import { colors } from '../constants';
@@ -390,14 +399,18 @@ export default Vue.extend({
 
         this.charts[typ].chartFn(typ, camps, data);
 
-        if (this.charts[typ].donutFn) {
+        if (this.charts[typ].donutFn && this.settings['privacy.individual_tracking']) {
           this.charts[typ].donutFn(typ, camps, data);
         }
         this.charts[typ].loading = false;
       });
     },
   },
 
+  computed: {
+    ...mapState(['settings']),
+  },
+
   created() {
     const now = dayjs().set('hour', 23).set('minute', 59).set('seconds', 0);
     this.form.to = now.toDate();

diff --git a/i18n/cs-cz.json b/i18n/cs-cz.json
@@ -5,7 +5,9 @@
     "analytics.count": "Count",
     "analytics.fromDate": "From",
     "analytics.invalidDates": "Invalid `from` or `to` dates.",
+    "analytics.isUnique": "The counts are unique per subscriber.",
     "analytics.links": "Links",
+    "analytics.nonUnique": "The counts are non-unique as individual subscriber tracking is turned off.",
     "analytics.title": "Analytics",
     "analytics.toDate": "To",
     "bounces.source": "Zdroj",

diff --git a/i18n/de.json b/i18n/de.json
@@ -5,7 +5,9 @@
     "analytics.count": "Count",
     "analytics.fromDate": "From",
     "analytics.invalidDates": "Invalid `from` or `to` dates.",
+    "analytics.isUnique": "The counts are unique per subscriber.",
     "analytics.links": "Links",
+    "analytics.nonUnique": "The counts are non-unique as individual subscriber tracking is turned off.",
     "analytics.title": "Analytics",
     "analytics.toDate": "To",
     "bounces.source": "Quelle",

diff --git a/i18n/en.json b/i18n/en.json
@@ -5,7 +5,9 @@
     "analytics.count": "Count",
     "analytics.fromDate": "From",
     "analytics.invalidDates": "Invalid `from` or `to` dates.",
+    "analytics.isUnique": "The counts are unique per subscriber.",
     "analytics.links": "Links",
+    "analytics.nonUnique": "The counts are non-unique as individual subscriber tracking is turned off.",
     "analytics.title": "Analytics",
     "analytics.toDate": "To",
     "bounces.source": "Source",

diff --git a/i18n/es.json b/i18n/es.json
@@ -5,7 +5,9 @@
     "analytics.count": "Numero",
     "analytics.fromDate": "Desde",
     "analytics.invalidDates": "La fecha `desde` o `hasta` no es válida.",
+    "analytics.isUnique": "The counts are unique per subscriber.",
     "analytics.links": "Enlaces",
+    "analytics.nonUnique": "The counts are non-unique as individual subscriber tracking is turned off.",
     "analytics.title": "Analíticas",
     "analytics.toDate": "Para",
     "bounces.source": "Fuente",

diff --git a/i18n/fr.json b/i18n/fr.json
@@ -5,7 +5,9 @@
     "analytics.count": "Compte",
     "analytics.fromDate": "Depuis",
     "analytics.invalidDates": "Dates invalides `depuis` ou `au`.",
+    "analytics.isUnique": "The counts are unique per subscriber.",
     "analytics.links": "Liens",
+    "analytics.nonUnique": "The counts are non-unique as individual subscriber tracking is turned off.",
     "analytics.title": "Analyses",
     "analytics.toDate": "Au",
     "bounces.source": "Source",

diff --git a/i18n/hu.json b/i18n/hu.json
@@ -5,7 +5,9 @@
     "analytics.count": "Számláló",
     "analytics.fromDate": "Ki től",
     "analytics.invalidDates": "Érvénytelen `tól` vagy `ig` dátum.",
+    "analytics.isUnique": "The counts are unique per subscriber.",
     "analytics.links": "Linkek",
+    "analytics.nonUnique": "The counts are non-unique as individual subscriber tracking is turned off.",
     "analytics.title": "Analytika",
     "analytics.toDate": "Ki nek",
     "bounces.source": "Forrás",

diff --git a/i18n/it.json b/i18n/it.json
@@ -5,7 +5,9 @@
     "analytics.count": "Count",
     "analytics.fromDate": "From",
     "analytics.invalidDates": "Invalid `from` or `to` dates.",
+    "analytics.isUnique": "The counts are unique per subscriber.",
     "analytics.links": "Links",
+    "analytics.nonUnique": "The counts are non-unique as individual subscriber tracking is turned off.",
     "analytics.title": "Analytics",
     "analytics.toDate": "To",
     "bounces.source": "Source",

diff --git a/i18n/ml.json b/i18n/ml.json
@@ -5,7 +5,9 @@
     "analytics.count": "Count",
     "analytics.fromDate": "From",
     "analytics.invalidDates": "Invalid `from` or `to` dates.",
+    "analytics.isUnique": "The counts are unique per subscriber.",
     "analytics.links": "Links",
+    "analytics.nonUnique": "The counts are non-unique as individual subscriber tracking is turned off.",
     "analytics.title": "Analytics",
     "analytics.toDate": "To",
     "bounces.source": "Source",

diff --git a/i18n/nl.json b/i18n/nl.json
@@ -5,7 +5,9 @@
     "analytics.count": "Aantal",
     "analytics.fromDate": "Van",
     "analytics.invalidDates": "Ongeldige `van` of `tot` datums.",
+    "analytics.isUnique": "The counts are unique per subscriber.",
     "analytics.links": "Links",
+    "analytics.nonUnique": "The counts are non-unique as individual subscriber tracking is turned off.",
     "analytics.title": "Analytics",
     "analytics.toDate": "Tot",
     "bounces.source": "Bron",

diff --git a/i18n/pl.json b/i18n/pl.json
@@ -5,7 +5,9 @@
     "analytics.count": "Count",
     "analytics.fromDate": "From",
     "analytics.invalidDates": "Invalid `from` or `to` dates.",
+    "analytics.isUnique": "The counts are unique per subscriber.",
     "analytics.links": "Linki",
+    "analytics.nonUnique": "The counts are non-unique as individual subscriber tracking is turned off.",
     "analytics.title": "Analityka",
     "analytics.toDate": "Do",
     "bounces.source": "Źródła",

diff --git a/i18n/pt-BR.json b/i18n/pt-BR.json
@@ -5,7 +5,9 @@
     "analytics.count": "Count",
     "analytics.fromDate": "From",
     "analytics.invalidDates": "Invalid `from` or `to` dates.",
+    "analytics.isUnique": "The counts are unique per subscriber.",
     "analytics.links": "Links",
+    "analytics.nonUnique": "The counts are non-unique as individual subscriber tracking is turned off.",
     "analytics.title": "Analytics",
     "analytics.toDate": "To",
     "bounces.source": "Source",

diff --git a/i18n/pt.json b/i18n/pt.json
@@ -5,7 +5,9 @@
     "analytics.count": "Count",
     "analytics.fromDate": "From",
     "analytics.invalidDates": "Invalid `from` or `to` dates.",
+    "analytics.isUnique": "The counts are unique per subscriber.",
     "analytics.links": "Links",
+    "analytics.nonUnique": "The counts are non-unique as individual subscriber tracking is turned off.",
     "analytics.title": "Analytics",
     "analytics.toDate": "To",
     "bounces.source": "Source",

diff --git a/i18n/ro.json b/i18n/ro.json
@@ -5,7 +5,9 @@
     "analytics.count": "Numară",
     "analytics.fromDate": "De la",
     "analytics.invalidDates": "Invalid `de la` sau `la` dată.",
+    "analytics.isUnique": "The counts are unique per subscriber.",
     "analytics.links": "Linkuri",
+    "analytics.nonUnique": "The counts are non-unique as individual subscriber tracking is turned off.",
     "analytics.title": "Analitiza",
     "analytics.toDate": "La",
     "bounces.source": "Sursa",

diff --git a/i18n/ru.json b/i18n/ru.json
@@ -5,7 +5,9 @@
     "analytics.count": "Count",
     "analytics.fromDate": "From",
     "analytics.invalidDates": "Invalid `from` or `to` dates.",
+    "analytics.isUnique": "The counts are unique per subscriber.",
     "analytics.links": "Links",
+    "analytics.nonUnique": "The counts are non-unique as individual subscriber tracking is turned off.",
     "analytics.title": "Analytics",
     "analytics.toDate": "To",
     "bounces.source": "Source",

diff --git a/i18n/tr.json b/i18n/tr.json
@@ -5,7 +5,9 @@
     "analytics.count": "Count",
     "analytics.fromDate": "From",
     "analytics.invalidDates": "Invalid `from` or `to` dates.",
+    "analytics.isUnique": "The counts are unique per subscriber.",
     "analytics.links": "Links",
+    "analytics.nonUnique": "The counts are non-unique as individual subscriber tracking is turned off.",
     "analytics.title": "Analytics",
     "analytics.toDate": "To",
     "bounces.source": "Source",

diff --git a/queries.sql b/queries.sql
@@ -567,21 +567,25 @@ u AS (
 SELECT * FROM camps;
 
 -- name: get-campaign-view-counts
+-- raw: true
+-- %s = * or DISTINCT subscriber_id (prepared based on based on individual tracking=on/off). Prepared on boot.
 WITH intval AS (
     -- For intervals < a week, aggregate counts hourly, otherwise daily.
     SELECT CASE WHEN (EXTRACT (EPOCH FROM ($3::TIMESTAMP - $2::TIMESTAMP)) / 86400) >= 7 THEN 'day' ELSE 'hour' END
 )
-SELECT campaign_id, COUNT(*) AS "count", DATE_TRUNC((SELECT * FROM intval), created_at) AS "timestamp"
+SELECT campaign_id, COUNT(%s) AS "count", DATE_TRUNC((SELECT * FROM intval), created_at) AS "timestamp"
     FROM campaign_views
     WHERE campaign_id=ANY($1) AND created_at >= $2 AND created_at <= $3
     GROUP BY campaign_id, "timestamp" ORDER BY "timestamp" ASC;
 
 -- name: get-campaign-click-counts
+-- raw: true
+-- %s = * or DISTINCT subscriber_id (prepared based on based on individual tracking=on/off). Prepared on boot.
 WITH intval AS (
     -- For intervals < a week, aggregate counts hourly, otherwise daily.
     SELECT CASE WHEN (EXTRACT (EPOCH FROM ($3::TIMESTAMP - $2::TIMESTAMP)) / 86400) >= 7 THEN 'day' ELSE 'hour' END
 )
-SELECT campaign_id, COUNT(*) AS "count", DATE_TRUNC((SELECT * FROM intval), created_at) AS "timestamp"
+SELECT campaign_id, COUNT(%s) AS "count", DATE_TRUNC((SELECT * FROM intval), created_at) AS "timestamp"
     FROM link_clicks
     WHERE campaign_id=ANY($1) AND created_at >= $2 AND created_at <= $3
     GROUP BY campaign_id, "timestamp" ORDER BY "timestamp" ASC;
@@ -597,7 +601,9 @@ SELECT campaign_id, COUNT(*) AS "count", DATE_TRUNC((SELECT * FROM intval), crea
     GROUP BY campaign_id, "timestamp" ORDER BY "timestamp" ASC;
 
 -- name: get-campaign-link-counts
-SELECT COUNT(*) AS "count", url
+-- raw: true
+-- %s = * or DISTINCT subscriber_id (prepared based on based on individual tracking=on/off). Prepared on boot.
+SELECT COUNT(%s) AS "count", url
     FROM link_clicks
     LEFT JOIN links ON (link_clicks.link_id = links.id)
     WHERE campaign_id=ANY($1) AND link_clicks.created_at >= $2 AND link_clicks.created_at <= $3