-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WEB-2941]chore: added transfer issues button to cycles #6294
Conversation
*feat: added transfer issues button to list
WalkthroughThis pull request introduces modifications across several components related to cycle management in a web application. The changes primarily focus on enhancing the transfer issues functionality, updating import paths, and adding new constants for cycle actions. The modifications span multiple files in the web core and UI components, introducing a new transfer issues modal, updating cycle-related stores, and adjusting component imports and exports. Changes
Possibly related PRs
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
web/core/components/cycles/transfer-issues-modal.tsx (1)
52-52
: Error toast for issue transfer.
The message is concise. Confirm whether more specific user guidance is needed for different error conditions.web/core/components/cycles/list/cycle-list-item-action.tsx (1)
248-259
: Conditional button for transferring issues.
This is a clear UI indicator. The label “Transfer {transferableIssuesCount} issues” is direct. Ensure a good UX if the user quickly toggles the cycle status or tries to open the modal multiple times.web/core/store/cycle.store.ts (1)
724-726
: ImplementupdateCycleState
functionality.
Currently empty. Provide the logic or remove if not needed yet. If it’s a placeholder, consider using a TODO comment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/ui/src/icons/transfer-icon.tsx
(1 hunks)web/ce/components/cycles/index.ts
(1 hunks)web/core/components/cycles/index.ts
(0 hunks)web/core/components/cycles/list/cycle-list-item-action.tsx
(7 hunks)web/core/components/cycles/list/cycles-list-item.tsx
(1 hunks)web/core/components/cycles/transfer-issues-modal.tsx
(3 hunks)web/core/components/issues/issue-layouts/roots/cycle-layout-root.tsx
(1 hunks)web/core/constants/cycle.ts
(1 hunks)web/core/store/cycle.store.ts
(3 hunks)web/ee/components/cycles/index.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- web/core/components/cycles/index.ts
✅ Files skipped from review due to trivial changes (2)
- packages/ui/src/icons/transfer-icon.tsx
- web/core/components/cycles/list/cycles-list-item.tsx
🔇 Additional comments (19)
web/ce/components/cycles/index.ts (1)
3-3
: Re-exporting quick-actions
is aligned with the new file structure.
This line re-exposes the functionalities from ./quick-actions
to consumers of this module. This looks good and consistent with the module's export strategy.
web/ee/components/cycles/index.ts (1)
3-3
: Confirm correctness of cross-edition reference.
Here the quick-actions
from the community edition (ce/components/cycles
) are being re-exported in the enterprise edition. Make sure it’s valid for the enterprise codebase to rely on community edition components. If so, it makes sense to keep this code DRY.
Consider running a check to ensure the path is correct and accessible:
✅ Verification successful
Let me verify the imports and cross-edition references further.
Let me try the searches again with correct file type specifications.
Let me try one last search with simpler patterns.
Cross-edition reference is valid and follows established pattern
The verification shows that:
- The re-export of CE components in EE is a common and established pattern in this codebase, with numerous examples across different components and modules.
- The specific path
ce/components/cycles/quick-actions
exists and is correctly referenced. - The cycles component structure is mirrored between CE and EE directories, indicating proper organization.
- The quick-actions component in CE is a legitimate React component with proper implementation.
This re-export helps maintain DRY principles by reusing CE components in EE when the functionality is identical, which is a valid architectural decision.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Using fd to find the quick-actions index, verifying the path "ce/components/cycles/quick-actions"
fd quick-actions
Length of output: 278
Script:
#!/bin/bash
# Check the content of the quick-actions file in CE
cat web/ce/components/cycles/quick-actions.tsx
# Check if there are any other imports of quick-actions from CE to EE
rg "from ['\"]ce/components/cycles/quick-actions" --type ts --type tsx
# Check if there's a pattern of CE to EE imports
rg "from ['\"]ce/" web/ee/ --type ts --type tsx
Length of output: 6681
Script:
#!/bin/bash
# Check for imports of quick-actions from CE to EE
rg -t typescript -t tsx "from ['\"]ce/components/cycles/quick-actions"
# Check for the pattern of CE to EE imports
rg -t typescript -t tsx "from ['\"]ce/" web/ee/
# Check if there's a pattern of CE components being re-exported in EE
rg -t typescript -t tsx "export.*from ['\"]ce/" web/ee/
Length of output: 291
Script:
#!/bin/bash
# Check for any imports or exports from CE in EE
rg "from.*ce/" web/ee/
rg "export.*from.*ce/" web/ee/
# Check the directory structure to understand the relationship between CE and EE
ls -la web/ce/components/cycles/
ls -la web/ee/components/cycles/
Length of output: 17990
web/core/constants/cycle.ts (1)
131-134
: Enum addition looks consistent with typical usage.
Adding ECycleAction
clarifies cycle actions, improving readability. The naming convention is clear and expandable for future actions.
web/core/components/cycles/transfer-issues-modal.tsx (7)
20-20
: Confirm cycleId
necessity and usage.
If there's a scenario where cycleId
might be undefined or irrelevant, consider making it optional. Otherwise, this approach is acceptable.
24-24
: Destructure cycleId
appropriately.
This clearly captures the new prop. No issues noted, though double-check if cycleId
must always be required.
29-29
: Efficient usage of store hooks.
Using currentProjectIncompleteCycleIds
, getCycleById
, and fetchCycleDetails
from useCycle()
is straightforward and maintains clarity.
36-46
: Resolve promise handling for transferIssue
.
The success callback is clearly setting UI feedback via toast and calling getCycleDetails
. This is good. If multiple async calls begin to stack, consider using a try/catch
block for improved maintainability.
57-70
: getCycleDetails
usage of Promise.all
.
Fetching both current and target cycle details in parallel is efficient. The catch block properly handles toast notifications. This is well structured.
115-116
: Icon usage clarity.
Displaying the TransferIcon
alongside a heading is intuitive. The styling with .fill-custom-text-100
ensures consistent appearance. No issues here.
126-126
: Styling consistency in input field.
The simplified class names improve readability. Looks good.
web/core/components/cycles/list/cycle-list-item-action.tsx (7)
3-3
: Refactor note on imports.
Imports remain organized and readable. If the file grows, consider grouping or conditionally loading heavy modules.
18-18
: Transfer icon import.
All relevant UI icons are imported together. This maintains consistency across the codebase.
23-23
: Conditional rendering for TransferIssuesModal
.
Bringing the modal into this file is logical, keeping related functionality together. Be mindful of import paths if the structure changes again.
36-37
: Using CycleQuickActions
.
Organizing quick actions under plane-web
aligns with the recent file restructuring. No issues noted.
61-62
: transferIssuesModal
state.
This state gating the visibility of the modal is straightforward. Good use of booleans for toggling modals.
84-85
: transferableIssuesCount
and showTransferIssues
.
This logic is concise. However, confirm if the cycle can have a completed status with zero completed issues. Possibly handle negative values or other edge cases.
228-232
: Passing cycleId
to TransferIssuesModal
.
Forwarding cycleId.toString()
ensures correct types. This further integrates the new property in the modal.
web/core/store/cycle.store.ts (2)
16-17
: Tracked cycle actions with ECycleAction
.
Introducing a specialized enum clarifies allowed actions. This is a good approach to strongly typed states.
96-96
: updateCycleState
signature in ICycleStore
.
Defines a clear contract for externally updating the cycle state. Ensure other store methods can integrate with this action consistently.
web/core/components/issues/issue-layouts/roots/cycle-layout-root.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
web/core/store/cycle.store.ts (1)
16-17
: Consider implementing a state machine for cycle transitions.The introduction of
ECycleAction
enum suggests a state-based approach to cycle management. Consider implementing a proper state machine to handle cycle transitions and validations.Recommendations:
- Define valid state transitions using the
ECycleAction
enum- Implement validation logic for state transitions
- Add transition hooks for side effects (e.g., notifications)
- Consider using a state machine library like XState for complex state management
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/core/components/issues/issue-layouts/roots/cycle-layout-root.tsx
(1 hunks)web/core/store/cycle.store.ts
(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/core/components/issues/issue-layouts/roots/cycle-layout-root.tsx
Description
A button was added to transfer issues to the completed cycles list.
Restructured files for cycles quick actions.
Updated transfer Icon view box.
Type of Change
Screenshots and Media
Screen.Recording.2024-12-31.at.4.13.37.PM.mov
References
WEB-2941
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style