mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-12 06:48:55 +00:00
ignacionelson
98597d462d
Delete a folder called Test and you could never have a folder called Test again. The deletion worked, the folder left the screen, and the name went with it — permanently, with an error that named a collision against a row the interface will not show you and offered nothing to do about it. Files and groups had it too. All three carry a unique index on slug and all three soft-delete, so the trashed row sat in the index holding a name nothing could reach. A public one failed outright at the validator, which checks the table and therefore sees rows the screen does not. A private one failed more quietly: the derived slug stepped around the trashed row into report-2, then report-3, once per deletion, climbing forever. The reservation was deliberate — a trashed row's slug was kept so that restoring it could not land on somebody else's URL. But nothing in this application restores anything. There is no restore() call, no route, no screen; File's own comment says as much. Soft deletes are here so rows can outlive their delete for foreign keys, the activity log and the erasure grace period, never so they can come back. The slug was being held for a page that could not return, and route binding already 404s the trashed row in the meantime. So deleting now hands the slug back, and the database is what makes that a rewrite rather than a gentler lookup: teaching the collision checks to skip trashed rows would leave two rows holding "report", which the unique index rejects whatever the application thinks. The slug moves to report__deleted-42 instead. Underscores are the whole trick — Str::slug() turns them into hyphens and Rules::slug() refuses them outright, so no derived slug and no hand-typed one can ever land on a vacated one. That is a guarantee about the character class rather than a hope about collisions. The format lives in VacatedSlug rather than on the trait because the migration needs it too and a trait constant cannot be reached through the trait's own name — the first version of this was a fatal error waiting for whoever ran migrations. The migration matters as much as the hook: without it the fix only helps installations that have never deleted anything, and every name already buried stays buried. The collision checks still count trashed rows. It costs nothing and keeps them honest about what the index will accept if a row is ever soft-deleted by something that bypasses model events. previous_file_id had this same bug and was fixed this same way, in File::detachOnDelete — a trashed row holding its predecessor's unique slot so the chain could never be re-linked. This is that fix, for the other four unique indexes' worth of the same mistake. users.email is the one left, and is deliberately not in here: an email address is a login identity rather than a URL handle, and freeing it silently is the wrong answer. Fixes #1645 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>