Make notification navigation reliable

assets/js/hooks.js

@@ -1037,39 +1037,6 @@ export const mountStaticAidMaps = root => {
 }
 
 export const Hooks = {
-  NotificationLink: {
-    mounted() {
-      this.markOpened = event => {
-        if (event.defaultPrevented || event.button > 1) return
-
-        const openUrl = this.el.dataset.openUrl
-        const csrfToken = document.querySelector("meta[name='csrf-token']")?.content
-        if (!openUrl || !csrfToken) return
-
-        // Do not await this request. The link owns navigation, while keepalive
-        // lets the small read-marker request finish during LiveView or full-page
-        // navigation without making the destination wait for it.
-        fetch(openUrl, {
-          method: "POST",
-          credentials: "same-origin",
-          keepalive: true,
-          headers: {
-            // This endpoint is protected by the regular browser pipeline,
-            // which accepts HTML requests. The 204 response has no body.
-            "accept": "text/html",
-            "x-csrf-token": csrfToken,
-          },
-        }).catch(() => {})
-      }
-
-      this.el.addEventListener("click", this.markOpened, {capture: true})
-    },
-
-    destroyed() {
-      this.el.removeEventListener("click", this.markOpened, {capture: true})
-    },
-  },
-
   SearchableCategoryPicker: {
     mounted() {
       this.refresh = () => {

e2e/tests/activity-moderation.spec.ts

@@ -189,7 +189,9 @@ test("activity approval, privacy controls, reporting, and moderation work end to
   await gotoLiveView(participant.page, activityURL);
   await waitForMapReady(participant.page);
   await participant.page.getByRole("button", { name: "Block organizer" }).click();
-  await expect(participant.page).toHaveURL(/\/activities$/);
+  await expect
+    .poll(() => new URL(participant.page.url()).pathname)
+    .toBe("/activities");
   await waitForMapReady(participant.page);
   await gotoLiveView(participant.page, "/profile");
   const blockedUsers = participant.page.getByRole("heading", { name: "Blocked users" }).locator("..");
@@ -237,36 +239,40 @@ test("activity approval, privacy controls, reporting, and moderation work end to
   await expect(admin.page.getByRole("heading", { name: "Trust and safety" })).toBeVisible();
 
   const reportCard = admin.page
-    .locator("article")
+    .locator("details")
     .filter({ hasText: reportDescription });
+  await reportCard.locator("summary").click();
   await reportCard.getByRole("button", { name: "View scoped evidence" }).click();
   const evidence = admin.page
     .getByRole("heading", { name: "Reported evidence" })
     .locator("xpath=ancestor::section");
   await expect(evidence.getByText(organizerMessageText)).toBeVisible();
   await evidence.getByRole("button", { name: "Close" }).click();
-  await reportCard.getByRole("combobox").selectOption("resolved");
-  await reportCard.getByPlaceholder("Resolution note").fill("Reviewed in isolated browser E2E.");
-  await reportCard.getByRole("button", { name: "Save" }).click();
+  await reportCard.locator("summary").click();
+  await reportCard.getByRole("combobox", { name: "Decision" }).selectOption("resolved");
+  await reportCard.getByLabel("Resolution note").fill("Reviewed in isolated browser E2E.");
+  await reportCard.getByRole("button", { name: "Save decision" }).click();
   await expect(admin.page.getByText("Report updated.")).toBeVisible();
 
+  await gotoLiveView(admin.page, "/moderation?queue=proposals");
   const proposalCard = admin.page
-    .locator("article")
+    .locator("details")
     .filter({ hasText: proposalTitle });
-  await proposalCard.getByPlaceholder("Rejection note").fill("Covered by an existing category.");
+  await proposalCard.locator(":scope > summary").click();
+  await proposalCard.getByLabel("Rejection note").fill("Covered by an existing category.");
   await proposalCard.getByRole("button", { name: "Reject" }).click();
   await expect(admin.page.getByText("Proposal rejected.")).toBeVisible();
 
   await gotoLiveView(admin.page, "/admin/users");
   await admin.page.getByLabel("Search users").fill(participantEmail);
-  await expect(admin.page.locator("main article")).toHaveCount(1);
   const participantRow = admin.page
-    .locator("article")
+    .locator("main tbody tr")
     .filter({ hasText: participantEmail });
   await expect(participantRow).toHaveCount(1);
-  await participantRow.getByRole("combobox").first().selectOption("restricted");
-  await participantRow.getByLabel("Internal note").fill("E2E restriction boundary.");
-  await participantRow.getByRole("button", { name: "Save" }).click();
+  await participantRow.getByRole("link", { name: /^Manage / }).click();
+  await admin.page.getByLabel("Status").selectOption("restricted");
+  await admin.page.getByLabel("Internal note").fill("E2E restriction boundary.");
+  await admin.page.getByRole("button", { name: "Save account status" }).click();
   await expect(admin.page.getByText("Account status updated.")).toBeVisible();
 
   await gotoLiveView(participant.page, "/categories/proposals");

e2e/tests/mutual-aid.spec.ts

@@ -70,8 +70,10 @@ test("two users complete medicine tracking, handover, realtime chat, and blind r
   await expect(requester.page.getByRole("heading", { name: requestTitle })).toBeVisible();
 
   await gotoLiveView(helper.page, requestURL);
-  await helper.page.getByRole("button", { name: "I can help" }).click();
-  await expect(helper.page.getByRole("heading", { name: "Private match chat" })).toBeVisible();
+  await clickUntilVisible(
+    helper.page.getByRole("button", { name: "I can help" }),
+    helper.page.getByRole("heading", { name: "Private match chat" }),
+  );
   await expect(requester.page.getByRole("heading", { name: "Private match chat" })).toBeVisible();
 
   await helper.page
@@ -207,10 +209,10 @@ test("two users complete medicine tracking, handover, realtime chat, and blind r
   ).toBeVisible();
 
   await gotoLiveView(replacement.page, roadsideURL);
-  await replacement.page.getByRole("button", { name: "I can help" }).click();
-  await expect(
+  await clickUntilVisible(
+    replacement.page.getByRole("button", { name: "I can help" }),
     replacement.page.getByRole("heading", { name: "Private match chat" }),
-  ).toBeVisible();
+  );
   const replacementName =
     runID && fixturePassword
       ? "Staging E2E Replacement Helper"

e2e/tests/notifications-data.spec.ts

@@ -123,7 +123,7 @@ test("nearby alerts, private notification inbox, preferences, and data export wo
   const inbox = subscriber.page
     .getByRole("heading", { name: "Inbox" })
     .locator("xpath=ancestor::section");
-  const notification = inbox.getByRole("button", { name: /New help request nearby/ });
+  const notification = inbox.getByRole("link", { name: /New help request nearby/ });
   await expect(notification).toBeVisible({ timeout: 20_000 });
   await expect(inbox.getByText(privateArea)).toHaveCount(0);
   await notification.click();

lib/who_need_help_web/controllers/notification_open_controller.ex

@@ -5,7 +5,7 @@ defmodule WhoNeedHelpWeb.NotificationOpenController do
 
   def create(conn, %{"id" => id}) do
     case Notifications.notification_opened(conn.assigns.current_scope, id) do
-      {:ok, _notification} -> send_resp(conn, :no_content, "")
+      {:ok, notification} -> redirect(conn, to: notification.path)
       {:error, :not_found} -> send_resp(conn, :not_found, "")
     end
   end

lib/who_need_help_web/live/notification_live.ex

@@ -282,11 +282,9 @@ defmodule WhoNeedHelpWeb.NotificationLive do
           <div :if={@notifications != []} class="mt-4 divide-y divide-base-300">
             <.link
               :for={notification <- @notifications}
-              navigate={notification.path}
+              href={~p"/notifications/#{notification.id}/opened"}
+              method="post"
               id={"notification-#{notification.id}"}
-              phx-hook="NotificationLink"
-              data-open-url={~p"/notifications/#{notification.id}/opened"}
-              data-notification-id={notification.id}
               class={[
                 "flex w-full items-start gap-3 px-2 py-4 text-left transition hover:bg-base-200",
                 is_nil(notification.read_at) && "font-semibold"

test/who_need_help_web/controllers/notification_open_controller_test.exs

@@ -7,7 +7,7 @@ defmodule WhoNeedHelpWeb.NotificationOpenControllerTest do
 
   import WhoNeedHelp.AccountsFixtures
 
-  test "marks an owned notification as read without redirecting", %{conn: conn} do
+  test "marks an owned notification as read and redirects to its destination", %{conn: conn} do
     user = user_fixture()
 
     {:ok, notification} =
@@ -21,7 +21,7 @@ defmodule WhoNeedHelpWeb.NotificationOpenControllerTest do
 
     conn = conn |> log_in_user(user) |> post(~p"/notifications/#{notification.id}/opened")
 
-    assert response(conn, 204) == ""
+    assert redirected_to(conn) == "/support"
     assert Repo.get!(Notification, notification.id).read_at
   end
 

test/who_need_help_web/live/mutual_aid_live_test.exs

@@ -82,7 +82,7 @@ defmodule WhoNeedHelpWeb.MutualAidLiveTest do
 
     assert has_element?(
              view,
-             "a[href='/support'][phx-hook='NotificationLink'][data-open-url='/notifications/#{notification.id}/opened'][data-notification-id='#{notification.id}']"
+             "a[href='/notifications/#{notification.id}/opened'][data-method='post']"
            )
 
     view