diff --git a/cmd/app.go b/cmd/app.go index 09ffafa..dd0717c 100644 --- a/cmd/app.go +++ b/cmd/app.go @@ -10,10 +10,11 @@ import ( "strings" "github.com/cupcicm/opp/core" + "github.com/cupcicm/opp/core/story" "github.com/urfave/cli/v2" ) -func MakeApp(out io.Writer, repo *core.Repo, gh func(context.Context) core.Gh) *cli.App { +func MakeApp(out io.Writer, in io.Reader, repo *core.Repo, gh func(context.Context) core.Gh, sf func(string, string) story.StoryFetcher) *cli.App { return &cli.App{ Name: "opp", Usage: "Create, update and merge Github pull requests from the command line.", @@ -21,7 +22,7 @@ func MakeApp(out io.Writer, repo *core.Repo, gh func(context.Context) core.Gh) * InitCommand(repo), CleanCommand(repo, gh), CloseCommand(repo, gh), - PrCommand(repo, gh), + PrCommand(in, repo, gh, sf), MergeCommand(repo, gh), StatusCommand(out, repo, gh), RebaseCommand(repo), diff --git a/cmd/pr.go b/cmd/pr.go index 583b2ed..bc7de19 100644 --- a/cmd/pr.go +++ b/cmd/pr.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "io" "sort" "strings" @@ -52,7 +53,7 @@ whether it reached the base branch or the tip of another PR first when walking b `) ) -func PrCommand(repo *core.Repo, gh func(context.Context) core.Gh) *cli.Command { +func PrCommand(in io.Reader, repo *core.Repo, gh func(context.Context) core.Gh, sf func(string, string) story.StoryFetcher) *cli.Command { cmd := &cli.Command{ Name: "pr", Aliases: []string{"pull-request", "new"}, @@ -90,7 +91,7 @@ func PrCommand(repo *core.Repo, gh func(context.Context) core.Gh) *cli.Command { if err != nil { return err } - pr := create{Repo: repo, Github: gh(cCtx.Context), StoryService: story.NewStoryService()} + pr := create{Repo: repo, Github: gh(cCtx.Context), StoryFetcher: sf} args, err := pr.SanitizeArgs(cCtx) if err != nil { return err @@ -103,7 +104,7 @@ func PrCommand(repo *core.Repo, gh func(context.Context) core.Gh) *cli.Command { } args = newArgs } - localPr, err := pr.Create(cCtx.Context, args) + localPr, err := pr.Create(cCtx.Context, in, args) if err != nil { return err } @@ -135,7 +136,7 @@ func PrCommand(repo *core.Repo, gh func(context.Context) core.Gh) *cli.Command { type create struct { Repo *core.Repo Github core.Gh - StoryService story.StoryService + StoryFetcher func(string, string) story.StoryFetcher } type args struct { @@ -311,11 +312,11 @@ func (c *create) RebasePrCommits(ctx context.Context, previousArgs *args) (*args }, nil } -func (c *create) Create(ctx context.Context, args *args) (*core.LocalPr, error) { +func (c *create) Create(ctx context.Context, in io.Reader, args *args) (*core.LocalPr, error) { // The first commit is the child-most one. lastCommit := args.Commits[0].Hash - title, body, err := c.GetBodyAndTitle(args.Commits) + title, body, err := c.GetBodyAndTitle(ctx, in, args.Commits) if err != nil { return nil, fmt.Errorf("could not get the pull request body and title: %w", err) } @@ -336,13 +337,14 @@ func (c *create) Create(ctx context.Context, args *args) (*core.LocalPr, error) return localPr, err } -func (c *create) GetBodyAndTitle(commits []*object.Commit) (string, string, error) { +func (c *create) GetBodyAndTitle(ctx context.Context, in io.Reader, commits []*object.Commit) (string, string, error) { rawTitle, rawBody := c.getRawBodyAndTitle(commits) commitMessages := make([]string, len(commits)) for i, c := range commits { commitMessages[i] = c.Message } - title, body, err := c.StoryService.EnrichBodyAndTitle(commitMessages, rawTitle, rawBody) + storyService := story.NewStoryService(c.StoryFetcher, in) + title, body, err := storyService.EnrichBodyAndTitle(ctx, commitMessages, rawTitle, rawBody) if err != nil { return "", "", fmt.Errorf("could not enrich the PR with the Story: %w", err) } diff --git a/cmd/pr_story_test.go b/cmd/pr_story_test.go index 612a809..f013bcc 100644 --- a/cmd/pr_story_test.go +++ b/cmd/pr_story_test.go @@ -5,11 +5,12 @@ import ( "testing" "github.com/cupcicm/opp/core" + "github.com/cupcicm/opp/core/story" "github.com/cupcicm/opp/core/tests" "github.com/google/go-github/v56/github" ) -func TestPrStory(t *testing.T) { +func TestPrStoryInCommits(t *testing.T) { remote := "cupcicm/pr/2" base := "master" draft := false @@ -38,12 +39,6 @@ func TestPrStory(t *testing.T) { expectedTitle: "my title [ABC-123]", expectedBody: "- Linear [ABC-123](https://my.base.url/browse/ABC-123)\n\nmy body", }, - { - name: "no story", - commitMessages: []string{"longest commit message\nlongest commit message body", "a\nb", "c\nd"}, - expectedTitle: "longest commit message", - expectedBody: "longest commit message body", - }, { name: "empty body", commitMessages: []string{"[ABC-123] my commit title", "a\nb"}, @@ -96,3 +91,109 @@ func TestPrStory(t *testing.T) { }) } } + +func TestPrStoryFetched(t *testing.T) { + remote := "cupcicm/pr/2" + base := "master" + draft := false + + testCases := []struct { + name string + commitMessages []string + fetchedStories []story.Story + errFetchStories bool + selectedStory string + expectedTitle string + expectedBody string + }{ + { + name: "no story", + commitMessages: []string{"longest commit message\nlongest commit message body", "a\nb", "c\nd"}, + fetchedStories: []story.Story{}, + errFetchStories: false, + selectedStory: "", + expectedTitle: "longest commit message", + expectedBody: "longest commit message body", + }, + { + name: "error fetching story", + commitMessages: []string{"longest commit message\nlongest commit message body", "a\nb", "c\nd"}, + fetchedStories: []story.Story{}, + errFetchStories: true, + selectedStory: "", + expectedTitle: "longest commit message", + expectedBody: "longest commit message body", + }, + { + name: "one story", + commitMessages: []string{"longest commit message\nlongest commit message body", "a\nb", "c\nd"}, + fetchedStories: []story.Story{{Title: "Story Title", Identifier: "ABC-123"}}, + errFetchStories: false, + selectedStory: "0", + expectedTitle: "[ABC-123] longest commit message", + expectedBody: "- Linear [ABC-123](https://my.base.url/browse/ABC-123)\n\nlongest commit message body", + }, + { + name: "two stories - first chosen", + commitMessages: []string{"longest commit message\nlongest commit message body", "a\nb", "c\nd"}, + fetchedStories: []story.Story{{Title: "Story Title", Identifier: "ABC-123"}, {Title: "Other Story Title", Identifier: "ABC-456"}}, + errFetchStories: false, + selectedStory: "0", + expectedTitle: "[ABC-123] longest commit message", + expectedBody: "- Linear [ABC-123](https://my.base.url/browse/ABC-123)\n\nlongest commit message body", + }, + { + name: "two stories - second chosen", + commitMessages: []string{"longest commit message\nlongest commit message body", "a\nb", "c\nd"}, + fetchedStories: []story.Story{{Title: "Story Title", Identifier: "ABC-123"}, {Title: "Other Story Title", Identifier: "ABC-456"}}, + errFetchStories: false, + selectedStory: "1", + expectedTitle: "[ABC-456] longest commit message", + expectedBody: "- Linear [ABC-456](https://my.base.url/browse/ABC-456)\n\nlongest commit message body", + }, + { + name: "two stories - invalid input", + commitMessages: []string{"longest commit message\nlongest commit message body", "a\nb", "c\nd"}, + fetchedStories: []story.Story{{Title: "Story Title", Identifier: "ABC-123"}, {Title: "Other Story Title", Identifier: "ABC-456"}}, + errFetchStories: false, + selectedStory: "invalid", + expectedTitle: "longest commit message", + expectedBody: "longest commit message body", + }, + { + name: "two stories - no input", + commitMessages: []string{"longest commit message\nlongest commit message body", "a\nb", "c\nd"}, + fetchedStories: []story.Story{{Title: "Story Title", Identifier: "ABC-123"}, {Title: "Other Story Title", Identifier: "ABC-456"}}, + errFetchStories: false, + selectedStory: "", + expectedTitle: "longest commit message", + expectedBody: "longest commit message body", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + r := tests.NewTestRepo(t) + + r.Repo.GitExec(context.Background(), "checkout origin/master").Run() + r.Repo.GitExec(context.Background(), "checkout -b test_branch").Run() + + wt := core.Must(r.Source.Worktree()) + + for _, commitMessage := range tc.commitMessages { + wt.Add("README.md") + r.Commit(commitMessage) + } + + prDetails := github.NewPullRequest{ + Title: &tc.expectedTitle, + Head: &remote, + Base: &base, + Body: &tc.expectedBody, + Draft: &draft, + } + + r.CreatePrAssertPrDetailsWithStories(t, "HEAD", 2, tc.fetchedStories, tc.errFetchStories, tc.selectedStory, prDetails) + }) + } +} diff --git a/core/config.go b/core/config.go index 2079aec..ec02deb 100644 --- a/core/config.go +++ b/core/config.go @@ -66,3 +66,7 @@ func GetStoryToolUrl() string { func EnrichPrBodyWithStoryEnabled() bool { return viper.GetBool("story.enrich") } + +func GetStoryToolToken() string { + return viper.GetString("story.token") +} diff --git a/core/story/story_fetcher.go b/core/story/story_fetcher.go index f036198..d446911 100644 --- a/core/story/story_fetcher.go +++ b/core/story/story_fetcher.go @@ -12,3 +12,12 @@ type Story struct { type StoryFetcher interface { FetchInProgressStories(context.Context) ([]Story, error) } + +func NewStoryFetcher(tool, token string) StoryFetcher { + switch tool { + case "linear": + return NewLinearStoryFetcher(token) + default: + panic("Story tool not supported to fetch stories") + } +} diff --git a/core/story/story_service.go b/core/story/story_service.go index b093e92..5a11552 100644 --- a/core/story/story_service.go +++ b/core/story/story_service.go @@ -1,8 +1,13 @@ package story import ( + "bufio" + "context" + "errors" "fmt" + "io" "regexp" + "strconv" "strings" "github.com/cupcicm/opp/core" @@ -15,19 +20,20 @@ const storyPattern = `\w+[-_]\d+` var storyPatternWithBrackets = fmt.Sprintf(`\[%s\]`, storyPattern) type StoryService interface { - EnrichBodyAndTitle(commitMessages []string, rawTitle, rawBody string) (title, body string, err error) + EnrichBodyAndTitle(ctx context.Context, commitMessages []string, rawTitle, rawBody string) (title, body string, err error) } -func NewStoryService() StoryService { +func NewStoryService(storyFetcher func(string, string) StoryFetcher, in io.Reader) StoryService { tool := core.GetStoryTool() url := core.GetStoryToolUrl() + token := core.GetStoryToolToken() - if tool == "" && url == "" { + if tool == "" && url == "" && token == "" { return &StoryServiceNoop{} } - if tool == "" || url == "" { - panic("please fill in all fields for the Story in the config (story.tool and story.url)") + if tool == "" || url == "" || token == "" { + panic("please fill in all Story fields in the config (story.tool, story.url and story.token)") } re, err := regexp.Compile(storyPattern) @@ -45,12 +51,14 @@ func NewStoryService() StoryService { reWithBrackets: reWithBrackets, tool: tool, url: url, + storyFetcher: storyFetcher(tool, token), + in: in, } } type StoryServiceNoop struct{} -func (s *StoryServiceNoop) EnrichBodyAndTitle(commitMessages []string, rawTitle, rawBody string) (title, body string, err error) { +func (s *StoryServiceNoop) EnrichBodyAndTitle(_ context.Context, _ []string, rawTitle, rawBody string) (title, body string, err error) { return rawTitle, rawBody, nil } @@ -59,10 +67,12 @@ type StoryServiceEnabled struct { reWithBrackets *regexp.Regexp tool string url string + storyFetcher StoryFetcher + in io.Reader } -func (s *StoryServiceEnabled) EnrichBodyAndTitle(commitMessages []string, rawTitle, rawBody string) (title, body string, err error) { - story, title := s.getStoryAndEnrichTitle(commitMessages, rawTitle) +func (s *StoryServiceEnabled) EnrichBodyAndTitle(ctx context.Context, commitMessages []string, rawTitle, rawBody string) (title, body string, err error) { + story, title := s.getStoryAndEnrichTitle(ctx, s.in, commitMessages, rawTitle) body, err = s.enrichBody(rawBody, story) if err != nil { return "", "", err @@ -70,14 +80,14 @@ func (s *StoryServiceEnabled) EnrichBodyAndTitle(commitMessages []string, rawTit return title, body, nil } -func (s *StoryServiceEnabled) getStoryAndEnrichTitle(commitMessages []string, rawTitle string) (story, title string) { +func (s *StoryServiceEnabled) getStoryAndEnrichTitle(ctx context.Context, in io.Reader, commitMessages []string, rawTitle string) (story, title string) { story, found := s.storyFromMessageOrTitle(rawTitle) if found { return story, rawTitle } - story, found = s.findStory(commitMessages) + story, found = s.findStory(ctx, in, commitMessages) if found { return story, strings.Join([]string{s.formatStoryInPRTitle(story), rawTitle}, " ") } @@ -85,13 +95,13 @@ func (s *StoryServiceEnabled) getStoryAndEnrichTitle(commitMessages []string, ra return "", rawTitle } -func (s *StoryServiceEnabled) findStory(commitMessages []string) (story string, found bool) { +func (s *StoryServiceEnabled) findStory(ctx context.Context, in io.Reader, commitMessages []string) (story string, found bool) { story, found = s.extractFromCommitMessages(commitMessages) if found { return story, true } - story, found = s.fetchStory() + story, found = s.fetchStory(ctx, in) if found { return story, true } @@ -99,9 +109,56 @@ func (s *StoryServiceEnabled) findStory(commitMessages []string) (story string, return "", false } -func (s *StoryServiceEnabled) fetchStory() (story string, found bool) { - // TODO(ClairePhi): implement - return "", false +func (s *StoryServiceEnabled) fetchStory(ctx context.Context, in io.Reader) (story string, found bool) { + stories, err := s.storyFetcher.FetchInProgressStories(ctx) + if err != nil { + fmt.Printf("could not fetch In Progress Stories: %s\n", err.Error()) + return "", false + } + + if len(stories) == 0 { + return "", false + } + + story, err = s.selectStory(in, stories) + if err != nil { + fmt.Printf("could not select the Story: %s\n", err.Error()) + return "", false + } + + return story, true +} + +func (s *StoryServiceEnabled) selectStory(in io.Reader, stories []Story) (selectedStory string, err error) { + fmt.Println("In Progress stories assigned to me:") + + for idx, story := range stories { + fmt.Printf("%d - [%s] %s\n", idx, story.Identifier, story.Title) + } + + fmt.Println("") + + fmt.Println("Choose index: ") + + // Taking input from user + reader := bufio.NewReader(in) + chosenIndex, err := reader.ReadString('\n') + if err != nil { + return "", errors.New("the input could not be read") + } + + chosenIndex = strings.TrimSuffix(chosenIndex, "\n") + + index, err := strconv.Atoi(chosenIndex) + if err != nil { + return "", errors.New("the input could not be converted to integer") + } + + if index < 0 || index > len(stories)-1 { + return "", errors.New("the input is out from the story range") + } + + return stories[index].Identifier, nil } func (s *StoryServiceEnabled) extractFromCommitMessages(messages []string) (story string, found bool) { diff --git a/core/tests/testrepo.go b/core/tests/testrepo.go index ebde45f..a9ac577 100644 --- a/core/tests/testrepo.go +++ b/core/tests/testrepo.go @@ -1,7 +1,9 @@ package tests import ( + "bytes" "context" + "errors" "fmt" "os" "path" @@ -11,6 +13,7 @@ import ( "github.com/cupcicm/opp/cmd" "github.com/cupcicm/opp/core" + "github.com/cupcicm/opp/core/story" "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/config" "github.com/go-git/go-git/v5/plumbing" @@ -28,12 +31,14 @@ type Paths struct { type TestRepo struct { *core.Repo - Source *git.Repository - GithubRepo *git.Repository - Paths Paths - GithubMock *GithubMock - App *cli.App - Out *strings.Builder + Source *git.Repository + GithubRepo *git.Repository + Paths Paths + GithubMock *GithubMock + StoryFetcherMock *StoryFetcherMock + App *cli.App + Out *strings.Builder + In *bytes.Buffer } func setConfig() { @@ -43,6 +48,7 @@ func setConfig() { viper.Set("repo.remote", "origin") viper.Set("story.tool", "linear") viper.Set("story.url", "https://my.base.url/browse") + viper.Set("story.token", "my token") } func NewTestRepo(t *testing.T) *TestRepo { @@ -60,7 +66,9 @@ func NewTestRepo(t *testing.T) *TestRepo { PullRequestsMock: &PullRequestsMock{}, IssuesMock: &IssuesMock{}, } + storyFetcherMock := &StoryFetcherMock{} var out strings.Builder + var in bytes.Buffer testRepo := TestRepo{ Source: source, GithubRepo: github, @@ -69,10 +77,14 @@ func NewTestRepo(t *testing.T) *TestRepo { Source: sourcePath, Destination: destPath, }, - GithubMock: mock, - Out: &out, - App: cmd.MakeApp(&out, repo, func(context.Context) core.Gh { + GithubMock: mock, + StoryFetcherMock: storyFetcherMock, + Out: &out, + In: &in, + App: cmd.MakeApp(&out, &in, repo, func(context.Context) core.Gh { return mock + }, func(string, string) story.StoryFetcher { + return storyFetcherMock }), } testRepo.PrepareSource() @@ -139,9 +151,16 @@ func (r *TestRepo) AssertHasPr(t *testing.T, n int) *core.LocalPr { } func (r *TestRepo) CreatePr(t *testing.T, ref string, prNumber int, args ...string) *core.LocalPr { + return r.CreatePrWithStories(t, ref, prNumber, []story.Story{}, false, "", args...) +} + +func (r *TestRepo) CreatePrWithStories(t *testing.T, ref string, prNumber int, stories []story.Story, errStories bool, selectedStory string, args ...string) *core.LocalPr { r.GithubMock.IssuesMock.CallListAndReturnPr(prNumber - 1) r.GithubMock.PullRequestsMock.CallCreate(prNumber) - + r.StoryFetcherMock.CallFetchInProgressStories(stories, errStories) + if !errStories && len(stories) > 0 { + r.In.Write([]byte(fmt.Sprintf("%s\n", selectedStory))) + } r.Run("pr", append(args, ref)...) return r.AssertHasPr(t, prNumber) } @@ -152,6 +171,12 @@ func (r *TestRepo) CreatePrAssertPrDetails(t *testing.T, ref string, prNumber in return pr } +func (r *TestRepo) CreatePrAssertPrDetailsWithStories(t *testing.T, ref string, prNumber int, stories []story.Story, errStories bool, selectedStory string, prDetails github.NewPullRequest, args ...string) *core.LocalPr { + pr := r.CreatePrWithStories(t, ref, prNumber, stories, errStories, selectedStory, args...) + r.GithubMock.PullRequestsMock.AssertCalled(t, "Create", mock.Anything, "cupcicm", "opp", &prDetails) + return pr +} + func (r *TestRepo) MergePr(t *testing.T, pr *core.LocalPr) error { tip := core.Must(r.GetLocalTip(pr)) r.GithubMock.PullRequestsMock.CallGetAndReturnMergeable(pr.PrNumber, true) @@ -267,3 +292,21 @@ func (m *PullRequestsMock) CallMerge(prNumber int, tip string) { &response, nil, nil, ).Once() } + +type StoryFetcherMock struct { + mock.Mock +} + +func (s *StoryFetcherMock) FetchInProgressStories(ctx context.Context) ([]story.Story, error) { + args := s.Mock.Called(ctx) + return args.Get(0).([]story.Story), args.Error(1) +} + +func (s *StoryFetcherMock) CallFetchInProgressStories(stories []story.Story, fetchError bool) { + var err error + if fetchError { + err = errors.New("Story fetch error") + } + + s.On("FetchInProgressStories", mock.Anything).Return(stories, err).Once() +} diff --git a/opp.go b/opp.go index b2bcd38..f09d570 100644 --- a/opp.go +++ b/opp.go @@ -10,6 +10,7 @@ import ( "github.com/cupcicm/opp/cmd" "github.com/cupcicm/opp/core" + "github.com/cupcicm/opp/core/story" "github.com/spf13/viper" ) @@ -21,6 +22,10 @@ func gh(ctx context.Context) core.Gh { return core.NewClient(ctx) } +func sf(tool, token string) story.StoryFetcher { + return story.NewStoryFetcher(tool, token) +} + func main() { repo := core.Current() viper.AddConfigPath(repo.DotOpDir()) @@ -29,7 +34,7 @@ func main() { viper.SetConfigType("yaml") viper.ReadInConfig() - root := cmd.MakeApp(os.Stdout, repo, gh) + root := cmd.MakeApp(os.Stdout, os.Stdin, repo, gh, sf) ctx, cancel := CommandContext() if !repo.OppEnabled() && (len(os.Args) != 2 || os.Args[1] != "init") { fmt.Println("Please run opp init first")