use CPAField

This commit is contained in:
Julien Tant
2025-02-25 11:53:54 -07:00
parent 988177024c
commit a4180d5d8f
6 changed files with 263 additions and 221 deletions
@@ -53,7 +53,7 @@ func createCPAField(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
var pf *model.PropertyField
var pf *model.CPAField
err := json.NewDecoder(r.Body).Decode(&pf)
if err != nil || pf == nil {
c.SetInvalidParamWithErr("property_field", err)
@@ -100,14 +100,15 @@ func TestListCPAFields(t *testing.T) {
th := Setup(t)
defer th.TearDown()
field := &model.PropertyField{
field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
Attrs: map[string]any{"visibility": "when_set"},
}
})
require.NoError(t, err)
createdField, err := th.App.CreateCPAField(field)
require.Nil(t, err)
require.NoError(t, err)
require.NotNil(t, createdField)
t.Run("endpoint should not work if no valid license is present", func(t *testing.T) {
@@ -158,10 +159,12 @@ func TestPatchCPAField(t *testing.T) {
th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterprise))
t.Run("a user without admin permissions should not be able to patch a field", func(t *testing.T) {
field := &model.PropertyField{
field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
}
})
require.NoError(t, err)
createdField, appErr := th.App.CreateCPAField(field)
require.Nil(t, appErr)
require.NotNil(t, createdField)
@@ -175,10 +178,12 @@ func TestPatchCPAField(t *testing.T) {
th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
webSocketClient := th.CreateConnectedWebSocketClient(t)
field := &model.PropertyField{
field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
}
})
require.NoError(t, err)
createdField, appErr := th.App.CreateCPAField(field)
require.Nil(t, appErr)
require.NotNil(t, createdField)
@@ -294,10 +299,12 @@ func TestListCPAValues(t *testing.T) {
th.RemovePermissionFromRole(model.PermissionViewMembers.Id, model.SystemUserRoleId)
defer th.AddPermissionToRole(model.PermissionViewMembers.Id, model.SystemUserRoleId)
field := &model.PropertyField{
field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
}
})
require.NoError(t, err)
createdField, appErr := th.App.CreateCPAField(field)
require.Nil(t, appErr)
require.NotNil(t, createdField)
@@ -328,10 +335,12 @@ func TestListCPAValues(t *testing.T) {
})
t.Run("should handle array values correctly", func(t *testing.T) {
arrayField := &model.PropertyField{
arrayField, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{
Name: model.NewId(),
Type: model.PropertyFieldTypeMultiselect,
}
})
require.NoError(t, err)
createdArrayField, appErr := th.App.CreateCPAField(arrayField)
require.Nil(t, appErr)
require.NotNil(t, createdArrayField)
@@ -518,10 +527,12 @@ func TestPatchCPAValues(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
field := &model.PropertyField{
field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
}
})
require.NoError(t, err)
createdField, appErr := th.App.CreateCPAField(field)
require.Nil(t, appErr)
require.NotNil(t, createdField)
@@ -609,10 +620,12 @@ func TestPatchCPAValues(t *testing.T) {
})
t.Run("should handle array values correctly", func(t *testing.T) {
arrayField := &model.PropertyField{
arrayField, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{
Name: model.NewId(),
Type: model.PropertyFieldTypeMultiselect,
}
})
require.NoError(t, err)
createdArrayField, appErr := th.App.CreateCPAField(arrayField)
require.Nil(t, appErr)
require.NotNil(t, createdArrayField)
@@ -7,7 +7,6 @@ import (
"encoding/json"
"net/http"
"sort"
"strings"
"github.com/mattermost/mattermost/server/public/model"
"github.com/mattermost/mattermost/server/v8/channels/store"
@@ -92,16 +91,12 @@ func (a *App) CreateCPAField(field *model.CPAField) (*model.PropertyField, *mode
return nil, model.NewAppError("CreateCPAField", "app.custom_profile_attributes.limit_reached.app_error", nil, "", http.StatusUnprocessableEntity).Wrap(err)
}
if appErr := field.Validate(); appErr != nil {
field.GroupID = groupID
if appErr := field.Sanitize(); appErr != nil {
return nil, appErr
}
field.GroupID = groupID
if appErr := field.Validate(); appErr != nil {
return nil, appErr
}
newField, err := a.Srv().propertyService.CreatePropertyField(field.ToPropertyField())
if err != nil {
var appErr *model.AppError
@@ -136,7 +131,7 @@ func (a *App) PatchCPAField(fieldID string, patch *model.PropertyFieldPatch) (*m
return nil, model.NewAppError("UpdateCPAField", "app.custom_profile_attributes.property_field_conversion.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
if appErr := cpaField.Validate(); appErr != nil {
if appErr := cpaField.Sanitize(); appErr != nil {
return nil, appErr
}
@@ -278,4 +273,3 @@ func (a *App) PatchCPAValues(userID string, fieldValueMap map[string]json.RawMes
return updatedValues, nil
}
@@ -47,12 +47,13 @@ func TestGetCPAField(t *testing.T) {
})
t.Run("should get an existing CPA field", func(t *testing.T) {
field := &model.PropertyField{
field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{
GroupID: cpaGroupID,
Name: "Test Field",
Type: model.PropertyFieldTypeText,
Attrs: model.StringInterface{model.CustomProfileAttributesPropertyAttrsVisibility: model.CustomProfileAttributesVisibilityHidden},
}
})
require.Nil(t, err)
createdField, err := th.App.CreateCPAField(field)
require.Nil(t, err)
@@ -120,7 +121,8 @@ func TestCreateCPAField(t *testing.T) {
require.NoError(t, cErr)
t.Run("should fail if the field is not valid", func(t *testing.T) {
field := &model.PropertyField{Name: model.NewId()}
field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{Name: model.NewId()})
require.Nil(t, err)
createdField, err := th.App.CreateCPAField(field)
require.NotNil(t, err)
@@ -128,11 +130,12 @@ func TestCreateCPAField(t *testing.T) {
})
t.Run("should not be able to create a property field for a different feature", func(t *testing.T) {
field := &model.PropertyField{
field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{
GroupID: model.NewId(),
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
}
})
require.NoError(t, err)
createdField, appErr := th.App.CreateCPAField(field)
require.Nil(t, appErr)
@@ -140,12 +143,13 @@ func TestCreateCPAField(t *testing.T) {
})
t.Run("should correctly create a CPA field", func(t *testing.T) {
field := &model.PropertyField{
field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{
GroupID: cpaGroupID,
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
Attrs: model.StringInterface{model.CustomProfileAttributesPropertyAttrsVisibility: model.CustomProfileAttributesVisibilityHidden},
}
})
require.NoError(t, err)
createdField, err := th.App.CreateCPAField(field)
require.Nil(t, err)
@@ -170,19 +174,23 @@ func TestCreateCPAField(t *testing.T) {
t.Run("should not be able to create CPA fields above the limit", func(t *testing.T) {
// we create the rest of the fields required to reach the limit
for i := 1; i <= CustomProfileAttributesFieldLimit; i++ {
field := &model.PropertyField{
field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
}
})
require.NoError(t, err)
createdField, err := th.App.CreateCPAField(field)
require.Nil(t, err)
require.NotZero(t, createdField.ID)
}
// then, we create a last one that would exceed the limit
field := &model.PropertyField{
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
field := &model.CPAField{
PropertyField: model.PropertyField{
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
},
}
createdField, err := th.App.CreateCPAField(field)
require.NotNil(t, err)
@@ -200,9 +208,11 @@ func TestCreateCPAField(t *testing.T) {
require.Nil(t, th.App.DeleteCPAField(fields[0].ID))
// creating a new one should work now
field := &model.PropertyField{
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
field := &model.CPAField{
PropertyField: model.PropertyField{
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
},
}
createdField, err := th.App.CreateCPAField(field)
require.Nil(t, err)
@@ -220,12 +230,14 @@ func TestPatchCPAField(t *testing.T) {
cpaGroupID, cErr := th.App.cpaGroupID()
require.NoError(t, cErr)
newField := &model.PropertyField{
newField, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{
GroupID: cpaGroupID,
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
Attrs: model.StringInterface{model.CustomProfileAttributesPropertyAttrsVisibility: model.CustomProfileAttributesVisibilityHidden},
}
})
require.NoError(t, err)
createdField, err := th.App.CreateCPAField(newField)
require.Nil(t, err)
@@ -248,6 +260,7 @@ func TestPatchCPAField(t *testing.T) {
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
}
field, err := th.App.Srv().propertyService.CreatePropertyField(newField)
require.NoError(t, err)
@@ -272,7 +285,7 @@ func TestPatchCPAField(t *testing.T) {
t.Run("should preserve option IDs when patching select field options", func(t *testing.T) {
// Create a select field with options
selectField := &model.PropertyField{
selectField, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{
GroupID: cpaGroupID,
Name: "Select Field",
Type: model.PropertyFieldTypeSelect,
@@ -288,7 +301,9 @@ func TestPatchCPAField(t *testing.T) {
},
},
},
}
})
require.NoError(t, err)
createdSelectField, err := th.App.CreateCPAField(selectField)
require.Nil(t, err)
@@ -351,11 +366,13 @@ func TestDeleteCPAField(t *testing.T) {
cpaGroupID, cErr := th.App.cpaGroupID()
require.NoError(t, cErr)
newField := &model.PropertyField{
newField, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{
GroupID: cpaGroupID,
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
}
})
require.NoError(t, err)
createdField, err := th.App.CreateCPAField(newField)
require.Nil(t, err)
@@ -643,176 +660,3 @@ func TestPatchCPAValue(t *testing.T) {
require.Equal(t, userID, updatedValue.TargetID)
})
}
func TestValidateCustomProfileAttributesField(t *testing.T) {
tests := []struct {
name string
field *model.PropertyField
expectError bool
errorId string
expectedAttrs model.StringInterface
}{
{
name: "valid text field with no value type",
field: &model.PropertyField{
Type: model.PropertyFieldTypeText,
Attrs: model.StringInterface{},
},
expectError: false,
expectedAttrs: model.StringInterface{
model.CustomProfileAttributesPropertyAttrsVisibility: "when_set",
},
},
{
name: "valid text field with valid value type and whitespace",
field: &model.PropertyField{
Type: model.PropertyFieldTypeText,
Attrs: model.StringInterface{
model.CustomProfileAttributesPropertyAttrsValueType: " email ",
},
},
expectError: false,
expectedAttrs: model.StringInterface{
model.CustomProfileAttributesPropertyAttrsVisibility: "when_set",
model.CustomProfileAttributesPropertyAttrsValueType: model.CustomProfileAttributesValueTypeEmail,
},
},
{
name: "valid text field with visibility and whitespace",
field: &model.PropertyField{
Type: model.PropertyFieldTypeText,
Attrs: model.StringInterface{
model.CustomProfileAttributesPropertyAttrsVisibility: " hidden ",
},
},
expectError: false,
expectedAttrs: model.StringInterface{
model.CustomProfileAttributesPropertyAttrsVisibility: model.CustomProfileAttributesVisibilityHidden,
},
},
{
name: "invalid text field with invalid value type",
field: &model.PropertyField{
Type: model.PropertyFieldTypeText,
Attrs: model.StringInterface{
model.CustomProfileAttributesPropertyAttrsValueType: "invalid_type",
},
},
expectError: true,
errorId: "app.custom_profile_attributes.unknown_value_type.app_error",
},
{
name: "valid select field with valid options",
field: &model.PropertyField{
Type: model.PropertyFieldTypeSelect,
Attrs: model.StringInterface{
model.PropertyFieldAttributeOptions: []any{
map[string]interface{}{
"name": "Option 1",
"color": "#123456",
},
map[string]interface{}{
"name": "Option 2",
"color": "#654321",
},
},
},
},
expectError: false,
expectedAttrs: model.StringInterface{
model.CustomProfileAttributesPropertyAttrsVisibility: model.CustomProfileAttributesVisibilityDefault,
model.PropertyFieldAttributeOptions: model.PropertyOptions[*model.CustomProfileAttributesSelectOption]{
{Name: "Option 1", Color: "#123456"},
{Name: "Option 2", Color: "#654321"},
},
},
},
{
name: "invalid select field with duplicate option names",
field: &model.PropertyField{
Type: model.PropertyFieldTypeSelect,
Attrs: model.StringInterface{
model.PropertyFieldAttributeOptions: []any{
map[string]interface{}{
"name": "Option 1",
"color": "opt1",
},
map[string]interface{}{
"name": "Option 1",
"color": "opt2",
},
},
},
},
expectError: true,
errorId: "app.custom_profile_attributes.invalid_options.app_error",
},
{
name: "invalid select field with non-array options",
field: &model.PropertyField{
Type: model.PropertyFieldTypeSelect,
Attrs: model.StringInterface{
model.PropertyFieldAttributeOptions: "not an array",
},
},
expectError: true,
errorId: "app.custom_profile_attributes.invalid_options.app_error",
},
{
name: "invalid select field with invalid option format",
field: &model.PropertyField{
Type: model.PropertyFieldTypeSelect,
Attrs: model.StringInterface{
model.PropertyFieldAttributeOptions: []interface{}{
"some string",
},
},
},
expectError: true,
errorId: "app.custom_profile_attributes.invalid_options.app_error",
},
{
name: "invalid field with unknown visibility",
field: &model.PropertyField{
Type: model.PropertyFieldTypeText,
Attrs: model.StringInterface{
model.CustomProfileAttributesPropertyAttrsVisibility: "unknown",
},
},
expectError: true,
errorId: "app.custom_profile_attributes.unknown_visibility.app_error",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := validateCustomProfileAttributesField(tt.field)
if tt.expectError {
require.NotNil(t, err)
require.Equal(t, tt.errorId, err.Id)
} else {
var ogErr error
if err != nil {
ogErr = err.Unwrap()
}
require.Nilf(t, err, "unexpected error: %v, with original error: %v", err, ogErr)
if tt.expectedAttrs != nil {
for key, value := range tt.expectedAttrs {
if key == model.PropertyFieldAttributeOptions {
expectedOptions := value.(model.PropertyOptions[*model.CustomProfileAttributesSelectOption])
actualOptions := tt.field.Attrs[model.PropertyFieldAttributeOptions].(model.PropertyOptions[*model.CustomProfileAttributesSelectOption])
// remove IDs from actualOptions to compare
for i := range actualOptions {
actualOptions[i].ID = ""
}
require.ElementsMatch(t, expectedOptions, actualOptions)
} else {
require.Equal(t, value, tt.field.Attrs[key])
}
}
}
}
})
}
}
@@ -128,7 +128,7 @@ func (c *CPAField) ToPropertyField() *PropertyField {
return &pf
}
func (c *CPAField) Validate() *AppError {
func (c *CPAField) Sanitize() *AppError {
switch c.Type {
case PropertyFieldTypeText:
if valueType := strings.TrimSpace(c.Attrs.ValueType); valueType != "" {
@@ -140,6 +140,14 @@ func (c *CPAField) Validate() *AppError {
case PropertyFieldTypeSelect, PropertyFieldTypeMultiselect:
options := c.Attrs.Options
// add an ID to options with no ID
for i := range options {
if options[i].ID == "" {
options[i].ID = NewId()
}
}
if err := options.IsValid(); err != nil {
return NewAppError("ValidateCPAField", "app.custom_profile_attributes.invalid_options.app_error", nil, "", http.StatusUnprocessableEntity).Wrap(err)
}
@@ -289,3 +289,186 @@ func TestCustomProfileAttributeSelectOptionIsValid(t *testing.T) {
})
}
}
func TestCPAField_Sanitize(t *testing.T) {
tests := []struct {
name string
field *CPAField
expectError bool
errorId string
expectedAttrs CPAAttrs
checkOptionsID bool
}{
{
name: "valid text field with no value type",
field: &CPAField{
PropertyField: PropertyField{
Type: PropertyFieldTypeText,
},
},
expectError: false,
expectedAttrs: CPAAttrs{
Visibility: "when_set",
},
},
{
name: "valid text field with valid value type and whitespace",
field: &CPAField{
PropertyField: PropertyField{
Type: PropertyFieldTypeText,
},
Attrs: CPAAttrs{
ValueType: " email ",
},
},
expectError: false,
expectedAttrs: CPAAttrs{
Visibility: "when_set",
ValueType: CustomProfileAttributesValueTypeEmail,
},
},
{
name: "valid text field with visibility and whitespace",
field: &CPAField{
PropertyField: PropertyField{
Type: PropertyFieldTypeText,
},
Attrs: CPAAttrs{
Visibility: " hidden ",
},
},
expectError: false,
expectedAttrs: CPAAttrs{
Visibility: CustomProfileAttributesVisibilityHidden,
},
},
{
name: "invalid text field with invalid value type",
field: &CPAField{
PropertyField: PropertyField{
Type: PropertyFieldTypeText,
},
Attrs: CPAAttrs{
ValueType: "invalid_type",
},
},
expectError: true,
errorId: "app.custom_profile_attributes.unknown_value_type.app_error",
},
{
name: "valid select field with valid options",
field: &CPAField{
PropertyField: PropertyField{
Type: PropertyFieldTypeSelect,
},
Attrs: CPAAttrs{
Options: []*CustomProfileAttributesSelectOption{
{
Name: "Option 1",
Color: "#123456",
},
{
Name: "Option 2",
Color: "#654321",
},
},
},
},
expectError: false,
expectedAttrs: CPAAttrs{
Visibility: CustomProfileAttributesVisibilityDefault,
Options: PropertyOptions[*CustomProfileAttributesSelectOption]{
{Name: "Option 1", Color: "#123456"},
{Name: "Option 2", Color: "#654321"},
},
},
},
{
name: "valid select field with valid options with ids",
field: &CPAField{
PropertyField: PropertyField{
Type: PropertyFieldTypeSelect,
},
Attrs: CPAAttrs{
Options: []*CustomProfileAttributesSelectOption{
{
ID: "t9ceh651eir4zkhyh4m54s5r7w",
Name: "Option 1",
Color: "#123456",
},
},
},
},
expectError: false,
expectedAttrs: CPAAttrs{
Visibility: CustomProfileAttributesVisibilityDefault,
Options: PropertyOptions[*CustomProfileAttributesSelectOption]{
{ID: "t9ceh651eir4zkhyh4m54s5r7w", Name: "Option 1", Color: "#123456"},
},
},
checkOptionsID: true,
},
{
name: "invalid select field with duplicate option names",
field: &CPAField{
PropertyField: PropertyField{
Type: PropertyFieldTypeSelect,
},
Attrs: CPAAttrs{
Options: []*CustomProfileAttributesSelectOption{
{
Name: "Option 1",
Color: "opt1",
},
{
Name: "Option 1",
Color: "opt2",
},
},
},
},
expectError: true,
errorId: "app.custom_profile_attributes.invalid_options.app_error",
},
{
name: "invalid field with unknown visibility",
field: &CPAField{
PropertyField: PropertyField{
Type: PropertyFieldTypeText,
},
Attrs: CPAAttrs{
Visibility: "unknown",
},
},
expectError: true,
errorId: "app.custom_profile_attributes.unknown_visibility.app_error",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := tt.field.Sanitize()
if tt.expectError {
require.NotNil(t, err)
require.Equal(t, tt.errorId, err.Id)
} else {
var ogErr error
if err != nil {
ogErr = err.Unwrap()
}
require.Nilf(t, err, "unexpected error: %v, with original error: %v", err, ogErr)
assert.Equal(t, tt.expectedAttrs.Visibility, tt.field.Attrs.Visibility)
assert.Equal(t, tt.expectedAttrs.ValueType, tt.field.Attrs.ValueType)
for i := range tt.expectedAttrs.Options {
if tt.checkOptionsID {
assert.Equal(t, tt.expectedAttrs.Options[i].ID, tt.field.Attrs.Options[i].ID)
}
assert.Equal(t, tt.expectedAttrs.Options[i].Name, tt.field.Attrs.Options[i].Name)
assert.Equal(t, tt.expectedAttrs.Options[i].Color, tt.field.Attrs.Options[i].Color)
}
}
})
}
}