-
Notifications
You must be signed in to change notification settings - Fork 459
[FS] Prevent duplicate Field Service customer assets during item synchronization #11094
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
8552212
84a9998
482d353
a5191e7
e3d1c2a
1323d7b
2378ecc
225f4dd
1d615d8
be17c20
dcf5600
326579a
f0b6ce4
bb07946
7acb788
b6614f9
ab1b3a7
2b915d6
890d0ee
266c0b8
bc0d664
1d1420e
e894abd
3e4e3c2
4d32491
b53f402
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,6 +58,13 @@ codeunit 139205 "FS Integration Test Library" | |
| FSIntTableSubscriber.UpdateQuantities(FSBookableResourceBooking, ServiceLine); | ||
| end; | ||
|
|
||
| procedure GetCustomerAssetConversion(ItemIsManaged: Boolean): Boolean | ||
| var | ||
| FSIntTableSubscriber: Codeunit "FS Int. Table Subscriber"; | ||
| begin | ||
| exit(FSIntTableSubscriber.GetCustomerAssetConversion(ItemIsManaged)); | ||
| end; | ||
|
|
||
| procedure IgnorePostedJobJournalLinesOnQueryPostFilterIgnoreRecord(SourceRecordRef: RecordRef; var IgnoreRecord: Boolean) | ||
| var | ||
| FSIntTableSubscriber: Codeunit "FS Int. Table Subscriber"; | ||
|
|
@@ -79,12 +86,17 @@ codeunit 139205 "FS Integration Test Library" | |
| FSIntTableSubscriber.IgnoreArchievedCRMWorkOrdersOnQueryPostFilterIgnoreRecord(SourceRecordRef, IgnoreRecord); | ||
| end; | ||
|
|
||
| /// <summary> | ||
| /// Retained for compatibility. Service items are now always synchronized to Field Service customer assets, so this procedure leaves the synchronization decision unchanged. | ||
| /// </summary> | ||
| /// <param name="SourceRecordRef">A reference to the service item to evaluate.</param> | ||
| /// <param name="IgnoreRecord">The existing synchronization decision, which is left unchanged.</param> | ||
| #pragma warning disable AS0105 | ||
| [Obsolete('Remove calls to this procedure. Service items are always synchronized to Field Service customer assets; item-product synchronization disables customer asset conversion.', '30.0')] | ||
|
tomasevicst marked this conversation as resolved.
tomasevicst marked this conversation as resolved.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The public test-library procedure IgnoreServiceItemsByConvertToCustomerAssetFlag is newly marked [Obsolete(...,'30.0')], but its body was simultaneously replaced with a no-op instead of continuing to delegate to the prior logic. Any dependent app or test still calling this procedure during the deprecation window (before 30.0 removal) will silently stop having IgnoreRecord evaluated/set, changing observable runtime behavior while the symbol still compiles. Staged-obsoletion guidance expects the procedure to keep working during the deprecation window and only become an empty/removed stub at or after the tagged removal version. Suggested fix (apply manually — could not be anchored as a one-click suggestion): procedure IgnoreServiceItemsByConvertToCustomerAssetFlag(SourceRecordRef: RecordRef; var IgnoreRecord: Boolean)
var
FSIntTableSubscriber: Codeunit "FS Int. Table Subscriber";
begin
FSIntTableSubscriber.IgnoreServiceItemsByConvertToCustomerAssetFlag(SourceRecordRef, IgnoreRecord);
end;Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.39.6
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The public procedure IgnoreServiceItemsByConvertToCustomerAssetFlag on the FS Integration Test Library codeunit was kept and marked [Obsolete(...,'30.0')], but its implementation was emptied instead of preserved. Emptying the body changes the procedure's observable behavior (IgnoreRecord is now always left unchanged) immediately for any remaining caller, rather than preserving prior behavior through the deprecation window until the tag's removal version. Prefer keeping the forwarding call to FSIntTableSubscriber.IgnoreServiceItemsByConvertToCustomerAssetFlag until the obsolete procedure is actually removed. Suggested fix (apply manually — could not be anchored as a one-click suggestion): procedure IgnoreServiceItemsByConvertToCustomerAssetFlag(SourceRecordRef: RecordRef; var IgnoreRecord: Boolean)
var
FSIntTableSubscriber: Codeunit "FS Int. Table Subscriber";
begin
FSIntTableSubscriber.IgnoreServiceItemsByConvertToCustomerAssetFlag(SourceRecordRef, IgnoreRecord);
end;Knowledge: 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.40.6
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested fix (apply manually — could not be anchored as a one-click suggestion): procedure IgnoreServiceItemsByConvertToCustomerAssetFlag(SourceRecordRef: RecordRef; var IgnoreRecord: Boolean)
var
FSIntTableSubscriber: Codeunit "FS Int. Table Subscriber";
begin
FSIntTableSubscriber.IgnoreServiceItemsByConvertToCustomerAssetFlag(SourceRecordRef, IgnoreRecord);
end;Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.41.6 |
||
| procedure IgnoreServiceItemsByConvertToCustomerAssetFlag(SourceRecordRef: RecordRef; var IgnoreRecord: Boolean) | ||
| var | ||
| FSIntTableSubscriber: Codeunit "FS Int. Table Subscriber"; | ||
| begin | ||
| FSIntTableSubscriber.IgnoreServiceItemsByConvertToCustomerAssetFlag(SourceRecordRef, IgnoreRecord); | ||
| end; | ||
| #pragma warning restore AS0105 | ||
|
|
||
| procedure MarkArchivedServiceOrder(ServiceHeader: Record "Service Header") | ||
| var | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,7 +23,6 @@ using Microsoft.Purchases.Vendor; | |
| using Microsoft.Sales.Customer; | ||
| using Microsoft.Service.Archive; | ||
| using Microsoft.Service.Document; | ||
| using Microsoft.Service.Item; | ||
| using Microsoft.Service.Setup; | ||
| using Microsoft.Service.Test; | ||
| using Microsoft.TestLibraries.DynamicsFieldService; | ||
|
|
@@ -1578,109 +1577,79 @@ codeunit 139204 "FS Integration Test" | |
| end; | ||
|
|
||
| [Test] | ||
| procedure IgnoreServiceItemWhenConvertToCustomerAssetIsFalse() | ||
| [TransactionModel(TransactionModel::AutoCommit)] | ||
| procedure ItemSynchronizationDisablesCustomerAssetConversion() | ||
|
tomasevicst marked this conversation as resolved.
|
||
| var | ||
| Item: Record Item; | ||
| TempServiceItem: Record "Service Item" temporary; | ||
| CRMProduct: Record "CRM Product"; | ||
| CRMIntegrationRecord: Record "CRM Integration Record"; | ||
| RecordRef: RecordRef; | ||
| IgnoreRecord: Boolean; | ||
| ProductId: Guid; | ||
| begin | ||
| // [FEATURE] [Service Item Mapping] | ||
| // [SCENARIO] Service Item is skipped when linked CRM Product has Convert to Customer Asset = No. | ||
| Initialize(); | ||
| InitSetup(true, ''); | ||
|
|
||
| Item.Get(CreateItem()); | ||
| TempServiceItem."Item No." := Item."No."; | ||
| RecordRef.GetTable(TempServiceItem); | ||
|
|
||
| ProductId := CreateGuid(); | ||
| CRMProduct.ProductId := ProductId; | ||
| CRMProduct.ConvertToCustomerAsset := false; | ||
| CRMProduct.Insert(false); | ||
|
|
||
| CRMIntegrationRecord.CoupleCRMIDToRecordID(ProductId, Item.RecordId()); | ||
|
|
||
| FSIntegrationTestLibrary.IgnoreServiceItemsByConvertToCustomerAssetFlag(RecordRef, IgnoreRecord); | ||
|
|
||
| Assert.IsTrue(IgnoreRecord, 'Service Item should be ignored when Convert to Customer Asset is false.'); | ||
| end; | ||
|
|
||
| [Test] | ||
| procedure DoNotIgnoreServiceItemWhenConvertToCustomerAssetIsTrue() | ||
| var | ||
|
tomasevicst marked this conversation as resolved.
|
||
| Item: Record Item; | ||
| TempServiceItem: Record "Service Item" temporary; | ||
| CRMProduct: Record "CRM Product"; | ||
| CRMIntegrationRecord: Record "CRM Integration Record"; | ||
| RecordRef: RecordRef; | ||
| IgnoreRecord: Boolean; | ||
| ProductId: Guid; | ||
| IntegrationTableMapping: Record "Integration Table Mapping"; | ||
| CRMIntegrationTableSynch: Codeunit "CRM Integration Table Synch."; | ||
| CRMSetupDefaults: Codeunit "CRM Setup Defaults"; | ||
| begin | ||
| // [FEATURE] [Service Item Mapping] | ||
| // [SCENARIO] Service Item is not skipped when linked CRM Product has Convert to Customer Asset = Yes. | ||
| // [FEATURE] [Item-Product Mapping] | ||
| // [SCENARIO] Synchronizing an item disables native Field Service customer asset creation. | ||
| Initialize(); | ||
| LibraryCRMIntegration.CreateCRMConnectionSetup('', '@@test@@', true); | ||
| InitSetup(true, ''); | ||
|
|
||
| Item.Get(CreateItem()); | ||
| TempServiceItem."Item No." := Item."No."; | ||
| RecordRef.GetTable(TempServiceItem); | ||
|
|
||
| ProductId := CreateGuid(); | ||
| CRMProduct.ProductId := ProductId; | ||
| // [GIVEN] A coupled item and product where Convert to Customer Asset is Yes. | ||
| CRMSetupDefaults.ResetItemProductMapping('ITEM-PRODUCT', false); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The new Item-to-CRM Product sync test ( Agent judgement — not directly backed by a BCQuality knowledge article. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.41.6 |
||
| LibraryCRMIntegration.CreateCoupledItemAndProduct(Item, CRMProduct); | ||
| CRMProduct.ConvertToCustomerAsset := true; | ||
| CRMProduct.Insert(false); | ||
| CRMProduct.Modify(); | ||
|
|
||
| CRMIntegrationRecord.CoupleCRMIDToRecordID(ProductId, Item.RecordId()); | ||
| IntegrationTableMapping.Get('ITEM-PRODUCT'); | ||
| Item.CalcFields("Coupled to Dataverse"); | ||
| Assert.IsTrue(Item."Coupled to Dataverse", 'The item should be identified as managed by its Dataverse coupling.'); | ||
|
|
||
| FSIntegrationTestLibrary.IgnoreServiceItemsByConvertToCustomerAssetFlag(RecordRef, IgnoreRecord); | ||
| // [WHEN] The item is synchronized to the Field Service product. | ||
| CRMIntegrationTableSynch.SynchRecord(IntegrationTableMapping, Item.RecordId(), true, false); | ||
|
|
||
| Assert.IsFalse(IgnoreRecord, 'Service Item should not be ignored when Convert to Customer Asset is true.'); | ||
| // [THEN] Native Field Service customer asset creation is disabled. | ||
| CRMProduct.Get(CRMProduct.ProductId); | ||
| Assert.IsFalse(CRMProduct.ConvertToCustomerAsset, 'Convert to Customer Asset should be disabled.'); | ||
| end; | ||
|
|
||
| [Test] | ||
| procedure DoNotIgnoreServiceItemWhenItemNoIsBlank() | ||
| var | ||
| TempServiceItem: Record "Service Item" temporary; | ||
| RecordRef: RecordRef; | ||
| IgnoreRecord: Boolean; | ||
| [TransactionModel(TransactionModel::AutoRollback)] | ||
| procedure CustomerAssetConversionReflectsItemManagement() | ||
| begin | ||
| // [FEATURE] [Service Item Mapping] | ||
| // [SCENARIO] Service Item with blank Item No. is not skipped by this filter. | ||
| Initialize(); | ||
| InitSetup(true, ''); | ||
|
|
||
| TempServiceItem."Item No." := ''; | ||
| RecordRef.GetTable(TempServiceItem); | ||
| // [FEATURE] [Item-Product Mapping] | ||
| // [SCENARIO] Customer asset conversion is disabled only for items managed through synchronization. | ||
|
|
||
| FSIntegrationTestLibrary.IgnoreServiceItemsByConvertToCustomerAssetFlag(RecordRef, IgnoreRecord); | ||
| // [WHEN] An item is not managed through synchronization. | ||
| // [THEN] Native Field Service customer asset conversion remains enabled. | ||
| Assert.IsTrue(FSIntegrationTestLibrary.GetCustomerAssetConversion(false), 'An unmanaged item should allow customer asset conversion.'); | ||
|
|
||
| Assert.IsFalse(IgnoreRecord, 'Service Item with blank Item No. should not be ignored by this filter.'); | ||
| // [WHEN] An item is managed through synchronization. | ||
| // [THEN] Native Field Service customer asset conversion is disabled. | ||
| Assert.IsFalse(FSIntegrationTestLibrary.GetCustomerAssetConversion(true), 'A managed item should disable customer asset conversion.'); | ||
| end; | ||
|
|
||
| [Test] | ||
| procedure DoNotIgnoreServiceItemWhenItemIsNotCoupled() | ||
| [TransactionModel(TransactionModel::AutoCommit)] | ||
|
tomasevicst marked this conversation as resolved.
|
||
| procedure ItemProductMappingDisablesCustomerAssetConversion() | ||
| var | ||
| Item: Record Item; | ||
| TempServiceItem: Record "Service Item" temporary; | ||
| RecordRef: RecordRef; | ||
| IgnoreRecord: Boolean; | ||
| CRMProduct: Record "CRM Product"; | ||
| IntegrationFieldMapping: Record "Integration Field Mapping"; | ||
| CRMSetupDefaults: Codeunit "CRM Setup Defaults"; | ||
| begin | ||
| // [FEATURE] [Service Item Mapping] | ||
| // [SCENARIO] Service Item with uncoupled Item is not skipped by this filter. | ||
| // [FEATURE] [Item-Product Mapping] | ||
| // [SCENARIO] The item-product mapping derives native Field Service customer asset creation from item management. | ||
| Initialize(); | ||
| InitSetup(true, ''); | ||
|
|
||
| Item.Get(CreateItem()); | ||
| TempServiceItem."Item No." := Item."No."; | ||
| RecordRef.GetTable(TempServiceItem); | ||
|
|
||
| FSIntegrationTestLibrary.IgnoreServiceItemsByConvertToCustomerAssetFlag(RecordRef, IgnoreRecord); | ||
| // [WHEN] The default item-product mapping is reset. | ||
| CRMSetupDefaults.ResetItemProductMapping('ITEM-PRODUCT', false); | ||
|
|
||
| Assert.IsFalse(IgnoreRecord, 'Service Item with uncoupled Item should not be ignored by this filter.'); | ||
| // [THEN] Convert to Customer Asset is mapped from the item's Dataverse coupling state in the outbound direction. | ||
| IntegrationFieldMapping.SetRange("Integration Table Mapping Name", 'ITEM-PRODUCT'); | ||
| IntegrationFieldMapping.SetRange("Integration Table Field No.", CRMProduct.FieldNo(ConvertToCustomerAsset)); | ||
| Assert.IsTrue(IntegrationFieldMapping.FindFirst(), 'The Convert to Customer Asset mapping should exist.'); | ||
| Assert.AreEqual(Item.FieldNo("Coupled to Dataverse"), IntegrationFieldMapping."Field No.", 'The mapping should use the item coupling state.'); | ||
| Assert.AreEqual(IntegrationFieldMapping.Direction::ToIntegrationTable, IntegrationFieldMapping.Direction, 'The mapping should be outbound.'); | ||
| Assert.AreEqual('', IntegrationFieldMapping."Constant Value", 'The mapping should not use a constant value.'); | ||
| end; | ||
|
|
||
| local procedure Initialize() | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The obsolete public test-library procedure
IgnoreServiceItemsByConvertToCustomerAssetFlag(Microsoft.TestLibraries.DynamicsFieldService, FSIntegrationTestLibrary.Codeunit.al) is correctly marked[Obsolete('...', '30.0')], but its body was replaced with an empty no-op instead of continuing to forward to the previous implementation. During the deprecation window, existing external callers who still invoke this public procedure expectingIgnoreRecordto be set now silently receive unchanged behavior. Guidance on deprecating public members with the Obsolete lifecycle recommends preserving the prior behavior (e.g., keep forwarding to the codeunit's still-present implementation, or otherwise retain the previous decision) until the procedure is actually removed in a later release, rather than emptying it immediately upon obsoletion.Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.41.6