Repository navigation
feat: Add Get started onboarding dialog #663
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
4ee1b3e
61e6fb6
5b3a043
7ccbbb4
a548a5a
1b62365
c96493c
6d09991
b43bbd3
b5c2f88
44f6bf6
b455108
8f298fd
9f5dfe5
c3666cf
048daab
d40bfda
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |
|
|
||
| from pilot.config import BenchConfig, FirewallRule, S3Config, WafCondition, WafRule, WorkerGroup | ||
| from pilot.config.alert_limit import RESOURCE_LIMIT_FIELDS | ||
| from pilot.config.bench import ONBOARDING_STATES | ||
| from pilot.config.llm import LLMConfig | ||
| from pilot.config.mail import MailConfig | ||
| from pilot.core.alerts import check_mail_credentials | ||
|
|
@@ -39,7 +40,8 @@ def __init__(self, config: BenchConfig, data: dict, mail: MailConfig | None = No | |
| def apply(self) -> str | None: | ||
| if error := self.get_group_type_error(): | ||
| return error | ||
| self._apply_bench() | ||
| if error := self._apply_bench(): | ||
| return error | ||
| self._apply_lite_mode() | ||
| self._apply_letsencrypt() | ||
| self._apply_workers() | ||
|
|
@@ -66,7 +68,7 @@ def get_group_type_error(self) -> str | None: | |
| return f"{name} must be {'a list' if expected is list else 'an object'}." | ||
| return None | ||
|
|
||
| def _apply_bench(self) -> None: | ||
| def _apply_bench(self) -> str | None: | ||
| bench = self.data.get("bench") or {} | ||
| if "http_port" in bench: | ||
| self.config.http_port = int(bench["http_port"]) | ||
|
|
@@ -76,6 +78,11 @@ def _apply_bench(self) -> None: | |
| self.config.default_branch = str(bench["default_branch"]).strip() | ||
| if "allow_developer_mode" in bench: | ||
| self.config.allow_developer_mode = bool(bench["allow_developer_mode"]) | ||
| if "onboarding" in bench: | ||
| if bench["onboarding"] not in ONBOARDING_STATES: | ||
| return f"bench.onboarding must be one of: {', '.join(repr(state) for state in ONBOARDING_STATES)}" | ||
| self.config.onboarding = bench["onboarding"] | ||
| return None | ||
|
Comment on lines
+81
to
+85
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 P1 No tests for the new onboarding state validation The new Prompt for AI agents |
||
|
|
||
| def _apply_lite_mode(self) -> None: | ||
| lite_mode = self.data.get("lite_mode") or {} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| import { expect, type Page, test } from '@playwright/test' | ||
|
|
||
| const mockBench = async (page: Page, onboarding: string) => { | ||
| const updates: object[] = [] | ||
| const created: object[] = [] | ||
|
|
||
| await page.route('**/api/v1/**', async (route) => { | ||
| const request = route.request() | ||
| const path = new URL(request.url()).pathname.replace('/api/v1/', '') | ||
| const method = request.method() | ||
|
|
||
| if (path === 'settings' && method === 'PATCH') updates.push(request.postDataJSON()) | ||
| if (path === 'sites' && method === 'POST') created.push(request.postDataJSON()) | ||
|
|
||
| const responses: Record<string, object> = { | ||
| bootstrap: { mode: 'ready', enabled: true }, | ||
| 'auth/session': { authenticated: true }, | ||
| settings: { | ||
| bench: { onboarding }, | ||
| s3: {}, | ||
| s3_providers: [{ value: 'aws', label: 'AWS', regions: ['us-east-1'] }], | ||
| }, | ||
| 'git/connection': { connected: false, central: { available: false } }, | ||
| sites: method === 'POST' ? { task_id: 'onboarding-task' } : [], | ||
| 'sites/wildcard-domains': { domains: [] }, | ||
| apps: [], | ||
| 'marketplace/apps': [], | ||
| notifications: { data: [], meta: { unread: 0, next_cursor: null } }, | ||
| } | ||
|
|
||
| await route.fulfill({ json: responses[path] ?? {} }) | ||
| }) | ||
|
|
||
| return { updates, created } | ||
| } | ||
|
|
||
| test('Opens on a fresh bench', async ({ page }) => { | ||
| await mockBench(page, '') | ||
| await page.goto('/sites') | ||
|
|
||
| await expect(page.getByRole('dialog').getByRole('heading', { name: 'Connect your GitHub' })).toBeVisible() | ||
| }) | ||
|
|
||
| test('Skips to the site step and opens its task', async ({ page }) => { | ||
| const { updates, created } = await mockBench(page, '') | ||
| await page.goto('/sites') | ||
|
|
||
| const dialog = page.getByRole('dialog') | ||
| await dialog.getByRole('button', { name: 'Skip' }).click() | ||
| await expect(dialog.getByRole('heading', { name: 'Where should backups go?' })).toBeVisible() | ||
|
|
||
| await dialog.getByRole('button', { name: 'Skip' }).click() | ||
| await dialog.getByLabel('Site name').fill('first.localhost') | ||
| await dialog.getByRole('button', { name: 'Create site' }).click() | ||
|
|
||
| await expect(page).toHaveURL(/\/insights\/tasks\/onboarding-task$/) | ||
| await expect(dialog).toBeHidden() | ||
| expect(created).toEqual([{ name: 'first.localhost', apps: [] }]) | ||
| expect(updates).toContainEqual({ bench: { onboarding: 'minimized' } }) | ||
| }) | ||
|
|
||
| test('Stays in the sidebar once minimized', async ({ page }) => { | ||
| await mockBench(page, 'minimized') | ||
| await page.goto('/sites') | ||
|
|
||
| await expect(page.getByRole('button', { name: 'Get started' })).toBeVisible() | ||
| await expect(page.getByRole('dialog')).toHaveCount(0) | ||
| }) | ||
|
|
||
| test('Stays hidden once dismissed', async ({ page }) => { | ||
| await mockBench(page, 'dismissed') | ||
|
|
||
| const settingsLoaded = page.waitForResponse('**/api/v1/settings') | ||
| await page.goto('/sites') | ||
| await settingsLoaded | ||
|
|
||
| await expect(page.getByRole('navigation', { name: 'Main' })).toBeVisible() | ||
| await expect(page.getByRole('button', { name: 'Get started' })).toHaveCount(0) | ||
| await expect(page.getByRole('dialog')).toHaveCount(0) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| <script setup lang="ts"> | ||
| defineProps<{ value: number }>() | ||
| </script> | ||
|
|
||
| <template> | ||
| <svg | ||
| viewBox="0 0 16 16" | ||
| fill="none" | ||
| stroke-width="2" | ||
| class="size-4 -rotate-90" | ||
| aria-hidden="true" | ||
| > | ||
| <circle cx="8" cy="8" r="6" class="stroke-current text-ink-gray-3" /> | ||
|
|
||
| <circle | ||
| cx="8" | ||
| cy="8" | ||
| r="6" | ||
| pathLength="100" | ||
| stroke-linecap="round" | ||
| :stroke-dasharray="`${value} 100`" | ||
| class="stroke-current text-ink-gray-8 transition-[stroke-dasharray] duration-500" | ||
| /> | ||
| </svg> | ||
| </template> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟠 P1 New bench.onboarding validation ships without any test
This adds a new accepted/rejected settings field and a new
str | Nonecontract on_apply_bench, but no test covers it. Every sibling applier that can reject input has one (e.g.tests/admin/backend/api/test_settings_lite_mode.py:25,test_settings_waf.py:53), and the project rule requires tests for behavior changes.Prompt for AI agents
Fix with your agent