-
Notifications
You must be signed in to change notification settings - Fork 563
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
repo details page #1888
repo details page #1888
Changes from 2 commits
6f62e73
d98e004
7f99aa6
ced36c6
5b522ad
fd6f0b5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
package controllers | ||
|
||
import ( | ||
"errors" | ||
"github.com/diggerhq/digger/backend/middleware" | ||
"github.com/diggerhq/digger/backend/models" | ||
"github.com/gin-gonic/gin" | ||
"gorm.io/gorm" | ||
"net/http" | ||
) | ||
|
||
func GetDashboardStatusApi(c *gin.Context) { | ||
organisationId := c.GetString(middleware.ORGANISATION_ID_KEY) | ||
organisationSource := c.GetString(middleware.ORGANISATION_SOURCE_KEY) | ||
|
||
var org models.Organisation | ||
err := models.DB.GormDB.Where("external_id = ? AND external_source = ?", organisationId, organisationSource).First(&org).Error | ||
if err != nil { | ||
if errors.Is(err, gorm.ErrRecordNotFound) { | ||
c.String(http.StatusNotFound, "Could not find organisation: "+organisationId) | ||
} | ||
return | ||
} | ||
|
||
response := make(map[string]interface{}) | ||
|
||
c.JSON(http.StatusOK, response) | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,65 @@ | ||||||||||||||||||||||||||||||||
package controllers | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
import ( | ||||||||||||||||||||||||||||||||
"errors" | ||||||||||||||||||||||||||||||||
"github.com/diggerhq/digger/backend/middleware" | ||||||||||||||||||||||||||||||||
"github.com/diggerhq/digger/backend/models" | ||||||||||||||||||||||||||||||||
orchestrator_scheduler "github.com/diggerhq/digger/libs/scheduler" | ||||||||||||||||||||||||||||||||
"github.com/gin-gonic/gin" | ||||||||||||||||||||||||||||||||
"gorm.io/gorm" | ||||||||||||||||||||||||||||||||
"log" | ||||||||||||||||||||||||||||||||
"net/http" | ||||||||||||||||||||||||||||||||
"strconv" | ||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
func GetJobsForRepoApi(c *gin.Context) { | ||||||||||||||||||||||||||||||||
organisationId := c.GetString(middleware.ORGANISATION_ID_KEY) | ||||||||||||||||||||||||||||||||
organisationSource := c.GetString(middleware.ORGANISATION_SOURCE_KEY) | ||||||||||||||||||||||||||||||||
repoId := c.Param("repo_id") | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
if repoId == "" { | ||||||||||||||||||||||||||||||||
log.Printf("missing parameter: repo_full_name") | ||||||||||||||||||||||||||||||||
c.String(http.StatusBadRequest, "missing parameter: repo_full_name") | ||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
var org models.Organisation | ||||||||||||||||||||||||||||||||
err := models.DB.GormDB.Where("external_id = ? AND external_source = ?", organisationId, organisationSource).First(&org).Error | ||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||
if errors.Is(err, gorm.ErrRecordNotFound) { | ||||||||||||||||||||||||||||||||
c.String(http.StatusNotFound, "Could not find organisation: "+organisationId) | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: Missing return statement after sending error response. If organization is not found, the function should return after sending the error response.
Suggested change
|
||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add missing return statement after error response When an error is encountered finding the organization, the function sets the status code and error message but there's no explicit return statement. This could lead to the function continuing execution unexpectedly. if err != nil {
if errors.Is(err, gorm.ErrRecordNotFound) {
c.String(http.StatusNotFound, "Could not find organisation: "+organisationId)
+ return
}
+ log.Printf("error finding organisation: %v", err)
+ c.String(http.StatusInternalServerError, "Error finding organisation")
return
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
repo, err := models.DB.GetRepoById(org.ID, repoId) | ||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||
log.Printf("could not fetch repo details") | ||||||||||||||||||||||||||||||||
c.String(http.StatusInternalServerError, "Unknown error occurred while fetching jobs from database") | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Log message doesn't include the actual error which makes debugging difficult
Suggested change
|
||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
jobsRes, err := models.DB.GetJobsByRepoName(org.ID, repo.RepoFullName) | ||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||
log.Printf("could not fetch job details") | ||||||||||||||||||||||||||||||||
c.String(http.StatusInternalServerError, "Unknown error occurred while fetching jobs from database") | ||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
// update the values of "status" accordingly | ||||||||||||||||||||||||||||||||
for i, j := range jobsRes { | ||||||||||||||||||||||||||||||||
statusInt, err := strconv.Atoi(j.Status) | ||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||
log.Printf("could not convert status to string: job id: %v status: %v", j.ID, j.Status) | ||||||||||||||||||||||||||||||||
continue | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
statusI := orchestrator_scheduler.DiggerJobStatus(statusInt) | ||||||||||||||||||||||||||||||||
jobsRes[i].Status = statusI.ToString() | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
response := make(map[string]interface{}) | ||||||||||||||||||||||||||||||||
response["repo"] = repo | ||||||||||||||||||||||||||||||||
response["jobs"] = jobsRes | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
c.JSON(http.StatusOK, response) | ||||||||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
package models | ||
|
||
import "time" | ||
|
||
type JobQueryResult struct { | ||
ID uint `gorm:"column:id"` | ||
CreatedAt time.Time `gorm:"column:created_at"` | ||
UpdatedAt time.Time `gorm:"column:updated_at"` | ||
DeletedAt *time.Time `gorm:"column:deleted_at"` | ||
DiggerJobID string `gorm:"column:digger_job_id"` | ||
Status string `gorm:"column:status"` | ||
WorkflowRunURL string `gorm:"column:workflow_run_url"` | ||
WorkflowFile string `gorm:"column:workflow_file"` | ||
TerraformOutput string `gorm:"column:terraform_output"` | ||
PRNumber int `gorm:"column:pr_number"` | ||
RepoFullName string `gorm:"column:repo_full_name"` | ||
BranchName string `gorm:"column:branch_name"` | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,61 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
package models | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"github.com/diggerhq/digger/libs/scheduler" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"time" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
func (db *Database) GetRepoCount(orgID uint) (int64, error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
var count int64 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
thirtyDaysAgo := time.Now().AddDate(0, 0, -30) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
result := db.GormDB.Model(&Repo{}). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Where("organisation_id = ? AND created_at >= ?", orgID, thirtyDaysAgo). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Count(&count) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if result.Error != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return 0, result.Error | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return count, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
func (db *Database) GetJobsCountThisMonth(orgID uint) (int64, error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
var count int64 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
thirtyDaysAgo := time.Now().AddDate(0, 0, -30) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
result := db.GormDB.Model(DiggerJob{}). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Joins("JOIN digger_batches ON digger_jobs.batch_id = digger_batches.id"). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Joins("JOIN github_app_installation_links ON digger_batches.github_installation_id = github_app_installation_links.github_installation_id"). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Joins("JOIN organisations ON github_app_installation_links.organisation_id = organisations.id"). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Where("digger_jobs.created_at >= ?", thirtyDaysAgo). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Count(&count) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: Missing WHERE clause to filter by the provided organization ID. This will count jobs for all organizations, not just the specified one.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if result.Error != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return 0, result.Error | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return count, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing organization filter in jobs count query The GetJobsCountThisMonth function doesn't filter by the provided orgID parameter in its WHERE clause, which seems inconsistent with the function's purpose. This would return counts for all organizations instead of just the requested one. result := db.GormDB.Model(DiggerJob{}).
Joins("JOIN digger_batches ON digger_jobs.batch_id = digger_batches.id").
Joins("JOIN github_app_installation_links ON digger_batches.github_installation_id = github_app_installation_links.github_installation_id").
Joins("JOIN organisations ON github_app_installation_links.organisation_id = organisations.id").
Where("digger_jobs.created_at >= ?", thirtyDaysAgo).
+ Where("organisations.id = ?", orgID).
Count(&count) 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
func (db *Database) GetSuccessfulJobsCountThisMonth(orgID uint) (int64, error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
var count int64 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
thirtyDaysAgo := time.Now().AddDate(0, 0, -30) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
result := db.GormDB.Model(DiggerJob{}). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Joins("JOIN digger_batches ON digger_jobs.batch_id = digger_batches.id"). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Joins("JOIN github_app_installation_links ON digger_batches.github_installation_id = github_app_installation_links.github_installation_id"). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Joins("JOIN organisations ON github_app_installation_links.organisation_id = organisations.id"). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Where("digger_jobs.created_at >= ?", thirtyDaysAgo). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Where("digger_jobs.status = ?", scheduler.DiggerJobSucceeded). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: Missing WHERE clause to filter by the provided organization ID. This will count successful jobs for all organizations, not just the specified one.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Count(&count) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if result.Error != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return 0, result.Error | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return count, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing organization filter in successful jobs count query Similar to the previous function, this query is missing the organization filter. Also, consider combining the WHERE clauses for better readability. result := db.GormDB.Model(DiggerJob{}).
Joins("JOIN digger_batches ON digger_jobs.batch_id = digger_batches.id").
Joins("JOIN github_app_installation_links ON digger_batches.github_installation_id = github_app_installation_links.github_installation_id").
Joins("JOIN organisations ON github_app_installation_links.organisation_id = organisations.id").
- Where("digger_jobs.created_at >= ?", thirtyDaysAgo).
- Where("digger_jobs.status = ?", scheduler.DiggerJobSucceeded).
+ Where("digger_jobs.created_at >= ? AND digger_jobs.status = ? AND organisations.id = ?",
+ thirtyDaysAgo, scheduler.DiggerJobSucceeded, orgID).
Count(&count) 📝 Committable suggestion
Suggested change
|
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.
logic: Missing proper error handling for non-ErrRecordNotFound errors. Should return an appropriate HTTP status code and error message.