diff --git a/.env.example b/.env.example index 24dd3951a..58c48edbf 100644 --- a/.env.example +++ b/.env.example @@ -14,6 +14,9 @@ ROLLBAR_ENV=development DATABASE_URL=postgres://postgres@localhost:5432/buy-for-your-school-development REDIS_URL=redis://localhost:6379 +# Miscellaneous +DAYS_A_JOURNEY_CAN_BE_INACTIVE_FOR=30 + # Contentful CONTENTFUL_URL=cdn.contentful.com CONTENTFUL_SPACE= @@ -31,4 +34,4 @@ DFE_SIGN_IN_ISSUER=https://test-oidc.signin.education.gov.uk:443 DFE_SIGN_IN_IDENTIFIER=buyforyourschool DFE_SIGN_IN_SECRET= DFE_SIGN_IN_REDIRECT_URL=http://localhost:3000/auth/dfe/callback -DFE_SIGN_IN_ENABLED=true +DFE_SIGN_IN_ENABLED=false diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c99389af..e8736ce31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,14 @@ The format is based on [Keep a Changelog 1.0.0]. ## [Unreleased] +## [release-007] - 2021-05-19 + +- Add `noindex,nofollow` meta tag to all pages, as per Gov.UK guidance +- fix API auth by switching mechanism from Basic to Token +- remove `Returning to this specification` URL from task list +- Add Tasks to the database when iterating through Sections from Contentful +- fix XSS vulnerability by sanitising all user answers + ## [release-006] - 2021-04-01 - specification templates are now sourced from the Contentful Category entry @@ -55,6 +63,7 @@ The format is based on [Keep a Changelog 1.0.0]. - add new dashboard page with the ability to create new specifications - users can only see their past journeys from the dashboard - new API endpoint to allow Contentful to invalidate cached entries, allowing caching to stay on which prevents the app from being very slow/crashing on journey start +- automatically delete Journey and associated records if we deem it to have become stale, to reclaim the unused database rows ## [release-005] - 2021-1-19 @@ -113,7 +122,8 @@ Contentful fixture - Contentful can redirect users to preview endpoints - users can be asked to answer a long text question -[unreleased]: https://github.com/DFE-Digital/buy-for-your-school/compare/release-006...HEAD +[unreleased]: https://github.com/DFE-Digital/buy-for-your-school/compare/release-007...HEAD +[release-007]: https://github.com/DFE-Digital/buy-for-your-school/compare/release-006...release-007 [release-006]: https://github.com/DFE-Digital/buy-for-your-school/compare/release-005...release-006 [release-005]: https://github.com/DFE-Digital/buy-for-your-school/compare/release-004...release-005 [release-004]: https://github.com/DFE-Digital/buy-for-your-school/compare/release-003...release-004 diff --git a/Gemfile b/Gemfile index a38027de5..70ab9fc8b 100644 --- a/Gemfile +++ b/Gemfile @@ -7,8 +7,8 @@ ruby "2.6.6" gem "bootsnap", ">= 1.1.0", require: false gem "climate_control" gem "coffee-rails", "~> 5.0" -gem "contentful", "~> 2.15" -gem "govuk_design_system_formbuilder", "~> 2.2" +gem "contentful", "~> 2.16" +gem "govuk_design_system_formbuilder", "~> 2.5" gem "high_voltage" gem "htmltoword" gem "jbuilder", "~> 2.11" @@ -33,7 +33,7 @@ gem "tzinfo-data", platforms: %i[mingw mswin x64_mingw jruby] gem "uglifier", ">= 1.3.0" group :development do - gem "listen", ">= 3.0.5", "< 3.5" + gem "listen", ">= 3.0.5", "< 3.6" gem "spring" gem "spring-watcher-listen", "~> 2.0.0" gem "web-console", ">= 3.3.0" diff --git a/Gemfile.lock b/Gemfile.lock index 47c8f9ba6..978acf4fd 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -99,7 +99,7 @@ GEM coffee-script-source (1.12.2) concurrent-ruby (1.1.8) connection_pool (2.2.3) - contentful (2.15.4) + contentful (2.16.0) http (> 0.8, < 5.0) multi_json (~> 1) crack (0.4.5) @@ -111,6 +111,7 @@ GEM activerecord (>= 5.a) database_cleaner-core (~> 2.0.0) database_cleaner-core (2.0.1) + deep_merge (1.2.1) diff-lcs (1.4.4) docile (1.3.4) domain_name (0.5.20190701) @@ -130,7 +131,7 @@ GEM railties (>= 5.0.0) faker (2.17.0) i18n (>= 1.6, < 2) - ffi (1.14.2) + ffi (1.15.0) ffi-compiler (1.0.1) ffi (>= 1.0.0) rake @@ -139,10 +140,11 @@ GEM raabro (~> 1.4) globalid (0.4.2) activesupport (>= 4.2.0) - govuk_design_system_formbuilder (2.2.0) - actionview (>= 5.2) - activemodel (>= 5.2) - activesupport (>= 5.2) + govuk_design_system_formbuilder (2.5.0) + actionview (>= 6.0) + activemodel (>= 6.0) + activesupport (>= 6.0) + deep_merge (~> 1.2.1) hashdiff (1.0.1) hashie (4.1.0) high_voltage (3.1.2) @@ -158,10 +160,10 @@ GEM http-cookie (1.0.3) domain_name (~> 0.5) http-form_data (2.3.0) - http-parser (1.2.1) + http-parser (1.2.3) ffi-compiler (>= 1.0, < 2.0) httpclient (2.8.3) - i18n (1.8.9) + i18n (1.8.10) concurrent-ruby (~> 1.0) jbuilder (2.11.2) activesupport (>= 5.0.0) @@ -175,12 +177,12 @@ GEM bindata launchy (2.5.0) addressable (~> 2.7) - libv8 (8.4.255.0) - liquid (5.0.0) - listen (3.4.1) + libv8-node (15.14.0.0) + liquid (5.0.1) + listen (3.5.1) rb-fsevent (~> 0.10, >= 0.10.3) rb-inotify (~> 0.9, >= 0.9.10) - loofah (2.9.0) + loofah (2.9.1) crass (~> 1.0.2) nokogiri (>= 1.5.9) mail (2.7.1) @@ -189,15 +191,15 @@ GEM method_source (1.0.0) mini_mime (1.0.3) mini_portile2 (2.5.0) - mini_racer (0.3.1) - libv8 (~> 8.4.255) + mini_racer (0.4.0) + libv8-node (~> 15.14.0.0) minitest (5.14.4) mock_redis (0.27.3) ruby2_keywords msgpack (1.4.2) multi_json (1.15.0) nio4r (2.5.7) - nokogiri (1.11.2) + nokogiri (1.11.3) mini_portile2 (~> 2.5.0) racc (~> 1.4) omniauth (1.9.1) @@ -221,10 +223,10 @@ GEM validate_url webfinger (>= 1.0.1) parallel (1.20.1) - parser (3.0.0.0) + parser (3.0.1.0) ast (~> 2.4.1) pg (1.2.3) - pry (0.14.0) + pry (0.14.1) coderay (~> 1.1) method_source (~> 1.0) public_suffix (4.0.6) @@ -278,7 +280,7 @@ GEM redis-namespace (1.8.1) redis (>= 3.0.4) regexp_parser (2.1.1) - rexml (3.2.4) + rexml (3.2.5) rollbar (3.1.2) rspec-core (3.10.1) rspec-support (~> 3.10.0) @@ -297,7 +299,7 @@ GEM rspec-mocks (~> 3.10) rspec-support (~> 3.10) rspec-support (3.10.2) - rubocop (1.11.0) + rubocop (1.12.1) parallel (~> 1.10) parser (>= 3.0.0.0) rainbow (>= 2.2.2, < 4.0) @@ -329,7 +331,7 @@ GEM rubyzip (>= 1.2.2) shoulda-matchers (4.5.1) activesupport (>= 4.2.0) - sidekiq (6.2.0) + sidekiq (6.2.1) connection_pool (>= 2.2.2) rack (~> 2.0) redis (>= 4.2.0) @@ -355,8 +357,8 @@ GEM actionpack (>= 4.0) activesupport (>= 4.0) sprockets (>= 3.0.0) - standard (1.0.4) - rubocop (= 1.11.0) + standard (1.0.5) + rubocop (= 1.12.1) rubocop-performance (= 1.10.1) swd (1.2.0) activesupport (>= 3) @@ -390,7 +392,7 @@ GEM webfinger (1.1.0) activesupport httpclient (>= 2.4) - webmock (3.12.1) + webmock (3.12.2) addressable (>= 2.3.6) crack (>= 0.3.2) hashdiff (>= 0.4.0, < 2.0.0) @@ -413,19 +415,19 @@ DEPENDENCIES capybara (>= 2.15) climate_control coffee-rails (~> 5.0) - contentful (~> 2.15) + contentful (~> 2.16) database_cleaner dotenv-rails factory_bot_rails faker - govuk_design_system_formbuilder (~> 2.2) + govuk_design_system_formbuilder (~> 2.5) high_voltage htmltoword jbuilder (~> 2.11) jquery-rails launchy liquid - listen (>= 3.0.5, < 3.5) + listen (>= 3.0.5, < 3.6) mini_racer mock_redis omniauth diff --git a/README.md b/README.md index 3974d5cf8..86c7c83b4 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,7 @@ A service to help school buying professionals create tender documents that compl 1. [Release process](/doc/release-process.md) 1. [Managing environment variables](/doc/managing-environment-variables.md) 1. [DfE Sign-in](/doc/dfe-sign-in.md) +1. [Console access](/doc/console-access.md) ## Making changes diff --git a/app/assets/stylesheets/components/_specification.scss b/app/assets/stylesheets/components/_specification.scss index a934a1ec6..5074ba1b2 100644 --- a/app/assets/stylesheets/components/_specification.scss +++ b/app/assets/stylesheets/components/_specification.scss @@ -15,4 +15,28 @@ @extend .govuk-list; @extend .govuk-list--number; } + + table { + @extend .govuk-table !optional; + } + + thead { + @extend .govuk-table__head !optional; + } + + tbody { + @extend .govuk-table__body !optional; + } + + tr { + @extend .govuk-table__row !optional; + } + + th { + @extend .govuk-table__header !optional; + } + + td { + @extend .govuk-table__cell !optional; + } } diff --git a/app/controllers/answers_controller.rb b/app/controllers/answers_controller.rb index e4526c6e9..528abb3e4 100644 --- a/app/controllers/answers_controller.rb +++ b/app/controllers/answers_controller.rb @@ -14,11 +14,7 @@ def create @answer = AnswerFactory.new(step: @step).call @answer.step = @step - result = SaveAnswer.new(answer: @answer).call( - further_information_params: further_information_params, - answer_params: answer_params, - date_params: date_params - ) + result = SaveAnswer.new(answer: @step.answer).call(params: prepared_params(step: @step)) @answer = result.object if result.success? @@ -33,11 +29,7 @@ def update @step = Step.find(step_id) @step_presenter = StepPresenter.new(@step) - result = SaveAnswer.new(answer: @step.answer).call( - further_information_params: further_information_params, - answer_params: answer_params, - date_params: date_params - ) + result = SaveAnswer.new(answer: @step.answer).call(params: prepared_params(step: @step)) @answer = result.object if result.success? @@ -57,6 +49,17 @@ def step_id params[:step_id] end + def prepared_params(step:) + case step.contentful_type + when "checkboxes", "radios" + further_information_params + when "single_date" + date_params + else + answer_params + end + end + def answer_params params.require(:answer).permit(:response, :further_information) end diff --git a/app/controllers/api/contentful/entries_controller.rb b/app/controllers/api/contentful/entries_controller.rb index a791b7591..9cffe3fb0 100644 --- a/app/controllers/api/contentful/entries_controller.rb +++ b/app/controllers/api/contentful/entries_controller.rb @@ -1,6 +1,8 @@ class Api::Contentful::EntriesController < ApplicationController + before_action :authenticate_api_user! + skip_before_action :authenticate_user! - http_basic_authenticate_with name: "api", password: ENV["CONTENTFUL_WEBHOOK_API_KEY"] + skip_before_action :verify_authenticity_token def changed Cache.delete(key: cache_key) @@ -14,4 +16,10 @@ def changed private def changed_params params.permit("entityId") end + + private def authenticate_api_user! + authenticate_or_request_with_http_token do |token, _options| + token == ENV["CONTENTFUL_WEBHOOK_API_KEY"] + end + end end diff --git a/app/controllers/preview/entries_controller.rb b/app/controllers/preview/entries_controller.rb index 92a660ba2..aebefe3f9 100644 --- a/app/controllers/preview/entries_controller.rb +++ b/app/controllers/preview/entries_controller.rb @@ -1,4 +1,6 @@ class Preview::EntriesController < ApplicationController + before_action :check_app_is_running_in_preview_env + def show @journey = Journey.create( category: "catering", @@ -19,4 +21,9 @@ def show def entry_id params[:id] end + + def check_app_is_running_in_preview_env + return if ENV["CONTENTFUL_PREVIEW_APP"].eql?("true") + render file: "public/404.html", status: :not_found, layout: false + end end diff --git a/app/jobs/delete_stale_journeys_job.rb b/app/jobs/delete_stale_journeys_job.rb new file mode 100644 index 000000000..ed6eca1e0 --- /dev/null +++ b/app/jobs/delete_stale_journeys_job.rb @@ -0,0 +1,9 @@ +class DeleteStaleJourneysJob < ApplicationJob + queue_as :default + + def perform + DeleteStaleJourneys.new.call + + Rollbar.info("Delete stale journeys task complete.") + end +end diff --git a/app/jobs/warm_entry_cache_job.rb b/app/jobs/warm_entry_cache_job.rb index 362895eff..60b01abc1 100644 --- a/app/jobs/warm_entry_cache_job.rb +++ b/app/jobs/warm_entry_cache_job.rb @@ -2,19 +2,23 @@ class WarmEntryCacheJob < ApplicationJob include CacheableEntry queue_as :caching - sidekiq_options retry: 5 def perform category = GetCategory.new(category_entry_id: ENV["CONTENTFUL_DEFAULT_CATEGORY_ENTRY_ID"]).call sections = GetSectionsFromCategory.new(category: category).call - steps = sections.map { |section| GetStepsFromSection.new(section: section).call } + steps = begin + sections.map { |section| GetStepsFromSection.new(section: section).call } + rescue GetStepsFromSection::RepeatEntryDetected + cache.extend_ttl_on_all_entries + return + end # TODO: Cache category and sections too [steps].flatten.each do |entry| store_in_cache(cache: cache, key: "contentful:entry:#{entry.id}", entry: entry) end - rescue GetStepsFromSection::RepeatEntryDetected - cache.extend_ttl_on_all_entries + + Rollbar.info("Cache warming task complete.") end private diff --git a/app/models/journey.rb b/app/models/journey.rb index fdc342671..1dea375b1 100644 --- a/app/models/journey.rb +++ b/app/models/journey.rb @@ -1,6 +1,7 @@ class Journey < ApplicationRecord self.implicit_order_column = "created_at" has_many :steps + has_many :sections has_many :visible_steps, -> { where(steps: {hidden: false}) }, class_name: "Step" belongs_to :user @@ -9,4 +10,12 @@ class Journey < ApplicationRecord def all_steps_completed? visible_steps.all? { |step| step.answer.present? } end + + def freshen! + attributes = {} + attributes[:last_worked_on] = Time.zone.now + attributes[:started] = true unless started == true + + update(attributes) + end end diff --git a/app/models/section.rb b/app/models/section.rb new file mode 100644 index 000000000..a4f71545d --- /dev/null +++ b/app/models/section.rb @@ -0,0 +1,7 @@ +class Section < ApplicationRecord + self.implicit_order_column = "created_at" + belongs_to :journey + has_many :tasks + + validates :title, :contentful_id, presence: true +end diff --git a/app/models/task.rb b/app/models/task.rb new file mode 100644 index 000000000..4708e9ac3 --- /dev/null +++ b/app/models/task.rb @@ -0,0 +1,6 @@ +class Task < ApplicationRecord + self.implicit_order_column = "created_at" + belongs_to :section + + validates :title, :contentful_id, presence: true +end diff --git a/app/services/create_journey.rb b/app/services/create_journey.rb index 1011c6ae5..150d2cc21 100644 --- a/app/services/create_journey.rb +++ b/app/services/create_journey.rb @@ -8,23 +8,32 @@ def initialize(category_name:, user:) def call category = GetCategory.new(category_entry_id: ENV["CONTENTFUL_DEFAULT_CATEGORY_ENTRY_ID"]).call - sections = GetSectionsFromCategory.new(category: category).call + contentful_sections = GetSectionsFromCategory.new(category: category).call journey = Journey.new( category: category_name, - liquid_template: category.specification_template, - user: user + user: user, + started: true, + last_worked_on: Time.zone.now, + liquid_template: category.specification_template ) - journey.section_groups = build_section_groupings(sections: sections) + journey.section_groups = build_section_groupings(sections: contentful_sections) journey.save! - sections.each do |section| - question_entries = GetStepsFromSection.new(section: section).call + contentful_sections.each do |contentful_section| + CreateSection.new(journey: journey, contentful_section: contentful_section).call + + question_entries = GetStepsFromSection.new(section: contentful_section).call question_entries.each do |entry| CreateJourneyStep.new( journey: journey, contentful_entry: entry ).call end + + tasks = GetTasksFromSection.new(section: contentful_section).call + tasks.each do |task| + CreateTask.new(section: contentful_section, contentful_task: task) + end end journey diff --git a/app/services/create_section.rb b/app/services/create_section.rb new file mode 100644 index 000000000..978ce1bcb --- /dev/null +++ b/app/services/create_section.rb @@ -0,0 +1,14 @@ +class CreateSection + attr_accessor :journey, :contentful_section + + def initialize(journey:, contentful_section:) + @journey = journey + @contentful_section = contentful_section + end + + def call + section = Section.new(journey: journey, contentful_id: contentful_section.id, title: contentful_section.title) + section.save! + section + end +end diff --git a/app/services/create_task.rb b/app/services/create_task.rb new file mode 100644 index 000000000..5f7d543ed --- /dev/null +++ b/app/services/create_task.rb @@ -0,0 +1,14 @@ +class CreateTask + attr_accessor :section, :contentful_task + + def initialize(section:, contentful_task:) + @section = section + @contentful_task = contentful_task + end + + def call + task = Task.new(section: section, title: contentful_task.title, contentful_id: contentful_task.id) + task.save! + task + end +end diff --git a/app/services/delete_stale_journeys.rb b/app/services/delete_stale_journeys.rb new file mode 100644 index 000000000..8ad517941 --- /dev/null +++ b/app/services/delete_stale_journeys.rb @@ -0,0 +1,14 @@ +class DeleteStaleJourneys + def call + qualifying_journeys = Journey.where(started: false) + .where("last_worked_on < ?", date_a_journey_can_be_inactive_until) + qualifying_journeys.destroy_all + end + + private def date_a_journey_can_be_inactive_until + return 1.month.ago unless ENV["DAYS_A_JOURNEY_CAN_BE_INACTIVE_FOR"].present? + + day_count = ENV["DAYS_A_JOURNEY_CAN_BE_INACTIVE_FOR"].to_i + day_count.days.ago + end +end diff --git a/app/services/get_answers_for_steps.rb b/app/services/get_answers_for_steps.rb index 5b25111ab..11a1e9d50 100644 --- a/app/services/get_answers_for_steps.rb +++ b/app/services/get_answers_for_steps.rb @@ -23,7 +23,9 @@ def call else raise UnexpectedAnswer.new("Trying to present an unknown type of answer: #{step.answer.class.name}") end - hash["answer_#{step.contentful_id}"] = answer.to_param.with_indifferent_access + safe_answer = StringSanitiser.new(args: answer.to_param).call + + hash["answer_#{step.contentful_id}"] = safe_answer.with_indifferent_access } end end diff --git a/app/services/get_tasks_from_section.rb b/app/services/get_tasks_from_section.rb new file mode 100644 index 000000000..414a56615 --- /dev/null +++ b/app/services/get_tasks_from_section.rb @@ -0,0 +1,19 @@ +class GetTasksFromSection + attr_accessor :section + + def initialize(section:) + self.section = section + end + + def call + return [] unless section.respond_to?(:tasks) + task_ids = [] + section.tasks.each do |task| + task_ids << task.id + end + + task_ids.map { |entry_id| + GetEntry.new(entry_id: entry_id).call + } + end +end diff --git a/app/services/save_answer.rb b/app/services/save_answer.rb index 925fdb24c..4f7a1cfcf 100644 --- a/app/services/save_answer.rb +++ b/app/services/save_answer.rb @@ -5,20 +5,15 @@ def initialize(answer:) self.step = answer.step end - def call(answer_params: {}, further_information_params: {}, date_params: {}) + def call(params:) result = Result.new(false, answer) - case step.contentful_type - when "checkboxes", "radios" - answer.assign_attributes(further_information_params) - when "single_date" - answer.assign_attributes(date_params) - else - answer.assign_attributes(answer_params) - end + safe_params = StringSanitiser.new(args: params.to_hash).call + answer.assign_attributes(safe_params) if answer.valid? answer.save + answer.step.journey.freshen! ToggleAdditionalSteps.new(step: answer.step).call result.success = true end diff --git a/app/services/string_sanitiser.rb b/app/services/string_sanitiser.rb new file mode 100644 index 000000000..539a02413 --- /dev/null +++ b/app/services/string_sanitiser.rb @@ -0,0 +1,32 @@ +class StringSanitiser + attr_accessor :args + def initialize(args:) + self.args = args + end + + def call + args.deep_merge(args) do |_, _, value| + if value.is_a?(Array) + value = value.map do |sub_value| + if sub_value.is_a?(String) + sub_value = sanitize(sub_value) + end + sub_value + end + elsif value.is_a?(String) + value = sanitize(value) + end + + value + end + end + + private def sanitize(value) + safe_list_sanitizer = Rails::Html::SafeListSanitizer.new + safe_list_sanitizer.sanitize(value, tags: allowed_html_tags) + end + + private def allowed_html_tags + %w[p b i] + end +end diff --git a/app/views/journeys/_resume_journey_notice.html.erb b/app/views/journeys/_resume_journey_notice.html.erb deleted file mode 100644 index 51efe08de..000000000 --- a/app/views/journeys/_resume_journey_notice.html.erb +++ /dev/null @@ -1,15 +0,0 @@ -
-
-

- <%= I18n.t("resume.notification.title") %> -

-
-
-

- <%= I18n.t("resume.notification.body") %> -

-

- <%=journey_url(@journey) %> -

-
-
diff --git a/app/views/journeys/show.html.erb b/app/views/journeys/show.html.erb index 0bbbeeb89..ab5c5d4aa 100644 --- a/app/views/journeys/show.html.erb +++ b/app/views/journeys/show.html.erb @@ -3,8 +3,6 @@ <%= link_to I18n.t("generic.button.back"), dashboard_path, class: "govuk-back-link" %>

<%= I18n.t("specifying.start_page.page_title") %>

-<%= render "resume_journey_notice" %> -
    <% section_group_with_steps(journey: @journey, steps: @step_presenters).each do |grouping| %>

    <%= grouping["title"] %>

    diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index e9ec97acb..c52241019 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -10,6 +10,7 @@ + <%= favicon_link_tag %> <%= favicon_link_tag 'govuk-mask-icon.svg', rel: 'mask-icon', type: 'image/svg+xml', color: '#0b0c0c' %> diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb index b4b90970f..b4cb562e4 100644 --- a/config/initializers/sidekiq.rb +++ b/config/initializers/sidekiq.rb @@ -1,5 +1,5 @@ Sidekiq.configure_server do |config| - config.redis = {url: "#{ENV["REDIS_URL"]}/2"} + config.redis = {url: "#{ENV["REDIS_URL"]}/0"} # Sidekiq Cron schedule_file = "config/schedule.yml" @@ -9,5 +9,5 @@ end Sidekiq.configure_client do |config| - config.redis = {url: "#{ENV["REDIS_URL"]}/2"} + config.redis = {url: "#{ENV["REDIS_URL"]}/0"} end diff --git a/config/locales/en.yml b/config/locales/en.yml index 48f6a8fcb..3fbd34747 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -86,10 +86,6 @@ en: edit_step_link_text: "Edit step in Contentful" preview_step_link_text: "Preview step in service" spec_template_tag_title: "Specification tag" - resume: - notification: - title: "Returning to this specification" - body: "You can return to this page to make changes or view your specification at any time, either by bookmarking this page in your browser or by making a note of the following address:" errors: contentful_entry_not_found: page_title: "An unexpected error occurred" diff --git a/config/schedule.yml b/config/schedule.yml index 544798e82..6c0414d1d 100644 --- a/config/schedule.yml +++ b/config/schedule.yml @@ -6,3 +6,10 @@ warm_contentful_entry_cache: provide some resilience to downtime or minor connection issues. As this service is used infrequently by real users, we automatically update the cache. + +delete_stale_journeys: + cron: "0 2 * * *" + class: "DeleteStaleJourneysJob" + description: Automatically remove Journey and associated child records when + we identify it as having become stale. This is intended to free up unused + database rows given every new specification generates 100-200 rows. diff --git a/db/migrate/20210330105313_add_started_state_to_journey.rb b/db/migrate/20210330105313_add_started_state_to_journey.rb new file mode 100644 index 000000000..28819fee9 --- /dev/null +++ b/db/migrate/20210330105313_add_started_state_to_journey.rb @@ -0,0 +1,6 @@ +class AddStartedStateToJourney < ActiveRecord::Migration[6.1] + def change + add_column :journeys, :started, :boolean, default: true + add_index :journeys, :started + end +end diff --git a/db/migrate/20210330111201_add_stale_time_stamp_to_journey.rb b/db/migrate/20210330111201_add_stale_time_stamp_to_journey.rb new file mode 100644 index 000000000..d42ba69fa --- /dev/null +++ b/db/migrate/20210330111201_add_stale_time_stamp_to_journey.rb @@ -0,0 +1,6 @@ +class AddStaleTimeStampToJourney < ActiveRecord::Migration[6.1] + def change + add_column :journeys, :last_worked_on, :datetime + add_index :journeys, :last_worked_on + end +end diff --git a/db/migrate/20210401100528_create_sections.rb b/db/migrate/20210401100528_create_sections.rb new file mode 100644 index 000000000..e4d37741b --- /dev/null +++ b/db/migrate/20210401100528_create_sections.rb @@ -0,0 +1,9 @@ +class CreateSections < ActiveRecord::Migration[6.1] + def change + create_table :sections, id: :uuid do |t| + t.references :journey, type: :uuid + t.string :title, null: false + t.timestamps + end + end +end diff --git a/db/migrate/20210401105730_create_tasks.rb b/db/migrate/20210401105730_create_tasks.rb new file mode 100644 index 000000000..6f23efe4c --- /dev/null +++ b/db/migrate/20210401105730_create_tasks.rb @@ -0,0 +1,10 @@ +class CreateTasks < ActiveRecord::Migration[6.1] + def change + create_table :tasks, id: :uuid do |t| + t.references :section, type: :uuid + t.string :title, null: false + t.string :contentful_id, null: false + t.timestamps + end + end +end diff --git a/db/migrate/20210407111600_add_contentful_id_to_sections.rb b/db/migrate/20210407111600_add_contentful_id_to_sections.rb new file mode 100644 index 000000000..d82fca7ca --- /dev/null +++ b/db/migrate/20210407111600_add_contentful_id_to_sections.rb @@ -0,0 +1,5 @@ +class AddContentfulIdToSections < ActiveRecord::Migration[6.1] + def change + add_column :sections, :contentful_id, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 71b77fe97..95f44eb2b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_03_25_145645) do +ActiveRecord::Schema.define(version: 2021_04_07_111600) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" @@ -43,6 +43,10 @@ t.jsonb "section_groups" t.uuid "user_id" t.index ["user_id"], name: "index_journeys_on_user_id" + t.boolean "started", default: true + t.index ["started"], name: "index_journeys_on_started" + t.datetime "last_worked_on" + t.index ["last_worked_on"], name: "index_journeys_on_last_worked_on" end create_table "long_text_answers", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| @@ -70,6 +74,15 @@ t.index ["step_id"], name: "index_radio_answers_on_step_id" end + create_table "sections", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| + t.uuid "journey_id" + t.string "title", null: false + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + t.string "contentful_id" + t.index ["journey_id"], name: "index_sections_on_journey_id" + end + create_table "short_text_answers", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.uuid "step_id" t.string "response", null: false @@ -105,6 +118,15 @@ t.index ["journey_id"], name: "index_steps_on_journey_id" end + create_table "tasks", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| + t.uuid "section_id" + t.string "title", null: false + t.string "contentful_id", null: false + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + t.index ["section_id"], name: "index_tasks_on_section_id" + end + create_table "users", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.string "dfe_sign_in_uid", null: false t.datetime "created_at", precision: 6, null: false diff --git a/doc/adr/0001-use-pull-request-templates.md b/doc/architecture/decisions/0011-use-pull-request-templates.md similarity index 100% rename from doc/adr/0001-use-pull-request-templates.md rename to doc/architecture/decisions/0011-use-pull-request-templates.md diff --git a/doc/console-access.md b/doc/console-access.md new file mode 100644 index 000000000..8f11877be --- /dev/null +++ b/doc/console-access.md @@ -0,0 +1,51 @@ +# Console access + +We may need a way to access live environments for debugging or incident management purposes in future. + +If we do need to open a rails console on production we should pair through the commands we execute to mitigate the risk of data loss. + +## Prerequisites + +You must have an account that has been invited to the Government Platform as a Service (GPaaS) account. DfE PaaS organisation administrators should be able to invite you if you [request in DfE's #digital-tools-support Slack channel](https://ukgovernmentdfe.slack.com/archives/CMS9V0JQL). + +You must have have been given 'Space developer' access to the intended space, for example "sct-prod". Note 'Space manager' is a separate role and does not include all `Space developer` permissions. + +[You can sign in to check your account and permissions here](https://admin.london.cloud.service.gov.uk). + +## Access + +1. From a local terminal login to Cloud Foundry and select the intended space + ``` + $ cf login -a api.london.cloud.service.gov.uk -u REDACTED@digital.education.gov.uk + ``` +1. See all available spaces + ``` + $ cf spaces + ``` +1. Change space + ``` + $ cf space + ``` +1. View available services + ``` + $ cf apps + ``` +1. Connect to the environment (in this case production) + ``` + $ cf ssh + ``` +1. Navigate to the application + ``` + $ cd /srv/app + ``` +1. Run the intended commands + ``` + $ export PATH="$PATH:/usr/local/bin" + $ /usr/local/bin/ruby bin/rails c + ``` + + or + + ``` + $ /usr/local/bin/ruby bin/rake db:seed + ``` diff --git a/spec/factories/journey.rb b/spec/factories/journey.rb index 2ca6309be..dbae336c7 100644 --- a/spec/factories/journey.rb +++ b/spec/factories/journey.rb @@ -3,6 +3,8 @@ category { "catering" } liquid_template { "Your answer was {{ answer_47EI2X2T5EDTpJX9WjRR9p }}" } section_groups { [] } + started { true } + last_worked_on { Time.zone.now } association :user, factory: :user diff --git a/spec/factories/section.rb b/spec/factories/section.rb new file mode 100644 index 000000000..97d550add --- /dev/null +++ b/spec/factories/section.rb @@ -0,0 +1,7 @@ +FactoryBot.define do + factory :section do + title { "Section title" } + contentful_id { "5m26U35YLau4cOaJq6FXZA" } + association :journey + end +end diff --git a/spec/features/school_buying_professionals/preview_a_journey_step_spec.rb b/spec/features/school_buying_professionals/preview_a_journey_step_spec.rb index e1e2b31d7..1dec07afc 100644 --- a/spec/features/school_buying_professionals/preview_a_journey_step_spec.rb +++ b/spec/features/school_buying_professionals/preview_a_journey_step_spec.rb @@ -1,16 +1,24 @@ feature "Users can preview a journey step" do - scenario "the appropriate step is displayed" do - stub_contentful_entry( - entry_id: "radio-question", - fixture_filename: "steps/radio-question.json" - ) + context "when the user is on the preview environment" do + around do |example| + ClimateControl.modify(CONTENTFUL_PREVIEW_APP: "true") do + example.run + end + end - user_is_signed_in + scenario "the appropriate step is displayed" do + stub_contentful_entry( + entry_id: "radio-question", + fixture_filename: "steps/radio-question.json" + ) - visit preview_entry_path("radio-question") + user_is_signed_in - expect(page).to have_content("Which service do you need?") - expect(page).to have_content("Catering") - expect(page).to have_content("Cleaning") + visit preview_entry_path("radio-question") + + expect(page).to have_content("Which service do you need?") + expect(page).to have_content("Catering") + expect(page).to have_content("Cleaning") + end end end diff --git a/spec/features/school_buying_professionals/resume_a_journey_spec.rb b/spec/features/school_buying_professionals/resume_a_journey_spec.rb deleted file mode 100644 index 4c5fd4b87..000000000 --- a/spec/features/school_buying_professionals/resume_a_journey_spec.rb +++ /dev/null @@ -1,20 +0,0 @@ -require "rails_helper" - -feature "Users can see how to resume a journey" do - let(:user) { create(:user) } - before { user_is_signed_in(user: user) } - - context "on the task list" do - let(:journey) { create(:journey, user: user) } - - scenario "displays the notification banner" do - visit journey_path(journey) - expect(page).to have_content(I18n.t("resume.notification.title")) - end - - scenario "displays the journey URL" do - visit journey_path(journey) - expect(page).to have_content(journey_url(journey)) - end - end -end diff --git a/spec/features/visitors/see_a_planning_start_page_spec.rb b/spec/features/visitors/see_a_planning_start_page_spec.rb index c9fb749f4..95bf8374b 100644 --- a/spec/features/visitors/see_a_planning_start_page_spec.rb +++ b/spec/features/visitors/see_a_planning_start_page_spec.rb @@ -54,4 +54,9 @@ expect(page).to have_content(I18n.t("specifying.start_page.page_title")) end + + scenario "the start page has the right content headers" do + visit root_path + expect(page).to have_xpath("//meta[@name=\"robots\" and contains(@content, \"noindex,nofollow\")]", visible: false) + end end diff --git a/spec/fixtures/contentful/sections/tasks-section.json b/spec/fixtures/contentful/sections/tasks-section.json new file mode 100644 index 000000000..dae1a6245 --- /dev/null +++ b/spec/fixtures/contentful/sections/tasks-section.json @@ -0,0 +1,41 @@ +{ + "sys": { + "space": { + "sys": { + "type": "Link", + "linkType": "Space", + "id": "rwl7tyzv9sys" + } + }, + "id": "tasks-section", + "type": "Entry", + "createdAt": "2021-04-12T10:41:36.073Z", + "updatedAt": "2021-04-12T10:42:51.119Z", + "environment": { + "sys": { + "id": "develop", + "type": "Link", + "linkType": "Environment" + } + }, + "revision": 2, + "contentType": { + "sys": { + "type": "Link", + "linkType": "ContentType", + "id": "section" + } + }, + "locale": "en-US" + }, + "fields": { + "title": "Section with tasks", + "tasks": [{ + "sys": { + "type": "Link", + "linkType": "Entry", + "id": "checkboxes-task" + } + }] + } +} diff --git a/spec/fixtures/contentful/tasks/checkboxes-task.json b/spec/fixtures/contentful/tasks/checkboxes-task.json new file mode 100644 index 000000000..73cb68edf --- /dev/null +++ b/spec/fixtures/contentful/tasks/checkboxes-task.json @@ -0,0 +1,46 @@ +{ + "metadata": { + "tags": [] + }, + "sys": { + "space": { + "sys": { + "type": "Link", + "linkType": "Space", + "id": "rwl7tyzv9sys" + } + }, + "id": "checkboxes-task", + "type": "Entry", + "createdAt": "2021-04-12T09:38:48.990Z", + "updatedAt": "2021-04-12T10:11:05.670Z", + "environment": { + "sys": { + "id": "develop", + "type": "Link", + "linkType": "Environment" + } + }, + "revision": 3, + "contentType": { + "sys": { + "type": "Link", + "linkType": "ContentType", + "id": "task" + } + }, + "locale": "en-US" + }, + "fields": { + "name": "Task 1", + "steps": [ + { + "sys": { + "type": "Link", + "linkType": "Entry", + "id": "1DqhwF2XkJJ0Um6NSweWlZ" + } + } + ] + } +} diff --git a/spec/jobs/delete_stale_journeys_job_spec.rb b/spec/jobs/delete_stale_journeys_job_spec.rb new file mode 100644 index 000000000..ba18d0601 --- /dev/null +++ b/spec/jobs/delete_stale_journeys_job_spec.rb @@ -0,0 +1,36 @@ +require "rails_helper" + +RSpec.describe DeleteStaleJourneysJob, type: :job do + include ActiveJob::TestHelper + + before(:each) do + ActiveJob::Base.queue_adapter = :test + end + + describe ".perform_later" do + it "enqueues a job asynchronously on the default queue" do + expect { + described_class.perform_later + }.to have_enqueued_job.on_queue("default") + end + + it "destroys all unstarted journeys" do + more_than_one_month_ago = (1.month + 1.day).ago + less_than_one_month_ago = (1.month - 1.day).ago + + legacy_journey = create(:journey, started: true, last_worked_on: nil) + unstarted_journey = create(:journey, started: false, last_worked_on: more_than_one_month_ago) + becoming_stale_journey = create(:journey, started: false, last_worked_on: less_than_one_month_ago) + active_journey = create(:journey, started: true, last_worked_on: more_than_one_month_ago) + + described_class.perform_later + perform_enqueued_jobs + + remaining_journeys = Journey.all + expect(remaining_journeys).to include(legacy_journey) + expect(remaining_journeys).to include(active_journey) + expect(remaining_journeys).to include(becoming_stale_journey) + expect(remaining_journeys).not_to include(unstarted_journey) + end + end +end diff --git a/spec/models/journey_spec.rb b/spec/models/journey_spec.rb index 550a82166..b4c1d1220 100644 --- a/spec/models/journey_spec.rb +++ b/spec/models/journey_spec.rb @@ -2,6 +2,7 @@ RSpec.describe Journey, type: :model do it { should have_many(:steps) } + it { should have_many(:sections) } describe "validations" do it { is_expected.to validate_presence_of(:category) } @@ -64,4 +65,29 @@ end end end + + describe "freshen!" do + it "set started to true" do + journey = build(:journey, :catering) + journey.freshen! + expect(journey.reload.started).to eq(true) + end + + it "sets the last_worked_on to now" do + travel_to Time.zone.local(2004, 11, 24, 1, 4, 44) + journey = build(:journey, :catering) + + journey.freshen! + + expect(journey.last_worked_on).to eq(Time.zone.now) + end + + context "when started is already true" do + it "does not update the record" do + journey = build(:journey, :catering, started: true) + expect(journey).not_to receive(:update).with(started: true) + journey.freshen! + end + end + end end diff --git a/spec/models/section_spec.rb b/spec/models/section_spec.rb new file mode 100644 index 000000000..a96ba194e --- /dev/null +++ b/spec/models/section_spec.rb @@ -0,0 +1,10 @@ +require "rails_helper" + +RSpec.describe Section, type: :model do + it { should belong_to(:journey) } + + describe "validations" do + it { is_expected.to validate_presence_of(:title) } + it { is_expected.to validate_presence_of(:contentful_id) } + end +end diff --git a/spec/models/task_spec.rb b/spec/models/task_spec.rb new file mode 100644 index 000000000..984d007a1 --- /dev/null +++ b/spec/models/task_spec.rb @@ -0,0 +1,10 @@ +require "rails_helper" + +RSpec.describe Task, type: :model do + it { should belong_to(:section) } + + describe "validations" do + it { is_expected.to validate_presence_of(:title) } + it { is_expected.to validate_presence_of(:contentful_id) } + end +end diff --git a/spec/requests/cache_invalidation_spec.rb b/spec/requests/cache_invalidation_spec.rb index 4075ccab4..86607c668 100644 --- a/spec/requests/cache_invalidation_spec.rb +++ b/spec/requests/cache_invalidation_spec.rb @@ -20,8 +20,9 @@ } } - headers = {"HTTP_AUTHORIZATION" => ActionController::HttpAuthentication::Basic - .encode_credentials("api", ENV["CONTENTFUL_WEBHOOK_API_KEY"])} + credentials = ActionController::HttpAuthentication::Token + .encode_credentials(ENV["CONTENTFUL_WEBHOOK_API_KEY"]) + headers = {"AUTHORIZATION" => credentials} post "/api/contentful/entry_updated", { params: fake_contentful_webook_payload, @@ -45,9 +46,13 @@ } } - # No basic auth + credentials = ActionController::HttpAuthentication::Token + .encode_credentials("an invalid token!") + headers = {"AUTHORIZATION" => credentials} + post "/api/contentful/entry_updated", params: fake_contentful_webook_payload, + headers: headers, as: :json expect(response).to have_http_status(:unauthorized) diff --git a/spec/requests/entry_preview_spec.rb b/spec/requests/entry_preview_spec.rb index 8a4c99add..2d85d1f94 100644 --- a/spec/requests/entry_preview_spec.rb +++ b/spec/requests/entry_preview_spec.rb @@ -3,24 +3,47 @@ RSpec.describe "Entry previews", type: :request do before { user_is_signed_in } - it "creates a dummy journey and redirects to the question creation flow" do - entry_id = "123" - fake_journey = create(:journey) - expect(Journey).to receive(:create) - .with(category: anything, user: anything, liquid_template: anything) - .and_return(fake_journey) + context "when PREVIEW_APP is configured to true" do + around do |example| + ClimateControl.modify(CONTENTFUL_PREVIEW_APP: "true") do + example.run + end + end - fake_get_contentful_entry = instance_double(Contentful::Entry) - allow_any_instance_of(GetEntry).to receive(:call) - .and_return(fake_get_contentful_entry) + it "creates a dummy journey and redirects to the question creation flow" do + entry_id = "123" + fake_journey = create(:journey) + expect(Journey).to receive(:create) + .with(category: anything, user: anything, liquid_template: anything) + .and_return(fake_journey) - fake_step = create(:step, :radio) - allow_any_instance_of(CreateJourneyStep).to receive(:call) - .and_return(fake_step) + fake_get_contentful_entry = instance_double(Contentful::Entry) + allow_any_instance_of(GetEntry).to receive(:call) + .and_return(fake_get_contentful_entry) - get "/preview/entries/#{entry_id}" + fake_step = create(:step, :radio) + allow_any_instance_of(CreateJourneyStep).to receive(:call) + .and_return(fake_step) - expect(response).to have_http_status(:found) - expect(response).to redirect_to("/journeys/#{fake_journey.id}/steps/#{fake_step.id}") + get "/preview/entries/#{entry_id}" + + expect(response).to have_http_status(:found) + expect(response).to redirect_to("/journeys/#{fake_journey.id}/steps/#{fake_step.id}") + end + end + + context "when PREVIEW_APP is configured to false" do + around do |example| + ClimateControl.modify(CONTENTFUL_PREVIEW_APP: "false") do + example.run + end + end + + it "does not create a dummy journey and shows not_found" do + get "/preview/entries/123" + + expect(Journey).to_not receive(:create) + expect(response).to have_http_status(:not_found) + end end end diff --git a/spec/services/create_journey_spec.rb b/spec/services/create_journey_spec.rb index 4fda6cdaf..74a7082e4 100644 --- a/spec/services/create_journey_spec.rb +++ b/spec/services/create_journey_spec.rb @@ -32,6 +32,27 @@ expect(Journey.last.user).to eq(user) end + it "sets started to true (until questions have been answered)" do + stub_contentful_category( + fixture_filename: "category-with-no-steps.json", + stub_steps: false + ) + described_class.new(category_name: "catering", user: build(:user)).call + expect(Journey.last.started).to eq(true) + end + + it "sets last_worked_on to now" do + travel_to Time.zone.local(2004, 11, 24, 1, 4, 44) + stub_contentful_category( + fixture_filename: "category-with-no-steps.json", + stub_steps: false + ) + + described_class.new(category_name: "catering", user: build(:user)).call + + expect(Journey.last.last_worked_on).to eq(Time.zone.now) + end + it "stores a copy of the Liquid template" do stub_contentful_category( fixture_filename: "category-with-liquid-template.json" diff --git a/spec/services/create_journey_step_spec.rb b/spec/services/create_journey_step_spec.rb index b13bbe57f..7f8e772c7 100644 --- a/spec/services/create_journey_step_spec.rb +++ b/spec/services/create_journey_step_spec.rb @@ -5,7 +5,7 @@ context "when the new step is of type step" do it "creates a local copy of the new step" do journey = create(:journey, :catering) - fake_entry = fake_contentful_step( + fake_entry = fake_contentful_step_or_task( contentful_fixture_filename: "steps/radio-question.json" ) @@ -64,7 +64,7 @@ context "when the question is of type 'short_text'" do it "sets help_text and options to nil" do journey = create(:journey, :catering) - fake_entry = fake_contentful_step( + fake_entry = fake_contentful_step_or_task( contentful_fixture_filename: "steps/short-text-question.json" ) @@ -75,7 +75,7 @@ it "replaces spaces with underscores" do journey = create(:journey, :catering) - fake_entry = fake_contentful_step( + fake_entry = fake_contentful_step_or_task( contentful_fixture_filename: "steps/short-text-question.json" ) @@ -88,7 +88,7 @@ context "when the new entry has a body field" do it "updates the step with the body" do journey = create(:journey, :catering) - fake_entry = fake_contentful_step( + fake_entry = fake_contentful_step_or_task( contentful_fixture_filename: "steps/static-content.json" ) @@ -106,7 +106,7 @@ context "when the new entry has a 'primaryCallToAction' field" do it "updates the step with the body" do journey = create(:journey, :catering) - fake_entry = fake_contentful_step( + fake_entry = fake_contentful_step_or_task( contentful_fixture_filename: "steps/primary-button.json" ) @@ -121,7 +121,7 @@ context "when no 'primaryCallToAction' is provided" do it "default copy is used for the button" do journey = create(:journey, :catering) - fake_entry = fake_contentful_step( + fake_entry = fake_contentful_step_or_task( contentful_fixture_filename: "steps/no-primary-button.json" ) @@ -136,7 +136,7 @@ context "when no 'skipCallToAction' is provided" do it "default copy is used for the button" do journey = create(:journey, :catering) - fake_entry = fake_contentful_step( + fake_entry = fake_contentful_step_or_task( contentful_fixture_filename: "steps/skippable-checkboxes-question.json" ) @@ -151,7 +151,7 @@ context "when no 'alwaysShowTheUser' is provided" do it "default hidden to true" do journey = create(:journey, :catering) - fake_entry = fake_contentful_step( + fake_entry = fake_contentful_step_or_task( contentful_fixture_filename: "steps/no-hidden-field.json" ) @@ -166,7 +166,7 @@ context "when 'showAdditionalQuestion' is provided" do it "stores the rule as JSON" do journey = create(:journey, :catering) - fake_entry = fake_contentful_step( + fake_entry = fake_contentful_step_or_task( contentful_fixture_filename: "steps/show-one-additional-question.json" ) @@ -186,7 +186,7 @@ context "when the new entry has an unexpected content model" do it "raises an error" do journey = create(:journey, :catering) - fake_entry = fake_contentful_step( + fake_entry = fake_contentful_step_or_task( contentful_fixture_filename: "steps/unexpected-contentful-type.json" ) @@ -197,7 +197,7 @@ it "raises a rollbar event" do journey = create(:journey, :catering) - fake_entry = fake_contentful_step( + fake_entry = fake_contentful_step_or_task( contentful_fixture_filename: "steps/unexpected-contentful-type.json" ) @@ -220,7 +220,7 @@ context "when the new step has an unexpected step type" do it "raises an error" do journey = create(:journey, :catering) - fake_entry = fake_contentful_step( + fake_entry = fake_contentful_step_or_task( contentful_fixture_filename: "steps/unexpected-contentful-question-type.json" ) @@ -231,7 +231,7 @@ it "raises a rollbar event" do journey = create(:journey, :catering) - fake_entry = fake_contentful_step( + fake_entry = fake_contentful_step_or_task( contentful_fixture_filename: "steps/unexpected-contentful-question-type.json" ) diff --git a/spec/services/create_section_spec.rb b/spec/services/create_section_spec.rb new file mode 100644 index 000000000..3d8dfeda5 --- /dev/null +++ b/spec/services/create_section_spec.rb @@ -0,0 +1,15 @@ +require "rails_helper" + +RSpec.describe CreateSection do + let(:journey) { FactoryBot.create(:journey) } + let(:contentful_section) { double(id: "5m26U35YLau4cOaJq6FXZA", title: "Section A") } + + describe "#call" do + it "creates a new section" do + expect { described_class.new(journey: journey, contentful_section: contentful_section).call } + .to change { Section.count }.by(1) + expect(Section.last.title).to eql("Section A") + expect(Section.last.journey).to eql(journey) + end + end +end diff --git a/spec/services/create_task_spec.rb b/spec/services/create_task_spec.rb new file mode 100644 index 000000000..68862d434 --- /dev/null +++ b/spec/services/create_task_spec.rb @@ -0,0 +1,16 @@ +require "rails_helper" + +RSpec.describe CreateTask do + let(:section) { FactoryBot.create(:section) } + let(:contentful_task) { double(id: "5m26U35YLau4cOaJq6FXZA", title: "Task 1") } + + describe "#call" do + it "creates a new task" do + expect { described_class.new(section: section, contentful_task: contentful_task).call } + .to change { Task.count }.by(1) + expect(Task.last.title).to eql("Task 1") + expect(Task.last.contentful_id).to eql("5m26U35YLau4cOaJq6FXZA") + expect(Task.last.section).to eql(section) + end + end +end diff --git a/spec/services/delete_stale_journeys_spec.rb b/spec/services/delete_stale_journeys_spec.rb new file mode 100644 index 000000000..aafe4cb5f --- /dev/null +++ b/spec/services/delete_stale_journeys_spec.rb @@ -0,0 +1,105 @@ +require "rails_helper" + +RSpec.describe DeleteStaleJourneys do + before(:each) { travel_to Time.zone.local(2021, 4, 8, 14, 24, 0) } + after(:each) { travel_back } + + describe "#call" do + it "destroys a journey and all associated records" do + journey = create(:journey, started: false, last_worked_on: (1.month + 1.day).ago) + step = create(:step, :radio, journey: journey) + _radio_answer = create(:radio_answer, step: step) + _short_text_answer = create(:short_text_answer, step: step) + + DeleteStaleJourneys.new.call + + expect(Journey.count).to eq(0) + expect(Step.count).to eq(0) + expect(RadioAnswer.count).to eq(0) + expect(ShortTextAnswer.count).to eq(0) + end + + it "only destroys journeys we think of as stale" do + stale_journey = create(:journey, started: false, last_worked_on: 2.months.ago) + + about_to_become_stale_journey = create(:journey, started: false, last_worked_on: 1.month.ago) + recently_created_journey = create(:journey, started: false, last_worked_on: 4.days.ago) + recently_active_journey = create(:journey, started: true, last_worked_on: 4.days.ago) + old_active_journey = create(:journey, started: true, last_worked_on: 2.months.ago) + + DeleteStaleJourneys.new.call + + remaining_journeys = Journey.all + expect(remaining_journeys).not_to include(stale_journey) + + expect(remaining_journeys).to include(about_to_become_stale_journey) + expect(remaining_journeys).to include(recently_created_journey) + expect(remaining_journeys).to include(recently_active_journey) + expect(remaining_journeys).to include(old_active_journey) + end + + context "when the journey is marked as started" do + it "is not destroyed regardless of last_worked_on" do + legacy_journey_with_no_activity = create(:journey, started: true, last_worked_on: nil) + old_journey_with_activity = create(:journey, started: true, last_worked_on: (1.month + 1.day).ago) + recent_journey_with_activity = create(:journey, started: true, last_worked_on: (1.month - 1.day).ago) + + DeleteStaleJourneys.new.call + + remaining_journeys = Journey.all + expect(remaining_journeys).to include(legacy_journey_with_no_activity) + expect(remaining_journeys).to include(old_journey_with_activity) + expect(remaining_journeys).to include(recent_journey_with_activity) + end + end + + context "when the journey is not marked as started" do + context "and the journey hasn't been worked on for more than 1 month" do + it "is destroyed" do + journey = create(:journey, started: false, last_worked_on: (1.month + 1.day).ago) + DeleteStaleJourneys.new.call + expect { Journey.find(journey.id) }.to raise_error(ActiveRecord::RecordNotFound) + end + end + + context "and the journey has been worked on in the last 1 month" do + it "is not destroyed" do + journey = create(:journey, started: true, last_worked_on: (1.month - 1.day).ago) + DeleteStaleJourneys.new.call + expect(Journey.find(journey.id)).to eq(journey) + end + end + + context "and the journey has been worked on exactly 1 month ago" do + it "is not destroyed" do + journey = create(:journey, started: true, last_worked_on: 1.month.ago) + DeleteStaleJourneys.new.call + expect(Journey.find(journey.id)).to eq(journey) + end + end + end + + context "when the environment variable DAYS_A_JOURNEY_CAN_BE_INACTIVE_FOR is set" do + around do |example| + ClimateControl.modify( + DAYS_A_JOURNEY_CAN_BE_INACTIVE_FOR: "5" + ) do + example.run + end + end + + it "only deletes unstarted journeys that were last modified more than 5 days ago" do + stale_journey = create(:journey, started: false, last_worked_on: 6.days.ago) + about_to_become_stale_journey = create(:journey, started: false, last_worked_on: 5.days.ago) + active_journey = create(:journey, started: false, last_worked_on: 4.days.ago) + + DeleteStaleJourneys.new.call + + remaining_journeys = Journey.all + expect(remaining_journeys).not_to include(stale_journey) + expect(remaining_journeys).to include(about_to_become_stale_journey) + expect(remaining_journeys).to include(active_journey) + end + end + end +end diff --git a/spec/services/get_answers_for_steps_spec.rb b/spec/services/get_answers_for_steps_spec.rb index 096b61533..57a4d2eba 100644 --- a/spec/services/get_answers_for_steps_spec.rb +++ b/spec/services/get_answers_for_steps_spec.rb @@ -205,5 +205,19 @@ expect(result.keys).not_to include("answer_#{unanswerable_step.contentful_id}") end end + + context "when the answer includes script characters" do + it "removes them from the answer that is then saved" do + answer = create(:short_text_answer, + response: "A little text") + + expect_any_instance_of(StringSanitiser).to receive(:call).and_call_original + + result = described_class.new(visible_steps: [answer.step]).call + + expect(result["answer_#{answer.step.contentful_id}"]) + .to eql({"response" => "alert('problem');A little text"}) + end + end end end diff --git a/spec/services/get_tasks_from_section_spec.rb b/spec/services/get_tasks_from_section_spec.rb new file mode 100644 index 000000000..8c62399bd --- /dev/null +++ b/spec/services/get_tasks_from_section_spec.rb @@ -0,0 +1,20 @@ +require "rails_helper" + +RSpec.describe GetTasksFromSection do + describe "#call" do + it "returns the list of entry objects referenced by the task list" do + section = fake_contentful_section( + contentful_fixture_filename: "sections/tasks-section.json" + ) + stub_contentful_section_tasks(sections: [section]) + + result = described_class.new(section: section).call + + expect(result).to be_kind_of(Array) + # INFO: We should test this is a Contentful::Entry however it wasn't + # possible to create an instance_double due to an unusual way the object + # is constructed within the gem. Creating the object seems overly complex. + expect(result.first.id).to eq("checkboxes-task") + end + end +end diff --git a/spec/services/save_answer_spec.rb b/spec/services/save_answer_spec.rb index 086e81576..793912dbc 100644 --- a/spec/services/save_answer_spec.rb +++ b/spec/services/save_answer_spec.rb @@ -2,11 +2,11 @@ RSpec.describe SaveAnswer do describe "#call" do - it "updates the answer with the answer_params" do + it "updates the answer with the params" do answer = create(:short_text_answer) params = ActionController::Parameters.new(response: "A little text").permit! - result = described_class.new(answer: answer).call(answer_params: params) + result = described_class.new(answer: answer).call(params: params) expect(result.success?).to eql(true) expect(result.object.response).to eql("A little text") @@ -19,7 +19,7 @@ expect(ToggleAdditionalSteps).to receive(:new).with(step: answer.step).and_return(toggle_service) expect(toggle_service).to receive(:call) - described_class.new(answer: answer).call(answer_params: {}) + described_class.new(answer: answer).call(params: {}) end context "when the step is a checkbox question" do @@ -27,7 +27,7 @@ answer = create(:checkbox_answers) params = ActionController::Parameters.new(response: ["A", "B"]).permit! - result = described_class.new(answer: answer).call(further_information_params: params) + result = described_class.new(answer: answer).call(params: params) expect(result.success?).to eql(true) expect(result.object.response).to eql(["A", "B"]) @@ -40,7 +40,7 @@ date = Date.new(2000, 1, 29) params = {response: date} - result = described_class.new(answer: answer).call(date_params: params) + result = described_class.new(answer: answer).call(params: params) expect(result.success?).to eql(true) expect(result.object.response).to eql(date) @@ -54,7 +54,7 @@ allow(answer).to receive(:valid?).and_return(false) expect(answer).not_to receive(:save) - described_class.new(answer: answer).call(answer_params: {}) + described_class.new(answer: answer).call(params: {}) end it "returns a failed result" do @@ -63,10 +63,35 @@ allow(answer).to receive(:valid?).and_return(false) - result = described_class.new(answer: answer).call(answer_params: params) + result = described_class.new(answer: answer).call(params: params) expect(result.success?).to eql(false) end end + + context "when the associated journey is unstarted" do + it "updates the journey to be started" do + journey = create(:journey, started: false) + step = create(:step, :radio, journey: journey) + answer = create(:short_text_answer, step: step) + + _result = described_class.new(answer: answer).call(params: {}) + + expect(journey.reload.started).to eq(true) + end + end + + context "when the answer includes script characters" do + it "removes them from the answer that is then saved" do + answer = create(:short_text_answer) + params = ActionController::Parameters.new(response: "A little text").permit! + + expect_any_instance_of(StringSanitiser).to receive(:call).and_call_original + + result = described_class.new(answer: answer).call(params: params) + + expect(result.object.response).to eql("alert('problem');A little text") + end + end end end diff --git a/spec/services/string_sanitiser_spec.rb b/spec/services/string_sanitiser_spec.rb new file mode 100644 index 000000000..c21e950f3 --- /dev/null +++ b/spec/services/string_sanitiser_spec.rb @@ -0,0 +1,69 @@ +require "rails_helper" + +RSpec.describe StringSanitiser do + describe "#call" do + it "strips malicious tags from every value in a hash" do + args = {key: "Some allowed text"} + result = described_class.new(args: args).call + expect(result).to eq(key: "alert('problem');Some allowed text") + end + + it "doesn't strip safe tags" do + args = {key: "

    paragraph

    bolditalic"} + result = described_class.new(args: args).call + expect(result).to eq(key: "

    paragraph

    bolditalic") + end + + context "when the value is an Array" do + it "strips malicious tags from every value of the array (only going 1 level deep)" do + args = {key: ["Some allowed text", "Link"]} + result = described_class.new(args: args).call + expect(result).to eq(key: ["alert('problem');Some allowed text", "Link"]) + end + + context "when the array contains non string values" do + it "doesn't raise an error" do + date = Date.current + args = {key: ["A string", date, 1]} + + result = described_class.new(args: args).call + + expect(result).to eq(key: ["A string", date, 1]) + end + end + end + + context "when the value is a hash of more args" do + it "strips malicious tags from every value in every hash" do + args = { + first_hash_key: { + second_hash_key: "2", + nested_hash: { + third_hash_key: "Link" + } + } + } + result = described_class.new(args: args).call + expect(result).to eq( + first_hash_key: { + second_hash_key: "alert('problem');2", + nested_hash: { + third_hash_key: "Link" + } + } + ) + end + end + + context "when the value is a Date" do + it "returns the date without raising an error" do + date = Date.current + args = {key: date} + + result = described_class.new(args: args).call + + expect(result).to eq(key: date) + end + end + end +end diff --git a/spec/support/contentful_helpers.rb b/spec/support/contentful_helpers.rb index b1edab7c0..2ee361d7e 100644 --- a/spec/support/contentful_helpers.rb +++ b/spec/support/contentful_helpers.rb @@ -4,7 +4,7 @@ def stub_contentful_entry( fixture_filename: "radio-question-example.json" ) contentful_connector = stub_contentful_connector - contentful_response = fake_contentful_step(contentful_fixture_filename: fixture_filename) + contentful_response = fake_contentful_step_or_task(contentful_fixture_filename: fixture_filename) allow(contentful_connector).to receive(:get_entry_by_id) .with(entry_id) .and_return(contentful_response) @@ -63,7 +63,7 @@ def stub_contentful_section_steps( sections.each do |section| fake_steps = section.steps.map { |step| - fake_step = fake_contentful_step(contentful_fixture_filename: "steps/#{step.id}.json") + fake_step = fake_contentful_step_or_task(contentful_fixture_filename: "steps/#{step.id}.json") expect(contentful_connector).to receive(:get_entry_by_id) .with(fake_step.id) .and_return(fake_step) @@ -83,13 +83,32 @@ def stub_contentful_category_steps( .and_return(contentful_connector) category.steps.each do |step| - step = fake_contentful_step(contentful_fixture_filename: "steps/#{step.id}.json") + step = fake_contentful_step_or_task(contentful_fixture_filename: "steps/#{step.id}.json") allow(contentful_connector).to receive(:get_entry_by_id) .with(step.id) .and_return(step) end end + def stub_contentful_section_tasks( + sections:, + contentful_connector: instance_double(ContentfulConnector) + ) + allow(ContentfulConnector).to receive(:new) + .and_return(contentful_connector) + + sections.each do |section| + fake_tasks = section.tasks.map { |task| + fake_task = fake_contentful_step_or_task(contentful_fixture_filename: "tasks/#{task.id}.json") + expect(contentful_connector).to receive(:get_entry_by_id) + .with(fake_task.id) + .and_return(fake_task) + fake_task + } + allow(section).to receive(:tasks).and_return(fake_tasks) + end + end + def stub_contentful_connector contentful_connector = instance_double(ContentfulConnector) expect(ContentfulConnector).to receive(:new) @@ -125,15 +144,19 @@ def fake_contentful_section(contentful_fixture_filename:) raw_response = File.read("#{Rails.root}/spec/fixtures/contentful/#{contentful_fixture_filename}") hash_response = JSON.parse(raw_response) + steps = hash_response.dig("fields", "steps") + tasks = hash_response.dig("fields", "tasks") + double( Contentful::Entry, id: hash_response.dig("sys", "id"), title: hash_response.dig("fields", "title"), - steps: hash_response.dig("fields", "steps").map { |step_hash| double(id: step_hash.dig("sys", "id")) } + steps: steps.nil? ? {} : steps.map { |step_hash| double(id: step_hash.dig("sys", "id")) }, + tasks: tasks.nil? ? {} : tasks.map { |task_hash| double(id: task_hash.dig("sys", "id")) } ) end - def fake_contentful_step(contentful_fixture_filename:) + def fake_contentful_step_or_task(contentful_fixture_filename:) raw_response = File.read("#{Rails.root}/spec/fixtures/contentful/#{contentful_fixture_filename}") hash_response = JSON.parse(raw_response) diff --git a/spec/support/sign_in_helpers.rb b/spec/support/sign_in_helpers.rb index 4f903547e..12c8049ac 100644 --- a/spec/support/sign_in_helpers.rb +++ b/spec/support/sign_in_helpers.rb @@ -14,7 +14,7 @@ def user_sign_in_attempt_fails(dsi_uid: SecureRandom.uuid) def user_starts_the_journey visit root_path - click_link I18n.t("generic.button.start") + click_button I18n.t("generic.button.start") click_link I18n.t("dashboard.create.link") end diff --git a/terraform/app/cdn.tf b/terraform/app/cdn.tf new file mode 100644 index 000000000..ca70b4dab --- /dev/null +++ b/terraform/app/cdn.tf @@ -0,0 +1,18 @@ +data "cloudfoundry_service" "cdn_route" { + name = "cdn-route" +} + +resource "cloudfoundry_service_instance" "cdn_route" { + count = local.environment == "prod" ? 1 : (local.environment == "staging" ? 1 : 0) + name = local.environment == "prod" ? "get-help-buying-for-schools" : "get-help-buying-for-schools-staging" + space = data.cloudfoundry_space.space.id + service_plan = data.cloudfoundry_service.cdn_route.service_plans["cdn-route"] + json_params = jsonencode( + { + domain = "${local.custom_hostname}.${local.custom_cloudfoundry_domain}" + } + ) + lifecycle { + prevent_destroy = true + } +}