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

comscore provider #76

Merged
merged 2 commits into from
May 2, 2019
Merged

comscore provider #76

merged 2 commits into from
May 2, 2019

Conversation

johndavidsimmons
Copy link
Collaborator

Documentation is sparse on Comscore web pixel, but I've gathered that "c" parameters are custom per client, thus I put them in a custom group. The only exceptions being the c1 parameter is the request type (seems to always be "2") and the c2 parameter is the client's ID. I called these out in a General group. Everything else falls into "Other"

Screen Shot 2019-04-30 at 10 10 48 AM

Screen Shot 2019-04-30 at 10 11 42 AM

@coveralls
Copy link

coveralls commented Apr 30, 2019

Coverage Status

Coverage increased (+0.2%) to 86.768% when pulling 30dd8ed on comscore into 74aac40 on development.

@MisterPhilip MisterPhilip changed the base branch from master to development April 30, 2019 16:36
Copy link
Owner

@MisterPhilip MisterPhilip left a comment

Choose a reason for hiding this comment

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

Here are my notes!

*
* @returns {Array}
*/
handleCustom(url, params) {
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think the handleCustom() is even necessary here. Both of the parameters are static and could be mapped in columnMapping() (account already is).

Copy link
Collaborator Author

@johndavidsimmons johndavidsimmons May 1, 2019

Choose a reason for hiding this comment

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

Can I put the value of the c1 & c2 parameters in a "General" group without using handleCustom? I would like to call them out in their own group as well as putting them in the "Custom" group since I believe they are the only constant, required parameters that do not change between clients, but are still technically "c##" parameters
Screen Shot 2019-04-30 at 8 24 44 PM

get columnMapping() {
return {
account: "c2",
requestType: "requestType"
Copy link
Owner

Choose a reason for hiding this comment

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

I would change this to c1 since it already exists in the keys() :)

Copy link
Collaborator Author

@johndavidsimmons johndavidsimmons May 1, 2019

Choose a reason for hiding this comment

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

I had the request type mapped in the column as c1, but seeing the "2" looks weird IMO, but I guess if thats what it is, it doesn't matter. Comscore's docs literally say that the c1 parameter is required and describes the event type, but is "prepopulated with a constant value of 2"
Screen Shot 2019-04-30 at 8 20 19 PM

src/providers/Comscore.js Outdated Show resolved Hide resolved
src/providers/Comscore.js Outdated Show resolved Hide resolved
@johndavidsimmons
Copy link
Collaborator Author

I just committed. Does it automatically become part of this PR? I simplified things a bit.

@MisterPhilip
Copy link
Owner

I just committed. Does it automatically become part of this PR? I simplified things a bit.

Yup! I'll review here in a bit. Thanks for the work :)

@MisterPhilip MisterPhilip self-requested a review May 1, 2019 18:30
@MisterPhilip MisterPhilip merged commit ed46d73 into development May 2, 2019
@MisterPhilip
Copy link
Owner

This is live!

@MisterPhilip MisterPhilip deleted the comscore branch September 10, 2020 21:39
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.

None yet

3 participants