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.

Sign up for GitHub

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

Jump to bottom

Finteza Analytics Adapter: fix cookies #4292

Merged
merged 7 commits into from
Oct 15, 2019
Merged

Finteza Analytics Adapter: fix cookies #4292

merged 7 commits into from
Oct 15, 2019

Conversation

finteza
Copy link
Contributor

@finteza finteza commented Oct 14, 2019

Fix reading and sending cookies in analytics requests

Type of change

  • Bugfix
  • Refactoring (no functional changes, no api changes)

bretg
bretg requested changes Oct 14, 2019
View reviewed changes
Copy link
Collaborator

@bretg bretg left a comment

Choose a reason for hiding this comment

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

Unit tests please. Also, does your Prebid.org doc entry need to mention this cookie, or do you have that covered in your product documentation?

@bretg bretg self-assigned this Oct 14, 2019
@finteza finteza requested a review from bretg October 15, 2019 07:23
@finteza
Copy link
Contributor Author

finteza commented Oct 15, 2019

@bretg

Also, does your Prebid.org doc entry need to mention this cookie, or do you have that covered in your product documentation?

Yes, of course. We have covered it on official website: https://www.finteza.com/en/cookies

@finteza
Copy link
Contributor Author

finteza commented Oct 15, 2019 •
edited
Loading

@bretg

Unit tests please.

Fixed.
But ci shows different errors inside tests for another modules, which we didn't touch.
Please check it.
It looks like somebody breaks setCookie method from src/utils

@bretg bretg merged commit ae63dde into prebid:master Oct 15, 2019
@pm-manasi-moghe pm-manasi-moghe mentioned this pull request Jan 13, 2021
Merged
10 tasks
@pm-manasi-moghe pm-manasi-moghe mentioned this pull request Jan 25, 2021
Closed
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bretg bretg bretg approved these changes

Assignees

@bretg bretg

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@finteza @bretg