From d6f1caf8d5fa1b03bf98bf427253cadcf4fec57a Mon Sep 17 00:00:00 2001 From: maebeale Date: Sun, 5 Jul 2026 00:09:34 -0400 Subject: [PATCH 01/15] Make logged communications editable inline, like comments Manual-log communications on the person, registration, and scholarship edit pages were add-only: once saved they became read-only, so a typo or wrong channel couldn't be corrected without touching the console. This gives them the same inline view/edit toggle the comments box already has, so admins can fix or remove a logged entry in place and save it with the parent form. Automated notifications stay read-only. Rather than add a near-duplicate Stimulus controller, the comment edit-toggle controller is generalized into a reusable `edit-toggle` that takes the view/edit CSS classes as values (with the comment body-sync as an optional, class-driven feature), and both boxes now share it. Co-Authored-By: Claude Opus 4.8 (1M context) --- AGENTS.md | 2 +- .../event_registrations_controller.rb | 2 +- app/controllers/people_controller.rb | 2 +- app/controllers/scholarships_controller.rb | 2 +- .../comment_edit_toggle_controller.js | 44 ------------- .../controllers/edit_toggle_controller.js | 64 ++++++++++++++++++ app/frontend/javascript/controllers/index.js | 4 +- app/models/event_registration.rb | 2 +- app/models/person.rb | 2 +- app/models/scholarship.rb | 2 +- app/views/event_registrations/_form.html.erb | 8 +-- .../_notifications.html.erb | 64 ++++++++++-------- .../_notification_fields.html.erb | 51 +++++++-------- .../notifications/_notification_form.html.erb | 26 ++++++++ .../notifications/_notification_row.html.erb | 15 +++++ app/views/organizations/_form.html.erb | 8 +-- app/views/people/_form.html.erb | 8 +-- app/views/people/_notifications.html.erb | 65 +++++++++++-------- app/views/scholarships/_form.html.erb | 8 +-- .../scholarships/_notifications.html.erb | 65 ++++++++++++------- app/views/users/_form.html.erb | 8 +-- app/views/workshops/_form.html.erb | 8 +-- config/locales/en.yml | 3 + spec/requests/event_registrations_spec.rb | 17 +++++ spec/requests/people_notifications_spec.rb | 30 +++++++++ spec/requests/scholarships_spec.rb | 12 ++++ 26 files changed, 340 insertions(+), 182 deletions(-) delete mode 100644 app/frontend/javascript/controllers/comment_edit_toggle_controller.js create mode 100644 app/frontend/javascript/controllers/edit_toggle_controller.js create mode 100644 app/views/notifications/_notification_form.html.erb create mode 100644 app/views/notifications/_notification_row.html.erb diff --git a/AGENTS.md b/AGENTS.md index b95413ced7..ad089c6f0a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -285,12 +285,12 @@ end - `cocoon` — Nested form handling (cocoon gem) - `collection` — Filter form auto-submit with debounce - `column_toggle` — Toggle table column visibility -- `comment_edit_toggle` — Inline comment editing mode - `comment_required` — Require a comment body once a topic or body is filled in - `confirm_email` — Email confirmation UI - `dirty_form` — Unsaved changes detection - `dismiss` — Dismissable elements - `dropdown` — Dropdown menus with keyboard/click-outside handling +- `edit_toggle` — Inline view/edit toggle for the comments and communications boxes (configurable view/edit CSS classes) - `event_staff_bio` — Loads a selected person's read-only profile bio (with edit link) alongside the editable event-specific bio on the staff form - `file_preview` — File upload preview - `grant_details` — Swaps a grant's eligibility criteria + tasks when the grant picker changes diff --git a/app/controllers/event_registrations_controller.rb b/app/controllers/event_registrations_controller.rb index 692566995e..8ca1063862 100644 --- a/app/controllers/event_registrations_controller.rb +++ b/app/controllers/event_registrations_controller.rb @@ -334,7 +334,7 @@ def event_registration_params organization_ids: [], registrant_attributes: [ :id, :shoutout_text ], comments_attributes: [ :id, :topic, :body, :flagged, :_destroy ], - notifications_attributes: [ :channel, :sender_id, :email_subject, :email_body_text, :noticeable_type, :noticeable_id ] + notifications_attributes: [ :id, :channel, :sender_id, :email_subject, :email_body_text, :noticeable_type, :noticeable_id, :_destroy ] ) end diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index c7d058625f..1c51370125 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -558,7 +558,7 @@ def person_params :_destroy ], comments_attributes: [ :id, :topic, :body, :flagged, :_destroy ], - notifications_attributes: [ :channel, :sender_id, :email_subject, :email_body_text, :noticeable_type, :noticeable_id ] + notifications_attributes: [ :id, :channel, :sender_id, :email_subject, :email_body_text, :noticeable_type, :noticeable_id, :_destroy ] ) end end diff --git a/app/controllers/scholarships_controller.rb b/app/controllers/scholarships_controller.rb index dd52dd14bd..ebf71dc18a 100644 --- a/app/controllers/scholarships_controller.rb +++ b/app/controllers/scholarships_controller.rb @@ -209,7 +209,7 @@ def scholarship_params params.require(:scholarship).permit( :amount_dollars, :amount_cents, :tasks_completed, :grant_id, :recipient_id, comments_attributes: [ :id, :topic, :body, :flagged, :_destroy ], - notifications_attributes: [ :channel, :sender_id, :email_subject, :email_body_text, :noticeable_type, :noticeable_id ] + notifications_attributes: [ :id, :channel, :sender_id, :email_subject, :email_body_text, :noticeable_type, :noticeable_id, :_destroy ] ) end diff --git a/app/frontend/javascript/controllers/comment_edit_toggle_controller.js b/app/frontend/javascript/controllers/comment_edit_toggle_controller.js deleted file mode 100644 index 108aa8abcd..0000000000 --- a/app/frontend/javascript/controllers/comment_edit_toggle_controller.js +++ /dev/null @@ -1,44 +0,0 @@ -import { Controller } from "@hotwired/stimulus"; - -// Connects to data-controller="comment-edit-toggle" -// -// Toggles between view and edit modes for inline comment editing. -// When exiting edit mode, syncs textarea values back to the truncated display. -// -export default class extends Controller { - static targets = ["editLabel", "viewLabel"]; - - connect() { - this.editing = false; - } - - toggle() { - this.editing = !this.editing; - - // When leaving edit mode, sync textarea values to view display - if (!this.editing) { - this.element.querySelectorAll(".nested-fields").forEach((item) => { - const textarea = item.querySelector(".comment-edit textarea"); - const viewBody = item.querySelector(".comment-view .comment-body"); - if (textarea && viewBody) { - const text = textarea.value; - viewBody.textContent = - text.length > 135 ? text.substring(0, 132) + "..." : text; - viewBody.title = text; - } - }); - } - - this.element - .querySelectorAll(".comment-view") - .forEach((el) => (el.style.display = this.editing ? "none" : "")); - this.element - .querySelectorAll(".comment-edit") - .forEach((el) => (el.style.display = this.editing ? "" : "none")); - - if (this.hasEditLabelTarget && this.hasViewLabelTarget) { - this.editLabelTarget.style.display = this.editing ? "none" : ""; - this.viewLabelTarget.style.display = this.editing ? "" : "none"; - } - } -} diff --git a/app/frontend/javascript/controllers/edit_toggle_controller.js b/app/frontend/javascript/controllers/edit_toggle_controller.js new file mode 100644 index 0000000000..9fbe754fae --- /dev/null +++ b/app/frontend/javascript/controllers/edit_toggle_controller.js @@ -0,0 +1,64 @@ +import { Controller } from "@hotwired/stimulus"; + +// Connects to data-controller="edit-toggle" +// +// Toggles a list of records between read-only view blocks and inline edit forms +// (used by the comments and communications boxes on the Person / Registration / +// Scholarship edit forms). Each persisted record renders a `.{view}` block and a +// hidden `.{edit}` block; clicking the toggle flips which is shown and swaps the +// button label. +// +// Configure the block classes per caller: +// data-edit-toggle-view-class-value="comment-view" +// data-edit-toggle-edit-class-value="comment-edit" +// data-edit-toggle-body-class-value="comment-body" (optional) +// +// When a body class is given, leaving edit mode syncs each edit textarea back +// into the matching truncated `.{body}` span so the view reflects unsaved edits. +// +export default class extends Controller { + static targets = ["editLabel", "viewLabel"]; + static values = { + viewClass: { type: String, default: "editable-view" }, + editClass: { type: String, default: "editable-edit" }, + bodyClass: { type: String, default: "" }, + truncate: { type: Number, default: 135 } + }; + + connect() { + this.editing = false; + } + + toggle() { + this.editing = !this.editing; + + // When leaving edit mode, sync edit textareas into their truncated view. + if (!this.editing && this.bodyClassValue) { + this.element.querySelectorAll(".nested-fields").forEach((item) => { + const textarea = item.querySelector(`.${this.editClassValue} textarea`); + const viewBody = item.querySelector( + `.${this.viewClassValue} .${this.bodyClassValue}` + ); + if (textarea && viewBody) { + const text = textarea.value; + const max = this.truncateValue; + viewBody.textContent = + text.length > max ? text.substring(0, max - 3) + "..." : text; + viewBody.title = text; + } + }); + } + + this.element + .querySelectorAll(`.${this.viewClassValue}`) + .forEach((el) => (el.style.display = this.editing ? "none" : "")); + this.element + .querySelectorAll(`.${this.editClassValue}`) + .forEach((el) => (el.style.display = this.editing ? "" : "none")); + + if (this.hasEditLabelTarget && this.hasViewLabelTarget) { + this.editLabelTarget.style.display = this.editing ? "none" : ""; + this.viewLabelTarget.style.display = this.editing ? "" : "none"; + } + } +} diff --git a/app/frontend/javascript/controllers/index.js b/app/frontend/javascript/controllers/index.js index 13e2909844..834879a523 100644 --- a/app/frontend/javascript/controllers/index.js +++ b/app/frontend/javascript/controllers/index.js @@ -48,8 +48,8 @@ application.register("confirm-email", ConfirmEmailController) import ColumnToggleController from "./column_toggle_controller" application.register("column-toggle", ColumnToggleController) -import CommentEditToggleController from "./comment_edit_toggle_controller" -application.register("comment-edit-toggle", CommentEditToggleController) +import EditToggleController from "./edit_toggle_controller" +application.register("edit-toggle", EditToggleController) import CommentRequiredController from "./comment_required_controller" application.register("comment-required", CommentRequiredController) diff --git a/app/models/event_registration.rb b/app/models/event_registration.rb index d187d7165a..3a491eae5b 100644 --- a/app/models/event_registration.rb +++ b/app/models/event_registration.rb @@ -15,7 +15,7 @@ class EventRegistration < ApplicationRecord has_many :checklist_completions, class_name: "EventRegistrationChecklistCompletion", dependent: :destroy accepts_nested_attributes_for :comments, allow_destroy: true, reject_if: proc { |attrs| attrs["body"].blank? } - accepts_nested_attributes_for :notifications, reject_if: proc { |attrs| attrs["email_subject"].blank? } + accepts_nested_attributes_for :notifications, allow_destroy: true, reject_if: proc { |attrs| attrs["email_subject"].blank? } # Lets the registration edit form edit the registrant's shout-out text (which # lives on the Person) inline, alongside the registration's own shout-out flag. accepts_nested_attributes_for :registrant diff --git a/app/models/person.rb b/app/models/person.rb index 261d41482e..28719d5099 100644 --- a/app/models/person.rb +++ b/app/models/person.rb @@ -99,7 +99,7 @@ class Person < ApplicationRecord accepts_nested_attributes_for :affiliations, allow_destroy: true, reject_if: proc { |attrs| attrs["organization_id"].blank? } accepts_nested_attributes_for :comments, allow_destroy: true, reject_if: proc { |attrs| attrs["body"].blank? } - accepts_nested_attributes_for :notifications, reject_if: proc { |attrs| attrs["email_subject"].blank? } + accepts_nested_attributes_for :notifications, allow_destroy: true, reject_if: proc { |attrs| attrs["email_subject"].blank? } # Search Cop include SearchCop diff --git a/app/models/scholarship.rb b/app/models/scholarship.rb index 91f7d4e6b8..f40d3b89a3 100644 --- a/app/models/scholarship.rb +++ b/app/models/scholarship.rb @@ -6,7 +6,7 @@ class Scholarship < ApplicationRecord has_many :notifications, as: :noticeable, dependent: :destroy accepts_nested_attributes_for :comments, allow_destroy: true, reject_if: proc { |attrs| attrs["body"].blank? } - accepts_nested_attributes_for :notifications, reject_if: proc { |attrs| attrs["email_subject"].blank? } + accepts_nested_attributes_for :notifications, allow_destroy: true, reject_if: proc { |attrs| attrs["email_subject"].blank? } validates :amount_cents, numericality: { greater_than_or_equal_to: 0 } validate :recipient_must_match_allocation_registrant diff --git a/app/views/event_registrations/_form.html.erb b/app/views/event_registrations/_form.html.erb index 4a7a474f28..3d05cb4a55 100644 --- a/app/views/event_registrations/_form.html.erb +++ b/app/views/event_registrations/_form.html.erb @@ -302,7 +302,7 @@ <% end %> <%# ---- Comments ---- %> -
+
@@ -337,11 +337,11 @@
diff --git a/app/views/event_registrations/_notifications.html.erb b/app/views/event_registrations/_notifications.html.erb index fa1aed0404..7db967589f 100644 --- a/app/views/event_registrations/_notifications.html.erb +++ b/app/views/event_registrations/_notifications.html.erb @@ -1,12 +1,14 @@ <%# ---- Notifications — communications for this registrant, newest first. - Persisted entries are read-only and paginated like the comments box; new - manual log entries (email/phone/text/video) can be added inline and are - saved with the registration form. Links open in a new tab so unsaved edits - aren't lost. ---- %> + Automated entries are read-only; manually logged entries addressed to this + registration can be edited inline (toggle "Edit communications", like the + comments box) and new ones added, both saved with the registration form. + Links open in a new tab so unsaved edits aren't lost. ---- %> <% registration = f.object %> <% email = registration.registrant.preferred_email %> <% notifications = email.present? ? Notification.email(email).order(created_at: :desc) : Notification.none %> -
+<% new_notifications = registration.notifications.select(&:new_record?) %> +<% editable_notifications = notifications.select { |n| n.manual_log? && n.noticeable_type == EventRegistration.name && n.noticeable_id == registration.id } %> +
@@ -23,40 +25,50 @@
- <% if notifications.any? %> -
+
+
<% notifications.each do |notification| %> - <% subject = notification.email_subject.presence || notification.kind.to_s.humanize %> - <% body = notification.email_body_text.to_s %> -
- <%= notification.created_at.strftime("%-m/%-d/%Y") %> - <%= notification.sender&.full_name.presence || "AWBW Portal" %> - "> - <%= subject %> - <% if body.present? %> - <%= body %> - <% end %> - -
+ <% if editable_notifications.include?(notification) %> + <%= f.simple_fields_for :notifications, notification do |nf| %> + <%= render "notifications/notification_fields", f: nf, record: registration %> + <% end %> + <% else %> +
+ <%= render "notifications/notification_row", notification: notification %> +
+ <% end %> + <% end %> + <%# New, unsaved entries — re-rendered here on validation error. %> + <%= f.simple_fields_for :notifications, new_notifications do |nf| %> + <%= render "notifications/notification_fields", f: nf, record: registration %> <% end %> -
- <% else %> +
+
+ + <% if notifications.empty? && new_notifications.empty? %>

<%= t("communications.none_for_registrant") %>

<% end %> - <%# Inline add — manual log entries, saved with the registration form. %> -
- <%= f.simple_fields_for :notifications, registration.notifications.select(&:new_record?) do |nf| %> - <%= render "notifications/notification_fields", f: nf, record: registration %> - <% end %> +
<%= link_to_add_association f, :notifications, partial: "notifications/notification_fields", render_options: { locals: { record: registration } }, + data: { association_insertion_node: "#registration-notification-list", association_insertion_method: "append" }, class: "inline-flex items-center gap-1.5 rounded-md px-2 py-1 text-xs font-medium text-gray-500 transition-colors hover:bg-gray-100 hover:text-gray-700 cursor-pointer" do %> <%= t("communications.add") %> <% end %> + + <% if editable_notifications.any? %> + + <% end %>
diff --git a/app/views/notifications/_notification_fields.html.erb b/app/views/notifications/_notification_fields.html.erb index 100f88189e..0eef703885 100644 --- a/app/views/notifications/_notification_fields.html.erb +++ b/app/views/notifications/_notification_fields.html.erb @@ -1,41 +1,36 @@ -<%# New, unsaved manual notification log — sender, channel, and subject on the - first row, body beneath. Fields use SimpleForm styling. Amber until the - registration form is saved; a blank sender means the system ("AWBW Portal"). - Persisted notifications render read-only in the registration's notifications - box, not through this partial. ---- %> +<%# ---- One manual-log communication, editable inline (mirrors the comment + edit-mode pattern). A persisted entry renders a read-only `.notification-view` + row plus a hidden `.notification-edit` form; the shared `edit-toggle` + controller flips between them. A new, unsaved entry renders the amber add + form directly. Only manually logged communications addressed to this record + render through this partial — automated notifications stay read-only in the + notifications box. A blank sender means the system ("AWBW Portal"). ---- %> <% sender_users = User.where(super_user: true).includes(:person).to_a %> <% sender_users |= [ current_user ] if current_user %> <% sender_options = sender_users.sort_by { |u| u.full_name.to_s.downcase }.map { |u| [ u.full_name, u.id ] } %> <% channel_default = Notification::MANUAL_CHANNELS.include?(f.object.channel) ? f.object.channel : "email" %> <% record = local_assigns[:record] %> -
+
<%# Record this communication against its parent (the "Record" column in the notifications index) — e.g. an event registration or a person. %> <% if record %> <%= f.hidden_field :noticeable_type, value: record.class.name %> <%= f.hidden_field :noticeable_id, value: record.id %> <% end %> -
- <%= f.input :sender_id, - collection: sender_options, - include_blank: "AWBW Portal", - selected: f.object.sender_id || current_user&.id, - label: "From", - wrapper_html: { class: "sm:col-span-2" } %> - <%= f.input :channel, - collection: Notification::MANUAL_CHANNELS.map { |c| [ c.titleize, c ] }, - include_blank: false, - selected: channel_default, - label: "Channel", - wrapper_html: { class: "sm:col-span-1" } %> - <%= f.input :email_subject, label: "Subject", placeholder: "Topic or subject line", - input_html: { rows: 1 }, wrapper_html: { class: "sm:col-span-3" } %> -
- <%= f.input :email_body_text, as: :text, label: "Body", input_html: { rows: 3, placeholder: "What happened? (e.g. left voicemail, sent reminder)" } %> - -
- <%= link_to_remove_association "Remove", f, - class: "text-sm text-gray-400 underline hover:text-red-600" %> -
+ <% if f.object.persisted? %> + <%= f.hidden_field :id %> +
+ <%= render "notifications/notification_row", notification: f.object %> +
+ + <% else %> +
+ <%= render "notifications/notification_form", f: f, sender_options: sender_options, + sender_selected: f.object.sender_id || current_user&.id, channel_default: channel_default %> +
+ <% end %>
diff --git a/app/views/notifications/_notification_form.html.erb b/app/views/notifications/_notification_form.html.erb new file mode 100644 index 0000000000..caea737538 --- /dev/null +++ b/app/views/notifications/_notification_form.html.erb @@ -0,0 +1,26 @@ +<%# ---- The editable fields for a manual-log communication (From / Channel / + Subject / Body + Remove). Shared by a new entry and the edit mode of a + persisted one in _notification_fields. ---- %> +
+ <%= f.input :sender_id, + collection: sender_options, + include_blank: "AWBW Portal", + selected: sender_selected, + label: "From", + wrapper_html: { class: "sm:col-span-2" } %> + <%= f.input :channel, + collection: Notification::MANUAL_CHANNELS.map { |c| [ c.titleize, c ] }, + include_blank: false, + selected: channel_default, + label: "Channel", + wrapper_html: { class: "sm:col-span-1" } %> + <%= f.input :email_subject, label: "Subject", placeholder: "Topic or subject line", + input_html: { rows: 1 }, wrapper_html: { class: "sm:col-span-3" } %> +
+ +<%= f.input :email_body_text, as: :text, label: "Body", input_html: { rows: 3, placeholder: "What happened? (e.g. left voicemail, sent reminder)" } %> + +
+ <%= link_to_remove_association "Remove", f, + class: "text-sm text-gray-400 underline hover:text-red-600" %> +
diff --git a/app/views/notifications/_notification_row.html.erb b/app/views/notifications/_notification_row.html.erb new file mode 100644 index 0000000000..1ed64c83b5 --- /dev/null +++ b/app/views/notifications/_notification_row.html.erb @@ -0,0 +1,15 @@ +<%# ---- Read-only display of one communication (date, sender, subject, body). + Shared by the notifications box (automated entries) and the view mode of an + editable manual-log entry in _notification_fields. ---- %> +<% subject = notification.email_subject.presence || notification.kind.to_s.humanize %> +<% body = notification.email_body_text.to_s %> +
+ <%= notification.created_at.strftime("%-m/%-d/%Y") %> + <%= notification.sender&.full_name.presence || "AWBW Portal" %> + "> + <%= subject %> + <% if body.present? %> + <%= body %> + <% end %> + +
diff --git a/app/views/organizations/_form.html.erb b/app/views/organizations/_form.html.erb index 855477e164..28903d6467 100644 --- a/app/views/organizations/_form.html.erb +++ b/app/views/organizations/_form.html.erb @@ -402,7 +402,7 @@
Comments
-
+
<%= f.simple_fields_for :comments do |cf| %> <%= render "comments/comment_fields", f: cf %> <% end %> @@ -413,10 +413,10 @@ type="button" class="text-sm text-gray-400 hover:text-blue-600 underline cursor-pointer whitespace-nowrap" style="float:right" - data-action="click->comment-edit-toggle#toggle" + data-action="click->edit-toggle#toggle" > - Edit Comments - Done Editing + Edit Comments + Done Editing <%= link_to_add_association "➕ Add Comment", diff --git a/app/views/people/_form.html.erb b/app/views/people/_form.html.erb index a277210426..a0a31291e4 100644 --- a/app/views/people/_form.html.erb +++ b/app/views/people/_form.html.erb @@ -524,7 +524,7 @@
Comments
<%= f.simple_fields_for :comments do |cf| %> <%= render "comments/comment_fields", f: cf %> @@ -535,9 +535,9 @@ <%= link_to_add_association "➕ Add Comment", f, diff --git a/app/views/people/_notifications.html.erb b/app/views/people/_notifications.html.erb index b68c0f0663..132a0c4c02 100644 --- a/app/views/people/_notifications.html.erb +++ b/app/views/people/_notifications.html.erb @@ -1,12 +1,15 @@ <%# ---- Notifications — communications addressed to this person, newest first. - Persisted entries are read-only and paginated like the comments box; new - manual log entries (email/phone/text/video) can be added inline and are - saved with the person form. Links open in a new tab so unsaved edits aren't - lost. Mirrors the registration edit notifications section. ---- %> + Automated entries are read-only; manually logged entries addressed to this + person can be edited inline (toggle "Edit communications", like the comments + box) and new ones added, both saved with the person form. Links open in a + new tab so unsaved edits aren't lost. Mirrors the registration edit + notifications section. ---- %> <% person = f.object %> <% email = person.preferred_email %> <% notifications = email.present? ? Notification.email(email).order(created_at: :desc) : Notification.none %> -
+<% new_notifications = person.notifications.select(&:new_record?) %> +<% editable_notifications = notifications.select { |n| n.manual_log? && n.noticeable_type == Person.name && n.noticeable_id == person.id } %> +
@@ -23,40 +26,50 @@
- <% if notifications.any? %> -
+
+
<% notifications.each do |notification| %> - <% subject = notification.email_subject.presence || notification.kind.to_s.humanize %> - <% body = notification.email_body_text.to_s %> -
- <%= notification.created_at.strftime("%-m/%-d/%Y") %> - <%= notification.sender&.full_name.presence || "AWBW Portal" %> - "> - <%= subject %> - <% if body.present? %> - <%= body %> - <% end %> - -
+ <% if editable_notifications.include?(notification) %> + <%= f.simple_fields_for :notifications, notification do |nf| %> + <%= render "notifications/notification_fields", f: nf, record: person %> + <% end %> + <% else %> +
+ <%= render "notifications/notification_row", notification: notification %> +
+ <% end %> + <% end %> + <%# New, unsaved entries — re-rendered here on validation error. %> + <%= f.simple_fields_for :notifications, new_notifications do |nf| %> + <%= render "notifications/notification_fields", f: nf, record: person %> <% end %> -
- <% else %> +
+
+ + <% if notifications.empty? && new_notifications.empty? %>

<%= t("communications.none_for_person") %>

<% end %> - <%# Inline add — manual log entries, saved with the person form. %> -
- <%= f.simple_fields_for :notifications, person.notifications.select(&:new_record?) do |nf| %> - <%= render "notifications/notification_fields", f: nf, record: person %> - <% end %> +
<%= link_to_add_association f, :notifications, partial: "notifications/notification_fields", render_options: { locals: { record: person } }, + data: { association_insertion_node: "#person-notification-list", association_insertion_method: "append" }, class: "inline-flex items-center gap-1.5 rounded-md px-2 py-1 text-xs font-medium text-gray-500 transition-colors hover:bg-gray-100 hover:text-gray-700 cursor-pointer" do %> <%= t("communications.add") %> <% end %> + + <% if editable_notifications.any? %> + + <% end %>
diff --git a/app/views/scholarships/_form.html.erb b/app/views/scholarships/_form.html.erb index 4b9a615f0a..794de9b8b6 100644 --- a/app/views/scholarships/_form.html.erb +++ b/app/views/scholarships/_form.html.erb @@ -199,7 +199,7 @@ <% end %> <%# ---- Comments ---- %> -
+
@@ -228,11 +228,11 @@
diff --git a/app/views/scholarships/_notifications.html.erb b/app/views/scholarships/_notifications.html.erb index 51b89894a6..9a3e22df68 100644 --- a/app/views/scholarships/_notifications.html.erb +++ b/app/views/scholarships/_notifications.html.erb @@ -1,11 +1,14 @@ <%# ---- Notifications — communications for the scholarship recipient, newest - first. Persisted entries are read-only and paginated; new manual log entries - (email/phone/text/video) can be added inline and saved with the scholarship - form. Mirrors the registration notifications box. ---- %> + first. Automated entries are read-only; manually logged entries addressed to + this scholarship can be edited inline (toggle "Edit communications", like the + comments box) and new ones added, both saved with the scholarship form. + Mirrors the registration notifications box. ---- %> <% scholarship = f.object %> <% email = scholarship.recipient.preferred_email %> <% notifications = email.present? ? Notification.email(email).order(created_at: :desc) : Notification.none %> -
+<% new_notifications = scholarship.notifications.select(&:new_record?) %> +<% editable_notifications = notifications.select { |n| n.manual_log? && n.noticeable_type == Scholarship.name && n.noticeable_id == scholarship.id } %> +
@@ -22,38 +25,50 @@
- <% if notifications.any? %> -
+
+
<% notifications.each do |notification| %> - <% subject = notification.email_subject.presence || notification.kind.to_s.humanize %> - <% body = notification.email_body_text.to_s %> -
- <%= notification.created_at.strftime("%-m/%-d/%Y") %> - <%= notification.sender&.full_name.presence || "AWBW Portal" %> - "> - <%= subject %> - <% if body.present? %> - <%= body %> - <% end %> - -
+ <% if editable_notifications.include?(notification) %> + <%= f.simple_fields_for :notifications, notification do |nf| %> + <%= render "notifications/notification_fields", f: nf, record: scholarship %> + <% end %> + <% else %> +
+ <%= render "notifications/notification_row", notification: notification %> +
+ <% end %> + <% end %> + <%# New, unsaved entries — re-rendered here on validation error. %> + <%= f.simple_fields_for :notifications, new_notifications do |nf| %> + <%= render "notifications/notification_fields", f: nf, record: scholarship %> <% end %> -
+
+
+ + <% if notifications.empty? && new_notifications.empty? %> +

<%= t("communications.none_for_scholarship") %>

<% end %> - <%# Inline add — manual log entries, saved with the scholarship form. %> -
- <%= f.simple_fields_for :notifications, scholarship.notifications.select(&:new_record?) do |nf| %> - <%= render "notifications/notification_fields", f: nf, event_registration: scholarship %> - <% end %> +
<%= link_to_add_association f, :notifications, partial: "notifications/notification_fields", - render_options: { locals: { event_registration: scholarship } }, + render_options: { locals: { record: scholarship } }, + data: { association_insertion_node: "#scholarship-notification-list", association_insertion_method: "append" }, class: "inline-flex items-center gap-1.5 rounded-md px-2 py-1 text-xs font-medium text-gray-500 transition-colors hover:bg-gray-100 hover:text-gray-700 cursor-pointer" do %> <%= t("communications.add") %> <% end %> + + <% if editable_notifications.any? %> + + <% end %>
diff --git a/app/views/users/_form.html.erb b/app/views/users/_form.html.erb index 07c9665ff0..ac9ca3d048 100644 --- a/app/views/users/_form.html.erb +++ b/app/views/users/_form.html.erb @@ -135,7 +135,7 @@

Comments

<%= f.simple_fields_for :comments do |cf| %> <%= render "comments/comment_fields", f: cf %> @@ -151,9 +151,9 @@ class: "btn btn-secondary-outline" %>
diff --git a/app/views/workshops/_form.html.erb b/app/views/workshops/_form.html.erb index 1b5d53594d..3feb183dbe 100644 --- a/app/views/workshops/_form.html.erb +++ b/app/views/workshops/_form.html.erb @@ -483,7 +483,7 @@
Comments
<%= f.simple_fields_for :comments do |cf| %> <%= render "comments/comment_fields", f: cf %> @@ -494,9 +494,9 @@ <%= link_to_add_association "➕ Add Comment", f, diff --git a/config/locales/en.yml b/config/locales/en.yml index 70aa4971cc..4d3ce39a59 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -39,11 +39,14 @@ en: back_link: "← Back to communications" empty: "No communications found." add: "Add communication" + edit: "Edit communications" + done_editing: "Done editing" resent: "Communication has been resent successfully." registration_title: "Registration communications" scholarship_title: "Scholarship communications" none_for_person: "No communications for this person yet." none_for_registrant: "No communications for this registrant yet." + none_for_scholarship: "No communications for this scholarship yet." activerecord: attributes: workshop_variation: diff --git a/spec/requests/event_registrations_spec.rb b/spec/requests/event_registrations_spec.rb index 0c9df0eff0..4c815a3ba1 100644 --- a/spec/requests/event_registrations_spec.rb +++ b/spec/requests/event_registrations_spec.rb @@ -416,6 +416,23 @@ def unrequest(registration) } }.not_to change(Notification, :count) end + + it "edits an existing logged notification in place" do + note = create(:notification, noticeable: existing_registration, + recipient_email: existing_registration.registrant.preferred_email, + channel: "phone", email_subject: "Left a voicemail", + kind: "manual_log", recipient_role: "person", notification_type: 0) + + patch event_registration_path(existing_registration), params: { + event_registration: { + notifications_attributes: { "0" => { id: note.id, channel: "email", email_subject: "Sent a reminder" } } + } + } + + note.reload + expect(note.channel).to eq("email") + expect(note.email_subject).to eq("Sent a reminder") + end end describe "DELETE /event_registrations/:id" do diff --git a/spec/requests/people_notifications_spec.rb b/spec/requests/people_notifications_spec.rb index 083e1a8381..f3c09a2639 100644 --- a/spec/requests/people_notifications_spec.rb +++ b/spec/requests/people_notifications_spec.rb @@ -35,6 +35,36 @@ end end + describe "PATCH /people/:id editing a logged notification" do + let!(:log) do + create(:notification, noticeable: person, recipient_email: person.preferred_email, + channel: "phone", email_subject: "Left a voicemail", + kind: "manual_log", recipient_role: "person", notification_type: 0) + end + + it "updates an existing manual notification log in place" do + patch person_path(person), params: { + person: { + notifications_attributes: { "0" => { id: log.id, channel: "email", email_subject: "Sent a reminder" } } + } + } + + log.reload + expect(log.channel).to eq("email") + expect(log.email_subject).to eq("Sent a reminder") + end + + it "removes a logged notification when marked for destruction" do + expect { + patch person_path(person), params: { + person: { + notifications_attributes: { "0" => { id: log.id, _destroy: "1" } } + } + } + }.to change { person.notifications.count }.by(-1) + end + end + describe "GET /people/:id/edit" do it "lists past communications addressed to the person" do person = create(:person, user: nil, email: "comms@example.com") diff --git a/spec/requests/scholarships_spec.rb b/spec/requests/scholarships_spec.rb index e9797fb449..6315c6feb4 100644 --- a/spec/requests/scholarships_spec.rb +++ b/spec/requests/scholarships_spec.rb @@ -233,6 +233,18 @@ expect(note.email_subject).to eq("Called recipient") expect(note.recipient_email).to eq(scholarship.recipient.preferred_email.presence || "n/a") end + + it "edits an existing logged notification in place" do + note = create(:notification, noticeable: scholarship, + recipient_email: scholarship.recipient.preferred_email.presence || "n/a", + email_subject: "Called recipient", kind: "manual_log", + recipient_role: "person", notification_type: 0) + + patch scholarship_path(scholarship), + params: { scholarship: { notifications_attributes: { "0" => { id: note.id, email_subject: "Emailed recipient" } } } } + + expect(note.reload.email_subject).to eq("Emailed recipient") + end end describe "DELETE /scholarships/:id" do From d581e8598ddb61bf385201b077e8306d895fa8bc Mon Sep 17 00:00:00 2001 From: maebeale Date: Tue, 7 Jul 2026 21:23:50 -0400 Subject: [PATCH 02/15] Update app/views/event_registrations/_form.html.erb Co-authored-by: Justin <16829344+jmilljr24@users.noreply.github.com> --- app/views/event_registrations/_form.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/event_registrations/_form.html.erb b/app/views/event_registrations/_form.html.erb index 3d05cb4a55..f034d83c06 100644 --- a/app/views/event_registrations/_form.html.erb +++ b/app/views/event_registrations/_form.html.erb @@ -337,7 +337,7 @@ diff --git a/app/views/people/_notifications.html.erb b/app/views/people/_notifications.html.erb index 132a0c4c02..5fb9a8272d 100644 --- a/app/views/people/_notifications.html.erb +++ b/app/views/people/_notifications.html.erb @@ -9,7 +9,7 @@ <% notifications = email.present? ? Notification.email(email).order(created_at: :desc) : Notification.none %> <% new_notifications = person.notifications.select(&:new_record?) %> <% editable_notifications = notifications.select { |n| n.manual_log? && n.noticeable_type == Person.name && n.noticeable_id == person.id } %> -
+
@@ -64,7 +64,7 @@ <% if editable_notifications.any? %> diff --git a/app/views/workshops/_form.html.erb b/app/views/workshops/_form.html.erb index 3feb183dbe..8c239282e4 100644 --- a/app/views/workshops/_form.html.erb +++ b/app/views/workshops/_form.html.erb @@ -483,7 +483,7 @@
Comments
<%= f.simple_fields_for :comments do |cf| %> <%= render "comments/comment_fields", f: cf %> @@ -494,7 +494,7 @@ From 6d3f331b8d83ef8ed8b1503ffa08e6d94967d064 Mon Sep 17 00:00:00 2001 From: maebeale Date: Tue, 7 Jul 2026 21:47:30 -0400 Subject: [PATCH 04/15] Document "targets, not class selectors" Stimulus convention MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codify the review feedback from #1942 (and the broader Stimulus audit in #1392) so the next controller doesn't repeat it: finding elements by a marker CSS class — or passing class names in as Values to query by — should be Stimulus targets instead. Kept CLAUDE.md and the Copilot mirror in sync. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/copilot-instructions.md | 2 +- CLAUDE.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index ba3880bbd4..edcf29d573 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -239,7 +239,7 @@ this). Match the existing pattern: Follow the [Stimulus Handbook](https://stimulus.hotwired.dev/handbook/introduction) and reference docs. Key rules: -**Targets over querySelector** — declare `static targets = [...]` and use `data-[controller]-target` attributes in views. Never use `this.element.querySelector` or `document.getElementById` to find elements that could be targets. Exception: elements outside the controller's scope (e.g., in a parent view). +**Targets over querySelector** — declare `static targets = [...]` and use `data-[controller]-target` attributes in views. Never use `this.element.querySelector` or `document.getElementById` to find elements that could be targets. **This includes CSS-class lookups: don't select elements by a marker/hook class (`querySelectorAll(".foo-view")`), and don't pass CSS class names in as `Values` to query by — mark the elements as targets and iterate `this.fooTargets` instead.** When two target sets line up one-to-one (e.g. `viewTargets`/`editTargets`), pair them by DOM order. Exception: elements outside the controller's scope (e.g., in a parent view). (Part of the ongoing Stimulus-conventions audit, rubyforgood/awbw#1392.) **Values API for state** — use `static values = { name: Type }` for any state that persists or drives UI. Do not store state in instance variables when a value would work. Use `[name]ValueChanged()` callbacks for reactive updates instead of manual syncing. diff --git a/CLAUDE.md b/CLAUDE.md index 71caffa950..d63d4dfe90 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -239,7 +239,7 @@ this). Match the existing pattern: Follow the [Stimulus Handbook](https://stimulus.hotwired.dev/handbook/introduction) and reference docs. Key rules: -**Targets over querySelector** — declare `static targets = [...]` and use `data-[controller]-target` attributes in views. Never use `this.element.querySelector` or `document.getElementById` to find elements that could be targets. Exception: elements outside the controller's scope (e.g., in a parent view). +**Targets over querySelector** — declare `static targets = [...]` and use `data-[controller]-target` attributes in views. Never use `this.element.querySelector` or `document.getElementById` to find elements that could be targets. **This includes CSS-class lookups: don't select elements by a marker/hook class (`querySelectorAll(".foo-view")`), and don't pass CSS class names in as `Values` to query by — mark the elements as targets and iterate `this.fooTargets` instead.** When two target sets line up one-to-one (e.g. `viewTargets`/`editTargets`), pair them by DOM order. Exception: elements outside the controller's scope (e.g., in a parent view). (Part of the ongoing Stimulus-conventions audit, rubyforgood/awbw#1392.) **Values API for state** — use `static values = { name: Type }` for any state that persists or drives UI. Do not store state in instance variables when a value would work. Use `[name]ValueChanged()` callbacks for reactive updates instead of manual syncing. From 01df0df35b8065dbf500eaa7f803b27e0a0a2668 Mon Sep 17 00:00:00 2001 From: maebeale Date: Wed, 8 Jul 2026 06:25:04 -0400 Subject: [PATCH 05/15] Restrict communications editing to sender-attributed logs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review-driven guards: the "From" dropdown on the communications form no longer offers a blank "AWBW Portal" option, so a hand-logged communication must name a real sender; and system ("AWBW Portal", sender-less) communications are now view-only — they render read-only in the box instead of getting an inline edit form, matching how automated notifications already behave. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../_notifications.html.erb | 3 ++- .../notifications/_notification_form.html.erb | 4 +++- app/views/people/_notifications.html.erb | 3 ++- .../scholarships/_notifications.html.erb | 3 ++- spec/requests/people_notifications_spec.rb | 23 ++++++++++++++++++- 5 files changed, 31 insertions(+), 5 deletions(-) diff --git a/app/views/event_registrations/_notifications.html.erb b/app/views/event_registrations/_notifications.html.erb index b54e1a306c..e4b4d6ad77 100644 --- a/app/views/event_registrations/_notifications.html.erb +++ b/app/views/event_registrations/_notifications.html.erb @@ -7,7 +7,8 @@ <% email = registration.registrant.preferred_email %> <% notifications = email.present? ? Notification.email(email).order(created_at: :desc) : Notification.none %> <% new_notifications = registration.notifications.select(&:new_record?) %> -<% editable_notifications = notifications.select { |n| n.manual_log? && n.noticeable_type == EventRegistration.name && n.noticeable_id == registration.id } %> +<%# System ("AWBW Portal", sender-less) communications are view-only. %> +<% editable_notifications = notifications.select { |n| n.manual_log? && n.sender_id.present? && n.noticeable_type == EventRegistration.name && n.noticeable_id == registration.id } %>
diff --git a/app/views/notifications/_notification_form.html.erb b/app/views/notifications/_notification_form.html.erb index caea737538..77de327725 100644 --- a/app/views/notifications/_notification_form.html.erb +++ b/app/views/notifications/_notification_form.html.erb @@ -2,9 +2,11 @@ Subject / Body + Remove). Shared by a new entry and the edit mode of a persisted one in _notification_fields. ---- %>
+ <%# No blank "AWBW Portal" option — a hand-logged communication must have a + real sender; system ("AWBW Portal") communications are view-only. %> <%= f.input :sender_id, collection: sender_options, - include_blank: "AWBW Portal", + include_blank: false, selected: sender_selected, label: "From", wrapper_html: { class: "sm:col-span-2" } %> diff --git a/app/views/people/_notifications.html.erb b/app/views/people/_notifications.html.erb index 5fb9a8272d..50fd7590c0 100644 --- a/app/views/people/_notifications.html.erb +++ b/app/views/people/_notifications.html.erb @@ -8,7 +8,8 @@ <% email = person.preferred_email %> <% notifications = email.present? ? Notification.email(email).order(created_at: :desc) : Notification.none %> <% new_notifications = person.notifications.select(&:new_record?) %> -<% editable_notifications = notifications.select { |n| n.manual_log? && n.noticeable_type == Person.name && n.noticeable_id == person.id } %> +<%# System ("AWBW Portal", sender-less) communications are view-only. %> +<% editable_notifications = notifications.select { |n| n.manual_log? && n.sender_id.present? && n.noticeable_type == Person.name && n.noticeable_id == person.id } %>
diff --git a/app/views/scholarships/_notifications.html.erb b/app/views/scholarships/_notifications.html.erb index 9877eed9a0..e00cdb22a5 100644 --- a/app/views/scholarships/_notifications.html.erb +++ b/app/views/scholarships/_notifications.html.erb @@ -7,7 +7,8 @@ <% email = scholarship.recipient.preferred_email %> <% notifications = email.present? ? Notification.email(email).order(created_at: :desc) : Notification.none %> <% new_notifications = scholarship.notifications.select(&:new_record?) %> -<% editable_notifications = notifications.select { |n| n.manual_log? && n.noticeable_type == Scholarship.name && n.noticeable_id == scholarship.id } %> +<%# System ("AWBW Portal", sender-less) communications are view-only. %> +<% editable_notifications = notifications.select { |n| n.manual_log? && n.sender_id.present? && n.noticeable_type == Scholarship.name && n.noticeable_id == scholarship.id } %>
diff --git a/spec/requests/people_notifications_spec.rb b/spec/requests/people_notifications_spec.rb index f3c09a2639..a2adbde3c1 100644 --- a/spec/requests/people_notifications_spec.rb +++ b/spec/requests/people_notifications_spec.rb @@ -37,7 +37,7 @@ describe "PATCH /people/:id editing a logged notification" do let!(:log) do - create(:notification, noticeable: person, recipient_email: person.preferred_email, + create(:notification, noticeable: person, sender: admin, recipient_email: person.preferred_email, channel: "phone", email_subject: "Left a voicemail", kind: "manual_log", recipient_role: "person", notification_type: 0) end @@ -76,5 +76,26 @@ expect(response.body).to include("Communications") expect(response.body).to include("Welcome aboard") end + + it "makes a sender-attributed log editable but keeps a system (AWBW Portal) log view-only" do + editable = create(:notification, noticeable: person, sender: admin, + recipient_email: person.preferred_email, email_subject: "Called them", + kind: "manual_log", recipient_role: "person", notification_type: 0) + system_log = create(:notification, noticeable: person, sender: nil, + recipient_email: person.preferred_email, email_subject: "Automated blast", + kind: "manual_log", recipient_role: "person", notification_type: 0) + + get edit_person_path(person) + + # Both are shown... + expect(response.body).to include("Called them") + expect(response.body).to include("Automated blast") + # ...but only the sender-attributed one gets an editable nested-attributes field. + id_fields = Nokogiri::HTML(response.body) + .css('input[name*="notifications_attributes"][name$="[id]"]') + .map { |input| input["value"] } + expect(id_fields).to include(editable.id.to_s) + expect(id_fields).not_to include(system_log.id.to_s) + end end end From 4367189aca15b5487158725a3f28254596c241e4 Mon Sep 17 00:00:00 2001 From: maebeale Date: Wed, 8 Jul 2026 06:39:49 -0400 Subject: [PATCH 06/15] Require subject when a communication row is in use (fix silent drop) Adding a communication with a body but no subject saved nothing: the model's reject_if drops a blank-subject entry server-side, but the form never marked subject required, so the entry vanished on save with no feedback. Subject is now marked required as soon as the row is touched (subject or body), so a partially filled entry can't be silently dropped while an untouched cocoon row stays optional and won't block the form. Generalized the comment-only `comment-required` controller into a shared `field-required` (participating `field` targets + the `required` target(s) to toggle) and moved both the comments and communications boxes onto it. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../comment_required_controller.js | 23 -------- .../controllers/field_required_controller.js | 26 +++++++++ app/frontend/javascript/controllers/index.js | 4 +- app/models/comment.rb | 2 +- app/views/comments/_comment_fields.html.erb | 16 +++--- .../notifications/_notification_form.html.erb | 55 +++++++++++-------- 6 files changed, 69 insertions(+), 57 deletions(-) delete mode 100644 app/frontend/javascript/controllers/comment_required_controller.js create mode 100644 app/frontend/javascript/controllers/field_required_controller.js diff --git a/app/frontend/javascript/controllers/comment_required_controller.js b/app/frontend/javascript/controllers/comment_required_controller.js deleted file mode 100644 index b2869865c3..0000000000 --- a/app/frontend/javascript/controllers/comment_required_controller.js +++ /dev/null @@ -1,23 +0,0 @@ -import { Controller } from "@hotwired/stimulus"; - -// Connects to data-controller="comment-required" -// -// Makes the comment body required only once the row is in use (the topic or the -// body has any content). An untouched, cocoon-added row stays optional so the -// parent form can still submit — reject_if drops empty rows server-side — while -// a filled-in topic can't be saved without an accompanying comment. -// -export default class extends Controller { - static targets = ["topic", "body"]; - - connect() { - this.update(); - } - - update() { - const inUse = - (this.hasTopicTarget && this.topicTarget.value.trim() !== "") || - this.bodyTarget.value.trim() !== ""; - this.bodyTarget.required = inUse; - } -} diff --git a/app/frontend/javascript/controllers/field_required_controller.js b/app/frontend/javascript/controllers/field_required_controller.js new file mode 100644 index 0000000000..242ca9c3e4 --- /dev/null +++ b/app/frontend/javascript/controllers/field_required_controller.js @@ -0,0 +1,26 @@ +import { Controller } from "@hotwired/stimulus"; + +// Connects to data-controller="field-required" +// +// Makes the marked required field(s) required only once the row is in use (any +// participating `field` target has content). An untouched, cocoon-added row +// stays optional so the parent form can still submit — reject_if drops empty +// rows server-side — while an engaged row can't be saved without its required +// field, so a partially-filled entry is never silently dropped. +// +// Mark every participating input with data-field-required-target="field" and the +// input(s) that must be filled with data-field-required-target="required" (an +// input can be both, e.g. a required field that also triggers the check). +// +export default class extends Controller { + static targets = ["field", "required"]; + + connect() { + this.update(); + } + + update() { + const inUse = this.fieldTargets.some((el) => el.value.trim() !== ""); + this.requiredTargets.forEach((el) => (el.required = inUse)); + } +} diff --git a/app/frontend/javascript/controllers/index.js b/app/frontend/javascript/controllers/index.js index 834879a523..8a7e5514ac 100644 --- a/app/frontend/javascript/controllers/index.js +++ b/app/frontend/javascript/controllers/index.js @@ -51,8 +51,8 @@ application.register("column-toggle", ColumnToggleController) import EditToggleController from "./edit_toggle_controller" application.register("edit-toggle", EditToggleController) -import CommentRequiredController from "./comment_required_controller" -application.register("comment-required", CommentRequiredController) +import FieldRequiredController from "./field_required_controller" +application.register("field-required", FieldRequiredController) import DirtyFormController from "./dirty_form_controller" application.register("dirty-form", DirtyFormController) diff --git a/app/models/comment.rb b/app/models/comment.rb index fe71aedd39..99279475aa 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -5,7 +5,7 @@ class Comment < ApplicationRecord # Required for new comments only — existing comments (some legacy ones may have # blank bodies) must stay savable, so this is intentionally not a blanket - # validation. The UI enforces it client-side via the comment-required controller. + # validation. The UI enforces it client-side via the field-required controller. validates :body, presence: true, unless: :persisted? scope :newest_first, -> { order(created_at: :desc) } diff --git a/app/views/comments/_comment_fields.html.erb b/app/views/comments/_comment_fields.html.erb index 776a23623c..1f61e77ecb 100644 --- a/app/views/comments/_comment_fields.html.erb +++ b/app/views/comments/_comment_fields.html.erb @@ -68,12 +68,12 @@ <% end %>
+ data-controller="field-required" + data-action="input->field-required#update">
<%= f.input_field :topic, placeholder: "Topic", - data: { comment_required_target: "topic" }, + data: { field_required_target: "field" }, class: "w-full bg-white rounded-md border border-gray-300 px-3 py-2 text-sm shadow-sm focus:border-blue-500 focus:ring focus:ring-blue-200 focus:outline-none" %>

Reason for this note

@@ -81,7 +81,7 @@ as: :text, rows: 1, required: false, - data: { comment_required_target: "body" }, + data: { field_required_target: "field required" }, class: "grow bg-white rounded-md border border-gray-300 px-3 py-2 text-sm shadow-sm focus:border-blue-500 focus:ring focus:ring-blue-200 focus:outline-none" %>
<%= link_to_remove_association "Remove", f, @@ -98,12 +98,12 @@ <%= truncate(current_first_name.to_s, length: 10, omission: "..") %>
+ data-controller="field-required" + data-action="input->field-required#update">
<%= f.input_field :topic, placeholder: "Topic", - data: { comment_required_target: "topic" }, + data: { field_required_target: "field" }, class: "w-full bg-white rounded-md border border-gray-300 px-3 py-2 shadow-sm focus:border-blue-500 focus:ring focus:ring-blue-200 focus:outline-none" %>

Reason for this note

@@ -112,7 +112,7 @@ rows: 2, required: false, placeholder: "Type your comment", - data: { comment_required_target: "body" }, + data: { field_required_target: "field required" }, class: "grow bg-white rounded-md border border-gray-300 px-3 py-2 shadow-sm focus:border-blue-500 focus:ring focus:ring-blue-200 focus:outline-none" %>
diff --git a/app/views/notifications/_notification_form.html.erb b/app/views/notifications/_notification_form.html.erb index 77de327725..f2d8210bef 100644 --- a/app/views/notifications/_notification_form.html.erb +++ b/app/views/notifications/_notification_form.html.erb @@ -1,28 +1,37 @@ <%# ---- The editable fields for a manual-log communication (From / Channel / Subject / Body + Remove). Shared by a new entry and the edit mode of a - persisted one in _notification_fields. ---- %> -
- <%# No blank "AWBW Portal" option — a hand-logged communication must have a - real sender; system ("AWBW Portal") communications are view-only. %> - <%= f.input :sender_id, - collection: sender_options, - include_blank: false, - selected: sender_selected, - label: "From", - wrapper_html: { class: "sm:col-span-2" } %> - <%= f.input :channel, - collection: Notification::MANUAL_CHANNELS.map { |c| [ c.titleize, c ] }, - include_blank: false, - selected: channel_default, - label: "Channel", - wrapper_html: { class: "sm:col-span-1" } %> - <%= f.input :email_subject, label: "Subject", placeholder: "Topic or subject line", - input_html: { rows: 1 }, wrapper_html: { class: "sm:col-span-3" } %> -
+ persisted one in _notification_fields. Subject is required once the row is in + use (server-side reject_if drops a blank-subject entry), so field-required + marks it required as soon as the subject or body is touched — a partially + filled entry can't be silently dropped, while an untouched added row stays + optional and won't block the parent form. ---- %> +
+
+ <%# No blank "AWBW Portal" option — a hand-logged communication must have a + real sender; system ("AWBW Portal") communications are view-only. %> + <%= f.input :sender_id, + collection: sender_options, + include_blank: false, + selected: sender_selected, + label: "From", + wrapper_html: { class: "sm:col-span-2" } %> + <%= f.input :channel, + collection: Notification::MANUAL_CHANNELS.map { |c| [ c.titleize, c ] }, + include_blank: false, + selected: channel_default, + label: "Channel", + wrapper_html: { class: "sm:col-span-1" } %> + <%= f.input :email_subject, label: "Subject", placeholder: "Topic or subject line", + input_html: { rows: 1, data: { field_required_target: "field required" } }, + wrapper_html: { class: "sm:col-span-3" } %> +
-<%= f.input :email_body_text, as: :text, label: "Body", input_html: { rows: 3, placeholder: "What happened? (e.g. left voicemail, sent reminder)" } %> + <%= f.input :email_body_text, as: :text, label: "Body", + input_html: { rows: 3, placeholder: "What happened? (e.g. left voicemail, sent reminder)", + data: { field_required_target: "field" } } %> -
- <%= link_to_remove_association "Remove", f, - class: "text-sm text-gray-400 underline hover:text-red-600" %> +
+ <%= link_to_remove_association "Remove", f, + class: "text-sm text-gray-400 underline hover:text-red-600" %> +
From a360fbeab05f774da7ddcdf201ba4a85eabd4a05 Mon Sep 17 00:00:00 2001 From: maebeale Date: Wed, 8 Jul 2026 06:52:50 -0400 Subject: [PATCH 07/15] Add channel icons and gate communication bodies to admins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three UI changes to the communications box: - Show a solid Font Awesome channel icon before each subject (envelope / phone / mobile handset for text / video), via a NotificationDecorator helper. Sender name is a fixed, snug width and truncates so the icons line up in a column regardless of name length. - Make the message body admin-only while keeping the subject (and icon) visible to anyone who can see the page — so a recipient viewing their own record (once person edit opens to them) sees what was sent but not the internal note. Adding/editing hand-noted communications stays admin-only. The person form now always renders the box and gates the admin bits inside, instead of hiding the whole section. Co-Authored-By: Claude Opus 4.8 (1M context) --- AGENTS.md | 1 - app/decorators/notification_decorator.rb | 18 +++++ .../_notifications.html.erb | 65 +++++++++--------- .../notifications/_notification_row.html.erb | 16 +++-- app/views/people/_form.html.erb | 7 +- app/views/people/_notifications.html.erb | 67 ++++++++++--------- .../scholarships/_notifications.html.erb | 63 +++++++++-------- .../decorators/notification_decorator_spec.rb | 23 +++++++ .../_notification_row.html.erb_spec.rb | 23 +++++++ 9 files changed, 183 insertions(+), 100 deletions(-) create mode 100644 spec/decorators/notification_decorator_spec.rb create mode 100644 spec/views/notifications/_notification_row.html.erb_spec.rb diff --git a/AGENTS.md b/AGENTS.md index ad089c6f0a..5cc35e2baa 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -285,7 +285,6 @@ end - `cocoon` — Nested form handling (cocoon gem) - `collection` — Filter form auto-submit with debounce - `column_toggle` — Toggle table column visibility -- `comment_required` — Require a comment body once a topic or body is filled in - `confirm_email` — Email confirmation UI - `dirty_form` — Unsaved changes detection - `dismiss` — Dismissable elements diff --git a/app/decorators/notification_decorator.rb b/app/decorators/notification_decorator.rb index 0f9f9a819e..13ba456183 100644 --- a/app/decorators/notification_decorator.rb +++ b/app/decorators/notification_decorator.rb @@ -1,8 +1,26 @@ class NotificationDecorator < ApplicationDecorator + # Solid Font Awesome icon per communication channel, shown before the subject + # in the communications box. "autoemail" (system email) reuses the envelope; + # "text" uses a mobile handset to stay distinct from the "phone" call icon. + CHANNEL_ICONS = { + "autoemail" => "fa-envelope", + "email" => "fa-envelope", + "phone" => "fa-phone", + "text" => "fa-mobile-screen-button", + "video" => "fa-video" + }.freeze + def title "Re #{noticeable_type} ##{noticeable_id}" end def detail(length: nil) end + + def channel_icon(**options) + icon_class = CHANNEL_ICONS[channel] + return "" if icon_class.blank? + + h.content_tag(:i, "", { class: "fa-solid #{icon_class} text-gray-400", title: channel.titleize, "aria-hidden": "true" }.merge(options)) + end end diff --git a/app/views/event_registrations/_notifications.html.erb b/app/views/event_registrations/_notifications.html.erb index e4b4d6ad77..cf8d5ed71f 100644 --- a/app/views/event_registrations/_notifications.html.erb +++ b/app/views/event_registrations/_notifications.html.erb @@ -1,21 +1,22 @@ <%# ---- Notifications — communications for this registrant, newest first. - Automated entries are read-only; manually logged entries addressed to this - registration can be edited inline (toggle "Edit communications", like the - comments box) and new ones added, both saved with the registration form. - Links open in a new tab so unsaved edits aren't lost. ---- %> + Subjects (and the channel icon) are visible to anyone who can see this page; + the message body and the ability to add or edit hand-noted communications + are admin-only. Automated entries are always read-only. Links open in a new + tab so unsaved edits aren't lost. ---- %> <% registration = f.object %> <% email = registration.registrant.preferred_email %> <% notifications = email.present? ? Notification.email(email).order(created_at: :desc) : Notification.none %> +<% admin = allowed_to?(:index?, with: NotificationPolicy) %> <% new_notifications = registration.notifications.select(&:new_record?) %> -<%# System ("AWBW Portal", sender-less) communications are view-only. %> -<% editable_notifications = notifications.select { |n| n.manual_log? && n.sender_id.present? && n.noticeable_type == EventRegistration.name && n.noticeable_id == registration.id } %> +<%# Only admins can edit; system ("AWBW Portal", sender-less) communications are view-only. %> +<% editable_notifications = admin ? notifications.select { |n| n.manual_log? && n.sender_id.present? && n.noticeable_type == EventRegistration.name && n.noticeable_id == registration.id } : [] %>

<%= t("communications.registration_title") %>

- <% if email.present? %> + <% if email.present? && admin %> <%= link_to notifications_path(email: email), 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 %> @@ -35,13 +36,15 @@ <% end %> <% else %>
- <%= render "notifications/notification_row", notification: notification %> + <%= render "notifications/notification_row", notification: notification, show_body: admin %>
<% end %> <% end %> - <%# New, unsaved entries — re-rendered here on validation error. %> - <%= f.simple_fields_for :notifications, new_notifications do |nf| %> - <%= render "notifications/notification_fields", f: nf, record: registration %> + <% if admin %> + <%# New, unsaved entries — re-rendered here on validation error. %> + <%= f.simple_fields_for :notifications, new_notifications do |nf| %> + <%= render "notifications/notification_fields", f: nf, record: registration %> + <% end %> <% end %>
@@ -51,25 +54,27 @@

<%= t("communications.none_for_registrant") %>

<% end %> -
- <%= link_to_add_association f, :notifications, - partial: "notifications/notification_fields", - render_options: { locals: { record: registration } }, - data: { association_insertion_node: "#registration-notification-list", association_insertion_method: "append" }, - class: "inline-flex items-center gap-1.5 rounded-md px-2 py-1 text-xs font-medium text-gray-500 transition-colors hover:bg-gray-100 hover:text-gray-700 cursor-pointer" do %> - - <%= t("communications.add") %> - <% end %> + <% if admin %> +
+ <%= link_to_add_association f, :notifications, + partial: "notifications/notification_fields", + render_options: { locals: { record: registration } }, + data: { association_insertion_node: "#registration-notification-list", association_insertion_method: "append" }, + class: "inline-flex items-center gap-1.5 rounded-md px-2 py-1 text-xs font-medium text-gray-500 transition-colors hover:bg-gray-100 hover:text-gray-700 cursor-pointer" do %> + + <%= t("communications.add") %> + <% end %> - <% if editable_notifications.any? %> - - <% end %> -
+ <% if editable_notifications.any? %> + + <% end %> +
+ <% end %>
diff --git a/app/views/notifications/_notification_row.html.erb b/app/views/notifications/_notification_row.html.erb index 1ed64c83b5..60c8e76dca 100644 --- a/app/views/notifications/_notification_row.html.erb +++ b/app/views/notifications/_notification_row.html.erb @@ -1,12 +1,18 @@ -<%# ---- Read-only display of one communication (date, sender, subject, body). - Shared by the notifications box (automated entries) and the view mode of an - editable manual-log entry in _notification_fields. ---- %> +<%# ---- Read-only display of one communication: channel icon + subject (always + visible), then the body (admin-only — pass show_body: false to hide it for + non-admins, e.g. the recipient viewing their own record). Shared by the + notifications box and the view mode of an editable manual-log entry. ---- %> +<% show_body = local_assigns.fetch(:show_body, true) %> <% subject = notification.email_subject.presence || notification.kind.to_s.humanize %> -<% body = notification.email_body_text.to_s %> +<% body = notification.email_body_text.to_s if show_body %> +<% sender_name = notification.sender&.full_name.presence || "AWBW Portal" %>
<%= notification.created_at.strftime("%-m/%-d/%Y") %> - <%= notification.sender&.full_name.presence || "AWBW Portal" %> + <%# Fixed, snug width (~"Umberto User") + truncate so the channel icons line up + regardless of name length, without a wide gap before the icon. %> + <%= sender_name %> "> + <%= notification.decorate.channel_icon %> <%= subject %> <% if body.present? %> <%= body %> diff --git a/app/views/people/_form.html.erb b/app/views/people/_form.html.erb index 9d7327d76a..e235e80a46 100644 --- a/app/views/people/_form.html.erb +++ b/app/views/people/_form.html.erb @@ -513,10 +513,9 @@
<% if f.object.persisted? %> - - <% if allowed_to?(:index?, with: NotificationPolicy) %> - <%= render "notifications", f: f %> - <% end %> + <%# Communications — subjects are visible to the person; the body and the + add/edit controls are admin-only (gated inside the partial). %> + <%= render "notifications", f: f %> <% if allowed_to?(:manage?, Comment) %> diff --git a/app/views/people/_notifications.html.erb b/app/views/people/_notifications.html.erb index 50fd7590c0..7cbf89070c 100644 --- a/app/views/people/_notifications.html.erb +++ b/app/views/people/_notifications.html.erb @@ -1,22 +1,23 @@ <%# ---- Notifications — communications addressed to this person, newest first. - Automated entries are read-only; manually logged entries addressed to this - person can be edited inline (toggle "Edit communications", like the comments - box) and new ones added, both saved with the person form. Links open in a - new tab so unsaved edits aren't lost. Mirrors the registration edit - notifications section. ---- %> + Subjects (and the channel icon) are visible to anyone who can see this page, + including the person themselves; the message body and the ability to add or + edit hand-noted communications are admin-only. Automated entries are always + read-only. Links open in a new tab so unsaved edits aren't lost. Mirrors the + registration edit notifications section. ---- %> <% person = f.object %> <% email = person.preferred_email %> <% notifications = email.present? ? Notification.email(email).order(created_at: :desc) : Notification.none %> +<% admin = allowed_to?(:index?, with: NotificationPolicy) %> <% new_notifications = person.notifications.select(&:new_record?) %> -<%# System ("AWBW Portal", sender-less) communications are view-only. %> -<% editable_notifications = notifications.select { |n| n.manual_log? && n.sender_id.present? && n.noticeable_type == Person.name && n.noticeable_id == person.id } %> +<%# Only admins can edit; system ("AWBW Portal", sender-less) communications are view-only. %> +<% editable_notifications = admin ? notifications.select { |n| n.manual_log? && n.sender_id.present? && n.noticeable_type == Person.name && n.noticeable_id == person.id } : [] %>

<%= t("communications.title") %>

- <% if email.present? %> + <% if email.present? && admin %> <%= link_to notifications_path(email: email), 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 %> @@ -36,13 +37,15 @@ <% end %> <% else %>
- <%= render "notifications/notification_row", notification: notification %> + <%= render "notifications/notification_row", notification: notification, show_body: admin %>
<% end %> <% end %> - <%# New, unsaved entries — re-rendered here on validation error. %> - <%= f.simple_fields_for :notifications, new_notifications do |nf| %> - <%= render "notifications/notification_fields", f: nf, record: person %> + <% if admin %> + <%# New, unsaved entries — re-rendered here on validation error. %> + <%= f.simple_fields_for :notifications, new_notifications do |nf| %> + <%= render "notifications/notification_fields", f: nf, record: person %> + <% end %> <% end %>
@@ -52,25 +55,27 @@

<%= t("communications.none_for_person") %>

<% end %> -
- <%= link_to_add_association f, :notifications, - partial: "notifications/notification_fields", - render_options: { locals: { record: person } }, - data: { association_insertion_node: "#person-notification-list", association_insertion_method: "append" }, - class: "inline-flex items-center gap-1.5 rounded-md px-2 py-1 text-xs font-medium text-gray-500 transition-colors hover:bg-gray-100 hover:text-gray-700 cursor-pointer" do %> - - <%= t("communications.add") %> - <% end %> + <% if admin %> +
+ <%= link_to_add_association f, :notifications, + partial: "notifications/notification_fields", + render_options: { locals: { record: person } }, + data: { association_insertion_node: "#person-notification-list", association_insertion_method: "append" }, + class: "inline-flex items-center gap-1.5 rounded-md px-2 py-1 text-xs font-medium text-gray-500 transition-colors hover:bg-gray-100 hover:text-gray-700 cursor-pointer" do %> + + <%= t("communications.add") %> + <% end %> - <% if editable_notifications.any? %> - - <% end %> -
+ <% if editable_notifications.any? %> + + <% end %> +
+ <% end %>
diff --git a/app/views/scholarships/_notifications.html.erb b/app/views/scholarships/_notifications.html.erb index e00cdb22a5..7bf0286033 100644 --- a/app/views/scholarships/_notifications.html.erb +++ b/app/views/scholarships/_notifications.html.erb @@ -1,21 +1,22 @@ <%# ---- Notifications — communications for the scholarship recipient, newest - first. Automated entries are read-only; manually logged entries addressed to - this scholarship can be edited inline (toggle "Edit communications", like the - comments box) and new ones added, both saved with the scholarship form. + first. Subjects (and the channel icon) are visible to anyone who can see this + page; the message body and the ability to add or edit hand-noted + communications are admin-only. Automated entries are always read-only. Mirrors the registration notifications box. ---- %> <% scholarship = f.object %> <% email = scholarship.recipient.preferred_email %> <% notifications = email.present? ? Notification.email(email).order(created_at: :desc) : Notification.none %> +<% admin = allowed_to?(:index?, with: NotificationPolicy) %> <% new_notifications = scholarship.notifications.select(&:new_record?) %> -<%# System ("AWBW Portal", sender-less) communications are view-only. %> -<% editable_notifications = notifications.select { |n| n.manual_log? && n.sender_id.present? && n.noticeable_type == Scholarship.name && n.noticeable_id == scholarship.id } %> +<%# Only admins can edit; system ("AWBW Portal", sender-less) communications are view-only. %> +<% editable_notifications = admin ? notifications.select { |n| n.manual_log? && n.sender_id.present? && n.noticeable_type == Scholarship.name && n.noticeable_id == scholarship.id } : [] %>

<%= t("communications.scholarship_title") %>

- <% if email.present? %> + <% if email.present? && admin %> <%= link_to notifications_path(email: email), 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 %> @@ -35,13 +36,15 @@ <% end %> <% else %>
- <%= render "notifications/notification_row", notification: notification %> + <%= render "notifications/notification_row", notification: notification, show_body: admin %>
<% end %> <% end %> - <%# New, unsaved entries — re-rendered here on validation error. %> - <%= f.simple_fields_for :notifications, new_notifications do |nf| %> - <%= render "notifications/notification_fields", f: nf, record: scholarship %> + <% if admin %> + <%# New, unsaved entries — re-rendered here on validation error. %> + <%= f.simple_fields_for :notifications, new_notifications do |nf| %> + <%= render "notifications/notification_fields", f: nf, record: scholarship %> + <% end %> <% end %>
@@ -51,25 +54,27 @@

<%= t("communications.none_for_scholarship") %>

<% end %> -
- <%= link_to_add_association f, :notifications, - partial: "notifications/notification_fields", - render_options: { locals: { record: scholarship } }, - data: { association_insertion_node: "#scholarship-notification-list", association_insertion_method: "append" }, - class: "inline-flex items-center gap-1.5 rounded-md px-2 py-1 text-xs font-medium text-gray-500 transition-colors hover:bg-gray-100 hover:text-gray-700 cursor-pointer" do %> - - <%= t("communications.add") %> - <% end %> + <% if admin %> +
+ <%= link_to_add_association f, :notifications, + partial: "notifications/notification_fields", + render_options: { locals: { record: scholarship } }, + data: { association_insertion_node: "#scholarship-notification-list", association_insertion_method: "append" }, + class: "inline-flex items-center gap-1.5 rounded-md px-2 py-1 text-xs font-medium text-gray-500 transition-colors hover:bg-gray-100 hover:text-gray-700 cursor-pointer" do %> + + <%= t("communications.add") %> + <% end %> - <% if editable_notifications.any? %> - - <% end %> -
+ <% if editable_notifications.any? %> + + <% end %> +
+ <% end %>
diff --git a/spec/decorators/notification_decorator_spec.rb b/spec/decorators/notification_decorator_spec.rb new file mode 100644 index 0000000000..973446588c --- /dev/null +++ b/spec/decorators/notification_decorator_spec.rb @@ -0,0 +1,23 @@ +require "rails_helper" + +RSpec.describe NotificationDecorator, type: :decorator do + describe "#channel_icon" do + { + "email" => "fa-envelope", + "autoemail" => "fa-envelope", + "phone" => "fa-phone", + "text" => "fa-mobile-screen-button", + "video" => "fa-video" + }.each do |channel, icon_class| + it "renders the solid #{icon_class} icon for the #{channel} channel" do + html = build_stubbed(:notification, channel: channel).decorate.channel_icon + expect(html).to include("fa-solid") + expect(html).to include(icon_class) + end + end + + it "renders nothing for a blank or unknown channel" do + expect(build_stubbed(:notification, channel: nil).decorate.channel_icon).to eq("") + end + end +end diff --git a/spec/views/notifications/_notification_row.html.erb_spec.rb b/spec/views/notifications/_notification_row.html.erb_spec.rb new file mode 100644 index 0000000000..ba9394af70 --- /dev/null +++ b/spec/views/notifications/_notification_row.html.erb_spec.rb @@ -0,0 +1,23 @@ +require "rails_helper" + +RSpec.describe "notifications/_notification_row", type: :view do + let(:notification) do + build_stubbed(:notification, channel: "phone", email_subject: "Called them", + email_body_text: "Left a voicemail about the deadline") + end + + it "shows the channel icon, subject, and body when show_body is true" do + render partial: "notifications/notification_row", locals: { notification: notification, show_body: true } + + expect(rendered).to have_css("i.fa-solid.fa-phone") + expect(rendered).to include("Called them") + expect(rendered).to include("Left a voicemail about the deadline") + end + + it "hides the body (admin-only content) when show_body is false" do + render partial: "notifications/notification_row", locals: { notification: notification, show_body: false } + + expect(rendered).to include("Called them") + expect(rendered).not_to include("Left a voicemail about the deadline") + end +end From 371314411ecc8d590fae4e0932fbb5a01895ae7c Mon Sep 17 00:00:00 2001 From: maebeale Date: Wed, 8 Jul 2026 07:01:56 -0400 Subject: [PATCH 08/15] Gate communication bodies by type; mark subject required MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refine the communications box so the body visibility matches what the content actually is: - An automated ("AWBW Portal"/autoemail) message body is the recipient's own email — it stays visible to everyone (inline and on hover), with no admin styling. - A hand-noted communication's body is an internal staff note — admin-only, shown with the admin-only bg-blue-100 styling and omitted entirely (text and hover title) for non-admins. Admins still add/edit the hand-noted ones. Also mark the Subject field required (asterisk) on the form, since a blank subject is dropped on save; field-required keeps it from blocking an untouched added row. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../_notifications.html.erb | 11 ++++--- .../_notification_fields.html.erb | 2 +- .../notifications/_notification_form.html.erb | 5 ++- .../notifications/_notification_row.html.erb | 21 ++++++++++--- app/views/people/_notifications.html.erb | 13 ++++---- .../scholarships/_notifications.html.erb | 11 ++++--- .../_notification_row.html.erb_spec.rb | 31 ++++++++++++++----- 7 files changed, 64 insertions(+), 30 deletions(-) diff --git a/app/views/event_registrations/_notifications.html.erb b/app/views/event_registrations/_notifications.html.erb index cf8d5ed71f..2d08d217f8 100644 --- a/app/views/event_registrations/_notifications.html.erb +++ b/app/views/event_registrations/_notifications.html.erb @@ -1,8 +1,9 @@ <%# ---- Notifications — communications for this registrant, newest first. - Subjects (and the channel icon) are visible to anyone who can see this page; - the message body and the ability to add or edit hand-noted communications - are admin-only. Automated entries are always read-only. Links open in a new - tab so unsaved edits aren't lost. ---- %> + Dates, subjects, and the channel icon are visible to anyone who can see this + page. An automated ("AWBW Portal") message body stays visible (it is the + recipient's own email); a hand-noted communication's body is an internal + staff note — admin-only. Adding/editing hand-noted communications is + admin-only too. Links open in a new tab so unsaved edits aren't lost. ---- %> <% registration = f.object %> <% email = registration.registrant.preferred_email %> <% notifications = email.present? ? Notification.email(email).order(created_at: :desc) : Notification.none %> @@ -36,7 +37,7 @@ <% end %> <% else %>
- <%= render "notifications/notification_row", notification: notification, show_body: admin %> + <%= render "notifications/notification_row", notification: notification, admin: admin %>
<% end %> <% end %> diff --git a/app/views/notifications/_notification_fields.html.erb b/app/views/notifications/_notification_fields.html.erb index 75f4febb69..584813a10c 100644 --- a/app/views/notifications/_notification_fields.html.erb +++ b/app/views/notifications/_notification_fields.html.erb @@ -21,7 +21,7 @@ <% if f.object.persisted? %> <%= f.hidden_field :id %>
- <%= render "notifications/notification_row", notification: f.object %> + <%= render "notifications/notification_row", notification: f.object, admin: true %>
diff --git a/app/views/notifications/_notification_row.html.erb b/app/views/notifications/_notification_row.html.erb index 60c8e76dca..1041af1201 100644 --- a/app/views/notifications/_notification_row.html.erb +++ b/app/views/notifications/_notification_row.html.erb @@ -1,8 +1,14 @@ <%# ---- Read-only display of one communication: channel icon + subject (always - visible), then the body (admin-only — pass show_body: false to hide it for - non-admins, e.g. the recipient viewing their own record). Shared by the - notifications box and the view mode of an editable manual-log entry. ---- %> -<% show_body = local_assigns.fetch(:show_body, true) %> + visible), then the body. The body of a hand-noted communication (manual_log) + is an internal staff note — admin-only, shown with the admin-only blue + styling and hidden from non-admins (no inline text, nothing in the hover + title). An automated ("AWBW Portal") message is the recipient's own email, so + its body stays visible to everyone. Pass admin: false for a non-admin viewer. + Shared by the notifications box and the view mode of an editable manual-log + entry. ---- %> +<% admin = local_assigns.fetch(:admin, true) %> +<% body_admin_only = notification.manual_log? %> +<% show_body = admin || !body_admin_only %> <% subject = notification.email_subject.presence || notification.kind.to_s.humanize %> <% body = notification.email_body_text.to_s if show_body %> <% sender_name = notification.sender&.full_name.presence || "AWBW Portal" %> @@ -15,7 +21,12 @@ <%= notification.decorate.channel_icon %> <%= subject %> <% if body.present? %> - <%= body %> + <% if body_admin_only %> + <%# Internal staff note — admin-only, marked with the admin-only blue styling. %> + <%= body %> + <% else %> + <%= body %> + <% end %> <% end %>
diff --git a/app/views/people/_notifications.html.erb b/app/views/people/_notifications.html.erb index 7cbf89070c..5ef03a90fc 100644 --- a/app/views/people/_notifications.html.erb +++ b/app/views/people/_notifications.html.erb @@ -1,9 +1,10 @@ <%# ---- Notifications — communications addressed to this person, newest first. - Subjects (and the channel icon) are visible to anyone who can see this page, - including the person themselves; the message body and the ability to add or - edit hand-noted communications are admin-only. Automated entries are always - read-only. Links open in a new tab so unsaved edits aren't lost. Mirrors the - registration edit notifications section. ---- %> + Dates, subjects, and the channel icon are visible to anyone who can see this + page, including the person. An automated ("AWBW Portal") message body is the + person's own email, so it stays visible to them; a hand-noted communication's + body is an internal staff note — admin-only. Adding/editing hand-noted + communications is admin-only too. Links open in a new tab so unsaved edits + aren't lost. Mirrors the registration edit notifications section. ---- %> <% person = f.object %> <% email = person.preferred_email %> <% notifications = email.present? ? Notification.email(email).order(created_at: :desc) : Notification.none %> @@ -37,7 +38,7 @@ <% end %> <% else %>
- <%= render "notifications/notification_row", notification: notification, show_body: admin %> + <%= render "notifications/notification_row", notification: notification, admin: admin %>
<% end %> <% end %> diff --git a/app/views/scholarships/_notifications.html.erb b/app/views/scholarships/_notifications.html.erb index 7bf0286033..6dccd61eee 100644 --- a/app/views/scholarships/_notifications.html.erb +++ b/app/views/scholarships/_notifications.html.erb @@ -1,8 +1,9 @@ <%# ---- Notifications — communications for the scholarship recipient, newest - first. Subjects (and the channel icon) are visible to anyone who can see this - page; the message body and the ability to add or edit hand-noted - communications are admin-only. Automated entries are always read-only. - Mirrors the registration notifications box. ---- %> + first. Dates, subjects, and the channel icon are visible to anyone who can + see this page. An automated ("AWBW Portal") message body stays visible (it is + the recipient's own email); a hand-noted communication's body is an internal + staff note — admin-only. Adding/editing hand-noted communications is + admin-only too. Mirrors the registration notifications box. ---- %> <% scholarship = f.object %> <% email = scholarship.recipient.preferred_email %> <% notifications = email.present? ? Notification.email(email).order(created_at: :desc) : Notification.none %> @@ -36,7 +37,7 @@ <% end %> <% else %>
- <%= render "notifications/notification_row", notification: notification, show_body: admin %> + <%= render "notifications/notification_row", notification: notification, admin: admin %>
<% end %> <% end %> diff --git a/spec/views/notifications/_notification_row.html.erb_spec.rb b/spec/views/notifications/_notification_row.html.erb_spec.rb index ba9394af70..1b0163a156 100644 --- a/spec/views/notifications/_notification_row.html.erb_spec.rb +++ b/spec/views/notifications/_notification_row.html.erb_spec.rb @@ -1,23 +1,40 @@ require "rails_helper" RSpec.describe "notifications/_notification_row", type: :view do - let(:notification) do - build_stubbed(:notification, channel: "phone", email_subject: "Called them", + let(:hand_noted) do + build_stubbed(:notification, kind: "manual_log", channel: "phone", email_subject: "Called them", email_body_text: "Left a voicemail about the deadline") end + let(:autoemail) do + build_stubbed(:notification, kind: "event_registration_confirmation", channel: "autoemail", + email_subject: "Registration confirmed", email_body_text: "Hello Kim, your spot is booked") + end - it "shows the channel icon, subject, and body when show_body is true" do - render partial: "notifications/notification_row", locals: { notification: notification, show_body: true } + it "shows the channel icon and subject for a hand-noted communication" do + render partial: "notifications/notification_row", locals: { notification: hand_noted, admin: true } expect(rendered).to have_css("i.fa-solid.fa-phone") expect(rendered).to include("Called them") - expect(rendered).to include("Left a voicemail about the deadline") end - it "hides the body (admin-only content) when show_body is false" do - render partial: "notifications/notification_row", locals: { notification: notification, show_body: false } + it "shows a hand-noted body only to admins, with the admin-only blue styling" do + render partial: "notifications/notification_row", locals: { notification: hand_noted, admin: true } + + expect(rendered).to have_css("span.admin-only.bg-blue-100", text: "Left a voicemail about the deadline") + end + + it "hides a hand-noted body from non-admins (no inline text, nothing to hover)" do + render partial: "notifications/notification_row", locals: { notification: hand_noted, admin: false } expect(rendered).to include("Called them") expect(rendered).not_to include("Left a voicemail about the deadline") end + + it "shows an automated body to everyone, without admin styling, and in the hover title" do + render partial: "notifications/notification_row", locals: { notification: autoemail, admin: false } + + expect(rendered).to include("Hello Kim, your spot is booked") + expect(rendered).not_to have_css("span.admin-only") + expect(rendered).to have_css("span[title*='Hello Kim, your spot is booked']") + end end From 5b5a3a0346d0750c0ca0a39647fa8d608534777b Mon Sep 17 00:00:00 2001 From: maebeale Date: Wed, 8 Jul 2026 07:13:33 -0400 Subject: [PATCH 09/15] Add /communications alias redirecting to /notifications The feature is called "Communications" in the UI but the controller and routes stay :notifications; this friendly URL redirects there, preserving any filter query so /communications?email=x still works. Co-Authored-By: Claude Opus 4.8 (1M context) --- config/routes.rb | 3 +++ spec/requests/notifications_spec.rb | 14 ++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/config/routes.rb b/config/routes.rb index c8b30485a0..f0e21ea196 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -170,6 +170,9 @@ post :resend end end + # Friendly alias — the feature is called "Communications" in the UI, but the + # controller and routes stay :notifications. Redirect, preserving any filters. + get "communications", to: redirect { |_params, req| [ "/notifications", req.query_string.presence ].compact.join("?") }, as: :communications resources :organizations do collection do get :check_duplicates diff --git a/spec/requests/notifications_spec.rb b/spec/requests/notifications_spec.rb index be49bd7bc0..267423d4d1 100644 --- a/spec/requests/notifications_spec.rb +++ b/spec/requests/notifications_spec.rb @@ -5,6 +5,20 @@ let(:regular_user) { create(:user) } let(:notification) { create(:notification, recipient_email: regular_user.email) } + describe "GET /communications (friendly alias)" do + before { sign_in admin } + + it "redirects to /notifications" do + get "/communications" + expect(response).to redirect_to("/notifications") + end + + it "preserves filter params on the redirect" do + get "/communications", params: { email: "kim" } + expect(response).to redirect_to("/notifications?email=kim") + end + end + describe "GET /notifications" do before { sign_in admin } From f15a619ef89a96f5b30e7d8b86014f17f693d514 Mon Sep 17 00:00:00 2001 From: maebeale Date: Wed, 8 Jul 2026 07:13:33 -0400 Subject: [PATCH 10/15] Let admins edit any hand-noted communication, not just sender-attributed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Editing was gated on a sender being present, which left hand-noted email logs read-only. The real rule is simpler: any hand-noted communication (manual_log — email/phone/text/video) owned by the record is editable; autoemails (system messages) never are. manual_log? already excludes autoemails, so the sender requirement was just over-restricting. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../_notifications.html.erb | 5 +++-- app/views/people/_notifications.html.erb | 5 +++-- .../scholarships/_notifications.html.erb | 5 +++-- spec/requests/people_notifications_spec.rb | 19 ++++++++++--------- 4 files changed, 19 insertions(+), 15 deletions(-) diff --git a/app/views/event_registrations/_notifications.html.erb b/app/views/event_registrations/_notifications.html.erb index 2d08d217f8..551d10a0dd 100644 --- a/app/views/event_registrations/_notifications.html.erb +++ b/app/views/event_registrations/_notifications.html.erb @@ -9,8 +9,9 @@ <% notifications = email.present? ? Notification.email(email).order(created_at: :desc) : Notification.none %> <% admin = allowed_to?(:index?, with: NotificationPolicy) %> <% new_notifications = registration.notifications.select(&:new_record?) %> -<%# Only admins can edit; system ("AWBW Portal", sender-less) communications are view-only. %> -<% editable_notifications = admin ? notifications.select { |n| n.manual_log? && n.sender_id.present? && n.noticeable_type == EventRegistration.name && n.noticeable_id == registration.id } : [] %> +<%# Admins can edit any hand-noted communication owned by this record; autoemails + (system messages, not manual_log) are always view-only. %> +<% editable_notifications = admin ? notifications.select { |n| n.manual_log? && n.noticeable_type == EventRegistration.name && n.noticeable_id == registration.id } : [] %>
diff --git a/app/views/people/_notifications.html.erb b/app/views/people/_notifications.html.erb index 5ef03a90fc..758dfb2ecc 100644 --- a/app/views/people/_notifications.html.erb +++ b/app/views/people/_notifications.html.erb @@ -10,8 +10,9 @@ <% notifications = email.present? ? Notification.email(email).order(created_at: :desc) : Notification.none %> <% admin = allowed_to?(:index?, with: NotificationPolicy) %> <% new_notifications = person.notifications.select(&:new_record?) %> -<%# Only admins can edit; system ("AWBW Portal", sender-less) communications are view-only. %> -<% editable_notifications = admin ? notifications.select { |n| n.manual_log? && n.sender_id.present? && n.noticeable_type == Person.name && n.noticeable_id == person.id } : [] %> +<%# Admins can edit any hand-noted communication owned by this record; autoemails + (system messages, not manual_log) are always view-only. %> +<% editable_notifications = admin ? notifications.select { |n| n.manual_log? && n.noticeable_type == Person.name && n.noticeable_id == person.id } : [] %>
diff --git a/app/views/scholarships/_notifications.html.erb b/app/views/scholarships/_notifications.html.erb index 6dccd61eee..a4741c85f2 100644 --- a/app/views/scholarships/_notifications.html.erb +++ b/app/views/scholarships/_notifications.html.erb @@ -9,8 +9,9 @@ <% notifications = email.present? ? Notification.email(email).order(created_at: :desc) : Notification.none %> <% admin = allowed_to?(:index?, with: NotificationPolicy) %> <% new_notifications = scholarship.notifications.select(&:new_record?) %> -<%# Only admins can edit; system ("AWBW Portal", sender-less) communications are view-only. %> -<% editable_notifications = admin ? notifications.select { |n| n.manual_log? && n.sender_id.present? && n.noticeable_type == Scholarship.name && n.noticeable_id == scholarship.id } : [] %> +<%# Admins can edit any hand-noted communication owned by this record; autoemails + (system messages, not manual_log) are always view-only. %> +<% editable_notifications = admin ? notifications.select { |n| n.manual_log? && n.noticeable_type == Scholarship.name && n.noticeable_id == scholarship.id } : [] %>
diff --git a/spec/requests/people_notifications_spec.rb b/spec/requests/people_notifications_spec.rb index a2adbde3c1..923ad02eb7 100644 --- a/spec/requests/people_notifications_spec.rb +++ b/spec/requests/people_notifications_spec.rb @@ -77,25 +77,26 @@ expect(response.body).to include("Welcome aboard") end - it "makes a sender-attributed log editable but keeps a system (AWBW Portal) log view-only" do - editable = create(:notification, noticeable: person, sender: admin, - recipient_email: person.preferred_email, email_subject: "Called them", - kind: "manual_log", recipient_role: "person", notification_type: 0) - system_log = create(:notification, noticeable: person, sender: nil, + it "makes a hand-noted communication editable but keeps an autoemail view-only" do + hand_noted = create(:notification, noticeable: person, sender: admin, + recipient_email: person.preferred_email, email_subject: "Called them", + channel: "email", kind: "manual_log", recipient_role: "person", notification_type: 0) + autoemail = create(:notification, noticeable: person, sender: nil, recipient_email: person.preferred_email, email_subject: "Automated blast", - kind: "manual_log", recipient_role: "person", notification_type: 0) + channel: "autoemail", kind: "event_registration_confirmation", + recipient_role: "person", notification_type: 0) get edit_person_path(person) # Both are shown... expect(response.body).to include("Called them") expect(response.body).to include("Automated blast") - # ...but only the sender-attributed one gets an editable nested-attributes field. + # ...but only the hand-noted one gets an editable nested-attributes field. id_fields = Nokogiri::HTML(response.body) .css('input[name*="notifications_attributes"][name$="[id]"]') .map { |input| input["value"] } - expect(id_fields).to include(editable.id.to_s) - expect(id_fields).not_to include(system_log.id.to_s) + expect(id_fields).to include(hand_noted.id.to_s) + expect(id_fields).not_to include(autoemail.id.to_s) end end end From 57842565475bc5980954b62cb183502be6923618 Mon Sep 17 00:00:00 2001 From: maebeale Date: Wed, 8 Jul 2026 07:25:15 -0400 Subject: [PATCH 11/15] Keep the autoemail channel reserved for portal-sent messages autoemail isn't offered in the manual channel dropdown, but the channel column defaults to "autoemail", so a hand-logged communication created without an explicit channel (or a legacy one) would carry it. Coerce any manual_log with a blank/autoemail channel to "email" on validation, so a hand-noted communication is never autoemail and legacy ones stay editable. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/models/notification.rb | 9 +++++++++ spec/models/notification_spec.rb | 30 ++++++++++++++++++++++++++++++ spec/requests/scholarships_spec.rb | 2 +- 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/app/models/notification.rb b/app/models/notification.rb index a510936911..65e6d9c221 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -114,6 +114,11 @@ class Notification < ApplicationRecord # edit page) records a contact that already happened — fill in the sensible # defaults so it validates without going through the delivery pipeline. before_validation :apply_manual_log_defaults, on: :create, if: -> { kind.blank? } + # "autoemail" is reserved for messages the portal sends automatically — it can + # be neither selected in the manual form nor set on a hand-logged (manual_log) + # communication. Fall back to a manual channel so one is never autoemail (the + # channel column defaults to "autoemail", so this also covers a blank choice). + before_validation :force_manual_log_channel, if: :manual_log? def manual_log? kind == "manual_log" @@ -221,4 +226,8 @@ def apply_manual_log_defaults self.notification_type ||= 0 self.delivered_at ||= Time.current end + + def force_manual_log_channel + self.channel = "email" if channel.blank? || channel == "autoemail" + end end diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index 019dc0b714..972df8ee75 100644 --- a/spec/models/notification_spec.rb +++ b/spec/models/notification_spec.rb @@ -8,6 +8,36 @@ it { should have_many(:child_notifications).class_name('Notification').with_foreign_key(:parent_notification_id) } end + describe "manual log channel" do + def build_notification(**attrs) + build(:notification, recipient_role: "person", recipient_email: "x@example.com", notification_type: 0, **attrs) + end + + it "coerces a hand-logged (manual_log) autoemail channel to email" do + notification = build_notification(kind: "manual_log", channel: "autoemail") + notification.valid? + expect(notification.channel).to eq("email") + end + + it "defaults a channel-less hand-logged communication to email, not the autoemail column default" do + notification = build_notification(kind: "manual_log", channel: nil) + notification.valid? + expect(notification.channel).to eq("email") + end + + it "keeps a chosen manual channel" do + notification = build_notification(kind: "manual_log", channel: "phone") + notification.valid? + expect(notification.channel).to eq("phone") + end + + it "leaves autoemail on a portal-sent notification" do + notification = build_notification(kind: "event_registration_confirmation", channel: "autoemail") + notification.valid? + expect(notification.channel).to eq("autoemail") + end + end + describe "KINDS" do it "includes account_email_change_requested" do expect(Notification::KINDS).to include("account_email_change_requested") diff --git a/spec/requests/scholarships_spec.rb b/spec/requests/scholarships_spec.rb index 6315c6feb4..8bef2963a0 100644 --- a/spec/requests/scholarships_spec.rb +++ b/spec/requests/scholarships_spec.rb @@ -237,7 +237,7 @@ it "edits an existing logged notification in place" do note = create(:notification, noticeable: scholarship, recipient_email: scholarship.recipient.preferred_email.presence || "n/a", - email_subject: "Called recipient", kind: "manual_log", + email_subject: "Called recipient", channel: "email", kind: "manual_log", recipient_role: "person", notification_type: 0) patch scholarship_path(scholarship), From 7087b8468e5a8c5227550e47441053f10ee8e5f5 Mon Sep 17 00:00:00 2001 From: maebeale Date: Wed, 8 Jul 2026 07:25:15 -0400 Subject: [PATCH 12/15] Clarify communications form labels; hover only the body - Subject label reads "Subject (visible to user)", Body "Body (hidden from user)", so admins know what the recipient will and won't see. - Only the body carries the hover tooltip now (the subject no longer does). Co-Authored-By: Claude Opus 4.8 (1M context) --- app/views/notifications/_notification_form.html.erb | 4 ++-- app/views/notifications/_notification_row.html.erb | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/views/notifications/_notification_form.html.erb b/app/views/notifications/_notification_form.html.erb index 3dbdba8557..ff23375ebd 100644 --- a/app/views/notifications/_notification_form.html.erb +++ b/app/views/notifications/_notification_form.html.erb @@ -24,12 +24,12 @@ <%# Marked required (shows the asterisk) — a blank subject is dropped on save. field-required only enforces it once the row is in use, so an untouched added row still won't block the parent form. %> - <%= f.input :email_subject, label: "Subject", placeholder: "Topic or subject line", required: true, + <%= f.input :email_subject, label: "Subject (visible to user)", placeholder: "Topic or subject line", required: true, input_html: { rows: 1, data: { field_required_target: "field required" } }, wrapper_html: { class: "sm:col-span-3" } %>
- <%= f.input :email_body_text, as: :text, label: "Body", + <%= f.input :email_body_text, as: :text, label: "Body (hidden from user)", input_html: { rows: 3, placeholder: "What happened? (e.g. left voicemail, sent reminder)", data: { field_required_target: "field" } } %> diff --git a/app/views/notifications/_notification_row.html.erb b/app/views/notifications/_notification_row.html.erb index 1041af1201..d03c678f0c 100644 --- a/app/views/notifications/_notification_row.html.erb +++ b/app/views/notifications/_notification_row.html.erb @@ -17,15 +17,15 @@ <%# Fixed, snug width (~"Umberto User") + truncate so the channel icons line up regardless of name length, without a wide gap before the icon. %> <%= sender_name %> - "> + <%= notification.decorate.channel_icon %> <%= subject %> <% if body.present? %> <% if body_admin_only %> - <%# Internal staff note — admin-only, marked with the admin-only blue styling. %> - <%= body %> + <%# Internal staff note — admin-only, marked with the admin-only blue styling. Only the body is hoverable. %> + <%= body %> <% else %> - <%= body %> + <%= body %> <% end %> <% end %> From 938f10aadd4ec06657b1167b7f693fd19fb164c8 Mon Sep 17 00:00:00 2001 From: maebeale Date: Wed, 8 Jul 2026 07:27:48 -0400 Subject: [PATCH 13/15] Lay out the communication form like the comments box From and Channel now stack in a narrow column, with Subject and Body beside them as two-row textareas, so the manual-log form matches the comments box layout instead of a full-width stack. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../notifications/_notification_form.html.erb | 63 ++++++++++--------- 1 file changed, 33 insertions(+), 30 deletions(-) diff --git a/app/views/notifications/_notification_form.html.erb b/app/views/notifications/_notification_form.html.erb index ff23375ebd..2c70d936f8 100644 --- a/app/views/notifications/_notification_form.html.erb +++ b/app/views/notifications/_notification_form.html.erb @@ -1,38 +1,41 @@ <%# ---- The editable fields for a manual-log communication (From / Channel / Subject / Body + Remove). Shared by a new entry and the edit mode of a - persisted one in _notification_fields. Subject is required once the row is in - use (server-side reject_if drops a blank-subject entry), so field-required - marks it required as soon as the subject or body is touched — a partially - filled entry can't be silently dropped, while an untouched added row stays - optional and won't block the parent form. ---- %> + persisted one in _notification_fields. From and Channel stack in a narrow + column; Subject and Body sit beside them as two-row textareas (comments-box + layout). Subject is required once the row is in use (server-side reject_if + drops a blank-subject entry), so field-required marks it required as soon as + the subject or body is touched — a partially filled entry can't be silently + dropped, while an untouched added row stays optional and won't block the + parent form. ---- %>
-
- <%# No blank "AWBW Portal" option — a hand-logged communication must have a - real sender; system ("AWBW Portal") communications are view-only. %> - <%= f.input :sender_id, - collection: sender_options, - include_blank: false, - selected: sender_selected, - label: "From", - wrapper_html: { class: "sm:col-span-2" } %> - <%= f.input :channel, - collection: Notification::MANUAL_CHANNELS.map { |c| [ c.titleize, c ] }, - include_blank: false, - selected: channel_default, - label: "Channel", - wrapper_html: { class: "sm:col-span-1" } %> - <%# Marked required (shows the asterisk) — a blank subject is dropped on save. - field-required only enforces it once the row is in use, so an untouched - added row still won't block the parent form. %> - <%= f.input :email_subject, label: "Subject (visible to user)", placeholder: "Topic or subject line", required: true, - input_html: { rows: 1, data: { field_required_target: "field required" } }, - wrapper_html: { class: "sm:col-span-3" } %> +
+ <%# From and Channel stacked. No blank "AWBW Portal" option — a hand-logged + communication must have a real sender; system messages are view-only. %> +
+ <%= f.input :sender_id, + collection: sender_options, + include_blank: false, + selected: sender_selected, + label: "From" %> + <%= f.input :channel, + collection: Notification::MANUAL_CHANNELS.map { |c| [ c.titleize, c ] }, + include_blank: false, + selected: channel_default, + label: "Channel" %> +
+ <%# Subject marked required (shows the asterisk) — a blank subject is dropped + on save. field-required only enforces it once the row is in use, so an + untouched added row still won't block the parent form. %> + <%= f.input :email_subject, as: :text, label: "Subject (visible to user)", required: true, + input_html: { rows: 2, placeholder: "Topic or subject line", + data: { field_required_target: "field required" } }, + wrapper_html: { class: "flex-1" } %> + <%= f.input :email_body_text, as: :text, label: "Body (hidden from user)", + input_html: { rows: 2, placeholder: "What happened? (e.g. left voicemail, sent reminder)", + data: { field_required_target: "field" } }, + wrapper_html: { class: "flex-1" } %>
- <%= f.input :email_body_text, as: :text, label: "Body (hidden from user)", - input_html: { rows: 3, placeholder: "What happened? (e.g. left voicemail, sent reminder)", - data: { field_required_target: "field" } } %> -
<%= link_to_remove_association "Remove", f, class: "text-sm text-gray-400 underline hover:text-red-600" %> From ff58e9d9397580ff4933ea66993216ef885825c6 Mon Sep 17 00:00:00 2001 From: maebeale Date: Wed, 8 Jul 2026 07:35:57 -0400 Subject: [PATCH 14/15] Make From/Channel compact inline fields in the communication form From and Channel now render as "label + field" rows (comment-field styling, value truncates) stacked in the narrow column, so they fit the two-row height beside the Subject and Body textareas instead of taking a stacked label-above-field each. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../notifications/_notification_form.html.erb | 33 +++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/app/views/notifications/_notification_form.html.erb b/app/views/notifications/_notification_form.html.erb index 2c70d936f8..143d6102b2 100644 --- a/app/views/notifications/_notification_form.html.erb +++ b/app/views/notifications/_notification_form.html.erb @@ -1,27 +1,26 @@ <%# ---- The editable fields for a manual-log communication (From / Channel / Subject / Body + Remove). Shared by a new entry and the edit mode of a - persisted one in _notification_fields. From and Channel stack in a narrow - column; Subject and Body sit beside them as two-row textareas (comments-box - layout). Subject is required once the row is in use (server-side reject_if - drops a blank-subject entry), so field-required marks it required as soon as - the subject or body is touched — a partially filled entry can't be silently - dropped, while an untouched added row stays optional and won't block the - parent form. ---- %> + persisted one in _notification_fields. From and Channel stack as compact + inline "label + field" rows in a narrow column (comment-field styling, value + truncates); Subject and Body sit beside them as two-row textareas. Subject is + required once the row is in use (server-side reject_if drops a blank-subject + entry), so field-required marks it required as soon as the subject or body is + touched — a partially filled entry can't be silently dropped, while an + untouched added row stays optional and won't block the parent form. ---- %> +<% select_class = "min-w-0 flex-1 truncate bg-white rounded-md border border-gray-300 px-3 py-2 text-sm shadow-sm focus:border-blue-500 focus:ring focus:ring-blue-200 focus:outline-none" %>
<%# From and Channel stacked. No blank "AWBW Portal" option — a hand-logged communication must have a real sender; system messages are view-only. %>
- <%= f.input :sender_id, - collection: sender_options, - include_blank: false, - selected: sender_selected, - label: "From" %> - <%= f.input :channel, - collection: Notification::MANUAL_CHANNELS.map { |c| [ c.titleize, c ] }, - include_blank: false, - selected: channel_default, - label: "Channel" %> + +
<%# Subject marked required (shows the asterisk) — a blank subject is dropped on save. field-required only enforces it once the row is in use, so an From 5b0ddedde2644883b00de8a32809fc5c140dd6e4 Mon Sep 17 00:00:00 2001 From: maebeale Date: Wed, 8 Jul 2026 07:57:38 -0400 Subject: [PATCH 15/15] Show communication sender as a non-editable comment-style chip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "From" is no longer a dropdown — it renders the same colored author badge the comments box uses, fixed to whoever logs the communication (submitted via a hidden field). A spacer aligns the chip with the top of the Subject input. Drops the now-unused super-user sender query from the fields partial. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../_notification_fields.html.erb | 11 ++--- .../notifications/_notification_form.html.erb | 42 +++++++++++++------ 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/app/views/notifications/_notification_fields.html.erb b/app/views/notifications/_notification_fields.html.erb index 584813a10c..ffd46bf295 100644 --- a/app/views/notifications/_notification_fields.html.erb +++ b/app/views/notifications/_notification_fields.html.erb @@ -4,10 +4,7 @@ between them. A new, unsaved entry renders the amber add form directly. Only manually logged communications addressed to this record render through this partial — automated notifications stay read-only in the - notifications box. A blank sender means the system ("AWBW Portal"). ---- %> -<% sender_users = User.where(super_user: true).includes(:person).to_a %> -<% sender_users |= [ current_user ] if current_user %> -<% sender_options = sender_users.sort_by { |u| u.full_name.to_s.downcase }.map { |u| [ u.full_name, u.id ] } %> + notifications box. The sender defaults to whoever logs it. ---- %> <% channel_default = Notification::MANUAL_CHANNELS.include?(f.object.channel) ? f.object.channel : "email" %> <% record = local_assigns[:record] %>
@@ -24,13 +21,11 @@ <%= render "notifications/notification_row", notification: f.object, admin: true %>
<% else %>
- <%= render "notifications/notification_form", f: f, sender_options: sender_options, - sender_selected: f.object.sender_id || current_user&.id, channel_default: channel_default %> + <%= render "notifications/notification_form", f: f, channel_default: channel_default %>
<% end %>
diff --git a/app/views/notifications/_notification_form.html.erb b/app/views/notifications/_notification_form.html.erb index 143d6102b2..d09e63ea98 100644 --- a/app/views/notifications/_notification_form.html.erb +++ b/app/views/notifications/_notification_form.html.erb @@ -1,22 +1,40 @@ <%# ---- The editable fields for a manual-log communication (From / Channel / Subject / Body + Remove). Shared by a new entry and the edit mode of a - persisted one in _notification_fields. From and Channel stack as compact - inline "label + field" rows in a narrow column (comment-field styling, value - truncates); Subject and Body sit beside them as two-row textareas. Subject is - required once the row is in use (server-side reject_if drops a blank-subject - entry), so field-required marks it required as soon as the subject or body is - touched — a partially filled entry can't be silently dropped, while an - untouched added row stays optional and won't block the parent form. ---- %> + persisted one in _notification_fields. From is a non-editable sender chip + (same colored badge the comments box uses for the author); Channel is a + compact select; Subject and Body sit beside them as two-row textareas. + Subject is required once the row is in use (server-side reject_if drops a + blank-subject entry), so field-required marks it required as soon as the + subject or body is touched — a partially filled entry can't be silently + dropped, while an untouched added row stays optional and won't block the + parent form. ---- %> +<% label_colors = [ + [ "bg-sky-100", "text-sky-800" ], + [ "bg-purple-100", "text-purple-800" ], + [ "bg-pink-100", "text-pink-800" ], + [ "bg-indigo-100", "text-indigo-800" ], + [ "bg-orange-100", "text-orange-800" ], + [ "bg-yellow-200", "text-yellow-800" ], + [ "bg-blue-100", "text-blue-800" ], + [ "bg-green-100", "text-green-800" ] +] %> +<% sender_user = f.object.sender || current_user %> +<% sender_color = label_colors[(sender_user&.id || 0) % label_colors.length] %> +<% sender_first = sender_user&.person&.first_name || sender_user&.name.to_s.split.first %> <% select_class = "min-w-0 flex-1 truncate bg-white rounded-md border border-gray-300 px-3 py-2 text-sm shadow-sm focus:border-blue-500 focus:ring focus:ring-blue-200 focus:outline-none" %>
- <%# From and Channel stacked. No blank "AWBW Portal" option — a hand-logged - communication must have a real sender; system messages are view-only. %>
-