Skip to content

Return friendly page for missing embed slugs#600

Open
skaphan wants to merge 4 commits intoartoonie:mainfrom
skaphan:friendly-embed-404
Open

Return friendly page for missing embed slugs#600
skaphan wants to merge 4 commits intoartoonie:mainfrom
skaphan:friendly-embed-404

Conversation

@skaphan
Copy link

@skaphan skaphan commented Mar 4, 2026

Summary

  • When an embedded visualization URL (/ve/<slug>) references a slug that no longer exists, return a user-friendly HTML page (200) instead of Django's default 404
  • This displays cleanly inside iframes, avoiding cross-origin error handling issues that make the default 404 invisible to embedding pages
  • Cache-Control headers prevent the friendly page from being cached, so the real visualization appears immediately once re-created

Test plan

  • Added test_embedded_404_returns_friendly_page to testSimple.py
  • Verified manually: embed iframe shows friendly message instead of blank/broken page when slug is missing

🤖 Generated with Claude Code

When an embedded visualization URL references a slug that no longer
exists, return a user-friendly HTML page (200) instead of Django default
404. This displays cleanly inside iframes and avoids cross-origin error
handling issues. Cache headers prevent the response from being cached so
the real visualization appears once re-created.

Co-Authored-By: Claude Opus 4.6
@artoonie
Copy link
Owner

artoonie commented Mar 4, 2026

Should we still return an error code 404, even with a custom 404 page? Would that display cleanly in iframes? I fear returning a 200 OK for a nonexistent page.

@skaphan
Copy link
Author

skaphan commented Mar 4, 2026 via email

@artoonie
Copy link
Owner

artoonie commented Mar 4, 2026

I'm not sure I fully understand why returning the same content with 200 vs 404 acts any differently in an iframe. My understanding is that it will work the same way. The only differences are in how Cloudflare and crawlers treat the page. (If I return 200, that page might show up in search engines if there are broken links from other websites, for example.)

Can you test the small change I made and see if this does everything you need it to?

@skaphan
Copy link
Author

skaphan commented Mar 5, 2026

Tested — the friendly HTML renders correctly in iframes with 404 status. You're right that the status code doesn't affect iframe rendering. Good call on using 404 for Cloudflare and crawlers. LGTM, please merge.

@artoonie artoonie mentioned this pull request Mar 5, 2026
@artoonie
Copy link
Owner

artoonie commented Mar 5, 2026

Running Heroku CI on #606

@artoonie
Copy link
Owner

artoonie commented Mar 5, 2026

Looks like it's failing two tests:

FAIL: test_create_scrapable_page (electionpage.tests.ElectionPageTests.test_create_scrapable_page)
With proper permissions, can create a scrapable election page
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/app/electionpage/tests.py", line 228, in test_create_scrapable_page
    self.assertIsNone(submit_with_num_elections_and_get_error('2', True))
    ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: '' is not None

and

FAIL: test_embedded_404_returns_friendly_page (visualizer.tests.testSimple.SimpleTests.test_embedded_404_returns_friendly_page)
When an embedded visualization slug doesn't exist, return a friendly
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/app/visualizer/tests/testSimple.py", line 302, in test_embedded_404_returns_friendly_page
    self.assertEqual(response.status_code, 200)
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 404 != 200

The view now correctly returns 404 with friendly HTML content.
Update the test to match.

Co-Authored-By: Claude Opus 4.6
@skaphan
Copy link
Author

skaphan commented Mar 5, 2026

Fixed the second error which was triggered by the change to returning 404 status. The first one (electionpage) appears to be unrelated. Not sure what is causing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants