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

Show warning about inaccessible activities to course admins on course show #3152

Merged
merged 7 commits into from
Oct 12, 2021
Merged
Show file tree
Hide file tree
Changes from 4 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: 7 additions & 1 deletion app/controllers/courses_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,13 @@ def show
redirect_unless_secret_correct
return if performed?
end
if current_user&.course_admin?(@course) && [email protected]_activities_accessible?
flash[:alert] = I18n.t('courses.show.has_private_exercises')
flash[:extra] = {
chvp marked this conversation as resolved.
Show resolved Hide resolved
'message' => I18n.t('courses.show.has_private_help'),
'url' => contact_url
}
end
@title = @course.name
@series = policy_scope(@course.series).includes(:evaluation)
@series_loaded = params[:secret].present? ? @course.series.count : 2
Expand Down Expand Up @@ -134,7 +141,6 @@ def create

respond_to do |format|
if @course.save
flash[:alert] = I18n.t('courses.create.added_private_exercises') unless @course.exercises.where(access: :private).count.zero?
@course.administrating_members << current_user unless @course.administrating_members.include?(current_user)
format.html { redirect_to @course, notice: I18n.t('controllers.created', model: Course.model_name.human) }
format.json { render :show, status: :created, location: @course }
Expand Down
1 change: 0 additions & 1 deletion app/models/activity.rb
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,6 @@ def accessible?(user, course)
else
return false unless course.visible_activities.pluck(:id).include? id
end
return true if user&.repository_admin? repository
return false unless access_public? \
|| repository.allowed_courses.pluck(:id).include?(course&.id)
return true if user&.member_of? course
Expand Down
4 changes: 4 additions & 0 deletions app/models/course.rb
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,10 @@ def secret_required?(user = nil)
true
end

def all_activities_accessible?
activities.where(access: :private).where.not(repository_id: usable_repositories).count.zero?
end

def invalidate_subscribed_members_count_cache
Rails.cache.delete(format(SUBSCRIBED_MEMBERS_COUNT_CACHE_STRING, id: id))
end
Expand Down
4 changes: 2 additions & 2 deletions config/locales/views/courses/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ en:
other: "You can't remove a course with more than %{count} submissions yourself."
contact_html: "<a href=\"%{contact_url}\">Contact us</a> if you want to delete the course anyway."
show:
has_private_exercises: "This course uses private exercises that it hasn't been granted the rights to."
has_private_help: "Contact us if you don't know how to solve this."
course: Course
hidden_show_link: "Secret link"
visibility-visible_for_all-info: "This course is visible for everyone: everyone can access this course from the course overview, and the contents are visible for everyone."
Expand Down Expand Up @@ -175,8 +177,6 @@ en:
exercise_count: Exercises
content_count: Reading activities
more_statistics: "More statistics >"
create:
added_private_exercises: "Private exercises were added while creating this course. To use these exercises, this course will need to be granted permission from the relevant repositories."
copy_courses_table:
info: "Course"
users: "# users"
Expand Down
6 changes: 3 additions & 3 deletions config/locales/views/courses/nl.yml
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ nl:
other: "Je kan een cursus met meer dan %{count} oplossingen niet zelf verwijderen."
contact_html: "<a href=\"%{contact_url}\">Contacteer ons</a> als je de cursus toch wilt verwijderen."
show:
has_private_exercises: "Deze cursus gebruikt privé oefeningen waartoe die geen rechten heeft."
chvp marked this conversation as resolved.
Show resolved Hide resolved
has_private_help: "Contacteer ons als je niet weet hoe je dit kan oplossen."
course: Cursus
hidden_show_link: 'Geheime link'
visibility-visible_for_all-info: 'Deze cursus is zichtbaar voor iedereen: ze wordt opgelijst in het cursusoverzicht en de inhoud is toegankelijk voor iedereen.'
Expand Down Expand Up @@ -171,8 +173,6 @@ nl:
content_count: Leesactiviteiten
submitted_solutions: Ingediende oplossingen
more_statistics: "Meer statistieken >"
create:
added_private_exercises: "Er werden privé oefeningen toegevoegd tijdens het aanmaken van deze cursus. Om deze oefeningen te kunnen gebruiken zal deze cursus toestemming moeten krijgen van de relevante repository's."
copy_courses_table:
info: "Cursus"
users: "# gebruikers"
Expand Down Expand Up @@ -204,7 +204,7 @@ nl:
enable: Schakel vragen in.
disable: Schakel vragen uit.
ical:
serie_deadline: "Deadline voor %{serie_name} van cursus %{course_name}: %{serie_url}"
serie_deadline: "Deadline voor %{serie_name} van cursus %{course_name}: %{serie_url}"
submissions:
show:
questions:
Expand Down
7 changes: 7 additions & 0 deletions test/controllers/courses_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@ class CoursesControllerTest < ActionDispatch::IntegrationTest

test_crud_actions

test 'should render with inaccessible activities' do
@instance.series << create(:series)
@instance.series.first.activities << create(:exercise, access: :private)
get course_url(@instance)
assert_response :success
end

test 'should reset token' do
old_secret = @instance.secret
post reset_token_course_url(@instance)
Expand Down
11 changes: 11 additions & 0 deletions test/models/course_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -249,4 +249,15 @@ class CourseTest < ActiveSupport::TestCase
assert course.destroy
assert_equal code, submission.reload.code
end

test 'all_activities_accessible? should be correct' do
course = create :course, series_count: 1, exercises_per_series: 0
ex = create :exercise, access: :public
course.series.first.exercises << ex
assert course.all_activities_accessible?
ex.update(access: :private)
assert_not course.all_activities_accessible?
course.usable_repositories << ex.repository
assert course.all_activities_accessible?
end
end