diff --git a/Changelog.md b/Changelog.md index 977bacd9b0..cd1d480bb0 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,17 @@ # Changelog +## [unreleased] +- Remove unmaintained locales (#5727) +- Added the ability to submit URLs (#5822) + +## [v2.0.6] +- Fix bug for menu icon not working on mobile devices / smaller screens (#5818) +- Fix bug for "Delete Group" button generating an invalid path (#5768) +- When role switched, 403 errors are displayed as flash messages after redirecting back to the previous page (#5785) +- Update wiki urls to point to https://github.com/MarkUsProject/Wiki (#5781) +- Fix bugs when submitting and cancelling remark requests (#5838) +- Do not trigger starter file changed timestamp when only starter_files_after_due assignment setting is changed (#5845) + ## [v2.0.5] - Add ability to annotate notebook (jupyter and Rmd) submissions (#5749) diff --git a/INSTALL b/INSTALL index 740c81b8a3..3c85f61dd9 100644 --- a/INSTALL +++ b/INSTALL @@ -7,9 +7,4 @@ Thanks for using MarkUs! -Installation instructions have moved to the MarkUs wiki. - -For production installation instructions: https://github.com/MarkUsProject/Markus/wiki/Installation -For development installation instruction: - - with Docker: https://github.com/MarkUsProject/Markus/wiki/Developer-Guide--Set-Up-With-Docker - - with Vagrant: https://github.com/MarkUsProject/Markus/wiki/Developer-Guide--Set-Up-With-Vagrant +Installation instructions have moved to the MarkUs wiki: https://github.com/MarkUsProject/Wiki diff --git a/README.md b/README.md index dc56413dc9..4f11447bbd 100644 --- a/README.md +++ b/README.md @@ -12,14 +12,13 @@ MarkUs is a Ruby on Rails and React web application for the submission and gradi - Graders can easily annotate students' code - Instructors can form groups or students can form groups on their own - Web-based course administration -- [RESTful API](https://github.com/MarkUsProject/Markus/wiki/RESTful-API) -- See the [Wiki pages](https://github.com/MarkUsProject/Markus/wiki) for more features +- See the [Wiki pages](https://github.com/MarkUsProject/Wiki) for more features ## 2. Installation -To install MarkUs for production, see the [Installation Guide](https://github.com/MarkUsProject/Markus/wiki/Installation) for details and step by step instructions. +To install MarkUs for production, see the [Installation Guide](https://github.com/MarkUsProject/Wiki/blob/release/Installation.md) for details and step by step instructions. -To install MarkUs for development, see either of the [docker installation](https://github.com/MarkUsProject/Markus/wiki/Developer-Guide--Set-Up-With-Docker) or [vagrant installation](https://github.com/MarkUsProject/Markus/wiki/Developer-Guide--Set-Up-With-Vagrant) guides. +To install MarkUs for development, see either of the [docker installation](https://github.com/MarkUsProject/Wiki/blob/master/Developer-Guide--Set-Up-With-Docker.md) or [vagrant installation](https://github.com/MarkUsProject/Wiki/blob/master/Developer-Guide--Set-Up-With-Vagrant) guides. ## 3. Who is Using MarkUs? diff --git a/app/MARKUS_VERSION b/app/MARKUS_VERSION index 155e58e4e4..8975a2132e 100644 --- a/app/MARKUS_VERSION +++ b/app/MARKUS_VERSION @@ -1 +1 @@ -VERSION=2.0.5,PATCH_LEVEL=DEV +VERSION=2.0.6,PATCH_LEVEL=DEV diff --git a/app/assets/javascripts/menu.js b/app/assets/javascripts/menu.js index 5ea8a0f5f5..1e195cfd40 100755 --- a/app/assets/javascripts/menu.js +++ b/app/assets/javascripts/menu.js @@ -32,7 +32,8 @@ function initMenu() { } }; } - -window.onload = () => { +// using addEventListener as opposed to direct assignment so that event listeners added elsewhere +// don't get overridden +window.addEventListener("DOMContentLoaded", () => { initMenu(); -}; +}); diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 01248c8cfa..d360b201f1 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -68,7 +68,7 @@ def set_markus_version k,v = token.split('=') version_info[k.downcase] = v end - @markus_version = "#{version_info['version']}.#{version_info['patch_level']}" + @markus_version = version_info['version'] end # Set locale according to URL parameter. If unknown parameter is @@ -127,7 +127,10 @@ def user_not_authorized # Render 403 if the current user is switching roles and they try to view a route for a different course def check_course_switch - user_not_authorized if session[:role_switch_course_id] && current_course&.id != session[:role_switch_course_id] + if session[:role_switch_course_id] && current_course&.id != session[:role_switch_course_id] + flash_message(:error, I18n.t('main.role_switch.forbidden_warning')) + redirect_back(fallback_location: course_assignments_path(session[:role_switch_course_id])) + end end def implicit_authorization_target diff --git a/app/controllers/assignments_controller.rb b/app/controllers/assignments_controller.rb index b0278a253d..76f63344ea 100644 --- a/app/controllers/assignments_controller.rb +++ b/app/controllers/assignments_controller.rb @@ -434,17 +434,19 @@ def update_starter_file success = true ApplicationRecord.transaction do assignment.assignment_properties.update!(starter_file_assignment_params) - all_changed = assignment.assignment_properties.saved_changes? - params[:sections].each do |section_params| + all_changed = + assignment.assignment_properties.saved_change_to_starter_file_type? || + assignment.assignment_properties.saved_change_to_default_starter_file_group_id? + params[:sections]&.each do |section_params| Section.find_by(id: section_params[:section_id]) &.update_starter_file_group(assignment.id, section_params[:group_id]) end starter_file_group_params.each do |group_params| starter_file_group = assignment.starter_file_groups.find_by(id: group_params[:id]) starter_file_group.update!(group_params) - all_changed ||= starter_file_group.saved_changes? || assignment.assignment_properties.saved_changes? + all_changed ||= starter_file_group.saved_changes? end - assignment.assignment_properties.update!(starter_file_updated_at: Time.current) + assignment.assignment_properties.update!(starter_file_updated_at: Time.current) if all_changed rescue ActiveRecord::RecordInvalid => e flash_message(:error, e.message) success = false diff --git a/app/controllers/grade_entry_forms_controller.rb b/app/controllers/grade_entry_forms_controller.rb index 93e0d086bc..37ba3fb068 100644 --- a/app/controllers/grade_entry_forms_controller.rb +++ b/app/controllers/grade_entry_forms_controller.rb @@ -148,7 +148,7 @@ def populate_grades_table .pluck_to_hash(*student_pluck_attrs) grades = current_role.grade_entry_students .where(grade_entry_form: grade_entry_form) - .joins(role: :end_user) + .joins(:grades) .pluck(:id, 'grades.grade_entry_item_id', 'grades.grade') .group_by { |x| x[0] } end diff --git a/app/controllers/main_controller.rb b/app/controllers/main_controller.rb index 5f23736919..e7226b3a04 100644 --- a/app/controllers/main_controller.rb +++ b/app/controllers/main_controller.rb @@ -23,7 +23,11 @@ class MainController < ApplicationController def login # redirect to main page if user is already logged in. if logged_in? && !request.post? - redirect_to controller: 'courses', action: 'index' + if allowed_to?(:role_is_switched?) + redirect_to course_assignments_path(session[:role_switch_course_id]) + else + redirect_to controller: 'courses', action: 'index' + end return end unless Settings.remote_auth_login_url || Settings.validate_file diff --git a/app/controllers/results_controller.rb b/app/controllers/results_controller.rb index d6f3f03e2b..4a0a9e2cf9 100644 --- a/app/controllers/results_controller.rb +++ b/app/controllers/results_controller.rb @@ -651,7 +651,7 @@ def update_overall_comment def update_remark_request @submission = Submission.find(params[:submission_id]) - @assignment = submission.grouping.assignment + @assignment = @submission.grouping.assignment if @assignment.past_remark_due_date? head :bad_request else @@ -684,7 +684,8 @@ def cancel_remark_request redirect_to controller: 'results', action: 'view_marks', - id: params[:id] + course_id: current_course.id, + id: submission.get_original_result.id end def delete_grace_period_deduction diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index d5a27134d6..ff58ed17c9 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -339,7 +339,8 @@ def update_files upload_files_helper(new_folders, new_files, unzip: unzip) do |f| if f.is_a?(String) # is a directory - success, msgs = add_folder(f, current_role, repo, path: path, txn: txn) + authorize! to: :manage_subdirectories? # ensure user is authorized for directories in zip files + success, msgs = add_folder(f, current_role, repo, path: path, txn: txn, required_files: required_files) should_commit &&= success messages = messages.concat msgs else diff --git a/app/helpers/repository_helper.rb b/app/helpers/repository_helper.rb index e513685f9b..010fa8869b 100644 --- a/app/helpers/repository_helper.rb +++ b/app/helpers/repository_helper.rb @@ -118,7 +118,7 @@ def remove_files(files, user, repo, path: '/', txn: nil, keep_folder: true) end end - def add_folder(folder_path, user, repo, path: '/', txn: nil) + def add_folder(folder_path, user, repo, path: '/', txn: nil, required_files: nil) messages = [] if txn.nil? @@ -132,6 +132,14 @@ def add_folder(folder_path, user, repo, path: '/', txn: nil) folder_path = current_path.join(folder_path) folder_path = folder_path.to_s + + # check if only required files are allowed for a submission + # allowed folders = paths in required files + if required_files.present? && required_files.none? { |file| file.starts_with?(folder_path) } + messages << [:invalid_folder_name, folder_path] + return false, messages + end + txn.add_path(folder_path) if commit_txn @@ -225,6 +233,8 @@ def flash_repository_messages(messages, course, suppress: nil) flash_message(:warning, I18n.t('student.submission.no_action_detected')) when :txn_conflicts flash_message(:error, partial: 'submissions/file_conflicts_list', locals: { conflicts: other_info }) + when :invalid_folder_name + flash_message(:error, I18n.t('student.submission.invalid_folder_name')) end end end diff --git a/app/models/starter_file_group.rb b/app/models/starter_file_group.rb index bc3233ffb8..c70be9418c 100644 --- a/app/models/starter_file_group.rb +++ b/app/models/starter_file_group.rb @@ -106,7 +106,7 @@ def update_default end def update_timestamp - assignment.assignment_properties.update(starter_file_updated_at: Time.current) + assignment.assignment_properties.update(starter_file_updated_at: Time.current) if saved_changes? end def set_name diff --git a/app/models/user.rb b/app/models/user.rb index 35c05572c8..458b84a8b2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -87,6 +87,10 @@ def admin_user? self.class == AdminUser end + def end_user? + self.instance_of?(EndUser) + end + def set_display_name strip_name self.display_name ||= "#{self.first_name} #{self.last_name}" diff --git a/app/policies/submission_policy.rb b/app/policies/submission_policy.rb index 76ab6487ef..b5a65ba45d 100644 --- a/app/policies/submission_policy.rb +++ b/app/policies/submission_policy.rb @@ -19,7 +19,7 @@ def manage_files? end def manage_subdirectories? - role.instructor? || role.ta? + user.end_user? end def view_files? diff --git a/app/views/annotation_categories/_annotation_upload_modal.html.erb b/app/views/annotation_categories/_annotation_upload_modal.html.erb index 9b31af1fd3..d08df5b750 100644 --- a/app/views/annotation_categories/_annotation_upload_modal.html.erb +++ b/app/views/annotation_categories/_annotation_upload_modal.html.erb @@ -4,7 +4,7 @@ <%= content_for :modal_content do %> - <%= t('upload_help_html', section_id: AnnotationCategory.name.pluralize.underscore.dasherize) %> + <%= t('upload_help_html', markus_version: @markus_version, section_id: AnnotationCategory.name.pluralize.underscore.dasherize) %> <%= form_tag({ action: 'upload' }, { multipart: true }) do %> diff --git a/app/views/assignments/_assignment_upload_modal.html.erb b/app/views/assignments/_assignment_upload_modal.html.erb index 1ef59c4b14..ad7925db42 100644 --- a/app/views/assignments/_assignment_upload_modal.html.erb +++ b/app/views/assignments/_assignment_upload_modal.html.erb @@ -10,7 +10,7 @@ upload_id: 'upload_file', button_id: 'submit_upload', nonce: true %> - <%= t('upload_help_html', section_id: Assignment.name.pluralize.underscore.dasherize) %> + <%= t('upload_help_html', markus_version: @markus_version, section_id: Assignment.name.pluralize.underscore.dasherize) %> <%= form_tag upload_assignments_course_path(@current_course), { multipart: true } do %> diff --git a/app/views/assignments/_cp_assignment_upload_modal.html.erb b/app/views/assignments/_cp_assignment_upload_modal.html.erb index 9fe94bcee6..a915ac7ca9 100644 --- a/app/views/assignments/_cp_assignment_upload_modal.html.erb +++ b/app/views/assignments/_cp_assignment_upload_modal.html.erb @@ -4,7 +4,7 @@ <%= content_for :modal_content do %> - <%= t('assignments.upload_config_help_html', section_id: 'assignment-configuration') %> + <%= t('assignments.upload_config_help_html', markus_version: @markus_version, section_id: 'assignment-configuration') %> <%= javascript_include_tag 'upload_button_control.js', diff --git a/app/views/assignments/show.html.erb b/app/views/assignments/show.html.erb index 909ca4d6eb..4ea0d42104 100644 --- a/app/views/assignments/show.html.erb +++ b/app/views/assignments/show.html.erb @@ -205,7 +205,7 @@ @grouping.accepted_students.size == 1 && @grouping.extension.nil? %> <%= button_to t('helpers.submit.delete', model: Group.model_name.human), - course_assignment_group_path(@current_course.id, @assignment.id), + course_assignment_group_path(@current_course.id, @assignment.id, @grouping.id), method: :delete, data: { confirm: t('groups.student_interface.confirm_delete_group') } %> diff --git a/app/views/criteria/_upload_modal.html.erb b/app/views/criteria/_upload_modal.html.erb index 9bcfb8632a..9378267be0 100644 --- a/app/views/criteria/_upload_modal.html.erb +++ b/app/views/criteria/_upload_modal.html.erb @@ -15,7 +15,7 @@

<%= t('assignments.due_date.marking_overwrite_warning_html') %>

<% end %> - <%= t('upload_help_html', section_id: Criterion.name.pluralize.underscore.dasherize) %> + <%= t('upload_help_html', markus_version: @markus_version, section_id: Criterion.name.pluralize.underscore.dasherize) %> <%= form_tag upload_course_assignment_criteria_path(@current_course, @assignment), multipart: true, size: 1 do %>

<%= file_field_tag :upload_file, required: true %>

diff --git a/app/views/grade_entry_forms/_upload_modal.html.erb b/app/views/grade_entry_forms/_upload_modal.html.erb index a09aaa4d5a..5eabf25cb9 100644 --- a/app/views/grade_entry_forms/_upload_modal.html.erb +++ b/app/views/grade_entry_forms/_upload_modal.html.erb @@ -3,7 +3,7 @@ <%= content_for :modal_open_link, '#uploadModal' %> <%= content_for :modal_content do %> - <%= t('upload_help_html', section_id: 'marks-spreadsheet-grades') %> + <%= t('upload_help_html', markus_version: @markus_version, section_id: 'marks-spreadsheet-grades') %> <%= form_tag upload_course_grade_entry_form_path(@current_course, @grade_entry_form), { multipart: true, size: 1 } do %> diff --git a/app/views/graders/_upload_modal.html.erb b/app/views/graders/_upload_modal.html.erb index 24d4b3d033..153a9c175c 100644 --- a/app/views/graders/_upload_modal.html.erb +++ b/app/views/graders/_upload_modal.html.erb @@ -10,7 +10,7 @@ upload_id: 'upload_file', button_id: 'upload-groupings', nonce: true %> - <%= t('upload_help_html', section_id: 'graders') %> + <%= t('upload_help_html', markus_version: @markus_version, section_id: 'graders') %> <%= form_tag upload_course_assignment_graders_path(@current_course, @assignment), { multipart: true } do %> diff --git a/app/views/groups/_upload_modal.html.erb b/app/views/groups/_upload_modal.html.erb index 448e539e4d..be06b4acf3 100644 --- a/app/views/groups/_upload_modal.html.erb +++ b/app/views/groups/_upload_modal.html.erb @@ -11,7 +11,7 @@ button_id: 'upload', nonce: true %>

- <%= t('upload_help_html', section_id: 'groups') %> + <%= t('upload_help_html', markus_version: @markus_version, section_id: 'groups') %>

<%= form_tag upload_course_assignment_groups_path(@current_course, @assignment), { multipart: true } do %> diff --git a/app/views/marks_graders/_upload_modal.html.erb b/app/views/marks_graders/_upload_modal.html.erb index 9682d36ab2..760d1dd5f5 100644 --- a/app/views/marks_graders/_upload_modal.html.erb +++ b/app/views/marks_graders/_upload_modal.html.erb @@ -10,7 +10,7 @@ button_id: 'upload', nonce: true %> - <%= t('upload_help_html', section_id: 'graders') %> + <%= t('upload_help_html', markus_version: @markus_version, section_id: 'graders') %> <%= form_tag({ controller: 'marks_graders', action: 'upload' }, diff --git a/app/views/peer_reviews/_upload_modal.html.erb b/app/views/peer_reviews/_upload_modal.html.erb index 2c5e28e2b0..df8b10727b 100644 --- a/app/views/peer_reviews/_upload_modal.html.erb +++ b/app/views/peer_reviews/_upload_modal.html.erb @@ -10,7 +10,7 @@ upload_id: 'upload_file', button_id: 'upload', nonce: true %> - <%= t('upload_help_html', section_id: 'peer-reviews') %> + <%= t('upload_help_html', markus_version: @markus_version, section_id: 'peer-reviews') %> <%= form_tag({ controller: 'peer_reviews', action: 'upload' }, diff --git a/app/views/shared/_flash_message.html.erb b/app/views/shared/_flash_message.html.erb index db84c70fc4..bf4902d255 100644 --- a/app/views/shared/_flash_message.html.erb +++ b/app/views/shared/_flash_message.html.erb @@ -18,11 +18,13 @@ <% end %> <% end %> <%= javascript_tag nonce: true do %> - window.onload = function () { + // using addEventListener as opposed to direct assignment so that event listeners added elsewhere + // don't get overridden + window.addEventListener("DOMContentLoaded", function () { Array.from(document.getElementsByClassName('hide-flash')).forEach(function (elem) { elem.addEventListener("click", function(e) { e.target.parentElement.style.display = 'none'; }) }) - } + }) <% end %> diff --git a/app/views/students/_upload_modal.html.erb b/app/views/students/_upload_modal.html.erb index 76f34e0bbf..270a48fa43 100644 --- a/app/views/students/_upload_modal.html.erb +++ b/app/views/students/_upload_modal.html.erb @@ -12,7 +12,7 @@ nonce: true %>

- <%= t('upload_help_html', section_id: 'students') %> + <%= t('upload_help_html', markus_version: @markus_version, section_id: 'students') %>

<%= form_tag upload_course_students_path(@current_course), { multipart: true } do %>
diff --git a/app/views/tags/_upload_modal.html.erb b/app/views/tags/_upload_modal.html.erb index 72b59a2925..b87cea2250 100644 --- a/app/views/tags/_upload_modal.html.erb +++ b/app/views/tags/_upload_modal.html.erb @@ -11,7 +11,7 @@ button_id: 'submit_upload', nonce: true %> - <%= t('upload_help_html', section_id: 'tags') %> + <%= t('upload_help_html', markus_version: @markus_version, section_id: 'tags') %> <%= form_tag upload_course_tags_path(@current_course, assignment_id: @assignment.id), { multipart: true, size: 1 } do %> diff --git a/app/views/tas/_upload_modal.html.erb b/app/views/tas/_upload_modal.html.erb index 03957080dd..5b7507b033 100644 --- a/app/views/tas/_upload_modal.html.erb +++ b/app/views/tas/_upload_modal.html.erb @@ -11,7 +11,7 @@ nonce: true %>

- <%= t('upload_help_html', section_id: 'students') %> + <%= t('upload_help_html', markus_version: @markus_version, section_id: 'students') %>

<%= form_tag upload_course_tas_path(@current_course), { multipart: true } do %>
diff --git a/app/views/users/_key_display.html.erb b/app/views/users/_key_display.html.erb index 4983cc0094..966f9e9d29 100644 --- a/app/views/users/_key_display.html.erb +++ b/app/views/users/_key_display.html.erb @@ -3,7 +3,7 @@ <%= User.human_attribute_name(:api_key) %>

- <%= t('users.api_key.help_html') %> + <%= t('users.api_key.help_html', markus_version: @markus_version,) %>

diff --git a/app/views/users/settings.html.erb b/app/views/users/settings.html.erb index 090ee68bcf..78ff0a3559 100644 --- a/app/views/users/settings.html.erb +++ b/app/views/users/settings.html.erb @@ -57,12 +57,12 @@

<%= t('help') %>

- + <%= t('key_pairs.help.windows') %>

- + <%= t('key_pairs.help.macOS_linux') %>

diff --git a/config/i18n-tasks.yml b/config/i18n-tasks.yml index 4d9b0319b4..9cf0d689fa 100644 --- a/config/i18n-tasks.yml +++ b/config/i18n-tasks.yml @@ -3,7 +3,7 @@ # The "main" locale. base_locale: en ## All available locales are inferred from the data by default. Alternatively, specify them explicitly: -# locales: [en] +locales: [en] ## Reporting locale, default: en. Available: en, ru. # internal_locale: en diff --git a/config/locales/defaults/download_upload/en.yml b/config/locales/defaults/download_upload/en.yml index a9d682049e..07ab3e8eb6 100644 --- a/config/locales/defaults/download_upload/en.yml +++ b/config/locales/defaults/download_upload/en.yml @@ -18,7 +18,7 @@ en: malformed_csv: The selected file was improperly formed. Please ensure the file meets MarkUs specifications. syntax_error: 'There is an error in the file you uploaded: %{error}' unparseable_csv: The selected file is not a CSV file (even though it may have a .csv extension). - upload_help_html: ' For information about the file format(s) supported, please visit this page.' + upload_help_html: ' For information about the file format(s) supported, please visit this page.' upload_success: one: "%{count} object successfully uploaded." other: "%{count} objects successfully uploaded." diff --git a/config/locales/en.yml b/config/locales/en.yml index 73fbd734b7..a1f4e08b7f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -38,6 +38,7 @@ en: file_conflicts: 'Your changes have not been made. The following file conflicts were detected:' file_too_large: The size of the uploaded file %{file_name} exceeds the maximum of %{max_size} MB. invalid_file_name: Invalid file name on submitted file + invalid_folder_name: Invalid folder name on submitted folder missing_file: Could not download %{file_name}. File may be missing. missing_files: 'You still need to submit %{file} required file(s) for this assignment:' no_action_detected: No actions were detected in the last submit. Nothing was changed. diff --git a/config/locales/views/assignments/en.yml b/config/locales/views/assignments/en.yml index 7a688322af..c2aee46498 100644 --- a/config/locales/views/assignments/en.yml +++ b/config/locales/views/assignments/en.yml @@ -82,5 +82,5 @@ en: started_on: 'You started on:' starter_file_prompt: Click below to download the starter files for this assessment. time_until_due_warning: This assessment is due before %{due_date} without penalty. - upload_config_help_html: "

This zip file will configure and create this assignment.

This does not configure student or grader settings.
For more information please visit this page.

" + upload_config_help_html: "

This zip file will configure and create this assignment.

This does not configure student or grader settings.
For more information please visit this page.

" upload_file_requirement: The list of uploaded files does not match the required files for this assignment. diff --git a/config/locales/views/main/en.yml b/config/locales/views/main/en.yml index 84999af44b..0335ecf608 100644 --- a/config/locales/views/main/en.yml +++ b/config/locales/views/main/en.yml @@ -17,6 +17,7 @@ en: password_not_blank: Your password must not be blank. role_switch: clear_role_switch_session: Cancel role switch + forbidden_warning: You do not have permission to view that page while switching roles. log_in_as: Log in as switch_role: Switch Role session_expired: Your session has expired. Please log in. diff --git a/config/locales/views/users/en.yml b/config/locales/views/users/en.yml index 984a411bfa..42363bb6f1 100644 --- a/config/locales/views/users/en.yml +++ b/config/locales/views/users/en.yml @@ -32,7 +32,7 @@ en: users: account_settings: Account Settings api_key: - help_html: The API key is used for authentication for the MarkUs API. See MarkUs Wiki for information on using the API. + help_html: The API key is used for authentication for the MarkUs API. See MarkUs Wiki for information on using the API. reset: Reset API key unavailable: Unavailable not_found: 'No user can be found with the user name: %{user_names}' diff --git a/doc/README_FOR_APP b/doc/README_FOR_APP index 4a1d1e5ea9..b4109487e8 100644 --- a/doc/README_FOR_APP +++ b/doc/README_FOR_APP @@ -7,7 +7,7 @@ Tickets and/or bug reports: https://github.com/MarkUsProject/Markus/issues Wiki: - https://github.com/MarkUsProject/Markus/wiki + https://github.com/MarkUsProject/Wiki Contact us: markus-users@cs.toronto.edu diff --git a/spec/controllers/assignments_controller_spec.rb b/spec/controllers/assignments_controller_spec.rb index 6f1eb06215..5213db1344 100644 --- a/spec/controllers/assignments_controller_spec.rb +++ b/spec/controllers/assignments_controller_spec.rb @@ -898,7 +898,8 @@ end describe '#update_starter_file' do subject { post_as role, :update_starter_file, params: params } - let(:assignment) { create :assignment } + let!(:assignment) { create :assignment } + let!(:grouping) { create :grouping, assignment: assignment } let(:starter_file_group1) do create :starter_file_group, assignment: assignment, name: 'name', entry_rename: 'name', use_rename: false end @@ -926,6 +927,19 @@ change { assignment.reload.default_starter_file_group_id }.from(nil).to(starter_file_group2.id) ) end + it 'should update starter_file_updated_at' do + expect { subject }.to( + change { assignment.assignment_properties.reload.starter_file_updated_at } + ) + end + context 'when a grouping for the assignment exists' do + it 'should update the grouping starter_file_changed attribute' do + grouping + expect { subject }.to( + change { grouping.reload.starter_file_changed }.from(false).to(true) + ) + end + end context 'when a section exists' do it 'should update section starter file mappings' do expect { subject }.to( @@ -964,17 +978,47 @@ end end end + context 'when only updating assignment starter_files_after_due attribute' do + let!(:params) do + { id: assignment.id, + course_id: role.course.id, + assignment: { starter_files_after_due: false }, + sections: [], + starter_file_groups: [{ id: starter_file_group1.id, + name: starter_file_group1.name, + entry_rename: starter_file_group1.entry_rename, + use_rename: starter_file_group1.use_rename }] } + end + it 'should update starter_files_after_due' do + expect { subject }.to( + change { assignment.assignment_properties.reload.starter_files_after_due }.from(true).to(false) + ) + end + it 'should not update starter_file_updated_at' do + expect { subject }.to_not( + change { assignment.assignment_properties.reload.starter_file_updated_at } + ) + end + context 'when a grouping for the assignment exists' do + it 'should not update the grouping starter_file_changed attribute' do + grouping + expect { subject }.to_not( + change { grouping.reload.starter_file_changed } + ) + end + end + end end context 'a grader' do let(:role) { create :ta } - it 'should return a 404 error' do + it 'should return a 403 error' do subject expect(response).to have_http_status(403) end end context 'a student' do let(:role) { create :student } - it 'should return a 404 error' do + it 'should return a 403 error' do subject expect(response).to have_http_status(403) end diff --git a/spec/controllers/grade_entry_forms_controller_spec.rb b/spec/controllers/grade_entry_forms_controller_spec.rb index b3d835f463..9b35993753 100644 --- a/spec/controllers/grade_entry_forms_controller_spec.rb +++ b/spec/controllers/grade_entry_forms_controller_spec.rb @@ -567,4 +567,58 @@ include_examples 'switch assignment tests' end end + + describe '#populate_grades_table' do + before(:each) do + get_as role, :populate_grades_table, params: { course_id: course.id, id: grade_entry_form_with_data.id } + end + context 'an instructor' do + let(:data_fields) { %w[_id released_to_student user_name first_name last_name hidden section_id] } + it 'returns a 200 response' do + expect(response.status).to eq 200 + end + it 'returns data' do + expect(response.parsed_body['data'].length).to be > 0 + end + it 'returns all student data' do + expect(response.parsed_body['data'].length).to eq course.students.count + end + it 'returns correct fields' do + data_fields << grade_entry_form_with_data.grade_entry_items.first.id.to_s + expect(response.parsed_body['data'][0].keys).to match_array(data_fields) + end + end + context 'a TA' do + let(:role) { create :ta } + it 'returns a 200 response' do + expect(response.status).to eq 200 + end + context 'who has not been assigned a student' do + it 'returns data' do + expect(response.parsed_body['data'].length).to eq 0 + end + end + context 'who has been assigned a student' do + let(:student) do + grade_entry_form_with_data.grade_entry_students.joins(role: :end_user).find_by('users.user_name': 'c8shosta') + end + let!(:grade_entry_student_ta) { create(:grade_entry_student_ta, ta: role, grade_entry_student: student) } + it 'returns data' do + get_as role, :populate_grades_table, params: { course_id: course.id, id: grade_entry_form_with_data.id } + expect(response.parsed_body['data'].length).to be > 0 + end + it 'returns the number of students assigned to the ta' do + get_as role, :populate_grades_table, params: { course_id: course.id, id: grade_entry_form_with_data.id } + expect(response.parsed_body['data'].length).to eq role.grade_entry_students.count + end + it 'returns the correct set of students' do + get_as role, :populate_grades_table, params: { course_id: course.id, id: grade_entry_form_with_data.id } + students = role.grade_entry_students + .where(grade_entry_form: grade_entry_form_with_data) + .joins(role: :end_user).pluck('users.user_name') + expect(response.parsed_body['data'].map { |stu| stu['user_name'] }).to match_array(students) + end + end + end + end end diff --git a/spec/controllers/main_controller_spec.rb b/spec/controllers/main_controller_spec.rb index 421098c1b0..89c11ea730 100644 --- a/spec/controllers/main_controller_spec.rb +++ b/spec/controllers/main_controller_spec.rb @@ -156,4 +156,30 @@ include_examples 'ta tests' end end + context 'when role switched' do + let(:course1) { create :course } + let(:course2) { create :course } + let(:instructor) { create :instructor, course_id: course1.id } + let(:instructor2) { create :instructor, course_id: course2.id } + let(:student) { create :student, course_id: course1.id } + before :each do + @controller = CoursesController.new + post_as instructor, :switch_role, params: { id: course1.id, effective_user_login: student.user_name } + end + it 'redirects the login route to the course homepage' do + @controller = MainController.new + get :login + expect(response).to redirect_to course_assignments_path(session[:role_switch_course_id]) + end + it 'flashes a forbidden error message on attempt to access another course' do + @controller = CoursesController.new + get :show, params: { id: course2.id } + expect(flash[:error]).not_to be_empty + end + it 'redirects to the original course on attempt to access another course' do + @controller = CoursesController.new + get :show, params: { id: course2.id } + expect(response).to redirect_to course_assignments_path(session[:role_switch_course_id]) + end + end end diff --git a/spec/controllers/results_controller_spec.rb b/spec/controllers/results_controller_spec.rb index 47b5d0c169..b6750b00b9 100644 --- a/spec/controllers/results_controller_spec.rb +++ b/spec/controllers/results_controller_spec.rb @@ -669,6 +669,103 @@ def self.test_unauthorized(route_name) test_assigns_not_nil :files end end + + describe '#update_remark_request' do + let(:assignment) { create :assignment, assignment_properties_attributes: { allow_remarks: true } } + let(:grouping) { create :grouping_with_inviter, assignment: assignment } + let(:student) { grouping.inviter } + let(:submission) do + s = create :submission, grouping: grouping + s.get_original_result.update!(released_to_students: true) + s + end + + context 'when saving a remark request message' do + let(:subject) do + patch_as student, + :update_remark_request, + params: { course_id: assignment.course_id, + submission_id: submission.id, + submission: { remark_request: 'Message' }, + save: true } + end + + before { subject } + + it 'updates the submission remark request message' do + expect(submission.reload.remark_request).to eq 'Message' + end + + it 'does not submit the remark request' do + expect(submission.reload.remark_result).to be_nil + end + end + + context 'when submitting a remark request' do + let(:subject) do + patch_as student, + :update_remark_request, + params: { course_id: assignment.course_id, + submission_id: submission.id, + submission: { remark_request: 'Message' }, + submit: true } + end + + before { subject } + + it 'updates the submission remark request message' do + expect(submission.reload.remark_request).to eq 'Message' + end + + it 'submits the remark request' do + expect(submission.reload.remark_result).to_not be_nil + end + + it 'unreleases the original result' do + expect(submission.get_original_result.reload.released_to_students).to be false + end + end + end + + describe '#cancel_remark_request' do + let(:assignment) { create :assignment, assignment_properties_attributes: { allow_remarks: true } } + let(:grouping) { create :grouping_with_inviter, assignment: assignment } + let(:student) { grouping.inviter } + let(:submission) do + s = create :submission, grouping: grouping, remark_request: 'original message', + remark_request_timestamp: Time.current + s.make_remark_result + s.results.reload + s.remark_result.update!(marking_state: Result::MARKING_STATES[:incomplete]) + s.get_original_result.update!(released_to_students: false) + + s + end + + let(:subject) do + delete_as student, + :cancel_remark_request, + params: { course_id: assignment.course_id, + id: submission.remark_result.id, + submission_id: submission.id } + end + + before { subject } + + it 'destroys the remark result' do + submission.non_pr_results.reload + expect(submission.remark_result).to be_nil + end + + it 'releases the original result' do + expect(submission.get_original_result.reload.released_to_students).to be true + end + + it 'redirects to the original result view' do + expect(response).to redirect_to view_marks_course_result_path(course_id: assignment.course_id, + id: submission.get_original_result.id) + end + end end context 'An instructor' do before(:each) { sign_in instructor } diff --git a/spec/controllers/submissions_controller_spec.rb b/spec/controllers/submissions_controller_spec.rb index c6da6cd31d..0a289f4cec 100644 --- a/spec/controllers/submissions_controller_spec.rb +++ b/spec/controllers/submissions_controller_spec.rb @@ -1,8 +1,5 @@ describe SubmissionsController do # TODO: add 'role is from a different course' shared tests to each route test below - after(:each) do - destroy_repos - end let(:course) { Course.first || create(:course) } shared_examples 'An authorized instructor and grader accessing #set_result_marking_state' do context '#set_result_marking_state' do @@ -184,6 +181,125 @@ expect(files['TestShapes.java']).to be_nil end end + + context 'when creating a folder with required files' do + let(:tree) do + @grouping.group.access_repo do |repo| + repo.get_latest_revision.tree_at_path(@assignment.repository_folder) + end + end + before :each do + @assignment.update!( + only_required_files: true, + assignment_files_attributes: [{ filename: 'test_zip/zip_subdir/TestShapes.java' }] + ) + end + it 'uploads a directory and returns a success' do + post_as @student, :update_files, + params: { course_id: course.id, assignment_id: @assignment.id, + new_folders: ['test_zip'] } + expect(response).to have_http_status :ok + end + it 'commits a single directory' do + post_as @student, :update_files, + params: { course_id: course.id, assignment_id: @assignment.id, + new_folders: ['test_zip'] } + expect(tree['test_zip']).not_to be_nil + end + it 'uploads a subdirectory' do + post_as @student, :update_files, + params: { course_id: course.id, assignment_id: @assignment.id, + new_folders: ['test_zip'] } + post_as @student, :update_files, + params: { course_id: course.id, assignment_id: @assignment.id, + new_folders: ['test_zip/zip_subdir'] } + expect(response).to have_http_status :ok + end + it 'commits a subdirectory' do + post_as @student, :update_files, + params: { course_id: course.id, assignment_id: @assignment.id, + new_folders: ['test_zip'] } + post_as @student, :update_files, + params: { course_id: course.id, assignment_id: @assignment.id, + new_folders: ['test_zip/zip_subdir'] } + expect(tree['test_zip/zip_subdir']).not_to be_nil + end + context 'when testing with a git repo', :keep_memory_repos do + before(:each) { allow(Settings.repository).to receive(:type).and_return('git') } + after(:each) { FileUtils.rm_r(Dir.glob(File.join(Settings.repository.storage, '*'))) } + it 'displays a failure message when attempting to create a subdirectory with no parent' do + post_as @student, :update_files, + params: { course_id: course.id, assignment_id: @assignment.id, + new_folders: ['test_zip/zip_subdir'] } + + expect(flash[:error]).to_not be_empty + end + end + it 'does not upload a non required directory and returns a failure' do + 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 + end + it 'does not commit the non required directory' do + post_as @student, :update_files, + params: { course_id: course.id, assignment_id: @assignment.id, + new_folders: ['bad_folder'] } + expect(tree['bad_folder']).to be_nil + end + it 'does not upload a non required subdirectory' do + 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 + end + it 'does not commit a non required subdirectory' do + post_as @student, :update_files, + params: { course_id: course.id, assignment_id: @assignment.id, + new_folders: ['bad_folder/bad_subdirectory'] } + expect(tree['bad_folder/bad_subdirectory']).to be_nil + end + end + + context 'when folders are required and uploading a zip file' do + let(:unzip) { 'true' } + before :each do + @assignment.update!( + only_required_files: true, + assignment_files_attributes: [{ filename: 'test_zip/zip_subdir/TestShapes.java' }, + { filename: 'test_zip/Shapes.java' }] + ) + end + + it 'should be able to create required folders' do + zip_file = fixture_file_upload('test_zip.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 :ok + end + it 'uploads the outer directory' do + zip_file = fixture_file_upload('test_zip.zip', 'application/zip') + post_as @student, :update_files, + params: { course_id: course.id, assignment_id: @assignment.id, + new_files: [zip_file], unzip: unzip } + tree = @grouping.group.access_repo do |repo| + repo.get_latest_revision.tree_at_path(@assignment.repository_folder) + end + expect(tree['test_zip']).not_to be_nil + end + it 'uploads the inner directory' do + zip_file = fixture_file_upload('test_zip.zip', 'application/zip') + post_as @student, :update_files, + params: { course_id: course.id, assignment_id: @assignment.id, + new_files: [zip_file], unzip: unzip } + tree = @grouping.group.access_repo do |repo| + repo.get_latest_revision.tree_at_path(@assignment.repository_folder) + end + expect(tree['test_zip/zip_subdir']).not_to be_nil + end + end end context 'uploading a zip file' do