BRD-1014-Add-Shopify-collections-facet-to-ShopifyAdapter

[PauliusInvertus]

Consumes the connector's new collections payload and emits collection titles as a localized facet field per product
(collections_{locale}, or collections in plain mode), with translation fallback to the default title.

BRD-1014

[PauliusInvertus]

@claude review

[claude]

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

---

PR Review: BRD-Add-Shopify-collections-facet-to-ShopifyAdapter

• Gather context and understand the request
• Read src/Adapters/ShopifyAdapter.php
• Read tests/Adapters/ShopifyAdapterTest.php
• Analyze diff and review code quality, correctness, edge cases
• Post final review feedback

---

Overall the implementation is well-structured and consistent with the existing adapter patterns. The core logic in buildProductCollectionsMap and resolveCollectionTitles is clean, defensively coded, and the docblocks accurately reflect the data shapes. A few things worth addressing:

---

Issues

1. Missing test: plain mode (no-locales path) with collections

buildPlainFields now accepts and emits a collections field, but there is no test that exercises this code path (i.e. calling transform($data, []) with collections present). The three new tests all call transform($data, ['en', 'lt']) or transform($data, ['en']), so buildLocaleFields is always exercised. The buildPlainFields branch — including the fallback resolveCollectionTitles($productCollections, $primaryLocale, $primaryLocale) call — has zero coverage.

Fix this →

---

2. Misleading default value for $primaryLocale in buildPlainFields

src/Adapters/ShopifyAdapter.php:213

string $primaryLocale = 'en',

This default is never hit — buildPlainFields is always called from transformProduct with an explicit $primaryLocale. The 'en' hardcode is a leftover from the refactor and could mask a bug if the method signature is ever called without the argument (e.g. in future tests or subclasses). Since this is a private method with an explicit caller, the default can simply be removed.

Fix this →

---

Observations (non-blocking)

3. array_unique deduplication in resolveCollectionTitles is implicit

src/Adapters/ShopifyAdapter.php:316

return array_values(array_unique($titles));

If two collections share the same translated title (or both fall back to the same default), duplicates are silently dropped. This is probably desired — but it's worth a one-line comment since it's a deliberate choice and not just incidental. It would also be worth a test case if title collisions are a realistic scenario in production data.

---

4. Translation fallback behavior for the primary locale is clear, but asymmetric

In resolveCollectionTitles (line 307–309):

$title = $locale === $primaryLocale
    ? $entry['default']
    : ($entry['translations'][$locale] ?? $entry['default']);

For the primary locale the translations array is intentionally bypassed. This matches the convention used elsewhere in the adapter (e.g. resolveTranslationsForLocale returns null for the primary locale). The behavior is correct, but a comment explaining why translations are skipped for the primary locale would help future readers, since Shopify can return translation entries for the primary locale in some store configurations.

---

Tests: what's covered well

• Happy path with two locales, one collection translated and one not (fallback to default) ✅
• Product not listed in any collection omits the field ✅
• No collections key in input at all omits the field ✅
• Multi-product: per-product isolation of the GID map ✅

The overall implementation is solid. The only actionable item is the missing plain-mode test (point 1) and the defaulted parameter cleanup (point 2).

[gemini-code-assist]

Code Review

This pull request introduces Shopify collection integration into the product transformation pipeline, enabling localized collection titles and product-to-collection mapping. The review feedback suggests refactoring the translation extraction logic to use an existing helper method and correcting typos in the test data.