fix: Restore actually works now — handler was checking wrong $_POST key (v3.4.2)

v3.4.1 added a redirect to the Restore handler thinking that was
the missing piece. It wasn't — the entire if-block was dead code.

The per-row Restore form in inc/wp-notes-display.php sends a
hidden $_POST['note_id'] (singular) when clicked. The handler in
wp_notes_page_callback() was checking for $_POST['done_ids']
(plural), an array of IDs from bulk-action checkboxes that were
removed back in v3.1.0. The mismatch meant the handler's
isset($_POST['done_ids']) guard was always false → handler body
never ran → clicking Restore was a no-op all the way back to
v3.1.0.

Pre-v3.4.0 this was masked because the page re-rendered with
both Active and Completed sections visible, so users might assume
they'd misclicked. v3.4.0's single-pane tab render made the
no-op symptom unmissable; v3.4.1's redirect was a phantom fix
because it lived inside the unreachable block.

Fix: rewrite the handler to match the working single-note
mark-done handler pattern that lives directly above it —
isset(note_id), absint, isset($done_notes[$note_id]) lookup,
move with restored_by annotation, redirect to Active tab. Adds
a load-bearing comment recording the bug history so future-
Claude doesn't reintroduce the dead-code structure.

Lesson recorded in the changelog: when a handler appears to
"do nothing", verify the $_POST keys match BEFORE assuming the
issue is downstream (missing redirect, failed update, etc.).
This commit is contained in:
2026-05-26 09:10:57 +01:00
parent 431c31a95b
commit 9df37a35e8
2 changed files with 71 additions and 28 deletions
+35 -28
View File
@@ -5,7 +5,7 @@
* Plugin Name: Logbook
* Plugin URI: https://icanhelp.ie/wp-notes
* Description: A lightweight task & logbook plugin for WordPress. Log your daily work, mark tasks done, and keep a tidy record inside the dashboard. Perfect for freelancers showing clients what's been delivered and students proving work to teachers.
* Version: 3.4.1
* Version: 3.4.2
* Requires at least: 5.0
* Requires PHP: 7.2
* Author: IR240474
@@ -33,7 +33,7 @@ if (!isset($wp_notes_init)) {
$wp_notes_init = true;
// Plugin Constants
if (!defined('WP_NOTES_VERSION')) define('WP_NOTES_VERSION', '3.4.1');
if (!defined('WP_NOTES_VERSION')) define('WP_NOTES_VERSION', '3.4.2');
if (!defined('WP_NOTES_FILE')) define('WP_NOTES_FILE', __FILE__);
if (!defined('WP_NOTES_PATH')) define('WP_NOTES_PATH', plugin_dir_path(__FILE__));
if (!defined('WP_NOTES_URL')) define('WP_NOTES_URL', plugin_dir_url(__FILE__));
@@ -1336,37 +1336,44 @@ function wp_notes_handle_actions() {
exit;
}
// Handle restore note action
if (isset($_POST['restore_note']) && isset($_POST['done_ids'])) {
// Handle restore note action (per-row form on the Completed tab).
//
// v3.4.2 fix: this handler used to expect $_POST['done_ids'] — an
// array of IDs from bulk-action checkboxes that were removed in
// v3.1.0. The per-row Restore form in inc/wp-notes-display.php
// sends a hidden $_POST['note_id'] (singular) instead, so the old
// handler's isset($_POST['done_ids']) check was always false and
// the handler body never executed. Clicking Restore therefore
// looked like it did nothing — David's "the complete log does not
// return to active." The v3.4.1 redirect added below was correct
// but never reached because the handler was already dead code.
//
// Rewritten to match the working mark-done single-note handler
// pattern: expect note_id (singular), absint() it, isset() lookup
// in $done_notes, move with the restored_by annotation, then
// redirect to the Active tab where the restored note now lives.
if (isset($_POST['restore_note']) && isset($_POST['note_id'])) {
$notes = get_option('wp_notes', array());
$done_notes = get_option('wp_done_notes', array());
$note_id = absint($_POST['note_id']);
$current_user = wp_get_current_user();
$new_done_notes = array();
foreach ($done_notes as $key => $note) {
if (in_array($key, $_POST['done_ids'])) {
$note['last_modified'] = current_time('mysql');
$note['restored_by'] = $current_user->display_name;
$notes[] = $note;
} else {
$new_done_notes[] = $note;
}
if (isset($done_notes[$note_id])) {
$note = $done_notes[$note_id];
$note['last_modified'] = current_time('mysql');
$note['restored_by'] = $current_user->display_name;
$notes[] = $note;
unset($done_notes[$note_id]);
update_option('wp_notes', $notes);
update_option('wp_done_notes', $done_notes);
// Redirect to the Active tab — without this, the page falls
// through and re-renders with the URL's current ?tab=completed,
// so the restored note (now on Active) would be invisible.
wp_redirect(admin_url('admin.php?page=wp-notes'));
exit;
}
update_option('wp_notes', $notes);
update_option('wp_done_notes', $new_done_notes);
// v3.4.1 fix: redirect to the Active tab so the restored note is
// visible in its new home. Without this redirect, the page falls
// through and re-renders with the URL's current ?tab=completed,
// so the user sees Completed (minus the now-restored note) and
// can't see the restored note on Active. The other action
// handlers above (new note, mark-as-done single, mark-as-done
// bulk) all redirect; restore was the odd one out. Pre-v3.4.0
// this was masked because both sections rendered on the same
// page — the v3.4.0 single-pane tab render exposed it.
wp_redirect(admin_url('admin.php?page=wp-notes'));
exit;
}
// Handle edit note action