diff --git a/.env.example b/.env.example index dcc48b9ce..cd3200131 100644 --- a/.env.example +++ b/.env.example @@ -48,9 +48,6 @@ POSTGRES_PASSWORD= # PGHOST=localhost # PGDATABASE=early_years_foundation_recovery_test -# user research -FEEDBACK_URL= - # Account unlock wait duration if :time is enabled UNLOCK_IN_MINUTES= @@ -82,8 +79,6 @@ CONTENTFUL_PREVIEW_TOKEN= CONTENTFUL_ENVIRONMENT=test # master, staging CONTENTFUL_PREVIEW=true -# Contentful DB migration flags -DISABLE_USER_ANSWER=true # Opt-in end-to-end testing (inactive for review apps) E2E=true diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f5ca5e8ea..cf00acbed 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,7 +48,6 @@ jobs: --health-timeout 5s --health-retries 5 - steps: - name: Checkout Code diff --git a/.rubocop.yml b/.rubocop.yml index 279cf276a..96e32f92a 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -14,7 +14,7 @@ inherit_mode: require: rubocop-performance AllCops: - TargetRubyVersion: 3.1.3 + TargetRubyVersion: 3.2.2 Style/StringLiterals: EnforcedStyle: single_quotes diff --git a/Gemfile b/Gemfile index b26a5e850..5471c3e32 100644 --- a/Gemfile +++ b/Gemfile @@ -50,7 +50,7 @@ gem 'govuk_markdown' gem 'govuk_notify_rails' -# Sentry -Monitor errors +# Monitor errors gem 'sentry-rails' gem 'sentry-ruby' diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index 852f31878..e7973d2c9 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -103,15 +103,20 @@ pre.code-tip { font-family: monospace !important; } } - // -------------------------------------------- -.app-sidebar { - padding: govuk-spacing(5) govuk-spacing(3); - background-color: govuk-colour('light-grey'); +#feedback-cta { + padding: govuk-spacing(5); + background-color: govuk-organisation-colour('department-for-education'); + + * { + color: govuk-colour('white'); + } - *:last-child { - margin: 0; + .govuk-button { + background-color: govuk-organisation-colour('department-for-education'); + border-color: govuk-colour('white'); + box-shadow: none; } } diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index fad98a8af..da5bf519b 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -75,16 +75,23 @@ def set_time_zone(&block) end # @see Auditing - # @return [User] + # @return [User, nil] def current_user return bot if bot? super end + # @return [Guest, nil] + def guest + visit = Visit.find_by(visit_token: cookies[:course_feedback]) || current_visit + Guest.new(visit: visit) if visit.present? + end + # @see Auditing # @return [Boolean] def user_signed_in? + return false if current_user&.guest? return true if bot? super diff --git a/app/controllers/close_accounts_controller.rb b/app/controllers/close_accounts_controller.rb index 426777bec..8dc094248 100644 --- a/app/controllers/close_accounts_controller.rb +++ b/app/controllers/close_accounts_controller.rb @@ -46,8 +46,4 @@ def confirm; end def user_params params.require(:user).permit(:closed_reason, :closed_reason_custom) end - - def user_password_params - params.require(:user).permit(:current_password) - end end diff --git a/app/controllers/concerns/learning.rb b/app/controllers/concerns/learning.rb index 77a623c4d..2924076d8 100644 --- a/app/controllers/concerns/learning.rb +++ b/app/controllers/concerns/learning.rb @@ -55,9 +55,13 @@ def module_table ModuleDebugDecorator.new(progress_service).rows end - # @return [PaginationDecorator] + # @return [PaginationDecorator, FeedbackPaginationDecorator] def section_bar - PaginationDecorator.new(content) + if content.feedback_question? || content.previous_item.feedback_question? + FeedbackPaginationDecorator.new(content, current_user) + else + PaginationDecorator.new(content) + end end # ---------------------------------------------------------------------------- @@ -67,6 +71,6 @@ def section_bar # @note memoization ensures validation errors work # @return [UserAnswer, Response] def current_user_response - @current_user_response ||= current_user.response_for(content) + @current_user_response ||= content.feedback_question? ? current_user.response_for_shared(content, mod) : current_user.response_for(content) end end diff --git a/app/controllers/errors_controller.rb b/app/controllers/errors_controller.rb index 66d66dd00..d21b70ae5 100644 --- a/app/controllers/errors_controller.rb +++ b/app/controllers/errors_controller.rb @@ -1,21 +1,22 @@ class ErrorsController < ApplicationController before_action :log_error - # 404 error def not_found render status: :not_found end - # 422 error def unprocessable_entity render status: :unprocessable_entity end - # 500 error def internal_server_error render status: :internal_server_error end + def service_unavailable + render status: :service_unavailable + end + private def log_error diff --git a/app/controllers/feedback_controller.rb b/app/controllers/feedback_controller.rb new file mode 100644 index 000000000..551e4204e --- /dev/null +++ b/app/controllers/feedback_controller.rb @@ -0,0 +1,112 @@ +class FeedbackController < ApplicationController + before_action :research_participation, only: :show + + helper_method :content, + :mod, + :current_user_response + + def index; end + + def show + if content_name.eql? 'thank-you' + track_feedback_complete + render :thank_you + end + end + + def update + if current_user.profile_updated? && save_response! + flash[:success] = 'Your details have been updated' + redirect_to user_path + elsif save_response! + feedback_cookie + track_feedback_start + redirect_to feedback_path(helpers.next_page.name) + else + render :show, status: :unprocessable_entity + end + end + +private + + # @note + # associate the user research participation question to the course form + # if answered during a training module + # + def research_participation + response = current_user.user_research_response + if content.skippable? && response.present? && !response.training_module.eql?('course') + response.update!(training_module: 'course') + end + end + + # @return [Boolean] + def save_response! + current_user_response.update( + answers: user_answers, + correct: true, + text_input: response_params[:text_input], + ) + end + + # @return [Course] + def mod + Course.config + end + + # @return [Training::Question, Training::Page] + def content + mod.page_by_name(content_name) + end + + # @return [User, Guest, nil] + def current_user + super || guest + end + + # @param question [Training::Question] default: current question + # @return [Response] + def current_user_response(question = content) + @current_user_response ||= current_user.response_for_shared(question, mod) + end + + # @return [String] + def content_name + params[:id] + end + + # OPTIMIZE: duplicated from ResponsesController + def response_params + params.require(:response).permit! + end + + # OPTIMIZE: duplicated from ResponsesController + def user_answers + Array(response_params[:answers]).compact_blank.map(&:to_i) + end + + # @return [Hash] + def feedback_cookie + cookies[:course_feedback] = { value: current_user.visit_token } + end + + # @return [Boolean, nil] + def track_feedback_start + track('feedback_start') if untracked?('feedback_start') + end + + # @return [Boolean, nil] + def track_feedback_complete + track('feedback_complete') if untracked?('feedback_complete') + end + + # @param key [String] + # @return [Boolean] + def untracked?(key) + if current_user.guest? + Event.where(visit_id: current_visit, name: key).empty? + else + Event.where(user_id: current_user, name: key).empty? + end + end +end diff --git a/app/controllers/training/questions_controller.rb b/app/controllers/training/questions_controller.rb index 83e428c18..e710c4a81 100644 --- a/app/controllers/training/questions_controller.rb +++ b/app/controllers/training/questions_controller.rb @@ -32,7 +32,11 @@ def show # @see Tracking # @return [Event] Show action def track_events - if track_confidence_start? + if track_feedback_start? + track('feedback_start') + elsif track_feedback_complete? + track('feedback_complete') + elsif track_confidence_start? track('confidence_check_start') elsif track_assessment_start? track('summative_assessment_start') @@ -52,6 +56,16 @@ def track_confidence_start? content.first_confidence? && confidence_start_untracked? end + # @return [Boolean] + def track_feedback_start? + content.first_feedback? && feedback_start_untracked? + end + + # @return [Boolean] + def track_feedback_complete? + content.last_feedback? && feedback_complete_untracked? + end + # @return [Boolean] def track_assessment_start? content.first_assessment? && summative_start_untracked? @@ -68,5 +82,15 @@ def summative_start_untracked? def confidence_start_untracked? untracked?('confidence_check_start', training_module_id: mod.name) end + + # @return [Boolean] + def feedback_start_untracked? + untracked?('feedback_start', training_module_id: mod.name) + end + + # @return [Boolean] + def feedback_complete_untracked? + untracked?('feedback_complete', training_module_id: mod.name) + end end end diff --git a/app/controllers/training/responses_controller.rb b/app/controllers/training/responses_controller.rb index 85677889b..b2d9c288a 100644 --- a/app/controllers/training/responses_controller.rb +++ b/app/controllers/training/responses_controller.rb @@ -6,6 +6,7 @@ module Training class ResponsesController < ApplicationController include Learning + include Pagination before_action :authenticate_registered_user! @@ -41,10 +42,10 @@ def response_params # @note migrate from user_answer to response # @return [Boolean] def save_response! - correct_answers = content.confidence_question? ? true : content.correct_answers.eql?(user_answers) + correct_answers = content.opinion_question? ? true : content.correct_answers.eql?(user_answers) if Rails.application.migrated_answers? - current_user_response.update(answers: user_answers, correct: correct_answers) + current_user_response.update(answers: user_answers, correct: correct_answers, text_input: user_answer_text) else current_user_response.update(answer: user_answers, correct: correct_answers) end @@ -55,13 +56,17 @@ def user_answers Array(response_params[:answers]).compact_blank.map(&:to_i) end + def user_answer_text + response_params[:text_input] + end + def redirect assessment.grade! if content.last_assessment? if content.formative_question? redirect_to training_module_question_path(mod.name, content.name) else - redirect_to training_module_page_path(mod.name, content.next_item.name) + redirect_to training_module_page_path(mod.name, helpers.next_page.name) end end diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 77b2da40a..fbfe8b624 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -23,28 +23,6 @@ def update_name end end - # @see config/initializers/devise.rb - def update_password - if current_user.update_with_password(user_password_params) - track('user_password_change', success: true) - bypass_sign_in(current_user) - redirect_to user_path, notice: 'Your new password has been saved.' - else - track('user_password_change', success: false) - render :edit_password, status: :unprocessable_entity - end - end - - def update_email - if current_user.update(user_params) - track('user_email_change', success: true) - redirect_to user_path, notice: t('notice.email_changed') - else - track('user_email_change', success: false) - render :edit_email, status: :unprocessable_entity - end - end - def update_training_emails if current_user.update(user_params) track('user_training_emails_change', success: true) diff --git a/app/decorators/feedback_pagination_decorator.rb b/app/decorators/feedback_pagination_decorator.rb new file mode 100644 index 000000000..6a7a60d6d --- /dev/null +++ b/app/decorators/feedback_pagination_decorator.rb @@ -0,0 +1,53 @@ +# Overrides heading +# Checks if one-off questions should be skipped +# +class FeedbackPaginationDecorator < PaginationDecorator + # TODO: Add type check for feedback question type + + # @!attribute [r] user + # @return [User] + param :user, Types.Instance(User), required: true + + # @return [String] + def heading + 'Additional feedback' + end + +private + + # @return [Integer] + def current_page + return super unless skip_question? && after_skippable_question? + + super - 1 + end + + # @return [Integer] + def page_total + return super unless skip_question? + + super - 1 + end + + # @return [Boolean] + def after_skippable_question? + content.section_content.index(content) > content.section_content.index(skippable_question) + end + + # @return [Training::Question] + def skippable_question + content.section_content.find(&:skippable?) + end + + # @return [Array] + def other_forms + Training::Module.live.to_a - [content.parent] << Course.config + end + + # @return [Boolean] + def skip_question? + other_forms.any? do |form| + user.response_for_shared(skippable_question, form).responded? + end + end +end diff --git a/app/decorators/module_overview_decorator.rb b/app/decorators/module_overview_decorator.rb index f196c0375..09220055f 100644 --- a/app/decorators/module_overview_decorator.rb +++ b/app/decorators/module_overview_decorator.rb @@ -42,6 +42,7 @@ def sections display_line: position != mod.submodule_count, icon: status(content_items), subsections: subsections(submodule: submodule, items: content_items), + hide: content_items.first.feedback_question?, } end end diff --git a/app/decorators/next_page_decorator.rb b/app/decorators/next_page_decorator.rb index 3076bb1e8..9f9a79f7d 100644 --- a/app/decorators/next_page_decorator.rb +++ b/app/decorators/next_page_decorator.rb @@ -7,24 +7,26 @@ class NextPageDecorator extend Dry::Initializer # @!attribute [r] user - # @return [User] - option :user, Types.Instance(User), required: true + # @return [User, Guest] + option :user, Types.Instance(User) | Types.Instance(Guest), required: true # @!attribute [r] mod - # @return [Training::Module] - option :mod, Types::TrainingModule, required: true + # @return [Course, Training::Module] + option :mod, Types::Parent, required: true # @!attribute [r] content # @return [Training::Page, Training::Question, Training::Video] option :content, Types::TrainingContent, required: true # @!attribute [r] assessment # @return [AssessmentProgress] - option :assessment, required: true + option :assessment # @return [String] def name if content.interruption_page? mod.content_start.name + elsif skip_next_question? + next_next_item.name else - content.next_item.name + next_item.name end end @@ -32,13 +34,14 @@ def name # @return [String] def text case - when next? then label[:next] - when missing? then label[:missing] - when content.section? then label[:section] - when test_start? then label[:start_test] - when test_finish? then label[:finish_test] - when finish? then label[:finish] - when save? then label[:save_continue] + when next? then label[:next] + when missing? then label[:missing] + when content_section? then label[:section] + when confidence_outro? then label[:give_feedback] + when test_start? then label[:start_test] + when test_finish? then label[:finish_test] + when finish? then label[:finish] + when save? then label[:save_continue] else label[:next] end @@ -79,7 +82,7 @@ def answered? # @return [Boolean] def finish? - content.next_item.certificate? + next_item.certificate? end # @return [Boolean] @@ -91,16 +94,42 @@ def test_start? # @return [Boolean] def test_finish? - content.next_item.assessment_results? && !disable_question_submission? + next_item.assessment_results? && !disable_question_submission? end # @return [Boolean] def missing? - content.next_item.eql?(content) && wip? + next_item.eql?(content) && wip? + end + + # @return [Boolean] + def confidence_outro? + mod.feedback_questions.first.previous_item.eql?(content) + end + + # @return [Boolean] + def content_section? + content.section? && !content.feedback_question? end # @return [Boolean] def wip? Rails.application.preview? || Rails.env.test? end + + # @note only used if a skippable question follows a non-question + # @return [Boolean] + def skip_next_question? + user.skip_question?(next_item) + end + + # @return [Training::Page, Training::Question, Training::Video] + def next_next_item + content.with_parent(mod).next_next_item + end + + # @return [Training::Page, Training::Question, Training::Video] + def next_item + content.with_parent(mod).next_item + end end diff --git a/app/decorators/pagination_decorator.rb b/app/decorators/pagination_decorator.rb index 1960fac0f..0e5bb9e0a 100644 --- a/app/decorators/pagination_decorator.rb +++ b/app/decorators/pagination_decorator.rb @@ -10,6 +10,8 @@ class PaginationDecorator # @return [String] def heading + return I18n.t('summary_intro.heading') if content.thankyou? + content.section_content.first.heading end diff --git a/app/decorators/previous_page_decorator.rb b/app/decorators/previous_page_decorator.rb new file mode 100644 index 000000000..1a688b9d4 --- /dev/null +++ b/app/decorators/previous_page_decorator.rb @@ -0,0 +1,78 @@ +# +# Button or link labels to the previous page +# @see [LinkHelper#link_to_previous] +# +class PreviousPageDecorator + extend Dry::Initializer + + # @!attribute [r] user + # @return [User, Guest] + option :user, Types.Instance(User) | Types.Instance(Guest), required: true + # @!attribute [r] mod + # @return [Course, Training::Module] + option :mod, Types::Parent, required: true + # @!attribute [r] content + # @return [Training::Page, Training::Question, Training::Video] + option :content, Types::TrainingContent, required: true + + # @return [String] + def name + if skip_previous_question? + previous_previous_item.name + elsif feedback_not_started? + mod.feedback_questions.first.previous_item.name + else + previous_item.name + end + end + + # @return [String] + def style + content_section? ? 'section-intro-previous-button' : 'govuk-button--secondary' + end + + # @see [Pagination] + # @return [String] + def text + label[:previous] + end + +private + + # @return [Hash=>Symbol] + def label + I18n.t(:previous_page) + end + + # @return [Boolean] + def content_section? + content.section? && !content.feedback_question? + end + + # @return [Boolean] + def skip_previous_question? + user.skip_question?(previous_item) + end + + # @return [Boolean] + def answered?(question) + return false unless question.feedback_question? + + user.response_for_shared(question, mod).responded? + end + + # @return [Boolean] + def feedback_not_started? + content.thankyou? && !answered?(previous_item) + end + + # @return [Training::Page, Training::Question, Training::Video] + def previous_previous_item + content.with_parent(mod).previous_previous_item + end + + # @return [Training::Page, Training::Question, Training::Video] + def previous_item + content.with_parent(mod).previous_item + end +end diff --git a/app/forms/form_builder.rb b/app/forms/form_builder.rb index 7df51912c..dd2f9f05d 100644 --- a/app/forms/form_builder.rb +++ b/app/forms/form_builder.rb @@ -1,3 +1,6 @@ +# @see https://govuk-form-builder.netlify.app +# +# class FormBuilder < GOVUKDesignSystemFormBuilder::FormBuilder def govuk_text_field(attribute_name, options = {}) super(attribute_name, **options.reverse_merge(width: 'two-thirds')) @@ -11,6 +14,7 @@ def govuk_password_field(attribute_name, options = {}) super(attribute_name, **options.reverse_merge(width: 'two-thirds')) end + # @return [String] def terms_and_conditions_check_box govuk_check_box :terms_and_conditions_agreed_at, Time.zone.now, @@ -22,25 +26,77 @@ def terms_and_conditions_check_box end # @param option [Training::Answer::Option] + # @return [String] def question_radio_button(option) govuk_radio_button :answers, option.id, label: { text: option.label }, - link_errors: true, disabled: option.disabled?, checked: option.checked? end # @param option [Training::Answer::Option] + # @option text [String] + # @option more [Boolean] + # @return [String] + def other_radio_button(option, text:, more:) + govuk_radio_button :answers, + option.id, + label: { text: option.label }, + disabled: option.disabled?, + checked: option.checked? do + if more + govuk_text_area :text_input, label: { text: text } + else + govuk_text_field :text_input, label: { text: text } + end + end + end + + # @option checked [Boolean] + # @option text [String] + # @return [String] + def or_radio_button(text:, checked:) + govuk_radio_button :answers, + 0, + label: { text: text }, + checked: checked + end + + # @option checked [Boolean] + # @option text [String] + # @return [String] + def or_checkbox_button(text:, checked:) + govuk_check_box :answers, + 0, + label: { text: text }, + checked: checked + end + + # @param option [Training::Answer::Option] + # @return [String] def question_check_box(option) govuk_check_box :answers, option.id, label: { text: option.label }, - link_errors: true, disabled: option.disabled?, checked: option.checked? end + # @param option [Training::Answer::Option] + # @option text [String] + # @return [String] + def other_check_box(option, text:) + govuk_check_box :answers, + option.id, + label: { text: option.label }, + disabled: option.disabled?, + checked: option.checked? do + govuk_text_field :text_input, label: { text: text } + end + end + + # @return [String] def select_trainee_setting govuk_collection_select :setting_type_id, Trainee::Setting.all, :name, :title, @@ -52,6 +108,7 @@ def select_trainee_setting form_group: { classes: %w[data-hj-suppress] } end + # @return [String] def select_trainee_authority govuk_collection_select :local_authority, Trainee::Authority.all, :name, :name, @@ -63,6 +120,7 @@ def select_trainee_authority form_group: { classes: %w[data-hj-suppress] } end + # @return [String] def select_trainee_experience govuk_collection_radio_buttons :early_years_experience, Trainee::Experience.all, :id, :name, @@ -71,6 +129,8 @@ def select_trainee_experience form_group: { classes: %w[data-hj-suppress] } end + # @param field [Symbol] + # @return [String] def opt_in_out(field) govuk_collection_radio_buttons field, FormOption.build(field), :id, :name, diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index d15b39d35..1f090dd86 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -57,4 +57,9 @@ def html_title(*parts) def calculate_module_state CalculateModuleState.new(user: current_user).call end + + # @return [Training::Question] feedback skippable + def user_research_question + Course.config.feedback.find(&:skippable?) + end end diff --git a/app/helpers/link_helper.rb b/app/helpers/link_helper.rb index 84afac311..246376581 100644 --- a/app/helpers/link_helper.rb +++ b/app/helpers/link_helper.rb @@ -40,17 +40,11 @@ def link_to_next # @return [String] previous page or module overview def link_to_previous - path = - if content.interruption_page? - training_module_path(mod.name) - else - training_module_page_path(mod.name, content.previous_item.name) - end - - style = content.section? ? 'section-intro-previous-button' : 'govuk-button--secondary' + path = content.interruption_page? ? training_module_path(mod.name) : training_module_page_path(mod.name, previous_page.name) - govuk_button_link_to 'Previous', path, - class: style, + govuk_button_link_to previous_page.text, path, + id: 'previous-action', + class: previous_page.style, aria: { label: t('pagination.previous') } end @@ -72,13 +66,27 @@ def link_to_retake_or_results(mod) end end + # @return [String] + def link_to_skip_feedback + govuk_link_to t('links.feedback.skip'), training_module_page_path(mod.name, mod.thankyou_page.name) + end + # @return [NextPageDecorator] def next_page NextPageDecorator.new( user: current_user, mod: mod, content: content, - assessment: assessment_progress_service(mod), + assessment: (mod.is_a?(Training::Module) ? assessment_progress_service(mod) : nil), + ) + end + + # @return [PreviousPageDecorator] + def previous_page + PreviousPageDecorator.new( + user: current_user, + mod: mod, + content: content, ) end end diff --git a/app/models/concerns/content_types.rb b/app/models/concerns/content_types.rb index 47426ed61..b7a1f8d5f 100644 --- a/app/models/concerns/content_types.rb +++ b/app/models/concerns/content_types.rb @@ -22,7 +22,17 @@ def topic_intro? # @return [Boolean] def is_question? - formative_question? || summative_question? || confidence_question? + factual_question? || opinion_question? + end + + # @return [Boolean] + def opinion_question? + confidence_question? || feedback_question? + end + + # @return [Boolean] + def factual_question? + formative_question? || summative_question? end # @return [Boolean] @@ -79,6 +89,11 @@ def confidence_question? page_type.eql?('confidence') end + # @return [Boolean] + def feedback_question? + page_type.eql?('feedback') + end + # @return [Boolean] def thankyou? page_type.eql?('thankyou') diff --git a/app/models/concerns/pagination.rb b/app/models/concerns/pagination.rb index 794ca82f8..c1759ea8d 100644 --- a/app/models/concerns/pagination.rb +++ b/app/models/concerns/pagination.rb @@ -3,7 +3,7 @@ module Pagination # @return [Boolean] def section? - submodule_intro? || summary_intro? || certificate? + submodule_intro? || summary_intro? || feedback_intro? || certificate? end # @return [Boolean] @@ -32,26 +32,54 @@ def next_item parent.page_by_id(next_item_id) || self end + # @return [nil, Training::Page, Training::Video, Training::Question] + def next_next_item + parent.page_by_id(next_next_item_id) + end + + # @return [nil, Training::Page, Training::Video, Training::Question] + def previous_previous_item + parent.page_by_id(previous_previous_item_id) + end + # @return [String] def previous_item_id parent.pages[content_index - 1].id end + # @return [String, nil] + def previous_previous_item_id + parent.pages[content_index - 2]&.id + end + # @return [String, nil] def next_item_id parent.pages[content_index + 1]&.id end + # @return [String, nil] + def next_next_item_id + parent.pages[content_index + 2]&.id + end + # @return [Array] def section_content - parent.content_sections.fetch(submodule) + if parent.is_a? Training::Module # OPTIMIZE: introduce parent check predicates? + parent.content_sections.fetch(submodule) + else + Course.config.pages + end end # TODO: duplicated in overview decorator #fetch_submodule_topic # # @return [Array] def subsection_content - parent.content_subsections.fetch([submodule, topic]) + if parent.is_a? Training::Module + parent.content_subsections.fetch([submodule, topic]) + else + Course.config.pages + end end # @return [nil, Integer] @@ -91,6 +119,11 @@ def position_within(collection) private + # @return [Boolean] + def feedback_intro? + feedback_question? && first_feedback? + end + # @return [Integer] def content_index parent.pages.rindex(self) diff --git a/app/models/course.rb b/app/models/course.rb index 66b9f81cc..ded14b2b5 100644 --- a/app/models/course.rb +++ b/app/models/course.rb @@ -15,11 +15,45 @@ def self.content_type_id # @return [Course] def self.config - fetch_or_store('course') { first } + fetch_or_store(to_key('course')) { first } + end + + # @return [String] mod.name + def name + 'course' + end + + # @return [Array] mod.content_sections + def content_sections + Types::EMPTY_ARRAY + end + + # @return [Array] mod.content_subsections + def content_subsections + Types::EMPTY_ARRAY + end + + # @return [Array] + def pages + feedback.to_a.map do |question| + question.define_singleton_method(:parent) { Course.config } + question + end end # @return [Array] - def feedback - super.to_a + def feedback_questions + pages.select(&:feedback_question?) + end + + # @see Pagination + # @return [Training::Question] + def page_by_id(id) + pages.find { |page| page.id.eql?(id) } + end + + # @return [Training::Question] + def page_by_name(name) + pages.find { |page| page.name.eql?(name) } end end diff --git a/app/models/data_analysis/guest_feedback_scores.rb b/app/models/data_analysis/guest_feedback_scores.rb new file mode 100644 index 000000000..661d40e95 --- /dev/null +++ b/app/models/data_analysis/guest_feedback_scores.rb @@ -0,0 +1,38 @@ +module DataAnalysis + class GuestFeedbackScores + include ToCsv + + class << self + # @return [Array] + def column_names + %w[ + Guest + Question + Answers + Created + Updated + ] + end + + # @return [Array Mixed}>] + def dashboard + Response.visitor.feedback.order(:visit_id, :question_name).select( + :visit_id, + :question_name, + :answers, + :created_at, + :updated_at, + ).map do |user| + decorator.call user.attributes.symbolize_keys.except(:id) + end + end + + private + + # @return [CoercionDecorator] + def decorator + @decorator ||= CoercionDecorator.new + end + end + end +end diff --git a/app/models/data_analysis/module_feedback_forms.rb b/app/models/data_analysis/module_feedback_forms.rb new file mode 100644 index 000000000..970884c63 --- /dev/null +++ b/app/models/data_analysis/module_feedback_forms.rb @@ -0,0 +1,27 @@ +module DataAnalysis + class ModuleFeedbackForms + include ToCsv + + class << self + # @return [Array] + def column_names + %w[ + Module + Started + Completed + ] + end + + # @return [Array] + def dashboard + Training::Module.live.map do |mod| + { + mod: mod.title, + started: Event.where_module(mod.name).feedback_start.count, + completed: Event.where_module(mod.name).feedback_complete.count, + } + end + end + end + end +end diff --git a/app/models/data_analysis/modules_per_month.rb b/app/models/data_analysis/modules_per_month.rb index cefe79fa5..32b276a13 100644 --- a/app/models/data_analysis/modules_per_month.rb +++ b/app/models/data_analysis/modules_per_month.rb @@ -31,7 +31,7 @@ def dashboard # @return [Hash] def assessments_by_month if Rails.application.migrated_answers? - Assessment.all.group_by { |assessment| assessment.completed_at.strftime('%B %Y') } + Assessment.complete.group_by { |assessment| assessment.completed_at.strftime('%B %Y') } else UserAssessment.all.group_by { |assessment| assessment.created_at.strftime('%B %Y') } end diff --git a/app/models/data_analysis/user_feedback_scores.rb b/app/models/data_analysis/user_feedback_scores.rb new file mode 100644 index 000000000..25daad6a3 --- /dev/null +++ b/app/models/data_analysis/user_feedback_scores.rb @@ -0,0 +1,59 @@ +module DataAnalysis + class UserFeedbackScores + include ToCsv + + class << self + # @return [Array] + def column_names + [ + 'User ID', + 'Role', + 'Custom Role', + 'Setting', + 'Custom Setting', + 'Local Authority', + 'Years Experience', + 'Module', + 'Question', + 'Answers', + 'Created', + 'Updated', + ] + end + + # @return [Array Mixed}>] + def dashboard + User.with_feedback.order(:user_id, :question_name).select(*agreed_attributes).map do |user| + decorator.call user.attributes.symbolize_keys.except(:id) + end + end + + private + + # @return [CoercionDecorator] + def decorator + @decorator ||= CoercionDecorator.new + end + + # @note Personally identifiable information must not be revealed + # + # @return [Array] + def agreed_attributes + %w[ + user_id + role_type + role_type_other + setting_type + setting_type_other + local_authority + early_years_experience + training_module + question_name + answers + responses.created_at + responses.updated_at + ] + end + end + end +end diff --git a/app/models/event.rb b/app/models/event.rb index 7afd171ac..cf43b50fc 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -37,6 +37,9 @@ class Event < ApplicationRecord scope :module_complete, -> { where(name: 'module_complete') } scope :confidence_check_complete, -> { where(name: 'confidence_check_complete') } + scope :feedback_start, -> { where(name: 'feedback_start') } + scope :feedback_complete, -> { where(name: 'feedback_complete') } + # @param mod_names [Array] filter by Training::Module names scope :where_module, lambda { |*mod_names| where("properties -> 'training_module_id' ?| array[:values]", values: mod_names) diff --git a/app/models/guest.rb b/app/models/guest.rb new file mode 100644 index 000000000..b9ff346c1 --- /dev/null +++ b/app/models/guest.rb @@ -0,0 +1,66 @@ +# +# Fallback "current_user" object for visitor feedback forms +# +class Guest < Dry::Struct + # @!attribute [r] visit + # @return [Visit] + attribute :visit, Types.Instance(Visit) + + # @return [String] + delegate :visit_token, to: :visit + + # @return [Boolean] + def guest? + true + end + + # @return [Boolean] + def course_started? + false + end + + # @return [Boolean] + def profile_updated? + false + end + + # @return [nil] + def user_research_response + nil + end + + # @param question [Training::Question] + # @return [Boolean] + def skip_question?(question) + question.skippable? || question.name.eql?('prevent-from-completing-training') + end + + # @param content [Training::Question] feedback questions + # @param mod [Course] + # @return [Response] + def response_for_shared(content, mod = Course.config) + responses.find_or_initialize_by( + question_type: content.page_type, + question_name: content.name, + training_module: mod.name, + visit_id: visit.id, + ) + end + + # @return [Boolean] + def completed_course_feedback? + guest_questions.count.eql?(responses.count) + end + +private + + # @return [Array] + def guest_questions + Course.config.feedback_questions.reject { |question| skip_question?(question) } + end + + # @return [Response::ActiveRecord_Relation] + def responses + Response.course_feedback.where(visit_id: visit.id) + end +end diff --git a/app/models/response.rb b/app/models/response.rb index f902d7990..687155399 100644 --- a/app/models/response.rb +++ b/app/models/response.rb @@ -4,14 +4,19 @@ class Response < ApplicationRecord include ToCsv - belongs_to :user + belongs_to :user, optional: true belongs_to :assessment, optional: true + belongs_to :visit, optional: true - validates :answers, presence: true + validates :training_module, presence: true + validates :question_type, inclusion: { in: %w[formative summative confidence feedback] } + validates :answers, presence: true, unless: -> { text_input_only? } scope :incorrect, -> { where(correct: false) } scope :correct, -> { where(correct: true) } + scope :visitor, -> { where(user_id: nil) } + scope :ungraded, -> { where(graded: false) } scope :graded, -> { where(graded: true) } @@ -19,12 +24,17 @@ class Response < ApplicationRecord scope :summative, -> { where(question_type: 'summative') } scope :confidence, -> { where(question_type: 'confidence') } scope :feedback, -> { where(question_type: 'feedback') } + scope :course_feedback, -> { feedback.where(training_module: 'course') } delegate :to_partial_path, :legend, to: :question - # @return [Training::Module] + # @return [Training::Module, Course] def mod - Training::Module.by_name(training_module) + if training_module.eql?('course') + Course.config + else + Training::Module.by_name(training_module) + end end # @return [Training::Question] @@ -36,11 +46,34 @@ def question def options if question.formative_question? || assessment&.graded? question.options(checked: answers, disabled: responded?) + + # the numeric value for "Or" could be all options plus one but "zero" is consistent + elsif question.feedback_question? && !question.has_other? && question.has_or? && text_input.present? + question.options(checked: [0]) + else question.options(checked: answers) end end + # @return [Boolean] + def checked_other? + question.has_other? && answers.include?(question.options.last.id) + end + + # @see Question#options + # @return [Boolean] + def checked_or? + question.has_or? && answers.include?(0) + end + + # Validate that an option is selected unless question only expects text. + # + # @return [Boolean] + def text_input_only? + question.only_text? + end + # @return [Boolean] def archived? archived @@ -48,12 +81,12 @@ def archived? # @return [Boolean] def responded? - answers.any? + answers.any? || text_input.present? end # @return [Boolean] def correct? - question.confidence_question? || question.correct_answers.eql?(answers) + question.opinion_question? || question.correct_answers.eql?(answers) end # @return [Boolean] diff --git a/app/models/training/answer.rb b/app/models/training/answer.rb index 5b70ff72f..6c695f52a 100644 --- a/app/models/training/answer.rb +++ b/app/models/training/answer.rb @@ -58,6 +58,7 @@ def options(disabled: false, checked: []) correct: value, disabled: disabled, checked: checked.include?(order), + last: json.size.eql?(order), ) end end @@ -87,10 +88,12 @@ class Option < Dry::Struct attribute :correct, Types::Params::Bool.fallback(false) attribute :disabled, Types::Params::Bool.default(false) attribute :checked, Types::Params::Bool.default(false) + attribute :last, Types::Params::Bool.default(false) alias_method :correct?, :correct alias_method :checked?, :checked alias_method :disabled?, :disabled + alias_method :last?, :last # @return [Hash{Symbol => nil, String}] def schema diff --git a/app/models/training/content.rb b/app/models/training/content.rb index bf6a2701d..8a5431196 100644 --- a/app/models/training/content.rb +++ b/app/models/training/content.rb @@ -16,6 +16,13 @@ def parent @parent ||= Training::Module.by_content_id(id) end + # @param mod [Course, Training::Module] + # @return [Training::Page, Training::Video, Training::Question] + def with_parent(mod) + @parent = mod + self + end + # @return [String] def debug_summary <<~SUMMARY @@ -57,5 +64,10 @@ def title def notes? (topic_intro? || text_page?) && notes end + + # @return [Boolean] + def skippable? + false + end end end diff --git a/app/models/training/module.rb b/app/models/training/module.rb index fcacbc98f..53ca67f3b 100644 --- a/app/models/training/module.rb +++ b/app/models/training/module.rb @@ -60,8 +60,10 @@ def self.by_name(name) end end + # Module content must not be shared and have unique names + # # @param id [String] - # @return [Training::Module] + # @return [Training::Module] first result def self.by_content_id(id) ordered.find { |mod| mod.content.find { |content| content.id.eql?(id) } } end @@ -109,7 +111,7 @@ def content # @return [Hash{ Integer => Array }] def content_sections - content.slice_before(&:section?).each.with_index(1).to_h.invert + @content_sections ||= content.slice_before(&:section?).each.with_index(1).to_h.invert end # @return [Hash{ Array => Array }] @@ -135,6 +137,7 @@ def submodule_count # Selects from ordered array # + # @param id [String] # @return [Training::Page, Training::Video, Training::Question] def page_by_id(id) pages.find { |page| page.id.eql?(id) } @@ -142,6 +145,7 @@ def page_by_id(id) # Selects from ordered array # + # @param name [String] # @return [Training::Page, Training::Video, Training::Question] def page_by_name(name) pages.find { |page| page.name.eql?(name) } @@ -259,6 +263,11 @@ def confidence_questions content.select(&:confidence_question?) end + # @return [Array] + def feedback_questions + content.select(&:feedback_question?) + end + # @param text [String] # @return [Array] def answers_with(text) diff --git a/app/models/training/question.rb b/app/models/training/question.rb index a017a0e35..0c9d6c9df 100644 --- a/app/models/training/question.rb +++ b/app/models/training/question.rb @@ -17,13 +17,64 @@ def answer # @return [String] powered by JSON not type def to_partial_path partial = multi_select? ? 'check_boxes' : 'radio_buttons' - partial = "learning_#{partial}" if formative_question? - "training/questions/#{partial}" + + if feedback_question? + partial = 'text_area' if only_text? + "feedback/#{partial}" + else + partial = "learning_#{partial}" if formative_question? + "training/questions/#{partial}" + end end + # Factual questions: dynamic based on available correct options + # Opinion questions: + # - Feedback (default: false) + # - Confidence (always: false) + # # @return [Boolean] def multi_select? - confidence_question? ? false : answer.multi_select? + if feedback_question? + !!multi_select + elsif confidence_question? + false + else + answer.multi_select? + end + end + + # @return [Boolean] textarea by itself no validations + def only_text? + options.empty? && has_more? + end + + # @return [Boolean] "Could you give use reasons for your answer" + def and_text? + options.present? && !multi_select? && has_more? + end + + # Turns the last "other" option input field into a textarea + # + # @return [Boolean] + def has_more? + feedback_question? && more.present? + end + + # @return [Boolean] + def has_other? + feedback_question? && other.present? + end + + # Additional "Or" option is appended and given index zero + # + # @return [Boolean] + def has_or? + feedback_question? && self.or.present? + end + + # @return [Boolean] default: false + def skippable? + feedback_question? && !!skippable end # @return [Boolean] event tracking @@ -41,6 +92,16 @@ def last_assessment? parent.summative_questions.last.eql?(self) end + # @return [Boolean] + def first_feedback? + parent.feedback_questions.first.eql?(self) + end + + # @return [Boolean] + def last_feedback? + parent.feedback_questions.last.eql?(self) + end + # @return [Boolean] def true_false? return false if multi_select? @@ -74,6 +135,8 @@ def legend #{body} LEGEND + elsif feedback_question? + body.to_s else "#{body} (Select one answer)" end diff --git a/app/models/user.rb b/app/models/user.rb index 952566938..1d6cbbafa 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -66,13 +66,16 @@ def test_user? devise :database_authenticatable, :rememberable, :lockable, :timeoutable, :omniauthable, omniauth_providers: [:openid_connect] - has_many :responses - has_many :user_answers - if Rails.application.migrated_answers? + has_many :responses + has_many :feedback_responses, -> { where(question_type: 'feedback') }, class_name: 'Response' has_many :assessments + + # feedback + scope :with_feedback, -> { joins(:responses).merge(Response.feedback) } else has_many :user_assessments + has_many :user_answers end has_many :visits @@ -209,6 +212,36 @@ def gov_one? !gov_one_id.nil? end + # @return [Boolean] + def guest? + false + end + + # @see FeedbackPaginationDecorator + # + # Checks all feedback forms for answers + # + # @param question [Training::Question] + # @return [Boolean] + def skip_question?(question) + return false unless question.skippable? + + (Training::Module.live.to_a << Course.config).any? do |form| + response_for_shared(question, form).responded? + end + end + + # @see ResponsesController#response_params + # @param content [Training::Question] + # @return [UserAnswer, Response] + def response_for_shared(content, mod) + responses.find_or_initialize_by( + question_type: content.page_type, + question_name: content.name, + training_module: mod.name, + ) + end + # @see ResponsesController#response_params # @param content [Training::Question] # @return [UserAnswer, Response] @@ -257,12 +290,6 @@ def email_delivery_status notify_callback.to_h.symbolize_keys.fetch(:status, 'unknown') end - # @return [String] - def password_last_changed - timestamp = password_changed_events&.last&.time || created_at - timestamp.to_date&.to_formatted_s(:rfc822) - end - # @return [CourseProgress] course activity query interface def course @course ||= CourseProgress.new(user: self) @@ -354,15 +381,17 @@ def role_other? end # @return [Boolean] - def role_applicable? - role_type != 'Not applicable' - end - - # return [Boolean] def training_emails_recipient? training_emails || training_emails.nil? end + # @return [Boolean] + def research_participant? + preference = user_research_response.nil? ? false : user_research_response.answers.eql?([1]) + update(research_participant: preference) + research_participant + end + # @return [Boolean] def private_beta_registration_complete? !!private_beta_registration_complete @@ -411,6 +440,7 @@ def redact! password: 'RedactedUser12!@', notify_callback: nil) + responses.feedback.update_all(text_input: nil) mail_events.destroy_all notes.destroy_all end @@ -421,6 +451,11 @@ def dashboard_row data_attributes.dup.merge(module_ttc) end + # @return [Boolean] + def profile_updated? + events.any? && events.last.name.eql?('profile_page') + end + # @return [Boolean] def completed_available_modules? available_modules = ModuleRelease.pluck(:name) @@ -439,6 +474,25 @@ def content_changes @content_changes ||= ContentChanges.new(user: self) end + # @return [Boolean] + def completed_course_feedback? + responses.course_feedback.count.eql? Course.config.feedback_questions.count + end + + # @return [String] + def visit_token + visits.last.visit_token + end + + def feedback_attributes + data_attributes + end + + # @return [Response] + def user_research_response + responses.feedback.find { |response| response.question.skippable? } + end + private # @return [Hash] diff --git a/app/services/content_integrity.rb b/app/services/content_integrity.rb index 726ca4d33..24f0de719 100644 --- a/app/services/content_integrity.rb +++ b/app/services/content_integrity.rb @@ -1,33 +1,26 @@ -# Validate whether a module's content meets minimum functional requirements +# Validate module content meets minimum functional requirements # class ContentIntegrity extend Dry::Initializer option :module_name, Types::String - # NB: Able to be validated in the CMS editor - # # @return [Hash{Symbol=>String}] valid as upcoming module MODULE_VALIDATIONS = { upcoming: 'Missing upcoming text', about: 'Missing about text', description: 'Missing description text', - criteria: 'Missing criteria list', outcomes: 'Missing outcomes list', - duration: 'Missing duration number', position: 'Missing position number', - thumbnail: 'Missing thumbnail image', }.freeze # @return [Hash{Symbol=>String}] valid as released module CONTENT_VALIDATIONS = { - # type text: 'Missing text pages', video: 'Missing video pages', - formative: 'Missing formative questions', assessment_intro: 'Missing assessment intro page', confidence_intro: 'Missing confidence intro page', recap: 'Missing recap page', @@ -41,11 +34,12 @@ class ContentIntegrity thankyou: 'Penultimate page is wrong type', certificate: 'Last page is wrong type', - # type and frequency + # questions + formative: 'Missing formative questions', + feedback: 'Missing feedback questions', summative: 'Insufficient summative questions', - confidence: 'Insufficient confidence checks', - - question_answers: 'Question answers are incorrectly formatted', # TODO: which question? + confidence: 'Insufficient confidence questions', + factual: 'Factual questions have sufficient options', }.freeze # @return [nil] @@ -56,7 +50,7 @@ def call log "#{module_name.upcase}: " + (valid? ? 'pass' : 'fail') end - # @return [Boolean] Validate modules with content + # @return [Boolean] def valid? (module_results + content_results).all? && mod.pages? end @@ -162,8 +156,8 @@ def video? end # @return [Boolean] - def question_answers? - mod.questions.all? { |question| question.answer.valid? } + def factual? + mod.questions.select(&:factual_question?).all? { |question| question.answer.valid? } end # @return [Boolean] @@ -171,6 +165,11 @@ def formative? mod.formative_questions.any? end + # @return [Boolean] + def feedback? + mod.feedback_questions.any? + end + # 'Brain development and how children learn' has fewest # @return [Boolean] def confidence? diff --git a/app/services/dashboard.rb b/app/services/dashboard.rb index 12243b034..ea2653619 100644 --- a/app/services/dashboard.rb +++ b/app/services/dashboard.rb @@ -34,6 +34,9 @@ class Dashboard { model: 'DataAnalysis::UserModuleCompletion', folder: 'nonlinear', file: 'user_module_completion' }, { model: 'DataAnalysis::UserModuleCompletionCount', folder: 'nonlinear', file: 'user_module_completions_count' }, { model: 'DataAnalysis::ReturningUsers', folder: 'nonlinear', file: 'returning_users' }, + { model: 'DataAnalysis::ModuleFeedbackForms', folder: 'feedback', file: 'feedback_forms' }, + { model: 'DataAnalysis::UserFeedbackScores', folder: 'feedback', file: 'feedback_user_scores' }, + { model: 'DataAnalysis::GuestFeedbackScores', folder: 'feedback', file: 'feedback_guest_scores' }, ].freeze # @return [String] 30-06-2022-09-30 diff --git a/app/services/module_progress.rb b/app/services/module_progress.rb index c66f93516..fb9e6c403 100644 --- a/app/services/module_progress.rb +++ b/app/services/module_progress.rb @@ -1,10 +1,11 @@ +# OPTIMIZE: N+1 query +# # Overall module progress: # - whether a page was visited -# - whether a page was skipped +# - whether any/all/no pages in a section were visited # - whether key events have been recorded (start/complete) # - the last page visited # - the furthest page visited -# - the furthest page visited # class ModuleProgress extend Dry::Initializer @@ -39,24 +40,9 @@ def furthest_page # @return [Training::Page, Training::Question, Training::Video] def resume_page - # unvisited.first&.previous_item || mod.first_content_page mod.page_by_name(milestone) || mod.first_content_page end - # Identify new content that has not been seen and would effect module state - # - # @see FillPageViews task - # @return [Boolean] - def skipped? - if unvisited.none? - false - elsif completed? && unvisited.any? # seen last content page but has gaps - true - elsif gaps? - true - end - end - # @see CourseProgress # @return [Boolean] def completed? @@ -124,12 +110,6 @@ def successful_attempt? end end - # In progress modules with new pages that have been skipped - # @return [Boolean] - def gaps? - (unvisited.first.id..unvisited.last.id).count != unvisited.map(&:id).count - end - # @return [Array] def visited mod.content.select { |page| visited?(page) } diff --git a/app/views/errors/service_unavailable.html.slim b/app/views/errors/service_unavailable.html.slim new file mode 100644 index 000000000..e595c414c --- /dev/null +++ b/app/views/errors/service_unavailable.html.slim @@ -0,0 +1,7 @@ +- content_for :page_title do + = html_title 'Service unavailable' + +.govuk-grid-row + .govuk-grid-column-two-thirds + h1.govuk-heading-xl + | Service unavailable diff --git a/app/views/errors/unprocessable_entity.html.slim b/app/views/errors/unprocessable_entity.html.slim deleted file mode 100644 index 9e874b70e..000000000 --- a/app/views/errors/unprocessable_entity.html.slim +++ /dev/null @@ -1,12 +0,0 @@ --# FIXME: add missing 422 page content - -/ .govuk-grid-row - .govuk-grid-column-two-thirds - h1.govuk-heading-xl - | Page not found - p.govuk-body - | If you typed the web address, check it is correct. - p.govuk-body - | If you pasted the web address, check you copied the entire address. - p.govuk-body - | If the web address is correct or you selected a link or button, contact Early years child development training to report a fault with the service. diff --git a/app/views/feedback/_check_boxes.html.slim b/app/views/feedback/_check_boxes.html.slim new file mode 100644 index 000000000..6b0629d13 --- /dev/null +++ b/app/views/feedback/_check_boxes.html.slim @@ -0,0 +1,16 @@ += f.govuk_check_boxes_fieldset :answers, legend: { text: response.legend } do + + - response.options.each do |option| + + - if content.has_other? && option.last? + = f.other_check_box(option, text: content.other) + - else + = f.question_check_box(option) + + - if content.has_or? && option.last? + = f.govuk_radio_divider 'Or' + = f.or_checkbox_button(text: content.or, checked: response.checked_or?) + + + - if content.has_more? + = f.govuk_text_area :text_input, label: { text: t('feedback.reasons'), class: 'govuk-!-font-weight-bold govuk-!-margin-top-8 govuk-!-margin-bottom-4' } diff --git a/app/views/feedback/_cta.html.slim b/app/views/feedback/_cta.html.slim new file mode 100644 index 000000000..b5f7bd8cd --- /dev/null +++ b/app/views/feedback/_cta.html.slim @@ -0,0 +1,7 @@ +#feedback-cta + .dfe-width-container + .govuk-grid-row + .govuk-grid-column-full + = m('feedback.cta') + + = govuk_link_to 'Provide feedback', feedback_index_path, class: 'govuk-button' diff --git a/app/views/feedback/_radio_buttons.html.slim b/app/views/feedback/_radio_buttons.html.slim new file mode 100644 index 000000000..f4410cf58 --- /dev/null +++ b/app/views/feedback/_radio_buttons.html.slim @@ -0,0 +1,21 @@ += f.govuk_radio_buttons_fieldset :answers, legend: { text: response.legend } do + = f.hidden_field :answers + + - if content.skippable? + p.govuk-hint = t('feedback.research') + + - response.options.each do |option| + + - if content.has_other? && option.last? + = f.other_radio_button(option, text: content.other, more: content.has_more?) + + - elsif content.has_or? && option.last? + = f.question_radio_button(option) + = f.govuk_radio_divider 'Or' + = f.or_radio_button(text: content.or, checked: response.checked_or?) + + - else + = f.question_radio_button(option) + + - if content.has_more? && !content.has_other? + = f.govuk_text_area :text_input, label: { text: t('feedback.reasons'), class: 'govuk-!-font-weight-bold govuk-!-margin-top-8 govuk-!-margin-bottom-4' } diff --git a/app/views/feedback/_text_area.html.slim b/app/views/feedback/_text_area.html.slim new file mode 100644 index 000000000..9f349b208 --- /dev/null +++ b/app/views/feedback/_text_area.html.slim @@ -0,0 +1,2 @@ += f.govuk_fieldset legend: { text: content.legend } do + = f.govuk_text_area :text_input, rows: 9, label: nil, aria: { label: content.legend } diff --git a/app/views/feedback/index.html.slim b/app/views/feedback/index.html.slim new file mode 100644 index 000000000..684d11145 --- /dev/null +++ b/app/views/feedback/index.html.slim @@ -0,0 +1,23 @@ +- content_for :page_title do + = html_title 'Feedback' + +.govuk-grid-row + .govuk-grid-column-full + + - if current_user&.completed_course_feedback? + = m('feedback.complete') + - else + = m('feedback.intro', contact_us: Rails.application.credentials.contact_us) + + hr.govuk-section-break.govuk-section-break--l.govuk-section-break--visible class='govuk-!-margin-top-4' + + .govuk-button-group + - if current_user.completed_course_feedback? + + - unless current_user.guest? + = govuk_button_link_to t('previous_page.previous'), my_modules_path, secondary: true + + = govuk_button_link_to t('links.feedback.update'), feedback_path(mod.pages.first.name) + + - else + = govuk_button_link_to t('next_page.next'), feedback_path(mod.pages.first.name) diff --git a/app/views/feedback/show.html.slim b/app/views/feedback/show.html.slim new file mode 100644 index 000000000..cd9aa0a2e --- /dev/null +++ b/app/views/feedback/show.html.slim @@ -0,0 +1,27 @@ +- content_for :page_title do + = html_title content.name + += render 'training/questions/debug' + += form_with model: current_user_response, url: feedback_path, method: :patch do |f| + .govuk-grid-row + .govuk-grid-column-two-thirds + = govuk_back_link href: url_for(:back) + + = f.govuk_error_summary + + = render partial: content.to_partial_path, locals: { f: f }, object: current_user_response, as: :response + + .govuk-grid-column-full + hr.govuk-section-break.govuk-section-break--l.govuk-section-break--visible class='govuk-!-margin-top-4' + + .govuk-button-group + - if current_user.profile_updated? + = f.govuk_submit t('links.save') + - else + - if content.first_feedback? + = govuk_button_link_to t('previous_page.previous'), feedback_index_path, secondary: true + - else + = govuk_button_link_to t('previous_page.previous'), feedback_path(previous_page.name), secondary: true + + = f.govuk_submit t('next_page.next') diff --git a/app/views/feedback/thank_you.html.slim b/app/views/feedback/thank_you.html.slim new file mode 100644 index 000000000..795be81a2 --- /dev/null +++ b/app/views/feedback/thank_you.html.slim @@ -0,0 +1,15 @@ +- content_for :page_title do + = html_title 'Thank you' + +.govuk-grid-row + .govuk-grid-column-full + h1.govuk-heading-xl= content.heading + + = m(content.body) + + hr.govuk-section-break.govuk-section-break--l.govuk-section-break--visible class='govuk-!-margin-top-4' + + - if current_user.guest? + = govuk_button_link_to t('links.home'), root_path + - else + = govuk_button_link_to t('links.my_modules'), my_modules_path diff --git a/app/views/home/index.html.slim b/app/views/home/index.html.slim index 678d133e4..8284bf40a 100644 --- a/app/views/home/index.html.slim +++ b/app/views/home/index.html.slim @@ -4,6 +4,10 @@ - content_for :hero do = render 'hero' +- content_for :cta do + - if current_user + = render 'feedback/cta' + = render 'learning/cms_debug' = render 'debug' diff --git a/app/views/layouts/_banner.html.slim b/app/views/layouts/_banner.html.slim index 3b722519e..9741eb19a 100644 --- a/app/views/layouts/_banner.html.slim +++ b/app/views/layouts/_banner.html.slim @@ -1,2 +1,2 @@ = govuk_phase_banner(tag: { text: 'Beta' }, classes: 'noprint') do - == t 'phase_banner', link: govuk_link_to('feedback', Rails.configuration.feedback_url, target: :_blank) + == t 'phase_banner', link: govuk_link_to('feedback', feedback_index_path) diff --git a/app/views/layouts/_footer.html.slim b/app/views/layouts/_footer.html.slim index 0cf08714e..40c310c18 100644 --- a/app/views/layouts/_footer.html.slim +++ b/app/views/layouts/_footer.html.slim @@ -17,5 +17,4 @@ = govuk_link_to t('links.footer.contact_us'), Rails.application.credentials.contact_us, class: 'govuk-footer__link', target: '_blank' li.govuk-footer__list-item - -# TODO: make feedback link internal - = govuk_link_to t('links.footer.feedback'), Rails.configuration.feedback_url, target: '_blank', class: 'govuk-footer__link' + = govuk_link_to t('links.footer.feedback'), feedback_index_path, class: 'govuk-footer__link' diff --git a/app/views/layouts/application.html.slim b/app/views/layouts/application.html.slim index 058357fe1..dbaf1df74 100644 --- a/app/views/layouts/application.html.slim +++ b/app/views/layouts/application.html.slim @@ -22,4 +22,5 @@ html.govuk-template lang='en' = render 'layouts/flash' = yield + = yield :cta = render 'layouts/footer' diff --git a/app/views/layouts/hero.html.slim b/app/views/layouts/hero.html.slim index dc64dea9c..65c6b9339 100644 --- a/app/views/layouts/hero.html.slim +++ b/app/views/layouts/hero.html.slim @@ -28,4 +28,5 @@ html.govuk-template lang='en' .dfe-width-container class='govuk-!-padding-top-7 govuk-!-padding-bottom-7' = yield + = yield :cta = render 'layouts/footer' diff --git a/app/views/learning/_debug.html.slim b/app/views/learning/_debug.html.slim index ff5cf4f67..b0bb2fa2e 100644 --- a/app/views/learning/_debug.html.slim +++ b/app/views/learning/_debug.html.slim @@ -1,5 +1,3 @@ - if debug? pre.debug_dump - - current_user.course.debug_summary.each do |summary| - = summary - hr + = current_user.course.debug_summary diff --git a/app/views/learning/show.html.slim b/app/views/learning/show.html.slim index b54830f61..078ef28dc 100644 --- a/app/views/learning/show.html.slim +++ b/app/views/learning/show.html.slim @@ -8,6 +8,9 @@ | My modules p You can complete the Early years child development training in any order. However, to support your understanding of the training, you may find it helpful to complete the modules in order. +- content_for :cta do + = render 'feedback/cta' + = render 'cms_debug' #started.govuk-grid-row diff --git a/app/views/training/assessments/show.html.slim b/app/views/training/assessments/show.html.slim index 16350d541..716f85e08 100644 --- a/app/views/training/assessments/show.html.slim +++ b/app/views/training/assessments/show.html.slim @@ -37,6 +37,6 @@ hr.govuk-section-break.govuk-section-break--l.govuk-section-break--visible class = link_to_next - else - = govuk_button_link_to 'Go to My modules', my_modules_path, class: 'govuk-button--secondary' + = govuk_button_link_to 'Go to My modules', my_modules_path, secondary: true = govuk_button_link_to 'Retake test', new_training_module_assessment_path(mod.name) diff --git a/app/views/training/modules/_debug.html.slim b/app/views/training/modules/_debug.html.slim index 551f04b71..2dde998ed 100644 --- a/app/views/training/modules/_debug.html.slim +++ b/app/views/training/modules/_debug.html.slim @@ -1,5 +1,7 @@ - if debug? pre.debug_dump + = govuk_link_to 'Structure', training_module_structure_path(mod.name) + hr h4 Module = mod.debug_summary diff --git a/app/views/training/modules/show.html.slim b/app/views/training/modules/show.html.slim index a197c4c47..f5e88fc95 100644 --- a/app/views/training/modules/show.html.slim +++ b/app/views/training/modules/show.html.slim @@ -31,7 +31,9 @@ hr.govuk-section-break.govuk-section-break--m.govuk-section-break--visible.module-overview-section-break - module_progress.sections.each do |section| - = render 'section', **section + - unless section[:hide] + = render 'section', **section + /= the feedback section is not visible in the module overview = govuk_button_link_to link_to_action[1], class: 'govuk-button--start', id: 'module-call-to-action' do | #{link_to_action[0]} diff --git a/app/views/training/pages/_content.html.slim b/app/views/training/pages/_content.html.slim index 8b4ee79a0..c65d75efd 100644 --- a/app/views/training/pages/_content.html.slim +++ b/app/views/training/pages/_content.html.slim @@ -11,3 +11,6 @@ .govuk-button-group = link_to_previous = link_to_next + + - if !content.interruption_page? && content.next_item.feedback_question? + = link_to_skip_feedback diff --git a/app/views/training/pages/certificate.html.slim b/app/views/training/pages/certificate.html.slim index 015abcad8..8a224d50b 100644 --- a/app/views/training/pages/certificate.html.slim +++ b/app/views/training/pages/certificate.html.slim @@ -16,6 +16,9 @@ =< govuk_link_to 'My account', user_path | . +- content_for :cta do + = render 'feedback/cta' + .govuk-grid-row .govuk-grid-column-three-quarters-from-desktop id=('award' if pdf?) diff --git a/app/views/training/questions/_debug.html.slim b/app/views/training/questions/_debug.html.slim index ac7358407..1ae64a886 100644 --- a/app/views/training/questions/_debug.html.slim +++ b/app/views/training/questions/_debug.html.slim @@ -12,8 +12,34 @@ hr | Multiple choice: #{content.multi_select?} hr + | FEEDBACK + br + | Or: #{content.or} + br + | Other: #{content.other} + br + | More: #{content.more} + br + | User type: #{current_user&.class.name} + br + | Cookie: #{current_user.visit_token} + br + | Skippable: #{content.skippable?} + br + | Skip next question: #{current_user.skip_question?(content.next_item)} + br + | Only text input: #{content.only_text?} + br + | Additional text input: #{content.and_text?} + hr | Responded: #{current_user_response.responded?} hr + | Answered: + br + | Chosen option(s): #{current_user_response.answers} + br + | Text input: #{current_user_response.text_input} + hr | Response Options: - current_user_response.options.map do |option| br diff --git a/app/views/user/_debug.html.slim b/app/views/user/_debug.html.slim index ed975149c..c331df2db 100644 --- a/app/views/user/_debug.html.slim +++ b/app/views/user/_debug.html.slim @@ -17,3 +17,5 @@ | training_emails: #{current_user.training_emails} br | early_years_emails: #{current_user.early_years_emails} + br + | research_participant: #{current_user.research_participant} diff --git a/app/views/user/show.html.slim b/app/views/user/show.html.slim index 010c52abd..701c876a3 100644 --- a/app/views/user/show.html.slim +++ b/app/views/user/show.html.slim @@ -1,8 +1,9 @@ -= render 'debug' - - content_for :page_title do = html_title t('my_account.title') +- content_for :cta do + = render 'feedback/cta' + .govuk-grid-row .govuk-grid-column-full h1.govuk-heading-l Manage your account @@ -40,11 +41,22 @@ = govuk_summary_list do |email_preferences| - email_preferences.with_row do |row| - - row.with_key { m('my_account.email_preferences') } + - row.with_key { m('my_account.email_preferences.heading') } - row.with_value { nil } - email_preferences.with_row do |row| - - row.with_key { 'Email updates about this training course' } - - row.with_value(text: t(current_user.training_emails_recipient?, scope: 'my_account.training_emails'), classes: %w[data-hj-suppress]) - - row.with_action(text: 'Change email preferences', href: edit_training_emails_user_path, html_attributes: { id: :edit_training_emails_user }) + - row.with_key { t('my_account.email_preferences.label') } + - row.with_value(text: t(current_user.training_emails_recipient?, scope: 'my_account.email_preferences.preference'), classes: %w[data-hj-suppress]) + - row.with_action(text: t('my_account.email_preferences.link_text'), href: edit_training_emails_user_path, html_attributes: { id: :edit_training_emails_user }) + + = govuk_summary_list do |research_preferences| + - research_preferences.with_row do |row| + - row.with_key { m('my_account.research_preferences.heading') } + - row.with_value { nil } + - research_preferences.with_row do |row| + - row.with_key { t('my_account.research_preferences.label') } + - row.with_value(text: t(current_user.research_participant?, scope: 'my_account.research_preferences.preference'), classes: %w[data-hj-suppress]) + - row.with_action(text: t('my_account.research_preferences.link_text'), href: feedback_path(user_research_question.name)) + + = render 'debug' = m('my_account.closing.information') diff --git a/cms/CONTENTFUL.md b/cms/CONTENTFUL.md index 7eb828beb..f1dc41fd9 100644 --- a/cms/CONTENTFUL.md +++ b/cms/CONTENTFUL.md @@ -223,3 +223,44 @@ Information for onboarding content editors: - `API`: Application Programming Interface. - `Delivery API`: The mechanism that returns published content. - `Preview API`: The mechanism that returns both published and draft content. + + +## Question types + +Questions come in two flavours, factual questions (divided into either `formative` or `summative`) and opinion questions (divided into either `confidence` or `feedback`). + +### Feedback questions + +Feedback questions use additional model fields to control their increased functionality. + +These questions can: + +- be shared and used across multiple training modules +- have user-defined text in an optional input field or textarea +- replace radio buttons and checkboxes completely with a free text input +- be asked only once + +The different permutations are controlled using these fields: + +- answers [Array] +- or [String] +- other [String] +- more [Boolean] +- skippable [Boolean] +- multi_select [Boolean] + +In order to show a text input when the last option is selected, provide a string in the `other` field. +This string becomes the label text. +If you want to provide the user with more space to write, also turn on the `more` field. +This changes the text input to a textarea field. + +If instead you want to append a textarea field after to record the reasons for their answer, just enable `more` without `other`. + +To give an alternative `answer` separated by a divider, provide a string in the `or` field. + +If the question is a feedback question it will not dynamically use the number of correct answers +to determine if it is a radio button or checkbox. +`multi_select` defaults to off using the radio button template. + +If the question needs to be hidden once it is answered, so it is not answered again in another form, enable `skippable`. + diff --git a/cms/migrate/01-create-page.js b/cms/migrate/01-create-page.js index 64c4eda73..c73c24a6e 100644 --- a/cms/migrate/01-create-page.js +++ b/cms/migrate/01-create-page.js @@ -82,16 +82,6 @@ module.exports = function(migration) { helpText: 'All page content including sub-headings, bullet points and images.', }) - /* number */ - - page.changeFieldControl('submodule', 'builtin', 'numberEditor', { - helpText: 'Select the sub-module number the page belongs to, the second number of the page name.' - }) - - page.changeFieldControl('topic', 'builtin', 'numberEditor', { - helpText: 'Select the topic number the page belongs to, the third number in the page name.' - }) - /* toggle */ page.changeFieldControl('notes', 'builtin', 'boolean', { diff --git a/cms/migrate/02-create-question.js b/cms/migrate/02-create-question.js index bec12dac8..46c9af269 100644 --- a/cms/migrate/02-create-question.js +++ b/cms/migrate/02-create-question.js @@ -3,7 +3,7 @@ module.exports = function(migration) { const question = migration.createContentType('question', { name: 'Question', displayField: 'name', - description: 'Formative, Summative or Confidence' + description: 'Formative, Summative, Confidence or Feedback' }) /* Fields ----------------------------------------------------------------- */ @@ -34,6 +34,7 @@ module.exports = function(migration) { 'formative', 'summative', 'confidence', + 'feedback', ] } ] @@ -51,6 +52,7 @@ module.exports = function(migration) { formative: customise both summative: customise failure only (success unused) confidence: both use default + feedback: both use default */ question.createField('success_message', { @@ -76,6 +78,47 @@ module.exports = function(migration) { type: 'Object', }) + /* Feedback Only ---------------------------------------------------------- */ + + // the last option has an additional conditional text input + question.createField('other', { + name: 'Other', + type: 'Text', + }) + + // an extra option is appended with an index of zero + question.createField('or', { + name: 'Or', + type: 'Text', + }) + + /* + - increases the other input to a text area + - appends an additional textbox + - replaces options with a textbox + */ + question.createField('more', { + name: 'More', + type: 'Boolean' + }) + + /* + overrides default + ====================== + formative and summative are dynamic based off number of correct options + confidence are hard-coded + feedback are controlled by content editors + */ + question.createField('multi_select', { + name: 'Multi select', + type: 'Boolean' + }) + + question.createField('skippable', { + name: 'Skippable', + type: 'Boolean' + }) + /* Interface -------------------------------------------------------------- */ /* JSON */ @@ -98,14 +141,24 @@ module.exports = function(migration) { helpText: 'Displayed after "That’s not quite right" if the user selects the wrong answer.' }) - /* number */ + /* toggle */ + + question.changeFieldControl('multi_select', 'builtin', 'boolean', { + helpText: 'Select multiple options? (default no)', + trueLabel: 'yes', + falseLabel: 'no' + }) - question.changeFieldControl('submodule', 'builtin', 'numberEditor', { - helpText: 'Select the sub-module number the page belongs to, the second number of the page name.' + question.changeFieldControl('skippable', 'builtin', 'boolean', { + helpText: 'Hide once answered? (default no)', + trueLabel: 'yes', + falseLabel: 'no' }) - question.changeFieldControl('topic', 'builtin', 'numberEditor', { - helpText: 'Select the topic number the page belongs to, the third number in the page name.' + question.changeFieldControl('more', 'builtin', 'boolean', { + helpText: 'Allow more user input? (default no)', + trueLabel: 'yes', + falseLabel: 'no' }) } diff --git a/cms/migrate/03-create-video.js b/cms/migrate/03-create-video.js index 82b6ee1c7..53eac84dd 100644 --- a/cms/migrate/03-create-video.js +++ b/cms/migrate/03-create-video.js @@ -76,16 +76,6 @@ module.exports = function(migration) { helpText: 'Title of video.', }) - /* number */ - - video.changeFieldControl('submodule', 'builtin', 'numberEditor', { - helpText: 'Select the sub-module number the page belongs to, the second number of the page name.' - }) - - video.changeFieldControl('topic', 'builtin', 'numberEditor', { - helpText: 'Select the topic number the page belongs to, the third number in the page name.' - }) - /* markdown */ video.changeFieldControl('body', 'builtin', 'markdown', { diff --git a/cms/migrate/04-create-training-module.js b/cms/migrate/04-create-training-module.js index edc843ce4..57a6077e5 100644 --- a/cms/migrate/04-create-training-module.js +++ b/cms/migrate/04-create-training-module.js @@ -38,6 +38,7 @@ module.exports = function(migration) { required: true }) + // markdown not permitted trainingModule.createField('upcoming', { name: 'Upcoming', type: 'Text', @@ -50,7 +51,7 @@ module.exports = function(migration) { }) // markdown not permitted - trainingModule.createField('description', { // list moved to outcomes + trainingModule.createField('description', { name: 'Description', type: 'Text', required: true, @@ -61,7 +62,7 @@ module.exports = function(migration) { ] }) - trainingModule.createField('outcomes', { // list moved out of description + trainingModule.createField('outcomes', { name: 'Skills', type: 'Text', required: true @@ -74,7 +75,7 @@ module.exports = function(migration) { }) // markdown not permitted - trainingModule.createField('about', { // formerly objective + trainingModule.createField('about', { name: 'About', type: 'Text', required: true, diff --git a/config/application.rb b/config/application.rb index 2223690ee..5cee8b8a7 100644 --- a/config/application.rb +++ b/config/application.rb @@ -47,9 +47,8 @@ class Application < Rails::Application # @note Nudge mail must only run once per day config.mail_job_interval = ENV.fetch('MAIL_JOB_INTERVAL', '0 12 * * *') # Noon daily - config.user_password = ENV.fetch('USER_PASSWORD', 'Str0ngPa$$w0rd12') - config.bot_token = ENV['BOT_TOKEN'] - config.feedback_url = ENV.fetch('FEEDBACK_URL', '#FEEDBACK_URL_env_var_missing') # TODO: deprecate + config.user_password = ENV.fetch('USER_PASSWORD', 'Str0ngPa$$w0rd12') + config.bot_token = ENV['BOT_TOKEN'] config.hotjar_site_id = ENV.fetch('HOTJAR_SITE_ID', '#HOTJAR_SITE_ID_env_var_missing') config.google_analytics_tracking_id = ENV.fetch('TRACKING_ID', '#TRACKING_ID_env_var_missing') diff --git a/config/initializers/types.rb b/config/initializers/types.rb index ac3927709..e9ffa3111 100644 --- a/config/initializers/types.rb +++ b/config/initializers/types.rb @@ -4,6 +4,8 @@ module Types include Dry.Types() include Dry::Core::Constants + Parent = Instance(Training::Module) | Instance(Course) + TrainingModule = Instance(Training::Module) TrainingContent = Instance(Training::Page) | Instance(Training::Question) | Instance(Training::Video) end diff --git a/config/locales/en.yml b/config/locales/en.yml index 8ad49dd5f..87ee93cd1 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -31,6 +31,7 @@ en: blank: Choose an option. # Validations ---------------------------------------------------------------- + # https://guides.rubyonrails.org/i18n.html#translations-for-active-record-models activerecord: errors: @@ -39,6 +40,8 @@ en: attributes: answers: blank: Please select an answer. + invalid: Please select an option. + user_answer: attributes: answer: @@ -93,9 +96,18 @@ en: start_test: Start test save_continue: Save and continue finish_test: Finish test - finish: Finish + finish: View certificate + give_feedback: Give feedback + + previous_page: + previous: Previous links: + home: Go to home + my_modules: Go to my modules + feedback: + update: Update my feedback + skip: Skip feedback save: Save continue: Continue cancel: Cancel @@ -189,11 +201,6 @@ en: Please rate to what extent you agree with the statements on the following pages. - thankyou: - heading: Thank you - body: | - You can also {external}[give feedback about the training](%{feedback_url}){/external} - certificate: heading: Download your certificate @@ -251,12 +258,6 @@ en: You may wish to revisit some of the learning before taking the test again. - notice: - email_changed: | - We have sent an email to your new email address with a link to click to confirm the change. - - If you have not received the email after a few minutes, please check your spam folder. - # Pages ---------------------------------------------------------------------- # /my-learning @@ -281,11 +282,25 @@ en: name_information: This is the name that will appear on your end of module certificate. You can use this setting to change how your name appears. Changing your name on this account will not affect your GOV.UK One Login setting_details: | ## Your setting details - email_preferences: | - ## Your email preferences - training_emails: - true: You have chosen to receive emails about this training course. - false: You have chosen not to receive emails about this training course. + + email_preferences: + heading: | + ## Your email preferences + label: Email updates about this training course + link_text: Change email preferences + preference: + true: You have chosen to receive emails about this training course. + false: You have chosen not to receive emails about this training course. + + research_preferences: + heading: | + ## Your research preferences + label: Willing to speak to a researcher to improve this service + link_text: Change research preferences + preference: + true: You have chosen to participate in research. + false: You have chosen not to participate in research. + early_years_experience: Time worked in early years password_changed: Password last changed on %{date} @@ -519,6 +534,41 @@ en: This module covers: %{criteria} + # /feedback + feedback: + cta: | + # Can you help us improve this training further? + + We are constantly looking at ways we can improve this training for you and your colleagues. + We really value your feedback. + It will take no more than 5 minutes to answer all the questions. + + intro: | + # Give feedback + + The purpose of this feedback form is to gather your opinion on the child development + training course that the Department for Education has created for early years practitioners. + + For more information on how you data will be used, please view our + {external}[privacy notice](https://www.gov.uk/government/publications/privacy-information-members-of-the-public/privacy-information-members-of-the-public){/external}. + + Completing this form is voluntary and you can withdraw your feedback at any time. + + By completing this form you have understood the above and consent to take part. + + ## Technical support queries + + If you have any questions about how to use this website or are experiencing any + technical issues, please {external}[use our contact form](%{contact_us}){/external} so that our team can follow up with your enquiry. + complete: | + # You have already submitted feedback + + Thank you for helping to improve this training + # one-off question + research: | + Participation is entirely voluntary, and you can withdraw at any point before, + during or after the research takes place and you do not need to give a reason. + reasons: Could you give us reasons for your answer? # /gov-one/info gov_one_info: diff --git a/config/routes.rb b/config/routes.rb index 49a4ab7ce..f7c58d52d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -5,8 +5,8 @@ get 'my-modules', to: 'learning#show' # @see User#course get '404', to: 'errors#not_found', via: :all - get '422', to: 'errors#unprocessable_entity', via: :all get '500', to: 'errors#internal_server_error', via: :all + get '503', to: 'errors#service_unavailable', via: :all resources :settings, controller: :settings, only: %i[show create] @@ -72,6 +72,8 @@ end end + resources :feedback, only: %i[index show update] + post 'notify', to: 'notify#update' post 'change', to: 'release#update' post 'release', to: 'release#new' diff --git a/config/sitemap.rb b/config/sitemap.rb index 84a229657..158b971e4 100644 --- a/config/sitemap.rb +++ b/config/sitemap.rb @@ -36,27 +36,25 @@ # errors add '/404' - # add '/422' add '/500' + add '/503' add course_overview_path add experts_path + add feedback_index_path - Training::Module.live.each do |mod| - add about_path(mod.name) + Course.config.pages.each do |content| + add feedback_path(content.name) end add new_user_session_path - # private pages - # ------------------------------------------ - # account add user_path # edit registration/account add edit_registration_terms_and_conditions_path - add edit_registration_name_path # unless Rails.application.gov_one_login? + add edit_registration_name_path add edit_registration_setting_type_path add edit_registration_setting_type_other_path add edit_registration_local_authority_path @@ -75,13 +73,12 @@ add my_modules_path add user_notes_path + # course content if Training::Module.live.any? - # Course common start page - mod = Training::Module.live.first - add training_module_page_path(mod.name, mod.pages.first.name) + mod = Training::Module.live.sample + add about_path(mod.name) - # Course content random module - Training::Module.live.sample.content.each do |page| + mod.content.each do |page| if page.is_question? add training_module_question_path(page.parent.name, page.name) elsif page.assessment_results? diff --git a/data/KPI.md b/data/KPI.md index d02d482a6..19227977f 100644 --- a/data/KPI.md +++ b/data/KPI.md @@ -32,9 +32,6 @@ Event.where(name: 'module_content_page').where_properties(seed: true).count #=> 8680 ``` -- `skipped: true` to preserve a user's progress when new module content pages are injected ([FillPageViews](../../FillPageViews)). - This will be necessary until content versioning is implemented. - - `cloned: true` where named events such as `module_start` are created for users who have already started the module. #### Event Dictionary @@ -86,33 +83,32 @@ Example event data from the `ahoy_visits` table. ## Transactions -| Done | Feature | Key | Controllers | Path | -| :--- | :--- | :--- | :--- | :--- | -| [x] | Homepage | `home_page` | `HomeController` | `/` | -| [x] | Monitoring progress | `learning_page` | `LearningController` | `/my-modules` | -| [x] | Course overview | `course_overview_page` | `Training::ModulesController` | `/modules` | -| [x] | Module overview | `module_overview_page` | `Training::ModulesController` | `/modules/{alpha}` | -| [x] | Module content | `module_content_page` | `Training::PagesController` | `/modules/{alpha}/content-pages/{1}` | -| [x] | Static page content | `static_page` | `PagesController` | `/example-page` | -| [x] | Account completion | `user_registration` | `Registration::Controller` | `/registration/{attr}` | -| [x] | User profile | `profile_page` | `UserController` | `/my-account` | -| [x] | User name change | `user_name_change` | `UserController` | `/my-account/update-name` | -| [x] | User email change | `user_email_change` | `UserController` | `/my-account/update-email` | -| [x] | User password change | `user_password_change` | `UserController` | `/my-account/update-password` | -| [x] | Email address taken | `email_address_taken` | `RegistrationsController` | `/users/sign-up` | -| [x] | User inactivity logout | `error_page` | `ErrorsController` | `/timeout` | -| [x] | 404 Error | `error_page` | `ErrorsController` | `/404` | -| [x] | 500 Error | `error_page` | `ErrorsController` | `/500` | -| [x] | Module start | `module_start` | `Training::PagesController` | `/modules/{alpha}/content-pages/intro` | -| [x] | Module complete | `module_complete` | `Training::ModulesController` | `/modules/{alpha}/certificate` | -| [x] | Questionnaire answered | `questionnaire_answer` | `Training::QuestionsController` | `/modules/{alpha}/questionnaires/{path}` | -| [x] | Summative assessment start | `summative_assessment_start` | `Training::QuestionsController` | `/modules/{alpha}/questionnaires/{path}` | -| [x] | Summative assessment complete | `summative_assessment_complete` | `Training::AssessmentsController` | `/modules/{alpha}/assessment-results/{path}` | -| [x] | Confidence check start | `confidence_check_start` | `Training::QuestionsController` | `/modules/{alpha}/questionnaires/{path}` | -| [x] | Confidence check complete | `confidence_check_complete` | `Training::PagesController` | `/modules/{alpha}/questionnaires/{path}` | -| [x] | User note created | `user_note_created` | `Training::NotesController` | `/my-account/learning-log` | -| [x] | User note updated | `user_note_updated` | `Training::NotesController` | `/my-account/learning-log` | - +| Feature | Key | Controllers | Path | +| :--- | :--- | :--- | :--- | +| Homepage | `home_page` | `HomeController` | `/` | +| Monitoring progress | `learning_page` | `LearningController` | `/my-modules` | +| Course overview | `course_overview_page` | `Training::ModulesController` | `/modules` | +| Module overview | `module_overview_page` | `Training::ModulesController` | `/modules/{alpha}` | +| Module content | `module_content_page` | `Training::PagesController` | `/modules/{alpha}/content-pages/{1}` | +| Static page content | `static_page` | `PagesController` | `/example-page` | +| Account completion | `user_registration` | `Registration::Controller` | `/registration/{attr}` | +| User profile | `profile_page` | `UserController` | `/my-account` | +| User name change | `user_name_change` | `UserController` | `/my-account/update-name` | +| Email address taken | `email_address_taken` | `RegistrationsController` | `/users/sign-up` | +| User inactivity logout | `error_page` | `ErrorsController` | `/timeout` | +| 404 Error | `error_page` | `ErrorsController` | `/404` | +| 500 Error | `error_page` | `ErrorsController` | `/500` | +| Module start | `module_start` | `Training::PagesController` | `/modules/{alpha}/content-pages/intro` | +| Module complete | `module_complete` | `Training::ModulesController` | `/modules/{alpha}/certificate` | +| Questionnaire answered | `questionnaire_answer` | `Training::QuestionsController` | `/modules/{alpha}/questionnaires/{path}` | +| Summative assessment start | `summative_assessment_start` | `Training::QuestionsController` | `/modules/{alpha}/questionnaires/{path}` | +| Summative assessment complete | `summative_assessment_complete` | `Training::AssessmentsController` | `/modules/{alpha}/assessment-results/{path}` | +| Confidence check start | `confidence_check_start` | `Training::QuestionsController` | `/modules/{alpha}/questionnaires/{path}` | +| Confidence check complete | `confidence_check_complete` | `Training::PagesController` | `/modules/{alpha}/questionnaires/{path}` | +| User note created | `user_note_created` | `Training::NotesController` | `/my-account/learning-log` | +| User note updated | `user_note_updated` | `Training::NotesController` | `/my-account/learning-log` | +| Feedback started | `feedback_start` | `FeedbackController` | `/feedback/{1}` | +| Feedback completed | `feedback_complete` | `FeedbackController` | `/feedback/thank-you` | ## Metrics diff --git a/db/migrate/20240308140000_add_freetext_responses.rb b/db/migrate/20240308140000_add_freetext_responses.rb new file mode 100644 index 000000000..20bef43ee --- /dev/null +++ b/db/migrate/20240308140000_add_freetext_responses.rb @@ -0,0 +1,5 @@ +class AddFreetextResponses < ActiveRecord::Migration[7.1] + def change + add_column :responses, :text_input, :text + end +end diff --git a/db/migrate/20240308142000_add_guest_responses.rb b/db/migrate/20240308142000_add_guest_responses.rb new file mode 100644 index 000000000..2289faff9 --- /dev/null +++ b/db/migrate/20240308142000_add_guest_responses.rb @@ -0,0 +1,9 @@ +class AddGuestResponses < ActiveRecord::Migration[7.1] + def change + change_column_null :responses, :user_id, true + + change_table :responses, bulk: true do |t| + t.references :visit, null: true, foreign_key: true + end + end +end diff --git a/db/migrate/20240509133351_add_research_preference_to_users.rb b/db/migrate/20240509133351_add_research_preference_to_users.rb new file mode 100644 index 000000000..ee2af644c --- /dev/null +++ b/db/migrate/20240509133351_add_research_preference_to_users.rb @@ -0,0 +1,5 @@ +class AddResearchPreferenceToUsers < ActiveRecord::Migration[7.1] + def change + add_column :users, :research_participant, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index 53ca95631..00e887e6c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -225,6 +225,7 @@ t.boolean "early_years_emails" t.string "gov_one_id" t.string "early_years_experience" + t.boolean "research_participant" t.jsonb "notify_callback" t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true t.index ["email"], name: "index_users_on_email", unique: true diff --git a/docker-compose.yml b/docker-compose.yml index 5342043ff..fb7fd526a 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -2,7 +2,6 @@ # # --- -version: '3.8' services: app: container_name: recovery_prod @@ -21,7 +20,6 @@ services: - RAILS_MASTER_KEY - RAILS_LOG_TO_STDOUT - PROXY_CERT - - GOV_ONE_LOGIN depends_on: - db ports: diff --git a/lib/content_test_schema.rb b/lib/content_test_schema.rb index 7703a1c44..9e7943d64 100644 --- a/lib/content_test_schema.rb +++ b/lib/content_test_schema.rb @@ -1,6 +1,6 @@ # # Transform basic Content schema into RSpec actionable AST -# +# Answering all factual questions either correctly or incorrectly # class ContentTestSchema extend Dry::Initializer @@ -46,7 +46,7 @@ def inputs ] elsif payload[:note] [ - [:make_note, 'note-body-field', payload[:note]], + [:input_text, 'note-body-field', payload[:note]], [:click_on, 'Save and continue'], ] elsif type.match?(/assessment_intro/) @@ -59,7 +59,7 @@ def inputs ] elsif type.match?(/thankyou/) [ - [:click_on, 'Finish'], + [:click_on, 'View certificate'], ] elsif type.match?(/certificate/) [] @@ -149,6 +149,6 @@ def next_schema # @return [Boolean] def skip? - !pass && type.match?(/results|confidence|thank|certificate/) + !pass && (type.match?(/results|confidence|thank|certificate/) || slug.match?(/feedback/)) end end diff --git a/lib/fill_page_views.rb b/lib/fill_page_views.rb deleted file mode 100644 index 651301352..000000000 --- a/lib/fill_page_views.rb +++ /dev/null @@ -1,77 +0,0 @@ -# :nocov: -# Assumes gaps in page views due to skipping or revisions to content -# -# Loop over active users and modules and inject page view events for skipped pages -# -class FillPageViews - def call - users.find_each(batch_size: 1_000) do |user| - unless user.registration_complete - log "user [#{user.id}]" - next - end - - tracker = Ahoy::Tracker.new(user: user, controller: 'training/pages') - - training_modules.each do |mod| - progress = ModuleProgress.new(user: user, mod: mod) - - unless progress.furthest_page - log "user [#{user.id}] module [#{mod.position}] - not started" - next - end - - unless progress.skipped? - log "user [#{user.id}] module [#{mod.position}] - not skipped" - next - end - - skipped = 0 - page = progress.furthest_page.name - - mod.content.each do |content| - break if content.eql?(progress.furthest_page.next_item) - - if progress.visited?(content) - next - else - tracker.track('module_content_page', { - uid: content.id, - mod_uid: mod.id, - skipped: true, - id: content.name, - action: 'show', - controller: 'training/pages', - training_module_id: mod.name, - }) - - skipped += 1 - end - end - - log "user [#{user.id}] module [#{mod.position}] - [#{skipped}] skipped before page [#{page}]" - end - end - end - -private - - def users - User.order(:id).all - end - - def training_modules - Training::Module.ordered - end - - # @param message [String] - # @return [String, nil] - def log(message) - if ENV['RAILS_LOG_TO_STDOUT'].present? - Rails.logger.info(message) - elsif ENV['VERBOSE'].present? - puts message - end - end -end -# :nocov: diff --git a/lib/tasks/eyfs.rake b/lib/tasks/eyfs.rake index d03212ddf..a4d9b7664 100644 --- a/lib/tasks/eyfs.rake +++ b/lib/tasks/eyfs.rake @@ -24,12 +24,6 @@ namespace :eyfs do end namespace :jobs do - # NB: Not yet CMS compatible - desc 'Add page view events for injected module items' - task plug_content: :environment do - FillPageViewsJob.enqueue - end - # Queueing a dashboard job via Rake inverts the default and will not # upload files to Looker Studio unless explicitly requested. # diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb new file mode 100644 index 000000000..48fd3f3c9 --- /dev/null +++ b/spec/controllers/application_controller_spec.rb @@ -0,0 +1,26 @@ +require 'rails_helper' + +RSpec.describe ApplicationController, type: :controller do + describe '#guest' do + subject(:guest) { controller.send(:guest) } + + let(:cookie_token) { 'some-token' } + + it 'is nil without a current_visit' do + expect(guest).to be_nil + end + + it 'is a Guest with a current_visit' do + allow(controller).to receive(:current_visit).and_return(create(:visit)) + expect(guest).to be_a Guest + end + + it 'restores a previous visit from a cookie' do + allow(controller).to receive(:current_visit).and_return(create(:visit)) + create(:visit, visit_token: cookie_token) + request.cookies[:course_feedback] = cookie_token + expect(guest).to be_a Guest + expect(guest.visit_token).to eq cookie_token + end + end +end diff --git a/spec/controllers/feedback_controller_spec.rb b/spec/controllers/feedback_controller_spec.rb new file mode 100644 index 000000000..965efd9a3 --- /dev/null +++ b/spec/controllers/feedback_controller_spec.rb @@ -0,0 +1,120 @@ +require 'rails_helper' + +RSpec.describe FeedbackController, type: :controller do + context 'when user is signed in' do + let(:user) { create :user, :registered } + + before { sign_in user } + + describe 'GET #show' do + context 'with last page' do + it 'is tracked as complete' do + expect(controller).to receive(:track_feedback_complete) + get :show, params: { id: 'thank-you' } + end + end + + it 'returns a success response' do + get :show, params: { id: 'feedback-radio-only' } + expect(response).to have_http_status(:success) + end + + context 'when the shared question was answered during a training module' do + let(:answer) do + create :response, + user: user, + training_module: 'alpha', + question_name: 'feedback-skippable', + question_type: 'feedback', + answers: [1] # yes / participate + end + + it 'moved to the main form' do + expect { + get :show, params: { id: 'feedback-skippable' } + }.to change { + answer.reload.training_module + }.from('alpha').to('course') + end + end + end + + describe 'GET #index' do + it 'returns a success response' do + get :index + expect(response).to have_http_status(:success) + end + end + + describe 'POST #update' do + context 'with valid params' do + let(:params) do + { + id: 'feedback-skippable', + response: { + answers: %w[1], + }, + } + end + + it 'is persisted' do + expect { + post :update, params: params + }.to change(Response, :count).by(1) + end + + context 'and first response' do + it 'redirects to the next feedback content' do + post :update, params: params + expect(response).to have_http_status(:redirect) + expect(response).to redirect_to feedback_path('thank-you') + end + end + + context 'and subsequent responses' do + it 'redirects to the profile page' do + create :event, user: user, name: 'profile_page' + post :update, params: params + expect(response).to have_http_status(:redirect) + expect(response).to redirect_to user_path + end + end + + it 'is tracked as started' do + expect(controller).to receive(:track_feedback_start) + expect(cookies[:course_feedback]).not_to be_present + post :update, params: params + expect(cookies[:course_feedback]).to be_present + end + end + + context 'with invalid params' do + let(:params) do + { + id: 'feedback-radio-only', + response: { + answers: [''], + }, + } + end + + it 'is not processed' do + post :update, params: params + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'is not persisted' do + expect { + post :update, params: params + }.not_to change(Response, :count) + end + + it 'is not tracked as started' do + expect(controller).not_to receive(:track_feedback_start) + post :update, params: params + expect(cookies[:course_feedback]).not_to be_present + end + end + end + end +end diff --git a/spec/controllers/training/responses_controller_spec.rb b/spec/controllers/training/responses_controller_spec.rb index d7b9ab17d..a7df10bf7 100644 --- a/spec/controllers/training/responses_controller_spec.rb +++ b/spec/controllers/training/responses_controller_spec.rb @@ -8,7 +8,7 @@ patch :update, params: { training_module_id: 'alpha', id: question_name, - response: { answers: answers }, + response: { answers: answers, text_input: 'Text input' }, } else patch :update, params: { @@ -78,6 +78,25 @@ specify { expect(records).to be 0 } end end + + context 'when the question expects text and is answered' do + let(:question_name) { 'feedback-textarea-only' } + let(:answers) { [] } + + context 'with text input' do + let(:text_input) { 'Text input for feedback question' } + + specify { expect(response).to have_http_status(:redirect) } + specify { expect(records).to be 1 } + end + + context 'with no text input' do + let(:text_input) { nil } + + specify { expect(response).to have_http_status(:redirect) } + specify { expect(records).to be 1 } + end + end end end end diff --git a/spec/decorators/feedback_pagination_decorator_spec.rb b/spec/decorators/feedback_pagination_decorator_spec.rb new file mode 100644 index 000000000..601453e3f --- /dev/null +++ b/spec/decorators/feedback_pagination_decorator_spec.rb @@ -0,0 +1,61 @@ +require 'rails_helper' + +RSpec.describe FeedbackPaginationDecorator do + subject(:decorator) do + described_class.new(content, user) + end + + let(:user) { create :user } + let(:mod) { Training::Module.by_name(:alpha) } + let(:content) { mod.page_by_name('feedback-radio-only') } + + it '#heading' do + expect(decorator.heading).to eq 'Additional feedback' + end + + it '#section_numbers' do + expect(decorator.section_numbers).to eq 'Section 4 of 5' + end + + it '#page_numbers' do + expect(decorator.page_numbers).to eq 'Page 1 of 9' + end + + it '#percentage' do + expect(decorator.percentage).to eq '11%' + end + + context 'when one-off questions have already been answered' do + before do + create :response, + question_name: 'feedback-skippable', + training_module: 'bravo', + answers: [1], + correct: true, + user: user, + question_type: 'feedback' + end + + it '#page_numbers' do + expect(decorator.page_numbers).to eq 'Page 1 of 8' + end + end + + context 'when one-off questions are being answered' do + let(:content) { mod.page_by_name('feedback-skippable') } + + before do + create :response, + question_name: 'feedback-skippable', + training_module: 'alpha', + answers: [1], + correct: true, + user: user, + question_type: 'feedback' + end + + it '#page_numbers' do + expect(decorator.page_numbers).to eq 'Page 8 of 9' + end + end +end diff --git a/spec/decorators/module_debug_decorator_spec.rb b/spec/decorators/module_debug_decorator_spec.rb index 643215a21..d26c14842 100644 --- a/spec/decorators/module_debug_decorator_spec.rb +++ b/spec/decorators/module_debug_decorator_spec.rb @@ -10,9 +10,24 @@ describe '#rows' do let(:output) { decorator.rows } - xit do - expect(output).to eq [ - [], + it 'has heading' do + expect(output[0]).to eq %w[ + Position Visited Sections Progress Submodule Topic Pages Model Type Name + ] + end + + it 'has rows' do + expect(output[1]).to eq [ + '1st', + 'false', + 'Section 1 of 5', + '25%', + '1', + '0', + 'Page 1 of 4', + 'page', + 'sub_module_intro', + '1-1', ] end end diff --git a/spec/decorators/module_overview_decorator_spec.rb b/spec/decorators/module_overview_decorator_spec.rb index 4716256a8..66b3ac273 100644 --- a/spec/decorators/module_overview_decorator_spec.rb +++ b/spec/decorators/module_overview_decorator_spec.rb @@ -55,7 +55,7 @@ end it 'goes to the certificate' do - expect(user.events.count).to be 32 + expect(user.events.count).to be 41 expect(state).to be :completed expect(page_name).to eq '1-3-4' end diff --git a/spec/decorators/next_page_decorator_spec.rb b/spec/decorators/next_page_decorator_spec.rb index 5ea783757..38c9da1cd 100644 --- a/spec/decorators/next_page_decorator_spec.rb +++ b/spec/decorators/next_page_decorator_spec.rb @@ -100,7 +100,7 @@ let(:content) { mod.page_by_name('1-3-3-5') } it '#text' do - expect(decorator.text).to eq 'Finish' + expect(decorator.text).to eq 'View certificate' end end end diff --git a/spec/decorators/pagination_decorator_spec.rb b/spec/decorators/pagination_decorator_spec.rb index 115ec7afe..8a76b5ec3 100644 --- a/spec/decorators/pagination_decorator_spec.rb +++ b/spec/decorators/pagination_decorator_spec.rb @@ -13,7 +13,7 @@ end it '#section_numbers' do - expect(decorator.section_numbers).to eq 'Section 1 of 4' + expect(decorator.section_numbers).to eq 'Section 1 of 5' end it '#page_numbers' do @@ -23,4 +23,24 @@ it '#percentage' do expect(decorator.percentage).to eq '29%' end + + describe 'skippable questions' do + let(:content) { mod.page_by_name('feedback-textarea-only') } + + context 'when answered' do + before do + create(:response, question_name: content.name, text_input: 'text input') + end + + it '#page_numbers' do + expect(decorator.page_numbers).to eq 'Page 3 of 9' + end + end + + context 'when unanswered' do + it '#page_numbers' do + expect(decorator.page_numbers).to eq 'Page 3 of 9' + end + end + end end diff --git a/spec/decorators/previous_page_decorator_spec.rb b/spec/decorators/previous_page_decorator_spec.rb new file mode 100644 index 000000000..4f7aa3c8e --- /dev/null +++ b/spec/decorators/previous_page_decorator_spec.rb @@ -0,0 +1,74 @@ +# Helps to make notes: +# +# feedback-intro (opinion_intro) +# end-of-module-feedback-1 (feedback) +# end-of-module-feedback-3 (feedback) +# feedback-textarea-only (feedback) +# end-of-module-feedback-5 (feedback) <-- SKIPPABLE +# 1-3-3-5 (thankyou) +# +# +require 'rails_helper' + +RSpec.describe PreviousPageDecorator do + subject(:decorator) do + described_class.new(user: user, mod: mod, content: content, assessment: assessment) + end + + let(:user) { create :user, :registered } + let(:mod) { Training::Module.by_name(:alpha) } + let(:content) { mod.page_by_name('1-1-2') } + let(:assessment) { double } + + describe '#style' do + it do + expect(decorator.style).to eq 'govuk-button--secondary' + end + + context 'when page introduces a section' do + let(:content) { mod.page_by_name('1-2') } + + it do + expect(decorator.style).to eq 'section-intro-previous-button' + end + end + end + + describe '#text' do + it do + expect(decorator.text).to eq 'Previous' + end + end + + describe '#name' do + it 'is previous page name' do + expect(decorator.name).to eq '1-1-1' + end + + context 'when feedback was skipped' do + let(:content) { mod.page_by_name('1-3-3-5') } + + it 'skips back to start of feedback form' do + expect(decorator.name).to eq 'feedback-intro' + end + end + + context 'when previous page is skippable' do + let(:content) { mod.page_by_name('1-3-3-5') } + + context 'and answered' do + before do + create :response, + question_name: 'feedback-skippable', + training_module: mod.name, + answers: [1], + correct: true, + user: user, + question_type: 'feedback' + end + + specify { expect(decorator.name).to eq 'feedback-checkbox-other-or' } + end + end + end +end diff --git a/spec/factories/users.rb b/spec/factories/users.rb index b66b37932..f1ad95aab 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -40,10 +40,12 @@ trait :agency_childminder do registered setting_type_id { 'childminder_agency' } - setting_type { 'Childminder as part of an agency' } # Why are we persisting the titles? + setting_type { 'Childminder as part of an agency' } setting_type_other { nil } role_type { 'Childminder' } role_type_other { nil } + early_years_experience { '6-9' } + local_authority { 'Hertfordshire' } end trait :independent_childminder do @@ -53,6 +55,8 @@ setting_type_other { nil } role_type { 'Childminder' } role_type_other { nil } + early_years_experience { '0-2' } + local_authority { 'Leeds' } end trait :team_member do diff --git a/spec/forms/application_form_spec.rb b/spec/forms/application_form_spec.rb new file mode 100644 index 000000000..dd1772ed6 --- /dev/null +++ b/spec/forms/application_form_spec.rb @@ -0,0 +1,17 @@ +require 'rails_helper' + +RSpec.describe ApplicationForm do + subject(:form) { described_class.new } + + describe '#parent title' do + specify do + expect(form.parent.title).to be_nil + end + end + + describe '#save' do + specify do + expect { form.save }.to raise_error ApplicationForm::FormError + end + end +end diff --git a/spec/forms/form_builder_spec.rb b/spec/forms/form_builder_spec.rb new file mode 100644 index 000000000..81fcd96e2 --- /dev/null +++ b/spec/forms/form_builder_spec.rb @@ -0,0 +1,37 @@ +require 'rails_helper' + +RSpec.describe FormBuilder do + let(:assigns) { {} } + let(:controller) { ActionController::Base.new } + let(:lookup_context) { ActionView::LookupContext.new(nil) } + let(:helper) { ActionView::Base.new(lookup_context, assigns, controller) } + let(:object) { create :user } + let(:object_name) { :user } + let(:builder) { described_class.new(object_name, object, helper, {}) } + + describe '#select_trainee_setting' do + subject(:output) { builder.select_trainee_setting } + + it 'element' do + expect(output).to include '
Page::Resource#name' do diff --git a/spec/models/concerns/content_types_spec.rb b/spec/models/concerns/content_types_spec.rb index ce534e64b..aefd543f0 100644 --- a/spec/models/concerns/content_types_spec.rb +++ b/spec/models/concerns/content_types_spec.rb @@ -88,6 +88,12 @@ specify { expect(content).to be_confidence_question } end + describe '#feedback_question?' do + before { content.page_type = 'feedback' } + + specify { expect(content).to be_feedback_question } + end + describe '#thankyou?' do before { content.page_type = 'thankyou' } @@ -120,6 +126,7 @@ assessment_results confidence_intro confidence + feedback thankyou certificate ]) diff --git a/spec/models/concerns/pagination_spec.rb b/spec/models/concerns/pagination_spec.rb index 23041eaf5..92d90d966 100644 --- a/spec/models/concerns/pagination_spec.rb +++ b/spec/models/concerns/pagination_spec.rb @@ -24,15 +24,33 @@ describe '#section?' do it 'returns true if the page defines a section boundary' do - expect(page.section?).to eq(false) - expect(mod.page_by_name('1-2').section?).to eq(true) + expect(page).not_to be_section + expect(mod.page_by_name('1-2')).to be_section end end describe '#subsection?' do it 'returns true if the page defines a subsection boundary' do - expect(page.subsection?).to eq(false) - expect(mod.page_by_name('1-3-2').subsection?).to eq(true) # assessment subsection + expect(page).not_to be_subsection + expect(mod.page_by_name('1-3-2')).to be_subsection # assessment subsection + end + end + + describe '#with_parent' do + subject(:page) { mod.page_by_name('feedback-checkbox-other-or') } + + let(:mod) { Training::Module.by_name(:bravo) } + + it 'navigates shared content in the correct parent' do + expect(page.parent.name).to eq 'alpha' + expect(page.with_parent(mod).parent.name).to eq 'bravo' + + expect(page.next_item.name).to eq 'feedback-skippable' + expect(page.with_parent(mod).next_item.name).to eq 'feedback-skippable' + expect(page.with_parent(mod).next_next_item.name).to eq '1-3-3-5-bravo' + + expect(page.with_parent(mod).previous_item.name).to eq 'feedback-radio-more' + expect(page.with_parent(mod).previous_previous_item.name).to eq 'feedback-checkbox-other-more' end end end diff --git a/spec/models/course_spec.rb b/spec/models/course_spec.rb index 114624d05..9f615ef8e 100644 --- a/spec/models/course_spec.rb +++ b/spec/models/course_spec.rb @@ -16,10 +16,38 @@ expect(course.internal_mailbox).to eq 'child-development.training@education.gov.uk' end - it 'feedback' do - expect(course.feedback).to be_empty - # - only one question type - # - number of questions + it 'pages' do + expect(course.pages.count).to eq 9 + expect(course.pages.first.page_type).to eq 'feedback' + expect(course.pages.last.page_type).to eq 'text_page' + end + + it 'feedback_questions' do + expect(course.feedback_questions.count).to eq 8 + end + end + + # PoC to ensure exisiting parent#pages logic is reusable + # + describe 'site-wide feedback form navigation/pagination' do + let(:parent) { described_class.config } + let(:pages) { described_class.config.pages } + + it 'parent has pages' do + expect(parent.pages.first).to be_a Training::Question + expect(pages.first.parent.pages.last).to be_a Training::Page + end + + it 'pages have a parent' do + expect(pages.first.parent).to be_a described_class + expect(pages.first.parent).to eq pages.last.parent + end + + it 'page order uing previous_item/next_item' do + expect(pages.first.name).to eq 'feedback-radio-only' + expect(pages.first.next_item.name).to eq 'feedback-checkbox-only' + expect(pages.first.next_item.next_item.name).to eq 'feedback-textarea-only' + expect(pages.first.next_item.previous_item.name).to eq 'feedback-radio-only' end end end diff --git a/spec/models/data_analysis/confidence_check_scores_spec.rb b/spec/models/data_analysis/confidence_check_scores_spec.rb index add96392d..4e8d852cb 100644 --- a/spec/models/data_analysis/confidence_check_scores_spec.rb +++ b/spec/models/data_analysis/confidence_check_scores_spec.rb @@ -4,9 +4,9 @@ before do skip unless Rails.application.migrated_answers? - create(:response, question_type: 'confidence', training_module: 'module_1', question_name: 'q1', answers: [1]) - create(:response, question_type: 'confidence', training_module: 'module_1', question_name: 'q2', answers: [2]) - create(:response, question_type: 'confidence', training_module: 'module_1', question_name: 'q2', answers: [2]) + create(:response, question_type: 'confidence', training_module: 'alpha', question_name: '1-3-3-1', answers: [1]) + create(:response, question_type: 'confidence', training_module: 'alpha', question_name: '1-3-3-2', answers: [2]) + create(:response, question_type: 'confidence', training_module: 'alpha', question_name: '1-3-3-2', answers: [2]) end let(:headers) do @@ -21,14 +21,14 @@ let(:rows) do [ { - module_name: 'module_1', - question_name: 'q1', + module_name: 'alpha', + question_name: '1-3-3-1', answers: [1], count: 1, }, { - module_name: 'module_1', - question_name: 'q2', + module_name: 'alpha', + question_name: '1-3-3-2', answers: [2], count: 2, }, diff --git a/spec/models/data_analysis/guest_feedback_scores_spec.rb b/spec/models/data_analysis/guest_feedback_scores_spec.rb new file mode 100644 index 000000000..7c00c82bf --- /dev/null +++ b/spec/models/data_analysis/guest_feedback_scores_spec.rb @@ -0,0 +1,73 @@ +require 'rails_helper' + +RSpec.describe DataAnalysis::GuestFeedbackScores do + let(:user) { create :user, :registered } + let(:guest) { Guest.new(visit: Visit.new) } + let(:headers) do + %w[ + Guest + Question + Answers + Created + Updated + ] + end + let(:rows) do + [ + { + visit_id: guest.visit.id, + question_name: 'feedback-checkbox-only', + answers: [1, 2], + created_at: '2024-01-02 00:00:00', + updated_at: '2024-01-03 00:00:00', + }, + { + visit_id: guest.visit.id, + question_name: 'feedback-radio-only', + answers: [1], + created_at: '2023-01-01 00:00:00', + updated_at: '2023-01-01 00:00:00', + }, + ] + end + + before do + skip unless Rails.application.migrated_answers? + + # users not included + create(:response, + question_type: 'feedback', + training_module: 'course', + question_name: 'feedback-checkbox-only', + answers: [1, 2]) + create(:response, + question_type: 'feedback', + training_module: 'alpha', + question_name: 'feedback-textarea-only', + text_input: 'opinion', + answers: []) + + # guest responses + create(:response, + user: nil, + visit: guest.visit, + question_type: 'feedback', + training_module: 'course', + question_name: 'feedback-radio-only', + answers: [1], + created_at: Time.zone.local(2023, 1, 1), + updated_at: Time.zone.local(2023, 1, 1)) + + create(:response, + user: nil, + visit: guest.visit, + question_type: 'feedback', + training_module: 'course', + question_name: 'feedback-checkbox-only', + answers: [1, 2], + created_at: Time.zone.local(2024, 1, 2), + updated_at: Time.zone.local(2024, 1, 3)) + end + + it_behaves_like 'a data export model' +end diff --git a/spec/models/data_analysis/high_fail_questions_spec.rb b/spec/models/data_analysis/high_fail_questions_spec.rb index 39506515d..abc353491 100644 --- a/spec/models/data_analysis/high_fail_questions_spec.rb +++ b/spec/models/data_analysis/high_fail_questions_spec.rb @@ -17,8 +17,8 @@ fail_rate_percentage: 0.5, }, { - module_name: 'module_1', - question_name: 'q2', + module_name: 'alpha', + question_name: '1-3-2-2', fail_rate_percentage: 1.0, }, ] @@ -28,37 +28,37 @@ if Rails.application.migrated_answers? create :response, question_type: 'summative', - training_module: 'module_1', - question_name: 'q1', + training_module: 'alpha', + question_name: '1-3-2-1', answers: [1], correct: true create :response, question_type: 'summative', - training_module: 'module_2', - question_name: 'q1', + training_module: 'bravo', + question_name: '1-3-2-1', answers: [1], correct: true create :response, question_type: 'summative', - training_module: 'module_1', - question_name: 'q2', + training_module: 'alpha', + question_name: '1-3-2-2', answers: [2], correct: false create :response, question_type: 'summative', - training_module: 'module_1', - question_name: 'q2', + training_module: 'alpha', + question_name: '1-3-2-2', answers: [2], correct: false else - create(:user_answer, :correct, :questionnaire, :summative, module: 'module_1', name: 'q1') - create(:user_answer, :correct, :questionnaire, :summative, module: 'module_2', name: 'q1') - create(:user_answer, :incorrect, :questionnaire, :summative, module: 'module_1', name: 'q2') - create(:user_answer, :incorrect, :questionnaire, :summative, module: 'module_1', name: 'q2') + create(:user_answer, :correct, :questionnaire, :summative, module: 'alpha', name: '1-3-2-1') + create(:user_answer, :correct, :questionnaire, :summative, module: 'bravo', name: '1-3-2-1') + create(:user_answer, :incorrect, :questionnaire, :summative, module: 'alpha', name: '1-3-2-2') + create(:user_answer, :incorrect, :questionnaire, :summative, module: 'alpha', name: '1-3-2-2') end end diff --git a/spec/models/data_analysis/module_feedback_forms_spec.rb b/spec/models/data_analysis/module_feedback_forms_spec.rb new file mode 100644 index 000000000..ee8b3a380 --- /dev/null +++ b/spec/models/data_analysis/module_feedback_forms_spec.rb @@ -0,0 +1,48 @@ +require 'rails_helper' + +RSpec.describe DataAnalysis::ModuleFeedbackForms do + let(:headers) do + %w[ + Module + Started + Completed + ] + end + + let(:rows) do + [ + { + mod: 'First Training Module', + started: 2, + completed: 2, + }, + { + mod: 'Second Training Module', + started: 1, + completed: 1, + }, + { + mod: 'Third Training Module', + started: 1, + completed: 0, + }, + ] + end + + before do + user_1 = create(:user) + create :event, name: 'feedback_start', user: user_1, properties: { 'training_module_id' => 'alpha' } + create :event, name: 'feedback_complete', user: user_1, properties: { 'training_module_id' => 'alpha' } + create :event, name: 'feedback_start', user: user_1, properties: { 'training_module_id' => 'bravo' } + create :event, name: 'feedback_complete', user: user_1, properties: { 'training_module_id' => 'bravo' } + + user_2 = create(:user) + create :event, name: 'feedback_start', user: user_2, properties: { 'training_module_id' => 'alpha' } + create :event, name: 'feedback_complete', user: user_2, properties: { 'training_module_id' => 'alpha' } + + user_3 = create(:user) + create :event, name: 'feedback_start', user: user_3, properties: { 'training_module_id' => 'charlie' } + end + + it_behaves_like 'a data export model' +end diff --git a/spec/models/data_analysis/modules_per_month_spec.rb b/spec/models/data_analysis/modules_per_month_spec.rb index 0b03535af..750f0e4da 100644 --- a/spec/models/data_analysis/modules_per_month_spec.rb +++ b/spec/models/data_analysis/modules_per_month_spec.rb @@ -50,6 +50,7 @@ create :assessment, :passed, user: user_1, completed_at: Time.zone.local(2023, 1, 1) create :assessment, :failed, user: user_1, completed_at: Time.zone.local(2023, 2, 1) create :assessment, :passed, user: user_2, completed_at: Time.zone.local(2023, 3, 1) + create :assessment, user: user_2, training_module: 'bravo' else create(:user_assessment, :passed, user_id: user_1.id, score: 100, module: 'alpha', created_at: Time.zone.local(2023, 1, 1)) create(:user_assessment, :failed, user_id: user_1.id, score: 0, module: 'alpha', created_at: Time.zone.local(2023, 2, 1)) diff --git a/spec/models/data_analysis/user_feedback_scores_spec.rb b/spec/models/data_analysis/user_feedback_scores_spec.rb new file mode 100644 index 000000000..22a2a56e8 --- /dev/null +++ b/spec/models/data_analysis/user_feedback_scores_spec.rb @@ -0,0 +1,157 @@ +require 'rails_helper' + +RSpec.describe DataAnalysis::UserFeedbackScores do + let(:user_1) { create :user, :agency_childminder } + let(:user_2) { create :user, :independent_childminder } + let(:headers) do + [ + 'User ID', + 'Role', + 'Custom Role', + 'Setting', + 'Custom Setting', + 'Local Authority', + 'Years Experience', + 'Module', + 'Question', + 'Answers', + 'Created', + 'Updated', + ] + end + let(:rows) do + [ + { + user_id: user_1.id, + role_type: 'Childminder', + role_type_other: nil, + setting_type: 'Childminder as part of an agency', + setting_type_other: nil, + local_authority: 'Hertfordshire', + early_years_experience: '6-9', + training_module: 'course', + question_name: 'feedback-checkbox-only', + answers: [1, 2], + created_at: '2023-01-01 00:00:00', + updated_at: '2023-01-01 00:00:00', + }, + { + user_id: user_1.id, + role_type: 'Childminder', + role_type_other: nil, + setting_type: 'Childminder as part of an agency', + setting_type_other: nil, + local_authority: 'Hertfordshire', + early_years_experience: '6-9', + training_module: 'course', + question_name: 'feedback-textarea-only', + answers: [], + created_at: '2023-01-01 00:00:00', + updated_at: '2023-01-01 00:00:00', + }, + { + user_id: user_2.id, + role_type: 'Childminder', + role_type_other: nil, + setting_type: 'Independent childminder', + setting_type_other: nil, + local_authority: 'Leeds', + early_years_experience: '0-2', + training_module: 'alpha', + question_name: 'feedback-checkbox-only', + answers: [1, 2, 3, 4], + created_at: '2023-01-01 00:00:00', + updated_at: '2023-01-01 00:00:00', + }, + { + user_id: user_2.id, + role_type: 'Childminder', + role_type_other: nil, + setting_type: 'Independent childminder', + setting_type_other: nil, + local_authority: 'Leeds', + early_years_experience: '0-2', + training_module: 'alpha', + question_name: 'feedback-radio-only', + answers: [1], + created_at: '2023-01-01 00:00:00', + updated_at: '2023-01-01 00:00:00', + }, + { + user_id: user_2.id, + role_type: 'Childminder', + role_type_other: nil, + setting_type: 'Independent childminder', + setting_type_other: nil, + local_authority: 'Leeds', + early_years_experience: '0-2', + training_module: 'alpha', + question_name: 'feedback-textarea-only', + answers: [], + created_at: '2023-01-01 00:00:00', + updated_at: '2023-01-01 00:00:00', + }, + ] + end + + before do + skip unless Rails.application.migrated_answers? + + # guests not included + create(:response, + user: nil, + visit: Visit.new, + question_type: 'feedback', + training_module: 'course', + question_name: 'feedback-radio-only', + answers: [1]) + + # course + create(:response, + user: user_1, + question_type: 'feedback', + training_module: 'course', + question_name: 'feedback-checkbox-only', + answers: [1, 2], + created_at: Time.zone.local(2023, 1, 1), + updated_at: Time.zone.local(2023, 1, 1)) + create(:response, + user: user_1, + question_type: 'feedback', + training_module: 'course', + question_name: 'feedback-textarea-only', + text_input: 'potential PII', + answers: [], + created_at: Time.zone.local(2023, 1, 1), + updated_at: Time.zone.local(2023, 1, 1)) + + # module + create(:response, + user: user_2, + question_type: 'feedback', + training_module: 'alpha', + question_name: 'feedback-radio-only', + answers: [1], + created_at: Time.zone.local(2023, 1, 1), + updated_at: Time.zone.local(2023, 1, 1)) + create(:response, + user: user_2, + question_type: 'feedback', + training_module: 'alpha', + question_name: 'feedback-checkbox-only', + answers: [1, 2, 3, 4], + created_at: Time.zone.local(2023, 1, 1), + updated_at: Time.zone.local(2023, 1, 1)) + create(:response, + user: user_2, + question_type: 'feedback', + training_module: 'alpha', + question_name: 'feedback-textarea-only', + text_input: 'potential PII', + answers: [], + created_at: Time.zone.local(2023, 1, 1), + updated_at: Time.zone.local(2023, 1, 1)) + end + + it_behaves_like 'a data export model' +end diff --git a/spec/models/guest_spec.rb b/spec/models/guest_spec.rb new file mode 100644 index 000000000..f9ed310ef --- /dev/null +++ b/spec/models/guest_spec.rb @@ -0,0 +1,37 @@ +require 'rails_helper' + +RSpec.describe Guest, type: :model do + subject(:guest) { described_class.new(visit: visit) } + + let(:visit) { create :visit } + + it 'needs a visit' do + expect { described_class.new }.to raise_error Dry::Struct::Error + expect { guest }.not_to raise_error + end + + describe '#guest?' do + specify do + expect(guest).to be_guest + end + end + + describe '#response_for_shared' do + before do + create :response, question_name: content.name, training_module: 'course' + end + + let(:content) { Course.config.pages.first } + let(:response) { guest.response_for_shared(content) } + + specify do + expect(response).to be_a Response + end + end + + describe '#course_started?' do + specify do + expect(guest).not_to be_course_started + end + end +end diff --git a/spec/models/response_feedback_spec.rb b/spec/models/response_feedback_spec.rb new file mode 100644 index 000000000..44ce84e69 --- /dev/null +++ b/spec/models/response_feedback_spec.rb @@ -0,0 +1,29 @@ +require 'rails_helper' + +RSpec.describe Response, '#feedback', type: :model do + let(:user) { create :user } + let(:params) do + { + training_module: 'alpha', + correct: true, + user: user, + question_type: 'feedback', + } + end + + before do + skip unless Rails.application.migrated_answers? + end + + # validate answers array + describe '#answers' do + subject(:response) do + build :response, + **params, + question_name: 'feedback-skippable', + answers: [1] + end + + specify { expect(response).to be_valid } + end +end diff --git a/spec/models/response_spec.rb b/spec/models/response_spec.rb index 629c61da0..c0f85bcc7 100644 --- a/spec/models/response_spec.rb +++ b/spec/models/response_spec.rb @@ -3,13 +3,10 @@ RSpec.describe Response, type: :model do subject(:response) { user.response_for(question) } - before do - skip unless Rails.application.migrated_answers? - response.update!(answers: [1], correct: true) - end - let(:user) { create :user } - + let(:question) do + Training::Module.by_name('alpha').page_by_name('1-1-4-1') + end let(:headers) do %w[ id @@ -22,15 +19,15 @@ updated_at question_type assessment_id + text_input + visit_id ] end + let(:rows) { [response] } - let(:rows) do - [response] - end - - let(:question) do - Training::Module.by_name('alpha').page_by_name('1-1-4-1') + before do + skip unless Rails.application.migrated_answers? + response.update!(answers: [1], correct: true) end it_behaves_like 'a data export model' diff --git a/spec/models/training/module_spec.rb b/spec/models/training/module_spec.rb index a2c4920cc..94a4aeb90 100644 --- a/spec/models/training/module_spec.rb +++ b/spec/models/training/module_spec.rb @@ -35,8 +35,8 @@ it 'returns sections' do expect(sections).to be_a Hash - expect(sections.keys).to eq [1, 2, 3, 4] - expect(sections.values.map(&:count)).to eq [7, 5, 20, 1] + expect(sections.keys).to eq [1, 2, 3, 4, 5] + expect(sections.values.map(&:count)).to eq [7, 5, 20, 9, 1] end end @@ -45,14 +45,14 @@ it 'returns subsections' do expect(subsections).to be_a Hash - expect(subsections.keys).to eq [[1, 0], [1, 1], [1, 2], [1, 3], [1, 4], [2, 0], [2, 1], [3, 0], [3, 1], [3, 2], [3, 3], [4, 0]] - expect(subsections.values.map(&:count)).to eq [1, 1, 1, 2, 2, 1, 4, 1, 1, 12, 6, 1] + expect(subsections.keys).to eq [[1, 0], [1, 1], [1, 2], [1, 3], [1, 4], [2, 0], [2, 1], [3, 0], [3, 1], [3, 2], [3, 3], [4, 0], [5, 0]] + expect(subsections.values.map(&:count)).to eq [1, 1, 1, 2, 2, 1, 4, 1, 1, 12, 6, 9, 1] end end describe '#submodule_count' do it 'returns the number of sections' do - expect(mod.submodule_count).to eq 4 + expect(mod.submodule_count).to eq 5 end end diff --git a/spec/models/training/question_spec.rb b/spec/models/training/question_spec.rb index b1c5a217d..437dbb3b1 100644 --- a/spec/models/training/question_spec.rb +++ b/spec/models/training/question_spec.rb @@ -88,6 +88,28 @@ expect(question.legend).to start_with 'True or false?' end end + + context 'when the question is a feedback question' do + subject(:question) do + Training::Module.by_name('alpha').page_by_name('feedback-radio-only') + end + + let(:first_option) { question.options.first } + + specify do + expect(first_option.label).to eq 'Option 1' + end + end + + context 'when the question is a feedback free text question' do + subject(:question) do + Training::Module.by_name('alpha').page_by_name('feedback-textarea-only') + end + + specify do + expect(question.answers).to eq [] + end + end end describe '#debug_summary' do diff --git a/spec/models/training/response_spec.rb b/spec/models/training/response_spec.rb index ca935ba5d..8280d1eeb 100644 --- a/spec/models/training/response_spec.rb +++ b/spec/models/training/response_spec.rb @@ -6,6 +6,7 @@ training_module: 'alpha', question_name: question_name, answers: answers, + question_type: 'formative', ) end @@ -27,6 +28,7 @@ let(:answers) { [1] } it '#correct?' do + expect(response.errors.full_messages).to be_empty expect(response).to be_correct end @@ -100,4 +102,16 @@ end end end + + context 'with radio buttons for feedback question' do + let(:question_name) { 'feedback-radio-only' } + + describe 'and no answer' do + let(:answers) { nil } + + it 'is invalid' do + expect(response).to be_invalid + end + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 38191d8a3..6b35e1727 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -5,6 +5,12 @@ expect(build(:user, :registered)).to be_valid end + describe '#guest?' do + specify do + expect(build(:user, :registered)).not_to be_guest + end + end + describe '#first_name' do it 'must be present' do expect(build(:user, :registered, first_name: nil)).not_to be_valid diff --git a/spec/services/course_progress_spec.rb b/spec/services/course_progress_spec.rb index 9dc54e15a..e953819fe 100644 --- a/spec/services/course_progress_spec.rb +++ b/spec/services/course_progress_spec.rb @@ -117,7 +117,7 @@ draft: false started: false completed: false - last: 1-3-3-5 + last: 1-3-3-5-bravo certificate: 1-3-4 milestone: N/A --- diff --git a/spec/services/dashboard_spec.rb b/spec/services/dashboard_spec.rb index 6c4b33e20..785665cb4 100644 --- a/spec/services/dashboard_spec.rb +++ b/spec/services/dashboard_spec.rb @@ -18,7 +18,7 @@ let(:data_files) { Dir.glob path.join('*/*/*.csv') } describe 'exported files' do - specify { expect(data_files.count).to be 22 } + specify { expect(data_files.count).to be described_class::DATA_SOURCES.size } end it 'exports data in CSV format' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 5861e6c8d..11b62c3a5 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,5 +1,5 @@ require 'simplecov' -SimpleCov.minimum_coverage 90 +SimpleCov.minimum_coverage 92 SimpleCov.start 'rails' require 'pry' diff --git a/spec/support/ast/alpha-fail-response.yml b/spec/support/ast/alpha-fail-response.yml index f233e7fdf..52062f719 100644 --- a/spec/support/ast/alpha-fail-response.yml +++ b/spec/support/ast/alpha-fail-response.yml @@ -1,4 +1,4 @@ -# +# Answer all factual questions incorrectly # @see ContentTestSchema # --- @@ -30,7 +30,7 @@ - :path: /modules/alpha/content-pages/1-1-3-1 :text: 1-1-3-1 :inputs: - - - :make_note + - - :input_text - note-body-field - hello world - - :click_on diff --git a/spec/support/ast/alpha-fail.yml b/spec/support/ast/alpha-fail.yml index e7d1ca58e..e0bc31513 100644 --- a/spec/support/ast/alpha-fail.yml +++ b/spec/support/ast/alpha-fail.yml @@ -1,4 +1,4 @@ -# +# Answer all factual questions incorrectly # @see ContentTestSchema # --- @@ -30,7 +30,7 @@ - :path: /modules/alpha/content-pages/1-1-3-1 :text: 1-1-3-1 :inputs: - - - :make_note + - - :input_text - note-body-field - hello world - - :click_on diff --git a/spec/support/ast/alpha-pass-response.yml b/spec/support/ast/alpha-pass-response-skip-feedback.yml similarity index 95% rename from spec/support/ast/alpha-pass-response.yml rename to spec/support/ast/alpha-pass-response-skip-feedback.yml index d59146ebc..cd16d4db2 100644 --- a/spec/support/ast/alpha-pass-response.yml +++ b/spec/support/ast/alpha-pass-response-skip-feedback.yml @@ -1,4 +1,4 @@ -# +# Answer all factual questions correctly and skip feedback # @see ContentTestSchema # --- @@ -30,7 +30,7 @@ - :path: /modules/alpha/content-pages/1-1-3-1 :text: 1-1-3-1 :inputs: - - - :make_note + - - :input_text - note-body-field - hello world - - :click_on @@ -215,11 +215,16 @@ - response-answers-5-field - - :click_on - Next +- :path: /modules/alpha/content-pages/feedback-intro + :text: Additional feedback + :inputs: + - - :click_on + - Skip feedback - :path: /modules/alpha/content-pages/1-3-3-5 :text: Thank you :inputs: - - :click_on - - Finish + - View certificate - :path: /modules/alpha/content-pages/1-3-4 :text: Congratulations! :inputs: [] diff --git a/spec/support/ast/alpha-pass-response-with-feedback.yml b/spec/support/ast/alpha-pass-response-with-feedback.yml new file mode 100644 index 000000000..d922ae964 --- /dev/null +++ b/spec/support/ast/alpha-pass-response-with-feedback.yml @@ -0,0 +1,273 @@ +# Answer all factual questions correctly +# +# @see ContentTestSchema +# @note Fixture includes feedback form but answering is not implemented +# +--- +- :path: /modules/alpha/content-pages/what-to-expect + :text: What to expect during the training + :inputs: + - - :click_on + - Next +- :path: /modules/alpha/content-pages/1-1 + :text: The first submodule + :inputs: + - - :click_on + - Start section +- :path: /modules/alpha/content-pages/1-1-1 + :text: 1-1-1 + :inputs: + - - :click_on + - Next +- :path: /modules/alpha/content-pages/1-1-2 + :text: 1-1-2 + :inputs: + - - :click_on + - Next +- :path: /modules/alpha/content-pages/1-1-3 + :text: 1-1-3 + :inputs: + - - :click_on + - Next +- :path: /modules/alpha/content-pages/1-1-3-1 + :text: 1-1-3-1 + :inputs: + - - :input_text + - note-body-field + - hello world + - - :click_on + - Save and continue +- :path: /modules/alpha/content-pages/1-1-4 + :text: 1-1-4 + :inputs: + - - :click_on + - Next +- :path: /modules/alpha/questionnaires/1-1-4-1 + :text: Question One - Select from following + :inputs: + - - :choose + - response-answers-1-field + - - :click_on + - Next + - - :click_on + - Next +- :path: /modules/alpha/content-pages/1-2 + :text: The second submodule + :inputs: + - - :click_on + - Start section +- :path: /modules/alpha/content-pages/1-2-1 + :text: 1-2-1 + :inputs: + - - :click_on + - Next +- :path: /modules/alpha/questionnaires/1-2-1-1 + :text: Question Two - Select from following + :inputs: + - - :check + - response-answers-1-field + - - :check + - response-answers-3-field + - - :click_on + - Next + - - :click_on + - Next +- :path: /modules/alpha/content-pages/1-2-1-2 + :text: 1-2-1-2 + :inputs: + - - :click_on + - Next +- :path: /modules/alpha/questionnaires/1-2-1-3 + :text: Question Three - Select from following + :inputs: + - - :check + - response-answers-1-field + - - :check + - response-answers-3-field + - - :click_on + - Next + - - :click_on + - Next +- :path: /modules/alpha/content-pages/1-3 + :text: Summary and next steps + :inputs: + - - :click_on + - Start section +- :path: /modules/alpha/content-pages/1-3-1 + :text: Recap + :inputs: + - - :click_on + - Next +- :path: /modules/alpha/content-pages/1-3-2 + :text: End of module test + :inputs: + - - :click_on + - Start test +- :path: /modules/alpha/questionnaires/1-3-2-1 + :text: Question One - Select from following + :inputs: + - - :check + - response-answers-1-field + - - :check + - response-answers-3-field + - - :click_on + - Save and continue +- :path: /modules/alpha/questionnaires/1-3-2-2 + :text: Question Two - Select from following + :inputs: + - - :check + - response-answers-2-field + - - :check + - response-answers-3-field + - - :click_on + - Save and continue +- :path: /modules/alpha/questionnaires/1-3-2-3 + :text: Question Three - Select from following + :inputs: + - - :check + - response-answers-3-field + - - :check + - response-answers-4-field + - - :click_on + - Save and continue +- :path: /modules/alpha/questionnaires/1-3-2-4 + :text: Question Four - Select from following + :inputs: + - - :choose + - response-answers-3-field + - - :click_on + - Save and continue +- :path: /modules/alpha/questionnaires/1-3-2-5 + :text: Question Five - Select from following + :inputs: + - - :choose + - response-answers-3-field + - - :click_on + - Save and continue +- :path: /modules/alpha/questionnaires/1-3-2-6 + :text: Question Six - Select from following + :inputs: + - - :choose + - response-answers-3-field + - - :click_on + - Save and continue +- :path: /modules/alpha/questionnaires/1-3-2-7 + :text: Question Seven - Select from following + :inputs: + - - :choose + - response-answers-3-field + - - :click_on + - Save and continue +- :path: /modules/alpha/questionnaires/1-3-2-8 + :text: Question Eight - Select from following + :inputs: + - - :choose + - response-answers-3-field + - - :click_on + - Save and continue +- :path: /modules/alpha/questionnaires/1-3-2-9 + :text: Question Nine - Select from following + :inputs: + - - :choose + - response-answers-3-field + - - :click_on + - Save and continue +- :path: /modules/alpha/questionnaires/1-3-2-10 + :text: Question Ten - Select from following + :inputs: + - - :choose + - response-answers-3-field + - - :click_on + - Finish test +- :path: /modules/alpha/assessment-result/1-3-2-11 + :text: Assessment results + :inputs: + - - :click_on + - Next +- :path: /modules/alpha/content-pages/1-3-3 + :text: Reflect on your learning + :inputs: + - - :click_on + - Next +- :path: /modules/alpha/questionnaires/1-3-3-1 + :text: Question One - Select from 1 to 5 + :inputs: + - - :choose + - response-answers-5-field + - - :click_on + - Next +- :path: /modules/alpha/questionnaires/1-3-3-2 + :text: Question Two - Select from 1 to 5 + :inputs: + - - :choose + - response-answers-5-field + - - :click_on + - Next +- :path: /modules/alpha/questionnaires/1-3-3-3 + :text: Question Three - Select from 1 to 5 + :inputs: + - - :choose + - response-answers-5-field + - - :click_on + - Next +- :path: /modules/alpha/questionnaires/1-3-3-4 + :text: Question Four - Select from 1 to 5 + :inputs: + - - :choose + - response-answers-5-field + - - :click_on + - Next +- :path: /modules/alpha/content-pages/feedback-intro + :text: Additional feedback + :inputs: + - - :click_on + - Next +- :path: /modules/alpha/content-pages/feedback-radio-only + :text: Feedback radio buttons only + :inputs: + - - :click_on + - Next +- :path: /modules/alpha/content-pages/feedback-checkbox-only + :text: Feedback checkboxes only + :inputs: + - - :click_on + - Next +- :path: /modules/alpha/content-pages/feedback-textarea-only + :text: Feedback textarea only + :inputs: + - - :click_on + - Next +- :path: /modules/alpha/content-pages/feedback-radio-other-more + :text: Feedback radio buttons with large other + :inputs: + :inputs: + - - :click_on + - Next +- :path: /modules/alpha/content-pages/feedback-checkbox-other-more + :text: Feedback checkbox with large other + :inputs: + - - :click_on + - Next +- :path: /modules/alpha/content-pages/feedback-radio-more + :text: Feedback radio buttons with additional reasons + :inputs: + - - :click_on + - Next +- :path: /modules/alpha/content-pages/feedback-checkbox-other-or + :text: Feedback checkboxes with Other and Or + :inputs: + - - :click_on + - Next +- :path: /modules/alpha/content-pages/feedback-skippable + :text: Skippable + :inputs: + - - :click_on + - Next +- :path: /modules/alpha/content-pages/1-3-3-5 + :text: Thank you + :inputs: + - - :click_on + - View certificate +- :path: /modules/alpha/content-pages/1-3-4 + :text: Congratulations! + :inputs: [] diff --git a/spec/support/ast/alpha-pass.yml b/spec/support/ast/alpha-pass.yml index 3dfe96bda..2990e9269 100644 --- a/spec/support/ast/alpha-pass.yml +++ b/spec/support/ast/alpha-pass.yml @@ -1,4 +1,4 @@ -# +# # Answer all factual questions correctly # @see ContentTestSchema # --- @@ -30,7 +30,7 @@ - :path: /modules/alpha/content-pages/1-1-3-1 :text: 1-1-3-1 :inputs: - - - :make_note + - - :input_text - note-body-field - hello world - - :click_on diff --git a/spec/support/ast/bravo-fail.yml b/spec/support/ast/bravo-fail.yml index 6bb0e0d3e..fe99fa2e2 100644 --- a/spec/support/ast/bravo-fail.yml +++ b/spec/support/ast/bravo-fail.yml @@ -1,3 +1,6 @@ +# Answer all factual questions incorrectly +# @see ContentTestSchema +# --- - :path: "/modules/bravo/content-pages/what-to-expect" :text: What to expect during the training diff --git a/spec/support/ast/course-feedback-guest.yml b/spec/support/ast/course-feedback-guest.yml new file mode 100644 index 000000000..37f3325ab --- /dev/null +++ b/spec/support/ast/course-feedback-guest.yml @@ -0,0 +1,88 @@ +--- +- :path: /feedback + :text: Additional feedback + :inputs: + - - :click_on + - Next +- :path: /feedback/feedback-radio-only + :text: Feedback radio buttons only + :inputs: + - - :choose + - response-answers-1-field + - - :click_on + - Next +- :path: /feedback/feedback-checkbox-only + :text: Feedback checkboxes only + :inputs: + - - :check + - response-answers-1-field + - - :click_on + - Next +- :path: /feedback/feedback-textarea-only + :text: Feedback textarea only + :inputs: + - - :input_text + - response-text-input-field + - free text + - - :click_on + - Next +- :path: /feedback/feedback-radio-other-more + :text: Feedback radio buttons with large other + :inputs: + - - :choose + - response-answers-5-field + - - :input_text + - response-text-input-field + - other text + - - :click_on + - Next +- :path: /feedback/feedback-checkbox-other-more + :text: Feedback checkbox with large other + :inputs: + - - :check + - response-answers-1-field + # WIP + # - - :check + # - response-answers-5-field + # - - :input_text + # - response-text-input-field + # - other text + - - :click_on + - Next + +- :path: /feedback/feedback-radio-more + :text: Feedback radio buttons with additional reasons + :inputs: + - - :choose + - response-answers-1-field + # WIP + # - - :input_text + # - response-text-input-field + # - other text + - - :click_on + - Next +- :path: /feedback/feedback-checkbox-other-or + :text: Feedback checkboxes with Other and Or + :inputs: + - - :check + - response-answers-1-field + # OR + # - - :check + # - response-answers-0-field + # OTHER + # - - :check + # - response-answers-5-field + # - - :input_text + # - response-text-input-field + # - other text + - - :click_on + - Next +# skippable -------------------------------------------------------------------- +# - :path: /feedback/feedback-skippable +# :text: Skippable +# :inputs: +# - - :choose +# - response-answers-1-field +# - - :click_on +# - Next +# skippable -------------------------------------------------------------------- diff --git a/spec/support/ast/course-feedback-user.yml b/spec/support/ast/course-feedback-user.yml new file mode 100644 index 000000000..849af3507 --- /dev/null +++ b/spec/support/ast/course-feedback-user.yml @@ -0,0 +1,88 @@ +--- +- :path: /feedback + :text: Additional feedback + :inputs: + - - :click_on + - Next +- :path: /feedback/feedback-radio-only + :text: Feedback radio buttons only + :inputs: + - - :choose + - response-answers-1-field + - - :click_on + - Next +- :path: /feedback/feedback-checkbox-only + :text: Feedback checkboxes only + :inputs: + - - :check + - response-answers-1-field + - - :click_on + - Next +- :path: /feedback/feedback-textarea-only + :text: Feedback textarea only + :inputs: + - - :input_text + - response-text-input-field + - free text + - - :click_on + - Next +- :path: /feedback/feedback-radio-other-more + :text: Feedback radio buttons with large other + :inputs: + - - :choose + - response-answers-5-field + - - :input_text + - response-text-input-field + - other text + - - :click_on + - Next +- :path: /feedback/feedback-checkbox-other-more + :text: Feedback checkbox with large other + :inputs: + - - :check + - response-answers-1-field + # WIP + # - - :check + # - response-answers-5-field + # - - :input_text + # - response-text-input-field + # - other text + - - :click_on + - Next + +- :path: /feedback/feedback-radio-more + :text: Feedback radio buttons with additional reasons + :inputs: + - - :choose + - response-answers-1-field + # WIP + # - - :input_text + # - response-text-input-field + # - other text + - - :click_on + - Next +- :path: /feedback/feedback-checkbox-other-or + :text: Feedback checkboxes with Other and Or + :inputs: + - - :check + - response-answers-1-field + # OR + # - - :check + # - response-answers-0-field + # OTHER + # - - :check + # - response-answers-5-field + # - - :input_text + # - response-text-input-field + # - other text + - - :click_on + - Next +# skippable -------------------------------------------------------------------- +- :path: /feedback/feedback-skippable + :text: Skippable + :inputs: + - - :choose + - response-answers-1-field + - - :click_on + - Next +# skippable -------------------------------------------------------------------- diff --git a/spec/support/ast/schema.rb b/spec/support/ast/schema.rb index 7e50dad30..41b1d83c2 100644 --- a/spec/support/ast/schema.rb +++ b/spec/support/ast/schema.rb @@ -1,6 +1,6 @@ require 'content_test_schema' # TODO: remove need to wrap "fill_in" kwargs -def make_note(field, input) +def input_text(field, input) fill_in field, with: input end diff --git a/spec/support/shared/with_content.rb b/spec/support/shared/with_content.rb new file mode 100644 index 000000000..821490c3f --- /dev/null +++ b/spec/support/shared/with_content.rb @@ -0,0 +1,125 @@ +RSpec.shared_context 'with content' do + # /srv/data/modules/alpha.yml + # /srv/data/modules/bravo.yml + # /srv/data/modules/charlie.yml + # + # /srv/data/modules/brain-development-and-how-children-learn.yml + # /srv/data/modules/child-development-and-the-eyfs.yml + # /srv/data/modules/personal-social-and-emotional-development.yml + # + let(:module_data) do + Rails.root.join('data/modules').children.select { |mod| mod.extname.eql?('.yml') } + end + + # { + # alpha => { + # what-to-expect => { + # thpe => interruption_page + # } + # }, + # ... + # } + # + let(:course_content) do + module_data.map { |source| YAML.load_file(source) }.map(&:reduce).to_h + end + + # alpha + # bravo + # charlie + # + # brain-development-and-how-children-learn + # child-development-and-the-eyfs + # personal-social-and-emotional-development + # + let(:module_names) do + module_data.map { |mod| File.basename(mod, '.yml') } + end + + # { + # alpha => [ + # what-to-expect, + # before-you-start, + # intro, + # ], + # ... + # } + # + # entries in data/modules/foo.yml + let(:module_content) do + module_names.index_with { |mod_name| course_content[mod_name].keys }.to_h + end + + let(:module_types) do + module_names.index_with do |mod_name| + course_content[mod_name].map { |_page, meta| meta['type'] } + end + end + + let(:essential_types) do + %w[ + interruption_page + sub_module_intro + text_page + formative_questionnaire + video_page + summary_intro + recap_page + assessment_intro + summative_questionnaire + assessment_results + confidence_intro + confidence_questionnaire + feedback + thankyou + certificate + ] + end + + # { + # alpha => [ + # what-to-expect, + # before-you-start, + # intro, + # ], + # ... + # } + # + # entries in config/locales/modules/foo.yml + let(:page_content) do + module_names.index_with { |mod_name| I18n.t(mod_name, scope: 'modules').keys.map(&:to_s) }.to_h + end + + let(:questions) do + type.classify.constantize.all.group_by { |q| q[:training_module] } + end + + let(:questions_total) do + questions.map { |_k, v| v.count }.reduce(&:+) + end + + let(:data_dir) { 'data/formative-questionnaires' } + + let(:type) { 'formative_questionnaire' } + + let(:questionnaire_data) do + Rails.root.join(data_dir).children.select { |mod| mod.extname.eql?('.yml') } + end + + let(:questionnaire_content) do + questionnaire_data.map { |source| YAML.load_file(source) }.map(&:reduce).to_h + end + + it 'question pages have question data' do + module_names.map do |mod_name| + questions = questionnaire_content[mod_name] + question_pages = course_content[mod_name].select { |_name, meta| meta['type'].eql?(type) } + + expect(questions.present?).to eql question_pages.present? + + if question_pages && questions + expect(questions.keys - question_pages.keys).to be_empty + end + end + end +end diff --git a/spec/system/account_page_spec.rb b/spec/system/account_page_spec.rb index f669e26ae..2775992fd 100644 --- a/spec/system/account_page_spec.rb +++ b/spec/system/account_page_spec.rb @@ -16,7 +16,12 @@ expect(page).to have_content 'Changing your name on this account will not affect your GOV.UK One Login' expect(page).to have_link 'Change setting details' + expect(page).to have_link 'Change email preferences' + expect(page).to have_text 'You have chosen to receive emails about this training course.' + + expect(page).to have_link 'Change research preferences' + expect(page).to have_text 'You have chosen not to participate in research.' expect(page).to have_text 'Closing your account' end @@ -52,5 +57,30 @@ expect(page).to have_text 'Developer' expect(page).to have_text 'Not applicable' end + + describe 'research participation preference' do + before do + create :response, + question_name: 'feedback-skippable', + training_module: 'course', + answers: [1], + correct: true, + user: user, + question_type: 'feedback' + + visit '/my-account' + end + + it 'changes response' do + expect(page).to have_text 'You have chosen to participate in research.' + click_on 'Change research preferences' + expect(page).to have_current_path '/feedback/feedback-skippable' + choose 'Option 2' + click_button 'Save' + expect(page).to have_current_path '/my-account' + expect(page).to have_text 'You have chosen not to participate in research.' + expect(page).to have_text 'Your details have been updated' + end + end end end diff --git a/spec/system/common_page_spec.rb b/spec/system/common_page_spec.rb index 5234be1a4..9bd9f26fd 100644 --- a/spec/system/common_page_spec.rb +++ b/spec/system/common_page_spec.rb @@ -26,6 +26,14 @@ end end + describe 'feedback intro' do + subject(:common_page) { '/modules/alpha/content-pages/feedback-intro' } + + it 'uses generic content' do + expect(page).to have_content 'Additional feedback' + end + end + describe 'thank you' do subject(:common_page) { '/modules/alpha/content-pages/1-3-3-5' } diff --git a/spec/system/course_feedback_spec.rb b/spec/system/course_feedback_spec.rb new file mode 100644 index 000000000..cd416e12b --- /dev/null +++ b/spec/system/course_feedback_spec.rb @@ -0,0 +1,118 @@ +require 'rails_helper' + +describe 'Course feedback' do + context 'with unauthenticated user' do + include_context 'with automated path' + let(:fixture) { 'spec/support/ast/course-feedback-guest.yml' } + + it 'returns to homepage once completed' do + expect(page).to have_current_path '/feedback/thank-you' + expect(page).to have_content 'Thank you' + click_on 'Go to home' + expect(page).to have_current_path '/' + end + + it 'saves all answers' do + expect(Response.course_feedback.count).to be 7 + end + + it 'records milestone events' do + expect(Event.feedback_start.count).to be 1 + expect(Event.feedback_complete.count).to be 1 + end + + it 'is linked to a visit not a user' do + expect(Response.course_feedback.first.user).not_to be_present + expect(Response.course_feedback.first.visit).to be_present + end + + describe 'additional text input' do + let(:response) do + Response.course_feedback.find_by(question_name: 'feedback-radio-other-more') + end + + it 'is persisted' do + expect(response.text_input).to eq 'other text' + end + end + + context 'when already completed' do + it 'can be updated' do + visit '/feedback' + click_on 'Update my feedback' + expect(page).to have_current_path '/feedback/feedback-radio-only' + expect(page).to have_checked_field 'Option 1' + end + end + end + + context 'with authenticated user' do + include_context 'with user' + include_context 'with automated path' + let(:fixture) { 'spec/support/ast/course-feedback-user.yml' } + + it 'returns to modules page once completed' do + expect(page).to have_current_path '/feedback/thank-you' + expect(page).to have_content 'Thank you' + click_on 'Go to my modules' + expect(page).to have_current_path '/my-modules' + expect(page).to have_content 'My modules' + end + + it 'saves all answers' do + expect(Response.course_feedback.count).to be 8 + end + + it 'records milestone events' do + expect(Event.feedback_start.count).to be 1 + expect(Event.feedback_complete.count).to be 1 + end + + it 'is linked to a user not a visit' do + expect(Response.course_feedback.first.visit).not_to be_present + expect(Response.course_feedback.first.user).to be_present + end + + describe 'additional text input' do + let(:response) do + Response.course_feedback.find_by(question_name: 'feedback-radio-other-more') + end + + it 'is persisted' do + expect(response.text_input).to eq 'other text' + end + end + + context 'when already completed' do + it 'can be updated' do + visit '/feedback' + click_on 'Update my feedback' + expect(page).to have_current_path '/feedback/feedback-radio-only' + expect(page).to have_checked_field 'Option 1' + end + end + end + + describe 'one-off questions' do + include_context 'with user' + + context 'when already answered' do + before do + create :response, + question_name: 'feedback-skippable', + training_module: 'alpha', + answers: [1], + correct: true, + user: user, + question_type: 'feedback' + end + + it 'skips the question' do + visit '/feedback/feedback-checkbox-other-or' + check 'response-answers-1-field' + click_on 'Next' + expect(page).to have_current_path '/feedback/thank-you' + end + end + end +end diff --git a/spec/system/event_log_spec.rb b/spec/system/event_log_spec.rb index 1632e1de7..1aee2015a 100644 --- a/spec/system/event_log_spec.rb +++ b/spec/system/event_log_spec.rb @@ -4,6 +4,7 @@ include_context 'with events' include_context 'with user' include_context 'with automated path' + let(:fixture) { 'spec/support/ast/alpha-pass-response-skip-feedback.yml' } describe 'confidence check' do context 'when viewing the first question' do @@ -49,7 +50,7 @@ end context 'when all questions are answered incorrectly' do - # let(:fixture) { 'spec/support/ast/alpha-fail-response.yml' } + let(:fixture) { 'spec/support/ast/alpha-fail-response.yml' } let(:happy) { false } it 'tracks answers and failed attempt' do @@ -77,7 +78,7 @@ describe 'visiting every page' do it 'tracks start and completion' do expect(events.where(name: 'module_start').size).to be 1 - expect(events.where(name: 'module_content_page').size).to be 34 + expect(events.where(name: 'module_content_page').size).to be 35 expect(events.where(name: 'module_complete').size).to eq 1 end end diff --git a/spec/system/feedback_form_spec.rb b/spec/system/feedback_spec.rb similarity index 62% rename from spec/system/feedback_form_spec.rb rename to spec/system/feedback_spec.rb index 2208ec168..40212ceb5 100644 --- a/spec/system/feedback_form_spec.rb +++ b/spec/system/feedback_spec.rb @@ -1,17 +1,15 @@ require 'rails_helper' -RSpec.describe 'Feedback form' do +RSpec.describe 'Feedback' do before do visit '/' end - let(:feedback_url) { Rails.configuration.feedback_url } - describe 'in beta banner' do specify do within '.govuk-phase-banner' do expect(page).to have_text 'This is a new service, your feedback will help us improve it.' - expect(page).to have_link 'feedback', href: feedback_url + expect(page).to have_link 'feedback', href: '/feedback' end end end @@ -19,7 +17,7 @@ describe 'in footer' do specify do within '.govuk-footer' do - expect(page).to have_link 'Feedback', href: feedback_url + expect(page).to have_link 'Feedback', href: '/feedback' end end end diff --git a/spec/system/module_feedback_spec.rb b/spec/system/module_feedback_spec.rb new file mode 100644 index 000000000..d919baac8 --- /dev/null +++ b/spec/system/module_feedback_spec.rb @@ -0,0 +1,53 @@ +require 'rails_helper' + +RSpec.describe 'Module feedback' do + include_context 'with progress' + include_context 'with user' + + before do + visit '/modules/alpha/questionnaires/feedback-radio-only' + end + + it 'validates answers' do + click_on 'Next' + expect(page).to have_content 'Please select an answer' + end + + describe 'one-off questions' do + context 'when not already answered' do + it 'pagination counts the question' do + expect(page).to have_content 'Page 1 of 9' + visit '/modules/alpha/questionnaires/feedback-skippable' + expect(page).to have_content 'Page 8 of 9' + end + end + + context 'when already answered' do + before do + create :response, + question_name: 'feedback-skippable', + training_module: 'bravo', + answers: [1], + correct: true, + user: user, + question_type: 'feedback' + end + + # FeedbackPaginationDecorator#page_total + it 'pagination does not count the question' do + visit '/modules/alpha/questionnaires/feedback-radio-only' + expect(page).to have_content 'Page 1 of 8' + end + + # Training::ResponsesController#redirect + it 'skips the question' do + visit '/modules/alpha/questionnaires/feedback-checkbox-other-or' + expect(page).to have_content 'Page 7 of 8' + check 'response-answers-1-field' + click_on 'Next' + expect(page).to have_content 'Page 8 of 8' + expect(page).to have_current_path '/modules/alpha/content-pages/1-3-3-5' + end + end + end +end diff --git a/spec/system/module_overview_content_spec.rb b/spec/system/module_overview_content_spec.rb index f66dd51ef..dee7e6991 100644 --- a/spec/system/module_overview_content_spec.rb +++ b/spec/system/module_overview_content_spec.rb @@ -9,19 +9,19 @@ end it 'has back button' do - expect(page).to have_link('Back to My modules', href: '/my-modules') + expect(page).to have_link 'Back to My modules', href: '/my-modules' end it 'has the module number and name' do - expect(page).to have_content('Module 1: First Training Module') + expect(page).to have_content 'Module 1: First Training Module' end it 'has the module description' do - expect(page).to have_content('first module description') + expect(page).to have_content 'first module description' end it 'has a call to action button to start the module' do - expect(page).to have_link('Start module') + expect(page).to have_link 'Start module' end it 'has the section headings' do @@ -30,6 +30,11 @@ .and have_content('Summary and next steps') end + it 'hides feedback section' do + expect(page).to have_content 'Reflect on your learning' + expect(page).not_to have_content 'Additional feedback' + end + it 'has the topic names' do expect(page).to have_content('1-1-1') .and have_content('1-1-2') diff --git a/spec/system/module_overview_progress_spec.rb b/spec/system/module_overview_progress_spec.rb index 52c67d6c6..606b7c078 100644 --- a/spec/system/module_overview_progress_spec.rb +++ b/spec/system/module_overview_progress_spec.rb @@ -27,7 +27,7 @@ expect(page).not_to have_link 'Reflect on your learning' end - within '#section-content-4' do + within '#section-content-5' do expect(page).to have_content 'not started', count: 1 expect(page).not_to have_link 'Download your certificate' end @@ -198,7 +198,7 @@ expect(page).to have_content 'complete', count: 3 end - within '#section-content-4' do + within '#section-content-5' do expect(page).to have_content 'complete', count: 1 end end diff --git a/spec/system/my_learning_spec.rb b/spec/system/my_learning_spec.rb index 5acc07787..3a596c2e0 100644 --- a/spec/system/my_learning_spec.rb +++ b/spec/system/my_learning_spec.rb @@ -71,7 +71,7 @@ expect(page).not_to have_text 'You have not started any modules.' expect(page).to have_text 'You have read 1 pages' # FIXME: plurality - expect(page).to have_text 'Your progress: 3%' + expect(page).to have_text 'Your progress: 2%' end end end diff --git a/spec/system/page_title_spec.rb b/spec/system/page_title_spec.rb index c9a65e0f3..92cb5eee9 100644 --- a/spec/system/page_title_spec.rb +++ b/spec/system/page_title_spec.rb @@ -4,6 +4,10 @@ context 'when user is not authenticated' do it { expect(root_path).to have_page_title 'Home page' } + it { expect(feedback_index_path).to have_page_title 'Feedback' } + it { expect(feedback_path('feedback-radio-only')).to have_page_title 'feedback-radio-only' } + it { expect(feedback_path('thank-you')).to have_page_title 'Thank you' } + it { expect(course_overview_path).to have_page_title 'About training' } it { expect(experts_path).to have_page_title 'The experts' } it { expect(about_path('alpha')).to have_page_title 'First Training Module' } @@ -48,39 +52,48 @@ context 'and viewing module content' do [ - ['what-to-expect', 'First Training Module : What to expect during the training'], - ['1-1', 'First Training Module : The first submodule'], - ['1-1-1', 'First Training Module : 1-1-1'], - ['1-1-2', 'First Training Module : 1-1-2'], - ['1-1-3', 'First Training Module : 1-1-3'], - ['1-1-3-1', 'First Training Module : 1-1-3-1'], - ['1-1-4', 'First Training Module : 1-1-4'], - ['1-2', 'First Training Module : The second submodule'], - ['1-2-1', 'First Training Module : 1-2-1'], - ['1-2-1-1', 'First Training Module : 1-2-1-1'], - ['1-2-1-2', 'First Training Module : 1-2-1-2'], - ['1-2-1-3', 'First Training Module : 1-2-1-3'], - ['1-3', 'First Training Module : Summary and next steps'], - ['1-3-1', 'First Training Module : Recap'], - ['1-3-2', 'First Training Module : End of module test'], - ['1-3-2-1', 'First Training Module : 1-3-2-1'], - ['1-3-2-2', 'First Training Module : 1-3-2-2'], - ['1-3-2-3', 'First Training Module : 1-3-2-3'], - ['1-3-2-4', 'First Training Module : 1-3-2-4'], - ['1-3-2-5', 'First Training Module : 1-3-2-5'], - ['1-3-2-6', 'First Training Module : 1-3-2-6'], - ['1-3-2-7', 'First Training Module : 1-3-2-7'], - ['1-3-2-8', 'First Training Module : 1-3-2-8'], - ['1-3-2-9', 'First Training Module : 1-3-2-9'], - ['1-3-2-10', 'First Training Module : 1-3-2-10'], - ['1-3-2-11', 'First Training Module : Assessment results'], - ['1-3-3', 'First Training Module : Reflect on your learning'], - ['1-3-3-1', 'First Training Module : 1-3-3-1'], - ['1-3-3-2', 'First Training Module : 1-3-3-2'], - ['1-3-3-3', 'First Training Module : 1-3-3-3'], - ['1-3-3-4', 'First Training Module : 1-3-3-4'], - ['1-3-3-5', 'First Training Module : Thank you'], - ['1-3-4', 'First Training Module : Download your certificate'], + ['what-to-expect', 'First Training Module : What to expect during the training'], + ['1-1', 'First Training Module : The first submodule'], + ['1-1-1', 'First Training Module : 1-1-1'], + ['1-1-2', 'First Training Module : 1-1-2'], + ['1-1-3', 'First Training Module : 1-1-3'], + ['1-1-3-1', 'First Training Module : 1-1-3-1'], + ['1-1-4', 'First Training Module : 1-1-4'], + ['1-2', 'First Training Module : The second submodule'], + ['1-2-1', 'First Training Module : 1-2-1'], + ['1-2-1-1', 'First Training Module : 1-2-1-1'], + ['1-2-1-2', 'First Training Module : 1-2-1-2'], + ['1-2-1-3', 'First Training Module : 1-2-1-3'], + ['1-3', 'First Training Module : Summary and next steps'], + ['1-3-1', 'First Training Module : Recap'], + ['1-3-2', 'First Training Module : End of module test'], + ['1-3-2-1', 'First Training Module : 1-3-2-1'], + ['1-3-2-2', 'First Training Module : 1-3-2-2'], + ['1-3-2-3', 'First Training Module : 1-3-2-3'], + ['1-3-2-4', 'First Training Module : 1-3-2-4'], + ['1-3-2-5', 'First Training Module : 1-3-2-5'], + ['1-3-2-6', 'First Training Module : 1-3-2-6'], + ['1-3-2-7', 'First Training Module : 1-3-2-7'], + ['1-3-2-8', 'First Training Module : 1-3-2-8'], + ['1-3-2-9', 'First Training Module : 1-3-2-9'], + ['1-3-2-10', 'First Training Module : 1-3-2-10'], + ['1-3-2-11', 'First Training Module : Assessment results'], + ['1-3-3', 'First Training Module : Reflect on your learning'], + ['1-3-3-1', 'First Training Module : 1-3-3-1'], + ['1-3-3-2', 'First Training Module : 1-3-3-2'], + ['1-3-3-3', 'First Training Module : 1-3-3-3'], + ['1-3-3-4', 'First Training Module : 1-3-3-4'], + ['feedback-intro', 'First Training Module : Additional feedback'], + ['feedback-radio-only', 'First Training Module : feedback-radio-only'], + ['feedback-checkbox-only', 'First Training Module : feedback-checkbox-only'], + ['feedback-textarea-only', 'First Training Module : feedback-textarea-only'], + ['feedback-radio-other-more', 'First Training Module : feedback-radio-other-more'], + ['feedback-checkbox-other-more', 'First Training Module : feedback-checkbox-other-more'], + ['feedback-radio-more', 'First Training Module : feedback-radio-more'], + ['feedback-checkbox-other-or', 'First Training Module : feedback-checkbox-other-or'], + ['feedback-skippable', 'First Training Module : feedback-skippable'], + ['1-3-3-5', 'First Training Module : Thank you'], + ['1-3-4', 'First Training Module : Download your certificate'], ].each do |page, title| describe "/modules/alpha/content-pages/#{page}" do let(:path) do diff --git a/spec/system/registered_user/closing_account_spec.rb b/spec/system/registered_user/closing_account_spec.rb index 06fd7f3d4..ed44aa059 100644 --- a/spec/system/registered_user/closing_account_spec.rb +++ b/spec/system/registered_user/closing_account_spec.rb @@ -67,11 +67,16 @@ end context 'when on confirmation page' do - let!(:note) { create(:note) } - before do - user.notes.push(note) - user.save! + user.notes.create!(training_module: 'alpha', body: 'this is a note') + user.responses.feedback.create!( + training_module: 'course', + question_name: 'feedback-textarea-only', + question_type: 'feedback', + text_input: 'this is feedback', + correct: true, + ) + visit '/my-account/close/confirm' end @@ -95,6 +100,8 @@ expect(user.last_name).to eq 'User' expect(user.email).to have_text 'redacted_user' expect(user.notes.count).to eq 0 + expect(user.responses.feedback.count).to eq 1 + expect(user.responses.feedback.first.text_input).to be_nil expect(user.valid_password?('RedactedUser12!@')).to eq true end end diff --git a/spec/system/summative_assessment_spec.rb b/spec/system/summative_assessment_spec.rb index 856406ab9..08b634a81 100644 --- a/spec/system/summative_assessment_spec.rb +++ b/spec/system/summative_assessment_spec.rb @@ -4,6 +4,7 @@ include_context 'with progress' include_context 'with user' + let(:fixture) { 'spec/support/ast/alpha-pass-response-skip-feedback.yml' } let(:first_question_path) { '/modules/alpha/questionnaires/1-3-2-1' } before do @@ -58,7 +59,7 @@ let(:fixture) do if Rails.application.migrated_answers? - 'spec/support/ast/alpha-pass-response.yml' + 'spec/support/ast/alpha-pass-response-skip-feedback.yml' else 'spec/support/ast/alpha-pass.yml' end diff --git a/ui/e2e_spec.rb b/ui/e2e_spec.rb index 8cd3b0e23..59350b0d2 100644 --- a/ui/e2e_spec.rb +++ b/ui/e2e_spec.rb @@ -7,7 +7,7 @@ YAML.load_file('ui/support/ast/child-development-and-the-eyfs.yml') end - def make_note(field, input) + def input_text(field, input) fill_in field, with: input end diff --git a/ui/support/ast/child-development-and-the-eyfs.yml b/ui/support/ast/child-development-and-the-eyfs.yml index 6528a49f2..6470f10f9 100644 --- a/ui/support/ast/child-development-and-the-eyfs.yml +++ b/ui/support/ast/child-development-and-the-eyfs.yml @@ -24,7 +24,7 @@ :path: /modules/child-development-and-the-eyfs/content-pages/1-1-1-1 :text: How understanding child development benefits the child :inputs: - - - :make_note + - - :input_text - note-body-field - hello world - - :click_on @@ -43,7 +43,7 @@ :path: /modules/child-development-and-the-eyfs/content-pages/1-1-1-2 :text: How understanding child development benefits the practitioner :inputs: - - - :make_note + - - :input_text - note-body-field - hello world - - :click_on @@ -62,7 +62,7 @@ :path: /modules/child-development-and-the-eyfs/content-pages/1-1-1-3 :text: How understanding child development benefits the wider support network :inputs: - - - :make_note + - - :input_text - note-body-field - hello world - - :click_on @@ -71,7 +71,7 @@ :path: /modules/child-development-and-the-eyfs/content-pages/1-1-1-4 :text: The potential impact of limited child development knowledge :inputs: - - - :make_note + - - :input_text - note-body-field - hello world - - :click_on @@ -90,7 +90,7 @@ :path: /modules/child-development-and-the-eyfs/content-pages/1-1-1-5 :text: How improved child development knowledge supports practice - scaffolding learning :inputs: - - - :make_note + - - :input_text - note-body-field - hello world - - :click_on @@ -119,7 +119,7 @@ :path: /modules/child-development-and-the-eyfs/content-pages/1-1-1-7 :text: How improved child development knowledge supports practice - effective holistic support :inputs: - - - :make_note + - - :input_text - note-body-field - hello world - - :click_on @@ -224,7 +224,7 @@ :path: /modules/child-development-and-the-eyfs/content-pages/1-1-2-9 :text: The importance of language skills :inputs: - - - :make_note + - - :input_text - note-body-field - hello world - - :click_on @@ -275,7 +275,7 @@ :path: /modules/child-development-and-the-eyfs/content-pages/1-2-1-1 :text: Acknowledging children’s individual strengths :inputs: - - - :make_note + - - :input_text - note-body-field - hello world - - :click_on @@ -300,7 +300,7 @@ :path: /modules/child-development-and-the-eyfs/content-pages/1-2-1-3 :text: Supporting individual potential :inputs: - - - :make_note + - - :input_text - note-body-field - hello world - - :click_on @@ -309,7 +309,7 @@ :path: /modules/child-development-and-the-eyfs/content-pages/1-2-1-4 :text: Successful enabling environments :inputs: - - - :make_note + - - :input_text - note-body-field - hello world - - :click_on @@ -338,7 +338,7 @@ :path: /modules/child-development-and-the-eyfs/content-pages/1-2-1-5 :text: 'A balanced approach to learning: child led play' :inputs: - - - :make_note + - - :input_text - note-body-field - hello world - - :click_on @@ -389,7 +389,7 @@ :path: /modules/child-development-and-the-eyfs/content-pages/1-2-2-1 :text: An effective EYFS :inputs: - - - :make_note + - - :input_text - note-body-field - hello world - - :click_on