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

Add table name to AddHandlebarsTransformers propertyTransformer #189

Closed
johnwc opened this issue Nov 20, 2021 · 23 comments
Closed

Add table name to AddHandlebarsTransformers propertyTransformer #189

johnwc opened this issue Nov 20, 2021 · 23 comments
Labels
enhancement New feature or request
Milestone

Comments

@johnwc
Copy link

johnwc commented Nov 20, 2021

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.

@johnwc johnwc added the enhancement New feature or request label Nov 20, 2021
@tonysneed
Copy link
Contributor

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

@johnwc
Copy link
Author

johnwc commented Nov 24, 2021

Does not look to be the same issue.

@tonysneed tonysneed reopened this Nov 24, 2021
@tonysneed tonysneed added this to the v6 milestone Nov 24, 2021
@tonysneed
Copy link
Contributor

@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
Copy link
Author

johnwc commented Dec 15, 2021

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

@tonysneed
Copy link
Contributor

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

@tonysneed
Copy link
Contributor

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

@johnwc
Copy link
Author

johnwc commented Jan 6, 2022

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
Copy link

Herdo commented Aug 14, 2022

@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
Copy link
Author

johnwc commented Jun 18, 2023

@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
Copy link
Contributor

ErikEJ commented Jun 18, 2023

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

@johnwc
Copy link
Author

johnwc commented Jun 18, 2023

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

@ErikEJ
Copy link
Contributor

ErikEJ commented Jun 18, 2023

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

@johnwc
Copy link
Author

johnwc commented Jun 18, 2023

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
Copy link
Contributor

ErikEJ commented Jun 18, 2023

Docs are in the EF Core repo wiki, and no

@johnwc
Copy link
Author

johnwc commented Jun 18, 2023

@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
Copy link
Contributor

ErikEJ commented Jun 18, 2023

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

@johnwc
Copy link
Author

johnwc commented Jun 18, 2023

@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
Copy link
Contributor

ErikEJ commented Jun 20, 2023

@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
Copy link
Contributor

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
Copy link
Contributor

ErikEJ commented Jun 20, 2023

@tonysneed Sorry, you are correct. Misunderstood the question

@johnwc
Copy link
Author

johnwc commented Jun 20, 2023

@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
Copy link
Contributor

how can we resolve #189 (comment) 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
Copy link
Author

johnwc commented Jun 21, 2023

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants