Skip to content

feat(web-analytics): make heatmap chromium sandbox configurable#60794

Merged
Piccirello merged 1 commit into
masterfrom
feat/heatmap-chromium-sandbox-flag
Jun 2, 2026
Merged

feat(web-analytics): make heatmap chromium sandbox configurable#60794
Piccirello merged 1 commit into
masterfrom
feat/heatmap-chromium-sandbox-flag

Conversation

@Piccirello
Copy link
Copy Markdown
Member

Gate the --no-sandbox launch flag for the local heatmap Chromium behind a new HEATMAP_CHROMIUM_NO_SANDBOX setting. Lets the sandbox be re-enabled per-environment once the runtime permits it.

Gate the --no-sandbox launch flag for the local heatmap Chromium behind a new HEATMAP_CHROMIUM_NO_SANDBOX setting (defaults to passing the flag, preserving current behavior). Lets the sandbox be re-enabled per-environment once the runtime permits it.
@assign-reviewers-posthog assign-reviewers-posthog Bot requested a review from a team May 31, 2026 01:15
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 31, 2026

Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
products/web_analytics/backend/tasks/test/test_heatmap_screenshot.py:564-574
The first test case passes an empty `overrides` dict, so `override_settings(**{})` is a no-op and `settings.HEATMAP_CHROMIUM_NO_SANDBOX` keeps whatever value was loaded from the process environment at Django startup. If `HEATMAP_CHROMIUM_NO_SANDBOX=false` is set in a developer's local `.env` or in a CI job, this case will fail unexpectedly — while the second case `("no_sandbox_enabled", True, True)` already explicitly tests the `True` branch. Either remove the first case (redundant once you have the explicit-`True` case) or, to specifically verify the compile-time default, assert the *settings default* separately rather than relying on the live environment.

```suggestion
    @parameterized.expand(
        [
            ("no_sandbox_enabled", True, True),
            ("no_sandbox_disabled", False, False),
        ]
    )
    def test_no_sandbox_flag_respects_setting(
        self, _name: str, setting_value: bool, should_include: bool
    ) -> None:
        overrides = {"HEATMAP_CHROMIUM_NO_SANDBOX": setting_value}
```

Reviews (1): Last reviewed commit: "feat(web-analytics): make heatmap chromi..." | Re-trigger Greptile

@Piccirello Piccirello added the stamphog Request AI review from stamphog label Jun 2, 2026
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simple, backwards-compatible configurability change with no production risk. The bot's unresolved concern about the first test case not explicitly pinning the setting to True is a minor test-robustness nit, not a showstopper.

@Piccirello Piccirello merged commit 6408f13 into master Jun 2, 2026
296 checks passed
@Piccirello Piccirello deleted the feat/heatmap-chromium-sandbox-flag branch June 2, 2026 15:34
@deployment-status-posthog
Copy link
Copy Markdown

deployment-status-posthog Bot commented Jun 2, 2026

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-06-02 16:31 UTC Run
prod-us ✅ Deployed 2026-06-02 16:57 UTC Run
prod-eu ✅ Deployed 2026-06-02 17:02 UTC Run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stamphog Request AI review from stamphog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant