diff --git a/README.md b/README.md index 795afd8..ba37294 100644 --- a/README.md +++ b/README.md @@ -77,7 +77,9 @@ setono_meta_conversions_api: message_bus: messenger.default_bus # The pixels to send events to (empty by default). Alternatively provide pixels from your own source by - # aliasing Setono\MetaConversionsApiBundle\Provider\PixelProviderInterface to your own service + # aliasing Setono\MetaConversionsApiBundle\Provider\PixelProviderInterface to your own service. + # The access token is only needed for server side tracking: client side tracking renders fbq() calls, which + # only need the pixel id. A pixel without an access token is skipped server side, with a warning in the log pixels: - id: '%env(META_PIXEL_ID)%' access_token: '%env(META_ACCESS_TOKEN)%' diff --git a/UPGRADE.md b/UPGRADE.md index 42b32f2..1a2dc66 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -75,6 +75,13 @@ anything to `framework.messenger`. `SendEvent` is dispatched on your application `?ConsentContextInterface $consentContext` and `bool $consentEnabled` / `bool $clientSideEnabled` / `bool $serverSideEnabled` arguments. Adapt subclasses, decorators and custom service definitions. +## Pixel access token + +`pixels[].access_token` is no longer required. Client side tracking only needs the pixel id, so a client-side-only +setup no longer has to configure a dummy token. Server side, a pixel without an access token is skipped and logged as +a warning instead of being posted to Meta, rejected with a 400 and retried by Messenger until it lands in the failure +transport. + ## Test event code The `_testEventCode` / `_test_event_code` query parameter is no longer honoured unconditionally. diff --git a/composer.json b/composer.json index d0a00ae..7514ccd 100644 --- a/composer.json +++ b/composer.json @@ -12,6 +12,7 @@ "require": { "php": ">=8.1", "composer/semver": "^3.0", + "psr/log": "^1.1 || ^2.0 || ^3.0", "setono/bot-detection-bundle": "^1.7", "setono/consent-contracts": "^1.1", "setono/meta-conversions-api-php-sdk": "^1.1", diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 1095a9f..0054ad3 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -61,7 +61,10 @@ public function getConfigTreeBuilder(): TreeBuilder ->arrayPrototype() ->children() ->scalarNode('id')->isRequired()->cannotBeEmpty()->end() - ->scalarNode('access_token')->isRequired()->cannotBeEmpty()->end() + ->scalarNode('access_token') + ->info('Only needed for server side tracking. Client side tracking renders fbq() calls, which only need the pixel id') + ->defaultNull() + ->end() ->end() ->end() ->end() diff --git a/src/Message/Handler/SendEventHandler.php b/src/Message/Handler/SendEventHandler.php index 2f74af5..00c21dd 100644 --- a/src/Message/Handler/SendEventHandler.php +++ b/src/Message/Handler/SendEventHandler.php @@ -4,17 +4,59 @@ namespace Setono\MetaConversionsApiBundle\Message\Handler; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; use Setono\MetaConversionsApi\Client\ClientInterface; +use Setono\MetaConversionsApi\Event\Event; +use Setono\MetaConversionsApi\Pixel\Pixel; use Setono\MetaConversionsApiBundle\Message\Command\SendEvent; final class SendEventHandler { - public function __construct(private readonly ClientInterface $client) - { + private readonly LoggerInterface $logger; + + public function __construct( + private readonly ClientInterface $client, + ?LoggerInterface $logger = null, + ) { + $this->logger = $logger ?? new NullLogger(); } public function __invoke(SendEvent $message): void { - $this->client->sendEvent($message->event); + $event = $message->event; + + // A pixel without an access token cannot be used server side: Meta answers 400, the SDK throws, and + // Messenger retries the message until it ends up in the failure transport. One warning is more useful. + // Client side tracking is unaffected, because rendering fbq() calls only needs the pixel id + $pixels = array_values(array_filter( + $event->pixels, + fn (Pixel $pixel): bool => $this->hasAccessToken($pixel, $event), + )); + + if ([] === $pixels) { + return; + } + + // Cloned so the event the application still holds is not mutated when the command is handled synchronously + $event = clone $event; + $event->pixels = $pixels; + + $this->client->sendEvent($event); + } + + private function hasAccessToken(Pixel $pixel, Event $event): bool + { + if (null !== $pixel->accessToken) { + return true; + } + + $this->logger->warning('The pixel {pixel} has no access token, so the event {event_name} ({event_id}) was not sent to it', [ + 'pixel' => $pixel->id, + 'event_name' => $event->eventName, + 'event_id' => $event->eventId, + ]); + + return false; } } diff --git a/src/Provider/ConfigurationBasedPixelProvider.php b/src/Provider/ConfigurationBasedPixelProvider.php index a59e4bc..0472ec8 100644 --- a/src/Provider/ConfigurationBasedPixelProvider.php +++ b/src/Provider/ConfigurationBasedPixelProvider.php @@ -8,15 +8,16 @@ final class ConfigurationBasedPixelProvider implements PixelProviderInterface { - /** @var list */ + /** @var list */ private readonly array $pixels; /** - * @param list $pixels + * @param list $pixels */ public function __construct(array $pixels) { - // this will filter all pixels where the id _or_ the access_token is empty + // A pixel without an id is useless, both client and server side. An empty access token is kept: + // client side tracking does not need one, and the send handler logs and skips such a pixel $this->pixels = array_values(array_filter($pixels, static fn (array $pixel): bool => '' !== $pixel['id'])); } diff --git a/src/Resources/config/services/conditional/server_side.xml b/src/Resources/config/services/conditional/server_side.xml index e2034eb..fec6183 100644 --- a/src/Resources/config/services/conditional/server_side.xml +++ b/src/Resources/config/services/conditional/server_side.xml @@ -12,8 +12,10 @@ + + diff --git a/tests/Integration/DependencyInjection/SetonoMetaConversionsApiExtensionTest.php b/tests/Integration/DependencyInjection/SetonoMetaConversionsApiExtensionTest.php index 068d9d6..6c1075e 100644 --- a/tests/Integration/DependencyInjection/SetonoMetaConversionsApiExtensionTest.php +++ b/tests/Integration/DependencyInjection/SetonoMetaConversionsApiExtensionTest.php @@ -69,6 +69,21 @@ public function it_does_not_load_client_side_event_subscribers_when_client_side_ $this->assertContainerBuilderNotHasService(AddLibraryToTagBagSubscriber::class); } + #[Test] + public function it_accepts_a_pixel_without_an_access_token(): void + { + // Client side tracking only renders fbq() calls, which need the pixel id and nothing else + $this->load([ + 'pixels' => [ + ['id' => '1234'], + ], + ]); + + $this->assertContainerBuilderHasParameter('setono_meta_conversions_api.pixels', [ + ['id' => '1234', 'access_token' => null], + ]); + } + #[Test] public function it_rejects_a_user_agent_filter_that_is_not_a_valid_regular_expression(): void { diff --git a/tests/Unit/Message/Handler/SendEventHandlerTest.php b/tests/Unit/Message/Handler/SendEventHandlerTest.php new file mode 100644 index 0000000..6e38834 --- /dev/null +++ b/tests/Unit/Message/Handler/SendEventHandlerTest.php @@ -0,0 +1,86 @@ +pixels = [new Pixel('1234', 's3cr3t')]; + + $client = $this->createMock(ClientInterface::class); + $client->expects(self::once())->method('sendEvent')->with($event); + + (new SendEventHandler($client))(new SendEvent($event)); + } + + #[Test] + public function it_skips_pixels_without_an_access_token(): void + { + $event = new Event(Event::EVENT_VIEW_CONTENT); + $event->pixels = [new Pixel('no-token'), new Pixel('1234', 's3cr3t')]; + + $sent = null; + $client = $this->createMock(ClientInterface::class); + $client->expects(self::once())->method('sendEvent')->willReturnCallback( + static function (Event $event) use (&$sent): void { + $sent = $event; + }, + ); + + (new SendEventHandler($client))(new SendEvent($event)); + + self::assertInstanceOf(Event::class, $sent); + self::assertEquals([new Pixel('1234', 's3cr3t')], $sent->pixels); + } + + #[Test] + public function it_does_not_send_when_no_pixel_has_an_access_token(): void + { + $event = new Event(Event::EVENT_VIEW_CONTENT); + $event->pixels = [new Pixel('no-token')]; + + $client = $this->createMock(ClientInterface::class); + $client->expects(self::never())->method('sendEvent'); + + (new SendEventHandler($client))(new SendEvent($event)); + } + + #[Test] + public function it_does_not_mutate_the_event_it_was_given(): void + { + $event = new Event(Event::EVENT_VIEW_CONTENT); + $event->pixels = [new Pixel('no-token'), new Pixel('1234', 's3cr3t')]; + + $sent = null; + $client = $this->createMock(ClientInterface::class); + $client->method('sendEvent')->willReturnCallback( + static function (Event $event) use (&$sent): void { + $sent = $event; + }, + ); + + (new SendEventHandler($client))(new SendEvent($event)); + + // The application may still hold the event when the command is handled synchronously + self::assertNotSame($event, $sent); + self::assertSame( + ['no-token', '1234'], + array_map(static fn (Pixel $pixel): string => $pixel->id, $event->pixels), + ); + } +}