diff --git a/forum/backends/mysql/api.py b/forum/backends/mysql/api.py index 1e09f64a..93591ac8 100644 --- a/forum/backends/mysql/api.py +++ b/forum/backends/mysql/api.py @@ -1223,12 +1223,16 @@ def retire_all_content( comments = Comment.objects.filter(author__pk=user_id) for comment in comments: comment.body = RETIRED_BODY + comment.retired_username = username + comment.author_username = username comment.save() comment_threads = CommentThread.objects.filter(author__pk=user_id) for comment_thread in comment_threads: comment_thread.body = RETIRED_BODY comment_thread.title = RETIRED_TITLE + comment_thread.retired_username = username + comment_thread.author_username = username comment_thread.save() @staticmethod @@ -1949,6 +1953,10 @@ def replace_username_in_all_content(user_id: str, username: str) -> None: user = User.objects.get(pk=user_id) user.username = username user.save() + + # Update author_username in all content + Comment.objects.filter(author=user).update(author_username=username) + CommentThread.objects.filter(author=user).update(author_username=username) except User.DoesNotExist as exc: raise ValueError("User does not exist") from exc diff --git a/forum/backends/mysql/models.py b/forum/backends/mysql/models.py index d7cfe6ec..3924a0d6 100644 --- a/forum/backends/mysql/models.py +++ b/forum/backends/mysql/models.py @@ -96,6 +96,18 @@ class Content(models.Model): author: models.ForeignKey[User, User] = models.ForeignKey( User, on_delete=models.CASCADE ) + author_username: models.CharField[Optional[str], str] = models.CharField( + max_length=255, + null=True, + blank=True, + help_text="Username at time of posting, preserved for historical accuracy", + ) + retired_username: models.CharField[Optional[str], str] = models.CharField( + max_length=255, + null=True, + blank=True, + help_text="Username to display if author account was retired", + ) course_id: models.CharField[str, str] = models.CharField(max_length=255) body: models.TextField[str, str] = models.TextField() visible: models.BooleanField[bool, bool] = models.BooleanField(default=True) @@ -184,6 +196,16 @@ def get_votes(self) -> dict[str, Any]: votes["count"] = votes["count"] return votes + def save(self, *args: Any, **kwargs: Any) -> None: + """Set author_username on creation if not already set.""" + if not self.pk and not self.author_username: + # On creation, store the current username + if self.retired_username: + self.author_username = self.retired_username + elif self.author: + self.author_username = self.author.username + super().save(*args, **kwargs) + def to_dict(self) -> dict[str, Any]: """Return a dictionary representation of the content.""" raise NotImplementedError @@ -288,7 +310,9 @@ def to_dict(self) -> dict[str, Any]: "closed_by_id": str(self.closed_by.pk) if self.closed_by else None, "close_reason_code": self.close_reason_code, "author_id": str(self.author.pk), - "author_username": self.author.username, + "author_username": self.author_username + or self.retired_username + or self.author.username, "updated_at": self.updated_at, "created_at": self.created_at, "last_activity_at": self.last_activity_at, @@ -469,7 +493,9 @@ def to_dict(self) -> dict[str, Any]: "author_id": str(self.author.pk), "comment_thread_id": str(self.comment_thread.pk), "child_count": self.child_count, - "author_username": self.author.username, + "author_username": self.author_username + or self.retired_username + or self.author.username, "sk": str(self.pk), "updated_at": self.updated_at, "created_at": self.created_at, diff --git a/forum/migration_helpers.py b/forum/migration_helpers.py index e61692cc..86a85db3 100644 --- a/forum/migration_helpers.py +++ b/forum/migration_helpers.py @@ -91,6 +91,20 @@ def create_or_update_thread(thread_data: dict[str, Any]) -> None: author = get_user_or_none(thread_data["author_id"]) if not author: return + + # Preserve author_username from MongoDB (historical username) + author_username = thread_data.get("author_username") + + # Preserve retired_username from MongoDB + retired_username = thread_data.get("retired_username") + + # If author_username is not provided, use retired_username or fallback to current username + if not author_username: + if retired_username: + author_username = retired_username + else: + author_username = author.username + mongo_thread_id = str(thread_data["_id"]) mongo_content, _ = MongoContent.objects.get_or_create( mongo_id=mongo_thread_id, @@ -98,6 +112,8 @@ def create_or_update_thread(thread_data: dict[str, Any]) -> None: if not mongo_content.content_object_id: thread = CommentThread.objects.create( author=author, + author_username=author_username, + retired_username=retired_username, course_id=thread_data["course_id"], title=get_trunc_title(thread_data.get("title", "")), body=thread_data["body"], @@ -128,6 +144,20 @@ def create_or_update_comment(comment_data: dict[str, Any]) -> None: author = get_user_or_none(comment_data["author_id"]) if not author: return + + # Preserve author_username from MongoDB (historical username) + author_username = comment_data.get("author_username") + + # Preserve retired_username from MongoDB + retired_username = comment_data.get("retired_username") + + # If author_username is not provided, use retired_username or fallback to current username + if not author_username: + if retired_username: + author_username = retired_username + else: + author_username = author.username + mongo_thread_id = str(comment_data["comment_thread_id"]) mongo_thread = MongoContent.objects.filter(mongo_id=mongo_thread_id).first() if not mongo_thread: @@ -169,6 +199,8 @@ def create_or_update_comment(comment_data: dict[str, Any]) -> None: if not mongo_comment.content_object_id: comment = Comment.objects.create( author=author, + author_username=author_username, + retired_username=retired_username, comment_thread=thread, parent=parent, course_id=comment_data["course_id"], diff --git a/forum/migrations/0004_add_author_username_fields.py b/forum/migrations/0004_add_author_username_fields.py new file mode 100644 index 00000000..2036f0c3 --- /dev/null +++ b/forum/migrations/0004_add_author_username_fields.py @@ -0,0 +1,43 @@ +# Generated by Django 4.2 on 2024-10-24 13:36 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("forum", "0003_alter_commentthread_title"), + ] + + operations = [ + migrations.AddField( + model_name="comment", + name="author_username", + field=models.CharField( + blank=True, + help_text="Username at time of posting, preserved for historical accuracy", + max_length=255, + null=True, + ), + ), + migrations.AddField( + model_name="commentthread", + name="author_username", + field=models.CharField( + blank=True, + help_text="Username at time of posting, preserved for historical accuracy", + max_length=255, + null=True, + ), + ), + migrations.AddField( + model_name="commentthread", + name="retired_username", + field=models.CharField( + blank=True, + help_text="Username to display if author account was retired", + max_length=255, + null=True, + ), + ), + ] diff --git a/tests/test_backends/test_mysql/test_models.py b/tests/test_backends/test_mysql/test_models.py index 6e9b1fc5..f483970e 100644 --- a/tests/test_backends/test_mysql/test_models.py +++ b/tests/test_backends/test_mysql/test_models.py @@ -841,3 +841,279 @@ def test_subscription_unique_together() -> None: Subscription.objects.create( subscriber=user, source_content_type=content_type, source_object_id=1 ) + + +@pytest.mark.django_db +def test_comment_thread_author_username_set_on_creation() -> None: + """Test that author_username is automatically set on CommentThread creation.""" + user = User.objects.create( + username="testuser", email="test@example.com", password="password" + ) + comment_thread = CommentThread.objects.create( + author=user, + course_id="course123", + title="Test Thread", + body="This is a test thread", + thread_type="discussion", + context="course", + ) + assert comment_thread.author_username == "testuser" + + +@pytest.mark.django_db +def test_comment_author_username_set_on_creation() -> None: + """Test that author_username is automatically set on Comment creation.""" + user = User.objects.create( + username="testuser", email="test@example.com", password="password" + ) + comment_thread = CommentThread.objects.create( + author=user, + course_id="course123", + title="Test Thread", + body="This is a test thread", + thread_type="discussion", + context="course", + ) + comment = Comment.objects.create( + author=user, + course_id="course123", + body="This is a test comment", + comment_thread=comment_thread, + ) + assert comment.author_username == "testuser" + + +@pytest.mark.django_db +def test_comment_thread_author_username_preserved_when_provided() -> None: + """Test that author_username is preserved when explicitly provided.""" + user = User.objects.create( + username="newusername", email="test@example.com", password="password" + ) + comment_thread = CommentThread.objects.create( + author=user, + author_username="originalusername", + course_id="course123", + title="Test Thread", + body="This is a test thread", + thread_type="discussion", + context="course", + ) + assert comment_thread.author_username == "originalusername" + + +@pytest.mark.django_db +def test_comment_author_username_preserved_when_provided() -> None: + """Test that author_username is preserved when explicitly provided.""" + user = User.objects.create( + username="newusername", email="test@example.com", password="password" + ) + comment_thread = CommentThread.objects.create( + author=user, + course_id="course123", + title="Test Thread", + body="This is a test thread", + thread_type="discussion", + context="course", + ) + comment = Comment.objects.create( + author=user, + author_username="originalusername", + course_id="course123", + body="This is a test comment", + comment_thread=comment_thread, + ) + assert comment.author_username == "originalusername" + + +@pytest.mark.django_db +def test_comment_thread_retired_username_priority_on_creation() -> None: + """Test that retired_username takes priority when set during creation.""" + user = User.objects.create( + username="retired_user_abc123", email="test@example.com", password="password" + ) + comment_thread = CommentThread.objects.create( + author=user, + retired_username="originaluser", + course_id="course123", + title="Test Thread", + body="This is a test thread", + thread_type="discussion", + context="course", + ) + # retired_username should be used as author_username + assert comment_thread.author_username == "originaluser" + assert comment_thread.retired_username == "originaluser" + + +@pytest.mark.django_db +def test_comment_retired_username_priority_on_creation() -> None: + """Test that retired_username takes priority when set during creation.""" + user = User.objects.create( + username="retired_user_abc123", email="test@example.com", password="password" + ) + comment_thread = CommentThread.objects.create( + author=user, + course_id="course123", + title="Test Thread", + body="This is a test thread", + thread_type="discussion", + context="course", + ) + comment = Comment.objects.create( + author=user, + retired_username="originaluser", + course_id="course123", + body="This is a test comment", + comment_thread=comment_thread, + ) + # retired_username should be used as author_username + assert comment.author_username == "originaluser" + assert comment.retired_username == "originaluser" + + +@pytest.mark.django_db +def test_comment_thread_to_dict_uses_author_username() -> None: + """Test that to_dict uses author_username when available.""" + user = User.objects.create( + username="currentuser", email="test@example.com", password="password" + ) + comment_thread = CommentThread.objects.create( + author=user, + author_username="historicaluser", + course_id="course123", + title="Test Thread", + body="This is a test thread", + thread_type="discussion", + context="course", + ) + thread_dict = comment_thread.to_dict() + assert thread_dict["author_username"] == "historicaluser" + + +@pytest.mark.django_db +def test_comment_to_dict_uses_author_username() -> None: + """Test that to_dict uses author_username when available.""" + user = User.objects.create( + username="currentuser", email="test@example.com", password="password" + ) + comment_thread = CommentThread.objects.create( + author=user, + course_id="course123", + title="Test Thread", + body="This is a test thread", + thread_type="discussion", + context="course", + ) + comment = Comment.objects.create( + author=user, + author_username="historicaluser", + course_id="course123", + body="This is a test comment", + comment_thread=comment_thread, + ) + comment_dict = comment.to_dict() + assert comment_dict["author_username"] == "historicaluser" + + +@pytest.mark.django_db +def test_comment_thread_to_dict_fallback_to_retired_username() -> None: + """Test that to_dict falls back to retired_username when author_username is None.""" + user = User.objects.create( + username="retired_user_abc123", email="test@example.com", password="password" + ) + comment_thread = CommentThread.objects.create( + author=user, + retired_username="retireduser", + course_id="course123", + title="Test Thread", + body="This is a test thread", + thread_type="discussion", + context="course", + ) + # Manually set author_username to None to test fallback + comment_thread.author_username = None + comment_thread.save(update_fields=["author_username"]) + + thread_dict = comment_thread.to_dict() + assert thread_dict["author_username"] == "retireduser" + + +@pytest.mark.django_db +def test_comment_to_dict_fallback_to_retired_username() -> None: + """Test that to_dict falls back to retired_username when author_username is None.""" + user = User.objects.create( + username="retired_user_abc123", email="test@example.com", password="password" + ) + comment_thread = CommentThread.objects.create( + author=user, + course_id="course123", + title="Test Thread", + body="This is a test thread", + thread_type="discussion", + context="course", + ) + comment = Comment.objects.create( + author=user, + retired_username="retireduser", + course_id="course123", + body="This is a test comment", + comment_thread=comment_thread, + ) + # Manually set author_username to None to test fallback + comment.author_username = None + comment.save(update_fields=["author_username"]) + + comment_dict = comment.to_dict() + assert comment_dict["author_username"] == "retireduser" + + +@pytest.mark.django_db +def test_comment_thread_to_dict_fallback_to_current_username() -> None: + """Test that to_dict falls back to current username when both author_username and retired_username are None.""" + user = User.objects.create( + username="currentuser", email="test@example.com", password="password" + ) + comment_thread = CommentThread.objects.create( + author=user, + course_id="course123", + title="Test Thread", + body="This is a test thread", + thread_type="discussion", + context="course", + ) + # Manually set both to None to test final fallback + comment_thread.author_username = None + comment_thread.retired_username = None + comment_thread.save(update_fields=["author_username", "retired_username"]) + + thread_dict = comment_thread.to_dict() + assert thread_dict["author_username"] == "currentuser" + + +@pytest.mark.django_db +def test_comment_to_dict_fallback_to_current_username() -> None: + """Test that to_dict falls back to current username when both author_username and retired_username are None.""" + user = User.objects.create( + username="currentuser", email="test@example.com", password="password" + ) + comment_thread = CommentThread.objects.create( + author=user, + course_id="course123", + title="Test Thread", + body="This is a test thread", + thread_type="discussion", + context="course", + ) + comment = Comment.objects.create( + author=user, + course_id="course123", + body="This is a test comment", + comment_thread=comment_thread, + ) + # Manually set both to None to test final fallback + comment.author_username = None + comment.retired_username = None + comment.save(update_fields=["author_username", "retired_username"]) + + comment_dict = comment.to_dict() + assert comment_dict["author_username"] == "currentuser" diff --git a/tests/test_management/test_commands/test_migration_commands.py b/tests/test_management/test_commands/test_migration_commands.py index 0f8a4520..06965897 100644 --- a/tests/test_management/test_commands/test_migration_commands.py +++ b/tests/test_management/test_commands/test_migration_commands.py @@ -704,3 +704,245 @@ def test_partial_user_existence_migration(patched_mongodb: Database[Any]) -> Non # Validate invalid content is skipped assert not MongoContent.objects.filter(mongo_id=comment_thread_id_invalid).exists() assert not MongoContent.objects.filter(mongo_id=comment_id_invalid).exists() + + +def test_migrate_thread_preserves_author_username( + patched_mongodb: Database[Any], +) -> None: + """Test that author_username is preserved during thread migration.""" + comment_thread_id = ObjectId() + patched_mongodb.contents.insert_one( + { + "_id": comment_thread_id, + "_type": "CommentThread", + "author_id": "1", + "author_username": "historical_username", + "course_id": "test_course", + "title": "Test Thread", + "body": "Test body", + "created_at": timezone.now(), + "updated_at": timezone.now(), + "votes": {"up": [], "down": []}, + "abuse_flaggers": [], + "historical_abuse_flaggers": [], + "last_activity_at": timezone.now(), + } + ) + + User.objects.create(id=1, username="current_username") + call_command("forum_migrate_course_from_mongodb_to_mysql", "test_course") + + mongo_thread = MongoContent.objects.get(mongo_id=comment_thread_id) + thread = CommentThread.objects.get(pk=mongo_thread.content_object_id) + assert thread.author_username == "historical_username" + assert thread.author.username == "current_username" + + +def test_migrate_comment_preserves_author_username( + patched_mongodb: Database[Any], +) -> None: + """Test that author_username is preserved during comment migration.""" + comment_thread_id = ObjectId() + comment_id = ObjectId() + patched_mongodb.contents.insert_many( + [ + { + "_id": comment_thread_id, + "_type": "CommentThread", + "author_id": "1", + "course_id": "test_course", + "title": "Test Thread", + "body": "Test body", + "created_at": timezone.now(), + "updated_at": timezone.now(), + "votes": {"up": [], "down": []}, + "abuse_flaggers": [], + "historical_abuse_flaggers": [], + "last_activity_at": timezone.now(), + }, + { + "_id": comment_id, + "_type": "Comment", + "author_id": "1", + "author_username": "historical_username", + "course_id": "test_course", + "body": "Test comment", + "created_at": timezone.now(), + "updated_at": timezone.now(), + "comment_thread_id": comment_thread_id, + "votes": {"up": [], "down": []}, + "abuse_flaggers": [], + "historical_abuse_flaggers": [], + "depth": 0, + "sk": f"{comment_id}", + }, + ] + ) + + User.objects.create(id=1, username="current_username") + call_command("forum_migrate_course_from_mongodb_to_mysql", "test_course") + + mongo_comment = MongoContent.objects.get(mongo_id=comment_id) + comment = Comment.objects.get(pk=mongo_comment.content_object_id) + assert comment.author_username == "historical_username" + assert comment.author.username == "current_username" + + +def test_migrate_thread_preserves_retired_username( + patched_mongodb: Database[Any], +) -> None: + """Test that retired_username is preserved during thread migration.""" + comment_thread_id = ObjectId() + patched_mongodb.contents.insert_one( + { + "_id": comment_thread_id, + "_type": "CommentThread", + "author_id": "1", + "retired_username": "retired_user", + "course_id": "test_course", + "title": "Test Thread", + "body": "Test body", + "created_at": timezone.now(), + "updated_at": timezone.now(), + "votes": {"up": [], "down": []}, + "abuse_flaggers": [], + "historical_abuse_flaggers": [], + "last_activity_at": timezone.now(), + } + ) + + User.objects.create(id=1, username="retired_user_abc123") + call_command("forum_migrate_course_from_mongodb_to_mysql", "test_course") + + mongo_thread = MongoContent.objects.get(mongo_id=comment_thread_id) + thread = CommentThread.objects.get(pk=mongo_thread.content_object_id) + assert thread.retired_username == "retired_user" + assert thread.author_username == "retired_user" + + +def test_migrate_comment_preserves_retired_username( + patched_mongodb: Database[Any], +) -> None: + """Test that retired_username is preserved during comment migration.""" + comment_thread_id = ObjectId() + comment_id = ObjectId() + patched_mongodb.contents.insert_many( + [ + { + "_id": comment_thread_id, + "_type": "CommentThread", + "author_id": "1", + "course_id": "test_course", + "title": "Test Thread", + "body": "Test body", + "created_at": timezone.now(), + "updated_at": timezone.now(), + "votes": {"up": [], "down": []}, + "abuse_flaggers": [], + "historical_abuse_flaggers": [], + "last_activity_at": timezone.now(), + }, + { + "_id": comment_id, + "_type": "Comment", + "author_id": "1", + "retired_username": "retired_user", + "course_id": "test_course", + "body": "Test comment", + "created_at": timezone.now(), + "updated_at": timezone.now(), + "comment_thread_id": comment_thread_id, + "votes": {"up": [], "down": []}, + "abuse_flaggers": [], + "historical_abuse_flaggers": [], + "depth": 0, + "sk": f"{comment_id}", + }, + ] + ) + + User.objects.create(id=1, username="retired_user_abc123") + call_command("forum_migrate_course_from_mongodb_to_mysql", "test_course") + + mongo_comment = MongoContent.objects.get(mongo_id=comment_id) + comment = Comment.objects.get(pk=mongo_comment.content_object_id) + assert comment.retired_username == "retired_user" + assert comment.author_username == "retired_user" + + +def test_migrate_thread_fallback_to_current_username( + patched_mongodb: Database[Any], +) -> None: + """Test that migration falls back to current username when author_username is missing.""" + comment_thread_id = ObjectId() + patched_mongodb.contents.insert_one( + { + "_id": comment_thread_id, + "_type": "CommentThread", + "author_id": "1", + "course_id": "test_course", + "title": "Test Thread", + "body": "Test body", + "created_at": timezone.now(), + "updated_at": timezone.now(), + "votes": {"up": [], "down": []}, + "abuse_flaggers": [], + "historical_abuse_flaggers": [], + "last_activity_at": timezone.now(), + } + ) + + User.objects.create(id=1, username="current_username") + call_command("forum_migrate_course_from_mongodb_to_mysql", "test_course") + + mongo_thread = MongoContent.objects.get(mongo_id=comment_thread_id) + thread = CommentThread.objects.get(pk=mongo_thread.content_object_id) + assert thread.author_username == "current_username" + + +def test_migrate_comment_fallback_to_current_username( + patched_mongodb: Database[Any], +) -> None: + """Test that migration falls back to current username when author_username is missing.""" + comment_thread_id = ObjectId() + comment_id = ObjectId() + patched_mongodb.contents.insert_many( + [ + { + "_id": comment_thread_id, + "_type": "CommentThread", + "author_id": "1", + "course_id": "test_course", + "title": "Test Thread", + "body": "Test body", + "created_at": timezone.now(), + "updated_at": timezone.now(), + "votes": {"up": [], "down": []}, + "abuse_flaggers": [], + "historical_abuse_flaggers": [], + "last_activity_at": timezone.now(), + }, + { + "_id": comment_id, + "_type": "Comment", + "author_id": "1", + "course_id": "test_course", + "body": "Test comment", + "created_at": timezone.now(), + "updated_at": timezone.now(), + "comment_thread_id": comment_thread_id, + "votes": {"up": [], "down": []}, + "abuse_flaggers": [], + "historical_abuse_flaggers": [], + "depth": 0, + "sk": f"{comment_id}", + }, + ] + ) + + User.objects.create(id=1, username="current_username") + call_command("forum_migrate_course_from_mongodb_to_mysql", "test_course") + + mongo_comment = MongoContent.objects.get(mongo_id=comment_id) + comment = Comment.objects.get(pk=mongo_comment.content_object_id) + assert comment.author_username == "current_username"