-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
feat: fix reporting close functionality + misc #6066
base: dev
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request reintroduces the MongoDB exporter import in the reporting package and inverts the logic in the Changes
Sequence Diagram(s)sequenceDiagram
participant RC as ReportingClient
participant T as Tracker
participant IC as IssueCloser
RC->>T: ShouldFilter(event)
alt Tracker does NOT filter event (!ShouldFilter)
RC->>IC: Proceed with closing issue
else Tracker filters event
RC->>RC: Skip to next iteration
end
sequenceDiagram
participant J as JiraIntegration
participant DB as JIRA Database
Note over J,DB: Issue Processing in Jira Tracker
J->>J: Call FindExistingIssue(event, useStatus)
alt Creating Issue (useStatus=true)
J->>DB: Query with status filter included
else Closing Issue (useStatus=false)
J->>DB: Query without status filter
end
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pkg/reporting/reporting.go
(2 hunks)pkg/reporting/trackers/jira/jira.go
(3 hunks)
🔇 Additional comments (5)
pkg/reporting/reporting.go (1)
9-9
: LGTM!The MongoDB exporter import has been reintroduced.
pkg/reporting/trackers/jira/jira.go (4)
303-303
: LGTM!The addition of the
useStatus
parameter enhances the flexibility of theFindExistingIssue
method by allowing control over whether to consider issue status in the JQL query.
243-243
: LGTM!Correctly passing
true
foruseStatus
to consider status when finding existing issues during creation/update.
268-268
: LGTM!Correctly passing
false
foruseStatus
to ignore status when finding existing issues during closure.
309-312
: LGTM!The JQL query has been improved:
- Base query is now cleaner
- Status condition is conditionally added based on
useStatus
parameter
@@ -329,7 +330,7 @@ func (c *ReportingClient) CreateIssue(event *output.ResultEvent) error { | |||
// CloseIssue closes an issue in the tracker | |||
func (c *ReportingClient) CloseIssue(event *output.ResultEvent) error { | |||
for _, tracker := range c.trackers { | |||
if tracker.ShouldFilter(event) { | |||
if !tracker.ShouldFilter(event) { |
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.
Fix the inverted condition in CloseIssue method.
The condition has been inverted which changes the intended behavior. Issues will now be closed when they should NOT be filtered, which appears to be incorrect.
Apply this diff to fix the logic:
- if !tracker.ShouldFilter(event) {
+ if tracker.ShouldFilter(event) {
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
if !tracker.ShouldFilter(event) { | |
if tracker.ShouldFilter(event) { |
Proposed changes
Checklist
Summary by CodeRabbit