Make logged communications editable inline, like comments#1942
Conversation
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) <noreply@anthropic.com>
| toggle() { | ||
| this.editing = !this.editing; | ||
|
|
||
| // When leaving edit mode, sync edit textareas into their truncated view. |
There was a problem hiding this comment.
🤖 From Claude: Generalized from comment-edit-toggle. The body-sync (updating the truncated view from the textarea on exit) is now opt-in via bodyClassValue — comments set it, communications leave it blank since their view row has no single truncated body span.
| <% notifications = email.present? ? Notification.email(email).order(created_at: :desc) : Notification.none %> | ||
| <section class="overflow-hidden rounded-xl border border-gray-200 bg-white shadow-sm"> | ||
| <% 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 } %> |
There was a problem hiding this comment.
🤖 From Claude: The list is matched by recipient email (broad), but only manual_log entries whose noticeable is this record can be edited via nested attributes — automated notifications and email-matched ones owned by other records stay read-only.
| 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? } |
There was a problem hiding this comment.
🤖 From Claude: allow_destroy: true (with :_destroy permitted in the controller) lets the inline "Remove" link delete a logged entry — the reject_if blank-subject guard is bypassed for destroys, matching how comments work.
|
|
||
| // When leaving edit mode, sync edit textareas into their truncated view. | ||
| if (!this.editing && this.bodyClassValue) { | ||
| this.element.querySelectorAll(".nested-fields").forEach((item) => { |
There was a problem hiding this comment.
This could be a good opportunity to update this controller to use stimulus targets vs. depending on class targets.
| type="button" | ||
| 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" | ||
| data-action="click->comment-edit-toggle#toggle" | ||
| data-action="click->edit-toggle#toggle" |
There was a problem hiding this comment.
| data-action="click->edit-toggle#toggle" | |
| data-action="edit-toggle#toggle" |
Click is implicit on buttons
| viewClass: { type: String, default: "editable-view" }, | ||
| editClass: { type: String, default: "editable-edit" }, | ||
| bodyClass: { type: String, default: "" }, |
There was a problem hiding this comment.
| viewClass: { type: String, default: "editable-view" }, | |
| editClass: { type: String, default: "editable-edit" }, | |
| bodyClass: { type: String, default: "" }, |
I believe these could all be done with stimulus targets instead of setting a class as a selector.
🤖 suggested review level: 3 Read 📖 contained inline-edit logic reusing the comment edit-toggle pattern across 3 edit pages
What is the goal of this PR and why is this important?
manual_logentries owned by the record are editable.How did you approach the change?
comment-edit-toggleStimulus controller into one reusableedit-toggle(view/edit CSS classes passed as values; comment body-sync kept as an optional, class-driven feature). Comments and communications now share it — no near-duplicate controller._notification_fieldsnow renders a read-only.notification-viewrow + hidden.notification-editform for persisted entries (mirrors_comment_fields); extracted_notification_row(shared read-only row) and_notification_form(shared editable fields).:id/:_destroyonnotifications_attributesand addedallow_destroy: trueso existing logs update/remove through the parent form.event_registration:instead ofrecord:(hiddennoticeablefields weren't rendering).Anything else to add?
manual_logentries are editable — the rest render read-only, so no page loses visibility of automated communications.