Fix profile photo upload (#2242) (#2280)

Author: gregjkal

config/settings.py

@@ -504,6 +504,7 @@
     MEDIA_BUCKET_NAME = env("MEDIA_BUCKET_NAME", default="changeme")
     AWS_STORAGE_BUCKET_NAME = MEDIA_BUCKET_NAME
     AWS_S3_OBJECT_PARAMETERS = {"CacheControl": "max-age=86400"}
+    AWS_S3_USE_THREADS = False  # boto3 threads deadlock under gevent
     AWS_DEFAULT_ACL = None
     AWS_S3_ENDPOINT_URL = env(
         "AWS_S3_ENDPOINT_URL", default="https://sfo2.digitaloceanspaces.com"

templates/libraries/_library_grid_list_item.html

@@ -10,8 +10,8 @@ <h3 class="pb-2 text-xl md:text-2xl capitalize border-b border-gray-700">
       {% include "libraries/includes/_documentation_link_icon.html" %}
     </div>
       {% for author in library.authors.all %}
-        {% if author.image %}
-          <img src="{{ author.image.url }}" class="inline float-right rounded w-[30px] ml-1" alt="{{ author.display_name }}" />
+        {% if author.profile_image %}
+          <img src="{{ author.profile_image.url }}" class="inline float-right rounded w-[30px] ml-1" alt="{{ author.display_name }}" />
         {% endif %}
       {% endfor %}
     </h3>

templates/news/detail.html

@@ -55,15 +55,17 @@ <h1 class="text-3xl">
         </span>
         {{ entry.title }}</h1>
       <div class="space-x-3 mt-3 flex items-center">
-        {% if entry.author.image %}
+        {% with author_thumb=entry.author.get_thumbnail_url %}
+        {% if author_thumb %}
           <span class="inline-block h-[30px] w-[30px] overflow-hidden rounded border border-gray-400 dark:border-gray-500">
-            <img src="{{ entry.author.image_thumbnail.url }}" alt="{{ entry.author.display_name }}" class="h-full w-full object-cover">
+            <img src="{{ author_thumb }}" alt="{{ entry.author.display_name }}" class="h-full w-full object-cover">
           </span>
         {% else %}
           <span class="inline-block h-[30px] w-[30px] bg-white rounded dark:text-white dark:bg-slate border border-gray-400 dark:border-gray-500">
             <i class="text-2xl fas fa-user ml-1" title="{{ entry.author.display_name }}"></i>
           </span>
         {% endif %}
+        {% endwith %}
         {% if entry.author.display_name %}
           <div class="inline-block p-0 m-0">
             {{ entry.author.display_name }}<br />

templates/news/list.html

@@ -191,9 +191,9 @@ <h2 class="py-0 my-0 text-xl font-semibold mr-4">
                 {% endif %}
               </div>
               <div class="pt-2 mt-4 w-1/3 text-xs text-right">
-                {% if entry.author.image %}
+                {% if entry.author.profile_image %}
                   <span class="inline-block h-[36px] w-[36px] overflow-hidden rounded-lg  ">
-                    <img src="{{ entry.author.image.url }}" alt="{{ entry.author.display_name }}" class="h-full w-full object-cover">
+                    <img src="{{ entry.author.profile_image.url }}" alt="{{ entry.author.display_name }}" class="h-full w-full object-cover">
                   </span>
                 {% else %}
                   <span class="inline-block h-[36px] w-[36px] bg-white rounded-lg dark:text-white dark:bg-slate border">

templates/users/includes/user_profile_image.html

@@ -1,5 +1,5 @@
-{% if user.image %}
-  <img src="{{ user.image.url }}" alt="user" class="inline -mt-1 rounded-sm cursor-pointer w-[30px]" @click="userOpen = !userOpen" />
+{% if user.profile_image %}
+  <img src="{{ user.profile_image.url }}" alt="user" class="inline -mt-1 rounded-sm cursor-pointer w-[30px]" @click="userOpen = !userOpen" />
 {% else %}
   <i class="inline mr-2 cursor-pointer fas fa-user text-charcoal dark:text-white/60" @click="userOpen = !userOpen"></i>
 {% endif %}

templates/users/profile.html

@@ -88,8 +88,8 @@ <h3>{% trans 'Update Profile Photo' %}</h3>
                     {% endfor %}
                   {% endif %}
 
-                  {% if user.image %}
-                    <img src="{{ user.image.url }}" alt="user" class="ml-4 inline -mt-1 rounded bg-white dark:bg-slate w-[30px] mr-2" />
+                  {% if user.profile_image %}
+                    <img src="{{ user.profile_image.url }}" alt="user" class="ml-4 inline -mt-1 rounded bg-white dark:bg-slate w-[30px] mr-2" />
                   {% endif %}
 
                   {% render_field field class='text-sm text-grey-500 !border-0 file:mr-5 file:py-2 file:px-6 file:rounded-lg file:border-0 file:text-sm file:font-medium file:bg-white file:text-slate hover:file:cursor-pointer hover:file:bg-orange hover:file:text-white' %}

users/forms.py

@@ -1,6 +1,7 @@
 import os
 
 from django.contrib.auth import get_user_model
+from django.core.files.uploadedfile import UploadedFile
 from django import forms
 
 from allauth.account.forms import ResetPasswordKeyForm
@@ -136,30 +137,35 @@ def clean(self):
         return cleaned_data
 
     def save(self, commit=True):
-        # Temporarily store the old image
         old_image = self.instance.profile_image
+        old_image_name = old_image.name if old_image else None
+        new_image_data = self.cleaned_data.get("profile_image")
+        has_new_upload = isinstance(new_image_data, UploadedFile)
+
         # Save the new image
         user = super().save(commit=False)
         if not old_image:
             # reset image on image delete checked
             user.image_uploaded = False
-        elif self.cleaned_data["profile_image"] != old_image:
-            # Delete the old image file if there's a new image being uploaded
-            old_image.delete(save=False)
-
-        if new_image := self.cleaned_data.get("profile_image"):
-            _, file_extension = os.path.splitext(new_image.name)
+        elif has_new_upload and old_image_name:
+            # Delete the old file directly from storage (not via FieldFile.delete(),
+            # which closes file handles and interferes with the pending upload)
+            old_image.storage.delete(old_image_name)
 
-            # Strip the leading period from the file extension.
+        if has_new_upload:
+            _, file_extension = os.path.splitext(new_image_data.name)
             file_extension = file_extension.lstrip(".")
-
-            new_image.name = f"{user.profile_image_filename_root}.{file_extension}"
-            user.profile_image = new_image
+            new_image_data.name = f"{user.profile_image_filename_root}.{file_extension}"
+            user.profile_image = new_image_data
             user.image_uploaded = True
 
         if commit:
             user.save()
 
+        # Invalidate the cached thumbnail so ImageKit regenerates it
+        if has_new_upload:
+            user.delete_cached_thumbnail()
+
         return user
 
 

users/models.py

@@ -264,9 +264,24 @@ class User(BaseUser):
     # elapsed.
     delete_permanently_at = models.DateTimeField(null=True, editable=False)
 
+    def delete_cached_thumbnail(self):
+        """Delete the cached ImageKit thumbnail so it regenerates on next access."""
+        if not self.profile_image:
+            return
+        try:
+            from imagekit.cachefiles.backends import CacheFileState
+
+            thumb = self.image_thumbnail
+            if thumb.name:
+                thumb.storage.delete(thumb.name)
+            thumb.cachefile_backend.set_state(thumb, CacheFileState.DOES_NOT_EXIST)
+        except (OSError, AttributeError):
+            logger.debug("Failed to invalidate thumbnail cache", exc_info=True)
+
     def save_image_from_provider(self, avatar_url):
         from django.core.files.base import ContentFile
 
+        self.delete_cached_thumbnail()
         response = requests.get(avatar_url)
         filename = f"{self.profile_image_filename_root}.png"
         self.profile_image.save(filename, ContentFile(response.content), save=True)
@@ -286,7 +301,7 @@ def claim(self):
     def get_thumbnail_url(self):
         # convenience method for templates
         if self.profile_image and self.image_thumbnail:
-            with suppress(AttributeError, MissingSource):
+            with suppress(AttributeError, MissingSource, FileNotFoundError, OSError):
                 return getattr(self.image_thumbnail, "url", None)
 
     def get_avatar_url(self):
@@ -308,7 +323,7 @@ def get_avatar_url(self):
     def get_hq_image_url(self):
         # convenience method for templates
         if self.hq_image and self.hq_image_render:
-            with suppress(AttributeError, MissingSource):
+            with suppress(AttributeError, MissingSource, FileNotFoundError, OSError):
                 return getattr(self.hq_image_render, "url", None)
 
     @property
@@ -340,10 +355,10 @@ def delete_account(self):
         self.last_name = "Doe"
         self.display_name = "John Doe"
         self.email = "deleted-{}@example.com".format(uuid.uuid4())
+        self.delete_cached_thumbnail()
         image = self.profile_image
         transaction.on_commit(lambda: image.delete())
         self.profile_image = None
-        self.image_thumbnail = None
         self.delete_permanently_at = None
         self.save()
 

users/tests/test_forms.py

@@ -202,7 +202,7 @@ def create_test_image_file(filename="test.png"):
         content_type="image/jpeg",
     )
 
-    form = UserProfilePhotoForm({"profile_image": new_image}, instance=user)
+    form = UserProfilePhotoForm({}, {"profile_image": new_image}, instance=user)
     assert form.is_valid()
     updated_user = form.save()
     updated_user.refresh_from_db()