State level action fixes & improvements (#941)

esoterik · web-flow · commit 555094e62705 · 2025-04-22T17:14:40.000-07:00
* Refactor state level email actions
* Clean up state level refactor + fix its styles
* Some small cleanup around state-level actions
* Update activism director -&gt; Jason
diff --git a/app/assets/javascripts/application/tools/email.js b/app/assets/javascripts/application/tools/email.js
@@ -14,21 +14,21 @@ $(document).on('ready', function() {
     $('#email-tool').hide();
   });
 
-  $(".state-rep-email").hide();
+  $("#state-email-tool").on("ajax:beforeSend", function() {
+    show_progress_bars();
+  });
 
-  $("form.state-rep-lookup").on("ajax:complete", function(e, xhr, status) {
+  $(".state-rep-lookup").on("ajax:complete", function(xhr, data, 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);
+      $(".progress-striped").hide();
+      $(".address-lookup").remove();
+    } else if (data.responseText) {
+      show_error(data.responseText, $form);
     } else {
       show_error("Something went wrong. Please try again later.", $form);
     }
diff --git a/app/assets/javascripts/application/tools/shared.js b/app/assets/javascripts/application/tools/shared.js
@@ -15,4 +15,4 @@ function show_error(error, form) {
 function update_tabs(from, to) {
   $(".page-indicator div.page" + from).removeClass('active');
   $(".page-indicator div.page" + to).addClass('active');
-}
+}
diff --git a/app/assets/stylesheets/application/action_page.scss b/app/assets/stylesheets/application/action_page.scss
@@ -1316,3 +1316,9 @@ html.js #affiliations {
   }
 }
 
+.state-email-body {
+  padding: 0.5rem 1.5rem;
+  form {
+    padding: 0;
+  }
+}
diff --git a/app/controllers/tools_controller.rb b/app/controllers/tools_controller.rb
@@ -131,18 +131,13 @@ def state_reps
     @actionPage = @email_campaign.action_page
     # TODO: strong params this
     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"] unless sr["emails"].nil?
-    end
-    # single-rep lookup only
-    @state_rep_email = state_rep_emails.flatten.first
-    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
+    @state_reps = CivicApi.state_rep_search(address, @email_campaign.leg_level)
+
+    # Get first non-null email for a state rep
+    @state_rep_email = @state_reps.map { |sr| sr["emails"] }.flatten.compact.first
+
+    unless @state_reps.present?
+      render plain: "No representatives found", status: 200
     end
   end
 
diff --git a/app/lib/civic_api.rb b/app/lib/civic_api.rb
@@ -6,27 +6,52 @@
 # do not allow holdbacks, A/B testing, or similar experiments."
 
 module CivicApi
-  VALID_ROLES = %w[legislatorLowerBody legislatorUpperBody headOfGovernment].freeze
+  VALID_ROLES = %w[legislatorLowerBody legislatorUpperBody
+                   headOfGovernment].freeze
 
+  #  4/22 note - address and roles are both strings for now, as we
+  #  don't yet support multiple targets for state campaigns
   def self.state_rep_search(address, roles)
-    raise ArgumentError, "required argument is nil" unless [address, roles].all?
-
-    raise ArgumentError, "Invalid role for Civic API #{roles}" unless VALID_ROLES.include?(roles)
+    unless [address, roles].all?(&:present?)
+      missing_fields = []
+      missing_fields << :address unless address.present?
+      missing_fields << :roles unless roles.present?
+      raise ArgumentError, "required argument(s) `#{missing_fields.join(", ")}` is nil"
+    end
+    unless VALID_ROLES.include?(roles)
+      raise ArgumentError, "Invalid role for Civic API `#{roles}`"
+    end
 
     # `includeOffices` param is needed in order to get officials list
-    # `administrativeArea1` param restricts the search to state-level legislators (and governors)
-    params = { address: address, includeOffices: true, levels: "administrativeArea1", roles: roles, key: civic_api_key }
+    # `administrativeArea1` param restricts the search to state-level
+    #   legislators (and governors)
+    params = {
+      address: address,
+      includeOffices: true,
+      levels: "administrativeArea1",
+      roles: roles,
+      key: civic_api_key
+    }
 
-    get params
+    response = get params
+    JSON.parse(response.body)["officials"]
   end
 
   def self.all_state_reps_for_role(state, roles)
     raise ArgumentError, "required argument is nil" unless [state, roles].all?
 
     # need to append division information to API route
-    path_params = { ocdId: "ocd-division%2Fcountry%3Aus%2Fstate%3A#{state.downcase}" }
-    # `administrativeArea1` param restricts the search to state-level legislators (and governors)
-    query_params = { levels: "administrativeArea1", recursive: true, roles: roles, key: civic_api_key }
+    path_params = {
+      ocdId: "ocd-division%2Fcountry%3Aus%2Fstate%3A#{state.downcase}"
+    }
+    # `administrativeArea1` param restricts the search to state-level
+    #   legislators (and governors)
+    query_params = {
+      levels: "administrativeArea1",
+      recursive: true,
+      roles: roles,
+      key: civic_api_key
+    }
 
     params = { path_params: path_params, query_params: query_params }
 
diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb
@@ -14,6 +14,8 @@
 
   <title><%= page_title -%></title>
   <%= stylesheet_link_tag "application", media: "all" %>
+  <%= javascript_include_tag "application" %>
+
   <%= yield(:head) if content_for?(:head) -%>
   <%= csrf_meta_tags %>
 
@@ -97,9 +99,6 @@
       </div>
     </div>
 
-
-  <%= javascript_include_tag "application" %>
-
   <%= matomo_tracking_embed %>
 
   <%= yield(:body) if content_for?(:body) -%>
diff --git a/app/views/tools/_container.html.erb b/app/views/tools/_container.html.erb
@@ -6,7 +6,13 @@
   <% else -%>
     <%= render "tools/petition" if @actionPage.enable_petition? && @actionPage.petition %>
     <%= render "tools/call" if @actionPage.enable_call? && @actionPage.call_campaign %>
-    <%= render "tools/email" if @actionPage.enable_email? && @actionPage.email_campaign %>
+    <% if @actionPage.enable_email? && @actionPage.email_campaign %>
+      <% if @actionPage.email_campaign.state? %>
+        <%= render "tools/state_leg_email" %>
+      <% else %>
+        <%= render "tools/email" %>
+      <% end %>
+    <% end %>
     <%= render "tools/congress_message" if @actionPage.enable_congress_message? && @actionPage.congress_message_campaign %>
     <%= render "tools/tweet" if @actionPage.enable_tweet? && @actionPage.tweet %>
     <%= render "tools/mailing_list" if @no_tools == true %>
diff --git a/app/views/tools/_email.html.erb b/app/views/tools/_email.html.erb
@@ -1,10 +1,3 @@
-<% if @email_campaign.state? %>
-  <div class="page-indicator">
-    <div class="page1 active">1</div>
-    <div class="page2">2</div>
-    <div class="page3">3</div>
-  </div>
-<% end %>
 <div id='email-tool' class="tool" data-action-id="<%= @actionPage.id %>">
   <div class="tool-container">
     <div class="tool-heading">
@@ -15,11 +8,7 @@
     </div>
 
     <div class="email-body">
-      <% if @email_campaign.state? %>
-        <%= render 'tools/lookup_state_rep' %>
-      <% else %>
-        <%= render 'tools/send_email' %>
-      <% end %>
+      <%= render 'tools/send_email' %>
     </div>
   </div>
 </div>
diff --git a/app/views/tools/_lookup_state_rep.html.erb b/app/views/tools/_lookup_state_rep.html.erb
@@ -1,43 +1,40 @@
-<div class="state-rep-lookup">
+<div class="address-lookup">
   <p>Enter your address below and we'll help you email your representatives (US addresses only).</p>
-  <div class="address-lookup">
-    <%= form_tag(tools_state_reps_path, method: :get, remote: true, class: "form state-rep-lookup") do %>
-      <%= hidden_field_tag :email_campaign_id, @actionPage.email_campaign.id %>
+  <%= form_tag(tools_state_reps_path, method: :post, remote: true, class: "form state-rep-lookup") do %>
+    <%= hidden_field_tag :email_campaign_id, @actionPage.email_campaign.id %>
 
-      <fieldset id="lookup-address">
-        <h3>
-          <span class="customize-message-popover" data-toggle="tooltip" data-trigger="hover">?
-            <span class="tooltiptext">We'll use your address to find your state representatives.</span>
-          </span>
-          Look up your state representatives
-        </h3>
-        <div id="errors"></div>
+    <fieldset id="lookup-address">
+      <h3>
+        <span class="customize-message-popover" data-toggle="tooltip" data-trigger="hover">?
+          <span class="tooltiptext">We'll use your address to find your state representatives.</span>
+        </span>
+        Look up your state representatives
+      </h3>
+      <div id="errors"></div>
 
-        <div class="form-group">
-          <%= text_field_tag :street_address, (current_user && current_user.street_address),
-            placeholder: "Street Address", "aria-label": "Street Address",
-            required: true, class: "form-control" %>
-        </div>
+      <div class="form-group">
+        <%= text_field_tag :street_address, (current_user && current_user.street_address),
+          placeholder: "Street Address", "aria-label": "Street Address",
+          required: true, class: "form-control" %>
+      </div>
 
-        <div class="form-group">
-          <%= text_field_tag :zipcode, (current_user && current_user.zipcode),
-            placeholder: "Zip Code", "aria-label": "Zip Code", required: true,
-            class: "form-control", title: "Must be 5 numeric numbers",
-            pattern: "\\d{5}", maxlength: 5 %>
-        </div>
+      <div class="form-group">
+        <%= text_field_tag :zipcode, (current_user && current_user.zipcode),
+          placeholder: "Zip Code", "aria-label": "Zip Code", required: true,
+          class: "form-control", title: "Must be 5 numeric numbers",
+          pattern: "\\d{5}", maxlength: 5 %>
+      </div>
 
-        <div class="form-errors info-circle error hidden"></div>
+      <div class="form-errors info-circle error hidden"></div>
 
-        <p class="privacy-notice">Uses <a href="https://developers.google.com/terms/api-services-user-data-policy" target="_blank">Google</a>'s APIs (
+      <p class="privacy-notice">Uses <a href="https://developers.google.com/terms/api-services-user-data-policy" target="_blank">Google</a>'s APIs (
         <span class="privacy-notice-popover" data-toggle="tooltip" data-trigger="hover">why?
           <!-- Attribution Guidelines: https://developers.google.com/civic-information/docs/data_guidelines?hl=en#data-availability -->
           <span class="tooltiptext">Representative information powered by the Civic Information API.</span>
         </span>).
-        </p>
-      </fieldset>
-      <button type="submit" class="eff-button" id="button-state-rep-lookup"role="button" aria-pressed="false">See Your Representatives</button>
-      <%= render "tools/loading" -%>
-  </div>
-<% end %>
+      </p>
+    </fieldset>
+    <input type="submit" class="eff-button" value="Find your reps">
+    <%= render "tools/loading" -%>
+  <% end %>
 </div>
-<div class="state-reps"></div>
diff --git a/app/views/tools/_state_leg_email.html.erb b/app/views/tools/_state_leg_email.html.erb
@@ -0,0 +1,25 @@
+<div class="page-indicator">
+  <div class="page1 active">1</div>
+  <div class="page2">2</div>
+  <div class="page3">3</div>
+</div>
+<div id='state-email-tool' class="tool" data-action-id="<%= @actionPage.id %>">
+  <div class="tool-container">
+    <div class="tool-heading">
+      <h2 class="tool-title">Take action</h2>
+    </div>
+
+    <div class="tool-body state-email-body">
+      <%= render 'tools/lookup_state_rep' %>
+      <div class="state-reps"></div>
+    </div>
+  </div>
+</div>
+
+  <div class="update-user-data-container" style="display: none;">
+    <% if current_user %>
+      <input checked="checked" id="update_user_data" name="update_user_data" type="checkbox" value="yes">
+      <label for="update_user_data" value="Save my name and address for next time."></label>
+    <% end %>
+  </div>
+</div>
diff --git a/app/views/tools/_state_reps.html.erb b/app/views/tools/_state_reps.html.erb
@@ -1,13 +1,15 @@
 <div class="state-rep-names-and-emails">
-  <%= "This action is for the #{legislative_level_from_state_representative_info(@email_campaign.leg_level)}." %>
+  <p><%= "This action is for the #{legislative_level_from_state_representative_info(@email_campaign.leg_level)}." %></p>
   <% @state_reps.each do |sr| %>
     <div>
+      <p>
       <%= "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 %>
+      </p>
     </div>
   <% end %>
 </div>
diff --git a/app/views/tools/state_reps.js.erb b/app/views/tools/state_reps.js.erb
@@ -0,0 +1 @@
+$(".state-reps").html("<%= escape_javascript render(partial: "state_reps") %>");
diff --git a/config/action_email.md b/config/action_email.md
@@ -16,7 +16,7 @@ $SHAREBUTTONS
 
 Thank you for helping us defend privacy and free speech in the digital age,
 
-Gennie Gebhart
+Jason Kelley
 Activism Director
 Electronic Frontier Foundation
 
diff --git a/config/environments/development.rb b/config/environments/development.rb
@@ -67,7 +67,7 @@
   # Debug mode disables concatenation and preprocessing of assets.
   # This option may cause significant delays in view rendering with a large
   # number of complex assets.
-  config.assets.debug = false
+  config.assets.debug = true
 
   # Suppress logger output for asset requests.
   config.assets.quiet = true
diff --git a/config/routes.rb b/config/routes.rb
@@ -15,7 +15,7 @@
   post "tools/message-congress"
   get "tools/reps"
   get "tools/reps_raw"
-  get "tools/state_reps"
+  post "tools/state_reps"
   get "tools/social_buttons_count"
 
   get "smarty_streets/street_address", controller: :smarty_streets
diff --git a/config/secrets.yml b/config/secrets.yml
@@ -72,6 +72,8 @@ test:
     civi_key: xyz
   smarty_streets_id: abc
   mailings_from: "EFF Action <actioncenter@eff.org>"
+  google_civic_api_url: "https://civic.example.com"
+  google_civic_api_key: "test-key-for-civic-api"
 
 production: *default
 staging: *default
diff --git a/spec/controllers/tools_controller_spec.rb b/spec/controllers/tools_controller_spec.rb
@@ -80,26 +80,6 @@
       expect(response).to redirect_to(uri)
     end
   end
-
-  describe "#state_reps" do
-    let(:email_campaign) { FactoryBot.create(:email_campaign, :state_leg) }
-    let(:address) { "815 Eddy St 94109" }
-    let(:json_parseable_state_officials) { '{"officials": [{"name": "Sponge Bob", "party": "Sandy Party", "emails": ["spongebob@clarinetfans.annoying"]}]}' }
-
-    before do
-      Rails.application.config.google_civic_api_url = "http://civic.example.com"
-      Rails.application.secrets.google_civic_api_key = "test-key-for-civic-api"
-
-      stub_request(:get, "http://civic.example.com/?address=%20&includeOffices=true&key=test-key-for-civic-api&levels=administrativeArea1&roles=legislatorUpperBody")
-        .to_return(status: 200, body: json_parseable_state_officials, headers: {})
-    end
-
-    it "should render JSON with the state officials array" do
-      get :state_reps, params: { email_campaign_id: email_campaign.action_page.email_campaign_id }
-
-      expect(response).to have_http_status(200)
-    end
-  end
 end
 
 def create_signature_and_have_user_sign
diff --git a/spec/lib/civic_api_spec.rb b/spec/lib/civic_api_spec.rb
diff --git a/spec/requests/tools_spec.rb b/spec/requests/tools_spec.rb
diff --git a/spec/system/action_pages/state_leg_email_action_spec.rb b/spec/system/action_pages/state_leg_email_action_spec.rb

Original file line number	Diff line number	Diff line change
`@@ -1316,3 +1316,9 @@ html.js #affiliations {`
`1316`	`1316`	`}`
`1317`	`1317`	`}`
`1318`	`1318`
	`1319`	`+.state-email-body {`
	`1320`	`+ padding: 0.5rem 1.5rem;`
	`1321`	`+ form {`
	`1322`	`+ padding: 0;`
	`1323`	`+ }`
	`1324`	`+}`