From f343e78cbe9de58013752ca381437204e3d149a1 Mon Sep 17 00:00:00 2001 From: Hampton Lintorn-Catlin Date: Wed, 13 May 2026 12:48:03 -0400 Subject: [PATCH 1/2] Auto-refresh device list on Settings after enable/disable Wraps the per-device list in a turbo-frame and reloads it from the Stimulus controller after subscribe/unsubscribe succeeds, so the new browser appears (or disappears) without a full page refresh. Changes - Extracts the device list into _devices.html.erb wrapped in a . - Adds GET /web_push/devices that renders just that partial back into the same frame. - web_push_settings_controller.js: new devicesUrl Stimulus value; after enable/disable success, sets the frame's src to refetch. Generated with Amp --- .../web_push/subscriptions_controller.rb | 9 ++++++ .../coplan/web_push_settings_controller.js | 13 ++++++++ .../settings/settings/_devices.html.erb | 28 +++++++++++++++++ .../settings/settings/_notifications.html.erb | 30 ++++--------------- engine/config/routes.rb | 4 +++ 5 files changed, 59 insertions(+), 25 deletions(-) create mode 100644 engine/app/views/coplan/settings/settings/_devices.html.erb diff --git a/engine/app/controllers/coplan/web_push/subscriptions_controller.rb b/engine/app/controllers/coplan/web_push/subscriptions_controller.rb index 5552856..d24b799 100644 --- a/engine/app/controllers/coplan/web_push/subscriptions_controller.rb +++ b/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 diff --git a/engine/app/javascript/controllers/coplan/web_push_settings_controller.js b/engine/app/javascript/controllers/coplan/web_push_settings_controller.js index b15aa62..60ec3a7 100644 --- a/engine/app/javascript/controllers/coplan/web_push_settings_controller.js +++ b/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() { diff --git a/engine/app/views/coplan/settings/settings/_devices.html.erb b/engine/app/views/coplan/settings/settings/_devices.html.erb new file mode 100644 index 0000000..ae1fa80 --- /dev/null +++ b/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? %> +
+
Devices receiving notifications
+
    + <% web_push_subscriptions.each do |sub| %> +
  • +
    <%= sub.device_label %>
    +
    + 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 %> +
    +
    • + <% end %> +
+

+ To remove a device, sign in to CoPlan from that browser and click "Disable on this device". +

+
+ <% end %> +<% end %> diff --git a/engine/app/views/coplan/settings/settings/_notifications.html.erb b/engine/app/views/coplan/settings/settings/_notifications.html.erb index 03ed2e4..c2fa0b4 100644 --- a/engine/app/views/coplan/settings/settings/_notifications.html.erb +++ b/engine/app/views/coplan/settings/settings/_notifications.html.erb @@ -1,6 +1,8 @@ <% return unless CoPlan.configuration.web_push_configured? %> -
+
Browser notifications
@@ -28,28 +30,6 @@
- <% if @web_push_subscriptions.any? %> -
-
Devices receiving notifications
-
    - <% @web_push_subscriptions.each do |sub| %> -
  • -
    <%= sub.device_label %>
    -
    - 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 %> -
    -
    • - <% end %> -
-

- To remove a device, sign in to CoPlan from that browser and click "Disable on this device". -

-
- <% end %> + <%= render partial: "coplan/settings/settings/devices", + locals: { web_push_subscriptions: @web_push_subscriptions } %>
diff --git a/engine/config/routes.rb b/engine/config/routes.rb index e7e801e..beabb6e 100644 --- a/engine/config/routes.rb +++ b/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" From c0bc65bb9216408cae2ab27f41ec4d68ca6eabd8 Mon Sep 17 00:00:00 2001 From: Hampton Lintorn-Catlin Date: Wed, 13 May 2026 14:19:24 -0400 Subject: [PATCH 2/2] Improve Web Push notification UX (icon, length, SW debug visibility) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three small fixes shaken out by end-to-end testing: 1. Service worker now shows the CoPlan logo as the notification icon instead of falling back to the browser's app chrome (Chrome's puzzle piece in the test). The icon URL is templated into the SW body at request time so it always points at the current digested asset. 2. SW push handler no longer silently bails when event.data is empty — it shows a fallback notification. This makes the DevTools Application → Service Workers → Push button useful for verifying the handler is wired up without needing to send a real push. Also adds a console log on every push and a .catch on showNotification so future failures surface immediately instead of disappearing. 3. PayloadForNotification body truncation 140 → 90 chars. Chrome / macOS show roughly 80–100 chars before they truncate further; setting it lower means we control where the cut happens (and add an ellipsis on a word boundary) instead of ending mid-word with no visual hint. Generated with Amp --- .../coplan/service_workers_controller.rb | 12 ++++--- .../app/javascript/coplan_service_worker.js | 35 ++++++++++++++----- .../web_push/payload_for_notification.rb | 5 ++- 3 files changed, 38 insertions(+), 14 deletions(-) diff --git a/engine/app/controllers/coplan/service_workers_controller.rb b/engine/app/controllers/coplan/service_workers_controller.rb index 29203c1..fd5376e 100644 --- a/engine/app/controllers/coplan/service_workers_controller.rb +++ b/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 diff --git a/engine/app/javascript/coplan_service_worker.js b/engine/app/javascript/coplan_service_worker.js index 3d19790..01a5a07 100644 --- a/engine/app/javascript/coplan_service_worker.js +++ b/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) => { diff --git a/engine/app/services/coplan/web_push/payload_for_notification.rb b/engine/app/services/coplan/web_push/payload_for_notification.rb index 9685197..632908f 100644 --- a/engine/app/services/coplan/web_push/payload_for_notification.rb +++ b/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