-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Mailchimp unsubscribe webhook #5804
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
class IncomingWebhooks::MailchimpUnsubscribesController < ApplicationController | ||
class InvalidListID < StandardError; end | ||
|
||
LIST_MAPPINGS = { | ||
mailchimp_newsletter_id: :email_newsletter, | ||
mailchimp_sustaining_members_id: :email_membership_newsletter, | ||
mailchimp_tag_moderators_id: :email_tag_mod_newsletter, | ||
mailchimp_community_moderators_id: :email_community_mod_newsletter | ||
}.freeze | ||
|
||
def create | ||
not_authorized unless valid_secret? | ||
user = User.find_by!(email: params.dig(:data, :email)) | ||
user.update(email_type => false) | ||
vaidehijoshi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
end | ||
|
||
private | ||
|
||
def valid_secret? | ||
params[:secret] == SiteConfig.mailchimp_incoming_webhook_secret | ||
end | ||
|
||
def email_type | ||
list_id = params.dig(:data, :list_id) | ||
key = LIST_MAPPINGS.keys.detect { |k| SiteConfig.public_send(k) == list_id } | ||
raise InvalidListID unless key | ||
|
||
LIST_MAPPINGS[key] | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -154,6 +154,10 @@ | |
resources :reads, only: [:create] | ||
end | ||
|
||
namespace :incoming_webhooks do | ||
post "/mailchimp/:secret/unsubscribe", to: "mailchimp_unsubscribes#create", as: :mailchimp_unsubscribe | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can probably change this at one point and use routing DSL trickery, but for now this seems good enough. |
||
end | ||
|
||
resources :messages, only: [:create] | ||
resources :chat_channels, only: %i[index show create update] | ||
resources :chat_channel_memberships, only: %i[create update destroy] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
require "rails_helper" | ||
|
||
RSpec.describe "IncomingWebhooks::MailchimpUnsubscribesController", type: :request do | ||
let(:user) { create(:user, email_digest_periodic: true) } | ||
|
||
describe "POST /webhooks/mailchimp/:secret/unsubscribe" do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there's a typo here but I don't think it should hold the PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you mean that it should say |
||
let(:secret) { "secret" } | ||
let(:list_id) { "1234" } | ||
let(:params) { { data: { email: user.email, list_id: list_id } } } | ||
|
||
before do | ||
allow(SiteConfig).to receive(:mailchimp_incoming_webhook_secret).and_return(secret) | ||
end | ||
|
||
it "return not authorized if the secret is incorrect" do | ||
expect do | ||
post "/incoming_webhooks/mailchimp/wrong_secret/unsubscribe", params: params | ||
end.to raise_error(Pundit::NotAuthorizedError) | ||
end | ||
|
||
it "unsubscribes the user if the secret is correct" do | ||
SiteConfig.mailchimp_newsletter_id = list_id | ||
|
||
expect do | ||
post "/incoming_webhooks/mailchimp/#{secret}/unsubscribe", params: params | ||
end.to change { user.reload.email_newsletter }.from(true).to(false) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nice, i didn't know about the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's also expect do
User.create(...)
end.to change(User, :count).by(1) |
||
end | ||
end | ||
end |
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.
from what i can tell, we're not using most of these keys; what do we need them for? (just trying to get context on this)
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 the mapping of option names in the
SiteConfig
model to newsletter types. We're using it here:Once we have mapped from list id to attribute name, we use the latter in the update
user.update(email_type => false)
.Does that clear it up?