[WEB-2941]chore: added transfer issues button to cycles#6296
[WEB-2941]chore: added transfer issues button to cycles#6296
Conversation
WalkthroughThis pull request introduces changes to multiple components related to issue transfer functionality in a cycle management system. The modifications include updating the Changes
Sequence DiagramsequenceDiagram
participant User
participant CycleListItemAction
participant TransferIssuesModal
participant CycleAPI
User->>CycleListItemAction: Click Transfer Issues
CycleListItemAction->>TransferIssuesModal: Open Modal
TransferIssuesModal->>CycleAPI: Transfer Issues
CycleAPI-->>TransferIssuesModal: Transfer Successful
TransferIssuesModal->>TransferIssuesModal: Fetch Cycle Details
TransferIssuesModal-->>User: Close Modal
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
web/core/components/cycles/transfer-issues-modal.tsx (4)
24-24: Improve destructuring approach if needed.
Destructuringpropsin a single line is concise, but consider destructuring all relevant props together for consistency if you plan to add more props in the future.
57-70:getCycleDetailsconcurrency consideration.
Doing both fetches in a singlePromise.allis efficient. However, since both requests share the same error handler, ensure each request logs or surfaces its specific error details if needed. Otherwise, it might be unclear which fetch fails.
115-116: Confirm the icon’s sizing.
The updatedTransferIconusage (classNamew-5 fill-custom-text-100) might render differently across devices. Double-check the UI, especially on mobile breakpoints, to ensure a consistent user experience.
126-126: Placeholder text improvements.
"Search for a cycle..." is a good hint. For better accessibility, consider including anaria-labelor more descriptive text to guide screen readers.web/core/components/cycles/list/cycle-list-item-action.tsx (2)
60-61: Modal visibility state naming.
The state variabletransferIssuesModalis self-explanatory, though some teams prefer naming booleans with "is"/"show"/"open" as a prefix. The current naming is acceptable; just remain consistent.
247-258: Use a button element for accessibility.
The clickable<div>for transferring issues can be replaced with a<button>for semantics. This makes the element more accessible and consistent with other interactive components.- <div - className="px-2 h-6 text-custom-primary-200 flex items-center ..." - onClick={() => { setTransferIssuesModal(true) }} - > + <button + type="button" + className="px-2 h-6 text-custom-primary-200 flex items-center ..." + onClick={() => { setTransferIssuesModal(true) }} + >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/ui/src/icons/transfer-icon.tsx(1 hunks)web/core/components/cycles/list/cycle-list-item-action.tsx(7 hunks)web/core/components/cycles/transfer-issues-modal.tsx(3 hunks)web/core/components/issues/issue-layouts/roots/cycle-layout-root.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/ui/src/icons/transfer-icon.tsx
🔇 Additional comments (7)
web/core/components/issues/issue-layouts/roots/cycle-layout-root.tsx (1)
89-93: Ensure consistent validation of cycleId across the application.
The cycleId prop is passed to TransferIssuesModal for clearer context. Confirm that cycleId is always defined in all calling components and that no concurrency or async fetch issues cause a race condition before cycleId is available.
web/core/components/cycles/transfer-issues-modal.tsx (3)
20-20: Prop introduction recognized.
The new cycleId: string prop is a welcome addition for clarity. This ensures the component can accurately identify the current cycle.
29-29: Check loading states for cycle IDs.
While you import currentProjectIncompleteCycleIds, ensure the store or upstream state is not out-of-sync with the new cycleId prop. If these states are updated asynchronously, you may consider additional checks or loading states to handle possible mismatches.
36-46: Robustness in transferIssue.
The refactored function with { new_cycle_id: string } clarifies type safety. Just ensure:
- The
new_cycle_idalways differs from the currentcycleId; or handle if a user tries to “transfer” within the same cycle. - The success toast is triggered after the request finishes.
Also applies to: 52-52
web/core/components/cycles/list/cycle-list-item-action.tsx (3)
3-3: Imports are well-organized.
The imports for TransferIcon and TransferIssuesModal are clearly placed. No issues observed.
Also applies to: 18-18, 23-23
83-84: Clarify or guard against negative issue counts.
transferableIssuesCount = total_issues - completed_issues implicitly relies on completed_issues <= total_issues. Validate these assumptions or clamp values to avoid negative counts.
227-231: Confirm cycleId usage in modal.
Passing cycleId.toString() to TransferIssuesModal is proper, but ensure no edge cases exist if cycleId were undefined or non-string. Quick check is recommended.
Description
A button was added to transfer issues to the completed cycles list.
Type of Change
Screenshots and Media
Screen.Recording.2024-12-31.at.4.13.37.PM.mov
References
WEB-2941
Summary by CodeRabbit
New Features
Bug Fixes
UI Updates