Extensions list: sort_by parameter #159

Merged
Márton Lente merged 36 commits from filter-sort into main 2024-06-03 12:57:45 +02:00
13 changed files with 230 additions and 166 deletions
Showing only changes of commit b9a7e26a7f - Show all commits

View File

@ -7,8 +7,7 @@ from django.core.exceptions import FieldDoesNotExist
from django.db import models
from django.db.models.constants import LOOKUP_SEP
from django.urls import reverse
from django.utils.html import escape
from django.utils.html import format_html
from django.utils.html import escape, format_html
def get_admin_change_path(obj=None, app_label=None, model_name=None, pk=None):
@ -27,7 +26,7 @@ def get_admin_change_url(obj=None, app_label=None, model_name=None, pk=None, tex
"""Return a link to the admin change page for a given object."""
url = get_admin_change_path(obj=obj, app_label=app_label, model_name=model_name, pk=pk)
if url:
text = repr(obj) if obj else text
text = str(obj) if obj else text
return format_html(f'<a href="{url}">{text}</a>')
return ''
@ -57,6 +56,27 @@ def get_related_admin_change_url(obj, related_field):
return get_admin_change_url(instance)
def link_to(field_name, title=None):
if not title:
title = field_name.replace('_', ' ')
related_field_name = None
if '.' in field_name:
field_name, related_field_name = field_name.split('.')
@admin.display(description=title, ordering=field_name)
def _raw(obj):
if related_field_name:
target_obj = getattr(getattr(obj, field_name), related_field_name)
else:
target_obj = getattr(obj, field_name)
admin_url = get_admin_change_url(target_obj)
return admin_url
_raw.__name__ = field_name
return _raw
class CommaSearchInAdminMixin:
def get_search_id_field(self, request):
"""

View File

@ -80,7 +80,7 @@ class VersionFactory(DjangoModelFactory):
),
)
ratings = factory.RelatedFactoryList(
RatingFactory, size=lambda: random.randint(1, 50), factory_related_name='version'
RatingFactory, size=lambda: random.randint(0, 5), factory_related_name='version'
)
@factory.post_generation

View File

@ -250,6 +250,25 @@ class ExtensionUpdateForm(forms.ModelForm):
def save(self, *args, **kwargs):
"""Save the nested form(set)s, then the main form."""
if self.instance.is_listed:
updated_fields = set()
if 'description' in self.changed_data:
updated_fields.add('description')
if 'source' in self.featured_image_form.changed_data:
updated_fields.add('featured image')
if 'source' in self.icon_form.changed_data:
updated_fields.add('icon')
for form in self.add_preview_formset:
if 'source' in form.changed_data:
updated_fields.add('previews')
if updated_fields:
reviewers.models.ApprovalActivity(
user=self.request.user,
extension=self.instance,
type=reviewers.models.ApprovalActivity.ActivityType.COMMENT,
message='updated ' + ', '.join(sorted(updated_fields)),
).save()
self.edit_preview_formset.save()
self.add_preview_formset.save()
@ -280,6 +299,7 @@ class ExtensionUpdateForm(forms.ModelForm):
type=reviewers.models.ApprovalActivity.ActivityType.AWAITING_REVIEW,
message=self.msg_awaiting_review,
).save()
return super().save(*args, **kwargs)

View File

@ -626,3 +626,46 @@ class UpdateTest(CheckFilePropertiesMixin, TestCase):
self.assertEqual(response.status_code, 302, _get_all_form_errors(response))
extension.refresh_from_db()
self.assertEqual(extension.team.slug, 'test-team')
def test_updated_fields_comment_if_listed(self):
extension = create_approved_version().extension
nr_comments = extension.review_activity.count()
author = extension.authors.first()
self.client.force_login(author)
url = extension.get_manage_url()
file_name1 = 'test_preview_image_0001.png'
with open(TEST_FILES_DIR / file_name1, 'rb') as fp1:
response = self.client.post(
url,
{
**POST_DATA,
'form-TOTAL_FORMS': ['1'],
'form-0-source': fp1,
'description': 'new description',
'save': '',
},
)
self.assertEqual(response.status_code, 302, _get_all_form_errors(response))
self.assertEqual(extension.review_activity.count(), nr_comments + 1)
self.assertEqual(extension.review_activity.last().message, 'updated description, previews')
def test_updated_fields_no_comment_if_not_listed(self):
extension = create_version().extension
nr_comments = extension.review_activity.count()
author = extension.authors.first()
self.client.force_login(author)
url = extension.get_manage_url()
file_name1 = 'test_preview_image_0001.png'
with open(TEST_FILES_DIR / file_name1, 'rb') as fp1:
response = self.client.post(
url,
{
**POST_DATA,
'form-TOTAL_FORMS': ['1'],
'form-0-source': fp1,
'description': 'new description',
'save': '',
},
)
self.assertEqual(response.status_code, 302, _get_all_form_errors(response))
self.assertEqual(extension.review_activity.count(), nr_comments)

View File

@ -3,10 +3,9 @@ import logging
from django.conf import settings
from django.contrib import admin
from django.template.loader import render_to_string
from django.urls import reverse
from django.utils.safestring import mark_safe
from .models import File, FileValidation
from common.admin import link_to
import files.signals
logger = logging.getLogger(__name__)
@ -74,18 +73,35 @@ class FileAdmin(admin.ModelAdmin):
'date_modified',
'date_status_changed',
('extension', admin.EmptyFieldListFilter),
('version', admin.EmptyFieldListFilter),
('icon_of', admin.EmptyFieldListFilter),
('featured_image_of', admin.EmptyFieldListFilter),
('preview', admin.EmptyFieldListFilter),
)
list_display = (
'original_name',
'extension_link',
'user',
link_to('extension'),
link_to('user'),
'date_created',
'type',
'status',
link_to('version'),
link_to('icon_of'),
link_to('featured_image_of'),
link_to('preview.extension', 'preview of'),
'is_ok',
)
list_select_related = ('version__extension', 'user', 'extension', 'version', 'validation')
list_select_related = (
'version__extension',
'user',
'extension',
'version',
'validation',
'icon_of',
'featured_image_of',
'preview__extension',
)
autocomplete_fields = ['user']
readonly_fields = (
@ -155,20 +171,6 @@ class FileAdmin(admin.ModelAdmin):
inlines = [FileValidationInlineAdmin]
actions = [schedule_scan, make_thumbnails]
def extension_link(self, obj):
return (
mark_safe(
'<a href="{}" target="_blank">{}</a>'.format(
reverse('admin:extensions_extension_change', args=(obj.extension_id,)),
obj.extension,
)
)
if obj.extension_id
else '-'
)
extension_link.short_description = 'Extension'
def is_ok(self, obj):
return obj.validation.is_ok if hasattr(obj, 'validation') else None

View File

@ -9,3 +9,11 @@
padding-right: 0.5rem;
padding-left: 0.5rem;
}
.field-original_name,
.field-icon_of,
.field-featured_image_of,
.field-preview,
.field-extension,
.field-version {
word-break: break-word;
}

View File

@ -75,7 +75,7 @@ class BIDSession:
return self._badger_api_session
@classmethod
def get_oauth_user_info(cls, oauth_user_id: str) -> OAuthUserInfo:
def get_oauth_user_info(cls, oauth_user_id: int) -> OAuthUserInfo:
"""Return OAuthUserInfo record for a given Blender ID.
Used primarily to look up our own user ID associated with an external Blender ID,
@ -88,71 +88,38 @@ class BIDSession:
)
@classmethod
def get_oauth_token(cls, oauth_user_id: str) -> OAuthToken:
def get_oauth_token(cls, oauth_user_id: int) -> OAuthToken:
"""Return OAuthToken for a given ID to be used in authenticated requests to Blender ID."""
return OAuthToken.objects.filter(oauth_user_id=oauth_user_id).last()
def get_user_info(self, oauth_user_id: str) -> Dict[str, Any]:
"""Retrieve user info from Blender ID service using a user-specific OAuth2 session.
User info is returned in the following format:
{
"id": 2,
"full_name": "Jane Doe",
"email": "jane@example.com",
"nickname": "janedoe",
"roles": {"dev_core": True},
}
"""
token = self.get_oauth_token(oauth_user_id)
if not token:
raise BIDMissingAccessToken(f'No access token found for {oauth_user_id}')
session = self._make_session(access_token=token.access_token)
resp = session.get(self.settings.url_userinfo)
resp.raise_for_status()
payload = resp.json()
assert isinstance(payload, dict)
return payload
def get_avatar_url(self, oauth_user_id: str) -> str:
"""Return a Blender ID URL to the avatar for a given OAuth ID."""
return urljoin(self.settings.url_base, f'api/user/{oauth_user_id}/avatar')
def get_badges_url(self, oauth_user_id: str) -> str:
def get_badges_url(self, oauth_user_id: int) -> str:
"""Return a Blender ID URL to the avatar for a given OAuth ID."""
return urljoin(self.settings.url_base, f'api/badges/{oauth_user_id}')
def get_user_by_id_url(self, oauth_user_id: str) -> str:
"""Return a Blender ID URL for the user with a given OAuth ID."""
return urljoin(self.settings.url_base, f'api/user/{oauth_user_id}')
def get_check_user_by_email_url(self, email: str) -> str:
"""Return a Blender ID URL for checking existence of a record with a given email."""
return urljoin(self.settings.url_base, f'api/check-user/{email}')
def get_badger_api_url(self, action: str, role: str, oauth_user_id: str) -> str:
def get_badger_api_url(self, action: str, role: str, oauth_user_id: int) -> str:
"""Return a Blender ID API URL for granting/revoking roles."""
assert action in ('grant', 'revoke'), f'{action} is not a known Blender ID API action'
assert role in (
'cloud_subscriber',
'cloud_has_subscription',
'sprite_fright',
'charge',
), f'{role} is not a known Blender ID badge'
return urljoin(self.settings.url_base, f'api/badger/{action}/{role}/{oauth_user_id}')
def get_avatar(self, oauth_user_id: str) -> Tuple[str, io.BytesIO]:
"""Retrieve an avatar from Blender ID service using an OAuth2 session.
Return file name and content of an avatar for the given 'oauth_user_id'.
"""
resp = self.session.get(self.get_avatar_url(oauth_user_id))
def download_avatar_url(self, avatar_url: str) -> Tuple[str, io.BytesIO]:
"""Download an avatar from a given URL."""
resp = self.session.get(avatar_url)
resp.raise_for_status()
name = pathlib.Path(urlparse(resp.url).path).name
return name, io.BytesIO(resp.content)
def get_badges(self, oauth_user_id: str) -> Dict[str, Any]:
def get_badges(self, oauth_user_id: int) -> Dict[str, Any]:
"""Retrieve badges from Blender ID service using a user-specific OAuth2 session."""
token = self.get_oauth_token(oauth_user_id)
if not token:
@ -165,46 +132,22 @@ class BIDSession:
return badges
def copy_avatar_from_blender_id(self, user):
"""
Attempt to retrieve an avatar from Blender ID and save it into our storage.
If either OAuth info or Blender ID service isn't available, log an error and return.
"""
if not hasattr(user, 'oauth_info'):
logger.warning(f'Cannot copy avatar from Blender ID: {user} is missing OAuth info')
return
oauth_info = user.oauth_info
def copy_image_from_avatar_url(self, user, avatar_url):
"""Fetch Blender ID avatar and save it to our storage."""
try:
name, content = self.get_avatar(oauth_info.oauth_user_id)
clear_existing_image = avatar_url is None
if user.image:
# Delete the previous file
user.image.delete(save=False)
user.image.delete(save=clear_existing_image)
if avatar_url:
name, content = self.download_avatar_url(avatar_url)
user.image.save(name, content, save=True)
logger.info(f'Profile image updated for {user}')
logger.info('Profile image updated for pk=%s', user.pk)
except requests.HTTPError:
logger.warning(f'Failed to retrieve an image for {user} from Blender ID')
except Exception:
logger.exception(f'Failed to copy an image for {user}')
def update_username(self, user, oauth_user_id):
"""Update username of a given user, fetching it from Blender ID.
FIXME(anna): webhook payload doesn't include username, hence this separate method.
"""
try:
user_info = self.get_user_info(oauth_user_id)
if user_info['nickname'] != user.username:
# TODO(anna) handle duplicate usernames
user.username = user_info['nickname']
user.save(update_fields=['username'])
except BIDMissingAccessToken:
logger.warning(f'Unable to retrieve username for {user}: no access token')
except requests.exceptions.HTTPError:
logger.warning(f'Unable to update username for {user}: HTTPError')
except Exception:
logger.exception(f'Unable to update username for {user}')
def copy_badges_from_blender_id(self, user):
"""
Attempt to retrieve badges from Blender ID and save them in the user record.
@ -239,13 +182,6 @@ class BIDSession:
resp = self.badger_api_session.post(url)
resp.raise_for_status()
def get_user_by_id(self, oauth_user_id: str) -> Dict[str, str]:
"""Get Blender ID user info using the API OAuth token."""
url = self.get_user_by_id_url(oauth_user_id=oauth_user_id)
resp = self.badger_api_session.get(url)
resp.raise_for_status()
return resp.json()
def check_user_by_email(self, email: str) -> Dict[str, str]:
"""Check if Blender ID with a given email exists."""
url = self.get_check_user_by_email_url(email=email)

View File

@ -1,23 +0,0 @@
"""Fetch and save profile images from Blender ID."""
import logging
from django.core.management.base import BaseCommand
from django.contrib.auth import get_user_model
User = get_user_model()
logger = logging.getLogger(__name__)
logger.setLevel(logging.INFO)
class Command(BaseCommand):
"""Command for fetching and saving images for accounts that don't have any."""
def handle(self, *args, **options): # noqa: D102
query = User.objects.filter(last_login__isnull=False, profile__image__isnull=True).order_by(
'-last_login'
)
logger.info('%s records to update total', query.count())
for user in query:
if user.image:
continue # already has an image, skipping
user.copy_avatar_from_blender_id()

View File

@ -18,7 +18,9 @@ logger = logging.getLogger(__name__)
def user_image_upload_to(instance, filename):
assert instance.pk
prefix = 'avatars/'
_hash = get_sha256_from_value(instance.pk)
# Blender ID avatar URL is based on file hash,
# so is the filename taken from that URL, so this combination is unique enough
_hash = get_sha256_from_value(str(instance.pk) + filename)
extension = Path(filename).suffix
path = Path(prefix, _hash[:2], _hash).with_suffix(extension)
return path
@ -91,7 +93,14 @@ class User(TrackChangesMixin, AbstractUser):
)
self.is_active = False
self.date_deletion_requested = parse_datetime(date_deletion_requested)
self.save(update_fields=['is_active', 'date_deletion_requested'])
self.is_subscribed_to_notification_emails = False
self.save(
update_fields=[
'is_active',
'date_deletion_requested',
'is_subscribed_to_notification_emails',
]
)
@transaction.atomic
def anonymize_or_delete(self):

View File

@ -40,7 +40,8 @@ def update_user(
instance.confirmed_email_at = parse_datetime(oauth_info.get('confirmed_email_at') or '')
instance.save()
bid.copy_avatar_from_blender_id(user=instance)
if oauth_info.get('avatar_url'):
bid.copy_image_from_avatar_url(user=instance, avatar_url=oauth_info['avatar_url'])
bid.copy_badges_from_blender_id(user=instance)

View File

@ -44,11 +44,14 @@ class TestBlenderIDWebhook(TestCase):
fixtures = ['dev']
webhook_payload = {
'avatar_changed': False,
'avatar_url': None,
'email': 'newmail@example.com',
'full_name': 'Иван Васильевич Doe',
'id': 2,
'old_email': 'mail@example.com',
'roles': [],
'nickname': 'ivandoe',
'old_nickname': 'old_ivandoe',
}
def setUp(self):
@ -59,6 +62,7 @@ class TestBlenderIDWebhook(TestCase):
self.user = OAuthUserFactory(
email='mail@example.com',
oauth_info__oauth_user_id='2',
username='very-original-username',
oauth_tokens__oauth_user_id='2',
oauth_tokens__access_token='testaccesstoken',
oauth_tokens__refresh_token='testrefreshtoken',
@ -116,6 +120,7 @@ class TestBlenderIDWebhook(TestCase):
user = User.objects.get(id=self.user.pk)
self.assertEqual(user.full_name, 'Иван Васильевич Doe')
self.assertEqual(user.email, 'newmail@example.com')
self.assertEqual(user.username, 'ivandoe')
self.assertEqual(
user.badges,
{
@ -138,24 +143,11 @@ class TestBlenderIDWebhook(TestCase):
response = self.client.post(
self.url, body, content_type='application/json', **prepare_hmac_header(body)
)
self.assertRegex(logs.output[0], 'Cannot update user: no OAuth info found for ID 999')
self.assertEqual(response.status_code, 204)
@responses.activate
def test_user_modified_logs_errors_when_blender_id_user_info_broken(self):
body = self.webhook_payload
# Mock a "broken" user info response
responses.replace(
responses.GET, f'{BLENDER_ID_BASE_URL}api/me', status=403, body='Unauthorized'
self.assertRegex(
logs.output[0],
'WARNING:users.views.webhooks:Skipping user-modified: no OAuth info found for ID 999',
)
with self.assertLogs('users.blender_id', level='WARNING') as logs:
response = self.client.post(
self.url, body, content_type='application/json', **prepare_hmac_header(body)
)
self.assertRegex(logs.output[0], 'Unable to update username for ', logs.output[0])
self.assertEqual(response.status_code, 204)
@responses.activate
@ -163,6 +155,7 @@ class TestBlenderIDWebhook(TestCase):
body = {
**self.webhook_payload,
'avatar_changed': True,
'avatar_url': 'http://id.local:8000/media/avatar/fo/ob/foobar_128x128.jpg',
}
with self.assertLogs('users.blender_id', level='INFO') as logs:
@ -228,11 +221,14 @@ class TestIntegrityErrors(TransactionTestCase):
maxDiff = None
webhook_payload = {
'avatar_changed': False,
'avatar_url': None,
'email': 'newmail@example.com',
'full_name': 'Иван Васильевич Doe',
'id': 2,
'old_email': 'mail@example.com',
'roles': [],
'nickname': 'ivandoe',
'old_nickname': 'ivandoe',
}
def setUp(self):
@ -242,6 +238,7 @@ class TestIntegrityErrors(TransactionTestCase):
# Prepare a user
self.user = OAuthUserFactory(
email='mail@example.com',
username='very-original-username',
oauth_info__oauth_user_id='2',
oauth_tokens__oauth_user_id='2',
oauth_tokens__access_token='testaccesstoken',
@ -268,3 +265,50 @@ class TestIntegrityErrors(TransactionTestCase):
# Email was not updated
self.assertEqual(self.user.email, 'mail@example.com')
self.assertEqual(another_user.email, 'jane@example.com')
@responses.activate
def test_user_modified_does_not_allow_duplicate_username(self):
# Same email as in the webhook payload for another user
another_user = OAuthUserFactory(email='somename@example.com', username='thejane')
body = {
**self.webhook_payload,
'nickname': 'thejane',
}
with self.assertLogs('users.views.webhooks', level='ERROR') as logs:
response = self.client.post(
self.url, body, content_type='application/json', **prepare_hmac_header(body)
)
self.assertRegex(logs.output[0], 'Unable to update username for')
self.assertEqual(response.status_code, 204)
self.assertEqual(response.content, b'')
# Username was not updated
self.assertEqual(self.user.username, 'very-original-username')
self.assertEqual(another_user.username, 'thejane')
@responses.activate
def test_user_modified_does_not_allow_duplicate_email_and_username(self):
# Same email as in the webhook payload for another user
another_user = OAuthUserFactory(email='jane@example.com', username='thejane')
body = {
**self.webhook_payload,
'email': 'jane@example.com',
'nickname': 'thejane',
}
with self.assertLogs('users.views.webhooks', level='ERROR') as logs:
response = self.client.post(
self.url, body, content_type='application/json', **prepare_hmac_header(body)
)
self.assertRegex(logs.output[0], 'Unable to update email for')
self.assertRegex(logs.output[1], 'Unable to update username for')
self.assertEqual(response.status_code, 204)
self.assertEqual(response.content, b'')
# Email was not updated
self.assertEqual(self.user.email, 'mail@example.com')
self.assertEqual(another_user.email, 'jane@example.com')
# Username was not updated
self.assertEqual(self.user.username, 'very-original-username')
self.assertEqual(another_user.username, 'thejane')

View File

@ -8,12 +8,6 @@ User = get_user_model()
def mock_blender_id_responses() -> None:
"""Set up mock responses of Blender ID service."""
base_url = settings.BLENDER_ID['BASE_URL']
responses.add(
responses.GET,
f'{base_url}api/user/2/avatar',
status=302,
headers={'Location': f'{base_url}media/cache/1c/da/1cda54d605799b1f4b0dc080.jpg'},
)
responses.add(
responses.GET,
f'{base_url}api/badges/2',
@ -50,7 +44,7 @@ def mock_blender_id_responses() -> None:
with open('common/static/common/images/blank-profile-pic.png', 'rb') as out:
responses.add(
responses.GET,
'http://id.local:8000/media/cache/1c/da/1cda54d605799b1f4b0dc080.jpg',
f'{base_url}media/avatar/fo/ob/foobar_128x128.jpg',
body=out,
stream=True,
)

View File

@ -32,13 +32,16 @@ def user_modified_webhook(request: HttpRequest) -> HttpResponse:
Payload is expected to have the following format:
{
"avatar_changed": false,
"avatar_url": null,
"confirmed_email_at": "2022-09-01T13:47:00+00:00",
"date_deletion_requested": "2020-01-25T09:51:00+00:00",
"email": "newmail@example.com",
"full_name": "John Doe",
"id": 2,
"nickname": "some-nickname",
"old_email": "mail@example.com",
"old_nickname": "some-old-nickname",
"roles": ["role1", "role2"],
"confirmed_email_at": "2022-09-01T13:47:00+00:00",
"date_deletion_requested": "2020-01-25T09:51:00+00:00",
}
"""
hmac_secret = settings.BLENDER_ID['WEBHOOK_USER_MODIFIED_SECRET']
@ -74,6 +77,13 @@ def user_modified_webhook(request: HttpRequest) -> HttpResponse:
logger.exception('Malformed JSON received')
return HttpResponseBadRequest('Malformed JSON')
oauth_user_id = payload['id']
try:
bid.get_oauth_user_info(oauth_user_id)
except ObjectDoesNotExist:
logger.warning(f'Skipping user-modified: no OAuth info found for ID {oauth_user_id}')
return HttpResponse(status=204)
handle_user_modified(payload)
return HttpResponse(status=204)
@ -82,7 +92,7 @@ def user_modified_webhook(request: HttpRequest) -> HttpResponse:
@background()
def handle_user_modified(payload: Dict[Any, Any]) -> None:
"""Handle payload of a user modified webhook, updating User when necessary."""
oauth_user_id = str(payload['id'])
oauth_user_id = payload['id']
try:
oauth_user_info = bid.get_oauth_user_info(oauth_user_id)
except ObjectDoesNotExist:
@ -90,17 +100,20 @@ def handle_user_modified(payload: Dict[Any, Any]) -> None:
return
user = oauth_user_info.user
args = [oauth_user_id, user.pk]
if payload.get('date_deletion_requested'):
user.request_deletion(payload['date_deletion_requested'])
return
for _from, _to in (('email', 'email'), ('nickname', 'username')):
if payload[_from] == getattr(user, _to):
continue
try:
if payload['email'] != user.email:
user.email = payload['email']
user.save(update_fields=['email'])
setattr(user, _to, payload[_from])
user.save(update_fields=[_to])
except IntegrityError:
logger.exception(f'Unable to update email for {user}: duplicate email')
logger.exception('Unable to update %s for OAuth ID=%s pk=%s', _to, *args)
update_fields = set()
if payload['full_name'] != user.full_name:
@ -115,10 +128,7 @@ def handle_user_modified(payload: Dict[Any, Any]) -> None:
user.save(update_fields=update_fields)
if payload.get('avatar_changed') or not user.image:
bid.copy_avatar_from_blender_id(user=user)
# Attempt to update the username
bid.update_username(user, oauth_user_id)
bid.copy_image_from_avatar_url(user=user, avatar_url=payload['avatar_url'])
# Attempt to update the badges
bid.copy_badges_from_blender_id(user=user)