[svermaak]

Follow-up to #1887, implementing option C as agreed in #1867 (comment).

Fixes #1867

The problem

When two GET requests in one batch resolve to the same key, MergeBatchResultsWithModel merges the second model into the first and marks the duplicate as deleted. Using that duplicate afterwards reported:

This model instance was deleted, you can't use it anymore

Nothing was deleted. The caller goes looking for a delete that never happened, which is what cost the reporter of #1867 the debugging time.

The change

RemoveFromParentCollection now records whether the removal came from the batch consolidation, and CheckDeleted reports that case through a dedicated ErrorType.InstanceWasConsolidatedInBatch as you asked for. The regular Delete / RemoveFromModel path calls the parameterless overload and keeps its existing error type and message.

On the breaking change you were willing to accept: it turns out we don't have to take one. ErrorType has no explicit values, so adding the member next to InstanceWasDeleted would renumber the 14 members after it, which silently shifts the values seen by anything compiled against an older PnP.Core. Appending it instead keeps every existing value stable, so this is purely additive. That is the only reason the new member sits at the bottom of the enum rather than with its relatives, and there is a comment there saying so.

Before and after

Two GetByIdBatchAsync calls for the same list in one batch, with different projections, then reading Title off the second instance. Run against a real tenant.

Before:

After:

Tests

Two new tests in BatchClientTests, offline by design like the existing ones in that region:

• ConsolidatedInBatchInstanceExplainsItWasConsolidated - the consolidated instance reports the new error type and message
• DeletedInstanceStillExplainsItWasDeleted - a genuinely deleted instance keeps the old error type and message, so neither can leak into the delete path

I mutation tested the first one: pointing the consolidation throw back at ErrorType.InstanceWasDeleted makes it fail and leaves the second passing.

Full offline suite on this branch: 1922 passed, 7 failed, 5 skipped. The 7 are pre-existing failures unrelated to this change (TimeZoneConversionTests, TimeZoneUtcToLocalTimeTest, TimeZoneUtcToLocalTime2Test, TestSPOAccessToken, TestGraphAccessToken, RepeatedCloneOffLineTest, SchedulePagePublish), none of which touch batching or model state.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.64%. Comparing base (63545f3) to head (a8b8ea5).
⚠️ Report is 3242 commits behind head on dev.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff             @@
##              dev    #1922       +/-   ##
===========================================
- Coverage   82.42%   62.64%   -19.79%     
===========================================
  Files         416     1296      +880     
  Lines       28590   108660    +80070     
  Branches        0     9731     +9731     
===========================================
+ Hits        23565    68065    +44500     
- Misses       5025    38635    +33610     
- Partials        0     1960     +1960     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.