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

[CPDNPQ-2501] Remove Cohort/Schedule#editable? #2143

Merged
merged 1 commit into from
Jan 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions app/controllers/npq_separation/admin/cohorts_controller.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
class NpqSeparation::Admin::CohortsController < NpqSeparation::AdminController
before_action :ensure_super_admin, except: %i[index show]
before_action :cohort, only: %i[show edit update destroy]
before_action :ensure_editable, only: %i[edit update destroy]

def index
@pagy, @cohorts = pagy(Cohort.all.order(start_year: :desc))
Expand Down Expand Up @@ -64,11 +63,4 @@ def ensure_super_admin
redirect_to action: :index
end
end

def ensure_editable
unless @cohort.editable?
flash[:error] = "This cohort is not editable"
redirect_to npq_separation_admin_cohort_path(@cohort)
end
end
end
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
class NpqSeparation::Admin::SchedulesController < NpqSeparation::AdminController
before_action :ensure_super_admin, except: :show
before_action :schedule, only: %i[show edit update destroy]
before_action :ensure_editable, only: %i[edit update destroy]

def show; end

Expand Down Expand Up @@ -64,11 +63,4 @@ def ensure_super_admin
redirect_to npq_separation_admin_cohort_path(cohort)
end
end

def ensure_editable
unless @schedule.editable?
flash[:error] = "This schedule is not editable"
redirect_to npq_separation_admin_cohort_schedule_path(cohort, @schedule)
end
end
end
4 changes: 0 additions & 4 deletions app/models/cohort.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,4 @@ def registration_start_date_matches_start_year

errors.add(:registration_start_date, "year must match the start year") if registration_start_date.year != start_year
end

def editable?
registration_start_date.future? && statements.none? && declarations.none?
end
end
2 changes: 0 additions & 2 deletions app/models/schedule.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ class Schedule < ApplicationRecord
belongs_to :cohort
has_many :courses, through: :course_group

delegate :editable?, to: :cohort

normalizes :allowed_declaration_types, with: ->(value) { value.reject(&:blank?) }

validates :name, presence: true
Expand Down
6 changes: 2 additions & 4 deletions app/views/npq_separation/admin/cohorts/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,17 @@
end
%>

<% if current_admin.super_admin? && @cohort.editable? %>
<% if current_admin.super_admin? %>
<%= govuk_button_link_to('Edit cohort details', edit_npq_separation_admin_cohort_path(@cohort)) %>
<%= govuk_button_link_to('Delete cohort', npq_separation_admin_cohort_path(@cohort), method: :delete, warning: true) %>
<% end %>
<% if current_admin.super_admin? %>
<%= govuk_button_link_to('Create statements', new_npq_separation_admin_cohort_statement_path(@cohort), secondary: true) %>
<% end %>

<hr class="govuk-section-break govuk-section-break--l">

<h2 class="govuk-heading-m">Schedules</h1>

<% if current_admin.super_admin? && @cohort.editable? %>
<% if current_admin.super_admin? %>
<%= govuk_button_link_to('New schedule', new_npq_separation_admin_cohort_schedule_path(@cohort)) %>
<% end %>

Expand Down
2 changes: 1 addition & 1 deletion app/views/npq_separation/admin/schedules/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
end
%>

<% if current_admin.super_admin? && @schedule.editable? %>
<% if current_admin.super_admin? %>
<%= govuk_button_link_to 'Edit schedule details', edit_npq_separation_admin_cohort_schedule_path(@schedule.cohort, @schedule) %>
<%= govuk_button_link_to 'Delete schedule', npq_separation_admin_cohort_schedule_path(@schedule.cohort, @schedule), method: :delete, warning: true %>
<% end %>
68 changes: 23 additions & 45 deletions spec/features/npq_separation/admin/cohorts_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,54 +63,32 @@
expect(cohort.registration_start_date).to eq(Date.new(2029, 3, 2))
end

context "with an editable cohort" do
before do
allow_any_instance_of(Cohort).to receive(:editable?).and_return(true)
end

scenario "editing" do
cohort.update! funding_cap: false

navigate_to_cohort
click_on edit_button_text

fill_in "Start year", with: "2025"
check "Funding cap", visible: :all
fill_in "Day", with: "6"
fill_in "Month", with: "5"
fill_in "Year", with: "2025"

expect { click_on "Update cohort" }.not_to(change(Cohort, :count))
expect(page).to have_text("Cohort updated")

cohort.reload
expect(cohort.start_year).to be(2025)
expect(cohort.funding_cap).to be(true)
expect(cohort.registration_start_date.to_date).to eq(Date.new(2025, 5, 6))
end

scenario "deletion" do
navigate_to_cohort
click_on delete_button_text

expect { click_on "Confirm" }.to change(Cohort, :count).by(-1)
end
end
scenario "editing" do
cohort.update! funding_cap: false

navigate_to_cohort
click_on edit_button_text

fill_in "Start year", with: "2025"
check "Funding cap", visible: :all
fill_in "Day", with: "6"
fill_in "Month", with: "5"
fill_in "Year", with: "2025"

context "with a non-editable cohort" do
before do
allow_any_instance_of(Cohort).to receive(:editable?).and_return(false)
end
expect { click_on "Update cohort" }.not_to(change(Cohort, :count))
expect(page).to have_text("Cohort updated")

scenario "cannot edit" do
navigate_to_cohort
expect(page).not_to have_link(edit_button_text)
end
cohort.reload
expect(cohort.start_year).to be(2025)
expect(cohort.funding_cap).to be(true)
expect(cohort.registration_start_date.to_date).to eq(Date.new(2025, 5, 6))
end

scenario "deletion" do
navigate_to_cohort
click_on delete_button_text

scenario "cannot delete" do
navigate_to_cohort
expect(page).not_to have_link(delete_button_text)
end
expect { click_on "Confirm" }.to change(Cohort, :count).by(-1)
end
end

Expand Down
71 changes: 22 additions & 49 deletions spec/features/npq_separation/admin/schedules_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,63 +56,36 @@
admin.update! super_admin: true
end

context "when the cohort is editable" do
before do
allow_any_instance_of(Cohort).to receive(:editable?).and_return(true)
end

scenario "creation" do
visit_cohort
click_on new_button_text

fill_in_schedule_form(course_group)

expect { click_on "Create schedule" }.to change(Schedule, :count).by(1)
scenario "creation" do
visit_cohort
click_on new_button_text

schedule = Schedule.order(created_at: :desc).first
expect(page).to have_text("Schedule created")
expect_filled_in_schedule_attributes(schedule, course_group)
end
fill_in_schedule_form(course_group)

scenario "editing" do
navigate_to_schedule
click_on edit_button_text
fill_in_schedule_form(course_group)
expect { click_on "Create schedule" }.to change(Schedule, :count).by(1)

expect { click_on "Update schedule" }.not_to(change(Schedule, :count))
schedule = Schedule.order(created_at: :desc).first
expect(page).to have_text("Schedule created")
expect_filled_in_schedule_attributes(schedule, course_group)
end

schedule.reload
expect(page).to have_text("Schedule updated")
expect_filled_in_schedule_attributes(schedule, course_group)
end
scenario "editing" do
navigate_to_schedule
click_on edit_button_text
fill_in_schedule_form(course_group)

scenario "deletion" do
navigate_to_schedule
expect { click_on "Update schedule" }.not_to(change(Schedule, :count))

click_on delete_button_text
expect { click_on "Confirm" }.to change(Schedule, :count).by(-1)
end
schedule.reload
expect(page).to have_text("Schedule updated")
expect_filled_in_schedule_attributes(schedule, course_group)
end

context "when the cohort is not editable" do
before do
allow_any_instance_of(Cohort).to receive(:editable?).and_return(false)
end

scenario "cannot create" do
visit_cohort
expect(page).not_to have_link(new_button_text)
end

scenario "cannot edit" do
navigate_to_schedule
expect(page).not_to have_link(edit_button_text)
end

scenario "cannot delete" do
navigate_to_schedule
expect(page).not_to have_link(delete_button_text)
end
scenario "deletion" do
navigate_to_schedule

click_on delete_button_text
expect { click_on "Confirm" }.to change(Schedule, :count).by(-1)
end
end

Expand Down
28 changes: 0 additions & 28 deletions spec/models/cohort_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,32 +74,4 @@
end
end
end

describe "#editable?" do
subject { cohort.editable? }

context "when conditions are met" do
before { cohort.update! start_year: 2029, registration_start_date: Date.new(2029, 12, 31) }

it { is_expected.to be true }
end

context "when the cohort has declarations" do
before { create(:declaration, cohort:) }

it { is_expected.to be false }
end

context "when the cohort has statements" do
before { create(:statement, cohort:) }

it { is_expected.to be false }
end

context "when registration_start_date is not in the future" do
before { cohort.update! start_year: Time.zone.today.year, registration_start_date: Date.yesterday }

it { is_expected.to be false }
end
end
end
16 changes: 0 additions & 16 deletions spec/models/schedule_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,4 @@
it { is_expected.to belong_to(:course_group) }
it { is_expected.to belong_to(:cohort) }
end

describe "#editable?" do
subject { schedule.editable? }

context "when cohort#editable? is true" do
before { allow(schedule.cohort).to receive(:editable?).and_return(true) }

it { is_expected.to be true }
end

context "when cohort#editable? is false" do
before { allow(schedule.cohort).to receive(:editable?).and_return(false) }

it { is_expected.to be false }
end
end
end
Loading
Loading