Skip to content
This repository has been archived by the owner on Oct 11, 2024. It is now read-only.

Commit

Permalink
provisions to identify fields with multiple value (#5109)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
HiteshRepo authored Jan 29, 2024
1 parent 6ef2c2d commit 8ac7e6c
Show file tree
Hide file tree
Showing 3 changed files with 149 additions and 29 deletions.
3 changes: 3 additions & 0 deletions src/pkg/services/m365/api/consts.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ const (
ChildCountFieldNamePart = "ChildCount"
LookupIDFieldNamePart = "LookupId"

ODataTypeFieldNamePart = "@odata.type"
ODataTypeFieldNameStringVal = "Collection(Edm.String)"

ReadOnlyOrHiddenFieldNamePrefix = "_"
DescoratorFieldNamePrefix = "@"

Expand Down
72 changes: 60 additions & 12 deletions src/pkg/services/m365/api/lists.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package api
import (
"context"
"fmt"
"reflect"
"strings"

"github.com/alcionai/clues"
Expand All @@ -18,6 +19,11 @@ import (

var ErrSkippableListTemplate = clues.New("unable to create lists with skippable templates")

type columnDetails struct {
isMultipleEnabled bool
hasDefaultedToText bool
}

// ---------------------------------------------------------------------------
// controller
// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -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())
Expand All @@ -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 (
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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()
Expand All @@ -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())
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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 '<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) {
for k, v := range additionalData {
nestedFields, ok := v.(map[string]any)
Expand Down Expand Up @@ -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
}
103 changes: 86 additions & 17 deletions src/pkg/services/m365/api/lists_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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: {},
},
},
}
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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": "[email protected]",
},
{
"LookupId": 11,
"LookupValue": "Who2",
"Email": "[email protected]",
},
},
},
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
Expand Down

0 comments on commit 8ac7e6c

Please sign in to comment.