UN-3240 [FEAT] Add HITL navigation to sidebar and remove from profile dropdown#1804
UN-3240 [FEAT] Add HITL navigation to sidebar and remove from profile dropdown#1804vishnuszipstack merged 4 commits intomainfrom
Conversation
Summary by CodeRabbit
WalkthroughRelocates Human-in-the-Loop (HITL) review UI from the top nav to the side nav, adding side-menu HITL items with hover popover and role-based visibility, introduces sidebar icon CSS, and removes review-related code from the top navigation. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx`:
- Around line 407-435: The code may insert duplicate "REVIEW" entries on
re-renders because it unconditionally splices into the shared data array; before
splicing, check whether data already contains an item with mainTitle "REVIEW"
(or an item with isHITL true) and only insert if none exists; update the block
that computes isHITLRole and the conditional using manualReviewSettingsEnabled
&& isHITLRole && isUnstract to also verify e.g. !data.some(item =>
item.mainTitle === "REVIEW" || (item.subMenu && item.subMenu.some(sm =>
sm.isHITL))) before performing data.splice (use the same identifiers: data,
settingsIndex, splice, isHITL).
- Around line 153-209: getActiveHITLKey currently uses includes("/review") which
incorrectly matches other routes (e.g. /settings/review); update
getActiveHITLKey to scope checks to the org review base path by using the
orgName parameter: compute currentPath = globalThis.location.pathname and test
using startsWith or a regex against `/${orgName}/review` (and the more specific
`/${orgName}/review/approve` and `/${orgName}/review/download_and_sync`) so the
function returns "approve", "download" or "review" only when the path actually
belongs to that org's review routes; this ensures HITLPopoverContent and
getHITLMenuItems reflect the correct active key.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (3)
frontend/src/components/navigations/side-nav-bar/SideNavBar.cssfrontend/src/components/navigations/side-nav-bar/SideNavBar.jsxfrontend/src/components/navigations/top-nav-bar/TopNavBar.jsx
💤 Files with no reviewable changes (1)
- frontend/src/components/navigations/top-nav-bar/TopNavBar.jsx
|
@vishnuszipstack Please resolve the conflicts. Seems like code rabbit comment is valid address that too |
frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx
Outdated
Show resolved
Hide resolved
… (UN-3240) Move HITL navigation from the profile dropdown menu to the left sidebar with a hover popover showing role-based menu items (Review for all HITL roles, Approve and Download & Sync for supervisor/admin). Remove the now-redundant Review, Approve, and Download & Sync items from the TopNavBar profile dropdown. Clean up unused state and icon imports. Note: Companion changes for in-page tabs (ReviewHeader), queue deletion (UN-3241), and nested table support (UN-3242) are in the enterprise plugin layer (gitignored). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove unused isAdmin variable in TopNavBar useEffect - Remove unused UNSTRACT_ADMIN import in TopNavBar - Remove unused Modal import from ReviewHeader (used in ManualReviewPage) - Fix prettier formatting in ReviewHeader antd import - Fix prettier formatting in ManualReviewPage delete handler Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
chandrasekharan-zipstack
left a comment
There was a problem hiding this comment.
Drop a screenshot in the PR, also was the design review done?
- Scope getActiveHITLKey to org review routes using startsWith instead of includes - Guard against duplicate REVIEW section insertion on re-render - Extract HITL menu items into a constant array - Remove unused icon imports from TopNavBar - Fix biome lint issues (block statements, unused variable) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
cde0f07 to
b8ac20f
Compare
|
Addressed all review comments and resolved merge conflicts (rebased onto latest main):
Also fixed biome lint issues (block statements, unused variable) and removed unused icon imports from TopNavBar. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1678
✅ getActiveHITLKey scoping - Now correctly uses ✅ Duplicate REVIEW section guard - The ✅ HITL_MENU_ITEMS refactoring - Clean constant array with All fixes look great, and the code is much more maintainable now. Nice work resolving the merge conflicts and addressing the lint issues as well! 🎉 If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
screenshot added in cloud pr @chandrasekharan-zipstack also design review done. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx (1)
184-196: Consider extracting role constants to reduce duplication.The role arrays
["unstract_supervisor", "unstract_admin"](line 185-188) and["unstract_reviewer", "unstract_supervisor", "unstract_admin"](lines 463-466) are defined inline in multiple places. Extracting these to named constants would improve maintainability.♻️ Optional: Extract role constants
+const HITL_SUPERVISOR_ROLES = ["unstract_supervisor", "unstract_admin"]; +const HITL_ALL_ROLES = ["unstract_reviewer", ...HITL_SUPERVISOR_ROLES]; + const HITL_MENU_ITEMS = [ // ... ]; const getHITLMenuItems = (orgName, role) => { - const isSupervisorOrAdmin = [ - "unstract_supervisor", - "unstract_admin", - ].includes(role); + const isSupervisorOrAdmin = HITL_SUPERVISOR_ROLES.includes(role); // ... };Then at line 462:
- const isHITLRole = [ - "unstract_reviewer", - "unstract_supervisor", - "unstract_admin", - ].includes(role); + const isHITLRole = HITL_ALL_ROLES.includes(role);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx` around lines 184 - 196, Extract the repeated role arrays into named constants (e.g., const ROLE_SUPERVISORS = ["unstract_supervisor", "unstract_admin"]; const ROLE_REVIEWERS = ["unstract_reviewer", ...ROLE_SUPERVISORS]) and replace the inline arrays in getHITLMenuItems and the other place that checks ["unstract_reviewer","unstract_supervisor","unstract_admin"] with these constants; update references in getHITLMenuItems (isSupervisorOrAdmin) and the reviewer/supervisor checks so the logic is unchanged but duplication is removed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx`:
- Around line 184-196: Extract the repeated role arrays into named constants
(e.g., const ROLE_SUPERVISORS = ["unstract_supervisor", "unstract_admin"]; const
ROLE_REVIEWERS = ["unstract_reviewer", ...ROLE_SUPERVISORS]) and replace the
inline arrays in getHITLMenuItems and the other place that checks
["unstract_reviewer","unstract_supervisor","unstract_admin"] with these
constants; update references in getHITLMenuItems (isSupervisorOrAdmin) and the
reviewer/supervisor checks so the logic is unchanged but duplication is removed.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (3)
frontend/src/components/navigations/side-nav-bar/SideNavBar.cssfrontend/src/components/navigations/side-nav-bar/SideNavBar.jsxfrontend/src/components/navigations/top-nav-bar/TopNavBar.jsx
Frontend Lint Report (Biome)✅ All checks passed! No linting or formatting issues found. |
|
) * [FIX] Restore HITL sidebar navigation removed by sidebar refactor The sidebar hover UX refactor in #1768 inadvertently removed all HITL navigation code that was added in #1804 (UN-3240). This restores the REVIEW section with role-based popover menu items. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * [FIX] Restore HITL sidebar icon styling removed by sidebar refactor Add back .sidebar-antd-icon CSS that was also removed by #1768. This styles the antd FileProtectOutlined icon to match the sidebar theme (grey by default, white on hover/active). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>



What
Why
How
HITLPopoverContentcomponent with role-based menu items (Review for all HITL roles, Approve and Download & Sync for supervisor/admin). Added conditional "REVIEW" section between MANAGE and SETTINGS usingFileProtectOutlinedicon. Uses same Popover pattern as Platform settings.approverStatus/reviewerStatusstate andUNSTRACT_ADMIN,DownloadOutlined,FileProtectOutlined,LikeOutlinedimports..sidebar-antd-iconstyles for Ant Design icon in sidebar with hover/active color transitions.Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)
reviewPageHeaderlogic in TopNavBar is preserved for page context display.Database Migrations
Env Config
Relevant Docs
Related Issues or PRs
Dependencies Versions