Allow integrations to have custom parameters for queries #26
Conversation
WalkthroughAdds ProductLoader; converts MemoryManager methods from static to instance and injects it into consumers; refactors ProductWalker to a private-constructor factory using injected ProductLoader, MemoryManager, and query args; introduces per-args option_key handling in async flows; adjusts JsonFileFeed filename logic; removes typed Container property; adds multiple unit tests and DI test helpers. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ate. Use the current time in the file name for uniqueness, but also `wp_hash()` (salt) to make it impossible to guess the last part of the filename.
There was a problem hiding this comment.
LGTM! Thanks for refactoring and enhancing the support for the POS catalog integration.
On the testing side:
- I tried
wp product-feed generate --integration=openai --silent, though opening the file in a JSON viewer app (Dadroit JSON Viewer) said that it's not valid JSON 🤔 - Tested integration in the core branch. Tried passing different
fieldsparameters there, which means different args for the generator, and the status is associated with each parameter 👍
src/Storage/JsonFileFeed.php
Outdated
| $hash_data = $this->base_name . gmdate( 'r' ); | ||
| $file_name = $this->base_name . '-' . time() . '-' . wp_hash( $hash_data ) . '.json'; |
There was a problem hiding this comment.
I would prefer this change in another PR, and then have a proper test for it.
There was a problem hiding this comment.
The approach looks good with introducing the interface method get_product_feed_query_args! Please keep finishing it, especially with tests, and move changes related to filename (#26 (review)) to another PR
RadoslavGeorgiev
force-pushed
the
wooptp-41-product-feed-allow-custom-pos-parameters
branch
from
d6ad43b to
5de6e1b
Compare
jaclync
left a comment
jaclync
left a comment
There was a problem hiding this comment.
Thanks for the follow-up enhancements! The action is now triggered right away in the local site consistently 🎉 Will test more in other sites. Left some non-blocking comments for future considerations.
|
From the POS side, would just like to hear your thoughts on the fields support p1761787711991059/1761032826.289079-slack-C09EWAXSYD9 - feel free to respond here or in the thread for more visibility 🙂 |
I have a concept in my mind, but it's not backed by immediate knowledge of how exactly the core REST API works. I'll try to close this PR today so I can open the next one, showcasing my idea. |
- Memory management is not tested yet. - Add a ProductLoader that can be mocked - Fix a bug in the walker where a round number of procucts (ex. 100 products and batch size 10) would require a useless additional iteration.
|
@htdat I'm marking this one as ready for review. I'm yet to add tests for settings, but I think those will mostly be incosequential. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/Core/Plugin.php (1)
31-31: Verify the necessity of removing type safety.The type declaration was removed from the
$containerproperty, reducing type safety. While the enriched summary mentions "more flexible runtime typing in tests," consider whether a better pattern exists (e.g., using an interface or abstract base class that both production and test containers implement) rather than sacrificing type safety.If the test container implements a common interface with the production Container, you could type the property as that interface instead:
private ContainerInterface $container;This would maintain type safety while allowing test container substitution. Can you confirm whether this pattern was considered, or if there's a specific technical constraint requiring the untyped property?
src/Feed/ProductLoader.php (1)
19-31: Consider adding parameter type hint for stricter typing.The wrapper pattern is appropriate for testability, though the
$argsparameter lacks a type hint. Consider declaring it asarray $argsfor better type safety, unless there's a specific reason to accept other types.Apply this diff if you want stricter typing:
- public function get_products( $args ) { + public function get_products( array $args ) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
src/CLI/Command.php(3 hunks)src/Core/Plugin.php(1 hunks)src/Feed/ProductLoader.php(1 hunks)src/Feed/ProductWalker.php(7 hunks)src/Integrations/IntegrationInterface.php(1 hunks)src/Integrations/OpenAi/OpenAiIntegration.php(4 hunks)src/Integrations/OpenAi/ScheduledActionManager.php(1 hunks)src/Integrations/POSCatalog/ApiController.php(1 hunks)src/Integrations/POSCatalog/AsyncGenerator.php(7 hunks)src/Integrations/POSCatalog/POSIntegration.php(2 hunks)src/Storage/JsonFileFeed.php(1 hunks)src/Utils/MemoryManager.php(3 hunks)tests/unit/ProductFeed/Feed/ProductWalkerTest.php(1 hunks)tests/unit/ProductFeed/ProductFeedTestCase.php(1 hunks)tests/unit/ProductFeed/Storage/JsonFileFeedTest.php(1 hunks)tests/unit/ProductFeed/TestContainer.php(1 hunks)tests/unit/ProductFeed/Utils/TestUtilsTest.php(1 hunks)tests/unit/bootstrap.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.php
📄 CodeRabbit inference engine (.cursorrules)
When suggesting with Tab or writing code, always obey the WordPress Coding Standards (project PHPCS config in phpcs.xml.dist)
Files:
src/Integrations/OpenAi/OpenAiIntegration.phpsrc/Integrations/POSCatalog/ApiController.phpsrc/Feed/ProductLoader.phpsrc/Utils/MemoryManager.phpsrc/Integrations/IntegrationInterface.phptests/unit/ProductFeed/Storage/JsonFileFeedTest.phptests/unit/ProductFeed/TestContainer.phpsrc/Storage/JsonFileFeed.phpsrc/CLI/Command.phpsrc/Core/Plugin.phptests/unit/ProductFeed/ProductFeedTestCase.phptests/unit/ProductFeed/Utils/TestUtilsTest.phpsrc/Integrations/POSCatalog/AsyncGenerator.phpsrc/Integrations/OpenAi/ScheduledActionManager.phptests/unit/bootstrap.phptests/unit/ProductFeed/Feed/ProductWalkerTest.phpsrc/Integrations/POSCatalog/POSIntegration.phpsrc/Feed/ProductWalker.php
🧠 Learnings (4)
📓 Common learnings
Learnt from: RadoslavGeorgiev
PR: woocommerce/OpenAI-Product-Feed#21
File: src/Utils/MemoryManager.php:35-60
Timestamp: 2025-10-24T17:54:03.161Z
Learning: In the woocommerce/OpenAI-Product-Feed repository, the MemoryManager::flush_caches() method uses aggressive cache clearing including direct $wp_object_cache property manipulation. This is an intentional, tried & tested pattern copied from another project that is necessary for generating very large product feeds (70k-200k products). Without this aggressive approach, large exports fail. The cache clearing happens after feed processing is complete, before shipping the file to another server, not during normal page execution.
📚 Learning: 2025-10-22T15:49:57.065Z
Learnt from: RadoslavGeorgiev
PR: woocommerce/OpenAI-Product-Feed#13
File: src/API/Controllers/ApiController.php:44-50
Timestamp: 2025-10-22T15:49:57.065Z
Learning: The ApiController in src/API/Controllers/ApiController.php is planned to be moved and refactored to be OpenAI platform-specific, which is why it currently requires concrete ProductMapper and FeedValidator implementations from the OpenAI platform rather than interface types.
Applied to files:
src/Integrations/OpenAi/OpenAiIntegration.phpsrc/Integrations/OpenAi/ScheduledActionManager.phpsrc/Feed/ProductWalker.php
📚 Learning: 2025-10-24T17:54:03.161Z
Learnt from: RadoslavGeorgiev
PR: woocommerce/OpenAI-Product-Feed#21
File: src/Utils/MemoryManager.php:35-60
Timestamp: 2025-10-24T17:54:03.161Z
Learning: In the woocommerce/OpenAI-Product-Feed repository, the MemoryManager::flush_caches() method uses aggressive cache clearing including direct $wp_object_cache property manipulation. This is an intentional, tried & tested pattern copied from another project that is necessary for generating very large product feeds (70k-200k products). Without this aggressive approach, large exports fail. The cache clearing happens after feed processing is complete, before shipping the file to another server, not during normal page execution.
Applied to files:
src/Feed/ProductLoader.phpsrc/Utils/MemoryManager.phpsrc/CLI/Command.phpsrc/Integrations/POSCatalog/AsyncGenerator.phpsrc/Feed/ProductWalker.php
📚 Learning: 2025-10-22T06:06:47.231Z
Learnt from: RadoslavGeorgiev
PR: woocommerce/OpenAI-Product-Feed#13
File: src/Platforms/OpenAI/ProductMapper.php:99-107
Timestamp: 2025-10-22T06:06:47.231Z
Learning: In the woocommerce/OpenAI-Product-Feed repository, exception messages should use escaping functions (like esc_html()) to sanitize translations before throwing, as translations may contain HTML. Do not suggest removing escaping from exception messages in this codebase.
Applied to files:
src/Integrations/POSCatalog/AsyncGenerator.phpsrc/Feed/ProductWalker.php
🧬 Code graph analysis (13)
src/Integrations/OpenAi/OpenAiIntegration.php (6)
src/Integrations/IntegrationInterface.php (5)
get_id(29-29)get_product_feed_query_args(66-66)create_feed(73-73)get_product_mapper(80-80)get_feed_validator(87-87)src/Integrations/POSCatalog/POSIntegration.php (5)
get_id(44-46)get_product_feed_query_args(51-55)create_feed(92-94)get_product_mapper(99-101)get_feed_validator(106-108)src/Integrations/POSCatalog/ProductMapper.php (2)
get_id(64-66)ProductMapper(22-339)src/Storage/JsonFileFeed.php (1)
JsonFileFeed(24-186)src/Core/DependencyManagement/Container.php (1)
get(53-55)src/Integrations/POSCatalog/FeedValidator.php (1)
FeedValidator(21-32)
src/Integrations/POSCatalog/ApiController.php (1)
src/Integrations/POSCatalog/AsyncGenerator.php (2)
force_regeneration(191-218)get_status(87-136)
src/Integrations/IntegrationInterface.php (3)
src/Core/Plugin.php (2)
activate(99-113)deactivate(118-122)src/Integrations/OpenAi/OpenAiIntegration.php (3)
activate(71-75)deactivate(80-85)get_product_feed_query_args(97-101)src/Integrations/POSCatalog/POSIntegration.php (3)
activate(78-80)deactivate(85-87)get_product_feed_query_args(51-55)
tests/unit/ProductFeed/Storage/JsonFileFeedTest.php (1)
src/Storage/JsonFileFeed.php (6)
JsonFileFeed(24-186)start(82-133)end(156-163)get_file_path(170-172)get_file_url(179-185)add_entry(141-149)
tests/unit/ProductFeed/TestContainer.php (1)
src/Core/DependencyManagement/RuntimeContainer.php (1)
RuntimeContainer(15-207)
src/CLI/Command.php (2)
src/Utils/MemoryManager.php (2)
MemoryManager(19-103)get_available_memory(25-33)src/Feed/ProductWalker.php (2)
ProductWalker(22-265)from_integration(117-156)
tests/unit/ProductFeed/ProductFeedTestCase.php (2)
tests/unit/ProductFeed/TestContainer.php (2)
TestContainer(12-29)reset_all_replacements(26-28)openai-product-feed-for-woo.php (1)
wpfoai_plugin(70-72)
tests/unit/ProductFeed/Utils/TestUtilsTest.php (6)
src/Core/DependencyManagement/ContainerException.php (1)
ContainerException(12-23)src/Integrations/OpenAi/OpenAiIntegration.php (1)
OpenAiIntegration(29-132)tests/unit/ProductFeed/ProductFeedTestCase.php (2)
ProductFeedTestCase(14-103)createMock(49-51)src/Utils/StringHelper.php (2)
StringHelper(19-45)bool_string(27-30)tests/unit/ProductFeed/TestContainer.php (2)
replace_with_concrete(19-21)reset_all_replacements(26-28)openai-product-feed-for-woo.php (1)
wpfoai_get_service(80-82)
src/Integrations/POSCatalog/AsyncGenerator.php (4)
src/Integrations/IntegrationInterface.php (2)
create_feed(73-73)get_id(29-29)src/Feed/ProductWalker.php (2)
ProductWalker(22-265)from_integration(117-156)src/Storage/JsonFileFeed.php (2)
get_file_url(179-185)get_file_path(170-172)src/Feed/FeedInterface.php (2)
get_file_url(48-48)get_file_path(41-41)
src/Integrations/OpenAi/ScheduledActionManager.php (1)
src/Feed/ProductWalker.php (2)
ProductWalker(22-265)from_integration(117-156)
tests/unit/ProductFeed/Feed/ProductWalkerTest.php (6)
tests/unit/ProductFeed/ProductFeedTestCase.php (2)
ProductFeedTestCase(14-103)createMock(49-51)src/Utils/MemoryManager.php (1)
MemoryManager(19-103)src/Feed/ProductLoader.php (1)
ProductLoader(19-32)tests/unit/ProductFeed/TestContainer.php (1)
replace_with_concrete(19-21)src/Feed/WalkerProgress.php (1)
WalkerProgress(15-60)src/Feed/ProductWalker.php (4)
ProductWalker(22-265)from_integration(117-156)set_batch_size(164-167)walk(186-231)
src/Integrations/POSCatalog/POSIntegration.php (5)
src/Integrations/IntegrationInterface.php (6)
get_id(29-29)get_product_feed_query_args(66-66)activate(47-47)deactivate(58-58)create_feed(73-73)get_product_mapper(80-80)src/Integrations/OpenAi/OpenAiIntegration.php (6)
get_id(90-92)get_product_feed_query_args(97-101)activate(71-75)deactivate(80-85)create_feed(106-108)get_product_mapper(113-116)src/Core/Plugin.php (3)
activate(99-113)deactivate(118-122)get(142-144)src/Storage/JsonFileFeed.php (1)
JsonFileFeed(24-186)src/Core/DependencyManagement/Container.php (1)
get(53-55)
src/Feed/ProductWalker.php (6)
src/Feed/ProductLoader.php (2)
ProductLoader(19-32)get_products(29-31)src/Utils/MemoryManager.php (3)
MemoryManager(19-103)get_available_memory(25-33)flush_caches(38-60)src/Integrations/IntegrationInterface.php (3)
get_product_feed_query_args(66-66)get_product_mapper(80-80)get_feed_validator(87-87)src/Integrations/OpenAi/OpenAiIntegration.php (3)
get_product_feed_query_args(97-101)get_product_mapper(113-116)get_feed_validator(121-124)src/Integrations/POSCatalog/POSIntegration.php (3)
get_product_feed_query_args(51-55)get_product_mapper(99-101)get_feed_validator(106-108)openai-product-feed-for-woo.php (1)
wpfoai_get_service(80-82)
🔇 Additional comments (19)
tests/unit/bootstrap.php (1)
106-110: LGTM!The test bootstrap correctly loads ProductFeed test utilities after dependency injection initialization, enabling standardized test helpers across the test suite.
tests/unit/ProductFeed/Storage/JsonFileFeedTest.php (1)
1-126: LGTM!Comprehensive test coverage for JsonFileFeed including file creation, entry handling, error scenarios, and proper test isolation with cleanup helpers.
src/Integrations/POSCatalog/ApiController.php (1)
82-93: LGTM!Initializes parameter support for future extensibility and appropriately removes sensitive fields (action_id, path) from the API response before returning to the client.
src/Integrations/IntegrationInterface.php (1)
60-66: LGTM!The new
get_product_feed_query_args()method appropriately extends the integration contract, enabling each integration to specify custom product query arguments. The@seereference towc_get_products()helpfully clarifies the expected format.src/Storage/JsonFileFeed.php (1)
104-115: LGTM!The new deterministic file naming scheme (base_name + timestamp + hash) is well-documented with a reference to the GitHub discussion. The prior review concern about testing is now addressed with comprehensive test coverage in JsonFileFeedTest.php.
src/CLI/Command.php (2)
35-53: LGTM!Properly applies dependency injection for MemoryManager, enabling instance-based memory management and improving testability.
126-126: LGTM!The ProductWalker factory method
from_integration()centralizes walker initialization and ensures consistent dependency injection across integration contexts.src/Integrations/OpenAi/OpenAiIntegration.php (2)
56-56: LGTM!Adopting
{@inheritdoc}reduces documentation duplication and aligns with the feedback from past reviews, improving maintainability.Also applies to: 69-69, 78-78, 88-88, 103-103, 111-111, 119-119, 127-127
94-101: LGTM!The implementation correctly specifies query arguments for the OpenAI feed (simple and variation product types), fulfilling the IntegrationInterface contract and supporting the centralized ProductWalker initialization flow.
src/Integrations/POSCatalog/POSIntegration.php (1)
48-55: LGTM! POS integration correctly includes variable products.The POS integration appropriately includes
'variable'products in the feed query args, which distinguishes it from the OpenAI integration that only includes'simple'and'variation'types. This aligns with the PR objectives stating that POS requires parent products while OpenAI treats variations as purchasable products.src/Utils/MemoryManager.php (2)
25-33: Conversion to instance method enables dependency injection.The refactor from static to instance method supports the broader architectural shift toward dependency injection, allowing MemoryManager to be injected into ProductWalker and tested with mocks.
38-60: Instance method conversion completed correctly.The flush_caches method and its dependency on collect_garbage have been properly updated to use instance-based calls (
$this->), and collect_garbage visibility has been appropriately reduced to private.tests/unit/ProductFeed/TestContainer.php (1)
12-28: LGTM! Test utility provides clean DI replacement.The TestContainer appropriately extends RuntimeContainer to provide test-specific mock replacement and reset capabilities. The direct manipulation of protected
resolved_cacheis valid within the subclass and enables flexible test setup.src/Integrations/OpenAi/ScheduledActionManager.php (1)
69-71: Factory pattern simplifies walker instantiation.The migration to
ProductWalker::from_integration()successfully centralizes dependency wiring and simplifies the caller code. The integration provides the necessary components (mapper, validator, query args) while the feed remains externally provided as intended.tests/unit/ProductFeed/Utils/TestUtilsTest.php (1)
15-53: Comprehensive test coverage for container replacement.The test effectively validates container replacement functionality including class substitution, interface binding, mock injection, and reset behavior. The test structure is clear and assertions are appropriate.
tests/unit/ProductFeed/Feed/ProductWalkerTest.php (1)
60-246: Excellent comprehensive test coverage for ProductWalker.This test thoroughly validates the refactored ProductWalker including:
- Pagination with various batch sizes and product counts
- Memory management sequencing (availability checks and cache flushes)
- Validation error handling (11th product rejection)
- Filter application (wpfoai_product_feed_args)
- Progress callback reporting
- Feed lifecycle (start/add_entry/end)
The test structure is well-organized despite its complexity, with clear sections for mock setup, expectations, and execution.
tests/unit/ProductFeed/ProductFeedTestCase.php (3)
22-35: Test lifecycle management is well-structured.The setUp and tearDown methods properly initialize and clean up the test container, ensuring test isolation through replacement resets.
53-88: Container replacement logic handles test isolation correctly.The
get_replacement_containermethod appropriately manages the static test container lifecycle, ensuring the plugin uses a testable container while preserving the original resolved cache. The reflection-based approach is necessary for modifying internal plugin state.
90-102: Standard reflection pattern for test access.The
make_property_accessiblehelper follows the standard PHPUnit pattern for accessing private/protected properties during testing.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/Integrations/POSCatalog/AsyncGenerator.php (1)
238-249: Key ordering normalization is still missing from the past comment's suggestion.While
$args ?? []correctly handles the null case, arrays with identical keys but different order (e.g.,['foo' => 1, 'bar' => 2]vs.['bar' => 2, 'foo' => 1]) will serialize differently and produce distinct hashes. This can lead to duplicate option keys, redundant generation jobs, and orphaned status options/files—the core issue flagged in the earlier review.Apply this diff to add stable key sorting before hashing:
private function get_option_key( ?array $args = null ): string { + $normalized_args = $args ?? []; + + if ( ! empty( $normalized_args ) ) { + ksort( $normalized_args ); + } + return 'feed_status_' . md5( // WPCS dislikes serialize for security reasons, but it will be hashed immediately. // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize serialize( [ 'integration' => $this->integration->get_id(), - 'args' => $args ?? [], + 'args' => $normalized_args, ] ) ); }If
$argscan contain nested arrays, consider a recursive sort or usejson_encode()withJSON_FORCE_OBJECTfor canonical representation.
🧹 Nitpick comments (2)
tests/unit/ProductFeed/ProductFeedTestCase.php (2)
22-29: Consider simplifying the test container initialization.The ternary
isset( $this->test_container ) ? $this->test_container : nullchecks if the property is set before it's assigned. In standard PHPUnit usage,setUp()runs on a fresh instance per test, so$this->test_containerwould not typically be set. The pattern might accommodate subclass scenarios, but it adds cognitive overhead.If subclass flexibility isn't needed, simplify to:
- $this->test_container = $this->get_replacement_container( - isset( $this->test_container ) ? $this->test_container : null - ); + $this->test_container = $this->get_replacement_container();And update the method signature to make the parameter truly optional with a default value.
78-81: Deep reflection chain couples tests to container implementation.The code accesses
plugin->container->container->initial_resolved_cachethrough nested reflection calls. This is fragile and tightly couples the test infrastructure to specific container implementation details.Consider whether the container architecture could expose
initial_resolved_cachethrough a test-friendly accessor method to reduce reflection depth and improve maintainability. If the nested structure is temporary or specific to the current implementation, document why this depth is necessary.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Integrations/POSCatalog/AsyncGenerator.php(7 hunks)tests/unit/ProductFeed/ProductFeedTestCase.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.php
📄 CodeRabbit inference engine (.cursorrules)
When suggesting with Tab or writing code, always obey the WordPress Coding Standards (project PHPCS config in phpcs.xml.dist)
Files:
tests/unit/ProductFeed/ProductFeedTestCase.phpsrc/Integrations/POSCatalog/AsyncGenerator.php
🧠 Learnings (2)
📚 Learning: 2025-10-24T17:54:03.161Z
Learnt from: RadoslavGeorgiev
PR: woocommerce/OpenAI-Product-Feed#21
File: src/Utils/MemoryManager.php:35-60
Timestamp: 2025-10-24T17:54:03.161Z
Learning: In the woocommerce/OpenAI-Product-Feed repository, the MemoryManager::flush_caches() method uses aggressive cache clearing including direct $wp_object_cache property manipulation. This is an intentional, tried & tested pattern copied from another project that is necessary for generating very large product feeds (70k-200k products). Without this aggressive approach, large exports fail. The cache clearing happens after feed processing is complete, before shipping the file to another server, not during normal page execution.
Applied to files:
src/Integrations/POSCatalog/AsyncGenerator.php
📚 Learning: 2025-10-22T06:06:47.231Z
Learnt from: RadoslavGeorgiev
PR: woocommerce/OpenAI-Product-Feed#13
File: src/Platforms/OpenAI/ProductMapper.php:99-107
Timestamp: 2025-10-22T06:06:47.231Z
Learning: In the woocommerce/OpenAI-Product-Feed repository, exception messages should use escaping functions (like esc_html()) to sanitize translations before throwing, as translations may contain HTML. Do not suggest removing escaping from exception messages in this codebase.
Applied to files:
src/Integrations/POSCatalog/AsyncGenerator.php
🧬 Code graph analysis (2)
tests/unit/ProductFeed/ProductFeedTestCase.php (2)
tests/unit/ProductFeed/TestContainer.php (2)
TestContainer(12-29)reset_all_replacements(26-28)openai-product-feed-for-woo.php (1)
wpfoai_plugin(70-72)
src/Integrations/POSCatalog/AsyncGenerator.php (7)
src/Integrations/IntegrationInterface.php (2)
create_feed(73-73)get_id(29-29)src/Integrations/POSCatalog/POSIntegration.php (2)
create_feed(92-94)get_id(44-46)src/Integrations/OpenAi/OpenAiIntegration.php (2)
create_feed(106-108)get_id(90-92)src/Feed/ProductWalker.php (2)
ProductWalker(22-265)from_integration(117-156)src/Feed/WalkerProgress.php (1)
WalkerProgress(15-60)src/Storage/JsonFileFeed.php (2)
get_file_url(179-185)get_file_path(170-172)src/Feed/FeedInterface.php (2)
get_file_url(48-48)get_file_path(41-41)
🔇 Additional comments (5)
tests/unit/ProductFeed/ProductFeedTestCase.php (3)
37-51: LGTM! Good use of generics for type safety.This override provides better IDE support and static analysis by combining
MockObjectwith the mocked class type. The runtime behavior is unchanged, and the phpcs:ignore is appropriate since the linter cannot recognize the static analysis benefit.
59-88: Verify that static test container sharing across all test classes is safe.The static
$test_containeron line 61 is shared across all tests in all test classes that extendProductFeedTestCase. The comment confirms this is intentional, but it could lead to test pollution if one test class modifies the container in ways that affect subsequent test classes.Confirm that:
- All test classes properly reset replacements in
tearDown()(they do via line 34).- The
initial_resolved_cachecaptured at container creation is sufficient to restore clean state.- Test execution order won't cause cross-contamination between different test suites.
If test isolation issues arise, consider using a static property per test class rather than a globally shared static variable.
90-102: LGTM! Clean reflection helper.Standard pattern for accessing private/protected properties in tests. The implementation is straightforward and well-documented.
src/Integrations/POSCatalog/AsyncGenerator.php (2)
276-304: Solid defensive validation logic.The implementation correctly guards against two edge cases: completed feeds with missing files (manual deletion or external interference) and scheduled jobs stuck beyond 10 minutes (Action Scheduler throttling or failure). Returning
falsetriggers regeneration viaget_status(), which is the appropriate recovery path.
156-156: Good use of factory pattern for walker instantiation.The
ProductWalker::from_integration()factory correctly centralizes initialization logic, allowing each integration to provide custom query args, mappers, and validators. This aligns well with the PR's goal of supporting multi-integration usage with per-integration parameters.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/Integrations/POSCatalog/AsyncGenerator.php (2)
87-100: Add type guard before callingvalidate_status().If the option is corrupted (e.g., manually modified to a non-array value),
get_option()can return a value that passes thefalse !== $statuscheck but will trigger aTypeErrorwhen passed tovalidate_status(array $status). For consistency with line 147, add anis_array()guard here.Apply this diff:
- if ( false !== $status && ! $this->validate_status( $status ) ) { + if ( false !== $status && ( ! is_array( $status ) || ! $this->validate_status( $status ) ) ) { $status = false; }
191-198: Add type guard inforce_regeneration()as well.The same type-safety issue exists here: if the option is corrupted to a non-array,
validate_status()will throw aTypeError. Add anis_array()check before callingvalidate_status().Apply this diff:
- if ( false === $status || ! $this->validate_status( $status ) ) { + if ( false === $status || ! is_array( $status ) || ! $this->validate_status( $status ) ) { return $this->get_status( $args ); }
🧹 Nitpick comments (1)
src/Integrations/POSCatalog/AsyncGenerator.php (1)
238-254: Consider recursive normalization for nested arrays.The current normalization uses
ksort(), which only sorts top-level keys. If$argscontains nested arrays, their key order won't be normalized, and semantically identical argument sets could produce different hashes (e.g.,['a' => ['z' => 1, 'y' => 2]]vs.['a' => ['y' => 2, 'z' => 1]]). If nested parameters are expected in the future, consider a recursive sort to ensure canonical representation.Example recursive normalization helper:
private function normalize_args( $args ) { if ( ! is_array( $args ) ) { return $args; } ksort( $args ); foreach ( $args as $key => $value ) { $args[ $key ] = $this->normalize_args( $value ); } return $args; }Then use it in
get_option_key():private function get_option_key( ?array $args = null ): string { - $normalized_args = $args ?? []; - if ( ! empty( $normalized_args ) ) { - ksort( $normalized_args ); - } + $normalized_args = $this->normalize_args( $args ?? [] ); return 'feed_status_' . md5(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Integrations/POSCatalog/AsyncGenerator.php(7 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.php
📄 CodeRabbit inference engine (.cursorrules)
When suggesting with Tab or writing code, always obey the WordPress Coding Standards (project PHPCS config in phpcs.xml.dist)
Files:
src/Integrations/POSCatalog/AsyncGenerator.php
🧠 Learnings (2)
📚 Learning: 2025-10-24T17:54:03.161Z
Learnt from: RadoslavGeorgiev
PR: woocommerce/OpenAI-Product-Feed#21
File: src/Utils/MemoryManager.php:35-60
Timestamp: 2025-10-24T17:54:03.161Z
Learning: In the woocommerce/OpenAI-Product-Feed repository, the MemoryManager::flush_caches() method uses aggressive cache clearing including direct $wp_object_cache property manipulation. This is an intentional, tried & tested pattern copied from another project that is necessary for generating very large product feeds (70k-200k products). Without this aggressive approach, large exports fail. The cache clearing happens after feed processing is complete, before shipping the file to another server, not during normal page execution.
Applied to files:
src/Integrations/POSCatalog/AsyncGenerator.php
📚 Learning: 2025-10-22T06:06:47.231Z
Learnt from: RadoslavGeorgiev
PR: woocommerce/OpenAI-Product-Feed#13
File: src/Platforms/OpenAI/ProductMapper.php:99-107
Timestamp: 2025-10-22T06:06:47.231Z
Learning: In the woocommerce/OpenAI-Product-Feed repository, exception messages should use escaping functions (like esc_html()) to sanitize translations before throwing, as translations may contain HTML. Do not suggest removing escaping from exception messages in this codebase.
Applied to files:
src/Integrations/POSCatalog/AsyncGenerator.php
🧬 Code graph analysis (1)
src/Integrations/POSCatalog/AsyncGenerator.php (7)
src/Integrations/IntegrationInterface.php (2)
create_feed(73-73)get_id(29-29)src/Integrations/OpenAi/OpenAiIntegration.php (2)
create_feed(106-108)get_id(90-92)src/Integrations/POSCatalog/POSIntegration.php (2)
create_feed(92-94)get_id(44-46)src/Feed/ProductWalker.php (2)
ProductWalker(22-265)from_integration(117-156)src/Feed/WalkerProgress.php (1)
WalkerProgress(15-60)src/Storage/JsonFileFeed.php (2)
get_file_url(179-185)get_file_path(170-172)src/Feed/FeedInterface.php (2)
get_file_url(48-48)get_file_path(41-41)
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/unit/ProductFeed/Integrations/OpenAi/SettingsTest.php (1)
70-75: Semantically incorrect default value.The default value
'https://example.com/'is a URL, butseller_nameshould be a name string. While this doesn't affect the test (sinceget_bloginfo('name')always returns a value), it's semantically confusing.Consider using a more appropriate default:
$this->assertEquals( get_bloginfo( 'name' ), - $this->sut->get( 'seller_name', 'https://example.com/' ) + $this->sut->get( 'seller_name', 'Default Store Name' ) );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Integrations/OpenAi/Settings.php(2 hunks)tests/unit/ProductFeed/Integrations/OpenAi/SettingsTest.php(1 hunks)tests/unit/bootstrap.php(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/unit/bootstrap.php
🧰 Additional context used
📓 Path-based instructions (1)
**/*.php
📄 CodeRabbit inference engine (.cursorrules)
When suggesting with Tab or writing code, always obey the WordPress Coding Standards (project PHPCS config in phpcs.xml.dist)
Files:
tests/unit/ProductFeed/Integrations/OpenAi/SettingsTest.phpsrc/Integrations/OpenAi/Settings.php
🧬 Code graph analysis (1)
tests/unit/ProductFeed/Integrations/OpenAi/SettingsTest.php (2)
tests/unit/ProductFeed/ProductFeedTestCase.php (1)
ProductFeedTestCase(14-103)src/Integrations/OpenAi/Settings.php (5)
Settings(19-174)get(39-70)get_endpoint_url(77-90)extend_providers(99-135)save_settings(143-163)
🔇 Additional comments (9)
src/Integrations/OpenAi/Settings.php (4)
46-47: LGTM!The simplified privacy URL retrieval with fallback is clear and correct.
48-51: LGTM!The terms and returns URL retrieval correctly chains WooCommerce functions with a safe fallback.
54-56: LGTM!The shop URL retrieval with fallback is implemented correctly.
144-146: LGTM!The conditional nonce check correctly bypasses verification during unit tests when the
PRODUCT_FEED_UNIT_TESTSconstant is defined and true, while maintaining security checks in production.tests/unit/ProductFeed/Integrations/OpenAi/SettingsTest.php (5)
28-43: LGTM!The test correctly validates both fallback and configured privacy URL scenarios. Using a product as a test fixture is unconventional but works fine for permalink testing.
45-68: LGTM!The test comprehensively validates both
tos_urlandreturns_urlretrieval with proper fallback handling.
77-142: LGTM!These tests provide comprehensive coverage of:
- Registry option retrieval with proper namespace precedence
- Endpoint URL behavior (configured, null, and filtered scenarios)
All logic is correct and assertions are appropriate.
144-169: LGTM!The
extend_providerstest correctly validates that new fields are added while preserving existing fields.
171-192: LGTM!The
save_settingstest correctly validates the settings persistence flow, including proper nonce setup and verification of the updated registry values. The nonce configuration properly supports the conditional check in the Settings class.
htdat
left a comment
htdat
left a comment
There was a problem hiding this comment.
LGTM with some inline comments to do a bit more testing and extensibility. Please address them and merge (without additional reviews)
If possible, I would still like the filename generation is ported to another PR https://github.com/woocommerce/OpenAI-Product-Feed/pull/26/files#r2472346171
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/Integrations/POSCatalog/AsyncGenerator.php (1)
238-254: Consider recursive key sorting for nested arrays.The current normalization performs a shallow
ksort()on top-level keys only. If$argsmay contain nested associative arrays where key order could vary (e.g.,['filters' => ['a' => 1, 'b' => 2]]vs['filters' => ['b' => 2, 'a' => 1]]), they would produce different hashes and duplicate option keys.If nested arrays are expected, apply a recursive sort before hashing:
private function get_option_key( ?array $args = null ): string { $normalized_args = $args ?? []; if ( ! empty( $normalized_args ) ) { - ksort( $normalized_args ); + $this->ksort_recursive( $normalized_args ); } return 'feed_status_' . md5( // WPCS dislikes serialize for security reasons, but it will be hashed immediately. // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize serialize( [ 'integration' => $this->integration->get_id(), 'args' => $normalized_args, ] ) ); } + +/** + * Recursively sorts array keys. + * + * @param array &$array The array to sort. + * @return void + */ +private function ksort_recursive( array &$array ): void { + ksort( $array ); + foreach ( $array as &$value ) { + if ( is_array( $value ) ) { + $this->ksort_recursive( $value ); + } + } +}tests/unit/ProductFeed/Storage/JsonFileFeedTest.php (2)
20-23: Add file cleanup to prevent test pollution.The tearDown method only removes filters but doesn't clean up feed files created during tests. This could lead to test pollution where files from previous test runs affect subsequent runs.
Consider cleaning up created files in tearDown:
public function tearDown(): void { parent::tearDown(); + $directory = wp_upload_dir()['basedir'] . '/product-feeds'; + if ( is_dir( $directory ) ) { + global $wp_filesystem; + WP_Filesystem(); + $wp_filesystem->rmdir( $directory, true ); + } remove_all_filters( 'wpfoai_feed_time' ); }
83-100: TypeError tests rely on PHP type system rather than explicit validation.These tests verify that calling methods before
start()results in errors by relying on PHP's type system (uninitialized properties causing TypeError when returned/used). While this approach works and keeps the implementation simple, explicit validation with descriptive error messages would provide a better developer experience.Consider adding explicit state validation in JsonFileFeed if clearer error messages would benefit consumers:
public function get_file_path(): string { if ( ! isset( $this->file_path ) ) { throw new \LogicException( 'Feed must be started before accessing file path.' ); } return $this->file_path; }This would require updating these tests to expect
\LogicExceptioninstead of\TypeError.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Integrations/POSCatalog/AsyncGenerator.php(7 hunks)src/Storage/JsonFileFeed.php(1 hunks)tests/unit/ProductFeed/Storage/JsonFileFeedTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Storage/JsonFileFeed.php
🧰 Additional context used
📓 Path-based instructions (1)
**/*.php
📄 CodeRabbit inference engine (.cursorrules)
When suggesting with Tab or writing code, always obey the WordPress Coding Standards (project PHPCS config in phpcs.xml.dist)
Files:
tests/unit/ProductFeed/Storage/JsonFileFeedTest.phpsrc/Integrations/POSCatalog/AsyncGenerator.php
🧠 Learnings (2)
📚 Learning: 2025-10-24T17:54:03.161Z
Learnt from: RadoslavGeorgiev
Repo: woocommerce/OpenAI-Product-Feed PR: 21
File: src/Utils/MemoryManager.php:35-60
Timestamp: 2025-10-24T17:54:03.161Z
Learning: In the woocommerce/OpenAI-Product-Feed repository, the MemoryManager::flush_caches() method uses aggressive cache clearing including direct $wp_object_cache property manipulation. This is an intentional, tried & tested pattern copied from another project that is necessary for generating very large product feeds (70k-200k products). Without this aggressive approach, large exports fail. The cache clearing happens after feed processing is complete, before shipping the file to another server, not during normal page execution.
Applied to files:
src/Integrations/POSCatalog/AsyncGenerator.php
📚 Learning: 2025-10-22T06:06:47.231Z
Learnt from: RadoslavGeorgiev
Repo: woocommerce/OpenAI-Product-Feed PR: 13
File: src/Platforms/OpenAI/ProductMapper.php:99-107
Timestamp: 2025-10-22T06:06:47.231Z
Learning: In the woocommerce/OpenAI-Product-Feed repository, exception messages should use escaping functions (like esc_html()) to sanitize translations before throwing, as translations may contain HTML. Do not suggest removing escaping from exception messages in this codebase.
Applied to files:
src/Integrations/POSCatalog/AsyncGenerator.php
🧬 Code graph analysis (2)
tests/unit/ProductFeed/Storage/JsonFileFeedTest.php (2)
tests/unit/ProductFeed/ProductFeedTestCase.php (2)
ProductFeedTestCase(14-103)tearDown(31-35)src/Storage/JsonFileFeed.php (6)
JsonFileFeed(24-193)start(82-140)end(163-170)get_file_path(177-179)get_file_url(186-192)add_entry(148-156)
src/Integrations/POSCatalog/AsyncGenerator.php (6)
src/Integrations/IntegrationInterface.php (2)
create_feed(73-73)get_id(29-29)src/Integrations/OpenAi/OpenAiIntegration.php (2)
create_feed(106-108)get_id(90-92)src/Integrations/POSCatalog/POSIntegration.php (2)
create_feed(92-94)get_id(44-46)src/Feed/ProductWalker.php (2)
ProductWalker(22-265)from_integration(117-156)src/Feed/WalkerProgress.php (1)
WalkerProgress(15-60)src/Storage/JsonFileFeed.php (2)
get_file_url(186-192)get_file_path(177-179)
🔇 Additional comments (7)
src/Integrations/POSCatalog/AsyncGenerator.php (5)
12-13: LGTM: Imports and constants updated for multi-integration support.The imports, documentation, and constant updates align well with the PR's goal of making AsyncGenerator reusable across integrations. The 24-hour expiry and generic naming are appropriate.
Also applies to: 22-22, 30-30, 37-37, 45-45
84-136: LGTM: Per-args option key enables multi-parameter feed generation.The refactor to derive option keys from both integration ID and args enables the same integration to generate distinct feeds for different parameter sets. Race condition handling via unscheduling and the manual queue runner dispatch for immediate processing are appropriate for the use case.
144-182: LGTM: Action callback correctly uses dynamic option key.The method correctly receives and uses the dynamic
$option_keythroughout for status persistence. The migration toProductWalker::from_integration()aligns with the broader PR refactor.
191-218: LGTM: Force regeneration correctly handles dynamic option keys.The method correctly derives the option key from args and handles each state appropriately. The logic ensures that regeneration is scoped to the specific args passed in.
227-230: LGTM: Deletion action signature updated correctly.The updated signature correctly accepts both
$option_keyand$path, matching the scheduling call infeed_generation_action().tests/unit/ProductFeed/Storage/JsonFileFeedTest.php (2)
25-49: Hash verification approach differs from past review suggestion but is valid.The test verifies that the expected hash is present in the file path (line 41) by pre-computing it with the known timestamp. The past review comment from htdat suggested extracting the timestamp from the filename and then verifying the hash, which would require implementation changes to embed the full timestamp (not just Y-m-d date) in the filename.
The current approach is deterministic and correctly validates hash generation by controlling the time via the
wpfoai_feed_timefilter. However, it doesn't independently extract and verify the hash from the filename as originally suggested.Based on past review comments from htdat.
102-121: Excellent error handling test with proper cleanup.This test correctly validates the exception thrown when directory creation fails, and the try-finally block ensures cleanup of the blocking file regardless of test outcome.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Integrations/POSCatalog/AsyncGenerator.php (1)
238-254: Consider recursive sorting for nested arguments.The normalization correctly handles
nullvs[], andksortprovides stable ordering for top-level keys. However, if$argscontains nested arrays, different key orderings in those nested structures will produce different hashes for semantically equivalent arguments, potentially causing duplicate feed generations.Consider using a recursive sort to ensure nested arrays are also canonicalized:
private function get_option_key( ?array $args = null ): string { $normalized_args = $args ?? []; if ( ! empty( $normalized_args ) ) { - ksort( $normalized_args ); + $this->normalize_args_recursive( $normalized_args ); } return 'feed_status_' . md5( // WPCS dislikes serialize for security reasons, but it will be hashed immediately. // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize serialize( [ 'integration' => $this->integration->get_id(), 'args' => $normalized_args, ] ) ); } + +/** + * Recursively sorts array keys to ensure consistent hashing. + * + * @param array &$array The array to normalize (passed by reference). + * @return void + */ +private function normalize_args_recursive( array &$array ): void { + ksort( $array ); + foreach ( $array as &$value ) { + if ( is_array( $value ) ) { + $this->normalize_args_recursive( $value ); + } + } +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Integrations/POSCatalog/AsyncGenerator.php(7 hunks)tests/unit/ProductFeed/Storage/JsonFileFeedTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/unit/ProductFeed/Storage/JsonFileFeedTest.php
🧰 Additional context used
📓 Path-based instructions (1)
**/*.php
📄 CodeRabbit inference engine (.cursorrules)
When suggesting with Tab or writing code, always obey the WordPress Coding Standards (project PHPCS config in phpcs.xml.dist)
Files:
src/Integrations/POSCatalog/AsyncGenerator.php
🧠 Learnings (2)
📚 Learning: 2025-10-24T17:54:03.161Z
Learnt from: RadoslavGeorgiev
Repo: woocommerce/OpenAI-Product-Feed PR: 21
File: src/Utils/MemoryManager.php:35-60
Timestamp: 2025-10-24T17:54:03.161Z
Learning: In the woocommerce/OpenAI-Product-Feed repository, the MemoryManager::flush_caches() method uses aggressive cache clearing including direct $wp_object_cache property manipulation. This is an intentional, tried & tested pattern copied from another project that is necessary for generating very large product feeds (70k-200k products). Without this aggressive approach, large exports fail. The cache clearing happens after feed processing is complete, before shipping the file to another server, not during normal page execution.
Applied to files:
src/Integrations/POSCatalog/AsyncGenerator.php
📚 Learning: 2025-10-22T06:06:47.231Z
Learnt from: RadoslavGeorgiev
Repo: woocommerce/OpenAI-Product-Feed PR: 13
File: src/Platforms/OpenAI/ProductMapper.php:99-107
Timestamp: 2025-10-22T06:06:47.231Z
Learning: In the woocommerce/OpenAI-Product-Feed repository, exception messages should use escaping functions (like esc_html()) to sanitize translations before throwing, as translations may contain HTML. Do not suggest removing escaping from exception messages in this codebase.
Applied to files:
src/Integrations/POSCatalog/AsyncGenerator.php
🧬 Code graph analysis (1)
src/Integrations/POSCatalog/AsyncGenerator.php (4)
src/Integrations/POSCatalog/POSIntegration.php (2)
create_feed(92-94)get_id(44-46)src/Integrations/IntegrationInterface.php (2)
create_feed(73-73)get_id(29-29)src/Feed/ProductWalker.php (1)
ProductWalker(22-265)src/Storage/JsonFileFeed.php (2)
get_file_url(186-192)get_file_path(177-179)
🔇 Additional comments (7)
src/Integrations/POSCatalog/AsyncGenerator.php (7)
12-13: LGTM! Changes align with past feedback.The import additions, constant renaming to remove POS-specific naming, and the 24-hour feed expiry align with previous review discussions.
Also applies to: 30-30, 37-37, 45-45
87-136: LGTM! Well-structured per-args status management.The method correctly derives a unique option key per integration and arguments, validates existing status, and properly clears stale actions before scheduling. The manual ActionScheduler dispatch is appropriately guarded with class existence checks.
144-182: LGTM! Properly parameterized generation callback.The method correctly accepts and uses the option_key parameter throughout, integrates with the new ProductWalker factory method, and schedules deletion with the appropriate parameters.
191-218: LGTM! Consistent regeneration logic.The method properly derives the option key from arguments, validates status, and handles state transitions correctly. Cleanup and restart logic appropriately passes args through to get_status.
227-230: LGTM! Clean deletion handling.The updated signature accepts both option_key and path, and correctly deletes the option before the file to prevent orphaned status entries.
281-318: LGTM! Comprehensive status validation.The method thoroughly validates completed feeds (file existence) and scheduled jobs (timeout detection), with a filterable timeout threshold properly documented. All edge cases from previous reviews have been addressed.
263-270: LGTM! Progress tracking logic is sound.The progress calculation and status updates are correct.
Description
This PR centralizes
ProductWalkerinitialization, delegates query argument preparation to integrations, and allows theAsyncGeneratorto work with custom parameters, treating each set of integration + parameters as an individual entity.The changes are being implemented for multiple reasons:
AsyncGenerator's abstraction to work with multiple integrations.Walker initialization
Rather than instantiating the Walker directly in multiple places, that is now done through a centralized static factory method of the Walker. The thinking behind the method is as follows:
FeedInterface) as an external parameter. Even though all feeds areJsonFileFeedcurrently, that might easily change based not only on the provider but also on context (CLI vs REST, etc.)wpfoai_product_feed_argsfilter to include the integration as an argument for integration-specific hooks.Allow individual arguments in the async generator
The generator moves away from the single option that was previously used to a dynamic option key, based on the integration and additional external parameters.
External parameters can be provided by other actors like the POS
ApiController. At this stage, they are always an empty array:https://github.com/woocommerce/OpenAI-Product-Feed/blob/f20480dd1066cf057079f6ee7c9737250d885bcf/src/Integrations/POSCatalog/ApiController.php#L82-L85
After some discussions in p1761032826289079-slack-C09EWAXSYD9 tomorrow, this will likely change. But even if it does not, the functionality will be available for when needed.
Tests
I've added tests for most files that were not covered with tests. Below are the exceptions. There are quite a few of them, but they seem... reasonable. Please don't hesitate to disagree with me.
PluginCLI\CommandDeliveryMethods\PushFilecheck_setup()and some failures. I can't properly test curl in unit testsUtils\MemoryManagerIntegrations\POSCatalog\*Integrations\OpenAi\DevHelpersIntegrations\OpenAi\ProductFieldsControllerIntegrations\OpenAi\FeedSchemais_field_required, but it is tested through theFeedValidator.Integrations\OpenAi\OpenAiIntegrationIntegrations\OpenAi\SettingsTesting instructions
This PR is focusing on abstraction/refactoring, bringing nearly no functional changes. The easiest way that covers the most ground would be to generate both an OpenAI feed through CLI and a POS one through the REST API:
The first step is to make sure that both files get generated. Once they are generated, compare both. The POS catalog should contain more entries, as it includes variable products.
Once the REST controller's parameters are defined and implemented, their outcome should be tested as well.
Checklist
npm run test:phpdoes not return errors.npm run lint:phpdoes not return errors.SettingsSummary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests