From b416e0cdd7e3d01f48c1479a76f7a1b43fd94471 Mon Sep 17 00:00:00 2001
From: Peter Sanchez <peter@netlandish.com>
Date: Fri, 28 Feb 2025 21:47:49 -0600
Subject: [PATCH] Adding restrictions to avoid saving multiple bookmarks with
 the same organization. Now the system will not add the same link twice to the
 same organization.

Changelog-changed: No longer allowing duplicate bookmarks to be saved
  under the same organization.
Signed-off-by: Peter Sanchez <peter@netlandish.com>
---
 api/api_test.go                               |  4 +--
 cmd/migrations.go                             |  7 +++++
 core/import.go                                | 28 +++++++++++++++++--
 migrations/0003_add_org_links_unique.down.sql |  1 +
 migrations/0003_add_org_links_unique.up.sql   | 13 +++++++++
 migrations/test_migration.up.sql              |  4 ++-
 models/org_link.go                            | 12 +++++---
 models/schema.sql                             |  1 +
 8 files changed, 60 insertions(+), 10 deletions(-)
 create mode 100644 migrations/0003_add_org_links_unique.down.sql
 create mode 100644 migrations/0003_add_org_links_unique.up.sql

diff --git a/api/api_test.go b/api/api_test.go
index ff76dbf..018aeb4 100644
--- a/api/api_test.go
+++ b/api/api_test.go
@@ -516,7 +516,7 @@ func TestAPI(t *testing.T) {
 
 		orgLinks, err = models.GetOrgLinks(dbCtx, &database.FilterOptions{})
 		c.NoError(err)
-		c.Equal(4, len(orgLinks))
+		c.Equal(3, len(orgLinks))
 
 		tags, err = models.GetTags(dbCtx, &database.FilterOptions{})
 		c.NoError(err)
@@ -742,7 +742,7 @@ func TestAPI(t *testing.T) {
 		op.Var("slug", "personal-org")
 		err := links.Execute(ctx, op, &result)
 		c.NoError(err)
-		c.Equal(2, len(result.OrgLinks.Result))
+		c.Equal(1, len(result.OrgLinks.Result))
 
 		op = gqlclient.NewOperation(q)
 		op.Var("slug", "business_org")
diff --git a/cmd/migrations.go b/cmd/migrations.go
index ce7353f..4552650 100644
--- a/cmd/migrations.go
+++ b/cmd/migrations.go
@@ -38,5 +38,12 @@ func GetMigrations() []migrate.Migration {
 			0,
 			links.MigrateFS,
 		),
+		migrate.FSFileMigration(
+			"0003_add_org_links_unique",
+			"migrations/0003_add_org_links_unique.up.sql",
+			"migrations/0003_add_org_links_unique.down.sql",
+			0,
+			links.MigrateFS,
+		),
 	}
 }
diff --git a/core/import.go b/core/import.go
index d58cfab..12a0aac 100644
--- a/core/import.go
+++ b/core/import.go
@@ -350,6 +350,24 @@ func importOrgLinks(ctx context.Context, objAdapter *importAdapter, baseURLMap m
 	if len(orgLinks) == 0 {
 		return nil
 	}
+
+	orgLinks = func() []*models.OrgLink {
+		oMap := make(map[string]bool)
+		nLinks := make([]*models.OrgLink, 0)
+
+		for _, ol := range orgLinks {
+			olId := fmt.Sprintf("%d:%d", ol.BaseURLID.Int64, ol.OrgID)
+			if _, ok := oMap[olId]; ok {
+				// Found a duplicate, continue
+				continue
+			} else {
+				oMap[olId] = true
+				nLinks = append(nLinks, ol)
+			}
+		}
+		return nLinks
+	}()
+
 	err := models.OrgLinkStoreBatch(ctx, orgLinks)
 	if err != nil {
 		return err
@@ -407,7 +425,10 @@ func ImportFromPinBoard(ctx context.Context, path string,
 	billEnabled := links.BillingEnabled(srv.Config)
 
 	for {
-		var pinBoardList []*pinBoardObj
+		var (
+			pinBoardList []*pinBoardObj
+			count        int
+		)
 		for dcode.More() {
 			var pbObj *pinBoardObj
 			err := dcode.Decode(&pbObj)
@@ -416,12 +437,13 @@ func ImportFromPinBoard(ctx context.Context, path string,
 				continue
 			}
 			pinBoardList = append(pinBoardList, pbObj)
-			if len(pinBoardList) == step {
+			count++
+			if count == step {
 				break
 			}
 		}
 
-		listlen := len(pinBoardList)
+		listlen := count
 		if listlen > 0 {
 			adapter := &importAdapter{
 				elementType: pinBoardType,
diff --git a/migrations/0003_add_org_links_unique.down.sql b/migrations/0003_add_org_links_unique.down.sql
new file mode 100644
index 0000000..0c14c5d
--- /dev/null
+++ b/migrations/0003_add_org_links_unique.down.sql
@@ -0,0 +1 @@
+ALTER TABLE org_links DROP CONSTRAINT unique_base_url_org;
diff --git a/migrations/0003_add_org_links_unique.up.sql b/migrations/0003_add_org_links_unique.up.sql
new file mode 100644
index 0000000..5c872a8
--- /dev/null
+++ b/migrations/0003_add_org_links_unique.up.sql
@@ -0,0 +1,13 @@
+-- Necesary for any existing duplicates
+WITH duplicate_cte AS (
+    SELECT id, 
+           ROW_NUMBER() OVER (PARTITION BY base_url_id, org_id ORDER BY created_on DESC) AS row_num
+    FROM org_links
+)
+DELETE FROM org_links
+WHERE id IN (
+    SELECT id FROM duplicate_cte WHERE row_num > 1
+);
+
+-- Now create the constraint
+ALTER TABLE org_links ADD CONSTRAINT unique_base_url_org UNIQUE (base_url_id, org_id);
diff --git a/migrations/test_migration.up.sql b/migrations/test_migration.up.sql
index 758cdd1..4b9d26f 100644
--- a/migrations/test_migration.up.sql
+++ b/migrations/test_migration.up.sql
@@ -9,11 +9,13 @@ INSERT INTO organizations (owner_id, name, slug, settings) VALUES (2, 'api test
 
 INSERT INTO base_urls (url, hash) VALUES ('http://base.com', 'abcdefg');
 
+INSERT INTO base_urls (url, hash) VALUES ('http://base2.com', 'abcdefg2');
+
 INSERT INTO org_links (title, url, base_url_id, user_id, org_id, visibility, hash) VALUES
     ('Public Business url', 'http://base.com?vis=public', 1, 1, 2, 'PUBLIC', 'hash1');
 
 INSERT INTO org_links (title, url, base_url_id, user_id, org_id, visibility, hash) VALUES
-    ('Private Business url', 'http://base.com?vis=private', 1, 1, 2, 'PRIVATE', 'hash2');
+    ('Private Business url', 'http://base2.com?vis=private', 2, 1, 2, 'PRIVATE', 'hash2');
 
 INSERT INTO domains (name, lookup_name, org_id, level, service, status) VALUES ('short domain', 'short.domain.org', 1, 'SYSTEM', 'SHORT', 'APPROVED');
 INSERT INTO domains (name, lookup_name, org_id, service, status, level) VALUES ('listing domain', 'list.domain.org', 1, 'LIST', 'APPROVED', 'USER');
diff --git a/models/org_link.go b/models/org_link.go
index 2aa9519..67f5e9d 100644
--- a/models/org_link.go
+++ b/models/org_link.go
@@ -148,10 +148,12 @@ func (o *OrgLink) Store(ctx context.Context) error {
 					"unread", "starred", "archive_url", "type", "hash").
 				Values(o.Title, o.URL, o.Description, o.BaseURLID, o.OrgID, o.UserID, o.Visibility,
 					o.Unread, o.Starred, o.ArchiveURL, o.Type, o.Hash).
-				Suffix(`RETURNING id, created_on, updated_on`).
+				Suffix(`ON CONFLICT (base_url_id, org_id) DO UPDATE SET 
+					updated_on = CURRENT_TIMESTAMP 
+					RETURNING id, hash, created_on, updated_on`).
 				PlaceholderFormat(sq.Dollar).
 				RunWith(tx).
-				ScanContext(ctx, &o.ID, &o.CreatedOn, &o.UpdatedOn)
+				ScanContext(ctx, &o.ID, &o.Hash, &o.CreatedOn, &o.UpdatedOn)
 		} else {
 			err = sq.
 				Update("org_links").
@@ -283,7 +285,9 @@ func OrgLinkStoreBatch(ctx context.Context, links []*OrgLink) error {
 				link.UserID, link.Visibility, link.Hash, link.Type, link.Unread)
 		}
 		rows, err := batch.
-			Suffix(`RETURNING id`).
+			Suffix(`ON CONFLICT (base_url_id, org_id) DO UPDATE SET 
+				updated_on = CURRENT_TIMESTAMP 
+				RETURNING id, hash`).
 			PlaceholderFormat(sq.Dollar).
 			RunWith(tx).
 			QueryContext(ctx)
@@ -296,7 +300,7 @@ func OrgLinkStoreBatch(ctx context.Context, links []*OrgLink) error {
 		// Add ID's to new entries
 		for _, link := range links {
 			rows.Next()
-			if err = rows.Scan(&link.ID); err != nil {
+			if err = rows.Scan(&link.ID, &link.Hash); err != nil {
 				return err
 			}
 		}
diff --git a/models/schema.sql b/models/schema.sql
index 93d5d7a..16028d5 100644
--- a/models/schema.sql
+++ b/models/schema.sql
@@ -174,6 +174,7 @@ CREATE TABLE org_links (
   archive_url TEXT DEFAULT '',
   created_on TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP,
   updated_on TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP
+  CONSTRAINT unique_base_url_org UNIQUE (base_url_id, org_id)
 );
 
 CREATE INDEX org_links_id_idx ON org_links (id);
-- 
2.45.3