Browse Source

Fix invalid link click registrations

The link_clicks.link_id table was NULLable incorrectly. Links that
do not exist should not register a tracking entry. Fix the query
and also update the schema + migration (breaking table change).
Kailash Nadh 4 years ago
parent
commit
a1aeba22bb
4 changed files with 21 additions and 15 deletions
  1. 5 4
      cmd/public.go
  2. 5 0
      internal/migrations/v0.8.0.go
  3. 9 9
      queries.sql
  4. 2 2
      schema.sql

+ 5 - 4
cmd/public.go

@@ -300,13 +300,14 @@ func handleLinkRedirect(c echo.Context) error {
 
 	var url string
 	if err := app.queries.RegisterLinkClick.Get(&url, linkUUID, campUUID, subUUID); err != nil {
-		if err != sql.ErrNoRows {
-			app.log.Printf("error fetching redirect link: %s", err)
+		if pqErr, ok := err.(*pq.Error); ok && pqErr.Column == "link_id" {
+			return c.Render(http.StatusNotFound, tplMessage,
+				makeMsgTpl("Invalid link", "", "The requested link is invalid."))
 		}
 
+		app.log.Printf("error fetching redirect link: %s", err)
 		return c.Render(http.StatusInternalServerError, tplMessage,
-			makeMsgTpl("Error opening link", "",
-				"There was an error opening the link. Please try later."))
+			makeMsgTpl("Error opening link", "", "There was an error opening the link. Please try later."))
 	}
 
 	return c.Redirect(http.StatusTemporaryRedirect, url)

+ 5 - 0
internal/migrations/v0.8.0.go

@@ -13,6 +13,11 @@ func V0_8_0(db *sqlx.DB, fs stuffbin.FileSystem, ko *koanf.Koanf) error {
 		ON CONFLICT DO NOTHING;
 	INSERT INTO settings (key, value) VALUES ('messengers', '[]')
 		ON CONFLICT DO NOTHING;
+
+	-- Link clicks shouldn't exist if there's no corresponding link.
+	-- links_clicks.link_id should have been NOT NULL originally.
+	DELETE FROM link_clicks WHERE link_id is NULL;
+	ALTER TABLE link_clicks ALTER COLUMN link_id SET NOT NULL;
 	`)
 	return err
 }

+ 9 - 9
queries.sql

@@ -671,16 +671,16 @@ DELETE FROM media WHERE id=$1 RETURNING filename;
 INSERT INTO links (uuid, url) VALUES($1, $2) ON CONFLICT (url) DO UPDATE SET url=EXCLUDED.url RETURNING uuid;
 
 -- name: register-link-click
-WITH link AS (
-    SELECT url, links.id AS link_id, campaigns.id as campaign_id, subscribers.id AS subscriber_id FROM links
-    LEFT JOIN campaigns ON (campaigns.uuid = $2)
-    LEFT JOIN subscribers ON (CASE WHEN $3::TEXT != '' THEN subscribers.uuid = $3::UUID ELSE FALSE END)
-    WHERE links.uuid = $1
+WITH link AS(
+    SELECT id, url FROM links WHERE uuid = $1
 )
-INSERT INTO link_clicks (campaign_id, subscriber_id, link_id)
-    VALUES((SELECT campaign_id FROM link), (SELECT subscriber_id FROM link), (SELECT link_id FROM link))
-    RETURNING (SELECT url FROM link);
-
+INSERT INTO link_clicks (campaign_id, subscriber_id, link_id) VALUES(
+    (SELECT id FROM campaigns WHERE uuid = $2),
+    (SELECT id FROM subscribers WHERE
+        (CASE WHEN $3::TEXT != '' THEN subscribers.uuid = $3::UUID ELSE FALSE END)
+    ),
+    (SELECT id FROM link)
+) RETURNING (SELECT url FROM link);
 
 -- name: get-dashboard-charts
 WITH clicks AS (

+ 2 - 2
schema.sql

@@ -145,8 +145,8 @@ CREATE TABLE links (
 
 DROP TABLE IF EXISTS link_clicks CASCADE;
 CREATE TABLE link_clicks (
-    campaign_id      INTEGER REFERENCES campaigns(id) ON DELETE CASCADE ON UPDATE CASCADE,
-    link_id          INTEGER REFERENCES links(id) ON DELETE CASCADE ON UPDATE CASCADE,
+    campaign_id      INTEGER NULL REFERENCES campaigns(id) ON DELETE CASCADE ON UPDATE CASCADE,
+    link_id          INTEGER NOT NULL REFERENCES links(id) ON DELETE CASCADE ON UPDATE CASCADE,
 
     -- Subscribers may be deleted, but the link counts should remain.
     subscriber_id    INTEGER NULL REFERENCES subscribers(id) ON DELETE SET NULL ON UPDATE CASCADE,