Link Probo tasks to existing Linear issues - #1969
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 22 files
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
cec98d2 to
1e8c8de
Compare
1e8c8de to
a889566
Compare
There was a problem hiding this comment.
All reported issues were addressed across 15 files (changes from recent commits).
Requires human review: Auto-approval blocked because this review re-detected 2 unresolved issues already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
c7404e5 to
0891696
Compare
| if existingLink != nil && !isLinearPublishPending(existingLink) && existingLink.ExternalID != issue.ID { | ||
| if err := s.abandonIncompleteLinearPublish(ctx, accounts, scope, existingLink); err != nil { | ||
| return nil, err | ||
| } | ||
| } |
There was a problem hiding this comment.
If I understand correctly here, you are abandoning an incomplete publish started earlier to start a new one with a proper state, is that it?
| err := s.pg.WithTx( | ||
| ctx, | ||
| func(ctx context.Context, tx pg.Tx) error { | ||
| task = &coredata.Task{} | ||
| if err := task.LoadByID(ctx, tx, scope, taskID); err != nil { | ||
| return fmt.Errorf("cannot load task %q: %w", taskID, err) | ||
| } | ||
|
|
||
| existing := &coredata.TaskExternalLink{} | ||
| if err := existing.LoadByTaskID(ctx, tx, scope, taskID); err == nil { | ||
| if isLinearPublishComplete(existing) { | ||
| return ErrTaskAlreadyLinked | ||
| } | ||
|
|
||
| existingLink = existing | ||
| } else if !errors.Is(err, coredata.ErrResourceNotFound) { | ||
| return fmt.Errorf("cannot load task external link: %w", err) | ||
| } | ||
|
|
||
| return nil | ||
| }, | ||
| ) | ||
| if err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
Do we really need a transaction here as we are only reading stuff? Or should we expand it to span until later modification we are doing?
There was a problem hiding this comment.
No. Changed to a connection.
| if err := s.claimPendingLinearPublish(ctx, scope, task, account.connector, destination); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
There was a problem hiding this comment.
I may miss something but it looks like you are claiming the line for update too far after reseting its state therefore we can be expose to a "dirty" state started by another query, can we?
lukkor
left a comment
There was a problem hiding this comment.
I may have missed some mechanism protecting the service here but I feel like we can be expose to incomplete/inconsistent state via concurrency.
1e384c1 to
90bb1c5
Compare
The PR now lock the link row and commit the decision before creating or archiving in Linear. |
9560132 to
8c2ae58
Compare
There was a problem hiding this comment.
All reported issues were addressed across 11 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
8c2ae58 to
e7ec74d
Compare
There was a problem hiding this comment.
All reported issues were addressed across 12 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
e7ec74d to
6be2c9b
Compare
There was a problem hiding this comment.
All reported issues were addressed across 8 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
6be2c9b to
a496910
Compare
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
e589daf to
d60b470
Compare
d60b470 to
44089c7
Compare
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
72e3a68 to
44b2554
Compare
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
5029099 to
2908795
Compare
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
2908795 to
cc7d153
Compare
Publishing only created a new Linear issue. Team and issue pickers now search and page on the server, and choosing an existing issue links the Probo task instead. The new-task dialog offers the same choice when Linear Sync is connected. Status and priority menus open in the dialog overlay so they no longer break the layout. linearTeams and linearIssues return connections so every client pages the same way. The unpaginated team list is gone. Signed-off-by: Sacha Al Himdani <sacha@probo.com>
cc7d153 to
b60356e
Compare
Publishing only created a new Linear issue. Team and issue pickers now search and page on the server, and choosing an existing issue links the Probo task instead. The new-task dialog offers the same choice when Linear Sync is connected. Status and priority menus open in the dialog overlay so they no longer break the layout.
linearTeams and linearIssues return connections so every client pages the same way. The unpaginated team list is gone.
Summary by cubic
Publishing only created a new Linear issue. Team and issue pickers now search and page on the server, and choosing an existing issue links the Probo task instead. The task detail page and new-task dialog offer the same choice when Linear Sync is connected, and status and priority menus open in the dialog overlay so they no longer break the modal layout.
linearTeamsandlinearIssuesreturn connections so every client pages the same way; the unpaginated team list is gone.Tests
+286-31GraphQL API
+279-12linearIssuesconnection andlinkTaskToLinearmutation alongsidelinearTeams.MCP
+312-16linkTaskToLineartool alongside the existing publish tool and pages list-linear-teams through the connection.prb (CLI)
+375-31prb task link-linearandprb task list-linear-issues, makeslist-linear-teamssearch and page, and clarifies publish-linear leaves the task unchanged.Service
+1360-152App: console
+888-66Package: n8n-node
+351-13Package: ui
+28-5Written for commit b60356e. Summary will update on new commits.