Skip to content

Commit

Permalink
Review content and bug fix
Browse files Browse the repository at this point in the history
  • Loading branch information
elceebee committed May 20, 2024
1 parent 3728609 commit 7731046
Show file tree
Hide file tree
Showing 9 changed files with 41 additions and 16 deletions.
2 changes: 1 addition & 1 deletion app/controllers/publish/courses/study_mode_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def update

@course_study_mode_form = CourseStudyModeForm.new(@course, params: study_mode_params)
if @course_study_mode_form.save!
course_updated_message('Full time or part time')
course_updated_message I18n.t('publish.providers.study_mode.form.study_pattern')

redirect_to details_publish_provider_recruitment_cycle_course_path(
provider.provider_code,
Expand Down
6 changes: 6 additions & 0 deletions app/forms/publish/course_study_mode_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ class CourseStudyModeForm < BaseModelForm
presence: true,
inclusion: { in: Course.study_modes.keys }

def study_mode_checked?(value)
mode = study_mode.nil? ? model.study_mode : study_mode

mode == value || mode == 'full_time_or_part_time'
end

private

def valid_before_save
Expand Down
2 changes: 1 addition & 1 deletion app/views/publish/courses/_basic_details_tab.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
end

summary_list.with_row(html_attributes: { data: { qa: "course__study_mode" } }) do |row|
row.with_key { "Full time or part time" }
row.with_key { t("publish.providers.study_mode.form.study_pattern") }
row.with_value { course.study_mode&.humanize }

if course.cannot_change_study_mode?
Expand Down
2 changes: 1 addition & 1 deletion app/views/publish/courses/confirmation.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
<% end %>

<% summary_list.with_row(html_attributes: { data: { qa: "course__study_mode" } }) do |row| %>
<% row.with_key { "Full time or part time" } %>
<% row.with_key { t("publish.providers.study_mode.form.study_pattern") } %>
<% row.with_value { course.study_mode&.humanize } %>
<% unless course.teacher_degree_apprenticeship?
row.with_action(
Expand Down
2 changes: 1 addition & 1 deletion app/views/publish/courses/study_mode/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
<%= f.govuk_check_box :study_mode,
value,
label: { text: t("edit_options.study_modes.#{value}.label") },
checked: course.study_mode.in?([value, "full_time_or_part_time"]),
checked: @course_study_mode_form.study_mode_checked?(value),
link_errors: index.zero? %>

<% end %>
Expand Down
6 changes: 1 addition & 5 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,6 @@ en:
label: "Part time"
full_time:
label: "Full time"
full_time_or_part_time:
label: "Full time or part time"
apprenticeship:
pg_teaching_apprenticeship:
label: "Yes"
Expand Down Expand Up @@ -839,7 +837,7 @@ en:
publish/course_study_mode_form:
attributes:
study_mode:
blank: You must choose a study pattern. Select all that apply.
blank: Select a study pattern
publish/course_withdrawal_form:
attributes:
confirm_course_code:
Expand Down Expand Up @@ -982,8 +980,6 @@ en:
label: "Part time"
full_time:
label: "Full time"
full_time_or_part_time:
label: "Full time or part time"
can_sponsor_skilled_worker_visas:
true:
label: "Yes"
Expand Down
13 changes: 10 additions & 3 deletions spec/features/publish/courses/editing_course_study_mode_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,12 @@
end

scenario 'updating with invalid data' do
and_there_is_a_course_with_no_study_mode
and_there_is_a_part_time_course_i_want_to_edit
when_i_visit_the_course_study_mode_page
and_i_deselect_part_time_study_mode
and_i_submit
then_i_should_see_an_error_message
and_nothing_is_selected
end

def given_i_am_authenticated_as_a_provider_user
Expand Down Expand Up @@ -73,7 +75,7 @@ def and_i_submit
end

def then_i_should_see_a_success_message
expect(page).to have_content(I18n.t('success.saved', value: 'Full time or part time'))
expect(page).to have_content(I18n.t('success.saved', value: 'Study pattern'))
end

def then_i_see_part_time_selected
Expand All @@ -99,9 +101,14 @@ def then_i_see_full_time_selected
expect(publish_course_study_mode_edit_page.part_time.checked?).to be false
end

def and_nothing_is_selected
expect(publish_course_study_mode_edit_page.part_time.checked?).to be false
expect(publish_course_study_mode_edit_page.full_time.checked?).to be false
end

def then_i_should_see_an_error_message
expect(publish_course_study_mode_edit_page.error_messages)
.to include('You must choose a study pattern. Select all that apply.')
.to include('Select a study pattern')
end

def provider
Expand Down
2 changes: 1 addition & 1 deletion spec/features/publish/courses/new_study_mode_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def then_i_am_met_with_the_schools_page(study_mode)

def then_i_am_met_with_errors
expect(page).to have_content('There is a problem')
expect(page).to have_content('You must choose a study pattern. Select all that apply.').twice
expect(page).to have_content('Select a study pattern').twice
end

def selected_params(study_mode)
Expand Down
22 changes: 19 additions & 3 deletions spec/forms/publish/course_study_mode_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ module Publish
let(:params) { {} }

describe '#save!' do
it 'does not call the course.ensure_site_statuses_match_study_mode' do
it 'does not save any changes' do
expect(subject.save!).to be false
end
end
Expand All @@ -23,8 +23,12 @@ module Publish

describe '#save!' do
it 'does calls the course.ensure_site_statuses_match_study_mode' do
expect(course).to receive(:changed?)
expect(course).not_to receive(:ensure_site_statuses_match_study_mode)
allow(course).to receive(:changed?).and_return true
expect(course).to receive(:ensure_site_statuses_match_study_mode)
subject.save!
end

it 'saves the study mode as part time' do
subject.save!

expect(course.study_mode).to eq 'part_time'
Expand All @@ -36,6 +40,12 @@ module Publish
let(:params) { { study_mode: %w[full_time part_time] } }

it 'does calls the course.ensure_site_statuses_match_study_mode' do
allow(course).to receive(:changed?).and_return true
expect(course).to receive(:ensure_site_statuses_match_study_mode)
subject.save!
end

it 'saves the study mode as full_time_or_part_time' do
subject.save!

expect(course.study_mode).to eq 'full_time_or_part_time'
Expand All @@ -46,6 +56,12 @@ module Publish
let(:params) { { study_mode: ['full_time'] } }

it 'does calls the course.ensure_site_statuses_match_study_mode' do
allow(course).to receive(:changed?).and_return true
expect(course).to receive(:ensure_site_statuses_match_study_mode)
subject.save!
end

it 'saves the study mode as full time' do
subject.save!

expect(course.study_mode).to eq 'full_time'
Expand Down

0 comments on commit 7731046

Please sign in to comment.