TrackableEntities / EntityFrameworkCore.Scaffolding.Handlebars

Scaffold EF Core models using Handlebars templates.
MIT License
208 stars 53 forks source link

Add table name to AddHandlebarsTransformers propertyTransformer #189

Closed johnwc closed 2 years ago

johnwc commented 2 years ago

As the title explains, add the table name to the propertyTransformer function for AddHandlebarsTransformers so that we know what table/class the property is for.

tonysneed commented 2 years ago

Looks like this may be a duplicate of #188. Please re-open if this is not the case.

johnwc commented 2 years ago

Does not look to be the same issue.

tonysneed commented 2 years ago

@johnwc AddHandlebarsTransformers now accepts arguments with an IEntityType parameter, which allows you to get the table name by calling the GetTableName extension method.

Will this fulfill your need?

johnwc commented 2 years ago

Yes, that will work perfectly. When will this update be released?

tonysneed commented 2 years ago

@johnwc It's included with the v6.0.0 release, which I published to NuGet yesterday.

tonysneed commented 2 years ago

@johnwc Please re-open if you need further help.

johnwc commented 2 years ago

So, trying to determine the property to set. I wrote this quick debug code to see what it comes up with, since I can't step through the code. Why is it coming up twice?

services.AddHandlebarsTransformers2(
    propertyTransformer: (i, e) =>
    {
        var table = i.GetTableName();
        switch (table)
        {
            case "fan_events":
                switch(e.PropertyName)
                {
                    case "Status":
                        Console.WriteLine("propertyTransformer: {0}.{1}: {2}", table, e.PropertyName, e.PropertyType);
                        break;
                }
                break;
        }
        return e;
    });

Outcome...

> dotnet ef dbcontext scaffold "..." "Pomelo.EntityFrameworkCore.MySql" --force -o DAL -c MCDb --no-onconfiguring
Build started...
Build succeeded.
Using ServerVersion '8.0.23-mysql'.
propertyTransformer: fan_events.Status: FanEvent  <-- ????
propertyTransformer: fan_events.Status: int
Herdo commented 1 year ago

@johnwc I just noticed the same thing while working on #216 . Apparently, the DeclaringType of the property is passed into the PropertyType parameter, instead of the property's type itself. Therefore, FanEvent is the entity type in your case.

@tonysneed Is this intentional? If not, I'll try to fix this alongside 216.

johnwc commented 1 year ago

@tonysneed @Herdo finally getting around to wanting to rename nav properties and ran into an issue. We have a table called user_tweaks with 3 foreign keys to three different tables. When trying to rename the inverse navigation properties using navPropertyTransformer, the PropertyName and PropertyType is the same for each of the navigation properties, and there is no way to know which property relates to which inverse entity.

Here is a console debug to help explain.

navPropertyTransformer: (i, e) =>
{
    var table = i.GetTableName() ?? i.GetViewName();
    Console.WriteLine("navPropertyTransformer: {0}.{1}: {2} {3}", table, e.PropertyName, e.PropertyType, i.Name);
    return e;
}

One of those belongs to the inverse property for Request entity, one for User Entity, and other for Share entity.

navPropertyTransformer: user_tweaks.UserTweaks: UserTweak UserTweak
navPropertyTransformer: user_tweaks.UserTweaks: UserTweak UserTweak
navPropertyTransformer: user_tweaks.UserTweaks: UserTweak UserTweak
ErikEJ commented 1 year ago

@johnwc EF Core Power Tools now has property renaming, just FYI

johnwc commented 1 year ago

@ErikEJ If I rename a navigation property, then do a scaffold will the name change persist?

ErikEJ commented 1 year ago

Yes, you need to configure renaming once in a json file

johnwc commented 1 year ago

Yes, you need to configure renaming once in a json file

@ErikEJ Where can I find documentation how to do it? Can I use diagram to rename it?

ErikEJ commented 1 year ago

Docs are in the EF Core repo wiki, and no

johnwc commented 1 year ago

@ErikEJ does the renaming json file support setting the type for a column? Like changing int to a Enum? Is there documented schemas for the json files so I can read all what can be configured with them?

ErikEJ commented 1 year ago

Did you read the wiki. No enum renaming, there are other ways to do that, as documented.

johnwc commented 1 year ago

@ErikEJ yes, I read through the wiki and saw no documentation on schema for json files. Can I use my existing AddHandlebarsScaffolding with your tool? So it can take care of the type modifications. We're not wanting to take on managing T4/handlebar templates just to change int to enum for a handful of properties.

ErikEJ commented 1 year ago

@johnwc The format and feature is documented here: https://github.com/ErikEJ/EFCorePowerTools/wiki/Reverse-Engineering#custom-renaming-with-efptproperty-renamingjson

Can I use my existing AddHandlebarsScaffolding with your tool?

Yes, I believe so, what have you tried?

tonysneed commented 1 year ago

Can I use my existing AddHandlebarsScaffolding with your tool?

My understanding is that AddHandlebarsScaffolding is not supported by EFC PT. This takes place in IDesignTimeServices.ConfigureDesignTimeServices.

ErikEJ commented 1 year ago

@tonysneed Sorry, you are correct. Misunderstood the question

johnwc commented 1 year ago

@tonysneed seeing EFC PT doesn't support calling AddHandlebarsScaffolding, how can we resolve the issue I raised with knowing which inverse nav property belongs to which entity?

@ErikEJ is there any plans to include allowing to call AddHandlebarsScaffolding in EFC PT?

tonysneed commented 1 year ago

how can we resolve https://github.com/TrackableEntities/EntityFrameworkCore.Scaffolding.Handlebars/issues/189#issuecomment-1596004863 with knowing which inverse nav property belongs to which entity

Would you be able to propose a solution? If you like, you can open a new issue for this and submit a PR with the solution.

johnwc commented 1 year ago

@tonysneed Unfortunately I had surgery on my shoulder last week and I am down to like 20% productivity. So, it would take me a while to produce a PR for you.

If you would like me to create a new issue for this, I can though? To stay backward compatible, my proposal would be to create a new class named EntityNavPropertyInfo that inherits from EntityPropertyInfo. Using that new class for the T2 parameter for the navPropertyTransformer callbacks definition Func<T1, T2, TResult>. Within that new class add a readonly property that allows us to know what the Type is for the Inverse Entity, if the nav property is for an Inverse entity. For my example above, it would be either Request, User, or Share.

In reality what I was expecting it to be like, was that for the navPropertyTransformer the IEntityType be the type of the entity the nav property belongs to and not the type it points to. As the e.PropertyType already tells me what Entity Type it points to.

navPropertyTransformer: (i, e) =>
{
    var table = i.GetTableName() ?? i.GetViewName();
    Console.WriteLine("navPropertyTransformer: {0}.{1}: {2} {3}", table, e.PropertyName, e.PropertyType, i.Name);
    return e;
}

So this outcome...

navPropertyTransformer: user_tweaks.UserTweaks: UserTweak UserTweak
navPropertyTransformer: user_tweaks.UserTweaks: UserTweak UserTweak
navPropertyTransformer: user_tweaks.UserTweaks: UserTweak UserTweak

I expected to be...

navPropertyTransformer: user_tweaks.UserTweaks: UserTweak Request
navPropertyTransformer: user_tweaks.UserTweaks: UserTweak User
navPropertyTransformer: user_tweaks.UserTweaks: UserTweak Share

To not stay backward compatible, I propose the IEntityType for navPropertyTransformer be changed to using the entity type that the nav property belongs to. Ex: Request, User, or Share. In this scenario, you could always create a new method named AddHandlebarsTransformersWithEntityType with the correction, and then [Obsolete] the AddHandlebarsTransformers2 for change management.