From 16bb1583d4df1660861c0b6bf416d8333fc3cb7a Mon Sep 17 00:00:00 2001 From: "jack.coggin" Date: Mon, 27 Nov 2023 09:43:45 +0000 Subject: [PATCH 1/7] add terms and conditions page --- .../terms_and_conditions_controller.rb | 35 +++++++++++++++++++ .../registration/terms_and_conditions_form.rb | 14 ++++++++ .../terms_and_conditions/edit.html.slim | 22 ++++++++++++ 3 files changed, 71 insertions(+) create mode 100644 app/controllers/registration/terms_and_conditions_controller.rb create mode 100644 app/forms/registration/terms_and_conditions_form.rb create mode 100644 app/views/registration/terms_and_conditions/edit.html.slim diff --git a/app/controllers/registration/terms_and_conditions_controller.rb b/app/controllers/registration/terms_and_conditions_controller.rb new file mode 100644 index 000000000..79166cabf --- /dev/null +++ b/app/controllers/registration/terms_and_conditions_controller.rb @@ -0,0 +1,35 @@ +module Registration + class TermsAndConditionsController < BaseController + def edit; end + + def update + form.terms_and_conditions_agreed_at = user_params[:terms_and_conditions_agreed_at] + + if form.save + if current_user.registration_complete? + redirect_to user_path, notice: t(:details_updated) + else + redirect_to edit_registration_name_path + end + else + render :edit, status: :unprocessable_entity + end + end + + private + + # @return [Hash] + def user_params + params.require(:user).permit(:terms_and_conditions_agreed_at) + end + + # @return [Registration::NameForm] + def form + @form ||= + TermsAndConditionsForm.new( + user: current_user, + terms_and_conditions_agreed_at: current_user.terms_and_conditions_agreed_at, + ) + end + end +end diff --git a/app/forms/registration/terms_and_conditions_form.rb b/app/forms/registration/terms_and_conditions_form.rb new file mode 100644 index 000000000..e0e0c3a7a --- /dev/null +++ b/app/forms/registration/terms_and_conditions_form.rb @@ -0,0 +1,14 @@ +module Registration + class TermsAndConditionsForm < BaseForm + attr_accessor :terms_and_conditions_agreed_at + + validates :terms_and_conditions_agreed_at, presence: true + + # @return [Boolean] + def save + return false unless valid? + + user.update!(terms_and_conditions_agreed_at: terms_and_conditions_agreed_at) + end + end +end diff --git a/app/views/registration/terms_and_conditions/edit.html.slim b/app/views/registration/terms_and_conditions/edit.html.slim new file mode 100644 index 000000000..4d0e4f1a3 --- /dev/null +++ b/app/views/registration/terms_and_conditions/edit.html.slim @@ -0,0 +1,22 @@ += render 'user/debug' + +- content_for :page_title do + = html_title 'Terms and Conditions' + +.govuk-grid-row + .govuk-grid-column-two-thirds-from-desktop + = form_for form, url: registration_terms_and_conditions_path, method: :patch do |f| + = f.govuk_error_summary + + h1.govuk-heading-l = t('register_terms_and_conditions.heading') + + h3 = t('register_terms_and_conditions.subheading') + + = f.govuk_check_boxes_fieldset :terms_and_conditions_agreed_at, + legend: { class: 'govuk-visually-hidden', text: 'Terms and conditions'}, classes: 'light-grey-box' do + = m('register_terms_and_conditions.legend') + = f.terms_and_conditions_check_box + + + .govuk-button-group + = f.govuk_submit t('links.continue') From f2ead426c0950189f7323b10292c344c368927c2 Mon Sep 17 00:00:00 2001 From: "jack.coggin" Date: Mon, 27 Nov 2023 11:18:05 +0000 Subject: [PATCH 2/7] update localisations and specs --- app/controllers/application_controller.rb | 2 +- .../users/omniauth_callbacks_controller.rb | 4 ++-- app/controllers/users/sessions_controller.rb | 2 +- config/locales/en.yml | 7 +++++++ config/routes.rb | 1 + .../users/omniauth_callbacks_controller_spec.rb | 13 +++++++------ spec/lib/seed_snippets_spec.rb | 2 +- .../confirmed_user/completing_registration_spec.rb | 12 ++++++++++++ 8 files changed, 32 insertions(+), 11 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index b7ca4f15e..7b9ba514a 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -20,7 +20,7 @@ def authenticate_registered_user! authenticate_user! unless user_signed_in? return true if current_user.registration_complete? - redirect_to edit_registration_name_path, notice: 'Please complete registration' + redirect_to edit_registration_terms_and_conditions_path, notice: 'Please complete registration' end def configure_permitted_parameters diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index 2e6f0d920..2a3d189aa 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/app/controllers/users/omniauth_callbacks_controller.rb @@ -12,7 +12,7 @@ def openid_connect return error_redirect unless valid_tokens?(tokens_response) id_token = auth_service.decode_id_token(tokens_response['id_token'])[0] - session[:id_token] = tokens_response['id_token'] + session[:id_token] = id_token gov_one_id = id_token['sub'] return error_redirect unless auth_service.valid_id_token?(id_token, session[:gov_one_auth_nonce]) @@ -78,7 +78,7 @@ def after_sign_in_path_for(resource) elsif resource.private_beta_registration_complete? static_path('new-registration') else - edit_registration_name_path + edit_registration_terms_and_conditions_path end end end diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index d70bff7ee..a4db408cb 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -17,7 +17,7 @@ def after_sign_in_path_for(resource) elsif resource.private_beta_registration_complete? static_path('new-registration') else - edit_registration_name_path + edit_registration_terms_and_conditions_path end end end diff --git a/config/locales/en.yml b/config/locales/en.yml index ec06dafcc..f04bc9b73 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -449,6 +449,13 @@ en: complete_registration: Thank you for creating an Early years child development training account. You can now start your first module. update_registration: Thank you for updating your Early years child development training account. You can now continue. + # /registration/terms-and-conditions/edit + register_terms_and_conditions: + heading: Set up your training account + subheading: Agree to our terms and conditions + legend: | + To use this service, you must accept the [terms and conditions](/terms-and-conditions) and [privacy policy](/privacy-policy). + # /registration/name/edit register_name: heading: About you diff --git a/config/routes.rb b/config/routes.rb index 9de3c4b5c..ec9655633 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -37,6 +37,7 @@ end namespace :registration do + resource :terms_and_conditions, only: %i[edit update], path: 'terms-and-conditions' resource :name, only: %i[edit update] resource :setting_type, only: %i[edit update], path: 'setting-type' resource :setting_type_other, only: %i[edit update], path: 'setting-type-other' diff --git a/spec/controllers/users/omniauth_callbacks_controller_spec.rb b/spec/controllers/users/omniauth_callbacks_controller_spec.rb index 339497fa4..132d453b9 100644 --- a/spec/controllers/users/omniauth_callbacks_controller_spec.rb +++ b/spec/controllers/users/omniauth_callbacks_controller_spec.rb @@ -4,6 +4,7 @@ let(:auth_service) { instance_double(GovOneAuthService) } let(:access_token) { 'mock_access_token' } let(:id_token) { 'mock_id_token' } + let(:decoded_id_token) { { 'sub' => 'mock_sub' } } let(:email) { 'test@example.com' } let(:params) do { 'code' => 'mock_code', 'state' => 'mock_state' } @@ -16,9 +17,9 @@ allow(GovOneAuthService).to receive(:new).and_return(auth_service) allow(auth_service).to receive(:tokens).and_return({ 'access_token' => access_token, 'id_token' => id_token }) - allow(auth_service).to receive(:user_info).and_return({ 'email' => email }) + allow(auth_service).to receive(:user_info).and_return({ 'email' => email, 'sub' => 'mock_sub' }) allow(auth_service).to receive(:jwt_assertion).and_return('mock_jwt_assertion') - allow(auth_service).to receive(:decode_id_token).and_return([{ 'sub' => 'mock_sub' }]) + allow(auth_service).to receive(:decode_id_token).and_return([decoded_id_token]) allow(auth_service).to receive(:valid_id_token?).and_return(true) end @@ -33,8 +34,8 @@ end it 'redirects to complete registration' do - expect(session[:id_token]).to eq id_token - expect(response).to redirect_to edit_registration_name_path + expect(session[:id_token]).to eq decoded_id_token + expect(response).to redirect_to edit_registration_terms_and_conditions_path end end @@ -49,7 +50,7 @@ end it 'redirects to /my-modules' do - expect(session[:id_token]).to eq id_token + expect(session[:id_token]).to eq decoded_id_token expect(response).to redirect_to my_modules_path end end @@ -61,7 +62,7 @@ end it 'redirects to /my-modules' do - expect(session[:id_token]).to eq id_token + expect(session[:id_token]).to eq decoded_id_token expect(response).to redirect_to my_modules_path end end diff --git a/spec/lib/seed_snippets_spec.rb b/spec/lib/seed_snippets_spec.rb index 5041dca24..fbcf787bf 100644 --- a/spec/lib/seed_snippets_spec.rb +++ b/spec/lib/seed_snippets_spec.rb @@ -5,7 +5,7 @@ subject(:locales) { described_class.new.call } it 'converts all translations' do - expect(locales.count).to be 191 + expect(locales.count).to be 194 end it 'dot separated key -> Page::Resource#name' do diff --git a/spec/system/confirmed_user/completing_registration_spec.rb b/spec/system/confirmed_user/completing_registration_spec.rb index 420391fb0..bcd43c72b 100644 --- a/spec/system/confirmed_user/completing_registration_spec.rb +++ b/spec/system/confirmed_user/completing_registration_spec.rb @@ -6,6 +6,18 @@ let(:user) { create :user, :confirmed } it 'requires name and a setting type and email preferences and a complete' do + expect(page).to have_text('Terms and conditions') + + click_button 'Continue' + + expect(page).to have_text('There is a problem') + .and have_text('You must accept the terms and conditions and privacy policy to create an account.') + + expect(page).to have_text('Agree to our terms and conditions') + + check 'I confirm that I accept the terms and conditions and privacy policy.' + click_button 'Continue' + expect(page).to have_text('About you') click_button 'Continue' From 28f80d561d0567e17d8ef83053d82a2393bf5ab1 Mon Sep 17 00:00:00 2001 From: "jack.coggin" Date: Mon, 27 Nov 2023 17:14:57 +0000 Subject: [PATCH 3/7] update specs --- config/sitemap.rb | 1 + spec/requests/authentication_spec.rb | 4 ++-- spec/system/sign_in_spec.rb | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/config/sitemap.rb b/config/sitemap.rb index 901358c90..733b813ea 100644 --- a/config/sitemap.rb +++ b/config/sitemap.rb @@ -71,6 +71,7 @@ # edit registration/account add edit_email_user_path add edit_password_user_path + add edit_registration_terms_and_conditions_path add edit_registration_name_path add edit_registration_setting_type_path add edit_registration_setting_type_other_path diff --git a/spec/requests/authentication_spec.rb b/spec/requests/authentication_spec.rb index 66da4ccf2..d2ad5bb74 100644 --- a/spec/requests/authentication_spec.rb +++ b/spec/requests/authentication_spec.rb @@ -4,7 +4,7 @@ # RSpec.describe 'Authentication', type: :request do describe 'viewing authenticate_user! controller action' do - let(:action_path) { edit_registration_name_path } + let(:action_path) { edit_registration_terms_and_conditions_path } context 'with User not signed in' do it 'redirects to sign in page' do @@ -89,7 +89,7 @@ it 'redirects to finish registration' do get action_path - expect(response).to redirect_to(edit_registration_name_path) + expect(response).to redirect_to(edit_registration_terms_and_conditions_path) end it 'displays message to complete registration' do diff --git a/spec/system/sign_in_spec.rb b/spec/system/sign_in_spec.rb index a55ae8c2c..3cd4eec9e 100644 --- a/spec/system/sign_in_spec.rb +++ b/spec/system/sign_in_spec.rb @@ -45,7 +45,7 @@ context 'and enters valid credentials' do it 'signs in successfully' do - expect(page).to have_text('About you') # extra registration + expect(page).to have_text('Agree to our terms and conditions') # extra registration end end From dad5b3ae3d771d56e8017eefb2832996de4f9b87 Mon Sep 17 00:00:00 2001 From: "jack.coggin" Date: Fri, 1 Dec 2023 15:47:51 +0000 Subject: [PATCH 4/7] add feature flag to redirect --- app/controllers/application_controller.rb | 6 +++++- app/controllers/users/sessions_controller.rb | 4 +++- config/application.rb | 5 +++++ .../confirmed_user/completing_registration_spec.rb | 9 ++++----- 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 7b9ba514a..3dd694581 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -20,7 +20,11 @@ def authenticate_registered_user! authenticate_user! unless user_signed_in? return true if current_user.registration_complete? - redirect_to edit_registration_terms_and_conditions_path, notice: 'Please complete registration' + if Rails.application.gov_one_login? + redirect_to edit_registration_terms_and_conditions_path, notice: 'Please complete registration' + else + redirect_to edit_registration_name_path, notice: 'Please complete registration' + end end def configure_permitted_parameters diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index a4db408cb..aeb2584a5 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -16,8 +16,10 @@ def after_sign_in_path_for(resource) end elsif resource.private_beta_registration_complete? static_path('new-registration') - else + elsif Rails.application.gov_one_login? edit_registration_terms_and_conditions_path + else + edit_registration_name_path end end end diff --git a/config/application.rb b/config/application.rb index 42c3e1478..8a0b7345f 100644 --- a/config/application.rb +++ b/config/application.rb @@ -101,6 +101,11 @@ def maintenance? Types::Params::Bool[ENV.fetch('MAINTENANCE', false)] end + # @return [Boolean] + def gov_one_login? + Types::Params::Bool[ENV.fetch('GOV_ONE_LOGIN', false)] + end + # @return [ActiveSupport::TimeWithZone] def public_beta_launch_date Time.zone.local(2023, 2, 9, 15, 0, 0) diff --git a/spec/system/confirmed_user/completing_registration_spec.rb b/spec/system/confirmed_user/completing_registration_spec.rb index bcd43c72b..105e843c9 100644 --- a/spec/system/confirmed_user/completing_registration_spec.rb +++ b/spec/system/confirmed_user/completing_registration_spec.rb @@ -1,15 +1,16 @@ require 'rails_helper' RSpec.describe 'Confirmed users completing registration' do - include_context 'with user' + before do + allow(Rails.application).to receive(:gov_one_login?).and_return(true) + end + include_context 'with user' let(:user) { create :user, :confirmed } it 'requires name and a setting type and email preferences and a complete' do expect(page).to have_text('Terms and conditions') - click_button 'Continue' - expect(page).to have_text('There is a problem') .and have_text('You must accept the terms and conditions and privacy policy to create an account.') @@ -60,14 +61,12 @@ expect(page).to have_text('What is your role?') .and have_text('Enter your job title.') - click_button 'Continue' expect(page).to have_text('There is a problem') .and have_text('Enter your job title.') fill_in 'Enter your job title.', with: 'user defined job title' - click_button 'Continue' expect(page).to have_text('Do you want to get email updates about this training course?') From 1c9fd7c632f87d0cdbe0e7958018dd38b9b9f217 Mon Sep 17 00:00:00 2001 From: "jack.coggin" Date: Fri, 1 Dec 2023 16:05:42 +0000 Subject: [PATCH 5/7] update sign_in_spec --- spec/system/sign_in_spec.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/system/sign_in_spec.rb b/spec/system/sign_in_spec.rb index 3cd4eec9e..9480f0168 100644 --- a/spec/system/sign_in_spec.rb +++ b/spec/system/sign_in_spec.rb @@ -43,6 +43,10 @@ context 'when user is confirmed' do let(:user) { create :user, :confirmed } + before do + allow(Rails.application).to receive(:gov_one_login?).and_return(true) + end + context 'and enters valid credentials' do it 'signs in successfully' do expect(page).to have_text('Agree to our terms and conditions') # extra registration From b915b6a66b5de260dd61047d41560f4d76d54e30 Mon Sep 17 00:00:00 2001 From: "jack.coggin" Date: Mon, 4 Dec 2023 17:05:14 +0000 Subject: [PATCH 6/7] update authentication_spec --- spec/requests/authentication_spec.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/spec/requests/authentication_spec.rb b/spec/requests/authentication_spec.rb index d2ad5bb74..466b3af5c 100644 --- a/spec/requests/authentication_spec.rb +++ b/spec/requests/authentication_spec.rb @@ -85,7 +85,10 @@ end context 'with partially registered User' do - before { sign_in create(:user, :confirmed) } + before do + allow(Rails.application).to receive(:gov_one_login?).and_return(true) + sign_in create(:user, :confirmed) + end it 'redirects to finish registration' do get action_path From d1b61eb89668ea2885cea826354b7a7a328e8417 Mon Sep 17 00:00:00 2001 From: "jack.coggin" Date: Tue, 5 Dec 2023 09:15:05 +0000 Subject: [PATCH 7/7] update sign_in system spec --- spec/system/sign_in_spec.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/spec/system/sign_in_spec.rb b/spec/system/sign_in_spec.rb index 9480f0168..b955ce97a 100644 --- a/spec/system/sign_in_spec.rb +++ b/spec/system/sign_in_spec.rb @@ -5,6 +5,7 @@ let(:password) { 'Str0ngPa$$w0rd' } before do + allow(Rails.application).to receive(:gov_one_login?).and_return(true) visit '/users/sign-in' fill_in 'Email address', with: email_address fill_in 'Password', with: password @@ -43,10 +44,6 @@ context 'when user is confirmed' do let(:user) { create :user, :confirmed } - before do - allow(Rails.application).to receive(:gov_one_login?).and_return(true) - end - context 'and enters valid credentials' do it 'signs in successfully' do expect(page).to have_text('Agree to our terms and conditions') # extra registration