-
Notifications
You must be signed in to change notification settings - Fork 113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: update facebook destinations API version to v18.0 #2828
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2828 +/- ##
========================================
Coverage 87.17% 87.17%
========================================
Files 772 772
Lines 28792 28805 +13
Branches 6767 6767
========================================
+ Hits 25099 25112 +13
Misses 3350 3350
Partials 343 343 ☔ View full report in Codecov by Sentry. |
@sandeepdsvs Facebook conversion destination is missing. Have we already upgraded it to v18? |
Already updated as part of the last release |
Important Auto Review SkippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke the WalkthroughThe changes across the codebase primarily involve updating the Facebook Graph API version from v17.0 to v18.0 in various JavaScript files. This update affects endpoint URLs and is consistent across multiple integration tests and configuration files. Additionally, there are minor adjustments to payload sizes and token parameters to align with the new API version requirements. Changes
Assessment against linked issues
TipsChat with CodeRabbit Bot (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (14)
- src/v0/destinations/facebook_pixel/transform.js (1 hunks)
- src/v0/destinations/fb/transform.js (1 hunks)
- src/v0/destinations/fb_custom_audience/config.js (2 hunks)
- test/integrations/destinations/facebook_pixel/dataDelivery/data.ts (9 hunks)
- test/integrations/destinations/facebook_pixel/network.ts (9 hunks)
- test/integrations/destinations/facebook_pixel/processor/data.ts (38 hunks)
- test/integrations/destinations/facebook_pixel/router/data.ts (2 hunks)
- test/integrations/destinations/fb/dataDelivery/data.ts (1 hunks)
- test/integrations/destinations/fb/network.ts (2 hunks)
- test/integrations/destinations/fb/processor/data.ts (11 hunks)
- test/integrations/destinations/fb_custom_audience/dataDelivery/data.ts (1 hunks)
- test/integrations/destinations/fb_custom_audience/network.ts (1 hunks)
- test/integrations/destinations/fb_custom_audience/processor/data.ts (21 hunks)
- test/integrations/destinations/fb_custom_audience/router/data.ts (9 hunks)
Files skipped from review due to trivial changes (6)
- src/v0/destinations/facebook_pixel/transform.js
- src/v0/destinations/fb/transform.js
- test/integrations/destinations/facebook_pixel/processor/data.ts
- test/integrations/destinations/facebook_pixel/router/data.ts
- test/integrations/destinations/fb_custom_audience/processor/data.ts
- test/integrations/destinations/fb_custom_audience/router/data.ts
Additional comments: 33
src/v0/destinations/fb_custom_audience/config.js (2)
1-1: The update of
BASE_URL
to use the new Facebook Graph API version v18.0 is correct and aligns with the intended changes.96-97: The updated comment provides additional clarity on the rationale behind the maximum payload size, which is a good practice for maintainability.
test/integrations/destinations/facebook_pixel/dataDelivery/data.ts (9)
25-31: The update of the Facebook Graph API version from 'v17.0' to 'v18.0' in the endpoint URL is consistent with the summary provided. Ensure that the new version is supported and that all necessary changes have been made to accommodate any new API requirements or deprecations.
90-96: The access token in the endpoint URL has been updated. Verify that the new access token is valid and has the appropriate permissions for the API calls being made.
140-146: The endpoint URL contains an access token with 'invalid_timestamp' in its name. Ensure that this token is intended for use in tests that simulate an invalid timestamp scenario.
210-216: The endpoint URL contains an access token with 'throttled' in its name. Verify that this token is intended for use in tests that simulate API rate limiting or throttling scenarios.
275-281: The endpoint URL contains an access token with 'invalid_account_id' in its name. Ensure that this token is intended for use in tests that simulate scenarios where the account ID is invalid.
343-349: The endpoint URL contains an access token with 'not_found' in its name. Verify that this token is intended for use in tests that simulate scenarios where the access token is not found or invalid.
412-418: The endpoint URL has been updated to use the new API version 'v18.0'. Verify that the access token provided is correct and that the endpoint is functioning as expected with the new API version.
478-484: The endpoint URL has been updated to use the new API version 'v18.0'. Verify that the access token provided is correct and that the endpoint is functioning as expected with the new API version.
544-550: The endpoint URL has been updated to use the new API version 'v18.0'. Verify that the access token provided is correct and that the endpoint is functioning as expected with the new API version. Additionally, ensure that the 'unhandled_response' scenario is properly tested and handled in the code.
test/integrations/destinations/facebook_pixel/network.ts (1)
- 4-10: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [4-181]
The updates to the Facebook API version from v17.0 to v18.0 in the URLs, the modifications to the access tokens, and the expected responses in the
httpRes
objects are consistent with the new API version's requirements. The use ofgetFormData
for populating request data and the consistent use of headers and methods across requests are also noted. No issues are found in the hunks provided.test/integrations/destinations/fb/dataDelivery/data.ts (10)
33-33: The endpoint URL has been correctly updated to the new Facebook API version v18.0.
112-112: The access token used here appears to be for a valid scenario test case.
177-177: The access token 'invalid_timestamp_correct_access_token' suggests a specific test case for an invalid timestamp error, which is appropriate for testing error handling.
243-243: The access token 'throttled_valid_access_token' is indicative of a test case for API throttling, which is a good test to have for rate limiting scenarios.
314-314: The access token 'invalid_account_id_valid_access_token' is used to simulate a scenario where the account ID is invalid, which is a relevant test case for error handling.
52-62: The output object correctly reflects an error response with a status code of 400 and an appropriate error message for an invalid OAuth token.
131-139: The output object correctly reflects a successful response with a status code of 200 and a message indicating successful request processing.
196-211: The output object correctly simulates an error response for an event timestamp that is too old, with a detailed user message explaining the issue.
272-282: The output object correctly simulates an API throttling scenario with a status code of 429 and an error message indicating that the user request limit has been reached.
343-356: The output object correctly simulates an error response for an unsupported operation, with a detailed error message and a link to the Graph API documentation.
test/integrations/destinations/fb/network.ts (2)
19-25: The update to the API version in the URL is correct and aligns with the pull request's intent to upgrade to v18.0.
42-45: The addition of the 'x-forwarded-for' header in the second network call should be verified to ensure it's being used correctly and for the right purpose, as it can have implications for how the server interprets the origin of the request.
test/integrations/destinations/fb/processor/data.ts (1)
- 618-621: The update of the Facebook API endpoint from v17.0 to v18.0 is correctly reflected in the code.
test/integrations/destinations/fb_custom_audience/dataDelivery/data.ts (1)
- 1-504: The updates to the test data, including the
endpoint
URLs,session_id
property, and status codes, are correctly implemented to reflect the new API version and expected behavior.test/integrations/destinations/fb_custom_audience/network.ts (7)
7-7: The endpoint URLs have been correctly updated to use the new v18.0 version of the Facebook Graph API.
4-4: Verify that the
version
field within thehttpReq
object is intended to represent the request format version and not the Facebook API version, as the API version is specified in the endpoint URL.65-65: Ensure that the updated
status
codes anddata
objects in thehttpRes
responses are consistent with the expected behavior of the new Facebook API version.13-30: Confirm that the changes to the
schema
anddata
fields within thepayload
are aligned with the requirements of the new Facebook API version.9-9: The
headers
have been updated appropriately to include newtest-dest-response-key
values for different test scenarios.99-103: The
error
messages within thehttpRes
data objects have been updated to reflect new error conditions. Verify that these messages are accurate and correspond to the expected errors from the Facebook API.411-411: The
status
code 429 correctly indicates rate limiting, which is consistent with the error message provided. Ensure that the test case for rate limiting is handled as expected in the integration tests.
Kudos, SonarCloud Quality Gate passed! |
* feat: update facebook destinations API version to v18.0 * feat: updated fb_pixel tests to pick version dynamically from config.js * feat: updated fb tests to pick version dynamically from config.js * feat: updated fb_custom_audience tests to pick version dynamically from config.js
Description of the change
Resolves INT-975
Updated API version of Facebook Pixel, Facebook App Events, Facebook Custom Audience destinations to v18.0
Type of change
Related issues
Checklists
Development
Code review
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Chores