Fix recycle bin features#59
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 340676be22
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| m = AppController._RECYCLE_UUID_RE.match(recycled_path.stem) | ||
| if m: | ||
| return m.group(1) + recycled_path.suffix |
There was a problem hiding this comment.
_original_name_from_recycled assumes any stem ending in .8hex is a recycle suffix and removes it, but legacy bins can contain legitimate filenames in that format (for example, photo.a1b2c3d4.jpg). Those files are then misclassified as restorable and restore_single_bin restores them under a truncated name (photo.jpg), which silently renames user files. The restore detector needs a stronger discriminator than a regex-only suffix match.
Useful? React with 👍 / 👎.
| (_, jpg_bin), (_, raw_bin) = record | ||
| return ( | ||
| (jpg_bin is not None and jpg_bin in restored_paths) | ||
| or (raw_bin is not None and raw_bin in restored_paths) |
There was a problem hiding this comment.
restore_single_bin records restored files as absolute paths (because bin_path is resolved), then _record_stale compares those absolute paths directly against stored delete_history paths. If FastStack is started with a relative image directory, delete records can contain relative recycle paths, so this comparison misses restored entries and leaves stale undo records behind. In that case, pressing Undo after a manual restore can fail against now-missing recycle-bin files.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@faststack/app.py`:
- Around line 7440-7448: _collect_active_bins currently exposes the local bin as
soon as it exists which allows a user to restore a bin that still has pending
"delete" jobs; because only completed "delete" records are pruned in
_on_delete_finished, this can reintroduce stale undo/delete history. Update
_collect_active_bins to exclude any recycle bin (including the local_bin at
image_dir / "image recycle bin") that has outstanding/pending delete jobs or
uncompleted "delete" records—query the same job/record store that
_on_delete_finished consults (the delete-records or job queue) and filter out
bins with any non-terminal delete entries before adding them to
active_recycle_bins. Ensure you reference and use the same status/check logic as
_on_delete_finished so only bins with fully completed delete processing are
returned.
- Around line 7556-7582: The loop over bin_path.iterdir() can skip files as
entries are moved; change the iteration to first snapshot the directory by
calling list(bin_path.iterdir()) (or similar) and iterate that list instead so
moves don't affect the iterator; update the loop that references
bin_path.iterdir() in the restore routine (the block that calls
self._original_name_from_recycled, moves files into dest_dir, updates
restored_paths and result counters, and logs errors) to iterate over the
snapshot to ensure all files are processed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5e182a95-a664-47bb-91a5-2d44ed7729c3
📒 Files selected for processing (5)
ChangeLog.mdfaststack/app.pyfaststack/imaging/prefetch.pyfaststack/qml/Main.qmlfaststack/ui/provider.py
1 participant
Summary by CodeRabbit
New Features
Improvements