fix: let the notification Delete skip the unlock when the mail goes to Trash #85

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<typeof vi.fn>).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, unknown>, 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<typeof vi.fn>).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 () => {

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<Email[]> {
+): 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, any>, 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<void> {
   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,
     });
   }