sensei
sensei copied to clipboard
Automattic
Keep current status for a question when it is updated
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
- Go to Sensei LMS -> Questions.
- Create a new question and publish it.
- Open the question in the editor. (Probably, it is already open in the editor for you.)
- Modify the question content and click "Update".
- 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
- [x] PR title and description contain sufficient detail and accurately describe the changes
- [ ] Acceptance criteria is met
- [x] Decisions are publicly documented
- [x] 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[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
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?
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.
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: https://github.com/Automattic/sensei/pull/7603/commits/485caf53f6fb0e5872f249e2b2a851d3cb158787