Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,21 @@ const mountComponent = (props: Record<string, unknown> = {}) => {
global: {
plugins: [createTestI18n()],
stubs: {
UInput: {
props: ['modelValue', 'placeholder', 'disabled', 'maxlength', 'tabindex'],
emits: ['update:modelValue'],
template: `
<input
type="text"
:value="modelValue"
:placeholder="placeholder"
:disabled="disabled"
:maxlength="maxlength"
:tabindex="tabindex"
@input="$emit('update:modelValue', $event.target.value)"
/>
`,
},
USelectMenu: {
props: ['modelValue', 'items', 'disabled'],
emits: ['update:modelValue'],
Expand Down Expand Up @@ -199,6 +214,17 @@ describe('OnboardingCoreSettingsStep', () => {
languagesError.value = null;
});

it('keeps valid server name controls hidden', async () => {
const { wrapper } = mountComponent();
await flushPromises();

const serverNameLabel = wrapper.findAll('label').find((label) => label.text() === 'Server Name');
const serverNameControl = serverNameLabel?.element.parentElement;
expect(serverNameControl?.classList.contains('hidden')).toBe(true);
expect(serverNameControl?.getAttribute('aria-hidden')).toBe('true');
expect(wrapper.find('input[placeholder="Tower"]').attributes('tabindex')).toBe('-1');
});

it('prefers browser timezone over API on initial setup when draft timezone is empty', async () => {
onboardingStore.completed.value = false;

Expand Down Expand Up @@ -593,7 +619,7 @@ describe('OnboardingCoreSettingsStep', () => {
expect(onComplete).toHaveBeenCalledTimes(1);
});

it('blocks submission with invalid server name', async () => {
it('does not block submission with invalid hidden server name', async () => {
const { wrapper, onComplete } = mountComponent();
await flushPromises();

Expand All @@ -612,8 +638,15 @@ describe('OnboardingCoreSettingsStep', () => {
await submitButton.trigger('click');
await flushPromises();

expect(setCoreSettingsMock).not.toHaveBeenCalled();
expect(onComplete).not.toHaveBeenCalled();
expect(setCoreSettingsMock).toHaveBeenCalledWith({
serverName: 'bad name!',
serverDescription: '',
timeZone: 'UTC',
theme: 'white',
language: 'en_US',
useSsh: false,
});
expect(onComplete).toHaveBeenCalledTimes(1);
});

it('blocks submission with too-long server description', async () => {
Expand Down Expand Up @@ -768,7 +801,7 @@ describe('OnboardingCoreSettingsStep', () => {
expect(onComplete).toHaveBeenCalledTimes(1);
});

it('keeps initialized empty server name invalid even if baseline has a valid name', async () => {
it('uses baseline server name when initialized draft server name is empty', async () => {
draftStore.coreSettingsInitialized = true;
draftStore.serverName = '';
draftStore.serverDescription = '';
Expand All @@ -795,7 +828,14 @@ describe('OnboardingCoreSettingsStep', () => {
await submitButton.trigger('click');
await flushPromises();

expect(setCoreSettingsMock).not.toHaveBeenCalled();
expect(onComplete).not.toHaveBeenCalled();
expect(setCoreSettingsMock).toHaveBeenCalledWith({
serverName: 'TowerBaseline',
serverDescription: '',
timeZone: 'UTC',
theme: 'white',
language: 'en_US',
useSsh: false,
});
expect(onComplete).toHaveBeenCalledTimes(1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,13 @@ describe('OnboardingSummaryStep', () => {
refetchInstalledPluginsMock.mockResolvedValue(undefined);
});

it('marks the server name hidden in the summary card', () => {
const { wrapper } = mountComponent();

const serverNameLabel = wrapper.findAll('span').find((span) => span.text() === 'Server Name');
expect(serverNameLabel?.element.parentElement?.classList.contains('hidden')).toBe(true);
});

it.each([
{
caseName: 'skips install when plugin is already present',
Expand Down
18 changes: 13 additions & 5 deletions web/src/components/Onboarding/steps/OnboardingCoreSettingsStep.vue
Original file line number Diff line number Diff line change
Expand Up @@ -355,8 +355,14 @@ const languageItems = computed(() => {
});

const isLanguageDisabled = computed(() => isLanguagesLoading.value || !!languagesQueryError.value);
const resolveSubmittedServerName = () =>
serverName.value ||
coreSettingsResult.value?.server?.name?.trim() ||
coreSettingsResult.value?.vars?.name?.trim() ||
TRUSTED_DEFAULT_PROFILE.serverName;
Comment on lines +358 to +362
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.

⚠️ Potential issue | 🟡 Minor

Handle whitespace-only server names as empty before fallback resolution.

Line 359 currently treats ' ' as a valid override because it is truthy, so fallback to baseline/default is skipped.

💡 Suggested fix
-const resolveSubmittedServerName = () =>
-  serverName.value ||
+const resolveSubmittedServerName = () =>
+  serverName.value.trim() ||
   coreSettingsResult.value?.server?.name?.trim() ||
   coreSettingsResult.value?.vars?.name?.trim() ||
   TRUSTED_DEFAULT_PROFILE.serverName;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/components/Onboarding/steps/OnboardingCoreSettingsStep.vue` around
lines 358 - 362, resolveSubmittedServerName currently treats whitespace-only
serverName.value as valid; update the resolver to trim and treat
empty/whitespace-only values as falsy before falling back. Specifically,
normalize serverName.value with .trim() and check that the trimmed result is
non-empty, then fall back to coreSettingsResult.value?.server?.name?.trim(),
coreSettingsResult.value?.vars?.name?.trim(), and finally
TRUSTED_DEFAULT_PROFILE.serverName; ensure you reference the
resolveSubmittedServerName function and the serverName and coreSettingsResult
symbols when making the change.


const handleSubmit = async () => {
if (serverNameValidation.value || serverDescriptionValidation.value) {
if (serverDescriptionValidation.value) {
error.value = t('common.error');
return;
}
Expand All @@ -366,7 +372,7 @@ const handleSubmit = async () => {

try {
draftStore.setCoreSettings({
serverName: serverName.value,
serverName: resolveSubmittedServerName(),
serverDescription: serverDescription.value,
timeZone: selectedTimeZone.value,
theme: selectedTheme.value,
Expand Down Expand Up @@ -441,7 +447,8 @@ const isBusy = computed(() => isSaving.value || (props.isSavingStep ?? false));
<!-- Local Hostname Badge -->
<div
v-if="currentHostname"
class="flex items-center gap-1 rounded border border-gray-200 bg-gray-100 px-2.5 py-0.5 text-xs font-semibold text-gray-800 dark:border-gray-700 dark:bg-gray-800 dark:text-gray-200"
class="hidden items-center gap-1 rounded border border-gray-200 bg-gray-100 px-2.5 py-0.5 text-xs font-semibold text-gray-800 dark:border-gray-700 dark:bg-gray-800 dark:text-gray-200"
aria-hidden="true"
>
<GlobeAltIcon class="text-primary h-3 w-3" />
{{ currentHostname }}
Expand All @@ -455,7 +462,7 @@ const isBusy = computed(() => isSaving.value || (props.isSavingStep ?? false));
<!-- Top Grid: Server Identity & Region -->
<div class="mb-8 grid grid-cols-1 gap-x-6 gap-y-8 md:grid-cols-2">
<!-- Server Name -->
<div class="flex flex-col gap-2">
<div class="hidden flex-col gap-2" aria-hidden="true">
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep server-name editable while submit validation is active

Hiding this entire server-name control makes onboarding impossible to complete whenever serverName is empty or invalid (for example, a bad value from draft/API/activation metadata), because submit is still blocked by serverNameValidation in both handleSubmit and the Next button disable condition. Before this change users could correct the value; after this change they can get stuck on this step with no visible way to fix it.

Useful? React with 👍 / 👎.

<label class="text-highlighted text-base font-bold">
{{ t('onboarding.coreSettings.serverName') }}
</label>
Expand All @@ -468,6 +475,7 @@ const isBusy = computed(() => isSaving.value || (props.isSavingStep ?? false));
size="lg"
class="w-full"
:class="{ '!border-red-500 focus:!border-red-500': !!serverNameValidation }"
tabindex="-1"
/>
<p v-if="serverNameValidation" class="text-sm font-medium text-red-500">
{{ serverNameValidation }}
Expand Down Expand Up @@ -619,7 +627,7 @@ const isBusy = computed(() => isSaving.value || (props.isSavingStep ?? false));
<BrandButton
:text="t('onboarding.coreSettings.next')"
class="!bg-primary hover:!bg-primary/90 w-full min-w-[160px] !text-white shadow-md transition-all hover:shadow-lg sm:w-auto"
:disabled="isBusy || !!serverNameValidation || !!serverDescriptionValidation"
:disabled="isBusy || !!serverDescriptionValidation"
:loading="isBusy"
@click="handleSubmit"
:icon-right="ChevronRightIcon"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,8 @@ const handleCancelPowerAction = () => {
<div class="flex flex-col gap-0.5">
<span
v-if="activationCode?.system?.serverName"
class="text-highlighted text-lg font-bold"
class="text-highlighted hidden text-lg font-bold"
aria-hidden="true"
>{{ activationCode.system.serverName }}</span
>
<span v-if="activationCode?.system?.model" class="text-muted font-medium">{{
Expand Down
7 changes: 5 additions & 2 deletions web/src/components/Onboarding/steps/OnboardingSummaryStep.vue
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,6 @@ const handleComplete = async () => {
try {
const promises = [];
const baselineLoaded = isApplyDataReady.value;
const targetCoreSettings = resolveTargetCoreSettings();
let hadNonOptimisticFailures = false;
let hadWarnings = !baselineLoaded;
let hadSshVerificationUncertainty = false;
Expand Down Expand Up @@ -619,6 +618,7 @@ const handleComplete = async () => {
? Boolean(coreSettingsResult.value?.vars?.useSsh || false)
: TRUSTED_DEFAULT_PROFILE.useSsh;
const currentSysModel = baselineLoaded ? coreSettingsResult.value?.vars?.sysModel || '' : '';
const targetCoreSettings = resolveTargetCoreSettings();
const serverNameChanged = baselineLoaded ? targetCoreSettings.serverName !== currentName : false;
const shouldApplyPartnerSysModel = Boolean(
isFreshInstall.value &&
Expand Down Expand Up @@ -1113,7 +1113,10 @@ const handleBack = () => {
</h3>
</div>
<div class="space-y-3">
<div class="flex flex-col gap-1 text-sm sm:flex-row sm:items-start sm:justify-between">
<div
class="hidden flex-col gap-1 text-sm sm:flex-row sm:items-start sm:justify-between"
aria-hidden="true"
>
<span class="text-muted">{{ t('onboarding.coreSettings.serverName') }}</span>
<span class="text-highlighted font-medium break-all sm:text-right">{{ serverName }}</span>
</div>
Expand Down
Loading