From 2fe83669383110b67cc78c81c152858126d2e4d4 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 31 Oct 2026 13:47:05 +0100 Subject: [PATCH] fix: let the notification Delete skip the unlock when the mail goes to Trash #85 --- .../__tests__/push-background-task.test.ts | 45 ++++++++++++++++--- src/lib/push-background-task.ts | 28 ++++++++++-- 2 files changed, 63 insertions(+), 10 deletions(-) diff --git a/src/lib/__tests__/push-background-task.test.ts b/src/lib/__tests__/push-background-task.test.ts index c90a53a6..6f3bf43e 100644 --- a/src/lib/__tests__/push-background-task.test.ts +++ b/src/lib/__tests__/push-background-task.test.ts @@ -218,6 +218,39 @@ describe('pushBackgroundTask notifications', () => { ); }); + // #85: Delete only moves to Trash, so with a Trash it needs no unlock and + // the shade stays open; without one the unlock stays. + it.each([ + ['without unlocking when the account has a Trash', [{ id: 'tr', role: 'trash' }], false], + ['behind the unlock when the account has no Trash', [{ id: 'in', role: 'inbox' }], true], + ])('offers Delete %s', async (_label, mailboxes, needsUnlock) => { + (secureFetch as ReturnType).mockImplementation(async (url: string, init?: { body?: string }) => { + if (url.endsWith('/.well-known/jmap')) { + return { + ok: true, + json: async () => ({ + apiUrl: 'https://mail.example.com/jmap/', + primaryAccounts: { 'urn:ietf:params:jmap:mail': 'jmap-primary' }, + accounts: { 'jmap-primary': {} }, + }), + }; + } + const calls = JSON.parse(init?.body ?? '{}').methodCalls as Array<[string, Record, string]>; + return { + ok: true, + json: async () => ({ + methodResponses: calls.map(([name, , id]) => (name === 'Mailbox/get' + ? [name, { list: mailboxes }, id] + : [name, { list: [{ id: 'm1', threadId: 't1', keywords: {}, subject: 'Hi', from: [{ email: 'bob@example.com' }] }] }, id])), + }), + }; + }); + + await pushBackgroundTask({ kind: 'jmap-email-push', accountLabel: 'alice', accountId: 'jmap-primary', emailIds: '["m1"]' }); + + expect(showNotification).toHaveBeenCalledWith(expect.objectContaining({ emailId: 'm1', deleteNeedsUnlock: needsUnlock })); + }); + it('shows a generic notification when the lookup fails, not none (#1041)', async () => { (secureFetch as ReturnType).mockImplementation(async (url: string) => { if (url.endsWith('/.well-known/jmap')) return { ok: false, status: 503 }; @@ -693,14 +726,14 @@ describe('pushBackgroundTask keeps the push subscription alive', () => { await recorded(2 * DAY); await push(); expect(showNotification).toHaveBeenCalled(); - expect(server.calls).toEqual(['Email/get', 'PushSubscription/get', 'PushSubscription/set', 'PushSubscription/get']); + expect(server.calls).toEqual(['Email/get', 'Mailbox/get', 'PushSubscription/get', 'PushSubscription/set', 'PushSubscription/get']); expect(await AsyncStorage.getItem(`push:subscriptionExpires:v1:${LOCAL}`)).toBe(server.expires); // Recorded now: the next pushes leave it alone for a day. server.calls = []; await AsyncStorage.multiRemove([`push:notifiedIds:v1:${LOCAL}`, `push:lastNotifiedEmailId:v2:${LOCAL}`]); await push(); - expect(server.calls).toEqual(['Email/get']); + expect(server.calls).toEqual(['Email/get', 'Mailbox/get']); }); it('extends it also when the push brings nothing new to show', async () => { @@ -721,7 +754,7 @@ describe('pushBackgroundTask keeps the push subscription alive', () => { await AsyncStorage.multiRemove([`push:notifiedIds:v1:${LOCAL}`, `push:lastNotifiedEmailId:v2:${LOCAL}`]); await setup(); await push(); - expect(server.calls).toEqual(['Email/get', 'PushSubscription/get', 'PushSubscription/set', 'PushSubscription/get']); + expect(server.calls).toEqual(['Email/get', 'Mailbox/get', 'PushSubscription/get', 'PushSubscription/set', 'PushSubscription/get']); expect(await AsyncStorage.getItem(`push:subscriptionExpires:v1:${LOCAL}`)).toBe(server.expires); // The server's copy has a week left: recorded, not written. @@ -730,7 +763,7 @@ describe('pushBackgroundTask keeps the push subscription alive', () => { await AsyncStorage.multiRemove([`push:notifiedIds:v1:${LOCAL}`, `push:lastNotifiedEmailId:v2:${LOCAL}`]); await setup(); await push(); - expect(server.calls).toEqual(['Email/get', 'PushSubscription/get']); + expect(server.calls).toEqual(['Email/get', 'Mailbox/get', 'PushSubscription/get']); expect(await AsyncStorage.getItem(`push:subscriptionExpires:v1:${LOCAL}`)).toBe(serverExpires); } }); @@ -748,7 +781,7 @@ describe('pushBackgroundTask keeps the push subscription alive', () => { await AsyncStorage.removeItem(`push:optedOut:v1:${LOCAL}`); await setup(); await push(); - expect(server.calls).toEqual(['Email/get']); + expect(server.calls).toEqual(['Email/get', 'Mailbox/get']); } }); @@ -756,7 +789,7 @@ describe('pushBackgroundTask keeps the push subscription alive', () => { await recorded(2 * DAY); server.deviceClientId = 'dcid-someone-else'; await push(); - expect(server.calls).toEqual(['Email/get', 'PushSubscription/get']); + expect(server.calls).toEqual(['Email/get', 'Mailbox/get', 'PushSubscription/get']); }); it('still notifies when the server cannot be asked about the subscription', async () => { diff --git a/src/lib/push-background-task.ts b/src/lib/push-background-task.ts index 86f22ea2..eeb5e238 100644 --- a/src/lib/push-background-task.ts +++ b/src/lib/push-background-task.ts @@ -128,6 +128,11 @@ interface ShowNotificationOptions { groupTitle: string; markReadLabel?: string; deleteLabel?: string; + // Whether Delete asks to unlock the phone first (#85). Delete only ever + // moves the message to Trash; with a Trash to move it to, nothing is lost + // and the action runs from the lock screen and the open shade. Without + // one, or when that is not known, the unlock stays. + deleteNeedsUnlock?: boolean; replyLabel?: string; } @@ -374,16 +379,29 @@ async function jmapPost( return body.methodResponses ?? []; } +/** + * The messages to notify, and whether their account has a Trash for the + * notification's Delete to move them to (asked in the same request). An + * answer without the folders counts as no Trash. + */ async function detachedGetEmails( session: DetachedSession, accountId: string, ids: string[], -): Promise { +): Promise<{ emails: Email[]; hasTrash: boolean }> { const responses = await jmapPost(session, [ ['Email/get', { accountId, ids, properties: EMAIL_PROPERTIES }, '0'], + ['Mailbox/get', { accountId, properties: ['id', 'role'] }, '1'], ]); const body = methodResult(responses, 'Email/get'); - return (body.list as Email[]) ?? []; + return { emails: (body.list as Email[]) ?? [], hasTrash: answerHasTrash(responses) }; +} + +/** Whether the Mailbox/get answer (call id '1') lists a Trash. */ +export function answerHasTrash(responses: Array<[string, Record, string]>): boolean { + const answer = responses.find(([name, , id]) => name === 'Mailbox/get' && id === '1'); + const list = answer?.[1]?.list; + return Array.isArray(list) && list.some((m: Mailbox) => m?.role === 'trash'); } async function detachedNewestUnreadInboxIds( @@ -543,6 +561,7 @@ async function processAccountForPush(accountId: string, payload: RelayPushData, async function notifyForPush(accountId: string, session: DetachedSession, payload: RelayPushData): Promise { const alreadyNotified = await readNotifiedIds(accountId); let candidates: Email[]; + let hasTrash = false; let emailAccountId = session.jmapAccountId; if (payload.emailIds.length > 0) { @@ -552,7 +571,7 @@ async function notifyForPush(accountId: string, session: DetachedSession, payloa const fresh = payload.emailIds.filter((id) => !alreadyNotified.includes(id)); if (fresh.length === 0) return; emailAccountId = payload.jmapAccountId ?? session.jmapAccountId; - candidates = await detachedGetEmails(session, emailAccountId, fresh); + ({ emails: candidates, hasTrash } = await detachedGetEmails(session, emailAccountId, fresh)); } else { // Legacy `jmap-state-change` payload without ids: look at the newest // unread inbox messages and notify the ones not shown before. The ring @@ -561,7 +580,7 @@ async function notifyForPush(accountId: string, session: DetachedSession, payloa const ids = await detachedNewestUnreadInboxIds(session, LEGACY_QUERY_LIMIT); const fresh = ids.filter((id) => !alreadyNotified.includes(id)); if (fresh.length === 0) return; - candidates = await detachedGetEmails(session, session.jmapAccountId, fresh); + ({ emails: candidates, hasTrash } = await detachedGetEmails(session, session.jmapAccountId, fresh)); } const toNotify = selectNotifiableEmails(candidates, alreadyNotified); @@ -613,6 +632,7 @@ async function notifyForPush(accountId: string, session: DetachedSession, payloa groupTitle, markReadLabel, deleteLabel, + deleteNeedsUnlock: !hasTrash, replyLabel, }); }