bterkuile/fizzy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Stack notifications everywhere

Browse Source 
We had client-side notification stacking in the tray since launch, but now we want to stack notifications in the notifications page, in API responses and in email bundles.
This commit is contained in:
Stanko K.R.
2026-02-12 10:04:32 +01:00
parent 680a718379
commit 36ee253a1a
35 changed files with 265 additions and 320 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/controllers/cards/readings_controller.rb
+2 -2
View File
@@ -2,12 +2,12 @@ class Cards::ReadingsController < ApplicationController
include CardScoped
def create
@notifications = @card.read_by(Current.user)
@card.read_by(Current.user)
record_board_access
end
def destroy
@notifications = @card.unread_by(Current.user)
@card.unread_by(Current.user)
record_board_access
end
app/controllers/notifications/trays_controller.rb
+1 -1
View File
@@ -1,6 +1,6 @@
class Notifications::TraysController < ApplicationController
def show
@notifications = Current.user.notifications.preloaded.unread.ordered.limit(100)
@notifications = Current.user.notifications.unread.preloaded.ordered.limit(100)
# Invalidate on the whole set instead of the unread set since the max updated at in the unread set
# can stay the same when reading old notifications.
app/helpers/notifications_helper.rb
+2 -4
View File
@@ -29,9 +29,7 @@ module NotificationsHelper
def notification_tag(notification, &)
tag.div id: dom_id(notification), class: "tray__item tray__item--notification", data: {
navigable_list_target: "item",
notifications_tray_target: "notification",
card_id: notification.card.id,
timestamp: notification.created_at.to_i
card_id: notification.card.id
} do
link_to(notification,
class: [ "card card--notification", { "card--closed": notification.card.closed? }, { "unread": !notification.read? } ],
@@ -58,7 +56,7 @@ module NotificationsHelper
data: { action: "form#submit:stop badge#update:stop", form_target: "submit" },
form: { data: { controller: "form" } } do
concat(icon_tag("remove"))
concat(tag.span("1", class: "badge-count", data: { group_count: "" }))
concat(tag.span(notification.unread_count, class: "badge-count")) if notification.unread_count > 1
end
end
end
app/javascript/controllers/notifications_tray_controller.js
-89
View File
@@ -1,89 +0,0 @@
import { Controller } from "@hotwired/stimulus"
export default class extends Controller {
static targets = [ "notification", "hiddenNotifications" ]
static classes = [ "grouped" ]
connect() {
this.group()
}
group() {
const notificationsByCardId = this.#groupNotificationsByCardId()
for (const cardId in notificationsByCardId) {
const notifications = notificationsByCardId[cardId]
if (notifications.length > 1) {
this.#renderGroup(notifications)
}
}
this.grouped = true
}
notificationTargetConnected(notification) {
if (this.grouped && notification.parentElement !== this.hiddenNotificationsTarget) {
this.#groupNotification(notification)
}
}
#groupNotificationsByCardId() {
const notificationsByCardId = {}
this.notificationTargets.forEach(notification => {
const cardId = notification.dataset.cardId
notificationsByCardId[cardId] ||= []
notificationsByCardId[cardId].push(notification)
})
return notificationsByCardId
}
#groupNotification(notification) {
const groupedNotifications = this.#groupedNotificationsFor(notification)
if (groupedNotifications.length > 1) {
this.#renderGroup(groupedNotifications)
}
}
#groupedNotificationsFor(notification) {
const cardId = notification.dataset.cardId
return this.notificationTargets
.filter(notification => notification.dataset.cardId === cardId)
}
#renderGroup(groupedNotifications) {
groupedNotifications.sort((notification_1, notification_2) => parseInt(notification_1.dataset.timestamp) - parseInt(notification_2.dataset.timestamp))
this.#showAsGrouped(groupedNotifications[0], groupedNotifications.length)
groupedNotifications.slice(1).forEach(notification => this.#hideInGroup(notification))
}
#showAsGrouped(notification, size) {
notification.classList.add(this.groupedClass)
this.#showGroupedNotification(notification)
this.#setGroupCount(notification, size)
}
#hideInGroup(notification) {
this.#hideGroupedNotification(notification)
this.#setGroupCount(notification, "")
}
// We use a hidden container instead of hiding the notifications directly so that the CSS that sort the
// tray element indexes doesn't get messed up with the child positions changing.
#showGroupedNotification(notification) {
if (notification.parentElement === this.hiddenNotificationsTarget) {
this.hiddenNotificationsTarget.parentElement.insertBefore(notification, this.hiddenNotificationsTarget)
}
}
#hideGroupedNotification(notification) {
this.hiddenNotificationsTarget.appendChild(notification)
}
#setGroupCount(notification, count) {
notification.querySelector("[data-group-count]").textContent = count
}
}
app/models/card/readable.rb
+2 -18
View File
@@ -2,15 +2,11 @@ module Card::Readable
extend ActiveSupport::Concern
def read_by(user)
notifications_for(user).tap do |notifications|
notifications.each(&:read)
end
user.notifications.find_by(card: self)&.read
end
def unread_by(user)
all_notifications_for(user).tap do |notifications|
notifications.each(&:unread)
end
user.notifications.find_by(card: self)&.unread
end
def remove_inaccessible_notifications
@@ -25,18 +21,6 @@ module Card::Readable
Card::RemoveInaccessibleNotificationsJob.perform_later(self)
end
def notifications_for(user)
scope = user.notifications.unread
scope.where(source: event_notification_sources)
.or(scope.where(source: mention_notification_sources))
end
def all_notifications_for(user)
scope = user.notifications
scope.where(source: event_notification_sources)
.or(scope.where(source: mention_notification_sources))
end
def event_notification_sources
events.or(comment_creation_events)
end
app/models/concerns/push_notifiable.rb
+1
View File
@@ -3,6 +3,7 @@ module PushNotifiable
included do
after_create_commit :push_notification_later
after_update_commit :push_notification_later, if: :source_id_previously_changed?
end
private
app/models/notification.rb
+28 -17
View File
@@ -5,32 +5,39 @@ class Notification < ApplicationRecord
belongs_to :user
belongs_to :creator, class_name: "User"
belongs_to :source, polymorphic: true
belongs_to :card
scope :unread, -> { where(read_at: nil) }
scope :read, -> { where.not(read_at: nil) }
scope :ordered, -> { order(read_at: :desc, created_at: :desc) }
scope :ordered, -> { order(read_at: :desc, updated_at: :desc) }
scope :preloaded, -> { preload(:card, :creator, :account, source: [ :board, :creator, { eventable: [ :closure, :board, :assignments ] } ]) }
after_create_commit :broadcast_unread
after_destroy_commit :broadcast_read
before_validation :set_card
after_create :bundle
after_update :bundle, if: :source_id_previously_changed?
scope :preloaded, -> { preload(:creator, :account, source: [ :board, :creator, { eventable: [ :closure, :board, :assignments ] } ]) }
after_create_commit -> { broadcast_prepend_later_to user, :notifications, target: "notifications" }
after_update_commit -> { broadcast_update }
after_destroy_commit -> { broadcast_remove_to user, :notifications }
delegate :notifiable_target, to: :source
delegate :card, to: :source
def self.read_all
all.each { |notification| notification.read }
class << self
def read_all
all.each(&:read)
end
def unread_all
all.each(&:unread)
end
end
def read
update!(read_at: Time.current)
broadcast_read
update!(read_at: Time.current, unread_count: 0)
end
def unread
update!(read_at: nil)
broadcast_unread
update!(read_at: nil, unread_count: 1)
end
def read?
@@ -38,15 +45,19 @@ class Notification < ApplicationRecord
end
private
def broadcast_unread
broadcast_prepend_later_to user, :notifications, target: "notifications"
end
def broadcast_read
broadcast_remove_to user, :notifications
def set_card
self.card = source.card
end
def bundle
user.bundle(self) if user.settings.bundling_emails?
end
def broadcast_update
if read?
broadcast_remove_to(user, :notifications)
else
broadcast_prepend_later_to(user, :notifications, target: "notifications")
end
end
end
app/models/notification/bundle.rb
+2 -2
View File
@@ -5,7 +5,7 @@ class Notification::Bundle < ApplicationRecord
enum :status, %i[ pending processing delivered ]
scope :due, -> { pending.where("ends_at <= ?", Time.current) }
scope :containing, ->(notification) { where("starts_at <= ? AND ends_at > ?", notification.created_at, notification.created_at) }
scope :containing, ->(notification) { where("starts_at <= ? AND ends_at > ?", notification.updated_at, notification.updated_at) }
scope :overlapping_with, ->(other_bundle) do
where(
"(starts_at <= ? AND ends_at >= ?) OR (starts_at <= ? AND ends_at >= ?) OR (starts_at >= ? AND ends_at <= ?)",
@@ -33,7 +33,7 @@ class Notification::Bundle < ApplicationRecord
end
def notifications
user.notifications.where(created_at: window).unread
user.notifications.where(updated_at: window).unread
end
def deliver
app/models/notifier.rb
+7 -1
View File
@@ -16,7 +16,13 @@ class Notifier
if should_notify?
# Processing recipients in order avoids deadlocks if notifications overlap.
recipients.sort_by(&:id).map do |recipient|
Notification.create! user: recipient, source: source, creator: creator
notification = Notification.find_or_initialize_by(user: recipient, card: source.card)
notification.source = source
notification.creator = creator
notification.read_at = nil
notification.unread_count = (notification.unread_count || 0) + 1
notification.save!
notification
end
end
end
app/models/user/notifiable.rb
+3 -3
View File
@@ -25,12 +25,12 @@ module User::Notifiable
def expand_pending_bundle_for(notification)
pending = notification_bundles.pending.last
if pending.present? && notification.created_at < pending.starts_at
pending.update!(starts_at: notification.created_at) # expand the window to include this notification
if pending.present? && notification.updated_at < pending.starts_at
pending.update!(starts_at: notification.updated_at) # expand the window to include this notification
end
end
def create_bundle_for(notification)
notification_bundles.create!(starts_at: notification.created_at)
notification_bundles.create!(starts_at: notification.updated_at)
end
end
app/views/cards/readings/create.turbo_stream.erb
-3
View File
@@ -1,3 +0,0 @@
<% @notifications.each do |notification| %>
<%= turbo_stream.remove notification %>
<% end %>
app/views/notifications/_notification.html.erb
+1 -1
View File
@@ -2,7 +2,7 @@
<%# Helper Dependency Updated: avatar_image_tag 2025-12-15 %>
<%= notification_tag notification do %>
<%= render "notifications/notification/header", notification: notification do %>
<%= notification_toggle_read_button(notification, url: card_reading_path(notification.card)) %>
<%= notification_toggle_read_button(notification, url: notification_reading_path(notification)) %>
<% end %>
<%= render "notifications/notification/body", notification: notification %>
<% end %>
app/views/notifications/_notification.json.jbuilder
+1 -1
View File
@@ -1,5 +1,5 @@
json.cache! notification do
json.(notification, :id)
json.(notification, :id, :unread_count)
json.read notification.read?
json.read_at notification.read_at&.utc
json.created_at notification.created_at.utc
app/views/notifications/_tray.html.erb
+2 -3
View File
@@ -8,9 +8,8 @@
data-navigable-list-reverse-navigation-value="true"
data-action="keydown->navigable-list#navigate dialog:show@document->navigable-list#reset turbo-visit->navigable-list#reset keydown.esc->dialog#close:stop click@document->dialog#closeOnClickOutside">
<%= turbo_frame_tag "notifications", src: tray_notifications_path, refresh: "morph", data: {
controller: "notifications-tray frame-reloader",
notifications_tray_grouped_class: "notification--grouped",
action: "turbo:frame-render->notifications-tray#group focus@window->frame-reloader#reload" } %>
controller: "frame-reloader",
action: "focus@window->frame-reloader#reload" } %>
<div class="tray__item tray__item--hat txt-x-small gap-half">
<div data-navigable-list-target="item" class="full-width">
app/views/notifications/index.json.jbuilder
+1 -1
View File
@@ -1 +1 @@
json.array! (@unread || []) + @page.records, partial: "notifications/notification", as: :notification
json.array! (@unread || []) + @page.records, partial: "notifications/notification", as: :notification, cached: true
app/views/notifications/index/_notification.html.erb
-6
View File
@@ -1,6 +0,0 @@
<%= notification_tag notification do %>
<%= render "notifications/notification/header", notification: notification do %>
<%= notification_toggle_read_button(notification, url: notification_reading_path(notification)) %>
<% end %>
<%= render "notifications/notification/body", notification: notification %>
<% end %>
app/views/notifications/index/_read_notifications.html.erb
+1 -1
View File
@@ -3,7 +3,7 @@
<h2 class="txt-medium margin-block-start-double margin-block-end-half txt-uppercase translucent">Previously seen</h2>
<div id="notifications_list_read" contents>
<%= render partial: "notifications/index/notification", collection: page.records, cached: true %>
<%= render partial: "notifications/notification", collection: page.records, cached: true %>
</div>
</section>
<% end %>
app/views/notifications/index/_unread_notifications.html.erb
+2 -2
View File
@@ -11,12 +11,12 @@
<% end %>
<div id="notifications_list" contents>
<%= render partial: "notifications/index/notification", collection: unread, cached: true %>
<%= render partial: "notifications/notification", collection: unread, cached: true %>
</div>
<% if unread.any? %>
<% total_unread_count = Current.user.notifications.unread.count %>
<% if Current.user.notifications.unread.count > NotificationsController::MAX_UNREAD_NOTIFICATIONS %>
<% if total_unread_count > NotificationsController::MAX_UNREAD_NOTIFICATIONS %>
<div class="fill-highlight txt-x-small border-radius pad-block-half pad-inline">
Showing the <%= NotificationsController::MAX_UNREAD_NOTIFICATIONS %> most recent (<%= total_unread_count - NotificationsController::MAX_UNREAD_NOTIFICATIONS %> are hidden)
</div>
app/views/notifications/trays/show.html.erb
-1
View File
@@ -1,4 +1,3 @@
<%= turbo_frame_tag "notifications" do %>
<%= render partial: "notifications/notification", collection: @notifications, cached: true %>
<div hidden data-notifications-tray-target="hiddenNotifications"></div>
<% end %>
db/migrate/20260209165805_notifications_data_migration.rb
+84
View File
@@ -0,0 +1,84 @@
class NotificationsDataMigration < ActiveRecord::Migration[8.2]
BATCH_SIZE = 10_000
class Notification < ActiveRecord::Base
self.table_name = "notifications"
end
def change
reversible do |dir|
dir.up do
populate_card_id
collapse_duplicates
end
end
change_column_null :notifications, :card_id, false
add_index :notifications, [ :user_id, :card_id ], unique: true
end
private
def populate_card_id
execute(<<~SQL)
UPDATE notifications
SET card_id = (
SELECT CASE events.eventable_type
WHEN 'Card' THEN events.eventable_id
WHEN 'Comment' THEN (SELECT comments.card_id FROM comments WHERE comments.id = events.eventable_id)
END
FROM events
WHERE events.id = notifications.source_id
)
WHERE notifications.card_id IS NULL
AND notifications.source_type = 'Event'
SQL
execute(<<~SQL)
UPDATE notifications
SET card_id = (
SELECT CASE mentions.source_type
WHEN 'Card' THEN mentions.source_id
WHEN 'Comment' THEN (SELECT comments.card_id FROM comments WHERE comments.id = mentions.source_id)
END
FROM mentions
WHERE mentions.id = notifications.source_id
)
WHERE notifications.card_id IS NULL
AND notifications.source_type = 'Mention'
SQL
end
def collapse_duplicates
loop do
duplicates = Notification.find_by_sql(<<~SQL)
SELECT user_id, card_id,
MAX(id) AS keep_id,
COUNT(*) AS total,
SUM(CASE WHEN read_at IS NULL THEN 1 ELSE 0 END) AS unread_total
FROM notifications
WHERE card_id IS NOT NULL
GROUP BY user_id, card_id
HAVING COUNT(*) > 1
LIMIT #{BATCH_SIZE}
SQL
break if duplicates.empty?
duplicates.each do |row|
Notification.where(user_id: row.user_id, card_id: row.card_id)
.where.not(id: row.keep_id)
.delete_all
Notification.where(id: row.keep_id)
.update_all(unread_count: row.unread_total.to_i)
end
end
# Set unread_count for remaining non-collapsed notifications
execute(<<~SQL)
UPDATE notifications
SET unread_count = CASE WHEN read_at IS NULL THEN 1 ELSE 0 END
WHERE unread_count = 0 AND card_id IS NOT NULL
SQL
end
end
db/schema.rb
Generated
+2 -1
View File
@@ -385,7 +385,7 @@ ActiveRecord::Schema[8.2].define(version: 2026_02_11_122517) do
create_table "notifications", id: :uuid, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t|
t.uuid "account_id", null: false
t.uuid "card_id"
t.uuid "card_id", null: false
t.datetime "created_at", null: false
t.uuid "creator_id"
t.datetime "read_at"
@@ -397,6 +397,7 @@ ActiveRecord::Schema[8.2].define(version: 2026_02_11_122517) do
t.index ["account_id"], name: "index_notifications_on_account_id"
t.index ["creator_id"], name: "index_notifications_on_creator_id"
t.index ["source_type", "source_id"], name: "index_notifications_on_source"
t.index ["user_id", "card_id"], name: "index_notifications_on_user_id_and_card_id", unique: true
t.index ["user_id", "read_at", "created_at"], name: "index_notifications_on_user_id_and_read_at_and_created_at", order: { read_at: :desc, created_at: :desc }
t.index ["user_id"], name: "index_notifications_on_user_id"
end
db/schema_sqlite.rb
+2 -1
View File
@@ -385,7 +385,7 @@ ActiveRecord::Schema[8.2].define(version: 2026_02_11_122517) do
create_table "notifications", id: :uuid, force: :cascade do |t|
t.uuid "account_id", null: false
t.uuid "card_id"
t.uuid "card_id", null: false
t.datetime "created_at", null: false
t.uuid "creator_id"
t.datetime "read_at"
@@ -397,6 +397,7 @@ ActiveRecord::Schema[8.2].define(version: 2026_02_11_122517) do
t.index ["account_id"], name: "index_notifications_on_account_id"
t.index ["creator_id"], name: "index_notifications_on_creator_id"
t.index ["source_type", "source_id"], name: "index_notifications_on_source"
t.index ["user_id", "card_id"], name: "index_notifications_on_user_id_and_card_id", unique: true
t.index ["user_id", "read_at", "created_at"], name: "index_notifications_on_user_id_and_read_at_and_created_at", order: { read_at: :desc, created_at: :desc }
t.index ["user_id"], name: "index_notifications_on_user_id"
end
test/controllers/cards/readings_controller_test.rb
+4 -28
View File
@@ -8,7 +8,7 @@ class Cards::ReadingsControllerTest < ActionDispatch::IntegrationTest
test "create" do
freeze_time
assert_changes -> { notifications(:logo_published_kevin).reload.read? }, from: false, to: true do
assert_changes -> { notifications(:logo_assignment_kevin).reload.read? }, from: false, to: true do
assert_changes -> { accesses(:writebook_kevin).reload.accessed_at }, from: nil, to: Time.current do
post card_reading_url(cards(:logo)), as: :turbo_stream
end
@@ -17,20 +17,10 @@ class Cards::ReadingsControllerTest < ActionDispatch::IntegrationTest
assert_response :success
end
test "read one notification on card visit" do
assert_changes -> { notifications(:logo_published_kevin).reload.read? }, from: false, to: true do
post card_reading_path(cards(:logo)), as: :turbo_stream
end
assert_response :success
end
test "read multiple notifications on card visit" do
assert_changes -> { notifications(:logo_published_kevin).reload.read? }, from: false, to: true do
test "read notification on card visit" do
assert_changes -> { notifications(:logo_assignment_kevin).reload.read? }, from: false, to: true do
post card_reading_path(cards(:logo)), as: :turbo_stream
end
end
assert_response :success
end
@@ -38,10 +28,9 @@ class Cards::ReadingsControllerTest < ActionDispatch::IntegrationTest
test "destroy" do
freeze_time
notifications(:logo_published_kevin).read
notifications(:logo_assignment_kevin).read
assert_changes -> { notifications(:logo_published_kevin).reload.read? }, from: true, to: false do
assert_changes -> { notifications(:logo_assignment_kevin).reload.read? }, from: true, to: false do
assert_changes -> { accesses(:writebook_kevin).reload.accessed_at }, to: Time.current do
delete card_reading_url(cards(:logo)), as: :turbo_stream
end
@@ -50,25 +39,12 @@ class Cards::ReadingsControllerTest < ActionDispatch::IntegrationTest
assert_response :success
end
test "unread one notification on destroy" do
notifications(:logo_published_kevin).read
assert_changes -> { notifications(:logo_published_kevin).reload.read? }, from: true, to: false do
delete card_reading_path(cards(:logo)), as: :turbo_stream
end
assert_response :success
end
test "unread multiple notifications on destroy" do
notifications(:logo_published_kevin).read
test "unread notification on destroy" do
notifications(:logo_assignment_kevin).read
assert_changes -> { notifications(:logo_published_kevin).reload.read? }, from: true, to: false do
assert_changes -> { notifications(:logo_assignment_kevin).reload.read? }, from: true, to: false do
delete card_reading_path(cards(:logo)), as: :turbo_stream
end
end
assert_response :success
end
test/controllers/notifications/bulk_readings_controller_test.rb
+2 -2
View File
@@ -6,7 +6,7 @@ class Notifications::BulkReadingsControllerTest < ActionDispatch::IntegrationTes
end
test "create marks all notifications as read" do
assert_changes -> { notifications(:logo_published_kevin).reload.read? }, from: false, to: true do
assert_changes -> { notifications(:logo_assignment_kevin).reload.read? }, from: false, to: true do
assert_changes -> { notifications(:layout_commented_kevin).reload.read? }, from: false, to: true do
post bulk_reading_path
end
@@ -24,7 +24,7 @@ class Notifications::BulkReadingsControllerTest < ActionDispatch::IntegrationTes
end
test "create as JSON" do
assert_changes -> { notifications(:logo_published_kevin).reload.read? }, from: false, to: true do
assert_changes -> { notifications(:logo_assignment_kevin).reload.read? }, from: false, to: true do
assert_changes -> { notifications(:layout_commented_kevin).reload.read? }, from: false, to: true do
post bulk_reading_path, as: :json
end
test/controllers/notifications/readings_controller_test.rb
+11 -12
View File
@@ -3,38 +3,37 @@ require "test_helper"
class Notifications::ReadingsControllerTest < ActionDispatch::IntegrationTest
setup do
sign_in_as :kevin
@notification = notifications(:logo_assignment_kevin)
end
test "create" do
assert_changes -> { notifications(:logo_published_kevin).reload.read? }, from: false, to: true do
post notification_reading_path(notifications(:logo_published_kevin), format: :turbo_stream)
assert_changes -> { @notification.reload.read? }, from: false, to: true do
post notification_reading_path(@notification, format: :turbo_stream)
assert_response :success
end
end
test "destroy" do
notification = notifications(:logo_published_kevin)
notification.read # Mark as read first
@notification.read
assert_changes -> { notification.reload.read? }, from: true, to: false do
delete notification_reading_path(notification, format: :turbo_stream)
assert_changes -> { @notification.reload.read? }, from: true, to: false do
delete notification_reading_path(@notification, format: :turbo_stream)
assert_response :success
end
end
test "create as JSON" do
assert_changes -> { notifications(:logo_published_kevin).reload.read? }, from: false, to: true do
post notification_reading_path(notifications(:logo_published_kevin)), as: :json
assert_changes -> { @notification.reload.read? }, from: false, to: true do
post notification_reading_path(@notification), as: :json
assert_response :no_content
end
end
test "destroy as JSON" do
notification = notifications(:logo_published_kevin)
notification.read
@notification.read
assert_changes -> { notification.reload.read? }, from: true, to: false do
delete notification_reading_path(notification), as: :json
assert_changes -> { @notification.reload.read? }, from: true, to: false do
delete notification_reading_path(@notification), as: :json
assert_response :no_content
end
end
test/controllers/notifications_controller_test.rb
+3 -4
View File
@@ -10,18 +10,17 @@ class NotificationsControllerTest < ActionDispatch::IntegrationTest
assert_response :success
assert_kind_of Array, @response.parsed_body
assert @response.parsed_body.any? { |n| n["id"] == notifications(:logo_published_kevin).id }
assert @response.parsed_body.any? { |n| n["id"] == notifications(:logo_assignment_kevin).id }
end
test "index as JSON includes notification attributes" do
get notifications_path, as: :json
notification = @response.parsed_body.find { |n| n["id"] == notifications(:logo_published_kevin).id }
notification = @response.parsed_body.find { |n| n["id"] == notifications(:logo_assignment_kevin).id }
assert_not_nil notification["title"]
assert_not_nil notification["body"]
assert_not_nil notification["created_at"]
assert_not_nil notification["card"]
assert_not_nil notification["creator"]
assert_not_nil notification["unread_count"]
end
end
test/fixtures/notifications.yml
Vendored
+8 -18
View File
@@ -1,15 +1,9 @@
logo_published_kevin:
id: <%= ActiveRecord::FixtureSet.identify("logo_published_kevin", :uuid) %>
user: kevin_uuid
source: logo_published_uuid (Event)
created_at: <%= 1.week.ago %>
creator: david_uuid
account: 37s_uuid
logo_assignment_kevin:
id: <%= ActiveRecord::FixtureSet.identify("logo_assignment_kevin", :uuid) %>
user: kevin_uuid
source: logo_assignment_km_uuid (Event)
card: logo_uuid
unread_count: 2
created_at: <%= 1.week.ago %>
creator: david_uuid
account: 37s_uuid
@@ -18,22 +12,18 @@ layout_commented_kevin:
id: <%= ActiveRecord::FixtureSet.identify("layout_commented_kevin", :uuid) %>
user: kevin_uuid
source: layout_commented_uuid (Event)
card: layout_uuid
unread_count: 1
created_at: <%= 1.week.ago %>
creator: david_uuid
account: 37s_uuid
logo_card_david_mention_by_jz:
id: <%= ActiveRecord::FixtureSet.identify("logo_card_david_mention_by_jz_notif", :uuid) %>
user: david_uuid
source: logo_card_david_mention_by_jz_uuid (Mention)
created_at: <%= 1.week.ago %>
creator: david_uuid
account: 37s_uuid
logo_comment_david_mention_by_jz:
id: <%= ActiveRecord::FixtureSet.identify("logo_comment_david_mention_by_jz_notif", :uuid) %>
logo_mentioned_david:
id: <%= ActiveRecord::FixtureSet.identify("logo_mentioned_david", :uuid) %>
user: david_uuid
source: logo_comment_david_mention_by_jz_uuid (Mention)
card: logo_uuid
unread_count: 2
created_at: <%= 1.week.ago %>
creator: david_uuid
account: 37s_uuid
test/mailers/notification/bundle_mailer_test.rb
+1
View File
@@ -3,6 +3,7 @@ require "test_helper"
class Notification::BundleMailerTest < ActionMailer::TestCase
setup do
@user = users(:david)
@user.notifications.destroy_all
@bundle = Notification::Bundle.create!(
user: @user,
test/models/card/readable_test.rb
+15 -38
View File
@@ -1,60 +1,41 @@
require "test_helper"
class Card::ReadableTest < ActiveSupport::TestCase
test "read clears events notifications" do
assert_changes -> { notifications(:logo_published_kevin).reload.read? }, from: false, to: true do
test "read marks notification as read" do
assert_changes -> { notifications(:logo_assignment_kevin).reload.read? }, from: false, to: true do
cards(:logo).read_by(users(:kevin))
end
end
end
test "read clear mentions in the description" do
assert_changes -> { notifications(:logo_card_david_mention_by_jz).reload.read? }, from: false, to: true do
test "read marks mention notification as read" do
assert_changes -> { notifications(:logo_mentioned_david).reload.read? }, from: false, to: true do
cards(:logo).read_by(users(:david))
end
end
test "read clear mentions in comments" do
assert_changes -> { notifications(:logo_comment_david_mention_by_jz).reload.read? }, from: false, to: true do
cards(:logo).read_by(users(:david))
end
end
test "read clears notifications from the comments" do
test "read marks comment notification as read" do
assert_changes -> { notifications(:layout_commented_kevin).reload.read? }, from: false, to: true do
cards(:layout).read_by(users(:kevin))
end
end
test "unread marks events notifications as unread" do
notifications(:logo_published_kevin).read
test "unread marks notification as unread" do
notifications(:logo_assignment_kevin).read
assert_changes -> { notifications(:logo_published_kevin).reload.read? }, from: true, to: false do
assert_changes -> { notifications(:logo_assignment_kevin).reload.read? }, from: true, to: false do
cards(:logo).unread_by(users(:kevin))
end
end
end
test "unread marks mentions in the description as unread" do
notifications(:logo_card_david_mention_by_jz).read
test "unread marks mention notification as unread" do
notifications(:logo_mentioned_david).read
assert_changes -> { notifications(:logo_card_david_mention_by_jz).reload.read? }, from: true, to: false do
assert_changes -> { notifications(:logo_mentioned_david).reload.read? }, from: true, to: false do
cards(:logo).unread_by(users(:david))
end
end
test "unread marks mentions in comments as unread" do
notifications(:logo_comment_david_mention_by_jz).read
assert_changes -> { notifications(:logo_comment_david_mention_by_jz).reload.read? }, from: true, to: false do
cards(:logo).unread_by(users(:david))
end
end
test "unread marks notifications from the comments as unread" do
test "unread marks comment notification as unread" do
notifications(:layout_commented_kevin).read
assert_changes -> { notifications(:layout_commented_kevin).reload.read? }, from: true, to: false do
@@ -68,8 +49,8 @@ class Card::ReadableTest < ActiveSupport::TestCase
david = users(:david)
assert card.accessible_to?(kevin)
kevin_notifications = [ notifications(:logo_published_kevin), notifications(:logo_assignment_kevin) ]
david_notifications = [ notifications(:logo_card_david_mention_by_jz), notifications(:logo_comment_david_mention_by_jz) ]
kevin_notification = notifications(:logo_assignment_kevin)
david_notification = notifications(:logo_mentioned_david)
# Kevin loses access
card.board.accesses.find_by(user: kevin).destroy
@@ -78,14 +59,10 @@ class Card::ReadableTest < ActiveSupport::TestCase
card.remove_inaccessible_notifications
# Kevin's notifications removed
kevin_notifications.each do |notification|
assert_not Notification.exists?(notification.id)
end
# Kevin's notification removed
assert_not Notification.exists?(kevin_notification.id)
# David's notifications preserved
david_notifications.each do |notification|
assert Notification.exists?(notification.id)
end
# David's notification preserved
assert Notification.exists?(david_notification.id)
end
end
test/models/notification/bundle_test.rb
+6 -5
View File
@@ -5,6 +5,7 @@ class Notification::BundleTest < ActiveSupport::TestCase
setup do
@user = users(:david)
@user.notifications.destroy_all
@user.settings.bundle_email_every_few_hours!
end
@@ -25,7 +26,7 @@ class Notification::BundleTest < ActiveSupport::TestCase
end
end
test "notifications are bundled withing the aggregation period" do
test "notifications are bundled within the aggregation period" do
@user.notification_bundles.destroy_all
notification_1 = assert_difference -> { @user.notification_bundles.pending.count }, 1 do
@@ -34,12 +35,12 @@ class Notification::BundleTest < ActiveSupport::TestCase
travel_to 3.hours.from_now
notification_2 = assert_no_difference -> { @user.notification_bundles.count } do
@user.notifications.create!(source: events(:logo_published), creator: @user)
@user.notifications.create!(source: events(:layout_published), creator: @user)
end
travel_to 3.days.from_now
notification_3 = assert_difference -> { @user.notification_bundles.pending.count }, 1 do
@user.notifications.create!(source: events(:logo_published), creator: @user)
@user.notifications.create!(source: events(:text_published), creator: @user)
end
assert_equal 2, @user.notification_bundles.count
@@ -150,10 +151,10 @@ class Notification::BundleTest < ActiveSupport::TestCase
test "out-of-order notification bundling should still work" do
first_notification = @user.notifications.create!(source: events(:logo_published), creator: @user)
second_notification = @user.notifications.create!(source: events(:logo_published), creator: @user)
second_notification = @user.notifications.create!(source: events(:layout_commented), creator: @user)
@user.notification_bundles.destroy_all
assert first_notification.created_at < second_notification.created_at
assert first_notification.updated_at <= second_notification.updated_at
@user.bundle(second_notification)
@user.bundle(first_notification)
test/models/notification_pusher_test.rb
+1 -4
View File
@@ -3,10 +3,7 @@ require "test_helper"
class NotificationPusherTest < ActiveSupport::TestCase
setup do
@user = users(:david)
@notification = @user.notifications.create!(
source: events(:logo_published),
creator: users(:jason)
)
@notification = notifications(:logo_mentioned_david)
@pusher = NotificationPusher.new(@notification)
@user.push_subscriptions.create!(
test/models/notification_test.rb
+48 -30
View File
@@ -1,49 +1,67 @@
require "test_helper"
class NotificationTest < ActiveSupport::TestCase
test "unread marks notification as unread" do
notification = notifications(:logo_published_kevin)
notification.read # Mark as read first
assert_changes -> { notification.reload.read? }, from: true, to: false do
notification.unread
end
end
test "unread broadcasts to notifications" do
notification = notifications(:logo_published_kevin)
notification.read # Mark as read first
assert_turbo_stream_broadcasts([ notification.user, :notifications ], count: 1) do
perform_enqueued_jobs do
notification.unread
end
end
end
test "read marks notification as read" do
notification = notifications(:logo_published_kevin)
# Ensure it starts as unread
notification.update!(read_at: nil)
notification = notifications(:logo_assignment_kevin)
notification.update!(read_at: nil, unread_count: 2)
assert_changes -> { notification.reload.read? }, from: false, to: true do
notification.read
end
assert_equal 0, notification.unread_count
end
test "read broadcasts to notifications" do
notification = notifications(:logo_published_kevin)
# Ensure it starts as unread
notification.update!(read_at: nil)
test "unread marks notification as unread" do
notification = notifications(:logo_assignment_kevin)
notification.read
assert_changes -> { notification.reload.read? }, from: true, to: false do
notification.unread
end
assert_equal 1, notification.unread_count
end
test "read_all marks all notifications and resets unread counts" do
kevin = users(:kevin)
kevin.notifications.unread.read_all
assert kevin.notifications.reload.all?(&:read?)
assert kevin.notifications.reload.all? { |n| n.unread_count == 0 }
end
test "unread_count tracks notification count per card" do
notification = notifications(:logo_assignment_kevin)
assert_equal 2, notification.unread_count
end
test "broadcasting on create prepends" do
kevin = users(:kevin)
layout = cards(:layout)
notifications(:layout_commented_kevin).destroy
perform_enqueued_jobs do
Notification.create!(user: kevin, source: events(:layout_commented), creator: users(:david))
end
assert_turbo_stream_broadcasts [ kevin, :notifications ]
end
test "broadcasting on update when read removes" do
notification = notifications(:layout_commented_kevin)
assert_turbo_stream_broadcasts([ notification.user, :notifications ], count: 1) do
perform_enqueued_jobs do
notification.read
end
end
end
test "deleting notification broadcasts its removal" do
notification = notifications(:logo_published_kevin)
notification.update!(read_at: nil)
test "broadcasting on destroy removes" do
notification = notifications(:logo_assignment_kevin)
assert_turbo_stream_broadcasts([ notification.user, :notifications ], count: 1) do
notification.destroy
test/models/notifier/event_notifier_test.rb
+7 -7
View File
@@ -71,18 +71,18 @@ class Notifier::EventNotifierTest < ActiveSupport::TestCase
test "create notifications on publish for mentionees" do
users(:kevin).mentioned_by(users(:david), at: cards(:logo))
assert_difference -> { users(:kevin).notifications.count }, +1 do
Notifier.for(events(:logo_published)).notify
end
notifications = Notifier.for(events(:logo_published)).notify
assert_includes notifications.map(&:user), users(:kevin)
end
test "don'create notifications on publish for mentionees that are not watching" do
test "create notifications on publish for mentionees that are not watching" do
users(:kevin).mentioned_by(users(:david), at: cards(:logo))
cards(:logo).unwatch_by(users(:kevin))
assert_difference -> { users(:kevin).notifications.count }, +1 do
Notifier.for(events(:logo_published)).notify
end
notifications = Notifier.for(events(:logo_published)).notify
assert_includes notifications.map(&:user), users(:kevin)
end
test "don't create notifications on comment for mentionees" do
test/models/user/notifiable_test.rb
+3 -2
View File
@@ -3,6 +3,7 @@ require "test_helper"
class User::NotifiableTest < ActiveSupport::TestCase
setup do
@user = users(:david)
@user.notifications.destroy_all
@user.settings.bundle_email_every_few_hours!
end
@@ -12,7 +13,7 @@ class User::NotifiableTest < ActiveSupport::TestCase
end
bundle = @user.notification_bundles.last
assert_equal notification.created_at, bundle.starts_at
assert_equal notification.updated_at, bundle.starts_at
assert bundle.pending?
end
@@ -20,7 +21,7 @@ class User::NotifiableTest < ActiveSupport::TestCase
@user.notifications.create!(source: events(:logo_published), creator: @user)
assert_no_difference -> { @user.notification_bundles.count } do
@user.notifications.create!(source: events(:logo_published), creator: @user)
@user.notifications.create!(source: events(:layout_published), creator: @user)
end
end
end
test/system/smoke_test.rb
+4 -4
View File
@@ -67,13 +67,13 @@ class SmokeTest < ApplicationSystemTestCase
test "dismissing notifications" do
sign_in_as(users(:david))
notif = notifications(:logo_card_david_mention_by_jz)
notification = notifications(:logo_mentioned_david)
assert_selector "div##{dom_id(notif)}"
assert_selector "div##{dom_id(notification)}"
within_window(open_new_window) { visit card_url(notif.card) }
within_window(open_new_window) { visit card_url(notification.card) }
assert_no_selector "div##{dom_id(notif)}"
assert_no_selector "div##{dom_id(notification)}"
end
test "dragging card to a new column" do