fix(mail): db:generate_feedback_chips has no production guard and modifies existing outcomes - #113
Open
gnahk66 wants to merge 1561 commits into
Open
fix(mail): db:generate_feedback_chips has no production guard and modifies existing outcomes#113gnahk66 wants to merge 1561 commits into
gnahk66 wants to merge 1561 commits into
Conversation
…tion/11.0.x-all-features-20260824
… integration/11.0.x-all-features-20260824
…n/11.0.x-all-features-20260824 # Conflicts: # db/schema.rb # test/sidekiq/scheduled_job_test.rb
…ll-features-20260824
…ll-features-20260824
…ll-features-20260824 # Conflicts: # test/api/tasks_api_test.rb
…ll-features-20260824
…ll-features-20260824
…to integration/11.0.x-all-features-20260824
…tion/11.0.x-all-features-20260824
…gration/11.0.x-all-features-20260824
…gration/11.0.x-all-features-20260824
…gration/11.0.x-all-features-20260824
…l-concurrency ci(codeql): cancel superseded CodeQL runs on the same ref
fix(api): mailer text leak, image bloat, dependabot path and an api-root auth guard
chore(ci): lint workflow YAML with actionlint
check_jplag_similarity ran only from rake tasks and the nightly container cron, and that cron does not treat a threshold change as a reason to rescan, so a convenor who lowered plagiarism_warn_pct had to email an administrator and wait. Add CheckUnitSimilarityJob, POST /units/:id/similarity/scan gated on a new :run_similarity_scan permission for convenors and admins, and a config/schedule.yml entry so the nightly run is visible in Sidekiq. The job locks per unit id, so a nightly child and an on-demand request for the same unit reject rather than race on the shared jplag directory, and the nightly run moves out of the crontab so it does not run in both places. A 30-minute cooldown rate limits the endpoint.
The task update endpoint added a Discussed in class comment whenever discussed was not nil, so discussed:false marked the task discussed, and that comment type cannot be removed from the UI. discussed:false now removes the most recent discussed comment. The removal is refused when it would leave a discussion required task complete without evidence, and is otherwise deferred until the trigger and grade have applied so a refused request does not destroy the comment.
remove_comment_read_entry passed a conditions hash to ActiveRecord::Relation#delete_all, which takes no arguments on Rails 8, so every mark-as-unread raised ArgumentError and the Grape rescue turned it into a 500. Scope the receipts with where(...) first, then delete_all. Adds api tests for the mark-as-unread endpoint: the happy path (receipt removed, comment unread) and an unauthorised caller rejected. The endpoint had no coverage before.
…e-preference feat(users): persist and timestamp the theme preference
feat(units): on-demand plagiarism rescan for convenors
fix(tasks): make discussed:false unmark a task instead of marking it
…achment-error-status fix(discussions): return 4xx for invalid audio attachments
fix(domain): "Mark comment as unread" always returns 500 - delete_all
…1-feedback-state CPD-FB01: expose privacy-safe dashboard feedback state
fix(uploads): an oversized or empty chat attachment is answered with
docs(docs): the student import lead time is read from a misspelled variable
fix(uploads): the portfolio upload endpoint enforces no size limit at all
fix(push): add notification icon and badge
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
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.
Description
Fixes db:generate_feedback_chips generating fake feedback chips for existing learning outcomes without invoking the production guard.
This change:
Adds [:skip_prod, :environment] to use the existing production confirmation guard.
Collects learning outcome IDs from both create_list loops.
Restricts chip generation to those newly created outcomes.
Adds Minitest coverage for outcome isolation and production refusal.
The existing guard remains unchanged: entering Yes permits production execution, and staging does not trigger confirmation.
Fixes # (issue)
Dependencies: no new package dependencies.
Target branch: 11.0.x.
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Test file: test/sidekiq/generate_feedback_data_test.rb
Outcome isolation: verifies existing outcomes receive no additional chips and newly created outcomes receive the expected chips.
Production refusal: simulates production and answers No, verifying an exception occurs without creating outcomes or chips.
Run from doubtfire-deploy/development:
docker compose exec -e RAILS_ENV=test doubtfire-api bin/rails test test/sidekiq/generate_feedback_data_test.rb
The tests use the test database. Production is simulated through stubbing; no production database is used.
Manual check beyond the tests: reviewed skip_prod.rake and confirmed that production permits explicit Yes confirmation, while staging is not guarded.
Test setup note: test:setup failed because it references the missing db:setup_jplag_submissions task. The test database was populated using RAILS_ENV=test bin/rails db:populate; no setup-task source changes were made.
Checklist:
If you have any questions, please contact @macite or @jakerenzella.