Skip to content

task: set status betterstack page conditionally for staging and production#434

Open
melifaro wants to merge 3 commits intomainfrom
task/conditional-status-page
Open

task: set status betterstack page conditionally for staging and production#434
melifaro wants to merge 3 commits intomainfrom
task/conditional-status-page

Conversation

@melifaro
Copy link
Collaborator

Set conditional Betterstack status pages in Launchpad:

Copilot AI review requested due to automatic review settings February 16, 2026 09:17
@atlantis-platform-engineering
Error: This repo is not allowlisted for Atlantis.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Launchpad GUI frame to show a Betterstack status page link/badge only for staging and production API roots, and adds unit tests to lock in the expected URL selection behavior.

Changes:

  • Added get_status_page_url(api_root) helper to map API root → status page URL (or None for dev/test/unknown).
  • Made the “Check Platform Status” menu item and Betterstack badge conditional on the resolved status URL.
  • Added unit tests covering production/staging/dev/test/unknown API root behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/aignostics/gui/_frame.py Adds environment-based status page URL selection and conditionally renders/refreshes the status UI.
tests/aignostics/gui/frame_test.py Adds unit tests for get_status_page_url across known/unknown API roots.
tests/aignostics/gui/__init__.py Marks the GUI tests folder as a package (consistent with other test modules).

@codecov
Copy link

codecov bot commented Feb 16, 2026

Codecov Report

❌ Patch coverage is 90.32258% with 3 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/aignostics/gui/_frame.py 78.57% 0 Missing and 3 partials ⚠️
Files with missing lines Coverage Δ
src/aignostics/platform/__init__.py 100.00% <ø> (ø)
src/aignostics/platform/_constants.py 100.00% <100.00%> (ø)
src/aignostics/platform/_settings.py 86.70% <100.00%> (+0.98%) ⬆️
src/aignostics/gui/_frame.py 76.92% <78.57%> (-0.77%) ⬇️

... and 18 files with indirect coverage changes

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
CLASSES_FULL_SIZE = f"{CLASSES_FULL_WIDTH} {CLASSES_FULL_HEIGHT}"


def get_status_page_url(api_root: str) -> str | None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it make sense to move this logic to here?

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

ui.run_javascript("document.getElementById('betterstack').src = document.getElementById('betterstack').src;")
# Only refresh the status iframe if it exists (production/staging)
if get_status_page_url(settings().api_root):
ui.run_javascript("document.getElementById('betterstack').src = document.getElementById('betterstack').src;")
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

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

ui.run_javascript unconditionally dereferences document.getElementById('betterstack') when api_root is prod/staging. On initial page load (or during navigation) the timer callback can run before the iframe is in the DOM, which will cause a JS error (null.src). Consider guarding in the JS itself (check element exists) so the refresh is safe regardless of render timing.

Suggested change
ui.run_javascript("document.getElementById('betterstack').src = document.getElementById('betterstack').src;")
ui.run_javascript(
"var iframe = document.getElementById('betterstack');"
"if (iframe) { iframe.src = iframe.src; }"
)

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure whether this is an actual problem or not. Are you able to start the Launchpad against all environments?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@olivermeyer yes, it looks correct against the branch - no Betterstack status page icon for dev and test, different links for staging and prod.

@sonarqubecloud
Copy link

Comment on lines +520 to +532
# Set status_page_url based on environment (independent of auth fields)
if "status_page_url" not in values:
match api_root:
case x if x == API_ROOT_DEV:
values["status_page_url"] = STATUS_PAGE_URL_DEV
case x if x == API_ROOT_TEST:
values["status_page_url"] = STATUS_PAGE_URL_TEST
case x if x == API_ROOT_STAGING:
values["status_page_url"] = STATUS_PAGE_URL_STAGING
case x if x == API_ROOT_PRODUCTION:
values["status_page_url"] = STATUS_PAGE_URL_PRODUCTION
case _:
values["status_page_url"] = None
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not put this into the existing match..case block below? Then you can simply extend the existing tests (e.g. this one for prod).

Copy link
Collaborator

Choose a reason for hiding this comment

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

If we decide to move the status page URL resolution to the existing match..case block in the settings module, we don't need new tests; if we decide otherwise then we do need new tests but these are not in the right place. They should be in tests/aignostics/platform/settings_test.py.

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

Comments