Improve Web Push notification UX (icon, length, SW debug visibility)

engine/app/controllers/coplan/service_workers_controller.rb

@@ -5,8 +5,10 @@ class ServiceWorkersController < ApplicationController
     skip_before_action :authenticate_coplan_user!
     skip_before_action :verify_authenticity_token, raise: false
 
-    SW_PATH = CoPlan::Engine.root.join("app/javascript/coplan_service_worker.js")
-    SW_BODY = SW_PATH.read.freeze
+    SW_PATH     = CoPlan::Engine.root.join("app/javascript/coplan_service_worker.js")
+    SW_TEMPLATE = SW_PATH.read.freeze
+    ICON_TOKEN  = "__COPLAN_NOTIFICATION_ICON__"
+    ICON_ASSET  = "coplan/coplan-logo-sm.png"
 
     def show
       # No Service-Worker-Allowed header on purpose: default scope is the
@@ -15,9 +17,11 @@ def show
       #
       # Render inline rather than send_file: the JS lives inside the gem,
       # so any reverse proxy that intercepts X-Sendfile (NGINX et al) won't
-      # reach it. The file is small and cached in memory at boot.
+      # reach it. The file is small and cached in memory at boot, then we
+      # substitute the digested icon URL per request.
       response.headers["Cache-Control"] = "no-cache"
-      render plain: SW_BODY, content_type: "application/javascript"
+      render plain: SW_TEMPLATE.gsub(ICON_TOKEN, view_context.asset_path(ICON_ASSET)),
+             content_type: "application/javascript"
     end
   end
 end

engine/app/controllers/coplan/web_push/subscriptions_controller.rb

@@ -17,6 +17,15 @@ def create
         render json: { id: record.id, created_at: record.created_at }, status: :created
       end
 
+      # GET /web_push/devices
+      # Renders the device list inside its turbo-frame so the Settings page
+      # can refresh just that section after enabling/disabling on this browser.
+      def devices
+        @web_push_subscriptions = current_user.web_push_subscriptions.order(created_at: :desc)
+        render partial: "coplan/settings/settings/devices",
+               locals: { web_push_subscriptions: @web_push_subscriptions }
+      end
+
       # DELETE /web_push/subscription
       # Body: { subscription: { endpoint } }
       def destroy

engine/app/javascript/controllers/coplan/web_push_settings_controller.js

@@ -5,6 +5,7 @@ import * as WebPush from "coplan/web_push"
 // browser's subscription state (which the server can't know — it's per-device).
 export default class extends Controller {
   static targets = ["enableButton", "disableButton", "status"]
+  static values  = { devicesUrl: { type: String, default: "" } }
 
   async connect() {
     await this._refresh()
@@ -15,6 +16,7 @@ export default class extends Controller {
     try {
       await WebPush.subscribe()
       this._setStatus("Notifications enabled on this device.")
+      this._reloadDevices()
     } catch (err) {
       this._setStatus(this._friendlyError(err))
     }
@@ -26,12 +28,23 @@ export default class extends Controller {
     try {
       await WebPush.unsubscribe()
       this._setStatus("Notifications disabled on this device.")
+      this._reloadDevices()
     } catch (err) {
       this._setStatus(this._friendlyError(err))
     }
     await this._refresh()
   }
 
+  // Tell the device-list turbo-frame to refetch itself so the row for this
+  // browser appears (or disappears) without a full page reload.
+  _reloadDevices() {
+    if (!this.devicesUrlValue) return
+    const frame = document.getElementById("web-push-devices")
+    if (!frame) return
+    // Setting src triggers a fetch even when the URL hasn't changed.
+    frame.src = this.devicesUrlValue
+  }
+
   // ---- internals ----
 
   async _refresh() {

engine/app/javascript/coplan_service_worker.js

@@ -14,24 +14,41 @@ self.addEventListener("activate", (event) => {
 // Push payloads are JSON: { title, body, url, tag }
 // `tag` groups/dedups notifications (e.g., "comment-thread-#{id}").
 // `url` may include a hash for deep-linking to a thread popover.
+//
+// We always call showNotification() — even when the push has no data — both
+// because Chrome will spawn a generic "This site has been updated in the
+// background" notification if we don't (counts as a violation of the
+// userVisibleOnly: true contract), and because it makes the DevTools
+// "Push" test button useful for verifying the SW is actually wired up.
 self.addEventListener("push", (event) => {
-  if (!event.data) return
-
-  let payload
-  try {
-    payload = event.data.json()
-  } catch {
-    payload = { title: "CoPlan", body: event.data.text() }
+  let payload = { title: "CoPlan", body: "" }
+  if (event.data) {
+    try {
+      payload = event.data.json()
+    } catch {
+      payload = { title: "CoPlan", body: event.data.text() }
+    }
+  } else {
+    payload = { title: "CoPlan", body: "Test push (no payload)" }
   }
 
   const title = payload.title || "CoPlan"
   const options = {
     body: payload.body || "",
     tag: payload.tag,
-    data: { url: payload.url }
+    data: { url: payload.url },
+    // Substituted by ServiceWorkersController at request time so the SW always
+    // picks up the current digested asset path for the CoPlan logo.
+    icon: "__COPLAN_NOTIFICATION_ICON__",
+    badge: "__COPLAN_NOTIFICATION_ICON__"
   }
 
-  event.waitUntil(self.registration.showNotification(title, options))
+  console.log("[coplan-sw] push received, showing notification", { title, options })
+  event.waitUntil(
+    self.registration.showNotification(title, options).catch((err) => {
+      console.error("[coplan-sw] showNotification failed", err)
+    })
+  )
 })
 
 self.addEventListener("notificationclick", (event) => {

engine/app/services/coplan/web_push/payload_for_notification.rb

@@ -5,7 +5,10 @@ module WebPush
     # specific thread; tag groups updates for the same thread so successive
     # replies replace each other rather than piling up.
     class PayloadForNotification
-      BODY_TRUNCATE = 140
+      # macOS / Chrome show roughly 80–100 chars of body before truncating with
+      # an ellipsis of their own — keep the payload close to that so we control
+      # where the cut happens and don't end mid-word.
+      BODY_TRUNCATE = 90
 
       def self.call(notification)
         new(notification).call

engine/app/views/coplan/settings/settings/_devices.html.erb

@@ -0,0 +1,28 @@
+<%# Per-device list rendered both in-page (initial load) and as a turbo-frame %>
+<%# response after the Stimulus controller refreshes it post-enable/disable. %>
+<%= turbo_frame_tag "web-push-devices" do %>
+  <% if web_push_subscriptions.any? %>
+    <div class="settings-subsection">
+      <div class="settings-subsection__heading">Devices receiving notifications</div>
+      <ul class="device-list">
+        <% web_push_subscriptions.each do |sub| %>
+          <li class="device-list__item">
+            <div class="device-list__name"><%= sub.device_label %></div>
+            <div class="device-list__meta">
+              Added <%= time_ago_in_words(sub.created_at) %> ago
+              <% if sub.last_delivered_at %>
+                · last notified <%= time_ago_in_words(sub.last_delivered_at) %> ago
+                · <%= pluralize(sub.notifications_delivered_count, "notification") %>
+              <% else %>
+                · no notifications yet
+              <% end %>
+            </div>
+          </li>
+        <% end %>
+      </ul>
+      <p class="settings-subsection__hint">
+        To remove a device, sign in to CoPlan from that browser and click "Disable on this device".
+      </p>
+    </div>
+  <% end %>
+<% end %>

engine/app/views/coplan/settings/settings/_notifications.html.erb

@@ -1,6 +1,8 @@
 <% return unless CoPlan.configuration.web_push_configured? %>
 
-<div class="card mb-md" data-controller="coplan--web-push-settings">
+<div class="card mb-md"
+     data-controller="coplan--web-push-settings"
+     data-coplan--web-push-settings-devices-url-value="<%= coplan.web_push_devices_path %>">
   <div class="settings-row">
     <div class="settings-row__main">
       <div class="settings-row__label">Browser notifications</div>
@@ -28,28 +30,6 @@
     </div>
   </div>
 
-  <% if @web_push_subscriptions.any? %>
-    <div class="settings-subsection">
-      <div class="settings-subsection__heading">Devices receiving notifications</div>
-      <ul class="device-list">
-        <% @web_push_subscriptions.each do |sub| %>
-          <li class="device-list__item">
-            <div class="device-list__name"><%= sub.device_label %></div>
-            <div class="device-list__meta">
-              Added <%= time_ago_in_words(sub.created_at) %> ago
-              <% if sub.last_delivered_at %>
-                · last notified <%= time_ago_in_words(sub.last_delivered_at) %> ago
-                · <%= pluralize(sub.notifications_delivered_count, "notification") %>
-              <% else %>
-                · no notifications yet
-              <% end %>
-            </div>
-          </li>
-        <% end %>
-      </ul>
-      <p class="settings-subsection__hint">
-        To remove a device, sign in to CoPlan from that browser and click "Disable on this device".
-      </p>
-    </div>
-  <% end %>
+  <%= render partial: "coplan/settings/settings/devices",
+             locals: { web_push_subscriptions: @web_push_subscriptions } %>
 </div>

engine/config/routes.rb

@@ -76,6 +76,10 @@
   # PushManager and uniquely identify a (browser, device, app) tuple per user.
   scope :web_push, module: "web_push", as: :web_push do
     resource :subscription, only: [:create, :destroy], controller: "subscriptions"
+    # Turbo-frame target for the per-device list on the Settings page.
+    # Reloaded by the settings Stimulus controller after enable/disable so
+    # the list reflects the new browser without a full page refresh.
+    get "devices", to: "subscriptions#devices", as: :devices
   end
 
   root "plans#index"