Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=209.85.219.169; helo=mail-yb1-f169.google.com; envelope-from=peter@netlandish.com; receiver=<UNKNOWN> 
From: Peter Sanchez <peter@netlandish.com>
To: ~netlandish/links-dev@lists.code.netlandish.com
Cc: Peter Sanchez <peter@netlandish.com>
Subject: [PATCH links] Update how base_url counter is updated. This is a more accurate way of dealing with this.
Date: Wed, 21 May 2025 12:55:13 -0600
Message-ID: <20250521185516.12459-1-peter@netlandish.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

Also always update base_url counter when calling `updateLink` mutation.
Found an edge case where counters were higher than they should be.

Changelog-Changed: How `BaseURL.UpdateCounter` works. Now uses a subselect
  to set proper count
Changelog-Changed: Now will alwyas call `UpdateCounter` when calling the
  `updateLink` mutation. This addresses a small counter bug / edge case.
---
 api/graph/schema.resolvers.go | 61 ++++++++++++++++++-----------------
 models/base_url.go            | 21 ++++++------
 2 files changed, 42 insertions(+), 40 deletions(-)

diff --git a/api/graph/schema.resolvers.go b/api/graph/schema.resolvers.go
index 18a5734..8bd3f3d 100644
--- a/api/graph/schema.resolvers.go
+++ b/api/graph/schema.resolvers.go
@@ -558,12 +558,9 @@ func (r *mutationResolver) AddLink(ctx context.Context, input *model.LinkInput)
 		}
 	}
 
-	// If the link if public, add +1 to the base url counter
-	if OrgLink.Visibility == models.OrgLinkVisibilityPublic {
-		err = BaseURL.UpdateCounter(ctx, true)
-		if err != nil {
-			return nil, err
-		}
+	err = BaseURL.UpdateCounter(ctx)
+	if err != nil {
+		return nil, err
 	}
 
 	c := server.EchoForContext(ctx)
@@ -665,6 +662,8 @@ func (r *mutationResolver) UpdateLink(ctx context.Context, input *model.UpdateLi
 	}
 	srv := server.ForContext(ctx)
 
+	var BaseURL *models.BaseURL
+
 	// If the input has changed
 	if input.URL != nil {
 		if orgLink.Type == models.NoteType {
@@ -673,7 +672,7 @@ func (r *mutationResolver) UpdateLink(ctx context.Context, input *model.UpdateLi
 				WithCode(valid.ErrValidationCode)
 			return nil, nil
 		}
-		BaseURL := &models.BaseURL{
+		BaseURL = &models.BaseURL{
 			URL: links.StripURLFragment(*input.URL),
 		}
 		err = BaseURL.Store(ctx)
@@ -699,17 +698,13 @@ func (r *mutationResolver) UpdateLink(ctx context.Context, input *model.UpdateLi
 			return nil, nil
 		} else if string(*input.Visibility) == models.OrgLinkVisibilityPublic && orgLink.Type == models.NoteType {
 			// NOTE: when making a note public we want to process its url and creata a base url obj
-			BaseURL := &models.BaseURL{
+			BaseURL = &models.BaseURL{
 				URL: orgLink.URL,
 			}
 			err = BaseURL.Store(ctx)
 			if err != nil {
 				return nil, err
 			}
-			err = BaseURL.UpdateCounter(ctx, true)
-			if err != nil {
-				return nil, err
-			}
 			srv.QueueTask("general", core.ParseBaseURLTask(srv, BaseURL, nil))
 		}
 		orgLink.Visibility = string(*input.Visibility)
@@ -773,6 +768,19 @@ func (r *mutationResolver) UpdateLink(ctx context.Context, input *model.UpdateLi
 		}
 	}
 
+	// Update BaseURL counter
+	if BaseURL == nil {
+		BaseURL = &models.BaseURL{ID: orgLink.BaseURLID}
+		err := BaseURL.Load(ctx)
+		if err != nil {
+			return nil, err
+		}
+	}
+	err = BaseURL.UpdateCounter(ctx)
+	if err != nil {
+		return nil, err
+	}
+
 	c := server.EchoForContext(ctx)
 	mdata := make(map[string]any)
 	mdata["org_id"] = org.ID
@@ -860,23 +868,20 @@ func (r *mutationResolver) DeleteLink(ctx context.Context, hash string) (*model.
 	}
 
 	deletedID := link.Hash
-	visibility := link.Visibility
 	baseURLID := link.BaseURLID
 	err = link.Delete(ctx)
 	if err != nil {
 		return nil, err
 	}
-	// If the link if public, add +1 to the base url counter
-	if visibility == models.OrgLinkVisibilityPublic {
-		baseURL := &models.BaseURL{ID: baseURLID}
-		err = baseURL.Load(ctx)
-		if err != nil {
-			return nil, err
-		}
-		err = baseURL.UpdateCounter(ctx, false)
-		if err != nil {
-			return nil, err
-		}
+
+	baseURL := &models.BaseURL{ID: baseURLID}
+	err = baseURL.Load(ctx)
+	if err != nil {
+		return nil, err
+	}
+	err = baseURL.UpdateCounter(ctx)
+	if err != nil {
+		return nil, err
 	}
 
 	c := server.EchoForContext(ctx)
@@ -1001,11 +1006,9 @@ func (r *mutationResolver) AddNote(ctx context.Context, input *model.NoteInput)
 		return nil, err
 	}
 
-	if string(input.Visibility) == models.OrgLinkVisibilityPublic {
-		err = BaseURL.UpdateCounter(ctx, true)
-		if err != nil {
-			return nil, err
-		}
+	err = BaseURL.UpdateCounter(ctx)
+	if err != nil {
+		return nil, err
 	}
 
 	OrgLinkNote := &models.OrgLink{
diff --git a/models/base_url.go b/models/base_url.go
index b552287..affaa04 100644
--- a/models/base_url.go
+++ b/models/base_url.go
@@ -220,23 +220,22 @@ func (b *BaseURL) ToLocalTZ(tz string) error {
 }
 
 // UpdateCounter ...
-func (b *BaseURL) UpdateCounter(ctx context.Context, add bool) error {
-	// XXX Probably should change this to just use a subselect count(*)
-	op := "+"
-	if !add {
-		op = "-"
-	}
-	query := fmt.Sprintf(`
+func (b *BaseURL) UpdateCounter(ctx context.Context) error {
+	query := `
 		UPDATE base_urls
-		SET counter = counter%s1
+		SET counter = (
+			SELECT count(*)
+			FROM org_links
+			WHERE base_url_id=$1
+			AND visibility='PUBLIC'
+		)
 		WHERE id = $1
 		RETURNING (updated_on)
-	`, op)
-	err := database.WithTx(ctx, nil, func(tx *sql.Tx) error {
+	`
+	return database.WithTx(ctx, nil, func(tx *sql.Tx) error {
 		row := tx.QueryRowContext(ctx, query, b.ID)
 		return row.Scan(&b.UpdatedOn)
 	})
-	return err
 }
 
 // TagsToString will convert linked tags to a comma separated string (used in forms)
-- 
2.47.2