diff --git a/.env.example b/.env.example index df498325f..178000046 100644 --- a/.env.example +++ b/.env.example @@ -11,6 +11,7 @@ ROLLBAR_ENV=development DATABASE_URL=postgres://postgres@localhost:5432/buy-for-your-school-development REDIS_CACHE_URL=redis://localhost:6379/1 +REDIS_SIDEKIQ_URL=redis://localhost:6379/2 # Contentful CONTENTFUL_URL=cdn.contentful.com diff --git a/.env.test b/.env.test index 36b12b2ff..1ed0b00d1 100644 --- a/.env.test +++ b/.env.test @@ -6,7 +6,8 @@ # Reference: https://github.com/bkeepers/dotenv#what-other-env-files-can-i-use DATABASE_URL=postgres://postgres@localhost:5432/buy-for-your-school-test -REDIS_CACHE_URL=redis://localhost:6379/2 +REDIS_CACHE_URL=redis://localhost:6379/11 +REDIS_SIDEKIQ_URL=redis://localhost:6379/12 # Contentful CONTENTFUL_URL=cdn.contentful.com diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c4aae4a9..260c36446 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,21 @@ The format is based on [Keep a Changelog 1.0.0]. ## [Unreleased] +## [release-004] - 2020-12-17 + +- fix primary key type on long_text_answers table to UUID +- nightly task to warm the Contentful cache for all entries +- form button content is configurable through Contentful +- users can be asked to provide a single date answer +- add Webmock to prevent real http requests in the test suite +- content users can see a journey map of all the Contentful steps +- users can be asked to provide multiple answers via a checkbox question +- journey map shows an error to the content team if a duplicate entry is detected +- journey map shows an error to the content team if the journey doesn't end within 50 steps +- refactor how we store and access a Step's associated Contentful Entry ID +- users can edit their answers +- only add Contentful entries that form part of a valid journey to the cache + ## [release-003] - 2020-12-07 - add database foreign key constraints for better data integrity @@ -36,7 +51,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-003...HEAD +[unreleased]: https://github.com/DFE-Digital/buy-for-your-school/compare/release-004...HEAD +[release-004]: https://github.com/DFE-Digital/buy-for-your-school/compare/release-003...release-004 [release-003]: https://github.com/DFE-Digital/buy-for-your-school/compare/release-002...release-003 [release-002]: https://github.com/DFE-Digital/buy-for-your-school/compare/release-001...release-002 [release-001]: https://github.com/DFE-Digital/buy-for-your-school/compare/release-000...release-001 diff --git a/Gemfile b/Gemfile index 40627cfff..f8ffc488b 100644 --- a/Gemfile +++ b/Gemfile @@ -18,8 +18,10 @@ gem "puma", "~> 5.1" gem "redis", "~> 4.2" gem "redis-namespace" gem "rollbar" -gem "rails", "~> 6.0.0" +gem "rails", "~> 6.1.0" gem "sass-rails", "~> 6.0" +gem "sidekiq", "~> 6.1" +gem "sidekiq-cron", "~> 1.2" gem "turbolinks", "~> 5" gem "tzinfo-data", platforms: %i[mingw mswin x64_mingw jruby] gem "uglifier", ">= 1.3.0" @@ -37,6 +39,7 @@ group :test do gem "selenium-webdriver" gem "shoulda-matchers" gem "simplecov" + gem "webmock" end group :development do diff --git a/Gemfile.lock b/Gemfile.lock index 41d862126..3811a422a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,61 +1,65 @@ GEM remote: https://rubygems.org/ specs: - actioncable (6.0.3.4) - actionpack (= 6.0.3.4) + actioncable (6.1.0) + actionpack (= 6.1.0) + activesupport (= 6.1.0) nio4r (~> 2.0) websocket-driver (>= 0.6.1) - actionmailbox (6.0.3.4) - actionpack (= 6.0.3.4) - activejob (= 6.0.3.4) - activerecord (= 6.0.3.4) - activestorage (= 6.0.3.4) - activesupport (= 6.0.3.4) + actionmailbox (6.1.0) + actionpack (= 6.1.0) + activejob (= 6.1.0) + activerecord (= 6.1.0) + activestorage (= 6.1.0) + activesupport (= 6.1.0) mail (>= 2.7.1) - actionmailer (6.0.3.4) - actionpack (= 6.0.3.4) - actionview (= 6.0.3.4) - activejob (= 6.0.3.4) + actionmailer (6.1.0) + actionpack (= 6.1.0) + actionview (= 6.1.0) + activejob (= 6.1.0) + activesupport (= 6.1.0) mail (~> 2.5, >= 2.5.4) rails-dom-testing (~> 2.0) - actionpack (6.0.3.4) - actionview (= 6.0.3.4) - activesupport (= 6.0.3.4) - rack (~> 2.0, >= 2.0.8) + actionpack (6.1.0) + actionview (= 6.1.0) + activesupport (= 6.1.0) + rack (~> 2.0, >= 2.0.9) rack-test (>= 0.6.3) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.2.0) - actiontext (6.0.3.4) - actionpack (= 6.0.3.4) - activerecord (= 6.0.3.4) - activestorage (= 6.0.3.4) - activesupport (= 6.0.3.4) + actiontext (6.1.0) + actionpack (= 6.1.0) + activerecord (= 6.1.0) + activestorage (= 6.1.0) + activesupport (= 6.1.0) nokogiri (>= 1.8.5) - actionview (6.0.3.4) - activesupport (= 6.0.3.4) + actionview (6.1.0) + activesupport (= 6.1.0) builder (~> 3.1) erubi (~> 1.4) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.1, >= 1.2.0) - activejob (6.0.3.4) - activesupport (= 6.0.3.4) + activejob (6.1.0) + activesupport (= 6.1.0) globalid (>= 0.3.6) - activemodel (6.0.3.4) - activesupport (= 6.0.3.4) - activerecord (6.0.3.4) - activemodel (= 6.0.3.4) - activesupport (= 6.0.3.4) - activestorage (6.0.3.4) - actionpack (= 6.0.3.4) - activejob (= 6.0.3.4) - activerecord (= 6.0.3.4) + activemodel (6.1.0) + activesupport (= 6.1.0) + activerecord (6.1.0) + activemodel (= 6.1.0) + activesupport (= 6.1.0) + activestorage (6.1.0) + actionpack (= 6.1.0) + activejob (= 6.1.0) + activerecord (= 6.1.0) + activesupport (= 6.1.0) marcel (~> 0.3.1) - activesupport (6.0.3.4) + mimemagic (~> 0.3.2) + activesupport (6.1.0) concurrent-ruby (~> 1.0, >= 1.0.2) - i18n (>= 0.7, < 2) - minitest (~> 5.1) - tzinfo (~> 1.1) - zeitwerk (~> 2.2, >= 2.2.2) + i18n (>= 1.6, < 2) + minitest (>= 5.1) + tzinfo (~> 2.0) + zeitwerk (~> 2.3) addressable (2.7.0) public_suffix (>= 2.0.2, < 5.0) ast (2.4.1) @@ -68,7 +72,7 @@ GEM msgpack (~> 1.0) brakeman (4.10.0) builder (3.2.4) - bullet (6.1.0) + bullet (6.1.2) activesupport (>= 3.0.0) uniform_notifier (~> 1.11) byebug (11.1.3) @@ -91,9 +95,11 @@ GEM execjs coffee-script-source (1.12.2) concurrent-ruby (1.1.7) + connection_pool (2.2.3) contentful (2.15.4) http (> 0.8, < 5.0) multi_json (~> 1) + crack (0.4.4) crass (1.0.6) database_cleaner (1.8.5) diff-lcs (1.4.4) @@ -105,6 +111,8 @@ GEM dotenv (= 2.7.6) railties (>= 3.2) erubi (1.10.0) + et-orbi (1.2.4) + tzinfo execjs (2.7.0) factory_bot (6.1.0) activesupport (>= 5.0.0) @@ -117,12 +125,16 @@ GEM ffi-compiler (1.0.1) ffi (>= 1.0.0) rake + fugit (1.4.1) + et-orbi (~> 1.1, >= 1.1.8) + raabro (~> 1.4) globalid (0.4.2) activesupport (>= 4.2.0) govuk_design_system_formbuilder (2.1.5) actionview (>= 5.2) activemodel (>= 5.2) activesupport (>= 5.2) + hashdiff (1.0.1) high_voltage (3.1.2) http (4.4.1) addressable (~> 2.3) @@ -176,25 +188,26 @@ GEM coderay (~> 1.1) method_source (~> 1.0) public_suffix (4.0.6) - puma (5.1.0) + puma (5.1.1) nio4r (~> 2.0) + raabro (1.4.0) rack (2.2.3) rack-test (1.1.0) rack (>= 1.0, < 3) - rails (6.0.3.4) - actioncable (= 6.0.3.4) - actionmailbox (= 6.0.3.4) - actionmailer (= 6.0.3.4) - actionpack (= 6.0.3.4) - actiontext (= 6.0.3.4) - actionview (= 6.0.3.4) - activejob (= 6.0.3.4) - activemodel (= 6.0.3.4) - activerecord (= 6.0.3.4) - activestorage (= 6.0.3.4) - activesupport (= 6.0.3.4) - bundler (>= 1.3.0) - railties (= 6.0.3.4) + rails (6.1.0) + actioncable (= 6.1.0) + actionmailbox (= 6.1.0) + actionmailer (= 6.1.0) + actionpack (= 6.1.0) + actiontext (= 6.1.0) + actionview (= 6.1.0) + activejob (= 6.1.0) + activemodel (= 6.1.0) + activerecord (= 6.1.0) + activestorage (= 6.1.0) + activesupport (= 6.1.0) + bundler (>= 1.15.0) + railties (= 6.1.0) sprockets-rails (>= 2.0.0) rails-dom-testing (2.0.3) activesupport (>= 4.2.0) @@ -202,12 +215,12 @@ GEM rails-html-sanitizer (1.3.0) loofah (~> 2.3) rails_layout (1.0.42) - railties (6.0.3.4) - actionpack (= 6.0.3.4) - activesupport (= 6.0.3.4) + railties (6.1.0) + actionpack (= 6.1.0) + activesupport (= 6.1.0) method_source rake (>= 0.8.7) - thor (>= 0.20.3, < 2.0) + thor (~> 1.0) rainbow (3.0.0) rake (13.0.1) rb-fsevent (0.10.4) @@ -267,6 +280,13 @@ GEM rubyzip (>= 1.2.2) shoulda-matchers (4.4.1) activesupport (>= 4.2.0) + sidekiq (6.1.2) + connection_pool (>= 2.2.2) + rack (~> 2.0) + redis (>= 4.2.0) + sidekiq-cron (1.2.0) + fugit (~> 1.1) + sidekiq (>= 4.2.1) simplecov (0.20.0) docile (~> 1.1) simplecov-html (~> 0.11) @@ -286,17 +306,16 @@ GEM actionpack (>= 4.0) activesupport (>= 4.0) sprockets (>= 3.0.0) - standard (0.10.1) + standard (0.10.2) rubocop (= 1.4.2) rubocop-performance (= 1.9.1) thor (1.0.1) - thread_safe (0.3.6) tilt (2.0.10) turbolinks (5.2.1) turbolinks-source (~> 5.2) turbolinks-source (5.2.0) - tzinfo (1.2.8) - thread_safe (~> 0.1) + tzinfo (2.0.3) + concurrent-ruby (~> 1.0) uglifier (4.2.0) execjs (>= 0.3.0, < 3) unf (0.1.4) @@ -309,6 +328,10 @@ GEM activemodel (>= 6.0.0) bindex (>= 0.4.0) railties (>= 6.0.0) + webmock (3.10.0) + addressable (>= 2.3.6) + crack (>= 0.3.2) + hashdiff (>= 0.4.0, < 2.0.0) websocket-driver (0.7.3) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) @@ -344,7 +367,7 @@ DEPENDENCIES pg pry puma (~> 5.1) - rails (~> 6.0.0) + rails (~> 6.1.0) rails_layout redis (~> 4.2) redis-namespace @@ -353,6 +376,8 @@ DEPENDENCIES sass-rails (~> 6.0) selenium-webdriver shoulda-matchers + sidekiq (~> 6.1) + sidekiq-cron (~> 1.2) simplecov spring spring-commands-rspec @@ -362,6 +387,7 @@ DEPENDENCIES tzinfo-data uglifier (>= 1.3.0) web-console (>= 3.3.0) + webmock RUBY VERSION ruby 2.6.6p146 diff --git a/app/controllers/answers_controller.rb b/app/controllers/answers_controller.rb index c4019c885..1d1bd3c52 100644 --- a/app/controllers/answers_controller.rb +++ b/app/controllers/answers_controller.rb @@ -6,9 +6,15 @@ def create @step = Step.find(step_id) @answer = AnswerFactory.new(step: @step).call - @answer.assign_attributes(answer_params) @answer.step = @step + case @step.contentful_type + when "checkboxes" + @answer.assign_attributes(checkbox_params) + else + @answer.assign_attributes(answer_params) + end + if @answer.valid? @answer.save if @journey.next_entry_id.present? @@ -17,7 +23,22 @@ def create redirect_to journey_path(@journey) end else - render "steps/new.#{@step.contentful_type}" + render "steps/#{@step.contentful_type}", locals: {layout: "steps/new_form_wrapper"} + end + end + + def update + @journey = Journey.find(journey_id) + @step = Step.find(step_id) + @answer = @step.answer + + @answer.assign_attributes(answer_params) + + if @answer.valid? + @answer.save + redirect_to journey_path(@journey) + else + render "steps/#{@step.contentful_type}", locals: {layout: "steps/edit_form_wrapper"} end end @@ -34,4 +55,8 @@ def step_id def answer_params params.require(:answer).permit(:response) end + + def checkbox_params + params.require(:answer).permit(response: []) + end end diff --git a/app/controllers/journey_maps_controller.rb b/app/controllers/journey_maps_controller.rb new file mode 100644 index 000000000..eab7d3d64 --- /dev/null +++ b/app/controllers/journey_maps_controller.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class JourneyMapsController < ApplicationController + rescue_from BuildJourneyOrder::RepeatEntryDetected do |exception| + render "errors/repeat_step_in_the_contentful_journey", status: 500, locals: {error: exception} + end + + rescue_from BuildJourneyOrder::TooManyChainedEntriesDetected do |exception| + render "errors/too_many_steps_in_the_contentful_journey", status: 500, locals: {error: exception} + end + + def new + entries = GetAllContentfulEntries.new.call + @journey_map = BuildJourneyOrder.new( + entries: entries.to_a, + starting_entry_id: ENV["CONTENTFUL_PLANNING_START_ENTRY_ID"] + ).call + end +end diff --git a/app/controllers/journeys_controller.rb b/app/controllers/journeys_controller.rb index 39774f62d..3b8815076 100644 --- a/app/controllers/journeys_controller.rb +++ b/app/controllers/journeys_controller.rb @@ -8,8 +8,9 @@ def new def show @journey = Journey.includes( - steps: [:radio_answer, :short_text_answer] + steps: [:radio_answer, :short_text_answer, :long_text_answer] ).find(journey_id) + @steps = @journey.steps.map { |step| StepPresenter.new(step) } end private diff --git a/app/controllers/steps_controller.rb b/app/controllers/steps_controller.rb index 20cee9e3e..f25f4da6b 100644 --- a/app/controllers/steps_controller.rb +++ b/app/controllers/steps_controller.rb @@ -31,7 +31,15 @@ def show @step = Step.find(params[:id]) @answer = AnswerFactory.new(step: @step).call - render "new.#{@step.contentful_type}" + render @step.contentful_type, locals: {layout: "steps/new_form_wrapper"} + end + + def edit + @journey = Journey.find(journey_id) + @step = Step.find(params[:id]) + @answer = @step.answer + + render "steps/#{@step.contentful_type}", locals: {layout: "steps/edit_form_wrapper"} end private diff --git a/app/helpers/step_helper.rb b/app/helpers/step_helper.rb new file mode 100644 index 000000000..5b7ced134 --- /dev/null +++ b/app/helpers/step_helper.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module StepHelper + def radio_options(array_of_options:) + array_of_options.map { |option| + OpenStruct.new(id: option.downcase, name: option.titleize) + } + end + + def checkbox_options(array_of_options:) + array_of_options.map { |option| + OpenStruct.new(id: option.downcase, name: option.titleize) + } + end +end diff --git a/app/jobs/warm_entry_cache_job.rb b/app/jobs/warm_entry_cache_job.rb new file mode 100644 index 000000000..3b04dfcbe --- /dev/null +++ b/app/jobs/warm_entry_cache_job.rb @@ -0,0 +1,29 @@ +class WarmEntryCacheJob < ApplicationJob + include CacheableEntry + + queue_as :caching + sidekiq_options retry: 5 + + def perform + entries = BuildJourneyOrder.new( + entries: GetAllContentfulEntries.new.call, + starting_entry_id: ENV["CONTENTFUL_PLANNING_START_ENTRY_ID"] + ).call + + entries.each do |entry| + store_in_cache(cache: cache, key: "contentful:entry:#{entry.id}", entry: entry) + end + rescue BuildJourneyOrder::RepeatEntryDetected, + BuildJourneyOrder::TooManyChainedEntriesDetected + cache.extend_ttl_on_all_entries + end + + private + + def cache + @cache ||= Cache.new( + enabled: ENV.fetch("CONTENTFUL_ENTRY_CACHING"), + ttl: ENV.fetch("CONTENTFUL_ENTRY_CACHING_TTL", 60 * 60 * 72) + ) + end +end diff --git a/app/models/checkbox_answers.rb b/app/models/checkbox_answers.rb new file mode 100644 index 000000000..c93e5b78d --- /dev/null +++ b/app/models/checkbox_answers.rb @@ -0,0 +1,13 @@ +class CheckboxAnswers < ActiveRecord::Base + self.implicit_order_column = "created_at" + belongs_to :step + + validates :response, presence: true + + def response=(args) + return if args.blank? + args.reject!(&:blank?) if args.is_a?(Array) + + super(args) + end +end diff --git a/app/models/single_date_answer.rb b/app/models/single_date_answer.rb new file mode 100644 index 000000000..b85a192c3 --- /dev/null +++ b/app/models/single_date_answer.rb @@ -0,0 +1,6 @@ +class SingleDateAnswer < ActiveRecord::Base + self.implicit_order_column = "created_at" + belongs_to :step + + validates :response, presence: true +end diff --git a/app/models/step.rb b/app/models/step.rb index 6ce516683..dc464aeb3 100644 --- a/app/models/step.rb +++ b/app/models/step.rb @@ -5,16 +5,20 @@ class Step < ApplicationRecord has_one :radio_answer has_one :short_text_answer has_one :long_text_answer - - def radio_options - options.map { |option| OpenStruct.new(id: option.downcase, name: option.titleize) } - end + has_one :single_date_answer + has_one :checkbox_answers def answer - @answer ||= radio_answer || short_text_answer || long_text_answer + @answer ||= + radio_answer || + short_text_answer || + long_text_answer || + single_date_answer || + checkbox_answers end - def question? - contentful_model == "question" + def primary_call_to_action_text + return I18n.t("generic.button.next") unless super.present? + super end end diff --git a/app/presenters/step_presenter.rb b/app/presenters/step_presenter.rb new file mode 100644 index 000000000..e018c14d8 --- /dev/null +++ b/app/presenters/step_presenter.rb @@ -0,0 +1,5 @@ +class StepPresenter < SimpleDelegator + def question? + contentful_model == "question" + end +end diff --git a/app/services/answer_factory.rb b/app/services/answer_factory.rb index eb3cfc5d6..b42b9392e 100644 --- a/app/services/answer_factory.rb +++ b/app/services/answer_factory.rb @@ -10,6 +10,8 @@ def call when "radios" then RadioAnswer.new when "short_text" then ShortTextAnswer.new when "long_text" then LongTextAnswer.new + when "single_date" then SingleDateAnswer.new + when "checkboxes" then CheckboxAnswers.new end end end diff --git a/app/services/build_journey_order.rb b/app/services/build_journey_order.rb new file mode 100644 index 000000000..b2de6141d --- /dev/null +++ b/app/services/build_journey_order.rb @@ -0,0 +1,64 @@ +class BuildJourneyOrder + class RepeatEntryDetected < StandardError; end + + class TooManyChainedEntriesDetected < StandardError; end + + ENTRY_JOURNEY_MAX_LENGTH = 50 + + attr_accessor :entries, :starting_entry_id + + def initialize(entries:, starting_entry_id:) + self.entries = entries + self.starting_entry_id = starting_entry_id + end + + def call + recursive_path( + entry_lookup: entry_lookup_hash, + next_entry_id: starting_entry_id, + entries: [] + ) + end + + private + + def entry_lookup_hash + @entry_lookup_hash ||= entries.to_h { |entry| [entry.id, entry] } + end + + def recursive_path(entry_lookup:, next_entry_id:, entries:) + entry = entry_lookup.fetch(next_entry_id, nil) + + if entries.include?(entry) + send_rollbar_error(message: "A repeated Contentful entry was found in the same journey", entry_id: entry.id) + raise RepeatEntryDetected.new(entry.id) + end + + if entries.count >= ENTRY_JOURNEY_MAX_LENGTH + send_rollbar_error(message: "More than #{ENTRY_JOURNEY_MAX_LENGTH} steps were found in a journey map", entry_id: entry.id) + raise TooManyChainedEntriesDetected.new(entry.id) + end + + entries << entry if entry.present? + + if entry.respond_to?(:next) + recursive_path( + entry_lookup: entry_lookup, + next_entry_id: entry.next.id, + entries: entries + ) + else + entries + end + end + + def send_rollbar_error(message:, entry_id:) + Rollbar.error( + message, + contentful_url: ENV["CONTENTFUL_URL"], + contentful_space_id: ENV["CONTENTFUL_SPACE"], + contentful_environment: ENV["CONTENTFUL_ENVIRONMENT"], + contentful_entry_id: entry_id + ) + end +end diff --git a/app/services/cache.rb b/app/services/cache.rb index d6b0d1d0a..685a35757 100644 --- a/app/services/cache.rb +++ b/app/services/cache.rb @@ -1,8 +1,7 @@ class Cache attr_accessor :enabled, :key, :ttl - def initialize(enabled:, key:, ttl:) + def initialize(enabled:, ttl:) self.enabled = enabled - self.key = key self.ttl = ttl end @@ -10,7 +9,7 @@ def redis_cache @redis_cache ||= RedisCache.redis end - def hit? + def hit?(key:) if enabled == "true" redis_cache.exists?(key) else @@ -18,13 +17,22 @@ def hit? end end - def get + def get(key:) redis_cache.get(key) end - def set(value:) + def set(key:, value:) return unless enabled == "true" redis_cache.set(key, value) redis_cache.expire(key, ttl) end + + def extend_ttl_on_all_entries(extension: (60 * 60 * 24)) + redis_cache.keys("contentful:entry:*").map do |key| + previous_ttl = redis_cache.ttl(key) + extended_ttl = extension + previous_ttl + + redis_cache.expire(key, extended_ttl) + end + end end diff --git a/app/services/concerns/cacheable_entry.rb b/app/services/concerns/cacheable_entry.rb index f265f6225..3375184f4 100644 --- a/app/services/concerns/cacheable_entry.rb +++ b/app/services/concerns/cacheable_entry.rb @@ -1,23 +1,22 @@ module CacheableEntry extend ActiveSupport::Concern - def find_and_build_entry_from_cache(cache:) + def find_and_build_entry_from_cache(cache:, key:) Contentful::ResourceBuilder.new( - load_from_cache(cache: cache) + load_from_cache(cache: cache, key: key) ).run end - def store_in_cache(cache:, entry:) + def store_in_cache(cache:, key:, entry:) return unless entry.present? && entry.respond_to?(:raw) - - cache.set(value: JSON.dump(entry.raw.to_json)) + cache.set(key: key, value: JSON.dump(entry.raw.to_json)) end private - def load_from_cache(cache:) + def load_from_cache(cache:, key:) # rubocop:disable Security/JSONLoad - serialised_json_string = cache.get + serialised_json_string = cache.get(key: key) unserialised_json_string = JSON.restore(serialised_json_string) JSON.parse(unserialised_json_string) # rubocop:enable Security/JSONLoad diff --git a/app/services/create_journey_step.rb b/app/services/create_journey_step.rb index d8af03a5b..e4da8b37a 100644 --- a/app/services/create_journey_step.rb +++ b/app/services/create_journey_step.rb @@ -4,7 +4,14 @@ class UnexpectedContentfulModel < StandardError; end class UnexpectedContentfulStepType < StandardError; end ALLOWED_CONTENTFUL_MODELS = %w[question staticContent].freeze - ALLOWED_CONTENTFUL_ENTRY_TYPES = %w[radios short_text long_text paragraphs].freeze + ALLOWED_CONTENTFUL_ENTRY_TYPES = %w[ + radios + short_text + long_text + paragraphs + single_date + checkboxes + ].freeze attr_accessor :journey, :contentful_entry def initialize(journey:, contentful_entry:) @@ -27,9 +34,11 @@ def call title: title, help_text: help_text, body: body, + contentful_id: content_entry_id, contentful_model: content_model, contentful_type: step_type, options: options, + primary_call_to_action_text: primary_call_to_action_text, raw: raw, journey: journey ) @@ -88,6 +97,11 @@ def step_type contentful_entry.type.tr(" ", "_") end + def primary_call_to_action_text + return nil unless contentful_entry.respond_to?(:primary_call_to_action) + contentful_entry.primary_call_to_action + end + def raw contentful_entry.raw end diff --git a/app/services/get_contentful_entry.rb b/app/services/get_contentful_entry.rb index 665e81a12..304d3d975 100644 --- a/app/services/get_contentful_entry.rb +++ b/app/services/get_contentful_entry.rb @@ -9,17 +9,16 @@ def initialize(entry_id:, contentful_connector: ContentfulConnector.new) @contentful_connector = contentful_connector self.cache = Cache.new( enabled: ENV.fetch("CONTENTFUL_ENTRY_CACHING"), - key: "contentful:entry:#{entry_id}", - ttl: ENV.fetch("CONTENTFUL_ENTRY_CACHING_TTL", 172_800) # 48 hours + ttl: ENV.fetch("CONTENTFUL_ENTRY_CACHING_TTL", 60 * 60 * 72) ) end def call - if cache.hit? - entry = find_and_build_entry_from_cache(cache: cache) + if cache.hit?(key: cache_key) + entry = find_and_build_entry_from_cache(cache: cache, key: cache_key) else entry = @contentful_connector.get_entry_by_id(entry_id) - store_in_cache(cache: cache, entry: entry) + store_in_cache(cache: cache, key: cache_key, entry: entry) end if entry.nil? @@ -32,6 +31,10 @@ def call private + def cache_key + "contentful:entry:#{entry_id}" + end + def send_rollbar_warning Rollbar.warning( "The following Contentful entry identifier could not be found.", diff --git a/app/views/errors/repeat_step_in_the_contentful_journey.html.erb b/app/views/errors/repeat_step_in_the_contentful_journey.html.erb new file mode 100644 index 000000000..aba030975 --- /dev/null +++ b/app/views/errors/repeat_step_in_the_contentful_journey.html.erb @@ -0,0 +1,6 @@ +<%= content_for :title, I18n.t("errors.repeat_step_in_the_contentful_journey.page_title") %> + +

<%= I18n.t("errors.repeat_step_in_the_contentful_journey.page_title") %>

+

+ <%= I18n.t("errors.repeat_step_in_the_contentful_journey.page_body", entry_id: error.message) %> +

diff --git a/app/views/errors/too_many_steps_in_the_contentful_journey.html.erb b/app/views/errors/too_many_steps_in_the_contentful_journey.html.erb new file mode 100644 index 000000000..f63b298cf --- /dev/null +++ b/app/views/errors/too_many_steps_in_the_contentful_journey.html.erb @@ -0,0 +1,6 @@ +<%= content_for :title, I18n.t("errors.too_many_steps_in_the_contentful_journey.page_title") %> + +

<%= I18n.t("errors.too_many_steps_in_the_contentful_journey.page_title") %>

+

+ <%= I18n.t("errors.too_many_steps_in_the_contentful_journey.page_body", entry_id: error.message, step_count: BuildJourneyOrder::ENTRY_JOURNEY_MAX_LENGTH) %> +

diff --git a/app/views/journey_maps/new.html.erb b/app/views/journey_maps/new.html.erb new file mode 100644 index 000000000..02268a6b5 --- /dev/null +++ b/app/views/journey_maps/new.html.erb @@ -0,0 +1,10 @@ +<%= content_for :title, I18n.t("journey_map.page_title") %> +

<%= I18n.t("journey_map.page_title") %>

+ +
    + <% @journey_map.each do |entry| %> +
  1. + <%= link_to entry.title, "https://app.contentful.com/spaces/#{ENV["CONTENTFUL_SPACE"]}/environments/#{ENV["CONTENTFUL_ENVIRONMENT"]}/entries/#{entry.id}", target: :blank, class: "govuk-link" %> +
    2. + <% end %> +
diff --git a/app/views/journeys/_checkboxes_answer.html.erb b/app/views/journeys/_checkboxes_answer.html.erb new file mode 100644 index 000000000..5bb8d711b --- /dev/null +++ b/app/views/journeys/_checkboxes_answer.html.erb @@ -0,0 +1,5 @@ +
+
<%= step.title %>
+
<%= answer.response.reject(&:blank?).map(&:capitalize).join(", ") %>
+
<%= link_to I18n.t("generic.button.change_answer"), edit_journey_step_path(@journey, step), class: "govuk-link" %>
+
diff --git a/app/views/journeys/_long_text_answer.html.erb b/app/views/journeys/_long_text_answer.html.erb index e3fdd9bd6..6f13b8a51 100644 --- a/app/views/journeys/_long_text_answer.html.erb +++ b/app/views/journeys/_long_text_answer.html.erb @@ -1,4 +1,5 @@
<%= step.title %>
<%= simple_format(step.answer.response) %>
+
<%= link_to I18n.t("generic.button.change_answer"), edit_journey_step_path(@journey, step), class: "govuk-link" %>
diff --git a/app/views/journeys/_radios_answer.html.erb b/app/views/journeys/_radios_answer.html.erb index 7774e2258..feb47caec 100644 --- a/app/views/journeys/_radios_answer.html.erb +++ b/app/views/journeys/_radios_answer.html.erb @@ -1,4 +1,5 @@
<%= step.title %>
<%= answer.response.capitalize %>
+
<%= link_to I18n.t("generic.button.change_answer"), edit_journey_step_path(@journey, step), class: "govuk-link" %>
diff --git a/app/views/journeys/_short_text_answer.html.erb b/app/views/journeys/_short_text_answer.html.erb index 35e6b130e..2f009b423 100644 --- a/app/views/journeys/_short_text_answer.html.erb +++ b/app/views/journeys/_short_text_answer.html.erb @@ -1,4 +1,5 @@
<%= step.title %>
<%= answer.response %>
+
<%= link_to I18n.t("generic.button.change_answer"), edit_journey_step_path(@journey, step), class: "govuk-link" %>
diff --git a/app/views/journeys/_single_date_answer.html.erb b/app/views/journeys/_single_date_answer.html.erb new file mode 100644 index 000000000..2e008f4cc --- /dev/null +++ b/app/views/journeys/_single_date_answer.html.erb @@ -0,0 +1,5 @@ +
+
<%= step.title %>
+
<%= I18n.l(answer.response) %>
+
<%= link_to I18n.t("generic.button.change_answer"), edit_journey_step_path(@journey, step), class: "govuk-link" %>
+
diff --git a/app/views/journeys/show.html.erb b/app/views/journeys/show.html.erb index d98c848ca..ceb4f5d5f 100644 --- a/app/views/journeys/show.html.erb +++ b/app/views/journeys/show.html.erb @@ -2,7 +2,7 @@

<%= @journey.category.capitalize %>

- <% @journey.steps.each do |step| %> + <% @steps.each do |step| %> <% if step.question? %> <%= render "#{step.contentful_type}_answer", step: step, answer: step.answer %> <% end %> diff --git a/app/views/steps/_edit_form_wrapper.html.erb b/app/views/steps/_edit_form_wrapper.html.erb new file mode 100644 index 000000000..72300574c --- /dev/null +++ b/app/views/steps/_edit_form_wrapper.html.erb @@ -0,0 +1,5 @@ +<%= content_for :title, @step.title %> +<%= form_for @answer, as: :answer, method: "put", url: journey_step_answer_path(@journey, @step, @answer) do |f| %> + <%= yield f %> + <%= f.submit I18n.t("generic.button.update"), class: "govuk-button" %> +<% end %> diff --git a/app/views/steps/_new_form_wrapper.html.erb b/app/views/steps/_new_form_wrapper.html.erb new file mode 100644 index 000000000..f72359486 --- /dev/null +++ b/app/views/steps/_new_form_wrapper.html.erb @@ -0,0 +1,5 @@ +<%= content_for :title, @step.title %> +<%= form_for @answer, as: :answer, url: journey_step_answers_path(@journey, @step), method: "post" do |f| %> + <%= yield f %> + <%= f.submit @step.primary_call_to_action_text, class: "govuk-button" %> +<% end %> diff --git a/app/views/steps/checkboxes.html.erb b/app/views/steps/checkboxes.html.erb new file mode 100644 index 000000000..63d43274f --- /dev/null +++ b/app/views/steps/checkboxes.html.erb @@ -0,0 +1,8 @@ +<%= render layout: layout do |f| %> + <%= f.govuk_collection_check_boxes :response, + checkbox_options(array_of_options: @step.options), + :id, + :name, + legend: { text: @step.title, size: "xl" } + %> +<% end %> diff --git a/app/views/steps/long_text.html.erb b/app/views/steps/long_text.html.erb new file mode 100644 index 000000000..5933f8d45 --- /dev/null +++ b/app/views/steps/long_text.html.erb @@ -0,0 +1,8 @@ +<%= render layout: layout do |f| %> + <%= f.govuk_text_area :response, + label: { text: @step.title, size: 'xl' }, + hint: { text: @step.help_text }, + width: "one-third", + rows: 9 + %> +<% end %> diff --git a/app/views/steps/new.long_text.html.erb b/app/views/steps/new.long_text.html.erb deleted file mode 100644 index dfa991e31..000000000 --- a/app/views/steps/new.long_text.html.erb +++ /dev/null @@ -1,11 +0,0 @@ -<%= content_for :title, @step.title %> - -<%= form_for @answer, as: :answer, url: journey_step_answers_path(@journey, @step), method: "post" do |f| %> - <%= f.govuk_text_area :response, - label: { text: @step.title, size: 'xl' }, - hint: { text: @step.help_text }, - width: "one-third", - rows: 9 - %> - <%= f.submit I18n.t("generic.button.soft_finish"), class: "govuk-button" %> -<% end %> diff --git a/app/views/steps/new.short_text.html.erb b/app/views/steps/new.short_text.html.erb deleted file mode 100644 index 4ee893f35..000000000 --- a/app/views/steps/new.short_text.html.erb +++ /dev/null @@ -1,10 +0,0 @@ -<%= content_for :title, @step.title %> - -<%= form_for @answer, as: :answer, url: journey_step_answers_path(@journey, @step), method: "post" do |f| %> - <%= f.govuk_text_field :response, - label: { text: @step.title, size: 'xl' }, - hint: { text: @step.help_text }, - width: "one-third" - %> - <%= f.submit I18n.t("generic.button.soft_finish"), class: "govuk-button" %> -<% end %> diff --git a/app/views/steps/new.paragraphs.html.erb b/app/views/steps/paragraphs.html.erb similarity index 68% rename from app/views/steps/new.paragraphs.html.erb rename to app/views/steps/paragraphs.html.erb index 7306d4b5e..67a00db56 100644 --- a/app/views/steps/new.paragraphs.html.erb +++ b/app/views/steps/paragraphs.html.erb @@ -4,4 +4,4 @@
<%= simple_format(@step.body, wrapper_tag: "p", class: "govuk-body") %>
-<%= link_to I18n.t("generic.button.next"), new_journey_step_path, class: "govuk-button" %> +<%= link_to @step.primary_call_to_action_text, new_journey_step_path, class: "govuk-button" %> diff --git a/app/views/steps/new.radios.html.erb b/app/views/steps/radios.html.erb similarity index 51% rename from app/views/steps/new.radios.html.erb rename to app/views/steps/radios.html.erb index cb5558178..d0dcfb0ce 100644 --- a/app/views/steps/new.radios.html.erb +++ b/app/views/steps/radios.html.erb @@ -1,9 +1,7 @@ -<%= content_for :title, @step.title %> - -<%= form_for @answer, as: :answer, url: journey_step_answers_path(@journey, @step), method: "post" do |f| %> +<%= render layout: layout do |f| %> <%= f.hidden_field :response, value: nil %> <%= f.govuk_collection_radio_buttons :response, - @step.radio_options, + radio_options(array_of_options: @step.options), :id, ->(option) { option.name }, :description, @@ -11,5 +9,4 @@ hint: { text: @step.help_text }, inline: false %> - <%= f.submit I18n.t("generic.button.soft_finish"), class: "govuk-button" %> <% end %> diff --git a/app/views/steps/short_text.html.erb b/app/views/steps/short_text.html.erb new file mode 100644 index 000000000..3d414cc28 --- /dev/null +++ b/app/views/steps/short_text.html.erb @@ -0,0 +1,7 @@ +<%= render layout: layout do |f| %> + <%= f.govuk_text_field :response, + label: { text: @step.title, size: 'xl' }, + hint: { text: @step.help_text }, + width: "one-third" + %> +<% end %> diff --git a/app/views/steps/single_date.html.erb b/app/views/steps/single_date.html.erb new file mode 100644 index 000000000..0cc149ca1 --- /dev/null +++ b/app/views/steps/single_date.html.erb @@ -0,0 +1,6 @@ +<%= render layout: layout do |f| %> + <%= f.govuk_date_field :response, + legend: { text: @step.title, size: "xl" }, + hint: { text: @step.help_text } + %> +<% end %> diff --git a/config/application.rb b/config/application.rb index 4ba252194..0b9010946 100644 --- a/config/application.rb +++ b/config/application.rb @@ -32,5 +32,13 @@ class Application < Rails::Application # Application configuration can go into files in config/initializers # -- all .rb files in that directory are automatically loaded after loading # the framework and any gems in your application. + + # Automatically add UUID as the type of primary key on new tables, if you + # use the Rails migration generator with 'create' + config.generators do |g| + g.orm :active_record, primary_key_type: :uuid + end + + config.active_job.queue_adapter = :sidekiq end end diff --git a/config/environments/test.rb b/config/environments/test.rb index cc7c118b5..e5e40667f 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -61,5 +61,7 @@ Bullet.raise = true # raise an error if n+1 query occurs Bullet.add_whitelist type: :unused_eager_loading, class_name: "Step", association: :radio_answer Bullet.add_whitelist type: :unused_eager_loading, class_name: "Step", association: :short_text_answer + Bullet.add_whitelist type: :unused_eager_loading, class_name: "Step", association: :long_text_answer + Bullet.add_whitelist type: :unused_eager_loading, class_name: "Step", association: :single_date_answer end end diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb new file mode 100644 index 000000000..e80787b84 --- /dev/null +++ b/config/initializers/sidekiq.rb @@ -0,0 +1,13 @@ +Sidekiq.configure_server do |config| + config.redis = {url: ENV["REDIS_SIDEKIQ_URL"]} + + # Sidekiq Cron + schedule_file = "config/schedule.yml" + if File.exist?(schedule_file) + Sidekiq::Cron::Job.load_from_hash YAML.load_file(schedule_file) + end +end + +Sidekiq.configure_client do |config| + config.redis = {url: ENV["REDIS_SIDEKIQ_URL"]} +end diff --git a/config/locales/en.yml b/config/locales/en.yml index f607c4ca4..64910dae2 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1,11 +1,15 @@ en: app: name: Buy for your school + date: + formats: + default: "%-d %b %Y" generic: button: start: "Continue" next: "Continue" - soft_finish: "Save and continue later" + change_answer: "Change" + update: "Update" banner: beta: tag: "beta" @@ -32,6 +36,8 @@ en: before_you_start_body: "It will take around 3 minutes to complete the process" errors: contentful_entry_not_found: "An unexpected error occurred. The starting step has been revoked by the content team." + journey_map: + page_title: "Contentful entry map" errors: contentful_entry_not_found: page_title: "An unexpected error occurred" @@ -42,3 +48,9 @@ en: unexpected_contentful_step_type: page_title: "An unexpected error occurred" page_body: "The service has had a problem trying to retrieve the required step. The team have been notified of this problem and you should be able to retry shortly." + repeat_step_in_the_contentful_journey: + page_title: "An unexpected error occurred" + page_body: "One or more steps in the Contentful journey would leave the user in an infinite loop. This entry ID was presented more than once to the user: %{entry_id}" + too_many_steps_in_the_contentful_journey: + page_title: "An unexpected error occurred" + page_body: "More than %{step_count} steps were found in the Contentful journey. Is the journey missing an end? The last Entry ID was: %{entry_id}" diff --git a/config/routes.rb b/config/routes.rb index 6e3bf2aa6..af256d000 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -4,9 +4,10 @@ get "health_check" => "application#health_check" root to: "high_voltage/pages#show", id: "planning_start_page" + resource :journey_map, only: [:new] resources :journeys, only: [:new, :show] do - resources :steps, only: [:new, :show] do - resources :answers, only: [:create] + resources :steps, only: [:new, :show, :edit] do + resources :answers, only: [:create, :update] end end diff --git a/config/schedule.yml b/config/schedule.yml new file mode 100644 index 000000000..544798e82 --- /dev/null +++ b/config/schedule.yml @@ -0,0 +1,8 @@ +warm_contentful_entry_cache: + cron: "0 3 * * *" + class: "WarmEntryCacheJob" + description: This is a content driven service and the content is sourced by + the Contentful API. As a single point of failure we have a Redis cache to + provide some resilience to downtime or minor connection issues. As this + service is used infrequently by real users, we automatically update the + cache. diff --git a/config/sidekiq.yml b/config/sidekiq.yml new file mode 100644 index 000000000..e7bf86115 --- /dev/null +++ b/config/sidekiq.yml @@ -0,0 +1,6 @@ +--- +:concurrency: 5 +:queues: + - default + - [high_priority, 2] + - [caching, 1] diff --git a/db/migrate/20201203103421_change_long_text_answer_to_uuid.rb b/db/migrate/20201203103421_change_long_text_answer_to_uuid.rb new file mode 100644 index 000000000..b4b14f905 --- /dev/null +++ b/db/migrate/20201203103421_change_long_text_answer_to_uuid.rb @@ -0,0 +1,17 @@ +class ChangeLongTextAnswerToUuid < ActiveRecord::Migration[6.0] + def up + enable_extension "uuid-ossp" unless extension_enabled?("uuid-ossp") + + add_column :long_text_answers, :uuid, :uuid, default: "gen_random_uuid()", null: false + + change_table :long_text_answers do |t| + t.remove :id + t.rename :uuid, :id + end + execute "ALTER TABLE long_text_answers ADD PRIMARY KEY (id);" + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/migrate/20201207171647_add_primary_call_to_action_text_to_step.rb b/db/migrate/20201207171647_add_primary_call_to_action_text_to_step.rb new file mode 100644 index 000000000..19a710411 --- /dev/null +++ b/db/migrate/20201207171647_add_primary_call_to_action_text_to_step.rb @@ -0,0 +1,5 @@ +class AddPrimaryCallToActionTextToStep < ActiveRecord::Migration[6.0] + def change + add_column :steps, :primary_call_to_action_text, :string + end +end diff --git a/db/migrate/20201208110342_create_single_date_answer.rb b/db/migrate/20201208110342_create_single_date_answer.rb new file mode 100644 index 000000000..c84bbebd0 --- /dev/null +++ b/db/migrate/20201208110342_create_single_date_answer.rb @@ -0,0 +1,9 @@ +class CreateSingleDateAnswer < ActiveRecord::Migration[6.0] + def change + create_table :single_date_answers, id: :uuid do |t| + t.references :step, type: :uuid + t.date :response, null: false + t.timestamps + end + end +end diff --git a/db/migrate/20201209122555_create_checkbox_answers.rb b/db/migrate/20201209122555_create_checkbox_answers.rb new file mode 100644 index 000000000..5424846e6 --- /dev/null +++ b/db/migrate/20201209122555_create_checkbox_answers.rb @@ -0,0 +1,9 @@ +class CreateCheckboxAnswers < ActiveRecord::Migration[6.0] + def change + create_table :checkbox_answers, id: :uuid do |t| + t.references :step, type: :uuid + t.string :response, array: true, default: [] + t.timestamps + end + end +end diff --git a/db/migrate/20201216150806_add_contentful_id_to_step.rb b/db/migrate/20201216150806_add_contentful_id_to_step.rb new file mode 100644 index 000000000..0f3e8eba0 --- /dev/null +++ b/db/migrate/20201216150806_add_contentful_id_to_step.rb @@ -0,0 +1,20 @@ +class AddContentfulIdToStep < ActiveRecord::Migration[6.1] + def up + add_column :steps, :contentful_id, :string + + ActiveRecord::Base.transaction do + Step.all.map do |step| + contentful_id = JSON.parse( + step.raw.gsub("=>", ":") + ).dig("sys", "id") + step.update(contentful_id: contentful_id) + end + end + + change_column :steps, :contentful_id, :string, null: false + end + + def down + remove_column :steps, :contentful_id, :string + end +end diff --git a/db/migrate/20201216160028_change_step_raw_field_to_json_b.rb b/db/migrate/20201216160028_change_step_raw_field_to_json_b.rb new file mode 100644 index 000000000..3b0fb4708 --- /dev/null +++ b/db/migrate/20201216160028_change_step_raw_field_to_json_b.rb @@ -0,0 +1,27 @@ +class ChangeStepRawFieldToJsonB < ActiveRecord::Migration[6.1] + def up + add_column :steps, :raw_jsonb, :jsonb + ActiveRecord::Base.transaction do + Step.all.map do |step| + hash = JSON.parse(step.raw.gsub("=>", ":")) + step.update(raw_jsonb: hash) + end + end + remove_column :steps, :raw + rename_column :steps, :raw_jsonb, :raw + change_column :steps, :raw, :jsonb, null: false + end + + def down + add_column :steps, :raw_binary, :binary + ActiveRecord::Base.transaction do + Step.all.map do |step| + string = step.raw.to_s + step.update(raw_binary: string) + end + end + remove_column :steps, :raw + rename_column :steps, :raw_binary, :raw + change_column :steps, :raw, :binary, null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index f43e73088..e24feb3ef 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -2,19 +2,28 @@ # of editing this file, please use the migrations feature of Active Record to # incrementally modify your database, and then regenerate this schema definition. # -# This file is the source Rails uses to define your schema when running `rails -# db:schema:load`. When creating a new database, `rails db:schema:load` tends to +# This file is the source Rails uses to define your schema when running `bin/rails +# db:schema:load`. When creating a new database, `bin/rails db:schema:load` tends to # be faster and is potentially less error prone than running all of your # migrations from scratch. Old migrations may fail to apply correctly if those # migrations use external dependencies or application code. # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_12_02_150359) do +ActiveRecord::Schema.define(version: 2020_12_16_160028) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" + enable_extension "uuid-ossp" + + create_table "checkbox_answers", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| + t.uuid "step_id" + t.string "response", default: [], array: true + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + t.index ["step_id"], name: "index_checkbox_answers_on_step_id" + end create_table "journeys", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.string "category", null: false @@ -23,7 +32,7 @@ t.string "next_entry_id" end - create_table "long_text_answers", force: :cascade do |t| + create_table "long_text_answers", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.uuid "step_id" t.text "response", null: false t.datetime "created_at", precision: 6, null: false @@ -47,17 +56,27 @@ t.index ["step_id"], name: "index_short_text_answers_on_step_id" end + create_table "single_date_answers", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| + t.uuid "step_id" + t.date "response", null: false + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + t.index ["step_id"], name: "index_single_date_answers_on_step_id" + end + create_table "steps", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.uuid "journey_id" t.string "title", null: false t.string "help_text" t.string "contentful_type", null: false t.string "options", array: true - t.binary "raw", null: false t.datetime "created_at", precision: 6, null: false t.datetime "updated_at", precision: 6, null: false t.text "body" t.string "contentful_model" + t.string "primary_call_to_action_text" + t.string "contentful_id", null: false + t.jsonb "raw", null: false t.index ["journey_id"], name: "index_steps_on_journey_id" end diff --git a/doc/architecture/decisions/0008-use-sidekiq-for-asynchronous-tasks.md b/doc/architecture/decisions/0008-use-sidekiq-for-asynchronous-tasks.md new file mode 100644 index 000000000..26d412b5b --- /dev/null +++ b/doc/architecture/decisions/0008-use-sidekiq-for-asynchronous-tasks.md @@ -0,0 +1,28 @@ +# 8. use sidekiq for asynchronous tasks + +Date: 2020-11-30 + +## Status + +Accepted + +## Context + +We need a way for the service to automatically and regularly run a task. + +We already have Redis available as our caching layer and Sidekiq works great with it. + +An alternative could be to use Cron for scheduled tasks and a postgres backed asynchronous jobs, perhaps even run inline. We know how to get Sidekiq running with Docker and reusing Redis (rather than Postgres) for job data that is ephemeral feels a better fit given we already have Redis. + +## Decision + +Use Sidekiq for processing asynchronous tasks. + +## Consequences + +- We add another process/box of complexity to the system architecture though we would need to do this with Cron too +- It should take less time to add another asynchronous task in future +- It should take less time to add another scheduled task +- A clear separation of concerns between persistent data being in Postgres and ephemeral data in Redis +- Sidekiq can manage the scheduled task queue on initialiser which automatically takes care of provisioning jobs on all environments +- Adding Sidekiq takes more time now and will require another widget to Terraform when moving to GPaaS. We have done this before. diff --git a/doc/release-process.md b/doc/release-process.md index 6ade9bdff..adae30ae1 100644 --- a/doc/release-process.md +++ b/doc/release-process.md @@ -52,4 +52,4 @@ Let everybody know that a new release has been shipped. The usual place to do this is #sct-buy-for-your-school, with a message like: -> 🚢 Release # for Buy for Your School is now live. Changes in this release: [link to changelog] 🚀 +> 🚢 Release ### for Buy for your school is now live. (Changes in this release)[https://github.com/DFE-Digital/buy-for-your-school/blob/develop/CHANGELOG.md#release-###---YYYY-MM-DD] - (Production environment)[https://dfe-buy-for-your-school-beta.herokuapp.com/] 🚀 diff --git a/docker-compose.yml b/docker-compose.yml index da51c0054..a68d0031a 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -9,6 +9,7 @@ services: command: bundle exec rails s -p 3000 -b '0.0.0.0' ports: - "3000:3000" + image: "buy_for_your_school:dev" depends_on: - db - redis @@ -17,6 +18,7 @@ services: environment: DATABASE_URL: postgres://postgres:password@db:5432/buy-for-your-school-development REDIS_CACHE_URL: redis://redis:6379/1 + REDIS_SIDEKIQ_URL: redis://redis:6379/2 volumes: - .:/srv/app tty: true @@ -40,6 +42,17 @@ services: networks: - dev + sidekiq: + image: buy_for_your_school:dev + command: bundle exec sidekiq -C config/sidekiq.yml + depends_on: + - db + - redis + volumes: + - .:/srv/app + networks: + - dev + networks: dev: volumes: diff --git a/heroku.yml b/heroku.yml index a9aa82ec7..17b96f089 100644 --- a/heroku.yml +++ b/heroku.yml @@ -7,3 +7,7 @@ release: - rake db:migrate run: web: bundle exec puma -C config/puma.rb + worker: + command: + - bundle exec sidekiq -C config/sidekiq.yml + image: web diff --git a/spec/factories/answer.rb b/spec/factories/answer.rb index fd1a42adf..6b89a1498 100644 --- a/spec/factories/answer.rb +++ b/spec/factories/answer.rb @@ -1,19 +1,31 @@ FactoryBot.define do factory :radio_answer do - association :step, factory: :step, contentful_type: "radios" + association :step, factory: :step, contentful_type: "radios", contentful_model: "question" response { "Green" } end factory :short_text_answer do - association :step, factory: :step, contentful_type: "short_text" + association :step, factory: :step, contentful_type: "short_text", contentful_model: "question" response { "Green" } end factory :long_text_answer do - association :step, factory: :step, contentful_type: "long_text" + association :step, factory: :step, contentful_type: "long_text", contentful_model: "question" response { "Lots of green" } end + + factory :single_date_answer do + association :step, factory: :step, contentful_type: "single_date" + + response { 1.year.from_now } + end + + factory :checkbox_answers do + association :step, factory: :step, contentful_type: "checkboxes" + + response { ["breakfast", "lunch", ""] } + end end diff --git a/spec/factories/step.rb b/spec/factories/step.rb index 40b9cfd3c..e1ed7a3b0 100644 --- a/spec/factories/step.rb +++ b/spec/factories/step.rb @@ -2,7 +2,8 @@ factory :step do title { "What is your favourite colour?" } help_text { "Choose the primary colour closest to your choice" } - raw { "{\"sys\":{}}" } + raw { "{\"sys\":{\"id\"=>\"123\"}}" } + contentful_id { "123" } association :journey, factory: :journey @@ -27,6 +28,20 @@ association :long_text_answer end + trait :single_date do + options { nil } + contentful_model { "question" } + contentful_type { "single_date" } + association :single_date_answer + end + + trait :checkbox_answers do + options { ["Brown", "Gold"] } + contentful_model { "question" } + contentful_type { "checkboxes" } + association :checkbox_answers + end + trait :static_content do options { nil } contentful_model { "staticContent" } diff --git a/spec/features/visitors/anyone_can_complete_a_journey_spec.rb b/spec/features/visitors/anyone_can_complete_a_journey_spec.rb index 34b905b95..c27a39bd1 100644 --- a/spec/features/visitors/anyone_can_complete_a_journey_spec.rb +++ b/spec/features/visitors/anyone_can_complete_a_journey_spec.rb @@ -1,6 +1,14 @@ require "rails_helper" feature "Anyone can start a journey" do + around do |example| + ClimateControl.modify( + CONTENTFUL_PLANNING_START_ENTRY_ID: "1UjQurSOi5MWkcRuGxdXZS" + ) do + example.run + end + end + scenario "Start page includes a call to action" do stub_get_contentful_entry @@ -15,7 +23,7 @@ choose("Catering") - click_on(I18n.t("generic.button.soft_finish")) + click_on(I18n.t("generic.button.next")) end scenario "an answer must be provided" do @@ -27,7 +35,7 @@ # Omit a choice - click_on(I18n.t("generic.button.soft_finish")) + click_on(I18n.t("generic.button.next")) expect(page).to have_content("can't be blank") end @@ -56,118 +64,163 @@ entry_id: "5lYcZs1ootDrOnk09LDLZg", fixture_filename: "no-next-question-example.json" ) - click_on(I18n.t("generic.button.soft_finish")) + + click_on(I18n.t("generic.button.next")) choose("Stationary") - click_on(I18n.t("generic.button.soft_finish")) + click_on(I18n.t("generic.button.next")) expect(page).to have_content("Catering") expect(page).to have_content("Stationary") end end - scenario "a Contentful entry_id does not exist" do - contentful_client = stub_contentful_client - - allow(contentful_client).to receive(:entry) - .with(anything) - .and_raise(GetContentfulEntry::EntryNotFound.new("The following Contentful error could not be found: sss ")) + context "when the Contentful model is of type question" do + context "when Contentful entry is of type short_text" do + around do |example| + ClimateControl.modify( + CONTENTFUL_PLANNING_START_ENTRY_ID: "hfjJgWRg4xiiiImwVRDtZ" + ) do + example.run + end + end - visit root_path + scenario "user can answer using free text" do + stub_get_contentful_entry( + entry_id: "hfjJgWRg4xiiiImwVRDtZ", + fixture_filename: "short-text-question-example.json" + ) - click_on(I18n.t("generic.button.start")) + visit root_path + click_on(I18n.t("generic.button.start")) - expect(page).to have_content(I18n.t("errors.contentful_entry_not_found.page_title")) - expect(page).to have_content(I18n.t("errors.contentful_entry_not_found.page_body")) - end + fill_in "answer[response]", with: "email@example.com" + click_on(I18n.t("generic.button.next")) - context "when Contentful entry is of type short_text" do - around do |example| - ClimateControl.modify( - CONTENTFUL_PLANNING_START_ENTRY_ID: "hfjJgWRg4xiiiImwVRDtZ" - ) do - example.run + expect(page).to have_content("email@example") end end - scenario "user can answer using free text" do - stub_get_contentful_entry( - entry_id: "hfjJgWRg4xiiiImwVRDtZ", - fixture_filename: "short-text-question-example.json" - ) + context "when Contentful entry is of type long_text" do + around do |example| + ClimateControl.modify( + CONTENTFUL_PLANNING_START_ENTRY_ID: "2jWIO1MrVIya9NZrFWT4e" + ) do + example.run + end + end - visit root_path - click_on(I18n.t("generic.button.start")) + scenario "user can answer using free text with multiple lines" do + stub_get_contentful_entry( + entry_id: "2jWIO1MrVIya9NZrFWT4e", + fixture_filename: "long-text-question-example.json" + ) - fill_in "answer[response]", with: "email@example.com" - click_on(I18n.t("generic.button.soft_finish")) + visit root_path + click_on(I18n.t("generic.button.start")) - expect(page).to have_content("email@example") + fill_in "answer[response]", with: "We would like a supplier to provide catering from September 2020.\r\nThey must be able to supply us for 3 years minumum." + click_on(I18n.t("generic.button.next")) + + within(".govuk-summary-list") do + paragraphs_elements = find_all("p") + expect(paragraphs_elements.first.text).to have_content("We would like a supplier to provide catering from September 2020.") + expect(paragraphs_elements.last.text).to have_content("They must be able to supply us for 3 years minumum.") + end + end end - end - context "when Contentful entry is of type long_text" do - around do |example| - ClimateControl.modify( - CONTENTFUL_PLANNING_START_ENTRY_ID: "2jWIO1MrVIya9NZrFWT4e" - ) do - example.run + context "when Contentful entry is of type single_date" do + around do |example| + ClimateControl.modify( + CONTENTFUL_PLANNING_START_ENTRY_ID: "55G5kpCLLL3h5yBQLiVlYy" + ) do + example.run + end + end + + scenario "user can answer using a date input" do + stub_get_contentful_entry( + entry_id: "55G5kpCLLL3h5yBQLiVlYy", + fixture_filename: "single-date-example.json" + ) + + visit root_path + click_on(I18n.t("generic.button.start")) + + fill_in "answer[response(3i)]", with: "12" + fill_in "answer[response(2i)]", with: "8" + fill_in "answer[response(1i)]", with: "2020" + + click_on(I18n.t("generic.button.next")) + + expect(page).to have_content("12 Aug 2020") end end - scenario "user can answer using free text with multiple lines" do - stub_get_contentful_entry( - entry_id: "2jWIO1MrVIya9NZrFWT4e", - fixture_filename: "long-text-question-example.json" - ) + context "when Contentful entry is of type checkboxes" do + around do |example| + ClimateControl.modify( + CONTENTFUL_PLANNING_START_ENTRY_ID: "1DqhwF2XkJJ0Um6NSweWlZ" + ) do + example.run + end + end - visit root_path - click_on(I18n.t("generic.button.start")) + scenario "user can select multiple answers" do + stub_get_contentful_entry( + entry_id: "1DqhwF2XkJJ0Um6NSweWlZ", + fixture_filename: "checkbox-example.json" + ) + + visit root_path + click_on(I18n.t("generic.button.start")) - fill_in "answer[response]", with: "We would like a supplier to provide catering from September 2020.\r\nThey must be able to supply us for 3 years minumum." - click_on(I18n.t("generic.button.soft_finish")) + check "Breakfast" + check "Lunch" - within(".govuk-summary-list") do - paragraphs_elements = find_all("p") - expect(paragraphs_elements.first.text).to have_content("We would like a supplier to provide catering from September 2020.") - expect(paragraphs_elements.last.text).to have_content("They must be able to supply us for 3 years minumum.") + click_on(I18n.t("generic.button.next")) + + expect(page).to have_content("Breakfast, Lunch") end end end - context "when Contentful entry is of type static_content" do - around do |example| - ClimateControl.modify( - CONTENTFUL_PLANNING_START_ENTRY_ID: "5kZ9hIFDvNCEhjWs72SFwj" - ) do - example.run + context "when the Contentful model is of type staticContent" do + context "when Contentful entry is of type paragraphs" do + around do |example| + ClimateControl.modify( + CONTENTFUL_PLANNING_START_ENTRY_ID: "5kZ9hIFDvNCEhjWs72SFwj" + ) do + example.run + end end - end - scenario "user can read static content and proceed without answering" do - stub_get_contentful_entry( - entry_id: "5kZ9hIFDvNCEhjWs72SFwj", - fixture_filename: "static-content-example.json" - ) + scenario "user can read static content and proceed without answering" do + stub_get_contentful_entry( + entry_id: "5kZ9hIFDvNCEhjWs72SFwj", + fixture_filename: "static-content-example.json" + ) - visit root_path - click_on(I18n.t("generic.button.start")) + visit root_path + click_on(I18n.t("generic.button.start")) - expect(page).to have_content("When you should start") + expect(page).to have_content("When you should start") - within(".static-content") do - paragraphs_elements = find_all("p") - expect(paragraphs_elements.first.text).to have_content("Procuring a new catering contract can take up to 6 months to consult, create, review and award.") - expect(paragraphs_elements.last.text).to have_content("Usually existing contracts start and end in the month of September. We recommend starting this process around March.") - end + within(".static-content") do + paragraphs_elements = find_all("p") + expect(paragraphs_elements.first.text).to have_content("Procuring a new catering contract can take up to 6 months to consult, create, review and award.") + expect(paragraphs_elements.last.text).to have_content("Usually existing contracts start and end in the month of September. We recommend starting this process around March.") + end - click_on(I18n.t("generic.button.next")) + click_on(I18n.t("generic.button.next")) - expect(page).to have_content("Catering") + expect(page).to have_content("Catering") + end end end - context "when Contentful entry model wasn't a type of question" do + context "when Contentful entry model wasn't an expected type" do around do |example| ClimateControl.modify( CONTENTFUL_PLANNING_START_ENTRY_ID: "6EKsv389ETYcQql3htK3Z2" @@ -191,7 +244,7 @@ end end - context "when Contentful step entry wasn't an expected type" do + context "when the Contentful Entry wasn't an expected step type" do around do |example| ClimateControl.modify( CONTENTFUL_PLANNING_START_ENTRY_ID: "8as7df68uhasdnuasdf" @@ -214,4 +267,19 @@ expect(page).to have_content(I18n.t("errors.unexpected_contentful_step_type.page_body")) end end + + scenario "a Contentful entry_id does not exist" do + contentful_client = stub_contentful_client + + allow(contentful_client).to receive(:entry) + .with(anything) + .and_raise(GetContentfulEntry::EntryNotFound.new("The following Contentful error could not be found: sss ")) + + visit root_path + + click_on(I18n.t("generic.button.start")) + + expect(page).to have_content(I18n.t("errors.contentful_entry_not_found.page_title")) + expect(page).to have_content(I18n.t("errors.contentful_entry_not_found.page_body")) + end end diff --git a/spec/features/visitors/anyone_can_edit_their_answers_spec.rb b/spec/features/visitors/anyone_can_edit_their_answers_spec.rb new file mode 100644 index 000000000..ee64ff27d --- /dev/null +++ b/spec/features/visitors/anyone_can_edit_their_answers_spec.rb @@ -0,0 +1,37 @@ +require "rails_helper" + +feature "Users can edit their answers" do + let(:answer) { create(:short_text_answer, response: "answer") } + scenario "The journey summary page displays a change link" do + visit journey_path(answer.step.journey) + + expect(page).to have_content("answer") + expect(page).to have_content("Change") + end + + scenario "The edited answer is saved" do + visit journey_path(answer.step.journey) + + click_on(I18n.t("generic.button.change_answer")) + + fill_in "answer[response]", with: "email@example.com" + + click_on(I18n.t("generic.button.update")) + + expect(page).to have_content("email@example.com") + end + + context "An error is thrown" do + scenario "When an answer is invalid" do + visit journey_path(answer.step.journey) + + click_on(I18n.t("generic.button.change_answer")) + + fill_in "answer[response]", with: "" + + click_on(I18n.t("generic.button.update")) + + expect(page).to have_content("can't be blank") + end + end +end diff --git a/spec/features/visitors/anyone_can_see_the_map_of_journey_steps_page_spec.rb b/spec/features/visitors/anyone_can_see_the_map_of_journey_steps_page_spec.rb new file mode 100644 index 000000000..44ea047b6 --- /dev/null +++ b/spec/features/visitors/anyone_can_see_the_map_of_journey_steps_page_spec.rb @@ -0,0 +1,81 @@ +feature "Users can see all the steps of a journey" do + around do |example| + ClimateControl.modify( + CONTENTFUL_PLANNING_START_ENTRY_ID: "5kZ9hIFDvNCEhjWs72SFwj" + ) do + example.run + end + end + + scenario "Multiple journey steps" do + stub_get_contentful_entries( + entry_id: "5kZ9hIFDvNCEhjWs72SFwj", + fixture_filename: "closed-path-with-multiple-example.json" + ) + + visit new_journey_map_path + + expect(page).to have_content(I18n.t("journey_map.page_title")) + + within(".govuk-list") do + list_items = find_all("li") + within(list_items.first) do + expect(page).to have_link("When you should start", href: "https://app.contentful.com/spaces/#{ENV["CONTENTFUL_SPACE"]}/environments/#{ENV["CONTENTFUL_ENVIRONMENT"]}/entries/5kZ9hIFDvNCEhjWs72SFwj") + end + within(list_items.last) do + expect(page).to have_link("Which service do you need?", href: "https://app.contentful.com/spaces/#{ENV["CONTENTFUL_SPACE"]}/environments/#{ENV["CONTENTFUL_ENVIRONMENT"]}/entries/hfjJgWRg4xiiiImwVRDtZ") + end + end + end + + context "when the map isn't valid" do + context "when the same entry is found twice" do + around do |example| + ClimateControl.modify( + CONTENTFUL_PLANNING_START_ENTRY_ID: "5kZ9hIFDvNCEhjWs72SFwj" + ) do + example.run + end + end + + it "returns an error message" do + stub_get_contentful_entries( + entry_id: "5kZ9hIFDvNCEhjWs72SFwj", + fixture_filename: "repeat-entry-example.json" + ) + + visit new_journey_map_path + + expect(page).to have_content(I18n.t("errors.repeat_step_in_the_contentful_journey.page_title")) + expect(page).to have_content( + I18n.t("errors.repeat_step_in_the_contentful_journey.page_body", entry_id: "5kZ9hIFDvNCEhjWs72SFwj") + ) + end + end + + context "when the chain becomes obviously too long" do + around do |example| + ClimateControl.modify( + CONTENTFUL_PLANNING_START_ENTRY_ID: "5kZ9hIFDvNCEhjWs72SFwj" + ) do + example.run + end + end + + it "returns an error message" do + stub_const("BuildJourneyOrder::ENTRY_JOURNEY_MAX_LENGTH", 1) + stub_get_contentful_entries( + entry_id: "hfjJgWRg4xiiiImwVRDtZ", + fixture_filename: "closed-path-with-multiple-example.json" + ) + + visit new_journey_map_path + + expect(page).to have_content(I18n.t("errors.too_many_steps_in_the_contentful_journey.page_title")) + expect(page).to have_content( + I18n.t("errors.too_many_steps_in_the_contentful_journey.page_body", entry_id: "hfjJgWRg4xiiiImwVRDtZ", step_count: 1) + ) + end + end + end +end diff --git a/spec/fixtures/contentful/checkbox-example.json b/spec/fixtures/contentful/checkbox-example.json new file mode 100644 index 000000000..d606d24f7 --- /dev/null +++ b/spec/fixtures/contentful/checkbox-example.json @@ -0,0 +1,41 @@ +{ + "sys": { + "space": { + "sys": { + "type": "Link", + "linkType": "Space", + "id": "rwl7tyzv9sys" + } + }, + "id": "1DqhwF2XkJJ0Um6NSweWlZ", + "type": "Entry", + "createdAt": "2020-12-09T11:53:52.744Z", + "updatedAt": "2020-12-09T11:53:52.744Z", + "environment": { + "sys": { + "id": "develop", + "type": "Link", + "linkType": "Environment" + } + }, + "revision": 1, + "contentType": { + "sys": { + "type": "Link", + "linkType": "ContentType", + "id": "question" + } + }, + "locale": "en-US" + }, + "fields": { + "slug": "/everyday-services", + "type": "checkboxes", + "title": "Everyday services that are required and need to be considered", + "options": [ + "breakfast", + "lunch", + "dinner" + ] + } +} diff --git a/spec/fixtures/contentful/closed-path-with-multiple-example.json b/spec/fixtures/contentful/closed-path-with-multiple-example.json new file mode 100644 index 000000000..23b0b4e99 --- /dev/null +++ b/spec/fixtures/contentful/closed-path-with-multiple-example.json @@ -0,0 +1,95 @@ +{ + "sys": { + "type": "Array" + }, + "total": 2, + "skip": 0, + "limit": 100, + "items": [ + { + "sys": { + "space": { + "sys": { + "type": "Link", + "linkType": "Space", + "id": "rwl7tyzv9sys" + } + }, + "id": "5kZ9hIFDvNCEhjWs72SFwj", + "type": "Entry", + "createdAt": "2020-12-02T10:48:35.748Z", + "updatedAt": "2020-12-02T10:48:35.748Z", + "environment": { + "sys": { + "id": "develop", + "type": "Link", + "linkType": "Environment" + } + }, + "revision": 1, + "contentType": { + "sys": { + "type": "Link", + "linkType": "ContentType", + "id": "staticContent" + } + }, + "locale": "en-US" + }, + "fields": { + "slug": "/timelines", + "title": "When you should start", + "body": "Procuring a new catering contract can take up to 6 months to consult, create, review and award. \n\nUsually existing contracts start and end in the month of September. We recommend starting this process around March.", + "type": "paragraphs", + "next": { + "sys": { + "type": "Link", + "linkType": "Entry", + "id": "hfjJgWRg4xiiiImwVRDtZ" + } + } + } + }, + { + "sys": { + "space": { + "sys": { + "type": "Link", + "linkType": "Space", + "id": "rwl7tyzv9sys" + } + }, + "id": "hfjJgWRg4xiiiImwVRDtZ", + "type": "Entry", + "createdAt": "2020-11-04T12:28:30.442Z", + "updatedAt": "2020-11-26T16:39:54.188Z", + "environment": { + "sys": { + "id": "develop", + "type": "Link", + "linkType": "Environment" + } + }, + "revision": 6, + "contentType": { + "sys": { + "type": "Link", + "linkType": "ContentType", + "id": "question" + } + }, + "locale": "en-US" + }, + "fields": { + "slug": "/dev-start-which-service", + "title": "Which service do you need?", + "helpText": "Tell us which service you need.", + "type": "radios", + "options": [ + "Catering", + "Cleaning" + ] + } + } + ] +} diff --git a/spec/fixtures/contentful/multiple-entries-example.json b/spec/fixtures/contentful/multiple-entries-example.json new file mode 100644 index 000000000..ec1d49501 --- /dev/null +++ b/spec/fixtures/contentful/multiple-entries-example.json @@ -0,0 +1,102 @@ +{ + "sys": { + "type": "Array" + }, + "total": 2, + "skip": 0, + "limit": 100, + "items": [ + { + "sys": { + "space": { + "sys": { + "type": "Link", + "linkType": "Space", + "id": "rwl7tyzv9sys" + } + }, + "id": "5kZ9hIFDvNCEhjWs72SFwj", + "type": "Entry", + "createdAt": "2020-12-02T10:48:35.748Z", + "updatedAt": "2020-12-02T10:48:35.748Z", + "environment": { + "sys": { + "id": "develop", + "type": "Link", + "linkType": "Environment" + } + }, + "revision": 1, + "contentType": { + "sys": { + "type": "Link", + "linkType": "ContentType", + "id": "staticContent" + } + }, + "locale": "en-US" + }, + "fields": { + "slug": "/timelines", + "title": "When you should start", + "body": "Procuring a new catering contract can take up to 6 months to consult, create, review and award. \n\nUsually existing contracts start and end in the month of September. We recommend starting this process around March.", + "type": "paragraphs", + "next": { + "sys": { + "type": "Link", + "linkType": "Entry", + "id": "hfjJgWRg4xiiiImwVRDtZ" + } + } + } + }, + { + "sys": { + "space": { + "sys": { + "type": "Link", + "linkType": "Space", + "id": "rwl7tyzv9sys" + } + }, + "id": "hfjJgWRg4xiiiImwVRDtZ", + "type": "Entry", + "createdAt": "2020-11-04T12:28:30.442Z", + "updatedAt": "2020-11-26T16:39:54.188Z", + "environment": { + "sys": { + "id": "develop", + "type": "Link", + "linkType": "Environment" + } + }, + "revision": 6, + "contentType": { + "sys": { + "type": "Link", + "linkType": "ContentType", + "id": "question" + } + }, + "locale": "en-US" + }, + "fields": { + "slug": "/dev-start-which-service", + "title": "Which service do you need?", + "helpText": "Tell us which service you need.", + "type": "radios", + "options": [ + "Catering", + "Cleaning" + ], + "next": { + "sys": { + "type": "Link", + "linkType": "Entry", + "id": "52Ni9UFvZj8BYXSbhs373C" + } + } + } + } + ] +} diff --git a/spec/fixtures/contentful/no-primary-button-example.json b/spec/fixtures/contentful/no-primary-button-example.json new file mode 100644 index 000000000..07f995490 --- /dev/null +++ b/spec/fixtures/contentful/no-primary-button-example.json @@ -0,0 +1,37 @@ +{ + "sys": { + "space": { + "sys": { + "type": "Link", + "linkType": "Space", + "id": "rwl7tyzv9sys" + } + }, + "id": "5kZ9hIFDvNCEhjWs72SFwj", + "type": "Entry", + "createdAt": "2020-12-02T10:48:35.748Z", + "updatedAt": "2020-12-07T17:04:21.854Z", + "environment": { + "sys": { + "id": "develop", + "type": "Link", + "linkType": "Environment" + } + }, + "revision": 2, + "contentType": { + "sys": { + "type": "Link", + "linkType": "ContentType", + "id": "staticContent" + } + }, + "locale": "en-US" + }, + "fields": { + "slug": "/timelines", + "title": "When you should start", + "body": "Procuring a new catering contract can take up to 6 months to consult, create, review and award. \n\nUsually existing contracts start and end in the month of September. We recommend starting this process around March.", + "type": "paragraphs" + } +} diff --git a/spec/fixtures/contentful/primary-button-example.json b/spec/fixtures/contentful/primary-button-example.json new file mode 100644 index 000000000..5e80952ab --- /dev/null +++ b/spec/fixtures/contentful/primary-button-example.json @@ -0,0 +1,38 @@ +{ + "sys": { + "space": { + "sys": { + "type": "Link", + "linkType": "Space", + "id": "rwl7tyzv9sys" + } + }, + "id": "5kZ9hIFDvNCEhjWs72SFwj", + "type": "Entry", + "createdAt": "2020-12-02T10:48:35.748Z", + "updatedAt": "2020-12-07T17:04:21.854Z", + "environment": { + "sys": { + "id": "develop", + "type": "Link", + "linkType": "Environment" + } + }, + "revision": 2, + "contentType": { + "sys": { + "type": "Link", + "linkType": "ContentType", + "id": "staticContent" + } + }, + "locale": "en-US" + }, + "fields": { + "slug": "/timelines", + "title": "When you should start", + "body": "Procuring a new catering contract can take up to 6 months to consult, create, review and award. \n\nUsually existing contracts start and end in the month of September. We recommend starting this process around March.", + "type": "paragraphs", + "primaryCallToAction": "Go onwards!" + } +} diff --git a/spec/fixtures/contentful/repeat-entry-example.json b/spec/fixtures/contentful/repeat-entry-example.json new file mode 100644 index 000000000..72abdf553 --- /dev/null +++ b/spec/fixtures/contentful/repeat-entry-example.json @@ -0,0 +1,146 @@ +{ + "sys": { + "type": "Array" + }, + "total": 4, + "skip": 0, + "limit": 100, + "items": [ + { + "sys": { + "space": { + "sys": { + "type": "Link", + "linkType": "Space", + "id": "rwl7tyzv9sys" + } + }, + "id": "5kZ9hIFDvNCEhjWs72SFwj", + "type": "Entry", + "createdAt": "2020-12-02T10:48:35.748Z", + "updatedAt": "2020-12-02T10:48:35.748Z", + "environment": { + "sys": { + "id": "develop", + "type": "Link", + "linkType": "Environment" + } + }, + "revision": 1, + "contentType": { + "sys": { + "type": "Link", + "linkType": "ContentType", + "id": "staticContent" + } + }, + "locale": "en-US" + }, + "fields": { + "slug": "/timelines", + "title": "When you should start", + "body": "Procuring a new catering contract can take up to 6 months to consult, create, review and award. \n\nUsually existing contracts start and end in the month of September. We recommend starting this process around March.", + "type": "paragraphs", + "next": { + "sys": { + "type": "Link", + "linkType": "Entry", + "id": "hfjJgWRg4xiiiImwVRDtZ" + } + } + } + }, + { + "sys": { + "space": { + "sys": { + "type": "Link", + "linkType": "Space", + "id": "rwl7tyzv9sys" + } + }, + "id": "hfjJgWRg4xiiiImwVRDtZ", + "type": "Entry", + "createdAt": "2020-11-04T12:28:30.442Z", + "updatedAt": "2020-11-26T16:39:54.188Z", + "environment": { + "sys": { + "id": "develop", + "type": "Link", + "linkType": "Environment" + } + }, + "revision": 6, + "contentType": { + "sys": { + "type": "Link", + "linkType": "ContentType", + "id": "question" + } + }, + "locale": "en-US" + }, + "fields": { + "slug": "/dev-start-which-service", + "title": "Which service do you need?", + "helpText": "Tell us which service you need.", + "type": "radios", + "options": [ + "Catering", + "Cleaning" + ], + "next": { + "sys": { + "type": "Link", + "linkType": "Entry", + "id": "5kZ9hIFDvNCEhjWs72SFwj" + } + } + } + }, + { + "sys": { + "space": { + "sys": { + "type": "Link", + "linkType": "Space", + "id": "rwl7tyzv9sys" + } + }, + "id": "5kZ9hIFDvNCEhjWs72SFwj", + "type": "Entry", + "createdAt": "2020-12-02T10:48:35.748Z", + "updatedAt": "2020-12-02T10:48:35.748Z", + "environment": { + "sys": { + "id": "develop", + "type": "Link", + "linkType": "Environment" + } + }, + "revision": 1, + "contentType": { + "sys": { + "type": "Link", + "linkType": "ContentType", + "id": "staticContent" + } + }, + "locale": "en-US" + }, + "fields": { + "slug": "/timelines", + "title": "When you should start", + "body": "Procuring a new catering contract can take up to 6 months to consult, create, review and award. \n\nUsually existing contracts start and end in the month of September. We recommend starting this process around March.", + "type": "paragraphs", + "next": { + "sys": { + "type": "Link", + "linkType": "Entry", + "id": "hfjJgWRg4xiiiImwVRDtZ" + } + } + } + } + ] +} diff --git a/spec/fixtures/contentful/single-date-example.json b/spec/fixtures/contentful/single-date-example.json new file mode 100644 index 000000000..752133c8f --- /dev/null +++ b/spec/fixtures/contentful/single-date-example.json @@ -0,0 +1,36 @@ +{ + "sys": { + "space": { + "sys": { + "type": "Link", + "linkType": "Space", + "id": "rwl7tyzv9sys" + } + }, + "id": "55G5kpCLLL3h5yBQLiVlYy", + "type": "Entry", + "createdAt": "2020-12-08T10:43:19.102Z", + "updatedAt": "2020-12-08T10:43:19.102Z", + "environment": { + "sys": { + "id": "develop", + "type": "Link", + "linkType": "Environment" + } + }, + "revision": 1, + "contentType": { + "sys": { + "type": "Link", + "linkType": "ContentType", + "id": "question" + } + }, + "locale": "en-US" + }, + "fields": { + "slug": "/starting-date", + "type": "single date", + "title": "When will this start?" + } +} diff --git a/spec/helpers/step_helper_spec.rb b/spec/helpers/step_helper_spec.rb new file mode 100644 index 000000000..aa5172f53 --- /dev/null +++ b/spec/helpers/step_helper_spec.rb @@ -0,0 +1,25 @@ +require "rails_helper" + +RSpec.describe StepHelper, type: :helper do + describe "#radio_options" do + it "returns an array of OpenStruct objects" do + options = ["green", "yellow"] + result = helper.radio_options(array_of_options: options) + expect(result).to match([ + OpenStruct.new(id: "green", name: "Green"), + OpenStruct.new(id: "yellow", name: "Yellow") + ]) + end + end + + describe "#checkbox_options" do + it "returns an array of OpenStruct objects" do + options = ["red", "blue"] + result = helper.checkbox_options(array_of_options: options) + expect(result).to match([ + OpenStruct.new(id: "red", name: "Red"), + OpenStruct.new(id: "blue", name: "Blue") + ]) + end + end +end diff --git a/spec/jobs/warm_entry_cache_job_spec.rb b/spec/jobs/warm_entry_cache_job_spec.rb new file mode 100644 index 000000000..7de1f74b3 --- /dev/null +++ b/spec/jobs/warm_entry_cache_job_spec.rb @@ -0,0 +1,87 @@ +require "rails_helper" + +RSpec.describe WarmEntryCacheJob, type: :job do + include ActiveJob::TestHelper + + before(:each) { ActiveJob::Base.queue_adapter = :test } + after(:each) { RedisCache.redis.flushdb } + + around do |example| + ClimateControl.modify( + CONTENTFUL_PLANNING_START_ENTRY_ID: "5kZ9hIFDvNCEhjWs72SFwj", + CONTENTFUL_ENTRY_CACHING: "true" + ) do + example.run + end + end + + describe ".perform_later" do + it "enqueues a job asynchronously on the caching queue" do + expect { + described_class.perform_later + }.to have_enqueued_job.on_queue("caching") + end + + it "asks GetAllContentfulEntries for the Contentful entries" do + stub_get_contentful_entries( + entry_id: "5kZ9hIFDvNCEhjWs72SFwj", + fixture_filename: "multiple-entries-example.json" + ) + + described_class.perform_later + perform_enqueued_jobs + + expect(RedisCache.redis.get("contentful:entry:5kZ9hIFDvNCEhjWs72SFwj")) + .to eql( + "\"{\\\"sys\\\":{\\\"space\\\":{\\\"sys\\\":{\\\"type\\\":\\\"Link\\\",\\\"linkType\\\":\\\"Space\\\",\\\"id\\\":\\\"rwl7tyzv9sys\\\"}},\\\"id\\\":\\\"5kZ9hIFDvNCEhjWs72SFwj\\\",\\\"type\\\":\\\"Entry\\\",\\\"createdAt\\\":\\\"2020-12-02T10:48:35.748Z\\\",\\\"updatedAt\\\":\\\"2020-12-02T10:48:35.748Z\\\",\\\"environment\\\":{\\\"sys\\\":{\\\"id\\\":\\\"develop\\\",\\\"type\\\":\\\"Link\\\",\\\"linkType\\\":\\\"Environment\\\"}},\\\"revision\\\":1,\\\"contentType\\\":{\\\"sys\\\":{\\\"type\\\":\\\"Link\\\",\\\"linkType\\\":\\\"ContentType\\\",\\\"id\\\":\\\"staticContent\\\"}},\\\"locale\\\":\\\"en-US\\\"},\\\"fields\\\":{\\\"slug\\\":\\\"/timelines\\\",\\\"title\\\":\\\"When you should start\\\",\\\"body\\\":\\\"Procuring a new catering contract can take up to 6 months to consult, create, review and award. \\\\n\\\\nUsually existing contracts start and end in the month of September. We recommend starting this process around March.\\\",\\\"type\\\":\\\"paragraphs\\\",\\\"next\\\":{\\\"sys\\\":{\\\"type\\\":\\\"Link\\\",\\\"linkType\\\":\\\"Entry\\\",\\\"id\\\":\\\"hfjJgWRg4xiiiImwVRDtZ\\\"}}}}\"" + ) + expect(RedisCache.redis.get("contentful:entry:hfjJgWRg4xiiiImwVRDtZ")) + .to eql( + "\"{\\\"sys\\\":{\\\"space\\\":{\\\"sys\\\":{\\\"type\\\":\\\"Link\\\",\\\"linkType\\\":\\\"Space\\\",\\\"id\\\":\\\"rwl7tyzv9sys\\\"}},\\\"id\\\":\\\"hfjJgWRg4xiiiImwVRDtZ\\\",\\\"type\\\":\\\"Entry\\\",\\\"createdAt\\\":\\\"2020-11-04T12:28:30.442Z\\\",\\\"updatedAt\\\":\\\"2020-11-26T16:39:54.188Z\\\",\\\"environment\\\":{\\\"sys\\\":{\\\"id\\\":\\\"develop\\\",\\\"type\\\":\\\"Link\\\",\\\"linkType\\\":\\\"Environment\\\"}},\\\"revision\\\":6,\\\"contentType\\\":{\\\"sys\\\":{\\\"type\\\":\\\"Link\\\",\\\"linkType\\\":\\\"ContentType\\\",\\\"id\\\":\\\"question\\\"}},\\\"locale\\\":\\\"en-US\\\"},\\\"fields\\\":{\\\"slug\\\":\\\"/dev-start-which-service\\\",\\\"title\\\":\\\"Which service do you need?\\\",\\\"helpText\\\":\\\"Tell us which service you need.\\\",\\\"type\\\":\\\"radios\\\",\\\"options\\\":[\\\"Catering\\\",\\\"Cleaning\\\"],\\\"next\\\":{\\\"sys\\\":{\\\"type\\\":\\\"Link\\\",\\\"linkType\\\":\\\"Entry\\\",\\\"id\\\":\\\"52Ni9UFvZj8BYXSbhs373C\\\"}}}}\"" + ) + end + + context "when the journey order cannot be built" do + it "does not add new items to the cache" do + allow_any_instance_of(BuildJourneyOrder) + .to receive(:call) + .and_raise(BuildJourneyOrder::RepeatEntryDetected) + + stub_get_contentful_entries( + entry_id: "5kZ9hIFDvNCEhjWs72SFwj", + fixture_filename: "repeat-entry-example.json" + ) + + described_class.perform_later + perform_enqueued_jobs + + expect(RedisCache.redis).not_to receive(:set).with("contentful:entry:5kZ9hIFDvNCEhjWs72SFwj", anything) + end + + it "extends the TTL on all existing items by 24 hours" do + RedisCache.redis.set("contentful:entry:5kZ9hIFDvNCEhjWs72SFwj", "\"{\\}\"") + + allow_any_instance_of(BuildJourneyOrder) + .to receive(:call) + .and_raise(BuildJourneyOrder::RepeatEntryDetected) + + stub_get_contentful_entries( + entry_id: "5kZ9hIFDvNCEhjWs72SFwj", + fixture_filename: "repeat-entry-example.json" + ) + + freeze_time do + ttl_extension = 60 * 60 * 24 + existing_ttl = -1 + + expect(RedisCache.redis) + .to receive(:expire) + .with("contentful:entry:5kZ9hIFDvNCEhjWs72SFwj", ttl_extension + existing_ttl) + + described_class.perform_later + perform_enqueued_jobs + end + end + end + end +end diff --git a/spec/models/checkbox_answers_spec.rb b/spec/models/checkbox_answers_spec.rb new file mode 100644 index 000000000..e2bf38f8e --- /dev/null +++ b/spec/models/checkbox_answers_spec.rb @@ -0,0 +1,32 @@ +require "rails_helper" + +RSpec.describe CheckboxAnswers, type: :model do + it { should belong_to(:step) } + + describe "#response" do + it "returns an array of strings" do + answer = build(:checkbox_answers, response: ["Blue", "Green"]) + expect(answer.response).to eq(["Blue", "Green"]) + end + end + + describe "validations" do + it { is_expected.to validate_presence_of(:response) } + end + + describe "#response=" do + context "when the response only includes blank items" do + it "should be invalid" do + answer = build(:checkbox_answers, response: ["", ""]) + expect(answer.valid?).to eq(false) + end + end + + context "when the response a mix of present and blank items" do + it "should only store the present values" do + answer = build(:checkbox_answers, response: ["Foo", ""]) + expect(answer.response).to eq(["Foo"]) + end + end + end +end diff --git a/spec/models/long_text_answer_spec.rb b/spec/models/long_text_answer_spec.rb new file mode 100644 index 000000000..f71ec374c --- /dev/null +++ b/spec/models/long_text_answer_spec.rb @@ -0,0 +1,14 @@ +require "rails_helper" + +RSpec.describe LongTextAnswer, type: :model do + it { should belong_to(:step) } + + it "captures the users response as a string" do + answer = build(:long_text_answer, response: "Yellow") + expect(answer.response).to eql("Yellow") + end + + describe "validations" do + it { is_expected.to validate_presence_of(:response) } + end +end diff --git a/spec/models/single_date_answer_spec.rb b/spec/models/single_date_answer_spec.rb new file mode 100644 index 000000000..58d8ac896 --- /dev/null +++ b/spec/models/single_date_answer_spec.rb @@ -0,0 +1,16 @@ +require "rails_helper" + +RSpec.describe SingleDateAnswer, type: :model do + it { should belong_to(:step) } + + describe "#response" do + it "returns a date" do + answer = build(:single_date_answer, response: Date.new(2020, 10, 1)) + expect(answer.response).to eq(Date.new(2020, 10, 1)) + end + end + + describe "validations" do + it { is_expected.to validate_presence_of(:response) } + end +end diff --git a/spec/models/step_spec.rb b/spec/models/step_spec.rb index 11bfb2abe..54c8a1956 100644 --- a/spec/models/step_spec.rb +++ b/spec/models/step_spec.rb @@ -4,6 +4,9 @@ it { should belong_to(:journey) } it { should have_one(:radio_answer) } it { should have_one(:short_text_answer) } + it { should have_one(:long_text_answer) } + it { should have_one(:single_date_answer) } + it { should have_one(:checkbox_answers) } it "store the basic fields of a contentful response" do step = build(:step, @@ -18,11 +21,8 @@ end it "captures the raw contentful response" do - contentful_json_response = JSON("foo": {}) - step = build(:step, - raw: contentful_json_response) - - expect(step.raw).to eql("{\"foo\":{}}") + step = build(:step, raw: {foo: "bar"}) + expect(step.raw).to eql({"foo" => "bar"}) end describe "#answer" do @@ -41,20 +41,28 @@ expect(step.answer).to eq(short_text_answer) end end - end - describe "#question?" do - context "when the contentful model is 'question'" do - it "returns true" do - step = build(:step, :radio, contentful_model: "question") - expect(step.question?).to eq(true) + context "when a LongTextAnswer is associated to the step" do + it "returns the LongTextAnswer object" do + long_text_answer = create(:long_text_answer) + step = create(:step, :long_text, long_text_answer: long_text_answer) + expect(step.answer).to eq(long_text_answer) + end + end + + context "when a SingleDateAnswer is associated to the step" do + it "returns the SingleDateAnswer object" do + single_date_answer = create(:single_date_answer) + step = create(:step, :single_date, single_date_answer: single_date_answer) + expect(step.answer).to eq(single_date_answer) end end - context "when the contentful model is NOT 'question'" do - it "returns false" do - step = build(:step, contentful_model: "staticContent") - expect(step.question?).to eq(false) + context "when a CheckboxAnswers is associated to the step" do + it "returns the CheckboxAnswers object" do + checkbox_answers = create(:checkbox_answers) + step = create(:step, :checkbox_answers, checkbox_answers: checkbox_answers) + expect(step.answer).to eq(checkbox_answers) end end end diff --git a/spec/presenters/step_presenter_spec.rb b/spec/presenters/step_presenter_spec.rb new file mode 100644 index 000000000..1fc44e9e2 --- /dev/null +++ b/spec/presenters/step_presenter_spec.rb @@ -0,0 +1,21 @@ +require "rails_helper" + +RSpec.describe StepPresenter do + describe "#question?" do + context "when the contentful model is 'question'" do + it "returns true" do + step = build(:step, :radio, contentful_model: "question") + presenter = described_class.new(step) + expect(presenter.question?).to eq(true) + end + end + + context "when the contentful model is NOT 'question'" do + it "returns false" do + step = build(:step, contentful_model: "staticContent") + presenter = described_class.new(step) + expect(presenter.question?).to eq(false) + end + end + end +end diff --git a/spec/requests/contentful_caching_spec.rb b/spec/requests/contentful_caching_spec.rb index c723b5684..ef2843584 100644 --- a/spec/requests/contentful_caching_spec.rb +++ b/spec/requests/contentful_caching_spec.rb @@ -40,7 +40,7 @@ RedisCache.redis.del("contentful:entry:1UjQurSOi5MWkcRuGxdXZS") end - it "sets a TTL to 48 hours by default" do + it "sets a TTL to 72 hours by default" do journey = create(:journey, next_entry_id: "1UjQurSOi5MWkcRuGxdXZS") stub_get_contentful_entry( entry_id: "1UjQurSOi5MWkcRuGxdXZS", @@ -51,7 +51,7 @@ get new_journey_step_path(journey) expect(RedisCache.redis.ttl("contentful:entry:1UjQurSOi5MWkcRuGxdXZS")) - .to eq(172_800) + .to eq(60 * 60 * 72) end RedisCache.redis.del("contentful:entry:1UjQurSOi5MWkcRuGxdXZS") diff --git a/spec/services/build_journey_order_spec.rb b/spec/services/build_journey_order_spec.rb new file mode 100644 index 000000000..f4ce4ea2a --- /dev/null +++ b/spec/services/build_journey_order_spec.rb @@ -0,0 +1,94 @@ +require "rails_helper" + +RSpec.describe BuildJourneyOrder do + describe "#call" do + around do |example| + ClimateControl.modify( + CONTENTFUL_PLANNING_START_ENTRY_ID: "5kZ9hIFDvNCEhjWs72SFwj" + ) do + example.run + end + end + + context "when there are multiple entries" do + it "returns each ID in the order they appear" do + fake_entries = fake_contentful_entry_array( + contentful_fixture_filename: "closed-path-with-multiple-example.json" + ) + + result = described_class.new( + entries: fake_entries, + starting_entry_id: "5kZ9hIFDvNCEhjWs72SFwj" + ).call + + expect(result).to match([fake_entries.first, fake_entries.last]) + end + end + + context "when the journey visits the same node twice" do + around do |example| + ClimateControl.modify( + CONTENTFUL_PLANNING_START_ENTRY_ID: "5kZ9hIFDvNCEhjWs72SFwj" + ) do + example.run + end + end + + it "raises a rollbar event" do + fake_entries = fake_contentful_entry_array( + contentful_fixture_filename: "repeat-entry-example.json" + ) + + expect(Rollbar).to receive(:error) + .with("A repeated Contentful entry was found in the same journey", + contentful_url: ENV["CONTENTFUL_URL"], + contentful_space_id: ENV["CONTENTFUL_SPACE"], + contentful_environment: ENV["CONTENTFUL_ENVIRONMENT"], + contentful_entry_id: "5kZ9hIFDvNCEhjWs72SFwj") + .and_call_original + + expect { + described_class.new( + entries: fake_entries, + starting_entry_id: "5kZ9hIFDvNCEhjWs72SFwj" + ).call + }.to raise_error(BuildJourneyOrder::RepeatEntryDetected) + end + end + + context "when the journey visits more than the maximum permitted number of entries" do + around do |example| + ClimateControl.modify( + CONTENTFUL_PLANNING_START_ENTRY_ID: "5kZ9hIFDvNCEhjWs72SFwj" + ) do + example.run + end + end + + it "raises a rollbar event with the last entry" do + # Creating 50 linked entries in a fixture slows the test suite down + # We assume these 50 entries are chained together in a linear sequence. + fake_entries = fake_contentful_entry_array( + contentful_fixture_filename: "closed-path-with-multiple-example.json" + ) + expect(BuildJourneyOrder::ENTRY_JOURNEY_MAX_LENGTH).to eq(50) + stub_const("BuildJourneyOrder::ENTRY_JOURNEY_MAX_LENGTH", 1) + + expect(Rollbar).to receive(:error) + .with("More than #{BuildJourneyOrder::ENTRY_JOURNEY_MAX_LENGTH} steps were found in a journey map", + contentful_url: ENV["CONTENTFUL_URL"], + contentful_space_id: ENV["CONTENTFUL_SPACE"], + contentful_environment: ENV["CONTENTFUL_ENVIRONMENT"], + contentful_entry_id: "hfjJgWRg4xiiiImwVRDtZ") + .and_call_original + + expect { + described_class.new( + entries: fake_entries, + starting_entry_id: "5kZ9hIFDvNCEhjWs72SFwj" + ).call + }.to raise_error(BuildJourneyOrder::TooManyChainedEntriesDetected) + end + end + end +end diff --git a/spec/services/cache_spec.rb b/spec/services/cache_spec.rb index 52b3b4184..e8c473adc 100644 --- a/spec/services/cache_spec.rb +++ b/spec/services/cache_spec.rb @@ -1,16 +1,19 @@ require "rails_helper" RSpec.describe Cache do + after(:each) do + RedisCache.redis.flushdb + end + it "requires a key, enabled flag and ttl" do - result = described_class.new(key: "redis-key", enabled: "true", ttl: 123) - expect(result.key).to eql("redis-key") + result = described_class.new(enabled: "true", ttl: 123) expect(result.enabled).to eql("true") expect(result.ttl).to eql(123) end describe "#redis_cache" do it "returns an object that quacks like a redis instance (since MockRedis is used in Test)" do - result = described_class.new(key: anything, enabled: anything, ttl: anything) + result = described_class.new(enabled: anything, ttl: anything) .redis_cache expect(result.respond_to?(:get)).to eq(true) @@ -23,23 +26,23 @@ describe "#hit?" do context "when enabled" do it "asks redis if that key exists" do - cache = described_class.new(key: "key-to-find", enabled: "true", ttl: anything) + cache = described_class.new(enabled: "true", ttl: anything) redis = cache.redis_cache expect(redis).to receive(:exists?).with("key-to-find") - cache.hit? + cache.hit?(key: "key-to-find") end end context "when disabled" do it "returns false" do - cache = described_class.new(key: "key-to-find", enabled: "false", ttl: anything) + cache = described_class.new(enabled: "false", ttl: anything) redis = cache.redis_cache expect(redis).not_to receive(:exists?).with("key-to-find") - result = cache.hit? + result = cache.hit?(key: "key-to-find") expect(result).to eq(false) end end @@ -47,37 +50,74 @@ describe "#get" do it "asks redis to get that key" do - cache = described_class.new(key: "key-to-find", enabled: "false", ttl: anything) + cache = described_class.new(enabled: "false", ttl: anything) redis = cache.redis_cache expect(redis).to receive(:get).with("key-to-find") - cache.get + cache.get(key: "key-to-find") end end describe "#set" do context "when enabled" do it "asks redis to set that key with the ttl" do - cache = described_class.new(key: "key-to-set", enabled: "true", ttl: 123) + cache = described_class.new(enabled: "true", ttl: 123) redis = cache.redis_cache expect(redis).to receive(:set).with("key-to-set", "value-to-set") expect(redis).to receive(:expire).with("key-to-set", 123) - cache.set(value: "value-to-set") + cache.set(key: "key-to-set", value: "value-to-set") end end context "when disabled" do it "does not ask redis to set that key" do - cache = described_class.new(key: "key-to-set", enabled: "false", ttl: 123) + cache = described_class.new(enabled: "false", ttl: 123) redis = cache.redis_cache expect(redis).not_to receive(:set).with("key-to-set", "value-to-set") expect(redis).not_to receive(:expire).with("key-to-set", 123) - cache.set(value: "value-to-set") + cache.set(key: "key-to-set", value: "value-to-set") + end + end + end + + describe "#extend_ttl_on_all_entries" do + it "only updates redis keys associated to contentful entries" do + cache = described_class.new(enabled: anything, ttl: anything) + redis = cache.redis_cache + + redis.set("contentful:entry:123", "value") + expect(redis).to receive(:expire).with("contentful:entry:123", (60 * 60 * 24) + -1) + + cache.extend_ttl_on_all_entries + end + + context "when other redis keys exist" do + it "only updates redis keys associated to contentful entries" do + cache = described_class.new(enabled: anything, ttl: anything) + redis = cache.redis_cache + + redis.set("another_key", "another_value") + expect(redis).not_to receive(:expire).with("another_key") + + cache.extend_ttl_on_all_entries + end + end + + context "when the key already has a TTL" do + it "sets a combined TTL with the existing and the extension" do + cache = described_class.new(enabled: anything, ttl: anything) + redis = cache.redis_cache + + redis.set("contentful:entry:123", "value") + redis.expire("contentful:entry:123", 10) + expect(redis).to receive(:expire).with("contentful:entry:123", (60 * 60 * 24) + 10) + + cache.extend_ttl_on_all_entries end end end diff --git a/spec/services/create_journey_step_spec.rb b/spec/services/create_journey_step_spec.rb index 86edab1cf..4d08d6028 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_entry( + fake_entry = fake_contentful_entry( contentful_fixture_filename: "radio-question-example.json" ) @@ -13,15 +13,53 @@ expect(step.title).to eq("Which service do you need?") expect(step.help_text).to eq("Tell us which service you need.") + expect(step.contentful_id).to eq("1UjQurSOi5MWkcRuGxdXZS") expect(step.contentful_model).to eq("question") expect(step.contentful_type).to eq("radios") expect(step.options).to eq(["Catering", "Cleaning"]) - expect(step.raw).to eq(fake_entry.raw) + expect(step.raw).to eq( + "fields" => { + "helpText" => "Tell us which service you need.", + "options" => ["Catering", "Cleaning"], + "slug" => "/which-service", + "title" => "Which service do you need?", + "type" => "radios" + }, + "sys" => { + "contentType" => { + "sys" => { + "id" => "question", + "linkType" => "ContentType", + "type" => "Link" + } + }, + "createdAt" => "2020-09-07T10:56:40.585Z", + "environment" => { + "sys" => { + "id" => "master", + "linkType" => "Environment", + "type" => "Link" + } + }, + "id" => "1UjQurSOi5MWkcRuGxdXZS", + "locale" => "en-US", + "revision" => 7, + "space" => { + "sys" => { + "id" => "jspwts36h1os", + "linkType" => "Space", + "type" => "Link" + } + }, + "type" => "Entry", + "updatedAt" => "2020-09-14T22:16:54.633Z" + } + ) end it "updates the journey with a new next_entry_id" do journey = create(:journey, :catering) - fake_entry = fake_contentful_step_entry( + fake_entry = fake_contentful_entry( contentful_fixture_filename: "has-next-question-example.json" ) @@ -34,7 +72,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_entry( + fake_entry = fake_contentful_entry( contentful_fixture_filename: "short-text-question-example.json" ) @@ -45,7 +83,7 @@ it "replaces spaces with underscores" do journey = create(:journey, :catering) - fake_entry = fake_contentful_step_entry( + fake_entry = fake_contentful_entry( contentful_fixture_filename: "short-text-question-example.json" ) @@ -58,7 +96,7 @@ context "when the new step does not have a following step" do it "updates the journey by setting the next_entry_id to nil" do journey = create(:journey, :catering) - fake_entry = fake_contentful_step_entry( + fake_entry = fake_contentful_entry( contentful_fixture_filename: "radio-question-example.json" ) @@ -71,7 +109,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_entry( + fake_entry = fake_contentful_entry( contentful_fixture_filename: "static-content-example.json" ) @@ -86,10 +124,40 @@ end end + 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_entry( + contentful_fixture_filename: "primary-button-example.json" + ) + + step, _answer = described_class.new( + journey: journey, contentful_entry: fake_entry + ).call + + expect(step.primary_call_to_action_text).to eq("Go onwards!") + end + end + + context "when no 'primaryCallToAction' is provided" do + it "default copy is used for the button" do + journey = create(:journey, :catering) + fake_entry = fake_contentful_entry( + contentful_fixture_filename: "no-primary-button-example.json" + ) + + step, _answer = described_class.new( + journey: journey, contentful_entry: fake_entry + ).call + + expect(step.primary_call_to_action_text).to eq(I18n.t("generic.button.next")) + end + end + 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_entry( + fake_entry = fake_contentful_entry( contentful_fixture_filename: "an-unexpected-model-example.json" ) @@ -100,7 +168,7 @@ it "raises a rollbar event" do journey = create(:journey, :catering) - fake_entry = fake_contentful_step_entry( + fake_entry = fake_contentful_entry( contentful_fixture_filename: "an-unexpected-model-example.json" ) @@ -123,7 +191,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_entry( + fake_entry = fake_contentful_entry( contentful_fixture_filename: "an-unexpected-question-type-example.json" ) @@ -134,7 +202,7 @@ it "raises a rollbar event" do journey = create(:journey, :catering) - fake_entry = fake_contentful_step_entry( + fake_entry = fake_contentful_entry( contentful_fixture_filename: "an-unexpected-question-type-example.json" ) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 2fe7150d9..b5c1e182a 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -20,6 +20,9 @@ SimpleCov.minimum_coverage 99 SimpleCov.start "rails" +require "webmock/rspec" +WebMock.disable_net_connect!(allow_localhost: true) + RSpec.configure do |config| # rspec-expectations config goes here. You can use an alternate # assertion/expectation library such as wrong or the stdlib/minitest diff --git a/spec/support/contentful_helpers.rb b/spec/support/contentful_helpers.rb index 819898e5b..7d0389c6b 100644 --- a/spec/support/contentful_helpers.rb +++ b/spec/support/contentful_helpers.rb @@ -6,7 +6,7 @@ def stub_get_contentful_entry( raw_response = File.read("#{Rails.root}/spec/fixtures/contentful/#{fixture_filename}") contentful_connector = stub_contentful_connector - contentful_response = fake_contentful_step_entry(contentful_fixture_filename: fixture_filename) + contentful_response = fake_contentful_entry(contentful_fixture_filename: fixture_filename) allow(contentful_connector).to receive(:get_entry_by_id) .with(entry_id) .and_return(contentful_response) @@ -15,6 +15,21 @@ def stub_get_contentful_entry( .and_return(raw_response) end + def stub_get_contentful_entries( + entry_id: "5kZ9hIFDvNCEhjWs72SFwj", + fixture_filename: "multiple-entries-example.json" + ) + raw_response = File.read("#{Rails.root}/spec/fixtures/contentful/#{fixture_filename}") + + contentful_connector = stub_contentful_connector + contentful_response = fake_contentful_entry_array(contentful_fixture_filename: fixture_filename) + allow(contentful_connector).to receive(:get_all_entries) + .and_return(contentful_response) + + allow(contentful_response).to receive(:raw) + .and_return(raw_response) + end + def stub_contentful_connector contentful_connector = instance_double(ContentfulConnector) expect(ContentfulConnector).to receive(:new) @@ -30,13 +45,7 @@ def stub_contentful_client contentful_client end - def stub_contentful_step(fake_entry: fake_contentful_step_entry) - get_contentful_step_double = instance_double(GetContentfulEntry) - allow(GetContentfulEntry).to receive(:new).and_return(get_contentful_step_double) - allow(get_contentful_step_double).to receive(:call).and_return(fake_entry) - end - - def fake_contentful_step_entry(contentful_fixture_filename:) + def fake_contentful_entry(contentful_fixture_filename:) raw_response = File.read("#{Rails.root}/spec/fixtures/contentful/#{contentful_fixture_filename}") hash_response = JSON.parse(raw_response) @@ -49,8 +58,16 @@ def fake_contentful_step_entry(contentful_fixture_filename:) options: hash_response.dig("fields", "options"), type: hash_response.dig("fields", "type"), next: double(id: hash_response.dig("fields", "next", "sys", "id")), - raw: raw_response, + primary_call_to_action: hash_response.dig("fields", "primaryCallToAction"), + raw: hash_response, content_type: double(id: hash_response.dig("sys", "contentType", "sys", "id")) ) end + + def fake_contentful_entry_array(contentful_fixture_filename:) + raw_response = File.read("#{Rails.root}/spec/fixtures/contentful/#{contentful_fixture_filename}") + response_hash = JSON.parse(raw_response) + + Contentful::ResourceBuilder.new(response_hash).run + end end