Skip to content

feat(preprod): Add post-on-added/removed PR comment toggles (EME-1046)#114130

Open
runningcode wants to merge 3 commits intomasterfrom
no/eme-1046-snapshot-pr-comments-only-if-diff-fe
Open

feat(preprod): Add post-on-added/removed PR comment toggles (EME-1046)#114130
runningcode wants to merge 3 commits intomasterfrom
no/eme-1046-snapshot-pr-comments-only-if-diff-fe

Conversation

@runningcode
Copy link
Copy Markdown
Contributor

@runningcode runningcode commented Apr 28, 2026

Summary

Replace the only-if-diff sub-toggle with explicit "Post on Added" / "Post on Removed" toggles under the main PR comments switch:

  • Main switch (preprodSnapshotPrCommentsEnabled) now governs whether comments post at all. Enabling it always means "post when there are changes" — the prior always-post mode is gone.
  • Post on Added Snapshots (preprodSnapshotPrCommentsPostOnAdded, default off) — treat newly added snapshots as a change worth posting.
  • Post on Removed Snapshots (preprodSnapshotPrCommentsPostOnRemoved, default on) — treat removed snapshots as a change worth posting.

Backend support is in #114302 — must land first.

EME-1046

@linear-code
Copy link
Copy Markdown

linear-code Bot commented Apr 28, 2026

@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Apr 28, 2026
…046)

Adds a nested "Only post when there are changes" sub-toggle to the
snapshot PR comments setting. The sub-toggle is only shown when the
parent toggle is enabled and persists to the new
preprodSnapshotPrCommentsOnlyIfDiff project option.

Depends on the backend serializer change that exposes the
preprodSnapshotPrCommentsOnlyIfDiff field on the project resource.
@runningcode runningcode force-pushed the no/eme-1046-snapshot-pr-comments-only-if-diff-fe branch from eaf348a to 5f7fddf Compare April 28, 2026 08:31
@runningcode runningcode marked this pull request as ready for review April 28, 2026 08:42
@runningcode runningcode requested review from a team as code owners April 28, 2026 08:42
…1046)

Surface dedicated PR-comment toggles for treating added/removed
snapshots as diffs worth posting. These appear nested under
"Only post when there are changes" since they only affect that
gate.

Decoupled from the snapshot status-check fail-on-added/removed
options on the backend (#114302), so projects can tune PR
comments independently of red/green status check semantics.
@runningcode runningcode changed the title feat(preprod): Add only-if-diff toggle to PR comments settings (EME-1046) feat(preprod): Add diff-based PR comment settings (EME-1046) Apr 29, 2026
…1046)

The only-if-diff option is collapsed into the main PR comments
toggle on the backend (#114302). Drop the nested toggle from the
settings panel and surface the post-on-added/post-on-removed
sub-toggles directly under the main switch.
@runningcode runningcode changed the title feat(preprod): Add diff-based PR comment settings (EME-1046) feat(preprod): Add post-on-added/removed PR comment toggles (EME-1046) Apr 29, 2026
</Flex>
<Flex align="center" justify="between">
<Stack gap="xs">
<Text bold>{t('Post on Removed Snapshots')}</Text>
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.

Bug: Rapidly clicking the snapshot comment toggle switches can send two identical requests due to a stale closure, preventing the setting from being toggled back.
Severity: MEDIUM

Suggested Fix

Introduce local state within the SnapshotPrCommentsToggle component to manage the checked state of the switches optimistically. Update this local state immediately on click to provide instant visual feedback and use this state to calculate the payload for the update mutation. This decouples the UI from the asynchronous propagation of the project prop.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: static/app/views/settings/project/preprod/snapshotPrCommentsToggle.tsx#L97

Potential issue: The `Switch` components for toggling snapshot pull request comments are
fully controlled by props from the `project` object. When a switch is clicked, an
asynchronous update is triggered, but the component does not re-render synchronously. If
a user clicks the switch again before the asynchronous state update completes, the
`onClick` handler's closure captures a stale value for the setting. This causes the
toggle logic (e.g., `!postOnAdded`) to compute the same result for both clicks, sending
two identical requests instead of toggling the setting back.

runningcode added a commit that referenced this pull request May 5, 2026
…E-1046) (#114302)

Gives snapshot PR comments the same granular controls as snapshot status
checks. Previously PR comments shared the status-check options for
deciding what counts as a diff; now they have their own independent set.

Here's a link to where these settings will eventually go: 
https://sentry.sentry.io/settings/projects/hackernews-android/snapshots/
This is just the backend for now though

**Drop only-if-diff toggle**

The `sentry:preprod_snapshot_pr_comments_only_if_diff` option is
removed. Enabling PR comments now always implies "post only when there
are changes" — commenting on every build regardless of diffs is no
longer possible.

**Dedicated post-on options**

Four independent project options control which snapshot change types
trigger a PR comment:

| Option | Default |
|--------|---------|
| `post_on_added` | `False` |
| `post_on_removed` | `True` |
| `post_on_changed` | `True` |
| `post_on_renamed` | `False` |

Defaults match the status-check equivalents so no project sees a
behavior change from this decoupling. The task no longer imports from
the status-check module.

The frontend changes (surfacing these toggles in the settings UI) are in
#114130.

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
cleptric pushed a commit that referenced this pull request May 5, 2026
…E-1046) (#114302)

Gives snapshot PR comments the same granular controls as snapshot status
checks. Previously PR comments shared the status-check options for
deciding what counts as a diff; now they have their own independent set.

Here's a link to where these settings will eventually go: 
https://sentry.sentry.io/settings/projects/hackernews-android/snapshots/
This is just the backend for now though

**Drop only-if-diff toggle**

The `sentry:preprod_snapshot_pr_comments_only_if_diff` option is
removed. Enabling PR comments now always implies "post only when there
are changes" — commenting on every build regardless of diffs is no
longer possible.

**Dedicated post-on options**

Four independent project options control which snapshot change types
trigger a PR comment:

| Option | Default |
|--------|---------|
| `post_on_added` | `False` |
| `post_on_removed` | `True` |
| `post_on_changed` | `True` |
| `post_on_renamed` | `False` |

Defaults match the status-check equivalents so no project sees a
behavior change from this decoupling. The task no longer imports from
the status-check module.

The frontend changes (surfacing these toggles in the settings UI) are in
#114130.

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant