fix(clinical): detect duplicate schedules by SQL error number by rlorenzo · Pull Request #229 · ucdavis/VIPER

rlorenzo · 2026-06-17T01:14:19Z

What

In ScheduleEditService.AddInstructorAsync, the post-SaveChanges catch decides whether a DB failure means "instructor already scheduled" by SQL Server error number (2627 unique-constraint, 2601 duplicate-key-in-unique-index) instead of substring-matching the exception message.

private static bool IsDuplicateKeyViolation(Exception ex)
{
    var sqlEx = ex as SqlException ?? ex.InnerException as SqlException;
    return sqlEx is not null
        && sqlEx.Errors.Cast<SqlError>().Any(e => e.Number is 2627 or 2601);
}

Why

The old check matched "duplicate" / "unique" / "constraint" / "violation of primary key" against message.ToLower(). Two problems:

Latent bug: other constraint failures — foreign-key or check-constraint violations (error 547, whose message contains "constraint") — were mis-reported as "already scheduled." Now only true duplicate/unique violations get that message; everything else falls through to the generic "database operation failed" message.
Locale fragility: ToLower() plus English error-text matching is culture-sensitive (Turkish-I) and breaks if SQL messages are localized. Error numbers are stable and locale-invariant.

Notes

Behavior-preserving for the real duplicate case; only re-routes non-duplicate constraint errors to the correct (generic) message.
The duplicate pre-check (before SaveChanges) is unchanged; AddInstructorAsync_WithScheduleConflicts and all 2084 tests pass.
Supersedes the message-matching helper currently on Development via refactor(codeql): decompose complex conditions + harden Vite web-root check #227; the overlap will be resolved (keeping this version) at the Development merge.

codecov-commenter · 2026-06-17T01:15:00Z

Bundle Report

Bundle size has no change ✅

codecov-commenter · 2026-06-17T01:15:40Z

Codecov Report

❌ Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 44.53%. Comparing base (1a0996e) to head (b9765ea).

Files with missing lines	Patch %	Lines
.../ClinicalScheduler/Services/ScheduleEditService.cs	83.33%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #229      +/-   ##
==========================================
+ Coverage   44.51%   44.53%   +0.01%     
==========================================
  Files         895      895              
  Lines       51760    51759       -1     
  Branches     4827     4828       +1     
==========================================
+ Hits        23041    23050       +9     
+ Misses      28149    28138      -11     
- Partials      570      571       +1

Flag	Coverage Δ
backend	`44.69% <83.33%> (+0.01%)`	⬆️
frontend	`41.32% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

Copilot

Pull request overview

Refines error handling in the Clinical Scheduler’s ScheduleEditService.AddInstructorAsync so that “already scheduled” is only returned for true SQL Server duplicate/unique key violations, using stable SQL error numbers instead of locale-dependent substring matching on exception messages.

Changes:

Replaces exception message substring matching with SQL Server error-number detection (2627, 2601) to identify duplicate-key violations.
Adds a focused helper (IsDuplicateKeyViolation) to unwrap EF/SQL exceptions and check SqlException.Errors.

rlorenzo · 2026-06-26T04:11:34Z

@coderabbitai full review

coderabbitai · 2026-06-26T04:11:38Z

✅ Action performed

Full review finished.

coderabbitai · 2026-06-26T04:13:58Z

📝 Walkthrough

Walkthrough

AddInstructorAsync now detects SQL Server duplicate-key violations by error number instead of exception-message text, returning the existing “already scheduled” error for those cases and a generic database failure for other exceptions.

Changes

Duplicate-key error classification

Layer / File(s)	Summary
Duplicate-key classification `web/Areas/ClinicalScheduler/Services/ScheduleEditService.cs`	`AddInstructorAsync` now calls `IsDuplicateKeyViolation` inside its database-exception catch path, and the new helper checks `SqlException.Errors` for SQL Server error numbers `2627` and `2601`.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

ucdavis/VIPER#193: Related ScheduleEditService exception-handling changes around SQL-specific duplicate/unique key handling.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: duplicate schedule detection now uses SQL error numbers.
Description check	✅ Passed	The description matches the code change and explains the SQL error-number-based duplicate detection.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/clinical-duplicate-key-detection

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

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@web/Areas/ClinicalScheduler/Services/ScheduleEditService.cs`:
- Around line 250-252: Add targeted tests for the exception classification logic
in IsDuplicateKeyViolation within ScheduleEditService so both branches are
covered. Create tests that feed the service a duplicate-key SqlException with
error numbers 2627 and 2601 and verify the duplicate-schedule path, plus a
non-duplicate SqlException/DbUpdateException that verifies the generic
database-failure path. Make sure the tests assert the downstream API error
mapping contract stays distinct for these two cases.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 109ef738-ea05-4f15-b163-60f3a498908e

📥 Commits

Reviewing files that changed from the base of the PR and between 1a0996e and 06ab238.

📒 Files selected for processing (1)

web/Areas/ClinicalScheduler/Services/ScheduleEditService.cs

Replace locale-fragile message-substring matching with SQL Server error numbers 2627/2601 (unique constraint / duplicate key in a unique index) to decide whether a save failure means the instructor is already scheduled. Other database errors (foreign-key, check constraints) now fall through to the generic message instead of mis-reporting "already scheduled", and the check no longer depends on English error text or a culture-specific ToLower().

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

+        public override int SaveChanges()
+            => SaveException is not null ? throw SaveException : base.SaveChanges();
+
+        public override Task<int> SaveChangesAsync(CancellationToken cancellationToken = default)
+            => SaveException is not null ? Task.FromException<int>(SaveException) : base.SaveChangesAsync(cancellationToken);


rlorenzo requested a review from Copilot June 17, 2026 01:14

Copilot started reviewing on behalf of rlorenzo June 17, 2026 01:14 View session

Copilot AI reviewed Jun 17, 2026

View reviewed changes

rlorenzo mentioned this pull request Jun 17, 2026

refactor(codeql): decompose complex conditions + harden Vite web-root check #227

Open

rlorenzo force-pushed the fix/clinical-duplicate-key-detection branch from 384a05b to 06ab238 Compare June 26, 2026 03:06

coderabbitai Bot reviewed Jun 26, 2026

View reviewed changes

Comment thread web/Areas/ClinicalScheduler/Services/ScheduleEditService.cs

rlorenzo force-pushed the fix/clinical-duplicate-key-detection branch from 06ab238 to b9765ea Compare June 26, 2026 05:30

rlorenzo requested a review from Copilot June 26, 2026 05:31

Copilot started reviewing on behalf of rlorenzo June 26, 2026 05:31 View session

Copilot AI reviewed Jun 26, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(clinical): detect duplicate schedules by SQL error number#229

fix(clinical): detect duplicate schedules by SQL error number#229
rlorenzo wants to merge 1 commit into
mainfrom
fix/clinical-duplicate-key-detection

rlorenzo commented Jun 17, 2026

Uh oh!

codecov-commenter commented Jun 17, 2026

Uh oh!

codecov-commenter commented Jun 17, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

rlorenzo commented Jun 26, 2026

Uh oh!

coderabbitai Bot commented Jun 26, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jun 26, 2026

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

rlorenzo commented Jun 17, 2026

What

Why

Notes

Uh oh!

codecov-commenter commented Jun 17, 2026

Bundle Report

Uh oh!

codecov-commenter commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

rlorenzo commented Jun 26, 2026

Uh oh!

coderabbitai Bot commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Jun 26, 2026

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov-commenter commented Jun 17, 2026 •

edited

Loading

coderabbitai Bot commented Jun 26, 2026 •

edited

Loading