diff --git a/Gemfile b/Gemfile index 87593b00..42b81940 100644 --- a/Gemfile +++ b/Gemfile @@ -29,6 +29,8 @@ gem "terser" gem "rswag-api" gem "rswag-ui" gem "seed-fu" +gem "devise", "~> 4.9" +gem "rails-controller-testing" group :development do gem "better_errors" @@ -60,5 +62,3 @@ group :test do gem "climate_control" gem "timecop" end - -gem "devise", "~> 4.9" diff --git a/Gemfile.lock b/Gemfile.lock index b4dc3ed4..fbcbe5fb 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -280,6 +280,10 @@ GEM activesupport (= 7.2.2.1) bundler (>= 1.15.0) railties (= 7.2.2.1) + rails-controller-testing (1.0.5) + actionpack (>= 5.0.1.rc1) + actionview (>= 5.0.1.rc1) + activesupport (>= 5.0.1.rc1) rails-dom-testing (2.2.0) activesupport (>= 5.0.0) minitest @@ -497,6 +501,7 @@ DEPENDENCIES pry-rails puma (~> 6.0) rails (~> 7.2) + rails-controller-testing rails_layout rollbar rspec-rails diff --git a/app/controllers/officer/decisions_controller.rb b/app/controllers/officer/decisions_controller.rb new file mode 100644 index 00000000..c9575181 --- /dev/null +++ b/app/controllers/officer/decisions_controller.rb @@ -0,0 +1,21 @@ +class Officer::DecisionsController < ApplicationController + def new + @application_decision = AssessmentDecisionForm.new(application_decision: nil) + end + + def create + application = LandingApplication.find(params[:landing_application_id]) + @application_decision = AssessmentDecisionForm.new(application_decision: params.dig(:assessment_decision_form, "application_decision")) + + application.application_decision = @application_decision.application_decision + + if @application_decision.valid? + application.application_decision_made_at = Time.current + application.assessor = current_user + application.save + redirect_to(officer_landing_applications_path) + else + render :new + end + end +end diff --git a/app/forms/assessment_decision_form.rb b/app/forms/assessment_decision_form.rb new file mode 100644 index 00000000..add34e54 --- /dev/null +++ b/app/forms/assessment_decision_form.rb @@ -0,0 +1,22 @@ +class AssessmentDecisionForm + include ActiveModel::Model + + attr_accessor :application_decision, :application_decision_made_at + + def initialize(application_decision:) + @application_decision = application_decision + end + + validates :application_decision, + presence: { + message: "Select either 'Approve' or 'Deny'" + } + + validate :ensure_decision_valid + + def ensure_decision_valid + unless LandingApplication.application_decisions.key?(application_decision) + errors.add(:application_decision, "Select either 'Approve' or 'Deny'") + end + end +end diff --git a/app/models/landing_application.rb b/app/models/landing_application.rb index 7ddbebe6..4648df61 100644 --- a/app/models/landing_application.rb +++ b/app/models/landing_application.rb @@ -1,3 +1,7 @@ class LandingApplication < ApplicationRecord belongs_to :destination, foreign_key: :destination_id, class_name: "LandableBody" + + belongs_to :assessor, foreign_key: :assessor_id, class_name: "User", optional: true + + enum :application_decision, [:approved, :denied], validate: {allow_nil: true} end diff --git a/app/models/user.rb b/app/models/user.rb index 092233cc..0f206fb6 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,3 +1,5 @@ class User < ApplicationRecord devise :database_authenticatable, :recoverable + + has_many :assessed_applications, class_name: :LandingApplication, foreign_key: :assessor_id end diff --git a/app/views/officer/decisions/new.html.erb b/app/views/officer/decisions/new.html.erb new file mode 100644 index 00000000..660385e9 --- /dev/null +++ b/app/views/officer/decisions/new.html.erb @@ -0,0 +1,17 @@ +
+
+

Assess a landing application

+ + <%= form_for @application_decision, method: :post, url: officer_landing_application_decisions_path do |form| -%> + <%= form.govuk_error_summary %> + + <%= form.govuk_radio_buttons_fieldset :application_decision, legend: { text: 'Do you approve this application for landing?' } do %> + <%= form.govuk_radio_button :application_decision, 'approved', label: { text: 'Approve' }, link_errors: true %> + <%= form.govuk_radio_button :application_decision, 'denied', label: { text: 'Deny' } %> + <% end %> + + <%= form.govuk_submit "Save and continue"%> + <% end %> +
+
+ \ No newline at end of file diff --git a/app/views/officer/landing_applications/index.html.erb b/app/views/officer/landing_applications/index.html.erb index b3b9a5de..c68300a9 100644 --- a/app/views/officer/landing_applications/index.html.erb +++ b/app/views/officer/landing_applications/index.html.erb @@ -1,7 +1,6 @@

Landing applications

- <%= govuk_table do |table| table.with_caption(size: 'm', text: 'Applications to assess') @@ -26,7 +25,7 @@ row.with_cell(text: application.landing_date) row.with_cell(text: application.departure_date) row.with_cell(text: application.application_submitted_at.to_date, html_attributes: { id: "submission-date" }) - row.with_cell(text: '') + application.application_decision ? row.with_cell(text: application.application_decision.capitalize, html_attributes: { id: "application-#{application.id}" }) : row.with_cell {govuk_button_link_to 'Make a decision', new_officer_landing_application_decision_path(landing_application_id: application.id), secondary: true, class: "govuk-!-margin-bottom-0", html_attributes: { id: "application-#{application.id}" }} end end end @@ -34,6 +33,5 @@ end %> -
diff --git a/config/routes.rb b/config/routes.rb index bc8a00f7..5fef59fd 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -27,7 +27,9 @@ end namespace :officer do - get :"landing-applications", to: "landing_applications#index" + resources :landing_applications, path: "landing-applications", only: :index do + resources :decisions, only: [:new, :create] + end end namespace :api do diff --git a/db/fixtures/development/02_landing_applications.rb b/db/fixtures/development/02_landing_applications.rb index e45e3b8e..64d6336e 100644 --- a/db/fixtures/development/02_landing_applications.rb +++ b/db/fixtures/development/02_landing_applications.rb @@ -16,13 +16,25 @@ def seed landing_date = landing_date(temporality: seed.fetch(:temporality)) s.landing_date = landing_date s.departure_date = up_to_twelve_days_ahead(landing_date) - s.application_submitted_at = upto_forty_five_days_before(landing_date) + + application_submitted_at = upto_forty_five_days_before(landing_date) + s.application_submitted_at = application_submitted_at + + application_decision = [:approved, :denied, nil].sample + s.application_decision = application_decision + if [:approved, :denied].include?(application_decision) + s.application_decision_made_at = between_submission_and_landing_dates(landing_date, application_submitted_at) + end s.application_reference = ApplicationReferenceGenerator.generate end end end + def between_submission_and_landing_dates(landing_date, submission_date) + rand(submission_date..landing_date) + end + def up_to_twelve_days_ahead(landing_date) landing_date + rand(1..12).days end diff --git a/db/migrate/20240909125011_add_user_association_to_landing_application.rb b/db/migrate/20240909125011_add_user_association_to_landing_application.rb new file mode 100644 index 00000000..d4c34604 --- /dev/null +++ b/db/migrate/20240909125011_add_user_association_to_landing_application.rb @@ -0,0 +1,5 @@ +class AddUserAssociationToLandingApplication < ActiveRecord::Migration[7.2] + def change + add_reference :landing_applications, :assessor, foreign_key: {to_table: :users}, type: :uuid + end +end diff --git a/db/migrate/20240913140449_add_decision_enum_to_landing_application.rb b/db/migrate/20240913140449_add_decision_enum_to_landing_application.rb new file mode 100644 index 00000000..4f1de0f6 --- /dev/null +++ b/db/migrate/20240913140449_add_decision_enum_to_landing_application.rb @@ -0,0 +1,5 @@ +class AddDecisionEnumToLandingApplication < ActiveRecord::Migration[7.2] + def change + change_column :landing_applications, :application_decision, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index 443a29d6..dd95187c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -39,11 +39,13 @@ t.date "departure_date", null: false t.string "application_reference", null: false t.datetime "application_submitted_at", null: false - t.string "application_decision" t.datetime "application_decision_made_at" t.string "permit_id" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.uuid "assessor_id" + t.integer "application_decision" + t.index ["assessor_id"], name: "index_landing_applications_on_assessor_id" t.index ["destination_id"], name: "index_landing_applications_on_destination_id" end @@ -60,4 +62,5 @@ end add_foreign_key "landing_applications", "landable_bodies", column: "destination_id" + add_foreign_key "landing_applications", "users", column: "assessor_id" end diff --git a/spec/controllers/officer/decisions_controller_spec.rb b/spec/controllers/officer/decisions_controller_spec.rb new file mode 100644 index 00000000..8641f254 --- /dev/null +++ b/spec/controllers/officer/decisions_controller_spec.rb @@ -0,0 +1,64 @@ +require "rails_helper" + +RSpec.describe Officer::DecisionsController do + let(:officer) do + FactoryBot.create(:user) + end + + before { sign_in officer } + + describe "GET to :new" do + context "when officer is signed in" + it "calls AssessmentDecisionForm.new" do + allow(AssessmentDecisionForm).to receive(:new) + + get :new, params: {landing_application_id: "abc123"} + + expect(AssessmentDecisionForm).to have_received(:new).with(application_decision: nil) + end + end + + describe "POST to :create" do + context "when a valid decision is submitted" + it "updates the application and redirects to the landing applications index page" do + form = instance_double(AssessmentDecisionForm, "valid?" => true, "application_decision" => "denied") + allow(AssessmentDecisionForm).to receive(:new).and_return(form) + + landing_application = instance_double(LandingApplication, "valid?" => true, :save => double, "application_decision=" => "denied", "application_decision_made_at=" => Time.current, "assessor=" => officer) + allow(LandingApplication).to receive(:find).and_return(landing_application) + + post :create, params: {landing_application_id: "abc123", assessment_decision_form: {application_decision: "denied"}} + + expect(landing_application).to have_received(:save) + expect(response).to redirect_to(officer_landing_applications_path) + end + + context "when no decision is submitted" + it "doesn't save the application and renders :new" do + form = instance_double(AssessmentDecisionForm, "valid?" => false, "application_decision" => nil) + allow(AssessmentDecisionForm).to receive(:new).and_return(form) + + landing_application = instance_double(LandingApplication, "valid?" => false, :save => double, "application_decision=" => nil) + allow(LandingApplication).to receive(:find).and_return(landing_application) + + post :create, params: {landing_application_id: "abc123", assessment_decision_form: {application_decision: nil}} + + expect(landing_application).not_to have_received(:save) + expect(response).to render_template("new") + end + + context "when a non-enum value is submitted" + it "doesn't save the application and renders :new" do + form = instance_double(AssessmentDecisionForm, "valid?" => false, "application_decision" => "proved") + allow(AssessmentDecisionForm).to receive(:new).and_return(form) + + landing_application = instance_double(LandingApplication, :save => double, "application_decision=" => "proved") + allow(LandingApplication).to receive(:find).and_return(landing_application) + + post :create, params: {landing_application_id: "abc123", assessment_decision_form: {application_decision: "proved"}} + + expect(landing_application).not_to have_received(:save) + expect(response).to render_template("new") + end + end +end diff --git a/spec/factories/landing_applications.rb b/spec/factories/landing_applications.rb index b141caca..14aafc0e 100644 --- a/spec/factories/landing_applications.rb +++ b/spec/factories/landing_applications.rb @@ -1,6 +1,7 @@ FactoryBot.define do factory :landing_application do association(:destination, factory: :landable_body) + association(:assessor, factory: :user) pilot_email { "alan@nasa.org.uk" } pilot_name { "Alan Oliver" } pilot_licence_id { "1233ABC00123" } diff --git a/spec/features/officer/assess_landing_application_spec.rb b/spec/features/officer/assess_landing_application_spec.rb new file mode 100644 index 00000000..219fb6de --- /dev/null +++ b/spec/features/officer/assess_landing_application_spec.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +# Feature: Officer assesses a landing application +# So that a landing permit can be issued if appropriate +# As an officer assessing a landing application +# I want to record my decision (accepted or rejected) + +RSpec.feature "Officer assesses a landing application" do + # Scenario: Officer views landing application decision page + # Given there are existing applications in the database + # When I select make a decision on an application on the application list page + # Then I should see the application decision page + + before do + create_applications + @first_application_id = LandingApplication.first.id + sign_in FactoryBot.create(:user) + end + + scenario "Officer views landing application decision page" do + visit officer_landing_applications_path + when_i_select_make_a_decision + then_i_should_see_the_decision_page + end + + # Scenario: Officer assesses a landing application + # Given I am on the application decision page + # When I complete the form + # Then I should be redirected back to the list of application + # And I should see my decision + + scenario "Officer assesses a landing application" do + visit new_officer_landing_application_decision_path(landing_application_id: @first_application_id) + when_i_complete_the_form + then_i_should_be_redirected_to_application_list_page + and_i_should_see_my_decision + end + + # Scenario: Officer completes form incorrectly + # Given I am on the application decision page + # When I fail to enter a decision on the form + # Then I should see an error message + + scenario "Officer completes form incorrectly" do + visit new_officer_landing_application_decision_path(landing_application_id: @first_application_id) + when_i_fail_to_enter_a_decision_on_the_form + then_i_should_see_an_error_message + end + + def then_i_should_see_an_error_message + expect(page).to have_content("Select either 'Approve' or 'Deny'") + end + + def when_i_fail_to_enter_a_decision_on_the_form + click_button("Save and continue") + end + + def and_i_should_see_my_decision + page.find_by_id("application-#{@first_application_id}", text: "Approved") + end + + def then_i_should_be_redirected_to_application_list_page + expect(current_path).to eq("/officer/landing-applications") + end + + def when_i_complete_the_form + page.choose("Approve") + click_button("Save and continue") + end + + def when_i_select_make_a_decision + click_link("Make a decision", match: :first) + end + + def then_i_should_see_the_decision_page + expect(page).to have_content("Do you approve this application for landing?") + end +end diff --git a/spec/features/officer/view_landing_applications_spec.rb b/spec/features/officer/view_landing_applications_spec.rb index d6bb1efa..7f47de64 100644 --- a/spec/features/officer/view_landing_applications_spec.rb +++ b/spec/features/officer/view_landing_applications_spec.rb @@ -42,10 +42,4 @@ def should_see_list_of_applications expect(page).to have_content(landing_application.departure_date) end end - - def create_applications - FactoryBot.create(:landing_application, pilot_name: "Jane", application_submitted_at: Time.new(2022)) - FactoryBot.create(:landing_application, pilot_name: "Fred", application_submitted_at: Time.new(2023)) - FactoryBot.create(:landing_application, pilot_name: "Sam", application_submitted_at: Time.new(2024)) - end end diff --git a/spec/forms/assessment_decision_form_spec.rb b/spec/forms/assessment_decision_form_spec.rb new file mode 100644 index 00000000..97a08186 --- /dev/null +++ b/spec/forms/assessment_decision_form_spec.rb @@ -0,0 +1,36 @@ +RSpec.describe AssessmentDecisionForm do + describe "it validates presence of application_decision" do + context "when application_decision is missing" do + let(:form) { AssessmentDecisionForm.new(application_decision: nil) } + + it "should flag an error" do + form.valid? + + aggregate_failures do + expect(form.errors).to include(:application_decision) + expect(form.errors.full_messages.join).to match("Select either 'Approve' or 'Deny'") + end + end + end + + context "when application_decision is present" do + let(:form) { AssessmentDecisionForm.new(application_decision: "approved") } + + it "should NOT flag an error" do + form.valid? + + expect(form.errors).to_not include(:application_decision) + end + end + + context "when application_decision is present with an incorrect value" do + let(:form) { AssessmentDecisionForm.new(application_decision: "something else") } + + it "should flag an error" do + form.valid? + + expect(form.errors).to include(:application_decision) + end + end + end +end diff --git a/spec/models/landing_application_spec.rb b/spec/models/landing_application_spec.rb new file mode 100644 index 00000000..cf9bfa43 --- /dev/null +++ b/spec/models/landing_application_spec.rb @@ -0,0 +1,13 @@ +RSpec.describe LandingApplication do + describe "belongs_to *assessor* association" do + let(:assessor) { FactoryBot.create(:user) } + + it "has an #assessor association" do + landing_application = FactoryBot.create(:landing_application, assessor: assessor) + + landing_application.assessor.reload + + expect(landing_application.assessor.id).to eq(assessor.id) + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb new file mode 100644 index 00000000..991c44ab --- /dev/null +++ b/spec/models/user_spec.rb @@ -0,0 +1,17 @@ +RSpec.describe User do + describe "has_many *assessed_applications* association" do + it "has a #assessed_application association" do + user = FactoryBot.create(:user) + + application = FactoryBot.create(:landing_application, assessor: user) + + user.assessed_applications = [application] + + user.save! + + user.reload + + expect(user.assessed_applications.first).to eq(application) + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 92cf1f49..3a7d68f7 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -101,3 +101,9 @@ # as the one that triggered the failure. Kernel.srand config.seed end + +def create_applications + FactoryBot.create(:landing_application, pilot_name: "Jane", application_submitted_at: Time.new(2022), assessor_id: nil) + FactoryBot.create(:landing_application, pilot_name: "Fred", application_submitted_at: Time.new(2023), assessor_id: nil) + FactoryBot.create(:landing_application, pilot_name: "Sam", application_submitted_at: Time.new(2024), assessor_id: nil) +end