-
-
Notifications
You must be signed in to change notification settings - Fork 264
Fix Issue #112: Track api route was not unsubscribing contacts + documentation fixes #113
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor change requested.
Once that is done, you may bump the version in the main package.json
to 1.0.8
and I will merge it in!
contact = await prisma.contact.create({ | ||
data: { | ||
email, | ||
subscribed: subscribed ?? true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is breaking behaviour. If a contact is first created, it should be put on subscribed unless explicitly mentioned otherwise
if (subscribed !== undefined && contact.subscribed !== subscribed) { | ||
contact = await prisma.contact.update({ | ||
where: { id: contact.id }, | ||
data: { subscribed }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure that Prisma does not complain about subscribed possibly being null here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, made a fix now. If subscribed is null, then defaults to true.
Could you just double check, do you want nothing to happen when subscribed is undefined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just gave it a quick go myself.
This is what you are looking for, and will work just fine! I'm pretty sure it will only be undefined
if someone explicitly passes in that value. If no subscribed
is passed then we can just null-check.
if (subscribed !== null && contact.subscribed !== subscribed) {
contact = await prisma.contact.update({where: {id: contact.id}, data: {subscribed}});
redis.del(Keys.Contact.id(contact.id));
redis.del(Keys.Contact.email(project.id, contact.email));
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your patience. Only my 2nd contribution ever.
Was struggling and thought it was a bug in my code until I realised nothing I do makes a difference. Haha.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for contributing. Don't worry about making mistakes, I make them all the time :)
The https://api.useplunk.com/v1/track api route was not unsubscribing contacts if they are currently subscribed.
Fixed .env.example for database connection to work when contributing.
Added a line in contributing doc for yarn install.