Skip to content

ADFA-3671: Save git credentials#1178

Open
dara-abijo-adfa wants to merge 2 commits intostagefrom
ADFA-3671-save-git-credentials
Open

ADFA-3671: Save git credentials#1178
dara-abijo-adfa wants to merge 2 commits intostagefrom
ADFA-3671-save-git-credentials

Conversation

@dara-abijo-adfa
Copy link
Copy Markdown
Contributor

If a user clones a repository using credentials, the credentials are saved if the clone is successful so they do not need to enter them again when they want to pull/push.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 14, 2026

📝 Walkthrough

Release Notes

Feature

  • Users no longer need to re-enter git credentials after cloning a repository; credentials are automatically saved upon successful clone

Implementation Changes

  • Updated CloneRepositoryViewModel to accept and use GitCredentialsManager dependency for persisting credentials
  • Migrated CloneRepositoryFragment from Fragment-scoped ViewModel delegates to Koin's by viewModel() for improved dependency management
  • Added new Koin viewModel binding for CloneRepositoryViewModel in core DI module

API Changes

  • Removed GitCredentialsManager.hasCredentials() public method (unused internally)
  • Made GitCredentialsManager.saveCredentials() private; use saveCredentialsIfNeeded() instead for new implementations

Risk/Best Practice Note

  • The removal of hasCredentials() and privatization of saveCredentials() are API breaking changes for any external code depending on these public methods. Internal codebase review shows no local usage, but external consumers should be notified of the API change.

Walkthrough

The PR integrates GitCredentialsManager into the clone repository workflow by registering CloneRepositoryViewModel in Koin, switching the fragment to use Koin-based viewModel injection, updating the ViewModel to accept and invoke credential persistence after successful clones, and refactoring the credentials manager API to restrict direct writes.

Changes

Cohort / File(s) Summary
Dependency Injection Setup
app/src/main/java/com/itsaky/androidide/di/AppModule.kt
Added Koin viewModel binding for CloneRepositoryViewModel with two dependencies resolved via get().
Fragment ViewModel Injection
app/src/main/java/com/itsaky/androidide/fragments/CloneRepositoryFragment.kt
Replaced Fragment-scoped ViewModel delegation from by viewModels() to Koin's by viewModel() with updated import statement.
ViewModel and Credential Management
app/src/main/java/com/itsaky/androidide/viewmodel/CloneRepositoryViewModel.kt, git-core/src/main/java/com/itsaky/androidide/git/core/GitCredentialsManager.kt
Updated CloneRepositoryViewModel constructor to accept GitCredentialsManager dependency and added credential persistence call after successful clone; refactored GitCredentialsManager API by making saveCredentials() private and removing hasCredentials() method.

Sequence Diagram

sequenceDiagram
    participant User
    participant Fragment as CloneRepositoryFragment
    participant ViewModel as CloneRepositoryViewModel
    participant CredMgr as GitCredentialsManager
    
    User->>Fragment: Initiates clone
    Fragment->>ViewModel: Triggers clone action
    ViewModel->>ViewModel: Perform clone operation
    alt Clone Successful
        ViewModel->>CredMgr: saveCredentialsIfNeeded(username, token)
        CredMgr->>CredMgr: Save credentials to storage
        CredMgr-->>ViewModel: Complete
        ViewModel-->>Fragment: Emit Success state
    else Clone Failed
        ViewModel-->>Fragment: Emit Error state
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

  • PR #1128: Introduces GitCredentialsManager and integrates it into GitBottomSheetViewModel and related fragments, laying groundwork that this PR builds upon for CloneRepositoryViewModel integration.
  • PR #1005: Introduces the clone repository feature with CloneRepositoryViewModel and CloneRepositoryFragment, which this PR extends with credential persistence functionality.

Suggested reviewers

  • itsaky-adfa
  • Daniel-ADFA
  • jomen-adfa
  • jatezzz

Poem

🐰 Git clones with credentials now kept,
Through Koin's injection, dependencies swept,
Credentials saved when clone succeeds with delight,
Private APIs guard secrets so tight! 🔐

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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 'ADFA-3671: Save git credentials' clearly and concisely summarizes the main change of the pull request, which is to automatically save git credentials after a successful repository clone.
Description check ✅ Passed The description explains the user-facing benefit of the change: credentials are saved after successful clone operations so users don't need to re-enter them for pull/push operations, which is directly related to the changeset.

✏️ 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-3671-save-git-credentials

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.

🧹 Nitpick comments (1)
git-core/src/main/java/com/itsaky/androidide/git/core/GitCredentialsManager.kt (1)

46-48: Narrow the exception type in credential save path.

Line 46 catches Exception, which can mask unexpected defects. Catch only expected crypto/storage exceptions (such as IllegalStateException, InvalidKeyException, BadPaddingException) and let others fail fast.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@git-core/src/main/java/com/itsaky/androidide/git/core/GitCredentialsManager.kt`
around lines 46 - 48, The catch-all Exception in GitCredentialsManager (the save
credentials code path that currently does log.error("Failed to save
credentials", e)) is too broad; replace it with specific catches for expected
crypto/storage failures (e.g., IllegalStateException, InvalidKeyException,
BadPaddingException) and log those, and allow any other unexpected exceptions to
propagate (rethrow) so they fail fast; implement this as multiple catch blocks
in the same method (the save credentials method in GitCredentialsManager) using
log.error for the expected exception handlers and rethrow for the rest.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@git-core/src/main/java/com/itsaky/androidide/git/core/GitCredentialsManager.kt`:
- Around line 46-48: The catch-all Exception in GitCredentialsManager (the save
credentials code path that currently does log.error("Failed to save
credentials", e)) is too broad; replace it with specific catches for expected
crypto/storage failures (e.g., IllegalStateException, InvalidKeyException,
BadPaddingException) and log those, and allow any other unexpected exceptions to
propagate (rethrow) so they fail fast; implement this as multiple catch blocks
in the same method (the save credentials method in GitCredentialsManager) using
log.error for the expected exception handlers and rethrow for the rest.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ddb0206b-0d85-4ac6-a779-44b02fcaa348

📥 Commits

Reviewing files that changed from the base of the PR and between 8787677 and bfad1b6.

📒 Files selected for processing (4)
  • app/src/main/java/com/itsaky/androidide/di/AppModule.kt
  • app/src/main/java/com/itsaky/androidide/fragments/CloneRepositoryFragment.kt
  • app/src/main/java/com/itsaky/androidide/viewmodel/CloneRepositoryViewModel.kt
  • git-core/src/main/java/com/itsaky/androidide/git/core/GitCredentialsManager.kt

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.

1 participant