IDE-321 Feat: Finalize multi-step undo/redo system

[harshsomankar123-tech]

Jira Ticket: https://catrobat.atlassian.net/browse/IDE-321

Description

Implements a robust project-scoped multi-step undo/redo system. This PR focuses exclusively on the core reliability of the undo/redo chain, state persistence, and UI synchronization.

Key Fixes & Features:

• Multi-Step Stability: Fixes stale code.xml snapshots by ensuring project saves before every undo/redo push.
• Race Condition Protection: Adds an isUndoRedoInProgress guard to prevent overlapping async project loads during rapid clicking.
• Correct UI Refresh: Fixes the fragment lookup bug (findFragmentByTag → findFragmentById) so the script view correctly updates after every operation.
• Context Preservation: Ensures sceneName and spriteName are correctly stored in all history entries.
• Memory Safety: Deep-copies variable lists to prevent shared reference mutation between history steps.
• Modern UI: Undo/Redo buttons stay visible but greyed out when no history is available, providing better visual feedback.
• History Management: Prunes snapshots after 1 hour (TTL) and clears history during project delete, rename, or copy.

Screen Recording

Screen.Recording.2026-03-31.at.06.33.02.mov

Screen.Recording.2026-04-18.at.20.17.31.mov

Checklist for this pull request

Please review the contributing guidelines and wiki pages of this repository.

• Include the name of the Jira ticket in the PR’s title
• Include a summary of the changes plus the relevant context
• Choose the proper base branch (develop)
• Confirm that the changes follow the project’s coding guidelines
• Verify that the changes generate no compiler or linter warnings
• Perform a self-review of the changes
• Verify to commit no other files than the intentionally changed ones
• Include reasonable and readable tests verifying the added or changed behavior
• Confirm that new and existing unit tests pass locally
• Check that the commits’ message style matches the project’s guideline
• Stick to the project’s gitflow workflow
• Verify that your changes do not have any conflicts with the base branch
• After the PR, verify that all CI checks have passed
• Post a message in the catroid-stage or catroid-ide Slack channel and ask for a code reviewer

[Copilot]

Pull request overview

Implements a project-scoped multi-step undo/redo mechanism by introducing a dedicated snapshot/history manager, wiring it into ScriptFragment/SpriteActivity, and updating UI/tests to reflect the new enabled/disabled behavior.

Changes:

• Added ProjectUndoManager to manage per-project snapshot stacks (undo/redo) and variable snapshots.
• Updated ScriptFragment and SpriteActivity to drive undo/redo via the new manager and keep menu items visible but disabled when unavailable.
• Added/updated unit + UI tests and ensured undo history is cleared on project delete/rename.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
catroid/src/main/java/org/catrobat/catroid/ui/recyclerview/fragment/ProjectUndoManager.java	New undo/redo snapshot manager with disk-backed snapshots, TTL cleanup, and clear helpers.
catroid/src/main/java/org/catrobat/catroid/ui/recyclerview/fragment/ScriptFragment.java	Integrates multi-step undo/redo, adds async-guard, and refreshes UI after project reload.
catroid/src/main/java/org/catrobat/catroid/ui/SpriteActivity.java	Adds lazy ProjectUndoManager, introduces redo menu handling, and updates menu enable/visibility logic.
catroid/src/main/res/menu/menu_script_activity.xml	Adds redo action and defaults undo/redo to disabled + hidden until prepared.
catroid/src/main/java/org/catrobat/catroid/common/Constants.java	Adds UNDO_DIRECTORY_NAME constant for snapshot storage.
catroid/src/main/java/org/catrobat/catroid/ui/recyclerview/fragment/ProjectListFragment.kt	Clears undo history when deleting/renaming projects from the list.
catroid/src/main/java/org/catrobat/catroid/ui/fragment/ProjectOptionsFragment.kt	Clears undo history on project rename/delete via project options.
catroid/src/androidTest/java/org/catrobat/catroid/uiespresso/ui/fragment/UndoTest.java	Updates existing undo assertions and adds multi-step + rapid-click coverage.
catroid/src/test/java/org/catrobat/catroid/ui/recyclerview/fragment/ProjectUndoManagerTest.java	Adds unit tests for stack limiting/history clearing (currently has issues).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-advanced-security]

detekt found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

[wslany]

I've tested the latest apk and found the following issues:

1. disabling bricks deletes the history: instead, enabling or disabling bricks should be handled as undoable/redoable actions.
2. inserting new bricks is ignored for the history: instead, inserting bricks should be handled as undoable/redoable actions.
3. copying bricks or blocks is ignored for the history: instead, copying bricks or blocks should be handled as undoable/redoable actions.
4. moving bricks or blocks is ignored for the history: instead, moving bricks or blocks should be handled as undoable/redoable actions.

[wslany]

Thanks for the follow-up changes. I reviewed the latest push and it looks like the missing actions are now wired into the undo manager in ScriptFragment: insert, copy, move, and comment in/out now call copyProjectForUndoOption() before mutating the script.

I still see two issues that would be good to address before moving the ticket in PO review:

1. Starting a drag records an undo step even if the user cancels without changing anything

   copyProjectForUndoOption() is called before startMoving(...) for move/copy flows. If the user starts moving a brick and then cancels/back-presses, SpriteActivity.onBackPressed() only calls cancelMove(), but the snapshot remains on the undo stack. This creates a no-op history entry even though no project change was committed.

   Ideally, the undo snapshot should be created only when the move is actually committed, or canceled/removed when the drag is aborted.

2. The new actions are not yet protected by regression tests

   The current script undo tests still mainly cover delete-based undo/redo. I couldn’t find meaningful tests for the newly supported actions:

   • inserting a brick
   • copying a brick
   • copying a block/control structure
   • moving a brick
   • moving a block/control structure
   • commenting/uncommenting bricks
   • canceling a started move without creating a no-op undo entry

   Since this undo/redo system will likely be extended later to project-level changes such as actors/objects, scenes, groups, looks, and sounds, it would be valuable to add explicit regression tests now for each newly supported script action. That would make the current behavior much safer to extend without accidentally breaking existing undo/redo coverage.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[wslany]

Thanks for the follow-up changes. I reviewed the latest push, and the earlier drag-cancel bug looks fixed now: the undo snapshot for moves is deferred until the move is actually committed, so canceling a drag no longer leaves a no-op undo entry in history.

I still see two remaining issues before I’d call this fully ready:

1. The new regression tests do not exercise the production wiring they are meant to protect

   The new ScriptUndoRegressionTest cases drive ProjectUndoManager directly via pushState() / popUndo(), but they do not invoke the actual production paths in ScriptFragment / BrickListView such as:

   • pendingMoveUndoSnapshot
   • onMoveCommitted()
   • insert handlers
   • copy handlers
   • comment in/out handlers
   • real drag-start / drag-cancel flows

   Because of that, these tests would still pass even if the integration in the UI layer were broken. In particular, the cancel-move tests only verify the case where no snapshot was pushed at all, rather than simulating an actual started move that gets canceled before commit.

   I think we still need at least one meaningful integration-style regression test per newly supported script action:

   • insert brick
   • copy brick
   • copy control structure / block
   • move brick
   • move control structure / block
   • comment out / comment in
   • start move and cancel without creating an undo entry

2. One touched file still has the old copyright year

   BrickListView.kt was modified in this PR, but its header still says:

   Copyright (C) 2010-2025 The Catrobat Team

   Since this file is touched in 2026, it should be updated to:

   Copyright (C) 2010-2026 The Catrobat Team

[wslany]

Thanks!!! One small, easy to fix remaining test gap: The new ScriptUndoIntegrationTest is a strong improvement and now exercises the real production wiring for copy, move, comment, delete, and cancel-move. But the original missing-action list also included inserting new bricks, and there is still no integration test here for the addBrick() path. Since insert has its own undo wiring in ScriptFragment and is one of the actions this PR claims to support, it still needs a real UI-level undo/redo regression test to keep that path protected.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud