Merge branch 'main' into feature/feedback-table-rework

.github/workflows/test.yml

@@ -39,9 +39,6 @@ jobs:
       with:
         node-version: 16.x
         cache: yarn
-    - name: Install JS dependencies
-      run: |
-        yarn install
     - name: Run tests
       env:
         CI: true
@@ -51,8 +48,6 @@ jobs:
         git config --global user.name "Dodona"
         sudo sysctl fs.inotify.max_user_watches=524288
         sudo sysctl -p
-        bin/rake css:build
-        yarn build:js
         bundle exec rails test
     - name: Upload coverage to Codecov
       uses: codecov/codecov-action@v3
@@ -114,9 +109,6 @@ jobs:
         cache: yarn
     - name: Setup chromium-chromedriver
       uses: nanasess/setup-chromedriver@master
-    - name: Install JS dependencies
-      run: |
-        yarn install
     - name: Show installed chrome version
       run: |
         google-chrome --version

Gemfile

@@ -124,15 +124,15 @@ gem 'rubyzip', '~> 2.3.2'
 gem 'dalli', '~> 3.2.6'
 
 # Generate 'random' values like usernames, emails, ...
-gem 'faker', '~> 3.2.1'
+gem 'faker', '~> 3.2.2'
 
 # Profiling
 gem 'flamegraph', '~> 0.9.5'
 gem 'memory_profiler', '~> 1.0.1'
 gem 'rack-mini-profiler', '~> 3.1.1'
 gem 'stackprof', '~> 0.2.25'
 
-gem 'ddtrace', '~> 1.15.0'
+gem 'ddtrace', '~> 1.16.0'
 
 # Make sure filesystem changes only happen at the end of a transaction
 gem 'after_commit_everywhere', '~> 1.3.1'
@@ -154,7 +154,7 @@ group :development, :test do
 
   # Adds support for Capybara system testing and selenium driver
   gem 'capybara', '~> 3.39.2'
-  gem 'selenium-webdriver', '~> 4.14.0'
+  gem 'selenium-webdriver', '~> 4.15.0'
 end
 
 group :test do
@@ -193,4 +193,4 @@ gem 'tzinfo-data', platforms: %i[mingw mswin x64_mingw jruby]
 gem 'docker-api', '~> 2.2.0'
 
 # Used for syncing deadlines with an external calendar
-gem 'icalendar', '~> 2.9'
+gem 'icalendar', '~> 2.10'

Gemfile.lock

@@ -142,11 +142,11 @@ GEM
       railties (>= 6.0.0)
     daemons (1.4.1)
     dalli (3.2.6)
-    datadog-ci (0.2.0)
+    datadog-ci (0.3.0)
       msgpack
     date (3.3.3)
-    ddtrace (1.15.0)
-      datadog-ci (~> 0.2.0)
+    ddtrace (1.16.0)
+      datadog-ci (~> 0.3.0)
       debase-ruby_core_source (= 3.2.2)
       libdatadog (~> 5.0.0.1.0)
       libddwaf (~> 1.14.0.0.0)
@@ -187,7 +187,7 @@ GEM
     factory_bot_rails (6.2.0)
       factory_bot (~> 6.2.0)
       railties (>= 5.0.0)
-    faker (3.2.1)
+    faker (3.2.2)
       i18n (>= 1.8.11, < 2)
     faraday (2.7.4)
       faraday-net_http (>= 2.0, < 3.1)
@@ -217,7 +217,7 @@ GEM
     i18n-js (4.2.3)
       glob (>= 0.4.0)
       i18n
-    icalendar (2.9.0)
+    icalendar (2.10.0)
       ice_cube (~> 0.16)
     ice_cube (0.16.4)
     image_processing (1.12.2)
@@ -467,7 +467,7 @@ GEM
       ffi (~> 1.12)
     ruby2_keywords (0.0.5)
     rubyzip (2.3.2)
-    selenium-webdriver (4.14.0)
+    selenium-webdriver (4.15.0)
       rexml (~> 3.2, >= 3.2.5)
       rubyzip (>= 1.2.2, < 3.0)
       websocket (~> 1.0)
@@ -572,7 +572,7 @@ DEPENDENCIES
   counter_culture (~> 3.5)
   cssbundling-rails (~> 1.3.3)
   dalli (~> 3.2.6)
-  ddtrace (~> 1.15.0)
+  ddtrace (~> 1.16.0)
   delayed_job_active_record (~> 4.1.8)
   delayed_job_web (~> 1.4.4)
   devise (~> 4.9.3)
@@ -581,13 +581,13 @@ DEPENDENCIES
   ed25519
   exception_notification (~> 4.5.0)
   factory_bot_rails (~> 6.2.0)
-  faker (~> 3.2.1)
+  faker (~> 3.2.2)
   flamegraph (~> 0.9.5)
   has_scope (~> 0.8.2)
   hcaptcha (~> 7.1.0)
   httparty (~> 0.21.0)
   i18n-js (~> 4.2.3)
-  icalendar (~> 2.9)
+  icalendar (~> 2.10)
   image_processing (~> 1.12.2)
   jbuilder (~> 2.11.5)
   jsbundling-rails (~> 1.2.1)
@@ -625,7 +625,7 @@ DEPENDENCIES
   rubocop-rails (~> 2.22.1)
   ruby-saml (~> 1.16.0)
   rubyzip (~> 2.3.2)
-  selenium-webdriver (~> 4.14.0)
+  selenium-webdriver (~> 4.15.0)
   simplecov (~> 0.22.0)
   simplecov-cobertura (~> 2.1.0)
   slack-notifier (~> 2.4.0)

app/assets/javascripts/editor.ts

@@ -1,5 +1,5 @@
 import { closeBrackets, closeBracketsKeymap, autocompletion } from "@codemirror/autocomplete";
-import { defaultKeymap, history, historyKeymap } from "@codemirror/commands";
+import { defaultKeymap, history, historyKeymap, indentWithTab } from "@codemirror/commands";
 import {
     bracketMatching,
     foldGutter,
@@ -132,6 +132,7 @@ const editorSetup = (() => [
         ...defaultKeymap,
         ...historyKeymap,
         ...foldKeymap,
+        indentWithTab
     ]),
     syntaxHighlighting(rougeStyle, {
         fallback: true

app/controllers/lti_controller.rb

@@ -36,14 +36,19 @@ def do_redirect
   end
 
   def content_selection
+    return head :unauthorized unless current_user&.a_course_admin?
+
     @supported = @lti_message.accept_types.include?(LTI::Messages::Types::DeepLinkingResponse::LtiResourceLink::TYPE)
-    @grouped_courses = policy_scope(Course.all).group_by(&:year)
+    @grouped_courses = current_user.administrating_courses.group_by(&:year)
     @multiple = @lti_message.accept_multiple
   end
 
   def series_and_activities
     # Eager load the activities
     @course = Course.includes(series: [:activities]).find_by(id: params[:id])
+
+    return head :unauthorized unless current_user&.admin_of?(@course)
+
     @series = policy_scope(@course.series)
     @multiple = ActiveModel::Type::Boolean.new.cast(params[:multiple])
   end

app/javascript/packs/application_pack.js

@@ -37,10 +37,8 @@ import { userState } from "state/Users";
 // Initialize clipboard.js
 initClipboard();
 
-// Don't show drawer if we don't want a drawer.
-if (!window.dodona.hideDrawer) {
-    ready.then(() => new Drawer());
-}
+// Init drawer
+ready.then(() => new Drawer());
 
 ready.then(initTooltips);
 

app/jobs/remove_activities_job.rb

@@ -0,0 +1,46 @@
+class RemoveActivitiesJob < ApplicationJob
+  # permanently remove activities that match all of the following criteria:
+  # - status is 'removed'
+  # - updated_at is more than 1 month ago
+  # - not part of an evaluation
+  # - one of the following is true:
+  #   - draft is true (never published)
+  #   - series_memberships is empty and less then 25 submissions and latest submission is more than 1 month ago
+  #
+  # Destroy is called on each activity individually to ensure that callbacks are run
+  # This means the activity will be removed from any series, evaluations it is a member of
+  # and any submissions will be removed
+  queue_as :cleaning
+
+  def perform
+    ContentPage.where(status: 'removed').where('updated_at < ?', 1.month.ago).find_each do |activity|
+      if activity.draft? || activity.series_memberships.empty?
+        # destroy series memberships first explicitly, as they are dependent: :restrict_with_error
+        activity.series_memberships.destroy_all
+
+        activity.destroy
+      end
+    end
+
+    Exercise.where(status: 'removed').where('updated_at < ?', 1.month.ago).find_each do |activity|
+      unless activity.draft?
+        next if activity.series_memberships.present?
+        next if activity.submissions.count >= 25
+        next if activity.submissions.present? && activity.submissions.reorder(:created_at).last.created_at > 1.month.ago
+      end
+
+      next if EvaluationExercise.exists?(exercise_id: activity.id)
+
+      # destroy submissions first explicitly, as they are dependent: :restrict_with_error
+      activity.submissions.destroy_all
+
+      # destroy series memberships first explicitly, as they are dependent: :restrict_with_error
+      activity.series_memberships.destroy_all
+
+      activity.destroy
+    end
+
+    # rerun this job in 1 month
+    RemoveActivitiesJob.set(wait: 1.month).perform_later
+  end
+end

app/models/activity.rb

@@ -500,7 +500,7 @@ def deep_merge_configs(parent_conf, child_conf)
   def lowercase_labels(hash)
     return unless hash
 
-    hash['labels'] = hash['labels'].map(&:downcase).uniq if hash.key? 'labels'
+    hash['labels'] = hash['labels'].map(&:to_s).map(&:downcase).uniq if hash.key? 'labels'
     hash
   end
 

package.json

@@ -31,7 +31,7 @@
     "babel-plugin-macros": "^3.1.0",
     "bootstrap": "5.3.2",
     "clipboard": "^2.0.11",
-    "core-js": "^3.33.1",
+    "core-js": "^3.33.2",
     "d3": "^7.8.5",
     "dayjs": "^1.11.10",
     "dragula": "^3.7.3",
@@ -49,25 +49,25 @@
     "webpack-cli": "^5.1.4",
     "@codemirror/autocomplete": "^6.10.2",
     "@codemirror/commands": "^6.3.0",
-    "@codemirror/view": "^6.21.4",
+    "@codemirror/view": "^6.22.0",
     "@codemirror/state": "^6.3.1",
     "@codemirror/language": "^6.9.2",
     "@codemirror/language-data": "^6.3.1",
     "@lezer/common": "^1.0.4",
     "@lezer/highlight": "^1.1.6",
-    "@lezer/lr": "^1.3.13",
+    "@lezer/lr": "^1.3.14",
     "codemirror-lang-r": "^0.1.0-2",
     "codemirror-lang-prolog": "^0.1.0",
     "@replit/codemirror-lang-csharp": "^6.2.0"
   },
   "devDependencies": {
-    "@open-wc/testing-helpers": "^2.3.0",
+    "@open-wc/testing-helpers": "^2.3.2",
     "@testing-library/dom": "^9.3.3",
     "@testing-library/user-event": "^14.5.1",
     "@typescript-eslint/eslint-plugin": "^5.62.0",
     "@typescript-eslint/parser": "^5.62.0",
     "babel-plugin-istanbul": "^6.1.1",
-    "eslint": "^8.52.0",
+    "eslint": "^8.53.0",
     "eslint-config-google": "^0.14.0",
     "eslint-plugin-jest": "^27.6.0",
     "eslint-plugin-lit": "^1.10.1",
@@ -78,7 +78,7 @@
     "nyc": "^15.1.0",
     "stylelint": "^15.11.0",
     "stylelint-config-standard": "^34.0.0",
-    "stylelint-config-standard-scss": "^11.0.0",
+    "stylelint-config-standard-scss": "^11.1.0",
     "ts-jest": "^26.5.6",
     "webpack-bundle-analyzer": "^4.9.1"
   }

test/controllers/lti_controller_test.rb

@@ -18,10 +18,14 @@ def setup
   end
 
   test 'content selection shows courses' do
+    user = create :staff
     courses = create_list :course, 2
+    user.administrating_courses << courses
     payload = lti_payload('nonce', 'target', 'LtiDeepLinkingRequest')
     id_token = encode_jwt(payload)
 
+    sign_in user
+
     get content_selection_path, params: {
       id_token: id_token,
       provider_id: @provider.id
@@ -33,6 +37,41 @@ def setup
     end
   end
 
+  test 'content selection should not show courses that are not administrated by the user' do
+    user = create :staff
+    courses = create_list :course, 2
+    payload = lti_payload('nonce', 'target', 'LtiDeepLinkingRequest')
+    id_token = encode_jwt(payload)
+
+    sign_in user
+
+    get content_selection_path, params: {
+      id_token: id_token,
+      provider_id: @provider.id
+    }
+
+    assert_response :ok
+    courses.each do |course|
+      assert_select 'option', { text: course.name, count: 0 }
+    end
+  end
+
+  test 'content selection should return unauthorized if the user is not administrating any courses' do
+    user = create :student
+    create_list :course, 2
+    payload = lti_payload('nonce', 'target', 'LtiDeepLinkingRequest')
+    id_token = encode_jwt(payload)
+
+    sign_in user
+
+    get content_selection_path, params: {
+      id_token: id_token,
+      provider_id: @provider.id
+    }
+
+    assert_response :unauthorized
+  end
+
   test 'content selection payload is correct' do
     series = create :series, exercise_count: 1
     payload = lti_payload('nonce', 'target', 'LtiDeepLinkingRequest')