updated putBundle with add policy conditions functionality with unit, component tests

api/bundle_update_test.go

@@ -47,15 +47,15 @@ func TestPutBundle_Success(t *testing.T) {
 			CreatedBy:    &models.User{Email: "creator@example.com"},
 			UpdatedAt:    &now,
 			ManagedBy:    models.ManagedByDataAdmin,
-			PreviewTeams: &[]models.PreviewTeam{{ID: "team-1"}},
+			PreviewTeams: &[]models.PreviewTeam{{ID: team1}},
 		}
 
 		updateRequest := &models.Bundle{
 			Title:        title1,
 			BundleType:   models.BundleTypeManual,
 			State:        models.BundleStateDraft,
 			ManagedBy:    models.ManagedByDataAdmin,
-			PreviewTeams: &[]models.PreviewTeam{{ID: "team-1"}},
+			PreviewTeams: &[]models.PreviewTeam{{ID: team1}},
 		}
 
 		updateRequestJSON, err := json.Marshal(updateRequest)
@@ -145,7 +145,7 @@ func TestPutBundle_Success(t *testing.T) {
 
 		Convey("When putBundle is called with new preview teams and an existing preview team", func() {
 			bundleRequest := updateRequest
-			bundleRequest.PreviewTeams = &[]models.PreviewTeam{{ID: "team-1"}, {ID: "team-2"}, {ID: "team-3"}}
+			bundleRequest.PreviewTeams = &[]models.PreviewTeam{{ID: team1}, {ID: "team-2"}, {ID: "team-3"}}
 			bundleRequestJSON, err := json.Marshal(bundleRequest)
 			So(err, ShouldBeNil)
 
@@ -178,6 +178,137 @@ func TestPutBundle_Success(t *testing.T) {
 	})
 }
 
+func TestPutBundle_BackfillsPolicyConditionsForNewPreviewTeams(t *testing.T) {
+	t.Parallel()
+
+	Convey("Given a valid PUT request that adds a new preview team to a bundle with existing content", t, func() {
+		now := time.Now().UTC()
+		existingBundle := &models.Bundle{
+			ID:           bundle1,
+			Title:        "Original Title",
+			BundleType:   models.BundleTypeManual,
+			ETag:         "original-etag",
+			State:        models.BundleStateDraft,
+			CreatedAt:    &now,
+			CreatedBy:    &models.User{Email: "publisher1@example.com"},
+			UpdatedAt:    &now,
+			ManagedBy:    models.ManagedByDataAdmin,
+			PreviewTeams: &[]models.PreviewTeam{{ID: team1}},
+		}
+
+		updateRequest := &models.Bundle{
+			Title:        title1,
+			BundleType:   models.BundleTypeManual,
+			State:        models.BundleStateDraft,
+			ManagedBy:    models.ManagedByDataAdmin,
+			PreviewTeams: &[]models.PreviewTeam{{ID: team1}, {ID: "team-2"}},
+		}
+
+		updateRequestJSON, err := json.Marshal(updateRequest)
+		So(err, ShouldBeNil)
+
+		mockedDatastore := &storetest.StorerMock{
+			GetBundleFunc: func(ctx context.Context, id string) (*models.Bundle, error) {
+				if id == bundle1 {
+					return existingBundle, nil
+				}
+				return nil, apierrors.ErrBundleNotFound
+			},
+			CheckBundleExistsByTitleUpdateFunc: func(ctx context.Context, title, excludeID string) (bool, error) {
+				if title == title1 && excludeID == bundle1 {
+					return false, nil
+				}
+				return false, nil
+			},
+			UpdateBundleFunc: func(ctx context.Context, bundleID string, bundle *models.Bundle) (*models.Bundle, error) {
+				if bundleID == bundle1 {
+					bundle.ID = bundleID
+					return bundle, nil
+				}
+				return nil, errors.New("failed to update bundle")
+			},
+			UpdateBundleETagFunc: func(ctx context.Context, bundleID, email string) (*models.Bundle, error) {
+				if bundleID == bundle1 {
+					updatedBundle := *existingBundle
+					updatedBundle.Title = title1
+					updatedBundle.ETag = newEtag
+					updatedBundle.PreviewTeams = &[]models.PreviewTeam{{ID: team1}, {ID: "team-2"}}
+					return &updatedBundle, nil
+				}
+				return nil, errors.New("failed to update ETag")
+			},
+			CreateEventFunc: func(ctx context.Context, event *models.Event) error {
+				if event.Action == models.ActionUpdate && event.Resource == urlString {
+					return nil
+				}
+				return errors.New("failed to create event")
+			},
+			GetContentItemsByBundleIDFunc: func(ctx context.Context, bundleID string) ([]*models.ContentItem, error) {
+				return []*models.ContentItem{
+					{
+						ID: "content-1",
+						Metadata: models.Metadata{
+							DatasetID: "staticDataset1",
+							EditionID: "2025",
+						},
+					},
+				}, nil
+			},
+		}
+
+		team2PolicyLookupCount := 0
+		mockPermissionsClient := &permissionsAPISDKMock.ClienterMock{
+			GetPolicyFunc: func(ctx context.Context, id string, headers permissionsAPISDK.Headers) (*permissionsAPIModels.Policy, error) {
+				if id == team1 {
+					return &permissionsAPIModels.Policy{ID: team1}, nil
+				}
+
+				if id == "team-2" {
+					team2PolicyLookupCount++
+					if team2PolicyLookupCount == 1 {
+						return nil, errors.New("404 policy not found")
+					}
+					return &permissionsAPIModels.Policy{ID: "team-2"}, nil
+				}
+
+				return nil, errors.New("404 policy not found")
+			},
+			PostPolicyWithIDFunc: func(ctx context.Context, id string, policy permissionsAPIModels.PolicyInfo, headers permissionsAPISDK.Headers) (*permissionsAPIModels.Policy, error) {
+				return nil, nil
+			},
+			PutPolicyFunc: func(ctx context.Context, id string, policy permissionsAPIModels.Policy, headers permissionsAPISDK.Headers) error {
+				So(id, ShouldEqual, "team-2")
+				So(policy.Condition.Values, ShouldContain, "staticDataset1")
+				So(policy.Condition.Values, ShouldContain, "staticDataset1/2025")
+				So(len(policy.Condition.Values), ShouldEqual, 2)
+				return nil
+			},
+		}
+
+		bundleAPI := GetBundleAPIWithMocks(store.Datastore{Backend: mockedDatastore}, &datasetAPISDKMock.ClienterMock{}, mockPermissionsClient, false)
+
+		Convey("When putBundle is called", func() {
+			r := httptest.NewRequest("PUT", "/bundles/bundle-1", bytes.NewReader(updateRequestJSON))
+			r = mux.SetURLVars(r, map[string]string{"bundle-id": bundle1})
+			r.Header.Set("If-Match", "original-etag")
+			r.Header.Set("Authorization", "Bearer test-auth-token")
+			w := httptest.NewRecorder()
+
+			bundleAPI.putBundle(w, r)
+
+			Convey("Then it should return 200 OK", func() {
+				So(w.Code, ShouldEqual, http.StatusOK)
+			})
+
+			Convey("And create a policy for the new team and backfill existing bundle content conditions", func() {
+				So(len(mockPermissionsClient.GetPolicyCalls()), ShouldEqual, 3)
+				So(len(mockPermissionsClient.PostPolicyWithIDCalls()), ShouldEqual, 1)
+				So(len(mockPermissionsClient.PutPolicyCalls()), ShouldEqual, 1)
+			})
+		})
+	})
+}
+
 func TestPutBundleNoPreviewTeam_Success(t *testing.T) {
 	t.Parallel()
 
@@ -442,7 +573,7 @@ func TestPutBundle_BundleNotFound_Failure(t *testing.T) {
 			BundleType:   models.BundleTypeManual,
 			State:        models.BundleStateDraft,
 			ManagedBy:    models.ManagedByDataAdmin,
-			PreviewTeams: &[]models.PreviewTeam{{ID: "team-1"}},
+			PreviewTeams: &[]models.PreviewTeam{{ID: team1}},
 		}
 
 		updateRequestJSON, err := json.Marshal(updateRequest)
@@ -496,7 +627,7 @@ func TestPutBundle_AuthenticationFailure(t *testing.T) {
 			BundleType:   models.BundleTypeManual,
 			State:        models.BundleStateDraft,
 			ManagedBy:    models.ManagedByDataAdmin,
-			PreviewTeams: &[]models.PreviewTeam{{ID: "team-1"}},
+			PreviewTeams: &[]models.PreviewTeam{{ID: team1}},
 		}
 
 		updateRequestJSON, err := json.Marshal(updateRequest)
@@ -622,7 +753,7 @@ func TestPutBundle_CreateBundlePolicies_Failure(t *testing.T) {
 			BundleType:   models.BundleTypeManual,
 			State:        models.BundleStateDraft,
 			ManagedBy:    models.ManagedByDataAdmin,
-			PreviewTeams: &[]models.PreviewTeam{{ID: "team-1"}},
+			PreviewTeams: &[]models.PreviewTeam{{ID: team1}},
 		}
 
 		updateRequestJSON, err := json.Marshal(updateRequest)

application/policies.go

@@ -3,6 +3,7 @@ package application
 import (
 	"context"
 	"net/http"
+	"sort"
 	"strconv"
 	"strings"
 
@@ -311,13 +312,34 @@ func (s *StateMachineBundleAPI) AddPolicyConditionsForAddedPreviewTeams(ctx cont
 		}
 
 		for value := range valuesToAdd {
-			if !existingValues[value] {
-				policy.Condition.Values = append(policy.Condition.Values, value)
+			existingValues[value] = true
+		}
+
+		updatedValues := make([]string, 0, len(existingValues))
+		for value := range existingValues {
+			updatedValues = append(updatedValues, value)
+		}
+		sort.Strings(updatedValues)
+
+		currentValues := append([]string{}, policy.Condition.Values...)
+		sort.Strings(currentValues)
+
+		valuesChanged := len(currentValues) != len(updatedValues)
+		if !valuesChanged {
+			for i := range updatedValues {
+				if updatedValues[i] != currentValues[i] {
+					valuesChanged = true
+					break
+				}
 			}
 		}
 
-		err = s.PermissionsAPIClient.PutPolicy(ctx, team.ID, *policy, permissionsAPISDK.Headers{Authorization: authToken})
-		if err != nil {
+		if !valuesChanged {
+			continue
+		}
+
+		policy.Condition.Values = updatedValues
+		if err := s.PermissionsAPIClient.PutPolicy(ctx, team.ID, *policy, permissionsAPISDK.Headers{Authorization: authToken}); err != nil {
 			return err
 		}
 	}

application/policies_test.go

@@ -748,7 +748,7 @@ func TestAddPolicyConditionsForAddedPreviewTeams(t *testing.T) {
 
 			Convey("Then no duplicate values are added", func() {
 				So(err, ShouldBeNil)
-				So(len(mockPermissionsAPIClient.PutPolicyCalls()), ShouldEqual, 1)
+				So(len(mockPermissionsAPIClient.PutPolicyCalls()), ShouldEqual, 0)
 			})
 		})
 

features/bundle_update.feature

@@ -170,6 +170,64 @@ Feature: Update a bundle - PUT /bundles/{id}
             ]
             """
 
+    Scenario: PUT /bundles/{id} backfills policy condition values when preview team is added after content
+        Given I have these content items:
+            """
+            [
+                {
+                    "id": "content-1",
+                    "bundle_id": "bundle-1",
+                    "content_type": "DATASET",
+                    "metadata": {
+                        "dataset_id": "static-test-dataset",
+                        "edition_id": "time-series",
+                        "version_id": 1
+                    },
+                    "links": {
+                        "edit": "/datasets/static-test-dataset/editions/time-series/versions/1",
+                        "preview": "/datasets/static-test-dataset/editions/time-series/versions/1"
+                    }
+                }
+            ]
+            """
+        And I am an admin user
+        And I set the header "If-Match" to "etag-bundle-1"
+        When I PUT "/bundles/bundle-1"
+            """
+            {
+                "bundle_type": "MANUAL",
+                "preview_teams": [
+                    {
+                        "id": "team-preview-1"
+                    }
+                ],
+                "state": "DRAFT",
+                "title": "Updated Bundle Title",
+                "managed_by": "DATA-ADMIN"
+            }
+            """
+        Then the HTTP status code should be "200"
+        And the following policies should exist:
+            """
+            [
+                {
+                    "id": "team-preview-1",
+                    "role": "datasets-previewer",
+                    "entities": [
+                        "groups/team-preview-1"
+                    ],
+                    "condition": {
+                        "attribute": "dataset_edition",
+                        "operator": "StringEquals",
+                        "values": [
+                            "static-test-dataset",
+                            "static-test-dataset/time-series"
+                        ]
+                    }
+                }
+            ]
+            """
+
     Scenario: PUT /bundles/{id} with state transition from DRAFT to IN_REVIEW
         Given I am an admin user
         And I set the header "If-Match" to "etag-bundle-1"