feat: enable stops redesign (attempt #2)

* refactor: enable stops redesign

Switches the feature flag: now we can use it to re-enable the old design.

* fix(StopController): refactor tests to redesign

- fix: enable redirecting child stop IDs to parent stop pages, to match the original stop page
- stop checking for zone number or routes, the new stop page doesn't assign those variables
- the new stop page doesn't render any react content on the server, so remove that test

apps/site/config/config.exs

@@ -30,8 +30,8 @@ config :site, SiteWeb.ViewHelpers, google_tag_manager_id: System.get_env("GOOGLE
 
 config :laboratory,
   features: [
-    {:stops_redesign, "Stops Page Redesign (Q1/Q2 2023)",
-     "Revamping of the Stop pages as part of the 🚉 Website - Stops Page Redesign epic"},
+    {:old_stops_redesign, "Stops Page Redesign (2023)",
+     "Enable this to revert back to the old version."},
     {:force_otp1, "Force OpenTripPlanner v1",
      "Override randomized assignment between OTP instances and force OTP1."},
     {:force_otp2, "Force OpenTripPlanner v2",

apps/site/lib/site_web/controllers/stop_controller.ex

@@ -57,7 +57,7 @@ defmodule SiteWeb.StopController do
 
   @spec show(Plug.Conn.t(), map()) :: Plug.Conn.t()
   def show(conn, %{"id" => stop_id} = params) do
-    if Laboratory.enabled?(conn, :stops_redesign) do
+    if !Laboratory.enabled?(conn, :old_stops_redesign) do
       # TODO: Render relevant template with relevant data!
       # SHOULD return a Plug.Conn, via a template. See:
       # https://hexdocs.pm/phoenix/Phoenix.Controller.html#render/2
@@ -69,19 +69,26 @@ defmodule SiteWeb.StopController do
         |> Repo.get()
 
       if stop do
-        routes_by_stop = Routes.Repo.by_stop(stop_id, include: "stop.connecting_stops")
-
-        conn
-        |> assign(:breadcrumbs, breadcrumbs(stop, routes_by_stop))
-        |> meta_description(stop, routes_by_stop)
-        |> render("show-redesign.html", %{
-          stop_id: stop_id,
-          routes_by_stop: routes_by_stop
-        })
+        if Repo.has_parent?(stop) do
+          conn
+          |> redirect(to: stop_path(conn, :show, Repo.get_parent(stop)))
+          |> halt()
+        else
+          routes_by_stop = Routes.Repo.by_stop(stop_id, include: "stop.connecting_stops")
+
+          conn
+          |> assign(:breadcrumbs, breadcrumbs(stop, routes_by_stop))
+          |> meta_description(stop, routes_by_stop)
+          |> render("show-redesign.html", %{
+            stop_id: stop_id,
+            routes_by_stop: routes_by_stop
+          })
+        end
       else
         check_cms_or_404(conn)
       end
     else
+      # no cover
       show_old(conn, params)
     end
   end

apps/site/test/site_web/controllers/stop_controller_test.exs

@@ -1,21 +1,11 @@
 defmodule SiteWeb.StopControllerTest do
-  use SiteWeb.ConnCase
+  use SiteWeb.ConnCase, async: false
   alias Routes.Route
   alias SiteWeb.StopController
   alias Stops.Stop
   alias Util.Breadcrumb
   import Mock
 
-  test "renders react content server-side", %{conn: conn} do
-    assert [{"div", _, content}] =
-             conn
-             |> get(stop_path(conn, :show, "place-sstat"))
-             |> html_response(200)
-             |> Floki.find("#react-root")
-
-    assert [_ | _] = content
-  end
-
   test "redirects to subway stops on index", %{conn: conn} do
     conn = conn |> get(stop_path(conn, :index))
     assert redirected_to(conn) == stop_path(conn, :show, :subway)
@@ -51,31 +41,10 @@ defmodule SiteWeb.StopControllerTest do
     assert redirected_to(conn) == stop_path(conn, :show, "Four Corners / Geneva")
   end
 
-  test "assigns routes for this stop", %{conn: conn} do
-    conn =
-      conn
-      |> get(stop_path(conn, :show, "place-sstat"))
-
-    assert conn.assigns.routes
-  end
-
-  test "assigns ferry routes", %{conn: conn} do
-    with_mock(Laboratory, [], enabled?: fn _, :stops_redesign -> false end) do
-      conn =
-        conn
-        |> get(stop_path(conn, :show, "Boat-Charlestown"))
-
-      assert [ferry] = conn.assigns.routes
-      assert %{group_name: :ferry, routes: [%{route: %{id: "Boat-F4"}}]} = ferry
-    end
-  end
-
-  test "assigns the zone number for the current stop", %{conn: conn} do
-    conn =
-      conn
-      |> get(stop_path(conn, :show, "place-WML-0442"))
+  test "shows a stop ID", %{conn: conn} do
+    conn = conn |> get(stop_path(conn, :show, "place-sstat"))
 
-    assert conn.assigns.zone_number == "8"
+    assert conn.assigns.stop_id
   end
 
   test "sets a custom meta description for stops", %{conn: conn} do
@@ -149,18 +118,16 @@ defmodule SiteWeb.StopControllerTest do
 
   describe "show/2" do
     test "should set the title and meta description of the page", %{conn: conn} do
-      with_mock(Laboratory, [], enabled?: fn _, :stops_redesign -> true end) do
-        conn =
-          conn
-          |> get(stop_path(conn, :show, "place-wondl"))
+      conn =
+        conn
+        |> get(stop_path(conn, :show, "place-wondl"))
 
-        [_stations, station_name] = conn.assigns.breadcrumbs
+      [_stations, station_name] = conn.assigns.breadcrumbs
 
-        assert "Wonderland" = station_name.text
+      assert "Wonderland" = station_name.text
 
-        assert "Station serving MBTA Subway and Bus lines at 1300 North Shore Rd, Revere, MA 02151." =
-                 conn.assigns.meta_description
-      end
+      assert "Station serving MBTA Subway and Bus lines at 1300 North Shore Rd, Revere, MA 02151." =
+               conn.assigns.meta_description
     end
   end