handles multiple persons list items (#5111)

handles multiple persons list items.

**Original `person` list with single value**:
![Person-List](https://github.com/alcionai/corso/assets/48874082/a4a87cde-f907-4fc7-94da-f9ddda0f5a18)
 
**Restored `person` list with single value**:
![Restored-Person-List](https://github.com/alcionai/corso/assets/48874082/6b5c2a8b-743c-4020-9393-356d28948bf0)

**Original `person` list with multi value**:
![Person-List-Multi](https://github.com/alcionai/corso/assets/48874082/18d2c536-67ac-4b28-87be-2352764f2c95)

**Restored `person` list with multi value**:
![Restored-Person-List-Multi](https://github.com/alcionai/corso/assets/48874082/f9694e0d-d2cc-48d9-94f2-16b61c5b7cdb)

#### Does this PR need a docs update or release note?

- [ ] ✅ Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [x] ⛔ No

#### Type of change

<!--- Please check the type of change your PR introduces: --->
- [x] 🌻 Feature
- [ ] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Supportability/Tests
- [ ] 💻 CI/Deployment
- [ ] 🧹 Tech Debt/Cleanup

#### Issue(s)
#5108 
#5084 

#### Test Plan

<!-- How will this be tested prior to merging.-->
- [x] 💪 Manual
- [x] ⚡ Unit test
- [x] 💚 E2E

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       = "@"

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 {
+	for _, colDetails := range columnNames {
-		if val, ok := fieldData[colName]; ok {
+		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 '<columnName>@@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
 	}
 }

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 {
+			checkFn: func(cd models.ColumnDefinitionable, colNames map[string]*columnDetails) bool {
-				return cd.GetNumber() != nil
+				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 {
+			checkFn: func(cd models.ColumnDefinitionable, colNames map[string]*columnDetails) bool {
-				return cd.GetPersonOrGroup() != nil
+				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) bool {
+			checkFn: func(cd models.ColumnDefinitionable, colNames map[string]*columnDetails) bool {
-				return cd.GetText() != nil
+				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, 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,
+			length: 0,
-			expectedColNames: map[string]*columnDetails{TitleColumnName: {}},
+			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,
+			length: 0,
-			expectedColNames: map[string]*columnDetails{TitleColumnName: {}},
+			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,
+			length: 0,
-			expectedColNames: map[string]*columnDetails{TitleColumnName: {}},
+			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: {},
+				TitleColumnName: {
-				textColumnName:  {},
+					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{
+				"Persons": []any{
-					{
+					map[string]any{
-						"LookupId":    10,
+						"LookupId":    ptr.To(float64(10)),
-						"LookupValue": "Who1",
+						"LookupValue": ptr.To("Who1"),
-						"Email":       "Who1@10rqc2.onmicrosoft.com",
+						"Email":       ptr.To("Who1@10rqc2.onmicrosoft.com"),
 					},
-					{
+					map[string]any{
-						"LookupId":    11,
+						"LookupId":    ptr.To(float64(11)),
-						"LookupValue": "Who2",
+						"LookupValue": ptr.To("Who2"),
-						"Email":       "Who2@10rqc2.onmicrosoft.com",
+						"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)
+			res := setAdditionalDataByColumnNames(origFs, test.colDetails)
-			test.assertFn(t, colNames[test.colName].isMultipleEnabled)
+			assert.Equal(t, test.expectedResult, res)
 		})
 	}
 }