Skip to content

Commit

Permalink
Fix invalid link click registrations
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
knadh committed Oct 24, 2020
1 parent 7cecbbb commit a1aeba2
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 15 deletions.
9 changes: 5 additions & 4 deletions cmd/public.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 5 additions & 0 deletions internal/migrations/v0.8.0.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
18 changes: 9 additions & 9 deletions queries.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
4 changes: 2 additions & 2 deletions schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit a1aeba2

Please sign in to comment.