[EN] Allow entity objects to specify inherited attributes

[cabal95]

Prerequisites

• [X] Put an X between the brackets on this line if you have done all of the following:
  • Can you reproduce the problem on a fresh install or the demo site?
  • Did you include your Rock version number and client culture setting?
  • Did you perform a cursory search to see if your bug or enhancement is already reported?

Description

Currently, entities that have inherited attributes (GroupMember, Group, GroupType, RegistrationRegistrant, EventItem) have all the logic for how they get their inherited attributes buried inside the Rock.Attributes.Helper.LoadAttributes() method. This presents three issues:

1. A rather large method (250 lines) that can sometimes be a little hard to read through and understand all the logic.
2. Complexity in adding inherited attributes to a new core entity because we have to be very careful about not breaking anything else in LoadAttributes()
3. Impossible for custom (plug-in) entities to have inherited attributes as they would need to be able to modify the LoadAttributes() method.

Request/Recommendation

A new virtual method on IEntity called GetInheritedAttributes that would allow the entity itself to retrieve it's inherited attributes. Existing code would be moved out of LoadAttributes and into the concrete implementations of the relevant entity types.

Performance Impact

There shouldn't be much/any performance impact. There would be one extra method call for every call to LoadAttributes(), but that should be more than offset by not having to do as many checks for which entity type we are loading attributes for.

The default implementation would simply return an empty list or null to indicate that it has no inherited attributes.

Versions

• Rock Version: 6.8
• Client Culture Setting: en-US

[jonedmiston]

This makes a lot of sense. Might change the method from IEntity to the abstract Entity class so it's not a breaking change. See any issues with that?

[cabal95]

I keep forgetting that in C# you can't change an interface without it breaking compatibility. The only concern I would have is, since LoadAttributes is not a "class typed" method (e.g. LoadAttributes< T >), I'm not sure if we can do a if ( entity is Entity<T> ) { } and then cast it to that type, since we don't know what T would be. I'm not familiar enough to know if there is a (normal) way around that.

Obviously we can do pretty much anything with Reflection if we need to get check if the object is of type IEntity and then check if it has a method called GetInheritedAttributes and call it via Invoke.

[cabal95]

I'm thinking the proper way to do this is probably with a new interface called IHasInheritedAttributes that defines the GetInheritedAttributes. Then only the entities that have inherited attributes would "use" the IHasInheritedAttributes and LoadAttributes() could key off that.