Master data review: spec updates and C1/C2/M3-M6 fixes
Spec (docs/reviewing/master-data.md): - M1: Remove margin from stored product fields (it's a derived frontend value) - M2: Clarify posting window only restricts transaction dates, not master data - M5: Document AccountFormulaManager::delete() as archive, not physical delete Code: - C1: Fix CompanySettingManager property typo (company_id → companyId) — prevented PHP 8.4 dynamic property fatal on all posting window operations - C2: Fix WarehouseManager::deleteWarehouse() guard — was comparing warehouse_name (string) against warehouse id column (no-op); now correctly blocks on storage rows and active stock rows - M3: Remove hard-delete of td_rack_log in deleteStorage() — retain rack history consistent with soft-delete philosophy elsewhere - M4: Add reference guards to ChartOfAccounts::delete() (blocks on GL items, formula items, product account mappings) and DepartmentManager::delete() (blocks on GL items) - M6: Fix CompanySettingManager::handle() partial update — only upsert keys present in the request, not all allowed keys defaulted Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Thanakorn S
co-authored by
Claude Sonnet 4.6
Claude Sonnet 4.6
parent
cb36d3b8fd
commit
dfeb57533a
@@ -19,7 +19,7 @@ class CompanySettingManager
|
||||
{
|
||||
private PDO $pdo;
|
||||
private ?PDO $transactionPdo;
|
||||
private int $company_id;
|
||||
private int $companyId;
|
||||
|
||||
// ── Known keys with their defaults ───────────────────────────────────────
|
||||
private const DEFAULTS = [
|
||||
@@ -316,7 +316,9 @@ class CompanySettingManager
|
||||
$allowed = array_keys(self::DEFAULTS);
|
||||
$incoming = [];
|
||||
foreach ($allowed as $key) {
|
||||
$incoming[$key] = $data[$key] ?? self::DEFAULTS[$key];
|
||||
if (array_key_exists($key, $data)) {
|
||||
$incoming[$key] = $data[$key];
|
||||
}
|
||||
}
|
||||
|
||||
$blocked = $this->blockedChanges($incoming);
|
||||
|
||||
@@ -711,24 +711,37 @@ class WarehouseManager {
|
||||
throw new Exception("Warehouse not found.");
|
||||
}
|
||||
|
||||
// Block if any md_storage references this warehouse by name
|
||||
// Block if any md_storage references this warehouse
|
||||
$sth = $this->pdo->prepare(
|
||||
"SELECT COUNT(*) FROM md_storage
|
||||
WHERE company_id = :company_id
|
||||
AND warehouse = :warehouse_name"
|
||||
WHERE company_id = :company_id
|
||||
AND warehouse = :warehouse_id"
|
||||
);
|
||||
$sth->execute([
|
||||
':company_id' => $this->company_id,
|
||||
':warehouse_name' => $row['warehouse_name'],
|
||||
':company_id' => $this->company_id,
|
||||
':warehouse_id' => $warehouse_id,
|
||||
]);
|
||||
|
||||
if ($sth->fetchColumn() > 0) {
|
||||
if ((int)$sth->fetchColumn() > 0) {
|
||||
throw new Exception(
|
||||
"Cannot delete — warehouse \"{$row['warehouse_name']}\" " .
|
||||
"still has storage locations assigned to it."
|
||||
);
|
||||
}
|
||||
|
||||
// Block if the warehouse's stock table has any rows
|
||||
$stock_table = $this->stockTableNameFromWarehouseId($warehouse_id);
|
||||
$sth = $this->pdo->prepare(
|
||||
"SELECT COUNT(*) FROM `{$stock_table}`
|
||||
WHERE company_id = :company_id"
|
||||
);
|
||||
$sth->execute([':company_id' => $this->company_id]);
|
||||
if ((int)$sth->fetchColumn() > 0) {
|
||||
throw new Exception(
|
||||
"Cannot delete — warehouse \"{$row['warehouse_name']}\" " .
|
||||
"still has stock records. Clear the stock first."
|
||||
);
|
||||
}
|
||||
|
||||
// Append delete event to log
|
||||
$log = json_decode($row['log'] ?? '[]', true) ?: [];
|
||||
$log[] = $this->buildLogEntry('delete');
|
||||
@@ -906,8 +919,8 @@ class WarehouseManager {
|
||||
* Soft-delete a storage record and its associated rack rows.
|
||||
*
|
||||
* Blocks deletion if any rack under this storage_id is occupied.
|
||||
* Also hard-deletes td_rack_log entries for those racks before soft-deleting
|
||||
* the md_rack rows themselves, then soft-deletes md_storage.
|
||||
* Soft-deletes md_rack rows (company_id negation) and retains td_rack_log
|
||||
* for audit purposes. Then soft-deletes md_storage.
|
||||
*
|
||||
* Must be called inside dbTransaction() by the caller.
|
||||
*
|
||||
@@ -949,26 +962,6 @@ class WarehouseManager {
|
||||
);
|
||||
}
|
||||
|
||||
// Collect rack IDs before deletion — needed for rack_log cleanup
|
||||
$sth = $this->pdo->prepare(
|
||||
"SELECT id FROM md_rack
|
||||
WHERE company_id = :company_id
|
||||
AND storage_id = :storage_id"
|
||||
);
|
||||
$sth->execute([
|
||||
':company_id' => $this->company_id,
|
||||
':storage_id' => $storage_id,
|
||||
]);
|
||||
$rack_ids = $sth->fetchAll(PDO::FETCH_COLUMN);
|
||||
|
||||
// Hard-delete td_rack_log for these racks (log records have no independent value)
|
||||
if (!empty($rack_ids)) {
|
||||
$placeholders = implode(',', array_fill(0, count($rack_ids), '?'));
|
||||
$this->pdo->prepare(
|
||||
"DELETE FROM td_rack_log WHERE md_rack_id IN ($placeholders)"
|
||||
)->execute($rack_ids);
|
||||
}
|
||||
|
||||
// Soft-delete all md_rack rows under this storage
|
||||
$this->pdo->prepare(
|
||||
"UPDATE md_rack
|
||||
|
||||
@@ -106,10 +106,45 @@ class ChartOfAccounts
|
||||
public function delete(int $id): void
|
||||
{
|
||||
$sth = $this->pdo->prepare(
|
||||
"UPDATE md_account SET status = 0
|
||||
WHERE company_id = :cid AND id = :id"
|
||||
"SELECT account_code FROM md_account
|
||||
WHERE company_id = :cid AND id = :id LIMIT 1"
|
||||
);
|
||||
$sth->execute([':cid' => $this->companyId, ':id' => $id]);
|
||||
$row = $sth->fetch(PDO::FETCH_ASSOC);
|
||||
if (!$row) throw new Exception('Account not found.');
|
||||
$code = $row['account_code'];
|
||||
|
||||
$sth = $this->pdo->prepare(
|
||||
"SELECT COUNT(*) FROM td_gl_item WHERE account_code = :code LIMIT 1"
|
||||
);
|
||||
$sth->execute([':code' => $code]);
|
||||
if ((int)$sth->fetchColumn() > 0) {
|
||||
throw new Exception("Cannot deactivate — account {$code} has GL journal entries.");
|
||||
}
|
||||
|
||||
$sth = $this->pdo->prepare(
|
||||
"SELECT COUNT(*) FROM md_account_formula_item
|
||||
WHERE company_id = :cid AND account_code = :code LIMIT 1"
|
||||
);
|
||||
$sth->execute([':cid' => $this->companyId, ':code' => $code]);
|
||||
if ((int)$sth->fetchColumn() > 0) {
|
||||
throw new Exception("Cannot deactivate — account {$code} is used in one or more account formulas.");
|
||||
}
|
||||
|
||||
$sth = $this->pdo->prepare(
|
||||
"SELECT COUNT(*) FROM md_product
|
||||
WHERE company_id = :cid
|
||||
AND (sales_account_code = :code OR purchase_account_code = :code) LIMIT 1"
|
||||
);
|
||||
$sth->execute([':cid' => $this->companyId, ':code' => $code]);
|
||||
if ((int)$sth->fetchColumn() > 0) {
|
||||
throw new Exception("Cannot deactivate — account {$code} is mapped to one or more products.");
|
||||
}
|
||||
|
||||
$this->pdo->prepare(
|
||||
"UPDATE md_account SET status = 0
|
||||
WHERE company_id = :cid AND id = :id"
|
||||
)->execute([':cid' => $this->companyId, ':id' => $id]);
|
||||
}
|
||||
|
||||
public function isPostingAccount(string $account_code): bool
|
||||
|
||||
@@ -71,10 +71,17 @@ class DepartmentManager
|
||||
public function delete(int $id): void
|
||||
{
|
||||
$sth = $this->pdo->prepare(
|
||||
"SELECT COUNT(*) FROM td_gl_item WHERE department_id = :id LIMIT 1"
|
||||
);
|
||||
$sth->execute([':id' => $id]);
|
||||
if ((int)$sth->fetchColumn() > 0) {
|
||||
throw new Exception('Cannot deactivate — this department has GL journal entries.');
|
||||
}
|
||||
|
||||
$this->pdo->prepare(
|
||||
"UPDATE md_department SET status = 0
|
||||
WHERE company_id = :cid AND id = :id"
|
||||
);
|
||||
$sth->execute([':cid' => $this->companyId, ':id' => $id]);
|
||||
)->execute([':cid' => $this->companyId, ':id' => $id]);
|
||||
}
|
||||
|
||||
public function getStats(): array
|
||||
|
||||
Reference in New Issue
Block a user