Skip to content

Introduce WP_List_Table::get_views_links().#2828

Closed
costdev wants to merge 31 commits intoWordPress:trunkfrom
costdev:WP_List_Table_get_admin_status_links
Closed

Introduce WP_List_Table::get_views_links().#2828
costdev wants to merge 31 commits intoWordPress:trunkfrom
costdev:WP_List_Table_get_admin_status_links

Conversation

@costdev
Copy link
Contributor

@costdev costdev commented Jun 19, 2022 •
edited
Loading

This PR introduces WP_List_Table::get_views_links() to abstract the link markup generation for list tables.

Includes PHPUnit tests with full line/branch coverage.

Trac ticket: https://core.trac.wordpress.org/ticket/42066

@costdev costdev force-pushed the WP_List_Table_get_admin_status_links branch from 9dda2f6 to c34e260 Compare June 19, 2022 06:58
@costdev costdev marked this pull request as ready for review June 19, 2022 07:12
@costdev costdev force-pushed the WP_List_Table_get_admin_status_links branch from c34e260 to 8cdfecd Compare July 12, 2022 01:07
@costdev costdev changed the title Introduce WP_List_Table::get_admin_status_links(). Introduce WP_List_Table::get_views_links(). Jul 12, 2022
@costdev costdev force-pushed the WP_List_Table_get_admin_status_links branch from 8cdfecd to 6fe0cbc Compare July 12, 2022 01:12
peterwilsoncc
peterwilsoncc reviewed Jul 26, 2022
View reviewed changes
Copy link
Contributor

@peterwilsoncc peterwilsoncc left a comment

Choose a reason for hiding this comment

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

Added a few notes inline.

A bit of extra escaping is needed as the highest priority item.

@costdev costdev force-pushed the WP_List_Table_get_admin_status_links branch from 6fe0cbc to 6f02e78 Compare July 26, 2022 22:08
peterwilsoncc
peterwilsoncc reviewed Jul 27, 2022
View reviewed changes
Copy link
Contributor

@peterwilsoncc peterwilsoncc 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 follow up.

My one last thought is about tests. There don't seem to be any tests for get_views() in the core test suite.

Would it be possible to add some to ensure the markup is equivalent on the old vs new?

@costdev costdev force-pushed the WP_List_Table_get_admin_status_links branch from 6f02e78 to 525b54b Compare August 28, 2022 06:22
@costdev costdev requested a review from peterwilsoncc August 28, 2022 06:33
costdev
costdev commented Aug 28, 2022
View reviewed changes
costdev
costdev commented Aug 28, 2022
View reviewed changes
src/wp-admin/includes/class-wp-posts-list-table.php
number_format_i18n( $this->user_posts_count )
);

$mine = $this->get_edit_link( $mine_args, $mine_inner_html, $class );
Copy link
Contributor Author

@costdev costdev Aug 28, 2022

Choose a reason for hiding this comment

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

FYI: WP_Posts_List_Table::get_edit_link() is still used by:

  • WP_Posts_List_Table::column_author()
  • WP_Posts_List_Table::column_default()

These can be replaced to use get_views_links(), but this would lose context.

This is another use case for a general function like the proposed wp_create_links().

costdev
costdev commented Aug 28, 2022
View reviewed changes
@costdev costdev force-pushed the WP_List_Table_get_admin_status_links branch 2 times, most recently from 54d4064 to bffc00b Compare September 2, 2022 03:41
costdev added 15 commits September 2, 2022 04:47
Add tests for WP_List_Table::get_views_links().
57b7e5f
Add WP_List_Table::get_views_links().
809ab76
Posts: Use ::get_views_links() in list table.
3a7181f 
`WP_Posts_List_Table::get_edit_link()` is still used by:
- `::column_author()`
- `::column_default()`

These can be replaced to use `get_views_links()`, but this would lose
context.

This is another use case for a general function like the proposed
`wp_create_links()`.
Comments: Use ::get_views_links() in list table.
588c02b
Users: Use ::get_views_links() in list table.
7d4de21
Plugins: Use ::get_views_links() in list table.
5a358f2
Plugin Install: Use ::get_views_links() in list table.
d1fbe28
Theme Install: Use ::get_views_links() in list table.
fea9e02
Privacy Requests: Use ::get_views_links() in list table.
4361d93
MS Sites: Use ::get_views_links() in list table.
3de35a5
MS Users: Use ::get_views_links() in list table.
47f5f9d
MS Themes: Use ::get_views_links() in list table.
0425791
WP_List_Table: Allow empty $link_data array.
9f34460
WP_List_Table: Use empty() for URL and label.
bcb9adc
WP_List_Table: Improve escaping.
8b4c3cb
costdev added 15 commits September 2, 2022 04:47
Add tests for WP_Posts_List_Table::get_views().
bcdadf3
Add tests for WP_Comments_List_Table::get_views().
815daf5
Add tests for WP_Post_Comments_List_Table::get_views().
ea3e1cd
Add tests for WP_Users_List_Table::get_views().
ac36df8
Add tests for WP_Plugins_List_Table::get_views().
e54bf12
Add tests for WP_Plugin_Install_List_Table::get_views().
f400ab7
Add tests for WP_Theme_Install_List_Table::get_views().
9e30826
Add tests for WP_Privacy_Requests_List_Table::get_views().
79639ea
Add tests for WP_MS_Sites_List_Table::get_views().
0d11699
Add tests for WP_MS_Users_List_Table::get_views().
ae2ebd7
Add tests for WP_MS_Themes_List_Table::get_views().
45333b4
Tests: Replace \' with " to fix failures.
e4a63bb
Tests: Replace & with & to fix failures.
5945f30
Tests: Get actual user counts in test data.
be0780b 
This test was previously faulty and used the counts from a local install.

This has now been resolved using:
- `get_user_count()` for 'all'
- `count( get_super_admins() )` for 'super'
Tests: Fix docblock alignment.
92a05b4
@costdev costdev force-pushed the WP_List_Table_get_admin_status_links branch from bffc00b to 92a05b4 Compare September 2, 2022 03:49
peterwilsoncc
peterwilsoncc reviewed Sep 9, 2022
View reviewed changes
Copy link
Contributor

@peterwilsoncc peterwilsoncc left a comment

Choose a reason for hiding this comment

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

LGTM but a minor change is needed to prevent the tests running in single site mode using the correct group.

Tests well when I pulled down the branch and applied it. I couldn't see any visual changes switching between this branch and trunk.

@costdev costdev force-pushed the WP_List_Table_get_admin_status_links branch from e50ef98 to d26fdc0 Compare September 9, 2022 09:00
@dream-encode
Copy link
Contributor

dream-encode commented Sep 19, 2022

Merged into core in https://core.trac.wordpress.org/changeset/54215.

@costdev costdev deleted the WP_List_Table_get_admin_status_links branch September 19, 2022 21:15
westonruter
westonruter reviewed Mar 6, 2026
View reviewed changes
src/wp-admin/includes/class-wp-posts-list-table.php
number_format_i18n( $this->user_posts_count )
);

$mine = $this->get_edit_link( $mine_args, $mine_inner_html, $class );
Copy link
Member

@westonruter westonruter Mar 6, 2026

Choose a reason for hiding this comment

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

With the removal of $class being passed in here, it seems the variable is no longer used. This is was flagged by PHPStan. In #11151 I've included cb7a8c3 to address this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@westonruter westonruter westonruter left review comments

@peterwilsoncc peterwilsoncc Awaiting requested review from peterwilsoncc

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@costdev @dream-encode @westonruter @peterwilsoncc