This repository has been archived by the owner on Oct 11, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 45
handles multiple lookup field values #5112
Merged
Merged
Changes from 26 commits
Commits
Show all changes
34 commits
Select commit
Hold shift + click to select a range
6faf18c
provisions to identify fields with multiple value
HiteshRepo fa4ea5b
handles multiple persons list items
HiteshRepo 26c957d
handles multiple lookup field values
HiteshRepo b770d02
fix test TestColumnDefinitionable_LegacyColumns
HiteshRepo 573686c
Merge remote-tracking branch 'origin/lists-multiple' into person-mult…
HiteshRepo 7c22a0a
Merge remote-tracking branch 'origin/person-multiple' into lookup-mul…
HiteshRepo 43c678d
updates function name that sets multiple enabled by data type
HiteshRepo 265ef59
resolves conflicts with lists-multiple
HiteshRepo 162d32e
updates function name that sets multiple enabled by data type
HiteshRepo ea7c988
fix merge conflicts
HiteshRepo feedafe
Merge remote-tracking branch 'origin/person-multiple' into lookup-mul…
HiteshRepo e2f2732
skips setting odatatype for text columns
HiteshRepo 5daae6f
merge conflicts with lists-multiple
HiteshRepo 3d77b56
resolve conflicts with person-multiple
HiteshRepo 98b7e5e
fix lint
HiteshRepo 00f1405
resolve conflicts with base
HiteshRepo 8cb30e6
Merge remote-tracking branch 'origin/person-multiple' into lookup-mul…
HiteshRepo 01a524f
simplifies logic to distinguish namings
HiteshRepo 9aa1ae7
resolve conflicts with base
HiteshRepo aac0d39
fix test TestColumnDefinitionable_LegacyColumns
HiteshRepo 9b8aa6c
Merge remote-tracking branch 'origin/person-multiple' into lookup-mul…
HiteshRepo abdf620
Merge remote-tracking branch 'origin/main' into lists-multiple
HiteshRepo 0c7572e
updates comment for colDetails names
HiteshRepo 5774003
Merge remote-tracking branch 'origin/lists-multiple' into person-mult…
HiteshRepo 20cee12
fixes nit
HiteshRepo fa4a6ca
Merge remote-tracking branch 'origin/person-multiple' into lookup-mul…
HiteshRepo 55aed0d
address review comment
HiteshRepo 193d1d3
address review comments
HiteshRepo 9177702
address review comments
HiteshRepo 75a13fb
fix type cast
HiteshRepo d250d72
Merge remote-tracking branch 'origin/person-multiple' into lookup-mul…
HiteshRepo 7346d54
resolve conflicts with main
HiteshRepo 65602ba
Merge remote-tracking branch 'origin/person-multiple' into lookup-mul…
HiteshRepo 0485c51
resolve merge conflicts with main
HiteshRepo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ package api | |
import ( | ||
"context" | ||
"fmt" | ||
"reflect" | ||
"strings" | ||
|
||
"github.com/alcionai/clues" | ||
|
@@ -18,6 +19,15 @@ import ( | |
|
||
var ErrSkippableListTemplate = clues.New("unable to create lists with skippable templates") | ||
|
||
type columnDetails struct { | ||
createFieldName string | ||
getFieldName string | ||
isPersonColumn bool | ||
isLookupColumn bool | ||
isMultipleEnabled bool | ||
hasDefaultedToText bool | ||
} | ||
|
||
// --------------------------------------------------------------------------- | ||
// controller | ||
// --------------------------------------------------------------------------- | ||
|
@@ -277,7 +287,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 +304,10 @@ 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: { | ||
getFieldName: TitleColumnName, | ||
createFieldName: TitleColumnName, | ||
}} | ||
|
||
for _, cd := range orig.GetColumns() { | ||
var ( | ||
|
@@ -316,8 +329,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 +339,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 +366,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 +382,23 @@ 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, | ||
) { | ||
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()) | ||
|
@@ -390,22 +421,50 @@ func setColumnType(newColumn *models.ColumnDefinition, orig models.ColumnDefinit | |
case orig.GetNumber() != nil: | ||
newColumn.SetNumber(orig.GetNumber()) | ||
case orig.GetLookup() != nil: | ||
colDetails.isLookupColumn = true | ||
isMultipleEnabled := ptr.Val(orig.GetLookup().GetAllowMultipleValues()) | ||
colDetails.isMultipleEnabled = isMultipleEnabled | ||
|
||
if isMultipleEnabled { | ||
colDetails.createFieldName = colName + LookupIDFieldNamePart | ||
} else { | ||
updatedName := colName + LookupIDFieldNamePart | ||
colDetails.getFieldName = updatedName | ||
colDetails.createFieldName = updatedName | ||
} | ||
|
||
newColumn.SetLookup(orig.GetLookup()) | ||
case orig.GetThumbnail() != nil: | ||
newColumn.SetThumbnail(orig.GetThumbnail()) | ||
case orig.GetTerm() != nil: | ||
newColumn.SetTerm(orig.GetTerm()) | ||
case orig.GetPersonOrGroup() != nil: | ||
colDetails.isPersonColumn = true | ||
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 | ||
|
||
newColumn.SetText(models.NewTextColumn()) | ||
} | ||
|
||
columnNames[colName] = 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 | ||
|
@@ -442,7 +501,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 +522,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 +531,102 @@ func setAdditionalDataByColumnNames( | |
fieldData := orig.GetAdditionalData() | ||
filteredData := make(map[string]any) | ||
|
||
for colName := range columnNames { | ||
if _, ok := fieldData[colName]; ok { | ||
filteredData[colName] = fieldData[colName] | ||
for _, colDetails := range columnNames { | ||
if val, ok := fieldData[colDetails.getFieldName]; ok { | ||
setMultipleEnabledByFieldData(val, colDetails) | ||
filteredData[colDetails.createFieldName] = val | ||
populateMultipleValues(val, filteredData, colDetails) | ||
} | ||
|
||
specifyODataType(filteredData, colDetails, colDetails.createFieldName) | ||
} | ||
|
||
return filteredData | ||
} | ||
|
||
func populateMultipleValues(val any, filteredData map[string]any, colDetails *columnDetails) { | ||
if !colDetails.isMultipleEnabled { | ||
return | ||
} | ||
|
||
if !colDetails.isPersonColumn && | ||
!colDetails.isLookupColumn { | ||
return | ||
} | ||
|
||
multiNestedFields, ok := val.([]any) | ||
if !ok || len(multiNestedFields) == 0 { | ||
return | ||
} | ||
|
||
lookupIDs := make([]float64, 0) | ||
|
||
checkFields := func(colDetails *columnDetails) []string { | ||
if colDetails.isLookupColumn { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a switch statement might be better here. Also, why an ad-hoc func? There's nothing here dependent upon runtime state. Seems like you can move it to an official func and give it some quick unit tests. |
||
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, checkFields(colDetails)...) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same cleanup recommendations here as the other PR |
||
lookupID, ok := md[LookupIDKey].(*float64) | ||
if !ok { | ||
continue | ||
} | ||
|
||
lookupIDs = append(lookupIDs, ptr.Val(lookupID)) | ||
} | ||
} | ||
|
||
filteredData[colDetails.createFieldName] = lookupIDs | ||
} | ||
|
||
func setMultipleEnabledByFieldData(val any, colDetails *columnDetails) { | ||
// already set while column definition | ||
// not required to determined from field values | ||
if colDetails.isMultipleEnabled { | ||
return | ||
} | ||
|
||
// 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 '<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 | ||
} | ||
|
||
// only specify odata.type for columns holding multiple data | ||
if !colDetails.isMultipleEnabled { | ||
return | ||
} | ||
|
||
switch { | ||
case colDetails.isPersonColumn, colDetails.isLookupColumn: | ||
filteredData[colName+ODataTypeFieldNamePart] = ODataTypeFieldNameIntVal | ||
default: | ||
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) | ||
|
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as the other PR, this can get reduced.