URL parameter targets for call tool (#991)

* backend changes to allow calls to target passed in params, w spec * pass relevant url params in to call tool js * remove misuse of country_code call_tool parameter * Abstraction for exposing data hash for call tool plugin * Use ExposedData helper. * finish out back-end for passing targets through params * update yarn snapshots * close the loop, get the front-end to handle prefilled targets * deploy call-preselect to staging * fix flow errors * add missing config for call targeting
SumOfUs · Jul 7, 2017 · bf817af · bf817af
1 parent cceb438
commit bf817af
Show file tree

Hide file tree

Showing 18 changed files with 338 additions and 47 deletions.
diff --git a/app/controllers/api/calls_controller.rb b/app/controllers/api/calls_controller.rb
@@ -1,4 +1,5 @@
 # frozen_string_literal: true
+
 class Api::CallsController < ApplicationController
   skip_before_action :verify_authenticity_token, raise: false
 
@@ -16,7 +17,8 @@ def create
 
   def call_params
     params.require(:call)
-      .permit(:member_phone_number, :target_id)
+      .permit(:member_phone_number, :target_id, :target_title, :target_name,
+              :target_phone_number, :target_phone_extension, :checksum)
       .merge(page_id: params[:page_id],
              member_id: recognized_member&.id)
   end

diff --git a/app/javascript/call_tool/CallToolView.js b/app/javascript/call_tool/CallToolView.js
@@ -59,6 +59,11 @@ type OwnProps = {
   countries: Country[],
   countriesPhoneCodes: CountryPhoneCode[],
   onSuccess?: (target: any) => void,
+  targetPhoneNumber?: string,
+  targetPhoneExtension?: string,
+  targetName?: string,
+  targetTitle?: string,
+  checksum?: string,
   intl: IntlShape,
 };
 
@@ -148,6 +153,19 @@ class CallToolView extends Component {
     return this.props.targets;
   }
 
+  hasPrefilledTarget() {
+    return !!this.props.targetPhoneNumber && !!this.props.checksum;
+  }
+
+  prefilledTargetForDisplay() {
+    return {
+      countryCode: '',
+      name: this.props.targetName,
+      title: this.props.targetTitle,
+      id: 'prefilled',
+    };
+  }
+
   guessMemberPhoneCountryCode(countryCode: string) {
     const country = find(this.props.countries, t => {
       return t.code === countryCode;
@@ -177,13 +195,13 @@ class CallToolView extends Component {
     return sample(this.candidates());
   }
 
-  selectTarget = (id: string) => {
+  selectTarget(id: string) {
     const target = find(this.props.targets, { id });
     this.setState(prevState => ({
       ...prevState,
       selectedTarget: target,
     }));
-  };
+  }
 
   selectNewTargetFromCountryCode(countryCode: string) {
     return sample(this.candidates(countryCode));
@@ -195,16 +213,32 @@ class CallToolView extends Component {
     this.setState({ errors: {}, loading: true });
     ChampaignAPI.calls
       .create({
+        ...this.targetHash(),
         pageId: this.props.pageId,
         memberPhoneNumber:
           this.state.form.memberPhoneCountryCode +
             this.state.form.memberPhoneNumber,
-        // $FlowIgnore
-        targetId: this.state.selectedTarget.id,
       })
       .then(this.submitSuccessful.bind(this), this.submitFailed.bind(this));
   }
 
+  targetHash() {
+    if (this.hasPrefilledTarget()) {
+      return {
+        targetPhoneExtension: this.props.targetPhoneExtension,
+        targetPhoneNumber: this.props.targetPhoneNumber,
+        targetTitle: this.props.targetTitle,
+        targetName: this.props.targetName,
+        checksum: this.props.checksum,
+      };
+    } else {
+      return {
+        // $FlowIgnore
+        targetId: this.state.selectedTarget.id,
+      };
+    }
+  }
+
   validateForm() {
     const newErrors = {};
 
@@ -289,13 +323,21 @@ class CallToolView extends Component {
           </div>}
 
         <Form
-          allowManualTargetSelection={this.props.allowManualTargetSelection}
-          targetByCountryEnabled={this.props.targetByCountryEnabled}
+          allowManualTargetSelection={
+            this.props.allowManualTargetSelection && !this.hasPrefilledTarget()
+          }
+          targetByCountryEnabled={
+            this.props.targetByCountryEnabled && !this.hasPrefilledTarget()
+          }
           restrictToSingleCountry={!!this.props.restrictedCountryCode}
           countries={this.props.countries}
           countriesPhoneCodes={this.props.countriesPhoneCodes}
           targets={this.candidates()}
-          selectedTarget={this.state.selectedTarget}
+          selectedTarget={
+            this.hasPrefilledTarget()
+              ? this.prefilledTargetForDisplay()
+              : this.state.selectedTarget
+          }
           form={this.state.form}
           errors={this.state.errors}
           onCountryCodeChange={this.countryCodeChanged.bind(this)}

diff --git a/app/javascript/components/DonationBands/__snapshots__/DonationBands.test.js.snap b/app/javascript/components/DonationBands/__snapshots__/DonationBands.test.js.snap
@@ -10,6 +10,7 @@ exports[`renders correctly 1`] = `
       "call_tool.errors.phone_number.cant_connect": "can't connect to this phone number, please check it's correct or try another one",
       "call_tool.errors.phone_number.is_invalid": "is not a valid phone number",
       "call_tool.errors.phone_number.too_short": "must have at least 6 digits",
+      "call_tool.errors.target.missing": "Something wasn't right about the number you're trying to call.",
       "call_tool.errors.target.outdated": "It seems the number we're trying to connect you to is no longer available. Please reload the page and try again.",
       "call_tool.errors.unknown": "Oops! Something went wrong, please try a different phone number or again in a few minutes.",
       "call_tool.fine_print": "SumOfUs will protect <a href=\\"https://sumofus.org/privacy\\" target=\\"_blank\\">your privacy</a>, and keep you updated. If you provide your phone number we may also call or SMS you about campaigns.",
@@ -223,6 +224,7 @@ exports[`renders correctly 1`] = `
             "call_tool.errors.phone_number.cant_connect": "can't connect to this phone number, please check it's correct or try another one",
             "call_tool.errors.phone_number.is_invalid": "is not a valid phone number",
             "call_tool.errors.phone_number.too_short": "must have at least 6 digits",
+            "call_tool.errors.target.missing": "Something wasn't right about the number you're trying to call.",
             "call_tool.errors.target.outdated": "It seems the number we're trying to connect you to is no longer available. Please reload the page and try again.",
             "call_tool.errors.unknown": "Oops! Something went wrong, please try a different phone number or again in a few minutes.",
             "call_tool.fine_print": "SumOfUs will protect <a href=\\"https://sumofus.org/privacy\\" target=\\"_blank\\">your privacy</a>, and keep you updated. If you provide your phone number we may also call or SMS you about campaigns.",

diff --git a/app/javascript/components/__snapshots__/ComponentWrapper.test.js.snap b/app/javascript/components/__snapshots__/ComponentWrapper.test.js.snap
@@ -25,6 +25,7 @@ exports[`Snapshots: With default messages object 1`] = `
       "call_tool.errors.phone_number.cant_connect": "can't connect to this phone number, please check it's correct or try another one",
       "call_tool.errors.phone_number.is_invalid": "is not a valid phone number",
       "call_tool.errors.phone_number.too_short": "must have at least 6 digits",
+      "call_tool.errors.target.missing": "Something wasn't right about the number you're trying to call.",
       "call_tool.errors.target.outdated": "It seems the number we're trying to connect you to is no longer available. Please reload the page and try again.",
       "call_tool.errors.unknown": "Oops! Something went wrong, please try a different phone number or again in a few minutes.",
       "call_tool.fine_print": "SumOfUs will protect <a href=\\"https://sumofus.org/privacy\\" target=\\"_blank\\">your privacy</a>, and keep you updated. If you provide your phone number we may also call or SMS you about campaigns.",

diff --git a/app/javascript/util/ChampaignAPI.js b/app/javascript/util/ChampaignAPI.js
@@ -47,12 +47,17 @@ type CreateCallParams = {
 const createCall = function(
   params: CreateCallParams
 ): Promise<OperationResponse> {
-  const payload = {
-    call: {
-      member_phone_number: params.memberPhoneNumber,
-      target_id: params.targetId,
-    },
-  };
+  const inner = {};
+  inner.member_phone_number = params.memberPhoneNumber;
+  if (!!params.targetPhoneExtension)
+    inner.target_phone_extension = params.targetPhoneExtension;
+  if (!!params.targetPhoneNumber)
+    inner.target_phone_number = params.targetPhoneNumber;
+  if (!!params.targetTitle) inner.target_title = params.targetTitle;
+  if (!!params.targetName) inner.target_name = params.targetName;
+  if (!!params.checksum) inner.checksum = params.checksum;
+  if (!!params.targetId) inner.target_id = params.targetId;
+  const payload = { call: inner };
 
   return new Promise((resolve, reject) => {
     $.post(`/api/pages/${params.pageId}/call`, payload)

diff --git a/app/liquid/liquid_renderer.rb b/app/liquid/liquid_renderer.rb
@@ -1,8 +1,9 @@
 # frozen_string_literal: true
+
 class LiquidRenderer
   include Rails.application.routes.url_helpers
 
-  HIDDEN_FIELDS = %w(source bucket referrer_id rid akid referring_akid).freeze
+  HIDDEN_FIELDS = %w[source bucket referrer_id rid akid referring_akid].freeze
 
   def initialize(page, location: nil, member: nil, url_params: {}, payment_methods: [])
     @page = page
@@ -116,7 +117,10 @@ def thermometer
   end
 
   def call_tool_data
-    plugin_data.deep_symbolize_keys[:plugins][:call_tool]
+    CallTool::ExposedData.new(
+      plugin_data.deep_symbolize_keys[:plugins][:call_tool],
+      @url_params
+    ).to_h
   end
 
   def email_target_data

diff --git a/app/services/call_creator.rb b/app/services/call_creator.rb
@@ -1,6 +1,8 @@
 # frozen_string_literal: true
+
 class CallCreator
   include Rails.application.routes.url_helpers
+  attr_accessor :call, :page
 
   def initialize(params)
     @params = params.clone
@@ -9,26 +11,22 @@ def initialize(params)
 
   def run
     sanitize_params!
-    page = Page.find(@params[:page_id])
-    @call = Call.new(page: page,
-                     member_id: @params[:member_id],
-                     member_phone_number: @params[:member_phone_number],
-                     target_id: @params[:target_id])
-
-    validate_target
-    validate_call_tool
+    @page = Page.find(@params[:page_id])
+    build_call_record
+    validate_target if errors.blank?
+    validate_call_tool if errors.blank?
 
     if errors.blank?
       Call.transaction do
-        place_call if @call.save
+        place_call if call.save
       end
     end
 
     errors.blank?
   end
 
   def errors
-    @call.errors.messages.clone.tap do |e|
+    call.errors.messages.clone.tap do |e|
       @errors.each do |key, val|
         e[key] ||= []
         e[key] += val
@@ -45,16 +43,38 @@ def sanitize_params!
   rescue Phony::NormalizationError
   end
 
+  def build_call_record
+    non_target_params = {
+      page: page,
+      member_id: @params[:member_id],
+      member_phone_number: @params[:member_phone_number]
+    }
+    full_params = if valid_manual_target
+                    non_target_params.merge(target: manual_target)
+                  elsif @params[:target_id].present?
+                    non_target_params.merge(target_id: @params[:target_id])
+                  else
+                    add_error(:base, I18n.t('call_tool.errors.target.missing'))
+                    non_target_params.merge(target: {})
+                  end
+
+    @call = Call.new(full_params)
+  end
+
+  def valid_manual_target
+    CallTool::ChecksumValidator.validate(@params[:target_phone_number], @params[:checksum])
+  end
+
   # TODO: Move method to service class, handle error messages in there.
   def place_call
     client = Twilio::REST::Client.new.account.calls
     client.create(
-      from: @call.caller_id,
-      to: @call.member_phone_number,
-      url: call_start_url(@call),
-      status_callback: member_call_event_url(@call),
+      from: call.caller_id,
+      to: call.member_phone_number,
+      url: call_start_url(call),
+      status_callback: member_call_event_url(call),
       status_callback_method: 'POST',
-      status_callback_event: %w(initiated ringing answered completed)
+      status_callback_event: %w[initiated ringing answered completed]
     )
   rescue Twilio::REST::RequestError => e
     # 13223: Dial: Invalid phone number format
@@ -63,11 +83,11 @@ def place_call
     # 13226: Dial: Invalid country code
     # 21211: Invalid 'To' Phone Number
     # 21214: 'To' phone number cannot be reached
-    @call.update!(twilio_error_code: e.code, status: 'failed')
+    call.update!(twilio_error_code: e.code, status: 'failed')
     if (e.code >= 13_223 && e.code <= 13_226) || [21_211, 21_214].include?(e.code)
       add_error(:member_phone_number, I18n.t('call_tool.errors.phone_number.cant_connect'))
     else
-      Rails.logger.error("Twilio Error: API responded with code #{e.code} for #{@call.attributes.inspect}")
+      Rails.logger.error("Twilio Error: API responded with code #{e.code} for #{call.attributes.inspect}")
       add_error(:base, I18n.t('call_tool.errors.unknown'))
     end
   end
@@ -76,13 +96,13 @@ def place_call
   # of target_ids on the browser are no longer valid.
   # This validation checks for this edge case.
   def validate_target
-    if @call.target.blank? && @params[:target_id].present?
+    if call.target.blank? && @params[:target_id].present?
       add_error(:base, I18n.t('call_tool.errors.target.outdated'))
     end
   end
 
   def validate_call_tool
-    if @call.caller_id.blank?
+    if call.caller_id.blank?
       add_error(:base, 'Please configure a Caller ID before trying to use the call tool')
     end
   end
@@ -91,4 +111,13 @@ def add_error(key, message)
     @errors[key] ||= []
     @errors[key] << message
   end
+
+  def manual_target
+    {
+      phone_number: @params[:target_phone_number],
+      phone_extension: @params[:target_phone_extension],
+      name: @params[:target_name],
+      title: @params[:target_title] || ''
+    }
+  end
 end
diff --git a/app/services/call_tool/checksum_validator.rb b/app/services/call_tool/checksum_validator.rb
@@ -0,0 +1,9 @@
+module CallTool
+  class ChecksumValidator
+    def self.validate(phone_number, checksum)
+      return false if phone_number.blank? || checksum.blank?
+      unhashed = "#{phone_number}#{Settings.calls.targeting_secret}"
+      checksum == Digest::SHA256.hexdigest(unhashed)[0..5]
+    end
+  end
+end
diff --git a/app/services/call_tool/exposed_data.rb b/app/services/call_tool/exposed_data.rb
@@ -0,0 +1,23 @@
+module CallTool
+  class ExposedData
+    attr_reader :query
+    RELEVANT_ATTRIBUTES = %i[target_name target_title target_phone_number target_phone_extension checksum].freeze
+
+    def initialize(plugin_data, query)
+      @query = query
+      @plugin_data = plugin_data
+    end
+
+    def to_h
+      return @plugin_data unless encoded_target_valid?
+
+      @plugin_data.map do |key, data|
+        [key, data.merge(query.slice(*RELEVANT_ATTRIBUTES))]
+      end.to_h
+    end
+
+    def encoded_target_valid?
+      CallTool::ChecksumValidator.validate(@query[:target_phone_number], @query[:checksum])
+    end
+  end
+end
diff --git a/circle.yml b/circle.yml
@@ -36,7 +36,7 @@ deployment:
       - BRAINTREE_TOKEN_URL=$PRODUCTION_BRAINTREE_TOKEN_URL ./bin/build.sh
       - ./bin/deploy.sh $CIRCLE_SHA1 'champaign' 'env-production' 'champaign-assets-production' 'logs3.papertrailapp.com:44107' 'actions.sumofus.org'
   staging:
-    branch:  'development'
+    branch:  'call-preselect'
     commands:
       - BRAINTREE_TOKEN_URL=$STAGING_BRAINTREE_TOKEN_URL ./bin/build.sh
       - ./bin/deploy.sh $CIRCLE_SHA1 'champaign' 'env-staging' 'champaign-assets-staging' 'logs3.papertrailapp.com:34848' 'action-staging.sumofus.org'
diff --git a/config/locales/member_facing.de.yml b/config/locales/member_facing.de.yml
@@ -82,6 +82,9 @@ de:
     select_target: "Geben Sie Ihr Land und Ihre Telefonnummer ein -- wir rufen Sie dann umgehend an. "
     errors:
       unknown: "Entschuldigung, hier hat sich ein Fehler eingeschlichen. Bitte probieren Sie es in ein paar Minuten noch einmal, oder verwenden Sie eine andere Telefonnummer. "
+      target:
+        outdated: "Entschuldigung, hier hat sich ein Fehler eingeschlichen. Bitte probieren Sie es in ein paar Minuten noch einmal, oder verwenden Sie eine andere Telefonnummer. "
+        missing: "Entschuldigung, hier hat sich ein Fehler eingeschlichen. Bitte probieren Sie es in ein paar Minuten noch einmal, oder verwenden Sie eine andere Telefonnummer. "
       phone_number:
         too_short: "enthält mindestens 6 Ziffern"
         cant_connect: "Es kann keine Verbindung zu dieser Telefonnummer hergestellt werden. Bitte überprüfen Sie, ob die Nummer korrekt ist, oder versuchen Sie es über eine andere Nummer"

diff --git a/config/locales/member_facing.en.yml b/config/locales/member_facing.en.yml
@@ -107,6 +107,7 @@ en:
       unknown: "Oops! Something went wrong, please try a different phone number or again in a few minutes."
       target:
         outdated: "It seems the number we're trying to connect you to is no longer available. Please reload the page and try again."
+        missing: "Something wasn't right about the number you're trying to call."
       phone_number:
         too_short: "must have at least 6 digits"
         cant_connect: "can't connect to this phone number, please check it's correct or try another one"

diff --git a/config/locales/member_facing.fr.yml b/config/locales/member_facing.fr.yml
@@ -82,6 +82,9 @@ fr:
     select_target: "Ecrivez simplement votre pays et votre numéro de téléphone -- et nous vous contacterons dans quelques instants."
     errors:
       unknown: "Oups ! Quelque chose n'a pas fonctionné, veuillez réessayer dans quelques minutes ou utiliser un autre numéro de téléphone."
+      target:
+        outdated: "Oups ! Quelque chose n'a pas fonctionné, veuillez réessayer dans quelques minutes ou utiliser un autre numéro de téléphone."
+        missing: "Oups ! Quelque chose n'a pas fonctionné, veuillez réessayer dans quelques minutes ou utiliser un autre numéro de téléphone."
       phone_number:
         too_short: "Doit comporter au moins 6 chiffres"
         cant_connect: "Nous ne pouvons vous mettre en relation avec ce correspondant. Veuillez vérifier le numéro de téléphone, ou en essayer un autre. "