Skip to content

Count only active learners against the learners cap#714

Merged
payamnj merged 1 commit into
masterfrom
fix/can-enroll-learner-active-only
Jul 11, 2026
Merged

Count only active learners against the learners cap#714
payamnj merged 1 commit into
masterfrom
fix/can-enroll-learner-active-only

Conversation

@payamnj

@payamnj payamnj commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Summary

`Organization.can_enroll_learner()` (`django_email_learning/models/organizations.py`) previously counted every `Learner` row belonging to the organization against `MAX_LEARNERS_PER_ORGANIZATION`, regardless of enrollment status. A learner with no enrollment at all, or only an unverified/completed/deactivated one, still counted toward the cap.

It now counts distinct learners with at least one `EnrollmentStatus.ACTIVE` enrollment:

```python
def can_enroll_learner(self) -> bool:
cap = self.get_learners_cap()
if not cap:
return True
active_learner_count = self.learner_set.filter(enrollments__status=EnrollmentStatus.ACTIVE).distinct().count()
return active_learner_count < cap


\`.distinct()\` matters here — without it, a single learner active in multiple courses would be counted once per enrollment (multiple joined rows), inflating the count.

## Test changes

- Every existing cap test across the enrollment paths (platform \`EnrollmentsView\`, public \`EnrollView\`, \`EnrollCommand\`, the organization API's \`can_enroll_learner\` field, and the public \`enrollmentOpen\` context) created a bare \`Learner\` with no enrollment to simulate "cap reached." Updated each to give that learner an active enrollment, since a bare \`Learner\` no longer counts.
- Added \`test_can_enroll_learner_ignores_learners_without_active_enrollment\` — learners with no enrollment, or only unverified/completed/deactivated ones, don't count.
- Added \`test_can_enroll_learner_counts_distinct_learners_not_enrollments\` — a learner active in two courses only counts once. I verified this test actually catches a missing \`.distinct()\` (temporarily removed it, confirmed the test fails, restored it) before finalizing.

## Docs

Updated \`docs/source/platform/learners.rst\`'s "Learner Capacity" section to document the active-only counting.

## Test plan

- [x] \`pytest -q\` — 786 passed, 81.84% coverage
- [x] \`mypy\` — no issues across 164 source files
- [x] \`python manage.py makemigrations --check\` — no drift (query-only change, no schema change)
- [x] \`ruff check\` / \`ruff format\` — clean
- [x] Manually confirmed the distinct-count test fails without \`.distinct()\` and passes with it

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Organization.can_enroll_learner() previously counted every Learner row
in the organization, regardless of enrollment status — a learner with
no enrollment, or only an unverified/completed/deactivated one, still
counted against MAX_LEARNERS_PER_ORGANIZATION. It now counts distinct
learners with at least one ACTIVE enrollment.

Updated all existing cap tests across the enrollment paths (platform,
public, EnrollCommand) to give the "existing" learner an active
enrollment, since a bare Learner no longer counts. Added tests for the
two new behaviors: learners without an active enrollment don't count,
and a learner active in multiple courses only counts once (.distinct()).

Documented the active-only counting in docs/source/platform/learners.rst.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@payamnj payamnj merged commit cfc3e56 into master Jul 11, 2026
9 checks passed
@payamnj payamnj deleted the fix/can-enroll-learner-active-only branch July 11, 2026 06:25
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.

1 participant