refactor: drop the unused pending issue state, ListOpen and InboxMergeRequest
- commit
- f8b1e92017d28b2d619cdc8ba95fe704818ef94e
- author
- brice · · 2026-10-19 08:55 UTC
- parent
- 1486f6d
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
13 files · +57 -135
M internal/adapters/kvstore/export_test.go +8 -0
| @@ -57,3 +57,11 @@ func (m *MergeRequests) DropMRRecordForTest(ctx context.Context, repo domain.Rep | |||
| 57 | 57 | _, err := m.db.Apply(ctx, &b) | |
| 58 | 58 | return err | |
| 59 | 59 | } | |
| 60 | + | ||
| 61 | + | // PutRawIssueForTest stores value as the issue record, bypassing the codec. | |
| 62 | + | func (i *Issues) PutRawIssueForTest(ctx context.Context, repo domain.RepoName, n int64, value []byte) error { | |
| 63 | + | var b kv.Batch | |
| 64 | + | b.Put(issueKey(repo, n), value) | |
| 65 | + | _, err := i.db.Apply(ctx, &b) | |
| 66 | + | return err | |
| 67 | + | } | |
| @@ -57,3 +57,11 @@ func (m *MergeRequests) DropMRRecordForTest(ctx context.Context, repo domain.Rep | |||||
| 57 | _, err := m.db.Apply(ctx, &b) | 57 | _, err := m.db.Apply(ctx, &b) | ||
| 58 | return err | 58 | return err | ||
| 59 | } | 59 | } | ||
| 60 | + | ||||
| 61 | + | // PutRawIssueForTest stores value as the issue record, bypassing the codec. | |||
| 62 | + | func (i *Issues) PutRawIssueForTest(ctx context.Context, repo domain.RepoName, n int64, value []byte) error { | |||
| 63 | + | var b kv.Batch | |||
| 64 | + | b.Put(issueKey(repo, n), value) | |||
| 65 | + | _, err := i.db.Apply(ctx, &b) | |||
| 66 | + | return err | |||
| 67 | + | } | |||
M internal/adapters/kvstore/issues.go +1 -36
| @@ -178,39 +178,6 @@ func (i *Issues) List(ctx context.Context, repo domain.RepoName, params ports.Li | |||
| 178 | 178 | return page, nil | |
| 179 | 179 | } | |
| 180 | 180 | ||
| 181 | - | // ListOpen returns the open issues of the repository, highest number first, | |
| 182 | - | // through the index: only the open ones are read. | |
| 183 | - | // | |
| 184 | - | //hexcheck:ignore suspicious-business-logic-in-adapter reads the open issues through their index: a storage concern, no policy | |
| 185 | - | func (i *Issues) ListOpen(ctx context.Context, repo domain.RepoName) ([]domain.Issue, error) { | |
| 186 | - | var numbers []int64 | |
| 187 | - | for key := range i.db.Snapshot(ctx).Prefix(ctx, []byte(issueOpenRepoPrefix(repo))) { | |
| 188 | - | if err := ctx.Err(); err != nil { | |
| 189 | - | return nil, err | |
| 190 | - | } | |
| 191 | - | n, err := strconv.ParseInt(string(key[strings.LastIndexByte(string(key), '/')+1:]), 10, 64) | |
| 192 | - | if err != nil { | |
| 193 | - | return nil, fmt.Errorf("index key %q: %w", key, err) | |
| 194 | - | } | |
| 195 | - | numbers = append(numbers, n) | |
| 196 | - | } | |
| 197 | - | slices.Sort(numbers) | |
| 198 | - | var out []domain.Issue | |
| 199 | - | for _, n := range slices.Backward(numbers) { | |
| 200 | - | issue, err := i.Get(ctx, repo, n) | |
| 201 | - | if errors.Is(err, domain.ErrNotFound) { | |
| 202 | - | continue // closed or deleted since the snapshot of the index | |
| 203 | - | } | |
| 204 | - | if err != nil { | |
| 205 | - | return nil, err | |
| 206 | - | } | |
| 207 | - | if issue.IsOpen() { | |
| 208 | - | out = append(out, issue) | |
| 209 | - | } | |
| 210 | - | } | |
| 211 | - | return out, nil | |
| 212 | - | } | |
| 213 | - | ||
| 214 | 181 | // ListOpenAll returns the open issues of every repository through the isopen/ | |
| 215 | 182 | // index: the issues it names are read, no other. | |
| 216 | 183 | // | |
| @@ -428,8 +395,6 @@ func encodeIssueState(s domain.IssueState) (storagepb.IssueState, error) { | |||
| 428 | 395 | return storagepb.IssueState_ISSUE_STATE_OPEN, nil | |
| 429 | 396 | case domain.IssueStateClosed: | |
| 430 | 397 | return storagepb.IssueState_ISSUE_STATE_CLOSED, nil | |
| 431 | - | case domain.IssueStatePending: | |
| 432 | - | return storagepb.IssueState_ISSUE_STATE_PENDING, nil | |
| 433 | 398 | } | |
| 434 | 399 | return 0, fmt.Errorf("%w: issue state %q", domain.ErrInvalid, s) | |
| 435 | 400 | } | |
| @@ -441,7 +406,7 @@ func decodeIssueState(s storagepb.IssueState) (domain.IssueState, error) { | |||
| 441 | 406 | case storagepb.IssueState_ISSUE_STATE_CLOSED: | |
| 442 | 407 | return domain.IssueStateClosed, nil | |
| 443 | 408 | case storagepb.IssueState_ISSUE_STATE_PENDING: | |
| 444 | - | return domain.IssueStatePending, nil | |
| 409 | + | return "", errors.New("stored issue is pending: a numbered issue is open or closed") | |
| 445 | 410 | } | |
| 446 | 411 | return "", fmt.Errorf("unknown issue state %d", s) | |
| 447 | 412 | } | |
| @@ -178,39 +178,6 @@ func (i *Issues) List(ctx context.Context, repo domain.RepoName, params ports.Li | |||||
| 178 | return page, nil | 178 | return page, nil | ||
| 179 | } | 179 | } | ||
| 180 | 180 | ||||
| 181 | - | // ListOpen returns the open issues of the repository, highest number first, | |||
| 182 | - | // through the index: only the open ones are read. | |||
| 183 | - | // | |||
| 184 | - | //hexcheck:ignore suspicious-business-logic-in-adapter reads the open issues through their index: a storage concern, no policy | |||
| 185 | - | func (i *Issues) ListOpen(ctx context.Context, repo domain.RepoName) ([]domain.Issue, error) { | |||
| 186 | - | var numbers []int64 | |||
| 187 | - | for key := range i.db.Snapshot(ctx).Prefix(ctx, []byte(issueOpenRepoPrefix(repo))) { | |||
| 188 | - | if err := ctx.Err(); err != nil { | |||
| 189 | - | return nil, err | |||
| 190 | - | } | |||
| 191 | - | n, err := strconv.ParseInt(string(key[strings.LastIndexByte(string(key), '/')+1:]), 10, 64) | |||
| 192 | - | if err != nil { | |||
| 193 | - | return nil, fmt.Errorf("index key %q: %w", key, err) | |||
| 194 | - | } | |||
| 195 | - | numbers = append(numbers, n) | |||
| 196 | - | } | |||
| 197 | - | slices.Sort(numbers) | |||
| 198 | - | var out []domain.Issue | |||
| 199 | - | for _, n := range slices.Backward(numbers) { | |||
| 200 | - | issue, err := i.Get(ctx, repo, n) | |||
| 201 | - | if errors.Is(err, domain.ErrNotFound) { | |||
| 202 | - | continue // closed or deleted since the snapshot of the index | |||
| 203 | - | } | |||
| 204 | - | if err != nil { | |||
| 205 | - | return nil, err | |||
| 206 | - | } | |||
| 207 | - | if issue.IsOpen() { | |||
| 208 | - | out = append(out, issue) | |||
| 209 | - | } | |||
| 210 | - | } | |||
| 211 | - | return out, nil | |||
| 212 | - | } | |||
| 213 | - | ||||
| 214 | // ListOpenAll returns the open issues of every repository through the isopen/ | 181 | // ListOpenAll returns the open issues of every repository through the isopen/ | ||
| 215 | // index: the issues it names are read, no other. | 182 | // index: the issues it names are read, no other. | ||
| 216 | // | 183 | // | ||
| @@ -428,8 +395,6 @@ func encodeIssueState(s domain.IssueState) (storagepb.IssueState, error) { | |||||
| 428 | return storagepb.IssueState_ISSUE_STATE_OPEN, nil | 395 | return storagepb.IssueState_ISSUE_STATE_OPEN, nil | ||
| 429 | case domain.IssueStateClosed: | 396 | case domain.IssueStateClosed: | ||
| 430 | return storagepb.IssueState_ISSUE_STATE_CLOSED, nil | 397 | return storagepb.IssueState_ISSUE_STATE_CLOSED, nil | ||
| 431 | - | case domain.IssueStatePending: | |||
| 432 | - | return storagepb.IssueState_ISSUE_STATE_PENDING, nil | |||
| 433 | } | 398 | } | ||
| 434 | return 0, fmt.Errorf("%w: issue state %q", domain.ErrInvalid, s) | 399 | return 0, fmt.Errorf("%w: issue state %q", domain.ErrInvalid, s) | ||
| 435 | } | 400 | } | ||
| @@ -441,7 +406,7 @@ func decodeIssueState(s storagepb.IssueState) (domain.IssueState, error) { | |||||
| 441 | case storagepb.IssueState_ISSUE_STATE_CLOSED: | 406 | case storagepb.IssueState_ISSUE_STATE_CLOSED: | ||
| 442 | return domain.IssueStateClosed, nil | 407 | return domain.IssueStateClosed, nil | ||
| 443 | case storagepb.IssueState_ISSUE_STATE_PENDING: | 408 | case storagepb.IssueState_ISSUE_STATE_PENDING: | ||
| 444 | - | return domain.IssueStatePending, nil | 409 | + | return "", errors.New("stored issue is pending: a numbered issue is open or closed") |
| 445 | } | 410 | } | ||
| 446 | return "", fmt.Errorf("unknown issue state %d", s) | 411 | return "", fmt.Errorf("unknown issue state %d", s) | ||
| 447 | } | 412 | } | ||
M internal/adapters/kvstore/issues_pending_test.go +1 -2
| @@ -131,8 +131,7 @@ func TestPendingIssueApproveSharesNumbers(t *testing.T) { | |||
| 131 | 131 | require.NoError(t, err) | |
| 132 | 132 | assert.Equal(t, issue.Title, got.Title) | |
| 133 | 133 | assert.True(t, got.External, "the flag is stored") | |
| 134 | - | open, err := s.Issues().ListOpen(ctx, n) | |
| 135 | - | require.NoError(t, err) | |
| 134 | + | open := openOf(t, s.Issues(), n) | |
| 136 | 135 | require.Len(t, open, 1) | |
| 137 | 136 | assert.Equal(t, int64(2), open[0].Number) | |
| 138 | 137 | ||
| @@ -131,8 +131,7 @@ func TestPendingIssueApproveSharesNumbers(t *testing.T) { | |||||
| 131 | require.NoError(t, err) | 131 | require.NoError(t, err) | ||
| 132 | assert.Equal(t, issue.Title, got.Title) | 132 | assert.Equal(t, issue.Title, got.Title) | ||
| 133 | assert.True(t, got.External, "the flag is stored") | 133 | assert.True(t, got.External, "the flag is stored") | ||
| 134 | - | open, err := s.Issues().ListOpen(ctx, n) | 134 | + | open := openOf(t, s.Issues(), n) |
| 135 | - | require.NoError(t, err) | |||
| 136 | require.Len(t, open, 1) | 135 | require.Len(t, open, 1) | ||
| 137 | assert.Equal(t, int64(2), open[0].Number) | 136 | assert.Equal(t, int64(2), open[0].Number) | ||
| 138 | 137 | ||||
M internal/adapters/kvstore/issues_test.go +35 -7
| @@ -4,12 +4,14 @@ package kvstore_test | |||
| 4 | 4 | ||
| 5 | 5 | import ( | |
| 6 | 6 | "context" | |
| 7 | + | "sort" | |
| 7 | 8 | "strconv" | |
| 8 | 9 | "sync" | |
| 9 | 10 | "testing" | |
| 10 | 11 | ||
| 11 | 12 | "github.com/stretchr/testify/assert" | |
| 12 | 13 | "github.com/stretchr/testify/require" | |
| 14 | + | "google.golang.org/protobuf/proto" | |
| 13 | 15 | ||
| 14 | 16 | storagepb "bnema.dev/ermite/gen/ermite/storage" | |
| 15 | 17 | "bnema.dev/ermite/internal/adapters/kvstore" | |
| @@ -189,15 +191,12 @@ func TestDeleteDropsTheIssuesOfTheRepository(t *testing.T) { | |||
| 189 | 191 | comments, err := is.ListComments(ctx, n, 1) | |
| 190 | 192 | require.NoError(t, err) | |
| 191 | 193 | assert.Empty(t, comments) | |
| 192 | - | open, err := is.ListOpen(ctx, n) | |
| 193 | - | require.NoError(t, err) | |
| 194 | - | assert.Empty(t, open) | |
| 194 | + | assert.Empty(t, openOf(t, is, n)) | |
| 195 | 195 | assert.Zero(t, countKeys(t, s, "is/proj/")) | |
| 196 | 196 | assert.Zero(t, countKeys(t, s, "isc/proj/")) | |
| 197 | 197 | assert.Zero(t, countKeys(t, s, "isopen/proj/")) | |
| 198 | 198 | ||
| 199 | - | open, err = is.ListOpen(ctx, sib) | |
| 200 | - | require.NoError(t, err) | |
| 199 | + | open := openOf(t, is, sib) | |
| 201 | 200 | assert.Len(t, open, 1, "a repository whose name starts like the deleted one keeps its issues") | |
| 202 | 201 | comments, err = is.ListComments(ctx, sib, 1) | |
| 203 | 202 | require.NoError(t, err) | |
| @@ -219,8 +218,7 @@ func TestOpenIssueIndexFollowsUpdates(t *testing.T) { | |||
| 219 | 218 | require.NoError(t, err) | |
| 220 | 219 | ||
| 221 | 220 | numbers := func() []int64 { | |
| 222 | - | open, err := is.ListOpen(ctx, n) | |
| 223 | - | require.NoError(t, err) | |
| 221 | + | open := openOf(t, is, n) | |
| 224 | 222 | var out []int64 | |
| 225 | 223 | for _, i := range open { | |
| 226 | 224 | out = append(out, i.Number) | |
| @@ -502,3 +500,33 @@ func TestIssueListPagesThroughManyIssues(t *testing.T) { | |||
| 502 | 500 | assert.Empty(t, page.Issues) | |
| 503 | 501 | assert.Empty(t, page.NextPageToken) | |
| 504 | 502 | } | |
| 503 | + | ||
| 504 | + | // openOf lists the open issues of one repository through the isopen/ index. | |
| 505 | + | func openOf(t *testing.T, is *kvstore.Issues, repo domain.RepoName) []domain.Issue { | |
| 506 | + | t.Helper() | |
| 507 | + | all, err := is.ListOpenAll(context.Background()) | |
| 508 | + | require.NoError(t, err) | |
| 509 | + | var out []domain.Issue | |
| 510 | + | for _, i := range all { | |
| 511 | + | if i.Repo == repo { | |
| 512 | + | out = append(out, i) | |
| 513 | + | } | |
| 514 | + | } | |
| 515 | + | sort.Slice(out, func(a, b int) bool { return out[a].Number > out[b].Number }) | |
| 516 | + | return out | |
| 517 | + | } | |
| 518 | + | ||
| 519 | + | func TestStoredPendingIssueStateIsCorrupt(t *testing.T) { | |
| 520 | + | ctx := context.Background() | |
| 521 | + | s, _, n := repoWithMRs(t) | |
| 522 | + | is := s.Issues() | |
| 523 | + | raw, err := proto.Marshal(&storagepb.Issue{Number: 1, Title: "x", State: storagepb.IssueState_ISSUE_STATE_PENDING}) | |
| 524 | + | require.NoError(t, err) | |
| 525 | + | require.NoError(t, is.PutRawIssueForTest(ctx, n, 1, raw)) | |
| 526 | + | _, err = is.Get(ctx, n, 1) | |
| 527 | + | assert.ErrorContains(t, err, "pending") | |
| 528 | + | _, err = is.List(ctx, n, ports.ListIssuesParams{}) | |
| 529 | + | assert.ErrorContains(t, err, "pending") | |
| 530 | + | _, err = is.List(ctx, n, ports.ListIssuesParams{State: domain.IssueStateOpen}) | |
| 531 | + | assert.ErrorContains(t, err, "pending") | |
| 532 | + | } | |
| @@ -4,12 +4,14 @@ package kvstore_test | |||||
| 4 | 4 | ||||
| 5 | import ( | 5 | import ( | ||
| 6 | "context" | 6 | "context" | ||
| 7 | + | "sort" | |||
| 7 | "strconv" | 8 | "strconv" | ||
| 8 | "sync" | 9 | "sync" | ||
| 9 | "testing" | 10 | "testing" | ||
| 10 | 11 | ||||
| 11 | "github.com/stretchr/testify/assert" | 12 | "github.com/stretchr/testify/assert" | ||
| 12 | "github.com/stretchr/testify/require" | 13 | "github.com/stretchr/testify/require" | ||
| 14 | + | "google.golang.org/protobuf/proto" | |||
| 13 | 15 | ||||
| 14 | storagepb "bnema.dev/ermite/gen/ermite/storage" | 16 | storagepb "bnema.dev/ermite/gen/ermite/storage" | ||
| 15 | "bnema.dev/ermite/internal/adapters/kvstore" | 17 | "bnema.dev/ermite/internal/adapters/kvstore" | ||
| @@ -189,15 +191,12 @@ func TestDeleteDropsTheIssuesOfTheRepository(t *testing.T) { | |||||
| 189 | comments, err := is.ListComments(ctx, n, 1) | 191 | comments, err := is.ListComments(ctx, n, 1) | ||
| 190 | require.NoError(t, err) | 192 | require.NoError(t, err) | ||
| 191 | assert.Empty(t, comments) | 193 | assert.Empty(t, comments) | ||
| 192 | - | open, err := is.ListOpen(ctx, n) | 194 | + | assert.Empty(t, openOf(t, is, n)) |
| 193 | - | require.NoError(t, err) | |||
| 194 | - | assert.Empty(t, open) | |||
| 195 | assert.Zero(t, countKeys(t, s, "is/proj/")) | 195 | assert.Zero(t, countKeys(t, s, "is/proj/")) | ||
| 196 | assert.Zero(t, countKeys(t, s, "isc/proj/")) | 196 | assert.Zero(t, countKeys(t, s, "isc/proj/")) | ||
| 197 | assert.Zero(t, countKeys(t, s, "isopen/proj/")) | 197 | assert.Zero(t, countKeys(t, s, "isopen/proj/")) | ||
| 198 | 198 | ||||
| 199 | - | open, err = is.ListOpen(ctx, sib) | 199 | + | open := openOf(t, is, sib) |
| 200 | - | require.NoError(t, err) | |||
| 201 | assert.Len(t, open, 1, "a repository whose name starts like the deleted one keeps its issues") | 200 | assert.Len(t, open, 1, "a repository whose name starts like the deleted one keeps its issues") | ||
| 202 | comments, err = is.ListComments(ctx, sib, 1) | 201 | comments, err = is.ListComments(ctx, sib, 1) | ||
| 203 | require.NoError(t, err) | 202 | require.NoError(t, err) | ||
| @@ -219,8 +218,7 @@ func TestOpenIssueIndexFollowsUpdates(t *testing.T) { | |||||
| 219 | require.NoError(t, err) | 218 | require.NoError(t, err) | ||
| 220 | 219 | ||||
| 221 | numbers := func() []int64 { | 220 | numbers := func() []int64 { | ||
| 222 | - | open, err := is.ListOpen(ctx, n) | 221 | + | open := openOf(t, is, n) |
| 223 | - | require.NoError(t, err) | |||
| 224 | var out []int64 | 222 | var out []int64 | ||
| 225 | for _, i := range open { | 223 | for _, i := range open { | ||
| 226 | out = append(out, i.Number) | 224 | out = append(out, i.Number) | ||
| @@ -502,3 +500,33 @@ func TestIssueListPagesThroughManyIssues(t *testing.T) { | |||||
| 502 | assert.Empty(t, page.Issues) | 500 | assert.Empty(t, page.Issues) | ||
| 503 | assert.Empty(t, page.NextPageToken) | 501 | assert.Empty(t, page.NextPageToken) | ||
| 504 | } | 502 | } | ||
| 503 | + | ||||
| 504 | + | // openOf lists the open issues of one repository through the isopen/ index. | |||
| 505 | + | func openOf(t *testing.T, is *kvstore.Issues, repo domain.RepoName) []domain.Issue { | |||
| 506 | + | t.Helper() | |||
| 507 | + | all, err := is.ListOpenAll(context.Background()) | |||
| 508 | + | require.NoError(t, err) | |||
| 509 | + | var out []domain.Issue | |||
| 510 | + | for _, i := range all { | |||
| 511 | + | if i.Repo == repo { | |||
| 512 | + | out = append(out, i) | |||
| 513 | + | } | |||
| 514 | + | } | |||
| 515 | + | sort.Slice(out, func(a, b int) bool { return out[a].Number > out[b].Number }) | |||
| 516 | + | return out | |||
| 517 | + | } | |||
| 518 | + | ||||
| 519 | + | func TestStoredPendingIssueStateIsCorrupt(t *testing.T) { | |||
| 520 | + | ctx := context.Background() | |||
| 521 | + | s, _, n := repoWithMRs(t) | |||
| 522 | + | is := s.Issues() | |||
| 523 | + | raw, err := proto.Marshal(&storagepb.Issue{Number: 1, Title: "x", State: storagepb.IssueState_ISSUE_STATE_PENDING}) | |||
| 524 | + | require.NoError(t, err) | |||
| 525 | + | require.NoError(t, is.PutRawIssueForTest(ctx, n, 1, raw)) | |||
| 526 | + | _, err = is.Get(ctx, n, 1) | |||
| 527 | + | assert.ErrorContains(t, err, "pending") | |||
| 528 | + | _, err = is.List(ctx, n, ports.ListIssuesParams{}) | |||
| 529 | + | assert.ErrorContains(t, err, "pending") | |||
| 530 | + | _, err = is.List(ctx, n, ports.ListIssuesParams{State: domain.IssueStateOpen}) | |||
| 531 | + | assert.ErrorContains(t, err, "pending") | |||
| 532 | + | } | |||
M internal/adapters/rpc/convert_test.go +6 -0
| @@ -375,3 +375,9 @@ func TestWatchEventToPBSkipsPendingRemoved(t *testing.T) { | |||
| 375 | 375 | t.Parallel() | |
| 376 | 376 | assert.Nil(t, watchEventToPB(ports.WatchEvent{Event: domain.IssuePendingRemoved{}})) | |
| 377 | 377 | } | |
| 378 | + | ||
| 379 | + | func TestIssueStateToPBNeverPending(t *testing.T) { | |
| 380 | + | t.Parallel() | |
| 381 | + | assert.Equal(t, pb.IssueState_ISSUE_STATE_UNSPECIFIED, issueStateToPB(domain.IssueState("pending"))) | |
| 382 | + | assert.Equal(t, pb.IssueState_ISSUE_STATE_OPEN, issueStateToPB(domain.IssueStateOpen)) | |
| 383 | + | } | |
| @@ -375,3 +375,9 @@ func TestWatchEventToPBSkipsPendingRemoved(t *testing.T) { | |||||
| 375 | t.Parallel() | 375 | t.Parallel() | ||
| 376 | assert.Nil(t, watchEventToPB(ports.WatchEvent{Event: domain.IssuePendingRemoved{}})) | 376 | assert.Nil(t, watchEventToPB(ports.WatchEvent{Event: domain.IssuePendingRemoved{}})) | ||
| 377 | } | 377 | } | ||
| 378 | + | ||||
| 379 | + | func TestIssueStateToPBNeverPending(t *testing.T) { | |||
| 380 | + | t.Parallel() | |||
| 381 | + | assert.Equal(t, pb.IssueState_ISSUE_STATE_UNSPECIFIED, issueStateToPB(domain.IssueState("pending"))) | |||
| 382 | + | assert.Equal(t, pb.IssueState_ISSUE_STATE_OPEN, issueStateToPB(domain.IssueStateOpen)) | |||
| 383 | + | } | |||
M internal/adapters/rpc/issue.go +1 -3
| @@ -26,14 +26,12 @@ func issueStateToPB(s domain.IssueState) pb.IssueState { | |||
| 26 | 26 | return pb.IssueState_ISSUE_STATE_OPEN | |
| 27 | 27 | case domain.IssueStateClosed: | |
| 28 | 28 | return pb.IssueState_ISSUE_STATE_CLOSED | |
| 29 | - | case domain.IssueStatePending: | |
| 30 | - | return pb.IssueState_ISSUE_STATE_PENDING | |
| 31 | 29 | } | |
| 32 | 30 | return pb.IssueState_ISSUE_STATE_UNSPECIFIED | |
| 33 | 31 | } | |
| 34 | 32 | ||
| 35 | 33 | // issueStateFromPB maps the state filter of a listing; unspecified is the | |
| 36 | - | // empty state, which means every state. Pending is not listable. | |
| 34 | + | // empty state, which means every state. Pending is not listable (InvalidArgument): a domain Issue is never pending. | |
| 37 | 35 | func issueStateFromPB(s pb.IssueState) (domain.IssueState, error) { | |
| 38 | 36 | switch s { | |
| 39 | 37 | case pb.IssueState_ISSUE_STATE_UNSPECIFIED: | |
| @@ -26,14 +26,12 @@ func issueStateToPB(s domain.IssueState) pb.IssueState { | |||||
| 26 | return pb.IssueState_ISSUE_STATE_OPEN | 26 | return pb.IssueState_ISSUE_STATE_OPEN | ||
| 27 | case domain.IssueStateClosed: | 27 | case domain.IssueStateClosed: | ||
| 28 | return pb.IssueState_ISSUE_STATE_CLOSED | 28 | return pb.IssueState_ISSUE_STATE_CLOSED | ||
| 29 | - | case domain.IssueStatePending: | |||
| 30 | - | return pb.IssueState_ISSUE_STATE_PENDING | |||
| 31 | } | 29 | } | ||
| 32 | return pb.IssueState_ISSUE_STATE_UNSPECIFIED | 30 | return pb.IssueState_ISSUE_STATE_UNSPECIFIED | ||
| 33 | } | 31 | } | ||
| 34 | 32 | ||||
| 35 | // issueStateFromPB maps the state filter of a listing; unspecified is the | 33 | // issueStateFromPB maps the state filter of a listing; unspecified is the | ||
| 36 | - | // empty state, which means every state. Pending is not listable. | 34 | + | // empty state, which means every state. Pending is not listable (InvalidArgument): a domain Issue is never pending. |
| 37 | func issueStateFromPB(s pb.IssueState) (domain.IssueState, error) { | 35 | func issueStateFromPB(s pb.IssueState) (domain.IssueState, error) { | ||
| 38 | switch s { | 36 | switch s { | ||
| 39 | case pb.IssueState_ISSUE_STATE_UNSPECIFIED: | 37 | case pb.IssueState_ISSUE_STATE_UNSPECIFIED: | ||
M internal/adapters/web/inbox_test.go +2 -2
| @@ -29,9 +29,9 @@ func inboxOverview(t testing.TB) ports.InboxOverview { | |||
| 29 | 29 | require.NoError(t, err) | |
| 30 | 30 | at := time.Now().Add(-2 * time.Hour) | |
| 31 | 31 | ov := ports.InboxOverview{ | |
| 32 | - | MergeRequests: []ports.InboxMergeRequest{{MergeRequest: domain.MergeRequest{ | |
| 32 | + | MergeRequests: []domain.MergeRequest{{ | |
| 33 | 33 | Repo: repoName(t, "alpha"), Number: 4, Title: mrTitle, State: domain.MRStateOpen, UpdatedAt: at, | |
| 34 | - | }}}, | |
| 34 | + | }}, | |
| 35 | 35 | Issues: []domain.Issue{{Repo: repoName(t, "beta"), Number: 7, Title: isTitle, State: domain.IssueStateOpen, UpdatedAt: at}}, | |
| 36 | 36 | Repos: []domain.RepoName{repoName(t, "alpha"), repoName(t, "beta")}, | |
| 37 | 37 | } | |
| @@ -29,9 +29,9 @@ func inboxOverview(t testing.TB) ports.InboxOverview { | |||||
| 29 | require.NoError(t, err) | 29 | require.NoError(t, err) | ||
| 30 | at := time.Now().Add(-2 * time.Hour) | 30 | at := time.Now().Add(-2 * time.Hour) | ||
| 31 | ov := ports.InboxOverview{ | 31 | ov := ports.InboxOverview{ | ||
| 32 | - | MergeRequests: []ports.InboxMergeRequest{{MergeRequest: domain.MergeRequest{ | 32 | + | MergeRequests: []domain.MergeRequest{{ |
| 33 | Repo: repoName(t, "alpha"), Number: 4, Title: mrTitle, State: domain.MRStateOpen, UpdatedAt: at, | 33 | Repo: repoName(t, "alpha"), Number: 4, Title: mrTitle, State: domain.MRStateOpen, UpdatedAt: at, | ||
| 34 | - | }}}, | 34 | + | }}, |
| 35 | Issues: []domain.Issue{{Repo: repoName(t, "beta"), Number: 7, Title: isTitle, State: domain.IssueStateOpen, UpdatedAt: at}}, | 35 | Issues: []domain.Issue{{Repo: repoName(t, "beta"), Number: 7, Title: isTitle, State: domain.IssueStateOpen, UpdatedAt: at}}, | ||
| 36 | Repos: []domain.RepoName{repoName(t, "alpha"), repoName(t, "beta")}, | 36 | Repos: []domain.RepoName{repoName(t, "alpha"), repoName(t, "beta")}, | ||
| 37 | } | 37 | } | ||
M internal/domain/issue.go +0 -2
| @@ -49,8 +49,6 @@ type IssueState string | |||
| 49 | 49 | ||
| 50 | 50 | // Issue states. | |
| 51 | 51 | const ( | |
| 52 | - | // IssueStatePending awaits moderation. Nothing creates it yet. | |
| 53 | - | IssueStatePending IssueState = "pending" | |
| 54 | 52 | // IssueStateOpen is being worked on. | |
| 55 | 53 | IssueStateOpen IssueState = "open" | |
| 56 | 54 | // IssueStateClosed is done with; it can be reopened. | |
| @@ -49,8 +49,6 @@ type IssueState string | |||||
| 49 | 49 | ||||
| 50 | // Issue states. | 50 | // Issue states. | ||
| 51 | const ( | 51 | const ( | ||
| 52 | - | // IssueStatePending awaits moderation. Nothing creates it yet. | |||
| 53 | - | IssueStatePending IssueState = "pending" | |||
| 54 | // IssueStateOpen is being worked on. | 52 | // IssueStateOpen is being worked on. | ||
| 55 | IssueStateOpen IssueState = "open" | 53 | IssueStateOpen IssueState = "open" | ||
| 56 | // IssueStateClosed is done with; it can be reopened. | 54 | // IssueStateClosed is done with; it can be reopened. | ||
M internal/ports/dto.go +1 -7
| @@ -323,12 +323,6 @@ type LoginLink struct { | |||
| 323 | 323 | ExpiresAt time.Time | |
| 324 | 324 | } | |
| 325 | 325 | ||
| 326 | - | // InboxMergeRequest is an open merge request of the owner's inbox. The merge | |
| 327 | - | // request list of a repository shows no CI state, so none is carried here. | |
| 328 | - | type InboxMergeRequest struct { | |
| 329 | - | domain.MergeRequest | |
| 330 | - | } | |
| 331 | - | ||
| 332 | 326 | // InboxCounts are the numbers of open items across every repository, whatever | |
| 333 | 327 | // the filter. | |
| 334 | 328 | type InboxCounts struct{ MergeRequests, Issues, Pending int } | |
| @@ -340,7 +334,7 @@ type InboxOverview struct { | |||
| 340 | 334 | Counts InboxCounts | |
| 341 | 335 | // MergeRequests and Issues are those of the filtered repository, or of | |
| 342 | 336 | // every repository when there is no filter. Counts always counts all. | |
| 343 | - | MergeRequests []InboxMergeRequest | |
| 337 | + | MergeRequests []domain.MergeRequest | |
| 344 | 338 | Issues []domain.Issue | |
| 345 | 339 | // Pending are the issues awaiting moderation, in every repository | |
| 346 | 340 | // whatever the filter, oldest first. At most domain.MaxPendingIssues. | |
| @@ -323,12 +323,6 @@ type LoginLink struct { | |||||
| 323 | ExpiresAt time.Time | 323 | ExpiresAt time.Time | ||
| 324 | } | 324 | } | ||
| 325 | 325 | ||||
| 326 | - | // InboxMergeRequest is an open merge request of the owner's inbox. The merge | |||
| 327 | - | // request list of a repository shows no CI state, so none is carried here. | |||
| 328 | - | type InboxMergeRequest struct { | |||
| 329 | - | domain.MergeRequest | |||
| 330 | - | } | |||
| 331 | - | ||||
| 332 | // InboxCounts are the numbers of open items across every repository, whatever | 326 | // InboxCounts are the numbers of open items across every repository, whatever | ||
| 333 | // the filter. | 327 | // the filter. | ||
| 334 | type InboxCounts struct{ MergeRequests, Issues, Pending int } | 328 | type InboxCounts struct{ MergeRequests, Issues, Pending int } | ||
| @@ -340,7 +334,7 @@ type InboxOverview struct { | |||||
| 340 | Counts InboxCounts | 334 | Counts InboxCounts | ||
| 341 | // MergeRequests and Issues are those of the filtered repository, or of | 335 | // MergeRequests and Issues are those of the filtered repository, or of | ||
| 342 | // every repository when there is no filter. Counts always counts all. | 336 | // every repository when there is no filter. Counts always counts all. | ||
| 343 | - | MergeRequests []InboxMergeRequest | 337 | + | MergeRequests []domain.MergeRequest |
| 344 | Issues []domain.Issue | 338 | Issues []domain.Issue | ||
| 345 | // Pending are the issues awaiting moderation, in every repository | 339 | // Pending are the issues awaiting moderation, in every repository | ||
| 346 | // whatever the filter, oldest first. At most domain.MaxPendingIssues. | 340 | // whatever the filter, oldest first. At most domain.MaxPendingIssues. | ||
M internal/ports/mocks/IssueStore.go +0 -68
| @@ -332,74 +332,6 @@ func (_c *IssueStore_ListComments_Call) RunAndReturn(run func(ctx context.Contex | |||
| 332 | 332 | return _c | |
| 333 | 333 | } | |
| 334 | 334 | ||
| 335 | - | // ListOpen provides a mock function for the type IssueStore | |
| 336 | - | func (_mock *IssueStore) ListOpen(ctx context.Context, repo domain.RepoName) ([]domain.Issue, error) { | |
| 337 | - | ret := _mock.Called(ctx, repo) | |
| 338 | - | ||
| 339 | - | if len(ret) == 0 { | |
| 340 | - | panic("no return value specified for ListOpen") | |
| 341 | - | } | |
| 342 | - | ||
| 343 | - | var r0 []domain.Issue | |
| 344 | - | var r1 error | |
| 345 | - | if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName) ([]domain.Issue, error)); ok { | |
| 346 | - | return returnFunc(ctx, repo) | |
| 347 | - | } | |
| 348 | - | if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName) []domain.Issue); ok { | |
| 349 | - | r0 = returnFunc(ctx, repo) | |
| 350 | - | } else { | |
| 351 | - | if ret.Get(0) != nil { | |
| 352 | - | r0 = ret.Get(0).([]domain.Issue) | |
| 353 | - | } | |
| 354 | - | } | |
| 355 | - | if returnFunc, ok := ret.Get(1).(func(context.Context, domain.RepoName) error); ok { | |
| 356 | - | r1 = returnFunc(ctx, repo) | |
| 357 | - | } else { | |
| 358 | - | r1 = ret.Error(1) | |
| 359 | - | } | |
| 360 | - | return r0, r1 | |
| 361 | - | } | |
| 362 | - | ||
| 363 | - | // IssueStore_ListOpen_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'ListOpen' | |
| 364 | - | type IssueStore_ListOpen_Call struct { | |
| 365 | - | *mock.Call | |
| 366 | - | } | |
| 367 | - | ||
| 368 | - | // ListOpen is a helper method to define mock.On call | |
| 369 | - | // - ctx context.Context | |
| 370 | - | // - repo domain.RepoName | |
| 371 | - | func (_e *IssueStore_Expecter) ListOpen(ctx any, repo any) *IssueStore_ListOpen_Call { | |
| 372 | - | return &IssueStore_ListOpen_Call{Call: _e.mock.On("ListOpen", ctx, repo)} | |
| 373 | - | } | |
| 374 | - | ||
| 375 | - | func (_c *IssueStore_ListOpen_Call) Run(run func(ctx context.Context, repo domain.RepoName)) *IssueStore_ListOpen_Call { | |
| 376 | - | _c.Call.Run(func(args mock.Arguments) { | |
| 377 | - | var arg0 context.Context | |
| 378 | - | if args[0] != nil { | |
| 379 | - | arg0 = args[0].(context.Context) | |
| 380 | - | } | |
| 381 | - | var arg1 domain.RepoName | |
| 382 | - | if args[1] != nil { | |
| 383 | - | arg1 = args[1].(domain.RepoName) | |
| 384 | - | } | |
| 385 | - | run( | |
| 386 | - | arg0, | |
| 387 | - | arg1, | |
| 388 | - | ) | |
| 389 | - | }) | |
| 390 | - | return _c | |
| 391 | - | } | |
| 392 | - | ||
| 393 | - | func (_c *IssueStore_ListOpen_Call) Return(issues []domain.Issue, err error) *IssueStore_ListOpen_Call { | |
| 394 | - | _c.Call.Return(issues, err) | |
| 395 | - | return _c | |
| 396 | - | } | |
| 397 | - | ||
| 398 | - | func (_c *IssueStore_ListOpen_Call) RunAndReturn(run func(ctx context.Context, repo domain.RepoName) ([]domain.Issue, error)) *IssueStore_ListOpen_Call { | |
| 399 | - | _c.Call.Return(run) | |
| 400 | - | return _c | |
| 401 | - | } | |
| 402 | - | ||
| 403 | 335 | // ListOpenAll provides a mock function for the type IssueStore | |
| 404 | 336 | func (_mock *IssueStore) ListOpenAll(ctx context.Context) ([]domain.Issue, error) { | |
| 405 | 337 | ret := _mock.Called(ctx) | |
| @@ -332,74 +332,6 @@ func (_c *IssueStore_ListComments_Call) RunAndReturn(run func(ctx context.Contex | |||||
| 332 | return _c | 332 | return _c | ||
| 333 | } | 333 | } | ||
| 334 | 334 | ||||
| 335 | - | // ListOpen provides a mock function for the type IssueStore | |||
| 336 | - | func (_mock *IssueStore) ListOpen(ctx context.Context, repo domain.RepoName) ([]domain.Issue, error) { | |||
| 337 | - | ret := _mock.Called(ctx, repo) | |||
| 338 | - | ||||
| 339 | - | if len(ret) == 0 { | |||
| 340 | - | panic("no return value specified for ListOpen") | |||
| 341 | - | } | |||
| 342 | - | ||||
| 343 | - | var r0 []domain.Issue | |||
| 344 | - | var r1 error | |||
| 345 | - | if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName) ([]domain.Issue, error)); ok { | |||
| 346 | - | return returnFunc(ctx, repo) | |||
| 347 | - | } | |||
| 348 | - | if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName) []domain.Issue); ok { | |||
| 349 | - | r0 = returnFunc(ctx, repo) | |||
| 350 | - | } else { | |||
| 351 | - | if ret.Get(0) != nil { | |||
| 352 | - | r0 = ret.Get(0).([]domain.Issue) | |||
| 353 | - | } | |||
| 354 | - | } | |||
| 355 | - | if returnFunc, ok := ret.Get(1).(func(context.Context, domain.RepoName) error); ok { | |||
| 356 | - | r1 = returnFunc(ctx, repo) | |||
| 357 | - | } else { | |||
| 358 | - | r1 = ret.Error(1) | |||
| 359 | - | } | |||
| 360 | - | return r0, r1 | |||
| 361 | - | } | |||
| 362 | - | ||||
| 363 | - | // IssueStore_ListOpen_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'ListOpen' | |||
| 364 | - | type IssueStore_ListOpen_Call struct { | |||
| 365 | - | *mock.Call | |||
| 366 | - | } | |||
| 367 | - | ||||
| 368 | - | // ListOpen is a helper method to define mock.On call | |||
| 369 | - | // - ctx context.Context | |||
| 370 | - | // - repo domain.RepoName | |||
| 371 | - | func (_e *IssueStore_Expecter) ListOpen(ctx any, repo any) *IssueStore_ListOpen_Call { | |||
| 372 | - | return &IssueStore_ListOpen_Call{Call: _e.mock.On("ListOpen", ctx, repo)} | |||
| 373 | - | } | |||
| 374 | - | ||||
| 375 | - | func (_c *IssueStore_ListOpen_Call) Run(run func(ctx context.Context, repo domain.RepoName)) *IssueStore_ListOpen_Call { | |||
| 376 | - | _c.Call.Run(func(args mock.Arguments) { | |||
| 377 | - | var arg0 context.Context | |||
| 378 | - | if args[0] != nil { | |||
| 379 | - | arg0 = args[0].(context.Context) | |||
| 380 | - | } | |||
| 381 | - | var arg1 domain.RepoName | |||
| 382 | - | if args[1] != nil { | |||
| 383 | - | arg1 = args[1].(domain.RepoName) | |||
| 384 | - | } | |||
| 385 | - | run( | |||
| 386 | - | arg0, | |||
| 387 | - | arg1, | |||
| 388 | - | ) | |||
| 389 | - | }) | |||
| 390 | - | return _c | |||
| 391 | - | } | |||
| 392 | - | ||||
| 393 | - | func (_c *IssueStore_ListOpen_Call) Return(issues []domain.Issue, err error) *IssueStore_ListOpen_Call { | |||
| 394 | - | _c.Call.Return(issues, err) | |||
| 395 | - | return _c | |||
| 396 | - | } | |||
| 397 | - | ||||
| 398 | - | func (_c *IssueStore_ListOpen_Call) RunAndReturn(run func(ctx context.Context, repo domain.RepoName) ([]domain.Issue, error)) *IssueStore_ListOpen_Call { | |||
| 399 | - | _c.Call.Return(run) | |||
| 400 | - | return _c | |||
| 401 | - | } | |||
| 402 | - | ||||
| 403 | // ListOpenAll provides a mock function for the type IssueStore | 335 | // ListOpenAll provides a mock function for the type IssueStore | ||
| 404 | func (_mock *IssueStore) ListOpenAll(ctx context.Context) ([]domain.Issue, error) { | 336 | func (_mock *IssueStore) ListOpenAll(ctx context.Context) ([]domain.Issue, error) { | ||
| 405 | ret := _mock.Called(ctx) | 337 | ret := _mock.Called(ctx) | ||
M internal/ports/ports.go +0 -3
| @@ -258,9 +258,6 @@ type IssueStore interface { | |||
| 258 | 258 | // List returns a page of the issues of the repository in the given state | |
| 259 | 259 | // (any when empty), highest number first. | |
| 260 | 260 | List(ctx context.Context, repo domain.RepoName, params ListIssuesParams) (IssuePage, error) | |
| 261 | - | // ListOpen returns the open issues of the repository, highest number | |
| 262 | - | // first, reading only the open ones (through an index). | |
| 263 | - | ListOpen(ctx context.Context, repo domain.RepoName) ([]domain.Issue, error) | |
| 264 | 261 | // ListOpenAll returns the open issues of every repository, reading only | |
| 265 | 262 | // the open ones (through the same index). The order is unspecified. | |
| 266 | 263 | ListOpenAll(ctx context.Context) ([]domain.Issue, error) | |
| @@ -258,9 +258,6 @@ type IssueStore interface { | |||||
| 258 | // List returns a page of the issues of the repository in the given state | 258 | // List returns a page of the issues of the repository in the given state | ||
| 259 | // (any when empty), highest number first. | 259 | // (any when empty), highest number first. | ||
| 260 | List(ctx context.Context, repo domain.RepoName, params ListIssuesParams) (IssuePage, error) | 260 | List(ctx context.Context, repo domain.RepoName, params ListIssuesParams) (IssuePage, error) | ||
| 261 | - | // ListOpen returns the open issues of the repository, highest number | |||
| 262 | - | // first, reading only the open ones (through an index). | |||
| 263 | - | ListOpen(ctx context.Context, repo domain.RepoName) ([]domain.Issue, error) | |||
| 264 | // ListOpenAll returns the open issues of every repository, reading only | 261 | // ListOpenAll returns the open issues of every repository, reading only | ||
| 265 | // the open ones (through the same index). The order is unspecified. | 262 | // the open ones (through the same index). The order is unspecified. | ||
| 266 | ListOpenAll(ctx context.Context) ([]domain.Issue, error) | 263 | ListOpenAll(ctx context.Context) ([]domain.Issue, error) | ||
M internal/usecase/inbox.go +2 -2
| @@ -90,7 +90,7 @@ func (in *Inbox) Overview(ctx context.Context, p domain.Principal, filter domain | |||
| 90 | 90 | repos[m.Repo] = true | |
| 91 | 91 | out.Counts.MergeRequests++ | |
| 92 | 92 | if filter.IsZero() || m.Repo == filter { | |
| 93 | - | out.MergeRequests = append(out.MergeRequests, ports.InboxMergeRequest{MergeRequest: m}) | |
| 93 | + | out.MergeRequests = append(out.MergeRequests, m) | |
| 94 | 94 | } | |
| 95 | 95 | } | |
| 96 | 96 | for _, i := range issues { | |
| @@ -126,7 +126,7 @@ func (in *Inbox) Overview(ctx context.Context, p domain.Principal, filter domain | |||
| 126 | 126 | out.Counts.Pending = len(out.Pending) | |
| 127 | 127 | } | |
| 128 | 128 | ||
| 129 | - | slices.SortStableFunc(out.MergeRequests, func(a, b ports.InboxMergeRequest) int { | |
| 129 | + | slices.SortStableFunc(out.MergeRequests, func(a, b domain.MergeRequest) int { | |
| 130 | 130 | return compareRecent(a.UpdatedAt, a.CreatedAt, a.Repo, a.Number, b.UpdatedAt, b.CreatedAt, b.Repo, b.Number) | |
| 131 | 131 | }) | |
| 132 | 132 | slices.SortStableFunc(out.Issues, func(a, b domain.Issue) int { | |
| @@ -90,7 +90,7 @@ func (in *Inbox) Overview(ctx context.Context, p domain.Principal, filter domain | |||||
| 90 | repos[m.Repo] = true | 90 | repos[m.Repo] = true | ||
| 91 | out.Counts.MergeRequests++ | 91 | out.Counts.MergeRequests++ | ||
| 92 | if filter.IsZero() || m.Repo == filter { | 92 | if filter.IsZero() || m.Repo == filter { | ||
| 93 | - | out.MergeRequests = append(out.MergeRequests, ports.InboxMergeRequest{MergeRequest: m}) | 93 | + | out.MergeRequests = append(out.MergeRequests, m) |
| 94 | } | 94 | } | ||
| 95 | } | 95 | } | ||
| 96 | for _, i := range issues { | 96 | for _, i := range issues { | ||
| @@ -126,7 +126,7 @@ func (in *Inbox) Overview(ctx context.Context, p domain.Principal, filter domain | |||||
| 126 | out.Counts.Pending = len(out.Pending) | 126 | out.Counts.Pending = len(out.Pending) | ||
| 127 | } | 127 | } | ||
| 128 | 128 | ||||
| 129 | - | slices.SortStableFunc(out.MergeRequests, func(a, b ports.InboxMergeRequest) int { | 129 | + | slices.SortStableFunc(out.MergeRequests, func(a, b domain.MergeRequest) int { |
| 130 | return compareRecent(a.UpdatedAt, a.CreatedAt, a.Repo, a.Number, b.UpdatedAt, b.CreatedAt, b.Repo, b.Number) | 130 | return compareRecent(a.UpdatedAt, a.CreatedAt, a.Repo, a.Number, b.UpdatedAt, b.CreatedAt, b.Repo, b.Number) | ||
| 131 | }) | 131 | }) | ||
| 132 | slices.SortStableFunc(out.Issues, func(a, b domain.Issue) int { | 132 | slices.SortStableFunc(out.Issues, func(a, b domain.Issue) int { | ||
M internal/usecase/issues.go +0 -3
| @@ -188,9 +188,6 @@ func (i *Issues) AddComment(ctx context.Context, p domain.Principal, name domain | |||
| 188 | 188 | var c domain.IssueComment | |
| 189 | 189 | saved := false | |
| 190 | 190 | for range maxWriteRetries { | |
| 191 | - | if issue.State == domain.IssueStatePending { | |
| 192 | - | return domain.IssueComment{}, fmt.Errorf("%w: #%d is %s", domain.ErrPreconditionFailed, number, issue.State) | |
| 193 | - | } | |
| 194 | 191 | if issue.CommentSeq >= MaxCommentsPerIssue { | |
| 195 | 192 | return domain.IssueComment{}, fmt.Errorf("%w: #%d already has %d comments", domain.ErrResourceExhausted, number, MaxCommentsPerIssue) | |
| 196 | 193 | } | |
| @@ -188,9 +188,6 @@ func (i *Issues) AddComment(ctx context.Context, p domain.Principal, name domain | |||||
| 188 | var c domain.IssueComment | 188 | var c domain.IssueComment | ||
| 189 | saved := false | 189 | saved := false | ||
| 190 | for range maxWriteRetries { | 190 | for range maxWriteRetries { | ||
| 191 | - | if issue.State == domain.IssueStatePending { | |||
| 192 | - | return domain.IssueComment{}, fmt.Errorf("%w: #%d is %s", domain.ErrPreconditionFailed, number, issue.State) | |||
| 193 | - | } | |||
| 194 | if issue.CommentSeq >= MaxCommentsPerIssue { | 191 | if issue.CommentSeq >= MaxCommentsPerIssue { | ||
| 195 | return domain.IssueComment{}, fmt.Errorf("%w: #%d already has %d comments", domain.ErrResourceExhausted, number, MaxCommentsPerIssue) | 192 | return domain.IssueComment{}, fmt.Errorf("%w: #%d already has %d comments", domain.ErrResourceExhausted, number, MaxCommentsPerIssue) | ||
| 196 | } | 193 | } | ||