Skip to content

ENG-1796 Duplicate entries in spaces_in_group function #1077

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
maparent wants to merge 2 commits into
base: main
Choose a base branch
from
+64 −53
Open

ENG-1796 Duplicate entries in spaces_in_group function #1077

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2ee31d0
ENG-1796 change the view to avoid duplicates
maparent May 25, 2026
a0af0c3
actually add group information to pseudo-account view
maparent May 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 22 additions & 25 deletions apps/website/test/integration/listGroupMembers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import assert from "assert";
import { describe, it, beforeAll, afterAll } from "vitest";
import { createClient } from "@supabase/supabase-js";
import type { Database } from "@repo/database/dbTypes";
import type { Database, Tables } from "@repo/database/dbTypes";
import type { DGSupabaseClient } from "@repo/database/lib/client";
import {
fetchOrCreateSpaceDirect,
Expand All @@ -14,7 +14,7 @@ const ANON_KEY = process.env.SUPABASE_PUBLISHABLE_KEY!;
const SERVICE_KEY = process.env.SUPABASE_SECRET_KEY!;
const PASSWORD = "abcdefgh";

type GroupSpaceInfo = Database["public"]["CompositeTypes"]["group_space_info"];
type PseudoAccountInfo = Tables<"my_pseudo_accounts">;

const freshClient = (): DGSupabaseClient =>
createClient<Database, "public">(SUPABASE_URL, ANON_KEY);
Expand Down Expand Up @@ -121,19 +121,18 @@ describe("list group members flow", { tags: ["database"] }, () => {

const expectedSpaceIds = [spaceId1, spaceId2];
// Step 3: user1 lists group members
const { data: data1, error: error1 } = await client1.rpc(
"spaces_in_group",
{
p_group_id: createdGroupId, // eslint-disable-line @typescript-eslint/naming-convention
},
);
const { data: data1, error: error1 } = await client1
.from("my_pseudo_accounts")
.select()
.eq("group_id", createdGroupId);

assert(error1 === null, error1 ? error1.message : "");
assert(data1 !== null, "group spaces should not be empty");
assert(data1.length === 2, "There should be two spaces");
console.log(data1);
const spacesSeenBy1 = Object.fromEntries(
data1.filter((gm) => gm.id !== null).map((gm) => [gm.id, gm]),
) as Record<number, GroupSpaceInfo>;
data1.filter((gm) => gm.space_id !== null).map((gm) => [gm.space_id, gm]),
) as Record<number, PseudoAccountInfo>;
assert(
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
expectedSpaceIds.every((id) => spacesSeenBy1[id] !== undefined),
Expand All @@ -145,17 +144,15 @@ describe("list group members flow", { tags: ["database"] }, () => {
),
);
// Step 4: user2 lists group members
const { data: data2, error: error2 } = await client2.rpc(
"spaces_in_group",
{
p_group_id: createdGroupId, // eslint-disable-line @typescript-eslint/naming-convention
},
);
const { data: data2, error: error2 } = await client2
.from("my_pseudo_accounts")
.select()
.eq("group_id", createdGroupId);
assert(error2 === null, error2 ? error2.message : "");
assert(data2 !== null, "group spaces should not be empty");
assert(data2.length === 2, "There should be two spaces");
const spacesSeenBy2 = new Set(
data2.map((gm) => gm.id).filter((id) => id !== null),
data2.map((gm) => gm.space_id).filter((id) => id !== null),
);
assert(
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
Expand All @@ -173,19 +170,19 @@ describe("list group members flow", { tags: ["database"] }, () => {
});
assert(!errorPublishSpace2);
// Step 6: that space is now seen as published by 1.
const { data: data1b, error: error1b } = await client1.rpc(
"spaces_in_group",
{
p_group_id: createdGroupId, // eslint-disable-line @typescript-eslint/naming-convention
},
);
const { data: data1b, error: error1b } = await client1
.from("my_pseudo_accounts")
.select()
.eq("group_id", createdGroupId);

assert(error1b === null, error1b ? error1b.message : "");
assert(data1b !== null, "group spaces should not be empty");
assert(data1b.length === 2, "There should be two spaces");
const spacesSeenBy1b = Object.fromEntries(
data1b.filter((gm) => gm.id !== null).map((gm) => [gm.id, gm]),
) as Record<number, GroupSpaceInfo>;
data1b
.filter((gm) => gm.space_id !== null)
.map((gm) => [gm.space_id, gm]),
) as Record<number, PseudoAccountInfo>;
assert(
spacesSeenBy1b[spaceId2]?.sharing_permissions,
"Second space should now be seen as shared",
Expand Down
29 changes: 10 additions & 19 deletions packages/database/src/dbTypes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1322,7 +1322,9 @@ export type Database = {
}
my_pseudo_accounts: {
Row: {
admin: boolean | null
dg_account: string | null
group_id: string | null
id: number | null
name: string | null
platform: Database["public"]["Enums"]["Platform"] | null
Expand All @@ -1332,6 +1334,13 @@ export type Database = {
space_id: number | null
}
Relationships: [
{
foreignKeyName: "group_membership_group_id_fkey"
columns: ["group_id"]
isOneToOne: false
referencedRelation: "my_groups"
referencedColumns: ["id"]
},
{
foreignKeyName: "PlatformAccount_dg_account_fkey"
columns: ["dg_account"]
Expand Down Expand Up @@ -1803,16 +1812,6 @@ export type Database = {
isSetofReturn: true
}
}
spaces_in_group: {
Args: { p_group_id: string }
Returns: Database["public"]["CompositeTypes"]["group_space_info"][]
SetofOptions: {
from: "*"
to: "group_space_info"
isOneToOne: false
isSetofReturn: true
}
}
unowned_account_in_shared_space: {
Args: { p_account_id: number }
Returns: boolean
Expand Down Expand Up @@ -1983,15 +1982,6 @@ export type Database = {
| Database["public"]["CompositeTypes"]["account_local_input"]
| null
}
group_space_info: {
id: number | null
name: string | null
platform: Database["public"]["Enums"]["Platform"] | null
sharing_permissions:
| Database["public"]["Enums"]["SpaceAccessPermissions"]
| null
admin: boolean | null
}
inline_embedding_input: {
model: string | null
vector: number[] | null
Expand Down Expand Up @@ -2178,3 +2168,4 @@ export const Constants = {
},
},
} as const

22 changes: 22 additions & 0 deletions packages/database/supabase/migrations/20260525003723_correct_pseudo_account.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
DROP VIEW public.my_pseudo_accounts;

CREATE VIEW public.my_pseudo_accounts AS
SELECT
pa.id,
pa.platform,
pa.dg_account,
gm.group_id,
gm.admin,
sa.space_id,
sp.name,
grpsa.permissions AS sharing_permissions
FROM public."PlatformAccount" AS pa
JOIN public.group_membership AS gm ON (member_id = pa.dg_account)
JOIN public.group_membership AS gm2 ON (gm2.member_id = auth.uid() AND gm2.group_id = gm.group_id)
JOIN public."SpaceAccess" AS sa ON (sa.account_uid = pa.dg_account)
JOIN public."Space" AS sp ON (sp.id = sa.space_id)
LEFT OUTER JOIN public."SpaceAccess" AS grpsa ON (grpsa.account_uid = gm.group_id AND grpsa.space_id = sp.id);
Comment on lines +14 to +18
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reinstate pseudo-account constraints in member view

The redefined my_pseudo_accounts view no longer filters to anonymous editor accounts, so it now includes any PlatformAccount/SpaceAccess rows for a group member. In groups where members also have non-anonymous accounts or non-editor access rows, this will inflate results and can reintroduce incorrect/duplicate member-space entries. Add back the predicate that limits rows to the intended pseudo-account shape.

Useful? React with 👍 / 👎.


DROP FUNCTION public.spaces_in_group;

DROP TYPE public.group_space_info;
Comment on lines +20 to +22
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep declarative schema aligned with dropped group RPC

This migration drops spaces_in_group and group_space_info, but packages/database/supabase/schemas/account.sql still defines both objects. Since this repo uses declarative schemas, that leaves the desired schema inconsistent with applied migrations and will cause future schema diffs to reintroduce the dropped RPC/type unexpectedly. Please remove the corresponding CREATE TYPE/CREATE FUNCTION from the schema (or stop dropping them here) so the two sources of truth stay consistent.

Useful? React with 👍 / 👎.

19 changes: 10 additions & 9 deletions packages/database/supabase/schemas/account.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,16 +463,18 @@ SELECT
pa.id,
pa.platform,
pa.dg_account,
gm.group_id,
gm.admin,
sa.space_id,
sp.name,
mysa.permissions AS sharing_permissions
grpsa.permissions AS sharing_permissions
FROM public."PlatformAccount" AS pa
JOIN public.group_membership AS gm ON (member_id = dg_account)
JOIN public.group_membership AS gm ON (member_id = pa.dg_account)
JOIN public.group_membership AS gm2 ON (gm2.member_id = auth.uid() AND gm2.group_id = gm.group_id)
JOIN public."SpaceAccess" AS sa ON (sa.account_uid = dg_account)
JOIN public."SpaceAccess" AS sa ON (sa.account_uid = pa.dg_account)
JOIN public."Space" AS sp ON (sp.id = sa.space_id)
LEFT OUTER JOIN public."SpaceAccess" AS mysa ON (mysa.account_uid = gm.group_id AND mysa.space_id = sp.id)
WHERE pa.agent_type = 'anonymous' AND sa.permissions = 'editor';
LEFT OUTER JOIN public."SpaceAccess" AS grpsa ON (grpsa.account_uid = gm.group_id AND grpsa.space_id = sp.id);


CREATE TYPE public.group_space_info AS (
id BIGINT,
Expand All @@ -486,10 +488,9 @@ CREATE OR REPLACE FUNCTION public.spaces_in_group(p_group_id UUID) RETURNS SETOF
STABLE
SET search_path = ''
LANGUAGE sql AS $$
SELECT pa.space_id as id, pa.name, pa.platform, pa.sharing_permissions, gm.admin
FROM public.my_pseudo_accounts AS pa
JOIN public.group_membership AS gm ON (gm.member_id = pa.dg_account)
WHERE gm.group_id = p_group_id;
SELECT space_id as id, name, platform, sharing_permissions, admin
FROM public.my_pseudo_accounts
WHERE group_id = p_group_id;
$$;

CREATE OR REPLACE FUNCTION public.accept_group_invitation(token varchar) RETURNS boolean
Expand Down
Loading