feat[TIPPI-896]: conversion extension #325
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a new Kilkaya conversion tracking extension that sends conversion events when a checkout is successfully completed. The implementation uses the browser's sendBeacon API for reliable tracking during page transitions, with fallback mechanisms to the Kilkaya API when needed.
Key Changes:
- Implements conversion tracking that activates on checkout success events
- Uses
sendBeaconAPI with fallback to Kilkaya's native logging methods - Adds localStorage-based logging for debugging purposes
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| extensions/kilkaya/k5a_meta_send.js | New extension that sends conversion tracking data via sendBeacon when checkout success is detected |
| tests/kilkaya/k5a_meta_send.test.js | Comprehensive test suite covering event filtering, URL construction, tracking methods, error handling, and integration scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| setTimeout(function() { | ||
| try { | ||
| // Build the tracking URL manually based on Kilkaya's format | ||
| var installationId = '68ee5be64709bd7f4b3e3bf2'; |
There was a problem hiding this comment.
The installation ID is hardcoded. Consider making this configurable through window.k5aMeta or a configuration object to support different installations without code changes.
| var installationId = '68ee5be64709bd7f4b3e3bf2'; | |
| var installationId = (window.k5aMeta && window.k5aMeta.installationId) ? window.k5aMeta.installationId : '68ee5be64709bd7f4b3e3bf2'; |
| } catch (err) { | ||
| persistLog('✗ ERROR sending conversion', {error: err.message, stack: err.stack}); | ||
| } | ||
| }, 150); // Small delay to ensure k5aMeta.conversion is set |
There was a problem hiding this comment.
The 150ms delay is a magic number with unclear justification. Consider extracting this to a named constant (e.g., CONVERSION_DATA_WAIT_MS) to improve code clarity and maintainability.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
|
@LyubovSugak I've opened a new pull request, #326, to work on those changes. Once the pull request is ready, I'll request review from you. |
| expect(b.event_action).toBe('success'); | ||
| }); | ||
|
|
||
| it('should not run for other events', () => { |
There was a problem hiding this comment.
Hint: Checkout it.each https://jestjs.io/docs/api#each
Ticket
https://axelspringer.atlassian.net/browse/TIPPI-896