Make phone number non-editable LG-1722

app/controllers/users/phone_setup_controller.rb

@@ -9,12 +9,12 @@ class PhoneSetupController < ApplicationController
     before_action :set_setup_presenter
 
     def index
-      @user_phone_form = UserPhoneForm.new(current_user, nil)
+      @user_phone_form = UserPhoneForm.new(current_user)
       analytics.track_event(Analytics::USER_REGISTRATION_PHONE_SETUP_VISIT)
     end
 
     def create
-      @user_phone_form = UserPhoneForm.new(current_user, nil)
+      @user_phone_form = UserPhoneForm.new(current_user)
       result = @user_phone_form.submit(user_phone_form_params)
       analytics.track_event(Analytics::MULTI_FACTOR_AUTH_PHONE_SETUP, result.to_h)
 

app/controllers/users/phones_controller.rb

@@ -6,11 +6,11 @@ class PhonesController < ReauthnRequiredController
 
     def add
       user_session[:phone_id] = nil
-      @user_phone_form = UserPhoneForm.new(current_user, nil)
+      @user_phone_form = UserPhoneForm.new(current_user)
     end
 
     def create
-      @user_phone_form = UserPhoneForm.new(current_user, nil)
+      @user_phone_form = UserPhoneForm.new(current_user)
       if @user_phone_form.submit(user_params).success?
         confirm_phone
         bypass_sign_in current_user
@@ -22,15 +22,16 @@ def create
     def edit
       set_phone_id
       # memoized for view
-      user_phone_form
+      @edit_phone_form = EditPhoneForm.new(current_user, phone_configuration)
     end
 
     def update
-      if user_phone_form.submit(user_params).success? && !already_has_phone?
+      @edit_phone_form = EditPhoneForm.new(current_user, phone_configuration)
+      if @edit_phone_form.submit(user_params).success?
         process_updates
         bypass_sign_in current_user
       else
-        render_edit
+        render :edit
       end
     end
 
@@ -50,10 +51,6 @@ def delete
 
     private
 
-    def user_phone_form
-      @user_phone_form ||= UserPhoneForm.new(current_user, phone_configuration)
-    end
-
     def render_edit
       flash.now[:error] = t('errors.messages.phone_duplicate') if already_has_phone?
       render :edit
@@ -81,8 +78,7 @@ def delivery_preference
     end
 
     def process_updates
-      form = @user_phone_form
-      if form.phone_config_changed?
+      if @edit_phone_form.phone_config_changed?
         analytics.track_event(Analytics::PHONE_CHANGE_REQUESTED)
 
         OtpPreferenceUpdater.new(

app/forms/edit_phone_form.rb

@@ -0,0 +1,66 @@
+class EditPhoneForm
+  include ActiveModel::Model
+  include RememberDeviceConcern
+
+  validates :otp_delivery_preference, inclusion: { in: %w[voice sms] }
+
+  attr_accessor :phone, :otp_delivery_preference, :otp_make_default_number, :phone_configuration
+
+  def initialize(user, phone_configuration)
+    self.user = user
+    self.phone_configuration = phone_configuration
+    self.otp_make_default_number = true if default_phone_configuration?
+  end
+
+  def submit(params)
+    ingest_submitted_params(params)
+    success = valid?
+
+    self.phone = submitted_phone unless success
+    revoke_remember_device(user) if success
+    FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes)
+  end
+
+  def delivery_preference_sms?
+    phone_configuration&.delivery_preference == 'sms'
+  end
+
+  def delivery_preference_voice?
+    phone_configuration&.delivery_preference == 'voice'
+  end
+
+  def phone_config_changed?
+    return true if phone_configuration&.delivery_preference != otp_delivery_preference
+    return true if otp_make_default_number && !default_phone_configuration?
+    false
+  end
+
+  # :reek:FeatureEnvy
+  def masked_number
+    phone_number = phone_configuration.phone
+    return '' if !phone_number || phone_number.blank?
+    "***-***-#{phone_number[-4..-1]}"
+  end
+
+  private
+
+  attr_accessor :user, :submitted_phone
+
+  def extra_analytics_attributes
+    {
+      otp_delivery_preference: otp_delivery_preference,
+    }
+  end
+
+  def ingest_submitted_params(params)
+    delivery_prefs = params[:otp_delivery_preference]
+    default_prefs = params[:otp_make_default_number]
+
+    self.otp_delivery_preference = delivery_prefs if delivery_prefs
+    self.otp_make_default_number = true if default_prefs
+  end
+
+  def default_phone_configuration?
+    phone_configuration == user.default_phone_configuration
+  end
+end

app/forms/user_phone_form.rb

@@ -7,17 +7,12 @@ class UserPhoneForm
   validates :otp_delivery_preference, inclusion: { in: %w[voice sms] }
 
   attr_accessor :phone, :international_code, :otp_delivery_preference,
-                :otp_make_default_number, :phone_configuration
+                :otp_make_default_number
 
-  def initialize(user, phone_configuration)
+  def initialize(user)
     self.user = user
-    self.phone_configuration = phone_configuration
-    if phone_configuration.nil?
-      self.otp_delivery_preference = user.otp_delivery_preference
-    else
-      prefill_phone_number(phone_configuration)
-    end
-    self.otp_make_default_number = true if default_phone_configuration?
+    self.otp_delivery_preference = user.otp_delivery_preference
+    self.otp_make_default_number = false
   end
 
   def submit(params)
@@ -32,23 +27,15 @@ def submit(params)
   end
 
   def delivery_preference_sms?
-    return true if phone_configuration.blank?
-    phone_configuration&.delivery_preference == 'sms'
+    true
   end
 
   def delivery_preference_voice?
-    phone_configuration&.delivery_preference == 'voice'
+    false
   end
 
   def already_has_phone?
-    formatted_user_phone != phone && user.phone_configurations.map(&:phone).include?(phone)
-  end
-
-  def phone_config_changed?
-    return true if formatted_user_phone != phone
-    return true if phone_configuration&.delivery_preference != otp_delivery_preference
-    return true if otp_make_default_number && !default_phone_configuration?
-    false
+    user.phone_configurations.map(&:phone).include?(phone)
   end
 
   # :reek:FeatureEnvy
@@ -92,12 +79,4 @@ def ingest_submitted_params(params)
     self.otp_delivery_preference = delivery_prefs if delivery_prefs
     self.otp_make_default_number = true if default_prefs
   end
-
-  def default_phone_configuration?
-    phone_configuration == user.default_phone_configuration
-  end
-
-  def formatted_user_phone
-    phone_configuration&.formatted_phone
-  end
 end

app/views/users/phone_setup/index.html.erb

@@ -12,10 +12,22 @@
   <%= t('two_factor_authentication.phone_fee_disclosure') %>
 </p>
 
-<%= render 'users/shared/phone_form',
-    http_method: :patch,
-    target_url: phone_setup_path,
-    button: t('forms.buttons.send_security_code') %>
+<%= simple_form_for(@user_phone_form,
+                    html: { autocomplete: 'off', method: :patch, role: 'form' },
+                    data: { international_phone_form: true },
+                    url: phone_setup_path) do |f| %>
+
+    <%= render 'users/shared/phone_number_edit', f: f %>
+
+  <%= render 'users/shared/otp_delivery_preference_selection',
+             form_obj: @user_phone_form%>
+  <% if TwoFactorAuthentication::PhonePolicy.new(current_user).enabled? %>
+    <%= render 'users/shared/otp_make_default_number',
+               form_obj: @user_phone_form%>
+  <% end %>
+  <%= f.button :submit, t('forms.buttons.send_security_code'), class: 'no-auto-enable btn-wide' %>
+<% end %>
+
 
 <div class="mt2 pt1 border-top">
   <%= link_to t('two_factor_authentication.choose_another_option'), two_factor_options_path %>

app/views/users/phones/add.html.erb

@@ -10,10 +10,21 @@
   <%= t('two_factor_authentication.phone_fee_disclosure') %>
 </p>
 
-<%= render 'users/shared/phone_form',
-    http_method: :post,
-    target_url: add_phone_path,
-    button: t('forms.buttons.continue') %>
+<%= simple_form_for(@user_phone_form,
+                    html: {autocomplete: 'off', method: :post, role: 'form'},
+                    data: {international_phone_form: true},
+                    url: add_phone_path) do |f| %>
+
+  <%= render 'users/shared/phone_number_edit', f: f %>
+
+  <%= render 'users/shared/otp_delivery_preference_selection',
+             form_obj: @user_phone_form %>
+  <% if TwoFactorAuthentication::PhonePolicy.new(current_user).enabled? %>
+    <%= render 'users/shared/otp_make_default_number',
+               form_obj: @user_phone_form %>
+  <% end %>
+  <%= f.button :submit, t('forms.buttons.continue'), class: 'no-auto-enable btn-wide' %>
+<% end %>
 
 <%= render 'shared/cancel', link: account_path %>
 

app/views/users/phones/edit.html.erb

@@ -1,21 +1,36 @@
-<% title t('titles.edit_info.phone') %><h1 class="h3 my0">
+<% title t('titles.edit_info.phone') %>
+<h1 class="h3 my0">
   <%= t('headings.edit_info.phone') %>
+</h1>
 
-</h1><%= render 'users/shared/phone_form',
-        http_method: :put,
-        target_url: manage_phone_path,
-        button: t('forms.buttons.submit.confirm_change') %>
+<%= simple_form_for(@edit_phone_form,
+                    html: {autocomplete: 'off', method: :put, role: 'form'},
+                    data: {international_phone_form: true},
+                    url: manage_phone_path) do |f| %>
 
-<% if MfaPolicy.new(current_user).more_than_two_factors_enabled? && @user_phone_form.phone.present? %>
-  <br />
+  <div class="mb1 h4">
+    <%= t('two_factor_authentication.phone_label') %>:&nbsp;
+    <strong><%= @edit_phone_form.masked_number %></strong>
+  </div><br/>
+
+  <%= render 'users/shared/otp_delivery_preference_selection',
+             form_obj: @edit_phone_form %>
+  <% if TwoFactorAuthentication::PhonePolicy.new(current_user).enabled? %>
+    <%= render 'users/shared/otp_make_default_number',
+               form_obj: @edit_phone_form %>
+  <% end %>
+  <%= f.button :submit, t('forms.buttons.submit.confirm_change'), class: 'no-auto-enable btn-wide' %>
+<% end %>
+
+
+<% if MfaPolicy.new(current_user).more_than_two_factors_enabled? %>
+  <br/>
   <div class="sm-col-8 mb3">
-    <%= button_to t('forms.phone.buttons.delete'), manage_phone_url(id: params[:id]),
-        class: 'btn btn-danger btn-wide',
-        method: :delete %>
+    <%= button_to t('forms.phone.buttons.delete'), manage_phone_path(id: params[:id]),
+                  class: 'btn btn-danger btn-wide',
+                  method: :delete %>
   </div>
 <% end %>
 
 <%= render 'shared/cancel', link: account_path %>
 
-<%= stylesheet_link_tag 'intl-tel-number/intlTelInput' %>
-<%= javascript_pack_tag 'intl-tel-input' %>

app/views/users/shared/_otp_delivery_preference_selection.html.erb

@@ -12,7 +12,7 @@
     >
       <div class="radio">
         <%= radio_button_tag 'user_phone_form[otp_delivery_preference]',
-          :sms, @user_phone_form.delivery_preference_sms?,
+          :sms, form_obj.delivery_preference_sms?,
           class: :otp_delivery_preference_sms %>
         <span class="indicator"></span>
         <%= t('two_factor_authentication.otp_delivery_preference.sms') %>
@@ -24,7 +24,7 @@
     >
       <div class="radio">
         <%= radio_button_tag 'user_phone_form[otp_delivery_preference]',
-            :voice, @user_phone_form.delivery_preference_voice?,
+            :voice, form_obj.delivery_preference_voice?,
             class: :otp_delivery_preference_voice %>
         <span class="indicator"></span>
         <%= t('two_factor_authentication.otp_delivery_preference.voice') %>

app/views/users/shared/_otp_make_default_number.html.erb

@@ -9,7 +9,7 @@
     <label class="btn-border col-8">
       <div class="checkbox">
         <%= check_box_tag 'user_phone_form[otp_make_default_number]',
-            :otp_make_default_number, @user_phone_form.otp_make_default_number %>
+            :otp_make_default_number, form_obj.otp_make_default_number %>
         <span class="indicator"></span>
         <%= t('two_factor_authentication.otp_make_default_number.label') %>
       </div>

spec/controllers/users/phone_setup_controller_spec.rb

@@ -18,7 +18,7 @@
 
         expect(@analytics).to receive(:track_event).
           with(Analytics::USER_REGISTRATION_PHONE_SETUP_VISIT)
-        expect(UserPhoneForm).to receive(:new).with(user, nil)
+        expect(UserPhoneForm).to receive(:new).with(user)
 
         get :index