-
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-2388] chore: theme changes and workspace draft issue total count updated #5805
Conversation
WalkthroughThe pull request includes modifications to the Changes
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: 0
🧹 Outside diff range and nitpick comments (1)
web/app/[workspaceSlug]/(projects)/drafts/header.tsx (1)
47-51
: Approved: Updated pagination count logicThe change from
paginationInfo?.count
topaginationInfo?.total_count
looks good. This update likely provides a more accurate representation of the total number of draft issues.For improved readability, consider using a ternary operator instead of an if-else statement with an empty fragment:
- {paginationInfo?.total_count && paginationInfo?.total_count > 0 ? ( - <CountChip count={paginationInfo?.total_count} /> - ) : ( - <></> - )} + {paginationInfo?.total_count && paginationInfo?.total_count > 0 ? ( + <CountChip count={paginationInfo.total_count} /> + ) : null}This change eliminates the need for an empty fragment and makes the code more concise.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- apiserver/plane/app/views/workspace/draft.py (1 hunks)
- packages/ui/src/header/helper.tsx (1 hunks)
- web/app/[workspaceSlug]/(projects)/drafts/header.tsx (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/ui/src/header/helper.tsx
🧰 Additional context used
🔇 Additional comments (1)
apiserver/plane/app/views/workspace/draft.py (1)
213-213
: Approved: Correctly updated the model in permission decoratorChanging
model=Issue
tomodel=DraftIssue
in the@allow_permission
decorator aligns the model with thedestroy
method's purpose of deleting draft issues. This ensures that permission checks are accurately applied to theDraftIssue
model.
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: 0
🧹 Outside diff range and nitpick comments (3)
web/ce/components/issues/issue-details/issue-identifier.tsx (2)
37-37
: New componentIssueTypeIdentifier
needs implementationThe
IssueTypeIdentifier
component is correctly defined, but it currently renders an empty fragment. If this is intentional and the implementation is planned for the future, consider adding a TODO comment to clarify the intent and track the pending work.Consider adding a TODO comment like this:
export const IssueTypeIdentifier: FC<TIssueTypeIdentifier> = observer((props) => { // TODO: Implement IssueTypeIdentifier component return <></>; });
32-37
: Consider documenting the feature implementation planThe addition of
TIssueTypeIdentifier
type andIssueTypeIdentifier
component suggests the beginning of a new feature related to issue type identification. To ensure smooth collaboration and feature tracking:
- Consider adding a comment explaining the purpose and planned usage of these new additions.
- If this is part of a larger feature, it might be helpful to create and link to a feature specification document or issue tracker item.
This documentation will help other developers understand the context and future plans for these components.
web/core/components/issues/workspace-draft/draft-issue-block.tsx (1)
54-54
: LGTM: New issue type identifier added correctlyThe
IssueTypeIdentifier
component is correctly implemented with proper conditional rendering. This enhances the component by displaying the issue type when available.For consistency with the surrounding code, consider wrapping the new component in a
div
with the same classes as theIdentifierText
component:-{issue?.type_id && <IssueTypeIdentifier issueTypeId={issue.type_id} />} +{issue?.type_id && ( + <div className="text-xs font-medium text-custom-text-300"> + <IssueTypeIdentifier issueTypeId={issue.type_id} /> + </div> +)}This will ensure consistent styling and layout with the project identifier.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- packages/types/src/workspace-draft-issues/base.d.ts (1 hunks)
- web/ce/components/issues/issue-details/issue-identifier.tsx (2 hunks)
- web/core/components/issues/workspace-draft/draft-issue-block.tsx (2 hunks)
🧰 Additional context used
🔇 Additional comments (4)
packages/types/src/workspace-draft-issues/base.d.ts (1)
30-30
: LGTM! Consider verifying the impact of this change.The addition of the
type_id
property to theTWorkspaceDraftIssue
type is appropriate and consistent with the existing structure. This change suggests that workspace draft issues now have a type classification, which could enhance the functionality and organization of draft issues.To ensure this change doesn't introduce any unintended consequences, please run the following script to check for any usage of
TWorkspaceDraftIssue
that might need updating:This script will help identify areas of the codebase that might need to be updated to accommodate the new
type_id
property.✅ Verification successful
Verified:
type_id
Addition Confirmed SafeThe addition of the
type_id
property to theTWorkspaceDraftIssue
type has been successfully verified. The property is consistently integrated across all usages in the codebase without introducing any conflicts or issues.
type_id: string;
is appropriately added and aligns with existing type structures.- No existing implementations are adversely affected by this change.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for usage of TWorkspaceDraftIssue type in the codebase # Search for files that import or use TWorkspaceDraftIssue rg --type typescript --type tsx "TWorkspaceDraftIssue" -l | xargs -I {} rg --type typescript --type tsx "TWorkspaceDraftIssue" -C 5 {} # Search for any direct usage of the type (e.g., in variable declarations) rg --type typescript --type tsx ": TWorkspaceDraftIssue" -C 5 # Search for any spread operations on TWorkspaceDraftIssue objects rg --type typescript --type tsx "\.\.\.(\w+: TWorkspaceDraftIssue)" -C 5Length of output: 380
Script:
#!/bin/bash # Description: Check for usage of TWorkspaceDraftIssue type in the codebase # Search for files that import or use TWorkspaceDraftIssue rg --type ts --type tsx "TWorkspaceDraftIssue" -l | xargs -I {} rg --type ts --type tsx "TWorkspaceDraftIssue" -C 5 {} # Search for any direct usage of the type (e.g., in variable declarations) rg --type ts --type tsx ": TWorkspaceDraftIssue" -C 5 # Search for any spread operations on TWorkspaceDraftIssue objects rg --type ts --type tsx "\.\.\.(\w+: TWorkspaceDraftIssue)" -C 5Length of output: 327
Script:
#!/bin/bash # Description: Check for usage of TWorkspaceDraftIssue type in the codebase using glob patterns # Search for files that import or use TWorkspaceDraftIssue rg "TWorkspaceDraftIssue" --glob "*.ts" --glob "*.tsx" -l | xargs -I {} rg "TWorkspaceDraftIssue" --glob "*.ts" --glob "*.tsx" -C 5 {} # Search for any direct usage of the type (e.g., in variable declarations) rg ": TWorkspaceDraftIssue" --glob "*.ts" --glob "*.tsx" -C 5 # Search for any spread operations on TWorkspaceDraftIssue objects rg "\.\.\.(\w+: TWorkspaceDraftIssue)" --glob "*.ts" --glob "*.tsx" -C 5Length of output: 20354
web/ce/components/issues/issue-details/issue-identifier.tsx (1)
32-35
: LGTM: New typeTIssueTypeIdentifier
is well-definedThe new type
TIssueTypeIdentifier
is correctly defined with appropriate properties. It maintains consistency with existing size-related properties in the file.web/core/components/issues/workspace-draft/draft-issue-block.tsx (2)
11-11
: LGTM: New import added correctlyThe new import for
IssueTypeIdentifier
is correctly placed and follows the established naming conventions. This import aligns with the new functionality added to the component.
Line range hint
1-138
: Summary: Successful integration of IssueTypeIdentifierThe changes in this file effectively integrate the
IssueTypeIdentifier
component into theDraftIssueBlock
. This addition enhances the component by displaying the issue type alongside the project identifier when available.The implementation is clean, follows existing patterns, and doesn't introduce any breaking changes. It improves the information density of the component without compromising its structure or performance.
Summary:
This PR includes workspace draft issues and theme change.
Summary by CodeRabbit
New Features
CountChip
component to reflect total pagination count.IssueTypeIdentifier
component to enrich the information displayed in the draft issue block.Bug Fixes
Style