Copy link Copy Markdown

Contributor

Copy link Copy Markdown

Contributor Author

This PR now has tests and the linked PRs work correctly, so I reckon it's ready for review.

Nothing new here in terms of functionality, it's just refactored stuff from the content and auth servers.

@mozilla/fxa-devs r?

Copy link Copy Markdown

Choose a reason for hiding this comment

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

r+ - looks great, works well for me when testing against the content server.

		* is an object containing `group` and `event` properties
		* but here `group` can be a string or a function. If it's
		* a function, it will be passed the matched `eventCategory`
		* as its argument and should return the group string.

Copy link Copy Markdown

Choose a reason for hiding this comment

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

Can you leave a note that this returns a function, which is the actual mapper? I was confused how this function was confused until I looked at how this function is invoked from the content server.

		if (! newsletter_state) {
		const { marketingOptIn } = data;

		if (marketingOptIn === true \|\| marketingOptIn === false) {

Copy link Copy Markdown

Choose a reason for hiding this comment

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

I have to admit I was confused by this, are you just trying to filter out non-boolean types?

Copy link Copy Markdown

Contributor Author

Choose a reason for hiding this comment

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

Yep, my intent is to make sure undefined does not produce the same effect as false.

This was referenced

Apr 9, 2018

Merged