diff --git a/.flake8 b/.flake8 index ce40cf1c..d8159a74 100644 --- a/.flake8 +++ b/.flake8 @@ -1,6 +1,6 @@ [flake8] application-import-names = api,app,authentication,blossom,blossom_wiki,engineeringblog,ocr,payments,utils,website -ignore = ANN101, D100, D101, D105, D106, W503, E203 +ignore = ANN101, ANN401, D100, D101, D105, D106, W503, E203 import-order-style = pycharm max-complexity = 10 max-line-length = 90 diff --git a/api/migrations/0007_source_id.py b/api/migrations/0007_source_id.py index 932ee460..d029b2b9 100644 --- a/api/migrations/0007_source_id.py +++ b/api/migrations/0007_source_id.py @@ -11,6 +11,8 @@ class Migration(migrations.Migration): operations = [ migrations.AddField( - model_name="source", name="id", field=models.IntegerField(default=0), + model_name="source", + name="id", + field=models.IntegerField(default=0), ), ] diff --git a/api/migrations/0008_remove_source_id.py b/api/migrations/0008_remove_source_id.py index 3bcec24e..a7f229f6 100644 --- a/api/migrations/0008_remove_source_id.py +++ b/api/migrations/0008_remove_source_id.py @@ -10,5 +10,8 @@ class Migration(migrations.Migration): ] operations = [ - migrations.RemoveField(model_name="source", name="id",), + migrations.RemoveField( + model_name="source", + name="id", + ), ] diff --git a/api/migrations/0010_auto_20200831_0335.py b/api/migrations/0010_auto_20200831_0335.py index e4bd8057..1dde3d54 100644 --- a/api/migrations/0010_auto_20200831_0335.py +++ b/api/migrations/0010_auto_20200831_0335.py @@ -11,7 +11,12 @@ class Migration(migrations.Migration): operations = [ migrations.RenameField( - model_name="submission", old_name="image_url", new_name="content_url", + model_name="submission", + old_name="image_url", + new_name="content_url", + ), + migrations.RemoveField( + model_name="submission", + name="is_image", ), - migrations.RemoveField(model_name="submission", name="is_image",), ] diff --git a/api/migrations/0025_accountmigration.py b/api/migrations/0025_accountmigration.py new file mode 100644 index 00000000..bba5662f --- /dev/null +++ b/api/migrations/0025_accountmigration.py @@ -0,0 +1,77 @@ +# Generated by Django 3.2.14 on 2022-07-18 00:07 + +import django.db.models.deletion +import django.utils.timezone +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ("api", "0024_alter_transcriptioncheck_status"), + ] + + operations = [ + migrations.CreateModel( + name="AccountMigration", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "create_time", + models.DateTimeField(default=django.utils.timezone.now), + ), + ( + "slack_channel_id", + models.CharField( + blank=True, default=None, max_length=50, null=True + ), + ), + ( + "slack_message_ts", + models.CharField( + blank=True, default=None, max_length=50, null=True + ), + ), + ("affected_submissions", models.ManyToManyField(to="api.Submission")), + ( + "moderator", + models.ForeignKey( + default=None, + null=True, + on_delete=django.db.models.deletion.SET_DEFAULT, + to=settings.AUTH_USER_MODEL, + ), + ), + ( + "new_user", + models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="new_user", + to=settings.AUTH_USER_MODEL, + ), + ), + ( + "old_user", + models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="old_user", + to=settings.AUTH_USER_MODEL, + ), + ), + ], + ), + ] diff --git a/api/models.py b/api/models.py index b39ea1c4..fd47da53 100644 --- a/api/models.py +++ b/api/models.py @@ -376,7 +376,10 @@ class TranscriptionCheckStatus(models.TextChoices): # An internal note for the moderators internal_note = models.CharField( - max_length=1000, null=True, blank=True, default=None, + max_length=1000, + null=True, + blank=True, + default=None, ) # The time the check has been created. @@ -400,10 +403,64 @@ def get_slack_url(self) -> Optional[str]: return None url_response = client.chat_getPermalink( - channel=self.slack_channel_id, message_ts=self.slack_message_ts, + channel=self.slack_channel_id, + message_ts=self.slack_message_ts, ) if not url_response.get("ok"): return None return url_response.get("permalink") + + +class AccountMigration(models.Model): + create_time = models.DateTimeField(default=timezone.now) + old_user = models.ForeignKey( + "authentication.BlossomUser", + on_delete=models.SET_NULL, + related_name="old_user", + null=True, + blank=True, + ) + new_user = models.ForeignKey( + "authentication.BlossomUser", + on_delete=models.SET_NULL, + related_name="new_user", + null=True, + blank=True, + ) + # keep track of submissions that were modified in case we need to roll back + affected_submissions = models.ManyToManyField(Submission) + # who has approved this migration? + moderator = models.ForeignKey( + "authentication.BlossomUser", + default=None, + null=True, + on_delete=models.SET_DEFAULT, + ) + # The info needed to update the Slack message of the check + slack_channel_id = models.CharField( + max_length=50, default=None, null=True, blank=True + ) + slack_message_ts = models.CharField( + max_length=50, default=None, null=True, blank=True + ) + + def perform_migration(self) -> None: + """Move all submissions attributed to one account to another.""" + existing_submissions = Submission.objects.filter(completed_by=self.old_user) + self.affected_submissions.add(*existing_submissions) + + existing_submissions.update( + claimed_by=self.new_user, completed_by=self.new_user + ) + + def get_number_affected(self) -> int: + """Get the number of records that have been modified.""" + return self.affected_submissions.count() + + def revert(self) -> None: + """Undo the account migration.""" + self.affected_submissions.update( + claimed_by=self.old_user, completed_by=self.old_user + ) diff --git a/api/slack/actions.py b/api/slack/actions.py index 87f90b15..21e8de7e 100644 --- a/api/slack/actions.py +++ b/api/slack/actions.py @@ -11,6 +11,7 @@ from api.models import Submission from api.slack import client +from api.slack.commands.migrate_user import process_migrate_user from api.slack.transcription_check.actions import process_check_action from app.reddit_actions import approve_post, remove_post from blossom.strings import translation @@ -34,8 +35,12 @@ def process_action(data: Dict) -> None: value: str = data["actions"][0].get("value") if value.startswith("check"): process_check_action(data) - elif "approve" in value or "remove" in value: + elif "submission" in value: + # buttons related to approving or removing submissions on the app and on Reddit process_submission_report_update(data) + elif "migration" in value: + # buttons related to account gamma migrations + process_migrate_user(data) else: client.chat_postMessage( channel=data["channel"]["id"], @@ -151,7 +156,8 @@ def process_submission_report_update(data: dict) -> None: # Find the submission corresponding to the message submissions = Submission.objects.filter( - report_slack_channel_id=channel_id, report_slack_message_ts=message_ts, + report_slack_channel_id=channel_id, + report_slack_message_ts=message_ts, ) if len(submissions) == 0: diff --git a/api/slack/commands/__init__.py b/api/slack/commands/__init__.py index 3661b1df..531f51f7 100644 --- a/api/slack/commands/__init__.py +++ b/api/slack/commands/__init__.py @@ -11,6 +11,7 @@ from api.slack.commands.dadjoke import dadjoke_cmd from api.slack.commands.help import help_cmd from api.slack.commands.info import info_cmd +from api.slack.commands.migrate_user import migrate_user_cmd from api.slack.commands.ping import ping_cmd from api.slack.commands.reset import reset_cmd from api.slack.commands.unwatch import unwatch_cmd @@ -58,6 +59,7 @@ def process_command(data: Dict) -> None: "dadjoke": dadjoke_cmd, "help": help_cmd, "info": info_cmd, + "migrate": migrate_user_cmd, "ping": ping_cmd, "reset": reset_cmd, "unwatch": unwatch_cmd, diff --git a/api/slack/commands/check.py b/api/slack/commands/check.py index ee7e4217..45ca2a6e 100644 --- a/api/slack/commands/check.py +++ b/api/slack/commands/check.py @@ -73,7 +73,8 @@ def check_cmd(channel: str, message: str) -> None: # Get the link for the check response = client.chat_getPermalink( - channel=check.slack_channel_id, message_ts=check.slack_message_ts, + channel=check.slack_channel_id, + message_ts=check.slack_message_ts, ) permalink = response.data.get("permalink") diff --git a/api/slack/commands/migrate_user.py b/api/slack/commands/migrate_user.py new file mode 100644 index 00000000..f4c3a8ba --- /dev/null +++ b/api/slack/commands/migrate_user.py @@ -0,0 +1,236 @@ +from copy import deepcopy + +from api.models import AccountMigration +from api.slack import client +from api.slack.transcription_check.messages import reply_to_action_with_ping +from api.slack.utils import get_reddit_username, parse_user +from authentication.models import BlossomUser +from blossom.strings import translation + +i18n = translation() + +HEADER_BLOCK = { + "type": "section", + "text": { + "type": "mrkdwn", + "text": ( + "Account migration requested from *{0}* to *{1}*." + " Verify that these account names are correct before proceeding!" + ), + }, +} +MOD_BLOCK = { + "type": "section", + "text": { + "type": "mrkdwn", + "text": "Approved by *u/{0}*.", + }, +} +CANCEL_BLOCK = { + "type": "section", + "text": { + "type": "plain_text", + "text": "Action cancelled.", + }, +} +FINAL_BLOCK = { + "type": "section", + "text": { + "type": "plain_text", + "text": ( + "This can no longer be acted upon. Please start the process from the" + " beginning if you wish to repeat this action." + ), + }, +} +DIVIDER_BLOCK = {"type": "divider"} +ACTION_BLOCK = {"type": "actions", "elements": []} +APPROVE_BUTTON = { + "type": "button", + "text": { + "type": "plain_text", + "text": "Approve", + }, + "style": "primary", + "confirm": { + "title": {"type": "plain_text", "text": "Are you sure?"}, + "text": { + "type": "mrkdwn", + "text": ( + "Make sure you've doublechecked the account names" + " before you approve this!" + ), + }, + "confirm": {"type": "plain_text", "text": "Do it"}, + "deny": {"type": "plain_text", "text": "Cancel"}, + }, + "value": "approve_migration_{}", +} +CANCEL_BUTTON = { + "type": "button", + "text": { + "type": "plain_text", + "text": "Cancel", + }, + "value": "cancel_migration_{}", + "style": "danger", +} +REVERT_BUTTON = { + "type": "button", + "text": { + "type": "plain_text", + "text": "Revert", + }, + "confirm": { + "title": {"type": "plain_text", "text": "Are you sure?"}, + "text": { + "type": "mrkdwn", + "text": ( + "Make sure you've doublechecked the account names" + " before you approve this!" + ), + }, + "confirm": {"type": "plain_text", "text": "Do it"}, + "deny": {"type": "plain_text", "text": "Cancel"}, + }, + "value": "revert_migration_{}", +} + + +def _create_blocks( + migration: AccountMigration, + approve_cancel: bool = False, + revert: bool = False, + cancel: bool = False, + final: bool = False, +) -> list[dict]: + blocks = [] + header = deepcopy(HEADER_BLOCK) + header["text"]["text"] = HEADER_BLOCK["text"]["text"].format( + migration.old_user.username, migration.new_user.username + ) + blocks.append(header) + + if migration.moderator and revert: + # show who approved it while when we show the button to revert it + mod_block = deepcopy(MOD_BLOCK) + mod_block["text"]["text"] = MOD_BLOCK["text"]["text"].format( + migration.moderator.username + ) + blocks.append(mod_block) + + blocks.append(DIVIDER_BLOCK) + + action_block = deepcopy(ACTION_BLOCK) + + if approve_cancel: + approve_button = deepcopy(APPROVE_BUTTON) + approve_button["value"] = APPROVE_BUTTON["value"].format(migration.id) + cancel_button = deepcopy(CANCEL_BUTTON) + cancel_button["value"] = CANCEL_BUTTON["value"].format(migration.id) + action_block["elements"].append(approve_button) + action_block["elements"].append(cancel_button) + + if revert: + revert_button = deepcopy(REVERT_BUTTON) + revert_button["value"] = revert_button["value"].format(migration.id) + action_block["elements"].append(revert_button) + + if cancel: + blocks.append(CANCEL_BLOCK) + + if final: + blocks.append(FINAL_BLOCK) + + if len(action_block["elements"]) > 0: + # can't have an action block with zero elements. + blocks.append(action_block) + return blocks + + +def migrate_user_cmd(channel: str, message: str) -> None: + """Migrate all gamma from one user to another.""" + parsed_message = message.split() + blocks = None + msg = None + migration = None # appease linter + if len(parsed_message) < 3: + # Needs to have two usernames + msg = i18n["slack"]["errors"]["missing_multiple_usernames"] + elif len(parsed_message) == 3: + old_user, old_username = parse_user(parsed_message[1]) + new_user, new_username = parse_user(parsed_message[2]) + if not old_user: + msg = i18n["slack"]["errors"]["unknown_username"].format( + username=old_username + ) + if not new_user: + msg = i18n["slack"]["errors"]["unknown_username"].format( + username=new_username + ) + + if old_user and new_user: + migration = AccountMigration.objects.create( + old_user=old_user, new_user=new_user + ) + blocks = _create_blocks(migration, approve_cancel=True) + + else: + msg = i18n["slack"]["errors"]["too_many_params"] + + args = {"channel": channel} + if msg: + args |= {"text": msg} + if blocks: + args |= {"blocks": blocks} + + response = client.chat_postMessage(**args) + + if blocks: + migration.slack_channel_id = response["channel"] + migration.slack_message_ts = response["message"]["ts"] + migration.save() + + +def process_migrate_user(data: dict) -> None: + """Handle the button responses from Slack.""" + value = data["actions"][0].get("value") + parts = value.split("_") + action = parts[0] + migration_id = parts[2] + mod_username = get_reddit_username(client, data["user"]) + + migration = AccountMigration.objects.filter(id=migration_id).first() + mod = BlossomUser.objects.filter(username=mod_username).first() + + if migration is None: + reply_to_action_with_ping( + data, f"I couldn't find a check with ID {migration_id}!" + ) + return + if mod is None: + reply_to_action_with_ping( + data, + f"I couldn't find a mod with username u/{mod_username}.\n" + "Did you set your username on Slack?", + ) + return + + if not migration.moderator: + migration.moderator = mod + migration.save() + + if action == "approve": + migration.perform_migration() + blocks = _create_blocks(migration, revert=True) + elif action == "revert": + migration.revert() + blocks = _create_blocks(migration, final=True) # Show no buttons here. + else: + blocks = _create_blocks(migration, cancel=True) + + client.chat_update( + channel=migration.slack_channel_id, + ts=migration.slack_message_ts, + blocks=blocks, + ) diff --git a/api/slack/commands/warnings.py b/api/slack/commands/warnings.py index f87efc4a..188807c1 100644 --- a/api/slack/commands/warnings.py +++ b/api/slack/commands/warnings.py @@ -45,9 +45,9 @@ def _get_warning_checks(user: BlossomUser) -> List[TranscriptionCheck]: ) # Aggregate the warnings and sort them by the transcription date - warnings: List[TranscriptionCheck] = list(pending_warnings) + list( - resolved_warnings - ) + list(unfixed_warnings) + warnings: List[TranscriptionCheck] = ( + list(pending_warnings) + list(resolved_warnings) + list(unfixed_warnings) + ) warnings.sort(key=lambda ch: ch.transcription.create_time) return warnings diff --git a/api/slack/transcription_check/actions.py b/api/slack/transcription_check/actions.py index 756cd22c..0c647627 100644 --- a/api/slack/transcription_check/actions.py +++ b/api/slack/transcription_check/actions.py @@ -123,10 +123,8 @@ def process_check_action(data: Dict) -> None: return # If the action isn't a claim, the check must already be claimed if action != "claim" and check.moderator is None: - logger.warning(f"Check {check_id} has not been claimed yet!",) - reply_to_action_with_ping( - data, f"Check {check_id} has not been claimed yet!", - ) + logger.warning(f"Check {check_id} has not been claimed yet!") + reply_to_action_with_ping(data, f"Check {check_id} has not been claimed yet!") # Update to make sure the proper controls are shown update_check_message(check) return @@ -146,9 +144,10 @@ def process_check_action(data: Dict) -> None: # Try to update the DB model based on the action if not _update_db_model(check, mod, action): # Unknown action type - logger.warning(f"Action '{action}' is invalid for check {check_id}!",) + logger.warning(f"Action '{action}' is invalid for check {check_id}!") reply_to_action_with_ping( - data, f"Action '{action}' is invalid for check {check_id}!", + data, + f"Action '{action}' is invalid for check {check_id}!", ) return diff --git a/api/slack/transcription_check/messages.py b/api/slack/transcription_check/messages.py index 2de627f7..1042f01c 100644 --- a/api/slack/transcription_check/messages.py +++ b/api/slack/transcription_check/messages.py @@ -60,7 +60,7 @@ def update_check_message(check: TranscriptionCheck) -> None: blocks = construct_transcription_check_blocks(check) response = client.chat_update( - channel=check.slack_channel_id, ts=check.slack_message_ts, blocks=blocks, + channel=check.slack_channel_id, ts=check.slack_message_ts, blocks=blocks ) if not response["ok"]: logger.error(f"Could not update check {check.id} on Slack!") diff --git a/api/tests/slack/commands/test_account_migration.py b/api/tests/slack/commands/test_account_migration.py new file mode 100644 index 00000000..b3777f78 --- /dev/null +++ b/api/tests/slack/commands/test_account_migration.py @@ -0,0 +1,334 @@ +from unittest.mock import patch + +from api.models import AccountMigration, Submission +from api.slack.commands.migrate_user import ( + _create_blocks, + migrate_user_cmd, + process_migrate_user, +) +from blossom.strings import translation +from utils.test_helpers import create_submission, create_user + +i18n = translation() + + +def test_perform_migration() -> None: + """Verify that account migration works correctly.""" + user1 = create_user(id=100, username="Paddington") + user2 = create_user(id=200, username="Moddington") + + submission1 = create_submission(claimed_by=user1, completed_by=user1) + create_submission(claimed_by=user2, completed_by=user2) + + assert Submission.objects.filter(completed_by=user1).count() == 1 + assert Submission.objects.filter(completed_by=user2).count() == 1 + + migration = AccountMigration.objects.create(old_user=user1, new_user=user2) + migration.perform_migration() + assert migration.affected_submissions.count() == 1 + assert Submission.objects.filter(completed_by=user1).count() == 0 + assert Submission.objects.filter(completed_by=user2).count() == 2 + + submission1.refresh_from_db() + assert submission1.claimed_by == user2 + assert submission1.completed_by == user2 + + +def test_revert() -> None: + """Verify that reverting an account migration works.""" + user1 = create_user(id=100, username="Paddington") + user2 = create_user(id=200, username="Moddington") + + submission1 = create_submission(claimed_by=user1, completed_by=user1) + create_submission(claimed_by=user2, completed_by=user2) + + assert Submission.objects.filter(completed_by=user1).count() == 1 + assert Submission.objects.filter(completed_by=user2).count() == 1 + + migration = AccountMigration.objects.create(old_user=user1, new_user=user2) + migration.perform_migration() + + assert Submission.objects.filter(completed_by=user1).count() == 0 + assert Submission.objects.filter(completed_by=user2).count() == 2 + + migration.revert() + + assert Submission.objects.filter(completed_by=user1).count() == 1 + assert Submission.objects.filter(completed_by=user2).count() == 1 + + submission1.refresh_from_db() + assert submission1.claimed_by == user1 + assert submission1.completed_by == user1 + + +def test_create_blocks() -> None: + """Verify that blocks are created by default as expected.""" + user1 = create_user(id=100, username="Paddington") + user2 = create_user(id=200, username="Moddington") + migration = AccountMigration.objects.create(old_user=user1, new_user=user2) + + # no buttons requested + blocks = _create_blocks(migration) + # header and divider + assert len(blocks) == 2 + assert "Paddington" in blocks[0]["text"]["text"] + assert "Moddington" in blocks[0]["text"]["text"] + + +def test_create_blocks_with_revert_button() -> None: + """Verify that blocks are created with the revert button as expected.""" + user1 = create_user(id=100, username="Paddington") + user2 = create_user(id=200, username="Moddington") + migration = AccountMigration.objects.create(old_user=user1, new_user=user2) + + blocks = _create_blocks(migration, revert=True) + assert len(blocks) == 3 + assert len(blocks[2]["elements"]) == 1 + assert blocks[2]["elements"][0]["value"] == f"revert_migration_{migration.id}" + + +def test_create_blocks_with_approve_cancel_buttons() -> None: + """Verify that blocks are created with approve and cancel buttons.""" + user1 = create_user(id=100, username="Paddington") + user2 = create_user(id=200, username="Moddington") + migration = AccountMigration.objects.create(old_user=user1, new_user=user2) + + blocks = _create_blocks(migration, approve_cancel=True) + assert len(blocks) == 3 + assert len(blocks[2]["elements"]) == 2 + assert blocks[2]["elements"][0]["value"] == f"approve_migration_{migration.id}" + assert blocks[2]["elements"][1]["value"] == f"cancel_migration_{migration.id}" + + +def test_create_blocks_with_mod() -> None: + """Verify that the mod section is created appropriately.""" + user1 = create_user(id=100, username="Paddington") + user2 = create_user(id=200, username="Bear") + user3 = create_user(id=201, username="Mod Moddington") + migration = AccountMigration.objects.create( + old_user=user1, new_user=user2, moderator=user3 + ) + + blocks = _create_blocks(migration, revert=True) + assert len(blocks) == 4 + assert blocks[1] == { + "type": "section", + "text": { + "type": "mrkdwn", + "text": "Approved by *u/Mod Moddington*.", + }, + } + + +def test_migrate_user_cmd() -> None: + """Verify that the slack command for migration works as expected.""" + user1 = create_user(id=100, username="Paddington") + user2 = create_user(id=200, username="Moddington") + + assert AccountMigration.objects.count() == 0 + + with patch("api.slack.commands.migrate_user.client.chat_postMessage") as mock: + mock.return_value = {"channel": "AAA", "message": {"ts": 1234}} + migrate_user_cmd(channel="abc", message="migrate paddington moddington") + + mock.assert_called_once() + assert len(mock.call_args.kwargs["blocks"]) == 3 + assert mock.call_args.kwargs["channel"] == "abc" + + assert AccountMigration.objects.count() == 1 + migration = AccountMigration.objects.first() + assert migration.old_user == user1 + assert migration.new_user == user2 + assert migration.slack_channel_id == "AAA" + assert migration.slack_message_ts == "1234" + + +def test_migrate_user_cmd_missing_users() -> None: + """Verify error for missing users.""" + with patch("api.slack.commands.migrate_user.client.chat_postMessage") as mock: + migrate_user_cmd(channel="abc", message="migrate") + + # no blocks when there's text here + assert mock.call_args.kwargs.get("blocks") is None + assert ( + mock.call_args.kwargs["text"] + == i18n["slack"]["errors"]["missing_multiple_usernames"] + ) + assert mock.call_args.kwargs["channel"] == "abc" + + +def test_migrate_user_cmd_wrong_first_user() -> None: + """Verify error for missing first user.""" + create_user(id=100, username="Paddington") + with patch("api.slack.commands.migrate_user.client.chat_postMessage") as mock: + migrate_user_cmd(channel="abc", message="migrate AAAA Paddington") + + # no blocks when there's text here + assert mock.call_args.kwargs.get("blocks") is None + assert mock.call_args.kwargs["text"] == i18n["slack"]["errors"][ + "unknown_username" + ].format(username="AAAA") + assert mock.call_args.kwargs["channel"] == "abc" + + +def test_migrate_user_cmd_wrong_second_user() -> None: + """Verify error for missing second user.""" + create_user(id=100, username="Paddington") + with patch("api.slack.commands.migrate_user.client.chat_postMessage") as mock: + migrate_user_cmd(channel="abc", message="migrate Paddington BBBB") + + assert mock.call_args.kwargs.get("blocks") is None + assert mock.call_args.kwargs["text"] == i18n["slack"]["errors"][ + "unknown_username" + ].format(username="BBBB") + assert mock.call_args.kwargs["channel"] == "abc" + + +def test_migrate_user_cmd_too_many_users() -> None: + """Verify error for too many users.""" + with patch("api.slack.commands.migrate_user.client.chat_postMessage") as mock: + migrate_user_cmd(channel="abc", message="migrate A B C") + + assert mock.call_args.kwargs.get("blocks") is None + assert mock.call_args.kwargs["text"] == i18n["slack"]["errors"]["too_many_params"] + assert mock.call_args.kwargs["channel"] == "abc" + + +def test_process_migrate_user() -> None: + """Verify migration works when called via buttons.""" + user1 = create_user(id=100, username="Paddington") + user2 = create_user(id=200, username="Moddington") + + create_submission(claimed_by=user1, completed_by=user1) + create_submission(claimed_by=user2, completed_by=user2) + + assert Submission.objects.filter(completed_by=user1).count() == 1 + assert Submission.objects.filter(completed_by=user2).count() == 1 + + migration = AccountMigration.objects.create( + old_user=user1, new_user=user2, slack_message_ts=123, slack_channel_id="AAA" + ) + + with patch( + "api.slack.commands.migrate_user.get_reddit_username", + lambda _, us: us["name"], + ), patch( + "api.slack.commands.migrate_user.client.chat_update" + ) as message_mock, patch( + "api.slack.commands.migrate_user.reply_to_action_with_ping", + return_value={}, + ) as reply_mock: + process_migrate_user( + { + "actions": [{"value": f"approve_migration_{migration.id}"}], + "user": {"name": "Moddington"}, + } + ) + + assert Submission.objects.filter(completed_by=user1).count() == 0 + assert Submission.objects.filter(completed_by=user2).count() == 2 + + message_mock.assert_called_once() + # header, mod approved, divider, revert button + assert len(message_mock.call_args.kwargs["blocks"]) == 4 + + revert_button = message_mock.call_args.kwargs["blocks"][3]["elements"][0] + assert revert_button["value"] == f"revert_migration_{migration.id}" + + with patch( + "api.slack.commands.migrate_user.get_reddit_username", + lambda _, us: us["name"], + ), patch( + "api.slack.commands.migrate_user.client.chat_update" + ) as message_mock, patch( + "api.slack.commands.migrate_user.reply_to_action_with_ping", + return_value={}, + ) as reply_mock: + process_migrate_user( + { + "actions": [{"value": f"revert_migration_{migration.id}"}], + "user": {"name": "Moddington"}, + } + ) + + assert Submission.objects.filter(completed_by=user1).count() == 1 + assert Submission.objects.filter(completed_by=user2).count() == 1 + + # we've reverted -- no more buttons for you + assert len(message_mock.call_args.kwargs["blocks"]) == 3 + + with patch( + "api.slack.commands.migrate_user.get_reddit_username", + lambda _, us: us["name"], + ), patch( + "api.slack.commands.migrate_user.client.chat_update" + ) as message_mock, patch( + "api.slack.commands.migrate_user.reply_to_action_with_ping", + return_value={}, + ) as reply_mock: + process_migrate_user( + { + "actions": [{"value": f"cancel_migration_{migration.id}"}], + "user": {"name": "Moddington"}, + } + ) + + message_mock.assert_called_once() + assert ( + message_mock.call_args.kwargs["blocks"][-1]["text"]["text"] + == "Action cancelled." + ) + + reply_mock.assert_not_called() + + +def test_migrate_user_no_migration() -> None: + """Verify error for nonexistent migration.""" + create_user(id=200, username="Moddington") + + with patch( + "api.slack.commands.migrate_user.get_reddit_username", + lambda _, us: us["name"], + ), patch( + "api.slack.commands.migrate_user.client.chat_update" + ) as message_mock, patch( + "api.slack.commands.migrate_user.reply_to_action_with_ping", + return_value={}, + ) as reply_mock: + process_migrate_user( + { + "actions": [{"value": "approve_migration_1"}], + "user": {"name": "Moddington"}, + } + ) + + message_mock.assert_not_called() + reply_mock.assert_called_once() + assert reply_mock.call_args.args[-1] == "I couldn't find a check with ID 1!" + + +def test_migrate_user_wrong_username() -> None: + """Verify error for wrong mod username on Slack.""" + user1 = create_user(id=200, username="Moddington") + + migration = AccountMigration.objects.create(old_user=user1, new_user=user1) + + with patch( + "api.slack.commands.migrate_user.get_reddit_username", + lambda _, us: us["name"], + ), patch( + "api.slack.commands.migrate_user.client.chat_update" + ) as message_mock, patch( + "api.slack.commands.migrate_user.reply_to_action_with_ping", + return_value={}, + ) as reply_mock: + process_migrate_user( + { + "actions": [{"value": f"approve_migration_{migration.id}"}], + "user": {"name": "AA"}, + } + ) + + message_mock.assert_not_called() + reply_mock.assert_called_once() + assert "I couldn't find a mod with username u/AA." in reply_mock.call_args.args[-1] diff --git a/api/tests/slack/commands/test_dadjoke.py b/api/tests/slack/commands/test_dadjoke.py index 63d6b2a2..7ad99d79 100644 --- a/api/tests/slack/commands/test_dadjoke.py +++ b/api/tests/slack/commands/test_dadjoke.py @@ -8,9 +8,7 @@ i18n = translation() -@pytest.mark.parametrize( - "message", ["dadjoke", "dadjoke <@asdf>", "dadjoke a b c"], -) +@pytest.mark.parametrize("message", ["dadjoke", "dadjoke <@asdf>", "dadjoke a b c"]) def test_dadjoke_target(message: str) -> None: """Verify that dadjokes are delivered appropriately.""" with patch("api.slack.commands.dadjoke.client.chat_postMessage") as mock: diff --git a/api/tests/slack/commands/test_warnings.py b/api/tests/slack/commands/test_warnings.py index 58989fe9..389225c6 100644 --- a/api/tests/slack/commands/test_warnings.py +++ b/api/tests/slack/commands/test_warnings.py @@ -42,7 +42,8 @@ def test_warning_entry(client: Client) -> None: create_time=datetime(2022, 2, 3, 13, 2), ) check = create_check( - transcription=transcription, status=CheckStatus.WARNING_RESOLVED, + transcription=transcription, + status=CheckStatus.WARNING_RESOLVED, ) expected = i18n["slack"]["warnings"]["warning_entry"].format( @@ -76,7 +77,10 @@ def test_get_warning_checks(client: Client) -> None: ] for (ch_id, status) in check_properties: - submission = create_submission(claimed_by=user, completed_by=user,) + submission = create_submission( + claimed_by=user, + completed_by=user, + ) transcription = create_transcription( submission=submission, user=user, create_time=datetime(2022, 3, 2, ch_id) ) @@ -99,7 +103,10 @@ def test_warning_text_no_warnings(client: Client) -> None: ] for (ch_id, status) in check_properties: - submission = create_submission(claimed_by=user, completed_by=user,) + submission = create_submission( + claimed_by=user, + completed_by=user, + ) transcription = create_transcription( submission=submission, user=user, create_time=datetime(2022, 3, 2, ch_id) ) diff --git a/api/tests/slack/test_utils.py b/api/tests/slack/test_utils.py index c7042cd2..29b264ff 100644 --- a/api/tests/slack/test_utils.py +++ b/api/tests/slack/test_utils.py @@ -256,7 +256,10 @@ def users_profile_get(self, user: str): Submission(url="https://example.com", source=Source(name="blossom")), "blossom", ), - (Submission(url=None, source=Source(name="blossom")), "blossom",), + ( + Submission(url=None, source=Source(name="blossom")), + "blossom", + ), ], ) def test_get_source(submission: Submission, expected: str) -> None: diff --git a/api/tests/slack/transcription_check/test_actions.py b/api/tests/slack/transcription_check/test_actions.py index f71a42c5..960f4545 100644 --- a/api/tests/slack/transcription_check/test_actions.py +++ b/api/tests/slack/transcription_check/test_actions.py @@ -56,7 +56,10 @@ def _is_time_recent(start: datetime, time: Optional[datetime]) -> bool: ], ) def test_update_db_model_status( - client: Client, action: str, pre_status: CheckStatus, post_status: CheckStatus, + client: Client, + action: str, + pre_status: CheckStatus, + post_status: CheckStatus, ) -> None: """Test the updating of the status of the check DB model.""" client, headers, user = setup_user_client(client, id=100, username="Userson") @@ -135,7 +138,8 @@ def test_update_db_model_set_complete_time(client: Client, action: str) -> None: @pytest.mark.parametrize( - "action", ["pending", "comment-pending", "warning-pending"], + "action", + ["pending", "comment-pending", "warning-pending"], ) def test_update_db_model_unset_complete_time(client: Client, action: str) -> None: """Test that the complete time is updated after reverting completion.""" @@ -143,7 +147,11 @@ def test_update_db_model_unset_complete_time(client: Client, action: str) -> Non mod = create_user(id=200, username="Moddington") submission = create_submission(claimed_by=user, completed_by=user) transcription = create_transcription(submission=submission, user=user) - check = create_check(transcription, moderator=mod, complete_time=timezone.now(),) + check = create_check( + transcription, + moderator=mod, + complete_time=timezone.now(), + ) assert check.complete_time is not None assert _update_db_model(check, mod, action) diff --git a/api/tests/submissions/test_submission_creation.py b/api/tests/submissions/test_submission_creation.py index 184836fc..f09c4c2b 100644 --- a/api/tests/submissions/test_submission_creation.py +++ b/api/tests/submissions/test_submission_creation.py @@ -121,7 +121,11 @@ def test_create_no_id(self, client: Client) -> None: ], ) def test_ocr_on_create( - self, client: Client, settings: SettingsWrapper, test_input: str, output: str, + self, + client: Client, + settings: SettingsWrapper, + test_input: str, + output: str, ) -> None: """Verify that a new submission completes the OCR process.""" settings.ENABLE_OCR = True diff --git a/api/tests/submissions/test_submission_done.py b/api/tests/submissions/test_submission_done.py index 5a0ca70f..bf113ecd 100644 --- a/api/tests/submissions/test_submission_done.py +++ b/api/tests/submissions/test_submission_done.py @@ -129,10 +129,13 @@ def test_done_already_completed(self, client: Client) -> None: assert result.status_code == status.HTTP_409_CONFLICT @pytest.mark.parametrize( - "should_check_transcription", [True, False], + "should_check_transcription", + [True, False], ) def test_done_random_checks( - self, client: Client, should_check_transcription: bool, + self, + client: Client, + should_check_transcription: bool, ) -> None: """Test whether the random checks for the done process are invoked correctly.""" with patch( @@ -156,7 +159,8 @@ def test_done_random_checks( assert mock.call_count == 0 @pytest.mark.parametrize( - "gamma, expected", [(24, False), (25, True), (26, False)], + "gamma, expected", + [(24, False), (25, True), (26, False)], ) def test_check_for_rank_up( self, client: Client, gamma: int, expected: bool diff --git a/api/tests/submissions/test_submission_expired.py b/api/tests/submissions/test_submission_expired.py index 12ed6acd..50ac1314 100644 --- a/api/tests/submissions/test_submission_expired.py +++ b/api/tests/submissions/test_submission_expired.py @@ -122,7 +122,9 @@ def test_missing_source(self, client: Client) -> None: client, headers, user = setup_user_client(client) result = client.get( - reverse("submission-expired"), content_type="application/json", **headers, + reverse("submission-expired"), + content_type="application/json", + **headers, ) assert result.status_code == status.HTTP_400_BAD_REQUEST diff --git a/api/tests/submissions/test_submission_get.py b/api/tests/submissions/test_submission_get.py index 49d3604c..62f0bca3 100644 --- a/api/tests/submissions/test_submission_get.py +++ b/api/tests/submissions/test_submission_get.py @@ -288,13 +288,16 @@ def test_list_with_contains_filters( client, headers, user = setup_user_client(client, id=123) create_submission( - id=1, title="This is a title", + id=1, + title="This is a title", ) create_submission( - id=2, title="Another title", + id=2, + title="Another title", ) create_submission( - id=3, title="TITLE IS GOOD", + id=3, + title="TITLE IS GOOD", ) result = client.get( @@ -316,10 +319,12 @@ def test_list_with_removed_filter( client, headers, user = setup_user_client(client, id=123) create_submission( - id=1, removed_from_queue=True, + id=1, + removed_from_queue=True, ) create_submission( - id=2, removed_from_queue=False, + id=2, + removed_from_queue=False, ) result = client.get( diff --git a/api/tests/submissions/test_submission_in_progress.py b/api/tests/submissions/test_submission_in_progress.py index 427413d4..4e8bc2cb 100644 --- a/api/tests/submissions/test_submission_in_progress.py +++ b/api/tests/submissions/test_submission_in_progress.py @@ -28,7 +28,9 @@ def test_in_progress_recent_submissions(self, client: Client) -> None: client, headers, user = setup_user_client(client) reddit, _ = Source.objects.get_or_create(name="reddit") create_submission( - claimed_by=user, claim_time=timezone.now(), source=reddit, + claimed_by=user, + claim_time=timezone.now(), + source=reddit, ) result = client.get( diff --git a/api/tests/submissions/test_submission_leaderboard.py b/api/tests/submissions/test_submission_leaderboard.py index 0c05d637..79c33b83 100644 --- a/api/tests/submissions/test_submission_leaderboard.py +++ b/api/tests/submissions/test_submission_leaderboard.py @@ -36,7 +36,10 @@ class TestSubmissionLeaderboard: ], ) def test_top_leaderboard( - self, client: Client, data: List[UserData], expected: List[UserData], + self, + client: Client, + data: List[UserData], + expected: List[UserData], ) -> None: """Test if the top leaderboard is set up correctly.""" BlossomUser.objects.all().delete() @@ -45,7 +48,9 @@ def test_top_leaderboard( for obj in data: user_id = obj.get("id") cur_user = create_user( - id=user_id, username=f"user-{user_id}", is_volunteer=True, + id=user_id, + username=f"user-{user_id}", + is_volunteer=True, ) for _ in range(obj.get("gamma")): create_submission(completed_by=cur_user) @@ -120,7 +125,10 @@ def test_user_leaderboard( assert extract_ids(results["above"]) == expected_above assert extract_ids(results["below"]) == expected_below - def test_filtered_leaderboard(self, client: Client,) -> None: + def test_filtered_leaderboard( + self, + client: Client, + ) -> None: """Test if the submissions for the rate is calculated on are filtered.""" BlossomUser.objects.all().delete() date_joined = datetime(2021, 11, 3, tzinfo=pytz.UTC) diff --git a/api/tests/submissions/test_submission_rate.py b/api/tests/submissions/test_submission_rate.py index 3f864499..11117be8 100644 --- a/api/tests/submissions/test_submission_rate.py +++ b/api/tests/submissions/test_submission_rate.py @@ -259,7 +259,10 @@ def test_time_frames( response = result.json() assert response["results"] == results - def test_rate_filtering(self, client: Client,) -> None: + def test_rate_filtering( + self, + client: Client, + ) -> None: """Verify that filters can be applied to the submissions.""" client, headers, user = setup_user_client(client, id=123456) @@ -299,7 +302,10 @@ def test_rate_filtering(self, client: Client,) -> None: {"count": 3, "date": "2021-06-20T00:00:00Z"}, ] - def test_rate_timezones(self, client: Client,) -> None: + def test_rate_timezones( + self, + client: Client, + ) -> None: """Verify that the timezone is applied correctly, if specified.""" client, headers, user = setup_user_client(client, id=123456) diff --git a/api/tests/test_find.py b/api/tests/test_find.py index fd0f7191..4eea7268 100644 --- a/api/tests/test_find.py +++ b/api/tests/test_find.py @@ -82,7 +82,11 @@ def test_extract_core_url() -> None: # Submission URL ("https://reddit.com/r/antiwork/comments/q1tlcf/work_is_work/", "url", True), # ToR Submission Core URL - ("https://reddit.com/r/TranscribersOfReddit/comments/q1tnhc", "tor_url", True,), + ( + "https://reddit.com/r/TranscribersOfReddit/comments/q1tnhc", + "tor_url", + True, + ), # ToR Submission URL ( "https://reddit.com/r/TranscribersOfReddit/comments/q1tnhc/antiwork_image_work_is_work/", @@ -198,7 +202,9 @@ def test_find_in_progress(client: Client, url: str, expected: bool) -> None: ) result = client.get( - reverse("find") + f"?url={url}", content_type="application/json", **headers, + reverse("find") + f"?url={url}", + content_type="application/json", + **headers, ) if expected: @@ -299,7 +305,9 @@ def test_find_completed(client: Client, url: str, expected: bool) -> None: ) result = client.get( - reverse("find") + f"?url={url}", content_type="application/json", **headers, + reverse("find") + f"?url={url}", + content_type="application/json", + **headers, ) if expected: diff --git a/api/tests/test_misc.py b/api/tests/test_misc.py index 8e0240d5..d2569bc5 100644 --- a/api/tests/test_misc.py +++ b/api/tests/test_misc.py @@ -17,7 +17,10 @@ def test_ping(client: Client) -> None: """Test whether the ping request returns correctly.""" - result = client.get(reverse("ping"), content_type="application/json",) + result = client.get( + reverse("ping"), + content_type="application/json", + ) assert result.json() == {"ping?!": "PONG"} assert result.status_code == status.HTTP_200_OK @@ -26,7 +29,11 @@ def test_summary(client: Client) -> None: """Test whether the summary request provides a correctly formatted summary.""" client, headers, _ = setup_user_client(client) - result = client.get(reverse("summary"), content_type="application/json", **headers,) + result = client.get( + reverse("summary"), + content_type="application/json", + **headers, + ) assert "transcription_count" in result.json().keys() assert "days_since_inception" in result.json().keys() @@ -53,7 +60,11 @@ def test_active_volunteer_count(client: Client) -> None: create_submission(completed_by=user_2, complete_time=three_weeks_ago) create_submission(completed_by=user_3, complete_time=one_week_ago) - result = client.get(reverse("summary"), content_type="application/json", **headers,) + result = client.get( + reverse("summary"), + content_type="application/json", + **headers, + ) assert result.status_code == status.HTTP_200_OK assert result.json()["active_volunteer_count"] == 2 @@ -77,7 +88,11 @@ def test_active_volunteer_count_aggregation(client: Client) -> None: create_submission(completed_by=user_1, complete_time=three_weeks_ago) create_submission(completed_by=user_2, complete_time=two_days_ago) - result = client.get(reverse("summary"), content_type="application/json", **headers,) + result = client.get( + reverse("summary"), + content_type="application/json", + **headers, + ) assert result.status_code == status.HTTP_200_OK assert result.json()["active_volunteer_count"] == 2 diff --git a/api/tests/test_transcriptions.py b/api/tests/test_transcriptions.py index 3a014da4..866b8245 100644 --- a/api/tests/test_transcriptions.py +++ b/api/tests/test_transcriptions.py @@ -142,13 +142,22 @@ def test_list_with_contains_filters( submission = create_submission(id=1) create_transcription( - submission, user, id=2, text="This is a very interesting text and such.", + submission, + user, + id=2, + text="This is a very interesting text and such.", ) create_transcription( - submission, user, id=3, text="A text is a form of literature.", + submission, + user, + id=3, + text="A text is a form of literature.", ) create_transcription( - submission, user, id=4, text="Bla bla bla bla.", + submission, + user, + id=4, + text="Bla bla bla bla.", ) result = client.get( diff --git a/api/views/find.py b/api/views/find.py index 7d3d1a46..680f5844 100644 --- a/api/views/find.py +++ b/api/views/find.py @@ -143,7 +143,10 @@ def get(self, request: Request) -> Response: url = request.query_params.get("url") normalized_url = normalize_url(url) if normalized_url is None: - return Response(data="Invalid URL.", status=status.HTTP_400_BAD_REQUEST,) + return Response( + data="Invalid URL.", + status=status.HTTP_400_BAD_REQUEST, + ) data = find_by_url(normalized_url) if data is None: diff --git a/api/views/misc.py b/api/views/misc.py index 0466946d..2ab780ab 100644 --- a/api/views/misc.py +++ b/api/views/misc.py @@ -42,7 +42,8 @@ def generate_summary() -> Dict: is_volunteer=True, is_bot=False ).count(), "active_volunteer_count": Submission.objects.filter( - completed_by__isnull=False, complete_time__gte=date_minus_two_weeks, + completed_by__isnull=False, + complete_time__gte=date_minus_two_weeks, ) .values("completed_by") .distinct() diff --git a/api/views/submission.py b/api/views/submission.py index 5e9e00ce..eacbcde5 100644 --- a/api/views/submission.py +++ b/api/views/submission.py @@ -790,7 +790,10 @@ def bulkcheck(self, request: Request) -> Response: responses={404: "No volunteer with the specified ID."}, ) @action(detail=False, methods=["get"]) - def leaderboard(self, request: Request,) -> Response: + def leaderboard( + self, + request: Request, + ) -> Response: """Get the leaderboard for the given user.""" user_id = request.GET.get("user_id", None) if user_id is not None: diff --git a/blossom/strings/en_US.toml b/blossom/strings/en_US.toml index 5de139a3..9dcea1fc 100644 --- a/blossom/strings/en_US.toml +++ b/blossom/strings/en_US.toml @@ -65,6 +65,7 @@ message_parse_error="Sorry, something went wrong and I couldn't parse your messa empty_message_error="Sorry, I wasn't able to get text out of that. Try again." too_many_params="Too many parameters; please try again." missing_username="I don't see a username in your request -- make sure you're formatting your request as \"@Blossom {action} {argument}\". Example: \"@Blossom dadjoke @itsthejoker\"" +missing_multiple_usernames="This command uses multiple usernames -- make sure you're formatting your command as \"@Blossom {action} {argument} {argument} ...\"." unknown_username="Sorry, I couldn't find a user with the name `u/{username}`. Please check your spelling." unknown_payload="Received unknown payload from Slack with key I don't recognize. Unknown key: {}" diff --git a/pyproject.toml b/pyproject.toml index 7723bd35..ca830675 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "blossom" -version = "1.75.0" +version = "1.77.0" description = "The site!" authors = ["Grafeas Group Ltd. "]