Guard course_welcome email against re-welcoming students with course history by donnapep · Pull Request #8046 · Automattic/sensei

donnapep · 2026-06-30T15:48:41Z

Fixes Automattic/sensei-pro#2633

Problem

The course_welcome email fires on the not-enrolled → enrolled transition. When the enrolment provider set changes (e.g. de/reactivating WC Paid Courses), Sensei runs a site-wide enrolment recalculation that un-enrols and re-enrols provider-only students — re-firing the transition and resending the welcome, in bulk on paid sites.

Change

Send at most once. After sending, record a per-student/per-course flag in user meta and skip if already set. Key is blog-prefixed (per-site on multisite), matching enrolment-results storage.
Backfill. One-time upgrade job marks existing student/course relationships as already welcomed, via the resolved course-progress repository — works with HPPS on or off.
History guard (fallback). Still skips students who completed the course or started a lesson, covering existing students until the backfill flags them.
Uninstall cleans up the new meta. New enrollees (no flag, no history) still get the first welcome.

Test plan

Requires a split install: Sensei LMS and Sensei Pro (WC Paid Courses) active as separate plugins, plus WooCommerce (deactivating WC Paid Courses must leave Sensei LMS active — on the all-in-one bundle it deactivates Sensei too and does not reproduce).

Setup: WooCommerce active; Sensei LMS active on its own; WC Paid Courses active separately; Welcome email enabled (Sensei LMS → Settings → Emails); a paid course linked to a product; a learner enrolled via a completed order for that product. Use Mailpit.
Deactivate WC Paid Courses only.
Tools → Scheduled Actions, search sensei_calculate_learner_enrolments; wait for a recent one under Complete.
Reactivate WC Paid Courses.
Tools → Scheduled Actions again; wait for another sensei_calculate_learner_enrolments Complete after reactivation.
Check email: pre-fix the learner gets the "Welcome to " email again; with fix they do not.
New enrollee in a published course still receives the welcome once.

Known limitation

The backfill is async. Until it finishes after upgrade, a provider change that re-fires a transition could re-welcome a not-yet-processed existing student. Bounded by backfill duration; engaged students still covered by the history guard.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Adds PHPUnit coverage to capture the currently-broken behavior where the course_welcome email is re-sent when an enrolment transition re-fires for a student who already has progress/completion history in the course. This is a tests-only PR intended to go red until the production guard is implemented.

Changes:

Add a failing test asserting no welcome email is sent for a student who already completed the course.
Add a failing test asserting no welcome email is sent for a student who already completed a lesson within the course.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

github-actions · 2026-06-30T15:58:35Z

WordPress Playground Preview

The changes in this pull request can previewed and tested using a WordPress Playground instance.

Copilot

Pull request overview

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

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Replaces the per-lesson get_completed_lesson_ids() loop with a single user_started_lesson_count() query, avoiding N+1 on large courses. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Persist a per-student/per-course "welcome sent" flag as user meta so the Course Welcome email is never sent twice when the enrolment status event re-fires (enrol/unenrol/re-enrol, recalculation). The flag is read and written independently of HPPS, so it behaves identically in every storage mode. Add an upgrade backfill job that marks all existing course-progress relationships as already welcomed (enumerated through the resolved course progress repository, so it works for both comments and custom-tables backends). This ensures existing students are never (re-)welcomed after upgrade. Register the new user meta for uninstall cleanup, and also clean up the previously-omitted sensei_email_unsubscribed_* user meta. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EDPuYvcqYgz4fHN44YxaWp

The upgrade gate compared against 4.27.0 while this ships as a 4.26.2 patch, so the backfill would re-enqueue on every upgrade until a 4.27.0 release. Retarget the method, gate, and tests to 4.26.2. The welcome-sent user meta was stored unprefixed, so on multisite the same course ID would collide across sites and suppress a legitimate welcome. Blog-prefix the key to match course enrolment results storage, and update the data cleaner pattern and tests accordingly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Enabling the welcome email does not send to existing students by itself; a later enrolment transition would. Reword the backfill docblock to match. Collapse the two course-welcome changelog entries into one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Drop unused closure params and use pre-increment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Avoids a new PossiblyNullArgument that pushed the file past its Psalm baseline occurrence count. Behavior-neutral: is_upgrade already implies a stored version. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

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

Test presence rather than truthiness so a pre-existing falsy value is never overwritten. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Drop the bespoke tables-mode test that manually created the schema and forced the tables repository. The backfill behaves identically across backends, so follow the grading-suite idiom: run the existing tests against comments normally and against tables under the ENABLE_HPPS run. Add maybe_enable_hpps_tables_repository()/maybe_reset_hpps_repository() wrappers to the HPPS helper trait so the is_hpps_tables_mode() guard isn't duplicated at each call site. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Follow testMethodName_Conditions_Expectation per the Sensei unit-test conventions (senseip2, 2022): drop the "When" prefix from the conditions segment for the tests added in this PR (enqueue, run_batch, and the Course_Welcome guard/flag tests). Pre-existing tests are left as-is. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

donnapep · 2026-07-03T16:13:35Z

@claude Please review.

claude · 2026-07-03T16:21:09Z

The overall approach is sound — meta-flag primary guard paired with the history-guard fallback and async backfill is a sensible layered design. A few things to address before merging:

Convention: short-array syntax in two test methods
Lines 229, 235, 239–244 and 262, 264, 269, 273–278 in test-class-course-welcome.php still use [ ] instead of array( ... ). The project's PHPCS config and AGENTS.md both call for the long form in new PHP code. The later tests in the same file (added in the same PR) already use array( ... ), so this is just a finish pass on the first two methods.

Consistency: has_welcome_email_been_sent uses a truthy check; the backfill uses metadata_exists

In class-course-welcome.php:

return (bool) get_user_meta( $student_id, self::get_welcome_sent_meta_key( $course_id ), true );

In the backfill:

if ( ! metadata_exists( 'user', $user_id, $meta_key ) ) {

Both check "does the flag exist", but metadata_exists is the canonical way to express that. If the key were ever stored with a falsy value (e.g. 0 or an empty string due to a bug or direct DB edit), get_user_meta would return false and allow a re-send while metadata_exists would still block. Low probability, but worth aligning — consider switching has_welcome_email_been_sent to use metadata_exists for parity.

Design note: flag set unconditionally after send_email_action
mark_welcome_email_sent is called regardless of whether email delivery succeeded (e.g. if wp_mail returns false, or if a sensei_email_send filter suppresses delivery downstream). This is consistent with Sensei's fire-and-forget action pattern, and the tradeoff (never re-welcome vs. possibly missing a first welcome) is acceptable for this bug. Worth capturing as an inline comment so future readers don't assume it was an oversight.

…lag-set Use metadata_exists in has_welcome_email_been_sent to match the backfill's existence check, so a flag stored with a falsy value still suppresses re-sends. Convert the two remaining short-array test methods to array() and add an inline note explaining why the sent flag is set regardless of delivery outcome. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

donnapep · 2026-07-03T18:05:57Z

Thanks @claude — all three addressed in f0d4f8f:

Short-array syntax — the two remaining test methods in test-class-course-welcome.php are now array( ... ).
metadata_exists parity — has_welcome_email_been_sent now uses metadata_exists( 'user', ... ) instead of a truthy get_user_meta check, so a flag stored with a falsy value still suppresses re-sends, matching the backfill.
Unconditional flag-set — added an inline comment explaining that the flag is set regardless of delivery outcome (fire-and-forget dispatch; we favour never re-welcoming over guaranteeing a single delivery).

claude · 2026-07-03T18:10:17Z

Confirmed all three look good:

Short-array syntax — both testWelcomeToCourseForStudent_StudentAlreadyCompletedCourse_DoesNotSendEmail and testWelcomeToCourseForStudent_StudentAlreadyStartedLessonInCourse_DoesNotSendEmail now use array( ... ) throughout.
metadata_exists parity — has_welcome_email_been_sent now uses metadata_exists( 'user', $student_id, ... ), consistent with the backfill.
Fire-and-forget comment — the rationale for the unconditional mark_welcome_email_sent call is clearly captured inline.

The rest of the diff also looks correct: the backfill guard against re-runs (metadata_exists before update_user_meta), the version check in v4_26_2_backfill_course_welcome_email_sent, the data-cleaner regex patterns (blog-prefixed for the sent flag, un-prefixed for the unsubscribe key), and the test coverage for the new backfill scheduling conditions. LGTM.

Copilot

Pull request overview

Copilot reviewed 13 out of 14 changed files in this pull request and generated no new comments.

Add failing tests for course_welcome re-send on existing students

9b953f0

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings June 30, 2026 15:48

Copilot started reviewing on behalf of donnapep June 30, 2026 15:49 View session

donnapep self-assigned this Jun 30, 2026

donnapep added this to the 4.26.2 milestone Jun 30, 2026

donnapep added the No Changelog label Jun 30, 2026

Copilot AI reviewed Jun 30, 2026

View reviewed changes

Comment thread tests/unit-tests/internal/emails/generators/test-class-course-welcome.php Outdated

Comment thread tests/unit-tests/internal/emails/generators/test-class-course-welcome.php Outdated

Guard course_welcome against re-welcoming students with course history

d21668f

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

donnapep removed the No Changelog label Jun 30, 2026

donnapep changed the title ~~Add failing tests for course_welcome re-send on existing students~~ Guard course_welcome email against re-welcoming students with course history Jun 30, 2026

donnapep requested a review from Copilot June 30, 2026 15:59

Copilot started reviewing on behalf of donnapep June 30, 2026 16:00 View session

Copilot AI reviewed Jun 30, 2026

View reviewed changes

donnapep and others added 8 commits June 30, 2026 13:06

Run test cleanup before assertion to avoid hook leaks

161be15

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Use repository-backed lesson count in welcome guard

8cc3002

Replaces the per-lesson get_completed_lesson_ids() loop with a single user_started_lesson_count() query, avoiding N+1 on large courses. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Merge branch 'trunk' into fix/course-welcome-skip-students-with-progress

8000379

Fix lint warnings in course welcome send-count test

863b195

Drop unused closure params and use pre-increment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

donnapep requested a review from Copilot July 3, 2026 13:31

Copilot started reviewing on behalf of donnapep July 3, 2026 13:32 View session

Copilot AI reviewed Jul 3, 2026

View reviewed changes

Comment thread includes/internal/emails/generators/class-course-welcome.php

Comment thread includes/update-tasks/class-sensei-update-backfill-course-welcome-email-sent.php

donnapep and others added 4 commits July 3, 2026 09:51

Use metadata_exists in welcome backfill presence check

6e52bd5

Test presence rather than truthiness so a pre-existing falsy value is never overwritten. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Bump version to 4.26.2

1560451

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

donnapep requested a review from Copilot July 3, 2026 18:11

Copilot started reviewing on behalf of donnapep July 3, 2026 18:11 View session

Copilot AI reviewed Jul 3, 2026

View reviewed changes

donnapep merged commit 2819a79 into trunk Jul 3, 2026
22 checks passed

donnapep deleted the fix/course-welcome-skip-students-with-progress branch July 3, 2026 18:22

Uh oh!

Conversation

donnapep commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Change

Test plan

Known limitation

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

WordPress Playground Preview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

donnapep commented Jul 3, 2026

Uh oh!

claude Bot commented Jul 3, 2026

Uh oh!

donnapep commented Jul 3, 2026

Uh oh!

claude Bot commented Jul 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

donnapep commented Jun 30, 2026 •

edited

Loading

github-actions Bot commented Jun 30, 2026 •

edited

Loading