Files Recycle Bin PR Review

Files recycle bin - PR review

PR #353 by @cloudcogsio · Issue #293 · Verdict: merge + fix · 5.4.0

Files gets a Recycle bin: restore what was deleted, delete it for good, empty it, and have it emptied for you after a period you choose. User page: Files § Deleting and the recycle bin. Decisions: Recycle bin - design. How it works: Recycle bin - Developer Guide.

This is the second of the series @cloudcogsio proposed after S3 storage (PR review).

✅ What was good - keep it this way

What	Why it matters
One service for every rule (FilesRecycleService); the endpoints and the job only pass input in and out	One place to read, one place to change
Permissions as they were before the delete (levelsIncludingDeleted()), with the same resolver as everything else	A deleted folder is judged exactly as it would have been live
Hidden subfolders kept out of Look inside, the counts and Delete permanently	The bin never shows - or lets anyone destroy - what they could not see
Stored keys recorded in the same transaction as the rows (files_purge_pending)	A killed request or a deploy forgets nothing; the job finishes the work
Never hangs on slow storage - asked once per purge, a 20-second budget, the rest left to the job	A big folder never times out the person deleting it
Bytes through filesPurgeBytesIfUnused()	A copy that shares stored bytes is never broken
Links from other records stay honest - deleted while in the bin, working after a restore, kept but dead after a purge	A ticket never points at nothing without saying so
An invisible upgrade - nothing is purged until the setting is saved	Upgrading never deletes anything
A thorough test suite in a rolled-back transaction, live S3 included	The fixes below were pinned with a few more checks beside it
Three open questions asked, and one risky choice flagged rather than decided quietly	The flagged choice became a rule - see below

🔧 What was added and fixed when merging

Decisions (16 Nov 2026)

Ed answered the PR's questions and one he raised himself; each became code. All in the design page:

• The retention job never deletes a folder holding anything its deleter could not have deleted by hand - the PR flagged this choice. Such folders are kept and listed in System → Maintenance → Deleted folders kept.
• Deleting or moving a folder needs Modify on everything inside it, and the refusal never names a folder the person cannot see; Debug Tools D020 shows an administrator the whole picture. Folder control - Developer Guide.
• A folder of yours inside a deleted folder you cannot see shows in your bin and can be restored on its own, permissions frozen so nobody gains or loses access.
• The clock starts when purging is switched on - nothing already in the bin goes within the hour.
• Delete permanently stays at Modify; the desktop icon is for everyone.

1. The purge read its list before it locked the row 🟠

purge() read the folders, files and stored keys, then locked the entry with FOR UPDATE and checked it was still deleted. Anything that changed in between - a folder brought out of it - would be deleted from an out-of-date list. The lists are now read after the lock, and the job only purges the stay in the bin it listed (deleted_datetime compared).

2. "Analyst 0" meant "the system" 🟠

The job called purge($conn, 0, ...), and 0 skipped every permission check. (int) of a missing session value is also 0. The job now uses purgeAs($conn, null, ...); purge() and restore() refuse anything below 1.

Smaller

• The Empty recycle bin button is now Empty (one-word buttons); the dialog keeps the full title.
• The Files settings page comment said three tabs; there are five.
• The PR's test now sets its own baseline (whatever the database already has in its bin is moved out of the way inside the rolled-back transaction) and sets the switch-on time for the retention checks.
• The 80 new strings are translated into Danish, German, Greek, Spanish, French, Indonesian, Polish and Ukrainian; the two strings that said "no recycle bin yet" were cleared from every locale.

Left as it is

• Original location shows the full path of the folder an entry was in, as Search and the audit do (the PR's question 1). Consistent with the rest of Files.
• The 410 "This file has been deleted from Files." is plain English, like every other message from api/documents/download.php.

🧪 What was checked

Test	Checks
tests/files-recycle-bin.php (the PR's)	67
tests/files-recycle-held.php - clock, held, rescue, race, who acts; every rule with a control	34
tests/files-folder-delete-refusal.php - delete and move over HTTP, a 10 × 10 × 10 tree, D020	24
files-versions, files-attach, files-storage-settings, system-maintenance, system-page-layout, web-exposure-guard, get-must-not-write	all pass

And on screen: the Recycle bin window with a Deleted along with a folder above it row, the Maintenance card showing Found: 1, and the Deleted folders kept page.

---

See also: Files · Recycle bin - design · Recycle bin - Developer Guide