Repository navigation
[FI] Delocalize Depreciation Differences into a first-party extension - #9326
Milica Đukić (djukicmilica) wants to merge 70 commits into
Conversation
Move the Finland "Depreciation Differences" regulatory feature out of the FI BaseApp layer into a new first-party extension app at src/Apps/FI/DepreciationDifferencesFI, and obsolete the in-place BaseApp implementation behind CLEAN29 / CLEANSCHEMA32 guards and a feature flag, following the pattern of PR #9197. New app: - Table extensions for FA Posting Group, FA Ledger Entry, Source Code Setup - Page extensions for FA Posting Group Card / List - Calc. and Post Depreciation Difference report (+ RDLC) and posting buffer table - Feature, subscribers, upgrade and upgrade-tag codeunits; permission set ext BaseApp obsoletions (Pending 29.0 / Removed 32.0): - Obsolete depreciation-difference fields, report and buffer table - Wrap references in pages, permissions, demo data and tests - Register the app and its test in the BCApps build registry Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 731eaef8-bab7-4b1c-840e-f409b4de0195
5a95665 to
8b26e1e
Compare
…de Setup Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c9e418b3-19a8-4eaa-ba20-8aa03d619fb7
Good Sense Reviewer - Round 17Recommendation: Request ChangesWhat this PR doesThis moves Finnish depreciation-difference calculation, posting, setup, migration, and tests into a first-party extension. The latest commit removes the invalid demo-data event subscriber and restores the legacy field assignments under That removes one invalid reference, but it does not complete the demo-data compatibility fix. Status of previous suggestions
New observations (commits since round 16)None - the latest commit targets S7, which remains open. Problem-solution fitFit: Partial The delocalization remains a coherent feature increment, and the latest commit removes an invalid subscriber. The demo-data compatibility path still does not compile or provide a verified extension-owned integration for the legacy generator. Risk assessment and necessityRisk: The change affects G/L posting setup, migrated ledger flags, per-company upgrade tags, and demo-data initialization. The current default and clean FI builds fail with AL0185 in the extension test app. The legacy generator can also leave the new extension posting-group fields uninitialized after feature enablement. The changed RDLC still has no rendered screenshot. Necessity: Delocalization is needed to remove Finland-specific functionality from the base application while preserving existing data and upgrade behavior. The demo-data compatibility path must compile and initialize the extension fields before this change is safe to merge.
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 99f6f898-8482-413c-ba3c-ada5389ae15a
Good Sense Reviewer - Round 18Recommendation: Request ChangesWhat this PR doesThis moves Finnish depreciation-difference calculation, posting, setup, migration, and tests into a first-party extension. The latest commit adds the Contoso Coffee Demo Dataset as a dependency of the extension test app so Create FA Posting Group can be resolved. The dependency resolves that codeunit, but its public API does not contain InsertData. Both the default and clean FI builds now fail with AL0132 at the regression test. The test also calls the generator directly instead of running the Contoso Demo Tool lifecycle that binds the extension-owned subscriber, so it still does not verify the intended integration path. Status of previous suggestions
New observations (commits since round 17)None - the latest commit targets S7, which remains open. Problem-solution fitFit: Partial The delocalization remains a coherent feature increment. The demo-data integration has a plausible extension-owned design, but its regression test still does not compile or exercise the lifecycle that activates the subscriber. Risk assessment and necessityRisk: The change affects G/L posting setup, migrated ledger flags, per-company upgrade tags, and demo-data initialization. The current default and clean FI builds fail with AL0132 in the extension test app. The intended demo-data subscriber path therefore remains unverified. The changed RDLC still has no rendered screenshot. Necessity: Delocalization is needed to remove Finland-specific functionality from the base application while preserving existing data and upgrade behavior. The regression test must compile and exercise the supported demo-data generation path before this change is safe to merge.
|
Exercise the Contoso setup workflow for enabled and disabled feature states across all five fixed asset posting groups. Add the FI demo-data dependency and grant the test app access to Contoso module state. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d71e872-f1f6-4046-b6ed-80b37c198ee1
Good Sense Reviewer - Round 19Recommendation: Accept with SuggestionsWhat this PR doesThis moves Finnish depreciation-difference calculation, posting, setup, migration, and tests into a first-party extension. The latest commit replaces the invalid direct demo-generator call with tests that run the public Contoso setup workflow for both feature states. The tests now select the Fixed Asset module, invoke setup generation through the page action, and let the extension bind its posting-group subscriber around the real generation lifecycle. They verify all five posting groups in both the transitional and clean behavior, while the manifest changes provide the required FI dependency and internal test access. This addresses the remaining demo-data integration issue, and both FI app build variants compile successfully. Status of previous suggestions
New observations (commits since round 18)None - the latest commit addresses S7 without introducing another review finding. Problem-solution fitFit: Strong The delocalization is a coherent feature increment. The updated tests now cover the intended demo-data integration in both enabled and disabled transition states, matching the implementation path. Risk assessment and necessityRisk: The change affects G/L posting setup, migrated ledger flags, per-company upgrade tags, and demo-data initialization. The demo-data regression now exercises the real subscriber lifecycle and validates every standard fixed-asset posting group. The changed RDLC still has no rendered screenshot, so its visual result remains unverified. Necessity: Delocalization is needed to remove Finland-specific functionality from the base application while preserving existing data and upgrade behavior. The latest tests provide the missing integration coverage; adding rendered layout evidence remains a non-blocking review request.
|
Keep standalone CLEAN tests on the completed-upgrade path and verify migration through a documented disposable-database upgrade check. Guard obsolete-field mapping logic and imports consistently for CLEANSCHEMA33. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 44f5f2d5-b573-449b-9d7e-40a06615f665
Keep the migration verification harness local rather than shipping it with the extension tests. Preserve the AL test and schema-guard corrections. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 44f5f2d5-b573-449b-9d7e-40a06615f665
|
The report raises plain errors when the depreciation-difference accounts are missing from a named FA Posting Group, even though the user can be taken to that related setup record to correct the configuration. Use an ErrorInfo with a Show-it navigation action targeting the identified FA Posting Group. Knowledge: Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.50.6 |
Good Sense Reviewer - Round 20Recommendation: Request ChangesWhat this PR doesThis moves Finnish depreciation-difference calculation, posting, setup, migration, and tests into a first-party extension. The latest commits adjust clean-schema guards, stabilize the Contoso tests, and replace the forced-upgrade migration test with an idempotency test. The clean-schema guards and Contoso test isolation are consistent with the current object lifecycle. However, the new forced-upgrade test sets the migration tag before it calls the upgrade codeunit, so the production procedure exits before any legacy field is copied. The CLEAN30 financial-data migration is therefore no longer tested. Status of previous suggestions
New observations (commits since round 19)S8 (🔴 High): Restore forced migration field-copy coverage Problem-solution fitFit: Partial The delocalization remains a coherent feature increment, and the latest schema and Contoso changes fit it. The replacement upgrade test no longer proves that retained legacy financial fields are migrated in the CLEAN30 path. Risk assessment and necessityRisk: The change migrates fixed-asset posting accounts, a ledger-entry flag, and the source code used by financial processing. A defect in the forced CLEAN30 migration can leave extension fields empty or wrong, and the current test cannot detect it because it exercises only the already-tagged exit path. The changed RDLC also still has no rendered screenshot. Necessity: Delocalization is needed to remove Finland-specific functionality from the base application while preserving existing data and upgrade behavior. The schema-guard and test-isolation changes are useful, but direct coverage of the forced migration must remain before this financial upgrade path is safe.
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: aced1f3f-c55d-4177-bc2d-301bf59ea88b
|
The AL source file name Knowledge: Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.50.6 |
|
The AL source file name Knowledge: Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.50.6 |
Good Sense Reviewer - Round 21Recommendation: Request ChangesWhat this PR doesThis moves Finnish depreciation-difference calculation, posting, setup, migration, and tests into a first-party extension. The latest commit initializes the Contoso demo setup record before rebuilding localization account mappings, which supplies the FI defaults that the single-instance mapping needs. The test initialization change is correctly placed and does not alter production behavior. It does not change the CLEAN30 migration test, so the forced field-copy path remains uncovered. Status of previous suggestions
New observations (commits since round 20)None - the latest commit only initializes the Contoso setup record before refreshing test mappings. Problem-solution fitFit: Partial The delocalization remains a coherent feature increment, and the latest test initialization change fits it. The CLEAN30 test still does not prove that retained legacy financial fields are copied by the forced migration path. Risk assessment and necessityRisk: The change migrates fixed-asset posting accounts, a ledger-entry flag, and the source code used by financial processing. A defect in the forced CLEAN30 migration can leave extension fields empty or wrong, while the current test exits before the copy runs. The changed RDLC also still lacks rendered visual evidence. Necessity: Delocalization is needed to remove Finland-specific functionality from the base application while preserving existing data and upgrade behavior. Initializing the Contoso setup record makes the affected tests more reliable, but direct coverage of the forced migration remains necessary before the financial upgrade path is safe.
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0b9f1b46-c4a9-45fc-ae54-e0fc9bb6c770
Good Sense Reviewer - Round 22Recommendation: Request ChangesWhat this PR doesThis moves Finnish depreciation-difference calculation, posting, setup, migration, and tests into a first-party extension. The latest commit changes the Contoso fixed-asset demo tests to generate the selected module and its setup dependencies instead of injecting synthetic account mappings. The revised test path is sound. Clearing module progress makes the page rebuild its module state, the Fixed Asset filter lets dependency resolution generate the required setup, and the assertions verify both generated G/L accounts and posting-group assignments. This commit does not change the forced CLEAN30 migration test, so the open migration-coverage blocker remains. Status of previous suggestions
New observations (commits since round 21)None - the latest commit makes the existing Contoso demo tests use the real dependency-generated setup. Problem-solution fitFit: Partial The delocalization remains a coherent feature increment, and the latest test change better represents real demo-data generation. The CLEAN30 test still does not prove that retained legacy financial fields are copied by the forced migration path. Risk assessment and necessityRisk: The latest commit changes test setup only and reduces reliance on synthetic mappings. The main remaining risk is the forced upgrade of fixed-asset posting accounts, the ledger-entry flag, and the financial source code: the current test exits before any DataTransfer runs. The changed RDLC also still lacks rendered visual evidence. Necessity: Delocalization is needed to remove Finland-specific functionality from the base application while preserving existing data and upgrade behavior. The more realistic Contoso tests are useful, but direct coverage of the forced financial migration remains necessary before this upgrade path is safe.
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f8517b0d-6564-4ee4-bea0-059b6b4a9164
Good Sense Reviewer - Round 23Recommendation: Request ChangesWhat this PR doesThis moves Finnish depreciation-difference calculation, posting, setup, migration, and tests into a first-party extension. The latest commit adds the new posting-buffer table to the LOCAL permission set and adds a regression test for that effective permission. The new extension restores full buffer access for users assigned LOCAL. It does not preserve the separate read access that the old buffer received through the assignable LOCAL READ permission set, so the permission move is not complete. Status of previous suggestions
New observations (commits since round 22)S9 (🟠 Moderate): Preserve access through LOCAL READ Problem-solution fitFit: Partial The delocalization remains a coherent feature increment, and the latest commit restores normal LOCAL access to the moved buffer. The forced financial migration is still not exercised, and the read-only permission path is not preserved. Risk assessment and necessityRisk: Users assigned only LOCAL READ can lose access to the moved posting buffer. The larger remaining risk is the forced upgrade of fixed-asset posting accounts, the ledger-entry flag, and the financial source code because the current test exits before any DataTransfer runs. The changed RDLC also still lacks rendered visual evidence. Necessity: Delocalization is needed to remove Finland-specific functionality from the base application while preserving existing data, permissions, and upgrade behavior. The LOCAL fix is necessary, but the read-only permission and forced migration paths also need to remain compatible.
|
This reverts commit c099b89. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f8517b0d-6564-4ee4-bea0-059b6b4a9164
Use a non-writing feature lookup and check the report ID before evaluating the feature. Add regression coverage for unrelated reports and enabled, disabled, and pending feature states. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f8517b0d-6564-4ee4-bea0-059b6b4a9164
Summary
Delocalizes the Finland Depreciation Differences regulatory feature from the FI BaseApp layer into a new first-party extension app, and obsoletes the in-place BaseApp implementation behind
CLEAN30/CLEANSCHEMA33guards and a feature key (DepreciationDifferencesFI).New extension app --
src/Apps/FI/DepreciationDifferencesFITransition-only feature-management objects (the feature codeunit, subscribers, Feature Data Update implementation, and enum registration) compile only while
CLEAN30is not defined. InCLEAN30builds, theUpgrade Depreciation Diff. FIcodeunit (Subtype = Upgrade) performs the field migration directly. The field-copy logic is shared by both migration paths through the always-compiledDep Diff FI Helper Procedurescodeunit, avoiding duplicated migration code. Legacy schema remains available untilCLEANSCHEMA33.BaseApp obsoletions (
ObsoleteState: Pending,30.0/ removed33.0, schema removed underCLEANSCHEMA33)Calc. and Post Depr. Diff.), and the legacyDepr. Diff. Posting Buffertable.CLEAN30guard. BaseApp production objects no longer depend on the feature key; transitional runtime checks remain in the extension, demo-data producers, and test infrastructure whileCLEAN30is not defined.Dep Diff FI Data Class.subscriber.CLEAN30; equivalent depreciation-difference tests are included in the extension test app.Integration and migration compatibility
build/projects.json,build/groups.json, and the Apps FI AL-Go project settings.CLEANSCHEMA33and update its transformation test for the intentionally obsolete source field.Verification
Implementation and build registration are complete and the app is correctly wired into the BCApps build registry. Local full compile/publish/test was not run because the local enlistment's Base Application symbols were not provisioned. PR CI has completed successfully with 170 successful checks and 2 skipped checks.
Fixes: AB#622873
Linked Platform PR for feature key
https://microsoft.ghe.com/bic/BC-Platform/pull/47112