Skip to content

optable-targeting: Fix query string construction when IDs are missing. #4138

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

justadreamer
Copy link
Contributor

🔧 Type of changes

  • bugfix

✨ What's the context?

When there was no input IDs extracted from the incoming request to be sent to Optable Targeting Endpoint - the query string appended to the main endpoint (already containing &t=&o= parameters) would be formed without a leading & essentially leading to query strings like &t=tenant&origin=origingdpr=0 instead of &t=tenant=tenant&origin=origin&gdpr=0 - that lead to Targeting API requests failing with a 404. The test was there, but the test itself was incorrect, thus the bug was not caught :( The code and test are fixed now.

🧠 Rationale behind the change

To eliminate the majority of Targeting API calls resulting in 404s in production

🧪 Test plan

How do you know the changes are safe to ship to production?

  • Unit Tests
  • Manual Integration Tests

🏎 Quality check

  • Are your changes following our code style guidelines? Should be
  • Are there any breaking changes in your code? No breaking changes
  • Does your test coverage exceed 90%? Should be
  • Are there any erroneous console logs, debuggers or leftover code in your changes? None

@Net-burst Net-burst requested a review from CTMBNara August 15, 2025 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants