<%= t('lti.course_not_found') %>
+<%= t('lti.course_not_set_up') %>
+diff --git a/.github/workflows/test_ci.yml b/.github/workflows/test_ci.yml
index 3a425ce1e6..f302b922e8 100644
--- a/.github/workflows/test_ci.yml
+++ b/.github/workflows/test_ci.yml
@@ -43,10 +43,49 @@ jobs:
steps:
- name: Checkout repo
uses: actions/checkout@v4
- - name: Install system dependencies
- run: |
- sudo apt-get update
- sudo apt-get -yqq install libpq-dev cmake ghostscript pandoc imagemagick libmagickwand-dev git libgl1 tesseract-ocr pandoc poppler-utils
+ - name: Install and cache system dependencies
+ uses: awalsh128/cache-apt-pkgs-action@latest
+ with:
+ packages: |
+ libpq-dev
+ cmake
+ ghostscript
+ pandoc
+ imagemagick
+ libmagickwand-dev
+ git
+ libgl1
+ tesseract-ocr
+ pandoc
+ poppler-utils
+ fonts-liberation
+ libasound2
+ libatk-bridge2.0-0
+ libatk1.0-0
+ libatspi2.0-0
+ libcairo2
+ libcups2
+ libdbus-1-3
+ libdrm2
+ libegl1
+ libgbm1
+ libglib2.0-0
+ libgtk-3-0
+ libnspr4
+ libnss3
+ libpango-1.0-0
+ libx11-6
+ libx11-xcb1
+ libxcb1
+ libxcomposite1
+ libxdamage1
+ libxext6
+ libxfixes3
+ libxrandr2
+ libxshmfence1
+ version: 1.0
+ # Packages 'fonts-liberation' and onward are needed for playwright's chromium installation.
+ # The list was taken from: https://github.com/microsoft/playwright/blob/main/packages/playwright-core/src/server/registry/nativeDeps.ts#L37-L63
- name: Set up ruby and cache gems
uses: ruby/setup-ruby@v1
with:
@@ -86,7 +125,6 @@ jobs:
python3.10 -m venv venv
./venv/bin/pip install -r requirements-jupyter.txt -r requirements-scanner.txt
./venv/bin/playwright install chromium
- ./venv/bin/playwright install-deps chromium
- name: Configure server
run: |
sudo rm -f /etc/localtime
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 7de877eb60..19f0d6c978 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -1,6 +1,6 @@
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
- rev: v5.0.0
+ rev: v6.0.0
hooks:
- id: check-illegal-windows-names
- id: check-json
@@ -17,12 +17,12 @@ repos:
- id: end-of-file-fixer
- id: trailing-whitespace
- repo: https://github.com/rbubley/mirrors-prettier
- rev: v3.6.2
+ rev: v3.7.3
hooks:
- id: prettier
types_or: [javascript, jsx, css, scss, html]
- repo: https://github.com/thibaudcolas/pre-commit-stylelint
- rev: v16.21.1
+ rev: v16.26.1
hooks:
- id: stylelint
additional_dependencies: [
@@ -39,7 +39,7 @@ repos:
app/assets/stylesheets/common/_reset.scss
)$
- repo: https://github.com/rubocop/rubocop
- rev: v1.77.0
+ rev: v1.81.7
hooks:
- id: rubocop
args: ["--autocorrect"]
@@ -52,7 +52,7 @@ repos:
Vagrantfile
)$
additional_dependencies:
- - rubocop-rails:2.27.0
+ - rubocop-rails:2.33.4
- rubocop-performance:1.23.0
- rubocop-factory_bot:2.26.1
- rubocop-rspec:3.2.0
diff --git a/.rubocop.yml b/.rubocop.yml
index 3241e00a86..68812dd020 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -1,10 +1,10 @@
require:
- - rubocop-rails
- rubocop-performance
- rubocop-factory_bot
- rubocop-rspec_rails
- rubocop-capybara
plugins:
+ - rubocop-rails
- rubocop-rspec
AllCops:
@@ -15,7 +15,7 @@ AllCops:
- 'Vagrantfile'
NewCops: enable
SuggestExtensions: false
- TargetRubyVersion: 2.7
+ TargetRubyVersion: 3.1
Capybara/ClickLinkOrButtonStyle: # TODO: Enable and fix errors after reviewing system tests
Enabled: false
@@ -187,6 +187,9 @@ Rails/RequireDependency:
Rails/SkipsModelValidations:
Enabled: false
+Rails/StrongParametersExpect: # TODO: Enable this one and fix all issues
+ Enabled: false
+
Rails/UniqueValidationWithoutIndex:
Enabled: false
diff --git a/Changelog.md b/Changelog.md
index ccfddbec05..9376e000f6 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -1,5 +1,46 @@
# Changelog
+## [v2.9.0]
+
+### β¨ New features and improvements
+- Added touch event support for PDF and image annotations in grading view (#7736)
+- Added datetime-based visibility scheduling for assessments with `visible_on` and `visible_until` columns (#7697)
+- Added frontend UI for assignment visibility scheduling with three visibility options and section-specific overrides (#7717)
+- Added new loading spinner icon for tables (#7602)
+- Added functionality to apply bonuses and penalties as a percentage of the student's earned marks to ExtraMark model (#7702)
+- Switched to consistent Font Awesome chevrons for expander icons (#7713)
+- Install Ruby-LSP to allow development inside different IDEs such as VSCode (#7718)
+- Ensure only instructors and admins can link course, as LMS launch MarkUs button made available for all users (#7714)
+- Include student number in roster sync from Canvas (#7731)
+- Add API endpoint `add_test_run` that allows independent user submissions of test executions to MarkUs (#7730)
+- Display timeout status for autotest runs in the Test Results table. (#7734)
+- Assign extra marks in test definition. (Currently limited to pytest files) (#7728)
+- Enable zip downloads of test results (#7733)
+- Create rake task to remove orphaned end users (#7741)
+- Enable scanned assignments the ability to add inactive students (#7737)
+
+### π Bug fixes
+- Fix name column search in graders table (#7693)
+- Check against mtime instead of atime for clean up of git repo directories in tmp folder (#7706)
+- Update Model: Fix level validation checks through use of a custom validator (#7696)
+- Fixed test group results table to display `extra_info` field from all test groups (#7710)
+- Fixed syncing grades with Canvas to not include inactive students (#7759)
+
+### π§ Internal changes
+- Updated Github Actions CI to use cache-apt-pkgs to speed up workflow runs (#7645)
+- Converted "Create Group" functionality to React modal (#7663)
+- Added tests to improve coverage for `AnnotationCategory`'s `self.to_json` method
+- Added tests to the Criteria Controller class to achieve full test coverage
+- Refactored Criterion subclasses to remove redundant code
+- Converted "Rename Group" functionality to React modal (#7673)
+- Fixed Rack deprecation warnings by updating HTTP status code symbols (#7675)
+- Refactored "Reuse Groups" functionality to use React modal and relocated button to action box row (#7688)
+- Updated pre-commit `rubocop-rails` version to 2.33.4 (#7691)
+- Refactored MarksPanel child components and converted the components into hook-based function components
+- Refactored jQuery active marks panel component tracking logic into React
+- Updated the course summary table to use `@tanstack/react-table` v8 (#7732)
+- Refactored `test_run_table.jsx` by extracting nested components into separate files (#7739)
+
## [v2.8.2]
### β¨ New features and improvements
@@ -27,11 +68,10 @@
### π§ Internal changes
- Updated the assignment summary table to use `@tanstack/react-table` v8 (#7630)
+- Updated Github Actions CI to use cache-apt-pkgs to speed up workflow runs (#7645)
## [v2.8.0]
-### π¨ Breaking changes
-
### β¨ New features and improvements
- Improved layout and labeling in the assignment settings form for both standard and timed assessments. (#7531)
- Design improvement of tables when the data is empty. (#7557)
diff --git a/Gemfile b/Gemfile
index 508913d260..6a066f2b82 100644
--- a/Gemfile
+++ b/Gemfile
@@ -8,7 +8,7 @@ source 'https://rubygems.org'
# Bundler requires these gems in all environments
gem 'puma'
-gem 'rails', '~> 8.0.2.1'
+gem 'rails', '~> 8.0.3'
gem 'sprockets'
gem 'sprockets-rails'
@@ -37,7 +37,7 @@ gem 'histogram'
# Internationalization
gem 'i18n'
gem 'i18n-js'
-gem 'rails-i18n', '~> 8.0.1'
+gem 'rails-i18n', '~> 8.0.2'
# Redis
gem 'redis', '~> 5.4.1'
@@ -46,7 +46,7 @@ gem 'redis', '~> 5.4.1'
gem 'combine_pdf'
gem 'prawn'
gem 'prawn-qrcode'
-gem 'rmagick', '~> 6.1.2'
+gem 'rmagick', '~> 6.1.4'
gem 'rtesseract'
# Ruby miscellany
@@ -85,6 +85,10 @@ group :development do
gem 'listen' # to listen for changes in i18n-js files
end
+group :development_extra, optional: true do
+ gem 'ruby-lsp', require: false
+end
+
group :test do
gem 'factory_bot_rails'
gem 'fuubar'
@@ -106,7 +110,7 @@ group :development, :test do
gem 'capybara'
gem 'debug', '>= 1.0.0'
gem 'i18n-tasks', require: false
- gem 'rspec-rails', '~> 8.0.1'
+ gem 'rspec-rails', '~> 8.0.2'
gem 'selenium-webdriver'
end
diff --git a/Gemfile.lock b/Gemfile.lock
index 10666f9f2a..e85dbeb45d 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -3,29 +3,29 @@ GEM
specs:
action_policy (0.7.5)
ruby-next-core (>= 1.0)
- actioncable (8.0.2.1)
- actionpack (= 8.0.2.1)
- activesupport (= 8.0.2.1)
+ actioncable (8.0.3)
+ actionpack (= 8.0.3)
+ activesupport (= 8.0.3)
nio4r (~> 2.0)
websocket-driver (>= 0.6.1)
zeitwerk (~> 2.6)
- actionmailbox (8.0.2.1)
- actionpack (= 8.0.2.1)
- activejob (= 8.0.2.1)
- activerecord (= 8.0.2.1)
- activestorage (= 8.0.2.1)
- activesupport (= 8.0.2.1)
+ actionmailbox (8.0.3)
+ actionpack (= 8.0.3)
+ activejob (= 8.0.3)
+ activerecord (= 8.0.3)
+ activestorage (= 8.0.3)
+ activesupport (= 8.0.3)
mail (>= 2.8.0)
- actionmailer (8.0.2.1)
- actionpack (= 8.0.2.1)
- actionview (= 8.0.2.1)
- activejob (= 8.0.2.1)
- activesupport (= 8.0.2.1)
+ actionmailer (8.0.3)
+ actionpack (= 8.0.3)
+ actionview (= 8.0.3)
+ activejob (= 8.0.3)
+ activesupport (= 8.0.3)
mail (>= 2.8.0)
rails-dom-testing (~> 2.2)
- actionpack (8.0.2.1)
- actionview (= 8.0.2.1)
- activesupport (= 8.0.2.1)
+ actionpack (8.0.3)
+ actionview (= 8.0.3)
+ activesupport (= 8.0.3)
nokogiri (>= 1.8.5)
rack (>= 2.2.4)
rack-session (>= 1.0.1)
@@ -33,42 +33,42 @@ GEM
rails-dom-testing (~> 2.2)
rails-html-sanitizer (~> 1.6)
useragent (~> 0.16)
- actiontext (8.0.2.1)
- actionpack (= 8.0.2.1)
- activerecord (= 8.0.2.1)
- activestorage (= 8.0.2.1)
- activesupport (= 8.0.2.1)
+ actiontext (8.0.3)
+ actionpack (= 8.0.3)
+ activerecord (= 8.0.3)
+ activestorage (= 8.0.3)
+ activesupport (= 8.0.3)
globalid (>= 0.6.0)
nokogiri (>= 1.8.5)
- actionview (8.0.2.1)
- activesupport (= 8.0.2.1)
+ actionview (8.0.3)
+ activesupport (= 8.0.3)
builder (~> 3.1)
erubi (~> 1.11)
rails-dom-testing (~> 2.2)
rails-html-sanitizer (~> 1.6)
- activejob (8.0.2.1)
- activesupport (= 8.0.2.1)
+ activejob (8.0.3)
+ activesupport (= 8.0.3)
globalid (>= 0.3.6)
activejob-status (1.0.1)
activejob (>= 6.0)
activesupport (>= 6.0)
- activemodel (8.0.2.1)
- activesupport (= 8.0.2.1)
+ activemodel (8.0.3)
+ activesupport (= 8.0.3)
activemodel-serializers-xml (1.0.3)
activemodel (>= 5.0.0.a)
activesupport (>= 5.0.0.a)
builder (~> 3.1)
- activerecord (8.0.2.1)
- activemodel (= 8.0.2.1)
- activesupport (= 8.0.2.1)
+ activerecord (8.0.3)
+ activemodel (= 8.0.3)
+ activesupport (= 8.0.3)
timeout (>= 0.4.0)
- activestorage (8.0.2.1)
- actionpack (= 8.0.2.1)
- activejob (= 8.0.2.1)
- activerecord (= 8.0.2.1)
- activesupport (= 8.0.2.1)
+ activestorage (8.0.3)
+ actionpack (= 8.0.3)
+ activejob (= 8.0.3)
+ activerecord (= 8.0.3)
+ activesupport (= 8.0.3)
marcel (~> 1.0)
- activesupport (8.0.2.1)
+ activesupport (8.0.3)
base64
benchmark (>= 0.3)
bigdecimal
@@ -88,12 +88,12 @@ GEM
execjs (~> 2)
awesome_print (1.9.2)
base64 (0.3.0)
- benchmark (0.4.1)
+ benchmark (0.5.0)
better_errors (2.10.1)
erubi (>= 1.0.0)
rack (>= 0.9.0)
rouge (>= 1.0.0)
- bigdecimal (3.2.2)
+ bigdecimal (3.3.1)
binding_of_caller (1.0.1)
debug_inspector (>= 1.2.0)
bootsnap (1.18.6)
@@ -102,7 +102,7 @@ GEM
racc
browser (6.2.0)
builder (3.3.0)
- bullet (8.0.8)
+ bullet (8.1.0)
activesupport (>= 3.0.0)
uniform_notifier (~> 1.11)
capybara (3.40.0)
@@ -122,10 +122,10 @@ GEM
config (5.6.1)
deep_merge (~> 1.2, >= 1.2.1)
ostruct
- connection_pool (2.5.3)
+ connection_pool (2.5.4)
cookies_eu (1.7.8)
js_cookie_rails (~> 2.2.0)
- crack (1.0.0)
+ crack (1.0.1)
bigdecimal
rexml
crass (1.0.6)
@@ -177,11 +177,11 @@ GEM
dry-initializer (~> 3.2)
dry-schema (~> 1.14)
zeitwerk (~> 2.6)
- erb (5.0.1)
+ erb (5.0.2)
erubi (1.13.1)
et-orbi (1.2.11)
tzinfo
- exception_notification (5.0.0)
+ exception_notification (5.0.1)
actionmailer (>= 7.1, < 9)
activesupport (>= 7.1, < 9)
execjs (2.10.0)
@@ -200,9 +200,9 @@ GEM
rspec-core (~> 3.0)
ruby-progressbar (~> 1.4)
glob (0.4.0)
- globalid (1.2.1)
+ globalid (1.3.0)
activesupport (>= 6.1)
- hashdiff (1.2.0)
+ hashdiff (1.2.1)
highline (3.1.2)
reline
histogram (0.2.4.1)
@@ -222,7 +222,7 @@ GEM
rainbow (>= 2.2.2, < 4.0)
ruby-progressbar (~> 1.8, >= 1.8.1)
terminal-table (>= 1.5.1)
- io-console (0.8.0)
+ io-console (0.8.1)
irb (1.15.2)
pp (>= 0.6.0)
rdoc (>= 4.0.0)
@@ -234,10 +234,11 @@ GEM
railties (>= 3.1)
jsbundling-rails (1.3.1)
railties (>= 6.0.0)
- json (2.13.2)
+ json (2.16.0)
jwt (2.10.1)
base64
kgio (2.11.4)
+ language_server-protocol (3.17.0.5)
listen (3.9.0)
rb-fsevent (~> 0.10, >= 0.10.3)
rb-inotify (~> 0.9, >= 0.9.10)
@@ -251,17 +252,17 @@ GEM
net-imap
net-pop
net-smtp
- marcel (1.0.4)
+ marcel (1.1.0)
matrix (0.4.2)
mini_mime (1.1.5)
mini_portile2 (2.8.9)
- minitest (5.25.5)
+ minitest (5.26.0)
mono_logger (1.1.2)
msgpack (1.8.0)
multi_json (1.15.0)
mustermann (3.0.4)
ruby2_keywords (~> 0.0.1)
- net-imap (0.5.8)
+ net-imap (0.5.11)
date
net-protocol
net-pop (0.1.2)
@@ -270,8 +271,8 @@ GEM
timeout
net-smtp (0.5.1)
net-protocol
- nio4r (2.7.4)
- nokogiri (1.18.9)
+ nio4r (2.7.5)
+ nokogiri (1.18.10)
mini_portile2 (~> 2.8.2)
racc (~> 1.4)
observer (0.1.2)
@@ -280,8 +281,8 @@ GEM
ast (~> 2.4.1)
racc
pdf-core (0.10.0)
- pg (1.6.0)
- pkg-config (1.6.2)
+ pg (1.6.2)
+ pkg-config (1.6.5)
pluck_to_hash (1.0.2)
activerecord (>= 4.0.2)
activesupport (>= 4.0.2)
@@ -295,11 +296,12 @@ GEM
prawn (>= 1)
rqrcode (>= 1.0.0)
prettyprint (0.2.0)
+ prism (1.6.0)
psych (5.2.6)
date
stringio
public_suffix (6.0.2)
- puma (6.6.1)
+ puma (7.1.0)
nio4r (~> 2.0)
raabro (1.4.0)
racc (1.8.1)
@@ -318,20 +320,20 @@ GEM
rack (>= 1.3)
rackup (2.2.1)
rack (>= 3)
- rails (8.0.2.1)
- actioncable (= 8.0.2.1)
- actionmailbox (= 8.0.2.1)
- actionmailer (= 8.0.2.1)
- actionpack (= 8.0.2.1)
- actiontext (= 8.0.2.1)
- actionview (= 8.0.2.1)
- activejob (= 8.0.2.1)
- activemodel (= 8.0.2.1)
- activerecord (= 8.0.2.1)
- activestorage (= 8.0.2.1)
- activesupport (= 8.0.2.1)
+ rails (8.0.3)
+ actioncable (= 8.0.3)
+ actionmailbox (= 8.0.3)
+ actionmailer (= 8.0.3)
+ actionpack (= 8.0.3)
+ actiontext (= 8.0.3)
+ actionview (= 8.0.3)
+ activejob (= 8.0.3)
+ activemodel (= 8.0.3)
+ activerecord (= 8.0.3)
+ activestorage (= 8.0.3)
+ activesupport (= 8.0.3)
bundler (>= 1.15.0)
- railties (= 8.0.2.1)
+ railties (= 8.0.3)
rails-controller-testing (1.0.5)
actionpack (>= 5.0.1.rc1)
actionview (>= 5.0.1.rc1)
@@ -343,20 +345,21 @@ GEM
rails-html-sanitizer (1.6.2)
loofah (~> 2.21)
nokogiri (>= 1.15.7, != 1.16.7, != 1.16.6, != 1.16.5, != 1.16.4, != 1.16.3, != 1.16.2, != 1.16.1, != 1.16.0.rc1, != 1.16.0)
- rails-i18n (8.0.1)
+ rails-i18n (8.0.2)
i18n (>= 0.7, < 2)
railties (>= 8.0.0, < 9)
rails_performance (1.4.2)
browser
railties
redis
- railties (8.0.2.1)
- actionpack (= 8.0.2.1)
- activesupport (= 8.0.2.1)
+ railties (8.0.3)
+ actionpack (= 8.0.3)
+ activesupport (= 8.0.3)
irb (~> 1.13)
rackup (>= 1.0.0)
rake (>= 12.2)
thor (~> 1.0, >= 1.2.2)
+ tsort (>= 0.2)
zeitwerk (~> 2.6)
rainbow (3.1.1)
raindrops (0.20.0)
@@ -364,6 +367,8 @@ GEM
rb-fsevent (0.11.2)
rb-inotify (0.10.1)
ffi (~> 1.0)
+ rbs (3.9.5)
+ logger
rdoc (6.14.2)
erb
psych (>= 4.0.0)
@@ -375,7 +380,7 @@ GEM
redis-namespace (1.11.0)
redis (>= 4)
regexp_parser (2.9.0)
- reline (0.6.1)
+ reline (0.6.2)
io-console (~> 0.5)
responders (3.1.1)
actionpack (>= 5.2)
@@ -390,8 +395,8 @@ GEM
redis (>= 3.3)
resque (>= 1.27)
rufus-scheduler (~> 3.2, != 3.3)
- rexml (3.4.1)
- rmagick (6.1.2)
+ rexml (3.4.4)
+ rmagick (6.1.4)
observer (~> 0.1)
pkg-config (~> 1.4)
rouge (4.1.3)
@@ -407,7 +412,7 @@ GEM
rspec-mocks (3.13.5)
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.13.0)
- rspec-rails (8.0.1)
+ rspec-rails (8.0.2)
actionpack (>= 7.2)
activesupport (>= 7.2)
railties (>= 7.2)
@@ -415,22 +420,26 @@ GEM
rspec-expectations (~> 3.13)
rspec-mocks (~> 3.13)
rspec-support (~> 3.13)
- rspec-support (3.13.4)
+ rspec-support (3.13.5)
rtesseract (3.1.4)
+ ruby-lsp (0.26.2)
+ language_server-protocol (~> 3.17.0)
+ prism (>= 1.2, < 2.0)
+ rbs (>= 3, < 5)
ruby-next-core (1.1.1)
ruby-progressbar (1.13.0)
ruby-rc4 (0.1.5)
ruby2_keywords (0.0.5)
- rubyzip (2.4.1)
+ rubyzip (3.0.2)
rufus-scheduler (3.9.2)
fugit (~> 1.1, >= 1.11.1)
rugged (1.9.0)
securerandom (0.4.1)
- selenium-webdriver (4.34.0)
+ selenium-webdriver (4.35.0)
base64 (~> 0.2)
logger (~> 1.4)
rexml (~> 3.2, >= 3.2.5)
- rubyzip (>= 1.2.2, < 3.0)
+ rubyzip (>= 1.2.2, < 4.0)
websocket (~> 1.0)
shoulda-callback-matchers (1.1.4)
activesupport (>= 3)
@@ -442,7 +451,7 @@ GEM
simplecov-html (~> 0.11)
simplecov_json_formatter (~> 0.1)
simplecov-html (0.12.3)
- simplecov-lcov (0.8.0)
+ simplecov-lcov (0.9.0)
simplecov_json_formatter (0.1.4)
sinatra (4.2.0)
logger (>= 1.6.0)
@@ -469,6 +478,7 @@ GEM
tilt (2.6.1)
timecop (0.9.10)
timeout (0.4.3)
+ tsort (0.2.0)
ttfunk (1.8.0)
bigdecimal (~> 3.1)
tzinfo (2.0.6)
@@ -479,10 +489,10 @@ GEM
unicorn (6.1.0)
kgio (~> 2.6)
raindrops (~> 0.7)
- uniform_notifier (1.17.0)
- uri (1.0.3)
+ uniform_notifier (1.18.0)
+ uri (1.1.0)
useragent (0.16.11)
- webmock (3.25.1)
+ webmock (3.26.1)
addressable (>= 2.8.0)
crack (>= 0.3.2)
hashdiff (>= 0.4.0, < 2.0.0)
@@ -540,19 +550,20 @@ DEPENDENCIES
prawn-qrcode
puma
rack-cors
- rails (~> 8.0.2.1)
+ rails (~> 8.0.3)
rails-controller-testing
rails-html-sanitizer
- rails-i18n (~> 8.0.1)
+ rails-i18n (~> 8.0.2)
rails_performance
redcarpet
redis (~> 5.4.1)
responders
resque
resque-scheduler
- rmagick (~> 6.1.2)
- rspec-rails (~> 8.0.1)
+ rmagick (~> 6.1.4)
+ rspec-rails (~> 8.0.2)
rtesseract
+ ruby-lsp
rubyzip
rugged
selenium-webdriver
diff --git a/app/MARKUS_VERSION b/app/MARKUS_VERSION
index 2dc78ad971..ceb1a8f879 100644
--- a/app/MARKUS_VERSION
+++ b/app/MARKUS_VERSION
@@ -1 +1 @@
-VERSION=v2.8.2,PATCH_LEVEL=DEV
+VERSION=v2.9.0,PATCH_LEVEL=DEV
diff --git a/app/assets/javascripts/Annotations/image_annotation_manager.js b/app/assets/javascripts/Annotations/image_annotation_manager.js
index 40ed7baf3d..b4cf57b6eb 100644
--- a/app/assets/javascripts/Annotations/image_annotation_manager.js
+++ b/app/assets/javascripts/Annotations/image_annotation_manager.js
@@ -121,6 +121,9 @@ class ImageAnnotationManager extends AnnotationManager {
};
document.getElementById("image_container").onmousemove = this.render_holders.bind(this);
+
+ // Touch event handlers
+ this.image_preview.addEventListener("touchstart", this.start_select_box_touch.bind(this));
}
}
@@ -174,35 +177,41 @@ class ImageAnnotationManager extends AnnotationManager {
this.sel_box.style.display = "block";
this.sel_box.style.left = xy_coords[0] + "px";
this.sel_box.style.top = xy_coords[1] + "px";
- this.sel_box.onmousemove = this.mouse_move.bind(this);
- this.sel_box.onmouseup = this.stop_select_box.bind(this);
- this.image_preview.onmousemove = this.mouse_move.bind(this);
- this.image_preview.onmouseup = this.stop_select_box.bind(this);
+ // Bind handlers for tracking mouse movement
+ this.bound_mouse_move = this.mouse_move.bind(this);
+ this.bound_stop_select_box = this.stop_select_box.bind(this);
+
+ this.sel_box.addEventListener("mousemove", this.bound_mouse_move);
+ this.sel_box.addEventListener("mouseup", this.bound_stop_select_box);
+ this.image_preview.addEventListener("mousemove", this.bound_mouse_move);
+ this.image_preview.addEventListener("mouseup", this.bound_stop_select_box);
for (let annotation_id in this.annotations) {
let holder = document.getElementById("annotation_holder_" + annotation_id);
- holder.onmousemove = this.mouse_move.bind(this);
- holder.onmouseup = this.stop_select_box.bind(this);
- holder.onmousedown = null;
+ holder.removeEventListener("mousedown", this.bound_start_select_box);
+ holder.addEventListener("mousemove", this.bound_mouse_move);
+ holder.addEventListener("mouseup", this.bound_stop_select_box);
}
}
// Stop tracking the mouse and open up modal to create an image annotation.
stop_select_box() {
- this.image_preview.onmousemove = this.check_for_annotations.bind(this);
- this.image_preview.onmouseup = null;
- this.image_preview.onmousemove = null;
-
- this.sel_box.onmousemove = null;
- this.sel_box.onmouseup = null;
+ this.sel_box.removeEventListener("mousemove", this.bound_mouse_move);
+ this.sel_box.removeEventListener("mouseup", this.bound_stop_select_box);
+ this.image_preview.removeEventListener("mousemove", this.bound_mouse_move);
+ this.image_preview.removeEventListener("mouseup", this.bound_stop_select_box);
for (let annotation_id in this.annotations) {
let holder = document.getElementById("annotation_holder_" + annotation_id);
- holder.onmousemove = this.check_for_annotations.bind(this);
- holder.onmousedown = this.start_select_box.bind(this);
- holder.onmouseup = null;
- holder.onmouseleave = this.check_for_annotations.bind(this);
+ holder.removeEventListener("mousemove", this.bound_mouse_move);
+ holder.removeEventListener("mouseup", this.bound_stop_select_box);
+
+ // Re-bind the start handler
+ if (!this.bound_start_select_box) {
+ this.bound_start_select_box = this.start_select_box.bind(this);
+ }
+ holder.addEventListener("mousedown", this.bound_start_select_box);
}
}
@@ -222,6 +231,77 @@ class ImageAnnotationManager extends AnnotationManager {
this.sel_box.style.height = Math.abs(xy_coords[1] - this.sel_box.orig_y) + "px";
}
+ /**
+ * Touch event handlers for creating annotations
+ */
+
+ // Start tracking the touch to create an annotation.
+ start_select_box_touch(e) {
+ // Prevent default to avoid scrolling while annotating
+ e.preventDefault();
+
+ this.hide_image_annotations();
+ this.hide_selection_box();
+ let touch = e.touches[0];
+ let xy_coords = get_absolute_cursor_pos_touch(touch);
+
+ this.sel_box.orig_x = xy_coords[0];
+ this.sel_box.orig_y = xy_coords[1];
+ this.sel_box.style.display = "block";
+ this.sel_box.style.left = xy_coords[0] + "px";
+ this.sel_box.style.top = xy_coords[1] + "px";
+
+ // Bind handlers for tracking touch movement
+ this.bound_touch_move = this.touch_move.bind(this);
+ this.bound_stop_select_box_touch = this.stop_select_box_touch.bind(this);
+
+ this.sel_box.addEventListener("touchmove", this.bound_touch_move);
+ this.sel_box.addEventListener("touchend", this.bound_stop_select_box_touch);
+ this.image_preview.addEventListener("touchmove", this.bound_touch_move);
+ this.image_preview.addEventListener("touchend", this.bound_stop_select_box_touch);
+
+ for (let annotation_id in this.annotations) {
+ let holder = document.getElementById("annotation_holder_" + annotation_id);
+ holder.removeEventListener("touchstart", this.bound_start_select_box_touch);
+ holder.addEventListener("touchmove", this.bound_touch_move);
+ holder.addEventListener("touchend", this.bound_stop_select_box_touch);
+ }
+ }
+
+ // Stop tracking the touch and open up modal to create an image annotation.
+ stop_select_box_touch() {
+ this.sel_box.removeEventListener("touchmove", this.bound_touch_move);
+ this.sel_box.removeEventListener("touchend", this.bound_stop_select_box_touch);
+ this.image_preview.removeEventListener("touchmove", this.bound_touch_move);
+ this.image_preview.removeEventListener("touchend", this.bound_stop_select_box_touch);
+
+ for (let annotation_id in this.annotations) {
+ let holder = document.getElementById("annotation_holder_" + annotation_id);
+ holder.removeEventListener("touchmove", this.bound_touch_move);
+ holder.removeEventListener("touchend", this.bound_stop_select_box_touch);
+
+ // Re-bind the start handler
+ if (!this.bound_start_select_box_touch) {
+ this.bound_start_select_box_touch = this.start_select_box_touch.bind(this);
+ }
+ holder.addEventListener("touchstart", this.bound_start_select_box_touch);
+ }
+ }
+
+ // Draw red selection outline for touch
+ touch_move(e) {
+ // Prevent default to avoid scrolling while annotating
+ e.preventDefault();
+
+ let touch = e.touches[0];
+ let xy_coords = get_absolute_cursor_pos_touch(touch);
+
+ this.sel_box.style.left = Math.min(xy_coords[0], this.sel_box.orig_x) + "px";
+ this.sel_box.style.width = Math.abs(xy_coords[0] - this.sel_box.orig_x) + "px";
+ this.sel_box.style.top = Math.min(xy_coords[1], this.sel_box.orig_y) + "px";
+ this.sel_box.style.height = Math.abs(xy_coords[1] - this.sel_box.orig_y) + "px";
+ }
+
getSelection(warn_no_selection = true) {
let box = this.get_selection_box_coordinates();
if (!box) {
@@ -325,3 +405,27 @@ function get_absolute_cursor_pos(e) {
return [posx, posy];
}
+
+// Get the coordinates of a touch event relative to image container
+// and return them in an array of the form [x, y].
+function get_absolute_cursor_pos_touch(touch) {
+ let posx = 0;
+ let posy = 0;
+
+ if (!touch) return [0, 0];
+
+ if (touch.pageX || touch.pageY) {
+ posx = touch.pageX;
+ posy = touch.pageY;
+ } else if (touch.clientX || touch.clientY) {
+ posx = touch.clientX + document.body.scrollLeft + document.documentElement.scrollLeft;
+ posy = touch.clientY + document.body.scrollTop + document.documentElement.scrollTop;
+ }
+
+ let image_container = document.getElementById("image_container");
+ let codeviewer = document.getElementById("codeviewer");
+ posx -= image_container.offsetLeft - codeviewer.scrollLeft;
+ posy -= image_container.offsetTop - codeviewer.scrollTop;
+
+ return [posx, posy];
+}
diff --git a/app/assets/javascripts/Annotations/pdf_annotation_manager.js b/app/assets/javascripts/Annotations/pdf_annotation_manager.js
index a5dee80421..e3334b727e 100644
--- a/app/assets/javascripts/Annotations/pdf_annotation_manager.js
+++ b/app/assets/javascripts/Annotations/pdf_annotation_manager.js
@@ -230,7 +230,7 @@
let annotation_text_displayer = this.annotation_text_displayer;
$control.onmousemove = function (ev) {
- let point = getRelativePointForMouseEvent(ev, $page, -1);
+ let point = getRelativePointForEvent(ev, $page, -1);
annotation_text_displayer.setDisplayNodeParent($page[0]);
annotation_text_displayer.displayCollection(
@@ -278,15 +278,9 @@
y: 0,
};
- // Press down, activate the selection box
- $pages.mousedown(ev => {
- if (ev.which !== 1 && ev.target.id === "sel_box") {
- return;
- }
-
- let point = getRelativePointForMouseEvent(ev);
-
- this.setSelectionBox($(ev.delegateTarget), {
+ // Helper: Start selection box
+ const startSelection = (point, $target) => {
+ this.setSelectionBox($target, {
x: point.x,
y: point.y,
width: 0,
@@ -296,25 +290,24 @@
start = point;
selectionBoxActive = true;
- });
+ };
- // Change the selection box
- $pages.mousemove(ev => {
+ // Helper: Update selection box dimensions
+ const updateSelection = (point, $target) => {
if (!selectionBoxActive) {
return;
}
- let point = getRelativePointForMouseEvent(ev); // Mouse position
- this.setSelectionBox($(ev.delegateTarget), {
+ this.setSelectionBox($target, {
x: Math.min(start.x, point.x),
y: Math.min(start.y, point.y),
width: Math.abs(start.x - point.x),
height: Math.abs(start.y - point.y),
});
- });
+ };
- // Finish the selection box
- $pages.mouseup(() => {
+ // Helper: End selection box
+ const endSelection = () => {
let size = this.selectionBoxSize();
// If the box is REALLY small then hide it
@@ -323,28 +316,69 @@
}
selectionBoxActive = false;
+ };
+
+ // Mouse event handlers
+ $pages.mousedown(ev => {
+ if (ev.which !== 1 && ev.target.id === "sel_box") {
+ return;
+ }
+
+ let point = getRelativePointForEvent(ev);
+ startSelection(point, $(ev.delegateTarget));
+ });
+
+ $pages.mousemove(ev => {
+ let point = getRelativePointForEvent(ev);
+ updateSelection(point, $(ev.delegateTarget));
+ });
+
+ $pages.mouseup(() => {
+ endSelection();
+ });
+
+ // Touch event handlers
+ $pages.on("touchstart", ev => {
+ // Prevent default to avoid scrolling while annotating
+ ev.preventDefault();
+
+ let touch = ev.originalEvent.touches[0];
+ let point = getRelativePointForEvent(touch, ev.delegateTarget, undefined);
+ startSelection(point, $(ev.delegateTarget));
+ });
+
+ $pages.on("touchmove", ev => {
+ // Prevent default to avoid scrolling while annotating
+ ev.preventDefault();
+
+ let touch = ev.originalEvent.touches[0];
+ let point = getRelativePointForEvent(touch, ev.delegateTarget, undefined);
+ updateSelection(point, $(ev.delegateTarget));
+ });
+
+ $pages.on("touchend", () => {
+ endSelection();
});
}
}
/**
- * Returns the selection point in percentage units for an event on
- * a element.
+ * Returns the selection point in percentage units for a mouse or touch event.
*
- * @param {Event} ev The event that occurred.
- * @param {String|DOMNode|jQuery} relativeTo The element to calculate the offset for.
- * @param {number} mouseOffset Custom mouse offset value.
+ * @param {Event|Touch} eventOrTouch The event or touch object.
+ * @param {String|DOMNode|jQuery} relativeTo The element to calculate the offset for.
+ * @param {number} mouseOffset Custom mouse offset value.
* @return {{x: number, y:number}} The relative point in the element the event occurred in.
*/
- function getRelativePointForMouseEvent(ev, relativeTo, mouseOffset) {
- let $elem = relativeTo ? $(relativeTo) : $(ev.delegateTarget);
+ function getRelativePointForEvent(eventOrTouch, relativeTo, mouseOffset) {
+ let $elem = relativeTo ? $(relativeTo) : $(eventOrTouch.delegateTarget);
let offset = $elem.offset();
let width = $elem.width();
let height = $elem.height();
- let x = ev.pageX - offset.left - (mouseOffset || MOUSE_OFFSET);
- let y = ev.pageY - offset.top - (mouseOffset || MOUSE_OFFSET);
+ let x = eventOrTouch.pageX - offset.left - (mouseOffset || MOUSE_OFFSET);
+ let y = eventOrTouch.pageY - offset.top - (mouseOffset || MOUSE_OFFSET);
return {
x: 1 - (width - x) / width,
diff --git a/app/assets/javascripts/Grader/marking.js b/app/assets/javascripts/Grader/marking.js
index d8d8227994..e7e28002e0 100644
--- a/app/assets/javascripts/Grader/marking.js
+++ b/app/assets/javascripts/Grader/marking.js
@@ -24,71 +24,3 @@
domContentLoadedCB();
}
})();
-
-// designate $next_criteria as the currently selected criteria
-function activeCriterion($next_criteria) {
- if (!$next_criteria.hasClass("active-criterion")) {
- const $criteria_list = $(".marks-list > li");
- // remove all previous active-criterion (there should only be one)
- $criteria_list.removeClass("active-criterion");
- // scroll the $next_criteria to the top of the criterion bar
- $("#mark_viewer").animate(
- {
- scrollTop: $next_criteria.offset().top - $criteria_list.first().offset().top,
- },
- 100
- );
- $next_criteria.addClass("active-criterion");
- // Unfocus any exisiting textfields/radio buttons
- $(".flexible_criterion input, .checkbox_criterion input").blur();
- // Remove any active rubrics
- $(".active-rubric").removeClass("active-rubric");
- if ($next_criteria.hasClass("flexible_criterion")) {
- var $input = $next_criteria.find('input[type="text"]');
- // This step is necessary for focusing the cursor at the end of input
- $input.focus().val($input.val());
- } else if ($next_criteria.hasClass("rubric_criterion")) {
- $selected = $next_criteria.find(".rubric-level.selected");
- if ($selected.length) {
- $selected.addClass("active-rubric");
- } else {
- $next_criteria.find("tr>td")[0].addClass("active-rubric");
- }
- } else if ($next_criteria.hasClass("checkbox_criterion")) {
- $selected_option = $next_criteria.find("input[checked]")[0];
- if ($selected_option) {
- $selected_option.focus();
- } else {
- $next_criteria.find("input")[0].focus();
- }
- }
- // If this current criteria is not expanded, expand it
- if (!$next_criteria.hasClass("expanded")) {
- if ($next_criteria.hasClass("rubric_criterion")) {
- $next_criteria.children(".criterion_title").click();
- } else {
- $next_criteria.find(".criterion_expand").click();
- }
- }
- }
-}
-
-// Set the active-criterion to the next sibling
-function nextCriterion() {
- $next_criterion = $(".active-criterion").next("li:not(.unassigned)");
- // If no next criterion exists, loop back to the first one
- if (!$next_criterion.length) {
- $next_criterion = $(".marks-list > li:not(.unassigned)").first();
- }
- activeCriterion($next_criterion);
-}
-
-// Set the active-criterion to the previous sibling
-function prevCriterion() {
- $prev_criterion = $(".active-criterion").prev("li:not(.unassigned)");
- // If no previous criterion exists, loop back to the last one
- if (!$prev_criterion.length) {
- $prev_criterion = $(".marks-list > li:not(.unassigned)").last();
- }
- activeCriterion($prev_criterion);
-}
diff --git a/app/assets/javascripts/Groups/index.js b/app/assets/javascripts/Groups/index.js
index 2a518483f0..7c9e34a462 100644
--- a/app/assets/javascripts/Groups/index.js
+++ b/app/assets/javascripts/Groups/index.js
@@ -1,13 +1,8 @@
-var modalCreate,
- modalNotesGroup,
- modalAssignmentGroupReUse = null;
+var modalCreate, modalNotesGroup;
(function () {
const domContentLoadedCB = function () {
- window.modal_rename = new ModalMarkus("#rename_group_dialog");
- modalCreate = new ModalMarkus("#create_group_dialog");
modalNotesGroup = new ModalMarkus("#notes_dialog");
- modalAssignmentGroupReUse = new ModalMarkus("#assignment_group_use_dialog");
};
if (document.readyState === "loading") {
diff --git a/app/assets/javascripts/create_assignment.js b/app/assets/javascripts/create_assignment.js
index c6ff519ed3..70107d9e84 100644
--- a/app/assets/javascripts/create_assignment.js
+++ b/app/assets/javascripts/create_assignment.js
@@ -157,6 +157,10 @@ function change_submission_rule() {
"disabled",
"disabled"
);
+
+ $(".penalty-type-selector").hide();
+ $(".penalty-type-selector select").prop("disabled", true);
+
if ($("#grace_period_submission_rule").is(":checked")) {
$("#grace_periods").show();
$("#grace_periods input").prop("disabled", "");
@@ -164,9 +168,38 @@ function change_submission_rule() {
if ($("#penalty_decay_period_submission_rule").is(":checked")) {
$("#penalty_decay_periods").show();
$("#penalty_decay_periods input").prop("disabled", "");
+ $("#penalty_type_selector_decay").show();
+ $("#penalty_type_selector_decay select").prop("disabled", "");
+ $("#penalty_decay_periods .deduction-unit").text(
+ $("#penalty_type_selector_decay select").val() === "points"
+ ? I18n.t("submission_rules.deduction_unit.marks")
+ : I18n.t("submission_rules.deduction_unit.percentage")
+ );
}
if ($("#penalty_period_submission_rule").is(":checked")) {
$("#penalty_periods").show();
$("#penalty_periods input").prop("disabled", "");
+ $("#penalty_type_selector_period").show();
+ $("#penalty_type_selector_period select").prop("disabled", "");
+ $("#penalty_periods .deduction-unit").text(
+ $("#penalty_type_selector_period select").val() === "points"
+ ? I18n.t("submission_rules.deduction_unit.marks")
+ : I18n.t("submission_rules.deduction_unit.percentage")
+ );
}
+ $("#penalty_type_selector_decay select, #penalty_type_selector_period select").on(
+ "change",
+ function (event) {
+ var isDecay =
+ $(event.target).closest(".penalty-type-selector").attr("id") ===
+ "penalty_type_selector_decay";
+ var selector = isDecay ? "#penalty_decay_periods" : "#penalty_periods";
+
+ if ($(event.target).val() === "points") {
+ $(selector + " .deduction-unit").text(I18n.t("submission_rules.deduction_unit.marks"));
+ } else {
+ $(selector + " .deduction-unit").text(I18n.t("submission_rules.deduction_unit.percentage"));
+ }
+ }
+ );
}
diff --git a/app/assets/stylesheets/common/_markus.scss b/app/assets/stylesheets/common/_markus.scss
index 7867e82139..5811ba5455 100644
--- a/app/assets/stylesheets/common/_markus.scss
+++ b/app/assets/stylesheets/common/_markus.scss
@@ -878,6 +878,7 @@ ul.tags {
}
.ReactModal__Overlay--after-open {
+ background-color: rgba(0, 0, 0, 0.75) !important;
z-index: 100;
}
@@ -923,6 +924,10 @@ ul.tags {
height: 10em;
max-height: 10em;
}
+
+ form label {
+ margin-right: 0.3em;
+ }
}
/** Menus */
@@ -1003,24 +1008,12 @@ nav {
}
/** Dropdown menu */
-.arrow-down {
- border-left: 7.5px solid transparent;
- border-right: 7.5px solid transparent;
- border-top: 7.5px solid $primary-one;
- float: right;
- height: 0;
- margin-top: 4px;
- width: 0;
-}
-
+.arrow-down,
.arrow-up {
- border-bottom: 7.5px solid $primary-one;
- border-left: 7.5px solid transparent;
- border-right: 7.5px solid transparent;
+ color: $primary-one;
float: right;
- height: 0;
- margin-top: 4px;
- width: 0;
+ margin-left: 5px;
+ margin-top: 1px;
}
.dropdown {
@@ -1095,11 +1088,6 @@ nav {
text-transform: uppercase;
}
}
-
- .arrow-down,
- .arrow-up {
- margin-left: 5px;
- }
}
.nested-submenu {
diff --git a/app/assets/stylesheets/common/_table.scss b/app/assets/stylesheets/common/_table.scss
index 17aff5af9e..624f208e45 100644
--- a/app/assets/stylesheets/common/_table.scss
+++ b/app/assets/stylesheets/common/_table.scss
@@ -237,30 +237,14 @@
padding-right: 22px;
}
+ .rt-expandable {
+ text-align: center;
+ }
+
.rt-expander {
display: inline-block;
position: relative;
margin: 0 10px;
- color: transparent;
-
- &::after {
- content: '';
- position: absolute;
- width: 0;
- height: 0;
- top: 50%;
- left: 50%;
- transform: translate(-50%, -50%) rotate(-90deg);
- border-left: 5.04px solid transparent;
- border-right: 5.04px solid transparent;
- border-top: 7px solid $sharp-line;
- transition: all 0.3s var(--easeOutBack);
- cursor: pointer;
- }
-
- &.-open::after {
- transform: translate(-50%, -50%) rotate(0deg);
- }
}
.rt-resizer {
@@ -453,7 +437,8 @@
cursor: default;
}
- .rt-expander {
+ .rt-expander,
+ .rt-expander-custom {
display: none;
}
}
@@ -520,21 +505,22 @@
color: $sharp-line;
}
- .rt-expandable:empty {
- cursor: default;
- }
+ .rt-tbody {
+ .rt-td.rt-expandable {
+ text-align: center;
- .rt-expander::after {
- border-top-color: $sharp-line;
- }
+ &:empty {
+ cursor: default;
+ }
- .hide-rt-expander {
- &.rt-expandable {
- cursor: default;
- }
+ &.hide-rt-expander {
+ cursor: default;
- .rt-expander {
- display: none;
+ .rt-expander,
+ .rt-expander-custom {
+ display: none;
+ }
+ }
}
}
@@ -658,3 +644,9 @@
margin-bottom: 1em;
width: 100%;
}
+
+// Additional custom styles
+.loading-spinner {
+ padding: 10px 0;
+ text-align: center;
+}
diff --git a/app/assets/stylesheets/common/core.scss b/app/assets/stylesheets/common/core.scss
index 3694f75186..e5d864ff6b 100644
--- a/app/assets/stylesheets/common/core.scss
+++ b/app/assets/stylesheets/common/core.scss
@@ -332,3 +332,25 @@ option.uncollected {
display: inline-block;
width: 16px;
}
+
+// Section-specific visibility radio buttons - vertical stacking
+.section-visibility-options {
+ display: flex;
+ flex-direction: column;
+ gap: 0.5em;
+}
+
+.section-visibility-option {
+ display: flex;
+ align-items: center;
+ gap: 0.25em;
+
+ input[type='radio'] {
+ margin: 0;
+ }
+
+ label {
+ margin: 0;
+ cursor: pointer;
+ }
+}
diff --git a/app/assets/stylesheets/grader.scss b/app/assets/stylesheets/grader.scss
index 08c853ac44..2a61dd3caa 100644
--- a/app/assets/stylesheets/grader.scss
+++ b/app/assets/stylesheets/grader.scss
@@ -133,9 +133,10 @@
font-weight: bold;
padding-bottom: 5px;
- .arrow-down,
- .arrow-up {
+ .chevron-expandable {
+ float: left;
margin-right: 5px;
+ margin-top: 2px;
}
}
diff --git a/app/contracts/test_results_contract.rb b/app/contracts/test_results_contract.rb
new file mode 100644
index 0000000000..982df0727f
--- /dev/null
+++ b/app/contracts/test_results_contract.rb
@@ -0,0 +1,66 @@
+class TestResultsContract < Dry::Validation::Contract
+ params do
+ required(:error).maybe(:string)
+ required(:status).filled(:string)
+
+ required(:test_groups).array(:hash) do
+ required(:time).maybe(:integer)
+ optional(:timeout).maybe(:integer)
+ optional(:stderr).maybe(:string)
+ optional(:malformed).maybe(:string)
+
+ required(:tests).array(:hash) do
+ required(:name).filled(:string)
+ required(:time).maybe(:integer)
+ required(:output).value(:string)
+ required(:status).filled(:string)
+ required(:marks_total).filled(:integer)
+ required(:marks_earned).filled(:integer)
+ end
+
+ required(:extra_info).maybe(:hash) do
+ required(:name).filled(:string)
+ optional(:criterion).maybe(:string)
+ required(:test_group_id).filled(:integer)
+ required(:display_output).value(:string)
+ end
+
+ optional(:annotations).array(:hash) do
+ required(:content).filled(:string)
+ required(:filename).filled(:string)
+ optional(:type).filled(:string)
+ optional(:line_start).maybe(:integer)
+ optional(:line_end).maybe(:integer)
+ optional(:column_start).maybe(:integer)
+ optional(:column_end).maybe(:integer)
+ optional(:x1).maybe(:integer)
+ optional(:x2).maybe(:integer)
+ optional(:y1).maybe(:integer)
+ optional(:y2).maybe(:integer)
+ optional(:start_node).maybe(:string)
+ optional(:start_offset).maybe(:integer)
+ optional(:end_node).maybe(:string)
+ optional(:end_offset).maybe(:integer)
+ end
+
+ optional(:feedback).array(:hash) do
+ required(:filename).filled(:string)
+ required(:mime_type).filled(:string)
+ required(:content).filled(:string)
+ optional(:compression).filled(:string)
+ end
+
+ optional(:tags).array(:hash) do
+ required(:name).filled(:string)
+ optional(:description).maybe(:string)
+ end
+
+ optional(:overall_comment).maybe(:string)
+ optional(:extra_marks).array(:hash) do
+ required(:unit).filled(:string)
+ required(:mark).filled(:integer)
+ required(:description).filled(:string)
+ end
+ end
+ end
+end
diff --git a/app/controllers/admin/courses_controller.rb b/app/controllers/admin/courses_controller.rb
index 8572b74428..901a81a902 100644
--- a/app/controllers/admin/courses_controller.rb
+++ b/app/controllers/admin/courses_controller.rb
@@ -1,6 +1,7 @@
module Admin
class CoursesController < ApplicationController
include AutomatedTestsHelper::AutotestApi
+
DEFAULT_FIELDS = [:id, :name, :is_hidden, :display_name].freeze
before_action { authorize! }
@@ -36,7 +37,7 @@ def update
def test_autotest_connection
settings = current_course.autotest_setting
- return head :unprocessable_entity unless settings&.url
+ return head :unprocessable_content unless settings&.url
begin
get_schema(current_course.autotest_setting)
flash_now(:success, I18n.t('automated_tests.manage_connection.test_success', url: settings.url))
@@ -50,7 +51,7 @@ def test_autotest_connection
def reset_autotest_connection
settings = current_course.autotest_setting
- return head :unprocessable_entity unless settings&.url
+ return head :unprocessable_content unless settings&.url
@current_job = AutotestResetUrlJob.perform_later(current_course,
settings.url,
request.protocol + request.host_with_port,
diff --git a/app/controllers/api/assignments_controller.rb b/app/controllers/api/assignments_controller.rb
index d4b25704e3..b464e1fb0a 100644
--- a/app/controllers/api/assignments_controller.rb
+++ b/app/controllers/api/assignments_controller.rb
@@ -11,7 +11,7 @@ class AssignmentsController < MainApiController
:student_form_groups, :remark_due_date, :remark_message,
:assign_graders_to_criteria, :enable_test, :enable_student_tests, :allow_remarks,
:display_grader_names_to_students, :group_name_autogenerated,
- :repository_folder, :is_hidden, :vcs_submit, :token_period,
+ :repository_folder, :is_hidden, :visible_on, :visible_until, :vcs_submit, :token_period,
:non_regenerating_tokens, :unlimited_tokens, :token_start_date, :token_end_date, :has_peer_review,
:starter_file_type, :default_starter_file_group_id].freeze
@@ -57,7 +57,7 @@ def create
if has_missing_params?([:short_identifier, :due_date, :description])
# incomplete/invalid HTTP params
render 'shared/http_status', locals: { code: '422', message:
- HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+ HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_content
return
end
@@ -205,7 +205,7 @@ def update_test_specs
begin
content = JSON.parse params[:specs]
rescue JSON::ParserError => e
- render 'shared/http_status', locals: { code: '422', message: e.message }, status: :unprocessable_entity
+ render 'shared/http_status', locals: { code: '422', message: e.message }, status: :unprocessable_content
return
end
end
@@ -213,7 +213,7 @@ def update_test_specs
render 'shared/http_status',
locals: { code: '422',
message: HttpStatusHelper::ERROR_CODE['message']['422'] },
- status: :unprocessable_entity
+ status: :unprocessable_content
else
AutotestSpecsJob.perform_now(request.protocol + request.host_with_port, assignment, content)
end
diff --git a/app/controllers/api/courses_controller.rb b/app/controllers/api/courses_controller.rb
index 0bf8635d4f..3abd94843d 100644
--- a/app/controllers/api/courses_controller.rb
+++ b/app/controllers/api/courses_controller.rb
@@ -2,6 +2,7 @@ module Api
# API controller for Courses
class CoursesController < MainApiController
include AutomatedTestsHelper::AutotestApi
+
DEFAULT_FIELDS = [:id, :name, :is_hidden, :display_name].freeze
def index
@@ -27,7 +28,7 @@ def show
def create
Course.create!(params.permit(:name, :is_hidden, :display_name))
rescue ActiveRecord::SubclassNotFound, ActiveRecord::RecordInvalid => e
- render 'shared/http_status', locals: { code: '422', message: e.to_s }, status: :unprocessable_entity
+ render 'shared/http_status', locals: { code: '422', message: e.to_s }, status: :unprocessable_content
rescue StandardError
render 'shared/http_status', locals: { code: '500', message:
HttpStatusHelper::ERROR_CODE['message']['500'] }, status: :internal_server_error
@@ -39,7 +40,7 @@ def create
def update
current_course.update!(params.permit(:name, :is_hidden, :display_name))
rescue ActiveRecord::SubclassNotFound, ActiveRecord::RecordInvalid => e
- render 'shared/http_status', locals: { code: '422', message: e.to_s }, status: :unprocessable_entity
+ render 'shared/http_status', locals: { code: '422', message: e.to_s }, status: :unprocessable_content
rescue StandardError
render 'shared/http_status', locals: { code: '500', message:
HttpStatusHelper::ERROR_CODE['message']['500'] }, status: :internal_server_error
@@ -51,7 +52,7 @@ def update
def update_autotest_url
AutotestResetUrlJob.perform_now(current_course, params[:url], request.protocol + request.host_with_port)
rescue ActiveRecord::SubclassNotFound, ActiveRecord::RecordInvalid => e
- render 'shared/http_status', locals: { code: '422', message: e.to_s }, status: :unprocessable_entity
+ render 'shared/http_status', locals: { code: '422', message: e.to_s }, status: :unprocessable_content
rescue StandardError
render 'shared/http_status', locals: { code: '500', message:
HttpStatusHelper::ERROR_CODE['message']['500'] }, status: :internal_server_error
@@ -83,7 +84,7 @@ def test_autotest_connection
end
else
render 'shared/http_status', locals: { code: '422', message:
- I18n.t('automated_tests.no_autotest_settings') }, status: :unprocessable_entity
+ I18n.t('automated_tests.no_autotest_settings') }, status: :unprocessable_content
end
end
@@ -100,7 +101,7 @@ def reset_autotest_connection
end
else
render 'shared/http_status', locals: { code: '422', message:
- I18n.t('automated_tests.no_autotest_settings') }, status: :unprocessable_entity
+ I18n.t('automated_tests.no_autotest_settings') }, status: :unprocessable_content
end
end
diff --git a/app/controllers/api/feedback_files_controller.rb b/app/controllers/api/feedback_files_controller.rb
index c7489ba816..8a8422c96c 100644
--- a/app/controllers/api/feedback_files_controller.rb
+++ b/app/controllers/api/feedback_files_controller.rb
@@ -56,7 +56,7 @@ def create
if has_missing_params?([:filename, :mime_type, :file_content])
# incomplete/invalid HTTP params
render 'shared/http_status', locals: { code: '422', message:
- HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+ HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_content
return
end
@@ -85,7 +85,7 @@ def create
message: I18n.t('oversize_feedback_file',
file_size: ActiveSupport::NumberHelper.number_to_human_size(size_diff),
max_file_size: submission.course.max_file_size / 1_000_000) },
- status: :payload_too_large
+ status: :content_too_large
return
end
if submission.feedback_files.create(filename: params[:filename],
diff --git a/app/controllers/api/grade_entry_forms_controller.rb b/app/controllers/api/grade_entry_forms_controller.rb
index efa47903e2..f4250ce488 100644
--- a/app/controllers/api/grade_entry_forms_controller.rb
+++ b/app/controllers/api/grade_entry_forms_controller.rb
@@ -1,6 +1,7 @@
module Api
class GradeEntryFormsController < MainApiController
- DEFAULT_FIELDS = [:id, :short_identifier, :description, :due_date, :is_hidden, :show_total].freeze
+ DEFAULT_FIELDS = [:id, :short_identifier, :description, :due_date, :is_hidden, :visible_on, :visible_until,
+ :show_total].freeze
# Sends the contents of the specified grade entry form
# Requires: id
@@ -40,7 +41,7 @@ def create
if has_missing_params?([:short_identifier])
# incomplete/invalid HTTP params
render 'shared/http_status', locals: { code: '422', message:
- HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+ HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_content
return
end
@@ -60,7 +61,7 @@ def create
new_form = GradeEntryForm.new(create_params)
unless new_form.save
render 'shared/http_status', locals: { code: '422', message:
- new_form.errors.full_messages.first }, status: :unprocessable_entity
+ new_form.errors.full_messages.first }, status: :unprocessable_content
raise ActiveRecord::Rollback
end
@@ -69,7 +70,7 @@ def create
grade_item = new_form.grade_entry_items.build(**column_params, position: i + 1)
unless grade_item.save
render 'shared/http_status', locals: { code: '422', message:
- grade_item.errors.full_messages.first }, status: :unprocessable_entity
+ grade_item.errors.full_messages.first }, status: :unprocessable_content
raise ActiveRecord::Rollback
end
end
@@ -137,7 +138,7 @@ def update_grades
if has_missing_params?([:user_name, :grade_entry_items])
# incomplete/invalid HTTP params
render 'shared/http_status', locals: { code: '422', message:
- HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+ HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_content
return
end
@@ -151,7 +152,7 @@ def update_grades
if grade_entry_student.nil?
# There is no student with that user_name
render 'shared/http_status', locals: { code: '422', message:
- 'There is no student with that user_name' }, status: :unprocessable_entity
+ 'There is no student with that user_name' }, status: :unprocessable_content
return
end
@@ -162,7 +163,7 @@ def update_grades
if grade_entry_item.nil?
# There is no such grade entry item
render 'shared/http_status', locals: { code: '422', message:
- "There is no grade entry item named #{item}" }, status: :unprocessable_entity
+ "There is no grade entry item named #{item}" }, status: :unprocessable_content
raise ActiveRecord::Rollback
end
grade = grade_entry_student.grades.find_or_create_by(grade_entry_item_id: grade_entry_item.id)
diff --git a/app/controllers/api/groups_controller.rb b/app/controllers/api/groups_controller.rb
index b284d0c4bc..b571f04938 100644
--- a/app/controllers/api/groups_controller.rb
+++ b/app/controllers/api/groups_controller.rb
@@ -20,7 +20,7 @@ def create
end
rescue StandardError => e
render 'shared/http_status', locals: { code: '422', message:
- e.message }, status: :unprocessable_entity
+ e.message }, status: :unprocessable_content
end
end
@@ -68,7 +68,7 @@ def add_members
if self.grouping.nil?
# The group doesn't have a grouping associated with that assignment
render 'shared/http_status', locals: { code: '422', message:
- 'The group is not involved with that assignment' }, status: :unprocessable_entity
+ 'The group is not involved with that assignment' }, status: :unprocessable_content
return
end
@@ -281,7 +281,7 @@ def update_marking_state
if has_missing_params?([:marking_state])
# incomplete/invalid HTTP params
render 'shared/http_status', locals: { code: '422', message:
- HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+ HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_content
return
end
result = self.grouping&.current_submission_used&.get_latest_result
@@ -336,7 +336,7 @@ def extension
else
# cannot delete a non existent extension; render failure
render 'shared/http_status', locals: { code: '422', message:
- HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+ HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_content
end
when 'POST'
if grouping.extension.nil?
@@ -349,7 +349,7 @@ def extension
else
# cannot create extension as it already exists; render failure
render 'shared/http_status', locals: { code: '422', message:
- HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+ HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_content
end
when 'PATCH'
if grouping.extension.present?
@@ -362,11 +362,11 @@ def extension
else
# cannot update extension as it does not exists; render failure
render 'shared/http_status', locals: { code: '422', message:
- HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+ HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_content
end
end
rescue ActiveRecord::RecordInvalid => e
- render 'shared/http_status', locals: { code: '422', message: e.to_s }, status: :unprocessable_entity
+ render 'shared/http_status', locals: { code: '422', message: e.to_s }, status: :unprocessable_content
end
def collect_submission
@@ -375,7 +375,7 @@ def collect_submission
released = @grouping.current_submission_used.results.exists?(released_to_students: true)
if released
render 'shared/http_status', locals: { code: '422', message:
- HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+ HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_content
return
end
end
@@ -401,10 +401,52 @@ def collect_submission
HttpStatusHelper::ERROR_CODE['message']['201'] }, status: :created
end
+ def add_test_run
+ m_logger = MarkusLogger.instance
+ # Validate test results against contract schema
+ validation = TestResultsContract.new.call(params[:test_results].as_json)
+
+ if validation.failure?
+ return render json: { errors: validation.errors.to_hash }, status: :unprocessable_content
+ end
+
+ # Verify grouping exists and is authorized (grouping helper method handles this)
+ if grouping.nil?
+ return render json: { errors: 'Group not found for this assignment' }, status: :not_found
+ end
+
+ # Verify submission exists before attempting to create test run
+ submission = grouping.current_submission_used
+ if submission.nil?
+ return render json: { errors: 'No submission exists for this grouping' }, status: :unprocessable_content
+ end
+
+ begin
+ ActiveRecord::Base.transaction do
+ test_run = TestRun.create!(
+ status: :in_progress,
+ role: current_role,
+ grouping: grouping,
+ submission: submission
+ )
+
+ test_run.update_results!(JSON.parse(params[:test_results].to_json))
+ render json: { status: 'success', test_run_id: test_run.id }, status: :created
+ end
+ rescue ActiveRecord::RecordInvalid => e
+ render json: { errors: e.record.errors.full_messages }, status: :unprocessable_content
+ rescue StandardError => e
+ m_logger.log("Test results processing failed: #{e.message}\n#{e.backtrace.join("\n")}")
+ render json: { errors: 'Failed to process test results' }, status: :internal_server_error
+ end
+ end
+
private
def assignment
- @assignment ||= Assignment.find_by(id: params[:assignment_id])
+ return @assignment if defined?(@assignment)
+
+ @assignment = Assignment.find_by(id: params[:assignment_id])
end
def grouping
diff --git a/app/controllers/api/main_api_controller.rb b/app/controllers/api/main_api_controller.rb
index b2d2d3c426..076628d17f 100644
--- a/app/controllers/api/main_api_controller.rb
+++ b/app/controllers/api/main_api_controller.rb
@@ -17,7 +17,7 @@ class MainApiController < ActionController::Base # rubocop:disable Rails/Applica
before_action { authorize! }
AUTHTYPE = 'MarkUsAuth'.freeze
- AUTH_TOKEN_REGEX = /#{AUTHTYPE} ([^\s,]+)/.freeze
+ AUTH_TOKEN_REGEX = /#{AUTHTYPE} ([^\s,]+)/
def page_not_found(message = HttpStatusHelper::ERROR_CODE['message']['404'])
render 'shared/http_status',
@@ -70,12 +70,12 @@ def get_collection(collection)
filter_params = params[:filter] ? params[:filter].permit(self.class::DEFAULT_FIELDS) : {}
if params[:filter].present? && filter_params.empty?
render 'shared/http_status', locals: { code: '422', message:
- 'Invalid or malformed parameter values' }, status: :unprocessable_entity
+ 'Invalid or malformed parameter values' }, status: :unprocessable_content
false
elsif filter_params.empty?
- collection.order('id')
+ collection.order(:id)
else
- collection.order('id').where(**filter_params)
+ collection.order(:id).where(**filter_params)
end
end
diff --git a/app/controllers/api/roles_controller.rb b/app/controllers/api/roles_controller.rb
index 4463b907be..db725c9da6 100644
--- a/app/controllers/api/roles_controller.rb
+++ b/app/controllers/api/roles_controller.rb
@@ -129,7 +129,7 @@ def create_role
HttpStatusHelper::ERROR_CODE['message']['201'] }, status: :created
end
rescue ActiveRecord::SubclassNotFound, ActiveRecord::RecordInvalid, ActiveRecord::RecordNotFound => e
- render 'shared/http_status', locals: { code: '422', message: e.to_s }, status: :unprocessable_entity
+ render 'shared/http_status', locals: { code: '422', message: e.to_s }, status: :unprocessable_content
rescue StandardError
render 'shared/http_status', locals: { code: '500', message:
HttpStatusHelper::ERROR_CODE['message']['500'] }, status: :internal_server_error
@@ -151,7 +151,7 @@ def update_role(role)
render 'shared/http_status', locals: { code: '200', message:
HttpStatusHelper::ERROR_CODE['message']['200'] }, status: :ok
rescue ActiveRecord::SubclassNotFound, ActiveRecord::RecordInvalid, ActiveRecord::RecordNotFound => e
- render 'shared/http_status', locals: { code: '422', message: e.to_s }, status: :unprocessable_entity
+ render 'shared/http_status', locals: { code: '422', message: e.to_s }, status: :unprocessable_content
rescue StandardError
render 'shared/http_status', locals: { code: '500', message:
HttpStatusHelper::ERROR_CODE['message']['500'] }, status: :internal_server_error
@@ -161,7 +161,7 @@ def find_role_by_username
if has_missing_params?([:user_name])
# incomplete/invalid HTTP params
render 'shared/http_status', locals: { code: '422', message:
- HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+ HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_content
return
end
@@ -181,7 +181,7 @@ def filtered_roles
if filter_params.empty?
render 'shared/http_status',
locals: { code: '422', message: 'Invalid or malformed parameter values' },
- status: :unprocessable_entity
+ status: :unprocessable_content
return false
else
return collection.where(**filter_params)
diff --git a/app/controllers/api/sections_controller.rb b/app/controllers/api/sections_controller.rb
index ef9e2391e2..a4e5a57eb1 100644
--- a/app/controllers/api/sections_controller.rb
+++ b/app/controllers/api/sections_controller.rb
@@ -35,7 +35,7 @@ def create
locals: { code: '201', message: HttpStatusHelper::ERROR_CODE['message']['201'] }, status: :created
else
render 'shared/http_status', locals: { code: '422', message:
- HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+ HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_content
end
end
diff --git a/app/controllers/api/starter_file_groups_controller.rb b/app/controllers/api/starter_file_groups_controller.rb
index 80675f58f8..11772f7e3d 100644
--- a/app/controllers/api/starter_file_groups_controller.rb
+++ b/app/controllers/api/starter_file_groups_controller.rb
@@ -61,7 +61,7 @@ def create_file
if has_missing_params?([:filename, :file_content])
# incomplete/invalid HTTP params
render 'shared/http_status', locals: { code: '422', message:
- HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+ HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_content
return
end
@@ -73,7 +73,7 @@ def create_file
file_path = FileHelper.checked_join(starter_file_group.path, params[:filename])
if file_path.nil?
render 'shared/http_status', locals: { code: '422', message:
- HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+ HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_content
else
File.write(file_path, content, mode: 'wb')
update_entries_and_warn(starter_file_group)
@@ -91,14 +91,14 @@ def create_folder
if has_missing_params?([:folder_path])
# incomplete/invalid HTTP params
render 'shared/http_status', locals: { code: '422', message:
- HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+ HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_content
return
end
folder_path = FileHelper.checked_join(starter_file_group.path, params[:folder_path])
if folder_path.nil?
render 'shared/http_status', locals: { code: '422', message:
- HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+ HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_content
else
FileUtils.mkdir_p(folder_path)
update_entries_and_warn(starter_file_group)
@@ -116,13 +116,13 @@ def remove_file
if has_missing_params?([:filename])
# incomplete/invalid HTTP params
render 'shared/http_status', locals: { code: '422', message:
- HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+ HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_content
return
end
file_path = FileHelper.checked_join(starter_file_group.path, params[:filename])
if file_path.nil?
render 'shared/http_status', locals: { code: '422', message:
- HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+ HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_content
else
File.delete(file_path)
update_entries_and_warn(starter_file_group)
@@ -140,14 +140,14 @@ def remove_folder
if has_missing_params?([:folder_path])
# incomplete/invalid HTTP params
render 'shared/http_status', locals: { code: '422', message:
- HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+ HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_content
return
end
folder_path = FileHelper.checked_join(starter_file_group.path, params[:folder_path])
if folder_path.nil?
render 'shared/http_status', locals: { code: '422', message:
- HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+ HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_content
else
FileUtils.rm_rf(folder_path)
update_entries_and_warn(starter_file_group)
diff --git a/app/controllers/api/submission_files_controller.rb b/app/controllers/api/submission_files_controller.rb
index 60d94ace0b..feda235d17 100644
--- a/app/controllers/api/submission_files_controller.rb
+++ b/app/controllers/api/submission_files_controller.rb
@@ -3,6 +3,7 @@ module Api
# Uses Rails' RESTful routes (check 'rake routes' for the configured routes)
class SubmissionFilesController < MainApiController
include SubmissionsHelper
+
# Returns the requested submission file, or a zip containing all submission
# files, including all annotations if requested
# Requires: assignment_id, group_id
@@ -40,7 +41,7 @@ def index
file = revision.files_at_path(File.join(assignment.repository_folder, path))[file_name]
if file.nil?
render 'shared/http_status', locals: { code: '422', message:
- HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+ HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_content
return
end
file_contents = repo.download_as_string(file)
@@ -88,7 +89,7 @@ def create_folders
if has_missing_params?([:folder_path])
# incomplete/invalid HTTP params
render 'shared/http_status', locals: { code: '422', message:
- HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+ HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_content
return
end
success, messages = grouping.access_repo do |repo|
@@ -115,7 +116,7 @@ def remove_file
if has_missing_params?([:filename])
# incomplete/invalid HTTP params
render 'shared/http_status', locals: { code: '422', message:
- HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+ HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_content
return
end
@@ -143,7 +144,7 @@ def remove_folder
if has_missing_params?([:folder_path])
# incomplete/invalid HTTP params
render 'shared/http_status', locals: { code: '422', message:
- HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+ HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_content
return
end
success, messages = grouping.access_repo do |repo|
diff --git a/app/controllers/api/tags_controller.rb b/app/controllers/api/tags_controller.rb
index 932cf24d84..f76e7c97dc 100644
--- a/app/controllers/api/tags_controller.rb
+++ b/app/controllers/api/tags_controller.rb
@@ -36,7 +36,7 @@ def create
grouping.tags << new_tag
end
rescue StandardError => e
- render 'shared/http_status', locals: { code: '422', message: e.to_s }, status: :unprocessable_entity
+ render 'shared/http_status', locals: { code: '422', message: e.to_s }, status: :unprocessable_content
else
render 'shared/http_status',
locals: { code: '201', message: HttpStatusHelper::ERROR_CODE['message']['201'] }, status: :created
@@ -50,7 +50,7 @@ def update
begin
tag.update!(**self.tag_params)
rescue StandardError => e
- render 'shared/http_status', locals: { code: '422', message: e.to_s }, status: :unprocessable_entity
+ render 'shared/http_status', locals: { code: '422', message: e.to_s }, status: :unprocessable_content
else
render 'shared/http_status',
locals: { code: '200', message: HttpStatusHelper::ERROR_CODE['message']['200'] }, status: :ok
diff --git a/app/controllers/api/users_controller.rb b/app/controllers/api/users_controller.rb
index c0bc586cb6..8cb6de3f7f 100644
--- a/app/controllers/api/users_controller.rb
+++ b/app/controllers/api/users_controller.rb
@@ -23,7 +23,7 @@ def create
if has_missing_params?([:user_name, :type, :first_name, :last_name])
# incomplete/invalid HTTP params
render 'shared/http_status', locals: { code: '422', message:
- HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+ HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_content
return
end
@@ -47,11 +47,11 @@ def create
AdminUser.create!(params.permit(*DEFAULT_FIELDS))
else
render 'shared/http_status', locals: { code: '422', message: 'Unknown user type' },
- status: :unprocessable_entity
+ status: :unprocessable_content
return
end
rescue ActiveRecord::SubclassNotFound, ActiveRecord::RecordInvalid => e
- render 'shared/http_status', locals: { code: '422', message: e.to_s }, status: :unprocessable_entity
+ render 'shared/http_status', locals: { code: '422', message: e.to_s }, status: :unprocessable_content
else
render 'shared/http_status',
locals: { code: '201', message: HttpStatusHelper::ERROR_CODE['message']['201'] }, status: :created
@@ -85,7 +85,7 @@ def update
end
user.update!(user_params)
rescue ActiveRecord::SubclassNotFound, ActiveRecord::RecordInvalid => e
- render 'shared/http_status', locals: { code: '422', message: e.to_s }, status: :unprocessable_entity
+ render 'shared/http_status', locals: { code: '422', message: e.to_s }, status: :unprocessable_content
rescue StandardError
render 'shared/http_status', locals: { code: '500', message:
HttpStatusHelper::ERROR_CODE['message']['500'] }, status: :internal_server_error
@@ -102,7 +102,7 @@ def update_by_username
# incomplete/invalid HTTP params
render 'shared/http_status',
locals: { code: '422', message: HttpStatusHelper::ERROR_CODE['message']['422'] },
- status: :unprocessable_entity
+ status: :unprocessable_content
return
end
@@ -113,7 +113,7 @@ def update_by_username
end
user.update!(user_params)
rescue ActiveRecord::SubclassNotFound, ActiveRecord::RecordInvalid => e
- render 'shared/http_status', locals: { code: '422', message: e.to_s }, status: :unprocessable_entity
+ render 'shared/http_status', locals: { code: '422', message: e.to_s }, status: :unprocessable_content
rescue StandardError
render 'shared/http_status', locals: { code: '500', message:
HttpStatusHelper::ERROR_CODE['message']['500'] }, status: :internal_server_error
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index d1b3575409..98b82b522f 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -56,8 +56,8 @@ def page_not_found
protected
- def use_time_zone(&block)
- Time.use_zone(current_user.time_zone, &block)
+ def use_time_zone(&)
+ Time.use_zone(current_user.time_zone, &)
end
# Set locale according to URL parameter. If unknown parameter is
diff --git a/app/controllers/assignments_controller.rb b/app/controllers/assignments_controller.rb
index b4cb38e683..51f5ffd456 100644
--- a/app/controllers/assignments_controller.rb
+++ b/app/controllers/assignments_controller.rb
@@ -2,6 +2,7 @@ class AssignmentsController < ApplicationController
include RepositoryHelper
include RoutingHelper
include AutomatedTestsHelper
+
responders :flash
before_action { authorize! }
@@ -286,6 +287,22 @@ def download_test_results
type: 'text/csv',
filename: filename
end
+ format.zip do
+ data = @assignment.summary_test_result_json
+ zip_name = "#{@assignment.short_identifier}_test_results.zip"
+
+ zip_path = File.join('tmp', zip_name)
+ json_filename = "#{@assignment.short_identifier}_test_results.json"
+
+ FileUtils.rm_f(zip_path)
+
+ Zip::File.open(zip_path, create: true) do |zip_file|
+ zip_file.get_output_stream(json_filename) do |f|
+ f.write(data)
+ end
+ end
+ send_file zip_path, filename: zip_name, type: 'application/zip', disposition: 'attachment'
+ end
end
end
@@ -534,7 +551,7 @@ def set_boolean_graders_options
unless assignment.update(attributes)
flash_now(:error, assignment.errors.full_messages.join(' '))
- head :unprocessable_entity
+ head :unprocessable_content
return
end
head :ok
@@ -831,7 +848,30 @@ def process_assignment_form(assignment)
periods = period_attrs.to_h.values.map { |h| h[:id].presence }
assignment.submission_rule.periods.where.not(id: periods).find_each(&:destroy)
end
- assignment.assign_attributes(assignment_params)
+
+ # Handle "scheduled" visibility option
+ # When is_hidden="scheduled", convert to is_hidden=false with datetime values
+ params_copy = assignment_params.deep_dup
+ if params_copy[:is_hidden] == Assessment::SCHEDULED_VISIBILITY
+ params_copy[:is_hidden] = false
+ elsif params_copy[:is_hidden].present?
+ # Clear datetime fields when switching to "Hidden" or "Visible"
+ params_copy[:visible_on] = nil
+ params_copy[:visible_until] = nil
+ end
+
+ # Handle section-specific "scheduled" visibility
+ params_copy[:assessment_section_properties_attributes]&.each_value do |section_props|
+ if section_props[:is_hidden] == Assessment::SCHEDULED_VISIBILITY
+ section_props[:is_hidden] = false
+ elsif section_props[:is_hidden].present?
+ # Clear datetime fields when switching to "Hidden", "Visible", or "Default"
+ section_props[:visible_on] = nil
+ section_props[:visible_until] = nil
+ end
+ end
+
+ assignment.assign_attributes(params_copy)
SubmissionRule.where(assignment: assignment).where.not(id: assignment.submission_rule.id).find_each(&:destroy)
process_timed_duration(assignment) if assignment.is_timed
assignment.repository_folder = short_identifier
@@ -906,6 +946,8 @@ def assignment_params
:message,
:due_date,
:is_hidden,
+ :visible_on,
+ :visible_until,
:parent_assessment_id,
assignment_properties_attributes: [
:id,
@@ -941,7 +983,9 @@ def assignment_params
:section_id,
:due_date,
:start_time,
- :is_hidden
+ :is_hidden,
+ :visible_on,
+ :visible_until
],
assignment_files_attributes: [
:_destroy,
@@ -952,6 +996,7 @@ def assignment_params
:_destroy,
:id,
:type,
+ :penalty_type,
{ periods_attributes: [
:id,
:deduction,
@@ -980,6 +1025,7 @@ def submission_rule_params
:_destroy,
:id,
:type,
+ :penalty_type,
{ periods_attributes: [
:id,
:deduction,
diff --git a/app/controllers/automated_tests_controller.rb b/app/controllers/automated_tests_controller.rb
index cf5642e704..8b7b61532d 100644
--- a/app/controllers/automated_tests_controller.rb
+++ b/app/controllers/automated_tests_controller.rb
@@ -219,17 +219,17 @@ def upload_specs
test_specs = JSON.parse file_content
rescue JSON::ParserError
flash_now(:error, I18n.t('automated_tests.invalid_specs_file'))
- head :unprocessable_entity
+ head :unprocessable_content
rescue StandardError => e
flash_now(:error, e.message)
- head :unprocessable_entity
+ head :unprocessable_content
else
@current_job = AutotestSpecsJob.perform_later(request.protocol + request.host_with_port, assignment, test_specs)
session[:job_id] = @current_job.job_id
render 'shared/_poll_job'
end
else
- head :unprocessable_entity
+ head :unprocessable_content
end
end
diff --git a/app/controllers/canvas_controller.rb b/app/controllers/canvas_controller.rb
index 96d437afac..d2b32851d8 100644
--- a/app/controllers/canvas_controller.rb
+++ b/app/controllers/canvas_controller.rb
@@ -39,7 +39,8 @@ def get_config
custom_fields: {
user_id: '$Canvas.user.id',
course_id: '$Canvas.course.id',
- course_name: '$Canvas.course.name'
+ course_name: '$Canvas.course.name',
+ student_number: '$Canvas.user.sisIntegrationId'
}
}
diff --git a/app/controllers/courses_controller.rb b/app/controllers/courses_controller.rb
index ba6a4dcafd..5e24656da5 100644
--- a/app/controllers/courses_controller.rb
+++ b/app/controllers/courses_controller.rb
@@ -79,7 +79,7 @@ def switch_role
render partial: 'role_switch_handler',
formats: [:js], handlers: [:erb],
locals: { error: I18n.t('main.cannot_role_switch_to_self') },
- status: :unprocessable_entity
+ status: :unprocessable_content
return
end
@@ -88,7 +88,7 @@ def switch_role
render partial: 'role_switch_handler',
formats: [:js], handlers: [:erb],
locals: { error: I18n.t('main.cannot_role_switch') },
- status: :unprocessable_entity
+ status: :unprocessable_content
return
end
diff --git a/app/controllers/criteria_controller.rb b/app/controllers/criteria_controller.rb
index 1b911c6bb0..5a6ece2dfc 100644
--- a/app/controllers/criteria_controller.rb
+++ b/app/controllers/criteria_controller.rb
@@ -42,7 +42,7 @@ def create
@criterion.errors.full_messages.each do |message|
flash_message(:error, message)
end
- head :unprocessable_entity
+ head :unprocessable_content
end
end
@@ -107,7 +107,7 @@ def update
@criterion.errors.full_messages.each do |message|
flash_message(:error, message)
end
- head :unprocessable_entity
+ head :unprocessable_content
end
end
diff --git a/app/controllers/exam_templates_controller.rb b/app/controllers/exam_templates_controller.rb
index 56303e611c..3a90083db4 100644
--- a/app/controllers/exam_templates_controller.rb
+++ b/app/controllers/exam_templates_controller.rb
@@ -102,7 +102,7 @@ def download_generate
exam_template = record
path = FileHelper.checked_join(exam_template.tmp_path, params[:file_name])
if path.nil?
- head :unprocessable_entity
+ head :unprocessable_content
else
send_file(path,
filename: params[:file_name],
@@ -264,7 +264,7 @@ def download_error_file
@assignment = record.assignment
path = FileHelper.checked_join(exam_template.base_path, 'error', params[:file_name])
if path.nil?
- head :unprocessable_entity
+ head :unprocessable_content
else
send_file(path,
filename: params[:file_name],
diff --git a/app/controllers/grade_entry_forms_controller.rb b/app/controllers/grade_entry_forms_controller.rb
index fd1e206b6f..a5f4ab4ae5 100644
--- a/app/controllers/grade_entry_forms_controller.rb
+++ b/app/controllers/grade_entry_forms_controller.rb
@@ -3,6 +3,7 @@
class GradeEntryFormsController < ApplicationController
include GradeEntryFormsHelper
include RoutingHelper
+
before_action { authorize! }
layout 'assignment_content'
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index b737bed253..12cb41fd2a 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -117,7 +117,9 @@ def index
respond_to do |format|
format.html
format.json do
- render json: @assignment.all_grouping_data
+ render json: @assignment.all_grouping_data.merge(
+ clone_assignments: @clone_assignments.as_json(only: [:id, :short_identifier])
+ )
end
end
end
@@ -181,11 +183,13 @@ def get_names
.where(groupings: { assessment_id: params[:assignment_id] }))
.pluck_to_hash(:id, 'users.id_number', 'users.user_name',
'users.first_name', 'users.last_name', 'roles.hidden')
+
names = names.map do |h|
+ inactive = h['roles.hidden'] ? I18n.t('student.inactive') : ''
{ id: h[:id],
id_number: h['users.id_number'],
user_name: h['users.user_name'],
- value: "#{h['users.first_name']} #{h['users.last_name']}#{h['roles.hidden'] ? ' (inactive)' : ''}" }
+ value: "#{h['users.first_name']} #{h['users.last_name']}#{inactive}" }
end
render json: names
end
@@ -199,12 +203,15 @@ def assign_student_and_next
if params[:s_id].present?
student = current_course.students.find(params[:s_id])
end
+ replace_pattern = /#{Regexp.escape(I18n.t('student.inactive'))}\s*$/
+ student_name = params[:names].sub(replace_pattern, '').strip
+
# if the user has typed in the whole name without select, or if they typed a name different from the select s_id
- if student.nil? || "#{student.first_name} #{student.last_name}" != params[:names]
+ if student.nil? || "#{student.first_name} #{student.last_name}" != student_name
student = current_course.students.joins(:user).where(
'lower(CONCAT(first_name, \' \', last_name)) like ? OR lower(CONCAT(last_name, \' \', first_name)) like ?',
- ApplicationRecord.sanitize_sql_like(params[:names].downcase),
- ApplicationRecord.sanitize_sql_like(params[:names].downcase)
+ ApplicationRecord.sanitize_sql_like(student_name.downcase),
+ ApplicationRecord.sanitize_sql_like(student_name.downcase)
).first
end
if student.nil?
@@ -346,7 +353,7 @@ def use_another_assignment_groups
target_assignment = Assignment.find(params[:assignment_id])
source_assignment = Assignment.find(params[:clone_assignment_id])
- return head :unprocessable_entity if target_assignment.course != source_assignment.course
+ return head :unprocessable_content if target_assignment.course != source_assignment.course
if source_assignment.nil?
flash_message(:warning, t('groups.clone_warning.could_not_find_source'))
@@ -371,7 +378,7 @@ def accept_invitation
current_role.join(@grouping)
rescue ActiveRecord::RecordInvalid, RuntimeError => e
flash_message(:error, e.message)
- status = :unprocessable_entity
+ status = :unprocessable_content
else
m_logger = MarkusLogger.instance
m_logger.log("Student '#{current_role.user_name}' joined group " \
@@ -389,7 +396,7 @@ def decline_invitation
@grouping.decline_invitation(current_role)
rescue RuntimeError => e
flash_message(:error, e.message)
- status = :unprocessable_entity
+ status = :unprocessable_content
else
m_logger = MarkusLogger.instance
m_logger.log("Student '#{current_role.user_name}' declined invitation for group '#{@grouping.group.group_name}'.")
diff --git a/app/controllers/lti_deployments_controller.rb b/app/controllers/lti_deployments_controller.rb
index 56ed3f2f6d..94f9ed7c8d 100644
--- a/app/controllers/lti_deployments_controller.rb
+++ b/app/controllers/lti_deployments_controller.rb
@@ -12,7 +12,7 @@ class LtiDeploymentsController < ApplicationController
def launch
if params[:client_id].blank? || params[:login_hint].blank? ||
params[:target_link_uri].blank? || params[:lti_message_hint].blank?
- head :unprocessable_entity
+ head :unprocessable_content
return
end
nonce = rand(10 ** 30).to_s.rjust(30, '0')
@@ -91,7 +91,12 @@ def redirect_login
deployment_id: lti_params[LtiDeployment::LTI_CLAIMS[:deployment_id]],
lms_course_name: lti_params[LtiDeployment::LTI_CLAIMS[:context]]['title'],
lms_course_label: lti_params[LtiDeployment::LTI_CLAIMS[:context]]['label'],
- lms_course_id: lti_params[LtiDeployment::LTI_CLAIMS[:custom]]['course_id'] }
+ lms_course_id: lti_params[LtiDeployment::LTI_CLAIMS[:custom]]['course_id'],
+ user_roles: lti_params[LtiDeployment::LTI_CLAIMS[:roles]] }
+ if lti_params.key?(LtiDeployment::LTI_CLAIMS[:rlid])
+ rlid = lti_params[LtiDeployment::LTI_CLAIMS[:rlid]]['id']
+ lti_data[:resource_link_id] = rlid
+ end
if lti_params.key?(LtiDeployment::LTI_CLAIMS[:names_role])
name_and_roles_endpoint = lti_params[LtiDeployment::LTI_CLAIMS[:names_role]]['context_memberships_url']
lti_data[:names_role_service] = name_and_roles_endpoint
@@ -122,7 +127,10 @@ def redirect_login
lti_deployment = LtiDeployment.find_or_initialize_by(lti_client: lti_client,
external_deployment_id: lti_data[:deployment_id],
lms_course_id: lti_data[:lms_course_id])
- lti_deployment.update!(lms_course_name: lti_data[:lms_course_name])
+ lti_deployment.update!(
+ lms_course_name: lti_data[:lms_course_name],
+ resource_link_id: lti_data[:resource_link_id]
+ )
session[:lti_course_label] = lti_data[:lms_course_label]
if lti_data.key?(:names_role_service)
names_service = LtiService.find_or_initialize_by(lti_deployment: lti_deployment, service_type: 'namesrole')
@@ -135,9 +143,18 @@ def redirect_login
LtiUser.find_or_create_by(user: @real_user, lti_client: lti_client,
lti_user_id: lti_data[:lti_user_id])
if lti_deployment.course.nil?
- # Redirect to course picker page
- redirect_to choose_course_lti_deployment_path(lti_deployment)
+ # Check if the user has any of the privileged roles
+ has_privileged_role = lti_data[:user_roles].any? do |role_uri|
+ LtiDeployment::LTI_PRIVILEGED_ROLES.include?(role_uri)
+ end
+ has_ta_role = lti_data[:user_roles].include?(LtiDeployment::LTI_ROLES[:ta])
+ if has_privileged_role && !has_ta_role
+ redirect_to choose_course_lti_deployment_path(lti_deployment)
+ else
+ redirect_to course_not_set_up_lti_deployment_path(lti_deployment)
+ end
else
+ # Course is linked, proceed to the course path
redirect_to course_path(lti_deployment.course)
end
ensure
@@ -150,6 +167,10 @@ def public_jwk
render json: { keys: [jwk.export] }
end
+ def course_not_set_up
+ render 'course_not_set_up', status: :not_found
+ end
+
def choose_course
@lti_deployment = record
if request.post?
@@ -176,7 +197,7 @@ def check_host
known_lti_hosts << URI(root_url).host
if known_lti_hosts.exclude?(URI(request.referer).host)
render 'shared/http_status', locals: { code: '422', message: I18n.t('lti.config_error') },
- status: :unprocessable_entity, layout: false
+ status: :unprocessable_content, layout: false
nil
end
end
diff --git a/app/controllers/main_controller.rb b/app/controllers/main_controller.rb
index cd2ce16f7d..2a5f879294 100644
--- a/app/controllers/main_controller.rb
+++ b/app/controllers/main_controller.rb
@@ -118,7 +118,7 @@ def about
# Render 404 error (page not found) if no other route matches.
# See config/routes.rb
- def page_not_found # rubocop:disable Lint/UselessMethodDefinition
+ def page_not_found
super
end
diff --git a/app/controllers/results_controller.rb b/app/controllers/results_controller.rb
index aef1bfa5e4..963fafc93b 100644
--- a/app/controllers/results_controller.rb
+++ b/app/controllers/results_controller.rb
@@ -284,6 +284,7 @@ def edit
def run_tests
submission = record.submission
+
assignment = Grouping.find(submission.grouping_id).assignment
# If no test groups can be run by instructors, flash appropriate message and return early
test_group_categories = assignment.test_groups.pluck(:autotest_settings).pluck('category')
@@ -292,6 +293,7 @@ def run_tests
flash_now(:info, I18n.t('automated_tests.no_instructor_runnable_tests'))
return
end
+
flash_message(:notice, I18n.t('automated_tests.autotest_run_job.status.in_progress'))
AutotestRunJob.perform_later(request.protocol + request.host_with_port,
current_role.id,
diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb
index 1530b53ef1..3f3d9e6cf5 100644
--- a/app/controllers/submissions_controller.rb
+++ b/app/controllers/submissions_controller.rb
@@ -3,6 +3,7 @@
class SubmissionsController < ApplicationController
include SubmissionsHelper
include RepositoryHelper
+
before_action { authorize! }
authorize :from_codeviewer, through: :from_codeviewer_param
@@ -461,7 +462,7 @@ def update_files
messages << commit_msg
head :ok
else
- head :unprocessable_entity
+ head :unprocessable_content
end
end
flash_repository_messages messages, @grouping.course
@@ -507,7 +508,7 @@ def download_file
max_content_size = params[:max_content_size].blank? ? -1 : params[:max_content_size].to_i
if max_content_size != -1 && file_contents.size > max_content_size
- head :payload_too_large
+ head :content_too_large
return
end
diff --git a/app/helpers/automated_tests_helper.rb b/app/helpers/automated_tests_helper.rb
index fd61f88c0f..5aa6a1696c 100644
--- a/app/helpers/automated_tests_helper.rb
+++ b/app/helpers/automated_tests_helper.rb
@@ -121,6 +121,7 @@ def get_markus_address(host_with_port)
# Sends RESTful api requests to the autotester
module AutotestApi
include AutomatedTestsHelper
+
AUTOTEST_USERNAME = "markus_#{Rails.application.config.relative_url_root}".freeze
class LimitExceededException < StandardError; end
diff --git a/app/helpers/lti_helper.rb b/app/helpers/lti_helper.rb
index 324b4add2b..751fccb51e 100644
--- a/app/helpers/lti_helper.rb
+++ b/app/helpers/lti_helper.rb
@@ -10,6 +10,15 @@ def roster_sync(lti_deployment, role_types, can_create_users: false, can_create_
auth_data = lti_deployment.lti_client.get_oauth_token([LtiDeployment::LTI_SCOPES[:names_role]])
names_service = lti_deployment.lti_services.find_by!(service_type: 'namesrole')
membership_uri = URI(names_service.url)
+ if lti_deployment.resource_link_id.present?
+ query = begin
+ URI.decode_www_form(String(membership_uri.query))
+ rescue StandardError
+ []
+ end
+ query << ['rlid', lti_deployment.resource_link_id]
+ membership_uri.query = URI.encode_www_form(query)
+ end
member_info, follow = get_member_data(lti_deployment, membership_uri, auth_data)
while follow != false
additional_data, follow = get_member_data(lti_deployment, follow, auth_data)
@@ -18,11 +27,18 @@ def roster_sync(lti_deployment, role_types, can_create_users: false, can_create_
user_data = member_info.filter_map do |user|
unless user['status'] == 'Inactive' || user['roles'].include?(LtiDeployment::LTI_ROLES['test_user']) ||
role_types.none? { |role| user['roles'].include?(role) }
+ student_number = user.dig('message', 0, LtiDeployment::LTI_CLAIMS[:custom], 'student_number')
+ id_number_value = if student_number.blank? || student_number == '$Canvas.user.sisIntegrationId'
+ nil
+ else
+ student_number
+ end
{ user_name: user['lis_person_sourcedid'].nil? ? user['name'].delete(' ') : user['lis_person_sourcedid'],
first_name: user['given_name'],
last_name: user['family_name'],
display_name: user['name'],
email: user['email'],
+ id_number: id_number_value,
lti_user_id: user['user_id'],
roles: user['roles'] }
end
@@ -101,6 +117,7 @@ def get_assignment_marks(lti_deployment, assignment)
released_results = assignment.released_marks
.joins(grouping: [{ accepted_student_memberships: { role: { user: :lti_users } } }])
.where('lti_users.lti_client': lti_deployment.lti_client)
+ .where(roles: { hidden: false })
.pluck('lti_users.lti_user_id', 'results.id')
result_ids = released_results.pluck(1)
grades = Result.get_total_marks(result_ids)
diff --git a/app/helpers/submissions_helper.rb b/app/helpers/submissions_helper.rb
index 125fc5a49a..15434b5746 100644
--- a/app/helpers/submissions_helper.rb
+++ b/app/helpers/submissions_helper.rb
@@ -57,7 +57,7 @@ def upload_file(grouping, only_required_files: false)
if has_missing_params?([:filename, :mime_type, :file_content])
# incomplete/invalid HTTP params
render 'shared/http_status', locals: { code: '422', message:
- HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+ HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_content
return
end
@@ -66,7 +66,7 @@ def upload_file(grouping, only_required_files: false)
if FileHelper.checked_join(path.to_s, filename).nil?
message = I18n.t('errors.invalid_path')
- render 'shared/http_status', locals: { code: '422', message: message }, status: :unprocessable_entity
+ render 'shared/http_status', locals: { code: '422', message: message }, status: :unprocessable_content
return
end
@@ -75,7 +75,7 @@ def upload_file(grouping, only_required_files: false)
if only_required_files && required_files.exclude?(filename)
message = t('assignments.upload_file_requirement', file_name: params[:filename]) +
"\n#{Assignment.human_attribute_name(:assignment_files)}: #{required_files.join(', ')}"
- render 'shared/http_status', locals: { code: '422', message: message }, status: :unprocessable_entity
+ render 'shared/http_status', locals: { code: '422', message: message }, status: :unprocessable_content
return
end
diff --git a/app/javascript/Components/DropDown/MultiSelectDropDown.jsx b/app/javascript/Components/DropDown/MultiSelectDropDown.jsx
index 08285b28fd..3226270312 100644
--- a/app/javascript/Components/DropDown/MultiSelectDropDown.jsx
+++ b/app/javascript/Components/DropDown/MultiSelectDropDown.jsx
@@ -64,9 +64,9 @@ export class MultiSelectDropdown extends React.Component {
let expanded = this.state.expanded;
let arrow;
if (expanded !== false) {
- arrow = ;
+ arrow =
{children}
; } diff --git a/app/javascript/Components/Modals/assignment_group_use_modal.jsx b/app/javascript/Components/Modals/assignment_group_use_modal.jsx new file mode 100644 index 0000000000..4854204d21 --- /dev/null +++ b/app/javascript/Components/Modals/assignment_group_use_modal.jsx @@ -0,0 +1,90 @@ +import React from "react"; +import Modal from "react-modal"; + +export default class AssignmentGroupUseModal extends React.Component { + constructor(props) { + super(props); + this.state = { + assignmentId: "", + isLoading: false, + }; + } + + componentDidMount() { + Modal.setAppElement("body"); + } + + componentDidUpdate(prevProps) { + if (this.props.isOpen && !prevProps.isOpen) { + if (this.props.cloneAssignments.length > 0) { + this.setState({ + assignmentId: this.props.cloneAssignments[0].id, + }); + } else { + this.setState({ + assignmentId: "", + }); + } + } + } + + handleChange = event => { + this.setState({assignmentId: event.target.value}); + }; + + handleSubmit = event => { + event.preventDefault(); + if (window.confirm(I18n.t("groups.delete_groups_linked"))) { + this.setState({isLoading: true}); + this.props.onSubmit(this.state.assignmentId); + } + }; + + render() { + return ( +| <%= I18n.t('assignments.section_settings.visibility') %> | +<%= I18n.t('assignments.section_hidden.visible_from') %> | +<%= I18n.t('assignments.section_hidden.visible_until') %> | <%= f.fields_for :assessment_section_properties, @assessment_section_properties do |due_date_f| %>||
|---|---|---|---|---|
| <%= obj.section.name %> | <% if @assignment.is_timed %>@@ -149,12 +237,40 @@ size: 35 %> | - <%= due_date_f.label :is_hidden, I18n.t('assignments.section_hidden.default'), value: '' %> - <%= due_date_f.radio_button :is_hidden, '' %> - <%= due_date_f.label :is_hidden, I18n.t('assignments.section_hidden.visible'), value: 'false' %> - <%= due_date_f.radio_button :is_hidden, false %> - <%= due_date_f.label :is_hidden, I18n.t('assignments.section_hidden.hidden'), value: 'true' %> - <%= due_date_f.radio_button :is_hidden, true %> + + | ++ <%= due_date_f.text_field :visible_on, + class: 'assessment_section_properties_input section-datetime-input', + value: obj.visible_on.nil? ? '' : l(obj.visible_on, format: :flatpickr), + placeholder: I18n.t('assignments.section_hidden.visible_from_placeholder'), + size: 25, + disabled: obj.is_hidden != Assessment::SCHEDULED_VISIBILITY %> + | ++ <%= due_date_f.text_field :visible_until, + class: 'assessment_section_properties_input section-datetime-input', + value: obj.visible_until.nil? ? '' : l(obj.visible_until, format: :flatpickr), + placeholder: I18n.t('assignments.section_hidden.visible_until_placeholder'), + size: 25, + disabled: obj.is_hidden != Assessment::SCHEDULED_VISIBILITY %> |
| - <%= due_date_f.label :is_hidden, I18n.t('assignments.section_hidden.default'), value: '' %> - <%= due_date_f.radio_button :is_hidden, '' %> - <%= due_date_f.label :is_hidden, I18n.t('assignments.section_hidden.visible'), value: 'false' %> - <%= due_date_f.radio_button :is_hidden, false %> - <%= due_date_f.label :is_hidden, I18n.t('assignments.section_hidden.hidden'), value: 'true' %> - <%= due_date_f.radio_button :is_hidden, true %> + | <% end %> diff --git a/app/views/assignments/_penalty_decay_period.html.erb b/app/views/assignments/_penalty_decay_period.html.erb index ef022043d9..c2d201105d 100644 --- a/app/views/assignments/_penalty_decay_period.html.erb +++ b/app/views/assignments/_penalty_decay_period.html.erb @@ -9,7 +9,8 @@|||
| - <%= f.number_field :deduction, min: 0, step: :any, required: true %> % + <%= f.number_field :deduction, min: 0, step: :any, required: true %> + % | <%= f.number_field :interval, min: 0.001, step: :any, required: true %> diff --git a/app/views/assignments/_penalty_period.html.erb b/app/views/assignments/_penalty_period.html.erb index beee987208..c798b78ca9 100644 --- a/app/views/assignments/_penalty_period.html.erb +++ b/app/views/assignments/_penalty_period.html.erb @@ -25,7 +25,8 @@ | - <%= f.number_field :deduction, min: 0, step: :any, required: true %> % + <%= f.number_field :deduction, min: 0, step: :any, required: true %> + % |
<% if !f.object.nil? && !f.object.new_record? %>
diff --git a/app/views/assignments/_read.html.erb b/app/views/assignments/_read.html.erb
index 48bb261922..bb8a5fe8d0 100644
--- a/app/views/assignments/_read.html.erb
+++ b/app/views/assignments/_read.html.erb
@@ -93,12 +93,13 @@
<% end %>
<% elsif @assignment.submission_rule.type == "PenaltyDecayPeriodSubmissionRule" %>
+ <% suffix = @assignment.submission_rule.penalty_type || ExtraMark::PERCENTAGE %>
<% @assignment.submission_rule.periods.each do |p| %>
<% if p == @assignment.submission_rule.periods.first %>
- <%= t('lti.course_not_found') %>+<%= t('lti.course_not_set_up') %> +Test error message ') end end @@ -493,6 +550,56 @@ expect(response).to have_http_status(:bad_request) end end + + context 'when updating with assignment files' do + let!(:assignment_file1) { create(:assignment_file, assignment: assignment) } + let!(:assignment_file2) { create(:assignment_file, assignment: assignment) } + + before do + put_as instructor, + :update, + params: { + course_id: course.id, + id: flexible_criterion.id, + flexible_criterion: { + name: 'Updated Criterion', + assignment_files: [assignment_file1.id, assignment_file2.id] + } + }, + format: :js + end + + it 'should associate assignment files with criterion' do + expect(flexible_criterion.reload.assignment_files).to include(assignment_file1, assignment_file2) + end + + it 'should respond with success' do + expect(subject).to respond_with(:success) + end + end + + context 'when update fails with validation errors' do + before do + put_as instructor, + :update, + params: { + course_id: course.id, + id: flexible_criterion.id, + flexible_criterion: { + max_mark: -1 # Invalid max_mark to trigger validation error + } + }, + format: :js + end + + it 'should display error messages' do + expect(flash[:error]).not_to be_empty + end + + it 'should respond with unprocessable entity' do + expect(subject).to respond_with(:unprocessable_content) + end + end end describe '#edit' do @@ -544,6 +651,28 @@ expect { FlexibleCriterion.find(flexible_criterion.id) }.to raise_error(ActiveRecord::RecordNotFound) end + + context 'when assignment marks are released' do + let!(:released_assignment) { create(:assignment_with_criteria_and_released_results) } + let!(:criterion_with_released_marks) do + released_assignment.criteria.find_by(type: 'FlexibleCriterion') + end + + before do + delete_as instructor, + :destroy, + params: { course_id: released_assignment.course_id, id: criterion_with_released_marks.id }, + format: :js + end + + it 'should flash error message' do + expect(flash[:error]).to have_message(I18n.t('criteria.errors.released_marks')) + end + + it 'should not delete the criterion' do + expect(FlexibleCriterion.find(criterion_with_released_marks.id)).to be_present + end + end end end @@ -744,7 +873,7 @@ end it 'should respond with unprocessable entity' do - expect(subject).to respond_with(:unprocessable_entity) + expect(subject).to respond_with(:unprocessable_content) end end @@ -765,6 +894,33 @@ expect(subject).to render_template(:update) end end + + context 'when updating with assignment files' do + let!(:assignment_file1) { create(:assignment_file, assignment: assignment) } + let!(:assignment_file2) { create(:assignment_file, assignment: assignment) } + + before do + put_as instructor, + :update, + params: { + course_id: course.id, + id: rubric_criterion.id, + rubric_criterion: { + name: 'Updated Rubric Criterion', + assignment_files: [assignment_file1.id, assignment_file2.id] + } + }, + format: :js + end + + it 'should associate assignment files with criterion' do + expect(rubric_criterion.reload.assignment_files).to include(assignment_file1, assignment_file2) + end + + it 'should respond with success' do + expect(subject).to respond_with(:success) + end + end end end @@ -806,7 +962,7 @@ end it 'should respond with unprocessable entity' do - expect(subject).to respond_with(:unprocessable_entity) + expect(subject).to respond_with(:unprocessable_content) end end @@ -1245,6 +1401,28 @@ end describe '#upload' do + context 'when marks are released' do + let!(:released_assignment) { create(:assignment_with_criteria_and_released_results) } + let(:test_file) { fixture_file_upload('criteria/upload_yml_mixed.yaml', 'text/yaml') } + + before do + post_as instructor, :upload, + params: { + course_id: released_assignment.course_id, + assignment_id: released_assignment.id, + upload_file: test_file + } + end + + it 'should flash error message' do + expect(flash[:error]).to have_message(I18n.t('criteria.errors.released_marks')) + end + + it 'should redirect to index' do + expect(response).to redirect_to(action: 'index', id: released_assignment.id) + end + end + it_behaves_like 'a controller supporting upload', formats: [:yml] do let(:params) { { course_id: course.id, assignment_id: assignment.id } } end diff --git a/spec/controllers/exam_templates_controller_spec.rb b/spec/controllers/exam_templates_controller_spec.rb index 0243cd92ea..756713f4b4 100644 --- a/spec/controllers/exam_templates_controller_spec.rb +++ b/spec/controllers/exam_templates_controller_spec.rb @@ -318,7 +318,7 @@ end it 'responds with an error status code' do - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:unprocessable_content) end end end @@ -332,7 +332,7 @@ end it 'responds with an error status code' do - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:unprocessable_content) end end end diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index c439fa2d67..d16813da7e 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -956,6 +956,99 @@ format: :json } expect(response).to have_http_status(:not_found) end + + context 'when assigning inactive students' do + let!(:inactive_student) do + create(:student, + hidden: true, + user: create(:end_user, + user_name: 'inactive_student', + first_name: 'Inactive', + last_name: 'Student', + id_number: '99999')) + end + + before do + grouping2 = create(:grouping, assignment: assignment) + create(:version_used_submission, grouping: grouping2) + end + + it 'assigns an inactive student to the grouping when s_id is provided' do + post_as instructor, :assign_student_and_next, + params: { course_id: course.id, + assignment_id: assignment.id, + assignment: assignment.id, + names: "#{inactive_student.first_name} #{inactive_student.last_name}", + s_id: inactive_student.id, + g_id: grouping1.id, + format: :json } + expect(grouping1.memberships.first.role).to eq inactive_student + end + + it 'assigns an inactive student to the grouping by name only' do + post_as instructor, :assign_student_and_next, + params: { course_id: course.id, + assignment_id: assignment.id, + assignment: assignment.id, + names: "#{inactive_student.first_name} #{inactive_student.last_name}", + g_id: grouping1.id, + format: :json } + expect(grouping1.memberships.first.role).to eq inactive_student + end + end + + context 'testing student selection logic when s_id and names may differ' do + let!(:student2) do + create(:student, + user: create(:end_user, + user_name: 'c9test2', + first_name: 'Different', + last_name: 'Person', + id_number: '67890')) + end + + before do + grouping2 = create(:grouping, assignment: assignment) + create(:version_used_submission, grouping: grouping2) + end + + it 'uses name lookup when no dropdown selection is made (student.nil?)' do + post_as instructor, :assign_student_and_next, + params: { course_id: course.id, + assignment_id: assignment.id, + assignment: assignment.id, + names: "#{student1.first_name} #{student1.last_name}", + g_id: grouping1.id, + format: :json } + expect(grouping1.memberships.first.role).to eq student1 + end + + it 'prioritizes manually edited name over dropdown selection when they differ' do + # Simulates: user selected student2 from dropdown, then manually edited the text to student1's name + post_as instructor, :assign_student_and_next, + params: { course_id: course.id, + assignment_id: assignment.id, + assignment: assignment.id, + names: "#{student1.first_name} #{student1.last_name}", + s_id: student2.id, + g_id: grouping1.id, + format: :json } + expect(grouping1.memberships.first.role).to eq student1 + expect(grouping1.memberships.map(&:role)).not_to include(student2) + end + + it 'uses dropdown selection when it matches typed name' do + post_as instructor, :assign_student_and_next, + params: { course_id: course.id, + assignment_id: assignment.id, + assignment: assignment.id, + names: "#{student2.first_name} #{student2.last_name}", + s_id: student2.id, + g_id: grouping1.id, + format: :json } + expect(grouping1.memberships.first.role).to eq student2 + end + end end end @@ -1052,14 +1145,14 @@ it 'fails to accept when there is no invitation' do post_as @current_student, :accept_invitation, params: { course_id: course.id, assignment_id: grouping.assessment_id, grouping_id: grouping.id } - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:unprocessable_content) end it 'fails to accept when the invitation has already been accepted' do create(:accepted_student_membership, role: @current_student, grouping: grouping) post_as @current_student, :accept_invitation, params: { course_id: course.id, assignment_id: grouping.assessment_id, grouping_id: grouping.id } - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:unprocessable_content) end it 'fails to accept when another invitation has already been accepted' do @@ -1068,7 +1161,7 @@ create(:accepted_student_membership, role: @current_student, grouping: grouping2) post_as @current_student, :accept_invitation, params: { course_id: course.id, assignment_id: grouping.assessment_id, grouping_id: grouping.id } - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:unprocessable_content) end it 'rejects other pending invitations' do @@ -1104,13 +1197,13 @@ create(:accepted_student_membership, role: @current_student, grouping: grouping) post_as @current_student, :decline_invitation, params: { course_id: course.id, assignment_id: grouping.assessment_id, grouping_id: grouping.id } - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:unprocessable_content) end it 'fails to reject when there is no invitation' do post_as @current_student, :decline_invitation, params: { course_id: course.id, assignment_id: grouping.assessment_id, grouping_id: grouping.id } - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:unprocessable_content) end end diff --git a/spec/controllers/lti_deployments_controller_spec.rb b/spec/controllers/lti_deployments_controller_spec.rb index a5d7ff9949..f8c76ac33e 100644 --- a/spec/controllers/lti_deployments_controller_spec.rb +++ b/spec/controllers/lti_deployments_controller_spec.rb @@ -11,7 +11,8 @@ describe '#choose_course' do let!(:course) { create(:course) } let(:instructor) { create(:instructor, course: course) } - let!(:lti) { create(:lti_deployment) } + let(:test_rlid) { 'a-unique-resource-link-id-12345' } + let!(:lti) { create(:lti_deployment, resource_link_id: test_rlid) } describe 'get' do it 'is inaccessible unless logged in' do @@ -48,6 +49,12 @@ expect(lti.course).to eq(course) end + it 'retains the resource_link_id after updating the course' do + post_as instructor, :choose_course, params: { id: lti.id, course: course.id } + lti.reload + expect(lti.resource_link_id).to eq(test_rlid) + end + context 'when the user does not have permission to link' do let(:course2) { create(:course) } let(:instructor2) { create(:instructor, course: course2) } @@ -63,7 +70,8 @@ end describe '#create_course' do - let!(:lti_deployment) { create(:lti_deployment, lms_course_name: 'csc108') } + let(:test_rlid) { 'another-unique-rlid-67890' } + let!(:lti_deployment) { create(:lti_deployment, lms_course_name: 'csc108', resource_link_id: test_rlid) } let(:course_params) do { id: lti_deployment.id, display_name: 'Introduction to Computer Science', name: lti_deployment.lms_course_name } end @@ -85,6 +93,11 @@ it 'creates an instructor role for the user' do expect(Role.find_by(user: instructor.user, course: Course.find_by(name: 'csc108'))).not_to be_nil end + + it 'retains the resource_link_id after course creation' do + lti_deployment.reload + expect(lti_deployment.resource_link_id).to eq(test_rlid) + end end context 'as an admin user' do diff --git a/spec/controllers/main_controller_spec.rb b/spec/controllers/main_controller_spec.rb index a9ecbd49a5..645893e427 100644 --- a/spec/controllers/main_controller_spec.rb +++ b/spec/controllers/main_controller_spec.rb @@ -1,5 +1,6 @@ describe MainController do include SessionHandler + let(:student) { create(:student) } let(:ta) { create(:ta) } let(:instructor) { create(:instructor) } diff --git a/spec/controllers/submissions_controller_spec.rb b/spec/controllers/submissions_controller_spec.rb index 5ede5aef7e..2c34369b38 100644 --- a/spec/controllers/submissions_controller_spec.rb +++ b/spec/controllers/submissions_controller_spec.rb @@ -194,7 +194,7 @@ post_as @student, :update_files, params: { course_id: course.id, assignment_id: @assignment.id, new_folders: [dir], path: '/' } - expect(response).to have_http_status :unprocessable_entity + expect(response).to have_http_status :unprocessable_content expect(Dir).not_to exist(File.join(@grouping.group.repo_path, @grouping.assignment.repository_folder, dir)) end @@ -204,7 +204,7 @@ params: { course_id: course.id, assignment_id: @assignment.id, delete_files: ['../../../../../LICENSE'], path: '/' } - expect(response).to have_http_status :unprocessable_entity + expect(response).to have_http_status :unprocessable_content expect(File).to exist( File.expand_path(File.join(@grouping.group.repo_path, '../../../../../LICENSE')) ) @@ -215,7 +215,7 @@ params: { course_id: course.id, assignment_id: @assignment.id, delete_folders: ['../../../../../doc'], path: '/' } - expect(response).to have_http_status :unprocessable_entity + expect(response).to have_http_status :unprocessable_content expect(Dir).to exist( File.expand_path(File.join(@grouping.group.repo_path, '../../../../../doc')) ) @@ -380,7 +380,7 @@ post_as @student, :update_files, params: { course_id: course.id, assignment_id: @assignment.id, new_files: [file1, file2] } - expect(response).to have_http_status :unprocessable_entity + expect(response).to have_http_status :unprocessable_content # Check to see if the file was added @grouping.group.access_repo do |repo| @@ -456,7 +456,7 @@ post_as @student, :update_files, params: { course_id: course.id, assignment_id: @assignment.id, new_folders: ['bad_folder'] } - expect(response).to have_http_status :unprocessable_entity + expect(response).to have_http_status :unprocessable_content end it 'does not commit the non required directory' do @@ -470,7 +470,7 @@ post_as @student, :update_files, params: { course_id: course.id, assignment_id: @assignment.id, new_folders: ['bad_folder/bad_subdirectory'] } - expect(response).to have_http_status :unprocessable_entity + expect(response).to have_http_status :unprocessable_content end it 'does not commit a non required subdirectory' do @@ -532,14 +532,14 @@ post_as @student, :update_files, params: { course_id: course.id, assignment_id: @assignment.id, new_files: ['.git'] } - expect(response).to have_http_status :unprocessable_entity + expect(response).to have_http_status :unprocessable_content end it 'should not be allowed in a zip file' do zip_file = fixture_file_upload('test_zip_git_file.zip', 'application/zip') post_as @student, :update_files, params: { course_id: course.id, assignment_id: @assignment.id, new_files: [zip_file], unzip: unzip } - expect(response).to have_http_status :unprocessable_entity + expect(response).to have_http_status :unprocessable_content end it 'should not create a .git file in the repo' do @@ -561,14 +561,14 @@ post_as @student, :update_files, params: { course_id: course.id, assignment_id: @assignment.id, new_folders: ['.git'] } - expect(response).to have_http_status :unprocessable_entity + expect(response).to have_http_status :unprocessable_content end it 'should not be allowed in a zip file' do zip_file = fixture_file_upload('test_zip_git_folder.zip', 'application/zip') post_as @student, :update_files, params: { course_id: course.id, assignment_id: @assignment.id, new_files: [zip_file], unzip: unzip } - expect(response).to have_http_status :unprocessable_entity + expect(response).to have_http_status :unprocessable_content end it 'should not create a .git directory in the repo' do @@ -2066,7 +2066,7 @@ def self.test_no_flash max_content_size: SAMPLE_FILE_CONTENT.size - 1 } end - it { expect(response).to have_http_status(:payload_too_large) } + it { expect(response).to have_http_status(:content_too_large) } it 'should have an empty response body' do expect(response.body).to be_empty diff --git a/spec/db/check_repo_permissions_spec.rb b/spec/db/check_repo_permissions_spec.rb index 4a2cdbe54a..f9b3643fb2 100644 --- a/spec/db/check_repo_permissions_spec.rb +++ b/spec/db/check_repo_permissions_spec.rb @@ -186,6 +186,85 @@ end end + context 'the assignment has datetime-based visibility' do + context 'when visible_on is in the future' do + before { grouping.assignment.update! visible_on: 1.hour.from_now, visible_until: 2.hours.from_now } + + it 'should fail' do + expect(script_success?).to be_falsy + end + end + + context 'when visible_until is in the past' do + before { grouping.assignment.update! visible_on: 2.hours.ago, visible_until: 1.hour.ago } + + it 'should fail' do + expect(script_success?).to be_falsy + end + end + + context 'when current time is within visibility window' do + before { grouping.assignment.update! visible_on: 1.hour.ago, visible_until: 1.hour.from_now } + + it 'should succeed' do + expect(script_success?).to be_truthy + end + end + + context 'when only visible_on is set and in the past' do + before { grouping.assignment.update! visible_on: 1.hour.ago, visible_until: nil } + + it 'should succeed' do + expect(script_success?).to be_truthy + end + end + + context 'when only visible_until is set and in the future' do + before { grouping.assignment.update! visible_on: nil, visible_until: 1.hour.from_now } + + it 'should succeed' do + expect(script_success?).to be_truthy + end + end + end + + context 'the assignment has section-specific datetime visibility' do + before { role.update! section_id: section.id } + + context 'when section visible_on is in the future' do + before do + create(:assessment_section_properties, assessment: grouping.assignment, section: section, + visible_on: 1.hour.from_now, visible_until: 2.hours.from_now) + end + + it 'should fail' do + expect(script_success?).to be_falsy + end + end + + context 'when section visible_until is in the past' do + before do + create(:assessment_section_properties, assessment: grouping.assignment, section: section, + visible_on: 2.hours.ago, visible_until: 1.hour.ago) + end + + it 'should fail' do + expect(script_success?).to be_falsy + end + end + + context 'when current time is within section visibility window' do + before do + create(:assessment_section_properties, assessment: grouping.assignment, section: section, + visible_on: 1.hour.ago, visible_until: 1.hour.from_now) + end + + it 'should succeed' do + expect(script_success?).to be_truthy + end + end + end + context 'the assessment is timed' do let(:due_date) { 10.hours.from_now } diff --git a/spec/factories/assignment_files.rb b/spec/factories/assignment_files.rb index 2cb0f9de57..d53bfe0a86 100644 --- a/spec/factories/assignment_files.rb +++ b/spec/factories/assignment_files.rb @@ -3,6 +3,6 @@ FactoryBot.define do factory :assignment_file do association :assignment - filename { Faker::Lorem.word } + sequence(:filename) { |n| "#{Faker::Lorem.word}_#{n}" } end end diff --git a/spec/factories/assignments.rb b/spec/factories/assignments.rb index 9f65cc7bda..12b5b38507 100644 --- a/spec/factories/assignments.rb +++ b/spec/factories/assignments.rb @@ -42,6 +42,13 @@ end end + factory :assignment_with_criteria_and_released_results, parent: :assignment_with_criteria_and_results do + after(:create) do |a| + # Release marks by setting released_to_students to true on all current results + a.current_results.update_all(released_to_students: true) + end + end + factory :assignment_with_criteria_and_test_results, parent: :assignment do after(:create) do |a| create_list(:flexible_criterion, 3, assignment: a) diff --git a/spec/factories/extra_marks.rb b/spec/factories/extra_marks.rb index b215af1788..0019bdcf87 100644 --- a/spec/factories/extra_marks.rb +++ b/spec/factories/extra_marks.rb @@ -1,14 +1,23 @@ FactoryBot.define do - factory :extra_mark do + factory :extra_mark, class: 'ExtraMark' do association :result description { Faker::Lorem.sentence } unit { 'percentage' } extra_mark { -10.0 } - end - factory :extra_mark_points, class: 'ExtraMark' do - association :result - description { Faker::Lorem.sentence } - unit { 'points' } - extra_mark { 1 } + + factory :extra_mark_percentage do + unit { 'percentage' } + extra_mark { -10.0 } + end + + factory :extra_mark_points do + unit { 'points' } + extra_mark { 1 } + end + + factory :extra_mark_percentage_of_mark do + unit { 'percentage_of_mark' } + extra_mark { 5.0 } + end end end diff --git a/spec/helpers/automated_tests_helper_spec.rb b/spec/helpers/automated_tests_helper_spec.rb index ccefc1d671..1eb40287a7 100644 --- a/spec/helpers/automated_tests_helper_spec.rb +++ b/spec/helpers/automated_tests_helper_spec.rb @@ -1,5 +1,6 @@ describe AutomatedTestsHelper do include ApplicationHelper + describe '.update_test_groups_from_specs' do subject { update_test_groups_from_specs assignment, specs } diff --git a/spec/helpers/lti_helper_spec.rb b/spec/helpers/lti_helper_spec.rb index 9069cb1370..247657e60b 100644 --- a/spec/helpers/lti_helper_spec.rb +++ b/spec/helpers/lti_helper_spec.rb @@ -39,7 +39,10 @@ roles: [ LtiDeployment::LTI_ROLES[:learner] - ] }, + ], + message: [{ + LtiDeployment::LTI_CLAIMS[:custom] => { 'student_number' => '999123456' } + }] }, { status: 'Active', name: 'second user', picture: 'http://example.com/picture.png', given_name: 'student.first_name', @@ -51,7 +54,10 @@ roles: [ LtiDeployment::LTI_ROLES[:learner] - ] }, + ], + message: [{ + LtiDeployment::LTI_CLAIMS[:custom] => { 'student_number' => '999654321' } + }] }, { status: 'Active', name: 'third user', picture: 'http://example.com/picture.png', given_name: 'student.first_name', @@ -160,6 +166,62 @@ end end end + + context 'when handling student ID numbers' do + subject do + roster_sync lti_deployment, [LtiDeployment::LTI_ROLES[:ta], LtiDeployment::LTI_ROLES[:learner]], + can_create_users: true, can_create_roles: true + end + + let(:placeholder_student_number) { '999111222' } + let(:memberships_with_placeholder) do + [ + { status: 'Active', + name: student.user.display_name, + given_name: student.user.first_name, + family_name: student.user.last_name, + lis_person_sourcedid: student.user.user_name, + email: student.user.email, + user_id: 'lti_user_id', + roles: [LtiDeployment::LTI_ROLES[:learner]], + message: [{ + LtiDeployment::LTI_CLAIMS[:custom] => { 'student_number' => placeholder_student_number } + }] }, + memberships[1].merge(message: [{ + LtiDeployment::LTI_CLAIMS[:custom] => { 'student_number' => '$Canvas.user.sisIntegrationId' } + }]), + memberships[2].dup.except(:message) + ] + end + + before do + lti_students_placeholder = { id: '...', context: { id: '...', label: 'tst1', title: 'test course' }, + members: memberships_with_placeholder }.to_json + allow_any_instance_of(LtiDeployment).to( + receive(:send_lti_request!).and_return(OpenStruct.new(body: lti_students_placeholder)) + ) + end + + it 'correctly saves the student number for a valid user' do + student.destroy! + student.user.destroy! + subject + user = EndUser.find_by(user_name: student.user_name) + expect(user.id_number).to eq(placeholder_student_number) + end + + it 'sets the id_number to NULL for the Canvas placeholder' do + subject + user = EndUser.find_by(user_name: 'second_username') + expect(user.id_number).to be_nil + end + + it 'sets the id_number to NULL when the custom claim is missing' do + subject + user = EndUser.find_by(user_name: 'third_username') + expect(user.id_number).to be_nil + end + end end context 'when syncing learners and TAs' do @@ -434,6 +496,49 @@ expect(get_assignment_marks(lti_deployment, assessment)).not_to be_empty end end + + context 'when some students are inactive (hidden)' do + let(:student1) { create(:student, course: course) } + let(:student2) { create(:student, course: course) } + let(:assignment) { create(:assignment, course: course) } + + let!(:grouping) do + create(:grouping, assignment: assignment).tap do |g| + create(:student_membership, + grouping: g, + role: student1, + membership_status: StudentMembership::STATUSES[:accepted]) + + create(:student_membership, + grouping: g, + role: student2, + membership_status: StudentMembership::STATUSES[:accepted]) + end + end + + before do + create(:result, + grouping: grouping, + released_to_students: true, + marking_state: Result::MARKING_STATES[:complete]) + + student2.update!(hidden: true) + + [student1.user, student2.user].each do |usr| + create(:lti_user, user: usr, lti_client: lti_deployment.lti_client) + end + end + + it 'does not include hidden students in the marks hash' do + marks = get_assignment_marks(lti_deployment, assignment) + + id1 = student1.user.lti_users.first.lti_user_id + id2 = student2.user.lti_users.first.lti_user_id + + expect(marks.keys).to include(id1) + expect(marks.keys).not_to include(id2) + end + end end describe '#get_grade_entry_form_marks' do diff --git a/spec/jobs/autotest_reset_url_job_spec.rb b/spec/jobs/autotest_reset_url_job_spec.rb index 389f7cd368..80a52959ae 100644 --- a/spec/jobs/autotest_reset_url_job_spec.rb +++ b/spec/jobs/autotest_reset_url_job_spec.rb @@ -1,5 +1,6 @@ describe AutotestResetUrlJob do include AutomatedTestsHelper + let(:host_with_port) { 'http://localhost:3000' } let(:url) { 'http://example.com' } let(:course) { create(:course) } diff --git a/spec/lib/tasks/users_spec.rb b/spec/lib/tasks/users_spec.rb new file mode 100644 index 0000000000..510b81bad0 --- /dev/null +++ b/spec/lib/tasks/users_spec.rb @@ -0,0 +1,50 @@ +require 'rails_helper' +require 'rake' + +RSpec.describe 'db:prune_orphaned_end_users', type: :task do + before do + Rake.application.rake_require('tasks/users') + Rake::Task.define_task(:environment) + task.reenable + end + + let(:task) { Rake::Task['db:prune_orphaned_end_users'] } + + def valid_end_user(user_name) + EndUser.create!( + user_name: user_name, + first_name: 'First', + last_name: 'Last', + display_name: 'First Last' + ) + end + + context 'when orphaned end users exist' do + it 'deletes all orphaned end users and prints output' do + user1 = valid_end_user('orphan1') + user2 = valid_end_user('orphan2') + + relation = EndUser.where(id: [user1.id, user2.id]) + + allow(EndUser).to receive(:get_orphaned_users).and_return(relation) + + expect do + expect do + task.invoke + end.to output(/Found 2 orphaned EndUser records/).to_stdout + end.to change { EndUser.count }.by(-2) + end + end + + context 'when no orphaned end users exist' do + it 'prints the appropriate message and deletes nothing' do + allow(EndUser).to receive(:get_orphaned_users).and_return(EndUser.none) + + expect do + task.invoke + end.to output(/No orphaned EndUser records found/).to_stdout + + expect(EndUser.count).to eq(0) + end + end +end diff --git a/spec/mailers/notification_mailer_spec.rb b/spec/mailers/notification_mailer_spec.rb index d2dfc94021..567cba3eef 100644 --- a/spec/mailers/notification_mailer_spec.rb +++ b/spec/mailers/notification_mailer_spec.rb @@ -2,6 +2,7 @@ RSpec.describe NotificationMailer do include ERB::Util + RSpec.shared_examples 'an email' do it 'renders the disclaimer in the body of the email.' do expect(mail.body.to_s).to include('This is an automated email. Please do not reply.') diff --git a/spec/models/annotation_category_spec.rb b/spec/models/annotation_category_spec.rb index f1662fe327..1c4143d59e 100644 --- a/spec/models/annotation_category_spec.rb +++ b/spec/models/annotation_category_spec.rb @@ -353,4 +353,77 @@ end end end + + describe '.to_json' do + let(:assignment) { create(:assignment) } + + context 'when there is a single category and single text' do + let!(:category) { create(:annotation_category, assignment: assignment, annotation_category_name: 'Test A') } + let!(:text) { create(:annotation_text, annotation_category: category, content: 'Test A text') } + + it 'returns a JSON for the category with its text' do + category_json = AnnotationCategory.to_json([category]).first + expect(category_json[:id]).to eq(category.id) + expect(category_json[:annotation_category_name]).to eq('Test A') + expect(category_json[:texts].size).to eq(1) + + text_json = category_json[:texts].first + expect(text_json[:id]).to eq(text.id) + expect(text_json[:content]).to eq('Test A text') + expect(text_json[:deduction]).to be_nil + end + end + + context 'when there is a category with multiple texts' do + let!(:category) { create(:annotation_category, assignment: assignment, annotation_category_name: 'Test B') } + + it 'returns a JSON including all the texts for the category' do + create(:annotation_text, annotation_category: category, content: 'Test B text 1') + create(:annotation_text, annotation_category: category, content: 'Test B text 2') + + category_json = AnnotationCategory.to_json([category]).first + expect(category_json[:id]).to eq(category.id) + expect(category_json[:annotation_category_name]).to eq('Test B') + expect(category_json[:texts].size).to eq(2) + + texts_json = category_json[:texts] + expect(texts_json.pluck(:content)).to match_array(['Test B text 1', 'Test B text 2']) + expect(texts_json.pluck(:deduction)).to all(be_nil) + end + end + + context 'when there are multiple categories with text(s)' do + let!(:category1) do + create(:annotation_category, assignment: assignment, annotation_category_name: 'Category A') + end + let!(:category2) do + create(:annotation_category, assignment: assignment, annotation_category_name: 'Category B') + end + + it 'returns JSON for all categories with their texts' do + text1 = create(:annotation_text, annotation_category: category1, content: 'Category A text 1') + text2 = create(:annotation_text, annotation_category: category1, content: 'Category A text 2') + text3 = create(:annotation_text, annotation_category: category2, content: 'Category B text 3') + + categories_json = AnnotationCategory.to_json([category1, category2]) + category_ids = categories_json.pluck(:id) # rubocop:disable Rails/PluckId + + expect(category_ids).to match_array([category1.id, category2.id]) + + all_texts = categories_json.flat_map { |c| c[:texts] } + expect(all_texts).to include({ id: text1.id, content: 'Category A text 1', deduction: nil }) + expect(all_texts).to include({ id: text2.id, content: 'Category A text 2', deduction: nil }) + expect(all_texts).to include({ id: text3.id, content: 'Category B text 3', deduction: nil }) + end + end + + context 'when a category has no texts' do + let!(:category) { create(:annotation_category, assignment: assignment, annotation_category_name: 'Empty A') } + + it 'returns an empty texts array' do + category_json = AnnotationCategory.to_json([category]).first + expect(category_json[:texts]).to eq([]) + end + end + end end diff --git a/spec/models/assessment_spec.rb b/spec/models/assessment_spec.rb index 8499782bc2..6290c59c1f 100644 --- a/spec/models/assessment_spec.rb +++ b/spec/models/assessment_spec.rb @@ -19,4 +19,41 @@ expected_error = I18n.t(error_key, attribute: 'Short identifier') expect(assessment.errors[:short_identifier]).to include(expected_error) end + + describe 'datetime visibility validation' do + let(:course) { create(:course) } + + it 'allows nil visible_on and visible_until' do + assessment = create(:assignment, course: course, visible_on: nil, visible_until: nil) + expect(assessment).to be_valid + end + + it 'allows only visible_on to be set' do + assessment = create(:assignment, course: course, visible_on: 1.day.ago, visible_until: nil) + expect(assessment).to be_valid + end + + it 'allows only visible_until to be set' do + assessment = create(:assignment, course: course, visible_on: nil, visible_until: 1.day.from_now) + expect(assessment).to be_valid + end + + it 'allows visible_on before visible_until' do + assessment = create(:assignment, course: course, visible_on: 1.day.ago, visible_until: 1.day.from_now) + expect(assessment).to be_valid + end + + it 'rejects visible_on equal to visible_until' do + time = Time.current + assessment = build(:assignment, course: course, visible_on: time, visible_until: time) + expect(assessment).not_to be_valid + expect(assessment.errors[:visible_until]).to include('must be after visible_on') + end + + it 'rejects visible_on after visible_until' do + assessment = build(:assignment, course: course, visible_on: 1.day.from_now, visible_until: 1.day.ago) + expect(assessment).not_to be_valid + expect(assessment.errors[:visible_until]).to include('must be after visible_on') + end + end end diff --git a/spec/models/course_spec.rb b/spec/models/course_spec.rb index e32c2c48a8..46faf605bb 100644 --- a/spec/models/course_spec.rb +++ b/spec/models/course_spec.rb @@ -238,7 +238,7 @@ let(:desired_attributes) do ['short_identifier', 'description', 1.day.from_now.at_beginning_of_minute, 'message', 1, 2, 1, true, true, 2.days.from_now.at_beginning_of_minute, 'remark_message', - true, true, false, true, true, true, true, false, true, true] + true, true, false, true, true, true, true, false, nil, nil, true, true] end let(:assignment) do csv = desired_attributes.to_csv @@ -358,7 +358,7 @@ let(:desired_attributes) do ['short_identifier', 'description', 1.day.from_now.at_beginning_of_minute, 'message', 1, 2, 1, true, true, 2.days.from_now.at_beginning_of_minute, 'remark_message', - true, true, false, true, true, true, true, false, true, true] + true, true, false, true, true, true, true, false, nil, nil, true, true] end let(:assignment) do desired_attribute_value_hash = Assignment::DEFAULT_FIELDS.zip(desired_attributes).to_h @@ -390,7 +390,7 @@ let!(:new_assignment_attr) do ['new', 'abc', 1.day.from_now.at_beginning_of_minute, 'message', 1, 2, 1, true, true, 2.days.from_now.at_beginning_of_minute, 'remark_message', - true, true, false, true, true, true, true, false, true, false] + true, true, false, true, true, true, true, false, nil, nil, true, false] end let!(:returned) do # create the old assignment @@ -732,7 +732,8 @@ def create_assignment_csv_string(assignment) assignment.student_form_groups, assignment.remark_due_date, assignment.remark_message, assignment.assign_graders_to_criteria, assignment.enable_test, assignment.enable_student_tests, assignment.allow_remarks, assignment.display_grader_names_to_students, assignment.display_median_to_students, - assignment.group_name_autogenerated, assignment.is_hidden, assignment.vcs_submit, assignment.has_peer_review].to_csv + assignment.group_name_autogenerated, assignment.is_hidden, assignment.visible_on, assignment.visible_until, + assignment.vcs_submit, assignment.has_peer_review].to_csv end def create_assignment_symbol_to_value_map(assignment) @@ -757,6 +758,8 @@ def create_assignment_symbol_to_value_map(assignment) display_median_to_students: assignment.display_median_to_students, group_name_autogenerated: assignment.group_name_autogenerated, is_hidden: assignment.is_hidden, + visible_on: assignment.visible_on, + visible_until: assignment.visible_until, vcs_submit: assignment.vcs_submit, has_peer_review: assignment.has_peer_review } end diff --git a/spec/models/grouping_spec.rb b/spec/models/grouping_spec.rb index 5619d50d39..c186548234 100644 --- a/spec/models/grouping_spec.rb +++ b/spec/models/grouping_spec.rb @@ -1274,6 +1274,11 @@ def expect_updated_criteria_coverage_count_eq(expected_count) expect(data[0]['test_results'][0]['feedback_files']).to eq expected end + + it 'should include the error_type field for each test group result' do + error_type = data[0]['test_results'][0]['test_group_results.error_type'] + expect(error_type).to eq(test_run.test_group_results.first.error_type) + end else it 'should not return data' do expect(data).to be_empty @@ -1641,25 +1646,25 @@ def expect_updated_criteria_coverage_count_eq(expected_count) end it 'should let one navigate right if there is a result directly to the right' do - groupings = assignment.groupings.joins(:group).order('group_name') + groupings = assignment.groupings.joins(:group).order(:group_name) new_grouping = groupings.first.get_next_grouping(role, false) expect(new_grouping.group_id).to eq(groupings.last.group_id) end it 'should let one navigate left if there is a result directly to the left' do - groupings = assignment.groupings.joins(:group).order('group_name') + groupings = assignment.groupings.joins(:group).order(:group_name) new_grouping = groupings.last.get_next_grouping(role, true) expect(new_grouping.group_id).to eq(groupings.first.group_id) end it 'should not one navigate right if there is no result directly to the right' do - groupings = assignment.groupings.joins(:group).order('group_name') + groupings = assignment.groupings.joins(:group).order(:group_name) new_grouping = groupings.last.get_next_grouping(role, false) expect(new_grouping).to be_nil end it 'should not let one navigate left if there is no result directly to the left' do - groupings = assignment.groupings.joins(:group).order('group_name') + groupings = assignment.groupings.joins(:group).order(:group_name) new_grouping = groupings.first.get_next_grouping(role, true) expect(new_grouping).to be_nil end @@ -1668,13 +1673,13 @@ def expect_updated_criteria_coverage_count_eq(expected_count) let(:groupings_collected) { [false, true] } it 'should let me navigate to the right if any result exists towards the right' do - groupings = assignment.groupings.joins(:group).order('group_name') + groupings = assignment.groupings.joins(:group).order(:group_name) new_grouping = groupings.first.get_next_grouping(role, false) expect(new_grouping.group_id).to eq(groupings.last.group_id) end it 'should let one navigate left if there is a result directly to the left' do - groupings = assignment.groupings.joins(:group).order('group_name') + groupings = assignment.groupings.joins(:group).order(:group_name) new_grouping = groupings.last.get_next_grouping(role, true) expect(new_grouping.group_id).to eq(groupings.first.group_id) end @@ -1695,25 +1700,25 @@ def expect_updated_criteria_coverage_count_eq(expected_count) end it 'should let one navigate right if there is a result directly to the right' do - groupings = assignment.groupings.joins(:group).order('group_name') + groupings = assignment.groupings.joins(:group).order(:group_name) new_grouping = groupings.first.get_next_grouping(role, false) expect(new_grouping.group_id).to be(groupings.last.group_id) end it 'should let one navigate left if there is a result directly to the left' do - groupings = assignment.groupings.joins(:group).order('group_name') + groupings = assignment.groupings.joins(:group).order(:group_name) new_grouping = groupings.last.get_next_grouping(role, true) expect(new_grouping.group_id).to be(groupings.first.group_id) end it 'should not one navigate right if there is no result directly to the right' do - groupings = assignment.groupings.joins(:group).order('group_name') + groupings = assignment.groupings.joins(:group).order(:group_name) new_grouping = groupings.last.get_next_grouping(role, false) expect(new_grouping).to be_nil end it 'should not let one navigate left if there is no result directly to the left' do - groupings = assignment.groupings.joins(:group).order('group_name') + groupings = assignment.groupings.joins(:group).order(:group_name) new_grouping = groupings.first.get_next_grouping(role, true) expect(new_grouping).to be_nil end @@ -1727,13 +1732,13 @@ def expect_updated_criteria_coverage_count_eq(expected_count) end it 'should let me navigate to the right if any result exists towards the right' do - groupings = assignment.groupings.joins(:group).order('group_name') + groupings = assignment.groupings.joins(:group).order(:group_name) new_grouping = groupings.first.get_next_grouping(role, false) expect(new_grouping.group_id).to eq(groupings.last.group_id) end it 'should let one navigate left if there is a result directly to the left' do - groupings = assignment.groupings.joins(:group).order('group_name') + groupings = assignment.groupings.joins(:group).order(:group_name) new_grouping = groupings.last.get_next_grouping(role, true) expect(new_grouping.group_id).to eq(groupings.first.group_id) end diff --git a/spec/models/level_spec.rb b/spec/models/level_spec.rb index 390acfcaf1..039805df66 100644 --- a/spec/models/level_spec.rb +++ b/spec/models/level_spec.rb @@ -7,11 +7,4 @@ it { is_expected.to have_one(:course) } it { is_expected.to validate_numericality_of(:mark).is_greater_than_or_equal_to(0) } - - describe 'uniqueness validations' do - subject { create(:level, mark: 0.5) } - - it { is_expected.to validate_uniqueness_of(:mark).scoped_to(:criterion_id) } - it { is_expected.to validate_uniqueness_of(:name).scoped_to(:criterion_id) } - end end diff --git a/spec/models/mark_spec.rb b/spec/models/mark_spec.rb index e45f0c488c..56981e756e 100644 --- a/spec/models/mark_spec.rb +++ b/spec/models/mark_spec.rb @@ -58,9 +58,9 @@ create(:rubric_mark, mark: 4) end - it 'equals to mark times weight' do + it 'equals to mark times max mark' do related_rubric = rubric_mark.criterion - expect(rubric_mark.mark).to eq(related_rubric.weight) + expect(rubric_mark.mark).to eq(related_rubric.max_mark) end end diff --git a/spec/models/penalty_decay_period_submission_rule_spec.rb b/spec/models/penalty_decay_period_submission_rule_spec.rb index c1c08be754..bf55e6bdfe 100644 --- a/spec/models/penalty_decay_period_submission_rule_spec.rb +++ b/spec/models/penalty_decay_period_submission_rule_spec.rb @@ -9,7 +9,8 @@ Timecop.freeze(due_date + submission_time_offset) do apply_rule rule_overtime_message = rule.overtime_message(grouping) - expected_overtime_message = I18n.t 'penalty_decay_period_submission_rules.overtime_message', + penalty_suffix = rule.penalty_type || ExtraMark::PERCENTAGE + expected_overtime_message = I18n.t "penalty_decay_period_submission_rules.overtime_message_#{penalty_suffix}", potential_penalty: potential_penalty expect(rule_overtime_message).to eq expected_overtime_message end @@ -64,4 +65,60 @@ it_behaves_like 'valid overtime message', 2.0, 25.hours end + + context 'when penalty_type is percentage_of_mark' do + before { rule.update!(penalty_type: ExtraMark::PERCENTAGE_OF_MARK) } + + context 'when the group submitted during the first penalty period' do + include_context 'submission_rule_during_first' + + it 'creates a percentage_of_mark penalty' do + apply_rule + expect(result.extra_marks.first.unit).to eq ExtraMark::PERCENTAGE_OF_MARK + expect(result.extra_marks.first.extra_mark).to eq(-1.0) + end + + it_behaves_like 'valid overtime message', 1.0, 10.hours + end + + context 'when the group submitted during the second penalty period' do + include_context 'submission_rule_during_second' + + it 'creates a percentage_of_mark penalty' do + apply_rule + expect(result.extra_marks.first.unit).to eq ExtraMark::PERCENTAGE_OF_MARK + expect(result.extra_marks.first.extra_mark).to eq(-2.0) + end + + it_behaves_like 'valid overtime message', 2.0, 25.hours + end + end + + context 'when penalty_type is points' do + before { rule.update!(penalty_type: ExtraMark::POINTS) } + + context 'when the group submitted during the first penalty period' do + include_context 'submission_rule_during_first' + + it 'creates a points penalty' do + apply_rule + expect(result.extra_marks.first.unit).to eq ExtraMark::POINTS + expect(result.extra_marks.first.extra_mark).to eq(-1.0) + end + + it_behaves_like 'valid overtime message', 1.0, 10.hours + end + + context 'when the group submitted during the second penalty period' do + include_context 'submission_rule_during_second' + + it 'creates a points penalty' do + apply_rule + expect(result.extra_marks.first.unit).to eq ExtraMark::POINTS + expect(result.extra_marks.first.extra_mark).to eq(-2.0) + end + + it_behaves_like 'valid overtime message', 2.0, 25.hours + end + end end diff --git a/spec/models/penalty_period_submission_rule_spec.rb b/spec/models/penalty_period_submission_rule_spec.rb index da7292f887..d347cad2af 100644 --- a/spec/models/penalty_period_submission_rule_spec.rb +++ b/spec/models/penalty_period_submission_rule_spec.rb @@ -9,7 +9,8 @@ Timecop.freeze(due_date + submission_time_offset) do apply_rule rule_overtime_message = rule.overtime_message(grouping) - expected_overtime_message = I18n.t 'penalty_decay_period_submission_rules.overtime_message', + penalty_suffix = rule.penalty_type || ExtraMark::PERCENTAGE + expected_overtime_message = I18n.t "penalty_period_submission_rules.overtime_message_#{penalty_suffix}", potential_penalty: potential_penalty expect(rule_overtime_message).to eq expected_overtime_message end @@ -64,4 +65,60 @@ it_behaves_like 'valid overtime message', 2.0, 25.hours end + + context 'when penalty_type is percentage_of_mark' do + before { rule.update!(penalty_type: ExtraMark::PERCENTAGE_OF_MARK) } + + context 'when the group submitted during the first penalty period' do + include_context 'submission_rule_during_first' + + it 'creates a percentage_of_mark penalty' do + apply_rule + expect(result.extra_marks.first.unit).to eq ExtraMark::PERCENTAGE_OF_MARK + expect(result.extra_marks.first.extra_mark).to eq(-1.0) + end + + it_behaves_like 'valid overtime message', 1.0, 10.hours + end + + context 'when the group submitted during the second penalty period' do + include_context 'submission_rule_during_second' + + it 'creates a percentage_of_mark penalty' do + apply_rule + expect(result.extra_marks.first.unit).to eq ExtraMark::PERCENTAGE_OF_MARK + expect(result.extra_marks.first.extra_mark).to eq(-2.0) + end + + it_behaves_like 'valid overtime message', 2.0, 25.hours + end + end + + context 'when penalty_type is points' do + before { rule.update!(penalty_type: ExtraMark::POINTS) } + + context 'when the group submitted during the first penalty period' do + include_context 'submission_rule_during_first' + + it 'creates a points penalty' do + apply_rule + expect(result.extra_marks.first.unit).to eq ExtraMark::POINTS + expect(result.extra_marks.first.extra_mark).to eq(-1.0) + end + + it_behaves_like 'valid overtime message', 1.0, 10.hours + end + + context 'when the group submitted during the second penalty period' do + include_context 'submission_rule_during_second' + + it 'creates a points penalty' do + apply_rule + expect(result.extra_marks.first.unit).to eq ExtraMark::POINTS + expect(result.extra_marks.first.extra_mark).to eq(-2.0) + end + + it_behaves_like 'valid overtime message', 2.0, 25.hours + end + end end diff --git a/spec/models/result_spec.rb b/spec/models/result_spec.rb index fe5d70668e..de7f34a45f 100644 --- a/spec/models/result_spec.rb +++ b/spec/models/result_spec.rb @@ -172,6 +172,58 @@ expect(Result.get_total_extra_marks(ids)).to eq(expected) end end + + context 'with percentage_of_mark extra marks' do + it 'should calculate extra marks based on percentage of earned score' do + ids = Result.ids + result = Result.find(ids.first) + subtotal = result.get_subtotal + extra_mark = 5.0 + + create(:extra_mark_percentage_of_mark, result: result, extra_mark: extra_mark) + expected = Hash.new { |h, k| h[k] = nil } + expected[ids.first] = (extra_mark * subtotal / 100).round(2) + expect(Result.get_total_extra_marks(ids)).to eq(expected) + end + + it 'should handle zero subtotal when calculating percentage of score' do + ids = Result.ids + result = Result.find(ids.first) + result.marks.update_all(mark: 0) + extra_mark = 10.0 + + create(:extra_mark_percentage_of_mark, result: result, extra_mark: extra_mark) + expected = Hash.new { |h, k| h[k] = nil } + expected[ids.first] = 0.0 + expect(Result.get_total_extra_marks(ids)).to eq(expected) + end + + it 'should calculate extra marks based on percentage of earned score with negative deduction' do + ids = Result.ids + result = Result.find(ids.first) + subtotal = result.get_subtotal + extra_mark = -5.0 + + create(:extra_mark_percentage_of_mark, result: result, extra_mark: extra_mark) + expected = Hash.new { |h, k| h[k] = nil } + expected[ids.first] = (extra_mark * subtotal / 100).round(2) + expect(Result.get_total_extra_marks(ids)).to eq(expected) + end + + it 'should round to 2 decimal places' do + ids = Result.ids + result = Result.find(ids.first) + subtotal = result.get_subtotal + extra_mark = 7.0 + + create(:extra_mark_percentage_of_mark, result: result, extra_mark: extra_mark) + actual = Result.get_total_extra_marks(ids)[ids.first] + expect(actual).to eq(actual.round(2)) + + expected = (extra_mark * subtotal / 100).round(2) + expect(actual).to eq(expected) + end + end end describe '#get_total_mark' do diff --git a/spec/models/rubric_criterion_spec.rb b/spec/models/rubric_criterion_spec.rb index ac58582630..dcce6de781 100644 --- a/spec/models/rubric_criterion_spec.rb +++ b/spec/models/rubric_criterion_spec.rb @@ -190,6 +190,22 @@ end context 'has basic levels functionality' do + describe 'uniqueness validations' do + it 'raise error on duplicate mark' do + @criterion.levels.create(name: 'A', description: 'A', mark: 5) + expect do + @criterion.levels.create!(name: 'B', description: 'B', mark: 5) + end.to raise_error(ActiveRecord::RecordInvalid, /Mark has already been taken/) + end + + it 'raise error on duplicate name' do + @criterion.levels.create(name: 'A', description: 'A', mark: 5) + expect do + @criterion.levels.create!(name: 'A', description: 'B', mark: 6) + end.to raise_error(ActiveRecord::RecordInvalid, /Name has already been taken/) + end + end + describe 'can add levels' do it 'not raise error' do expect(@criterion.levels.length).to eq(5) @@ -199,6 +215,23 @@ end end + describe 'can swap values in a single transaction' do + it 'not raise error' do + # Create records + a = @criterion.levels.create(name: 'testname', description: 'Description for level', mark: '5') + b = @criterion.levels.create(name: 'sillyname', description: 'Description for level 2', mark: '6') + + @criterion.update!( + levels_attributes: [ + { id: a.id, name: 'placeholder' }, + { id: b.id, name: 'testname' } + ] + ) + expect(@criterion.levels.find(a.id).name).to eq 'placeholder' + expect(@criterion.levels.find(b.id).name).to eq 'testname' + end + end + describe 'can delete levels' do it 'not raise error' do expect(@criterion.levels.length).to eq(5) diff --git a/spec/models/starter_file_entry_spec.rb b/spec/models/starter_file_entry_spec.rb index 73fd91c4a5..429dba0d35 100644 --- a/spec/models/starter_file_entry_spec.rb +++ b/spec/models/starter_file_entry_spec.rb @@ -49,7 +49,7 @@ it 'should add files to an open zip file' do FileUtils.rm_f(zip_path) - Zip::File.open(zip_path, Zip::File::CREATE) do |zip_file| + Zip::File.open(zip_path, create: true) do |zip_file| starter_file_entry.add_files_to_zip_file(zip_file) end Zip::File.open(zip_path) do |zip_file| diff --git a/spec/models/student_membership_spec.rb b/spec/models/student_membership_spec.rb index 41ffef772e..72087cfdcc 100644 --- a/spec/models/student_membership_spec.rb +++ b/spec/models/student_membership_spec.rb @@ -143,8 +143,8 @@ def namer(bool) expect(membership.inviter?).to be true end - update_hash = { create: true, destroy: true, inviter: false, accepted: false, pending: true, rejected: true } - it_behaves_like 'vcs_submit=true', :inviter_student_membership, update_hash + it_behaves_like 'vcs_submit=true', :inviter_student_membership, + { create: true, destroy: true, inviter: false, accepted: false, pending: true, rejected: true } it_behaves_like 'vcs_submit=false', :inviter_student_membership end @@ -155,8 +155,8 @@ def namer(bool) expect(membership.inviter?).to be false end - update_hash = { create: true, destroy: true, inviter: false, accepted: false, pending: true, rejected: true } - it_behaves_like 'vcs_submit=true', :accepted_student_membership, update_hash + it_behaves_like 'vcs_submit=true', :accepted_student_membership, + { create: true, destroy: true, inviter: false, accepted: false, pending: true, rejected: true } it_behaves_like 'vcs_submit=false', :accepted_student_membership end @@ -167,8 +167,8 @@ def namer(bool) expect(membership.inviter?).to be false end - update_hash = { create: false, destroy: false, inviter: true, accepted: true, pending: false, rejected: false } - it_behaves_like 'vcs_submit=true', :student_membership, update_hash + it_behaves_like 'vcs_submit=true', :student_membership, + { create: false, destroy: false, inviter: true, accepted: true, pending: false, rejected: false } it_behaves_like 'vcs_submit=false', :student_membership end @@ -179,8 +179,8 @@ def namer(bool) expect(membership.inviter?).to be false end - update_hash = { create: false, destroy: false, inviter: true, accepted: true, pending: false, rejected: false } - it_behaves_like 'vcs_submit=true', :rejected_student_membership, update_hash + it_behaves_like 'vcs_submit=true', :rejected_student_membership, + { create: false, destroy: false, inviter: true, accepted: true, pending: false, rejected: false } it_behaves_like 'vcs_submit=false', :rejected_student_membership end end diff --git a/spec/models/student_spec.rb b/spec/models/student_spec.rb index 63851a3bba..0fee6e98ac 100644 --- a/spec/models/student_spec.rb +++ b/spec/models/student_spec.rb @@ -395,4 +395,119 @@ expect(build(:student, user: create(:autotest_user))).not_to be_valid end end + + describe '#visible_assessments with datetime visibility' do + let(:course) { create(:course) } + let(:student) { create(:student, course: course) } + let(:section) { create(:section, course: course) } + let(:student_with_section) { create(:student, course: course, section: section) } + + context 'with global visibility (no section)' do + it 'returns assessment when is_hidden=false and no datetime set' do + assignment = create(:assignment, course: course, is_hidden: false) + expect(student.visible_assessments).to include(assignment) + end + + it 'hides assessment when is_hidden=true and no datetime set' do + assignment = create(:assignment, course: course, is_hidden: true) + expect(student.visible_assessments).not_to include(assignment) + end + + it 'shows assessment when datetime range includes current time (overrides is_hidden=true)' do + assignment = create(:assignment, course: course, is_hidden: true, + visible_on: 1.day.ago, visible_until: 1.day.from_now) + expect(student.visible_assessments).to include(assignment) + end + + it 'hides assessment when visible_on is in the future' do + assignment = create(:assignment, course: course, is_hidden: false, + visible_on: 1.day.from_now, visible_until: 2.days.from_now) + expect(student.visible_assessments).not_to include(assignment) + end + + it 'hides assessment when visible_until is in the past' do + assignment = create(:assignment, course: course, is_hidden: false, + visible_on: 2.days.ago, visible_until: 1.day.ago) + expect(student.visible_assessments).not_to include(assignment) + end + + it 'shows assessment when only visible_on is set and in the past' do + assignment = create(:assignment, course: course, is_hidden: true, + visible_on: 1.day.ago, visible_until: nil) + expect(student.visible_assessments).to include(assignment) + end + + it 'shows assessment when only visible_until is set and in the future' do + assignment = create(:assignment, course: course, is_hidden: true, + visible_on: nil, visible_until: 1.day.from_now) + expect(student.visible_assessments).to include(assignment) + end + end + + context 'with section-specific visibility' do + it 'uses section-specific is_hidden when no datetime set' do + assignment = create(:assignment, course: course, is_hidden: true) + create(:assessment_section_properties, assessment: assignment, section: section, is_hidden: false) + expect(student_with_section.visible_assessments).to include(assignment) + end + + it 'section is_hidden=true overrides global is_hidden=false' do + assignment = create(:assignment, course: course, is_hidden: false) + create(:assessment_section_properties, assessment: assignment, section: section, is_hidden: true) + expect(student_with_section.visible_assessments).not_to include(assignment) + end + + it 'section datetime overrides global datetime' do + assignment = create(:assignment, course: course, is_hidden: false, + visible_on: 1.day.ago, visible_until: 1.day.from_now) + # Section says it's not visible yet + create(:assessment_section_properties, assessment: assignment, section: section, + visible_on: 1.day.from_now, visible_until: 2.days.from_now) + expect(student_with_section.visible_assessments).not_to include(assignment) + end + + it 'section datetime makes hidden assessment visible when range includes current time' do + assignment = create(:assignment, course: course, is_hidden: true) + # Section has datetime visibility + create(:assessment_section_properties, assessment: assignment, section: section, + visible_on: 1.day.ago, visible_until: 1.day.from_now) + expect(student_with_section.visible_assessments).to include(assignment) + end + + it 'hides assessment when section visible_on is in the future' do + assignment = create(:assignment, course: course, is_hidden: false) + create(:assessment_section_properties, assessment: assignment, section: section, + visible_on: 1.day.from_now, visible_until: 2.days.from_now) + expect(student_with_section.visible_assessments).not_to include(assignment) + end + + it 'hides assessment when section visible_until is in the past' do + assignment = create(:assignment, course: course, is_hidden: false) + create(:assessment_section_properties, assessment: assignment, section: section, + visible_on: 2.days.ago, visible_until: 1.day.ago) + expect(student_with_section.visible_assessments).not_to include(assignment) + end + end + + context 'with assessment_id parameter' do + it 'returns specific assessment if visible' do + assignment = create(:assignment, course: course, is_hidden: false) + result = student.visible_assessments(assessment_id: assignment.id) + expect(result).to include(assignment) + end + + it 'returns empty if specific assessment is not visible' do + assignment = create(:assignment, course: course, is_hidden: true) + result = student.visible_assessments(assessment_id: assignment.id) + expect(result).not_to include(assignment) + end + + it 'respects datetime visibility for specific assessment' do + assignment = create(:assignment, course: course, is_hidden: true, + visible_on: 1.day.ago, visible_until: 1.day.from_now) + result = student.visible_assessments(assessment_id: assignment.id) + expect(result).to include(assignment) + end + end + end end diff --git a/spec/models/submission_rule_spec.rb b/spec/models/submission_rule_spec.rb index d79a3f4b75..21bb08ba7c 100644 --- a/spec/models/submission_rule_spec.rb +++ b/spec/models/submission_rule_spec.rb @@ -1,26 +1,24 @@ -shared_examples 'due_date_calculations' do |assignment_past, section_past, section_enabled: true| +shared_examples 'due_date_calculations' do |assignment_past, section_past_param, section_enabled: true| assignment_due_date_str = 'assignment due date' - section_due_date_str = 'section_due_date' + section_due_date_str = section_enabled ? 'section_due_date' : assignment_due_date_str + section_past = section_enabled ? section_past_param : assignment_past let(:assignment_due_date) { assignment.due_date } let(:section_due_date_due_date) { section_enabled ? section_due_date.due_date : assignment_due_date } - unless section_enabled - section_past = assignment_past - section_due_date_str = assignment_due_date_str - end - before { assignment.reload } describe '#can_collect_all_now?' do it "should return #{assignment_past && section_past}" do - expect(assignment.submission_rule.can_collect_all_now?).to eq(assignment_past && section_past) + section_past_value = section_enabled ? section_past_param : assignment_past + expect(assignment.submission_rule.can_collect_all_now?).to eq(assignment_past && section_past_value) end end describe '#can_collect_grouping_now?(grouping) with section' do it "should return #{section_past}" do - expect(assignment.submission_rule.can_collect_grouping_now?(grouping_with_section)).to eq(section_past) + section_past_value = section_enabled ? section_past_param : assignment_past + expect(assignment.submission_rule.can_collect_grouping_now?(grouping_with_section)).to eq(section_past_value) end end diff --git a/spec/models/test_group_result_spec.rb b/spec/models/test_group_result_spec.rb index 6c0d9d6f19..f8ad206db5 100644 --- a/spec/models/test_group_result_spec.rb +++ b/spec/models/test_group_result_spec.rb @@ -95,5 +95,21 @@ expect(@test_group_result).not_to be_valid end end + + context 'test group result with timeout error' do + it 'has timeout status displayed in the test run table' do + @test_group_result.error_type = :timeout + expect(@test_group_result).to be_valid + expect(@test_group_result.save).to be true + expect(@test_group_result.error_type).to eq('timeout') + end + + it 'has no status displayed in the test run table' do + @test_group_result.error_type = nil + expect(@test_group_result).to be_valid + expect(@test_group_result.save).to be true + expect(@test_group_result.error_type).to be_nil + end + end end end diff --git a/spec/models/test_run_spec.rb b/spec/models/test_run_spec.rb index d64bd090a4..a8556dc896 100644 --- a/spec/models/test_run_spec.rb +++ b/spec/models/test_run_spec.rb @@ -173,6 +173,151 @@ }] }.deep_stringify_keys end + context 'extra marks' do + let(:base_results) { results.deep_dup } # keep original intact + + shared_context 'extra marks setup' do + before do + submission.reload + submission.current_result.update!(released_to_students: false) + allow(grouping).to receive(:current_submission_used).and_return(submission) + end + end + + context 'when the test run has a submission and a valid POINTS extra marks' do + include_context 'extra marks setup' + + let(:submission) { create(:version_used_submission, grouping: grouping) } + let(:test_run) do + create(:test_run, status: :in_progress, submission: submission, grouping: grouping, autotest_test_id: 1) + end + + before do + base_results['test_groups'].first['extra_marks'] = [ + { + 'mark' => 3, + 'unit' => 'points', + 'description' => 'this is a test comment' + } + ] + end + + it 'creates an ExtraMark on the submission result' do + expect { test_run.update_results!(base_results) }.to change { ExtraMark.count }.by(1) + em = ExtraMark.last + expect(em.unit).to eq ExtraMark::POINTS + expect(em.extra_mark).to eq 3 + expect(em.result_id).to eq submission.current_result.id + expect(em.description).to eq 'this is a test comment' + end + + it 'does not add the points to the test_group_result.marks_earned' do + test_run.update_results!(base_results) + tgr = TestGroupResult.find_by(test_group_id: test_group.id, test_run_id: test_run.id) + # baseline earned = 1 (from first test), extra marks are not added to marks_earned + expect(tgr.marks_earned).to eq 1 + expect(tgr.marks_total).to eq 2 # unchanged + end + end + + context 'when the test run has a submission and a non-POINTS unit (percentage)' do + include_context 'extra marks setup' + + let(:submission) { create(:version_used_submission, grouping: grouping) } + let(:test_run) do + create(:test_run, status: :in_progress, submission: submission, grouping: grouping, autotest_test_id: 1) + end + + before do + base_results['test_groups'].first['extra_marks'] = [ + { 'unit' => ExtraMark::PERCENTAGE, 'mark' => 10, 'description' => '10% bump' } + ] + end + + it 'creates an ExtraMark but does not change marks_earned' do + expect { test_run.update_results!(base_results) }.to change { ExtraMark.count }.by(1) + tgr = TestGroupResult.find_by(test_group_id: test_group.id, test_run_id: test_run.id) + # baseline earned remains 1 since non-POINTS units donβt add to marks_earned + expect(tgr.marks_earned).to eq 1 + expect(tgr.marks_total).to eq 2 + end + end + + context 'when the unit is invalid' do + include_context 'extra marks setup' + + let(:submission) { create(:version_used_submission, grouping: grouping) } + let(:test_run) do + create(:test_run, status: :in_progress, submission: submission, grouping: grouping, autotest_test_id: 1) + end + + before do + base_results['test_groups'].first['extra_marks'] = [ + { 'unit' => 'bananas', 'mark' => 99, 'description' => 'nope' } + ] + end + + it 'ignores the comment and does not create an ExtraMark nor change marks' do + expect { test_run.update_results!(base_results) }.not_to(change { ExtraMark.count }) + tgr = TestGroupResult.find_by(test_group_id: test_group.id, test_run_id: test_run.id) + expect(tgr.marks_earned).to eq 1 + expect(tgr.marks_total).to eq 2 + end + end + + context 'when extra_marks is an empty array' do + include_context 'extra marks setup' + + let(:submission) { create(:version_used_submission, grouping: grouping) } + let(:test_run) do + create(:test_run, status: :in_progress, submission: submission, grouping: grouping, autotest_test_id: 1) + end + + before do + base_results['test_groups'].first['extra_marks'] = [] + end + + it 'does nothing' do + expect { test_run.update_results!(base_results) }.not_to(change { ExtraMark.count }) + tgr = TestGroupResult.find_by(test_group_id: test_group.id, test_run_id: test_run.id) + expect(tgr.marks_earned).to eq 1 + expect(tgr.marks_total).to eq 2 + end + end + + context 'when extra_marks is missing' do + let(:submission) { create(:version_used_submission, grouping: grouping) } + let(:test_run) do + create(:test_run, status: :in_progress, submission: submission, grouping: grouping, autotest_test_id: 1) + end + + it 'does nothing' do + expect { test_run.update_results!(base_results) }.not_to(change { ExtraMark.count }) + tgr = TestGroupResult.find_by(test_group_id: test_group.id, test_run_id: test_run.id) + expect(tgr.marks_earned).to eq 1 + expect(tgr.marks_total).to eq 2 + end + end + + context 'when there is no submission result (no submission on the test run)' do + let(:test_run) { create(:test_run, status: :in_progress, grouping: grouping, autotest_test_id: 1) } + + before do + base_results['test_groups'].first['extra_marks'] = [ + { 'unit' => ExtraMark::POINTS, 'mark' => 5, 'description' => 'should be ignored' } + ] + end + + it 'does not create ExtraMark and does not change marks_earned' do + expect { test_run.update_results!(base_results) }.not_to(change { ExtraMark.count }) + tgr = TestGroupResult.find_by(test_group_id: test_group.id, test_run_id: test_run.id) + # no submission => add_extra_marks returns 0, so only earned = 1 baseline + expect(tgr.marks_earned).to eq 1 + expect(tgr.marks_total).to eq 2 + end + end + end + context 'when there are feedback files' do let(:feedback_files) { test_group.test_group_results.first.feedback_files } diff --git a/spec/policies/assignment_policy_spec.rb b/spec/policies/assignment_policy_spec.rb index 32fc7171d8..f55fb09b37 100644 --- a/spec/policies/assignment_policy_spec.rb +++ b/spec/policies/assignment_policy_spec.rb @@ -423,5 +423,57 @@ before { assessment_section_properties.update(is_hidden: true) } end end + + context 'with datetime visibility' do + let(:role) { create(:student) } + + succeed 'when visible_on is in the past and visible_until is in the future' do + before { assignment.update(is_hidden: true, visible_on: 1.day.ago, visible_until: 1.day.from_now) } + end + + failed 'when visible_on is in the future' do + before { assignment.update(is_hidden: false, visible_on: 1.day.from_now, visible_until: 2.days.from_now) } + end + + failed 'when visible_until is in the past' do + before { assignment.update(is_hidden: false, visible_on: 2.days.ago, visible_until: 1.day.ago) } + end + + succeed 'when only visible_on is set and in the past' do + before { assignment.update(is_hidden: true, visible_on: 1.day.ago, visible_until: nil) } + end + + succeed 'when only visible_until is set and in the future' do + before { assignment.update(is_hidden: true, visible_on: nil, visible_until: 1.day.from_now) } + end + + context 'with section-specific datetime' do + let(:role) { create(:student, section: new_section) } + + succeed 'when section datetime is valid' do + before do + assignment.update(is_hidden: true) + create(:assessment_section_properties, assessment: assignment, section: new_section, + visible_on: 1.day.ago, visible_until: 1.day.from_now) + end + end + + failed 'when section visible_on is in the future' do + before do + assignment.update(is_hidden: false) + create(:assessment_section_properties, assessment: assignment, section: new_section, + visible_on: 1.day.from_now, visible_until: 2.days.from_now) + end + end + + failed 'when section visible_until is in the past' do + before do + assignment.update(is_hidden: false) + create(:assessment_section_properties, assessment: assignment, section: new_section, + visible_on: 2.days.ago, visible_until: 1.day.ago) + end + end + end + end end end diff --git a/spec/routing/routes_spec.rb b/spec/routing/routes_spec.rb index f9f655bcb4..4ac12671e8 100644 --- a/spec/routing/routes_spec.rb +++ b/spec/routing/routes_spec.rb @@ -1,7 +1,7 @@ describe 'routing' do Rails.application.routes.routes.map do |r| spec = r.path.spec.to_s - next if spec.match %r{^/rails|^/assets|^/cable|.*\*path\(.:format\)}.freeze + next if %r{^/rails|^/assets|^/cable|.*\*path\(.:format\)}.match?(spec) r.verb.split('|').each do |verb| it "#{verb}: #{r.path.spec}" do parts = r.required_parts.index_with { |_part| '1' } diff --git a/spec/support/course_association_shared_examples.rb b/spec/support/course_association_shared_examples.rb index 8d36edffb4..5a24dd5973 100644 --- a/spec/support/course_association_shared_examples.rb +++ b/spec/support/course_association_shared_examples.rb @@ -1,5 +1,6 @@ shared_examples 'course associations' do include CourseAssociationHelper + it 'should be valid when all belongs_to associations belong to the same course' do expect(subject).to be_valid end diff --git a/spec/support/lti_controller_examples.rb b/spec/support/lti_controller_examples.rb index f98a709d1d..7c06c869fc 100644 --- a/spec/support/lti_controller_examples.rb +++ b/spec/support/lti_controller_examples.rb @@ -19,6 +19,29 @@ root_uri.query = launch_params.to_query root_uri.to_s end + let(:mock_roles) { [LtiDeployment::LTI_ROLES[:instructor]] } + + def create_pub_jwk + @create_pub_jwk ||= JWT::JWK.new(OpenSSL::PKey::RSA.new(1024)) + end + + def generate_payload(roles, nonce) + { aud: client_id, + iss: host, + nonce: nonce, + LtiDeployment::LTI_CLAIMS[:deployment_id] => 'some_deployment_id', + LtiDeployment::LTI_CLAIMS[:context] => { label: 'csc108', title: 'test' }, + LtiDeployment::LTI_CLAIMS[:custom] => { course_id: 1, user_id: 1 }, + LtiDeployment::LTI_CLAIMS[:user_id] => 'some_user_id', + LtiDeployment::LTI_CLAIMS[:roles] => roles } + end + + def generate_lti_jwt(roles, nonce) + payload = generate_payload(roles, nonce) + pub_jwk = create_pub_jwk + JWT.encode(payload, pub_jwk.keypair, 'RS256', { kid: pub_jwk.kid }) + end + describe '#launch' do context 'when launching with invalid parameters' do let(:lti_message_hint) { 'opaque string' } @@ -27,33 +50,33 @@ it 'responds with unprocessable_entity if no parameters are passed' do request.headers['Referer'] = host post :launch, params: {} - expect(subject).to respond_with(:unprocessable_entity) + expect(subject).to respond_with(:unprocessable_content) end it 'responds with unprocessable_entity if lti_message_hint is not passed' do request.headers['Referer'] = host post :launch, params: { client_id: client_id, target_link_uri: target_link_uri, login_hint: login_hint } - expect(subject).to respond_with(:unprocessable_entity) + expect(subject).to respond_with(:unprocessable_content) end it 'responds with unprocessable_entity if client_id is not passed' do request.headers['Referer'] = host post :launch, params: { lti_message_hint: lti_message_hint, target_link_uri: target_link_uri, login_hint: login_hint } - expect(subject).to respond_with(:unprocessable_entity) + expect(subject).to respond_with(:unprocessable_content) end it 'responds with unprocessable_entity if target_link_uri is not passed' do request.headers['Referer'] = host post :launch, params: { lti_message_hint: lti_message_hint, client_id: client_id, login_hint: login_hint } - expect(subject).to respond_with(:unprocessable_entity) + expect(subject).to respond_with(:unprocessable_content) end it 'responds with unprocessable_entity if login_hint is not passed' do request.headers['Referer'] = host post :launch, params: { lti_message_hint: lti_message_hint, client_id: client_id, target_link_uri: target_link_uri } - expect(subject).to respond_with(:unprocessable_entity) + expect(subject).to respond_with(:unprocessable_content) end context 'when all required params exist' do @@ -98,6 +121,8 @@ let(:nonce) { rand(10 ** 30).to_s.rjust(30, '0') } before do + stub_request(:get, jwk_url).to_return(status: 200, body: { keys: [create_pub_jwk.export] }.to_json) + lti_launch_data = {} lti_launch_data[:client_id] = client_id lti_launch_data[:iss] = host @@ -129,27 +154,10 @@ end context 'with correct parameters' do - let(:payload) do - { aud: client_id, - iss: host, - nonce: nonce, - LtiDeployment::LTI_CLAIMS[:deployment_id] => 'some_deployment_id', - LtiDeployment::LTI_CLAIMS[:context] => { - label: 'csc108', - title: 'test' - }, - LtiDeployment::LTI_CLAIMS[:custom] => { - course_id: 1, - user_id: 1 - }, - LtiDeployment::LTI_CLAIMS[:user_id] => 'some_user_id' } - end - let(:pub_jwk) { JWT::JWK.new(OpenSSL::PKey::RSA.new(1024)) } - let(:lti_jwt) { JWT.encode(payload, pub_jwk.keypair, 'RS256', { kid: pub_jwk.kid }) } + let(:lti_jwt) { generate_lti_jwt(mock_roles, nonce) } before do session[:client_id] = client_id - stub_request(:get, jwk_url).to_return(status: 200, body: { keys: [pub_jwk.export] }.to_json) end it 'successfully decodes the jwt and redirects' do @@ -190,6 +198,53 @@ end end + context 'with LTI role authorization' do + let(:admin_lti_uri) { LtiDeployment::LTI_ROLES[:admin] } + let(:student_lti_uri) { LtiDeployment::LTI_ROLES[:learner] } + let(:ta_lti_uri) { LtiDeployment::LTI_ROLES[:ta] } + let(:instructor_lti_uri) { LtiDeployment::LTI_ROLES[:instructor] } + + context 'when LTI role is Instructor' do + let(:lti_jwt) { generate_lti_jwt([instructor_lti_uri], nonce) } + + it 'redirects to course chooser' do + request.headers['Referer'] = host + post_as instructor, :redirect_login, params: { state: session.id.to_s, id_token: lti_jwt } + expect(response).to redirect_to(choose_course_lti_deployment_path(LtiDeployment.first)) + end + end + + context 'when LTI role is Admin' do + let(:lti_jwt) { generate_lti_jwt([admin_lti_uri], nonce) } + + it 'redirects to course chooser' do + request.headers['Referer'] = host + post_as instructor, :redirect_login, params: { state: session.id.to_s, id_token: lti_jwt } + expect(response).to redirect_to(choose_course_lti_deployment_path(LtiDeployment.first)) + end + end + + context 'when LTI role is Student' do + let(:lti_jwt) { generate_lti_jwt([student_lti_uri], nonce) } + + it 'redirects to "not set up" page' do + request.headers['Referer'] = host + post_as instructor, :redirect_login, params: { state: session.id.to_s, id_token: lti_jwt } + expect(response).to redirect_to(course_not_set_up_lti_deployment_path(LtiDeployment.first)) + end + end + + context 'when LTI role is TA (even with Instructor claim)' do + let(:lti_jwt) { generate_lti_jwt([ta_lti_uri, instructor_lti_uri], nonce) } + + it 'redirects to "not set up" page' do + request.headers['Referer'] = host + post_as instructor, :redirect_login, params: { state: session.id.to_s, id_token: lti_jwt } + expect(response).to redirect_to(course_not_set_up_lti_deployment_path(LtiDeployment.first)) + end + end + end + context 'get' do it 'returns an error if not logged in' do request.headers['Referer'] = host @@ -212,7 +267,8 @@ lms_course_name: 'Introduction to Computer Science', lms_course_label: 'CSC108', lms_course_id: 1, - lti_user_id: 'user_id' } + lti_user_id: 'user_id', + user_roles: mock_roles } end let(:payload) do { aud: client_id, @@ -225,14 +281,12 @@ LtiDeployment::LTI_CLAIMS[:custom] => { course_id: 1, user_id: 1 - } } + }, + LtiDeployment::LTI_CLAIMS[:roles] => mock_roles } end - let(:pub_jwk) { JWT::JWK.new(OpenSSL::PKey::RSA.new(1024)) } - let(:lti_jwt) { JWT.encode(payload, pub_jwk.keypair, 'RS256', { kid: pub_jwk.kid }) } before do cookies.permanent.encrypted[:lti_data] = { value: JSON.generate(lti_data), expires: 5.minutes.from_now } - stub_request(:get, jwk_url).to_return(status: 200, body: { keys: [pub_jwk.export] }.to_json) end it 'successfully decodes the jwt and redirects' do @@ -294,7 +348,7 @@ post_as instructor, :launch, params: { lti_message_hint: 'hint', login_hint: 'hint', client_id: 'LMS defined ID', target_link_uri: 'test.com' } - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:unprocessable_content) end end end |