From e864f7078dd7ef7356bbc848b0c2f0a99209a824 Mon Sep 17 00:00:00 2001
From: Lori Bailey <44073106+elceebee@users.noreply.github.com>
Date: Tue, 21 May 2024 13:05:13 +0100
Subject: [PATCH] Move Course Length to its own page

---
 .../publish/courses/fees_controller.rb        |   2 +-
 .../publish/courses/length_controller.rb      |  61 ++++++++
 .../publish/courses/salary_controller.rb      |   2 +-
 app/forms/publish/course_fee_form.rb          |   3 -
 app/forms/publish/course_length_form.rb       |  40 +++++
 app/forms/publish/course_salary_form.rb       |   7 +-
 .../publish/funding_type_form_methods.rb      |  22 ---
 .../courses/_course_length_field.html.erb     |  27 ++--
 .../courses/_description_content.html.erb     |   6 +-
 .../_other_course_length_field.html.erb       |   6 +-
 app/views/publish/courses/fees/edit.html.erb  |   6 +-
 .../publish/courses/length/edit.html.erb      |  39 +++++
 .../publish/courses/salary/edit.html.erb      |   8 +-
 config/locales/en.yml                         |  28 +++-
 config/routes/publish.rb                      |   2 +
 ...rb => editing_course_funding_type_spec.rb} |  72 +++------
 .../courses/editing_course_length_spec.rb     | 148 ++++++++++++++++++
 .../publish/viewing_a_course_preview_spec.rb  |   3 +-
 spec/forms/publish/course_fee_form_spec.rb    |  31 +---
 spec/forms/publish/course_length_form_spec.rb |  46 ++++++
 spec/forms/publish/course_salary_form_spec.rb |  32 +---
 .../page_objects/publish/course_fee_edit.rb   |   7 -
 22 files changed, 413 insertions(+), 185 deletions(-)
 create mode 100644 app/controllers/publish/courses/length_controller.rb
 create mode 100644 app/forms/publish/course_length_form.rb
 create mode 100644 app/views/publish/courses/length/edit.html.erb
 rename spec/features/publish/courses/{editing_course_length_and_funding_type_spec.rb => editing_course_funding_type_spec.rb} (67%)
 create mode 100644 spec/features/publish/courses/editing_course_length_spec.rb
 create mode 100644 spec/forms/publish/course_length_form_spec.rb

diff --git a/app/controllers/publish/courses/fees_controller.rb b/app/controllers/publish/courses/fees_controller.rb
index 20bfee71b6..96db79e5ab 100644
--- a/app/controllers/publish/courses/fees_controller.rb
+++ b/app/controllers/publish/courses/fees_controller.rb
@@ -26,7 +26,7 @@ def update
           if goto_preview?
             redirect_to preview_publish_provider_recruitment_cycle_course_path(provider.provider_code, recruitment_cycle.year, course.course_code)
           else
-            course_updated_message('Course length and fees')
+            course_updated_message I18n.t('publish.providers.course_fees.edit.course_fees')
 
             redirect_to publish_provider_recruitment_cycle_course_path(
               provider.provider_code,
diff --git a/app/controllers/publish/courses/length_controller.rb b/app/controllers/publish/courses/length_controller.rb
new file mode 100644
index 0000000000..cd7dab0544
--- /dev/null
+++ b/app/controllers/publish/courses/length_controller.rb
@@ -0,0 +1,61 @@
+# frozen_string_literal: true
+
+module Publish
+  module Courses
+    class LengthController < PublishController
+      before_action :redirect_if_not_editable
+
+      def edit
+        authorize(provider)
+
+        @course_length_form = CourseLengthForm.new(course_enrichment)
+
+        @course_length_form.valid? if show_errors_on_publish?
+      end
+
+      def update
+        authorize(provider)
+
+        @course_length_form = CourseLengthForm.new(course_enrichment, params: length_params)
+
+        if @course_length_form.save!
+          course_updated_message I18n.t('publish.providers.course_length.edit.course_length')
+
+          redirect_to publish_provider_recruitment_cycle_course_path(
+            provider.provider_code,
+            recruitment_cycle.year,
+            course.course_code
+          )
+
+        else
+          render :edit
+        end
+      end
+
+      private
+
+      def length_params
+        params.require(:publish_course_length_form)
+              .permit(*CourseLengthForm::FIELDS)
+      end
+
+      def course
+        @course ||= CourseDecorator.new(provider.courses.find_by!(course_code: params[:code]))
+      end
+
+      def course_enrichment
+        @course_enrichment ||= course.enrichments.find_or_initialize_draft
+      end
+
+      def redirect_if_not_editable
+        return unless course.cannot_change_course_length?
+
+        redirect_to publish_provider_recruitment_cycle_course_path(
+          provider.provider_code,
+          recruitment_cycle.year,
+          course.course_code
+        )
+      end
+    end
+  end
+end
diff --git a/app/controllers/publish/courses/salary_controller.rb b/app/controllers/publish/courses/salary_controller.rb
index a8b69422a9..91f577f295 100644
--- a/app/controllers/publish/courses/salary_controller.rb
+++ b/app/controllers/publish/courses/salary_controller.rb
@@ -16,7 +16,7 @@ def update
         @course_salary_form = CourseSalaryForm.new(course_enrichment, params: formatted_params)
 
         if @course_salary_form.save!
-          course_updated_message('Course length and salary')
+          course_updated_message I18n.t('publish.providers.course_salary.edit.course_salary')
 
           redirect_to publish_provider_recruitment_cycle_course_path(
             provider.provider_code,
diff --git a/app/forms/publish/course_fee_form.rb b/app/forms/publish/course_fee_form.rb
index d47210acf3..ba4a2afffa 100644
--- a/app/forms/publish/course_fee_form.rb
+++ b/app/forms/publish/course_fee_form.rb
@@ -9,8 +9,6 @@ class CourseFeeForm < BaseModelForm
     include FundingTypeFormMethods
 
     FIELDS = %i[
-      course_length
-      course_length_other_length
       fee_uk_eu
       fee_international
       fee_details
@@ -19,7 +17,6 @@ class CourseFeeForm < BaseModelForm
 
     attr_accessor(*FIELDS)
 
-    validates :course_length, presence: true
     validates :fee_uk_eu, presence: true
     validates :fee_international, presence: true, if: -> { course.can_sponsor_student_visa? }
 
diff --git a/app/forms/publish/course_length_form.rb b/app/forms/publish/course_length_form.rb
new file mode 100644
index 0000000000..ca5d6c5d3f
--- /dev/null
+++ b/app/forms/publish/course_length_form.rb
@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+module Publish
+  class CourseLengthForm < BaseModelForm
+    alias course_enrichment model
+
+    FIELDS = %i[course_length course_length_other_length].freeze
+
+    attr_accessor(*FIELDS)
+
+    validates :course_length, presence: true
+
+    private
+
+    def compute_fields
+      course_enrichment
+        .attributes
+        .symbolize_keys
+        .slice(*FIELDS)
+        .merge(formatted_params)
+        .symbolize_keys
+    end
+
+    def formatted_params
+      if custom_length_provided?
+        new_attributes.merge(course_length: new_attributes[:course_length_other_length])
+      else
+        new_attributes
+      end
+    end
+
+    def custom_length_provided?
+      new_attributes[:course_length] == 'Other' && new_attributes[:course_length_other_length].present?
+    end
+
+    def fields_to_ignore_before_save
+      [:course_length_other_length]
+    end
+  end
+end
diff --git a/app/forms/publish/course_salary_form.rb b/app/forms/publish/course_salary_form.rb
index 11410ca127..83ef8055ec 100644
--- a/app/forms/publish/course_salary_form.rb
+++ b/app/forms/publish/course_salary_form.rb
@@ -6,15 +6,10 @@ class CourseSalaryForm < BaseModelForm
 
     include FundingTypeFormMethods
 
-    FIELDS = %i[
-      course_length
-      course_length_other_length
-      salary_details
-    ].freeze
+    FIELDS = %i[salary_details].freeze
 
     attr_accessor(*FIELDS)
 
-    validates :course_length, presence: true
     validates :salary_details, presence: true
     validates :salary_details, words_count: { maximum: 250, message: :too_long }
 
diff --git a/app/forms/publish/funding_type_form_methods.rb b/app/forms/publish/funding_type_form_methods.rb
index 2db19c5026..753cde8208 100644
--- a/app/forms/publish/funding_type_form_methods.rb
+++ b/app/forms/publish/funding_type_form_methods.rb
@@ -2,10 +2,6 @@
 
 module Publish
   module FundingTypeFormMethods
-    def other_course_length?
-      course_length_is_other?(course_length)
-    end
-
     private
 
     def compute_fields
@@ -14,29 +10,11 @@ def compute_fields
         .symbolize_keys
         .slice(*declared_fields)
         .merge(new_attributes)
-        .merge(**hydrate_other_course_length)
         .symbolize_keys
     end
 
-    def hydrate_other_course_length
-      return {} unless course_length_is_other?(course_enrichment[:course_length])
-
-      {
-        course_length: 'Other',
-        course_length_other_length: course_enrichment[:course_length]
-      }
-    end
-
-    def fields_to_ignore_before_save
-      [:course_length_other_length]
-    end
-
     def course
       course_enrichment.course
     end
-
-    def course_length_is_other?(value)
-      value.presence && %w[OneYear TwoYears].exclude?(value)
-    end
   end
 end
diff --git a/app/views/publish/courses/_course_length_field.html.erb b/app/views/publish/courses/_course_length_field.html.erb
index 27e81154fd..c452581e30 100644
--- a/app/views/publish/courses/_course_length_field.html.erb
+++ b/app/views/publish/courses/_course_length_field.html.erb
@@ -1,19 +1,24 @@
-<%= f.govuk_radio_buttons_fieldset(:course_length,
-  legend: { text: "Course length", size: "m" },
+<%= f.govuk_radio_buttons_fieldset(
+  :course_length,
+  legend: nil,
   form_group: {
     id: form_object.errors.key?(:course_length) ? "course_length-error" : "course-length"
-  }) do %>
-    <%= f.govuk_radio_button(:course_length, "OneYear",
-      checked: @copied_fields_values&.value?("OneYear") || @course.course_length == "OneYear",
+  }
+) do %>
+  <legend>
+    <h1 class="govuk-heading-l">
+      <span class="govuk-caption-l"><%= @course.name_and_code %></span>
+      <%= t("publish.providers.course_length.edit.course_length") %>
+    </h1>
+  </legend>
+    <%= f.govuk_radio_button(:course_length, t("publish.providers.course_length.edit.one_year.value"),
       data: { qa: "course_course_length_oneyear" },
-      label: { text: "1 year" },
+      label: { text: t("publish.providers.course_length.edit.one_year.label") },
       link_errors: true) %>
 
-    <%= f.govuk_radio_button(:course_length, "TwoYears",
-      checked: @copied_fields_values&.value?("TwoYears") || @course.course_length == "TwoYears",
-      label: { text: "Up to 2 years" },
+    <%= f.govuk_radio_button(:course_length, t("publish.providers.course_length.edit.two_years.value"),
+      label: { text: t("publish.providers.course_length.edit.two_years.label") },
       data: { qa: "course_course_length_twoyears" }) %>
 
-    <%= render partial: "publish/courses/other_course_length_field", locals: { f:, course_object: @source_course ? source_course : @course } %>
-
+    <%= render partial: "publish/courses/other_course_length_field", locals: { f:, course_object: @course } %>
 <% end %>
diff --git a/app/views/publish/courses/_description_content.html.erb b/app/views/publish/courses/_description_content.html.erb
index 5bca40a534..e956647b3b 100644
--- a/app/views/publish/courses/_description_content.html.erb
+++ b/app/views/publish/courses/_description_content.html.erb
@@ -47,7 +47,7 @@
       "Course length",
       value_provided?(course.length),
       %w[course_length],
-      action_path: course.cannot_change_course_length? ? nil : "#{fees_publish_provider_recruitment_cycle_course_path(@provider.provider_code, course.recruitment_cycle_year, course.course_code)}#course-length",
+      action_path: course.cannot_change_course_length? ? nil : length_publish_provider_recruitment_cycle_course_path(@provider.provider_code, course.recruitment_cycle_year, course.course_code),
       action_visually_hidden_text: "course length"
     ) %>
 
@@ -103,7 +103,7 @@
       "Course length",
       value_provided?(course.length),
       %w[course_length],
-      action_path: course.cannot_change_course_length? ? nil : "#{salary_publish_provider_recruitment_cycle_course_path(@provider.provider_code, course.recruitment_cycle_year, course.course_code)}#course-length",
+      action_path: course.cannot_change_course_length? ? nil : length_publish_provider_recruitment_cycle_course_path(@provider.provider_code, course.recruitment_cycle_year, course.course_code),
       action_visually_hidden_text: "course length"
     ) %>
 
@@ -113,7 +113,7 @@
       "Salary",
       value_provided?(course.salary_details),
       %w[salary_details],
-      action_path: course.is_withdrawn? ? nil : "#{salary_publish_provider_recruitment_cycle_course_path(@provider.provider_code, course.recruitment_cycle_year, course.course_code)}#salary",
+      action_path: course.is_withdrawn? ? nil : salary_publish_provider_recruitment_cycle_course_path(@provider.provider_code, course.recruitment_cycle_year, course.course_code),
       action_visually_hidden_text: "salary"
     ) %>
   <% end %>
diff --git a/app/views/publish/courses/_other_course_length_field.html.erb b/app/views/publish/courses/_other_course_length_field.html.erb
index d9633117ec..f6a94efd4c 100644
--- a/app/views/publish/courses/_other_course_length_field.html.erb
+++ b/app/views/publish/courses/_other_course_length_field.html.erb
@@ -1,11 +1,11 @@
-<%= f.govuk_radio_button(:course_length, "Other",
+<%= f.govuk_radio_button(:course_length, t("publish.providers.course_length.edit.other.value"),
   checked: course_object.other_course_length?,
-  label: { text: "Other" },
+  label: { text: t("publish.providers.course_length.edit.other.label") },
   data: { qa: "course_course_length_other" }) do %>
 
   <%= f.govuk_text_field(:course_length_other_length,
     value: course_object.other_course_length? ? course_object.course_length : "",
-    label: { text: "Course length", size: "s" },
+    label: { text: t("publish.providers.course_length.edit.custom_length.label"), size: "s" },
     width: 20,
     data: { qa: "course_course_length_other_length" }) %>
 <% end %>
diff --git a/app/views/publish/courses/fees/edit.html.erb b/app/views/publish/courses/fees/edit.html.erb
index 559e6bde38..82ffe7d392 100644
--- a/app/views/publish/courses/fees/edit.html.erb
+++ b/app/views/publish/courses/fees/edit.html.erb
@@ -1,4 +1,4 @@
-<% page_title = "Course length and fees" %>
+<% page_title = t("publish.providers.course_fees.edit.course_fees") %>
 <% content_for :page_title, title_with_error_prefix("#{page_title} – #{@course.name_and_code}", @course_fee_form.errors.any?) %>
 
 <% if params[:copy_from].present? %>
@@ -26,11 +26,9 @@
         <%= page_title %>
       </h1>
 
-      <%= render partial: "publish/courses/course_length_field", locals: { f:, form_object: @course_fee_form } %>
-
       <hr class="govuk-section-break govuk-section-break--visible govuk-section-break--l">
 
-      <h3 class="govuk-heading-m">Course fees</h3>
+      <h2 class="govuk-heading-m"><%= t("publish.providers.course_fees.edit.course_fees") %></h2>
 
       <%= f.govuk_text_field(:fee_uk_eu,
         form_group: { id: @course_fee_form.errors.key?(:fee_uk_eu) ? "fee_uk_eu-error" : "fee-uk" },
diff --git a/app/views/publish/courses/length/edit.html.erb b/app/views/publish/courses/length/edit.html.erb
new file mode 100644
index 0000000000..bc159da531
--- /dev/null
+++ b/app/views/publish/courses/length/edit.html.erb
@@ -0,0 +1,39 @@
+<% content_for :page_title,
+               title_with_error_prefix(
+                 t("publish.providers.course_length.edit.page_title", course_name_and_code: @course.name_and_code),
+                 @course_length_form.errors.any?
+               ) %>
+
+<div class="govuk-grid-row">
+  <div class="govuk-grid-column-two-thirds">
+    <%= form_with(
+          model: @course_length_form,
+          url: length_publish_provider_recruitment_cycle_course_path(@provider.provider_code,
+                                                                     @course.recruitment_cycle_year,
+                                                                     @course.course_code),
+          data: { qa: "enrichment-form", module: "form-check-leave" },
+          method: :patch,
+          local: true
+        ) do |f| %>
+
+      <% content_for :before_content do %>
+        <%= govuk_back_link_to(
+              back_link_path(
+                param_form_key: f.object_name.to_sym,
+                params:,
+                provider_code: @provider.provider_code,
+                recruitment_cycle_year: @course.recruitment_cycle_year,
+                course_code: @course.course_code
+              )
+            ) %>
+      <% end %>
+
+      <%= f.govuk_error_summary %>
+
+      <%= render partial: "publish/courses/course_length_field", locals: { f:, form_object: @course_length_form } %>
+
+      <%= f.govuk_submit t("publish.providers.course_length.edit.update_course_length") %>
+
+    <% end %>
+  </div>
+</div>
diff --git a/app/views/publish/courses/salary/edit.html.erb b/app/views/publish/courses/salary/edit.html.erb
index 1af3b39f2e..029d848ca6 100644
--- a/app/views/publish/courses/salary/edit.html.erb
+++ b/app/views/publish/courses/salary/edit.html.erb
@@ -1,4 +1,4 @@
-<% page_title = "Course length and salary" %>
+<% page_title = t("publish.providers.course_salary.edit.course_salary") %>
 <% content_for :page_title, title_with_error_prefix("#{page_title} – #{@course.name_and_code}", @course_salary_form.errors.any?) %>
 
 <% content_for :before_content do %>
@@ -22,12 +22,6 @@
         <%= page_title %>
       </h1>
 
-      <%= render partial: "publish/courses/course_length_field", locals: { f:, form_object: @course_salary_form } %>
-
-      <hr class="govuk-section-break govuk-section-break--visible govuk-section-break--l">
-
-      <h3 class="govuk-heading-m">Salary</h3>
-
       <p class="govuk-body">Give details about the salary for this course.</p>
       <p class="govuk-body">You should:</p>
       <ul class="govuk-list govuk-list--bullet">
diff --git a/config/locales/en.yml b/config/locales/en.yml
index fe194d4d54..32ffe1f955 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -293,6 +293,28 @@ en:
     update: "Update user"
   publish:
     providers:
+      course_length:
+        edit:
+          course_length: Course length
+          page_title: Course length - %{course_name_and_code}
+          one_year:
+            label: 1 year
+            value: OneYear
+          two_years:
+            label: Up to 2 years
+            value: TwoYears
+          other:
+            label: Other
+            value: Other
+          custom_length:
+            label: Course length
+          update_course_length: Update course length
+      course_salary:
+        edit:
+          course_salary: Course salary
+      course_fees:
+        edit:
+          course_fees: Course fees
       study_mode:
         form:
           select_all_that_apply: Select all that apply
@@ -792,8 +814,6 @@ en:
               too_long: "Reduce the word count for how school placements work"
         publish/course_fee_form:
           attributes:
-            course_length:
-              blank: "Enter a course length"
             fee_uk_eu:
               blank: "Enter fees for UK students"
               not_a_number: "Course fees for UK and EU students must be a valid number"
@@ -810,6 +830,10 @@ en:
               too_long: "Reduce the word count for fee details"
             financial_support:
               too_long: "Reduce the word count for financial support"
+        publish/course_length_form:
+          attributes:
+            course_length:
+              blank: Enter a course length
         publish/course_salary_form:
           attributes:
             course_length:
diff --git a/config/routes/publish.rb b/config/routes/publish.rb
index 3c6b5c63d4..2badae725e 100644
--- a/config/routes/publish.rb
+++ b/config/routes/publish.rb
@@ -163,6 +163,8 @@
         patch '/about', on: :member, to: 'courses/course_information#update'
         get '/requirements', on: :member, to: 'courses/requirements#edit'
         patch '/requirements', on: :member, to: 'courses/requirements#update'
+        get '/length', on: :member, to: 'courses/length#edit'
+        patch '/length', on: :member, to: 'courses/length#update'
         get '/fees', on: :member, to: 'courses/fees#edit'
         patch '/fees', on: :member, to: 'courses/fees#update'
         get '/salary', on: :member, to: 'courses/salary#edit'
diff --git a/spec/features/publish/courses/editing_course_length_and_funding_type_spec.rb b/spec/features/publish/courses/editing_course_funding_type_spec.rb
similarity index 67%
rename from spec/features/publish/courses/editing_course_length_and_funding_type_spec.rb
rename to spec/features/publish/courses/editing_course_funding_type_spec.rb
index 8172fcea39..3e1daf1f33 100644
--- a/spec/features/publish/courses/editing_course_length_and_funding_type_spec.rb
+++ b/spec/features/publish/courses/editing_course_funding_type_spec.rb
@@ -2,18 +2,18 @@
 
 require 'rails_helper'
 
-feature 'Editing course length and funding type' do
+feature 'Editing funding type' do
   before do
     given_i_am_authenticated_as_a_provider_user
   end
 
   context 'fee based' do
-    scenario 'i can update the course length and fee' do
+    scenario 'i can update the course fee' do
       and_there_is_a_course_i_want_to_edit(:fee_type_based)
       when_i_visit_the_course_fee_edit_page
-      and_i_update_the_length_and_fee
+      and_i_update_the_fee
       and_i_submit_the(publish_course_fee_edit_page)
-      then_i_should_see_the_correct_success_message('Course length and fees updated')
+      then_i_should_see_the_correct_success_message('Course fees updated')
       and_the_course_fee_is_updated
     end
 
@@ -24,25 +24,24 @@
     end
 
     context 'copying content from another course' do
-      let!(:course2) do
+      let!(:biology_course) do
         create(
           :course,
           provider:,
           name: 'Biology',
-          enrichments: [course2_enrichment]
+          enrichments: [biology_course_enrichment]
         )
       end
 
       let!(:course3) do
         create(:course,
                provider:,
-               name: 'Biology',
+               name: 'Mathematics',
                enrichments: [course3_enrichment])
       end
 
-      let(:course2_enrichment) do
+      let(:biology_course_enrichment) do
         build(:course_enrichment,
-              course_length: 'OneYear',
               fee_uk_eu: '8000',
               fee_international: '20000',
               fee_details: 'Test fee details',
@@ -51,7 +50,6 @@
 
       let(:course3_enrichment) do
         build(:course_enrichment,
-              course_length: '5 years',
               fee_uk_eu: '',
               fee_international: '',
               fee_details: '',
@@ -61,11 +59,10 @@
       scenario 'all fields get copied if all are present' do
         and_there_is_a_course_i_want_to_edit(:fee_type_based)
         when_i_visit_the_course_fee_edit_page
-        publish_course_fee_edit_page.copy_content.copy(course2)
+        publish_course_fee_edit_page.copy_content.copy(biology_course)
 
         [
           'Your changes are not yet saved',
-          'Course length',
           'Fee for UK students',
           'Fee for international students',
           'Fee details',
@@ -74,38 +71,9 @@
           expect(publish_course_fee_edit_page.copy_content_warning).to have_content(name)
         end
 
-        expect(publish_course_fee_edit_page.course_length.one_year).to be_checked
-        expect(publish_course_fee_edit_page.course_length.upto_two_years).not_to be_checked
-        expect(publish_course_fee_edit_page.course_length.other).not_to be_checked
-        expect(publish_course_fee_edit_page.course_length.other_text.value).to be_blank
-        expect(publish_course_fee_edit_page.uk_fee.value).to eq(course2_enrichment.fee_uk_eu.to_s)
-        expect(publish_course_fee_edit_page.international_fee.value).to eq(course2_enrichment.fee_international.to_s)
-        expect(publish_course_fee_edit_page.financial_support.value).to eq(course2_enrichment.financial_support)
-      end
-
-      scenario 'with custom course length and all other fields empty' do
-        and_there_is_a_course_i_want_to_edit(:fee_type_based)
-        when_i_visit_the_course_fee_edit_page
-        publish_course_fee_edit_page.copy_content.copy(course3)
-
-        [
-          'Your changes are not yet saved',
-          'Course length'
-        ].each do |name|
-          expect(publish_course_fee_edit_page.copy_content_warning).to have_content(name)
-        end
-
-        [
-          'Fee for UK students',
-          'Fee for international students',
-          'Fee details',
-          'Financial support'
-        ].each do |name|
-          expect(publish_course_fee_edit_page.copy_content_warning).to have_no_content(name)
-        end
-
-        expect(publish_course_fee_edit_page.course_length.other).to be_checked
-        expect(publish_course_fee_edit_page.course_length.other_text.value).to eq(course3_enrichment.course_length)
+        expect(publish_course_fee_edit_page.uk_fee.value).to eq(biology_course_enrichment.fee_uk_eu.to_s)
+        expect(publish_course_fee_edit_page.international_fee.value).to eq(biology_course_enrichment.fee_international.to_s)
+        expect(publish_course_fee_edit_page.financial_support.value).to eq(biology_course_enrichment.financial_support)
       end
     end
 
@@ -119,12 +87,12 @@
   end
 
   context 'salary based' do
-    scenario 'i can update the course length and salary details' do
+    scenario 'i can update the course salary details' do
       and_there_is_a_course_i_want_to_edit(:salary_type_based)
       when_i_visit_the_course_salary_page
-      and_i_update_the_length_and_salary_details
+      and_i_update_the_salary_details
       and_i_submit_the(publish_course_salary_edit_page)
-      then_i_should_see_the_correct_success_message('Course length and salary updated')
+      then_i_should_see_the_correct_success_message('Course salary updated')
       and_the_course_salary_is_updated
     end
 
@@ -161,17 +129,15 @@ def then_i_should_see_the_reuse_content
     expect(publish_course_fee_edit_page).to have_use_content
   end
 
-  def and_i_update_the_length_and_fee
+  def and_i_update_the_fee
     @new_uk_fee = 10_000
 
-    publish_course_fee_edit_page.course_length.upto_two_years.choose
     publish_course_fee_edit_page.uk_fee.set(@new_uk_fee)
   end
 
-  def and_i_update_the_length_and_salary_details
+  def and_i_update_the_salary_details
     @new_salary_details = 'new salary details'
 
-    publish_course_salary_edit_page.course_length.upto_two_years.choose
     publish_course_salary_edit_page.salary_details.set(@new_salary_details)
   end
 
@@ -192,20 +158,18 @@ def then_i_should_see_the_correct_success_message(message)
   end
 
   def then_i_should_see_a_success_message
-    expect(page).to have_content('Course length and fees updated')
+    expect(page).to have_content('Course fees updated')
   end
 
   def and_the_course_fee_is_updated
     enrichment = course.reload.enrichments.find_or_initialize_draft
 
-    expect(enrichment.course_length).to eq('TwoYears')
     expect(enrichment.fee_uk_eu).to eq(@new_uk_fee)
   end
 
   def and_the_course_salary_is_updated
     enrichment = course.reload.enrichments.find_or_initialize_draft
 
-    expect(enrichment.course_length).to eq('TwoYears')
     expect(enrichment.salary_details).to eq(@new_salary_details)
   end
 
diff --git a/spec/features/publish/courses/editing_course_length_spec.rb b/spec/features/publish/courses/editing_course_length_spec.rb
new file mode 100644
index 0000000000..80fd60fd43
--- /dev/null
+++ b/spec/features/publish/courses/editing_course_length_spec.rb
@@ -0,0 +1,148 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+feature 'Editing course length' do
+  scenario 'I enter invalid data' do
+    given_i_am_authenticated_as_a_provider_user
+    and_there_is_a_course_without_a_length_i_want_to_edit
+    when_i_visit_the_course_length_edit_page
+    and_i_submit_the_form
+    then_i_an_error_message
+  end
+
+  scenario 'I update the course length to a standard length (eg Up to two years)' do
+    given_i_am_authenticated_as_a_provider_user
+    and_there_is_a_1_year_course_i_want_to_edit
+    when_i_visit_the_course_length_edit_page
+    then_i_see_one_year_selected
+
+    when_i_update_the_length_to_2_years
+    and_i_submit_the_form
+    then_i_see_a_success_message
+    and_the_course_length_is_two_years
+
+    when_i_visit_the_course_length_edit_page
+    then_i_see_two_years_selected
+  end
+
+  scenario 'I update the course length with a custom length' do
+    given_i_am_authenticated_as_a_provider_user
+    and_there_is_a_5_year_course_i_want_to_edit
+    when_i_visit_the_course_length_edit_page
+    then_i_see_the_custom_length_of_5_years
+
+    when_i_update_the_length_to_a_custom_length
+    and_i_submit_the_form
+    then_i_see_a_success_message
+    and_the_course_length_is_the_custom_length
+
+    when_i_visit_the_course_length_edit_page
+    then_i_see_the_custom_length
+  end
+
+  scenario 'I try to edit course that should not be edited' do
+    given_i_am_authenticated_as_a_provider_user
+    and_there_is_an_uneditable_course
+    when_i_visit_the_course_length_edit_page
+    then_i_am_redirected_to_the_summary_page
+  end
+
+  private
+
+  def given_i_am_authenticated_as_a_provider_user
+    given_i_am_authenticated(user: create(:user, :with_provider))
+  end
+
+  def and_there_is_a_course_without_a_length_i_want_to_edit
+    and_there_is_a_course_i_want_to_edit(nil)
+  end
+
+  def and_there_is_a_1_year_course_i_want_to_edit
+    and_there_is_a_course_i_want_to_edit('OneYear')
+  end
+
+  def and_there_is_a_5_year_course_i_want_to_edit
+    and_there_is_a_course_i_want_to_edit('5 years')
+  end
+
+  def and_there_is_an_uneditable_course
+    given_a_course_exists(program_type: 'TDA')
+  end
+
+  def and_there_is_a_course_i_want_to_edit(course_length)
+    given_a_course_exists(enrichments: [build(:course_enrichment, :published, course_length:)])
+  end
+
+  def then_i_am_redirected_to_the_summary_page
+    expect(page).to have_current_path publish_provider_recruitment_cycle_course_path(
+      provider_code: provider.provider_code,
+      recruitment_cycle_year: provider.recruitment_cycle_year,
+      code: course.course_code
+    ), ignore_query: true
+  end
+
+  def when_i_visit_the_course_length_edit_page
+    visit length_publish_provider_recruitment_cycle_course_path(
+      provider.provider_code,
+      course.recruitment_cycle_year,
+      course.course_code
+    )
+  end
+
+  def when_i_update_the_length_to_2_years
+    choose 'Up to 2 years'
+  end
+
+  def when_i_update_the_length_to_a_custom_length
+    fill_in 'Course length', with: 'Three years'
+  end
+
+  def then_i_see_two_years_selected
+    expect(find_field('Up to 2 years')).to be_checked
+  end
+
+  def then_i_see_one_year_selected
+    expect(find_field('1 year')).to be_checked
+  end
+
+  def then_i_see_the_custom_length_of_5_years
+    expect(find_field('Other')).to be_checked
+    expect(find_field('Course length').value).to eq '5 years'
+  end
+
+  def then_i_see_the_custom_length
+    expect(find_field('Other')).to be_checked
+    expect(find_field('Course length').value).to eq 'Three years'
+  end
+
+  def and_i_submit_the_form
+    click_on 'Update course length'
+  end
+
+  def then_i_an_error_message
+    expect(page).to have_content('Enter a course length').twice
+  end
+
+  def then_i_see_a_success_message
+    expect(page).to have_content('Course length updated')
+  end
+
+  def and_the_course_length_is_two_years
+    enrichment = course.reload.enrichments.find_or_initialize_draft
+
+    expect(enrichment.course_length).to eq 'TwoYears'
+    expect(page).to have_content 'Up to 2 years'
+  end
+
+  def and_the_course_length_is_the_custom_length
+    enrichment = course.reload.enrichments.find_or_initialize_draft
+
+    expect(enrichment.course_length).to eq('Three years')
+    expect(page).to have_content 'Three years'
+  end
+
+  def provider
+    @current_user.providers.first
+  end
+end
diff --git a/spec/features/publish/viewing_a_course_preview_spec.rb b/spec/features/publish/viewing_a_course_preview_spec.rb
index 64c0ff6ac0..10d0b1e297 100644
--- a/spec/features/publish/viewing_a_course_preview_spec.rb
+++ b/spec/features/publish/viewing_a_course_preview_spec.rb
@@ -434,10 +434,9 @@ def and_i_submit_a_valid_form
   end
 
   def and_i_submit_a_valid_course_fees
-    choose '1 year'
     fill_in 'Fee for UK students', with: '100'
 
-    click_link_or_button 'Update course length and fees'
+    click_link_or_button 'Update course fees'
   end
 
   def provider
diff --git a/spec/forms/publish/course_fee_form_spec.rb b/spec/forms/publish/course_fee_form_spec.rb
index 025659dd30..83bfa64893 100644
--- a/spec/forms/publish/course_fee_form_spec.rb
+++ b/spec/forms/publish/course_fee_form_spec.rb
@@ -11,7 +11,6 @@ module Publish
     subject { described_class.new(enrichment, params:) }
 
     describe 'validations' do
-      it { is_expected.to validate_presence_of(:course_length) }
       it { is_expected.to validate_presence_of(:fee_uk_eu) }
 
       it 'validates UK/EU Fee' do
@@ -65,41 +64,15 @@ module Publish
       end
     end
 
-    context 'hydrating user set course length value' do
-      before do
-        enrichment.course_length = 'some user length'
-      end
-
-      it 'sets the course length value to other length' do
-        expect(subject.course_length).to eq('Other')
-      end
-
-      it 'sets the course length other length value to the user input' do
-        expect(subject.course_length_other_length).to eq('some user length')
-      end
-    end
-
-    describe '#other_course_length?' do
-      before do
-        enrichment.course_length = 'some length'
-      end
-
-      it 'returns true if value is user set' do
-        expect(subject.other_course_length?).to be_truthy
-      end
-    end
-
     describe '#save!' do
-      let(:params) { { course_length: 'some new value', fee_uk_eu: 12_000 } }
+      let(:params) { { fee_uk_eu: 12_000 } }
 
       before do
         enrichment.fee_uk_eu = 9500
-        enrichment.course_length = 'OneYear'
       end
 
       it 'saves the provider with any new attributes' do
-        expect { subject.save! }.to change(enrichment, :course_length).from('OneYear').to('some new value')
-                                                                      .and change(enrichment, :fee_uk_eu).from(9500).to(12_000)
+        expect { subject.save! }.to change(enrichment, :fee_uk_eu).from(9500).to(12_000)
       end
     end
   end
diff --git a/spec/forms/publish/course_length_form_spec.rb b/spec/forms/publish/course_length_form_spec.rb
new file mode 100644
index 0000000000..2fb6951b2f
--- /dev/null
+++ b/spec/forms/publish/course_length_form_spec.rb
@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+module Publish
+  describe CourseLengthForm, type: :model do
+    let(:params) { {} }
+    let(:course) { build(:course, :fee_type_based) }
+    let(:enrichment) { course.enrichments.find_or_initialize_draft }
+
+    describe 'validations' do
+      subject { described_class.new(enrichment, params:) }
+
+      it { is_expected.to validate_presence_of(:course_length) }
+    end
+
+    describe '#save!' do
+      context 'with standard course value' do
+        it 'saves standard value' do
+          enrichment.update(course_length: 'TwoYears')
+          params = { course_length: 'OneYear' }
+          subject = described_class.new(enrichment, params:)
+          expect { subject.save! }.to change(enrichment, :course_length).from('TwoYears').to('OneYear')
+        end
+      end
+
+      context 'custom length, not specified' do
+        it 'saves length as "Other"' do
+          enrichment.update(course_length: 'TwoYears')
+          params = { course_length: 'Other', course_length_other_length: nil }
+          subject = described_class.new(enrichment, params:)
+          expect { subject.save! }.to change(enrichment, :course_length).from('TwoYears').to('Other')
+        end
+      end
+
+      context 'custom length, specified' do
+        it 'saves length as with custom value' do
+          enrichment.update(course_length: 'TwoYears')
+          params = { course_length: 'Other', course_length_other_length: 'Some user input' }
+          subject = described_class.new(enrichment, params:)
+          expect { subject.save! }.to change(enrichment, :course_length).from('TwoYears').to('Some user input')
+        end
+      end
+    end
+  end
+end
diff --git a/spec/forms/publish/course_salary_form_spec.rb b/spec/forms/publish/course_salary_form_spec.rb
index aeb7e67392..122e39dfcb 100644
--- a/spec/forms/publish/course_salary_form_spec.rb
+++ b/spec/forms/publish/course_salary_form_spec.rb
@@ -11,8 +11,6 @@ module Publish
     subject { described_class.new(enrichment, params:) }
 
     describe 'validations' do
-      it { is_expected.to validate_presence_of(:course_length) }
-
       context 'salary details' do
         before do
           enrichment.salary_details = Faker::Lorem.sentence(word_count: 251)
@@ -27,41 +25,15 @@ module Publish
       end
     end
 
-    context 'hydrating user set course length value' do
-      before do
-        enrichment.course_length = 'some user length'
-      end
-
-      it 'sets the course length value to other length' do
-        expect(subject.course_length).to eq('Other')
-      end
-
-      it 'sets the course length other length value to the user input' do
-        expect(subject.course_length_other_length).to eq('some user length')
-      end
-    end
-
-    describe '#other_course_length?' do
-      before do
-        enrichment.course_length = 'some length'
-      end
-
-      it 'returns true if value is user set' do
-        expect(subject.other_course_length?).to be_truthy
-      end
-    end
-
     describe '#save!' do
-      let(:params) { { course_length: 'some new value', salary_details: 'some text' } }
+      let(:params) { { salary_details: 'some text' } }
 
       before do
         enrichment.salary_details = Faker::Lorem.sentence(word_count: 249)
-        enrichment.course_length = nil
       end
 
       it 'saves the provider with any new attributes' do
-        expect { subject.save! }.to change(enrichment, :course_length).from(nil).to('some new value')
-                                                                      .and change(enrichment, :salary_details).to('some text')
+        expect { subject.save! }.to change(enrichment, :salary_details).to('some text')
       end
     end
   end
diff --git a/spec/support/page_objects/publish/course_fee_edit.rb b/spec/support/page_objects/publish/course_fee_edit.rb
index 608e80ba7c..9356902a39 100644
--- a/spec/support/page_objects/publish/course_fee_edit.rb
+++ b/spec/support/page_objects/publish/course_fee_edit.rb
@@ -10,13 +10,6 @@ class CourseFeeEdit < PageObjects::Base
 
       sections :errors, Sections::ErrorLink, '.govuk-error-summary__list li>a'
 
-      section :course_length, '#course-length' do
-        element :one_year, '#publish-course-fee-form-course-length-oneyear-field'
-        element :upto_two_years, '#publish-course-fee-form-course-length-twoyears-field'
-        element :other, '#publish-course-fee-form-course-length-other-field'
-        element :other_text, '#publish-course-fee-form-course-length-other-length-field'
-      end
-
       element :copy_content_warning, '[data-qa="copy-course-warning"]'
       element :uk_fee, '#publish-course-fee-form-fee-uk-eu-field'
       element :international_fee, '#publish-course-fee-form-fee-international-field'