mirror of
https://github.com/outline/outline.git
synced 2026-06-13 03:14:59 +03:00
fix: documents.list with Draft status filter throws database error (#12426)
* fix: documents.list with Draft status filter throws database error The count() query referenced $memberships.id$ in WHERE but had no membership include, causing "missing FROM-clause entry for table memberships". The findAll path was also silently dropping drafts because withMembershipScope defaulted to defaultScope (which filters publishedAt != null). Pre-fetch the user's UserMembership document IDs and filter by id IN (...) on both find and count, and pass includeDrafts: true when the Draft filter is active. * Preserve template/trial filters when including drafts * Move template/trial filters into withDrafts scope * Revert withDrafts scope filters, apply at call site instead Adding template/trial filters to withDrafts broke includes in places like Share's withCollectionPermissions where the document include must remain optional (LEFT JOIN) — adding a where promoted it to INNER JOIN and dropped shares without a documentId.
This commit is contained in:
@@ -707,6 +707,49 @@ describe("#documents.list", () => {
|
||||
expect(body.data.length).toEqual(0);
|
||||
});
|
||||
|
||||
it("should return draft documents when filtering with statusFilter", async () => {
|
||||
const user = await buildUser();
|
||||
const document = await buildDraftDocument({
|
||||
userId: user.id,
|
||||
teamId: user.teamId,
|
||||
});
|
||||
const res = await server.post("/api/documents.list", user, {
|
||||
body: {
|
||||
statusFilter: [StatusFilter.Draft],
|
||||
},
|
||||
});
|
||||
const body = await res.json();
|
||||
expect(res.status).toEqual(200);
|
||||
expect(body.data.length).toEqual(1);
|
||||
expect(body.data[0].id).toEqual(document.id);
|
||||
expect(body.pagination.total).toEqual(1);
|
||||
});
|
||||
|
||||
it("should return drafts shared directly with the user", async () => {
|
||||
const user = await buildUser();
|
||||
const author = await buildUser({ teamId: user.teamId });
|
||||
const document = await buildDraftDocument({
|
||||
userId: author.id,
|
||||
teamId: user.teamId,
|
||||
});
|
||||
await UserMembership.create({
|
||||
documentId: document.id,
|
||||
userId: user.id,
|
||||
createdById: author.id,
|
||||
permission: DocumentPermission.Read,
|
||||
});
|
||||
const res = await server.post("/api/documents.list", user, {
|
||||
body: {
|
||||
statusFilter: [StatusFilter.Draft],
|
||||
},
|
||||
});
|
||||
const body = await res.json();
|
||||
expect(res.status).toEqual(200);
|
||||
expect(body.data.length).toEqual(1);
|
||||
expect(body.data[0].id).toEqual(document.id);
|
||||
expect(body.pagination.total).toEqual(1);
|
||||
});
|
||||
|
||||
it("should not return archived documents", async () => {
|
||||
const user = await buildUser();
|
||||
const document = await buildDocument({
|
||||
|
||||
@@ -243,6 +243,19 @@ router.post(
|
||||
}
|
||||
|
||||
if (statusFilter?.includes(StatusFilter.Draft)) {
|
||||
// Pre-fetch document IDs the user has a direct membership on so the
|
||||
// filter can be expressed without referencing the (separately-loaded)
|
||||
// memberships association, which would otherwise break the COUNT query.
|
||||
const membershipDocumentIds = (
|
||||
await UserMembership.findAll({
|
||||
attributes: ["documentId"],
|
||||
where: {
|
||||
userId: user.id,
|
||||
documentId: { [Op.ne]: null },
|
||||
},
|
||||
})
|
||||
).map((m) => m.documentId as string);
|
||||
|
||||
statusQuery.push({
|
||||
[Op.and]: [
|
||||
{
|
||||
@@ -255,7 +268,7 @@ router.post(
|
||||
[Op.or]: [
|
||||
// Only ever include draft results for the user's own documents
|
||||
{ createdById: user.id },
|
||||
{ "$memberships.id$": { [Op.ne]: null } },
|
||||
{ id: membershipDocumentIds },
|
||||
],
|
||||
},
|
||||
],
|
||||
@@ -292,12 +305,24 @@ router.post(
|
||||
: undefined
|
||||
: [[sort, direction]];
|
||||
|
||||
const includeDrafts = !!statusFilter?.includes(StatusFilter.Draft);
|
||||
|
||||
// The withDrafts scope drops the defaultScope filters, so re-apply the
|
||||
// ones we still want — templates and trial-import documents should never
|
||||
// appear in this listing.
|
||||
if (includeDrafts) {
|
||||
where[Op.and].push({
|
||||
template: false,
|
||||
sourceMetadata: { trial: { [Op.is]: null } },
|
||||
});
|
||||
}
|
||||
|
||||
// When sorting by index, pagination is already handled by slicing documentIds,
|
||||
// so we skip the SQL-level offset to avoid double-pagination
|
||||
const { results: documents, pagination } = await paginateQuery(
|
||||
ctx,
|
||||
({ offset: queryOffset, limit: queryLimit }) =>
|
||||
Document.withMembershipScope(user.id).findAll({
|
||||
Document.withMembershipScope(user.id, { includeDrafts }).findAll({
|
||||
where,
|
||||
order: orderClause as Order,
|
||||
offset: sort === "index" ? 0 : queryOffset,
|
||||
@@ -306,7 +331,10 @@ router.post(
|
||||
documentIds,
|
||||
},
|
||||
}),
|
||||
() => Document.count({ where })
|
||||
() =>
|
||||
Document.withMembershipScope(user.id, { includeDrafts }).count({
|
||||
where,
|
||||
})
|
||||
);
|
||||
|
||||
const data = await presentDocuments(ctx, documents);
|
||||
|
||||
Reference in New Issue
Block a user