Automattic / Co-Authors-Plus

Multiple bylines and Guest Authors for WordPress
https://wordpress.org/plugins/co-authors-plus/
GNU General Public License v2.0
287 stars 205 forks source link

Fix PHP version 7.4 requirement #1034

Closed alecgeatches closed 2 months ago

alecgeatches commented 2 months ago

Description

Changes in https://github.com/Automattic/Co-Authors-Plus/pull/1032 increased the required PHP version to 7.4, but missed a few steps, including:

alecgeatches commented 2 months ago

Tests are failing with PHPUnit 7 removed:

@php ./vendor/bin/phpunit --exclude=ms-required --no-coverage
Warning: PHP Warning:  Class 'PHPUnit\Util\Getopt' not found in /tmp/wordpress-tests-lib/includes/phpunit6/compat.php on line 18
Error: Looks like you're using PHPUnit 9.6.19. WordPress requires at least PHPUnit 5.7.21 and is currently only compatible with PHPUnit up to 7.x.
Please use the latest PHPUnit version from the 7.x branch.
Script @php ./vendor/bin/phpunit --exclude=ms-required --no-coverage handling the integration event returned with error code 1
Error: Process completed with exit code 1.

It seems that we need to bundle the WordPress version change and integration setup changes together. I'll do that step in a separate PR, where we'll also raise the WordPress version to 5.9.

GaryJones commented 2 months ago

@alecgeatches Please review the extra commits I added. composer cs && composer integration && composer lint all pass fine.

alecgeatches commented 2 months ago

@GaryJones Wow! Lot of updated code, especially with function return type hinting in https://github.com/Automattic/Co-Authors-Plus/pull/1034/commits/c1d7f63f74ba05d404a9fbbc08de1ecfdcb5346c.

I noticed while reviewing that some functions were missing return type hints. These were the first couple I noticed in a file that was already updated with type hints:

https://github.com/Automattic/Co-Authors-Plus/blob/c1d7f63f74ba05d404a9fbbc08de1ecfdcb5346c/php/class-coauthors-guest-authors.php#L346 https://github.com/Automattic/Co-Authors-Plus/blob/c1d7f63f74ba05d404a9fbbc08de1ecfdcb5346c/php/class-coauthors-guest-authors.php#L1241


I used ripgrep to look for functions without a : and found a few others. I'm happy to add these types, but curious if there was a reason any of these were left out? Thanks!

> rg 'function [^:]+\{$' --type php

template-tags.php
3:function get_coauthors( $post_id = 0 ) {
50:function is_coauthor_for_post( $user, $post_id = 0 ) {
89:function coauthors__echo( $tag, $type = 'tag', $separators = array(), $tag_args = null, $echo = true ) {
161:function coauthors( $between = null, $betweenLast = null, $before = null, $after = null, $echo = true ) {
186:function coauthors_posts_links( $between = null, $betweenLast = null, $before = null, $after = null, $echo = true ) {
228:function coauthors_posts_links_single( $author ) {
269:function coauthors_firstnames( $between = null, $betweenLast = null, $before = null, $after = null, $echo = true ) {
293:function coauthors_lastnames( $between = null, $betweenLast = null, $before = null, $after = null, $echo = true ) {
317:function coauthors_nicknames( $between = null, $betweenLast = null, $before = null, $after = null, $echo = true ) {
341:function coauthors_links( $between = null, $betweenLast = null, $before = null, $after = null, $echo = true ) {
386:function coauthors_emails( $between = null, $betweenLast = null, $before = null, $after = null, $echo = true ) {
407:function coauthors_links_single( $author ) {
440:function coauthors_ids( $between = null, $betweenLast = null, $before = null, $after = null, $echo = true ) {
463:function get_the_coauthor_meta( $field, $user_id = false ) {
504:function the_coauthor_meta( $field, $user_id = 0 ) {
524:function coauthors_get_users( $args = array() ) {
586:function coauthors_wp_list_authors( $args = array() ) {
715:function coauthors_get_avatar( $coauthor, $size = 32, $default = '', $alt = false, $class = null ) {

php/class-coauthors-iterator.php
18: public function __construct( $postID = 0 ) {
62: public function get_position() {
78: public function get_all() {

tests/Integration/TemplateTagsTest.php
13: public function set_up() {

tests/Integration/ManageCoAuthorsTest.php
27: public function set_up() {

tests/Integration/TestCase.php
15: public function set_up() {
22: protected function create_subscriber( $user_login = 'subscriber' ) {
31: protected function create_contributor( $user_login = 'contributor' ) {
40: protected function create_author( $user_login = 'author' ) {
49: protected function create_editor( $user_login = 'editor' ) {
58: protected function create_guest_author( $user_login = 'guest_author' ) {
68: protected function create_post( \WP_User $author = null ) {

tests/Integration/bootstrap.php
35:function _manually_load_plugin() {

tests/Integration/EndpointsTest.php
20: public function set_up() {

tests/Integration/GuestAuthorsTest.php
14: public function set_up() {

tests/Integration/CoAuthorsPlusTest.php
11: public function set_up() {

php/blocks/block-coauthors/class-block-coauthors.php
144:        return function ( $value ) use ( $fns ) {

php/class-coauthors-template-filters.php
9:  public function __construct() {
26: public function filter_the_author_rss( $the_author ) {

php/class-coauthors-endpoint.php
44: public function __construct( $coauthors_instance ) {

php/class-coauthors-wp-list-table.php
16: public function __construct() {
31: public function prepare_items() {
151:    public function filter_query_for_search( $where ) {
163:    public function no_items() {
170:    public function get_columns() {
187:    public function single_row( $item ) {
200:    public function column_default( $item, $column_name ) {
280:    public function extra_tablenav( $which ) {
299:    public function display() {

php/api/endpoints/class-coauthors-controller.php
41: public function __construct( CoAuthors_Plus $coauthors_plus ) {
134:    public function get_item( $request ) {
187:    public function get_items( $request ) {
322:    public function prepare_item_for_response( $author, $request ) {

php/integrations/amp.php
4:function cap_add_amp_actions() {
9:function cap_update_amp_json_metadata( $metadata, $post ) {
24:function cap_set_amp_author_meta_template( $file, $type, $post ) {

upgrade.php
2:function coauthors_plus_upgrade( $from ) {
14:function coauthors_plus_upgrade_20() {

php/class-coauthors-guest-authors.php
22: public function __construct() {
155:    public function filter_post_updated_messages( $messages ) {
346:    public function action_parse_request( $query ) {
652:    public function filter_wp_dropdown_users_to_disable( $output ) {
762:    public function manage_guest_author_filter_post_data( $post_data, $original_args ) {
894:    public function get_guest_author_by( $key, $value, $force = false ) {
1020:   public function get_guest_author_fields( $groups = 'all' ) {
1096:   public function get_post_meta_key( $key ) {
1141:   public function get_all_linked_accounts( $force = false ) {
1172:   public function filter_update_post_metadata( $retnull, $object_id, $meta_key, $meta_value, $prev_value ) {
1241:   public function create( $args ) {
1308:   public function delete( $id, $reassign_to = false ) {
1364:   public function create_guest_author_from_user_id( $user_id ) {
1399:   public function filter_wp_insert_post_empty_content( $maybe_empty, $postarr ) {
1452:   public function filter_get_avatar( $avatar, $id_or_email, $size, $default ) {
1545:   public function filter_personal_data_exporter( $exporters ) {

co-authors-plus.php
64: function wp_notify_postauthor( $comment_id, $comment_type = '' ) {
186:function cap_filter_comment_moderation_email_recipients( $recipients, $comment_id ) {
213:function cap_get_coauthor_terms_for_post( $post_id ) {

php/class-coauthors-plus.php
34: public function __construct() {
307:    public function get_coauthor_by( $key, $value, $force = false ) {
505:    public function _filter_manage_posts_columns( $posts_columns ) {
578:    public function _filter_manage_users_custom_column( $value, $column_name, $user_id ) {
637:    public function action_edited_term_taxonomy_flush_cache( $tt_id, $taxonomy ) {
662:    public function update_author_term_post_count( $term ) {
697:    public function posts_join_filter( $join, $query ) {
848:    public function posts_groupby_filter( $groupby, $query ) {
871:    public function coauthors_set_post_author_field( $data, $postarr ) {
1082:   public function filter_wp_get_object_terms( $terms, $object_ids, $taxonomies, $args ) {
1155:   public function current_user_can_set_authors() {
1373:   public function filter_terms_clauses( $pieces ) {
1422:   public function filter_views( $views ) {
1519:   public function filter_user_has_cap( $allcaps, $caps, $args ) {
1573:   public function get_author_term( $coauthor ) {
1601:   public function update_author_term( $coauthor ) {

php/integrations/yoast.php
288:    public static function allow_indexing_guest_author_archive( $robots, $presentation ) {
317:    public static function fix_guest_author_archive_url_presenter( $url, $presenter ) {

php/integrations/yoast/class-coauthor.php
37: public function generate() {
63: public function generate_from_user_id( $user_id ) {
76: public function generate_from_guest_author( $guest_author ) {
95: protected function determine_user_id() {
GaryJones commented 2 months ago

I'm happy to add these types, but curious if there was a reason any of these were left out? Thanks!

@alecgeatches I used the PhpStorm inspection feature to find and update the return types (and the other changes). In this case, the inspection would have looked for @return tags in the function/method DocBlock, and if it was present (unlike your first example), and not a union type (like your second example, needs PHP 8.0), then it made the change.

You're right, in that more exist that could be manually added.

alecgeatches commented 2 months ago

@alecgeatches I used the PhpStorm inspection feature to find and update the return types (and the other changes). In this case, the inspection would have looked for @return tags in the function/method DocBlock, and if it was present (unlike your first example), and not a union type (like your second example, needs PHP 8.0), then it made the change.

Got it! I don't think we need to update every function here in that case, just trying to understand your method. I'm glad that a lot of that work was able to be done automatically.

I'd be happy to get this merged as-is for https://github.com/Automattic/Co-Authors-Plus/pull/1035 and the next release.