Adds a mechanism to delete CPA values for a given user (#30330)

* Adds a mechanism to delete CPA values for a given user

This requires improving the Property Value service to enable delete
all values for a given target, so a new method was created that allows
to delete filtering by targetType and targetID (required) and
optionally for a specific groupID in case the caller wants to affect
all values for a target (useful in case you remove a post for example
and want to delete all values pointing to that post regardless of the
feature they belong to) or only those that belong to a specific
feature.

* Fix property value tests

* Fix after merge and update method name

* Fix linter

---------

Co-authored-by: Miguel de la Cruz <miguel@ctrlz.es>
Co-authored-by: Mattermost Build <build@mattermost.com>

server/channels/app/custom_profile_attributes.go

@@ -272,3 +272,21 @@ func (a *App) PatchCPAValues(userID string, fieldValueMap map[string]json.RawMes
 
 	return updatedValues, nil
 }
+
+func (a *App) DeleteCPAValues(userID string) *model.AppError {
+	groupID, err := a.CpaGroupID()
+	if err != nil {
+		return model.NewAppError("DeleteCPAValues", "app.custom_profile_attributes.cpa_group_id.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
+	}
+
+	if err := a.Srv().propertyService.DeletePropertyValuesForTarget(groupID, "user", userID); err != nil {
+		return model.NewAppError("DeleteCPAValues", "app.custom_profile_attributes.delete_property_values_for_user.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
+	}
+
+	message := model.NewWebSocketEvent(model.WebsocketEventCPAValuesUpdated, "", "", "", nil, "")
+	message.Add("user_id", userID)
+	message.Add("values", map[string]json.RawMessage{})
+	a.Publish(message)
+
+	return nil
+}

server/channels/app/custom_profile_attributes_test.go

@@ -698,3 +698,74 @@ func TestPatchCPAValue(t *testing.T) {
 		})
 	})
 }
+
+func TestDeleteCPAValues(t *testing.T) {
+	os.Setenv("MM_FEATUREFLAGS_CUSTOMPROFILEATTRIBUTES", "true")
+	defer os.Unsetenv("MM_FEATUREFLAGS_CUSTOMPROFILEATTRIBUTES")
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	cpaGroupID, cErr := th.App.CpaGroupID()
+	require.NoError(t, cErr)
+
+	userID := model.NewId()
+	otherUserID := model.NewId()
+
+	// Create multiple fields and values for the user
+	var createdFields []*model.PropertyField
+	for i := 1; i <= 3; i++ {
+		field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{
+			GroupID: cpaGroupID,
+			Name:    fmt.Sprintf("Field %d", i),
+			Type:    model.PropertyFieldTypeText,
+		})
+		require.NoError(t, err)
+		createdField, appErr := th.App.CreateCPAField(field)
+		require.Nil(t, appErr)
+		createdFields = append(createdFields, createdField)
+
+		// Create a value for this field
+		value, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(fmt.Sprintf(`"Value %d"`, i)))
+		require.Nil(t, appErr)
+		require.NotNil(t, value)
+	}
+
+	// Verify values exist before deletion
+	values, appErr := th.App.ListCPAValues(userID)
+	require.Nil(t, appErr)
+	require.Len(t, values, 3)
+
+	// Test deleting values for user
+	t.Run("should delete all values for a user", func(t *testing.T) {
+		appErr := th.App.DeleteCPAValues(userID)
+		require.Nil(t, appErr)
+
+		// Verify values are gone
+		values, appErr := th.App.ListCPAValues(userID)
+		require.Nil(t, appErr)
+		require.Empty(t, values)
+	})
+
+	t.Run("should handle deleting values for a user with no values", func(t *testing.T) {
+		appErr := th.App.DeleteCPAValues(otherUserID)
+		require.Nil(t, appErr)
+	})
+
+	t.Run("should not affect values for other users", func(t *testing.T) {
+		// Create values for another user
+		for _, field := range createdFields {
+			value, appErr := th.App.PatchCPAValue(otherUserID, field.ID, json.RawMessage(`"Other user value"`))
+			require.Nil(t, appErr)
+			require.NotNil(t, value)
+		}
+
+		// Delete values for original user
+		appErr := th.App.DeleteCPAValues(userID)
+		require.Nil(t, appErr)
+
+		// Verify other user's values still exist
+		values, appErr := th.App.ListCPAValues(otherUserID)
+		require.Nil(t, appErr)
+		require.Len(t, values, 3)
+	})
+}

server/channels/app/properties/property_value.go

@@ -56,3 +56,7 @@ func (ps *PropertyService) UpsertPropertyValues(values []*model.PropertyValue) (
 func (ps *PropertyService) DeletePropertyValue(groupID, id string) error {
 	return ps.valueStore.Delete(groupID, id)
 }
+
+func (ps *PropertyService) DeletePropertyValuesForTarget(groupID string, targetType string, targetID string) error {
+	return ps.valueStore.DeleteForTarget(groupID, targetType, targetID)
+}

server/channels/store/retrylayer/retrylayer.go

@@ -9337,6 +9337,27 @@ func (s *RetryLayerPropertyValueStore) DeleteForField(id string) error {
 
 }
 
+func (s *RetryLayerPropertyValueStore) DeleteForTarget(groupID string, targetType string, targetID string) error {
+
+	tries := 0
+	for {
+		err := s.PropertyValueStore.DeleteForTarget(groupID, targetType, targetID)
+		if err == nil {
+			return nil
+		}
+		if !isRepeatableError(err) {
+			return err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return err
+		}
+		timepkg.Sleep(100 * timepkg.Millisecond)
+	}
+
+}
+
 func (s *RetryLayerPropertyValueStore) Get(groupID string, id string) (*model.PropertyValue, error) {
 
 	tries := 0

server/channels/store/sqlstore/property_value_store.go

@@ -339,3 +339,26 @@ func (s *SqlPropertyValueStore) DeleteForField(fieldID string) error {
 
 	return nil
 }
+
+func (s *SqlPropertyValueStore) DeleteForTarget(groupID string, targetType string, targetID string) error {
+	if targetType == "" || targetID == "" {
+		return store.NewErrInvalidInput("PropertyValue", "target", "type or id empty")
+	}
+
+	builder := s.getQueryBuilder().
+		Delete("PropertyValues").
+		Where(sq.Eq{
+			"TargetType": targetType,
+			"TargetID":   targetID,
+		})
+
+	if groupID != "" {
+		builder = builder.Where(sq.Eq{"GroupID": groupID})
+	}
+
+	if _, err := s.GetMaster().ExecBuilder(builder); err != nil {
+		return errors.Wrap(err, "property_value_delete_for_target_exec")
+	}
+
+	return nil
+}

server/channels/store/store.go

@@ -1106,6 +1106,7 @@ type PropertyValueStore interface {
 	Upsert(values []*model.PropertyValue) ([]*model.PropertyValue, error)
 	Delete(groupID string, id string) error
 	DeleteForField(id string) error
+	DeleteForTarget(groupID string, targetType string, targetID string) error
 }
 
 type AccessControlPolicyStore interface {

server/channels/store/storetest/mocks/PropertyValueStore.go

@@ -80,6 +80,24 @@ func (_m *PropertyValueStore) DeleteForField(id string) error {
 	return r0
 }
 
+// DeleteForTarget provides a mock function with given fields: groupID, targetType, targetID
+func (_m *PropertyValueStore) DeleteForTarget(groupID string, targetType string, targetID string) error {
+	ret := _m.Called(groupID, targetType, targetID)
+
+	if len(ret) == 0 {
+		panic("no return value specified for DeleteForTarget")
+	}
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(string, string, string) error); ok {
+		r0 = rf(groupID, targetType, targetID)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
 // Get provides a mock function with given fields: groupID, id
 func (_m *PropertyValueStore) Get(groupID string, id string) (*model.PropertyValue, error) {
 	ret := _m.Called(groupID, id)

server/channels/store/storetest/property_value_store.go

@@ -26,6 +26,7 @@ func TestPropertyValueStore(t *testing.T, rctx request.CTX, ss store.Store, s Sq
 	t.Run("DeletePropertyValue", func(t *testing.T) { testDeletePropertyValue(t, rctx, ss) })
 	t.Run("SearchPropertyValues", func(t *testing.T) { testSearchPropertyValues(t, rctx, ss) })
 	t.Run("DeleteForField", func(t *testing.T) { testDeleteForField(t, rctx, ss) })
+	t.Run("DeleteForTarget", func(t *testing.T) { testDeleteForTarget(t, rctx, ss) })
 }
 
 func testCreatePropertyValue(t *testing.T, _ request.CTX, ss store.Store) {
@@ -981,3 +982,133 @@ func testDeleteForField(t *testing.T, _ request.CTX, ss store.Store) {
 	require.NoError(t, err)
 	require.Zero(t, nonDeletedValue.DeleteAt)
 }
+
+func testDeleteForTarget(t *testing.T, _ request.CTX, ss store.Store) {
+	groupID1 := model.NewId()
+	groupID2 := model.NewId()
+	groupID3 := model.NewId()
+	targetID1 := model.NewId()
+	targetID2 := model.NewId()
+	targetType := "test_type"
+
+	// Create test values for first group and first target
+	value1 := &model.PropertyValue{
+		TargetID:   targetID1,
+		TargetType: targetType,
+		GroupID:    groupID1,
+		FieldID:    model.NewId(),
+		Value:      json.RawMessage(`"value 1"`),
+	}
+
+	value2 := &model.PropertyValue{
+		TargetID:   targetID1,
+		TargetType: targetType,
+		GroupID:    groupID1,
+		FieldID:    model.NewId(),
+		Value:      json.RawMessage(`"value 2"`),
+	}
+
+	// Create test value for second group but same target
+	value3 := &model.PropertyValue{
+		TargetID:   targetID1,
+		TargetType: targetType,
+		GroupID:    groupID2,
+		FieldID:    model.NewId(),
+		Value:      json.RawMessage(`"value 3"`),
+	}
+
+	// Create test value for first group but different target
+	value4 := &model.PropertyValue{
+		TargetID:   targetID2,
+		TargetType: targetType,
+		GroupID:    groupID1,
+		FieldID:    model.NewId(),
+		Value:      json.RawMessage(`"value 4"`),
+	}
+
+	// Create test value with different target type
+	value5 := &model.PropertyValue{
+		TargetID:   targetID1,
+		TargetType: "other_type",
+		GroupID:    groupID1,
+		FieldID:    model.NewId(),
+		Value:      json.RawMessage(`"value 5"`),
+	}
+
+	// Create test value for third group and first target
+	value6 := &model.PropertyValue{
+		TargetID:   targetID1,
+		TargetType: targetType,
+		GroupID:    groupID3,
+		FieldID:    model.NewId(),
+		Value:      json.RawMessage(`"value 6"`),
+	}
+
+	for _, value := range []*model.PropertyValue{value1, value2, value3, value4, value5, value6} {
+		_, err := ss.PropertyValue().Create(value)
+		require.NoError(t, err)
+	}
+
+	t.Run("should return error if targetType or targetID is empty", func(t *testing.T) {
+		err := ss.PropertyValue().DeleteForTarget(groupID1, "", targetID1)
+		require.Error(t, err)
+		var eii *store.ErrInvalidInput
+		require.ErrorAs(t, err, &eii)
+
+		err = ss.PropertyValue().DeleteForTarget(groupID1, targetType, "")
+		require.Error(t, err)
+		require.ErrorAs(t, err, &eii)
+
+		// Verify values were not deleted
+		values, err := ss.PropertyValue().GetMany("", []string{value1.ID, value2.ID})
+		require.NoError(t, err)
+		require.NotZero(t, values)
+		require.Len(t, values, 2)
+	})
+
+	t.Run("should delete only property values for a target within the specified group", func(t *testing.T) {
+		// Delete values for the first target in first group
+		err := ss.PropertyValue().DeleteForTarget(groupID1, targetType, targetID1)
+		require.NoError(t, err)
+
+		// Verify values from first group and first target were hard-deleted
+		deletedValues, err := ss.PropertyValue().GetMany("", []string{value1.ID, value2.ID})
+		require.Error(t, err)
+		require.Zero(t, deletedValues)
+
+		// Verify value from second group was not deleted
+		nonDeletedGroupValue, err := ss.PropertyValue().Get("", value3.ID)
+		require.NoError(t, err)
+		require.NotNil(t, nonDeletedGroupValue)
+
+		// Verify value from first group but different target was not deleted
+		nonDeletedTargetValue, err := ss.PropertyValue().Get("", value4.ID)
+		require.NoError(t, err)
+		require.NotNil(t, nonDeletedTargetValue)
+
+		// Verify value with different target type was not deleted
+		nonDeletedTypeValue, err := ss.PropertyValue().Get("", value5.ID)
+		require.NoError(t, err)
+		require.NotNil(t, nonDeletedTypeValue)
+	})
+
+	t.Run("should delete all values for a target regardless of group", func(t *testing.T) {
+		err := ss.PropertyValue().DeleteForTarget("", targetType, targetID1)
+		require.NoError(t, err)
+
+		// Verify values from other groups with targetID1 were deleted
+		deletedValues, err := ss.PropertyValue().GetMany("", []string{value3.ID, value6.ID})
+		require.Error(t, err)
+		require.Zero(t, deletedValues)
+
+		// Verify value with different target ID was not deleted
+		nonDeletedValue, err := ss.PropertyValue().Get("", value4.ID)
+		require.NoError(t, err)
+		require.NotNil(t, nonDeletedValue)
+
+		// Verify value with different target type was not deleted
+		nonDeletedTypeValue, err := ss.PropertyValue().Get("", value5.ID)
+		require.NoError(t, err)
+		require.NotNil(t, nonDeletedTypeValue)
+	})
+}

server/channels/store/timerlayer/timerlayer.go

@@ -7403,6 +7403,22 @@ func (s *TimerLayerPropertyValueStore) DeleteForField(id string) error {
 	return err
 }
 
+func (s *TimerLayerPropertyValueStore) DeleteForTarget(groupID string, targetType string, targetID string) error {
+	start := time.Now()
+
+	err := s.PropertyValueStore.DeleteForTarget(groupID, targetType, targetID)
+
+	elapsed := float64(time.Since(start)) / float64(time.Second)
+	if s.Root.Metrics != nil {
+		success := "false"
+		if err == nil {
+			success = "true"
+		}
+		s.Root.Metrics.ObserveStoreMethodDuration("PropertyValueStore.DeleteForTarget", success, elapsed)
+	}
+	return err
+}
+
 func (s *TimerLayerPropertyValueStore) Get(groupID string, id string) (*model.PropertyValue, error) {
 	start := time.Now()
 

server/i18n/en.json

@@ -5038,6 +5038,10 @@
     "id": "app.custom_profile_attributes.create_property_field.app_error",
     "translation": "Unable to create Custom Profile Attribute field"
   },
+  {
+    "id": "app.custom_profile_attributes.delete_property_values_for_user.app_error",
+    "translation": "Unable to delete Custom Profile Attribute Values for user"
+  },
   {
     "id": "app.custom_profile_attributes.get_property_field.app_error",
     "translation": "Unable to get Custom Profile Attribute field"