Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ def default_settings(monkeypatch, settings):

settings.FEATURES[features.IGNORE_EDX_FAILURES] = False
settings.FEATURES[features.SYNC_ON_DASHBOARD_LOAD] = False
settings.FEATURES[features.ENABLE_PROGRAM_SPECIFIC_PATHWAY_SCHOOLS] = False


@pytest.fixture(autouse=True)
Expand Down
29 changes: 27 additions & 2 deletions courses/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
PaidCourseRun,
PaidProgram,
PartnerSchool,
PartnerSchoolProgram,
Program,
ProgramCertificate,
ProgramCollectionItem,
Expand Down Expand Up @@ -1137,18 +1138,42 @@ def get_queryset(self, request):
return qs.select_related("programcertificate", "courseruncertificate")


class PartnerSchoolProgramInline(admin.TabularInline):
"""Inline for assigning a partner school to programs."""

model = PartnerSchoolProgram
extra = 1
fields = ("program", "email", "alt_email")
autocomplete_fields = ("program",)
verbose_name = "Program assignment"
verbose_name_plural = (
"Program assignments (add one row per recipient address; "
"leave email blank to use the school's default)"
)


@admin.register(PartnerSchool)
class PartnerSchoolAdmin(TimestampedModelAdmin):
"""Admin for PartnerSchool"""

model = PartnerSchool
list_display = ["name", "email"]
inlines = [PartnerSchoolProgramInline]
list_display = ["name", "email", "assigned_programs"]
list_filter = ["programs"]
search_fields = ["name", "email"]

def get_queryset(self, request): # noqa: ARG002
"""Use the all_objects manager so we can see everything."""

return self.model.all_objects.get_queryset()
return self.model.all_objects.get_queryset().prefetch_related("programs")

@admin.display(description="Programs")
def assigned_programs(self, obj):
"""Comma-separated list of assigned programs for the changelist."""

return ", ".join(
sorted({program.readable_id for program in obj.programs.all()})
)

def delete_model(self, request, obj): # noqa: ARG002
"""Soft-delete the model."""
Expand Down
41 changes: 40 additions & 1 deletion courses/admin_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@
from django.urls import reverse

from courses.admin import CourseRunEnrollmentAdmin
from courses.factories import CourseRunEnrollmentFactory
from courses.factories import (
CourseRunEnrollmentFactory,
PartnerSchoolFactory,
PartnerSchoolProgramFactory,
ProgramFactory,
)
from courses.models import CourseRunEnrollment
from openedx.constants import OPENEDX_ENROLLMENT_REPAIR_MAX_RETRIES

Expand Down Expand Up @@ -56,3 +61,37 @@ def test_repair_exhausted_display(edx_enrolled, retry_count, expected):
admin_instance = CourseRunEnrollmentAdmin(CourseRunEnrollment, django_admin.site)

assert admin_instance.repair_exhausted(enrollment) is expected


def test_partner_school_admin_change_page_lists_program_inline(admin_client):
"""The change page exposes the program assignment inline, including alt_email."""
school = PartnerSchoolFactory.create()
program = ProgramFactory.create(title="Supply Chain Management")
PartnerSchoolProgramFactory.create(partner_school=school, program=program)

resp = admin_client.get(
reverse("admin:courses_partnerschool_change", args=[school.id])
)

assert resp.status_code == 200
assert b"program_links" in resp.content
assert b"alt_email" in resp.content


def test_partner_school_admin_changelist_filters_by_program(admin_client):
"""The changelist can be filtered down to one program's schools."""
scm = ProgramFactory.create()
dedp = ProgramFactory.create()
scm_school = PartnerSchoolFactory.create(name="SCM Only School")
dedp_school = PartnerSchoolFactory.create(name="DEDP Only School")
PartnerSchoolProgramFactory.create(partner_school=scm_school, program=scm)
PartnerSchoolProgramFactory.create(partner_school=dedp_school, program=dedp)

resp = admin_client.get(
reverse("admin:courses_partnerschool_changelist"),
{"programs__id__exact": scm.id},
)

assert resp.status_code == 200
assert b"SCM Only School" in resp.content
assert b"DEDP Only School" not in resp.content
27 changes: 27 additions & 0 deletions courses/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
first_or_none,
has_equal_properties,
)
from mitol.olposthog.features import is_enabled
from opaque_keys.edx.keys import CourseKey
from requests.exceptions import ConnectionError as RequestsConnectionError
from requests.exceptions import HTTPError
Expand All @@ -48,6 +49,7 @@
Department,
EnrollmentMode,
PaidCourseRun,
PartnerSchool,
Program,
ProgramCertificate,
ProgramEnrollment,
Expand Down Expand Up @@ -1496,6 +1498,31 @@ def manage_program_certificate_access(user, program, revoke_state):
return True


def partner_schools_for_program(program):
"""
Return the pathway schools a learner may share this program's record with.

While ENABLE_PROGRAM_SPECIFIC_PATHWAY_SCHOOLS is off this returns every active
school, which is the pre-12321 behavior. Once the flag is on, only schools
assigned to this program are returned.

`.distinct()` is required: a school with more than one recipient row for the
program joins once per row and would otherwise be listed twice.

Args:
program (Program): the program whose record is being shared

Returns:
QuerySet of PartnerSchool
"""
schools = PartnerSchool.objects.all()

if is_enabled(features.ENABLE_PROGRAM_SPECIFIC_PATHWAY_SCHOOLS):
schools = schools.filter(programs=program).distinct()

return schools


def resolve_courseware_object_from_id(
courseware_id: str,
) -> Program | Course | CourseRun | None:
Expand Down
60 changes: 60 additions & 0 deletions courses/api_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
manage_course_run_certificate_access,
manage_program_certificate_access,
override_user_grade,
partner_schools_for_program,
process_course_run_grade_certificate,
pull_course_modes,
sync_course_mode,
Expand All @@ -76,6 +77,8 @@
CourseRunGradeFactory,
DepartmentFactory,
EnrollmentModeFactory,
PartnerSchoolFactory,
PartnerSchoolProgramFactory,
ProgramCertificateFactory,
ProgramEnrollmentFactory,
ProgramFactory,
Expand All @@ -98,6 +101,7 @@
)
from ecommerce.factories import LineFactory, OrderFactory, ProductFactory
from ecommerce.models import Basket, OrderStatus
from main import features
from main.constants import USER_MSG_TYPE_B2B_ENROLL_SUCCESS
from main.test_utils import MockHttpError
from openedx.constants import (
Expand Down Expand Up @@ -4067,3 +4071,59 @@ def _side_effect(user, program, force_create=False): # noqa: FBT002
assert stats["failed"] == 1
assert stats["processed"] == 2
assert mock_generate.call_count == 2


def test_partner_schools_for_program_unfiltered_when_flag_off(settings):
"""With the flag off every active school is returned, preserving old behavior."""
settings.FEATURES[features.ENABLE_PROGRAM_SPECIFIC_PATHWAY_SCHOOLS] = False
scm = ProgramFactory.create()
dedp = ProgramFactory.create()
scm_school = PartnerSchoolFactory.create(name="SCM School")
dedp_school = PartnerSchoolFactory.create(name="DEDP School")
PartnerSchoolProgramFactory.create(partner_school=scm_school, program=scm)
PartnerSchoolProgramFactory.create(partner_school=dedp_school, program=dedp)

result = partner_schools_for_program(scm)

assert sorted(school.name for school in result) == ["DEDP School", "SCM School"]


def test_partner_schools_for_program_filtered_when_flag_on(settings):
"""With the flag on only the program's own schools are returned."""
settings.FEATURES[features.ENABLE_PROGRAM_SPECIFIC_PATHWAY_SCHOOLS] = True
scm = ProgramFactory.create()
dedp = ProgramFactory.create()
scm_school = PartnerSchoolFactory.create(name="SCM School")
dedp_school = PartnerSchoolFactory.create(name="DEDP School")
PartnerSchoolProgramFactory.create(partner_school=scm_school, program=scm)
PartnerSchoolProgramFactory.create(partner_school=dedp_school, program=dedp)

result = partner_schools_for_program(scm)

assert [school.name for school in result] == ["SCM School"]


def test_partner_schools_for_program_deduplicates_multi_recipient_school(settings):
"""A school with two recipient rows appears once when the flag is on."""
settings.FEATURES[features.ENABLE_PROGRAM_SPECIFIC_PATHWAY_SCHOOLS] = True
program = ProgramFactory.create()
school = PartnerSchoolFactory.create(name="Reykjavik University")
PartnerSchoolProgramFactory.create(
partner_school=school, program=program, email="vd@example.com"
)
PartnerSchoolProgramFactory.create(
partner_school=school, program=program, email="cs@example.com"
)

result = partner_schools_for_program(program)

assert [school.name for school in result] == ["Reykjavik University"]


def test_partner_schools_for_program_excludes_unassigned_when_flag_on(settings):
"""An untagged school is invisible once filtering is live."""
settings.FEATURES[features.ENABLE_PROGRAM_SPECIFIC_PATHWAY_SCHOOLS] = True
program = ProgramFactory.create()
PartnerSchoolFactory.create(name="Unassigned School")

assert list(partner_schools_for_program(program)) == []
11 changes: 11 additions & 0 deletions courses/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
EnrollmentMode,
LearnerProgramRecordShare,
PartnerSchool,
PartnerSchoolProgram,
Program,
ProgramCertificate,
ProgramEnrollment,
Expand Down Expand Up @@ -386,6 +387,16 @@ class Meta:
model = PartnerSchool


class PartnerSchoolProgramFactory(DjangoModelFactory):
partner_school = SubFactory(PartnerSchoolFactory)
program = SubFactory(ProgramFactory)
email = fuzzy.FuzzyText(suffix="@example.com")
alt_email = ""

class Meta:
model = PartnerSchoolProgram


class LearnerProgramRecordShareFactory(DjangoModelFactory):
user = SubFactory(UserFactory)
program = SubFactory(ProgramFactory)
Expand Down
32 changes: 25 additions & 7 deletions courses/mail_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import logging

from mitol.mail.api import get_message_sender
from mitol.olposthog.features import is_enabled

from courses.messages import (
CourseRunEnrollmentMessage,
Expand All @@ -11,6 +12,7 @@
PartnerSchoolSharingMessage,
)
from courses.models import CourseRun
from main import features
from main.settings import SITE_BASE_URL

log = logging.getLogger()
Expand Down Expand Up @@ -77,13 +79,29 @@ def send_partner_school_sharing_message(learner_record):
learner_record (LearnerProgramRecordShare): the learner record to send
"""
try:
with get_message_sender(PartnerSchoolSharingMessage) as sender:
sender.build_and_send_message(
learner_record.partner_school.email,
{
"learner_record": learner_record,
"record_link": f"{SITE_BASE_URL}/records/shared/{learner_record.share_uuid}",
},
# Second of two deliberate flag reads for hq#12321 (the other is
# courses.api.partner_schools_for_program). Gating mail here keeps the
# flag's promise: entering program assignments cannot change delivery
# until the flag is flipped, so data-entry mistakes stay harmless.
if is_enabled(features.ENABLE_PROGRAM_SPECIFIC_PATHWAY_SCHOOLS):
recipients = learner_record.partner_school.emails_for_program(
learner_record.program
)
else:
recipients = [learner_record.partner_school.email]
context = {
"learner_record": learner_record,
"record_link": f"{SITE_BASE_URL}/records/shared/{learner_record.share_uuid}",
}
with get_message_sender(PartnerSchoolSharingMessage) as sender:
for recipient in recipients:
try:
sender.build_and_send_message(recipient, context)
except Exception: # pylint: disable=broad-except # noqa: PERF203
log.exception(
"Error sending partner school sharing email to %s for share %s",
recipient,
learner_record.share_uuid,
)
except Exception: # pylint: disable=broad-except
log.exception("Error sending partner school sharing email")
Comment thread
AhtishamShahid marked this conversation as resolved.
Loading