Skip to content

LNK-4374: Yearl Goals - Backend Overhaul - Census - #1201

Closed
nvmLantana wants to merge 29 commits into
devfrom
nvm/LNK-4374_CensusManagerQueries
Closed

LNK-4374: Yearl Goals - Backend Overhaul - Census#1201
nvmLantana wants to merge 29 commits into
devfrom
nvm/LNK-4374_CensusManagerQueries

Conversation

@nvmLantana

@nvmLantana nvmLantana commented Oct 23, 2025

Copy link
Copy Markdown
Contributor

🛠️ Description of Changes

Please provide a high-level overview of the changes included in this PR.

🧪 Testing Performed

Please describe the testing that was performed on the changes included in this PR.

🧑‍🔬 Unit Testing

  • I have written or updated unit tests to cover my changes

📓 Documentation Updated

Please update any relevant sections in the project documentation that were impacted by the changes in the PR.

Summary by CodeRabbit

  • New Features

    • Added search and filtering capabilities for census configurations with pagination and sorting support
    • Enhanced census configuration management with validation and existence checks
  • Improvements

    • Updated API response models for census configuration endpoints to provide consistent data structures
    • Improved database ID generation and transaction handling for configuration operations

@coderabbitai

coderabbitai Bot commented Oct 23, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

The changes refactor the Census module's data layer into a Manager/Query separation pattern, introduce new strongly-typed models for API and domain operations, redesign entity inheritance (removing BaseEntityExtended), and establish a new database transaction abstraction layer.

Changes

Cohort / File(s) Summary
Model Layer Refactoring
DotNet/Census/Application/Models/CensusConfigApiModel.cs, DotNet/Census/Models/CensusConfigModel.cs, DotNet/Census/Models/CreateCensusConfigModel.cs, DotNet/Census/Models/UpdateCensusConfigModel.cs, DotNet/Census/Models/SearchCensusConfigModel.cs
Renames public API model from CensusConfigModel to CensusConfigApiModel; introduces new domain models CensusConfigModel with FromDomain() factory; adds input models CreateCensusConfigModel and UpdateCensusConfigModel; creates SearchCensusConfigModel for paginated queries with sorting support.
Entity & Context Updates
DotNet/Census/Domain/Entities/CensusConfigEntity.cs, DotNet/Census/Domain/Context/CensusContext.cs
Removes BaseEntityExtended inheritance from CensusConfigEntity; adds explicit Id (Guid), CreateDate, and ModifyDate properties; replaces HasConversion-based GUID handling with ValueGeneratedOnAdd() for three entity ID properties in context.
Repository & Database Abstraction
DotNet/Census/Application/Interfaces/ICensusSchedulingRepository.cs, DotNet/Census/Application/Repositories/Scheduling/CensusSchedulingRepository.cs, DotNet/Census/Domain/Repositories/Database.cs
Updates scheduling repository interface/implementation to accept CensusConfigModel instead of CensusConfigEntity; normalizes FacilityID to FacilityId; introduces new IDatabase interface with transaction and repository abstraction; implements Database class wrapping EF Core operations.
Manager Refactoring
DotNet/Census/Domain/Managers/CensusConfigManager.cs
Complete rewrite replacing legacy CRUD methods with async CreateAsync(), UpdateAsync(), DeleteAsync() APIs; adopts IDatabase abstraction; adds transactional workflows with validation; replaces IBaseEntityRepository<CensusConfigEntity> with IDatabase dependency.
Query Layer
DotNet/Census/Domain/Queries/CensusConfigQueries.cs
New query implementation with ICensusConfigQueries interface providing GetAsync(), SearchAsync(), and PagedSearchAsync() methods; returns CensusConfigModel and PagedConfigModel<CensusConfigModel>; includes dynamic sorting and pagination.
Service & Controller Updates
DotNet/Census/Application/Services/ScheduleService.cs, DotNet/Census/Controllers/CensusConfigController.cs
Updates ScheduleService to use ICensusConfigQueries instead of ICensusConfigManager; refactors CensusConfigController to use query-driven existence checks, new API model types, and async manager methods; adjusts response types to CensusConfigModel.
Dependency Injection
DotNet/Census/Program.cs
Replaces repository registration from IBaseEntityRepository<CensusConfigEntity> with IEntityRepository<CensusConfigEntity, CensusContext>; adds registration for ICensusConfigQueriesCensusConfigQueries.

Sequence Diagram

sequenceDiagram
    participant Controller as CensusConfigController
    participant Manager as CensusConfigManager
    participant Queries as CensusConfigQueries
    participant Database as IDatabase
    participant Repo as CensusConfigRepository
    participant Context as CensusContext

    rect rgb(220, 240, 255)
    Note over Controller,Context: Previous Flow (Old Manager/Repository)
    Controller->>Manager: AddOrUpdateCensusConfig()
    Manager->>Repo: Direct entity operations
    Repo->>Context: SaveChanges
    end

    rect rgb(240, 255, 240)
    Note over Controller,Context: New Flow (Manager/Query Split)
    
    rect rgb(255, 250, 240)
    Note over Controller,Queries: Read Path
    Controller->>Queries: GetAsync(facilityId)
    Queries->>Context: Query CensusConfigs
    Queries-->>Controller: CensusConfigModel
    end
    
    rect rgb(240, 250, 255)
    Note over Manager,Context: Write Path (with Transactions)
    Controller->>Manager: CreateAsync(CreateCensusConfigModel)
    Manager->>Database: BeginTransactionAsync()
    Manager->>Repo: Query/Add entity via repository
    Manager->>Database: SaveChangesAsync()
    Manager->>Database: CommitTransactionAsync()
    Manager-->>Controller: CensusConfigModel
    Manager->>Repo: Trigger scheduling updates
    end
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

This refactoring introduces heterogeneous changes across the data layer: significant architectural shifts (Manager/Query split, new database abstraction), entity structural changes (inheritance removal, ID generation), dense logic in manager transactional flows and query pagination/sorting, and pervasive updates across controllers, services, and DI registration. Multiple files require independent reasoning about domain logic changes, validation flows, and API surface modifications.

Possibly related PRs

Suggested labels

db-change

Suggested reviewers

  • arianamihailescu
  • dvargaslantana
  • seanmcilvenna

Poem

🐰 Hops with joy through refactored halls,
Manager queries answer the calls,
Models new and entities refined,
Transactions flow with tests in mind,
The data layer's been redesigned!

Pre-merge checks and finishing touches

❌ Failed checks (3 warnings)
Check name Status Explanation Resolution
Linked Issues Check ⚠️ Warning The PR partially addresses the objectives from LNK-4374 but falls short on a critical requirement. The changes successfully implement the Manager/Query structure (CensusConfigQueries interface and implementation, refactored CensusConfigManager with CreateAsync/UpdateAsync/DeleteAsync methods), introduce proper data models (CensusConfigModel, CreateCensusConfigModel, UpdateCensusConfigModel, SearchCensusConfigModel), and update the database layer with IDatabase abstraction. However, the linked issue explicitly requires "ensuring testability through integration and unit tests" with "all tests passing and meeting quality standards," and the PR provides no evidence that unit tests or integration tests have been added to cover these changes. The unchecked unit testing checkbox further confirms this gap.
Docstring Coverage ⚠️ Warning Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check ⚠️ Warning PR description is almost entirely a placeholder template with no actual implementation details provided by the author. Fill in all three template sections: provide a detailed overview of the changes (especially the Manager/Query redesign), describe the testing performed (which appears to be checked but undocumented), and update relevant documentation.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title "LNK-4374: Yearl Goals - Backend Overhaul - Census" is partially related to the changeset. It correctly identifies that this is a backend overhaul focused on the Census service, which aligns with the primary refactoring work being performed. The title refers to a real aspect of the changes (Census backend refactoring) but is somewhat broad and generic, reading more as an epic/issue reference than a specific PR summary. While the title conveys the general nature of the work, it does not clearly summarize the specific implementation details such as the introduction of the Manager/Query pattern or the data model restructuring. Note: There is a typo in the title ("Yearl" instead of "Yearly").
Out of Scope Changes Check ✅ Passed All code changes in this pull request are in scope for the Census data layer refactoring objective. The modifications span the Census module across interfaces (ICensusSchedulingRepository), repositories (CensusSchedulingRepository, Database, CensusConfigQueries), managers (CensusConfigManager), controllers (CensusConfigController), domain entities (CensusConfigEntity), models (CensusConfigModel, CreateCensusConfigModel, SearchCensusConfigModel, UpdateCensusConfigModel), services (ScheduleService), and dependency injection (Program.cs). All changes align with implementing the Manager/Query pattern and establishing proper data models for the Census service. No unrelated bug fixes, refactoring of other services, or out-of-scope enhancements are present.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch nvm/LNK-4374_CensusManagerQueries

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 15

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (7)
DotNet/Census/Domain/Context/CensusContext.cs (1)

49-57: Design-time factory likely won’t compile/run: UseSqlServer() requires a connection.

UseSqlServer without a connection string/DbConnection has no valid overload in EF Core. Provide a connection (e.g., env/config) or remove the factory.

- optionsBuilder.UseSqlServer();
+ var cs = Environment.GetEnvironmentVariable("EF_SQLSERVER_CONNECTION")
+          ?? "Server=(localdb)\\MSSQLLocalDB;Database=Census;Trusted_Connection=True;MultipleActiveResultSets=true";
+ optionsBuilder.UseSqlServer(cs);

Add a README note for EF tools to set EF_SQLSERVER_CONNECTION.

DotNet/Census/Application/Services/ScheduleService.cs (1)

81-84: Fix await on nullable Task (awaiting Scheduler?.Shutdown can NRE).

Awaiting a null Task throws. Guard explicitly.

- await Scheduler?.Shutdown(cancellationToken);
+ if (Scheduler != null)
+ {
+     await Scheduler.Shutdown(cancellationToken);
+ }
DotNet/Census/Program.cs (1)

261-279: Minor: fix typo in ProblemDetails message.

“assistence” → “assistance”.

- ctx.ProblemDetails.Detail = "An error occured in our API. Please use the trace id when requesting assistence.";
+ ctx.ProblemDetails.Detail = "An error occurred in our API. Please use the trace id when requesting assistance.";
DotNet/Census/Application/Repositories/Scheduling/CensusSchedulingRepository.cs (1)

39-47: async void swallows exceptions; return Task and await at call sites

Make CreateJobAndTrigger async Task; update interface and callers (e.g., ScheduleService).

-public async void CreateJobAndTrigger(CensusConfigModel facility, IScheduler scheduler)
+public async Task CreateJobAndTrigger(CensusConfigModel facility, IScheduler scheduler)
 {
     IJobDetail job = CreateJob(facility);
     await scheduler.AddJob(job, true);
     ITrigger trigger = CreateTrigger(facility.ScheduledTrigger, job.Key);
     await scheduler.ScheduleJob(trigger);
 }

Also update calls:

-    censusSchedulingRepo.CreateJobAndTrigger(facility, Scheduler);
+    await censusSchedulingRepo.CreateJobAndTrigger(facility, Scheduler);
DotNet/Census/Controllers/CensusConfigController.cs (3)

105-116: Sanitize path parameter facilityId before use.

Apply input sanitization for path params, same as FacilityController. Use HtmlInputSanitizer.Sanitize() at method start.

As per coding guidelines.

 public async Task<ActionResult<CensusConfigModel>> Get(string facilityId)
 {
+    facilityId = facilityId?.Sanitize();
     try
     {
         var result = await _censusConfigQueries.GetAsync(facilityId, HttpContext.RequestAborted);
 public async Task<IActionResult> Delete(string facilityId)
 {
+    facilityId = facilityId?.Sanitize();
     try
     {
         await _censusConfigManager.DeleteAsync(facilityId, HttpContext.RequestAborted);

Also applies to: 209-216


43-59: Sanitize body field censusConfig.FacilityId prior to validation.

Sanitize user‑supplied strings before checks and downstream calls.

As per coding guidelines.

 public async Task<IActionResult> Create(CensusConfigApiModel censusConfig)
 {
+    censusConfig.FacilityId = censusConfig.FacilityId?.Sanitize();
 public async Task<ActionResult<CensusConfigModel>> Put(CensusConfigApiModel censusConfig, string facilityId)
 {
+    facilityId = facilityId?.Sanitize();
+    censusConfig.FacilityId = censusConfig.FacilityId?.Sanitize();

Also applies to: 145-166


206-216: DELETE: fix response type mismatch; return 204; map KeyNotFoundException to 404.

Current attributes promise 204 but method returns 202 and maps all errors to 500. Align with REST and manager exceptions.

-[ProducesResponseType(StatusCodes.Status204NoContent)]
+[ProducesResponseType(StatusCodes.Status204NoContent)]
+[ProducesResponseType(StatusCodes.Status404NotFound)]
 [ProducesResponseType(StatusCodes.Status500InternalServerError)]
@@
 public async Task<IActionResult> Delete(string facilityId)
 {
+    facilityId = facilityId?.Sanitize();
     try
     {
         await _censusConfigManager.DeleteAsync(facilityId, HttpContext.RequestAborted);
-
-        return Accepted();
+        return NoContent();
     }
+    catch (KeyNotFoundException)
+    {
+        return NotFound();
+    }
     catch (Exception ex)
     {
         _logger.LogError(ex, "Exception encountered in CensusConfigController.Delete");
         return Problem(
             detail: "An error occurred while processing your request.",
             statusCode: StatusCodes.Status500InternalServerError
         );
     }
 }

Also applies to: 217-225

🧹 Nitpick comments (20)
DotNet/Census/Models/CreateCensusConfigModel.cs (1)

3-7: Add non-null enforcement and basic validation on creation DTO.

Both properties are non-nullable but not initialized; at runtime this risks nulls slipping into downstream layers. Prefer C# 11 required members or default suppressions; optionally add DataAnnotations for invariants.

-    public class CreateCensusConfigModel
+    public class CreateCensusConfigModel
     {
-        public string FacilityId { get; set; }
-        public string ScheduledTrigger { get; set; }
+        public required string FacilityId { get; set; }
+        public required string ScheduledTrigger { get; set; }
     }

If the project isn't on C# 11, replace required with:

- public string FacilityId { get; set; }
+ public string FacilityId { get; set; } = default!;
- public string ScheduledTrigger { get; set; }
+ public string ScheduledTrigger { get; set; } = default!;

Consider validating ScheduledTrigger as a Quartz cron (e.g., in manager/service).

DotNet/Census/Domain/Context/CensusContext.cs (1)

41-46: Harden JSON conversion against nulls and serializer differences.

Deserializing null/empty strings will throw. Add null/empty guards and a ValueComparer for proper change tracking of dictionaries.

-    .HasConversion(
-        v => JsonSerializer.Serialize(v, new JsonSerializerOptions()),
-        v => JsonSerializer.Deserialize<Dictionary<string, string>>(v, new JsonSerializerOptions())
-);
+    .HasConversion(
+        v => JsonSerializer.Serialize(v ?? new Dictionary<string,string>(), new JsonSerializerOptions()),
+        v => string.IsNullOrWhiteSpace(v)
+            ? new Dictionary<string,string>()
+            : JsonSerializer.Deserialize<Dictionary<string, string>>(v, new JsonSerializerOptions()) ?? new Dictionary<string,string>()
+)
+    .Metadata.SetValueComparer(new ValueComparer<Dictionary<string,string>>(
+        (d1, d2) => d1!.OrderBy(kv => kv.Key).SequenceEqual(d2!.OrderBy(kv => kv.Key)),
+        d => d.Aggregate(0, (a, kv) => HashCode.Combine(a, kv.Key, kv.Value)),
+        d => d.ToDictionary(kv => kv.Key, kv => kv.Value)));
DotNet/Census/Application/Models/CensusConfigApiModel.cs (1)

6-12: Tighten model validation; remove class-level BindRequired.

BindRequired is intended for properties; on a class it’s ignored. Keep [Required] on properties and enforce non-nullability to avoid CS8618 and runtime nulls.

- [BindRequired]
- public class CensusConfigApiModel
+ public class CensusConfigApiModel
 {
-    [Required]
-    public string FacilityId { get; set; }
-    [Required]
-    public string ScheduledTrigger { get; set; }
+    [Required] public string FacilityId { get; set; } = default!;
+    [Required] public string ScheduledTrigger { get; set; } = default!;
}

Optionally add cron validation (e.g., custom CronExpressionAttribute) and model-level FluentValidation for richer feedback.

DotNet/Census/Models/UpdateCensusConfigModel.cs (1)

3-7: Mirror non-null/validation on update DTO.

Keep invariants consistent between create and update.

-    public class UpdateCensusConfigModel
+    public class UpdateCensusConfigModel
     {
-        public string FacilityId { get; set; }
-        public string ScheduledTrigger { get; set; }
+        public required string FacilityId { get; set; }
+        public required string ScheduledTrigger { get; set; }
     }

If not on C# 11, initialize with = default! and validate in manager/service.

DotNet/Census/Application/Services/ScheduleService.cs (2)

20-26: Remove unused _topicJobs.

Not used elsewhere; keep code lean.

-    private static Dictionary<string, Type> _topicJobs = new Dictionary<string, Type>();
-    static ScheduleService()
-    {
-        _topicJobs.Add(KafkaTopic.PatientCensusScheduled.ToString(), typeof(SchedulePatientListRetrieval));
-    }
+    // Removed unused _topicJobs

42-75: Add unit tests for StartAsync scheduling flow.

  • Paginates through all results.
  • Handles repository exceptions per facility without aborting.
  • Awaits CreateJobAndTriggerAsync calls.
    Use xUnit + Moq; avoid any real Quartz/DB calls. I can scaffold tests if helpful.
DotNet/Census/Domain/Entities/CensusConfigEntity.cs (1)

9-13: Align naming and nullability; verify interceptor overlap.

  • Prefer FacilityId (Id casing) for consistency with models and LINQ, or map a column name if DB needs FacilityID. Renaming will require a migration.
- public string FacilityID { get; set; }
+ public string FacilityId { get; set; } = default!;
  • Initialize non-nullable strings or mark as required to avoid CS8618:
- public string ScheduledTrigger { get; set; }
+ public string ScheduledTrigger { get; set; } = default!;
  • CreateDate default + UpdateBaseEntityInterceptor: confirm no double-setting or drift with UTC handling.

Please confirm UpdateBaseEntityInterceptor doesn’t expect a specific base type and correctly sets CreateDate/ModifyDate for this entity.

DotNet/Census/Program.cs (1)

154-158: Repository lifetime should match DbContext (Scoped).

IEntityRepository depends on a Scoped DbContext; make the repository Scoped to avoid accidental cross-scope usage.

- builder.Services.AddTransient<IEntityRepository<CensusConfigEntity>, EntityRepository<CensusConfigEntity, CensusContext>>();
+ builder.Services.AddScoped<IEntityRepository<CensusConfigEntity>, EntityRepository<CensusConfigEntity, CensusContext>>();
DotNet/Census/Models/CensusConfigModel.cs (1)

5-25: Mapping looks correct; consider non-null enforcement.

FromDomain mapping is clear and correct. To quell CS8618 and ensure invariants, mark strings as required or initialize.

- public string FacilityId { get; set; }
- public string ScheduledTrigger { get; set; }
+ public required string FacilityId { get; set; }
+ public required string ScheduledTrigger { get; set; }
DotNet/Census/Domain/Repositories/Database.cs (2)

10-15: Tighten DB abstraction: tokens and immutability

  • SaveChangesAsync should accept CancellationToken to propagate cancellations.
  • Make CensusConfigRepository get-only to prevent mutation after construction.
-public interface IDatabase
+public interface IDatabase
 {
-    IEntityRepository<CensusConfigEntity> CensusConfigRepository { get; set; }
-
-    Task SaveChangesAsync();
+    IEntityRepository<CensusConfigEntity> CensusConfigRepository { get; }
+    Task SaveChangesAsync(CancellationToken token = default);
     Task<IDbContextTransaction> BeginTransactionAsync(CancellationToken token);
     Task CommitTransactionAsync(CancellationToken token);
     Task RollbackTransactionAsync(CancellationToken token);
 }
 public class Database : IDatabase
 {
     private readonly CensusContext _dbContext;
-    public IEntityRepository<CensusConfigEntity> CensusConfigRepository { get; set; }
+    public IEntityRepository<CensusConfigEntity> CensusConfigRepository { get; }
 
     public Database(
         CensusContext context,
-        IEntityRepository<CensusConfigEntity> queryConfigurationRepository)
+        IEntityRepository<CensusConfigEntity> censusConfigRepository)
     {
         _dbContext = context;
-        CensusConfigRepository = queryConfigurationRepository;
+        CensusConfigRepository = censusConfigRepository;
     }
 
-    public async Task SaveChangesAsync()
+    public async Task SaveChangesAsync(CancellationToken token = default)
     {
-        await _dbContext.SaveChangesAsync();
+        await _dbContext.SaveChangesAsync(token);
     }

35-48: Consistency of transaction APIs across layers

Managers mix IDatabase transaction methods and repository-level Start/Commit/Rollback. Pick one pattern (prefer DbContext/IDatabase) to avoid nested or inconsistent transactions.

Would you like a follow-up PR note to standardize transaction usage in CensusConfigManager? Based on learnings.

DotNet/Census/Domain/Queries/CensusConfigQueries.cs (2)

54-59: Prefer EF.Property for dynamic sorting; whitelist known columns

Reflection + Convert to object can force client eval. EF.Property translates reliably and avoids boxing.

-        query = model.SortOrder switch
+        // Allowlisted sort columns only
+        var sortKey = SetSortKeyForCensusConfig(sanitizedSortBy);
+        query = model.SortOrder switch
         {
-            SortOrder.Ascending => query.OrderBy(SetSortBy<CensusConfigEntity>(model.SortBy)),
-            SortOrder.Descending => query.OrderByDescending(SetSortBy<CensusConfigEntity>(model.SortBy)),
+            SortOrder.Ascending => query.OrderBy(e => EF.Property<object>(e, sortKey)),
+            SortOrder.Descending => query.OrderByDescending(e => EF.Property<object>(e, sortKey)),
             _ => query
         };

Add below (replace the generic SetSortBy):

-    private Expression<Func<T, object>> SetSortBy<T>(string? sortBy)
-    {
-        var type = typeof(T);
-        var inputSortBy = sortBy?.Trim();
-        string sortKey = "Id"; // default
-        if (!string.IsNullOrEmpty(inputSortBy))
-        {
-            var prop = type.GetProperty(inputSortBy, BindingFlags.Public | BindingFlags.Instance | BindingFlags.IgnoreCase);
-            if (prop != null)
-            {
-                sortKey = prop.Name;
-            }
-        }
-        var parameter = Expression.Parameter(type, "p");
-        var property = Expression.Property(parameter, sortKey);
-        var converted = Expression.Convert(property, typeof(object));
-        return Expression.Lambda<Func<T, object>>(converted, parameter);
-    }
+    private static string SetSortKeyForCensusConfig(string? sortBy)
+    {
+        var input = sortBy?.Trim();
+        // allowlist; names must match CensusConfigEntity properties
+        var allowed = new HashSet<string>(StringComparer.OrdinalIgnoreCase)
+        { "Id", "FacilityID", "ScheduledTrigger", "CreateDate", "ModifyDate" };
+        return !string.IsNullOrEmpty(input) && allowed.Contains(input) ? allowed.First(k => k.Equals(input, StringComparison.OrdinalIgnoreCase)) : "Id";
+    }

28-34: GetAsync: consider direct query, not paged call

For a single facility lookup, query directly with AsNoTracking(). Also decide whether uniqueness is enforced; SingleOrDefault will throw on duplicates.

-    return (await PagedSearchAsync(new SearchCensusConfigModel
-    {
-        FacilityId = facilityId,
-    }, cancellationToken)).Records.SingleOrDefault();
+    return await _dbContext.CensusConfigs
+        .AsNoTracking()
+        .Where(c => c.FacilityID == facilityId)
+        .Select(c => new CensusConfigModel { Id = c.Id, FacilityId = c.FacilityID, ScheduledTrigger = c.ScheduledTrigger, CreateDate = c.CreateDate, ModifyDate = c.ModifyDate })
+        .SingleOrDefaultAsync(cancellationToken);

Confirm DB uniqueness on CensusConfigEntity.FacilityID to justify SingleOrDefault; otherwise use FirstOrDefault.

DotNet/Census/Application/Repositories/Scheduling/CensusSchedulingRepository.cs (2)

28-36: Reduce JobData payload; pass only what the Job needs

Storing whole CensusConfigModel in JobDataMap can cause serialization overhead. Prefer FacilityId and read details at execution.

-    jobDataMap.Put(CensusConstants.Scheduler.Facility, facility);
+    jobDataMap.Put(CensusConstants.Scheduler.Facility, facility.FacilityId);

In SchedulePatientListRetrieval, resolve config via ICensusConfigQueries using FacilityId.


13-18: Idempotent add: consider deleting the Job entity too

You unschedule triggers but keep the Job. Since AddJob(..., replace: true) is used, this is mostly safe; consider scheduler.DeleteJob(jobKey) after unscheduling to avoid stale job data.

Would you like a follow-up patch to delete the job after unscheduling?

DotNet/Census/Controllers/CensusConfigController.cs (4)

1-1: PR hygiene: please add testing + docs summary.

This PR isn’t TECH_DEBT; per guidelines, include “what testing was performed” and update any impacted docs. A brief high‑level overview would also help reviewers.

As per coding guidelines.


102-104: GET metadata should include 404.

Method returns NotFound(); add ProducesResponseType 404.

 [ProducesResponseType(StatusCodes.Status200OK, Type = typeof(CensusConfigModel))]
+[ProducesResponseType(StatusCodes.Status404NotFound)]
 [ProducesResponseType(StatusCodes.Status500InternalServerError)]

31-33: Minor: fix summary typo.

“o given” → “a given”.


43-59: Unit tests to add (xUnit + Moq).

  • Create: 400 on missing FacilityId; 400 on missing ScheduledTrigger; 400 invalid cron; 409 when duplicate; 201 with Location on success; 404 when MissingTenantConfigurationException.
  • Get: 200 with body; 404 when null.
  • Put: 400 when path/body mismatch; 400 missing ScheduledTrigger; 400 invalid cron; 404 when not found; 200 on success.
  • Delete: 204 on success; 404 on KeyNotFoundException.
  • Sanitization: verify facilityId is sanitized before dependencies receive it.

As per coding guidelines.

Also applies to: 145-166

DotNet/Census/Domain/Managers/CensusConfigManager.cs (1)

38-45: Optional: Validate cron at domain layer.

Controller validates cron, but callers other than MVC could bypass it. Consider validating ScheduledTrigger in manager to keep invariants local.

Also applies to: 80-90

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 2cac631 and 5250330.

📒 Files selected for processing (15)
  • DotNet/Census/Application/Interfaces/ICensusSchedulingRepository.cs (1 hunks)
  • DotNet/Census/Application/Models/CensusConfigApiModel.cs (1 hunks)
  • DotNet/Census/Application/Repositories/Scheduling/CensusSchedulingRepository.cs (4 hunks)
  • DotNet/Census/Application/Services/ScheduleService.cs (2 hunks)
  • DotNet/Census/Controllers/CensusConfigController.cs (8 hunks)
  • DotNet/Census/Domain/Context/CensusContext.cs (1 hunks)
  • DotNet/Census/Domain/Entities/CensusConfigEntity.cs (1 hunks)
  • DotNet/Census/Domain/Managers/CensusConfigManager.cs (1 hunks)
  • DotNet/Census/Domain/Queries/CensusConfigQueries.cs (1 hunks)
  • DotNet/Census/Domain/Repositories/Database.cs (1 hunks)
  • DotNet/Census/Models/CensusConfigModel.cs (1 hunks)
  • DotNet/Census/Models/CreateCensusConfigModel.cs (1 hunks)
  • DotNet/Census/Models/SearchCensusConfigModel.cs (1 hunks)
  • DotNet/Census/Models/UpdateCensusConfigModel.cs (1 hunks)
  • DotNet/Census/Program.cs (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.cs

⚙️ CodeRabbit configuration file

**/*.cs: The HtmlInputSanitizer class's Sanitize() and SanitizeAndRemove() methods should be used when dealing with string query parameters from REST requests.

Files:

  • DotNet/Census/Models/CreateCensusConfigModel.cs
  • DotNet/Census/Application/Models/CensusConfigApiModel.cs
  • DotNet/Census/Domain/Entities/CensusConfigEntity.cs
  • DotNet/Census/Program.cs
  • DotNet/Census/Models/CensusConfigModel.cs
  • DotNet/Census/Controllers/CensusConfigController.cs
  • DotNet/Census/Application/Interfaces/ICensusSchedulingRepository.cs
  • DotNet/Census/Models/UpdateCensusConfigModel.cs
  • DotNet/Census/Domain/Context/CensusContext.cs
  • DotNet/Census/Application/Services/ScheduleService.cs
  • DotNet/Census/Models/SearchCensusConfigModel.cs
  • DotNet/Census/Application/Repositories/Scheduling/CensusSchedulingRepository.cs
  • DotNet/Census/Domain/Queries/CensusConfigQueries.cs
  • DotNet/Census/Domain/Repositories/Database.cs
  • DotNet/Census/Domain/Managers/CensusConfigManager.cs
**

⚙️ CodeRabbit configuration file

**: Pull requests that have "TECH_DEBT" in the title should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates,
and logging improvements. These TECH_DEBT PRs must not affect core functionality. All PRs that are not considered technical debt must include information on what
testing was performed in the description of the PR. If it does not, ask the author to provide details on what testing was performed.
When reviewing code, suggest unit tests using XUnit in the following scenarios:

  • If/Else or Switch/Case blocks are introduced or modified — ensure each branch has a corresponding unit test.
  • Logic that depends on service or interface configuration — suggest tests to validate different implementations are correctly resolved.
  • No network activity (HTTP calls, sockets, etc.) should appear in unit tests. Recommend using mocks (via Moq) for any external communication.
    Large unit tests should be avoided; keeping unit tests small and focused on targeted business logic (i.e. string sanitization)

**: Pull requests that have DOCS in the title should only contain changes related to documentation within the /docs folder or in .md files through-out the code-base. The description
of the PR should specify what documentation was updated. Documentation updates should use EventCatalog.dev structure, where service-specific functionality should be described
in the service's index.mdx (i.e. /services/XXX/index.mdx or /domains/XXX/services/YYY/index.mdx). Configurations that are shared by multiple services should be
reflected in the /docs/docs/config files.

Files:

  • DotNet/Census/Models/CreateCensusConfigModel.cs
  • DotNet/Census/Application/Models/CensusConfigApiModel.cs
  • DotNet/Census/Domain/Entities/CensusConfigEntity.cs
  • DotNet/Census/Program.cs
  • DotNet/Census/Models/CensusConfigModel.cs
  • DotNet/Census/Controllers/CensusConfigController.cs
  • DotNet/Census/Application/Interfaces/ICensusSchedulingRepository.cs
  • DotNet/Census/Models/UpdateCensusConfigModel.cs
  • DotNet/Census/Domain/Context/CensusContext.cs
  • DotNet/Census/Application/Services/ScheduleService.cs
  • DotNet/Census/Models/SearchCensusConfigModel.cs
  • DotNet/Census/Application/Repositories/Scheduling/CensusSchedulingRepository.cs
  • DotNet/Census/Domain/Queries/CensusConfigQueries.cs
  • DotNet/Census/Domain/Repositories/Database.cs
  • DotNet/Census/Domain/Managers/CensusConfigManager.cs
🧬 Code graph analysis (8)
DotNet/Census/Program.cs (4)
DotNet/Shared/Domain/Repositories/Implementations/EntityRepository.cs (2)
  • EntityRepository (11-274)
  • EntityRepository (15-18)
DotNet/Census/Domain/Context/CensusContext.cs (4)
  • CensusContext (10-59)
  • CensusContext (17-19)
  • CensusContext (21-21)
  • CensusContext (51-57)
DotNet/Census/Application/Repositories/CensusEntityRepository.cs (2)
  • CensusEntityRepository (7-13)
  • CensusEntityRepository (9-12)
DotNet/Census/Domain/Queries/CensusConfigQueries.cs (2)
  • CensusConfigQueries (19-107)
  • CensusConfigQueries (23-26)
DotNet/Census/Controllers/CensusConfigController.cs (5)
DotNet/Tenant/Controllers/FacilityController.cs (8)
  • ProducesResponseType (103-138)
  • ProducesResponseType (144-179)
  • ProducesResponseType (187-232)
  • ProducesResponseType (240-271)
  • ProducesResponseType (280-335)
  • ProducesResponseType (343-379)
  • ProducesResponseType (387-478)
  • ProducesResponseType (480-570)
DotNet/Census/Models/CensusConfigModel.cs (2)
  • CensusConfigModel (5-26)
  • CensusConfigModel (13-25)
DotNet/Census/Domain/Managers/CensusConfigManager.cs (6)
  • Task (13-13)
  • Task (14-14)
  • Task (15-15)
  • Task (38-78)
  • Task (80-122)
  • Task (124-134)
DotNet/Census/Models/CreateCensusConfigModel.cs (1)
  • CreateCensusConfigModel (3-7)
DotNet/Census/Models/UpdateCensusConfigModel.cs (1)
  • UpdateCensusConfigModel (3-7)
DotNet/Census/Application/Interfaces/ICensusSchedulingRepository.cs (2)
DotNet/Census/Models/CensusConfigModel.cs (2)
  • CensusConfigModel (5-26)
  • CensusConfigModel (13-25)
DotNet/Census/Application/Repositories/Scheduling/CensusSchedulingRepository.cs (1)
  • CreateJobAndTrigger (39-47)
DotNet/Census/Application/Services/ScheduleService.cs (3)
DotNet/Census/Models/SearchCensusConfigModel.cs (1)
  • SearchCensusConfigModel (5-12)
DotNet/Census/Application/Interfaces/ICensusSchedulingRepository.cs (1)
  • CreateJobAndTrigger (17-17)
DotNet/Census/Application/Repositories/Scheduling/CensusSchedulingRepository.cs (1)
  • CreateJobAndTrigger (39-47)
DotNet/Census/Application/Repositories/Scheduling/CensusSchedulingRepository.cs (4)
DotNet/Census/Application/Interfaces/ICensusSchedulingRepository.cs (6)
  • Task (9-9)
  • Task (11-11)
  • Task (13-13)
  • Task (15-15)
  • CreateJobAndTrigger (17-17)
  • IJobDetail (19-19)
DotNet/Census/Application/Services/ScheduleService.cs (3)
  • Task (42-75)
  • Task (77-79)
  • Task (81-84)
DotNet/Census/Domain/Managers/CensusConfigManager.cs (6)
  • Task (13-13)
  • Task (14-14)
  • Task (15-15)
  • Task (38-78)
  • Task (80-122)
  • Task (124-134)
DotNet/Census/Models/CensusConfigModel.cs (2)
  • CensusConfigModel (5-26)
  • CensusConfigModel (13-25)
DotNet/Census/Domain/Queries/CensusConfigQueries.cs (5)
DotNet/Census/Models/CensusConfigModel.cs (2)
  • CensusConfigModel (5-26)
  • CensusConfigModel (13-25)
DotNet/Census/Models/SearchCensusConfigModel.cs (1)
  • SearchCensusConfigModel (5-12)
DotNet/Census/Domain/Context/CensusContext.cs (4)
  • CensusContext (10-59)
  • CensusContext (17-19)
  • CensusContext (21-21)
  • CensusContext (51-57)
DotNet/Census/Application/Services/ScheduleService.cs (3)
  • Task (42-75)
  • Task (77-79)
  • Task (81-84)
DotNet/Census/Domain/Managers/CensusConfigManager.cs (5)
  • Task (13-13)
  • Task (14-14)
  • Task (15-15)
  • Task (38-78)
  • Task (80-122)
DotNet/Census/Domain/Repositories/Database.cs (2)
DotNet/Census/Domain/Context/CensusContext.cs (4)
  • CensusContext (10-59)
  • CensusContext (17-19)
  • CensusContext (21-21)
  • CensusContext (51-57)
DotNet/Census/Domain/Managers/CensusConfigManager.cs (5)
  • Task (13-13)
  • Task (14-14)
  • Task (15-15)
  • Task (38-78)
  • Task (80-122)
DotNet/Census/Domain/Managers/CensusConfigManager.cs (6)
DotNet/Census/Models/CensusConfigModel.cs (2)
  • CensusConfigModel (5-26)
  • CensusConfigModel (13-25)
DotNet/Census/Models/CreateCensusConfigModel.cs (1)
  • CreateCensusConfigModel (3-7)
DotNet/Census/Models/UpdateCensusConfigModel.cs (1)
  • UpdateCensusConfigModel (3-7)
DotNet/Census/Application/Interfaces/ICensusSchedulingRepository.cs (4)
  • Task (9-9)
  • Task (11-11)
  • Task (13-13)
  • Task (15-15)
DotNet/Census/Application/Repositories/Scheduling/CensusSchedulingRepository.cs (4)
  • Task (13-18)
  • Task (65-93)
  • Task (136-148)
  • Task (150-168)
DotNet/Shared/Domain/Repositories/Implementations/EntityRepository.cs (2)
  • Update (44-47)
  • Remove (49-52)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Unit Tests for Java
  • GitHub Check: Unit Tests for DotNet
  • GitHub Check: Integration Tests
  • GitHub Check: Smoke Test with Docker Compose
  • GitHub Check: Analyze (csharp)
  • GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (2)
DotNet/Census/Domain/Context/CensusContext.cs (1)

37-40: Verify computed column references actual mapped column names.

The expression "CONCAT(FacilityId, '-', CensusDateTime)" must match the actual mapped column names on PatientCensusHistoricEntity. Mismatch (e.g., FacilityID vs FacilityId) will fail at migration/runtime.

Please confirm the entity's column/property names or update the expression accordingly.

DotNet/Census/Program.cs (1)

51-58: PR hygiene: please provide testing details (non-TECH_DEBT).

Per guidelines, non-TECH_DEBT PRs must include what testing was performed. Please add:

  • Unit/integration tests added/updated (list).
  • Manual test scenarios and environments.
  • Any migration/rollback validation and data backfills.

I can propose an xUnit test checklist targeting CensusConfigQueries pagination/sorting and ScheduleService scheduling flow.

Comment thread DotNet/Census/Application/Interfaces/ICensusSchedulingRepository.cs Outdated
Comment thread DotNet/Census/Application/Repositories/Scheduling/CensusSchedulingRepository.cs Outdated
Comment thread DotNet/Census/Application/Services/ScheduleService.cs
Comment thread DotNet/Census/Application/Services/ScheduleService.cs
Comment thread DotNet/Census/Application/Services/ScheduleService.cs Outdated
Comment thread DotNet/Census/Domain/Managers/CensusConfigManager.cs
Comment thread DotNet/Census/Domain/Queries/CensusConfigQueries.cs
Comment thread DotNet/Census/Domain/Queries/CensusConfigQueries.cs
Comment thread DotNet/Census/Models/SearchCensusConfigModel.cs
Comment thread DotNet/Census/Models/SearchCensusConfigModel.cs
@nvmLantana
nvmLantana requested review from a team as code owners October 30, 2025 16:11
@nvmLantana nvmLantana closed this Mar 6, 2026
@nvmLantana
nvmLantana deleted the nvm/LNK-4374_CensusManagerQueries branch March 6, 2026 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant