fix(landed-cost): gate un-journaled COGS claim on daily batch enabled (gbzh)#285
Conversation
…l when the daily batch is off (gbzh) Follow-up to 3aph. refreshShipmentCogsForCostLayerChange claimed an un-journaled shipment's COGS revaluation delta (so callers drop it from the landed-cost / manufacturing COGS journal) on the assumption the daily batch would later post the updated cogsBatchAmount. If the daily batch is globally disabled, the batch never posts it, so the delta posted NOWHERE — an under-count. Gate the un-journaled claim on the daily batch actually being enabled (new isDailyBatchPostingEnabled() — active connector + sync_enabled + daily_batch_enabled, mirroring app/api/cron/accounting-daily-batch/route.ts), symmetric with the journaled COGS_REVERSAL gate. When the batch is off, the delta stays in the COGS journal so it still posts exactly once. The check is resolved at most once per call and injectable for tests. bd: onetwo3d-ims-gbzh Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b47cc4996
ℹ️ 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".
| if (dailyBatchPosts === null) { | ||
| dailyBatchPosts = await (options.isDailyBatchPostingEnabled ?? isDailyBatchPostingEnabled)() | ||
| } | ||
| if (dailyBatchPosts) cogsRevaluationDelta = addMoney(cogsRevaluationDelta, shipmentDelta) |
There was a problem hiding this comment.
Avoid double-counting when the batch is re-enabled
If the daily batch is disabled during this revaluation but is enabled before the affected shipment is journaled, this branch leaves the delta in the current landed-cost/manufacturing journal while the function has already persisted the recomputed cogsBatchAmount on the shipment. The daily batch Group B path later selects shipmentJournalDate: null shipments and posts the recomputed snapshot/COGS amount before marking them journaled (mirrored in both Xero and QuickBooks daily sync), so the same revaluation delta can be posted once in the current journal and again by the later batch. Consider either not persisting the recomputed shipment COGS when the delta is kept in the journal, or recording that the delta has already been journaled so the later batch does not include it again.
Useful? React with 👍 / 👎.
Summary
Fixes onetwo3d-ims-gbzh (P3, follow-up to the 3aph COGS double-post fix).
refreshShipmentCogsForCostLayerChangeclaimed an un-journaled shipment's COGS revaluation delta (so callers drop it from the landed-cost / manufacturing COGS journal) assuming the daily batch would later post the updatedcogsBatchAmount. If the daily batch is globally disabled, the batch never posts it → the delta posted nowhere (under-count).Fix
Gate the un-journaled claim on the daily batch actually being enabled — new
isDailyBatchPostingEnabled()(active connector +<connector>_sync_enabled+<connector>_daily_batch_enabled, mirroringapp/api/cron/accounting-daily-batch/route.ts), symmetric with the journaledCOGS_REVERSALgate from 3aph. When the batch is off, the delta stays in the COGS journal so COGS still posts exactly once. Resolved at most once per call; injectable for tests.Test plan
type-check,eslint,check:decimal-boundaries,test:unit(1492 pass)🤖 Generated with Claude Code