Skip to content

chore: refactor LCR bulk actions to align with Content Assignments#100

Merged
iloveagent57 merged 1 commit into
mainfrom
chore/lcr-bulk-action-refactor
Mar 26, 2026
Merged

chore: refactor LCR bulk actions to align with Content Assignments#100
iloveagent57 merged 1 commit into
mainfrom
chore/lcr-bulk-action-refactor

Conversation

@zwidekalanga

@zwidekalanga zwidekalanga commented Mar 16, 2026

Copy link
Copy Markdown
Contributor

Description:
Refactor LCR bulk actions to align with Content Assignments as bulk action in frontend should behave identically.

Jira:
ENT-11480

Merge checklist:

  • ./manage.py makemigrations has been run
    • Note: This must be run if you modified any models.
      • It may or may not make a migration depending on exactly what you modified, but it should still be run.

Post merge:

  • Ensure that your changes went out to the stage instance
  • Deploy to prod instance

@zwidekalanga
zwidekalanga requested review from a team as code owners March 16, 2026 11:42
@zwidekalanga
zwidekalanga force-pushed the chore/lcr-bulk-action-refactor branch 3 times, most recently from 1e522eb to 50f15dd Compare March 17, 2026 13:43
@codecov

codecov Bot commented Mar 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.71014% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.20%. Comparing base (dfdeddf) to head (d958f33).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ise_access/apps/api/v1/views/browse_and_request.py 64.86% 10 Missing and 3 partials ⚠️
enterprise_access/apps/subsidy_request/api.py 96.29% 0 Missing and 1 partial ⚠️

❌ Your patch status has failed because the patch coverage (79.71%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #100      +/-   ##
==========================================
+ Coverage   84.01%   84.20%   +0.18%     
==========================================
  Files         144      144              
  Lines       12226    12128      -98     
  Branches     1170     1155      -15     
==========================================
- Hits        10272    10212      -60     
+ Misses       1625     1596      -29     
+ Partials      329      320       -9     

☔ View full report in Codecov by Sentry.
📢 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.

@zwidekalanga
zwidekalanga force-pushed the chore/lcr-bulk-action-refactor branch 3 times, most recently from e2b4511 to 25ee164 Compare March 18, 2026 12:35
@iloveagent57 iloveagent57 self-assigned this Mar 18, 2026
@iloveagent57
iloveagent57 requested a review from Copilot March 18, 2026 16:42
@iloveagent57
iloveagent57 force-pushed the chore/lcr-bulk-action-refactor branch from 25ee164 to e05df3e Compare March 18, 2026 16:42

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

Refactors Learner Credit Request (LCR) bulk action behavior to mirror Content Assignments patterns, centralizing bulk logic in subsidy_request.api and updating the B&R v1 endpoints/serializers accordingly.

Changes:

  • Added state-group constants (APPROVABLE_STATES, DECLINABLE_STATES, REMINDABLE_STATES) and updated API/view filtering to use them.
  • Introduced decline_learner_credit_requests API and refactored remind/decline flows to use bulk action/audit creation patterns.
  • Updated DRF serializers/endpoints and expanded/adjusted test coverage for the new decline-all and API-layer behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
enterprise_access/apps/subsidy_request/tests/test_api.py Adds API-layer tests for approve/remind/cancel/decline bulk behaviors.
enterprise_access/apps/subsidy_request/models.py Removes unused bulk approve helper; keeps bulk decline behavior in model.
enterprise_access/apps/subsidy_request/constants.py Adds state-group constants to standardize bulk operation filtering.
enterprise_access/apps/subsidy_request/api.py Refactors bulk approve/remind and adds bulk decline API with audit/task behavior.
enterprise_access/apps/api/v1/views/browse_and_request.py Updates LCR endpoints: approve-all response body, decline (single) refactor, adds decline-all.
enterprise_access/apps/api/v1/tests/test_browse_and_request_views.py Updates view tests to match new decline-all/decline behaviors and routes.
enterprise_access/apps/api/serializers/subsidy_requests.py Adds decline-all serializer; makes remind require UUID list (removes legacy single UUID support).
enterprise_access/apps/api/serializers/init.py Exposes new serializer; removes bulk approve/decline serializers.
Comments suppressed due to low confidence (1)

enterprise_access/apps/subsidy_request/api.py:54

  • approve_learner_credit_requests returns {"approved": [], "failed": ...} when there are no approvable requests (line 53), but returns failed_approval in the normal path (line 122). This inconsistent contract can cause callers to silently miss failures (e.g., views that only read failed_approval). Consider standardizing on a single key (e.g., always failed_approval) across all return paths and updating callers/tests accordingly.
    requests_to_process = [
        req for req in learner_credit_requests
        if req.state in APPROVABLE_STATES
    ]
    if not requests_to_process:
        return {"approved": [], "failed": list(learner_credit_requests)}

Comment thread enterprise_access/apps/api/serializers/subsidy_requests.py
Comment thread enterprise_access/apps/api/v1/views/browse_and_request.py Outdated
Comment thread enterprise_access/apps/api/v1/views/browse_and_request.py Outdated

@iloveagent57 iloveagent57 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

approved to unblock, but this change is worth making: #100 (comment)

@zwidekalanga
zwidekalanga force-pushed the chore/lcr-bulk-action-refactor branch 2 times, most recently from 34da27c to 387c4c4 Compare March 23, 2026 09:01
@iloveagent57
iloveagent57 force-pushed the chore/lcr-bulk-action-refactor branch from 387c4c4 to d958f33 Compare March 26, 2026 14:21
@iloveagent57
iloveagent57 merged commit 7099e87 into main Mar 26, 2026
4 of 5 checks passed
@iloveagent57
iloveagent57 deleted the chore/lcr-bulk-action-refactor branch March 26, 2026 14:40
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