Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GetFormModelProperty function not considering compositions #172

Closed
joshreid opened this issue Feb 11, 2018 · 3 comments
Closed

GetFormModelProperty function not considering compositions #172

joshreid opened this issue Feb 11, 2018 · 3 comments

Comments

@joshreid
Copy link

joshreid commented Feb 11, 2018

Hi @kjac

I have found that the GetFormModelProperty function does not consider the possibility of property being in a composition in the contentType.

So I have revised code in ContentHelper.cs below to fix - can give pull request if you want.

public static PropertyType GetFormModelProperty(IContentType contentType)
{
	List<PropertyType> propertyTypes = contentType.PropertyTypes.ToList();
	propertyTypes.AddRange(contentType.CompositionPropertyTypes);
			
	var property = propertyTypes.FirstOrDefault(p => p.PropertyEditorAlias == FormModel.PropertyEditorAlias);
	return property;
}

Let me know if you need anything else and when you'll repackage - I'll run my custom built dll for now.

@kjac
Copy link
Owner

kjac commented Feb 11, 2018

Hi @joshreid,

Awesome, thank you!

Just to avoid creating the list if we don't have to, could you try this code and see if that works for you as well?

public static PropertyType GetFormModelProperty(IContentType contentType)
{
  bool IsFormModelPropertyEditor(PropertyType p) => p.PropertyEditorAlias == FormModel.PropertyEditorAlias;

  return contentType.PropertyTypes.FirstOrDefault(IsFormModelPropertyEditor)
         ?? contentType.CompositionPropertyTypes.FirstOrDefault(IsFormModelPropertyEditor);
}

@joshreid
Copy link
Author

All good thanks @kjac - yep that's better, and works as expected.

kjac pushed a commit that referenced this issue Feb 13, 2018
@kjac
Copy link
Owner

kjac commented Feb 22, 2018

Whoops, this one's in the latest version :) closing it now.

@kjac kjac closed this as completed Feb 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants