Skip to content

refactor(files_trashbin): Clean up move2trash and expand test coverage - #63069

Draft
joshtrichards wants to merge 4 commits into
masterfrom
jtr/Trashbin-cleanup
Draft

refactor(files_trashbin): Clean up move2trash and expand test coverage#63069
joshtrichards wants to merge 4 commits into
masterfrom
jtr/Trashbin-cleanup

Conversation

@joshtrichards

Copy link
Copy Markdown
Member
  • Resolves: #

Summary

This change primarily improves readability and documentation in the Trashbin::move2Trash() implementation while adding additional coverage for shared-file deletion and configured trashbin size limits.

Changes:

  • Clarify and expand PHPDoc for two inter-related trashbin operations.
  • Add parameter typing.
  • Simplify conditionals and improve control-flow readability.
  • Make comments more explicit around:
    • Shared-file copies.
    • Lock acquisition and timestamp retries.
    • Cache updates during moves.
    • Rollback behavior after failed moves.
    • Expiration scheduling.
  • Reformat the post_moveToTrash hook invocation for readability.
  • Add regression coverage for:
    • Shared files appearing in both the owner’s and deleting user’s trashbins.
    • Files exceeding the configured trashbin size being permanently deleted instead of retained.

Behavior

The changes are primarily comment cleanup, type clarification, and additional test coverage. The tests document expected behavior around shared files and configured trashbin limits.

TODO

  • ...

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

Flatten logic:
1. If there is not enough space, stop.
2. Copy the item.
3. If the copy did not materialize, stop.
4. Record metadata.
5. If bookkeeping fails, log the error, stop.

Clarify cross-user copy method:
- existing copyFilesToUser() docblock is vague
- does not explain why it exists / how failures are handled

Signed-off-by: Josh <josh.t.richards@gmail.com>
…2Trash

Also null check style improvements and make some useful but focused comment improvements:

- Explain why the timestamp is incremented
- Clarify the database-before-move ordering
- Explain the cache update branches
- Clarify the failed-original-deletion recovery
- Improve the owner/user comments
- Update the move2Trash() docblock

Signed-off-by: Josh <josh.t.richards@gmail.com>
- Configured size fallback: source remains, no trash item is created.
- Shared deletion: both owner and deleting user receive trash entries.

Assisted-by: Copilot:gpt-5.6-luna

Signed-off-by: Josh <josh.t.richards@gmail.com>
@joshtrichards joshtrichards added 2. developing Work in progress feature: trashbin tests Related to tests ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring) labels Aug 8, 2026
Signed-off-by: Josh <josh.t.richards@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2. developing Work in progress AI assisted feature: trashbin ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring) tests Related to tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants