Cherry Pick DYN-1476: AllowRankReductionAttribute on Zero Touch Properties (#9542)

[scottmitchell]

• update

• Brought truncated search results and full query search out from behind debug menu. Deleted debug menu items.

• Added CheckForRankReductionAttribute() method to FFIMemberInfo

• Cleaned up CheckForRankReductionAttribute

• Added AllowRankReductionAttribute tests

• Comments. Change method name.

• Update tests, method name

• Corrected test names

• Fixed annoying diff

Purpose

Cherry Pick this PR to 2.2.0

Declarations

Check these if you believe they are true

• The code base is in a better state after this PR
• Is documented according to the standards
• The level of testing this PR includes is appropriate
• User facing strings, if any, are extracted into *.resx files
• All tests pass using the self-service CI.
• Snapshot of UI changes, if any.
• Changes to the API follow Semantic Versioning, and are documented in the API Changes document.

Reviewers

FYIs

[QilongTang]

LGTM 👍

[mjkkirschner]

lets wait on this until we resolve the failing test / question about where to use this attribute.

[aparajit-pratap]

@scottmitchell please update this test:

Dynamo/test/Engine/ProtoTest/Associative/MicroFeatureTests.cs

Line 1159 in 0dbc0cc

r = a.Values;

as the Values property of a Dictionary has the AllowRankReduction attribute.

[mjkkirschner]

@aparajit-pratap - @scottmitchell and I wanted to talk to you first - and see what you thought about

1. why that attribute was used here in the first place and
2. will it potentially break people's graphs
3. is it worth deferring until later? (3.0) to preserve current behavior. (ie, we remove the attribute to keep values working as it is today)

[aparajit-pratap]

@mjkkirschner I'm okay with removing the attribute from the Values property altogether, especially because its name indicates its plural and it therefore should be okay for users to expect that it will always return a list, even if it is a list of a single value.

[scottmitchell]

@mjkkirschner @aparajit-pratap Sounds good, I'll create a PR removing the attribute from Dictionary.Values, and I'll add a TODO to look at adding this for 3.0

[mjkkirschner]

so we can close this?

[scottmitchell]

@mjkkirschner Yeah we can close this. I wasn't sure if it was better to change this PR or just close it and make a new one.

[scottmitchell]

Or wait.. Do you mean merge this one? And then we'll cherry pick the follow up PR also?

[mjkkirschner]

no I meant close this PR without merging and then on the 2.2 branch all we need to do is cherry pick the other change right?

[mjkkirschner]

either is fine I guess. your call.

[scottmitchell]

Ok that makes sense. Closing this one.

[aparajit-pratap]

Wait, I'm confused. Why has this been closed? I thought this fix was planned for 2.2 also.

[QilongTang]

@mjkkirschner @aparajit-pratap @scottmitchell Sorry for the confusion but we only need to cherry-pick one more change into this PR and we are done. I think it makes best sense to do both changes from master in a single PR to 2.2.0

[scottmitchell]

@QilongTang @mjkkirschner @aparajit-pratap Both PRs are cherry picked. I'll merge once the checks complete.

[QilongTang]

@scottmitchell Glad it worked!