feat(hiring): hiring approvals page + sidebar entry#4432
Open
ramonmatias19 wants to merge 1 commit intopaperclipai:masterfrom
Open
feat(hiring): hiring approvals page + sidebar entry#4432ramonmatias19 wants to merge 1 commit intopaperclipai:masterfrom
ramonmatias19 wants to merge 1 commit intopaperclipai:masterfrom
Conversation
Contributor
Greptile SummaryThis PR adds a dedicated
Confidence Score: 4/5Not safe to merge until the hardcoded Portuguese strings are replaced with i18n keys. One P1 finding (Portuguese hardcoded in a new English-first UI page) needs to be fixed before the feature is usable for non-Portuguese users. Routing and sidebar logic are otherwise correct. ui/src/pages/HiringApprovals.tsx — all user-facing strings need i18n.t() replacements. Important Files Changed
Prompt To Fix All With AIThis is a comment left during a code review.
Path: ui/src/pages/HiringApprovals.tsx
Line: 30-103
Comment:
**Hardcoded Portuguese strings throughout the page**
`HiringApprovals.tsx` is entirely in Portuguese — breadcrumb ("Contratação"), tab labels ("Pendentes" / "Todos"), empty-state copy, and error messages ("Falha ao aprovar", "Falha ao rejeitar") — while the rest of the app (including the `Sidebar.tsx` changes in this same PR) runs through `i18n.t()`. Any non-Portuguese user will see an untranslated UI, and this directly contradicts the i18n migration this PR is supposed to introduce.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: ui/src/App.tsx
Line: 303-304
Comment:
**Unrelated test routes added in hiring-approvals PR**
Two routes — `tests/ux/chat` and `tests/ux/runs` — are added to the `UnprefixedBoardRedirect` block. They have no connection to the hiring-approvals feature and appear to be unintentional scope creep. If these are needed, they should be in a separate PR.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: ui/src/pages/HiringApprovals.tsx
Line: 50-52
Comment:
**`approveMutation` navigates to `/approvals/${id}` instead of a hiring-approvals route**
After a successful approve action, the user is redirected to `/approvals/${id}?resolved=approved` — the generic approvals detail page. While `ApprovalDetail` may be shared, the breadcrumb trail and back-navigation will land the user outside the `/hiring-approvals` context. Consider whether the detail link and post-approve redirect should use a hiring-approvals-scoped path, or at minimum document this is intentional.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "feat(hiring): hiring approvals page + si..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+30
to
+103
| }, [setBreadcrumbs]); | ||
|
|
||
| const { data, isLoading, error } = useQuery({ | ||
| queryKey: queryKeys.approvals.list(selectedCompanyId!), | ||
| queryFn: () => approvalsApi.list(selectedCompanyId!), | ||
| enabled: !!selectedCompanyId, | ||
| }); | ||
|
|
||
| const { data: agents } = useQuery({ | ||
| queryKey: queryKeys.agents.list(selectedCompanyId!), | ||
| queryFn: () => agentsApi.list(selectedCompanyId!), | ||
| enabled: !!selectedCompanyId, | ||
| }); | ||
|
|
||
| const approveMutation = useMutation({ | ||
| mutationFn: (id: string) => approvalsApi.approve(id), | ||
| onSuccess: (_approval, id) => { | ||
| setActionError(null); | ||
| queryClient.invalidateQueries({ queryKey: queryKeys.approvals.list(selectedCompanyId!) }); | ||
| navigate(`/approvals/${id}?resolved=approved`); | ||
| }, | ||
| onError: (err) => { | ||
| setActionError(err instanceof Error ? err.message : "Falha ao aprovar"); | ||
| }, | ||
| }); | ||
|
|
||
| const rejectMutation = useMutation({ | ||
| mutationFn: (id: string) => approvalsApi.reject(id), | ||
| onSuccess: () => { | ||
| setActionError(null); | ||
| queryClient.invalidateQueries({ queryKey: queryKeys.approvals.list(selectedCompanyId!) }); | ||
| }, | ||
| onError: (err) => { | ||
| setActionError(err instanceof Error ? err.message : "Falha ao rejeitar"); | ||
| }, | ||
| }); | ||
|
|
||
| const hiringApprovals = (data ?? []).filter((a) => a.type === "hire_agent"); | ||
|
|
||
| const filtered = hiringApprovals | ||
| .filter( | ||
| (a) => statusFilter === "all" || a.status === "pending" || a.status === "revision_requested", | ||
| ) | ||
| .sort((a, b) => new Date(b.createdAt).getTime() - new Date(a.createdAt).getTime()); | ||
|
|
||
| const pendingCount = hiringApprovals.filter( | ||
| (a) => a.status === "pending" || a.status === "revision_requested", | ||
| ).length; | ||
|
|
||
| if (!selectedCompanyId) { | ||
| return <p className="text-sm text-muted-foreground">Selecione uma empresa primeiro.</p>; | ||
| } | ||
|
|
||
| if (isLoading) { | ||
| return <PageSkeleton variant="approvals" />; | ||
| } | ||
|
|
||
| return ( | ||
| <div className="space-y-4"> | ||
| <div className="flex items-center justify-between"> | ||
| <Tabs value={statusFilter} onValueChange={(v) => navigate(`/hiring-approvals/${v}`)}> | ||
| <PageTabBar items={[ | ||
| { value: "pending", label: <>Pendentes{pendingCount > 0 && ( | ||
| <span className={cn( | ||
| "ml-1.5 rounded-full px-1.5 py-0.5 text-[10px] font-medium", | ||
| "bg-yellow-500/20 text-yellow-500" | ||
| )}> | ||
| {pendingCount} | ||
| </span> | ||
| )}</> }, | ||
| { value: "all", label: "Todos" }, | ||
| ]} /> | ||
| </Tabs> | ||
| </div> |
Contributor
There was a problem hiding this comment.
HiringApprovals.tsx is entirely in Portuguese — breadcrumb ("Contratação"), tab labels ("Pendentes" / "Todos"), empty-state copy, and error messages ("Falha ao aprovar", "Falha ao rejeitar") — while the rest of the app (including the Sidebar.tsx changes in this same PR) runs through i18n.t(). Any non-Portuguese user will see an untranslated UI, and this directly contradicts the i18n migration this PR is supposed to introduce.
Prompt To Fix With AI
This is a comment left during a code review.
Path: ui/src/pages/HiringApprovals.tsx
Line: 30-103
Comment:
**Hardcoded Portuguese strings throughout the page**
`HiringApprovals.tsx` is entirely in Portuguese — breadcrumb ("Contratação"), tab labels ("Pendentes" / "Todos"), empty-state copy, and error messages ("Falha ao aprovar", "Falha ao rejeitar") — while the rest of the app (including the `Sidebar.tsx` changes in this same PR) runs through `i18n.t()`. Any non-Portuguese user will see an untranslated UI, and this directly contradicts the i18n migration this PR is supposed to introduce.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
303
to
+304
| <Route path="execution-workspaces/:workspaceId/issues" element={<UnprefixedBoardRedirect />} /> | ||
| <Route path="hiring-approvals" element={<UnprefixedBoardRedirect />} /> |
Contributor
There was a problem hiding this comment.
Two routes — tests/ux/chat and tests/ux/runs — are added to the UnprefixedBoardRedirect block. They have no connection to the hiring-approvals feature and appear to be unintentional scope creep. If these are needed, they should be in a separate PR.
Prompt To Fix With AI
This is a comment left during a code review.
Path: ui/src/App.tsx
Line: 303-304
Comment:
**Unrelated test routes added in hiring-approvals PR**
Two routes — `tests/ux/chat` and `tests/ux/runs` — are added to the `UnprefixedBoardRedirect` block. They have no connection to the hiring-approvals feature and appear to be unintentional scope creep. If these are needed, they should be in a separate PR.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Comment on lines
+50
to
+52
| }, | ||
| onError: (err) => { | ||
| setActionError(err instanceof Error ? err.message : "Falha ao aprovar"); |
Contributor
There was a problem hiding this comment.
approveMutation navigates to /approvals/${id} instead of a hiring-approvals route
After a successful approve action, the user is redirected to /approvals/${id}?resolved=approved — the generic approvals detail page. While ApprovalDetail may be shared, the breadcrumb trail and back-navigation will land the user outside the /hiring-approvals context. Consider whether the detail link and post-approve redirect should use a hiring-approvals-scoped path, or at minimum document this is intentional.
Prompt To Fix With AI
This is a comment left during a code review.
Path: ui/src/pages/HiringApprovals.tsx
Line: 50-52
Comment:
**`approveMutation` navigates to `/approvals/${id}` instead of a hiring-approvals route**
After a successful approve action, the user is redirected to `/approvals/${id}?resolved=approved` — the generic approvals detail page. While `ApprovalDetail` may be shared, the breadcrumb trail and back-navigation will land the user outside the `/hiring-approvals` context. Consider whether the detail link and post-approve redirect should use a hiring-approvals-scoped path, or at minimum document this is intentional.
How can I resolve this? If you propose a fix, please make it concise.
ramonmatias19
force-pushed
the
feature/hiring-approvals
branch
from
4dd1668 to
54e2809
Compare
Split hiring approvals out of the generic Approvals page so pending agent hires get their own review surface with /pending and /all tabs, linked from the sidebar with a pending-count badge. - Add ui/src/pages/HiringApprovals.tsx - Add /hiring-approvals, /hiring-approvals/pending, /hiring-approvals/all routes (board-scoped + unprefixed redirects) - Add UserPlus sidebar item that surfaces pending hire_agent approvals; also migrates the rest of the sidebar labels to i18n.t() in the same change set
ramonmatias19
force-pushed
the
feature/hiring-approvals
branch
from
54e2809 to
1fd5ac8
Compare
itsderek23
added a commit
to itsderek23/paperclip
that referenced
this pull request
Apr 24, 2026
Three fixes found while running releasebot against PR paperclipai#4432: - ensureShaReachable now falls back to fetching the SHA directly from origin when it's not in pull/<n>/head. This covers the case where origin/master has advanced past the PR base since the last local fetch. - Summary prompt now receives a structured Facts block with exact per-side pass/fail counts and visual-review verdict counts, with an explicit instruction not to invert them. Fixes a hallucination where the summary claimed "Playwright failed on the after side" when the after side had zero failures. - --skip-install now preflights cli/node_modules/tsx/dist/cli.mjs in both worktrees and exits early with a clear message, instead of failing mid-boot with MODULE_NOT_FOUND. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Split hiring approvals out of the generic Approvals page so pending agent hires get their own review surface with
/pendingand/alltabs, linked from the sidebar with a pending-count badge.ui/src/pages/HiringApprovals.tsx/hiring-approvals,/hiring-approvals/pending,/hiring-approvals/allroutes (board-scoped + unprefixed redirects)UserPlussidebar item that surfaces pendinghire_agentapprovals; also migrates the rest of the sidebar labels toi18n.t()in the same change setTest plan
hire_agentapproval — verify it shows on/hiring-approvals/pendingand not/approvals