[WEB-2401] chore: worklog enhancements #5982
Conversation
anmolsinghbhatia
requested review from
SatishGandham,
rahulramesha and
sriramveeraghanta
WalkthroughThe pull request introduces 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
web/core/components/issues/issue-detail/issue-activity/root.tsx (2)
54-59: LGTM: Well-structured permission checks.The permission logic effectively implements the requirement to restrict worklog access to admins and assignees. The derived values are clearly named and logically structured.
Consider extracting these permission checks into a custom hook for better reusability:
const useWorklogPermissions = ( currentUserProjectRole: string, issue?: Issue, currentUser?: User, isIntakeIssue = false ) => { const isAdmin = (currentUserProjectRole ?? EUserPermissions.GUEST) === EUserPermissions.ADMIN; const isGuest = (currentUserProjectRole ?? EUserPermissions.GUEST) === EUserPermissions.GUEST; const isAssigned = issue?.assignee_ids && currentUser?.id ? issue?.assignee_ids.includes(currentUser?.id) : false; return { isAdmin, isGuest, isAssigned, isWorklogButtonEnabled: !isIntakeIssue && !isGuest && (isAdmin || isAssigned) }; };
Line range hint
82-127: Enhance error handling in activity operations.While the error handling is comprehensive, there are opportunities for improvement:
Consider these enhancements:
// Add type-safe error handling type APIError = { message: string; status?: number; }; const handleError = (error: unknown, operation: string) => { const message = error instanceof Error ? error.message : "An unexpected error occurred"; console.error(`${operation} failed:`, error); setToast({ title: "Error!", type: TOAST_TYPE.ERROR, message: `${operation} failed. ${message}`, }); }; // Usage in operations: createComment: async (data) => { try { if (!workspaceSlug || !projectId || !issueId) throw new Error("Required fields missing: workspace, project, or issue ID"); const comment = await createComment(workspaceSlug, projectId, issueId, data); setToast({ title: "Success!", type: TOAST_TYPE.SUCCESS, message: "Comment created successfully.", }); return comment; } catch (error) { handleError(error, "Comment creation"); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
web/core/components/issues/issue-detail/issue-activity/root.tsx(2 hunks)
🔇 Additional comments (2)
web/core/components/issues/issue-detail/issue-activity/root.tsx (2)
14-14: LGTM: Import addition aligns with user validation requirements.
The addition of the useUser hook is necessary for implementing the user-based validation for time log entries.
Line range hint 156-164: LGTM: Proper implementation of permission-based UI rendering.
The worklog button visibility is correctly controlled by the permission checks, aligning with the requirement to restrict access to admins and assignees.
Let's verify the permission checks are consistently applied:
✅ Verification successful
Permission checks for worklog button are correctly implemented
The verification confirms that the worklog button's visibility is properly controlled through the isWorklogButtonEnabled flag, which implements the following checks:
- Prevents access for guest users (
!isGuest) - Restricts access to admins or assignees (
isAdmin || isAssigned) - Excludes intake issues (
!isIntakeIssue)
The implementation aligns with the permission requirements and is consistently applied in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other worklog-related components to ensure consistent permission checks
rg -l "worklog" --type tsx
# Search for similar permission patterns to ensure consistency
ast-grep --pattern 'const is$_ = $_'
Length of output: 93
Script:
#!/bin/bash
# Search for worklog-related components with .tsx extension
rg -l "worklog" -g "*.tsx"
# Search for permission-related patterns in TypeScript/React files
rg -g "*.tsx" "const is\w+Enabled" -A 2
Length of output: 8500
Script:
#!/bin/bash
# Check the worklog button component implementation for permission checks
rg -g "*.tsx" "IssueActivityWorklogCreateButton" -B 5 -A 5
# Check the implementation of isWorklogButtonEnabled in related files
rg -g "*.tsx" "isWorklogButtonEnabled" -B 3 -A 3
Length of output: 5435
Changes:
This PR includes following changes:
Reference:
[WEB-2401]
Summary by CodeRabbit
New Features
IssueActivitycomponent to incorporate user roles and permissions for improved functionality.Bug Fixes
Refactor