Skip to content

Compare PT refund CaseData fields by value and fix mislabeled error#668

Open
StefanKert wants to merge 1 commit into
mainfrom
fix/pt-refund-casedata-value-equality
Open

Compare PT refund CaseData fields by value and fix mislabeled error#668
StefanKert wants to merge 1 commit into
mainfrom
fix/pt-refund-casedata-value-equality

Conversation

@StefanKert
Copy link
Copy Markdown
Member

Summary

Full refunds in QueuePT were rejected with Validation error []: EEEE_Full refund does not match the original invoice '<ref>'. ... (Field: cbCustomer) whenever the receipt's ftChargeItemCaseData (or ftReceiptCaseData / ftPayItemCaseData) was a non-null value such as "" on both sides.

Root cause

ftReceiptCaseData, ftChargeItemCaseData, and ftPayItemCaseData are typed as System.Object on the v2 ReceiptRequest, ChargeItem, and PayItem. RefundValidator compared them with !=, which on object is reference equality. After JSON deserialization both sides are distinct JsonElement instances, so equal values (e.g. "" on the stored original and "" on the incoming refund) always compared as different and the refund failed.

The check inside CompareChargeItems additionally reported the failure as (Field: cbCustomer) — a copy-paste of the cbCustomer mismatch label one method above — which made the error look like a customer-object problem rather than a CaseData problem.

Existing tests didn't catch this because none of them set the *CaseData fields, so both sides were null and null != null is false.

Fix

  • Add a CaseDataEquals(object?, object?) helper that treats null and "" as equivalent and otherwise compares via JsonSerializer.Serialize.
  • Replace the three reference comparisons in CompareReceiptRequest, CompareChargeItems, and ComparePayItems with CaseDataEquals.
  • Correct the Mismatch("cbCustomer") label inside CompareChargeItems to Mismatch("ftChargeItemCaseData").

Test plan

  • Re-run the failing scenario: refund a B2C invoice where the original was persisted with "ftChargeItemCaseData": "" and the refund payload also sends "". Validation should now pass.
  • Existing PT refund acceptance/unit tests still pass (fiskaltrust.Middleware.Localization.QueuePT.AcceptanceTest, fiskaltrust.Middleware.Localization.v2.UnitTest).
  • Spot check: a refund whose ftChargeItemCaseData actually differs from the original still fails, and the error message now reads (Field: ftChargeItemCaseData) rather than (Field: cbCustomer).

Follow-ups (not in this PR)

  • Consider replacing the JsonSerializer.Serialize string compare with JsonNode.DeepEquals so structurally-equal but differently-ordered objects also compare equal.
  • The same object-typed *CaseData fields exist in QueueES's RefundValidator — worth a follow-up audit there.
  • Decide whether the "" ≡ null leniency should be policy across the codebase or scoped to refund validation only.

🤖 Generated with Claude Code

ftReceiptCaseData, ftChargeItemCaseData, and ftPayItemCaseData are typed
as System.Object on v2 ReceiptRequest/ChargeItem/PayItem, so the existing
!= checks did reference equality. After JSON deserialization both sides
are distinct JsonElement instances, which made a stored "" never compare
equal to an incoming "" and caused full refunds to be rejected. Replace
the three reference comparisons with CaseDataEquals, which treats null
and empty string as equivalent and otherwise compares via
JsonSerializer.Serialize.

Also fix the copy-paste label in CompareChargeItems: the ftChargeItemCaseData
mismatch was reported as "(Field: cbCustomer)", which made the failure
look like a customer-object mismatch.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@StefanKert StefanKert requested a review from a team as a code owner May 21, 2026 16:04
@github-actions github-actions Bot added queue Related to all queues market-pt Related to the portugese market labels May 21, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 86f73e81b1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@github-actions
Copy link
Copy Markdown

Queue Test Results

   43 files     43 suites   2m 52s ⏱️
  750 tests   747 ✅ 3 💤 0 ❌
1 876 runs  1 870 ✅ 6 💤 0 ❌

Results for commit 86f73e8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

market-pt Related to the portugese market queue Related to all queues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant