From bb981a525d4b273a4bfbff9de1e35c829adff264 Mon Sep 17 00:00:00 2001 From: James Mead Date: Wed, 9 Aug 2023 13:55:17 +0100 Subject: [PATCH 1/5] Add user research recruitment banner Trello: https://trello.com/c/yGMU3sR2 We're only showing the banner on the dashboard so that we don't overwhelm people by showing it everywhere. Ideally we'd have used the GOV.UK Components button [1] for the two buttons. However, the `govuk-button--inverse` CSS class [2] is not supported by the component and there's no easy way to override it. Since we're not using that component we've had to introduce `ApplicationHelper#arrow_svg` to generate the arrow SVG for the start-style button [3]. It would be worth opening a PR to add an `inverse` property to the GOV.UK Components button so we can avoid this duplication. [1]: https://components.publishing.service.gov.uk/component-guide/button [2]: https://design-system.service.gov.uk/components/button/#buttons-on-dark-backgrounds [3]: https://design-system.service.gov.uk/components/button/#start-buttons --- .../_user_research_recruitment_banner.scss | 22 ++++++++++++ app/assets/stylesheets/application.scss | 1 + app/helpers/application_helper.rb | 12 +++++++ app/views/layouts/admin_layout.html.erb | 2 ++ app/views/root/index.html.erb | 20 +++++++++++ .../user_research_recruitment_banner_test.rb | 34 +++++++++++++++++++ 6 files changed, 91 insertions(+) create mode 100644 app/assets/stylesheets/_user_research_recruitment_banner.scss create mode 100644 test/integration/user_research_recruitment_banner_test.rb diff --git a/app/assets/stylesheets/_user_research_recruitment_banner.scss b/app/assets/stylesheets/_user_research_recruitment_banner.scss new file mode 100644 index 000000000..e38d13f74 --- /dev/null +++ b/app/assets/stylesheets/_user_research_recruitment_banner.scss @@ -0,0 +1,22 @@ +.user-research-recruitment-banner { + background-color: $govuk-brand-colour; + @include govuk-responsive-padding(8, "top"); +} + +.user-research-recruitment-banner__divider { + border-bottom: 1px solid govuk-colour("white"); +} + +.user-research-recruitment-banner__title { + color: govuk-colour("white"); + @include govuk-responsive-margin(5, "bottom"); +} + +.user-research-recruitment-banner__intro { + color: govuk-colour("white"); +} + +.user-research-recruitment-banner__buttons { + @include govuk-responsive-padding(6, "bottom"); + align-items: center; +} diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index 0c2e82fdb..5f596438c 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -1,4 +1,5 @@ @import "govuk_publishing_components/all_components"; +@import "user_research_recruitment_banner"; // TODO: move into component .gem-c-success-alert, diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 133cc6cb3..c00c86d14 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -35,4 +35,16 @@ def sanitised_fullpath uri.query_values = uri.query_values.reject { |key, _value| SENSITIVE_QUERY_PARAMETERS.include?(key) } uri.to_s end + + def start_button_text(text) + "#{text} #{arrow_svg}".html_safe + end + + def arrow_svg + %( + + ) + end end diff --git a/app/views/layouts/admin_layout.html.erb b/app/views/layouts/admin_layout.html.erb index 5f68cd388..e6ff12650 100644 --- a/app/views/layouts/admin_layout.html.erb +++ b/app/views/layouts/admin_layout.html.erb @@ -11,6 +11,8 @@ navigation_items: navigation_items, }%> + <%= yield(:user_research_recruitment_banner) %> +
<% if yield(:back_link).present? %> <%= render "govuk_publishing_components/components/back_link", href: yield(:back_link) %> diff --git a/app/views/root/index.html.erb b/app/views/root/index.html.erb index 5af694e18..47463ce4c 100644 --- a/app/views/root/index.html.erb +++ b/app/views/root/index.html.erb @@ -1,5 +1,25 @@ <% content_for :title, "Your applications" %> + <% content_for :user_research_recruitment_banner do %> +
+
+
+

Help us improve GOV.UK Publishing

+

We're holding research sessions to make Publishing work better.

+
+ <%= button_to start_button_text('Find out more'), + url_for, + class: 'govuk-!-font-size-24 govuk-!-font-weight-bold govuk-button govuk-button--start govuk-button--inverse' + %> + <%= button_to 'Hide this', + url_for, + class: 'govuk-button govuk-button--secondary govuk-button--inverse' + %> +
+
+
+ <% end %> +
diff --git a/test/integration/user_research_recruitment_banner_test.rb b/test/integration/user_research_recruitment_banner_test.rb new file mode 100644 index 000000000..9e8a20699 --- /dev/null +++ b/test/integration/user_research_recruitment_banner_test.rb @@ -0,0 +1,34 @@ +require "test_helper" + +class UserResearchRecruitmentBannerTest < ActionDispatch::IntegrationTest + should "not display the banner on the login page" do + visit new_user_session_path + + assert_not has_content?(user_research_recruitment_banner_title) + end + + should "display the banner on the dashboard" do + user = create(:user, name: "user-name", email: "user@example.com") + visit new_user_session_path + signin_with(user) + + assert has_content?(user_research_recruitment_banner_title) + assert has_css?("form", text: "Find out more") + end + + should "not display the banner on any page other than the dashboard" do + user = create(:user, name: "user-name", email: "user@example.com") + visit new_user_session_path + signin_with(user) + + click_on "Change your email or password" + + assert_not has_content?(user_research_recruitment_banner_title) + end + +private + + def user_research_recruitment_banner_title + "Help us improve GOV.UK Publishing" + end +end From 3f2872d9f7cbaae87308b24ed325730e4e85e5f2 Mon Sep 17 00:00:00 2001 From: James Mead Date: Wed, 9 Aug 2023 14:13:53 +0100 Subject: [PATCH 2/5] Allow users to hide the user research recruitment banner Trello: https://trello.com/c/yGMU3sR2 We set a session cookie to hide the banner until the user restarts their browser. --- app/controllers/root_controller.rb | 7 +++++ .../user_research_recruitment_controller.rb | 9 +++++++ app/views/root/index.html.erb | 4 ++- config/routes.rb | 2 ++ ...er_research_recruitment_controller_test.rb | 25 ++++++++++++++++++ .../user_research_recruitment_banner_test.rb | 26 +++++++++++++++++++ 6 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 app/controllers/user_research_recruitment_controller.rb create mode 100644 test/controllers/user_research_recruitment_controller_test.rb diff --git a/app/controllers/root_controller.rb b/app/controllers/root_controller.rb index 70ea029f4..d0d9ecab4 100644 --- a/app/controllers/root_controller.rb +++ b/app/controllers/root_controller.rb @@ -14,4 +14,11 @@ def index def signin_required @application = ::Doorkeeper::Application.find_by(id: session.delete(:signin_missing_for_application)) end + +private + + def show_user_research_recruitment_banner? + !cookies[:dismiss_user_research_recruitment_banner] + end + helper_method :show_user_research_recruitment_banner? end diff --git a/app/controllers/user_research_recruitment_controller.rb b/app/controllers/user_research_recruitment_controller.rb new file mode 100644 index 000000000..bf9a46213 --- /dev/null +++ b/app/controllers/user_research_recruitment_controller.rb @@ -0,0 +1,9 @@ +class UserResearchRecruitmentController < ApplicationController + before_action :authenticate_user! + skip_after_action :verify_authorized + + def dismiss_banner + cookies[:dismiss_user_research_recruitment_banner] = true + redirect_to root_path + end +end diff --git a/app/views/root/index.html.erb b/app/views/root/index.html.erb index 47463ce4c..cbcd02383 100644 --- a/app/views/root/index.html.erb +++ b/app/views/root/index.html.erb @@ -1,5 +1,6 @@ <% content_for :title, "Your applications" %> +<% if show_user_research_recruitment_banner? %> <% content_for :user_research_recruitment_banner do %>
@@ -12,13 +13,14 @@ class: 'govuk-!-font-size-24 govuk-!-font-weight-bold govuk-button govuk-button--start govuk-button--inverse' %> <%= button_to 'Hide this', - url_for, + user_research_recruitment_dismiss_banner_path, class: 'govuk-button govuk-button--secondary govuk-button--inverse' %>
<% end %> +<% end %>
diff --git a/config/routes.rb b/config/routes.rb index 7103862cb..3fcdee8d3 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -77,4 +77,6 @@ get "/signin-required" => "root#signin_required" root to: "root#index" + + post "/user-research-recruitment/dismiss-banner" => "user_research_recruitment#dismiss_banner" end diff --git a/test/controllers/user_research_recruitment_controller_test.rb b/test/controllers/user_research_recruitment_controller_test.rb new file mode 100644 index 000000000..70f1e7b9a --- /dev/null +++ b/test/controllers/user_research_recruitment_controller_test.rb @@ -0,0 +1,25 @@ +require "test_helper" + +class UserResearchRecruitmentControllerTest < ActionController::TestCase + test "#dismiss_banner requires signed in users" do + post :dismiss_banner + + assert_redirected_to new_user_session_path + end + + test "#dismiss_banner sets session cookie" do + sign_in create(:user) + + post :dismiss_banner + + assert cookies[:dismiss_user_research_recruitment_banner] + end + + test "#dismiss_banner redirects to root path" do + sign_in create(:user) + + post :dismiss_banner + + assert_redirected_to root_path + end +end diff --git a/test/integration/user_research_recruitment_banner_test.rb b/test/integration/user_research_recruitment_banner_test.rb index 9e8a20699..5c3195717 100644 --- a/test/integration/user_research_recruitment_banner_test.rb +++ b/test/integration/user_research_recruitment_banner_test.rb @@ -26,6 +26,32 @@ class UserResearchRecruitmentBannerTest < ActionDispatch::IntegrationTest assert_not has_content?(user_research_recruitment_banner_title) end + should "hide the banner until the next session" do + user = create(:user, name: "user-name", email: "user@example.com") + + using_session("Session 1") do + visit new_user_session_path + signin_with(user) + + assert has_content?(user_research_recruitment_banner_title) + + within ".user-research-recruitment-banner" do + click_on "Hide this" + end + + visit root_path + + assert_not has_content?(user_research_recruitment_banner_title) + end + + using_session("Session 2") do + visit new_user_session_path + signin_with(user) + + assert has_content?(user_research_recruitment_banner_title) + end + end + private def user_research_recruitment_banner_title From 926269a1af4c2aa17d143aeb36cfb731016c7d2b Mon Sep 17 00:00:00 2001 From: James Mead Date: Wed, 9 Aug 2023 14:23:02 +0100 Subject: [PATCH 3/5] Hide user research recruitment banner permanently Trello: https://trello.com/c/yGMU3sR2 If the user clicks on "Find out more" then we assume that they've completed the Google Form and therefore we don't want to annoy them by continuing to show them the recruitment banner. We've introduced the `UserResearchRecruitmentController#participate` action so that we can record that the user has clicked the button before sending them off to the Google Form. --- app/controllers/root_controller.rb | 2 +- .../user_research_recruitment_controller.rb | 7 ++++ app/views/root/index.html.erb | 2 +- config/routes.rb | 1 + ...arch_recruitment_banner_hidden_to_users.rb | 5 +++ db/schema.rb | 3 +- ...er_research_recruitment_controller_test.rb | 23 ++++++++++++ .../user_research_recruitment_banner_test.rb | 36 ++++++++++++++++++- 8 files changed, 75 insertions(+), 4 deletions(-) create mode 100644 db/migrate/20230804094159_add_user_research_recruitment_banner_hidden_to_users.rb diff --git a/app/controllers/root_controller.rb b/app/controllers/root_controller.rb index d0d9ecab4..297c303f8 100644 --- a/app/controllers/root_controller.rb +++ b/app/controllers/root_controller.rb @@ -18,7 +18,7 @@ def signin_required private def show_user_research_recruitment_banner? - !cookies[:dismiss_user_research_recruitment_banner] + !cookies[:dismiss_user_research_recruitment_banner] && !current_user.user_research_recruitment_banner_hidden? end helper_method :show_user_research_recruitment_banner? end diff --git a/app/controllers/user_research_recruitment_controller.rb b/app/controllers/user_research_recruitment_controller.rb index bf9a46213..a5c865c4e 100644 --- a/app/controllers/user_research_recruitment_controller.rb +++ b/app/controllers/user_research_recruitment_controller.rb @@ -1,4 +1,6 @@ class UserResearchRecruitmentController < ApplicationController + USER_RESEARCH_RECRUITMENT_FORM_URL = "https://docs.google.com/forms/d/1Bdu_GqOrSR4j6mbuzXkFTQg6FRktRMQc8Y-q879Mny8/viewform".freeze + before_action :authenticate_user! skip_after_action :verify_authorized @@ -6,4 +8,9 @@ def dismiss_banner cookies[:dismiss_user_research_recruitment_banner] = true redirect_to root_path end + + def participate + current_user.update!(user_research_recruitment_banner_hidden: true) + redirect_to USER_RESEARCH_RECRUITMENT_FORM_URL, allow_other_host: true + end end diff --git a/app/views/root/index.html.erb b/app/views/root/index.html.erb index cbcd02383..c56bf5438 100644 --- a/app/views/root/index.html.erb +++ b/app/views/root/index.html.erb @@ -9,7 +9,7 @@

We're holding research sessions to make Publishing work better.

<%= button_to start_button_text('Find out more'), - url_for, + user_research_recruitment_participate_path, class: 'govuk-!-font-size-24 govuk-!-font-weight-bold govuk-button govuk-button--start govuk-button--inverse' %> <%= button_to 'Hide this', diff --git a/config/routes.rb b/config/routes.rb index 3fcdee8d3..7dbb1bd9e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -79,4 +79,5 @@ root to: "root#index" post "/user-research-recruitment/dismiss-banner" => "user_research_recruitment#dismiss_banner" + post "/user-research-recruitment/participate" => "user_research_recruitment#participate" end diff --git a/db/migrate/20230804094159_add_user_research_recruitment_banner_hidden_to_users.rb b/db/migrate/20230804094159_add_user_research_recruitment_banner_hidden_to_users.rb new file mode 100644 index 000000000..fa15027a5 --- /dev/null +++ b/db/migrate/20230804094159_add_user_research_recruitment_banner_hidden_to_users.rb @@ -0,0 +1,5 @@ +class AddUserResearchRecruitmentBannerHiddenToUsers < ActiveRecord::Migration[7.0] + def change + add_column :users, :user_research_recruitment_banner_hidden, :boolean, default: false + end +end diff --git a/db/schema.rb b/db/schema.rb index fd8029a12..23ce30460 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_08_02_095323) do +ActiveRecord::Schema[7.0].define(version: 2023_08_04_094159) do create_table "batch_invitation_application_permissions", id: :integer, charset: "utf8mb3", force: :cascade do |t| t.integer "batch_invitation_id", null: false t.integer "supported_permission_id", null: false @@ -209,6 +209,7 @@ t.boolean "require_2sv", default: false, null: false t.string "reason_for_2sv_exemption" t.date "expiry_date_for_2sv_exemption" + t.boolean "user_research_recruitment_banner_hidden", default: false t.index ["email"], name: "index_users_on_email", unique: true t.index ["invitation_token"], name: "index_users_on_invitation_token" t.index ["invited_by_id"], name: "index_users_on_invited_by_id" diff --git a/test/controllers/user_research_recruitment_controller_test.rb b/test/controllers/user_research_recruitment_controller_test.rb index 70f1e7b9a..72b7222d2 100644 --- a/test/controllers/user_research_recruitment_controller_test.rb +++ b/test/controllers/user_research_recruitment_controller_test.rb @@ -22,4 +22,27 @@ class UserResearchRecruitmentControllerTest < ActionController::TestCase assert_redirected_to root_path end + + test "#participate requires users to be signed in" do + post :participate + + assert_redirected_to new_user_session_path + end + + test "#participate sets user_research_recruitment_banner_hidden to true for the current_user" do + user = create(:user) + sign_in user + + post :participate + + assert user.user_research_recruitment_banner_hidden? + end + + test "#participate redirects to the google form" do + sign_in create(:user) + + post :participate + + assert_redirected_to UserResearchRecruitmentController::USER_RESEARCH_RECRUITMENT_FORM_URL + end end diff --git a/test/integration/user_research_recruitment_banner_test.rb b/test/integration/user_research_recruitment_banner_test.rb index 5c3195717..bbc5b52d2 100644 --- a/test/integration/user_research_recruitment_banner_test.rb +++ b/test/integration/user_research_recruitment_banner_test.rb @@ -13,7 +13,7 @@ class UserResearchRecruitmentBannerTest < ActionDispatch::IntegrationTest signin_with(user) assert has_content?(user_research_recruitment_banner_title) - assert has_css?("form", text: "Find out more") + assert has_css?("form[action='#{user_research_recruitment_participate_path}']", text: "Find out more") end should "not display the banner on any page other than the dashboard" do @@ -52,9 +52,43 @@ class UserResearchRecruitmentBannerTest < ActionDispatch::IntegrationTest end end + should "hide the banner permanently if the user clicks the button to participate in user research" do + user = create(:user, name: "user-name", email: "user@example.com") + + using_session("Session 1") do + visit new_user_session_path + signin_with(user) + + assert has_content?(user_research_recruitment_banner_title) + + within ".user-research-recruitment-banner" do + allowing_request_to_user_research_recruitment_google_form do + click_on "Find out more" + end + end + + visit root_path + + assert_not has_content?(user_research_recruitment_banner_title) + end + + using_session("Session 2") do + visit new_user_session_path + signin_with(user) + + assert_not has_content?(user_research_recruitment_banner_title) + end + end + private def user_research_recruitment_banner_title "Help us improve GOV.UK Publishing" end + + def allowing_request_to_user_research_recruitment_google_form + yield + rescue ActionController::RoutingError + raise unless current_url == UserResearchRecruitmentController::USER_RESEARCH_RECRUITMENT_FORM_URL + end end From 58e21edb22ab26eb5316b558c7422a94d5bcd631 Mon Sep 17 00:00:00 2001 From: James Mead Date: Wed, 9 Aug 2023 14:40:36 +0100 Subject: [PATCH 4/5] Open the UR Recruitment Google Form in a new browser tab MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Based on this feedback from Joseph [1]: > When you click ‘Sign up’, it would be better if this opened a new tab > rather than replacing the current tab - this could be quite frustrating > for some users who’d then need to navigate back to Signon and might > actually cause people to close the form immediately. [1]: https://trello.com/c/yGMU3sR2/211#comment-64d0dc81295a6fd92483e962 --- app/views/root/index.html.erb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/views/root/index.html.erb b/app/views/root/index.html.erb index c56bf5438..9e3bacceb 100644 --- a/app/views/root/index.html.erb +++ b/app/views/root/index.html.erb @@ -10,7 +10,8 @@
<%= button_to start_button_text('Find out more'), user_research_recruitment_participate_path, - class: 'govuk-!-font-size-24 govuk-!-font-weight-bold govuk-button govuk-button--start govuk-button--inverse' + class: 'govuk-!-font-size-24 govuk-!-font-weight-bold govuk-button govuk-button--start govuk-button--inverse', + form: { target: '_blank' } %> <%= button_to 'Hide this', user_research_recruitment_dismiss_banner_path, From e62a77fc316636df8f84970092cf0238caf957cc Mon Sep 17 00:00:00 2001 From: James Mead Date: Wed, 9 Aug 2023 18:29:01 +0100 Subject: [PATCH 5/5] WIP: Fix layout on mobile --- app/helpers/application_helper.rb | 12 ------------ app/views/root/index.html.erb | 25 ++++++++++++++----------- 2 files changed, 14 insertions(+), 23 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index c00c86d14..133cc6cb3 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -35,16 +35,4 @@ def sanitised_fullpath uri.query_values = uri.query_values.reject { |key, _value| SENSITIVE_QUERY_PARAMETERS.include?(key) } uri.to_s end - - def start_button_text(text) - "#{text} #{arrow_svg}".html_safe - end - - def arrow_svg - %( - - ) - end end diff --git a/app/views/root/index.html.erb b/app/views/root/index.html.erb index 9e3bacceb..b64d84bc8 100644 --- a/app/views/root/index.html.erb +++ b/app/views/root/index.html.erb @@ -7,17 +7,20 @@

Help us improve GOV.UK Publishing

We're holding research sessions to make Publishing work better.

-
- <%= button_to start_button_text('Find out more'), - user_research_recruitment_participate_path, - class: 'govuk-!-font-size-24 govuk-!-font-weight-bold govuk-button govuk-button--start govuk-button--inverse', - form: { target: '_blank' } - %> - <%= button_to 'Hide this', - user_research_recruitment_dismiss_banner_path, - class: 'govuk-button govuk-button--secondary govuk-button--inverse' - %> -
+ <%= form_tag user_research_recruitment_participate_path do %> +
+ + + +
+ <% end %>
<% end %>