From 1ca6f6d6fba116248b7b8dacbd80f11acde8b68f Mon Sep 17 00:00:00 2001 From: Miguel de la Cruz Date: Mon, 31 Mar 2025 18:51:16 +0200 Subject: [PATCH] Adds a groupID filter to the property service methods (#30420) * Adds a groupID filter to the property service methods This allows the property service caller to directly ensure that a given call is only going to affect a field or value that belongs to a given group, instead of (for example) retrieving a property value before deleting it by id to ensure that the value belongs to a specific property group. The groupID filter is optional and has no effect if called with the empty string value. The changes also remove references to input sanitization on trimming the whitespace for the CPA field names and validate at the API level the input for the field patch endpoint. * Fix linter --------- Co-authored-by: Miguel de la Cruz Co-authored-by: Mattermost Build --- .../api4/custom_profile_attributes.go | 14 +- .../channels/app/custom_profile_attributes.go | 15 +- .../app/custom_profile_attributes_test.go | 10 +- .../channels/app/properties/property_field.go | 22 +- .../channels/app/properties/property_value.go | 12 +- .../channels/store/retrylayer/retrylayer.go | 16 +- .../store/sqlstore/property_field_store.go | 12 +- .../store/sqlstore/property_value_store.go | 12 +- server/channels/store/store.go | 8 +- .../storetest/mocks/PropertyFieldStore.go | 28 +-- .../storetest/mocks/PropertyValueStore.go | 28 +-- .../store/storetest/property_field_store.go | 146 ++++++++++++-- .../store/storetest/property_value_store.go | 190 +++++++++++++++++- .../channels/store/timerlayer/timerlayer.go | 16 +- server/i18n/en.json | 4 + server/public/model/property_field.go | 23 ++- server/public/model/property_field_test.go | 37 +++- 17 files changed, 475 insertions(+), 118 deletions(-) diff --git a/server/channels/api4/custom_profile_attributes.go b/server/channels/api4/custom_profile_attributes.go index 9817a4f2a6..c46f0bd82d 100644 --- a/server/channels/api4/custom_profile_attributes.go +++ b/server/channels/api4/custom_profile_attributes.go @@ -61,7 +61,7 @@ func createCPAField(c *Context, w http.ResponseWriter, r *http.Request) { return } - pf.SanitizeInput() + pf.Name = strings.TrimSpace(pf.Name) auditRec := c.MakeAuditRecord("createCPAField", audit.Fail) defer c.LogAuditRec(auditRec) @@ -106,7 +106,17 @@ func patchCPAField(c *Context, w http.ResponseWriter, r *http.Request) { return } - patch.SanitizeInput() + if patch.Name != nil { + *patch.Name = strings.TrimSpace(*patch.Name) + } + if err := patch.IsValid(); err != nil { + if appErr, ok := err.(*model.AppError); ok { + c.Err = appErr + } else { + c.Err = model.NewAppError("createCPAField", "api.custom_profile_attributes.invalid_field_patch", nil, "", http.StatusBadRequest) + } + return + } auditRec := c.MakeAuditRecord("patchCPAField", audit.Fail) defer c.LogAuditRec(auditRec) diff --git a/server/channels/app/custom_profile_attributes.go b/server/channels/app/custom_profile_attributes.go index 3c290fe621..997465dd7c 100644 --- a/server/channels/app/custom_profile_attributes.go +++ b/server/channels/app/custom_profile_attributes.go @@ -137,7 +137,9 @@ func (a *App) PatchCPAField(fieldID string, patch *model.PropertyFieldPatch) (*m return nil, appErr } - patchedField, err := a.Srv().propertyService.UpdatePropertyField(cpaField.ToPropertyField()) + // we've already ensured that the field exists for the CPA group, + // we don't need to specify the groupID for the update + patchedField, err := a.Srv().propertyService.UpdatePropertyField("", cpaField.ToPropertyField()) if err != nil { var nfErr *store.ErrNotFound switch { @@ -161,16 +163,7 @@ func (a *App) DeleteCPAField(id string) *model.AppError { return model.NewAppError("DeleteCPAField", "app.custom_profile_attributes.cpa_group_id.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } - if _, err := a.Srv().propertyService.GetPropertyField(groupID, id); err != nil { - switch { - case errors.Is(err, sql.ErrNoRows): - return model.NewAppError("DeleteCPAField", "app.custom_profile_attributes.property_field_not_found.app_error", nil, "", http.StatusNotFound) - default: - return model.NewAppError("DeleteCPAField", "app.custom_profile_attributes.get_property_field.app_error", nil, "", http.StatusInternalServerError).Wrap(err) - } - } - - if err := a.Srv().propertyService.DeletePropertyField(id); err != nil { + if err := a.Srv().propertyService.DeletePropertyField(groupID, id); err != nil { var nfErr *store.ErrNotFound switch { case errors.As(err, &nfErr): diff --git a/server/channels/app/custom_profile_attributes_test.go b/server/channels/app/custom_profile_attributes_test.go index 9cec3a1a5b..0910622f32 100644 --- a/server/channels/app/custom_profile_attributes_test.go +++ b/server/channels/app/custom_profile_attributes_test.go @@ -390,9 +390,9 @@ func TestDeleteCPAField(t *testing.T) { } t.Run("should fail if the field doesn't exist", func(t *testing.T) { - appErr := th.App.DeleteCPAField(model.NewId()) - require.NotNil(t, appErr) - require.Equal(t, "app.custom_profile_attributes.property_field_not_found.app_error", appErr.Id) + err := th.App.DeleteCPAField(model.NewId()) + require.NotNil(t, err) + require.Equal(t, "app.custom_profile_attributes.property_field_delete.app_error", err.Id) }) t.Run("should not allow to delete a field outside of CPA", func(t *testing.T) { @@ -406,7 +406,7 @@ func TestDeleteCPAField(t *testing.T) { dErr := th.App.DeleteCPAField(field.ID) require.NotNil(t, dErr) - require.Equal(t, "app.custom_profile_attributes.property_field_not_found.app_error", dErr.Id) + require.Equal(t, "app.custom_profile_attributes.property_field_delete.app_error", dErr.Id) }) t.Run("should correctly delete the field", func(t *testing.T) { @@ -622,7 +622,7 @@ func TestPatchCPAValue(t *testing.T) { } createdField, err := th.App.Srv().propertyService.CreatePropertyField(newField) require.NoError(t, err) - err = th.App.Srv().propertyService.DeletePropertyField(createdField.ID) + err = th.App.Srv().propertyService.DeletePropertyField(cpaGroupID, createdField.ID) require.NoError(t, err) userID := model.NewId() diff --git a/server/channels/app/properties/property_field.go b/server/channels/app/properties/property_field.go index 322f53877b..ddf9387642 100644 --- a/server/channels/app/properties/property_field.go +++ b/server/channels/app/properties/property_field.go @@ -4,6 +4,8 @@ package properties import ( + "fmt" + "github.com/mattermost/mattermost/server/public/model" ) @@ -32,8 +34,8 @@ func (ps *PropertyService) SearchPropertyFields(groupID, targetID string, opts m return ps.fieldStore.SearchPropertyFields(opts) } -func (ps *PropertyService) UpdatePropertyField(field *model.PropertyField) (*model.PropertyField, error) { - fields, err := ps.UpdatePropertyFields([]*model.PropertyField{field}) +func (ps *PropertyService) UpdatePropertyField(groupID string, field *model.PropertyField) (*model.PropertyField, error) { + fields, err := ps.UpdatePropertyFields(groupID, []*model.PropertyField{field}) if err != nil { return nil, err } @@ -41,13 +43,21 @@ func (ps *PropertyService) UpdatePropertyField(field *model.PropertyField) (*mod return fields[0], nil } -func (ps *PropertyService) UpdatePropertyFields(fields []*model.PropertyField) ([]*model.PropertyField, error) { - return ps.fieldStore.Update(fields) +func (ps *PropertyService) UpdatePropertyFields(groupID string, fields []*model.PropertyField) ([]*model.PropertyField, error) { + return ps.fieldStore.Update(groupID, fields) } -func (ps *PropertyService) DeletePropertyField(id string) error { +func (ps *PropertyService) DeletePropertyField(groupID, id string) error { + // if groupID is not empty, we need to check first that the field belongs to the group + if groupID != "" { + if _, err := ps.GetPropertyField(groupID, id); err != nil { + return fmt.Errorf("error getting property field %q for group %q: %w", id, groupID, err) + } + } + if err := ps.valueStore.DeleteForField(id); err != nil { return err } - return ps.fieldStore.Delete(id) + + return ps.fieldStore.Delete(groupID, id) } diff --git a/server/channels/app/properties/property_value.go b/server/channels/app/properties/property_value.go index 11b65743d0..2a8775f572 100644 --- a/server/channels/app/properties/property_value.go +++ b/server/channels/app/properties/property_value.go @@ -27,8 +27,8 @@ func (ps *PropertyService) SearchPropertyValues(groupID, targetID string, opts m return ps.valueStore.SearchPropertyValues(opts) } -func (ps *PropertyService) UpdatePropertyValue(value *model.PropertyValue) (*model.PropertyValue, error) { - values, err := ps.UpdatePropertyValues([]*model.PropertyValue{value}) +func (ps *PropertyService) UpdatePropertyValue(groupID string, value *model.PropertyValue) (*model.PropertyValue, error) { + values, err := ps.UpdatePropertyValues(groupID, []*model.PropertyValue{value}) if err != nil { return nil, err } @@ -36,8 +36,8 @@ func (ps *PropertyService) UpdatePropertyValue(value *model.PropertyValue) (*mod return values[0], nil } -func (ps *PropertyService) UpdatePropertyValues(values []*model.PropertyValue) ([]*model.PropertyValue, error) { - return ps.valueStore.Update(values) +func (ps *PropertyService) UpdatePropertyValues(groupID string, values []*model.PropertyValue) ([]*model.PropertyValue, error) { + return ps.valueStore.Update(groupID, values) } func (ps *PropertyService) UpsertPropertyValue(value *model.PropertyValue) (*model.PropertyValue, error) { @@ -53,6 +53,6 @@ func (ps *PropertyService) UpsertPropertyValues(values []*model.PropertyValue) ( return ps.valueStore.Upsert(values) } -func (ps *PropertyService) DeletePropertyValue(id string) error { - return ps.valueStore.Delete(id) +func (ps *PropertyService) DeletePropertyValue(groupID, id string) error { + return ps.valueStore.Delete(groupID, id) } diff --git a/server/channels/store/retrylayer/retrylayer.go b/server/channels/store/retrylayer/retrylayer.go index de877740fa..be4cb0441d 100644 --- a/server/channels/store/retrylayer/retrylayer.go +++ b/server/channels/store/retrylayer/retrylayer.go @@ -9012,11 +9012,11 @@ func (s *RetryLayerPropertyFieldStore) Create(field *model.PropertyField) (*mode } -func (s *RetryLayerPropertyFieldStore) Delete(id string) error { +func (s *RetryLayerPropertyFieldStore) Delete(groupID string, id string) error { tries := 0 for { - err := s.PropertyFieldStore.Delete(id) + err := s.PropertyFieldStore.Delete(groupID, id) if err == nil { return nil } @@ -9096,11 +9096,11 @@ func (s *RetryLayerPropertyFieldStore) SearchPropertyFields(opts model.PropertyF } -func (s *RetryLayerPropertyFieldStore) Update(fields []*model.PropertyField) ([]*model.PropertyField, error) { +func (s *RetryLayerPropertyFieldStore) Update(groupID string, fields []*model.PropertyField) ([]*model.PropertyField, error) { tries := 0 for { - result, err := s.PropertyFieldStore.Update(fields) + result, err := s.PropertyFieldStore.Update(groupID, fields) if err == nil { return result, nil } @@ -9180,11 +9180,11 @@ func (s *RetryLayerPropertyValueStore) Create(value *model.PropertyValue) (*mode } -func (s *RetryLayerPropertyValueStore) Delete(id string) error { +func (s *RetryLayerPropertyValueStore) Delete(groupID string, id string) error { tries := 0 for { - err := s.PropertyValueStore.Delete(id) + err := s.PropertyValueStore.Delete(groupID, id) if err == nil { return nil } @@ -9285,11 +9285,11 @@ func (s *RetryLayerPropertyValueStore) SearchPropertyValues(opts model.PropertyV } -func (s *RetryLayerPropertyValueStore) Update(values []*model.PropertyValue) ([]*model.PropertyValue, error) { +func (s *RetryLayerPropertyValueStore) Update(groupID string, values []*model.PropertyValue) ([]*model.PropertyValue, error) { tries := 0 for { - result, err := s.PropertyValueStore.Update(values) + result, err := s.PropertyValueStore.Update(groupID, values) if err == nil { return result, nil } diff --git a/server/channels/store/sqlstore/property_field_store.go b/server/channels/store/sqlstore/property_field_store.go index 85f07c3ce6..27a294f42b 100644 --- a/server/channels/store/sqlstore/property_field_store.go +++ b/server/channels/store/sqlstore/property_field_store.go @@ -150,7 +150,7 @@ func (s *SqlPropertyFieldStore) SearchPropertyFields(opts model.PropertyFieldSea return fields, nil } -func (s *SqlPropertyFieldStore) Update(fields []*model.PropertyField) (_ []*model.PropertyField, err error) { +func (s *SqlPropertyFieldStore) Update(groupID string, fields []*model.PropertyField) (_ []*model.PropertyField, err error) { if len(fields) == 0 { return nil, nil } @@ -207,6 +207,10 @@ func (s *SqlPropertyFieldStore) Update(fields []*model.PropertyField) (_ []*mode Set("DeleteAt", deleteAtCase). Where(sq.Eq{"id": ids}) + if groupID != "" { + builder = builder.Where(sq.Eq{"GroupID": groupID}) + } + result, err := transaction.ExecBuilder(builder) if err != nil { return nil, errors.Wrap(err, "property_field_update_exec") @@ -227,12 +231,16 @@ func (s *SqlPropertyFieldStore) Update(fields []*model.PropertyField) (_ []*mode return fields, nil } -func (s *SqlPropertyFieldStore) Delete(id string) error { +func (s *SqlPropertyFieldStore) Delete(groupID string, id string) error { builder := s.getQueryBuilder(). Update("PropertyFields"). Set("DeleteAt", model.GetMillis()). Where(sq.Eq{"id": id}) + if groupID != "" { + builder = builder.Where(sq.Eq{"GroupID": groupID}) + } + result, err := s.GetMaster().ExecBuilder(builder) if err != nil { return errors.Wrapf(err, "failed to delete property field with id: %s", id) diff --git a/server/channels/store/sqlstore/property_value_store.go b/server/channels/store/sqlstore/property_value_store.go index a06b2d3547..30cc286409 100644 --- a/server/channels/store/sqlstore/property_value_store.go +++ b/server/channels/store/sqlstore/property_value_store.go @@ -141,7 +141,7 @@ func (s *SqlPropertyValueStore) SearchPropertyValues(opts model.PropertyValueSea return values, nil } -func (s *SqlPropertyValueStore) Update(values []*model.PropertyValue) (_ []*model.PropertyValue, err error) { +func (s *SqlPropertyValueStore) Update(groupID string, values []*model.PropertyValue) (_ []*model.PropertyValue, err error) { if len(values) == 0 { return nil, nil } @@ -186,6 +186,10 @@ func (s *SqlPropertyValueStore) Update(values []*model.PropertyValue) (_ []*mode Set("UpdateAt", updateTime). Where(sq.Eq{"id": ids}) + if groupID != "" { + builder = builder.Where(sq.Eq{"GroupID": groupID}) + } + result, err := transaction.ExecBuilder(builder) if err != nil { return nil, errors.Wrap(err, "property_value_update_exec") @@ -297,12 +301,16 @@ func (s *SqlPropertyValueStore) Upsert(values []*model.PropertyValue) (_ []*mode return updatedValues, nil } -func (s *SqlPropertyValueStore) Delete(id string) error { +func (s *SqlPropertyValueStore) Delete(groupID string, id string) error { builder := s.getQueryBuilder(). Update("PropertyValues"). Set("DeleteAt", model.GetMillis()). Where(sq.Eq{"id": id}) + if groupID != "" { + builder = builder.Where(sq.Eq{"GroupID": groupID}) + } + result, err := s.GetMaster().ExecBuilder(builder) if err != nil { return errors.Wrapf(err, "failed to delete property value with id: %s", id) diff --git a/server/channels/store/store.go b/server/channels/store/store.go index 5f7ebae115..7dd9560591 100644 --- a/server/channels/store/store.go +++ b/server/channels/store/store.go @@ -1092,8 +1092,8 @@ type PropertyFieldStore interface { GetMany(groupID string, ids []string) ([]*model.PropertyField, error) CountForGroup(groupID string, includeDeleted bool) (int64, error) SearchPropertyFields(opts model.PropertyFieldSearchOpts) ([]*model.PropertyField, error) - Update(fields []*model.PropertyField) ([]*model.PropertyField, error) - Delete(id string) error + Update(groupID string, fields []*model.PropertyField) ([]*model.PropertyField, error) + Delete(groupID string, id string) error } type PropertyValueStore interface { @@ -1101,9 +1101,9 @@ type PropertyValueStore interface { Get(groupID, id string) (*model.PropertyValue, error) GetMany(groupID string, ids []string) ([]*model.PropertyValue, error) SearchPropertyValues(opts model.PropertyValueSearchOpts) ([]*model.PropertyValue, error) - Update(values []*model.PropertyValue) ([]*model.PropertyValue, error) + Update(groupID string, values []*model.PropertyValue) ([]*model.PropertyValue, error) Upsert(values []*model.PropertyValue) ([]*model.PropertyValue, error) - Delete(id string) error + Delete(groupID string, id string) error DeleteForField(id string) error } diff --git a/server/channels/store/storetest/mocks/PropertyFieldStore.go b/server/channels/store/storetest/mocks/PropertyFieldStore.go index d4139251ad..58f7b571ce 100644 --- a/server/channels/store/storetest/mocks/PropertyFieldStore.go +++ b/server/channels/store/storetest/mocks/PropertyFieldStore.go @@ -72,17 +72,17 @@ func (_m *PropertyFieldStore) Create(field *model.PropertyField) (*model.Propert return r0, r1 } -// Delete provides a mock function with given fields: id -func (_m *PropertyFieldStore) Delete(id string) error { - ret := _m.Called(id) +// Delete provides a mock function with given fields: groupID, id +func (_m *PropertyFieldStore) Delete(groupID string, id string) error { + ret := _m.Called(groupID, id) if len(ret) == 0 { panic("no return value specified for Delete") } var r0 error - if rf, ok := ret.Get(0).(func(string) error); ok { - r0 = rf(id) + if rf, ok := ret.Get(0).(func(string, string) error); ok { + r0 = rf(groupID, id) } else { r0 = ret.Error(0) } @@ -180,9 +180,9 @@ func (_m *PropertyFieldStore) SearchPropertyFields(opts model.PropertyFieldSearc return r0, r1 } -// Update provides a mock function with given fields: fields -func (_m *PropertyFieldStore) Update(fields []*model.PropertyField) ([]*model.PropertyField, error) { - ret := _m.Called(fields) +// Update provides a mock function with given fields: groupID, fields +func (_m *PropertyFieldStore) Update(groupID string, fields []*model.PropertyField) ([]*model.PropertyField, error) { + ret := _m.Called(groupID, fields) if len(ret) == 0 { panic("no return value specified for Update") @@ -190,19 +190,19 @@ func (_m *PropertyFieldStore) Update(fields []*model.PropertyField) ([]*model.Pr var r0 []*model.PropertyField var r1 error - if rf, ok := ret.Get(0).(func([]*model.PropertyField) ([]*model.PropertyField, error)); ok { - return rf(fields) + if rf, ok := ret.Get(0).(func(string, []*model.PropertyField) ([]*model.PropertyField, error)); ok { + return rf(groupID, fields) } - if rf, ok := ret.Get(0).(func([]*model.PropertyField) []*model.PropertyField); ok { - r0 = rf(fields) + if rf, ok := ret.Get(0).(func(string, []*model.PropertyField) []*model.PropertyField); ok { + r0 = rf(groupID, fields) } else { if ret.Get(0) != nil { r0 = ret.Get(0).([]*model.PropertyField) } } - if rf, ok := ret.Get(1).(func([]*model.PropertyField) error); ok { - r1 = rf(fields) + if rf, ok := ret.Get(1).(func(string, []*model.PropertyField) error); ok { + r1 = rf(groupID, fields) } else { r1 = ret.Error(1) } diff --git a/server/channels/store/storetest/mocks/PropertyValueStore.go b/server/channels/store/storetest/mocks/PropertyValueStore.go index c2296d7183..4fae9dcd65 100644 --- a/server/channels/store/storetest/mocks/PropertyValueStore.go +++ b/server/channels/store/storetest/mocks/PropertyValueStore.go @@ -44,17 +44,17 @@ func (_m *PropertyValueStore) Create(value *model.PropertyValue) (*model.Propert return r0, r1 } -// Delete provides a mock function with given fields: id -func (_m *PropertyValueStore) Delete(id string) error { - ret := _m.Called(id) +// Delete provides a mock function with given fields: groupID, id +func (_m *PropertyValueStore) Delete(groupID string, id string) error { + ret := _m.Called(groupID, id) if len(ret) == 0 { panic("no return value specified for Delete") } var r0 error - if rf, ok := ret.Get(0).(func(string) error); ok { - r0 = rf(id) + if rf, ok := ret.Get(0).(func(string, string) error); ok { + r0 = rf(groupID, id) } else { r0 = ret.Error(0) } @@ -170,9 +170,9 @@ func (_m *PropertyValueStore) SearchPropertyValues(opts model.PropertyValueSearc return r0, r1 } -// Update provides a mock function with given fields: values -func (_m *PropertyValueStore) Update(values []*model.PropertyValue) ([]*model.PropertyValue, error) { - ret := _m.Called(values) +// Update provides a mock function with given fields: groupID, values +func (_m *PropertyValueStore) Update(groupID string, values []*model.PropertyValue) ([]*model.PropertyValue, error) { + ret := _m.Called(groupID, values) if len(ret) == 0 { panic("no return value specified for Update") @@ -180,19 +180,19 @@ func (_m *PropertyValueStore) Update(values []*model.PropertyValue) ([]*model.Pr var r0 []*model.PropertyValue var r1 error - if rf, ok := ret.Get(0).(func([]*model.PropertyValue) ([]*model.PropertyValue, error)); ok { - return rf(values) + if rf, ok := ret.Get(0).(func(string, []*model.PropertyValue) ([]*model.PropertyValue, error)); ok { + return rf(groupID, values) } - if rf, ok := ret.Get(0).(func([]*model.PropertyValue) []*model.PropertyValue); ok { - r0 = rf(values) + if rf, ok := ret.Get(0).(func(string, []*model.PropertyValue) []*model.PropertyValue); ok { + r0 = rf(groupID, values) } else { if ret.Get(0) != nil { r0 = ret.Get(0).([]*model.PropertyValue) } } - if rf, ok := ret.Get(1).(func([]*model.PropertyValue) error); ok { - r1 = rf(values) + if rf, ok := ret.Get(1).(func(string, []*model.PropertyValue) error); ok { + r1 = rf(groupID, values) } else { r1 = ret.Error(1) } diff --git a/server/channels/store/storetest/property_field_store.go b/server/channels/store/storetest/property_field_store.go index a37214c8d1..21a8148658 100644 --- a/server/channels/store/storetest/property_field_store.go +++ b/server/channels/store/storetest/property_field_store.go @@ -182,7 +182,7 @@ func testUpdatePropertyField(t *testing.T, _ request.CTX, ss store.Store) { Type: model.PropertyFieldTypeText, CreateAt: model.GetMillis(), } - updatedField, err := ss.PropertyField().Update([]*model.PropertyField{field}) + updatedField, err := ss.PropertyField().Update("", []*model.PropertyField{field}) require.Zero(t, updatedField) require.ErrorContains(t, err, "failed to update, some property fields were not found, got 0 of 1") }) @@ -198,13 +198,13 @@ func testUpdatePropertyField(t *testing.T, _ request.CTX, ss store.Store) { require.NotZero(t, field.ID) field.GroupID = "" - updatedField, err := ss.PropertyField().Update([]*model.PropertyField{field}) + updatedField, err := ss.PropertyField().Update("", []*model.PropertyField{field}) require.Zero(t, updatedField) require.ErrorContains(t, err, "model.property_field.is_valid.app_error") field.GroupID = model.NewId() field.Name = "" - updatedField, err = ss.PropertyField().Update([]*model.PropertyField{field}) + updatedField, err = ss.PropertyField().Update("", []*model.PropertyField{field}) require.Zero(t, updatedField) require.ErrorContains(t, err, "model.property_field.is_valid.app_error") }) @@ -248,7 +248,7 @@ func testUpdatePropertyField(t *testing.T, _ request.CTX, ss store.Store) { "options": []string{"x", "y", "z"}, } - _, err := ss.PropertyField().Update([]*model.PropertyField{field1, field2}) + _, err := ss.PropertyField().Update("", []*model.PropertyField{field1, field2}) require.NoError(t, err) // Verify first field @@ -303,16 +303,16 @@ func testUpdatePropertyField(t *testing.T, _ request.CTX, ss store.Store) { field1.Name = "Valid update" field2.GroupID = "Invalid ID" - _, err := ss.PropertyField().Update([]*model.PropertyField{field1, field2}) + _, err := ss.PropertyField().Update("", []*model.PropertyField{field1, field2}) require.ErrorContains(t, err, "model.property_field.is_valid.app_error") // Check that fields were not updated - updated1, err := ss.PropertyField().Get(groupID, field1.ID) + updated1, err := ss.PropertyField().Get("", field1.ID) require.NoError(t, err) require.Equal(t, "Field 1", updated1.Name) require.Equal(t, originalUpdateAt1, updated1.UpdateAt) - updated2, err := ss.PropertyField().Get(groupID, field2.ID) + updated2, err := ss.PropertyField().Get("", field2.ID) require.NoError(t, err) require.Equal(t, groupID, updated2.GroupID) require.Equal(t, originalUpdateAt2, updated2.UpdateAt) @@ -347,7 +347,7 @@ func testUpdatePropertyField(t *testing.T, _ request.CTX, ss store.Store) { field1.Name = "Updated First" - _, err = ss.PropertyField().Update([]*model.PropertyField{field1, field2}) + _, err = ss.PropertyField().Update("", []*model.PropertyField{field1, field2}) require.Error(t, err) require.ErrorContains(t, err, "failed to update, some property fields were not found") @@ -357,11 +357,88 @@ func testUpdatePropertyField(t *testing.T, _ request.CTX, ss store.Store) { require.Equal(t, "First field", updated1.Name) require.Equal(t, originalUpdateAt, updated1.UpdateAt) }) + + t.Run("should update fields with matching groupID", func(t *testing.T) { + // Create fields with the same groupID + groupID := model.NewId() + field1 := &model.PropertyField{ + GroupID: groupID, + Name: "Group Field 1", + Type: model.PropertyFieldTypeText, + } + field2 := &model.PropertyField{ + GroupID: groupID, + Name: "Group Field 2", + Type: model.PropertyFieldTypeText, + } + + for _, field := range []*model.PropertyField{field1, field2} { + _, err := ss.PropertyField().Create(field) + require.NoError(t, err) + } + + // Update the fields with the matching groupID + field1.Name = "Updated Group Field 1" + field2.Name = "Updated Group Field 2" + + updatedFields, err := ss.PropertyField().Update(groupID, []*model.PropertyField{field1, field2}) + require.NoError(t, err) + require.Len(t, updatedFields, 2) + + // Verify the fields were updated + for _, field := range []*model.PropertyField{field1, field2} { + updated, err := ss.PropertyField().Get("", field.ID) + require.NoError(t, err) + require.Contains(t, updated.Name, "Updated Group Field") + } + }) + + t.Run("should not update fields with non-matching groupID", func(t *testing.T) { + // Create fields with different groupIDs + groupID1 := model.NewId() + groupID2 := model.NewId() + + field1 := &model.PropertyField{ + GroupID: groupID1, + Name: "Field in Group 1", + Type: model.PropertyFieldTypeText, + } + field2 := &model.PropertyField{ + GroupID: groupID2, + Name: "Field in Group 2", + Type: model.PropertyFieldTypeText, + } + + for _, field := range []*model.PropertyField{field1, field2} { + _, err := ss.PropertyField().Create(field) + require.NoError(t, err) + } + + originalName1 := field1.Name + originalName2 := field2.Name + + // Try to update both fields but filter by groupID1 + field1.Name = "Updated Field in Group 1" + field2.Name = "Updated Field in Group 2" + + _, err := ss.PropertyField().Update(groupID1, []*model.PropertyField{field1, field2}) + require.Error(t, err) + require.ErrorContains(t, err, "failed to update, some property fields were not found") + + // Verify neither field was updated due to transaction rollback + updated1, err := ss.PropertyField().Get("", field1.ID) + require.NoError(t, err) + require.Equal(t, originalName1, updated1.Name) + + updated2, err := ss.PropertyField().Get("", field2.ID) + require.NoError(t, err) + require.Equal(t, originalName2, updated2.Name) + }) } func testDeletePropertyField(t *testing.T, _ request.CTX, ss store.Store) { t.Run("should fail on nonexisting field", func(t *testing.T) { - err := ss.PropertyField().Delete(model.NewId()) + err := ss.PropertyField().Delete("", model.NewId()) var enf *store.ErrNotFound require.ErrorAs(t, err, &enf) }) @@ -377,7 +454,7 @@ func testDeletePropertyField(t *testing.T, _ request.CTX, ss store.Store) { require.NoError(t, err) require.NotEmpty(t, field.ID) - err = ss.PropertyField().Delete(field.ID) + err = ss.PropertyField().Delete("", field.ID) require.NoError(t, err) // Verify the field was soft-deleted @@ -392,6 +469,49 @@ func testDeletePropertyField(t *testing.T, _ request.CTX, ss store.Store) { require.NoError(t, err) require.NotEmpty(t, field.ID) }) + + t.Run("should be able to delete a field with matching groupID", func(t *testing.T) { + groupID := model.NewId() + field := &model.PropertyField{ + GroupID: groupID, + Name: "Field with specific group", + Type: model.PropertyFieldTypeText, + } + _, err := ss.PropertyField().Create(field) + require.NoError(t, err) + require.NotZero(t, field.ID) + + err = ss.PropertyField().Delete(groupID, field.ID) + require.NoError(t, err) + + // Verify the field was soft-deleted + deletedField, err := ss.PropertyField().Get(groupID, field.ID) + require.NoError(t, err) + require.NotZero(t, deletedField.DeleteAt) + }) + + t.Run("should fail when deleting with non-matching groupID", func(t *testing.T) { + groupID := model.NewId() + field := &model.PropertyField{ + GroupID: groupID, + Name: "Another field with specific group", + Type: model.PropertyFieldTypeText, + } + _, err := ss.PropertyField().Create(field) + require.NoError(t, err) + require.NotZero(t, field.ID) + + // Try to delete with wrong groupID + err = ss.PropertyField().Delete(model.NewId(), field.ID) + require.Error(t, err) + var enf *store.ErrNotFound + require.ErrorAs(t, err, &enf) + + // Verify the field was not deleted + nonDeletedField, err := ss.PropertyField().Get(groupID, field.ID) + require.NoError(t, err) + require.Zero(t, nonDeletedField.DeleteAt) + }) } func testCountForGroup(t *testing.T, _ request.CTX, ss store.Store) { @@ -443,7 +563,7 @@ func testCountForGroup(t *testing.T, _ request.CTX, ss store.Store) { _, err := ss.PropertyField().Create(deletedField) require.NoError(t, err) - err = ss.PropertyField().Delete(deletedField.ID) + err = ss.PropertyField().Delete("", deletedField.ID) require.NoError(t, err) // Count should be 5 since the deleted field shouldn't be counted @@ -475,7 +595,7 @@ func testCountForGroup(t *testing.T, _ request.CTX, ss store.Store) { _, err := ss.PropertyField().Create(deletedField) require.NoError(t, err) - err = ss.PropertyField().Delete(deletedField.ID) + err = ss.PropertyField().Delete("", deletedField.ID) require.NoError(t, err) // Count should be 6 since we're including deleted fields @@ -527,7 +647,7 @@ func testSearchPropertyFields(t *testing.T, _ request.CTX, ss store.Store) { } // Delete one field for deletion tests - require.NoError(t, ss.PropertyField().Delete(field4.ID)) + require.NoError(t, ss.PropertyField().Delete("", field4.ID)) tests := []struct { name string diff --git a/server/channels/store/storetest/property_value_store.go b/server/channels/store/storetest/property_value_store.go index 7d458f639d..8c3fa64530 100644 --- a/server/channels/store/storetest/property_value_store.go +++ b/server/channels/store/storetest/property_value_store.go @@ -111,6 +111,43 @@ func testGetPropertyValue(t *testing.T, _ request.CTX, ss store.Store) { require.Zero(t, value) require.ErrorIs(t, err, sql.ErrNoRows) }) + + t.Run("should be able to retrieve an existing property value with matching groupID", func(t *testing.T) { + groupID := model.NewId() + newValue := &model.PropertyValue{ + TargetID: model.NewId(), + TargetType: "test_type", + GroupID: groupID, + FieldID: model.NewId(), + Value: json.RawMessage(`"test value with group"`), + } + _, err := ss.PropertyValue().Create(newValue) + require.NoError(t, err) + require.NotZero(t, newValue.ID) + + value, err := ss.PropertyValue().Get(groupID, newValue.ID) + require.NoError(t, err) + require.Equal(t, newValue.ID, value.ID) + require.Equal(t, newValue.Value, value.Value) + }) + + t.Run("should fail when retrieving a value with non-matching groupID", func(t *testing.T) { + newValue := &model.PropertyValue{ + TargetID: model.NewId(), + TargetType: "test_type", + GroupID: model.NewId(), + FieldID: model.NewId(), + Value: json.RawMessage(`"test value with specific group"`), + } + _, err := ss.PropertyValue().Create(newValue) + require.NoError(t, err) + require.NotZero(t, newValue.ID) + + // Try to get the value with a different group ID + value, err := ss.PropertyValue().Get(model.NewId(), newValue.ID) + require.Zero(t, value) + require.ErrorIs(t, err, sql.ErrNoRows) + }) } func testGetManyPropertyValues(t *testing.T, _ request.CTX, ss store.Store) { @@ -185,7 +222,7 @@ func testUpdatePropertyValue(t *testing.T, _ request.CTX, ss store.Store) { Value: json.RawMessage(`"test value"`), CreateAt: model.GetMillis(), } - updatedValue, err := ss.PropertyValue().Update([]*model.PropertyValue{value}) + updatedValue, err := ss.PropertyValue().Update("", []*model.PropertyValue{value}) require.Zero(t, updatedValue) require.ErrorContains(t, err, "failed to update, some property values were not found, got 0 of 1") }) @@ -203,13 +240,13 @@ func testUpdatePropertyValue(t *testing.T, _ request.CTX, ss store.Store) { require.NotZero(t, value.ID) value.TargetID = "" - updatedValue, err := ss.PropertyValue().Update([]*model.PropertyValue{value}) + updatedValue, err := ss.PropertyValue().Update("", []*model.PropertyValue{value}) require.Zero(t, updatedValue) require.ErrorContains(t, err, "model.property_value.is_valid.app_error") value.TargetID = model.NewId() value.GroupID = "" - updatedValue, err = ss.PropertyValue().Update([]*model.PropertyValue{value}) + updatedValue, err = ss.PropertyValue().Update("", []*model.PropertyValue{value}) require.Zero(t, updatedValue) require.ErrorContains(t, err, "model.property_value.is_valid.app_error") }) @@ -241,7 +278,7 @@ func testUpdatePropertyValue(t *testing.T, _ request.CTX, ss store.Store) { value1.Value = json.RawMessage(`"updated value 1"`) value2.Value = json.RawMessage(`"updated value 2"`) - _, err := ss.PropertyValue().Update([]*model.PropertyValue{value1, value2}) + _, err := ss.PropertyValue().Update("", []*model.PropertyValue{value1, value2}) require.NoError(t, err) // Verify first value @@ -288,7 +325,7 @@ func testUpdatePropertyValue(t *testing.T, _ request.CTX, ss store.Store) { value1.Value = json.RawMessage(`"Valid update"`) value2.GroupID = "Invalid ID" - _, err := ss.PropertyValue().Update([]*model.PropertyValue{value1, value2}) + _, err := ss.PropertyValue().Update("", []*model.PropertyValue{value1, value2}) require.Error(t, err) require.Contains(t, err.Error(), "model.property_value.is_valid.app_error") @@ -332,7 +369,7 @@ func testUpdatePropertyValue(t *testing.T, _ request.CTX, ss store.Store) { value1.Value = json.RawMessage(`"Updated Value 1"`) - _, err = ss.PropertyValue().Update([]*model.PropertyValue{value1, value2}) + _, err = ss.PropertyValue().Update("", []*model.PropertyValue{value1, value2}) require.Error(t, err) require.ErrorContains(t, err, "failed to update, some property values were not found") @@ -342,6 +379,91 @@ func testUpdatePropertyValue(t *testing.T, _ request.CTX, ss store.Store) { require.Equal(t, json.RawMessage(`"Value 1"`), updated1.Value) require.Equal(t, originalUpdateAt, updated1.UpdateAt) }) + + t.Run("should update values with matching groupID", func(t *testing.T) { + // Create values with the same groupID + groupID := model.NewId() + value1 := &model.PropertyValue{ + TargetID: model.NewId(), + TargetType: "test_type", + GroupID: groupID, + FieldID: model.NewId(), + Value: json.RawMessage(`"Group Value 1"`), + } + value2 := &model.PropertyValue{ + TargetID: model.NewId(), + TargetType: "test_type", + GroupID: groupID, + FieldID: model.NewId(), + Value: json.RawMessage(`"Group Value 2"`), + } + + for _, value := range []*model.PropertyValue{value1, value2} { + _, err := ss.PropertyValue().Create(value) + require.NoError(t, err) + } + + // Update the values with the matching groupID + value1.Value = json.RawMessage(`"Updated Group Value 1"`) + value2.Value = json.RawMessage(`"Updated Group Value 2"`) + + updatedValues, err := ss.PropertyValue().Update(groupID, []*model.PropertyValue{value1, value2}) + require.NoError(t, err) + require.Len(t, updatedValues, 2) + + // Verify the values were updated + for _, value := range []*model.PropertyValue{value1, value2} { + updated, err := ss.PropertyValue().Get("", value.ID) + require.NoError(t, err) + require.Contains(t, string(updated.Value), "Updated Group Value") + } + }) + + t.Run("should not update values with non-matching groupID", func(t *testing.T) { + // Create values with different groupIDs + groupID1 := model.NewId() + groupID2 := model.NewId() + + value1 := &model.PropertyValue{ + TargetID: model.NewId(), + TargetType: "test_type", + GroupID: groupID1, + FieldID: model.NewId(), + Value: json.RawMessage(`"Value in Group 1"`), + } + value2 := &model.PropertyValue{ + TargetID: model.NewId(), + TargetType: "test_type", + GroupID: groupID2, + FieldID: model.NewId(), + Value: json.RawMessage(`"Value in Group 2"`), + } + + for _, value := range []*model.PropertyValue{value1, value2} { + _, err := ss.PropertyValue().Create(value) + require.NoError(t, err) + } + + originalValue1 := string(value1.Value) + originalValue2 := string(value2.Value) + + // Try to update both values but filter by groupID1 + value1.Value = json.RawMessage(`"Updated Value in Group 1"`) + value2.Value = json.RawMessage(`"Updated Value in Group 2"`) + + _, err := ss.PropertyValue().Update(groupID1, []*model.PropertyValue{value1, value2}) + require.Error(t, err) + require.ErrorContains(t, err, "failed to update, some property values were not found") + + // Verify neither value was updated due to transaction rollback + updated1, err := ss.PropertyValue().Get("", value1.ID) + require.NoError(t, err) + require.Equal(t, originalValue1, string(updated1.Value)) + + updated2, err := ss.PropertyValue().Get("", value2.ID) + require.NoError(t, err) + require.Equal(t, originalValue2, string(updated2.Value)) + }) } func testUpsertPropertyValue(t *testing.T, _ request.CTX, ss store.Store) { @@ -509,7 +631,7 @@ func testUpsertPropertyValue(t *testing.T, _ request.CTX, ss store.Store) { func testDeletePropertyValue(t *testing.T, _ request.CTX, ss store.Store) { t.Run("should fail on nonexisting value", func(t *testing.T) { - err := ss.PropertyValue().Delete(model.NewId()) + err := ss.PropertyValue().Delete("", model.NewId()) var enf *store.ErrNotFound require.ErrorAs(t, err, &enf) }) @@ -526,7 +648,7 @@ func testDeletePropertyValue(t *testing.T, _ request.CTX, ss store.Store) { require.NoError(t, err) require.NotEmpty(t, value.ID) - err = ss.PropertyValue().Delete(value.ID) + err = ss.PropertyValue().Delete("", value.ID) require.NoError(t, err) // Verify the value was soft-deleted @@ -548,6 +670,54 @@ func testDeletePropertyValue(t *testing.T, _ request.CTX, ss store.Store) { require.NotEmpty(t, value.ID) require.Equal(t, sameDetailsValue.Value, value.Value) }) + + t.Run("should be able to delete a value with matching groupID", func(t *testing.T) { + groupID := model.NewId() + value := &model.PropertyValue{ + TargetID: model.NewId(), + TargetType: "test_type", + GroupID: groupID, + FieldID: model.NewId(), + Value: json.RawMessage(`"value with specific group"`), + } + _, err := ss.PropertyValue().Create(value) + require.NoError(t, err) + require.NotZero(t, value.ID) + + // Delete with matching groupID + err = ss.PropertyValue().Delete(groupID, value.ID) + require.NoError(t, err) + + // Verify the value was soft-deleted + deletedValue, err := ss.PropertyValue().Get(groupID, value.ID) + require.NoError(t, err) + require.NotZero(t, deletedValue.DeleteAt) + }) + + t.Run("should fail when deleting with non-matching groupID", func(t *testing.T) { + groupID := model.NewId() + value := &model.PropertyValue{ + TargetID: model.NewId(), + TargetType: "test_type", + GroupID: groupID, + FieldID: model.NewId(), + Value: json.RawMessage(`"another value with specific group"`), + } + _, err := ss.PropertyValue().Create(value) + require.NoError(t, err) + require.NotZero(t, value.ID) + + // Try to delete with wrong groupID + err = ss.PropertyValue().Delete(model.NewId(), value.ID) + require.Error(t, err) + var enf *store.ErrNotFound + require.ErrorAs(t, err, &enf) + + // Verify the value was not deleted + nonDeletedValue, err := ss.PropertyValue().Get(groupID, value.ID) + require.NoError(t, err) + require.Zero(t, nonDeletedValue.DeleteAt) + }) } func testSearchPropertyValues(t *testing.T, _ request.CTX, ss store.Store) { @@ -595,7 +765,7 @@ func testSearchPropertyValues(t *testing.T, _ request.CTX, ss store.Store) { } // Delete one value for deletion tests - require.NoError(t, ss.PropertyValue().Delete(value4.ID)) + require.NoError(t, ss.PropertyValue().Delete("", value4.ID)) tests := []struct { name string @@ -748,7 +918,7 @@ func testCreatePropertyValueWithArray(t *testing.T, _ request.CTX, ss store.Stor require.NotZero(t, created.ID) created.Value = json.RawMessage(`["updated1", "updated2", "updated3"]`) - updated, err := ss.PropertyValue().Update([]*model.PropertyValue{created}) + updated, err := ss.PropertyValue().Update("", []*model.PropertyValue{created}) require.NoError(t, err) require.NotZero(t, updated) diff --git a/server/channels/store/timerlayer/timerlayer.go b/server/channels/store/timerlayer/timerlayer.go index af46ab3b49..a8d45609a5 100644 --- a/server/channels/store/timerlayer/timerlayer.go +++ b/server/channels/store/timerlayer/timerlayer.go @@ -7153,10 +7153,10 @@ func (s *TimerLayerPropertyFieldStore) Create(field *model.PropertyField) (*mode return result, err } -func (s *TimerLayerPropertyFieldStore) Delete(id string) error { +func (s *TimerLayerPropertyFieldStore) Delete(groupID string, id string) error { start := time.Now() - err := s.PropertyFieldStore.Delete(id) + err := s.PropertyFieldStore.Delete(groupID, id) elapsed := float64(time.Since(start)) / float64(time.Second) if s.Root.Metrics != nil { @@ -7217,10 +7217,10 @@ func (s *TimerLayerPropertyFieldStore) SearchPropertyFields(opts model.PropertyF return result, err } -func (s *TimerLayerPropertyFieldStore) Update(fields []*model.PropertyField) ([]*model.PropertyField, error) { +func (s *TimerLayerPropertyFieldStore) Update(groupID string, fields []*model.PropertyField) ([]*model.PropertyField, error) { start := time.Now() - result, err := s.PropertyFieldStore.Update(fields) + result, err := s.PropertyFieldStore.Update(groupID, fields) elapsed := float64(time.Since(start)) / float64(time.Second) if s.Root.Metrics != nil { @@ -7281,10 +7281,10 @@ func (s *TimerLayerPropertyValueStore) Create(value *model.PropertyValue) (*mode return result, err } -func (s *TimerLayerPropertyValueStore) Delete(id string) error { +func (s *TimerLayerPropertyValueStore) Delete(groupID string, id string) error { start := time.Now() - err := s.PropertyValueStore.Delete(id) + err := s.PropertyValueStore.Delete(groupID, id) elapsed := float64(time.Since(start)) / float64(time.Second) if s.Root.Metrics != nil { @@ -7361,10 +7361,10 @@ func (s *TimerLayerPropertyValueStore) SearchPropertyValues(opts model.PropertyV return result, err } -func (s *TimerLayerPropertyValueStore) Update(values []*model.PropertyValue) ([]*model.PropertyValue, error) { +func (s *TimerLayerPropertyValueStore) Update(groupID string, values []*model.PropertyValue) ([]*model.PropertyValue, error) { start := time.Now() - result, err := s.PropertyValueStore.Update(values) + result, err := s.PropertyValueStore.Update(groupID, values) elapsed := float64(time.Since(start)) / float64(time.Second) if s.Root.Metrics != nil { diff --git a/server/i18n/en.json b/server/i18n/en.json index 2c4350c506..c6eb59669c 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -1873,6 +1873,10 @@ "id": "api.custom_profile_attributes.field_not_found", "translation": "trying to patch a field that does not exist" }, + { + "id": "api.custom_profile_attributes.invalid_field_patch", + "translation": "invalid custom profile attribute field patch" + }, { "id": "api.custom_profile_attributes.license_error", "translation": "Your license does not support Custom Profile Attributes." diff --git a/server/public/model/property_field.go b/server/public/model/property_field.go index 669ac275e6..67894c50cc 100644 --- a/server/public/model/property_field.go +++ b/server/public/model/property_field.go @@ -8,7 +8,6 @@ import ( "errors" "fmt" "net/http" - "strings" ) type PropertyFieldType string @@ -94,10 +93,6 @@ func (pf *PropertyField) IsValid() error { return nil } -func (pf *PropertyField) SanitizeInput() { - pf.Name = strings.TrimSpace(pf.Name) -} - type PropertyFieldPatch struct { Name *string `json:"name"` Type *PropertyFieldType `json:"type"` @@ -116,10 +111,22 @@ func (pfp *PropertyFieldPatch) Auditable() map[string]any { } } -func (pfp *PropertyFieldPatch) SanitizeInput() { - if pfp.Name != nil { - pfp.Name = NewPointer(strings.TrimSpace(*pfp.Name)) +func (pfp *PropertyFieldPatch) IsValid() error { + if pfp.Name != nil && *pfp.Name == "" { + return NewAppError("PropertyFieldPatch.IsValid", "model.property_field.is_valid.app_error", map[string]any{"FieldName": "name", "Reason": "value cannot be empty"}, "", http.StatusBadRequest) } + + if pfp.Type != nil && + *pfp.Type != PropertyFieldTypeText && + *pfp.Type != PropertyFieldTypeSelect && + *pfp.Type != PropertyFieldTypeMultiselect && + *pfp.Type != PropertyFieldTypeDate && + *pfp.Type != PropertyFieldTypeUser && + *pfp.Type != PropertyFieldTypeMultiuser { + return NewAppError("PropertyFieldPatch.IsValid", "model.property_field.is_valid.app_error", map[string]any{"FieldName": "type", "Reason": "unknown value"}, "", http.StatusBadRequest) + } + + return nil } func (pf *PropertyField) Patch(patch *PropertyFieldPatch) { diff --git a/server/public/model/property_field_test.go b/server/public/model/property_field_test.go index ff9c143831..7b6ecbd375 100644 --- a/server/public/model/property_field_test.go +++ b/server/public/model/property_field_test.go @@ -123,11 +123,38 @@ func TestPropertyField_IsValid(t *testing.T) { }) } -func TestPropertyField_SanitizeInput(t *testing.T) { - t.Run("trims spaces from name", func(t *testing.T) { - pf := &PropertyField{Name: " test field "} - pf.SanitizeInput() - assert.Equal(t, "test field", pf.Name) +func TestPropertyFieldPatch_IsValid(t *testing.T) { + t.Run("valid patch", func(t *testing.T) { + patch := &PropertyFieldPatch{ + Name: NewPointer("test field"), + Type: NewPointer(PropertyFieldTypeText), + } + require.NoError(t, patch.IsValid()) + }) + + t.Run("empty name", func(t *testing.T) { + patch := &PropertyFieldPatch{ + Name: NewPointer(""), + Type: NewPointer(PropertyFieldTypeText), + } + require.Error(t, patch.IsValid()) + }) + + t.Run("invalid type", func(t *testing.T) { + invalidType := PropertyFieldType("invalid") + patch := &PropertyFieldPatch{ + Name: NewPointer("test field"), + Type: &invalidType, + } + require.Error(t, patch.IsValid()) + }) + + t.Run("nil values are valid", func(t *testing.T) { + patch := &PropertyFieldPatch{ + Name: nil, + Type: nil, + } + require.NoError(t, patch.IsValid()) }) }