Improve exception handling and refactor unused methods in changes

[hahn-kev]

Right now if an edit is called on a new entity it will throw System.NotSupportedException: type JsonPatchChange1 does not support NewEntity..., I fixed how the type name is generated so it will print the generic type eg JsonPatchChange<Entry> and also to include the commit and entity Id in the error message to aid debugging.

Summary by CodeRabbit

• Refactor
  • Enhanced internal change handling and improved error messaging with additional diagnostic context

[coderabbitai]

Warning

Review limit reached

@myieye, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 27 minutes and 6 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c4870b8e-703a-45fa-b8dd-0fd72c596f51

📥 Commits

Reviewing files that changed from the base of the PR and between acb3c08 and da07c47.

📒 Files selected for processing (1)

• src/SIL.Harmony/Changes/EditChange.cs

📝 Walkthrough

Walkthrough

Two change-handling classes are updated to improve error diagnostics and implement proper no-op behavior. CreateChange.ApplyChange now returns default with a clarifying comment; EditChange adds EF Core Infrastructure support and enhances exception messages with ShortDisplayName() and contextual entity/commit IDs.

Changes

Change Class Improvements

Layer / File(s)	Summary
CreateChange ApplyChange no-op implementation src/SIL.Harmony/Changes/CreateChange.cs	ApplyChange override returns default instead of throwing NotSupportedException, with a comment explaining the method is bypassed by base class logic.
EditChange error diagnostics src/SIL.Harmony/Changes/EditChange.cs	using Microsoft.EntityFrameworkCore.Infrastructure; is added, and NewEntity exception message is updated to use ShortDisplayName() and include CommitId and EntityId for better error context.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

• sillsdev/harmony#54: The main PR's change to stop CreateChange<T>.ApplyChange from throwing (since CreateChange is now skipped by the new capability-based control flow) is directly tied to this PR's Change<T>/SnapshotWorker refactor that adds SupportsApplyChange/SupportsNewEntity and alters apply/new behavior for deleted vs. undeleted entities.

Suggested reviewers

• jasonleenaylor

Poem

🐰 A rabbit's refrain:
Two changes so small, yet precise and clear,
No throwing, just returns—a better frontier!
With ShortDisplayName and context so bright,
Error messages glow with diagnostic light. 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% 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
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Improve exception handling and refactor unused methods in changes' is partially related to the changeset. The exception handling improvement is directly evident in the changes (better error messages with GetType().ShortDisplayName() and additional context), but the claim about refactoring unused methods is unclear since the changes primarily involve modifying NotSupportedException behavior rather than removing or refactoring unused code.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch improve-new-on-edit-failure

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

src/SIL.Harmony/Changes/CreateChange.cs (1)

1-1: ⚡ Quick win

Remove unused import.

The Microsoft.EntityFrameworkCore.Infrastructure namespace is imported but not used in this file. The ShortDisplayName() extension method is only used in EditChange.cs.

🧹 Proposed fix

-using Microsoft.EntityFrameworkCore.Infrastructure;
-
 namespace SIL.Harmony.Changes;

🤖 Prompt for 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.

In `@src/SIL.Harmony/Changes/CreateChange.cs` at line 1, Remove the unused using
directive by deleting the "using Microsoft.EntityFrameworkCore.Infrastructure;"
statement from CreateChange.cs; keep all other usings and code intact (the
ShortDisplayName() extension is only needed in EditChange.cs, so no other
changes are required).

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

Nitpick comments:
In `@src/SIL.Harmony/Changes/CreateChange.cs`:
- Line 1: Remove the unused using directive by deleting the "using
Microsoft.EntityFrameworkCore.Infrastructure;" statement from CreateChange.cs;
keep all other usings and code intact (the ShortDisplayName() extension is only
needed in EditChange.cs, so no other changes are required).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 802ab686-7334-4694-a341-3c3b1857c395

📥 Commits

Reviewing files that changed from the base of the PR and between 31dc78b and acb3c08.

📒 Files selected for processing (2)

• src/SIL.Harmony/Changes/CreateChange.cs
• src/SIL.Harmony/Changes/EditChange.cs

[myieye]

I've been wanting to fix the grammar/typo in EditChange's exception message for a while.
Now was just the right time 😄.