[Shopify] Fix #7724: Resolve Option 1 Name from existing Shopify Variant records

[tVisionDeb]

Summary

Fixes #7724 - regression introduced in PR #7660 where Option 1 Name was being reset to 'Variant' for Shopify-origin products whose variants already have a different Option 1 Name (e.g. 'Color', 'Size', etc.).

Root Cause

In FillInProductVariantData, after resolving the Option 1 Name from existing Shopify variant records, the code failed to set Option 1 Value when the resolved name was anything other than 'Variant'. The guard condition was missing the JustResolvedOption1Name flag.

Fix

Added a JustResolvedOption1Name boolean so that Option 1 Value is always set when the name was just resolved from Shopify - regardless of what the resolved name is (e.g. 'Color', 'Size', 'Variant', etc.).

Tests Added

New test codeunit 139649 - Shpfy Variant Option1Name Test with 5 scenarios:

1. UnitTestOption1NameInheritedFromShopifyOriginProduct - Shopify-origin product with existing variants keeps their Option 1 Name and sets Option 1 Value correctly
2. UnitTestOption1NameDefaultsToVariantForBCOriginProduct - BC-origin product (no existing Shopify variants) gets 'Variant' as Option 1 Name
3. UnitTestOption1NameDefaultsToVariantWhenNoExistingVariants - No existing variants defaults to 'Variant'
4. UnitTestOption1NameDefaultsToVariantWhenAmbiguous - Ambiguous case (multiple different Option 1 Names) defaults to 'Variant'
5. UnitTestOption1NameAndValuePreservedOnUpdatePath - Update path: pre-existing Option 1 Name is preserved and Option 1 Value is set correctly

All 5 new tests pass locally on BC v29. Existing codeunit 139601 (Product Export) also passes - no regression introduced.

[tVisionDeb]

Hi @onbuyuka - thank you for the thorough review on the previous PR (#8048). I have addressed all your feedback:

Changes made:

1. JustResolvedOption1Name guard - The fix now correctly sets Option 1 Value whenever the name was just resolved from Shopify (regardless of the resolved value), not just when the name is literally 'Variant'.

2. ResolveOption1Name procedure - Extracted into a clean local procedure ResolveOption1Name(ProductId: BigInteger): Text[50] that queries existing Shpfy Variant records for the product and returns the common name if unambiguous, or falls back to 'Variant'.

3. Test coverage - Added new test codeunit 139649 "Shpfy Variant Option1Name Test" with 5 test methods covering all the cases discussed in the review:

   • Shopify-origin product inherits Option 1 Name from existing variants
   • BC-origin product defaults to 'Variant'
   • No existing variants defaults to 'Variant'
   • Ambiguous (conflicting names) defaults to 'Variant'
   • Update path preserves existing name and sets value correctly

Local test results (BC v29 container):

• Codeunit 139649 (new tests): PASSED (all 5 tests)
• Codeunit 139601 (existing Product Export tests): PASSED (no regression)

Ready for re-review.

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Performance} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Uncached DB read per variant export in ResolveOption1Name

ResolveOption1Name executes a full FindSet() on Shpfy Variant every time FillInProductVariantData is called with a blank Option 1 Name. For a product with N blank-name variants being exported in a single run, this produces N redundant database reads returning the same result for the same Product ID.

Recommendation:

• Cache the resolved name at the product level before iterating over variants, e.g. compute it once per product and pass it in, or use a local dictionary keyed by Product ID within the calling loop.

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Security} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Missing shop code filter in ResolveOption1Name

ResolveOption1Name filters Shpfy Variant only by 'Product Id', ignoring 'Shop Code'. In a multi-shop deployment, a variant from a different shop that happens to share the same numeric Product ID would be included in the lookup, potentially returning that shop's Option 1 Name for the wrong shop's product.

Recommendation:

• Add a shop code filter before FindSet(). Pass the shop code as a parameter (or read it from the existing ShopifyVariant record that is already in scope at the call site).

Suggested change

	ShpfyVariant.SetRange("Product Id", ProductId);
	local procedure ResolveOption1Name(ProductId: BigInteger; ShopCode: Code[20]): Text[50]
	var
	ShpfyVariant: Record "Shpfy Variant";
	...
	begin
	ShpfyVariant.SetRange("Shop Code", ShopCode);
	ShpfyVariant.SetRange("Product Id", ProductId);
	if not ShpfyVariant.FindSet() then
	exit('Variant');

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Style} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Random(999999) IDs risk cross-test data collisions

ShopifyProduct.Id and ShopifyVariant.Id are assigned with Random(999999). The small range means that concurrent or sequential test runs in a shared test database can produce duplicate IDs across different test methods, causing ResolveOption1Name to pick up variants belonging to a different test's product and producing intermittent, hard-to-diagnose failures.

Recommendation:

• Use a higher-range random (e.g. Random(2147483647)) or derive a deterministic unique ID from the test context. If a sequence table is available, prefer that. At minimum, ensure variants are cleaned up per test or filtered with a unique shop code per test method.

Suggested change

	begin
	ShopifyProduct.Id := Random(2147483647);
	ShopifyVariant.Id := Random(2147483647);

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