#53988 closed enhancement (fixed)
remove duplicate code in wp_list_filter()
| Reported by: | pbearne | Owned by: | hellofromTonya |
|---|---|---|---|
| Priority: | normal | Milestone: | 5.9 |
| Component: | General | Version: | |
| Severity: | normal | Keywords: | has-patch commit |
| Cc: | Focuses: |
Description
The code in wp_list_filter() is the same as the code in wp_filter_object_list() with no $field so I have created a patch to wrap wp_filter_object_list removing duplicated code
Attachments (1)
Change History (12)
This ticket was mentioned in PR #1615 on WordPress/wordpress-develop by pbearne.
5 years ago
#1
- Keywords has-patch added
#2
@
5 years ago
I added tests for this in ticket https://core.trac.wordpress.org/ticket/53987
#3
follow-up:
↓ 4
@
5 years ago
- Milestone Awaiting Review → 5.9
#4
in reply to: ↑ 3
@
5 years ago
Replying to SergeyBiryukov:
So I think I would prefer reinstating that.
Scratch that, it looks like that would require creating a second WP_List_Util instance, which might have a performance or memory impact, depending on the size of the list.
#6
@
5 years ago
The unit tests for wp_list_filter() currently fail with this PR:
There was 1 failure:
1) Tests_Functions_wpListUtil::test_wp_list_filter with data set "invalid operator" (array(stdClass Object (...), stdClass Object (...)), array('bar'), 'XOR', array())
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
Array (
+ 0 => stdClass Object (...)
+ 1 => stdClass Object (...)
)
/var/www/tests/phpunit/includes/abstract-testcase.php:813
/var/www/tests/phpunit/tests/functions/wpListUtil.php:373
#7
@
5 years ago
The failure is because WP_List_Util::filter() performs validation of the $operator argument, but in case of an invalid operator it just returns early and does not assign the $this->output value that wp_filter_object_list() returns.
This means that wp_filter_object_list() and wp_list_filter() treat an invalid operator differently:
wp_filter_object_list()returns the original array.wp_list_filter()returns an empty array.
53988.diff fixes that and the tests seem to pass now, though it might be up for discussion whether the current behavior is a bug or a feature.
#8
@
5 years ago
- Keywords commit added
- Owner set to
- Status new → reviewing
In reviewing and testing 53988.diff, @SergeyBiryukov I think you did resolve an issue. Bailing out without resetting the $output property to an empty array means WP_List_Util::get_output() may not be in the right state when invoked. This was why the tests were failing.
The patch looks good to me. Marking it for commit.
hellofromtonya commented on PR #1615:
5 years ago
#10
Closing as the alternate patch was committed via changeset https://core.trac.wordpress.org/changeset/52066.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
Trac ticket: https://core.trac.wordpress.org/ticket/53988