From 49eb705d1a4428bac0badee8bcff0cacfa29ba7c Mon Sep 17 00:00:00 2001 From: Roy Liu <rliu@gitlab.com> Date: Mon, 22 Apr 2024 14:05:56 +0000 Subject: [PATCH] Update UI for Duo Pro trial errors --- ee/app/helpers/trials_helper.rb | 14 +++++++++++ .../trials/base_apply_trial_service.rb | 3 ++- .../trials/base_create_service.rb | 4 +++- .../trials/_form_errors.html.haml | 4 +++- .../trials/duo_pro/_form_errors.html.haml | 6 +++++ .../trials/duo_pro/step_lead_failed.html.haml | 2 +- .../trials/duo_pro/trial_failed.html.haml | 2 +- .../trials/step_lead_failed.html.haml | 2 +- .../trials/trial_failed.html.haml | 2 +- ee/spec/helpers/trials_helper_spec.rb | 24 +++++++++++++++++++ .../trials/duo_pro_controller_spec.rb | 4 ++-- .../subscriptions/trials_controller_spec.rb | 6 ++--- .../trials/apply_duo_pro_service_spec.rb | 6 +++-- .../trials/apply_trial_service_spec.rb | 6 +++-- .../support/helpers/features/trial_helpers.rb | 10 ++++---- locale/gitlab.pot | 12 +++++++--- 16 files changed, 84 insertions(+), 23 deletions(-) create mode 100644 ee/app/views/subscriptions/trials/duo_pro/_form_errors.html.haml diff --git a/ee/app/helpers/trials_helper.rb b/ee/app/helpers/trials_helper.rb index f8e6f82d9166e..66333bd9ffac5 100644 --- a/ee/app/helpers/trials_helper.rb +++ b/ee/app/helpers/trials_helper.rb @@ -92,6 +92,20 @@ def namespace_options_for_listbox(namespaces) options end + def trial_form_errors_message(result) + unless result.reason == GitlabSubscriptions::Trials::BaseApplyTrialService::GENERIC_TRIAL_ERROR + return result.errors.to_sentence + end + + support_link_url = 'https://support.gitlab.com/hc/en-us' + support_link = link_to('', support_link_url, target: '_blank', + rel: 'noopener noreferrer') + safe_format( + _('Please try again or reach out to %{support_link_start}GitLab Support%{support_link_end}.'), + tag_pair(support_link, :support_link_start, :support_link_end) + ) + end + private def current_namespaces_for_selector(namespaces) diff --git a/ee/app/services/gitlab_subscriptions/trials/base_apply_trial_service.rb b/ee/app/services/gitlab_subscriptions/trials/base_apply_trial_service.rb index 94b8f8876ed8e..d23b52d29f276 100644 --- a/ee/app/services/gitlab_subscriptions/trials/base_apply_trial_service.rb +++ b/ee/app/services/gitlab_subscriptions/trials/base_apply_trial_service.rb @@ -4,6 +4,7 @@ module GitlabSubscriptions module Trials class BaseApplyTrialService include ::Gitlab::Utils::StrongMemoize + GENERIC_TRIAL_ERROR = :generic_trial_error def self.execute(args = {}) instance = new(**args) @@ -31,7 +32,7 @@ def generate_trial ServiceResponse.success else - ServiceResponse.error(message: response.dig(:data, :errors)) + ServiceResponse.error(message: response.dig(:data, :errors), reason: GENERIC_TRIAL_ERROR) end end diff --git a/ee/app/services/gitlab_subscriptions/trials/base_create_service.rb b/ee/app/services/gitlab_subscriptions/trials/base_create_service.rb index 0cf1af58a9cad..d3f9647ad814b 100644 --- a/ee/app/services/gitlab_subscriptions/trials/base_create_service.rb +++ b/ee/app/services/gitlab_subscriptions/trials/base_create_service.rb @@ -119,7 +119,9 @@ def after_trial_success_hook def after_trial_error_hook(result) ServiceResponse.error( - message: result.message, payload: { namespace_id: trial_params[:namespace_id] }, reason: TRIAL_FAILED + message: result.message, + payload: { namespace_id: trial_params[:namespace_id] }, + reason: result.reason || TRIAL_FAILED ) end diff --git a/ee/app/views/subscriptions/trials/_form_errors.html.haml b/ee/app/views/subscriptions/trials/_form_errors.html.haml index 55c3eaff043f1..09b2f187fdeb1 100644 --- a/ee/app/views/subscriptions/trials/_form_errors.html.haml +++ b/ee/app/views/subscriptions/trials/_form_errors.html.haml @@ -1,4 +1,6 @@ -= render Pajamas::AlertComponent.new(variant: :danger, dismissible: false, title: _('We have found the following errors:'), +- title = _("We're sorry, your trial could not be created because our system did not respond successfully.") + += render Pajamas::AlertComponent.new(variant: :danger, dismissible: false, title: title, alert_options: { class: 'gl-mt-3 gl-mb-5' }) do |c| - c.with_body do = errors diff --git a/ee/app/views/subscriptions/trials/duo_pro/_form_errors.html.haml b/ee/app/views/subscriptions/trials/duo_pro/_form_errors.html.haml new file mode 100644 index 0000000000000..efa0ca81e9c28 --- /dev/null +++ b/ee/app/views/subscriptions/trials/duo_pro/_form_errors.html.haml @@ -0,0 +1,6 @@ +- title = _("We're sorry, your GitLab Duo Pro trial could not be created because our system did not respond successfully.") + += render Pajamas::AlertComponent.new(variant: :danger, dismissible: false, title: title, + alert_options: { class: 'gl-mt-3 gl-mb-5' }) do |c| + - c.with_body do + = errors diff --git a/ee/app/views/subscriptions/trials/duo_pro/step_lead_failed.html.haml b/ee/app/views/subscriptions/trials/duo_pro/step_lead_failed.html.haml index 66c19ddd5e007..6cee4e280a8f2 100644 --- a/ee/app/views/subscriptions/trials/duo_pro/step_lead_failed.html.haml +++ b/ee/app/views/subscriptions/trials/duo_pro/step_lead_failed.html.haml @@ -1,4 +1,4 @@ - content_for :before_form do - = render 'subscriptions/trials/form_errors', errors: @result.errors.to_sentence + = render 'form_errors', errors: trial_form_errors_message(@result) = render 'lead_form' diff --git a/ee/app/views/subscriptions/trials/duo_pro/trial_failed.html.haml b/ee/app/views/subscriptions/trials/duo_pro/trial_failed.html.haml index c306d5f81600a..060b46b179052 100644 --- a/ee/app/views/subscriptions/trials/duo_pro/trial_failed.html.haml +++ b/ee/app/views/subscriptions/trials/duo_pro/trial_failed.html.haml @@ -1,4 +1,4 @@ - content_for :before_form do - = render 'subscriptions/trials/form_errors', errors: @result.errors.to_sentence + = render 'form_errors', errors: trial_form_errors_message(@result) = render 'select_namespace_form' diff --git a/ee/app/views/subscriptions/trials/step_lead_failed.html.haml b/ee/app/views/subscriptions/trials/step_lead_failed.html.haml index 3723f07e98bf9..6cee4e280a8f2 100644 --- a/ee/app/views/subscriptions/trials/step_lead_failed.html.haml +++ b/ee/app/views/subscriptions/trials/step_lead_failed.html.haml @@ -1,4 +1,4 @@ - content_for :before_form do - = render 'form_errors', errors: @result.errors.to_sentence + = render 'form_errors', errors: trial_form_errors_message(@result) = render 'lead_form' diff --git a/ee/app/views/subscriptions/trials/trial_failed.html.haml b/ee/app/views/subscriptions/trials/trial_failed.html.haml index de43e1442713d..060b46b179052 100644 --- a/ee/app/views/subscriptions/trials/trial_failed.html.haml +++ b/ee/app/views/subscriptions/trials/trial_failed.html.haml @@ -1,4 +1,4 @@ - content_for :before_form do - = render 'form_errors', errors: @result.errors.to_sentence + = render 'form_errors', errors: trial_form_errors_message(@result) = render 'select_namespace_form' diff --git a/ee/spec/helpers/trials_helper_spec.rb b/ee/spec/helpers/trials_helper_spec.rb index 3dfe64cb8283a..0c65697ef6dd8 100644 --- a/ee/spec/helpers/trials_helper_spec.rb +++ b/ee/spec/helpers/trials_helper_spec.rb @@ -376,4 +376,28 @@ end end end + + describe '#trial_form_errors_message' do + let(:result) { ServiceResponse.error(message: ['some error']) } + + subject { helper.trial_form_errors_message(result) } + + it 'returns error message from the result directly' do + is_expected.to eq('some error') + end + + context 'when the error has :generic_trial_error as reason' do + let(:result) do + ServiceResponse.error(message: ['some error'], + reason: GitlabSubscriptions::Trials::BaseApplyTrialService::GENERIC_TRIAL_ERROR) + end + + it 'overrides the error message' do + is_expected.to include('Please try again or reach out to') + is_expected.to include( + '<a target="_blank" rel="noopener noreferrer" href="https://support.gitlab.com/hc/en-us">GitLab Support</a>' + ) + end + end + end end diff --git a/ee/spec/requests/subscriptions/trials/duo_pro_controller_spec.rb b/ee/spec/requests/subscriptions/trials/duo_pro_controller_spec.rb index 66dbcc0ad6bdf..2d6d75637f268 100644 --- a/ee/spec/requests/subscriptions/trials/duo_pro_controller_spec.rb +++ b/ee/spec/requests/subscriptions/trials/duo_pro_controller_spec.rb @@ -203,7 +203,7 @@ expect(post_create).to render_select_namespace expect(response.body).to include('data-namespace-create-errors="_error_"') - expect(response.body).not_to include(_('We have found the following errors:')) + expect(response.body).not_to include(_('your GitLab Duo Pro trial could not be created')) end end @@ -215,7 +215,7 @@ it 'renders the select namespace form again with trial creation errors only' do expect(post_create).to render_select_namespace - expect(response.body).to include(_('We have found the following errors:')) + expect(response.body).to include(_('your GitLab Duo Pro trial could not be created')) end end diff --git a/ee/spec/requests/subscriptions/trials_controller_spec.rb b/ee/spec/requests/subscriptions/trials_controller_spec.rb index 1b0da7cf8d834..029e77e60415d 100644 --- a/ee/spec/requests/subscriptions/trials_controller_spec.rb +++ b/ee/spec/requests/subscriptions/trials_controller_spec.rb @@ -195,7 +195,7 @@ it 'renders lead form' do expect(post_create).to have_gitlab_http_status(:ok) - expect(response.body).to include(_('We have found the following errors:')) + expect(response.body).to include(_('your trial could not be created')) expect(response.body).to include(_('Start your Free Ultimate Trial')) expect(response.body).to include(s_('Trial|Your GitLab Ultimate trial lasts for 30 days, ' \ 'but you can keep your free GitLab account forever. ' \ @@ -231,7 +231,7 @@ expect(post_create).to render_select_namespace expect(response.body).to include('data-namespace-create-errors="_error_"') - expect(response.body).not_to include(_('We have found the following errors:')) + expect(response.body).not_to include(_('your trial could not be created')) end end @@ -243,7 +243,7 @@ it 'renders the select namespace form again with trial creation errors only' do expect(post_create).to render_select_namespace - expect(response.body).to include(_('We have found the following errors:')) + expect(response.body).to include(_('your trial could not be created')) end end diff --git a/ee/spec/services/gitlab_subscriptions/trials/apply_duo_pro_service_spec.rb b/ee/spec/services/gitlab_subscriptions/trials/apply_duo_pro_service_spec.rb index 538b48e81c408..c1ce0e01a1071 100644 --- a/ee/spec/services/gitlab_subscriptions/trials/apply_duo_pro_service_spec.rb +++ b/ee/spec/services/gitlab_subscriptions/trials/apply_duo_pro_service_spec.rb @@ -53,8 +53,10 @@ context 'with error while applying the trial' do let(:response) { { success: false, data: { errors: ['some error'] } } } - it 'returns success: false with errors' do - expect(execute).to be_error.and have_attributes(message: ['some error']) + it 'returns success: false with errors and reason' do + expect(execute).to be_error.and have_attributes( + message: ['some error'], reason: described_class::GENERIC_TRIAL_ERROR + ) end end end diff --git a/ee/spec/services/gitlab_subscriptions/trials/apply_trial_service_spec.rb b/ee/spec/services/gitlab_subscriptions/trials/apply_trial_service_spec.rb index 720e5bcb43a69..a162344d6209b 100644 --- a/ee/spec/services/gitlab_subscriptions/trials/apply_trial_service_spec.rb +++ b/ee/spec/services/gitlab_subscriptions/trials/apply_trial_service_spec.rb @@ -53,8 +53,10 @@ context 'with error while applying the trial' do let(:response) { { success: false, data: { errors: ['some error'] } } } - it 'returns success: false with errors' do - expect(execute).to be_error.and have_attributes(message: ['some error']) + it 'returns success: false with errors and reason' do + expect(execute).to be_error.and have_attributes( + message: ['some error'], reason: described_class::GENERIC_TRIAL_ERROR + ) end it_behaves_like 'does not record an onboarding progress action' diff --git a/ee/spec/support/helpers/features/trial_helpers.rb b/ee/spec/support/helpers/features/trial_helpers.rb index a909e8df4949a..c9a380d256035 100644 --- a/ee/spec/support/helpers/features/trial_helpers.rb +++ b/ee/spec/support/helpers/features/trial_helpers.rb @@ -16,8 +16,9 @@ def expect_to_be_on_group_page(path: 'gitlab', name: 'gitlab') def expect_to_be_on_namespace_selection_with_errors expect_to_be_on_namespace_selection - expect(page).to have_content('We have found the following errors') - expect(page).to have_content('_trial_fail_') + expect(page).to have_content('could not be created because our system did not respond successfully') + expect(page).to have_content('Please try again or reach out to GitLab Support.') + expect(page).to have_link('GitLab Support', href: 'https://support.gitlab.com/hc/en-us') end def expect_to_be_on_namespace_selection @@ -34,7 +35,7 @@ def expect_to_have_namespace_creation_errors(group_name: '_invalid group name_', end def expect_to_be_on_lead_form_with_errors - expect(page).to have_content('We have found the following errors') + expect(page).to have_content('could not be created because our system did not respond successfully') expect(page).to have_content('_lead_fail_') expect(page).to have_content('Number of employees') @@ -224,7 +225,8 @@ def stub_apply_trial(trial_type: '', namespace_id: anything, success: true, extr trial_success = if success ServiceResponse.success else - ServiceResponse.error(message: '_trial_fail_') + ServiceResponse.error(message: '_trial_fail_', + reason: GitlabSubscriptions::Trials::BaseApplyTrialService::GENERIC_TRIAL_ERROR) end apply_trial_class = diff --git a/locale/gitlab.pot b/locale/gitlab.pot index cb280e5ce8a85..10322a34be36e 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -38355,6 +38355,9 @@ msgstr "" msgid "Please try again" msgstr "" +msgid "Please try again or reach out to %{support_link_start}GitLab Support%{support_link_end}." +msgstr "" + msgid "Please try and refresh the page. If the problem persists please contact support." msgstr "" @@ -57278,9 +57281,6 @@ msgstr "" msgid "We found your token in a public project and have automatically revoked it to protect your account." msgstr "" -msgid "We have found the following errors:" -msgstr "" - msgid "We heard back from your device. You have been authenticated." msgstr "" @@ -57323,6 +57323,12 @@ msgstr "" msgid "We're experiencing difficulties and this tab content is currently unavailable." msgstr "" +msgid "We're sorry, your GitLab Duo Pro trial could not be created because our system did not respond successfully." +msgstr "" + +msgid "We're sorry, your trial could not be created because our system did not respond successfully." +msgstr "" + msgid "We've detected some unusual activity" msgstr "" -- GitLab