From ae3830b02333ff1d79d1e6659be13529a40033b0 Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Wed, 13 Sep 2023 20:42:39 +0300 Subject: [PATCH 1/3] feat: [AXIM-20] Add profile_image to API CommentViewSet --- .../discussion/rest_api/serializers.py | 6 ++++ .../discussion/rest_api/tests/test_api.py | 35 +++++++++++++++++++ .../rest_api/tests/test_serializers.py | 7 ++++ .../discussion/rest_api/tests/test_views.py | 28 +++++++++++++++ 4 files changed, 76 insertions(+) diff --git a/lms/djangoapps/discussion/rest_api/serializers.py b/lms/djangoapps/discussion/rest_api/serializers.py index 65778a34f1aa..f080ae25a19a 100644 --- a/lms/djangoapps/discussion/rest_api/serializers.py +++ b/lms/djangoapps/discussion/rest_api/serializers.py @@ -42,6 +42,7 @@ from openedx.core.djangoapps.django_comment_common.comment_client.utils import CommentClientRequestError from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings from openedx.core.lib.api.serializers import CourseKeyField +from openedx.core.djangoapps.user_api.accounts.serializers import AccountLegacyProfileSerializer User = get_user_model() @@ -502,6 +503,7 @@ class CommentSerializer(_ContentSerializer): child_count = serializers.IntegerField(read_only=True) children = serializers.SerializerMethodField(required=False) abuse_flagged_any_user = serializers.SerializerMethodField(required=False) + profile_image = serializers.SerializerMethodField(read_only=True) non_updatable_fields = NON_UPDATABLE_COMMENT_FIELDS @@ -584,6 +586,10 @@ def get_abuse_flagged_any_user(self, obj): if _validate_privileged_access(self.context): return len(obj.get("abuse_flaggers", [])) > 0 + def get_profile_image(self, obj): + request = self.context["request"] + return AccountLegacyProfileSerializer.get_profile_image(request.user.profile, request.user, request) + def validate(self, attrs): """ Ensure that parent_id identifies a comment that is actually in the diff --git a/lms/djangoapps/discussion/rest_api/tests/test_api.py b/lms/djangoapps/discussion/rest_api/tests/test_api.py index 98c27feb58c1..ebe4a429c42c 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_api.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_api.py @@ -1478,6 +1478,13 @@ def test_discussion_content(self): "anonymous_to_peers": False, "last_edit": None, "edit_by_label": None, + "profile_image": { + "has_image": False, + "image_url_full": "http://testserver/static/default_500.png", + "image_url_large": "http://testserver/static/default_120.png", + "image_url_medium": "http://testserver/static/default_50.png", + "image_url_small": "http://testserver/static/default_30.png", + }, }, { "id": "test_comment_2", @@ -1505,6 +1512,13 @@ def test_discussion_content(self): "anonymous_to_peers": False, "last_edit": None, "edit_by_label": None, + "profile_image": { + "has_image": False, + "image_url_full": "http://testserver/static/default_500.png", + "image_url_large": "http://testserver/static/default_120.png", + "image_url_medium": "http://testserver/static/default_50.png", + "image_url_small": "http://testserver/static/default_30.png", + }, }, ] actual_comments = self.get_comment_list( @@ -2252,6 +2266,13 @@ def test_success(self, parent_id, mock_emit): "anonymous_to_peers": False, "last_edit": None, "edit_by_label": None, + "profile_image": { + "has_image": False, + "image_url_full": "http://testserver/static/default_500.png", + "image_url_large": "http://testserver/static/default_120.png", + "image_url_medium": "http://testserver/static/default_50.png", + "image_url_small": "http://testserver/static/default_30.png", + }, } assert actual == expected expected_url = ( @@ -2352,6 +2373,13 @@ def test_success_in_black_out_with_user_access(self, parent_id, mock_emit): "anonymous_to_peers": False, "last_edit": None, "edit_by_label": None, + "profile_image": { + "has_image": False, + "image_url_full": "http://testserver/static/default_500.png", + "image_url_large": "http://testserver/static/default_120.png", + "image_url_medium": "http://testserver/static/default_50.png", + "image_url_small": "http://testserver/static/default_30.png", + }, } assert actual == expected expected_url = ( @@ -3188,6 +3216,13 @@ def test_basic(self, parent_id): "can_delete": True, "last_edit": None, "edit_by_label": None, + "profile_image": { + "has_image": False, + "image_url_full": "http://testserver/static/default_500.png", + "image_url_large": "http://testserver/static/default_120.png", + "image_url_medium": "http://testserver/static/default_50.png", + "image_url_small": "http://testserver/static/default_30.png", + }, } assert actual == expected assert parsed_body(httpretty.last_request()) == { diff --git a/lms/djangoapps/discussion/rest_api/tests/test_serializers.py b/lms/djangoapps/discussion/rest_api/tests/test_serializers.py index d0ded0f013d3..e2035c9299d9 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_serializers.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_serializers.py @@ -448,6 +448,13 @@ def test_basic(self): "can_delete": False, "last_edit": None, "edit_by_label": None, + "profile_image": { + "has_image": False, + "image_url_full": "http://testserver/static/default_500.png", + "image_url_large": "http://testserver/static/default_120.png", + "image_url_medium": "http://testserver/static/default_50.png", + "image_url_small": "http://testserver/static/default_30.png", + }, } assert self.serialize(comment) == expected diff --git a/lms/djangoapps/discussion/rest_api/tests/test_views.py b/lms/djangoapps/discussion/rest_api/tests/test_views.py index 7f6fa5772585..806cf9b11d31 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_views.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_views.py @@ -1972,6 +1972,13 @@ def expected_response_comment(self, overrides=None): "anonymous_to_peers": False, "last_edit": None, "edit_by_label": None, + "profile_image": { + "has_image": False, + "image_url_full": "http://testserver/static/default_500.png", + "image_url_large": "http://testserver/static/default_120.png", + "image_url_medium": "http://testserver/static/default_50.png", + "image_url_small": "http://testserver/static/default_30.png", + }, } response_data.update(overrides or {}) return response_data @@ -2398,6 +2405,13 @@ def test_basic(self): "anonymous_to_peers": False, "last_edit": None, "edit_by_label": None, + "profile_image": { + "has_image": False, + "image_url_full": "http://testserver/static/default_500.png", + "image_url_large": "http://testserver/static/default_120.png", + "image_url_medium": "http://testserver/static/default_50.png", + "image_url_small": "http://testserver/static/default_30.png", + }, } response = self.client.post( self.url, @@ -2490,6 +2504,13 @@ def expected_response_data(self, overrides=None): "anonymous_to_peers": False, "last_edit": None, "edit_by_label": None, + "profile_image": { + "has_image": False, + "image_url_full": "http://testserver/static/default_500.png", + "image_url_large": "http://testserver/static/default_120.png", + "image_url_medium": "http://testserver/static/default_50.png", + "image_url_small": "http://testserver/static/default_30.png", + }, } response_data.update(overrides or {}) return response_data @@ -2680,6 +2701,13 @@ def test_basic(self): "anonymous_to_peers": False, "last_edit": None, "edit_by_label": None, + "profile_image": { + "has_image": False, + "image_url_full": "http://testserver/static/default_500.png", + "image_url_large": "http://testserver/static/default_120.png", + "image_url_medium": "http://testserver/static/default_50.png", + "image_url_small": "http://testserver/static/default_30.png", + }, } response = self.client.get(self.url) From da9266e31d72e96706b5a8be464885480cb78c72 Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Wed, 8 Nov 2023 17:43:57 +0200 Subject: [PATCH 2/3] refactor: [FC-0031] Move get_profile_image method to api --- lms/djangoapps/discussion/rest_api/serializers.py | 4 ++-- openedx/core/djangoapps/user_api/accounts/api.py | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/discussion/rest_api/serializers.py b/lms/djangoapps/discussion/rest_api/serializers.py index f080ae25a19a..5f18281c422c 100644 --- a/lms/djangoapps/discussion/rest_api/serializers.py +++ b/lms/djangoapps/discussion/rest_api/serializers.py @@ -41,8 +41,8 @@ from openedx.core.djangoapps.django_comment_common.comment_client.user import User as CommentClientUser from openedx.core.djangoapps.django_comment_common.comment_client.utils import CommentClientRequestError from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings +from openedx.core.djangoapps.user_api.accounts.api import get_profile_images from openedx.core.lib.api.serializers import CourseKeyField -from openedx.core.djangoapps.user_api.accounts.serializers import AccountLegacyProfileSerializer User = get_user_model() @@ -588,7 +588,7 @@ def get_abuse_flagged_any_user(self, obj): def get_profile_image(self, obj): request = self.context["request"] - return AccountLegacyProfileSerializer.get_profile_image(request.user.profile, request.user, request) + return get_profile_images(request.user.profile, request.user, request) def validate(self, attrs): """ diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index bc4fd1d1a769..786a903c64c4 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/openedx/core/djangoapps/user_api/accounts/api.py @@ -33,6 +33,8 @@ AccountValidationError, PreferenceValidationError ) +from openedx.core.djangoapps.user_api.accounts.image_helpers import get_profile_image_urls_for_user +from openedx.core.djangoapps.user_api.accounts.serializers import PROFILE_IMAGE_KEY_PREFIX from openedx.core.djangoapps.user_api.preferences.api import update_user_preferences from openedx.core.djangoapps.user_authn.utils import check_pwned_password from openedx.core.djangoapps.user_authn.views.registration_form import validate_name, validate_username @@ -524,6 +526,19 @@ def get_email_existence_validation_error(email): return _validate(_validate_email_doesnt_exist, errors.AccountEmailAlreadyExists, email) +def get_profile_images(user_profile, user, request=None): + """ + Returns metadata about a user's profile image. + """ + data = {'has_image': user_profile.has_profile_image} + urls = get_profile_image_urls_for_user(user, request) + data.update({ + f'{PROFILE_IMAGE_KEY_PREFIX}_{size_display_name}': url + for size_display_name, url in urls.items() + }) + return data + + def _get_user_and_profile(username): """ Helper method to return the legacy user and profile objects based on username. From e27f674e12c411b0c4bdf59de96d7f386e6c4a8d Mon Sep 17 00:00:00 2001 From: Glib Glugovskiy Date: Tue, 14 Nov 2023 00:43:10 +0200 Subject: [PATCH 3/3] fix: remove duplicate implementation and correct the docstring --- .../core/djangoapps/user_api/accounts/api.py | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index 786a903c64c4..7e8d34be4745 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/openedx/core/djangoapps/user_api/accounts/api.py @@ -33,8 +33,6 @@ AccountValidationError, PreferenceValidationError ) -from openedx.core.djangoapps.user_api.accounts.image_helpers import get_profile_image_urls_for_user -from openedx.core.djangoapps.user_api.accounts.serializers import PROFILE_IMAGE_KEY_PREFIX from openedx.core.djangoapps.user_api.preferences.api import update_user_preferences from openedx.core.djangoapps.user_authn.utils import check_pwned_password from openedx.core.djangoapps.user_authn.views.registration_form import validate_name, validate_username @@ -529,14 +527,18 @@ def get_email_existence_validation_error(email): def get_profile_images(user_profile, user, request=None): """ Returns metadata about a user's profile image. + + The output is a dict that looks like: + + { + "has_image": False, + "image_url_full": "http://testserver/static/default_500.png", + "image_url_large": "http://testserver/static/default_120.png", + "image_url_medium": "http://testserver/static/default_50.png", + "image_url_small": "http://testserver/static/default_30.png", + } """ - data = {'has_image': user_profile.has_profile_image} - urls = get_profile_image_urls_for_user(user, request) - data.update({ - f'{PROFILE_IMAGE_KEY_PREFIX}_{size_display_name}': url - for size_display_name, url in urls.items() - }) - return data + return AccountLegacyProfileSerializer.get_profile_image(user_profile, user, request) def _get_user_and_profile(username):