diff --git a/CHANGELOG.rst b/CHANGELOG.rst index ae320716..efd994f2 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -18,6 +18,15 @@ Unreleased * Fix: Set default value for ``pinned`` field on ``CommentThread`` to ``False`` to prevent NULL sort bug. +0.4.4 - 2025-10-21 +****************** + +Fixed +----- + +* Do not modify LMS users during retirement — this is handled by the LMS retirement pipeline. +* Ignore missing users during retirement. + [0.4.0] – 2026-03-12 ********************* diff --git a/forum/__init__.py b/forum/__init__.py index 15fb549a..63ea5f32 100644 --- a/forum/__init__.py +++ b/forum/__init__.py @@ -2,4 +2,4 @@ Openedx forum app. """ -__version__ = "0.4.3" +__version__ = "0.4.4" diff --git a/forum/api/users.py b/forum/api/users.py index 8355f8d3..82c5d716 100644 --- a/forum/api/users.py +++ b/forum/api/users.py @@ -134,15 +134,8 @@ def retire_user( backend = get_backend(course_id)() user = backend.get_user(user_id) if not user: - raise ForumV2RequestError(f"user not found with id: {user_id}") - backend.update_user( - user_id, - data={ - "email": "", - "username": retired_username, - "read_states": [], - }, - ) + return {"message": f"User not found with id: {user_id}"} + backend.update_user(user_id, {"read_states": []}) backend.unsubscribe_all(user_id) backend.retire_all_content(user_id, retired_username) diff --git a/tests/e2e/test_users.py b/tests/e2e/test_users.py index de1119e2..e64ec610 100644 --- a/tests/e2e/test_users.py +++ b/tests/e2e/test_users.py @@ -685,6 +685,8 @@ def test_retire_user_inactive(api_client: APIClient, patched_get_backend: Any) - backend = patched_get_backend() user_id = backend.find_or_create_user(user_id="1", username="user1") user = backend.get_user(user_id) or {} + original_username = user["username"] + original_email = user["email"] # Verify user is not subscribed to any threads response = api_client.get_json( @@ -709,9 +711,10 @@ def test_retire_user_inactive(api_client: APIClient, patched_get_backend: Any) - ) assert response.status_code == 200 + # Retiring user in the forum backend should not touch LMS User model. user = backend.get_user(user_id) or {} - assert user["username"] == retired_username - assert user["email"] == "" + assert user["username"] == original_username + assert user["email"] == original_email content = backend.get_user_contents_by_username(retired_username) assert len(content) == 0 diff --git a/tests/test_views/test_users.py b/tests/test_views/test_users.py index 9cdfaf62..dfbc1abd 100644 --- a/tests/test_views/test_users.py +++ b/tests/test_views/test_users.py @@ -393,7 +393,11 @@ def test_attempts_to_retire_user_without_sending_retired_username( def test_attempts_to_retire_non_existent_user( api_client: APIClient, patched_get_backend: Any ) -> None: - """Test retire non-existent user.""" + """ + Test retire non-existent user. + + Retiring a non-existent user should return 200, since the user can be considered already retired. + """ backend = patched_get_backend user_id = backend.generate_id() retired_username = "retired_user_test" @@ -401,7 +405,7 @@ def test_attempts_to_retire_non_existent_user( f"/api/v2/users/{user_id}/retire", data={"retired_username": retired_username}, ) - assert response.status_code == 400 + assert response.status_code == 200 def test_retire_user(api_client: APIClient, patched_get_backend: Any) -> None: @@ -416,6 +420,7 @@ def test_retire_user(api_client: APIClient, patched_get_backend: Any) -> None: setup_10_threads(user_id, username, backend) retired_username = "retired_username_ABCD1234" user = backend.get_user(user_id) + email = user["email"] assert user assert user["username"] == username @@ -426,8 +431,11 @@ def test_retire_user(api_client: APIClient, patched_get_backend: Any) -> None: assert response.status_code == 200 user = backend.get_user(user_id) assert user - assert user["username"] == retired_username - assert user["email"] == "" + + # Retiring user in the forum backend should not touch LMS User model. + assert user["username"] == username + assert user["email"] == email + contents = list(backend.get_contents(author_id=user_id)) assert len(contents) > 0 for content in contents: @@ -454,6 +462,7 @@ def test_retire_user_with_subscribed_threads( setup_10_threads(user_id, username, backend) retired_username = "retired_username_ABCD1234" user = backend.get_user(user_id) + email = user["email"] assert user assert user["username"] == username thread_id = backend.create_thread( @@ -483,8 +492,11 @@ def test_retire_user_with_subscribed_threads( user = backend.get_user(user_id) assert user - assert user["username"] == retired_username - assert user["email"] == "" + + # Retiring user in the forum backend should not touch LMS User model. + assert user["username"] == username + assert user["email"] == email + # User should be subscribed to no threads. response = api_client.get( f"/api/v2/users/{user_id}/subscribed_threads?course_id=course1",