diff --git a/app/api/api_root.rb b/app/api/api_root.rb index bde5f02365..09e9f0787d 100644 --- a/app/api/api_root.rb +++ b/app/api/api_root.rb @@ -60,6 +60,7 @@ class ApiRoot < Grape::API mount LearningAlignmentApi mount ProjectsApi mount SettingsApi + mount StaffGrantExtensionApi mount StudentsApi mount Submission::PortfolioApi mount Submission::PortfolioEvidenceApi @@ -100,6 +101,7 @@ class ApiRoot < Grape::API AuthenticationHelpers.add_auth_to LearningOutcomesApi AuthenticationHelpers.add_auth_to LearningAlignmentApi AuthenticationHelpers.add_auth_to ProjectsApi + AuthenticationHelpers.add_auth_to StaffGrantExtensionApi AuthenticationHelpers.add_auth_to StudentsApi AuthenticationHelpers.add_auth_to Submission::PortfolioApi AuthenticationHelpers.add_auth_to Submission::PortfolioEvidenceApi diff --git a/app/api/extension_comments_api.rb b/app/api/extension_comments_api.rb index 3bde9171de..966d69e6ff 100644 --- a/app/api/extension_comments_api.rb +++ b/app/api/extension_comments_api.rb @@ -10,25 +10,21 @@ class ExtensionCommentsApi < Grape::API requires :weeks_requested, type: Integer, desc: 'The details of the request' end post '/projects/:project_id/task_def_id/:task_definition_id/request_extension' do - project = Project.find(params[:project_id]) - task_definition = project.unit.task_definitions.find(params[:task_definition_id]) - task = project.task_for_task_definition(task_definition) - - # check permissions using specific permission has with addition of request extension if allowed in unit - unless authorise? current_user, task, :request_extension, ->(role, perm_hash, other) { task.specific_permission_hash(role, perm_hash, other) } - error!({ error: 'Not authorised to request an extension for this task' }, 403) + # Use the ExtensionService to handle the extension request + result = ExtensionService.grant_extension( + params[:project_id], + params[:task_definition_id], + current_user, + params[:weeks_requested], + params[:comment] + ) + + # Handle the service response + if result[:success] + present result[:result].serialize(current_user), Grape::Presenters::Presenter + else + error!({ error: result[:error] }, result[:status]) end - - error!({ error: 'Extension weeks can not be 0.' }, 403) if params[:weeks_requested] == 0 - - max_duration = task.weeks_can_extend - duration = params[:weeks_requested] - duration = max_duration unless params[:weeks_requested] <= max_duration - - error!({ error: 'Extensions cannot be granted beyond task deadline.' }, 403) if duration <= 0 - - result = task.apply_for_extension(current_user, params[:comment], duration) - present result.serialize(current_user), Grape::Presenters::Presenter end desc 'Assess an extension for a task' diff --git a/app/api/staff_grant_extension_api.rb b/app/api/staff_grant_extension_api.rb new file mode 100644 index 0000000000..9c50aa48bb --- /dev/null +++ b/app/api/staff_grant_extension_api.rb @@ -0,0 +1,126 @@ +require 'grape' + +# +# API endpoint for staff to grant extensions to multiple students at once +# +class StaffGrantExtensionApi < Grape::API + helpers AuthenticationHelpers + helpers AuthorisationHelpers + helpers DbHelpers + + before do + authenticated? + error!({ + error: 'Not authorized to grant extensions', + code: 'UNAUTHORIZED', + details: {} + }, 403) unless current_user.has_tutor_capability? + end + + desc 'Grant extensions to multiple students', + detail: 'This endpoint allows staff to grant extensions to multiple students at once for a specific task. The operation is atomic - either all extensions are granted or none are. Students not found in the unit are automatically skipped without affecting the transaction.', + success: [ + { code: 201, message: 'Extensions granted successfully' } + ], + failure: [ + { code: 400, message: 'Some extensions failed to be granted' }, + { code: 403, message: 'Not authorized to grant extensions for this unit' }, + { code: 404, message: 'Unit or task definition not found' }, + { code: 500, message: 'Internal server error' } + ], + response: { + successful: [ + { + student_id: 'Integer - ID of the student', + project_id: 'Integer - ID of the project', + weeks_requested: 'Integer - Number of weeks extension granted', + extension_response: 'String - Human readable message with new due date', + task_status: 'String - Updated status of the task' + } + ], + failed: [ + { + student_id: 'Integer - ID of the student', + project_id: 'Integer - ID of the project', + error: 'String - Error message explaining why extension failed' + } + ], + skipped: [ + { + student_id: 'Integer - ID of the student', + reason: 'String - Reason why the student was skipped' + } + ] + } + params do + requires :student_ids, type: Array[Integer], desc: 'List of student IDs to grant extensions to' + requires :task_definition_id, type: Integer, desc: 'Task definition ID' + requires :weeks_requested, type: Integer, desc: 'Number of weeks to extend by' + requires :comment, type: String, desc: 'Reason for extension' + end + post '/units/:unit_id/staff-grant-extension' do + unit = Unit.find(params[:unit_id]) + task_definition = unit.task_definitions.find(params[:task_definition_id]) + + # Use transaction to ensure atomic operation + ActiveRecord::Base.transaction do + results = { + successful: [], + failed: [], + skipped: [] + } + + params[:student_ids].each do |student_id| + # Find project for this student in the unit + project = unit.projects.find_by(user_id: student_id) + if project.nil? + results[:skipped] << { + student_id: student_id, + reason: 'Student not found in unit' + } + next + end + + result = ExtensionService.grant_extension( + project.id, + task_definition.id, + current_user, + params[:weeks_requested], + params[:comment], + true # is_staff_grant = true + ) + + if result[:success] + extension_comment = result[:result] + results[:successful] << { + student_id: student_id, + project_id: project.id, + weeks_requested: extension_comment.extension_weeks, + extension_response: extension_comment.extension_response, + task_status: extension_comment.task.status + } + else + results[:failed] << { + student_id: student_id, + project_id: project.id, + error: result[:error] + } + # If it's a validation error (403), raise it immediately + error!({ error: result[:error] }, result[:status]) if result[:status] == 403 + end + end + + # If any extensions failed (but not due to validation), rollback the entire transaction + if results[:failed].any? + error!({ error: 'Some extensions failed to be granted', results: results }, 400) + end + + status 201 + present results, with: Grape::Presenters::Presenter + end + rescue ActiveRecord::RecordNotFound + error!({ error: 'Unit or task definition not found' }, 404) + rescue StandardError + error!({ error: 'An unexpected error occurred' }, 500) + end +end diff --git a/app/models/unit.rb b/app/models/unit.rb index 175e62c790..9fe9631d99 100644 --- a/app/models/unit.rb +++ b/app/models/unit.rb @@ -28,7 +28,8 @@ def self.permissions :download_stats, :download_unit_csv, :download_grades, - :exceed_capacity + :exceed_capacity, + :grant_extensions ] # What can convenors do with units? diff --git a/app/services/extension_service.rb b/app/services/extension_service.rb new file mode 100644 index 0000000000..01af8cc4c4 --- /dev/null +++ b/app/services/extension_service.rb @@ -0,0 +1,52 @@ +class ExtensionService + def self.grant_extension(project_id, task_definition_id, user, weeks_requested, comment, is_staff_grant = false) + # Find project and task + project = Project.find(project_id) + task_definition = project.unit.task_definitions.find(task_definition_id) + task = project.task_for_task_definition(task_definition) + + # ===== Common Validation Logic (used by both endpoints) ===== + # Validate extension weeks + return { success: false, error: 'Extension weeks cannot be 0', status: 403 } if weeks_requested == 0 + + # Calculate max duration + max_duration = task.weeks_can_extend + duration = weeks_requested + duration = max_duration unless weeks_requested <= max_duration + + # Check if extension would exceed deadline + return { success: false, error: 'Extensions cannot be granted beyond task deadline', status: 403 } if duration <= 0 + + # ===== Student-Initiated Extension Logic (current endpoint) ===== + unless is_staff_grant + # Check task-level authorization for student requests with specific permission hash + unless AuthorisationHelpers.authorise?(user, task, :request_extension, ->(role, perm_hash, other) { task.specific_permission_hash(role, perm_hash, other) }) + return { success: false, error: 'Not authorised to request an extension for this task', status: 403 } + end + end + + # ===== Staff Grant Logic (new endpoint) ===== + if is_staff_grant + # Check unit-level authorization for staff grants + unless AuthorisationHelpers.authorise?(user, project.unit, :grant_extensions) + return { success: false, error: 'Not authorised to grant extensions for this unit', status: 403 } + end + end + + # ===== Common Extension Logic ===== + # Apply the extension + result = task.apply_for_extension(user, comment, duration) + + # Auto-approve if it's a staff grant + if is_staff_grant + extension_comment = result.becomes(ExtensionComment) + extension_comment.assess_extension(user, true, true) + end + + { success: true, result: result, status: 201 } + rescue ActiveRecord::RecordNotFound => e + { success: false, error: 'Task or project not found', status: 404 } + rescue StandardError => e + { success: false, error: e.message, status: 500 } + end +end diff --git a/test/api/staff_grant_extension_test.rb b/test/api/staff_grant_extension_test.rb new file mode 100644 index 0000000000..f03b9a62e1 --- /dev/null +++ b/test/api/staff_grant_extension_test.rb @@ -0,0 +1,249 @@ +require 'test_helper' + +class StaffGrantExtensionTest < ActiveSupport::TestCase + include Rack::Test::Methods + include TestHelpers::AuthHelper + include TestHelpers::JsonHelper + + def app + Rails.application + end + + def test_staff_grant_extension_success + unit = FactoryBot.create(:unit) + project = unit.projects.first + staff = FactoryBot.create(:user, role: Role.tutor) + unit.employ_staff(staff, Role.tutor) + + td = TaskDefinition.new({ + unit_id: unit.id, + tutorial_stream: unit.tutorial_streams.first, + name: 'Staff Grant Extension Test', + description: 'Test task for staff grant extension', + weighting: 4, + target_grade: 0, + start_date: Time.zone.now - 1.week, + target_date: Time.zone.now + 1.week, + due_date: Time.zone.now + 2.weeks, + abbreviation: 'STAFFGRANTTEST', + restrict_status_updates: false, + upload_requirements: [], + plagiarism_warn_pct: 0.8, + is_graded: false, + max_quality_pts: 0 + }) + td.save! + + data_to_post = { + student_ids: [project.student.id], + task_definition_id: td.id, + weeks_requested: 1, + comment: 'Staff granted extension' + } + + add_auth_header_for user: staff + post_json "/api/units/#{unit.id}/staff-grant-extension", data_to_post + assert_equal 201, last_response.status + + response = last_response_body + assert response["successful"].length == 1, 'Should have one successful extension' + assert response["failed"].empty?, 'Should have no failed extensions' + assert response["successful"][0]["student_id"] == project.student.id, 'Should match the student ID' + assert response["successful"][0]["weeks_requested"] == 1, 'Should have requested 1 week' + assert response["successful"][0]["extension_response"].present?, 'Should have extension response' + assert response["successful"][0]["task_status"].present?, 'Should have task status' + + td.destroy! + unit.destroy! + end + + def test_staff_grant_extension_unauthorized + unit = FactoryBot.create(:unit) + project = unit.projects.first + student = project.student # Using student instead of staff + td = unit.task_definitions.first + + data_to_post = { + student_ids: [project.student.id], + task_definition_id: td.id, + weeks_requested: 1, + comment: 'Unauthorized attempt' + } + + add_auth_header_for user: student + post_json "/api/units/#{unit.id}/staff-grant-extension", data_to_post + assert_equal 403, last_response.status, 'Should not allow non-staff to grant extensions' + end + + def test_staff_grant_extension_invalid_weeks + unit = FactoryBot.create(:unit) + project = unit.projects.first + staff = FactoryBot.create(:user, role: Role.tutor) + unit.employ_staff(staff, Role.tutor) + td = unit.task_definitions.first + + data_to_post = { + student_ids: [project.student.id], + task_definition_id: td.id, + weeks_requested: 0, + comment: 'Invalid weeks' + } + + add_auth_header_for user: staff + post_json "/api/units/#{unit.id}/staff-grant-extension", data_to_post + assert_equal 403, last_response.status, 'Should not allow 0 weeks extension' + end + + def test_staff_grant_extension_negative_weeks + unit = FactoryBot.create(:unit) + project = unit.projects.first + staff = FactoryBot.create(:user, role: Role.tutor) + unit.employ_staff(staff, Role.tutor) + td = unit.task_definitions.first + + data_to_post = { + student_ids: [project.student.id], + task_definition_id: td.id, + weeks_requested: -1, + comment: 'Negative weeks' + } + + add_auth_header_for user: staff + post_json "/api/units/#{unit.id}/staff-grant-extension", data_to_post + assert_equal 403, last_response.status, 'Should not allow negative weeks extension' + end + + def test_staff_grant_extension_missing_params + unit = FactoryBot.create(:unit) + staff = FactoryBot.create(:user, role: Role.tutor) + unit.employ_staff(staff, Role.tutor) + + data_to_post = { + student_ids: [1], + # Missing task_definition_id and weeks_requested + comment: 'Missing params' + } + + add_auth_header_for user: staff + post_json "/api/units/#{unit.id}/staff-grant-extension", data_to_post + assert_equal 400, last_response.status, 'Should require all parameters' + end + + def test_staff_grant_extension_transaction_rollback + unit = FactoryBot.create(:unit) + project = unit.projects.first + staff = FactoryBot.create(:user, role: Role.tutor) + unit.employ_staff(staff, Role.tutor) + td = unit.task_definitions.first + + # Test case 1: One valid student, one skipped student + data_to_post = { + student_ids: [project.student.id, 999999], # One valid, one invalid + task_definition_id: td.id, + weeks_requested: 1, + comment: 'Transaction test with skipped student' + } + + add_auth_header_for user: staff + post_json "/api/units/#{unit.id}/staff-grant-extension", data_to_post + assert_equal 201, last_response.status, 'Should succeed for valid student' + + response = last_response_body + assert response["successful"].length == 1, 'Should have one successful extension' + assert response["skipped"].length == 1, 'Should have one skipped student' + assert response["failed"].empty?, 'Should have no failed extensions' + assert response["skipped"][0]["student_id"] == 999999, 'Should have skipped the invalid student ID' + assert response["skipped"][0]["reason"] == 'Student not found in unit', 'Should have correct skip reason' + + # Verify only the valid student got an extension + task = project.task_for_task_definition(td) + assert task.extensions == 1, 'Should have one extension for the valid student' + + # Test case 2: Test actual transaction rollback + # Create a second project to test with + project2 = unit.projects.create!( + user: FactoryBot.create(:user, role: Role.student), + enrolled: true + ) + + # First, grant extensions to both students + data_to_post = { + student_ids: [project.student.id, project2.student.id], + task_definition_id: td.id, + weeks_requested: 1, + comment: 'Initial extensions' + } + + add_auth_header_for user: staff + post_json "/api/units/#{unit.id}/staff-grant-extension", data_to_post + assert_equal 201, last_response.status, 'Should succeed for both students' + + # Verify both students got extensions + task1 = project.task_for_task_definition(td) + task2 = project2.task_for_task_definition(td) + assert task1.extensions == 2, 'First student should have two extensions' + assert task2.extensions == 1, 'Second student should have one extension' + + # Now try to grant extensions with a task that would cause a failure + # Use a task that's past its deadline to force a failure + td.due_date = Time.zone.now - 1.day + td.save! + + data_to_post = { + student_ids: [project.student.id, project2.student.id], + task_definition_id: td.id, + weeks_requested: 1, + comment: 'Transaction rollback test' + } + + add_auth_header_for user: staff + post_json "/api/units/#{unit.id}/staff-grant-extension", data_to_post + assert_equal 403, last_response.status, 'Should fail with 403 when task is past deadline' + + # Verify neither student got a new extension (transaction rolled back) + task1.reload + task2.reload + assert task1.extensions == 2, 'First student should still have two extensions' + assert task2.extensions == 1, 'Second student should still have one extension' + + td.destroy! + unit.destroy! + end + + def test_staff_grant_extension_invalid_unit + unit = FactoryBot.create(:unit) + project = unit.projects.first + staff = FactoryBot.create(:user, role: Role.tutor) + unit.employ_staff(staff, Role.tutor) + td = unit.task_definitions.first + + data_to_post = { + student_ids: [project.student.id], + task_definition_id: td.id, + weeks_requested: 1, + comment: 'Invalid unit' + } + + add_auth_header_for user: staff + post_json "/api/units/999999/staff-grant-extension", data_to_post + assert_equal 404, last_response.status, 'Should return 404 for invalid unit' + end + + def test_staff_grant_extension_invalid_task + unit = FactoryBot.create(:unit) + project = unit.projects.first + staff = FactoryBot.create(:user, role: Role.tutor) + unit.employ_staff(staff, Role.tutor) + + data_to_post = { + student_ids: [project.student.id], + task_definition_id: 999999, + weeks_requested: 1, + comment: 'Invalid task' + } + + add_auth_header_for user: staff + post_json "/api/units/#{unit.id}/staff-grant-extension", data_to_post + assert_equal 404, last_response.status, 'Should return 404 for invalid task definition' + end +end