Changeset 9898

Timestamp:

05/22/2020 05:12:41 AM (5 years ago)

Author:

dd32

Message:

Trac: Attempt to include PR Review status in the PR status on trac.

Fixes #5052.

Location:

sites/trunk

Files:

• api.wordpress.org/public_html/dotorg/trac/pr/functions.php (modified) (2 diffs)
• wordpress.org/public_html/style/trac/wp-trac.js (modified) (2 diffs)

sites/trunk/api.wordpress.org/public_html/dotorg/trac/pr/functions.php

@@ -44 +44 @@
     }
 
+    $_reviews = api_request(
+        '/repos/' . $repo . '/pulls/' . intval( $pr ) . '/reviews',
+        null,
+        [ 'Accept: application/vnd.github.antiope-preview+json' ]
+    );
+    $reviews = [];
+    foreach ( $_reviews as $r ) {
+        if (
+            in_array( $r->state, [ 'CHANGES_REQUESTED', 'APPROVED' ] ) &&
+            ! in_array( $r->user->login, $reviews[ $r->state ] ?? [], true )
+        ) {
+            $reviews[ $r->state ][] = $r->user->login;
+        }
+    }
+
     return (object) [
         'repo'            => $data->base->repo->full_name,
@@ -55 +70 @@
         'mergeable_state' => $data->mergeable_state,
         'check_runs'      => $check_runs,
+        'reviews'         => $reviews,
         'body'            => $data->body,
         'user'            => (object) [

sites/trunk/wordpress.org/public_html/style/trac/wp-trac.js

@@ -1698 +1698 @@
             // Logic to determine what the PRs status is
             function prStatus( data ) {
-                // Closed?
+                var stack = [],
+                    emojiState = '';
+
+                // Closed? Skip everything else.
                 if ( data.closed_at ) {
                     return '✅ Closed';
+                }
+
+                // Merge State then
+                switch ( data.mergeable_state ) {
+                    case 'draft':
+                        stack.push( 'Draft' );
+                        break;
+                    case 'blocked':
+                        // All Good, Changes Requested, or unit tests failing.
+                        // Why 'blocked' is returned for some PRs is not obvious.
+                        if (
+                            data.reviews.CHANGES_REQUESTED ||
+                            (
+                                data.check_runs &&
+                                'failed' == Object.values( data.check_runs ).reduce( function( result, element ) {
+                                    return 'failed' == element ? element : result;
+                                } )
+                            )
+                        ) {
+                            // Let the unit tests / reviews section take care of it.
+                            break;
+                        } // else fall through.
+                    case 'clean':
+                        emojiState = '✅';
+                        stack.push( 'All checks pass' );
+                        break;
+                    case 'dirty':
+                        emojiState = '❌';
+                        stack.push( 'Merge conflicts' );
+                        break;
+                    case 'unstable':
+                        emojiState = '❌';
+                        stack.push( 'Failing tests' );
+                        break;
+                    case 'unknown':
+                        stack.push( 'Unknown' );
+                        break;
                 }
 
@@ -1706 +1746 @@
                 if ( data.check_runs ) {
                     for ( var provider in data.check_runs ) {
-                        switch( data.check_runs[ provider ] ) {
+                        switch ( data.check_runs[ provider ] ) {
                             case 'in_progress':
-                                return provider + ' running';
+                                stack.push( provider + ' running' );
+                                break;
                             case 'failed':
-                                return '❌ ' + provider + ' failed';
+                                emojiState = '❌';
+                                stack.push( provider );
+                                break;
                             case 'success':
                                 continue;
                             default:
-                                return '❌ ' + provider + ' ' + data.check_runs[ provider ];
-                        }
-                    }
-                }
-
-                // Merge State then
-                switch ( data.mergeable_state ) {
-                    case 'draft':
-                        return 'Draft';
-                    case 'blocked': // This seems to be returned for our App with PRs but not others..
-                    case 'clean':
-                        return '✅ All checks pass';
-                    case 'dirty':
-                        return '❌ Merge conflicts';
-                    case 'unstable': // Not seen, Unit Tests above should catch it.
-                        return '❌ Failing tests';
-                    case 'unknown':
-                        return 'Unknown';
-                }
+                                stack.push( provider + ' ' + data.check_runs[ provider ] );
+                                break;
+                        }
+                    }
+                }
+
+                // Changes requested?
+                if ( data.reviews ) {
+                    if ( data.reviews.APPROVED ) {
+                        emojiState = '✅';
+                        stack.push(
+                            $('<span>Approved</span>').prop(
+                                'title',
+                                'Changes approved by: ' + data.reviews.APPROVED.join(', ')
+                            )[0].outerHTML
+                        );
+                    }
+                    if ( data.reviews.CHANGES_REQUESTED ) {
+                        emojiState = '❌';
+                        stack.push(
+                            $('<span>Changes Requested</span>').prop(
+                                'title',
+                                'Changes requested by: ' + data.reviews.CHANGES_REQUESTED.join(', ')
+                            )[0].outerHTML
+                        );
+                    }
+                }
+
+                return emojiState + ' ' + stack.join( ', ' );
             }