From 4d6b17e46e3fcb61ab12f15fb0101e74c01b2c6f Mon Sep 17 00:00:00 2001 From: Rob Skilling Date: Thu, 9 Jan 2025 12:34:36 +0000 Subject: [PATCH 1/4] Rewrite non-existent authors to our selected archive author Before: if a user had authored a post, but then the user had subsequently been deleted without their posts being re-assigned, this resulted in a blank author being displayed in the front-end, and the "Author" spinner in the block editor never resolving. Now: we can set an "archive_author" option in `wp_options`. This should be set to the ID of the author we want to default to in cases where the original author has been removed without re-assignment. When that option is set, the next time the post is viewed in the front-end, the author will be re-written to be the archive author. --- app/Theme/FixNonExistentAuthors.php | 49 +++++++ app/di.php | 1 + lib/byline.php | 1 + spec/theme/fix_not_existent_authors.spec.php | 136 +++++++++++++++++++ 4 files changed, 187 insertions(+) create mode 100644 app/Theme/FixNonExistentAuthors.php create mode 100644 spec/theme/fix_not_existent_authors.spec.php diff --git a/app/Theme/FixNonExistentAuthors.php b/app/Theme/FixNonExistentAuthors.php new file mode 100644 index 00000000..def50896 --- /dev/null +++ b/app/Theme/FixNonExistentAuthors.php @@ -0,0 +1,49 @@ +ID); + if ($post_author_id > 1) { + $post_author = get_user_by('id', $post_author_id); + if (!($post_author)) { + error_log("author of post {$post->ID} is deleted user $post_author_id", 0); + add_filter('wp_insert_post_data', [$this, 'setArchiveAuthor'], 99, 1); + wp_update_post($post); + } + } + } + + public function setArchiveAuthor($postData) + { + $fix_types = ["page", "post", "attachment"]; + + if (!in_array($postData['post_type'], $fix_types)) { + return $postData; + } + + $archive_user_option = get_option('archive_author'); + + if (!empty($archive_user_option)) { + if (is_int($archive_user_option)) { + $archive_author_id = $archive_user_option; + } elseif (is_string($archive_user_option) && ctype_digit($archive_user_option)) { + $archive_author_id = intval($archive_user_option); + } + if (!empty($archive_author_id)) { + $postData['post_author'] = $archive_author_id; + } + } + + return $postData; + } +} diff --git a/app/di.php b/app/di.php index bee76543..dcc9d9a2 100644 --- a/app/di.php +++ b/app/di.php @@ -26,3 +26,4 @@ )); $registrar->addInstance(new \GovUKBlogs\Theme\ThemeSetup()); $registrar->addInstance(new \GovUKBlogs\Theme\OldRootsCleanup()); +$registrar->addInstance(new \GovUKBlogs\Theme\FixNonExistentAuthors()); diff --git a/lib/byline.php b/lib/byline.php index 4d180621..8b5a59d3 100644 --- a/lib/byline.php +++ b/lib/byline.php @@ -3,6 +3,7 @@ # Display the author for a post, using coauthors if available and falling back to the WordPress user if not. function gds_byline() { + do_action('gds_byline'); ?> Posted by: fixNonExistentAuthors = new \GovUKBlogs\Theme\FixNonExistentAuthors(); + }); + + it('implements the Registerable interface', function () { + expect($this->fixNonExistentAuthors)->toBeAnInstanceOf(\Dxw\Iguana\Registerable::class); + }); + + describe('->register()', function () { + it('adds the action', function () { + allow('add_action')->toBeCalled(); + expect('add_action')->toBeCalled()->once()->with('gds_byline', [$this->fixNonExistentAuthors, 'replaceAbsentAuthor']); + $this->fixNonExistentAuthors->register(); + }); + }); + + describe('->replaceAbsentAuthor()', function () { + context('the post author ID is 1', function () { + it('does nothing', function () { + global $post; + $post = (object) [ + "ID" => 123 + ]; + allow('get_post_field')->toBeCalled()->andReturn(1); + expect('wp_update_post')->not->toBeCalled(); + + $this->fixNonExistentAuthors->replaceAbsentAuthor(); + }); + }); + context('the post author ID is greater than 1', function () { + context('but the user exists', function () { + it('does nothing', function () { + global $post; + $post = (object) [ + "ID" => 123 + ]; + allow('get_post_field')->toBeCalled()->andReturn(2); + allow('get_user_by')->toBeCalled()->andReturn((object) [ + "ID" => 2, + "user_login" => "valid_user" + ]); + expect('wp_update_post')->not->toBeCalled(); + + $this->fixNonExistentAuthors->replaceAbsentAuthor(); + }); + }); + context('and the user does not exist', function () { + it('adds the filter and calls update_post', function () { + global $post; + $post = (object) [ + "ID" => 123 + ]; + allow('get_post_field')->toBeCalled()->andReturn(2); + allow('get_user_by')->toBeCalled()->andReturn(false); + allow('add_filter')->toBeCalled(); + allow('error_log')->toBeCalled(); + expect('error_log')->toBeCalled()->once()->with('author of post 123 is deleted user 2', 0); + expect('add_filter')->toBeCalled()->once()->with('wp_insert_post_data', [$this->fixNonExistentAuthors, 'setArchiveAuthor'], 99, 1); + allow('wp_update_post')->toBeCalled(); + expect('wp_update_post')->toBeCalled()->once(); + + $this->fixNonExistentAuthors->replaceAbsentAuthor(); + }); + }); + }); + }); + + describe('->setArchiveAuthor()', function () { + context('the post is not one of the types we want to fix', function () { + it('returns the post data unamended', function () { + $postData = [ + 'post_type' => 'custom_post_type', + 'post_author' => 123 + ]; + expect('get_option')->not->toBeCalled(); + + $result = $this->fixNonExistentAuthors->setArchiveAuthor($postData); + + expect($result)->toEqual($postData); + }); + }); + context('the post is one of the type we want to fix', function () { + beforeEach(function () { + $this->postData = [ + 'post_type' => 'post', + 'post_author' => 123 + ]; + }); + context('but the archive_author option is not set', function () { + it('returns the post data unamended', function () { + allow('get_option')->toBeCalled()->andReturn(false); + + $result = $this->fixNonExistentAuthors->setArchiveAuthor($this->postData); + + expect($result)->toEqual($this->postData); + }); + }); + context('and the archive_author option is an integer', function () { + it('amends the post data to set the post_author to the archive_author value', function () { + allow('get_option')->toBeCalled()->andReturn(456); + + $result = $this->fixNonExistentAuthors->setArchiveAuthor($this->postData); + + expect($result)->toEqual([ + 'post_type' => 'post', + 'post_author' => 456 + ]); + }); + }); + context('and the archive_author option is a string containing only an integer', function () { + it('amends the post data to set the post_author to the archive_author value', function () { + allow('get_option')->toBeCalled()->andReturn('456'); + + $result = $this->fixNonExistentAuthors->setArchiveAuthor($this->postData); + + expect($result)->toEqual([ + 'post_type' => 'post', + 'post_author' => 456 + ]); + }); + }); + context('and the archive_author option is a string containing non-numeric characters', function () { + it('returns the post data unamended', function () { + allow('get_option')->toBeCalled()->andReturn('456foo'); + + $result = $this->fixNonExistentAuthors->setArchiveAuthor($this->postData); + + expect($result)->toEqual($this->postData); + }); + }); + }); + }); +}); From 738cbd55804910de7a895734985e1d939d0465c5 Mon Sep 17 00:00:00 2001 From: Rob Skilling Date: Thu, 9 Jan 2025 14:42:15 +0000 Subject: [PATCH 2/4] Account for Co-Authors relationships when adjusting for deleted authors Before: when we re-allocated the post author if the existing post author no longer existed, we just changed the author id attached to the post. This fixed the author display in the front-end, but did not resolve the ever-spinning circle in the back-end, as if co-authors is active, co-author relationships (defined by taxonomy term relationships) was still in place. Now: if we have amended the post author ID, we also remove any co-author term relationships with the post in question. This means the newly allocated post author now displays in both the front and back-ends. --- app/Theme/FixNonExistentAuthors.php | 8 ++++-- spec/theme/fix_not_existent_authors.spec.php | 29 ++++++++++++++++---- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/app/Theme/FixNonExistentAuthors.php b/app/Theme/FixNonExistentAuthors.php index def50896..243bbef6 100644 --- a/app/Theme/FixNonExistentAuthors.php +++ b/app/Theme/FixNonExistentAuthors.php @@ -17,13 +17,13 @@ public function replaceAbsentAuthor() $post_author = get_user_by('id', $post_author_id); if (!($post_author)) { error_log("author of post {$post->ID} is deleted user $post_author_id", 0); - add_filter('wp_insert_post_data', [$this, 'setArchiveAuthor'], 99, 1); + add_filter('wp_insert_post_data', [$this, 'setArchiveAuthor'], 99, 2); wp_update_post($post); } } } - public function setArchiveAuthor($postData) + public function setArchiveAuthor($postData, $postArray) { $fix_types = ["page", "post", "attachment"]; @@ -41,9 +41,11 @@ public function setArchiveAuthor($postData) } if (!empty($archive_author_id)) { $postData['post_author'] = $archive_author_id; + if (taxonomy_exists('author')) { + wp_delete_object_term_relationships($postArray['ID'], 'author'); + } } } - return $postData; } } diff --git a/spec/theme/fix_not_existent_authors.spec.php b/spec/theme/fix_not_existent_authors.spec.php index ce873490..38d23793 100644 --- a/spec/theme/fix_not_existent_authors.spec.php +++ b/spec/theme/fix_not_existent_authors.spec.php @@ -58,7 +58,7 @@ allow('add_filter')->toBeCalled(); allow('error_log')->toBeCalled(); expect('error_log')->toBeCalled()->once()->with('author of post 123 is deleted user 2', 0); - expect('add_filter')->toBeCalled()->once()->with('wp_insert_post_data', [$this->fixNonExistentAuthors, 'setArchiveAuthor'], 99, 1); + expect('add_filter')->toBeCalled()->once()->with('wp_insert_post_data', [$this->fixNonExistentAuthors, 'setArchiveAuthor'], 99, 2); allow('wp_update_post')->toBeCalled(); expect('wp_update_post')->toBeCalled()->once(); @@ -77,7 +77,7 @@ ]; expect('get_option')->not->toBeCalled(); - $result = $this->fixNonExistentAuthors->setArchiveAuthor($postData); + $result = $this->fixNonExistentAuthors->setArchiveAuthor($postData, []); expect($result)->toEqual($postData); }); @@ -93,7 +93,7 @@ it('returns the post data unamended', function () { allow('get_option')->toBeCalled()->andReturn(false); - $result = $this->fixNonExistentAuthors->setArchiveAuthor($this->postData); + $result = $this->fixNonExistentAuthors->setArchiveAuthor($this->postData, []); expect($result)->toEqual($this->postData); }); @@ -101,8 +101,9 @@ context('and the archive_author option is an integer', function () { it('amends the post data to set the post_author to the archive_author value', function () { allow('get_option')->toBeCalled()->andReturn(456); + allow('taxonomy_exists')->toBeCalled()->andReturn(false); - $result = $this->fixNonExistentAuthors->setArchiveAuthor($this->postData); + $result = $this->fixNonExistentAuthors->setArchiveAuthor($this->postData, []); expect($result)->toEqual([ 'post_type' => 'post', @@ -113,8 +114,9 @@ context('and the archive_author option is a string containing only an integer', function () { it('amends the post data to set the post_author to the archive_author value', function () { allow('get_option')->toBeCalled()->andReturn('456'); + allow('taxonomy_exists')->toBeCalled()->andReturn(false); - $result = $this->fixNonExistentAuthors->setArchiveAuthor($this->postData); + $result = $this->fixNonExistentAuthors->setArchiveAuthor($this->postData, []); expect($result)->toEqual([ 'post_type' => 'post', @@ -126,11 +128,26 @@ it('returns the post data unamended', function () { allow('get_option')->toBeCalled()->andReturn('456foo'); - $result = $this->fixNonExistentAuthors->setArchiveAuthor($this->postData); + $result = $this->fixNonExistentAuthors->setArchiveAuthor($this->postData, []); expect($result)->toEqual($this->postData); }); }); + context('and the "author" taxonomy exists, indicating that co-authors is in use', function () { + it('removes any existing co-author relationships with this post, as well as amending the author ID', function () { + allow('get_option')->toBeCalled()->andReturn(456); + allow('taxonomy_exists')->toBeCalled()->andReturn(true); + allow('wp_delete_object_term_relationships')->toBeCalled(); + expect('wp_delete_object_term_relationships')->toBeCalled()->once()->with(123, 'author'); + + $result = $this->fixNonExistentAuthors->setArchiveAuthor($this->postData, ['ID' => 123]); + + expect($result)->toEqual([ + 'post_type' => 'post', + 'post_author' => 456 + ]); + }); + }); }); }); }); From da37d00b89099b04ad5a4b7bb961e6f6e7b221c6 Mon Sep 17 00:00:00 2001 From: Rob Skilling Date: Thu, 9 Jan 2025 14:46:33 +0000 Subject: [PATCH 3/4] Update CHANGELOG --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cc461fd0..ad33e6b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed + +- If a post has an author that no longer exists, the authorship of that post is re-allocated to the user with ID matching the `archive_author` option (if that options exists) + ## [6.0.2] - 2024-12-13 ### Fixed From cf0b5a8b6044ee05bec5f1ca949ed399c102d955 Mon Sep 17 00:00:00 2001 From: Rob Skilling Date: Thu, 9 Jan 2025 16:50:00 +0000 Subject: [PATCH 4/4] Store the `archive_author` as a network option Therefore, it only needs to be set once for all sites, rather than once per subsite. It is stored in the `wp_sitemeta` table. --- app/Theme/FixNonExistentAuthors.php | 2 +- spec/theme/fix_not_existent_authors.spec.php | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/Theme/FixNonExistentAuthors.php b/app/Theme/FixNonExistentAuthors.php index 243bbef6..6e9f16aa 100644 --- a/app/Theme/FixNonExistentAuthors.php +++ b/app/Theme/FixNonExistentAuthors.php @@ -31,7 +31,7 @@ public function setArchiveAuthor($postData, $postArray) return $postData; } - $archive_user_option = get_option('archive_author'); + $archive_user_option = get_network_option(null, 'archive_author'); if (!empty($archive_user_option)) { if (is_int($archive_user_option)) { diff --git a/spec/theme/fix_not_existent_authors.spec.php b/spec/theme/fix_not_existent_authors.spec.php index 38d23793..397d2ef4 100644 --- a/spec/theme/fix_not_existent_authors.spec.php +++ b/spec/theme/fix_not_existent_authors.spec.php @@ -75,7 +75,7 @@ 'post_type' => 'custom_post_type', 'post_author' => 123 ]; - expect('get_option')->not->toBeCalled(); + expect('get_network_option')->not->toBeCalled(); $result = $this->fixNonExistentAuthors->setArchiveAuthor($postData, []); @@ -91,7 +91,7 @@ }); context('but the archive_author option is not set', function () { it('returns the post data unamended', function () { - allow('get_option')->toBeCalled()->andReturn(false); + allow('get_network_option')->toBeCalled()->andReturn(false); $result = $this->fixNonExistentAuthors->setArchiveAuthor($this->postData, []); @@ -100,7 +100,7 @@ }); context('and the archive_author option is an integer', function () { it('amends the post data to set the post_author to the archive_author value', function () { - allow('get_option')->toBeCalled()->andReturn(456); + allow('get_network_option')->toBeCalled()->andReturn(456); allow('taxonomy_exists')->toBeCalled()->andReturn(false); $result = $this->fixNonExistentAuthors->setArchiveAuthor($this->postData, []); @@ -113,7 +113,7 @@ }); context('and the archive_author option is a string containing only an integer', function () { it('amends the post data to set the post_author to the archive_author value', function () { - allow('get_option')->toBeCalled()->andReturn('456'); + allow('get_network_option')->toBeCalled()->andReturn('456'); allow('taxonomy_exists')->toBeCalled()->andReturn(false); $result = $this->fixNonExistentAuthors->setArchiveAuthor($this->postData, []); @@ -126,7 +126,7 @@ }); context('and the archive_author option is a string containing non-numeric characters', function () { it('returns the post data unamended', function () { - allow('get_option')->toBeCalled()->andReturn('456foo'); + allow('get_network_option')->toBeCalled()->andReturn('456foo'); $result = $this->fixNonExistentAuthors->setArchiveAuthor($this->postData, []); @@ -135,7 +135,7 @@ }); context('and the "author" taxonomy exists, indicating that co-authors is in use', function () { it('removes any existing co-author relationships with this post, as well as amending the author ID', function () { - allow('get_option')->toBeCalled()->andReturn(456); + allow('get_network_option')->toBeCalled()->andReturn(456); allow('taxonomy_exists')->toBeCalled()->andReturn(true); allow('wp_delete_object_term_relationships')->toBeCalled(); expect('wp_delete_object_term_relationships')->toBeCalled()->once()->with(123, 'author');