From 2614b072f2a20f7d9d32a30bd5da54c902948b18 Mon Sep 17 00:00:00 2001 From: Kailash Nadh Date: Tue, 1 Feb 2022 23:40:03 +0530 Subject: [PATCH] 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 --- cmd/init.go | 34 +++++++++++++++++------- cmd/install.go | 8 ++---- cmd/main.go | 13 ++++++--- cmd/queries.go | 2 +- frontend/src/views/CampaignAnalytics.vue | 15 ++++++++++- i18n/cs-cz.json | 2 ++ i18n/de.json | 2 ++ i18n/en.json | 2 ++ i18n/es.json | 2 ++ i18n/fr.json | 2 ++ i18n/hu.json | 2 ++ i18n/it.json | 2 ++ i18n/ml.json | 2 ++ i18n/nl.json | 2 ++ i18n/pl.json | 2 ++ i18n/pt-BR.json | 2 ++ i18n/pt.json | 2 ++ i18n/ro.json | 2 ++ i18n/ru.json | 2 ++ i18n/tr.json | 2 ++ queries.sql | 12 ++++++--- 21 files changed, 89 insertions(+), 25 deletions(-) diff --git a/cmd/init.go b/cmd/init.go index 179d1da02..f50140b49 100644 --- 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 index 1b1be1931..0a82d94cf 100644 --- 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 index 0fb251a09..b75903730 100644 --- 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 index 33bc3e0c9..90c6a9b2f 100644 --- 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 index 285aa053d..ecd20dad1 100644 --- a/frontend/src/views/CampaignAnalytics.vue +++ b/frontend/src/views/CampaignAnalytics.vue @@ -44,6 +44,14 @@ +

+ + +

+ +
@@ -68,6 +76,7 @@