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.

Sign up for GitHub

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

Promote pull request properties #5169

Merged
merged 3 commits into from
Dec 10, 2024
Merged

Promote pull request properties #5169

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ff1c8b3
Promote pull request properties
JAORMX Dec 10, 2024
96f89c4
Fix target/source confusion
JAORMX Dec 10, 2024
0148334
Enhance logs.
JAORMX Dec 10, 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
16 changes: 16 additions & 0 deletions internal/entities/properties/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,22 @@ const (
RepoPropertyIsFork = "is_fork"
)

// Pull Request property keys
const (
// PullRequestCommitSHA represents the commit SHA of the pull request
PullRequestCommitSHA = "commit_sha"
// PullRequestBaseCloneURL represents the clone URL of the base repository
PullRequestBaseCloneURL = "base_clone_url"
// PullRequestBaseDefaultBranch represents the default branch of the base repository
PullRequestBaseDefaultBranch = "base_default_branch"
// PullRequestTargetCloneURL represents the clone URL of the target repository
PullRequestTargetCloneURL = "target_clone_url"
// PullRequestTargetBranch represents the default branch of the target repository
PullRequestTargetBranch = "target_branch"
// PullRequestUpstreamURL represents the URL of the pull request in the provider
PullRequestUpstreamURL = "upstream_url"
)

// Artifact property keys
const (
// ArtifactPropertyType represents the type of the artifact (e.g 'container')
Expand Down
16 changes: 14 additions & 2 deletions internal/providers/github/properties/pull_request.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@ var prPropertyDefinitions = []propertyOrigin{
// general entity
properties.PropertyName,
properties.PropertyUpstreamID,
properties.PullRequestCommitSHA,
properties.PullRequestBaseCloneURL,
properties.PullRequestBaseDefaultBranch,
properties.PullRequestTargetCloneURL,
properties.PullRequestTargetBranch,
properties.PullRequestUpstreamURL,
// github-specific
PullPropertyURL,
PullPropertyNumber,
Expand Down Expand Up @@ -150,8 +156,14 @@ func getPrWrapper(

prProps := map[string]any{
// general entity
properties.PropertyUpstreamID: properties.NumericalValueToUpstreamID(prReply.GetID()),
properties.PropertyName: fmt.Sprintf("%s/%s/%d", owner, name, intId),
properties.PropertyUpstreamID: properties.NumericalValueToUpstreamID(prReply.GetID()),
properties.PropertyName: fmt.Sprintf("%s/%s/%d", owner, name, intId),
properties.PullRequestCommitSHA: prReply.GetHead().GetSHA(),
properties.PullRequestBaseCloneURL: prReply.GetBase().GetRepo().GetCloneURL(),
properties.PullRequestBaseDefaultBranch: prReply.GetBase().GetRepo().GetDefaultBranch(),
properties.PullRequestTargetCloneURL: prReply.GetHead().GetRepo().GetCloneURL(),
properties.PullRequestTargetBranch: prReply.GetHead().GetRef(),
properties.PullRequestUpstreamURL: prReply.GetHTMLURL(),
// github-specific
PullPropertyURL: prReply.GetHTMLURL(),
// our proto representation uses int64 for the number but GH uses int
Expand Down
8 changes: 0 additions & 8 deletions internal/providers/gitlab/properties.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,8 @@ const (
PullRequestProjectID = "gitlab/project_id"
// PullRequestNumber represents the gitlab merge request number
PullRequestNumber = "gitlab/merge_request_number"
// PullRequestSourceBranch represents the gitlab source branch
PullRequestSourceBranch = "gitlab/source_branch"
// PullRequestTargetBranch represents the gitlab target branch
PullRequestTargetBranch = "gitlab/target_branch"
// PullRequestAuthor represents the gitlab author
PullRequestAuthor = "gitlab/author"
// PullRequestCommitSHA represents the gitlab commit SHA
PullRequestCommitSHA = "gitlab/commit_sha"
// PullRequestURL represents the gitlab merge request URL
PullRequestURL = "gitlab/merge_request_url"
)

// Release Properties
Expand Down
65 changes: 43 additions & 22 deletions internal/providers/gitlab/pull_request_properties.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ func FormatPullRequestUpstreamID(id int) string {
return fmt.Sprintf("%d", id)
}

//nolint:gocyclo // TODO: Refactor to reduce complexity
func (c *gitlabClient) getPropertiesForPullRequest(
ctx context.Context, getByProps *properties.Properties,
) (*properties.Properties, error) {
Expand Down Expand Up @@ -87,15 +88,24 @@ func (c *gitlabClient) getPropertiesForPullRequest(
return nil, fmt.Errorf("failed to get project: %w", err)
}

outProps, err := gitlabMergeRequestToProperties(mr, proj)
sourceproj := proj
if mr.SourceProjectID != 0 && mr.SourceProjectID != proj.ID {
sourceproj, err = c.getGitLabProject(ctx, FormatRepositoryUpstreamID(mr.SourceProjectID))
if err != nil {
return nil, fmt.Errorf("failed to get target project: %w", err)
}
}

outProps, err := gitlabMergeRequestToProperties(mr, proj, sourceproj)
if err != nil {
return nil, fmt.Errorf("failed to convert merge request to properties: %w", err)
}

return outProps, nil
}

func gitlabMergeRequestToProperties(mr *gitlab.MergeRequest, proj *gitlab.Project) (*properties.Properties, error) {
func gitlabMergeRequestToProperties(
mr *gitlab.MergeRequest, proj *gitlab.Project, sourceproj *gitlab.Project) (*properties.Properties, error) {
ns, err := getGitlabProjectNamespace(proj)
if err != nil {
return nil, fmt.Errorf("failed to get namespace: %w", err)
Expand All @@ -105,18 +115,20 @@ func gitlabMergeRequestToProperties(mr *gitlab.MergeRequest, proj *gitlab.Projec

outProps, err := properties.NewProperties(map[string]any{
// Unique upstream ID for the merge request
properties.PropertyUpstreamID: FormatPullRequestUpstreamID(mr.ID),
properties.PropertyName: formatPullRequestName(ns, projName, FormatPullRequestUpstreamID(mr.IID)),
RepoPropertyNamespace: ns,
RepoPropertyProjectName: projName,
properties.PropertyUpstreamID: FormatPullRequestUpstreamID(mr.ID),
properties.PropertyName: formatPullRequestName(ns, projName, FormatPullRequestUpstreamID(mr.IID)),
properties.PullRequestCommitSHA: mr.SHA,
properties.PullRequestBaseCloneURL: sourceproj.HTTPURLToRepo,
properties.PullRequestBaseDefaultBranch: mr.SourceBranch,
properties.PullRequestTargetCloneURL: proj.HTTPURLToRepo,
properties.PullRequestTargetBranch: mr.TargetBranch,
properties.PullRequestUpstreamURL: mr.WebURL,
RepoPropertyNamespace: ns,
RepoPropertyProjectName: projName,
// internal ID of the merge request
PullRequestNumber: FormatPullRequestUpstreamID(mr.IID),
PullRequestProjectID: FormatRepositoryUpstreamID(proj.ID),
PullRequestSourceBranch: mr.SourceBranch,
PullRequestTargetBranch: mr.TargetBranch,
PullRequestCommitSHA: mr.SHA,
PullRequestAuthor: int64(mr.Author.ID),
PullRequestURL: mr.WebURL,
PullRequestNumber: FormatPullRequestUpstreamID(mr.IID),
PullRequestProjectID: FormatRepositoryUpstreamID(proj.ID),
PullRequestAuthor: int64(mr.Author.ID),
})
if err != nil {
return nil, fmt.Errorf("failed to create properties: %w", err)
Expand Down Expand Up @@ -146,12 +158,12 @@ func pullRequestV1FromProperties(prProps *properties.Properties) (*pbinternal.Pu
return nil, fmt.Errorf("failed to get project name: %w", err)
}

commitSha, err := getStringProp(prProps, PullRequestCommitSHA)
commitSha, err := getStringProp(prProps, properties.PullRequestCommitSHA)
if err != nil {
return nil, fmt.Errorf("failed to get commit SHA: %w", err)
}

mrURL, err := getStringProp(prProps, PullRequestURL)
mrURL, err := getStringProp(prProps, properties.PullRequestUpstreamURL)
if err != nil {
return nil, fmt.Errorf("failed to get merge request URL: %w", err)
}
Expand All @@ -161,20 +173,29 @@ func pullRequestV1FromProperties(prProps *properties.Properties) (*pbinternal.Pu
return nil, fmt.Errorf("failed to get author ID: %w", err)
}

basecloneurl := prProps.GetProperty(properties.PullRequestBaseCloneURL).GetString()
targetcloneurl := prProps.GetProperty(properties.PullRequestTargetCloneURL).GetString()
basebranch := prProps.GetProperty(properties.PullRequestBaseDefaultBranch).GetString()
targetbranch := prProps.GetProperty(properties.PullRequestTargetBranch).GetString()

// parse UpstreamID to int64
id, err := strconv.ParseInt(iid, 10, 64)
if err != nil {
return nil, fmt.Errorf("failed to parse upstream ID: %w", err)
}

pbPR := &pbinternal.PullRequest{
Number: id,
RepoOwner: ns,
RepoName: projName,
CommitSha: commitSha,
AuthorId: authorID,
Url: mrURL,
Properties: prProps.ToProtoStruct(),
Number: id,
RepoOwner: ns,
RepoName: projName,
CommitSha: commitSha,
AuthorId: authorID,
Url: mrURL,
BaseCloneUrl: basecloneurl,
TargetCloneUrl: targetcloneurl,
BaseRef: basebranch,
TargetRef: targetbranch,
Properties: prProps.ToProtoStruct(),
}

return pbPR, nil
Expand Down
Loading