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.

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 PHP 8.1 deprecation notice on Students page #7038

Merged
merged 2 commits into from
Jul 20, 2023

Conversation

donnapep
Copy link
Collaborator

@donnapep donnapep commented Jul 20, 2023

Resolves #7029.

Proposed Changes

wp_kses was the culprit because null was being passed as the first parameter. This is because we were passing the result of the call to render_bulk_action_select_box, which has no return value, so null was used instead. The net result was that wp_kses had no effect.

This PR changes the implementation of render_bulk_action_select_box (and renames it) such that the HTML is returned. Now, null is no longer passed to wp_kses and the call actually works. This became apparent when I realized that the dropdown wasn't displaying correctly, because the class attribute was being stripped out. So I added class as part of the allowed HTML for wp_kses.

Testing Instructions

  1. On a PHP 8.1 site, browse to the Students page.
  2. Ensure the deprecated notice isn't being logged / displayed.
  3. Check that the bulk action dropdown looks good and is populated correctly.

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
  • 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

@donnapep donnapep added this to the 4.16.1 milestone Jul 20, 2023
@donnapep donnapep self-assigned this Jul 20, 2023
@donnapep donnapep requested review from a team and Imran92 July 20, 2023 01:28
@codecov
Copy link

codecov bot commented Jul 20, 2023

Codecov Report

Merging #7038 (b664404) into trunk (5cbcc2e) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##              trunk    #7038      +/-   ##
============================================
- Coverage     48.03%   48.02%   -0.01%     
  Complexity    10458    10458              
============================================
  Files           573      573              
  Lines         44141    44145       +4     
  Branches        402      402              
============================================
  Hits          21201    21201              
- Misses        22613    22617       +4     
  Partials        327      327              
Impacted Files Coverage Δ
.../class-sensei-learners-admin-bulk-actions-view.php 55.47% <0.00%> (-0.86%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5435fd2...b664404. Read the comment docs.

Copy link
Contributor

@Imran92 Imran92 left a comment

Choose a reason for hiding this comment

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

Thanks for the explanation 💯 And it's working nicely 🚀

@donnapep donnapep merged commit 50b8bcf into trunk Jul 20, 2023
18 of 20 checks passed
@donnapep donnapep deleted the fix/php81-deprecation-notice branch July 20, 2023 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecated notice on Students page using PHP 8.1
2 participants