diff --git a/src/pkg/services/m365/api/consts.go b/src/pkg/services/m365/api/consts.go index 676ce910f4..b5342f8c80 100644 --- a/src/pkg/services/m365/api/consts.go +++ b/src/pkg/services/m365/api/consts.go @@ -57,12 +57,18 @@ const ( HyperlinkDescriptionKey = "Description" HyperlinkURLKey = "Url" + LookupIDKey = "LookupId" + LookupValueKey = "LookupValue" + + PersonEmailKey = "Email" + LinkTitleFieldNamePart = "LinkTitle" ChildCountFieldNamePart = "ChildCount" LookupIDFieldNamePart = "LookupId" ODataTypeFieldNamePart = "@odata.type" ODataTypeFieldNameStringVal = "Collection(Edm.String)" + ODataTypeFieldNameIntVal = "Collection(Edm.Int32)" ReadOnlyOrHiddenFieldNamePrefix = "_" DescoratorFieldNamePrefix = "@" diff --git a/src/pkg/services/m365/api/lists.go b/src/pkg/services/m365/api/lists.go index 9dc26a783c..35843a106d 100644 --- a/src/pkg/services/m365/api/lists.go +++ b/src/pkg/services/m365/api/lists.go @@ -20,6 +20,9 @@ import ( var ErrSkippableListTemplate = clues.New("unable to create lists with skippable templates") type columnDetails struct { + createFieldName string + getFieldName string + isPersonColumn bool isMultipleEnabled bool hasDefaultedToText bool } @@ -300,7 +303,10 @@ func ToListable(orig models.Listable, listName string) (models.Listable, map[str newList.SetParentReference(orig.GetParentReference()) columns := make([]models.ColumnDefinitionable, 0) - columnNames := map[string]*columnDetails{TitleColumnName: {}} + columnNames := map[string]*columnDetails{TitleColumnName: { + getFieldName: TitleColumnName, + createFieldName: TitleColumnName, + }} for _, cd := range orig.GetColumns() { var ( @@ -380,8 +386,18 @@ func setColumnType( orig models.ColumnDefinitionable, columnNames map[string]*columnDetails, ) { + colName := ptr.Val(newColumn.GetName()) colDetails := &columnDetails{} + // for certain columns like 'person', the column name is say 'personName'. + // if the list item for that column holds single value, + // the field data is fetched as '{"personNameLookupId": "10"}' + // if the list item for that column holds multiple values, + // the field data is fetched as '{"personName": [{"lookupId": 10}, {"lookupId": 11}]}'. + // Hence this function helps us to determine which name to use while accessing stored data + colDetails.getFieldName = colName + colDetails.createFieldName = colName + switch { case orig.GetText() != nil: newColumn.SetText(orig.GetText()) @@ -410,6 +426,16 @@ func setColumnType( case orig.GetTerm() != nil: newColumn.SetTerm(orig.GetTerm()) case orig.GetPersonOrGroup() != nil: + colDetails.isPersonColumn = true + isMultipleEnabled := ptr.Val(orig.GetPersonOrGroup().GetAllowMultipleSelection()) + colDetails.isMultipleEnabled = isMultipleEnabled + updatedName := colName + LookupIDFieldNamePart + colDetails.createFieldName = updatedName + + if !isMultipleEnabled { + colDetails.getFieldName = updatedName + } + newColumn.SetPersonOrGroup(orig.GetPersonOrGroup()) default: colDetails.hasDefaultedToText = true @@ -417,7 +443,7 @@ func setColumnType( newColumn.SetText(models.NewTextColumn()) } - columnNames[ptr.Val(newColumn.GetName())] = colDetails + columnNames[colName] = colDetails } // CloneListItem creates a new `SharePoint.ListItem` and stores the original item's @@ -490,24 +516,59 @@ func setAdditionalDataByColumnNames( fieldData := orig.GetAdditionalData() filteredData := make(map[string]any) - for colName, colDetails := range columnNames { - if val, ok := fieldData[colName]; ok { + for _, colDetails := range columnNames { + if val, ok := fieldData[colDetails.getFieldName]; ok { // for columns like 'choice', even though it has an option to hold single/multiple values, // the columnDefinition property 'allowMultipleValues' is not available. // Hence we determine single/multiple from the actual field data. - if isSlice(val) { + if !colDetails.isMultipleEnabled && isSlice(val) { colDetails.isMultipleEnabled = true } - filteredData[colName] = fieldData[colName] + filteredData[colDetails.createFieldName] = val + populateMultipleValues(val, filteredData, colDetails) } - specifyODataType(filteredData, colDetails, colName) + specifyODataType(filteredData, colDetails, colDetails.createFieldName) } return filteredData } +func populateMultipleValues(val any, filteredData map[string]any, colDetails *columnDetails) { + if !colDetails.isMultipleEnabled { + return + } + + if !colDetails.isPersonColumn { + return + } + + multiNestedFields, ok := val.([]any) + if !ok || len(multiNestedFields) == 0 { + return + } + + lookupIDs := make([]float64, 0) + lookupKeys := []string{LookupIDKey, LookupValueKey, PersonEmailKey} + + for _, nestedFields := range multiNestedFields { + md, ok := nestedFields.(map[string]any) + if !ok || !keys.HasKeys(md, lookupKeys...) { + continue + } + + v, ok := md[LookupIDKey].(*float64) + if !ok { + continue + } + + lookupIDs = append(lookupIDs, ptr.Val(v)) + } + + filteredData[colDetails.createFieldName] = lookupIDs +} + // when creating list items with multiple values for a single column // we let the API know that we are sending a collection. // Hence this adds an additional field '@@odata.type' @@ -520,7 +581,15 @@ func specifyODataType(filteredData map[string]any, colDetails *columnDetails, co return } - if colDetails.isMultipleEnabled { + // only specify odata.type for columns holding multiple data + if !colDetails.isMultipleEnabled { + return + } + + switch { + case colDetails.isPersonColumn: + filteredData[colName+ODataTypeFieldNamePart] = ODataTypeFieldNameIntVal + default: filteredData[colName+ODataTypeFieldNamePart] = ODataTypeFieldNameStringVal } } diff --git a/src/pkg/services/m365/api/lists_test.go b/src/pkg/services/m365/api/lists_test.go index 37cba33ac1..2b07dbe712 100644 --- a/src/pkg/services/m365/api/lists_test.go +++ b/src/pkg/services/m365/api/lists_test.go @@ -234,43 +234,71 @@ func (suite *ListsUnitSuite) TestColumnDefinitionable_ColumnType() { tests := []struct { name string getOrig func() models.ColumnDefinitionable - checkFn func(models.ColumnDefinitionable) bool + checkFn func(cd models.ColumnDefinitionable, colNames map[string]*columnDetails) bool }{ { - name: "column type should be number", + name: "number column", getOrig: func() models.ColumnDefinitionable { numColumn := models.NewNumberColumn() cd := models.NewColumnDefinition() cd.SetNumber(numColumn) + cd.SetName(ptr.To("num-col")) return cd }, - checkFn: func(cd models.ColumnDefinitionable) bool { - return cd.GetNumber() != nil + checkFn: func(cd models.ColumnDefinitionable, colNames map[string]*columnDetails) bool { + return cd.GetNumber() != nil && + colNames["num-col"].getFieldName == "num-col" && + colNames["num-col"].createFieldName == "num-col" }, }, { - name: "column type should be person or group", + name: "person or group column, single value", getOrig: func() models.ColumnDefinitionable { pgColumn := models.NewPersonOrGroupColumn() cd := models.NewColumnDefinition() cd.SetPersonOrGroup(pgColumn) + cd.SetName(ptr.To("pg-col")) return cd }, - checkFn: func(cd models.ColumnDefinitionable) bool { - return cd.GetPersonOrGroup() != nil + checkFn: func(cd models.ColumnDefinitionable, colNames map[string]*columnDetails) bool { + return cd.GetPersonOrGroup() != nil && + colNames["pg-col"].getFieldName == "pg-colLookupId" && + colNames["pg-col"].createFieldName == "pg-colLookupId" }, }, { - name: "column type should default to text", + name: "person or group column, multiple value", getOrig: func() models.ColumnDefinitionable { - return models.NewColumnDefinition() + pgColumn := models.NewPersonOrGroupColumn() + pgColumn.SetAllowMultipleSelection(ptr.To(true)) + + cd := models.NewColumnDefinition() + cd.SetPersonOrGroup(pgColumn) + cd.SetName(ptr.To("pg-col")) + + return cd + }, + checkFn: func(cd models.ColumnDefinitionable, colNames map[string]*columnDetails) bool { + return cd.GetPersonOrGroup() != nil && + colNames["pg-col"].getFieldName == "pg-col" && + colNames["pg-col"].createFieldName == "pg-colLookupId" + }, + }, + { + name: "defaulted to text column", + getOrig: func() models.ColumnDefinitionable { + cd := models.NewColumnDefinition() + cd.SetName(ptr.To("some-col")) + return cd }, - checkFn: func(cd models.ColumnDefinitionable) bool { - return cd.GetText() != nil + checkFn: func(cd models.ColumnDefinitionable, colNames map[string]*columnDetails) bool { + return cd.GetText() != nil && + colNames["some-col"].getFieldName == "some-col" && + colNames["some-col"].createFieldName == "some-col" }, }, } @@ -282,7 +310,7 @@ func (suite *ListsUnitSuite) TestColumnDefinitionable_ColumnType() { newCd := cloneColumnDefinitionable(test.getOrig(), colNames) require.NotEmpty(t, newCd) - assert.True(t, test.checkFn(newCd)) + assert.True(t, test.checkFn(newCd, colNames)) }) } } @@ -345,8 +373,11 @@ func (suite *ListsUnitSuite) TestColumnDefinitionable_LegacyColumns() { }) return lst }, - length: 0, - expectedColNames: map[string]*columnDetails{TitleColumnName: {}}, + length: 0, + expectedColNames: map[string]*columnDetails{TitleColumnName: { + getFieldName: TitleColumnName, + createFieldName: TitleColumnName, + }}, }, { name: "title and legacy columns", @@ -360,8 +391,11 @@ func (suite *ListsUnitSuite) TestColumnDefinitionable_LegacyColumns() { }) return lst }, - length: 0, - expectedColNames: map[string]*columnDetails{TitleColumnName: {}}, + length: 0, + expectedColNames: map[string]*columnDetails{TitleColumnName: { + getFieldName: TitleColumnName, + createFieldName: TitleColumnName, + }}, }, { name: "readonly and legacy columns", @@ -375,8 +409,11 @@ func (suite *ListsUnitSuite) TestColumnDefinitionable_LegacyColumns() { }) return lst }, - length: 0, - expectedColNames: map[string]*columnDetails{TitleColumnName: {}}, + length: 0, + expectedColNames: map[string]*columnDetails{TitleColumnName: { + getFieldName: TitleColumnName, + createFieldName: TitleColumnName, + }}, }, { name: "legacy and a text column", @@ -392,8 +429,14 @@ func (suite *ListsUnitSuite) TestColumnDefinitionable_LegacyColumns() { }, length: 1, expectedColNames: map[string]*columnDetails{ - TitleColumnName: {}, - textColumnName: {}, + TitleColumnName: { + getFieldName: TitleColumnName, + createFieldName: TitleColumnName, + }, + textColumnName: { + getFieldName: textColumnName, + createFieldName: textColumnName, + }, }, }, } @@ -442,7 +485,10 @@ func (suite *ListsUnitSuite) TestFieldValueSetable() { origFs = models.NewFieldValueSet() origFs.SetAdditionalData(additionalData) - colNames["itemName"] = &columnDetails{} + colNames["itemName"] = &columnDetails{ + getFieldName: "itemName", + createFieldName: "itemName", + } fs = retrieveFieldData(origFs, colNames) fsAdditionalData = fs.GetAdditionalData() @@ -510,7 +556,10 @@ func (suite *ListsUnitSuite) TestFieldValueSetable_Location() { origFs.SetAdditionalData(additionalData) colNames := map[string]*columnDetails{ - "MyAddress": {}, + "MyAddress": { + getFieldName: "MyAddress", + createFieldName: "MyAddress", + }, } fs := retrieveFieldData(origFs, colNames) @@ -709,51 +758,95 @@ func (suite *ListsUnitSuite) TestSetAdditionalDataByColumnNames() { tests := []struct { name string additionalData map[string]any - colName string + colDetails map[string]*columnDetails assertFn assert.BoolAssertionFunc + expectedResult map[string]any }{ { name: "choice column, single value", additionalData: map[string]any{ - "choice": "good", + "choice": ptr.To("good"), + }, + colDetails: map[string]*columnDetails{ + "choice": { + getFieldName: "choice", + createFieldName: "choice", + }, }, - colName: "choice", assertFn: assert.False, + expectedResult: map[string]any{ + "choice": ptr.To("good"), + }, }, { name: "choice column, multiple values", additionalData: map[string]any{ - "choice": []string{"good", "ok"}, + "choice": []*string{ + ptr.To("good"), + ptr.To("ok"), + }, + }, + colDetails: map[string]*columnDetails{ + "choice": { + getFieldName: "choice", + createFieldName: "choice", + }, }, - colName: "choice", assertFn: assert.True, + expectedResult: map[string]any{ + "choice@odata.type": "Collection(Edm.String)", + "choice": []*string{ + ptr.To("good"), + ptr.To("ok"), + }, + }, }, { name: "person column, single value", additionalData: map[string]any{ - "PersonsLookupId": 10, + "PersonsLookupId": ptr.To(10), + }, + colDetails: map[string]*columnDetails{ + "Persons": { + isPersonColumn: true, + getFieldName: "PersonsLookupId", + createFieldName: "PersonsLookupId", + }, }, - colName: "PersonsLookupId", assertFn: assert.False, + expectedResult: map[string]any{ + "PersonsLookupId": ptr.To(10), + }, }, { name: "person column, multiple values", additionalData: map[string]any{ - "Persons": []map[string]any{ - { - "LookupId": 10, - "LookupValue": "Who1", - "Email": "Who1@10rqc2.onmicrosoft.com", + "Persons": []any{ + map[string]any{ + "LookupId": ptr.To(float64(10)), + "LookupValue": ptr.To("Who1"), + "Email": ptr.To("Who1@10rqc2.onmicrosoft.com"), }, - { - "LookupId": 11, - "LookupValue": "Who2", - "Email": "Who2@10rqc2.onmicrosoft.com", + map[string]any{ + "LookupId": ptr.To(float64(11)), + "LookupValue": ptr.To("Who2"), + "Email": ptr.To("Who2@10rqc2.onmicrosoft.com"), }, }, }, - colName: "Persons", + colDetails: map[string]*columnDetails{ + "Persons": { + isPersonColumn: true, + isMultipleEnabled: true, + getFieldName: "Persons", + createFieldName: "PersonsLookupId", + }, + }, assertFn: assert.True, + expectedResult: map[string]any{ + "PersonsLookupId": []float64{10, 11}, + "PersonsLookupId@odata.type": "Collection(Edm.Int32)", + }, }, } @@ -761,13 +854,9 @@ func (suite *ListsUnitSuite) TestSetAdditionalDataByColumnNames() { origFs := models.NewFieldValueSet() origFs.SetAdditionalData(test.additionalData) - colNames := map[string]*columnDetails{ - test.colName: {}, - } - suite.Run(test.name, func() { - setAdditionalDataByColumnNames(origFs, colNames) - test.assertFn(t, colNames[test.colName].isMultipleEnabled) + res := setAdditionalDataByColumnNames(origFs, test.colDetails) + assert.Equal(t, test.expectedResult, res) }) } }