diff --git a/server/cmd/mmctl/commands/cpa.go b/server/cmd/mmctl/commands/cpa.go index 7a3b135a7f4..9a0b0b6790c 100644 --- a/server/cmd/mmctl/commands/cpa.go +++ b/server/cmd/mmctl/commands/cpa.go @@ -4,11 +4,15 @@ package commands import ( + "context" "encoding/json" "fmt" "maps" + "strings" "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/v8/cmd/mmctl/client" + "github.com/mattermost/mattermost/server/v8/cmd/mmctl/printer" "github.com/spf13/cobra" ) @@ -83,3 +87,129 @@ func hasAttrsChanges(cmd *cobra.Command) bool { cmd.Flags().Changed("attrs") || cmd.Flags().Changed("option") } + +func getFieldFromArg(c client.Client, fieldArg string) (*model.PropertyField, error) { + fields, _, err := c.ListCPAFields(context.TODO()) + if err != nil { + return nil, fmt.Errorf("failed to get CPA fields: %w", err) + } + + if model.IsValidId(fieldArg) { + for _, field := range fields { + if field.ID == fieldArg { + return field, nil + } + } + } + + for _, field := range fields { + if field.Name == fieldArg { + return field, nil + } + } + + return nil, fmt.Errorf("failed to get field for %q", fieldArg) +} + +// setupCPATemplateContext sets up template functions for field and value resolution +func setupCPATemplateContext(c client.Client) error { + // Get all fields once for the entire command + fields, _, err := c.ListCPAFields(context.TODO()) + if err != nil { + return fmt.Errorf("failed to get CPA fields for template context: %w", err) + } + + fieldMap := make(map[string]*model.PropertyField) + for _, field := range fields { + fieldMap[field.ID] = field + } + + // Set template function to resolve field ID to field name + printer.SetTemplateFunc("fieldName", func(fieldID string) string { + if field, exists := fieldMap[fieldID]; exists { + return field.Name + } + return fieldID // fallback to field ID if not found + }) + + // Set template function to get field type + printer.SetTemplateFunc("fieldType", func(fieldID string) string { + if field, exists := fieldMap[fieldID]; exists { + return string(field.Type) + } + return "unknown" + }) + + // Set template function to resolve field value to human-readable format + printer.SetTemplateFunc("resolveValue", func(fieldID string, rawValue json.RawMessage) string { + field, exists := fieldMap[fieldID] + if !exists { + return string(rawValue) + } + + return resolveDisplayValue(field, rawValue) + }) + + return nil +} + +// resolveDisplayValue converts raw field values to human-readable display format +func resolveDisplayValue(field *model.PropertyField, rawValue json.RawMessage) string { + switch field.Type { + case model.PropertyFieldTypeSelect, model.PropertyFieldTypeMultiselect: + return resolveOptionDisplayValue(field, rawValue) + default: + var value any + if err := json.Unmarshal(rawValue, &value); err != nil { + return string(rawValue) + } + return fmt.Sprintf("%v", value) + } +} + +// resolveOptionDisplayValue converts option IDs to option names for select/multiselect fields +func resolveOptionDisplayValue(field *model.PropertyField, rawValue json.RawMessage) string { + // Convert PropertyField to CPAField to access options + cpaField, err := model.NewCPAFieldFromPropertyField(field) + if err != nil { + return string(rawValue) + } + + if len(cpaField.Attrs.Options) == 0 { + return string(rawValue) + } + + // Create option lookup map + optionMap := make(map[string]string) + for _, option := range cpaField.Attrs.Options { + optionMap[option.ID] = option.Name + } + + if field.Type == model.PropertyFieldTypeSelect { + // Single select - expect a string + var optionID string + if err := json.Unmarshal(rawValue, &optionID); err != nil { + return string(rawValue) + } + if optionName, exists := optionMap[optionID]; exists { + return optionName + } + return optionID + } + + // Multiselect - expect an array + var optionIDs []string + if err := json.Unmarshal(rawValue, &optionIDs); err != nil { + return string(rawValue) + } + + optionNames := make([]string, 0, len(optionIDs)) + for _, optionID := range optionIDs { + if optionName, exists := optionMap[optionID]; exists { + optionNames = append(optionNames, optionName) + } else { + optionNames = append(optionNames, optionID) + } + } + return fmt.Sprintf("[%s]", strings.Join(optionNames, ", ")) +} diff --git a/server/cmd/mmctl/commands/cpa_field.go b/server/cmd/mmctl/commands/cpa_field.go index 169fc5e2726..c2e636e2400 100644 --- a/server/cmd/mmctl/commands/cpa_field.go +++ b/server/cmd/mmctl/commands/cpa_field.go @@ -38,22 +38,23 @@ var CPAFieldCreateCmd = &cobra.Command{ } var CPAFieldEditCmd = &cobra.Command{ - Use: "edit [field-id]", + Use: "edit [field]", Short: "Edit a CPA field", - Long: "Edit an existing Custom Profile Attribute field.", + Long: "Edit an existing Custom Profile Attribute field by ID or name.", Example: ` cpa field edit n4qdbtro4j8x3n8z81p48ww9gr --name "Department Name" --managed - cpa field edit 8kj9xm4p6f3y7n2z9q5w8r1t4v --option Go --option React --option Python --option Java - cpa field edit 3h7k9m2x5b8v4n6p1q9w7r3t2y --managed=false`, + cpa field edit Department --option Go --option React --option Python --option Java + cpa field edit Skills --managed=false`, Args: cobra.ExactArgs(1), RunE: withClient(cpaFieldEditCmdF), } var CPAFieldDeleteCmd = &cobra.Command{ - Use: "delete [field-id]", + Use: "delete [field]", Short: "Delete a CPA field", - Long: "Delete a Custom Profile Attribute field. This will automatically delete all user values for this field.", + Long: "Delete a Custom Profile Attribute field by ID or name. This will automatically delete all user values for this field.", Example: ` cpa field delete n4qdbtro4j8x3n8z81p48ww9gr --confirm - cpa field delete 8kj9xm4p6f3y7n2z9q5w8r1t4v --confirm`, + cpa field delete Department --confirm + cpa field delete Skills --confirm`, Args: cobra.ExactArgs(1), RunE: withClient(cpaFieldDeleteCmdF), } @@ -199,7 +200,10 @@ func cpaFieldCreateCmdF(c client.Client, cmd *cobra.Command, args []string) erro } func cpaFieldEditCmdF(c client.Client, cmd *cobra.Command, args []string) error { - fieldID := args[0] + field, fErr := getFieldFromArg(c, args[0]) + if fErr != nil { + return fErr + } // Build patch object patch := &model.PropertyFieldPatch{} @@ -221,7 +225,7 @@ func cpaFieldEditCmdF(c client.Client, cmd *cobra.Command, args []string) error } // Update the field - updatedField, _, err := c.PatchCPAField(context.TODO(), fieldID, patch) + updatedField, _, err := c.PatchCPAField(context.TODO(), field.ID, patch) if err != nil { return fmt.Errorf("failed to update CPA field: %w", err) } @@ -247,8 +251,6 @@ func cpaFieldEditCmdF(c client.Client, cmd *cobra.Command, args []string) error } func cpaFieldDeleteCmdF(c client.Client, cmd *cobra.Command, args []string) error { - fieldID := args[0] - confirmFlag, _ := cmd.Flags().GetBool("confirm") if !confirmFlag { if err := getConfirmation("Are you sure you want to delete this CPA field?", true); err != nil { @@ -256,13 +258,18 @@ func cpaFieldDeleteCmdF(c client.Client, cmd *cobra.Command, args []string) erro } } + field, fErr := getFieldFromArg(c, args[0]) + if fErr != nil { + return fErr + } + // Delete the field - _, err := c.DeleteCPAField(context.TODO(), fieldID) + _, err := c.DeleteCPAField(context.TODO(), field.ID) if err != nil { return fmt.Errorf("failed to delete CPA field: %w", err) } printer.SetSingle(true) - printer.Print(fmt.Sprintf("Successfully deleted CPA field: %s", fieldID)) + printer.Print(fmt.Sprintf("Successfully deleted CPA field: %s", args[0])) return nil } diff --git a/server/cmd/mmctl/commands/cpa_field_e2e_test.go b/server/cmd/mmctl/commands/cpa_field_e2e_test.go index 30b3bb13eb1..06e304364b3 100644 --- a/server/cmd/mmctl/commands/cpa_field_e2e_test.go +++ b/server/cmd/mmctl/commands/cpa_field_e2e_test.go @@ -193,7 +193,7 @@ func (s *MmctlE2ETestSuite) TestCPAFieldEditCmd() { err = cpaFieldEditCmdF(c, cmd, []string{"nonexistent-field-id"}) s.Require().NotNil(err) - s.Require().Contains(err.Error(), "failed to update CPA field") + s.Require().Contains(err.Error(), "failed to get field for \"nonexistent-field-id\"") }) s.RunForSystemAdminAndLocal("Edit field using --name and --option flags", func(c client.Client) { @@ -303,6 +303,58 @@ func (s *MmctlE2ETestSuite) TestCPAFieldEditCmd() { // Verify that managed flag was set correctly s.Require().Equal("admin", cpaField.Attrs.Managed) }) + + s.RunForSystemAdminAndLocal("Edit field by name", func(c client.Client) { + printer.Clean() + s.cleanCPAFields() + + // First create a field to edit + field := &model.CPAField{ + PropertyField: model.PropertyField{ + Name: "Department", + Type: model.PropertyFieldTypeText, + TargetType: "user", + }, + Attrs: model.CPAAttrs{ + Managed: "", + }, + } + + createdField, appErr := s.th.App.CreateCPAField(field) + s.Require().Nil(appErr) + + // Now edit the field using its name instead of ID + cmd := &cobra.Command{} + cmd.Flags().String("name", "", "") + cmd.Flags().Bool("managed", false, "") + cmd.Flags().String("attrs", "", "") + cmd.Flags().StringSlice("option", []string{}, "") + + err := cmd.Flags().Set("name", "Team") + s.Require().Nil(err) + err = cmd.Flags().Set("managed", "true") + s.Require().Nil(err) + + // Edit using field name "Department" instead of the field ID + err = cpaFieldEditCmdF(c, cmd, []string{"Department"}) + s.Require().Nil(err) + s.Require().Len(printer.GetLines(), 1) + s.Require().Len(printer.GetErrorLines(), 0) + + // Verify the success message + output := printer.GetLines()[0].(string) + s.Require().Contains(output, "Field Team successfully updated") + + // Verify field was actually updated by retrieving it + updatedField, appErr := s.th.App.GetCPAField(createdField.ID) + s.Require().Nil(appErr) + s.Require().Equal("Team", updatedField.Name) + + // Convert to CPAField to check managed status + cpaField, err := model.NewCPAFieldFromPropertyField(updatedField) + s.Require().Nil(err) + s.Require().Equal("admin", cpaField.Attrs.Managed) + }) } func (s *MmctlE2ETestSuite) TestCPAFieldDeleteCmd() { @@ -355,6 +407,53 @@ func (s *MmctlE2ETestSuite) TestCPAFieldDeleteCmd() { s.Require().False(fieldExists, "Field should have been deleted but still exists in the list") }) + s.RunForSystemAdminAndLocal("Delete existing field by name", func(c client.Client) { + printer.Clean() + s.cleanCPAFields() + + // First create a field to delete + field := &model.CPAField{ + PropertyField: model.PropertyField{ + Name: "Department", + Type: model.PropertyFieldTypeText, + TargetType: "user", + }, + } + + createdField, appErr := s.th.App.CreateCPAField(field) + s.Require().Nil(appErr) + + cmd := &cobra.Command{} + cmd.Flags().Bool("confirm", false, "") + + err := cmd.Flags().Set("confirm", "true") + s.Require().Nil(err) + + // Delete using field name instead of ID + err = cpaFieldDeleteCmdF(c, cmd, []string{"Department"}) + s.Require().Nil(err) + s.Require().Len(printer.GetLines(), 1) + s.Require().Len(printer.GetErrorLines(), 0) + + // Verify the success message + output := printer.GetLines()[0].(string) + s.Require().Contains(output, "Successfully deleted CPA field: Department") + + // Verify field was actually deleted by checking if it exists in the list + fields, appErr := s.th.App.ListCPAFields() + s.Require().Nil(appErr) + + // Field should not be in the list anymore + fieldExists := false + for _, field := range fields { + if field.ID == createdField.ID { + fieldExists = true + break + } + } + s.Require().False(fieldExists, "Field should have been deleted but still exists in the list") + }) + s.RunForSystemAdminAndLocal("Delete nonexistent field", func(c client.Client) { printer.Clean() s.cleanCPAFields() @@ -367,6 +466,21 @@ func (s *MmctlE2ETestSuite) TestCPAFieldDeleteCmd() { err = cpaFieldDeleteCmdF(c, cmd, []string{"nonexistent-field-id"}) s.Require().NotNil(err) - s.Require().Contains(err.Error(), "failed to delete CPA field") + s.Require().Contains(err.Error(), `failed to get field for "nonexistent-field-id"`) + }) + + s.RunForSystemAdminAndLocal("Delete nonexistent field by name", func(c client.Client) { + printer.Clean() + s.cleanCPAFields() + + cmd := &cobra.Command{} + cmd.Flags().Bool("confirm", false, "") + + err := cmd.Flags().Set("confirm", "true") + s.Require().Nil(err) + + err = cpaFieldDeleteCmdF(c, cmd, []string{"NonexistentField"}) + s.Require().NotNil(err) + s.Require().Contains(err.Error(), `failed to get field for "NonexistentField"`) }) } diff --git a/server/cmd/mmctl/commands/cpa_field_test.go b/server/cmd/mmctl/commands/cpa_field_test.go index 3441ed50d2c..a126e9d3c03 100644 --- a/server/cmd/mmctl/commands/cpa_field_test.go +++ b/server/cmd/mmctl/commands/cpa_field_test.go @@ -437,8 +437,17 @@ func (s *MmctlUnitTestSuite) TestCPAFieldEditCmd() { printer.SetFormat(printer.FormatPlain) viper.Set("json", false) + fieldID := model.NewId() + mockFields := []*model.PropertyField{ + { + ID: fieldID, + Name: "Department", + Type: model.PropertyFieldTypeText, + }, + } + expectedField := &model.PropertyField{ - ID: "field-id", + ID: fieldID, Name: "New Department", Type: model.PropertyFieldTypeText, TargetType: "user", @@ -448,7 +457,13 @@ func (s *MmctlUnitTestSuite) TestCPAFieldEditCmd() { newName := "New Department" s.client. EXPECT(). - PatchCPAField(context.TODO(), "field-id", &model.PropertyFieldPatch{ + ListCPAFields(context.TODO()). + Return(mockFields, &model.Response{}, nil). + Times(1) + + s.client. + EXPECT(). + PatchCPAField(context.TODO(), fieldID, &model.PropertyFieldPatch{ Name: &newName, }). Return(expectedField, &model.Response{}, nil). @@ -457,7 +472,7 @@ func (s *MmctlUnitTestSuite) TestCPAFieldEditCmd() { cmd := &cobra.Command{} cmd.Flags().String("name", "", "") _ = cmd.Flags().Set("name", "New Department") - err := cpaFieldEditCmdF(s.client, cmd, []string{"field-id"}) + err := cpaFieldEditCmdF(s.client, cmd, []string{fieldID}) s.Require().NoError(err) lines := printer.GetLines() @@ -470,8 +485,9 @@ func (s *MmctlUnitTestSuite) TestCPAFieldEditCmd() { printer.SetFormat(printer.FormatPlain) viper.Set("json", false) + fieldID := model.NewId() expectedField := &model.PropertyField{ - ID: "field-id", + ID: fieldID, Name: "Department", Type: model.PropertyFieldTypeText, TargetType: "user", @@ -483,9 +499,17 @@ func (s *MmctlUnitTestSuite) TestCPAFieldEditCmd() { expectedAttrs := model.StringInterface{ "managed": "admin", } + + mockFields := []*model.PropertyField{expectedField} s.client. EXPECT(). - PatchCPAField(context.TODO(), "field-id", &model.PropertyFieldPatch{ + ListCPAFields(context.TODO()). + Return(mockFields, &model.Response{}, nil). + Times(1) + + s.client. + EXPECT(). + PatchCPAField(context.TODO(), fieldID, &model.PropertyFieldPatch{ Attrs: &expectedAttrs, }). Return(expectedField, &model.Response{}, nil). @@ -496,7 +520,7 @@ func (s *MmctlUnitTestSuite) TestCPAFieldEditCmd() { cmd.Flags().String("attrs", "", "") cmd.Flags().StringSlice("option", []string{}, "") _ = cmd.Flags().Set("managed", "true") - err := cpaFieldEditCmdF(s.client, cmd, []string{"field-id"}) + err := cpaFieldEditCmdF(s.client, cmd, []string{fieldID}) s.Require().NoError(err) lines := printer.GetLines() @@ -509,8 +533,9 @@ func (s *MmctlUnitTestSuite) TestCPAFieldEditCmd() { printer.SetFormat(printer.FormatPlain) viper.Set("json", false) + fieldID := model.NewId() expectedField := &model.PropertyField{ - ID: "field-id", + ID: fieldID, Name: "Department", Type: model.PropertyFieldTypeText, TargetType: "user", @@ -522,9 +547,17 @@ func (s *MmctlUnitTestSuite) TestCPAFieldEditCmd() { expectedAttrs := model.StringInterface{ "managed": "", } + + mockFields := []*model.PropertyField{expectedField} s.client. EXPECT(). - PatchCPAField(context.TODO(), "field-id", &model.PropertyFieldPatch{ + ListCPAFields(context.TODO()). + Return(mockFields, &model.Response{}, nil). + Times(1) + + s.client. + EXPECT(). + PatchCPAField(context.TODO(), fieldID, &model.PropertyFieldPatch{ Attrs: &expectedAttrs, }). Return(expectedField, &model.Response{}, nil). @@ -535,7 +568,7 @@ func (s *MmctlUnitTestSuite) TestCPAFieldEditCmd() { cmd.Flags().String("attrs", "", "") cmd.Flags().StringSlice("option", []string{}, "") _ = cmd.Flags().Set("managed", "false") - err := cpaFieldEditCmdF(s.client, cmd, []string{"field-id"}) + err := cpaFieldEditCmdF(s.client, cmd, []string{fieldID}) s.Require().NoError(err) lines := printer.GetLines() @@ -548,8 +581,9 @@ func (s *MmctlUnitTestSuite) TestCPAFieldEditCmd() { printer.SetFormat(printer.FormatPlain) viper.Set("json", false) + fieldID := model.NewId() expectedField := &model.PropertyField{ - ID: "field-id", + ID: fieldID, Name: "Department", Type: model.PropertyFieldTypeText, TargetType: "user", @@ -563,9 +597,17 @@ func (s *MmctlUnitTestSuite) TestCPAFieldEditCmd() { "visibility": "always", "required": true, } + + mockFields := []*model.PropertyField{expectedField} s.client. EXPECT(). - PatchCPAField(context.TODO(), "field-id", &model.PropertyFieldPatch{ + ListCPAFields(context.TODO()). + Return(mockFields, &model.Response{}, nil). + Times(1) + + s.client. + EXPECT(). + PatchCPAField(context.TODO(), fieldID, &model.PropertyFieldPatch{ Attrs: &expectedAttrs, }). Return(expectedField, &model.Response{}, nil). @@ -576,7 +618,7 @@ func (s *MmctlUnitTestSuite) TestCPAFieldEditCmd() { cmd.Flags().String("attrs", "", "") cmd.Flags().StringSlice("option", []string{}, "") _ = cmd.Flags().Set("attrs", `{"visibility":"always","required":true}`) - err := cpaFieldEditCmdF(s.client, cmd, []string{"field-id"}) + err := cpaFieldEditCmdF(s.client, cmd, []string{fieldID}) s.Require().NoError(err) lines := printer.GetLines() @@ -589,8 +631,9 @@ func (s *MmctlUnitTestSuite) TestCPAFieldEditCmd() { printer.SetFormat(printer.FormatPlain) viper.Set("json", false) + fieldID := model.NewId() expectedField := &model.PropertyField{ - ID: "field-id", + ID: fieldID, Name: "Skills", Type: model.PropertyFieldTypeMultiselect, TargetType: "user", @@ -603,11 +646,18 @@ func (s *MmctlUnitTestSuite) TestCPAFieldEditCmd() { }, } + mockFields := []*model.PropertyField{expectedField} s.client. EXPECT(). - PatchCPAField(context.TODO(), "field-id", gomock.Any()). - DoAndReturn(func(ctx context.Context, fieldID string, patch *model.PropertyFieldPatch) (*model.PropertyField, *model.Response, error) { - s.Require().Equal("field-id", fieldID) + ListCPAFields(context.TODO()). + Return(mockFields, &model.Response{}, nil). + Times(1) + + s.client. + EXPECT(). + PatchCPAField(context.TODO(), fieldID, gomock.Any()). + DoAndReturn(func(ctx context.Context, receivedFieldID string, patch *model.PropertyFieldPatch) (*model.PropertyField, *model.Response, error) { + s.Require().Equal(fieldID, receivedFieldID) s.Require().NotNil(patch.Attrs) options, ok := (*patch.Attrs)["options"].([]*model.CustomProfileAttributesSelectOption) @@ -631,7 +681,7 @@ func (s *MmctlUnitTestSuite) TestCPAFieldEditCmd() { _ = cmd.Flags().Set("option", "Go") _ = cmd.Flags().Set("option", "React") _ = cmd.Flags().Set("option", "Python") - err := cpaFieldEditCmdF(s.client, cmd, []string{"field-id"}) + err := cpaFieldEditCmdF(s.client, cmd, []string{fieldID}) s.Require().NoError(err) lines := printer.GetLines() @@ -644,8 +694,9 @@ func (s *MmctlUnitTestSuite) TestCPAFieldEditCmd() { printer.SetFormat(printer.FormatPlain) viper.Set("json", false) + fieldID := model.NewId() expectedField := &model.PropertyField{ - ID: "field-id", + ID: fieldID, Name: "Department", Type: model.PropertyFieldTypeText, TargetType: "user", @@ -655,11 +706,18 @@ func (s *MmctlUnitTestSuite) TestCPAFieldEditCmd() { }, } + mockFields := []*model.PropertyField{expectedField} s.client. EXPECT(). - PatchCPAField(context.TODO(), "field-id", gomock.Any()). - DoAndReturn(func(ctx context.Context, fieldID string, patch *model.PropertyFieldPatch) (*model.PropertyField, *model.Response, error) { - s.Require().Equal("field-id", fieldID) + ListCPAFields(context.TODO()). + Return(mockFields, &model.Response{}, nil). + Times(1) + + s.client. + EXPECT(). + PatchCPAField(context.TODO(), fieldID, gomock.Any()). + DoAndReturn(func(ctx context.Context, receivedFieldID string, patch *model.PropertyFieldPatch) (*model.PropertyField, *model.Response, error) { + s.Require().Equal(fieldID, receivedFieldID) s.Require().NotNil(patch.Attrs) // individual flags should take precedence over attrs @@ -676,7 +734,7 @@ func (s *MmctlUnitTestSuite) TestCPAFieldEditCmd() { cmd.Flags().StringSlice("option", []string{}, "") _ = cmd.Flags().Set("managed", "true") _ = cmd.Flags().Set("attrs", `{"visibility":"always","managed":""}`) - err := cpaFieldEditCmdF(s.client, cmd, []string{"field-id"}) + err := cpaFieldEditCmdF(s.client, cmd, []string{fieldID}) s.Require().NoError(err) lines := printer.GetLines() @@ -690,18 +748,26 @@ func (s *MmctlUnitTestSuite) TestCPAFieldEditCmd() { viper.Set("json", false) newName := "New Name" + fieldID := model.NewId() expectedField := &model.PropertyField{ - ID: "field-id", + ID: fieldID, Name: "New Name", Type: model.PropertyFieldTypeText, TargetType: "user", Attrs: make(model.StringInterface), } + mockFields := []*model.PropertyField{expectedField} + s.client. + EXPECT(). + ListCPAFields(context.TODO()). + Return(mockFields, &model.Response{}, nil). + Times(1) + // Should only pass name, no attrs s.client. EXPECT(). - PatchCPAField(context.TODO(), "field-id", &model.PropertyFieldPatch{ + PatchCPAField(context.TODO(), fieldID, &model.PropertyFieldPatch{ Name: &newName, }). Return(expectedField, &model.Response{}, nil). @@ -710,7 +776,7 @@ func (s *MmctlUnitTestSuite) TestCPAFieldEditCmd() { cmd := &cobra.Command{} cmd.Flags().String("name", "", "") _ = cmd.Flags().Set("name", "New Name") - err := cpaFieldEditCmdF(s.client, cmd, []string{"field-id"}) + err := cpaFieldEditCmdF(s.client, cmd, []string{fieldID}) s.Require().NoError(err) lines := printer.GetLines() @@ -721,12 +787,25 @@ func (s *MmctlUnitTestSuite) TestCPAFieldEditCmd() { s.Run("Should handle error for invalid attrs JSON syntax", func() { printer.Clean() + fieldID := model.NewId() + mockField := &model.PropertyField{ + ID: fieldID, + Name: "Department", + Type: model.PropertyFieldTypeText, + } + mockFields := []*model.PropertyField{mockField} + s.client. + EXPECT(). + ListCPAFields(context.TODO()). + Return(mockFields, &model.Response{}, nil). + Times(1) + cmd := &cobra.Command{} cmd.Flags().Bool("managed", false, "") cmd.Flags().String("attrs", "", "") cmd.Flags().StringSlice("option", []string{}, "") _ = cmd.Flags().Set("attrs", `{"invalid": json}`) // Invalid JSON - err := cpaFieldEditCmdF(s.client, cmd, []string{"field-id"}) + err := cpaFieldEditCmdF(s.client, cmd, []string{fieldID}) s.Require().Error(err) s.Require().Contains(err.Error(), "failed to parse attrs JSON") }) @@ -734,42 +813,209 @@ func (s *MmctlUnitTestSuite) TestCPAFieldEditCmd() { s.Run("Should handle API error when PatchCPAField client call fails", func() { printer.Clean() + fieldID := model.NewId() + mockField := &model.PropertyField{ + ID: fieldID, + Name: "Department", + Type: model.PropertyFieldTypeText, + } + mockFields := []*model.PropertyField{mockField} + s.client. + EXPECT(). + ListCPAFields(context.TODO()). + Return(mockFields, &model.Response{}, nil). + Times(1) + expectedError := errors.New("API error") s.client. EXPECT(). - PatchCPAField(context.TODO(), "field-id", gomock.Any()). + PatchCPAField(context.TODO(), fieldID, gomock.Any()). Return(nil, &model.Response{}, expectedError). Times(1) cmd := &cobra.Command{} cmd.Flags().String("name", "", "") _ = cmd.Flags().Set("name", "New Name") - err := cpaFieldEditCmdF(s.client, cmd, []string{"field-id"}) + err := cpaFieldEditCmdF(s.client, cmd, []string{fieldID}) s.Require().Error(err) s.Require().Contains(err.Error(), "failed to update CPA field") s.Require().Contains(err.Error(), "API error") }) + + s.Run("Should successfully edit field by name", func() { + printer.Clean() + printer.SetFormat(printer.FormatPlain) + viper.Set("json", false) + + fieldID := model.NewId() + mockFields := []*model.PropertyField{ + { + ID: fieldID, + Name: "Department", + Type: model.PropertyFieldTypeText, + }, + } + + expectedField := &model.PropertyField{ + ID: fieldID, + Name: "Team", + Type: model.PropertyFieldTypeText, + TargetType: "user", + Attrs: model.StringInterface{ + "managed": "admin", + }, + } + + newName := "Team" + expectedAttrs := model.StringInterface{ + "managed": "admin", + } + + s.client. + EXPECT(). + ListCPAFields(context.TODO()). + Return(mockFields, &model.Response{}, nil). + Times(1) + + s.client. + EXPECT(). + PatchCPAField(context.TODO(), fieldID, &model.PropertyFieldPatch{ + Name: &newName, + Attrs: &expectedAttrs, + }). + Return(expectedField, &model.Response{}, nil). + Times(1) + + cmd := &cobra.Command{} + cmd.Flags().String("name", "", "") + cmd.Flags().Bool("managed", false, "") + cmd.Flags().String("attrs", "", "") + cmd.Flags().StringSlice("option", []string{}, "") + _ = cmd.Flags().Set("name", "Team") + _ = cmd.Flags().Set("managed", "true") + err := cpaFieldEditCmdF(s.client, cmd, []string{"Department"}) + s.Require().NoError(err) + + lines := printer.GetLines() + s.Require().Len(lines, 1) + s.Require().Contains(lines[0], "Field Team successfully updated") + }) } func (s *MmctlUnitTestSuite) TestCPAFieldDeleteCmd() { s.Run("Should successfully delete field with --confirm flag", func() { printer.Clean() + fieldID := model.NewId() + mockFields := []*model.PropertyField{ + { + ID: fieldID, + Name: "Department", + Type: model.PropertyFieldTypeText, + }, + } + s.client. EXPECT(). - DeleteCPAField(context.TODO(), "field-id"). + ListCPAFields(context.TODO()). + Return(mockFields, &model.Response{}, nil). + Times(1) + + s.client. + EXPECT(). + DeleteCPAField(context.TODO(), fieldID). Return(&model.Response{}, nil). Times(1) cmd := &cobra.Command{} cmd.Flags().Bool("confirm", false, "") _ = cmd.Flags().Set("confirm", "true") - err := cpaFieldDeleteCmdF(s.client, cmd, []string{"field-id"}) + err := cpaFieldDeleteCmdF(s.client, cmd, []string{fieldID}) s.Require().NoError(err) lines := printer.GetLines() s.Require().Len(lines, 1) - s.Require().Contains(lines[0], "Successfully deleted CPA field: field-id") + s.Require().Contains(lines[0], "Successfully deleted CPA field: "+fieldID) + }) + + s.Run("Should successfully delete field by name with --confirm flag", func() { + printer.Clean() + + fieldID := model.NewId() + mockFields := []*model.PropertyField{ + { + ID: fieldID, + Name: "Department", + Type: model.PropertyFieldTypeText, + }, + } + + s.client. + EXPECT(). + ListCPAFields(context.TODO()). + Return(mockFields, &model.Response{}, nil). + Times(1) + + s.client. + EXPECT(). + DeleteCPAField(context.TODO(), fieldID). + Return(&model.Response{}, nil). + Times(1) + + cmd := &cobra.Command{} + cmd.Flags().Bool("confirm", false, "") + _ = cmd.Flags().Set("confirm", "true") + err := cpaFieldDeleteCmdF(s.client, cmd, []string{"Department"}) + s.Require().NoError(err) + + lines := printer.GetLines() + s.Require().Len(lines, 1) + s.Require().Contains(lines[0], "Successfully deleted CPA field: Department") + }) + + s.Run("Should handle getFieldFromArg error when field not found", func() { + printer.Clean() + + fieldID := model.NewId() + mockFields := []*model.PropertyField{ + { + ID: fieldID, + Name: "Department", + Type: model.PropertyFieldTypeText, + }, + } + + s.client. + EXPECT(). + ListCPAFields(context.TODO()). + Return(mockFields, &model.Response{}, nil). + Times(1) + + cmd := &cobra.Command{} + cmd.Flags().Bool("confirm", false, "") + _ = cmd.Flags().Set("confirm", "true") + err := cpaFieldDeleteCmdF(s.client, cmd, []string{"NonexistentField"}) + s.Require().Error(err) + s.Require().Contains(err.Error(), `failed to get field for "NonexistentField"`) + }) + + s.Run("Should handle ListCPAFields API error in getFieldFromArg", func() { + printer.Clean() + + expectedError := errors.New("API error") + s.client. + EXPECT(). + ListCPAFields(context.TODO()). + Return(nil, &model.Response{}, expectedError). + Times(1) + + cmd := &cobra.Command{} + cmd.Flags().Bool("confirm", false, "") + _ = cmd.Flags().Set("confirm", "true") + err := cpaFieldDeleteCmdF(s.client, cmd, []string{"field-name"}) + s.Require().Error(err) + s.Require().Contains(err.Error(), "failed to get CPA fields") + s.Require().Contains(err.Error(), "API error") }) s.Run("Should error when --confirm flag is not provided in non-interactive shell", func() { @@ -786,17 +1032,32 @@ func (s *MmctlUnitTestSuite) TestCPAFieldDeleteCmd() { s.Run("Should handle API error when DeleteCPAField client call fails", func() { printer.Clean() + fieldID := model.NewId() + mockFields := []*model.PropertyField{ + { + ID: fieldID, + Name: "Department", + Type: model.PropertyFieldTypeText, + }, + } + + s.client. + EXPECT(). + ListCPAFields(context.TODO()). + Return(mockFields, &model.Response{}, nil). + Times(1) + expectedError := errors.New("API error") s.client. EXPECT(). - DeleteCPAField(context.TODO(), "field-id"). + DeleteCPAField(context.TODO(), fieldID). Return(&model.Response{}, expectedError). Times(1) cmd := &cobra.Command{} cmd.Flags().Bool("confirm", false, "") _ = cmd.Flags().Set("confirm", "true") - err := cpaFieldDeleteCmdF(s.client, cmd, []string{"field-id"}) + err := cpaFieldDeleteCmdF(s.client, cmd, []string{fieldID}) s.Require().Error(err) s.Require().Contains(err.Error(), "failed to delete CPA field") s.Require().Contains(err.Error(), "API error") diff --git a/server/cmd/mmctl/commands/cpa_value.go b/server/cmd/mmctl/commands/cpa_value.go index b3cd45a3c25..6ff54860604 100644 --- a/server/cmd/mmctl/commands/cpa_value.go +++ b/server/cmd/mmctl/commands/cpa_value.go @@ -27,12 +27,12 @@ var CPAValueListCmd = &cobra.Command{ } var CPAValueSetCmd = &cobra.Command{ - Use: "set [user] [field-id]", + Use: "set [user] [field]", Short: "Set a CPA value for a user", - Long: "Set a Custom Profile Attribute field value for a specific user.", - Example: ` cpa value set john.doe@company.com kx8m2w4r9p3q7n5t1j6h8s4c9e --value "Engineering" - cpa value set johndoe q7n3t8w5r2m9k4x6p1j3h7s8c4 --value "Go" --value "React" --value "Python" - cpa value set user123 w9r5t2n8k4x7p3q6m1j9h4s7c2 --value "Senior"`, + Long: "Set a Custom Profile Attribute field value for a specific user by field ID or name.", + Example: ` cpa value set john.doe@company.com kx8m2w4r9p3q7n5t1j6h8s4c9e --value Engineering + cpa value set johndoe Department --value Engineering + cpa value set user123 Skills --value Go --value React --value Python`, Args: cobra.ExactArgs(2), RunE: withClient(cpaValueSetCmdF), } @@ -52,6 +52,11 @@ func init() { func cpaValueListCmdF(c client.Client, cmd *cobra.Command, args []string) error { userArg := args[0] + // Setup template context for field and value resolution + if tErr := setupCPATemplateContext(c); tErr != nil { + return tErr + } + // Resolve user user, err := getUserFromArg(c, userArg) if err != nil { @@ -68,14 +73,14 @@ func cpaValueListCmdF(c client.Client, cmd *cobra.Command, args []string) error keypair := map[string]any{ fieldID: value, } - printer.PrintT("{{range $k, $v := .}}FieldID: {{$k}}, Value: {{printf \"%s\" $v}}{{end}}", keypair) + printer.PrintT("{{range $k, $v := .}}{{fieldName $k}} ({{fieldType $k}}): {{resolveValue $k $v}}{{end}}", keypair) } return nil } func cpaValueSetCmdF(c client.Client, cmd *cobra.Command, args []string) error { userArg := args[0] - fieldID := args[1] + fieldArg := args[1] // Get values from flag values, err := cmd.Flags().GetStringSlice("value") @@ -89,38 +94,31 @@ func cpaValueSetCmdF(c client.Client, cmd *cobra.Command, args []string) error { return err } - // Get field info to validate - fields, _, err := c.ListCPAFields(context.TODO()) + // Resolve field + field, err := getFieldFromArg(c, fieldArg) if err != nil { - return fmt.Errorf("failed to get CPA fields: %w", err) + return err } - var targetField *model.PropertyField - for _, field := range fields { - if field.ID == fieldID { - targetField = field - break - } - } - - if targetField == nil { - return fmt.Errorf("field %s not found", fieldID) + // Setup template context for field and value resolution + if tErr := setupCPATemplateContext(c); tErr != nil { + return tErr } // Resolve option names to IDs for select/multiselect fields - resolvedValues, err := resolveOptionNamesToIDs(targetField, values) + resolvedValues, err := resolveOptionNamesToIDs(field, values) if err != nil { return fmt.Errorf("failed to resolve option values: %w", err) } // Prepare the value for marshaling var valueToMarshal any - if len(resolvedValues) == 1 { - // Single value - valueToMarshal = resolvedValues[0] - } else { + if field.Type == model.PropertyFieldTypeMultiselect || field.Type == model.PropertyFieldTypeMultiuser { // Multiple values valueToMarshal = resolvedValues + } else { + // Single value + valueToMarshal = resolvedValues[0] } // Set the value using PatchCPAValues @@ -130,19 +128,21 @@ func cpaValueSetCmdF(c client.Client, cmd *cobra.Command, args []string) error { } patchValues := map[string]json.RawMessage{ - fieldID: valueJSON, + field.ID: valueJSON, } - updatedValues, _, err := c.PatchCPAValuesForUser(context.TODO(), user.Id, patchValues) - if err != nil { - return fmt.Errorf("failed to set CPA value: %w", err) + updatedValues, _, vErr := c.PatchCPAValuesForUser(context.TODO(), user.Id, patchValues) + if vErr != nil { + return fmt.Errorf("failed to set CPA value: %w", vErr) } printer.SetSingle(true) - printer.Print(updatedValues) - - valueStr := fmt.Sprintf("%v", valueToMarshal) - fmt.Printf("Successfully set CPA value for user %s, field %s: %s\n", user.Username, targetField.Name, valueStr) + for fieldID, value := range updatedValues { + keypair := map[string]any{ + fieldID: value, + } + printer.PrintT("{{range $k, $v := .}}Successfully updated value for field {{fieldName $k}}: {{resolveValue $k $v}}{{end}}", keypair) + } return nil } diff --git a/server/cmd/mmctl/commands/cpa_value_e2e_test.go b/server/cmd/mmctl/commands/cpa_value_e2e_test.go index 3d9c8918a94..66a57063ca9 100644 --- a/server/cmd/mmctl/commands/cpa_value_e2e_test.go +++ b/server/cmd/mmctl/commands/cpa_value_e2e_test.go @@ -74,15 +74,27 @@ func (s *MmctlE2ETestSuite) TestCPAValueList() { _, appErr = s.th.App.PatchCPAValues(s.th.BasicUser.Id, updates, false) s.Require().Nil(appErr) - // Test listing the values + // Test listing the values with plain format (human-readable) + printer.SetFormat(printer.FormatPlain) err := cpaValueListCmdF(c, &cobra.Command{}, []string{s.th.BasicUser.Email}) s.Require().Nil(err) s.Require().Len(printer.GetLines(), 1) s.Require().Len(printer.GetErrorLines(), 0) - // Check that the value returned corresponds to Engineering + // Check that the human-readable format is used + output := printer.GetLines()[0].(string) + s.Require().Equal("Department (text): Engineering", output) + + // Test with JSON format to ensure raw data is preserved + printer.Clean() + printer.SetFormat(printer.FormatJSON) + err = cpaValueListCmdF(c, &cobra.Command{}, []string{s.th.BasicUser.Email}) + s.Require().Nil(err) + s.Require().Len(printer.GetLines(), 1) + s.Require().Len(printer.GetErrorLines(), 0) + + // Check that JSON format outputs raw data structure outputMap := printer.GetLines()[0].(map[string]any) - // The output contains field ID as key and value as the map value s.Require().Contains(outputMap, createdField.ID) s.Require().Equal(`"Engineering"`, string(outputMap[createdField.ID].(json.RawMessage))) }) @@ -256,8 +268,75 @@ func (s *MmctlE2ETestSuite) TestCPAValueSet() { s.Require().Contains(actualValue, goOptionID) s.Require().Contains(actualValue, reactOptionID) s.Require().Contains(actualValue, pythonOptionID) + }) + + s.Run("Set a single value for multiselect type field", func() { + c := s.th.SystemAdminClient + printer.Clean() + s.cleanCPAFields() + s.cleanCPAValuesForUser(s.th.BasicUser.Id) + + // Create a multiselect field with options + multiselectField := &model.CPAField{ + PropertyField: model.PropertyField{ + Name: "Programming Languages", + Type: model.PropertyFieldTypeMultiselect, + TargetType: "user", + }, + Attrs: model.CPAAttrs{ + Managed: "", + Options: []*model.CustomProfileAttributesSelectOption{ + {ID: model.NewId(), Name: "Go"}, + {ID: model.NewId(), Name: "Python"}, + {ID: model.NewId(), Name: "JavaScript"}, + }, + }, + } + + createdField, appErr := s.th.App.CreateCPAField(multiselectField) + s.Require().Nil(appErr) + + // Convert to CPAField to access options + cpaField, err := model.NewCPAFieldFromPropertyField(createdField) + s.Require().Nil(err) + + // Set a single value using option name + cmd := &cobra.Command{} + cmd.Flags().StringSlice("value", []string{}, "") + + err = cmd.Flags().Set("value", "Python") + s.Require().Nil(err) + + err = cpaValueSetCmdF(c, cmd, []string{s.th.BasicUser.Email, createdField.ID}) + s.Require().Nil(err) + + // Verify the value was set (should be stored as an array with single option ID) + values, appErr := s.th.App.ListCPAValues(s.th.BasicUser.Id) + s.Require().Nil(appErr) + s.Require().Len(values, 1) + s.Require().Equal(createdField.ID, values[0].FieldID) + + // Find the option ID for verification + var pythonOptionID string + for _, option := range cpaField.Attrs.Options { + if option.Name == "Python" { + pythonOptionID = option.ID + break + } + } + + // The multiselect value should be stored as an array with single option ID + // Even for single value, multiselect fields store values as arrays + actualValue := string(values[0].Value) + s.Require().Contains(actualValue, pythonOptionID) s.Require().Contains(actualValue, "[") s.Require().Contains(actualValue, "]") + // Verify it doesn't contain other option IDs + for _, option := range cpaField.Attrs.Options { + if option.Name != "Python" { + s.Require().NotContains(actualValue, option.ID) + } + } }) s.Run("Set value for user type field", func() { diff --git a/server/cmd/mmctl/commands/cpa_value_test.go b/server/cmd/mmctl/commands/cpa_value_test.go index 1709ec00c3c..b1e7f2e985c 100644 --- a/server/cmd/mmctl/commands/cpa_value_test.go +++ b/server/cmd/mmctl/commands/cpa_value_test.go @@ -25,11 +25,40 @@ func (s *MmctlUnitTestSuite) TestCPAValueListCmd() { Username: "testuser", } - mockValues := map[string]json.RawMessage{ - "field1": json.RawMessage(`"Engineering"`), - "field2": json.RawMessage(`["Go", "React", "Python"]`), + fieldID1 := model.NewId() + fieldID2 := model.NewId() + + mockFields := []*model.PropertyField{ + { + ID: fieldID1, + Name: "Department", + Type: model.PropertyFieldTypeText, + }, + { + ID: fieldID2, + Name: "Skills", + Type: model.PropertyFieldTypeMultiselect, + Attrs: model.StringInterface{ + "options": []*model.CustomProfileAttributesSelectOption{ + {ID: "opt1", Name: "Go"}, + {ID: "opt2", Name: "React"}, + {ID: "opt3", Name: "Python"}, + }, + }, + }, } + mockValues := map[string]json.RawMessage{ + fieldID1: json.RawMessage(`"Engineering"`), + fieldID2: json.RawMessage(`["opt1", "opt2", "opt3"]`), + } + + s.client. + EXPECT(). + ListCPAFields(context.TODO()). + Return(mockFields, &model.Response{}, nil). + Times(1) + s.client. EXPECT(). GetUserByEmail(context.TODO(), "testuser@example.com", ""). @@ -47,6 +76,145 @@ func (s *MmctlUnitTestSuite) TestCPAValueListCmd() { lines := printer.GetLines() s.Require().NotEmpty(lines) + + // Check that we have human-readable output + found := false + for _, line := range lines { + if lineStr, ok := line.(string); ok { + if lineStr == "Department (text): Engineering" { + found = true + break + } + } + } + s.Require().True(found, "Should contain human-readable field name and value") + }) + + s.Run("Should output raw data structure when --json flag is used", func() { + printer.Clean() + printer.SetFormat(printer.FormatJSON) + + mockUser := &model.User{ + Id: "user123", + Username: "testuser", + } + + fieldID1 := model.NewId() + mockFields := []*model.PropertyField{ + { + ID: fieldID1, + Name: "Department", + Type: model.PropertyFieldTypeText, + }, + } + + mockValues := map[string]json.RawMessage{ + fieldID1: json.RawMessage(`"Engineering"`), + } + + s.client. + EXPECT(). + ListCPAFields(context.TODO()). + Return(mockFields, &model.Response{}, nil). + Times(1) + + s.client. + EXPECT(). + GetUserByEmail(context.TODO(), "testuser@example.com", ""). + Return(mockUser, &model.Response{}, nil). + Times(1) + + s.client. + EXPECT(). + ListCPAValues(context.TODO(), "user123"). + Return(mockValues, &model.Response{}, nil). + Times(1) + + err := cpaValueListCmdF(s.client, &cobra.Command{}, []string{"testuser@example.com"}) + s.Require().NoError(err) + + lines := printer.GetLines() + s.Require().NotEmpty(lines) + + // Check that JSON format outputs raw data structure + found := false + for _, line := range lines { + if lineMap, ok := line.(map[string]any); ok { + if val, exists := lineMap[fieldID1]; exists { + if rawVal, ok := val.(json.RawMessage); ok && string(rawVal) == `"Engineering"` { + found = true + break + } + } + } + } + s.Require().True(found, "JSON output should contain raw field ID and value") + }) + + s.Run("Should resolve multiselect option names correctly", func() { + printer.Clean() + printer.SetFormat(printer.FormatPlain) + + mockUser := &model.User{ + Id: "user123", + Username: "testuser", + } + + fieldID := model.NewId() + mockFields := []*model.PropertyField{ + { + ID: fieldID, + Name: "Skills", + Type: model.PropertyFieldTypeMultiselect, + Attrs: model.StringInterface{ + "options": []*model.CustomProfileAttributesSelectOption{ + {ID: "opt1", Name: "Go"}, + {ID: "opt2", Name: "React"}, + {ID: "opt3", Name: "Python"}, + }, + }, + }, + } + + mockValues := map[string]json.RawMessage{ + fieldID: json.RawMessage(`["opt1", "opt3"]`), + } + + s.client. + EXPECT(). + ListCPAFields(context.TODO()). + Return(mockFields, &model.Response{}, nil). + Times(1) + + s.client. + EXPECT(). + GetUserByEmail(context.TODO(), "testuser@example.com", ""). + Return(mockUser, &model.Response{}, nil). + Times(1) + + s.client. + EXPECT(). + ListCPAValues(context.TODO(), "user123"). + Return(mockValues, &model.Response{}, nil). + Times(1) + + err := cpaValueListCmdF(s.client, &cobra.Command{}, []string{"testuser@example.com"}) + s.Require().NoError(err) + + lines := printer.GetLines() + s.Require().NotEmpty(lines) + + // Check that multiselect options are resolved to names + found := false + for _, line := range lines { + if lineStr, ok := line.(string); ok { + if lineStr == "Skills (multiselect): [Go, Python]" { + found = true + break + } + } + } + s.Require().True(found, "Should resolve multiselect option IDs to names") }) s.Run("Should handle empty value list scenario", func() { @@ -58,6 +226,12 @@ func (s *MmctlUnitTestSuite) TestCPAValueListCmd() { Username: "testuser", } + s.client. + EXPECT(). + ListCPAFields(context.TODO()). + Return([]*model.PropertyField{}, &model.Response{}, nil). + Times(1) + s.client. EXPECT(). GetUserByEmail(context.TODO(), "testuser@example.com", ""). @@ -88,6 +262,12 @@ func (s *MmctlUnitTestSuite) TestCPAValueListCmd() { expectedError := errors.New("API error") + s.client. + EXPECT(). + ListCPAFields(context.TODO()). + Return([]*model.PropertyField{}, &model.Response{}, nil). + Times(1) + s.client. EXPECT(). GetUserByEmail(context.TODO(), "testuser@example.com", ""). @@ -106,12 +286,35 @@ func (s *MmctlUnitTestSuite) TestCPAValueListCmd() { s.Require().Contains(err.Error(), "API error") }) + s.Run("Should handle API error when ListCPAFields fails", func() { + printer.Clean() + + expectedError := errors.New("fields API error") + + s.client. + EXPECT(). + ListCPAFields(context.TODO()). + Return(nil, &model.Response{}, expectedError). + Times(1) + + err := cpaValueListCmdF(s.client, &cobra.Command{}, []string{"testuser@example.com"}) + s.Require().Error(err) + s.Require().Contains(err.Error(), "failed to get CPA fields for template context") + s.Require().Contains(err.Error(), "fields API error") + }) + s.Run("Should handle getUserFromArg error", func() { printer.Clean() notFoundError := errors.New("user not found") notFoundResponse := &model.Response{StatusCode: http.StatusNotFound} + s.client. + EXPECT(). + ListCPAFields(context.TODO()). + Return([]*model.PropertyField{}, &model.Response{}, nil). + Times(1) + // getUserFromArg tries email first, then username, then user ID // All should return NotFoundError so it tries all methods s.client. @@ -147,16 +350,17 @@ func (s *MmctlUnitTestSuite) TestCPAValueSetCmd() { Username: "testuser", } + fieldID := model.NewId() mockFields := []*model.PropertyField{ { - ID: "field123", + ID: fieldID, Name: "Department", Type: model.PropertyFieldTypeText, }, } mockUpdatedValues := map[string]json.RawMessage{ - "field123": json.RawMessage(`"Engineering"`), + fieldID: json.RawMessage(`"Engineering"`), } cmd := &cobra.Command{} @@ -172,7 +376,7 @@ func (s *MmctlUnitTestSuite) TestCPAValueSetCmd() { EXPECT(). ListCPAFields(context.TODO()). Return(mockFields, &model.Response{}, nil). - Times(1) + Times(2) s.client. EXPECT(). @@ -180,7 +384,7 @@ func (s *MmctlUnitTestSuite) TestCPAValueSetCmd() { Return(mockUpdatedValues, &model.Response{}, nil). Times(1) - err := cpaValueSetCmdF(s.client, cmd, []string{"testuser@example.com", "field123"}) + err := cpaValueSetCmdF(s.client, cmd, []string{"testuser@example.com", fieldID}) s.Require().NoError(err) }) @@ -193,16 +397,17 @@ func (s *MmctlUnitTestSuite) TestCPAValueSetCmd() { Username: "testuser", } + fieldID := model.NewId() mockFields := []*model.PropertyField{ { - ID: "field123", + ID: fieldID, Name: "Skills", Type: model.PropertyFieldTypeMultiselect, }, } mockUpdatedValues := map[string]json.RawMessage{ - "field123": json.RawMessage(`["Go", "React", "Python"]`), + fieldID: json.RawMessage(`["Go", "React", "Python"]`), } cmd := &cobra.Command{} @@ -218,7 +423,7 @@ func (s *MmctlUnitTestSuite) TestCPAValueSetCmd() { EXPECT(). ListCPAFields(context.TODO()). Return(mockFields, &model.Response{}, nil). - Times(1) + Times(2) s.client. EXPECT(). @@ -226,7 +431,7 @@ func (s *MmctlUnitTestSuite) TestCPAValueSetCmd() { Return(mockUpdatedValues, &model.Response{}, nil). Times(1) - err := cpaValueSetCmdF(s.client, cmd, []string{"testuser@example.com", "field123"}) + err := cpaValueSetCmdF(s.client, cmd, []string{"testuser@example.com", fieldID}) s.Require().NoError(err) }) @@ -263,7 +468,7 @@ func (s *MmctlUnitTestSuite) TestCPAValueSetCmd() { err := cpaValueSetCmdF(s.client, cmd, []string{"testuser@example.com", "nonexistent_field"}) s.Require().Error(err) - s.Require().Contains(err.Error(), "field nonexistent_field not found") + s.Require().Contains(err.Error(), "failed to get field for \"nonexistent_field\"") }) s.Run("Should handle API error when PatchCPAValuesForUser fails", func() { @@ -274,9 +479,10 @@ func (s *MmctlUnitTestSuite) TestCPAValueSetCmd() { Username: "testuser", } + fieldID := model.NewId() mockFields := []*model.PropertyField{ { - ID: "field123", + ID: fieldID, Name: "Department", Type: model.PropertyFieldTypeText, }, @@ -297,7 +503,7 @@ func (s *MmctlUnitTestSuite) TestCPAValueSetCmd() { EXPECT(). ListCPAFields(context.TODO()). Return(mockFields, &model.Response{}, nil). - Times(1) + Times(2) s.client. EXPECT(). @@ -305,7 +511,7 @@ func (s *MmctlUnitTestSuite) TestCPAValueSetCmd() { Return(nil, &model.Response{}, expectedError). Times(1) - err := cpaValueSetCmdF(s.client, cmd, []string{"testuser@example.com", "field123"}) + err := cpaValueSetCmdF(s.client, cmd, []string{"testuser@example.com", fieldID}) s.Require().Error(err) s.Require().Contains(err.Error(), "failed to set CPA value") s.Require().Contains(err.Error(), "permission denied") diff --git a/server/cmd/mmctl/docs/mmctl_cpa_field_delete.rst b/server/cmd/mmctl/docs/mmctl_cpa_field_delete.rst index 9702a074eea..8bb6b28bd9e 100644 --- a/server/cmd/mmctl/docs/mmctl_cpa_field_delete.rst +++ b/server/cmd/mmctl/docs/mmctl_cpa_field_delete.rst @@ -9,11 +9,11 @@ Synopsis ~~~~~~~~ -Delete a Custom Profile Attribute field. This will automatically delete all user values for this field. +Delete a Custom Profile Attribute field by ID or name. This will automatically delete all user values for this field. :: - mmctl cpa field delete [field-id] [flags] + mmctl cpa field delete [field] [flags] Examples ~~~~~~~~ @@ -21,7 +21,8 @@ Examples :: cpa field delete n4qdbtro4j8x3n8z81p48ww9gr --confirm - cpa field delete 8kj9xm4p6f3y7n2z9q5w8r1t4v --confirm + cpa field delete Department --confirm + cpa field delete Skills --confirm Options ~~~~~~~ diff --git a/server/cmd/mmctl/docs/mmctl_cpa_field_edit.rst b/server/cmd/mmctl/docs/mmctl_cpa_field_edit.rst index 664484aa973..7ec06bb6e9a 100644 --- a/server/cmd/mmctl/docs/mmctl_cpa_field_edit.rst +++ b/server/cmd/mmctl/docs/mmctl_cpa_field_edit.rst @@ -9,11 +9,11 @@ Synopsis ~~~~~~~~ -Edit an existing Custom Profile Attribute field. +Edit an existing Custom Profile Attribute field by ID or name. :: - mmctl cpa field edit [field-id] [flags] + mmctl cpa field edit [field] [flags] Examples ~~~~~~~~ @@ -21,8 +21,8 @@ Examples :: cpa field edit n4qdbtro4j8x3n8z81p48ww9gr --name "Department Name" --managed - cpa field edit 8kj9xm4p6f3y7n2z9q5w8r1t4v --option Go --option React --option Python --option Java - cpa field edit 3h7k9m2x5b8v4n6p1q9w7r3t2y --managed=false + cpa field edit Department --option Go --option React --option Python --option Java + cpa field edit Skills --managed=false Options ~~~~~~~ diff --git a/server/cmd/mmctl/docs/mmctl_cpa_value_set.rst b/server/cmd/mmctl/docs/mmctl_cpa_value_set.rst index 02a310a1b0b..52e6d821d91 100644 --- a/server/cmd/mmctl/docs/mmctl_cpa_value_set.rst +++ b/server/cmd/mmctl/docs/mmctl_cpa_value_set.rst @@ -9,20 +9,20 @@ Synopsis ~~~~~~~~ -Set a Custom Profile Attribute field value for a specific user. +Set a Custom Profile Attribute field value for a specific user by field ID or name. :: - mmctl cpa value set [user] [field-id] [flags] + mmctl cpa value set [user] [field] [flags] Examples ~~~~~~~~ :: - cpa value set john.doe@company.com kx8m2w4r9p3q7n5t1j6h8s4c9e --value "Engineering" - cpa value set johndoe q7n3t8w5r2m9k4x6p1j3h7s8c4 --value "Go" --value "React" --value "Python" - cpa value set user123 w9r5t2n8k4x7p3q6m1j9h4s7c2 --value "Senior" + cpa value set john.doe@company.com kx8m2w4r9p3q7n5t1j6h8s4c9e --value Engineering + cpa value set johndoe Department --value Engineering + cpa value set user123 Skills --value Go --value React --value Python Options ~~~~~~~