140 lines
6.2 KiB
Markdown
140 lines
6.2 KiB
Markdown
---
|
|
task: Fix accounting theme switch+apply — switching theme should auto-seed accounts, apply should not silently fail
|
|
cycles: 5
|
|
context: true
|
|
private: false
|
|
started: 2026-05-29T00:00:00Z
|
|
finished: 2026-05-29T00:00:30Z
|
|
---
|
|
|
|
## files
|
|
- `resources/js/Pages/ManageAccounts.vue` [lines 457-491] — switchTheme() and reapplyTheme() JS functions
|
|
- `app/Http/Controllers/Accounting/AccountingController.php` [lines 677-715] — setTheme() and applyTheme() controllers; also getAccountsTree() lines 78-109 has a scoping bug
|
|
- `app/Support/AccountingTheme.php` [lines 201-276] — static apply() / applyNode() logic
|
|
|
|
## steps
|
|
|
|
### Bug 1 — "Switch" does not seed accounts (primary UX failure)
|
|
1. In `ManageAccounts.vue` `switchTheme()` (line ~457), after `POST /admin/accounting/theme/set` succeeds, immediately call `POST /admin/accounting/theme/apply` (reuse `reapplyTheme()` logic inline or call it).
|
|
Concrete change: after `await loadThemes(); await fetchAll();` inside the `if (res.data?.success)` block, also call `await reapplyTheme()` — OR refactor: make `switchTheme()` call a combined backend endpoint.
|
|
|
|
Simplest safe fix: after confirming `setTheme` success, call `reapplyTheme()` directly:
|
|
```js
|
|
if (res.data?.success) {
|
|
await setThemeKey(selectThemeKey.value); // already done
|
|
await reapplyTheme(); // NEW: seed accounts for the newly-active theme
|
|
}
|
|
```
|
|
Since `reapplyTheme()` reads the current theme from the backend (not a parameter), and `setTheme` already saved it, the order is correct.
|
|
|
|
### Bug 2 — `getAccountsTree()` discards scope on grandchildren (silent data loss)
|
|
2. In `AccountingController.php` `getAccountsTree()` (lines 100-102), the innermost `with()` closure does:
|
|
```php
|
|
$this->scopeAccounts($q2, $storeIds, $isBig3); // return value DISCARDED
|
|
```
|
|
Fix: capture the return value:
|
|
```php
|
|
$q2 = $this->scopeAccounts($q2, $storeIds, $isBig3);
|
|
```
|
|
Line 102 — change `$this->scopeAccounts($q2, $storeIds, $isBig3);` → `$q2 = $this->scopeAccounts($q2, $storeIds, $isBig3);`
|
|
|
|
### Bug 3 — `applyTheme()` has no error handling; DB failures are silent 500s
|
|
3. In `AccountingController.php` `applyTheme()` (line ~696), wrap the `AccountingTheme::apply()` call in try/catch:
|
|
```php
|
|
try {
|
|
$stats = \App\Support\AccountingTheme::apply();
|
|
} catch (\Throwable $e) {
|
|
return response()->json([
|
|
'success' => false,
|
|
'message' => 'Failed to apply theme: ' . $e->getMessage(),
|
|
], 422);
|
|
}
|
|
```
|
|
|
|
### Bug 4 — switchTheme UX: don't allow re-clicking Switch on already-active theme after apply
|
|
4. After `reapplyTheme()` completes inside `switchTheme()`, call `loadThemes()` once more so `themeInfo.current` is refreshed and the Switch button re-disables correctly. (If `reapplyTheme()` already calls `loadThemes()`, this is automatic — just verify no double-call confusion.)
|
|
|
|
## context
|
|
|
|
### ManageAccounts.vue switchTheme() (lines 457-473):
|
|
```js
|
|
async function switchTheme() {
|
|
if (!selectThemeKey.value || selectThemeKey.value === themeInfo.value?.current) return;
|
|
switchingTheme.value = true;
|
|
try {
|
|
const res = await axios.post('/admin/accounting/theme/set', { key: selectThemeKey.value });
|
|
if (res.data?.success) {
|
|
await loadThemes();
|
|
await fetchAll();
|
|
// BUG: no apply call here — accounts never seeded
|
|
} else {
|
|
showNotice(res.data?.message || 'Could not switch theme.', { variant: 'danger' });
|
|
}
|
|
} catch (e) {
|
|
showNotice(e.response?.data?.message || 'Could not switch theme.', { variant: 'danger' });
|
|
} finally {
|
|
switchingTheme.value = false;
|
|
}
|
|
}
|
|
```
|
|
|
|
### ManageAccounts.vue reapplyTheme() (lines 475-491):
|
|
```js
|
|
async function reapplyTheme() {
|
|
applyingTheme.value = true;
|
|
try {
|
|
const res = await axios.post('/admin/accounting/theme/apply', {});
|
|
if (res.data?.success) {
|
|
showNotice(res.data.message || 'Theme applied.', { variant: 'success', title: 'Theme Applied' });
|
|
await loadThemes();
|
|
await fetchAll();
|
|
} else {
|
|
showNotice(res.data?.message || 'Apply failed.', { variant: 'danger' });
|
|
}
|
|
} catch (e) {
|
|
showNotice(e.response?.data?.message || 'Apply failed.', { variant: 'danger' });
|
|
} finally {
|
|
applyingTheme.value = false;
|
|
}
|
|
}
|
|
```
|
|
|
|
### AccountingController.php getAccountsTree() children scope (lines 96-105):
|
|
```php
|
|
->with(['children' => function ($q) use ($storeIds, $isBig3) {
|
|
$q = $q->where('is_active', true);
|
|
$q = $this->scopeAccounts($q, $storeIds, $isBig3); // ✓ captured
|
|
$q->with(['children' => function ($q2) use ($storeIds, $isBig3) {
|
|
$q2 = $q2->where('is_active', true);
|
|
$this->scopeAccounts($q2, $storeIds, $isBig3); // ✗ BUG: return discarded
|
|
}]);
|
|
}])
|
|
```
|
|
|
|
### AccountingController.php applyTheme() (lines 696-715):
|
|
```php
|
|
public function applyTheme(Request $request)
|
|
{
|
|
if (!UserPermissions::isActionPermitted(...)) {
|
|
return ResponseHelper::returnUnauthorized();
|
|
}
|
|
$stats = \App\Support\AccountingTheme::apply(); // no try-catch
|
|
return response()->json(['success' => true, 'message' => ..., 'data' => $stats]);
|
|
}
|
|
```
|
|
|
|
### AccountingTheme::apply() tree source:
|
|
- Reads `Config::get('accounting.themes', [])` from `config/accounting/themes.php`
|
|
- `banana_trading` key has `tree` with 4 root nodes (Sales, Supplier Purchases, Delivery & Logistics, Operating Expenses) + leaf children
|
|
- `blank` key has `'tree' => []` — if `current()` falls back to `blank`, apply creates 0 accounts
|
|
|
|
### Scope for Big3 vs store-level:
|
|
- `scopeAccounts($query, $storeIds, $isBig3)`: Big3 → `whereNull('store_id')`, store-level → `whereIn('store_id', $storeIds)`
|
|
- `applyNode()` creates accounts with `store_id` absent (NULL) → correct for Big3 global chart
|
|
- Root query: `whereNull('parent_id') AND is_active = 1 AND store_id IS NULL` → should match theme-applied accounts
|
|
|
|
## notes
|
|
- dictionary: ai-docs/dictionary.md
|
|
- linters: none detected
|
|
- constraints: Do NOT delete existing accounts; apply is idempotent/additive. Do NOT change the `setTheme` backend — it is correct as-is (just saves key). The frontend fix in `switchTheme()` is the primary fix. Wrap `reapplyTheme()` call inside `switchTheme()` with `switchingTheme.value = true` still active so the button stays disabled during the full switch+apply operation.
|