diff --git a/src/pkg/services/m365/api/consts.go b/src/pkg/services/m365/api/consts.go index 96778c532a..676ce910f4 100644 --- a/src/pkg/services/m365/api/consts.go +++ b/src/pkg/services/m365/api/consts.go @@ -61,6 +61,9 @@ const ( ChildCountFieldNamePart = "ChildCount" LookupIDFieldNamePart = "LookupId" + ODataTypeFieldNamePart = "@odata.type" + ODataTypeFieldNameStringVal = "Collection(Edm.String)" + ReadOnlyOrHiddenFieldNamePrefix = "_" DescoratorFieldNamePrefix = "@" diff --git a/src/pkg/services/m365/api/lists.go b/src/pkg/services/m365/api/lists.go index 80493f4cea..9dc26a783c 100644 --- a/src/pkg/services/m365/api/lists.go +++ b/src/pkg/services/m365/api/lists.go @@ -3,6 +3,7 @@ package api import ( "context" "fmt" + "reflect" "strings" "github.com/alcionai/clues" @@ -18,6 +19,11 @@ import ( var ErrSkippableListTemplate = clues.New("unable to create lists with skippable templates") +type columnDetails struct { + isMultipleEnabled bool + hasDefaultedToText bool +} + // --------------------------------------------------------------------------- // controller // --------------------------------------------------------------------------- @@ -277,7 +283,7 @@ func BytesToListable(bytes []byte) (models.Listable, error) { // not attached in this method. // ListItems are not included in creation of new list, and have to be restored // in separate call. -func ToListable(orig models.Listable, listName string) (models.Listable, map[string]any) { +func ToListable(orig models.Listable, listName string) (models.Listable, map[string]*columnDetails) { newList := models.NewList() newList.SetContentTypes(orig.GetContentTypes()) @@ -294,7 +300,7 @@ func ToListable(orig models.Listable, listName string) (models.Listable, map[str newList.SetParentReference(orig.GetParentReference()) columns := make([]models.ColumnDefinitionable, 0) - columnNames := map[string]any{TitleColumnName: nil} + columnNames := map[string]*columnDetails{TitleColumnName: {}} for _, cd := range orig.GetColumns() { var ( @@ -316,8 +322,7 @@ func ToListable(orig models.Listable, listName string) (models.Listable, map[str continue } - columns = append(columns, cloneColumnDefinitionable(cd)) - columnNames[ptr.Val(cd.GetName())] = nil + columns = append(columns, cloneColumnDefinitionable(cd, columnNames)) } newList.SetColumns(columns) @@ -327,7 +332,10 @@ func ToListable(orig models.Listable, listName string) (models.Listable, map[str // cloneColumnDefinitionable utility function for encapsulating models.ColumnDefinitionable data // into new object for upload. -func cloneColumnDefinitionable(orig models.ColumnDefinitionable) models.ColumnDefinitionable { +func cloneColumnDefinitionable( + orig models.ColumnDefinitionable, + columnNames map[string]*columnDetails, +) models.ColumnDefinitionable { newColumn := models.NewColumnDefinition() // column attributes @@ -351,7 +359,7 @@ func cloneColumnDefinitionable(orig models.ColumnDefinitionable) models.ColumnDe newColumn.SetEnforceUniqueValues(orig.GetEnforceUniqueValues()) // column types - setColumnType(newColumn, orig) + setColumnType(newColumn, orig, columnNames) // Requires nil checks to avoid Graph error: 'General exception while processing' defaultValue := orig.GetDefaultValue() @@ -367,7 +375,13 @@ func cloneColumnDefinitionable(orig models.ColumnDefinitionable) models.ColumnDe return newColumn } -func setColumnType(newColumn *models.ColumnDefinition, orig models.ColumnDefinitionable) { +func setColumnType( + newColumn *models.ColumnDefinition, + orig models.ColumnDefinitionable, + columnNames map[string]*columnDetails, +) { + colDetails := &columnDetails{} + switch { case orig.GetText() != nil: newColumn.SetText(orig.GetText()) @@ -398,14 +412,18 @@ func setColumnType(newColumn *models.ColumnDefinition, orig models.ColumnDefinit case orig.GetPersonOrGroup() != nil: newColumn.SetPersonOrGroup(orig.GetPersonOrGroup()) default: + colDetails.hasDefaultedToText = true + newColumn.SetText(models.NewTextColumn()) } + + columnNames[ptr.Val(newColumn.GetName())] = colDetails } // CloneListItem creates a new `SharePoint.ListItem` and stores the original item's // M365 data into it set fields. // - https://learn.microsoft.com/en-us/graph/api/resources/listitem?view=graph-rest-1.0 -func CloneListItem(orig models.ListItemable, columnNames map[string]any) models.ListItemable { +func CloneListItem(orig models.ListItemable, columnNames map[string]*columnDetails) models.ListItemable { newItem := models.NewListItem() // list item data @@ -442,7 +460,7 @@ func CloneListItem(orig models.ListItemable, columnNames map[string]any) models. // additionalData map // Further documentation on FieldValueSets: // - https://learn.microsoft.com/en-us/graph/api/resources/fieldvalueset?view=graph-rest-1.0 -func retrieveFieldData(orig models.FieldValueSetable, columnNames map[string]any) models.FieldValueSetable { +func retrieveFieldData(orig models.FieldValueSetable, columnNames map[string]*columnDetails) models.FieldValueSetable { fields := models.NewFieldValueSet() additionalData := setAdditionalDataByColumnNames(orig, columnNames) @@ -463,7 +481,7 @@ func retrieveFieldData(orig models.FieldValueSetable, columnNames map[string]any func setAdditionalDataByColumnNames( orig models.FieldValueSetable, - columnNames map[string]any, + columnNames map[string]*columnDetails, ) map[string]any { if orig == nil { return make(map[string]any) @@ -472,15 +490,41 @@ func setAdditionalDataByColumnNames( fieldData := orig.GetAdditionalData() filteredData := make(map[string]any) - for colName := range columnNames { - if _, ok := fieldData[colName]; ok { + for colName, colDetails := range columnNames { + if val, ok := fieldData[colName]; 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) { + colDetails.isMultipleEnabled = true + } + filteredData[colName] = fieldData[colName] } + + specifyODataType(filteredData, colDetails, colName) } return filteredData } +// 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' +// with value depending on type of column. +func specifyODataType(filteredData map[string]any, colDetails *columnDetails, colName string) { + // text column itself does not allow holding multiple values + // some columns like 'term'/'managed metadata' have, + // but they get defaulted to text column. + if colDetails.hasDefaultedToText { + return + } + + if colDetails.isMultipleEnabled { + filteredData[colName+ODataTypeFieldNamePart] = ODataTypeFieldNameStringVal + } +} + func hasAddressFields(additionalData map[string]any) (map[string]any, string, bool) { for k, v := range additionalData { nestedFields, ok := v.(map[string]any) @@ -631,3 +675,7 @@ func ListCollisionKey(list models.Listable) string { return ptr.Val(list.GetDisplayName()) } + +func isSlice(val any) bool { + return reflect.TypeOf(val).Kind() == reflect.Slice +} diff --git a/src/pkg/services/m365/api/lists_test.go b/src/pkg/services/m365/api/lists_test.go index 100cb9288a..37cba33ac1 100644 --- a/src/pkg/services/m365/api/lists_test.go +++ b/src/pkg/services/m365/api/lists_test.go @@ -162,12 +162,12 @@ func (suite *ListsUnitSuite) TestColumnDefinitionable_GetValidation() { for _, test := range tests { suite.Run(test.name, func() { t := suite.T() + colNames := map[string]*columnDetails{} orig := test.getOrig() - newCd := cloneColumnDefinitionable(orig) + newCd := cloneColumnDefinitionable(orig, colNames) require.NotEmpty(t, newCd) - test.expect(t, newCd.GetValidation()) }) } @@ -219,9 +219,10 @@ func (suite *ListsUnitSuite) TestColumnDefinitionable_GetDefaultValue() { for _, test := range tests { suite.Run(test.name, func() { t := suite.T() + colNames := map[string]*columnDetails{} orig := test.getOrig() - newCd := cloneColumnDefinitionable(orig) + newCd := cloneColumnDefinitionable(orig, colNames) require.NotEmpty(t, newCd) test.expect(t, newCd) @@ -277,9 +278,8 @@ func (suite *ListsUnitSuite) TestColumnDefinitionable_ColumnType() { for _, test := range tests { suite.Run(test.name, func() { t := suite.T() - - orig := test.getOrig() - newCd := cloneColumnDefinitionable(orig) + colNames := map[string]*columnDetails{} + newCd := cloneColumnDefinitionable(test.getOrig(), colNames) require.NotEmpty(t, newCd) assert.True(t, test.checkFn(newCd)) @@ -332,7 +332,7 @@ func (suite *ListsUnitSuite) TestColumnDefinitionable_LegacyColumns() { name string getList func() *models.List length int - expectedColNames map[string]any + expectedColNames map[string]*columnDetails }{ { name: "all legacy columns", @@ -346,7 +346,7 @@ func (suite *ListsUnitSuite) TestColumnDefinitionable_LegacyColumns() { return lst }, length: 0, - expectedColNames: map[string]any{TitleColumnName: nil}, + expectedColNames: map[string]*columnDetails{TitleColumnName: {}}, }, { name: "title and legacy columns", @@ -361,7 +361,7 @@ func (suite *ListsUnitSuite) TestColumnDefinitionable_LegacyColumns() { return lst }, length: 0, - expectedColNames: map[string]any{TitleColumnName: nil}, + expectedColNames: map[string]*columnDetails{TitleColumnName: {}}, }, { name: "readonly and legacy columns", @@ -376,7 +376,7 @@ func (suite *ListsUnitSuite) TestColumnDefinitionable_LegacyColumns() { return lst }, length: 0, - expectedColNames: map[string]any{TitleColumnName: nil}, + expectedColNames: map[string]*columnDetails{TitleColumnName: {}}, }, { name: "legacy and a text column", @@ -391,9 +391,9 @@ func (suite *ListsUnitSuite) TestColumnDefinitionable_LegacyColumns() { return lst }, length: 1, - expectedColNames: map[string]any{ - TitleColumnName: nil, - textColumnName: nil, + expectedColNames: map[string]*columnDetails{ + TitleColumnName: {}, + textColumnName: {}, }, }, } @@ -432,7 +432,7 @@ func (suite *ListsUnitSuite) TestFieldValueSetable() { origFs := models.NewFieldValueSet() origFs.SetAdditionalData(additionalData) - colNames := map[string]any{} + colNames := map[string]*columnDetails{} fs := retrieveFieldData(origFs, colNames) fsAdditionalData := fs.GetAdditionalData() @@ -442,7 +442,7 @@ func (suite *ListsUnitSuite) TestFieldValueSetable() { origFs = models.NewFieldValueSet() origFs.SetAdditionalData(additionalData) - colNames["itemName"] = struct{}{} + colNames["itemName"] = &columnDetails{} fs = retrieveFieldData(origFs, colNames) fsAdditionalData = fs.GetAdditionalData() @@ -509,8 +509,8 @@ func (suite *ListsUnitSuite) TestFieldValueSetable_Location() { origFs := models.NewFieldValueSet() origFs.SetAdditionalData(additionalData) - colNames := map[string]any{ - "MyAddress": nil, + colNames := map[string]*columnDetails{ + "MyAddress": {}, } fs := retrieveFieldData(origFs, colNames) @@ -703,6 +703,75 @@ func (suite *ListsUnitSuite) TestConcatenateHyperlinkFields() { } } +func (suite *ListsUnitSuite) TestSetAdditionalDataByColumnNames() { + t := suite.T() + + tests := []struct { + name string + additionalData map[string]any + colName string + assertFn assert.BoolAssertionFunc + }{ + { + name: "choice column, single value", + additionalData: map[string]any{ + "choice": "good", + }, + colName: "choice", + assertFn: assert.False, + }, + { + name: "choice column, multiple values", + additionalData: map[string]any{ + "choice": []string{"good", "ok"}, + }, + colName: "choice", + assertFn: assert.True, + }, + { + name: "person column, single value", + additionalData: map[string]any{ + "PersonsLookupId": 10, + }, + colName: "PersonsLookupId", + assertFn: assert.False, + }, + { + name: "person column, multiple values", + additionalData: map[string]any{ + "Persons": []map[string]any{ + { + "LookupId": 10, + "LookupValue": "Who1", + "Email": "Who1@10rqc2.onmicrosoft.com", + }, + { + "LookupId": 11, + "LookupValue": "Who2", + "Email": "Who2@10rqc2.onmicrosoft.com", + }, + }, + }, + colName: "Persons", + assertFn: assert.True, + }, + } + + for _, test := range tests { + 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) + }) + } +} + type ListsAPIIntgSuite struct { tester.Suite its intgTesterSetup