diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index f138adfaf..ab13e6523 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -135,7 +135,7 @@ func (self *CommitLoader) GetCommits(opts GetCommitsOptions) ([]*models.Commit, } if ancestor != "" { - commits = self.setCommitMergedStatuses(ancestor, commits) + commits = setCommitMergedStatuses(ancestor, commits) } return commits, nil @@ -492,10 +492,11 @@ func (self *CommitLoader) commitFromPatch(content string) *models.Commit { } } -func (self *CommitLoader) setCommitMergedStatuses(ancestor string, commits []*models.Commit) []*models.Commit { +func setCommitMergedStatuses(ancestor string, commits []*models.Commit) []*models.Commit { passedAncestor := false for i, commit := range commits { - if strings.HasPrefix(ancestor, commit.Sha) { + // some commits aren't really commits and don't have sha's, such as the update-ref todo + if commit.Sha != "" && strings.HasPrefix(ancestor, commit.Sha) { passedAncestor = true } if commit.Status != models.StatusPushed && commit.Status != models.StatusUnpushed { diff --git a/pkg/commands/git_commands/commit_loader_test.go b/pkg/commands/git_commands/commit_loader_test.go index bcbc33c02..c3cfb0585 100644 --- a/pkg/commands/git_commands/commit_loader_test.go +++ b/pkg/commands/git_commands/commit_loader_test.go @@ -506,3 +506,50 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { }) } } + +func TestCommitLoader_setCommitMergedStatuses(t *testing.T) { + type scenario struct { + testName string + commits []*models.Commit + ancestor string + expectedCommits []*models.Commit + } + + scenarios := []scenario{ + { + testName: "basic", + commits: []*models.Commit{ + {Sha: "12345", Name: "1", Action: models.ActionNone, Status: models.StatusUnpushed}, + {Sha: "67890", Name: "2", Action: models.ActionNone, Status: models.StatusPushed}, + {Sha: "abcde", Name: "3", Action: models.ActionNone, Status: models.StatusPushed}, + }, + ancestor: "67890", + expectedCommits: []*models.Commit{ + {Sha: "12345", Name: "1", Action: models.ActionNone, Status: models.StatusUnpushed}, + {Sha: "67890", Name: "2", Action: models.ActionNone, Status: models.StatusMerged}, + {Sha: "abcde", Name: "3", Action: models.ActionNone, Status: models.StatusMerged}, + }, + }, + { + testName: "with update-ref", + commits: []*models.Commit{ + {Sha: "12345", Name: "1", Action: models.ActionNone, Status: models.StatusUnpushed}, + {Sha: "", Name: "", Action: todo.UpdateRef, Status: models.StatusNone}, + {Sha: "abcde", Name: "3", Action: models.ActionNone, Status: models.StatusPushed}, + }, + ancestor: "deadbeef", + expectedCommits: []*models.Commit{ + {Sha: "12345", Name: "1", Action: models.ActionNone, Status: models.StatusUnpushed}, + {Sha: "", Name: "", Action: todo.UpdateRef, Status: models.StatusNone}, + {Sha: "abcde", Name: "3", Action: models.ActionNone, Status: models.StatusPushed}, + }, + }, + } + + for _, scenario := range scenarios { + t.Run(scenario.testName, func(t *testing.T) { + expectedCommits := setCommitMergedStatuses(scenario.ancestor, scenario.commits) + assert.Equal(t, scenario.expectedCommits, expectedCommits) + }) + } +}