mirror of
https://github.com/myronblair/jarvis
synced 2026-07-28 08:43:00 -05:00
fix: address 8 code-review findings in KB topic manager
- Fix 1 (ORDER BY): use table alias t.id to force integer PK ordering; topic_id alias was causing alphabetical sort, breaking rotation offsets - Fix 2 (PDOException): wrap INSERT/UPDATE in try/catch in kb_topic_save; duplicate topic_id now returns clean JSON error instead of HTTP 500 - Fix 3 (XSS/onclick): DEL button passes only t.id; tmDelete looks up name from _topicsData, removing JSON.stringify from onclick attribute - Fix 4 (validation): topic_id must contain at least one [a-z0-9] after sanitization; "@@@" to "___" no longer passes required-field check - Fix 5 (stale logs): guard comment and log messages updated from 20h to 4h to match the actual 14400s threshold - Fix 6 (dedup): tmEsc() replaced with existing esc() throughout; removed duplicate function definition - Fix 7 (toggle rollback): tmToggle uses inline fetch to revert el.checked on API failure instead of leaving the UI in wrong state - Fix 8 (confirm): tmDelete uses openModal confirmation instead of confirm() dialog, consistent with project live-popup convention Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014p87VFec84hNaf2WpvmLrW
This commit is contained in:
+43
-29
@@ -641,20 +641,26 @@ if ($action) {
|
||||
$name = trim($_POST['topic_name'] ?? '');
|
||||
$desc = trim($_POST['description'] ?? '');
|
||||
$act = (int)!empty($_POST['active']);
|
||||
if (!$tid||!$cat||!$name||!$desc) bad('All fields required');
|
||||
if ($id) {
|
||||
JarvisDB::execute(
|
||||
'UPDATE kb_generator_topics SET topic_id=?,category=?,topic_name=?,description=?,active=?,updated_at=NOW() WHERE id=?',
|
||||
[$tid,$cat,$name,$desc,$act,$id]
|
||||
);
|
||||
j(['ok'=>true,'msg'=>'Topic updated']);
|
||||
} else {
|
||||
JarvisDB::execute(
|
||||
'INSERT INTO kb_generator_topics (topic_id,category,topic_name,description,active) VALUES (?,?,?,?,?)',
|
||||
[$tid,$cat,$name,$desc,$act]
|
||||
);
|
||||
j(['ok'=>true,'msg'=>'Topic created']);
|
||||
if (!$tid || !preg_match('/[a-z0-9]/', $tid) || !$cat || !$name || !$desc)
|
||||
bad('All fields required — topic_id must contain at least one letter or digit');
|
||||
try {
|
||||
if ($id) {
|
||||
JarvisDB::execute(
|
||||
'UPDATE kb_generator_topics SET topic_id=?,category=?,topic_name=?,description=?,active=?,updated_at=NOW() WHERE id=?',
|
||||
[$tid,$cat,$name,$desc,$act,$id]
|
||||
);
|
||||
} else {
|
||||
JarvisDB::execute(
|
||||
'INSERT INTO kb_generator_topics (topic_id,category,topic_name,description,active) VALUES (?,?,?,?,?)',
|
||||
[$tid,$cat,$name,$desc,$act]
|
||||
);
|
||||
}
|
||||
} catch (\PDOException $e) {
|
||||
bad(strpos($e->getMessage(), 'Duplicate entry') !== false
|
||||
? 'topic_id already in use — choose a different slug'
|
||||
: 'Database error');
|
||||
}
|
||||
j(['ok'=>true,'msg'=> $id ? 'Topic updated' : 'Topic created']);
|
||||
|
||||
case 'kb_topic_delete':
|
||||
$id = (int)($_POST['id'] ?? 0); if (!$id) bad('Missing id');
|
||||
@@ -2557,16 +2563,16 @@ function tmFilter() {
|
||||
</tr></thead>
|
||||
<tbody>${rows.map(t => `
|
||||
<tr style="border-bottom:1px solid rgba(255,255,255,0.03);font-size:0.65rem">
|
||||
<td style="padding:4px 6px;color:var(--text-dim);font-size:0.57rem">${tmEsc(t.topic_id)}</td>
|
||||
<td style="padding:4px 6px"><span style="background:var(--bg2);padding:1px 5px;border-radius:2px;font-size:0.57rem;white-space:nowrap">${tmEsc(t.category)}</span></td>
|
||||
<td style="padding:4px 6px;max-width:220px;overflow:hidden;text-overflow:ellipsis;white-space:nowrap" title="${tmEsc(t.description)}">${tmEsc(t.topic_name)}</td>
|
||||
<td style="padding:4px 6px;color:var(--text-dim);font-size:0.57rem">${esc(t.topic_id)}</td>
|
||||
<td style="padding:4px 6px"><span style="background:var(--bg2);padding:1px 5px;border-radius:2px;font-size:0.57rem;white-space:nowrap">${esc(t.category)}</span></td>
|
||||
<td style="padding:4px 6px;max-width:220px;overflow:hidden;text-overflow:ellipsis;white-space:nowrap" title="${esc(t.description)}">${esc(t.topic_name)}</td>
|
||||
<td style="padding:4px 6px;text-align:center">
|
||||
<input type="checkbox" ${t.active==1?'checked':''} onchange="tmToggle(${t.id},this)" style="cursor:pointer;accent-color:var(--green)">
|
||||
</td>
|
||||
<td style="padding:4px 6px;text-align:center;color:var(--text-dim);font-size:0.6rem">${t.run_count||0}</td>
|
||||
<td style="padding:4px 6px;text-align:right;white-space:nowrap">
|
||||
<button class="btn btn-sm btn-yellow" style="padding:2px 7px;font-size:0.58rem" onclick="tmEditTopic(${t.id})">EDIT</button>
|
||||
<button class="btn btn-sm btn-red" style="padding:2px 7px;font-size:0.58rem;margin-left:3px" onclick="tmDelete(${t.id},${JSON.stringify(t.topic_name)})">DEL</button>
|
||||
<button class="btn btn-sm btn-red" style="padding:2px 7px;font-size:0.58rem;margin-left:3px" onclick="tmDelete(${t.id})">DEL</button>
|
||||
</td>
|
||||
</tr>`).join('')}
|
||||
</tbody></table>`;
|
||||
@@ -2630,24 +2636,32 @@ async function tmSaveTopic() {
|
||||
}
|
||||
|
||||
async function tmToggle(id, el) {
|
||||
apiPost('kb_topic_toggle', {id}, (res) => {
|
||||
const prev = !el.checked;
|
||||
const fd = new FormData();
|
||||
fd.append('action', 'kb_topic_toggle');
|
||||
fd.append('id', id);
|
||||
try {
|
||||
const r = await fetch(location.href, {method:'POST', body:fd});
|
||||
const d = await r.json();
|
||||
if (d.error) { toast(d.error, 'err'); el.checked = prev; return; }
|
||||
const t = _topicsData.find(x => x.id == id);
|
||||
if (t) t.active = res.active ? 1 : 0;
|
||||
if (t) t.active = d.active ? 1 : 0;
|
||||
tmFilter();
|
||||
});
|
||||
} catch(e) { toast('Toggle failed', 'err'); el.checked = prev; }
|
||||
}
|
||||
|
||||
async function tmDelete(id, name) {
|
||||
if (!confirm('Delete topic "' + name + '"?\nThis cannot be undone.')) return;
|
||||
apiPost('kb_topic_delete', {id}, async () => {
|
||||
toast('Topic deleted', 'ok');
|
||||
await tmLoadTopics();
|
||||
});
|
||||
async function tmDelete(id) {
|
||||
const t = _topicsData.find(x => x.id == id);
|
||||
const name = t?.topic_name || 'this topic';
|
||||
openModal('CONFIRM DELETE',
|
||||
`<div style="font-family:var(--mono);font-size:0.75rem;line-height:2;padding:4px 0">Delete topic:<br>` +
|
||||
`<span style="color:var(--red)">${esc(name)}</span><br>` +
|
||||
`<span style="color:var(--text-dim);font-size:0.65rem">This cannot be undone.</span></div>`,
|
||||
() => { apiPost('kb_topic_delete', {id}, async () => { toast('Topic deleted', 'ok'); await openTopicsManager(); }); },
|
||||
'DELETE');
|
||||
}
|
||||
|
||||
function tmEsc(s) {
|
||||
return String(s||'').replace(/&/g,'&').replace(/</g,'<').replace(/>/g,'>').replace(/"/g,'"');
|
||||
}
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user