Skip to content

[29.x] Bug 649257: [Excise Tax] Improving Excise Duty Calculation - #11116

Open
v-rohangarg20 wants to merge 4 commits into
releases/29.xfrom
bugs/649257-Excise-Tax-Improving-Excise-Duty-Calculation-29.x
Open

[29.x] Bug 649257: [Excise Tax] Improving Excise Duty Calculation#11116
v-rohangarg20 wants to merge 4 commits into
releases/29.xfrom
bugs/649257-Excise-Tax-Improving-Excise-Duty-Calculation-29.x

Conversation

@v-rohangarg20

@v-rohangarg20 v-rohangarg20 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Fixes AB#649257

Issue

Excise duty calculation in the 29.x branch did not support the new rate-based calculation types and could calculate an incorrect tax amount when the final item category changed the applicable calculation type.

Cause

The 29.x calculation flow used the legacy item and fixed-asset rate model without an explicit calculation type for specific-per-unit, ad valorem, and hybrid duties. For item ledger entries, the taxable amount could be determined before the final rate lookup and calculation type had been applied. Existing rates also needed to be migrated without introducing schema changes from newer releases.

Solution

  • Added the Excise Tax Rate table, list page, and Excise Calculation Type enum.
  • Applied the final rate and calculation type before calculating the taxable amount for item ledger entries.
  • Added migration from the existing item/fixed-asset rate table.
  • Updated permissions, pages, journal lines, transaction logs, and tests.
  • Excluded main-branch-only deprecation changes so the 29.x servicing schema remains compatible.

Validation

  • Updated the affected Excise Taxes app and test app.
  • Applied the changes as a 29.x backport without conflicts.

The item-ledger path evaluated RequiresTaxableAmount before the OnAfterUpdateExciseJournalLineFromItemLedgerEntry subscriber applied the rate for the ledger entry item category. When the item category and the ledger entry category resolved to different calculation types, an ad valorem or hybrid line could keep a zero taxable amount and calculate the wrong tax. The taxable amount is now set after the event so the final calculation type is used.

Also fixed VerifyFAJournalLinesForAcquisitionCost to create the fixed asset rate with an explicit Excise Calculation Type, so the rate lookup resolves as the test expects.
…29.x

The main-branch change deprecated table 7413 and page 7412 in favour of the new Excise Tax Rate table, gating them behind CLEAN30/CLEANSCHEMA33. Deprecating an object in a servicing branch is not valid: the obsolete tags 30.0 and 33.0 refer to versions newer than 29.x, and those CLEAN symbols are not defined here.

The old table and page are therefore restored byte-identical to the 29.x baseline, and the preprocessor gates are dropped from the permission sets and the upgrade codeunit so the rate migration always runs. Only the additive part of the feature - the Excise Tax Rate table, the Excise Tax Rates page, the calculation type enum and the calculation changes - is backported.
@v-rohangarg20
v-rohangarg20 requested a review from a team September 7, 2026 07:04
@v-rohangarg20
v-rohangarg20 requested a review from a team as a code owner September 7, 2026 07:04
@github-actions github-actions Bot added AL: Apps (W1) Add-on apps for W1 Team: Finance GitHub request for Finance area labels Sep 7, 2026
@github-actions github-actions Bot added this to the Version 29.1 milestone Sep 7, 2026
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 1

Recommendation: Request Changes

What this PR does

This change adds rate-based excise duty calculation types, a new rate table with item-category matching, migration from the old rate table, journal and transaction-log fields, and tests for the new rate model. The main calculation flow is in the right place: item ledger entry data is copied, the item category is set, the final rate is applied, and taxable amount is then calculated only for ad valorem or hybrid rates.

Problem-solution fit

Fit: Strong

The reported bug is about wrong excise duty when the applicable rate type depends on the final item category. The code change targets that path directly by carrying the item category into the journal line before rate selection and by calculating taxable amount after the final calculation type is known.

Suggestions

S1 (🔴 High): Add a posted-entry regression test
The changed item ledger entry path sets "Excise Taxable Amount" from posted value entries, but the new tests only exercise a manually created journal line. Please add a test that posts or creates an item ledger entry, changes the applicable item category rate, generates the excise journal line, and verifies the taxable amount and tax amount. This is a tax amount path, so the exact regression should be covered before merge.

Risk assessment and necessity

Risk: This is a sensitive tax calculation change. A wrong rate lookup or taxable amount can create an incorrect excise tax amount in the journal and then copy that wrong amount to the transaction log.

Necessity: The change is needed because the old model only supported specific-per-unit rates and could not model category-driven ad valorem or hybrid rates. The scope is large for a servicing branch, but it is focused on the excise duty rate model and includes migration and UI updates.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11116 round=1 by=alexei-dobriansky at=2026-09-07T13:41:43Z lastSha=81db1347dc6742be45e9dfe435ece9d8084a1ecc reviewKey=5a1662755c121386a4a3fdbc4fee7b4ff997704a1137263c86f981e4a84b1b3b suggestions=S1@27a707ca

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Labels

AL: Apps (W1) Add-on apps for W1 Team: Finance GitHub request for Finance area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants