From 9bad69a362253740d1e6c573c3a09ef98df98bcb Mon Sep 17 00:00:00 2001
From: gearnode <bryan@frimin.fr>
Date: Wed, 9 Apr 2025 22:12:57 -0700
Subject: [PATCH] Simplify policy update mechanism

Signed-off-by: gearnode <bryan@frimin.fr>
---
 CHANGELOG.md                |  5 +++
 pkg/coredata/policy.go      | 79 +++++++++++--------------------------
 pkg/probo/policy_service.go | 44 +++++++++++++++------
 3 files changed, 59 insertions(+), 69 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 370b2a0fb..9bda07cc4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
 
+### Changed
+
+- Simplified policy data model by removing version field and optimistic concurrency 
+- Refactored policy update flow to load-modify-save pattern
+
 ## [0.4.1] - 2025-04-09
 
 ### Changed
diff --git a/pkg/coredata/policy.go b/pkg/coredata/policy.go
index 2c580300b..badc2f9b6 100644
--- a/pkg/coredata/policy.go
+++ b/pkg/coredata/policy.go
@@ -23,19 +23,9 @@ type (
 		ReviewDate     *time.Time   `db:"review_date"`
 		CreatedAt      time.Time    `db:"created_at"`
 		UpdatedAt      time.Time    `db:"updated_at"`
-		Version        int          `db:"version"`
 	}
 
 	Policies []*Policy
-
-	UpdatePolicyParams struct {
-		ExpectedVersion int
-		Name            *string
-		Content         *string
-		Status          *PolicyStatus
-		ReviewDate      **time.Time
-		OwnerID         *gid.GID
-	}
 )
 
 func (p Policy) CursorKey(orderBy PolicyOrderField) page.CursorKey {
@@ -65,8 +55,7 @@ SELECT
     content,
     review_date,
     created_at,
-    updated_at,
-    version
+    updated_at
 FROM
     policies
 WHERE
@@ -112,8 +101,7 @@ SELECT
     content,
     review_date,
     created_at,
-    updated_at,
-    version
+    updated_at
 FROM
     policies
 WHERE
@@ -160,8 +148,7 @@ INSERT INTO
         content,
         review_date,
         created_at,
-        updated_at,
-        version
+        updated_at
     )
 VALUES (
     @tenant_id,
@@ -173,8 +160,7 @@ VALUES (
     @content,
     @review_date,
     @created_at,
-    @updated_at,
-    @version
+    @updated_at
 );
 `
 
@@ -189,7 +175,6 @@ VALUES (
 		"review_date":     p.ReviewDate,
 		"created_at":      p.CreatedAt,
 		"updated_at":      p.UpdatedAt,
-		"version":         p.Version,
 	}
 	_, err := conn.Exec(ctx, q, args)
 	return err
@@ -217,58 +202,38 @@ func (p *Policy) Update(
 	ctx context.Context,
 	conn pg.Conn,
 	scope Scoper,
-	params UpdatePolicyParams,
 ) error {
 	q := `
-UPDATE policies SET
-    name = COALESCE(@name, name),
-    status = COALESCE(@status, status),
-    content = COALESCE(@content, content),
-    review_date = COALESCE(@review_date, review_date),
-    owner_id = COALESCE(@owner_id, owner_id),
-    updated_at = @updated_at,
-    version = version + 1
+UPDATE
+	policies
+SET
+	name = @name,
+	status = @status,
+	content = @content,
+	review_date = @review_date,
+	owner_id = @owner_id,
+	updated_at = @updated_at
 WHERE %s
     AND id = @policy_id
-    AND version = @expected_version
-RETURNING 
-    id,
-    organization_id,
-    owner_id,
-    name,
-    content,
-    review_date,
-    created_at,
-    updated_at,
-	status,
-    version
 `
 	q = fmt.Sprintf(q, scope.SQLFragment())
 
 	args := pgx.StrictNamedArgs{
-		"policy_id":        p.ID,
-		"expected_version": params.ExpectedVersion,
-		"updated_at":       time.Now(),
-		"name":             params.Name,
-		"content":          params.Content,
-		"status":           params.Status,
-		"review_date":      params.ReviewDate,
-		"owner_id":         params.OwnerID,
+		"policy_id":   p.ID,
+		"updated_at":  time.Now(),
+		"name":        p.Name,
+		"content":     p.Content,
+		"status":      p.Status,
+		"review_date": p.ReviewDate,
+		"owner_id":    p.OwnerID,
 	}
 	maps.Copy(args, scope.SQLArguments())
 
-	rows, err := conn.Query(ctx, q, args)
+	_, err := conn.Exec(ctx, q, args)
 	if err != nil {
-		return fmt.Errorf("cannot query policies: %w", err)
+		return fmt.Errorf("cannot update policy: %w", err)
 	}
 
-	policy, err := pgx.CollectExactlyOneRow(rows, pgx.RowToStructByName[Policy])
-	if err != nil {
-		return fmt.Errorf("cannot collect policy: %w", err)
-	}
-
-	*p = policy
-
 	return nil
 }
 
diff --git a/pkg/probo/policy_service.go b/pkg/probo/policy_service.go
index fa0de7c19..290e6e940 100644
--- a/pkg/probo/policy_service.go
+++ b/pkg/probo/policy_service.go
@@ -105,22 +105,42 @@ func (s *PolicyService) Update(
 	ctx context.Context,
 	req UpdatePolicyRequest,
 ) (*coredata.Policy, error) {
-	params := coredata.UpdatePolicyParams{
-		ExpectedVersion: req.ExpectedVersion,
-		Name:            req.Name,
-		Content:         req.Content,
-		Status:          req.Status,
-		ReviewDate:      &req.ReviewDate,
-		OwnerID:         req.OwnerID,
-	}
-
-	policy := &coredata.Policy{ID: req.ID}
+	policy := &coredata.Policy{}
 
 	err := s.svc.pg.WithTx(
 		ctx,
 		func(conn pg.Conn) error {
-			return policy.Update(ctx, conn, s.svc.scope, params)
-		})
+			if err := policy.LoadByID(ctx, conn, s.svc.scope, req.ID); err != nil {
+				return fmt.Errorf("cannot load policy %q: %w", req.ID, err)
+			}
+
+			if req.Name != nil {
+				policy.Name = *req.Name
+			}
+
+			if req.Content != nil {
+				policy.Content = *req.Content
+			}
+
+			if req.Status != nil {
+				policy.Status = *req.Status
+			}
+
+			if req.ReviewDate != nil {
+				policy.ReviewDate = req.ReviewDate
+			}
+
+			if req.OwnerID != nil {
+				policy.OwnerID = *req.OwnerID
+			}
+
+			if err := policy.Update(ctx, conn, s.svc.scope); err != nil {
+				return fmt.Errorf("cannot update policy: %w", err)
+			}
+
+			return nil
+		},
+	)
 	if err != nil {
 		return nil, err
 	}