Skip to content

feat: reorganize settings system configuration#8777

Open
Soulter wants to merge 1 commit into
AstrBotDevs:masterfrom
Soulter:codex/settings-system-config-ui
Open

feat: reorganize settings system configuration#8777
Soulter wants to merge 1 commit into
AstrBotDevs:masterfrom
Soulter:codex/settings-system-config-ui

Conversation

@Soulter

@Soulter Soulter commented Jun 14, 2026

Copy link
Copy Markdown
Member

Summary

  • move system configuration into the Settings page with grouped navigation and auto-save behavior
  • restyle settings controls, cleanup actions, OpenAPI management, and GitHub proxy selector
  • remove the legacy migration dialog/API surface and regenerate related frontend API/openapi assets

Tests

  • pnpm typecheck
  • ruff format .
  • ruff check . --exclude packages

Note: running ruff check . without exclusions is currently blocked by the untracked local packages/ directory.

Summary by Sourcery

Reorganize the dashboard settings experience with grouped navigation and integrated system configuration, while removing the legacy migration flow and related API surface.

New Features:

  • Introduce a multi-section Settings layout that surfaces system configuration alongside appearance, network, security, maintenance, and OpenAPI management with grouped navigation.
  • Add auto-loading and auto-saving of system configuration from the backend, including restart prompts and two-factor confirmation for sensitive changes.

Enhancements:

  • Restyle settings-related controls including theme selection, sidebar customization, backup/restart actions, proxy selection, and T2I template editor buttons for a more consistent UI.
  • Simplify the storage cleanup panel into a compact summary row with a contextual cleanup menu.
  • Update icon subsets, confirmation dialog styles, and sidebar navigation targets to align with the new settings structure.

Chores:

  • Remove the legacy migration dialog, i18n resources, backend migration endpoints, and OpenAPI schema/types, along with associated service wiring and version flags.
  • Redirect legacy system configuration routes and hashes to the new Settings system configuration section and regenerate the frontend OpenAPI client artifacts.

@dosubot dosubot Bot added size:XXL This PR changes 1000+ lines, ignoring generated files. area:webui The bug / feature is about webui(dashboard) of astrbot. labels Jun 14, 2026

@sourcery-ai sourcery-ai Bot left a comment

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.

Hey - I've found 1 issue, and left some high level feedback:

  • The new system config auto-save logic relies on timeouts (systemConfigAutoSaveTimer) but does not clear them on component unmount; consider adding an onUnmounted hook to clear any pending timers to avoid saves firing after navigation.
  • The settings section selection is only read from window.location.hash on mount and is never written back or updated on hash changes; if deep-linking and browser back/forward navigation between sections is desirable, consider syncing activeSettingsSection to the URL hash and listening for hashchange events.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The new system config auto-save logic relies on timeouts (`systemConfigAutoSaveTimer`) but does not clear them on component unmount; consider adding an `onUnmounted` hook to clear any pending timers to avoid saves firing after navigation.
- The settings section selection is only read from `window.location.hash` on mount and is never written back or updated on hash changes; if deep-linking and browser back/forward navigation between sections is desirable, consider syncing `activeSettingsSection` to the URL hash and listening for `hashchange` events.

## Individual Comments

### Comment 1
<location path="dashboard/src/views/Settings.vue" line_range="498-503" />
<code_context>
+const configSave2faSaving = ref(false);
+const configSave2faRotationHint = ref('');
+const configSavePendingData = ref(null);
+const systemConfigAutoSaveTimer = ref(null);
+const activeSettingsSection = ref('general');
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Auto-save timer should be cleared on component unmount to avoid stray saves and leaks

The `systemConfigAutoSaveTimer` timeout is never cleared on unmount. If the user navigates away while a save is scheduled, the callback can still run against a destroyed view and the timer ref will persist. Please add an `onBeforeUnmount`/`onUnmounted` hook to `clearTimeout(systemConfigAutoSaveTimer.value)` and reset the ref to `null`.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +498 to 503
const systemConfigAutoSaveTimer = ref(null);
const activeSettingsSection = ref('general');

const apiKeyExpiryOptions = computed(() => [
{ title: tm('apiKey.expiryOptions.day1'), value: 1 },
{ title: tm('apiKey.expiryOptions.day7'), value: 7 },

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.

issue (bug_risk): Auto-save timer should be cleared on component unmount to avoid stray saves and leaks

The systemConfigAutoSaveTimer timeout is never cleared on unmount. If the user navigates away while a save is scheduled, the callback can still run against a destroyed view and the timer ref will persist. Please add an onBeforeUnmount/onUnmounted hook to clearTimeout(systemConfigAutoSaveTimer.value) and reset the ref to null.

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request removes the legacy database migration assistant and restructures the Settings page into a tabbed layout, integrating system configuration options directly with auto-save and 2FA support. Feedback on these changes highlights a potential UI soft-lock in Settings.vue when cancelling the 2FA dialog, as only TOTP settings are reverted instead of the entire configuration. Additionally, it is recommended to clear the auto-save timer in an onUnmounted hook to prevent background API calls, and to reduce the excessively large text-h3 typography class used for the title in ConfirmDialog.vue.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +774 to +791
const handleConfigSave2faCancel = () => {
if (systemConfigLastSavedSnapshot.value && systemConfigData.value?.dashboard?.totp) {
try {
const savedConfig = JSON.parse(systemConfigLastSavedSnapshot.value);
const savedTotp = savedConfig?.dashboard?.totp;
if (savedTotp) {
systemConfigData.value.dashboard.totp.enable = savedTotp.enable;
systemConfigData.value.dashboard.totp.secret = savedTotp.secret;
systemConfigData.value.dashboard.totp.recovery_code_hash = savedTotp.recovery_code_hash;
}
} catch (_) {
// Ignore invalid snapshots.
}
}
configSavePendingData.value = null;
configSave2faError.value = '';
configSave2faDialogVisible.value = false;
};

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.

high

When the user cancels the 2FA dialog, only the TOTP-related settings are reverted. If other settings (like timezone or proxy) were changed and triggered the 2FA prompt, they will remain modified in systemConfigData.value. This keeps systemConfigHasChanges.value as true, causing the auto-save timer or focusout events to repeatedly trigger the 2FA dialog, soft-locking the UI. Reverting the entire systemConfigData.value to the last saved snapshot on cancellation solves this robustly.

const handleConfigSave2faCancel = () => {
    if (systemConfigLastSavedSnapshot.value) {
        try {
            systemConfigData.value = JSON.parse(systemConfigLastSavedSnapshot.value);
        } catch (_) {
            // Ignore invalid snapshots.
        }
    }
    configSavePendingData.value = null;
    configSave2faError.value = '';
    configSave2faDialogVisible.value = false;
};

<v-dialog v-model="isOpen" max-width="400">
<v-card>
<v-card-title class="text-h6">{{ title }}</v-card-title>
<v-card-title class="text-h3 pl-6 pt-4">{{ title }}</v-card-title>

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.

medium

The typography class text-h3 is extremely large in Vuetify 3 (3rem / 48px). For a standard confirmation dialog title, this will likely cause awkward wrapping and layout issues, especially on mobile devices. Consider using a more standard size like text-h5 or text-h6.

      <v-card-title class="text-h5 pl-6 pt-4">{{ title }}</v-card-title>

</template>

<script setup>
import { computed, onMounted, ref, watch } from 'vue';

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.

medium

Import onUnmounted from vue to ensure that the auto-save timer is properly cleared when the component is destroyed, preventing potential memory leaks and background API calls.

import { computed, onMounted, onUnmounted, ref, watch } from 'vue';

Comment on lines +910 to 924
onMounted(async () => {
await Promise.all([loadApiKeys(), loadSystemConfig()]);
const hash = window.location.hash;
if (hash.includes('settings-appearance')) {
activeSettingsSection.value = 'appearance';
} else if (hash.includes('settings-network')) {
activeSettingsSection.value = 'network';
} else if (hash.includes('settings-security')) {
activeSettingsSection.value = 'security';
} else if (hash.includes('settings-maintenance')) {
activeSettingsSection.value = 'maintenance';
} else if (hash.includes('settings-openapi')) {
activeSettingsSection.value = 'openapi';
}
});

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.

medium

Add an onUnmounted hook to clear the systemConfigAutoSaveTimer when the component is unmounted. This prevents background timeouts from firing and attempting to save configuration after the user has navigated away from the Settings page.

onMounted(async () => {
    await Promise.all([loadApiKeys(), loadSystemConfig()]);
    const hash = window.location.hash;
    if (hash.includes('settings-appearance')) {
        activeSettingsSection.value = 'appearance';
    } else if (hash.includes('settings-network')) {
        activeSettingsSection.value = 'network';
    } else if (hash.includes('settings-security')) {
        activeSettingsSection.value = 'security';
    } else if (hash.includes('settings-maintenance')) {
        activeSettingsSection.value = 'maintenance';
    } else if (hash.includes('settings-openapi')) {
        activeSettingsSection.value = 'openapi';
    }
});

onUnmounted(() => {
    if (systemConfigAutoSaveTimer.value) {
        clearTimeout(systemConfigAutoSaveTimer.value);
    }
});

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

Labels

area:webui The bug / feature is about webui(dashboard) of astrbot. size:XXL This PR changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant