Fix taxonomies import when filters already exist - #17358
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe taxonomy importer now preserves existing component taxonomy filter IDs, appends imported IDs only when missing, and verifies that repeated imports do not duplicate filters. ChangesTaxonomy filter import
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
It seems like you did not give us much information about what you are trying to do here. We would appreciate it if you could provide us with more info about this issue/PR! |
There was a problem hiding this comment.
Pull request overview
Updates the taxonomy maintenance importer so importing taxonomy filters into components no longer overwrites pre-existing taxonomy_filters, addressing cases where components already have filters configured.
Changes:
- Preserve and extend existing component
settings[:taxonomy_filters]during import instead of replacing it. - Avoid duplicating the same taxonomy filter ID in the component settings.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
decidim-core/lib/decidim/maintenance/taxonomy_importer.rb:91
settings[:taxonomy_filters]may already contain integer IDs (e.g. some specs settaxonomy_filters: [taxonomy_filter.id]). In that case, the currentinclude?(filter.id.to_s)check won’t detect an existing filter (123 vs "123") and will append a duplicate entry. Normalizing existing values to strings (and uniquing) before appending avoids mixed-type duplicates and makes the method idempotent across input types.
settings = component.settings.to_h
settings[:taxonomy_filters] ||= []
settings[:taxonomy_filters] << filter.id.to_s unless settings[:taxonomy_filters].include?(filter.id.to_s)
component.update!(settings:)
🎩 What? Why?
The rake task
bin/rails decidim:taxonomies:import_planimports filters created from the old categorization system. However there is a small bug and only imports the last filter defined.For instance if you have JSON file defined like:
{ "imported_taxonomies": { "Scopes": { "taxonomies": {...} "filters": [ { "name": "Scopes filter", "components": [ "gid://decidim-nyc/Decidim::Component/11" ] } ] }, "Categories": { "taxonomies": {...} "filters": [ { "name": "Categories filter", "components": [ "gid://decidim-nyc/Decidim::Component/11" ] } ] } } }Only the last filter will be imported in the Component 11.
This fixes the situation by ensuring to import whatever configuration already available.
📌 Related Issues
Link your PR to an issue
Testing
This probably only happens on situations where you customiza the JSON plan file created from a migration from version 29 to 30.
📷 Screenshots
Please add screenshots of the changes you are proposing
N/A
Summary by CodeRabbit