diff --git a/common/includes/slack/trac/comment-handler.php b/common/includes/slack/trac/comment-handler.php index ae47f599eb..8278bd8f99 100644 --- a/common/includes/slack/trac/comment-handler.php +++ b/common/includes/slack/trac/comment-handler.php @@ -16,6 +16,13 @@ class Comment_Handler { protected $comment_id; protected $comment_url; + /** + * The ticket's component, read from the notification's ticket properties table. + * + * @var string + */ + protected $component; + /** * Workflow keywords added to the ticket by this change. * @@ -63,6 +70,11 @@ function run() { $this->send->send( $firehose ); } + $component_firehose = $this->trac->get_component_firehose_channel( $this->component ); + if ( $component_firehose ) { + $this->send->send( $component_firehose ); + } + // Send notifications for Workflow Keyword additions. $this->send_keyword_notifications(); } @@ -200,9 +212,13 @@ function process_message() { } // Remove top border of table. array_shift( $lines ); - // Remove ticket properties table. + // Remove ticket properties table, keeping the component. It can be in either column. + $component = ''; while ( $lines && 0 !== strpos( current( $lines ), '------' ) ) { - array_shift( $lines ); + $line = array_shift( $lines ); + if ( preg_match( '/(?:^|\|)\s*Component:\s+([^|]*?)\s*(?:\||$)/', $line, $matches ) ) { + $component = $matches[1]; + } } // Remove bottom border of table. array_shift( $lines ); @@ -290,6 +306,7 @@ function process_message() { $this->author = $author; $this->comment = $comment; $this->changes = $changes; + $this->component = $component; $this->ticket_id = $ticket_id; $this->ticket_url = $ticket_url; $this->comment_id = $comment_id; diff --git a/common/includes/slack/trac/commit-handler.php b/common/includes/slack/trac/commit-handler.php index cf22dbc45a..492bae15ab 100644 --- a/common/includes/slack/trac/commit-handler.php +++ b/common/includes/slack/trac/commit-handler.php @@ -20,9 +20,14 @@ public function __construct( Send $send, $trac, $repo, $rev ) { } public function run() { - $this->generate_payload(); + // One payload per commit format, so each channel gets the full message or just the summary. + $payloads = array(); foreach ( $this->trac->get_commit_channels( $this->svnlook( 'changed' ), $this->svnlook( 'log' ) ) as $channel ) { - $this->send->send( $channel ); + $format = $this->trac->get_commit_format( $channel ); + if ( ! isset( $payloads[ $format ] ) ) { + $payloads[ $format ] = $this->generate_payload( $format ); + } + $payloads[ $format ]->send( $channel ); } } @@ -34,9 +39,20 @@ public function set_svnlook_executable( $path ) { $this->svnlook = $path; } - protected function generate_payload() { + /** + * Builds the Slack message for the commit. + * + * @param string $format 'full' for the whole commit message, 'summary' for its first line. + * @return Send A copy of the Send object with the commit attached. + */ + protected function generate_payload( $format = 'full' ) { + $log = $this->svnlook( 'log' ); + if ( 'summary' === $format ) { + $log = Commit::get_summary( $log ); + } + $author = $this->svnlook( 'author' ); - $message = Commit::format_commit_for_slack( $this->trac, $this->svnlook( 'log' ) ); + $message = Commit::format_commit_for_slack( $this->trac, $log ); $date = $this->svnlook( 'date' ); $url = $this->trac->get_commit_template( $this->rev ); $revision = 'r' . $this->rev; @@ -46,21 +62,26 @@ protected function generate_payload() { $title = "$username $revision"; $fallback = "$username $revision by $author"; - $this->send->set_username( $username ); - $this->send->set_icon( $icon ); - $this->send->add_attachment( array( - 'title' => $title, - 'title_link' => $url, - 'author_name' => $author, - 'author_icon' => sprintf( 'https://wordpress.org/grav-redirect.php?user=%s&s=32', $author ), - 'color' => $color, - 'text' => $message, - 'fallback' => $fallback, - 'mrkdwn_in' => array( 'text' ), - 'ts' => $date, - 'footer' => sprintf( '<%s|%s>', $this->trac->get_url(), $this->trac->get_name() ), - 'footer_icon' => sprintf( '%s/chrome/common/trac.ico', $this->trac->get_url() ), - ) ); + $send = clone $this->send; + $send->set_username( $username ); + $send->set_icon( $icon ); + $send->add_attachment( + array( + 'title' => $title, + 'title_link' => $url, + 'author_name' => $author, + 'author_icon' => sprintf( 'https://wordpress.org/grav-redirect.php?user=%s&s=32', $author ), + 'color' => $color, + 'text' => $message, + 'fallback' => $fallback, + 'mrkdwn_in' => array( 'text' ), + 'ts' => $date, + 'footer' => sprintf( '<%s|%s>', $this->trac->get_url(), $this->trac->get_name() ), + 'footer_icon' => sprintf( '%s/chrome/common/trac.ico', $this->trac->get_url() ), + ) + ); + + return $send; } protected function svnlook( $subcommand ) { diff --git a/common/includes/slack/trac/commit.php b/common/includes/slack/trac/commit.php index 067a6c789a..cef5b5ba3d 100644 --- a/common/includes/slack/trac/commit.php +++ b/common/includes/slack/trac/commit.php @@ -92,6 +92,22 @@ function fetch() { return $this->data; } + /** + * Gets the summary of a commit message: its first non-empty line. + * + * @param string $message Commit message. + * @return string + */ + public static function get_summary( $message ) { + foreach ( explode( "\n", $message ) as $line ) { + $line = trim( $line ); + if ( '' !== $line ) { + return $line; + } + } + return ''; + } + static function format_commit_for_slack( Trac $trac, $message ) { // Convert ASCII numbers to an UTF-8 character, like ?\226?\128?\148 => — (m-dash). $message = preg_replace_callback( '/(?:\?\\\\(\d{1,3}))/', function( $matches ) { diff --git a/common/includes/slack/trac/config.php b/common/includes/slack/trac/config.php index a4e0d1b4b4..cbeef0202e 100644 --- a/common/includes/slack/trac/config.php +++ b/common/includes/slack/trac/config.php @@ -12,6 +12,13 @@ class Core extends Trac { protected $primary_channel_ticket_format = 'title'; + /** + * Post only the first line of each commit message to #core. + * + * @var string + */ + protected $primary_channel_commit_format = 'summary'; + /** * File paths that cause commits to be piped to particular channels. * Start regex matches with # as your delimiter. @@ -54,7 +61,7 @@ class Core extends Trac { 'webpack.config.js' => '#core-build-test-tools', 'wp-tests-config-sample.php' => '#core-build-test-tools', '#\.n(v|p)mrc#' => '#core-build-test-tools', - '#\.version-support-[a-z]+\.json' => '#core-build-test-tools', + '#\.version-support-[a-z]+\.json#' => '#core-build-test-tools', '#^package(-lock)?\.json$#' => '#core-build-test-tools', '#^php[a-z]+\.xml\.dist$#' => '#core-build-test-tools', ); @@ -70,34 +77,97 @@ class Core extends Trac { ); /** - * Components or focuses that cause new tickets to be piped to particular channels. + * Maps each Core Trac component to its component specific channel. + * + * Notifications for a component go to that channel's -firehose, -newtickets and -commits + * channels, not to the channel itself. + * + * Subcomponents map to their parent's channel, unless they have their own. + * + * Some components have no discussion channel, or an archived one, but still have + * -firehose, -newtickets and -commits channels. + * + * Retired components have an empty value, and only post to the global channels. + * + * @var array */ - protected $ticket_component_filters = array( + protected $component_channels = array( + 'AI' => '#core-ai', 'Build/Test Tools' => '#core-build-test-tools', 'Bundled Theme' => '#core-themes', - 'Customize' => '#core-customize', - 'Date/Time' => '#core-datetime', - 'Press This' => '#core-pressthis', - 'multisite' => '#core-multisite', - 'Networks and Sites' => '#core-multisite', - 'REST API' => '#core-restapi', - 'rest-api' => '#core-restapi', + 'Editor' => '#core-editor', + 'HTML API' => '#core-html-api', 'I18N' => '#core-i18n', + 'Import' => '#core-importers', + 'Interactivity API' => '#core-interactivity-api', 'Media' => '#core-media', - 'docs' => '#core-docs', - 'css' => '#core-css', - 'javascript' => '#core-js', - 'coding-standards' => '#core-coding-standards', - 'performance' => '#core-performance', + 'Networks and Sites' => '#core-multisite', 'Privacy' => '#core-privacy', - 'privacy' => '#core-privacy', - 'Site Health' => '#core-site-health', - 'Sitemaps' => '#core-sitemaps', + 'REST API' => '#core-restapi', + 'Themes' => '#core-themes', 'Upgrade/Install' => '#core-upgrade-install', - 'Application Passwords' => [ '#core-passwords' => true, '#core-restapi' => true ], + + // Subcomponents. + 'Abilities API' => '#core-ai', + 'Application Passwords' => '#core-restapi', + 'Autosave' => '#core-editor', + 'Canonical' => '#core-permalinks', + 'Charset' => '#core-formatting', + 'Embeds' => '#core-media', + 'Emoji' => '#core-formatting', + 'Gallery' => '#core-media', 'Login and Registration' => '#core-passwords', - 'HTML API' => '#core-html-api', - 'Interactivity API' => '#core-interactivity-api', + 'Menus' => '#core-themes', + 'Multisite' => '#core-multisite', + 'Notes' => '#core-notes', + 'Pings/Trackbacks' => '#core-comments', + 'Post Formats' => '#core-posts-post-types', + 'Post Thumbnails' => '#core-posts-post-types', + 'Quick/Bulk Edit' => '#core-editor', + 'Revisions' => '#core-posts-post-types', + 'Rewrite Rules' => '#core-permalinks', + 'Role/Capability' => '#core-users', + 'Shortcodes' => '#core-formatting', + 'TinyMCE' => '#core-editor', // #core-tinymce has been archived since 2025-06-24. + 'Upload' => '#core-media', + 'Widgets' => '#core-themes', + + // Components with an archived component specific channel. + 'Bootstrap/Load' => '#core-bootstrap', + 'Comments' => '#core-comments', + 'Customize' => '#core-customize', + 'Date/Time' => '#core-datetime', + 'HTTP API' => '#core-http-api', + 'Site Health' => '#core-site-health', + 'Sitemaps' => '#core-sitemaps', + + // Components that have never had a component specific channel. + 'Administration' => '#core-administration', + 'Cache API' => '#core-cache-api', + 'Cron API' => '#core-cron-api', + 'Database' => '#core-database', + 'Export' => '#core-export', + 'External Libraries' => '#core-external-libraries', + 'Feeds' => '#core-feeds', + 'Filesystem API' => '#core-filesystem-api', + 'Formatting' => '#core-formatting', + 'General' => '#core-general', + 'Help/About' => '#core-help-about', + 'Mail' => '#core-mail', + 'Options, Meta APIs' => '#core-options-meta-apis', + 'Permalinks' => '#core-permalinks', + 'Plugins' => '#core-plugins', + 'Posts, Post Types' => '#core-posts-post-types', + 'Query' => '#core-query', + 'Script Loader' => '#core-script-loader', + 'Security' => '#core-security', + 'Taxonomy' => '#core-taxonomy', + 'Toolbar' => '#core-toolbar', + 'Users' => '#core-users', + 'XML-RPC' => '#core-xml-rpc', + + // Retired components. + 'Press This' => '', // #core-pressthis is archived. ); } diff --git a/common/includes/slack/trac/trac.php b/common/includes/slack/trac/trac.php index 34314412eb..1eb4829e5e 100644 --- a/common/includes/slack/trac/trac.php +++ b/common/includes/slack/trac/trac.php @@ -23,6 +23,15 @@ class Trac implements User { // 'title', 'fields', 'description' protected $primary_channel_ticket_format = 'description'; + /** + * How much of the commit message to post to the primary channel. + * + * 'full' posts the whole commit message, 'summary' only its first line. + * + * @var string + */ + protected $primary_channel_commit_format = 'full'; + protected $bypass_primary_channel_for_commit_filter_matches = false; protected $bypass_primary_channel_for_ticket_filter_matches = false; @@ -36,6 +45,19 @@ class Trac implements User { */ protected $ticket_keyword_filters = array(); + /** + * Maps Trac components (or commit message prefixes) to component specific channels. + * + * Notifications for a component go to that channel's -firehose, -newtickets and -commits + * channels, not to the channel itself. This includes commits that $commit_path_filters + * sends to one of these channels. + * + * Components with an empty value have no component channels. + * + * @var array + */ + protected $component_channels = array(); + protected $color = '#0073aa'; protected $icon = ':wordpress:'; @@ -250,6 +272,21 @@ function get_commit_channels( $changed_files = null, $log_text = null ) { } } + foreach ( $this->component_channels as $component => $channel ) { + if ( $channel && 0 === stripos( $log_text, $component ) ) { + $channels[ $channel ] = true; + } + } + + // Commits for a component go to its -commits and -firehose channels rather than the component channel itself. + foreach ( array_unique( array_filter( $this->component_channels ) ) as $channel ) { + if ( ! empty( $channels[ $channel ] ) ) { + unset( $channels[ $channel ] ); + $channels[ $channel . '-commits' ] = true; + $channels[ $channel . '-firehose' ] = true; + } + } + if ( $this->firehose_channel ) { $channels[ $this->firehose_channel ] = true; } @@ -291,6 +328,14 @@ function get_ticket_channels( $ticket = null ) { } } + // New tickets in a component with a channel go to that channel's -newtickets and -firehose channels. + if ( isset( $ticket->component ) && ! empty( $this->component_channels[ $ticket->component ] ) ) { + $channel = $this->component_channels[ $ticket->component ]; + + $channels[ $channel . '-newtickets' ] = true; + $channels[ $channel . '-firehose' ] = true; + } + if ( $this->firehose_channel ) { $channels[ $this->firehose_channel ] = true; } @@ -302,6 +347,19 @@ function get_firehose_channel() { return $this->firehose_channel; } + /** + * Gets the firehose channel for a component. + * + * @param string $component Trac component name. + * @return string|false Channel name, or false if the component has no channels. + */ + public function get_component_firehose_channel( $component ) { + if ( ! empty( $this->component_channels[ $component ] ) ) { + return $this->component_channels[ $component ] . '-firehose'; + } + return false; + } + /** * Gets the channels a ticket update should be posted to when a workflow keyword is added. * @@ -323,6 +381,19 @@ function get_ticket_format( $channel ) { return 'description'; } + /** + * Gets how much of the commit message to post to a channel. + * + * @param string $channel Channel name. + * @return string 'full' or 'summary'. + */ + public function get_commit_format( $channel ) { + if ( $channel === $this->primary_channel ) { + return $this->primary_channel_commit_format; + } + return 'full'; + } + /** * Stop message text people typed from being read as Slack markup. * diff --git a/common/includes/tests/slack/trac/channels.php b/common/includes/tests/slack/trac/channels.php new file mode 100644 index 0000000000..84c66e045f --- /dev/null +++ b/common/includes/tests/slack/trac/channels.php @@ -0,0 +1,141 @@ +get_commit_channels( $changed_files, $log ); + + sort( $channels ); + sort( $expected ); + $this->assertSame( $expected, $channels ); + } + + /** + * Data provider for test_core_commit_channels(). + * + * @return array[] + */ + public static function data_core_commit_channels(): array { + $global = array( '#core', '#core-commits', '#core-firehose' ); + + return array( + 'component prefix' => array( + 'trunk/src/wp-admin/import.php', + "Import: Check the file type.\n\nFixes #12345.", + array_merge( $global, array( '#core-importers-commits', '#core-importers-firehose' ) ), + ), + 'component without a discussion channel' => array( + 'trunk/src/wp-includes/class-wp-query.php', + 'Query: Cache the results.', + array_merge( $global, array( '#core-query-commits', '#core-query-firehose' ) ), + ), + 'archived channel path filter' => array( + 'trunk/src/wp-includes/sitemaps/class-wp-sitemaps.php', + 'Fix a typo.', + array_merge( $global, array( '#core-sitemaps-commits', '#core-sitemaps-firehose' ) ), + ), + 'subcomponent prefix' => array( + 'trunk/src/wp-includes/embed.php', + 'Embeds: Fix the iframe title.', + array_merge( $global, array( '#core-media-commits', '#core-media-firehose' ) ), + ), + 'bundled themes' => array( + 'trunk/src/wp-includes/theme.php', + 'Bundled Themes: Fix a typo.', + array_merge( $global, array( '#core-themes-commits', '#core-themes-firehose' ) ), + ), + 'path filter' => array( + 'trunk/src/wp-includes/html-api/class-wp-html-processor.php', + 'Fix a typo.', + array_merge( $global, array( '#core-html-api-commits', '#core-html-api-firehose' ) ), + ), + 'path filter and prefix' => array( + 'trunk/src/wp-includes/media.php', + 'I18N: Add context to a string.', + array_merge( $global, array( '#core-i18n-commits', '#core-i18n-firehose', '#core-media-commits', '#core-media-firehose' ) ), + ), + 'no component' => array( + 'trunk/src/wp-includes/formatting.php', + 'Docs: Correct a parameter type.', + $global, + ), + ); + } + + /** + * New Core tickets post to the global channels and the component's channels. + */ + public function test_core_new_ticket_channels(): void { + $ticket = (object) array( + 'component' => 'Gallery', + 'focuses' => 'css, javascript', + 'keywords' => 'has-patch needs-testing', + ); + + $channels = Trac::get( 'core' )->get_ticket_channels( $ticket ); + + sort( $channels ); + $this->assertSame( + array( '#core', '#core-firehose', '#core-media-firehose', '#core-media-newtickets', '#core-newtickets', '#core-test' ), + $channels + ); + } + + /** + * New Core tickets in a component that isn't listed post only to the global channels. + */ + public function test_core_new_ticket_unlisted_component(): void { + $channels = Trac::get( 'core' )->get_ticket_channels( (object) array( 'component' => 'Text Changes' ) ); + + sort( $channels ); + $this->assertSame( array( '#core', '#core-firehose', '#core-newtickets' ), $channels ); + } + + /** + * Core ticket updates post to the component's firehose. + */ + public function test_core_component_firehose_channel(): void { + $trac = Trac::get( 'core' ); + + $this->assertSame( '#core-passwords-firehose', $trac->get_component_firehose_channel( 'Login and Registration' ) ); + $this->assertSame( '#core-formatting-firehose', $trac->get_component_firehose_channel( 'Shortcodes' ) ); + $this->assertFalse( $trac->get_component_firehose_channel( 'Text Changes' ) ); + $this->assertFalse( $trac->get_component_firehose_channel( 'Not A Component' ) ); + } + + /** + * Meta has no component channels, so its routing is unchanged. + */ + public function test_meta_commit_channels_are_unchanged(): void { + $channels = Trac::get( 'meta' )->get_commit_channels( + 'sites/trunk/wordpress.org/public_html/wp-content/plugins/plugin-directory/class-plugin-directory.php', + 'Plugin Directory: Fix a thing.' + ); + + sort( $channels ); + $this->assertSame( array( '#meta', '#meta-commits', '#meta-firehose', '#pluginreview' ), $channels ); + } +} diff --git a/common/includes/tests/slack/trac/comment-handler.php b/common/includes/tests/slack/trac/comment-handler.php index 9cf7faf8a0..c4dbbe764c 100644 --- a/common/includes/tests/slack/trac/comment-handler.php +++ b/common/includes/tests/slack/trac/comment-handler.php @@ -60,6 +60,32 @@ public function test_last_line_is_kept( array $body, string $comment, array $cha $this->assertSame( $changes, $handler->changes ); } + /** + * The ticket's component is read from either column of the properties table. + */ + public function test_component_is_read(): void { + $body = array( + '', + 'Comment (by desrosj):', + '', + ' Does anyone have any idea why Nacin would make this change?', + ); + + $handler = new \Dotorg\Slack\Trac\Comment_Handler( new \Dotorg\Slack\Send( '' ), $this->get_email( $body ) ); + $handler->process_message(); + $this->assertSame( 'Trac', $handler->component ); + + $email = str_replace( + array( 'Component: Trac | Resolution:', ' Type: defect (bug) | Status: new' ), + array( ' Type: defect (bug) | Resolution:', ' Priority: normal | Component: Build/Test Tools' ), + $this->get_email( $body ) + ); + + $handler = new \Dotorg\Slack\Trac\Comment_Handler( new \Dotorg\Slack\Send( '' ), $email ); + $handler->process_message(); + $this->assertSame( 'Build/Test Tools', $handler->component ); + } + /** * Data provider for test_last_line_is_kept(). * diff --git a/common/includes/tests/slack/trac/commit-handler.php b/common/includes/tests/slack/trac/commit-handler.php new file mode 100644 index 0000000000..6d9756f528 --- /dev/null +++ b/common/includes/tests/slack/trac/commit-handler.php @@ -0,0 +1,160 @@ +sent = $sent; + } + + public function send( $channel, $thread = false ) { + $attachments = $this->get_attachments(); + $this->sent[ $channel ] = $attachments[0]['text']; + } + }; + + $handler = new class( $send, $trac, '/repo', 1 ) extends Commit_Handler { + public $svnlook_output = array(); + + protected function svnlook( $subcommand ) { + return $this->svnlook_output[ $subcommand ]; + } + }; + + $handler->svnlook_output = array( + 'changed' => $changed_files, + 'log' => $log, + 'author' => 'desrosj', + 'date' => '1762732800', + ); + $handler->run(); + + return $sent->getArrayCopy(); + } + + /** + * #core gets the commit summary; every other channel gets the full message. + */ + public function test_core_commit_summary_only_goes_to_core(): void { + $sent = $this->run_handler( 'core', 'trunk/src/wp-includes/media.php', self::LOG ); + + $this->assertSame( + array( '#core', '#core-commits', '#core-firehose', '#core-media-commits', '#core-media-firehose' ), + $this->sorted_keys( $sent ) + ); + + $this->assertSame( 'Media: Add a missing alt attribute.', $sent['#core'] ); + + foreach ( array( '#core-commits', '#core-firehose', '#core-media-commits', '#core-media-firehose' ) as $channel ) { + $this->assertStringContainsString( 'Props desrosj.', $sent[ $channel ], "$channel should get the full commit message." ); + } + } + + /** + * Meta posts the full commit message everywhere, including its primary channel. + */ + public function test_meta_commit_is_unchanged(): void { + $sent = $this->run_handler( + 'meta', + 'sites/trunk/wordpress.org/public_html/wp-content/plugins/plugin-directory/class-plugin-directory.php', + "Plugin Directory: Fix a thing.\n\nProps desrosj." + ); + + $this->assertNotEmpty( $sent ); + foreach ( $sent as $channel => $text ) { + $this->assertStringContainsString( 'Props desrosj.', $text, "$channel should get the full commit message." ); + } + } + + /** + * Only Core's primary channel uses the summary format. + */ + public function test_get_commit_format(): void { + $core = Trac::get( 'core' ); + + $this->assertSame( 'summary', $core->get_commit_format( '#core' ) ); + $this->assertSame( 'full', $core->get_commit_format( '#core-commits' ) ); + $this->assertSame( 'full', $core->get_commit_format( '#core-media-commits' ) ); + $this->assertSame( 'full', Trac::get( 'meta' )->get_commit_format( '#meta' ) ); + } + + /** + * The summary is the first non-empty line of the commit message. + */ + public function test_get_summary(): void { + $this->assertSame( 'Media: Add a missing alt attribute.', Commit::get_summary( self::LOG ) ); + $this->assertSame( 'Docs: Fix a typo.', Commit::get_summary( "\n Docs: Fix a typo. \n\nProps someone." ) ); + $this->assertSame( 'One line.', Commit::get_summary( 'One line.' ) ); + $this->assertSame( '', Commit::get_summary( '' ) ); + } + + /** + * Every -newtickets channel gets the same format as #core-newtickets: the full ticket. + * Only #core gets the title alone. + */ + public function test_newtickets_channels_match_core_newtickets_format(): void { + $core = Trac::get( 'core' ); + $map = ( new ReflectionProperty( $core, 'component_channels' ) )->getValue( $core ); + + $this->assertSame( 'title', $core->get_ticket_format( '#core' ) ); + $this->assertSame( 'description', $core->get_ticket_format( '#core-newtickets' ) ); + + foreach ( array_unique( array_filter( $map ) ) as $channel ) { + $this->assertSame( + $core->get_ticket_format( '#core-newtickets' ), + $core->get_ticket_format( $channel . '-newtickets' ), + "$channel-newtickets should match #core-newtickets." + ); + } + } + + /** + * Returns an array's keys, sorted. + * + * @param array $array Array. + * @return string[] + */ + private function sorted_keys( array $array ): array { + $keys = array_keys( $array ); + sort( $keys ); + return $keys; + } +}