Merge pull request #293 from pulibrary/207-item-404

Render 404 with the correct status when item is not found
pulibrary · Jan 13, 2025 · 999491f · 999491f
2 parents 31d515a + 9bc483a
commit 999491f
Show file tree

Hide file tree

Showing 10 changed files with 108 additions and 53 deletions.
diff --git a/lib/dpul_collections_web/controllers/error_html.ex b/lib/dpul_collections_web/controllers/error_html.ex
@@ -7,13 +7,13 @@ defmodule DpulCollectionsWeb.ErrorHTML do
   use DpulCollectionsWeb, :html
 
   # If you want to customize your error pages,
-  # uncomment the embed_templates/1 call below
-  # and add pages to the error directory:
+  # add pages to the error directory, e.g.:
   #
   #   * lib/dpul_collections_web/controllers/error_html/404.html.heex
   #   * lib/dpul_collections_web/controllers/error_html/500.html.heex
   #
-  # embed_templates "error_html/*"
+  # they are used via the embed_templates/1 call below
+  embed_templates "error_html/*"
 
   # The default is to render a plain text page based on
   # the template name. For example, "404.html" becomes

diff --git a/lib/dpul_collections_web/controllers/error_html/404.html.heex b/lib/dpul_collections_web/controllers/error_html/404.html.heex
@@ -0,0 +1,40 @@
+<%!-- Note that we do not render templates through our application layout.
+    This is to avoid circular errors. --%>
+<!DOCTYPE html>
+<html lang="en" class="[scrollbar-gutter:stable]">
+  <head>
+    <meta charset="utf-8" />
+    <meta name="viewport" content="width=device-width, initial-scale=1" />
+    <title>DpulCollections</title>
+    <link rel="stylesheet" href="/assets/app.css" />
+    <link rel="stylesheet" href="https://unpkg.com/[email protected]/dist/style.css" />
+    <script src="https://unpkg.com/vue@3/dist/vue.global.js">
+    </script>
+    <script src="https://unpkg.com/[email protected]/dist/lux-styleguidist.iife.js">
+    </script>
+    <script>
+      document.addEventListener("DOMContentLoaded", function(){
+        const { createApp } = Vue
+        createApp().use(Lux.default).mount('#app')
+      });
+    </script>
+  </head>
+
+  <body class="bg-white antialiased">
+    <div class="flex flex-col min-h-screen" id="app">
+      <%= DpulCollectionsWeb.LuxComponents.header(assigns) %>
+      <div class="flex-1">
+        <main class="max-w-7xl mx-auto py-6 px-4 sm:px-6 lg:px-8">
+          <div class="grid grid-flow-row auto-rows-max gap-20">
+            <div id="error" class="grid place-self-center gap-10 max-w-prose">
+              <p class="text-xl text-center">
+                <%= gettext("Page not found") %>
+              </p>
+            </div>
+          </div>
+        </main>
+      </div>
+      <%= DpulCollectionsWeb.LuxComponents.footer(assigns) %>
+    </div>
+  </body>
+</html>
diff --git a/lib/dpul_collections_web/live/item_live.ex b/lib/dpul_collections_web/live/item_live.ex
@@ -17,17 +17,12 @@ defmodule DpulCollectionsWeb.ItemLive do
     push_patch(socket, to: item.url, replace: true)
   end
 
-  defp build_socket(socket, item, _) do
-    assign(socket, item: item)
+  defp build_socket(_, nil, _) do
+    raise DpulCollectionsWeb.ItemLive.NotFoundError
   end
 
-  # Render a message if no item was found in Solr.
-  def render(assigns) when is_nil(assigns.item) do
-    ~H"""
-    <div class="my-5 grid grid-flow-row auto-rows-max gap-10">
-      <span><%= gettext("Item not found") %></span>
-    </div>
-    """
+  defp build_socket(socket, item, _) do
+    assign(socket, item: item)
   end
 
   def render(assigns) do

diff --git a/lib/dpul_collections_web/live/item_live/not_found_error.ex b/lib/dpul_collections_web/live/item_live/not_found_error.ex
@@ -0,0 +1,3 @@
+defmodule DpulCollectionsWeb.ItemLive.NotFoundError do
+  defexception message: "Item not found", plug_status: 404
+end
diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot
@@ -52,7 +52,6 @@ msgstr ""
 msgid "close"
 msgstr ""
 
-#: lib/dpul_collections_web/live/item_live.ex:54
 #: lib/dpul_collections_web/live/item_live.ex:95
 #, elixir-autogen, elixir-format
 msgid "Download"
@@ -63,11 +62,6 @@ msgstr ""
 msgid "Apply"
 msgstr ""
 
-#: lib/dpul_collections_web/live/item_live.ex:28
-#, elixir-autogen, elixir-format
-msgid "Item not found"
-msgstr ""
-
 #: lib/dpul_collections_web/components/lux-components.ex:16
 #, elixir-autogen, elixir-format
 msgid "Language"
@@ -129,17 +123,27 @@ msgstr ""
 msgid "We invite you to be inspired by our globally diverse collections of"
 msgstr ""
 
-#: lib/dpul_collections_web/components/lux-components.ex:33
+#: lib/dpul_collections_web/components/lux-components.ex:37
 #, elixir-autogen, elixir-format
 msgid "Accessibility Help"
 msgstr ""
 
-#: lib/dpul_collections_web/components/lux-components.ex:31
+#: lib/dpul_collections_web/components/lux-components.ex:35
 #, elixir-autogen, elixir-format
 msgid "Copyright Policy"
 msgstr ""
 
-#: lib/dpul_collections_web/components/lux-components.ex:32
+#: lib/dpul_collections_web/components/lux-components.ex:36
 #, elixir-autogen, elixir-format
 msgid "Privacy Notice"
 msgstr ""
+
+#: lib/dpul_collections_web/live/item_live.ex:53
+#, elixir-autogen, elixir-format
+msgid "Download PDF"
+msgstr ""
+
+#: lib/dpul_collections_web/controllers/error_html/404.html.heex:31
+#, elixir-autogen, elixir-format
+msgid "Page not found"
+msgstr ""
diff --git a/priv/gettext/en/LC_MESSAGES/default.po b/priv/gettext/en/LC_MESSAGES/default.po
@@ -52,7 +52,6 @@ msgstr ""
 msgid "close"
 msgstr ""
 
-#: lib/dpul_collections_web/live/item_live.ex:54
 #: lib/dpul_collections_web/live/item_live.ex:95
 #, elixir-autogen, elixir-format
 msgid "Download"
@@ -63,11 +62,6 @@ msgstr ""
 msgid "Apply"
 msgstr ""
 
-#: lib/dpul_collections_web/live/item_live.ex:28
-#, elixir-autogen, elixir-format
-msgid "Item not found"
-msgstr ""
-
 #: lib/dpul_collections_web/components/lux-components.ex:16
 #, elixir-autogen, elixir-format
 msgid "Language"
@@ -129,17 +123,27 @@ msgstr ""
 msgid "We invite you to be inspired by our globally diverse collections of"
 msgstr ""
 
-#: lib/dpul_collections_web/components/lux-components.ex:33
+#: lib/dpul_collections_web/components/lux-components.ex:37
 #, elixir-autogen, elixir-format
 msgid "Accessibility Help"
 msgstr ""
 
-#: lib/dpul_collections_web/components/lux-components.ex:31
+#: lib/dpul_collections_web/components/lux-components.ex:35
 #, elixir-autogen, elixir-format
 msgid "Copyright Policy"
 msgstr ""
 
-#: lib/dpul_collections_web/components/lux-components.ex:32
+#: lib/dpul_collections_web/components/lux-components.ex:36
 #, elixir-autogen, elixir-format
 msgid "Privacy Notice"
 msgstr ""
+
+#: lib/dpul_collections_web/live/item_live.ex:53
+#, elixir-autogen, elixir-format, fuzzy
+msgid "Download PDF"
+msgstr ""
+
+#: lib/dpul_collections_web/controllers/error_html/404.html.heex:31
+#, elixir-autogen, elixir-format, fuzzy
+msgid "Page not found"
+msgstr ""
diff --git a/priv/gettext/es/LC_MESSAGES/default.po b/priv/gettext/es/LC_MESSAGES/default.po
@@ -52,7 +52,6 @@ msgstr "Nosotros no podemos encontrar internet"
 msgid "close"
 msgstr "cerrar"
 
-#: lib/dpul_collections_web/live/item_live.ex:54
 #: lib/dpul_collections_web/live/item_live.ex:95
 #, elixir-autogen, elixir-format
 msgid "Download"
@@ -63,11 +62,6 @@ msgstr "Descargar"
 msgid "Apply"
 msgstr "Aplicar"
 
-#: lib/dpul_collections_web/live/item_live.ex:28
-#, elixir-autogen, elixir-format
-msgid "Item not found"
-msgstr "Artículo no encontrado"
-
 #: lib/dpul_collections_web/components/lux-components.ex:16
 #, elixir-autogen, elixir-format
 msgid "Language"
@@ -129,17 +123,27 @@ msgstr "Artículos efímeros. Estamos ansiosos por ver cómo utilizas estos mate
 msgid "We invite you to be inspired by our globally diverse collections of"
 msgstr "Te invitamos a inspirarte con nuestras colecciones globalmente diversas de"
 
-#: lib/dpul_collections_web/components/lux-components.ex:33
+#: lib/dpul_collections_web/components/lux-components.ex:37
 #, elixir-autogen, elixir-format
 msgid "Accessibility Help"
 msgstr "Ayuda de accesibilidad"
 
-#: lib/dpul_collections_web/components/lux-components.ex:31
+#: lib/dpul_collections_web/components/lux-components.ex:35
 #, elixir-autogen, elixir-format
 msgid "Copyright Policy"
 msgstr "Política de derechos de autor"
 
-#: lib/dpul_collections_web/components/lux-components.ex:32
+#: lib/dpul_collections_web/components/lux-components.ex:36
 #, elixir-autogen, elixir-format
 msgid "Privacy Notice"
 msgstr "Aviso de privacidad"
+
+#: lib/dpul_collections_web/live/item_live.ex:53
+#, elixir-autogen, elixir-format, fuzzy
+msgid "Download PDF"
+msgstr "Descargar PDF"
+
+#: lib/dpul_collections_web/controllers/error_html/404.html.heex:31
+#, elixir-autogen, elixir-format, fuzzy
+msgid "Page not found"
+msgstr "Página no encontrada"
diff --git a/test/dpul_collections_web/controllers/error_html_test.exs b/test/dpul_collections_web/controllers/error_html_test.exs
@@ -5,7 +5,7 @@ defmodule DpulCollectionsWeb.ErrorHTMLTest do
   import Phoenix.Template
 
   test "renders 404.html" do
-    assert render_to_string(DpulCollectionsWeb.ErrorHTML, "404", "html", []) == "Not Found"
+    assert render_to_string(DpulCollectionsWeb.ErrorHTML, "404", "html", []) =~ "not found"
   end
 
   test "renders 500.html" do

diff --git a/test/dpul_collections_web/live/item_live_test.exs b/test/dpul_collections_web/live/item_live_test.exs
@@ -54,11 +54,6 @@ defmodule DpulCollectionsWeb.ItemLiveTest do
     assert redirected_to(conn, 302) == "/i/învăţămîntul-trebuie-urmărească-dez/item/1"
   end
 
-  test "/item/{:id} does not redirect with a bad id", %{conn: conn} do
-    conn = get(conn, "/item/badid1")
-    assert conn.status == 200
-  end
-
   test "/i/{:slug}/item/{:id} redirects when slug is incorrect",
        %{conn: conn} do
     conn = get(conn, "/i/not-a-real-slug/item/1")
@@ -131,15 +126,15 @@ defmodule DpulCollectionsWeb.ItemLiveTest do
     assert response =~ "زلزلہ"
   end
 
-  test "/i/{:slug}/item/{:id} does not redirect with a bad id", %{conn: conn} do
-    conn = get(conn, "/i/not-a-real-slug/item/badid1")
-    response = html_response(conn, 200)
-    assert response =~ "Item not found"
+  test "/i/{:slug}/item/{:id} 404s with a bad id", %{conn: conn} do
+    assert_error_sent 404, fn ->
+      get(conn, "/i/not-a-real-slug/item/badid1")
+    end
   end
 
-  test "GET /item/{:id} response whith a bad id", %{conn: conn} do
-    conn = get(conn, "/item/badid1")
-    response = html_response(conn, 200)
-    assert response =~ "Item not found"
+  test "GET /item/{:id} 404s with a bad id", %{conn: conn} do
+    assert_error_sent 404, fn ->
+      get(conn, "/item/badid1")
+    end
   end
 end
diff --git a/test/dpul_collections_web/live/search_live_test.exs b/test/dpul_collections_web/live/search_live_test.exs
@@ -42,6 +42,16 @@ defmodule DpulCollectionsWeb.SearchLiveTest do
            |> Enum.any?()
   end
 
+  test "GET /search with a query that has no results", %{conn: conn} do
+    {:ok, view, _html} = live(conn, "/search?q=therewontbeanyresults")
+
+    assert view
+           |> has_element?(
+             "#item-counter",
+             "No items found"
+           )
+  end
+
   test "GET /search renders thumbnails for each resource", %{conn: conn} do
     {:ok, view, _html} = live(conn, "/search?")