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

Copy courses between providers in Support console #4750

Merged
merged 12 commits into from
Dec 10, 2024
2 changes: 2 additions & 0 deletions app/components/tab_navigation.html.erb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
<%= content_for :tab_navigation_actions %>

<nav class="app-tab-navigation govuk-!-margin-bottom-7" aria-label="Sub navigation">
<ul class="app-tab-navigation__list">
<% items.each do |item| %>
Expand Down
35 changes: 35 additions & 0 deletions app/controllers/support/providers/copy_courses_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# frozen_string_literal: true

module Support
module Providers
class CopyCoursesController < SupportController
before_action :recruitment_cycle

def new
@target_provider = Provider.find(params[:provider_id])
@copy_courses_form = CopyCoursesForm.new(@target_provider)
end

def create
@target_provider = Provider.find(params[:provider_id])
@copy_courses_form = CopyCoursesForm.new(@target_provider, recruitment_cycle.providers.find_by(provider_code: params[:course][:autocompleted_provider_code]))

if @copy_courses_form.valid?
sites_copy_to_course = params[:schools] ? Sites::CopyToCourseService : ->(*) {}

copier = ::Courses::CopyToProviderService.new(sites_copy_to_course:, enrichments_copy_to_course: Enrichments::CopyToCourseService.new, force: true)

Provider.transaction do
@copy_courses_form.provider.courses.map do |course|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the PR description it mentions "Copy all courses or some courses" but here is all courses. Can we remove the "some courses" from description just to avoid confusion?

I assume the copying specific courses would be done on a later interaction when we will have a support ticket?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I've not implemented "some courses" because unwanted courses can be deleted

copier.execute(course:, new_provider: @copy_courses_form.target_provider)
end
end

redirect_to support_recruitment_cycle_provider_path(recruitment_cycle.year, @copy_courses_form.target_provider.id)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any indication everything went sucessfully? Should we add one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screencast.from.10-12-24.10.47.38.webm

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll change it to redirect to the courses tab too

else
render :new
end
end
end
end
end
21 changes: 21 additions & 0 deletions app/forms/support/copy_courses_form.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# frozen_string_literal: true

module Support
class CopyCoursesForm
include ActiveModel::Model

attr_reader :target_provider, :provider

def initialize(target_provider, provider = nil)
@target_provider = target_provider
@provider = provider
end

validates :target_provider, :provider, presence: true
validate :different_providers

def different_providers
errors.add(:provider, message: 'Choose different providers') if target_provider == provider
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ const providerSuggestionTemplate = (result) => result && `${result.provider_name
const onConfirm = (input) => (option) => (input.value = option ? option.id : '')

function init () {
const recruitmentCycleYear = document.getElementById('accredited_provider_search_form_recruitment_cycle_year').value
const recruitmentCycleYear = document.getElementById('accredited_provider_search_form_recruitment_cycle_year')?.value
if (!recruitmentCycleYear) return
const options = {
path: `/api/${recruitmentCycleYear}/accredited_provider_suggestions`,
template: {
Expand Down
3 changes: 3 additions & 0 deletions app/views/support/courses/index.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
<%= render PageTitle.new(title: "support.providers.courses.index") %>
<% content_for :tab_navigation_actions do %>
<%= govuk_link_to "Copy Courses", new_support_recruitment_cycle_provider_copy_course_path(@recruitment_cycle.year, @provider), class: "govuk-button" %>
<% end %>

<table class="govuk-table">
<thead class="govuk-table__head">
Expand Down
45 changes: 45 additions & 0 deletions app/views/support/providers/copy_courses/new.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<%= render PageTitle.new(title: "support.providers.copy_courses.new", has_errors: @copy_courses_form.errors.present?) %>

<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds-from-desktop">
<div>
<%= form_with model: @copy_courses_form, url: support_recruitment_cycle_provider_copy_courses_path(@recruitment_cycle.year, @target_provider) do |form| %>
<div class="govuk-grid-row">
<div class="govuk-grid-column-full">
<%= form.govuk_error_summary %>
</div>
</div>
<h1 class="govuk-heading-l"><%= @target_provider.provider_name %></h1>
<h2 class="govuk-heading-m">Copy courses from:</h2>
<div class="govuk-grid-row">
<div class="govuk-grid-column-full">
<div class="govuk-form-group">
<%= form.govuk_text_field :provider,
id: "provider",
value: params[:query],
class: "govuk-input",
data: { qa: "provider-search" },
label: -> { tag.div("Search for an organisation to copy courses from:") + tag.div("Enter the name or provider code", class: %w[govuk-hint govuk-!-display-inline]) } %>
<div id="provider-autocomplete"></div>
</div>
</div>
</div>

<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds">
<div class="govuk-form-group">
<%= form.govuk_check_box :schools, true, false, label: { text: "Copy placement schools?" }, hint: { text: tag.div("Any placement schools that are not already linked to the target provider will not be copied", class: %w[govuk-hint govuk-!-font-size-16]) }, multiple: false %>
</div>
</div>
</div>

<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds">
<%= form.govuk_submit "Copy courses" %>
</div>
</div>
<% end %>
</div>
</div>
</div>
</div>
4 changes: 4 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,8 @@ en:
show: "View a provider"
edit: "Edit a provider"
edit_contact: "Edit provider contact details"
copy_courses:
new: Copy courses
courses:
index: "Courses"
edit: "Edit course details"
Expand Down Expand Up @@ -1028,6 +1030,8 @@ en:
blank: "Enter a first name"
last_name:
blank: "Enter a last name"
support/copy_courses_form:
blank: Provider can't be blank
support/user_form:
attributes:
email:
Expand Down
2 changes: 2 additions & 0 deletions config/routes/support.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@
get '/check', on: :collection, to: 'accredited_providers/checks#show'
put '/check', on: :collection, to: 'accredited_providers/checks#update'
end

resources :copy_courses, only: %i[new create]
end
end
resources :users do
Expand Down
31 changes: 31 additions & 0 deletions spec/forms/support/copy_course_form_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# frozen_string_literal: true

require 'rails_helper'

module Support
describe CopyCoursesForm, type: :model do
subject { described_class.new(target_provider, provider) }

let(:provider) { create(:provider) }

describe 'validations' do
let(:target_provider) { provider }

context 'unique provider' do
it 'is not valid' do
expect(subject).not_to be_valid
expect(subject.errors.messages[:provider]).to include('Choose different providers')
end
end

context 'provider must be present' do
let(:target_provider) { nil }

it 'is not valid' do
expect(subject).not_to be_valid
expect(subject.errors.messages[:target_provider]).to include("Provider can't be blank")
end
end
end
end
end
41 changes: 41 additions & 0 deletions spec/requests/support/copy_courses_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# frozen_string_literal: true

require 'rails_helper'

RSpec.describe 'Support::CopyCourses' do
include DfESignInUserHelper
let(:user) { create(:user, :admin) }
let(:source_provider) { create(:provider, courses: create_list(:course, 3, :with_full_time_sites)) }
let!(:target_provider) { create(:provider, sites: source_provider.courses.flat_map(&:sites)) }
let!(:year) { find_or_create(:recruitment_cycle).year }

before { host! URI(Settings.base_url).host }

describe 'GET new' do
it 'responds with 200' do
login_user(user)
get "/support/2025/providers/#{source_provider.id}/copy_courses/new"
expect(response).to have_http_status(:ok)
end
end

describe 'POST create' do
context 'with schools' do
it 'copies the courses with schools' do
login_user(user)
post "/support/#{year}/providers/#{target_provider.id}/copy_courses", params: { 'course[autocompleted_provider_code]' => source_provider.provider_code, schools: '1' }
expect(target_provider.reload.courses.length).to eq(3)
courses = target_provider.reload.courses
expect(courses.flat_map(&:sites).length).to eq(3)
end
end

context 'without schools' do
it 'copies the course without schools' do
login_user(user)
post "/support/#{year}/providers/#{target_provider.id}/copy_courses", params: { 'course[autocompleted_provider_code]' => source_provider.provider_code }
expect(target_provider.reload.courses.length).to eq(3)
end
end
end
end
40 changes: 40 additions & 0 deletions spec/system/support/providers/copy_courses_to_provider_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# frozen_string_literal: true

require 'spec_helper'

RSpec.describe 'Support', service: :publish, skip: 'can not get chromedriver working on Alpine' do
include DfESignInUserHelper

let(:courses) do
[
create(:course, :unpublished, :with_full_time_sites),
create(:course, :published, :with_full_time_sites),
create(:course, :withdrawn, :with_full_time_sites)
]
end
let!(:source_provider) { create(:provider, provider_name: 'Source Provider', courses:) }
let!(:target_provider) { create(:provider, provider_name: 'Target Provider') }
let(:user) { create(:user, :admin) }

before do
sign_in_system_test(user:)
end

it 'copy courses from one provider to another', :js do
visit '/support'
click_on 'Target Provider'
click_on 'Courses'
click_on 'Copy Courses'
autocomplete = page.find('input#provider')
autocomplete.set(source_provider.provider_code)
sleep 3
li = page.find('ul#provider__listbox li', visible: false)
li.click
page.find_by_id('schools-true-field', visible: false).click
click_on 'Copy courses'
click_on 'Courses'
courses.map(&:name).each do |course_name|
expect(page).to have_content(course_name)
end
end
end
Loading