From 0d1cdaa34fd3e8680ba665d9a61c3f7c39691343 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Sch=C3=B6ldstr=C3=B6m?= Date: Fri, 29 May 2026 08:05:31 -0300 Subject: [PATCH] fix(gravityforms): surface silent restore failures + disambiguate undo labels + filter inputs on forms-list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three bugs surfaced from a v0.4.0 gds-assistant session: 1. Undo silently lied. RestoreSnapshot::restoreForm / restoreFeed only checked is_wp_error() on the GFAPI return — both GFAPI::update_form and GFAPI::update_feed return literal `false` on validation/save failure without raising a WP_Error. That meant a failed restore flowed through to gds-assistant as success and the chat UI displayed "Undone" while the form stayed unchanged. Now we treat `false` as an explicit failure with a "GFAPI returned false" error message. 2. Audit-log labels were ambiguous after multiple updates. Two consecutive updates on the same form both produced the snapshot label "Revert form 'X' to its previous state" so the user had no way to tell which undo button reverted which change. The label now includes a short delta summary — `(fields: 10 → 5)` when the field count changed, `(changed: title, confirmations)` when only top-level metadata moved, falling back to a UTC timestamp for shape-identical edits. 3. gds/forms-list silently hid disabled / trashed forms. The user said "find form 102" but the model only ever saw active + non-trashed forms and couldn't surface it. Two new optional input flags — include_inactive and include_trashed — let callers widen the listing; defaults remain unchanged for backward compat. Description updated to point the model at them. Companion to a gds-assistant prompt fix that nudges the model to trust read results and consult the new include flags before declaring an item gone. Lint clean (pint). Existing GravityFormsAbility integration tests unchanged in shape — only the audit label was extended, which the tests don't pin. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../GravityForms/GravityFormsAbility.php | 77 +++++++++++++++++-- src/Undo/RestoreSnapshot.php | 17 ++++ 2 files changed, 89 insertions(+), 5 deletions(-) diff --git a/src/Integrations/GravityForms/GravityFormsAbility.php b/src/Integrations/GravityForms/GravityFormsAbility.php index c3128e9..f357d1b 100644 --- a/src/Integrations/GravityForms/GravityFormsAbility.php +++ b/src/Integrations/GravityForms/GravityFormsAbility.php @@ -23,9 +23,16 @@ public static function register(): void HelpAbility::registerAbility('gds/forms-list', [ 'label' => 'List Forms', - 'description' => 'List Gravity Forms. Delegates to GF REST API v2.', + 'description' => 'List Gravity Forms. Defaults to active, non-trashed forms — pass `include_inactive` and/or `include_trashed` to widen the listing when the user is hunting for a form they "remember was here" (e.g. a disabled or trashed contact form).', 'category' => 'gds-content', - 'input_schema' => self::getRestInputSchema('/gf/v2/forms'), + 'input_schema' => [ + 'type' => 'object', + 'properties' => [ + 'include_inactive' => ['type' => 'boolean', 'description' => 'Include inactive forms (status off). Default false.'], + 'include_trashed' => ['type' => 'boolean', 'description' => 'Include trashed forms. Default false.'], + ], + 'additionalProperties' => true, + ], 'output_schema' => ['type' => 'array', 'items' => ['type' => 'object', 'additionalProperties' => true]], 'permission_callback' => '__return_true', 'execute_callback' => [$instance, 'listForms'], @@ -199,8 +206,20 @@ public function listForms(mixed $input = []): array|WP_Error return new WP_Error('gf_not_available', 'Gravity Forms is not active.'); } - // GFAPI::get_forms(active, trash) — default returns only active, non-trashed. - $forms = \GFAPI::get_forms(true, false); + $input = (array) ($input ?? []); + // GFAPI::get_forms($active, $trash): + // $active = null → ignore active filter (returns active + inactive) + // $active = true → active only + // $trash = true → trashed only + // $trash = false → not trashed + // $trash = null → ignore trash filter (both) + $includeInactive = ! empty($input['include_inactive']); + $includeTrashed = ! empty($input['include_trashed']); + + $active = $includeInactive ? null : true; + $trash = $includeTrashed ? null : false; + + $forms = \GFAPI::get_forms($active, $trash); $result = []; foreach ($forms as $form) { @@ -383,7 +402,55 @@ public function updateForm(mixed $input = []): array|WP_Error // ($current was read before the merge, so capture is free). $saved = json_decode(json_encode(\GFAPI::get_form($id)), true) ?: []; - return $this->reversible($saved, 'restore-form', ['id' => $id, 'form' => $current], "Revert form \"{$saved['title']}\" to its previous state"); + // Make consecutive undo entries for the same form distinguishable — + // two updates on a single form would otherwise both read "Revert form + // 'X' to its previous state", leaving the user no way to tell which + // snapshot is which when undoing from the chat UI. + $deltaSummary = self::summarizeUpdateDelta($current, $saved); + $label = "Revert form \"{$saved['title']}\" to its previous state"; + if ($deltaSummary !== '') { + $label .= " ({$deltaSummary})"; + } + + return $this->reversible($saved, 'restore-form', ['id' => $id, 'form' => $current], $label); + } + + /** + * Short human-readable summary of what changed in a forms-update, used to + * disambiguate audit-log labels when the user updates the same form + * multiple times. Picks the most informative dimension(s) — field count + * changes lead; otherwise list which top-level keys differ. + */ + private static function summarizeUpdateDelta(array $before, array $after): string + { + $bits = []; + + $bf = count(is_array($before['fields'] ?? null) ? $before['fields'] : []); + $af = count(is_array($after['fields'] ?? null) ? $after['fields'] : []); + if ($bf !== $af) { + $bits[] = "fields: {$bf} → {$af}"; + } + + $watched = ['title', 'description', 'button', 'confirmations', 'notifications']; + $changed = []; + foreach ($watched as $key) { + $b = $before[$key] ?? null; + $a = $after[$key] ?? null; + if ($b !== $a && json_encode($b) !== json_encode($a)) { + $changed[] = $key; + } + } + if ($changed) { + $bits[] = 'changed: '.implode(', ', $changed); + } + + // Same field count + same watched keys: at least timestamp it so two + // identical-shape edits still produce different labels. + if (! $bits) { + $bits[] = 'saved '.gmdate('H:i:s').' UTC'; + } + + return implode(', ', $bits); } /** diff --git a/src/Undo/RestoreSnapshot.php b/src/Undo/RestoreSnapshot.php index 3c78db1..c409eb8 100644 --- a/src/Undo/RestoreSnapshot.php +++ b/src/Undo/RestoreSnapshot.php @@ -354,10 +354,21 @@ private static function restoreForm(array $data): array|WP_Error if (! $id || ! $form) { return new WP_Error('restore_failed', 'Missing form snapshot.'); } + // GFAPI::update_form returns false on validation/save failure without + // raising a WP_Error — checking only is_wp_error() lets a silent + // failure flow through as "Undone", which is what surfaced the bug + // "undo said done but the field didn't change". Treat false as an + // explicit failure so we don't lie to the user. $result = \GFAPI::update_form($form, $id); if (is_wp_error($result)) { return $result; } + if ($result === false) { + return new WP_Error( + 'restore_failed', + "Failed to restore form {$id}: GFAPI::update_form returned false (validation rejected or save failed).", + ); + } return ['restored' => 'form', 'id' => $id]; } @@ -389,6 +400,12 @@ private static function restoreFeed(array $data): array|WP_Error if (is_wp_error($result)) { return $result; } + if ($result === false) { + return new WP_Error( + 'restore_failed', + "Failed to restore feed {$feedId}: GFAPI::update_feed returned false.", + ); + } // update_feed only restores meta; the form binding / active state / // order are separate properties that the edit could also have changed.