diff --git a/AGENTS.md b/AGENTS.md index a7cfc602cb..88b8b4e164 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -183,6 +183,7 @@ end - `ModelDeduper` — Deduplication logic - `RichTextMigrator` — Rich text migration utility - `DisplayImagePresenter` — Image display logic +- `RelatedComments` — Gathers all comments related to a commentable (type-aware: registrant/user/active orgs, etc.) for the comments overview page ### Event Registrations diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index b0c9cfa0bd..4f100b7e93 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -1,12 +1,19 @@ class CommentsController < ApplicationController before_action :set_commentable + before_action :require_commentable, only: :create def index authorize! - @comments = @commentable.comments.newest_first.paginate(page: params[:page], per_page: 10) respond_to do |format| - format.html { render partial: "comments/list", locals: { commentable: @commentable, comments: @comments } } + format.html do + if turbo_frame_request? && @commentable + @comments = @commentable.comments.newest_first.paginate(page: params[:page], per_page: 10) + render partial: "comments/list", locals: { commentable: @commentable, comments: @comments } + else + @comments = index_comments.paginate(page: params[:page], per_page: 10) + end + end end end @@ -33,20 +40,36 @@ def create private + # Scoped to a commentable: every comment related to that record (see + # RelatedComments). Top-level /comments: a preview of every comment in the + # system alongside the record each is about. + def index_comments + return all_comments unless @commentable + + RelatedComments.new(@commentable).comments + end + + def all_comments + Comment.includes(:commentable, created_by: :person, updated_by: :person).newest_first + end + def set_commentable - if params[:person_id] - @commentable = Person.find(params[:person_id]) - elsif params[:user_id] - @commentable = User.find(params[:user_id]) - elsif params[:organization_id] - @commentable = Organization.find(params[:organization_id]) - elsif params[:event_registration_id] - @commentable = EventRegistration.find(params[:event_registration_id]) - elsif params[:workshop_id] - @commentable = Workshop.find(params[:workshop_id]) - else - redirect_to root_path, alert: "Invalid commentable resource" - end + @commentable = + if params[:person_id] + Person.find(params[:person_id]) + elsif params[:user_id] + User.find(params[:user_id]) + elsif params[:organization_id] + Organization.find(params[:organization_id]) + elsif params[:event_registration_id] + EventRegistration.find(params[:event_registration_id]) + elsif params[:workshop_id] + Workshop.find(params[:workshop_id]) + end + end + + def require_commentable + redirect_to root_path, alert: "Invalid commentable resource" unless @commentable end def comment_params diff --git a/app/helpers/comments_helper.rb b/app/helpers/comments_helper.rb new file mode 100644 index 0000000000..c90dc7e98d --- /dev/null +++ b/app/helpers/comments_helper.rb @@ -0,0 +1,39 @@ +module CommentsHelper + # One-line description of what a record's related-comments overview pulls in, + # mirroring the logic in RelatedComments. + def related_comments_summary(commentable) + case commentable + when EventRegistration then "Includes the registrant, their user account, and active organizations." + when Person then "Includes their user account and active organizations." + when Organization then "Includes people with an active affiliation." + when User then "Includes the linked person and their active organizations." + when Workshop then "Includes the workshop's creator and their organizations." + else "Comments on this record." + end + end + + # Maps a comment's polymorphic commentable to the label, name, link, and badge + # styling used in the "About" column of the full-page comments index. + def comment_subject(commentable) + case commentable + when EventRegistration + { type: "Registration", name: commentable.event&.title || "Registration", + path: edit_event_registration_path(commentable), badge: "bg-blue-100 text-blue-800" } + when Person + { type: "Person", name: commentable.full_name, + path: person_path(commentable), badge: "bg-green-100 text-green-800" } + when Organization + { type: "Organization", name: commentable.name, + path: organization_path(commentable), badge: "bg-purple-100 text-purple-800" } + when User + { type: "User account", name: commentable.person&.name || commentable.name.presence || commentable.email, + path: user_path(commentable), badge: "bg-amber-100 text-amber-800" } + when Workshop + { type: "Workshop", name: commentable.try(:title).presence || commentable.try(:name) || "Workshop", + path: edit_workshop_path(commentable), badge: "bg-rose-100 text-rose-800" } + else + { type: commentable.class.name, name: commentable.try(:name) || "##{commentable.id}", + path: nil, badge: "bg-gray-100 text-gray-800" } + end + end +end diff --git a/app/services/related_comments.rb b/app/services/related_comments.rb new file mode 100644 index 0000000000..4bde751e83 --- /dev/null +++ b/app/services/related_comments.rb @@ -0,0 +1,70 @@ +# Collects every comment "related" to a commentable, where the meaning of +# related depends on the commentable's type. The type is the signal of which +# comment box the user opened the overview from (each box links to its own +# polymorphic comments path), so this is the single place that decides what +# context to surface for each kind of record. +class RelatedComments + def initialize(commentable) + @commentable = commentable + end + + def comments + commentable_groups + .map { |type, ids| Comment.where(commentable_type: type, commentable_id: ids) } + .reduce(:or) + .includes(:commentable, created_by: :person, updated_by: :person) + .newest_first + end + + private + + def commentable_groups + ([ @commentable ] + related_records).compact.uniq + .group_by { |record| record.class.base_class.name } + .transform_values { |records| records.map(&:id).uniq } + end + + def related_records + case @commentable + when EventRegistration then registration_related + when Person then person_related + when Organization then organization_related + when User then user_related + when Workshop then workshop_related + else [] + end + end + + def registration_related + person = @commentable.registrant + return [] unless person + + [ person, person.user, *active_organizations(person) ] + end + + def person_related + [ @commentable.user, *active_organizations(@commentable) ] + end + + def organization_related + @commentable.people.merge(Affiliation.active).distinct.to_a + end + + def user_related + person = @commentable.person + return [] unless person + + [ person, *active_organizations(person) ] + end + + def workshop_related + creator = @commentable.created_by + return [] unless creator + + [ creator, creator.person, *(creator.person ? active_organizations(creator.person) : []) ] + end + + def active_organizations(person) + person.organizations.merge(Affiliation.active).distinct.to_a + end +end diff --git a/app/views/comments/_view_all_link.html.erb b/app/views/comments/_view_all_link.html.erb new file mode 100644 index 0000000000..ac6c98d658 --- /dev/null +++ b/app/views/comments/_view_all_link.html.erb @@ -0,0 +1,6 @@ +<%= link_to polymorphic_path([ commentable, :comments ]), + class: "inline-flex items-center gap-1.5 text-xs font-medium text-gray-500 hover:text-gray-700 hover:underline whitespace-nowrap", + target: "_blank", rel: "noopener" do %> + View all related comments + +<% end %> diff --git a/app/views/comments/index.html.erb b/app/views/comments/index.html.erb index 184223619f..d2fffaa214 100644 --- a/app/views/comments/index.html.erb +++ b/app/views/comments/index.html.erb @@ -1,2 +1,69 @@ <% content_for(:page_bg_class, "admin-only bg-blue-100") %> -<%= render "comments/list", commentable: @commentable, comments: @comments %> +
+
+ <% if @commentable %> + <% subject = comment_subject(@commentable) %> + <% if subject[:path] %> + <%= link_to "← Back", subject[:path], class: "text-sm text-gray-500 hover:text-gray-700 mb-2 inline-block" %> + <% end %> +

Related comments

+

+ <%= subject[:type] %> + <%= subject[:name] %> +

+

<%= related_comments_summary(@commentable) %>

+ <% else %> +

All comments

+

Every comment in the system, with the record each one is about.

+ <% end %> +
+ + <% if @comments.any? %> +
+ + + + + + + + + + + + <% @comments.each do |comment| %> + <% subject = comment_subject(comment.commentable) %> + <% author = comment.created_by&.person&.name || comment.created_by&.name %> + <% editor = comment.updated_by && comment.updated_by != comment.created_by ? (comment.updated_by.person&.name || comment.updated_by.name) : nil %> + + + + + + + <% end %> + +
DateAboutAuthorComment
<%= comment.created_at.strftime("%-m/%-d/%Y %-I:%M %p") %> + <%= subject[:type] %> +
+ <% if subject[:path] %> + <%= link_to subject[:name], subject[:path], class: "text-blue-600 hover:text-blue-800", data: { turbo_frame: "_top" } %> + <% else %> + <%= subject[:name] %> + <% end %> +
+ 		+ <%= author.presence || "—" %> + <% if editor %> + edited by <%= editor %> + <% end %> + <%= simple_format(comment.body, {}, sanitize: true) %>
+
+ + <% if @comments.total_pages > 1 %> + + <% end %> + <% else %> +

There are no comments to show yet.

+ <% end %> +
diff --git a/app/views/event_registrations/_form.html.erb b/app/views/event_registrations/_form.html.erb index f342f2867c..2c2b766f35 100644 --- a/app/views/event_registrations/_form.html.erb +++ b/app/views/event_registrations/_form.html.erb @@ -255,12 +255,9 @@

Registration comments

- <%= link_to event_registration_comments_path(f.object), - class: "ml-auto inline-flex items-center gap-1.5 text-xs font-medium text-gray-500 hover:text-gray-700 hover:underline", - target: "_blank", rel: "noopener" do %> - View all - - <% end %> +
+ <%= render "comments/view_all_link", commentable: f.object %> +
diff --git a/app/views/organizations/_form.html.erb b/app/views/organizations/_form.html.erb index 9fdc032c6a..23088796e7 100644 --- a/app/views/organizations/_form.html.erb +++ b/app/views/organizations/_form.html.erb @@ -380,7 +380,10 @@ <% if allowed_to?(:manage?, Comment) %>
-
Comments
+
+ Comments + <%= render "comments/view_all_link", commentable: f.object %> +
<%= f.simple_fields_for :comments do |cf| %> diff --git a/app/views/people/_form.html.erb b/app/views/people/_form.html.erb index 535a7f45f1..bcdf2a1c42 100644 --- a/app/views/people/_form.html.erb +++ b/app/views/people/_form.html.erb @@ -380,7 +380,10 @@ <% if allowed_to?(:manage?, Comment) %>
-
Comments
+
+ Comments + <%= render "comments/view_all_link", commentable: f.object %> +
-

Comments

+
+

Comments

+ <%= render "comments/view_all_link", commentable: f.object %> +
<% if allowed_to?(:manage?, Comment) %>
-
Comments
+
+ Comments + <%= render "comments/view_all_link", commentable: f.object %> +
"comments_list" } + + expect(response).to have_http_status(:success) + expect(response.body).to include("Registration note") + end + end + + describe "GET /comments" do + it "renders every comment in the system regardless of commentable" do + create(:comment, commentable: create(:person), body: "Person-wide note") + create(:comment, commentable: create(:organization), body: "Org-wide note") + + get comments_path + + expect(response).to have_http_status(:success) + expect(response.body).to include("All comments") + expect(response.body).to include("Person-wide note") + expect(response.body).to include("Org-wide note") + end + + it "is restricted to admins" do + sign_out admin + sign_in create(:user) + + get comments_path + + expect(response).to redirect_to(root_path) + end + end +end diff --git a/spec/services/related_comments_spec.rb b/spec/services/related_comments_spec.rb new file mode 100644 index 0000000000..c791951fdc --- /dev/null +++ b/spec/services/related_comments_spec.rb @@ -0,0 +1,82 @@ +require "rails_helper" + +RSpec.describe RelatedComments do + def bodies_for(commentable) + described_class.new(commentable).comments.map(&:body) + end + + describe "EventRegistration" do + it "gathers the registration, registrant, their user, and active orgs" do + registrant = create(:person) + reg = create(:event_registration, registrant:) + org = create(:organization) + create(:affiliation, person: registrant, organization: org) + inactive_org = create(:organization) + create(:affiliation, person: registrant, organization: inactive_org, inactive: true) + + create(:comment, commentable: reg, body: "reg") + create(:comment, commentable: registrant, body: "person") + create(:comment, commentable: registrant.user, body: "user") + create(:comment, commentable: org, body: "org") + create(:comment, commentable: inactive_org, body: "inactive-org") + + expect(bodies_for(reg)).to contain_exactly("reg", "person", "user", "org") + end + end + + describe "Person" do + it "gathers the person, their user, and active orgs" do + person = create(:person) + org = create(:organization) + create(:affiliation, person:, organization: org) + + create(:comment, commentable: person, body: "person") + create(:comment, commentable: person.user, body: "user") + create(:comment, commentable: org, body: "org") + + expect(bodies_for(person)).to contain_exactly("person", "user", "org") + end + end + + describe "Organization" do + it "gathers the org and people with an active affiliation" do + org = create(:organization) + active_person = create(:person) + create(:affiliation, person: active_person, organization: org) + inactive_person = create(:person) + create(:affiliation, person: inactive_person, organization: org, inactive: true) + + create(:comment, commentable: org, body: "org") + create(:comment, commentable: active_person, body: "active") + create(:comment, commentable: inactive_person, body: "inactive") + + expect(bodies_for(org)).to contain_exactly("org", "active") + end + end + + describe "User" do + it "gathers the user, its person, and the person's active orgs" do + person = create(:person) + org = create(:organization) + create(:affiliation, person:, organization: org) + + create(:comment, commentable: person.user, body: "user") + create(:comment, commentable: person, body: "person") + create(:comment, commentable: org, body: "org") + + expect(bodies_for(person.user)).to contain_exactly("user", "person", "org") + end + end + + describe "Workshop" do + it "gathers the workshop and its creator" do + creator = create(:user) + workshop = create(:workshop, created_by: creator) + + create(:comment, commentable: workshop, body: "workshop") + create(:comment, commentable: creator, body: "creator") + + expect(bodies_for(workshop)).to include("workshop", "creator") + end + end +end