Skip to content

Organizations: allow uploading avatar #12254

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ media/json
media/man
media/pdf
media/static
media/usercontent
/static
node_modules
nodeenv
Expand Down
5 changes: 5 additions & 0 deletions dockerfiles/nginx/web.conf.template
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ server {
break;
}

location /usercontent/ {
proxy_pass http://storage:9000/usercontent/;
break;
}

location / {
proxy_pass http://web:8000/;
proxy_set_header X-Forwarded-Host $host;
Expand Down
19 changes: 18 additions & 1 deletion readthedocs/organizations/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from django.core.validators import EmailValidator
from django.db.models import Q
from django.utils.translation import gettext_lazy as _
from PIL import Image

from readthedocs.core.history import SimpleHistoryModelForm
from readthedocs.core.permissions import AdminPermission
Expand Down Expand Up @@ -36,7 +37,7 @@ class OrganizationForm(SimpleHistoryModelForm):

class Meta:
model = Organization
fields = ["name", "email", "description", "url"]
fields = ["name", "email", "avatar", "description", "url"]
labels = {
"name": _("Organization Name"),
"email": _("Billing Email"),
Expand Down Expand Up @@ -78,6 +79,22 @@ def clean_name(self):
)
return name

def clean_avatar(self):
avatar = self.cleaned_data.get("avatar")
if avatar:
if avatar.size > 750 * 1024:
raise forms.ValidationError(
_("Avatar image size must not exceed 750KB."),
)
try:
img = Image.open(avatar)
except Exception:
raise ValidationError("Could not process image. Please upload a valid image file.")
width, height = img.size
if width > 500 or height > 500:
raise ValidationError("The image dimensions cannot exceed 500x500 pixels.")
return avatar


class OrganizationSignupFormBase(OrganizationForm):
"""
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# Generated by Django 5.2.3 on 2025-06-17 22:20
import django.core.validators
from django.db import migrations
from django.db import models
from django_safemigrate import Safe

import readthedocs.organizations.models


class Migration(migrations.Migration):
safe = Safe.before_deploy()

dependencies = [
("organizations", "0015_remove_unused_indexes"),
("projects", "0148_remove_unused_indexes"),
]

operations = [
migrations.AddField(
model_name="historicalorganization",
name="avatar",
field=models.TextField(
blank=True,
help_text="Avatar for your organization (JPG or PNG format, max 500x500px, 750KB)",
max_length=100,
null=True,
validators=[
django.core.validators.FileExtensionValidator(
allowed_extensions=["jpg", "jpeg", "png"]
)
],
verbose_name="Avatar",
),
),
migrations.AddField(
model_name="organization",
name="avatar",
field=models.ImageField(
blank=True,
help_text="Avatar for your organization (JPG or PNG format, max 500x500px, 750KB)",
null=True,
storage=readthedocs.organizations.models._get_user_content_storage,
upload_to=readthedocs.organizations.models._upload_organization_avatar_to,
validators=[
django.core.validators.FileExtensionValidator(
allowed_extensions=["jpg", "jpeg", "png"]
)
],
verbose_name="Avatar",
),
),
migrations.AlterField(
model_name="organization",
name="projects",
field=models.ManyToManyField(
blank=True,
related_name="organizations",
to="projects.project",
verbose_name="Projects",
),
),
]
73 changes: 73 additions & 0 deletions readthedocs/organizations/models.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
"""Organizations models."""

from pathlib import Path
from uuid import uuid4

import structlog
from autoslug import AutoSlugField
from django.contrib.auth.models import User
from django.contrib.contenttypes.fields import GenericRelation
from django.contrib.contenttypes.models import ContentType
from django.core.files.storage import storages
from django.core.validators import FileExtensionValidator
from django.db import models
from django.urls import reverse
from django.utils.crypto import salted_hmac
from django.utils.translation import gettext_lazy as _
from django_gravatar.helpers import get_gravatar_url
from djstripe.enums import SubscriptionStatus

from readthedocs.core.history import ExtraHistoricalRecords
Expand All @@ -26,6 +32,36 @@
log = structlog.get_logger(__name__)


def _upload_organization_avatar_to(instance, filename):
"""
Generate the upload path for the organization avatar.

The name of the file is an UUID, and the extension is preserved.
If the instance already has an avatar, we use its name to keep the same UUID.
"""
extension = filename.split(".")[-1].lower()
try:
previous_avatar = Organization.objects.get(pk=instance.pk).avatar
except Organization.DoesNotExist:
previous_avatar = None

if not previous_avatar:
uuid = uuid4().hex
else:
uuid = Path(previous_avatar.name).stem
return f"avatars/organizations/{uuid}.{extension}"


def _get_user_content_storage():
"""
Get the storage for user content.

Use a function for storage instead of directly assigning the instance
to avoid hardcoding the backend in the migration file.
"""
return storages["usercontent"]


class Organization(models.Model):
"""Organization model."""

Expand All @@ -38,6 +74,7 @@ class Organization(models.Model):
"projects.Project",
verbose_name=_("Projects"),
related_name="organizations",
blank=True,
)
owners = models.ManyToManyField(
User,
Expand Down Expand Up @@ -129,6 +166,16 @@ class Organization(models.Model):
object_id_field="attached_to_id",
)

avatar = models.ImageField(
_("Avatar"),
upload_to=_upload_organization_avatar_to,
storage=_get_user_content_storage,
validators=[FileExtensionValidator(allowed_extensions=["jpg", "jpeg", "png"])],
blank=True,
null=True,
help_text="Avatar for your organization (JPG or PNG format, max 500x500px, 750KB)",
)

# Managers
objects = OrganizationQuerySet.as_manager()
history = ExtraHistoricalRecords()
Expand Down Expand Up @@ -190,6 +237,14 @@ def save(self, *args, **kwargs):
if self.stripe_customer:
self.stripe_id = self.stripe_customer.id

# If the avatar is being changed, delete the previous one.
try:
previous_avatar = Organization.objects.get(pk=self.pk).avatar
except Organization.DoesNotExist:
previous_avatar = None
if previous_avatar and previous_avatar != self.avatar:
previous_avatar.delete(save=False)

super().save(*args, **kwargs)

def get_stripe_metadata(self):
Expand All @@ -214,6 +269,24 @@ def add_member(self, user, team):
member = TeamMember.objects.create(team=team, member=user)
return member

def get_avatar_url(self):
"""
Get the URL of the organization's avatar.

Use the `avatar` field if it exists, otherwise use
the gravatar from the organization's email.
"""
if self.avatar:
return self.avatar.url
return get_gravatar_url(self.email, size=100)

def delete(self, *args, **kwargs):
"""Override delete method to clean up related resources."""
# Delete the avatar file.
if self.avatar:
self.avatar.delete(save=False)
super().delete(*args, **kwargs)


class OrganizationOwner(models.Model):
"""Intermediate table for Organization <-> User relationships."""
Expand Down
103 changes: 103 additions & 0 deletions readthedocs/organizations/tests/test_views.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import csv
from django.core.files.uploadedfile import SimpleUploadedFile
import io
import itertools
from unittest import mock

Expand All @@ -9,6 +11,7 @@
from django.urls import reverse
from django.utils import timezone
from django_dynamic_fixture import get
from PIL import Image

from readthedocs.audit.models import AuditLog
from readthedocs.core.utils import slugify
Expand Down Expand Up @@ -59,6 +62,106 @@ def test_update(self):
# The slug hasn't changed.
self.assertEqual(self.organization.slug, org_slug)

def _create_image(self, size=(100, 100), format='PNG'):
"""Helper to create an in-memory image file."""
image = Image.new(mode='RGB', size=size, color=(0, 0, 0))
image_bytes = io.BytesIO()
image.save(image_bytes, format=format)
image_bytes.seek(0)
return image_bytes

def test_update_avatar(self):
avatar_file = SimpleUploadedFile(
name='test.png',
content=self._create_image(size=(100, 100)).read(),
content_type='image/png'
)

response = self.client.post(
reverse("organization_edit", args=[self.organization.slug]),
{
"name": "New name",
"email": "dev@example.com",
"description": "Description",
"url": "https://readthedocs.org",
"avatar": avatar_file,
},
)
assert response.status_code == 302
self.organization.refresh_from_db()
assert self.organization.avatar
assert self.organization.avatar.name.startswith("avatars/organizations/")
assert self.organization.avatar.name.endswith(".png")

def test_update_avatar_invalid_dimensions(self):
avatar_file = SimpleUploadedFile(
name='test.png',
content=self._create_image(size=(1000, 1000)).read(),
content_type='image/png'
)

response = self.client.post(
reverse("organization_edit", args=[self.organization.slug]),
{
"name": "New name",
"email": "dev@example.com",
"description": "Description",
"url": "https://readthedocs.org",
"avatar": avatar_file,
},
)
assert response.status_code == 200
form = response.context_data['form']
assert not form.is_valid()
assert 'avatar' in form.errors
assert "The image dimensions cannot exceed" in form.errors['avatar'][0]

def test_update_avatar_invalid_image(self):
avatar_file = SimpleUploadedFile(
name='test.txt',
content=b'This is not an image file.',
content_type='text/plain'
)

response = self.client.post(
reverse("organization_edit", args=[self.organization.slug]),
{
"name": "New name",
"email": "dev@example.com",
"description": "Description",
"url": "https://readthedocs.org",
"avatar": avatar_file,
},
)
assert response.status_code == 200
form = response.context_data['form']
assert not form.is_valid()
assert 'avatar' in form.errors
assert "Upload a valid image." in form.errors['avatar'][0]

def test_update_avatar_invalid_extension(self):
avatar_file = SimpleUploadedFile(
name='test.gif',
content=self._create_image(size=(100, 100), format='GIF').read(),
content_type='image/gif'
)

response = self.client.post(
reverse("organization_edit", args=[self.organization.slug]),
{
"name": "New name",
"email": "dev@example.com",
"description": "Description",
"url": "https://readthedocs.org",
"avatar": avatar_file,
},
)
assert response.status_code == 200
form = response.context_data['form']
assert not form.is_valid()
assert 'avatar' in form.errors
assert "File extension “gif” is not allowed" in form.errors['avatar'][0]

def test_change_name(self):
"""
Changing the name of the organization won't change the slug.
Expand Down
8 changes: 8 additions & 0 deletions readthedocs/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import subprocess

import structlog
from pathlib import Path
from celery.schedules import crontab
from corsheaders.defaults import default_headers
from django.conf.global_settings import PASSWORD_HASHERS
Expand Down Expand Up @@ -1062,6 +1063,13 @@ def STORAGES(self):
"staticfiles": {
"BACKEND": "readthedocs.storage.s3_storage.S3StaticStorage"
},
"usercontent": {
"BACKEND": "django.core.files.storage.FileSystemStorage",
"OPTIONS": {
"location": Path(self.MEDIA_ROOT) / "usercontent",
"allow_overwrite": True,
}
},
}

@property
Expand Down
Loading