refactor(domain): build approved issues in the domain
- commit
- 1486f6d6d03f5a3fc490e01f04a83b1581cf421f
- author
- brice · · 2026-10-19 08:53 UTC
- parent
- 0284609
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
8 files · +74 -38
M internal/adapters/kvstore/issues_pending.go +3 -14
| @@ -29,9 +29,6 @@ const ( | |||
| 29 | 29 | banPrefix = "isban/" // isban/<hex of the hash> | |
| 30 | 30 | issueKeyKey = "meta/issuekey" | |
| 31 | 31 | issueKeyLen = 32 | |
| 32 | - | ||
| 33 | - | // anonymousAuthor labels an approved issue whose submitter gave no name. | |
| 34 | - | anonymousAuthor = "anonymous" | |
| 35 | 32 | ) | |
| 36 | 33 | ||
| 37 | 34 | // PendingIssues is the ermite-kv backed ports.PendingIssueStore. It shares the | |
| @@ -219,9 +216,7 @@ func (s *PendingIssues) Count(ctx context.Context) (int, error) { | |||
| 219 | 216 | // Approve numbers the pending issue from mrseq/<repo> (see createNumbered), | |
| 220 | 217 | // writes the open issue and its index key and deletes the pending record, in | |
| 221 | 218 | // one batch that requires the pending record to be unchanged. | |
| 222 | - | // | |
| 223 | - | //hexcheck:ignore suspicious-business-logic-in-adapter takes the next number under a version precondition: a storage concern, no policy | |
| 224 | - | func (s *PendingIssues) Approve(ctx context.Context, repo domain.RepoName, id domain.PendingIssueID) (domain.Issue, error) { | |
| 219 | + | func (s *PendingIssues) Approve(ctx context.Context, repo domain.RepoName, id domain.PendingIssueID, now time.Time) (domain.Issue, error) { | |
| 225 | 220 | var number int64 | |
| 226 | 221 | err := createNumbered(ctx, s.db, s.locks, repo, func(b *kv.Batch, n int64) error { | |
| 227 | 222 | key := pendingKey(repo, id) | |
| @@ -233,14 +228,8 @@ func (s *PendingIssues) Approve(ctx context.Context, repo domain.RepoName, id do | |||
| 233 | 228 | if err != nil { | |
| 234 | 229 | return err | |
| 235 | 230 | } | |
| 236 | - | author := p.Author | |
| 237 | - | if author == "" { | |
| 238 | - | author = anonymousAuthor | |
| 239 | - | } | |
| 240 | - | issue := domain.Issue{ | |
| 241 | - | Repo: repo, Number: n, Title: p.Title, Body: p.Body, State: domain.IssueStateOpen, | |
| 242 | - | Author: author, CreatedAt: p.CreatedAt, UpdatedAt: p.CreatedAt, External: true, | |
| 243 | - | } | |
| 231 | + | issue := domain.ApprovedIssue(p, now) | |
| 232 | + | issue.Repo, issue.Number = repo, n | |
| 244 | 233 | val, err := encodeIssue(issue) | |
| 245 | 234 | if err != nil { | |
| 246 | 235 | return err | |
| @@ -29,9 +29,6 @@ const ( | |||||
| 29 | banPrefix = "isban/" // isban/<hex of the hash> | 29 | banPrefix = "isban/" // isban/<hex of the hash> | ||
| 30 | issueKeyKey = "meta/issuekey" | 30 | issueKeyKey = "meta/issuekey" | ||
| 31 | issueKeyLen = 32 | 31 | issueKeyLen = 32 | ||
| 32 | - | ||||
| 33 | - | // anonymousAuthor labels an approved issue whose submitter gave no name. | |||
| 34 | - | anonymousAuthor = "anonymous" | |||
| 35 | ) | 32 | ) | ||
| 36 | 33 | ||||
| 37 | // PendingIssues is the ermite-kv backed ports.PendingIssueStore. It shares the | 34 | // PendingIssues is the ermite-kv backed ports.PendingIssueStore. It shares the | ||
| @@ -219,9 +216,7 @@ func (s *PendingIssues) Count(ctx context.Context) (int, error) { | |||||
| 219 | // Approve numbers the pending issue from mrseq/<repo> (see createNumbered), | 216 | // Approve numbers the pending issue from mrseq/<repo> (see createNumbered), | ||
| 220 | // writes the open issue and its index key and deletes the pending record, in | 217 | // writes the open issue and its index key and deletes the pending record, in | ||
| 221 | // one batch that requires the pending record to be unchanged. | 218 | // one batch that requires the pending record to be unchanged. | ||
| 222 | - | // | 219 | + | func (s *PendingIssues) Approve(ctx context.Context, repo domain.RepoName, id domain.PendingIssueID, now time.Time) (domain.Issue, error) { |
| 223 | - | //hexcheck:ignore suspicious-business-logic-in-adapter takes the next number under a version precondition: a storage concern, no policy | |||
| 224 | - | func (s *PendingIssues) Approve(ctx context.Context, repo domain.RepoName, id domain.PendingIssueID) (domain.Issue, error) { | |||
| 225 | var number int64 | 220 | var number int64 | ||
| 226 | err := createNumbered(ctx, s.db, s.locks, repo, func(b *kv.Batch, n int64) error { | 221 | err := createNumbered(ctx, s.db, s.locks, repo, func(b *kv.Batch, n int64) error { | ||
| 227 | key := pendingKey(repo, id) | 222 | key := pendingKey(repo, id) | ||
| @@ -233,14 +228,8 @@ func (s *PendingIssues) Approve(ctx context.Context, repo domain.RepoName, id do | |||||
| 233 | if err != nil { | 228 | if err != nil { | ||
| 234 | return err | 229 | return err | ||
| 235 | } | 230 | } | ||
| 236 | - | author := p.Author | 231 | + | issue := domain.ApprovedIssue(p, now) |
| 237 | - | if author == "" { | 232 | + | issue.Repo, issue.Number = repo, n |
| 238 | - | author = anonymousAuthor | |||
| 239 | - | } | |||
| 240 | - | issue := domain.Issue{ | |||
| 241 | - | Repo: repo, Number: n, Title: p.Title, Body: p.Body, State: domain.IssueStateOpen, | |||
| 242 | - | Author: author, CreatedAt: p.CreatedAt, UpdatedAt: p.CreatedAt, External: true, | |||
| 243 | - | } | |||
| 244 | val, err := encodeIssue(issue) | 233 | val, err := encodeIssue(issue) | ||
| 245 | if err != nil { | 234 | if err != nil { | ||
| 246 | return err | 235 | return err | ||
M internal/adapters/kvstore/issues_pending_test.go +5 -5
| @@ -114,7 +114,7 @@ func TestPendingIssueApproveSharesNumbers(t *testing.T) { | |||
| 114 | 114 | require.NoError(t, ps.Put(ctx, named)) | |
| 115 | 115 | require.NoError(t, ps.Put(ctx, anon)) | |
| 116 | 116 | ||
| 117 | - | issue, err := ps.Approve(ctx, n, named.ID) | |
| 117 | + | issue, err := ps.Approve(ctx, n, named.ID, created.Add(time.Hour)) | |
| 118 | 118 | require.NoError(t, err) | |
| 119 | 119 | assert.Equal(t, int64(2), issue.Number) | |
| 120 | 120 | assert.Equal(t, domain.IssueStateOpen, issue.State) | |
| @@ -124,7 +124,7 @@ func TestPendingIssueApproveSharesNumbers(t *testing.T) { | |||
| 124 | 124 | ||
| 125 | 125 | _, err = ps.Get(ctx, n, named.ID) | |
| 126 | 126 | assert.ErrorIs(t, err, domain.ErrNotFound, "the pending record is gone") | |
| 127 | - | _, err = ps.Approve(ctx, n, named.ID) | |
| 127 | + | _, err = ps.Approve(ctx, n, named.ID, created.Add(time.Hour)) | |
| 128 | 128 | assert.ErrorIs(t, err, domain.ErrNotFound, "approved once") | |
| 129 | 129 | ||
| 130 | 130 | got, err := s.Issues().Get(ctx, n, 2) | |
| @@ -136,7 +136,7 @@ func TestPendingIssueApproveSharesNumbers(t *testing.T) { | |||
| 136 | 136 | require.Len(t, open, 1) | |
| 137 | 137 | assert.Equal(t, int64(2), open[0].Number) | |
| 138 | 138 | ||
| 139 | - | second, err := ps.Approve(ctx, n, anon.ID) | |
| 139 | + | second, err := ps.Approve(ctx, n, anon.ID, created.Add(time.Hour)) | |
| 140 | 140 | require.NoError(t, err) | |
| 141 | 141 | assert.Equal(t, int64(3), second.Number) | |
| 142 | 142 | assert.Equal(t, "anonymous", second.Author) | |
| @@ -165,7 +165,7 @@ func TestPendingIssueApproveConcurrent(t *testing.T) { | |||
| 165 | 165 | wg.Add(1) | |
| 166 | 166 | go func() { | |
| 167 | 167 | defer wg.Done() | |
| 168 | - | is, err := ps.Approve(ctx, n, id) | |
| 168 | + | is, err := ps.Approve(ctx, n, id, created.Add(time.Hour)) | |
| 169 | 169 | if assert.NoError(t, err) { | |
| 170 | 170 | mu.Lock() | |
| 171 | 171 | seen[is.Number] = true | |
| @@ -195,7 +195,7 @@ func TestPendingIssueApproveOnDeletedRepoFails(t *testing.T) { | |||
| 195 | 195 | require.NoError(t, ps.Put(ctx, p)) | |
| 196 | 196 | require.NoError(t, s.Delete(ctx, n)) | |
| 197 | 197 | ||
| 198 | - | _, err := ps.Approve(ctx, n, p.ID) | |
| 198 | + | _, err := ps.Approve(ctx, n, p.ID, created.Add(time.Hour)) | |
| 199 | 199 | assert.ErrorIs(t, err, domain.ErrNotFound) | |
| 200 | 200 | assert.ErrorIs(t, ps.Put(ctx, newPending(t, n, "t", "", created)), domain.ErrNotFound) | |
| 201 | 201 | assert.Zero(t, countKeys(t, s, "is/")) | |
| @@ -114,7 +114,7 @@ func TestPendingIssueApproveSharesNumbers(t *testing.T) { | |||||
| 114 | require.NoError(t, ps.Put(ctx, named)) | 114 | require.NoError(t, ps.Put(ctx, named)) | ||
| 115 | require.NoError(t, ps.Put(ctx, anon)) | 115 | require.NoError(t, ps.Put(ctx, anon)) | ||
| 116 | 116 | ||||
| 117 | - | issue, err := ps.Approve(ctx, n, named.ID) | 117 | + | issue, err := ps.Approve(ctx, n, named.ID, created.Add(time.Hour)) |
| 118 | require.NoError(t, err) | 118 | require.NoError(t, err) | ||
| 119 | assert.Equal(t, int64(2), issue.Number) | 119 | assert.Equal(t, int64(2), issue.Number) | ||
| 120 | assert.Equal(t, domain.IssueStateOpen, issue.State) | 120 | assert.Equal(t, domain.IssueStateOpen, issue.State) | ||
| @@ -124,7 +124,7 @@ func TestPendingIssueApproveSharesNumbers(t *testing.T) { | |||||
| 124 | 124 | ||||
| 125 | _, err = ps.Get(ctx, n, named.ID) | 125 | _, err = ps.Get(ctx, n, named.ID) | ||
| 126 | assert.ErrorIs(t, err, domain.ErrNotFound, "the pending record is gone") | 126 | assert.ErrorIs(t, err, domain.ErrNotFound, "the pending record is gone") | ||
| 127 | - | _, err = ps.Approve(ctx, n, named.ID) | 127 | + | _, err = ps.Approve(ctx, n, named.ID, created.Add(time.Hour)) |
| 128 | assert.ErrorIs(t, err, domain.ErrNotFound, "approved once") | 128 | assert.ErrorIs(t, err, domain.ErrNotFound, "approved once") | ||
| 129 | 129 | ||||
| 130 | got, err := s.Issues().Get(ctx, n, 2) | 130 | got, err := s.Issues().Get(ctx, n, 2) | ||
| @@ -136,7 +136,7 @@ func TestPendingIssueApproveSharesNumbers(t *testing.T) { | |||||
| 136 | require.Len(t, open, 1) | 136 | require.Len(t, open, 1) | ||
| 137 | assert.Equal(t, int64(2), open[0].Number) | 137 | assert.Equal(t, int64(2), open[0].Number) | ||
| 138 | 138 | ||||
| 139 | - | second, err := ps.Approve(ctx, n, anon.ID) | 139 | + | second, err := ps.Approve(ctx, n, anon.ID, created.Add(time.Hour)) |
| 140 | require.NoError(t, err) | 140 | require.NoError(t, err) | ||
| 141 | assert.Equal(t, int64(3), second.Number) | 141 | assert.Equal(t, int64(3), second.Number) | ||
| 142 | assert.Equal(t, "anonymous", second.Author) | 142 | assert.Equal(t, "anonymous", second.Author) | ||
| @@ -165,7 +165,7 @@ func TestPendingIssueApproveConcurrent(t *testing.T) { | |||||
| 165 | wg.Add(1) | 165 | wg.Add(1) | ||
| 166 | go func() { | 166 | go func() { | ||
| 167 | defer wg.Done() | 167 | defer wg.Done() | ||
| 168 | - | is, err := ps.Approve(ctx, n, id) | 168 | + | is, err := ps.Approve(ctx, n, id, created.Add(time.Hour)) |
| 169 | if assert.NoError(t, err) { | 169 | if assert.NoError(t, err) { | ||
| 170 | mu.Lock() | 170 | mu.Lock() | ||
| 171 | seen[is.Number] = true | 171 | seen[is.Number] = true | ||
| @@ -195,7 +195,7 @@ func TestPendingIssueApproveOnDeletedRepoFails(t *testing.T) { | |||||
| 195 | require.NoError(t, ps.Put(ctx, p)) | 195 | require.NoError(t, ps.Put(ctx, p)) | ||
| 196 | require.NoError(t, s.Delete(ctx, n)) | 196 | require.NoError(t, s.Delete(ctx, n)) | ||
| 197 | 197 | ||||
| 198 | - | _, err := ps.Approve(ctx, n, p.ID) | 198 | + | _, err := ps.Approve(ctx, n, p.ID, created.Add(time.Hour)) |
| 199 | assert.ErrorIs(t, err, domain.ErrNotFound) | 199 | assert.ErrorIs(t, err, domain.ErrNotFound) | ||
| 200 | assert.ErrorIs(t, ps.Put(ctx, newPending(t, n, "t", "", created)), domain.ErrNotFound) | 200 | assert.ErrorIs(t, ps.Put(ctx, newPending(t, n, "t", "", created)), domain.ErrNotFound) | ||
| 201 | assert.Zero(t, countKeys(t, s, "is/")) | 201 | assert.Zero(t, countKeys(t, s, "is/")) | ||
M internal/domain/pending_issue.go +18 -0
| @@ -85,6 +85,24 @@ func ParsePendingAuthor(s string) (string, error) { | |||
| 85 | 85 | return c, nil | |
| 86 | 86 | } | |
| 87 | 87 | ||
| 88 | + | // AnonymousAuthor labels an approved issue whose submitter gave no name. | |
| 89 | + | const AnonymousAuthor = "anonymous" | |
| 90 | + | ||
| 91 | + | // ApprovedIssue is the open, external issue a pending submission becomes when | |
| 92 | + | // the owner approves it at now. The store sets the number. A submitter who gave | |
| 93 | + | // no name is AnonymousAuthor; the issue keeps the submission time as its | |
| 94 | + | // creation time. | |
| 95 | + | func ApprovedIssue(p PendingIssue, now time.Time) Issue { | |
| 96 | + | author := p.Author | |
| 97 | + | if author == "" { | |
| 98 | + | author = AnonymousAuthor | |
| 99 | + | } | |
| 100 | + | return Issue{ | |
| 101 | + | Repo: p.Repo, Title: p.Title, Body: p.Body, State: IssueStateOpen, | |
| 102 | + | Author: author, CreatedAt: p.CreatedAt, UpdatedAt: now, External: true, | |
| 103 | + | } | |
| 104 | + | } | |
| 105 | + | ||
| 88 | 106 | // PendingIssue is an issue submitted through the public form, awaiting the | |
| 89 | 107 | // owner's decision. It has no number: approval gives it one. | |
| 90 | 108 | type PendingIssue struct { | |
| @@ -85,6 +85,24 @@ func ParsePendingAuthor(s string) (string, error) { | |||||
| 85 | return c, nil | 85 | return c, nil | ||
| 86 | } | 86 | } | ||
| 87 | 87 | ||||
| 88 | + | // AnonymousAuthor labels an approved issue whose submitter gave no name. | |||
| 89 | + | const AnonymousAuthor = "anonymous" | |||
| 90 | + | ||||
| 91 | + | // ApprovedIssue is the open, external issue a pending submission becomes when | |||
| 92 | + | // the owner approves it at now. The store sets the number. A submitter who gave | |||
| 93 | + | // no name is AnonymousAuthor; the issue keeps the submission time as its | |||
| 94 | + | // creation time. | |||
| 95 | + | func ApprovedIssue(p PendingIssue, now time.Time) Issue { | |||
| 96 | + | author := p.Author | |||
| 97 | + | if author == "" { | |||
| 98 | + | author = AnonymousAuthor | |||
| 99 | + | } | |||
| 100 | + | return Issue{ | |||
| 101 | + | Repo: p.Repo, Title: p.Title, Body: p.Body, State: IssueStateOpen, | |||
| 102 | + | Author: author, CreatedAt: p.CreatedAt, UpdatedAt: now, External: true, | |||
| 103 | + | } | |||
| 104 | + | } | |||
| 105 | + | ||||
| 88 | // PendingIssue is an issue submitted through the public form, awaiting the | 106 | // PendingIssue is an issue submitted through the public form, awaiting the | ||
| 89 | // owner's decision. It has no number: approval gives it one. | 107 | // owner's decision. It has no number: approval gives it one. | ||
| 90 | type PendingIssue struct { | 108 | type PendingIssue struct { | ||
M internal/domain/pending_issue_test.go +22 -0
| @@ -66,3 +66,25 @@ func TestSubmissionLimitsAreTheDocumentedOnes(t *testing.T) { | |||
| 66 | 66 | assert.Equal(t, 200, domain.MaxPendingIssues) | |
| 67 | 67 | assert.Equal(t, 30*24*time.Hour, domain.BanDuration) | |
| 68 | 68 | } | |
| 69 | + | ||
| 70 | + | func TestApprovedIssue(t *testing.T) { | |
| 71 | + | t.Parallel() | |
| 72 | + | at := time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) | |
| 73 | + | now := at.Add(time.Hour) | |
| 74 | + | title, err := domain.ParseIssueTitle("t") | |
| 75 | + | require.NoError(t, err) | |
| 76 | + | repo, err := domain.ParseRepoName("proj") | |
| 77 | + | require.NoError(t, err) | |
| 78 | + | ||
| 79 | + | named := domain.ApprovedIssue(domain.PendingIssue{Repo: repo, Title: title, Author: "Alex", CreatedAt: at}, now) | |
| 80 | + | assert.Equal(t, domain.IssueStateOpen, named.State) | |
| 81 | + | assert.Equal(t, "Alex", named.Author) | |
| 82 | + | assert.True(t, named.External) | |
| 83 | + | assert.Equal(t, repo, named.Repo) | |
| 84 | + | assert.Equal(t, at, named.CreatedAt) | |
| 85 | + | assert.Equal(t, now, named.UpdatedAt) | |
| 86 | + | assert.Zero(t, named.Number, "the store numbers it") | |
| 87 | + | ||
| 88 | + | anon := domain.ApprovedIssue(domain.PendingIssue{Repo: repo, Title: title, CreatedAt: at}, now) | |
| 89 | + | assert.Equal(t, domain.AnonymousAuthor, anon.Author) | |
| 90 | + | } | |
| @@ -66,3 +66,25 @@ func TestSubmissionLimitsAreTheDocumentedOnes(t *testing.T) { | |||||
| 66 | assert.Equal(t, 200, domain.MaxPendingIssues) | 66 | assert.Equal(t, 200, domain.MaxPendingIssues) | ||
| 67 | assert.Equal(t, 30*24*time.Hour, domain.BanDuration) | 67 | assert.Equal(t, 30*24*time.Hour, domain.BanDuration) | ||
| 68 | } | 68 | } | ||
| 69 | + | ||||
| 70 | + | func TestApprovedIssue(t *testing.T) { | |||
| 71 | + | t.Parallel() | |||
| 72 | + | at := time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) | |||
| 73 | + | now := at.Add(time.Hour) | |||
| 74 | + | title, err := domain.ParseIssueTitle("t") | |||
| 75 | + | require.NoError(t, err) | |||
| 76 | + | repo, err := domain.ParseRepoName("proj") | |||
| 77 | + | require.NoError(t, err) | |||
| 78 | + | ||||
| 79 | + | named := domain.ApprovedIssue(domain.PendingIssue{Repo: repo, Title: title, Author: "Alex", CreatedAt: at}, now) | |||
| 80 | + | assert.Equal(t, domain.IssueStateOpen, named.State) | |||
| 81 | + | assert.Equal(t, "Alex", named.Author) | |||
| 82 | + | assert.True(t, named.External) | |||
| 83 | + | assert.Equal(t, repo, named.Repo) | |||
| 84 | + | assert.Equal(t, at, named.CreatedAt) | |||
| 85 | + | assert.Equal(t, now, named.UpdatedAt) | |||
| 86 | + | assert.Zero(t, named.Number, "the store numbers it") | |||
| 87 | + | ||||
| 88 | + | anon := domain.ApprovedIssue(domain.PendingIssue{Repo: repo, Title: title, CreatedAt: at}, now) | |||
| 89 | + | assert.Equal(t, domain.AnonymousAuthor, anon.Author) | |||
| 90 | + | } | |||
M internal/ports/mocks/PendingIssueStore.go +19 -12
| @@ -6,6 +6,7 @@ package mocks | |||
| 6 | 6 | ||
| 7 | 7 | import ( | |
| 8 | 8 | "context" | |
| 9 | + | "time" | |
| 9 | 10 | ||
| 10 | 11 | "bnema.dev/ermite/internal/domain" | |
| 11 | 12 | "bnema.dev/ermite/internal/ports" | |
| @@ -49,8 +50,8 @@ func (_m *PendingIssueStore) EXPECT() *PendingIssueStore_Expecter { | |||
| 49 | 50 | } | |
| 50 | 51 | ||
| 51 | 52 | // Approve provides a mock function for the type PendingIssueStore | |
| 52 | - | func (_mock *PendingIssueStore) Approve(ctx context.Context, repo domain.RepoName, id domain.PendingIssueID) (domain.Issue, error) { | |
| 53 | - | ret := _mock.Called(ctx, repo, id) | |
| 53 | + | func (_mock *PendingIssueStore) Approve(ctx context.Context, repo domain.RepoName, id domain.PendingIssueID, now time.Time) (domain.Issue, error) { | |
| 54 | + | ret := _mock.Called(ctx, repo, id, now) | |
| 54 | 55 | ||
| 55 | 56 | if len(ret) == 0 { | |
| 56 | 57 | panic("no return value specified for Approve") | |
| @@ -58,16 +59,16 @@ func (_mock *PendingIssueStore) Approve(ctx context.Context, repo domain.RepoNam | |||
| 58 | 59 | ||
| 59 | 60 | var r0 domain.Issue | |
| 60 | 61 | var r1 error | |
| 61 | - | if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName, domain.PendingIssueID) (domain.Issue, error)); ok { | |
| 62 | - | return returnFunc(ctx, repo, id) | |
| 62 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName, domain.PendingIssueID, time.Time) (domain.Issue, error)); ok { | |
| 63 | + | return returnFunc(ctx, repo, id, now) | |
| 63 | 64 | } | |
| 64 | - | if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName, domain.PendingIssueID) domain.Issue); ok { | |
| 65 | - | r0 = returnFunc(ctx, repo, id) | |
| 65 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName, domain.PendingIssueID, time.Time) domain.Issue); ok { | |
| 66 | + | r0 = returnFunc(ctx, repo, id, now) | |
| 66 | 67 | } else { | |
| 67 | 68 | r0 = ret.Get(0).(domain.Issue) | |
| 68 | 69 | } | |
| 69 | - | if returnFunc, ok := ret.Get(1).(func(context.Context, domain.RepoName, domain.PendingIssueID) error); ok { | |
| 70 | - | r1 = returnFunc(ctx, repo, id) | |
| 70 | + | if returnFunc, ok := ret.Get(1).(func(context.Context, domain.RepoName, domain.PendingIssueID, time.Time) error); ok { | |
| 71 | + | r1 = returnFunc(ctx, repo, id, now) | |
| 71 | 72 | } else { | |
| 72 | 73 | r1 = ret.Error(1) | |
| 73 | 74 | } | |
| @@ -83,11 +84,12 @@ type PendingIssueStore_Approve_Call struct { | |||
| 83 | 84 | // - ctx context.Context | |
| 84 | 85 | // - repo domain.RepoName | |
| 85 | 86 | // - id domain.PendingIssueID | |
| 86 | - | func (_e *PendingIssueStore_Expecter) Approve(ctx any, repo any, id any) *PendingIssueStore_Approve_Call { | |
| 87 | - | return &PendingIssueStore_Approve_Call{Call: _e.mock.On("Approve", ctx, repo, id)} | |
| 87 | + | // - now time.Time | |
| 88 | + | func (_e *PendingIssueStore_Expecter) Approve(ctx any, repo any, id any, now any) *PendingIssueStore_Approve_Call { | |
| 89 | + | return &PendingIssueStore_Approve_Call{Call: _e.mock.On("Approve", ctx, repo, id, now)} | |
| 88 | 90 | } | |
| 89 | 91 | ||
| 90 | - | func (_c *PendingIssueStore_Approve_Call) Run(run func(ctx context.Context, repo domain.RepoName, id domain.PendingIssueID)) *PendingIssueStore_Approve_Call { | |
| 92 | + | func (_c *PendingIssueStore_Approve_Call) Run(run func(ctx context.Context, repo domain.RepoName, id domain.PendingIssueID, now time.Time)) *PendingIssueStore_Approve_Call { | |
| 91 | 93 | _c.Call.Run(func(args mock.Arguments) { | |
| 92 | 94 | var arg0 context.Context | |
| 93 | 95 | if args[0] != nil { | |
| @@ -101,10 +103,15 @@ func (_c *PendingIssueStore_Approve_Call) Run(run func(ctx context.Context, repo | |||
| 101 | 103 | if args[2] != nil { | |
| 102 | 104 | arg2 = args[2].(domain.PendingIssueID) | |
| 103 | 105 | } | |
| 106 | + | var arg3 time.Time | |
| 107 | + | if args[3] != nil { | |
| 108 | + | arg3 = args[3].(time.Time) | |
| 109 | + | } | |
| 104 | 110 | run( | |
| 105 | 111 | arg0, | |
| 106 | 112 | arg1, | |
| 107 | 113 | arg2, | |
| 114 | + | arg3, | |
| 108 | 115 | ) | |
| 109 | 116 | }) | |
| 110 | 117 | return _c | |
| @@ -115,7 +122,7 @@ func (_c *PendingIssueStore_Approve_Call) Return(issue domain.Issue, err error) | |||
| 115 | 122 | return _c | |
| 116 | 123 | } | |
| 117 | 124 | ||
| 118 | - | func (_c *PendingIssueStore_Approve_Call) RunAndReturn(run func(ctx context.Context, repo domain.RepoName, id domain.PendingIssueID) (domain.Issue, error)) *PendingIssueStore_Approve_Call { | |
| 125 | + | func (_c *PendingIssueStore_Approve_Call) RunAndReturn(run func(ctx context.Context, repo domain.RepoName, id domain.PendingIssueID, now time.Time) (domain.Issue, error)) *PendingIssueStore_Approve_Call { | |
| 119 | 126 | _c.Call.Return(run) | |
| 120 | 127 | return _c | |
| 121 | 128 | } | |
| @@ -6,6 +6,7 @@ package mocks | |||||
| 6 | 6 | ||||
| 7 | import ( | 7 | import ( | ||
| 8 | "context" | 8 | "context" | ||
| 9 | + | "time" | |||
| 9 | 10 | ||||
| 10 | "bnema.dev/ermite/internal/domain" | 11 | "bnema.dev/ermite/internal/domain" | ||
| 11 | "bnema.dev/ermite/internal/ports" | 12 | "bnema.dev/ermite/internal/ports" | ||
| @@ -49,8 +50,8 @@ func (_m *PendingIssueStore) EXPECT() *PendingIssueStore_Expecter { | |||||
| 49 | } | 50 | } | ||
| 50 | 51 | ||||
| 51 | // Approve provides a mock function for the type PendingIssueStore | 52 | // Approve provides a mock function for the type PendingIssueStore | ||
| 52 | - | func (_mock *PendingIssueStore) Approve(ctx context.Context, repo domain.RepoName, id domain.PendingIssueID) (domain.Issue, error) { | 53 | + | func (_mock *PendingIssueStore) Approve(ctx context.Context, repo domain.RepoName, id domain.PendingIssueID, now time.Time) (domain.Issue, error) { |
| 53 | - | ret := _mock.Called(ctx, repo, id) | 54 | + | ret := _mock.Called(ctx, repo, id, now) |
| 54 | 55 | ||||
| 55 | if len(ret) == 0 { | 56 | if len(ret) == 0 { | ||
| 56 | panic("no return value specified for Approve") | 57 | panic("no return value specified for Approve") | ||
| @@ -58,16 +59,16 @@ func (_mock *PendingIssueStore) Approve(ctx context.Context, repo domain.RepoNam | |||||
| 58 | 59 | ||||
| 59 | var r0 domain.Issue | 60 | var r0 domain.Issue | ||
| 60 | var r1 error | 61 | var r1 error | ||
| 61 | - | if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName, domain.PendingIssueID) (domain.Issue, error)); ok { | 62 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName, domain.PendingIssueID, time.Time) (domain.Issue, error)); ok { |
| 62 | - | return returnFunc(ctx, repo, id) | 63 | + | return returnFunc(ctx, repo, id, now) |
| 63 | } | 64 | } | ||
| 64 | - | if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName, domain.PendingIssueID) domain.Issue); ok { | 65 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName, domain.PendingIssueID, time.Time) domain.Issue); ok { |
| 65 | - | r0 = returnFunc(ctx, repo, id) | 66 | + | r0 = returnFunc(ctx, repo, id, now) |
| 66 | } else { | 67 | } else { | ||
| 67 | r0 = ret.Get(0).(domain.Issue) | 68 | r0 = ret.Get(0).(domain.Issue) | ||
| 68 | } | 69 | } | ||
| 69 | - | if returnFunc, ok := ret.Get(1).(func(context.Context, domain.RepoName, domain.PendingIssueID) error); ok { | 70 | + | if returnFunc, ok := ret.Get(1).(func(context.Context, domain.RepoName, domain.PendingIssueID, time.Time) error); ok { |
| 70 | - | r1 = returnFunc(ctx, repo, id) | 71 | + | r1 = returnFunc(ctx, repo, id, now) |
| 71 | } else { | 72 | } else { | ||
| 72 | r1 = ret.Error(1) | 73 | r1 = ret.Error(1) | ||
| 73 | } | 74 | } | ||
| @@ -83,11 +84,12 @@ type PendingIssueStore_Approve_Call struct { | |||||
| 83 | // - ctx context.Context | 84 | // - ctx context.Context | ||
| 84 | // - repo domain.RepoName | 85 | // - repo domain.RepoName | ||
| 85 | // - id domain.PendingIssueID | 86 | // - id domain.PendingIssueID | ||
| 86 | - | func (_e *PendingIssueStore_Expecter) Approve(ctx any, repo any, id any) *PendingIssueStore_Approve_Call { | 87 | + | // - now time.Time |
| 87 | - | return &PendingIssueStore_Approve_Call{Call: _e.mock.On("Approve", ctx, repo, id)} | 88 | + | func (_e *PendingIssueStore_Expecter) Approve(ctx any, repo any, id any, now any) *PendingIssueStore_Approve_Call { |
| 89 | + | return &PendingIssueStore_Approve_Call{Call: _e.mock.On("Approve", ctx, repo, id, now)} | |||
| 88 | } | 90 | } | ||
| 89 | 91 | ||||
| 90 | - | func (_c *PendingIssueStore_Approve_Call) Run(run func(ctx context.Context, repo domain.RepoName, id domain.PendingIssueID)) *PendingIssueStore_Approve_Call { | 92 | + | func (_c *PendingIssueStore_Approve_Call) Run(run func(ctx context.Context, repo domain.RepoName, id domain.PendingIssueID, now time.Time)) *PendingIssueStore_Approve_Call { |
| 91 | _c.Call.Run(func(args mock.Arguments) { | 93 | _c.Call.Run(func(args mock.Arguments) { | ||
| 92 | var arg0 context.Context | 94 | var arg0 context.Context | ||
| 93 | if args[0] != nil { | 95 | if args[0] != nil { | ||
| @@ -101,10 +103,15 @@ func (_c *PendingIssueStore_Approve_Call) Run(run func(ctx context.Context, repo | |||||
| 101 | if args[2] != nil { | 103 | if args[2] != nil { | ||
| 102 | arg2 = args[2].(domain.PendingIssueID) | 104 | arg2 = args[2].(domain.PendingIssueID) | ||
| 103 | } | 105 | } | ||
| 106 | + | var arg3 time.Time | |||
| 107 | + | if args[3] != nil { | |||
| 108 | + | arg3 = args[3].(time.Time) | |||
| 109 | + | } | |||
| 104 | run( | 110 | run( | ||
| 105 | arg0, | 111 | arg0, | ||
| 106 | arg1, | 112 | arg1, | ||
| 107 | arg2, | 113 | arg2, | ||
| 114 | + | arg3, | |||
| 108 | ) | 115 | ) | ||
| 109 | }) | 116 | }) | ||
| 110 | return _c | 117 | return _c | ||
| @@ -115,7 +122,7 @@ func (_c *PendingIssueStore_Approve_Call) Return(issue domain.Issue, err error) | |||||
| 115 | return _c | 122 | return _c | ||
| 116 | } | 123 | } | ||
| 117 | 124 | ||||
| 118 | - | func (_c *PendingIssueStore_Approve_Call) RunAndReturn(run func(ctx context.Context, repo domain.RepoName, id domain.PendingIssueID) (domain.Issue, error)) *PendingIssueStore_Approve_Call { | 125 | + | func (_c *PendingIssueStore_Approve_Call) RunAndReturn(run func(ctx context.Context, repo domain.RepoName, id domain.PendingIssueID, now time.Time) (domain.Issue, error)) *PendingIssueStore_Approve_Call { |
| 119 | _c.Call.Return(run) | 126 | _c.Call.Return(run) | ||
| 120 | return _c | 127 | return _c | ||
| 121 | } | 128 | } | ||
M internal/ports/ports.go +3 -3
| @@ -296,10 +296,10 @@ type PendingIssueStore interface { | |||
| 296 | 296 | // reading at most domain.MaxPendingIssues+1 of them. | |
| 297 | 297 | Count(ctx context.Context) (int, error) | |
| 298 | 298 | // Approve takes the next number of the repository (the one merge requests | |
| 299 | - | // and issues share), writes the open issue (its author is the pending | |
| 300 | - | // one, or "anonymous"), indexes it as open and deletes the pending record, | |
| 299 | + | // and issues share), writes the issue domain.ApprovedIssue(pending, now) | |
| 300 | + | // with that number, indexes it as open and deletes the pending record, | |
| 301 | 301 | // in one batch. It returns domain.ErrNotFound when either is missing. | |
| 302 | - | Approve(ctx context.Context, repo domain.RepoName, id domain.PendingIssueID) (domain.Issue, error) | |
| 302 | + | Approve(ctx context.Context, repo domain.RepoName, id domain.PendingIssueID, now time.Time) (domain.Issue, error) | |
| 303 | 303 | // Delete removes the pending issue, or returns domain.ErrNotFound. | |
| 304 | 304 | Delete(ctx context.Context, repo domain.RepoName, id domain.PendingIssueID) error | |
| 305 | 305 | } | |
| @@ -296,10 +296,10 @@ type PendingIssueStore interface { | |||||
| 296 | // reading at most domain.MaxPendingIssues+1 of them. | 296 | // reading at most domain.MaxPendingIssues+1 of them. | ||
| 297 | Count(ctx context.Context) (int, error) | 297 | Count(ctx context.Context) (int, error) | ||
| 298 | // Approve takes the next number of the repository (the one merge requests | 298 | // Approve takes the next number of the repository (the one merge requests | ||
| 299 | - | // and issues share), writes the open issue (its author is the pending | 299 | + | // and issues share), writes the issue domain.ApprovedIssue(pending, now) |
| 300 | - | // one, or "anonymous"), indexes it as open and deletes the pending record, | 300 | + | // with that number, indexes it as open and deletes the pending record, |
| 301 | // in one batch. It returns domain.ErrNotFound when either is missing. | 301 | // in one batch. It returns domain.ErrNotFound when either is missing. | ||
| 302 | - | Approve(ctx context.Context, repo domain.RepoName, id domain.PendingIssueID) (domain.Issue, error) | 302 | + | Approve(ctx context.Context, repo domain.RepoName, id domain.PendingIssueID, now time.Time) (domain.Issue, error) |
| 303 | // Delete removes the pending issue, or returns domain.ErrNotFound. | 303 | // Delete removes the pending issue, or returns domain.ErrNotFound. | ||
| 304 | Delete(ctx context.Context, repo domain.RepoName, id domain.PendingIssueID) error | 304 | Delete(ctx context.Context, repo domain.RepoName, id domain.PendingIssueID) error | ||
| 305 | } | 305 | } | ||
M internal/usecase/issue_moderation.go +2 -2
| @@ -74,7 +74,8 @@ func (m *IssueModeration) Approve(ctx context.Context, p domain.Principal, repo | |||
| 74 | 74 | if r.Settings.Issues.OrDefault() == domain.IssuesOff { | |
| 75 | 75 | return domain.Issue{}, domain.ErrNotFound | |
| 76 | 76 | } | |
| 77 | - | issue, err := m.pending.Approve(ctx, repo, id) | |
| 77 | + | now := m.clock.Now().UTC() | |
| 78 | + | issue, err := m.pending.Approve(ctx, repo, id, now) | |
| 78 | 79 | if err != nil { | |
| 79 | 80 | if errors.Is(err, domain.ErrNotFound) { | |
| 80 | 81 | return domain.Issue{}, err | |
| @@ -84,7 +85,6 @@ func (m *IssueModeration) Approve(ctx context.Context, p domain.Principal, repo | |||
| 84 | 85 | log := zerowrap.FromCtx(ctx) | |
| 85 | 86 | log.Info().Str("repo", repo.String()).Int64("number", issue.Number). | |
| 86 | 87 | Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("pending issue approved") | |
| 87 | - | now := m.clock.Now().UTC() | |
| 88 | 88 | m.bus.Publish(ctx, domain.IssueOpened{Issue: issue, At: now, Visibility: r.Visibility, CreatedAt: r.CreatedAt}) | |
| 89 | 89 | return issue, nil | |
| 90 | 90 | } | |
| @@ -74,7 +74,8 @@ func (m *IssueModeration) Approve(ctx context.Context, p domain.Principal, repo | |||||
| 74 | if r.Settings.Issues.OrDefault() == domain.IssuesOff { | 74 | if r.Settings.Issues.OrDefault() == domain.IssuesOff { | ||
| 75 | return domain.Issue{}, domain.ErrNotFound | 75 | return domain.Issue{}, domain.ErrNotFound | ||
| 76 | } | 76 | } | ||
| 77 | - | issue, err := m.pending.Approve(ctx, repo, id) | 77 | + | now := m.clock.Now().UTC() |
| 78 | + | issue, err := m.pending.Approve(ctx, repo, id, now) | |||
| 78 | if err != nil { | 79 | if err != nil { | ||
| 79 | if errors.Is(err, domain.ErrNotFound) { | 80 | if errors.Is(err, domain.ErrNotFound) { | ||
| 80 | return domain.Issue{}, err | 81 | return domain.Issue{}, err | ||
| @@ -84,7 +85,6 @@ func (m *IssueModeration) Approve(ctx context.Context, p domain.Principal, repo | |||||
| 84 | log := zerowrap.FromCtx(ctx) | 85 | log := zerowrap.FromCtx(ctx) | ||
| 85 | log.Info().Str("repo", repo.String()).Int64("number", issue.Number). | 86 | log.Info().Str("repo", repo.String()).Int64("number", issue.Number). | ||
| 86 | Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("pending issue approved") | 87 | Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("pending issue approved") | ||
| 87 | - | now := m.clock.Now().UTC() | |||
| 88 | m.bus.Publish(ctx, domain.IssueOpened{Issue: issue, At: now, Visibility: r.Visibility, CreatedAt: r.CreatedAt}) | 88 | m.bus.Publish(ctx, domain.IssueOpened{Issue: issue, At: now, Visibility: r.Visibility, CreatedAt: r.CreatedAt}) | ||
| 89 | return issue, nil | 89 | return issue, nil | ||
| 90 | } | 90 | } | ||
M internal/usecase/issue_moderation_test.go +2 -2
| @@ -87,7 +87,7 @@ func TestIssueModerationApprove(t *testing.T) { | |||
| 87 | 87 | t.Run("publishes IssueOpened after the commit", func(t *testing.T) { | |
| 88 | 88 | u := newModerationUnit(t, domain.IssuesPublic) | |
| 89 | 89 | issue := domain.Issue{Repo: u.repo.Name, Number: 7, State: domain.IssueStateOpen} | |
| 90 | - | u.pending.EXPECT().Approve(mock.Anything, u.repo.Name, u.id).Return(issue, nil).Once() | |
| 90 | + | u.pending.EXPECT().Approve(mock.Anything, u.repo.Name, u.id, now).Return(issue, nil).Once() | |
| 91 | 91 | u.bus.EXPECT().Publish(mock.Anything, domain.IssueOpened{Issue: issue, At: now, Visibility: u.repo.Visibility, CreatedAt: u.repo.CreatedAt}).Once() | |
| 92 | 92 | got, err := u.mod.Approve(ctx, domain.Owner(), u.repo.Name, u.id) | |
| 93 | 93 | require.NoError(t, err) | |
| @@ -102,7 +102,7 @@ func TestIssueModerationApprove(t *testing.T) { | |||
| 102 | 102 | }) | |
| 103 | 103 | t.Run("no event when the store refuses", func(t *testing.T) { | |
| 104 | 104 | u := newModerationUnit(t, domain.IssuesOwner) | |
| 105 | - | u.pending.EXPECT().Approve(mock.Anything, u.repo.Name, u.id).Return(domain.Issue{}, domain.ErrNotFound).Once() | |
| 105 | + | u.pending.EXPECT().Approve(mock.Anything, u.repo.Name, u.id, now).Return(domain.Issue{}, domain.ErrNotFound).Once() | |
| 106 | 106 | _, err := u.mod.Approve(ctx, domain.Root(), u.repo.Name, u.id) | |
| 107 | 107 | assert.ErrorIs(t, err, domain.ErrNotFound) | |
| 108 | 108 | }) | |
| @@ -87,7 +87,7 @@ func TestIssueModerationApprove(t *testing.T) { | |||||
| 87 | t.Run("publishes IssueOpened after the commit", func(t *testing.T) { | 87 | t.Run("publishes IssueOpened after the commit", func(t *testing.T) { | ||
| 88 | u := newModerationUnit(t, domain.IssuesPublic) | 88 | u := newModerationUnit(t, domain.IssuesPublic) | ||
| 89 | issue := domain.Issue{Repo: u.repo.Name, Number: 7, State: domain.IssueStateOpen} | 89 | issue := domain.Issue{Repo: u.repo.Name, Number: 7, State: domain.IssueStateOpen} | ||
| 90 | - | u.pending.EXPECT().Approve(mock.Anything, u.repo.Name, u.id).Return(issue, nil).Once() | 90 | + | u.pending.EXPECT().Approve(mock.Anything, u.repo.Name, u.id, now).Return(issue, nil).Once() |
| 91 | u.bus.EXPECT().Publish(mock.Anything, domain.IssueOpened{Issue: issue, At: now, Visibility: u.repo.Visibility, CreatedAt: u.repo.CreatedAt}).Once() | 91 | u.bus.EXPECT().Publish(mock.Anything, domain.IssueOpened{Issue: issue, At: now, Visibility: u.repo.Visibility, CreatedAt: u.repo.CreatedAt}).Once() | ||
| 92 | got, err := u.mod.Approve(ctx, domain.Owner(), u.repo.Name, u.id) | 92 | got, err := u.mod.Approve(ctx, domain.Owner(), u.repo.Name, u.id) | ||
| 93 | require.NoError(t, err) | 93 | require.NoError(t, err) | ||
| @@ -102,7 +102,7 @@ func TestIssueModerationApprove(t *testing.T) { | |||||
| 102 | }) | 102 | }) | ||
| 103 | t.Run("no event when the store refuses", func(t *testing.T) { | 103 | t.Run("no event when the store refuses", func(t *testing.T) { | ||
| 104 | u := newModerationUnit(t, domain.IssuesOwner) | 104 | u := newModerationUnit(t, domain.IssuesOwner) | ||
| 105 | - | u.pending.EXPECT().Approve(mock.Anything, u.repo.Name, u.id).Return(domain.Issue{}, domain.ErrNotFound).Once() | 105 | + | u.pending.EXPECT().Approve(mock.Anything, u.repo.Name, u.id, now).Return(domain.Issue{}, domain.ErrNotFound).Once() |
| 106 | _, err := u.mod.Approve(ctx, domain.Root(), u.repo.Name, u.id) | 106 | _, err := u.mod.Approve(ctx, domain.Root(), u.repo.Name, u.id) | ||
| 107 | assert.ErrorIs(t, err, domain.ErrNotFound) | 107 | assert.ErrorIs(t, err, domain.ErrNotFound) | ||
| 108 | }) | 108 | }) | ||