Skip to content

Commit

Permalink
Merge pull request #1662 from DFE-Digital/fix-cannot-delete-subject
Browse files Browse the repository at this point in the history
  • Loading branch information
thomasleese authored Sep 7, 2023
2 parents d6c03e4 + 34779f0 commit d83f1d0
Show file tree
Hide file tree
Showing 11 changed files with 47 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,11 @@ def confirm_age_range_subjects_form_params
:age_range_max,
:age_range_note,
:subject_1,
:subject_1_raw,
:subject_2,
:subject_2_raw,
:subject_3,
:subject_3_raw,
:subjects_note,
)
end
Expand Down
3 changes: 3 additions & 0 deletions app/forms/assessor_interface/check_age_range_subjects_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,11 @@ def permittable_parameters
age_range_max
age_range_note
subject_1
subject_1_raw
subject_2
subject_2_raw
subject_3
subject_3_raw
subjects_note
]
[args, kwargs]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,15 @@ module AssessorInterface::AgeRangeSubjectsForm
}

attribute :subject_1, :string
attribute :subject_1_raw, :string
attribute :subject_2, :string
attribute :subject_2_raw, :string
attribute :subject_3, :string
attribute :subject_3_raw, :string
attribute :subjects_note, :string

validates :subject_1, presence: true
validates :subject_1_raw, presence: true
end

def save
Expand All @@ -52,7 +56,11 @@ def update_age_range
end

def update_subjects
subjects = [subject_1, subject_2, subject_3].compact_blank
subjects = [
subject_1_raw.present? ? subject_1 : "",
subject_2_raw.present? ? subject_2 : "",
subject_3_raw.present? ? subject_3 : "",
].compact_blank
note = subjects_note.presence || ""
assessment.update!(subjects:, subjects_note: note)
end
Expand Down
2 changes: 1 addition & 1 deletion app/javascript/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,6 @@ var loadCountryAutoComplete = () => {
};

loadCountryAutoComplete();
dfeAutocomplete({});
dfeAutocomplete({ rawAttribute: true });

checkboxSearchFilter("app-applications-filters-assessor", "Search assessors");
10 changes: 5 additions & 5 deletions app/lib/dqt/subject.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@

class DQT::Subject
class << self
def for_id(id)
def for(value)
# these three subjects are not coded by HESA so we've agreed these encodings with the DQT team
return "999001" if id == "citizenship"
return "999002" if id == "physical_education"
return "999003" if id == "design_and_technology"
return "999001" if value == "citizenship"
return "999002" if value == "physical_education"
return "999003" if value == "design_and_technology"

MAPPING.invert.fetch(id)
MAPPING.invert.fetch(value)
end

# https://www.hesa.ac.uk/collection/c22053/xml/c22053/c22053codelists.xsd
Expand Down
2 changes: 1 addition & 1 deletion app/lib/dqt/trn_request_params.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def qualification_params
end

def subjects
assessment.subjects.map { |id| Subject.for_id(id) }
assessment.subjects.map { |value| Subject.for(value) }
end

def teaching_qualification
Expand Down
14 changes: 7 additions & 7 deletions app/models/subject.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,24 @@
class Subject
include ActiveModel::Model

attr_accessor :id, :name
attr_accessor :value, :name

class << self
def all
@all = find(ALL_IDS)
@all = find(ALL_VALUES)
end

def find(ids)
(ALL_IDS & ids).map { |id| create(id:) }
def find(values)
(ALL_VALUES & values).map { |value| create(value:) }
end

private

def create(id:)
Subject.new(id:, name: I18n.t("subjects.#{id}"))
def create(value:)
Subject.new(value:, name: I18n.t("subjects.#{value}"))
end

ALL_IDS = %w[
ALL_VALUES = %w[
ancient_hebrew
applied_biology
applied_chemistry
Expand Down
6 changes: 3 additions & 3 deletions app/views/shared/_age_range_subjects_form_fields.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@
form_field: f.govuk_select(
field,
options_for_select(
Subject.all.map { |subject| [subject.name, subject.id] }.unshift([nil, nil]),
dfe_autocomplete_options(Subject.all),
f.object.send(field),
),
)
),
)
) %>
<% end %>

Expand Down
10 changes: 5 additions & 5 deletions spec/lib/dqt/subject_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
require "rails_helper"

RSpec.describe DQT::Subject do
describe "#for_id" do
subject(:dqt_code) { described_class.for_id(id) }
describe "#for" do
subject(:dqt_code) { described_class.for(value) }

shared_examples "DQT code" do |id|
let(:id) { id }
shared_examples "DQT code" do |value|
let(:value) { value }
it { is_expected.to_not be_nil }
end

Subject.all.map { |subject| it_behaves_like "DQT code", subject.id }
Subject.all.map { |subject| it_behaves_like "DQT code", subject.value }
end
end
4 changes: 2 additions & 2 deletions spec/models/subject_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@

it "contains the right elements" do
expect(find.count).to eq(2)
expect(find.first.id).to eq("ancient_hebrew")
expect(find.second.id).to eq("economics")
expect(find.first.value).to eq("ancient_hebrew")
expect(find.second.value).to eq("economics")
end
end
end
9 changes: 8 additions & 1 deletion spec/support/shared_examples/age_range_subjects_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
it { is_expected.to_not validate_presence_of(:age_range_note) }

it { is_expected.to validate_presence_of(:subject_1) }
it { is_expected.to validate_presence_of(:subject_1_raw) }
it { is_expected.to_not validate_presence_of(:subject_2) }
it { is_expected.to_not validate_presence_of(:subject_3) }
it { is_expected.to_not validate_presence_of(:subjects_note) }
Expand All @@ -48,7 +49,12 @@

describe "with valid attributes and no note" do
let(:age_range_subjects_attributes) do
{ age_range_min: "7", age_range_max: "11", subject_1: "Subject" }
{
age_range_min: "7",
age_range_max: "11",
subject_1: "Subject",
subject_1_raw: "Subject",
}
end

it { is_expected.to be true }
Expand Down Expand Up @@ -79,6 +85,7 @@
age_range_max: "11",
age_range_note: "A note.",
subject_1: "Subject",
subject_1_raw: "Subject",
subjects_note: "Another note.",
}
end
Expand Down

0 comments on commit d83f1d0

Please sign in to comment.