Skip to content
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

Fix Ads Purchase Intent funnel keyword with higher weight is ignored #5180

Merged
merged 1 commit into from
Apr 8, 2020

Conversation

moritzhaller
Copy link
Contributor

@moritzhaller moritzhaller commented Apr 7, 2020

Resolves brave/brave-browser#8965

Submitter Checklist:

Test Plan:

  • Clean profile
  • Connect to US
  • Run Brave with command line: /usr/bin/brave-browser --enable-logging=stderr --vmodule=brave_ads=3 --brave-ads-staging --rewards=staging=true
  • Enable Rewards
  • Search for "audi a5 dealer opening times sell" in URL bar
  • Check weight for automotive purchase intent by make-audi in Default/ads_service/client.json

Expected result:

  • The highest weight should be used
  • audi a5 dealer opening times sell - weight 3

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@moritzhaller moritzhaller self-assigned this Apr 7, 2020
@moritzhaller moritzhaller changed the title Fix issues/8965 by traversing whole funnel weight list and return max weight Fix Ads Purchase Intent funnel keyword with higher weight is ignored Apr 7, 2020
Copy link
Collaborator

@tmancey tmancey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM++

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ads Purchase Intent funnel keyword with higher weight is ignored
2 participants