provisions to identify fields with multiple value (#5109)

provisions to identify fields with multiple value.
Some fields can hold multiple values and single value based on a property called `allowMultipleValues`.
For example, `Lookup` column:
```
"lookup": {
    "allowMultipleValues": true,
    "allowUnlimitedLength": false,
    "columnName": "Title",
    "listId": "21b45bf2-e495-4582-b114-839577ff8e4f"
}
```
But `choice` columns, even though allows to set multiple choices/value, does not have that particular field `allowMultipleValues` to indicate.
So in this PR we are trying determine the same by the stored values while restoring.

**Original list with `choice` column in site**:
![Choice-List-Multi](https://github.com/alcionai/corso/assets/48874082/d4457b3c-0230-4a69-8467-f64b7d7d4f04)

**Restored list with `choice` column in site**
![Restored-Choice-List-Multi](https://github.com/alcionai/corso/assets/48874082/78478055-0e84-43d0-ac83-262b564ce778)
The color does not come through though


#### 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 

#### Test Plan

<!-- How will this be tested prior to merging.-->
- [x] 💪 Manual
- [x]  Unit test
- [x] 💚 E2E
This commit is contained in:
Hitesh Pattanayak 2024-01-30 02:57:35 +05:30 committed by GitHub
parent 6ef2c2d494
commit 8ac7e6caa2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 149 additions and 29 deletions

View File

@ -61,6 +61,9 @@ const (
ChildCountFieldNamePart = "ChildCount" ChildCountFieldNamePart = "ChildCount"
LookupIDFieldNamePart = "LookupId" LookupIDFieldNamePart = "LookupId"
ODataTypeFieldNamePart = "@odata.type"
ODataTypeFieldNameStringVal = "Collection(Edm.String)"
ReadOnlyOrHiddenFieldNamePrefix = "_" ReadOnlyOrHiddenFieldNamePrefix = "_"
DescoratorFieldNamePrefix = "@" DescoratorFieldNamePrefix = "@"

View File

@ -3,6 +3,7 @@ package api
import ( import (
"context" "context"
"fmt" "fmt"
"reflect"
"strings" "strings"
"github.com/alcionai/clues" "github.com/alcionai/clues"
@ -18,6 +19,11 @@ import (
var ErrSkippableListTemplate = clues.New("unable to create lists with skippable templates") var ErrSkippableListTemplate = clues.New("unable to create lists with skippable templates")
type columnDetails struct {
isMultipleEnabled bool
hasDefaultedToText bool
}
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
// controller // controller
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
@ -277,7 +283,7 @@ func BytesToListable(bytes []byte) (models.Listable, error) {
// not attached in this method. // not attached in this method.
// ListItems are not included in creation of new list, and have to be restored // ListItems are not included in creation of new list, and have to be restored
// in separate call. // 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 := models.NewList()
newList.SetContentTypes(orig.GetContentTypes()) newList.SetContentTypes(orig.GetContentTypes())
@ -294,7 +300,7 @@ func ToListable(orig models.Listable, listName string) (models.Listable, map[str
newList.SetParentReference(orig.GetParentReference()) newList.SetParentReference(orig.GetParentReference())
columns := make([]models.ColumnDefinitionable, 0) columns := make([]models.ColumnDefinitionable, 0)
columnNames := map[string]any{TitleColumnName: nil} columnNames := map[string]*columnDetails{TitleColumnName: {}}
for _, cd := range orig.GetColumns() { for _, cd := range orig.GetColumns() {
var ( var (
@ -316,8 +322,7 @@ func ToListable(orig models.Listable, listName string) (models.Listable, map[str
continue continue
} }
columns = append(columns, cloneColumnDefinitionable(cd)) columns = append(columns, cloneColumnDefinitionable(cd, columnNames))
columnNames[ptr.Val(cd.GetName())] = nil
} }
newList.SetColumns(columns) 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 // cloneColumnDefinitionable utility function for encapsulating models.ColumnDefinitionable data
// into new object for upload. // 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() newColumn := models.NewColumnDefinition()
// column attributes // column attributes
@ -351,7 +359,7 @@ func cloneColumnDefinitionable(orig models.ColumnDefinitionable) models.ColumnDe
newColumn.SetEnforceUniqueValues(orig.GetEnforceUniqueValues()) newColumn.SetEnforceUniqueValues(orig.GetEnforceUniqueValues())
// column types // column types
setColumnType(newColumn, orig) setColumnType(newColumn, orig, columnNames)
// Requires nil checks to avoid Graph error: 'General exception while processing' // Requires nil checks to avoid Graph error: 'General exception while processing'
defaultValue := orig.GetDefaultValue() defaultValue := orig.GetDefaultValue()
@ -367,7 +375,13 @@ func cloneColumnDefinitionable(orig models.ColumnDefinitionable) models.ColumnDe
return newColumn 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 { switch {
case orig.GetText() != nil: case orig.GetText() != nil:
newColumn.SetText(orig.GetText()) newColumn.SetText(orig.GetText())
@ -398,14 +412,18 @@ func setColumnType(newColumn *models.ColumnDefinition, orig models.ColumnDefinit
case orig.GetPersonOrGroup() != nil: case orig.GetPersonOrGroup() != nil:
newColumn.SetPersonOrGroup(orig.GetPersonOrGroup()) newColumn.SetPersonOrGroup(orig.GetPersonOrGroup())
default: default:
colDetails.hasDefaultedToText = true
newColumn.SetText(models.NewTextColumn()) newColumn.SetText(models.NewTextColumn())
} }
columnNames[ptr.Val(newColumn.GetName())] = colDetails
} }
// CloneListItem creates a new `SharePoint.ListItem` and stores the original item's // CloneListItem creates a new `SharePoint.ListItem` and stores the original item's
// M365 data into it set fields. // M365 data into it set fields.
// - https://learn.microsoft.com/en-us/graph/api/resources/listitem?view=graph-rest-1.0 // - 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() newItem := models.NewListItem()
// list item data // list item data
@ -442,7 +460,7 @@ func CloneListItem(orig models.ListItemable, columnNames map[string]any) models.
// additionalData map // additionalData map
// Further documentation on FieldValueSets: // Further documentation on FieldValueSets:
// - https://learn.microsoft.com/en-us/graph/api/resources/fieldvalueset?view=graph-rest-1.0 // - 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() fields := models.NewFieldValueSet()
additionalData := setAdditionalDataByColumnNames(orig, columnNames) additionalData := setAdditionalDataByColumnNames(orig, columnNames)
@ -463,7 +481,7 @@ func retrieveFieldData(orig models.FieldValueSetable, columnNames map[string]any
func setAdditionalDataByColumnNames( func setAdditionalDataByColumnNames(
orig models.FieldValueSetable, orig models.FieldValueSetable,
columnNames map[string]any, columnNames map[string]*columnDetails,
) map[string]any { ) map[string]any {
if orig == nil { if orig == nil {
return make(map[string]any) return make(map[string]any)
@ -472,15 +490,41 @@ func setAdditionalDataByColumnNames(
fieldData := orig.GetAdditionalData() fieldData := orig.GetAdditionalData()
filteredData := make(map[string]any) filteredData := make(map[string]any)
for colName := range columnNames { for colName, colDetails := range columnNames {
if _, ok := fieldData[colName]; ok { 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] filteredData[colName] = fieldData[colName]
} }
specifyODataType(filteredData, colDetails, colName)
} }
return filteredData 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 '<columnName>@@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) { func hasAddressFields(additionalData map[string]any) (map[string]any, string, bool) {
for k, v := range additionalData { for k, v := range additionalData {
nestedFields, ok := v.(map[string]any) nestedFields, ok := v.(map[string]any)
@ -631,3 +675,7 @@ func ListCollisionKey(list models.Listable) string {
return ptr.Val(list.GetDisplayName()) return ptr.Val(list.GetDisplayName())
} }
func isSlice(val any) bool {
return reflect.TypeOf(val).Kind() == reflect.Slice
}

View File

@ -162,12 +162,12 @@ func (suite *ListsUnitSuite) TestColumnDefinitionable_GetValidation() {
for _, test := range tests { for _, test := range tests {
suite.Run(test.name, func() { suite.Run(test.name, func() {
t := suite.T() t := suite.T()
colNames := map[string]*columnDetails{}
orig := test.getOrig() orig := test.getOrig()
newCd := cloneColumnDefinitionable(orig) newCd := cloneColumnDefinitionable(orig, colNames)
require.NotEmpty(t, newCd) require.NotEmpty(t, newCd)
test.expect(t, newCd.GetValidation()) test.expect(t, newCd.GetValidation())
}) })
} }
@ -219,9 +219,10 @@ func (suite *ListsUnitSuite) TestColumnDefinitionable_GetDefaultValue() {
for _, test := range tests { for _, test := range tests {
suite.Run(test.name, func() { suite.Run(test.name, func() {
t := suite.T() t := suite.T()
colNames := map[string]*columnDetails{}
orig := test.getOrig() orig := test.getOrig()
newCd := cloneColumnDefinitionable(orig) newCd := cloneColumnDefinitionable(orig, colNames)
require.NotEmpty(t, newCd) require.NotEmpty(t, newCd)
test.expect(t, newCd) test.expect(t, newCd)
@ -277,9 +278,8 @@ func (suite *ListsUnitSuite) TestColumnDefinitionable_ColumnType() {
for _, test := range tests { for _, test := range tests {
suite.Run(test.name, func() { suite.Run(test.name, func() {
t := suite.T() t := suite.T()
colNames := map[string]*columnDetails{}
orig := test.getOrig() newCd := cloneColumnDefinitionable(test.getOrig(), colNames)
newCd := cloneColumnDefinitionable(orig)
require.NotEmpty(t, newCd) require.NotEmpty(t, newCd)
assert.True(t, test.checkFn(newCd)) assert.True(t, test.checkFn(newCd))
@ -332,7 +332,7 @@ func (suite *ListsUnitSuite) TestColumnDefinitionable_LegacyColumns() {
name string name string
getList func() *models.List getList func() *models.List
length int length int
expectedColNames map[string]any expectedColNames map[string]*columnDetails
}{ }{
{ {
name: "all legacy columns", name: "all legacy columns",
@ -346,7 +346,7 @@ func (suite *ListsUnitSuite) TestColumnDefinitionable_LegacyColumns() {
return lst return lst
}, },
length: 0, length: 0,
expectedColNames: map[string]any{TitleColumnName: nil}, expectedColNames: map[string]*columnDetails{TitleColumnName: {}},
}, },
{ {
name: "title and legacy columns", name: "title and legacy columns",
@ -361,7 +361,7 @@ func (suite *ListsUnitSuite) TestColumnDefinitionable_LegacyColumns() {
return lst return lst
}, },
length: 0, length: 0,
expectedColNames: map[string]any{TitleColumnName: nil}, expectedColNames: map[string]*columnDetails{TitleColumnName: {}},
}, },
{ {
name: "readonly and legacy columns", name: "readonly and legacy columns",
@ -376,7 +376,7 @@ func (suite *ListsUnitSuite) TestColumnDefinitionable_LegacyColumns() {
return lst return lst
}, },
length: 0, length: 0,
expectedColNames: map[string]any{TitleColumnName: nil}, expectedColNames: map[string]*columnDetails{TitleColumnName: {}},
}, },
{ {
name: "legacy and a text column", name: "legacy and a text column",
@ -391,9 +391,9 @@ func (suite *ListsUnitSuite) TestColumnDefinitionable_LegacyColumns() {
return lst return lst
}, },
length: 1, length: 1,
expectedColNames: map[string]any{ expectedColNames: map[string]*columnDetails{
TitleColumnName: nil, TitleColumnName: {},
textColumnName: nil, textColumnName: {},
}, },
}, },
} }
@ -432,7 +432,7 @@ func (suite *ListsUnitSuite) TestFieldValueSetable() {
origFs := models.NewFieldValueSet() origFs := models.NewFieldValueSet()
origFs.SetAdditionalData(additionalData) origFs.SetAdditionalData(additionalData)
colNames := map[string]any{} colNames := map[string]*columnDetails{}
fs := retrieveFieldData(origFs, colNames) fs := retrieveFieldData(origFs, colNames)
fsAdditionalData := fs.GetAdditionalData() fsAdditionalData := fs.GetAdditionalData()
@ -442,7 +442,7 @@ func (suite *ListsUnitSuite) TestFieldValueSetable() {
origFs = models.NewFieldValueSet() origFs = models.NewFieldValueSet()
origFs.SetAdditionalData(additionalData) origFs.SetAdditionalData(additionalData)
colNames["itemName"] = struct{}{} colNames["itemName"] = &columnDetails{}
fs = retrieveFieldData(origFs, colNames) fs = retrieveFieldData(origFs, colNames)
fsAdditionalData = fs.GetAdditionalData() fsAdditionalData = fs.GetAdditionalData()
@ -509,8 +509,8 @@ func (suite *ListsUnitSuite) TestFieldValueSetable_Location() {
origFs := models.NewFieldValueSet() origFs := models.NewFieldValueSet()
origFs.SetAdditionalData(additionalData) origFs.SetAdditionalData(additionalData)
colNames := map[string]any{ colNames := map[string]*columnDetails{
"MyAddress": nil, "MyAddress": {},
} }
fs := retrieveFieldData(origFs, colNames) 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 { type ListsAPIIntgSuite struct {
tester.Suite tester.Suite
its intgTesterSetup its intgTesterSetup