Skip to content

[main] bug 640523 - Bonus depreciation functionality in CZ#9568

Open
DavidHolusa wants to merge 2 commits into
mainfrom
bugs/640523-master-BonusDepreciationInCZ
Open

[main] bug 640523 - Bonus depreciation functionality in CZ#9568
DavidHolusa wants to merge 2 commits into
mainfrom
bugs/640523-master-BonusDepreciationInCZ

Conversation

@DavidHolusa

@DavidHolusa DavidHolusa commented Jul 17, 2026

Copy link
Copy Markdown

What & why

Bug fix to implement missing bonus depreciation functionality in the CZ (Czech Republic) fixed asset localization.

Linked work

Fixes AB#640523

How I validated this

  • I read the full diff and it contains only changes I intended.
  • I built the affected app(s) locally with no new analyzer warnings.
  • I ran the change in Business Central and confirmed it behaves as expected.
  • I added or updated tests for the new behavior, or explained below why none are needed.

@DavidHolusa
DavidHolusa requested a review from a team July 17, 2026 08:26
@github-actions github-actions Bot added the Finance GitHub request for Finance area label Jul 17, 2026
@github-actions github-actions Bot added this to the Version 29.0 milestone Jul 17, 2026
@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Copilot PR Review

Iteration 2 · Outcome: completed

Knowledge source: https://github.com/microsoft/BCQuality@186d8a131465475c79244d994acb872cd5c0d4bf

Findings by domain

Findings split into Knowledge-backed (cite a BCQuality article) and Agent (the agent's own judgement, no matching BCQuality rule).

Domain Findings Knowledge-backed Agent Inline Fallback
Agent 1 0 1 0 1
Testing 1 1 0 0 1

Totals: 1 knowledge-backed · 1 agent findings.

Orchestrator pre-filter (2 file(s) excluded)

  • layer-disabled (knowledge) : 2 file(s)

Findings produced by the AL review agent v1.7.3. Reply 👎 on any inline comment to flag false positives.

…preciationCZF.Report.al


Fill in the VAT Reporting Date

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Agent} \quad \color{gray}{\texttt{\small Iteration\ 2}}$

InsertBonusDepreciationFAJnlLine and InsertBonusDepreciationGenJnlLine gate the new bonus-depreciation feature on FASetup.BonusDepreciationCorrectlySetupCZF(), but FASetup is only populated by FASetup.Get() inside the request page's OnOpenPage trigger, and only when DeprBookCode is still blank at that point ('if DeprBookCode = ''' then FASetup.Get()).

OnPreReport - which always runs, unlike OnOpenPage - never calls FASetup.Get() here, unlike the equivalent W1 base report (CalculateDepreciation.Report.al), whose OnPreReport explicitly calls FASetup.Get(). Any programmatic invocation that pre-sets DeprBookCode before the request page runs (for example InitializeRequest followed by UseRequestPage(false), the exact pattern LibraryFixedAssetCZF.RunCalculateDepreciation uses) skips the OnOpenPage branch entirely, leaving FASetup as a blank/uninitialized record. BonusDepreciationCorrectlySetupCZF() then reads blank fields, returns false, and the whole new bonus-depreciation feature silently never activates - no error, no log, in what is likely the report's most common non-interactive invocation path. If confirmed, this is severity-blocking for the feature's correctness; treat the minor severity here as an artifact of this being an agent (non-knowledge-backed) finding, not as a signal that the underlying defect is low-impact.

Recommendation:

  • call FASetup.Get() unconditionally in OnPreReport (mirroring the W1 base report), instead of relying on the conditional OnOpenPage population.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

        FASetup.Get();
        DepreciationBook.Get(DeprBookCode);

Line mapping was unavailable, so this was posted as an issue comment.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

@github-actions

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Testing} \quad \color{gray}{\texttt{\small Iteration\ 2}}$

InsertBonusDepreciationFAJnlLine/InsertBonusDepreciationGenJnlLine add a new bonus-depreciation posting path to the CZF localization, but no test in src/Apps/CZ/FixedAssetLocalization/test exercises it (no file or test method mentions 'Bonus' or 'CalculateDepreciationCZF' beyond the existing non-bonus scenarios).

The equivalent W1 base-app feature has an extensive dedicated test suite (ERMFABonusDepreciation.Codeunit.al) covering eligibility, amount calculation, and duplicate-application guards.

Recommendation:

  • add CZF test coverage mirroring the W1 suite, at minimum for BonusDepreciationCorrectlySetupCZF, BonusDepreciationAppliedCZF (duplicate-application guard), and BonusDepreciationAmountCZF (rounding), run through RunCalculateDepreciation.

Line mapping was unavailable, so this was posted as an issue comment.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

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

Labels

Finance GitHub request for Finance area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants