BRD-1030-Localize-productUrl-per-locale-in-ShopifyAdapter

[PauliusInvertus]

Fixes localized product URLs in indexed Shopify documents. Per-locale productUrl_ now uses the locale prefix (/lt/...) and the translated handle from the Shopify translations payload, instead of duplicating the default-locale URL across every locale.

Build productUrl_ with the active locale prefix and the
translated handle when present, falling back to the default handle.
Preserves preview query strings for dev stores.

[gemini-code-assist]

Code Review

This pull request introduces localized product URL generation for the Shopify adapter. It adds logic to extract product handles and construct URLs that incorporate locale prefixes and translated handles while preserving existing query strings. Feedback suggests also preserving URL fragments (anchors) during reconstruction to avoid potential data loss for deep-linked URLs.

[PauliusInvertus]

@claude review

[claude]

Claude finished @PauliusInvertus's task in 1m 19s —— View job

---

Code Review

• Gather context
• Read changed files
• Analyze code changes
• Post review feedback

---

The implementation is clean and well-structured. The two new private methods are focused, the fallback chain is sound, and the existing tests are correctly updated. A few notes:

---

Fragment not preserved (agree with gemini-code-assist)

buildLocaleProductUrl at line 485 preserves the query string but silently drops URL fragments (#section, #variant-anchor). Shopify themes sometimes deep-link to a variant or section via the fragment. One-line fix:

        if (isset($parsed['query']) && $parsed['query'] !== '') {
            $url .= '?' . $parsed['query'];
        }
        if (isset($parsed['fragment']) && $parsed['fragment'] !== '') {
            $url .= '#' . $parsed['fragment'];
        }

Fix this →

---

Primary locale URL is reconstructed, not passed through

When $locale === $primaryLocale, the method still rebuilds the URL from its parsed parts (scheme://host/products/handle?query). This is fine for typical Shopify stores, but if a store's onlineStoreUrl ever has a path prefix (e.g. /en/products/snowboard where en is the primary locale), the prefix is silently stripped. This is a low-risk edge case given Shopify's conventions, but worth a comment near the $isPrimary block explaining the assumption.

---

Missing test: URL-fallback path in extractDefaultHandle

The method has a fallback that parses the handle from the URL when the handle GraphQL field is absent. There's no test covering this path — it's exercised by the preview-URL test but handle is set there. A test with handle absent would confirm the regex works:

public function testExtractDefaultHandleFallsBackToUrlParsing(): void
{
    $product = $this->makeProduct(...);
    // no $product['node']['handle'] set
    $product['node']['onlineStoreUrl'] = 'https://shop.example.com/products/my-board';
    // assert productUrl_lt becomes https://shop.example.com/lt/products/my-board
}

Fix this →

---

Minor: buildLocaleFields parameter count

The method now takes 12 parameters. Not a blocker, but worth flagging — grouping URL-related params into a small value object (or just the $defaultHandle + $productUrl into a ProductUrl struct) would make future additions easier. Skip for this PR if out of scope.

---

What's good

• Fallback chain is solid: GraphQL handle → URL-parsed handle → original URL unchanged.
• Primary locale correctly skips locale prefix and translated handle.
• Query string preserved for dev-store preview tokens — well thought out.
• All three new test cases are meaningful and cover the happy path properly.
• Docblock comments explain the why, not just the what.

Overall this is a correct and clean implementation. The fragment issue and the missing URL-fallback test are the only concrete items worth addressing before merge.