-
Notifications
You must be signed in to change notification settings - Fork 409
feat(api): Staff Grant Extension - Upstream Rebase #565
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
SteveDala
wants to merge
27
commits into
doubtfire-lms:10.0.x
Choose a base branch
from
SteveDala:SGE-rebase-upstream
base: 10.0.x
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 26 commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
7981de8
docs: fix typo in API root, add doc_version
SteveDala 0e89378
Merge branch '10.0.x' of https://github.com/doubtfire-lms/doubtfire-a…
SteveDala 069a023
Merge branch '10.0.x' of https://github.com/SteveDala/doubtfire-api i…
SteveDala 835bf79
Merge branch '10.0.x' of https://github.com/SteveDala/doubtfire-api i…
SteveDala 1312537
chore: update gems
SteveDala a90072d
feat(api): add staff grant extension endpoint
d27fd32
refactor(tests): replace double quotes with single quotes in non-inte…
SahiruWithanage fb80917
refactor(api): unify extension handling via shared service
SahiruWithanage ed7ae3d
feat(notifications): send extension notifications
SahiruWithanage c4162a1
Make a comment line change
SahiruWithanage b885a34
fix(mailer): fix email sender and add error handling
SahiruWithanage 82ea701
feat: add notification table and model
samindiii 6a310ba
feat: define notification api to GET and DELETE
samindiii 3c99671
Create in-system notifications for students with successfull extensions
samindiii dca5e00
feat: fix email notifications for SGE feature
SahiruWithanage 6bee233
refactor: use unit code instead of unit name in extension notifications
SahiruWithanage fe6cd01
Merge branch 'feature/staff-grant-extension-backend-t1-complete' of h…
SteveDala 755f467
chore: update gems, regen lockfile, schema version
SteveDala 1b8c724
fix: remove duplicate api endpoints
SteveDala 4b2978b
chore: update latest from doubtfire
SteveDala c7f9b66
chore: address rubocop nits
SteveDala 7a98ff2
chore: Remove shebang from Rakefile (rubocop)
SteveDala bf6347c
fix: address testing issues with SGE rebase
SteveDala 9dad019
chore: remove block comment (rubocop)
SteveDala 4408f58
fix: broken regex for last names with '
SteveDala 90c8789
fix: failing test due to name ambiguity
SteveDala 1df9bb2
fix: address changes requested to #565
SteveDala File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
SteveDala marked this conversation as resolved.
Show resolved
Hide resolved
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SteveDala marked this conversation as resolved.
Show resolved
Hide resolved
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| class NotificationsApi < Grape::API | ||
| helpers AuthenticationHelpers | ||
| helpers AuthorisationHelpers | ||
|
|
||
| before do | ||
| authenticated? | ||
| end | ||
|
|
||
| desc 'Get current user notifications' | ||
| get '/notifications' do | ||
| notifications = current_user.notifications.order(created_at: :desc) | ||
| # Return array of notifications as JSON (id and message only) | ||
| notifications.as_json(only: [:id, :message]) | ||
| end | ||
|
|
||
| desc 'Delete user notification by id' | ||
| delete '/notifications/:id' do | ||
| notification = current_user.notifications.find_by(id: params[:id]) | ||
| error!({ error: 'Notification not found' }, 404) unless notification | ||
| notification.destroy | ||
| status 204 | ||
| end | ||
|
|
||
| desc 'Delete all user notifications' | ||
| delete '/notifications' do | ||
| current_user.notifications.delete_all | ||
| status 204 | ||
| end | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,178 @@ | ||
| 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? | ||
| unless current_user.has_tutor_capability? | ||
| error!( | ||
| { | ||
| error: 'Not authorized to grant extensions', | ||
| code: 'UNAUTHORIZED', | ||
| details: {} | ||
| }, | ||
| 403 | ||
| ) | ||
| end | ||
| 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 (1-4)' | ||
| requires :comment, type: String, desc: 'Reason for extension (max 300 characters)' | ||
| 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], | ||
| 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, | ||
| extension_comment: extension_comment # Store internally for notifications | ||
| } | ||
| 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 | ||
|
|
||
| # Send notifications only if successful and after processing all students | ||
| if results[:successful].any? | ||
| # Use the extension comments directly from the service results (thread-safe) | ||
| successful_extensions = results[:successful].map do |result| | ||
| extension_comment = result[:extension_comment] | ||
| if extension_comment.nil? | ||
| Rails.logger.warn "No extension comment found for project #{result[:project_id]}" | ||
| nil | ||
| else | ||
| Rails.logger.debug "Using extension comment: #{extension_comment.id} for project #{result[:project_id]}" | ||
| extension_comment | ||
| end | ||
| end | ||
|
|
||
| # Filter out any nil results in case a comment wasn't found | ||
| successful_extensions.compact! | ||
| Rails.logger.info "Processing #{successful_extensions.count} successful extensions for notifications" | ||
|
|
||
| if successful_extensions.any? | ||
| begin | ||
| Rails.logger.info "Sending extension notifications for #{successful_extensions.count} extensions" | ||
| NotificationsMailer.extension_granted( | ||
| successful_extensions, | ||
| current_user, | ||
| params[:student_ids].count, | ||
| results[:failed], | ||
| true # is_staff_grant = true | ||
| ).deliver_now | ||
| Rails.logger.info "Extension notifications sent successfully" | ||
| rescue StandardError => e | ||
| Rails.logger.error "Failed to send extension notifications: #{e.message}" | ||
| Rails.logger.error e.backtrace.join("\n") | ||
| # Don't fail the entire request if email fails, but log the error | ||
| end | ||
|
|
||
| # Create in-system notifications for successful extensions | ||
| results[:successful].each do |result| | ||
| student = User.find_by(id: result[:student_id]) | ||
| next unless student | ||
|
|
||
| Notification.create!( | ||
| user_id: student.id, | ||
| message: "#{unit.code}: You were granted an extension for task '#{task_definition.name}'." | ||
| ) | ||
| end | ||
| end | ||
| 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 |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.