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

Partial fix for 6674. Can configure navigations between owner and owned types. #20222

Closed
wants to merge 1 commit into from

Conversation

lajones
Copy link
Contributor

@lajones lajones commented Mar 8, 2020

Partial fix for #6674. Adds the ability to configure navigations between owner and owned types.

@lajones lajones self-assigned this Mar 8, 2020
/// </param>
/// <returns> The same builder instance so that multiple configuration calls can be chained. </returns>
public virtual OwnedNavigationBuilder OwnerToOwnedNavigation(
[NotNull] Action<NavigationBuilder> navigationConfiguration)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I'm open to renaming this and the one below.
Advantages: it makes the Owner-to-Owned-Type relationship clear.
Disadvantages: it's easy to get OwnerToOwnedNavigation and OwnedToOwnerNavigation mixed up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about this name the alternative of having OwnsOne with two lambdas doesn't appear that bad.

You can rename this to WithOwnedNavigation for now and we can discuss in design meeting.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a problem with putting 2 lambdas on OwnsOne(). We already have the two methods below.

        public virtual OwnedNavigationBuilder OwnsOne(
            [NotNull] string ownedTypeName,
            [NotNull] string navigationName)

and

        public virtual EntityTypeBuilder OwnsOne(
            [NotNull] string ownedTypeName,
            [NotNull] string navigationName,
            [NotNull] Action<OwnedNavigationBuilder> buildAction)
        {

But if you add an extra optional Action<NavigationBuilder> parameter to both and try to use them, the compiler complains saying it doesn't know whether you're calling the first one or the second one with the 4th parameter null. I.e. nb => nb.UsePropertyAccessMode(...) is valid on both of them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would only add it to the overload that has the buildAction

/// An action which configures the navigation property from the owned type to its owner.
/// </param>
/// <returns> The same builder instance so that multiple configuration calls can be chained. </returns>
public virtual OwnedNavigationBuilder OwnedToOwnerNavigation(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is not needed. just add the parameter to WithOwner()

@lajones
Copy link
Contributor Author

lajones commented Mar 14, 2020

Not taking this approach any more. Superseded by #20271 and #20282

@lajones lajones closed this Mar 14, 2020
@AndriySvyryd AndriySvyryd deleted the 20200308_Issue6674_OwnsXyz_01 branch March 14, 2020 03:25
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

Successfully merging this pull request may close these issues.

2 participants