From bc23d1a2b720419b399e8f831fba7a845c8abf99 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Tue, 14 Apr 2026 13:46:02 +0200 Subject: [PATCH 1/2] feat(preprod): Add settings link to snapshot PR comments Add a link to the project's snapshot settings page in the snapshot PR comment, matching the existing pattern used in build distribution PR comments. The link points to the mobile-builds settings page with tab=snapshots. Co-Authored-By: Claude Opus 4.6 --- .../preprod/vcs/pr_comments/snapshot_tasks.py | 1 + .../vcs/pr_comments/snapshot_templates.py | 12 +++++- .../pr_comments/test_snapshot_templates.py | 42 ++++++++++++++++--- 3 files changed, 48 insertions(+), 7 deletions(-) diff --git a/src/sentry/preprod/vcs/pr_comments/snapshot_tasks.py b/src/sentry/preprod/vcs/pr_comments/snapshot_tasks.py index 9e505367e58f5d..491d784b04beac 100644 --- a/src/sentry/preprod/vcs/pr_comments/snapshot_tasks.py +++ b/src/sentry/preprod/vcs/pr_comments/snapshot_tasks.py @@ -172,6 +172,7 @@ def create_preprod_snapshot_pr_comment_task( base_artifact_map, changes_map, approvals_map=approvals_map, + project=artifact.project, ) existing_comment_id = find_existing_comment_id(all_for_pr, "snapshots") diff --git a/src/sentry/preprod/vcs/pr_comments/snapshot_templates.py b/src/sentry/preprod/vcs/pr_comments/snapshot_templates.py index db4266446e9206..b6cc0e4bbd40d9 100644 --- a/src/sentry/preprod/vcs/pr_comments/snapshot_templates.py +++ b/src/sentry/preprod/vcs/pr_comments/snapshot_templates.py @@ -2,6 +2,7 @@ from django.utils.translation import gettext_lazy as _ +from sentry.models.project import Project from sentry.preprod.models import PreprodArtifact, PreprodComparisonApproval from sentry.preprod.snapshots.models import PreprodSnapshotComparison, PreprodSnapshotMetrics from sentry.preprod.url_utils import get_preprod_artifact_comparison_url, get_preprod_artifact_url @@ -21,6 +22,7 @@ def format_snapshot_pr_comment( base_artifact_map: dict[int, PreprodArtifact], changes_map: dict[int, bool], approvals_map: dict[int, PreprodComparisonApproval] | None = None, + project: Project | None = None, ) -> str: """Format a PR comment for snapshot comparisons.""" if not artifacts: @@ -86,7 +88,15 @@ def format_snapshot_pr_comment( f" | {status} |" ) - return f"{_HEADER}\n\n{COMPARISON_TABLE_HEADER}" + "\n".join(table_rows) + sections = [f"{_HEADER}\n\n{COMPARISON_TABLE_HEADER}" + "\n".join(table_rows)] + + if project is not None: + settings_url = project.organization.absolute_url( + f"/settings/projects/{project.slug}/mobile-builds/", query="tab=snapshots" + ) + sections.append(f"[⚙️ {project.name} Snapshot Settings]({settings_url})") + + return "\n\n".join(sections) def _name_cell( diff --git a/tests/sentry/preprod/vcs/pr_comments/test_snapshot_templates.py b/tests/sentry/preprod/vcs/pr_comments/test_snapshot_templates.py index 86977281ae2df9..7c554ce2ee7acc 100644 --- a/tests/sentry/preprod/vcs/pr_comments/test_snapshot_templates.py +++ b/tests/sentry/preprod/vcs/pr_comments/test_snapshot_templates.py @@ -76,7 +76,7 @@ def _create_approval(self, artifact: PreprodArtifact) -> PreprodComparisonApprov class FormatSnapshotPrCommentEmptyTest(SnapshotPrCommentTestBase): def test_empty_artifacts_raises(self) -> None: with pytest.raises(ValueError, match="Cannot format PR comment for empty artifact list"): - format_snapshot_pr_comment([], {}, {}, {}, {}) + format_snapshot_pr_comment([], {}, {}, {}, {}, project=self.project) @cell_silo_test @@ -87,7 +87,7 @@ def test_no_metrics_shows_processing(self) -> None: app_id="com.example.app", ) - result = format_snapshot_pr_comment([artifact], {}, {}, {}, {}) + result = format_snapshot_pr_comment([artifact], {}, {}, {}, {}, project=self.project) assert "## Sentry Snapshot Testing" in result assert "Processing" in result @@ -103,6 +103,7 @@ def test_no_comparison_with_base_shows_processing(self) -> None: {}, {head_artifact.id: base_artifact}, {}, + project=self.project, ) assert "Processing" in result @@ -121,6 +122,7 @@ def test_pending_comparison_shows_processing(self) -> None: {head_metrics.id: comparison}, {head_artifact.id: base_artifact}, {}, + project=self.project, ) assert "Processing" in result @@ -139,6 +141,7 @@ def test_processing_comparison_shows_processing(self) -> None: {head_metrics.id: comparison}, {head_artifact.id: base_artifact}, {}, + project=self.project, ) assert "Processing" in result @@ -160,6 +163,7 @@ def test_failed_comparison_shows_failure(self) -> None: {head_metrics.id: comparison}, {head_artifact.id: base_artifact}, {}, + project=self.project, ) assert "Comparison failed" in result @@ -189,6 +193,7 @@ def test_no_changes_shows_unchanged(self) -> None: {head_metrics.id: comparison}, {head_artifact.id: base_artifact}, {}, + project=self.project, ) assert "## Sentry Snapshot Testing" in result @@ -219,6 +224,7 @@ def test_changes_show_needs_approval(self) -> None: {head_metrics.id: comparison}, {head_artifact.id: base_artifact}, {head_artifact.id: True}, + project=self.project, ) assert "Needs approval" in result @@ -246,6 +252,7 @@ def test_approved_shows_approved_status(self) -> None: {head_artifact.id: base_artifact}, {head_artifact.id: True}, approvals_map={head_artifact.id: approval}, + project=self.project, ) assert "Approved" in result @@ -277,6 +284,7 @@ def test_multiple_artifacts(self) -> None: comparisons_map, base_artifact_map, {}, + project=self.project, ) for i in range(3): @@ -299,6 +307,7 @@ def test_section_links_include_artifact_url(self) -> None: {head_metrics.id: comparison}, {head_artifact.id: base_artifact}, {head_artifact.id: True}, + project=self.project, ) assert f"/preprod/snapshots/{head_artifact.id}" in result @@ -323,6 +332,7 @@ def test_zero_counts_are_not_linked(self) -> None: {head_metrics.id: comparison}, {head_artifact.id: base_artifact}, {}, + project=self.project, ) assert "?section=" not in result @@ -339,17 +349,31 @@ def test_table_header_present(self) -> None: {head_metrics.id: comparison}, {}, {}, + project=self.project, ) assert "| Name | Added | Removed | Modified | Renamed | Unchanged | Status |" in result + def test_settings_link_included(self) -> None: + artifact, metrics = self._create_artifact_with_metrics() + + result = format_snapshot_pr_comment( + [artifact], {artifact.id: metrics}, {}, {}, {}, project=self.project + ) + + assert f"/settings/projects/{self.project.slug}/mobile-builds/" in result + assert "tab=snapshots" in result + assert f"{self.project.name} Snapshot Settings" in result + @cell_silo_test class FormatSnapshotPrCommentNoBaseTest(SnapshotPrCommentTestBase): def test_no_base_shows_uploaded_with_count(self) -> None: artifact, metrics = self._create_artifact_with_metrics(app_name="My App", image_count=15) - result = format_snapshot_pr_comment([artifact], {artifact.id: metrics}, {}, {}, {}) + result = format_snapshot_pr_comment( + [artifact], {artifact.id: metrics}, {}, {}, {}, project=self.project + ) assert "## Sentry Snapshot Testing" in result assert "15 uploaded" in result @@ -359,7 +383,9 @@ def test_no_base_shows_uploaded_with_count(self) -> None: def test_no_base_uses_same_table_header(self) -> None: artifact, metrics = self._create_artifact_with_metrics() - result = format_snapshot_pr_comment([artifact], {artifact.id: metrics}, {}, {}, {}) + result = format_snapshot_pr_comment( + [artifact], {artifact.id: metrics}, {}, {}, {}, project=self.project + ) assert "| Name | Added | Removed | Modified | Renamed | Unchanged | Status |" in result @@ -374,7 +400,9 @@ def test_no_base_multiple_artifacts(self) -> None: artifacts.append(artifact) metrics_map[artifact.id] = metrics - result = format_snapshot_pr_comment(artifacts, metrics_map, {}, {}, {}) + result = format_snapshot_pr_comment( + artifacts, metrics_map, {}, {}, {}, project=self.project + ) assert "com.example.app0" in result assert "com.example.app1" in result @@ -384,6 +412,8 @@ def test_no_base_multiple_artifacts(self) -> None: def test_no_base_app_id_shown(self) -> None: artifact, metrics = self._create_artifact_with_metrics(app_id="com.example.myapp") - result = format_snapshot_pr_comment([artifact], {artifact.id: metrics}, {}, {}, {}) + result = format_snapshot_pr_comment( + [artifact], {artifact.id: metrics}, {}, {}, {}, project=self.project + ) assert "`com.example.myapp`" in result From 57da1430e0f7afcc19649f02871dab863e3cd229 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Tue, 14 Apr 2026 13:48:01 +0200 Subject: [PATCH 2/2] ref(preprod): Make project a required param in snapshot PR comment Co-Authored-By: Claude Opus 4.6 --- .../vcs/pr_comments/snapshot_templates.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/sentry/preprod/vcs/pr_comments/snapshot_templates.py b/src/sentry/preprod/vcs/pr_comments/snapshot_templates.py index b6cc0e4bbd40d9..8b15c4f8413eb5 100644 --- a/src/sentry/preprod/vcs/pr_comments/snapshot_templates.py +++ b/src/sentry/preprod/vcs/pr_comments/snapshot_templates.py @@ -22,7 +22,8 @@ def format_snapshot_pr_comment( base_artifact_map: dict[int, PreprodArtifact], changes_map: dict[int, bool], approvals_map: dict[int, PreprodComparisonApproval] | None = None, - project: Project | None = None, + *, + project: Project, ) -> str: """Format a PR comment for snapshot comparisons.""" if not artifacts: @@ -88,15 +89,14 @@ def format_snapshot_pr_comment( f" | {status} |" ) - sections = [f"{_HEADER}\n\n{COMPARISON_TABLE_HEADER}" + "\n".join(table_rows)] + settings_url = project.organization.absolute_url( + f"/settings/projects/{project.slug}/mobile-builds/", query="tab=snapshots" + ) - if project is not None: - settings_url = project.organization.absolute_url( - f"/settings/projects/{project.slug}/mobile-builds/", query="tab=snapshots" - ) - sections.append(f"[⚙️ {project.name} Snapshot Settings]({settings_url})") + table = f"{_HEADER}\n\n{COMPARISON_TABLE_HEADER}" + "\n".join(table_rows) + settings_link = f"[⚙️ {project.name} Snapshot Settings]({settings_url})" - return "\n\n".join(sections) + return f"{table}\n\n{settings_link}" def _name_cell(