From 6faf18c2e887ee848eee3c28d035a15fc6c50559 Mon Sep 17 00:00:00 2001 From: HiteshRepo Date: Wed, 24 Jan 2024 15:17:21 +0530 Subject: [PATCH 01/14] provisions to identify fields with multiple value --- src/pkg/services/m365/api/consts.go | 3 + src/pkg/services/m365/api/lists.go | 53 +++++++++++---- src/pkg/services/m365/api/lists_test.go | 89 ++++++++++++++++++++++--- 3 files changed, 123 insertions(+), 22 deletions(-) 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..cb9efe4ada 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,10 @@ import ( var ErrSkippableListTemplate = clues.New("unable to create lists with skippable templates") +type columnDetails struct { + isMultipleEnabled bool +} + // --------------------------------------------------------------------------- // controller // --------------------------------------------------------------------------- @@ -277,7 +282,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 +299,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 +321,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 +331,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 +358,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 +374,11 @@ 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, +) { switch { case orig.GetText() != nil: newColumn.SetText(orig.GetText()) @@ -400,12 +411,14 @@ func setColumnType(newColumn *models.ColumnDefinition, orig models.ColumnDefinit default: newColumn.SetText(models.NewTextColumn()) } + + columnNames[ptr.Val(newColumn.GetName())] = &columnDetails{} } // 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 +455,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 +476,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 +485,31 @@ 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 { + setMultipleEnabled(val, colDetails) + filteredData[colName] = fieldData[colName] } + + specifyODataType(filteredData, colDetails, colName) } return filteredData } +func setMultipleEnabled(val any, colDetails *columnDetails) { + if reflect.TypeOf(val).Kind() == reflect.Slice { + colDetails.isMultipleEnabled = true + } +} + +func specifyODataType(filteredData map[string]any, colDetails *columnDetails, colName string) { + 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) diff --git a/src/pkg/services/m365/api/lists_test.go b/src/pkg/services/m365/api/lists_test.go index 100cb9288a..64dc0f1ecc 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)) @@ -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 From fa4ea5b9b825c840a8b924a627e06617e47477fe Mon Sep 17 00:00:00 2001 From: HiteshRepo Date: Wed, 24 Jan 2024 18:35:44 +0530 Subject: [PATCH 02/14] handles multiple persons list items --- src/pkg/services/m365/api/consts.go | 6 ++ src/pkg/services/m365/api/lists.go | 93 +++++++++++++++++++++++-- src/pkg/services/m365/api/lists_test.go | 74 ++++++++++++++------ 3 files changed, 144 insertions(+), 29 deletions(-) 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 cb9efe4ada..34a276504e 100644 --- a/src/pkg/services/m365/api/lists.go +++ b/src/pkg/services/m365/api/lists.go @@ -20,6 +20,7 @@ import ( var ErrSkippableListTemplate = clues.New("unable to create lists with skippable templates") type columnDetails struct { + isPersonColumn bool isMultipleEnabled bool } @@ -379,6 +380,8 @@ func setColumnType( orig models.ColumnDefinitionable, columnNames map[string]*columnDetails, ) { + colDetails := &columnDetails{} + switch { case orig.GetText() != nil: newColumn.SetText(orig.GetText()) @@ -407,12 +410,14 @@ func setColumnType( case orig.GetTerm() != nil: newColumn.SetTerm(orig.GetTerm()) case orig.GetPersonOrGroup() != nil: + colDetails.isPersonColumn = true + colDetails.isMultipleEnabled = ptr.Val(orig.GetPersonOrGroup().GetAllowMultipleSelection()) newColumn.SetPersonOrGroup(orig.GetPersonOrGroup()) default: newColumn.SetText(models.NewTextColumn()) } - columnNames[ptr.Val(newColumn.GetName())] = &columnDetails{} + columnNames[ptr.Val(newColumn.GetName())] = colDetails } // CloneListItem creates a new `SharePoint.ListItem` and stores the original item's @@ -485,27 +490,103 @@ func setAdditionalDataByColumnNames( fieldData := orig.GetAdditionalData() filteredData := make(map[string]any) + // for certain columns like 'person', the column name is for example '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 is function helps us to determine which name to use while accessing stored data + nameForGet := func(isMultipleEnabled bool, colName, updatedName string) string { + if isMultipleEnabled { + return colName + } + + return updatedName + } + for colName, colDetails := range columnNames { - if val, ok := fieldData[colName]; ok { - setMultipleEnabled(val, colDetails) + updatedColName := updateColName(colName, colDetails) + getColName := nameForGet(colDetails.isMultipleEnabled, colName, updatedColName) - filteredData[colName] = fieldData[colName] + if val, ok := fieldData[getColName]; ok { + setMultipleEnabled(val, colDetails) + filteredData[updatedColName] = val + populateMultipleValues(val, filteredData, updatedColName, colDetails) } - specifyODataType(filteredData, colDetails, colName) + specifyODataType(filteredData, colDetails, updatedColName) } return filteredData } +func populateMultipleValues(val any, filteredData map[string]any, colName string, colDetails *columnDetails) { + if !colDetails.isMultipleEnabled { + return + } + + if !colDetails.isPersonColumn { + return + } + + multiNestedFields, ok := val.([]any) + if !ok || len(multiNestedFields) == 0 { + return + } + + lookupIDs := make([]float64, 0) + + for _, nestedFields := range multiNestedFields { + if md, ok := nestedFields.(map[string]any); ok && keys.HasKeys(md, + []string{LookupIDKey, LookupValueKey, PersonEmailKey}...) { + lookupID, ok := md[LookupIDKey].(*float64) + if !ok { + continue + } + + lookupIDs = append(lookupIDs, ptr.Val(lookupID)) + } + } + + filteredData[colName] = lookupIDs +} + func setMultipleEnabled(val any, colDetails *columnDetails) { + // already set while column definition + // not required to determined from field values + if colDetails.isMultipleEnabled { + return + } + + // for columns like choice, the columnDefinition property 'allowMultipleValues' + // is not available, even though it has an option to hold single/multiple values. + // Hence we determine single/multiple from the actual field data. if reflect.TypeOf(val).Kind() == reflect.Slice { colDetails.isMultipleEnabled = true } } +func updateColName(colName string, colDetails *columnDetails) string { + if !colDetails.isPersonColumn { + return colName + } + + return colName + LookupIDFieldNamePart +} + +// 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) { - if colDetails.isMultipleEnabled { + 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 64dc0f1ecc..d139fd32bf 100644 --- a/src/pkg/services/m365/api/lists_test.go +++ b/src/pkg/services/m365/api/lists_test.go @@ -709,51 +709,83 @@ 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": {}, }, - 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": {}, }, - 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}, }, - 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, + }, + }, assertFn: assert.True, + expectedResult: map[string]any{ + "PersonsLookupId": []float64{10, 11}, + "PersonsLookupId@odata.type": "Collection(Edm.Int32)", + }, }, } @@ -761,13 +793,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) }) } } From 26c957d225f260708a52e0e25d5371bfbb69cc5b Mon Sep 17 00:00:00 2001 From: HiteshRepo Date: Wed, 24 Jan 2024 18:56:59 +0530 Subject: [PATCH 03/14] handles multiple lookup field values --- src/pkg/services/m365/api/lists.go | 26 +++++++++++++---- src/pkg/services/m365/api/lists_test.go | 39 +++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 5 deletions(-) diff --git a/src/pkg/services/m365/api/lists.go b/src/pkg/services/m365/api/lists.go index 34a276504e..6e7884c7ea 100644 --- a/src/pkg/services/m365/api/lists.go +++ b/src/pkg/services/m365/api/lists.go @@ -21,6 +21,7 @@ var ErrSkippableListTemplate = clues.New("unable to create lists with skippable type columnDetails struct { isPersonColumn bool + isLookupColumn bool isMultipleEnabled bool } @@ -404,6 +405,8 @@ func setColumnType( case orig.GetNumber() != nil: newColumn.SetNumber(orig.GetNumber()) case orig.GetLookup() != nil: + colDetails.isLookupColumn = true + colDetails.isMultipleEnabled = ptr.Val(orig.GetLookup().GetAllowMultipleValues()) newColumn.SetLookup(orig.GetLookup()) case orig.GetThumbnail() != nil: newColumn.SetThumbnail(orig.GetThumbnail()) @@ -525,7 +528,8 @@ func populateMultipleValues(val any, filteredData map[string]any, colName string return } - if !colDetails.isPersonColumn { + if !colDetails.isPersonColumn && + !colDetails.isLookupColumn { return } @@ -536,9 +540,20 @@ func populateMultipleValues(val any, filteredData map[string]any, colName string lookupIDs := make([]float64, 0) + checkFields := func(colDetails *columnDetails) []string { + if colDetails.isLookupColumn { + return []string{LookupIDKey, LookupValueKey} + } + + if colDetails.isPersonColumn { + return []string{LookupIDKey, LookupValueKey, PersonEmailKey} + } + + return []string{} + } + for _, nestedFields := range multiNestedFields { - if md, ok := nestedFields.(map[string]any); ok && keys.HasKeys(md, - []string{LookupIDKey, LookupValueKey, PersonEmailKey}...) { + if md, ok := nestedFields.(map[string]any); ok && keys.HasKeys(md, checkFields(colDetails)...) { lookupID, ok := md[LookupIDKey].(*float64) if !ok { continue @@ -567,7 +582,8 @@ func setMultipleEnabled(val any, colDetails *columnDetails) { } func updateColName(colName string, colDetails *columnDetails) string { - if !colDetails.isPersonColumn { + if !colDetails.isPersonColumn && + !colDetails.isLookupColumn { return colName } @@ -584,7 +600,7 @@ func specifyODataType(filteredData map[string]any, colDetails *columnDetails, co } switch { - case colDetails.isPersonColumn: + case colDetails.isPersonColumn || colDetails.isLookupColumn: 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 d139fd32bf..057cade9a8 100644 --- a/src/pkg/services/m365/api/lists_test.go +++ b/src/pkg/services/m365/api/lists_test.go @@ -787,6 +787,45 @@ func (suite *ListsUnitSuite) TestSetAdditionalDataByColumnNames() { "PersonsLookupId@odata.type": "Collection(Edm.Int32)", }, }, + { + name: "lookup column, single value", + additionalData: map[string]any{ + "ReferLookupId": ptr.To(10), + }, + colDetails: map[string]*columnDetails{ + "Refer": {isLookupColumn: true}, + }, + assertFn: assert.False, + expectedResult: map[string]any{ + "ReferLookupId": ptr.To(10), + }, + }, + { + name: "lookup column, multiple values", + additionalData: map[string]any{ + "Refers": []any{ + map[string]any{ + "LookupId": ptr.To(float64(10)), + "LookupValue": ptr.To("item-1"), + }, + map[string]any{ + "LookupId": ptr.To(float64(11)), + "LookupValue": ptr.To("item-1"), + }, + }, + }, + colDetails: map[string]*columnDetails{ + "Refers": { + isLookupColumn: true, + isMultipleEnabled: true, + }, + }, + assertFn: assert.True, + expectedResult: map[string]any{ + "RefersLookupId": []float64{10, 11}, + "RefersLookupId@odata.type": "Collection(Edm.Int32)", + }, + }, } for _, test := range tests { From b770d0269d98319cfa9966a27bcb38a815a886d2 Mon Sep 17 00:00:00 2001 From: HiteshRepo Date: Wed, 24 Jan 2024 19:13:03 +0530 Subject: [PATCH 04/14] fix test TestColumnDefinitionable_LegacyColumns --- src/pkg/services/m365/api/lists_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/pkg/services/m365/api/lists_test.go b/src/pkg/services/m365/api/lists_test.go index 64dc0f1ecc..37cba33ac1 100644 --- a/src/pkg/services/m365/api/lists_test.go +++ b/src/pkg/services/m365/api/lists_test.go @@ -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: {}, }, }, } From 43c678d4b957311ecc224607ed25c7cfa560bc01 Mon Sep 17 00:00:00 2001 From: HiteshRepo Date: Thu, 25 Jan 2024 11:30:38 +0530 Subject: [PATCH 05/14] updates function name that sets multiple enabled by data type --- src/pkg/services/m365/api/lists.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/pkg/services/m365/api/lists.go b/src/pkg/services/m365/api/lists.go index cb9efe4ada..d398970d6f 100644 --- a/src/pkg/services/m365/api/lists.go +++ b/src/pkg/services/m365/api/lists.go @@ -487,8 +487,7 @@ func setAdditionalDataByColumnNames( for colName, colDetails := range columnNames { if val, ok := fieldData[colName]; ok { - setMultipleEnabled(val, colDetails) - + setMultipleEnabledByFieldData(val, colDetails) filteredData[colName] = fieldData[colName] } @@ -498,12 +497,19 @@ func setAdditionalDataByColumnNames( return filteredData } -func setMultipleEnabled(val any, colDetails *columnDetails) { +func setMultipleEnabledByFieldData(val any, colDetails *columnDetails) { + // 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 reflect.TypeOf(val).Kind() == reflect.Slice { colDetails.isMultipleEnabled = true } } +// 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) { if colDetails.isMultipleEnabled { filteredData[colName+ODataTypeFieldNamePart] = ODataTypeFieldNameStringVal From 162d32efd479226ec6777f736bcb373f8af7f133 Mon Sep 17 00:00:00 2001 From: HiteshRepo Date: Thu, 25 Jan 2024 11:30:38 +0530 Subject: [PATCH 06/14] updates function name that sets multiple enabled by data type --- src/pkg/services/m365/api/lists.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/pkg/services/m365/api/lists.go b/src/pkg/services/m365/api/lists.go index cb9efe4ada..3cfb3ad7e6 100644 --- a/src/pkg/services/m365/api/lists.go +++ b/src/pkg/services/m365/api/lists.go @@ -487,7 +487,7 @@ func setAdditionalDataByColumnNames( for colName, colDetails := range columnNames { if val, ok := fieldData[colName]; ok { - setMultipleEnabled(val, colDetails) + setMultipleEnabledByFieldData(val, colDetails) filteredData[colName] = fieldData[colName] } @@ -498,12 +498,19 @@ func setAdditionalDataByColumnNames( return filteredData } -func setMultipleEnabled(val any, colDetails *columnDetails) { +func setMultipleEnabledByFieldData(val any, colDetails *columnDetails) { + // 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 reflect.TypeOf(val).Kind() == reflect.Slice { colDetails.isMultipleEnabled = true } } +// 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) { if colDetails.isMultipleEnabled { filteredData[colName+ODataTypeFieldNamePart] = ODataTypeFieldNameStringVal From e2f2732fa88455eaea6b9a16e96f677b24e8b763 Mon Sep 17 00:00:00 2001 From: HiteshRepo Date: Thu, 25 Jan 2024 16:59:08 +0530 Subject: [PATCH 07/14] skips setting odatatype for text columns --- src/pkg/services/m365/api/lists.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/pkg/services/m365/api/lists.go b/src/pkg/services/m365/api/lists.go index 3cfb3ad7e6..4b297291a6 100644 --- a/src/pkg/services/m365/api/lists.go +++ b/src/pkg/services/m365/api/lists.go @@ -20,7 +20,8 @@ import ( var ErrSkippableListTemplate = clues.New("unable to create lists with skippable templates") type columnDetails struct { - isMultipleEnabled bool + isMultipleEnabled bool + hasDefaultedToText bool } // --------------------------------------------------------------------------- @@ -379,6 +380,8 @@ func setColumnType( orig models.ColumnDefinitionable, columnNames map[string]*columnDetails, ) { + colDetails := &columnDetails{} + switch { case orig.GetText() != nil: newColumn.SetText(orig.GetText()) @@ -409,10 +412,11 @@ func setColumnType( case orig.GetPersonOrGroup() != nil: newColumn.SetPersonOrGroup(orig.GetPersonOrGroup()) default: + colDetails.hasDefaultedToText = true newColumn.SetText(models.NewTextColumn()) } - columnNames[ptr.Val(newColumn.GetName())] = &columnDetails{} + columnNames[ptr.Val(newColumn.GetName())] = colDetails } // CloneListItem creates a new `SharePoint.ListItem` and stores the original item's @@ -512,6 +516,13 @@ func setMultipleEnabledByFieldData(val any, colDetails *columnDetails) { // 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 } From 98b7e5e6d839aa38b7cea3b3849d2fe8b048cbc7 Mon Sep 17 00:00:00 2001 From: HiteshRepo Date: Thu, 25 Jan 2024 17:10:32 +0530 Subject: [PATCH 08/14] fix lint --- src/pkg/services/m365/api/lists.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/pkg/services/m365/api/lists.go b/src/pkg/services/m365/api/lists.go index 4b297291a6..668a551e72 100644 --- a/src/pkg/services/m365/api/lists.go +++ b/src/pkg/services/m365/api/lists.go @@ -413,6 +413,7 @@ func setColumnType( newColumn.SetPersonOrGroup(orig.GetPersonOrGroup()) default: colDetails.hasDefaultedToText = true + newColumn.SetText(models.NewTextColumn()) } @@ -503,7 +504,7 @@ func setAdditionalDataByColumnNames( } func setMultipleEnabledByFieldData(val any, colDetails *columnDetails) { - // for columns like choice, even though it has an option to hold single/multiple values, + // 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 reflect.TypeOf(val).Kind() == reflect.Slice { From 01a524fad31085c4414400e15c487beeb94c8f59 Mon Sep 17 00:00:00 2001 From: HiteshRepo Date: Thu, 25 Jan 2024 18:41:19 +0530 Subject: [PATCH 09/14] simplifies logic to distinguish namings --- src/pkg/services/m365/api/lists.go | 70 ++++++++++---------- src/pkg/services/m365/api/lists_test.go | 85 +++++++++++++++++++------ 2 files changed, 102 insertions(+), 53 deletions(-) diff --git a/src/pkg/services/m365/api/lists.go b/src/pkg/services/m365/api/lists.go index 3988b6492e..0010a07413 100644 --- a/src/pkg/services/m365/api/lists.go +++ b/src/pkg/services/m365/api/lists.go @@ -20,6 +20,8 @@ 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 @@ -301,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 ( @@ -381,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 for example '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 is 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()) @@ -412,7 +427,17 @@ func setColumnType( newColumn.SetTerm(orig.GetTerm()) case orig.GetPersonOrGroup() != nil: colDetails.isPersonColumn = true - colDetails.isMultipleEnabled = ptr.Val(orig.GetPersonOrGroup().GetAllowMultipleSelection()) + isMultipleEnabled := ptr.Val(orig.GetPersonOrGroup().GetAllowMultipleSelection()) + colDetails.isMultipleEnabled = isMultipleEnabled + + if isMultipleEnabled { + colDetails.createFieldName = colName + LookupIDFieldNamePart + } else { + updatedName := colName + LookupIDFieldNamePart + colDetails.getFieldName = updatedName + colDetails.createFieldName = updatedName + } + newColumn.SetPersonOrGroup(orig.GetPersonOrGroup()) default: colDetails.hasDefaultedToText = true @@ -420,7 +445,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 @@ -493,37 +518,20 @@ func setAdditionalDataByColumnNames( fieldData := orig.GetAdditionalData() filteredData := make(map[string]any) - // for certain columns like 'person', the column name is for example '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 is function helps us to determine which name to use while accessing stored data - nameForGet := func(isMultipleEnabled bool, colName, updatedName string) string { - if isMultipleEnabled { - return colName - } - - return updatedName - } - - for colName, colDetails := range columnNames { - updatedColName := updateColName(colName, colDetails) - getColName := nameForGet(colDetails.isMultipleEnabled, colName, updatedColName) - - if val, ok := fieldData[getColName]; ok { + for _, colDetails := range columnNames { + if val, ok := fieldData[colDetails.getFieldName]; ok { setMultipleEnabledByFieldData(val, colDetails) - filteredData[updatedColName] = val - populateMultipleValues(val, filteredData, updatedColName, colDetails) + filteredData[colDetails.createFieldName] = val + populateMultipleValues(val, filteredData, colDetails) } - specifyODataType(filteredData, colDetails, updatedColName) + specifyODataType(filteredData, colDetails, colDetails.createFieldName) } return filteredData } -func populateMultipleValues(val any, filteredData map[string]any, colName string, colDetails *columnDetails) { +func populateMultipleValues(val any, filteredData map[string]any, colDetails *columnDetails) { if !colDetails.isMultipleEnabled { return } @@ -551,7 +559,7 @@ func populateMultipleValues(val any, filteredData map[string]any, colName string } } - filteredData[colName] = lookupIDs + filteredData[colDetails.createFieldName] = lookupIDs } func setMultipleEnabledByFieldData(val any, colDetails *columnDetails) { @@ -569,14 +577,6 @@ func setMultipleEnabledByFieldData(val any, colDetails *columnDetails) { } } -func updateColName(colName string, colDetails *columnDetails) string { - if !colDetails.isPersonColumn { - return colName - } - - return colName + LookupIDFieldNamePart -} - // 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' diff --git a/src/pkg/services/m365/api/lists_test.go b/src/pkg/services/m365/api/lists_test.go index 834d7a4dc4..14b56143ee 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)) }) } } @@ -393,7 +421,10 @@ func (suite *ListsUnitSuite) TestColumnDefinitionable_LegacyColumns() { length: 1, expectedColNames: map[string]*columnDetails{ TitleColumnName: {}, - textColumnName: {}, + textColumnName: { + getFieldName: textColumnName, + createFieldName: textColumnName, + }, }, }, } @@ -442,7 +473,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 +544,10 @@ func (suite *ListsUnitSuite) TestFieldValueSetable_Location() { origFs.SetAdditionalData(additionalData) colNames := map[string]*columnDetails{ - "MyAddress": {}, + "MyAddress": { + getFieldName: "MyAddress", + createFieldName: "MyAddress", + }, } fs := retrieveFieldData(origFs, colNames) @@ -719,7 +756,10 @@ func (suite *ListsUnitSuite) TestSetAdditionalDataByColumnNames() { "choice": ptr.To("good"), }, colDetails: map[string]*columnDetails{ - "choice": {}, + "choice": { + getFieldName: "choice", + createFieldName: "choice", + }, }, assertFn: assert.False, expectedResult: map[string]any{ @@ -735,7 +775,10 @@ func (suite *ListsUnitSuite) TestSetAdditionalDataByColumnNames() { }, }, colDetails: map[string]*columnDetails{ - "choice": {}, + "choice": { + getFieldName: "choice", + createFieldName: "choice", + }, }, assertFn: assert.True, expectedResult: map[string]any{ @@ -752,7 +795,11 @@ func (suite *ListsUnitSuite) TestSetAdditionalDataByColumnNames() { "PersonsLookupId": ptr.To(10), }, colDetails: map[string]*columnDetails{ - "Persons": {isPersonColumn: true}, + "Persons": { + isPersonColumn: true, + getFieldName: "PersonsLookupId", + createFieldName: "PersonsLookupId", + }, }, assertFn: assert.False, expectedResult: map[string]any{ @@ -779,6 +826,8 @@ func (suite *ListsUnitSuite) TestSetAdditionalDataByColumnNames() { "Persons": { isPersonColumn: true, isMultipleEnabled: true, + getFieldName: "Persons", + createFieldName: "PersonsLookupId", }, }, assertFn: assert.True, From aac0d39e9833c758670639f62f92664afbff9608 Mon Sep 17 00:00:00 2001 From: HiteshRepo Date: Thu, 25 Jan 2024 19:34:50 +0530 Subject: [PATCH 10/14] fix test TestColumnDefinitionable_LegacyColumns --- src/pkg/services/m365/api/lists_test.go | 26 ++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/pkg/services/m365/api/lists_test.go b/src/pkg/services/m365/api/lists_test.go index 14b56143ee..2b07dbe712 100644 --- a/src/pkg/services/m365/api/lists_test.go +++ b/src/pkg/services/m365/api/lists_test.go @@ -373,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", @@ -388,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", @@ -403,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", @@ -420,7 +429,10 @@ func (suite *ListsUnitSuite) TestColumnDefinitionable_LegacyColumns() { }, length: 1, expectedColNames: map[string]*columnDetails{ - TitleColumnName: {}, + TitleColumnName: { + getFieldName: TitleColumnName, + createFieldName: TitleColumnName, + }, textColumnName: { getFieldName: textColumnName, createFieldName: textColumnName, From 0c7572e8f4972eb43eef6997d567eed825d1a30e Mon Sep 17 00:00:00 2001 From: HiteshRepo Date: Tue, 30 Jan 2024 00:24:59 +0530 Subject: [PATCH 11/14] updates comment for colDetails names --- src/pkg/services/m365/api/lists.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pkg/services/m365/api/lists.go b/src/pkg/services/m365/api/lists.go index 0010a07413..c95be12976 100644 --- a/src/pkg/services/m365/api/lists.go +++ b/src/pkg/services/m365/api/lists.go @@ -389,12 +389,12 @@ func setColumnType( colName := ptr.Val(newColumn.GetName()) colDetails := &columnDetails{} - // for certain columns like 'person', the column name is for example 'personName' + // 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 is function helps us to determine which name to use while accessing stored data + // Hence this function helps us to determine which name to use while accessing stored data colDetails.getFieldName = colName colDetails.createFieldName = colName From 20cee12cd7349d56dbd73d357d5915ae5358a756 Mon Sep 17 00:00:00 2001 From: HiteshRepo Date: Tue, 30 Jan 2024 00:26:41 +0530 Subject: [PATCH 12/14] fixes nit --- src/pkg/services/m365/api/lists.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pkg/services/m365/api/lists.go b/src/pkg/services/m365/api/lists.go index 4437bdd9ab..6daeef3c7d 100644 --- a/src/pkg/services/m365/api/lists.go +++ b/src/pkg/services/m365/api/lists.go @@ -620,7 +620,7 @@ func specifyODataType(filteredData map[string]any, colDetails *columnDetails, co } switch { - case colDetails.isPersonColumn || colDetails.isLookupColumn: + case colDetails.isPersonColumn, colDetails.isLookupColumn: filteredData[colName+ODataTypeFieldNamePart] = ODataTypeFieldNameIntVal default: filteredData[colName+ODataTypeFieldNamePart] = ODataTypeFieldNameStringVal From 55aed0de282f9a492c10374f4531af4287fd78a3 Mon Sep 17 00:00:00 2001 From: HiteshRepo Date: Tue, 30 Jan 2024 01:13:55 +0530 Subject: [PATCH 13/14] address review comment --- src/pkg/services/m365/api/lists.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/pkg/services/m365/api/lists.go b/src/pkg/services/m365/api/lists.go index 668a551e72..9dc26a783c 100644 --- a/src/pkg/services/m365/api/lists.go +++ b/src/pkg/services/m365/api/lists.go @@ -492,7 +492,12 @@ func setAdditionalDataByColumnNames( for colName, colDetails := range columnNames { if val, ok := fieldData[colName]; ok { - setMultipleEnabledByFieldData(val, colDetails) + // 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] } @@ -503,15 +508,6 @@ func setAdditionalDataByColumnNames( return filteredData } -func setMultipleEnabledByFieldData(val any, colDetails *columnDetails) { - // 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 reflect.TypeOf(val).Kind() == reflect.Slice { - colDetails.isMultipleEnabled = true - } -} - // 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' @@ -679,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 +} From 75a13fb94a25a233889dd50c4c10e7549e722c2f Mon Sep 17 00:00:00 2001 From: HiteshRepo Date: Tue, 30 Jan 2024 01:50:17 +0530 Subject: [PATCH 14/14] fix type cast --- src/pkg/services/m365/api/lists.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/pkg/services/m365/api/lists.go b/src/pkg/services/m365/api/lists.go index 21ced6a539..35843a106d 100644 --- a/src/pkg/services/m365/api/lists.go +++ b/src/pkg/services/m365/api/lists.go @@ -12,7 +12,6 @@ import ( "github.com/alcionai/corso/src/internal/common/keys" "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/common/str" - "github.com/alcionai/corso/src/internal/common/tform" "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/services/m365/api/graph" @@ -559,12 +558,12 @@ func populateMultipleValues(val any, filteredData map[string]any, colDetails *co continue } - v, err := tform.AnyValueToT[float64](LookupIDKey, md) - if err != nil { + v, ok := md[LookupIDKey].(*float64) + if !ok { continue } - lookupIDs = append(lookupIDs, v) + lookupIDs = append(lookupIDs, ptr.Val(v)) } filteredData[colDetails.createFieldName] = lookupIDs