Skip to content

Apply comment visibility settings to recording history - #8088

Open
3mdistal wants to merge 6 commits into
t3/clips-trash-comment-install-fixesfrom
t3/clips-comment-visibility
Open

3mdistal wants to merge 6 commits into
t3/clips-trash-comment-install-fixesfrom
t3/clips-comment-visibility

Conversation

@3mdistal

@3mdistal 3mdistal commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Turning comments off hides saved comment text from viewers in notification previews and recording search. The owner retains access through the comment list, player, and screen context, with a read-only discussion panel. Authenticated owners entering through share links are redirected to that player.

Recording reactions retain their independent setting when comments are off. The signed-in player header preserves its existing role and preview exclusions.

Creation, replies, edits, comment reactions, and resolution remain disabled. Authorized deletion retains the approved cleanup exception, subject to the existing author/editor, Trash, and expiry checks.

Stacked on #8059. This implements the clarified comments-off policy separately; it will target main after the base PR merges.

Verification:

  • Notification and search regressions failed before production edits; owner and enabled-comment controls passed.
  • Owner read regressions failed before the read-only exemption.
  • All 306 tests across 17 relevant suites pass, including live reply/edit transitions, owner agent history, rendered owner-panel controls, and existing write restrictions. Clips typecheck and formatting pass.
  • The reaction regression failed before the fix; the final root rerun passes 141 tests across nine affected suites. The excluded action fixture also passes a direct TypeScript semantic check after a failing type probe.
  • E2B real-UI verification with a fake signed-in owner reproduced the disabled reaction button, then verified a successful reaction, database persistence and reload. Comments stayed disabled; reactions-off and preview controls passed. The sandbox and browsers were terminated. Unconfigured sandbox media storage returned the same unrelated thumbnail-upload 503 before and after.
  • All 93 repository guards passed across the sweep and dependency-setup recovery reruns, with GNU grep on PATH. Current CI strict-lint and oracle-change checks also pass; no baseline was changed.
  • Independent Codex Sol review found no remaining actionable issues after failing-first verification of both review findings.

No schema or stored-data changes.

@github-actions

Copy link
Copy Markdown
Contributor

Here's a visual recap of what changed:

Visual recap

Open the full interactive recap

builder-io-integration[bot]

This comment was marked as outdated.

Copy link
Copy Markdown
Contributor

Thanks for the update and follow-up details. This adds an owner-only read-only discussion panel and changes how disabled comments appear across Clips history, so please get Shomix's Clips-owner review before approval.

@3mdistal

Copy link
Copy Markdown
Contributor Author

Alice confirmed the comments-off policy on Oct 22: viewers lose access to saved comments everywhere, while owners retain read access. This PR implements that policy, including the owner read-only panel. It will remain open pending the requested Clips-owner review and your approval.

Copy link
Copy Markdown
Contributor

Thanks for the update and policy confirmation. I checked the owner share-link redirect and the documented cleanup exception; this still needs Shomix’s Clips-owner review before approval.

builder-io-integration[bot]

This comment was marked as outdated.

builder-io-integration[bot]

This comment was marked as outdated.

builder-io-integration[bot]

This comment was marked as outdated.

builder-io-integration[bot]

This comment was marked as outdated.

@builder-io-integration builder-io-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.

Builder reviewed your changes — looks good ✅

Review Details

Incremental review summary

The latest update separates canReact from canComment, preserving the independent reaction setting while retaining role and preview exclusions. The added player-route tests exercise the comments-off/reactions-on interaction, reactions-off visibility, preview restriction, and outgoing mutation payload; the current fix also resolves the previously open reaction-control finding, whose thread I cleared before this review. I found no new runtime defect. One low-severity test-coverage opportunity was reported: the route test uses a mocked mutation and verifies the emitted payload, not database persistence. Risk remains high for the overall PR because its underlying changes govern access to saved discussion. The review system may filter this low-severity observation.

Findings

  • No new runtime issues found.
  • ⚪ Low: the route-level reaction test does not itself verify action persistence.

🧪 Browser testing: Skipped — the browser-test-planner agent is unavailable in this environment. The PR description reports separate E2B real-UI verification.

This branch has not been deployed

No deployments
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