Skip to content
Merged
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
4 changes: 4 additions & 0 deletions app/forms/edit_phone_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ def default_phone_configuration?
phone_configuration == user.default_phone_configuration
end

def one_phone_configured?
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The EditPhoneForm class doesn't currently have great test coverage for its methods, but could we try adding some for this new method?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree 🙂

user.phone_configurations.count == 1
end

private

attr_writer :delivery_preference, :make_default_number
Expand Down
2 changes: 1 addition & 1 deletion app/views/accounts/_phone.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<div class="grid-row grid-gap-2">
<div class="grid-col-fill">
<%= PhoneFormatter.format(phone_configuration.phone) %>
<% if current_user.default_phone_configuration == phone_configuration %>
<% if current_user.default_phone_configuration == phone_configuration && MfaContext.new(current_user).phone_configurations.count > 1 %>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have an issue with phone showing default even if its just one? or do we think its not needed information.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the design, it was deemed as not needing that information

(<%= I18n.t('account.index.default') %>)
<% end %>
</div>
Expand Down
15 changes: 12 additions & 3 deletions app/views/users/edit_phone/_make_default_number.html.erb
Original file line number Diff line number Diff line change
@@ -1,16 +1,25 @@
<div class="margin-bottom-4">
<fieldset class="margin-0 padding-0 border-0">
<legend class="margin-bottom-1">
<%= t('two_factor_authentication.otp_make_default_number.title') %>
<% if @edit_phone_form.one_phone_configured? %>
<%= t('two_factor_authentication.otp_make_default_number.one_number_title') %>
<% else %>
<%= t('two_factor_authentication.otp_make_default_number.title') %>
<% end %>
</legend>
<p class="margin-top-0 margin-bottom-2" id="otp_make_default_number_instruction">
<%= t('two_factor_authentication.otp_make_default_number.instruction') %>
<p class="margin-top-0 margin-bottom-2">
<% if @edit_phone_form.one_phone_configured? %>
<%= t('two_factor_authentication.otp_make_default_number.one_number_instruction') %>
<% else %>
<%= t('two_factor_authentication.otp_make_default_number.instruction') %>
<% end %>
</p>
<%= check_box_tag(
'edit_phone_form[make_default_number]',
:otp_make_default_number,
@edit_phone_form.default_phone_configuration?,
class: 'usa-checkbox__input usa-checkbox__input--bordered',
disabled: @edit_phone_form.one_phone_configured?,
) %>
<label
class="usa-checkbox__label"
Expand Down
7 changes: 5 additions & 2 deletions config/locales/two_factor_authentication/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,12 @@ en:
voice: Phone call
voice_unsupported: We are unable to call phone numbers in %{location}.
otp_make_default_number:
instruction: Please check the box below if you want this to be the default phone
number you receive text messages or phone calls.
instruction: Make this phone number the default number where you receive text
message or phone calls.
label: Default phone number
one_number_instruction: You must have more than one phone number added to select
a default phone number.
one_number_title: This is your default number
title: Make this your default phone number?
personal_key_fallback:
question: Don’t have your personal key?
Expand Down
8 changes: 5 additions & 3 deletions config/locales/two_factor_authentication/es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,12 @@ es:
voice: Llamada telefónica
voice_unsupported: No podemos llamar a números de teléfono en %{location}.
otp_make_default_number:
instruction: Marque la casilla a continuación si desea que este sea el teléfono
predeterminado número que recibe mensajes de texto o llamadas
telefónicas.
instruction: Establece este número de teléfono como predeterminado para que
recibas en él mensajes de texto o llamadas telefónicas.
label: Número de teléfono predeterminado
one_number_instruction: Debes tener más de un número de teléfono agregado para
seleccionar un número de teléfono predeterminado.
one_number_title: Este es tu número predeterminado
title: '¿Es este tu número de teléfono predeterminado?'
personal_key_fallback:
question: '¿No tiene su clave personal?'
Expand Down
8 changes: 5 additions & 3 deletions config/locales/two_factor_authentication/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,12 @@ fr:
voice_unsupported: Il nous est impossible d’appeler des numéros de téléphone
dans le %{location}.
otp_make_default_number:
instruction: Veuillez cocher la case ci-dessous si vous voulez que ce soit le
téléphone par défaut. numéro que vous recevez des messages texte ou des
appels téléphoniques.
instruction: Faites de ce numéro de téléphone le numéro par défaut où vous
recevez des messages texte ou des appels téléphoniques.
label: Numéro de téléphone par défaut
one_number_instruction: Vous devez avoir ajouté plus d’un numéro de téléphone
pour pouvoir sélectionner un numéro de téléphone par défaut.
one_number_title: Il s’agit de votre numéro par défaut
title: Faites-en votre numéro de téléphone par défaut?
personal_key_fallback:
question: Vous n’avez pas votre clé personnelle?
Expand Down
5 changes: 5 additions & 0 deletions spec/features/phone/default_phone_selection_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@
expiration: TwoFactorAuthenticatable::DIRECT_OTP_VALID_FOR_MINUTES,
)
end

it 'does not indicate that it is the default number on the account page ' do
sign_in_before_2fa(user)
expect(page).not_to have_content t('account.index.default')
end
end

context 'when the user creates a new default phone number' do
Expand Down
14 changes: 14 additions & 0 deletions spec/features/phone/edit_phone_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,18 @@

expect(page).to have_content('The page you were looking for doesn’t exist')
end

context 'with only one phone number' do
it 'does not allow you to check default phone number if only one number is set up' do
user = create(:user, :signed_up)
phone_configuration = user.phone_configurations.first
sign_in_and_2fa_user(user)

visit(manage_phone_path(id: phone_configuration.id))
expect(page).to have_field(
t('two_factor_authentication.otp_make_default_number.label'),
disabled: true,
)
end
end
end
23 changes: 22 additions & 1 deletion spec/forms/edit_phone_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
}
end

subject { described_class.new(user, phone_configuration) }
subject { EditPhoneForm.new(user, phone_configuration) }

describe '#submit' do
context 'when delivery_preference is not voice or sms' do
Expand Down Expand Up @@ -107,4 +107,25 @@
end
end
end

describe '#one_phone_configured?' do
context 'when editing a form with only one number set up' do
let(:user) { create(:user, :signed_up) }
let(:phone_configuration) { user.phone_configurations.first }

it 'recognizes it as the only method set up' do
expect(subject.one_phone_configured?).to eq(true)
end
end

context 'when editing a form with multiple numbers set up' do
before do
create(:phone_configuration, user: user, made_default_at: Time.zone.now)
end

it 'recognizes that there are multiple phone methods set up' do
expect(user.phone_configurations.count).to eq(2)
end
end
end
end