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

fix ignore edited comments bug #459

Merged
merged 2 commits into from
Aug 17, 2022
Merged

fix ignore edited comments bug #459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1e6177d
fix ignore edited comments bug
devinburnette Aug 17, 2022
7a970fb
also change the candidate abstraction to match
devinburnette Aug 17, 2022
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
2 changes: 1 addition & 1 deletion policy/approval/approve.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ func (r *Rule) filterEditedCandidates(ctx context.Context, prctx pull.Context, c
var allowedCandidates []*common.Candidate
for _, candidate := range candidates {
if r.Options.IgnoreEditedComments {
if candidate.UpdatedAt == candidate.CreatedAt {
if candidate.LastEditedAt.IsZero() {
allowedCandidates = append(allowedCandidates, candidate)
}
}
Expand Down
86 changes: 43 additions & 43 deletions policy/approval/approve_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,69 +39,69 @@ func TestIsApproved(t *testing.T) {
AuthorValue: "mhaypenny",
CommentsValue: []*pull.Comment{
{
CreatedAt: now.Add(10 * time.Second),
UpdatedAt: now.Add(10 * time.Second),
Author: "other-user",
Body: "Why did you do this?",
CreatedAt: now.Add(10 * time.Second),
LastEditedAt: time.Time{},
Author: "other-user",
Body: "Why did you do this?",
},
{
CreatedAt: now.Add(20 * time.Second),
UpdatedAt: now.Add(20 * time.Second),
Author: "comment-approver",
Body: "LGTM :+1: :shipit:",
CreatedAt: now.Add(20 * time.Second),
LastEditedAt: time.Time{},
Author: "comment-approver",
Body: "LGTM :+1: :shipit:",
},
{
CreatedAt: now.Add(30 * time.Second),
UpdatedAt: now.Add(30 * time.Second),
Author: "disapprover",
Body: "I don't like things! :-1:",
CreatedAt: now.Add(30 * time.Second),
LastEditedAt: time.Time{},
Author: "disapprover",
Body: "I don't like things! :-1:",
},
{
CreatedAt: now.Add(40 * time.Second),
UpdatedAt: now.Add(40 * time.Second),
Author: "mhaypenny",
Body: ":+1: my stuff is cool",
CreatedAt: now.Add(40 * time.Second),
LastEditedAt: time.Time{},
Author: "mhaypenny",
Body: ":+1: my stuff is cool",
},
{
CreatedAt: now.Add(50 * time.Second),
UpdatedAt: now.Add(50 * time.Second),
Author: "contributor-author",
Body: ":+1: I added to this PR",
CreatedAt: now.Add(50 * time.Second),
LastEditedAt: time.Time{},
Author: "contributor-author",
Body: ":+1: I added to this PR",
},
{
CreatedAt: now.Add(60 * time.Second),
UpdatedAt: now.Add(60 * time.Second),
Author: "contributor-committer",
Body: ":+1: I also added to this PR",
CreatedAt: now.Add(60 * time.Second),
LastEditedAt: time.Time{},
Author: "contributor-committer",
Body: ":+1: I also added to this PR",
},
{
CreatedAt: now.Add(70 * time.Second),
UpdatedAt: now.Add(71 * time.Second),
Author: "comment-editor",
Body: "LGTM :+1: :shipit:",
CreatedAt: now.Add(70 * time.Second),
LastEditedAt: now.Add(71 * time.Second),
Author: "comment-editor",
Body: "LGTM :+1: :shipit:",
},
},
ReviewsValue: []*pull.Review{
{
CreatedAt: now.Add(70 * time.Second),
UpdatedAt: now.Add(70 * time.Second),
Author: "disapprover",
State: pull.ReviewChangesRequested,
Body: "I _really_ don't like things!",
CreatedAt: now.Add(70 * time.Second),
LastEditedAt: time.Time{},
Author: "disapprover",
State: pull.ReviewChangesRequested,
Body: "I _really_ don't like things!",
},
{
CreatedAt: now.Add(80 * time.Second),
UpdatedAt: now.Add(80 * time.Second),
Author: "review-approver",
State: pull.ReviewApproved,
Body: "I LIKE THIS",
CreatedAt: now.Add(80 * time.Second),
LastEditedAt: time.Time{},
Author: "review-approver",
State: pull.ReviewApproved,
Body: "I LIKE THIS",
},
{
CreatedAt: now.Add(90 * time.Second),
UpdatedAt: now.Add(91 * time.Second),
Author: "review-comment-editor",
State: pull.ReviewApproved,
Body: "I LIKE THIS",
CreatedAt: now.Add(90 * time.Second),
LastEditedAt: now.Add(91 * time.Second),
Author: "review-comment-editor",
State: pull.ReviewApproved,
Body: "I LIKE THIS",
},
},
CommitsValue: []*pull.Commit{
Expand Down
24 changes: 12 additions & 12 deletions policy/common/methods.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ type Methods struct {
}

type Candidate struct {
User string
CreatedAt time.Time
UpdatedAt time.Time
User string
CreatedAt time.Time
LastEditedAt time.Time
}

type CandidatesByCreationTime []*Candidate
Expand All @@ -64,9 +64,9 @@ func (m *Methods) Candidates(ctx context.Context, prctx pull.Context) ([]*Candid
for _, c := range comments {
if m.CommentMatches(c.Body) {
candidates = append(candidates, &Candidate{
User: c.Author,
CreatedAt: c.CreatedAt,
UpdatedAt: c.UpdatedAt,
User: c.Author,
CreatedAt: c.CreatedAt,
LastEditedAt: c.LastEditedAt,
})
}
}
Expand All @@ -83,16 +83,16 @@ func (m *Methods) Candidates(ctx context.Context, prctx pull.Context) ([]*Candid
if len(m.GithubReviewCommentPatterns) > 0 {
if m.githubReviewCommentMatches(r.Body) {
candidates = append(candidates, &Candidate{
User: r.Author,
CreatedAt: r.CreatedAt,
UpdatedAt: r.UpdatedAt,
User: r.Author,
CreatedAt: r.CreatedAt,
LastEditedAt: r.LastEditedAt,
})
}
} else {
candidates = append(candidates, &Candidate{
User: r.Author,
CreatedAt: r.CreatedAt,
UpdatedAt: r.UpdatedAt,
User: r.Author,
CreatedAt: r.CreatedAt,
LastEditedAt: r.LastEditedAt,
})
}
}
Expand Down
20 changes: 10 additions & 10 deletions pull/context.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,10 @@ type Signature struct {
}

type Comment struct {
CreatedAt time.Time
UpdatedAt time.Time
Author string
Body string
CreatedAt time.Time
LastEditedAt time.Time
Author string
Body string
}

type ReviewState string
Expand All @@ -198,12 +198,12 @@ const (
)

type Review struct {
CreatedAt time.Time
UpdatedAt time.Time
Author string
State ReviewState
Body string
SHA string
CreatedAt time.Time
LastEditedAt time.Time
Author string
State ReviewState
Body string
SHA string

Teams []string
}
Expand Down
48 changes: 24 additions & 24 deletions pull/github.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -978,11 +978,11 @@ func (pi v4PageInfo) UpdateCursor(vars map[string]interface{}, name string) bool
}

type v4PullRequestReview struct {
Author v4Actor
State string
Body string
SubmittedAt time.Time
UpdatedAt time.Time
Author v4Actor
State string
Body string
SubmittedAt time.Time
LastEditedAt time.Time

Commit struct {
OID string
Expand All @@ -1004,38 +1004,38 @@ func (r *v4PullRequestReview) ToReview() *Review {
}

return &Review{
CreatedAt: r.SubmittedAt,
UpdatedAt: r.UpdatedAt,
Author: r.Author.GetV3Login(),
State: ReviewState(strings.ToLower(r.State)),
Body: r.Body,
SHA: r.Commit.OID,
Teams: teams,
CreatedAt: r.SubmittedAt,
LastEditedAt: r.LastEditedAt,
Author: r.Author.GetV3Login(),
State: ReviewState(strings.ToLower(r.State)),
Body: r.Body,
SHA: r.Commit.OID,
Teams: teams,
}
}

func (r *v4PullRequestReview) ToComment() *Comment {
return &Comment{
CreatedAt: r.SubmittedAt,
UpdatedAt: r.UpdatedAt,
Author: r.Author.GetV3Login(),
Body: r.Body,
CreatedAt: r.SubmittedAt,
LastEditedAt: r.LastEditedAt,
Author: r.Author.GetV3Login(),
Body: r.Body,
}
}

type v4IssueComment struct {
Author v4Actor
Body string
CreatedAt time.Time
UpdatedAt time.Time
Author v4Actor
Body string
CreatedAt time.Time
LastEditedAt time.Time
}

func (c *v4IssueComment) ToComment() *Comment {
return &Comment{
CreatedAt: c.CreatedAt,
UpdatedAt: c.UpdatedAt,
Author: c.Author.GetV3Login(),
Body: c.Body,
CreatedAt: c.CreatedAt,
LastEditedAt: c.LastEditedAt,
Author: c.Author.GetV3Login(),
Body: c.Body,
}
}

Expand Down
12 changes: 6 additions & 6 deletions pull/github_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,19 +201,19 @@ func TestReviews(t *testing.T) {

assert.Equal(t, "mhaypenny", reviews[0].Author)
assert.Equal(t, expectedTime, reviews[0].CreatedAt)
assert.Equal(t, expectedTime, reviews[0].UpdatedAt)
assert.Equal(t, expectedTime, reviews[0].LastEditedAt)
assert.Equal(t, ReviewChangesRequested, reviews[0].State)
assert.Equal(t, "", reviews[0].Body)

assert.Equal(t, "bkeyes", reviews[1].Author)
assert.Equal(t, expectedTime.Add(time.Second), reviews[1].CreatedAt)
assert.Equal(t, expectedTime.Add(time.Second), reviews[1].UpdatedAt)
assert.Equal(t, expectedTime.Add(time.Second), reviews[1].LastEditedAt)
assert.Equal(t, ReviewApproved, reviews[1].State)
assert.Equal(t, "the body", reviews[1].Body)

assert.Equal(t, "jgiannuzzi", reviews[2].Author)
assert.Equal(t, expectedTime.Add(-4*time.Second).Add(5*time.Minute), reviews[2].CreatedAt)
assert.Equal(t, expectedTime.Add(-4*time.Second).Add(5*time.Minute), reviews[2].UpdatedAt)
assert.Equal(t, expectedTime.Add(-4*time.Second).Add(5*time.Minute), reviews[2].LastEditedAt)
assert.Equal(t, ReviewCommented, reviews[2].State)
assert.Equal(t, "A review comment", reviews[2].Body)

Expand Down Expand Up @@ -266,17 +266,17 @@ func TestComments(t *testing.T) {

assert.Equal(t, "bkeyes", comments[0].Author)
assert.Equal(t, expectedTime, comments[0].CreatedAt)
assert.Equal(t, expectedTime, comments[0].UpdatedAt)
assert.Equal(t, expectedTime, comments[0].LastEditedAt)
assert.Equal(t, ":+1:", comments[0].Body)

assert.Equal(t, "bulldozer[bot]", comments[1].Author)
assert.Equal(t, expectedTime.Add(time.Minute), comments[1].CreatedAt)
assert.Equal(t, expectedTime.Add(time.Minute), comments[1].UpdatedAt)
assert.Equal(t, expectedTime.Add(time.Minute), comments[1].LastEditedAt)
assert.Equal(t, "I merge!", comments[1].Body)

assert.Equal(t, "jgiannuzzi", comments[2].Author)
assert.Equal(t, expectedTime.Add(10*time.Minute), comments[2].CreatedAt)
assert.Equal(t, expectedTime.Add(10*time.Minute), comments[2].UpdatedAt)
assert.Equal(t, expectedTime.Add(10*time.Minute), comments[2].LastEditedAt)
assert.Equal(t, "A review comment", comments[2].Body)

// verify that the commit list is cached
Expand Down
10 changes: 5 additions & 5 deletions pull/testdata/responses/pull_comments.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
},
"body": ":+1:",
"createdAt": "2018-06-27T20:28:22Z",
"updatedAt": "2018-06-27T20:28:22Z"
"lastEditedAt": "2018-06-27T20:28:22Z"
}
]
},
Expand All @@ -35,7 +35,7 @@
"state": "CHANGES_REQUESTED",
"body": "",
"submittedAt": "2018-06-27T20:33:26Z",
"updatedAt": "2018-06-27T20:33:26Z"
"lastEditedAt": "2018-06-27T20:33:26Z"
}
]
}
Expand Down Expand Up @@ -63,7 +63,7 @@
},
"body": "I merge!",
"createdAt": "2018-06-27T20:29:22Z",
"updatedAt": "2018-06-27T20:29:22Z"
"lastEditedAt": "2018-06-27T20:29:22Z"
}
]
},
Expand All @@ -80,7 +80,7 @@
"state": "APPROVED",
"body": "the body",
"submittedAt": "2018-06-27T20:33:27Z",
"updatedAt": "2018-06-27T20:33:27Z"
"lastEditedAt": "2018-06-27T20:33:27Z"
},
{
"author": {
Expand All @@ -89,7 +89,7 @@
"state": "COMMENTED",
"body": "A review comment",
"submittedAt": "2018-06-27T20:38:22Z",
"updatedAt": "2018-06-27T20:38:22Z"
"lastEditedAt": "2018-06-27T20:38:22Z"
}
]
}
Expand Down
6 changes: 3 additions & 3 deletions pull/testdata/responses/pull_reviews.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"state": "CHANGES_REQUESTED",
"body": "",
"submittedAt": "2018-06-27T20:33:26Z",
"updatedAt": "2018-06-27T20:33:26Z"
"lastEditedAt": "2018-06-27T20:33:26Z"
}
]
}
Expand Down Expand Up @@ -46,7 +46,7 @@
"state": "APPROVED",
"body": "the body",
"submittedAt": "2018-06-27T20:33:27Z",
"updatedAt": "2018-06-27T20:33:27Z"
"lastEditedAt": "2018-06-27T20:33:27Z"
},
{
"author": {
Expand All @@ -55,7 +55,7 @@
"state": "COMMENTED",
"body": "A review comment",
"submittedAt": "2018-06-27T20:38:22Z",
"updatedAt": "2018-06-27T20:38:22Z"
"lastEditedAt": "2018-06-27T20:38:22Z"
}
]
}
Expand Down
Loading