feat: implement message deletion functionality and improve draft handling

- Added `MoveToTrash` method in `WebmailRepository` to move messages to the Trash folder instead of deleting them permanently. - Introduced `handleDelete` method in `mailAPI` to handle message deletion requests from the client. - Updated router to include a new route for message deletion. - Enhanced the compose page to manage draft integrity, ensuring drafts are correctly associated with sent messages. - Implemented UI changes to allow users to delete messages from the mail folder view. - Added tests to verify the correct behavior of message deletion and draft handling. - Updated database migration scripts to reconcile folder counters and ensure accurate unread counts. - Added tests for TLS-RPT report parsing and webmail repository functionalities.
2026-08-20 15:32:14 -03:00
1 parent 4f1b9c5217
commit 880dced9e4
@@ -22,7 +22,12 @@ type PersistInboundMessageInput struct {
 	FromDisplayName string
 	MessageIDHeader string
 	HasAttachments  bool
-	SPFResult       string
+	// AlreadySeen marks a copy the user themselves composed - their own Sent
+	// copy, or a draft. It is not unread mail, and counting it as such is why
+	// the Sent folder carried an unread badge that nothing could clear: no
+	// reader ever opens their own outgoing copy, so the flag was never set.
+	AlreadySeen bool
+	SPFResult   string
 	DKIMResult      string
 	DMARCResult     string
 }
@@ -2,6 +2,7 @@ package port

 import (
 	"context"
+	"errors"
 	"time"

 	"github.com/google/uuid"
@@ -63,4 +64,16 @@ type WebmailRepository interface {
 	MarkSeen(ctx context.Context, mailboxID, folderName string, uid uint32, seen bool) error
 	// Expunge soft-deletes one message, keeping the folder counters in step.
 	Expunge(ctx context.Context, mailboxID, folderName string, uid uint32) error
+	// MoveToTrash relocates one message into Trash and returns its new UID
+	// there. This is what deleting a message means everywhere but in Trash
+	// itself, where Expunge is the operation instead.
+	MoveToTrash(ctx context.Context, mailboxID, folderName string, uid uint32) (uint32, error)
 }
+
+// ErrNoTrashFolder reports a mailbox with nowhere to put deleted mail, rather
+// than the message being destroyed instead.
+var ErrNoTrashFolder = errors.New("webmail: this mailbox has no Trash folder")
+
+// ErrAlreadyInTrash tells the caller to expunge rather than move: a message
+// already in Trash has no further destination.
+var ErrAlreadyInTrash = errors.New("webmail: message is already in Trash")
@@ -121,9 +121,11 @@ func TestHtmlToPlainTextIsReadable(t *testing.T) {
 // --- draft storage -------------------------------------------------------

 type stubWebmailRepo struct {
-	mailboxID  string
-	expunged   []uint32
-	expungeErr error
+	mailboxID    string
+	expunged     []uint32
+	expungeErr   error
+	movedToTrash []uint32
+	moveErr      error
 }

 func (s *stubWebmailRepo) FindMailboxIDByAddress(context.Context, string) (string, error) {
@@ -143,6 +145,39 @@ func (s *stubWebmailRepo) Expunge(_ context.Context, _, _ string, uid uint32) er
 	s.expunged = append(s.expunged, uid)
 	return s.expungeErr
 }
+func (s *stubWebmailRepo) MoveToTrash(_ context.Context, _, _ string, uid uint32) (uint32, error) {
+	if s.moveErr != nil {
+		return 0, s.moveErr
+	}
+	s.movedToTrash = append(s.movedToTrash, uid)
+	return uid, nil
+}
+
+// newSendableUseCase wires the smallest use case that can actually complete a
+// Send: a real submission path over recording stubs.
+func newSendableUseCase(t *testing.T, mailboxID uuid.UUID, repo *stubWebmailRepo) *WebmailUseCase {
+	t.Helper()
+	return newSendableUseCaseWith(t, mailboxID, repo, &recordingMessageRepository{})
+}
+
+func newSendableUseCaseWith(
+	t *testing.T, mailboxID uuid.UUID, repo *stubWebmailRepo, messages *recordingMessageRepository,
+) *WebmailUseCase {
+	t.Helper()
+	account := &port.MailboxAuth{
+		ID: mailboxID, EmailAddress: "me@example.test",
+		DomainName: "example.test", MaxRecipientsPerHour: 100,
+	}
+	submission := NewProcessOutboundEmailUseCase(
+		nil, &countingOutboundRepo{}, &capturingBlobStorage{}, &recordingSigner{})
+	return (&WebmailUseCase{
+		repo:       repo,
+		auth:       &stubAuthRepo{account: account},
+		submission: submission,
+		localHost:  "mail.example.test",
+	}).WithLocalFiling(
+		&stubBlobStorage{ref: port.BlobRef{ID: uuid.New(), SHA256: "d", SizeBytes: 5}}, messages)
+}

 type stubAuthRepo struct{ account *port.MailboxAuth }

@@ -210,3 +245,130 @@ func TestDraftSavesWithoutRecipients(t *testing.T) {
 		t.Fatalf("a draft with no recipients must still save: %v", err)
 	}
 }
+
+// --- sending, and what it leaves behind ----------------------------------
+
+// The bug as the user hit it: write a message, send it, and the draft the
+// autosave had stored was still sitting in Drafts afterwards - a duplicate of
+// a message already on its way, that nothing on any screen could remove.
+func TestSendingDiscardsTheDraftItCameFrom(t *testing.T) {
+	mailboxID := uuid.New()
+	repo := &stubWebmailRepo{mailboxID: mailboxID.String()}
+	uc := newSendableUseCase(t, mailboxID, repo)
+
+	if err := uc.Send(context.Background(), ComposeInput{
+		From:     "me@example.test",
+		To:       []string{"you@example.test"},
+		Subject:  "Finished",
+		HTML:     "<p>done</p>",
+		DraftUID: 42,
+	}); err != nil {
+		t.Fatalf("send: %v", err)
+	}
+
+	if len(repo.expunged) != 1 || repo.expunged[0] != 42 {
+		t.Errorf("the draft was left in Drafts after sending: expunged %v, want [42]", repo.expunged)
+	}
+}
+
+// A message composed from scratch has no draft behind it, and must not try to
+// remove UID 0 - which is not a UID at all.
+func TestSendingWithoutADraftExpungesNothing(t *testing.T) {
+	mailboxID := uuid.New()
+	repo := &stubWebmailRepo{mailboxID: mailboxID.String()}
+	uc := newSendableUseCase(t, mailboxID, repo)
+
+	if err := uc.Send(context.Background(), ComposeInput{
+		From: "me@example.test", To: []string{"you@example.test"}, Text: "hi",
+	}); err != nil {
+		t.Fatalf("send: %v", err)
+	}
+	if len(repo.expunged) != 0 {
+		t.Errorf("nothing should have been expunged, got %v", repo.expunged)
+	}
+}
+
+// The Sent copy is a message the user wrote; it is not unread mail, and filing
+// it as unread is what left the Sent folder with a badge nothing could clear.
+func TestSentCopyIsFiledAsAlreadyRead(t *testing.T) {
+	mailboxID := uuid.New()
+	repo := &stubWebmailRepo{mailboxID: mailboxID.String()}
+	messages := &recordingMessageRepository{}
+	uc := newSendableUseCaseWith(t, mailboxID, repo, messages)
+
+	if err := uc.Send(context.Background(), ComposeInput{
+		From: "me@example.test", To: []string{"you@example.test"}, Text: "hi",
+	}); err != nil {
+		t.Fatalf("send: %v", err)
+	}
+
+	if len(messages.persisted) != 1 {
+		t.Fatalf("want one filed copy, got %d", len(messages.persisted))
+	}
+	if !messages.persisted[0].AlreadySeen {
+		t.Error("the Sent copy was filed as unread mail")
+	}
+}
+
+// Likewise a draft: nothing ever opens one, so if it is filed unread its
+// folder carries a badge forever.
+func TestDraftIsFiledAsAlreadyRead(t *testing.T) {
+	mailboxID := uuid.New()
+	messages := &recordingMessageRepository{}
+	uc := (&WebmailUseCase{
+		repo:      &stubWebmailRepo{mailboxID: mailboxID.String()},
+		auth:      &stubAuthRepo{account: &port.MailboxAuth{ID: mailboxID, EmailAddress: "me@example.test"}},
+		localHost: "mail.example.test",
+	}).WithLocalFiling(
+		&stubBlobStorage{ref: port.BlobRef{ID: uuid.New(), SHA256: "d", SizeBytes: 5}}, messages)
+
+	if _, err := uc.SaveDraft(context.Background(), ComposeInput{
+		From: "me@example.test", Subject: "half",
+	}, 0); err != nil {
+		t.Fatalf("save draft: %v", err)
+	}
+	if len(messages.persisted) != 1 || !messages.persisted[0].AlreadySeen {
+		t.Error("the draft was filed as unread mail")
+	}
+}
+
+// --- deleting ------------------------------------------------------------
+
+// Deleting from an ordinary folder moves the message to Trash.
+func TestDeleteMovesToTrash(t *testing.T) {
+	mailboxID := uuid.New()
+	repo := &stubWebmailRepo{mailboxID: mailboxID.String()}
+	uc := &WebmailUseCase{
+		repo:      repo,
+		auth:      &stubAuthRepo{account: &port.MailboxAuth{ID: mailboxID, EmailAddress: "me@example.test"}},
+		localHost: "mail.example.test",
+	}
+
+	if err := uc.Delete(context.Background(), "me@example.test", "inbox", 7); err != nil {
+		t.Fatalf("delete: %v", err)
+	}
+	if len(repo.movedToTrash) != 1 || repo.movedToTrash[0] != 7 {
+		t.Errorf("the message was not moved to Trash: %v", repo.movedToTrash)
+	}
+	if len(repo.expunged) != 0 {
+		t.Errorf("deleting from the inbox must not destroy the message: expunged %v", repo.expunged)
+	}
+}
+
+// Deleting from Trash is the second press, and that one really removes it.
+func TestDeleteFromTrashExpunges(t *testing.T) {
+	mailboxID := uuid.New()
+	repo := &stubWebmailRepo{mailboxID: mailboxID.String(), moveErr: port.ErrAlreadyInTrash}
+	uc := &WebmailUseCase{
+		repo:      repo,
+		auth:      &stubAuthRepo{account: &port.MailboxAuth{ID: mailboxID, EmailAddress: "me@example.test"}},
+		localHost: "mail.example.test",
+	}
+
+	if err := uc.Delete(context.Background(), "me@example.test", "trash", 3); err != nil {
+		t.Fatalf("delete from trash: %v", err)
+	}
+	if len(repo.expunged) != 1 || repo.expunged[0] != 3 {
+		t.Errorf("emptying from Trash did not remove the message: %v", repo.expunged)
+	}
+}
@@ -138,6 +138,13 @@ type ComposeInput struct {
 	// that cannot or will not render HTML still has something to show.
 	Text string
 	HTML string
+	// DraftUID is the autosaved draft this message grew out of, or 0 when it
+	// was composed without one ever being stored.
+	//
+	// Sending used to ignore it entirely, so a message that had been autosaved
+	// went out and its draft stayed in Drafts: a duplicate of mail already on
+	// its way, which no screen in the webmail could then delete.
+	DraftUID uint32
 }

 // ErrNoRecipients rejects a submission with nobody to deliver to, before it
@@ -185,9 +192,46 @@ func (uc *WebmailUseCase) Send(ctx context.Context, input ComposeInput) error {
 	if err := uc.fileLocalCopy(ctx, account.ID, account.EmailAddress, "Sent", payload); err != nil {
 		log.Printf("webmail: could not file the Sent copy for %s: %v", account.EmailAddress, err)
 	}
+
+	// The draft this message grew out of has been superseded by the real
+	// thing. Swallowed like the Sent copy above and for the same reason: the
+	// mail is already queued, and failing the request now would only invite a
+	// second copy to be sent.
+	if input.DraftUID != 0 {
+		if mailboxID, idErr := uc.mailboxID(ctx, account.EmailAddress); idErr == nil {
+			if err := uc.repo.Expunge(ctx, mailboxID, "Drafts", input.DraftUID); err != nil {
+				log.Printf("webmail: could not discard the sent draft %d: %v", input.DraftUID, err)
+			}
+		}
+	}
 	return nil
 }

+// Delete removes one message the way a mail client does: from any ordinary
+// folder it goes to Trash, and from Trash it is really expunged.
+//
+// The webmail had no delete at all. Expunge existed but was reachable only
+// when an autosave superseded a draft, so anything the user wanted rid of -
+// most visibly the draft left behind by a sent message - stayed put.
+func (uc *WebmailUseCase) Delete(ctx context.Context, address, folder string, uid uint32) error {
+	mailboxID, err := uc.mailboxID(ctx, address)
+	if err != nil {
+		return err
+	}
+
+	_, err = uc.repo.MoveToTrash(ctx, mailboxID, folder, uid)
+	switch {
+	case err == nil:
+		return nil
+	case errors.Is(err, port.ErrAlreadyInTrash), errors.Is(err, port.ErrNoTrashFolder):
+		// Already in Trash, or nowhere to move it to. Either way the second
+		// press is the one that removes it for good.
+		return uc.repo.Expunge(ctx, mailboxID, folder, uid)
+	default:
+		return err
+	}
+}
+
 // fileLocalCopy stores a message the user themselves composed into one of
 // their own folders. It reuses the inbound persistence path so the copy gets a
 // real IMAP UID and shows up over IMAP as well as in the webmail.
@@ -213,6 +257,7 @@ func (uc *WebmailUseCase) fileLocalCopy(
 		FromDisplayName:  headers.FromDisplayName,
 		MessageIDHeader:  headers.MessageID,
 		HasAttachments:   headers.HasAttachments,
+		AlreadySeen:      true,
 	}})
 	return err
 }
@@ -261,6 +306,7 @@ func (uc *WebmailUseCase) SaveDraft(ctx context.Context, input ComposeInput, rep
 		FromDisplayName:  headers.FromDisplayName,
 		MessageIDHeader:  headers.MessageID,
 		HasAttachments:   headers.HasAttachments,
+		AlreadySeen:      true,
 	}})
 	if err != nil {
 		return 0, err