From fc7970a2d1244c040ea1768d7991165835b5e3e2 Mon Sep 17 00:00:00 2001 From: amblued Date: Wed, 26 Aug 2026 22:47:54 +0800 Subject: [PATCH] fix(spaces): add context to summary query errors Preserve the underlying database error across all drivers and cover the wrapping behavior with focused tests. Make creation tests wait for the asynchronous completion callbacks to avoid timing-dependent assertions. --- store/db/mysql/space.go | 9 ++++-- store/db/mysql/space_test.go | 26 ++++++++++++++++++ store/db/postgres/space.go | 11 ++++++-- store/db/postgres/space_test.go | 26 ++++++++++++++++++ store/db/sqlite/space.go | 11 ++++++-- store/db/sqlite/space_test.go | 26 ++++++++++++++++++ web/tests/create-space-dialog.test.tsx | 38 ++++++++++++++++++++++---- 7 files changed, 136 insertions(+), 11 deletions(-) create mode 100644 store/db/mysql/space_test.go create mode 100644 store/db/postgres/space_test.go create mode 100644 store/db/sqlite/space_test.go diff --git a/store/db/mysql/space.go b/store/db/mysql/space.go index 25c64461..139f4a03 100644 --- a/store/db/mysql/space.go +++ b/store/db/mysql/space.go @@ -444,15 +444,20 @@ func scanMySQLSpaceWithSummary(row mysqlRowScanner, space *store.Space, withSumm return row.Scan(&space.ID, &space.UID, &space.Title, &space.Description, &space.CurrentUserRole, &space.MemberCount) } +func scanMySQLSpaceSummary(row mysqlRowScanner, space *store.Space) error { + return errors.Wrap(row.Scan(&space.CurrentUserRole, &space.MemberCount), "failed to populate MySQL space summary") +} + func populateMySQLSpaceSummary(ctx context.Context, tx *sql.Tx, space *store.Space, userID int32) error { - return tx.QueryRowContext(ctx, `SELECT viewer_member.role, COUNT(active_member.user_id) + row := tx.QueryRowContext(ctx, `SELECT viewer_member.role, COUNT(active_member.user_id) FROM space_member viewer_member JOIN user viewer_user ON viewer_user.id = viewer_member.user_id JOIN space_member active_member ON active_member.space_id = viewer_member.space_id AND active_member.status = 'ACTIVE' AND active_member.role IN ('ADMIN', 'USER') JOIN user active_user ON active_user.id = active_member.user_id AND active_user.row_status = 'NORMAL' WHERE viewer_member.space_id = ? AND viewer_member.user_id = ? AND viewer_member.status = 'ACTIVE' AND viewer_member.role IN ('ADMIN', 'USER') AND viewer_user.row_status = 'NORMAL' - GROUP BY viewer_member.role`, space.ID, userID).Scan(&space.CurrentUserRole, &space.MemberCount) + GROUP BY viewer_member.role`, space.ID, userID) + return scanMySQLSpaceSummary(row, space) } func getMySQLSpace(ctx context.Context, tx *sql.Tx, id int32) (*store.Space, error) { diff --git a/store/db/mysql/space_test.go b/store/db/mysql/space_test.go new file mode 100644 index 00000000..bb57a289 --- /dev/null +++ b/store/db/mysql/space_test.go @@ -0,0 +1,26 @@ +package mysql + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/usememos/memos/store" +) + +type failingMySQLSpaceSummaryRow struct { + err error +} + +func (row failingMySQLSpaceSummaryRow) Scan(...any) error { + return row.err +} + +func TestScanMySQLSpaceSummaryWrapsErrors(t *testing.T) { + cause := errors.New("scan failed") + err := scanMySQLSpaceSummary(failingMySQLSpaceSummaryRow{err: cause}, &store.Space{}) + + require.ErrorIs(t, err, cause) + require.ErrorContains(t, err, "failed to populate MySQL space summary") +} diff --git a/store/db/postgres/space.go b/store/db/postgres/space.go index 8fceb2c6..98143d69 100644 --- a/store/db/postgres/space.go +++ b/store/db/postgres/space.go @@ -137,15 +137,22 @@ func (d *DB) UpdateSpace(ctx context.Context, update *store.UpdateSpace, actorUs return space, nil } +type postgresSpaceSummaryRowScanner interface{ Scan(...any) error } + +func scanPostgresSpaceSummary(row postgresSpaceSummaryRowScanner, space *store.Space) error { + return errors.Wrap(row.Scan(&space.CurrentUserRole, &space.MemberCount), "failed to populate PostgreSQL space summary") +} + func populatePostgresSpaceSummary(ctx context.Context, tx *sql.Tx, space *store.Space, userID int32) error { - return tx.QueryRowContext(ctx, `SELECT viewer_member.role, COUNT(active_member.user_id) + row := tx.QueryRowContext(ctx, `SELECT viewer_member.role, COUNT(active_member.user_id) FROM space_member viewer_member JOIN "user" viewer_user ON viewer_user.id = viewer_member.user_id JOIN space_member active_member ON active_member.space_id = viewer_member.space_id AND active_member.status = 'ACTIVE' AND active_member.role IN ('ADMIN', 'USER') JOIN "user" active_user ON active_user.id = active_member.user_id AND active_user.row_status = 'NORMAL' WHERE viewer_member.space_id = $1 AND viewer_member.user_id = $2 AND viewer_member.status = 'ACTIVE' AND viewer_member.role IN ('ADMIN', 'USER') AND viewer_user.row_status = 'NORMAL' - GROUP BY viewer_member.role`, space.ID, userID).Scan(&space.CurrentUserRole, &space.MemberCount) + GROUP BY viewer_member.role`, space.ID, userID) + return scanPostgresSpaceSummary(row, space) } func (d *DB) CreateSpaceInvitation(ctx context.Context, create *store.SpaceInvitation, actorUserID int32) (*store.SpaceInvitation, error) { diff --git a/store/db/postgres/space_test.go b/store/db/postgres/space_test.go new file mode 100644 index 00000000..c2e5cd52 --- /dev/null +++ b/store/db/postgres/space_test.go @@ -0,0 +1,26 @@ +package postgres + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/usememos/memos/store" +) + +type failingPostgresSpaceSummaryRow struct { + err error +} + +func (row failingPostgresSpaceSummaryRow) Scan(...any) error { + return row.err +} + +func TestScanPostgresSpaceSummaryWrapsErrors(t *testing.T) { + cause := errors.New("scan failed") + err := scanPostgresSpaceSummary(failingPostgresSpaceSummaryRow{err: cause}, &store.Space{}) + + require.ErrorIs(t, err, cause) + require.ErrorContains(t, err, "failed to populate PostgreSQL space summary") +} diff --git a/store/db/sqlite/space.go b/store/db/sqlite/space.go index 1bb22471..ee6d0c2c 100644 --- a/store/db/sqlite/space.go +++ b/store/db/sqlite/space.go @@ -128,15 +128,22 @@ func (d *DB) UpdateSpace(ctx context.Context, update *store.UpdateSpace, actorUs return space, nil } +type sqliteSpaceSummaryRowScanner interface{ Scan(...any) error } + +func scanSQLiteSpaceSummary(row sqliteSpaceSummaryRowScanner, space *store.Space) error { + return errors.Wrap(row.Scan(&space.CurrentUserRole, &space.MemberCount), "failed to populate SQLite space summary") +} + func populateSQLiteSpaceSummary(ctx context.Context, tx dbExecutor, space *store.Space, userID int32) error { - return tx.QueryRowContext(ctx, `SELECT viewer_member.role, COUNT(active_member.user_id) + row := tx.QueryRowContext(ctx, `SELECT viewer_member.role, COUNT(active_member.user_id) FROM space_member viewer_member JOIN user viewer_user ON viewer_user.id = viewer_member.user_id JOIN space_member active_member ON active_member.space_id = viewer_member.space_id AND active_member.status = 'ACTIVE' AND active_member.role IN ('ADMIN', 'USER') JOIN user active_user ON active_user.id = active_member.user_id AND active_user.row_status = 'NORMAL' WHERE viewer_member.space_id = ? AND viewer_member.user_id = ? AND viewer_member.status = 'ACTIVE' AND viewer_member.role IN ('ADMIN', 'USER') AND viewer_user.row_status = 'NORMAL' - GROUP BY viewer_member.role`, space.ID, userID).Scan(&space.CurrentUserRole, &space.MemberCount) + GROUP BY viewer_member.role`, space.ID, userID) + return scanSQLiteSpaceSummary(row, space) } func (d *DB) CreateSpaceInvitation(ctx context.Context, create *store.SpaceInvitation, actorUserID int32) (*store.SpaceInvitation, error) { diff --git a/store/db/sqlite/space_test.go b/store/db/sqlite/space_test.go new file mode 100644 index 00000000..747c63fb --- /dev/null +++ b/store/db/sqlite/space_test.go @@ -0,0 +1,26 @@ +package sqlite + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/usememos/memos/store" +) + +type failingSQLiteSpaceSummaryRow struct { + err error +} + +func (row failingSQLiteSpaceSummaryRow) Scan(...any) error { + return row.err +} + +func TestScanSQLiteSpaceSummaryWrapsErrors(t *testing.T) { + cause := errors.New("scan failed") + err := scanSQLiteSpaceSummary(failingSQLiteSpaceSummaryRow{err: cause}, &store.Space{}) + + require.ErrorIs(t, err, cause) + require.ErrorContains(t, err, "failed to populate SQLite space summary") +} diff --git a/web/tests/create-space-dialog.test.tsx b/web/tests/create-space-dialog.test.tsx index 4434eb8e..9f888198 100644 --- a/web/tests/create-space-dialog.test.tsx +++ b/web/tests/create-space-dialog.test.tsx @@ -42,9 +42,11 @@ describe("CreateSpaceDialog", () => { fireEvent.change(screen.getByLabelText("common.description"), { target: { value: " Plans " } }); fireEvent.click(screen.getByRole("button", { name: "common.create" })); - await waitFor(() => expect(mocks.mutateAsync).toHaveBeenCalledWith({ title: "Product", description: "Plans" })); - expect(mocks.onCreated).toHaveBeenCalledWith({ name: "spaces/product", title: "Product", description: "Plans" }); - expect(mocks.onOpenChange).toHaveBeenCalledWith(false); + await waitFor(() => { + expect(mocks.onCreated).toHaveBeenCalledWith({ name: "spaces/product", title: "Product", description: "Plans" }); + expect(mocks.onOpenChange).toHaveBeenCalledWith(false); + }); + expect(mocks.mutateAsync).toHaveBeenCalledWith({ title: "Product", description: "Plans" }); }); it("can create without activating the new Space", async () => { @@ -53,9 +55,35 @@ describe("CreateSpaceDialog", () => { fireEvent.change(screen.getByLabelText("common.name"), { target: { value: "Product" } }); fireEvent.click(screen.getByRole("button", { name: "common.create" })); - await waitFor(() => expect(mocks.mutateAsync).toHaveBeenCalledOnce()); + await waitFor(() => expect(mocks.onOpenChange).toHaveBeenCalledWith(false)); + expect(mocks.mutateAsync).toHaveBeenCalledOnce(); expect(mocks.onCreated).not.toHaveBeenCalled(); - expect(mocks.onOpenChange).toHaveBeenCalledWith(false); + }); + + it("waits for creation to finish before closing and reporting the Space", async () => { + let resolveCreate!: (space: { name: string; title: string; description: string }) => void; + mocks.mutateAsync.mockReturnValue( + new Promise((resolve) => { + resolveCreate = resolve; + }), + ); + render(); + + fireEvent.change(screen.getByLabelText("common.name"), { target: { value: "Product" } }); + fireEvent.click(screen.getByRole("button", { name: "common.create" })); + + expect(mocks.mutateAsync).toHaveBeenCalledOnce(); + expect(mocks.onOpenChange).not.toHaveBeenCalled(); + expect(mocks.onCreated).not.toHaveBeenCalled(); + + await act(async () => { + resolveCreate({ name: "spaces/product", title: "Product", description: "" }); + }); + + await waitFor(() => { + expect(mocks.onOpenChange).toHaveBeenCalledWith(false); + expect(mocks.onCreated).toHaveBeenCalledWith({ name: "spaces/product", title: "Product", description: "" }); + }); }); it("prevents dismissal while creation is pending", async () => {