Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Keep current status for a question when it is updated #7603

Merged
merged 9 commits into from
Jul 30, 2024
Merged

Keep current status for a question when it is updated #7603

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c6682c5
Keep current status for a question when it is updated
merkushin May 21, 2024
d663017
Use current status only for existing questions
merkushin May 30, 2024
969082e
Use wp_update_post to keep values of unchanged fields
merkushin May 31, 2024
bfd0375
Use wp_update_post for category question saving
merkushin May 31, 2024
738b566
Use wp_insert_post for new questions
merkushin Jun 3, 2024
94c11d9
Merge branch 'trunk' into fix/question-status-reset
merkushin Jul 24, 2024
485caf5
Reset post_content when question description not provided
merkushin Jul 24, 2024
d9655ee
Fix psalm issues
merkushin Jul 24, 2024
269be0d
Merge branch 'trunk' into fix/question-status-reset
merkushin Jul 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelog/fix-question-status-reset
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fixed

Keep current status for a question when it is updated
7 changes: 5 additions & 2 deletions includes/rest-api/class-sensei-rest-api-question-helpers-trait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,9 @@ private function save_question( $question, $status = 'publish' ) {

if ( $status ) {
$post_args['post_status'] = $status;
} elseif ( ! $is_new ) {
// If status is not provided, use the current status of the question.
$post_args['post_status'] = get_post_status( $question_id );
merkushin marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

@Imran92 Imran92 May 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job! Solution works perfectly as expected! 🚀

I was just thinking of another solution and wanted to share with you.

Looks like this is a known issue caused by using wp_insert_post for updating posts instead of wp_update_post https://developer.wordpress.org/reference/functions/wp_insert_post/#comment-5150.

So I was just wondering, instead of fetching the status manually here and re-assigning it, do you think it'd be a better and more understandable solution to use the wp_update_post function when we are actually just updating the post? An added advantage will be, we'll be using the right function for the right job.

So we just remove the line above and we replace Line 199 with something like $result = $is_new ? wp_insert_post( $post_args ) : wp_update_post( $post_args );.

WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Imran92!

Yes, it makes sense for the sake of simplicity.

Thinking about it further, I see a little sense of using a ternary. We can simply use wp_update_post (wp_update_post calls wp_insert_post under the hood).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated here: 969082e

It isn't that important for the question category, but updated it for consistency: bfd0375

}

// Force publish the question if it's part of a quiz.
Expand Down Expand Up @@ -437,7 +440,7 @@ private function get_question( WP_Post $question ): array {
*
* @return array
*/
private function get_category_question( WP_Post $question ) : array {
private function get_category_question( WP_Post $question ): array {
$category = (int) get_post_meta( $question->ID, 'category', true );
$number = (int) get_post_meta( $question->ID, 'number', true );

Expand Down Expand Up @@ -494,7 +497,7 @@ private function get_question_common_properties( WP_Post $question ): array {
*
* @return array Media info. It includes the type, id, url and title.
*/
private function get_question_media( int $question_media_id, int $question_id ) : array {
private function get_question_media( int $question_media_id, int $question_id ): array {
$question_media = [];
$mimetype = get_post_mime_type( $question_media_id );
$attachment = get_post( $question_media_id );
Expand Down
26 changes: 26 additions & 0 deletions tests/unit-tests/rest-api/test-class-sensei-rest-api-questions-controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,32 @@ public function testQuestionUpdateToDraft() {
$this->assertEquals( 'draft', get_post_status( $question_id ) );
}

public function testQuestionUpdate_WhenPublishedQuestionAndNoStatusInPayload_DoesnChangeStatusToDraft() {
/* Arrange. */
$this->login_as_admin();

$question_id = $this->factory->question->create();

$this->save_question_post(
$question_id,
[
'status' => 'publish',
'content' => '<!-- wp:sensei-lms/quiz-question {"title":"Test"} --><!-- /wp:sensei-lms/quiz-question -->',
]
);

/* Act. */
$this->save_question_post(
$question_id,
[
'content' => '<!-- wp:sensei-lms/quiz-question {"title":"Test"} --><!-- /wp:sensei-lms/quiz-question -->',
]
);

/* Assert. */
$this->assertEquals( 'publish', get_post_status( $question_id ) );
}

public function testQuestionWithCategoryUpdateToDraft() {
$this->login_as_admin();

Expand Down
Loading