Merge branch 'main' into CLDC-358-conditional-page-routing

Gemfile

@@ -19,8 +19,12 @@ gem "jbuilder", "~> 2.7"
 gem "bootsnap", ">= 1.4.4", require: false
 # Gov.UK frontend components
 gem "govuk-components"
+# Gov.UK component form builder DSL
 gem "govuk_design_system_formbuilder"
+# Turbo & Stimulus
 gem "hotwire-rails"
+# Soft delete ActiveRecords objects
+gem "discard"
 
 group :development, :test do
   # Call 'byebug' anywhere in the code to stop execution and get a debugger console

Gemfile.lock

@@ -123,12 +123,14 @@ GEM
       rack-test (>= 0.6.3)
       regexp_parser (>= 1.5, < 3.0)
       xpath (~> 3.2)
-    childprocess (3.0.0)
+    childprocess (4.1.0)
     coderay (1.1.3)
     concurrent-ruby (1.1.9)
     crass (1.0.6)
     deep_merge (1.2.1)
     diff-lcs (1.4.4)
+    discard (1.2.0)
+      activerecord (>= 4.2, < 7)
     docile (1.4.0)
     dotenv (2.7.6)
     dotenv-rails (2.7.6)
@@ -143,7 +145,7 @@ GEM
     ffi (1.15.4)
     globalid (0.5.2)
       activesupport (>= 5.0)
-    govuk-components (2.1.2)
+    govuk-components (2.1.3)
       activemodel (>= 6.0)
       railties (>= 6.0)
       view_component (~> 2.39.0)
@@ -171,7 +173,7 @@ GEM
       mini_mime (>= 0.1.1)
     marcel (1.0.2)
     method_source (1.0.0)
-    mini_mime (1.1.1)
+    mini_mime (1.1.2)
     minitest (5.14.4)
     msgpack (1.4.2)
     nio4r (2.5.8)
@@ -194,7 +196,7 @@ GEM
       byebug (~> 11.0)
       pry (~> 0.13.0)
     public_suffix (4.0.6)
-    puma (5.5.0)
+    puma (5.5.2)
       nio4r (~> 2.0)
     racc (1.5.2)
     rack (2.2.3)
@@ -277,8 +279,9 @@ GEM
       sass (~> 3.5, >= 3.5.5)
     scss_lint-govuk (0.2.0)
       scss_lint
-    selenium-webdriver (3.142.7)
+    selenium-webdriver (4.0.0)
-      childprocess (>= 0.5, < 4.0)
+      childprocess (>= 0.5, < 5.0)
+      rexml (~> 3.2, >= 3.2.5)
       rubyzip (>= 1.2.2)
     semantic_range (3.0.0)
     simplecov (0.21.2)
@@ -294,10 +297,10 @@ GEM
       actionpack (>= 4.0)
       activesupport (>= 4.0)
       sprockets (>= 3.0.0)
-    stimulus-rails (0.7.0)
+    stimulus-rails (0.7.1)
       rails (>= 6.0.0)
     thor (1.1.0)
-    turbo-rails (0.8.1)
+    turbo-rails (7.1.0)
       rails (>= 6.0.0)
     tzinfo (2.0.4)
       concurrent-ruby (~> 1.0)
@@ -331,6 +334,7 @@ DEPENDENCIES
   bootsnap (>= 1.4.4)
   byebug
   capybara
+  discard
   dotenv-rails
   factory_bot_rails
   govuk-components

README.md

@@ -4,6 +4,12 @@
 
 This is the codebase for the Ruby on Rails app that will handle the submission of Lettings and Sales of Social Housing in England data.
 
+
+## API documentation
+
+API documentation can be found here: https://communitiesuk.github.io/mhclg-data-collection-beta/. This is driven by [OpenAPI docs](docs/api/DLUHC-CORE-Data.v1.json)
+
+
 ## Required Setup
 
 Pre-requisites:
@@ -70,6 +76,16 @@ Once the app is deployed:
 2. Check logs:\
 `cf logs dluhc-core --recent`
 
+#### Troubleshooting deployments
+
+A failed Github deployment action will occasionally leave a Cloud Foundry deployment in a broken state. As a result all subsequent Github deployment actions will also fail with the message `Cannot update this process while a deployment is in flight`.
+
+`
+cf cancel-deployment dluhc-core
+`
+
+You'd then need to check the logs and fix the issue that caused the initial deployment to fail.
+
 ## CI/CD
 
 When a commit is made to `main` the following GitHub action jobs are triggered:

app/controllers/case_logs_controller.rb

@@ -1,17 +1,14 @@
 class CaseLogsController < ApplicationController
-  skip_before_action :verify_authenticity_token, if: :json_create_request?
+  skip_before_action :verify_authenticity_token, if: :json_api_request?
-  before_action :authenticate, if: :json_create_request?
+  before_action :authenticate, if: :json_api_request?
-  # rubocop:disable Style/ClassVars
-  @@form_handler = FormHandler.instance
-  # rubocop:enable Style/ClassVars
 
   def index
-    @submitted_case_logs = CaseLog.where(status: 1)
+    @completed_case_logs = CaseLog.completed
-    @in_progress_case_logs = CaseLog.where(status: 0)
+    @in_progress_case_logs = CaseLog.in_progress
   end
 
   def create
-    case_log = CaseLog.create(create_params)
+    case_log = CaseLog.create(api_case_log_params)
     respond_to do |format|
       format.html { redirect_to case_log }
       format.json do
@@ -24,19 +21,40 @@ class CaseLogsController < ApplicationController
     end
   end
 
-  # We don't have a dedicated non-editable show view
+  def update
+    if (case_log = CaseLog.find_by(id: params[:id]))
+      if case_log.update(api_case_log_params)
+        render json: case_log, status: :ok
+      else
+        render json: { errors: case_log.errors.full_messages }, status: :unprocessable_entity
+      end
+    else
+      render json: { error: "Case Log #{params[:id]} not found" }, status: :not_found
+    end
+  end
+
   def show
-    edit
+    respond_to do |format|
+      # We don't have a dedicated non-editable show view
+      format.html { edit }
+      format.json do
+        if (case_log = CaseLog.find_by(id: params[:id]))
+          render json: case_log, status: :ok
+        else
+          render json: { error: "Case Log #{params[:id]} not found" }, status: :not_found
+        end
+      end
+    end
   end
 
   def edit
-    @form = @@form_handler.get_form("2021_2022")
+    @form = FormHandler.instance.get_form("2021_2022")
     @case_log = CaseLog.find(params[:id])
     render :edit
   end
 
   def submit_form
-    form = @@form_handler.get_form("2021_2022")
+    form = FormHandler.instance.get_form("2021_2022")
     @case_log = CaseLog.find(params[:id])
     previous_page = params[:case_log][:previous_page]
     questions_for_page = form.questions_for_page(previous_page)
@@ -51,15 +69,27 @@ class CaseLogsController < ApplicationController
     end
   end
 
+  def destroy
+    if (case_log = CaseLog.find_by(id: params[:id]))
+      if case_log.discard
+        head :no_content
+      else
+        render json: { errors: case_log.errors.full_messages }, status: :unprocessable_entity
+      end
+    else
+      render json: { error: "Case Log #{params[:id]} not found" }, status: :not_found
+    end
+  end
+
   def check_answers
-    form = @@form_handler.get_form("2021_2022")
+    form = FormHandler.instance.get_form("2021_2022")
     @case_log = CaseLog.find(params[:case_log_id])
     current_url = request.env["PATH_INFO"]
     subsection = current_url.split("/")[-2]
-    render "form/check_answers", locals: { case_log: @case_log, subsection: subsection, form: form }
+    render "form/check_answers", locals: { subsection: subsection, form: form }
   end
 
-  form = @@form_handler.get_form("2021_2022")
+  form = FormHandler.instance.get_form("2021_2022")
   form.all_pages.map do |page_key, page_info|
     define_method(page_key) do |_errors = {}|
       @case_log = CaseLog.find(params[:case_log_id])
@@ -69,6 +99,8 @@ class CaseLogsController < ApplicationController
 
 private
 
+  API_ACTIONS = %w[create show update destroy].freeze
+
   def question_responses(questions_for_page)
     questions_for_page.each_with_object({}) do |(question_key, question_info), result|
       question_params = params["case_log"][question_key]
@@ -83,15 +115,15 @@ private
     end
   end
 
-  def json_create_request?
+  def json_api_request?
-    (request["action"] == "create") && request.format.json?
+    API_ACTIONS.include?(request["action"]) && request.format.json?
   end
 
   def authenticate
     http_basic_authenticate_or_request_with name: ENV["API_USER"], password: ENV["API_KEY"]
   end
 
-  def create_params
+  def api_case_log_params
     return {} unless params[:case_log]
 
     params.require(:case_log).permit(CaseLog.editable_fields)

app/helpers/check_answers_helper.rb

@@ -55,9 +55,9 @@ module CheckAnswersHelper
   def condition_not_met(case_log, question_key, question, condition)
     case question["type"]
     when "numeric"
-      # rubocop:disable Security/Eval
+      operator = condition[/[<>=]+/].to_sym
-      case_log[question_key].blank? || !eval(case_log[question_key].to_s + condition)
+      operand = condition[/\d+/].to_i
-    # rubocop:enable Security/Eval
+      case_log[question_key].blank? || !case_log[question_key].send(operator, operand)
     when "radio"
       case_log[question_key].blank? || !condition.include?(case_log[question_key])
     else

app/helpers/tasklist_helper.rb

@@ -15,7 +15,7 @@ module TasklistHelper
 
   def get_subsection_status(subsection_name, case_log, questions)
     if subsection_name == "declaration"
-      return all_questions_completed(case_log) ? :not_started : :cannot_start_yet
+      return case_log.completed? ? :not_started : :cannot_start_yet
     end
 
     return :not_started if questions.all? { |question| case_log[question].blank? }
@@ -47,10 +47,6 @@ module TasklistHelper
 
 private
 
-  def all_questions_completed(case_log)
-    case_log.attributes.all? { |_question, answer| answer.present? }
-  end
-
   def is_incomplete?(subsection, case_log, questions)
     status = get_subsection_status(subsection, case_log, questions)
     %i[not_started in_progress].include?(status)

app/javascript/controllers/conditional_question_controller.js

@@ -28,7 +28,10 @@ export default class extends Controller {
           div.style.display = "block"
         } else {
           div.style.display = "none"
-          let buttons = document.getElementsByName(`case_log[${targetQuestion}]`)
+          let buttons = document.getElementsByName(`case_log[${targetQuestion}]`);
+          if (buttons.length == 0){
+            buttons = document.getElementsByName(`case_log[${targetQuestion}][]`);
+          }
           Object.entries(buttons).forEach(([idx, button]) => {
             button.checked = false;
           })

app/models/case_log.rb

@@ -1,7 +1,7 @@
 class CaseLogValidator < ActiveModel::Validator
-  # Methods need to be named 'validate_' followed by field name
+  # Methods to be used on save and continue need to be named 'validate_'
-  # this is how the metaprogramming of the method name being
+  # followed by field name this is how the metaprogramming of the method
-  # call in the validate method works.
+  # name being call in the validate method works.
 
   def validate_tenant_age(record)
     if record.tenant_age && !/^[1-9][0-9]?$|^120$/.match?(record.tenant_age.to_s)
@@ -9,13 +9,45 @@ class CaseLogValidator < ActiveModel::Validator
     end
   end
 
+  def validate_property_number_of_times_relet(record)
+    if record.property_number_of_times_relet && !/^[1-9]$|^0[1-9]$|^1[0-9]$|^20$/.match?(record.property_number_of_times_relet.to_s)
+      record.errors.add :property_number_of_times_relet, "must be between 0 and 20"
+    end
+  end
+
+  def validate_reasonable_preference(record)
+    if record.homelessness == "No" && record.reasonable_preference == "Yes"
+      record.errors.add :reasonable_preference, "can not be Yes if Not Homeless immediately prior to this letting has been selected"
+    elsif record.reasonable_preference == "Yes"
+      if !record.reasonable_preference_reason_homeless && !record.reasonable_preference_reason_unsatisfactory_housing && !record.reasonable_preference_reason_medical_grounds && !record.reasonable_preference_reason_avoid_hardship && !record.reasonable_preference_reason_do_not_know
+        record.errors.add :reasonable_preference_reason, "- if reasonable preference is Yes, a reason must be given"
+      end
+    elsif record.reasonable_preference == "No"
+      if record.reasonable_preference_reason_homeless || record.reasonable_preference_reason_unsatisfactory_housing || record.reasonable_preference_reason_medical_grounds || record.reasonable_preference_reason_avoid_hardship || record.reasonable_preference_reason_do_not_know
+        record.errors.add :reasonable_preference_reason, "- if reasonable preference is No, no reasons should be given"
+      end
+    end
+  end
+  
+  def validate_other_reason_for_leaving_last_settled_home(record)
+    if record.reason_for_leaving_last_settled_home == "Other" && record.other_reason_for_leaving_last_settled_home.blank?
+      record.errors.add :other_reason_for_leaving_last_settled_home, "If reason for leaving settled home is other then the other reason must be provided"
+    end
+
+    if record.reason_for_leaving_last_settled_home != "Other" && record.other_reason_for_leaving_last_settled_home.present?
+      record.errors.add :other_reason_for_leaving_last_settled_home, "The other reason must not be provided if the reason for leaving settled home was not other"
+    end
+  end
+
   def validate(record)
     # If we've come from the form UI we only want to validate the specific fields
     # that have just been submitted. If we're submitting a log via API or Bulk Upload
     # we want to validate all data fields.
     question_to_validate = options[:previous_page]
-    if question_to_validate && respond_to?("validate_#{question_to_validate}")
+    if question_to_validate 
+      if respond_to?("validate_#{question_to_validate}")
         public_send("validate_#{question_to_validate}", record)
+      end 
     else
       # This assumes that all methods in this class other than this one are
       # validations to be run
@@ -26,29 +58,73 @@ class CaseLogValidator < ActiveModel::Validator
 end
 
 class CaseLog < ApplicationRecord
+  include Discard::Model
+  default_scope -> { kept }
+  scope :in_progress, -> { where(status: "in_progress") }
+  scope :completed, -> { where(status: "completed") }
+
   validate :instance_validations
   before_save :update_status!
 
   attr_writer :previous_page
 
-  enum status: { "in progress" => 0, "submitted" => 1 }
+  enum status: { "not_started" => 0, "in_progress" => 1, "completed" => 2 }
 
-  AUTOGENERATED_FIELDS = %w[status created_at updated_at id].freeze
+  AUTOGENERATED_FIELDS = %w[id status created_at updated_at discarded_at].freeze
 
   def instance_validations
     validates_with CaseLogValidator, ({ previous_page: @previous_page } || {})
   end
 
+  def self.editable_fields
+    attribute_names - AUTOGENERATED_FIELDS
+  end
+
+  def completed?
+    status == "completed"
+  end
+
+  def not_started?
+    status == "not_started"
+  end
+
+  def in_progress?
+    status == "in_progress"
+  end
+
+private
+
   def update_status!
-    self.status = (all_fields_completed? && errors.empty? ? "submitted" : "in progress")
+    self.status = if all_fields_completed? && errors.empty?
+                    "completed"
+                  elsif all_fields_nil?
+                    "not_started"
+                  else
+                    "in_progress"
+                  end
   end
 
   def all_fields_completed?
-    mandatory_fields = attributes.except(*AUTOGENERATED_FIELDS)
     mandatory_fields.none? { |_key, val| val.nil? }
   end
 
-  def self.editable_fields
+  def all_fields_nil?
-    attribute_names - AUTOGENERATED_FIELDS
+    mandatory_fields.all? { |_key, val| val.nil? }
+  end
+
+  def mandatory_fields
+    required = attributes.except(*AUTOGENERATED_FIELDS)
+
+    dynamically_not_required = []
+
+    if reason_for_leaving_last_settled_home != "Other"
+      dynamically_not_required << "other_reason_for_leaving_last_settled_home"
+    end
+
+    if net_income.to_i.zero?
+      dynamically_not_required << "net_income_frequency"
+    end
+
+    required.delete_if { |key, _value| dynamically_not_required.include?(key) }
   end
 end

app/views/case_logs/index.html.erb

@@ -10,10 +10,10 @@
       <%= render partial: "log_list", locals: { case_logs: @in_progress_case_logs, title: "Logs you need to complete", date_title: "Last Changed" } %>
     <% end %>
 
-    <% if @submitted_case_logs.present? %>
+    <% if @completed_case_logs.present? %>
-      <%= render partial: "log_list", locals: { case_logs: @submitted_case_logs, title: "Logs you've submitted", date_title: "Date Submitted" } %>
+      <%= render partial: "log_list", locals: { case_logs: @completed_case_logs, title: "Logs you've submitted", date_title: "Date Submitted" } %>
     <% end %>
 
-    <p><a href="#" class="govuk-link">See all completed logs (<%= @submitted_case_logs.count %>)</a></p>
+    <p><a href="#" class="govuk-link">See all completed logs (<%= @completed_case_logs.count %>)</a></p>
   </div>
 </div>

app/views/form/_check_answers_table.html.erb

@@ -4,10 +4,10 @@
       <%= question_info["check_answer_label"].to_s %>
     <dt>
     <dd class="govuk-summary-list__value">
-      <%= case_log[question_title] %>
+      <%= @case_log[question_title] %>
     </dd>
     <dd class="govuk-summary-list__actions">
-      <%= create_update_answer_link(case_log[question_title], case_log["id"], page)%>
+      <%= create_update_answer_link(@case_log[question_title], @case_log.id, page)%>
     </dd>
   </div>
 </dl>

app/views/form/check_answers.html.erb

@@ -2,11 +2,11 @@
   <div class="govuk-grid-row">
     <div class="govuk-grid-column-two-thirds-from-desktop">
       <h1 class="govuk-heading-l">Check the answers you gave for <%= subsection.humanize(capitalize: false) %></h1>
-      <%= display_answered_questions_summary(subsection, case_log, form) %>
+      <%= display_answered_questions_summary(subsection, @case_log, form) %>
       <% form.pages_for_subsection(subsection).each do |page, page_info| %>
         <% page_info["questions"].each do |question_title, question_info| %>
-          <% if total_questions(subsection, case_log, form).include?(question_title) %>
+          <% if total_questions(subsection, @case_log, form).include?(question_title) %>
-            <%= render partial: 'form/check_answers_table', locals: { question_title: question_title, question_info: question_info, case_log: case_log, page: page } %>
+            <%= render partial: 'form/check_answers_table', locals: { question_title: question_title, question_info: question_info, case_log: @case_log, page: page } %>
           <%end %>
         <%end %>
       <% end %>

config/forms/2021_2022.json

@@ -31,7 +31,7 @@
                   "hint_text": "",
                   "type": "numeric",
                   "min": 0,
-                  "max": 150,
+                  "max": 120,
                   "step": 1
                 }
               }

db/migrate/20211014154616_add_discarded_at_to_case_logs.rb

@@ -0,0 +1,6 @@
+class AddDiscardedAtToCaseLogs < ActiveRecord::Migration[6.1]
+  def change
+    add_column :case_logs, :discarded_at, :datetime
+    add_index :case_logs, :discarded_at
+  end
+end

db/migrate/20211015090040_update_property_number_of_times_relet_type.rb

@@ -0,0 +1,9 @@
+class UpdatePropertyNumberOfTimesReletType < ActiveRecord::Migration[6.1]
+  def up
+    change_column :case_logs, :property_number_of_times_relet, "integer USING CAST(property_number_of_times_relet AS integer)"
+  end
+
+  def down
+    change_column :case_logs, :property_number_of_times_relet, :string
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2021_10_13_113607) do
+ActiveRecord::Schema.define(version: 2021_10_15_090040) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -84,7 +84,6 @@ ActiveRecord::Schema.define(version: 2021_10_13_113607) do
     t.string "property_void_date"
     t.string "property_major_repairs"
     t.string "property_major_repairs_date"
-    t.string "property_number_of_times_relet"
     t.string "property_wheelchair_accessible"
     t.string "net_income"
     t.string "net_income_frequency"
@@ -131,6 +130,9 @@ ActiveRecord::Schema.define(version: 2021_10_13_113607) do
     t.boolean "reasonable_preference_reason_medical_grounds"
     t.boolean "reasonable_preference_reason_avoid_hardship"
     t.boolean "reasonable_preference_reason_do_not_know"
+    t.datetime "discarded_at"
+    t.integer "property_number_of_times_relet"
+    t.index ["discarded_at"], name: "index_case_logs_on_discarded_at"
   end
 
 end

docs/adr/adr-007-data-validations.md

@@ -9,7 +9,7 @@ These are handled slightly differently:
 
 ##### Validity checks
 
-These run for all submitted data. Every time a form page (in the UI is submitted), the fields related to that form page will be checked to ensure that any responses given are valid. If they are not, an error message will be shown on screen, and it will not be possible to "Save and continue" until the response is fixed or removed.
+These run for all submitted data. Every time a form page (in the UI) is submitted, the fields related to that form page will be checked to ensure that any responses given are valid. If they are not, an error message will be shown on screen, and it will not be possible to "Save and continue" until the response is fixed or removed.
 
 Similarly if an API request is made to create a case log with data that contains _invalid_ fields, that data will be rejected, and an error message will be returned.
 
@@ -18,10 +18,10 @@ Similarly if an API request is made to create a case log with data that contains
 
 These are not strictly error checks since it's possible to submit partial data. In the form UI it is possible to click "Save and continue" and move past questions that you might not know right now, and leave them to come back to later. We shouldn't prevent this workflow.
 
-Similar the API client (3rd party software system) may not have all the required data and may only be submitting a partial log. This is still a valid use case so we should not be enforcing presence checks and returning errors based on them for either submission type.
+Similarly the API client (3rd party software system) may not have all the required data and may only be submitting a partial log. This is still a valid use case so we should not be enforcing presence checks and returning errors based on them for either submission type.
 
 Instead we determine the _status_ of the case log based the presence checks. Every time data is submitted (via a form page, bulk upload or API), before saving the data, the system will check whether all fields have been completed *and* pass validity checks. If so, the case log will be marked as *completed*, if not it will be marked as *in progress*.
 
 By default all fields that a Case Log has will be assumed to be required unless explicitly marked as not required (for example as a result of other answers rendering a question inapplicable).
 
-On the form UI this will work by by not allowing you to "submit" the form, until all presence checks have been satisfied, but all other navigation is allowed. On the API this will work by returning a Case Log that is "in progress" if you've submitted a partial log, or "completed" if you've submitted a full log, or "Errors" if you've submitted an invalid log.
+On the form UI this will work by not allowing you to "submit" the form, until all presence checks have been satisfied, but all other navigation is allowed. On the API this will work by returning a Case Log that is "in progress" if you've submitted a partial log, or "completed" if you've submitted a full log, or "Errors" if you've submitted an invalid log.

docs/index.html

@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+<meta charset="UTF-8">
+<meta name="viewport" content="width=device-width, initial-scale=1">
+<link rel="stylesheet" type="text/css" href="//pagecdn.io/lib/swagger-ui/v3.31.1/swagger-ui.css">
+<title>OpenAPI DLUHC CORE Data Collection</title>
+<body><div id="openapi"><script src="//pagecdn.io/lib/swagger-ui/v3.31.1/swagger-ui-bundle.js"></script>
+<script>
+window.onload = function () {
+  const ui = SwaggerUIBundle({
+    url: "api/DLUHC-CORE-Data.v1.json",
+    dom_id: "#openapi"
+  })
+}
+</script>
+</body>

spec/factories/case_log.rb

@@ -2,14 +2,14 @@ FactoryBot.define do
   factory :case_log do
     sequence(:id) { |i| i }
     trait :in_progress do
-      status { 0 }
+      status { 1 }
       tenant_code { "TH356" }
       property_postcode { "SW2 6HI" }
       previous_postcode { "P0 5ST" }
       tenant_age { "12" }
     end
-    trait :submitted do
+    trait :completed do
-      status { 1 }
+      status { 2 }
       tenant_code { "BZ737" }
       property_postcode { "NW1 7TY" }
     end

spec/fixtures/complete_case_log.json

@@ -38,7 +38,7 @@
       "person_8_age": 2,
       "person_8_gender": "Prefer not to say",
       "person_8_economic_status": "Child under 16",
-      "homelessness": "No",
+      "homelessness": "Yes - other homelessness",
       "reason_for_leaving_last_settled_home": "Other problems with neighbours",
       "benefit_cap_spare_room_subsidy": "No",
       "armed_forces_active": "No",
@@ -68,8 +68,8 @@
       "property_major_repairs_date": "05/05/2020",
       "property_number_of_times_relet": 2,
       "property_wheelchair_accessible": true,
-      "net_income": 1000,
+      "net_income": 0,
-      "net_income_frequency": "Monthly",
+      "net_income_frequency": null,
       "net_income_uc_proportion": "Some",
       "housing_benefit": "Universal Credit with housing element, but not Housing Benefit",
       "rent_frequency": "Weekly",
@@ -89,7 +89,7 @@
       "chr_letting": false,
       "cap_letting": false,
       "outstanding_rent_or_charges": 25,
-      "other_reason_for_leaving_last_settled_home": "Other reason",
+      "other_reason_for_leaving_last_settled_home": null,
       "accessibility_requirements_fully_wheelchair_accessible_housing": true,
       "accessibility_requirements_wheelchair_access_to_essential_rooms": false,
       "accessibility_requirements_level_access_housing": false,
@@ -107,7 +107,7 @@
       "condition_effects_mental_health": false,
       "condition_effects_social_or_behavioral": false,
       "condition_effects_other": false,
-      "condition_effects_prefer_not_to_say": false,
+      "condition_effects_prefer_not_to_say": true,
       "reasonable_preference_reason_homeless": false,
       "reasonable_preference_reason_unsatisfactory_housing": false,
       "reasonable_preference_reason_medical_grounds": false,

spec/helpers/tasklist_helper_spec.rb

@@ -1,8 +1,9 @@
 require "rails_helper"
 
 RSpec.describe TasklistHelper do
-  let!(:empty_case_log) { FactoryBot.create(:case_log) }
+  let(:empty_case_log) { FactoryBot.build(:case_log) }
-  let!(:case_log) { FactoryBot.create(:case_log, :in_progress) }
+  let(:case_log) { FactoryBot.build(:case_log, :in_progress) }
+  let(:completed_case_log) { FactoryBot.build(:case_log, :completed) }
   form_handler = FormHandler.instance
   let(:form) { form_handler.get_form("test_form") }
 
@@ -35,7 +36,6 @@ RSpec.describe TasklistHelper do
     end
 
     it "returns not started if the subsection is declaration and all the questions are completed" do
-      completed_case_log = CaseLog.new(case_log.attributes.map { |key, value| Hash[key, value || "value"] }.reduce(:merge))
       status = get_subsection_status("declaration", completed_case_log, declaration_questions)
       expect(status).to eq(:not_started)
     end

spec/models/case_log_spec.rb

@@ -13,5 +13,82 @@ RSpec.describe Form, type: :model do
     it "validates age is over 0" do
       expect { CaseLog.create!(tenant_age: 0) }.to raise_error(ActiveRecord::RecordInvalid)
     end
+
+    it "validates number of relets is a number" do
+      expect { CaseLog.create!(property_number_of_times_relet: "random") }.to raise_error(ActiveRecord::RecordInvalid)
+    end
+
+    it "validates number of relets is under 20" do
+      expect { CaseLog.create!(property_number_of_times_relet: 21) }.to raise_error(ActiveRecord::RecordInvalid)
+    end
+
+    it "validates number of relets is over 0" do
+      expect { CaseLog.create!(property_number_of_times_relet: 0) }.to raise_error(ActiveRecord::RecordInvalid)
+    end
+
+    describe "reasonable preference validation" do
+      it "if given reasonable preference is yes a reason must be selected" do
+        expect { 
+          CaseLog.create!(reasonable_preference: "Yes",
+            reasonable_preference_reason_homeless: nil,
+            reasonable_preference_reason_unsatisfactory_housing: nil,
+            reasonable_preference_reason_medical_grounds: nil,
+            reasonable_preference_reason_avoid_hardship: nil,
+            reasonable_preference_reason_do_not_know: nil
+          ) 
+        }.to raise_error(ActiveRecord::RecordInvalid)
+      end
+
+      it "if not previously homeless reasonable preference should not be selected" do
+        expect { 
+          CaseLog.create!(
+            homelessness: "No",
+            reasonable_preference: "Yes"
+          ) 
+        }.to raise_error(ActiveRecord::RecordInvalid)
+      end
+
+      it "if not given reasonable preference a reason should not be selected" do
+        expect { 
+          CaseLog.create!(
+            homelessness: "Yes",
+            reasonable_preference: "No",
+            reasonable_preference_reason_homeless: true
+          )
+        }.to raise_error(ActiveRecord::RecordInvalid)
+      end
+    end
+    context "other reason for leaving last settled home validation" do
+      it "must be provided if main reason for leaving last settled home was given as other" do
+        expect {
+          CaseLog.create!(reason_for_leaving_last_settled_home: "Other",
+                          other_reason_for_leaving_last_settled_home: nil)
+        }.to raise_error(ActiveRecord::RecordInvalid)
+      end
+
+      it "must not be provided if the main reason for leaving settled home is not other" do
+        expect {
+          CaseLog.create!(reason_for_leaving_last_settled_home: "Repossession",
+                          other_reason_for_leaving_last_settled_home: "the other reason provided")
+        }.to raise_error(ActiveRecord::RecordInvalid)
+      end
+    end
+  end
+
+  describe "status" do
+    let!(:empty_case_log) { FactoryBot.create(:case_log) }
+    let!(:in_progress_case_log) { FactoryBot.create(:case_log, :in_progress) }
+
+    it "is set to not started for an empty case log" do
+      expect(empty_case_log.not_started?).to be(true)
+      expect(empty_case_log.in_progress?).to be(false)
+      expect(empty_case_log.completed?).to be(false)
+    end
+
+    it "is set to not started for an empty case log" do
+      expect(in_progress_case_log.in_progress?).to be(true)
+      expect(in_progress_case_log.not_started?).to be(false)
+      expect(in_progress_case_log.completed?).to be(false)
+    end
   end
 end

spec/requests/case_log_controller_spec.rb

@@ -1,18 +1,12 @@
 require "rails_helper"
 
 RSpec.describe CaseLogsController, type: :request do
-  describe "POST #create" do
-    let(:tenant_code) { "T365" }
-    let(:tenant_age) { 35 }
-    let(:property_postcode) { "SE11 6TY" }
   let(:api_username) { "test_user" }
   let(:api_password) { "test_password" }
   let(:basic_credentials) do
     ActionController::HttpAuthentication::Basic
                         .encode_credentials(api_username, api_password)
   end
-    let(:in_progress) { "in progress" }
-    let(:submitted) { "submitted" }
 
   let(:headers) do
     {
@@ -22,18 +16,30 @@ RSpec.describe CaseLogsController, type: :request do
     }
   end
 
+  before do
+    allow(ENV).to receive(:[])
+    allow(ENV).to receive(:[]).with("API_USER").and_return(api_username)
+    allow(ENV).to receive(:[]).with("API_KEY").and_return(api_password)
+  end
+
+  describe "POST #create" do
+    let(:tenant_code) { "T365" }
+    let(:tenant_age) { 35 }
+    let(:property_number_of_times_relet) { 12 }
+    let(:property_postcode) { "SE11 6TY" }
+    let(:in_progress) { "in_progress" }
+    let(:completed) { "completed" }
+
     let(:params) do
       {
         "tenant_code": tenant_code,
         "tenant_age": tenant_age,
         "property_postcode": property_postcode,
+        "property_number_of_times_relet": property_number_of_times_relet,
       }
     end
 
     before do
-      allow(ENV).to receive(:[])
-      allow(ENV).to receive(:[]).with("API_USER").and_return(api_username)
-      allow(ENV).to receive(:[]).with("API_KEY").and_return(api_password)
       post "/case_logs", headers: headers, params: params.to_json
     end
 
@@ -55,11 +61,12 @@ RSpec.describe CaseLogsController, type: :request do
 
     context "invalid json params" do
       let(:tenant_age) { 2000 }
+      let(:property_number_of_times_relet) { 21 }
 
       it "validates case log parameters" do
         json_response = JSON.parse(response.body)
         expect(response).to have_http_status(:unprocessable_entity)
-        expect(json_response["errors"]).to eq(["Tenant age must be between 0 and 120"])
+        expect(json_response["errors"]).to match_array(["Tenant age must be between 0 and 120", "Property number of times relet must be between 0 and 20"])
       end
     end
 
@@ -75,9 +82,165 @@ RSpec.describe CaseLogsController, type: :request do
         JSON.parse(File.open("spec/fixtures/complete_case_log.json").read)
       end
 
-      it "marks the record as submitted" do
+      it "marks the record as completed" do
+        json_response = JSON.parse(response.body)
+        expect(json_response["status"]).to eq(completed)
+      end
+    end
+
+    context "request with invalid credentials" do
+      let(:basic_credentials) do
+        ActionController::HttpAuthentication::Basic.encode_credentials(api_username, "Oops")
+      end
+
+      it "returns 401" do
+        expect(response).to have_http_status(:unauthorized)
+      end
+    end
+  end
+
+  describe "GET" do
+    let(:case_log) { FactoryBot.create(:case_log, :completed) }
+    let(:id) { case_log.id }
+
+    before do
+      get "/case_logs/#{id}", headers: headers
+    end
+
+    it "returns http success" do
+      expect(response).to have_http_status(:success)
+    end
+
+    it "returns a serialized Case Log" do
       json_response = JSON.parse(response.body)
-        expect(json_response["status"]).to eq(submitted)
+      expect(json_response["status"]).to eq(case_log.status)
+    end
+  end
+
+  describe "PATCH" do
+    let(:case_log) do
+      FactoryBot.create(:case_log, :in_progress, tenant_code: "Old Value", property_postcode: "Old Value")
+    end
+    let(:params) do
+      { tenant_code: "New Value" }
+    end
+    let(:id) { case_log.id }
+
+    before do
+      patch "/case_logs/#{id}", headers: headers, params: params.to_json
+    end
+
+    it "returns http success" do
+      expect(response).to have_http_status(:success)
+    end
+
+    it "updates the case log with the given fields and keeps original values where none are passed" do
+      case_log.reload
+      expect(case_log.tenant_code).to eq("New Value")
+      expect(case_log.property_postcode).to eq("Old Value")
+    end
+
+    context "invalid case log id" do
+      let(:id) { (CaseLog.order(:id).last&.id || 0) + 1 }
+
+      it "returns 404" do
+        expect(response).to have_http_status(:not_found)
+      end
+    end
+
+    context "invalid case log params" do
+      let(:params) { { tenant_age: 200 } }
+
+      it "returns 422" do
+        expect(response).to have_http_status(:unprocessable_entity)
+      end
+
+      it "returns an error message" do
+        json_response = JSON.parse(response.body)
+        expect(json_response["errors"]).to eq(["Tenant age must be between 0 and 120"])
+      end
+    end
+
+    context "request with invalid credentials" do
+      let(:basic_credentials) do
+        ActionController::HttpAuthentication::Basic.encode_credentials(api_username, "Oops")
+      end
+
+      it "returns 401" do
+        expect(response).to have_http_status(:unauthorized)
+      end
+    end
+  end
+
+  # We don't really have any meaningful distinction between PUT and PATCH here since you can update some or all
+  # fields in both cases, and both route to #Update. Rails generally recommends PATCH as it more closely matches
+  # what actually happens to an ActiveRecord object and what we're doing here, but either is allowed.
+  describe "PUT" do
+    let(:case_log) do
+      FactoryBot.create(:case_log, :in_progress, tenant_code: "Old Value", property_postcode: "Old Value")
+    end
+    let(:params) do
+      { tenant_code: "New Value" }
+    end
+    let(:id) { case_log.id }
+
+    before do
+      put "/case_logs/#{id}", headers: headers, params: params.to_json
+    end
+
+    it "returns http success" do
+      expect(response).to have_http_status(:success)
+    end
+
+    it "updates the case log with the given fields and keeps original values where none are passed" do
+      case_log.reload
+      expect(case_log.tenant_code).to eq("New Value")
+      expect(case_log.property_postcode).to eq("Old Value")
+    end
+
+    context "invalid case log id" do
+      let(:id) { (CaseLog.order(:id).last&.id || 0) + 1 }
+
+      it "returns 404" do
+        expect(response).to have_http_status(:not_found)
+      end
+    end
+
+    context "request with invalid credentials" do
+      let(:basic_credentials) do
+        ActionController::HttpAuthentication::Basic.encode_credentials(api_username, "Oops")
+      end
+
+      it "returns 401" do
+        expect(response).to have_http_status(:unauthorized)
+      end
+    end
+  end
+
+  describe "DELETE" do
+    let!(:case_log) do
+      FactoryBot.create(:case_log, :in_progress)
+    end
+    let(:id) { case_log.id }
+
+    before do
+      delete "/case_logs/#{id}", headers: headers
+    end
+
+    it "returns http success" do
+      expect(response).to have_http_status(:success)
+    end
+
+    it "soft deletes the case log" do
+      expect { CaseLog.find(id) }.to raise_error(ActiveRecord::RecordNotFound)
+      expect(CaseLog.with_discarded.find(id)).to be_a(CaseLog)
+    end
+
+    context "invalid case log id" do
+      let(:id) { (CaseLog.order(:id).last&.id || 0) + 1 }
+
+      it "returns 404" do
+        expect(response).to have_http_status(:not_found)
       end
     end
 

spec/views/case_log_index_view_spec.rb

@@ -1,12 +1,12 @@
 require "rails_helper"
 RSpec.describe "case_logs/index" do
   let(:in_progress_log) { FactoryBot.build(:case_log, :in_progress) }
-  let(:submitted_log) { FactoryBot.build(:case_log, :submitted) }
+  let(:completed_log) { FactoryBot.build(:case_log, :completed) }
 
   context "given an in progress log list" do
     it "renders a table for in progress logs only" do
       assign(:in_progress_case_logs, [in_progress_log])
-      assign(:submitted_case_logs, [])
+      assign(:completed_case_logs, [])
       render
       expect(rendered).to match(/<table class="govuk-table">/)
       expect(rendered).to match(/Logs you need to complete/)
@@ -16,23 +16,23 @@ RSpec.describe "case_logs/index" do
     end
   end
 
-  context "given a submitted log list" do
+  context "given a completed log list" do
     it "renders a table for in progress logs only" do
       assign(:in_progress_case_logs, [])
-      assign(:submitted_case_logs, [submitted_log])
+      assign(:completed_case_logs, [completed_log])
       render
       expect(rendered).to match(/<table class="govuk-table">/)
       expect(rendered).to match(/Logs you&#39;ve submitted/)
       expect(rendered).not_to match(/Logs you need to complete/)
-      expect(rendered).to match(submitted_log.tenant_code)
+      expect(rendered).to match(completed_log.tenant_code)
-      expect(rendered).to match(submitted_log.property_postcode)
+      expect(rendered).to match(completed_log.property_postcode)
     end
   end
 
-  context "given a submitted log list and an in_progress log list" do
+  context "given a completed log list and an in_progress log list" do
     it "renders two tables, one for each status" do
       assign(:in_progress_case_logs, [in_progress_log])
-      assign(:submitted_case_logs, [submitted_log])
+      assign(:completed_case_logs, [completed_log])
       render
       expect(rendered).to match(/<table class="govuk-table">/)
       expect(rendered).to match(/Logs you&#39;ve submitted/)