diff --git a/classes/abilities/class-abilities.php b/classes/abilities/class-abilities.php index 1e3298cb8..16af34762 100644 --- a/classes/abilities/class-abilities.php +++ b/classes/abilities/class-abilities.php @@ -157,6 +157,12 @@ public function register_abilities() { 'permission_callback' => [ $this, 'can_fix' ], 'execute_callback' => [ $this->recommendations, 'complete' ], 'readonly' => false, + // Most of what this applies is a settings change, but two + // recommendations trash WordPress's placeholder content. The + // annotation describes what the ability can do, not what a + // given call happens to do, so it is declared destructive and + // a client prompts before any of it runs. + 'destructive' => true, ] ) ); @@ -173,8 +179,9 @@ public function register_abilities() { * @return array */ private function ability_args( array $args ) { - $readonly = $args['readonly'] ?? true; - unset( $args['readonly'] ); + $readonly = $args['readonly'] ?? true; + $destructive = $args['destructive'] ?? false; + unset( $args['readonly'], $args['destructive'] ); return \array_merge( [ @@ -184,9 +191,7 @@ private function ability_args( array $args ) { 'show_in_rest' => true, 'annotations' => [ 'readonly' => $readonly, - // Nothing registered here deletes or overwrites content: - // the write ability changes settings from a fixed list. - 'destructive' => false, + 'destructive' => $destructive, 'idempotent' => true, ], ], diff --git a/classes/abilities/class-recommendation-fixes.php b/classes/abilities/class-recommendation-fixes.php index 0d4bcc8db..9c1826ffc 100644 --- a/classes/abilities/class-recommendation-fixes.php +++ b/classes/abilities/class-recommendation-fixes.php @@ -148,6 +148,70 @@ class Recommendation_Fixes { 'value' => false, 'summary' => 'Disable author feeds in All in One SEO.', ], + + /* + * Telling the plugin which existing page serves a given role. + * + * These do not create anything. The recommendation asks whether the site + * has an About page; this records the answer and, when the answer is + * yes, which page it is. + * + * The plugin deliberately does not try to find the page itself. Deciding + * that "About Emilia" at /about-us/ is the About page -- and that + * "Job opening" is not -- is a judgement about titles, slugs, navigation + * and content in whatever language the site is written in. A caller that + * can read the site's pages is far better at that than a title match + * would be, and a wrong guess made in PHP would be silent. + * + * So the division is: the caller identifies the page, and this verifies + * the ID names a real published page of an allowed type before writing. + */ + 'set-page-about' => [ + 'page_type' => 'about', + 'input' => 'value', + 'summary' => 'Record which existing page is the About page.', + ], + 'set-page-contact' => [ + 'page_type' => 'contact', + 'input' => 'value', + 'summary' => 'Record which existing page is the Contact page.', + ], + 'set-page-faq' => [ + 'page_type' => 'faq', + 'input' => 'value', + 'summary' => 'Record which existing page is the FAQ page.', + ], + + /* + * Deleting WordPress's own placeholder content. + * + * These are the only entries that remove something rather than change a + * setting, so they are the only ones annotated destructive. Two things + * make them defensible anyway: the target is not ambiguous -- the data + * collector resolves the specific post WordPress ships, by slug with a + * title fallback -- and the recommendation exists precisely because the + * site owner is being asked to delete it. + * + * They are trashed, not force-deleted. The dashboard's own JavaScript + * passes force=true and removes the post outright; an agent acting + * unattended should leave a way back, and the task's completion check + * passes either way because it looks for a published post. + * + * confirm_only keeps them out of next-mode, so a daily unattended run + * never deletes anything: they can only be applied by naming the + * provider explicitly. + */ + 'hello-world' => [ + 'delete' => 'post', + 'confirm_only' => true, + 'summary' => 'Move the default "Hello world!" post to the trash.', + ], + 'sample-page' => [ + 'delete' => 'page', + 'confirm_only' => true, + 'summary' => 'Move the default "Sample Page" to the trash.', + ], + 'aioseo-media-pages' => [ 'seo' => 'aioseo', // Attachment redirection lives under dynamicOptions, not options, and @@ -225,6 +289,14 @@ public static function apply( $provider_id, $value = null ) { ); } + if ( isset( $fix['page_type'] ) ) { + return self::apply_page_type( $fix, $value ); + } + + if ( isset( $fix['delete'] ) ) { + return self::apply_deletion( $provider_id ); + } + if ( isset( $fix['seo'] ) ) { return self::apply_seo_setting( $fix ); } @@ -246,6 +318,156 @@ public static function apply( $provider_id, $value = null ) { return true; } + /** + * Record which page serves a given role. + * + * The page type comes from the table; only the page ID comes from the + * caller, and it is checked before anything is written: it must name a post + * that exists, is published, and is of a post type the site treats as a + * page. An ID that does not pass is an error rather than a silent no-op, + * because "we recorded your About page" is worth being true. + * + * @param array $fix The fix definition. + * @param mixed $value The page ID supplied by the caller. + * + * @return true|\WP_Error + */ + private static function apply_page_type( array $fix, $value ) { + $page_id = \is_numeric( $value ) ? (int) $value : 0; + + if ( 1 > $page_id ) { + return new \WP_Error( + 'progress_planner_missing_page_id', + \__( 'This recommendation needs the ID of the page that serves this role.', 'progress-planner' ) + ); + } + + $page = \get_post( $page_id ); + + if ( ! $page ) { + return new \WP_Error( + 'progress_planner_no_such_page', + \__( 'There is no page with that ID.', 'progress-planner' ) + ); + } + + if ( 'publish' !== $page->post_status ) { + return new \WP_Error( + 'progress_planner_page_not_published', + \__( 'That page is not published, so it cannot serve this role yet.', 'progress-planner' ) + ); + } + + if ( ! \in_array( $page->post_type, self::page_post_types(), true ) ) { + return new \WP_Error( + 'progress_planner_not_a_page', + \__( 'That post is not a page.', 'progress-planner' ) + ); + } + + \progress_planner()->get_admin__page_settings()->set_page_values( + [ + (string) $fix['page_type'] => [ + 'id' => $page_id, + 'have_page' => 'yes', + ], + ] + ); + + return true; + } + + /** + * The post types that can serve a page role. + * + * Hierarchical public post types, which is what WordPress means by a page, + * rather than a hardcoded 'page' -- a site may serve these roles from a + * custom post type. + * + * @return array + */ + private static function page_post_types() { + $types = \get_post_types( + [ + 'public' => true, + 'hierarchical' => true, + ] + ); + + return \array_values( $types ); + } + + /** + * Move a placeholder post to the trash. + * + * The post ID comes from the provider's own data collector, which resolves + * WordPress's default content by slug. Nothing here takes an ID from the + * caller, so an ability argument cannot point this at arbitrary content. + * + * @param string $provider_id The provider ID. + * + * @return true|\WP_Error + */ + private static function apply_deletion( $provider_id ) { + $provider = \progress_planner()->get_suggested_tasks()->get_tasks_manager()->get_task_provider( $provider_id ); + + if ( ! $provider || ! \method_exists( $provider, 'get_data_collector' ) ) { + return new \WP_Error( + 'progress_planner_no_target', + \__( 'That recommendation is not available on this site.', 'progress-planner' ) + ); + } + + $post_id = (int) $provider->get_data_collector()->collect(); + + if ( ! $post_id || ! \get_post( $post_id ) ) { + return new \WP_Error( + 'progress_planner_no_target', + \__( 'The default content this recommendation refers to is no longer there.', 'progress-planner' ) + ); + } + + if ( ! \wp_trash_post( $post_id ) ) { + return new \WP_Error( + 'progress_planner_delete_failed', + \__( 'The content could not be moved to the trash.', 'progress-planner' ) + ); + } + + return true; + } + + /** + * Whether applying a fix removes content rather than changing a setting. + * + * Drives the destructive annotation, which is what a client shows a person + * before calling. + * + * @param string $provider_id The provider ID. + * + * @return bool + */ + public static function is_destructive( $provider_id ) { + $fix = self::get( $provider_id ); + + return null !== $fix && isset( $fix['delete'] ); + } + + /** + * Whether a fix may only be applied by naming its provider explicitly. + * + * Keeps anything that removes content out of an unattended run. + * + * @param string $provider_id The provider ID. + * + * @return bool + */ + public static function is_confirm_only( $provider_id ) { + $fix = self::get( $provider_id ); + + return null !== $fix && ! empty( $fix['confirm_only'] ); + } + /** * Apply a setting owned by an SEO plugin. * diff --git a/classes/abilities/class-recommendations.php b/classes/abilities/class-recommendations.php index 0bfbfa26e..cd8a90570 100644 --- a/classes/abilities/class-recommendations.php +++ b/classes/abilities/class-recommendations.php @@ -135,12 +135,23 @@ public function complete( $input = [] ) { // for work that did not happen. $completed = $this->is_satisfied( $provider, $task ); + // The wording distinguishes a settings change from a deletion: an agent + // relaying this to a person should not describe trashing a post as + // changing a setting. + if ( Recommendation_Fixes::is_destructive( $provider_id ) ) { + $message = $completed + ? \__( 'The content was moved to the trash and the recommendation is now satisfied. It can be restored from the trash if that was not intended.', 'progress-planner' ) + : \__( 'The content was moved to the trash, but the recommendation is not reported as satisfied yet.', 'progress-planner' ); + } else { + $message = $completed + ? \__( 'The setting was changed and the recommendation is now satisfied.', 'progress-planner' ) + : \__( 'The setting was changed, but the recommendation is not reported as satisfied yet.', 'progress-planner' ); + } + return $this->result( true, $completed ? 'completed' : 'applied_not_yet_complete', - $completed - ? \__( 'The setting was changed and the recommendation is now satisfied.', 'progress-planner' ) - : \__( 'The setting was changed, but the recommendation is not reported as satisfied yet.', 'progress-planner' ), + $message, $task ); } @@ -174,8 +185,25 @@ private function result( $applied, $status, $message, $task ) { * @return bool */ private function is_satisfied( $provider, $task ) { - return \method_exists( $provider, 'is_task_completed' ) - && (bool) $provider->is_task_completed( $task->get_task_id() ); + if ( \method_exists( $provider, 'is_task_completed' ) + && (bool) $provider->is_task_completed( $task->get_task_id() ) + ) { + return true; + } + + // Some providers answer through should_add_task() instead: the task + // exists precisely while the condition is unmet, so "would not be added + // now" means satisfied. The set-page providers are the case in point -- + // they do not override is_task_completed() at all. + // + // This can still report false immediately after a write. Page-type + // lookups are memoised in a static cache with no invalidation, so within + // one request the check may read state from before the change. That is + // why the status distinguishes "applied" from "satisfied" rather than + // assuming the two are the same: the next request sees it correctly, and + // a caller is told what actually happened either way. + return \method_exists( $provider, 'should_add_task' ) + && false === (bool) $provider->should_add_task(); } /** @@ -198,8 +226,13 @@ private function get_next_fixable_provider_id() { $provider_id = $task->get_provider_id(); // A fix needing a value cannot be chosen unattended: there is no - // correct tagline to invent on the site owner's behalf. - if ( ! Recommendation_Fixes::has_fix( $provider_id ) || Recommendation_Fixes::needs_value( $provider_id ) ) { + // correct tagline to invent on the site owner's behalf. Nor can one + // that removes content, however well scoped -- an unattended run + // should never be the thing that deleted something. + if ( ! Recommendation_Fixes::has_fix( $provider_id ) + || Recommendation_Fixes::needs_value( $provider_id ) + || Recommendation_Fixes::is_confirm_only( $provider_id ) + ) { continue; } @@ -285,6 +318,7 @@ private function prepare( $task ) { // recommendations it is allowed to apply. 'fixable' => Recommendation_Fixes::has_fix( $provider_id ), 'needs_value' => Recommendation_Fixes::needs_value( $provider_id ), + 'destructive' => Recommendation_Fixes::is_destructive( $provider_id ), ]; } } diff --git a/classes/abilities/class-schemas.php b/classes/abilities/class-schemas.php index d534ec944..d90809a8e 100644 --- a/classes/abilities/class-schemas.php +++ b/classes/abilities/class-schemas.php @@ -75,7 +75,7 @@ public static function complete_recommendation_input() { ], 'value' => [ 'type' => 'string', - 'description' => \__( 'The value to set, for recommendations that need one: the tagline text, a timezone identifier such as "Europe/Amsterdam", or a date format string. Recommendations with only one correct outcome ignore this.', 'progress-planner' ), + 'description' => \__( 'The value to set, for recommendations that need one: the tagline text, a timezone identifier such as "Europe/Amsterdam", a date format string, or -- for the recommendations that ask which page serves a role -- the ID of an existing published page. Recommendations with only one correct outcome ignore this. Check needs_value on a recommendation to see whether one is required.', 'progress-planner' ), ], ], ]; @@ -217,6 +217,10 @@ public static function recommendation() { 'type' => 'boolean', 'description' => \__( 'Whether applying it requires a value from the caller, such as the tagline text.', 'progress-planner' ), ], + 'destructive' => [ + 'type' => 'boolean', + 'description' => \__( 'Whether applying it removes content rather than changing a setting. These are only applied when named explicitly, never picked automatically.', 'progress-planner' ), + ], ], ]; } diff --git a/classes/class-suggested-tasks.php b/classes/class-suggested-tasks.php index 70542077b..cd1791c97 100644 --- a/classes/class-suggested-tasks.php +++ b/classes/class-suggested-tasks.php @@ -244,12 +244,121 @@ public function maybe_complete_task() { } } + /** + * The alphabet used for confirmation codes. + * + * Excludes characters that are easily confused when read aloud or retyped: + * 0/O, 1/I/L, 5/S, 8/B. A code is only useful if a person relays it + * correctly on the first try. + * + * @var string + */ + const CONFIRMATION_CODE_ALPHABET = '234679ACDEFGHJKMNPQRTUVWXYZ'; + + /** + * The length of a confirmation code. + * + * @var int + */ + const CONFIRMATION_CODE_LENGTH = 4; + + /** + * Generate a short confirmation code for a task. + * + * WHY THIS EXISTS, alongside the 32-character token below: + * + * Both prove the same thing -- that the email actually arrived, because + * the value only exists inside the delivered message. They differ in who + * can realistically relay them. + * + * The token is delivered as the href of a "Click here" link. A person + * cannot read it off the screen: they would have to right-click, copy the + * link address, and paste a 128-character URL. That is fine for clicking + * and useless for telling someone. + * + * This matters because email delivery is the one check a site cannot + * verify for itself, and it is increasingly answered through an AI + * assistant rather than the dashboard. An assistant with mailbox access + * reads the token and needs nothing from the user. An assistant without it + * has to ask, and "tell it the code from the email" only works if the code + * is short enough to say out loud. + * + * The code is deliberately weaker than the token: four characters from a + * 27-character alphabet is about 19 bits. What makes that acceptable is + * everything around it -- the code is single-use, expires in 24 hours, is + * scoped to one user and one task, is only accepted from a caller who + * already holds manage_options, and failed attempts are rate limited. An + * attacker who could satisfy all of that can complete the task by simply + * clicking a button in wp-admin, so the code is not the weak link. + * + * Codes are stored uppercase and compared case-insensitively, because a + * person retyping one should not have to think about it. + * + * @param string $task_id The task ID. + * @param int $user_id The user ID. + * + * @return string The generated confirmation code. + */ + public function generate_task_confirmation_code( $task_id, $user_id ) { + $alphabet = self::CONFIRMATION_CODE_ALPHABET; + $max = \strlen( $alphabet ) - 1; + $code = ''; + + for ( $i = 0; $i < self::CONFIRMATION_CODE_LENGTH; $i++ ) { + $code .= $alphabet[ \wp_rand( 0, $max ) ]; + } + + \set_transient( + 'prpl_confirm_' . $task_id . '_' . $user_id, + $code, + DAY_IN_SECONDS + ); + + return $code; + } + + /** + * Verify a task confirmation code. + * + * @param string $task_id The task ID. + * @param int $user_id The user ID. + * @param string $provided_code The code supplied by the caller. + * + * @return bool + */ + public function verify_task_confirmation_code( $task_id, $user_id, $provided_code ) { + $stored_code = \get_transient( 'prpl_confirm_' . $task_id . '_' . $user_id ); + + if ( ! $stored_code ) { + return false; // Expired, already used, or never issued. + } + + // hash_equals to keep the comparison constant-time; strtoupper because + // the code is meant to be retyped by a person. + return \hash_equals( (string) $stored_code, \strtoupper( \trim( $provided_code ) ) ); + } + + /** + * Delete a task confirmation code after use. + * + * @param string $task_id The task ID. + * @param int $user_id The user ID. + * + * @return bool True if deleted, false otherwise. + */ + public function delete_task_confirmation_code( $task_id, $user_id ) { + return \delete_transient( 'prpl_confirm_' . $task_id . '_' . $user_id ); + } + /** * Generate a secure token for task completion via email link. * * This token prevents CSRF attacks by ensuring only legitimate email * links can mark tasks as complete. * + * See generate_task_confirmation_code() above for why a second, shorter + * value exists next to this one. + * * @param string $task_id The task ID. * @param int $user_id The user ID. * diff --git a/classes/suggested-tasks/providers/class-email-sending.php b/classes/suggested-tasks/providers/class-email-sending.php index 50f9e3064..a9ca3e86d 100644 --- a/classes/suggested-tasks/providers/class-email-sending.php +++ b/classes/suggested-tasks/providers/class-email-sending.php @@ -110,6 +110,34 @@ public function init() { // $this->email_content is not read anywhere else. } + /** + * Build the test email body. + * + * The body carries two ways to confirm the message arrived: a link, which + * carries the full token and is what a person clicks, and a short code, + * which is what a person can read out to an AI assistant that has no + * access to their mailbox. Both prove the same thing -- that the message + * was delivered -- because neither value exists anywhere the recipient + * could reach without receiving it. See + * Suggested_Tasks::generate_task_confirmation_code() for the reasoning. + * + * @param string $token The completion token. + * @param int $user_id The user the token and code belong to. + * + * @return string + */ + protected function get_email_content( $token, $user_id ) { + $code = \progress_planner()->get_suggested_tasks()->generate_task_confirmation_code( $this->get_task_id(), $user_id ); + + return \sprintf( + /* translators: %1$s: the admin URL, %2$s: the assistant's name, %3$s: a short confirmation code. */ + \__( 'You just used Progress Planner to verify if sending email works on your website.

The good news; it does! Click here to mark %2$s\'s Recommendation as completed.

Using an AI assistant? Give it this confirmation code instead: %3$s', 'progress-planner' ), + \admin_url( 'admin.php?page=progress-planner&prpl_complete_task=' . $this->get_task_id() . '&token=' . $token ), + \esc_html( \progress_planner()->get_ui__branding()->get_ravi_name() ), + \esc_html( $code ) + ); + } + /** * Get the troubleshooting guide URL. * @@ -273,12 +301,7 @@ public function ajax_test_email_sending() { $user_id = \get_current_user_id(); $token = \progress_planner()->get_suggested_tasks()->generate_task_completion_token( $this->get_task_id(), $user_id ); - $email_content = \sprintf( - // translators: %1$s the admin URL. - \__( 'You just used Progress Planner to verify if sending email works on your website.

The good news; it does! Click here to mark %2$s\'s Recommendation as completed.', 'progress-planner' ), - \admin_url( 'admin.php?page=progress-planner&prpl_complete_task=' . $this->get_task_id() . '&token=' . $token ), - \esc_html( \progress_planner()->get_ui__branding()->get_ravi_name() ) - ); + $email_content = $this->get_email_content( $token, $user_id ); $headers = [ 'Content-Type: text/html; charset=UTF-8' ]; diff --git a/tests/phpunit/test-class-abilities.php b/tests/phpunit/test-class-abilities.php index 1fff245b8..59f68e78e 100644 --- a/tests/phpunit/test-class-abilities.php +++ b/tests/phpunit/test-class-abilities.php @@ -58,6 +58,24 @@ public function setUp(): void { $this->recommendations = new \Progress_Planner\Abilities\Recommendations(); } + /** + * Tear down test. + * + * Activities live in a custom table that WP_UnitTestCase does not roll + * back, and post IDs are reused across tests. A row left here would be seen + * by a later test that happens to be handed the same ID and asserts it has + * no activity, so this class clears what it caused. + * + * @return void + */ + public function tearDown(): void { + global $wpdb; + + $wpdb->query( 'TRUNCATE TABLE ' . $wpdb->prefix . 'progress_planner_activities' ); // phpcs:ignore WordPress.DB + + parent::tearDown(); + } + /** * Invoke a private or protected method on an object. * @@ -663,4 +681,187 @@ public function test_seo_fix_requires_its_plugin() { $this->assertWPError( $result ); $this->assertSame( 'progress_planner_seo_plugin_inactive', $result->get_error_code() ); } + + /** + * Test that the placeholder deletions are marked destructive. + * + * @return void + */ + public function test_placeholder_deletions_are_destructive() { + $this->assertTrue( Recommendation_Fixes::is_destructive( 'hello-world' ) ); + $this->assertTrue( Recommendation_Fixes::is_destructive( 'sample-page' ) ); + $this->assertFalse( Recommendation_Fixes::is_destructive( 'disable-comments' ) ); + } + + /** + * Test that anything destructive can only be applied by name. + * + * An unattended run must never be the thing that deleted something. + * + * @return void + */ + public function test_destructive_fixes_are_confirm_only() { + $this->assertTrue( Recommendation_Fixes::is_confirm_only( 'hello-world' ) ); + $this->assertTrue( Recommendation_Fixes::is_confirm_only( 'sample-page' ) ); + $this->assertFalse( Recommendation_Fixes::is_confirm_only( 'disable-comments' ) ); + } + + /** + * Test that applying the hello-world fix trashes the post rather than + * deleting it outright. + * + * @return void + */ + public function test_hello_world_fix_trashes_the_post() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + + $post_id = self::factory()->post->create( + [ + 'post_title' => 'Hello world!', + 'post_name' => 'hello-world', + 'post_status' => 'publish', + ] + ); + + $this->seed_task( 'hello-world' ); + + $result = $this->recommendations->complete( [ 'provider_id' => 'hello-world' ] ); + + $this->assertTrue( $result['applied'] ); + $this->assertSame( 'trash', \get_post_status( $post_id ), 'The post should be recoverable from the trash.' ); + } + + /** + * Test that next mode never picks a destructive fix. + * + * @return void + */ + public function test_next_mode_never_deletes() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + + $post_id = self::factory()->post->create( + [ + 'post_title' => 'Hello world!', + 'post_name' => 'hello-world', + 'post_status' => 'publish', + ] + ); + + $this->seed_task( 'hello-world' ); + + $result = $this->recommendations->complete( [] ); + + if ( null !== $result['task'] ) { + $this->assertNotSame( 'hello-world', $result['task']['provider_id'] ); + } + + $this->assertSame( 'publish', \get_post_status( $post_id ), 'Next mode must not trash anything.' ); + } + + /** + * Test that a deletion reports missing content rather than failing oddly. + * + * @return void + */ + public function test_deletion_reports_missing_target() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + + $result = Recommendation_Fixes::apply( 'hello-world' ); + + if ( \is_wp_error( $result ) ) { + $this->assertSame( 'progress_planner_no_target', $result->get_error_code() ); + } else { + $this->assertTrue( $result ); + } + } + + /** + * Test that the page-role recommendations need a page ID. + * + * @return void + */ + public function test_page_role_fixes_need_a_value() { + foreach ( [ 'set-page-about', 'set-page-contact', 'set-page-faq' ] as $provider_id ) { + $this->assertTrue( Recommendation_Fixes::has_fix( $provider_id ) ); + $this->assertTrue( Recommendation_Fixes::needs_value( $provider_id ) ); + $this->assertFalse( Recommendation_Fixes::is_destructive( $provider_id ) ); + } + } + + /** + * Test that a valid page is recorded as serving the role. + * + * @return void + */ + public function test_page_role_records_the_page() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + + $page_id = self::factory()->post->create( + [ + 'post_title' => 'About Us', + 'post_type' => 'page', + 'post_status' => 'publish', + ] + ); + + $this->assertTrue( Recommendation_Fixes::apply( 'set-page-about', (string) $page_id ) ); + + $slugs = \wp_get_object_terms( $page_id, 'progress_planner_page_types', [ 'fields' => 'slugs' ] ); + + $this->assertContains( 'about', (array) $slugs ); + } + + /** + * Test that an unusable page ID is refused rather than recorded. + * + * "We recorded your About page" is worth being true, so each of these is an + * error rather than a silent no-op. + * + * @return void + */ + public function test_page_role_refuses_unusable_ids() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + + $cases = [ + 'progress_planner_missing_page_id' => null, + 'progress_planner_no_such_page' => '99999999', + 'progress_planner_page_not_published' => (string) self::factory()->post->create( + [ + 'post_type' => 'page', + 'post_status' => 'draft', + ] + ), + 'progress_planner_not_a_page' => (string) self::factory()->post->create( + [ + 'post_type' => 'post', + 'post_status' => 'publish', + ] + ), + ]; + + foreach ( $cases as $expected_code => $value ) { + $result = Recommendation_Fixes::apply( 'set-page-about', $value ); + + $this->assertWPError( $result ); + $this->assertSame( $expected_code, $result->get_error_code() ); + } + } + + /** + * Test that next mode never picks a page-role fix. + * + * There is no correct page to choose on the owner's behalf. + * + * @return void + */ + public function test_next_mode_skips_page_role_fixes() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + $this->seed_task( 'set-page-about' ); + + $result = $this->recommendations->complete( [] ); + + $picked = $result['task']['provider_id'] ?? null; + + $this->assertNotSame( 'set-page-about', $picked ); + } } diff --git a/tests/phpunit/test-class-suggested-tasks.php b/tests/phpunit/test-class-suggested-tasks.php index fec2a15d4..6be4baf68 100644 --- a/tests/phpunit/test-class-suggested-tasks.php +++ b/tests/phpunit/test-class-suggested-tasks.php @@ -102,4 +102,91 @@ public function test_task_cleanup() { \wp_cache_flush_group( \Progress_Planner\Suggested_Tasks_DB::GET_TASKS_CACHE_GROUP ); // Clear the cache. $this->assertEquals( \count( $tasks_to_keep ), \count( \progress_planner()->get_suggested_tasks_db()->get_tasks_by( [ 'post_status' => 'publish' ] ) ) ); } + + /** + * Test that a confirmation code round-trips. + * + * @return void + */ + public function test_confirmation_code_verifies() { + $suggested_tasks = \progress_planner()->get_suggested_tasks(); + $code = $suggested_tasks->generate_task_confirmation_code( 'sending-email', 1 ); + + $this->assertSame( 4, \strlen( $code ) ); + $this->assertTrue( $suggested_tasks->verify_task_confirmation_code( 'sending-email', 1, $code ) ); + } + + /** + * Test that a code is accepted however a person retypes it. + * + * A code only has value if it survives being read aloud and typed back, + * so case and surrounding whitespace must not matter. + * + * @return void + */ + public function test_confirmation_code_is_forgiving_about_formatting() { + $suggested_tasks = \progress_planner()->get_suggested_tasks(); + $code = $suggested_tasks->generate_task_confirmation_code( 'sending-email', 1 ); + + $this->assertTrue( $suggested_tasks->verify_task_confirmation_code( 'sending-email', 1, \strtolower( $code ) ) ); + $this->assertTrue( $suggested_tasks->verify_task_confirmation_code( 'sending-email', 1, ' ' . $code . ' ' ) ); + } + + /** + * Test that a wrong code is rejected. + * + * @return void + */ + public function test_confirmation_code_rejects_a_wrong_code() { + $suggested_tasks = \progress_planner()->get_suggested_tasks(); + $suggested_tasks->generate_task_confirmation_code( 'sending-email', 1 ); + + $this->assertFalse( $suggested_tasks->verify_task_confirmation_code( 'sending-email', 1, 'ZZZZ' ) ); + } + + /** + * Test that a code belongs to one user and one task. + * + * @return void + */ + public function test_confirmation_code_is_scoped() { + $suggested_tasks = \progress_planner()->get_suggested_tasks(); + $code = $suggested_tasks->generate_task_confirmation_code( 'sending-email', 1 ); + + $this->assertFalse( $suggested_tasks->verify_task_confirmation_code( 'sending-email', 2, $code ) ); + $this->assertFalse( $suggested_tasks->verify_task_confirmation_code( 'other-task', 1, $code ) ); + } + + /** + * Test that a code cannot be reused once consumed. + * + * @return void + */ + public function test_confirmation_code_is_single_use() { + $suggested_tasks = \progress_planner()->get_suggested_tasks(); + $code = $suggested_tasks->generate_task_confirmation_code( 'sending-email', 1 ); + + $suggested_tasks->delete_task_confirmation_code( 'sending-email', 1 ); + + $this->assertFalse( $suggested_tasks->verify_task_confirmation_code( 'sending-email', 1, $code ) ); + } + + /** + * Test that codes avoid characters that are misread. + * + * @return void + */ + public function test_confirmation_code_avoids_ambiguous_characters() { + $suggested_tasks = \progress_planner()->get_suggested_tasks(); + + for ( $i = 0; $i < 40; $i++ ) { + $code = $suggested_tasks->generate_task_confirmation_code( 'sending-email', 1 ); + + $this->assertSame( + 0, + \preg_match( '/[01OILSB58]/', $code ), + "Code {$code} contains an easily confused character." + ); + } + } }