Fix silent insert failure in tables-based progress repositories

[donnapep]

Summary

$wpdb->insert() returns false on failure, but none of the tables-based repositories checked this return value. When an insert failed, the code silently continued with $wpdb->insert_id of 0, constructing invalid objects and losing student data without any indication.

This PR:

• Throws a RuntimeException when $wpdb->insert() returns false in all tables-based repositories (progress, submission, answer, grade)
• Adds try-catch handling at all call sites so failures are logged rather than crashing
• Wraps secondary store (dual-write sync) operations in try-catch so a sync failure doesn't block the primary store
• Adds @throws annotations to repository interfaces and implementations

Test plan

Happy path

• With storage set to tables (Sensei > Settings > Experimental), enroll as a student, start a lesson, complete it, take a quiz — progress saves normally
• With storage set to comments (default), repeat the above — progress saves normally

Error handling

To simulate a database insert failure, temporarily rename the progress table:

RENAME TABLE wp_sensei_lms_progress TO wp_sensei_lms_progress_backup;

With WP_DEBUG_LOG enabled and storage set to tables:

• Visit a lesson page as an enrolled student — should see error notices ("An error occurred while starting the course/lesson. Please try again.") and log entries in wp-content/debug.log
• Click "Complete Lesson" — should not crash; error notices remain, progress stays at 0%
• Restore the table and flush the cache when done:

  RENAME TABLE wp_sensei_lms_progress_backup TO wp_sensei_lms_progress;

  wp cache flush
  

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR fixes a silent data-loss bug in the tables-based student progress repositories. Previously, Tables_Based_Lesson_Progress_Repository::create() and Tables_Based_Quiz_Progress_Repository::create() ignored the return value of $wpdb->insert(). A failed insert would silently continue with insert_id = 0, producing a fake progress object with a zero ID that could corrupt downstream operations. The fix stores the return value and throws a \RuntimeException immediately on failure.

Changes:

• Add $result variable to capture $wpdb->insert() return value and throw a \RuntimeException if it equals false in both Tables_Based_Lesson_Progress_Repository and Tables_Based_Quiz_Progress_Repository.
• Add @throws \RuntimeException PHPDoc annotation to the create() methods of both repositories.
• Add a changelog entry for the fix.

Reviewed changes

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

File	Description
includes/internal/student-progress/lesson-progress/repositories/class-tables-based-lesson-progress-repository.php	Captures insert() return value and throws RuntimeException on failure; adds @throws docblock
includes/internal/student-progress/quiz-progress/repositories/class-tables-based-quiz-progress-repository.php	Same fix applied to the quiz progress repository
changelog/fix-tables-based-repo-insert-error-handling	New changelog entry describing the patch

Note: The identical bug exists in Tables_Based_Course_Progress_Repository::create() (line 83 of class-tables-based-course-progress-repository.php) but was not addressed in this PR.

---

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

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

Pull request overview

Copilot reviewed 37 out of 37 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

includes/update-tasks/class-sensei-update-legacy-quiz-data.php:74

• run_batch() soft-deletes the legacy answer comment unconditionally. If the insert into the new tables fails (caught RuntimeException), the legacy comment is still deleted, which can permanently lose the only copy of the student’s answer/grade and prevents a retry on the next batch run. Only delete the comment after a successful migration, or continue from the catch block without deleting so the batch can retry later.

---

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

You can also share your feedback on Copilot code review. Take the survey.

[donnapep]

@merkushin This one introduces logic for handling insert failures to the custom table. Just inserts for now but will expand to include other database operations once the approach is vetted.

[mdawaffe]

Most things just error_log(). Some things throw. A few things call Sensei()->notices->add_notice(). Is there a pattern for when things should do what?

[mdawaffe]

This (and most of the others) never actually throw. Should they?