From d0b003c1b1c600f3904ea720988065d751ec25a2 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 31 Mar 2023 14:45:20 -0400 Subject: [PATCH 01/21] LG-9216: A/B test for tabbed Sign In view changelog: Upcoming Features, Sign In Experience, Implement A/B test for tabbed Sign In view --- .../tab_navigation_component.html.erb | 14 ++++++++++ app/components/tab_navigation_component.rb | 9 +++++++ .../sign_up/registrations_controller.rb | 1 + app/controllers/users/sessions_controller.rb | 1 + app/views/devise/sessions/new.html.erb | 26 ++++++++++++++----- app/views/sign_up/registrations/new.html.erb | 15 +++++++++++ config/application.yml.default | 1 + config/initializers/ab_tests.rb | 5 ++++ config/locales/account/en.yml | 1 + config/locales/headings/en.yml | 1 + lib/identity_config.rb | 1 + 11 files changed, 69 insertions(+), 6 deletions(-) create mode 100644 app/components/tab_navigation_component.html.erb create mode 100644 app/components/tab_navigation_component.rb diff --git a/app/components/tab_navigation_component.html.erb b/app/components/tab_navigation_component.html.erb new file mode 100644 index 00000000000..2f0dae1ddc9 --- /dev/null +++ b/app/components/tab_navigation_component.html.erb @@ -0,0 +1,14 @@ +<%= content_tag(:nav, aria: { label: }, **tag_options) do %> + +<% end %> diff --git a/app/components/tab_navigation_component.rb b/app/components/tab_navigation_component.rb new file mode 100644 index 00000000000..4b95a3ae27a --- /dev/null +++ b/app/components/tab_navigation_component.rb @@ -0,0 +1,9 @@ +class TabNavigationComponent < BaseComponent + attr_reader :label, :routes, :tag_options + + def initialize(label:, routes:, **tag_options) + @label = label + @routes = routes + @tag_options = tag_options + end +end diff --git a/app/controllers/sign_up/registrations_controller.rb b/app/controllers/sign_up/registrations_controller.rb index 0f26e3fb160..b1708c71dda 100644 --- a/app/controllers/sign_up/registrations_controller.rb +++ b/app/controllers/sign_up/registrations_controller.rb @@ -15,6 +15,7 @@ def new attempts_tracker: irs_attempts_api_tracker, ) analytics.user_registration_enter_email_visit + @a_b_test_bucket = AbTests::SIGN_IN.bucket(request.ip) render :new, locals: { request_id: nil }, formats: :html end diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index 65f59aa7e7c..a85d26ac4c5 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -28,6 +28,7 @@ def new @request_id = request_id_if_valid @ial = sp_session_ial @browser_is_ie11 = browser_is_ie11? + @a_b_test_bucket = AbTests::SIGN_IN.bucket(request.ip) super end diff --git a/app/views/devise/sessions/new.html.erb b/app/views/devise/sessions/new.html.erb index a719f4ba22f..8ac13580508 100644 --- a/app/views/devise/sessions/new.html.erb +++ b/app/views/devise/sessions/new.html.erb @@ -10,9 +10,23 @@ <% if decorated_session.sp_name %> <%= render 'sign_up/registrations/sp_registration_heading' %> -<% else %> +<% elsif @a_b_test_bucket == :default %> <%= render PageHeadingComponent.new.with_content(decorated_session.new_session_heading) %> <% end %> + +<% if @a_b_test_bucket == :tabbed %> + <%= render TabNavigationComponent.new( + label: t('account.login.tab_navigation'), + routes: [ + { text: t('links.next'), path: root_path }, + { text: t('links.create_account'), path: sign_up_email_path }, + ], + class: 'margin-bottom-4', + ) %> + + <%= render PageHeadingComponent.new.with_content(t('headings.sign_in_returning_users')) %> +<% end %> + <%= render 'shared/sp_alert', section: 'sign_in' %> <%= simple_form_for( @@ -35,20 +49,20 @@ field_options: { required: true }, ) %> <%= f.input :request_id, as: :hidden, input_html: { value: @request_id } %> -
- <%= f.submit t('links.next'), full_width: true, wide: false %> + <%= f.submit t('links.next'), full_width: true, wide: false %> + <% if @a_b_test_bucket == :default %>

<%= t('headings.create_account_with_sp.cta', app_name: APP_NAME) %>

<%= link_to( t('links.create_account'), sign_up_email_url(request_id: @request_id), - class: 'usa-button usa-button--big usa-button--outline usa-button--full-width margin-top-1', + class: 'usa-button usa-button--big usa-button--outline usa-button--full-width margin-bottom-105', ) %> -
+ <% end %> <% end %> <% if @ial && desktop_device? %> -
+
<%= link_to( t('account.login.piv_cac'), login_piv_cac_url, diff --git a/app/views/sign_up/registrations/new.html.erb b/app/views/sign_up/registrations/new.html.erb index c18bca98ebf..4829747a38f 100644 --- a/app/views/sign_up/registrations/new.html.erb +++ b/app/views/sign_up/registrations/new.html.erb @@ -2,6 +2,21 @@ <%= render 'shared/sp_alert', section: 'sign_up' %> +<% if @a_b_test_bucket == :tabbed %> + <% if decorated_session.sp_name %> + <%= render 'sign_up/registrations/sp_registration_heading' %> + <% end %> + + <%= render TabNavigationComponent.new( + label: t('account.login.tab_navigation'), + routes: [ + { text: t('links.next'), path: root_path }, + { text: t('links.create_account'), path: sign_up_email_path }, + ], + class: 'margin-bottom-4', + ) %> +<% end %> + <%= render PageHeadingComponent.new.with_content(t('titles.registrations.new')) %> <%= simple_form_for( diff --git a/config/application.yml.default b/config/application.yml.default index dc0d86c0267..b57e2fe9034 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -306,6 +306,7 @@ session_timeout_warning_seconds: 150 session_total_duration_timeout_in_minutes: 720 ses_configuration_set_name: '' set_remember_device_session_expiration: false +sign_in_a_b_testing: '{"default":100}' sp_handoff_bounce_max_seconds: 2 show_user_attribute_deprecation_warnings: false otp_min_attempts_remaining_warning_count: 3 diff --git a/config/initializers/ab_tests.rb b/config/initializers/ab_tests.rb index 361813ce4f3..58279e248c4 100644 --- a/config/initializers/ab_tests.rb +++ b/config/initializers/ab_tests.rb @@ -19,6 +19,11 @@ module AbTests }, ) + SIGN_IN = AbTestBucket.new( + experiment_name: 'Sign In Experience', + buckets: IdentityConfig.store.sign_in_a_b_testing, + ) + def self.in_person_cta_variant_testing_buckets buckets = Hash.new percents = IdentityConfig.store.in_person_cta_variant_testing_percents diff --git a/config/locales/account/en.yml b/config/locales/account/en.yml index db5af6e59b9..102656e5623 100644 --- a/config/locales/account/en.yml +++ b/config/locales/account/en.yml @@ -73,6 +73,7 @@ en: login: ie_not_supported: Internet Explorer 11 is no longer supported as of %{date} piv_cac: Sign in with your government employee ID + tab_navigation: Account tab navigation navigation: access_services: Access your government benefits and services from your %{app_name} account. diff --git a/config/locales/headings/en.yml b/config/locales/headings/en.yml index e5a43d89591..1e5cd7fe6e1 100644 --- a/config/locales/headings/en.yml +++ b/config/locales/headings/en.yml @@ -62,6 +62,7 @@ en: session_timeout_warning: Need more time? sign_in_with_sp: Sign in to continue to %{sp} sign_in_without_sp: Sign in + sign_in_returning_users: Sign in for returning users sp_handoff_bounced: There was a problem connecting to %{sp_name} ssn: Social Security Number state_id: State-issued ID diff --git a/lib/identity_config.rb b/lib/identity_config.rb index c25562d9e9d..8b6cbfa4aec 100644 --- a/lib/identity_config.rb +++ b/lib/identity_config.rb @@ -410,6 +410,7 @@ def self.build_store(config_map) config.add(:ses_configuration_set_name, type: :string) config.add(:set_remember_device_session_expiration, type: :boolean) config.add(:show_user_attribute_deprecation_warnings, type: :boolean) + config.add(:sign_in_a_b_testing, type: :json, options: { symbolize_names: true }) config.add(:skip_encryption_allowed_list, type: :json) config.add(:sp_handoff_bounce_max_seconds, type: :integer) config.add(:state_tracking_enabled, type: :boolean) From d57599197e4bf7c2b7285085b9932a0e3224e4b4 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 5 Apr 2023 08:29:14 -0400 Subject: [PATCH 02/21] Add specs for TabNavigationComponent --- .../tab_navigation_component_spec.rb | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 spec/components/tab_navigation_component_spec.rb diff --git a/spec/components/tab_navigation_component_spec.rb b/spec/components/tab_navigation_component_spec.rb new file mode 100644 index 00000000000..c51ee0f0f45 --- /dev/null +++ b/spec/components/tab_navigation_component_spec.rb @@ -0,0 +1,40 @@ +require 'rails_helper' + +RSpec.describe TabNavigationComponent, type: :component do + let(:label) { 'Navigation' } + let(:routes) { [{ path: '/first', text: 'First' }, { path: '/second', text: 'Second' }] } + let(:tag_options) { { label:, routes: } } + + subject(:rendered) do + render_inline TabNavigationComponent.new(**tag_options) + end + + it 'renders labelled navigation' do + expect(rendered).to have_css('nav[aria-label="Navigation"]') + expect(rendered).to have_link('First') { |link| !is_current_link?(link) } + expect(rendered).to have_link('Second') { |link| !is_current_link?(link) } + end + + context 'with tag options' do + let(:tag_options) { super().merge(data: { foo: 'bar' }) } + + it 'renders with tag options forwarded to navigation' do + expect(rendered).to have_css('nav[data-foo="bar"]') + end + end + + context 'with link for current request' do + before do + allow(request).to receive(:path).and_return('/first') + end + + it 'renders current link as highlighted' do + expect(rendered).to have_link('First') { |link| is_current_link?(link) } + expect(rendered).to have_link('Second') { |link| !is_current_link?(link) } + end + end + + def is_current_link?(link) + !link.matches_css?('.usa-button--outline') + end +end From b2d003844d9b0a6058e1482c16fbb88497a895a0 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 5 Apr 2023 08:42:00 -0400 Subject: [PATCH 03/21] AddTabNavigationComponent preview --- .../tab_navigation_component_preview.rb | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 spec/components/previews/tab_navigation_component_preview.rb diff --git a/spec/components/previews/tab_navigation_component_preview.rb b/spec/components/previews/tab_navigation_component_preview.rb new file mode 100644 index 00000000000..218be364117 --- /dev/null +++ b/spec/components/previews/tab_navigation_component_preview.rb @@ -0,0 +1,30 @@ +class TabNavigationComponentPreview < BaseComponentPreview + # @!group Preview + def default + render TabNavigationComponent.new( + label: 'Navigation', + routes: [ + { path: lookbook_path('preview'), text: 'Preview' }, + { path: lookbook_path('workbench'), text: 'Workbench' }, + ], + ) + end + # @!endgroup + + # @param label text + def workbench(label: 'Navigation') + render TabNavigationComponent.new( + label:, + routes: [ + { path: lookbook_path('preview'), text: 'Preview' }, + { path: lookbook_path('workbench'), text: 'Workbench' }, + ], + ) + end + + private + + def lookbook_path(example) + Lookbook::Engine.routes.url_helpers.lookbook_preview_path("tab_navigation/#{example}") + end +end From 91bfe4ac1ab2e40b2b77b083852e3aa751c3a1c0 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 5 Apr 2023 09:03:51 -0400 Subject: [PATCH 04/21] Add translations for headings --- app/views/devise/sessions/new.html.erb | 2 +- app/views/sign_up/registrations/new.html.erb | 6 ++++-- config/locales/headings/en.yml | 3 ++- config/locales/headings/es.yml | 2 ++ config/locales/headings/fr.yml | 2 ++ 5 files changed, 11 insertions(+), 4 deletions(-) diff --git a/app/views/devise/sessions/new.html.erb b/app/views/devise/sessions/new.html.erb index 8ac13580508..98ca1b605e6 100644 --- a/app/views/devise/sessions/new.html.erb +++ b/app/views/devise/sessions/new.html.erb @@ -24,7 +24,7 @@ class: 'margin-bottom-4', ) %> - <%= render PageHeadingComponent.new.with_content(t('headings.sign_in_returning_users')) %> + <%= render PageHeadingComponent.new.with_content(t('headings.sign_in_existing_users')) %> <% end %> <%= render 'shared/sp_alert', section: 'sign_in' %> diff --git a/app/views/sign_up/registrations/new.html.erb b/app/views/sign_up/registrations/new.html.erb index 4829747a38f..1d8d6e0b1ae 100644 --- a/app/views/sign_up/registrations/new.html.erb +++ b/app/views/sign_up/registrations/new.html.erb @@ -15,9 +15,11 @@ ], class: 'margin-bottom-4', ) %> -<% end %> -<%= render PageHeadingComponent.new.with_content(t('titles.registrations.new')) %> + <%= render PageHeadingComponent.new.with_content(t('headings.create_account_new_users')) %> +<% else %> + <%= render PageHeadingComponent.new.with_content(t('titles.registrations.new')) %> +<% end %> <%= simple_form_for( @register_user_email_form, diff --git a/config/locales/headings/en.yml b/config/locales/headings/en.yml index 1e5cd7fe6e1..9cfbc1fc43e 100644 --- a/config/locales/headings/en.yml +++ b/config/locales/headings/en.yml @@ -62,7 +62,8 @@ en: session_timeout_warning: Need more time? sign_in_with_sp: Sign in to continue to %{sp} sign_in_without_sp: Sign in - sign_in_returning_users: Sign in for returning users + sign_in_existing_users: Sign in for existing users + create_account_new_users: Create an account for new users sp_handoff_bounced: There was a problem connecting to %{sp_name} ssn: Social Security Number state_id: State-issued ID diff --git a/config/locales/headings/es.yml b/config/locales/headings/es.yml index 01e8ebd575c..38cf5e192eb 100644 --- a/config/locales/headings/es.yml +++ b/config/locales/headings/es.yml @@ -62,6 +62,8 @@ es: session_timeout_warning: '¿Necesita más tiempo?' sign_in_with_sp: Iniciar sesión para continuar con %{sp} sign_in_without_sp: Iniciar sesión + sign_in_existing_users: Iniciar sesión para usuarios existentes + create_account_new_users: Crear una cuenta para usuarios nuevos sp_handoff_bounced: Hubo un problema al conectarse a %{sp_name} ssn: Número de seguro social state_id: Documento de identidad emitido por el estado diff --git a/config/locales/headings/fr.yml b/config/locales/headings/fr.yml index 491d5073c65..8f5bdcd7462 100644 --- a/config/locales/headings/fr.yml +++ b/config/locales/headings/fr.yml @@ -65,6 +65,8 @@ fr: session_timeout_warning: Vous avez besoin de plus de temps? sign_in_with_sp: Connectez-vous pour continuer à %{sp} sign_in_without_sp: Connexion + sign_in_existing_users: S'identifier pour les utilisateurs existants + create_account_new_users: Créer un compte pour les nouveaux utilisateurs sp_handoff_bounced: Un problème est survenu lors de la connexion à %{sp_name} ssn: Numéro de sécurité sociale state_id: Carte d’identité délivrée par l’État From 4cd1d87aae5f66b55d729c68b1552e2155a81dab Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 5 Apr 2023 10:31:36 -0400 Subject: [PATCH 05/21] Rename ivar for sign in bucket See: https://github.com/18F/identity-idp/pull/8112/files#r1154971724 Co-authored-by: Zach Margolis --- app/controllers/sign_up/registrations_controller.rb | 2 +- app/views/sign_up/registrations/new.html.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/sign_up/registrations_controller.rb b/app/controllers/sign_up/registrations_controller.rb index b1708c71dda..b266e8ac648 100644 --- a/app/controllers/sign_up/registrations_controller.rb +++ b/app/controllers/sign_up/registrations_controller.rb @@ -15,7 +15,7 @@ def new attempts_tracker: irs_attempts_api_tracker, ) analytics.user_registration_enter_email_visit - @a_b_test_bucket = AbTests::SIGN_IN.bucket(request.ip) + @sign_in_a_b_test_bucket = AbTests::SIGN_IN.bucket(request.ip) render :new, locals: { request_id: nil }, formats: :html end diff --git a/app/views/sign_up/registrations/new.html.erb b/app/views/sign_up/registrations/new.html.erb index 1d8d6e0b1ae..5111188dfde 100644 --- a/app/views/sign_up/registrations/new.html.erb +++ b/app/views/sign_up/registrations/new.html.erb @@ -2,7 +2,7 @@ <%= render 'shared/sp_alert', section: 'sign_up' %> -<% if @a_b_test_bucket == :tabbed %> +<% if @sign_in_a_b_test_bucket == :tabbed %> <% if decorated_session.sp_name %> <%= render 'sign_up/registrations/sp_registration_heading' %> <% end %> From ca21ca93500fd0e93bbce8dc9ce3b7c6a3180149 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 5 Apr 2023 10:36:49 -0400 Subject: [PATCH 06/21] Switch to session ID for bucket discriminator May be more reliable than IP if we're expecting to log values in later events --- app/controllers/sign_up/registrations_controller.rb | 2 +- app/controllers/users/sessions_controller.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/sign_up/registrations_controller.rb b/app/controllers/sign_up/registrations_controller.rb index b266e8ac648..9d0eb9e3316 100644 --- a/app/controllers/sign_up/registrations_controller.rb +++ b/app/controllers/sign_up/registrations_controller.rb @@ -15,7 +15,7 @@ def new attempts_tracker: irs_attempts_api_tracker, ) analytics.user_registration_enter_email_visit - @sign_in_a_b_test_bucket = AbTests::SIGN_IN.bucket(request.ip) + @sign_in_a_b_test_bucket = AbTests::SIGN_IN.bucket(session.id) render :new, locals: { request_id: nil }, formats: :html end diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index a85d26ac4c5..f85806914c2 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -28,7 +28,7 @@ def new @request_id = request_id_if_valid @ial = sp_session_ial @browser_is_ie11 = browser_is_ie11? - @a_b_test_bucket = AbTests::SIGN_IN.bucket(request.ip) + @a_b_test_bucket = AbTests::SIGN_IN.bucket(session.id) super end From 4c1af2b47343257ed196834058be19a47b9ff9df Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 6 Apr 2023 08:08:08 -0400 Subject: [PATCH 07/21] Add aria-current to current link --- app/components/tab_navigation_component.html.erb | 1 + spec/components/tab_navigation_component_spec.rb | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/components/tab_navigation_component.html.erb b/app/components/tab_navigation_component.html.erb index 2f0dae1ddc9..5db1683ddcd 100644 --- a/app/components/tab_navigation_component.html.erb +++ b/app/components/tab_navigation_component.html.erb @@ -6,6 +6,7 @@ action: ->(**tag_options, &block) { link_to(route[:path], **tag_options, &block) }, big: true, outline: route[:path] != request.path, + aria: { current: route[:path] == request.path ? 'page' : nil }, class: 'grid-col', ).with_content(route[:text]) %> diff --git a/spec/components/tab_navigation_component_spec.rb b/spec/components/tab_navigation_component_spec.rb index c51ee0f0f45..33ab8a22c15 100644 --- a/spec/components/tab_navigation_component_spec.rb +++ b/spec/components/tab_navigation_component_spec.rb @@ -35,6 +35,6 @@ end def is_current_link?(link) - !link.matches_css?('.usa-button--outline') + link.matches_css?('[aria-current="page"]:not(.usa-button--outline)') end end From d393bc9245a3b3fe11c70955200326555854fa94 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 7 Apr 2023 08:45:11 -0400 Subject: [PATCH 08/21] Rename ivar for consistency See: https://github.com/18F/identity-idp/pull/8112/files#r1159978157 --- app/controllers/users/sessions_controller.rb | 2 +- app/views/devise/sessions/new.html.erb | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index f85806914c2..f411ab0e8df 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -28,7 +28,7 @@ def new @request_id = request_id_if_valid @ial = sp_session_ial @browser_is_ie11 = browser_is_ie11? - @a_b_test_bucket = AbTests::SIGN_IN.bucket(session.id) + @sign_in_a_b_test_bucket = AbTests::SIGN_IN.bucket(session.id) super end diff --git a/app/views/devise/sessions/new.html.erb b/app/views/devise/sessions/new.html.erb index 98ca1b605e6..28c4cdca957 100644 --- a/app/views/devise/sessions/new.html.erb +++ b/app/views/devise/sessions/new.html.erb @@ -10,11 +10,11 @@ <% if decorated_session.sp_name %> <%= render 'sign_up/registrations/sp_registration_heading' %> -<% elsif @a_b_test_bucket == :default %> +<% elsif @sign_in_a_b_test_bucket == :default %> <%= render PageHeadingComponent.new.with_content(decorated_session.new_session_heading) %> <% end %> -<% if @a_b_test_bucket == :tabbed %> +<% if @sign_in_a_b_test_bucket == :tabbed %> <%= render TabNavigationComponent.new( label: t('account.login.tab_navigation'), routes: [ @@ -50,7 +50,7 @@ ) %> <%= f.input :request_id, as: :hidden, input_html: { value: @request_id } %> <%= f.submit t('links.next'), full_width: true, wide: false %> - <% if @a_b_test_bucket == :default %> + <% if @sign_in_a_b_test_bucket == :default %>

<%= t('headings.create_account_with_sp.cta', app_name: APP_NAME) %>

From d554b7b5078c527efcacbfea3c513821e33f2c48 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 7 Apr 2023 08:54:18 -0400 Subject: [PATCH 09/21] Enhance TabNavigationComponent to handle query parameters --- .../tab_navigation_component.html.erb | 4 +- app/components/tab_navigation_component.rb | 6 +++ .../tab_navigation_component_spec.rb | 42 +++++++++++++++++++ 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/app/components/tab_navigation_component.html.erb b/app/components/tab_navigation_component.html.erb index 5db1683ddcd..c9a89a8598a 100644 --- a/app/components/tab_navigation_component.html.erb +++ b/app/components/tab_navigation_component.html.erb @@ -5,8 +5,8 @@ <%= render ButtonComponent.new( action: ->(**tag_options, &block) { link_to(route[:path], **tag_options, &block) }, big: true, - outline: route[:path] != request.path, - aria: { current: route[:path] == request.path ? 'page' : nil }, + outline: !is_current_path?(route[:path]), + aria: { current: is_current_path?(route[:path]) ? 'page' : nil }, class: 'grid-col', ).with_content(route[:text]) %> diff --git a/app/components/tab_navigation_component.rb b/app/components/tab_navigation_component.rb index 4b95a3ae27a..c9b78092788 100644 --- a/app/components/tab_navigation_component.rb +++ b/app/components/tab_navigation_component.rb @@ -6,4 +6,10 @@ def initialize(label:, routes:, **tag_options) @routes = routes @tag_options = tag_options end + + def is_current_path?(path) + request.path == URI.parse(path).path + rescue URI::InvalidURIError + false + end end diff --git a/spec/components/tab_navigation_component_spec.rb b/spec/components/tab_navigation_component_spec.rb index 33ab8a22c15..46225b53aa5 100644 --- a/spec/components/tab_navigation_component_spec.rb +++ b/spec/components/tab_navigation_component_spec.rb @@ -32,6 +32,48 @@ expect(rendered).to have_link('First') { |link| is_current_link?(link) } expect(rendered).to have_link('Second') { |link| !is_current_link?(link) } end + + context 'with routes defining full URL' do + let(:routes) do + [ + { path: 'https://example.com/first', text: 'First' }, + { path: 'https://example.com/second', text: 'Second' }, + ] + end + + it 'renders current link as highlighted' do + expect(rendered).to have_link('First') { |link| is_current_link?(link) } + expect(rendered).to have_link('Second') { |link| !is_current_link?(link) } + end + end + + context 'with routes including query parameters' do + let(:routes) do + [ + { path: '/first?foo=bar', text: 'First' }, + { path: '/second?foo=bar', text: 'Second' }, + ] + end + + it 'renders current link as highlighted' do + expect(rendered).to have_link('First') { |link| is_current_link?(link) } + expect(rendered).to have_link('Second') { |link| !is_current_link?(link) } + end + end + + context 'unparseable route' do + let(:routes) do + [ + { path: '😬', text: 'First' }, + { path: '😬', text: 'Second' }, + ] + end + + it 'renders gracefully without highlighted link' do + expect(rendered).to have_link('First') { |link| !is_current_link?(link) } + expect(rendered).to have_link('Second') { |link| !is_current_link?(link) } + end + end end def is_current_link?(link) From 471b4d6605b6da58e670f8688cebff77d2a70107 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 7 Apr 2023 08:56:57 -0400 Subject: [PATCH 10/21] Add request_id to tab navigation links --- app/views/devise/sessions/new.html.erb | 4 ++-- app/views/sign_up/registrations/new.html.erb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/views/devise/sessions/new.html.erb b/app/views/devise/sessions/new.html.erb index 28c4cdca957..f8e43649b44 100644 --- a/app/views/devise/sessions/new.html.erb +++ b/app/views/devise/sessions/new.html.erb @@ -18,8 +18,8 @@ <%= render TabNavigationComponent.new( label: t('account.login.tab_navigation'), routes: [ - { text: t('links.next'), path: root_path }, - { text: t('links.create_account'), path: sign_up_email_path }, + { text: t('links.next'), path: new_user_session_url(request_id: @request_id) }, + { text: t('links.create_account'), path: sign_up_email_url(request_id: @request_id) }, ], class: 'margin-bottom-4', ) %> diff --git a/app/views/sign_up/registrations/new.html.erb b/app/views/sign_up/registrations/new.html.erb index 5111188dfde..279bd9d3712 100644 --- a/app/views/sign_up/registrations/new.html.erb +++ b/app/views/sign_up/registrations/new.html.erb @@ -10,8 +10,8 @@ <%= render TabNavigationComponent.new( label: t('account.login.tab_navigation'), routes: [ - { text: t('links.next'), path: root_path }, - { text: t('links.create_account'), path: sign_up_email_path }, + { text: t('links.next'), path: new_user_session_url(request_id: sp_session[:request_id]) }, + { text: t('links.create_account'), path: sign_up_email_path(request_id: sp_session[:request_id]) }, ], class: 'margin-bottom-4', ) %> From 6e90373802f421a86eedb5e1251f8625e17a2a7a Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 7 Apr 2023 09:06:35 -0400 Subject: [PATCH 11/21] Include bucket in visit analytics --- app/controllers/sign_up/registrations_controller.rb | 4 +++- app/controllers/users/sessions_controller.rb | 9 +++++---- app/services/analytics_events.rb | 13 ++++++++++--- spec/controllers/users/sessions_controller_spec.rb | 9 +++++++-- 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/app/controllers/sign_up/registrations_controller.rb b/app/controllers/sign_up/registrations_controller.rb index 9d0eb9e3316..7ed957e8b7c 100644 --- a/app/controllers/sign_up/registrations_controller.rb +++ b/app/controllers/sign_up/registrations_controller.rb @@ -14,8 +14,10 @@ def new analytics: analytics, attempts_tracker: irs_attempts_api_tracker, ) - analytics.user_registration_enter_email_visit @sign_in_a_b_test_bucket = AbTests::SIGN_IN.bucket(session.id) + analytics.user_registration_enter_email_visit( + sign_in_a_b_test_bucket: @sign_in_a_b_test_bucket, + ) render :new, locals: { request_id: nil }, formats: :html end diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index f411ab0e8df..b059d3bd39b 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -19,16 +19,17 @@ class SessionsController < Devise::SessionsController after_action :add_csrf_token_header_to_response, only: [:keepalive] def new - analytics.sign_in_page_visit( - flash: flash[:alert], - stored_location: session['user_return_to'], - ) override_csp_for_google_analytics @request_id = request_id_if_valid @ial = sp_session_ial @browser_is_ie11 = browser_is_ie11? @sign_in_a_b_test_bucket = AbTests::SIGN_IN.bucket(session.id) + analytics.sign_in_page_visit( + flash: flash[:alert], + stored_location: session['user_return_to'], + sign_in_a_b_test_bucket: @sign_in_a_b_test_bucket, + ) super end diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 13c640ffb53..bed8cf32dfb 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -2701,12 +2701,14 @@ def session_total_duration_timeout # @param [String] flash # @param [String] stored_location + # @param [String] sign_in_a_b_test_bucket # tracks when a user visits the sign in page - def sign_in_page_visit(flash:, stored_location:, **extra) + def sign_in_page_visit(flash:, stored_location:, sign_in_a_b_test_bucket:, **extra) track_event( 'Sign in page visited', flash: flash, stored_location: stored_location, + sign_in_a_b_test_bucket:, **extra, ) end @@ -2952,8 +2954,13 @@ def webauthn_setup_visit(platform_authenticator:, errors:, enabled_mfa_methods_c end # Tracks when user visits enter email page - def user_registration_enter_email_visit - track_event('User Registration: enter email visited') + # @param [String] sign_in_a_b_test_bucket + def user_registration_enter_email_visit(sign_in_a_b_test_bucket:, **extra) + track_event( + 'User Registration: enter email visited', + sign_in_a_b_test_bucket:, + **extra, + ) end # @param [Integer] enabled_mfa_methods_count diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb index 6bcf286f36e..dbf93a55e55 100644 --- a/spec/controllers/users/sessions_controller_spec.rb +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -590,10 +590,15 @@ it 'tracks page visit, any alert flashes, and the Devise stored location' do stub_analytics allow(controller).to receive(:flash).and_return(alert: 'hello') + allow(AbTests::SIGN_IN).to receive(:bucket).and_return(:default) subject.session['user_return_to'] = mock_valid_site - properties = { flash: 'hello', stored_location: mock_valid_site } - expect(@analytics).to receive(:track_event).with('Sign in page visited', properties) + expect(@analytics).to receive(:track_event).with( + 'Sign in page visited', + flash: 'hello', + stored_location: mock_valid_site, + sign_in_a_b_test_bucket: :default, + ) get :new end From 71e0dd9a5a679a65e1c26a31c703e8554709364f Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 7 Apr 2023 09:06:41 -0400 Subject: [PATCH 12/21] Include all options in config default Clearer usage --- config/application.yml.default | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/application.yml.default b/config/application.yml.default index b57e2fe9034..25a606864d5 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -306,7 +306,7 @@ session_timeout_warning_seconds: 150 session_total_duration_timeout_in_minutes: 720 ses_configuration_set_name: '' set_remember_device_session_expiration: false -sign_in_a_b_testing: '{"default":100}' +sign_in_a_b_testing: '{"default":100,"tabbed":0}' sp_handoff_bounce_max_seconds: 2 show_user_attribute_deprecation_warnings: false otp_min_attempts_remaining_warning_count: 3 From 70df1b48bc51db6ef28a4a2c9a464e1dd9398865 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 7 Apr 2023 09:13:02 -0400 Subject: [PATCH 13/21] Track registration visit from homepage --- .../sign_up/registrations_controller.rb | 1 + app/services/analytics_events.rb | 4 ++- app/views/devise/sessions/new.html.erb | 4 +-- .../sign_up/registrations_controller_spec.rb | 27 +++++++++++++++++++ 4 files changed, 33 insertions(+), 3 deletions(-) diff --git a/app/controllers/sign_up/registrations_controller.rb b/app/controllers/sign_up/registrations_controller.rb index 7ed957e8b7c..c65c017c5b6 100644 --- a/app/controllers/sign_up/registrations_controller.rb +++ b/app/controllers/sign_up/registrations_controller.rb @@ -17,6 +17,7 @@ def new @sign_in_a_b_test_bucket = AbTests::SIGN_IN.bucket(session.id) analytics.user_registration_enter_email_visit( sign_in_a_b_test_bucket: @sign_in_a_b_test_bucket, + from_sign_in: params[:source] == 'sign_in', ) render :new, locals: { request_id: nil }, formats: :html end diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index bed8cf32dfb..3a5e033e8aa 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -2955,10 +2955,12 @@ def webauthn_setup_visit(platform_authenticator:, errors:, enabled_mfa_methods_c # Tracks when user visits enter email page # @param [String] sign_in_a_b_test_bucket - def user_registration_enter_email_visit(sign_in_a_b_test_bucket:, **extra) + # @param [Boolean] from_sign_in + def user_registration_enter_email_visit(sign_in_a_b_test_bucket:, from_sign_in:, **extra) track_event( 'User Registration: enter email visited', sign_in_a_b_test_bucket:, + from_sign_in:, **extra, ) end diff --git a/app/views/devise/sessions/new.html.erb b/app/views/devise/sessions/new.html.erb index f8e43649b44..00525d3702f 100644 --- a/app/views/devise/sessions/new.html.erb +++ b/app/views/devise/sessions/new.html.erb @@ -19,7 +19,7 @@ label: t('account.login.tab_navigation'), routes: [ { text: t('links.next'), path: new_user_session_url(request_id: @request_id) }, - { text: t('links.create_account'), path: sign_up_email_url(request_id: @request_id) }, + { text: t('links.create_account'), path: sign_up_email_url(request_id: @request_id, source: :sign_in) }, ], class: 'margin-bottom-4', ) %> @@ -56,7 +56,7 @@ <%= link_to( t('links.create_account'), - sign_up_email_url(request_id: @request_id), + sign_up_email_url(request_id: @request_id, source: :sign_in), class: 'usa-button usa-button--big usa-button--outline usa-button--full-width margin-bottom-105', ) %> <% end %> diff --git a/spec/controllers/sign_up/registrations_controller_spec.rb b/spec/controllers/sign_up/registrations_controller_spec.rb index 6f3511ad465..434bedb846a 100644 --- a/spec/controllers/sign_up/registrations_controller_spec.rb +++ b/spec/controllers/sign_up/registrations_controller_spec.rb @@ -26,6 +26,33 @@ to raise_error(Mime::Type::InvalidMimeType) end + it 'tracks visit event' do + stub_analytics + allow(AbTests::SIGN_IN).to receive(:bucket).and_return(:default) + + allow(@analytics).to receive(:track_event).with( + 'User Registration: enter email visited', + sign_in_a_b_test_bucket: :default, + from_sign_in: false, + ) + + get :new + end + + context 'with source parameter' do + it 'tracks visit event' do + stub_analytics + allow(AbTests::SIGN_IN).to receive(:bucket).and_return(:default) + + allow(@analytics).to receive(:track_event).with( + 'User Registration: enter email visited', + sign_in_a_b_test_bucket: :default, + from_sign_in: true, + ) + + get :new, params: { source: :sign_in } + end + context 'IdV unavailable' do before do allow(IdentityConfig.store).to receive(:idv_available).and_return(false) From 2b7be12cfc57b444120121ff9fdbbd862af6d24f Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 7 Apr 2023 09:32:33 -0400 Subject: [PATCH 14/21] Add analytics property for completions event --- .../concerns/sign_in_a_b_test_concern.rb | 5 +++ .../sign_up/completions_controller.rb | 6 ++- .../sign_up/registrations_controller.rb | 3 +- app/controllers/users/sessions_controller.rb | 3 +- app/services/analytics_events.rb | 3 ++ .../concerns/sign_in_a_b_test_concern_spec.rb | 38 +++++++++++++++++++ .../sign_up/completions_controller_spec.rb | 3 ++ .../sign_up/registrations_controller_spec.rb | 8 ++-- .../users/sessions_controller_spec.rb | 2 +- 9 files changed, 63 insertions(+), 8 deletions(-) create mode 100644 app/controllers/concerns/sign_in_a_b_test_concern.rb create mode 100644 spec/controllers/concerns/sign_in_a_b_test_concern_spec.rb diff --git a/app/controllers/concerns/sign_in_a_b_test_concern.rb b/app/controllers/concerns/sign_in_a_b_test_concern.rb new file mode 100644 index 00000000000..01db563b03a --- /dev/null +++ b/app/controllers/concerns/sign_in_a_b_test_concern.rb @@ -0,0 +1,5 @@ +module SignInABTestConcern + def sign_in_a_b_test_bucket + AbTests::SIGN_IN.bucket(sp_session[:request_id] || session.id) + end +end diff --git a/app/controllers/sign_up/completions_controller.rb b/app/controllers/sign_up/completions_controller.rb index 5cb9cb4438f..d8d47742c5c 100644 --- a/app/controllers/sign_up/completions_controller.rb +++ b/app/controllers/sign_up/completions_controller.rb @@ -1,6 +1,7 @@ module SignUp class CompletionsController < ApplicationController include SecureHeadersConcern + include SignInABTestConcern before_action :confirm_two_factor_authenticated before_action :verify_confirmed, if: :ial2? @@ -84,7 +85,10 @@ def analytics_attributes(page_occurence) end def track_completion_event(last_page) - analytics.user_registration_complete(**analytics_attributes(last_page)) + analytics.user_registration_complete( + **analytics_attributes(last_page), + sign_in_a_b_test_bucket:, + ) end def pii diff --git a/app/controllers/sign_up/registrations_controller.rb b/app/controllers/sign_up/registrations_controller.rb index c65c017c5b6..b56270d8e12 100644 --- a/app/controllers/sign_up/registrations_controller.rb +++ b/app/controllers/sign_up/registrations_controller.rb @@ -2,6 +2,7 @@ module SignUp class RegistrationsController < ApplicationController include PhoneConfirmation include ApplicationHelper # for ial2_requested? + include SignInABTestConcern before_action :confirm_two_factor_authenticated, only: [:destroy_confirm] before_action :require_no_authentication @@ -14,7 +15,7 @@ def new analytics: analytics, attempts_tracker: irs_attempts_api_tracker, ) - @sign_in_a_b_test_bucket = AbTests::SIGN_IN.bucket(session.id) + @sign_in_a_b_test_bucket = sign_in_a_b_test_bucket analytics.user_registration_enter_email_visit( sign_in_a_b_test_bucket: @sign_in_a_b_test_bucket, from_sign_in: params[:source] == 'sign_in', diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index b059d3bd39b..1cc09669b83 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -7,6 +7,7 @@ class SessionsController < Devise::SessionsController include RememberDeviceConcern include Ial2ProfileConcern include Api::CsrfTokenConcern + include SignInABTestConcern rescue_from ActionController::InvalidAuthenticityToken, with: :redirect_to_signin @@ -24,7 +25,7 @@ def new @request_id = request_id_if_valid @ial = sp_session_ial @browser_is_ie11 = browser_is_ie11? - @sign_in_a_b_test_bucket = AbTests::SIGN_IN.bucket(session.id) + @sign_in_a_b_test_bucket = sign_in_a_b_test_bucket analytics.sign_in_page_visit( flash: flash[:alert], stored_location: session['user_return_to'], diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 3a5e033e8aa..b9901011db7 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -2991,6 +2991,7 @@ def user_registration_cancellation(request_came_from:, **extra) # @param [String] service_provider_name # @param [String] page_occurence # @param [String] needs_completion_screen_reason + # @param [String] sign_in_a_b_test_bucket # @param [Array] sp_request_requested_attributes # @param [Array] sp_session_requested_attributes def user_registration_complete( @@ -2998,6 +2999,7 @@ def user_registration_complete( service_provider_name:, page_occurence:, needs_completion_screen_reason:, + sign_in_a_b_test_bucket:, sp_session_requested_attributes:, sp_request_requested_attributes: nil, ialmax: nil, @@ -3010,6 +3012,7 @@ def user_registration_complete( service_provider_name: service_provider_name, page_occurence: page_occurence, needs_completion_screen_reason: needs_completion_screen_reason, + sign_in_a_b_test_bucket:, sp_request_requested_attributes: sp_request_requested_attributes, sp_session_requested_attributes: sp_session_requested_attributes, **extra, diff --git a/spec/controllers/concerns/sign_in_a_b_test_concern_spec.rb b/spec/controllers/concerns/sign_in_a_b_test_concern_spec.rb new file mode 100644 index 00000000000..61acd114b21 --- /dev/null +++ b/spec/controllers/concerns/sign_in_a_b_test_concern_spec.rb @@ -0,0 +1,38 @@ +require 'rails_helper' + +RSpec.describe SignInABTestConcern, type: :controller do + controller ApplicationController do + include SignInABTestConcern + end + + describe '#sign_in_a_b_test_bucket' do + subject(:sign_in_a_b_test_bucket) { controller.sign_in_a_b_test_bucket } + + let(:sp_session) { {} } + + before do + allow(session).to receive(:id).and_return('session-id') + allow(controller).to receive(:sp_session).and_return(sp_session) + allow(AbTests::SIGN_IN).to receive(:bucket) do |discriminator| + case discriminator + when 'session-id' + :default + when 'request-id' + :tabbed + end + end + end + + it 'returns the bucket based on session id' do + expect(sign_in_a_b_test_bucket).to eq(:default) + end + + context 'with associated sp session request id' do + let(:sp_session) { { request_id: 'request-id' } } + + it 'returns the bucket based on request id' do + expect(sign_in_a_b_test_bucket).to eq(:tabbed) + end + end + end +end diff --git a/spec/controllers/sign_up/completions_controller_spec.rb b/spec/controllers/sign_up/completions_controller_spec.rb index d4bc5ca2325..a3525044fde 100644 --- a/spec/controllers/sign_up/completions_controller_spec.rb +++ b/spec/controllers/sign_up/completions_controller_spec.rb @@ -135,6 +135,7 @@ before do stub_analytics allow(@analytics).to receive(:track_event) + allow(controller).to receive(:sign_in_a_b_test_bucket).and_return(:default) @linker = instance_double(IdentityLinker) allow(@linker).to receive(:link_identity).and_return(true) allow(IdentityLinker).to receive(:new).and_return(@linker) @@ -158,6 +159,7 @@ service_provider_name: subject.decorated_session.sp_name, page_occurence: 'agency-page', needs_completion_screen_reason: :new_sp, + sign_in_a_b_test_bucket: :default, sp_request_requested_attributes: nil, sp_session_requested_attributes: nil, ) @@ -217,6 +219,7 @@ service_provider_name: subject.decorated_session.sp_name, page_occurence: 'agency-page', needs_completion_screen_reason: :new_sp, + sign_in_a_b_test_bucket: :default, sp_request_requested_attributes: nil, sp_session_requested_attributes: ['email'], ) diff --git a/spec/controllers/sign_up/registrations_controller_spec.rb b/spec/controllers/sign_up/registrations_controller_spec.rb index 434bedb846a..24d7512ba94 100644 --- a/spec/controllers/sign_up/registrations_controller_spec.rb +++ b/spec/controllers/sign_up/registrations_controller_spec.rb @@ -28,9 +28,9 @@ it 'tracks visit event' do stub_analytics - allow(AbTests::SIGN_IN).to receive(:bucket).and_return(:default) + allow(controller).to receive(:sign_in_a_b_test_bucket).and_return(:default) - allow(@analytics).to receive(:track_event).with( + expect(@analytics).to receive(:track_event).with( 'User Registration: enter email visited', sign_in_a_b_test_bucket: :default, from_sign_in: false, @@ -42,9 +42,9 @@ context 'with source parameter' do it 'tracks visit event' do stub_analytics - allow(AbTests::SIGN_IN).to receive(:bucket).and_return(:default) + allow(controller).to receive(:sign_in_a_b_test_bucket).and_return(:default) - allow(@analytics).to receive(:track_event).with( + expect(@analytics).to receive(:track_event).with( 'User Registration: enter email visited', sign_in_a_b_test_bucket: :default, from_sign_in: true, diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb index dbf93a55e55..ff494bf471d 100644 --- a/spec/controllers/users/sessions_controller_spec.rb +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -590,7 +590,7 @@ it 'tracks page visit, any alert flashes, and the Devise stored location' do stub_analytics allow(controller).to receive(:flash).and_return(alert: 'hello') - allow(AbTests::SIGN_IN).to receive(:bucket).and_return(:default) + allow(controller).to receive(:sign_in_a_b_test_bucket).and_return(:default) subject.session['user_return_to'] = mock_valid_site expect(@analytics).to receive(:track_event).with( From 53add70414875eaa7787e70e59f03bba0c4e3c70 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 7 Apr 2023 09:34:23 -0400 Subject: [PATCH 15/21] Fix new session view link parameter assertion --- spec/views/devise/sessions/new.html.erb_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/views/devise/sessions/new.html.erb_spec.rb b/spec/views/devise/sessions/new.html.erb_spec.rb index c67741e89df..b6fa1f94db1 100644 --- a/spec/views/devise/sessions/new.html.erb_spec.rb +++ b/spec/views/devise/sessions/new.html.erb_spec.rb @@ -41,7 +41,7 @@ expect(rendered). to have_link( - t('links.create_account'), href: sign_up_email_url(request_id: nil) + t('links.create_account'), href: sign_up_email_url(request_id: nil, source: :sign_in) ) end From 2add84b1e9b93d4dca2b63e1da53cb841b5df6a9 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 7 Apr 2023 09:38:27 -0400 Subject: [PATCH 16/21] Add specs for devise/sessions/new.html.erb --- .../devise/sessions/new.html.erb_spec.rb | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/spec/views/devise/sessions/new.html.erb_spec.rb b/spec/views/devise/sessions/new.html.erb_spec.rb index b6fa1f94db1..c897c072fd6 100644 --- a/spec/views/devise/sessions/new.html.erb_spec.rb +++ b/spec/views/devise/sessions/new.html.erb_spec.rb @@ -1,6 +1,8 @@ require 'rails_helper' describe 'devise/sessions/new.html.erb' do + let(:sign_in_a_b_test_bucket) { :default } + before do allow(view).to receive(:resource).and_return(build_stubbed(:user)) allow(view).to receive(:resource_name).and_return(:user) @@ -9,6 +11,7 @@ allow(view).to receive(:decorated_session).and_return(SessionDecorator.new) allow_any_instance_of(ActionController::TestRequest).to receive(:path). and_return('/') + @sign_in_a_b_test_bucket = sign_in_a_b_test_bucket assign(:ial, 1) end @@ -30,7 +33,7 @@ render end - it 'includes a link to log in' do + it 'has a page heading' do render expect(rendered).to have_content(t('headings.sign_in_without_sp')) @@ -170,4 +173,23 @@ expect(rendered).to have_selector('input.email') end end + + context 'with tabbed layout A/B test' do + let(:sign_in_a_b_test_bucket) { :tabbed } + + it 'has a page heading' do + render + + expect(rendered).to have_content(t('headings.sign_in_existing_users')) + end + + it 'includes a link to create a new account' do + render + + expect(rendered).to have_link( + t('links.create_account'), + href: sign_up_email_url(request_id: nil, source: :sign_in), + ) + end + end end From 256fa162d91002705204ce73637b900ebeb8f3fa Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 7 Apr 2023 09:41:44 -0400 Subject: [PATCH 17/21] Add specs for sign_up/registrations/new.html.erb --- .../devise/sessions/new.html.erb_spec.rb | 8 +++--- .../registrations/new.html.erb_spec.rb | 25 ++++++++++++++++++- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/spec/views/devise/sessions/new.html.erb_spec.rb b/spec/views/devise/sessions/new.html.erb_spec.rb index c897c072fd6..e9f5c5c3854 100644 --- a/spec/views/devise/sessions/new.html.erb_spec.rb +++ b/spec/views/devise/sessions/new.html.erb_spec.rb @@ -33,10 +33,10 @@ render end - it 'has a page heading' do + it 'has a localized page heading' do render - expect(rendered).to have_content(t('headings.sign_in_without_sp')) + expect(rendered).to have_selector('h1', text: t('headings.sign_in_without_sp')) end it 'includes a link to create a new account' do @@ -177,10 +177,10 @@ context 'with tabbed layout A/B test' do let(:sign_in_a_b_test_bucket) { :tabbed } - it 'has a page heading' do + it 'has a localized page heading' do render - expect(rendered).to have_content(t('headings.sign_in_existing_users')) + expect(rendered).to have_selector('h1', text: t('headings.sign_in_existing_users')) end it 'includes a link to create a new account' do diff --git a/spec/views/sign_up/registrations/new.html.erb_spec.rb b/spec/views/sign_up/registrations/new.html.erb_spec.rb index de563bc00a1..b86a300aebd 100644 --- a/spec/views/sign_up/registrations/new.html.erb_spec.rb +++ b/spec/views/sign_up/registrations/new.html.erb_spec.rb @@ -1,6 +1,8 @@ require 'rails_helper' describe 'sign_up/registrations/new.html.erb' do + let(:sign_in_a_b_test_bucket) { :default } + let(:sp) do build_stubbed( :service_provider, @@ -8,12 +10,14 @@ return_to_sp_url: 'www.awesomeness.com', ) end + before do allow(view).to receive(:current_user).and_return(nil) @register_user_email_form = RegisterUserEmailForm.new( analytics: FakeAnalytics.new, attempts_tracker: IrsAttemptsApiTrackingHelper::FakeAttemptsTracker.new, ) + @sign_in_a_b_test_bucket = sign_in_a_b_test_bucket view_context = ActionController::Base.new.view_context allow(view_context).to receive(:new_user_session_url). and_return('https://www.example.com/') @@ -37,7 +41,7 @@ render end - it 'has a localized header' do + it 'has a localized page heading' do render expect(rendered).to have_selector('h1', text: t('titles.registrations.new')) @@ -75,4 +79,23 @@ [target='_blank'][rel='noopener noreferrer']", ) end + + context 'with tabbed layout A/B test' do + let(:sign_in_a_b_test_bucket) { :tabbed } + + it 'has a localized page heading' do + render + + expect(rendered).to have_selector('h1', text: t('headings.create_account_new_users')) + end + + it 'includes a link to sign in' do + render + + expect(rendered).to have_link( + t('links.next'), + href: new_user_session_url(request_id: nil), + ) + end + end end From 146640880c433dbee19ce79f609f3a1f58b61cbc Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 7 Apr 2023 09:42:36 -0400 Subject: [PATCH 18/21] Normalize YAML --- config/locales/headings/en.yml | 4 ++-- config/locales/headings/es.yml | 4 ++-- config/locales/headings/fr.yml | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/config/locales/headings/en.yml b/config/locales/headings/en.yml index 9cfbc1fc43e..767d44b2d05 100644 --- a/config/locales/headings/en.yml +++ b/config/locales/headings/en.yml @@ -22,6 +22,7 @@ en: prompt: Are you sure you want to cancel? confirmations: new: Send another confirmation email + create_account_new_users: Create an account for new users create_account_with_sp: cta: First time using %{app_name}? sp_text: is using %{app_name} to allow you to sign in to your account safely and @@ -60,10 +61,9 @@ en: new: Use your PIV/CAC card to secure your account residential_address: Current residential address session_timeout_warning: Need more time? + sign_in_existing_users: Sign in for existing users sign_in_with_sp: Sign in to continue to %{sp} sign_in_without_sp: Sign in - sign_in_existing_users: Sign in for existing users - create_account_new_users: Create an account for new users sp_handoff_bounced: There was a problem connecting to %{sp_name} ssn: Social Security Number state_id: State-issued ID diff --git a/config/locales/headings/es.yml b/config/locales/headings/es.yml index 38cf5e192eb..e46608025c5 100644 --- a/config/locales/headings/es.yml +++ b/config/locales/headings/es.yml @@ -22,6 +22,7 @@ es: prompt: '¿Estas seguro que quieres cancelar?' confirmations: new: Enviar otro email de confirmación + create_account_new_users: Crear una cuenta para usuarios nuevos create_account_with_sp: cta: '¿Es la primera vez que utiliza %{app_name}?' sp_text: está utilizando %{app_name} para permitirle iniciar sesión en su cuenta @@ -60,10 +61,9 @@ es: new: Use su tarjeta PIV/CAC para asegurar su cuenta residential_address: Dirección residencial actual session_timeout_warning: '¿Necesita más tiempo?' + sign_in_existing_users: Iniciar sesión para usuarios existentes sign_in_with_sp: Iniciar sesión para continuar con %{sp} sign_in_without_sp: Iniciar sesión - sign_in_existing_users: Iniciar sesión para usuarios existentes - create_account_new_users: Crear una cuenta para usuarios nuevos sp_handoff_bounced: Hubo un problema al conectarse a %{sp_name} ssn: Número de seguro social state_id: Documento de identidad emitido por el estado diff --git a/config/locales/headings/fr.yml b/config/locales/headings/fr.yml index 8f5bdcd7462..bbb66734b05 100644 --- a/config/locales/headings/fr.yml +++ b/config/locales/headings/fr.yml @@ -22,6 +22,7 @@ fr: prompt: Es-tu sûre de vouloir annuler? confirmations: new: Envoyer un autre courriel de confirmation + create_account_new_users: Créer un compte pour les nouveaux utilisateurs create_account_with_sp: cta: Première fois que vous utilisez %{app_name}? sp_text: utilise %{app_name} pour vous permettre de vous connecter à votre @@ -63,10 +64,9 @@ fr: new: Utilisez votre carte PIV/CAC pour sécuriser votre compte residential_address: Adresse de résidence actuelle session_timeout_warning: Vous avez besoin de plus de temps? + sign_in_existing_users: S’identifier pour les utilisateurs existants sign_in_with_sp: Connectez-vous pour continuer à %{sp} sign_in_without_sp: Connexion - sign_in_existing_users: S'identifier pour les utilisateurs existants - create_account_new_users: Créer un compte pour les nouveaux utilisateurs sp_handoff_bounced: Un problème est survenu lors de la connexion à %{sp_name} ssn: Numéro de sécurité sociale state_id: Carte d’identité délivrée par l’État From 49e299362d6fa31179dbb713a0e687e045d21aac Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 7 Apr 2023 12:57:55 -0400 Subject: [PATCH 19/21] Fix specs to check for new parameter --- spec/support/features/session_helper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/support/features/session_helper.rb b/spec/support/features/session_helper.rb index b1e4d6e8b47..2db90c3916d 100644 --- a/spec/support/features/session_helper.rb +++ b/spec/support/features/session_helper.rb @@ -390,11 +390,11 @@ def sign_up_user_from_sp_without_confirming_email(email) click_sign_in_from_landing_page_then_click_create_account - expect(current_url).to eq sign_up_email_url(request_id: sp_request_id) + expect(current_url).to eq sign_up_email_url(request_id: sp_request_id, source: :sign_in) visit_landing_page_and_click_create_account_with_request_id(sp_request_id) - expect(current_url).to eq sign_up_email_url(request_id: sp_request_id) + expect(current_url).to eq sign_up_email_url(request_id: sp_request_id, source: :sign_in) expect_branded_experience submit_form_with_invalid_email From 5202870073d96381493d560361865c5b7d8c96b3 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 7 Apr 2023 13:22:08 -0400 Subject: [PATCH 20/21] Add approved label for tab navigation --- config/locales/account/en.yml | 2 +- config/locales/account/es.yml | 1 + config/locales/account/fr.yml | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/config/locales/account/en.yml b/config/locales/account/en.yml index 102656e5623..14757b71586 100644 --- a/config/locales/account/en.yml +++ b/config/locales/account/en.yml @@ -73,7 +73,7 @@ en: login: ie_not_supported: Internet Explorer 11 is no longer supported as of %{date} piv_cac: Sign in with your government employee ID - tab_navigation: Account tab navigation + tab_navigation: Account creation tabs navigation: access_services: Access your government benefits and services from your %{app_name} account. diff --git a/config/locales/account/es.yml b/config/locales/account/es.yml index 6fe8b55f70a..15da56e0a52 100644 --- a/config/locales/account/es.yml +++ b/config/locales/account/es.yml @@ -74,6 +74,7 @@ es: login: ie_not_supported: Internet Explorer 11 dejó de ser compatible a partir del %{date} piv_cac: Inicie sesión con su identificación de empleado del gobierno + tab_navigation: Pestañas de creación de cuenta navigation: access_services: Acceda a los beneficios y servicios de su gobierno desde su cuenta %{app_name}. diff --git a/config/locales/account/fr.yml b/config/locales/account/fr.yml index efa01267c15..7d00e071849 100644 --- a/config/locales/account/fr.yml +++ b/config/locales/account/fr.yml @@ -79,6 +79,7 @@ fr: login: ie_not_supported: Internet Explorer 11 n’est plus pris en charge à partir du %{date} piv_cac: Connectez-vous avec votre ID d’employé du gouvernement + tab_navigation: Onglets de création de compte navigation: access_services: Accédez à vos avantages et services gouvernementaux depuis votre compte %{app_name}. From f3422c6ed306c83ceab02e29c280553bd80ec12d Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Mon, 10 Apr 2023 08:03:41 -0400 Subject: [PATCH 21/21] Fix syntax error --- spec/controllers/sign_up/registrations_controller_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/controllers/sign_up/registrations_controller_spec.rb b/spec/controllers/sign_up/registrations_controller_spec.rb index 24d7512ba94..8a28af1c2d5 100644 --- a/spec/controllers/sign_up/registrations_controller_spec.rb +++ b/spec/controllers/sign_up/registrations_controller_spec.rb @@ -52,6 +52,7 @@ get :new, params: { source: :sign_in } end + end context 'IdV unavailable' do before do