From d153134d810101274994615c781cb2a08da668d5 Mon Sep 17 00:00:00 2001 From: Payam Date: Sat, 15 Aug 2026 16:49:44 +0400 Subject: [PATCH] feat(learners): send a learner's next content delivery immediately The enrollment dialog now shows the next scheduled content delivery under the course title, with an admin-only "send now" link beside it. Sending runs that one delivery as if the job had just picked it up: the email goes out, the schedule is marked delivered, and the follow-up work happens too - the next content is scheduled, or the enrollment graduates. Moving the schedule's time forward would not be equivalent, since the delivery job runs on a cron and the content would still wait for its next tick. The schedule is claimed with the same SCHEDULED -> PROCESSING compare-and-set the database queue uses, so a concurrent job run cannot send the same content twice, and DeliverContentsJob takes an optional queue so constructing one here does not claim work the job should handle. Implemented with the help of Claude Code. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 6 + .../jobs/deliver_contents_job.py | 17 +- .../platform/api/serializers/__init__.py | 2 + .../platform/api/serializers/learners.py | 41 +++++ django_email_learning/platform/api/urls.py | 7 + .../platform/api/views/__init__.py | 2 + .../platform/api/views/learners.py | 54 ++++++ .../platform/views/learners.py | 6 + .../services/manual_delivery_service.py | 120 +++++++++++++ docs/source/platform/learners.rst | 15 ++ frontend/platform/learners/Learners.jsx | 13 +- .../learners/components/NextDelivery.jsx | 81 +++++++++ .../src/test/platform/NextDelivery.test.jsx | 106 +++++++++++ .../api/test_views/test_enrollment_api.py | 62 ++++++- .../test_views/test_send_delivery_now_view.py | 165 ++++++++++++++++++ .../services/test_manual_delivery_service.py | 149 ++++++++++++++++ 16 files changed, 836 insertions(+), 10 deletions(-) create mode 100644 django_email_learning/services/manual_delivery_service.py create mode 100644 frontend/platform/learners/components/NextDelivery.jsx create mode 100644 frontend/src/test/platform/NextDelivery.test.jsx create mode 100644 tests/platform/api/test_views/test_send_delivery_now_view.py create mode 100644 tests/services/test_manual_delivery_service.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b7b071f..cf812d58 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm Changes prior to v1.0.0 are available in the [git history](https://github.com/AvaCodeSolutions/django-email-learning/commits/master). +## [Unreleased] + +### Added + +- **Send a learner's next content immediately from the Learners page** — The enrollment dialog now shows, under the course title, when the learner's next content is scheduled to arrive and which content it is, with a **send now** link beside it for organization admins. Sending runs the delivery there and then: the email goes out, the schedule is marked delivered, and the follow-up work happens exactly as it would during a job run — the next content is scheduled, or the enrollment graduates if that was the last one. Bringing the schedule's time forward was never equivalent, because the delivery job runs on a cron and the content would still wait for its next tick. Behind it is a new admin-only endpoint, `POST /api/platform/organizations//enrollments//delivery-schedules//send/`, and a `next_delivery` field on the enrollment detail response (`null` when nothing is scheduled). The schedule is claimed with the same `SCHEDULED → PROCESSING` compare-and-set the delivery queue uses, so a job run happening at the same moment cannot send the same content twice; a delivery that is no longer scheduled returns `409` and is left untouched, and one that fails to send is retried or blocked by the job's own retry logic. + ## [5.0.0] - 2026-08-15 > **Upgrading.** No migrations. One thing to check before you upgrade: if you call `POST /api/v1/enrollments/` and rely on the learner receiving a verification link, add `"verified": false` to the request body — that is now opt-in, and the default creates the enrollment active instead. Callers that read `status` from the `201` response get `active` rather than `unverified`. Nothing else in the API changed. diff --git a/django_email_learning/jobs/deliver_contents_job.py b/django_email_learning/jobs/deliver_contents_job.py index 309cc49f..9b66324f 100644 --- a/django_email_learning/jobs/deliver_contents_job.py +++ b/django_email_learning/jobs/deliver_contents_job.py @@ -40,8 +40,13 @@ def _get_delivery_workers() -> int: class DeliverContentsJob: - def __init__(self) -> None: - self.delivery_queue: TaskQueueProtocol[DeliverySchedule] = self.get_delivery_queue() + def __init__(self, delivery_queue: TaskQueueProtocol[DeliverySchedule] | None = None) -> None: + # A caller that only needs `process_delivery` for a schedule it has + # already claimed can pass its own queue, so constructing the job does + # not claim a batch of work the running job should be handling. + self.delivery_queue: TaskQueueProtocol[DeliverySchedule] = ( + delivery_queue if delivery_queue is not None else self.get_delivery_queue() + ) def start(self) -> JobExecution | None: job_execution = JobExecution.start_if_not_running(job_name=JobName.DELIVER_CONTENTS.value) @@ -80,7 +85,7 @@ def _run_sequential(self, job_execution: JobExecution) -> None: try: self.process_delivery(delivery_schedule) except Exception as e: - self._block_delivery(delivery_schedule, e) + self.block_delivery(delivery_schedule, e) # ── threaded (workers > 1) ─────────────────────────────────────────────── @@ -106,7 +111,7 @@ def _run_threaded(self, job_execution: JobExecution, workers: int) -> None: try: future.result() except Exception as e: - self._block_delivery(delivery_schedule, e) + self.block_delivery(delivery_schedule, e) # Avoid a tight spin-wait when all workers are busy if not done and futures: @@ -123,10 +128,10 @@ def _worker(self, delivery_schedule: DeliverySchedule) -> None: try: self.process_delivery(delivery_schedule) except Exception as e: - self._block_delivery(delivery_schedule, e) + self.block_delivery(delivery_schedule, e) raise - def _block_delivery(self, delivery_schedule: DeliverySchedule, exc: Exception) -> None: + def block_delivery(self, delivery_schedule: DeliverySchedule, exc: Exception) -> None: """Mark a delivery as BLOCKED and emit a metric.""" delivery_schedule.status = DeliveryStatus.BLOCKED delivery_schedule.save() diff --git a/django_email_learning/platform/api/serializers/__init__.py b/django_email_learning/platform/api/serializers/__init__.py index ec18e274..d8607bac 100644 --- a/django_email_learning/platform/api/serializers/__init__.py +++ b/django_email_learning/platform/api/serializers/__init__.py @@ -39,6 +39,7 @@ Event, EventType, LearnerDetailResponse, + NextDeliveryResponse, QuizSubmitedEvent, ReminderSentEvent, ) @@ -143,6 +144,7 @@ "EmailOpenedEvent", "Event", "EnrollmentResponse", + "NextDeliveryResponse", "LearnerDetailResponse", "GetOrCreateUserRequest", "UserResponse", diff --git a/django_email_learning/platform/api/serializers/learners.py b/django_email_learning/platform/api/serializers/learners.py index 0722d022..30dfc319 100644 --- a/django_email_learning/platform/api/serializers/learners.py +++ b/django_email_learning/platform/api/serializers/learners.py @@ -8,6 +8,7 @@ AssignmentSubmission, ContentDelivery, CourseContentType, + DeliverySchedule, DeliveryStatus, Enrollment, EnrollmentStatus, @@ -101,12 +102,51 @@ class Event(BaseModel): ) = Field(discriminator="type") # REGISTERED, VERIFIED, COURSE_COMPLETED have no additional data +class NextDeliveryResponse(BaseModel): + """The next content the learner is scheduled to receive on this enrollment.""" + + delivery_schedule_id: int + course_content_id: int + course_content_title: str + course_content_type: str + scheduled_at: datetime + + +def _next_delivery(enrollment: Enrollment) -> NextDeliveryResponse | None: + """The earliest still-scheduled delivery for `enrollment`, if any. + + Ordered by time, then id, matching the order the delivery job would reach + them in, so this names the delivery a "send now" acts on. + """ + schedule = ( + DeliverySchedule.objects.filter( + delivery__enrollment=enrollment, + status=DeliveryStatus.SCHEDULED, + ) + .select_related("delivery__course_content") + .order_by("time", "id") + .first() + ) + if schedule is None: + return None + + course_content = schedule.delivery.course_content + return NextDeliveryResponse( + delivery_schedule_id=schedule.id, + course_content_id=course_content.id, + course_content_title=course_content.title, + course_content_type=course_content.type, + scheduled_at=schedule.time, + ) + + class EnrollmentResponse(BaseModel): id: int learner: LearnerResponse course: CourseSummaryResponse status: EnrollmentStatus events: list[Event] + next_delivery: NextDeliveryResponse | None = None @staticmethod def from_django_model(enrollment: Enrollment) -> "EnrollmentResponse": @@ -267,6 +307,7 @@ def from_django_model(enrollment: Enrollment) -> "EnrollmentResponse": "course": enrollment.course, "status": enrollment.status, "events": events, + "next_delivery": _next_delivery(enrollment), } ) diff --git a/django_email_learning/platform/api/urls.py b/django_email_learning/platform/api/urls.py index 1cc5a1e4..676843b4 100644 --- a/django_email_learning/platform/api/urls.py +++ b/django_email_learning/platform/api/urls.py @@ -23,6 +23,7 @@ OrganizationsView, OrganizationUsersView, ReorderCourseContentView, + SendDeliveryScheduleNowView, SendLessonToPlatformUser, SendoutView, SingleApiKeyView, @@ -146,6 +147,12 @@ EnrollmentView.as_view(), name="enrollments_detail", ), + path( + "organizations//enrollments//" + "delivery-schedules//send/", + SendDeliveryScheduleNowView.as_view(), + name="delivery_schedule_send_now", + ), path( "organizations//courses//enrollments/", EnrollmentsView.as_view(), diff --git a/django_email_learning/platform/api/views/__init__.py b/django_email_learning/platform/api/views/__init__.py index 33423d5b..f60f0f42 100644 --- a/django_email_learning/platform/api/views/__init__.py +++ b/django_email_learning/platform/api/views/__init__.py @@ -18,6 +18,7 @@ EnrollmentsView, EnrollmentView, LearnersView, + SendDeliveryScheduleNowView, SingleLearnerView, ) from django_email_learning.platform.api.views.misc import ( @@ -84,6 +85,7 @@ "EnrollmentsView", "EnrollmentView", "EnrollmentsStatisticsView", + "SendDeliveryScheduleNowView", "NewsletterView", "NewsletterEmbedSnippetView", "SingleNewsletterView", diff --git a/django_email_learning/platform/api/views/learners.py b/django_email_learning/platform/api/views/learners.py index 45cfe7d7..11370ad8 100644 --- a/django_email_learning/platform/api/views/learners.py +++ b/django_email_learning/platform/api/views/learners.py @@ -18,6 +18,7 @@ from django_email_learning.models import ( Certificate, Course, + DeliverySchedule, Enrollment, EnrollmentStatus, Learner, @@ -37,6 +38,10 @@ from django_email_learning.services.command_models.verify_enrollment_command import ( VerifyEnrollmentCommand, ) +from django_email_learning.services.manual_delivery_service import ( + ManualDeliveryOutcome, + send_delivery_schedule_now, +) logger = logging.getLogger(__name__) @@ -197,6 +202,55 @@ def get(self, request, *args, **kwargs) -> JsonResponse: # type: ignore[no-unty return JsonResponse({"error": e.json()}, status=400) +@method_decorator(accessible_for(roles={"admin"}), name="post") +class SendDeliveryScheduleNowView(View): + """Sends one scheduled content delivery immediately. + + Admin-only: it puts an email in a learner's inbox and advances their + enrollment, which is a heavier action than the read access the rest of the + enrollment views grant. Changing the schedule's time would not do the same + job - the delivery job runs on a cron, so the content would still go out + whenever that next fires. + """ + + def post(self, request, *args, **kwargs) -> JsonResponse: # type: ignore[no-untyped-def] + try: + enrollment = Enrollment.objects.get( + id=kwargs["enrollment_id"], course__organization_id=kwargs["organization_id"] + ) + except Enrollment.DoesNotExist: + return JsonResponse({"error": "Enrollment not found"}, status=404) + + try: + delivery_schedule = DeliverySchedule.objects.get( + id=kwargs["delivery_schedule_id"], + delivery__enrollment=enrollment, + ) + except DeliverySchedule.DoesNotExist: + return JsonResponse({"error": "Delivery schedule not found"}, status=404) + + result = send_delivery_schedule_now(delivery_schedule) + + if result.outcome == ManualDeliveryOutcome.NOT_SCHEDULED: + return JsonResponse( + {"error": "Delivery is no longer scheduled", "delivery_status": result.delivery_status}, + status=409, + ) + if result.outcome == ManualDeliveryOutcome.FAILED: + logger.error( + f"Manual send of DeliverySchedule {delivery_schedule.id} failed with status {result.delivery_status}." + ) + return JsonResponse( + {"error": "Delivery failed", "delivery_status": result.delivery_status}, + status=500, + ) + + return JsonResponse( + {"status": result.delivery_status, "delivery_schedule_id": delivery_schedule.id}, + status=200, + ) + + @method_decorator(accessible_for(roles={"admin", "editor", "instructor", "viewer"}), name="get") class EnrollmentsStatisticsView(View): def get(self, request, *args, **kwargs) -> JsonResponse: # type: ignore[no-untyped-def] diff --git a/django_email_learning/platform/views/learners.py b/django_email_learning/platform/views/learners.py index 02745737..3f1ca11b 100644 --- a/django_email_learning/platform/views/learners.py +++ b/django_email_learning/platform/views/learners.py @@ -65,4 +65,10 @@ def get_locale_messages(self) -> Dict[str, str]: "reset_filters": _("Reset Filters"), "progress": _("Progress"), "no_learners_found": _("No learners found."), + "next_delivery": _("Next delivery"), + "send_now": _("send now"), + "sending": _("Sending..."), + "content_sent_successfully": _("Content sent."), + "content_send_failed": _("The content could not be sent."), + "delivery_no_longer_scheduled": _("This delivery is no longer scheduled."), } diff --git a/django_email_learning/services/manual_delivery_service.py b/django_email_learning/services/manual_delivery_service.py new file mode 100644 index 00000000..a9b3bf59 --- /dev/null +++ b/django_email_learning/services/manual_delivery_service.py @@ -0,0 +1,120 @@ +"""Runs a single scheduled content delivery immediately, outside the job. + +The delivery job claims whatever is due and processes it, which means an +operator who wants one learner to receive their next content *now* has no +lever other than editing the schedule's time and waiting for the next job +run - and the job runs on a cron, so "now" is really "some time within the +next interval". This module does the same work the job does for exactly one +`DeliverySchedule`, so the email goes out, the schedule is marked delivered, +and the follow-up scheduling (next content, or graduation) happens in the +same request. + +The row is claimed with the same `SCHEDULED -> PROCESSING` compare-and-set the +database queue uses, so a job run happening at the same moment cannot pick up +the same schedule and send the content twice. +""" + +import logging +from dataclasses import dataclass +from enum import StrEnum + +from django.utils import timezone + +from django_email_learning.jobs.deliver_contents_job import DeliverContentsJob +from django_email_learning.models import DeliverySchedule, DeliveryStatus +from django_email_learning.ports.task_queue_protocol import TaskQueueProtocol +from django_email_learning.services.metrics_service import metric_service + +logger = logging.getLogger(__name__) + + +class ManualDeliveryOutcome(StrEnum): + DELIVERED = "delivered" + NOT_SCHEDULED = "not_scheduled" + FAILED = "failed" + + +@dataclass(frozen=True) +class ManualDeliveryResult: + outcome: ManualDeliveryOutcome + delivery_status: str + + +class _EmptyDeliveryQueue(TaskQueueProtocol[DeliverySchedule]): + """A queue that never yields work. + + `DeliverContentsJob` builds its queue on construction, and the database + queue claims a batch of due schedules the moment it is built. Sending one + delivery must not take rows away from a running job, so the job instance + used here is given a queue with nothing in it: only `process_delivery` is + called, and that takes its schedule as an argument. + """ + + def next_task(self) -> DeliverySchedule | None: + return None + + +def send_delivery_schedule_now(delivery_schedule: DeliverySchedule) -> ManualDeliveryResult: + """Deliver `delivery_schedule` as if the job had just picked it up. + + Returns the outcome together with the schedule's resulting status. A + schedule that is not `SCHEDULED` when the claim runs is left untouched and + reported as `NOT_SCHEDULED` - it is already delivered, canceled, blocked, + or in the hands of a running job. + """ + claimed = DeliverySchedule.objects.filter(id=delivery_schedule.id, status=DeliveryStatus.SCHEDULED).update( + status=DeliveryStatus.PROCESSING, + # The delivery is happening now, so the schedule should say so: it keeps + # `delivered_at` consistent with `time`, and a retry after a failed + # attempt is then measured from now rather than from a future date. + time=timezone.now(), + ) + if not claimed: + delivery_schedule.refresh_from_db() + logger.info( + f"Manual delivery skipped for DeliverySchedule ID {delivery_schedule.id}: " + f"status is {delivery_schedule.status}, not {DeliveryStatus.SCHEDULED}." + ) + return ManualDeliveryResult( + outcome=ManualDeliveryOutcome.NOT_SCHEDULED, + delivery_status=delivery_schedule.status, + ) + + claimed_schedule = DeliverySchedule.objects.select_related( + "delivery__enrollment__learner", + "delivery__course_content__course__organization", + "delivery__course_content__course__imap_connection", + "delivery__course_content__lesson", + "delivery__course_content__quiz", + "delivery__course_content__assignment", + ).get(id=delivery_schedule.id) + + job = DeliverContentsJob(delivery_queue=_EmptyDeliveryQueue()) + try: + job.process_delivery(claimed_schedule) + except Exception as e: + job.block_delivery(claimed_schedule, e) + return ManualDeliveryResult(outcome=ManualDeliveryOutcome.FAILED, delivery_status=DeliveryStatus.BLOCKED) + + claimed_schedule.refresh_from_db() + if claimed_schedule.status == DeliveryStatus.PROCESSING: + # `process_delivery` recognised no content to send - a content row whose + # type has no matching lesson/quiz/assignment. Leaving it PROCESSING + # would hide it from the job forever, so hand it back to the schedule. + logger.error( + f"Manual delivery for DeliverySchedule ID {claimed_schedule.id} produced no delivery. " + f"Returning it to {DeliveryStatus.SCHEDULED}." + ) + claimed_schedule.status = DeliveryStatus.SCHEDULED + claimed_schedule.save() + metric_service.delivery_schedule_blocked(claimed_schedule.delivery.course_content.id) + return ManualDeliveryResult(outcome=ManualDeliveryOutcome.FAILED, delivery_status=claimed_schedule.status) + + if claimed_schedule.status != DeliveryStatus.DELIVERED: + logger.warning( + f"Manual delivery for DeliverySchedule ID {claimed_schedule.id} ended as {claimed_schedule.status}." + ) + return ManualDeliveryResult(outcome=ManualDeliveryOutcome.FAILED, delivery_status=claimed_schedule.status) + + logger.info(f"Manual delivery completed for DeliverySchedule ID {claimed_schedule.id}.") + return ManualDeliveryResult(outcome=ManualDeliveryOutcome.DELIVERED, delivery_status=claimed_schedule.status) diff --git a/docs/source/platform/learners.rst b/docs/source/platform/learners.rst index 6586eb20..fd020c30 100644 --- a/docs/source/platform/learners.rst +++ b/docs/source/platform/learners.rst @@ -35,6 +35,21 @@ Clicking on any learner's email address opens a detailed view providing comprehe :align: center +Sending the Next Content Immediately +------------------------------------ + +Under the course title, the enrollment view shows the learner's next scheduled content delivery — when it is due and which content it is. Organization admins also get a **send now** link next to it. + +**send now** delivers that one content straight away, exactly as the delivery job would have when its time arrived: the email is sent, the delivery is recorded against the enrollment, and the course moves on — the next content is scheduled, or the enrollment is completed if that was the last one. Simply editing the schedule's time would not have the same effect, since the delivery job runs on a schedule of its own and the content would still wait for its next run. + +A few things worth knowing: + +- Only deliveries that are still **scheduled** can be sent this way. A delivery that has already gone out, was canceled, or is blocked shows no link. +- If the delivery job happens to pick up the same delivery at that moment, only one of the two sends it — the other reports that the delivery is no longer scheduled. +- If sending fails, the delivery is retried or blocked by the same rules that apply during a job run, and the dialog reports that the content could not be sent. +- The action is restricted to organization admins. Other roles see the scheduled delivery but no link. + + Learner Capacity ----------------- diff --git a/frontend/platform/learners/Learners.jsx b/frontend/platform/learners/Learners.jsx index 04276d98..3bdaf62b 100644 --- a/frontend/platform/learners/Learners.jsx +++ b/frontend/platform/learners/Learners.jsx @@ -24,6 +24,7 @@ import apiClient from '../../src/apiClient.js'; import { sanitizeEndpointUrl, sanitizeImageUrl } from '../../src/sanitizeUrl.js'; const EnrollentList = lazy(() => import("./components/EnrollmentList.jsx")); +const NextDelivery = lazy(() => import("./components/NextDelivery.jsx")); const ENROLLMENT_STATUSES = ['active', 'completed', 'deactivated', 'canceled', 'inactive']; @@ -31,7 +32,7 @@ const ENROLLMENT_STATUSES = ['active', 'completed', 'deactivated', 'canceled', ' function Learners() { const [organizationId, setOrganizationId] = useState(null); - const { localeMessages, direction, apiBaseUrl: rawApiBaseUrl } = useAppContext(); + const { localeMessages, direction, userRole, apiBaseUrl: rawApiBaseUrl } = useAppContext(); const apiBaseUrl = sanitizeEndpointUrl(rawApiBaseUrl); const [learners, setLearners] = useState([]); const searcchInputRef = useRef(null); @@ -89,6 +90,16 @@ function Learners() { {data.learner.email} {data.course.title} + {data.next_delivery && ( + + showEnrollmentStatus(enrollmentId)} + /> + + )} diff --git a/frontend/platform/learners/components/NextDelivery.jsx b/frontend/platform/learners/components/NextDelivery.jsx new file mode 100644 index 00000000..f5336957 --- /dev/null +++ b/frontend/platform/learners/components/NextDelivery.jsx @@ -0,0 +1,81 @@ +import { Box, CircularProgress, Link, Typography } from '@mui/material'; +import SendIcon from '@mui/icons-material/Send'; +import { useState } from 'react'; +import { useAppContext } from '../../../src/render.jsx'; +import apiClient from '../../../src/apiClient.js'; + +/** + * The enrollment's next scheduled content delivery, with an admin-only + * "send now" link that delivers it immediately instead of waiting for the + * delivery job's next run. + * + * `onSent` is called after a successful send so the caller can reload the + * enrollment — the timeline gains a "content sent" event and the next + * delivery moves on to whatever was scheduled after it. + */ +function NextDelivery({ nextDelivery, sendUrl, canSend, onSent }) { + const { localeMessages } = useAppContext(); + const [sending, setSending] = useState(false); + const [error, setError] = useState(null); + + if (!nextDelivery) { + return null; + } + + const scheduledAt = String(nextDelivery.scheduled_at || '').replace('T', ' ').replace('Z', ''); + + const sendNow = () => { + setSending(true); + setError(null); + apiClient.post(sendUrl, {}) + .then(() => { + if (onSent) onSent(); + }) + .catch((apiError) => { + console.error('Error sending content delivery:', apiError); + setError(apiError.status === 409 + ? (localeMessages['delivery_no_longer_scheduled'] || 'This delivery is no longer scheduled.') + : (localeMessages['content_send_failed'] || 'The content could not be sent.')); + setSending(false); + }); + }; + + return ( + + + + {localeMessages['next_delivery'] || 'Next delivery'}: {scheduledAt} — {nextDelivery.course_content_title} + + {canSend && ( + + ( + {sending ? ( + + + {localeMessages['sending'] || 'Sending...'} + + ) : ( + + + {localeMessages['send_now'] || 'send now'} + + )} + ) + + )} + + {error && ( + {error} + )} + + ); +} + +export default NextDelivery; diff --git a/frontend/src/test/platform/NextDelivery.test.jsx b/frontend/src/test/platform/NextDelivery.test.jsx new file mode 100644 index 00000000..6a43597c --- /dev/null +++ b/frontend/src/test/platform/NextDelivery.test.jsx @@ -0,0 +1,106 @@ +import { describe, it, expect, vi } from 'vitest'; +import { screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { renderWithProviders } from '../test-utils'; +import NextDelivery from '../../../platform/learners/components/NextDelivery'; + +vi.mock('../../render.jsx'); + +const localeMessages = { + next_delivery: 'Next delivery', + send_now: 'send now', + sending: 'Sending...', + content_send_failed: 'The content could not be sent.', + delivery_no_longer_scheduled: 'This delivery is no longer scheduled.', +}; + +const nextDelivery = { + delivery_schedule_id: 7, + course_content_id: 3, + course_content_title: 'Lesson Two', + course_content_type: 'lesson', + scheduled_at: '2026-08-20T09:00:00Z', +}; + +const sendUrl = '/api/organizations/1/enrollments/5/delivery-schedules/7/send/'; + +function renderComponent(props = {}) { + return renderWithProviders( + , + { appContext: { localeMessages } } + ); +} + +describe('NextDelivery', () => { + it('renders nothing when there is no scheduled delivery', () => { + const { container } = renderComponent({ nextDelivery: null }); + expect(container).toBeEmptyDOMElement(); + }); + + it('shows the scheduled time and the content title', () => { + renderComponent(); + expect(screen.getByText(/Next delivery: 2026-08-20 09:00:00 — Lesson Two/)).toBeInTheDocument(); + }); + + it('shows the send now link when the user may send', () => { + renderComponent(); + expect(screen.getByRole('button', { name: /send now/i })).toBeInTheDocument(); + }); + + it('hides the send now link when the user may not send', () => { + renderComponent({ canSend: false }); + expect(screen.queryByRole('button', { name: /send now/i })).not.toBeInTheDocument(); + expect(screen.getByText(/Next delivery: 2026-08-20 09:00:00 — Lesson Two/)).toBeInTheDocument(); + }); + + it('posts to the send endpoint and notifies the caller', async () => { + const user = userEvent.setup(); + const onSent = vi.fn(); + renderComponent({ onSent }); + + await user.click(screen.getByRole('button', { name: /send now/i })); + + await waitFor(() => expect(onSent).toHaveBeenCalled()); + const [url, options] = global.fetch.mock.calls[0]; + expect(url).toBe(sendUrl); + expect(options.method).toBe('POST'); + }); + + it('shows an error when the delivery is no longer scheduled', async () => { + const user = userEvent.setup(); + const onSent = vi.fn(); + global.fetch.mockResolvedValue({ + ok: false, + status: 409, + json: () => Promise.resolve({ error: 'Delivery is no longer scheduled' }), + }); + renderComponent({ onSent }); + + await user.click(screen.getByRole('button', { name: /send now/i })); + + expect(await screen.findByText('This delivery is no longer scheduled.')).toBeInTheDocument(); + expect(onSent).not.toHaveBeenCalled(); + // The link comes back so the admin can retry once they know what happened. + expect(screen.getByRole('button', { name: /send now/i })).toBeInTheDocument(); + }); + + it('shows an error when sending fails', async () => { + const user = userEvent.setup(); + global.fetch.mockResolvedValue({ + ok: false, + status: 500, + json: () => Promise.resolve({ error: 'Delivery failed' }), + }); + renderComponent(); + + await user.click(screen.getByRole('button', { name: /send now/i })); + + expect(await screen.findByText('The content could not be sent.')).toBeInTheDocument(); + }); +}); diff --git a/tests/platform/api/test_views/test_enrollment_api.py b/tests/platform/api/test_views/test_enrollment_api.py index 8b513e9b..81b93dde 100644 --- a/tests/platform/api/test_views/test_enrollment_api.py +++ b/tests/platform/api/test_views/test_enrollment_api.py @@ -1,6 +1,18 @@ +from datetime import timedelta + from django.urls import reverse +from django.utils import timezone -from django_email_learning.models import Course, Enrollment, EnrollmentStatus, Learner, Organization +from django_email_learning.models import ( + ContentDelivery, + Course, + DeliverySchedule, + DeliveryStatus, + Enrollment, + EnrollmentStatus, + Learner, + Organization, +) def get_url(enrollment_id): @@ -59,9 +71,53 @@ def test_enrollment_api_cross_organization_returns_404(viewer_client): assert response.status_code == 404 -def test_enrollment_api_email_opened_event(viewer_client, content_delivery): - from django.utils import timezone +def test_enrollment_api_returns_the_next_scheduled_delivery(viewer_client, content_delivery, course_lesson_content): + delivery = ContentDelivery.objects.create( + enrollment=content_delivery.enrollment, + course_content=course_lesson_content, + ) + schedule = DeliverySchedule.objects.create(delivery=delivery, time=timezone.now() + timedelta(days=1)) + + response = viewer_client.get(get_url(enrollment_id=content_delivery.enrollment.id)) + + assert response.status_code == 200 + next_delivery = response.json()["next_delivery"] + assert next_delivery["delivery_schedule_id"] == schedule.id + assert next_delivery["course_content_id"] == course_lesson_content.id + assert next_delivery["course_content_title"] == course_lesson_content.title + assert next_delivery["course_content_type"] == course_lesson_content.type + assert next_delivery["scheduled_at"] is not None + + +def test_enrollment_api_next_delivery_is_the_earliest_scheduled_one( + viewer_client, content_delivery, course_lesson_content, course_assignment_content +): + later = ContentDelivery.objects.create( + enrollment=content_delivery.enrollment, + course_content=course_assignment_content, + ) + DeliverySchedule.objects.create(delivery=later, time=timezone.now() + timedelta(days=5)) + sooner = ContentDelivery.objects.create( + enrollment=content_delivery.enrollment, + course_content=course_lesson_content, + ) + sooner_schedule = DeliverySchedule.objects.create(delivery=sooner, time=timezone.now() + timedelta(days=1)) + response = viewer_client.get(get_url(enrollment_id=content_delivery.enrollment.id)) + + assert response.json()["next_delivery"]["delivery_schedule_id"] == sooner_schedule.id + + +def test_enrollment_api_next_delivery_is_null_without_a_scheduled_delivery(viewer_client, content_delivery): + assert not content_delivery.delivery_schedules.filter(status=DeliveryStatus.SCHEDULED).exists() + + response = viewer_client.get(get_url(enrollment_id=content_delivery.enrollment.id)) + + assert response.status_code == 200 + assert response.json()["next_delivery"] is None + + +def test_enrollment_api_email_opened_event(viewer_client, content_delivery): content_delivery.enrollment.status = EnrollmentStatus.ACTIVE content_delivery.enrollment.save() content_delivery.course_content.is_published = True diff --git a/tests/platform/api/test_views/test_send_delivery_now_view.py b/tests/platform/api/test_views/test_send_delivery_now_view.py new file mode 100644 index 00000000..8d010ede --- /dev/null +++ b/tests/platform/api/test_views/test_send_delivery_now_view.py @@ -0,0 +1,165 @@ +from datetime import timedelta +from unittest.mock import patch + +import pytest +from django.urls import reverse +from django.utils import timezone + +from django_email_learning.jobs.deliver_contents_job import SendLessonCommand +from django_email_learning.models import ( + ContentDelivery, + Course, + DeliverySchedule, + DeliveryStatus, + Enrollment, + EnrollmentStatus, + Learner, + Organization, +) + + +def get_url(enrollment_id, delivery_schedule_id, organization_id=1): + return reverse( + "django_email_learning:api_platform:delivery_schedule_send_now", + kwargs={ + "organization_id": organization_id, + "enrollment_id": enrollment_id, + "delivery_schedule_id": delivery_schedule_id, + }, + ) + + +@pytest.fixture +def scheduled_lesson_delivery(db, active_enrollment, course_lesson_content): + """A lesson scheduled a day from now — the case "send now" exists for.""" + delivery = ContentDelivery.objects.create( + enrollment=active_enrollment, + course_content=course_lesson_content, + ) + return DeliverySchedule.objects.create( + delivery=delivery, + time=timezone.now() + timedelta(days=1), + ) + + +def test_send_now_requires_authentication(anonymous_client, scheduled_lesson_delivery): + url = get_url(scheduled_lesson_delivery.delivery.enrollment.id, scheduled_lesson_delivery.id) + response = anonymous_client.post(url) + assert response.status_code == 401 + + +@pytest.mark.parametrize("client", ["viewer", "editor", "instructor"], indirect=True) +def test_send_now_is_forbidden_for_non_admins(client, scheduled_lesson_delivery): + url = get_url(scheduled_lesson_delivery.delivery.enrollment.id, scheduled_lesson_delivery.id) + response = client.post(url) + assert response.status_code == 403 + scheduled_lesson_delivery.refresh_from_db() + assert scheduled_lesson_delivery.status == DeliveryStatus.SCHEDULED + + +@pytest.mark.parametrize("client", ["org_admin", "superadmin"], indirect=True) +def test_send_now_delivers_the_scheduled_content(client, scheduled_lesson_delivery): + url = get_url(scheduled_lesson_delivery.delivery.enrollment.id, scheduled_lesson_delivery.id) + response = client.post(url) + + assert response.status_code == 200 + assert response.json()["status"] == DeliveryStatus.DELIVERED + assert response.json()["delivery_schedule_id"] == scheduled_lesson_delivery.id + + scheduled_lesson_delivery.refresh_from_db() + assert scheduled_lesson_delivery.status == DeliveryStatus.DELIVERED + assert scheduled_lesson_delivery.delivered_at is not None + # The schedule now says the delivery happened when it actually happened, + # rather than keeping the future time it was originally due at. + assert scheduled_lesson_delivery.time <= timezone.now() + + +def test_send_now_graduates_the_enrollment_after_the_last_content(org_admin_client, scheduled_lesson_delivery): + """The follow-up work the job does must happen too, not just the email.""" + enrollment = scheduled_lesson_delivery.delivery.enrollment + url = get_url(enrollment.id, scheduled_lesson_delivery.id) + + response = org_admin_client.post(url) + + assert response.status_code == 200 + enrollment.refresh_from_db() + assert enrollment.status == EnrollmentStatus.COMPLETED + + +def test_send_now_schedules_the_next_content( + org_admin_client, scheduled_lesson_delivery, course_quiz_content, quiz_with_questions +): + course_quiz_content.quiz = quiz_with_questions + course_quiz_content.is_published = True + course_quiz_content.save() + enrollment = scheduled_lesson_delivery.delivery.enrollment + url = get_url(enrollment.id, scheduled_lesson_delivery.id) + + response = org_admin_client.post(url) + + assert response.status_code == 200 + assert ContentDelivery.objects.filter(enrollment=enrollment, course_content=course_quiz_content).exists() + enrollment.refresh_from_db() + assert enrollment.status == EnrollmentStatus.ACTIVE + + +def test_send_now_returns_409_when_the_delivery_is_not_scheduled(org_admin_client, scheduled_lesson_delivery): + scheduled_lesson_delivery.status = DeliveryStatus.DELIVERED + scheduled_lesson_delivery.save() + url = get_url(scheduled_lesson_delivery.delivery.enrollment.id, scheduled_lesson_delivery.id) + + response = org_admin_client.post(url) + + assert response.status_code == 409 + assert response.json()["delivery_status"] == DeliveryStatus.DELIVERED + + +def test_send_now_returns_500_when_sending_fails(org_admin_client, scheduled_lesson_delivery): + url = get_url(scheduled_lesson_delivery.delivery.enrollment.id, scheduled_lesson_delivery.id) + + with patch.object(SendLessonCommand, "execute", side_effect=Exception("Simulated sending failure")): + response = org_admin_client.post(url) + + assert response.status_code == 500 + scheduled_lesson_delivery.refresh_from_db() + # A failed attempt is rescheduled by the job's own retry logic, exactly as + # it would be during a job run. + assert scheduled_lesson_delivery.status == DeliveryStatus.SCHEDULED + assert scheduled_lesson_delivery.failed_attempts == 1 + + +def test_send_now_returns_404_for_unknown_delivery_schedule(org_admin_client, scheduled_lesson_delivery): + url = get_url(scheduled_lesson_delivery.delivery.enrollment.id, scheduled_lesson_delivery.id + 1000) + assert org_admin_client.post(url).status_code == 404 + + +def test_send_now_returns_404_for_a_schedule_of_another_enrollment(org_admin_client, scheduled_lesson_delivery, course): + other_learner = Learner.objects.create(email="other-learner@example.com", organization_id=1) + other_enrollment = Enrollment.objects.create(learner=other_learner, course=course, status=EnrollmentStatus.ACTIVE) + + url = get_url(other_enrollment.id, scheduled_lesson_delivery.id) + + assert org_admin_client.post(url).status_code == 404 + scheduled_lesson_delivery.refresh_from_db() + assert scheduled_lesson_delivery.status == DeliveryStatus.SCHEDULED + + +def test_send_now_returns_404_for_an_enrollment_of_another_organization(org_admin_client, course_lesson_content): + other_org = Organization.objects.create(pk=2, name="Other Organization") + other_course = Course.objects.create( + title="Other Org Course", + slug="other-org-course", + organization=other_org, + ) + other_learner = Learner.objects.create(email="other-org-learner@example.com", organization=other_org) + other_enrollment = Enrollment.objects.create( + learner=other_learner, course=other_course, status=EnrollmentStatus.ACTIVE + ) + delivery = ContentDelivery.objects.create(enrollment=other_enrollment, course_content=course_lesson_content) + schedule = DeliverySchedule.objects.create(delivery=delivery) + + # Asked for under organization 1, which the admin does administer — the + # enrollment belongs to organization 2, so it must not be reachable. + assert org_admin_client.post(get_url(other_enrollment.id, schedule.id)).status_code == 404 + schedule.refresh_from_db() + assert schedule.status == DeliveryStatus.SCHEDULED diff --git a/tests/services/test_manual_delivery_service.py b/tests/services/test_manual_delivery_service.py new file mode 100644 index 00000000..c2447a60 --- /dev/null +++ b/tests/services/test_manual_delivery_service.py @@ -0,0 +1,149 @@ +from datetime import timedelta +from unittest.mock import patch + +import pytest +from django.utils import timezone + +import django_email_learning.services.manual_delivery_service as manual_delivery_service_module +from django_email_learning.models import ( + ContentDelivery, + CourseContent, + DeliverySchedule, + DeliveryStatus, + Lesson, +) +from django_email_learning.services.manual_delivery_service import ( + ManualDeliveryOutcome, + send_delivery_schedule_now, +) + + +@pytest.fixture +def scheduled_lesson_delivery(db, active_enrollment, course_lesson_content): + delivery = ContentDelivery.objects.create( + enrollment=active_enrollment, + course_content=course_lesson_content, + ) + return DeliverySchedule.objects.create( + delivery=delivery, + time=timezone.now() + timedelta(days=2), + ) + + +def test_delivers_a_scheduled_delivery(scheduled_lesson_delivery): + result = send_delivery_schedule_now(scheduled_lesson_delivery) + + assert result.outcome == ManualDeliveryOutcome.DELIVERED + assert result.delivery_status == DeliveryStatus.DELIVERED + + +def test_does_not_claim_work_from_the_running_job(db, active_enrollment, course_lesson_content): + """Constructing the job must not pull due schedules out of the queue. + + The database queue claims a batch the moment it is built, so a manual send + that built one would mark unrelated due deliveries as PROCESSING and never + process them. + """ + other_delivery = ContentDelivery.objects.create( + enrollment=active_enrollment, + course_content=course_lesson_content, + ) + due_elsewhere = DeliverySchedule.objects.create( + delivery=other_delivery, + time=timezone.now() - timedelta(minutes=5), + ) + target_delivery = ContentDelivery.objects.create( + enrollment=active_enrollment, + course_content=CourseContent.objects.create( + course=course_lesson_content.course, + priority=10, + type="lesson", + lesson=Lesson.objects.create(title="Second Lesson", content="Second lesson content"), + waiting_period=3600, + is_published=True, + ), + ) + target = DeliverySchedule.objects.create(delivery=target_delivery, time=timezone.now() + timedelta(days=1)) + + send_delivery_schedule_now(target) + + due_elsewhere.refresh_from_db() + assert due_elsewhere.status == DeliveryStatus.SCHEDULED + + +@pytest.mark.parametrize( + "status", + [DeliveryStatus.DELIVERED, DeliveryStatus.PROCESSING, DeliveryStatus.CANCELED, DeliveryStatus.BLOCKED], +) +def test_leaves_a_delivery_that_is_not_scheduled_alone(scheduled_lesson_delivery, status): + scheduled_lesson_delivery.status = status + scheduled_lesson_delivery.save() + original_time = scheduled_lesson_delivery.time + + result = send_delivery_schedule_now(scheduled_lesson_delivery) + + assert result.outcome == ManualDeliveryOutcome.NOT_SCHEDULED + assert result.delivery_status == status + scheduled_lesson_delivery.refresh_from_db() + assert scheduled_lesson_delivery.status == status + assert scheduled_lesson_delivery.time == original_time + + +def test_blocks_the_delivery_when_processing_raises(scheduled_lesson_delivery): + with ( + patch.object( + manual_delivery_service_module.DeliverContentsJob, + "process_delivery", + side_effect=Exception("Simulated processing failure"), + ), + patch.object(manual_delivery_service_module.metric_service, "delivery_schedule_blocked"), + ): + result = send_delivery_schedule_now(scheduled_lesson_delivery) + + assert result.outcome == ManualDeliveryOutcome.FAILED + assert result.delivery_status == DeliveryStatus.BLOCKED + scheduled_lesson_delivery.refresh_from_db() + assert scheduled_lesson_delivery.status == DeliveryStatus.BLOCKED + + +def test_returns_an_unsendable_delivery_to_scheduled(db, active_enrollment, course_assignment_content): + """A content row with nothing to send must not be left PROCESSING. + + `process_delivery` recognises no content in that case and returns without + touching the status, which would hide the schedule from the job forever. + Model validation rejects an assignment content with no assignment, so the + row is broken with a queryset update, the way a stray data change would. + """ + course_assignment_content.is_published = True + course_assignment_content.save() + CourseContent.objects.filter(id=course_assignment_content.id).update(assignment=None) + content_without_assignment = CourseContent.objects.get(id=course_assignment_content.id) + delivery = ContentDelivery.objects.create( + enrollment=active_enrollment, + course_content=content_without_assignment, + ) + schedule = DeliverySchedule.objects.create(delivery=delivery) + + with patch.object(manual_delivery_service_module.metric_service, "delivery_schedule_blocked") as blocked_metric: + result = send_delivery_schedule_now(schedule) + + assert result.outcome == ManualDeliveryOutcome.FAILED + assert result.delivery_status == DeliveryStatus.SCHEDULED + schedule.refresh_from_db() + assert schedule.status == DeliveryStatus.SCHEDULED + blocked_metric.assert_called_once_with(content_without_assignment.id) + + +def test_cancels_a_delivery_whose_content_is_unpublished(db, active_enrollment, course_lesson_content): + course_lesson_content.is_published = False + course_lesson_content.save() + delivery = ContentDelivery.objects.create( + enrollment=active_enrollment, + course_content=course_lesson_content, + ) + schedule = DeliverySchedule.objects.create(delivery=delivery) + + result = send_delivery_schedule_now(schedule) + + assert result.outcome == ManualDeliveryOutcome.FAILED + assert result.delivery_status == DeliveryStatus.CANCELED