Skip to content

Commit ae239a2

Browse files
donnapepdonnapep
authored
Merge pull request #7582 from Automattic/fix/publishing-lessons-in-course-updates
Fix draft lessons getting published when Course is updated
2 parents cd49e3a + 2ac96e8 commit ae239a2

File tree

4 files changed

+222
-8
lines changed

4 files changed

+222
-8
lines changed

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Significance: minor
2+
Type: fixed
3+
4+
Lessons being automatically published when Course is updated

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

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public static function instance() {
4545
*/
4646
public function init() {
4747
add_action( 'enqueue_block_editor_assets', array( $this, 'enqueue_pre_publish_panel_assets' ) );
48-
add_action( 'publish_course', array( $this, 'maybe_publish_lessons' ) );
48+
add_action( 'publish_course', array( $this, 'maybe_publish_lessons' ), 10, 3 );
4949
}
5050

5151
/**
@@ -64,19 +64,67 @@ public function enqueue_pre_publish_panel_assets() {
6464
*
6565
* @internal
6666
*
67-
* @param int $course_id Course ID.
67+
* @param int $course_id Course ID.
68+
* @param WP_Post $post Post object.
69+
* @param string $old_status Old post status.
6870
*/
69-
public function maybe_publish_lessons( $course_id ) {
71+
public function maybe_publish_lessons( $course_id, $post, $old_status ) {
72+
/**
73+
* When Saving/Publishing a Course from the Course editor, usually three primary network calls are made from the Gutenberg editor serially -
74+
*
75+
* 1. The first call is initiated automatically by GB the moment we click the Publish/Update button, it saves the whole block `markup` of the course, but doesn't save the structure of the course, so no lessons or modules.
76+
* 2. After first call is successful, Sensei explicitly makes a second call to save the structure of the course, i.e. the lessons and modules.
77+
* 3. After the second call is successful, Sensei triggers the Post save call again, similar to the first call.
78+
*
79+
* When we click on the Publish button for a new Course containing new unsaved lessons, the first call (1) publishes the Draft course, new lessons are not saved yet.
80+
* So if we try to find lessons under this Course at this point with this `publish_course` hook, we'll only get existing lessons, not the new ones.
81+
*
82+
* The second call (2) saves the new lessons and modules of the course. This is not a Post save/publish call, so it doesn't trigger the `publish_course` hook.
83+
*
84+
* When the second call is successful, Sensei triggers the Post save call again, which invokes this `publish_course`. By this time, the new lessons are saved and we can find and publish them.
85+
* An important point is `publish_course` hook is invoked even when 'Update' is clicked on an already published course. So before publishing the lessons using this hook, we need to check if the course is being published or just being updated.
86+
*/
7087
if ( ! current_user_can( 'publish_post', $course_id ) ) {
7188
return;
7289
}
7390

91+
// In parallel to the 3 calls mentioned above, GB also initiates some calls to save metabox data, they also trigger this hook. But we don't want to process anything for them.
92+
$uri = isset( $_SERVER['REQUEST_URI'] ) ? esc_url_raw( wp_unslash( $_SERVER['REQUEST_URI'] ) ) : '';
93+
$is_metabox_save_call = strpos( $uri, 'meta-box-loader=1' ) > 0;
94+
95+
if ( $is_metabox_save_call ) {
96+
return;
97+
}
98+
7499
$publish_lessons = get_post_meta( $course_id, 'sensei_course_publish_lessons', true );
75100

76101
if ( ! $publish_lessons ) {
77102
return;
78103
}
79104

105+
$publishing_meta_key = '_sensei_course_publishing_started';
106+
107+
// This is how we can determine if the current call is the main publish call or not. This should be true when call (1) is made to publish the course.
108+
$is_main_publish_call = 'publish' !== $old_status;
109+
110+
if ( $is_main_publish_call ) {
111+
// If it's the main publish call, we set this flag to use in the call (3) which will come later to determine if the call is made as part of the publishing sequence or just a normal update sequence.
112+
update_post_meta( $course_id, $publishing_meta_key, true );
113+
// We don't return early here, because we still need to publish the lessons, in case we are publishing a course with existing draft lessons.
114+
}
115+
116+
$is_publishing_started = get_post_meta( $course_id, $publishing_meta_key, true );
117+
118+
if ( ! $is_main_publish_call && ! $is_publishing_started ) {
119+
// If it's not the main publish call and the flag is not set, then it's just an update call sequence, We don't publish anything in normal update sequence, so we return early.
120+
return;
121+
}
122+
123+
if ( ! $is_main_publish_call ) {
124+
// If it has reached here, this means it is call (3), end of the sequence has been reached by the Publishing cycle. So we just delete the flag.
125+
delete_post_meta( $course_id, $publishing_meta_key );
126+
}
127+
80128
// Publish all draft lessons for this course.
81129
$lesson_ids = Sensei()->course->course_lessons( $course_id, 'draft', 'ids' );
82130

‎includes/class-sensei.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -721,6 +721,8 @@ public function initialize_global_objects() {
721721

722722
Sensei_Temporary_User::init();
723723

724+
Sensei_Course_Pre_Publish_Panel::instance()->init();
725+
724726
// Differentiate between administration and frontend logic.
725727
if ( is_admin() ) {
726728
// Load Admin Class.
@@ -732,7 +734,6 @@ public function initialize_global_objects() {
732734

733735
Sensei_No_Users_Table_Relationship::instance()->init();
734736
SenseiLMS_Plugin_Updater::init();
735-
Sensei_Course_Pre_Publish_Panel::instance()->init();
736737
} else {
737738

738739
// Load Frontend Class.

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

Lines changed: 165 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ public function setUp(): void {
4949

5050
public function tearDown(): void {
5151
parent::tearDown();
52-
5352
$this->factory->tearDown();
5453
}
5554

@@ -64,7 +63,7 @@ public function testMaybePublishLessons_InsufficientPermissions_DoesNotPublishLe
6463
update_post_meta( $this->course_id, 'sensei_course_publish_lessons', true );
6564

6665
/* Act */
67-
Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id );
66+
Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id, null, 'draft' );
6867

6968
/* Assert */
7069
$this->assertEquals( 'draft', get_post_status( $this->lesson_id ) );
@@ -81,7 +80,7 @@ public function testMaybePublishLessons_MetaIsFalse_DoesNotPublishLessons() {
8180
update_post_meta( $this->course_id, 'sensei_course_publish_lessons', false );
8281

8382
/* Act */
84-
Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id );
83+
Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id, null, 'draft' );
8584

8685
/* Assert */
8786
$this->assertEquals( 'draft', get_post_status( $this->lesson_id ) );
@@ -98,9 +97,171 @@ public function testMaybePublishLessons_SufficientPermissionsAndMetaIsTrue_DoesP
9897
update_post_meta( $this->course_id, 'sensei_course_publish_lessons', true );
9998

10099
/* Act */
101-
Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id );
100+
Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id, null, 'draft' );
102101

103102
/* Assert */
104103
$this->assertEquals( 'publish', get_post_status( $this->lesson_id ) );
105104
}
105+
106+
/**
107+
* Lessons aren't published if a published course is just being updated.
108+
*
109+
* @covers Sensei_Course_Pre_Publish_Panel::maybe_publish_lessons
110+
*/
111+
public function testMaybePublishLessons_WhenPreviousStateAlreadyPublished_DoesNotPublishLessons() {
112+
/* Arrange */
113+
$this->login_as_admin();
114+
update_post_meta( $this->course_id, 'sensei_course_publish_lessons', true );
115+
116+
/* Act */
117+
Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id, null, 'publish' );
118+
119+
/* Assert */
120+
$this->assertEquals( 'draft', get_post_status( $this->lesson_id ) );
121+
}
122+
123+
/**
124+
* Lessons are not published a course is actually publishing but meta is false.
125+
*
126+
* @covers Sensei_Course_Pre_Publish_Panel::maybe_publish_lessons
127+
*/
128+
public function testMaybePublishLessons_WhenFirstPublishedButMetaFalse_DoesNotPublishLessons() {
129+
/* Arrange */
130+
$this->login_as_admin();
131+
132+
/* Act */
133+
Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id, null, 'draft' );
134+
135+
/* Assert */
136+
$this->assertEquals( 'draft', get_post_status( $this->lesson_id ) );
137+
}
138+
139+
/**
140+
* When Course is switched to publish state, the flag is set.
141+
*
142+
* @covers Sensei_Course_Pre_Publish_Panel::maybe_publish_lessons
143+
*/
144+
public function testMaybePublishLessons_WhenFirstPublished_SetsThePublishContinuationFlag() {
145+
/* Arrange */
146+
$this->login_as_admin();
147+
update_post_meta( $this->course_id, 'sensei_course_publish_lessons', true );
148+
149+
/* Act */
150+
Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id, null, 'draft' );
151+
152+
/* Assert */
153+
$this->assertEquals( 1, get_post_meta( $this->course_id, '_sensei_course_publishing_started', true ) );
154+
}
155+
156+
/**
157+
* When request comes from metabox save call, the flag is not removed.
158+
*
159+
* @covers Sensei_Course_Pre_Publish_Panel::maybe_publish_lessons
160+
*/
161+
public function testMaybePublishLessons_WhenCallIsFromMetaboxSave_DoesNotRemoveContinuationFlag() {
162+
/* Arrange */
163+
$this->login_as_admin();
164+
update_post_meta( $this->course_id, 'sensei_course_publish_lessons', true );
165+
166+
Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id, null, 'draft' );
167+
$publish_call_flag = get_post_meta( $this->course_id, '_sensei_course_publishing_started', true );
168+
$_SERVER['REQUEST_URI'] = 'example.com/test=1&meta-box-loader=1';
169+
170+
/* Act */
171+
Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id, null, 'publish' );
172+
173+
/* Assert */
174+
$meta_save_call_flag = get_post_meta( $this->course_id, '_sensei_course_publishing_started', true );
175+
$this->assertEquals( 1, $meta_save_call_flag, 'The flag should not be removed when a meta save call is made.' );
176+
$this->assertEquals( 1, $publish_call_flag, 'The flag should be set when the first publish call is made.' );
177+
178+
$_SERVER['REQUEST_URI'] = '';
179+
}
180+
181+
/**
182+
* In the first subsequent call, the lessons are published.
183+
*
184+
* @covers Sensei_Course_Pre_Publish_Panel::maybe_publish_lessons
185+
*/
186+
public function testMaybePublishLessons_WhenFirstPublished_OnlyTheSubsequentCallPublishesTheLessons() {
187+
/* Arrange */
188+
$this->login_as_admin();
189+
update_post_meta( $this->course_id, 'sensei_course_publish_lessons', true );
190+
191+
// 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.
192+
Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id, null, 'draft' );
193+
194+
// 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.
195+
$unsaved_to_draft_lesson_id = $this->factory->lesson->create(
196+
[
197+
'post_status' => 'draft',
198+
'meta_input' => [
199+
'_lesson_course' => $this->course_id,
200+
],
201+
]
202+
);
203+
204+
/* Act */
205+
// 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,
206+
// because the Course already got published in the previous call.
207+
// Our Course publishing sequence from Gutenberg is like this:
208+
// GB sends Publish Course call -> Then we send the structure saving call -> Then we send a Course update call.
209+
Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id, null, 'publish' );
210+
211+
/* Assert */
212+
$this->assertEquals( 'publish', get_post_status( $unsaved_to_draft_lesson_id ) );
213+
}
214+
215+
/**
216+
* In the first subsequent call, the lessons are published, but not in the second or more subsequent calls.
217+
*
218+
* @covers Sensei_Course_Pre_Publish_Panel::maybe_publish_lessons
219+
*/
220+
public function testMaybePublishLessons_AfterFirstPublishSequence_FartherSubsequentCallsDoNotPublishLessons() {
221+
/* Arrange */
222+
// Check the comments in the previous test for the explanation of the testing.
223+
$this->login_as_admin();
224+
update_post_meta( $this->course_id, 'sensei_course_publish_lessons', true );
225+
226+
Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id, null, 'draft' );
227+
228+
$unsaved_to_draft_lesson_id = $this->factory->lesson->create(
229+
[
230+
'post_status' => 'draft',
231+
'meta_input' => [
232+
'_lesson_course' => $this->course_id,
233+
],
234+
]
235+
);
236+
Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id, null, 'publish' );
237+
238+
$unsaved_to_draft_lesson_id_2 = $this->factory->lesson->create(
239+
[
240+
'post_status' => 'draft',
241+
'meta_input' => [
242+
'_lesson_course' => $this->course_id,
243+
],
244+
]
245+
);
246+
Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id, null, 'publish' );
247+
248+
$unsaved_to_draft_lesson_id_3 = $this->factory->lesson->create(
249+
[
250+
'post_status' => 'draft',
251+
'meta_input' => [
252+
'_lesson_course' => $this->course_id,
253+
],
254+
]
255+
);
256+
257+
/* Act */
258+
// See comments in the previous test for the explanation of the 'old_status'.
259+
Sensei_Course_Pre_Publish_Panel::instance()->maybe_publish_lessons( $this->course_id, null, 'publish' );
260+
261+
/* Assert */
262+
$this->assertEquals( 'publish', get_post_status( $this->lesson_id ), 'The first lesson should be published, because it was already part of the Course before publishing.' );
263+
$this->assertEquals( 'publish', get_post_status( $unsaved_to_draft_lesson_id ), 'The second lesson should be published, because it was added to the Course structure before second update call.' );
264+
$this->assertEquals( 'draft', get_post_status( $unsaved_to_draft_lesson_id_2 ), 'The third lesson should not be published, because it was added to the Course structure after the second update call.' );
265+
$this->assertEquals( 'draft', get_post_status( $unsaved_to_draft_lesson_id_3 ), 'The fourth lesson should not be published, because it was added to the Course structure after the second update call.' );
266+
}
106267
}

0 commit comments

Comments
 (0)