Skip to content
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

WIP: support single comment summary with orchestrator mode #1850

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from
19 changes: 17 additions & 2 deletions backend/controllers/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,15 @@ func handlePullRequestEvent(gh utils.GithubClientProvider, payload *github.PullR
log.Printf("strconv.ParseInt error: %v", err)
commentReporterManager.UpdateComment(fmt.Sprintf(":x: could not handle commentId: %v", err))
}
batchId, _, err := utils.ConvertJobsToDiggerJobs(*diggerCommand, models.DiggerVCSGithub, organisationId, impactedJobsMap, impactedProjectsMap, projectsGraph, installationId, branch, prNumber, repoOwner, repoName, repoFullName, commitSha, commentId, diggerYmlStr, 0)

placeholderComment, err := ghService.PublishComment(prNumber, "<digger report placehoder>")
if err != nil {
log.Printf("strconv.ParseInt error: %v", err)
commentReporterManager.UpdateComment(fmt.Sprintf(":x: could not create placeholder commentId for report: %v", err))
return fmt.Errorf("comment reporter error: %v", err)
}

batchId, _, err := utils.ConvertJobsToDiggerJobs(*diggerCommand, models.DiggerVCSGithub, organisationId, impactedJobsMap, impactedProjectsMap, projectsGraph, installationId, branch, prNumber, repoOwner, repoName, repoFullName, commitSha, commentId, &placeholderComment.Id, diggerYmlStr, 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix redeclaration of batchId variable

The batchId variable is redeclared using := operator when it's already defined. This will cause a compilation error.

-batchId, _, err := utils.ConvertJobsToDiggerJobs(
+batchId, _, err = utils.ConvertJobsToDiggerJobs(

Also applies to: 958-958

🧰 Tools
🪛 golangci-lint (1.62.2)

538-538: no new variables on left side of :=

(typecheck)

if err != nil {
log.Printf("ConvertJobsToDiggerJobs error: %v", err)
commentReporterManager.UpdateComment(fmt.Sprintf(":x: ConvertJobsToDiggerJobs error: %v", err))
Expand Down Expand Up @@ -874,7 +882,14 @@ func handleIssueCommentEvent(gh utils.GithubClientProvider, payload *github.Issu
return fmt.Errorf("comment reporter error: %v", err)
}

batchId, _, err := utils.ConvertJobsToDiggerJobs(*diggerCommand, "github", orgId, impactedProjectsJobMap, impactedProjectsMap, projectsGraph, installationId, *branch, issueNumber, repoOwner, repoName, repoFullName, *commitSha, reporterCommentId, diggerYmlStr, 0)
placeholderComment, err := ghService.PublishComment(issueNumber, "<digger report placehoder>")
if err != nil {
log.Printf("strconv.ParseInt error: %v", err)
commentReporterManager.UpdateComment(fmt.Sprintf(":x: could not create placeholder commentId for report: %v", err))
return fmt.Errorf("comment reporter error: %v", err)
}

batchId, _, err := utils.ConvertJobsToDiggerJobs(*diggerCommand, "github", orgId, impactedProjectsJobMap, impactedProjectsMap, projectsGraph, installationId, *branch, issueNumber, repoOwner, repoName, repoFullName, *commitSha, reporterCommentId, &placeholderComment.Id, diggerYmlStr, 0)
if err != nil {
log.Printf("ConvertJobsToDiggerJobs error: %v", err)
commentReporterManager.UpdateComment(fmt.Sprintf(":x: ConvertJobsToDiggerJobs error: %v", err))
Expand Down
27 changes: 14 additions & 13 deletions backend/models/scheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,20 @@ const DiggerVCSGithub DiggerVCSType = "github"
const DiggerVCSGitlab DiggerVCSType = "gitlab"

type DiggerBatch struct {
ID uuid.UUID `gorm:"primary_key"`
VCS DiggerVCSType
PrNumber int
CommentId *int64
Status orchestrator_scheduler.DiggerBatchStatus
BranchName string
DiggerConfig string
GithubInstallationId int64
GitlabProjectId int
RepoFullName string
RepoOwner string
RepoName string
BatchType orchestrator_scheduler.DiggerCommand
ID uuid.UUID `gorm:"primary_key"`
VCS DiggerVCSType
PrNumber int
CommentId *int64
PlaceholderCommentIdForReport *string
Status orchestrator_scheduler.DiggerBatchStatus
BranchName string
DiggerConfig string
GithubInstallationId int64
GitlabProjectId int
RepoFullName string
RepoOwner string
RepoName string
BatchType orchestrator_scheduler.DiggerCommand
// used for module source grouping comments
SourceDetails []byte
}
Expand Down
29 changes: 15 additions & 14 deletions backend/models/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -617,22 +617,23 @@ func (db *Database) GetDiggerBatch(batchId *uuid.UUID) (*DiggerBatch, error) {
return batch, nil
}

func (db *Database) CreateDiggerBatch(vcsType DiggerVCSType, githubInstallationId int64, repoOwner string, repoName string, repoFullname string, PRNumber int, diggerConfig string, branchName string, batchType scheduler.DiggerCommand, commentId *int64, gitlabProjectId int) (*DiggerBatch, error) {
func (db *Database) CreateDiggerBatch(vcsType DiggerVCSType, githubInstallationId int64, repoOwner string, repoName string, repoFullname string, PRNumber int, diggerConfig string, branchName string, batchType scheduler.DiggerCommand, commentId *int64, placeholderCommentIdForReport *string, gitlabProjectId int) (*DiggerBatch, error) {
uid := uuid.New()
batch := &DiggerBatch{
ID: uid,
VCS: vcsType,
GithubInstallationId: githubInstallationId,
RepoOwner: repoOwner,
RepoName: repoName,
RepoFullName: repoFullname,
PrNumber: PRNumber,
CommentId: commentId,
Status: scheduler.BatchJobCreated,
BranchName: branchName,
DiggerConfig: diggerConfig,
BatchType: batchType,
GitlabProjectId: gitlabProjectId,
ID: uid,
VCS: vcsType,
GithubInstallationId: githubInstallationId,
RepoOwner: repoOwner,
RepoName: repoName,
RepoFullName: repoFullname,
PrNumber: PRNumber,
CommentId: commentId,
PlaceholderCommentIdForReport: placeholderCommentIdForReport,
Status: scheduler.BatchJobCreated,
BranchName: branchName,
DiggerConfig: diggerConfig,
BatchType: batchType,
GitlabProjectId: gitlabProjectId,
}
result := db.GormDB.Save(batch)
if result.Error != nil {
Expand Down
4 changes: 3 additions & 1 deletion backend/services/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,10 @@ func GetSpecFromJob(job models.DiggerJob) (*spec.Spec, error) {
CommentId: strconv.FormatInt(*batch.CommentId, 10),
Job: jobSpec,
Reporter: spec.ReporterSpec{
ReportingStrategy: "comments_per_run",
//ReportingStrategy: "comments_per_run",
ReportingStrategy: "always_same_comment",
ReporterType: "lazy",
ReportCommentId: job.Batch.PlaceholderCommentIdForReport,
},
Lock: spec.LockSpec{
LockType: "noop",
Expand Down
4 changes: 2 additions & 2 deletions backend/utils/graphs.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
)

// ConvertJobsToDiggerJobs jobs is map with project name as a key and a Job as a value
func ConvertJobsToDiggerJobs(jobType scheduler.DiggerCommand, vcsType models.DiggerVCSType, organisationId uint, jobsMap map[string]scheduler.Job, projectMap map[string]configuration.Project, projectsGraph graph.Graph[string, configuration.Project], githubInstallationId int64, branch string, prNumber int, repoOwner string, repoName string, repoFullName string, commitSha string, commentId int64, diggerConfigStr string, gitlabProjectId int) (*uuid.UUID, map[string]*models.DiggerJob, error) {
func ConvertJobsToDiggerJobs(jobType scheduler.DiggerCommand, vcsType models.DiggerVCSType, organisationId uint, jobsMap map[string]scheduler.Job, projectMap map[string]configuration.Project, projectsGraph graph.Graph[string, configuration.Project], githubInstallationId int64, branch string, prNumber int, repoOwner string, repoName string, repoFullName string, commitSha string, commentId int64, placeholderCommentIdForReport *string, diggerConfigStr string, gitlabProjectId int) (*uuid.UUID, map[string]*models.DiggerJob, error) {
result := make(map[string]*models.DiggerJob)
organisation, err := models.DB.GetOrganisationById(organisationId)
if err != nil {
Expand Down Expand Up @@ -43,7 +43,7 @@ func ConvertJobsToDiggerJobs(jobType scheduler.DiggerCommand, vcsType models.Dig

log.Printf("marshalledJobsMap: %v\n", marshalledJobsMap)

batch, err := models.DB.CreateDiggerBatch(vcsType, githubInstallationId, repoOwner, repoName, repoFullName, prNumber, diggerConfigStr, branch, jobType, &commentId, gitlabProjectId)
batch, err := models.DB.CreateDiggerBatch(vcsType, githubInstallationId, repoOwner, repoName, repoFullName, prNumber, diggerConfigStr, branch, jobType, &commentId, placeholderCommentIdForReport, gitlabProjectId)
if err != nil {
return nil, nil, fmt.Errorf("failed to create batch: %v", err)
}
Expand Down
4 changes: 2 additions & 2 deletions ee/backend/controllers/gitlab.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ func handlePullRequestEvent(gitlabProvider utils.GitlabProvider, payload *gitlab
log.Printf("strconv.ParseInt error: %v", err)
utils.InitCommentReporter(glService, prNumber, fmt.Sprintf(":x: could not handle commentId: %v", err))
}
batchId, _, err := utils.ConvertJobsToDiggerJobs(*diggerCommand, models.DiggerVCSGitlab, organisationId, impactedJobsMap, impactedProjectsMap, projectsGraph, 0, branch, prNumber, repoOwner, repoName, repoFullName, commitSha, commentId, diggeryamlStr, projectId)
batchId, _, err := utils.ConvertJobsToDiggerJobs(*diggerCommand, models.DiggerVCSGitlab, organisationId, impactedJobsMap, impactedProjectsMap, projectsGraph, 0, branch, prNumber, repoOwner, repoName, repoFullName, commitSha, commentId, nil, diggeryamlStr, projectId)
if err != nil {
log.Printf("ConvertJobsToDiggerJobs error: %v", err)
utils.InitCommentReporter(glService, prNumber, fmt.Sprintf(":x: ConvertJobsToDiggerJobs error: %v", err))
Expand Down Expand Up @@ -524,7 +524,7 @@ func handleIssueCommentEvent(gitlabProvider utils.GitlabProvider, payload *gitla
log.Printf("ParseInt err: %v", err)
return fmt.Errorf("parseint error: %v", err)
}
batchId, _, err := utils.ConvertJobsToDiggerJobs(*diggerCommand, models.DiggerVCSGitlab, organisationId, impactedProjectsJobMap, impactedProjectsMap, projectsGraph, 0, branch, issueNumber, repoOwner, repoName, repoFullName, commitSha, commentId64, diggerYmlStr, projectId)
batchId, _, err := utils.ConvertJobsToDiggerJobs(*diggerCommand, models.DiggerVCSGitlab, organisationId, impactedProjectsJobMap, impactedProjectsMap, projectsGraph, 0, branch, issueNumber, repoOwner, repoName, repoFullName, commitSha, commentId64, nil, diggerYmlStr, projectId)
if err != nil {
log.Printf("ConvertJobsToDiggerJobs error: %v", err)
utils.InitCommentReporter(glService, issueNumber, fmt.Sprintf(":x: ConvertJobsToDiggerJobs error: %v", err))
Expand Down
2 changes: 1 addition & 1 deletion ee/backend/hooks/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ var DriftReconcilliationHook ce_controllers.IssueCommentHook = func(gh utils.Git
utils.InitCommentReporter(ghService, issueNumber, fmt.Sprintf(":x: could not handle commentId: %v", err))
}

batchId, _, err := utils.ConvertJobsToDiggerJobs(*diggerCommand, "github", orgId, impactedProjectsJobMap, impactedProjectsMap, projectsGraph, installationId, defaultBranch, issueNumber, repoOwner, repoName, repoFullName, "", reporterCommentId, diggerYmlStr, 0)
batchId, _, err := utils.ConvertJobsToDiggerJobs(*diggerCommand, "github", orgId, impactedProjectsJobMap, impactedProjectsMap, projectsGraph, installationId, defaultBranch, issueNumber, repoOwner, repoName, repoFullName, "", reporterCommentId, nil, diggerYmlStr, 0)
if err != nil {
log.Printf("ConvertJobsToDiggerJobs error: %v", err)
utils.InitCommentReporter(ghService, issueNumber, fmt.Sprintf(":x: ConvertJobsToDiggerJobs error: %v", err))
Expand Down
63 changes: 55 additions & 8 deletions libs/comment_utils/reporting/reporting.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,42 @@ type ReportStrategy interface {
Report(ciService ci.PullRequestService, PrNumber int, report string, reportFormatter func(report string) string, supportsCollapsibleComment bool) (commentId string, commentUrl string, error error)
}

type AlwaysSameCommentStrategy struct {
Title string
CommentId string
TimeOfRun time.Time
}

func (strategy AlwaysSameCommentStrategy) Report(ciService ci.PullRequestService, PrNumber int, report string, reportFormatter func(report string) string, supportsCollapsibleComment bool) (string, string, error) {
comments, err := ciService.GetComments(PrNumber)
if err != nil {
return "", "", fmt.Errorf("error getting comments: %v", err)
}

var commentBody *string
var commentUrl string
for _, comment := range comments {
if comment.Id == strategy.CommentId {
commentBody = comment.Body
commentUrl = comment.Url
}
}

var reportTitle string
if strategy.Title != "" {
reportTitle = strategy.Title + " " + strategy.TimeOfRun.Format("2006-01-02 15:04:05 (MST)")
} else {
reportTitle = "Digger run report at " + strategy.TimeOfRun.Format("2006-01-02 15:04:05 (MST)")
}

err = appendToExistingComment(ciService, PrNumber, *commentBody, report, reportTitle, strategy.CommentId, supportsCollapsibleComment)
if err != nil {
return "", "", fmt.Errorf("error while adding to existing comment: %v", err)
}

return strategy.CommentId, commentUrl, err
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix potential nil pointer dereference in AlwaysSameCommentStrategy.Report method

If no comment matching strategy.CommentId is found, commentBody remains nil. Dereferencing it at line 138 will cause a panic. Ensure commentBody is not nil before dereferencing.

 	for _, comment := range comments {
 		if comment.Id == strategy.CommentId {
 			commentBody = comment.Body
 			commentUrl = comment.Url
 		}
 	}

+	if commentBody == nil {
+		return "", "", fmt.Errorf("no comment found with CommentId: %v", strategy.CommentId)
+	}

 	err = appendToExistingComment(ciService, PrNumber, *commentBody, report, reportTitle, strategy.CommentId, supportsCollapsibleComment)
📝 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.

Suggested change
func (strategy AlwaysSameCommentStrategy) Report(ciService ci.PullRequestService, PrNumber int, report string, reportFormatter func(report string) string, supportsCollapsibleComment bool) (string, string, error) {
comments, err := ciService.GetComments(PrNumber)
if err != nil {
return "", "", fmt.Errorf("error getting comments: %v", err)
}
var commentBody *string
var commentUrl string
for _, comment := range comments {
if comment.Id == strategy.CommentId {
commentBody = comment.Body
commentUrl = comment.Url
}
}
var reportTitle string
if strategy.Title != "" {
reportTitle = strategy.Title + " " + strategy.TimeOfRun.Format("2006-01-02 15:04:05 (MST)")
} else {
reportTitle = "Digger run report at " + strategy.TimeOfRun.Format("2006-01-02 15:04:05 (MST)")
}
err = appendToExistingComment(ciService, PrNumber, *commentBody, report, reportTitle, strategy.CommentId, supportsCollapsibleComment)
if err != nil {
return "", "", fmt.Errorf("error while adding to existing comment: %v", err)
}
return strategy.CommentId, commentUrl, err
}
func (strategy AlwaysSameCommentStrategy) Report(ciService ci.PullRequestService, PrNumber int, report string, reportFormatter func(report string) string, supportsCollapsibleComment bool) (string, string, error) {
comments, err := ciService.GetComments(PrNumber)
if err != nil {
return "", "", fmt.Errorf("error getting comments: %v", err)
}
var commentBody *string
var commentUrl string
for _, comment := range comments {
if comment.Id == strategy.CommentId {
commentBody = comment.Body
commentUrl = comment.Url
}
}
var reportTitle string
if strategy.Title != "" {
reportTitle = strategy.Title + " " + strategy.TimeOfRun.Format("2006-01-02 15:04:05 (MST)")
} else {
reportTitle = "Digger run report at " + strategy.TimeOfRun.Format("2006-01-02 15:04:05 (MST)")
}
if commentBody == nil {
return "", "", fmt.Errorf("no comment found with CommentId: %v", strategy.CommentId)
}
err = appendToExistingComment(ciService, PrNumber, *commentBody, report, reportTitle, strategy.CommentId, supportsCollapsibleComment)
if err != nil {
return "", "", fmt.Errorf("error while adding to existing comment: %v", err)
}
return strategy.CommentId, commentUrl, err
}


type CommentPerRunStrategy struct {
Title string
TimeOfRun time.Time
Expand Down Expand Up @@ -156,26 +192,37 @@ func upsertComment(ciService ci.PullRequestService, PrNumber int, report string,
return fmt.Sprintf("%v", comment.Id), comment.Url, nil
}

err := appendToExistingComment(ciService, PrNumber, commentBody, report, reportTitle, commentIdForThisRun, supportsCollapsible)
if err != nil {
return "", "", err
}

return fmt.Sprintf("%v", commentIdForThisRun), commentUrl, nil

}

func appendToExistingComment(ciService ci.PullRequestService, prNumber int, existingCommentBody string, reportToAppend string, reportTitle string, commentId string, supportsCollapsible bool) error {

// strip first and last lines
lines := strings.Split(commentBody, "\n")
lines := strings.Split(existingCommentBody, "\n")
lines = lines[1 : len(lines)-1]
commentBody = strings.Join(lines, "\n")
existingCommentBody = strings.Join(lines, "\n")

commentBody = commentBody + "\n\n" + report + "\n"
existingCommentBody = existingCommentBody + "\n\n" + reportToAppend + "\n"

var completeComment string
if !supportsCollapsible {
completeComment = utils.AsComment(reportTitle)(commentBody)
completeComment = utils.AsComment(reportTitle)(existingCommentBody)
} else {
completeComment = utils.AsCollapsibleComment(reportTitle, false)(commentBody)
completeComment = utils.AsCollapsibleComment(reportTitle, false)(existingCommentBody)
}

err := ciService.EditComment(PrNumber, commentIdForThisRun, completeComment)
err := ciService.EditComment(prNumber, commentId, completeComment)

if err != nil {
return "", "", fmt.Errorf("error editing comment: %v", err)
return fmt.Errorf("error editing comment: %v", err)
}
return fmt.Sprintf("%v", commentIdForThisRun), commentUrl, nil
return nil
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Prevent slice bounds out of range in appendToExistingComment

If existingCommentBody has fewer than two lines, slicing lines[1:len(lines)-1] will cause a panic. Ensure that lines has at least two elements before slicing.

 	// strip first and last lines
 	lines := strings.Split(existingCommentBody, "\n")
+	if len(lines) >= 2 {
 		lines = lines[1 : len(lines)-1]
+	} else {
+		lines = []string{}
+	}
 	existingCommentBody = strings.Join(lines, "\n")
📝 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.

Suggested change
func appendToExistingComment(ciService ci.PullRequestService, prNumber int, existingCommentBody string, reportToAppend string, reportTitle string, commentId string, supportsCollapsible bool) error {
// strip first and last lines
lines := strings.Split(commentBody, "\n")
lines := strings.Split(existingCommentBody, "\n")
lines = lines[1 : len(lines)-1]
commentBody = strings.Join(lines, "\n")
existingCommentBody = strings.Join(lines, "\n")
commentBody = commentBody + "\n\n" + report + "\n"
existingCommentBody = existingCommentBody + "\n\n" + reportToAppend + "\n"
var completeComment string
if !supportsCollapsible {
completeComment = utils.AsComment(reportTitle)(commentBody)
completeComment = utils.AsComment(reportTitle)(existingCommentBody)
} else {
completeComment = utils.AsCollapsibleComment(reportTitle, false)(commentBody)
completeComment = utils.AsCollapsibleComment(reportTitle, false)(existingCommentBody)
}
err := ciService.EditComment(PrNumber, commentIdForThisRun, completeComment)
err := ciService.EditComment(prNumber, commentId, completeComment)
if err != nil {
return "", "", fmt.Errorf("error editing comment: %v", err)
return fmt.Errorf("error editing comment: %v", err)
}
return fmt.Sprintf("%v", commentIdForThisRun), commentUrl, nil
return nil
func appendToExistingComment(ciService ci.PullRequestService, prNumber int, existingCommentBody string, reportToAppend string, reportTitle string, commentId string, supportsCollapsible bool) error {
// strip first and last lines
lines := strings.Split(existingCommentBody, "\n")
if len(lines) >= 2 {
lines = lines[1 : len(lines)-1]
} else {
lines = []string{}
}
existingCommentBody = strings.Join(lines, "\n")
existingCommentBody = existingCommentBody + "\n\n" + reportToAppend + "\n"
var completeComment string
if !supportsCollapsible {
completeComment = utils.AsComment(reportTitle)(existingCommentBody)
} else {
completeComment = utils.AsCollapsibleComment(reportTitle, false)(existingCommentBody)
}
err := ciService.EditComment(prNumber, commentId, completeComment)
if err != nil {
return fmt.Errorf("error editing comment: %v", err)
}
return nil

}

type LatestRunCommentStrategy struct {
Expand Down
7 changes: 4 additions & 3 deletions libs/spec/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@ import (
)

type ReporterSpec struct {
ReporterType string `json:"reporter_type"`
ReportingStrategy string `json:"reporting_strategy"`
ReportTerraformOutput bool `json:"report_terraform_output"`
ReporterType string `json:"reporter_type"`
ReportingStrategy string `json:"reporting_strategy"`
ReportTerraformOutput bool `json:"report_terraform_output"`
ReportCommentId *string `json:"report_comment_id,omitempty"`
}

type CommentUpdaterSpec struct {
Expand Down
7 changes: 7 additions & 0 deletions libs/spec/providers.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,13 @@ func (r ReporterProvider) GetReporter(title string, reporterSpec ReporterSpec, c
return reporting.LatestRunCommentStrategy{
TimeOfRun: time.Now(),
}
case "always_same_comment":
return reporting.AlwaysSameCommentStrategy{
Title: title,
TimeOfRun: time.Now(),
CommentId: *reporterSpec.ReportCommentId,
}

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add nil check for ReportCommentId

The code dereferences reporterSpec.ReportCommentId without checking if it's nil, which could lead to a panic.

Add a nil check before using the pointer:

 case "always_same_comment":
+    if reporterSpec.ReportCommentId == nil {
+        return reporting.MultipleCommentsStrategy{}
+    }
     return reporting.AlwaysSameCommentStrategy{
         Title:     title,
         TimeOfRun: time.Now(),
         CommentId: *reporterSpec.ReportCommentId,
     }
📝 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.

Suggested change
case "always_same_comment":
return reporting.AlwaysSameCommentStrategy{
Title: title,
TimeOfRun: time.Now(),
CommentId: *reporterSpec.ReportCommentId,
}
case "always_same_comment":
if reporterSpec.ReportCommentId == nil {
return reporting.MultipleCommentsStrategy{}
}
return reporting.AlwaysSameCommentStrategy{
Title: title,
TimeOfRun: time.Now(),
CommentId: *reporterSpec.ReportCommentId,
}

default:
return reporting.MultipleCommentsStrategy{}
}
Expand Down
Loading