-
Notifications
You must be signed in to change notification settings - Fork 13
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
Add before_action in IncomingEmail webhook controller #4443
Add before_action in IncomingEmail webhook controller #4443
Conversation
Co-authored-by: Ricardo Kreyhsig <rkreyhsig@codeforamerica.org>
Heroku app: https://gyr-review-app-4443-be682f5bc777.herokuapp.com/ |
Co-authored-by: Ricardo Kreyhsig <rkreyhsig@codeforamerica.org>
…_spec since I modified a constant in that class Co-authored-by: Ricardo Kreyhsig <rkreyhsig@codeforamerica.org>
@@ -1,6 +1,7 @@ | |||
class ApplicationRecord < ActiveRecord::Base | |||
self.abstract_class = true | |||
|
|||
STATE_INTAKE_CLASS_NAMES = %w[StateFileAzIntake StateFileNyIntake].map(&:to_s).freeze |
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.
Moving this here to make it available to all our models instead of just EfileSubmission
Co-authored-by: Ricardo Kreyhsig <rkreyhsig@codeforamerica.org>
@@ -706,4 +706,17 @@ | |||
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.
Adding this spec since I moved STATE_INTAKE_CLASS_NAMES
form the efile_submission
model, and I wanted to make sure I didn't break anything.
sender_email_address = parse_valid_email_address(from: params["from"], sender: params["sender"]) | ||
opted_out_state_intakes = StateFileBaseIntake.opted_out_state_file_intakes(sender_email_address) | ||
unless opted_out_state_intakes.empty? | ||
opted_out_state_intakes.each do |intake| |
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.
Not sure there's better logic here for updating only one intake, but since we don't have a unique validation constraint on the intake, there could be multiple intakes with the same email. I kept the pattern of creating multiple incoming_emails established in the MailgunWebhooksController
. Grabbing the last intake is also not a guarantee to grabbing the right one, so I am open to feedback 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.
I think updating all of them rather than just the last one is absolutely the right call
Co-authored-by: Ricardo Kreyhsig <rkreyhsig@codeforamerica.org>
https://www.pivotaltracker.com/story/show/187187633