MAE-1319: Add ArrayFieldFilter for GoCardless webhook routing#14
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request implements a flexible filtering mechanism for Svix webhook routing to accommodate GoCardless payloads. By moving from a hardcoded filtering approach to a strategy-based pattern, the system can now correctly handle both simple top-level field matching (Stripe) and complex array-based nested field matching (GoCardless). Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a strategy pattern for webhook filtering by adding an ArrayFieldFilter class, specifically designed to handle payloads with arrays of events such as those from GoCardless. It refactors JavaScript string escaping into a shared trait and updates the SvixProcessorConfig enum to provide processor-specific filter strategies. The review feedback identifies opportunities to improve the robustness of the generated JavaScript by validating input paths and ensuring null-safety for nested array accessors.
GoCardless webhooks contain an events array where each event has a nested links.organisation field. Unlike Stripe where account is a top-level field, a single GoCardless webhook can contain events for multiple organisations. The new ArrayFieldFilter iterates the array and matches if ANY item contains the target organisation. - Extract escapeJsString() into shared EscapesJsStrings trait - Add ArrayFieldFilter implementing FilterStrategyInterface - Add getFilterStrategy() to SvixProcessorConfig enum - Update SvixWebhookMiddleware to use config-driven filter strategy
187b90b to
3eba010
Compare
Overview
Add a new
ArrayFieldFilterstrategy to support GoCardless webhook routing via Svix. GoCardless webhooks contain aneventsarray where each event has a nestedlinks.organisationfield — unlike Stripe whereaccountis a top-level field. This filter iterates the array and matches if ANY item contains the target organisation, forwarding the full payload to the destination.Before
Only
SimpleFieldFilterexisted, which checks a single top-level field. This works for Stripe (account) but not for GoCardless's array-based multi-organisation payloads. The middleware hardcodedbuildRoutingFilter()for all processor types.After
ArrayFieldFiltergenerates JavaScript that iterates the events array with null-safe nested field accessSvixProcessorConfig::getFilterStrategy()returns the correct filter strategy per processor type (Stripe →SimpleFieldFilter, GoCardless →ArrayFieldFilter)SvixWebhookMiddlewareuses the config-driven filter strategy instead of the hardcodedbuildRoutingFilter()escapeJsString()extracted into sharedEscapesJsStringstrait used by both filtersTechnical Details
New files:
Civi/Svixclient/Filter/ArrayFieldFilter.php— ImplementsFilterStrategyInterface. Generates JS that iterates an array field, checks nested paths with null-safe guards, and returns the webhook on first matchCivi/Svixclient/Filter/EscapesJsStrings.php— Shared trait for JS string escaping (extracted fromSimpleFieldFilter)tests/phpunit/Civi/Svixclient/Filter/ArrayFieldFilterTest.php— 13 test cases covering JS generation, escaping, nested paths, andSvixProcessorConfigintegrationModified files:
Civi/Svixclient/Filter/SimpleFieldFilter.php— UsesEscapesJsStringstrait instead of private methodCivi/Svixclient/Enum/SvixProcessorConfig.php— NewgetFilterStrategy()methodCivi/Svixclient/Service/SvixWebhookMiddleware.php— Line 253:buildRoutingFilter()→buildFilter($config->getFilterStrategy())Generated JavaScript example (GoCardless):
Core overrides
N/A
Comments
SimpleFieldFilteris functionally unchangedbuildRoutingFilter()static method onCRM_Svixclient_Clientstill exists for backward compatibility but is no longer used internally