-
Notifications
You must be signed in to change notification settings - Fork 3k
feat(trackers): add site-url to optionally provide jira server URL for oauth #6716
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
WalkthroughThe Jira tracker integration was refactored to convert a package-level function into an instance method while adding a configurable SiteURL option to override browsable issue URLs, ensuring URL construction logic conditionally uses the provided SiteURL or falls back to the original behavior. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 0
🧹 Nitpick comments (1)
pkg/reporting/trackers/jira/jira.go (1)
355-377: Consider adding tests for the new SiteURL functionality.While the implementation is sound, the PR checklist indicates tests haven't been added yet. Consider adding unit tests to verify:
- URL construction when
SiteURLis provided- Fallback behavior when
SiteURLis empty- Proper handling of trailing slashes in
SiteURLWould you like me to help generate test cases for this functionality?
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/reporting/trackers/jira/jira.go(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Format Go code usinggo fmt ./...
Run static analysis usinggo vet ./...on Go code
Files:
pkg/reporting/trackers/jira/jira.go
pkg/reporting/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Result export and issue tracking integrations should be implemented in pkg/reporting/
Files:
pkg/reporting/trackers/jira/jira.go
🧬 Code graph analysis (1)
pkg/reporting/trackers/jira/jira.go (4)
pkg/reporting/trackers/gitlab/gitlab.go (1)
Integration(17-21)pkg/reporting/trackers/github/github.go (1)
Integration(23-26)pkg/reporting/trackers/gitea/gitea.go (1)
Integration(19-22)pkg/reporting/trackers/filters/filters.go (1)
CreateIssueResponse(13-16)
🔇 Additional comments (4)
pkg/reporting/trackers/jira/jira.go (4)
187-189: Well-documented optional field addition.The
SiteURLfield is properly documented with a clear use case for OAuth scenarios. The YAML and JSON tags follow the existing conventions in the codebase.
355-377: Solid implementation with proper fallback logic.The conversion from package-level function to instance method follows the pattern used in other tracker integrations. The conditional URL construction logic is clean and maintains backward compatibility when
SiteURLis not configured.
352-352: Correctly updated to instance method.The call site properly invokes the refactored instance method with the appropriate argument type.
392-392: Correctly updated to instance method.The call site properly invokes the refactored instance method with the appropriate argument type.
Adds optional `site-url` field to JIRA issue tracker configuration for specifying browsable URL when it differs from the API endpoint. This is particularly useful for OAuth-based JIRA Cloud integrations where `issue.Self` contains "api.atlassian.com" instead of the user-facing domain. Signed-off-by: Dwi Siswanto <[email protected]>
|
@coderabbitai create an issue for this PR. |
|
Tip For best results, initiate chat on the files or code changes. @dwisiswant0, I'd be happy to create an issue for this PR! However, I need a bit more information to create a meaningful issue:
Please provide details about what you'd like the issue to contain (title, description, and any specific points to include), and I'll create it for you. |
@coderabbitai as in the changes. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 5857 🏁 Script executed: Length of output: 54 @dwisiswant0, I've created the issue for this PR: #6717 The issue documents the feature enhancement to add optional |
Proposed changes
Checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.