spmeta2 icon indicating copy to clipboard operation
spmeta2 copied to clipboard

Extend FieldDefinition property "AddFieldOptions" to array

Open vlad-ivanov-d opened this issue 7 years ago • 6 comments

Hello,

Sometimes it's necessary to use more than one AddFieldOptions property in FieldDefinition (for example: BuiltInAddFieldOptions.AddToAllContentTypes and BuiltInAddFieldOptions.AddFieldInternalNameHint), but right now it's impossible.

So it will be good to create another property with an array type.

Reference to parent issue: https://github.com/SubPointSolutions/spmeta2/issues/1073

vlad-ivanov-d avatar Oct 20 '17 09:10 vlad-ivanov-d

Would you create an array or rather a list to contain those multiple options?

andreasblueher avatar Jan 31 '18 07:01 andreasblueher

List might be a way forward. We've got both arrays and lists here and there. Array and collection usage seem to be a legacy thing left out there for a backward compatibility. List looks ok as far as it can be serialized without issues.

avishnyakov avatar Jan 31 '18 07:01 avishnyakov

@vladivanovrf @andreasblueher getting back on this feature. There are a few related tickets and PR:

  • Extend FieldDefinition property "AddFieldOptions" to array #1091
  • FieldDefinition ignores InternalName #1073
  • PR: https://github.com/SubPointSolutions/spmeta2/pull/1099

Here is how it all works:

  1. @avishnyakov made a wrong review on the original ticket suggesting that it is not possible to setup more than one value for FieldDefinition.AddFieldOptions property:

https://github.com/SubPointSolutions/spmeta2/issues/1073#issuecomment-338144256

No, @vladivanovrf, unfortunately we can't. This is a "bug", more of a misdesign, overlooked decision made long time ago. We had a similar problem with ListViewDefinition, solved with a new property.

  1. We never looked into more details assuming that FieldDefinition.AddFieldOptions property is a string. Hence, the issue of setting up several values can only be solved by introducing a new property.

  2. Actually, this particular property is enum ( SPMeta2.Enumerations.BuiltInAddFieldOptions) which makes it possible to use normal "&" or "|" operation to combine several bits. This is, surely, absurdly overlooked ticket and whole situation around all this.

Here is how it can be done, and below some tests:

var fieldDef = ModelGeneratorService.GetRandomDefinition<FieldDefinition>(def =>
            {
                def.Hidden = false;

                def.ShowInDisplayForm = true;
                def.ShowInEditForm = true;
                def.ShowInListSettings = true;
                def.ShowInNewForm = true;
                def.ShowInViewForms = true;

                def.AddFieldOptions = BuiltInAddFieldOptions.AddToAllContentTypes
                    | BuiltInAddFieldOptions.AddFieldInternalNameHint;
            });

Full test:

https://github.com/SubPointSolutions/spmeta2/blob/f54e29be0724aecc3ee90fbd7c58fdccc4f8eb82/SPMeta2/SPMeta2.Regression.Tests/Impl/Scenarios/ContentTypeScenariosTest.cs#L381

With all this, the whole ticket and related PR done by @andreasblueher aren't necessary.

@vladivanovrf @andreasblueher could you please review this investigation and confirm outcomes? Again, this is epic and absolutely weirdly overlooked by nearly everyone. Hope that is a good learning so we can improve out attention to details.

SubPointSupport avatar May 22 '18 12:05 SubPointSupport

Folks, @vladivanovrf @andreasblueher, we'd appreciate if you could have a close look on this ticket and related PR https://github.com/SubPointSolutions/spmeta2/pull/1099

By looking into current functionality, the current ticket and corresponding PR don't have to be moved forward. Looking for your confirmation aiming to close them this week.

SubPointSupport avatar May 28 '18 11:05 SubPointSupport

Hey guys, I was sick for almost 2 weeks straight and visiting a SharePoint conference the week after so it took some time to get back here. I just tested the possibility to combine BuiltInAddFieldOptions using "|" and i can confirm that it is working as expected. I combined 3 different options and it worked.

If you change those options afterwards it's not updating the field accordingly, but this seems very far fetched to expect this kind of behavior so imho you can close this issue and delete the PR.

andreasblueher avatar Jun 07 '18 14:06 andreasblueher

I can confirm using "|" to combine those options works fine.

andreasblueher avatar Jul 24 '18 14:07 andreasblueher