-
Notifications
You must be signed in to change notification settings - Fork 0
Implement pass_tags_to_children for folder tag inheritance #127
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: tag_flattening
Are you sure you want to change the base?
Conversation
…emoved Co-authored-by: ploiu <43047560+ploiu@users.noreply.github.com>
Co-authored-by: ploiu <43047560+ploiu@users.noreply.github.com>
ploiu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the tags module, there should be a function called pass_tags_to_children that takes a folder and
- retrieves all descendant folders
- retrieves all descendant files
- do these for both retrieved groups
- don't inherit anything not on the passed folder
- inherit everything on the passed older
The pass_tags_to_children function should be called when a folder changes its tags directly.
1-liner sql statements are fine in rust code, but beyond that the query needs to be pulled out into its own sql file.
Basically, start from scratch, look at the original reqs, look at this comment, and implement the change. Be sure to add tests in folder_service and file_service to ensure tags are properly inherited. Don't worry about the 3 currently broken tests
.gitignore
Outdated
| /files | ||
| *.iml | ||
| *.sqlite* | ||
| *.db |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you add this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added *.db to ignore test database files that were created during debugging. I've removed this change in commit bb48c10.
- Created pass_tags_to_children in tags/service.rs - Added SQL queries for getting descendant folder/file IDs - Added repository functions for descendant retrieval - Called pass_tags_to_children from update_folder_tags - Added comprehensive tests in folder_service - Used 1-liner SQL in Rust, longer queries in separate files Co-authored-by: ploiu <43047560+ploiu@users.noreply.github.com>
I've implemented the
1-liner SQL statements are inline in Rust code, while recursive queries are in separate SQL files ( Added 3 comprehensive tests in |
ploiu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot getting closer, but look at feedback
| @@ -0,0 +1,7 @@ | |||
| -- Get all descendant folder IDs recursively | |||
| with recursive descendants as ( | |||
| select id from Folders where parentId = ?1 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This already exists (same for rust code equivalent) as get_child_folder_ids_recursive.sql
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed in commit 18340e3. Now using the existing get_child_folder_ids_recursive.sql via folder_repository::get_all_child_folder_ids.
- Removed duplicate SQL files (get_descendant_folder_ids.sql, get_descendant_file_ids.sql) - Removed duplicate repository functions from tags/repository.rs - Updated pass_tags_to_children to use existing folder_repository::get_all_child_folder_ids and folder_repository::get_child_files - Rewrote tests in folder_service to test update_folder instead of manually calling tag_service - Added tests in tags module for pass_tags_to_children function directly - All 6 new tests passing (3 in folder_service, 3 in tags/service) Co-authored-by: ploiu <43047560+ploiu@users.noreply.github.com>
ploiu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot absolutely 0 direct database interactions should happen in the service layer. Move sql to specific files
| assert_eq!(1, file1_tags.len()); | ||
| assert_eq!("tag1", file1_tags[0].title); | ||
| assert_eq!(1, file2_tags.len()); | ||
| assert_eq!("tag1", file2_tags[0].title); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assert that these tags are inherited
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit 6d20b3e. Added assertions to verify tags are inherited using inheritedFromId IS NOT NULL checks.
- Created SQL files for all tag operations (get_direct_tags_on_folder, remove/add inherited tags) - Added get_direct_tags_on_folder and other helper functions to tags/repository - Updated pass_tags_to_children to use repository functions instead of inline SQL - Enhanced tests to verify tags are inherited (not direct) using inheritedFromId checks - Fixed tests to reuse connections throughout test execution - Added tests for preserving direct tags on descendants when ancestor changes tags - All 8 tests passing (5 in folder_service, 3 in tags/service) Co-authored-by: ploiu <43047560+ploiu@users.noreply.github.com>
Fixed in commit 6d20b3e. All SQL queries moved to separate files in |
Implementation Plan
This PR implements tag inheritance propagation for folder tag updates.
Changes
Implemented
pass_tags_to_childrenfunction in the tags module that:folder_repository::get_all_child_folder_idsfolder_repository::get_child_filesIntegration
update_folder_tagscallspass_tags_to_childrenafter updating folder tagsupdate_folderin folder_serviceSQL Queries
All SQL queries moved to separate files in
src/assets/queries/tags/:get_direct_tags_on_folder.sql- Get direct tag IDs on a folderremove_inherited_tags_not_on_folder.sql- Remove obsolete inherited tags from foldersremove_inherited_tags_not_on_folder_from_file.sql- Remove obsolete inherited tags from filesadd_inherited_tag_to_folder.sql- Add inherited tag to folderadd_inherited_tag_to_file.sql- Add inherited tag to fileAll database interactions go through repository layer functions, with no direct SQL in service layer.
Testing
Added comprehensive tests in both
folder_serviceandtags/service:folder_service tests (test update_folder integration):
update_folder_propagates_tags_to_descendant_folders- Verifies tag propagation and inheritanceupdate_folder_propagates_tags_to_descendant_files- Verifies file tag propagation and inheritanceupdate_folder_removes_tags_not_on_parent- Verifies tag cleanupupdate_folder_preserves_direct_tags_on_descendant_folders- Verifies direct tags are preservedupdate_folder_preserves_direct_tags_on_descendant_files- Verifies direct file tags are preservedtags/service tests (test pass_tags_to_children directly):
pass_tags_to_children_adds_tags_to_descendant_folderspass_tags_to_children_adds_tags_to_descendant_filespass_tags_to_children_removes_tags_not_on_parentAll 8 tests passing. Tests verify that:
Tags are properly inherited (using
inheritedFromId IS NOT NULLchecks)Direct tags on descendants are preserved when ancestors change
Connections are reused throughout tests
Fixes add / remove direct tag for folder #114
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.