Add featured community listings surfaced as onboarding starter packages - #765
Conversation
📝 WalkthroughWalkthroughAdds admin-controlled featured community listings backed by ChangesFeatured community listings
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Admin
participant CommunityDetail
participant CommunityFeatureAPI
participant CommunityService
participant Database
Admin->>CommunityDetail: select feature or unfeature
CommunityDetail->>CommunityFeatureAPI: POST featured state
CommunityFeatureAPI->>CommunityService: update listing
CommunityService->>Database: write featured_at
Database-->>CommunityService: return updated listing
CommunityService-->>CommunityFeatureAPI: return effective state
CommunityFeatureAPI-->>CommunityDetail: refresh featured badge
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🔎 Preview deployed: https://kody-pr-765.kody-a99.workers.dev Worker: Mocks:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/worker/src/app/handlers/community-feature.ts`:
- Around line 42-51: Update the admin action handler’s logAuditEvent call for
the community listing feature operation to await the audit write instead of
discarding its promise, ensuring the success response is returned only after the
audit attempt completes.
In `@packages/worker/src/community/service.ts`:
- Around line 592-595: Update the repository method
setCommunityListingFeaturedAt used by the service call so featured: true retains
an existing featured_at value, using a null-coalescing database update rather
than refreshing it on retries; ensure featured: false still clears the
timestamp.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0dbc5716-4ddb-467d-8b07-eddc3ee57b5e
📒 Files selected for processing (33)
docs/contributing/architecture/primitives.yamldocs/contributing/community-packages.mddocs/use/community-packages.mde2e/community-featured.spec.tse2e/d1-utils.tspackages/worker/client/routes/community-detail.tsxpackages/worker/client/routes/onboarding.tsxpackages/worker/migrations/0060-community-featured-listings.sqlpackages/worker/src/app/community-data.tspackages/worker/src/app/community-detail-content.tsxpackages/worker/src/app/community-public-types.tspackages/worker/src/app/community-public.tspackages/worker/src/app/data-cache.tspackages/worker/src/app/handlers/community-detail.tsxpackages/worker/src/app/handlers/community-feature.node.test.tspackages/worker/src/app/handlers/community-feature.tspackages/worker/src/app/handlers/onboarding.tspackages/worker/src/app/loader-data.tspackages/worker/src/app/onboarding-data.node.test.tspackages/worker/src/app/onboarding-data.tspackages/worker/src/app/router.tspackages/worker/src/app/routes.tspackages/worker/src/app/ssr-render.node.test.tspackages/worker/src/community/community-flow-test-schema.tspackages/worker/src/community/community-flow.workers.test.tspackages/worker/src/community/repo.tspackages/worker/src/community/service.tspackages/worker/src/community/types.tspackages/worker/src/mcp/capabilities/community/domain.tspackages/worker/src/mcp/capabilities/community/get.tspackages/worker/src/mcp/capabilities/community/set-featured.tspackages/worker/src/mcp/capabilities/community/shared.tspackages/worker/tsconfig-client.json
| void logAuditEvent({ | ||
| db: env.APP_DB, | ||
| category: 'admin', | ||
| action: 'community_listing_feature', | ||
| result: 'success', | ||
| email: actor.email, | ||
| ip: getRequestIp(request) ?? undefined, | ||
| path: new URL(request.url).pathname, | ||
| reason: `listing_id=${listing.id};featured=${parsed.data.featured}`, | ||
| }) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Await the admin audit write before returning success.
Discarding this promise lets the response complete without guaranteeing that the state-changing admin action is persisted in the audit log. logAuditEvent already swallows persistence failures, so awaiting it will not turn audit failures into request failures.
Proposed fix
- void logAuditEvent({
+ await logAuditEvent({📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void logAuditEvent({ | |
| db: env.APP_DB, | |
| category: 'admin', | |
| action: 'community_listing_feature', | |
| result: 'success', | |
| email: actor.email, | |
| ip: getRequestIp(request) ?? undefined, | |
| path: new URL(request.url).pathname, | |
| reason: `listing_id=${listing.id};featured=${parsed.data.featured}`, | |
| }) | |
| await logAuditEvent({ | |
| db: env.APP_DB, | |
| category: 'admin', | |
| action: 'community_listing_feature', | |
| result: 'success', | |
| email: actor.email, | |
| ip: getRequestIp(request) ?? undefined, | |
| path: new URL(request.url).pathname, | |
| reason: `listing_id=${listing.id};featured=${parsed.data.featured}`, | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/worker/src/app/handlers/community-feature.ts` around lines 42 - 51,
Update the admin action handler’s logAuditEvent call for the community listing
feature operation to await the audit write instead of discarding its promise,
ensuring the success response is returned only after the audit attempt
completes.
| await setCommunityListingFeaturedAt(input.env.APP_DB, { | ||
| listingId: input.listingId, | ||
| featured: input.featured, | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve featured_at when the listing is already featured.
The repository setter refreshes this timestamp on every featured: true request. Because onboarding orders by featured_at and applies a limit, retrying the MCP operation—declared idempotent—can change which packages appear and may exclude the retried listing.
Make the database update preserve an existing timestamp, for example with COALESCE(featured_at, ?), while featured: false still clears it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/worker/src/community/service.ts` around lines 592 - 595, Update the
repository method setCommunityListingFeaturedAt used by the service call so
featured: true retains an existing featured_at value, using a null-coalescing
database update rather than refreshing it on retries; ensure featured: false
still clears the timestamp.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 02537b3. Configure here.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/worker/src/app/handlers/community-trust.node.test.ts (1)
68-78: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the revoke path for
featured.This test only verifies
trusted: true/featured: true. Add a revocation case assertingtrusted: falseandfeatured: false(and ideally a re-trust case) so the effective featured-state synchronization cannot regress.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/app/handlers/community-trust.node.test.ts` around lines 68 - 78, Extend the test around handler.handler and buildTrustRequest with a revoke case that submits trusted: false and asserts a successful response containing trusted: false and featured: false. Preferably follow it with a re-trust case asserting both values return to true, preserving the existing success and synchronization expectations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/worker/src/app/handlers/community-trust.node.test.ts`:
- Around line 68-78: Extend the test around handler.handler and
buildTrustRequest with a revoke case that submits trusted: false and asserts a
successful response containing trusted: false and featured: false. Preferably
follow it with a re-trust case asserting both values return to true, preserving
the existing success and synchronization expectations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 947fde7a-ccbc-4c35-a41c-a49f2cf74636
📒 Files selected for processing (3)
packages/worker/client/routes/community-detail.tsxpackages/worker/src/app/handlers/community-trust.node.test.tspackages/worker/src/app/handlers/community-trust.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/worker/client/routes/community-detail.tsx
3 participants
Summary
Third and final PR in the trusted/one-click-install/onboarding series (after #760 and #763). Admins can now mark trusted community listings as featured, and featured listings appear on
/onboardingas one-click-installable starter packages for new users.featured_atcolumn oncommunity_listings(migration0060). A listing is effectively featured only whilefeatured_at IS NOT NULLand it is effectively trusted (trusted_commit = pinned_commit), so an owner republish silently pulls it from onboarding until an admin re-trusts the new version; the stored mark survives and re-trusting restores it.setCommunityListingFeaturedservice (rejects untrusted/delisted listings) +listFeaturedCommunityListings(active + featured + trusted, capped at 6 for onboarding).community_set_featuredMCP capability (requiredRole: 'admin', invisible to non-admins, audited) and admin-onlyPOST /community/:listingId/feature.jsonhandler (audited).OnboardingFeaturedListingpayload — no README) linking to their detail pages where the one-click Install button (from Add one-click install for community listings #763) lives. Falls back to the existing "Explore community packages" CTA when nothing is featured. Featured data is omitted for unverified users and fails open to an empty list.community_getexposes the effectivefeaturedflag.primitives.yaml.Demo
Admin features the trusted
@jane/notes-digestlisting; the Featured badge appears, and the untrusted listing shows the disabled explanation instead.A fresh user sees the starter package on
/onboardingand one-click installs it.Testing
npm run validate(format, lint, typecheck, unit, Playwright E2E, MCP E2E — all green)community-feature.node.test.ts(RBAC, validation, error mapping)e2e/community-featured.spec.ts(member 403 + no admin panel, admin toggle, disabled button on untrusted, onboarding starter section appears/disappears)System recap — extends community listings (medium risk)
Mode: recap · Base:
main@236554c4· Head:7ad88523Classification: extends — no new primitives;
community-listingsgains an admin featured mark and the onboarding page gains a starter-package step built on it.Primitives touched
community-listingsfeatured_atcolumn,setCommunityListingFeatured/listFeaturedCommunityListings, badgeapp-ui/onboardingmcp-servercommunity_set_featuredcapability in the existing community domainrbacrequiredRole: 'admin'on the capability;requireUserWithRoleonfeature.jsond1-app-db0060-community-featured-listings.sqladdscommunity_listings.featured_atSystem map
Admin featuring flows from the detail-page toggle through RBAC into the community service and D1; onboarding reads the effectively featured listings back out as starter packages.
Legend: green = composes (wiring only) · amber = extended by this PR · red = new primitive · gray = context (unchanged, included only when an edge crosses it).
Before / after
Invariants
community-listing-isolation: unchanged — featuring only affects which public listings onboarding highlights; installs still go through the Add one-click install for community listings #763 fork + publish-check path and the featured flag never bypasses the untrusted acknowledgement (featuring requires effective trust).featured_at IS NOT NULL AND trusted, so an owner republish drops the listing from onboarding without an explicit revoke.primitives.yamlupdated in this PR (community-listingssummary + migration path).Summary by CodeRabbit