From 0c05404775e1930fdc2c57a5b09940f06ef16c5d Mon Sep 17 00:00:00 2001 From: frantorres Date: Sat, 17 Oct 2026 21:16:39 +0200 Subject: [PATCH 1/3] FreeScout: API text search and plugin ID indexing WPOrgSite: `GET /api/conversations` takes `body`, listing the conversations with a published email, reply or note containing the text, narrowed by `bodyThreadType`, `bodySince` and `bodyBefore`. The other parameters pick the conversations first, their threads are counted from the index and then read in chunks, and searches over 15,000 threads or 25,000 conversations are refused. WPOrgPluginReview: the review email index keeps each email's plugin ID, from its Review ID line, and `GET /api/conversations` takes `reviewPluginId` to list a plugin's conversations. The migration queues a batched backfill of the rows indexed before (`wporgpluginreview:index --plugin-ids`). Co-Authored-By: Claude Opus 5.5 --- freescout.wordpress.net/AGENTS.md | 4 +- .../Console/IndexReviews.php | 17 +- ...plugin_id_to_wporgpluginreview_reviews.php | 66 ++++ .../WPOrgPluginReview/Jobs/IndexPluginIds.php | 121 +++++++ .../WPOrgPluginReviewServiceProvider.php | 4 + .../Modules/WPOrgPluginReview/README.md | 8 +- .../Services/PluginConversations.php | 95 +++++ .../WPOrgPluginReview/Services/Review.php | 2 + .../tests/PluginConversationsTest.php | 232 ++++++++++++ .../Providers/WPOrgSiteServiceProvider.php | 4 + .../Modules/WPOrgSite/README.md | 10 + .../Services/ConversationTextSearch.php | 331 ++++++++++++++++++ .../tests/ConversationTextSearchTest.php | 271 ++++++++++++++ 13 files changed, 1156 insertions(+), 9 deletions(-) create mode 100644 freescout.wordpress.net/Modules/WPOrgPluginReview/Database/Migrations/2026_10_18_000000_add_plugin_id_to_wporgpluginreview_reviews.php create mode 100644 freescout.wordpress.net/Modules/WPOrgPluginReview/Jobs/IndexPluginIds.php create mode 100644 freescout.wordpress.net/Modules/WPOrgPluginReview/Services/PluginConversations.php create mode 100644 freescout.wordpress.net/Modules/WPOrgPluginReview/tests/PluginConversationsTest.php create mode 100644 freescout.wordpress.net/Modules/WPOrgSite/Services/ConversationTextSearch.php create mode 100644 freescout.wordpress.net/Modules/WPOrgSite/tests/ConversationTextSearchTest.php diff --git a/freescout.wordpress.net/AGENTS.md b/freescout.wordpress.net/AGENTS.md index 4e8cb10bf9..2043c4e6aa 100644 --- a/freescout.wordpress.net/AGENTS.md +++ b/freescout.wordpress.net/AGENTS.md @@ -11,10 +11,10 @@ Context for AI coding agents and developers working on the WordPress.org FreeSco - `Modules//` — one directory per module (`module.json`, `Providers/`, `Http/`, `Resources/`, `Database/Migrations/`, `Public/`, `tests/`). - `WPOrgAkismet` — checks new conversations from senders with Akismet (`rest.akismet.com`, its own key), and reports agents' corrections. - `WPOrgHelpScoutImport` — imports HelpScout mailboxes' conversations, with their tags and custom fields, and their saved replies, from HelpScout's API, page by page on the queue, without the events live email fires, and points WordPress.org's copy of conversations at the imported ones as each mailbox switches. Administrators run it under Manage » HelpScout Import. Remove it once every mailbox has moved. - - `WPOrgPluginReview` — the latest plugin review of a conversation in its sidebar, for the plugins team: the details, flags, and issues of its review emails' Review ID lines. It indexes which replies are review emails, and of what type; `php artisan wporgpluginreview:index` indexes them again. Only in the plugins team's mailbox, and, with the Teams module, only for members of the teams whose names start with "Plugin". Issue names and replies to copy are in its `Services/IssueNames.php` and `Services/FlagReplies.php`. + - `WPOrgPluginReview` — the latest plugin review of a conversation in its sidebar, for the plugins team: the details, flags, and issues of its review emails' Review ID lines. It indexes which replies are review emails, and of what type; `php artisan wporgpluginreview:index` indexes them again. Only in the plugins team's mailbox, and, with the Teams module, only for members of the teams whose names start with "Plugin". Issue names and replies to copy are in its `Services/IssueNames.php` and `Services/FlagReplies.php`. The index keeps each review email's plugin ID too, which the API's conversation list takes as `reviewPluginId`. - `WPOrgSearch` — gives searches without other filters core's After filter, a year ago unless the agent picks a date, and runs core's conversation search through its `search.conversations.perform` hook in one query with `COUNT(*) OVER()`, stopped after 20 seconds on MariaDB. - `WPOrgSidebar` — WordPress.org panels in the conversation sidebar, loaded over AJAX from `api.wordpress.org/dotorg/freescout/`. - - `WPOrgSite` — tweaks for how WordPress.org runs FreeScout, rather than features; add new ones here instead of starting a module. So far: the WordPress mark, a Modules page that lists only installed modules and neither updates nor deletes them, and conversation UX (keyboard shortcuts, `/` for saved replies, a collapsible sidebar, pinned notes and headers, an alternate design), each documented in its README along with the core gotchas they work around (`#app`'s `overflow: hidden`, Summernote's caret, core's global `scrollTo()`). + - `WPOrgSite` — tweaks for how WordPress.org runs FreeScout, rather than features; add new ones here instead of starting a module. So far: the WordPress mark, a Modules page that lists only installed modules and neither updates nor deletes them, and conversation UX (keyboard shortcuts, `/` for saved replies, a collapsible sidebar, pinned notes and headers, an alternate design), each documented in its README along with the core gotchas they work around (`#app`'s `overflow: hidden`, Summernote's caret, core's global `scrollTo()`). It also lets the API's conversation list search threads' text (`body`, bounded to reading 15,000 threads a search, as thread bodies can't be indexed). - `WPOrgSSO` — logs agents in through login.wordpress.org's SAML identity provider (wp-saml-idp, in the private dotorg repository), and connects every user to a WordPress.org account. Its SAML library is committed in its `vendor/`: FreeScout doesn't install module dependencies. After changing its `composer.json`, run `composer install --no-dev` in the module and commit `vendor/` (`git add -f`: the root `.gitignore` ignores it). - `WPOrgWebhooks` — queues conversation events to `api.wordpress.org/dotorg/freescout/webhook.php`, which records contributor stats, and keeps the copy of conversations (`wporg_helpscout`) the plugin directory reads. - `tests/` — shared PHPUnit bootstrap and base `TestCase`. diff --git a/freescout.wordpress.net/Modules/WPOrgPluginReview/Console/IndexReviews.php b/freescout.wordpress.net/Modules/WPOrgPluginReview/Console/IndexReviews.php index 5501494674..e39b4be39e 100644 --- a/freescout.wordpress.net/Modules/WPOrgPluginReview/Console/IndexReviews.php +++ b/freescout.wordpress.net/Modules/WPOrgPluginReview/Console/IndexReviews.php @@ -10,11 +10,13 @@ namespace Modules\WPOrgPluginReview\Console; use Illuminate\Console\Command; +use Modules\WPOrgPluginReview\Jobs\IndexPluginIds as IndexPluginIdsJob; use Modules\WPOrgPluginReview\Jobs\IndexReviews as IndexReviewsJob; /** * Brings the index of review emails up to date with every reply of the plugins team's mailbox, like after the module - * was switched off for a while, when their threads' hooks didn't run. + * was switched off for a while, when their threads' hooks didn't run. With `--plugin-ids`, only the review emails indexed + * before the index had their plugin IDs are read again. */ final class IndexReviews extends Command { @@ -23,7 +25,7 @@ final class IndexReviews extends Command { * * @var string */ - protected $signature = 'wporgpluginreview:index {--now : Index in this process instead of on the queue}'; + protected $signature = 'wporgpluginreview:index {--now : Index in this process instead of on the queue} {--plugin-ids : Only read again the review emails indexed without their plugin ID}'; /** * Command description. @@ -38,19 +40,22 @@ final class IndexReviews extends Command { * @return int Exit code. */ public function handle(): int { + $plugin_ids = (bool) $this->option( 'plugin-ids' ); + $job = $plugin_ids ? IndexPluginIdsJob::class : IndexReviewsJob::class; + if ( ! $this->option( 'now' ) ) { - IndexReviewsJob::dispatch(); - $this->line( 'Indexing the review emails on the queue.' ); + $job::dispatch(); + $this->line( $plugin_ids ? 'Indexing the plugin IDs of the review emails on the queue.' : 'Indexing the review emails on the queue.' ); return 0; } $last = 0; do { - $last = ( new IndexReviewsJob( $last ) )->index_batch(); + $last = ( new $job( $last ) )->index_batch(); } while ( null !== $last ); - $this->line( 'Indexed the review emails.' ); + $this->line( $plugin_ids ? 'Indexed the plugin IDs of the review emails.' : 'Indexed the review emails.' ); return 0; } diff --git a/freescout.wordpress.net/Modules/WPOrgPluginReview/Database/Migrations/2026_10_18_000000_add_plugin_id_to_wporgpluginreview_reviews.php b/freescout.wordpress.net/Modules/WPOrgPluginReview/Database/Migrations/2026_10_18_000000_add_plugin_id_to_wporgpluginreview_reviews.php new file mode 100644 index 0000000000..d934fa9a19 --- /dev/null +++ b/freescout.wordpress.net/Modules/WPOrgPluginReview/Database/Migrations/2026_10_18_000000_add_plugin_id_to_wporgpluginreview_reviews.php @@ -0,0 +1,66 @@ +unsignedInteger( 'plugin_id' )->nullable(); + $table->index( array( 'plugin_id', 'conversation_id' ) ); + } + ); + + // The review emails indexed already are read again on the queue, in batches; new ones get it as they're indexed. + try { + IndexPluginIds::dispatch(); + } catch ( \Throwable $e ) { + \Log::error( '[WPOrgPluginReview] Could not queue indexing the plugin IDs of the review emails: ' . $e->getMessage() ); + } + } + + /** + * Reverts the migration. + * + * @return void + */ + public function down(): void { + if ( ! Schema::hasTable( 'wporgpluginreview_reviews' ) || ! Schema::hasColumn( 'wporgpluginreview_reviews', 'plugin_id' ) ) { + return; + } + + Schema::table( + 'wporgpluginreview_reviews', + static function ( Blueprint $table ): void { + $table->dropIndex( array( 'plugin_id', 'conversation_id' ) ); + $table->dropColumn( 'plugin_id' ); + } + ); + } +} diff --git a/freescout.wordpress.net/Modules/WPOrgPluginReview/Jobs/IndexPluginIds.php b/freescout.wordpress.net/Modules/WPOrgPluginReview/Jobs/IndexPluginIds.php new file mode 100644 index 0000000000..b933b70813 --- /dev/null +++ b/freescout.wordpress.net/Modules/WPOrgPluginReview/Jobs/IndexPluginIds.php @@ -0,0 +1,121 @@ +after_id = $after_id; + } + + /** + * Indexes the next batch of rows, and queues the one after. + * + * @return void + */ + public function handle(): void { + $last = $this->index_batch(); + if ( null !== $last ) { + self::dispatch( $last ); + } + } + + /** + * Indexes the next batch of rows without a plugin ID. + * + * The rows are walked by their ID rather than picked again by their missing plugin ID, so a row that can't be read + * doesn't stop the ones after it. + * + * @return int|null ID of the last row read, to start the next batch after; null once there are no more. + */ + public function index_batch(): ?int { + $last = $this->after_id; + + for ( $chunk = 0; $chunk < self::CHUNKS_PER_JOB; $chunk++ ) { + $rows = \DB::table( Review::TABLE ) + ->where( 'id', '>', $last ) + ->whereNull( 'plugin_id' ) + ->orderBy( 'id' ) + ->limit( self::CHUNK ) + ->get( array( 'id', 'thread_id' ) ); + + if ( $rows->isEmpty() ) { + return null; + } + + $threads = Thread::query()->whereIn( 'id', $rows->pluck( 'thread_id' )->map( 'intval' )->all() )->get()->keyBy( 'id' ); + + foreach ( $rows as $row ) { + $thread = $threads->get( (int) $row->thread_id ); + if ( $thread ) { + Review::index_thread( $thread ); + } else { + // Its thread is gone without its hooks, like core deleting a conversation's threads with a query. + \DB::table( Review::TABLE )->where( 'id', $row->id )->delete(); + } + $last = (int) $row->id; + } + + if ( $rows->count() < self::CHUNK ) { + return null; + } + } + + return $last; + } +} diff --git a/freescout.wordpress.net/Modules/WPOrgPluginReview/Providers/WPOrgPluginReviewServiceProvider.php b/freescout.wordpress.net/Modules/WPOrgPluginReview/Providers/WPOrgPluginReviewServiceProvider.php index 0b3a40b2af..70ffbcf3c0 100644 --- a/freescout.wordpress.net/Modules/WPOrgPluginReview/Providers/WPOrgPluginReviewServiceProvider.php +++ b/freescout.wordpress.net/Modules/WPOrgPluginReview/Providers/WPOrgPluginReviewServiceProvider.php @@ -20,6 +20,7 @@ use Modules\WPOrgPluginReview\Console\IndexReviews; use Modules\WPOrgPluginReview\Services\FlagReplies; use Modules\WPOrgPluginReview\Services\OpenOrder; +use Modules\WPOrgPluginReview\Services\PluginConversations; use Modules\WPOrgPluginReview\Services\Review; use Modules\WPOrgPluginReview\Services\Reviewers; use Modules\WPOrgPluginReview\Services\Subject; @@ -177,6 +178,9 @@ static function ( mixed $conversation = null ): void { self::PANEL_PRIORITY ); + // The API & Webhooks module's conversation list, by the plugin of their review emails. + \Eventy::addFilter( 'api.conversations.query', array( PluginConversations::class, 'filter' ), 20, 2 ); + self::register_subject_hooks(); self::register_folder_hooks(); self::register_profile_hooks(); diff --git a/freescout.wordpress.net/Modules/WPOrgPluginReview/README.md b/freescout.wordpress.net/Modules/WPOrgPluginReview/README.md index 6b0bacbf06..24e03235ec 100644 --- a/freescout.wordpress.net/Modules/WPOrgPluginReview/README.md +++ b/freescout.wordpress.net/Modules/WPOrgPluginReview/README.md @@ -83,6 +83,12 @@ Issues are listed by the short names the team uses, in `Services/IssueNames.php` ## The index -The module keeps the table `wporgpluginreview_reviews`: which replies are review emails or follow-ups, and of what type, from their Review ID lines. Replies are indexed as they're published, changed (including when merging moves them to another conversation), and deleted, and a conversation's again when it moves to another mailbox, as that doesn't change its replies. Conversations deleted for good leave the index. The sidebar reads the latest review from the index; it reads a conversation in full, and indexes it again, when the latest rows' replies changed without their events, and reads the newer replies that may hold a Review ID but aren't indexed. +The module keeps the table `wporgpluginreview_reviews`: which replies are review emails or follow-ups, of what type, and about which plugin (the plugin ID between `P0TDX` and `HGN`, or 0 when the line names none), from their Review ID lines. Replies are indexed as they're published, changed (including when merging moves them to another conversation), and deleted, and a conversation's again when it moves to another mailbox, as that doesn't change its replies. Conversations deleted for good leave the index. The sidebar reads the latest review from the index; it reads a conversation in full, and indexes it again, when the latest rows' replies changed without their events, and reads the newer replies that may hold a Review ID but aren't indexed. Switching the module on for the first time runs its migration, which queues indexing the review emails FreeScout already has. After it was switched off for a while, or after a HelpScout import, index them again with `php artisan wporgpluginreview:index` (`--now` to do it in the command rather than on the queue). + +The plugin IDs came later. Their migration adds the column and queues reading the rows indexed before it again, in batches: 200 rows at a time, 25 batches per job, each job queuing the next. Only rows are read, so only review emails; replies without a Review ID line were never in the index. The rows whose reply isn't a review email any more, or is gone, leave the index. Until a row is read again, its plugin ID is empty, and lookups don't find it. `php artisan wporgpluginreview:index --plugin-ids` (with `--now`, in the command) does it again; it only reads the rows still without one. New review emails get it as they're indexed, in the same request that publishes them. + +### Looking up a plugin's conversations + +With the API & Webhooks module, `GET /api/conversations` takes a `reviewPluginId` parameter: only the conversations with a review email about that plugin are listed, whatever their status, age, or who wrote to them, combined with the module's other parameters (`status`, `tag`…). The plugins team's review tools find the conversation of the plugin they review this way: any conversation for a first review, so a colleague's isn't started again, and `status=active,pending` for the next ones. It's an indexed lookup, a few milliseconds however many emails the mailbox has, unlike searching the emails' text (WPOrgSite's `body`). A value that isn't a plugin ID answers 400, and an index without the column yet answers 503: an empty list would read as "this plugin has no conversation", and the review tools would start one. Code in `Services/PluginConversations.php`. diff --git a/freescout.wordpress.net/Modules/WPOrgPluginReview/Services/PluginConversations.php b/freescout.wordpress.net/Modules/WPOrgPluginReview/Services/PluginConversations.php new file mode 100644 index 0000000000..91484bc3a8 --- /dev/null +++ b/freescout.wordpress.net/Modules/WPOrgPluginReview/Services/PluginConversations.php @@ -0,0 +1,95 @@ +input( self::PARAMETER ); + + if ( null === $plugin_id || '' === $plugin_id ) { + return $query; + } + + // Refused rather than ignored: an ignored filter would list every conversation as the plugin's. + if ( ! is_scalar( $plugin_id ) || ! ctype_digit( (string) $plugin_id ) || (int) $plugin_id <= 0 ) { + abort( 400, 'The `' . self::PARAMETER . '` parameter takes a plugin ID, a positive number.' ); + } + + if ( ! self::ready() ) { + abort( 503, 'The plugin IDs of the review emails aren\'t indexed yet.' ); + } + + $query->whereIn( + 'conversations.id', + static function ( $reviews ) use ( $plugin_id ): void { + $reviews->select( 'conversation_id' )->from( Review::TABLE )->where( 'plugin_id', (int) $plugin_id ); + } + ); + + return $query; + } + + /** + * Whether the index has the plugin ID column, which its migration adds. + * + * @return bool + */ + private static function ready(): bool { + if ( null === self::$ready ) { + try { + self::$ready = Schema::hasColumn( Review::TABLE, 'plugin_id' ); + } catch ( \Throwable $e ) { + \Log::error( '[WPOrgPluginReview] Could not check the index of review emails: ' . $e->getMessage() ); + + return false; + } + } + + return self::$ready; + } +} diff --git a/freescout.wordpress.net/Modules/WPOrgPluginReview/Services/Review.php b/freescout.wordpress.net/Modules/WPOrgPluginReview/Services/Review.php index f384de49e4..a442918a26 100644 --- a/freescout.wordpress.net/Modules/WPOrgPluginReview/Services/Review.php +++ b/freescout.wordpress.net/Modules/WPOrgPluginReview/Services/Review.php @@ -416,6 +416,8 @@ private static function row( Thread $thread, array $review_id ): array { 'conversation_id' => (int) $thread->conversation_id, 'type' => mb_substr( $review_id['type'], 0, 50 ), 'reviewed_at' => $thread->created_at ? (string) $thread->created_at : null, + // 0 when the line names no plugin; null is left for the rows indexed before the column, see IndexPluginIds. + 'plugin_id' => max( 0, (int) ( $review_id['plugin_id'] ?? 0 ) ), ); } diff --git a/freescout.wordpress.net/Modules/WPOrgPluginReview/tests/PluginConversationsTest.php b/freescout.wordpress.net/Modules/WPOrgPluginReview/tests/PluginConversationsTest.php new file mode 100644 index 0000000000..f2dd7b0d38 --- /dev/null +++ b/freescout.wordpress.net/Modules/WPOrgPluginReview/tests/PluginConversationsTest.php @@ -0,0 +1,232 @@ +app->register( WPOrgPluginReviewServiceProvider::class ); + $this->reviewer = $this->create_user(); + $this->mailbox = $this->create_mailbox( 'Plugins', 'plugins@wordpress.test' ); + } + + /** + * Review emails are indexed with the plugin ID of their Review ID line, and 0 when it names none. + * + * @return void + */ + public function test_indexes_the_plugin_id_of_review_emails(): void { + $with = $this->review( 42 ); + $without = $this->create_thread( $this->conversation(), Thread::TYPE_MESSAGE, '

Review ID: R my-plugin/jane 8Aug26/4.3

', $this->reviewer, '2026-08-08 10:00:00' ); + + $this->assertSame( 42, $this->plugin_id( $with ) ); + $this->assertSame( 0, $this->plugin_id( $without ) ); + } + + /** + * The rows indexed before the plugin IDs get theirs; the ones that aren't review emails any more, or whose thread + * is gone, leave the index; and doing it again changes nothing. + * + * @return void + */ + public function test_indexes_the_plugin_ids_of_rows_indexed_before_them(): void { + $review = $this->review( 42 ); + $other = $this->review( 43 ); + $changed = $this->review( 44 ); + $kept = $this->review( 45 ); + + // As the rows were before the column, with a thread changed and another deleted without their hooks. + \DB::table( Review::TABLE )->where( 'thread_id', '!=', $kept->id )->update( array( 'plugin_id' => null ) ); + \DB::table( 'threads' )->where( 'id', $changed->id )->update( array( 'body' => '

Thanks for the update.

' ) ); + \DB::table( 'threads' )->where( 'id', $other->id )->delete(); + + $this->assertNull( ( new IndexPluginIds() )->index_batch() ); + + $this->assertSame( 42, $this->plugin_id( $review ) ); + $this->assertSame( 45, $this->plugin_id( $kept ) ); + $this->assertFalse( \DB::table( Review::TABLE )->where( 'thread_id', $other->id )->exists() ); + $this->assertFalse( \DB::table( Review::TABLE )->where( 'thread_id', $changed->id )->exists() ); + + \DB::table( Review::TABLE )->update( array( 'plugin_id' => null ) ); + \Artisan::call( + 'wporgpluginreview:index', + array( + '--plugin-ids' => true, + '--now' => true, + ) + ); + + $this->assertSame( array( 42, 45 ), \DB::table( Review::TABLE )->orderBy( 'plugin_id' )->pluck( 'plugin_id' )->map( 'intval' )->all() ); + } + + /** + * A batch starts after the last row the previous one read, so rows it can't read don't stop the rest. + * + * @return void + */ + public function test_continues_after_the_rows_of_the_previous_batch(): void { + $first = $this->review( 42 ); + $second = $this->review( 43 ); + \DB::table( Review::TABLE )->update( array( 'plugin_id' => null ) ); + + $after = (int) \DB::table( Review::TABLE )->where( 'thread_id', $first->id )->value( 'id' ); + $this->assertNull( ( new IndexPluginIds( $after ) )->index_batch() ); + + $this->assertNull( \DB::table( Review::TABLE )->where( 'thread_id', $first->id )->value( 'plugin_id' ) ); + $this->assertSame( 43, $this->plugin_id( $second ) ); + } + + /** + * The parameter lists every conversation with a review email about the plugin, open or closed, and combines with + * the module's other parameters. + * + * @return void + */ + public function test_lists_the_conversations_of_a_plugin(): void { + $open = $this->review( 42 )->conversation; + $closed = $this->review( 42 )->conversation; + $closed->update( array( 'status' => Conversation::STATUS_CLOSED ) ); + $this->review( 43 ); + $this->create_thread( $this->conversation(), Thread::TYPE_CUSTOMER, '

Review ID: R my-plugin/jane 8Aug26/4.3 (P0TDX42HGN)

', null, '2026-08-08 11:00:00' ); + + $this->assertSame( array( $open->id, $closed->id ), $this->search( '42' ) ); + $this->assertSame( array( $open->id ), $this->search( '42', fn( $query ) => $query->where( 'status', Conversation::STATUS_ACTIVE ) ) ); + $this->assertSame( array(), $this->search( '41' ) ); + $this->assertCount( 4, $this->search( null ) ); + } + + /** + * A value that isn't a plugin ID is refused rather than ignored, which would list every conversation as the plugin's. + * + * @return void + */ + public function test_refuses_what_isnt_a_plugin_id(): void { + foreach ( array( 'abc', '-3', '0', '4.2', array( 42 ) ) as $value ) { + try { + $this->search( $value ); + $this->fail( 'Not refused: ' . json_encode( $value ) ); + } catch ( HttpException $e ) { + $this->assertSame( 400, $e->getStatusCode() ); + } + } + } + + /** + * The module's provider adds the filter the API & Webhooks module runs. + * + * @return void + */ + public function test_is_added_to_the_api_query(): void { + $review = $this->review( 42 ); + $this->review( 43 ); + + $query = \Eventy::filter( 'api.conversations.query', $this->base_query(), Request::create( '/api/conversations', 'GET', array( PluginConversations::PARAMETER => '42' ) ) ); + + $this->assertSame( array( (int) $review->conversation_id ), array_map( 'intval', $query->pluck( 'conversations.id' )->all() ) ); + } + + /** + * A conversation in the plugins team's mailbox. + * + * @return Conversation + */ + private function conversation(): Conversation { + return $this->create_conversation( $this->mailbox, $this->create_sender() ); + } + + /** + * A review email about a plugin, in a conversation of its own. + * + * @param int $plugin_id Plugin ID. + * @return Thread + */ + private function review( int $plugin_id ): Thread { + return $this->create_thread( $this->conversation(), Thread::TYPE_MESSAGE, '

Review ID: R my-plugin/jane/8Aug26/T1 8Aug26/4.3 (P0TDX' . $plugin_id . 'HGN)

', $this->reviewer, '2026-08-08 10:00:00' ); + } + + /** + * The plugin ID a thread is indexed with. + * + * @param Thread $thread Thread. + * @return int|null + */ + private function plugin_id( Thread $thread ): ?int { + $plugin_id = \DB::table( Review::TABLE )->where( 'thread_id', $thread->id )->value( 'plugin_id' ); + + return null === $plugin_id ? null : (int) $plugin_id; + } + + /** + * Runs the filter on the conversation list query. + * + * @param mixed $plugin_id Value of the parameter; null leaves it out. + * @param callable|null $more Adds the module's other parameters to the query. + * @return int[] IDs of the conversations listed. + */ + private function search( mixed $plugin_id, ?callable $more = null ): array { + $query = $this->base_query(); + if ( $more ) { + $more( $query ); + } + $parameters = null === $plugin_id ? array() : array( PluginConversations::PARAMETER => $plugin_id ); + $query = PluginConversations::filter( $query, Request::create( '/api/conversations', 'GET', $parameters ) ); + + return array_map( 'intval', $query->pluck( 'conversations.id' )->all() ); + } + + /** + * The query the module starts from. + * + * @return mixed + */ + private function base_query(): mixed { + return Conversation::select( array( 'conversations.*' ) )->where( 'mailbox_id', $this->mailbox->id )->orderBy( 'conversations.id', 'asc' ); + } +} diff --git a/freescout.wordpress.net/Modules/WPOrgSite/Providers/WPOrgSiteServiceProvider.php b/freescout.wordpress.net/Modules/WPOrgSite/Providers/WPOrgSiteServiceProvider.php index 9a992c7846..423b4fb9c9 100644 --- a/freescout.wordpress.net/Modules/WPOrgSite/Providers/WPOrgSiteServiceProvider.php +++ b/freescout.wordpress.net/Modules/WPOrgSite/Providers/WPOrgSiteServiceProvider.php @@ -17,6 +17,7 @@ use Illuminate\View\View; use Modules\WPOrgSite\Http\Middleware\LockModules; use Modules\WPOrgSite\Services\AltConversation; +use Modules\WPOrgSite\Services\ConversationTextSearch; use Modules\WPOrgSite\Services\EnvSettings; use Modules\WPOrgSite\Services\PinnedNote; use Modules\WPOrgSite\Services\UserChoice; @@ -98,6 +99,9 @@ static function ( array $styles ): array { } ); + // The API & Webhooks module's conversation list, searched by the text of the threads too. + \Eventy::addFilter( 'api.conversations.query', array( ConversationTextSearch::class, 'filter' ), 20, 2 ); + \Eventy::addFilter( 'javascripts', static function ( array $javascripts ): array { diff --git a/freescout.wordpress.net/Modules/WPOrgSite/README.md b/freescout.wordpress.net/Modules/WPOrgSite/README.md index 5495195375..acbef03f5c 100644 --- a/freescout.wordpress.net/Modules/WPOrgSite/README.md +++ b/freescout.wordpress.net/Modules/WPOrgSite/README.md @@ -15,6 +15,16 @@ Adapts FreeScout to how WordPress.org runs it. Small changes to FreeScout's own - **Pinned header:** [a conversation's header can stay in view](#pinned-header) while the agent reads the emails below it. - **Alternate conversation design:** agents who choose so in their profile get [conversations laid out otherwise](#alternate-conversation-design), oldest email first, with Reply and Note at the bottom. - **Conversations:** [keyboard shortcuts](#keyboard-shortcuts); the status and assignee selectors pinned to the top of the window while their menu is open, further down long conversations where the toolbar has scrolled out of view, so they can be chosen there, like after pressing `s` or `a`; the subject at 18px and the threads' authors at 16px, rather than core's 23px and 18px, so more of the conversation fits; and clicking the number next to the subject copies it, without the `#`. Its tooltip says when it's copied; where the browser doesn't let the page write to the clipboard, it copies through a selected field instead. It's all in `Public/js/conversation.js`. +- **Searching conversations' text through the API:** with the API & Webhooks module, `GET /api/conversations` takes a `body` parameter, like HelpScout's `body:` search: only conversations with a published email, reply, or note containing that text are listed. It's matched literally (`%` and `_` included) and ignoring case. Three more parameters narrow which threads are read: `bodyThreadType` (`customer`, `message`, `note`, comma-separated) and `bodySince` / `bodyBefore` (threads created since, or before, an ISO 8601 date). The plugins team's review tools find a plugin's review by the code every review email ends with, reading only the team's emails of the last 6 months: `?mailboxId=12&status=active&tag=plugin-review&body=P0TDX123HGN&bodyThreadType=message&bodySince=2026-04-01T00:00:00Z&embed=threads`. The module still authenticates the key, limits it to its mailboxes, and paginates. Code in `Services/ConversationTextSearch.php`. + - **How it searches:** thread bodies can't be indexed for this, so a search reads the body of every thread it covers. The module's other parameters pick the conversations first, from their indexes. The threads of those conversations that the `body*` parameters cover are then counted from the threads' indexes, and only then read, 500 conversations at a time. The module's count and page get the matches as IDs, so the search runs once per request. A correlated `EXISTS` would leave the plan to MariaDB, which turns it into a scan of every thread when the other filters look weak. + - **Limits:** a search reads at most 15,000 threads (filter `wporgsite.conversation_text_search.max_threads`) and counts the threads of at most 25,000 conversations (`…max_conversations`); beyond that it's refused. Reading costs about 0.1 ms per review email of 20 KB, so 15,000 threads is about 2 seconds. Every page of a search runs it again, so keep searches to what fits in a page or two. + - **Measured** on a copy with 200,000 conversations: the last 6 months at 1,500 new reviews a week, 39,000 conversations with 3 GB of emails, and review emails of 20 KB on average. Warm cache, local Docker. + - Finding a plugin's review among ~1,000 active ones reads ~2,100 team emails: about 0.3 s. + - With a backlog of ~4,900 active reviews it reads ~14,000: about 1.9 s, close to the limit, so a backlog of about 5,000 active reviews is as far as the lookup goes. + - Every review email of 6 months (~100,000) would take 10–12 s and is refused. + - 2 months of approved reviews, searched a week at a time, take 3.5 s in 9 requests. + - Counting the threads of 32,000 conversations alone takes 0.1 s. + - **Errors, not empty lists:** a text shorter than 3 characters, an unknown thread type, a date that isn't one, or a search over the limits answers 400 with what to change, and a failure answers 500. An empty list would read as "nothing says that", which the review tools act on. ## Keyboard shortcuts diff --git a/freescout.wordpress.net/Modules/WPOrgSite/Services/ConversationTextSearch.php b/freescout.wordpress.net/Modules/WPOrgSite/Services/ConversationTextSearch.php new file mode 100644 index 0000000000..fa71846e24 --- /dev/null +++ b/freescout.wordpress.net/Modules/WPOrgSite/Services/ConversationTextSearch.php @@ -0,0 +1,331 @@ + + */ + private const THREAD_TYPES = array( + 'customer' => Thread::TYPE_CUSTOMER, + 'message' => Thread::TYPE_MESSAGE, + 'note' => Thread::TYPE_NOTE, + ); + + /** + * Limits the conversation list query to conversations with a thread containing the text. + * + * @param mixed $query Query for the conversations, with the module's other parameters applied. + * @param mixed $request Request. + * @return mixed + */ + public static function filter( mixed $query, mixed $request ): mixed { + if ( ! is_object( $query ) || ! method_exists( $query, 'getQuery' ) || ! is_object( $request ) || ! method_exists( $request, 'input' ) ) { + return $query; + } + + $text = $request->input( self::PARAMETER ); + + if ( null === $text || ( is_string( $text ) && '' === trim( $text ) ) ) { + return $query; + } + + if ( ! is_string( $text ) || mb_strlen( trim( $text ) ) < self::MIN_LENGTH ) { + self::refuse( 400, 'The `' . self::PARAMETER . '` text needs ' . self::MIN_LENGTH . ' characters or more.' ); + } + + $threads = array( + 'types' => self::thread_types( $request->input( self::THREAD_TYPE_PARAMETER ) ), + 'since' => self::date( self::SINCE_PARAMETER, $request->input( self::SINCE_PARAMETER ) ), + 'before' => self::date( self::BEFORE_PARAMETER, $request->input( self::BEFORE_PARAMETER ) ), + ); + + $max_conversations = (int) \Eventy::filter( 'wporgsite.conversation_text_search.max_conversations', self::MAX_CONVERSATIONS ); + $max_threads = (int) \Eventy::filter( 'wporgsite.conversation_text_search.max_threads', self::MAX_THREADS ); + + try { + $candidates = self::candidate_ids( $query, $max_conversations ); + $to_read = count( $candidates ) > $max_conversations ? 0 : self::count_threads( $query, $candidates, $threads ); + } catch ( \Throwable $e ) { + \Log::error( 'WPOrgSite: conversation text search: ' . $e->getMessage() ); + self::refuse( 500, 'The conversations could not be searched.' ); + } + + if ( count( $candidates ) > $max_conversations ) { + self::refuse( 400, 'The `' . self::PARAMETER . '` search reads the threads of ' . $max_conversations . ' conversations at most, and these filters match more. Narrow them with status, tag, createdSince, customerId or customerEmail.' ); + } + + if ( $to_read > $max_threads ) { + self::refuse( 400, 'The `' . self::PARAMETER . '` search would read ' . $to_read . ' threads, and it reads ' . $max_threads . ' at most. Narrow it with ' . self::THREAD_TYPE_PARAMETER . ', ' . self::SINCE_PARAMETER . ' and ' . self::BEFORE_PARAMETER . ', or the conversations with status, tag, createdSince, customerId or customerEmail.' ); + } + + try { + $matches = $to_read > 0 ? self::matching_ids( $query, $candidates, trim( $text ), $threads ) : array(); + } catch ( \Throwable $e ) { + \Log::error( 'WPOrgSite: conversation text search: ' . $e->getMessage() ); + self::refuse( 500, 'The conversations could not be searched.' ); + } + + if ( empty( $matches ) ) { + $query->whereRaw( '1 = 0' ); + } else { + $query->whereIn( 'conversations.id', $matches ); + } + + return $query; + } + + /** + * The thread types of the parameter, every searchable one when it names none. + * + * @param mixed $value Value of the parameter. + * @return int[] + */ + private static function thread_types( mixed $value ): array { + if ( null === $value || '' === $value ) { + return array_values( self::THREAD_TYPES ); + } + + $types = array(); + foreach ( explode( ',', is_string( $value ) ? $value : '' ) as $name ) { + $name = strtolower( trim( $name ) ); + if ( ! isset( self::THREAD_TYPES[ $name ] ) ) { + self::refuse( 400, 'The `' . self::THREAD_TYPE_PARAMETER . '` parameter takes customer, message, or note, separated by commas.' ); + } + $types[] = self::THREAD_TYPES[ $name ]; + } + + return array_values( array_unique( $types ) ); + } + + /** + * A date parameter in the database's time zone, like the module turns `createdSince` into, or null. + * + * @param string $parameter Name of the parameter, for the error. + * @param mixed $value Value of the parameter. + * @return string|null + * @throws \InvalidArgumentException If the value isn't a string; caught, so it's refused like any other value that isn't a date. + */ + private static function date( string $parameter, mixed $value ): ?string { + if ( null === $value || '' === $value ) { + return null; + } + + try { + if ( ! is_string( $value ) ) { + throw new \InvalidArgumentException( 'Not a string' ); + } + + return Carbon::parse( $value )->setTimezone( (string) config( 'app.timezone' ) )->format( 'Y-m-d H:i:s' ); + } catch ( \Throwable $e ) { + self::refuse( 400, 'The `' . $parameter . '` parameter takes a date, like 2026-04-01T00:00:00Z.' ); + } + } + + /** + * IDs of the conversations the module's other parameters select, one more than the limit at most. + * + * @param mixed $query Query for the conversations. + * @param int $max The most conversations a search covers. + * @return int[] + */ + private static function candidate_ids( mixed $query, int $max ): array { + $candidates = ( clone $query )->getQuery(); + + // Only the IDs: without the module's columns, order, and page. + $candidates->columns = null; + $candidates->orders = null; + $candidates->offset = null; + $candidates->limit = null; + + $ids = $candidates->limit( $max + 1 )->pluck( 'conversations.id' )->all(); + + return array_values( array_unique( array_map( 'intval', $ids ) ) ); + } + + /** + * The threads of the candidates a search would read, counted from the threads' indexes. + * + * Drafts are counted too: the state isn't in those indexes, and they're few. + * + * @param mixed $query Query for the conversations, for its connection. + * @param int[] $candidates Conversation IDs. + * @param array $threads Which threads: `types`, `since`, `before`. + * @return int + */ + private static function count_threads( mixed $query, array $candidates, array $threads ): int { + $count = 0; + foreach ( array_chunk( $candidates, self::CHUNK ) as $chunk ) { + $count += (int) self::threads_query( $query, $chunk, $threads )->count(); + } + + return $count; + } + + /** + * IDs of the conversations, among the candidates, with a published thread containing the text. + * + * @param mixed $query Query for the conversations, for its connection. + * @param int[] $candidates Conversation IDs. + * @param string $text Text to search for, matched literally. + * @param array $threads Which threads: `types`, `since`, `before`. + * @return int[] + */ + private static function matching_ids( mixed $query, array $candidates, string $text, array $threads ): array { + $like = '%' . addcslashes( $text, '\\%_' ) . '%'; + $operator = \Helper::isPgSql() ? 'ilike' : 'like'; + $matches = array(); + + foreach ( array_chunk( $candidates, self::CHUNK ) as $chunk ) { + $found = self::threads_query( $query, $chunk, $threads ) + ->where( 'state', Thread::STATE_PUBLISHED ) + ->where( 'body', $operator, $like ) + ->distinct() + ->pluck( 'conversation_id' ) + ->all(); + + array_push( $matches, ...array_map( 'intval', $found ) ); + } + + return array_values( array_unique( $matches ) ); + } + + /** + * The threads of some conversations a search covers. + * + * @param mixed $query Query for the conversations, for its connection. + * @param int[] $chunk Conversation IDs. + * @param array $threads Which threads: `types`, `since`, `before`. + * @return mixed + */ + private static function threads_query( mixed $query, array $chunk, array $threads ): mixed { + $builder = $query->getQuery()->getConnection()->table( 'threads' ) + ->whereIn( 'conversation_id', $chunk ) + ->whereIn( 'type', $threads['types'] ); + + if ( null !== $threads['since'] ) { + $builder->where( 'created_at', '>=', $threads['since'] ); + } + if ( null !== $threads['before'] ) { + $builder->where( 'created_at', '<', $threads['before'] ); + } + + return $builder; + } + + /** + * Answers the request with an error, which the module's API turns into JSON. + * + * @param int $status HTTP status. + * @param string $message Why. + * @return never + */ + private static function refuse( int $status, string $message ): never { + abort( $status, $message ); + } +} diff --git a/freescout.wordpress.net/Modules/WPOrgSite/tests/ConversationTextSearchTest.php b/freescout.wordpress.net/Modules/WPOrgSite/tests/ConversationTextSearchTest.php new file mode 100644 index 0000000000..108838b3c0 --- /dev/null +++ b/freescout.wordpress.net/Modules/WPOrgSite/tests/ConversationTextSearchTest.php @@ -0,0 +1,271 @@ +app->register( WPOrgSiteServiceProvider::class ); + } + + /** + * Only conversations with a published email, reply, or note containing the text are listed. + * + * @return void + */ + public function test_lists_conversations_whose_threads_contain_the_text(): void { + $mailbox = $this->create_mailbox(); + $agent = $this->create_user(); + + $in_reply = $this->create_conversation( $mailbox, $this->create_sender() ); + $this->create_thread( $in_reply, Thread::TYPE_MESSAGE, '

Review ID: R my-plugin/me/1Oct26/T1 1Oct26/1.0 (P0TDX123HGN)

', $agent, '2026-10-01 10:00:00' ); + + $in_note = $this->create_conversation( $mailbox, $this->create_sender() ); + $this->create_thread( $in_note, Thread::TYPE_NOTE, 'See P0TDX123HGN', $agent, '2026-10-01 10:00:00' ); + + $in_draft = $this->create_conversation( $mailbox, $this->create_sender() ); + $draft = $this->create_thread( $in_draft, Thread::TYPE_MESSAGE, 'P0TDX123HGN', $agent, '2026-10-01 10:00:00' ); + + $draft->state = Thread::STATE_DRAFT; + $draft->save(); + + $other = $this->create_conversation( $mailbox, $this->create_sender() ); + $this->create_thread( $other, Thread::TYPE_MESSAGE, 'P0TDX1234HGN', $agent, '2026-10-01 10:00:00' ); + + $this->assertSame( array( $in_reply->id, $in_note->id ), $this->search( 'P0TDX123HGN' ) ); + } + + /** + * The text is matched literally, so LIKE's wildcards in it don't match anything else. + * + * @return void + */ + public function test_matches_wildcards_literally(): void { + $mailbox = $this->create_mailbox(); + $agent = $this->create_user(); + + $percent = $this->create_conversation( $mailbox, $this->create_sender() ); + $this->create_thread( $percent, Thread::TYPE_MESSAGE, 'A 100% sure thing', $agent, '2026-10-01 10:00:00' ); + + $other = $this->create_conversation( $mailbox, $this->create_sender() ); + $this->create_thread( $other, Thread::TYPE_MESSAGE, 'A 1000 sure things', $agent, '2026-10-01 10:00:00' ); + + $this->assertSame( array( $percent->id ), $this->search( '100%' ) ); + $this->assertSame( array(), $this->search( '1_0' ) ); + $this->assertSame( array(), $this->search( '1\\0' ) ); + } + + /** + * Only the thread types asked for are read. + * + * @return void + */ + public function test_reads_only_the_thread_types_asked_for(): void { + $mailbox = $this->create_mailbox(); + $agent = $this->create_user(); + + $in_reply = $this->create_conversation( $mailbox, $this->create_sender() ); + $this->create_thread( $in_reply, Thread::TYPE_MESSAGE, 'P0TDX123HGN', $agent, '2026-10-01 10:00:00' ); + + $in_quote = $this->create_conversation( $mailbox, $this->create_sender() ); + $this->create_thread( $in_quote, Thread::TYPE_CUSTOMER, '
P0TDX123HGN
', null, '2026-10-01 10:00:00' ); + + $this->assertSame( array( $in_reply->id, $in_quote->id ), $this->search( 'P0TDX123HGN' ) ); + $this->assertSame( array( $in_reply->id ), $this->search( 'P0TDX123HGN', 'message' ) ); + $this->assertSame( array( $in_reply->id, $in_quote->id ), $this->search( 'P0TDX123HGN', 'Message, customer' ) ); + $this->assert_refused( 400, 'P0TDX123HGN', 'lineitem' ); + } + + /** + * Without the parameter the list is left alone, and a search that can't be made is refused rather than + * listing nothing, which would read as nothing matching. + * + * @return void + */ + public function test_leaves_other_lists_alone_and_refuses_what_it_cannot_search(): void { + $mailbox = $this->create_mailbox(); + $conversation = $this->create_conversation( $mailbox, $this->create_sender() ); + $this->create_thread( $conversation, Thread::TYPE_CUSTOMER, 'Hello there', null, '2026-10-01 10:00:00' ); + + $this->assertSame( array( $conversation->id ), $this->search( null ) ); + $this->assertSame( array( $conversation->id ), $this->search( ' ' ) ); + $this->assert_refused( 400, 'He' ); + $this->assert_refused( 400, array( 'Hello' ) ); + } + + /** + * Searches covering more conversations than the limit are refused, before any thread is read. + * + * @return void + */ + public function test_refuses_searches_over_too_many_conversations(): void { + $mailbox = $this->create_mailbox(); + foreach ( array( 1, 2, 3 ) as $i ) { + $conversation = $this->create_conversation( $mailbox, $this->create_sender() ); + $this->create_thread( $conversation, Thread::TYPE_CUSTOMER, 'Hello there ' . $i, null, '2026-10-01 10:00:00' ); + } + + \Eventy::addFilter( 'wporgsite.conversation_text_search.max_conversations', static fn(): int => 2 ); + + try { + $this->assert_refused( 400, 'Hello' ); + } finally { + \Eventy::removeAllFilters( 'wporgsite.conversation_text_search.max_conversations' ); + } + + $this->assertCount( 3, $this->search( 'Hello' ) ); + } + + /** + * Searches that would read more threads than the limit are refused, counting only the threads they read. + * + * @return void + */ + public function test_refuses_searches_over_too_many_threads(): void { + $mailbox = $this->create_mailbox(); + $agent = $this->create_user(); + $conversation = $this->create_conversation( $mailbox, $this->create_sender() ); + $this->create_thread( $conversation, Thread::TYPE_CUSTOMER, 'Hello there', null, '2026-10-01 10:00:00' ); + $this->create_thread( $conversation, Thread::TYPE_CUSTOMER, 'Hello again', null, '2026-10-02 10:00:00' ); + $this->create_thread( $conversation, Thread::TYPE_MESSAGE, 'Hello, P0TDX123HGN', $agent, '2026-10-03 10:00:00' ); + + \Eventy::addFilter( 'wporgsite.conversation_text_search.max_threads', static fn(): int => 2 ); + + try { + $this->assert_refused( 400, 'Hello' ); + $this->assertSame( array( $conversation->id ), $this->search( 'Hello', 'message' ) ); + $this->assertSame( array( $conversation->id ), $this->search( 'Hello', null, array( ConversationTextSearch::SINCE_PARAMETER => '2026-10-02T00:00:00Z' ) ) ); + } finally { + \Eventy::removeAllFilters( 'wporgsite.conversation_text_search.max_threads' ); + } + } + + /** + * Only the threads created since and before the dates asked for are read. + * + * @return void + */ + public function test_reads_only_the_threads_of_the_dates_asked_for(): void { + $mailbox = $this->create_mailbox(); + $agent = $this->create_user(); + + $september = $this->create_conversation( $mailbox, $this->create_sender() ); + $this->create_thread( $september, Thread::TYPE_MESSAGE, 'P0TDX123HGN', $agent, '2026-09-15 10:00:00' ); + + $october = $this->create_conversation( $mailbox, $this->create_sender() ); + $this->create_thread( $october, Thread::TYPE_MESSAGE, 'P0TDX123HGN', $agent, '2026-10-15 10:00:00' ); + + $this->assertSame( array( $september->id, $october->id ), $this->search( 'P0TDX123HGN' ) ); + $this->assertSame( array( $october->id ), $this->search( 'P0TDX123HGN', null, array( ConversationTextSearch::SINCE_PARAMETER => '2026-10-01T00:00:00Z' ) ) ); + $this->assertSame( array( $september->id ), $this->search( 'P0TDX123HGN', null, array( ConversationTextSearch::BEFORE_PARAMETER => '2026-10-01T00:00:00Z' ) ) ); + $this->assertSame( + array(), + $this->search( + 'P0TDX123HGN', + null, + array( + ConversationTextSearch::SINCE_PARAMETER => '2026-09-20T00:00:00Z', + ConversationTextSearch::BEFORE_PARAMETER => '2026-10-10T00:00:00Z', + ) + ) + ); + $this->assert_refused( 400, 'P0TDX123HGN', null, array( ConversationTextSearch::SINCE_PARAMETER => 'last tuesday-ish' ) ); + } + + /** + * The module's provider adds the filter the API & Webhooks module runs. + * + * @return void + */ + public function test_is_added_to_the_api_query(): void { + $mailbox = $this->create_mailbox(); + $conversation = $this->create_conversation( $mailbox, $this->create_sender() ); + $this->create_thread( $conversation, Thread::TYPE_CUSTOMER, 'Hello there', null, '2026-10-01 10:00:00' ); + + $query = \Eventy::filter( 'api.conversations.query', $this->base_query(), Request::create( '/api/conversations', 'GET', array( 'body' => 'Nothing like it' ) ) ); + + $this->assertSame( array(), $query->pluck( 'conversations.id' )->all() ); + } + + /** + * Runs the filter on the conversation list query. + * + * @param mixed $text Value of the parameter; null leaves it out. + * @param string|null $types Value of the thread type parameter; null leaves it out. + * @param array $more More parameters. + * @return int[] IDs of the conversations listed. + */ + private function search( mixed $text, ?string $types = null, array $more = array() ): array { + $parameters = array_filter( + array_merge( + array( + ConversationTextSearch::PARAMETER => $text, + ConversationTextSearch::THREAD_TYPE_PARAMETER => $types, + ), + $more + ), + static fn( $value ): bool => null !== $value + ); + $query = ConversationTextSearch::filter( $this->base_query(), Request::create( '/api/conversations', 'GET', $parameters ) ); + + return array_map( 'intval', $query->pluck( 'conversations.id' )->all() ); + } + + /** + * Asserts that a search is answered with an error. + * + * @param int $status HTTP status expected. + * @param mixed $text Value of the parameter. + * @param string|null $types Value of the thread type parameter; null leaves it out. + * @param array $more More parameters. + * @return void + */ + private function assert_refused( int $status, mixed $text, ?string $types = null, array $more = array() ): void { + try { + $this->search( $text, $types, $more ); + } catch ( HttpException $e ) { + $this->assertSame( $status, $e->getStatusCode() ); + $this->assertNotSame( '', $e->getMessage() ); + return; + } + + $this->fail( 'The search was not refused.' ); + } + + /** + * The query the module starts from. + * + * @return mixed + */ + private function base_query(): mixed { + return Conversation::select( array( 'conversations.*' ) )->orderBy( 'conversations.id', 'asc' ); + } +} From 6e93d1587a60137918e9068abadf1e6bc9fb172b Mon Sep 17 00:00:00 2001 From: Konstantin Obenland Date: Tue, 20 Oct 2026 14:05:37 -0500 Subject: [PATCH 2/3] FreeScout: Index plugin IDs from the start, and trim comments WPOrgPluginReview isn't deployed yet, so its table gets the plugin_id column when it's created: no migration, backfill job, `--plugin-ids` option, or 503 while the column is missing. Local environments rebuild the table. Shortens the docblocks and comments to what the code doesn't say, and removes only the test's own filters instead of every filter on the hook. Bumps WPOrgSite to 1.2.0. Co-Authored-By: Claude Opus 5.5 --- .../Console/IndexReviews.php | 17 +-- ...000000_create_wporgpluginreview_tables.php | 4 +- ...plugin_id_to_wporgpluginreview_reviews.php | 66 ---------- .../WPOrgPluginReview/Jobs/IndexPluginIds.php | 121 ------------------ .../WPOrgPluginReviewServiceProvider.php | 1 - .../Modules/WPOrgPluginReview/README.md | 4 +- .../Services/PluginConversations.php | 43 +------ .../WPOrgPluginReview/Services/Review.php | 1 - .../tests/PluginConversationsTest.php | 73 +---------- .../Providers/WPOrgSiteServiceProvider.php | 1 - .../Services/ConversationTextSearch.php | 56 ++------ .../Modules/WPOrgSite/module.json | 2 +- .../tests/ConversationTextSearchTest.php | 16 +-- 13 files changed, 40 insertions(+), 365 deletions(-) delete mode 100644 freescout.wordpress.net/Modules/WPOrgPluginReview/Database/Migrations/2026_10_18_000000_add_plugin_id_to_wporgpluginreview_reviews.php delete mode 100644 freescout.wordpress.net/Modules/WPOrgPluginReview/Jobs/IndexPluginIds.php diff --git a/freescout.wordpress.net/Modules/WPOrgPluginReview/Console/IndexReviews.php b/freescout.wordpress.net/Modules/WPOrgPluginReview/Console/IndexReviews.php index e39b4be39e..5501494674 100644 --- a/freescout.wordpress.net/Modules/WPOrgPluginReview/Console/IndexReviews.php +++ b/freescout.wordpress.net/Modules/WPOrgPluginReview/Console/IndexReviews.php @@ -10,13 +10,11 @@ namespace Modules\WPOrgPluginReview\Console; use Illuminate\Console\Command; -use Modules\WPOrgPluginReview\Jobs\IndexPluginIds as IndexPluginIdsJob; use Modules\WPOrgPluginReview\Jobs\IndexReviews as IndexReviewsJob; /** * Brings the index of review emails up to date with every reply of the plugins team's mailbox, like after the module - * was switched off for a while, when their threads' hooks didn't run. With `--plugin-ids`, only the review emails indexed - * before the index had their plugin IDs are read again. + * was switched off for a while, when their threads' hooks didn't run. */ final class IndexReviews extends Command { @@ -25,7 +23,7 @@ final class IndexReviews extends Command { * * @var string */ - protected $signature = 'wporgpluginreview:index {--now : Index in this process instead of on the queue} {--plugin-ids : Only read again the review emails indexed without their plugin ID}'; + protected $signature = 'wporgpluginreview:index {--now : Index in this process instead of on the queue}'; /** * Command description. @@ -40,22 +38,19 @@ final class IndexReviews extends Command { * @return int Exit code. */ public function handle(): int { - $plugin_ids = (bool) $this->option( 'plugin-ids' ); - $job = $plugin_ids ? IndexPluginIdsJob::class : IndexReviewsJob::class; - if ( ! $this->option( 'now' ) ) { - $job::dispatch(); - $this->line( $plugin_ids ? 'Indexing the plugin IDs of the review emails on the queue.' : 'Indexing the review emails on the queue.' ); + IndexReviewsJob::dispatch(); + $this->line( 'Indexing the review emails on the queue.' ); return 0; } $last = 0; do { - $last = ( new $job( $last ) )->index_batch(); + $last = ( new IndexReviewsJob( $last ) )->index_batch(); } while ( null !== $last ); - $this->line( $plugin_ids ? 'Indexed the plugin IDs of the review emails.' : 'Indexed the review emails.' ); + $this->line( 'Indexed the review emails.' ); return 0; } diff --git a/freescout.wordpress.net/Modules/WPOrgPluginReview/Database/Migrations/2026_10_03_000000_create_wporgpluginreview_tables.php b/freescout.wordpress.net/Modules/WPOrgPluginReview/Database/Migrations/2026_10_03_000000_create_wporgpluginreview_tables.php index 6e84e066b1..931d7fd3f6 100644 --- a/freescout.wordpress.net/Modules/WPOrgPluginReview/Database/Migrations/2026_10_03_000000_create_wporgpluginreview_tables.php +++ b/freescout.wordpress.net/Modules/WPOrgPluginReview/Database/Migrations/2026_10_03_000000_create_wporgpluginreview_tables.php @@ -13,7 +13,7 @@ use Modules\WPOrgPluginReview\Jobs\IndexReviews; /** - * Which threads are review emails, and of what type, from their Review ID lines. + * Which threads are review emails, of what type, and about which plugin, from their Review ID lines. */ class CreateWporgpluginreviewTables extends Migration { @@ -35,6 +35,8 @@ static function ( Blueprint $table ): void { $table->unsignedInteger( 'conversation_id' )->index(); $table->string( 'type', 50 ); $table->timestamp( 'reviewed_at' )->nullable(); + $table->unsignedInteger( 'plugin_id' )->default( 0 ); + $table->index( array( 'plugin_id', 'conversation_id' ) ); } ); diff --git a/freescout.wordpress.net/Modules/WPOrgPluginReview/Database/Migrations/2026_10_18_000000_add_plugin_id_to_wporgpluginreview_reviews.php b/freescout.wordpress.net/Modules/WPOrgPluginReview/Database/Migrations/2026_10_18_000000_add_plugin_id_to_wporgpluginreview_reviews.php deleted file mode 100644 index d934fa9a19..0000000000 --- a/freescout.wordpress.net/Modules/WPOrgPluginReview/Database/Migrations/2026_10_18_000000_add_plugin_id_to_wporgpluginreview_reviews.php +++ /dev/null @@ -1,66 +0,0 @@ -unsignedInteger( 'plugin_id' )->nullable(); - $table->index( array( 'plugin_id', 'conversation_id' ) ); - } - ); - - // The review emails indexed already are read again on the queue, in batches; new ones get it as they're indexed. - try { - IndexPluginIds::dispatch(); - } catch ( \Throwable $e ) { - \Log::error( '[WPOrgPluginReview] Could not queue indexing the plugin IDs of the review emails: ' . $e->getMessage() ); - } - } - - /** - * Reverts the migration. - * - * @return void - */ - public function down(): void { - if ( ! Schema::hasTable( 'wporgpluginreview_reviews' ) || ! Schema::hasColumn( 'wporgpluginreview_reviews', 'plugin_id' ) ) { - return; - } - - Schema::table( - 'wporgpluginreview_reviews', - static function ( Blueprint $table ): void { - $table->dropIndex( array( 'plugin_id', 'conversation_id' ) ); - $table->dropColumn( 'plugin_id' ); - } - ); - } -} diff --git a/freescout.wordpress.net/Modules/WPOrgPluginReview/Jobs/IndexPluginIds.php b/freescout.wordpress.net/Modules/WPOrgPluginReview/Jobs/IndexPluginIds.php deleted file mode 100644 index b933b70813..0000000000 --- a/freescout.wordpress.net/Modules/WPOrgPluginReview/Jobs/IndexPluginIds.php +++ /dev/null @@ -1,121 +0,0 @@ -after_id = $after_id; - } - - /** - * Indexes the next batch of rows, and queues the one after. - * - * @return void - */ - public function handle(): void { - $last = $this->index_batch(); - if ( null !== $last ) { - self::dispatch( $last ); - } - } - - /** - * Indexes the next batch of rows without a plugin ID. - * - * The rows are walked by their ID rather than picked again by their missing plugin ID, so a row that can't be read - * doesn't stop the ones after it. - * - * @return int|null ID of the last row read, to start the next batch after; null once there are no more. - */ - public function index_batch(): ?int { - $last = $this->after_id; - - for ( $chunk = 0; $chunk < self::CHUNKS_PER_JOB; $chunk++ ) { - $rows = \DB::table( Review::TABLE ) - ->where( 'id', '>', $last ) - ->whereNull( 'plugin_id' ) - ->orderBy( 'id' ) - ->limit( self::CHUNK ) - ->get( array( 'id', 'thread_id' ) ); - - if ( $rows->isEmpty() ) { - return null; - } - - $threads = Thread::query()->whereIn( 'id', $rows->pluck( 'thread_id' )->map( 'intval' )->all() )->get()->keyBy( 'id' ); - - foreach ( $rows as $row ) { - $thread = $threads->get( (int) $row->thread_id ); - if ( $thread ) { - Review::index_thread( $thread ); - } else { - // Its thread is gone without its hooks, like core deleting a conversation's threads with a query. - \DB::table( Review::TABLE )->where( 'id', $row->id )->delete(); - } - $last = (int) $row->id; - } - - if ( $rows->count() < self::CHUNK ) { - return null; - } - } - - return $last; - } -} diff --git a/freescout.wordpress.net/Modules/WPOrgPluginReview/Providers/WPOrgPluginReviewServiceProvider.php b/freescout.wordpress.net/Modules/WPOrgPluginReview/Providers/WPOrgPluginReviewServiceProvider.php index 70ffbcf3c0..faec5831b2 100644 --- a/freescout.wordpress.net/Modules/WPOrgPluginReview/Providers/WPOrgPluginReviewServiceProvider.php +++ b/freescout.wordpress.net/Modules/WPOrgPluginReview/Providers/WPOrgPluginReviewServiceProvider.php @@ -178,7 +178,6 @@ static function ( mixed $conversation = null ): void { self::PANEL_PRIORITY ); - // The API & Webhooks module's conversation list, by the plugin of their review emails. \Eventy::addFilter( 'api.conversations.query', array( PluginConversations::class, 'filter' ), 20, 2 ); self::register_subject_hooks(); diff --git a/freescout.wordpress.net/Modules/WPOrgPluginReview/README.md b/freescout.wordpress.net/Modules/WPOrgPluginReview/README.md index 24e03235ec..46771bff97 100644 --- a/freescout.wordpress.net/Modules/WPOrgPluginReview/README.md +++ b/freescout.wordpress.net/Modules/WPOrgPluginReview/README.md @@ -87,8 +87,6 @@ The module keeps the table `wporgpluginreview_reviews`: which replies are review Switching the module on for the first time runs its migration, which queues indexing the review emails FreeScout already has. After it was switched off for a while, or after a HelpScout import, index them again with `php artisan wporgpluginreview:index` (`--now` to do it in the command rather than on the queue). -The plugin IDs came later. Their migration adds the column and queues reading the rows indexed before it again, in batches: 200 rows at a time, 25 batches per job, each job queuing the next. Only rows are read, so only review emails; replies without a Review ID line were never in the index. The rows whose reply isn't a review email any more, or is gone, leave the index. Until a row is read again, its plugin ID is empty, and lookups don't find it. `php artisan wporgpluginreview:index --plugin-ids` (with `--now`, in the command) does it again; it only reads the rows still without one. New review emails get it as they're indexed, in the same request that publishes them. - ### Looking up a plugin's conversations -With the API & Webhooks module, `GET /api/conversations` takes a `reviewPluginId` parameter: only the conversations with a review email about that plugin are listed, whatever their status, age, or who wrote to them, combined with the module's other parameters (`status`, `tag`…). The plugins team's review tools find the conversation of the plugin they review this way: any conversation for a first review, so a colleague's isn't started again, and `status=active,pending` for the next ones. It's an indexed lookup, a few milliseconds however many emails the mailbox has, unlike searching the emails' text (WPOrgSite's `body`). A value that isn't a plugin ID answers 400, and an index without the column yet answers 503: an empty list would read as "this plugin has no conversation", and the review tools would start one. Code in `Services/PluginConversations.php`. +With the API & Webhooks module, `GET /api/conversations` takes a `reviewPluginId` parameter: only the conversations with a review email about that plugin are listed, whatever their status, age, or who wrote to them, combined with the module's other parameters (`status`, `tag`…). The plugins team's review tools find the conversation of the plugin they review this way: any conversation for a first review, so a colleague's isn't started again, and `status=active,pending` for the next ones. It's an indexed lookup, a few milliseconds however many emails the mailbox has, unlike searching the emails' text (WPOrgSite's `body`). A value that isn't a plugin ID answers 400. Code in `Services/PluginConversations.php`. diff --git a/freescout.wordpress.net/Modules/WPOrgPluginReview/Services/PluginConversations.php b/freescout.wordpress.net/Modules/WPOrgPluginReview/Services/PluginConversations.php index 91484bc3a8..aa0ec47329 100644 --- a/freescout.wordpress.net/Modules/WPOrgPluginReview/Services/PluginConversations.php +++ b/freescout.wordpress.net/Modules/WPOrgPluginReview/Services/PluginConversations.php @@ -9,17 +9,8 @@ namespace Modules\WPOrgPluginReview\Services; -use Illuminate\Support\Facades\Schema; - /** - * Adds a `reviewPluginId` parameter to the API's conversation list: only the conversations with a review email about - * that plugin are listed, by the plugin ID of their Review ID lines (`P0TDX{plugin ID}HGN`), from the index of review - * emails. The review tools look up the conversation of the plugin they're reviewing this way, whatever its status, - * whoever wrote to it, and however many emails the plugins team's mailbox has. - * - * It goes through the API & Webhooks module's `api.conversations.query` filter, so the module keeps authenticating the - * key, limiting it to its mailboxes, combining it with its other parameters (`status`, `tag`…), and paginating; nothing - * here depends on its classes. + * Adds a `reviewPluginId` parameter to the API & Webhooks module's conversation list, from the index of review emails. */ final class PluginConversations { @@ -30,13 +21,6 @@ final class PluginConversations { */ public const PARAMETER = 'reviewPluginId'; - /** - * Whether the index has its plugin IDs, checked once per process. - * - * @var bool|null - */ - private static $ready = null; - /** * Limits the conversation list query to the conversations with a review email about the plugin. * @@ -55,15 +39,11 @@ public static function filter( mixed $query, mixed $request ): mixed { return $query; } - // Refused rather than ignored: an ignored filter would list every conversation as the plugin's. + // Ignoring it would list every conversation as the plugin's. if ( ! is_scalar( $plugin_id ) || ! ctype_digit( (string) $plugin_id ) || (int) $plugin_id <= 0 ) { abort( 400, 'The `' . self::PARAMETER . '` parameter takes a plugin ID, a positive number.' ); } - if ( ! self::ready() ) { - abort( 503, 'The plugin IDs of the review emails aren\'t indexed yet.' ); - } - $query->whereIn( 'conversations.id', static function ( $reviews ) use ( $plugin_id ): void { @@ -73,23 +53,4 @@ static function ( $reviews ) use ( $plugin_id ): void { return $query; } - - /** - * Whether the index has the plugin ID column, which its migration adds. - * - * @return bool - */ - private static function ready(): bool { - if ( null === self::$ready ) { - try { - self::$ready = Schema::hasColumn( Review::TABLE, 'plugin_id' ); - } catch ( \Throwable $e ) { - \Log::error( '[WPOrgPluginReview] Could not check the index of review emails: ' . $e->getMessage() ); - - return false; - } - } - - return self::$ready; - } } diff --git a/freescout.wordpress.net/Modules/WPOrgPluginReview/Services/Review.php b/freescout.wordpress.net/Modules/WPOrgPluginReview/Services/Review.php index a442918a26..2a09eb2a35 100644 --- a/freescout.wordpress.net/Modules/WPOrgPluginReview/Services/Review.php +++ b/freescout.wordpress.net/Modules/WPOrgPluginReview/Services/Review.php @@ -416,7 +416,6 @@ private static function row( Thread $thread, array $review_id ): array { 'conversation_id' => (int) $thread->conversation_id, 'type' => mb_substr( $review_id['type'], 0, 50 ), 'reviewed_at' => $thread->created_at ? (string) $thread->created_at : null, - // 0 when the line names no plugin; null is left for the rows indexed before the column, see IndexPluginIds. 'plugin_id' => max( 0, (int) ( $review_id['plugin_id'] ?? 0 ) ), ); } diff --git a/freescout.wordpress.net/Modules/WPOrgPluginReview/tests/PluginConversationsTest.php b/freescout.wordpress.net/Modules/WPOrgPluginReview/tests/PluginConversationsTest.php index f2dd7b0d38..142136758e 100644 --- a/freescout.wordpress.net/Modules/WPOrgPluginReview/tests/PluginConversationsTest.php +++ b/freescout.wordpress.net/Modules/WPOrgPluginReview/tests/PluginConversationsTest.php @@ -14,7 +14,6 @@ use App\Thread; use App\User; use Illuminate\Http\Request; -use Modules\WPOrgPluginReview\Jobs\IndexPluginIds; use Modules\WPOrgPluginReview\Providers\WPOrgPluginReviewServiceProvider; use Modules\WPOrgPluginReview\Services\PluginConversations; use Modules\WPOrgPluginReview\Services\Review; @@ -22,11 +21,9 @@ use WordPressdotorg\FreeScout\Tests\TestCase; /** - * Covers the plugin IDs of the index of review emails, filling them in for the rows indexed before them, and the - * `reviewPluginId` parameter of the API's conversation list. + * Covers the plugin IDs in the index of review emails, and the `reviewPluginId` parameter of the API's conversation list. * - * The API & Webhooks module is premium, so it isn't in the test environment: the filter is run on the query the module - * would build, the way the module runs it. + * The API & Webhooks module isn't in the test environment, so the filter runs on the query it would build. */ final class PluginConversationsTest extends TestCase { @@ -71,61 +68,7 @@ public function test_indexes_the_plugin_id_of_review_emails(): void { } /** - * The rows indexed before the plugin IDs get theirs; the ones that aren't review emails any more, or whose thread - * is gone, leave the index; and doing it again changes nothing. - * - * @return void - */ - public function test_indexes_the_plugin_ids_of_rows_indexed_before_them(): void { - $review = $this->review( 42 ); - $other = $this->review( 43 ); - $changed = $this->review( 44 ); - $kept = $this->review( 45 ); - - // As the rows were before the column, with a thread changed and another deleted without their hooks. - \DB::table( Review::TABLE )->where( 'thread_id', '!=', $kept->id )->update( array( 'plugin_id' => null ) ); - \DB::table( 'threads' )->where( 'id', $changed->id )->update( array( 'body' => '

Thanks for the update.

' ) ); - \DB::table( 'threads' )->where( 'id', $other->id )->delete(); - - $this->assertNull( ( new IndexPluginIds() )->index_batch() ); - - $this->assertSame( 42, $this->plugin_id( $review ) ); - $this->assertSame( 45, $this->plugin_id( $kept ) ); - $this->assertFalse( \DB::table( Review::TABLE )->where( 'thread_id', $other->id )->exists() ); - $this->assertFalse( \DB::table( Review::TABLE )->where( 'thread_id', $changed->id )->exists() ); - - \DB::table( Review::TABLE )->update( array( 'plugin_id' => null ) ); - \Artisan::call( - 'wporgpluginreview:index', - array( - '--plugin-ids' => true, - '--now' => true, - ) - ); - - $this->assertSame( array( 42, 45 ), \DB::table( Review::TABLE )->orderBy( 'plugin_id' )->pluck( 'plugin_id' )->map( 'intval' )->all() ); - } - - /** - * A batch starts after the last row the previous one read, so rows it can't read don't stop the rest. - * - * @return void - */ - public function test_continues_after_the_rows_of_the_previous_batch(): void { - $first = $this->review( 42 ); - $second = $this->review( 43 ); - \DB::table( Review::TABLE )->update( array( 'plugin_id' => null ) ); - - $after = (int) \DB::table( Review::TABLE )->where( 'thread_id', $first->id )->value( 'id' ); - $this->assertNull( ( new IndexPluginIds( $after ) )->index_batch() ); - - $this->assertNull( \DB::table( Review::TABLE )->where( 'thread_id', $first->id )->value( 'plugin_id' ) ); - $this->assertSame( 43, $this->plugin_id( $second ) ); - } - - /** - * The parameter lists every conversation with a review email about the plugin, open or closed, and combines with - * the module's other parameters. + * Every conversation with a review email about the plugin is listed, open or closed, with the module's other filters. * * @return void */ @@ -143,7 +86,7 @@ public function test_lists_the_conversations_of_a_plugin(): void { } /** - * A value that isn't a plugin ID is refused rather than ignored, which would list every conversation as the plugin's. + * A value that isn't a plugin ID is refused rather than ignored. * * @return void */ @@ -195,12 +138,10 @@ private function review( int $plugin_id ): Thread { * The plugin ID a thread is indexed with. * * @param Thread $thread Thread. - * @return int|null + * @return int */ - private function plugin_id( Thread $thread ): ?int { - $plugin_id = \DB::table( Review::TABLE )->where( 'thread_id', $thread->id )->value( 'plugin_id' ); - - return null === $plugin_id ? null : (int) $plugin_id; + private function plugin_id( Thread $thread ): int { + return (int) \DB::table( Review::TABLE )->where( 'thread_id', $thread->id )->value( 'plugin_id' ); } /** diff --git a/freescout.wordpress.net/Modules/WPOrgSite/Providers/WPOrgSiteServiceProvider.php b/freescout.wordpress.net/Modules/WPOrgSite/Providers/WPOrgSiteServiceProvider.php index 423b4fb9c9..5c3bba2452 100644 --- a/freescout.wordpress.net/Modules/WPOrgSite/Providers/WPOrgSiteServiceProvider.php +++ b/freescout.wordpress.net/Modules/WPOrgSite/Providers/WPOrgSiteServiceProvider.php @@ -99,7 +99,6 @@ static function ( array $styles ): array { } ); - // The API & Webhooks module's conversation list, searched by the text of the threads too. \Eventy::addFilter( 'api.conversations.query', array( ConversationTextSearch::class, 'filter' ), 20, 2 ); \Eventy::addFilter( diff --git a/freescout.wordpress.net/Modules/WPOrgSite/Services/ConversationTextSearch.php b/freescout.wordpress.net/Modules/WPOrgSite/Services/ConversationTextSearch.php index fa71846e24..3667ab436d 100644 --- a/freescout.wordpress.net/Modules/WPOrgSite/Services/ConversationTextSearch.php +++ b/freescout.wordpress.net/Modules/WPOrgSite/Services/ConversationTextSearch.php @@ -13,32 +13,10 @@ use Carbon\Carbon; /** - * Adds a `body` parameter to the API's conversation list: only conversations with an email, reply, or note - * containing that text are listed, like HelpScout's `body:` search, which the plugins team's review tools - * relied on. Three more parameters narrow which threads are read: + * Adds a `body` text search to the API & Webhooks module's conversation list, like HelpScout's. * - * - `bodyThreadType`: comma-separated `customer`, `message`, `note`; all three when it's left out. - * - `bodySince`, `bodyBefore`: only threads created since, or before, a date (ISO 8601, like `createdSince`). - * - * The API & Webhooks module lists conversations by sender, subject, tag, and dates, but not by what they say. - * It goes through the module's `api.conversations.query` filter rather than a route of our own, so the module - * keeps authenticating the key, limiting it to its mailboxes, paginating, and formatting the list, and nothing - * here depends on its classes. - * - * Thread bodies can't be indexed for this (a `LIKE '%…%'`, and core's tables aren't ours to change), so every - * search reads the body of every thread it covers, and that's what it costs: about 0.1 ms per email of 20 KB. - * It's kept to a bounded amount of them: - * - * - The module's other parameters (`status`, `tag`, `createdSince`, `customerId`…) pick the conversations - * first, from their indexes, and only their threads are read, through the threads' conversation index. - * A correlated `EXISTS` would leave that to the optimizer, which rewrites it into a scan of every thread - * in the helpdesk when the other filters look weak. - * - The threads it would read are counted first, from that index, and searches over MAX_THREADS of them, or - * over MAX_CONVERSATIONS conversations, are refused: the answer asks for narrower filters. - * - The matches go back to the module's query as IDs, so its count and its page don't search again. - * - * Refusals and failures answer an error rather than an empty list: an empty list would read as "no - * conversation says that", and the review tools act on that answer. Only requests with `body` are affected. + * Thread bodies can't be indexed, so the module's other parameters pick the conversations first, and searches over + * MAX_CONVERSATIONS or MAX_THREADS are refused. */ final class ConversationTextSearch { @@ -50,21 +28,21 @@ final class ConversationTextSearch { public const PARAMETER = 'body'; /** - * The parameter narrowing which threads are read by their type. + * Thread types to read: `customer`, `message`, `note`, comma-separated; all of them by default. * * @var string */ public const THREAD_TYPE_PARAMETER = 'bodyThreadType'; /** - * The parameter reading only the threads created since a date. + * Only threads created since this date. * * @var string */ public const SINCE_PARAMETER = 'bodySince'; /** - * The parameter reading only the threads created before a date. + * Only threads created before this date. * * @var string */ @@ -78,22 +56,14 @@ final class ConversationTextSearch { public const MIN_LENGTH = 3; /** - * The most threads one search reads, what a search costs: about 1.5 seconds when they're the plugins team's - * review emails, of 20 KB on average. Finding a plugin's review among the active ones reads about 2,000 of - * them; every review email of 6 months is about 100,000, at 1,200 new reviews a week. - * - * Can be changed with the `wporgsite.conversation_text_search.max_threads` filter. + * The most threads one search reads, about 1.5 seconds of review emails. * * @var int */ public const MAX_THREADS = 15000; /** - * The most conversations whose threads one search counts. Only their IDs and their threads' index are read, - * about 0.1 seconds for 30,000, but it keeps a search with no other filter from listing the whole helpdesk - * before refusing it. 3 months of approved reviews are about 10,000 conversations. - * - * Can be changed with the `wporgsite.conversation_text_search.max_conversations` filter. + * The most conversations one search covers, so one without other filters is refused before listing them all. * * @var int */ @@ -209,7 +179,7 @@ private static function thread_types( mixed $value ): array { * @param string $parameter Name of the parameter, for the error. * @param mixed $value Value of the parameter. * @return string|null - * @throws \InvalidArgumentException If the value isn't a string; caught, so it's refused like any other value that isn't a date. + * @throws \InvalidArgumentException Caught, for a value that isn't a string. */ private static function date( string $parameter, mixed $value ): ?string { if ( null === $value || '' === $value ) { @@ -237,7 +207,7 @@ private static function date( string $parameter, mixed $value ): ?string { private static function candidate_ids( mixed $query, int $max ): array { $candidates = ( clone $query )->getQuery(); - // Only the IDs: without the module's columns, order, and page. + // Without the module's columns, order, and page. $candidates->columns = null; $candidates->orders = null; $candidates->offset = null; @@ -249,9 +219,7 @@ private static function candidate_ids( mixed $query, int $max ): array { } /** - * The threads of the candidates a search would read, counted from the threads' indexes. - * - * Drafts are counted too: the state isn't in those indexes, and they're few. + * The threads of the candidates a search would read, drafts included, as their state isn't indexed. * * @param mixed $query Query for the conversations, for its connection. * @param int[] $candidates Conversation IDs. @@ -319,7 +287,7 @@ private static function threads_query( mixed $query, array $chunk, array $thread } /** - * Answers the request with an error, which the module's API turns into JSON. + * Answers with an error, as an empty list would read as no match. * * @param int $status HTTP status. * @param string $message Why. diff --git a/freescout.wordpress.net/Modules/WPOrgSite/module.json b/freescout.wordpress.net/Modules/WPOrgSite/module.json index 5597cb7209..a4923ea054 100644 --- a/freescout.wordpress.net/Modules/WPOrgSite/module.json +++ b/freescout.wordpress.net/Modules/WPOrgSite/module.json @@ -3,7 +3,7 @@ "alias": "wporgsite", "img": "/modules/wporgsite/img/icon.svg", "description": "Adapts FreeScout to how WordPress.org runs it, e.g. modules come from deploys, not from updates here.", - "version": "1.1.0", + "version": "1.2.0", "detailsUrl": "https://github.com/WordPress/wordpress.org/tree/trunk/freescout.wordpress.net/Modules/WPOrgSite", "author": "WordPress.org", "authorUrl": "https://wordpress.org/", diff --git a/freescout.wordpress.net/Modules/WPOrgSite/tests/ConversationTextSearchTest.php b/freescout.wordpress.net/Modules/WPOrgSite/tests/ConversationTextSearchTest.php index 108838b3c0..cb3423be23 100644 --- a/freescout.wordpress.net/Modules/WPOrgSite/tests/ConversationTextSearchTest.php +++ b/freescout.wordpress.net/Modules/WPOrgSite/tests/ConversationTextSearchTest.php @@ -20,8 +20,7 @@ /** * Covers the `body` parameter of the API's conversation list. * - * The API & Webhooks module is premium, so it isn't in the test environment: the filter is run on the query - * the module would build, the way the module runs it. + * The API & Webhooks module isn't in the test environment, so the filter runs on the query it would build. */ final class ConversationTextSearchTest extends TestCase { @@ -105,8 +104,7 @@ public function test_reads_only_the_thread_types_asked_for(): void { } /** - * Without the parameter the list is left alone, and a search that can't be made is refused rather than - * listing nothing, which would read as nothing matching. + * Without the parameter the list is left alone; a search that can't be made is refused. * * @return void */ @@ -133,12 +131,13 @@ public function test_refuses_searches_over_too_many_conversations(): void { $this->create_thread( $conversation, Thread::TYPE_CUSTOMER, 'Hello there ' . $i, null, '2026-10-01 10:00:00' ); } - \Eventy::addFilter( 'wporgsite.conversation_text_search.max_conversations', static fn(): int => 2 ); + $max = static fn(): int => 2; + \Eventy::addFilter( 'wporgsite.conversation_text_search.max_conversations', $max ); try { $this->assert_refused( 400, 'Hello' ); } finally { - \Eventy::removeAllFilters( 'wporgsite.conversation_text_search.max_conversations' ); + \Eventy::removeFilter( 'wporgsite.conversation_text_search.max_conversations', $max ); } $this->assertCount( 3, $this->search( 'Hello' ) ); @@ -157,14 +156,15 @@ public function test_refuses_searches_over_too_many_threads(): void { $this->create_thread( $conversation, Thread::TYPE_CUSTOMER, 'Hello again', null, '2026-10-02 10:00:00' ); $this->create_thread( $conversation, Thread::TYPE_MESSAGE, 'Hello, P0TDX123HGN', $agent, '2026-10-03 10:00:00' ); - \Eventy::addFilter( 'wporgsite.conversation_text_search.max_threads', static fn(): int => 2 ); + $max = static fn(): int => 2; + \Eventy::addFilter( 'wporgsite.conversation_text_search.max_threads', $max ); try { $this->assert_refused( 400, 'Hello' ); $this->assertSame( array( $conversation->id ), $this->search( 'Hello', 'message' ) ); $this->assertSame( array( $conversation->id ), $this->search( 'Hello', null, array( ConversationTextSearch::SINCE_PARAMETER => '2026-10-02T00:00:00Z' ) ) ); } finally { - \Eventy::removeAllFilters( 'wporgsite.conversation_text_search.max_threads' ); + \Eventy::removeFilter( 'wporgsite.conversation_text_search.max_threads', $max ); } } From 5b29c3967521828b43e75f6942773b344145efbf Mon Sep 17 00:00:00 2001 From: Konstantin Obenland Date: Tue, 20 Oct 2026 14:08:05 -0500 Subject: [PATCH 3/3] FreeScout: Trim the README on searching conversations' text The measurements belong in the pull request; the README keeps the parameters, an example, the limits, and the errors. Why the matches go back as IDs moves to the code. Co-Authored-By: Claude Opus 5.5 --- freescout.wordpress.net/Modules/WPOrgSite/README.md | 13 +++---------- .../WPOrgSite/Services/ConversationTextSearch.php | 1 + 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/freescout.wordpress.net/Modules/WPOrgSite/README.md b/freescout.wordpress.net/Modules/WPOrgSite/README.md index acbef03f5c..9622ddc2da 100644 --- a/freescout.wordpress.net/Modules/WPOrgSite/README.md +++ b/freescout.wordpress.net/Modules/WPOrgSite/README.md @@ -15,16 +15,9 @@ Adapts FreeScout to how WordPress.org runs it. Small changes to FreeScout's own - **Pinned header:** [a conversation's header can stay in view](#pinned-header) while the agent reads the emails below it. - **Alternate conversation design:** agents who choose so in their profile get [conversations laid out otherwise](#alternate-conversation-design), oldest email first, with Reply and Note at the bottom. - **Conversations:** [keyboard shortcuts](#keyboard-shortcuts); the status and assignee selectors pinned to the top of the window while their menu is open, further down long conversations where the toolbar has scrolled out of view, so they can be chosen there, like after pressing `s` or `a`; the subject at 18px and the threads' authors at 16px, rather than core's 23px and 18px, so more of the conversation fits; and clicking the number next to the subject copies it, without the `#`. Its tooltip says when it's copied; where the browser doesn't let the page write to the clipboard, it copies through a selected field instead. It's all in `Public/js/conversation.js`. -- **Searching conversations' text through the API:** with the API & Webhooks module, `GET /api/conversations` takes a `body` parameter, like HelpScout's `body:` search: only conversations with a published email, reply, or note containing that text are listed. It's matched literally (`%` and `_` included) and ignoring case. Three more parameters narrow which threads are read: `bodyThreadType` (`customer`, `message`, `note`, comma-separated) and `bodySince` / `bodyBefore` (threads created since, or before, an ISO 8601 date). The plugins team's review tools find a plugin's review by the code every review email ends with, reading only the team's emails of the last 6 months: `?mailboxId=12&status=active&tag=plugin-review&body=P0TDX123HGN&bodyThreadType=message&bodySince=2026-04-01T00:00:00Z&embed=threads`. The module still authenticates the key, limits it to its mailboxes, and paginates. Code in `Services/ConversationTextSearch.php`. - - **How it searches:** thread bodies can't be indexed for this, so a search reads the body of every thread it covers. The module's other parameters pick the conversations first, from their indexes. The threads of those conversations that the `body*` parameters cover are then counted from the threads' indexes, and only then read, 500 conversations at a time. The module's count and page get the matches as IDs, so the search runs once per request. A correlated `EXISTS` would leave the plan to MariaDB, which turns it into a scan of every thread when the other filters look weak. - - **Limits:** a search reads at most 15,000 threads (filter `wporgsite.conversation_text_search.max_threads`) and counts the threads of at most 25,000 conversations (`…max_conversations`); beyond that it's refused. Reading costs about 0.1 ms per review email of 20 KB, so 15,000 threads is about 2 seconds. Every page of a search runs it again, so keep searches to what fits in a page or two. - - **Measured** on a copy with 200,000 conversations: the last 6 months at 1,500 new reviews a week, 39,000 conversations with 3 GB of emails, and review emails of 20 KB on average. Warm cache, local Docker. - - Finding a plugin's review among ~1,000 active ones reads ~2,100 team emails: about 0.3 s. - - With a backlog of ~4,900 active reviews it reads ~14,000: about 1.9 s, close to the limit, so a backlog of about 5,000 active reviews is as far as the lookup goes. - - Every review email of 6 months (~100,000) would take 10–12 s and is refused. - - 2 months of approved reviews, searched a week at a time, take 3.5 s in 9 requests. - - Counting the threads of 32,000 conversations alone takes 0.1 s. - - **Errors, not empty lists:** a text shorter than 3 characters, an unknown thread type, a date that isn't one, or a search over the limits answers 400 with what to change, and a failure answers 500. An empty list would read as "nothing says that", which the review tools act on. +- **Searching conversations' text through the API:** with the API & Webhooks module, `GET /api/conversations` takes a `body` parameter, like HelpScout's `body:` search: conversations with a published email, reply, or note containing the text, matched literally and ignoring case. `bodyThreadType` (`customer`, `message`, `note`, comma-separated), `bodySince`, and `bodyBefore` (ISO 8601) narrow which threads are read. The review tools find a plugin's review with `?mailboxId=12&status=active&tag=plugin-review&body=P0TDX123HGN&bodyThreadType=message&bodySince=2026-04-01T00:00:00Z`. Code in `Services/ConversationTextSearch.php`. + - **Limits:** thread bodies can't be indexed, so the module's other parameters pick the conversations first, and a search over 25,000 conversations or 15,000 threads (about 2 seconds) is refused; see the `wporgsite.conversation_text_search.max_conversations` and `…max_threads` filters. Every page runs the search again. + - **Errors:** a text under 3 characters, an unknown thread type, a bad date, or a search over the limits answers 400, and a failure 500, as an empty list would read as no match. ## Keyboard shortcuts diff --git a/freescout.wordpress.net/Modules/WPOrgSite/Services/ConversationTextSearch.php b/freescout.wordpress.net/Modules/WPOrgSite/Services/ConversationTextSearch.php index 3667ab436d..0cb44a8a54 100644 --- a/freescout.wordpress.net/Modules/WPOrgSite/Services/ConversationTextSearch.php +++ b/freescout.wordpress.net/Modules/WPOrgSite/Services/ConversationTextSearch.php @@ -144,6 +144,7 @@ public static function filter( mixed $query, mixed $request ): mixed { if ( empty( $matches ) ) { $query->whereRaw( '1 = 0' ); } else { + // IDs, not a correlated EXISTS, which MariaDB can turn into a scan of every thread. $query->whereIn( 'conversations.id', $matches ); }