Skip to content

ADFA-3270: Add erase installation file when done button to Plugin Manager#1119

Merged
Daniel-ADFA merged 4 commits into
stagefrom
ADFA-3270
Mar 26, 2026
Merged

ADFA-3270: Add erase installation file when done button to Plugin Manager#1119
Daniel-ADFA merged 4 commits into
stagefrom
ADFA-3270

Conversation

@Daniel-ADFA
Copy link
Copy Markdown
Contributor

Screen.Recording.2026-03-25.at.12.36.21.mov

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 25, 2026

📝 Walkthrough
  • New feature: add "erase installation file when done" checkbox to the plugin installation dialog so users can choose to delete the plugin source file after successful install.
  • New UI flow: file picking migrated from deprecated onActivityResult/REQUEST_CODE_PICK_PLUGIN to ActivityResultContracts.OpenDocument with persistable URI permission attempts.
  • Install flow: selected file validated by extension (.cgp) before prompting install confirmation; install event now carries deletion preference.
  • Persistence & deletion: persistent read permission is requested; when user opts in the ViewModel attempts to delete the source via DocumentsContract.deleteDocument(...) on an IO dispatcher and emits a localized error effect on failure.
  • Localization: replaced inline string messages with string resource IDs for all plugin lifecycle success/error effects and added multiple new localized strings (install/load/enable/disable/uninstall messages, unsupported file, no file manager, deletion failures, plugin manager init failure, and checkbox label).
  • UI/state changes:
    • PluginManagerUiEvent.InstallPlugin now includes deleteSourceAfterInstall: Boolean (InstallPlugin(uri: Uri, deleteSourceAfterInstall: Boolean).
    • PluginManagerUiEffect changed from string payloads to resource-ID payloads: ShowError(@StringRes messageResId: Int, formatArgs: List = emptyList()), ShowSuccess(@StringRes messageResId: Int).
    • Removed PluginManagerUiEvent.ClearMessages and the UiState errorMessage/successMessage string properties; showEmptyState logic no longer depends on errorMessage.
    • PluginManagerActivity updated to use the new picker launcher, show install dialog (reads checkbox), and emit InstallPlugin with checkbox state.
  • Minor UX: replaced a hardcoded "no file manager" toast with localized string.

Risks / Best-practice notes:

  • ⚠️ Permission & deletion risk: Deleting the source document requires the document provider to grant delete rights. Even with persistable read permission there may be no delete permission; deletion failures are handled but can surface to users.
  • ⚠️ Breaking API changes: Public/semi-public model/event/effect signatures changed (InstallPlugin and UiEffect types) and removal of message fields — downstream consumers must be updated.
  • ⚠️ One-time effects migration: Converting effects to resource IDs assumes callers/handlers supply/interpret format args correctly; mismatches could lead to incorrect messages.
  • ✓ Best practice: Using ActivityResultContracts.OpenDocument and persistable URI permissions matches modern Android guidance.
  • ✓ Best practice: Moving user-visible text to string resources improves localization and maintainability.

Walkthrough

Replaced legacy file-pick flow with ActivityResultContracts.OpenDocument, added an install confirmation dialog with a "delete source after install" checkbox, migrated UI effects to use string resource IDs (with optional format args), and implemented conditional deletion of the source document in the ViewModel using DocumentsContract.

Changes

Cohort / File(s) Summary
Activity UI Layer
app/src/main/java/com/itsaky/androidide/activities/PluginManagerActivity.kt
Replaced onActivityResult with ActivityResultContracts.OpenDocument launcher; persistable URI permission attempted; validate .cgp extension; show install confirmation dialog and emit InstallPlugin(uri, deleteFlag); use localized string resources for toasts/errors.
State & Event Models
app/src/main/java/com/itsaky/androidide/ui/models/PluginManagerUiState.kt
Removed errorMessage/successMessage; adjusted showEmptyState logic; InstallPlugin now includes deleteSourceAfterInstall: Boolean; PluginManagerUiEffect messages switched to @StringRes IDs with optional format args; removed ClearMessages.
ViewModel Logic
app/src/main/java/com/itsaky/androidide/viewmodels/PluginManagerViewModel.kt
Handle InstallPlugin(uri, deleteSourceAfterInstall) and pass flag to installer; emit success/failure effects using string resource IDs and format args; when delete flag true, attempt DocumentsContract.deleteDocument(uri) on IO dispatcher and emit error effect for deletion failures (including permission-specific message).
Dialog Layout
app/src/main/res/layout/dialog_install_plugin.xml
New layout with CheckBox (@+id/checkbox_delete_source) to capture user choice to delete source after install.
String Resources
resources/src/main/res/values/strings.xml
Added multiple strings for plugin lifecycle success/failure messages (with %1$s where applicable), deletion errors, unsupported file message, no-file-manager message, manager init failure, and checkbox label.

Sequence Diagram

sequenceDiagram
    participant User
    participant Activity
    participant Dialog
    participant ViewModel
    participant FileSystem
    participant UI

    User->>Activity: Start plugin install
    Activity->>Activity: Launch OpenDocument picker
    User->>Activity: Select file (Uri)
    Activity->>Activity: takePersistableUriPermission(uri)
    Activity->>Dialog: Show install confirmation (checkbox)
    User->>Dialog: Confirm with deleteFlag
    Dialog->>Activity: Return uri + deleteFlag
    Activity->>ViewModel: Emit InstallPlugin(uri, deleteFlag)
    ViewModel->>FileSystem: Install plugin from uri
    ViewModel->>UI: Emit ShowSuccess(res_id) or ShowError(res_id, args)
    alt deleteFlag == true
        ViewModel->>FileSystem: DocumentsContract.deleteDocument(uri)
        alt delete success
            ViewModel->>UI: (no-op or success)
        else delete fails
            ViewModel->>UI: Emit ShowError(delete_error_res_id, args)
        end
    end
    UI->>User: Display message
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • dara-abijo-adfa
  • jatezzz
  • jomen-adfa

Poem

🐰 A picker hops to find a file so neat,

Strings get proper homes and dialogs greet,
Check the box — will you keep or send away?
The ViewModel tries to tidy up the tray,
Plugins hop in, the rabbit cheers today! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 46.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding functionality to erase/delete the installation file after plugin installation, which is reflected throughout the changeset.
Description check ✅ Passed The description references a visual asset/video demonstrating the feature, which relates to the changeset's addition of the erase installation file functionality.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ADFA-3270

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/src/main/java/com/itsaky/androidide/activities/PluginManagerActivity.kt`:
- Around line 48-59: The plugin picker currently accepts any MIME type and
forwards arbitrary files to showInstallConfirmation, causing later APK fallback;
update the registerForActivityResult/OpenDocument usage (pluginPickerLauncher)
to limit MIME types to known plugin formats (e.g.,
"application/vnd.android.package-archive" and any other supported plugin
MIME(s)) and, in the callback before calling showInstallConfirmation(uri),
validate the selected file's extension or MIME (use contentResolver.getType(uri)
and/or extract the filename/extension from DocumentFile.fromSingleUri or query
display name) and immediately reject/notify the user if it is unsupported; apply
the same validation logic to the other picker path referenced at lines 217-219
so unsupported files never enter the install confirmation flow.

In `@app/src/main/java/com/itsaky/androidide/ui/models/PluginManagerUiState.kt`:
- Around line 10-17: The current logic makes showEmptyState false when hasError
is true, which hides the persistent empty/error UI in
PluginManagerActivity.updateUI() and leaves the screen blank; fix by adding a
dedicated error flag (e.g., val showErrorState) or changing showEmptyState to
account for errors so the empty/error view remains visible when
plugins.isEmpty() && !isLoading and isPluginManagerAvailable, and expose that
flag for PluginManagerActivity.updateUI() to render error copy and retry actions
(reference: hasError, showEmptyState, plugins, isLoading,
isPluginManagerAvailable, PluginManagerActivity.updateUI(), emptyState).

In
`@app/src/main/java/com/itsaky/androidide/viewmodels/PluginManagerViewModel.kt`:
- Around line 263-280: Move the restart prompt emission to after the source
deletion attempt and ensure deletion errors cannot escape to the install failure
path: in the onSuccess block emit ShowSuccess and call loadPlugins() first, then
if (deleteSourceAfterInstall) call deleteSourceDocument(uri) inside its own
try/catch (or make deleteSourceDocument swallow/log all exceptions and return a
success/failure flag) so no exception propagates; only after that completion
send PluginManagerUiEffect.ShowRestartPrompt. Reference:
deleteSourceAfterInstall, deleteSourceDocument(uri),
PluginManagerUiEffect.ShowSuccess, loadPlugins(), and
PluginManagerUiEffect.ShowRestartPrompt.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8b678516-28b8-4a9e-b9e3-4bc7480df67a

📥 Commits

Reviewing files that changed from the base of the PR and between 94dd217 and b960d2a.

📒 Files selected for processing (5)
  • app/src/main/java/com/itsaky/androidide/activities/PluginManagerActivity.kt
  • app/src/main/java/com/itsaky/androidide/ui/models/PluginManagerUiState.kt
  • app/src/main/java/com/itsaky/androidide/viewmodels/PluginManagerViewModel.kt
  • app/src/main/res/layout/dialog_install_plugin.xml
  • resources/src/main/res/values/strings.xml

Comment thread app/src/main/java/com/itsaky/androidide/ui/models/PluginManagerUiState.kt Outdated
Comment thread app/src/main/java/com/itsaky/androidide/ui/models/PluginManagerUiState.kt Outdated
Comment thread resources/src/main/res/values/strings.xml Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
app/src/main/java/com/itsaky/androidide/viewmodels/PluginManagerViewModel.kt (1)

261-267: ⚠️ Potential issue | 🟠 Major

Emit the restart prompt only after the source cleanup attempt finishes.

ShowRestartPrompt is still sent before deleteSourceDocument(uri). If the user restarts immediately, the optional delete can be interrupted, and any delete-failure message can end up hidden behind the modal even though install already succeeded.

Suggested diff
                         Log.d(TAG, "Plugin installed successfully")
                         _uiEffect.trySend(PluginManagerUiEffect.ShowSuccess(R.string.msg_plugin_installed))
                         loadPlugins()
-                        _uiEffect.trySend(PluginManagerUiEffect.ShowRestartPrompt)
 
                         if (deleteSourceAfterInstall) {
                             deleteSourceDocument(uri)
                         }
+
+                        _uiEffect.trySend(PluginManagerUiEffect.ShowRestartPrompt)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/src/main/java/com/itsaky/androidide/viewmodels/PluginManagerViewModel.kt`
around lines 261 - 267, The restart prompt is currently emitted before the
optional source cleanup, which can cause the delete to be interrupted; change
the sequence so that when deleteSourceAfterInstall is true you first call and
await deleteSourceDocument(uri) (or handle its completion) and only after that
call _uiEffect.trySend(PluginManagerUiEffect.ShowRestartPrompt); keep the
existing success toast and loadPlugins() order, but move the
_uiEffect.trySend(PluginManagerUiEffect.ShowRestartPrompt) to after the
deleteSourceDocument(uri) completion/await to ensure delete failures can surface
before showing the restart modal.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/src/main/java/com/itsaky/androidide/activities/PluginManagerActivity.kt`:
- Around line 58-72: The code currently calls
contentResolver.takePersistableUriPermission immediately after file selection;
instead, defer taking the persistable grant until the user explicitly confirms
in showInstallConfirmation, and ensure any grant is revoked in all exit paths
(unsupported file, canceled dialog, failed install). Move the call to
contentResolver.takePersistableUriPermission into the confirm/positive callback
inside showInstallConfirmation (or a helper invoked on confirm), and add a
paired revokePersistableUriPermission call in the cleanup paths (on cancel, on
unsupported file before returning, and always after install attempt completes),
and update deleteSourceDocument()/deleteSourceAfterInstall handling so
deleteSourceDocument() is only called after a successful install but revocation
runs unconditionally to avoid stale permissions.

---

Duplicate comments:
In
`@app/src/main/java/com/itsaky/androidide/viewmodels/PluginManagerViewModel.kt`:
- Around line 261-267: The restart prompt is currently emitted before the
optional source cleanup, which can cause the delete to be interrupted; change
the sequence so that when deleteSourceAfterInstall is true you first call and
await deleteSourceDocument(uri) (or handle its completion) and only after that
call _uiEffect.trySend(PluginManagerUiEffect.ShowRestartPrompt); keep the
existing success toast and loadPlugins() order, but move the
_uiEffect.trySend(PluginManagerUiEffect.ShowRestartPrompt) to after the
deleteSourceDocument(uri) completion/await to ensure delete failures can surface
before showing the restart modal.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d8dcfbea-7b8a-4d13-a2f4-ece090e2fd46

📥 Commits

Reviewing files that changed from the base of the PR and between b960d2a and e9e4457.

📒 Files selected for processing (4)
  • app/src/main/java/com/itsaky/androidide/activities/PluginManagerActivity.kt
  • app/src/main/java/com/itsaky/androidide/ui/models/PluginManagerUiState.kt
  • app/src/main/java/com/itsaky/androidide/viewmodels/PluginManagerViewModel.kt
  • resources/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (1)
  • resources/src/main/res/values/strings.xml

@Daniel-ADFA Daniel-ADFA merged commit b9a50a1 into stage Mar 26, 2026
2 checks passed
@Daniel-ADFA Daniel-ADFA deleted the ADFA-3270 branch March 26, 2026 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants