Skip to content

feat: add audit instrumentation for cancel remind and redeem - #272

Open
gshivajibiradar wants to merge 2 commits into
mainfrom
ENT-12029-audit-cancel-remind-redeem
Open

feat: add audit instrumentation for cancel remind and redeem#272
gshivajibiradar wants to merge 2 commits into
mainfrom
ENT-12029-audit-cancel-remind-redeem

Conversation

@gshivajibiradar

@gshivajibiradar gshivajibiradar commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

PR Title

feat: audit instrumentation for cancel, remind and redeem

PR Description

Implements audit instrumentation for the cancel, remind, and learner redemption flows in enterprise-access.

The changes ensure audit action records are created with the correct actor attribution and source for admin-initiated and learner-initiated operations.

Changes

  • Added audit action recording for content assignment cancellation.
  • Added audit instrumentation for reminder flows.
  • Propagated admin actor identity through the async reminder Celery task.
  • Added audit instrumentation for learner redemption.
  • Added the appropriate actor type, LMS user ID, learner information, and action source.
  • Added/updated tests covering the new audit paths.

Testing

  • 2122 tests passed
  • 37 tests skipped
  • Overall code coverage: 88%
  • Patch/diff coverage: 100%
  • 18 changed lines covered
  • 0 missing lines

@gshivajibiradar
gshivajibiradar requested review from a team as code owners August 20, 2026 09:34
@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.64%. Comparing base (54d4349) to head (89bbb55).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #272   +/-   ##
=======================================
  Coverage   87.64%   87.64%           
=======================================
  Files         157      157           
  Lines       13450    13454    +4     
  Branches     1303     1303           
=======================================
+ Hits        11788    11792    +4     
  Misses       1359     1359           
  Partials      303      303           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds audit instrumentation to content-assignment cancellation, reminder, and learner redemption flows so that assignment action rows capture actor attribution (admin vs learner vs system) and request source across sync and async boundaries.

Changes:

  • Writes CANCELLED audit actions synchronously in cancel_assignments() with admin actor attribution, decoupling audit from best-effort async email notification.
  • Propagates actor attribution through the reminder Celery task and records it on REMINDED actions.
  • Records learner actor attribution/source/customer UUID on REDEEMED actions (success + failure) and adds/updates tests for these paths.

Critical / high-severity issues to address:

  • The cancel-email Celery task still writes an errored CANCELLED action on failure via SendCancelEmailTask.add_errored_action, which can now create duplicate/misleading CANCELLED audit rows (one “successful cancel” from the API + one “email_error cancel” from the task) and the errored row currently lacks the new audit fields.
  • Reminder task failure path currently drops actor attribution (success path passes actor fields, but on_failure() does not), so errored REMINDED actions will be attributed to SYSTEM unless fixed.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
enterprise_access/apps/subsidy_access_policy/tests/test_models.py Extends redemption tests to assert learner actor attribution/source/customer UUID on REDEEMED actions.
enterprise_access/apps/subsidy_access_policy/models.py Passes learner actor attribution/source into redeemed action creation (success + error).
enterprise_access/apps/content_assignments/tests/test_tasks.py Adds tests ensuring reminder task preserves actor attribution and defaults safely when omitted.
enterprise_access/apps/content_assignments/tests/test_api.py Adds tests for cancel/remind API helpers to ensure actor attribution is passed/written correctly.
enterprise_access/apps/content_assignments/tasks.py Updates reminder task signature to accept actor attribution and records it on success; removes duplicated success-cancel action write in cancel-email task body.
enterprise_access/apps/content_assignments/models.py Extends action helper methods to store actor attribution/source/customer UUID + metadata for reminded/redeemed/cancelled actions.
enterprise_access/apps/content_assignments/api.py Writes CANCELLED audit actions synchronously during bulk cancellation; propagates actor attribution into reminder task payload.
enterprise_access/apps/api/v1/views/content_assignments/assignments_admin.py Passes admin actor attribution/source from DRF view into cancel/remind APIs.
enterprise_access/apps/api/v1/tests/test_assignment_views.py Updates view tests to assert actor attribution is passed into reminder task calls.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread enterprise_access/apps/content_assignments/tasks.py
Comment thread enterprise_access/apps/content_assignments/tasks.py
Comment thread enterprise_access/apps/content_assignments/tasks.py
Comment thread enterprise_access/apps/content_assignments/models.py
Comment thread enterprise_access/apps/content_assignments/api.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants