fix(webmail): Discard really discards the draft
The button was a link to the inbox, so the autosaved draft stayed in Drafts. It now removes the stored draft for good (not into Trash), stops the autosave, and cleans up a save that was already in flight. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
d283ff6f8d
commit
657a43d612
8 files changed
+132
-8
No files matched your search
@@ -402,6 +402,28 @@ func TestDeleteFromTrashExpunges(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// "Discard" in the composer promises the draft is gone. Delete would leave it
|
||||||
|
// in Trash, so discarding removes it outright - and only ever from Drafts.
|
||||||
|
func TestDiscardingADraftRemovesItForGood(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.DiscardDraft(context.Background(), "me@example.test", 9); err != nil {
|
||||||
|
t.Fatalf("discard: %v", err)
|
||||||
|
}
|
||||||
|
if len(repo.expunged) != 1 || repo.expunged[0] != 9 {
|
||||||
|
t.Errorf("the draft was not expunged: %v", repo.expunged)
|
||||||
|
}
|
||||||
|
if len(repo.moved) != 0 {
|
||||||
|
t.Errorf("a discarded draft was moved instead of removed: %v", repo.moved)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// --- moving between folders ---------------------------------------------
|
// --- moving between folders ---------------------------------------------
|
||||||
|
|
||||||
// Filing a message somewhere else is the one ordinary mail action the webmail
|
// Filing a message somewhere else is the one ordinary mail action the webmail
|
||||||
|
|||||||
@@ -283,6 +283,20 @@ func (uc *WebmailUseCase) Delete(ctx context.Context, address, folder string, ui
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// DiscardDraft removes a draft for good.
|
||||||
|
//
|
||||||
|
// Delete would move it to Trash, which is right for a message someone might
|
||||||
|
// want back and wrong for "Discard": the composer's button promises the draft
|
||||||
|
// is gone, and one waiting in Trash is a draft that is not. Only Drafts is
|
||||||
|
// reachable from here, so this cannot be used to expunge anything else.
|
||||||
|
func (uc *WebmailUseCase) DiscardDraft(ctx context.Context, address string, uid uint32) error {
|
||||||
|
mailboxID, err := uc.mailboxID(ctx, address)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
return uc.repo.Expunge(ctx, mailboxID, "Drafts", uid)
|
||||||
|
}
|
||||||
|
|
||||||
// SaveDraft files the message being composed into the Drafts folder.
|
// SaveDraft files the message being composed into the Drafts folder.
|
||||||
//
|
//
|
||||||
// Nothing did this before: the composer displayed "draft saved automatically"
|
// Nothing did this before: the composer displayed "draft saved automatically"
|
||||||
|
|||||||
@@ -300,6 +300,8 @@ func (m *mailAPI) handleDelete(w http.ResponseWriter, r *http.Request) {
|
|||||||
var body struct {
|
var body struct {
|
||||||
Folder string `json:"folder"`
|
Folder string `json:"folder"`
|
||||||
UID uint32 `json:"uid"`
|
UID uint32 `json:"uid"`
|
||||||
|
// DiscardDraft removes a draft for good instead of moving it to Trash.
|
||||||
|
DiscardDraft bool `json:"discard_draft"`
|
||||||
}
|
}
|
||||||
if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, 4096)).Decode(&body); err != nil {
|
if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, 4096)).Decode(&body); err != nil {
|
||||||
writeError(w, http.StatusBadRequest, "BAD_REQUEST", "Invalid JSON")
|
writeError(w, http.StatusBadRequest, "BAD_REQUEST", "Invalid JSON")
|
||||||
@@ -313,6 +315,15 @@ func (m *mailAPI) handleDelete(w http.ResponseWriter, r *http.Request) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if body.DiscardDraft {
|
||||||
|
if err := m.useCase.DiscardDraft(r.Context(), session.Email, body.UID); err != nil {
|
||||||
|
m.fail(w, err)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
writeJSON(w, http.StatusOK, map[string]bool{"deleted": true})
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
if err := m.useCase.Delete(r.Context(), session.Email, body.Folder, body.UID); err != nil {
|
if err := m.useCase.Delete(r.Context(), session.Email, body.Folder, body.UID); err != nil {
|
||||||
m.fail(w, err)
|
m.fail(w, err)
|
||||||
return
|
return
|
||||||
|
|||||||
@@ -329,7 +329,8 @@
|
|||||||
"trustSender": "Trust sender",
|
"trustSender": "Trust sender",
|
||||||
"trustedSender": "Trusted sender",
|
"trustedSender": "Trusted sender",
|
||||||
"trustSenderHint": "Never file this sender's mail as spam, and always show their images",
|
"trustSenderHint": "Never file this sender's mail as spam, and always show their images",
|
||||||
"trustedSenderHint": "Not filed as spam, images always shown. Click to stop trusting."
|
"trustedSenderHint": "Not filed as spam, images always shown. Click to stop trusting.",
|
||||||
|
"discardFailed": "The draft could not be discarded. Try again."
|
||||||
},
|
},
|
||||||
"settings": {
|
"settings": {
|
||||||
"actionDiscard": "Discard",
|
"actionDiscard": "Discard",
|
||||||
|
|||||||
@@ -329,7 +329,8 @@
|
|||||||
"trustSender": "Confiar en el remitente",
|
"trustSender": "Confiar en el remitente",
|
||||||
"trustedSender": "Remitente de confianza",
|
"trustedSender": "Remitente de confianza",
|
||||||
"trustSenderHint": "No enviar nunca el correo de este remitente a spam y mostrar siempre sus imágenes",
|
"trustSenderHint": "No enviar nunca el correo de este remitente a spam y mostrar siempre sus imágenes",
|
||||||
"trustedSenderHint": "No va a spam y sus imágenes siempre se muestran. Haz clic para dejar de confiar."
|
"trustedSenderHint": "No va a spam y sus imágenes siempre se muestran. Haz clic para dejar de confiar.",
|
||||||
|
"discardFailed": "No se pudo descartar el borrador. Inténtalo de nuevo."
|
||||||
},
|
},
|
||||||
"settings": {
|
"settings": {
|
||||||
"actionDiscard": "Descartar",
|
"actionDiscard": "Descartar",
|
||||||
|
|||||||
@@ -329,7 +329,8 @@
|
|||||||
"trustSender": "Confiar no remetente",
|
"trustSender": "Confiar no remetente",
|
||||||
"trustedSender": "Remetente confiável",
|
"trustedSender": "Remetente confiável",
|
||||||
"trustSenderHint": "Nunca mandar o e-mail deste remetente para o spam e sempre mostrar as imagens dele",
|
"trustSenderHint": "Nunca mandar o e-mail deste remetente para o spam e sempre mostrar as imagens dele",
|
||||||
"trustedSenderHint": "Não vai para o spam e as imagens sempre aparecem. Clique para deixar de confiar."
|
"trustedSenderHint": "Não vai para o spam e as imagens sempre aparecem. Clique para deixar de confiar.",
|
||||||
|
"discardFailed": "Não foi possível descartar o rascunho. Tente novamente."
|
||||||
},
|
},
|
||||||
"settings": {
|
"settings": {
|
||||||
"actionDiscard": "Descartar",
|
"actionDiscard": "Descartar",
|
||||||
|
|||||||
@@ -20,6 +20,7 @@ import {
|
|||||||
} from "lucide-react";
|
} from "lucide-react";
|
||||||
import { useTranslations } from "../../../../i18n/provider";
|
import { useTranslations } from "../../../../i18n/provider";
|
||||||
import { Button } from "../../../../components/ui/Button";
|
import { Button } from "../../../../components/ui/Button";
|
||||||
|
import { notifyMailStateChanged } from "../../../../lib/useFolders";
|
||||||
import { signatureToHtml } from "../../../../lib/signature";
|
import { signatureToHtml } from "../../../../lib/signature";
|
||||||
import { RecipientInput } from "../../../../components/RecipientInput";
|
import { RecipientInput } from "../../../../components/RecipientInput";
|
||||||
import { parseAddress, parseAddressList, prefixSubject } from "../../../../lib/recipients";
|
import { parseAddress, parseAddressList, prefixSubject } from "../../../../lib/recipients";
|
||||||
@@ -67,10 +68,15 @@ export default function ComposePage({
|
|||||||
const [sending, setSending] = useState(false);
|
const [sending, setSending] = useState(false);
|
||||||
const [undoSeconds, setUndoSeconds] = useState<number | null>(null);
|
const [undoSeconds, setUndoSeconds] = useState<number | null>(null);
|
||||||
const [draftStatus, setDraftStatus] = useState<string>("");
|
const [draftStatus, setDraftStatus] = useState<string>("");
|
||||||
|
const [discarding, setDiscarding] = useState(false);
|
||||||
const [sendError, setSendError] = useState<string | null>(null);
|
const [sendError, setSendError] = useState<string | null>(null);
|
||||||
// The UID of the draft already stored for this message. Sent back on each
|
// The UID of the draft already stored for this message. Sent back on each
|
||||||
// autosave so the server replaces it instead of leaving another copy.
|
// autosave so the server replaces it instead of leaving another copy.
|
||||||
const draftUidRef = useRef<number>(0);
|
const draftUidRef = useRef<number>(0);
|
||||||
|
// Set once the writer has discarded the message. An autosave already on its
|
||||||
|
// way when that happens would otherwise land afterwards and bring the draft
|
||||||
|
// back, so every save checks this before and after it talks to the server.
|
||||||
|
const discardedRef = useRef(false);
|
||||||
|
|
||||||
const editorRef = useRef<HTMLDivElement>(null);
|
const editorRef = useRef<HTMLDivElement>(null);
|
||||||
// The body, mirrored into React state.
|
// The body, mirrored into React state.
|
||||||
@@ -134,10 +140,27 @@ export default function ComposePage({
|
|||||||
};
|
};
|
||||||
}, [draftParam]);
|
}, [draftParam]);
|
||||||
|
|
||||||
|
// Removes a stored draft for good. discard_draft is what keeps it out of
|
||||||
|
// Trash: an ordinary delete moves it there, and a draft sitting in Trash is
|
||||||
|
// not discarded.
|
||||||
|
const removeDraft = async (uid: number): Promise<boolean> => {
|
||||||
|
try {
|
||||||
|
const res = await fetch("/api/v1/mail/delete", {
|
||||||
|
method: "POST",
|
||||||
|
headers: { "Content-Type": "application/json" },
|
||||||
|
body: JSON.stringify({ folder: "drafts", uid, discard_draft: true }),
|
||||||
|
});
|
||||||
|
return res.ok;
|
||||||
|
} catch {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
// Saves the draft for real. This used to be a setTimeout that set the words
|
// Saves the draft for real. This used to be a setTimeout that set the words
|
||||||
// "draft saved automatically" and nothing else - no request, no storage - so
|
// "draft saved automatically" and nothing else - no request, no storage - so
|
||||||
// closing the tab lost the message while the screen had just said otherwise.
|
// closing the tab lost the message while the screen had just said otherwise.
|
||||||
const saveDraft = useCallback(async () => {
|
const saveDraft = useCallback(async () => {
|
||||||
|
if (discardedRef.current) return;
|
||||||
const html = editorRef.current?.innerHTML ?? body;
|
const html = editorRef.current?.innerHTML ?? body;
|
||||||
if (to.length === 0 && !subject && !body.trim() && !html.trim()) return;
|
if (to.length === 0 && !subject && !body.trim() && !html.trim()) return;
|
||||||
setDraftStatus(t("mail.draftSaving"));
|
setDraftStatus(t("mail.draftSaving"));
|
||||||
@@ -159,6 +182,12 @@ export default function ComposePage({
|
|||||||
if (res.ok) {
|
if (res.ok) {
|
||||||
const data = await res.json().catch(() => ({}));
|
const data = await res.json().catch(() => ({}));
|
||||||
if (typeof data.uid === "number") draftUidRef.current = data.uid;
|
if (typeof data.uid === "number") draftUidRef.current = data.uid;
|
||||||
|
// Discarded while this save was in flight: what it just stored is a
|
||||||
|
// draft nobody wants, so it goes too.
|
||||||
|
if (discardedRef.current && draftUidRef.current > 0) {
|
||||||
|
void removeDraft(draftUidRef.current);
|
||||||
|
return;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
setDraftStatus(res.ok ? t("mail.draftSaved") : t("mail.draftSaveFailed"));
|
setDraftStatus(res.ok ? t("mail.draftSaved") : t("mail.draftSaveFailed"));
|
||||||
} catch {
|
} catch {
|
||||||
@@ -178,6 +207,28 @@ export default function ComposePage({
|
|||||||
return () => clearTimeout(timer);
|
return () => clearTimeout(timer);
|
||||||
}, [to, subject, cc, bcc, body, important, sending, saveDraft]);
|
}, [to, subject, cc, bcc, body, important, sending, saveDraft]);
|
||||||
|
|
||||||
|
// Throws the message away: stops the autosave, removes the draft already
|
||||||
|
// stored for it, and only then leaves. Leaving first was the old behaviour -
|
||||||
|
// a link to the inbox - and left the draft in Drafts, as if nothing had
|
||||||
|
// been discarded.
|
||||||
|
const discardMessage = async () => {
|
||||||
|
discardedRef.current = true;
|
||||||
|
setDiscarding(true);
|
||||||
|
setSendError(null);
|
||||||
|
const uid = draftUidRef.current;
|
||||||
|
if (uid > 0 && !(await removeDraft(uid))) {
|
||||||
|
// Still there, so say so and let the autosave carry on rather than
|
||||||
|
// leaving the page with a draft the writer thinks is gone.
|
||||||
|
discardedRef.current = false;
|
||||||
|
setDiscarding(false);
|
||||||
|
setSendError(t("mail.discardFailed"));
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
draftUidRef.current = 0;
|
||||||
|
notifyMailStateChanged();
|
||||||
|
window.location.href = "/user/mail/inbox";
|
||||||
|
};
|
||||||
|
|
||||||
const executeFormat = (command: string, value: string | undefined = undefined) => {
|
const executeFormat = (command: string, value: string | undefined = undefined) => {
|
||||||
document.execCommand(command, false, value);
|
document.execCommand(command, false, value);
|
||||||
editorRef.current?.focus();
|
editorRef.current?.focus();
|
||||||
@@ -310,11 +361,15 @@ export default function ComposePage({
|
|||||||
<Flag className="h-4 w-4" fill={important ? "currentColor" : "none"} />
|
<Flag className="h-4 w-4" fill={important ? "currentColor" : "none"} />
|
||||||
<span>{t("mail.highImportance")}</span>
|
<span>{t("mail.highImportance")}</span>
|
||||||
</button>
|
</button>
|
||||||
<Link href="/user/mail/inbox">
|
<Button
|
||||||
<Button variant="secondary" size="md">
|
type="button"
|
||||||
{t("mail.discard")}
|
variant="secondary"
|
||||||
</Button>
|
size="md"
|
||||||
</Link>
|
disabled={sending || discarding}
|
||||||
|
onClick={() => void discardMessage()}
|
||||||
|
>
|
||||||
|
{t("mail.discard")}
|
||||||
|
</Button>
|
||||||
<Button type="submit" variant="primary" size="md" disabled={sending}>
|
<Button type="submit" variant="primary" size="md" disabled={sending}>
|
||||||
<Send className="h-4 w-4" />
|
<Send className="h-4 w-4" />
|
||||||
<span>{sending ? t("mail.sending") : t("mail.sendMessage")}</span>
|
<span>{sending ? t("mail.sending") : t("mail.sendMessage")}</span>
|
||||||
|
|||||||
@@ -100,3 +100,22 @@ describe("the Reports folder", () => {
|
|||||||
expect(page).toMatch(/reports:\s*t\("mail\.reports"\)/);
|
expect(page).toMatch(/reports:\s*t\("mail\.reports"\)/);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("discarding a draft", () => {
|
||||||
|
// "Discard" was a link to the inbox. The autosaved draft stayed in Drafts, so
|
||||||
|
// a message the writer had thrown away came back the next time they looked.
|
||||||
|
it("is a button that removes the stored draft, not a link away", () => {
|
||||||
|
const compose = code(COMPOSE);
|
||||||
|
expect(compose).not.toMatch(/<Link href="\/user\/mail\/inbox">\s*<Button[^>]*>\s*\{t\("mail\.discard"\)\}/);
|
||||||
|
expect(compose).toMatch(/discard_draft:\s*true/);
|
||||||
|
expect(compose).toMatch(/onClick=\{\(\) => void discardMessage\(\)\}/);
|
||||||
|
});
|
||||||
|
|
||||||
|
// An autosave already in flight lands after the discard and puts the draft
|
||||||
|
// back, so the save has to know it was discarded.
|
||||||
|
it("stops the autosave and cleans up a save that was already on its way", () => {
|
||||||
|
const compose = code(COMPOSE);
|
||||||
|
expect(compose).toMatch(/if \(discardedRef\.current\) return;/);
|
||||||
|
expect(compose).toMatch(/discardedRef\.current && draftUidRef\.current > 0/);
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in new issue
Block a user