mirror of
https://github.com/bulwarkmail/native.git
synced 2026-10-31 08:06:36 +00:00
fix: notify new mail again when a push names Email but not EmailDelivery #34
On main every push ran the mail task, whose legacy path notifies the newest unread inbox mail. Device sync narrowed that to pushes naming EmailDelivery, so contact and calendar changes never notify. But a subscription made by an older build listens to Email and Mailbox until the app is opened again, and a server without EmailDelivery reports new mail only as those changes. Their new mail no longer showed a notification while the app was closed. Email and Mailbox now count as possible mail again, in the native push router and in the push task alike. A push that names only contact or calendar types still never takes the mail path. Checked with the push router and push task tests: an Email or Mailbox change starts the task and the legacy path notifies the message, next to the syncs a mixed map asks for.
This commit is contained in:
@@ -13,7 +13,7 @@ import org.json.JSONObject
|
||||
* The relay forwards the JMAP StateChange's `changed` map verbatim (FCM data
|
||||
* key `changed`: JMAP account id → type → state; only its first account is
|
||||
* repeated as `accountId`), so every account key is looked at. The mail task
|
||||
* runs only for mail: when some account's map has `EmailDelivery`, for an
|
||||
* runs only for mail: when some account's map has a mail type, for an
|
||||
* EmailPush, or when `changed` is missing or unreadable (older relays), so a
|
||||
* contact change never ends in a mail notification.
|
||||
*/
|
||||
@@ -28,7 +28,15 @@ object PushRouting {
|
||||
)
|
||||
|
||||
private const val EMAIL_PUSH_KIND = "jmap-email-push"
|
||||
private const val EMAIL_DELIVERY = "EmailDelivery"
|
||||
|
||||
/**
|
||||
* Types that may mean new mail: `EmailDelivery`, and `Email`/`Mailbox`,
|
||||
* which new mail changes too. A subscription made by an older build
|
||||
* listens to them until the launch-time resync replaces its types, and a
|
||||
* server without `EmailDelivery` has only them. Keep in sync with
|
||||
* MAIL_TYPES in src/lib/push-background-task.ts.
|
||||
*/
|
||||
private val MAIL_TYPES = setOf("EmailDelivery", "Email", "Mailbox")
|
||||
private val CONTACT_TYPES = setOf("ContactCard", "AddressBook")
|
||||
private val CALENDAR_TYPES = setOf("CalendarEvent", "Calendar")
|
||||
|
||||
@@ -42,7 +50,7 @@ object PushRouting {
|
||||
mail = true
|
||||
continue
|
||||
}
|
||||
if (EMAIL_DELIVERY in types) mail = true
|
||||
if (types.any { it in MAIL_TYPES }) mail = true
|
||||
val contacts = types.any { it in CONTACT_TYPES }
|
||||
val calendar = types.any { it in CALENDAR_TYPES }
|
||||
for (route in routes[jmapAccountId].orEmpty()) {
|
||||
|
||||
@@ -64,6 +64,26 @@ class PushRoutingTest {
|
||||
assertEquals(setOf("alice-work" to calendar), decision.syncs)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an Email or Mailbox change is possible mail, as a subscription from an older build sends it`() {
|
||||
for (changed in listOf(
|
||||
"""{"c":{"Email":"s1","Mailbox":"s2"}}""",
|
||||
"""{"c":{"Email":"s1"}}""",
|
||||
"""{"c":{"Mailbox":"s1"}}""",
|
||||
)) {
|
||||
val decision = PushRouting.decide(stateChange(changed), routes)
|
||||
assertTrue("changed=$changed", decision.startMailTask)
|
||||
assertTrue("changed=$changed", decision.syncs.isEmpty())
|
||||
}
|
||||
val mixed = PushRouting.decide(stateChange("""{"team":{"CalendarEvent":"e1"},"c":{"Email":"s1"}}"""), routes)
|
||||
assertTrue(mixed.startMailTask)
|
||||
assertEquals(setOf("alice-work" to calendar), mixed.syncs)
|
||||
// Types that are neither mail nor device sync say nothing about mail.
|
||||
val other = PushRouting.decide(stateChange("""{"c":{"Thread":"t1","ContactCard":"s1"}}"""), routes)
|
||||
assertFalse(other.startMailTask)
|
||||
assertEquals(setOf("alice@example.org" to contacts), other.syncs)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an email push always starts the mail task`() {
|
||||
val data = mapOf("kind" to "jmap-email-push", "accountId" to "c", "changed" to """{"c":{"EmailDelivery":"s"}}""")
|
||||
|
||||
@@ -237,6 +237,42 @@ describe('pushes for device sync (#34)', () => {
|
||||
await pushBackgroundTask(stateChange({ 'jmap-primary': { EmailDelivery: 'm1', ContactCard: 's1' } }));
|
||||
expect(jmapClient.getStoredCredentials).toHaveBeenCalledWith(LOCAL);
|
||||
});
|
||||
|
||||
it('takes an Email or Mailbox change for possible mail, as a subscription from an older build sends it', () => {
|
||||
expect(carriesNoMail(parseRelayPushData(stateChange({ a: { Email: 's1', Mailbox: 's2' } })))).toBe(false);
|
||||
expect(carriesNoMail(parseRelayPushData(stateChange({ a: { Email: 's1' } })))).toBe(false);
|
||||
expect(carriesNoMail(parseRelayPushData(stateChange({ a: { Mailbox: 's1' } })))).toBe(false);
|
||||
expect(carriesNoMail(parseRelayPushData(stateChange({ a: { CalendarEvent: 'e1' }, b: { Email: 's1' } })))).toBe(false);
|
||||
// Types that are neither mail nor device sync say nothing about mail.
|
||||
expect(carriesNoMail(parseRelayPushData(stateChange({ a: { Thread: 't1', ContactCard: 's1' } })))).toBe(true);
|
||||
});
|
||||
|
||||
it('notifies new mail from a subscription that listens to Email and Mailbox', async () => {
|
||||
(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': {} },
|
||||
}),
|
||||
};
|
||||
}
|
||||
// The legacy path: the inbox, its newest unread ids, then the messages.
|
||||
const [[name]] = JSON.parse(init?.body ?? '{}').methodCalls;
|
||||
const body = name === 'Mailbox/get'
|
||||
? { list: [{ id: 'inbox', role: 'inbox' }] }
|
||||
: name === 'Email/query'
|
||||
? { ids: ['m1'] }
|
||||
: { list: [{ id: 'm1', threadId: 't1', keywords: {}, subject: 'Hi', from: [{ email: 'bob@example.com' }] }] };
|
||||
return { ok: true, json: async () => ({ methodResponses: [[name, body, '0']] }) };
|
||||
});
|
||||
|
||||
await pushBackgroundTask(stateChange({ 'jmap-primary': { Email: 's1', Mailbox: 's2' } }));
|
||||
|
||||
expect(showNotification).toHaveBeenCalledWith(expect.objectContaining({ emailId: 'm1', accountId: LOCAL }));
|
||||
});
|
||||
});
|
||||
|
||||
describe('selectNotifiableEmails', () => {
|
||||
|
||||
@@ -378,18 +378,25 @@ async function detachedNewestUnreadInboxIds(
|
||||
return (qBody.ids as string[]) ?? [];
|
||||
}
|
||||
|
||||
// Types that may mean new mail: `EmailDelivery`, and `Email`/`Mailbox`, which
|
||||
// new mail changes too. A subscription made by an older build listens to them
|
||||
// until the launch-time resync replaces its types, and a server without
|
||||
// `EmailDelivery` has only them. Keep in sync with MAIL_TYPES in PushRouting.kt.
|
||||
const MAIL_TYPES = ['EmailDelivery', 'Email', 'Mailbox'];
|
||||
|
||||
/**
|
||||
* A StateChange push in which no account got new mail: only contact or
|
||||
* calendar changes, which the native push router already turned into device
|
||||
* syncs (#34, docs/device-sync.md "Triggers"). Without `EmailDelivery` there
|
||||
* is nothing to notify, and the legacy path (the newest unread mail) must
|
||||
* not run for it. An empty or missing `changed` map keeps the old behaviour.
|
||||
* syncs (#34, docs/device-sync.md "Triggers"). Without a mail type there is
|
||||
* nothing to notify, and the legacy path (the newest unread mail) must not
|
||||
* run for it. An empty or missing `changed` map keeps the old behaviour.
|
||||
*/
|
||||
export function carriesNoMail(payload: RelayPushData): boolean {
|
||||
if (payload.kind !== 'jmap-state-change' || !payload.changed) return false;
|
||||
const accounts = Object.values(payload.changed);
|
||||
if (accounts.length === 0) return false;
|
||||
return !accounts.some((types) => !!types && typeof types === 'object' && 'EmailDelivery' in types);
|
||||
return !accounts.some((types) => !!types && typeof types === 'object'
|
||||
&& MAIL_TYPES.some((type) => type in types));
|
||||
}
|
||||
|
||||
// Fired by BulwarkPushTaskService when a data FCM message arrives. Runs in a
|
||||
|
||||
Reference in New Issue
Block a user