From c981b2aad32f8dea5ba94c1b0f5502103efb60a8 Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Wed, 20 Oct 2021 09:08:52 -0700 Subject: [PATCH 1/4] Configuring Log Streaming to use RepoRelDir and Workspace to generate URL when project name is not specified --- server/router.go | 41 ++++++++++++++++++++-------- server/router_test.go | 62 +++++++++++++++++++++++++++++++++++++++++++ server/server.go | 25 ++++++++++------- 3 files changed, 108 insertions(+), 20 deletions(-) diff --git a/server/router.go b/server/router.go index f66e63eba..bab6d5ea2 100644 --- a/server/router.go +++ b/server/router.go @@ -3,6 +3,7 @@ package server import ( "fmt" "net/url" + "strings" "github.com/gorilla/mux" "github.com/pkg/errors" @@ -18,8 +19,11 @@ type Router struct { // LockViewRouteName is the named route for the lock view that can be Get'd // from the Underlying router. LockViewRouteName string - // ProjectJobsViewRouteName is the named route for the projects active jobs - ProjectJobsViewRouteName string + // ProjectBasedJobsViewRouteName is the named route for the projects active jobs + ProjectBasedJobsViewRouteName string + // DirWorkspaceBasedJobsViewRouteName is the named route for the projects active jobs + // which does not specify the project name. Repo Relative Dir and Workspace is used. + DirWorkspaceBasedJobsViewRouteName string // LockViewRouteIDQueryParam is the query parameter needed to construct the // lock view: underlying.Get(LockViewRouteName).URL(LockViewRouteIDQueryParam, "my id"). LockViewRouteIDQueryParam string @@ -42,15 +46,30 @@ func (r *Router) GenerateLockURL(lockID string) string { func (r *Router) GenerateProjectJobURL(ctx models.ProjectCommandContext) (string, error) { pull := ctx.Pull - jobURL, err := r.Underlying.Get(r.ProjectJobsViewRouteName).URL( - "org", pull.BaseRepo.Owner, - "repo", pull.BaseRepo.Name, - "pull", fmt.Sprintf("%d", pull.Num), - "project", ctx.ProjectName, - ) - - if err != nil { - return "", errors.Wrapf(err, "creating job url for %s/%d/%s", pull.BaseRepo.FullName, pull.Num, ctx.ProjectName) + var jobURL *url.URL + var err error + if ctx.ProjectName != "" { + jobURL, err = r.Underlying.Get(r.ProjectBasedJobsViewRouteName).URL( + "org", pull.BaseRepo.Owner, + "repo", pull.BaseRepo.Name, + "pull", fmt.Sprintf("%d", pull.Num), + "project", ctx.ProjectName, + ) + if err != nil { + return "", errors.Wrapf(err, "creating job url for %s/%d/%s", pull.BaseRepo.FullName, pull.Num, ctx.ProjectName) + } + } else { + directory := strings.ReplaceAll(ctx.RepoRelDir, "/", "-") + jobURL, err = r.Underlying.Get(r.DirWorkspaceBasedJobsViewRouteName).URL( + "org", pull.BaseRepo.Owner, + "repo", pull.BaseRepo.Name, + "pull", fmt.Sprintf("%d", pull.Num), + "directory", directory, + "workspace", ctx.Workspace, + ) + if err != nil { + return "", errors.Wrapf(err, "creating job url for %s/%d/%s/%s", pull.BaseRepo.FullName, pull.Num, directory, ctx.Workspace) + } } return r.AtlantisURL.String() + jobURL.String(), nil diff --git a/server/router_test.go b/server/router_test.go index 3a79d5640..270fc1833 100644 --- a/server/router_test.go +++ b/server/router_test.go @@ -6,6 +6,7 @@ import ( "github.com/gorilla/mux" "github.com/runatlantis/atlantis/server" + "github.com/runatlantis/atlantis/server/events/models" . "github.com/runatlantis/atlantis/testing" ) @@ -60,3 +61,64 @@ func TestRouter_GenerateLockURL(t *testing.T) { }) } } + +func TestGenerateProjectJobURL_ShouldGenerateURLWithProjectNameWhenProjectNameSpecified(t *testing.T) { + + atlantisURL, err := server.ParseAtlantisURL("http://localhost:4141") + Ok(t, err) + + underlyingRouter := mux.NewRouter() + underlyingRouter.HandleFunc("/jobs/{org}/{repo}/{pull}/{project}", func(_ http.ResponseWriter, _ *http.Request) {}).Methods("GET").Name("project-jobs-detail") + + router := server.Router{ + AtlantisURL: atlantisURL, + Underlying: underlyingRouter, + ProjectBasedJobsViewRouteName: "project-jobs-detail", + } + + ctx := models.ProjectCommandContext{ + Pull: models.PullRequest{ + BaseRepo: models.Repo{ + Owner: "test-owner", + Name: "test-repo", + }, + Num: 1, + }, + ProjectName: "test-project", + } + expectedURL := "http://localhost:4141/jobs/test-owner/test-repo/1/test-project" + gotURL, err := router.GenerateProjectJobURL(ctx) + Ok(t, err) + + Equals(t, expectedURL, gotURL) +} + +func TestGenerateProjectJobURL_ShouldGenerateURLWithDirectoryAndWorkspaceWhenProjectNameNotSpecified(t *testing.T) { + atlantisURL, err := server.ParseAtlantisURL("http://localhost:4141") + Ok(t, err) + + underlyingRouter := mux.NewRouter() + underlyingRouter.HandleFunc("/jobs/{org}/{repo}/{pull}/{directory}/{workspace}", func(_ http.ResponseWriter, _ *http.Request) {}).Methods("GET").Name("directory-workspace-jobs-detail") + + router := server.Router{ + AtlantisURL: atlantisURL, + Underlying: underlyingRouter, + DirWorkspaceBasedJobsViewRouteName: "directory-workspace-jobs-detail", + } + ctx := models.ProjectCommandContext{ + Pull: models.PullRequest{ + BaseRepo: models.Repo{ + Owner: "test-owner", + Name: "test-repo", + }, + Num: 1, + }, + RepoRelDir: "ops/terraform/test-root", + Workspace: "test-workspace", + } + expectedURL := "http://localhost:4141/jobs/test-owner/test-repo/1/ops-terraform-test-root/test-workspace" + gotURL, err := router.GenerateProjectJobURL(ctx) + Ok(t, err) + + Equals(t, expectedURL, gotURL) +} diff --git a/server/server.go b/server/server.go index b8680e1af..c5f81d8d8 100644 --- a/server/server.go +++ b/server/server.go @@ -73,9 +73,12 @@ const ( // route. ex: // mux.Router.Get(LockViewRouteName).URL(LockViewRouteIDQueryParam, "my id") LockViewRouteIDQueryParam = "id" - // ProjectJobsViewRouteName is the named route in mux.Router for the log stream view. - // Can be retrieved by mux.Router.Get(ProjectJobsViewRouteName) - ProjectJobsViewRouteName = "project-jobs-detail" + // ProjectBasedJobsViewRouteName is the named route in mux.Router for projects with a project name in + // the log stream view. Can be retrieved by mux.Router.Get(ProjectBasedJobsViewRouteName) + ProjectBasedJobsViewRouteName = "project-jobs-detail" + // DirWorkspaceBasedJobsViewRouteName is the named route in mux.Router for for projects without a + // project name in the log stream view. The relative directory and the workspace is used instead. + DirWorkspaceBasedJobsViewRouteName = "directory-workspace-jobs-detail" // binDirName is the name of the directory inside our data dir where // we download binaries. BinDirName = "bin" @@ -316,11 +319,12 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { underlyingRouter := mux.NewRouter() router := &Router{ - AtlantisURL: parsedURL, - LockViewRouteIDQueryParam: LockViewRouteIDQueryParam, - LockViewRouteName: LockViewRouteName, - ProjectJobsViewRouteName: ProjectJobsViewRouteName, - Underlying: underlyingRouter, + AtlantisURL: parsedURL, + LockViewRouteIDQueryParam: LockViewRouteIDQueryParam, + LockViewRouteName: LockViewRouteName, + ProjectBasedJobsViewRouteName: ProjectBasedJobsViewRouteName, + DirWorkspaceBasedJobsViewRouteName: DirWorkspaceBasedJobsViewRouteName, + Underlying: underlyingRouter, } projectCmdOutput := make(chan *models.ProjectCmdOutputLine) @@ -830,8 +834,11 @@ func (s *Server) Start() error { s.Router.HandleFunc("/locks", s.LocksController.DeleteLock).Methods("DELETE").Queries("id", "{id:.*}") s.Router.HandleFunc("/lock", s.LocksController.GetLock).Methods("GET"). Queries(LockViewRouteIDQueryParam, fmt.Sprintf("{%s}", LockViewRouteIDQueryParam)).Name(LockViewRouteName) - s.Router.HandleFunc("/jobs/{org}/{repo}/{pull}/{project}", s.JobsController.GetProjectJobs).Methods("GET").Name(ProjectJobsViewRouteName) + s.Router.HandleFunc("/jobs/{org}/{repo}/{pull}/{project}", s.JobsController.GetProjectJobs).Methods("GET").Name(ProjectBasedJobsViewRouteName) s.Router.HandleFunc("/jobs/{org}/{repo}/{pull}/{project}/ws", s.JobsController.GetProjectJobsWS).Methods("GET") + s.Router.HandleFunc("/jobs/{org}/{repo}/{pull}/{directory}/{workspace}", s.JobsController.GetProjectJobs).Methods("GET").Name(DirWorkspaceBasedJobsViewRouteName) + s.Router.HandleFunc("/jobs/{org}/{repo}/{pull}/{directory}/{workspace}/ws", s.JobsController.GetProjectJobsWS).Methods("GET") + n := negroni.New(&negroni.Recovery{ Logger: log.New(os.Stdout, "", log.LstdFlags), PrintStack: false, From c921bb847834e9fa8fdf46944af0145831a71ee7 Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Wed, 20 Oct 2021 14:41:53 -0700 Subject: [PATCH 2/4] Adding URL changes --- server/controllers/logstreaming_controller.go | 9 +++- server/events/models/models.go | 12 +++-- server/events/pull_closed_executor.go | 2 +- server/router.go | 46 ++++++++----------- server/router_test.go | 35 ++++++-------- server/server.go | 26 ++++------- 6 files changed, 60 insertions(+), 70 deletions(-) diff --git a/server/controllers/logstreaming_controller.go b/server/controllers/logstreaming_controller.go index cfe617947..2b955614c 100644 --- a/server/controllers/logstreaming_controller.go +++ b/server/controllers/logstreaming_controller.go @@ -43,11 +43,12 @@ func (p *pullInfo) String() string { type projectInfo struct { projectName string + workspace string pullInfo } func (p *projectInfo) String() string { - return fmt.Sprintf("%s/%s/%d/%s", p.org, p.repo, p.pull, p.projectName) + return fmt.Sprintf("%s/%s/%d/%s/%s", p.org, p.repo, p.pull, p.projectName, p.workspace) } func newPullInfo(r *http.Request) (*pullInfo, error) { @@ -87,9 +88,15 @@ func newProjectInfo(r *http.Request) (*projectInfo, error) { return nil, fmt.Errorf("Internal error: no project in route") } + workspace, ok := mux.Vars(r)["workspace"] + if !ok { + return nil, fmt.Errorf("Internal error: no workspace in route") + } + return &projectInfo{ pullInfo: *pullInfo, projectName: project, + workspace: workspace, }, nil } diff --git a/server/events/models/models.go b/server/events/models/models.go index 9dd894b54..41c1c6e3f 100644 --- a/server/events/models/models.go +++ b/server/events/models/models.go @@ -456,11 +456,17 @@ func (p ProjectCommandContext) GetShowResultFileName() string { // Gets a unique identifier for the current pull request as a single string func (p ProjectCommandContext) PullInfo() string { - return BuildPullInfo(p.BaseRepo.FullName, p.Pull.Num, p.ProjectName) + return BuildPullInfo(p.BaseRepo.FullName, p.Pull.Num, p.ProjectName, p.RepoRelDir, p.Workspace) } -func BuildPullInfo(repoName string, pullNum int, projectName string) string { - return fmt.Sprintf("%s/%d/%s", repoName, pullNum, projectName) +func BuildPullInfo(repoName string, pullNum int, projectName string, relDir string, workspace string) string { + var projectIdentifier string + if projectName == "" { + projectIdentifier = strings.ReplaceAll(relDir, "/", "-") + } else { + projectIdentifier = projectName + } + return fmt.Sprintf("%s/%d/%s/%s", repoName, pullNum, projectIdentifier, workspace) } // SplitRepoFullName splits a repo full name up into its owner and repo diff --git a/server/events/pull_closed_executor.go b/server/events/pull_closed_executor.go index 3e21f730d..e923faf51 100644 --- a/server/events/pull_closed_executor.go +++ b/server/events/pull_closed_executor.go @@ -88,7 +88,7 @@ func (p *PullClosedExecutor) CleanUpPull(repo models.Repo, pull models.PullReque // with same dir and workspace. If a project name has not been set, we'll use the dir and // workspace to build project key. // Source: https://www.runatlantis.io/docs/repo-level-atlantis-yaml.html#reference - projectKey := models.BuildPullInfo(pullStatus.Pull.BaseRepo.FullName, pull.Num, project.ProjectName) + projectKey := models.BuildPullInfo(pullStatus.Pull.BaseRepo.FullName, pull.Num, project.ProjectName, project.RepoRelDir, project.Workspace) p.LogStreamResourceCleaner.CleanUp(projectKey) } } diff --git a/server/router.go b/server/router.go index bab6d5ea2..83d1d50ca 100644 --- a/server/router.go +++ b/server/router.go @@ -19,11 +19,8 @@ type Router struct { // LockViewRouteName is the named route for the lock view that can be Get'd // from the Underlying router. LockViewRouteName string - // ProjectBasedJobsViewRouteName is the named route for the projects active jobs - ProjectBasedJobsViewRouteName string - // DirWorkspaceBasedJobsViewRouteName is the named route for the projects active jobs - // which does not specify the project name. Repo Relative Dir and Workspace is used. - DirWorkspaceBasedJobsViewRouteName string + // ProjectJobsViewRouteName is the named route for the projects active jobs + ProjectJobsViewRouteName string // LockViewRouteIDQueryParam is the query parameter needed to construct the // lock view: underlying.Get(LockViewRouteName).URL(LockViewRouteIDQueryParam, "my id"). LockViewRouteIDQueryParam string @@ -46,30 +43,23 @@ func (r *Router) GenerateLockURL(lockID string) string { func (r *Router) GenerateProjectJobURL(ctx models.ProjectCommandContext) (string, error) { pull := ctx.Pull - var jobURL *url.URL - var err error - if ctx.ProjectName != "" { - jobURL, err = r.Underlying.Get(r.ProjectBasedJobsViewRouteName).URL( - "org", pull.BaseRepo.Owner, - "repo", pull.BaseRepo.Name, - "pull", fmt.Sprintf("%d", pull.Num), - "project", ctx.ProjectName, - ) - if err != nil { - return "", errors.Wrapf(err, "creating job url for %s/%d/%s", pull.BaseRepo.FullName, pull.Num, ctx.ProjectName) - } + // Use relative path to create project identifier if project name not set. + var projectIdentifier string + if ctx.ProjectName == "" { + projectIdentifier = strings.ReplaceAll(ctx.RepoRelDir, "/", "-") } else { - directory := strings.ReplaceAll(ctx.RepoRelDir, "/", "-") - jobURL, err = r.Underlying.Get(r.DirWorkspaceBasedJobsViewRouteName).URL( - "org", pull.BaseRepo.Owner, - "repo", pull.BaseRepo.Name, - "pull", fmt.Sprintf("%d", pull.Num), - "directory", directory, - "workspace", ctx.Workspace, - ) - if err != nil { - return "", errors.Wrapf(err, "creating job url for %s/%d/%s/%s", pull.BaseRepo.FullName, pull.Num, directory, ctx.Workspace) - } + projectIdentifier = ctx.ProjectName + } + + jobURL, err := r.Underlying.Get(r.ProjectJobsViewRouteName).URL( + "org", pull.BaseRepo.Owner, + "repo", pull.BaseRepo.Name, + "pull", fmt.Sprintf("%d", pull.Num), + "project", projectIdentifier, + "workspace", ctx.Workspace, + ) + if err != nil { + return "", errors.Wrapf(err, "creating job url for %s/%d/%s/%s", pull.BaseRepo.FullName, pull.Num, projectIdentifier, ctx.Workspace) } return r.AtlantisURL.String() + jobURL.String(), nil diff --git a/server/router_test.go b/server/router_test.go index 270fc1833..ccabee44d 100644 --- a/server/router_test.go +++ b/server/router_test.go @@ -62,20 +62,22 @@ func TestRouter_GenerateLockURL(t *testing.T) { } } -func TestGenerateProjectJobURL_ShouldGenerateURLWithProjectNameWhenProjectNameSpecified(t *testing.T) { - +func setupJobsRouter(t *testing.T) *server.Router { atlantisURL, err := server.ParseAtlantisURL("http://localhost:4141") Ok(t, err) underlyingRouter := mux.NewRouter() - underlyingRouter.HandleFunc("/jobs/{org}/{repo}/{pull}/{project}", func(_ http.ResponseWriter, _ *http.Request) {}).Methods("GET").Name("project-jobs-detail") + underlyingRouter.HandleFunc("/jobs/{org}/{repo}/{pull}/{project}/{workspace}", func(_ http.ResponseWriter, _ *http.Request) {}).Methods("GET").Name("project-jobs-detail") - router := server.Router{ - AtlantisURL: atlantisURL, - Underlying: underlyingRouter, - ProjectBasedJobsViewRouteName: "project-jobs-detail", + return &server.Router{ + AtlantisURL: atlantisURL, + Underlying: underlyingRouter, + ProjectJobsViewRouteName: "project-jobs-detail", } +} +func TestGenerateProjectJobURL_ShouldGenerateURLWithProjectNameWhenProjectNameSpecified(t *testing.T) { + router := setupJobsRouter(t) ctx := models.ProjectCommandContext{ Pull: models.PullRequest{ BaseRepo: models.Repo{ @@ -85,8 +87,9 @@ func TestGenerateProjectJobURL_ShouldGenerateURLWithProjectNameWhenProjectNameSp Num: 1, }, ProjectName: "test-project", + Workspace: "default", } - expectedURL := "http://localhost:4141/jobs/test-owner/test-repo/1/test-project" + expectedURL := "http://localhost:4141/jobs/test-owner/test-repo/1/test-project/default" gotURL, err := router.GenerateProjectJobURL(ctx) Ok(t, err) @@ -94,17 +97,7 @@ func TestGenerateProjectJobURL_ShouldGenerateURLWithProjectNameWhenProjectNameSp } func TestGenerateProjectJobURL_ShouldGenerateURLWithDirectoryAndWorkspaceWhenProjectNameNotSpecified(t *testing.T) { - atlantisURL, err := server.ParseAtlantisURL("http://localhost:4141") - Ok(t, err) - - underlyingRouter := mux.NewRouter() - underlyingRouter.HandleFunc("/jobs/{org}/{repo}/{pull}/{directory}/{workspace}", func(_ http.ResponseWriter, _ *http.Request) {}).Methods("GET").Name("directory-workspace-jobs-detail") - - router := server.Router{ - AtlantisURL: atlantisURL, - Underlying: underlyingRouter, - DirWorkspaceBasedJobsViewRouteName: "directory-workspace-jobs-detail", - } + router := setupJobsRouter(t) ctx := models.ProjectCommandContext{ Pull: models.PullRequest{ BaseRepo: models.Repo{ @@ -114,9 +107,9 @@ func TestGenerateProjectJobURL_ShouldGenerateURLWithDirectoryAndWorkspaceWhenPro Num: 1, }, RepoRelDir: "ops/terraform/test-root", - Workspace: "test-workspace", + Workspace: "default", } - expectedURL := "http://localhost:4141/jobs/test-owner/test-repo/1/ops-terraform-test-root/test-workspace" + expectedURL := "http://localhost:4141/jobs/test-owner/test-repo/1/ops-terraform-test-root/default" gotURL, err := router.GenerateProjectJobURL(ctx) Ok(t, err) diff --git a/server/server.go b/server/server.go index c5f81d8d8..227f88a4c 100644 --- a/server/server.go +++ b/server/server.go @@ -73,12 +73,9 @@ const ( // route. ex: // mux.Router.Get(LockViewRouteName).URL(LockViewRouteIDQueryParam, "my id") LockViewRouteIDQueryParam = "id" - // ProjectBasedJobsViewRouteName is the named route in mux.Router for projects with a project name in - // the log stream view. Can be retrieved by mux.Router.Get(ProjectBasedJobsViewRouteName) - ProjectBasedJobsViewRouteName = "project-jobs-detail" - // DirWorkspaceBasedJobsViewRouteName is the named route in mux.Router for for projects without a - // project name in the log stream view. The relative directory and the workspace is used instead. - DirWorkspaceBasedJobsViewRouteName = "directory-workspace-jobs-detail" + // ProjectJobsViewRouteName is the named route in mux.Router for projects with a project name in + // the log stream view. Can be retrieved by mux.Router.Get(ProjectJobsViewRouteName) + ProjectJobsViewRouteName = "project-jobs-detail" // binDirName is the name of the directory inside our data dir where // we download binaries. BinDirName = "bin" @@ -319,12 +316,11 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { underlyingRouter := mux.NewRouter() router := &Router{ - AtlantisURL: parsedURL, - LockViewRouteIDQueryParam: LockViewRouteIDQueryParam, - LockViewRouteName: LockViewRouteName, - ProjectBasedJobsViewRouteName: ProjectBasedJobsViewRouteName, - DirWorkspaceBasedJobsViewRouteName: DirWorkspaceBasedJobsViewRouteName, - Underlying: underlyingRouter, + AtlantisURL: parsedURL, + LockViewRouteIDQueryParam: LockViewRouteIDQueryParam, + LockViewRouteName: LockViewRouteName, + ProjectJobsViewRouteName: ProjectJobsViewRouteName, + Underlying: underlyingRouter, } projectCmdOutput := make(chan *models.ProjectCmdOutputLine) @@ -834,10 +830,8 @@ func (s *Server) Start() error { s.Router.HandleFunc("/locks", s.LocksController.DeleteLock).Methods("DELETE").Queries("id", "{id:.*}") s.Router.HandleFunc("/lock", s.LocksController.GetLock).Methods("GET"). Queries(LockViewRouteIDQueryParam, fmt.Sprintf("{%s}", LockViewRouteIDQueryParam)).Name(LockViewRouteName) - s.Router.HandleFunc("/jobs/{org}/{repo}/{pull}/{project}", s.JobsController.GetProjectJobs).Methods("GET").Name(ProjectBasedJobsViewRouteName) - s.Router.HandleFunc("/jobs/{org}/{repo}/{pull}/{project}/ws", s.JobsController.GetProjectJobsWS).Methods("GET") - s.Router.HandleFunc("/jobs/{org}/{repo}/{pull}/{directory}/{workspace}", s.JobsController.GetProjectJobs).Methods("GET").Name(DirWorkspaceBasedJobsViewRouteName) - s.Router.HandleFunc("/jobs/{org}/{repo}/{pull}/{directory}/{workspace}/ws", s.JobsController.GetProjectJobsWS).Methods("GET") + s.Router.HandleFunc("/jobs/{org}/{repo}/{pull}/{project}/{workspace}", s.JobsController.GetProjectJobs).Methods("GET").Name(ProjectJobsViewRouteName) + s.Router.HandleFunc("/jobs/{org}/{repo}/{pull}/{project}/{workspace}/ws", s.JobsController.GetProjectJobsWS).Methods("GET") n := negroni.New(&negroni.Recovery{ Logger: log.New(os.Stdout, "", log.LstdFlags), From dc8539212879c6b5dfea41d326bef07eb78c66d6 Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Wed, 20 Oct 2021 14:54:13 -0700 Subject: [PATCH 3/4] Fixing tests --- server/events/pull_closed_executor_test.go | 1 + server/server.go | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/events/pull_closed_executor_test.go b/server/events/pull_closed_executor_test.go index 266557455..d4bd9d5c6 100644 --- a/server/events/pull_closed_executor_test.go +++ b/server/events/pull_closed_executor_test.go @@ -206,6 +206,7 @@ func TestCleanUpLogStreaming(t *testing.T) { BaseRepo: fixtures.GithubRepo, Pull: fixtures.Pull, ProjectName: *fixtures.Project.Name, + Workspace: "default", } go prjCmdOutHandler.Handle() diff --git a/server/server.go b/server/server.go index 227f88a4c..b6ac14df2 100644 --- a/server/server.go +++ b/server/server.go @@ -73,8 +73,7 @@ const ( // route. ex: // mux.Router.Get(LockViewRouteName).URL(LockViewRouteIDQueryParam, "my id") LockViewRouteIDQueryParam = "id" - // ProjectJobsViewRouteName is the named route in mux.Router for projects with a project name in - // the log stream view. Can be retrieved by mux.Router.Get(ProjectJobsViewRouteName) + // ProjectJobsViewRouteName is the named route in mux.Router for the log stream view. ProjectJobsViewRouteName = "project-jobs-detail" // binDirName is the name of the directory inside our data dir where // we download binaries. From b39ffe665dfc1eb9f0ffbc4fef8f59621eba80b5 Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Thu, 21 Oct 2021 11:36:33 -0700 Subject: [PATCH 4/4] Refactoring --- server/events/models/models.go | 14 ++++++++------ server/router.go | 11 +---------- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/server/events/models/models.go b/server/events/models/models.go index 41c1c6e3f..d96824a34 100644 --- a/server/events/models/models.go +++ b/server/events/models/models.go @@ -460,15 +460,17 @@ func (p ProjectCommandContext) PullInfo() string { } func BuildPullInfo(repoName string, pullNum int, projectName string, relDir string, workspace string) string { - var projectIdentifier string - if projectName == "" { - projectIdentifier = strings.ReplaceAll(relDir, "/", "-") - } else { - projectIdentifier = projectName - } + projectIdentifier := GetProjectIdentifier(relDir, projectName) return fmt.Sprintf("%s/%d/%s/%s", repoName, pullNum, projectIdentifier, workspace) } +func GetProjectIdentifier(relRepoDir string, projectName string) string { + if projectName != "" { + return projectName + } + return strings.ReplaceAll(relRepoDir, "/", "-") +} + // SplitRepoFullName splits a repo full name up into its owner and repo // name segments. If the repoFullName is malformed, may return empty // strings for owner or repo. diff --git a/server/router.go b/server/router.go index 83d1d50ca..560729470 100644 --- a/server/router.go +++ b/server/router.go @@ -3,7 +3,6 @@ package server import ( "fmt" "net/url" - "strings" "github.com/gorilla/mux" "github.com/pkg/errors" @@ -42,15 +41,7 @@ func (r *Router) GenerateLockURL(lockID string) string { func (r *Router) GenerateProjectJobURL(ctx models.ProjectCommandContext) (string, error) { pull := ctx.Pull - - // Use relative path to create project identifier if project name not set. - var projectIdentifier string - if ctx.ProjectName == "" { - projectIdentifier = strings.ReplaceAll(ctx.RepoRelDir, "/", "-") - } else { - projectIdentifier = ctx.ProjectName - } - + projectIdentifier := models.GetProjectIdentifier(ctx.RepoRelDir, ctx.ProjectName) jobURL, err := r.Underlying.Get(r.ProjectJobsViewRouteName).URL( "org", pull.BaseRepo.Owner, "repo", pull.BaseRepo.Name,