Skip to content

feat(product): T2 — EF Core persistence + ORM config (verbatim from monolith) #60

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.

Sign up for GitHub

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

Open
devin-ai-integration wants to merge 1 commit into
base: devin/product-t1-domain
Choose a base branch
from
Open

feat(product): T2 — EF Core persistence + ORM config (verbatim from monolith) #60

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/Services/Product/Product.API/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@

var app = builder.Build();

using (var scope = app.Services.CreateScope())
{
var db = scope.ServiceProvider.GetRequiredService<ProductDbContext>();
db.Database.EnsureCreated();
}
Comment on lines +16 to +20

@devin-ai-integration devin-ai-integration Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🚩 EnsureCreated() runs unconditionally in all environments and is incompatible with future migrations

The db.Database.EnsureCreated() call at src/Services/Product/Product.API/Program.cs:19 runs in every environment (including production), not just development. This is consistent with the Notification service (src/Services/Notification/Notification.API/Program.cs:26), so it follows the existing codebase pattern. However, EnsureCreated() does not create the __EFMigrationsHistory table and is fundamentally incompatible with EF Core migrations. If migrations are added later, Database.Migrate() will attempt to apply all migrations from scratch and fail because the tables already exist. This is fine as long as the team is aware they cannot mix the two approaches. If migrations are planned, this should be changed to Database.Migrate() before any migration is created.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@devin-ai-integration devin-ai-integration Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Acknowledged — this is intentionally consistent with the Notification service pattern (src/Services/Notification/Notification.API/Program.cs:26). The task spec explicitly calls for EnsureCreated() on startup. If the team later adopts EF migrations, this can be swapped to Database.Migrate() at that time.


if (app.Environment.IsDevelopment())
{
app.UseSwagger();
Expand Down
24 changes: 21 additions & 3 deletions src/Services/Product/Product.Infrastructure/Data/ProductDbContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Microsoft.EntityFrameworkCore;
using Product.Domain.Entities;

namespace Product.Infrastructure.Data;

Expand All @@ -8,9 +9,26 @@ public ProductDbContext(DbContextOptions<ProductDbContext> options) : base(optio
{
}

protected override void OnModelCreating(ModelBuilder modelBuilder)
public DbSet<Product.Domain.Entities.Product> Products => Set<Product.Domain.Entities.Product>();
public DbSet<ProductCategory> ProductCategories => Set<ProductCategory>();

protected override void OnModelCreating(ModelBuilder builder)
{
base.OnModelCreating(modelBuilder);
// TODO: Configure entity mappings migrated from monolith
base.OnModelCreating(builder);

// ProductCategory — verbatim from monolith
builder.Entity<ProductCategory>().Property(p => p.Name).IsRequired().HasMaxLength(100);
builder.Entity<ProductCategory>().Property(p => p.Description).HasMaxLength(500);
builder.Entity<ProductCategory>().ToTable("AppProductCategories");

// Product — verbatim from monolith
builder.Entity<Product.Domain.Entities.Product>().Property(p => p.Name).IsRequired().HasMaxLength(100);
builder.Entity<Product.Domain.Entities.Product>().HasIndex(p => p.Name);
builder.Entity<Product.Domain.Entities.Product>().Property(p => p.Description).HasMaxLength(500);
builder.Entity<Product.Domain.Entities.Product>().Property(p => p.Icon).IsUnicode(false).HasMaxLength(256);
builder.Entity<Product.Domain.Entities.Product>().HasOne(p => p.Parent).WithMany(p => p.Children).OnDelete(DeleteBehavior.Restrict);
builder.Entity<Product.Domain.Entities.Product>().Property(p => p.BuyingPrice).HasColumnType("decimal(18,2)");
builder.Entity<Product.Domain.Entities.Product>().Property(p => p.SellingPrice).HasColumnType("decimal(18,2)");
builder.Entity<Product.Domain.Entities.Product>().ToTable("AppProducts");
Comment on lines +24 to +32

@devin-ai-integration devin-ai-integration Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🚩 Product-to-ProductCategory relationship relies on EF Core convention rather than explicit configuration

The OnModelCreating method explicitly configures the Product self-referencing relationship (Parent/Children) but does not explicitly configure the Product-to-ProductCategory relationship. The Product entity has ProductCategoryId (int, non-nullable) and ProductCategory (required) at src/Services/Product/Product.Domain/Entities/Product.cs:17-18, while ProductCategory has Products collection at src/Services/Product/Product.Domain/Entities/ProductCategory.cs:9. EF Core will discover this by convention and configure it with cascade delete (since the FK is non-nullable). This works correctly, but the delete behavior differs from the self-referencing relationship which is explicitly set to Restrict. If the monolith used Restrict for this relationship too, that nuance would be lost. The comment says 'verbatim from monolith' so this may be intentional, but worth confirming.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@devin-ai-integration devin-ai-integration Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Correct — the Product→ProductCategory relationship uses EF Core's convention (cascade delete for non-nullable FK). This matches the monolith's configuration, which also did not explicitly configure this relationship's delete behavior — only the self-referencing Parent→Children relationship had an explicit Restrict. The convention-based cascade here is intentional and verbatim from the source.

}
}