From 641105fa963f44478ed183759478b57916456d24 Mon Sep 17 00:00:00 2001 From: sylph Date: Wed, 9 Apr 2025 21:47:20 -0700 Subject: [PATCH 1/7] Refactor state level email actions --- .../tools/{email.js.erb => email.js} | 16 +++---- .../javascripts/application/tools/shared.js | 2 +- app/controllers/tools_controller.rb | 13 +++--- app/lib/civic_api.rb | 41 +++++++++++++----- app/views/layouts/application.html.erb | 5 +-- app/views/tools/_container.html.erb | 8 +++- app/views/tools/_email.html.erb | 13 +----- app/views/tools/_lookup_state_rep.html.erb | 4 +- app/views/tools/_state_leg_email.html.erb | 24 +++++++++++ .../_state_reps.html.erb | 0 app/views/tools/state_reps.js.erb | 1 + config/environments/development.rb | 2 +- config/routes.rb | 2 +- spec/lib/civic_api_spec.rb | 41 ++++++++++++------ spec/requests/tools_spec.rb | 43 +++++++++++++++++++ .../state_leg_email_action_spec.rb | 16 +++++-- 16 files changed, 170 insertions(+), 61 deletions(-) rename app/assets/javascripts/application/tools/{email.js.erb => email.js} (75%) create mode 100644 app/views/tools/_state_leg_email.html.erb rename app/views/{action_page => tools}/_state_reps.html.erb (100%) create mode 100644 app/views/tools/state_reps.js.erb create mode 100644 spec/requests/tools_spec.rb diff --git a/app/assets/javascripts/application/tools/email.js.erb b/app/assets/javascripts/application/tools/email.js similarity index 75% rename from app/assets/javascripts/application/tools/email.js.erb rename to app/assets/javascripts/application/tools/email.js index 9a240425a..1018b9b6c 100644 --- a/app/assets/javascripts/application/tools/email.js.erb +++ 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(); + $form.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 index 97d07e676..1c9573b19 100644 --- 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'); -} \ No newline at end of file +} diff --git a/app/controllers/tools_controller.rb b/app/controllers/tools_controller.rb index eec3eca24..76308730b 100644 --- a/app/controllers/tools_controller.rb +++ b/app/controllers/tools_controller.rb @@ -131,18 +131,19 @@ 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_reps = CivicApi.state_rep_search(address, @email_campaign.leg_level) + + # TODO: clean this up, we just need to get the first non nil 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 + # end cleanup + + 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 index 3f28306de..008690a9c 100644 --- a/app/lib/civic_api.rb +++ b/app/lib/civic_api.rb @@ -6,27 +6,48 @@ # 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 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? + raise ArgumentError, "required argument is nil "\ + "{address: #{address}, roles: #{roles}}" + 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 index edef097e1..b77ea64b5 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -14,6 +14,8 @@ <%= page_title -%> <%= stylesheet_link_tag "application", media: "all" %> + <%= javascript_include_tag "application" %> + <%= yield(:head) if content_for?(:head) -%> <%= csrf_meta_tags %> @@ -97,9 +99,6 @@ - - <%= 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 index a27febe50..cd027b002 100644 --- 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 index 8db628e27..515ae9759 100644 --- a/app/views/tools/_email.html.erb +++ b/app/views/tools/_email.html.erb @@ -1,10 +1,3 @@ -<% if @email_campaign.state? %> -
-
1
-
2
-
3
-
-<% end %>
@@ -15,11 +8,7 @@
diff --git a/app/views/tools/_lookup_state_rep.html.erb b/app/views/tools/_lookup_state_rep.html.erb index a9fc61d4c..f3475d8a9 100644 --- a/app/views/tools/_lookup_state_rep.html.erb +++ b/app/views/tools/_lookup_state_rep.html.erb @@ -1,7 +1,7 @@

Enter your address below and we'll help you email your representatives (US addresses only).

- <%= form_tag(tools_state_reps_path, method: :get, remote: true, class: "form state-rep-lookup") do %> + <%= 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 %>
@@ -35,7 +35,7 @@ ).

- + <%= render "tools/loading" -%>
<% end %> diff --git a/app/views/tools/_state_leg_email.html.erb b/app/views/tools/_state_leg_email.html.erb new file mode 100644 index 000000000..cb0fefd65 --- /dev/null +++ b/app/views/tools/_state_leg_email.html.erb @@ -0,0 +1,24 @@ +
+
1
+
2
+
3
+
+
+
+
+

Take action

+
+ +
+ <%= render 'tools/lookup_state_rep' %> +
+
+
+ + +
diff --git a/app/views/action_page/_state_reps.html.erb b/app/views/tools/_state_reps.html.erb similarity index 100% rename from app/views/action_page/_state_reps.html.erb rename to app/views/tools/_state_reps.html.erb diff --git a/app/views/tools/state_reps.js.erb b/app/views/tools/state_reps.js.erb new file mode 100644 index 000000000..363b6dc4f --- /dev/null +++ b/app/views/tools/state_reps.js.erb @@ -0,0 +1 @@ +$(".state-reps").html("<%= escape_javascript render(partial: "state_reps") %>"); diff --git a/config/environments/development.rb b/config/environments/development.rb index 43314a031..4c1bb85e5 100644 --- 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 index 954366a12..ab52f122e 100644 --- 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/spec/lib/civic_api_spec.rb b/spec/lib/civic_api_spec.rb index a2920e45c..faadafd90 100644 --- a/spec/lib/civic_api_spec.rb +++ b/spec/lib/civic_api_spec.rb @@ -2,26 +2,36 @@ describe CivicApi do before do - Rails.application.config.google_civic_api_url = "http://civic.example.com" + Rails.application.config.google_civic_api_url = "https://civic.example.com" Rails.application.secrets.google_civic_api_key = "test-key-for-civic-api" end + let!(:data) do + { + "officials" => [{ + "name" => "Sponge Bob", + "party" => "Sandy Party", + "emails" => ["spongebob@clarinetfans.annoying"] + }] + } + end + let(:address) { "815 Eddy St 94109" } + describe ".state_rep_search" do - let(:email_campaign) { FactoryBot.create(:email_campaign, :state_leg) } - let(:address) { "815 Eddy St 94109" } + let(:role) { "legislatorUpperBody" } it "should get civic_api_url with the correct params" do expect(RestClient).to receive(:get) do |url, opts| - expect(url).to eq("http://civic.example.com") + expect(url).to eq("https://civic.example.com") expect(opts[:params]).not_to be_nil - expect(opts[:params][:address]).to eq("815 Eddy St 94109") + expect(opts[:params][:address]).to eq(address) expect(opts[:params][:includeOffices]).to eq(true) expect(opts[:params][:levels]).to eq("administrativeArea1") - expect(opts[:params][:roles]).to eq("legislatorUpperBody") + expect(opts[:params][:roles]).to eq(role) expect(opts[:params][:key]).to eq("test-key-for-civic-api") - end + end.and_return(double(body: data.to_json)) - CivicApi.state_rep_search(address, email_campaign.leg_level) + CivicApi.state_rep_search(address, role) end it "should raise ArgumentError if a required param is missing" do @@ -32,16 +42,23 @@ end.to raise_error(ArgumentError) expect do - CivicApi.state_rep_search(nil, email_campaign.leg_level) + CivicApi.state_rep_search(nil, role) end.to raise_error(ArgumentError) expect do CivicApi.state_rep_search(address) end.to raise_error(ArgumentError) + end - expect do - CivicApi.state_rep_search(address, email_campaign.leg_level) - end.not_to raise_error + it "returns an array of officials" do + role = CivicApi::VALID_ROLES.first + stub_request(:any, + "https://civic.example.com/?address=815%20Eddy%20St%2094109"\ + "&includeOffices=true&key=test-key-for-civic-api&"\ + "levels=administrativeArea1&roles=#{role}") + .to_return(status: 200, body: data.to_json, headers: {}) + expect(described_class.state_rep_search(address, role)).to \ + eq(data["officials"]) end end diff --git a/spec/requests/tools_spec.rb b/spec/requests/tools_spec.rb new file mode 100644 index 000000000..fd4da2bfe --- /dev/null +++ b/spec/requests/tools_spec.rb @@ -0,0 +1,43 @@ +require "rails_helper" + +RSpec.describe "Tools Controller", type: :request do + let!(:campaign) { FactoryBot.create(:email_campaign, :state_leg) } + let!(:action_page) { campaign.action_page } + let!(:officials) do + [{ + "name" => "Sponge Bob", + "party" => "Sandy Party", + "emails" => ["spongebob@clarinetfans.annoying"] + }] + end + let!(:params) do + { + street_address: "815 Eddy St", + zipcode: "94109", + email_campaign_id: campaign.id + } + end + let!(:address) { "#{params[:street_address]} #{params[:zipcode]}" } + + before do + Rails.application.config.google_civic_api_url = "https://civic.example.com" + Rails.application.secrets.google_civic_api_key = "test-key-for-civic-api" + end + + describe "GET tools/state_reps" do + # TODO: should this be a POST? + it "returns json containing rep data for a given address" do + # mock civic api + civic_api = class_double("CivicApi") + .as_stubbed_const(transfer_nested_constants: true) + allow(civic_api).to receive(:state_rep_search) + .with(address, campaign.leg_level) + .and_return(officials) + + get "/tools/state_reps", params: params + + expect(response).to have_http_status(200) + expect(response.body).to include(officials.first["name"]) + end + end +end diff --git a/spec/system/action_pages/state_leg_email_action_spec.rb b/spec/system/action_pages/state_leg_email_action_spec.rb index d7a56cf94..35e0147fc 100644 --- a/spec/system/action_pages/state_leg_email_action_spec.rb +++ b/spec/system/action_pages/state_leg_email_action_spec.rb @@ -4,14 +4,22 @@ let!(:state_action) do FactoryBot.create(:email_campaign, :state_leg).action_page end - let(:json_parseable_state_officials) { '{"officials": [{"name": "Sponge Bob", "party": "Sandy Party", "emails": ["spongebob@clarinetfans.annoying"]}]}' } + let(:data) do + { + "officials" => [{ + "name" => "Sponge Bob", + "party" => "Sandy Party", + "emails" => ["spongebob@clarinetfans.annoying"] + }] + } + end before do - Rails.application.config.google_civic_api_url = "http://civic.example.com" + Rails.application.config.google_civic_api_url = "https://civic.example.com" Rails.application.secrets.google_civic_api_key = "test-key-for-civic-api" - stub_request(:get, "http://civic.example.com/?address=815%20Eddy%20St%2094109&includeOffices=true&key=test-key-for-civic-api&levels=administrativeArea1&roles=legislatorUpperBody") - .to_return(status: 200, body: json_parseable_state_officials, headers: {}) + stub_request(:get, "https://civic.example.com/?address=815%20Eddy%20St%2094109&includeOffices=true&key=test-key-for-civic-api&levels=administrativeArea1&roles=legislatorUpperBody") + .to_return(status: 200, body: data.to_json, headers: {}) end it "allows vistors to see look up their representatives" do From 3b05f0f36b3dcb69435d35308fba156bc10bf91a Mon Sep 17 00:00:00 2001 From: sylph Date: Mon, 14 Apr 2025 14:20:37 -0700 Subject: [PATCH 2/7] Mock state rep request for testing in development --- app/controllers/tools_controller.rb | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/app/controllers/tools_controller.rb b/app/controllers/tools_controller.rb index 76308730b..d6f9a434c 100644 --- a/app/controllers/tools_controller.rb +++ b/app/controllers/tools_controller.rb @@ -1,9 +1,13 @@ require "rest_client" require "uri" require "json" +# TODO: remove +require "webmock" class ToolsController < ApplicationController include Tooling +# TODO: remove + include WebMock::API before_action :set_user before_action :set_action_page @@ -127,6 +131,25 @@ def email # 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 + # TODO: remove + WebMock.enable! + WebMock.disable_net_connect!(allow_localhost: true) + + mock_data = { + "officials" => [{ + "name" => "Sponge Bob", + "party" => "Sandy Party", + "emails" => ["spongebob@clarinetfans.annoying"] + }] + } + + Rails.application.config.google_civic_api_url = "https://civic.example.com" + Rails.application.secrets.google_civic_api_key = "test-key-for-civic-api" + + stub_request(:get, "https://civic.example.com/?address=815%20Eddy%20St%2094109&includeOffices=true&key=test-key-for-civic-api&levels=administrativeArea1&roles=legislatorLowerBody") + .to_return(status: 200, body: mock_data.to_json, headers: {}) + + # end remove @email_campaign = EmailCampaign.find(params[:email_campaign_id]) @actionPage = @email_campaign.action_page # TODO: strong params this From 7f92d859b116149d9065f654960aa6fadb4ed6bb Mon Sep 17 00:00:00 2001 From: sylph Date: Tue, 15 Apr 2025 15:35:06 -0700 Subject: [PATCH 3/7] Clean up state level refactor + fix its styles --- .../javascripts/application/tools/email.js | 2 +- .../stylesheets/application/action_page.scss | 6 ++ app/views/tools/_lookup_state_rep.html.erb | 61 +++++++++---------- app/views/tools/_state_leg_email.html.erb | 3 +- app/views/tools/_state_reps.html.erb | 4 +- spec/controllers/tools_controller_spec.rb | 20 ------ spec/requests/tools_spec.rb | 7 +-- .../state_leg_email_action_spec.rb | 2 +- 8 files changed, 45 insertions(+), 60 deletions(-) diff --git a/app/assets/javascripts/application/tools/email.js b/app/assets/javascripts/application/tools/email.js index 1018b9b6c..2c2087847 100644 --- a/app/assets/javascripts/application/tools/email.js +++ b/app/assets/javascripts/application/tools/email.js @@ -26,7 +26,7 @@ $(document).on('ready', function() { } update_tabs(1, 2); $(".progress-striped").hide(); - $form.remove(); + $(".address-lookup").remove(); } else if (data.responseText) { show_error(data.responseText, $form); } else { diff --git a/app/assets/stylesheets/application/action_page.scss b/app/assets/stylesheets/application/action_page.scss index 7a30dde8c..334ed7477 100644 --- 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/views/tools/_lookup_state_rep.html.erb b/app/views/tools/_lookup_state_rep.html.erb index f3475d8a9..e31e2dac8 100644 --- a/app/views/tools/_lookup_state_rep.html.erb +++ b/app/views/tools/_lookup_state_rep.html.erb @@ -1,43 +1,40 @@ -
+

Enter your address below and we'll help you email your representatives (US addresses only).

-
- <%= 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 %> + <%= 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 %> -
-

- ? - We'll use your address to find your state representatives. - - Look up your state representatives -

-
+
+

+ ? + We'll use your address to find your state representatives. + + Look up your state representatives +

+
-
- <%= text_field_tag :street_address, (current_user && current_user.street_address), - placeholder: "Street Address", "aria-label": "Street Address", - required: true, class: "form-control" %> -
+
+ <%= text_field_tag :street_address, (current_user && current_user.street_address), + placeholder: "Street Address", "aria-label": "Street Address", + required: true, class: "form-control" %> +
-
- <%= 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 %> -
+
+ <%= 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 %> +
- + -

Uses Google's APIs ( +

Uses Google's APIs ( why? Representative information powered by the Civic Information API. ). -

-
- - <%= render "tools/loading" -%> -
-<% end %> +

+ + + <%= render "tools/loading" -%> + <% end %>
-
diff --git a/app/views/tools/_state_leg_email.html.erb b/app/views/tools/_state_leg_email.html.erb index cb0fefd65..4ae68670f 100644 --- a/app/views/tools/_state_leg_email.html.erb +++ b/app/views/tools/_state_leg_email.html.erb @@ -9,8 +9,9 @@

Take action

-
+
<%= render 'tools/lookup_state_rep' %> +
diff --git a/app/views/tools/_state_reps.html.erb b/app/views/tools/_state_reps.html.erb index 58740e8d3..b97a50e27 100644 --- a/app/views/tools/_state_reps.html.erb +++ b/app/views/tools/_state_reps.html.erb @@ -1,13 +1,15 @@
- <%= "This action is for the #{legislative_level_from_state_representative_info(@email_campaign.leg_level)}." %> +

<%= "This action is for the #{legislative_level_from_state_representative_info(@email_campaign.leg_level)}." %>

<% @state_reps.each do |sr| %>
+

<%= "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 %> +

<% end %>
diff --git a/spec/controllers/tools_controller_spec.rb b/spec/controllers/tools_controller_spec.rb index 1782c00ea..1b94581b3 100644 --- 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/requests/tools_spec.rb b/spec/requests/tools_spec.rb index fd4da2bfe..6d20a363e 100644 --- a/spec/requests/tools_spec.rb +++ b/spec/requests/tools_spec.rb @@ -18,23 +18,22 @@ } end let!(:address) { "#{params[:street_address]} #{params[:zipcode]}" } + let!(:headers) { { "CONTENT_TYPE" => "application/javascript" } } before do Rails.application.config.google_civic_api_url = "https://civic.example.com" Rails.application.secrets.google_civic_api_key = "test-key-for-civic-api" end - describe "GET tools/state_reps" do - # TODO: should this be a POST? + describe "POST tools/state_reps" do it "returns json containing rep data for a given address" do - # mock civic api civic_api = class_double("CivicApi") .as_stubbed_const(transfer_nested_constants: true) allow(civic_api).to receive(:state_rep_search) .with(address, campaign.leg_level) .and_return(officials) - get "/tools/state_reps", params: params + post "/tools/state_reps", params: params, xhr: true expect(response).to have_http_status(200) expect(response.body).to include(officials.first["name"]) diff --git a/spec/system/action_pages/state_leg_email_action_spec.rb b/spec/system/action_pages/state_leg_email_action_spec.rb index 35e0147fc..98c7087ad 100644 --- a/spec/system/action_pages/state_leg_email_action_spec.rb +++ b/spec/system/action_pages/state_leg_email_action_spec.rb @@ -28,7 +28,7 @@ fill_in "street_address", with: "815 Eddy St" fill_in "zipcode", with: "94109" - click_on "See Your Representatives" + click_on "Find your reps" expect(page).to have_content("Sponge Bob") expect(page).not_to have_content("Thank You!") From aac6e59867c232b0b0766082042f23a3f4424eb4 Mon Sep 17 00:00:00 2001 From: Christa Hartsock Date: Tue, 22 Apr 2025 17:01:46 -0700 Subject: [PATCH 4/7] Some small cleanup around state-level actions --- app/controllers/tools_controller.rb | 10 ++-------- app/lib/civic_api.rb | 12 ++++++++---- config/secrets.yml | 2 ++ spec/lib/civic_api_spec.rb | 5 ----- spec/requests/tools_spec.rb | 5 ----- .../action_pages/state_leg_email_action_spec.rb | 3 --- 6 files changed, 12 insertions(+), 25 deletions(-) diff --git a/app/controllers/tools_controller.rb b/app/controllers/tools_controller.rb index d6f9a434c..ff3551d03 100644 --- a/app/controllers/tools_controller.rb +++ b/app/controllers/tools_controller.rb @@ -156,14 +156,8 @@ def state_reps address = "#{params[:street_address]} #{params[:zipcode]}" @state_reps = CivicApi.state_rep_search(address, @email_campaign.leg_level) - # TODO: clean this up, we just need to get the first non nil - 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 - # end cleanup + # 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 diff --git a/app/lib/civic_api.rb b/app/lib/civic_api.rb index 008690a9c..f000f9724 100644 --- a/app/lib/civic_api.rb +++ b/app/lib/civic_api.rb @@ -9,13 +9,17 @@ module CivicApi 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) - unless [address, roles].all? - raise ArgumentError, "required argument is nil "\ - "{address: #{address}, roles: #{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}" + raise ArgumentError, "Invalid role for Civic API `#{roles}`" end # `includeOffices` param is needed in order to get officials list diff --git a/config/secrets.yml b/config/secrets.yml index 36034bb3d..6c1373884 100644 --- a/config/secrets.yml +++ b/config/secrets.yml @@ -72,6 +72,8 @@ test: civi_key: xyz smarty_streets_id: abc mailings_from: "EFF Action " + 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/lib/civic_api_spec.rb b/spec/lib/civic_api_spec.rb index faadafd90..ba1ff7a94 100644 --- a/spec/lib/civic_api_spec.rb +++ b/spec/lib/civic_api_spec.rb @@ -1,11 +1,6 @@ require "rails_helper" describe CivicApi do - before do - Rails.application.config.google_civic_api_url = "https://civic.example.com" - Rails.application.secrets.google_civic_api_key = "test-key-for-civic-api" - end - let!(:data) do { "officials" => [{ diff --git a/spec/requests/tools_spec.rb b/spec/requests/tools_spec.rb index 6d20a363e..eeedfae67 100644 --- a/spec/requests/tools_spec.rb +++ b/spec/requests/tools_spec.rb @@ -20,11 +20,6 @@ let!(:address) { "#{params[:street_address]} #{params[:zipcode]}" } let!(:headers) { { "CONTENT_TYPE" => "application/javascript" } } - before do - Rails.application.config.google_civic_api_url = "https://civic.example.com" - Rails.application.secrets.google_civic_api_key = "test-key-for-civic-api" - end - describe "POST tools/state_reps" do it "returns json containing rep data for a given address" do civic_api = class_double("CivicApi") diff --git a/spec/system/action_pages/state_leg_email_action_spec.rb b/spec/system/action_pages/state_leg_email_action_spec.rb index 98c7087ad..622522b2f 100644 --- a/spec/system/action_pages/state_leg_email_action_spec.rb +++ b/spec/system/action_pages/state_leg_email_action_spec.rb @@ -15,9 +15,6 @@ end before do - Rails.application.config.google_civic_api_url = "https://civic.example.com" - Rails.application.secrets.google_civic_api_key = "test-key-for-civic-api" - stub_request(:get, "https://civic.example.com/?address=815%20Eddy%20St%2094109&includeOffices=true&key=test-key-for-civic-api&levels=administrativeArea1&roles=legislatorUpperBody") .to_return(status: 200, body: data.to_json, headers: {}) end From e1e2b00d5ec40891a37519239c99ca158b89533c Mon Sep 17 00:00:00 2001 From: Christa Hartsock Date: Tue, 22 Apr 2025 17:02:26 -0700 Subject: [PATCH 5/7] Update activism director -> Jason --- config/action_email.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/action_email.md b/config/action_email.md index dd6058e44..4bdfef131 100644 --- 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 From 26493427ee188a1217e530cd002a9de719b5be0e Mon Sep 17 00:00:00 2001 From: Christa Hartsock Date: Tue, 22 Apr 2025 17:02:34 -0700 Subject: [PATCH 6/7] Revert "Mock state rep request for testing in development" This reverts commit 3b05f0f36b3dcb69435d35308fba156bc10bf91a. --- app/controllers/tools_controller.rb | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/app/controllers/tools_controller.rb b/app/controllers/tools_controller.rb index ff3551d03..399ea0242 100644 --- a/app/controllers/tools_controller.rb +++ b/app/controllers/tools_controller.rb @@ -1,13 +1,9 @@ require "rest_client" require "uri" require "json" -# TODO: remove -require "webmock" class ToolsController < ApplicationController include Tooling -# TODO: remove - include WebMock::API before_action :set_user before_action :set_action_page @@ -131,25 +127,6 @@ def email # 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 - # TODO: remove - WebMock.enable! - WebMock.disable_net_connect!(allow_localhost: true) - - mock_data = { - "officials" => [{ - "name" => "Sponge Bob", - "party" => "Sandy Party", - "emails" => ["spongebob@clarinetfans.annoying"] - }] - } - - Rails.application.config.google_civic_api_url = "https://civic.example.com" - Rails.application.secrets.google_civic_api_key = "test-key-for-civic-api" - - stub_request(:get, "https://civic.example.com/?address=815%20Eddy%20St%2094109&includeOffices=true&key=test-key-for-civic-api&levels=administrativeArea1&roles=legislatorLowerBody") - .to_return(status: 200, body: mock_data.to_json, headers: {}) - - # end remove @email_campaign = EmailCampaign.find(params[:email_campaign_id]) @actionPage = @email_campaign.action_page # TODO: strong params this From b32d36e2766526323ab94f1eab99a92d056d7879 Mon Sep 17 00:00:00 2001 From: Christa Hartsock Date: Tue, 22 Apr 2025 17:07:04 -0700 Subject: [PATCH 7/7] Linter fixes --- app/controllers/tools_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/tools_controller.rb b/app/controllers/tools_controller.rb index 399ea0242..5266f6e52 100644 --- a/app/controllers/tools_controller.rb +++ b/app/controllers/tools_controller.rb @@ -133,8 +133,8 @@ def state_reps address = "#{params[:street_address]} #{params[:zipcode]}" @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 + # 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