| Index: trunk/extensions/FlaggedRevs/FlaggedRevs.php |
| — | — | @@ -507,7 +507,7 @@ |
| 508 | 508 | |
| 509 | 509 | # ######## Other ######### |
| 510 | 510 | # Determine what pages can be moved and patrolled |
| 511 | | -$wgHooks['userCan'][] = 'FlaggedRevsHooks::onUserCan'; |
| | 511 | +$wgHooks['getUserPermissionsErrors'][] = 'FlaggedRevsHooks::onUserCan'; |
| 512 | 512 | # Implicit autoreview rights group |
| 513 | 513 | $wgHooks['GetAutoPromoteGroups'][] = 'FlaggedRevsHooks::checkAutoPromote'; |
| 514 | 514 | |
| — | — | @@ -542,7 +542,7 @@ |
| 543 | 543 | # Visibility - experimental |
| 544 | 544 | if ( !empty( $wgFlaggedRevsVisible ) ) { |
| 545 | 545 | global $wgHooks; |
| 546 | | - $wgHooks['userCan'][] = 'FlaggedRevsHooks::userCanView'; |
| | 546 | + $wgHooks['getUserPermissionsErrors'][] = 'FlaggedRevsHooks::userCanView'; |
| 547 | 547 | } |
| 548 | 548 | # Don't show autoreview group everywhere |
| 549 | 549 | global $wgImplicitGroups; |
| Index: trunk/extensions/FlaggedRevs/FlaggedRevs.hooks.php |
| — | — | @@ -734,8 +734,9 @@ |
| 735 | 735 | * Check page move and patrol permissions for FlaggedRevs |
| 736 | 736 | */ |
| 737 | 737 | public static function onUserCan( $title, $user, $action, &$result ) { |
| 738 | | - if ( $result === false ) |
| | 738 | + if ( $result === false ) { |
| 739 | 739 | return true; // nothing to do |
| | 740 | + } |
| 740 | 741 | # Don't let users vandalize pages by moving them... |
| 741 | 742 | if ( $action === 'move' ) { |
| 742 | 743 | if ( !FlaggedRevs::isPageReviewable( $title ) || !$title->exists() ) |
| — | — | @@ -775,6 +776,18 @@ |
| 776 | 777 | return false; |
| 777 | 778 | } |
| 778 | 779 | } |
| | 780 | + # Enforce autoreview restrictions |
| | 781 | + } else if( $action === 'autoreview' ) { |
| | 782 | + # Get autoreview restriction settings... |
| | 783 | + $config = FlaggedRevs::getPageVisibilitySettings( $title, true ); |
| | 784 | + # Convert Sysop -> protect |
| | 785 | + $right = ( $config['autoreview'] === 'sysop' ) ? |
| | 786 | + 'protect' : $config['autoreview']; |
| | 787 | + # Check if the user has the required right, if any |
| | 788 | + if( $right != '' && !$user->isAllowed( $right ) ) { |
| | 789 | + $result = false; |
| | 790 | + return false; |
| | 791 | + } |
| 779 | 792 | } |
| 780 | 793 | return true; |
| 781 | 794 | } |
| — | — | @@ -831,130 +844,121 @@ |
| 832 | 845 | if ( !$rev || !$fa->isReviewable() ) { |
| 833 | 846 | return true; |
| 834 | 847 | } |
| | 848 | + if ( !$user ) { |
| | 849 | + $user = User::newFromId( $rev->getUser() ); |
| | 850 | + } |
| 835 | 851 | $title = $article->getTitle(); |
| 836 | 852 | $title->resetArticleID( $rev->getPage() ); // Avoid extra DB hit and lag issues |
| 837 | 853 | # Get what was just the current revision ID |
| 838 | 854 | $prevRevId = $rev->getParentId(); |
| 839 | | - $prevTimestamp = $flags = null; |
| 840 | | - # Get edit timestamp. Existance already valided by EditPage.php. If |
| 841 | | - # not present, then it the rev shouldn't be saved, like null edits. |
| | 855 | + $prevTimestamp = $frev = $flags = null; |
| | 856 | + # Get edit timestamp. Existance already validated by EditPage.php. |
| 842 | 857 | $editTimestamp = $wgRequest->getVal( 'wpEdittime' ); |
| 843 | | - # Get the user who made the edit |
| 844 | | - $user = is_null( $user ) ? User::newFromId( $rev->getUser() ) : $user; |
| 845 | | - # Is the page checked off to be reviewed? |
| 846 | | - # Autoreview if this is such a valid case... |
| | 858 | + # Is the page manually checked off to be reviewed? |
| 847 | 859 | if ( $wgRequest->getCheck( 'wpReviewEdit' ) && $user->isAllowed( 'review' ) ) { |
| 848 | 860 | # Check wpEdittime against the previous edit for verification |
| 849 | 861 | if ( $prevRevId ) { |
| 850 | | - $prevTimestamp = Revision::getTimestampFromId( $title, $prevRevId ); // use PK |
| | 862 | + $prevTimestamp = Revision::getTimestampFromId( $title, $prevRevId ); |
| 851 | 863 | } |
| 852 | | - # Review this revision of the page. Let articlesavecomplete hook do rc_patrolled bit. |
| 853 | | - # Don't do so if an edit was auto-merged in between though... |
| | 864 | + # Review this revision of the page unless edit was auto-merged in between... |
| 854 | 865 | if ( !$editTimestamp || !$prevTimestamp || $prevTimestamp == $editTimestamp ) { |
| 855 | | - $ok = FlaggedRevs::autoReviewEdit( $article, $user, $rev->getText(), $rev, |
| 856 | | - $flags, false ); |
| | 866 | + # Note: articlesavecomplete hook does rc_patrolled bit |
| | 867 | + $ok = FlaggedRevs::autoReviewEdit( |
| | 868 | + $article, $user, $rev->getText(), $rev, $flags, false ); |
| 857 | 869 | if ( $ok ) return true; // done! |
| 858 | 870 | } |
| 859 | 871 | } |
| 860 | | - # Get sync cache key |
| 861 | | - $key = wfMemcKey( 'flaggedrevs', 'includesSynced', $rev->getPage() ); |
| 862 | | - # Auto-reviewing must be enabled and user must have the required permissions |
| 863 | | - if ( !FlaggedRevs::autoReviewEdits() || !$user->isAllowed( 'autoreview' ) ) { |
| 864 | | - $isAllowed = false; // untrusted user |
| 865 | | - } else { |
| 866 | | - # Get autoreview restriction settings... |
| 867 | | - $config = FlaggedRevs::getPageVisibilitySettings( $title, true ); |
| 868 | | - # Convert Sysop -> protect |
| 869 | | - $right = ( $config['autoreview'] === 'sysop' ) ? |
| 870 | | - 'protect' : $config['autoreview']; |
| 871 | | - # Check if the user has the required right, if any |
| 872 | | - $isAllowed = ( $right == '' || $user->isAllowed( $right ) ); |
| | 872 | + # All cases below require auto-review of edits to be enabled |
| | 873 | + if( !FlaggedRevs::autoReviewEdits() ) { |
| | 874 | + return true; |
| 873 | 875 | } |
| 874 | | - # If $baseRevId passed in, this is a null edit |
| 875 | | - $isNullEdit = $baseRevId ? true : false; |
| 876 | | - $frev = null; |
| 877 | | - $reviewableNewPage = false; |
| | 876 | + # If a $baseRevId is passed in this is a null edit |
| | 877 | + $isNullEdit = (bool)$baseRevId; |
| 878 | 878 | # Get the revision ID the incoming one was based off... |
| 879 | 879 | if ( !$baseRevId && $prevRevId ) { |
| 880 | 880 | if ( is_null( $prevTimestamp ) ) { // may already be set |
| 881 | | - $prevTimestamp = Revision::getTimestampFromId( $title, $prevRevId ); // use PK |
| | 881 | + $prevTimestamp = Revision::getTimestampFromId( $title, $prevRevId ); |
| 882 | 882 | } |
| 883 | 883 | # The user just made an edit. The one before that should have |
| 884 | 884 | # been the current version. If not reflected in wpEdittime, an |
| 885 | 885 | # edit may have been auto-merged in between, in that case, discard |
| 886 | | - # the baseRevId given from the client... |
| | 886 | + # the baseRevId given from the client. |
| 887 | 887 | if ( !$editTimestamp || $prevTimestamp == $editTimestamp ) { |
| 888 | 888 | $baseRevId = intval( trim( $wgRequest->getVal( 'baseRevId' ) ) ); |
| 889 | 889 | } |
| 890 | | - # If baseRevId not given, assume the previous revision ID. |
| | 890 | + # If baseRevId not given, assume the previous revision ID (for bots). |
| 891 | 891 | # For auto-merges, this also occurs since the given ID is ignored. |
| 892 | | - # Also for bots that don't submit everything... |
| 893 | 892 | if ( !$baseRevId ) { |
| 894 | 893 | $baseRevId = $prevRevId; |
| 895 | 894 | } |
| 896 | 895 | } |
| 897 | | - global $wgMemc, $wgParserCacheExpireTime; |
| 898 | | - # User must have the required permissions for all autoreview cases |
| 899 | | - # except for simple self-reversions. |
| | 896 | + # Self-reversions to the stable version by anyone can be auto-reviewed... |
| | 897 | + $srev = FlaggedRevision::newFromStable( $title, FR_MASTER ); |
| | 898 | + if ( $srev && self::isSelfRevertToStable( $rev, $srev, $baseRevId, $user ) ) { |
| | 899 | + $flags = $srev->getTags(); // use old tags |
| | 900 | + FlaggedRevs::autoReviewEdit( $article, $user, $rev->getText(), $rev, $flags ); |
| | 901 | + return true; // done! |
| | 902 | + } |
| | 903 | + # Can this user auto-review this page? |
| | 904 | + $isAllowed = $title->getUserPermissionsErrors( 'autoreview', $user ) === array(); |
| 900 | 905 | if ( !$isAllowed ) { |
| 901 | | - $srev = FlaggedRevision::newFromStable( $title, FR_MASTER ); |
| 902 | | - # Check if this reverted to the stable version. Reverts to other reviewed |
| 903 | | - # revisions will not be allowed since we don't trust this user. |
| 904 | | - if ( $srev && $baseRevId == $srev->getRevId() ) { |
| 905 | | - # Check that this user is ONLY reverting his/herself. |
| 906 | | - if ( self::userWasLastAuthor( $article, $baseRevId, $user ) ) { |
| 907 | | - # Confirm the text; we can't trust this user. |
| 908 | | - if ( $rev->getText() == $srev->getRevText() ) { |
| 909 | | - $flags = FlaggedRevs::quickTags( FR_SIGHTED ); |
| 910 | | - $ok = FlaggedRevs::autoReviewEdit( $article, $user, $rev->getText(), |
| 911 | | - $rev, $flags ); |
| 912 | | - if ( $ok ) return true; // done! |
| 913 | | - } |
| 914 | | - } |
| 915 | | - } |
| 916 | | - # User does not have the permission for general autoreviewing... |
| 917 | | - $wgMemc->set( $key, FlaggedRevs::makeMemcObj( 'false' ), $wgParserCacheExpireTime ); |
| 918 | | - return true; // done! edit pending! |
| | 906 | + return true; // user does not have auto-review rights |
| 919 | 907 | } |
| | 908 | + $reviewableNewPage = false; |
| 920 | 909 | // New pages |
| 921 | 910 | if ( !$prevRevId ) { |
| 922 | 911 | $reviewableNewPage = FlaggedRevs::autoReviewNewPages(); |
| 923 | 912 | // Edits to existing pages |
| 924 | 913 | } elseif ( $baseRevId ) { |
| 925 | | - $frev = FlaggedRevision::newFromTitle( $title, $baseRevId, FR_MASTER ); |
| 926 | | - # If the base revision was not reviewed, check if the previous one was. |
| 927 | | - # This should catch null edits as well as normal ones. |
| 928 | | - if ( !$frev ) { |
| 929 | | - $frev = FlaggedRevision::newFromTitle( $title, $prevRevId, FR_MASTER ); |
| 930 | | - } |
| | 914 | + # Check if the base revision was reviewed... |
| | 915 | + $frev = ( $srev && $srev->getRevId() == $baseRevId ) |
| | 916 | + ? $srev // save ourselves a query |
| | 917 | + : FlaggedRevision::newFromTitle( $title, $baseRevId, FR_MASTER ); |
| 931 | 918 | } |
| 932 | 919 | // Is this an edit directly to the stable version? Is it a new page? |
| 933 | 920 | if ( $isAllowed && ( $reviewableNewPage || !is_null( $frev ) ) ) { |
| 934 | | - # Assume basic flagging level unless this is a null edit |
| 935 | | - if ( $isNullEdit ) $flags = $frev->getTags(); |
| | 921 | + if ( $isNullEdit && $frev ) { |
| | 922 | + $flags = $frev->getTags(); // Null edits always keep previous tags |
| | 923 | + } |
| 936 | 924 | # Review this revision of the page. Let articlesavecomplete hook do rc_patrolled bit... |
| 937 | | - $ok = FlaggedRevs::autoReviewEdit( $article, $user, $rev->getText(), $rev, $flags ); |
| 938 | | - } else { |
| 939 | | - $ok = false; |
| | 925 | + FlaggedRevs::autoReviewEdit( $article, $user, $rev->getText(), $rev, $flags ); |
| 940 | 926 | } |
| 941 | | - if ( !$ok ) { # Done! edit pending! |
| 942 | | - $wgMemc->set( $key, FlaggedRevs::makeMemcObj( 'false' ), $wgParserCacheExpireTime ); |
| 943 | | - } |
| 944 | 927 | return true; |
| 945 | 928 | } |
| 946 | 929 | |
| 947 | 930 | /** |
| 948 | | - * Check if a user was the last author of the revisions of a page |
| | 931 | + * Check if a user reverted himself to the stable version |
| 949 | 932 | */ |
| 950 | | - protected static function userWasLastAuthor( $article, $baseRevId, $user ) { |
| | 933 | + protected static function isSelfRevertToStable( $rev, $srev, $baseRevId, $user ) { |
| | 934 | + if( !$srev || $baseRevId != $srev->getRevId() ) { |
| | 935 | + return false; // user reports they are not the same |
| | 936 | + } |
| 951 | 937 | $dbw = wfGetDB( DB_MASTER ); |
| 952 | | - return !$dbw->selectField( 'revision', '1', |
| | 938 | + # Such a revert requires 1+ revs between it and the stable |
| | 939 | + $revertedRevs = $dbw->selectField( 'revision', '1', |
| 953 | 940 | array( |
| 954 | | - 'rev_page' => $article->getId(), |
| | 941 | + 'rev_page' => $rev->getPage(), |
| | 942 | + 'rev_id > ' . intval( $baseRevId ), // stable rev |
| | 943 | + 'rev_id < ' . intval( $rev->getId() ), // this rev |
| | 944 | + 'rev_user_text' => $user->getName() |
| | 945 | + ), __METHOD__ |
| | 946 | + ); |
| | 947 | + if( !$revertedRevs ) { |
| | 948 | + return false; // can't be a revert |
| | 949 | + } |
| | 950 | + # Check that this user is ONLY reverting his/herself. |
| | 951 | + $otherUsers = $dbw->selectField( 'revision', '1', |
| | 952 | + array( |
| | 953 | + 'rev_page' => $rev->getPage(), |
| 955 | 954 | 'rev_id > ' . intval( $baseRevId ), |
| 956 | 955 | 'rev_user_text != ' . $dbw->addQuotes( $user->getName() ) |
| 957 | 956 | ), __METHOD__ |
| 958 | 957 | ); |
| | 958 | + if( $otherUsers ) { |
| | 959 | + return false; // only looking for self-reverts |
| | 960 | + } |
| | 961 | + # Confirm the text because we can't trust this user. |
| | 962 | + return ( $rev->getText() == $srev->getRevText() ); |
| 959 | 963 | } |
| 960 | 964 | |
| 961 | 965 | /** |