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/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/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 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
+ +

+ 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"