diff options
59 files changed, 1869 insertions, 1059 deletions
diff --git a/api/graphql/resolvers/mutation.go b/api/graphql/resolvers/mutation.go index 9cd936a6..00c9e3c1 100644 --- a/api/graphql/resolvers/mutation.go +++ b/api/graphql/resolvers/mutation.go @@ -5,11 +5,12 @@ import ( "time" "github.com/MichaelMure/git-bug/api/auth" - "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/api/graphql/graph" "github.com/MichaelMure/git-bug/api/graphql/models" "github.com/MichaelMure/git-bug/bug" "github.com/MichaelMure/git-bug/cache" + "github.com/MichaelMure/git-bug/entity" + "github.com/MichaelMure/git-bug/util/text" ) var _ graph.MutationResolver = &mutationResolver{} @@ -50,7 +51,12 @@ func (r mutationResolver) NewBug(ctx context.Context, input models.NewBugInput) return nil, err } - b, op, err := repo.NewBugRaw(author, time.Now().Unix(), input.Title, input.Message, input.Files, nil) + b, op, err := repo.NewBugRaw(author, + time.Now().Unix(), + text.CleanupOneLine(input.Title), + text.Cleanup(input.Message), + input.Files, + nil) if err != nil { return nil, err } @@ -73,7 +79,11 @@ func (r mutationResolver) AddComment(ctx context.Context, input models.AddCommen return nil, err } - op, err := b.AddCommentRaw(author, time.Now().Unix(), input.Message, input.Files, nil) + op, err := b.AddCommentRaw(author, + time.Now().Unix(), + text.Cleanup(input.Message), + input.Files, + nil) if err != nil { return nil, err } @@ -101,7 +111,13 @@ func (r mutationResolver) EditComment(ctx context.Context, input models.EditComm return nil, err } - op, err := b.EditCommentRaw(author, time.Now().Unix(), entity.Id(input.Target), input.Message, nil) + op, err := b.EditCommentRaw( + author, + time.Now().Unix(), + entity.Id(input.Target), + text.Cleanup(input.Message), + nil, + ) if err != nil { return nil, err } @@ -129,7 +145,13 @@ func (r mutationResolver) ChangeLabels(ctx context.Context, input *models.Change return nil, err } - results, op, err := b.ChangeLabelsRaw(author, time.Now().Unix(), input.Added, input.Removed, nil) + results, op, err := b.ChangeLabelsRaw( + author, + time.Now().Unix(), + text.CleanupOneLineArray(input.Added), + text.CleanupOneLineArray(input.Removed), + nil, + ) if err != nil { return nil, err } @@ -219,7 +241,12 @@ func (r mutationResolver) SetTitle(ctx context.Context, input models.SetTitleInp return nil, err } - op, err := b.SetTitleRaw(author, time.Now().Unix(), input.Title, nil) + op, err := b.SetTitleRaw( + author, + time.Now().Unix(), + text.CleanupOneLine(input.Title), + nil, + ) if err != nil { return nil, err } diff --git a/bridge/core/import.go b/bridge/core/import.go index 0b0b4c68..c1965d4e 100644 --- a/bridge/core/import.go +++ b/bridge/core/import.go @@ -34,6 +34,9 @@ const ( // but not severe enough to consider the import a failure. ImportEventWarning + // The import system (web API) has reached the rate limit + ImportEventRateLimiting + // Error happened during import ImportEventError ) @@ -87,6 +90,8 @@ func (er ImportResult) String() string { parts = append(parts, fmt.Sprintf("err: %s", er.Err)) } return strings.Join(parts, " ") + case ImportEventRateLimiting: + return fmt.Sprintf("rate limiting: %s", er.Reason) default: panic("unknown import result") @@ -165,3 +170,10 @@ func NewImportIdentity(id entity.Id) ImportResult { Event: ImportEventIdentity, } } + +func NewImportRateLimiting(msg string) ImportResult { + return ImportResult{ + Reason: msg, + Event: ImportEventRateLimiting, + } +} diff --git a/bridge/github/config.go b/bridge/github/config.go index 2b5af7fb..1e23c8ee 100644 --- a/bridge/github/config.go +++ b/bridge/github/config.go @@ -251,7 +251,7 @@ func promptUserToGoToBrowser(url, userCode string) { fmt.Println("Please visit the following Github URL in a browser and enter your user authentication code.") fmt.Println() fmt.Println(" URL:", url) - fmt.Println(" user authentiation code:", userCode) + fmt.Println(" user authentication code:", userCode) fmt.Println() } diff --git a/bridge/github/export.go b/bridge/github/export.go index 1a59fbb3..264f2a23 100644 --- a/bridge/github/export.go +++ b/bridge/github/export.go @@ -504,7 +504,7 @@ func (ge *githubExporter) cacheGithubLabels(ctx context.Context, gc *githubv4.Cl return nil } -func (ge *githubExporter) getLabelID(gc *githubv4.Client, label string) (string, error) { +func (ge *githubExporter) getLabelID(label string) (string, error) { label = strings.ToLower(label) for cachedLabel, ID := range ge.cachedLabels { if label == strings.ToLower(cachedLabel) { @@ -598,7 +598,7 @@ func (ge *githubExporter) createGithubLabelV4(gc *githubv4.Client, label, labelC func (ge *githubExporter) getOrCreateGithubLabelID(ctx context.Context, gc *githubv4.Client, repositoryID string, label bug.Label) (string, error) { // try to get label id from cache - labelID, err := ge.getLabelID(gc, string(label)) + labelID, err := ge.getLabelID(string(label)) if err == nil { return labelID, nil } diff --git a/bridge/github/export_test.go b/bridge/github/export_test.go index b7a36bcf..78425e60 100644 --- a/bridge/github/export_test.go +++ b/bridge/github/export_test.go @@ -268,7 +268,7 @@ func TestGithubPushPull(t *testing.T) { require.True(t, ok) require.Equal(t, issueOrigin, target) - //TODO: maybe more tests to ensure bug final state + // TODO: maybe more tests to ensure bug final state }) } } diff --git a/bridge/github/import.go b/bridge/github/import.go index af62746f..f410cc65 100644 --- a/bridge/github/import.go +++ b/bridge/github/import.go @@ -15,6 +15,8 @@ import ( "github.com/MichaelMure/git-bug/util/text" ) +const EmptyTitlePlaceholder = "<empty string>" + // githubImporter implement the Importer interface type githubImporter struct { conf core.Configuration @@ -22,8 +24,8 @@ type githubImporter struct { // default client client *githubv4.Client - // iterator - iterator *iterator + // mediator to access the Github API + mediator *importMediator // send only channel out chan<- core.ImportResult @@ -31,7 +33,6 @@ type githubImporter struct { func (gi *githubImporter) Init(_ context.Context, repo *cache.RepoCache, conf core.Configuration) error { gi.conf = conf - creds, err := auth.List(repo, auth.WithTarget(target), auth.WithKind(auth.KindToken), @@ -40,11 +41,9 @@ func (gi *githubImporter) Init(_ context.Context, repo *cache.RepoCache, conf co if err != nil { return err } - - if len(creds) == 0 { + if len(creds) <= 0 { return ErrMissingIdentityToken } - gi.client = buildClient(creds[0].(*auth.Token)) return nil @@ -53,46 +52,95 @@ func (gi *githubImporter) Init(_ context.Context, repo *cache.RepoCache, conf co // ImportAll iterate over all the configured repository issues and ensure the creation of the // missing issues / timeline items / edits / label events ... func (gi *githubImporter) ImportAll(ctx context.Context, repo *cache.RepoCache, since time.Time) (<-chan core.ImportResult, error) { - gi.iterator = NewIterator(ctx, gi.client, 10, gi.conf[confKeyOwner], gi.conf[confKeyProject], since) + gi.mediator = NewImportMediator(ctx, gi.client, gi.conf[confKeyOwner], gi.conf[confKeyProject], since) out := make(chan core.ImportResult) gi.out = out go func() { defer close(gi.out) - - // Loop over all matching issues - for gi.iterator.NextIssue() { - issue := gi.iterator.IssueValue() - // create issue - b, err := gi.ensureIssue(repo, issue) - if err != nil { - err := fmt.Errorf("issue creation: %v", err) - out <- core.NewImportError(err, "") - return + var currBug *cache.BugCache + var currEvent ImportEvent + var nextEvent ImportEvent + var err error + for { + // An IssueEvent contains the issue in its most recent state. If an issue + // has at least one issue edit, then the history of the issue edits is + // represented by IssueEditEvents. That is, the unedited (original) issue + // might be saved only in the IssueEditEvent following the IssueEvent. + // Since we replicate the edit history we need to either use the IssueEvent + // (if there are no edits) or the IssueEvent together with its first + // IssueEditEvent (if there are edits). + // Exactly the same is true for comments and comment edits. + // As a consequence we need to look at the current event and one look ahead + // event. + currEvent = nextEvent + if currEvent == nil { + currEvent = gi.getEventHandleMsgs() } - - // loop over timeline items - for gi.iterator.NextTimelineItem() { - item := gi.iterator.TimelineItemValue() - err := gi.ensureTimelineItem(repo, b, item) + if currEvent == nil { + break + } + nextEvent = gi.getEventHandleMsgs() + + switch event := currEvent.(type) { + case RateLimitingEvent: + out <- core.NewImportRateLimiting(event.msg) + case IssueEvent: + // first: commit what is being held in currBug + if err = gi.commit(currBug, out); err != nil { + out <- core.NewImportError(err, "") + return + } + // second: create new issue + switch next := nextEvent.(type) { + case IssueEditEvent: + // consuming and using next event + nextEvent = nil + currBug, err = gi.ensureIssue(ctx, repo, &event.issue, &next.userContentEdit) + default: + currBug, err = gi.ensureIssue(ctx, repo, &event.issue, nil) + } + if err != nil { + err := fmt.Errorf("issue creation: %v", err) + out <- core.NewImportError(err, "") + return + } + case IssueEditEvent: + err = gi.ensureIssueEdit(ctx, repo, currBug, event.issueId, &event.userContentEdit) + if err != nil { + err = fmt.Errorf("issue edit: %v", err) + out <- core.NewImportError(err, "") + return + } + case TimelineEvent: + if next, ok := nextEvent.(CommentEditEvent); ok && event.Typename == "IssueComment" { + // consuming and using next event + nextEvent = nil + err = gi.ensureComment(ctx, repo, currBug, &event.timelineItem.IssueComment, &next.userContentEdit) + } else { + err = gi.ensureTimelineItem(ctx, repo, currBug, &event.timelineItem) + } if err != nil { err = fmt.Errorf("timeline item creation: %v", err) out <- core.NewImportError(err, "") return } - } - - if !b.NeedCommit() { - out <- core.NewImportNothing(b.Id(), "no imported operation") - } else if err := b.Commit(); err != nil { - // commit bug state - err = fmt.Errorf("bug commit: %v", err) - out <- core.NewImportError(err, "") - return + case CommentEditEvent: + err = gi.ensureCommentEdit(ctx, repo, currBug, event.commentId, &event.userContentEdit) + if err != nil { + err = fmt.Errorf("comment edit: %v", err) + out <- core.NewImportError(err, "") + return + } + default: + panic("Unknown event type") } } - - if err := gi.iterator.Error(); err != nil { + // commit what is being held in currBug before returning + if err = gi.commit(currBug, out); err != nil { + out <- core.NewImportError(err, "") + } + if err = gi.mediator.Error(); err != nil { gi.out <- core.NewImportError(err, "") } }() @@ -100,9 +148,35 @@ func (gi *githubImporter) ImportAll(ctx context.Context, repo *cache.RepoCache, return out, nil } -func (gi *githubImporter) ensureIssue(repo *cache.RepoCache, issue issue) (*cache.BugCache, error) { - // ensure issue author - author, err := gi.ensurePerson(repo, issue.Author) +func (gi *githubImporter) getEventHandleMsgs() ImportEvent { + for { + // read event from import mediator + event := gi.mediator.NextImportEvent() + // consume (and use) all rate limiting events + if e, ok := event.(RateLimitingEvent); ok { + gi.out <- core.NewImportRateLimiting(e.msg) + continue + } + return event + } +} + +func (gi *githubImporter) commit(b *cache.BugCache, out chan<- core.ImportResult) error { + if b == nil { + return nil + } + if !b.NeedCommit() { + out <- core.NewImportNothing(b.Id(), "no imported operation") + return nil + } else if err := b.Commit(); err != nil { + // commit bug state + return fmt.Errorf("bug commit: %v", err) + } + return nil +} + +func (gi *githubImporter) ensureIssue(ctx context.Context, repo *cache.RepoCache, issue *issue, issueEdit *userContentEdit) (*cache.BugCache, error) { + author, err := gi.ensurePerson(ctx, repo, issue.Author) if err != nil { return nil, err } @@ -112,113 +186,62 @@ func (gi *githubImporter) ensureIssue(repo *cache.RepoCache, issue issue) (*cach return excerpt.CreateMetadata[core.MetaKeyOrigin] == target && excerpt.CreateMetadata[metaKeyGithubId] == parseId(issue.Id) }) - if err != nil && err != bug.ErrBugNotExist { + if err == nil { + return b, nil + } + if err != bug.ErrBugNotExist { return nil, err } - // get issue edits - var issueEdits []userContentEdit - for gi.iterator.NextIssueEdit() { - issueEdits = append(issueEdits, gi.iterator.IssueEditValue()) + // At Github there exist issues with seemingly empty titles. An example is + // https://github.com/NixOS/nixpkgs/issues/72730 . + // The title provided by the GraphQL API actually consists of a space followed by a + // zero width space (U+200B). This title would cause the NewBugRaw() function to + // return an error: empty title. + title := string(issue.Title) + if title == " \u200b" { // U+200B == zero width space + title = EmptyTitlePlaceholder } - // if issueEdits is empty - if len(issueEdits) == 0 { - if err == bug.ErrBugNotExist { - cleanText, err := text.Cleanup(string(issue.Body)) - if err != nil { - return nil, err - } - - // create bug - b, _, err = repo.NewBugRaw( - author, - issue.CreatedAt.Unix(), - issue.Title, - cleanText, - nil, - map[string]string{ - core.MetaKeyOrigin: target, - metaKeyGithubId: parseId(issue.Id), - metaKeyGithubUrl: issue.Url.String(), - }) - if err != nil { - return nil, err - } - - // importing a new bug - gi.out <- core.NewImportBug(b.Id()) - } + var textInput string + if issueEdit != nil { + // use the first issue edit: it represents the bug creation itself + textInput = string(*issueEdit.Diff) } else { - // create bug from given issueEdits - for i, edit := range issueEdits { - if i == 0 && b != nil { - // The first edit in the github result is the issue creation itself, we already have that - continue - } - - cleanText, err := text.Cleanup(string(*edit.Diff)) - if err != nil { - return nil, err - } - - // if the bug doesn't exist - if b == nil { - // we create the bug as soon as we have a legit first edition - b, _, err = repo.NewBugRaw( - author, - issue.CreatedAt.Unix(), - issue.Title, // TODO: this is the *current* title, not the original one - cleanText, - nil, - map[string]string{ - core.MetaKeyOrigin: target, - metaKeyGithubId: parseId(issue.Id), - metaKeyGithubUrl: issue.Url.String(), - }, - ) - - if err != nil { - return nil, err - } - // importing a new bug - gi.out <- core.NewImportBug(b.Id()) - continue - } - - // other edits will be added as CommentEdit operations - target, err := b.ResolveOperationWithMetadata(metaKeyGithubId, parseId(issue.Id)) - if err == cache.ErrNoMatchingOp { - // original comment is missing somehow, issuing a warning - gi.out <- core.NewImportWarning(fmt.Errorf("comment ID %s to edit is missing", parseId(issue.Id)), b.Id()) - continue - } - if err != nil { - return nil, err - } + // if there are no issue edits then the issue struct holds the bug creation + textInput = string(issue.Body) + } - err = gi.ensureCommentEdit(repo, b, target, edit) - if err != nil { - return nil, err - } - } + // create bug + b, _, err = repo.NewBugRaw( + author, + issue.CreatedAt.Unix(), + text.CleanupOneLine(title), // TODO: this is the *current* title, not the original one + text.Cleanup(textInput), + nil, + map[string]string{ + core.MetaKeyOrigin: target, + metaKeyGithubId: parseId(issue.Id), + metaKeyGithubUrl: issue.Url.String(), + }) + if err != nil { + return nil, err } + // importing a new bug + gi.out <- core.NewImportBug(b.Id()) return b, nil } -func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.BugCache, item timelineItem) error { +func (gi *githubImporter) ensureIssueEdit(ctx context.Context, repo *cache.RepoCache, bug *cache.BugCache, ghIssueId githubv4.ID, edit *userContentEdit) error { + return gi.ensureCommentEdit(ctx, repo, bug, ghIssueId, edit) +} + +func (gi *githubImporter) ensureTimelineItem(ctx context.Context, repo *cache.RepoCache, b *cache.BugCache, item *timelineItem) error { switch item.Typename { case "IssueComment": - // collect all comment edits - var commentEdits []userContentEdit - for gi.iterator.NextCommentEdit() { - commentEdits = append(commentEdits, gi.iterator.CommentEditValue()) - } - - // ensureTimelineComment send import events over out chanel - err := gi.ensureTimelineComment(repo, b, item.IssueComment, commentEdits) + err := gi.ensureComment(ctx, repo, b, &item.IssueComment, nil) if err != nil { return fmt.Errorf("timeline comment creation: %v", err) } @@ -234,7 +257,7 @@ func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.Bug if err != cache.ErrNoMatchingOp { return err } - author, err := gi.ensurePerson(repo, item.LabeledEvent.Actor) + author, err := gi.ensurePerson(ctx, repo, item.LabeledEvent.Actor) if err != nil { return err } @@ -242,7 +265,7 @@ func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.Bug author, item.LabeledEvent.CreatedAt.Unix(), []string{ - string(item.LabeledEvent.Label.Name), + text.CleanupOneLine(string(item.LabeledEvent.Label.Name)), }, nil, map[string]string{metaKeyGithubId: id}, @@ -263,7 +286,7 @@ func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.Bug if err != cache.ErrNoMatchingOp { return err } - author, err := gi.ensurePerson(repo, item.UnlabeledEvent.Actor) + author, err := gi.ensurePerson(ctx, repo, item.UnlabeledEvent.Actor) if err != nil { return err } @@ -273,7 +296,7 @@ func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.Bug item.UnlabeledEvent.CreatedAt.Unix(), nil, []string{ - string(item.UnlabeledEvent.Label.Name), + text.CleanupOneLine(string(item.UnlabeledEvent.Label.Name)), }, map[string]string{metaKeyGithubId: id}, ) @@ -293,7 +316,7 @@ func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.Bug if err == nil { return nil } - author, err := gi.ensurePerson(repo, item.ClosedEvent.Actor) + author, err := gi.ensurePerson(ctx, repo, item.ClosedEvent.Actor) if err != nil { return err } @@ -319,7 +342,7 @@ func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.Bug if err == nil { return nil } - author, err := gi.ensurePerson(repo, item.ReopenedEvent.Actor) + author, err := gi.ensurePerson(ctx, repo, item.ReopenedEvent.Actor) if err != nil { return err } @@ -345,14 +368,25 @@ func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.Bug if err == nil { return nil } - author, err := gi.ensurePerson(repo, item.RenamedTitleEvent.Actor) + author, err := gi.ensurePerson(ctx, repo, item.RenamedTitleEvent.Actor) if err != nil { return err } + + // At Github there exist issues with seemingly empty titles. An example is + // https://github.com/NixOS/nixpkgs/issues/72730 . + // The title provided by the GraphQL API actually consists of a space followed + // by a zero width space (U+200B). This title would cause the NewBugRaw() + // function to return an error: empty title. + title := text.CleanupOneLine(string(item.RenamedTitleEvent.CurrentTitle)) + if title == " \u200b" { // U+200B == zero width space + title = EmptyTitlePlaceholder + } + op, err := b.SetTitleRaw( author, item.RenamedTitleEvent.CreatedAt.Unix(), - string(item.RenamedTitleEvent.CurrentTitle), + title, map[string]string{metaKeyGithubId: id}, ) if err != nil { @@ -366,97 +400,57 @@ func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.Bug return nil } -func (gi *githubImporter) ensureTimelineComment(repo *cache.RepoCache, b *cache.BugCache, item issueComment, edits []userContentEdit) error { - // ensure person - author, err := gi.ensurePerson(repo, item.Author) +func (gi *githubImporter) ensureCommentEdit(ctx context.Context, repo *cache.RepoCache, b *cache.BugCache, ghTargetId githubv4.ID, edit *userContentEdit) error { + // find comment + target, err := b.ResolveOperationWithMetadata(metaKeyGithubId, parseId(ghTargetId)) if err != nil { return err } - - targetOpID, err := b.ResolveOperationWithMetadata(metaKeyGithubId, parseId(item.Id)) - if err != nil && err != cache.ErrNoMatchingOp { + _, err = b.ResolveOperationWithMetadata(metaKeyGithubId, parseId(edit.Id)) + if err == nil { + return nil + } + if err != cache.ErrNoMatchingOp { // real error return err } - // if no edits are given we create the comment - if len(edits) == 0 { - if err == cache.ErrNoMatchingOp { - cleanText, err := text.Cleanup(string(item.Body)) - if err != nil { - return err - } - - // add comment operation - op, err := b.AddCommentRaw( - author, - item.CreatedAt.Unix(), - cleanText, - nil, - map[string]string{ - metaKeyGithubId: parseId(item.Id), - metaKeyGithubUrl: parseId(item.Url.String()), - }, - ) - if err != nil { - return err - } - - gi.out <- core.NewImportComment(op.Id()) - return nil - } - - } else { - for i, edit := range edits { - if i == 0 && targetOpID != "" { - // The first edit in the github result is the comment creation itself, we already have that - continue - } - - // ensure editor identity - editor, err := gi.ensurePerson(repo, edit.Editor) - if err != nil { - return err - } - - // create comment when target is empty - if targetOpID == "" { - cleanText, err := text.Cleanup(string(*edit.Diff)) - if err != nil { - return err - } + editor, err := gi.ensurePerson(ctx, repo, edit.Editor) + if err != nil { + return err + } - op, err := b.AddCommentRaw( - editor, - edit.CreatedAt.Unix(), - cleanText, - nil, - map[string]string{ - metaKeyGithubId: parseId(item.Id), - metaKeyGithubUrl: item.Url.String(), - }, - ) - if err != nil { - return err - } - gi.out <- core.NewImportComment(op.Id()) + if edit.DeletedAt != nil { + // comment deletion, not supported yet + return nil + } - // set target for the next edit now that the comment is created - targetOpID = op.Id() - continue - } + // comment edition + op, err := b.EditCommentRaw( + editor, + edit.CreatedAt.Unix(), + target, + text.Cleanup(string(*edit.Diff)), + map[string]string{ + metaKeyGithubId: parseId(edit.Id), + }, + ) - err = gi.ensureCommentEdit(repo, b, targetOpID, edit) - if err != nil { - return err - } - } + if err != nil { + return err } + + gi.out <- core.NewImportCommentEdition(op.Id()) return nil } -func (gi *githubImporter) ensureCommentEdit(repo *cache.RepoCache, b *cache.BugCache, target entity.Id, edit userContentEdit) error { - _, err := b.ResolveOperationWithMetadata(metaKeyGithubId, parseId(edit.Id)) +func (gi *githubImporter) ensureComment(ctx context.Context, repo *cache.RepoCache, b *cache.BugCache, comment *issueComment, firstEdit *userContentEdit) error { + author, err := gi.ensurePerson(ctx, repo, comment.Author) + if err != nil { + return err + } + + _, err = b.ResolveOperationWithMetadata(metaKeyGithubId, parseId(comment.Id)) if err == nil { return nil } @@ -465,50 +459,40 @@ func (gi *githubImporter) ensureCommentEdit(repo *cache.RepoCache, b *cache.BugC return err } - editor, err := gi.ensurePerson(repo, edit.Editor) + var textInput string + if firstEdit != nil { + // use the first comment edit: it represents the comment creation itself + textInput = string(*firstEdit.Diff) + } else { + // if there are not comment edits, then the comment struct holds the comment creation + textInput = string(comment.Body) + } + + // add comment operation + op, err := b.AddCommentRaw( + author, + comment.CreatedAt.Unix(), + text.Cleanup(textInput), + nil, + map[string]string{ + metaKeyGithubId: parseId(comment.Id), + metaKeyGithubUrl: comment.Url.String(), + }, + ) if err != nil { return err } - switch { - case edit.DeletedAt != nil: - // comment deletion, not supported yet - return nil - - case edit.DeletedAt == nil: - - cleanText, err := text.Cleanup(string(*edit.Diff)) - if err != nil { - return err - } - - // comment edition - op, err := b.EditCommentRaw( - editor, - edit.CreatedAt.Unix(), - target, - cleanText, - map[string]string{ - metaKeyGithubId: parseId(edit.Id), - }, - ) - - if err != nil { - return err - } - - gi.out <- core.NewImportCommentEdition(op.Id()) - return nil - } + gi.out <- core.NewImportComment(op.Id()) return nil } // ensurePerson create a bug.Person from the Github data -func (gi *githubImporter) ensurePerson(repo *cache.RepoCache, actor *actor) (*cache.IdentityCache, error) { +func (gi *githubImporter) ensurePerson(ctx context.Context, repo *cache.RepoCache, actor *actor) (*cache.IdentityCache, error) { // When a user has been deleted, Github return a null actor, while displaying a profile named "ghost" // in it's UI. So we need a special case to get it. if actor == nil { - return gi.getGhost(repo) + return gi.getGhost(ctx, repo) } // Look first in the cache @@ -521,7 +505,6 @@ func (gi *githubImporter) ensurePerson(repo *cache.RepoCache, actor *actor) (*ca } // importing a new identity - var name string var email string @@ -565,48 +548,37 @@ func (gi *githubImporter) ensurePerson(repo *cache.RepoCache, actor *actor) (*ca return i, nil } -func (gi *githubImporter) getGhost(repo *cache.RepoCache) (*cache.IdentityCache, error) { +func (gi *githubImporter) getGhost(ctx context.Context, repo *cache.RepoCache) (*cache.IdentityCache, error) { + loginName := "ghost" // Look first in the cache - i, err := repo.ResolveIdentityImmutableMetadata(metaKeyGithubLogin, "ghost") + i, err := repo.ResolveIdentityImmutableMetadata(metaKeyGithubLogin, loginName) if err == nil { return i, nil } if entity.IsErrMultipleMatch(err) { return nil, err } - - var q ghostQuery - - variables := map[string]interface{}{ - "login": githubv4.String("ghost"), - } - - ctx, cancel := context.WithTimeout(gi.iterator.ctx, defaultTimeout) - defer cancel() - - err = gi.client.Query(ctx, &q, variables) + user, err := gi.mediator.User(ctx, loginName) if err != nil { return nil, err } - - var name string - if q.User.Name != nil { - name = string(*q.User.Name) + userName := "" + if user.Name != nil { + userName = string(*user.Name) } - return repo.NewIdentityRaw( - name, + userName, "", - string(q.User.Login), - string(q.User.AvatarUrl), + string(user.Login), + string(user.AvatarUrl), nil, map[string]string{ - metaKeyGithubLogin: string(q.User.Login), + metaKeyGithubLogin: string(user.Login), }, ) } -// parseId convert the unusable githubv4.ID (an interface{}) into a string +// parseId converts the unusable githubv4.ID (an interface{}) into a string func parseId(id githubv4.ID) string { return fmt.Sprintf("%v", id) } diff --git a/bridge/github/import_mediator.go b/bridge/github/import_mediator.go new file mode 100644 index 00000000..873d5f62 --- /dev/null +++ b/bridge/github/import_mediator.go @@ -0,0 +1,436 @@ +package github + +import ( + "context" + "fmt" + "strings" + "time" + + "github.com/shurcooL/githubv4" +) + +const ( + // These values influence how fast the github graphql rate limit is exhausted. + NumIssues = 40 + NumIssueEdits = 100 + NumTimelineItems = 100 + NumCommentEdits = 100 + + ChanCapacity = 128 +) + +// importMediator provides a convenient interface to retrieve issues from the Github GraphQL API. +type importMediator struct { + // Github graphql client + gc *githubv4.Client + + // name of the repository owner on Github + owner string + + // name of the Github repository + project string + + // since specifies which issues to import. Issues that have been updated at or after the + // given date should be imported. + since time.Time + + // importEvents holds events representing issues, comments, edits, ... + // In this channel issues are immediately followed by their issue edits and comments are + // immediately followed by their comment edits. + importEvents chan ImportEvent + + // Sticky error + err error +} + +type ImportEvent interface { + isImportEvent() +} + +type RateLimitingEvent struct { + msg string +} + +func (RateLimitingEvent) isImportEvent() {} + +type IssueEvent struct { + issue +} + +func (IssueEvent) isImportEvent() {} + +type IssueEditEvent struct { + issueId githubv4.ID + userContentEdit +} + +func (IssueEditEvent) isImportEvent() {} + +type TimelineEvent struct { + issueId githubv4.ID + timelineItem +} + +func (TimelineEvent) isImportEvent() {} + +type CommentEditEvent struct { + commentId githubv4.ID + userContentEdit +} + +func (CommentEditEvent) isImportEvent() {} + +func (mm *importMediator) NextImportEvent() ImportEvent { + return <-mm.importEvents +} + +func NewImportMediator(ctx context.Context, client *githubv4.Client, owner, project string, since time.Time) *importMediator { + mm := importMediator{ + gc: client, + owner: owner, + project: project, + since: since, + importEvents: make(chan ImportEvent, ChanCapacity), + err: nil, + } + go func() { + mm.fillImportEvents(ctx) + close(mm.importEvents) + }() + return &mm +} + +type varmap map[string]interface{} + +func newIssueVars(owner, project string, since time.Time) varmap { + return varmap{ + "owner": githubv4.String(owner), + "name": githubv4.String(project), + "issueSince": githubv4.DateTime{Time: since}, + "issueFirst": githubv4.Int(NumIssues), + "issueEditLast": githubv4.Int(NumIssueEdits), + "issueEditBefore": (*githubv4.String)(nil), + "timelineFirst": githubv4.Int(NumTimelineItems), + "timelineAfter": (*githubv4.String)(nil), + "commentEditLast": githubv4.Int(NumCommentEdits), + "commentEditBefore": (*githubv4.String)(nil), + } +} + +func newIssueEditVars() varmap { + return varmap{ + "issueEditLast": githubv4.Int(NumIssueEdits), + } +} + +func newTimelineVars() varmap { + return varmap{ + "timelineFirst": githubv4.Int(NumTimelineItems), + "commentEditLast": githubv4.Int(NumCommentEdits), + "commentEditBefore": (*githubv4.String)(nil), + } +} + +func newCommentEditVars() varmap { + return varmap{ + "commentEditLast": githubv4.Int(NumCommentEdits), + } +} + +func (mm *importMediator) Error() error { + return mm.err +} + +func (mm *importMediator) User(ctx context.Context, loginName string) (*user, error) { + query := userQuery{} + vars := varmap{"login": githubv4.String(loginName)} + if err := mm.mQuery(ctx, &query, vars); err != nil { + return nil, err + } + return &query.User, nil +} + +func (mm *importMediator) fillImportEvents(ctx context.Context) { + initialCursor := githubv4.String("") + issues, hasIssues := mm.queryIssue(ctx, initialCursor) + for hasIssues { + for _, node := range issues.Nodes { + select { + case <-ctx.Done(): + return + case mm.importEvents <- IssueEvent{node.issue}: + } + + // issue edit events follow the issue event + mm.fillIssueEditEvents(ctx, &node) + // last come the timeline events + mm.fillTimelineEvents(ctx, &node) + } + if !issues.PageInfo.HasNextPage { + break + } + issues, hasIssues = mm.queryIssue(ctx, issues.PageInfo.EndCursor) + } +} + +func (mm *importMediator) fillIssueEditEvents(ctx context.Context, issueNode *issueNode) { + edits := &issueNode.UserContentEdits + hasEdits := true + for hasEdits { + for edit := range reverse(edits.Nodes) { + if edit.Diff == nil || string(*edit.Diff) == "" { + // issueEdit.Diff == nil happen if the event is older than early + // 2018, Github doesn't have the data before that. Best we can do is + // to ignore the event. + continue + } + select { + case <-ctx.Done(): + return + case mm.importEvents <- IssueEditEvent{issueId: issueNode.issue.Id, userContentEdit: edit}: + } + } + if !edits.PageInfo.HasPreviousPage { + break + } + edits, hasEdits = mm.queryIssueEdits(ctx, issueNode.issue.Id, edits.PageInfo.EndCursor) + } +} + +func (mm *importMediator) queryIssueEdits(ctx context.Context, nid githubv4.ID, cursor githubv4.String) (*userContentEditConnection, bool) { + vars := newIssueEditVars() + vars["gqlNodeId"] = nid + if cursor == "" { + vars["issueEditBefore"] = (*githubv4.String)(nil) + } else { + vars["issueEditBefore"] = cursor + } + query := issueEditQuery{} + if err := mm.mQuery(ctx, &query, vars); err != nil { + mm.err = err + return nil, false + } + connection := &query.Node.Issue.UserContentEdits + if len(connection.Nodes) <= 0 { + return nil, false + } + return connection, true +} + +func (mm *importMediator) fillTimelineEvents(ctx context.Context, issueNode *issueNode) { + items := &issueNode.TimelineItems + hasItems := true + for hasItems { + for _, item := range items.Nodes { + select { + case <-ctx.Done(): + return + case mm.importEvents <- TimelineEvent{issueId: issueNode.issue.Id, timelineItem: item}: + } + if item.Typename == "IssueComment" { + // Issue comments are different than other timeline items in that + // they may have associated user content edits. + // Right after the comment we send the comment edits. + mm.fillCommentEdits(ctx, &item) + } + } + if !items.PageInfo.HasNextPage { + break + } + items, hasItems = mm.queryTimeline(ctx, issueNode.issue.Id, items.PageInfo.EndCursor) + } +} + +func (mm *importMediator) queryTimeline(ctx context.Context, nid githubv4.ID, cursor githubv4.String) (*timelineItemsConnection, bool) { + vars := newTimelineVars() + vars["gqlNodeId"] = nid + if cursor == "" { + vars["timelineAfter"] = (*githubv4.String)(nil) + } else { + vars["timelineAfter"] = cursor + } + query := timelineQuery{} + if err := mm.mQuery(ctx, &query, vars); err != nil { + mm.err = err + return nil, false + } + connection := &query.Node.Issue.TimelineItems + if len(connection.Nodes) <= 0 { + return nil, false + } + return connection, true +} + +func (mm *importMediator) fillCommentEdits(ctx context.Context, item *timelineItem) { + // Here we are only concerned with timeline items of type issueComment. + if item.Typename != "IssueComment" { + return + } + // First: setup message handling while submitting GraphQL queries. + comment := &item.IssueComment + edits := &comment.UserContentEdits + hasEdits := true + for hasEdits { + for edit := range reverse(edits.Nodes) { + if edit.Diff == nil || string(*edit.Diff) == "" { + // issueEdit.Diff == nil happen if the event is older than early + // 2018, Github doesn't have the data before that. Best we can do is + // to ignore the event. + continue + } + select { + case <-ctx.Done(): + return + case mm.importEvents <- CommentEditEvent{commentId: comment.Id, userContentEdit: edit}: + } + } + if !edits.PageInfo.HasPreviousPage { + break + } + edits, hasEdits = mm.queryCommentEdits(ctx, comment.Id, edits.PageInfo.EndCursor) + } +} + +func (mm *importMediator) queryCommentEdits(ctx context.Context, nid githubv4.ID, cursor githubv4.String) (*userContentEditConnection, bool) { + vars := newCommentEditVars() + vars["gqlNodeId"] = nid + if cursor == "" { + vars["commentEditBefore"] = (*githubv4.String)(nil) + } else { + vars["commentEditBefore"] = cursor + } + query := commentEditQuery{} + if err := mm.mQuery(ctx, &query, vars); err != nil { + mm.err = err + return nil, false + } + connection := &query.Node.IssueComment.UserContentEdits + if len(connection.Nodes) <= 0 { + return nil, false + } + return connection, true +} + +func (mm *importMediator) queryIssue(ctx context.Context, cursor githubv4.String) (*issueConnection, bool) { + vars := newIssueVars(mm.owner, mm.project, mm.since) + if cursor == "" { + vars["issueAfter"] = (*githubv4.String)(nil) + } else { + vars["issueAfter"] = cursor + } + query := issueQuery{} + if err := mm.mQuery(ctx, &query, vars); err != nil { + mm.err = err + return nil, false + } + connection := &query.Repository.Issues + if len(connection.Nodes) <= 0 { + return nil, false + } + return connection, true +} + +func reverse(eds []userContentEdit) chan userContentEdit { + ret := make(chan userContentEdit) + go func() { + for i := range eds { + ret <- eds[len(eds)-1-i] + } + close(ret) + }() + return ret +} + +// mQuery executes a single GraphQL query. The variable query is used to derive the GraphQL query +// and it is used to populate the response into it. It should be a pointer to a struct that +// corresponds to the Github graphql schema and it has to implement the rateLimiter interface. If +// there is a Github rate limiting error, then the function sleeps and retries after the rate limit +// is expired. If there is another error, then the method will retry before giving up. +func (mm *importMediator) mQuery(ctx context.Context, query rateLimiter, vars map[string]interface{}) error { + if err := mm.queryOnce(ctx, query, vars); err == nil { + // success: done + return nil + } + // failure: we will retry + // To retry is important for importing projects with a big number of issues, because + // there may be temporary network errors or momentary internal errors of the github servers. + retries := 3 + var err error + for i := 0; i < retries; i++ { + // wait a few seconds before retry + sleepTime := time.Duration(8*(i+1)) * time.Second + timer := time.NewTimer(sleepTime) + select { + case <-ctx.Done(): + stop(timer) + return ctx.Err() + case <-timer.C: + } + err = mm.queryOnce(ctx, query, vars) + if err == nil { + // success: done + return nil + } + } + return err +} + +func (mm *importMediator) queryOnce(ctx context.Context, query rateLimiter, vars map[string]interface{}) error { + // first: just send the query to the graphql api + vars["dryRun"] = githubv4.Boolean(false) + qctx, cancel := context.WithTimeout(ctx, defaultTimeout) + defer cancel() + err := mm.gc.Query(qctx, query, vars) + if err == nil { + // no error: done + return nil + } + // matching the error string + if !strings.Contains(err.Error(), "API rate limit exceeded") { + // an error, but not the API rate limit error: done + return err + } + // a rate limit error + // ask the graphql api for rate limiting information + vars["dryRun"] = githubv4.Boolean(true) + qctx, cancel = context.WithTimeout(ctx, defaultTimeout) + defer cancel() + if err := mm.gc.Query(qctx, query, vars); err != nil { + return err + } + rateLimit := query.rateLimit() + if rateLimit.Cost > rateLimit.Remaining { + // sleep + resetTime := rateLimit.ResetAt.Time + // Add a few seconds (8) for good measure + resetTime = resetTime.Add(8 * time.Second) + msg := fmt.Sprintf("Github GraphQL API: import will sleep until %s", resetTime.String()) + select { + case <-ctx.Done(): + return ctx.Err() + case mm.importEvents <- RateLimitingEvent{msg}: + } + timer := time.NewTimer(time.Until(resetTime)) + select { + case <-ctx.Done(): + stop(timer) + return ctx.Err() + case <-timer.C: + } + } + // run the original query again + vars["dryRun"] = githubv4.Boolean(false) + qctx, cancel = context.WithTimeout(ctx, defaultTimeout) + defer cancel() + err = mm.gc.Query(qctx, query, vars) + return err // might be nil +} + +func stop(t *time.Timer) { + if !t.Stop() { + select { + case <-t.C: + default: + } + } +} diff --git a/bridge/github/import_query.go b/bridge/github/import_query.go index 228d204a..461daf94 100644 --- a/bridge/github/import_query.go +++ b/bridge/github/import_query.go @@ -2,37 +2,136 @@ package github import "github.com/shurcooL/githubv4" -type pageInfo struct { - EndCursor githubv4.String - HasNextPage bool - StartCursor githubv4.String - HasPreviousPage bool +type rateLimit struct { + Cost githubv4.Int + Limit githubv4.Int + NodeCount githubv4.Int + Remaining githubv4.Int + ResetAt githubv4.DateTime + Used githubv4.Int } -type actor struct { - Typename githubv4.String `graphql:"__typename"` +type rateLimiter interface { + rateLimit() rateLimit +} + +type userQuery struct { + RateLimit rateLimit `graphql:"rateLimit(dryRun: $dryRun)"` + User user `graphql:"user(login: $login)"` +} + +func (q *userQuery) rateLimit() rateLimit { + return q.RateLimit +} + +type labelsQuery struct { + //RateLimit rateLimit `graphql:"rateLimit(dryRun: $dryRun)"` + Repository struct { + Labels struct { + Nodes []struct { + ID string `graphql:"id"` + Name string `graphql:"name"` + Color string `graphql:"color"` + Description string `graphql:"description"` + } + PageInfo pageInfo + } `graphql:"labels(first: $first, after: $after)"` + } `graphql:"repository(owner: $owner, name: $name)"` +} + +type loginQuery struct { + //RateLimit rateLimit `graphql:"rateLimit(dryRun: $dryRun)"` + Viewer struct { + Login string `graphql:"login"` + } `graphql:"viewer"` +} + +type issueQuery struct { + RateLimit rateLimit `graphql:"rateLimit(dryRun: $dryRun)"` + Repository struct { + Issues issueConnection `graphql:"issues(first: $issueFirst, after: $issueAfter, orderBy: {field: CREATED_AT, direction: ASC}, filterBy: {since: $issueSince})"` + } `graphql:"repository(owner: $owner, name: $name)"` +} + +func (q *issueQuery) rateLimit() rateLimit { + return q.RateLimit +} + +type issueEditQuery struct { + RateLimit rateLimit `graphql:"rateLimit(dryRun: $dryRun)"` + Node struct { + Typename githubv4.String `graphql:"__typename"` + Issue struct { + UserContentEdits userContentEditConnection `graphql:"userContentEdits(last: $issueEditLast, before: $issueEditBefore)"` + } `graphql:"... on Issue"` + } `graphql:"node(id: $gqlNodeId)"` +} + +func (q *issueEditQuery) rateLimit() rateLimit { + return q.RateLimit +} + +type timelineQuery struct { + RateLimit rateLimit `graphql:"rateLimit(dryRun: $dryRun)"` + Node struct { + Typename githubv4.String `graphql:"__typename"` + Issue struct { + TimelineItems timelineItemsConnection `graphql:"timelineItems(first: $timelineFirst, after: $timelineAfter)"` + } `graphql:"... on Issue"` + } `graphql:"node(id: $gqlNodeId)"` +} + +func (q *timelineQuery) rateLimit() rateLimit { + return q.RateLimit +} + +type commentEditQuery struct { + RateLimit rateLimit `graphql:"rateLimit(dryRun: $dryRun)"` + Node struct { + Typename githubv4.String `graphql:"__typename"` + IssueComment struct { + UserContentEdits userContentEditConnection `graphql:"userContentEdits(last: $commentEditLast, before: $commentEditBefore)"` + } `graphql:"... on IssueComment"` + } `graphql:"node(id: $gqlNodeId)"` +} + +func (q *commentEditQuery) rateLimit() rateLimit { + return q.RateLimit +} + +type user struct { Login githubv4.String AvatarUrl githubv4.String - User struct { - Name *githubv4.String - Email githubv4.String - } `graphql:"... on User"` - Organization struct { - Name *githubv4.String - Email *githubv4.String - } `graphql:"... on Organization"` + Name *githubv4.String } -type actorEvent struct { - Id githubv4.ID - CreatedAt githubv4.DateTime - Actor *actor +type issueConnection struct { + Nodes []issueNode + PageInfo pageInfo } -type authorEvent struct { - Id githubv4.ID - CreatedAt githubv4.DateTime - Author *actor +type issueNode struct { + issue + UserContentEdits userContentEditConnection `graphql:"userContentEdits(last: $issueEditLast, before: $issueEditBefore)"` + TimelineItems timelineItemsConnection `graphql:"timelineItems(first: $timelineFirst, after: $timelineAfter)"` +} + +type issue struct { + authorEvent + Title githubv4.String + Number githubv4.Int + Body githubv4.String + Url githubv4.URI +} + +type timelineItemsConnection struct { + Nodes []timelineItem + PageInfo pageInfo +} + +type userContentEditConnection struct { + Nodes []userContentEdit + PageInfo pageInfo } type userContentEdit struct { @@ -46,12 +145,6 @@ type userContentEdit struct { Diff *githubv4.String } -type issueComment struct { - authorEvent // NOTE: contains Id - Body githubv4.String - Url githubv4.URI -} - type timelineItem struct { Typename githubv4.String `graphql:"__typename"` @@ -91,84 +184,43 @@ type timelineItem struct { } `graphql:"... on RenamedTitleEvent"` } -type ghostQuery struct { - User struct { - Login githubv4.String - AvatarUrl githubv4.String - Name *githubv4.String - } `graphql:"user(login: $login)"` -} - -type labelsQuery struct { - Repository struct { - Labels struct { - Nodes []struct { - ID string `graphql:"id"` - Name string `graphql:"name"` - Color string `graphql:"color"` - Description string `graphql:"description"` - } - PageInfo pageInfo - } `graphql:"labels(first: $first, after: $after)"` - } `graphql:"repository(owner: $owner, name: $name)"` -} - -type loginQuery struct { - Viewer struct { - Login string `graphql:"login"` - } `graphql:"viewer"` -} +type issueComment struct { + authorEvent // NOTE: contains Id + Body githubv4.String + Url githubv4.URI -type issueQuery struct { - Repository struct { - Issues struct { - Nodes []issue - PageInfo pageInfo - } `graphql:"issues(first: $issueFirst, after: $issueAfter, orderBy: {field: CREATED_AT, direction: ASC}, filterBy: {since: $issueSince})"` - } `graphql:"repository(owner: $owner, name: $name)"` + UserContentEdits userContentEditConnection `graphql:"userContentEdits(last: $commentEditLast, before: $commentEditBefore)"` } -type issue struct { - authorEvent - Title string - Number githubv4.Int - Body githubv4.String - Url githubv4.URI +type actor struct { + Typename githubv4.String `graphql:"__typename"` + Login githubv4.String + AvatarUrl githubv4.String + User struct { + Name *githubv4.String + Email githubv4.String + } `graphql:"... on User"` + Organization struct { + Name *githubv4.String + Email *githubv4.String + } `graphql:"... on Organization"` } -type issueEditQuery struct { - Node struct { - Typename githubv4.String `graphql:"__typename"` - Issue struct { - UserContentEdits struct { - Nodes []userContentEdit - TotalCount githubv4.Int - PageInfo pageInfo - } `graphql:"userContentEdits(last: $issueEditLast, before: $issueEditBefore)"` - } `graphql:"... on Issue"` - } `graphql:"node(id: $gqlNodeId)"` +type actorEvent struct { + Id githubv4.ID + CreatedAt githubv4.DateTime + Actor *actor } -type timelineQuery struct { - Node struct { - Typename githubv4.String `graphql:"__typename"` - Issue struct { - TimelineItems struct { - Nodes []timelineItem - PageInfo pageInfo - } `graphql:"timelineItems(first: $timelineFirst, after: $timelineAfter)"` - } `graphql:"... on Issue"` - } `graphql:"node(id: $gqlNodeId)"` +type authorEvent struct { + Id githubv4.ID + CreatedAt githubv4.DateTime + Author *actor } -type commentEditQuery struct { - Node struct { - Typename githubv4.String `graphql:"__typename"` - IssueComment struct { - UserContentEdits struct { - Nodes []userContentEdit - PageInfo pageInfo - } `graphql:"userContentEdits(last: $commentEditLast, before: $commentEditBefore)"` - } `graphql:"... on IssueComment"` - } `graphql:"node(id: $gqlNodeId)"` +type pageInfo struct { + EndCursor githubv4.String + HasNextPage bool + StartCursor githubv4.String + HasPreviousPage bool } diff --git a/bridge/github/iterator.go b/bridge/github/iterator.go deleted file mode 100644 index d21faae8..00000000 --- a/bridge/github/iterator.go +++ /dev/null @@ -1,423 +0,0 @@ -package github - -import ( - "context" - "time" - - "github.com/pkg/errors" - "github.com/shurcooL/githubv4" -) - -type iterator struct { - // Github graphql client - gc *githubv4.Client - - // The iterator will only query issues updated or created after the date given in - // the variable since. - since time.Time - - // Shared context, which is used for all graphql queries. - ctx context.Context - - // Sticky error - err error - - // Issue iterator - issueIter issueIter -} - -type issueIter struct { - iterVars - query issueQuery - issueEditIter []issueEditIter - timelineIter []timelineIter -} - -type issueEditIter struct { - iterVars - query issueEditQuery -} - -type timelineIter struct { - iterVars - query timelineQuery - commentEditIter []commentEditIter -} - -type commentEditIter struct { - iterVars - query commentEditQuery -} - -type iterVars struct { - // Iterator index - index int - - // capacity is the number of elements (issues, issue edits, timeline items, or - // comment edits) to query at a time. More capacity = more used memory = - // less queries to make. - capacity int - - // Variable assignments for graphql query - variables varmap -} - -type varmap map[string]interface{} - -func newIterVars(capacity int) iterVars { - return iterVars{ - index: -1, - capacity: capacity, - variables: varmap{}, - } -} - -// NewIterator creates and initialize a new iterator. -func NewIterator(ctx context.Context, client *githubv4.Client, capacity int, owner, project string, since time.Time) *iterator { - i := &iterator{ - gc: client, - since: since, - ctx: ctx, - issueIter: issueIter{ - iterVars: newIterVars(capacity), - timelineIter: make([]timelineIter, capacity), - issueEditIter: make([]issueEditIter, capacity), - }, - } - i.issueIter.variables.setOwnerProject(owner, project) - for idx := range i.issueIter.issueEditIter { - ie := &i.issueIter.issueEditIter[idx] - ie.iterVars = newIterVars(capacity) - } - for i1 := range i.issueIter.timelineIter { - tli := &i.issueIter.timelineIter[i1] - tli.iterVars = newIterVars(capacity) - tli.commentEditIter = make([]commentEditIter, capacity) - for i2 := range tli.commentEditIter { - cei := &tli.commentEditIter[i2] - cei.iterVars = newIterVars(capacity) - } - } - i.resetIssueVars() - return i -} - -func (v *varmap) setOwnerProject(owner, project string) { - (*v)["owner"] = githubv4.String(owner) - (*v)["name"] = githubv4.String(project) -} - -func (i *iterator) resetIssueVars() { - vars := &i.issueIter.variables - (*vars)["issueFirst"] = githubv4.Int(i.issueIter.capacity) - (*vars)["issueAfter"] = (*githubv4.String)(nil) - (*vars)["issueSince"] = githubv4.DateTime{Time: i.since} - i.issueIter.query.Repository.Issues.PageInfo.HasNextPage = true - i.issueIter.query.Repository.Issues.PageInfo.EndCursor = "" -} - -func (i *iterator) resetIssueEditVars() { - for idx := range i.issueIter.issueEditIter { - ie := &i.issueIter.issueEditIter[idx] - ie.variables["issueEditLast"] = githubv4.Int(ie.capacity) - ie.variables["issueEditBefore"] = (*githubv4.String)(nil) - ie.query.Node.Issue.UserContentEdits.PageInfo.HasNextPage = true - ie.query.Node.Issue.UserContentEdits.PageInfo.EndCursor = "" - } -} - -func (i *iterator) resetTimelineVars() { - for idx := range i.issueIter.timelineIter { - ip := &i.issueIter.timelineIter[idx] - ip.variables["timelineFirst"] = githubv4.Int(ip.capacity) - ip.variables["timelineAfter"] = (*githubv4.String)(nil) - ip.query.Node.Issue.TimelineItems.PageInfo.HasNextPage = true - ip.query.Node.Issue.TimelineItems.PageInfo.EndCursor = "" - } -} - -func (i *iterator) resetCommentEditVars() { - for i1 := range i.issueIter.timelineIter { - for i2 := range i.issueIter.timelineIter[i1].commentEditIter { - ce := &i.issueIter.timelineIter[i1].commentEditIter[i2] - ce.variables["commentEditLast"] = githubv4.Int(ce.capacity) - ce.variables["commentEditBefore"] = (*githubv4.String)(nil) - ce.query.Node.IssueComment.UserContentEdits.PageInfo.HasNextPage = true - ce.query.Node.IssueComment.UserContentEdits.PageInfo.EndCursor = "" - } - } -} - -// Error return last encountered error -func (i *iterator) Error() error { - if i.err != nil { - return i.err - } - return i.ctx.Err() // might return nil -} - -func (i *iterator) HasError() bool { - return i.err != nil || i.ctx.Err() != nil -} - -func (i *iterator) currIssueItem() *issue { - return &i.issueIter.query.Repository.Issues.Nodes[i.issueIter.index] -} - -func (i *iterator) currIssueEditIter() *issueEditIter { - return &i.issueIter.issueEditIter[i.issueIter.index] -} - -func (i *iterator) currTimelineIter() *timelineIter { - return &i.issueIter.timelineIter[i.issueIter.index] -} - -func (i *iterator) currCommentEditIter() *commentEditIter { - timelineIter := i.currTimelineIter() - return &timelineIter.commentEditIter[timelineIter.index] -} - -func (i *iterator) currIssueGqlNodeId() githubv4.ID { - return i.currIssueItem().Id -} - -// NextIssue returns true if there exists a next issue and advances the iterator by one. -// It is used to iterate over all issues. Queries to github are made when necessary. -func (i *iterator) NextIssue() bool { - if i.HasError() { - return false - } - index := &i.issueIter.index - issues := &i.issueIter.query.Repository.Issues - issueItems := &issues.Nodes - if 0 <= *index && *index < len(*issueItems)-1 { - *index += 1 - return true - } - - if !issues.PageInfo.HasNextPage { - return false - } - nextIssue := i.queryIssue() - return nextIssue -} - -// IssueValue returns the actual issue value. -func (i *iterator) IssueValue() issue { - return *i.currIssueItem() -} - -func (i *iterator) queryIssue() bool { - ctx, cancel := context.WithTimeout(i.ctx, defaultTimeout) - defer cancel() - if endCursor := i.issueIter.query.Repository.Issues.PageInfo.EndCursor; endCursor != "" { - i.issueIter.variables["issueAfter"] = endCursor - } - if err := i.gc.Query(ctx, &i.issueIter.query, i.issueIter.variables); err != nil { - i.err = err - return false - } - i.resetIssueEditVars() - i.resetTimelineVars() - issueItems := &i.issueIter.query.Repository.Issues.Nodes - if len(*issueItems) <= 0 { - i.issueIter.index = -1 - return false - } - i.issueIter.index = 0 - return true -} - -// NextIssueEdit returns true if there exists a next issue edit and advances the iterator -// by one. It is used to iterate over all the issue edits. Queries to github are made when -// necessary. -func (i *iterator) NextIssueEdit() bool { - if i.HasError() { - return false - } - ieIter := i.currIssueEditIter() - ieIdx := &ieIter.index - ieItems := ieIter.query.Node.Issue.UserContentEdits - if 0 <= *ieIdx && *ieIdx < len(ieItems.Nodes)-1 { - *ieIdx += 1 - return i.nextValidIssueEdit() - } - if !ieItems.PageInfo.HasNextPage { - return false - } - querySucc := i.queryIssueEdit() - if !querySucc { - return false - } - return i.nextValidIssueEdit() -} - -func (i *iterator) nextValidIssueEdit() bool { - // issueEdit.Diff == nil happen if the event is older than early 2018, Github doesn't have - // the data before that. Best we can do is to ignore the event. - if issueEdit := i.IssueEditValue(); issueEdit.Diff == nil || string(*issueEdit.Diff) == "" { - return i.NextIssueEdit() - } - return true -} - -// IssueEditValue returns the actual issue edit value. -func (i *iterator) IssueEditValue() userContentEdit { - iei := i.currIssueEditIter() - return iei.query.Node.Issue.UserContentEdits.Nodes[iei.index] -} - -func (i *iterator) queryIssueEdit() bool { - ctx, cancel := context.WithTimeout(i.ctx, defaultTimeout) - defer cancel() - iei := i.currIssueEditIter() - if endCursor := iei.query.Node.Issue.UserContentEdits.PageInfo.EndCursor; endCursor != "" { - iei.variables["issueEditBefore"] = endCursor - } - iei.variables["gqlNodeId"] = i.currIssueGqlNodeId() - if err := i.gc.Query(ctx, &iei.query, iei.variables); err != nil { - i.err = err - return false - } - issueEditItems := iei.query.Node.Issue.UserContentEdits.Nodes - if len(issueEditItems) <= 0 { - iei.index = -1 - return false - } - // The UserContentEditConnection in the Github API serves its elements in reverse chronological - // order. For our purpose we have to reverse the edits. - reverseEdits(issueEditItems) - iei.index = 0 - return true -} - -// NextTimelineItem returns true if there exists a next timeline item and advances the iterator -// by one. It is used to iterate over all the timeline items. Queries to github are made when -// necessary. -func (i *iterator) NextTimelineItem() bool { - if i.HasError() { - return false - } - tlIter := &i.issueIter.timelineIter[i.issueIter.index] - tlIdx := &tlIter.index - tlItems := tlIter.query.Node.Issue.TimelineItems - if 0 <= *tlIdx && *tlIdx < len(tlItems.Nodes)-1 { - *tlIdx += 1 - return true - } - if !tlItems.PageInfo.HasNextPage { - return false - } - nextTlItem := i.queryTimeline() - return nextTlItem -} - -// TimelineItemValue returns the actual timeline item value. -func (i *iterator) TimelineItemValue() timelineItem { - tli := i.currTimelineIter() - return tli.query.Node.Issue.TimelineItems.Nodes[tli.index] -} - -func (i *iterator) queryTimeline() bool { - ctx, cancel := context.WithTimeout(i.ctx, defaultTimeout) - defer cancel() - tli := i.currTimelineIter() - if endCursor := tli.query.Node.Issue.TimelineItems.PageInfo.EndCursor; endCursor != "" { - tli.variables["timelineAfter"] = endCursor - } - tli.variables["gqlNodeId"] = i.currIssueGqlNodeId() - if err := i.gc.Query(ctx, &tli.query, tli.variables); err != nil { - i.err = err - return false - } - i.resetCommentEditVars() - timelineItems := &tli.query.Node.Issue.TimelineItems - if len(timelineItems.Nodes) <= 0 { - tli.index = -1 - return false - } - tli.index = 0 - return true -} - -// NextCommentEdit returns true if there exists a next comment edit and advances the iterator -// by one. It is used to iterate over all issue edits. Queries to github are made when -// necessary. -func (i *iterator) NextCommentEdit() bool { - if i.HasError() { - return false - } - - tmlnVal := i.TimelineItemValue() - if tmlnVal.Typename != "IssueComment" { - // The timeline iterator does not point to a comment. - i.err = errors.New("Call to NextCommentEdit() while timeline item is not a comment") - return false - } - - cei := i.currCommentEditIter() - ceIdx := &cei.index - ceItems := &cei.query.Node.IssueComment.UserContentEdits - if 0 <= *ceIdx && *ceIdx < len(ceItems.Nodes)-1 { - *ceIdx += 1 - return i.nextValidCommentEdit() - } - if !ceItems.PageInfo.HasNextPage { - return false - } - querySucc := i.queryCommentEdit() - if !querySucc { - return false - } - return i.nextValidCommentEdit() -} - -func (i *iterator) nextValidCommentEdit() bool { - // if comment edit diff is a nil pointer or points to an empty string look for next value - if commentEdit := i.CommentEditValue(); commentEdit.Diff == nil || string(*commentEdit.Diff) == "" { - return i.NextCommentEdit() - } - return true -} - -// CommentEditValue returns the actual comment edit value. -func (i *iterator) CommentEditValue() userContentEdit { - cei := i.currCommentEditIter() - return cei.query.Node.IssueComment.UserContentEdits.Nodes[cei.index] -} - -func (i *iterator) queryCommentEdit() bool { - ctx, cancel := context.WithTimeout(i.ctx, defaultTimeout) - defer cancel() - cei := i.currCommentEditIter() - - if endCursor := cei.query.Node.IssueComment.UserContentEdits.PageInfo.EndCursor; endCursor != "" { - cei.variables["commentEditBefore"] = endCursor - } - tmlnVal := i.TimelineItemValue() - if tmlnVal.Typename != "IssueComment" { - i.err = errors.New("Call to queryCommentEdit() while timeline item is not a comment") - return false - } - cei.variables["gqlNodeId"] = tmlnVal.IssueComment.Id - if err := i.gc.Query(ctx, &cei.query, cei.variables); err != nil { - i.err = err - return false - } - ceItems := cei.query.Node.IssueComment.UserContentEdits.Nodes - if len(ceItems) <= 0 { - cei.index = -1 - return false - } - // The UserContentEditConnection in the Github API serves its elements in reverse chronological - // order. For our purpose we have to reverse the edits. - reverseEdits(ceItems) - cei.index = 0 - return true -} - -func reverseEdits(edits []userContentEdit) { - for i, j := 0, len(edits)-1; i < j; i, j = i+1, j-1 { - edits[i], edits[j] = edits[j], edits[i] - } -} diff --git a/bridge/gitlab/import.go b/bridge/gitlab/import.go index 7939f4e4..cc99c12e 100644 --- a/bridge/gitlab/import.go +++ b/bridge/gitlab/import.go @@ -137,22 +137,12 @@ func (gi *gitlabImporter) ensureIssue(repo *cache.RepoCache, issue *gitlab.Issue return nil, err } - // if bug was never imported - cleanTitle, err := text.Cleanup(issue.Title) - if err != nil { - return nil, err - } - cleanDesc, err := text.Cleanup(issue.Description) - if err != nil { - return nil, err - } - - // create bug + // if bug was never imported, create bug b, _, err = repo.NewBugRaw( author, issue.CreatedAt.Unix(), - cleanTitle, - cleanDesc, + text.CleanupOneLine(issue.Title), + text.Cleanup(issue.Description), nil, map[string]string{ core.MetaKeyOrigin: target, @@ -238,7 +228,7 @@ func (gi *gitlabImporter) ensureNote(repo *cache.RepoCache, b *cache.BugCache, n author, note.UpdatedAt.Unix(), firstComment.Id(), - issue.Description, + text.Cleanup(issue.Description), map[string]string{ metaKeyGitlabId: gitlabID, }, @@ -251,10 +241,7 @@ func (gi *gitlabImporter) ensureNote(repo *cache.RepoCache, b *cache.BugCache, n } case NOTE_COMMENT: - cleanText, err := text.Cleanup(body) - if err != nil { - return err - } + cleanText := text.Cleanup(body) // if we didn't import the comment if errResolve == cache.ErrNoMatchingOp { @@ -312,7 +299,7 @@ func (gi *gitlabImporter) ensureNote(repo *cache.RepoCache, b *cache.BugCache, n op, err := b.SetTitleRaw( author, note.CreatedAt.Unix(), - body, + text.CleanupOneLine(body), map[string]string{ metaKeyGitlabId: gitlabID, }, @@ -361,7 +348,7 @@ func (gi *gitlabImporter) ensureLabelEvent(repo *cache.RepoCache, b *cache.BugCa _, err = b.ForceChangeLabelsRaw( author, labelEvent.CreatedAt.Unix(), - []string{labelEvent.Label.Name}, + []string{text.CleanupOneLine(labelEvent.Label.Name)}, nil, map[string]string{ metaKeyGitlabId: parseID(labelEvent.ID), @@ -373,7 +360,7 @@ func (gi *gitlabImporter) ensureLabelEvent(repo *cache.RepoCache, b *cache.BugCa author, labelEvent.CreatedAt.Unix(), nil, - []string{labelEvent.Label.Name}, + []string{text.CleanupOneLine(labelEvent.Label.Name)}, map[string]string{ metaKeyGitlabId: parseID(labelEvent.ID), }, diff --git a/bridge/jira/import.go b/bridge/jira/import.go index 00148bb6..d601febb 100644 --- a/bridge/jira/import.go +++ b/bridge/jira/import.go @@ -232,19 +232,11 @@ func (ji *jiraImporter) ensureIssue(repo *cache.RepoCache, issue Issue) (*cache. } if err == bug.ErrBugNotExist { - cleanText, err := text.Cleanup(string(issue.Fields.Description)) - if err != nil { - return nil, err - } - - // NOTE(josh): newlines in titles appears to be rare, but it has been seen - // in the wild. It does not appear to be allowed in the JIRA web interface. - title := strings.Replace(issue.Fields.Summary, "\n", "", -1) b, _, err = repo.NewBugRaw( author, issue.Fields.Created.Unix(), - title, - cleanText, + text.CleanupOneLine(issue.Fields.Summary), + text.Cleanup(issue.Fields.Description), nil, map[string]string{ core.MetaKeyOrigin: target, @@ -289,10 +281,7 @@ func (ji *jiraImporter) ensureComment(repo *cache.RepoCache, b *cache.BugCache, // We don't know the original text... we only have the updated text. cleanText = "" } else { - cleanText, err = text.Cleanup(string(item.Body)) - if err != nil { - return err - } + cleanText = text.Cleanup(item.Body) } // add comment operation @@ -340,15 +329,11 @@ func (ji *jiraImporter) ensureComment(repo *cache.RepoCache, b *cache.BugCache, } // comment edition - cleanText, err := text.Cleanup(string(item.Body)) - if err != nil { - return err - } op, err := b.EditCommentRaw( editor, item.Updated.Unix(), targetOpID, - cleanText, + text.Cleanup(item.Body), map[string]string{ metaKeyJiraId: derivedID, }, @@ -513,8 +498,8 @@ func (ji *jiraImporter) ensureChange(repo *cache.RepoCache, b *cache.BugCache, e op, err := b.ForceChangeLabelsRaw( author, entry.Created.Unix(), - addedLabels, - removedLabels, + text.CleanupOneLineArray(addedLabels), + text.CleanupOneLineArray(removedLabels), map[string]string{ metaKeyJiraId: entry.ID, metaKeyJiraDerivedId: derivedID, @@ -571,7 +556,7 @@ func (ji *jiraImporter) ensureChange(repo *cache.RepoCache, b *cache.BugCache, e op, err := b.SetTitleRaw( author, entry.Created.Unix(), - string(item.ToString), + text.CleanupOneLine(item.ToString), map[string]string{ metaKeyJiraId: entry.ID, metaKeyJiraDerivedId: derivedID, @@ -589,7 +574,7 @@ func (ji *jiraImporter) ensureChange(repo *cache.RepoCache, b *cache.BugCache, e op, err := b.EditCreateCommentRaw( author, entry.Created.Unix(), - string(item.ToString), + text.Cleanup(item.ToString), map[string]string{ metaKeyJiraId: entry.ID, metaKeyJiraDerivedId: derivedID, diff --git a/bridge/launchpad/import.go b/bridge/launchpad/import.go index 6b5667ba..01f6e53b 100644 --- a/bridge/launchpad/import.go +++ b/bridge/launchpad/import.go @@ -9,6 +9,7 @@ import ( "github.com/MichaelMure/git-bug/bug" "github.com/MichaelMure/git-bug/cache" "github.com/MichaelMure/git-bug/entity" + "github.com/MichaelMure/git-bug/util/text" ) type launchpadImporter struct { @@ -83,8 +84,8 @@ func (li *launchpadImporter) ImportAll(ctx context.Context, repo *cache.RepoCach b, _, err = repo.NewBugRaw( owner, createdAt.Unix(), - lpBug.Title, - lpBug.Description, + text.CleanupOneLine(lpBug.Title), + text.Cleanup(lpBug.Description), nil, map[string]string{ core.MetaKeyOrigin: target, @@ -133,7 +134,7 @@ func (li *launchpadImporter) ImportAll(ctx context.Context, repo *cache.RepoCach op, err := b.AddCommentRaw( owner, createdAt.Unix(), - lpMessage.Content, + text.Cleanup(lpMessage.Content), nil, map[string]string{ metaKeyLaunchpadID: lpMessage.ID, diff --git a/bug/label.go b/bug/label.go index 9d1e99a7..79b5f591 100644 --- a/bug/label.go +++ b/bug/label.go @@ -4,7 +4,6 @@ import ( "crypto/sha256" "fmt" "image/color" - "strings" fcolor "github.com/fatih/color" @@ -58,12 +57,8 @@ func (l Label) Validate() error { return fmt.Errorf("empty") } - if strings.Contains(str, "\n") { - return fmt.Errorf("should be a single line") - } - - if !text.Safe(str) { - return fmt.Errorf("not fully printable") + if !text.SafeOneLine(str) { + return fmt.Errorf("label has unsafe characters") } return nil diff --git a/bug/op_create.go b/bug/op_create.go index 75b60bd8..1d01020c 100644 --- a/bug/op_create.go +++ b/bug/op_create.go @@ -3,7 +3,6 @@ package bug import ( "encoding/json" "fmt" - "strings" "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/entity/dag" @@ -94,11 +93,8 @@ func (op *CreateOperation) Validate() error { if text.Empty(op.Title) { return fmt.Errorf("title is empty") } - if strings.Contains(op.Title, "\n") { - return fmt.Errorf("title should be a single line") - } - if !text.Safe(op.Title) { - return fmt.Errorf("title is not fully printable") + if !text.SafeOneLine(op.Title) { + return fmt.Errorf("title has unsafe characters") } if !text.Safe(op.Message) { diff --git a/bug/op_set_metadata.go b/bug/op_set_metadata.go index ca19a838..28496fd8 100644 --- a/bug/op_set_metadata.go +++ b/bug/op_set_metadata.go @@ -2,11 +2,13 @@ package bug import ( "encoding/json" + "fmt" "github.com/pkg/errors" "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/identity" + "github.com/MichaelMure/git-bug/util/text" ) var _ Operation = &SetMetadataOperation{} @@ -43,6 +45,15 @@ func (op *SetMetadataOperation) Validate() error { return errors.Wrap(err, "target invalid") } + for key, val := range op.NewMetadata { + if !text.SafeOneLine(key) { + return fmt.Errorf("metadata key is unsafe") + } + if !text.Safe(val) { + return fmt.Errorf("metadata value is not fully printable") + } + } + return nil } diff --git a/bug/op_set_title.go b/bug/op_set_title.go index c6a26746..badd192c 100644 --- a/bug/op_set_title.go +++ b/bug/op_set_title.go @@ -3,7 +3,6 @@ package bug import ( "encoding/json" "fmt" - "strings" "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/identity" @@ -49,20 +48,12 @@ func (op *SetTitleOperation) Validate() error { return fmt.Errorf("title is empty") } - if strings.Contains(op.Title, "\n") { - return fmt.Errorf("title should be a single line") + if !text.SafeOneLine(op.Title) { + return fmt.Errorf("title has unsafe characters") } - if !text.Safe(op.Title) { - return fmt.Errorf("title should be fully printable") - } - - if strings.Contains(op.Was, "\n") { - return fmt.Errorf("previous title should be a single line") - } - - if !text.Safe(op.Was) { - return fmt.Errorf("previous title should be fully printable") + if !text.SafeOneLine(op.Was) { + return fmt.Errorf("previous title has unsafe characters") } return nil diff --git a/bug/operation.go b/bug/operation.go index 8daa2cde..b3610381 100644 --- a/bug/operation.go +++ b/bug/operation.go @@ -113,6 +113,8 @@ func operationUnmarshaller(author identity.Interface, raw json.RawMessage) (dag. op.Author_ = author case *CreateOperation: op.Author_ = author + case *EditCommentOperation: + op.Author_ = author case *LabelChangeOperation: op.Author_ = author case *NoOpOperation: diff --git a/cache/bug_cache.go b/cache/bug_cache.go index bbe9830f..a248e872 100644 --- a/cache/bug_cache.go +++ b/cache/bug_cache.go @@ -235,6 +235,7 @@ func (c *BugCache) SetTitleRaw(author *IdentityCache, unixTime int64, title stri return op, c.notifyUpdated() } +// Convenience function to edit the body of a bug (the first comment) func (c *BugCache) EditCreateComment(body string) (*bug.EditCommentOperation, error) { author, err := c.repoCache.GetUserIdentity() if err != nil { @@ -244,6 +245,7 @@ func (c *BugCache) EditCreateComment(body string) (*bug.EditCommentOperation, er return c.EditCreateCommentRaw(author, time.Now().Unix(), body, nil) } +// Convenience function to edit the body of a bug (the first comment) func (c *BugCache) EditCreateCommentRaw(author *IdentityCache, unixTime int64, body string, metadata map[string]string) (*bug.EditCommentOperation, error) { c.mu.Lock() op, err := bug.EditCreateComment(c.bug, author.Identity, unixTime, body) diff --git a/cache/repo_cache.go b/cache/repo_cache.go index 58022bda..14d5f3db 100644 --- a/cache/repo_cache.go +++ b/cache/repo_cache.go @@ -195,7 +195,7 @@ func (c *RepoCache) buildCache() error { c.bugExcerpts = make(map[entity.Id]*BugExcerpt) - allBugs := bug.ReadAll(c.repo) + allBugs := bug.ReadAllWithResolver(c.repo, newIdentityCacheResolverNoLock(c)) // wipe the index just to be sure err := c.repo.ClearBleveIndex("bug") diff --git a/cache/repo_cache_test.go b/cache/repo_cache_test.go index fab8fff0..d13fa026 100644 --- a/cache/repo_cache_test.go +++ b/cache/repo_cache_test.go @@ -86,11 +86,12 @@ func TestCache(t *testing.T) { require.Empty(t, cache.identities) require.Empty(t, cache.identitiesExcerpts) - // Reload, only excerpt are loaded + // Reload, only excerpt are loaded, but as we need to load the identities used in the bugs + // to check the signatures, we also load the identity used above cache, err = NewRepoCache(repo) require.NoError(t, err) require.Empty(t, cache.bugs) - require.Empty(t, cache.identities) + require.Len(t, cache.identities, 1) require.Len(t, cache.bugExcerpts, 2) require.Len(t, cache.identitiesExcerpts, 2) diff --git a/cache/resolvers.go b/cache/resolvers.go index 36b70d3b..e53c3660 100644 --- a/cache/resolvers.go +++ b/cache/resolvers.go @@ -20,3 +20,32 @@ func newIdentityCacheResolver(cache *RepoCache) *identityCacheResolver { func (i *identityCacheResolver) ResolveIdentity(id entity.Id) (identity.Interface, error) { return i.cache.ResolveIdentity(id) } + +var _ identity.Resolver = &identityCacheResolverNoLock{} + +// identityCacheResolverNoLock is an identity Resolver that retrieve identities from +// the cache, without locking it. +type identityCacheResolverNoLock struct { + cache *RepoCache +} + +func newIdentityCacheResolverNoLock(cache *RepoCache) *identityCacheResolverNoLock { + return &identityCacheResolverNoLock{cache: cache} +} + +func (ir *identityCacheResolverNoLock) ResolveIdentity(id entity.Id) (identity.Interface, error) { + cached, ok := ir.cache.identities[id] + if ok { + return cached, nil + } + + i, err := identity.ReadLocal(ir.cache.repo, id) + if err != nil { + return nil, err + } + + cached = NewIdentityCache(ir.cache, i) + ir.cache.identities[id] = cached + + return cached, nil +} diff --git a/commands/add.go b/commands/add.go index 17fbbc93..ad3b164b 100644 --- a/commands/add.go +++ b/commands/add.go @@ -4,6 +4,7 @@ import ( "github.com/spf13/cobra" "github.com/MichaelMure/git-bug/input" + "github.com/MichaelMure/git-bug/util/text" ) type addOptions struct { @@ -60,7 +61,10 @@ func runAdd(env *Env, opts addOptions) error { } } - b, _, err := env.backend.NewBug(opts.title, opts.message) + b, _, err := env.backend.NewBug( + text.CleanupOneLine(opts.title), + text.Cleanup(opts.message), + ) if err != nil { return err } diff --git a/commands/comment_add.go b/commands/comment_add.go index 6caf4943..438e983a 100644 --- a/commands/comment_add.go +++ b/commands/comment_add.go @@ -5,6 +5,7 @@ import ( _select "github.com/MichaelMure/git-bug/commands/select" "github.com/MichaelMure/git-bug/input" + "github.com/MichaelMure/git-bug/util/text" ) type commentAddOptions struct { @@ -62,7 +63,7 @@ func runCommentAdd(env *Env, opts commentAddOptions, args []string) error { } } - _, err = b.AddComment(opts.message) + _, err = b.AddComment(text.Cleanup(opts.message)) if err != nil { return err } diff --git a/commands/label_add.go b/commands/label_add.go index ce971f04..30c0d77f 100644 --- a/commands/label_add.go +++ b/commands/label_add.go @@ -4,6 +4,7 @@ import ( "github.com/spf13/cobra" _select "github.com/MichaelMure/git-bug/commands/select" + "github.com/MichaelMure/git-bug/util/text" ) func newLabelAddCommand() *cobra.Command { @@ -30,7 +31,7 @@ func runLabelAdd(env *Env, args []string) error { added := args - changes, _, err := b.ChangeLabels(added, nil) + changes, _, err := b.ChangeLabels(text.CleanupOneLineArray(added), nil) for _, change := range changes { env.out.Println(change) diff --git a/commands/label_rm.go b/commands/label_rm.go index f59ec02d..9fb3e096 100644 --- a/commands/label_rm.go +++ b/commands/label_rm.go @@ -4,6 +4,7 @@ import ( "github.com/spf13/cobra" _select "github.com/MichaelMure/git-bug/commands/select" + "github.com/MichaelMure/git-bug/util/text" ) func newLabelRmCommand() *cobra.Command { @@ -30,7 +31,7 @@ func runLabelRm(env *Env, args []string) error { removed := args - changes, _, err := b.ChangeLabels(nil, removed) + changes, _, err := b.ChangeLabels(nil, text.CleanupOneLineArray(removed)) for _, change := range changes { env.out.Println(change) diff --git a/commands/title_edit.go b/commands/title_edit.go index 3750f2cc..455c9384 100644 --- a/commands/title_edit.go +++ b/commands/title_edit.go @@ -5,6 +5,7 @@ import ( _select "github.com/MichaelMure/git-bug/commands/select" "github.com/MichaelMure/git-bug/input" + "github.com/MichaelMure/git-bug/util/text" ) type titleEditOptions struct { @@ -58,7 +59,7 @@ func runTitleEdit(env *Env, opts titleEditOptions, args []string) error { env.err.Println("No change, aborting.") } - _, err = b.SetTitle(opts.title) + _, err = b.SetTitle(text.CleanupOneLine(opts.title)) if err != nil { return err } diff --git a/entity/dag/clock.go b/entity/dag/clock.go index dc9bb72d..793fa1bf 100644 --- a/entity/dag/clock.go +++ b/entity/dag/clock.go @@ -9,7 +9,7 @@ import ( // ClockLoader is the repository.ClockLoader for Entity func ClockLoader(defs ...Definition) repository.ClockLoader { - clocks := make([]string, len(defs)*2) + clocks := make([]string, 0, len(defs)*2) for _, def := range defs { clocks = append(clocks, fmt.Sprintf(creationClockPattern, def.Namespace)) clocks = append(clocks, fmt.Sprintf(editClockPattern, def.Namespace)) @@ -18,8 +18,9 @@ func ClockLoader(defs ...Definition) repository.ClockLoader { return repository.ClockLoader{ Clocks: clocks, Witnesser: func(repo repository.ClockedRepo) error { - // We don't care about the actual identity so an IdentityStub will do - resolver := identity.NewStubResolver() + // we need to actually load the identities because of the commit signature check when reading, + // which require the full identities with crypto keys + resolver := identity.NewCachedResolver(identity.NewSimpleResolver(repo)) for _, def := range defs { // we actually just need to read all entities, diff --git a/identity/resolver.go b/identity/resolver.go index ab380a12..8e066e9d 100644 --- a/identity/resolver.go +++ b/identity/resolver.go @@ -1,6 +1,8 @@ package identity import ( + "sync" + "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/repository" ) @@ -34,3 +36,36 @@ func NewStubResolver() *StubResolver { func (s *StubResolver) ResolveIdentity(id entity.Id) (Interface, error) { return &IdentityStub{id: id}, nil } + +// CachedResolver is a resolver ensuring that loading is done only once through another Resolver. +type CachedResolver struct { + mu sync.RWMutex + resolver Resolver + identities map[entity.Id]Interface +} + +func NewCachedResolver(resolver Resolver) *CachedResolver { + return &CachedResolver{ + resolver: resolver, + identities: make(map[entity.Id]Interface), + } +} + +func (c *CachedResolver) ResolveIdentity(id entity.Id) (Interface, error) { + c.mu.RLock() + if i, ok := c.identities[id]; ok { + c.mu.RUnlock() + return i, nil + } + c.mu.RUnlock() + + c.mu.Lock() + defer c.mu.Unlock() + + i, err := c.resolver.ResolveIdentity(id) + if err != nil { + return nil, err + } + c.identities[id] = i + return i, nil +} diff --git a/identity/version.go b/identity/version.go index 1c35831e..9a52d089 100644 --- a/identity/version.go +++ b/identity/version.go @@ -4,7 +4,6 @@ import ( "crypto/rand" "encoding/json" "fmt" - "strings" "time" "github.com/pkg/errors" @@ -186,25 +185,16 @@ func (v *version) Validate() error { if text.Empty(v.name) && text.Empty(v.login) { return fmt.Errorf("either name or login should be set") } - if strings.Contains(v.name, "\n") { - return fmt.Errorf("name should be a single line") - } - if !text.Safe(v.name) { - return fmt.Errorf("name is not fully printable") + if !text.SafeOneLine(v.name) { + return fmt.Errorf("name has unsafe characters") } - if strings.Contains(v.login, "\n") { - return fmt.Errorf("login should be a single line") - } - if !text.Safe(v.login) { - return fmt.Errorf("login is not fully printable") + if !text.SafeOneLine(v.login) { + return fmt.Errorf("login has unsafe characters") } - if strings.Contains(v.email, "\n") { - return fmt.Errorf("email should be a single line") - } - if !text.Safe(v.email) { - return fmt.Errorf("email is not fully printable") + if !text.SafeOneLine(v.email) { + return fmt.Errorf("email has unsafe characters") } if v.avatarURL != "" && !text.ValidUrl(v.avatarURL) { diff --git a/repository/gogit.go b/repository/gogit.go index 248c34d5..d7c6823e 100644 --- a/repository/gogit.go +++ b/repository/gogit.go @@ -31,8 +31,13 @@ var _ ClockedRepo = &GoGitRepo{} var _ TestedRepo = &GoGitRepo{} type GoGitRepo struct { - r *gogit.Repository - path string + // Unfortunately, some parts of go-git are not thread-safe so we have to cover them with a big fat mutex here. + // See https://github.com/go-git/go-git/issues/48 + // See https://github.com/go-git/go-git/issues/208 + // See https://github.com/go-git/go-git/pull/186 + rMutex sync.Mutex + r *gogit.Repository + path string clocksMutex sync.Mutex clocks map[string]lamport.Clock @@ -335,7 +340,7 @@ func (repo *GoGitRepo) ClearBleveIndex(name string) error { repo.indexesMutex.Lock() defer repo.indexesMutex.Unlock() - path := filepath.Join(repo.path, "indexes", name) + path := filepath.Join(repo.path, "git-bug", "indexes", name) err := os.RemoveAll(path) if err != nil { @@ -448,6 +453,9 @@ func (repo *GoGitRepo) StoreData(data []byte) (Hash, error) { // ReadData will attempt to read arbitrary data from the given hash func (repo *GoGitRepo) ReadData(hash Hash) ([]byte, error) { + repo.rMutex.Lock() + defer repo.rMutex.Unlock() + obj, err := repo.r.BlobObject(plumbing.NewHash(hash.String())) if err != nil { return nil, err diff --git a/termui/termui.go b/termui/termui.go index ec5387a4..3e7f43b9 100644 --- a/termui/termui.go +++ b/termui/termui.go @@ -11,6 +11,7 @@ import ( "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/input" "github.com/MichaelMure/git-bug/query" + "github.com/MichaelMure/git-bug/util/text" ) var errTerminateMainloop = errors.New("terminate gocui mainloop") @@ -199,7 +200,10 @@ func newBugWithEditor(repo *cache.RepoCache) error { return errTerminateMainloop } else { - b, _, err = repo.NewBug(title, message) + b, _, err = repo.NewBug( + text.CleanupOneLine(title), + text.Cleanup(message), + ) if err != nil { return err } @@ -235,7 +239,7 @@ func addCommentWithEditor(bug *cache.BugCache) error { if err == input.ErrEmptyMessage { ui.msgPopup.Activate(msgPopupErrorTitle, "Empty message, aborting.") } else { - _, err := bug.AddComment(message) + _, err := bug.AddComment(text.Cleanup(message)) if err != nil { return err } @@ -270,7 +274,7 @@ func editCommentWithEditor(bug *cache.BugCache, target entity.Id, preMessage str } else if message == preMessage { ui.msgPopup.Activate(msgPopupErrorTitle, "No changes found, aborting.") } else { - _, err := bug.EditComment(target, message) + _, err := bug.EditComment(target, text.Cleanup(message)) if err != nil { return err } @@ -307,7 +311,7 @@ func setTitleWithEditor(bug *cache.BugCache) error { } else if title == snap.Title { ui.msgPopup.Activate(msgPopupErrorTitle, "No change, aborting.") } else { - _, err := bug.SetTitle(title) + _, err := bug.SetTitle(text.CleanupOneLine(title)) if err != nil { return err } diff --git a/util/text/transform.go b/util/text/transform.go index 59dc4e03..395a57be 100644 --- a/util/text/transform.go +++ b/util/text/transform.go @@ -8,7 +8,7 @@ import ( "golang.org/x/text/transform" ) -func Cleanup(text string) (string, error) { +func Cleanup(text string) string { // windows new line, Github, really ? text = strings.Replace(text, "\r\n", "\n", -1) @@ -23,9 +23,33 @@ func Cleanup(text string) (string, error) { })) sanitized, _, err := transform.String(t, text) if err != nil { - return "", err + // transform.String should never return an error as our transformer doesn't returns one. + // Confirmed with fuzzing. + panic(err) } // trim extra new line not displayed in the github UI but still present in the data - return strings.TrimSpace(sanitized), nil + return strings.TrimSpace(sanitized) +} + +func CleanupOneLine(text string) string { + // remove all unicode control characters *including* + // '\n', '\r' and '\t' + t := runes.Remove(runes.Predicate(unicode.IsControl)) + sanitized, _, err := transform.String(t, text) + if err != nil { + // transform.String should never return an error as our transformer doesn't returns one. + // Confirmed with fuzzing. + panic(err) + } + + // trim extra new line not displayed in the github UI but still present in the data + return strings.TrimSpace(sanitized) +} + +func CleanupOneLineArray(texts []string) []string { + for i := range texts { + texts[i] = CleanupOneLine(texts[i]) + } + return texts } diff --git a/util/text/validate.go b/util/text/validate.go index 51e94fb4..4c3f7065 100644 --- a/util/text/validate.go +++ b/util/text/validate.go @@ -33,6 +33,18 @@ func Safe(s string) bool { return true } +// Safe will tell if a character in the string is considered unsafe +// Currently trigger on all unicode control character +func SafeOneLine(s string) bool { + for _, r := range s { + if unicode.IsControl(r) { + return false + } + } + + return true +} + // ValidUrl will tell if the string contains what seems to be a valid URL func ValidUrl(s string) bool { if strings.Contains(s, "\n") { diff --git a/webui/.eslintrc.js b/webui/.eslintrc.js index 2dfa7543..125fe801 100644 --- a/webui/.eslintrc.js +++ b/webui/.eslintrc.js @@ -38,4 +38,5 @@ module.exports = { settings: { 'import/internal-regex': '^src/', }, + ignorePatterns: ['**/*.generated.tsx'], }; diff --git a/webui/package-lock.json b/webui/package-lock.json index 12dea8b9..b3b2a490 100644 --- a/webui/package-lock.json +++ b/webui/package-lock.json @@ -12325,6 +12325,11 @@ "resolved": "https://registry.npmjs.org/functional-red-black-tree/-/functional-red-black-tree-1.0.1.tgz", "integrity": "sha1-GwqzvVU7Kg1jmdKcDj6gslIHgyc=" }, + "gemoji": { + "version": "6.1.0", + "resolved": "https://registry.npmjs.org/gemoji/-/gemoji-6.1.0.tgz", + "integrity": "sha512-MOlX3doQ1fsfzxQX8Y+u6bC5Ssc1pBUBIPVyrS69EzKt+5LIZAOm0G5XGVNhwXFgkBF3r+Yk88ONyrFHo8iNFA==" + }, "generic-names": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/generic-names/-/generic-names-2.0.1.tgz", @@ -12572,7 +12577,8 @@ "growly": { "version": "1.3.0", "resolved": "https://registry.npmjs.org/growly/-/growly-1.3.0.tgz", - "integrity": "sha1-8QdIy+dq+WS3yWyTxrzCivEgwIE=" + "integrity": "sha1-8QdIy+dq+WS3yWyTxrzCivEgwIE=", + "optional": true }, "gzip-size": { "version": "5.1.1", @@ -16118,6 +16124,7 @@ "version": "8.0.1", "resolved": "https://registry.npmjs.org/node-notifier/-/node-notifier-8.0.1.tgz", "integrity": "sha512-BvEXF+UmsnAfYfoapKM9nGxnP+Wn7P91YfXmrKnfcYCx6VBeoN5Ez5Ogck6I8Bi5k4RlpqRYaw75pAwzX9OphA==", + "optional": true, "requires": { "growly": "^1.3.0", "is-wsl": "^2.2.0", @@ -16131,6 +16138,7 @@ "version": "7.3.4", "resolved": "https://registry.npmjs.org/semver/-/semver-7.3.4.tgz", "integrity": "sha512-tCfb2WLjqFAtXn4KEdxIhalnRtoKFN7nAwj0B3ZXCbQloV2tq5eDbcTmT68JJD3nRJq24/XgxtQKFIpQdtvmVw==", + "optional": true, "requires": { "lru-cache": "^6.0.0" } @@ -16138,12 +16146,14 @@ "uuid": { "version": "8.3.2", "resolved": "https://registry.npmjs.org/uuid/-/uuid-8.3.2.tgz", - "integrity": "sha512-+NYs2QeMWy+GWFOEm9xnn6HCDp0l7QBD7ml8zLUmJ+93Q5NF0NocErnwkTkXVFNiX3/fpC6afS8Dhb/gz7R7eg==" + "integrity": "sha512-+NYs2QeMWy+GWFOEm9xnn6HCDp0l7QBD7ml8zLUmJ+93Q5NF0NocErnwkTkXVFNiX3/fpC6afS8Dhb/gz7R7eg==", + "optional": true }, "which": { "version": "2.0.2", "resolved": "https://registry.npmjs.org/which/-/which-2.0.2.tgz", "integrity": "sha512-BLI3Tl1TW3Pvl70l3yq3Y64i+awpwXqsGBYWkkqMtnbXgrMD+yj7rhW0kuEDxzJaYXGjEW5ogapKNMEKNMjibA==", + "optional": true, "requires": { "isexe": "^2.0.0" } @@ -19227,6 +19237,15 @@ "fbjs": "^1.0.0" } }, + "remark-gemoji": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/remark-gemoji/-/remark-gemoji-6.0.0.tgz", + "integrity": "sha512-LDW2h6QqNzAbAcOjscgfkJW9/8TGBasBe/ji+3mCxHlJdhF2IEXFSmm/3tdEPP1JJDZ4y+Ea+xlFQ4tOIU9WvA==", + "requires": { + "gemoji": "^6.0.0", + "unist-util-visit": "^2.0.0" + } + }, "remark-html": { "version": "12.0.0", "resolved": "https://registry.npmjs.org/remark-html/-/remark-html-12.0.0.tgz", @@ -20219,7 +20238,8 @@ "shellwords": { "version": "0.1.1", "resolved": "https://registry.npmjs.org/shellwords/-/shellwords-0.1.1.tgz", - "integrity": "sha512-vFwSUfQvqybiICwZY5+DAWIPLKsWO31Q91JSKl3UYv+K5c2QRPzn0qzec6QPu1Qc9eHYItiP3NdJqNVqetYAww==" + "integrity": "sha512-vFwSUfQvqybiICwZY5+DAWIPLKsWO31Q91JSKl3UYv+K5c2QRPzn0qzec6QPu1Qc9eHYItiP3NdJqNVqetYAww==", + "optional": true }, "side-channel": { "version": "1.0.3", diff --git a/webui/package.json b/webui/package.json index 39696a25..47cdf8d0 100644 --- a/webui/package.json +++ b/webui/package.json @@ -22,6 +22,7 @@ "react-router": "^5.2.0", "react-router-dom": "^5.2.0", "react-scripts": "^4.0.0-next.98", + "remark-gemoji": "^6.0.0", "remark-html": "^12.0.0", "remark-parse": "^8.0.3", "remark-react": "^7.0.1", diff --git a/webui/src/components/BackToListButton.tsx b/webui/src/components/BackToListButton.tsx index 7ca53ad0..41e1d68a 100644 --- a/webui/src/components/BackToListButton.tsx +++ b/webui/src/components/BackToListButton.tsx @@ -1,4 +1,5 @@ import React from 'react'; +import { Link } from 'react-router-dom'; import Button from '@material-ui/core/Button'; import { makeStyles } from '@material-ui/core/styles'; @@ -25,7 +26,8 @@ function BackToListButton() { variant="contained" className={classes.backButton} aria-label="back to issue list" - href="/" + component={Link} + to="/" > <ArrowBackIcon /> Back to List diff --git a/webui/src/components/BugTitleForm/BugTitleForm.tsx b/webui/src/components/BugTitleForm/BugTitleForm.tsx index a7d5a820..665ecd4c 100644 --- a/webui/src/components/BugTitleForm/BugTitleForm.tsx +++ b/webui/src/components/BugTitleForm/BugTitleForm.tsx @@ -1,4 +1,5 @@ import React, { useState } from 'react'; +import { Link } from 'react-router-dom'; import { Button, makeStyles, Typography } from '@material-ui/core'; @@ -78,6 +79,10 @@ function BugTitleForm({ bug }: Props) { function submitNewTitle() { if (!isFormValid()) return; + if (bug.title === issueTitleInput.value) { + cancelChange(); + return; + } setTitle({ variables: { input: { @@ -106,7 +111,7 @@ function BugTitleForm({ bug }: Props) { function editableBugTitle() { return ( - <form className={classes.headerTitle} onSubmit={submitNewTitle}> + <form className={classes.headerTitle}> <BugTitleInput inputRef={(node) => { issueTitleInput = node; @@ -123,7 +128,7 @@ function BugTitleForm({ bug }: Props) { className={classes.saveButton} size="small" variant="contained" - type="submit" + onClick={() => submitNewTitle()} disabled={issueTitle.length === 0} > Save @@ -157,7 +162,8 @@ function BugTitleForm({ bug }: Props) { className={classes.greenButton} size="small" variant="contained" - href="/new" + component={Link} + to="/new" > New bug </Button> diff --git a/webui/src/components/CommentInput/CommentInput.tsx b/webui/src/components/CommentInput/CommentInput.tsx index c574538e..f12ee8d8 100644 --- a/webui/src/components/CommentInput/CommentInput.tsx +++ b/webui/src/components/CommentInput/CommentInput.tsx @@ -5,7 +5,7 @@ import Tabs from '@material-ui/core/Tabs'; import TextField from '@material-ui/core/TextField'; import { makeStyles } from '@material-ui/core/styles'; -import Content from 'src/components/Content'; +import Content from '../Content'; /** * Styles diff --git a/webui/src/components/Content/AnchorTag.tsx b/webui/src/components/Content/AnchorTag.tsx new file mode 100644 index 00000000..47d5e2fa --- /dev/null +++ b/webui/src/components/Content/AnchorTag.tsx @@ -0,0 +1,38 @@ +import React from 'react'; +import { Link } from 'react-router-dom'; + +import { makeStyles } from '@material-ui/core/styles'; + +const useStyles = makeStyles((theme) => ({ + tag: { + color: theme.palette.text.secondary, + }, +})); + +const AnchorTag = ({ children, href }: React.HTMLProps<HTMLAnchorElement>) => { + const classes = useStyles(); + const origin = window.location.origin; + const destination = href === undefined ? '' : href; + const isInternalLink = + destination.startsWith('/') || destination.startsWith(origin); + const internalDestination = destination.replace(origin, ''); + const internalLink = ( + <Link className={classes.tag} to={internalDestination}> + {children} + </Link> + ); + const externalLink = ( + <a + className={classes.tag} + href={destination} + target="_blank" + rel="noopener noreferrer" + > + {children} + </a> + ); + + return isInternalLink ? internalLink : externalLink; +}; + +export default AnchorTag; diff --git a/webui/src/components/Content/BlockQuoteTag.tsx b/webui/src/components/Content/BlockQuoteTag.tsx new file mode 100644 index 00000000..18c34d8a --- /dev/null +++ b/webui/src/components/Content/BlockQuoteTag.tsx @@ -0,0 +1,21 @@ +import React from 'react'; + +import { makeStyles } from '@material-ui/core/styles'; + +const useStyles = makeStyles((theme) => ({ + tag: { + color: theme.palette.text.secondary, + borderLeftWidth: '0.5ch', + borderLeftStyle: 'solid', + borderLeftColor: theme.palette.text.secondary, + marginLeft: 0, + paddingLeft: '0.5rem', + }, +})); + +const BlockQuoteTag = (props: React.HTMLProps<HTMLPreElement>) => { + const classes = useStyles(); + return <blockquote className={classes.tag} {...props} />; +}; + +export default BlockQuoteTag; diff --git a/webui/src/components/Content/ImageTag.tsx b/webui/src/components/Content/ImageTag.tsx index 70ee1bc0..29b01da3 100644 --- a/webui/src/components/Content/ImageTag.tsx +++ b/webui/src/components/Content/ImageTag.tsx @@ -14,9 +14,12 @@ const ImageTag = ({ }: React.ImgHTMLAttributes<HTMLImageElement>) => { const classes = useStyles(); return ( - <a href={props.src} target="_blank" rel="noopener noreferrer nofollow"> - <img className={classes.tag} alt={alt} {...props} /> - </a> + <> + <a href={props.src} target="_blank" rel="noopener noreferrer nofollow"> + <img className={classes.tag} alt={alt} {...props} /> + </a> + <br /> + </> ); }; diff --git a/webui/src/components/Content/index.tsx b/webui/src/components/Content/index.tsx index 56e52e1e..e4018809 100644 --- a/webui/src/components/Content/index.tsx +++ b/webui/src/components/Content/index.tsx @@ -1,26 +1,32 @@ import React from 'react'; +import gemoji from 'remark-gemoji'; import html from 'remark-html'; import parse from 'remark-parse'; import remark2react from 'remark-react'; import unified from 'unified'; +import AnchorTag from './AnchorTag'; +import BlockQuoteTag from './BlockQuoteTag'; import ImageTag from './ImageTag'; import PreTag from './PreTag'; type Props = { markdown: string }; const Content: React.FC<Props> = ({ markdown }: Props) => { - const processor = unified() + const content = unified() .use(parse) + .use(gemoji) .use(html) .use(remark2react, { remarkReactComponents: { img: ImageTag, pre: PreTag, + a: AnchorTag, + blockquote: BlockQuoteTag, }, - }); + }) + .processSync(markdown).result; - const contents: React.ReactNode = processor.processSync(markdown).contents; - return <>{contents}</>; + return <>{content}</>; }; export default Content; diff --git a/webui/src/components/Header/Header.tsx b/webui/src/components/Header/Header.tsx index 3064f6e4..63146cc9 100644 --- a/webui/src/components/Header/Header.tsx +++ b/webui/src/components/Header/Header.tsx @@ -67,14 +67,14 @@ const DisabledTabWithTooltip = (props: TabProps) => { function Header() { const classes = useStyles(); const location = useLocation(); - const [selectedTab, setTab] = React.useState(location.pathname); - const handleTabClick = ( - event: React.ChangeEvent<{}>, - newTabValue: string - ) => { - setTab(newTabValue); - }; + // Prevents error of invalid tab selection in <Tabs> + // Will return a valid tab path or false if path is unkown. + function highlightTab() { + const validTabs = ['/', '/code', '/pulls', '/settings']; + const tab = validTabs.find((tabPath) => tabPath === location.pathname); + return tab === undefined ? false : tab; + } return ( <> @@ -92,12 +92,7 @@ function Header() { </Toolbar> </AppBar> <div className={classes.offset} /> - <Tabs - centered - value={selectedTab} - onChange={handleTabClick} - aria-label="nav tabs" - > + <Tabs centered value={highlightTab()} aria-label="nav tabs"> <DisabledTabWithTooltip label="Code" value="/code" {...a11yProps(1)} /> <Tab label="Bugs" value="/" component={Link} to="/" {...a11yProps(2)} /> <DisabledTabWithTooltip diff --git a/webui/src/components/Label.tsx b/webui/src/components/Label.tsx index 111f6d7f..a1d3c6f9 100644 --- a/webui/src/components/Label.tsx +++ b/webui/src/components/Label.tsx @@ -1,56 +1,47 @@ import React from 'react'; +import { Chip } from '@material-ui/core'; import { common } from '@material-ui/core/colors'; -import { makeStyles } from '@material-ui/core/styles'; import { - getContrastRatio, darken, + getContrastRatio, } from '@material-ui/core/styles/colorManipulator'; +import { Color } from '../gqlTypes'; import { LabelFragment } from '../graphql/fragments.generated'; -import { Color } from 'src/gqlTypes'; + +const _rgb = (color: Color) => + 'rgb(' + color.R + ',' + color.G + ',' + color.B + ')'; // Minimum contrast between the background and the text color const contrastThreshold = 2.5; - // Guess the text color based on the background color const getTextColor = (background: string) => getContrastRatio(background, common.white) >= contrastThreshold ? common.white // White on dark backgrounds : common.black; // And black on light ones -const _rgb = (color: Color) => - 'rgb(' + color.R + ',' + color.G + ',' + color.B + ')'; - // Create a style object from the label RGB colors -const createStyle = (color: Color) => ({ +const createStyle = (color: Color, maxWidth?: string) => ({ backgroundColor: _rgb(color), color: getTextColor(_rgb(color)), borderBottomColor: darken(_rgb(color), 0.2), + maxWidth: maxWidth, }); -const useStyles = makeStyles((theme) => ({ - label: { - ...theme.typography.body1, - padding: '1px 6px 0.5px', - fontSize: '0.9em', - fontWeight: 500, - margin: '0.05em 1px calc(-1.5px + 0.05em)', - borderRadius: '3px', - display: 'inline-block', - borderBottom: 'solid 1.5px', - verticalAlign: 'bottom', - }, -})); - -type Props = { label: LabelFragment }; -function Label({ label }: Props) { - const classes = useStyles(); +type Props = { + label: LabelFragment; + maxWidth?: string; + className?: string; +}; +function Label({ label, maxWidth, className }: Props) { return ( - <span className={classes.label} style={createStyle(label.color)}> - {label.name} - </span> + <Chip + size={'small'} + label={label.name} + className={className} + style={createStyle(label.color, maxWidth)} + /> ); } - export default Label; diff --git a/webui/src/pages/bug/Bug.tsx b/webui/src/pages/bug/Bug.tsx index 25281f96..b32b0948 100644 --- a/webui/src/pages/bug/Bug.tsx +++ b/webui/src/pages/bug/Bug.tsx @@ -9,6 +9,7 @@ import Label from 'src/components/Label'; import { BugFragment } from './Bug.generated'; import CommentForm from './CommentForm'; import TimelineQuery from './TimelineQuery'; +import LabelMenu from './labels/LabelMenu'; /** * Css in JS Styles @@ -53,13 +54,15 @@ const useStyles = makeStyles((theme) => ({ listStyle: 'none', padding: 0, margin: 0, + display: 'flex', + flexDirection: 'row', + flexWrap: 'wrap', }, label: { - marginTop: theme.spacing(1), - marginBottom: theme.spacing(1), - '& > *': { - display: 'block', - }, + marginTop: theme.spacing(0.1), + marginBottom: theme.spacing(0.1), + marginLeft: theme.spacing(0.25), + marginRight: theme.spacing(0.25), }, noLabel: { ...theme.typography.body2, @@ -94,14 +97,16 @@ function Bug({ bug }: Props) { </IfLoggedIn> </div> <div className={classes.rightSidebar}> - <span className={classes.rightSidebarTitle}>Labels</span> + <span className={classes.rightSidebarTitle}> + <LabelMenu bug={bug} /> + </span> <ul className={classes.labelList}> {bug.labels.length === 0 && ( <span className={classes.noLabel}>None yet</span> )} {bug.labels.map((l) => ( <li className={classes.label} key={l.name}> - <Label label={l} key={l.name} /> + <Label label={l} key={l.name} maxWidth="25ch" /> </li> ))} </ul> diff --git a/webui/src/pages/bug/CommentForm.tsx b/webui/src/pages/bug/CommentForm.tsx index e70348a6..a8ce4319 100644 --- a/webui/src/pages/bug/CommentForm.tsx +++ b/webui/src/pages/bug/CommentForm.tsx @@ -17,14 +17,6 @@ const useStyles = makeStyles<Theme, StyleProps>((theme) => ({ container: { padding: theme.spacing(0, 2, 2, 2), }, - textarea: {}, - tabContent: { - margin: theme.spacing(2, 0), - }, - preview: { - borderBottom: `solid 3px ${theme.palette.grey['200']}`, - minHeight: '5rem', - }, actions: { display: 'flex', gap: '1em', diff --git a/webui/src/pages/bug/LabelChange.tsx b/webui/src/pages/bug/LabelChange.tsx index c40636c1..712c33fa 100644 --- a/webui/src/pages/bug/LabelChange.tsx +++ b/webui/src/pages/bug/LabelChange.tsx @@ -16,6 +16,11 @@ const useStyles = makeStyles((theme) => ({ author: { fontWeight: 'bold', }, + label: { + maxWidth: '50ch', + marginLeft: theme.spacing(0.25), + marginRight: theme.spacing(0.25), + }, })); type Props = { @@ -30,12 +35,12 @@ function LabelChange({ op }: Props) { <Author author={op.author} className={classes.author} /> {added.length > 0 && <span> added the </span>} {added.map((label, index) => ( - <Label key={index} label={label} /> + <Label key={index} label={label} className={classes.label} /> ))} {added.length > 0 && removed.length > 0 && <span> and</span>} {removed.length > 0 && <span> removed the </span>} {removed.map((label, index) => ( - <Label key={index} label={label} /> + <Label key={index} label={label} className={classes.label} /> ))} <span> {' '} diff --git a/webui/src/pages/bug/Message.tsx b/webui/src/pages/bug/Message.tsx index 2f4cbc59..39b11ccd 100644 --- a/webui/src/pages/bug/Message.tsx +++ b/webui/src/pages/bug/Message.tsx @@ -57,7 +57,8 @@ const useStyles = makeStyles((theme) => ({ }, body: { ...theme.typography.body2, - padding: '0.5rem', + paddingLeft: theme.spacing(1), + paddingRight: theme.spacing(1), }, headerActions: { color: theme.palette.info.contrastText, diff --git a/webui/src/pages/bug/MessageHistoryDialog.tsx b/webui/src/pages/bug/MessageHistoryDialog.tsx index 0ed33642..5879a373 100644 --- a/webui/src/pages/bug/MessageHistoryDialog.tsx +++ b/webui/src/pages/bug/MessageHistoryDialog.tsx @@ -22,6 +22,8 @@ import { import CloseIcon from '@material-ui/icons/Close'; import ExpandMoreIcon from '@material-ui/icons/ExpandMore'; +import Content from '../../components/Content'; + import { AddCommentFragment } from './MessageCommentFragment.generated'; import { CreateFragment } from './MessageCreateFragment.generated'; import { useMessageHistoryQuery } from './MessageHistory.generated'; @@ -108,6 +110,7 @@ const AccordionSummary = withStyles((theme) => ({ const AccordionDetails = withStyles((theme) => ({ root: { + display: 'block', padding: theme.spacing(2), }, }))(MuiAccordionDetails); @@ -214,6 +217,7 @@ function MessageHistoryDialog({ bugId, commentId, open, onClose }: Props) { {history?.map((edit, index) => ( <Accordion square + key={index} expanded={expanded === 'panel' + index} onChange={handleChange('panel' + index)} > @@ -224,7 +228,9 @@ function MessageHistoryDialog({ bugId, commentId, open, onClose }: Props) { > <Typography>{getSummary(index, edit.date)}</Typography> </AccordionSummary> - <AccordionDetails>{edit.message}</AccordionDetails> + <AccordionDetails> + <Content markdown={edit.message} /> + </AccordionDetails> </Accordion> ))} </DialogContent> diff --git a/webui/src/pages/bug/labels/LabelMenu.tsx b/webui/src/pages/bug/labels/LabelMenu.tsx new file mode 100644 index 00000000..645f472c --- /dev/null +++ b/webui/src/pages/bug/labels/LabelMenu.tsx @@ -0,0 +1,309 @@ +import React, { useEffect, useRef, useState } from 'react'; + +import { IconButton } from '@material-ui/core'; +import Menu from '@material-ui/core/Menu'; +import MenuItem from '@material-ui/core/MenuItem'; +import TextField from '@material-ui/core/TextField'; +import { makeStyles, withStyles } from '@material-ui/core/styles'; +import { darken } from '@material-ui/core/styles/colorManipulator'; +import CheckIcon from '@material-ui/icons/Check'; +import SettingsIcon from '@material-ui/icons/Settings'; + +import { Color } from '../../../gqlTypes'; +import { + ListLabelsDocument, + useListLabelsQuery, +} from '../../list/ListLabels.generated'; +import { BugFragment } from '../Bug.generated'; +import { GetBugDocument } from '../BugQuery.generated'; + +import { useSetLabelMutation } from './SetLabel.generated'; + +type DropdownTuple = [string, string, Color]; + +type FilterDropdownProps = { + children: React.ReactNode; + dropdown: DropdownTuple[]; + hasFilter?: boolean; + itemActive: (key: string) => boolean; + onClose: () => void; + toggleLabel: (key: string, active: boolean) => void; + onNewItem: (name: string) => void; +} & React.ButtonHTMLAttributes<HTMLButtonElement>; + +const CustomTextField = withStyles((theme) => ({ + root: { + margin: '0 8px 12px 8px', + '& label.Mui-focused': { + margin: '0 2px', + color: theme.palette.text.secondary, + }, + '& .MuiInput-underline::before': { + borderBottomColor: theme.palette.divider, + }, + '& .MuiInput-underline::after': { + borderBottomColor: theme.palette.divider, + }, + }, +}))(TextField); + +const ITEM_HEIGHT = 48; + +const useStyles = makeStyles((theme) => ({ + gearBtn: { + ...theme.typography.body2, + color: theme.palette.text.secondary, + padding: theme.spacing(0, 1), + fontWeight: 400, + textDecoration: 'none', + display: 'flex', + background: 'none', + border: 'none', + '&:hover': { + backgroundColor: 'transparent', + color: theme.palette.text.primary, + }, + }, + menu: { + '& .MuiMenu-paper': { + //somehow using "width" won't override the default width... + minWidth: '35ch', + }, + }, + labelcolor: { + minWidth: '0.5rem', + display: 'flex', + borderRadius: '0.25rem', + marginRight: '5px', + marginLeft: '3px', + }, + labelsheader: { + display: 'flex', + flexDirection: 'row', + }, + menuRow: { + display: 'flex', + alignItems: 'initial', + }, +})); + +const _rgb = (color: Color) => + 'rgb(' + color.R + ',' + color.G + ',' + color.B + ')'; + +// Create a style object from the label RGB colors +const createStyle = (color: Color) => ({ + backgroundColor: _rgb(color), + borderBottomColor: darken(_rgb(color), 0.2), +}); + +function FilterDropdown({ + children, + dropdown, + hasFilter, + itemActive, + onClose, + toggleLabel, + onNewItem, +}: FilterDropdownProps) { + const [open, setOpen] = useState(false); + const [filter, setFilter] = useState<string>(''); + const buttonRef = useRef<HTMLButtonElement>(null); + const searchRef = useRef<HTMLButtonElement>(null); + const classes = useStyles({ active: false }); + + useEffect(() => { + searchRef && searchRef.current && searchRef.current.focus(); + }, [filter]); + + return ( + <> + <div className={classes.labelsheader}> + Labels + <IconButton + ref={buttonRef} + onClick={() => setOpen(!open)} + className={classes.gearBtn} + disableRipple + > + <SettingsIcon fontSize={'small'} /> + </IconButton> + </div> + + <Menu + className={classes.menu} + getContentAnchorEl={null} + ref={searchRef} + anchorOrigin={{ + vertical: 'bottom', + horizontal: 'left', + }} + transformOrigin={{ + vertical: 'top', + horizontal: 'left', + }} + open={open} + onClose={() => { + setOpen(false); + onClose(); + }} + onExited={() => setFilter('')} + anchorEl={buttonRef.current} + PaperProps={{ + style: { + maxHeight: ITEM_HEIGHT * 4.5, + width: '25ch', + }, + }} + > + {hasFilter && ( + <CustomTextField + onChange={(e) => { + const { value } = e.target; + setFilter(value); + }} + onKeyDown={(e) => e.stopPropagation()} + value={filter} + label={`Filter ${children}`} + /> + )} + {filter !== '' && + dropdown.filter((d) => d[1].toLowerCase() === filter.toLowerCase()) + .length <= 0 && ( + <MenuItem + style={{ whiteSpace: 'normal', wordBreak: 'break-all' }} + onClick={() => { + onNewItem(filter); + setFilter(''); + setOpen(false); + }} + > + Create new label '{filter}' + </MenuItem> + )} + {dropdown + .sort(function (x, y) { + // true values first + return itemActive(x[1]) === itemActive(y[1]) ? 0 : x ? -1 : 1; + }) + .filter((d) => d[1].toLowerCase().includes(filter.toLowerCase())) + .map(([key, value, color]) => ( + <MenuItem + style={{ whiteSpace: 'normal', wordBreak: 'break-word' }} + onClick={() => { + toggleLabel(key, itemActive(key)); + }} + key={key} + selected={itemActive(key)} + > + <div className={classes.menuRow}> + {itemActive(key) && <CheckIcon />} + <div + className={classes.labelcolor} + style={createStyle(color)} + /> + {value} + </div> + </MenuItem> + ))} + </Menu> + </> + ); +} + +type Props = { + bug: BugFragment; +}; +function LabelMenu({ bug }: Props) { + const { data: labelsData } = useListLabelsQuery(); + const [bugLabelNames, setBugLabelNames] = useState( + bug.labels.map((l) => l.name) + ); + const [selectedLabels, setSelectedLabels] = useState( + bug.labels.map((l) => l.name) + ); + + const [setLabelMutation] = useSetLabelMutation(); + + function toggleLabel(key: string, active: boolean) { + const labels: string[] = active + ? selectedLabels.filter((label) => label !== key) + : selectedLabels.concat([key]); + setSelectedLabels(labels); + } + + function diff(oldState: string[], newState: string[]) { + const added = newState.filter((x) => !oldState.includes(x)); + const removed = oldState.filter((x) => !newState.includes(x)); + return { + added: added, + removed: removed, + }; + } + + const changeBugLabels = (selectedLabels: string[]) => { + const labels = diff(bugLabelNames, selectedLabels); + if (labels.added.length > 0 || labels.removed.length > 0) { + setLabelMutation({ + variables: { + input: { + prefix: bug.id, + added: labels.added, + Removed: labels.removed, + }, + }, + refetchQueries: [ + // TODO: update the cache instead of refetching + { + query: GetBugDocument, + variables: { id: bug.id }, + }, + { + query: ListLabelsDocument, + }, + ], + awaitRefetchQueries: true, + }) + .then((res) => { + setSelectedLabels(selectedLabels); + setBugLabelNames(selectedLabels); + }) + .catch((e) => console.log(e)); + } + }; + + function isActive(key: string) { + return selectedLabels.includes(key); + } + + function createNewLabel(name: string) { + changeBugLabels(selectedLabels.concat([name])); + } + + let labels: any = []; + if ( + labelsData?.repository && + labelsData.repository.validLabels && + labelsData.repository.validLabels.nodes + ) { + labels = labelsData.repository.validLabels.nodes.map((node) => [ + node.name, + node.name, + node.color, + ]); + } + + return ( + <FilterDropdown + onClose={() => changeBugLabels(selectedLabels)} + itemActive={isActive} + toggleLabel={toggleLabel} + dropdown={labels} + onNewItem={createNewLabel} + hasFilter + > + Labels + </FilterDropdown> + ); +} + +export default LabelMenu; diff --git a/webui/src/pages/bug/labels/SetLabel.graphql b/webui/src/pages/bug/labels/SetLabel.graphql new file mode 100644 index 00000000..44dfae11 --- /dev/null +++ b/webui/src/pages/bug/labels/SetLabel.graphql @@ -0,0 +1,13 @@ +mutation SetLabel($input: ChangeLabelInput) { + changeLabels(input: $input) { + results{ + status, + label{ + name, + color{R}, + color{G}, + color{B} + } + } + } +} diff --git a/webui/src/pages/list/BugRow.tsx b/webui/src/pages/list/BugRow.tsx index 1f5d22aa..87e45581 100644 --- a/webui/src/pages/list/BugRow.tsx +++ b/webui/src/pages/list/BugRow.tsx @@ -59,28 +59,36 @@ const useStyles = makeStyles((theme) => ({ width: '100%', lineHeight: '20px', }, + bugTitleWrapper: { + display: 'flex', + flexDirection: 'row', + flexWrap: 'wrap', + //alignItems: 'center', + }, title: { display: 'inline', color: theme.palette.text.primary, fontSize: '1.3rem', fontWeight: 500, + marginBottom: theme.spacing(1), + }, + label: { + maxWidth: '40ch', + marginLeft: theme.spacing(0.25), + marginRight: theme.spacing(0.25), }, details: { lineHeight: '1.5rem', color: theme.palette.text.secondary, }, - labels: { - paddingLeft: theme.spacing(1), - '& > *': { - display: 'inline-block', - }, - }, commentCount: { fontSize: '1rem', marginLeft: theme.spacing(0.5), }, commentCountCell: { display: 'inline-flex', + minWidth: theme.spacing(5), + marginLeft: theme.spacing(0.5), }, })); @@ -98,15 +106,12 @@ function BugRow({ bug }: Props) { <BugStatus status={bug.status} className={classes.status} /> <div className={classes.expand}> <Link to={'bug/' + bug.humanId}> - <div className={classes.expand}> + <div className={classes.bugTitleWrapper}> <span className={classes.title}>{bug.title}</span> - {bug.labels.length > 0 && ( - <span className={classes.labels}> - {bug.labels.map((l) => ( - <Label key={l.name} label={l} /> - ))} - </span> - )} + {bug.labels.length > 0 && + bug.labels.map((l) => ( + <Label key={l.name} label={l} className={classes.label} /> + ))} </div> </Link> <div className={classes.details}> @@ -115,12 +120,14 @@ function BugRow({ bug }: Props) { by {bug.author.displayName} </div> </div> - {commentCount > 0 && ( - <span className={classes.commentCountCell}> - <CommentOutlinedIcon aria-label="Comment count" /> - <span className={classes.commentCount}>{commentCount}</span> - </span> - )} + <span className={classes.commentCountCell}> + {commentCount > 0 && ( + <> + <CommentOutlinedIcon aria-label="Comment count" /> + <span className={classes.commentCount}>{commentCount}</span> + </> + )} + </span> </TableCell> </TableRow> ); diff --git a/webui/src/pages/list/Filter.tsx b/webui/src/pages/list/Filter.tsx index 66702078..3559b3ce 100644 --- a/webui/src/pages/list/Filter.tsx +++ b/webui/src/pages/list/Filter.tsx @@ -1,13 +1,36 @@ import clsx from 'clsx'; import { LocationDescriptor } from 'history'; -import React, { useState, useRef } from 'react'; +import React, { useRef, useState, useEffect } from 'react'; import { Link } from 'react-router-dom'; import Menu from '@material-ui/core/Menu'; import MenuItem from '@material-ui/core/MenuItem'; import { SvgIconProps } from '@material-ui/core/SvgIcon'; -import { makeStyles } from '@material-ui/core/styles'; +import TextField from '@material-ui/core/TextField'; +import { makeStyles, withStyles } from '@material-ui/core/styles'; +import { darken } from '@material-ui/core/styles/colorManipulator'; import ArrowDropDown from '@material-ui/icons/ArrowDropDown'; +import CheckIcon from '@material-ui/icons/Check'; + +import { Color } from '../../gqlTypes'; + +const CustomTextField = withStyles((theme) => ({ + root: { + margin: '0 8px 12px 8px', + '& label.Mui-focused': { + margin: '0 2px', + color: theme.palette.text.secondary, + }, + '& .MuiInput-underline::before': { + borderBottomColor: theme.palette.divider, + }, + '& .MuiInput-underline::after': { + borderBottomColor: theme.palette.divider, + }, + }, +}))(TextField); + +const ITEM_HEIGHT = 48; export type Query = { [key: string]: string[] }; @@ -80,9 +103,36 @@ const useStyles = makeStyles((theme) => ({ icon: { paddingRight: theme.spacing(0.5), }, + labelMenu: { + '& .MuiMenu-paper': { + //somehow using "width" won't override the default width... + minWidth: '35ch', + }, + }, + labelMenuItem: { + whiteSpace: 'normal', + wordBreak: 'break-word', + display: 'flex', + alignItems: 'initial', + }, + labelcolor: { + minWidth: '0.5rem', + display: 'flex', + borderRadius: '0.25rem', + marginRight: '5px', + marginLeft: '3px', + }, })); +const _rgb = (color: Color) => + 'rgb(' + color.R + ',' + color.G + ',' + color.B + ')'; + +// Create a style object from the label RGB colors +const createStyle = (color: Color) => ({ + backgroundColor: _rgb(color), + borderBottomColor: darken(_rgb(color), 0.2), +}); -type DropdownTuple = [string, string]; +type DropdownTuple = [string, string, Color?]; type FilterDropdownProps = { children: React.ReactNode; @@ -90,6 +140,7 @@ type FilterDropdownProps = { itemActive: (key: string) => boolean; icon?: React.ComponentType<SvgIconProps>; to: (key: string) => LocationDescriptor; + hasFilter?: boolean; } & React.ButtonHTMLAttributes<HTMLButtonElement>; function FilterDropdown({ @@ -98,12 +149,19 @@ function FilterDropdown({ itemActive, icon: Icon, to, + hasFilter, ...props }: FilterDropdownProps) { const [open, setOpen] = useState(false); + const [filter, setFilter] = useState<string>(''); const buttonRef = useRef<HTMLButtonElement>(null); + const searchRef = useRef<HTMLButtonElement>(null); const classes = useStyles({ active: false }); + useEffect(() => { + searchRef && searchRef.current && searchRef.current.focus(); + }, [filter]); + const content = ( <> {Icon && <Icon fontSize="small" classes={{ root: classes.icon }} />} @@ -123,7 +181,9 @@ function FilterDropdown({ <ArrowDropDown fontSize="small" /> </button> <Menu + className={classes.labelMenu} getContentAnchorEl={null} + ref={searchRef} anchorOrigin={{ vertical: 'bottom', horizontal: 'left', @@ -135,18 +195,45 @@ function FilterDropdown({ open={open} onClose={() => setOpen(false)} anchorEl={buttonRef.current} + PaperProps={{ + style: { + maxHeight: ITEM_HEIGHT * 4.5, + width: '25ch', + }, + }} > - {dropdown.map(([key, value]) => ( - <MenuItem - component={Link} - to={to(key)} - className={itemActive(key) ? classes.itemActive : undefined} - onClick={() => setOpen(false)} - key={key} - > - {value} - </MenuItem> - ))} + {hasFilter && ( + <CustomTextField + onChange={(e) => { + const { value } = e.target; + setFilter(value); + }} + onKeyDown={(e) => e.stopPropagation()} + value={filter} + label={`Filter ${children}`} + /> + )} + {dropdown + .filter((d) => d[1].toLowerCase().includes(filter.toLowerCase())) + .map(([key, value, color]) => ( + <MenuItem + component={Link} + to={to(key)} + className={classes.labelMenuItem} + selected={itemActive(key)} + onClick={() => setOpen(false)} + key={key} + > + {itemActive(key) && <CheckIcon />} + {color && ( + <div + className={classes.labelcolor} + style={createStyle(color)} + /> + )} + {value} + </MenuItem> + ))} </Menu> </> ); @@ -158,6 +245,7 @@ export type FilterProps = { icon?: React.ComponentType<SvgIconProps>; children: React.ReactNode; }; + function Filter({ active, to, children, icon: Icon }: FilterProps) { const classes = useStyles(); diff --git a/webui/src/pages/list/FilterToolbar.tsx b/webui/src/pages/list/FilterToolbar.tsx index 74eefe4c..e109578d 100644 --- a/webui/src/pages/list/FilterToolbar.tsx +++ b/webui/src/pages/list/FilterToolbar.tsx @@ -8,14 +8,16 @@ import CheckCircleOutline from '@material-ui/icons/CheckCircleOutline'; import ErrorOutline from '@material-ui/icons/ErrorOutline'; import { + Filter, FilterDropdown, FilterProps, - Filter, parse, - stringify, Query, + stringify, } from './Filter'; import { useBugCountQuery } from './FilterToolbar.generated'; +import { useListIdentitiesQuery } from './ListIdentities.generated'; +import { useListLabelsQuery } from './ListLabels.generated'; const useStyles = makeStyles((theme) => ({ toolbar: { @@ -35,6 +37,7 @@ type CountingFilterProps = { query: string; // the query used as a source to count the number of element children: React.ReactNode; } & FilterProps; + function CountingFilter({ query, children, ...props }: CountingFilterProps) { const { data, loading, error } = useBugCountQuery({ variables: { query }, @@ -57,14 +60,45 @@ type Props = { query: string; queryLocation: (query: string) => LocationDescriptor; }; + function FilterToolbar({ query, queryLocation }: Props) { const classes = useStyles(); const params: Query = parse(query); + const { data: identitiesData } = useListIdentitiesQuery(); + const { data: labelsData } = useListLabelsQuery(); + + let identities: any = []; + let labels: any = []; + + if ( + identitiesData?.repository && + identitiesData.repository.allIdentities && + identitiesData.repository.allIdentities.nodes + ) { + identities = identitiesData.repository.allIdentities.nodes.map((node) => [ + node.name, + node.name, + ]); + } + + if ( + labelsData?.repository && + labelsData.repository.validLabels && + labelsData.repository.validLabels.nodes + ) { + labels = labelsData.repository.validLabels.nodes.map((node) => [ + node.name, + node.name, + node.color, + ]); + } const hasKey = (key: string): boolean => params[key] && params[key].length > 0; const hasValue = (key: string, value: string): boolean => hasKey(key) && params[key].includes(value); + const containsValue = (key: string, value: string): boolean => + hasKey(key) && params[key].indexOf(value) !== -1; const loc = pipe(stringify, queryLocation); const replaceParam = (key: string, value: string) => ( params: Query @@ -78,6 +112,20 @@ function FilterToolbar({ query, queryLocation }: Props) { ...params, [key]: params[key] && params[key].includes(value) ? [] : [value], }); + const toggleOrAddParam = (key: string, value: string) => ( + params: Query + ): Query => { + const values = params[key]; + return { + ...params, + [key]: + params[key] && params[key].includes(value) + ? values.filter((v) => v !== value) + : values + ? [...values, value] + : [value], + }; + }; const clearParam = (key: string) => (params: Query): Query => ({ ...params, [key]: [], @@ -116,6 +164,22 @@ function FilterToolbar({ query, queryLocation }: Props) { <Filter active={hasKey('label')}>Label</Filter> */} <FilterDropdown + dropdown={identities} + itemActive={(key) => hasValue('author', key)} + to={(key) => pipe(toggleOrAddParam('author', key), loc)(params)} + hasFilter + > + Author + </FilterDropdown> + <FilterDropdown + dropdown={labels} + itemActive={(key) => containsValue('label', key)} + to={(key) => pipe(toggleOrAddParam('label', key), loc)(params)} + hasFilter + > + Labels + </FilterDropdown> + <FilterDropdown dropdown={[ ['id', 'ID'], ['creation', 'Newest'], @@ -124,7 +188,7 @@ function FilterToolbar({ query, queryLocation }: Props) { ['edit-asc', 'Least recently updated'], ]} itemActive={(key) => hasValue('sort', key)} - to={(key) => pipe(replaceParam('sort', key), loc)(params)} + to={(key) => pipe(toggleParam('sort', key), loc)(params)} > Sort </FilterDropdown> diff --git a/webui/src/pages/list/ListIdentities.graphql b/webui/src/pages/list/ListIdentities.graphql new file mode 100644 index 00000000..73073ae8 --- /dev/null +++ b/webui/src/pages/list/ListIdentities.graphql @@ -0,0 +1,13 @@ +query ListIdentities { + repository { + allIdentities { + nodes { + id + humanId + name + email + displayName + } + } + } +} diff --git a/webui/src/pages/list/ListLabels.graphql b/webui/src/pages/list/ListLabels.graphql new file mode 100644 index 00000000..8b2f561a --- /dev/null +++ b/webui/src/pages/list/ListLabels.graphql @@ -0,0 +1,10 @@ +query ListLabels { + repository { + validLabels { + nodes { + name, + color{R,G,B} + } + } + } +} diff --git a/webui/src/pages/list/ListQuery.tsx b/webui/src/pages/list/ListQuery.tsx index 500ccf77..2b46dca5 100644 --- a/webui/src/pages/list/ListQuery.tsx +++ b/webui/src/pages/list/ListQuery.tsx @@ -1,19 +1,23 @@ import { ApolloError } from '@apollo/client'; +import { pipe } from '@arrows/composition'; import React, { useState, useEffect, useRef } from 'react'; import { useLocation, useHistory, Link } from 'react-router-dom'; -import { Button } from '@material-ui/core'; +import { Button, FormControl, Menu, MenuItem } from '@material-ui/core'; import IconButton from '@material-ui/core/IconButton'; import InputBase from '@material-ui/core/InputBase'; import Paper from '@material-ui/core/Paper'; import { makeStyles, Theme } from '@material-ui/core/styles'; +import ArrowDropDownIcon from '@material-ui/icons/ArrowDropDown'; import ErrorOutline from '@material-ui/icons/ErrorOutline'; import KeyboardArrowLeft from '@material-ui/icons/KeyboardArrowLeft'; import KeyboardArrowRight from '@material-ui/icons/KeyboardArrowRight'; import Skeleton from '@material-ui/lab/Skeleton'; +import { useCurrentIdentityQuery } from '../../components/CurrentIdentity/CurrentIdentity.generated'; import IfLoggedIn from 'src/components/IfLoggedIn/IfLoggedIn'; +import { parse, Query, stringify } from './Filter'; import FilterToolbar from './FilterToolbar'; import List from './List'; import { useListBugsQuery } from './ListQuery.generated'; @@ -35,24 +39,17 @@ const useStyles = makeStyles<Theme, StylesProps>((theme) => ({ }, header: { display: 'flex', - padding: theme.spacing(2), - '& > h1': { - ...theme.typography.h6, - margin: theme.spacing(0, 2), - }, - alignItems: 'center', - justifyContent: 'space-between', + padding: theme.spacing(1), }, filterissueLabel: { fontSize: '14px', fontWeight: 'bold', paddingRight: '12px', }, - filterissueContainer: { + form: { display: 'flex', - flexDirection: 'row', - alignItems: 'flex-start', - justifyContents: 'left', + flexGrow: 1, + marginRight: theme.spacing(1), }, search: { borderRadius: theme.shape.borderRadius, @@ -62,7 +59,7 @@ const useStyles = makeStyles<Theme, StylesProps>((theme) => ({ borderWidth: '1px', backgroundColor: theme.palette.primary.light, padding: theme.spacing(0, 1), - width: ({ searching }) => (searching ? '20rem' : '15rem'), + width: '100%', transition: theme.transitions.create([ 'width', 'borderColor', @@ -192,6 +189,8 @@ function ListQuery() { const query = params.has('q') ? params.get('q') || '' : 'status:open'; const [input, setInput] = useState(query); + const [filterMenuIsOpen, setFilterMenuIsOpen] = useState(false); + const filterButtonRef = useRef<HTMLButtonElement>(null); const classes = useStyles({ searching: !!input }); @@ -293,35 +292,85 @@ function ListQuery() { history.push(queryLocation(input)); }; + const { + loading: ciqLoading, + error: ciqError, + data: ciqData, + } = useCurrentIdentityQuery(); + if (ciqError || ciqLoading || !ciqData?.repository?.userIdentity) { + return null; + } + const user = ciqData.repository.userIdentity; + + const loc = pipe(stringify, queryLocation); + const qparams: Query = parse(query); + const replaceParam = (key: string, value: string) => ( + params: Query + ): Query => ({ + ...params, + [key]: [value], + }); + return ( <Paper className={classes.main}> <header className={classes.header}> - <div className="filterissueContainer"> - <form onSubmit={formSubmit}> - <label className={classes.filterissueLabel} htmlFor="issuefilter"> - Filter - </label> - <InputBase - id="issuefilter" - placeholder="Filter" - value={input} - onInput={(e: any) => setInput(e.target.value)} - classes={{ - root: classes.search, - focused: classes.searchFocused, + <form className={classes.form} onSubmit={formSubmit}> + <FormControl> + <Button + aria-haspopup="true" + ref={filterButtonRef} + onClick={(e) => setFilterMenuIsOpen(true)} + > + Filter <ArrowDropDownIcon /> + </Button> + <Menu + open={filterMenuIsOpen} + onClose={() => setFilterMenuIsOpen(false)} + getContentAnchorEl={null} + anchorEl={filterButtonRef.current} + anchorOrigin={{ + vertical: 'bottom', + horizontal: 'left', }} - /> - <button type="submit" hidden> - Search - </button> - </form> - </div> + transformOrigin={{ + vertical: 'top', + horizontal: 'left', + }} + > + <MenuItem + component={Link} + to={pipe( + replaceParam('author', user.displayName), + replaceParam('sort', 'creation'), + loc + )(qparams)} + onClick={() => setFilterMenuIsOpen(false)} + > + Your newest issues + </MenuItem> + </Menu> + </FormControl> + <InputBase + id="issuefilter" + placeholder="Filter" + value={input} + onInput={(e: any) => setInput(e.target.value)} + classes={{ + root: classes.search, + focused: classes.searchFocused, + }} + /> + <button type="submit" hidden> + Search + </button> + </form> <IfLoggedIn> {() => ( <Button className={classes.greenButton} variant="contained" - href="/new" + component={Link} + to="/new" > New bug </Button> diff --git a/webui/types/remark-gemoji/index.d.ts b/webui/types/remark-gemoji/index.d.ts new file mode 100644 index 00000000..f15725b2 --- /dev/null +++ b/webui/types/remark-gemoji/index.d.ts @@ -0,0 +1,6 @@ +declare module 'remark-gemoji' { + import { Plugin } from 'unified'; + + const plugin: Plugin; + export default plugin; +} |