Skip to content

Commit

Permalink
Fix defect with bot users when 'delete_previous_comments' is enabled
Browse files Browse the repository at this point in the history
Indeed, in order to delete the correct comments, the golang code makes sure
that comment's `Author.Login` is the same as current `Viewer.Login`. But with
bot users, viewer's login is "githubApp[bot]" and author login is "githubApp".

As a consequence, for the comparison to succeed, we need to remove any
trailing "[bot]" suffix from both parties, and compare the resulting prefixes.

This isue was originally submitted as telia-oss#245 and
fixed later in monzo#4.
  • Loading branch information
bee-san authored and bgandon committed May 11, 2024
1 parent 829f558 commit f0c8ca2
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 1 deletion.
9 changes: 8 additions & 1 deletion github.go
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@ func (m *GithubClient) DeletePreviousComments(prNumber string) error {
}

for _, e := range getComments.Repository.PullRequest.Comments.Edges {
if e.Node.Author.Login == getComments.Viewer.Login {
if cleanBotUserName(e.Node.Author.Login) == cleanBotUserName(getComments.Viewer.Login) {
_, err := m.V3.Issues.DeleteComment(context.TODO(), m.Owner, m.Repository, e.Node.DatabaseId)
if err != nil {
return err
Expand All @@ -453,3 +453,10 @@ func parseRepository(s string) (string, string, error) {
}
return parts[0], parts[1], nil
}

func cleanBotUserName(username string) string {
if strings.HasSuffix(username, "[bot]") {
return strings.TrimSuffix(username, "[bot]")
}
return username
}
47 changes: 47 additions & 0 deletions github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,50 @@ func TestListTeamMembers(t *testing.T) {

require.Contains(t, val, "thelinuxfoundation")
}

func Test_cleanBotUserName(t *testing.T) {
type args struct {
username string
}
tests := []struct {
name string
args args
want string
}{
{
name: "Main Test Case",
args: args{
username: "infratest[bot]",
},
want: "infratest",
},
{
name: "Without suffix bot",
args: args{
username: "infratest",
},
want: "infratest",
},
{
name: "Bot only",
args: args{
username: "[bot]",
},
want: "",
},
{
name: "Bot at start",
args: args{
username: "[bot]infratest",
},
want: "[bot]infratest",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := cleanBotUserName(tt.args.username); got != tt.want {
t.Errorf("cleanBotUserName() = %v, want %v", got, tt.want)
}
})
}
}

0 comments on commit f0c8ca2

Please sign in to comment.