Skip to content

Commit c4ce616

Browse files
NickOvtNickOvt
authored
fix: fix hasDrafts for non collapsed threads message listing view (#1141)
* fix hasDrafts for non collapsed threads message listing view * fix logic for imap appended drafts too * update ncurc
1 parent c74f56a commit c4ce616

3 files changed

Lines changed: 137 additions & 15 deletions

File tree

‎.ncurc.js‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@ module.exports = {
1919
'openpgp',
2020

2121
// esm only since v18, breaks the CommonJS CLI scripts in bin/
22-
'yargs'
22+
'yargs',
23+
24+
// new major upgrade requires rewrite
25+
'mongo-cursor-pagination',
26+
'accesscontrol',
27+
'ioredis'
2328
]
2429
};

‎lib/api/messages.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,10 @@ module.exports = (db, server, messageHandler, userHandler, storageHandler, setti
134134
}
135135

136136
if (includeHasDrafts && matchDraftReferences) {
137+
// References contains the entire thread ancestry. In-Reply-To identifies the direct parent for both API and IMAP drafts.
137138
group.draftReferences = {
138139
$addToSet: {
139-
$cond: ['$draft', '$mimeTree.parsedHeader.references', false]
140+
$cond: ['$draft', '$mimeTree.parsedHeader.in-reply-to', false]
140141
}
141142
};
142143
}
@@ -172,7 +173,8 @@ module.exports = (db, server, messageHandler, userHandler, storageHandler, setti
172173
if (matchDraftReferences) {
173174
const draftReferences = new Set(
174175
((matchingThreadCount && matchingThreadCount.draftReferences) || [])
175-
.flatMap(references => (references || '').toString().split(/\s+/))
176+
.flatMap(reference => [].concat(reference || []))
177+
.flatMap(reference => reference.toString().split(/\s+/))
176178
.filter(reference => reference)
177179
);
178180

‎test/api/messages-test.js‎

Lines changed: 127 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ const expect = chai.expect;
1010
chai.config.includeStack = true;
1111
const config = require('@zone-eu/wild-config');
1212
const { ObjectId } = require('mongodb');
13+
const { ImapFlow } = require('imapflow');
1314
const { parseSearchQuery, getMongoDBQuery } = require('../../lib/search-query');
1415

1516
const server = supertest.agent(`http://127.0.0.1:${config.api.port}`);
@@ -259,6 +260,7 @@ describe('Messages tests', function () {
259260
let queryAttachmentMessageId;
260261
let queryFlaggedSeenAttachmentMessageId;
261262
let queryAltMailboxMessageId;
263+
let testUsername;
262264
let testAddress;
263265

264266
const queryFixture = {
@@ -335,7 +337,7 @@ describe('Messages tests', function () {
335337

336338
before(async () => {
337339
const testUserTag = Date.now().toString(36);
338-
const testUsername = `messagestestsuser-${testUserTag}`;
340+
testUsername = `messagestestsuser-${testUserTag}`;
339341
testAddress = `${testUsername}@web.zone.test`;
340342
queryFixture.fromAddress = testAddress;
341343

@@ -923,7 +925,7 @@ describe('Messages tests', function () {
923925
expect(search5.body.results).to.deep.eq(search.body.results); // Check if page 1 is equal to original page 1 after moving back from page 2
924926
});
925927

926-
it('should GET /users/:user/search expect success / collapseThreads controls the hasDrafts scope', async () => {
928+
it('should GET /users/:user/search expect success / collapseThreads controls hasDrafts scope and non-collapsed results match the exact draft reference', async () => {
927929
const mailboxResponse = await server
928930
.post(`/users/${user}/mailboxes`)
929931
.send({ path: `/search-collapse-threads-${Date.now().toString(36)}`, hidden: false, retention: 10000 })
@@ -954,6 +956,23 @@ describe('Messages tests', function () {
954956
})
955957
.expect(200);
956958

959+
// Keep the root in the References ancestry, but make only this reply the target of the next draft.
960+
await server.put(`/users/${user}/mailboxes/${mailbox}/messages/${reply.body.message.id}`).send({ draft: false }).expect(200);
961+
962+
const draftReply = await server
963+
.post(`/users/${user}/mailboxes/${mailbox}/messages`)
964+
.send({
965+
draft: true,
966+
to: [{ address: 'search-collapse@example.com' }],
967+
text: 'Draft reply to reply',
968+
reference: {
969+
mailbox,
970+
id: reply.body.message.id,
971+
action: 'reply'
972+
}
973+
})
974+
.expect(200);
975+
957976
const single = await server
958977
.post(`/users/${user}/mailboxes/${mailbox}/messages`)
959978
.send({
@@ -973,31 +992,36 @@ describe('Messages tests', function () {
973992
.send({})
974993
.expect(200);
975994

976-
expect(expandedPage.body.total).to.equal(2);
977-
expect(expandedPage.body.results.map(entry => entry.id)).to.deep.equal([reply.body.message.id]);
978-
expect(expandedPage.body.results[0].threadMessageCount).to.equal(2);
995+
expect(expandedPage.body.total).to.equal(3);
996+
expect(expandedPage.body.results.map(entry => entry.id)).to.deep.equal([draftReply.body.message.id]);
997+
expect(expandedPage.body.results[0].threadMessageCount).to.equal(3);
979998
expect(expandedPage.body.results[0]).to.not.have.property('hasDrafts');
980999

9811000
const expandedThread = await server
982-
.get(`/users/${user}/search?thread=${thread}&includeHasDrafts=true&limit=2`)
1001+
.get(`/users/${user}/search?thread=${thread}&includeHasDrafts=true&limit=3`)
9831002
.send({})
9841003
.expect(200);
9851004

986-
expect(expandedThread.body.results.map(entry => entry.id)).to.deep.equal([reply.body.message.id, root.body.message.id]);
987-
expect(expandedThread.body.results.map(entry => entry.hasDrafts)).to.deep.equal([false, true]);
1005+
expect(expandedThread.body.results.map(entry => entry.id)).to.deep.equal([
1006+
draftReply.body.message.id,
1007+
reply.body.message.id,
1008+
root.body.message.id
1009+
]);
1010+
expect(expandedThread.body.results.map(entry => entry.hasDrafts)).to.deep.equal([false, true, false]);
9881011
expect(expandedThread.body.results[0]).to.not.have.property('threadMessageCount');
9891012

9901013
const expandedMailboxPage = await server
991-
.get(`/users/${user}/mailboxes/${mailbox}/messages?includeHasDrafts=true&limit=3&order=desc`)
1014+
.get(`/users/${user}/mailboxes/${mailbox}/messages?includeHasDrafts=true&limit=4&order=desc`)
9921015
.send({})
9931016
.expect(200);
9941017

9951018
expect(expandedMailboxPage.body.results.map(entry => entry.id)).to.deep.equal([
9961019
single.body.message.id,
1020+
draftReply.body.message.id,
9971021
reply.body.message.id,
9981022
root.body.message.id
9991023
]);
1000-
expect(expandedMailboxPage.body.results.map(entry => entry.hasDrafts)).to.deep.equal([false, false, true]);
1024+
expect(expandedMailboxPage.body.results.map(entry => entry.hasDrafts)).to.deep.equal([false, false, true, false]);
10011025
expect(expandedMailboxPage.body.results[0]).to.not.have.property('threadMessageCount');
10021026

10031027
const collapsedPage1 = await server
@@ -1028,8 +1052,8 @@ describe('Messages tests', function () {
10281052
.send({})
10291053
.expect(200);
10301054

1031-
expect(collapsedPage2.body.results.map(entry => entry.id)).to.deep.equal([reply.body.message.id]);
1032-
expect(collapsedPage2.body.results[0].threadMessageCount).to.equal(2);
1055+
expect(collapsedPage2.body.results.map(entry => entry.id)).to.deep.equal([draftReply.body.message.id]);
1056+
expect(collapsedPage2.body.results[0].threadMessageCount).to.equal(3);
10331057
expect(collapsedPage2.body.results[0].hasDrafts).to.be.true;
10341058
expect(collapsedPage2.body.previousCursor).to.be.a('string');
10351059
expect(collapsedPage2.body.nextCursor).to.be.false;
@@ -1043,6 +1067,97 @@ describe('Messages tests', function () {
10431067
expect(archivedRoot).to.not.have.property('hasDrafts');
10441068
});
10451069

1070+
it('should GET /users/:user/search expect success / IMAP APPEND draft hasDrafts matches only the direct parent', async () => {
1071+
const mailboxPath = `imap-draft-reference-${Date.now().toString(36)}`;
1072+
const mailboxResponse = await server
1073+
.post(`/users/${user}/mailboxes`)
1074+
.send({ path: `/${mailboxPath}`, hidden: false, retention: 10000 })
1075+
.expect(200);
1076+
const mailbox = mailboxResponse.body.id;
1077+
1078+
const root = await server
1079+
.post(`/users/${user}/mailboxes/${mailbox}/messages`)
1080+
.send({
1081+
to: [{ address: 'imap-draft@example.com' }],
1082+
subject: 'IMAP Draft Thread',
1083+
text: 'Root message'
1084+
})
1085+
.expect(200);
1086+
1087+
const reply = await server
1088+
.post(`/users/${user}/mailboxes/${mailbox}/messages`)
1089+
.send({
1090+
to: [{ address: 'imap-draft@example.com' }],
1091+
text: 'Intermediate reply',
1092+
reference: {
1093+
mailbox,
1094+
id: root.body.message.id,
1095+
action: 'reply'
1096+
}
1097+
})
1098+
.expect(200);
1099+
1100+
await server.put(`/users/${user}/mailboxes/${mailbox}/messages/${reply.body.message.id}`).send({ draft: false }).expect(200);
1101+
1102+
const rootData = await server.get(`/users/${user}/mailboxes/${mailbox}/messages/${root.body.message.id}`).send({}).expect(200);
1103+
const replyData = await server.get(`/users/${user}/mailboxes/${mailbox}/messages/${reply.body.message.id}`).send({}).expect(200);
1104+
const rawDraft = [
1105+
`From: ${testAddress}`,
1106+
'To: imap-draft@example.com',
1107+
'Subject: Re: IMAP Draft Thread',
1108+
`Message-ID: <imap-draft-${Date.now().toString(36)}@web.zone.test>`,
1109+
`In-Reply-To: ${replyData.body.messageId}`,
1110+
`References: ${rootData.body.messageId} ${replyData.body.messageId}`,
1111+
'MIME-Version: 1.0',
1112+
'Content-Type: text/plain; charset=utf-8',
1113+
'',
1114+
'Draft uploaded with IMAP APPEND'
1115+
].join('\r\n');
1116+
1117+
const client = new ImapFlow({
1118+
host: '127.0.0.1',
1119+
port: config.imap.port,
1120+
secure: true,
1121+
auth: {
1122+
user: testUsername,
1123+
pass: 'secretpassword'
1124+
},
1125+
tls: {
1126+
rejectUnauthorized: false
1127+
},
1128+
logger: false
1129+
});
1130+
1131+
let appendResult;
1132+
try {
1133+
await client.connect();
1134+
appendResult = await client.append(mailboxPath, rawDraft, ['\\Draft']);
1135+
} finally {
1136+
if (client.usable) {
1137+
await client.logout();
1138+
} else {
1139+
client.close();
1140+
}
1141+
}
1142+
1143+
expect(appendResult.uid).to.be.a('number');
1144+
1145+
const appendedDraft = await server.get(`/users/${user}/mailboxes/${mailbox}/messages/${appendResult.uid}`).send({}).expect(200);
1146+
expect(appendedDraft.body).to.not.have.property('reference');
1147+
1148+
const expandedThread = await server
1149+
.get(`/users/${user}/search?thread=${rootData.body.thread}&includeHasDrafts=true&limit=3`)
1150+
.send({})
1151+
.expect(200);
1152+
1153+
expect(expandedThread.body.results.map(entry => entry.id)).to.deep.equal([
1154+
appendResult.uid,
1155+
reply.body.message.id,
1156+
root.body.message.id
1157+
]);
1158+
expect(expandedThread.body.results.map(entry => entry.hasDrafts)).to.deep.equal([false, true, false]);
1159+
});
1160+
10461161
it('should GET /users/:user/search expect success / q supports subject and in keywords', async () => {
10471162
const q = `subject:"${queryFixture.subjectKeyword}" in:${queryMailbox}`;
10481163
const search = await searchQ(q);

0 commit comments

Comments
 (0)