diff --git a/app/assets/stylesheets/models/activities.css.scss b/app/assets/stylesheets/models/activities.css.scss index ee02238432..2a5eff6b3a 100644 --- a/app/assets/stylesheets/models/activities.css.scss +++ b/app/assets/stylesheets/models/activities.css.scss @@ -8,6 +8,10 @@ color: var(--d-on-surface-muted); } + .activity-title .mdi::before { + vertical-align: bottom; + } + th.type-icon, td.type-icon { width: 18px; diff --git a/app/controllers/activities_controller.rb b/app/controllers/activities_controller.rb index 690bf647e7..f34eb6b56c 100644 --- a/app/controllers/activities_controller.rb +++ b/app/controllers/activities_controller.rb @@ -25,6 +25,7 @@ class ActivitiesController < ApplicationController has_scope :by_description_languages, as: 'description_languages', type: :array has_scope :by_judge, as: 'judge_id' has_scope :by_popularities, as: 'popularity', type: :array + has_scope :is_draft, as: 'draft', type: :boolean has_scope :repository_scope, as: 'tab' do |controller, scope, value| course = Series.find(controller.params[:id]).course if controller.params[:id] diff --git a/app/controllers/pages_controller.rb b/app/controllers/pages_controller.rb index ba2f81766b..7e71a5a301 100644 --- a/app/controllers/pages_controller.rb +++ b/app/controllers/pages_controller.rb @@ -22,6 +22,8 @@ def home @homepage_series = courses.map { |c| c.homepage_series(current_user, 0) }.flatten.sort_by(&:deadline) @jump_back_in = current_user.jump_back_in + + @draft_exercises = current_user.draft_exercises.reorder(updated_at: :desc) if current_user.a_repository_admin? else set_metrics respond_to do |format| diff --git a/app/models/activity.rb b/app/models/activity.rb index 1541fecc4e..97b92ec465 100644 --- a/app/models/activity.rb +++ b/app/models/activity.rb @@ -22,6 +22,7 @@ # description_nl_present :boolean default(FALSE) # description_en_present :boolean default(FALSE) # series_count :integer default(0), not null +# draft :boolean default(TRUE) # require 'pathname' @@ -80,6 +81,7 @@ class Activity < ApplicationRecord scope :by_programming_language, ->(programming_language) { includes(:programming_language).where(programming_languages: { name: programming_language }) } scope :by_type, ->(type) { where(type: type) } scope :by_judge, ->(judge) { where(judge_id: judge) } + scope :is_draft, -> { where(draft: true) } scope :by_description_languages, lambda { |languages| by_language = all # allow chaining of scopes by_language = by_language.where(description_en_present: true) if languages.include? 'en' @@ -322,12 +324,15 @@ def accessible?(user, course) return true if user&.zeus? return false unless access_public? \ || repository.allowed_courses.pluck(:id).include?(course&.id) - return true if user&.member_of? course + return true if user&.course_admin? course + return false if draft? + return true if user&.member_of?(course) return false if course.moderated && access_private? course.open_for_user?(user) else return true if user&.repository_admin? repository + return false if draft? access_public? end diff --git a/app/models/content_page.rb b/app/models/content_page.rb index 5b40275d30..36aba73448 100644 --- a/app/models/content_page.rb +++ b/app/models/content_page.rb @@ -22,6 +22,7 @@ # description_nl_present :boolean default(FALSE) # description_en_present :boolean default(FALSE) # series_count :integer default(0), not null +# draft :boolean default(TRUE) # class ContentPage < Activity diff --git a/app/models/exercise.rb b/app/models/exercise.rb index 23027bb738..034b759d21 100644 --- a/app/models/exercise.rb +++ b/app/models/exercise.rb @@ -22,6 +22,7 @@ # description_nl_present :boolean default(FALSE) # description_en_present :boolean default(FALSE) # series_count :integer default(0), not null +# draft :boolean default(TRUE) # require 'pathname' diff --git a/app/models/submission.rb b/app/models/submission.rb index 2c81f58dbf..4da2e8a49f 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -518,6 +518,7 @@ def update_fs end def report_if_internal_error + return if exercise&.draft? return unless status_changed? && send(:'internal error?') ExceptionNotifier.notify_exception( diff --git a/app/models/user.rb b/app/models/user.rb index ccd437bd9c..35a07a9491 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -107,6 +107,13 @@ class User < ApplicationRecord has_many :repositories, through: :repository_admins, source: :repository + has_many :draft_exercises, + lambda { + where activities: + { draft: true } + }, + through: :repositories, + source: :exercises has_many :annotations, dependent: :restrict_with_error has_many :questions, dependent: :restrict_with_error diff --git a/app/policies/activity_policy.rb b/app/policies/activity_policy.rb index 87af4de6f8..ea813b34a5 100644 --- a/app/policies/activity_policy.rb +++ b/app/policies/activity_policy.rb @@ -4,7 +4,7 @@ def resolve if user&.zeus? scope.all else - scope.where(access: :public, status: :ok).or(scope.where(repository: user&.repositories)) + scope.where(access: :public, status: :ok, draft: false).or(scope.where(repository: user&.repositories)) end end end @@ -75,7 +75,9 @@ def read? def permitted_attributes if update? - %i[access name_nl name_en] + permitted_attributes = %i[access name_nl name_en] + permitted_attributes << :draft if record.draft? + permitted_attributes else [] end diff --git a/app/views/activities/_activities_table.html.erb b/app/views/activities/_activities_table.html.erb index d58291d577..e171667bf2 100644 --- a/app/views/activities/_activities_table.html.erb +++ b/app/views/activities/_activities_table.html.erb @@ -25,11 +25,8 @@ <% local_assigns[:activities].each do |activity| %> - <% if user_signed_in? %> - <% if current_user.repository_admin?(activity.repository) || current_user.course_admin?(@course) %> - <%= render partial: 'activities/repository_status', locals: {activity: activity, series: local_assigns[:series]} %> - <% end %> - <%= render partial: 'activities/user_status_icon', locals: {activity: activity, series: local_assigns[:series], user: user} %> + <% if user_signed_in? && (current_user.repository_admin?(activity.repository) || current_user.course_admin?(@course)) %> + <%= render partial: 'activities/repository_status', locals: {activity: activity, series: local_assigns[:series]} %> <% end %> @@ -38,7 +35,10 @@ - + + <% if user_signed_in? %> + <%= render partial: 'activities/user_status_icon', locals: {activity: activity, series: local_assigns[:series], user: user} %> + <% end %> <% if activity.accessible?(current_user, @course) %> <%= link_to activity.name, get_activity_path.call(activity) %> <% elsif activity.access_public? %> diff --git a/app/views/activities/_draft_notice.html.erb b/app/views/activities/_draft_notice.html.erb new file mode 100644 index 0000000000..aef08b3522 --- /dev/null +++ b/app/views/activities/_draft_notice.html.erb @@ -0,0 +1,14 @@ +<% if @activity.draft? %> +
+ + <%= t "activities.show.alert_draft_html" %> + <% if policy(@activity).edit? %> + <% edit_path = @series.present? ? + course_series_activity_path(@series&.course, @series, @activity, {activity: {draft: false}}) : + activity_path(@activity, {activity: {draft: false}}) + %> + <%= link_to t("activities.show.alert_draft_edit"), edit_path, method: :put %> + + <% end %> +
+<% end %> diff --git a/app/views/activities/_form.html.erb b/app/views/activities/_form.html.erb index 8e3a0b2ae1..310f41a821 100644 --- a/app/views/activities/_form.html.erb +++ b/app/views/activities/_form.html.erb @@ -1,7 +1,3 @@ -<% content_for :javascripts do %> - <% # TODO rename pack to activity %> - <% javascript_include_tag 'exercise' %> -<% end %> <%= form_for(activity.becomes(Activity), :url => activity_scoped_path(activity: activity, course: @course, series: @series), :html => {:class => 'form-horizontal'}) do |f| %> <% if activity.errors.any? %>
@@ -32,13 +28,10 @@
-
<%= link_to t(".open_on_github"), activity.github_url, target: '_blank', rel: 'noopener' %>
+
<%= link_to t(".open_on_github"), activity.github_url, target: '_blank', rel: 'noopener' %>
-
<%= link_to t(".info_description"), info_activity_scoped_path(activity: @activity, course: @course, series: @series), rel: 'noopener' %>
+
<%= link_to t(".info_description"), info_activity_scoped_path(activity: @activity, course: @course, series: @series), rel: 'noopener' %>
<% end %> - diff --git a/app/views/activities/_repository_status.html.erb b/app/views/activities/_repository_status.html.erb index 4e4c9f135e..e41a5f5ccb 100644 --- a/app/views/activities/_repository_status.html.erb +++ b/app/views/activities/_repository_status.html.erb @@ -6,3 +6,6 @@ <% elsif activity.not_valid? %> <% end %> +<% if activity.draft? %> + +<% end %> diff --git a/app/views/activities/_series_activities_add_table.html.erb b/app/views/activities/_series_activities_add_table.html.erb index 7459488a06..005a1b7624 100644 --- a/app/views/activities/_series_activities_add_table.html.erb +++ b/app/views/activities/_series_activities_add_table.html.erb @@ -29,9 +29,7 @@ <% end %> - <%= raw "" if activity.access_private? %> - <%= raw "" if activity.removed? %> - <%= raw "" if activity.not_valid? %> + <%= render partial: 'activities/repository_status', locals: { activity: activity } %> <%= activity_icon(activity) %> diff --git a/app/views/activities/edit.html.erb b/app/views/activities/edit.html.erb index 8520a4aa5f..a7c847da23 100644 --- a/app/views/activities/edit.html.erb +++ b/app/views/activities/edit.html.erb @@ -1,6 +1,7 @@ <%= render 'navbar_links' %>
+ <%= render partial: 'draft_notice' %>

diff --git a/app/views/activities/info.html.erb b/app/views/activities/info.html.erb index cf65ce6fb4..960020b9bc 100644 --- a/app/views/activities/info.html.erb +++ b/app/views/activities/info.html.erb @@ -9,6 +9,7 @@ %>
+ <%= render partial: 'draft_notice' %>
<% if @activity.exercise? && @activity.solutions.any? %> <%= t '.visibility_solution_warning' %> diff --git a/app/views/activities/show.html.erb b/app/views/activities/show.html.erb index b8945018fe..2eaeadf936 100644 --- a/app/views/activities/show.html.erb +++ b/app/views/activities/show.html.erb @@ -26,6 +26,8 @@ next_tooltip_actionable = next_activity ? t('.navigation.next_actionable') : t('.navigation.back_to_course_actionable') end %> +<%= render partial: 'draft_notice' %> +
<% if @activity.exercise? %>
diff --git a/app/views/pages/_user_card.html.erb b/app/views/pages/_user_card.html.erb index 2673cfc273..d398f4dc0b 100644 --- a/app/views/pages/_user_card.html.erb +++ b/app/views/pages/_user_card.html.erb @@ -15,6 +15,27 @@
+<% if @draft_exercises.present? %> +
+
+
<%= t ".draft-exercises" %>
+ <% @draft_exercises.first(5).each do |exercise| %> +

+ <%= link_to activity_submissions_path(exercise), class: 'btn-icon float-end' do %> + + <% end %> + <%= activity_icon exercise %> + <%= link_to exercise.name, activity_path(exercise), class: "course-link", title: exercise.name %> + <%= link_to exercise.repository.name, repository_path(exercise.repository), class: "small text-muted course-link", title: exercise.repository.name %> +

+ <% end %> + <% if @draft_exercises.count > 5 %> + <%= link_to t(".all-draft-exercises"), activities_path(draft: true) %> + <% end %> +
+
+<% end %> + <% deadlines = @homepage_series %> <% if deadlines.any? %>
diff --git a/config/locales/views/activities/en.yml b/config/locales/views/activities/en.yml index 35ae41da10..c2a93c19a0 100644 --- a/config/locales/views/activities/en.yml +++ b/config/locales/views/activities/en.yml @@ -67,6 +67,7 @@ en: mine: "My activities" featured: "Featured activities" all: "All activities" + draft-notice: "This activity is a draft and thus not visible for students." show: code: Code handin: Hand in @@ -97,6 +98,8 @@ en: preloaded_info: "We have preloaded your latest submission into the editor." preloaded_clear: Clear editor. preloaded_restore: Restore the initial code. + alert_draft_html: This activity is a draft and thus not visible for students. + alert_draft_edit: Publish activity. series_activities_add_table: course_added_to_usable: "Adding this exercise will allow this course to use all of the private exercises in this exercise's repository. Are you sure?" edit: diff --git a/config/locales/views/activities/nl.yml b/config/locales/views/activities/nl.yml index 2b5d2f1117..1b6773c2c7 100644 --- a/config/locales/views/activities/nl.yml +++ b/config/locales/views/activities/nl.yml @@ -67,6 +67,7 @@ nl: mine: "Mijn activiteiten" featured: "Uitgelichte activiteiten" all: "Alle activiteiten" + draft-notice: "Deze activiteit is nog een concept. Ze is niet zichtbaar voor studenten." show: code: Code handin: Indienen @@ -97,6 +98,8 @@ nl: preloaded_info: We hebben jouw laatste oplossing ingeladen in de editor. preloaded_clear: Maak de editor leeg. preloaded_restore: Zet de voorbeeldcode terug. + alert_draft_html: Deze oefening is nog een concept. Ze is niet zichtbaar voor studenten. + alert_draft_edit: Deze oefening publiceren. series_activities_add_table: course_added_to_usable: "Deze oefening toevoegen zal deze cursus toegang geven tot alle privé oefeningen in de repository van deze oefening. Ben je zeker?" edit: diff --git a/config/locales/views/pages/en.yml b/config/locales/views/pages/en.yml index 3aea74565e..e498fc15de 100644 --- a/config/locales/views/pages/en.yml +++ b/config/locales/views/pages/en.yml @@ -101,6 +101,8 @@ en: submissions: Submissions correct-exercises: Correct exercises recent-exercises: Recent exercises + draft-exercises: Draft exercises + all-draft-exercises: All draft exercises getting_started_card: title: Hi there, text: It looks like you're new here. To get started, register for a course. diff --git a/config/locales/views/pages/nl.yml b/config/locales/views/pages/nl.yml index bfe7e3bed6..c1eee849de 100644 --- a/config/locales/views/pages/nl.yml +++ b/config/locales/views/pages/nl.yml @@ -101,6 +101,8 @@ nl: submissions: Ingediende oplossingen correct-exercises: Correcte oefeningen recent-exercises: Recente oefeningen + draft-exercises: Ongepubliceerde oefeningen + all-draft-exercises: Alle ongepubliceerde oefeningen getting_started_card: title: Hallo, text: Welkom op Dodona! Registreer je voor een cursus om van start te gaan. diff --git a/db/migrate/20230922115708_add_draft_to_activities.rb b/db/migrate/20230922115708_add_draft_to_activities.rb new file mode 100644 index 0000000000..c68e15ef53 --- /dev/null +++ b/db/migrate/20230922115708_add_draft_to_activities.rb @@ -0,0 +1,5 @@ +class AddDraftToActivities < ActiveRecord::Migration[7.0] + def change + add_column :activities, :draft, :boolean, default: false + end +end diff --git a/db/migrate/20231026075353_make_draft_default_true.rb b/db/migrate/20231026075353_make_draft_default_true.rb new file mode 100644 index 0000000000..cbdb1a6b16 --- /dev/null +++ b/db/migrate/20231026075353_make_draft_default_true.rb @@ -0,0 +1,5 @@ +class MakeDraftDefaultTrue < ActiveRecord::Migration[7.1] + def change + change_column_default :activities, :draft, from: false, to: true + end +end diff --git a/db/schema.rb b/db/schema.rb index a501f8e4a3..c7a60a2e37 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_08_10_105908) do +ActiveRecord::Schema[7.1].define(version: 2023_10_26_075353) do create_table "active_storage_attachments", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| t.string "name", null: false t.string "record_type", null: false @@ -59,6 +59,7 @@ t.boolean "description_nl_present", default: false t.boolean "description_en_present", default: false t.integer "series_count", default: 0, null: false + t.boolean "draft", default: true t.index ["judge_id"], name: "index_activities_on_judge_id" t.index ["name_nl"], name: "index_activities_on_name_nl" t.index ["path", "repository_id"], name: "index_activities_on_path_and_repository_id", unique: true diff --git a/db/seeds.rb b/db/seeds.rb index b62c99253c..070f80080e 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -293,8 +293,11 @@ def fill_series_with_realistic_submissions(s) RepositoryAdmin.create(repository: activity_repo, user: zeus) RepositoryAdmin.create(repository: big_activity_repo, user: zeus) - contents_list = ContentPage.all.to_a - exercises_list = Exercise.all.to_a + # remove draft status from all activities except the first 5 + Activity.where.not(id: Activity.first(5)).update_all(draft: false) + + contents_list = ContentPage.where(draft: false).to_a + exercises_list = Exercise.where(draft: false).all.to_a puts "Add series, content pages, exercises, read states and submissions to courses (#{Time.now - start})" diff --git a/test/controllers/activities_controller_test.rb b/test/controllers/activities_controller_test.rb index 62ab509f36..d6630565c8 100644 --- a/test/controllers/activities_controller_test.rb +++ b/test/controllers/activities_controller_test.rb @@ -698,12 +698,144 @@ def create_exercises_return_valid assert_not resp['has_read'] end - test 'should be able to acess index when not signed in' do + test 'should be able to access index when not signed in' do sign_out @user get activities_url assert_response :success end + + test 'repo admin should be able to access own draft activities' do + sign_out @user + user = users(:staff) + sign_in user + repo = create :repository, :git_stubbed + repo.admins << user + exercise = create :exercise, :config_stubbed, repository: repo, draft: true + get activity_url(exercise) + + assert_response :success + end + + test 'repo admin should not be able to access other draft activities' do + sign_out @user + user = users(:staff) + sign_in user + repo = create :repository, :git_stubbed + repo.admins << user + repo2 = create :repository, :git_stubbed + exercise = create :exercise, :config_stubbed, repository: repo2, draft: true + get activity_url(exercise) + + assert_redirected_to root_url + end + + test 'course admin should be able to access draft activities in course' do + sign_out @user + staff = users(:staff) + course_admin = users(:student) + course = create :course, series_count: 1 + course.administrating_members << staff + course.administrating_members << course_admin + + exercise = create :exercise, :config_stubbed, draft: true + course.series.first.exercises << exercise + + sign_in staff + get course_activity_url(course, exercise) + + assert_response :success + sign_out staff + + sign_in course_admin + get course_activity_url(course, exercise) + + assert_response :success + end + + test 'course admin should not be able to access draft activities in other courses' do + sign_out @user + staff = users(:staff) + course_admin = users(:student) + course = create :course, series_count: 1 + course.administrating_members << staff + course.administrating_members << course_admin + + course2 = create :course, series_count: 1 + + exercise = create :exercise, :config_stubbed, draft: true + course.series.first.exercises << exercise + course2.series.first.exercises << exercise + + sign_in staff + get course_activity_url(course2, exercise) + + assert_redirected_to root_url + sign_out staff + + sign_in course_admin + get course_activity_url(course2, exercise) + + assert_redirected_to root_url + end + + test 'student should not be able to access draft activities in course' do + sign_out @user + student = users(:student) + course = create :course, series_count: 1 + course.enrolled_members << student + + exercise = create :exercise, :config_stubbed, draft: true + course.series.first.exercises << exercise + + sign_in student + get course_activity_url(course, exercise) + + assert_redirected_to root_url + end + + test 'repository admin should be able to publish draft activities' do + stub_all_activities! + sign_out @user + user = users(:staff) + sign_in user + repo = create :repository, :git_stubbed + repo.admins << user + exercise = create :exercise, repository: repo, draft: true + + put activity_url(exercise), params: { activity: { draft: false } } + + assert_not exercise.reload.draft + end + + test 'repository admin should not be able to reset an activity to draft' do + stub_all_activities! + sign_out @user + user = users(:staff) + sign_in user + repo = create :repository, :git_stubbed + repo.admins << user + exercise = create :exercise, repository: repo, draft: false + + put activity_url(exercise), params: { activity: { draft: true } } + + assert_not exercise.reload.draft + end + + test 'repository admin can update draft activity' do + stub_all_activities! + sign_out @user + user = users(:staff) + sign_in user + repo = create :repository, :git_stubbed + repo.admins << user + exercise = create :exercise, repository: repo, draft: true, name_en: 'old name' + + put activity_url(exercise), params: { activity: { draft: true, name_en: 'new name' } } + + assert exercise.reload.draft + assert_equal 'new name', exercise.name_en + end end class ExerciseErrorMailerTest < ActionDispatch::IntegrationTest diff --git a/test/factories/content_pages.rb b/test/factories/content_pages.rb index 64df42ed1f..fda8ed24ba 100644 --- a/test/factories/content_pages.rb +++ b/test/factories/content_pages.rb @@ -22,6 +22,7 @@ # description_nl_present :boolean default(FALSE) # description_en_present :boolean default(FALSE) # series_count :integer default(0), not null +# draft :boolean default(TRUE) # require "#{File.dirname(__FILE__)}/../testhelpers/stub_helper.rb" @@ -31,6 +32,7 @@ factory :content_page do access { 'public' } status { 'ok' } + draft { false } sequence(:path) { |n| "content_page#{n}" } diff --git a/test/factories/exercises.rb b/test/factories/exercises.rb index 1399693a9c..fc4317dae8 100644 --- a/test/factories/exercises.rb +++ b/test/factories/exercises.rb @@ -22,6 +22,7 @@ # description_nl_present :boolean default(FALSE) # description_en_present :boolean default(FALSE) # series_count :integer default(0), not null +# draft :boolean default(TRUE) # require "#{File.dirname(__FILE__)}/../testhelpers/stub_helper.rb" @@ -36,6 +37,7 @@ description_en_present { false } access { 'public' } status { 'ok' } + draft { false } sequence(:path) { |n| "exercise#{n}" } diff --git a/test/fixtures/exercises.yml b/test/fixtures/exercises.yml index 51040f7026..6b8341dbdd 100644 --- a/test/fixtures/exercises.yml +++ b/test/fixtures/exercises.yml @@ -22,6 +22,7 @@ # description_nl_present :boolean default(FALSE) # description_en_present :boolean default(FALSE) # series_count :integer default(0), not null +# draft :boolean default(TRUE) # python_exercise: @@ -36,3 +37,4 @@ python_exercise: type: "Exercise" access_token: "12345" repository_token: "67890" + draft: false diff --git a/test/models/activity_test.rb b/test/models/activity_test.rb index 6e3d02ff59..397e416b06 100644 --- a/test/models/activity_test.rb +++ b/test/models/activity_test.rb @@ -22,6 +22,7 @@ # description_nl_present :boolean default(FALSE) # description_en_present :boolean default(FALSE) # series_count :integer default(0), not null +# draft :boolean default(TRUE) # require 'test_helper' diff --git a/test/models/exercise_test.rb b/test/models/exercise_test.rb index 541f8a7571..c262d3be84 100644 --- a/test/models/exercise_test.rb +++ b/test/models/exercise_test.rb @@ -22,6 +22,7 @@ # description_nl_present :boolean default(FALSE) # description_en_present :boolean default(FALSE) # series_count :integer default(0), not null +# draft :boolean default(TRUE) # require 'test_helper'