-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
🐛 Source Linkedin Ads: Fix issue where Accounts
stream did not correctly handle provided account IDs
#38013
🐛 Source Linkedin Ads: Fix issue where Accounts
stream did not correctly handle provided account IDs
#38013
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
@@ -11,7 +11,7 @@ data: | |||
connectorSubtype: api | |||
connectorType: source | |||
definitionId: 137ece28-5434-455c-8f34-69dc3782f451 | |||
dockerImageTag: 2.1.0 | |||
dockerImageTag: 2.1.1 |
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.
please remove dockerImageTag: 2.0.0
as issue is fixed now
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.
Removed
@@ -175,7 +175,7 @@ def test_next_page_token(self, requests_mock, response_json, expected): | |||
assert expected == result | |||
|
|||
def test_request_params(self): | |||
expected = "pageSize=500&q=search&search=(id:(values:List(1,2)))" | |||
expected = "pageSize=500&q=search&search=(id:(values:List(urn%3Ali%3AsponsoredAccount%3A1,urn%3Ali%3AsponsoredAccount%3A2)))" |
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.
should we add test config with account ids for check in cat to prevent similar issue in future?
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.
Added the new config
What
Fixed an issue where the Accounts stream did not correctly handle provided account IDs. Resolves: https://github.com/airbytehq/oncall/issues/5218
How
The format of IDs used for searches has changed from just an ID to a URN. Additionally, unlike all other search strings, URNs should be URL-encoded.
Added a new config for CAT to test multiple account IDs.
User Impact
Fixed an issue with the Accounts stream that occurred when account IDs were provided.
Can this PR be safely reverted and rolled back?