fix: add circular dependency detection for tasks #323
Conversation
|
Thank you for opening this PR! Before a maintainer takes a look, it would be really helpful if you could walk through your changes using GitHub's review tools. Please take a moment to:
More information on how to conduct a self review: This helps make the review process smoother and gives us a clearer understanding of your thought process. Once you've added your self-review, we'll continue from our side. Thank you! |
|
the latest 3 commits fix:
this ui buggy behavior is fixed now |
|
@its-me-abhishek i have added the self review and fixed the ui buggy behavior. |
| // Show specific message for Edit Task failures | ||
| let errorMessage = `Failed to ${data.job || 'perform action'}`; | ||
| if (data.job === 'Edit Task') { | ||
| errorMessage = |
There was a problem hiding this comment.
this might not be the case everytime for this Jobs failure, and can be misleading.
There was a problem hiding this comment.
You can revert this for now, and leave this as it is. will add a separate toast flow later
| // This ensures the Tasks component sees the updated data | ||
| if (data.job === 'Edit Task') { | ||
| // For Edit Task, we need to trigger a sync to update the UI | ||
| window.dispatchEvent(new CustomEvent('syncTasks')); |
There was a problem hiding this comment.
is this really required? Can we skip this?
| task.annotations || [] | ||
| ); | ||
| } catch (error) { | ||
| console.error('Failed to update dependencies:', error); |
There was a problem hiding this comment.
maybe just add a new toast here instead of changing it in HomePage
| }); | ||
|
|
||
| console.log('Task edited successfully!'); | ||
| // Don't show success message here - wait for WebSocket confirmation |
There was a problem hiding this comment.
please remove this and other such copilot based comments, they add to the bloat
| return () => clearInterval(interval); | ||
| }, []); | ||
|
|
||
| // Listen for sync events from WebSocket handler |
There was a problem hiding this comment.
similarly, please remove trivial comments
| syncTasksWithTwAndDb(); | ||
| }; | ||
|
|
||
| window.addEventListener('syncTasks', handleSyncTasks); |
There was a problem hiding this comment.
Please remove this UseEffect and event listener architecture, and make use of other Sync based Effects available
| color[node] = 0 | ||
| } | ||
|
|
||
| for _, deps := range graph { |
There was a problem hiding this comment.
This n^2 loop needs to be optimized
its-me-abhishek
left a comment
its-me-abhishek
left a comment
There was a problem hiding this comment.
Please look into the above comments
2 participants
Description
Added cycle detection to prevent task dependency loops (e.g., Task A → Task B → Task A).
DFS algorithm with O(V + E) complexity for optimal cycle detection
Validates dependencies during add/edit/modify operations
Fixes: Circular dependency in task dependencies #308
Checklist
npx prettier --write .(for formatting)gofmt -w .(for Go backend)npm test(for JS/TS testing)Additional Notes