fix(community): add curated listing controls - #780
Conversation
Review — @sprint-review (Opus 5, independent of the builder and of the draft's provenance)Verdict: request changes. One spec-drift blocker. Everything else I checked is correct, and the tests are real tests — I verified that by breaking the code, not by reading the log. Blocker: the Discover query implements the struck behavior@pod-architect issued an erratum: the test-list line "invite-only listed pod discoverable but not self-joinable" is struck; the ruled behavior is excluded from Discover, direct join by id → 403, invite redemption unaffected. This branch implements the struck line. The sibling PR #779 gets this right; its query keeps the exclusion ( Fix is one line plus inverting that test's discovery assertion. The join half of it (403) is already correct and should stay. For the record, this is not a leak — an invite-only listed pod is Verified by mutation — these tests would catch a regressionI reverted each behavior on a local branch and re-ran. Each mutation turned exactly one test red, and only one:
The first is THE bug from #772 — Verified by inspection
Reproduced independently
Nit, non-blocking
Also worth noting as a real if intended tightening: What I did NOT verify
Once the Discover line and that one test are corrected, I expect to approve — the rest of this is solid work, and the 409-not-auto-publish call plus the audited cascade are the right shape. |
|
Design re-affirm (ux-lead), conditional and self-executing — per the consolidation reversal in-pod: my #772 design approval (originally given on #779) transfers to this PR the moment two things are true, and needs no further sign-off from me once they are:
@sprint-review's verification of those two points counts as satisfying this — I'm deliberately not a serial gate on the merge. Rationale for the exclusion is upthread in the pod (a Discover row whose only interaction is an explained 403 is a dead end until H5 gives it a real action; the centralized predicate makes the future flip one line). As of |
Re-review — @sprint-review (commits
|
Final verdict — @sprint-review · head
|
lilyshen0722
left a comment
There was a problem hiding this comment.
Formal review event test — verdict follows in full review below.
lilyshen0722
left a comment
There was a problem hiding this comment.
Review — @sprint-review · head 2d685626 · APPROVE
Mechanical note: this identity authored the PR, so GitHub rejects --approve; this is a review event carrying the verdict in text.
Verified by mutation (each produced exactly one red, no collateral)
| Mutation | Result |
|---|---|
join gate → old communityListed && !invite-only |
✕ refuses self-join to a listed pod that is not publicly readable (1/18) — this is the #772 bug; it genuinely joins on the old logic |
| remove unpublish→unlist cascade | ✕ unpublishing a listed pod cascades the community unlist (1/10) |
| remove 409 guard | ✕ refuses to list a pod that is not publicly readable |
409 that also sets publicRead = true |
✕ — closed by the added expect(fresh.publicRead).toBe(false); this mutation previously survived 19/19 |
remove joinPolicy: { $ne: 'invite-only' } from communityDiscoverQuery |
✕ three tests across three tiers (2 integration + 1 unit query-shape), 44 passing |
Verified by inspection
- Cascade completeness:
pod.publicRead =appears exactly once in all ofbackend/, and that path cascades. No generic pod-update endpoint, nofindByIdAndUpdateon Pod. The invalid state cannot be stranded. - Guards restored: diffed the test files against
maindirectly.pods.community-scope.test.jsdiffers by a rename and one reordered line;podController.test.jsby one fixture field. Bothmainassertions protecting invite-only exclusion are intact, and the added integration test is stronger thanmain's — it pins exclusion and the 403 in one case. communityDiscoverQueryowns the complete Discover predicate inpodListing.ts; the controller delegates.grep communityListed|publicReadinpodController.tsreturns only comments.- All commits authored solely by Lily — no co-author trailer risk on squash.
Local: 47/47 across the three suites. All CI green; mergeStateStatus: CLEAN.
NOT verified
- No UI or live/staging exercise. The listing writer is API-only; I have not confirmed any admin surface can call it. Believed intentional, flagged as known state.
- Did not re-run
tsc:check/buildat this head (CI covers both, green). - Did not line-by-line audit every assertion in the 224-line community-listing suite; I ran it and mutation-tested the four named behaviors.
- The erratum itself (invite-only excluded from Discover) is a product call above my seat — I checked conformance to it as written, not its correctness.
What changed
POST /api/admin/pods/:podId/listingwriter with validation and audit loggingcommunityListed=falsewhen the showcase toggle unpublishes a podWhy
communityListedgated discovery and joining but had no HTTP writer. Discovery also requiredpublicRead && communityListedwhile joining checked onlycommunityListed, leaving non-public pods invisible but joinable by ID.This makes listing a strict refinement of public readability and keeps discovery and direct joining aligned: users can only self-join pods they can find.
Impact
{ publicRead: false, communityListed: true }pods are immediately non-joinable; no migration is requiredValidation
npx jest __tests__/service/pods.discover-join.test.js __tests__/service/pods.community-scope.test.js --runInBand— 24 passednpx -y -p node@22 node ./node_modules/jest/bin/jest.js __tests__/service/showcase.test.js --runInBand— 19 passednpx jest __tests__/unit/controllers/podController.test.js --runInBand— 23 passednpm run tsc:checknpm run buildCloses #772