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.

Sign up for GitHub

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

Correctly copy all series info when copying courses #6049

Merged
merged 4 commits into from
Jan 8, 2025
Merged

Correctly copy all series info when copying courses #6049

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
36f3847
Correctly copy all series info when copying courses
jorg-vr Jan 8, 2025
81b2a0b
Update test/controllers/courses_controller_test.rb
jorg-vr Jan 8, 2025
71f0017
Fix unfinished string
jorg-vr Jan 8, 2025
0758cef
Fully fix test
jorg-vr Jan 8, 2025
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
6 changes: 4 additions & 2 deletions app/controllers/courses_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ def create
deadlines: false
}.merge(@copy_options).symbolize_keys

@course.series = policy_scope(@copy_options[:base].series).map do |s|
@course.series = policy_scope(@copy_options[:base].series.reorder(id: :asc)).map do |s|
# rubocop:disable Style/MultilineTernaryOperator
Series.new(
series_memberships: @copy_options[:exercises] ?
Expand All @@ -169,7 +169,9 @@ def create
visibility_start: @copy_options[:hide_series] ? nil : s.visibility_start,
deadline: @copy_options[:deadlines] ? s.deadline : nil,
order: s.order,
progress_enabled: s.progress_enabled
progress_enabled: s.progress_enabled,
activities_visible: s.activities_visible,
activity_numbers_enabled: s.activity_numbers_enabled
)
# rubocop:enable Style/MultilineTernaryOperator
end
Expand Down
61 changes: 55 additions & 6 deletions test/controllers/courses_controller_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -747,18 +747,67 @@ def with_users_signed_in(users)
assert_includes course.subscribed_members, user
end

test 'a course copied by a regular student should not include hidden/closed series' do
add_students
user = @students.first
user.update(permission: :staff)
test "a course copied by staff who isn't course admin should not include hidden/closed series" do
user = create :user, permission: :staff
course = create :course
course.enrolled_members << user
create :series, course: course, visibility: :hidden
create :series, course: course, visibility: :closed
create :series, course: course, visibility: :timed, visibility_start: 1.day.from_now
create :series, course: course, visibility: :timed, visibility_start: 1.day.ago
create :series, course: course, visibility: :open

assert_equal 5, course.reload.series.count

sign_in user
new_course = build :course
post courses_url, params: { course: { name: new_course.name, description: new_course.description, visibility: new_course.visibility, registration: new_course.registration, teacher: new_course.teacher }, copy_options: { base_id: course.id }, format: :json }

assert_equal 2, Course.find(response.parsed_body['id']).series.count
end

test 'a copied course should contain all series with the same advanced settings' do
user = create :user, permission: :staff
course = create :course
_series = create :series, course: course, visibility: :hidden
course.administrating_members << user
create :series, course: course, progress_enabled: false, activities_visible: false, activity_numbers_enabled: true, name: 'foo'
create :series, course: course, progress_enabled: true, activities_visible: true, activity_numbers_enabled: false, name: 'bar'

sign_in user
new_course = build :course
post courses_url, params: { course: { name: new_course.name, description: new_course.description, visibility: new_course.visibility, registration: new_course.registration, teacher: new_course.teacher }, copy_options: { base_id: course.id }, format: :json }
new_course = Course.find(response.parsed_body['id'])

assert_equal 0, Course.find(response.parsed_body['id']).series.count
assert_equal 2, new_course.series.count

new_course.series.zip(course.series).each do |series, original_series|
assert_equal original_series.name, series.name
assert_equal original_series.progress_enabled, series.progress_enabled
assert_equal original_series.activities_visible, series.activities_visible
assert_equal original_series.activity_numbers_enabled, series.activity_numbers_enabled
end
end

test 'a copied course should maintain the same series order' do
user = create :user, permission: :staff
course = create :course
course.administrating_members << user
create :series, course: course, name: 'second'
create :series, course: course, name: 'first'
create :series, course: course, name: 'third', order: 1
create :series, course: course, name: 'fifth', order: 3
create :series, course: course, name: 'fourth', order: 2

sign_in user
new_course = build :course
post courses_url, params: { course: { name: new_course.name, description: new_course.description, visibility: new_course.visibility, registration: new_course.registration, teacher: new_course.teacher }, copy_options: { base_id: course.id }, format: :json }
new_course = Course.find(response.parsed_body['id'])

assert_equal course.series.pluck(:name), new_course.series.pluck(:name)
new_course.series.zip(course.series).each do |series, original_series|
assert_equal original_series.name, series.name
assert_equal original_series.order, series.order
end
end

test 'hidden course page shown to unsubscribed student should include registration url with secret' do
Expand Down
Loading