Skip to content

Commit

Permalink
handles multiple lookup field values (#5112)
Browse files Browse the repository at this point in the history
handles multiple lookup field values.

**Original `Lookup` list with single value**:
![Lookup-List-Single](https://github.com/alcionai/corso/assets/48874082/6a6b68cf-8fb9-4dfb-985e-702c4d74d3f0)

**Restored `Lookup` list with single value**:
![Restored-Lookup-List](https://github.com/alcionai/corso/assets/48874082/f97ac974-6a3b-4dd2-82c5-9f3596f9adaa)

**Original `Lookup` list with multiple values**:
![Lookup-List-Multi](https://github.com/alcionai/corso/assets/48874082/5f8b1b92-297f-4a66-b0b6-b5007d430690)

**Restored `Lookup` list with multiple values**:
![Restored-Lookup-List-Multi](https://github.com/alcionai/corso/assets/48874082/6c6d79ca-775d-4f50-abee-8090a28f3871)


#### 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
  • Loading branch information
HiteshRepo committed Jan 30, 2024
1 parent 734fd72 commit 08d4803
Show file tree
Hide file tree
Showing 2 changed files with 134 additions and 4 deletions.
30 changes: 26 additions & 4 deletions src/pkg/services/m365/api/lists.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ type columnDetails struct {
createFieldName string
getFieldName string
isPersonColumn bool
isLookupColumn bool
isMultipleEnabled bool
hasDefaultedToText bool
}
Expand Down Expand Up @@ -420,6 +421,16 @@ func setColumnType(
case orig.GetNumber() != nil:
newColumn.SetNumber(orig.GetNumber())
case orig.GetLookup() != nil:
colDetails.isLookupColumn = true
isMultipleEnabled := ptr.Val(orig.GetLookup().GetAllowMultipleValues())
colDetails.isMultipleEnabled = isMultipleEnabled
updatedName := colName + LookupIDFieldNamePart
colDetails.createFieldName = updatedName

if !isMultipleEnabled {
colDetails.getFieldName = updatedName
}

newColumn.SetLookup(orig.GetLookup())
case orig.GetThumbnail() != nil:
newColumn.SetThumbnail(orig.GetThumbnail())
Expand Down Expand Up @@ -540,7 +551,8 @@ func populateMultipleValues(val any, filteredData map[string]any, colDetails *co
return
}

if !colDetails.isPersonColumn {
if !colDetails.isPersonColumn &&
!colDetails.isLookupColumn {
return
}

Expand All @@ -550,11 +562,10 @@ func populateMultipleValues(val any, filteredData map[string]any, colDetails *co
}

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...) {
if !ok || !keys.HasKeys(md, checkFields(colDetails)...) {
continue
}

Expand All @@ -569,6 +580,17 @@ func populateMultipleValues(val any, filteredData map[string]any, colDetails *co
filteredData[colDetails.createFieldName] = lookupIDs
}

func checkFields(colDetails *columnDetails) []string {
switch {
case colDetails.isLookupColumn:
return []string{LookupIDKey, LookupValueKey}
case colDetails.isPersonColumn:
return []string{LookupIDKey, LookupValueKey, PersonEmailKey}
default:
return []string{}
}
}

// 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'
Expand All @@ -587,7 +609,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
Expand Down
108 changes: 108 additions & 0 deletions src/pkg/services/m365/api/lists_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,41 @@ func (suite *ListsUnitSuite) TestColumnDefinitionable_ColumnType() {
colNames["pg-col"].createFieldName == "pg-colLookupId"
},
},
{
name: "lookup column, single value",
getOrig: func() models.ColumnDefinitionable {
lookupColumn := models.NewLookupColumn()

cd := models.NewColumnDefinition()
cd.SetLookup(lookupColumn)
cd.SetName(ptr.To("refer-col"))

return cd
},
checkFn: func(cd models.ColumnDefinitionable, colNames map[string]*columnDetails) bool {
return cd.GetLookup() != nil &&
colNames["refer-col"].getFieldName == "refer-colLookupId" &&
colNames["refer-col"].createFieldName == "refer-colLookupId"
},
},
{
name: "lookup column, multiple value",
getOrig: func() models.ColumnDefinitionable {
lookupColumn := models.NewLookupColumn()
lookupColumn.SetAllowMultipleValues(ptr.To(true))

cd := models.NewColumnDefinition()
cd.SetLookup(lookupColumn)
cd.SetName(ptr.To("refer-col"))

return cd
},
checkFn: func(cd models.ColumnDefinitionable, colNames map[string]*columnDetails) bool {
return cd.GetLookup() != nil &&
colNames["refer-col"].getFieldName == "refer-col" &&
colNames["refer-col"].createFieldName == "refer-colLookupId"
},
},
{
name: "defaulted to text column",
getOrig: func() models.ColumnDefinitionable {
Expand Down Expand Up @@ -848,6 +883,51 @@ 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,
getFieldName: "ReferLookupId",
createFieldName: "ReferLookupId",
},
},
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,
getFieldName: "Refers",
createFieldName: "RefersLookupId",
},
},
assertFn: assert.True,
expectedResult: map[string]any{
"RefersLookupId": []float64{10, 11},
"RefersLookupId@odata.type": "Collection(Edm.Int32)",
},
},
}

for _, test := range tests {
Expand All @@ -861,6 +941,34 @@ func (suite *ListsUnitSuite) TestSetAdditionalDataByColumnNames() {
}
}

func (suite *ListsUnitSuite) TestCheckFields() {
t := suite.T()

tests := []struct {
name string
colDetails *columnDetails
expectedKeys []string
}{
{
name: "lookup column",
colDetails: &columnDetails{isLookupColumn: true},
expectedKeys: []string{LookupIDKey, LookupValueKey},
},
{
name: "person column",
colDetails: &columnDetails{isPersonColumn: true},
expectedKeys: []string{LookupIDKey, LookupValueKey, PersonEmailKey},
},
}

for _, test := range tests {
suite.Run(test.name, func() {
res := checkFields(test.colDetails)
assert.Equal(t, test.expectedKeys, res)
})
}
}

type ListsAPIIntgSuite struct {
tester.Suite
its intgTesterSetup
Expand Down

0 comments on commit 08d4803

Please sign in to comment.