Skip to content

Conversation

ronaldbarendse
Copy link

This PR adds a new method to the generated models, so the property alias can be retrieved without requiring an UmbracoContext and will also be faster (because it does less).

@zpqrtbnk
Copy link
Collaborator

zpqrtbnk commented Oct 4, 2019

Hey, thanks for the PR! If I understand correctly, it will allow me to do:

var bodyAlias = HomePage.GetModelPropertyTypeAlias(x => x.Body);

Is that it?

As part of #215 the plan was to generate a metadata class that would allow you to do:

var bodyAlias = MB.PropertyAlias.HomePage.Body;

And here, Body is a string constant = as fast as it can get. How does that compare to your PR? Would #215 satisfy your needs, or is there reasons to prefer your proposal? Happy to discuss.

@zpqrtbnk zpqrtbnk added state/backlog Backlog, To Do type/discuss Discussion, Question labels Oct 4, 2019
@ronaldbarendse
Copy link
Author

Your example is correct, but having all the aliasses as constants looks even more promising and aligns more with how Umbraco stores the default aliasses (for conventions, etc.) 👍 If #215 is implemented, this would become obsolete, so maybe we shouldn't generate the additional method for now...

I've cleaned up the already public methods on PublishedModelUtility, added overloads that don't require the TValue type constraint (the implmentation doesn't care about the result type of the expression) and limited TModel to IPublishedElement (as all MB generated classes inherit from this, via either PublishedContentModel or PublishedElementModel).

With this PR, you'll be able to get the property alias without an UmbracoContext using:

PublishedModelUtility.GetModelPropertyTypeAlias<Project>(p => p.Products);

@zpqrtbnk
Copy link
Collaborator

zpqrtbnk commented Oct 4, 2019

👍

@CarlSargunar
Copy link

+1 for getting this in as soon as poss, as I was looking at submitting a PR for exactly this functionality myself, but glad it's been considered and a fix been written.

@zpqrtbnk zpqrtbnk added this to the 4.0.0 milestone Nov 12, 2019
@ronaldbarendse
Copy link
Author

It would be great if this gets merged into the latest version, especially if #215 isn't yet implemented (but even if it is, this PR contains some bugfixes for existing code)...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/backlog Backlog, To Do type/discuss Discussion, Question

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants