-
Notifications
You must be signed in to change notification settings - Fork 4.4k
chore: add endpoint to retrieve commit history for artifacts #41433
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
Conversation
Add GET endpoint to fetch git commit logs in chronological order for git-connected artifacts.
- Add getCommitHistory method to CentralGitService and GitHandlingService
- Implement commit history retrieval in GitFSServiceCEImpl
- Add GET /{artifactId}/commits endpoint in GitArtifactControllerCE
- Returns list of commits with author, message, and timestamp details
WalkthroughA new commit history retrieval feature is introduced across the git service layer. The implementation adds a vertical stack of methods from REST controller through central services to filesystem operations, enabling clients to fetch chronologically ordered commit logs for git-connected artifacts with permission checks and proper error handling. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Controller as GitArtifactControllerCE
participant CentralSvc as CentralGitServiceCEImpl
participant HandlingSvc as GitHandlingServiceCE
participant FSService as GitFSServiceCEImpl
participant Git as Git Repository
Client->>Controller: GET /{artifactId}/commits?artifactType=X
Controller->>CentralSvc: getCommitHistory(artifactId, artifactType, GIT_TYPE)
CentralSvc->>CentralSvc: Resolve GitArtifactHelper by type
CentralSvc->>CentralSvc: Check artifact edit permission
CentralSvc->>CentralSvc: Fetch branched artifact by ID
CentralSvc->>HandlingSvc: getCommitHistory(branchedArtifact)
HandlingSvc->>FSService: getCommitHistory(branchedArtifact)
FSService->>FSService: Validate GitArtifactMetadata & refName
FSService->>FSService: Compute repository path
FSService->>Git: Checkout target branch
alt Checkout Success
FSService->>Git: Fetch commit history (log)
Git-->>FSService: List<GitLogDTO>
else Checkout/Log Error
FSService->>FSService: Wrap as GIT_ACTION_FAILED exception
end
FSService-->>HandlingSvc: Mono<List<GitLogDTO>>
HandlingSvc-->>CentralSvc: Mono<List<GitLogDTO>>
CentralSvc-->>Controller: Mono<List<GitLogDTO>>
Controller-->>Client: ResponseDTO(200, List<GitLogDTO>)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 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 |
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.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceCEImpl.java (1)
5-5: Consider aligning commit-history flow with existing locking/analytics patternsThe implementation correctly enforces edit permission and delegates to
gitHandlingService.getCommitHistory, but unlike other git flows that touch the working tree (status, commit, pull, merge, discard), this path doesn’t acquire a Redis git lock or emit analytics.Since the FS implementation performs a
checkoutToBranchbefore reading logs, you may want to (a) wrap this in the usualgitRedisUtils.acquireGitLock/usingWhenpattern and (b) optionally add a small analytics hook, for consistency and to avoid races with other git operations. Not a blocker, but worth considering.Also applies to: 2890-2900
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceCE.java(2 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceCEImpl.java(2 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/GitHandlingServiceCE.java(2 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/git/controllers/GitArtifactControllerCE.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/git/fs/GitFSServiceCEImpl.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / rts-build / build
- GitHub Check: server-unit-tests / server-unit-tests
- GitHub Check: server-spotless / spotless-check
🔇 Additional comments (2)
app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/GitHandlingServiceCE.java (1)
3-3: New commit-history service contract looks consistentThe
GitLogDTOimport andgetCommitHistory(Artifact branchedArtifact)signature align cleanly with the rest of the git handling API and downstream implementations. No issues from the interface side.Also applies to: 137-138
app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceCE.java (1)
3-3: Central git commit-history API is well-shapedThe
getCommitHistory(String branchedArtifactId, ArtifactType artifactType, GitType gitType)signature is consistent with existing central-git methods (e.g.,getStatus,pullArtifact) and exposes the right inputs for downstream resolution. Looks good.Also applies to: 106-106
...psmith-server/src/main/java/com/appsmith/server/git/controllers/GitArtifactControllerCE.java
Show resolved
Hide resolved
app/server/appsmith-server/src/main/java/com/appsmith/server/git/fs/GitFSServiceCEImpl.java
Show resolved
Hide resolved
Failed server tests
|
Add GET endpoint to fetch git commit logs in chronological order for git-connected artifacts. This endpoint can be helpful during troubleshooting calls with customers.
Description
Tip
Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team).
Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR.
Fixes #
Issue Numberor
Fixes
Issue URLWarning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.Sanity"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/19823507458
Commit: 09360db
Cypress dashboard.
Tags:
@tag.SanitySpec:
Mon, 01 Dec 2025 13:35:42 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.