diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 5c69fa9c..b8cbeb15 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -4,30 +4,6 @@ module ApplicationHelper include ApplicationHelper::GraphMethods include ApplicationHelper::TitleMethods - def inbox_count - return 0 unless user_signed_in? - - count = Inbox.select("COUNT(id) AS count") - .where(new: true) - .where(user_id: current_user.id) - .group(:user_id) - .order(:count) - .first - return nil if count.nil? - return nil unless count.count.positive? - - count.count - end - - def notification_count - return 0 unless user_signed_in? - - count = Notification.for(current_user).where(new: true).count - return nil unless count.positive? - - count - end - def privileged?(user) !current_user.nil? && ((current_user == user) || current_user.mod?) end diff --git a/app/helpers/bootstrap_helper.rb b/app/helpers/bootstrap_helper.rb index 9445de93..730713f2 100644 --- a/app/helpers/bootstrap_helper.rb +++ b/app/helpers/bootstrap_helper.rb @@ -5,6 +5,7 @@ module BootstrapHelper options = { badge: nil, badge_color: nil, + badge_attr: {}, icon: nil, class: "" }.merge(options) @@ -29,7 +30,7 @@ module BootstrapHelper ("badge-pill" if options[:badge_pill]) ].compact.join(" ") - body += " #{content_tag(:span, options[:badge], class: badge_class)}".html_safe + body += " #{content_tag(:span, options[:badge], class: badge_class, **options[:badge_attr])}".html_safe end content_tag(:li, link_to(body.html_safe, path, class: "nav-link"), class: classes) diff --git a/app/javascript/retrospring/controllers/pwa_badge_controller.ts b/app/javascript/retrospring/controllers/pwa_badge_controller.ts new file mode 100644 index 00000000..3fccef3f --- /dev/null +++ b/app/javascript/retrospring/controllers/pwa_badge_controller.ts @@ -0,0 +1,18 @@ +import { Controller } from '@hotwired/stimulus'; + +export default class extends Controller { + isPwa: boolean; + badgeCapable: boolean; + + initialize(): void { + this.isPwa = window.matchMedia('(display-mode: standalone)').matches; + this.badgeCapable = "setAppBadge" in navigator; + } + + connect(): void { + if (this.isPwa && this.badgeCapable) { + const count = Number.parseInt(this.element.innerText); + navigator.setAppBadge(count); + } + } +} diff --git a/app/javascript/retrospring/initializers/stimulus.ts b/app/javascript/retrospring/initializers/stimulus.ts index 6239a1f9..b0c93d0f 100644 --- a/app/javascript/retrospring/initializers/stimulus.ts +++ b/app/javascript/retrospring/initializers/stimulus.ts @@ -10,6 +10,7 @@ import CapabilitiesController from "retrospring/controllers/capabilities_control import CropperController from "retrospring/controllers/cropper_controller"; import InboxSharingController from "retrospring/controllers/inbox_sharing_controller"; import ToastController from "retrospring/controllers/toast_controller"; +import PwaBadgeController from "retrospring/controllers/pwa_badge_controller"; /** * This module sets up Stimulus and our controllers @@ -29,6 +30,7 @@ export default function (): void { window['Stimulus'].register('cropper', CropperController); window['Stimulus'].register('format-popup', FormatPopupController); window['Stimulus'].register('inbox-sharing', InboxSharingController); + window['Stimulus'].register('pwa-badge', PwaBadgeController); window['Stimulus'].register('theme', ThemeController); window['Stimulus'].register('toast', ToastController); } diff --git a/app/models/inbox.rb b/app/models/inbox.rb index 8c8c602f..6a74687f 100644 --- a/app/models/inbox.rb +++ b/app/models/inbox.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Inbox < ApplicationRecord - belongs_to :user + belongs_to :user, touch: :inbox_updated_at belongs_to :question attr_accessor :returning @@ -27,9 +27,10 @@ class Inbox < ApplicationRecord self.destroy end + def notification_icon = question.author_is_anonymous ? "/icons/maskable_icon_x128.png" : question.user.profile_picture.url(:small) + def as_push_notification { - type: :inbox, title: I18n.t( "frontend.push_notifications.inbox.title", user: if question.author_is_anonymous @@ -38,8 +39,11 @@ class Inbox < ApplicationRecord question.user.profile.safe_name end ), - icon: question.author_is_anonymous ? "/icons/maskable_icon_x128.png" : question.user.profile_picture.url(:small), - body: question.content.truncate(Question::SHORT_QUESTION_MAX_LENGTH) + icon: notification_icon, + body: question.content.truncate(Question::SHORT_QUESTION_MAX_LENGTH), + data: { + click_url: "/inbox", + }, } end end diff --git a/app/models/notification.rb b/app/models/notification.rb index 60890d17..3e15eae4 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Notification < ApplicationRecord - belongs_to :recipient, class_name: "User" + belongs_to :recipient, class_name: "User", touch: :notifications_updated_at belongs_to :target, polymorphic: true class << self @@ -11,11 +11,11 @@ class Notification < ApplicationRecord define_cursor_paginator :cursored_for_type, :for_type def for(recipient, **kwargs) - where(kwargs.merge!(recipient:)).includes(:target).order(:created_at).reverse_order + where(kwargs.merge!(recipient:)).order(:created_at).reverse_order end def for_type(recipient, type, **kwargs) - where(kwargs.merge!(recipient:)).includes(:target).where(type:).order(:created_at).reverse_order + where(kwargs.merge!(recipient:)).where(type:).order(:created_at).reverse_order end def notify(recipient, target) diff --git a/app/models/user.rb b/app/models/user.rb index 725582ef..92427a5d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -8,6 +8,7 @@ class User < ApplicationRecord # rubocop:disable Metrics/ClassLength include User::AnswerMethods include User::BanMethods include User::InboxMethods + include User::NotificationMethods include User::QuestionMethods include User::PushNotificationMethods include User::ReactionMethods diff --git a/app/models/user/inbox_methods.rb b/app/models/user/inbox_methods.rb index 1d961ce2..8e3b00fe 100644 --- a/app/models/user/inbox_methods.rb +++ b/app/models/user/inbox_methods.rb @@ -12,4 +12,18 @@ module User::InboxMethods .order(:created_at) .reverse_order end + + def unread_inbox_count + Rails.cache.fetch(inbox_cache_key) do + count = Inbox.where(new: true, user_id: id).count(:id) + + # Returning +nil+ here in order to not display a counter + # at all when there isn't anything in the user's inbox + return nil unless count.positive? + + count + end + end + + def inbox_cache_key = "#{cache_key}/unread_inbox_count-#{inbox_updated_at}" end diff --git a/app/models/user/notification_methods.rb b/app/models/user/notification_methods.rb new file mode 100644 index 00000000..db7e5018 --- /dev/null +++ b/app/models/user/notification_methods.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module User::NotificationMethods + def unread_notification_count + Rails.cache.fetch(notification_cache_key) do + count = Notification.for(self).where(new: true).count(:id) + + # Returning +nil+ here in order to not display a counter + # at all when there aren't any notifications + return nil unless count.positive? + + count + end + end + + def notification_cache_key = "#{cache_key}/unread_notification_count-#{notifications_updated_at}" +end diff --git a/app/models/user/push_notification_methods.rb b/app/models/user/push_notification_methods.rb index 01138633..b7b8f5d9 100644 --- a/app/models/user/push_notification_methods.rb +++ b/app/models/user/push_notification_methods.rb @@ -9,11 +9,17 @@ module User::PushNotificationMethods n.app = app n.registration_ids = [s.subscription.symbolize_keys] n.data = { - message: resource.as_push_notification.to_json + message: resource.as_push_notification.merge(notification_data).to_json, } n.save! PushNotificationWorker.perform_async(n.id) end end + + def notification_data = { + data: { + badge: unread_inbox_count, + }, + } end diff --git a/app/views/navigation/_desktop.html.haml b/app/views/navigation/_desktop.html.haml index eb8aba63..2d8093bc 100644 --- a/app/views/navigation/_desktop.html.haml +++ b/app/views/navigation/_desktop.html.haml @@ -10,7 +10,7 @@ DEV %ul.nav.navbar-nav.me-auto = nav_entry t("navigation.timeline"), root_path, icon: 'home' - = nav_entry t("navigation.inbox"), '/inbox', icon: 'inbox', badge: inbox_count + = nav_entry t("navigation.inbox"), "/inbox", icon: "inbox", badge: inbox_count, badge_attr: { data: { controller: "pwa-badge" } } - if APP_CONFIG.dig(:features, :discover, :enabled) || current_user.mod? = nav_entry t("navigation.discover"), discover_path, icon: 'compass' %ul.nav.navbar-nav diff --git a/app/views/navigation/_main.html.haml b/app/views/navigation/_main.html.haml index 989717df..a58e50b1 100644 --- a/app/views/navigation/_main.html.haml +++ b/app/views/navigation/_main.html.haml @@ -1,7 +1,10 @@ -- notifications = Notification.for(current_user).where(new: true).includes([:target]).limit(4) -= render 'navigation/desktop', notifications: notifications -= render 'navigation/mobile', notifications: notifications +:ruby + notifications = Notification.for(current_user).where(new: true).includes([:target]).limit(4) + inbox_count = current_user.unread_inbox_count + notification_count = current_user.unread_notification_count += render "navigation/desktop", notifications:, inbox_count:, notification_count: += render "navigation/mobile", inbox_count:, notification_count: -= render 'modal/ask' += render "modal/ask" %button.btn.btn-primary.btn-fab.d-block.d-lg-none.d-print-none{ data: { bs_target: "#modal-ask-followers", bs_toggle: :modal }, type: "button" } %i.fa.fa-pencil-square-o diff --git a/db/migrate/20230225143633_add_notification_and_inbox_timestamps_to_users.rb b/db/migrate/20230225143633_add_notification_and_inbox_timestamps_to_users.rb new file mode 100644 index 00000000..953a9207 --- /dev/null +++ b/db/migrate/20230225143633_add_notification_and_inbox_timestamps_to_users.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class AddNotificationAndInboxTimestampsToUsers < ActiveRecord::Migration[6.1] + def change + change_table :users, bulk: true do |t| + t.timestamp :notifications_updated_at + t.timestamp :inbox_updated_at + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 4d54a933..3cabc240 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2023_02_18_142952) do +ActiveRecord::Schema.define(version: 2023_02_25_143633) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -364,6 +364,8 @@ ActiveRecord::Schema.define(version: 2023_02_18_142952) do t.boolean "sharing_enabled", default: false t.boolean "sharing_autoclose", default: false t.string "sharing_custom_url" + t.datetime "notifications_updated_at" + t.datetime "inbox_updated_at" t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true t.index ["email"], name: "index_users_on_email", unique: true t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true diff --git a/lib/use_case/data_export/user.rb b/lib/use_case/data_export/user.rb index 4f7f8445..2bd4ceb1 100644 --- a/lib/use_case/data_export/user.rb +++ b/lib/use_case/data_export/user.rb @@ -13,6 +13,8 @@ module UseCase otp_secret_key reset_password_sent_at reset_password_token + inbox_updated_at + notifications_updated_at ].freeze IGNORED_FIELDS_PROFILES = %i[ diff --git a/public/service_worker.js b/public/service_worker.js index 8662f085..3e88521c 100644 --- a/public/service_worker.js +++ b/public/service_worker.js @@ -11,27 +11,22 @@ const OFFLINE_CACHE_PATHS = [ self.addEventListener('push', function (event) { if (event.data) { - const notification = event.data.json(); + const contents = event.data.json(); + navigator.setAppBadge(contents.data.badge); - event.waitUntil(self.registration.showNotification(notification.title, { - body: notification.body, - tag: notification.type, - icon: notification.icon, - })); + event.waitUntil(self.registration.showNotification(contents.title, contents)); } else { console.error("Push event received, but it didn't contain any data.", event); } }); self.addEventListener('notificationclick', async event => { - if (event.notification.tag === 'inbox') { + if ("click_url" in event.notification.data) { event.preventDefault(); - return clients.openWindow("/inbox", "_blank").then(result => { + return clients.openWindow(event.notification.data.click_url, "_blank").then(result => { event.notification.close(); return result; }); - } else { - console.warn(`Unhandled notification tag: ${event.notification.tag}`); } }); diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index abec3188..15d7601b 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -3,50 +3,6 @@ require "rails_helper" describe ApplicationHelper, type: :helper do - describe "#inbox_count" do - context "no signed in user" do - it "should return 0 as inbox count" do - expect(helper.inbox_count).to eq(0) - end - end - - context "user is signed in" do - let(:user) { FactoryBot.create(:user) } - let(:question) { FactoryBot.create(:question) } - - before do - sign_in(user) - Inbox.create(user_id: user.id, question_id: question.id, new: true) - end - - it "should return the inbox count" do - expect(helper.inbox_count).to eq(1) - end - end - end - - describe "#notification_count" do - context "no signed in user" do - it "should return 0 as notification count" do - expect(helper.notification_count).to eq(0) - end - end - - context "user is signed in" do - let(:user) { FactoryBot.create(:user) } - let(:another_user) { FactoryBot.create(:user) } - - before do - sign_in(user) - another_user.follow(user) - end - - it "should return the notification count" do - expect(helper.notification_count).to eq(1) - end - end - end - describe "#privileged" do context "current user and checked user do not match" do let(:user) { FactoryBot.create(:user) } diff --git a/spec/models/user/inbox_methods_spec.rb b/spec/models/user/inbox_methods_spec.rb new file mode 100644 index 00000000..efa7d16e --- /dev/null +++ b/spec/models/user/inbox_methods_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require "rails_helper" + +describe User::InboxMethods do + context "given a user" do + let(:user) { FactoryBot.create(:user) } + + describe "#unread_inbox_count" do + subject { user.unread_inbox_count } + + context "user has no questions in their inbox" do + it "should return nil" do + expect(subject).to eq(nil) + end + end + + context "user has 1 question in their inbox" do + # FactoryBot seems to have issues with setting the +new+ field on inbox entries + # so we can create it manually instead + let!(:inbox) { Inbox.create(question: FactoryBot.create(:question), user:, new: true) } + + it "should return 1" do + expect(subject).to eq(1) + end + end + end + end +end diff --git a/spec/models/user/notification_methods_spec.rb b/spec/models/user/notification_methods_spec.rb new file mode 100644 index 00000000..550d3116 --- /dev/null +++ b/spec/models/user/notification_methods_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require "rails_helper" + +describe User::NotificationMethods do + context "given a user" do + let(:user) { FactoryBot.create(:user) } + + describe "#unread_notification_count" do + subject { user.unread_notification_count } + + context "user has no notifications" do + it "should return nil" do + expect(subject).to eq(nil) + end + end + + context "user has a notification" do + let(:other_user) { FactoryBot.create(:user) } + + before do + other_user.follow(user) + end + + it "should return 1" do + expect(subject).to eq(1) + end + end + end + end +end diff --git a/spec/workers/question_worker_spec.rb b/spec/workers/question_worker_spec.rb index 8070392b..0795e55b 100644 --- a/spec/workers/question_worker_spec.rb +++ b/spec/workers/question_worker_spec.rb @@ -78,7 +78,7 @@ describe QuestionWorker do user: receiver, subscription: { endpoint: "This will not be used", - keys: {} + keys: {}, } ) receiver.follow(user)