Fix: dismiss grouped notifications by card from the notifications tray

This also splits the templates into two: index and tray, since the URL now changes.

app/controllers/cards/readings_controller.rb

@@ -8,6 +8,11 @@ class Cards::ReadingsController < ApplicationController
     record_board_access
   end
 
+  def destroy
+    @notifications = @card.unread_by(Current.user)
+    record_board_access
+  end
+
   private
     def record_board_access
       @card.board.accessed_by(Current.user)

app/helpers/notifications_helper.rb

@@ -34,9 +34,9 @@ module NotificationsHelper
     end
   end
 
-  def notification_toggle_read_button(notification)
+  def notification_toggle_read_button(notification, url:)
     if notification.read?
-      button_to notification_reading_path(notification),
+      button_to url,
           method: :delete,
           class: "card__notification-unread-indicator btn btn--circle borderless",
           title: "Mark as unread",
@@ -45,7 +45,7 @@ module NotificationsHelper
         concat(icon_tag("unseen"))
       end
     else
-      button_to notification_reading_path(notification),
+      button_to url,
           class: "card__notification-unread-indicator btn btn--circle borderless",
           title: "Mark as read",
           data: { action: "form#submit:stop badge#update:stop", form_target: "submit" },

app/models/card/readable.rb

@@ -7,9 +7,15 @@ module Card::Readable
     end
   end
 
+  def unread_by(user)
+    all_notifications_for(user).tap do |notifications|
+      notifications.each(&:unread)
+    end
+  end
+
   def remove_inaccessible_notifications
     User.find_each do |user|
-      notifications_for(user).destroy_all unless accessible_to?(user)
+      all_notifications_for(user).destroy_all unless accessible_to?(user)
     end
   end
 
@@ -24,6 +30,12 @@ module Card::Readable
         .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/views/notifications/_notification.html.erb

@@ -1,18 +1,8 @@
 <% cache notification do %>
   <%= notification_tag notification do %>
-
-    <header class="card__header">
-      <%= render "notifications/notification/header", notification: notification %>
-    </header>
-
-    <div class="card__body">
-      <div class="avatar txt-x-small">
-        <%= avatar_image_tag notification.creator %>
-      </div>
-
-      <h3 class="flex flex-column min-width flex-item-grow font-weight-normal">
-        <%= render "notifications/notification/#{notification.source_type.underscore}/body", notification: notification %>
-      </h3>
-    </div>
+    <%= render "notifications/notification/header", notification: notification do %>
+      <%= notification_toggle_read_button(notification, url: card_reading_path(notification.card)) %>
+    <% end %>
+    <%= render "notifications/notification/body", notification: notification %>
   <% end %>
 <% end %>

app/views/notifications/index.html.erb

@@ -26,7 +26,7 @@
     <% end %>
 
     <div id="notifications_list" contents>
-      <%= render partial: "notifications/notification", collection: @unread, cached: true %>
+      <%= render partial: "notifications/index/notification", collection: @unread, cached: true %>
     </div>
   </section>
 
@@ -34,7 +34,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/notification", collection: @page.records, cached: true %>
+      <%= render partial: "notifications/index/notification", collection: @page.records, cached: true %>
     </div>
   </section>
 </div>

app/views/notifications/index/_notification.html.erb

@@ -0,0 +1,6 @@
+<%= 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/notification/_body.html.erb

@@ -0,0 +1,9 @@
+<div class="card__body">
+  <div class="avatar txt-x-small">
+    <%= avatar_image_tag notification.creator %>
+  </div>
+
+  <h3 class="flex flex-column min-width flex-item-grow font-weight-normal">
+    <%= render "notifications/notification/#{notification.source_type.underscore}/body", notification: notification %>
+  </h3>
+</div>

app/views/notifications/notification/_header.html.erb

@@ -1,21 +1,23 @@
-<div class="card__board">
+<header class="card__header">
+  <div class="card__board">
   <span class="card__id">
     <span class="for-screen-reader">Card number</span>
     <%= notification.card.id %>
   </span>
-  <span class="card__board-name">
+    <span class="card__board-name">
     <span class="overflow-ellipsis"><%= notification.card.board.name %></span>
   </span>
-</div>
+  </div>
 
-<div class="card__notification-meta overflow-ellipsis flex-item-grow flex-item-no-shrink txt-align-end">
-  <span class="card__creator"><%= notification.creator.familiar_name %></span>
-</div>
+  <div class="card__notification-meta overflow-ellipsis flex-item-grow flex-item-no-shrink txt-align-end">
+    <span class="card__creator"><%= notification.creator.familiar_name %></span>
+  </div>
 
-<div class="card__notification-meta overflow-ellipsis flex-item-no-shrink">
-  <span class="card__timestamp"><%= local_datetime_tag(notification.created_at, style: :timeordate) %></span>
-</div>
+  <div class="card__notification-meta overflow-ellipsis flex-item-no-shrink">
+    <span class="card__timestamp"><%= local_datetime_tag(notification.created_at, style: :timeordate) %></span>
+  </div>
 
-<div class="card__notification-meta flex-item-no-shrink">
-  <%= notification_toggle_read_button(notification) %>
-</div>
+  <div class="card__notification-meta flex-item-no-shrink">
+    <%= yield %>
+  </div>
+</header>

config/routes.rb

@@ -77,6 +77,7 @@ Rails.application.routes.draw do
       resource :reading
       resource :triage
       resource :watch
+      resource :reading
 
       resources :assignments
       resources :steps

test/controllers/cards/readings_controller_test.rb

@@ -40,4 +40,42 @@ class Cards::ReadingsControllerTest < ActionDispatch::IntegrationTest
 
     assert_equal "false", response.headers["X-Writer-Affinity"]
   end
+
+  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 -> { accesses(:writebook_kevin).reload.accessed_at }, to: Time.current do
+        delete card_reading_url(cards(:logo)), as: :turbo_stream
+      end
+    end
+
+    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
+    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
 end

test/models/card/readable_test.rb

@@ -27,6 +27,41 @@ class Card::ReadableTest < ActiveSupport::TestCase
     end
   end
 
+  test "unread marks events notifications as unread" do
+    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
+        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
+
+    assert_changes -> { notifications(:logo_card_david_mention_by_jz).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
+    notifications(:layout_commented_kevin).read
+
+    assert_changes -> { notifications(:layout_commented_kevin).reload.read? }, from: true, to: false do
+      cards(:layout).unread_by(users(:kevin))
+    end
+  end
+
   test "remove inaccessible notifications" do
     card = cards(:logo)
     kevin = users(:kevin)