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.
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
Fix lesson bulk edit #7515
Fix lesson bulk edit #7515
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #7515 +/- ##
============================================
- Coverage 51.51% 51.50% -0.01%
Complexity 11231 11231
============================================
Files 622 622
Lines 47505 47511 +6
Branches 414 414
============================================
Hits 24471 24471
- Misses 22701 22707 +6
Partials 333 333
Continue to review full report in Codecov by Sentry.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works well for me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works as expected! 👍 👍 Just left a comment about a possible logic check change.
Also, do you think we should add an integration test here to make sure that if it breaks for some reason, we'll know? Because in this case, we didn't know how long it was broken for or what broke it. Having an integration test saving these meta fields can potentially save us from these scenarios I think
$quiz_grade_type = isset( $_REQUEST['quiz_grade_type'] ) ? sanitize_text_field( (string) wp_unslash( $_REQUEST['quiz_grade_type'] ) ) : ''; | ||
|
||
// Store the values for all selected posts. | ||
$lesson_ids = $_REQUEST['post'] ?? array(); // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized, WordPress.Security.ValidatedSanitizedInput.MissingUnslash -- Input is sanitized in the next lines. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably assigning to the empty array here is not necessary, because we already checked if it is an array on line 4337
Thanks, @Imran92! You're right. I'll create a separate PR with a test for it. |
Added a comment about a performance concern here #7523 (comment) |
Resolves #6992
I suppose something has changed in how the form works a few years ago. Updated the processing of data.
Proposed Changes
Testing Instructions
Pre-Merge Checklist