Skip to content

Edit and soft-delete private notes#290

Open
mageaustralia wants to merge 2 commits intoabhinavxd:mainfrom
mageaustralia:feat/edit-delete-private-notes
Open

Edit and soft-delete private notes#290
mageaustralia wants to merge 2 commits intoabhinavxd:mainfrom
mageaustralia:feat/edit-delete-private-notes

Conversation

@mageaustralia
Copy link
Copy Markdown

Note: I only spotted #261 after this work was done — was searching for open issues requesting note edit/delete and missed the PR. Different scope and design choice from #261 (which is API-only hard-delete). Happy to align with that PR's author if maintainers prefer one approach. Posting this in case it's useful as a starting point.

Summary

  • PUT and DELETE /conversations/:cuuid/messages/:uuid/note (perm messages:write)
  • Authorisation: only the original author can edit/delete; the SQL also guards on private = true AND type = 'outgoing' so a public reply can't be touched accidentally
  • Edit replaces the body in place via a textarea on the bubble (deliberately simple — Tiptap-for-notes can be a follow-up)
  • Delete is a soft-delete: row stays, content becomes a tombstone naming the actor (This note was deleted by <Name>), and meta.deleted = true so the UI hides further edit/delete controls. Audit trail is preserved.

Reactivity fixes that the new flow exposed

  • MessageCache.updateMessage was Object.assign-ing in place, so consumers holding the message ref never saw the change. Now swaps the array entry to a new object so Vue picks up the identity change.
  • conversationMessages computed didn't depend on messages.version, so in-place cache updates couldn't trigger a re-render. Now reads the version.
  • <Letter> in MessageBubble doesn't react to its :html prop changing. Added :key="sanitizedContent" so vue-letter remounts on content change.

Test plan

  • Send a private note → Edit / Delete buttons appear under the bubble
  • Edit → textarea opens, save → bubble updates immediately (no refresh)
  • Delete → confirm dialog → bubble shows the tombstone, Edit/Delete disappear
  • Open the same conversation as a different agent → Edit/Delete are NOT shown on someone else's note
  • curl -XPUT a non-private message → 403
  • curl -XDELETE another agent's private note (with valid session) → 403
  • After delete, the row still exists in conversation_messages with meta -> 'deleted' = true

Adds PUT and DELETE for /conversations/:cuuid/messages/:uuid/note. Only
the agent who authored the note can mutate it. Delete is a soft-delete:
the row stays, content is replaced with a tombstone naming the actor,
and meta.deleted=true so the UI can hide further controls.

Also fixes three reactivity bugs the new flow exposed:

  - MessageCache.updateMessage was Object.assign-ing in place; consumers
    holding the message ref never saw the change. Now swaps the array
    entry to a new object so Vue picks up the identity change.

  - conversationMessages computed didn't depend on messages.version, so
    in-place cache updates couldn't trigger a re-render. Now reads the
    version.

  - <Letter> in MessageBubble doesn't react to its :html prop changing.
    Added :key="sanitizedContent" to force remount on content change.
// content with a tombstone naming the actor and setting `meta.deleted=true`.
// The row stays in the table so the audit trail is preserved.
func (m *Manager) SoftDeletePrivateNote(uuid, actorName string) error {
tombstone := fmt.Sprintf("This note was deleted by %s", actorName)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just show "Deleted" (translated) as the bubble content, without the actor name?

The bubble already shows the sender's name above it, and since an agent can only delete their own private notes, the deleter is always the same as the author.

Showing the name a second time is redundant.

emitter.emit(EMITTER_EVENTS.SHOW_TOAST, {
variant: 'destructive',
title: t('globals.messages.delete'),
description: err?.response?.data?.message || err.message
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's already a utility for this

import { handleHTTPError } from '@shared-ui/utils/http.js'

e.g. usage

    emitter.emit(EMITTER_EVENTS.SHOW_TOAST, {
      variant: 'destructive',
      description: handleHTTPError(err).message
    })

Comment thread cmd/messages.go Outdated
if err != nil {
return sendErrorEnvelope(r, err)
}
if msg.ConversationUUID != cuuid || !msg.Private || msg.SenderType != cmodels.SenderTypeAgent || msg.SenderID != user.ID {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ownership/private-note check is duplicated in handleUpdatePrivateNote and handleDeletePrivateNote. Worth pulling into a small helper so the rule lives in one place.

@abhinavxd
Copy link
Copy Markdown
Owner

Added a few comments. Once those are addressed we can get this merged.

- Replace ad-hoc `err?.response?.data?.message || err.message` toasts in
  MessageBubble.vue with the existing `handleHTTPError` utility from
  `@shared-ui/utils/http.js`. Same shape as the rest of the codebase.

- Extract the duplicated 'is the caller the author of a private note on
  this conversation' check from `handleUpdatePrivateNote` and
  `handleDeletePrivateNote` into a single `loadOwnedPrivateNote`
  helper. The ownership rule now lives in one place.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants