Keep current status for a question when it is updated

[merkushin]

Resolves #7601

When we update a question, its status resets to "draft".

The issue appears only for questions that are not in use (are not part of a quiz or of a category).

Proposed Changes

• Preserve the post status for a question when it is being updated.

Testing Instructions

1. Go to Sensei LMS -> Questions.
2. Create a new question and publish it.
3. Open the question in the editor. (Probably, it is already open in the editor for you.)
4. Modify the question content and click "Update".
5. Make sure you didn't see the notification with text "Post is reverted to draft" and the post (question) is still published.

Pre-Merge Checklist

• PR title and description contain sufficient detail and accurately describe the changes
• Acceptance criteria is met
• Decisions are publicly documented
• Adheres to coding standards (PHP, JavaScript, CSS, HTML)
• All strings are translatable (without concatenation, handles plurals)
• Follows our naming conventions (P6rkRX-4oA-p2)
• Hooks (p6rkRX-1uS-p2) and functions are documented
• New UIs are responsive and use a mobile-first approach
• New UIs match the designs
• Different user privileges (admin, teacher, subscriber) are tested as appropriate
• Legacy courses (course without blocks) are tested
• Code is tested on the minimum supported PHP and WordPress versions
• User interface changes have been tested on the latest versions of Chrome, Firefox and Safari
• "Needs Documentation" label is added if this change requires updates to documentation
• Known issues are created as new GitHub issues

[github-actions]

Test the previous changes of this PR with WordPress Playground.

[github-actions]

Test the previous changes of this PR with WordPress Playground.

[Imran92]

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?

[merkushin]

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).

[merkushin]

Updated here: 969082e

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

[github-actions]

Test the previous changes of this PR with WordPress Playground.

[github-actions]

Test the previous changes of this PR with WordPress Playground.

[Imran92]

LGTM! The solution looks better now! Thanks for making the changes.

Looks like the PHP test is failing - https://github.com/Automattic/sensei/actions/runs/9323311876/job/25666195954?pr=7603 . Probably should be looked into before approval

[github-actions]

Test the previous changes of this PR with WordPress Playground.

[merkushin]

Thanks @Imran92!

I had to turn to your suggested approach with a ternary. I missed that in wp_update_post the post must exist.

However, it showed another problem. It looks like the behavior of wp_insert_post is expected: https://github.com/Automattic/sensei/actions/runs/9354613672/job/25747848349?pr=7603

The test checks that post_content became empty after update with no post_content provided.

On one hand it is a backward incompatibility, because that behavior was documented in tests.
On the other hand, it is a fix 😅

I'll think a bit more about it... What is your opinion about it?

[Imran92]

On one hand it is a backward incompatibility, because that behavior was documented in tests.
On the other hand, it is a fix 😅

This behavior looks hacky, and in general, I don't like these behaviors. We need to figure out why this behavior is expected in the test and if it breaks anything otherwise, this can be just an erroneous test too.

checks that post_content became empty after update with no post_content provided.

If the compatible behavior we're looking for is the post_content to be empty if no post_content is provided, we can maybe just assign post_content as empty in the wp_update_post's args when updating if post_content is not in the payload.

[github-actions]

Test the previous changes of this PR with WordPress Playground.

[github-actions]

Test the previous changes of this PR with WordPress Playground.

[merkushin]

We need to figure out why this behavior is expected in the test and if it breaks anything otherwise, this can be just an erroneous test too.

The test was added simultaneously with the method, so I think it wasn't erroneous. But it could be just a reflection of the solution that was used (wp_insert_post). I tested the question UI differently, didn't notice any issues with it. And it worked for 3 years without complaints.

If the compatible behavior we're looking for is the post_content to be empty if no post_content is provided, we can maybe just assign post_content as empty in the wp_update_post's args when updating if post_content is not in the payload.

Yep, I think that's the safest solution, updated it here: 485caf5

[merkushin]

PR updated and ready for review.

[github-actions]

Test the previous changes of this PR with WordPress Playground.

[m1r0]

Works as described. 👍