-
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-2678]feat: added functionality to add labels directly from dropdown #6211
Conversation
Warning Rate limit exceeded@mathalav55 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 7 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThe pull request introduces modifications to several components related to issue label management across the web application. The changes primarily focus on simplifying user permission handling, enhancing label creation functionality, and improving the interaction flow for adding labels. Key modifications include removing explicit user permission checks, adding new methods for label creation, and updating component interfaces to support more dynamic label management. Changes
Sequence DiagramsequenceDiagram
participant User
participant LabelSelect
participant LabelOperations
participant Backend
User->>LabelSelect: Enter label name
LabelSelect->>LabelOperations: onAddLabel()
LabelOperations->>Backend: Create Label
Backend-->>LabelOperations: Label Created
LabelOperations-->>LabelSelect: Update State
LabelSelect-->>User: Display New Label
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (9)
web/core/components/issues/issue-detail/label/select/label-select.tsx (4)
21-21
: Consider makingonAddLabel
optional in theIIssueLabelSelect
interfaceThe
onAddLabel
prop is added to theIIssueLabelSelect
interface. If there are scenarios whereIssueLabelSelect
doesn't require anonAddLabel
function, consider making this prop optional to enhance reusability.
25-36
: Initialize thesubmitting
state variable with explicit typingEnsure that the newly added
submitting
state variable is properly typed for better type safety and code clarity.
93-105
: RefactorsearchInputKeyDown
function for improved readabilityThe
searchInputKeyDown
function contains nested conditions that can be simplified. Consider restructuring the conditional logic to make the code more readable.Apply this diff to refactor the function:
const searchInputKeyDown = async (e: React.KeyboardEvent<HTMLInputElement>) => { if (e.key === "Escape" && query !== "") { e.stopPropagation(); setQuery(""); } else if (e.key === "Enter" && query !== "") { e.stopPropagation(); e.preventDefault(); await handleAddLabel(query); } };
183-193
: Ensure consistent loading indicators during label submissionWhen
submitting
is true, consider disabling the "Add" option to prevent multiple submissions. Also, ensure that theLoader
component is appropriately styled and positioned for better user experience.web/core/components/issues/issue-layouts/properties/labels.tsx (4)
68-68
: Properly initialize and type thesubmitting
state variableEnsure that the
submitting
state variable is explicitly typed to maintain type safety across the component.
77-81
: Remove unused imports and variablesThe
allowPermissions
import and related variables appear unused after modifications. Removing unused code enhances maintainability.
112-122
: RefactorsearchInputKeyDown
function for claritySimplify the conditional statements in
searchInputKeyDown
to improve readability and maintain consistency with similar functions.Apply this diff:
const searchInputKeyDown = async (e: React.KeyboardEvent<HTMLInputElement>) => { if (e.key === "Escape" && query !== "") { e.stopPropagation(); setQuery(""); } else if (e.key === "Enter" && query !== "") { e.stopPropagation(); e.preventDefault(); await handleAddLabel(query); } };
339-349
: Handle loading and submission states appropriately in the UIWhen
submitting
is true, update the UI to reflect the loading state, such as disabling inputs or showing a spinner, to improve user experience.web/core/components/issues/issue-layouts/kanban/block.tsx (1)
Line range hint
196-199
: Consider extracting URL construction logicThe URL construction logic in the
ControlLink
component could be made more maintainable by extracting it into a helper function.Consider creating a helper function:
+const getIssueUrl = ( + workspaceSlug: string, + issue: TIssue, + isEpic: boolean +): string => { + const baseUrl = `/${workspaceSlug}/projects/${issue.project_id}`; + const archivePath = issue.archived_at ? "archives/" : ""; + const issueType = isEpic ? "epics" : "issues"; + + return `${baseUrl}/${archivePath}${issueType}/${issue.id}`; +}; -<ControlLink - href={`/${workspaceSlug}/projects/${issue.project_id}/${issue.archived_at ? "archives/" : ""}${isEpic ? "epics" : "issues"}/${ - issue.id - }`} +<ControlLink + href={getIssueUrl(workspaceSlug, issue, isEpic)}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
web/core/components/issues/issue-detail/label/root.tsx
(1 hunks)web/core/components/issues/issue-detail/label/select/label-select.tsx
(3 hunks)web/core/components/issues/issue-detail/label/select/root.tsx
(1 hunks)web/core/components/issues/issue-layouts/kanban/block.tsx
(1 hunks)web/core/components/issues/issue-layouts/properties/labels.tsx
(7 hunks)
🔇 Additional comments (7)
web/core/components/issues/issue-detail/label/select/root.tsx (1)
29-29
: Pass onAddLabel
prop correctly to IssueLabelSelect
Adding the onAddLabel
prop enhances the label creation functionality. Ensure that it's correctly passed and that all dependencies are satisfied.
web/core/components/issues/issue-detail/label/root.tsx (2)
10-13
: Confirm the removal of user permission checks
The import and use of useUserPermissions
have been removed. Verify that this change aligns with the application's permission handling strategy and that necessary checks are in place elsewhere.
Line range hint 82-82
: Assess the impact of removing permission-based conditional rendering
By eliminating the conditional rendering based on user permissions, the ability to create labels might be accessible to all users. Ensure this is intentional and complies with the application's access control policies.
web/core/components/issues/issue-layouts/kanban/block.tsx (4)
9-9
: LGTM! Clean import addition
The addition of EIssueServiceType
import and its usage to differentiate between epics and regular issues is clean and appropriate.
Line range hint 28-43
: LGTM! Well-structured interface definitions
The interfaces are well-defined with proper typing and maintain backward compatibility through optional props. The isEpic
flag is appropriately added to support both epics and regular issues.
Also applies to: 45-54
Line range hint 156-170
: LGTM! Robust drag-and-drop implementation
The drag-and-drop implementation includes proper error handling with clear user feedback and correct permission checks. The toast messages are informative and help users understand why certain actions are not allowed.
Line range hint 1-227
: Verify integration with label management functionality
While the changes in this file look good, let's verify how it integrates with the label management enhancements mentioned in the PR objectives.
✅ Verification successful
Based on the search results, I can now provide a conclusive response about the integration with label management functionality:
Label management is properly integrated in the Kanban issue block
The code shows proper label management integration through:
- The
IssueProperties
component which includes label handling viaIssueBlockLabels
component - Proper permissions handling through
canEditIssueProperties
checks - Label display properties management through
displayProperties
prop
The changes in this file maintain proper label management by:
- Using the
IssueProperties
component which includes the label management functionality - Properly passing through the
displayProperties
andisReadOnly
props that control label editing permissions - Maintaining the component structure that allows label updates through the
updateIssue
prop
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the integration between KanbanIssueBlock and label management components
# Check for label-related component imports and usage
rg -l "IssueLabelSelect|IssuePropertyLabels" --type tsx
# Check for label-related props and handlers
rg "onAddLabel|handleLabelCreate" --type tsx -A 5
Length of output: 165
Script:
#!/bin/bash
# Let's try without file type restriction since tsx extension is not recognized
# Check for label-related components and their integration
rg -l "IssueLabelSelect|IssuePropertyLabels"
# Look for label management in IssueProperties component since it's used in the KanbanIssueBlock
rg "IssueProperties" -A 10
# Check the properties component implementation
fd "properties" --exec cat {} \;
Length of output: 66995
const handleAddLabel = async (labelName: string) => { | ||
setSubmitting(true); | ||
const label = await onAddLabel(workspaceSlug, projectId, { name: labelName, color: getRandomLabelColor() }); | ||
onSelect([...values, label.id]); | ||
setQuery(""); | ||
setSubmitting(false); |
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.
Add error handling to handleAddLabel
function
Currently, handleAddLabel
does not handle errors that might occur during label creation. To enhance robustness, wrap the async call in a try-catch block and handle potential exceptions.
Apply this diff to add error handling:
const handleAddLabel = async (labelName: string) => {
setSubmitting(true);
+ try {
const label = await onAddLabel(workspaceSlug, projectId, { name: labelName, color: getRandomLabelColor() });
onSelect([...values, label.id]);
setQuery("");
+ } catch (error) {
+ // Handle error, e.g., display an error message to the user
+ console.error("Failed to add label:", error);
+ }
setSubmitting(false);
};
Committable suggestion skipped: line range outside the PR's diff.
const { isMobile } = usePlatformOS(); | ||
const storeLabels = getProjectLabels(projectId); | ||
const { allowPermissions } = useUserPermissions(); | ||
|
||
const canCreateLabel = allowPermissions([EUserPermissions.ADMIN], EUserPermissionsLevel.PROJECT); |
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.
Utilize the canCreateLabel
variable or remove it
The canCreateLabel
variable is defined but not used. Ensure it's utilized where necessary or remove it to clean up the code.
const handleAddLabel = async (labelName: string) => { | ||
if (!projectId) return; | ||
setSubmitting(true); | ||
const label = await createLabel(workspaceSlug, projectId, { name: labelName, color: getRandomLabelColor() }); | ||
onChange([...value, label.id]); | ||
setQuery(""); | ||
setSubmitting(false); | ||
}; | ||
|
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.
Add error handling to handleAddLabel
function
Include error handling in handleAddLabel
to manage potential exceptions during label creation, enhancing the reliability of the component.
Apply this diff:
const handleAddLabel = async (labelName: string) => {
if (!projectId) return;
setSubmitting(true);
+ try {
const label = await createLabel(workspaceSlug, projectId, { name: labelName, color: getRandomLabelColor() });
onChange([...value, label.id]);
setQuery("");
+ } catch (error) {
+ // Handle error, e.g., display an error message to the user
+ console.error("Failed to add label:", error);
+ }
setSubmitting(false);
};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const handleAddLabel = async (labelName: string) => { | |
if (!projectId) return; | |
setSubmitting(true); | |
const label = await createLabel(workspaceSlug, projectId, { name: labelName, color: getRandomLabelColor() }); | |
onChange([...value, label.id]); | |
setQuery(""); | |
setSubmitting(false); | |
}; | |
const handleAddLabel = async (labelName: string) => { | |
if (!projectId) return; | |
setSubmitting(true); | |
try { | |
const label = await createLabel(workspaceSlug, projectId, { name: labelName, color: getRandomLabelColor() }); | |
onChange([...value, label.id]); | |
setQuery(""); | |
} catch (error) { | |
// Handle error, e.g., display an error message to the user | |
console.error("Failed to add label:", error); | |
} | |
setSubmitting(false); | |
}; |
…own (#6211) * enhancement:added functionality to add features directly from dropdown * fix: fixed import order * fix: fixed lint errors
* fix: adding language support package * fix: language support implementation using mobx * fix: adding more languages for support * fix: profile settings translations * feat: added language support for sidebar and user settings * feat: added language support for deactivation modal * fix: added project sync after transfer issues (#6200) * code refactor and improvement (#6203) * chore: package code refactoring * chore: component restructuring and refactor * chore: comment create improvement * refactor: enhance workspace and project wrapper modularity (#6207) * [WEB-2678]feat: added functionality to add labels directly from dropdown (#6211) * enhancement:added functionality to add features directly from dropdown * fix: fixed import order * fix: fixed lint errors * chore: added common component for project activity (#6212) * chore: added common component for project activity * fix: added enum * fix: added enum for initiatives * - Do not clear temp files that are locked. (#6214) - Handle edge cases in sync workspace * fix: labels empty state for drop down (#6216) * refactor: remove cn helper function from the editor package (#6217) * * feat: added language support to issue create modal in sidebar * fix: project activity type * * fix: added missing translations * fix: modified translation for plurals * fix: fixed spanish translation * dev: language type error in space user profile types * fix: type fixes * chore: added alpha tag --------- Co-authored-by: sriram veeraghanta <[email protected]> Co-authored-by: Anmol Singh Bhatia <[email protected]> Co-authored-by: Prateek Shourya <[email protected]> Co-authored-by: Akshita Goyal <[email protected]> Co-authored-by: Satish Gandham <[email protected]> Co-authored-by: Aaryan Khandelwal <[email protected]> Co-authored-by: gurusinath <[email protected]>
* fix: adding language support package * fix: language support implementation using mobx * fix: adding more languages for support * fix: profile settings translations * feat: added language support for sidebar and user settings * feat: added language support for deactivation modal * fix: added project sync after transfer issues (#6200) * code refactor and improvement (#6203) * chore: package code refactoring * chore: component restructuring and refactor * chore: comment create improvement * refactor: enhance workspace and project wrapper modularity (#6207) * [WEB-2678]feat: added functionality to add labels directly from dropdown (#6211) * enhancement:added functionality to add features directly from dropdown * fix: fixed import order * fix: fixed lint errors * chore: added common component for project activity (#6212) * chore: added common component for project activity * fix: added enum * fix: added enum for initiatives * - Do not clear temp files that are locked. (#6214) - Handle edge cases in sync workspace * fix: labels empty state for drop down (#6216) * refactor: remove cn helper function from the editor package (#6217) * * feat: added language support to issue create modal in sidebar * fix: project activity type * * fix: added missing translations * fix: modified translation for plurals * fix: fixed spanish translation * dev: language type error in space user profile types * fix: type fixes * chore: added alpha tag --------- Co-authored-by: sriram veeraghanta <[email protected]> Co-authored-by: Anmol Singh Bhatia <[email protected]> Co-authored-by: Prateek Shourya <[email protected]> Co-authored-by: Akshita Goyal <[email protected]> Co-authored-by: Satish Gandham <[email protected]> Co-authored-by: Aaryan Khandelwal <[email protected]> Co-authored-by: gurusinath <[email protected]>
* WIP * WIP * WIP * WIP * Create home preference if not exist * chore: handled the unique state name validation (#6299) * fix: changed the response structure (#6301) * [WEB-1964]chore: cycles actions restructuring (#6298) * chore: cycles quick actions restructuring * chore: added additional actions to cycle list actions * chore: cycle quick action structure * chore: added additional actions to cycle list actions * chore: added end cycle hook * fix: updated end cycle export --------- Co-authored-by: gurusinath <[email protected]> * fix: active cycle graph tooltip and endpoint validation (#6306) * [WEB-2870]feat: language support (#6215) * fix: adding language support package * fix: language support implementation using mobx * fix: adding more languages for support * fix: profile settings translations * feat: added language support for sidebar and user settings * feat: added language support for deactivation modal * fix: added project sync after transfer issues (#6200) * code refactor and improvement (#6203) * chore: package code refactoring * chore: component restructuring and refactor * chore: comment create improvement * refactor: enhance workspace and project wrapper modularity (#6207) * [WEB-2678]feat: added functionality to add labels directly from dropdown (#6211) * enhancement:added functionality to add features directly from dropdown * fix: fixed import order * fix: fixed lint errors * chore: added common component for project activity (#6212) * chore: added common component for project activity * fix: added enum * fix: added enum for initiatives * - Do not clear temp files that are locked. (#6214) - Handle edge cases in sync workspace * fix: labels empty state for drop down (#6216) * refactor: remove cn helper function from the editor package (#6217) * * feat: added language support to issue create modal in sidebar * fix: project activity type * * fix: added missing translations * fix: modified translation for plurals * fix: fixed spanish translation * dev: language type error in space user profile types * fix: type fixes * chore: added alpha tag --------- Co-authored-by: sriram veeraghanta <[email protected]> Co-authored-by: Anmol Singh Bhatia <[email protected]> Co-authored-by: Prateek Shourya <[email protected]> Co-authored-by: Akshita Goyal <[email protected]> Co-authored-by: Satish Gandham <[email protected]> Co-authored-by: Aaryan Khandelwal <[email protected]> Co-authored-by: gurusinath <[email protected]> * feat: introduced stacked bar chart and tree map chart. (#6305) * feat: add issue attachment external endpoint (#6307) * [PE-97] chore: re-order pages options (#6303) * chore: re-order pages dropdown options * chore: re-order pages dropdown options * fix: remove localdb tracing * [WEB-2937] feat: home recent activies list endpoint (#6295) * Crud for wuick links * Validate quick link existence * Add custom method for destroy and retrieve * Add List method * Remove print statements * List all the workspace quick links * feat: endpoint to get recently active items * Resolve conflicts * Resolve conflicts * Add filter to only list required entities * Return required fields * Add filter * Add filter * fix: remove emoji edit for uneditable pages (#6304) * Removed duplicate imports * feat: patch api * Enable sort order to be updatable * Return key name only insert missing keys use serializer to return data * Remove random generation of sort_order * Remove name field Remove random generation of sort_order --------- Co-authored-by: Bavisetti Narayan <[email protected]> Co-authored-by: Vamsi Krishna <[email protected]> Co-authored-by: gurusinath <[email protected]> Co-authored-by: Anmol Singh Bhatia <[email protected]> Co-authored-by: sriram veeraghanta <[email protected]> Co-authored-by: Prateek Shourya <[email protected]> Co-authored-by: Akshita Goyal <[email protected]> Co-authored-by: Satish Gandham <[email protected]> Co-authored-by: Aaryan Khandelwal <[email protected]> Co-authored-by: Nikhil <[email protected]>
Description
Added functionality to add labels directly from drop-down if the label does not exist.
Type of Change
References
WEB-2678
Summary by CodeRabbit
New Features
Bug Fixes
Documentation