diff --git a/Gemfile b/Gemfile index d7585007..7f83756c 100644 --- a/Gemfile +++ b/Gemfile @@ -3,7 +3,7 @@ source 'https://rubygems.org' ruby '~> 3.3' # Bundle edge Rails instead: gem "rails", github: "rails/rails", branch: "main" -gem 'rails', '~> 7.2.3.1' +gem 'rails', '~> 8.1.0' # Use postgres for all env dbs gem 'pg' diff --git a/Gemfile.lock b/Gemfile.lock index 1d063453..5d51c45e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,79 +1,80 @@ GEM remote: https://rubygems.org/ specs: - actioncable (7.2.3.1) - actionpack (= 7.2.3.1) - activesupport (= 7.2.3.1) + action_text-trix (2.1.19) + railties + actioncable (8.1.3) + actionpack (= 8.1.3) + activesupport (= 8.1.3) nio4r (~> 2.0) websocket-driver (>= 0.6.1) zeitwerk (~> 2.6) - actionmailbox (7.2.3.1) - actionpack (= 7.2.3.1) - activejob (= 7.2.3.1) - activerecord (= 7.2.3.1) - activestorage (= 7.2.3.1) - activesupport (= 7.2.3.1) + actionmailbox (8.1.3) + actionpack (= 8.1.3) + activejob (= 8.1.3) + activerecord (= 8.1.3) + activestorage (= 8.1.3) + activesupport (= 8.1.3) mail (>= 2.8.0) - actionmailer (7.2.3.1) - actionpack (= 7.2.3.1) - actionview (= 7.2.3.1) - activejob (= 7.2.3.1) - activesupport (= 7.2.3.1) + actionmailer (8.1.3) + actionpack (= 8.1.3) + actionview (= 8.1.3) + activejob (= 8.1.3) + activesupport (= 8.1.3) mail (>= 2.8.0) rails-dom-testing (~> 2.2) - actionpack (7.2.3.1) - actionview (= 7.2.3.1) - activesupport (= 7.2.3.1) - cgi + actionpack (8.1.3) + actionview (= 8.1.3) + activesupport (= 8.1.3) nokogiri (>= 1.8.5) - racc - rack (>= 2.2.4, < 3.3) + rack (>= 2.2.4) rack-session (>= 1.0.1) rack-test (>= 0.6.3) rails-dom-testing (~> 2.2) rails-html-sanitizer (~> 1.6) useragent (~> 0.16) - actiontext (7.2.3.1) - actionpack (= 7.2.3.1) - activerecord (= 7.2.3.1) - activestorage (= 7.2.3.1) - activesupport (= 7.2.3.1) + actiontext (8.1.3) + action_text-trix (~> 2.1.15) + actionpack (= 8.1.3) + activerecord (= 8.1.3) + activestorage (= 8.1.3) + activesupport (= 8.1.3) globalid (>= 0.6.0) nokogiri (>= 1.8.5) - actionview (7.2.3.1) - activesupport (= 7.2.3.1) + actionview (8.1.3) + activesupport (= 8.1.3) builder (~> 3.1) - cgi erubi (~> 1.11) rails-dom-testing (~> 2.2) rails-html-sanitizer (~> 1.6) - activejob (7.2.3.1) - activesupport (= 7.2.3.1) + activejob (8.1.3) + activesupport (= 8.1.3) globalid (>= 0.3.6) - activemodel (7.2.3.1) - activesupport (= 7.2.3.1) - activerecord (7.2.3.1) - activemodel (= 7.2.3.1) - activesupport (= 7.2.3.1) + activemodel (8.1.3) + activesupport (= 8.1.3) + activerecord (8.1.3) + activemodel (= 8.1.3) + activesupport (= 8.1.3) timeout (>= 0.4.0) - activestorage (7.2.3.1) - actionpack (= 7.2.3.1) - activejob (= 7.2.3.1) - activerecord (= 7.2.3.1) - activesupport (= 7.2.3.1) + activestorage (8.1.3) + actionpack (= 8.1.3) + activejob (= 8.1.3) + activerecord (= 8.1.3) + activesupport (= 8.1.3) marcel (~> 1.0) - activesupport (7.2.3.1) + activesupport (8.1.3) base64 - benchmark (>= 0.3) bigdecimal concurrent-ruby (~> 1.0, >= 1.3.1) connection_pool (>= 2.2.5) drb i18n (>= 1.6, < 2) + json logger (>= 1.4.2) - minitest (>= 5.1, < 6) + minitest (>= 5.1) securerandom (>= 0.3) tzinfo (~> 2.0, >= 2.0.5) + uri (>= 0.13.1) addressable (2.9.0) public_suffix (>= 2.0.2, < 8.0) annotaterb (4.23.0) @@ -87,7 +88,6 @@ GEM axe-core-rspec (4.12.0) axe-core-api (= 4.12.0) base64 (0.3.0) - benchmark (0.5.0) bigdecimal (4.1.2) bindex (0.8.1) blazer (3.4.0) @@ -115,7 +115,6 @@ GEM capybara-screenshot (1.0.27) capybara (>= 1.0, < 4) launchy - cgi (0.5.2) chartkick (5.2.1) childprocess (5.1.0) logger (~> 1.5) @@ -127,7 +126,7 @@ GEM crack (1.0.1) bigdecimal rexml - crass (1.0.6) + crass (1.0.7) csv (3.3.5) cucumber (10.2.0) base64 (~> 0.2) @@ -203,7 +202,7 @@ GEM sassc (~> 2.0) formatador (1.2.3) reline - globalid (1.3.0) + globalid (1.4.0) activesupport (>= 6.1) guard (2.20.1) formatador (>= 0.2.4) @@ -279,19 +278,21 @@ GEM crass (~> 1.0.2) nokogiri (>= 1.12.0) lumberjack (1.4.2) - mail (2.9.0) + mail (2.9.1) logger mini_mime (>= 0.1.1) net-imap net-pop net-smtp - marcel (1.1.0) + marcel (1.2.1) matrix (0.4.3) memoist3 (1.0.0) method_source (1.1.0) mini_mime (1.1.5) mini_portile2 (2.8.9) - minitest (5.27.0) + minitest (6.0.6) + drb (~> 2.0) + prism (~> 1.5) msgpack (1.8.3) multi_test (1.1.0) multi_xml (0.8.1) @@ -363,9 +364,6 @@ GEM coderay (~> 1.1) method_source (~> 1.0) reline (>= 0.6.0) - psych (5.4.0) - date - stringio public_suffix (7.0.5) puma (8.0.2) nio4r (~> 2.0) @@ -385,20 +383,20 @@ GEM rack (>= 1.0.0) rackup (2.3.1) rack (>= 3) - rails (7.2.3.1) - actioncable (= 7.2.3.1) - actionmailbox (= 7.2.3.1) - actionmailer (= 7.2.3.1) - actionpack (= 7.2.3.1) - actiontext (= 7.2.3.1) - actionview (= 7.2.3.1) - activejob (= 7.2.3.1) - activemodel (= 7.2.3.1) - activerecord (= 7.2.3.1) - activestorage (= 7.2.3.1) - activesupport (= 7.2.3.1) + rails (8.1.3) + actioncable (= 8.1.3) + actionmailbox (= 8.1.3) + actionmailer (= 8.1.3) + actionpack (= 8.1.3) + actiontext (= 8.1.3) + actionview (= 8.1.3) + activejob (= 8.1.3) + activemodel (= 8.1.3) + activerecord (= 8.1.3) + activestorage (= 8.1.3) + activesupport (= 8.1.3) bundler (>= 1.15.0) - railties (= 7.2.3.1) + railties (= 8.1.3) rails-controller-testing (1.0.5) actionpack (>= 5.0.1.rc1) actionview (>= 5.0.1.rc1) @@ -410,10 +408,9 @@ GEM rails-html-sanitizer (1.7.0) loofah (~> 2.25) 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) - railties (7.2.3.1) - actionpack (= 7.2.3.1) - activesupport (= 7.2.3.1) - cgi + railties (8.1.3) + actionpack (= 8.1.3) + activesupport (= 8.1.3) irb (~> 1.13) rackup (>= 1.0.0) rake (>= 12.2) @@ -425,9 +422,14 @@ GEM rb-fsevent (0.11.2) rb-inotify (0.11.1) ffi (~> 1.0) - rdoc (7.2.0) + rbs (4.0.3) + logger + prism (>= 1.6.0) + tsort + rdoc (8.0.0) erb - psych (>= 4.0.0) + prism (>= 1.6.0) + rbs (>= 4.0.0) tsort regexp_parser (2.12.0) reline (0.6.3) @@ -537,7 +539,6 @@ GEM sprockets (>= 3.0.0) stimulus-rails (1.3.4) railties (>= 6.0.0) - stringio (3.2.0) strong_migrations (2.8.0) activerecord (>= 7.2) sys-uname (1.5.1) @@ -575,7 +576,7 @@ GEM crack (>= 0.3.2) hashdiff (>= 0.4.0, < 2.0.0) websocket (1.2.11) - websocket-driver (0.8.0) + websocket-driver (0.8.2) base64 websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) @@ -628,7 +629,7 @@ DEPENDENCIES pg puma (>= 6.0) rack_session_access - rails (~> 7.2.3.1) + rails (~> 8.1.0) rails-controller-testing (~> 1.0) rspec-rails rspec-retry diff --git a/app/controllers/course_settings_controller.rb b/app/controllers/course_settings_controller.rb index d0238aee..25b3189b 100644 --- a/app/controllers/course_settings_controller.rb +++ b/app/controllers/course_settings_controller.rb @@ -59,22 +59,24 @@ def reset_email_templates end def course_settings_params - params.require(:course_settings).permit( - :enable_extensions, - :auto_approve_days, - :auto_approve_extended_request_days, - :max_auto_approve, - :enable_min_hours_before_deadline, - :min_hours_before_deadline, - :enable_gradescope, - :gradescope_course_url, - :extend_late_due_date, - :enable_emails, - :reply_email, - :email_subject, - :email_template, - :enable_slack_webhook_url, - :slack_webhook_url + params.expect( + course_settings: [ + :enable_extensions, + :auto_approve_days, + :auto_approve_extended_request_days, + :max_auto_approve, + :enable_min_hours_before_deadline, + :min_hours_before_deadline, + :enable_gradescope, + :gradescope_course_url, + :extend_late_due_date, + :enable_emails, + :reply_email, + :email_subject, + :email_template, + :enable_slack_webhook_url, + :slack_webhook_url + ] ) end diff --git a/app/controllers/form_settings_controller.rb b/app/controllers/form_settings_controller.rb index 8b14e53f..c0c3389c 100644 --- a/app/controllers/form_settings_controller.rb +++ b/app/controllers/form_settings_controller.rb @@ -37,10 +37,12 @@ def update private def form_setting_params - params.require(:form_setting).permit( - :reason_desc, :documentation_desc, :documentation_disp, - :custom_q1, :custom_q1_desc, :custom_q1_disp, - :custom_q2, :custom_q2_desc, :custom_q2_disp + params.expect( + form_setting: [ + :reason_desc, :documentation_desc, :documentation_disp, + :custom_q1, :custom_q1_desc, :custom_q1_disp, + :custom_q2, :custom_q2_desc, :custom_q2_disp + ] ) end diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index 03030535..a6258839 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -84,7 +84,7 @@ def create_for_student def update Request.merge_date_and_time!(params[:request]) - @request.assign_attributes(update_request_params) + @request.assign_attributes(request_params) return unless ensure_assignment_in_course if @request.save @@ -187,14 +187,12 @@ def set_form_settings @form_settings = @course.form_setting end + # The assignment is chosen at creation and is not editable afterwards, so the + # update action is not permitted to write assignment_id; other actions may. def request_params - params.require(:request).permit(:assignment_id, :reason, :documentation, :custom_q1, :custom_q2, :requested_due_date) - end - - # The assignment is chosen at creation and is not editable afterwards, so it - # is dropped from the params an update is allowed to write. - def update_request_params - request_params.except(:assignment_id) + permitted = [ :reason, :documentation, :custom_q1, :custom_q2, :requested_due_date ] + permitted.unshift(:assignment_id) unless action_name == 'update' + params.expect(request: permitted) end # Every request must reference an assignment in this course. A new request diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb new file mode 100644 index 00000000..7c8a5a73 --- /dev/null +++ b/app/mailers/application_mailer.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +# Base class for all application mailers. +class ApplicationMailer < ActionMailer::Base +end diff --git a/app/mailers/templated_mailer.rb b/app/mailers/templated_mailer.rb new file mode 100644 index 00000000..763ae5f5 --- /dev/null +++ b/app/mailers/templated_mailer.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +# Delivers a pre-rendered HTML email. The subject and body are already +# interpolated by EmailService, so the action just wraps them in a message +# rather than rendering a view template. +class TemplatedMailer < ApplicationMailer + def templated_email(to:, from:, reply_to:, subject:, body:) + mail( + to: to, + from: from, + reply_to: reply_to, + subject: subject, + body: body, + content_type: 'text/html' + ) + end +end diff --git a/app/services/email_service.rb b/app/services/email_service.rb index f6ebcced..96eefd89 100644 --- a/app/services/email_service.rb +++ b/app/services/email_service.rb @@ -27,13 +27,12 @@ def render_templates(subject_template, body_template, mapping) def send_email(to:, from:, reply_to:, subject_template:, body_template:, mapping:, deliver_later: false) rendered = render_templates(subject_template, body_template, mapping) - mail = ActionMailer::Base.mail( + mail = TemplatedMailer.templated_email( to: to, from: from, reply_to: reply_to, subject: rendered[:subject], - body: rendered[:body].gsub("\n", "
\n"), - content_type: 'text/html' + body: rendered[:body].gsub("\n", "
\n") ) deliver_later ? mail.deliver_later : mail.deliver_now diff --git a/config/application.rb b/config/application.rb index e298613f..09dc8b9d 100644 --- a/config/application.rb +++ b/config/application.rb @@ -21,7 +21,7 @@ module Flextensions class Application < Rails::Application # Initialize configuration defaults for originally generated Rails version. - config.load_defaults 7.2 + config.load_defaults 8.1 # Please, add to the `ignore` list any other `lib` subdirectories that do # not contain `.rb` files, or that should not be reloaded or eager loaded. diff --git a/public/400.html b/public/400.html new file mode 100644 index 00000000..282dbc8c --- /dev/null +++ b/public/400.html @@ -0,0 +1,114 @@ + + + + + + + The server cannot process the request due to a client error (400 Bad Request) + + + + + + + + + + + + + +
+
+ +
+
+

The server cannot process the request due to a client error. Please check the request and try again. If you’re the application owner check the logs for more information.

+
+
+ + + + diff --git a/spec/controllers/api/v1/assignments_controller_spec.rb b/spec/controllers/api/v1/assignments_controller_spec.rb index e3071f7a..5c8446e7 100644 --- a/spec/controllers/api/v1/assignments_controller_spec.rb +++ b/spec/controllers/api/v1/assignments_controller_spec.rb @@ -42,7 +42,7 @@ def json_response it 'returns status :bad_request when name is missing' do post :create, params: valid_params.except(:name) expect(response).to have_http_status(:bad_request) - expect(json_response['error']).to include('param is missing or the value is empty: name') + expect(json_response['error']).to include('param is missing or the value is empty or invalid: name') end end diff --git a/spec/controllers/api/v1/lmss_controller_spec.rb b/spec/controllers/api/v1/lmss_controller_spec.rb index 04d885e8..969daca7 100644 --- a/spec/controllers/api/v1/lmss_controller_spec.rb +++ b/spec/controllers/api/v1/lmss_controller_spec.rb @@ -29,7 +29,7 @@ def json_response it 'returns status :bad_request' do post :create, params: { course_id: @course.id, external_course_id: @external_course_id } expect(response).to have_http_status(:bad_request) - expect(response.body).to include('param is missing or the value is empty: lms_id') + expect(response.body).to include('param is missing or the value is empty or invalid: lms_id') end end diff --git a/spec/controllers/enrollments_controller_spec.rb b/spec/controllers/enrollments_controller_spec.rb index a95d0c43..f07342f0 100644 --- a/spec/controllers/enrollments_controller_spec.rb +++ b/spec/controllers/enrollments_controller_spec.rb @@ -58,7 +58,7 @@ allow_extended_requests: true } - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:unprocessable_content) expect(response.parsed_body['redirect_to']).to be_present end end diff --git a/spec/controllers/requests_controller_spec.rb b/spec/controllers/requests_controller_spec.rb index 8a128842..9bcd5837 100644 --- a/spec/controllers/requests_controller_spec.rb +++ b/spec/controllers/requests_controller_spec.rb @@ -311,6 +311,30 @@ expect(response).to redirect_to(course_request_path(course, request)) expect(flash[:notice]).to include('updated') end + + it 'does not reassign the request to a different assignment' do + other_assignment = Assignment.create!( + name: 'A2', + course_to_lms_id: course_to_lms.id, + due_date: 5.days.from_now, + external_assignment_id: 'x2', + enabled: true + ) + + patch :update, params: { + course_id: course.id, + id: request.id, + request: { + assignment_id: other_assignment.id, + reason: 'Updated reason', + requested_due_date: Date.tomorrow.to_s, + due_time: '12:00' + } + } + + expect(response).to redirect_to(course_request_path(course, request)) + expect(request.reload.assignment_id).to eq(assignment.id) + end end describe 'POST #cancel' do @@ -411,7 +435,7 @@ post :approve, params: { course_id: course.id, id: request.id }, format: :json payload = response.parsed_body - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:unprocessable_content) expect(payload['success']).to be(false) expect(payload['message']).to match(/failed/i) end @@ -454,7 +478,7 @@ post :reject, params: { course_id: course.id, id: request.id }, format: :json payload = response.parsed_body - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:unprocessable_content) expect(payload['success']).to be(false) expect(payload['message']).to match(/failed/i) end @@ -520,7 +544,7 @@ post :mass_approve, params: { course_id: course.id, request_ids: [] }, format: :json payload = response.parsed_body - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:unprocessable_content) expect(payload['success']).to be(false) expect(payload['message']).to match(/select at least one/i) end @@ -578,7 +602,7 @@ }, format: :json payload = response.parsed_body - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:unprocessable_content) expect(payload['success']).to be(false) expect(payload['message']).to match(/no pending requests/i) end