From ee1dfbb0450edebfbd863a3a463262da54af381b Mon Sep 17 00:00:00 2001 From: Charlotte Van Petegem Date: Fri, 8 Oct 2021 16:27:56 +0200 Subject: [PATCH 1/7] Show warning about inaccessible activities to course admins on course show --- app/controllers/courses_controller.rb | 2 +- app/models/course.rb | 4 ++++ config/locales/views/courses/en.yml | 3 +-- config/locales/views/courses/nl.yml | 5 ++--- test/models/course_test.rb | 11 +++++++++++ 5 files changed, 19 insertions(+), 6 deletions(-) diff --git a/app/controllers/courses_controller.rb b/app/controllers/courses_controller.rb index dfb8d9fafd..7ca4e817e4 100644 --- a/app/controllers/courses_controller.rb +++ b/app/controllers/courses_controller.rb @@ -47,6 +47,7 @@ def show redirect_unless_secret_correct return if performed? end + flash[:alert] = I18n.t('courses.show.has_private_exercises') if current_user&.course_admin?(@course) && !@course.all_activities_accessible? @title = @course.name @series = policy_scope(@course.series).includes(:evaluation) @series_loaded = params[:secret].present? ? @course.series.count : 2 @@ -134,7 +135,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 } diff --git a/app/models/course.rb b/app/models/course.rb index a98676dea3..eb98f95db0 100644 --- a/app/models/course.rb +++ b/app/models/course.rb @@ -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 diff --git a/config/locales/views/courses/en.yml b/config/locales/views/courses/en.yml index c8f3db212d..11c628025b 100644 --- a/config/locales/views/courses/en.yml +++ b/config/locales/views/courses/en.yml @@ -88,6 +88,7 @@ en: other: "You can't remove a course with more than %{count} submissions yourself." contact_html: "Contact us 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." 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." @@ -175,8 +176,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" diff --git a/config/locales/views/courses/nl.yml b/config/locales/views/courses/nl.yml index a9266d9651..928ffcbdf5 100644 --- a/config/locales/views/courses/nl.yml +++ b/config/locales/views/courses/nl.yml @@ -96,6 +96,7 @@ nl: other: "Je kan een cursus met meer dan %{count} oplossingen niet zelf verwijderen." contact_html: "Contacteer ons als je de cursus toch wilt verwijderen." show: + has_private_exercises: "Deze cursus gebruikt privé oefeningen waartoe die geen rechten heeft." 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.' @@ -171,8 +172,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" @@ -204,7 +203,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: diff --git a/test/models/course_test.rb b/test/models/course_test.rb index 65973fe5bb..b9c9046f2b 100644 --- a/test/models/course_test.rb +++ b/test/models/course_test.rb @@ -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 From 4373d6da3877a9dc4b4412eaedd4d339e2bed9f8 Mon Sep 17 00:00:00 2001 From: Charlotte Van Petegem Date: Sun, 10 Oct 2021 18:55:04 +0200 Subject: [PATCH 2/7] Link to contact form in warning --- app/controllers/courses_controller.rb | 8 +++++++- config/locales/views/courses/en.yml | 1 + config/locales/views/courses/nl.yml | 1 + 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/app/controllers/courses_controller.rb b/app/controllers/courses_controller.rb index 7ca4e817e4..2646691ac7 100644 --- a/app/controllers/courses_controller.rb +++ b/app/controllers/courses_controller.rb @@ -47,7 +47,13 @@ def show redirect_unless_secret_correct return if performed? end - flash[:alert] = I18n.t('courses.show.has_private_exercises') if current_user&.course_admin?(@course) && !@course.all_activities_accessible? + if current_user&.course_admin?(@course) && !@course.all_activities_accessible? + flash[:alert] = I18n.t('courses.show.has_private_exercises') + flash[:extra] = { + '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 diff --git a/config/locales/views/courses/en.yml b/config/locales/views/courses/en.yml index 11c628025b..4a96d6bfaa 100644 --- a/config/locales/views/courses/en.yml +++ b/config/locales/views/courses/en.yml @@ -89,6 +89,7 @@ en: contact_html: "Contact us 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." diff --git a/config/locales/views/courses/nl.yml b/config/locales/views/courses/nl.yml index 928ffcbdf5..1425e09661 100644 --- a/config/locales/views/courses/nl.yml +++ b/config/locales/views/courses/nl.yml @@ -97,6 +97,7 @@ nl: contact_html: "Contacteer ons als je de cursus toch wilt verwijderen." show: has_private_exercises: "Deze cursus gebruikt privé oefeningen waartoe die geen rechten heeft." + 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.' From 2dc9c2f3d95565b95cefe0386f05418ae783cadc Mon Sep 17 00:00:00 2001 From: Charlotte Van Petegem Date: Mon, 11 Oct 2021 08:51:18 +0200 Subject: [PATCH 3/7] Add basic test for rendering with inaccessible exercises --- test/controllers/courses_controller_test.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/controllers/courses_controller_test.rb b/test/controllers/courses_controller_test.rb index c332f5b4eb..0617b57823 100644 --- a/test/controllers/courses_controller_test.rb +++ b/test/controllers/courses_controller_test.rb @@ -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) From abc9db5cc3a300aec81d270d4270d82c3b11ccae Mon Sep 17 00:00:00 2001 From: Charlotte Van Petegem Date: Mon, 11 Oct 2021 10:51:38 +0200 Subject: [PATCH 4/7] Make inaccessible exercises in courses inaccessible to course admins as well --- app/models/activity.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/models/activity.rb b/app/models/activity.rb index 8a640304e4..d73fefe262 100644 --- a/app/models/activity.rb +++ b/app/models/activity.rb @@ -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 From 236e6ec0b1fd562b9c6a705ccd1f55ad0809500b Mon Sep 17 00:00:00 2001 From: Charlotte Van Petegem Date: Mon, 11 Oct 2021 11:42:07 +0200 Subject: [PATCH 5/7] Update config/locales/views/courses/nl.yml Co-authored-by: Niko Strijbol --- config/locales/views/courses/nl.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/locales/views/courses/nl.yml b/config/locales/views/courses/nl.yml index 1425e09661..202e1f0536 100644 --- a/config/locales/views/courses/nl.yml +++ b/config/locales/views/courses/nl.yml @@ -96,7 +96,7 @@ nl: other: "Je kan een cursus met meer dan %{count} oplossingen niet zelf verwijderen." contact_html: "Contacteer ons als je de cursus toch wilt verwijderen." show: - has_private_exercises: "Deze cursus gebruikt privé oefeningen waartoe die geen rechten heeft." + has_private_exercises: "Deze cursus gebruikt privé-oefeningen waartoe die geen rechten heeft." has_private_help: "Contacteer ons als je niet weet hoe je dit kan oplossen." course: Cursus hidden_show_link: 'Geheime link' From 133b38c9d1d4cfff6cbea0751b213460be5156a2 Mon Sep 17 00:00:00 2001 From: Charlotte Van Petegem Date: Mon, 11 Oct 2021 11:45:39 +0200 Subject: [PATCH 6/7] Use `flash.now` --- app/controllers/courses_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/courses_controller.rb b/app/controllers/courses_controller.rb index 2646691ac7..84bf9fb2b9 100644 --- a/app/controllers/courses_controller.rb +++ b/app/controllers/courses_controller.rb @@ -48,8 +48,8 @@ def show return if performed? end if current_user&.course_admin?(@course) && !@course.all_activities_accessible? - flash[:alert] = I18n.t('courses.show.has_private_exercises') - flash[:extra] = { + flash.now[:alert] = I18n.t('courses.show.has_private_exercises') + flash.now[:extra] = { 'message' => I18n.t('courses.show.has_private_help'), 'url' => contact_url } From f3bfa2b3938a09f8a7c26d8ab9cef011a25ab939 Mon Sep 17 00:00:00 2001 From: Charlotte Van Petegem Date: Tue, 12 Oct 2021 09:46:43 +0200 Subject: [PATCH 7/7] Add warning per inaccessible exercise with quicklink to fix if possible --- app/controllers/repositories_controller.rb | 4 ++-- app/views/activities/_series_activities_table.html.erb | 10 ++++++++++ config/locales/views/activities/en.yml | 3 +++ config/locales/views/activities/nl.yml | 3 +++ 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/app/controllers/repositories_controller.rb b/app/controllers/repositories_controller.rb index cb474ba282..095bf7e834 100644 --- a/app/controllers/repositories_controller.rb +++ b/app/controllers/repositories_controller.rb @@ -197,12 +197,12 @@ def model_update_response(success, model, return_path) toast = t('controllers.updated', model: model.model_name.human) format.json { head :no_content } format.js { render locals: { toast: toast } } - format.html { redirect_to return_path, notice: toast } + format.html { redirect_back fallback_location: return_path, notice: toast } else alert = t('controllers.update_failed', model: model.model_name.human) format.json { head :unprocessable_entity } format.js { render status: :bad_request, locals: { toast: alert } } - format.html { redirect_to return_path, alert: alert } + format.html { redirect_back fallback_location: return_path, alert: alert } end end end diff --git a/app/views/activities/_series_activities_table.html.erb b/app/views/activities/_series_activities_table.html.erb index 98816c6af0..47aba42e1c 100644 --- a/app/views/activities/_series_activities_table.html.erb +++ b/app/views/activities/_series_activities_table.html.erb @@ -60,6 +60,16 @@ <%= link_to activity.name, activity_path(activity) %> <% else %> <%= activity.name %> + <% if current_user.course_admin?(@course) && current_user.repository_admin?(activity.repository) %> + <%= form_with url: add_course_repository_path(activity.repository) do |form| %> + <%= form.hidden_field :course_id, value: @course.id %> + + <% end %> + <% elsif current_user.course_admin?(@course) %> + " data-bs-toggle="tooltip" data-bs-placement="right"> + <% end %> <% end %> diff --git a/config/locales/views/activities/en.yml b/config/locales/views/activities/en.yml index 589e93b85c..775ef89520 100644 --- a/config/locales/views/activities/en.yml +++ b/config/locales/views/activities/en.yml @@ -117,3 +117,6 @@ en: config_info: 'Take a look at the %{doc_site} for more information about configuring exercises.' doc_site: 'documentation site' activity_invalid: This exercise's configuration is invalid. + series_activities_table: + course_has_no_rights_with_fix: This course is not allowed to use this private exercise. Click here to grant this course those rights. + course_has_no_rights_with_contact: This course is not allowed to use this private exercise. Contact the author to grant rights or remove this exercise from the course. diff --git a/config/locales/views/activities/nl.yml b/config/locales/views/activities/nl.yml index ecb9bd1126..a08cc81ffd 100644 --- a/config/locales/views/activities/nl.yml +++ b/config/locales/views/activities/nl.yml @@ -118,3 +118,6 @@ nl: config_info: 'Neem een kijkje op de %{doc_site} voor meer informatie over het configureren van oefeningen.' doc_site: 'documentatiesite' activity_invalid: Deze oefening heeft geen geldige configuratie. + series_activities_table: + course_has_no_rights_with_fix: Deze cursus heeft niet de rechten om deze privé-oefening te gebruiken. Klik hier om deze rechten te geven. + course_has_no_rights_with_contact: Deze cursus heeft niet de rechten om deze privé-oefening te gebruiken. Contacteer de auteur of verwijder deze oefening uit de cursus.