From d0ae5b4bdc086495bcbcf8aefd89478d90595eb4 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Thu, 9 Jan 2025 15:53:31 +0000 Subject: [PATCH 01/40] Add notification model --- app/models/bubble.rb | 2 ++ app/models/notification.rb | 7 +++++++ app/models/user.rb | 2 ++ .../20250109153649_create_notifications.rb | 14 ++++++++++++++ db/schema.rb | 16 +++++++++++++++- test/models/notification_test.rb | 7 +++++++ 6 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 app/models/notification.rb create mode 100644 db/migrate/20250109153649_create_notifications.rb create mode 100644 test/models/notification_test.rb diff --git a/app/models/bubble.rb b/app/models/bubble.rb index 5da059a49..a731d7726 100644 --- a/app/models/bubble.rb +++ b/app/models/bubble.rb @@ -4,6 +4,8 @@ class Bubble < ApplicationRecord belongs_to :bucket, touch: true belongs_to :creator, class_name: "User", default: -> { Current.user } + has_many :notifications, dependent: :destroy + has_one_attached :image, dependent: :purge_later before_save :set_default_title diff --git a/app/models/notification.rb b/app/models/notification.rb new file mode 100644 index 000000000..56d2b8ffd --- /dev/null +++ b/app/models/notification.rb @@ -0,0 +1,7 @@ +class Notification < ApplicationRecord + belongs_to :user + belongs_to :bubble + + scope :unread, -> { where.not(:read) } + scope :ordered, -> { order(created_at: :desc) } +end diff --git a/app/models/user.rb b/app/models/user.rb index 907881cc5..cceadb922 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -18,6 +18,8 @@ class User < ApplicationRecord has_many :assignings, foreign_key: :assigner_id, class_name: "Assignment" has_many :assigned_bubbles, through: :assignments, source: :bubble + has_many :notifications, dependent: :destroy + has_one_attached :avatar validates_presence_of :email_address diff --git a/db/migrate/20250109153649_create_notifications.rb b/db/migrate/20250109153649_create_notifications.rb new file mode 100644 index 000000000..834d75b16 --- /dev/null +++ b/db/migrate/20250109153649_create_notifications.rb @@ -0,0 +1,14 @@ +class CreateNotifications < ActiveRecord::Migration[8.1] + def change + create_table :notifications do |t| + t.references :user, null: false, foreign_key: true + t.references :bubble, null: false, foreign_key: true + t.boolean :read, default: false, null: false + t.text :body, null: false + + t.timestamps + + t.index %i[ user_id read created_at ], order: { created_at: :desc } + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 1d008479c..da324c7fc 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[8.1].define(version: 2024_12_09_223551) do +ActiveRecord::Schema[8.1].define(version: 2025_01_09_153649) do create_table "accesses", force: :cascade do |t| t.integer "bucket_id", null: false t.integer "user_id", null: false @@ -176,6 +176,18 @@ ActiveRecord::Schema[8.1].define(version: 2024_12_09_223551) do t.index ["messageable_type", "messageable_id"], name: "index_messages_on_messageable", unique: true end + create_table "notifications", force: :cascade do |t| + t.integer "user_id", null: false + t.integer "bubble_id", null: false + t.boolean "read", default: false, null: false + t.text "body", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["bubble_id"], name: "index_notifications_on_bubble_id" + t.index ["user_id", "read", "created_at"], name: "index_notifications_on_user_id_and_read_and_created_at", order: { created_at: :desc } + t.index ["user_id"], name: "index_notifications_on_user_id" + end + create_table "pops", force: :cascade do |t| t.integer "bubble_id", null: false t.integer "user_id" @@ -244,6 +256,8 @@ ActiveRecord::Schema[8.1].define(version: 2024_12_09_223551) do add_foreign_key "bubbles", "workflow_stages", column: "stage_id" add_foreign_key "events", "event_summaries", column: "summary_id" add_foreign_key "messages", "bubbles" + add_foreign_key "notifications", "bubbles" + add_foreign_key "notifications", "users" add_foreign_key "pops", "bubbles" add_foreign_key "pops", "users" add_foreign_key "sessions", "users" diff --git a/test/models/notification_test.rb b/test/models/notification_test.rb new file mode 100644 index 000000000..a76e08d66 --- /dev/null +++ b/test/models/notification_test.rb @@ -0,0 +1,7 @@ +require "test_helper" + +class NotificationTest < ActiveSupport::TestCase + # test "the truth" do + # assert true + # end +end From 5a56c1602993a826cabbf89fa3c61ff2208ae11f Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Thu, 9 Jan 2025 17:10:47 +0000 Subject: [PATCH 02/40] Add a factory for creating notifications --- app/models/notifier.rb | 36 +++++++++++++++++++++++++++ app/models/notifier/assigned.rb | 10 ++++++++ app/models/notifier/created.rb | 10 ++++++++ test/models/notification_test.rb | 3 --- test/models/notifier/assigned_test.rb | 25 +++++++++++++++++++ test/models/notifier/created_test.rb | 19 ++++++++++++++ test/models/notifier_test.rb | 15 +++++++++++ 7 files changed, 115 insertions(+), 3 deletions(-) create mode 100644 app/models/notifier.rb create mode 100644 app/models/notifier/assigned.rb create mode 100644 app/models/notifier/created.rb create mode 100644 test/models/notifier/assigned_test.rb create mode 100644 test/models/notifier/created_test.rb create mode 100644 test/models/notifier_test.rb diff --git a/app/models/notifier.rb b/app/models/notifier.rb new file mode 100644 index 000000000..7f2267aa4 --- /dev/null +++ b/app/models/notifier.rb @@ -0,0 +1,36 @@ +class Notifier + attr_reader :event + + class << self + def for(event) + "Notifier::#{event.action.classify}".safe_constantize&.new(event) + end + end + + def generate + recipients.each do |recipient| + Notification.create! user: recipient, bubble: bubble, body: body + end + end + + private + def initialize(event) + @event = event + end + + def body + raise NotImplementedError + end + + def recipients + raise NotImplementedError + end + + def bubble + @event.summary.message.bubble + end + + def creator + @event.creator + end +end diff --git a/app/models/notifier/assigned.rb b/app/models/notifier/assigned.rb new file mode 100644 index 000000000..c18d35a56 --- /dev/null +++ b/app/models/notifier/assigned.rb @@ -0,0 +1,10 @@ +class Notifier::Assigned < Notifier + private + def body + "#{creator.name} assigned you: #{bubble.title}" + end + + def recipients + event.assignees.without(creator) + end +end diff --git a/app/models/notifier/created.rb b/app/models/notifier/created.rb new file mode 100644 index 000000000..1bdc3fe2f --- /dev/null +++ b/app/models/notifier/created.rb @@ -0,0 +1,10 @@ +class Notifier::Created < Notifier + private + def body + "#{creator.name} created a new item: #{bubble.title}" + end + + def recipients + bubble.bucket.users.without(creator) + end +end diff --git a/test/models/notification_test.rb b/test/models/notification_test.rb index a76e08d66..449f3c08d 100644 --- a/test/models/notification_test.rb +++ b/test/models/notification_test.rb @@ -1,7 +1,4 @@ require "test_helper" class NotificationTest < ActiveSupport::TestCase - # test "the truth" do - # assert true - # end end diff --git a/test/models/notifier/assigned_test.rb b/test/models/notifier/assigned_test.rb new file mode 100644 index 000000000..eafcd682a --- /dev/null +++ b/test/models/notifier/assigned_test.rb @@ -0,0 +1,25 @@ +require "test_helper" + +class Notifier::AssignedTest < ActiveSupport::TestCase + test "generate creates a notification for each recipient" do + assert_difference -> { Notification.count }, 1 do + assert_difference -> { users(:kevin).notifications.count }, 1 do + Notifier.for(events(:logo_assignment_km)).generate + end + end + end + + test "generate does not notify for self-assignments" do + event = EventSummary.last.events.create! action: :assigned, creator: users(:kevin), particulars: { assignee_ids: [ users(:kevin).id ] } + + assert_no_difference -> { Notification.count } do + Notifier.for(event).generate + end + end + + test "generate populates the notification details" do + Notifier.for(events(:logo_assignment_km)).generate + + assert_equal "David assigned you: The logo isn't big enough", Notification.last.body + end +end diff --git a/test/models/notifier/created_test.rb b/test/models/notifier/created_test.rb new file mode 100644 index 000000000..3e36cfcd6 --- /dev/null +++ b/test/models/notifier/created_test.rb @@ -0,0 +1,19 @@ +require "test_helper" + +class Notifier::CreatedTest < ActiveSupport::TestCase + test "generate creates a notification for each recipient" do + assert_difference -> { Notification.count }, 2 do + assert_difference -> { users(:kevin).notifications.count }, 1 do + assert_difference -> { users(:jz).notifications.count }, 1 do + Notifier.for(events(:logo_created)).generate + end + end + end + end + + test "generate populates the notification details" do + Notifier.for(events(:logo_created)).generate + + assert_equal "David created a new item: The logo isn't big enough", Notification.last.body + end +end diff --git a/test/models/notifier_test.rb b/test/models/notifier_test.rb new file mode 100644 index 000000000..62baf0c2d --- /dev/null +++ b/test/models/notifier_test.rb @@ -0,0 +1,15 @@ +require "test_helper" + +class NotifierTest < ActiveSupport::TestCase + test "for returns the matching notifier class for the event" do + assert_kind_of Notifier::Created, Notifier.for(events(:logo_created)) + end + + test "for does not raise an error when the event is not notifiable" do + assert_nothing_raised do + assert_no_difference -> { Notification.count } do + Notifier.for(events(:logo_boost_dhh)) + end + end + end +end From a082eb75f58e9208f3e1c5224aedaca2aebba558 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Thu, 9 Jan 2025 18:06:45 +0000 Subject: [PATCH 03/40] Track events for new comments too --- app/models/bubble/commentable.rb | 3 ++- app/models/event.rb | 2 +- app/models/event/comment.rb | 11 +++++++++++ app/models/message.rb | 2 +- app/models/notifier/commented.rb | 10 ++++++++++ 5 files changed, 25 insertions(+), 3 deletions(-) create mode 100644 app/models/event/comment.rb create mode 100644 app/models/notifier/commented.rb diff --git a/app/models/bubble/commentable.rb b/app/models/bubble/commentable.rb index f80f34777..152eeb7a0 100644 --- a/app/models/bubble/commentable.rb +++ b/app/models/bubble/commentable.rb @@ -5,8 +5,9 @@ module Bubble::Commentable scope :ordered_by_comments, -> { order comments_count: :desc } end - def comment_created + def comment_created(comment) increment! :comments_count + track_event :commented, comment_id: comment.id rescore end diff --git a/app/models/event.rb b/app/models/event.rb index 006e25ed0..0c01b1f45 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -1,5 +1,5 @@ class Event < ApplicationRecord - include Assignments, Stages + include Assignments, Comment, Stages belongs_to :creator, class_name: "User" belongs_to :summary, touch: true, class_name: "EventSummary" diff --git a/app/models/event/comment.rb b/app/models/event/comment.rb new file mode 100644 index 000000000..f6237e7b5 --- /dev/null +++ b/app/models/event/comment.rb @@ -0,0 +1,11 @@ +module Event::Comment + extend ActiveSupport::Concern + + included do + store_accessor :particulars, :comment_id + end + + def comment + @comment ||= Comment.find(comment_id) + end +end diff --git a/app/models/message.rb b/app/models/message.rb index f688f27dd..24e658ed3 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -10,7 +10,7 @@ class Message < ApplicationRecord private def created - bubble.comment_created if comment? + bubble.comment_created(comment) if comment? end def destroyed diff --git a/app/models/notifier/commented.rb b/app/models/notifier/commented.rb new file mode 100644 index 000000000..aad76a591 --- /dev/null +++ b/app/models/notifier/commented.rb @@ -0,0 +1,10 @@ +class Notifier::Commented < Notifier + private + def body + "#{creator.name} commented on: #{bubble.title}" + end + + def recipients + bubble.bucket.users.without(creator) + end +end From 6b5e098407f1d1458a81836b15af3e75c401747e Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Thu, 9 Jan 2025 18:11:01 +0000 Subject: [PATCH 04/40] Use a better return value --- app/models/notifier.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/notifier.rb b/app/models/notifier.rb index 7f2267aa4..be58c79c9 100644 --- a/app/models/notifier.rb +++ b/app/models/notifier.rb @@ -8,7 +8,7 @@ class Notifier end def generate - recipients.each do |recipient| + recipients.map do |recipient| Notification.create! user: recipient, bubble: bubble, body: body end end From 952895ae67f27106c9253f2061b780350a47ffa0 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Thu, 9 Jan 2025 18:25:35 +0000 Subject: [PATCH 05/40] WIP: show notifications in a tray --- app/assets/stylesheets/notifications.css | 15 +++++++++++++++ app/controllers/notifications_controller.rb | 5 +++++ app/helpers/notifications_helper.rb | 7 +++++++ app/models/notification.rb | 4 ++-- app/views/bubbles/index.html.erb | 2 ++ app/views/notifications/_notification.html.erb | 3 +++ app/views/notifications/index.html.erb | 3 +++ config/routes.rb | 1 + test/controllers/notifications_controller_test.rb | 7 +++++++ 9 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 app/assets/stylesheets/notifications.css create mode 100644 app/controllers/notifications_controller.rb create mode 100644 app/helpers/notifications_helper.rb create mode 100644 app/views/notifications/_notification.html.erb create mode 100644 app/views/notifications/index.html.erb create mode 100644 test/controllers/notifications_controller_test.rb diff --git a/app/assets/stylesheets/notifications.css b/app/assets/stylesheets/notifications.css new file mode 100644 index 000000000..164330528 --- /dev/null +++ b/app/assets/stylesheets/notifications.css @@ -0,0 +1,15 @@ +.notification-tray { + position: fixed; + width: 40ch; + bottom: 1rem; + left: calc(50% - 20ch); + display: flex; + flex-direction: column; + gap: 1rem; +} + +.notification { + background-color: #eeed; + border-radius: 6px; + padding: 0.5rem; +} diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb new file mode 100644 index 000000000..22e40167e --- /dev/null +++ b/app/controllers/notifications_controller.rb @@ -0,0 +1,5 @@ +class NotificationsController < ApplicationController + def index + @notifications = Current.user.notifications.unread.ordered.limit(20) + end +end diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb new file mode 100644 index 000000000..b7fe8de4d --- /dev/null +++ b/app/helpers/notifications_helper.rb @@ -0,0 +1,7 @@ +module NotificationsHelper + def notification_tray_tag + tag.div class: "notification-tray" do + turbo_frame_tag "notifications", src: notifications_path, data: { turbo_permanent: true } + end + end +end diff --git a/app/models/notification.rb b/app/models/notification.rb index 56d2b8ffd..5553dfad9 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -2,6 +2,6 @@ class Notification < ApplicationRecord belongs_to :user belongs_to :bubble - scope :unread, -> { where.not(:read) } - scope :ordered, -> { order(created_at: :desc) } + scope :unread, -> { where(read: false) } + scope :ordered, -> { order(read: :desc, created_at: :desc) } end diff --git a/app/views/bubbles/index.html.erb b/app/views/bubbles/index.html.erb index bc43bf2ab..7928afacf 100644 --- a/app/views/bubbles/index.html.erb +++ b/app/views/bubbles/index.html.erb @@ -50,3 +50,5 @@ <%= render partial: "bubbles/list/bubble", collection: @bubbles, cached: true %> + +<%= notification_tray_tag %> diff --git a/app/views/notifications/_notification.html.erb b/app/views/notifications/_notification.html.erb new file mode 100644 index 000000000..c87ac7166 --- /dev/null +++ b/app/views/notifications/_notification.html.erb @@ -0,0 +1,3 @@ +
+

<%= notification.body %>

+
diff --git a/app/views/notifications/index.html.erb b/app/views/notifications/index.html.erb new file mode 100644 index 000000000..ee91b07e4 --- /dev/null +++ b/app/views/notifications/index.html.erb @@ -0,0 +1,3 @@ +<%= turbo_frame_tag "notifications" do %> + <%= render @notifications %> +<% end %> diff --git a/config/routes.rb b/config/routes.rb index 5f16ae681..d0f860f96 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -13,6 +13,7 @@ Rails.application.routes.draw do end resources :bubbles + resources :notifications resources :buckets do resources :bubbles do diff --git a/test/controllers/notifications_controller_test.rb b/test/controllers/notifications_controller_test.rb new file mode 100644 index 000000000..20b28201d --- /dev/null +++ b/test/controllers/notifications_controller_test.rb @@ -0,0 +1,7 @@ +require "test_helper" + +class NotificationsControllerTest < ActionDispatch::IntegrationTest + # test "the truth" do + # assert true + # end +end From af5ac6511610605585985c5b2b31e315af7cc109 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Fri, 10 Jan 2025 10:12:47 +0000 Subject: [PATCH 06/40] Make notification target polymorphic --- app/models/bubble.rb | 2 +- app/models/notification.rb | 3 ++- app/models/notifier.rb | 2 +- db/migrate/20250109153649_create_notifications.rb | 5 +++-- db/schema.rb | 11 +++++++---- test/fixtures/notifications.yml | 15 +++++++++++++++ 6 files changed, 29 insertions(+), 9 deletions(-) create mode 100644 test/fixtures/notifications.yml diff --git a/app/models/bubble.rb b/app/models/bubble.rb index a731d7726..1e617189d 100644 --- a/app/models/bubble.rb +++ b/app/models/bubble.rb @@ -4,7 +4,7 @@ class Bubble < ApplicationRecord belongs_to :bucket, touch: true belongs_to :creator, class_name: "User", default: -> { Current.user } - has_many :notifications, dependent: :destroy + has_many :notifications, as: :resource, dependent: :destroy has_one_attached :image, dependent: :purge_later diff --git a/app/models/notification.rb b/app/models/notification.rb index 5553dfad9..20dea8f11 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -1,6 +1,7 @@ class Notification < ApplicationRecord belongs_to :user - belongs_to :bubble + belongs_to :creator, class_name: 'User' + belongs_to :resource, polymorphic: true scope :unread, -> { where(read: false) } scope :ordered, -> { order(read: :desc, created_at: :desc) } diff --git a/app/models/notifier.rb b/app/models/notifier.rb index be58c79c9..fb41e96f8 100644 --- a/app/models/notifier.rb +++ b/app/models/notifier.rb @@ -9,7 +9,7 @@ class Notifier def generate recipients.map do |recipient| - Notification.create! user: recipient, bubble: bubble, body: body + Notification.create! user: recipient, creator: event.creator, resource: bubble, body: body end end diff --git a/db/migrate/20250109153649_create_notifications.rb b/db/migrate/20250109153649_create_notifications.rb index 834d75b16..9da2376c5 100644 --- a/db/migrate/20250109153649_create_notifications.rb +++ b/db/migrate/20250109153649_create_notifications.rb @@ -2,13 +2,14 @@ class CreateNotifications < ActiveRecord::Migration[8.1] def change create_table :notifications do |t| t.references :user, null: false, foreign_key: true - t.references :bubble, null: false, foreign_key: true + t.references :creator, null: false, foreign_key: { to_table: :users } + t.references :resource, null: false, polymorphic: true, index: true t.boolean :read, default: false, null: false t.text :body, null: false t.timestamps - t.index %i[ user_id read created_at ], order: { created_at: :desc } + t.index %i[ user_id read created_at ], order: { read: :desc, created_at: :desc } end end end diff --git a/db/schema.rb b/db/schema.rb index da324c7fc..a267c8e6f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -178,13 +178,16 @@ ActiveRecord::Schema[8.1].define(version: 2025_01_09_153649) do create_table "notifications", force: :cascade do |t| t.integer "user_id", null: false - t.integer "bubble_id", null: false + t.integer "creator_id", null: false + t.string "resource_type", null: false + t.integer "resource_id", null: false t.boolean "read", default: false, null: false t.text "body", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.index ["bubble_id"], name: "index_notifications_on_bubble_id" - t.index ["user_id", "read", "created_at"], name: "index_notifications_on_user_id_and_read_and_created_at", order: { created_at: :desc } + t.index ["creator_id"], name: "index_notifications_on_creator_id" + t.index ["resource_type", "resource_id"], name: "index_notifications_on_resource" + t.index ["user_id", "read", "created_at"], name: "index_notifications_on_user_id_and_read_and_created_at", order: { read: :desc, created_at: :desc } t.index ["user_id"], name: "index_notifications_on_user_id" end @@ -256,8 +259,8 @@ ActiveRecord::Schema[8.1].define(version: 2025_01_09_153649) do add_foreign_key "bubbles", "workflow_stages", column: "stage_id" add_foreign_key "events", "event_summaries", column: "summary_id" add_foreign_key "messages", "bubbles" - add_foreign_key "notifications", "bubbles" add_foreign_key "notifications", "users" + add_foreign_key "notifications", "users", column: "creator_id" add_foreign_key "pops", "bubbles" add_foreign_key "pops", "users" add_foreign_key "sessions", "users" diff --git a/test/fixtures/notifications.yml b/test/fixtures/notifications.yml new file mode 100644 index 000000000..57a902def --- /dev/null +++ b/test/fixtures/notifications.yml @@ -0,0 +1,15 @@ +logo_created_kevin: + user: kevin + creator: david + resource: logo (Bubble) + read: false + body: "David created: The logo isn't big enough" + created_at: <%= 1.week.ago %> + +layout_created_kevin: + user: kevin + creator: david + resource: layout (Bubble) + read: false + body: "David created: Layout is broken" + created_at: <%= 1.week.ago %> From 8a515974085c85fdcfb2635eb6a6ef4382ad0981 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Fri, 10 Jan 2025 10:23:40 +0000 Subject: [PATCH 07/40] Add direct linking to comments --- app/models/bubble.rb | 4 +--- app/models/comment.rb | 2 +- app/models/concerns/notifiable.rb | 7 +++++++ config/routes.rb | 4 ++++ 4 files changed, 13 insertions(+), 4 deletions(-) create mode 100644 app/models/concerns/notifiable.rb diff --git a/app/models/bubble.rb b/app/models/bubble.rb index 1e617189d..67316354a 100644 --- a/app/models/bubble.rb +++ b/app/models/bubble.rb @@ -1,11 +1,9 @@ class Bubble < ApplicationRecord - include Assignable, Boostable, Colored, Commentable, Eventable, Messages, Poppable, Searchable, Staged, Taggable + include Assignable, Boostable, Colored, Commentable, Eventable, Messages, Notifiable, Poppable, Searchable, Staged, Taggable belongs_to :bucket, touch: true belongs_to :creator, class_name: "User", default: -> { Current.user } - has_many :notifications, as: :resource, dependent: :destroy - has_one_attached :image, dependent: :purge_later before_save :set_default_title diff --git a/app/models/comment.rb b/app/models/comment.rb index 3de28081f..b4fd7d0c5 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -1,5 +1,5 @@ class Comment < ApplicationRecord - include Searchable, Messageable + include Messageable, Notifiable, Searchable belongs_to :creator, class_name: "User", default: -> { Current.user } diff --git a/app/models/concerns/notifiable.rb b/app/models/concerns/notifiable.rb new file mode 100644 index 000000000..8cfc7b112 --- /dev/null +++ b/app/models/concerns/notifiable.rb @@ -0,0 +1,7 @@ +module Notifiable + extend ActiveSupport::Concern + + included do + has_many :notifications, as: :resource, dependent: :destroy + end +end diff --git a/config/routes.rb b/config/routes.rb index d0f860f96..6b5153088 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -12,6 +12,10 @@ Rails.application.routes.draw do route_for :bucket_bubble, bubble.bucket, bubble, options end + resolve "Comment" do |comment, options| + route_for :bucket_bubble, comment.bubble.bucket, comment.bubble, anchor: ActionView::RecordIdentifier.dom_id(comment) + end + resources :bubbles resources :notifications From 0c152b5439896edc42ac890a89bb1b08172c0964 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Fri, 10 Jan 2025 12:16:04 +0000 Subject: [PATCH 08/40] Style the tray a little bit --- app/assets/stylesheets/notifications.css | 61 ++++++++++++++++++- app/helpers/notifications_helper.rb | 4 +- app/views/bubbles/show.html.erb | 2 + .../notifications/_notification.html.erb | 10 ++- 4 files changed, 69 insertions(+), 8 deletions(-) diff --git a/app/assets/stylesheets/notifications.css b/app/assets/stylesheets/notifications.css index 164330528..03c21bfb5 100644 --- a/app/assets/stylesheets/notifications.css +++ b/app/assets/stylesheets/notifications.css @@ -1,15 +1,70 @@ .notification-tray { position: fixed; - width: 40ch; bottom: 1rem; left: calc(50% - 20ch); display: flex; flex-direction: column; gap: 1rem; + height: 4rem; + width: 40ch; } .notification { - background-color: #eeed; + position: absolute; + background-color: #eee; border-radius: 6px; - padding: 0.5rem; + border: solid 1px #ccc; + padding: 1rem; + display: flex; + align-items: center; + gap: 0.5rem; + width: 40ch; + bottom: 0; + + transform: translateY(var(--offset)); + z-index: var(--z-index); + transition: transform 0.2s; + + &:nth-child(1) { + --offset: 0; + --z-index: 0; + } + + &:nth-child(2) { + --offset: -8px; + --z-index: -1; + } + + &:nth-child(3) { + --offset: -16px; + --z-index: -2; + } + + &:nth-child(4) { + --offset: -24px; + --z-index: -3; + } + + &:nth-child(5) { + --offset: -32px; + --z-index: -4; + } +} + +.notification-tray:hover .notification { + &:nth-child(2) { + --offset: -100%; + } + + &:nth-child(3) { + --offset: -200%; + } + + &:nth-child(4) { + --offset: -300%; + } + + &:nth-child(5) { + --offset: -400%; + } } diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index b7fe8de4d..f159c1200 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -1,7 +1,7 @@ module NotificationsHelper def notification_tray_tag - tag.div class: "notification-tray" do - turbo_frame_tag "notifications", src: notifications_path, data: { turbo_permanent: true } + tag.div id: "notification-tray", class: "notification-tray", data: { turbo_permanent: true } do + turbo_frame_tag "notifications", src: notifications_path end end end diff --git a/app/views/bubbles/show.html.erb b/app/views/bubbles/show.html.erb index 00cd19766..681f91845 100644 --- a/app/views/bubbles/show.html.erb +++ b/app/views/bubbles/show.html.erb @@ -76,3 +76,5 @@
<%= turbo_frame_tag dom_id(@bubble, :stage_picker), src: new_bucket_bubble_stage_picker_path(@bubble.bucket, @bubble) %>
+ +<%= notification_tray_tag %> diff --git a/app/views/notifications/_notification.html.erb b/app/views/notifications/_notification.html.erb index c87ac7166..1d1bfd11f 100644 --- a/app/views/notifications/_notification.html.erb +++ b/app/views/notifications/_notification.html.erb @@ -1,3 +1,7 @@ -
-

<%= notification.body %>

-
+<%= link_to notification.resource, class: "notification", data: { turbo_frame: "_top" } do %> +
+ <%= avatar_image_tag(notification.creator) %> +
+ + <%= notification.body %> +<% end %> From dc53a39b9390c27ad41282a06fa62685d1a6a7ed Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Fri, 10 Jan 2025 12:46:00 +0000 Subject: [PATCH 09/40] Update notification body --- app/models/notifier/assigned.rb | 2 +- app/models/notifier/commented.rb | 2 +- app/models/notifier/created.rb | 2 +- test/models/notifier/assigned_test.rb | 2 +- test/models/notifier/created_test.rb | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/models/notifier/assigned.rb b/app/models/notifier/assigned.rb index c18d35a56..ff9594dd8 100644 --- a/app/models/notifier/assigned.rb +++ b/app/models/notifier/assigned.rb @@ -1,7 +1,7 @@ class Notifier::Assigned < Notifier private def body - "#{creator.name} assigned you: #{bubble.title}" + "assigned you: #{bubble.title}" end def recipients diff --git a/app/models/notifier/commented.rb b/app/models/notifier/commented.rb index aad76a591..d40476de6 100644 --- a/app/models/notifier/commented.rb +++ b/app/models/notifier/commented.rb @@ -1,7 +1,7 @@ class Notifier::Commented < Notifier private def body - "#{creator.name} commented on: #{bubble.title}" + "commented on: #{bubble.title}" end def recipients diff --git a/app/models/notifier/created.rb b/app/models/notifier/created.rb index 1bdc3fe2f..9630df5ae 100644 --- a/app/models/notifier/created.rb +++ b/app/models/notifier/created.rb @@ -1,7 +1,7 @@ class Notifier::Created < Notifier private def body - "#{creator.name} created a new item: #{bubble.title}" + "created: #{bubble.title}" end def recipients diff --git a/test/models/notifier/assigned_test.rb b/test/models/notifier/assigned_test.rb index eafcd682a..367531b0b 100644 --- a/test/models/notifier/assigned_test.rb +++ b/test/models/notifier/assigned_test.rb @@ -20,6 +20,6 @@ class Notifier::AssignedTest < ActiveSupport::TestCase test "generate populates the notification details" do Notifier.for(events(:logo_assignment_km)).generate - assert_equal "David assigned you: The logo isn't big enough", Notification.last.body + assert_equal "assigned you: The logo isn't big enough", Notification.last.body end end diff --git a/test/models/notifier/created_test.rb b/test/models/notifier/created_test.rb index 3e36cfcd6..b9ae7e7b8 100644 --- a/test/models/notifier/created_test.rb +++ b/test/models/notifier/created_test.rb @@ -14,6 +14,6 @@ class Notifier::CreatedTest < ActiveSupport::TestCase test "generate populates the notification details" do Notifier.for(events(:logo_created)).generate - assert_equal "David created a new item: The logo isn't big enough", Notification.last.body + assert_equal "created: The logo isn't big enough", Notification.last.body end end From fcce5b85e0ad80580226cf8a8e45b088739caf37 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Fri, 10 Jan 2025 12:57:44 +0000 Subject: [PATCH 10/40] Show notifications on bucket index --- app/views/buckets/index.html.erb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/views/buckets/index.html.erb b/app/views/buckets/index.html.erb index 9d831262b..22a1e00ce 100644 --- a/app/views/buckets/index.html.erb +++ b/app/views/buckets/index.html.erb @@ -20,3 +20,5 @@
<%= render @filters %>
+ +<%= notification_tray_tag %> From 58d5ed07a5ddb3a8117461f939e345ae5af42bb2 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Fri, 10 Jan 2025 12:57:59 +0000 Subject: [PATCH 11/40] Create notifications when tracking events --- app/models/bubble/eventable.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/models/bubble/eventable.rb b/app/models/bubble/eventable.rb index 05883a796..e8dd31e83 100644 --- a/app/models/bubble/eventable.rb +++ b/app/models/bubble/eventable.rb @@ -7,7 +7,8 @@ module Bubble::Eventable private def track_event(action, creator: Current.user, **particulars) - find_or_capture_event_summary.events.create! action: action, creator: creator, particulars: particulars + event = find_or_capture_event_summary.events.create! action: action, creator: creator, particulars: particulars + generate_notifications(event) end def find_or_capture_event_summary @@ -15,4 +16,8 @@ module Bubble::Eventable messages.last&.event_summary || capture(EventSummary.new).event_summary end end + + def generate_notifications(event) + Notifier.for(event)&.generate + end end From a7d9ec908c2c0687131da101e0336a46fd874f5c Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Fri, 10 Jan 2025 13:40:04 +0000 Subject: [PATCH 12/40] Adjust styles --- app/assets/stylesheets/notifications.css | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/app/assets/stylesheets/notifications.css b/app/assets/stylesheets/notifications.css index 03c21bfb5..041c1bf3f 100644 --- a/app/assets/stylesheets/notifications.css +++ b/app/assets/stylesheets/notifications.css @@ -21,6 +21,12 @@ width: 40ch; bottom: 0; + color: black; + + &:visited { + color: black; + } + transform: translateY(var(--offset)); z-index: var(--z-index); transition: transform 0.2s; @@ -49,6 +55,10 @@ --offset: -32px; --z-index: -4; } + + &:nth-child(1n + 6) { + display: none; + } } .notification-tray:hover .notification { From 876083c697fea8b32362ef279dbee2ff2469c65c Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Fri, 10 Jan 2025 13:40:38 +0000 Subject: [PATCH 13/40] Broadcast new notifications --- app/helpers/notifications_helper.rb | 17 +++++++++++++---- app/models/notification.rb | 2 ++ app/views/bubbles/index.html.erb | 2 +- app/views/bubbles/show.html.erb | 2 +- app/views/buckets/index.html.erb | 2 +- 5 files changed, 18 insertions(+), 7 deletions(-) diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index f159c1200..8ac611eb4 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -1,7 +1,16 @@ module NotificationsHelper - def notification_tray_tag - tag.div id: "notification-tray", class: "notification-tray", data: { turbo_permanent: true } do - turbo_frame_tag "notifications", src: notifications_path - end + def notification_tray + notification_stream_tag + notification_tray_tag end + + private + def notification_stream_tag + turbo_stream_from Current.user, :notifications + end + + def notification_tray_tag + tag.div id: "notification-tray", class: "notification-tray", data: { turbo_permanent: true } do + turbo_frame_tag("notifications", src: notifications_path) + end + end end diff --git a/app/models/notification.rb b/app/models/notification.rb index 20dea8f11..deaa9934a 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -5,4 +5,6 @@ class Notification < ApplicationRecord scope :unread, -> { where(read: false) } scope :ordered, -> { order(read: :desc, created_at: :desc) } + + broadcasts_to ->(notification) { [ notification.user, :notifications ] }, inserts_by: :prepend end diff --git a/app/views/bubbles/index.html.erb b/app/views/bubbles/index.html.erb index 7928afacf..a35ba09c9 100644 --- a/app/views/bubbles/index.html.erb +++ b/app/views/bubbles/index.html.erb @@ -51,4 +51,4 @@ -<%= notification_tray_tag %> +<%= notification_tray %> diff --git a/app/views/bubbles/show.html.erb b/app/views/bubbles/show.html.erb index 681f91845..938fcd864 100644 --- a/app/views/bubbles/show.html.erb +++ b/app/views/bubbles/show.html.erb @@ -77,4 +77,4 @@ <%= turbo_frame_tag dom_id(@bubble, :stage_picker), src: new_bucket_bubble_stage_picker_path(@bubble.bucket, @bubble) %> -<%= notification_tray_tag %> +<%= notification_tray %> diff --git a/app/views/buckets/index.html.erb b/app/views/buckets/index.html.erb index 22a1e00ce..b53e188fd 100644 --- a/app/views/buckets/index.html.erb +++ b/app/views/buckets/index.html.erb @@ -21,4 +21,4 @@ <%= render @filters %> -<%= notification_tray_tag %> +<%= notification_tray %> From 58ce85ce366ff1882113f1d8acb7aea1a13d51fd Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Fri, 10 Jan 2025 13:45:02 +0000 Subject: [PATCH 14/40] Link comment notifications to the comment --- app/models/notifier.rb | 10 +++++++--- app/models/notifier/commented.rb | 4 ++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/app/models/notifier.rb b/app/models/notifier.rb index fb41e96f8..e6942577e 100644 --- a/app/models/notifier.rb +++ b/app/models/notifier.rb @@ -9,7 +9,7 @@ class Notifier def generate recipients.map do |recipient| - Notification.create! user: recipient, creator: event.creator, resource: bubble, body: body + Notification.create! user: recipient, creator: event.creator, resource: resource, body: body end end @@ -27,10 +27,14 @@ class Notifier end def bubble - @event.summary.message.bubble + event.summary.message.bubble end def creator - @event.creator + event.creator + end + + def resource + bubble end end diff --git a/app/models/notifier/commented.rb b/app/models/notifier/commented.rb index d40476de6..02caf607c 100644 --- a/app/models/notifier/commented.rb +++ b/app/models/notifier/commented.rb @@ -7,4 +7,8 @@ class Notifier::Commented < Notifier def recipients bubble.bucket.users.without(creator) end + + def resource + event.comment + end end From fd1789ed58f3b0bf85858fa1d174e7cdf6ef295d Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Fri, 10 Jan 2025 14:26:00 +0000 Subject: [PATCH 15/40] Mark notifications read when clicked on --- .../notifications/readings_controller.rb | 5 +++++ app/helpers/notifications_helper.rb | 11 ++++++++++- .../notifications/readings_controller.js | 18 ++++++++++++++++++ app/views/notifications/_notification.html.erb | 2 +- config/routes.rb | 6 +++++- .../notifications/readings_controller_test.rb | 7 +++++++ 6 files changed, 46 insertions(+), 3 deletions(-) create mode 100644 app/controllers/notifications/readings_controller.rb create mode 100644 app/javascript/controllers/notifications/readings_controller.js create mode 100644 test/controllers/notifications/readings_controller_test.rb diff --git a/app/controllers/notifications/readings_controller.rb b/app/controllers/notifications/readings_controller.rb new file mode 100644 index 000000000..a5d0f7f11 --- /dev/null +++ b/app/controllers/notifications/readings_controller.rb @@ -0,0 +1,5 @@ +class Notifications::ReadingsController < ApplicationController + def create + Current.user.notifications.find(params[:notification_id]).update!(read: true) + end +end diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 8ac611eb4..1507a090f 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -3,13 +3,22 @@ module NotificationsHelper notification_stream_tag + notification_tray_tag end + def notification_tag(notification, &) + link_to notification.resource, class: "notification", + data: { + turbo_frame: "_top", + action: "notifications--readings#record", + notifications__readings_url_param: notification_readings_url(notification) + }, & + end + private def notification_stream_tag turbo_stream_from Current.user, :notifications end def notification_tray_tag - tag.div id: "notification-tray", class: "notification-tray", data: { turbo_permanent: true } do + tag.div id: "notification-tray", class: "notification-tray", data: { turbo_permanent: true, controller: "notifications--readings" } do turbo_frame_tag("notifications", src: notifications_path) end end diff --git a/app/javascript/controllers/notifications/readings_controller.js b/app/javascript/controllers/notifications/readings_controller.js new file mode 100644 index 000000000..5ec9ffca1 --- /dev/null +++ b/app/javascript/controllers/notifications/readings_controller.js @@ -0,0 +1,18 @@ +import { Controller } from "@hotwired/stimulus" + +export default class extends Controller { + record({ target, params: { url } }) { + navigator.sendBeacon(url, this.#csrfPayload()) + target.remove() + } + + #csrfPayload() { + const data = new FormData() + data.append("authenticity_token", this.#csrfToken()) + return data + } + + #csrfToken() { + return document.querySelector('meta[name="csrf-token"]').content + } +} diff --git a/app/views/notifications/_notification.html.erb b/app/views/notifications/_notification.html.erb index 1d1bfd11f..9a33e5c67 100644 --- a/app/views/notifications/_notification.html.erb +++ b/app/views/notifications/_notification.html.erb @@ -1,4 +1,4 @@ -<%= link_to notification.resource, class: "notification", data: { turbo_frame: "_top" } do %> +<%= notification_tag notification do %>
<%= avatar_image_tag(notification.creator) %>
diff --git a/config/routes.rb b/config/routes.rb index 6b5153088..38860013e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -17,7 +17,11 @@ Rails.application.routes.draw do end resources :bubbles - resources :notifications + resources :notifications do + scope module: :notifications do + resource :readings, only: :create + end + end resources :buckets do resources :bubbles do diff --git a/test/controllers/notifications/readings_controller_test.rb b/test/controllers/notifications/readings_controller_test.rb new file mode 100644 index 000000000..cce305a3d --- /dev/null +++ b/test/controllers/notifications/readings_controller_test.rb @@ -0,0 +1,7 @@ +require "test_helper" + +class Notifications::ReadingsControllerTest < ActionDispatch::IntegrationTest + # test "the truth" do + # assert true + # end +end From e67ac372d335423e0938d134d52bd75f3f090298 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Fri, 10 Jan 2025 14:27:36 +0000 Subject: [PATCH 16/40] Fix test --- test/models/bubble_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/models/bubble_test.rb b/test/models/bubble_test.rb index c8f4c3c16..33466acbd 100644 --- a/test/models/bubble_test.rb +++ b/test/models/bubble_test.rb @@ -6,11 +6,11 @@ class BubbleTest < ActiveSupport::TestCase end test "capturing messages" do - assert_difference "bubbles(:logo).messages.count", +1 do + assert_difference "bubbles(:logo).messages.comments.count", +1 do bubbles(:logo).capture Comment.new(body: "Agreed.") end - assert_equal "Agreed.", bubbles(:logo).messages.last.messageable.body.to_plain_text.chomp + assert_equal "Agreed.", bubbles(:logo).messages.comments.last.messageable.body.to_plain_text.chomp end test "boosting" do From 66d0f29d45a1f9d433428770f18609387450aed0 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Fri, 10 Jan 2025 14:41:36 +0000 Subject: [PATCH 17/40] Notify on pop as well And DRY up some duplication. --- app/models/bubble/poppable.rb | 5 ++++- app/models/notifier.rb | 2 +- app/models/notifier/commented.rb | 4 ---- app/models/notifier/created.rb | 4 ---- app/models/notifier/popped.rb | 6 ++++++ test/fixtures/events.yml | 6 ++++++ test/models/bubble/poppable_test.rb | 4 +++- test/models/notifier/popped_test.rb | 9 +++++++++ test/test_helpers/session_test_helper.rb | 8 ++++++++ 9 files changed, 37 insertions(+), 11 deletions(-) create mode 100644 app/models/notifier/popped.rb create mode 100644 test/models/notifier/popped_test.rb diff --git a/app/models/bubble/poppable.rb b/app/models/bubble/poppable.rb index b2043d5d0..c60474d89 100644 --- a/app/models/bubble/poppable.rb +++ b/app/models/bubble/poppable.rb @@ -13,7 +13,10 @@ module Bubble::Poppable end def pop!(user: Current.user) - create_pop!(user: user) unless popped? + unless popped? + create_pop!(user: user) + track_event :popped + end end def unpop diff --git a/app/models/notifier.rb b/app/models/notifier.rb index e6942577e..d61dd1c77 100644 --- a/app/models/notifier.rb +++ b/app/models/notifier.rb @@ -23,7 +23,7 @@ class Notifier end def recipients - raise NotImplementedError + bubble.bucket.users.without(creator) end def bubble diff --git a/app/models/notifier/commented.rb b/app/models/notifier/commented.rb index 02caf607c..178861405 100644 --- a/app/models/notifier/commented.rb +++ b/app/models/notifier/commented.rb @@ -4,10 +4,6 @@ class Notifier::Commented < Notifier "commented on: #{bubble.title}" end - def recipients - bubble.bucket.users.without(creator) - end - def resource event.comment end diff --git a/app/models/notifier/created.rb b/app/models/notifier/created.rb index 9630df5ae..03667f246 100644 --- a/app/models/notifier/created.rb +++ b/app/models/notifier/created.rb @@ -3,8 +3,4 @@ class Notifier::Created < Notifier def body "created: #{bubble.title}" end - - def recipients - bubble.bucket.users.without(creator) - end end diff --git a/app/models/notifier/popped.rb b/app/models/notifier/popped.rb new file mode 100644 index 000000000..26fdd72d3 --- /dev/null +++ b/app/models/notifier/popped.rb @@ -0,0 +1,6 @@ +class Notifier::Popped < Notifier + private + def body + "popped: #{bubble.title}" + end +end diff --git a/test/fixtures/events.yml b/test/fixtures/events.yml index d6be1dbe1..59ceb8621 100644 --- a/test/fixtures/events.yml +++ b/test/fixtures/events.yml @@ -72,3 +72,9 @@ shipping_created: action: created summary: shipping_initial_activity created_at: <%= 1.week.ago %> + +shipping_popped: + creator: kevin + action: popped + summary: shipping_initial_activity + created_at: <%= 2.days.ago %> diff --git a/test/models/bubble/poppable_test.rb b/test/models/bubble/poppable_test.rb index 5ce612309..cff8d1a38 100644 --- a/test/models/bubble/poppable_test.rb +++ b/test/models/bubble/poppable_test.rb @@ -9,7 +9,9 @@ class Bubble::PoppableTest < ActiveSupport::TestCase test "popping" do assert_not bubbles(:logo).popped? - bubbles(:logo).pop! + with_current_user(:kevin) do + bubbles(:logo).pop! + end assert bubbles(:logo).popped? end diff --git a/test/models/notifier/popped_test.rb b/test/models/notifier/popped_test.rb new file mode 100644 index 000000000..ca1289135 --- /dev/null +++ b/test/models/notifier/popped_test.rb @@ -0,0 +1,9 @@ +require "test_helper" + +class Notifier::PoppedTest < ActiveSupport::TestCase + test "generate populates the notification details" do + Notifier.for(events(:shipping_popped)).generate + + assert_equal "popped: We need to ship the app", Notification.last.body + end +end diff --git a/test/test_helpers/session_test_helper.rb b/test/test_helpers/session_test_helper.rb index 598afbd80..5666c5b28 100644 --- a/test/test_helpers/session_test_helper.rb +++ b/test/test_helpers/session_test_helper.rb @@ -13,4 +13,12 @@ module SessionTestHelper delete session_url assert_not cookies[:session_token].present? end + + def with_current_user(user) + user = users(user) unless user.is_a? User + Current.session = Session.new(user: user) + yield + ensure + Current.reset_all + end end From 6f7268f941ae27ff48107c8453d41b051ce3a398 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Fri, 10 Jan 2025 15:12:48 +0000 Subject: [PATCH 18/40] Set ID so a notification can't show up twice --- app/helpers/notifications_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 1507a090f..33a49f23a 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -4,7 +4,7 @@ module NotificationsHelper end def notification_tag(notification, &) - link_to notification.resource, class: "notification", + link_to notification.resource, id: dom_id(notification), class: "notification", data: { turbo_frame: "_top", action: "notifications--readings#record", From 07fcb7ee6ba19717c69117b22238f09da870ec6d Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Fri, 10 Jan 2025 15:12:59 +0000 Subject: [PATCH 19/40] More style shuffling --- app/assets/stylesheets/notifications.css | 27 +++++++++++-------- .../notifications/_notification.html.erb | 10 ++++--- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/app/assets/stylesheets/notifications.css b/app/assets/stylesheets/notifications.css index 041c1bf3f..fdb211269 100644 --- a/app/assets/stylesheets/notifications.css +++ b/app/assets/stylesheets/notifications.css @@ -11,22 +11,27 @@ .notification { position: absolute; - background-color: #eee; - border-radius: 6px; - border: solid 1px #ccc; - padding: 1rem; - display: flex; - align-items: center; - gap: 0.5rem; - width: 40ch; + padding: 4px; bottom: 0; + border-radius: 6px; - color: black; - - &:visited { + & .notification--content { + border-radius: 6px; + background-color: #eee; + border: solid 1px black; + padding: 1rem; + display: flex; + align-items: center; + gap: 0.5rem; + width: 40ch; color: black; + + &:visited { + color: black; + } } + transform: translateY(var(--offset)); z-index: var(--z-index); transition: transform 0.2s; diff --git a/app/views/notifications/_notification.html.erb b/app/views/notifications/_notification.html.erb index 9a33e5c67..7c7d99103 100644 --- a/app/views/notifications/_notification.html.erb +++ b/app/views/notifications/_notification.html.erb @@ -1,7 +1,9 @@ <%= notification_tag notification do %> -
- <%= avatar_image_tag(notification.creator) %> -
+
+
+ <%= avatar_image_tag(notification.creator) %> +
- <%= notification.body %> + <%= notification.body %> +
<% end %> From d53635822809480f8d3de137b113cfc443593be3 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Fri, 10 Jan 2025 15:15:13 +0000 Subject: [PATCH 20/40] Make fixture match expected content --- test/fixtures/notifications.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/fixtures/notifications.yml b/test/fixtures/notifications.yml index 57a902def..223f9d421 100644 --- a/test/fixtures/notifications.yml +++ b/test/fixtures/notifications.yml @@ -3,7 +3,7 @@ logo_created_kevin: creator: david resource: logo (Bubble) read: false - body: "David created: The logo isn't big enough" + body: "created: The logo isn't big enough" created_at: <%= 1.week.ago %> layout_created_kevin: @@ -11,5 +11,5 @@ layout_created_kevin: creator: david resource: layout (Bubble) read: false - body: "David created: Layout is broken" + body: "created: Layout is broken" created_at: <%= 1.week.ago %> From 31ac7950ce8d05dd406243818ffcf149ba02a92f Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Fri, 10 Jan 2025 15:34:58 +0000 Subject: [PATCH 21/40] Use turbo:click, not click Otherwise Turbo falls back to a regular Browser navigation. --- app/helpers/notifications_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 33a49f23a..9f84bd4b7 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -7,7 +7,7 @@ module NotificationsHelper link_to notification.resource, id: dom_id(notification), class: "notification", data: { turbo_frame: "_top", - action: "notifications--readings#record", + action: "turbo:click->notifications--readings#record", notifications__readings_url_param: notification_readings_url(notification) }, & end From 10c7b61af9e86e5b31e3138bf3f1de78d5f94a8e Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Fri, 10 Jan 2025 17:30:39 +0000 Subject: [PATCH 22/40] Split up a long line for clarity --- config/routes.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/config/routes.rb b/config/routes.rb index 38860013e..329e8fc7f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -13,7 +13,8 @@ Rails.application.routes.draw do end resolve "Comment" do |comment, options| - route_for :bucket_bubble, comment.bubble.bucket, comment.bubble, anchor: ActionView::RecordIdentifier.dom_id(comment) + options[:anchor] = ActionView::RecordIdentifier.dom_id(comment) + route_for :bucket_bubble, comment.bubble.bucket, comment.bubble, options end resources :bubbles From febcb6c344679f20b57e43a99dbc4e723acf2bd5 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Fri, 10 Jan 2025 17:37:54 +0000 Subject: [PATCH 23/40] A partial is clearer than a helper here --- app/helpers/notifications_helper.rb | 15 --------------- app/views/bubbles/index.html.erb | 2 +- app/views/bubbles/show.html.erb | 2 +- app/views/buckets/index.html.erb | 2 +- app/views/notifications/_tray.html.erb | 5 +++++ 5 files changed, 8 insertions(+), 18 deletions(-) create mode 100644 app/views/notifications/_tray.html.erb diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 9f84bd4b7..60a442ebd 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -1,8 +1,4 @@ module NotificationsHelper - def notification_tray - notification_stream_tag + notification_tray_tag - end - def notification_tag(notification, &) link_to notification.resource, id: dom_id(notification), class: "notification", data: { @@ -11,15 +7,4 @@ module NotificationsHelper notifications__readings_url_param: notification_readings_url(notification) }, & end - - private - def notification_stream_tag - turbo_stream_from Current.user, :notifications - end - - def notification_tray_tag - tag.div id: "notification-tray", class: "notification-tray", data: { turbo_permanent: true, controller: "notifications--readings" } do - turbo_frame_tag("notifications", src: notifications_path) - end - end end diff --git a/app/views/bubbles/index.html.erb b/app/views/bubbles/index.html.erb index a35ba09c9..40bc6772c 100644 --- a/app/views/bubbles/index.html.erb +++ b/app/views/bubbles/index.html.erb @@ -51,4 +51,4 @@ -<%= notification_tray %> +<%= render "notifications/tray" %> diff --git a/app/views/bubbles/show.html.erb b/app/views/bubbles/show.html.erb index 938fcd864..371600f9d 100644 --- a/app/views/bubbles/show.html.erb +++ b/app/views/bubbles/show.html.erb @@ -77,4 +77,4 @@ <%= turbo_frame_tag dom_id(@bubble, :stage_picker), src: new_bucket_bubble_stage_picker_path(@bubble.bucket, @bubble) %> -<%= notification_tray %> +<%= render "notifications/tray" %> diff --git a/app/views/buckets/index.html.erb b/app/views/buckets/index.html.erb index b53e188fd..7c2f59a91 100644 --- a/app/views/buckets/index.html.erb +++ b/app/views/buckets/index.html.erb @@ -21,4 +21,4 @@ <%= render @filters %> -<%= notification_tray %> +<%= render "notifications/tray" %> diff --git a/app/views/notifications/_tray.html.erb b/app/views/notifications/_tray.html.erb new file mode 100644 index 000000000..9b9d91c40 --- /dev/null +++ b/app/views/notifications/_tray.html.erb @@ -0,0 +1,5 @@ +<%= turbo_stream_from Current.user, :notifications %> + +<%= tag.div id: "notification-tray", class: "notification-tray", data: { turbo_permanent: true, controller: "notifications--readings" } do %> + <%= turbo_frame_tag("notifications", src: notifications_path) %> +<% end %> From df1225e58dbc669cb6ce976ec410b59ea9e8b19e Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Sat, 11 Jan 2025 16:44:21 +0000 Subject: [PATCH 24/40] Style --- app/models/notification.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/notification.rb b/app/models/notification.rb index deaa9934a..ecc60eda0 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -1,6 +1,6 @@ class Notification < ApplicationRecord belongs_to :user - belongs_to :creator, class_name: 'User' + belongs_to :creator, class_name: "User" belongs_to :resource, polymorphic: true scope :unread, -> { where(read: false) } From c62e800b08a1939acc6587b1c35e170cd14fd39c Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Mon, 13 Jan 2025 11:19:52 +0000 Subject: [PATCH 25/40] Notifications are all related to a Bubble --- app/models/bubble.rb | 2 ++ app/models/notification.rb | 1 + app/models/notifier.rb | 2 +- db/migrate/20250109153649_create_notifications.rb | 1 + db/schema.rb | 3 +++ test/fixtures/notifications.yml | 2 ++ 6 files changed, 10 insertions(+), 1 deletion(-) diff --git a/app/models/bubble.rb b/app/models/bubble.rb index 67316354a..5b6a0ec93 100644 --- a/app/models/bubble.rb +++ b/app/models/bubble.rb @@ -4,6 +4,8 @@ class Bubble < ApplicationRecord belongs_to :bucket, touch: true belongs_to :creator, class_name: "User", default: -> { Current.user } + has_many :notifications, dependent: :destroy + has_one_attached :image, dependent: :purge_later before_save :set_default_title diff --git a/app/models/notification.rb b/app/models/notification.rb index ecc60eda0..4c47a926b 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -1,6 +1,7 @@ class Notification < ApplicationRecord belongs_to :user belongs_to :creator, class_name: "User" + belongs_to :bubble belongs_to :resource, polymorphic: true scope :unread, -> { where(read: false) } diff --git a/app/models/notifier.rb b/app/models/notifier.rb index d61dd1c77..eef797d6e 100644 --- a/app/models/notifier.rb +++ b/app/models/notifier.rb @@ -9,7 +9,7 @@ class Notifier def generate recipients.map do |recipient| - Notification.create! user: recipient, creator: event.creator, resource: resource, body: body + Notification.create! user: recipient, creator: event.creator, bubble: bubble, resource: resource, body: body end end diff --git a/db/migrate/20250109153649_create_notifications.rb b/db/migrate/20250109153649_create_notifications.rb index 9da2376c5..c2d97b93f 100644 --- a/db/migrate/20250109153649_create_notifications.rb +++ b/db/migrate/20250109153649_create_notifications.rb @@ -3,6 +3,7 @@ class CreateNotifications < ActiveRecord::Migration[8.1] create_table :notifications do |t| t.references :user, null: false, foreign_key: true t.references :creator, null: false, foreign_key: { to_table: :users } + t.references :bubble, null: false, foreign_key: true t.references :resource, null: false, polymorphic: true, index: true t.boolean :read, default: false, null: false t.text :body, null: false diff --git a/db/schema.rb b/db/schema.rb index a267c8e6f..d8a702e39 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -179,12 +179,14 @@ ActiveRecord::Schema[8.1].define(version: 2025_01_09_153649) do create_table "notifications", force: :cascade do |t| t.integer "user_id", null: false t.integer "creator_id", null: false + t.integer "bubble_id", null: false t.string "resource_type", null: false t.integer "resource_id", null: false t.boolean "read", default: false, null: false t.text "body", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.index ["bubble_id"], name: "index_notifications_on_bubble_id" t.index ["creator_id"], name: "index_notifications_on_creator_id" t.index ["resource_type", "resource_id"], name: "index_notifications_on_resource" t.index ["user_id", "read", "created_at"], name: "index_notifications_on_user_id_and_read_and_created_at", order: { read: :desc, created_at: :desc } @@ -259,6 +261,7 @@ ActiveRecord::Schema[8.1].define(version: 2025_01_09_153649) do add_foreign_key "bubbles", "workflow_stages", column: "stage_id" add_foreign_key "events", "event_summaries", column: "summary_id" add_foreign_key "messages", "bubbles" + add_foreign_key "notifications", "bubbles" add_foreign_key "notifications", "users" add_foreign_key "notifications", "users", column: "creator_id" add_foreign_key "pops", "bubbles" diff --git a/test/fixtures/notifications.yml b/test/fixtures/notifications.yml index 223f9d421..554456028 100644 --- a/test/fixtures/notifications.yml +++ b/test/fixtures/notifications.yml @@ -1,6 +1,7 @@ logo_created_kevin: user: kevin creator: david + bubble: logo resource: logo (Bubble) read: false body: "created: The logo isn't big enough" @@ -9,6 +10,7 @@ logo_created_kevin: layout_created_kevin: user: kevin creator: david + bubble: logo resource: layout (Bubble) read: false body: "created: Layout is broken" From 271ac063f95d880de605e5948bc0b8c8a8190ad5 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Mon, 13 Jan 2025 11:21:07 +0000 Subject: [PATCH 26/40] Adjust a fixture --- test/fixtures/notifications.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/fixtures/notifications.yml b/test/fixtures/notifications.yml index 554456028..bb0035b95 100644 --- a/test/fixtures/notifications.yml +++ b/test/fixtures/notifications.yml @@ -11,7 +11,7 @@ layout_created_kevin: user: kevin creator: david bubble: logo - resource: layout (Bubble) + resource: layout_overflowing_david (Comment) read: false - body: "created: Layout is broken" + body: "re: Layout is broken" created_at: <%= 1.week.ago %> From 46989f6ebe9611ebab218e581cab3f1751148665 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Mon, 13 Jan 2025 12:31:31 +0000 Subject: [PATCH 27/40] Mark notification read when viewing the Bubble --- .../notifications/readings_controller.rb | 5 ----- app/controllers/readings_controller.rb | 8 ++++++++ app/helpers/notifications_helper.rb | 6 +----- .../notifications/readings_controller.js | 18 ------------------ .../controllers/readings_controller.js | 10 ++++++++++ app/views/bubbles/_bubble.html.erb | 4 +++- app/views/bubbles/show.html.erb | 2 +- app/views/notifications/_tray.html.erb | 2 +- app/views/readings/create.turbo_stream.erb | 3 +++ config/importmap.rb | 1 + config/routes.rb | 7 ++----- .../notifications/readings_controller_test.rb | 7 ------- test/fixtures/notifications.yml | 2 +- vendor/javascript/@rails--request.js | 4 ++++ 14 files changed, 35 insertions(+), 44 deletions(-) delete mode 100644 app/controllers/notifications/readings_controller.rb create mode 100644 app/controllers/readings_controller.rb delete mode 100644 app/javascript/controllers/notifications/readings_controller.js create mode 100644 app/javascript/controllers/readings_controller.js create mode 100644 app/views/readings/create.turbo_stream.erb delete mode 100644 test/controllers/notifications/readings_controller_test.rb create mode 100644 vendor/javascript/@rails--request.js diff --git a/app/controllers/notifications/readings_controller.rb b/app/controllers/notifications/readings_controller.rb deleted file mode 100644 index a5d0f7f11..000000000 --- a/app/controllers/notifications/readings_controller.rb +++ /dev/null @@ -1,5 +0,0 @@ -class Notifications::ReadingsController < ApplicationController - def create - Current.user.notifications.find(params[:notification_id]).update!(read: true) - end -end diff --git a/app/controllers/readings_controller.rb b/app/controllers/readings_controller.rb new file mode 100644 index 000000000..5fc57ec9c --- /dev/null +++ b/app/controllers/readings_controller.rb @@ -0,0 +1,8 @@ +class ReadingsController < ApplicationController + include BubbleScoped, BucketScoped + + def create + @notifications = Current.user.notifications.where(bubble: @bubble) + @notifications.update(read: true) + end +end diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 60a442ebd..8c2208a14 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -1,10 +1,6 @@ module NotificationsHelper def notification_tag(notification, &) link_to notification.resource, id: dom_id(notification), class: "notification", - data: { - turbo_frame: "_top", - action: "turbo:click->notifications--readings#record", - notifications__readings_url_param: notification_readings_url(notification) - }, & + data: { turbo_frame: "_top", }, & end end diff --git a/app/javascript/controllers/notifications/readings_controller.js b/app/javascript/controllers/notifications/readings_controller.js deleted file mode 100644 index 5ec9ffca1..000000000 --- a/app/javascript/controllers/notifications/readings_controller.js +++ /dev/null @@ -1,18 +0,0 @@ -import { Controller } from "@hotwired/stimulus" - -export default class extends Controller { - record({ target, params: { url } }) { - navigator.sendBeacon(url, this.#csrfPayload()) - target.remove() - } - - #csrfPayload() { - const data = new FormData() - data.append("authenticity_token", this.#csrfToken()) - return data - } - - #csrfToken() { - return document.querySelector('meta[name="csrf-token"]').content - } -} diff --git a/app/javascript/controllers/readings_controller.js b/app/javascript/controllers/readings_controller.js new file mode 100644 index 000000000..07182bb36 --- /dev/null +++ b/app/javascript/controllers/readings_controller.js @@ -0,0 +1,10 @@ +import { post } from "@rails/request.js" +import { Controller } from "@hotwired/stimulus" + +export default class extends Controller { + static values = { url: String } + + connect() { + post(this.urlValue, { responseKind: "turbo-stream" }) + } +} diff --git a/app/views/bubbles/_bubble.html.erb b/app/views/bubbles/_bubble.html.erb index 033364ed5..f00d502ff 100644 --- a/app/views/bubbles/_bubble.html.erb +++ b/app/views/bubbles/_bubble.html.erb @@ -2,7 +2,9 @@
+ data-animation-play-class="bubble--wobble" + data-animation-play-on-load-value="true" + data-action="mouseover->animation#play">

diff --git a/app/views/bubbles/show.html.erb b/app/views/bubbles/show.html.erb index 371600f9d..6c65a65bf 100644 --- a/app/views/bubbles/show.html.erb +++ b/app/views/bubbles/show.html.erb @@ -65,7 +65,7 @@

-
+
<%= render "bubbles/bubble", bubble: @bubble, editing: params[:editing] %>
diff --git a/app/views/notifications/_tray.html.erb b/app/views/notifications/_tray.html.erb index 9b9d91c40..af1c2d452 100644 --- a/app/views/notifications/_tray.html.erb +++ b/app/views/notifications/_tray.html.erb @@ -1,5 +1,5 @@ <%= turbo_stream_from Current.user, :notifications %> -<%= tag.div id: "notification-tray", class: "notification-tray", data: { turbo_permanent: true, controller: "notifications--readings" } do %> +<%= tag.div id: "notification-tray", class: "notification-tray", data: { turbo_permanent: true } do %> <%= turbo_frame_tag("notifications", src: notifications_path) %> <% end %> diff --git a/app/views/readings/create.turbo_stream.erb b/app/views/readings/create.turbo_stream.erb new file mode 100644 index 000000000..5d7fe7801 --- /dev/null +++ b/app/views/readings/create.turbo_stream.erb @@ -0,0 +1,3 @@ +<% @notifications.each do |notification| %> + <%= turbo_stream.remove notification %> +<% end %> diff --git a/config/importmap.rb b/config/importmap.rb index bf42e394c..87974cc7f 100644 --- a/config/importmap.rb +++ b/config/importmap.rb @@ -4,6 +4,7 @@ pin "application" pin "@hotwired/turbo-rails", to: "turbo.min.js" pin "@hotwired/stimulus", to: "stimulus.min.js" pin "@hotwired/stimulus-loading", to: "stimulus-loading.js" +pin "@rails/request.js", to: "@rails--request.js" # @0.0.11 pin "house", to: "house.min.js" pin_all_from "app/javascript/controllers", under: "controllers" diff --git a/config/routes.rb b/config/routes.rb index 329e8fc7f..ad0d9a2cf 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -18,16 +18,13 @@ Rails.application.routes.draw do end resources :bubbles - resources :notifications do - scope module: :notifications do - resource :readings, only: :create - end - end + resources :notifications resources :buckets do resources :bubbles do resources :boosts resources :comments + resource :readings, only: :create scope module: :bubbles do resource :image diff --git a/test/controllers/notifications/readings_controller_test.rb b/test/controllers/notifications/readings_controller_test.rb deleted file mode 100644 index cce305a3d..000000000 --- a/test/controllers/notifications/readings_controller_test.rb +++ /dev/null @@ -1,7 +0,0 @@ -require "test_helper" - -class Notifications::ReadingsControllerTest < ActionDispatch::IntegrationTest - # test "the truth" do - # assert true - # end -end diff --git a/test/fixtures/notifications.yml b/test/fixtures/notifications.yml index bb0035b95..802dcf68b 100644 --- a/test/fixtures/notifications.yml +++ b/test/fixtures/notifications.yml @@ -10,7 +10,7 @@ logo_created_kevin: layout_created_kevin: user: kevin creator: david - bubble: logo + bubble: layout resource: layout_overflowing_david (Comment) read: false body: "re: Layout is broken" diff --git a/vendor/javascript/@rails--request.js b/vendor/javascript/@rails--request.js new file mode 100644 index 000000000..b85d5c78a --- /dev/null +++ b/vendor/javascript/@rails--request.js @@ -0,0 +1,4 @@ +// @rails/request.js@0.0.11 downloaded from https://ga.jspm.io/npm:@rails/request.js@0.0.11/src/index.js + +class FetchResponse{constructor(t){this.response=t}get statusCode(){return this.response.status}get redirected(){return this.response.redirected}get ok(){return this.response.ok}get unauthenticated(){return this.statusCode===401}get unprocessableEntity(){return this.statusCode===422}get authenticationURL(){return this.response.headers.get("WWW-Authenticate")}get contentType(){const t=this.response.headers.get("Content-Type")||"";return t.replace(/;.*$/,"")}get headers(){return this.response.headers}get html(){return this.contentType.match(/^(application|text)\/(html|xhtml\+xml)$/)?this.text:Promise.reject(new Error(`Expected an HTML response but got "${this.contentType}" instead`))}get json(){return this.contentType.match(/^application\/.*json$/)?this.responseJson||(this.responseJson=this.response.json()):Promise.reject(new Error(`Expected a JSON response but got "${this.contentType}" instead`))}get text(){return this.responseText||(this.responseText=this.response.text())}get isTurboStream(){return this.contentType.match(/^text\/vnd\.turbo-stream\.html/)}get isScript(){return this.contentType.match(/\b(?:java|ecma)script\b/)}async renderTurboStream(){if(!this.isTurboStream)return Promise.reject(new Error(`Expected a Turbo Stream response but got "${this.contentType}" instead`));window.Turbo?await window.Turbo.renderStreamMessage(await this.text):console.warn("You must set `window.Turbo = Turbo` to automatically process Turbo Stream events with request.js")}async activeScript(){if(!this.isScript)return Promise.reject(new Error(`Expected a Script response but got "${this.contentType}" instead`));{const t=document.createElement("script");const e=document.querySelector("meta[name=csp-nonce]");const n=e&&e.content;n&&t.setAttribute("nonce",n);t.innerHTML=await this.text;document.body.appendChild(t)}}}class RequestInterceptor{static register(t){this.interceptor=t}static get(){return this.interceptor}static reset(){this.interceptor=void 0}}function getCookie(t){const e=document.cookie?document.cookie.split("; "):[];const n=`${encodeURIComponent(t)}=`;const s=e.find((t=>t.startsWith(n)));if(s){const t=s.split("=").slice(1).join("=");if(t)return decodeURIComponent(t)}}function compact(t){const e={};for(const n in t){const s=t[n];s!==void 0&&(e[n]=s)}return e}function metaContent(t){const e=document.head.querySelector(`meta[name="${t}"]`);return e&&e.content}function stringEntriesFromFormData(t){return[...t].reduce(((t,[e,n])=>t.concat(typeof n==="string"?[[e,n]]:[])),[])}function mergeEntries(t,e){for(const[n,s]of e)if(!(s instanceof window.File))if(t.has(n)&&!n.includes("[]")){t.delete(n);t.set(n,s)}else t.append(n,s)}class FetchRequest{constructor(t,e,n={}){this.method=t;this.options=n;this.originalUrl=e.toString()}async perform(){try{const t=RequestInterceptor.get();t&&await t(this)}catch(t){console.error(t)}const t=this.responseKind==="turbo-stream"&&window.Turbo?window.Turbo.fetch:window.fetch;const e=new FetchResponse(await t(this.url,this.fetchOptions));if(e.unauthenticated&&e.authenticationURL)return Promise.reject(window.location.href=e.authenticationURL);e.isScript&&await e.activeScript();const n=e.ok||e.unprocessableEntity;n&&e.isTurboStream&&await e.renderTurboStream();return e}addHeader(t,e){const n=this.additionalHeaders;n[t]=e;this.options.headers=n}sameHostname(){if(!this.originalUrl.startsWith("http:"))return true;try{return new URL(this.originalUrl).hostname===window.location.hostname}catch(t){return true}}get fetchOptions(){return{method:this.method.toUpperCase(),headers:this.headers,body:this.formattedBody,signal:this.signal,credentials:this.credentials,redirect:this.redirect}}get headers(){const t={"X-Requested-With":"XMLHttpRequest","Content-Type":this.contentType,Accept:this.accept};this.sameHostname()&&(t["X-CSRF-Token"]=this.csrfToken);return compact(Object.assign(t,this.additionalHeaders))}get csrfToken(){return getCookie(metaContent("csrf-param"))||metaContent("csrf-token")}get contentType(){return this.options.contentType?this.options.contentType:this.body==null||this.body instanceof window.FormData?void 0:this.body instanceof window.File?this.body.type:"application/json"}get accept(){switch(this.responseKind){case"html":return"text/html, application/xhtml+xml";case"turbo-stream":return"text/vnd.turbo-stream.html, text/html, application/xhtml+xml";case"json":return"application/json, application/vnd.api+json";case"script":return"text/javascript, application/javascript";default:return"*/*"}}get body(){return this.options.body}get query(){const t=(this.originalUrl.split("?")[1]||"").split("#")[0];const e=new URLSearchParams(t);let n=this.options.query;n=n instanceof window.FormData?stringEntriesFromFormData(n):n instanceof window.URLSearchParams?n.entries():Object.entries(n||{});mergeEntries(e,n);const s=e.toString();return s.length>0?`?${s}`:""}get url(){return this.originalUrl.split("?")[0].split("#")[0]+this.query}get responseKind(){return this.options.responseKind||"html"}get signal(){return this.options.signal}get redirect(){return this.options.redirect||"follow"}get credentials(){return this.options.credentials||"same-origin"}get additionalHeaders(){return this.options.headers||{}}get formattedBody(){const t=Object.prototype.toString.call(this.body)==="[object String]";const e=this.headers["Content-Type"]==="application/json";return e&&!t?JSON.stringify(this.body):this.body}}async function get(t,e){const n=new FetchRequest("get",t,e);return n.perform()}async function post(t,e){const n=new FetchRequest("post",t,e);return n.perform()}async function put(t,e){const n=new FetchRequest("put",t,e);return n.perform()}async function patch(t,e){const n=new FetchRequest("patch",t,e);return n.perform()}async function destroy(t,e){const n=new FetchRequest("delete",t,e);return n.perform()}export{FetchRequest,FetchResponse,RequestInterceptor,destroy,get,patch,post,put}; + From 196575c0a297aae8f7643b327d44580d90798c96 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Mon, 13 Jan 2025 12:54:15 +0000 Subject: [PATCH 28/40] Use a job to generate notifications --- app/jobs/generate_notifications_job.rb | 7 +++++++ app/models/bubble/eventable.rb | 6 +----- app/models/event.rb | 8 ++++++++ 3 files changed, 16 insertions(+), 5 deletions(-) create mode 100644 app/jobs/generate_notifications_job.rb diff --git a/app/jobs/generate_notifications_job.rb b/app/jobs/generate_notifications_job.rb new file mode 100644 index 000000000..e386b7b37 --- /dev/null +++ b/app/jobs/generate_notifications_job.rb @@ -0,0 +1,7 @@ +class GenerateNotificationsJob < ApplicationJob + queue_as :default + + def perform(event) + event.generate_notifications + end +end diff --git a/app/models/bubble/eventable.rb b/app/models/bubble/eventable.rb index e8dd31e83..64379e360 100644 --- a/app/models/bubble/eventable.rb +++ b/app/models/bubble/eventable.rb @@ -8,7 +8,7 @@ module Bubble::Eventable private def track_event(action, creator: Current.user, **particulars) event = find_or_capture_event_summary.events.create! action: action, creator: creator, particulars: particulars - generate_notifications(event) + event.generate_notifications_later end def find_or_capture_event_summary @@ -16,8 +16,4 @@ module Bubble::Eventable messages.last&.event_summary || capture(EventSummary.new).event_summary end end - - def generate_notifications(event) - Notifier.for(event)&.generate - end end diff --git a/app/models/event.rb b/app/models/event.rb index 0c01b1f45..95d4e1a54 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -9,4 +9,12 @@ class Event < ApplicationRecord scope :chronologically, -> { order created_at: :asc, id: :desc } scope :non_boosts, -> { where.not action: :boosted } scope :boosts, -> { where action: :boosted } + + def generate_notifications + Notifier.for(self)&.generate + end + + def generate_notifications_later + GenerateNotificationsJob.perform_later self + end end From 64795804ba5489b3d66631c05cb5378faa8dfa45 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Mon, 13 Jan 2025 13:15:20 +0000 Subject: [PATCH 29/40] Style --- app/helpers/notifications_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 8c2208a14..02b2e3cde 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -1,6 +1,6 @@ module NotificationsHelper def notification_tag(notification, &) link_to notification.resource, id: dom_id(notification), class: "notification", - data: { turbo_frame: "_top", }, & + data: { turbo_frame: "_top" }, & end end From 5f88cd3900280290636bd9e533d622763cc1cb85 Mon Sep 17 00:00:00 2001 From: Jason Zimdars Date: Mon, 13 Jan 2025 20:46:51 -0600 Subject: [PATCH 30/40] Design for notifications --- app/assets/stylesheets/notifications.css | 93 +++++++++---------- app/helpers/notifications_helper.rb | 2 +- app/models/notifier/assigned.rb | 2 +- app/models/notifier/commented.rb | 2 +- app/models/notifier/created.rb | 2 +- app/models/notifier/popped.rb | 2 +- .../notifications/_notification.html.erb | 7 +- app/views/notifications/_tray.html.erb | 2 +- 8 files changed, 57 insertions(+), 55 deletions(-) diff --git a/app/assets/stylesheets/notifications.css b/app/assets/stylesheets/notifications.css index fdb211269..4b58106a9 100644 --- a/app/assets/stylesheets/notifications.css +++ b/app/assets/stylesheets/notifications.css @@ -1,85 +1,84 @@ .notification-tray { - position: fixed; - bottom: 1rem; - left: calc(50% - 20ch); - display: flex; - flex-direction: column; - gap: 1rem; height: 4rem; - width: 40ch; + inset: auto auto var(--block-space); + position: fixed; } .notification { + inset: auto auto 0; position: absolute; - padding: 4px; - bottom: 0; - border-radius: 6px; + transform: translate(var(--offsetX), var(--offsetY)); + transform-origin: center center; + transition: transform 0.2s ease-in-out; + z-index: var(--z-index); - & .notification--content { - border-radius: 6px; - background-color: #eee; - border: solid 1px black; - padding: 1rem; - display: flex; - align-items: center; - gap: 0.5rem; - width: 40ch; - color: black; - - &:visited { - color: black; - } + .notification__content { + color: var(--color-ink); + inline-size: var(--width); } - - transform: translateY(var(--offset)); - z-index: var(--z-index); - transition: transform 0.2s; - &:nth-child(1) { - --offset: 0; + --offsetX: 0; + --offsetY: 0; + --width: 40ch; --z-index: 0; } &:nth-child(2) { - --offset: -8px; + --offsetX: 0.5ch; + --offsetY: calc(var(--block-space-half) * -1); + --width: 39ch; --z-index: -1; } &:nth-child(3) { - --offset: -16px; + --offsetX: 1ch; + --offsetY: calc(var(--block-space) * -1); + --width: 38ch; --z-index: -2; } &:nth-child(4) { - --offset: -24px; + --offsetX: 1.5ch; + --offsetY: calc(var(--block-space) * -1.5); + --width: 37ch; --z-index: -3; } &:nth-child(5) { - --offset: -32px; + --offsetX: 2ch; + --offsetY: calc(var(--block-space) * -2); + --width: 36ch; --z-index: -4; } &:nth-child(1n + 6) { display: none; } -} -.notification-tray:hover .notification { - &:nth-child(2) { - --offset: -100%; - } + .notification-tray:hover & { + &:nth-child(2) { + --offsetX: 0; + --offsetY: calc((100% + var(--block-space-half)) * -1); + --width: 40ch; + } - &:nth-child(3) { - --offset: -200%; - } + &:nth-child(3) { + --offsetX: 0; + --offsetY: calc((100% + var(--block-space-half)) * -2); + --width: 40ch; + } - &:nth-child(4) { - --offset: -300%; - } + &:nth-child(4) { + --offsetX: 0; + --offsetY: calc((100% + var(--block-space-half)) * -3); + --width: 40ch; + } - &:nth-child(5) { - --offset: -400%; + &:nth-child(5) { + --offsetX: 0; + --offsetY: calc((100% + var(--block-space-half)) * -4); + --width: 40ch; + } } } diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 02b2e3cde..61c40f19e 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -1,6 +1,6 @@ module NotificationsHelper def notification_tag(notification, &) - link_to notification.resource, id: dom_id(notification), class: "notification", + link_to notification.resource, id: dom_id(notification), class: "notification border-radius", data: { turbo_frame: "_top" }, & end end diff --git a/app/models/notifier/assigned.rb b/app/models/notifier/assigned.rb index ff9594dd8..52ac2644b 100644 --- a/app/models/notifier/assigned.rb +++ b/app/models/notifier/assigned.rb @@ -1,7 +1,7 @@ class Notifier::Assigned < Notifier private def body - "assigned you: #{bubble.title}" + "#{event.creator.name} assigned to you" end def recipients diff --git a/app/models/notifier/commented.rb b/app/models/notifier/commented.rb index 178861405..72cd01602 100644 --- a/app/models/notifier/commented.rb +++ b/app/models/notifier/commented.rb @@ -1,7 +1,7 @@ class Notifier::Commented < Notifier private def body - "commented on: #{bubble.title}" + "#{event.creator.name}" end def resource diff --git a/app/models/notifier/created.rb b/app/models/notifier/created.rb index 03667f246..a8da65c63 100644 --- a/app/models/notifier/created.rb +++ b/app/models/notifier/created.rb @@ -1,6 +1,6 @@ class Notifier::Created < Notifier private def body - "created: #{bubble.title}" + "Added by #{event.creator.name}" end end diff --git a/app/models/notifier/popped.rb b/app/models/notifier/popped.rb index 26fdd72d3..7fe7588cd 100644 --- a/app/models/notifier/popped.rb +++ b/app/models/notifier/popped.rb @@ -1,6 +1,6 @@ class Notifier::Popped < Notifier private def body - "popped: #{bubble.title}" + "Popped by #{event.creator.name}" end end diff --git a/app/views/notifications/_notification.html.erb b/app/views/notifications/_notification.html.erb index 7c7d99103..faf721470 100644 --- a/app/views/notifications/_notification.html.erb +++ b/app/views/notifications/_notification.html.erb @@ -1,9 +1,12 @@ <%= notification_tag notification do %> -
+
<%= avatar_image_tag(notification.creator) %>
- <%= notification.body %> +
+ <%= "RE: " if notification.resource.is_a?(Comment) %><%= notification.bubble.title %> + <%= notification.body %> · <%= time_ago_in_words(notification.created_at) %> ago +
<% end %> diff --git a/app/views/notifications/_tray.html.erb b/app/views/notifications/_tray.html.erb index af1c2d452..71ada65cb 100644 --- a/app/views/notifications/_tray.html.erb +++ b/app/views/notifications/_tray.html.erb @@ -1,5 +1,5 @@ <%= turbo_stream_from Current.user, :notifications %> -<%= tag.div id: "notification-tray", class: "notification-tray", data: { turbo_permanent: true } do %> +<%= tag.div id: "notification-tray", class: "notification-tray flex flex-column gap", data: { turbo_permanent: true } do %> <%= turbo_frame_tag("notifications", src: notifications_path) %> <% end %> From 30b58f716bee89328cbdc1f0836fb1f6e412331e Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Tue, 14 Jan 2025 10:21:47 +0000 Subject: [PATCH 31/40] Make `title` a notifier attribute --- app/models/notifier.rb | 7 ++++++- app/models/notifier/commented.rb | 4 ++++ app/views/notifications/_notification.html.erb | 2 +- db/migrate/20250109153649_create_notifications.rb | 1 + db/schema.rb | 1 + test/fixtures/notifications.yml | 6 ++++-- 6 files changed, 17 insertions(+), 4 deletions(-) diff --git a/app/models/notifier.rb b/app/models/notifier.rb index eef797d6e..9e286aa2f 100644 --- a/app/models/notifier.rb +++ b/app/models/notifier.rb @@ -9,7 +9,8 @@ class Notifier def generate recipients.map do |recipient| - Notification.create! user: recipient, creator: event.creator, bubble: bubble, resource: resource, body: body + Notification.create! user: recipient, creator: event.creator, + bubble: bubble, resource: resource, title: title, body: body end end @@ -18,6 +19,10 @@ class Notifier @event = event end + def title + bubble.title + end + def body raise NotImplementedError end diff --git a/app/models/notifier/commented.rb b/app/models/notifier/commented.rb index 72cd01602..8b416a517 100644 --- a/app/models/notifier/commented.rb +++ b/app/models/notifier/commented.rb @@ -1,5 +1,9 @@ class Notifier::Commented < Notifier private + def title + "RE: " + super + end + def body "#{event.creator.name}" end diff --git a/app/views/notifications/_notification.html.erb b/app/views/notifications/_notification.html.erb index faf721470..6e55e5b75 100644 --- a/app/views/notifications/_notification.html.erb +++ b/app/views/notifications/_notification.html.erb @@ -5,7 +5,7 @@
- <%= "RE: " if notification.resource.is_a?(Comment) %><%= notification.bubble.title %> + <%= notification.title %> <%= notification.body %> · <%= time_ago_in_words(notification.created_at) %> ago
diff --git a/db/migrate/20250109153649_create_notifications.rb b/db/migrate/20250109153649_create_notifications.rb index c2d97b93f..6a493d514 100644 --- a/db/migrate/20250109153649_create_notifications.rb +++ b/db/migrate/20250109153649_create_notifications.rb @@ -6,6 +6,7 @@ class CreateNotifications < ActiveRecord::Migration[8.1] t.references :bubble, null: false, foreign_key: true t.references :resource, null: false, polymorphic: true, index: true t.boolean :read, default: false, null: false + t.text :title, null: false t.text :body, null: false t.timestamps diff --git a/db/schema.rb b/db/schema.rb index d8a702e39..23aa986a1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -183,6 +183,7 @@ ActiveRecord::Schema[8.1].define(version: 2025_01_09_153649) do t.string "resource_type", null: false t.integer "resource_id", null: false t.boolean "read", default: false, null: false + t.text "title", null: false t.text "body", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false diff --git a/test/fixtures/notifications.yml b/test/fixtures/notifications.yml index 802dcf68b..656331761 100644 --- a/test/fixtures/notifications.yml +++ b/test/fixtures/notifications.yml @@ -4,7 +4,8 @@ logo_created_kevin: bubble: logo resource: logo (Bubble) read: false - body: "created: The logo isn't big enough" + title: Logo isn't big enough + body: "Added by: David" created_at: <%= 1.week.ago %> layout_created_kevin: @@ -13,5 +14,6 @@ layout_created_kevin: bubble: layout resource: layout_overflowing_david (Comment) read: false - body: "re: Layout is broken" + title: "RE: Layout is broken" + body: "David" created_at: <%= 1.week.ago %> From ab154f2f3518814e5cdde6813036141b7b5a28cb Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Tue, 14 Jan 2025 10:45:40 +0000 Subject: [PATCH 32/40] Generate notification content in a helper --- app/helpers/notifications_helper.rb | 21 ++++++++++++++++ app/models/notification.rb | 4 ++- app/models/notifier.rb | 25 ++++++------------- app/models/notifier/assigned.rb | 4 --- app/models/notifier/commented.rb | 8 ------ app/models/notifier/created.rb | 4 --- app/models/notifier/popped.rb | 4 --- .../notifications/_notification.html.erb | 4 +-- .../20250109153649_create_notifications.rb | 4 +-- db/schema.rb | 8 +++--- test/fixtures/events.yml | 7 ++++++ test/fixtures/notifications.yml | 10 +++----- 12 files changed, 47 insertions(+), 56 deletions(-) diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 61c40f19e..7c35a5080 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -1,4 +1,25 @@ module NotificationsHelper + def notification_title(notification) + title = notification.bubble.title + + if notification.resource.is_a? Comment + "RE: " + title + else + title + end + end + + def notification_body(notification) + name = notification.creator.name + + case notification.event.action + when "assigned" then "#{name} assigned to you" + when "created" then "Added by #{name}" + when "popped" then "Popped by by #{name}" + else name + end + end + def notification_tag(notification, &) link_to notification.resource, id: dom_id(notification), class: "notification border-radius", data: { turbo_frame: "_top" }, & diff --git a/app/models/notification.rb b/app/models/notification.rb index 4c47a926b..531be12bb 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -1,11 +1,13 @@ class Notification < ApplicationRecord belongs_to :user - belongs_to :creator, class_name: "User" + belongs_to :event belongs_to :bubble belongs_to :resource, polymorphic: true scope :unread, -> { where(read: false) } scope :ordered, -> { order(read: :desc, created_at: :desc) } + delegate :creator, to: :event + broadcasts_to ->(notification) { [ notification.user, :notifications ] }, inserts_by: :prepend end diff --git a/app/models/notifier.rb b/app/models/notifier.rb index 9e286aa2f..cb64ec77c 100644 --- a/app/models/notifier.rb +++ b/app/models/notifier.rb @@ -1,6 +1,8 @@ class Notifier attr_reader :event + delegate :creator, to: :event + class << self def for(event) "Notifier::#{event.action.classify}".safe_constantize&.new(event) @@ -9,8 +11,7 @@ class Notifier def generate recipients.map do |recipient| - Notification.create! user: recipient, creator: event.creator, - bubble: bubble, resource: resource, title: title, body: body + Notification.create! user: recipient, event: event, bubble: bubble, resource: resource end end @@ -19,27 +20,15 @@ class Notifier @event = event end - def title - bubble.title - end - - def body - raise NotImplementedError - end - def recipients bubble.bucket.users.without(creator) end - def bubble - event.summary.message.bubble - end - - def creator - event.creator - end - def resource bubble end + + def bubble + event.summary.message.bubble + end end diff --git a/app/models/notifier/assigned.rb b/app/models/notifier/assigned.rb index 52ac2644b..cabf8452a 100644 --- a/app/models/notifier/assigned.rb +++ b/app/models/notifier/assigned.rb @@ -1,9 +1,5 @@ class Notifier::Assigned < Notifier private - def body - "#{event.creator.name} assigned to you" - end - def recipients event.assignees.without(creator) end diff --git a/app/models/notifier/commented.rb b/app/models/notifier/commented.rb index 8b416a517..42ad4e5c8 100644 --- a/app/models/notifier/commented.rb +++ b/app/models/notifier/commented.rb @@ -1,13 +1,5 @@ class Notifier::Commented < Notifier private - def title - "RE: " + super - end - - def body - "#{event.creator.name}" - end - def resource event.comment end diff --git a/app/models/notifier/created.rb b/app/models/notifier/created.rb index a8da65c63..f2724a931 100644 --- a/app/models/notifier/created.rb +++ b/app/models/notifier/created.rb @@ -1,6 +1,2 @@ class Notifier::Created < Notifier - private - def body - "Added by #{event.creator.name}" - end end diff --git a/app/models/notifier/popped.rb b/app/models/notifier/popped.rb index 7fe7588cd..f83e721ce 100644 --- a/app/models/notifier/popped.rb +++ b/app/models/notifier/popped.rb @@ -1,6 +1,2 @@ class Notifier::Popped < Notifier - private - def body - "Popped by #{event.creator.name}" - end end diff --git a/app/views/notifications/_notification.html.erb b/app/views/notifications/_notification.html.erb index 6e55e5b75..97b884347 100644 --- a/app/views/notifications/_notification.html.erb +++ b/app/views/notifications/_notification.html.erb @@ -5,8 +5,8 @@
- <%= notification.title %> - <%= notification.body %> · <%= time_ago_in_words(notification.created_at) %> ago + <%= notification_title(notification) %> + <%= notification_body(notification) %> · <%= time_ago_in_words(notification.created_at) %> ago
<% end %> diff --git a/db/migrate/20250109153649_create_notifications.rb b/db/migrate/20250109153649_create_notifications.rb index 6a493d514..c4b1391d9 100644 --- a/db/migrate/20250109153649_create_notifications.rb +++ b/db/migrate/20250109153649_create_notifications.rb @@ -2,12 +2,10 @@ class CreateNotifications < ActiveRecord::Migration[8.1] def change create_table :notifications do |t| t.references :user, null: false, foreign_key: true - t.references :creator, null: false, foreign_key: { to_table: :users } + t.references :event, null: false, foreign_key: true t.references :bubble, null: false, foreign_key: true t.references :resource, null: false, polymorphic: true, index: true t.boolean :read, default: false, null: false - t.text :title, null: false - t.text :body, null: false t.timestamps diff --git a/db/schema.rb b/db/schema.rb index 23aa986a1..b50200c5c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -178,17 +178,15 @@ ActiveRecord::Schema[8.1].define(version: 2025_01_09_153649) do create_table "notifications", force: :cascade do |t| t.integer "user_id", null: false - t.integer "creator_id", null: false + t.integer "event_id", null: false t.integer "bubble_id", null: false t.string "resource_type", null: false t.integer "resource_id", null: false t.boolean "read", default: false, null: false - t.text "title", null: false - t.text "body", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false t.index ["bubble_id"], name: "index_notifications_on_bubble_id" - t.index ["creator_id"], name: "index_notifications_on_creator_id" + t.index ["event_id"], name: "index_notifications_on_event_id" t.index ["resource_type", "resource_id"], name: "index_notifications_on_resource" t.index ["user_id", "read", "created_at"], name: "index_notifications_on_user_id_and_read_and_created_at", order: { read: :desc, created_at: :desc } t.index ["user_id"], name: "index_notifications_on_user_id" @@ -263,8 +261,8 @@ ActiveRecord::Schema[8.1].define(version: 2025_01_09_153649) do add_foreign_key "events", "event_summaries", column: "summary_id" add_foreign_key "messages", "bubbles" add_foreign_key "notifications", "bubbles" + add_foreign_key "notifications", "events" add_foreign_key "notifications", "users" - add_foreign_key "notifications", "users", column: "creator_id" add_foreign_key "pops", "bubbles" add_foreign_key "pops", "users" add_foreign_key "sessions", "users" diff --git a/test/fixtures/events.yml b/test/fixtures/events.yml index 59ceb8621..8cf272f34 100644 --- a/test/fixtures/events.yml +++ b/test/fixtures/events.yml @@ -54,6 +54,13 @@ layout_created: summary: layout_initial_activity created_at: <%= 1.week.ago %> +layout_commented: + creator: david + action: comment + summary: layout_initial_activity + particulars: <%= { comment_id: ActiveRecord::FixtureSet.identify(:layout_overflowing_david) }.to_json %> + created_at: <%= 1.week.ago %> + layout_assignment_jz: creator: david action: assigned diff --git a/test/fixtures/notifications.yml b/test/fixtures/notifications.yml index 656331761..c73954558 100644 --- a/test/fixtures/notifications.yml +++ b/test/fixtures/notifications.yml @@ -1,19 +1,15 @@ logo_created_kevin: user: kevin - creator: david + event: logo_created bubble: logo resource: logo (Bubble) read: false - title: Logo isn't big enough - body: "Added by: David" created_at: <%= 1.week.ago %> -layout_created_kevin: +layout_commented_kevin: user: kevin - creator: david + event: layout_commented bubble: layout resource: layout_overflowing_david (Comment) read: false - title: "RE: Layout is broken" - body: "David" created_at: <%= 1.week.ago %> From 78b7fe0f3da0ac4a87fccf4d7c8663f4e184ce1a Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Tue, 14 Jan 2025 13:08:13 +0000 Subject: [PATCH 33/40] Fix tests --- test/fixtures/events.yml | 2 +- test/models/notifier/assigned_test.rb | 11 ++++++----- test/models/notifier/commented_test.rb | 19 +++++++++++++++++++ test/models/notifier/created_test.rb | 6 +++--- test/models/notifier/popped_test.rb | 14 ++++++++++++-- 5 files changed, 41 insertions(+), 11 deletions(-) create mode 100644 test/models/notifier/commented_test.rb diff --git a/test/fixtures/events.yml b/test/fixtures/events.yml index 8cf272f34..f1232faa6 100644 --- a/test/fixtures/events.yml +++ b/test/fixtures/events.yml @@ -56,7 +56,7 @@ layout_created: layout_commented: creator: david - action: comment + action: commented summary: layout_initial_activity particulars: <%= { comment_id: ActiveRecord::FixtureSet.identify(:layout_overflowing_david) }.to_json %> created_at: <%= 1.week.ago %> diff --git a/test/models/notifier/assigned_test.rb b/test/models/notifier/assigned_test.rb index 367531b0b..73a6a1236 100644 --- a/test/models/notifier/assigned_test.rb +++ b/test/models/notifier/assigned_test.rb @@ -1,7 +1,7 @@ require "test_helper" class Notifier::AssignedTest < ActiveSupport::TestCase - test "generate creates a notification for each recipient" do + test "creates a notification for the assignee" do assert_difference -> { Notification.count }, 1 do assert_difference -> { users(:kevin).notifications.count }, 1 do Notifier.for(events(:logo_assignment_km)).generate @@ -9,17 +9,18 @@ class Notifier::AssignedTest < ActiveSupport::TestCase end end - test "generate does not notify for self-assignments" do - event = EventSummary.last.events.create! action: :assigned, creator: users(:kevin), particulars: { assignee_ids: [ users(:kevin).id ] } + test "does not notify for self-assignments" do + event = EventSummary.last.events.create! action: :assigned, creator: users(:kevin), + particulars: { assignee_ids: [ users(:kevin).id ] } assert_no_difference -> { Notification.count } do Notifier.for(event).generate end end - test "generate populates the notification details" do + test "links to the bubble" do Notifier.for(events(:logo_assignment_km)).generate - assert_equal "assigned you: The logo isn't big enough", Notification.last.body + assert_equal bubbles(:logo), Notification.last.resource end end diff --git a/test/models/notifier/commented_test.rb b/test/models/notifier/commented_test.rb new file mode 100644 index 000000000..b56275a6b --- /dev/null +++ b/test/models/notifier/commented_test.rb @@ -0,0 +1,19 @@ +require "test_helper" + +class Notifier::CommentedTest < ActiveSupport::TestCase + test "creates a notification for each recipient" do + assert_difference -> { Notification.count }, 2 do + assert_difference -> { users(:kevin).notifications.count }, 1 do + assert_difference -> { users(:jz).notifications.count }, 1 do + Notifier.for(events(:layout_commented)).generate + end + end + end + end + + test "links to the bubble" do + Notifier.for(events(:layout_commented)).generate + + assert_equal comments(:layout_overflowing_david), Notification.last.resource + end +end diff --git a/test/models/notifier/created_test.rb b/test/models/notifier/created_test.rb index b9ae7e7b8..47bad1a05 100644 --- a/test/models/notifier/created_test.rb +++ b/test/models/notifier/created_test.rb @@ -1,7 +1,7 @@ require "test_helper" class Notifier::CreatedTest < ActiveSupport::TestCase - test "generate creates a notification for each recipient" do + test "creates a notification for each recipient" do assert_difference -> { Notification.count }, 2 do assert_difference -> { users(:kevin).notifications.count }, 1 do assert_difference -> { users(:jz).notifications.count }, 1 do @@ -11,9 +11,9 @@ class Notifier::CreatedTest < ActiveSupport::TestCase end end - test "generate populates the notification details" do + test "links to the bubble" do Notifier.for(events(:logo_created)).generate - assert_equal "created: The logo isn't big enough", Notification.last.body + assert_equal bubbles(:logo), Notification.last.resource end end diff --git a/test/models/notifier/popped_test.rb b/test/models/notifier/popped_test.rb index ca1289135..842a3f675 100644 --- a/test/models/notifier/popped_test.rb +++ b/test/models/notifier/popped_test.rb @@ -1,9 +1,19 @@ require "test_helper" class Notifier::PoppedTest < ActiveSupport::TestCase - test "generate populates the notification details" do + test "creates a notification for each recipient" do + assert_difference -> { Notification.count }, 2 do + assert_difference -> { users(:david).notifications.count }, 1 do + assert_difference -> { users(:jz).notifications.count }, 1 do + Notifier.for(events(:shipping_popped)).generate + end + end + end + end + + test "links to the bubble" do Notifier.for(events(:shipping_popped)).generate - assert_equal "popped: We need to ship the app", Notification.last.body + assert_equal bubbles(:shipping), Notification.last.resource end end From df260840ab87d2da2465c0b3169ab3a8abee66d7 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Tue, 14 Jan 2025 13:29:39 +0000 Subject: [PATCH 34/40] Simplify test --- test/models/notifier/assigned_test.rb | 11 +++++------ test/models/notifier/commented_test.rb | 10 +++------- test/models/notifier/created_test.rb | 10 +++------- test/models/notifier/popped_test.rb | 10 +++------- 4 files changed, 14 insertions(+), 27 deletions(-) diff --git a/test/models/notifier/assigned_test.rb b/test/models/notifier/assigned_test.rb index 73a6a1236..c78adce5f 100644 --- a/test/models/notifier/assigned_test.rb +++ b/test/models/notifier/assigned_test.rb @@ -2,15 +2,14 @@ require "test_helper" class Notifier::AssignedTest < ActiveSupport::TestCase test "creates a notification for the assignee" do - assert_difference -> { Notification.count }, 1 do - assert_difference -> { users(:kevin).notifications.count }, 1 do - Notifier.for(events(:logo_assignment_km)).generate - end - end + notifications = Notifier.for(events(:logo_assignment_km)).generate + + assert_equal [ users(:kevin) ], notifications.map(&:user) end test "does not notify for self-assignments" do - event = EventSummary.last.events.create! action: :assigned, creator: users(:kevin), + event = EventSummary.last.events.create! action: :assigned, + creator: users(:kevin), particulars: { assignee_ids: [ users(:kevin).id ] } assert_no_difference -> { Notification.count } do diff --git a/test/models/notifier/commented_test.rb b/test/models/notifier/commented_test.rb index b56275a6b..d106b4e40 100644 --- a/test/models/notifier/commented_test.rb +++ b/test/models/notifier/commented_test.rb @@ -2,13 +2,9 @@ require "test_helper" class Notifier::CommentedTest < ActiveSupport::TestCase test "creates a notification for each recipient" do - assert_difference -> { Notification.count }, 2 do - assert_difference -> { users(:kevin).notifications.count }, 1 do - assert_difference -> { users(:jz).notifications.count }, 1 do - Notifier.for(events(:layout_commented)).generate - end - end - end + notifications = Notifier.for(events(:layout_commented)).generate + + assert_equal users(:kevin, :jz).sort, notifications.map(&:user).sort end test "links to the bubble" do diff --git a/test/models/notifier/created_test.rb b/test/models/notifier/created_test.rb index 47bad1a05..0c7aaa546 100644 --- a/test/models/notifier/created_test.rb +++ b/test/models/notifier/created_test.rb @@ -2,13 +2,9 @@ require "test_helper" class Notifier::CreatedTest < ActiveSupport::TestCase test "creates a notification for each recipient" do - assert_difference -> { Notification.count }, 2 do - assert_difference -> { users(:kevin).notifications.count }, 1 do - assert_difference -> { users(:jz).notifications.count }, 1 do - Notifier.for(events(:logo_created)).generate - end - end - end + notifications = Notifier.for(events(:logo_created)).generate + + assert_equal users(:kevin, :jz).sort, notifications.map(&:user).sort end test "links to the bubble" do diff --git a/test/models/notifier/popped_test.rb b/test/models/notifier/popped_test.rb index 842a3f675..300568114 100644 --- a/test/models/notifier/popped_test.rb +++ b/test/models/notifier/popped_test.rb @@ -2,13 +2,9 @@ require "test_helper" class Notifier::PoppedTest < ActiveSupport::TestCase test "creates a notification for each recipient" do - assert_difference -> { Notification.count }, 2 do - assert_difference -> { users(:david).notifications.count }, 1 do - assert_difference -> { users(:jz).notifications.count }, 1 do - Notifier.for(events(:shipping_popped)).generate - end - end - end + notifications = Notifier.for(events(:shipping_popped)).generate + + assert_equal users(:david, :jz).sort, notifications.map(&:user).sort end test "links to the bubble" do From c11b5af5e0009db102f681b9bbc13963b7041c6d Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Tue, 14 Jan 2025 13:37:54 +0000 Subject: [PATCH 35/40] Make Stimulus controller more generic --- .../{readings_controller.js => beacon_controller.js} | 0 app/views/bubbles/show.html.erb | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename app/javascript/controllers/{readings_controller.js => beacon_controller.js} (100%) diff --git a/app/javascript/controllers/readings_controller.js b/app/javascript/controllers/beacon_controller.js similarity index 100% rename from app/javascript/controllers/readings_controller.js rename to app/javascript/controllers/beacon_controller.js diff --git a/app/views/bubbles/show.html.erb b/app/views/bubbles/show.html.erb index 6c65a65bf..db4dab58f 100644 --- a/app/views/bubbles/show.html.erb +++ b/app/views/bubbles/show.html.erb @@ -65,7 +65,7 @@ -
+
<%= render "bubbles/bubble", bubble: @bubble, editing: params[:editing] %>
From 415e872a2becbeb04243a9516fabd1fcbe9d6734 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Tue, 14 Jan 2025 13:57:48 +0000 Subject: [PATCH 36/40] Combine concerns --- app/models/event.rb | 2 +- app/models/event/assignments.rb | 11 ----------- app/models/event/comment.rb | 11 ----------- app/models/event/particulars.rb | 15 +++++++++++++++ app/models/event/stages.rb | 7 ------- 5 files changed, 16 insertions(+), 30 deletions(-) delete mode 100644 app/models/event/assignments.rb delete mode 100644 app/models/event/comment.rb create mode 100644 app/models/event/particulars.rb delete mode 100644 app/models/event/stages.rb diff --git a/app/models/event.rb b/app/models/event.rb index 95d4e1a54..8dfceeeeb 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -1,5 +1,5 @@ class Event < ApplicationRecord - include Assignments, Comment, Stages + include Particulars belongs_to :creator, class_name: "User" belongs_to :summary, touch: true, class_name: "EventSummary" diff --git a/app/models/event/assignments.rb b/app/models/event/assignments.rb deleted file mode 100644 index 56a6b9e6e..000000000 --- a/app/models/event/assignments.rb +++ /dev/null @@ -1,11 +0,0 @@ -module Event::Assignments - extend ActiveSupport::Concern - - included do - store_accessor :particulars, :assignee_ids - end - - def assignees - @assignees ||= account.users.where id: assignee_ids - end -end diff --git a/app/models/event/comment.rb b/app/models/event/comment.rb deleted file mode 100644 index f6237e7b5..000000000 --- a/app/models/event/comment.rb +++ /dev/null @@ -1,11 +0,0 @@ -module Event::Comment - extend ActiveSupport::Concern - - included do - store_accessor :particulars, :comment_id - end - - def comment - @comment ||= Comment.find(comment_id) - end -end diff --git a/app/models/event/particulars.rb b/app/models/event/particulars.rb new file mode 100644 index 000000000..e21b20a7c --- /dev/null +++ b/app/models/event/particulars.rb @@ -0,0 +1,15 @@ +module Event::Particulars + extend ActiveSupport::Concern + + included do + store_accessor :particulars, :assignee_ids, :comment_id, :stage_id, :stage_name + end + + def assignees + @assignees ||= account.users.where id: assignee_ids + end + + def comment + @comment ||= Comment.find(comment_id) + end +end diff --git a/app/models/event/stages.rb b/app/models/event/stages.rb deleted file mode 100644 index c6abe0bfc..000000000 --- a/app/models/event/stages.rb +++ /dev/null @@ -1,7 +0,0 @@ -module Event::Stages - extend ActiveSupport::Concern - - included do - store_accessor :particulars, :stage_id, :stage_name - end -end From 9ddb7b260b931119968ab4893a1241a0e567c6ec Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Tue, 14 Jan 2025 14:07:21 +0000 Subject: [PATCH 37/40] Remove empty test --- test/models/notification_test.rb | 4 ---- 1 file changed, 4 deletions(-) delete mode 100644 test/models/notification_test.rb diff --git a/test/models/notification_test.rb b/test/models/notification_test.rb deleted file mode 100644 index 449f3c08d..000000000 --- a/test/models/notification_test.rb +++ /dev/null @@ -1,4 +0,0 @@ -require "test_helper" - -class NotificationTest < ActiveSupport::TestCase -end From 9fc5311ddc662097a92411890c4637b11f358e5f Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Tue, 14 Jan 2025 14:10:24 +0000 Subject: [PATCH 38/40] Add simple controller test --- test/controllers/notifications_controller_test.rb | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/test/controllers/notifications_controller_test.rb b/test/controllers/notifications_controller_test.rb index 20b28201d..fc4c2bfee 100644 --- a/test/controllers/notifications_controller_test.rb +++ b/test/controllers/notifications_controller_test.rb @@ -1,7 +1,14 @@ require "test_helper" class NotificationsControllerTest < ActionDispatch::IntegrationTest - # test "the truth" do - # assert true - # end + setup do + sign_in_as :kevin + end + + test "index" do + get notifications_url + + assert_response :success + assert_select "div", text: /Layout is broken/ + end end From b8ecfa0125b90e9a7dbf20fea728c1f538d4316e Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Tue, 14 Jan 2025 14:23:46 +0000 Subject: [PATCH 39/40] Replace notifications on read, instead of removing Previously when a notification was read, we'd remove it from the list. This will often be fine, but if there were more unread notifications that hadn't been loaded yet (because there were too many for the initial page) then the list could become empty when really there are still more items to show. So instead of removing the items when they're read, we can reload the list with the new page of items. This should allow the older notifications to come into view when necessary. --- app/controllers/readings_controller.rb | 9 +++++++-- app/views/readings/create.turbo_stream.erb | 4 ++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/app/controllers/readings_controller.rb b/app/controllers/readings_controller.rb index 5fc57ec9c..9c5798ed3 100644 --- a/app/controllers/readings_controller.rb +++ b/app/controllers/readings_controller.rb @@ -2,7 +2,12 @@ class ReadingsController < ApplicationController include BubbleScoped, BucketScoped def create - @notifications = Current.user.notifications.where(bubble: @bubble) - @notifications.update(read: true) + mark_bubble_notifications_read + @notifications = Current.user.notifications.unread.ordered.limit(20) end + + private + def mark_bubble_notifications_read + Current.user.notifications.where(bubble: @bubble).update(read: true) + end end diff --git a/app/views/readings/create.turbo_stream.erb b/app/views/readings/create.turbo_stream.erb index 5d7fe7801..a66c881ce 100644 --- a/app/views/readings/create.turbo_stream.erb +++ b/app/views/readings/create.turbo_stream.erb @@ -1,3 +1,3 @@ -<% @notifications.each do |notification| %> - <%= turbo_stream.remove notification %> +<%= turbo_stream.update :notifications do %> + <%= render @notifications %> <% end %> From 345acb1c0b88a0e3e99d03181bd488283a6ebfce Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Tue, 14 Jan 2025 14:38:46 +0000 Subject: [PATCH 40/40] Add test for readings controller --- test/controllers/readings_controller_test.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 test/controllers/readings_controller_test.rb diff --git a/test/controllers/readings_controller_test.rb b/test/controllers/readings_controller_test.rb new file mode 100644 index 000000000..bb173b4a2 --- /dev/null +++ b/test/controllers/readings_controller_test.rb @@ -0,0 +1,15 @@ +require "test_helper" + +class ReadingsControllerTest < ActionDispatch::IntegrationTest + setup do + sign_in_as :kevin + end + + test "index" do + assert_changes -> { notifications(:logo_created_kevin).reload.read? }, from: false, to: true do + post bucket_bubble_readings_url(bubbles(:logo).bucket, bubbles(:logo)), as: :turbo_stream + end + + assert_response :success + end +end