Skip to content

Commit

Permalink
Changes made according to comments
Browse files Browse the repository at this point in the history
  • Loading branch information
syed87 committed Sep 26, 2023
1 parent dad5add commit fd98610
Show file tree
Hide file tree
Showing 21 changed files with 49 additions and 106 deletions.
Binary file added .DS_Store
Binary file not shown.
2 changes: 1 addition & 1 deletion app/controllers/support_interface/countries_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ def edit
@form =
CountryForm.new(
country:,
subject_limited: country.subject_limited,
eligibility_enabled: country.eligibility_enabled,
eligibility_skip_questions: country.eligibility_skip_questions,
has_regions: country.regions.count > 1,
Expand Down Expand Up @@ -58,7 +59,6 @@ def country
def country_params
params.require(:support_interface_country_form).permit(
:eligibility_enabled,
:eligibility_skip_questions,
:has_regions,
:other_information,
:qualifications_information,
Expand Down
9 changes: 5 additions & 4 deletions app/forms/support_interface/country_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,11 @@ def save!
end

def assign_country_attributes
country.assign_attributes(eligibility_enabled:, eligibility_skip_questions:, subject_limited:)
country.assign_attributes(
eligibility_enabled:,
eligibility_skip_questions:,
subject_limited:,
)

if has_regions
country.assign_attributes(
Expand Down Expand Up @@ -95,9 +99,6 @@ def eligibility_route
end

def eligibility_route=(value)
subject_limited_will_change!
eligibility_skip_questions_will_change!

case value
when "standard"
self.subject_limited = false
Expand Down
5 changes: 0 additions & 5 deletions app/lib/country_code.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,6 @@ def european_economic_area?(code)
Country::CODES_IN_EUROPEAN_ECONOMIC_AREA.include?(code)
end

def secondary_education_teaching_qualification_required?(code)
country = Country.find_by(code:)
country&.subject_limited || false
end

LOCATIONS_BY_COUNTRY_CODE =
Country::LOCATION_AUTOCOMPLETE_CANONICAL_LIST
.map { |row| [CountryCode.from_location(row.last), row.last] }
Expand Down
4 changes: 1 addition & 3 deletions app/models/application_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -227,9 +227,7 @@ def english_language_exempt?
end

def secondary_education_teaching_qualification_required?
CountryCode.secondary_education_teaching_qualification_required?(
country.code,
)
country.subject_limited
end

def created_under_new_regulations?
Expand Down
29 changes: 1 addition & 28 deletions app/models/country.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
# qualifications_information :text default(""), not null
# sanction_information :string default(""), not null
# status_information :string default(""), not null
# subject_limited :boolean default(FALSE)
# subject_limited :boolean default(FALSE), not null
# created_at :datetime not null
# updated_at :datetime not null
#
Expand All @@ -33,31 +33,4 @@ class Country < ApplicationRecord
YAML.load(File.read("lib/countries-in-european-economic-area.yaml"))

validates :code, inclusion: { in: CODES }

attribute :subject_limited, :boolean

def eligibility_route
if subject_limited
"expanded"
elsif eligibility_skip_questions
"reduced"
else
"standard"
end
end

def eligibility_route=(value)
subject_limited_will_change!
eligibility_skip_questions_will_change!
if value == "standard"
self.subject_limited = false
self.eligibility_skip_questions = false
elsif value == "reduced"
self.subject_limited = false
self.eligibility_skip_questions = true
elsif value == "expanded"
self.subject_limited = true
self.eligibility_skip_questions = false
end
end
end
4 changes: 1 addition & 3 deletions app/models/eligibility_check.rb
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,7 @@ def status
def qualified_for_subject_required?
return false if country_code.blank?

CountryCode.secondary_education_teaching_qualification_required?(
country_code,
)
Country.exists?(code: country_code, subject_limited: true)
end

private
Expand Down
5 changes: 0 additions & 5 deletions app/views/support_interface/countries/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,6 @@

<% if @country.regions.count > 1 %>
<%= render "shared/support_interface/country_region_information_fields", f: %>
<% else %>
<%= f.hidden_field :qualifications_information, value: '' %>
<%= f.hidden_field :sanction_information, value: '' %>
<%= f.hidden_field :status_information, value: '' %>
<%= f.hidden_field :other_information, value: '' %>
<% end %>

<%= f.govuk_submit "Preview", name: "preview", value: "true" do %>
Expand Down
7 changes: 3 additions & 4 deletions app/views/support_interface/countries/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,14 @@
<%= govuk_tag(text: "Ineligible", colour: "red", classes: ["govuk-!-margin-left-2"]) %>
<% end %>

<% if country.eligibility_skip_questions %>
<% if country.subject_limited %>
<%= govuk_tag(text: "Expanded eligibility", colour: "yellow", classes: ["govuk-!-margin-left-2"]) %>
<% elsif country.eligibility_skip_questions %>
<%= govuk_tag(text: "Reduced eligibility", colour: "yellow", classes: ["govuk-!-margin-left-2"]) %>
<% else %>
<%= govuk_tag(text: "Standard eligibility", colour: "grey", classes: ["govuk-!-margin-left-2"]) %>
<% end %>

<% if country.subject_limited %>
<%= govuk_tag(text: "Subject limited", colour: "yellow", classes: ["govuk-!-margin-left-2"]) %>
<% end %>
</h2>
<% end %>
<% end %>
Expand Down
13 changes: 13 additions & 0 deletions app/views/support_interface/countries/preview.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,19 @@
<%= raw GovukMarkdown.render(@country.qualifications_information) %>
<% end %>

<% if @country.eligibility_skip_questions_changed? || @country.subject_limited_changed? %>
<h3 class="govuk-heading-s">Which route will applicants take through the eligibility checker?</h3>
<p class="govuk-body">
<% if @country.subject_limited %>
<%= govuk_tag(text: "Expanded", colour: "yellow") %>
<% elsif @country.eligibility_skip_questions %>
<%= govuk_tag(text: "Reduced", colour: "yellow") %>
<% else %>
<%= govuk_tag(text: "Standard", colour: "grey") %>
<% end %>
</p>
<% end %>

<%= form_with model: @form, url: [:support_interface, @country], method: :put do |f| %>
<%= f.hidden_field :eligibility_enabled, value: @form.eligibility_enabled %>
<%= f.hidden_field :eligibility_skip_questions, value: @form.eligibility_skip_questions %>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
class AddSubjectLimitedToCountries < ActiveRecord::Migration[7.0]
def change
add_column :countries, :subject_limited, :boolean, default: false
add_column :countries,
:subject_limited,
:boolean,
default: false,
null: false
end
end
4 changes: 2 additions & 2 deletions db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 1 addition & 6 deletions db/seeds.rb
Original file line number Diff line number Diff line change
Expand Up @@ -372,9 +372,4 @@

subject_limited_countries = %w[GH IN JM NG SG ZA ZW]

subject_limited_countries.each do |country_code|
country = Country.find_by(code: country_code)
next unless country

country.update!(subject_limited: true)
end
Country.where(code: subject_limited_countries).update_all(subject_limited: true)
8 changes: 2 additions & 6 deletions spec/factories/countries.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
# qualifications_information :text default(""), not null
# sanction_information :string default(""), not null
# status_information :string default(""), not null
# subject_limited :boolean default(FALSE)
# subject_limited :boolean default(FALSE), not null
# created_at :datetime not null
# updated_at :datetime not null
#
Expand All @@ -22,11 +22,7 @@
factory :country do
sequence :code, Country::CODES.cycle

trait :requires_secondary_education_teaching_qualification do
subject_limited { true }
end

trait :doesnt_require_secondary_education_teaching_qualification do
trait :subject_limited do
subject_limited { true }
end

Expand Down
4 changes: 2 additions & 2 deletions spec/lib/assessment_factory_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@
end

describe "age range and subjects section" do
before { create(:country, code: "SG", subject_limited: true) }
before { create(:country, :subject_limited, code: "SG") }
it "is created" do
expect(sections.age_range_subjects.count).to eq(1)
end
Expand Down Expand Up @@ -333,7 +333,7 @@
end

describe "preliminary qualifications section" do
before { create(:country, code: "SG", subject_limited: true) }
before { create(:country, :subject_limited, code: "SG") }
it "is not created" do
expect(sections.preliminary.qualifications.count).to eq(0)
end
Expand Down
18 changes: 0 additions & 18 deletions spec/lib/country_code_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,22 +84,4 @@

include_examples "true with codes", Country::CODES_IN_EUROPEAN_ECONOMIC_AREA
end

describe "#secondary_education_teaching_qualification_required?" do
subject(:secondary_education_teaching_qualification_required?) do
described_class.secondary_education_teaching_qualification_required?(code.code)
end

context "when subject_limited is true" do
let(:country) { create(:country, code: "AU", subject_limited: true) }

it { is_expected.to be true }
end

context "when subject_limited is false" do
let(:country) { create(:country, code: "AU", subject_limited: false) }

it { is_expected.to be false }
end
end
end
2 changes: 1 addition & 1 deletion spec/lib/preliminary_assessment_sections_factory_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
let(:application_form) { create(:application_form) }

describe "#call" do
before { create(:country, code: "SG", subject_limited: true) }
before { create(:country, :subject_limited, code: "SG") }
subject(:assessment_sections) { described_class.call(application_form:) }

it { is_expected.to be_empty }
Expand Down
2 changes: 1 addition & 1 deletion spec/models/country_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
# qualifications_information :text default(""), not null
# sanction_information :string default(""), not null
# status_information :string default(""), not null
# subject_limited :boolean default(FALSE)
# subject_limited :boolean default(FALSE), not null
# created_at :datetime not null
# updated_at :datetime not null
#
Expand Down
12 changes: 3 additions & 9 deletions spec/models/eligibility_check_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,16 +96,15 @@

context "when filtering by subject" do
before { eligibility_check.country_code = "IN" }
before { create(:country, :subject_limited, code: "IN") }

context "when teach_children is false" do
before { create(:country, code: "IN", subject_limited: true) }
before { eligibility_check.teach_children = false }

it { is_expected.to include(:teach_children_secondary) }
end

context "when qualified_for_subject is false" do
before { create(:country, code: "IN", subject_limited: true) }
before { eligibility_check.qualified_for_subject = false }

it { is_expected.to include(:qualified_for_subject) }
Expand Down Expand Up @@ -291,12 +290,7 @@
subject { eligibility_check.status }

let(:eligibility_check) { described_class.new(attributes) }
let(:country) do
create(
:country,
:doesnt_require_secondary_education_teaching_qualification,
)
end
let(:country) { create(:country) }

context "when no attributes are present" do
let(:attributes) { nil }
Expand Down Expand Up @@ -403,7 +397,7 @@
before { eligibility_check.country_code = code }

context "with a relevant country" do
before { create(:country, code: "JM", subject_limited: true) }
before { create(:country, :subject_limited, code: "JM") }
let(:code) { "JM" }

it { is_expected.to be true }
Expand Down
2 changes: 1 addition & 1 deletion spec/system/eligibility_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ def given_countries_exist
it = create(:country, code: "IT")
create(:region, country: it, name: "Region")
create(:region, country: it, name: "Other Region")
create(:country, :with_national_region, code: "JM", subject_limited: true)
create(:country, :with_national_region, :subject_limited, code: "JM")
end

def then_i_do_not_see_the_start_page
Expand Down
12 changes: 6 additions & 6 deletions spec/system/support_interface/countries_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,18 +59,18 @@
private

def given_countries_exist
create(:region, :national, country: create(:country, code: "IE", subject_limited: false))
create(:region, :national, country: create(:country, code: "PL", subject_limited: false))
united_states = create(:country, code: "US", subject_limited: false)
create(:region, :national, country: create(:country, code: "IE"))
create(:region, :national, country: create(:country, code: "PL"))
united_states = create(:country, code: "US")
create(:region, name: "Hawaii", country: united_states)
create(:region, name: "New York", country: united_states)
create(:region, :national, country: create(:country, code: "ES", subject_limited: false))
create(:region, :national, country: create(:country, code: "ES"))
create(
:region,
name: "British Columbia",
country: create(:country, code: "CA", subject_limited: false),
country: create(:country, code: "CA"),
)
create(:region, :national, country: create(:country, code: "CY", subject_limited: false))
create(:region, :national, country: create(:country, code: "CY"))
end

def when_i_visit_the_countries_page
Expand Down

0 comments on commit fd98610

Please sign in to comment.