diff options
author | Michael Muré <batolettre@gmail.com> | 2022-11-13 12:31:38 +0100 |
---|---|---|
committer | Michael Muré <batolettre@gmail.com> | 2022-11-13 12:31:38 +0100 |
commit | 3c6ebc2bfd50b72ff786a2cfd3bbdeb15b478dd3 (patch) | |
tree | 8cf4307bf48f016e0f8728c311bcc53ce38432e3 | |
parent | 55a2e8e4485fe63fbda759540958c7190dfeb85c (diff) | |
download | git-bug-3c6ebc2bfd50b72ff786a2cfd3bbdeb15b478dd3.tar.gz |
core: bubble up the comment ID when created, or edited the first comment
-rw-r--r-- | api/graphql/resolvers/mutation.go | 6 | ||||
-rw-r--r-- | bridge/core/export.go | 92 | ||||
-rw-r--r-- | bridge/core/import.go | 107 | ||||
-rw-r--r-- | bridge/github/export.go | 14 | ||||
-rw-r--r-- | bridge/github/export_test.go | 7 | ||||
-rw-r--r-- | bridge/github/import.go | 22 | ||||
-rw-r--r-- | bridge/gitlab/export.go | 16 | ||||
-rw-r--r-- | bridge/gitlab/export_test.go | 7 | ||||
-rw-r--r-- | bridge/gitlab/import.go | 16 | ||||
-rw-r--r-- | bridge/jira/export.go | 14 | ||||
-rw-r--r-- | bridge/jira/import.go | 24 | ||||
-rw-r--r-- | bridge/launchpad/import.go | 4 | ||||
-rw-r--r-- | cache/bug_cache.go | 31 | ||||
-rw-r--r-- | commands/comment_add.go | 2 | ||||
-rw-r--r-- | entities/bug/op_add_comment.go | 6 | ||||
-rw-r--r-- | entities/bug/op_edit_comment.go | 8 | ||||
-rw-r--r-- | entities/bug/op_label_change.go | 1 | ||||
-rw-r--r-- | entities/bug/op_set_status.go | 1 | ||||
-rw-r--r-- | entities/bug/op_set_title.go | 6 | ||||
-rw-r--r-- | misc/random_bugs/create_random_bugs.go | 2 | ||||
-rw-r--r-- | termui/termui.go | 2 |
21 files changed, 196 insertions, 192 deletions
diff --git a/api/graphql/resolvers/mutation.go b/api/graphql/resolvers/mutation.go index f6296d63..3f9f7fe1 100644 --- a/api/graphql/resolvers/mutation.go +++ b/api/graphql/resolvers/mutation.go @@ -78,7 +78,7 @@ func (r mutationResolver) AddComment(ctx context.Context, input models.AddCommen return nil, err } - op, err := b.AddCommentRaw(author, + _, op, err := b.AddCommentRaw(author, time.Now().Unix(), text.Cleanup(input.Message), input.Files, @@ -110,7 +110,7 @@ func (r mutationResolver) AddCommentAndClose(ctx context.Context, input models.A return nil, err } - opAddComment, err := b.AddCommentRaw(author, + _, opAddComment, err := b.AddCommentRaw(author, time.Now().Unix(), text.Cleanup(input.Message), input.Files, @@ -148,7 +148,7 @@ func (r mutationResolver) AddCommentAndReopen(ctx context.Context, input models. return nil, err } - opAddComment, err := b.AddCommentRaw(author, + _, opAddComment, err := b.AddCommentRaw(author, time.Now().Unix(), text.Cleanup(input.Message), input.Files, diff --git a/bridge/core/export.go b/bridge/core/export.go index 6e0612fa..5bf16801 100644 --- a/bridge/core/export.go +++ b/bridge/core/export.go @@ -42,39 +42,39 @@ const ( // allow calling code to report on what is happening, collect metrics or // display meaningful errors if something went wrong. type ExportResult struct { - Err error - Event ExportEvent - ID entity.Id - Reason string + Err error + Event ExportEvent + EntityId entity.Id // optional for err, warning + Reason string } func (er ExportResult) String() string { switch er.Event { case ExportEventBug: - return fmt.Sprintf("new issue: %s", er.ID) + return fmt.Sprintf("[%s] new issue: %s", er.EntityId.Human(), er.EntityId) case ExportEventComment: - return fmt.Sprintf("new comment: %s", er.ID) + return fmt.Sprintf("[%s] new comment", er.EntityId.Human()) case ExportEventCommentEdition: - return fmt.Sprintf("updated comment: %s", er.ID) + return fmt.Sprintf("[%s] updated comment", er.EntityId.Human()) case ExportEventStatusChange: - return fmt.Sprintf("changed status: %s", er.ID) + return fmt.Sprintf("[%s] changed status", er.EntityId.Human()) case ExportEventTitleEdition: - return fmt.Sprintf("changed title: %s", er.ID) + return fmt.Sprintf("[%s] changed title", er.EntityId.Human()) case ExportEventLabelChange: - return fmt.Sprintf("changed label: %s", er.ID) + return fmt.Sprintf("[%s] changed label", er.EntityId.Human()) case ExportEventNothing: - if er.ID != "" { - return fmt.Sprintf("no actions taken for event %s: %s", er.ID, er.Reason) + if er.EntityId != "" { + return fmt.Sprintf("no actions taken on entity %s: %s", er.EntityId, er.Reason) } return fmt.Sprintf("no actions taken: %s", er.Reason) case ExportEventError: - if er.ID != "" { - return fmt.Sprintf("export error at %s: %s", er.ID, er.Err.Error()) + if er.EntityId != "" { + return fmt.Sprintf("export error on entity %s: %s", er.EntityId, er.Err.Error()) } return fmt.Sprintf("export error: %s", er.Err.Error()) case ExportEventWarning: - if er.ID != "" { - return fmt.Sprintf("warning at %s: %s", er.ID, er.Err.Error()) + if er.EntityId != "" { + return fmt.Sprintf("warning on entity %s: %s", er.EntityId, er.Err.Error()) } return fmt.Sprintf("warning: %s", er.Err.Error()) case ExportEventRateLimiting: @@ -85,69 +85,69 @@ func (er ExportResult) String() string { } } -func NewExportError(err error, id entity.Id) ExportResult { +func NewExportError(err error, entityId entity.Id) ExportResult { return ExportResult{ - ID: id, - Err: err, - Event: ExportEventError, + EntityId: entityId, + Err: err, + Event: ExportEventError, } } -func NewExportWarning(err error, id entity.Id) ExportResult { +func NewExportWarning(err error, entityId entity.Id) ExportResult { return ExportResult{ - ID: id, - Err: err, - Event: ExportEventWarning, + EntityId: entityId, + Err: err, + Event: ExportEventWarning, } } -func NewExportNothing(id entity.Id, reason string) ExportResult { +func NewExportNothing(entityId entity.Id, reason string) ExportResult { return ExportResult{ - ID: id, - Reason: reason, - Event: ExportEventNothing, + EntityId: entityId, + Reason: reason, + Event: ExportEventNothing, } } -func NewExportBug(id entity.Id) ExportResult { +func NewExportBug(entityId entity.Id) ExportResult { return ExportResult{ - ID: id, - Event: ExportEventBug, + EntityId: entityId, + Event: ExportEventBug, } } -func NewExportComment(id entity.Id) ExportResult { +func NewExportComment(entityId entity.Id) ExportResult { return ExportResult{ - ID: id, - Event: ExportEventComment, + EntityId: entityId, + Event: ExportEventComment, } } -func NewExportCommentEdition(id entity.Id) ExportResult { +func NewExportCommentEdition(entityId entity.Id) ExportResult { return ExportResult{ - ID: id, - Event: ExportEventCommentEdition, + EntityId: entityId, + Event: ExportEventCommentEdition, } } -func NewExportStatusChange(id entity.Id) ExportResult { +func NewExportStatusChange(entityId entity.Id) ExportResult { return ExportResult{ - ID: id, - Event: ExportEventStatusChange, + EntityId: entityId, + Event: ExportEventStatusChange, } } -func NewExportLabelChange(id entity.Id) ExportResult { +func NewExportLabelChange(entityId entity.Id) ExportResult { return ExportResult{ - ID: id, - Event: ExportEventLabelChange, + EntityId: entityId, + Event: ExportEventLabelChange, } } -func NewExportTitleEdition(id entity.Id) ExportResult { +func NewExportTitleEdition(entityId entity.Id) ExportResult { return ExportResult{ - ID: id, - Event: ExportEventTitleEdition, + EntityId: entityId, + Event: ExportEventTitleEdition, } } diff --git a/bridge/core/import.go b/bridge/core/import.go index c1965d4e..8fab476e 100644 --- a/bridge/core/import.go +++ b/bridge/core/import.go @@ -45,43 +45,45 @@ const ( // allow calling code to report on what is happening, collect metrics or // display meaningful errors if something went wrong. type ImportResult struct { - Err error - Event ImportEvent - ID entity.Id - Reason string + Err error + Event ImportEvent + EntityId entity.Id // optional for err, warnings + OperationId entity.Id // optional + ComponentId entity.CombinedId // optional + Reason string } func (er ImportResult) String() string { switch er.Event { case ImportEventBug: - return fmt.Sprintf("new issue: %s", er.ID) + return fmt.Sprintf("[%s] new issue: %s", er.EntityId.Human(), er.EntityId) case ImportEventComment: - return fmt.Sprintf("new comment: %s", er.ID) + return fmt.Sprintf("[%s] new comment: %s", er.EntityId.Human(), er.ComponentId) case ImportEventCommentEdition: - return fmt.Sprintf("updated comment: %s", er.ID) + return fmt.Sprintf("[%s] updated comment: %s", er.EntityId.Human(), er.ComponentId) case ImportEventStatusChange: - return fmt.Sprintf("changed status: %s", er.ID) + return fmt.Sprintf("[%s] changed status with op: %s", er.EntityId.Human(), er.OperationId) case ImportEventTitleEdition: - return fmt.Sprintf("changed title: %s", er.ID) + return fmt.Sprintf("[%s] changed title with op: %s", er.EntityId.Human(), er.OperationId) case ImportEventLabelChange: - return fmt.Sprintf("changed label: %s", er.ID) + return fmt.Sprintf("[%s] changed label with op: %s", er.EntityId.Human(), er.OperationId) case ImportEventIdentity: - return fmt.Sprintf("new identity: %s", er.ID) + return fmt.Sprintf("[%s] new identity: %s", er.EntityId.Human(), er.EntityId) case ImportEventNothing: - if er.ID != "" { - return fmt.Sprintf("no action taken for event %s: %s", er.ID, er.Reason) + if er.EntityId != "" { + return fmt.Sprintf("no action taken on entity %s: %s", er.EntityId, er.Reason) } return fmt.Sprintf("no action taken: %s", er.Reason) case ImportEventError: - if er.ID != "" { - return fmt.Sprintf("import error at id %s: %s", er.ID, er.Err.Error()) + if er.EntityId != "" { + return fmt.Sprintf("import error on entity %s: %s", er.EntityId, er.Err.Error()) } return fmt.Sprintf("import error: %s", er.Err.Error()) case ImportEventWarning: parts := make([]string, 0, 4) parts = append(parts, "warning:") - if er.ID != "" { - parts = append(parts, fmt.Sprintf("at id %s", er.ID)) + if er.EntityId != "" { + parts = append(parts, fmt.Sprintf("on entity %s", er.EntityId)) } if er.Reason != "" { parts = append(parts, fmt.Sprintf("reason: %s", er.Reason)) @@ -98,76 +100,81 @@ func (er ImportResult) String() string { } } -func NewImportError(err error, id entity.Id) ImportResult { +func NewImportError(err error, entityId entity.Id) ImportResult { return ImportResult{ - Err: err, - ID: id, - Event: ImportEventError, + Err: err, + EntityId: entityId, + Event: ImportEventError, } } -func NewImportWarning(err error, id entity.Id) ImportResult { +func NewImportWarning(err error, entityId entity.Id) ImportResult { return ImportResult{ - Err: err, - ID: id, - Event: ImportEventWarning, + Err: err, + EntityId: entityId, + Event: ImportEventWarning, } } -func NewImportNothing(id entity.Id, reason string) ImportResult { +func NewImportNothing(entityId entity.Id, reason string) ImportResult { return ImportResult{ - ID: id, - Reason: reason, - Event: ImportEventNothing, + EntityId: entityId, + Reason: reason, + Event: ImportEventNothing, } } -func NewImportBug(id entity.Id) ImportResult { +func NewImportBug(entityId entity.Id) ImportResult { return ImportResult{ - ID: id, - Event: ImportEventBug, + EntityId: entityId, + Event: ImportEventBug, } } -func NewImportComment(id entity.Id) ImportResult { +func NewImportComment(entityId entity.Id, commentId entity.CombinedId) ImportResult { return ImportResult{ - ID: id, - Event: ImportEventComment, + EntityId: entityId, + ComponentId: commentId, + Event: ImportEventComment, } } -func NewImportCommentEdition(id entity.Id) ImportResult { +func NewImportCommentEdition(entityId entity.Id, commentId entity.CombinedId) ImportResult { return ImportResult{ - ID: id, - Event: ImportEventCommentEdition, + EntityId: entityId, + ComponentId: commentId, + Event: ImportEventCommentEdition, } } -func NewImportStatusChange(id entity.Id) ImportResult { +func NewImportStatusChange(entityId entity.Id, opId entity.Id) ImportResult { return ImportResult{ - ID: id, - Event: ImportEventStatusChange, + EntityId: entityId, + OperationId: opId, + Event: ImportEventStatusChange, } } -func NewImportLabelChange(id entity.Id) ImportResult { +func NewImportLabelChange(entityId entity.Id, opId entity.Id) ImportResult { return ImportResult{ - ID: id, - Event: ImportEventLabelChange, + EntityId: entityId, + OperationId: opId, + Event: ImportEventLabelChange, } } -func NewImportTitleEdition(id entity.Id) ImportResult { +func NewImportTitleEdition(entityId entity.Id, opId entity.Id) ImportResult { return ImportResult{ - ID: id, - Event: ImportEventTitleEdition, + EntityId: entityId, + OperationId: opId, + Event: ImportEventTitleEdition, } } -func NewImportIdentity(id entity.Id) ImportResult { +func NewImportIdentity(entityId entity.Id) ImportResult { return ImportResult{ - ID: id, - Event: ImportEventIdentity, + EntityId: entityId, + Event: ImportEventIdentity, } } diff --git a/bridge/github/export.go b/bridge/github/export.go index 447e4342..675ed039 100644 --- a/bridge/github/export.go +++ b/bridge/github/export.go @@ -318,7 +318,7 @@ func (ge *githubExporter) exportBug(ctx context.Context, b *cache.BugCache, out return } - out <- core.NewExportComment(op.Id()) + out <- core.NewExportComment(b.Id()) // cache comment id ge.cachedOperationIDs[op.Id()] = id @@ -334,7 +334,7 @@ func (ge *githubExporter) exportBug(ctx context.Context, b *cache.BugCache, out return } - out <- core.NewExportCommentEdition(op.Id()) + out <- core.NewExportCommentEdition(b.Id()) id = bugGithubID url = bugGithubURL @@ -354,7 +354,7 @@ func (ge *githubExporter) exportBug(ctx context.Context, b *cache.BugCache, out return } - out <- core.NewExportCommentEdition(op.Id()) + out <- core.NewExportCommentEdition(b.Id()) // use comment id/url instead of issue id/url id = eid @@ -368,7 +368,7 @@ func (ge *githubExporter) exportBug(ctx context.Context, b *cache.BugCache, out return } - out <- core.NewExportStatusChange(op.Id()) + out <- core.NewExportStatusChange(b.Id()) id = bugGithubID url = bugGithubURL @@ -380,7 +380,7 @@ func (ge *githubExporter) exportBug(ctx context.Context, b *cache.BugCache, out return } - out <- core.NewExportTitleEdition(op.Id()) + out <- core.NewExportTitleEdition(b.Id()) id = bugGithubID url = bugGithubURL @@ -392,7 +392,7 @@ func (ge *githubExporter) exportBug(ctx context.Context, b *cache.BugCache, out return } - out <- core.NewExportLabelChange(op.Id()) + out <- core.NewExportLabelChange(b.Id()) id = bugGithubID url = bugGithubURL @@ -659,7 +659,7 @@ func (ge *githubExporter) createGithubIssue(ctx context.Context, gc *rateLimitHa return issue.ID, issue.URL, nil } -// add a comment to an issue and return it ID +// add a comment to an issue and return its ID func (ge *githubExporter) addCommentGithubIssue(ctx context.Context, gc *rateLimitHandlerClient, subjectID string, body string) (string, string, error) { m := &addCommentToIssueMutation{} input := githubv4.AddCommentInput{ diff --git a/bridge/github/export_test.go b/bridge/github/export_test.go index de2d3f34..2ebe9622 100644 --- a/bridge/github/export_test.go +++ b/bridge/github/export_test.go @@ -41,7 +41,7 @@ func testCases(t *testing.T, repo *cache.RepoCache) []*testCase { bugWithComments, _, err := repo.NewBug("bug with comments", "new bug") require.NoError(t, err) - _, err = bugWithComments.AddComment("new comment") + _, _, err = bugWithComments.AddComment("new comment") require.NoError(t, err) // bug with label changes @@ -71,11 +71,10 @@ func testCases(t *testing.T, repo *cache.RepoCache) []*testCase { entity.CombineIds(bugWithCommentEditions.Id(), createOp.Id()), "first comment edited") require.NoError(t, err) - commentOp, err := bugWithCommentEditions.AddComment("first comment") + commentId, _, err := bugWithCommentEditions.AddComment("first comment") require.NoError(t, err) - _, err = bugWithCommentEditions.EditComment( - entity.CombineIds(bugWithCommentEditions.Id(), commentOp.Id()), "first comment edited") + _, err = bugWithCommentEditions.EditComment(commentId, "first comment edited") require.NoError(t, err) // bug status changed diff --git a/bridge/github/import.go b/bridge/github/import.go index 5b2bf54b..7ccac3fb 100644 --- a/bridge/github/import.go +++ b/bridge/github/import.go @@ -274,7 +274,7 @@ func (gi *githubImporter) ensureTimelineItem(ctx context.Context, repo *cache.Re return err } - gi.out <- core.NewImportLabelChange(op.Id()) + gi.out <- core.NewImportLabelChange(b.Id(), op.Id()) return nil case "UnlabeledEvent": @@ -304,7 +304,7 @@ func (gi *githubImporter) ensureTimelineItem(ctx context.Context, repo *cache.Re return err } - gi.out <- core.NewImportLabelChange(op.Id()) + gi.out <- core.NewImportLabelChange(b.Id(), op.Id()) return nil case "ClosedEvent": @@ -330,7 +330,7 @@ func (gi *githubImporter) ensureTimelineItem(ctx context.Context, repo *cache.Re return err } - gi.out <- core.NewImportStatusChange(op.Id()) + gi.out <- core.NewImportStatusChange(b.Id(), op.Id()) return nil case "ReopenedEvent": @@ -356,7 +356,7 @@ func (gi *githubImporter) ensureTimelineItem(ctx context.Context, repo *cache.Re return err } - gi.out <- core.NewImportStatusChange(op.Id()) + gi.out <- core.NewImportStatusChange(b.Id(), op.Id()) return nil case "RenamedTitleEvent": @@ -392,7 +392,7 @@ func (gi *githubImporter) ensureTimelineItem(ctx context.Context, repo *cache.Re return err } - gi.out <- core.NewImportTitleEdition(op.Id()) + gi.out <- core.NewImportTitleEdition(b.Id(), op.Id()) return nil } @@ -425,11 +425,13 @@ func (gi *githubImporter) ensureCommentEdit(ctx context.Context, repo *cache.Rep return nil } + commentId := entity.CombineIds(b.Id(), target) + // comment edition - op, err := b.EditCommentRaw( + _, err = b.EditCommentRaw( editor, edit.CreatedAt.Unix(), - entity.CombineIds(b.Id(), target), + commentId, text.Cleanup(string(*edit.Diff)), map[string]string{ metaKeyGithubId: parseId(edit.Id), @@ -440,7 +442,7 @@ func (gi *githubImporter) ensureCommentEdit(ctx context.Context, repo *cache.Rep return err } - gi.out <- core.NewImportCommentEdition(op.Id()) + gi.out <- core.NewImportCommentEdition(b.Id(), commentId) return nil } @@ -469,7 +471,7 @@ func (gi *githubImporter) ensureComment(ctx context.Context, repo *cache.RepoCac } // add comment operation - op, err := b.AddCommentRaw( + commentId, _, err := b.AddCommentRaw( author, comment.CreatedAt.Unix(), text.Cleanup(textInput), @@ -483,7 +485,7 @@ func (gi *githubImporter) ensureComment(ctx context.Context, repo *cache.RepoCac return err } - gi.out <- core.NewImportComment(op.Id()) + gi.out <- core.NewImportComment(b.Id(), commentId) return nil } diff --git a/bridge/gitlab/export.go b/bridge/gitlab/export.go index 04972455..83465428 100644 --- a/bridge/gitlab/export.go +++ b/bridge/gitlab/export.go @@ -288,7 +288,7 @@ func (ge *gitlabExporter) exportBug(ctx context.Context, b *cache.BugCache, out return } - out <- core.NewExportComment(op.Id()) + out <- core.NewExportComment(b.Id()) idString = strconv.Itoa(id) // cache comment id @@ -307,7 +307,7 @@ func (ge *gitlabExporter) exportBug(ctx context.Context, b *cache.BugCache, out return } - out <- core.NewExportCommentEdition(op.Id()) + out <- core.NewExportCommentEdition(b.Id()) id = bugGitlabID } else { @@ -315,13 +315,13 @@ func (ge *gitlabExporter) exportBug(ctx context.Context, b *cache.BugCache, out // case comment edition operation: we need to edit the Gitlab comment commentID, ok := ge.cachedOperationIDs[targetId] if !ok { - out <- core.NewExportError(fmt.Errorf("unexpected error: comment id not found"), op.Target) + out <- core.NewExportError(fmt.Errorf("unexpected error: comment id not found"), b.Id()) return } commentIDint, err := strconv.Atoi(commentID) if err != nil { - out <- core.NewExportError(fmt.Errorf("unexpected comment id format"), op.Target) + out <- core.NewExportError(fmt.Errorf("unexpected comment id format"), b.Id()) return } @@ -331,7 +331,7 @@ func (ge *gitlabExporter) exportBug(ctx context.Context, b *cache.BugCache, out return } - out <- core.NewExportCommentEdition(op.Id()) + out <- core.NewExportCommentEdition(b.Id()) id = commentIDint } @@ -342,7 +342,7 @@ func (ge *gitlabExporter) exportBug(ctx context.Context, b *cache.BugCache, out return } - out <- core.NewExportStatusChange(op.Id()) + out <- core.NewExportStatusChange(b.Id()) id = bugGitlabID case *bug.SetTitleOperation: @@ -352,7 +352,7 @@ func (ge *gitlabExporter) exportBug(ctx context.Context, b *cache.BugCache, out return } - out <- core.NewExportTitleEdition(op.Id()) + out <- core.NewExportTitleEdition(b.Id()) id = bugGitlabID case *bug.LabelChangeOperation: @@ -378,7 +378,7 @@ func (ge *gitlabExporter) exportBug(ctx context.Context, b *cache.BugCache, out return } - out <- core.NewExportLabelChange(op.Id()) + out <- core.NewExportLabelChange(b.Id()) id = bugGitlabID default: panic("unhandled operation type case") diff --git a/bridge/gitlab/export_test.go b/bridge/gitlab/export_test.go index e9f3ae75..47d5a9b1 100644 --- a/bridge/gitlab/export_test.go +++ b/bridge/gitlab/export_test.go @@ -44,7 +44,7 @@ func testCases(t *testing.T, repo *cache.RepoCache) []*testCase { bugWithComments, _, err := repo.NewBug("bug with comments", "new bug") require.NoError(t, err) - _, err = bugWithComments.AddComment("new comment") + _, _, err = bugWithComments.AddComment("new comment") require.NoError(t, err) // bug with label changes @@ -68,11 +68,10 @@ func testCases(t *testing.T, repo *cache.RepoCache) []*testCase { entity.CombineIds(bugWithCommentEditions.Id(), createOp.Id()), "first comment edited") require.NoError(t, err) - commentOp, err := bugWithCommentEditions.AddComment("first comment") + commentId, _, err := bugWithCommentEditions.AddComment("first comment") require.NoError(t, err) - _, err = bugWithCommentEditions.EditComment( - entity.CombineIds(bugWithCommentEditions.Id(), commentOp.Id()), "first comment edited") + _, err = bugWithCommentEditions.EditComment(commentId, "first comment edited") require.NoError(t, err) // bug status changed diff --git a/bridge/gitlab/import.go b/bridge/gitlab/import.go index cf6b5ca6..c7909c8f 100644 --- a/bridge/gitlab/import.go +++ b/bridge/gitlab/import.go @@ -178,7 +178,7 @@ func (gi *gitlabImporter) ensureIssueEvent(repo *cache.RepoCache, b *cache.BugCa return err } - gi.out <- core.NewImportStatusChange(op.Id()) + gi.out <- core.NewImportStatusChange(b.Id(), op.Id()) case EventReopened: if errResolve == nil { @@ -196,7 +196,7 @@ func (gi *gitlabImporter) ensureIssueEvent(repo *cache.RepoCache, b *cache.BugCa return err } - gi.out <- core.NewImportStatusChange(op.Id()) + gi.out <- core.NewImportStatusChange(b.Id(), op.Id()) case EventDescriptionChanged: firstComment := b.Snapshot().Comments[0] @@ -219,7 +219,7 @@ func (gi *gitlabImporter) ensureIssueEvent(repo *cache.RepoCache, b *cache.BugCa return err } - gi.out <- core.NewImportTitleEdition(op.Id()) + gi.out <- core.NewImportTitleEdition(b.Id(), op.Id()) } case EventComment: @@ -229,7 +229,7 @@ func (gi *gitlabImporter) ensureIssueEvent(repo *cache.RepoCache, b *cache.BugCa if errResolve == cache.ErrNoMatchingOp { // add comment operation - op, err := b.AddCommentRaw( + commentId, _, err := b.AddCommentRaw( author, event.CreatedAt().Unix(), cleanText, @@ -241,7 +241,7 @@ func (gi *gitlabImporter) ensureIssueEvent(repo *cache.RepoCache, b *cache.BugCa if err != nil { return err } - gi.out <- core.NewImportComment(op.Id()) + gi.out <- core.NewImportComment(b.Id(), commentId) return nil } @@ -256,7 +256,7 @@ func (gi *gitlabImporter) ensureIssueEvent(repo *cache.RepoCache, b *cache.BugCa // compare local bug comment with the new event body if comment.Message != cleanText { // comment edition - op, err := b.EditCommentRaw( + _, err := b.EditCommentRaw( author, event.(NoteEvent).UpdatedAt.Unix(), comment.CombinedId(), @@ -267,7 +267,7 @@ func (gi *gitlabImporter) ensureIssueEvent(repo *cache.RepoCache, b *cache.BugCa if err != nil { return err } - gi.out <- core.NewImportCommentEdition(op.Id()) + gi.out <- core.NewImportCommentEdition(b.Id(), comment.CombinedId()) } return nil @@ -290,7 +290,7 @@ func (gi *gitlabImporter) ensureIssueEvent(repo *cache.RepoCache, b *cache.BugCa return err } - gi.out <- core.NewImportTitleEdition(op.Id()) + gi.out <- core.NewImportTitleEdition(b.Id(), op.Id()) case EventAddLabel: _, err = b.ForceChangeLabelsRaw( diff --git a/bridge/jira/export.go b/bridge/jira/export.go index 763d6a1c..8587a55d 100644 --- a/bridge/jira/export.go +++ b/bridge/jira/export.go @@ -315,7 +315,7 @@ func (je *jiraExporter) exportBug(ctx context.Context, b *cache.BugCache, out ch if err != nil { out <- core.NewExportError( fmt.Errorf("missing operation author credentials for user %.8s", - author.Id().String()), op.Id()) + author.Id().String()), b.Id()) continue } @@ -330,7 +330,7 @@ func (je *jiraExporter) exportBug(ctx context.Context, b *cache.BugCache, out ch return err } id = comment.ID - out <- core.NewExportComment(op.Id()) + out <- core.NewExportComment(b.Id()) // cache comment id je.cachedOperationIDs[op.Id()] = id @@ -345,7 +345,7 @@ func (je *jiraExporter) exportBug(ctx context.Context, b *cache.BugCache, out ch out <- core.NewExportError(err, b.Id()) return err } - out <- core.NewExportCommentEdition(op.Id()) + out <- core.NewExportCommentEdition(b.Id()) id = bugJiraID } else { // Otherwise it's an edit to an actual comment. A comment cannot be @@ -363,7 +363,7 @@ func (je *jiraExporter) exportBug(ctx context.Context, b *cache.BugCache, out ch out <- core.NewExportError(err, b.Id()) return err } - out <- core.NewExportCommentEdition(op.Id()) + out <- core.NewExportCommentEdition(b.Id()) // JIRA doesn't track all comment edits, they will only tell us about // the most recent one. We must invent a consistent id for the operation // so we use the comment ID plus the timestamp of the update, as @@ -384,7 +384,7 @@ func (je *jiraExporter) exportBug(ctx context.Context, b *cache.BugCache, out ch // update. In this case, just don't export the operation. continue } - out <- core.NewExportStatusChange(op.Id()) + out <- core.NewExportStatusChange(b.Id()) id = bugJiraID } else { out <- core.NewExportError(fmt.Errorf( @@ -398,7 +398,7 @@ func (je *jiraExporter) exportBug(ctx context.Context, b *cache.BugCache, out ch out <- core.NewExportError(err, b.Id()) return err } - out <- core.NewExportTitleEdition(op.Id()) + out <- core.NewExportTitleEdition(b.Id()) id = bugJiraID case *bug.LabelChangeOperation: @@ -409,7 +409,7 @@ func (je *jiraExporter) exportBug(ctx context.Context, b *cache.BugCache, out ch out <- core.NewExportError(err, b.Id()) return err } - out <- core.NewExportLabelChange(op.Id()) + out <- core.NewExportLabelChange(b.Id()) id = bugJiraID default: diff --git a/bridge/jira/import.go b/bridge/jira/import.go index 8043acf4..ff9fbb7a 100644 --- a/bridge/jira/import.go +++ b/bridge/jira/import.go @@ -286,7 +286,7 @@ func (ji *jiraImporter) ensureComment(repo *cache.RepoCache, b *cache.BugCache, } // add comment operation - op, err := b.AddCommentRaw( + commentId, op, err := b.AddCommentRaw( author, item.Created.Unix(), cleanText, @@ -299,7 +299,7 @@ func (ji *jiraImporter) ensureComment(repo *cache.RepoCache, b *cache.BugCache, return err } - ji.out <- core.NewImportComment(op.Id()) + ji.out <- core.NewImportComment(b.Id(), commentId) targetOpID = op.Id() } @@ -329,11 +329,13 @@ func (ji *jiraImporter) ensureComment(repo *cache.RepoCache, b *cache.BugCache, return err } + commentId := entity.CombineIds(b.Id(), targetOpID) + // comment edition - op, err := b.EditCommentRaw( + _, err = b.EditCommentRaw( editor, item.Updated.Unix(), - entity.CombineIds(b.Id(), targetOpID), + commentId, text.Cleanup(item.Body), map[string]string{ metaKeyJiraId: derivedID, @@ -344,7 +346,7 @@ func (ji *jiraImporter) ensureComment(repo *cache.RepoCache, b *cache.BugCache, return err } - ji.out <- core.NewImportCommentEdition(op.Id()) + ji.out <- core.NewImportCommentEdition(b.Id(), commentId) return nil } @@ -510,7 +512,7 @@ func (ji *jiraImporter) ensureChange(repo *cache.RepoCache, b *cache.BugCache, e return err } - ji.out <- core.NewImportLabelChange(op.Id()) + ji.out <- core.NewImportLabelChange(b.Id(), op.Id()) case "status": statusStr, hasMap := statusMap[item.To] @@ -528,7 +530,7 @@ func (ji *jiraImporter) ensureChange(repo *cache.RepoCache, b *cache.BugCache, e if err != nil { return err } - ji.out <- core.NewImportStatusChange(op.Id()) + ji.out <- core.NewImportStatusChange(b.Id(), op.Id()) case common.ClosedStatus.String(): op, err := b.CloseRaw( @@ -542,7 +544,7 @@ func (ji *jiraImporter) ensureChange(repo *cache.RepoCache, b *cache.BugCache, e if err != nil { return err } - ji.out <- core.NewImportStatusChange(op.Id()) + ji.out <- core.NewImportStatusChange(b.Id(), op.Id()) } } else { ji.out <- core.NewImportError( @@ -567,12 +569,12 @@ func (ji *jiraImporter) ensureChange(repo *cache.RepoCache, b *cache.BugCache, e return err } - ji.out <- core.NewImportTitleEdition(op.Id()) + ji.out <- core.NewImportTitleEdition(b.Id(), op.Id()) case "description": // NOTE(josh): JIRA calls it "description", which sounds more like the // title but it's actually the body - op, err := b.EditCreateCommentRaw( + commentId, _, err := b.EditCreateCommentRaw( author, entry.Created.Unix(), text.Cleanup(item.ToString), @@ -585,7 +587,7 @@ func (ji *jiraImporter) ensureChange(repo *cache.RepoCache, b *cache.BugCache, e return err } - ji.out <- core.NewImportCommentEdition(op.Id()) + ji.out <- core.NewImportCommentEdition(b.Id(), commentId) default: ji.out <- core.NewImportWarning( diff --git a/bridge/launchpad/import.go b/bridge/launchpad/import.go index 598ef80b..f81e3582 100644 --- a/bridge/launchpad/import.go +++ b/bridge/launchpad/import.go @@ -131,7 +131,7 @@ func (li *launchpadImporter) ImportAll(ctx context.Context, repo *cache.RepoCach // This is a new comment, we can add it. createdAt, _ := time.Parse(time.RFC3339, lpMessage.CreatedAt) - op, err := b.AddCommentRaw( + commentId, _, err := b.AddCommentRaw( owner, createdAt.Unix(), text.Cleanup(lpMessage.Content), @@ -144,7 +144,7 @@ func (li *launchpadImporter) ImportAll(ctx context.Context, repo *cache.RepoCach return } - out <- core.NewImportComment(op.Id()) + out <- core.NewImportComment(b.Id(), commentId) } if !b.NeedCommit() { diff --git a/cache/bug_cache.go b/cache/bug_cache.go index 6caaeb11..65e2068f 100644 --- a/cache/bug_cache.go +++ b/cache/bug_cache.go @@ -70,27 +70,27 @@ func (c *BugCache) ResolveOperationWithMetadata(key string, value string) (entit return matching[0], nil } -func (c *BugCache) AddComment(message string) (*bug.AddCommentOperation, error) { +func (c *BugCache) AddComment(message string) (entity.CombinedId, *bug.AddCommentOperation, error) { return c.AddCommentWithFiles(message, nil) } -func (c *BugCache) AddCommentWithFiles(message string, files []repository.Hash) (*bug.AddCommentOperation, error) { +func (c *BugCache) AddCommentWithFiles(message string, files []repository.Hash) (entity.CombinedId, *bug.AddCommentOperation, error) { author, err := c.repoCache.GetUserIdentity() if err != nil { - return nil, err + return entity.UnsetCombinedId, nil, err } return c.AddCommentRaw(author, time.Now().Unix(), message, files, nil) } -func (c *BugCache) AddCommentRaw(author *IdentityCache, unixTime int64, message string, files []repository.Hash, metadata map[string]string) (*bug.AddCommentOperation, error) { +func (c *BugCache) AddCommentRaw(author *IdentityCache, unixTime int64, message string, files []repository.Hash, metadata map[string]string) (entity.CombinedId, *bug.AddCommentOperation, error) { c.mu.Lock() - op, err := bug.AddComment(c.bug, author, unixTime, message, files, metadata) + commentId, op, err := bug.AddComment(c.bug, author, unixTime, message, files, metadata) c.mu.Unlock() if err != nil { - return nil, err + return entity.UnsetCombinedId, nil, err } - return op, c.notifyUpdated() + return commentId, op, c.notifyUpdated() } func (c *BugCache) ChangeLabels(added []string, removed []string) ([]bug.LabelChangeResult, *bug.LabelChangeOperation, error) { @@ -189,24 +189,24 @@ func (c *BugCache) SetTitleRaw(author *IdentityCache, unixTime int64, title stri } // EditCreateComment is a convenience function to edit the body of a bug (the first comment) -func (c *BugCache) EditCreateComment(body string) (*bug.EditCommentOperation, error) { +func (c *BugCache) EditCreateComment(body string) (entity.CombinedId, *bug.EditCommentOperation, error) { author, err := c.repoCache.GetUserIdentity() if err != nil { - return nil, err + return entity.UnsetCombinedId, nil, err } return c.EditCreateCommentRaw(author, time.Now().Unix(), body, nil) } // EditCreateCommentRaw is a 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) { +func (c *BugCache) EditCreateCommentRaw(author *IdentityCache, unixTime int64, body string, metadata map[string]string) (entity.CombinedId, *bug.EditCommentOperation, error) { c.mu.Lock() - op, err := bug.EditCreateComment(c.bug, author.Identity, unixTime, body, nil, metadata) + commentId, op, err := bug.EditCreateComment(c.bug, author.Identity, unixTime, body, nil, metadata) c.mu.Unlock() if err != nil { - return nil, err + return entity.UnsetCombinedId, nil, err } - return op, c.notifyUpdated() + return commentId, op, c.notifyUpdated() } func (c *BugCache) EditComment(target entity.CombinedId, message string) (*bug.EditCommentOperation, error) { @@ -225,11 +225,14 @@ func (c *BugCache) EditCommentRaw(author *IdentityCache, unixTime int64, target } c.mu.Lock() - op, err := bug.EditComment(c.bug, author.Identity, unixTime, comment.TargetId(), message, nil, metadata) + commentId, op, err := bug.EditComment(c.bug, author.Identity, unixTime, comment.TargetId(), message, nil, metadata) c.mu.Unlock() if err != nil { return nil, err } + if commentId != target { + panic("EditComment returned unexpected comment id") + } return op, c.notifyUpdated() } diff --git a/commands/comment_add.go b/commands/comment_add.go index e415763b..acac7994 100644 --- a/commands/comment_add.go +++ b/commands/comment_add.go @@ -69,7 +69,7 @@ func runCommentAdd(env *Env, opts commentAddOptions, args []string) error { } } - _, err = b.AddComment(text.Cleanup(opts.message)) + _, _, err = b.AddComment(text.Cleanup(opts.message)) if err != nil { return err } diff --git a/entities/bug/op_add_comment.go b/entities/bug/op_add_comment.go index b049ef16..5edef9d0 100644 --- a/entities/bug/op_add_comment.go +++ b/entities/bug/op_add_comment.go @@ -83,14 +83,14 @@ type AddCommentTimelineItem struct { func (a *AddCommentTimelineItem) IsAuthored() {} // AddComment is a convenience function to add a comment to a bug -func AddComment(b Interface, author identity.Interface, unixTime int64, message string, files []repository.Hash, metadata map[string]string) (*AddCommentOperation, error) { +func AddComment(b Interface, author identity.Interface, unixTime int64, message string, files []repository.Hash, metadata map[string]string) (entity.CombinedId, *AddCommentOperation, error) { op := NewAddCommentOp(author, unixTime, message, files) for key, val := range metadata { op.SetMetadata(key, val) } if err := op.Validate(); err != nil { - return nil, err + return entity.UnsetCombinedId, nil, err } b.Append(op) - return op, nil + return entity.CombineIds(b.Id(), op.Id()), op, nil } diff --git a/entities/bug/op_edit_comment.go b/entities/bug/op_edit_comment.go index b0897b0a..6ac09fd7 100644 --- a/entities/bug/op_edit_comment.go +++ b/entities/bug/op_edit_comment.go @@ -111,20 +111,20 @@ func NewEditCommentOp(author identity.Interface, unixTime int64, target entity.I } // EditComment is a convenience function to apply the operation -func EditComment(b Interface, author identity.Interface, unixTime int64, target entity.Id, message string, files []repository.Hash, metadata map[string]string) (*EditCommentOperation, error) { +func EditComment(b Interface, author identity.Interface, unixTime int64, target entity.Id, message string, files []repository.Hash, metadata map[string]string) (entity.CombinedId, *EditCommentOperation, error) { op := NewEditCommentOp(author, unixTime, target, message, files) for key, val := range metadata { op.SetMetadata(key, val) } if err := op.Validate(); err != nil { - return nil, err + return entity.UnsetCombinedId, nil, err } b.Append(op) - return op, nil + return entity.CombineIds(b.Id(), target), op, nil } // EditCreateComment is a convenience function to edit the body of a bug (the first comment) -func EditCreateComment(b Interface, author identity.Interface, unixTime int64, message string, files []repository.Hash, metadata map[string]string) (*EditCommentOperation, error) { +func EditCreateComment(b Interface, author identity.Interface, unixTime int64, message string, files []repository.Hash, metadata map[string]string) (entity.CombinedId, *EditCommentOperation, error) { createOp := b.FirstOp().(*CreateOperation) return EditComment(b, author, unixTime, createOp.Id(), message, files, metadata) } diff --git a/entities/bug/op_label_change.go b/entities/bug/op_label_change.go index 76b2ebef..0d13fe9e 100644 --- a/entities/bug/op_label_change.go +++ b/entities/bug/op_label_change.go @@ -105,7 +105,6 @@ func NewLabelChangeOperation(author identity.Interface, unixTime int64, added, r } type LabelChangeTimelineItem struct { - // id entity.Id combinedId entity.CombinedId Author identity.Interface UnixTime timestamp.Timestamp diff --git a/entities/bug/op_set_status.go b/entities/bug/op_set_status.go index 23be59a0..68199129 100644 --- a/entities/bug/op_set_status.go +++ b/entities/bug/op_set_status.go @@ -58,7 +58,6 @@ func NewSetStatusOp(author identity.Interface, unixTime int64, status common.Sta } type SetStatusTimelineItem struct { - // id entity.Id combinedId entity.CombinedId Author identity.Interface UnixTime timestamp.Timestamp diff --git a/entities/bug/op_set_title.go b/entities/bug/op_set_title.go index e9526b64..6e445aa6 100644 --- a/entities/bug/op_set_title.go +++ b/entities/bug/op_set_title.go @@ -30,7 +30,6 @@ func (op *SetTitleOperation) Apply(snapshot *Snapshot) { id := op.Id() item := &SetTitleTimelineItem{ - id: id, combinedId: entity.CombineIds(snapshot.Id(), id), Author: op.Author(), UnixTime: timestamp.Timestamp(op.UnixTime), @@ -70,7 +69,6 @@ func NewSetTitleOp(author identity.Interface, unixTime int64, title string, was } type SetTitleTimelineItem struct { - id entity.Id combinedId entity.CombinedId Author identity.Interface UnixTime timestamp.Timestamp @@ -78,10 +76,6 @@ type SetTitleTimelineItem struct { Was string } -func (s SetTitleTimelineItem) Id() entity.Id { - return s.id -} - func (s SetTitleTimelineItem) CombinedId() entity.CombinedId { return s.combinedId } diff --git a/misc/random_bugs/create_random_bugs.go b/misc/random_bugs/create_random_bugs.go index 23192bd5..7e94b61a 100644 --- a/misc/random_bugs/create_random_bugs.go +++ b/misc/random_bugs/create_random_bugs.go @@ -144,7 +144,7 @@ func paragraphs() string { } func comment(b bug.Interface, p identity.Interface, timestamp int64) { - _, _ = bug.AddComment(b, p, timestamp, paragraphs(), nil, nil) + _, _, _ = bug.AddComment(b, p, timestamp, paragraphs(), nil, nil) } func title(b bug.Interface, p identity.Interface, timestamp int64) { diff --git a/termui/termui.go b/termui/termui.go index 15476d89..4dd6e27d 100644 --- a/termui/termui.go +++ b/termui/termui.go @@ -239,7 +239,7 @@ func addCommentWithEditor(bug *cache.BugCache) error { if err == input.ErrEmptyMessage { ui.msgPopup.Activate(msgPopupErrorTitle, "Empty message, aborting.") } else { - _, err := bug.AddComment(text.Cleanup(message)) + _, _, err := bug.AddComment(text.Cleanup(message)) if err != nil { return err } |