[Subcontracting] Remove Subcontracting Purchase Provisioning Wizard#8167
[Subcontracting] Remove Subcontracting Purchase Provisioning Wizard#8167SPinkow wants to merge 3 commits into
Conversation
…contracting App. It will be added again if the necessary parts are moved to the BaseApp.
Trailing newline removed from Codeunit fileThis PR changes the final Recommendation:
Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why |
Trailing newline removed from test Codeunit fileThe final Recommendation:
Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why |
AnalysisScenario Validity: Correctness — consolidation onto
Correctness — wizard removal and dead surface left behind:
Side Effects:
Risk Assessment:
Recommendation: Accept with Suggestions
|
|
Consider deleting orphaned objects. |
…rdFromSubcontractingApp
| { | ||
| var | ||
| SubcManagementSetup: Record "Subc. Management Setup"; | ||
| ManufacturingSetup: Record "Manufacturing Setup"; |
There was a problem hiding this comment.
Module-level record variable never populated via Get()
The codeunit declares a module-level ManufacturingSetup: Record "Manufacturing Setup" variable but never calls Get() on it before invoking ManufacturingSetup.ItemChargeToRcptSubReferenceEnabled(). The intent to cache the setup record is not realized — each call to the method triggers its own fresh DB read internally. The same pattern exists in SubcPurchPostExt and SubcItemChargeAssPurch.PageExt.al.
Recommendation:
- Either populate the module-level variable with
ManufacturingSetup.Get()once (or lazily) and accessManufacturingSetup.RefItemChargeToRcptSubLinesdirectly, or remove the module-level variable and call the helper method on a local variable. This avoids the misleading appearance of caching where none occurs.
| ManufacturingSetup: Record "Manufacturing Setup"; | |
| var | |
| ManufacturingSetup: Record "Manufacturing Setup"; | |
| ManufacturingSetupLoaded: Boolean; | |
| local procedure GetManufacturingSetup() | |
| begin | |
| if ManufacturingSetupLoaded then | |
| exit; | |
| ManufacturingSetup.SetLoadFields(RefItemChargeToRcptSubLines); | |
| ManufacturingSetupLoaded := ManufacturingSetup.Get(); | |
| end; |
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why
Module-level ManufacturingSetup never populated, double DB readsLike Recommendation:
Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why |
Misleading manual setup description labelThe Recommendation:
Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why |
| ManualSetupCategory: Enum "Manual Setup Category"; | ||
| begin | ||
| sender.InsertManualSetup(SubcontractingLbl, SubcontractingLbl, SubcontractingDescriptionLbl, 0, ObjectType::Page, Page::"Subc. Management Setup", ManualSetupCategory::Uncategorized, SubcontractingKeyWordsLbl); | ||
| sender.InsertManualSetup(SubcontractingLbl, SubcontractingLbl, SubcontractingDescriptionLbl, 0, ObjectType::Page, Page::"Manufacturing Setup", ManualSetupCategory::Uncategorized, SubcontractingKeyWordsLbl); |
There was a problem hiding this comment.
Manual setup opens full Manufacturing Setup page
The Subcontracting app's manual setup entry now opens Page::"Manufacturing Setup", which is a general-purpose page with many unrelated fields. Users navigating from the Subcontracting App guided setup will see all Manufacturing Setup fields rather than only subcontracting-relevant ones, resulting in a poor user experience.
Recommendation:
- Consider navigating to a dedicated subcontracting section within Manufacturing Setup by using a filtered or anchor-based navigation, or create a lightweight card page extension that shows only the relevant subcontracting fields and link to that instead.
| sender.InsertManualSetup(SubcontractingLbl, SubcontractingLbl, SubcontractingDescriptionLbl, 0, ObjectType::Page, Page::"Manufacturing Setup", ManualSetupCategory::Uncategorized, SubcontractingKeyWordsLbl); | |
| sender.InsertManualSetup(SubcontractingLbl, SubcontractingLbl, SubcontractingDescriptionLbl, 0, ObjectType::Page, Page::"Subc. Manufacturing Setup", ManualSetupCategory::Uncategorized, SubcontractingKeyWordsLbl); | |
| // Note: Requires a dedicated page extension or page for subcontracting-specific setup fields. |
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why
Inconsistent caching of ManufacturingSetup across proceduresThe module-level Recommendation:
Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why |
| @@ -1,100 +0,0 @@ | |||
| // ------------------------------------------------------------------------------------------------ | |||
There was a problem hiding this comment.
No upgrade/migration for deleted table data
The 'Subc. Management Setup' table (table 99001501) is deleted entirely with no upgrade codeunit to migrate existing customer data. Fields such as 'Common Work Center No.', 'Preset Component Item No.', 'Def. provision flushing method', and all wizard configuration fields will be silently lost on upgrade for any tenant that had configured these values.
Recommendation:
- Add an upgrade codeunit that runs before the table is dropped, migrating any still-relevant fields to the ManufacturingSetup table extension or preserving the data for customers. At minimum, document that this data is intentionally abandoned and that it is safe to drop.
| // ------------------------------------------------------------------------------------------------ | |
| // Add a codeunit with SubType = Upgrade: | |
| [UpgradeTag] | |
| procedure UpgradeSubcManagementSetupToManufacturingSetup() | |
| var | |
| SubcMgmtSetup: Record "Subc. Management Setup"; | |
| ManufacturingSetup: Record "Manufacturing Setup"; | |
| begin | |
| if not SubcMgmtSetup.Get() then | |
| exit; | |
| if ManufacturingSetup.Get() then begin | |
| // migrate relevant fields if needed | |
| ManufacturingSetup.Modify(); | |
| end; | |
| end; |
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why
Field removal without data migrationField 99001506 'Rtng. Link Code Purch. Prov.' (Code[10]) is removed from the Manufacturing Setup table extension. Any existing tenant data in this field will be silently dropped on upgrade. This field was previously referenced in setup initialization code and tests. Recommendation:
Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why |
Permission entries removed without ObsoleteState transitionMultiple objects were removed from the Recommendation:
Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why |
Remove Subcontracting Purchase Provisioning Wizard temporary from Subcontracting App.
It will be added again if the necessary parts are moved to the BaseApp.
What & why
These changes removes the Subcontracting Purchase Provizioning Wizard and the related tests. The Wizard will be moved to the BaseApp. Because this dependency on the base app does not exists for this build now we decided to first remove the wizard and add the subcontraction app related festures and tests later agian.
Details about the changes:
This pull request makes significant changes to the Subcontracting app by removing dependencies on the custom
Subc. Management Setuptable and related objects, consolidating setup logic onto the standardManufacturing Setuptable. It also removes several obsolete codeunits and pages, and updates permissions accordingly. The changes improve maintainability and align the app more closely with standard manufacturing features.Key changes:
Removal of Subc. Management Setup and related objects
Subc. Management Setuptable and its data access have been removed from codeunits, replacing them with the standardManufacturing Setuptable. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]Subc. Management Setuppage and related codeunits and pages have been deleted (Subc. CrPurchSubcon(Yes/No),Subc. ItemJnlCheckExt,Subc. Create Prod. Rtng. Ext.). [1] [2] [3]Permissions and security updates
Subcontract. - Objs,Subcontract. - Read,Subcontract. - Edit). [1] [2] [3] [4] [5] [6] [7] [8]Setup and initialization changes
Manufacturing Setuppage instead of the removedSubc. Management Setuppage.Dependency and namespace updates
using Microsoft.Manufacturing.Setup;statements to files now usingManufacturing Setup. [1] [2] [3]Linked work
Fixes AB#619329
How I validated this
What I tested and the outcome (required — be specific: scenarios, commands, screenshots for UI changes)
Risk & compatibility
None