This repository was archived by the owner on Feb 4, 2026. It is now read-only.
Remove stale ancestor implicit tags when folder is moved#131
Merged
ploiu merged 8 commits intotag_flatteningfrom Nov 24, 2025
Merged
Remove stale ancestor implicit tags when folder is moved#131ploiu merged 8 commits intotag_flatteningfrom
ploiu merged 8 commits intotag_flatteningfrom
Conversation
- Detect parent_id changes in update_folder - Retrieve original ancestors before database update - Remove implicit tags from descendants that came from old ancestors - Add test: moving folder to another folder recalculates descendant implicit tags - Add test: moving folder to root removes all descendant implicit tags Co-authored-by: ploiu <43047560+ploiu@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix folder parentId change handling for descendants
Remove stale ancestor implicit tags when folder is moved
Nov 24, 2025
ploiu
suggested changes
Nov 24, 2025
src/service/folder_service.rs
Outdated
Comment on lines
+193
to
+204
| match folder_repository::get_child_files(&all_folder_ids, &con) { | ||
| Ok(_files) => { /* retrieved successfully, we just needed to check */ } | ||
| Err(e) => { | ||
| log::error!( | ||
| "Failed to retrieve descendant files for folder {}. Error is {e:?}\n{}", | ||
| folder.id, | ||
| Backtrace::force_capture() | ||
| ); | ||
| con.close().unwrap(); | ||
| return Err(UpdateFolderError::DbFailure); | ||
| } | ||
| }; |
Contributor
Author
There was a problem hiding this comment.
You're right - this check was unnecessary. The remove_implicit_tag_from_files and remove_implicit_tags_from_folders functions use the implicit_from_id (ancestor_id) to find and remove tags, not a list of file/folder IDs. Removed in commit a75399d.
- Move tag removal logic to new `handle_folder_move_for_tags` function - Remove unnecessary file retrieval check that wasn't being used - Repository functions already handle finding descendants automatically Co-authored-by: ploiu <43047560+ploiu@users.noreply.github.com>
- Add test: moving_folder_does_not_remove_tags_from_unaffected_folders - Add test: moving_folder_does_not_remove_explicit_tags_from_descendants - Fix handle_folder_move_for_tags to only remove tags from moved folder's descendants - Previously removed tags from ALL items with that implicit_from_id (affecting siblings) - Now specifically targets only descendants of the moved folder Co-authored-by: ploiu <43047560+ploiu@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implementation Complete ✅
Successfully implemented folder move implicit tag recalculation feature with refactoring and comprehensive testing based on code review feedback.
Implementation Details:
update_folderto detect parent_id changesChanges Made:
Modified
update_folderinsrc/service/folder_service.rs:parent_idchangesfolder_repository::get_ancestor_folders_with_idhandle_folder_move_for_tagsto remove old ancestor implicit tagsAdded
handle_folder_move_for_tagshelper function:implicit_from_id(including unaffected siblings)remove_implicit_tag_from_fileandremove_implicit_tag_from_folderfor targeted removalAdded four comprehensive test cases:
moving_a_folder_to_another_folder_recalculates_descendant_implicit_tags: Verifies that moving a folder removes old ancestor implicit tags from descendantsmoving_a_folder_to_root_removes_all_descendant_implicit_tags_from_original_ancestors: Verifies that moving to root removes all ancestor implicit tagsmoving_folder_does_not_remove_tags_from_unaffected_folders: Verifies sibling folders keep their tags when another folder movesmoving_folder_does_not_remove_explicit_tags_from_descendants: Verifies explicit tags on descendants are preserved during moveTest Results:
update_foldertests pass ✅ (increased from 26 tests)The implementation follows the minimal change principle, has been refactored based on code review feedback for better code organization, and includes comprehensive edge case testing to ensure correct behavior.
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.