Skip to content

Add two-step Undo and Redo to the map editor - #316

Merged
TheWitness merged 64 commits into
Cacti:developfrom
alcatron:feat/editor-undo-redo-20261017
Oct 17, 2026
Merged

TheWitness merged 64 commits into
Cacti:developfrom
alcatron:feat/editor-undo-redo-20261017

Conversation

@alcatron

@alcatron alcatron commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Description

Accidental map moves and deletions currently require manual repair. Add Undo and Redo tiles at the top right of the editor, with two saved steps backward or forward and action-specific tooltips. A new edit clears Redo; unchanged and failed saves do not consume a step.

Duplicated browser tabs receive separate histories; same-tab reloads preserve history. Ownership uses Web Locks with a SharedWorker fallback for HTTP installations.

History covers nodes, links, notes, via points, map properties, colours and raw object configuration. Undoing a node deletion restores its connected map links. Ctrl/Cmd+Z and Ctrl/Cmd+Shift+Z are supported; text fields retain native Undo, and open dialogs must be saved or cancelled first.

Snapshots use private temporary storage outside the web root, separately from Cacti's concurrently written session rows. Existing directory locks, revision checks, INCLUDE/path identities, managed-image checks and atomic writes protect restores. History expires after four hours and is bounded to eight map/tab histories and 8 MiB per session. Image-library files, map-file deletion, Cacti devices/graphs and personal editor preferences are outside this history. No database migration is required.

Includes editor documentation, translations and a feature#307 changelog entry.

How Has This Been Tested?

Local PHP 8.3, browser DOM regressions and a live Safari smoke test:

  • Full PHP suite: 321 tests, 6,291 assertions pass.
  • Required changed-line coverage: 100% (229/229 measured PHP lines).
  • Browser regression covers buttons, shortcuts, CSRF/tab identity, cloned tabs and reloads using both ownership APIs, startup mutation gating and failure fallback, pending saves, native text Undo and stale-map errors. PHP regressions cover expiry before a new save, actual JSON storage size, quota enforcement after a large restore, unchanged structured and raw-dialog saves after Undo (node/link; LF/CRLF/CR), intentional raw formatting/comment edits and renderer failure, untouched colour saves with strict label comparison, missing images referenced through INCLUDE files, included-scale group ordering and restored-title cache synchronization.
  • Manual Safari trial verified node creation, two Undo steps and two Redo steps on an isolated map; the toolbar styling was visually checked against the existing theme.
  • Manifest, translation-template and whitespace checks pass.

Related Issue

Fixes #307.

Motivation and Context

Recover from accidental map edits without manually reconstructing deleted objects or editing configuration files.

Types of changes

  • Bug fix
  • New feature
  • Breaking change

Checklist

  • Code follows the project style.
  • This change requires documentation.
  • Updated editor documentation in this repository.

Release notes for this change are consolidated in #314 under Weathermap 2.0 (security fixes, issue fixes, then features). Merge this PR last, after #309–#314. It includes integration commits for those preceding PRs so the combined CSS, translations and editor behaviour are resolved in advance; its diff narrows to Undo/Redo as the preceding PRs merge. The combined release rehearsal passes 329 PHP tests / 6,401 assertions and all 26 browser fixtures. Undo/Redo patch coverage relative to the preceding release stack is 100% (229/229); full-range coverage is 97.8%, with earlier curved-comment fallback paths uncovered.

@alcatron

Copy link
Copy Markdown
Contributor Author

Addressed the two previously missed findings from the latest review in a5fb6a3:

  • Ownership startup: install the request gate before awaiting ownership. Editing is temporarily inert while the owner is established, and a programmatic map mutation attempted during this interval is aborted before transport. After success, mutations receive the claimed history ID; after failure, normal editing remains available without history. Browser regression verifies a pending-startup save never reaches the server and verifies the failure fallback.
  • Restored map titles: after a successful Undo/Redo, reload the restored map under the existing lock and update weathermap_maps.titlecache only if the title changed, using the same configfile key as normal Map Properties saves. The regression checks title/cache values in both directions and confirms read-only status, node moves and failed restores do not update the cache.

The included-scale ordering finding is also fixed, with an inline reply and resolved thread.

Validation: 321 PHP tests / 6,291 assertions, 100% required PHP patch coverage (229/229 lines), browser history regression, manifest, translation-template and whitespace checks pass. Fresh automated checks are still needed for this commit.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Revision-sensitive filesystem restores and cross-browser tab ownership warrant final human integration verification.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@alcatron

Copy link
Copy Markdown
Contributor Author

Resolved the current develop merge conflicts in a557b17. GitHub now reports this PR as MERGEABLE; #311 is also MERGEABLE. #316 should still be merged last.

Fresh validation after the merge:

  • PHP suite: 329 tests, 6,401 assertions passed.
  • All 26 isolated browser regression fixtures passed.
  • Changed instrumented PHP coverage: 304/304 lines (100%); this metric excludes the editor action/controller files, whose behavior is exercised separately.
  • Manifest, translation-template and whitespace checks passed.
  • Manual Chromium test using the production history JavaScript and real PHP map/history engine: two moves, two Undo steps, two Redo steps, deletion of a node and its connected link, and restoration passed. History ownership persisted across reloads.
  • Manual Safari fallback-path test: copied session storage acquired a separate history owner, two saved moves and two Undo steps passed, with ownership retained across reloads.

The manual browser environment is an isolated disposable map with Cacti authentication/database stubs. It does not substitute for a final authenticated Cacti deployment smoke test. The latest commit also needs its new GitHub checks/review; previous-head check results should not be treated as results for this head.

@alcatron

Copy link
Copy Markdown
Contributor Author

Updated in 13d2aeb after #311 and #314 merged:

  • Merged current develop and resolved the toolbar CSS conflict, keeping the Undo/Redo controls alongside the merged properties styling.
  • Clarified the existing feature#307 entry under 2.0: two-step history per editor tab for saved node, link, note and map-setting edits (PR Add two-step Undo and Redo to the map editor #316). Security/issue/feature ordering and version 2.0 are preserved.
  • Fresh validation: 329 PHP tests / 6,401 assertions and all 26 browser regression fixtures passed. Instrumented PHP patch coverage against current develop is 229/229 lines; editor action/controller files are excluded from this coverage metric. Manifest, translation-template and whitespace checks passed.

This remains the final feature PR for the release. Fresh GitHub checks/review are required on this new head; the authenticated Cacti deployment smoke-test limitation noted above still applies.

@sonarqubecloud

Copy link
Copy Markdown

@TheWitness
TheWitness merged commit aee5ee0 into Cacti:develop Oct 17, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Undo of creating weathermap

4 participants