Repository navigation
ADFA-5444: Skip the Downloads restore/collision check for plugin templates - #2049
jimturner-adfa wants to merge 1 commit into
Conversation
…lates Uninstalling a plugin-provided template (e.g. from the Flutter plugin) unconditionally tried to "restore" it to Downloads, tripping a false "already exists" collision whenever an unrelated file happened to share its name there - even though the template was extracted straight from the plugin's own bundled resources and never touched Downloads in the first place. A plugin-provenance item now just gets deleted from the templates directory on uninstall, matching how PluginProjectManager and IdeTemplateServiceImpl already remove these templates elsewhere. User-imported templates keep the existing restore-to-Downloads behavior, since those genuinely came from there. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📝 Summary
Walkthrough
ChangesPlugin Template Uninstall
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The plugin uninstall path deletes plugin-provided templates without interacting with Downloads, while user-imported templates retain their restore behavior. No merge-blocking risk is identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
A rabbit watched the plugin file go, Comment |
|
Merge-order note: if this merges after #2045 (ADFA-5446), you'll hit one real conflict in Resolution to use when that conflict comes up: /**
* Removes [item] from the templates directory and reloads templates. A plugin-provided item is
* just deleted, since it never came from Downloads in the first place. A user-imported item is
* restored to Downloads first, failing with [TemplateReplaceConflictException] if a file of the
* same name already exists there, unless [overwrite] is true.
*/
suspend fun uninstallTemplate(
item: CgtFileItem,
overwrite: Boolean = false,
): Result<Unit>No other files conflict between the two PRs - confirmed with a full local merge simulation (compiles clean, all 22 tests across both PRs pass together). |
Elissa-AppDevforAll
left a comment
There was a problem hiding this comment.
I could uninstall the individual templates created by the Flutter plugin.
|
There's a logic bug here. It doesn't matter where a template comes from (plugin or cgt file). We don't need a "restore" function for templates. If you want to add a "disable" feature to match the plugins, I could see that. But uninstalling a template doesn't need special undo functionality. The user can re-install the original cgt file. |
| check(item.installed) { "'${item.name}' is not installed" } | ||
| check(item.provenance != TemplateProvenance.BUNDLED) { "Cannot uninstall the bundled template" } | ||
|
|
||
| if (item.provenance == TemplateProvenance.PLUGIN) { |
There was a problem hiding this comment.
Templates come from cgt files, not plugins.
ADFA-5444
Summary
Uninstalling a plugin-provided template (reported with the Flutter plugin) unconditionally tried to "restore" it to Downloads first, tripping a false "already exists" collision whenever an unrelated file happened to share its name there. The template was extracted straight from the plugin's own bundled resources (
PluginProjectManager.extractBundledCgtTemplates) and never touched Downloads in the first place, so the error was both blocking and factually wrong - matching the ticket's exact repro and screenshot.PluginProjectManager/IdeTemplateServiceImplalready remove plugin templates elsewhere (cleanupPluginTemplates/unregisterTemplate).Testing
uninstallTemplate_pluginProvenance_neverChecksOrTouchesDownloads; verified it fails without the fix with the exact reported error (IllegalStateException: A download named '...' already exists in ...) and passes with it.com.ali.fluttertemplate): created a same-named stale file in Downloads for one of its templates, confirmed uninstall previously would have blocked on it, then confirmed with the fix it deletes cleanly - the real template file is removed from the templates directory and the unrelated Downloads file is left untouched. No crashes.🤖 Generated with Claude Code