mirror of
https://codeberg.org/PostERG/xamxam.git
synced 2026-05-06 19:19:19 +02:00
Fix two backend correctness issues
- Wrap setThesisJury() in a transaction: the method did a DELETE then multiple INSERTs with no atomicity guarantee. A partial failure (e.g. findOrCreateSupervisor throwing) would leave the jury table with orphaned rows. The fix uses pdo->inTransaction() to avoid nesting when called from within an outer transaction, and performs beginTransaction/commit/rollBack otherwise. - Replace raw PDO query in admin/thanks.php with db->getThesisFiles(): the file listing after TFE submission was manually preparing a SELECT on thesis_files instead of calling the existing Database::getThesisFiles() method. Removes the getPDO() call entirely from that file.
This commit is contained in:
4
TODO.md
4
TODO.md
@@ -408,7 +408,7 @@ Goal: rename the tables and column to the canonical M2M pattern (`tags`, `thesis
|
|||||||
into `$problematique` but the value is **never used** (no matching column, no INSERT reference).
|
into `$problematique` but the value is **never used** (no matching column, no INSERT reference).
|
||||||
Deleted.
|
Deleted.
|
||||||
|
|
||||||
- [ ] **`setThesisJury()` not wrapped in a transaction** — the method does a DELETE then multiple
|
- [x] **`setThesisJury()` not wrapped in a transaction** — the method does a DELETE then multiple
|
||||||
INSERTs with no transaction guard of its own. If called from outside a transaction (e.g. a
|
INSERTs with no transaction guard of its own. If called from outside a transaction (e.g. a
|
||||||
future API endpoint) a partial failure leaves orphaned rows. Wrap the body in
|
future API endpoint) a partial failure leaves orphaned rows. Wrap the body in
|
||||||
`BEGIN … COMMIT / ROLLBACK` (check `$this->pdo->inTransaction()` to avoid nesting).
|
`BEGIN … COMMIT / ROLLBACK` (check `$this->pdo->inTransaction()` to avoid nesting).
|
||||||
@@ -551,7 +551,7 @@ Goal: rename the tables and column to the canonical M2M pattern (`tags`, `thesis
|
|||||||
|
|
||||||
### H — Minor / low-hanging fruit
|
### H — Minor / low-hanging fruit
|
||||||
|
|
||||||
- [ ] **`admin/thanks.php` duplicates `getThesisFiles()` with a raw PDO query** — lines 34–40
|
- [x] **`admin/thanks.php` duplicates `getThesisFiles()` with a raw PDO query** — lines 34–40
|
||||||
manually prepare `SELECT … FROM thesis_files WHERE thesis_id = ?` instead of calling
|
manually prepare `SELECT … FROM thesis_files WHERE thesis_id = ?` instead of calling
|
||||||
`$db->getThesisFiles($thesisId)` which already exists. Replace with the DB method.
|
`$db->getThesisFiles($thesisId)` which already exists. Replace with the DB method.
|
||||||
|
|
||||||
|
|||||||
@@ -25,7 +25,6 @@ if (isset($_GET['id'])) {
|
|||||||
if ($thesisId !== false && $thesisId > 0) {
|
if ($thesisId !== false && $thesisId > 0) {
|
||||||
try {
|
try {
|
||||||
$db = new Database();
|
$db = new Database();
|
||||||
$pdo = $db->getPDO();
|
|
||||||
|
|
||||||
// Get thesis data
|
// Get thesis data
|
||||||
$thesis = $db->getThesis($thesisId);
|
$thesis = $db->getThesis($thesisId);
|
||||||
@@ -33,15 +32,7 @@ if (isset($_GET['id'])) {
|
|||||||
if (!$thesis) {
|
if (!$thesis) {
|
||||||
$error = "TFE non trouvé.";
|
$error = "TFE non trouvé.";
|
||||||
} else {
|
} else {
|
||||||
// Get associated files
|
$files = $db->getThesisFiles($thesisId);
|
||||||
$stmt = $pdo->prepare("
|
|
||||||
SELECT file_type, file_name, file_size, mime_type, uploaded_at
|
|
||||||
FROM thesis_files
|
|
||||||
WHERE thesis_id = ?
|
|
||||||
ORDER BY file_type, uploaded_at
|
|
||||||
");
|
|
||||||
$stmt->execute([$thesisId]);
|
|
||||||
$files = $stmt->fetchAll();
|
|
||||||
}
|
}
|
||||||
} catch (Exception $e) {
|
} catch (Exception $e) {
|
||||||
error_log("Error loading thesis: " . $e->getMessage());
|
error_log("Error loading thesis: " . $e->getMessage());
|
||||||
|
|||||||
@@ -812,19 +812,33 @@ class Database {
|
|||||||
* $juryMembers: array of ['name' => string, 'role' => string, 'is_external' => int]
|
* $juryMembers: array of ['name' => string, 'role' => string, 'is_external' => int]
|
||||||
*/
|
*/
|
||||||
public function setThesisJury(int $thesisId, array $juryMembers): void {
|
public function setThesisJury(int $thesisId, array $juryMembers): void {
|
||||||
$this->pdo->prepare("DELETE FROM thesis_supervisors WHERE thesis_id = ?")->execute([$thesisId]);
|
$alreadyInTransaction = $this->pdo->inTransaction();
|
||||||
$stmt = $this->pdo->prepare("
|
if (!$alreadyInTransaction) {
|
||||||
INSERT INTO thesis_supervisors (thesis_id, supervisor_id, role, is_external, supervisor_order)
|
$this->pdo->beginTransaction();
|
||||||
VALUES (?, ?, ?, ?, ?)
|
}
|
||||||
");
|
try {
|
||||||
foreach ($juryMembers as $order => $member) {
|
$this->pdo->prepare("DELETE FROM thesis_supervisors WHERE thesis_id = ?")->execute([$thesisId]);
|
||||||
$name = trim($member['name'] ?? '');
|
$stmt = $this->pdo->prepare("
|
||||||
if ($name === '') continue;
|
INSERT INTO thesis_supervisors (thesis_id, supervisor_id, role, is_external, supervisor_order)
|
||||||
$supervisorId = $this->findOrCreateSupervisor($name);
|
VALUES (?, ?, ?, ?, ?)
|
||||||
$role = in_array($member['role'], ['president', 'promoteur', 'lecteur'])
|
");
|
||||||
? $member['role'] : 'promoteur';
|
foreach ($juryMembers as $order => $member) {
|
||||||
$isExternal = isset($member['is_external']) ? (int)$member['is_external'] : 0;
|
$name = trim($member['name'] ?? '');
|
||||||
$stmt->execute([$thesisId, $supervisorId, $role, $isExternal, $order + 1]);
|
if ($name === '') continue;
|
||||||
|
$supervisorId = $this->findOrCreateSupervisor($name);
|
||||||
|
$role = in_array($member['role'], ['president', 'promoteur', 'lecteur'])
|
||||||
|
? $member['role'] : 'promoteur';
|
||||||
|
$isExternal = isset($member['is_external']) ? (int)$member['is_external'] : 0;
|
||||||
|
$stmt->execute([$thesisId, $supervisorId, $role, $isExternal, $order + 1]);
|
||||||
|
}
|
||||||
|
if (!$alreadyInTransaction) {
|
||||||
|
$this->pdo->commit();
|
||||||
|
}
|
||||||
|
} catch (\Throwable $e) {
|
||||||
|
if (!$alreadyInTransaction && $this->pdo->inTransaction()) {
|
||||||
|
$this->pdo->rollBack();
|
||||||
|
}
|
||||||
|
throw $e;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user