Fix draft lessons getting published when Course is updated

changelog/fix-publishing-lessons-in-course-updates

@@ -0,0 +1,4 @@
+Significance: minor
+Type: fixed
+
+Lessons being automatically published when Course is updated

includes/admin/class-sensei-course-pre-publish-panel.php

@@ -45,7 +45,7 @@
 	 */
 	public function init() {
 		add_action( 'enqueue_block_editor_assets', array( $this, 'enqueue_pre_publish_panel_assets' ) );
-		add_action( 'publish_course', array( $this, 'maybe_publish_lessons' ) );
+		add_action( 'publish_course', array( $this, 'maybe_publish_lessons' ), 10, 3 );
 	}
 
 	/**
@@ -64,9 +64,11 @@
 	 *
 	 * @internal
 	 *
-	 * @param int $course_id Course ID.
+	 * @param int     $course_id  Course ID.
+	 * @param WP_Post $post       Post object.
+	 * @param string  $old_status Old post status.
 	 */
-	public function maybe_publish_lessons( $course_id ) {
+	public function maybe_publish_lessons( $course_id, $post, $old_status ) {
 		if ( ! current_user_can( 'publish_post', $course_id ) ) {
 			return;
 		}
@@ -77,6 +79,33 @@
 			return;
 		}
 
+		$publishing_meta_key = '_sensei_course_publishing_started';
+
+		// Even if the course is already published, each subsequent updates also triggers this hook anyway
+		// which caused the bug https://github.com/Automattic/sensei/issues/7555.
+		// So we need to check if it's an actual publish call.
+		$is_main_publish_call = 'publish' !== $old_status;
+
+		if ( $is_main_publish_call ) {
+			// This is the first call made, it's not the structure saving call, so the added/updated lessons are not yet saved at this point.
+			// So we set the flag to publish lessons on the next call, which is made after the structure is saved.
+			update_post_meta( $course_id, $publishing_meta_key, true );
+		}
+
+		$is_publishing_started = get_post_meta( $course_id, $publishing_meta_key, true );
+
+		if ( ! $is_main_publish_call && ! $is_publishing_started ) {
+			// If its not the "Publish" call and the flag is not set, then we don't need to publish lessons.
+			// Because it that case it's just a normal "Update" call.
+			return;
+		}
+
+		if ( ! $is_main_publish_call ) {
+			// If it's not the main publish call, then it's the structure saving call that comes immediately after the main publish call.
+			// So we can remove the flag now, because after this iteraction, the whole publishing cycle is complete.
+			delete_post_meta( $course_id, $publishing_meta_key );
+		}
+
 		// Publish all draft lessons for this course.
 		$lesson_ids = Sensei()->course->course_lessons( $course_id, 'draft', 'ids' );
 

includes/class-sensei.php

@@ -721,6 +721,8 @@
 
 		Sensei_Temporary_User::init();
 
+		Sensei_Course_Pre_Publish_Panel::instance()->init();
+
 		// Differentiate between administration and frontend logic.
 		if ( is_admin() ) {
 			// Load Admin Class.
@@ -732,7 +734,6 @@
 
 			Sensei_No_Users_Table_Relationship::instance()->init();
 			SenseiLMS_Plugin_Updater::init();
-			Sensei_Course_Pre_Publish_Panel::instance()->init();
 		} else {
 
 			// Load Frontend Class.

tests/unit-tests/admin/test-class-sensei-course-pre-publish-panel.php

@@ -49,7 +49,6 @@ public function setUp(): void {
 
 	public function tearDown(): void {
 		parent::tearDown();
-
 		$this->factory->tearDown();
 	}
 
@@ -64,7 +63,7 @@ public function testMaybePublishLessons_InsufficientPermissions_DoesNotPublishLe
 		update_post_meta( $this->course_id, 'sensei_course_publish_lessons', true );
 
 		/* Act */
-		Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id );
+		Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id, null, 'draft' );
 
 		/* Assert */
 		$this->assertEquals( 'draft', get_post_status( $this->lesson_id ) );
@@ -81,7 +80,7 @@ public function testMaybePublishLessons_MetaIsFalse_DoesNotPublishLessons() {
 		update_post_meta( $this->course_id, 'sensei_course_publish_lessons', false );
 
 		/* Act */
-		Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id );
+		Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id, null, 'draft' );
 
 		/* Assert */
 		$this->assertEquals( 'draft', get_post_status( $this->lesson_id ) );
@@ -98,9 +97,146 @@ public function testMaybePublishLessons_SufficientPermissionsAndMetaIsTrue_DoesP
 		update_post_meta( $this->course_id, 'sensei_course_publish_lessons', true );
 
 		/* Act */
-		Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id );
+		Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id, null, 'draft' );
+
+		/* Assert */
+		$this->assertEquals( 'publish', get_post_status( $this->lesson_id ) );
+	}
+
+	/**
+	 * Lessons aren't published if a published course is just being updated.
+	 *
+	 *  @covers Sensei_Course_Pre_Publish_Panel::maybe_publish_lessons
+	 */
+	public function testMaybePublishLessons_WhenPreviousStateAlreadyPublished_DoesNotPublishLessons() {
+		/* Arrange */
+		$this->login_as_admin();
+		update_post_meta( $this->course_id, 'sensei_course_publish_lessons', true );
+
+		/* Act */
+		Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id, null, 'publish' );
+
+		/* Assert */
+		$this->assertEquals( 'draft', get_post_status( $this->lesson_id ) );
+	}
+
+	/**
+	 * Lessons are not published a course is actually publishing but meta is false.
+	 *
+	 *  @covers Sensei_Course_Pre_Publish_Panel::maybe_publish_lessons
+	 */
+	public function testMaybePublishLessons_WhenFirstPublishedButMetaFalse_DoesNotPublishLessons() {
+		/* Arrange */
+		$this->login_as_admin();
+
+		/* Act */
+		Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id, null, 'draft' );
+
+		/* Assert */
+		$this->assertEquals( 'draft', get_post_status( $this->lesson_id ) );
+	}
+
+	/**
+	 * When Course is switched to publish state, the flag is set.
+	 *
+	 *  @covers Sensei_Course_Pre_Publish_Panel::maybe_publish_lessons
+	 */
+	public function testMaybePublishLessons_WhenFirstPublished_SetsThePublishContinuationFlag() {
+		/* Arrange */
+		$this->login_as_admin();
+		update_post_meta( $this->course_id, 'sensei_course_publish_lessons', true );
+
+		/* Act */
+		Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id, null, 'draft' );
+
+		/* Assert */
+		$this->assertEquals( 1, get_post_meta( $this->course_id, '_sensei_course_publishing_started', true ) );
+	}
+
+	/**
+	 * In the first subsequent call, the lessons are published.
+	 *
+	 *  @covers Sensei_Course_Pre_Publish_Panel::maybe_publish_lessons
+	 */
+	public function testMaybePublishLessons_WhenFirstPublished_OnlyTheSubsequentCallPublishesTheLessons() {
+		/* Arrange */
+		$this->login_as_admin();
+		update_post_meta( $this->course_id, 'sensei_course_publish_lessons', true );
+
+		// This call mimics the first publish call made by Gutenberg. The call to save the Course structure is not yet made. It's done after this call.
+		Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id, null, 'draft' );
+
+		// This mimics a new unsaved lesson that just became a saved one in draft state via the Course structure save call. This needs to get published in the next call.
+		$unsaved_to_draft_lesson_id = $this->factory->lesson->create(
+			[
+				'post_status' => 'draft',
+				'meta_input'  => [
+					'_lesson_course' => $this->course_id,
+				],
+			]
+		);
+
+		/* Act */
+		// Notice how the 'old_status' is 'publish' here. Because after publishing the post and the structure is saved, the old status is 'publish' for that Course,
+		// because the Course already got published in the previous call.
+		// Our Course publishing sequence from Gutenberg is like this:
+		// GB sends Publish Course call -> Then we send the structure saving call -> Then we send a Course update call.
+		Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id, null, 'publish' );
+
+		/* Assert */
+		$this->assertEquals( 'publish', get_post_status( $unsaved_to_draft_lesson_id ) );
+	}
+
+	/**
+	 * In the first subsequent call, the lessons are published, but not in the second or more subsequent calls.
+	 *
+	 *  @covers Sensei_Course_Pre_Publish_Panel::maybe_publish_lessons
+	 */
+	public function testMaybePublishLessons_AfterFirstPublishSequence_FartherSubsequentCallsDoNotPublishLessons() {
+		/* Arrange */
+		// Check the comments in the previous test for the explanation of the testing.
+		$this->login_as_admin();
+		update_post_meta( $this->course_id, 'sensei_course_publish_lessons', true );
+
+		Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id, null, 'draft' );
+
+		$unsaved_to_draft_lesson_id = $this->factory->lesson->create(
+			[
+				'post_status' => 'draft',
+				'meta_input'  => [
+					'_lesson_course' => $this->course_id,
+				],
+			]
+		);
+		Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id, null, 'publish' );
+
+		$unsaved_to_draft_lesson_id_2 = $this->factory->lesson->create(
+			[
+				'post_status' => 'draft',
+				'meta_input'  => [
+					'_lesson_course' => $this->course_id,
+				],
+			]
+		);
+		Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id, null, 'publish' );
+
+		$unsaved_to_draft_lesson_id_3 = $this->factory->lesson->create(
+			[
+				'post_status' => 'draft',
+				'meta_input'  => [
+					'_lesson_course' => $this->course_id,
+				],
+			]
+		);
+
+		/* Act */
+		// See comments in the previous test for the explanation of the 'old_status'.
+		Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id, null, 'publish' );
 
 		/* Assert */
 		$this->assertEquals( 'publish', get_post_status( $this->lesson_id ) );
+		$this->assertEquals( 'publish', get_post_status( $unsaved_to_draft_lesson_id ) );
+		$this->assertEquals( 'draft', get_post_status( $unsaved_to_draft_lesson_id_2 ) );
+		$this->assertEquals( 'draft', get_post_status( $unsaved_to_draft_lesson_id_3 ) );
 	}
 }