From 96bd0c51a9f3df26f95f745b5704cf866974e7ff Mon Sep 17 00:00:00 2001 From: AmbatI_Teja_Sri_Surya Date: Thu, 23 Oct 2025 17:17:38 -0500 Subject: [PATCH 01/26] Implement revert detection for already-reviewed edits - Add revert detection check to autoreview system - Implement @zache-fi's Superset approach for finding reviewed revisions - Add change_tag_params to Superset query for revert detection - Add comprehensive tests for revert detection functionality - Add ENABLE_REVERT_DETECTION configuration setting - Parse change tag parameters to extract reverted revision IDs - Query MediaWiki database for previously reviewed content by SHA1 Fixes #3 - Add check for already-reviewed reverted edits --- app/reviewer/settings.py | 4 + app/reviews/autoreview.py | 217 ++++++++++++++- .../autoreview/checks/revert_detection.py | 189 ++++++++++++++ app/reviews/services.py | 2 + app/reviews/tests/test_revert_detection.py | 247 ++++++++++++++++++ 5 files changed, 658 insertions(+), 1 deletion(-) create mode 100644 app/reviews/autoreview/checks/revert_detection.py create mode 100644 app/reviews/tests/test_revert_detection.py diff --git a/app/reviewer/settings.py b/app/reviewer/settings.py index 3b41250c..feb68937 100644 --- a/app/reviewer/settings.py +++ b/app/reviewer/settings.py @@ -125,6 +125,10 @@ PYWIKIBOT_SITE_FAMILY = os.getenv("PYWIKIBOT_SITE_FAMILY", "wikipedia") +# Revert detection configuration +# Enable/disable revert detection for already-reviewed edits +ENABLE_REVERT_DETECTION = os.getenv("ENABLE_REVERT_DETECTION", "True").lower() in ("true", "1", "yes") + # Default primary key field type # https://docs.djangoproject.com/en/4.2/ref/settings/#default-auto-field diff --git a/app/reviews/autoreview.py b/app/reviews/autoreview.py index 188f68ea..8ef29ea1 100644 --- a/app/reviews/autoreview.py +++ b/app/reviews/autoreview.py @@ -112,7 +112,45 @@ def _evaluate_revision( } ) - # Test 2: Bot editors can always be auto-approved. + # Test 2: Check for revert detection to previously reviewed content + revert_result = _check_revert_detection(revision, client) + if revert_result["status"] == "approve": + tests.append( + { + "id": "revert-detection", + "title": "Revert detection check", + "status": "ok", + "message": revert_result["message"], + } + ) + return { + "tests": tests, + "decision": AutoreviewDecision( + status="approve", + label="Would be auto-approved", + reason=revert_result["message"], + ), + } + elif revert_result["status"] == "block": + tests.append( + { + "id": "revert-detection", + "title": "Revert detection check", + "status": "fail", + "message": revert_result["message"], + } + ) + else: + tests.append( + { + "id": "revert-detection", + "title": "Revert detection check", + "status": "skip", + "message": revert_result["message"], + } + ) + + # Test 3: Bot editors can always be auto-approved. if _is_bot_user(revision, profile): tests.append( { @@ -696,3 +734,180 @@ def _find_invalid_isbns(text: str) -> list[str]: invalid_isbns.append(isbn_raw.strip()) return invalid_isbns + + +def _check_revert_detection(revision: PendingRevision, client: WikiClient) -> dict: + """ + Check if a revision is a revert to previously reviewed content. + + This implements the revert detection logic as described in issue #3. + + Args: + revision: PendingRevision object + client: WikiClient instance + + Returns: + Dict with status, message, and metadata + """ + from django.conf import settings + + # Check if revert detection is enabled + if not getattr(settings, 'ENABLE_REVERT_DETECTION', True): + return { + "status": "skip", + "message": "Revert detection is disabled", + "metadata": {} + } + + # Check for revert tags + revert_tags = {"mw-manual-revert", "mw-reverted", "mw-rollback", "mw-undo"} + change_tags = getattr(revision, 'change_tags', []) + + if not any(tag in change_tags for tag in revert_tags): + return { + "status": "skip", + "message": "No revert tags found", + "metadata": {"change_tags": change_tags} + } + + # Parse change tag parameters to get reverted revision IDs + reverted_rev_ids = _parse_revert_params(revision) + if not reverted_rev_ids: + return { + "status": "skip", + "message": "No reverted revision IDs found in change tags", + "metadata": {"change_tags": change_tags} + } + + # Check if any of the reverted revisions were previously reviewed + reviewed_revisions = _find_reviewed_revisions_by_sha1( + client, revision.page, reverted_rev_ids + ) + + if reviewed_revisions: + return { + "status": "approve", + "message": f"Revert to previously reviewed content (SHA1: {reviewed_revisions[0]['sha1']})", + "metadata": { + "reverted_rev_ids": reverted_rev_ids, + "reviewed_revisions": reviewed_revisions, + "revert_tags": [tag for tag in change_tags if tag in revert_tags] + } + } + + return { + "status": "block", + "message": "Revert detected but no previously reviewed content found", + "metadata": { + "reverted_rev_ids": reverted_rev_ids, + "revert_tags": [tag for tag in change_tags if tag in revert_tags] + } + } + + +def _parse_revert_params(revision) -> list[int]: + """ + Parse change tag parameters to extract reverted revision IDs. + + Args: + revision: PendingRevision object + + Returns: + List of reverted revision IDs + """ + import json + + try: + # Get change tag parameters from revision + change_tag_params = getattr(revision, 'change_tag_params', []) + if not change_tag_params: + return [] + + reverted_ids = [] + + for param_str in change_tag_params: + try: + # Parse JSON parameter + param_data = json.loads(param_str) + + # Extract reverted revision IDs + if 'oldestRevertedRevId' in param_data: + reverted_ids.append(param_data['oldestRevertedRevId']) + if 'newestRevertedRevId' in param_data: + reverted_ids.append(param_data['newestRevertedRevId']) + if 'originalRevisionId' in param_data: + reverted_ids.append(param_data['originalRevisionId']) + + except (json.JSONDecodeError, KeyError) as e: + logger.warning(f"Failed to parse change tag param: {param_str}, error: {e}") + continue + + return list(set(reverted_ids)) # Remove duplicates + + except Exception as e: + logger.error(f"Error parsing revert params for revision {revision.revid}: {e}") + return [] + + +def _find_reviewed_revisions_by_sha1(client, page, reverted_rev_ids: list[int]) -> list[dict]: + """ + Find previously reviewed revisions by SHA1 content hash. + + This implements @zache-fi's suggested Superset approach: + 1. Query MediaWiki database for older reviewed versions by SHA1 + 2. Check if any of the reverted revisions were previously reviewed + + Args: + client: WikiClient instance + page: PendingPage object + reverted_rev_ids: List of reverted revision IDs + + Returns: + List of reviewed revision data + """ + if not reverted_rev_ids: + return [] + + try: + # Execute Superset query to find reviewed revisions by SHA1 + # This follows @zache-fi's suggested SQL approach + revid_list = ','.join(str(revid) for revid in reverted_rev_ids) + + sql_query = f""" + SELECT + MAX(rev_id) as max_reviewable_rev_id_by_sha1, + rev_page, + content_sha1, + MAX(fr_rev_id) as max_old_reviewed_id + FROM + revision + LEFT JOIN flaggedrevs ON rev_id=fr_rev_id + JOIN slots ON slot_revision_id=rev_id + JOIN content ON slot_content_id=content_id + WHERE + rev_id IN ({revid_list}) + GROUP BY + rev_page, content_sha1 + """ + + # Execute query using SupersetQuery + from pywikibot.data.superset import SupersetQuery + superset = SupersetQuery(site=client.site) + results = superset.query(sql_query) + + # Filter results where content was previously reviewed + reviewed_revisions = [] + for result in results: + if result.get('max_old_reviewed_id') is not None: + reviewed_revisions.append({ + 'sha1': result.get('content_sha1'), + 'max_reviewed_id': result.get('max_old_reviewed_id'), + 'max_reviewable_id': result.get('max_reviewable_rev_id_by_sha1'), + 'page_id': result.get('rev_page') + }) + + return reviewed_revisions + + except Exception as e: + logger.error(f"Error finding reviewed revisions for page {page.pageid}: {e}") + return [] diff --git a/app/reviews/autoreview/checks/revert_detection.py b/app/reviews/autoreview/checks/revert_detection.py new file mode 100644 index 00000000..864dc8e1 --- /dev/null +++ b/app/reviews/autoreview/checks/revert_detection.py @@ -0,0 +1,189 @@ +""" +Revert detection check for already-reviewed edits. + +This check detects when a pending edit is a revert to previously reviewed content +by matching SHA1 content hashes and checking for revert tags. +""" + +import json +import logging +from typing import Any, Dict, List, Optional + +from django.conf import settings + +from ..utils.ores import CheckContext + +logger = logging.getLogger(__name__) + + +def check_revert_detection(context: CheckContext) -> Dict[str, Any]: + """ + Check if a revision is a revert to previously reviewed content. + + Args: + context: CheckContext containing revision and related data + + Returns: + Dict with check result including status, message, and metadata + """ + # Check if revert detection is enabled + if not getattr(settings, 'ENABLE_REVERT_DETECTION', True): + return { + "status": "skip", + "message": "Revert detection is disabled", + "metadata": {} + } + + revision = context.revision + page = revision.page + + # Check for revert tags + revert_tags = {"mw-manual-revert", "mw-reverted", "mw-rollback", "mw-undo"} + change_tags = getattr(revision, 'change_tags', []) + + if not any(tag in change_tags for tag in revert_tags): + return { + "status": "skip", + "message": "No revert tags found", + "metadata": {"change_tags": change_tags} + } + + # Parse change tag parameters to get reverted revision IDs + reverted_rev_ids = _parse_revert_params(revision) + if not reverted_rev_ids: + return { + "status": "skip", + "message": "No reverted revision IDs found in change tags", + "metadata": {"change_tags": change_tags} + } + + # Check if any of the reverted revisions were previously reviewed + reviewed_revisions = _find_reviewed_revisions_by_sha1( + context.client, page, reverted_rev_ids + ) + + if reviewed_revisions: + return { + "status": "approve", + "message": f"Revert to previously reviewed content (SHA1: {reviewed_revisions[0]['sha1']})", + "metadata": { + "reverted_rev_ids": reverted_rev_ids, + "reviewed_revisions": reviewed_revisions, + "revert_tags": [tag for tag in change_tags if tag in revert_tags] + } + } + + return { + "status": "block", + "message": "Revert detected but no previously reviewed content found", + "metadata": { + "reverted_rev_ids": reverted_rev_ids, + "revert_tags": [tag for tag in change_tags if tag in revert_tags] + } + } + + +def _parse_revert_params(revision) -> List[int]: + """ + Parse change tag parameters to extract reverted revision IDs. + + Args: + revision: PendingRevision object + + Returns: + List of reverted revision IDs + """ + try: + # Get change tag parameters from revision + change_tag_params = getattr(revision, 'change_tag_params', []) + if not change_tag_params: + return [] + + reverted_ids = [] + + for param_str in change_tag_params: + try: + # Parse JSON parameter + param_data = json.loads(param_str) + + # Extract reverted revision IDs + if 'oldestRevertedRevId' in param_data: + reverted_ids.append(param_data['oldestRevertedRevId']) + if 'newestRevertedRevId' in param_data: + reverted_ids.append(param_data['newestRevertedRevId']) + if 'originalRevisionId' in param_data: + reverted_ids.append(param_data['originalRevisionId']) + + except (json.JSONDecodeError, KeyError) as e: + logger.warning(f"Failed to parse change tag param: {param_str}, error: {e}") + continue + + return list(set(reverted_ids)) # Remove duplicates + + except Exception as e: + logger.error(f"Error parsing revert params for revision {revision.revid}: {e}") + return [] + + +def _find_reviewed_revisions_by_sha1(client, page, reverted_rev_ids: List[int]) -> List[Dict]: + """ + Find previously reviewed revisions by SHA1 content hash. + + This implements @zache-fi's suggested Superset approach: + 1. Query MediaWiki database for older reviewed versions by SHA1 + 2. Check if any of the reverted revisions were previously reviewed + + Args: + client: WikiClient instance + page: PendingPage object + reverted_rev_ids: List of reverted revision IDs + + Returns: + List of reviewed revision data + """ + if not reverted_rev_ids: + return [] + + try: + # Execute Superset query to find reviewed revisions by SHA1 + # This follows @zache-fi's suggested SQL approach + revid_list = ','.join(str(revid) for revid in reverted_rev_ids) + + sql_query = f""" + SELECT + MAX(rev_id) as max_reviewable_rev_id_by_sha1, + rev_page, + content_sha1, + MAX(fr_rev_id) as max_old_reviewed_id + FROM + revision + LEFT JOIN flaggedrevs ON rev_id=fr_rev_id + JOIN slots ON slot_revision_id=rev_id + JOIN content ON slot_content_id=content_id + WHERE + rev_id IN ({revid_list}) + GROUP BY + rev_page, content_sha1 + """ + + # Execute query using SupersetQuery + from pywikibot.data.superset import SupersetQuery + superset = SupersetQuery(site=client.site) + results = superset.query(sql_query) + + # Filter results where content was previously reviewed + reviewed_revisions = [] + for result in results: + if result.get('max_old_reviewed_id') is not None: + reviewed_revisions.append({ + 'sha1': result.get('content_sha1'), + 'max_reviewed_id': result.get('max_old_reviewed_id'), + 'max_reviewable_id': result.get('max_reviewable_rev_id_by_sha1'), + 'page_id': result.get('rev_page') + }) + + return reviewed_revisions + + except Exception as e: + logger.error(f"Error finding reviewed revisions for page {page.pageid}: {e}") + return [] diff --git a/app/reviews/services.py b/app/reviews/services.py index 3f3746cb..4ac91ea8 100644 --- a/app/reviews/services.py +++ b/app/reviews/services.py @@ -151,6 +151,7 @@ def fetch_pending_pages(self, limit: int = 10000) -> list[PendingPage]: a.actor_name, a.actor_user, group_concat(DISTINCT(ctd_name)) AS change_tags, + group_concat(DISTINCT(ct_params)) AS change_tags_params, group_concat(DISTINCT(ug_group)) AS user_groups, group_concat(DISTINCT(ufg_group)) AS user_former_groups, group_concat(DISTINCT(cl_to)) AS page_categories, @@ -378,6 +379,7 @@ def _prepare_superset_metadata(entry: dict) -> dict: metadata = dict(entry) for key in ( "change_tags", + "change_tags_params", "user_groups", "user_former_groups", "page_categories", diff --git a/app/reviews/tests/test_revert_detection.py b/app/reviews/tests/test_revert_detection.py new file mode 100644 index 00000000..d22b406a --- /dev/null +++ b/app/reviews/tests/test_revert_detection.py @@ -0,0 +1,247 @@ +""" +Tests for revert detection functionality. + +This module tests the revert detection check that identifies when +a pending edit is a revert to previously reviewed content. +""" + +import json +from unittest.mock import Mock, patch + +from django.test import TestCase +from django.conf import settings + +from reviews.models import PendingPage, PendingRevision, Wiki, WikiConfiguration +from reviews.services import WikiClient +from reviews.autoreview import _check_revert_detection, _parse_revert_params, _find_reviewed_revisions_by_sha1 + + +class RevertDetectionTests(TestCase): + """Test cases for revert detection functionality.""" + + def setUp(self): + """Set up test data.""" + self.wiki = Wiki.objects.create( + name="Test Wiki", + code="test", + family="wikipedia", + api_endpoint="https://test.wikipedia.org/w/api.php" + ) + self.config = WikiConfiguration.objects.create(wiki=self.wiki) + + self.page = PendingPage.objects.create( + wiki=self.wiki, + pageid=12345, + title="Test Page", + stable_revid=100, + ) + + self.revision = PendingRevision.objects.create( + page=self.page, + revid=200, + parentid=150, + user_name="TestUser", + user_id=1000, + change_tags=["mw-manual-revert"], + change_tag_params=[ + json.dumps({ + "revertId": 200, + "oldestRevertedRevId": 180, + "newestRevertedRevId": 190, + "originalRevisionId": 175 + }) + ] + ) + + self.client = Mock(spec=WikiClient) + self.client.site = Mock() + + def test_revert_detection_disabled(self): + """Test that revert detection is skipped when disabled.""" + with self.settings(ENABLE_REVERT_DETECTION=False): + result = _check_revert_detection(self.revision, self.client) + + self.assertEqual(result["status"], "skip") + self.assertEqual(result["message"], "Revert detection is disabled") + + def test_no_revert_tags(self): + """Test that revert detection is skipped when no revert tags are present.""" + self.revision.change_tags = ["mw-edit"] + self.revision.save() + + result = _check_revert_detection(self.revision, self.client) + + self.assertEqual(result["status"], "skip") + self.assertEqual(result["message"], "No revert tags found") + + def test_parse_revert_params(self): + """Test parsing of change tag parameters.""" + reverted_ids = _parse_revert_params(self.revision) + + expected_ids = [180, 190, 175] # From change_tag_params + self.assertEqual(set(reverted_ids), set(expected_ids)) + + def test_parse_revert_params_empty(self): + """Test parsing when no change tag parameters are present.""" + self.revision.change_tag_params = [] + self.revision.save() + + reverted_ids = _parse_revert_params(self.revision) + self.assertEqual(reverted_ids, []) + + def test_parse_revert_params_invalid_json(self): + """Test parsing with invalid JSON in change tag parameters.""" + self.revision.change_tag_params = ["invalid json"] + self.revision.save() + + reverted_ids = _parse_revert_params(self.revision) + self.assertEqual(reverted_ids, []) + + @patch('reviews.autoreview.SupersetQuery') + def test_find_reviewed_revisions_by_sha1_success(self, mock_superset): + """Test finding reviewed revisions by SHA1.""" + # Mock SupersetQuery results + mock_superset.return_value.query.return_value = [ + { + 'content_sha1': 'abc123', + 'max_old_reviewed_id': 150, + 'max_reviewable_rev_id_by_sha1': 180, + 'rev_page': 12345 + } + ] + + reverted_ids = [180, 190] + reviewed_revisions = _find_reviewed_revisions_by_sha1( + self.client, self.page, reverted_ids + ) + + self.assertEqual(len(reviewed_revisions), 1) + self.assertEqual(reviewed_revisions[0]['sha1'], 'abc123') + self.assertEqual(reviewed_revisions[0]['max_reviewed_id'], 150) + + @patch('reviews.autoreview.SupersetQuery') + def test_find_reviewed_revisions_by_sha1_no_results(self, mock_superset): + """Test when no reviewed revisions are found.""" + mock_superset.return_value.query.return_value = [] + + reverted_ids = [180, 190] + reviewed_revisions = _find_reviewed_revisions_by_sha1( + self.client, self.page, reverted_ids + ) + + self.assertEqual(reviewed_revisions, []) + + @patch('reviews.autoreview._find_reviewed_revisions_by_sha1') + def test_revert_detection_approve(self, mock_find_reviewed): + """Test revert detection when revert to reviewed content is found.""" + # Mock finding reviewed revisions + mock_find_reviewed.return_value = [ + { + 'sha1': 'abc123', + 'max_reviewed_id': 150, + 'max_reviewable_id': 180, + 'page_id': 12345 + } + ] + + result = _check_revert_detection(self.revision, self.client) + + self.assertEqual(result["status"], "approve") + self.assertIn("Revert to previously reviewed content", result["message"]) + self.assertIn("abc123", result["message"]) + + @patch('reviews.autoreview._find_reviewed_revisions_by_sha1') + def test_revert_detection_block(self, mock_find_reviewed): + """Test revert detection when no reviewed content is found.""" + # Mock no reviewed revisions found + mock_find_reviewed.return_value = [] + + result = _check_revert_detection(self.revision, self.client) + + self.assertEqual(result["status"], "block") + self.assertEqual(result["message"], "Revert detected but no previously reviewed content found") + + def test_revert_detection_no_reverted_ids(self): + """Test revert detection when no reverted revision IDs are found.""" + self.revision.change_tag_params = [] + self.revision.save() + + result = _check_revert_detection(self.revision, self.client) + + self.assertEqual(result["status"], "skip") + self.assertEqual(result["message"], "No reverted revision IDs found in change tags") + + def test_revert_detection_metadata(self): + """Test that revert detection returns proper metadata.""" + with patch('reviews.autoreview._find_reviewed_revisions_by_sha1') as mock_find: + mock_find.return_value = [{'sha1': 'abc123'}] + + result = _check_revert_detection(self.revision, self.client) + + self.assertIn("reverted_rev_ids", result["metadata"]) + self.assertIn("revert_tags", result["metadata"]) + self.assertIn("reviewed_revisions", result["metadata"]) + self.assertEqual(result["metadata"]["revert_tags"], ["mw-manual-revert"]) + + +class RevertDetectionIntegrationTests(TestCase): + """Integration tests for revert detection with real data.""" + + def setUp(self): + """Set up integration test data.""" + self.wiki = Wiki.objects.create( + name="Test Wiki", + code="test", + family="wikipedia", + api_endpoint="https://test.wikipedia.org/w/api.php" + ) + self.config = WikiConfiguration.objects.create(wiki=self.wiki) + + def test_revert_detection_with_real_revision(self): + """Test revert detection with a real revision setup.""" + page = PendingPage.objects.create( + wiki=self.wiki, + pageid=12345, + title="Test Page", + stable_revid=100, + ) + + # Create a revision with revert tags + revision = PendingRevision.objects.create( + page=page, + revid=200, + parentid=150, + user_name="TestUser", + user_id=1000, + change_tags=["mw-manual-revert", "mw-reverted"], + change_tag_params=[ + json.dumps({ + "revertId": 200, + "oldestRevertedRevId": 180, + "newestRevertedRevId": 190, + "originalRevisionId": 175 + }) + ] + ) + + # Mock the client + client = Mock(spec=WikiClient) + client.site = Mock() + + # Test with SupersetQuery mock + with patch('reviews.autoreview.SupersetQuery') as mock_superset: + mock_superset.return_value.query.return_value = [ + { + 'content_sha1': 'test_sha1', + 'max_old_reviewed_id': 150, + 'max_reviewable_rev_id_by_sha1': 180, + 'rev_page': 12345 + } + ] + + result = _check_revert_detection(revision, client) + + self.assertEqual(result["status"], "approve") + self.assertIn("test_sha1", result["message"]) + self.assertEqual(len(result["metadata"]["reverted_rev_ids"]), 3) + self.assertEqual(len(result["metadata"]["revert_tags"]), 2) From 6f8d57006d069df0319cf8e9d89d66635e7de47d Mon Sep 17 00:00:00 2001 From: AmbatI_Teja_Sri_Surya Date: Thu, 23 Oct 2025 19:27:08 -0500 Subject: [PATCH 02/26] Add Utility Function for Approving/Unapproving Pending Changes - Implement approve_revision() utility function with pywikibot API integration - Add PENDING_CHANGES_DRY_RUN setting for safe testing - Create Django management command test_pending_changes_review - Add comprehensive unit tests (12 test cases) - Implement dry-run mode with Merkityt_versiot_-kokeilu/* namespace check - Add proper error handling and logging - Support for approve/unapprove operations with custom values Fixes #106 --- app/reviewer/settings.py | 6 + app/reviews/management/__init__.py | 1 + app/reviews/management/commands/__init__.py | 1 + .../commands/test_pending_changes_review.py | 111 +++++++ app/reviews/tests/test_approval.py | 275 ++++++++++++++++++ app/reviews/utils/__init__.py | 1 + app/reviews/utils/approval.py | 123 ++++++++ 7 files changed, 518 insertions(+) create mode 100644 app/reviews/management/__init__.py create mode 100644 app/reviews/management/commands/__init__.py create mode 100644 app/reviews/management/commands/test_pending_changes_review.py create mode 100644 app/reviews/tests/test_approval.py create mode 100644 app/reviews/utils/__init__.py create mode 100644 app/reviews/utils/approval.py diff --git a/app/reviewer/settings.py b/app/reviewer/settings.py index feb68937..dcd3c343 100644 --- a/app/reviewer/settings.py +++ b/app/reviewer/settings.py @@ -129,6 +129,12 @@ # Enable/disable revert detection for already-reviewed edits ENABLE_REVERT_DETECTION = os.getenv("ENABLE_REVERT_DETECTION", "True").lower() in ("true", "1", "yes") +# Pending changes approval configuration +# Enable/disable dry-run mode for pending changes approval +# When True, only allows approvals on test pages (Merkityt_versiot_-kokeilu/*) +# When False, allows approvals on all pages +PENDING_CHANGES_DRY_RUN = os.getenv("PENDING_CHANGES_DRY_RUN", "True").lower() in ("true", "1", "yes") + # Default primary key field type # https://docs.djangoproject.com/en/4.2/ref/settings/#default-auto-field diff --git a/app/reviews/management/__init__.py b/app/reviews/management/__init__.py new file mode 100644 index 00000000..652875d2 --- /dev/null +++ b/app/reviews/management/__init__.py @@ -0,0 +1 @@ +# Management commands for reviews app diff --git a/app/reviews/management/commands/__init__.py b/app/reviews/management/commands/__init__.py new file mode 100644 index 00000000..2c1c7c15 --- /dev/null +++ b/app/reviews/management/commands/__init__.py @@ -0,0 +1 @@ +# Management commands diff --git a/app/reviews/management/commands/test_pending_changes_review.py b/app/reviews/management/commands/test_pending_changes_review.py new file mode 100644 index 00000000..f238438e --- /dev/null +++ b/app/reviews/management/commands/test_pending_changes_review.py @@ -0,0 +1,111 @@ +""" +Django management command to test pending changes review functionality. + +This command allows testing the approve_revision() utility function +with various parameters and configurations. +""" + +from django.core.management.base import BaseCommand, CommandError +from django.conf import settings +from reviews.utils.approval import approve_revision + + +class Command(BaseCommand): + help = 'Test pending changes review functionality (approve/unapprove revisions)' + + def add_arguments(self, parser): + parser.add_argument( + '--revid', + type=int, + required=True, + help='Revision ID to approve/unapprove' + ) + parser.add_argument( + '--comment', + type=str, + default='Test approval via management command', + help='Comment for the review (default: "Test approval via management command")' + ) + parser.add_argument( + '--unapprove', + action='store_true', + help='Unapprove the revision instead of approving it' + ) + parser.add_argument( + '--value', + type=int, + help='Flag value for the review (optional)' + ) + parser.add_argument( + '--dry-run', + action='store_true', + help='Show what would happen without making actual changes' + ) + + def handle(self, *args, **options): + revid = options['revid'] + comment = options['comment'] + unapprove = options['unapprove'] + value = options['value'] + dry_run = options['dry_run'] + + # Display current configuration + self.stdout.write(f"Current PENDING_CHANGES_DRY_RUN setting: {getattr(settings, 'PENDING_CHANGES_DRY_RUN', True)}") + + if dry_run: + self.stdout.write(self.style.WARNING("DRY-RUN MODE: No actual changes will be made")) + + # Display operation details + operation = "unapprove" if unapprove else "approve" + self.stdout.write(f"Operation: {operation}") + self.stdout.write(f"Revision ID: {revid}") + self.stdout.write(f"Comment: {comment}") + if value is not None: + self.stdout.write(f"Value: {value}") + + try: + # Call the approve_revision function + result = approve_revision( + revid=revid, + comment=comment, + value=value, + unapprove=unapprove + ) + + # Display results + if result['result'] == 'success': + if result.get('dry_run', False): + self.stdout.write( + self.style.SUCCESS(f"✅ {result['message']}") + ) + else: + self.stdout.write( + self.style.SUCCESS(f"✅ {result['message']}") + ) + else: + self.stdout.write( + self.style.ERROR(f"❌ {result['message']}") + ) + + # Display additional information + if 'api_response' in result: + self.stdout.write(f"API Response: {result['api_response']}") + + # Display dry-run information + if result.get('dry_run', False): + self.stdout.write( + self.style.WARNING( + "ℹ️ This was a dry-run operation. " + "Set PENDING_CHANGES_DRY_RUN=False to make actual changes." + ) + ) + + except Exception as e: + self.stdout.write( + self.style.ERROR(f"❌ Error: {str(e)}") + ) + raise CommandError(f"Failed to {operation} revision {revid}: {str(e)}") + + self.stdout.write( + self.style.SUCCESS(f"✅ Command completed successfully") + ) diff --git a/app/reviews/tests/test_approval.py b/app/reviews/tests/test_approval.py new file mode 100644 index 00000000..20732054 --- /dev/null +++ b/app/reviews/tests/test_approval.py @@ -0,0 +1,275 @@ +""" +Unit tests for the approval utility functions. + +Tests the approve_revision() function and related functionality. +""" + +import unittest +from unittest.mock import Mock, patch, MagicMock +from django.test import TestCase, override_settings +from django.conf import settings +from reviews.utils.approval import approve_revision, _get_page_title_from_revid + + +class ApprovalUtilityTests(TestCase): + """Test cases for the approval utility functions.""" + + def setUp(self): + """Set up test fixtures.""" + self.test_revid = 12345 + self.test_comment = "Test approval" + self.test_site = Mock() + self.test_site.code = 'fi' + self.test_site.family.name = 'wikipedia' + + @patch('reviews.utils.approval.Site') + @patch('reviews.utils.approval.Request') + @override_settings(PENDING_CHANGES_DRY_RUN=False) + def test_approve_revision_success(self, mock_request_class, mock_site_class): + """Test successful approval of a revision.""" + # Mock the site + mock_site_class.return_value = self.test_site + + # Mock the API request + mock_request = Mock() + mock_request.submit.return_value = {'review': {'result': 'success'}} + mock_request_class.return_value = mock_request + + # Call the function + result = approve_revision( + revid=self.test_revid, + comment=self.test_comment, + unapprove=False + ) + + # Assertions + self.assertEqual(result['result'], 'success') + self.assertFalse(result['dry_run']) + self.assertIn('Successfully approved', result['message']) + + # Verify API call was made + mock_request_class.assert_called_once() + mock_request.submit.assert_called_once() + + @patch('reviews.utils.approval.Site') + @patch('reviews.utils.approval.Request') + @override_settings(PENDING_CHANGES_DRY_RUN=False) + def test_approve_revision_unapprove(self, mock_request_class, mock_site_class): + """Test successful unapproval of a revision.""" + # Mock the site + mock_site_class.return_value = self.test_site + + # Mock the API request + mock_request = Mock() + mock_request.submit.return_value = {'review': {'result': 'success'}} + mock_request_class.return_value = mock_request + + # Call the function + result = approve_revision( + revid=self.test_revid, + comment=self.test_comment, + unapprove=True + ) + + # Assertions + self.assertEqual(result['result'], 'success') + self.assertFalse(result['dry_run']) + self.assertIn('Successfully unapproved', result['message']) + + @patch('reviews.utils.approval.Site') + @patch('reviews.utils.approval.Request') + @patch('reviews.utils.approval._get_page_title_from_revid') + @override_settings(PENDING_CHANGES_DRY_RUN=True) + def test_approve_revision_dry_run_production_page(self, mock_get_title, mock_request_class, mock_site_class): + """Test dry-run mode with production page (should skip).""" + # Mock the site + mock_site_class.return_value = self.test_site + + # Mock page title (production page) + mock_get_title.return_value = "Production_Page" + + # Call the function + result = approve_revision( + revid=self.test_revid, + comment=self.test_comment, + unapprove=False + ) + + # Assertions + self.assertEqual(result['result'], 'success') + self.assertTrue(result['dry_run']) + self.assertIn('DRY-RUN: Would approve', result['message']) + + # Verify API call was NOT made + mock_request_class.assert_not_called() + + @patch('reviews.utils.approval.Site') + @patch('reviews.utils.approval.Request') + @patch('reviews.utils.approval._get_page_title_from_revid') + @override_settings(PENDING_CHANGES_DRY_RUN=True) + def test_approve_revision_dry_run_test_page(self, mock_get_title, mock_request_class, mock_site_class): + """Test dry-run mode with test page (should proceed).""" + # Mock the site + mock_site_class.return_value = self.test_site + + # Mock page title (test page) + mock_get_title.return_value = "Merkityt_versiot_-kokeilu/Test_Page" + + # Mock the API request + mock_request = Mock() + mock_request.submit.return_value = {'review': {'result': 'success'}} + mock_request_class.return_value = mock_request + + # Call the function + result = approve_revision( + revid=self.test_revid, + comment=self.test_comment, + unapprove=False + ) + + # Assertions + self.assertEqual(result['result'], 'success') + self.assertFalse(result['dry_run']) + self.assertIn('Successfully approved', result['message']) + + # Verify API call was made + mock_request_class.assert_called_once() + + @patch('reviews.utils.approval.Site') + @patch('reviews.utils.approval.Request') + @override_settings(PENDING_CHANGES_DRY_RUN=False) + def test_approve_revision_with_value(self, mock_request_class, mock_site_class): + """Test approval with custom value parameter.""" + # Mock the site + mock_site_class.return_value = self.test_site + + # Mock the API request + mock_request = Mock() + mock_request.submit.return_value = {'review': {'result': 'success'}} + mock_request_class.return_value = mock_request + + # Call the function with value + result = approve_revision( + revid=self.test_revid, + comment=self.test_comment, + value=1, + unapprove=False + ) + + # Assertions + self.assertEqual(result['result'], 'success') + + # Verify API call was made with value parameter + mock_request_class.assert_called_once() + call_args = mock_request_class.call_args + self.assertEqual(call_args[1]['value'], '1') + + @patch('reviews.utils.approval.Site') + @patch('reviews.utils.approval.Request') + @override_settings(PENDING_CHANGES_DRY_RUN=False) + def test_approve_revision_api_error(self, mock_request_class, mock_site_class): + """Test handling of API errors.""" + # Mock the site + mock_site_class.return_value = self.test_site + + # Mock API error response + mock_request = Mock() + mock_request.submit.return_value = {'error': {'code': 'permissiondenied', 'info': 'Permission denied'}} + mock_request_class.return_value = mock_request + + # Call the function + result = approve_revision( + revid=self.test_revid, + comment=self.test_comment, + unapprove=False + ) + + # Assertions + self.assertEqual(result['result'], 'error') + self.assertIn('Failed to approve', result['message']) + + @patch('reviews.utils.approval.Site') + @override_settings(PENDING_CHANGES_DRY_RUN=False) + def test_approve_revision_exception(self, mock_site_class): + """Test handling of exceptions.""" + # Mock the site to raise an exception + mock_site_class.side_effect = Exception("Connection error") + + # Call the function + result = approve_revision( + revid=self.test_revid, + comment=self.test_comment, + unapprove=False + ) + + # Assertions + self.assertEqual(result['result'], 'error') + self.assertIn('Error approving', result['message']) + + @patch('reviews.utils.approval.Request') + def test_get_page_title_from_revid_success(self, mock_request_class): + """Test successful retrieval of page title.""" + # Mock the API request + mock_request = Mock() + mock_request.submit.return_value = { + 'query': { + 'pages': { + '123': { + 'title': 'Test_Page', + 'revisions': [{'revid': self.test_revid}] + } + } + } + } + mock_request_class.return_value = mock_request + + # Call the function + title = _get_page_title_from_revid(self.test_site, self.test_revid) + + # Assertions + self.assertEqual(title, 'Test_Page') + mock_request_class.assert_called_once() + + @patch('reviews.utils.approval.Request') + def test_get_page_title_from_revid_not_found(self, mock_request_class): + """Test handling when page title is not found.""" + # Mock the API request + mock_request = Mock() + mock_request.submit.return_value = {'query': {'pages': {}}} + mock_request_class.return_value = mock_request + + # Call the function + title = _get_page_title_from_revid(self.test_site, self.test_revid) + + # Assertions + self.assertIsNone(title) + + @patch('reviews.utils.approval.Request') + def test_get_page_title_from_revid_exception(self, mock_request_class): + """Test handling of exceptions in _get_page_title_from_revid.""" + # Mock the API request to raise an exception + mock_request = Mock() + mock_request.submit.side_effect = Exception("API error") + mock_request_class.return_value = mock_request + + # Call the function + title = _get_page_title_from_revid(self.test_site, self.test_revid) + + # Assertions + self.assertIsNone(title) + + +class ApprovalIntegrationTests(TestCase): + """Integration tests for the approval functionality.""" + + @override_settings(PENDING_CHANGES_DRY_RUN=True) + def test_dry_run_setting_respected(self): + """Test that the dry-run setting is properly respected.""" + # This test verifies that the setting is read correctly + self.assertTrue(getattr(settings, 'PENDING_CHANGES_DRY_RUN', True)) + + @override_settings(PENDING_CHANGES_DRY_RUN=False) + def test_dry_run_setting_disabled(self): + """Test that the dry-run setting can be disabled.""" + # This test verifies that the setting can be disabled + self.assertFalse(getattr(settings, 'PENDING_CHANGES_DRY_RUN', True)) diff --git a/app/reviews/utils/__init__.py b/app/reviews/utils/__init__.py new file mode 100644 index 00000000..7cff9f71 --- /dev/null +++ b/app/reviews/utils/__init__.py @@ -0,0 +1 @@ +# Utils package for reviews app diff --git a/app/reviews/utils/approval.py b/app/reviews/utils/approval.py new file mode 100644 index 00000000..c48c4704 --- /dev/null +++ b/app/reviews/utils/approval.py @@ -0,0 +1,123 @@ +""" +Utility functions for approving/unapproving pending changes revisions. + +This module provides functions to interact with MediaWiki's FlaggedRevs API +for approving or unapproving pending changes revisions. +""" + +import logging +from django.conf import settings +from pywikibot.data.api import Request +from pywikibot import Site + +logger = logging.getLogger(__name__) + + +def approve_revision(revid, comment, value=None, unapprove=False): + """ + Approve or unapprove a pending changes revision. + + Args: + revid (int): The revision ID for which to set the flags + comment (str): Comment for the review + value (int, optional): Flag value for the review. Defaults to None. + unapprove (bool, optional): If True, revision will be unapproved + rather than approved. Defaults to False. + + Returns: + dict: Result of the review operation + """ + try: + # Get the site (assuming we're working with Finnish Wikipedia) + site = Site('fi', 'wikipedia') + + # Check if we're in dry-run mode + if getattr(settings, 'PENDING_CHANGES_DRY_RUN', True): + # Get page title to check if it's in test namespace + page_title = _get_page_title_from_revid(site, revid) + + if page_title and not page_title.startswith('Merkityt_versiot_-kokeilu/'): + logger.info(f"DRY-RUN: Would {'unapprove' if unapprove else 'approve'} revision {revid} on {page_title}") + return { + 'result': 'success', + 'dry_run': True, + 'message': f"DRY-RUN: Would {'unapprove' if unapprove else 'approve'} revision {revid}" + } + + # Prepare API request parameters + params = { + 'action': 'review', + 'revid': revid, + 'comment': comment, + } + + # Add unapprove parameter if needed + if unapprove: + params['unapprove'] = '1' + + # Add value parameter if provided + if value is not None: + params['value'] = str(value) + + # Make the API request + request = Request(site=site, **params) + result = request.submit() + + # Check if the request was successful + if 'review' in result: + logger.info(f"Successfully {'unapproved' if unapprove else 'approved'} revision {revid}") + return { + 'result': 'success', + 'dry_run': False, + 'message': f"Successfully {'unapproved' if unapprove else 'approved'} revision {revid}", + 'api_response': result['review'] + } + else: + logger.error(f"Failed to {'unapprove' if unapprove else 'approve'} revision {revid}: {result}") + return { + 'result': 'error', + 'dry_run': False, + 'message': f"Failed to {'unapprove' if unapprove else 'approve'} revision {revid}", + 'api_response': result + } + + except Exception as e: + logger.error(f"Error {'unapproving' if unapprove else 'approving'} revision {revid}: {str(e)}") + return { + 'result': 'error', + 'dry_run': False, + 'message': f"Error {'unapproving' if unapprove else 'approving'} revision {revid}: {str(e)}" + } + + +def _get_page_title_from_revid(site, revid): + """ + Get the page title for a given revision ID. + + Args: + site: Pywikibot site object + revid (int): Revision ID + + Returns: + str: Page title or None if not found + """ + try: + request = Request( + site=site, + action='query', + prop='revisions', + revids=revid, + rvprop='title' + ) + result = request.submit() + + if 'query' in result and 'pages' in result['query']: + for page_id, page_data in result['query']['pages'].items(): + if 'revisions' in page_data: + return page_data['title'] + + return None + + except Exception as e: + logger.error(f"Error getting page title for revision {revid}: {str(e)}") + return None From b845264106c235426d21bc34a087bb1dbd77f9ee Mon Sep 17 00:00:00 2001 From: AmbatI_Teja_Sri_Surya Date: Wed, 29 Oct 2025 15:16:30 -0500 Subject: [PATCH 03/26] fix: satisfy CI for PR #107 (E501 wraps, UP035 typing cleanups, suppress S608 with validated ids) --- app/reviewer/settings.py | 10 +++- .../autoreview/checks/revert_detection.py | 47 ++++++++++--------- 2 files changed, 33 insertions(+), 24 deletions(-) diff --git a/app/reviewer/settings.py b/app/reviewer/settings.py index 285abe0a..225a2d05 100644 --- a/app/reviewer/settings.py +++ b/app/reviewer/settings.py @@ -127,13 +127,19 @@ # Revert detection configuration # Enable/disable revert detection for already-reviewed edits -ENABLE_REVERT_DETECTION = os.getenv("ENABLE_REVERT_DETECTION", "True").lower() in ("true", "1", "yes") +ENABLE_REVERT_DETECTION = ( + os.getenv("ENABLE_REVERT_DETECTION", "True").lower() + in ("true", "1", "yes") +) # Pending changes approval configuration # Enable/disable dry-run mode for pending changes approval # When True, only allows approvals on test pages (Merkityt_versiot_-kokeilu/*) # When False, allows approvals on all pages -PENDING_CHANGES_DRY_RUN = os.getenv("PENDING_CHANGES_DRY_RUN", "True").lower() in ("true", "1", "yes") +PENDING_CHANGES_DRY_RUN = ( + os.getenv("PENDING_CHANGES_DRY_RUN", "True").lower() + in ("true", "1", "yes") +) # ORES model thresholds (global defaults, per-wiki config takes precedence) ORES_DAMAGING_THRESHOLD = float(os.getenv("ORES_DAMAGING_THRESHOLD", "0.3")) diff --git a/app/reviews/autoreview/checks/revert_detection.py b/app/reviews/autoreview/checks/revert_detection.py index 864dc8e1..0e254f47 100644 --- a/app/reviews/autoreview/checks/revert_detection.py +++ b/app/reviews/autoreview/checks/revert_detection.py @@ -7,7 +7,7 @@ import json import logging -from typing import Any, Dict, List, Optional +from typing import Any from django.conf import settings @@ -16,7 +16,7 @@ logger = logging.getLogger(__name__) -def check_revert_detection(context: CheckContext) -> Dict[str, Any]: +def check_revert_detection(context: CheckContext) -> dict[str, Any]: """ Check if a revision is a revert to previously reviewed content. @@ -83,7 +83,7 @@ def check_revert_detection(context: CheckContext) -> Dict[str, Any]: } -def _parse_revert_params(revision) -> List[int]: +def _parse_revert_params(revision) -> list[int]: """ Parse change tag parameters to extract reverted revision IDs. @@ -125,7 +125,9 @@ def _parse_revert_params(revision) -> List[int]: return [] -def _find_reviewed_revisions_by_sha1(client, page, reverted_rev_ids: List[int]) -> List[Dict]: +def _find_reviewed_revisions_by_sha1( + client, page, reverted_rev_ids: list[int] +) -> list[dict]: """ Find previously reviewed revisions by SHA1 content hash. @@ -147,24 +149,25 @@ def _find_reviewed_revisions_by_sha1(client, page, reverted_rev_ids: List[int]) try: # Execute Superset query to find reviewed revisions by SHA1 # This follows @zache-fi's suggested SQL approach - revid_list = ','.join(str(revid) for revid in reverted_rev_ids) - - sql_query = f""" - SELECT - MAX(rev_id) as max_reviewable_rev_id_by_sha1, - rev_page, - content_sha1, - MAX(fr_rev_id) as max_old_reviewed_id - FROM - revision - LEFT JOIN flaggedrevs ON rev_id=fr_rev_id - JOIN slots ON slot_revision_id=rev_id - JOIN content ON slot_content_id=content_id - WHERE - rev_id IN ({revid_list}) - GROUP BY - rev_page, content_sha1 - """ + revid_list = ",".join(str(int(revid)) for revid in reverted_rev_ids) + + # ids are validated as integers above; safe to embed + sql_query = ( + "SELECT \n" + " MAX(rev_id) as max_reviewable_rev_id_by_sha1, \n" + " rev_page, \n" + " content_sha1, \n" + " MAX(fr_rev_id) as max_old_reviewed_id \n" + "FROM \n" + " revision \n" + " LEFT JOIN flaggedrevs ON rev_id=fr_rev_id\n" + " JOIN slots ON slot_revision_id=rev_id\n" + " JOIN content ON slot_content_id=content_id\n" + "WHERE \n" + f" rev_id IN ({revid_list})" # noqa: S608 + "\nGROUP BY \n" + " rev_page, content_sha1\n" + ) # Execute query using SupersetQuery from pywikibot.data.superset import SupersetQuery From 60561f5a90c9cd317ac52bc2b3700f5af589e1b2 Mon Sep 17 00:00:00 2001 From: AmbatI_Teja_Sri_Surya Date: Wed, 29 Oct 2025 15:19:49 -0500 Subject: [PATCH 04/26] ci: grant pull-requests: write for label job (fix 'Resource not accessible by integration') --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 46fc6cb2..69b4ce6d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,6 +15,7 @@ on: permissions: contents: read + pull-requests: write jobs: test: From fd64d2685c721252717e21b508904072e8208003 Mon Sep 17 00:00:00 2001 From: AmbatI_Teja_Sri_Surya Date: Wed, 29 Oct 2025 15:22:44 -0500 Subject: [PATCH 05/26] style: fix E501 and logging formatting in approval utility and tests --- app/reviews/tests/test_approval.py | 7 ++++++- app/reviews/utils/approval.py | 24 +++++++++++++++--------- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/app/reviews/tests/test_approval.py b/app/reviews/tests/test_approval.py index 20732054..14ca0ebb 100644 --- a/app/reviews/tests/test_approval.py +++ b/app/reviews/tests/test_approval.py @@ -174,7 +174,12 @@ def test_approve_revision_api_error(self, mock_request_class, mock_site_class): # Mock API error response mock_request = Mock() - mock_request.submit.return_value = {'error': {'code': 'permissiondenied', 'info': 'Permission denied'}} + mock_request.submit.return_value = { + 'error': { + 'code': 'permissiondenied', + 'info': 'Permission denied', + } + } mock_request_class.return_value = mock_request # Call the function diff --git a/app/reviews/utils/approval.py b/app/reviews/utils/approval.py index c48c4704..2c294dfd 100644 --- a/app/reviews/utils/approval.py +++ b/app/reviews/utils/approval.py @@ -37,11 +37,14 @@ def approve_revision(revid, comment, value=None, unapprove=False): page_title = _get_page_title_from_revid(site, revid) if page_title and not page_title.startswith('Merkityt_versiot_-kokeilu/'): - logger.info(f"DRY-RUN: Would {'unapprove' if unapprove else 'approve'} revision {revid} on {page_title}") + action = 'unapprove' if unapprove else 'approve' + logger.info( + "DRY-RUN: Would %s revision %s on %s", action, revid, page_title + ) return { 'result': 'success', 'dry_run': True, - 'message': f"DRY-RUN: Would {'unapprove' if unapprove else 'approve'} revision {revid}" + 'message': f"DRY-RUN: Would {action} revision {revid}" } # Prepare API request parameters @@ -65,28 +68,31 @@ def approve_revision(revid, comment, value=None, unapprove=False): # Check if the request was successful if 'review' in result: - logger.info(f"Successfully {'unapproved' if unapprove else 'approved'} revision {revid}") + action_past = 'unapproved' if unapprove else 'approved' + logger.info("Successfully %s revision %s", action_past, revid) return { 'result': 'success', 'dry_run': False, - 'message': f"Successfully {'unapproved' if unapprove else 'approved'} revision {revid}", + 'message': f"Successfully {action_past} revision {revid}", 'api_response': result['review'] } else: - logger.error(f"Failed to {'unapprove' if unapprove else 'approve'} revision {revid}: {result}") + action = 'unapprove' if unapprove else 'approve' + logger.error("Failed to %s revision %s: %s", action, revid, result) return { 'result': 'error', 'dry_run': False, - 'message': f"Failed to {'unapprove' if unapprove else 'approve'} revision {revid}", + 'message': f"Failed to {action} revision {revid}", 'api_response': result } except Exception as e: - logger.error(f"Error {'unapproving' if unapprove else 'approving'} revision {revid}: {str(e)}") + action_ing = 'unapproving' if unapprove else 'approving' + logger.error("Error %s revision %s: %s", action_ing, revid, e) return { 'result': 'error', 'dry_run': False, - 'message': f"Error {'unapproving' if unapprove else 'approving'} revision {revid}: {str(e)}" + 'message': f"Error {action_ing} revision {revid}: {e}" } @@ -119,5 +125,5 @@ def _get_page_title_from_revid(site, revid): return None except Exception as e: - logger.error(f"Error getting page title for revision {revid}: {str(e)}") + logger.error("Error getting page title for revision %s: %s", revid, e) return None From 0d686be233ac11e451f1d5bdf983bf53bfe99305 Mon Sep 17 00:00:00 2001 From: AmbatI_Teja_Sri_Surya Date: Wed, 29 Oct 2025 15:33:31 -0500 Subject: [PATCH 06/26] style(ci): fix W293 blanks and skip label job on forks --- .github/workflows/ci.yml | 2 +- app/reviews/autoreview/checks/revert_detection.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 69b4ce6d..e89c869c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -96,7 +96,7 @@ jobs: name: Update PR Labels runs-on: ubuntu-latest needs: test - if: always() && github.event_name == 'pull_request' + if: always() && github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false permissions: pull-requests: write diff --git a/app/reviews/autoreview/checks/revert_detection.py b/app/reviews/autoreview/checks/revert_detection.py index 0e254f47..a9d3daa1 100644 --- a/app/reviews/autoreview/checks/revert_detection.py +++ b/app/reviews/autoreview/checks/revert_detection.py @@ -19,10 +19,10 @@ def check_revert_detection(context: CheckContext) -> dict[str, Any]: """ Check if a revision is a revert to previously reviewed content. - + Args: context: CheckContext containing revision and related data - + Returns: Dict with check result including status, message, and metadata """ From f220c770c60ac40eae01f0905537b9af02d87ffd Mon Sep 17 00:00:00 2001 From: AmbatI_Teja_Sri_Surya Date: Wed, 29 Oct 2025 15:35:46 -0500 Subject: [PATCH 07/26] style: remove trailing whitespace (W293/W291) in revert_detection --- app/reviews/autoreview/checks/revert_detection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/reviews/autoreview/checks/revert_detection.py b/app/reviews/autoreview/checks/revert_detection.py index a9d3daa1..6d22032b 100644 --- a/app/reviews/autoreview/checks/revert_detection.py +++ b/app/reviews/autoreview/checks/revert_detection.py @@ -43,7 +43,7 @@ def check_revert_detection(context: CheckContext) -> dict[str, Any]: if not any(tag in change_tags for tag in revert_tags): return { - "status": "skip", + "status": "skip", "message": "No revert tags found", "metadata": {"change_tags": change_tags} } From a35b519af61c95edbfc318862939d5c88c59f336 Mon Sep 17 00:00:00 2001 From: AmbatI_Teja_Sri_Surya Date: Wed, 29 Oct 2025 15:47:47 -0500 Subject: [PATCH 08/26] style: remove trailing whitespace (fix W293/W291) --- .../autoreview/checks/revert_detection.py | 36 +++++------ .../commands/test_pending_changes_review.py | 16 ++--- app/reviews/tests/test_approval.py | 62 +++++++++---------- app/reviews/tests/test_revert_detection.py | 52 ++++++++-------- app/reviews/utils/approval.py | 22 +++---- 5 files changed, 94 insertions(+), 94 deletions(-) diff --git a/app/reviews/autoreview/checks/revert_detection.py b/app/reviews/autoreview/checks/revert_detection.py index 6d22032b..fe8adad5 100644 --- a/app/reviews/autoreview/checks/revert_detection.py +++ b/app/reviews/autoreview/checks/revert_detection.py @@ -33,21 +33,21 @@ def check_revert_detection(context: CheckContext) -> dict[str, Any]: "message": "Revert detection is disabled", "metadata": {} } - + revision = context.revision page = revision.page - + # Check for revert tags revert_tags = {"mw-manual-revert", "mw-reverted", "mw-rollback", "mw-undo"} change_tags = getattr(revision, 'change_tags', []) - + if not any(tag in change_tags for tag in revert_tags): return { "status": "skip", "message": "No revert tags found", "metadata": {"change_tags": change_tags} } - + # Parse change tag parameters to get reverted revision IDs reverted_rev_ids = _parse_revert_params(revision) if not reverted_rev_ids: @@ -56,12 +56,12 @@ def check_revert_detection(context: CheckContext) -> dict[str, Any]: "message": "No reverted revision IDs found in change tags", "metadata": {"change_tags": change_tags} } - + # Check if any of the reverted revisions were previously reviewed reviewed_revisions = _find_reviewed_revisions_by_sha1( context.client, page, reverted_rev_ids ) - + if reviewed_revisions: return { "status": "approve", @@ -72,7 +72,7 @@ def check_revert_detection(context: CheckContext) -> dict[str, Any]: "revert_tags": [tag for tag in change_tags if tag in revert_tags] } } - + return { "status": "block", "message": "Revert detected but no previously reviewed content found", @@ -98,14 +98,14 @@ def _parse_revert_params(revision) -> list[int]: change_tag_params = getattr(revision, 'change_tag_params', []) if not change_tag_params: return [] - + reverted_ids = [] - + for param_str in change_tag_params: try: # Parse JSON parameter param_data = json.loads(param_str) - + # Extract reverted revision IDs if 'oldestRevertedRevId' in param_data: reverted_ids.append(param_data['oldestRevertedRevId']) @@ -113,13 +113,13 @@ def _parse_revert_params(revision) -> list[int]: reverted_ids.append(param_data['newestRevertedRevId']) if 'originalRevisionId' in param_data: reverted_ids.append(param_data['originalRevisionId']) - + except (json.JSONDecodeError, KeyError) as e: logger.warning(f"Failed to parse change tag param: {param_str}, error: {e}") continue - + return list(set(reverted_ids)) # Remove duplicates - + except Exception as e: logger.error(f"Error parsing revert params for revision {revision.revid}: {e}") return [] @@ -145,7 +145,7 @@ def _find_reviewed_revisions_by_sha1( """ if not reverted_rev_ids: return [] - + try: # Execute Superset query to find reviewed revisions by SHA1 # This follows @zache-fi's suggested SQL approach @@ -168,12 +168,12 @@ def _find_reviewed_revisions_by_sha1( "\nGROUP BY \n" " rev_page, content_sha1\n" ) - + # Execute query using SupersetQuery from pywikibot.data.superset import SupersetQuery superset = SupersetQuery(site=client.site) results = superset.query(sql_query) - + # Filter results where content was previously reviewed reviewed_revisions = [] for result in results: @@ -184,9 +184,9 @@ def _find_reviewed_revisions_by_sha1( 'max_reviewable_id': result.get('max_reviewable_rev_id_by_sha1'), 'page_id': result.get('rev_page') }) - + return reviewed_revisions - + except Exception as e: logger.error(f"Error finding reviewed revisions for page {page.pageid}: {e}") return [] diff --git a/app/reviews/management/commands/test_pending_changes_review.py b/app/reviews/management/commands/test_pending_changes_review.py index f238438e..593a357a 100644 --- a/app/reviews/management/commands/test_pending_changes_review.py +++ b/app/reviews/management/commands/test_pending_changes_review.py @@ -51,10 +51,10 @@ def handle(self, *args, **options): # Display current configuration self.stdout.write(f"Current PENDING_CHANGES_DRY_RUN setting: {getattr(settings, 'PENDING_CHANGES_DRY_RUN', True)}") - + if dry_run: self.stdout.write(self.style.WARNING("DRY-RUN MODE: No actual changes will be made")) - + # Display operation details operation = "unapprove" if unapprove else "approve" self.stdout.write(f"Operation: {operation}") @@ -62,7 +62,7 @@ def handle(self, *args, **options): self.stdout.write(f"Comment: {comment}") if value is not None: self.stdout.write(f"Value: {value}") - + try: # Call the approve_revision function result = approve_revision( @@ -71,7 +71,7 @@ def handle(self, *args, **options): value=value, unapprove=unapprove ) - + # Display results if result['result'] == 'success': if result.get('dry_run', False): @@ -86,11 +86,11 @@ def handle(self, *args, **options): self.stdout.write( self.style.ERROR(f"❌ {result['message']}") ) - + # Display additional information if 'api_response' in result: self.stdout.write(f"API Response: {result['api_response']}") - + # Display dry-run information if result.get('dry_run', False): self.stdout.write( @@ -99,13 +99,13 @@ def handle(self, *args, **options): "Set PENDING_CHANGES_DRY_RUN=False to make actual changes." ) ) - + except Exception as e: self.stdout.write( self.style.ERROR(f"❌ Error: {str(e)}") ) raise CommandError(f"Failed to {operation} revision {revid}: {str(e)}") - + self.stdout.write( self.style.SUCCESS(f"✅ Command completed successfully") ) diff --git a/app/reviews/tests/test_approval.py b/app/reviews/tests/test_approval.py index 14ca0ebb..60726253 100644 --- a/app/reviews/tests/test_approval.py +++ b/app/reviews/tests/test_approval.py @@ -29,24 +29,24 @@ def test_approve_revision_success(self, mock_request_class, mock_site_class): """Test successful approval of a revision.""" # Mock the site mock_site_class.return_value = self.test_site - + # Mock the API request mock_request = Mock() mock_request.submit.return_value = {'review': {'result': 'success'}} mock_request_class.return_value = mock_request - + # Call the function result = approve_revision( revid=self.test_revid, comment=self.test_comment, unapprove=False ) - + # Assertions self.assertEqual(result['result'], 'success') self.assertFalse(result['dry_run']) self.assertIn('Successfully approved', result['message']) - + # Verify API call was made mock_request_class.assert_called_once() mock_request.submit.assert_called_once() @@ -58,19 +58,19 @@ def test_approve_revision_unapprove(self, mock_request_class, mock_site_class): """Test successful unapproval of a revision.""" # Mock the site mock_site_class.return_value = self.test_site - + # Mock the API request mock_request = Mock() mock_request.submit.return_value = {'review': {'result': 'success'}} mock_request_class.return_value = mock_request - + # Call the function result = approve_revision( revid=self.test_revid, comment=self.test_comment, unapprove=True ) - + # Assertions self.assertEqual(result['result'], 'success') self.assertFalse(result['dry_run']) @@ -84,22 +84,22 @@ def test_approve_revision_dry_run_production_page(self, mock_get_title, mock_req """Test dry-run mode with production page (should skip).""" # Mock the site mock_site_class.return_value = self.test_site - + # Mock page title (production page) mock_get_title.return_value = "Production_Page" - + # Call the function result = approve_revision( revid=self.test_revid, comment=self.test_comment, unapprove=False ) - + # Assertions self.assertEqual(result['result'], 'success') self.assertTrue(result['dry_run']) self.assertIn('DRY-RUN: Would approve', result['message']) - + # Verify API call was NOT made mock_request_class.assert_not_called() @@ -111,27 +111,27 @@ def test_approve_revision_dry_run_test_page(self, mock_get_title, mock_request_c """Test dry-run mode with test page (should proceed).""" # Mock the site mock_site_class.return_value = self.test_site - + # Mock page title (test page) mock_get_title.return_value = "Merkityt_versiot_-kokeilu/Test_Page" - + # Mock the API request mock_request = Mock() mock_request.submit.return_value = {'review': {'result': 'success'}} mock_request_class.return_value = mock_request - + # Call the function result = approve_revision( revid=self.test_revid, comment=self.test_comment, unapprove=False ) - + # Assertions self.assertEqual(result['result'], 'success') self.assertFalse(result['dry_run']) self.assertIn('Successfully approved', result['message']) - + # Verify API call was made mock_request_class.assert_called_once() @@ -142,12 +142,12 @@ def test_approve_revision_with_value(self, mock_request_class, mock_site_class): """Test approval with custom value parameter.""" # Mock the site mock_site_class.return_value = self.test_site - + # Mock the API request mock_request = Mock() mock_request.submit.return_value = {'review': {'result': 'success'}} mock_request_class.return_value = mock_request - + # Call the function with value result = approve_revision( revid=self.test_revid, @@ -155,10 +155,10 @@ def test_approve_revision_with_value(self, mock_request_class, mock_site_class): value=1, unapprove=False ) - + # Assertions self.assertEqual(result['result'], 'success') - + # Verify API call was made with value parameter mock_request_class.assert_called_once() call_args = mock_request_class.call_args @@ -171,7 +171,7 @@ def test_approve_revision_api_error(self, mock_request_class, mock_site_class): """Test handling of API errors.""" # Mock the site mock_site_class.return_value = self.test_site - + # Mock API error response mock_request = Mock() mock_request.submit.return_value = { @@ -181,14 +181,14 @@ def test_approve_revision_api_error(self, mock_request_class, mock_site_class): } } mock_request_class.return_value = mock_request - + # Call the function result = approve_revision( revid=self.test_revid, comment=self.test_comment, unapprove=False ) - + # Assertions self.assertEqual(result['result'], 'error') self.assertIn('Failed to approve', result['message']) @@ -199,14 +199,14 @@ def test_approve_revision_exception(self, mock_site_class): """Test handling of exceptions.""" # Mock the site to raise an exception mock_site_class.side_effect = Exception("Connection error") - + # Call the function result = approve_revision( revid=self.test_revid, comment=self.test_comment, unapprove=False ) - + # Assertions self.assertEqual(result['result'], 'error') self.assertIn('Error approving', result['message']) @@ -227,10 +227,10 @@ def test_get_page_title_from_revid_success(self, mock_request_class): } } mock_request_class.return_value = mock_request - + # Call the function title = _get_page_title_from_revid(self.test_site, self.test_revid) - + # Assertions self.assertEqual(title, 'Test_Page') mock_request_class.assert_called_once() @@ -242,10 +242,10 @@ def test_get_page_title_from_revid_not_found(self, mock_request_class): mock_request = Mock() mock_request.submit.return_value = {'query': {'pages': {}}} mock_request_class.return_value = mock_request - + # Call the function title = _get_page_title_from_revid(self.test_site, self.test_revid) - + # Assertions self.assertIsNone(title) @@ -256,10 +256,10 @@ def test_get_page_title_from_revid_exception(self, mock_request_class): mock_request = Mock() mock_request.submit.side_effect = Exception("API error") mock_request_class.return_value = mock_request - + # Call the function title = _get_page_title_from_revid(self.test_site, self.test_revid) - + # Assertions self.assertIsNone(title) diff --git a/app/reviews/tests/test_revert_detection.py b/app/reviews/tests/test_revert_detection.py index d22b406a..efcf729d 100644 --- a/app/reviews/tests/test_revert_detection.py +++ b/app/reviews/tests/test_revert_detection.py @@ -28,14 +28,14 @@ def setUp(self): api_endpoint="https://test.wikipedia.org/w/api.php" ) self.config = WikiConfiguration.objects.create(wiki=self.wiki) - + self.page = PendingPage.objects.create( wiki=self.wiki, pageid=12345, title="Test Page", stable_revid=100, ) - + self.revision = PendingRevision.objects.create( page=self.page, revid=200, @@ -52,7 +52,7 @@ def setUp(self): }) ] ) - + self.client = Mock(spec=WikiClient) self.client.site = Mock() @@ -60,7 +60,7 @@ def test_revert_detection_disabled(self): """Test that revert detection is skipped when disabled.""" with self.settings(ENABLE_REVERT_DETECTION=False): result = _check_revert_detection(self.revision, self.client) - + self.assertEqual(result["status"], "skip") self.assertEqual(result["message"], "Revert detection is disabled") @@ -68,16 +68,16 @@ def test_no_revert_tags(self): """Test that revert detection is skipped when no revert tags are present.""" self.revision.change_tags = ["mw-edit"] self.revision.save() - + result = _check_revert_detection(self.revision, self.client) - + self.assertEqual(result["status"], "skip") self.assertEqual(result["message"], "No revert tags found") def test_parse_revert_params(self): """Test parsing of change tag parameters.""" reverted_ids = _parse_revert_params(self.revision) - + expected_ids = [180, 190, 175] # From change_tag_params self.assertEqual(set(reverted_ids), set(expected_ids)) @@ -85,7 +85,7 @@ def test_parse_revert_params_empty(self): """Test parsing when no change tag parameters are present.""" self.revision.change_tag_params = [] self.revision.save() - + reverted_ids = _parse_revert_params(self.revision) self.assertEqual(reverted_ids, []) @@ -93,7 +93,7 @@ def test_parse_revert_params_invalid_json(self): """Test parsing with invalid JSON in change tag parameters.""" self.revision.change_tag_params = ["invalid json"] self.revision.save() - + reverted_ids = _parse_revert_params(self.revision) self.assertEqual(reverted_ids, []) @@ -109,12 +109,12 @@ def test_find_reviewed_revisions_by_sha1_success(self, mock_superset): 'rev_page': 12345 } ] - + reverted_ids = [180, 190] reviewed_revisions = _find_reviewed_revisions_by_sha1( self.client, self.page, reverted_ids ) - + self.assertEqual(len(reviewed_revisions), 1) self.assertEqual(reviewed_revisions[0]['sha1'], 'abc123') self.assertEqual(reviewed_revisions[0]['max_reviewed_id'], 150) @@ -123,12 +123,12 @@ def test_find_reviewed_revisions_by_sha1_success(self, mock_superset): def test_find_reviewed_revisions_by_sha1_no_results(self, mock_superset): """Test when no reviewed revisions are found.""" mock_superset.return_value.query.return_value = [] - + reverted_ids = [180, 190] reviewed_revisions = _find_reviewed_revisions_by_sha1( self.client, self.page, reverted_ids ) - + self.assertEqual(reviewed_revisions, []) @patch('reviews.autoreview._find_reviewed_revisions_by_sha1') @@ -143,9 +143,9 @@ def test_revert_detection_approve(self, mock_find_reviewed): 'page_id': 12345 } ] - + result = _check_revert_detection(self.revision, self.client) - + self.assertEqual(result["status"], "approve") self.assertIn("Revert to previously reviewed content", result["message"]) self.assertIn("abc123", result["message"]) @@ -155,9 +155,9 @@ def test_revert_detection_block(self, mock_find_reviewed): """Test revert detection when no reviewed content is found.""" # Mock no reviewed revisions found mock_find_reviewed.return_value = [] - + result = _check_revert_detection(self.revision, self.client) - + self.assertEqual(result["status"], "block") self.assertEqual(result["message"], "Revert detected but no previously reviewed content found") @@ -165,9 +165,9 @@ def test_revert_detection_no_reverted_ids(self): """Test revert detection when no reverted revision IDs are found.""" self.revision.change_tag_params = [] self.revision.save() - + result = _check_revert_detection(self.revision, self.client) - + self.assertEqual(result["status"], "skip") self.assertEqual(result["message"], "No reverted revision IDs found in change tags") @@ -175,9 +175,9 @@ def test_revert_detection_metadata(self): """Test that revert detection returns proper metadata.""" with patch('reviews.autoreview._find_reviewed_revisions_by_sha1') as mock_find: mock_find.return_value = [{'sha1': 'abc123'}] - + result = _check_revert_detection(self.revision, self.client) - + self.assertIn("reverted_rev_ids", result["metadata"]) self.assertIn("revert_tags", result["metadata"]) self.assertIn("reviewed_revisions", result["metadata"]) @@ -205,7 +205,7 @@ def test_revert_detection_with_real_revision(self): title="Test Page", stable_revid=100, ) - + # Create a revision with revert tags revision = PendingRevision.objects.create( page=page, @@ -223,11 +223,11 @@ def test_revert_detection_with_real_revision(self): }) ] ) - + # Mock the client client = Mock(spec=WikiClient) client.site = Mock() - + # Test with SupersetQuery mock with patch('reviews.autoreview.SupersetQuery') as mock_superset: mock_superset.return_value.query.return_value = [ @@ -238,9 +238,9 @@ def test_revert_detection_with_real_revision(self): 'rev_page': 12345 } ] - + result = _check_revert_detection(revision, client) - + self.assertEqual(result["status"], "approve") self.assertIn("test_sha1", result["message"]) self.assertEqual(len(result["metadata"]["reverted_rev_ids"]), 3) diff --git a/app/reviews/utils/approval.py b/app/reviews/utils/approval.py index 2c294dfd..ea65f35c 100644 --- a/app/reviews/utils/approval.py +++ b/app/reviews/utils/approval.py @@ -30,12 +30,12 @@ def approve_revision(revid, comment, value=None, unapprove=False): try: # Get the site (assuming we're working with Finnish Wikipedia) site = Site('fi', 'wikipedia') - + # Check if we're in dry-run mode if getattr(settings, 'PENDING_CHANGES_DRY_RUN', True): # Get page title to check if it's in test namespace page_title = _get_page_title_from_revid(site, revid) - + if page_title and not page_title.startswith('Merkityt_versiot_-kokeilu/'): action = 'unapprove' if unapprove else 'approve' logger.info( @@ -46,26 +46,26 @@ def approve_revision(revid, comment, value=None, unapprove=False): 'dry_run': True, 'message': f"DRY-RUN: Would {action} revision {revid}" } - + # Prepare API request parameters params = { 'action': 'review', 'revid': revid, 'comment': comment, } - + # Add unapprove parameter if needed if unapprove: params['unapprove'] = '1' - + # Add value parameter if provided if value is not None: params['value'] = str(value) - + # Make the API request request = Request(site=site, **params) result = request.submit() - + # Check if the request was successful if 'review' in result: action_past = 'unapproved' if unapprove else 'approved' @@ -85,7 +85,7 @@ def approve_revision(revid, comment, value=None, unapprove=False): 'message': f"Failed to {action} revision {revid}", 'api_response': result } - + except Exception as e: action_ing = 'unapproving' if unapprove else 'approving' logger.error("Error %s revision %s: %s", action_ing, revid, e) @@ -116,14 +116,14 @@ def _get_page_title_from_revid(site, revid): rvprop='title' ) result = request.submit() - + if 'query' in result and 'pages' in result['query']: for page_id, page_data in result['query']['pages'].items(): if 'revisions' in page_data: return page_data['title'] - + return None - + except Exception as e: logger.error("Error getting page title for revision %s: %s", revid, e) return None From f03702affe63ec3841541ed2cc831f5a10b52663 Mon Sep 17 00:00:00 2001 From: AmbatI_Teja_Sri_Surya Date: Wed, 29 Oct 2025 15:50:30 -0500 Subject: [PATCH 09/26] style: wrap long message and clean blank-line whitespace in revert_detection (fix E501/W293) --- app/reviews/autoreview/checks/revert_detection.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/app/reviews/autoreview/checks/revert_detection.py b/app/reviews/autoreview/checks/revert_detection.py index fe8adad5..c8264fb5 100644 --- a/app/reviews/autoreview/checks/revert_detection.py +++ b/app/reviews/autoreview/checks/revert_detection.py @@ -65,7 +65,10 @@ def check_revert_detection(context: CheckContext) -> dict[str, Any]: if reviewed_revisions: return { "status": "approve", - "message": f"Revert to previously reviewed content (SHA1: {reviewed_revisions[0]['sha1']})", + "message": ( + "Revert to previously reviewed content (SHA1: " + f"{reviewed_revisions[0]['sha1']})" + ), "metadata": { "reverted_rev_ids": reverted_rev_ids, "reviewed_revisions": reviewed_revisions, @@ -86,10 +89,10 @@ def check_revert_detection(context: CheckContext) -> dict[str, Any]: def _parse_revert_params(revision) -> list[int]: """ Parse change tag parameters to extract reverted revision IDs. - + Args: revision: PendingRevision object - + Returns: List of reverted revision IDs """ From fb2ab93b1d03905b8a840d7d08a5323dfaf931b4 Mon Sep 17 00:00:00 2001 From: AmbatI_Teja_Sri_Surya Date: Wed, 29 Oct 2025 15:55:00 -0500 Subject: [PATCH 10/26] style: clean docstring blanks and sort imports (fix W293/I001) --- app/reviews/autoreview/checks/revert_detection.py | 6 +++--- .../management/commands/test_pending_changes_review.py | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/app/reviews/autoreview/checks/revert_detection.py b/app/reviews/autoreview/checks/revert_detection.py index c8264fb5..658e7130 100644 --- a/app/reviews/autoreview/checks/revert_detection.py +++ b/app/reviews/autoreview/checks/revert_detection.py @@ -133,16 +133,16 @@ def _find_reviewed_revisions_by_sha1( ) -> list[dict]: """ Find previously reviewed revisions by SHA1 content hash. - + This implements @zache-fi's suggested Superset approach: 1. Query MediaWiki database for older reviewed versions by SHA1 2. Check if any of the reverted revisions were previously reviewed - + Args: client: WikiClient instance page: PendingPage object reverted_rev_ids: List of reverted revision IDs - + Returns: List of reviewed revision data """ diff --git a/app/reviews/management/commands/test_pending_changes_review.py b/app/reviews/management/commands/test_pending_changes_review.py index 593a357a..0c46162b 100644 --- a/app/reviews/management/commands/test_pending_changes_review.py +++ b/app/reviews/management/commands/test_pending_changes_review.py @@ -5,8 +5,9 @@ with various parameters and configurations. """ -from django.core.management.base import BaseCommand, CommandError from django.conf import settings +from django.core.management.base import BaseCommand, CommandError + from reviews.utils.approval import approve_revision From 5002f9fd3b7f1aa3930b912a95a87a2b8e3ac673 Mon Sep 17 00:00:00 2001 From: AmbatI_Teja_Sri_Surya Date: Wed, 29 Oct 2025 16:03:42 -0500 Subject: [PATCH 11/26] style: apply ruff fixes/format --- app/reviewer/settings.py | 14 +- .../autoreview/checks/revert_detection.py | 64 +++---- .../commands/test_pending_changes_review.py | 78 ++++----- app/reviews/tests/test_approval.py | 165 ++++++++---------- app/reviews/tests/test_revert_detection.py | 96 +++++----- app/reviews/utils/approval.py | 87 +++++---- 6 files changed, 224 insertions(+), 280 deletions(-) diff --git a/app/reviewer/settings.py b/app/reviewer/settings.py index 225a2d05..272def33 100644 --- a/app/reviewer/settings.py +++ b/app/reviewer/settings.py @@ -127,18 +127,20 @@ # Revert detection configuration # Enable/disable revert detection for already-reviewed edits -ENABLE_REVERT_DETECTION = ( - os.getenv("ENABLE_REVERT_DETECTION", "True").lower() - in ("true", "1", "yes") +ENABLE_REVERT_DETECTION = os.getenv("ENABLE_REVERT_DETECTION", "True").lower() in ( + "true", + "1", + "yes", ) # Pending changes approval configuration # Enable/disable dry-run mode for pending changes approval # When True, only allows approvals on test pages (Merkityt_versiot_-kokeilu/*) # When False, allows approvals on all pages -PENDING_CHANGES_DRY_RUN = ( - os.getenv("PENDING_CHANGES_DRY_RUN", "True").lower() - in ("true", "1", "yes") +PENDING_CHANGES_DRY_RUN = os.getenv("PENDING_CHANGES_DRY_RUN", "True").lower() in ( + "true", + "1", + "yes", ) # ORES model thresholds (global defaults, per-wiki config takes precedence) diff --git a/app/reviews/autoreview/checks/revert_detection.py b/app/reviews/autoreview/checks/revert_detection.py index 658e7130..34ecafdc 100644 --- a/app/reviews/autoreview/checks/revert_detection.py +++ b/app/reviews/autoreview/checks/revert_detection.py @@ -27,25 +27,21 @@ def check_revert_detection(context: CheckContext) -> dict[str, Any]: Dict with check result including status, message, and metadata """ # Check if revert detection is enabled - if not getattr(settings, 'ENABLE_REVERT_DETECTION', True): - return { - "status": "skip", - "message": "Revert detection is disabled", - "metadata": {} - } + if not getattr(settings, "ENABLE_REVERT_DETECTION", True): + return {"status": "skip", "message": "Revert detection is disabled", "metadata": {}} revision = context.revision page = revision.page # Check for revert tags revert_tags = {"mw-manual-revert", "mw-reverted", "mw-rollback", "mw-undo"} - change_tags = getattr(revision, 'change_tags', []) + change_tags = getattr(revision, "change_tags", []) if not any(tag in change_tags for tag in revert_tags): return { "status": "skip", "message": "No revert tags found", - "metadata": {"change_tags": change_tags} + "metadata": {"change_tags": change_tags}, } # Parse change tag parameters to get reverted revision IDs @@ -54,26 +50,23 @@ def check_revert_detection(context: CheckContext) -> dict[str, Any]: return { "status": "skip", "message": "No reverted revision IDs found in change tags", - "metadata": {"change_tags": change_tags} + "metadata": {"change_tags": change_tags}, } # Check if any of the reverted revisions were previously reviewed - reviewed_revisions = _find_reviewed_revisions_by_sha1( - context.client, page, reverted_rev_ids - ) + reviewed_revisions = _find_reviewed_revisions_by_sha1(context.client, page, reverted_rev_ids) if reviewed_revisions: return { "status": "approve", "message": ( - "Revert to previously reviewed content (SHA1: " - f"{reviewed_revisions[0]['sha1']})" + f"Revert to previously reviewed content (SHA1: {reviewed_revisions[0]['sha1']})" ), "metadata": { "reverted_rev_ids": reverted_rev_ids, "reviewed_revisions": reviewed_revisions, - "revert_tags": [tag for tag in change_tags if tag in revert_tags] - } + "revert_tags": [tag for tag in change_tags if tag in revert_tags], + }, } return { @@ -81,8 +74,8 @@ def check_revert_detection(context: CheckContext) -> dict[str, Any]: "message": "Revert detected but no previously reviewed content found", "metadata": { "reverted_rev_ids": reverted_rev_ids, - "revert_tags": [tag for tag in change_tags if tag in revert_tags] - } + "revert_tags": [tag for tag in change_tags if tag in revert_tags], + }, } @@ -98,7 +91,7 @@ def _parse_revert_params(revision) -> list[int]: """ try: # Get change tag parameters from revision - change_tag_params = getattr(revision, 'change_tag_params', []) + change_tag_params = getattr(revision, "change_tag_params", []) if not change_tag_params: return [] @@ -110,12 +103,12 @@ def _parse_revert_params(revision) -> list[int]: param_data = json.loads(param_str) # Extract reverted revision IDs - if 'oldestRevertedRevId' in param_data: - reverted_ids.append(param_data['oldestRevertedRevId']) - if 'newestRevertedRevId' in param_data: - reverted_ids.append(param_data['newestRevertedRevId']) - if 'originalRevisionId' in param_data: - reverted_ids.append(param_data['originalRevisionId']) + if "oldestRevertedRevId" in param_data: + reverted_ids.append(param_data["oldestRevertedRevId"]) + if "newestRevertedRevId" in param_data: + reverted_ids.append(param_data["newestRevertedRevId"]) + if "originalRevisionId" in param_data: + reverted_ids.append(param_data["originalRevisionId"]) except (json.JSONDecodeError, KeyError) as e: logger.warning(f"Failed to parse change tag param: {param_str}, error: {e}") @@ -128,9 +121,7 @@ def _parse_revert_params(revision) -> list[int]: return [] -def _find_reviewed_revisions_by_sha1( - client, page, reverted_rev_ids: list[int] -) -> list[dict]: +def _find_reviewed_revisions_by_sha1(client, page, reverted_rev_ids: list[int]) -> list[dict]: """ Find previously reviewed revisions by SHA1 content hash. @@ -174,19 +165,22 @@ def _find_reviewed_revisions_by_sha1( # Execute query using SupersetQuery from pywikibot.data.superset import SupersetQuery + superset = SupersetQuery(site=client.site) results = superset.query(sql_query) # Filter results where content was previously reviewed reviewed_revisions = [] for result in results: - if result.get('max_old_reviewed_id') is not None: - reviewed_revisions.append({ - 'sha1': result.get('content_sha1'), - 'max_reviewed_id': result.get('max_old_reviewed_id'), - 'max_reviewable_id': result.get('max_reviewable_rev_id_by_sha1'), - 'page_id': result.get('rev_page') - }) + if result.get("max_old_reviewed_id") is not None: + reviewed_revisions.append( + { + "sha1": result.get("content_sha1"), + "max_reviewed_id": result.get("max_old_reviewed_id"), + "max_reviewable_id": result.get("max_reviewable_rev_id_by_sha1"), + "page_id": result.get("rev_page"), + } + ) return reviewed_revisions diff --git a/app/reviews/management/commands/test_pending_changes_review.py b/app/reviews/management/commands/test_pending_changes_review.py index 0c46162b..7532f9d0 100644 --- a/app/reviews/management/commands/test_pending_changes_review.py +++ b/app/reviews/management/commands/test_pending_changes_review.py @@ -12,46 +12,41 @@ class Command(BaseCommand): - help = 'Test pending changes review functionality (approve/unapprove revisions)' + help = "Test pending changes review functionality (approve/unapprove revisions)" def add_arguments(self, parser): parser.add_argument( - '--revid', - type=int, - required=True, - help='Revision ID to approve/unapprove' + "--revid", type=int, required=True, help="Revision ID to approve/unapprove" ) parser.add_argument( - '--comment', + "--comment", type=str, - default='Test approval via management command', - help='Comment for the review (default: "Test approval via management command")' + default="Test approval via management command", + help='Comment for the review (default: "Test approval via management command")', ) parser.add_argument( - '--unapprove', - action='store_true', - help='Unapprove the revision instead of approving it' + "--unapprove", + action="store_true", + help="Unapprove the revision instead of approving it", ) + parser.add_argument("--value", type=int, help="Flag value for the review (optional)") parser.add_argument( - '--value', - type=int, - help='Flag value for the review (optional)' - ) - parser.add_argument( - '--dry-run', - action='store_true', - help='Show what would happen without making actual changes' + "--dry-run", + action="store_true", + help="Show what would happen without making actual changes", ) def handle(self, *args, **options): - revid = options['revid'] - comment = options['comment'] - unapprove = options['unapprove'] - value = options['value'] - dry_run = options['dry_run'] + revid = options["revid"] + comment = options["comment"] + unapprove = options["unapprove"] + value = options["value"] + dry_run = options["dry_run"] # Display current configuration - self.stdout.write(f"Current PENDING_CHANGES_DRY_RUN setting: {getattr(settings, 'PENDING_CHANGES_DRY_RUN', True)}") + self.stdout.write( + f"Current PENDING_CHANGES_DRY_RUN setting: {getattr(settings, 'PENDING_CHANGES_DRY_RUN', True)}" + ) if dry_run: self.stdout.write(self.style.WARNING("DRY-RUN MODE: No actual changes will be made")) @@ -67,33 +62,24 @@ def handle(self, *args, **options): try: # Call the approve_revision function result = approve_revision( - revid=revid, - comment=comment, - value=value, - unapprove=unapprove + revid=revid, comment=comment, value=value, unapprove=unapprove ) # Display results - if result['result'] == 'success': - if result.get('dry_run', False): - self.stdout.write( - self.style.SUCCESS(f"✅ {result['message']}") - ) + if result["result"] == "success": + if result.get("dry_run", False): + self.stdout.write(self.style.SUCCESS(f"✅ {result['message']}")) else: - self.stdout.write( - self.style.SUCCESS(f"✅ {result['message']}") - ) + self.stdout.write(self.style.SUCCESS(f"✅ {result['message']}")) else: - self.stdout.write( - self.style.ERROR(f"❌ {result['message']}") - ) + self.stdout.write(self.style.ERROR(f"❌ {result['message']}")) # Display additional information - if 'api_response' in result: + if "api_response" in result: self.stdout.write(f"API Response: {result['api_response']}") # Display dry-run information - if result.get('dry_run', False): + if result.get("dry_run", False): self.stdout.write( self.style.WARNING( "ℹ️ This was a dry-run operation. " @@ -102,11 +88,7 @@ def handle(self, *args, **options): ) except Exception as e: - self.stdout.write( - self.style.ERROR(f"❌ Error: {str(e)}") - ) + self.stdout.write(self.style.ERROR(f"❌ Error: {str(e)}")) raise CommandError(f"Failed to {operation} revision {revid}: {str(e)}") - self.stdout.write( - self.style.SUCCESS(f"✅ Command completed successfully") - ) + self.stdout.write(self.style.SUCCESS("✅ Command completed successfully")) diff --git a/app/reviews/tests/test_approval.py b/app/reviews/tests/test_approval.py index 60726253..66cc1729 100644 --- a/app/reviews/tests/test_approval.py +++ b/app/reviews/tests/test_approval.py @@ -4,11 +4,12 @@ Tests the approve_revision() function and related functionality. """ -import unittest -from unittest.mock import Mock, patch, MagicMock -from django.test import TestCase, override_settings +from unittest.mock import Mock, patch + from django.conf import settings -from reviews.utils.approval import approve_revision, _get_page_title_from_revid +from django.test import TestCase, override_settings + +from reviews.utils.approval import _get_page_title_from_revid, approve_revision class ApprovalUtilityTests(TestCase): @@ -19,11 +20,11 @@ def setUp(self): self.test_revid = 12345 self.test_comment = "Test approval" self.test_site = Mock() - self.test_site.code = 'fi' - self.test_site.family.name = 'wikipedia' + self.test_site.code = "fi" + self.test_site.family.name = "wikipedia" - @patch('reviews.utils.approval.Site') - @patch('reviews.utils.approval.Request') + @patch("reviews.utils.approval.Site") + @patch("reviews.utils.approval.Request") @override_settings(PENDING_CHANGES_DRY_RUN=False) def test_approve_revision_success(self, mock_request_class, mock_site_class): """Test successful approval of a revision.""" @@ -32,27 +33,23 @@ def test_approve_revision_success(self, mock_request_class, mock_site_class): # Mock the API request mock_request = Mock() - mock_request.submit.return_value = {'review': {'result': 'success'}} + mock_request.submit.return_value = {"review": {"result": "success"}} mock_request_class.return_value = mock_request # Call the function - result = approve_revision( - revid=self.test_revid, - comment=self.test_comment, - unapprove=False - ) + result = approve_revision(revid=self.test_revid, comment=self.test_comment, unapprove=False) # Assertions - self.assertEqual(result['result'], 'success') - self.assertFalse(result['dry_run']) - self.assertIn('Successfully approved', result['message']) + self.assertEqual(result["result"], "success") + self.assertFalse(result["dry_run"]) + self.assertIn("Successfully approved", result["message"]) # Verify API call was made mock_request_class.assert_called_once() mock_request.submit.assert_called_once() - @patch('reviews.utils.approval.Site') - @patch('reviews.utils.approval.Request') + @patch("reviews.utils.approval.Site") + @patch("reviews.utils.approval.Request") @override_settings(PENDING_CHANGES_DRY_RUN=False) def test_approve_revision_unapprove(self, mock_request_class, mock_site_class): """Test successful unapproval of a revision.""" @@ -61,26 +58,24 @@ def test_approve_revision_unapprove(self, mock_request_class, mock_site_class): # Mock the API request mock_request = Mock() - mock_request.submit.return_value = {'review': {'result': 'success'}} + mock_request.submit.return_value = {"review": {"result": "success"}} mock_request_class.return_value = mock_request # Call the function - result = approve_revision( - revid=self.test_revid, - comment=self.test_comment, - unapprove=True - ) + result = approve_revision(revid=self.test_revid, comment=self.test_comment, unapprove=True) # Assertions - self.assertEqual(result['result'], 'success') - self.assertFalse(result['dry_run']) - self.assertIn('Successfully unapproved', result['message']) + self.assertEqual(result["result"], "success") + self.assertFalse(result["dry_run"]) + self.assertIn("Successfully unapproved", result["message"]) - @patch('reviews.utils.approval.Site') - @patch('reviews.utils.approval.Request') - @patch('reviews.utils.approval._get_page_title_from_revid') + @patch("reviews.utils.approval.Site") + @patch("reviews.utils.approval.Request") + @patch("reviews.utils.approval._get_page_title_from_revid") @override_settings(PENDING_CHANGES_DRY_RUN=True) - def test_approve_revision_dry_run_production_page(self, mock_get_title, mock_request_class, mock_site_class): + def test_approve_revision_dry_run_production_page( + self, mock_get_title, mock_request_class, mock_site_class + ): """Test dry-run mode with production page (should skip).""" # Mock the site mock_site_class.return_value = self.test_site @@ -89,25 +84,23 @@ def test_approve_revision_dry_run_production_page(self, mock_get_title, mock_req mock_get_title.return_value = "Production_Page" # Call the function - result = approve_revision( - revid=self.test_revid, - comment=self.test_comment, - unapprove=False - ) + result = approve_revision(revid=self.test_revid, comment=self.test_comment, unapprove=False) # Assertions - self.assertEqual(result['result'], 'success') - self.assertTrue(result['dry_run']) - self.assertIn('DRY-RUN: Would approve', result['message']) + self.assertEqual(result["result"], "success") + self.assertTrue(result["dry_run"]) + self.assertIn("DRY-RUN: Would approve", result["message"]) # Verify API call was NOT made mock_request_class.assert_not_called() - @patch('reviews.utils.approval.Site') - @patch('reviews.utils.approval.Request') - @patch('reviews.utils.approval._get_page_title_from_revid') + @patch("reviews.utils.approval.Site") + @patch("reviews.utils.approval.Request") + @patch("reviews.utils.approval._get_page_title_from_revid") @override_settings(PENDING_CHANGES_DRY_RUN=True) - def test_approve_revision_dry_run_test_page(self, mock_get_title, mock_request_class, mock_site_class): + def test_approve_revision_dry_run_test_page( + self, mock_get_title, mock_request_class, mock_site_class + ): """Test dry-run mode with test page (should proceed).""" # Mock the site mock_site_class.return_value = self.test_site @@ -117,26 +110,22 @@ def test_approve_revision_dry_run_test_page(self, mock_get_title, mock_request_c # Mock the API request mock_request = Mock() - mock_request.submit.return_value = {'review': {'result': 'success'}} + mock_request.submit.return_value = {"review": {"result": "success"}} mock_request_class.return_value = mock_request # Call the function - result = approve_revision( - revid=self.test_revid, - comment=self.test_comment, - unapprove=False - ) + result = approve_revision(revid=self.test_revid, comment=self.test_comment, unapprove=False) # Assertions - self.assertEqual(result['result'], 'success') - self.assertFalse(result['dry_run']) - self.assertIn('Successfully approved', result['message']) + self.assertEqual(result["result"], "success") + self.assertFalse(result["dry_run"]) + self.assertIn("Successfully approved", result["message"]) # Verify API call was made mock_request_class.assert_called_once() - @patch('reviews.utils.approval.Site') - @patch('reviews.utils.approval.Request') + @patch("reviews.utils.approval.Site") + @patch("reviews.utils.approval.Request") @override_settings(PENDING_CHANGES_DRY_RUN=False) def test_approve_revision_with_value(self, mock_request_class, mock_site_class): """Test approval with custom value parameter.""" @@ -145,27 +134,24 @@ def test_approve_revision_with_value(self, mock_request_class, mock_site_class): # Mock the API request mock_request = Mock() - mock_request.submit.return_value = {'review': {'result': 'success'}} + mock_request.submit.return_value = {"review": {"result": "success"}} mock_request_class.return_value = mock_request # Call the function with value result = approve_revision( - revid=self.test_revid, - comment=self.test_comment, - value=1, - unapprove=False + revid=self.test_revid, comment=self.test_comment, value=1, unapprove=False ) # Assertions - self.assertEqual(result['result'], 'success') + self.assertEqual(result["result"], "success") # Verify API call was made with value parameter mock_request_class.assert_called_once() call_args = mock_request_class.call_args - self.assertEqual(call_args[1]['value'], '1') + self.assertEqual(call_args[1]["value"], "1") - @patch('reviews.utils.approval.Site') - @patch('reviews.utils.approval.Request') + @patch("reviews.utils.approval.Site") + @patch("reviews.utils.approval.Request") @override_settings(PENDING_CHANGES_DRY_RUN=False) def test_approve_revision_api_error(self, mock_request_class, mock_site_class): """Test handling of API errors.""" @@ -175,25 +161,21 @@ def test_approve_revision_api_error(self, mock_request_class, mock_site_class): # Mock API error response mock_request = Mock() mock_request.submit.return_value = { - 'error': { - 'code': 'permissiondenied', - 'info': 'Permission denied', + "error": { + "code": "permissiondenied", + "info": "Permission denied", } } mock_request_class.return_value = mock_request # Call the function - result = approve_revision( - revid=self.test_revid, - comment=self.test_comment, - unapprove=False - ) + result = approve_revision(revid=self.test_revid, comment=self.test_comment, unapprove=False) # Assertions - self.assertEqual(result['result'], 'error') - self.assertIn('Failed to approve', result['message']) + self.assertEqual(result["result"], "error") + self.assertIn("Failed to approve", result["message"]) - @patch('reviews.utils.approval.Site') + @patch("reviews.utils.approval.Site") @override_settings(PENDING_CHANGES_DRY_RUN=False) def test_approve_revision_exception(self, mock_site_class): """Test handling of exceptions.""" @@ -201,29 +183,20 @@ def test_approve_revision_exception(self, mock_site_class): mock_site_class.side_effect = Exception("Connection error") # Call the function - result = approve_revision( - revid=self.test_revid, - comment=self.test_comment, - unapprove=False - ) + result = approve_revision(revid=self.test_revid, comment=self.test_comment, unapprove=False) # Assertions - self.assertEqual(result['result'], 'error') - self.assertIn('Error approving', result['message']) + self.assertEqual(result["result"], "error") + self.assertIn("Error approving", result["message"]) - @patch('reviews.utils.approval.Request') + @patch("reviews.utils.approval.Request") def test_get_page_title_from_revid_success(self, mock_request_class): """Test successful retrieval of page title.""" # Mock the API request mock_request = Mock() mock_request.submit.return_value = { - 'query': { - 'pages': { - '123': { - 'title': 'Test_Page', - 'revisions': [{'revid': self.test_revid}] - } - } + "query": { + "pages": {"123": {"title": "Test_Page", "revisions": [{"revid": self.test_revid}]}} } } mock_request_class.return_value = mock_request @@ -232,15 +205,15 @@ def test_get_page_title_from_revid_success(self, mock_request_class): title = _get_page_title_from_revid(self.test_site, self.test_revid) # Assertions - self.assertEqual(title, 'Test_Page') + self.assertEqual(title, "Test_Page") mock_request_class.assert_called_once() - @patch('reviews.utils.approval.Request') + @patch("reviews.utils.approval.Request") def test_get_page_title_from_revid_not_found(self, mock_request_class): """Test handling when page title is not found.""" # Mock the API request mock_request = Mock() - mock_request.submit.return_value = {'query': {'pages': {}}} + mock_request.submit.return_value = {"query": {"pages": {}}} mock_request_class.return_value = mock_request # Call the function @@ -249,7 +222,7 @@ def test_get_page_title_from_revid_not_found(self, mock_request_class): # Assertions self.assertIsNone(title) - @patch('reviews.utils.approval.Request') + @patch("reviews.utils.approval.Request") def test_get_page_title_from_revid_exception(self, mock_request_class): """Test handling of exceptions in _get_page_title_from_revid.""" # Mock the API request to raise an exception @@ -271,10 +244,10 @@ class ApprovalIntegrationTests(TestCase): def test_dry_run_setting_respected(self): """Test that the dry-run setting is properly respected.""" # This test verifies that the setting is read correctly - self.assertTrue(getattr(settings, 'PENDING_CHANGES_DRY_RUN', True)) + self.assertTrue(getattr(settings, "PENDING_CHANGES_DRY_RUN", True)) @override_settings(PENDING_CHANGES_DRY_RUN=False) def test_dry_run_setting_disabled(self): """Test that the dry-run setting can be disabled.""" # This test verifies that the setting can be disabled - self.assertFalse(getattr(settings, 'PENDING_CHANGES_DRY_RUN', True)) + self.assertFalse(getattr(settings, "PENDING_CHANGES_DRY_RUN", True)) diff --git a/app/reviews/tests/test_revert_detection.py b/app/reviews/tests/test_revert_detection.py index efcf729d..30f0517f 100644 --- a/app/reviews/tests/test_revert_detection.py +++ b/app/reviews/tests/test_revert_detection.py @@ -9,11 +9,14 @@ from unittest.mock import Mock, patch from django.test import TestCase -from django.conf import settings +from reviews.autoreview import ( + _check_revert_detection, + _find_reviewed_revisions_by_sha1, + _parse_revert_params, +) from reviews.models import PendingPage, PendingRevision, Wiki, WikiConfiguration from reviews.services import WikiClient -from reviews.autoreview import _check_revert_detection, _parse_revert_params, _find_reviewed_revisions_by_sha1 class RevertDetectionTests(TestCase): @@ -25,7 +28,7 @@ def setUp(self): name="Test Wiki", code="test", family="wikipedia", - api_endpoint="https://test.wikipedia.org/w/api.php" + api_endpoint="https://test.wikipedia.org/w/api.php", ) self.config = WikiConfiguration.objects.create(wiki=self.wiki) @@ -44,13 +47,15 @@ def setUp(self): user_id=1000, change_tags=["mw-manual-revert"], change_tag_params=[ - json.dumps({ - "revertId": 200, - "oldestRevertedRevId": 180, - "newestRevertedRevId": 190, - "originalRevisionId": 175 - }) - ] + json.dumps( + { + "revertId": 200, + "oldestRevertedRevId": 180, + "newestRevertedRevId": 190, + "originalRevisionId": 175, + } + ) + ], ) self.client = Mock(spec=WikiClient) @@ -97,51 +102,42 @@ def test_parse_revert_params_invalid_json(self): reverted_ids = _parse_revert_params(self.revision) self.assertEqual(reverted_ids, []) - @patch('reviews.autoreview.SupersetQuery') + @patch("reviews.autoreview.SupersetQuery") def test_find_reviewed_revisions_by_sha1_success(self, mock_superset): """Test finding reviewed revisions by SHA1.""" # Mock SupersetQuery results mock_superset.return_value.query.return_value = [ { - 'content_sha1': 'abc123', - 'max_old_reviewed_id': 150, - 'max_reviewable_rev_id_by_sha1': 180, - 'rev_page': 12345 + "content_sha1": "abc123", + "max_old_reviewed_id": 150, + "max_reviewable_rev_id_by_sha1": 180, + "rev_page": 12345, } ] reverted_ids = [180, 190] - reviewed_revisions = _find_reviewed_revisions_by_sha1( - self.client, self.page, reverted_ids - ) + reviewed_revisions = _find_reviewed_revisions_by_sha1(self.client, self.page, reverted_ids) self.assertEqual(len(reviewed_revisions), 1) - self.assertEqual(reviewed_revisions[0]['sha1'], 'abc123') - self.assertEqual(reviewed_revisions[0]['max_reviewed_id'], 150) + self.assertEqual(reviewed_revisions[0]["sha1"], "abc123") + self.assertEqual(reviewed_revisions[0]["max_reviewed_id"], 150) - @patch('reviews.autoreview.SupersetQuery') + @patch("reviews.autoreview.SupersetQuery") def test_find_reviewed_revisions_by_sha1_no_results(self, mock_superset): """Test when no reviewed revisions are found.""" mock_superset.return_value.query.return_value = [] reverted_ids = [180, 190] - reviewed_revisions = _find_reviewed_revisions_by_sha1( - self.client, self.page, reverted_ids - ) + reviewed_revisions = _find_reviewed_revisions_by_sha1(self.client, self.page, reverted_ids) self.assertEqual(reviewed_revisions, []) - @patch('reviews.autoreview._find_reviewed_revisions_by_sha1') + @patch("reviews.autoreview._find_reviewed_revisions_by_sha1") def test_revert_detection_approve(self, mock_find_reviewed): """Test revert detection when revert to reviewed content is found.""" # Mock finding reviewed revisions mock_find_reviewed.return_value = [ - { - 'sha1': 'abc123', - 'max_reviewed_id': 150, - 'max_reviewable_id': 180, - 'page_id': 12345 - } + {"sha1": "abc123", "max_reviewed_id": 150, "max_reviewable_id": 180, "page_id": 12345} ] result = _check_revert_detection(self.revision, self.client) @@ -150,7 +146,7 @@ def test_revert_detection_approve(self, mock_find_reviewed): self.assertIn("Revert to previously reviewed content", result["message"]) self.assertIn("abc123", result["message"]) - @patch('reviews.autoreview._find_reviewed_revisions_by_sha1') + @patch("reviews.autoreview._find_reviewed_revisions_by_sha1") def test_revert_detection_block(self, mock_find_reviewed): """Test revert detection when no reviewed content is found.""" # Mock no reviewed revisions found @@ -159,7 +155,9 @@ def test_revert_detection_block(self, mock_find_reviewed): result = _check_revert_detection(self.revision, self.client) self.assertEqual(result["status"], "block") - self.assertEqual(result["message"], "Revert detected but no previously reviewed content found") + self.assertEqual( + result["message"], "Revert detected but no previously reviewed content found" + ) def test_revert_detection_no_reverted_ids(self): """Test revert detection when no reverted revision IDs are found.""" @@ -173,8 +171,8 @@ def test_revert_detection_no_reverted_ids(self): def test_revert_detection_metadata(self): """Test that revert detection returns proper metadata.""" - with patch('reviews.autoreview._find_reviewed_revisions_by_sha1') as mock_find: - mock_find.return_value = [{'sha1': 'abc123'}] + with patch("reviews.autoreview._find_reviewed_revisions_by_sha1") as mock_find: + mock_find.return_value = [{"sha1": "abc123"}] result = _check_revert_detection(self.revision, self.client) @@ -193,7 +191,7 @@ def setUp(self): name="Test Wiki", code="test", family="wikipedia", - api_endpoint="https://test.wikipedia.org/w/api.php" + api_endpoint="https://test.wikipedia.org/w/api.php", ) self.config = WikiConfiguration.objects.create(wiki=self.wiki) @@ -215,13 +213,15 @@ def test_revert_detection_with_real_revision(self): user_id=1000, change_tags=["mw-manual-revert", "mw-reverted"], change_tag_params=[ - json.dumps({ - "revertId": 200, - "oldestRevertedRevId": 180, - "newestRevertedRevId": 190, - "originalRevisionId": 175 - }) - ] + json.dumps( + { + "revertId": 200, + "oldestRevertedRevId": 180, + "newestRevertedRevId": 190, + "originalRevisionId": 175, + } + ) + ], ) # Mock the client @@ -229,13 +229,13 @@ def test_revert_detection_with_real_revision(self): client.site = Mock() # Test with SupersetQuery mock - with patch('reviews.autoreview.SupersetQuery') as mock_superset: + with patch("reviews.autoreview.SupersetQuery") as mock_superset: mock_superset.return_value.query.return_value = [ { - 'content_sha1': 'test_sha1', - 'max_old_reviewed_id': 150, - 'max_reviewable_rev_id_by_sha1': 180, - 'rev_page': 12345 + "content_sha1": "test_sha1", + "max_old_reviewed_id": 150, + "max_reviewable_rev_id_by_sha1": 180, + "rev_page": 12345, } ] diff --git a/app/reviews/utils/approval.py b/app/reviews/utils/approval.py index ea65f35c..c73528d1 100644 --- a/app/reviews/utils/approval.py +++ b/app/reviews/utils/approval.py @@ -6,9 +6,10 @@ """ import logging + from django.conf import settings -from pywikibot.data.api import Request from pywikibot import Site +from pywikibot.data.api import Request logger = logging.getLogger(__name__) @@ -16,111 +17,103 @@ def approve_revision(revid, comment, value=None, unapprove=False): """ Approve or unapprove a pending changes revision. - + Args: revid (int): The revision ID for which to set the flags comment (str): Comment for the review value (int, optional): Flag value for the review. Defaults to None. - unapprove (bool, optional): If True, revision will be unapproved + unapprove (bool, optional): If True, revision will be unapproved rather than approved. Defaults to False. - + Returns: dict: Result of the review operation """ try: # Get the site (assuming we're working with Finnish Wikipedia) - site = Site('fi', 'wikipedia') + site = Site("fi", "wikipedia") # Check if we're in dry-run mode - if getattr(settings, 'PENDING_CHANGES_DRY_RUN', True): + if getattr(settings, "PENDING_CHANGES_DRY_RUN", True): # Get page title to check if it's in test namespace page_title = _get_page_title_from_revid(site, revid) - if page_title and not page_title.startswith('Merkityt_versiot_-kokeilu/'): - action = 'unapprove' if unapprove else 'approve' - logger.info( - "DRY-RUN: Would %s revision %s on %s", action, revid, page_title - ) + if page_title and not page_title.startswith("Merkityt_versiot_-kokeilu/"): + action = "unapprove" if unapprove else "approve" + logger.info("DRY-RUN: Would %s revision %s on %s", action, revid, page_title) return { - 'result': 'success', - 'dry_run': True, - 'message': f"DRY-RUN: Would {action} revision {revid}" + "result": "success", + "dry_run": True, + "message": f"DRY-RUN: Would {action} revision {revid}", } # Prepare API request parameters params = { - 'action': 'review', - 'revid': revid, - 'comment': comment, + "action": "review", + "revid": revid, + "comment": comment, } # Add unapprove parameter if needed if unapprove: - params['unapprove'] = '1' + params["unapprove"] = "1" # Add value parameter if provided if value is not None: - params['value'] = str(value) + params["value"] = str(value) # Make the API request request = Request(site=site, **params) result = request.submit() # Check if the request was successful - if 'review' in result: - action_past = 'unapproved' if unapprove else 'approved' + if "review" in result: + action_past = "unapproved" if unapprove else "approved" logger.info("Successfully %s revision %s", action_past, revid) return { - 'result': 'success', - 'dry_run': False, - 'message': f"Successfully {action_past} revision {revid}", - 'api_response': result['review'] + "result": "success", + "dry_run": False, + "message": f"Successfully {action_past} revision {revid}", + "api_response": result["review"], } else: - action = 'unapprove' if unapprove else 'approve' + action = "unapprove" if unapprove else "approve" logger.error("Failed to %s revision %s: %s", action, revid, result) return { - 'result': 'error', - 'dry_run': False, - 'message': f"Failed to {action} revision {revid}", - 'api_response': result + "result": "error", + "dry_run": False, + "message": f"Failed to {action} revision {revid}", + "api_response": result, } except Exception as e: - action_ing = 'unapproving' if unapprove else 'approving' + action_ing = "unapproving" if unapprove else "approving" logger.error("Error %s revision %s: %s", action_ing, revid, e) return { - 'result': 'error', - 'dry_run': False, - 'message': f"Error {action_ing} revision {revid}: {e}" + "result": "error", + "dry_run": False, + "message": f"Error {action_ing} revision {revid}: {e}", } def _get_page_title_from_revid(site, revid): """ Get the page title for a given revision ID. - + Args: site: Pywikibot site object revid (int): Revision ID - + Returns: str: Page title or None if not found """ try: - request = Request( - site=site, - action='query', - prop='revisions', - revids=revid, - rvprop='title' - ) + request = Request(site=site, action="query", prop="revisions", revids=revid, rvprop="title") result = request.submit() - if 'query' in result and 'pages' in result['query']: - for page_id, page_data in result['query']['pages'].items(): - if 'revisions' in page_data: - return page_data['title'] + if "query" in result and "pages" in result["query"]: + for page_id, page_data in result["query"]["pages"].items(): + if "revisions" in page_data: + return page_data["title"] return None From b863347cc894fcb8b87c76106dfdd3a1647d4299 Mon Sep 17 00:00:00 2001 From: AmbatI_Teja_Sri_Surya Date: Wed, 29 Oct 2025 16:09:19 -0500 Subject: [PATCH 12/26] style: wrap long line to satisfy E501 in test_pending_changes_review --- app/reviews/management/commands/test_pending_changes_review.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/reviews/management/commands/test_pending_changes_review.py b/app/reviews/management/commands/test_pending_changes_review.py index 7532f9d0..d8230a7e 100644 --- a/app/reviews/management/commands/test_pending_changes_review.py +++ b/app/reviews/management/commands/test_pending_changes_review.py @@ -44,8 +44,9 @@ def handle(self, *args, **options): dry_run = options["dry_run"] # Display current configuration + current_dry_run = getattr(settings, "PENDING_CHANGES_DRY_RUN", True) self.stdout.write( - f"Current PENDING_CHANGES_DRY_RUN setting: {getattr(settings, 'PENDING_CHANGES_DRY_RUN', True)}" + f"Current PENDING_CHANGES_DRY_RUN setting: {current_dry_run}" ) if dry_run: From a7ef7853a555578c9d99d9a17c52122a2eba56b9 Mon Sep 17 00:00:00 2001 From: AmbatI_Teja_Sri_Surya Date: Wed, 29 Oct 2025 16:11:21 -0500 Subject: [PATCH 13/26] style: apply ruff formatting to test_pending_changes_review.py --- .../management/commands/test_pending_changes_review.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/reviews/management/commands/test_pending_changes_review.py b/app/reviews/management/commands/test_pending_changes_review.py index d8230a7e..bbb14d6e 100644 --- a/app/reviews/management/commands/test_pending_changes_review.py +++ b/app/reviews/management/commands/test_pending_changes_review.py @@ -45,9 +45,7 @@ def handle(self, *args, **options): # Display current configuration current_dry_run = getattr(settings, "PENDING_CHANGES_DRY_RUN", True) - self.stdout.write( - f"Current PENDING_CHANGES_DRY_RUN setting: {current_dry_run}" - ) + self.stdout.write(f"Current PENDING_CHANGES_DRY_RUN setting: {current_dry_run}") if dry_run: self.stdout.write(self.style.WARNING("DRY-RUN MODE: No actual changes will be made")) From 060aaa3a5a8db0b349424f8079abffe3268cf311 Mon Sep 17 00:00:00 2001 From: AmbatI_Teja_Sri_Surya Date: Wed, 29 Oct 2025 16:14:48 -0500 Subject: [PATCH 14/26] test-compat: re-export revert detection helpers and SupersetQuery; add wrapper _check_revert_detection --- app/reviews/autoreview/__init__.py | 31 ++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/app/reviews/autoreview/__init__.py b/app/reviews/autoreview/__init__.py index 9d48db4f..6bc831b0 100644 --- a/app/reviews/autoreview/__init__.py +++ b/app/reviews/autoreview/__init__.py @@ -1 +1,32 @@ from __future__ import annotations + +# Backwards-compatibility exports for older test imports +from pywikibot.data.superset import SupersetQuery # re-export for tests + +from .checks.revert_detection import ( + _find_reviewed_revisions_by_sha1, + _parse_revert_params, + check_revert_detection, +) +from .context import CheckContext + + +def _check_revert_detection(revision, client): + """Compatibility wrapper matching legacy signature used in tests.""" + context = CheckContext( + revision=revision, + client=client, + profile=None, + auto_groups={}, + blocking_categories={}, + redirect_aliases=[], + ) + return check_revert_detection(context) + + +__all__ = [ + "SupersetQuery", + "_check_revert_detection", + "_find_reviewed_revisions_by_sha1", + "_parse_revert_params", +] \ No newline at end of file From fc7b0be1158a7cfbfd807d326401a1999d3f31d0 Mon Sep 17 00:00:00 2001 From: AmbatI_Teja_Sri_Surya Date: Wed, 29 Oct 2025 16:21:56 -0500 Subject: [PATCH 15/26] style: add trailing newline (fix W292) --- app/reviews/autoreview/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/reviews/autoreview/__init__.py b/app/reviews/autoreview/__init__.py index 6bc831b0..477a675d 100644 --- a/app/reviews/autoreview/__init__.py +++ b/app/reviews/autoreview/__init__.py @@ -29,4 +29,4 @@ def _check_revert_detection(revision, client): "_check_revert_detection", "_find_reviewed_revisions_by_sha1", "_parse_revert_params", -] \ No newline at end of file +] From 0dc21b64df6c776d1885511341eef436e6a6bb03 Mon Sep 17 00:00:00 2001 From: AmbatI_Teja_Sri_Surya Date: Wed, 29 Oct 2025 16:24:12 -0500 Subject: [PATCH 16/26] fix: import CheckContext from reviews.autoreview.context to resolve ImportError in CI --- app/reviews/autoreview/checks/revert_detection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/reviews/autoreview/checks/revert_detection.py b/app/reviews/autoreview/checks/revert_detection.py index 34ecafdc..908aab0e 100644 --- a/app/reviews/autoreview/checks/revert_detection.py +++ b/app/reviews/autoreview/checks/revert_detection.py @@ -11,7 +11,7 @@ from django.conf import settings -from ..utils.ores import CheckContext +from ..context import CheckContext logger = logging.getLogger(__name__) From 4997d34da84b274b5e4ba1a5dd1068a1e414324f Mon Sep 17 00:00:00 2001 From: AmbatI_Teja_Sri_Surya Date: Wed, 29 Oct 2025 18:24:57 -0500 Subject: [PATCH 17/26] tests: stop passing non-model field change_tag_params to PendingRevision; attach dynamically --- app/reviews/tests/test_revert_detection.py | 42 +++++++++++----------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/app/reviews/tests/test_revert_detection.py b/app/reviews/tests/test_revert_detection.py index 30f0517f..bbd05005 100644 --- a/app/reviews/tests/test_revert_detection.py +++ b/app/reviews/tests/test_revert_detection.py @@ -46,17 +46,18 @@ def setUp(self): user_name="TestUser", user_id=1000, change_tags=["mw-manual-revert"], - change_tag_params=[ - json.dumps( - { - "revertId": 200, - "oldestRevertedRevId": 180, - "newestRevertedRevId": 190, - "originalRevisionId": 175, - } - ) - ], ) + # Not a model field; attach dynamically for the check logic + self.revision.change_tag_params = [ + json.dumps( + { + "revertId": 200, + "oldestRevertedRevId": 180, + "newestRevertedRevId": 190, + "originalRevisionId": 175, + } + ) + ] self.client = Mock(spec=WikiClient) self.client.site = Mock() @@ -212,17 +213,18 @@ def test_revert_detection_with_real_revision(self): user_name="TestUser", user_id=1000, change_tags=["mw-manual-revert", "mw-reverted"], - change_tag_params=[ - json.dumps( - { - "revertId": 200, - "oldestRevertedRevId": 180, - "newestRevertedRevId": 190, - "originalRevisionId": 175, - } - ) - ], ) + # Attach dynamic params expected by the check logic + revision.change_tag_params = [ + json.dumps( + { + "revertId": 200, + "oldestRevertedRevId": 180, + "newestRevertedRevId": 190, + "originalRevisionId": 175, + } + ) + ] # Mock the client client = Mock(spec=WikiClient) From ccc8d0c86fb3dbc0ec630ff9e77efe45ed6600b9 Mon Sep 17 00:00:00 2001 From: AmbatI_Teja_Sri_Surya Date: Wed, 29 Oct 2025 18:28:20 -0500 Subject: [PATCH 18/26] tests: supply required PendingRevision fields (timestamp, age_at_fetch, sha1, wikitext) --- app/reviews/tests/test_revert_detection.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/app/reviews/tests/test_revert_detection.py b/app/reviews/tests/test_revert_detection.py index bbd05005..13c7fd8a 100644 --- a/app/reviews/tests/test_revert_detection.py +++ b/app/reviews/tests/test_revert_detection.py @@ -6,9 +6,11 @@ """ import json +from datetime import timedelta from unittest.mock import Mock, patch from django.test import TestCase +from django.utils import timezone from reviews.autoreview import ( _check_revert_detection, @@ -46,6 +48,10 @@ def setUp(self): user_name="TestUser", user_id=1000, change_tags=["mw-manual-revert"], + timestamp=timezone.now(), + age_at_fetch=timedelta(seconds=0), + sha1=("0" * 40), + wikitext="Test content", ) # Not a model field; attach dynamically for the check logic self.revision.change_tag_params = [ @@ -213,6 +219,10 @@ def test_revert_detection_with_real_revision(self): user_name="TestUser", user_id=1000, change_tags=["mw-manual-revert", "mw-reverted"], + timestamp=timezone.now(), + age_at_fetch=timedelta(seconds=0), + sha1=("0" * 40), + wikitext="Test content", ) # Attach dynamic params expected by the check logic revision.change_tag_params = [ From cbe2f8ee50d6547cfcce5fac759e5f0a0f29bb4b Mon Sep 17 00:00:00 2001 From: AmbatI_Teja_Sri_Surya Date: Wed, 29 Oct 2025 18:31:16 -0500 Subject: [PATCH 19/26] tests: make SupersetQuery import patchable by tests (import from reviews.autoreview) --- app/reviews/autoreview/checks/revert_detection.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/reviews/autoreview/checks/revert_detection.py b/app/reviews/autoreview/checks/revert_detection.py index 908aab0e..4c42b5ec 100644 --- a/app/reviews/autoreview/checks/revert_detection.py +++ b/app/reviews/autoreview/checks/revert_detection.py @@ -10,6 +10,7 @@ from typing import Any from django.conf import settings +from .. import SupersetQuery from ..context import CheckContext @@ -163,9 +164,7 @@ def _find_reviewed_revisions_by_sha1(client, page, reverted_rev_ids: list[int]) " rev_page, content_sha1\n" ) - # Execute query using SupersetQuery - from pywikibot.data.superset import SupersetQuery - + # Execute query using SupersetQuery (imported from reviews.autoreview for test patching) superset = SupersetQuery(site=client.site) results = superset.query(sql_query) From a801a4e55d26ba75f72eeb518d98a3aade115b7e Mon Sep 17 00:00:00 2001 From: AmbatI_Teja_Sri_Surya Date: Wed, 29 Oct 2025 21:16:39 -0500 Subject: [PATCH 20/26] style: sort imports in revert_detection to satisfy I001 --- app/reviews/autoreview/checks/revert_detection.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/reviews/autoreview/checks/revert_detection.py b/app/reviews/autoreview/checks/revert_detection.py index 4c42b5ec..427627ed 100644 --- a/app/reviews/autoreview/checks/revert_detection.py +++ b/app/reviews/autoreview/checks/revert_detection.py @@ -10,9 +10,8 @@ from typing import Any from django.conf import settings -from .. import SupersetQuery - from ..context import CheckContext +from .. import SupersetQuery logger = logging.getLogger(__name__) From 95995bb1e0f636dba90a67996b32653a095ddb12 Mon Sep 17 00:00:00 2001 From: AmbatI_Teja_Sri_Surya Date: Wed, 29 Oct 2025 21:20:05 -0500 Subject: [PATCH 21/26] style: ruff import sort in revert_detection (fix I001) --- app/reviews/autoreview/checks/revert_detection.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/reviews/autoreview/checks/revert_detection.py b/app/reviews/autoreview/checks/revert_detection.py index 427627ed..600db5af 100644 --- a/app/reviews/autoreview/checks/revert_detection.py +++ b/app/reviews/autoreview/checks/revert_detection.py @@ -10,8 +10,9 @@ from typing import Any from django.conf import settings -from ..context import CheckContext + from .. import SupersetQuery +from ..context import CheckContext logger = logging.getLogger(__name__) From ceb62b139350bc59c805b311a04eb33f8c045af0 Mon Sep 17 00:00:00 2001 From: AmbatI_Teja_Sri_Surya Date: Wed, 29 Oct 2025 21:22:15 -0500 Subject: [PATCH 22/26] style: reorder local imports to satisfy I001 (context before package import) --- app/reviews/autoreview/checks/revert_detection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/reviews/autoreview/checks/revert_detection.py b/app/reviews/autoreview/checks/revert_detection.py index 600db5af..f354f4d4 100644 --- a/app/reviews/autoreview/checks/revert_detection.py +++ b/app/reviews/autoreview/checks/revert_detection.py @@ -11,8 +11,8 @@ from django.conf import settings -from .. import SupersetQuery from ..context import CheckContext +from .. import SupersetQuery logger = logging.getLogger(__name__) From f873d455d185444aac65f946538863f961f56d7e Mon Sep 17 00:00:00 2001 From: AmbatI_Teja_Sri_Surya Date: Wed, 29 Oct 2025 21:28:02 -0500 Subject: [PATCH 23/26] style: sort imports per ruff (I001) --- app/reviews/autoreview/checks/revert_detection.py | 2 +- ruff_errors.txt | Bin 0 -> 546 bytes 2 files changed, 1 insertion(+), 1 deletion(-) create mode 100644 ruff_errors.txt diff --git a/app/reviews/autoreview/checks/revert_detection.py b/app/reviews/autoreview/checks/revert_detection.py index f354f4d4..600db5af 100644 --- a/app/reviews/autoreview/checks/revert_detection.py +++ b/app/reviews/autoreview/checks/revert_detection.py @@ -11,8 +11,8 @@ from django.conf import settings -from ..context import CheckContext from .. import SupersetQuery +from ..context import CheckContext logger = logging.getLogger(__name__) diff --git a/ruff_errors.txt b/ruff_errors.txt new file mode 100644 index 0000000000000000000000000000000000000000..bead77debd39840957f76d3c1214fc1ef08fb6bf GIT binary patch literal 546 zcmcJMK}*9x5QX2l;D6X-3s%gflz&CAUTXR<%AG^si7p(NpCh0P>F_uu>KTz)1do3!ThD6zC!f(q7aHjs1H5KO literal 0 HcmV?d00001 From d793230d095c367748a702871f3fc625f87a1b8c Mon Sep 17 00:00:00 2001 From: AmbatI_Teja_Sri_Surya Date: Wed, 29 Oct 2025 21:34:09 -0500 Subject: [PATCH 24/26] chore: remove accidentally committed ruff_errors.txt --- ruff_errors.txt | Bin 546 -> 0 bytes 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 ruff_errors.txt diff --git a/ruff_errors.txt b/ruff_errors.txt deleted file mode 100644 index bead77debd39840957f76d3c1214fc1ef08fb6bf..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 546 zcmcJMK}*9x5QX2l;D6X-3s%gflz&CAUTXR<%AG^si7p(NpCh0P>F_uu>KTz)1do3!ThD6zC!f(q7aHjs1H5KO From 9ea00c846be30606bca535057f6148d0cc59de40 Mon Sep 17 00:00:00 2001 From: AmbatI_Teja_Sri_Surya Date: Wed, 29 Oct 2025 21:36:56 -0500 Subject: [PATCH 25/26] tests: route calls through reviews.autoreview for patching (SupersetQuery and _find_reviewed_revisions_by_sha1) --- .../autoreview/checks/revert_detection.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/app/reviews/autoreview/checks/revert_detection.py b/app/reviews/autoreview/checks/revert_detection.py index 600db5af..0437745b 100644 --- a/app/reviews/autoreview/checks/revert_detection.py +++ b/app/reviews/autoreview/checks/revert_detection.py @@ -1,3 +1,5 @@ +from __future__ import annotations + """ Revert detection check for already-reviewed edits. @@ -9,9 +11,9 @@ import logging from typing import Any +import reviews.autoreview as autoreview from django.conf import settings -from .. import SupersetQuery from ..context import CheckContext logger = logging.getLogger(__name__) @@ -55,7 +57,9 @@ def check_revert_detection(context: CheckContext) -> dict[str, Any]: } # Check if any of the reverted revisions were previously reviewed - reviewed_revisions = _find_reviewed_revisions_by_sha1(context.client, page, reverted_rev_ids) + reviewed_revisions = autoreview._find_reviewed_revisions_by_sha1( + context.client, page, reverted_rev_ids + ) if reviewed_revisions: return { @@ -96,7 +100,7 @@ def _parse_revert_params(revision) -> list[int]: if not change_tag_params: return [] - reverted_ids = [] + reverted_ids: list[int] = [] for param_str in change_tag_params: try: @@ -164,12 +168,12 @@ def _find_reviewed_revisions_by_sha1(client, page, reverted_rev_ids: list[int]) " rev_page, content_sha1\n" ) - # Execute query using SupersetQuery (imported from reviews.autoreview for test patching) - superset = SupersetQuery(site=client.site) + # Execute query using SupersetQuery (resolved through package for test patching) + superset = autoreview.SupersetQuery(site=client.site) results = superset.query(sql_query) # Filter results where content was previously reviewed - reviewed_revisions = [] + reviewed_revisions: list[dict] = [] for result in results: if result.get("max_old_reviewed_id") is not None: reviewed_revisions.append( From 552e9b537880a5357bd58cc7e5bbfd6aab85571a Mon Sep 17 00:00:00 2001 From: AmbatI_Teja_Sri_Surya Date: Fri, 31 Oct 2025 20:46:22 -0500 Subject: [PATCH 26/26] fix: Move imports to top of file to resolve E402 errors - Move all imports above module docstring in revert_detection.py - Module docstring should be after imports, not before - Satisfies E402 (module level import not at top of file) Fixes CI lint failures in PR #107 --- .../autoreview/checks/revert_detection.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/app/reviews/autoreview/checks/revert_detection.py b/app/reviews/autoreview/checks/revert_detection.py index 0437745b..3cef6c3b 100644 --- a/app/reviews/autoreview/checks/revert_detection.py +++ b/app/reviews/autoreview/checks/revert_detection.py @@ -1,21 +1,22 @@ from __future__ import annotations -""" -Revert detection check for already-reviewed edits. - -This check detects when a pending edit is a revert to previously reviewed content -by matching SHA1 content hashes and checking for revert tags. -""" - import json import logging from typing import Any -import reviews.autoreview as autoreview from django.conf import settings +import reviews.autoreview as autoreview + from ..context import CheckContext +""" +Revert detection check for already-reviewed edits. + +This check detects when a pending edit is a revert to previously reviewed content +by matching SHA1 content hashes and checking for revert tags. +""" + logger = logging.getLogger(__name__)