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

Comments UX improvements #1071

Merged
merged 44 commits into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
de36103
backend reportStatus method update
motatoes Jan 15, 2024
413ce73
cli to report resources updated back to backend
motatoes Jan 15, 2024
b1645c7
update mock
motatoes Jan 15, 2024
a5e5ab3
fix backend build, add tests
motatoes Jan 15, 2024
fa88eb2
implement publish comment method
motatoes Jan 16, 2024
818e042
create reporter
motatoes Jan 16, 2024
cbd839a
created and use commentReporter
motatoes Jan 16, 2024
a377587
fix cli build
motatoes Jan 16, 2024
18d62d3
receive comment by cli
motatoes Jan 16, 2024
14a2113
run migrations for commentID
motatoes Jan 16, 2024
97cdd8e
send commentid as val
motatoes Jan 16, 2024
4c1b941
replace with string
motatoes Jan 16, 2024
632dc41
add logging for trigger github workflow
motatoes Jan 16, 2024
f608d18
proper conversion to int
motatoes Jan 16, 2024
5b01c43
cli parse as int64
motatoes Jan 16, 2024
7213908
parse comments to int64
motatoes Jan 16, 2024
b869cd2
send the right execution results
motatoes Jan 18, 2024
4ffc944
send the right execution results
motatoes Jan 18, 2024
e9b6bd3
fix plan summary logic
motatoes Jan 18, 2024
481cdf8
update comments from cli
motatoes Jan 18, 2024
7bc28eb
fix build
motatoes Jan 18, 2024
3fc84e2
dump object
motatoes Jan 18, 2024
b5fdfb3
log json struct
motatoes Jan 18, 2024
c2b2024
preload job summary
motatoes Jan 18, 2024
ec394a7
fix resources
motatoes Jan 18, 2024
8847838
storage and joins
motatoes Jan 18, 2024
70f3d5a
Merge branch 'develop' into feat/comments-ux-improvments
motatoes Jan 24, 2024
9431616
go mod tidy
motatoes Jan 24, 2024
ebf383e
fix imports
motatoes Jan 24, 2024
5003725
fix the query
motatoes Jan 24, 2024
93da167
add test, fix case of Id
motatoes Jan 25, 2024
cf5fa92
fix build
motatoes Jan 25, 2024
746fa6d
run migrations
motatoes Jan 25, 2024
1e81001
tidying up and fixing tests
motatoes Jan 25, 2024
548b9e7
fix cli tests
motatoes Jan 25, 2024
b290127
fix cli tests
motatoes Jan 25, 2024
037fbde
update comments
motatoes Jan 25, 2024
6e62928
fix commenting issue
motatoes Jan 25, 2024
c2eea85
updated emojis
motatoes Jan 25, 2024
8ecc62e
handle no projects impacted case
motatoes Jan 25, 2024
7750ad3
fix empty checking logic
motatoes Jan 25, 2024
91fed84
add right emojis
motatoes Jan 25, 2024
5c07339
add a project holder string
motatoes Jan 25, 2024
652e692
update comment on every single stage of cli
motatoes Jan 25, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 69 additions & 35 deletions backend/controllers/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"context"
"encoding/json"
"fmt"
orchestrator_scheduler "github.com/diggerhq/digger/libs/orchestrator/scheduler"
"github.com/google/uuid"
"log"
"math/rand"
"net/http"
Expand All @@ -23,7 +25,6 @@ import (
"github.com/dominikbraun/graph"
"github.com/gin-gonic/gin"
"github.com/google/go-github/v58/github"
"github.com/google/uuid"
"github.com/samber/lo"
"golang.org/x/oauth2"
)
Expand Down Expand Up @@ -417,21 +418,41 @@ func handlePullRequestEvent(gh utils.GithubClientProvider, payload *github.PullR
return fmt.Errorf("error getting digger config")
}

commentReporter, err := utils.InitCommentReporter(ghService, prNumber, ":construction_worker: Digger starting...")
if err != nil {
log.Printf("Error initializing comment reporter: %v", err)
return fmt.Errorf("error initializing comment reporter")
}

impactedProjects, _, err := dg_github.ProcessGitHubPullRequestEvent(payload, config, projectsGraph, ghService)
if err != nil {
log.Printf("Error processing event: %v", err)
utils.InitCommentReporter(ghService, prNumber, fmt.Sprintf(":x: Error processing event: %v", err))
return fmt.Errorf("error processing event")
}

jobsForImpactedProjects, _, err := dg_github.ConvertGithubPullRequestEventToJobs(payload, impactedProjects, nil, config.Workflows)
if err != nil {
log.Printf("Error converting event to jobsForImpactedProjects: %v", err)
utils.InitCommentReporter(ghService, prNumber, fmt.Sprintf(":x: Error converting event to jobsForImpactedProjects: %v", err))
return fmt.Errorf("error converting event to jobsForImpactedProjects")
}

err = setPRStatusForJobs(ghService, prNumber, jobsForImpactedProjects)
err = utils.ReportInitialJobsStatus(commentReporter, jobsForImpactedProjects)
if err != nil {
log.Printf("Failed to comment initial status for jobs: %v", err)
utils.InitCommentReporter(ghService, prNumber, fmt.Sprintf(":x: Failed to comment initial status for jobs: %v", err))
return fmt.Errorf("failed to comment initial status for jobs")
}

if len(jobsForImpactedProjects) == 0 {
return nil
}

err = utils.SetPRStatusForJobs(ghService, prNumber, jobsForImpactedProjects)
if err != nil {
log.Printf("error setting status for PR: %v", err)
utils.InitCommentReporter(ghService, prNumber, fmt.Sprintf(":x: error setting status for PR: %v", err))
fmt.Errorf("error setting status for PR: %v", err)
}

Expand All @@ -448,18 +469,21 @@ func handlePullRequestEvent(gh utils.GithubClientProvider, payload *github.PullR
repo, err := GetRepoByInstllationId(installationId, repoOwner, repoName)
if err != nil {
log.Printf("GetRepoByInstallationId error: %v", err)
utils.InitCommentReporter(ghService, prNumber, fmt.Sprintf(":x: GetRepoByInstallationId error: %v", err))
return fmt.Errorf("error converting jobs, GetRepoByInstallationId error: %v", err)
}
batchType := getBatchType(jobsForImpactedProjects)
batchId, _, err := utils.ConvertJobsToDiggerJobs(impactedJobsMap, impactedProjectsMap, projectsGraph, installationId, *branch, prNumber, repoOwner, repoName, repoFullName, repo.DiggerConfig, batchType)
batchId, _, err := utils.ConvertJobsToDiggerJobs(impactedJobsMap, impactedProjectsMap, projectsGraph, installationId, *branch, prNumber, repoOwner, repoName, repoFullName, commentReporter.CommentId, repo.DiggerConfig, batchType)
if err != nil {
log.Printf("ConvertJobsToDiggerJobs error: %v", err)
utils.InitCommentReporter(ghService, prNumber, fmt.Sprintf(":x: ConvertJobsToDiggerJobs error: %v", err))
return fmt.Errorf("error converting jobs")
}

err = TriggerDiggerJobs(ghService.Client, repoOwner, repoName, batchId, prNumber, ghService)
if err != nil {
log.Printf("TriggerDiggerJobs error: %v", err)
utils.InitCommentReporter(ghService, prNumber, fmt.Sprintf(":x: TriggerDiggerJobs error: %v", err))
return fmt.Errorf("error triggerring GitHub Actions for Digger Jobs")
}

Expand Down Expand Up @@ -533,35 +557,17 @@ func GetRepoByInstllationId(installationId int64, repoOwner string, repoName str
return repo, nil
}

func getBatchType(jobs []orchestrator.Job) models.DiggerBatchType {
func getBatchType(jobs []orchestrator.Job) orchestrator_scheduler.DiggerBatchType {
allJobsContainApply := lo.EveryBy(jobs, func(job orchestrator.Job) bool {
return lo.Contains(job.Commands, "digger apply")
})
if allJobsContainApply == true {
return models.BatchTypeApply
return orchestrator_scheduler.BatchTypeApply
} else {
return models.BatchTypePlan
return orchestrator_scheduler.BatchTypePlan
}
}

func setPRStatusForJobs(prService *dg_github.GithubService, prNumber int, jobs []orchestrator.Job) error {
for _, job := range jobs {
for _, command := range job.Commands {
var err error
switch command {
case "digger plan":
err = prService.SetStatus(prNumber, "pending", job.ProjectName+"/plan")
case "digger apply":
err = prService.SetStatus(prNumber, "pending", job.ProjectName+"/apply")
}
if err != nil {
log.Printf("Erorr setting status: %v", err)
return fmt.Errorf("Error setting pr status: %v", err)
}
}
}
return nil
}
func handleIssueCommentEvent(gh utils.GithubClientProvider, payload *github.IssueCommentEvent) error {
installationId := *payload.Installation.ID
repoName := *payload.Repo.Name
Expand All @@ -570,21 +576,34 @@ func handleIssueCommentEvent(gh utils.GithubClientProvider, payload *github.Issu
cloneURL := *payload.Repo.CloneURL
issueNumber := *payload.Issue.Number

if *payload.Action != "created" {
log.Printf("comment is not of type 'created', ignoring")
return nil
}

ghService, config, projectsGraph, branch, err := getDiggerConfig(gh, installationId, repoFullName, repoOwner, repoName, cloneURL, issueNumber)
if err != nil {
log.Printf("getDiggerConfig error: %v", err)
return fmt.Errorf("error getting digger config")
}

commentReporter, err := utils.InitCommentReporter(ghService, issueNumber, ":construction_worker: Digger starting....")
if err != nil {
log.Printf("Error initializing comment reporter: %v", err)
return fmt.Errorf("error initializing comment reporter")
}

prBranchName, err := ghService.GetBranchName(issueNumber)
if err != nil {
log.Printf("GetBranchName error: %v", err)
utils.InitCommentReporter(ghService, issueNumber, fmt.Sprintf(":x: GetBranchName error: %v", err))
return fmt.Errorf("error while fetching branch name")
}

impactedProjects, requestedProject, _, err := dg_github.ProcessGitHubIssueCommentEvent(payload, config, projectsGraph, ghService)
if err != nil {
log.Printf("Error processing event: %v", err)
utils.InitCommentReporter(ghService, issueNumber, fmt.Sprintf(":x: Error processing event: %v", err))
return fmt.Errorf("error processing event")
}
log.Printf("GitHub IssueComment event processed successfully\n")
Expand All @@ -595,16 +614,28 @@ func handleIssueCommentEvent(gh utils.GithubClientProvider, payload *github.Issu
}

jobs, _, err := dg_github.ConvertGithubIssueCommentEventToJobs(payload, impactedProjects, requestedProject, config.Workflows, prBranchName)

if err != nil {
log.Printf("Error converting event to jobs: %v", err)
utils.InitCommentReporter(ghService, issueNumber, fmt.Sprintf(":x: Error converting event to jobs: %v", err))
return fmt.Errorf("error converting event to jobs")
}
log.Printf("GitHub IssueComment event converted to Jobs successfully\n")

err = setPRStatusForJobs(ghService, issueNumber, jobs)
err = utils.ReportInitialJobsStatus(commentReporter, jobs)
if err != nil {
log.Printf("Failed to comment initial status for jobs: %v", err)
utils.InitCommentReporter(ghService, issueNumber, fmt.Sprintf(":x: Failed to comment initial status for jobs: %v", err))
return fmt.Errorf("failed to comment initial status for jobs")
}

if len(jobs) == 0 {
return nil
}

err = utils.SetPRStatusForJobs(ghService, issueNumber, jobs)
if err != nil {
log.Printf("error setting status for PR: %v", err)
utils.InitCommentReporter(ghService, issueNumber, fmt.Sprintf(":x: error setting status for PR: %v", err))
fmt.Errorf("error setting status for PR: %v", err)
}

Expand All @@ -624,21 +655,28 @@ func handleIssueCommentEvent(gh utils.GithubClientProvider, payload *github.Issu
return fmt.Errorf("error converting jobs, GetRepoByInstallationId error: %v", err)
}
batchType := getBatchType(jobs)
batchId, _, err := utils.ConvertJobsToDiggerJobs(impactedProjectsJobMap, impactedProjectsMap, projectsGraph, installationId, *branch, issueNumber, repoOwner, repoName, repoFullName, repo.DiggerConfig, batchType)
batchId, _, err := utils.ConvertJobsToDiggerJobs(impactedProjectsJobMap, impactedProjectsMap, projectsGraph, installationId, *branch, issueNumber, repoOwner, repoName, repoFullName, commentReporter.CommentId, repo.DiggerConfig, batchType)
if err != nil {
log.Printf("ConvertJobsToDiggerJobs error: %v", err)
utils.InitCommentReporter(ghService, issueNumber, fmt.Sprintf(":x: ConvertJobsToDiggerJobs error: %v", err))
return fmt.Errorf("error convertingjobs")
}

err = TriggerDiggerJobs(ghService.Client, repoOwner, repoName, batchId, issueNumber, ghService)
if err != nil {
log.Printf("TriggerDiggerJobs error: %v", err)
utils.InitCommentReporter(ghService, issueNumber, fmt.Sprintf(":x: TriggerDiggerJobs error: %v", err))
return fmt.Errorf("error triggerring GitHub Actions for Digger Jobs")
}
return nil
}

func TriggerDiggerJobs(client *github.Client, repoOwner string, repoName string, batchId *uuid.UUID, prNumber int, prService *dg_github.GithubService) error {
batch, err := models.DB.GetDiggerBatch(batchId)
if err != nil {
log.Printf("failed to get digger batch, %v\n", err)
return fmt.Errorf("failed to get digger batch, %v\n", err)
}
diggerJobs, err := models.DB.GetPendingParentDiggerJobs(batchId)

if err != nil {
Expand All @@ -649,18 +687,14 @@ func TriggerDiggerJobs(client *github.Client, repoOwner string, repoName string,
log.Printf("number of diggerJobs:%v\n", len(diggerJobs))

for _, job := range diggerJobs {
if job.SerializedJob == nil {
if job.SerializedJobSpec == nil {
return fmt.Errorf("GitHub job can't be nil")
}
jobString := string(job.SerializedJob)
jobString := string(job.SerializedJobSpec)
log.Printf("jobString: %v \n", jobString)

// TODO: make workflow file name configurable
_, err = client.Actions.CreateWorkflowDispatchEventByFileName(context.Background(), repoOwner, repoName, "digger_workflow.yml", github.CreateWorkflowDispatchEventRequest{
Ref: job.Batch.BranchName,
Inputs: map[string]interface{}{"job": jobString, "id": job.DiggerJobId},
})

err = utils.TriggerGithubWorkflow(client, repoOwner, repoName, job, jobString, *batch.CommentId)
if err != nil {
log.Printf("failed to trigger github workflow, %v\n", err)
return fmt.Errorf("failed to trigger github workflow, %v\n", err)
Expand All @@ -670,7 +704,7 @@ func TriggerDiggerJobs(client *github.Client, repoOwner string, repoName string,
return fmt.Errorf("failed to set pr status, %v\n", err)
}

job.Status = models.DiggerJobTriggered
job.Status = orchestrator_scheduler.DiggerJobTriggered
err := models.DB.UpdateDiggerJob(&job)
if err != nil {
log.Printf("failed to trigger github workflow, %v\n", err)
Expand Down
47 changes: 24 additions & 23 deletions backend/controllers/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package controllers

import (
"encoding/json"
orchestrator_scheduler "github.com/diggerhq/digger/libs/orchestrator/scheduler"
"log"
"os"
"strings"
Expand Down Expand Up @@ -731,10 +732,10 @@ func TestJobsTreeWithOneJobsAndTwoProjects(t *testing.T) {
graph, err := configuration.CreateProjectDependencyGraph(projects)
assert.NoError(t, err)

_, result, err := utils.ConvertJobsToDiggerJobs(jobs, projectMap, graph, 41584295, "", 2, "diggerhq", "parallel_jobs_demo", "diggerhq/parallel_jobs_demo", "test", models.BatchTypeApply)
_, result, err := utils.ConvertJobsToDiggerJobs(jobs, projectMap, graph, 41584295, "", 2, "diggerhq", "parallel_jobs_demo", "diggerhq/parallel_jobs_demo", 123, "test", orchestrator_scheduler.BatchTypeApply)
assert.NoError(t, err)
assert.Equal(t, 1, len(result))
parentLinks, err := models.DB.GetDiggerJobParentLinksChildId(&result["dev"].DiggerJobId)
parentLinks, err := models.DB.GetDiggerJobParentLinksChildId(&result["dev"].DiggerJobID)
assert.NoError(t, err)
assert.Empty(t, parentLinks)
assert.NotContains(t, result, "prod")
Expand All @@ -760,17 +761,17 @@ func TestJobsTreeWithTwoDependantJobs(t *testing.T) {
projectMap["dev"] = project1
projectMap["prod"] = project2

_, result, err := utils.ConvertJobsToDiggerJobs(jobs, projectMap, graph, 123, "", 2, "", "", "test", "test", models.BatchTypeApply)
_, result, err := utils.ConvertJobsToDiggerJobs(jobs, projectMap, graph, 123, "", 2, "", "", "test", 123, "test", orchestrator_scheduler.BatchTypeApply)
assert.NoError(t, err)
assert.Equal(t, 2, len(result))

parentLinks, err := models.DB.GetDiggerJobParentLinksChildId(&result["dev"].DiggerJobId)
parentLinks, err := models.DB.GetDiggerJobParentLinksChildId(&result["dev"].DiggerJobID)
assert.NoError(t, err)
assert.Empty(t, parentLinks)
parentLinks, err = models.DB.GetDiggerJobParentLinksChildId(&result["prod"].DiggerJobId)
parentLinks, err = models.DB.GetDiggerJobParentLinksChildId(&result["prod"].DiggerJobID)
assert.NoError(t, err)

assert.Equal(t, result["dev"].DiggerJobId, parentLinks[0].ParentDiggerJobId)
assert.Equal(t, result["dev"].DiggerJobID, parentLinks[0].ParentDiggerJobId)
}

func TestJobsTreeWithTwoIndependentJobs(t *testing.T) {
Expand All @@ -793,17 +794,17 @@ func TestJobsTreeWithTwoIndependentJobs(t *testing.T) {
projectMap["dev"] = project1
projectMap["prod"] = project2

_, result, err := utils.ConvertJobsToDiggerJobs(jobs, projectMap, graph, 123, "", 2, "", "", "test", "test", models.BatchTypeApply)
_, result, err := utils.ConvertJobsToDiggerJobs(jobs, projectMap, graph, 123, "", 2, "", "", "test", 123, "test", orchestrator_scheduler.BatchTypeApply)
assert.NoError(t, err)
assert.Equal(t, 2, len(result))
parentLinks, err := models.DB.GetDiggerJobParentLinksChildId(&result["dev"].DiggerJobId)
parentLinks, err := models.DB.GetDiggerJobParentLinksChildId(&result["dev"].DiggerJobID)
assert.NoError(t, err)
assert.Empty(t, parentLinks)
assert.NotNil(t, result["dev"].SerializedJob)
parentLinks, err = models.DB.GetDiggerJobParentLinksChildId(&result["prod"].DiggerJobId)
assert.NotNil(t, result["dev"].SerializedJobSpec)
parentLinks, err = models.DB.GetDiggerJobParentLinksChildId(&result["prod"].DiggerJobID)
assert.NoError(t, err)
assert.Empty(t, parentLinks)
assert.NotNil(t, result["prod"].SerializedJob)
assert.NotNil(t, result["prod"].SerializedJobSpec)
}

func TestJobsTreeWithThreeLevels(t *testing.T) {
Expand Down Expand Up @@ -838,32 +839,32 @@ func TestJobsTreeWithThreeLevels(t *testing.T) {
projectMap["555"] = project5
projectMap["666"] = project6

_, result, err := utils.ConvertJobsToDiggerJobs(jobs, projectMap, graph, 123, "", 2, "", "", "test", "test", models.BatchTypeApply)
_, result, err := utils.ConvertJobsToDiggerJobs(jobs, projectMap, graph, 123, "", 2, "", "", "test", 123, "test", orchestrator_scheduler.BatchTypeApply)
assert.NoError(t, err)
assert.Equal(t, 6, len(result))
parentLinks, err := models.DB.GetDiggerJobParentLinksChildId(&result["111"].DiggerJobId)
parentLinks, err := models.DB.GetDiggerJobParentLinksChildId(&result["111"].DiggerJobID)
assert.NoError(t, err)
assert.Empty(t, parentLinks)

parentLinks, err = models.DB.GetDiggerJobParentLinksChildId(&result["222"].DiggerJobId)
parentLinks, err = models.DB.GetDiggerJobParentLinksChildId(&result["222"].DiggerJobID)
assert.NoError(t, err)
assert.Equal(t, result["111"].DiggerJobId, parentLinks[0].ParentDiggerJobId)
assert.Equal(t, result["111"].DiggerJobID, parentLinks[0].ParentDiggerJobId)

parentLinks, err = models.DB.GetDiggerJobParentLinksChildId(&result["333"].DiggerJobId)
parentLinks, err = models.DB.GetDiggerJobParentLinksChildId(&result["333"].DiggerJobID)
assert.NoError(t, err)
assert.Equal(t, result["111"].DiggerJobId, parentLinks[0].ParentDiggerJobId)
assert.Equal(t, result["111"].DiggerJobID, parentLinks[0].ParentDiggerJobId)

parentLinks, err = models.DB.GetDiggerJobParentLinksChildId(&result["444"].DiggerJobId)
parentLinks, err = models.DB.GetDiggerJobParentLinksChildId(&result["444"].DiggerJobID)
assert.NoError(t, err)
assert.Equal(t, result["222"].DiggerJobId, parentLinks[0].ParentDiggerJobId)
assert.Equal(t, result["222"].DiggerJobID, parentLinks[0].ParentDiggerJobId)

parentLinks, err = models.DB.GetDiggerJobParentLinksChildId(&result["555"].DiggerJobId)
parentLinks, err = models.DB.GetDiggerJobParentLinksChildId(&result["555"].DiggerJobID)
assert.NoError(t, err)
assert.Equal(t, result["222"].DiggerJobId, parentLinks[0].ParentDiggerJobId)
assert.Equal(t, result["222"].DiggerJobID, parentLinks[0].ParentDiggerJobId)

parentLinks, err = models.DB.GetDiggerJobParentLinksChildId(&result["666"].DiggerJobId)
parentLinks, err = models.DB.GetDiggerJobParentLinksChildId(&result["666"].DiggerJobID)
assert.NoError(t, err)
assert.Equal(t, result["333"].DiggerJobId, parentLinks[0].ParentDiggerJobId)
assert.Equal(t, result["333"].DiggerJobID, parentLinks[0].ParentDiggerJobId)
}

func TestGithubInstallationRepoAddedEvent(t *testing.T) {
Expand Down
Loading
Loading