Skip to content
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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ amazon_authorize_key=

# Third party integration (required)
congress_forms_url=http://phantomdc.example.com
google_civic_api_url=https://www.googleapis.com/civicinfo/v2/representatives/
google_civic_api_key=
smarty_streets_id=
smarty_streets_token=

Expand Down
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ Follow these instructions to run the Action Center using Docker (recommended). T
* Allows users to submit e-messages to congress
* [Call Congress](https://github.com/EFForg/call-congress) url and API key
* Connects calls between citizens and their congress person using the Twilio API
* [Google Civic Information API](https://developers.google.com/civic-information) url and API key
* Representative information powered by the Civic Information API
* We use this when we need to give a user the ability to find their representatives to complete a state-level email action
Copy link
Member

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

* Some key limitations: https://developers.google.com/civic-information/docs/data_guidelines?hl=en e.g. "Developer’s using the API should make every effort to ensure all users are met with the same experience. We do not allow holdbacks, A/B testing, or similar experiments."


## Using the Action Center
Expand Down
1 change: 1 addition & 0 deletions app/assets/javascripts/admin.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
//= require admin/gallery
//= require admin/action_pages
//= require admin/action_pages/petition-targets
//= require admin/action_pages/email
//= require admin/analytics
//= require_tree ./admin/components

Expand Down
15 changes: 15 additions & 0 deletions app/assets/javascripts/admin/action_pages/email.js
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();
});
});

19 changes: 0 additions & 19 deletions app/assets/javascripts/application/tools/congress_message.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,23 +105,4 @@ $(document).on("ready", function() {
e.preventDefault();
$('#customize-message .notice').addClass('down');
});

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');
}
});
20 changes: 20 additions & 0 deletions app/assets/javascripts/application/tools/email.js.erb
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,24 @@ $(document).on('ready', function() {
$('.thank-you').show();
$('#email-tool').hide();
});

$(".state-rep-email").hide();

$("form.state-rep-lookup").on("ajax:complete", function(e, xhr, status) {
var $form = $(this);
var data = xhr.responseJSON;
$('.state-rep-lookup').hide();
$('.state-reps').replaceWith(data.content);
if (status == "success") {
$form.remove();
$(".state-reps").html(data);

if ($("#action-content").length) {
$(window).scrollTop( $("#action-content").offset().top ); // go to top of page if on action center site
}
update_tabs(1, 2);
} else {
show_error("Something went wrong. Please try again later.", $form);
}
});
});
18 changes: 18 additions & 0 deletions app/assets/javascripts/application/tools/shared.js
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');
}
9 changes: 5 additions & 4 deletions app/controllers/admin/action_pages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,11 @@ def action_page_params
tweet_targets_attributes: [:id, :_destroy, :twitter_id, :image]
],
email_campaign_attributes: [
:id, :message, :subject, :target_house, :target_senate, :target_email,
:email_addresses, :target_bioguide_id, :bioguide_id, :alt_text_email_your_rep,
:alt_text_look_up_your_rep, :alt_text_extra_fields_explain, :topic_category_id,
:alt_text_look_up_helper, :alt_text_customize_message_helper, :campaign_tag
:id, :message, :subject, :state, :target_state_lower_chamber, :target_state_upper_chamber,
:target_governor, :target_email, :email_addresses, :target_bioguide_id,
:bioguide_id, :alt_text_email_your_rep, :alt_text_look_up_your_rep,
:alt_text_extra_fields_explain, :topic_category_id, :alt_text_look_up_helper,
:alt_text_customize_message_helper, :campaign_tag
],
congress_message_campaign_attributes: [
:id, :message, :subject, :target_house, :target_senate, { target_bioguide_list: [] },
Expand Down
29 changes: 28 additions & 1 deletion app/controllers/tools_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Copy link
Member

Choose a reason for hiding this comment

The 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 #service_uri -- maybe something like

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
Copy link
Member

Choose a reason for hiding this comment

The 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

Copy link
Member

Choose a reason for hiding this comment

The 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:

  • StateLevelInfo - Service object type class responsible for turning info from the controller into an array of state representative objects
  • CivicApi - Module responsible for making the API calls themselves (pretty much what we have now)
  • StateRep - Tiny class that makes nice state rep objects from JSON
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 CivicApi module in the first class if we wanted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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?

Copy link
Member

Choose a reason for hiding this comment

The 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

Expand Down
15 changes: 15 additions & 0 deletions app/helpers/email_campaign_helper.rb
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
35 changes: 29 additions & 6 deletions app/models/email_campaign.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Copy link
Member

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 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

Expand Down
17 changes: 17 additions & 0 deletions app/views/action_page/_state_reps.html.erb
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>
36 changes: 32 additions & 4 deletions app/views/admin/action_pages/_email_fields.html.erb
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>
Copy link
Member

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 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 %>
Expand All @@ -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>
Copy link
Member

Choose a reason for hiding this comment

The 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

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 %>
Loading