Forms can break when form field matches on wrong FieldType/Control

[nlBayside]

Prerequisites

• [X] Put an X between the brackets on this line if you have done all of the following:
  • Did you perform a search at https://github.com/issues?q=is%3Aissue+user%3ASparkDevNetwork+-repo%3ARock to see if your bug or enhancement is already reported?
  • Can you reproduce the problem on a fresh install or the demo site?
  • Did you include your Rock version number and client culture setting?

Description

This just occurred for one of our church's registrations and after looking through the code, it appears there is a problem with how the FieldVisibilityWrapper works. Here is the exception's stack trace that appeared today:

> Keep in mind that although it says "Simple Donation", this block acts very similarly to Rock's internal RegistrationEntry block.

Digging through the code, it appears to me like this is a problem with this section in the FieldVisibilityWrapper.cs file

When an attribute has the same Id as the RegistrationTemplateFormField, it will always grab the attribute's field type and not even consider the actual FormField. This only happens when an attribute exists with the same Id as the RegistrationTemplateFormField, and that attribute's field type has an EditValue that conflicts with the original. This isn't often, but this shouldn't have had a collision in the first place.

For example, one of the Form Fields on our registration was a PersonField; however, there existed a Workflow attribute with a KeyValueFilter Field Type. When Rock accesses that field type's EditValue property, it will stumble onto an exception. The PersonField control doesn't have a "filter" property, so when a method like ToJson() is called on it, it will create a Null Reference Exception.

I am unconcerned with the Field Type classes and null reference exceptions. What is concerning is that The FieldVisibilityWrapper class will grab an Attribute unassociated with a RegistrationTemplateForm, and continue to use it. Not sure what fixes would solve this issue, but it would be great to see those collisions avoided in the first place. This change has been around for 16 months, but I'm not sure what is the earliest version of Rock that it appears in.

Versions

• Rock Version: [v13]
• Client Culture Setting: [us-en]

[nlBayside]

A temporary fix on our side was to recreate the Registration Template so that we get new FormField Ids. It took three attempts because we kept colliding with some ValueFilter Field Types. It might be a bigger problem for us due to the number of templates we have. Our odds of collision are probably higher than if you have a small number of templates.

[MikeDPeterson]

@nlBayside Do you have a little more of the StackTrace and exception info? It sounds like you are getting a Null Value Exception, but I don't see that in the screenshot. If not, I think I can still figure this out, but it might be helpful to see.

[abpena1979]

Hi @nlBayside Thank you for your submission and partnership. Unfortunately since this issue is produced using a third-party block, we can't provide further assistance. If you are able replicate this issue with the Rock block, please feel free to open a new issue. Thanks again!

[nlBayside]

@nlBayside Do you have a little more of the StackTrace and exception info? It sounds like you are getting a Null Value Exception, but I don't see that in the screenshot. If not, I think I can still figure this out, but it might be helpful to see.

I sadly don't have anything more than the StackTrace and exception info provided. The server variables and cookies in the exception page would not provide more context unfortunately. I had to walk my way through the code to investigate where the null reference occurred. I will be writing any responses in the reopened issue I created. Thank you for your time

[nlBayside]

I think this should be reopened again because the FieldVisibilityWrapper class is also used for Workflow Action Form Attributes. The changes here work for Registration Template Forms, but disregard Workflow forms.

This method will return null for the Workflow Action Form Attributes since it won't grab anything from the RegistrationTemplateCache, and then "id" will not pass the HasValue check. Then, in the ApplyFieldVisibilityRules method, both field and fieldAttribute will return null. The. attributeValues Dictionary will not get any values added to it because the first if won't pass. If the attributeValues Dictionary is empty, then the code won't evaluate any conditional workflow fields. This skips over the (Workflow ActionForm Attribute)table's FieldVisibilityRulesJSON column.

I think an additional fix would be to set a Boolean property for FieldVisibilityWrapper every time it's created. So that the code will know whether it's from a Workflow or from a RegistrationTemplate. This would also let us skip checking the RegistrationTemplateCache when it's from a Workflow. Thanks for the hard work! I hope this issue can finally be resolved soon. I'd be okay with opening my own pull request for this as well if that's necessary.