-
Notifications
You must be signed in to change notification settings - Fork 62
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
Enable State-level Emails #903
base: master
Are you sure you want to change the base?
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,15 @@ | ||
$(document).ready(function() { | ||
var stateLevelTarget = $("#action_page_email_campaign_attributes_state"); | ||
var stateLevelTargetSelection = $("#state-level-target-selection"); | ||
|
||
if (stateLevelTarget.val() === "") | ||
stateLevelTargetSelection.hide(); | ||
|
||
stateLevelTarget.on("change", function() { | ||
if (stateLevelTarget.val() !== "") | ||
stateLevelTargetSelection.show(); | ||
else | ||
stateLevelTargetSelection.hide(); | ||
}); | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
function show_progress_bars() { | ||
$(".progress-striped").show(); | ||
$("#tools :submit").hide(); | ||
$("#tools input,textarea,button,select", $(this)).attr("disabled", "disabled"); | ||
} | ||
|
||
function show_error(error, form) { | ||
$(".progress-striped").hide(); | ||
form.find(":submit").show(); | ||
form.find(".alert-danger").remove(); | ||
$("#errors").append($('<div class="small alert alert-danger help-block">').text(error)); | ||
$("#tools input,textarea,button,select", form).removeAttr("disabled"); | ||
} | ||
|
||
function update_tabs(from, to) { | ||
$(".page-indicator div.page" + from).removeClass('active'); | ||
$(".page-indicator div.page" + to).addClass('active'); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -123,7 +123,34 @@ def email | |
@actionPage = @action_page | ||
render "email_target" | ||
else | ||
redirect_to @action_page.email_campaign.service_uri(params[:service]) | ||
if params[:state_rep_email] | ||
redirect_to @action_page.email_campaign.service_uri(params[:service], { email: params[:state_rep_email] }) | ||
else | ||
redirect_to @action_page.email_campaign.service_uri(params[:service]) | ||
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. It'd be nice if we used strong parameters here to pass only one argument to def email
# beginning omitted
redirect_to @action_page.email_campaign.service_uri(email_uri_params.to_h)
end
def email_uri_params
params.permit(%i(service state_rep_email))
end and then for the method def service_uri(service:, state_rep_email: nil)
# ...
end |
||
end | ||
end | ||
end | ||
|
||
# GET /tools/state_reps | ||
# | ||
# This endpoint is hit by the js for state legislator lookup-by-address actions. | ||
# It renders json containing html markup for presentation on the view | ||
def state_reps | ||
@email_campaign = EmailCampaign.find(params[:email_campaign_id]) | ||
@actionPage = @email_campaign.action_page | ||
address = "#{params[:street_address]} #{params[:zipcode]}" | ||
civic_api_response = CivicApi.state_rep_search(address, @email_campaign.leg_level) | ||
@state_reps = JSON.parse(civic_api_response.body)["officials"] | ||
state_rep_emails = [] | ||
@state_reps.each do |sr| | ||
state_rep_emails << sr["emails"] if !sr["emails"].nil? | ||
end | ||
# single-rep lookup only | ||
@state_rep_email = state_rep_emails.flatten.first | ||
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. It'd be nice if parsing the API response + assembling the address happened in the CivicApi module (or a new service object type class), so we could have something like this in the controller def state_reps
# ...
address = address_params.to_h.values.join(" ")
@state_reps, @state_rep_email = CivicApi.state_rep_info_for(address, @email_campaign.leg_level)
# ...
end
def address_params
# strong params for address
end 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. this might be overkill but it might be nice to have a setup like this:
class StateLevelInfo
VALID_ROLES = %w(legislatorLowerBody legislatorUpperBody
headOfGovernment).freeze
def self.reps_for(**args)
new(**args).reps
end
def new(leg_level:, address:)
@role = leg_level
@address = address
end
def reps
return unless valid_role?
return unless address.present?
return unless api_response.present?
# might need another guard clause here to check that we have a real,
# non-error response?
StateRep.parse_from_json(api_response)
end
private
attr_reader :role, :address
def valid_role?
# could potentally do this in the CivicApi module instead,
# in which case this class only needs to check that the role is present
VALID_ROLES.include? role
end
def api_response
@response ||= CivicApi.state_rep_search(address, role).try(:body)
end
end
class StateRep
attr_reader :name, :emails
def self.parse_from_json(json_response)
JSON.parse(json_response)["officials"].map do |h|
rep = h.symbolize_keys.slice(%i(name emails))
rep[:emails] = rep.emails.flatten # might not be necessary
new(**rep)
end
end
def new(name:, emails:)
@name = name
@emails = emails
end
def info_text
str = "Your representative is #{name}.\n"
str << if emails.present?
"They can be reached at #{emails.join(", ")}"
else
"We could not find their email address."
end
str
end
end
# In controller
def state_reps
@email_campaign = EmailCampaign.find(params[:email_campaign_id])
@actionPage = @email_campaign.action_page
address = address_params.to_h.values.join(" ")
@state_reps = StateLevelInfo.reps_for(role: @email_campaign.leg_level,
address: address)
@state_rep_email = @state_reps.first.try(:emails)
if @state_reps.present?
render json: { content: render_to_string(partial: "action_page/state_reps") }, status: 200
else
render json: { error: "No representatives found" }, status: 200
end
end <div class="state-rep-names-and-emails">
This action is for the <%= legislative_level_from_state_representative_info(@email_campaign.leg_level)} %>.
<% @state_reps.each do |sr| -%>
<div>
<%= sr.info_text %>
</div>
<% end %>
</div> We could even include 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. I like this idea a lot! Thank you for demonstrating how it would work so clearly! Because it's a larger design change, I might want to implement it out/test it when we do the call tool refactor, if that works? 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. Yes! Feel free to turn this into its own issue |
||
if @state_reps.present? | ||
render json: { content: render_to_string(partial: "action_page/state_reps") }, status: 200 | ||
else | ||
render json: { error: "No representatives found" }, status: 200 | ||
end | ||
end | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
module EmailCampaignHelper | ||
def legislative_level_from_state_representative_info(legislator_info) | ||
role = case legislator_info | ||
when "legislatorLowerBody" | ||
"state representative of the lower chamber" | ||
when "legislatorUpperBody" | ||
"state representative of the upper chamber" | ||
when "headOfGovernment" | ||
"Governor of the State" | ||
else | ||
"Invalid info" | ||
end | ||
role | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,9 @@ class EmailCampaign < ActiveRecord::Base | |
belongs_to :topic_category | ||
has_one :action_page | ||
|
||
# No DC | ||
STATES = %w(AK AL AR AZ CA CO CT DE FL GA HI IA ID IL IN KS KY LA MA MD ME MI MN MO MS MT NC ND NE NH NJ NM NV NY OH OK OR PA RI SC SD TN TX UT VA VT WA WI WV WY).freeze | ||
|
||
def email_your_rep_text(default) | ||
target_bioguide_text_or_default alt_text_email_your_rep, default | ||
end | ||
|
@@ -22,19 +25,39 @@ def extra_fields_explain_text(default) | |
target_bioguide_text_or_default alt_text_extra_fields_explain, default | ||
end | ||
|
||
def leg_level | ||
return "legislatorLowerBody" if self.target_state_lower_chamber | ||
return "legislatorUpperBody" if self.target_state_upper_chamber | ||
return "headOfGovernment" if self.target_governor | ||
"" | ||
end | ||
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. It'd be nice to use guard clauses here + have an explicit default value: def leg_level(email_campaign)
return "headOfGovernment" if email_campaign.target_governor
return "legislatorUpperBody" if email_campaign.target_state_upper_chamber
return "legislatorLowerBody" if email_campaign.target_state_lower_chamber
""
end |
||
|
||
include ERB::Util | ||
|
||
def service_uri(service) | ||
mailto_addresses = email_addresses.split(/\s*,\s*/).map do |email| | ||
u(email.gsub(" ", "")).gsub("%40", "@") | ||
def service_uri(service, opts = {}) | ||
mailto_addresses = opts[:email] | ||
mailto_addresses ||= email_addresses | ||
# look for custom email addresses set on the back end if there is no email param from the front-end, | ||
# as is the case when we send state-level emails -- we cannot store these email address in our db, | ||
# reason below: | ||
|
||
# https://developers.google.com/terms#e_prohibitions_on_content | ||
# Section 5.e.1., as of December 2022 | ||
# e. Prohibitions on Content | ||
# Unless expressly permitted by the content owner or by applicable law, you will not, and will not permit your end users or others acting on your behalf to, do the following with content returned from the APIs: | ||
# Scrape, build databases, or otherwise create permanent copies of such content, or keep cached copies longer than permitted by the cache header; | ||
|
||
# results in comma-separated string of email addresses | ||
default_mailto_addresses ||= mailto_addresses.split(/\s*,\s*/).map do |email| | ||
u(email.gsub(" ", "")).gsub("%40", "@").gsub("%2B", "+") | ||
end.join(",") | ||
|
||
{ | ||
default: "mailto:#{mailto_addresses}?#{query(body: message, subject: subject)}", | ||
default: "mailto:#{default_mailto_addresses}?#{query(body: message, subject: subject)}", | ||
|
||
gmail: "https://mail.google.com/mail/?view=cm&fs=1&#{{ to: email_addresses, body: message, su: subject }.to_query}", | ||
gmail: "https://mail.google.com/mail/?view=cm&fs=1&#{{ to: mailto_addresses, body: message, su: subject }.to_query}", | ||
|
||
hotmail: "https://outlook.live.com/default.aspx?rru=compose&#{{ to: email_addresses, body: message, subject: subject }.to_query}#page=Compose" | ||
hotmail: "https://outlook.live.com/default.aspx?rru=compose&#{{ to: mailto_addresses, body: message, subject: subject }.to_query}#page=Compose" | ||
}.fetch(service.to_sym) | ||
end | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
<div class="state-rep-names-and-emails"> | ||
<%= "This action is for the #{legislative_level_from_state_representative_info(@email_campaign.leg_level)}." %> | ||
<% @state_reps.each do |sr| %> | ||
<div> | ||
<%= "Your representative is #{sr['name']}" %>. | ||
<% if sr["emails"].present? %> | ||
<%= "They can be reached at: #{sr['emails'].join(', ')}" %> | ||
<% else %> | ||
<%= "We could not find their email address." %> | ||
<% end %> | ||
</div> | ||
<% end %> | ||
</div> | ||
|
||
<div class="state-rep-email"> | ||
<%= render 'tools/send_email' %> | ||
</div> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,4 @@ | ||
<%= f.fields_for(:email_campaign) do |sf| %> | ||
<div class="form-item"> | ||
<%= sf.label :email_addresses, "To" %> | ||
<%= sf.text_field :email_addresses %> | ||
</div> | ||
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. It'd be nice to leave this more prominent for regular email actions I think. Maybe it'd be nice to have a way to select action targets where the options are "Specific email addresses" or "State Legislatures", and then using JS to toggle visibility of relevant fields? We do something similar in the congress messages admin form. I haven't actually tested this locally yet so disregard if this is a thing that happens already! |
||
|
||
<div class="form-item"> | ||
<%= sf.label :subject %> | ||
|
@@ -13,4 +9,36 @@ | |
<%= sf.label :message %> | ||
<%= sf.text_area :message %> | ||
</div> | ||
|
||
<fieldset class="form-item"> | ||
<legend>Select State-Level Legislators</legend> | ||
|
||
<%= sf.label :state, class: "fancy" do %> | ||
<%= sf.select :state, options_for_select(EmailCampaign::STATES, @actionPage.email_campaign.state), include_blank: "- none -" %><span class="ui"></span> | ||
<% end %> | ||
|
||
<div id="state-level-target-selection"> | ||
<p>For now, please choose only one.</p> | ||
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. Is it possible that we'll want to be able to select more than one of these in the future? If not, this would be a great attribute set to use an enum for 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. Indeed! That is the future vision, to select more than one. |
||
<%= sf.label :target_state_lower_chamber do %> | ||
<%= sf.check_box :target_state_lower_chamber, class: "fancy" %><span class="ui"></span> | ||
Lower Chamber | ||
<% end %> | ||
|
||
<%= sf.label :target_state_upper_chamber do %> | ||
<%= sf.check_box :target_state_upper_chamber, class: "fancy" %><span class="ui"></span> | ||
Upper Chamber | ||
<% end %> | ||
|
||
<%= sf.label :target_governor do %> | ||
<%= sf.check_box :target_governor, class: "fancy" %><span class="ui"></span> | ||
Governor | ||
<% end %> | ||
</div> | ||
|
||
<br></br> | ||
|
||
<%= sf.label :email_addresses, "Or enter custom email addresses below:" %> | ||
<%= sf.text_field :email_addresses %> | ||
</fieldset> | ||
|
||
<% 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.
It'd be nice to add the API restrictions into the readme as well as the code comment you have in the
CivicApi
module