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

Modify to fix issue for #2076, Containment expand problem #2083

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

xuzhg
Copy link
Member

@xuzhg xuzhg commented Mar 12, 2020

Issues

This pull request fixes issue #2076.

Description

*This fixes the issue in #2076.

  1. For the model builder, all non-containment navigation property should have a navigation property binding.
    2)ODataQueryOptionParser should know the "Path" to parse the query option.

Checklist (Uncheck if it is not completed)

  • Test cases added
  • Build and test with one-click build and test script passed

Additional work necessary

If documentation update is needed, please add "Docs Needed" label to the issue and provide details about the required document change in the issue.

@xuzhg xuzhg force-pushed the issue2076ExpandOnNavigation branch from 6667e20 to 7f03890 Compare March 12, 2020 05:28
@mikepizzo mikepizzo added this to the 7.5 milestone May 6, 2020
@mikepizzo mikepizzo self-requested a review September 8, 2020 23:06
@xuzhg xuzhg removed this from the 7.5 milestone Sep 14, 2020

namespace AspNetCore3xEndpointSample.Web.Controllers
{
{/*
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason to keep the commented out version?


namespace AspNetCore3xEndpointSample.Web.Models
{
{/*
Copy link
Member

@mikepizzo mikepizzo Jan 5, 2021

Choose a reason for hiding this comment

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

why is this commented out? Is it no longer used? Just remove?

//public CustomerOrderContext(DbContextOptions<CustomerOrderContext> options)
// : base(options)
//{
//}
Copy link
Member

Choose a reason for hiding this comment

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

Can this be removed?

{
}

public DbSet<Customer> Customers { get; set; }

public DbSet<Order> Orders { get; set; }
// public DbSet<Order> Orders { get; set; }
Copy link
Member

Choose a reason for hiding this comment

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

remove?

builder.EntitySet<Order>("Orders");
var customers = builder.EntitySet<Customer>("Customers");
customers.Binding.HasManyPath(c => c.CustomerReferrals, true).HasRequiredBinding(r => r.ReferredCustomer, "Customers");
// builder.EntitySet<Order>("Orders");
Copy link
Member

Choose a reason for hiding this comment

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

remove?

@@ -32,7 +31,8 @@ public Startup(IConfiguration configuration)
// This method gets called by the runtime. Use this method to add services to the container.
public void ConfigureServices(IServiceCollection services)
{
services.AddDbContext<CustomerOrderContext>(opt => opt.UseLazyLoadingProxies().UseInMemoryDatabase("CustomerOrderList"));
//services.AddDbContext<CustomerOrderContext>(opt => opt.UseLazyLoadingProxies().UseInMemoryDatabase("CustomerOrderList"));
//services.AddScoped<CustomerOrderContext>(_ => new CustomerOrderContext(Configuration.GetConnectionString("DefaultConnection")));
services.AddOData();
Copy link
Member

Choose a reason for hiding this comment

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

Remove commented out lines?

@mikepizzo
Copy link
Member

Is there a test to verify that the issue reported in issue 2076 is addressed? I see changes to the sample but no code that calls them in new ways.

Copy link
Member

@mikepizzo mikepizzo left a comment

Choose a reason for hiding this comment

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

🕐

@mikepizzo
Copy link
Member

Hey @xuzhg --

Do you have an update on this PR? I left a few minor comments. Can we address, rebase, and merge, or are there open issues?

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