Skip to content

fix broken images in subscription emails - #3737

Open
shanbady wants to merge 7 commits into
mainfrom
shanbady/broken-email-image-fallback
Open

fix broken images in subscription emails#3737
shanbady wants to merge 7 commits into
mainfrom
shanbady/broken-email-image-fallback

Conversation

@shanbady

@shanbady shanbady commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

What are the relevant tickets?

Closes https://github.com/mitodl/hq/issues/12708

Description (What does it do?)

This PR ensures that images in emails properly fallback instead of appearing broken. There does not appear to be a single solution that works client-side and is supported by all mail clients. The proper fix in this case is to do a HEAD request - set the default (on the backend) if it broken.

How can this be tested?

  1. checkout main
  2. run the following script which generates a resource with an invalid image url and then sends a subscriptoion email with it:
import datetime

from django.conf import settings
from django.contrib.auth import get_user_model
from django.db import transaction

from learning_resources.models import LearningResource, LearningResourceImage
from learning_resources_search.api import percolate_matches_for_document
from learning_resources_search.models import PercolateQuery
from learning_resources_search.tasks import send_subscription_emails
from main.celery import app
from main.utils import now_in_utc
from profiles.models import Profile

User = get_user_model()
DEAD_IMAGE_URL = "https://ocw.mit.edu/images/this-image-was-removed-404.jpg"

# run the celery task inline, and print emails instead of sending them
app.conf.task_always_eager = True
settings.NOTIFICATION_EMAIL_BACKEND = "django.core.mail.backends.console.EmailBackend"


class Rollback(Exception):
    """Raised to roll the transaction back once the demo has run."""


try:
    with transaction.atomic():
        since = now_in_utc() - datetime.timedelta(days=1)
        recent = LearningResource.objects.filter(published=True, created_on__gt=since)

        # Two resources matching the SAME query, so both land in one digest
        chosen, query = [], None
        for resource in recent.select_related("image"):
            matches = percolate_matches_for_document(resource.id).filter(
                source_type=PercolateQuery.CHANNEL_SUBSCRIPTION_TYPE
            )
            if not resource.image:
                continue
            if query is None:
                query = matches.filter(users__isnull=False).first()
                if query:
                    chosen.append(resource)
            elif matches.filter(id=query.id).exists():
                chosen.append(resource)
            if len(chosen) == 2:
                break

        if len(chosen) < 2:
            raise SystemExit(
                "Could not find two recently-created resources that percolate-match "
                "a channel subscription query. Load fresh data and reindex, then retry."
            )

        keep, break_image_on = chosen
        print(f"\nUsing subscription query {query.id}: {query.original_query}\n")
        print(f"  intact image : {keep.title[:60]}")
        print(f"                 {keep.image.url[:90]}")
        print(f"  broken image : {break_image_on.title[:60]}")
        print(f"                 {DEAD_IMAGE_URL}\n")

        # Point one resource at a dead image URL
        break_image_on.image = LearningResourceImage.objects.create(
            url=DEAD_IMAGE_URL, alt=break_image_on.title
        )
        break_image_on.save()

        # Keep the digest to just these two resources
        recent.exclude(id__in=[keep.id, break_image_on.id]).update(
            created_on=since - datetime.timedelta(days=30)
        )

        # Send to a throwaway subscriber. EVERY query matching either resource
        # must be reassigned, not just the one we picked, or the digest goes to
        # real subscribers and prints their addresses.
        subscriber = User.objects.create(
            username="digest-preview-user", email="reviewer@example.com"
        )
        Profile.objects.create(user=subscriber, email_optin=True)
        matching_query_ids = {
            match.id
            for resource in (keep, break_image_on)
            for match in percolate_matches_for_document(resource.id)
        }
        for match in PercolateQuery.objects.filter(id__in=matching_query_ids):
            match.users.set([subscriber])

        print("=" * 78)
        print("Running send_subscription_emails - email printed below")
        print("=" * 78)

        send_subscription_emails.apply(("channel_subscription_type",), {"period": "daily"})

        raise Rollback
except Rollback:
    print("\n" + "=" * 78)
    print("Transaction rolled back - no data was changed, no email was sent.")
    print("=" * 78)
  1. copy and paste the generated email content (copy from opening to closing not the text version) and save it as email.html and view it in the browser
  2. note that the image is broken
  3. checkout this branch and note it resolves

Additional Context

There is a codeql failure in this PR that is being addressed separately as part of https://github.com/mitodl/hq/issues/12773

@shanbady shanbady changed the title Shanbady/broken email image fallback fix broken images in subscription emails Aug 6, 2026
Comment on lines +248 to +250
response = requests.head(
url, timeout=settings.REQUESTS_TIMEOUT, allow_redirects=True
)
Comment on lines +256 to +258
response = requests.get(
url, timeout=settings.REQUESTS_TIMEOUT, stream=True
)
…uest forgery'

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown

OpenAPI Changes

No changes detected

View full changelog

Unexpected changes? Ensure your branch is up-to-date with main (consider rebasing).

Comment thread learning_resources/utils.py Fixed
Comment thread learning_resources/utils.py Fixed
@shanbady shanbady added Needs Review An open Pull Request that is ready for review and removed Work in Progress labels Aug 9, 2026
@shanbady
shanbady marked this pull request as ready for review August 9, 2026 19:54
Copilot AI balanced review requested due to automatic review settings August 9, 2026 19:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds backend validation and fallback handling for broken images in subscription emails.

Changes:

  • Adds cached image URL reachability checks.
  • Uses default images for unreachable email thumbnails.
  • Schedules pruning and reindexing for unreachable resource images.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
main/settings_celery.py Schedules weekly image pruning.
learning_resources/utils.py Adds cached URL reachability checks.
learning_resources/utils_test.py Tests reachability and caching.
learning_resources/tasks.py Prunes unreachable images and reindexes resources.
learning_resources/tasks_test.py Tests image pruning behavior.
learning_resources_search/tasks.py Applies fallback images to subscription emails.
learning_resources_search/tasks_test.py Tests email image fallback behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

url, timeout=settings.REQUESTS_TIMEOUT, stream=True
)
response.close()
reachable = response.ok
)
removed = 0
for image in LearningResourceImage.objects.filter(id__in=image_ids).iterator():
if image_url_is_reachable(image.url):
Comment on lines +864 to +868
image_ids = (
LearningResource.objects.filter(published=True, image__isnull=False)
.values_list("image_id", flat=True)
.distinct()
)
Comment on lines +870 to +871
for image in LearningResourceImage.objects.filter(id__in=image_ids).iterator():
if image_url_is_reachable(image.url):
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Review An open Pull Request that is ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants