Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Officer records assessment decision #264

Merged
merged 11 commits into from
Dec 31, 2024
Merged
4 changes: 2 additions & 2 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -60,5 +62,3 @@ group :test do
gem "climate_control"
gem "timecop"
end

gem "devise", "~> 4.9"
5 changes: 5 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -497,6 +501,7 @@ DEPENDENCIES
pry-rails
puma (~> 6.0)
rails (~> 7.2)
rails-controller-testing
rails_layout
rollbar
rspec-rails
Expand Down
21 changes: 21 additions & 0 deletions app/controllers/officer/decisions_controller.rb
Original file line number Diff line number Diff line change
@@ -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
22 changes: 22 additions & 0 deletions app/forms/assessment_decision_form.rb
Original file line number Diff line number Diff line change
@@ -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
4 changes: 4 additions & 0 deletions app/models/landing_application.rb
Original file line number Diff line number Diff line change
@@ -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
2 changes: 2 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
class User < ApplicationRecord
devise :database_authenticatable, :recoverable

has_many :assessed_applications, class_name: :LandingApplication, foreign_key: :assessor_id
end
17 changes: 17 additions & 0 deletions app/views/officer/decisions/new.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<main class="govuk-main-wrapper">
<section class="container">
<h1 class='govuk-heading-l'>Assess a landing application</h1>

<%= 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 %>
</section>
</main>

4 changes: 1 addition & 3 deletions app/views/officer/landing_applications/index.html.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
<main class="govuk-main-wrapper">
<section class="container">
<h1 class='govuk-heading-l'>Landing applications</h1>

<%= govuk_table do |table|
table.with_caption(size: 'm', text: 'Applications to assess')

Expand All @@ -26,14 +25,13 @@
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

end

%>

</section>
</main>
4 changes: 3 additions & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 13 additions & 1 deletion db/fixtures/development/02_landing_applications.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for making this seeding realistic

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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddDecisionEnumToLandingApplication < ActiveRecord::Migration[7.2]
def change
change_column :landing_applications, :application_decision, :integer
end
end
5 changes: 4 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
64 changes: 64 additions & 0 deletions spec/controllers/officer/decisions_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions spec/factories/landing_applications.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
FactoryBot.define do
factory :landing_application do
association(:destination, factory: :landable_body)
association(:assessor, factory: :user)
pilot_email { "[email protected]" }
pilot_name { "Alan Oliver" }
pilot_licence_id { "1233ABC00123" }
Expand Down
78 changes: 78 additions & 0 deletions spec/features/officer/assess_landing_application_spec.rb
Original file line number Diff line number Diff line change
@@ -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")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you think we ought to have a "flash message" of the "success" variety to reassure the assessor that the decision has been recorded?

I also find it doubly reassuring in situations where I'm redirected to an index page to have a confirmation of which item it is I've just operated on, e.g.

Application ID ABC123 has been rejected/accepted

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
6 changes: 0 additions & 6 deletions spec/features/officer/view_landing_applications_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading
Loading