Skip to content

ENG-2335 Add a shared check for whether an imported node is published to a group - #1512

Merged
mdroidian merged 2 commits into
mainfrom
eng-2335-Add-a-shared-check-for-whether-an-imported-node-is-published-to-a-group
Oct 5, 2026
Merged

mdroidian merged 2 commits into
mainfrom
eng-2335-Add-a-shared-check-for-whether-an-imported-node-is-published-to-a-group

Conversation

@maparent

@maparent maparent commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

https://entire.io/gh/DiscourseGraphs/discourse-graph/trails/58

Reviewer brief

  • Result: getPublishedGroupIdsByRid returns, for each imported node RID, the user's groups that can read that node: groups with reader or editor access to its space, plus groups with partial access and a ResourceAccess grant for the node.
  • Review focus: the space-level read uses the my_pseudo_accounts view, not SpaceAccess, because the SpaceAccess select policy hides partial rows. See the ticket comment.
  • Risk or follow-up: the ResourceAccess batch size is set from the group count so that a batch stays under max_rows (1000). Rows from local ids that collide across a batch's spaces are not counted, and the hosted max_rows is assumed to match supabase/config.toml. Paging with getAllPages would remove both assumptions; I decided that it was adding scope, but it's certainly a reasonable thing to do.

Verification

Unit tests.
One bug was found during ENG-2326 testing (last separate commit)

Loom video

Code walk
https://www.loom.com/share/59a52c0988d04de8952ffc0fde813b07

Scope check

  • Ran $scope-check against ENG-2335 and the final diff.
  • Scope beyond Done When: None.

Standards check

  • Ran $dg-pr-adherence-check against the final diff and PR metadata.

Local delegated full review

  • Ran a comprehensive review of the entire final diff in a subagent with a fresh context.
    (Re-ran with the last commit)

https://linear.app/discourse-graphs/issue/ENG-2335/add-a-shared-check-for-whether-an-imported-node-is-published-to-a


Devin Review

@supabase

supabase Bot commented Oct 2, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@vercel

vercel Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
discourse-graph Ready Ready Preview Oct 4, 2026 3:10pm UTC

Request Review

@linear-code

linear-code Bot commented Oct 2, 2026

Copy link
Copy Markdown

ENG-2335

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 3 potential issues.

2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)

Devin Review

Comment thread packages/database/src/lib/groups.ts
Comment thread packages/database/src/lib/groups.ts
Comment thread packages/database/src/lib/groups.ts
@maparent
maparent force-pushed the eng-2335-Add-a-shared-check-for-whether-an-imported-node-is-published-to-a-group branch from 5330315 to 60df351 Compare October 2, 2026 21:24
@maparent
maparent force-pushed the eng-2335-Add-a-shared-check-for-whether-an-imported-node-is-published-to-a-group branch from 60df351 to 50cabfc Compare October 3, 2026 12:34
@maparent
maparent force-pushed the eng-2335-Add-a-shared-check-for-whether-an-imported-node-is-published-to-a-group branch from 50cabfc to bda2ebc Compare October 3, 2026 12:41
@maparent
maparent requested a review from mdroidian October 3, 2026 13:10
@mdroidian
mdroidian merged commit 2952dd5 into main Oct 5, 2026
12 checks passed
@mdroidian
mdroidian deleted the eng-2335-Add-a-shared-check-for-whether-an-imported-node-is-published-to-a-group branch October 5, 2026 04:12

This branch was successfully deployed

1 active deployment
Preview — 0199fda4 Deployed Oct 4, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants