Skip to content

feat(#347): add bulk document review dialog with shared notes #665

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.

Sign up for GitHub

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

Open
shubham79a wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

feat(#347): add bulk document review dialog with shared notes #665

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 105 additions & 21 deletions app/dashboard/consultant/[consultantId]/(features)/documents/DocumentsTab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,11 @@ export function DocumentsTab({
const [selectedIds, setSelectedIds] = useState<Set<string>>(new Set());
const [isBulkUpdating, setIsBulkUpdating] = useState(false);

// Bulk review dialog state
const [bulkReviewDialogOpen, setBulkReviewDialogOpen] = useState(false);
const [bulkReviewStatus, setBulkReviewStatus] = useState<string>("");
const [bulkReviewNotes, setBulkReviewNotes] = useState<string>("");

// Debounce search input. Search is local to the current page, so we don't
// need to reset server-side pagination on every keystroke.
useEffect(() => {
Expand Down Expand Up @@ -183,7 +188,7 @@ export function DocumentsTab({
setSelectedIds(next);
};

const handleBulkStatusUpdate = async (newStatus: string) => {
const handleBulkStatusUpdate = async (newStatus: string, notes?: string) => {
if (selectedIds.size === 0) return;
setIsBulkUpdating(true);

Expand All @@ -196,7 +201,10 @@ export function DocumentsTab({
{
method: "PATCH",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({ reviewStatus: newStatus }),
body: JSON.stringify({
reviewStatus: newStatus,
reviewNotes: notes?.trim() || null,
Comment on lines 202 to +206

Copilot AI Apr 11, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fetch() resolves even on non-2xx HTTP responses, so this Promise.allSettled([...fetch(...)]) will report fulfilled for 400/401/404 responses. That makes the succeeded/failed counts and toast inaccurate, and can close the dialog even when the server rejected updates. Consider wrapping the request in an async helper that checks response.ok and throws/returns a failure value so non-OK responses are counted as failures (optionally include the error body/message).

Copilot uses AI. Check for mistakes.
}),
},
),
),
Expand All @@ -206,14 +214,17 @@ export function DocumentsTab({
const failed = results.length - succeeded;

toast({
title: "Bulk Update Complete",
title: "Bulk Review Complete",
description: failed
? `${succeeded} updated, ${failed} failed`
: `${succeeded} document${succeeded !== 1 ? "s" : ""} updated to ${getStatusLabel(newStatus)}`,
variant: failed ? "destructive" : "default",
Comment on lines 218 to 221

@gemini-code-assist gemini-code-assist Bot Apr 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The failed count and the resulting toast variant will not accurately reflect server-side errors (e.g., 400 or 500 HTTP responses). This is because fetch only rejects on network failures, and Promise.allSettled considers any resolved promise as fulfilled regardless of the HTTP status code.

To fix this, you should ensure that the promises passed to Promise.allSettled reject when response.ok is false. For example, you can wrap the fetch call in an async function that checks the response status before returning.

});

setSelectedIds(new Set());
setBulkReviewDialogOpen(false);
setBulkReviewStatus("");
setBulkReviewNotes("");
Comment on lines 224 to +227

@gemini-code-assist gemini-code-assist Bot Apr 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The selection is cleared and the dialog is closed regardless of whether some updates failed. If failed > 0, it would be better to keep the failed items selected so the user can see which ones didn't update and potentially try again without having to re-select everything. Keeping the dialog open also allows the user to adjust the notes or status if needed.

Suggested change
setSelectedIds(new Set());
setBulkReviewDialogOpen(false);
setBulkReviewStatus("");
setBulkReviewNotes("");
if (failed === 0) {
setSelectedIds(new Set());
setBulkReviewDialogOpen(false);
setBulkReviewStatus("");
setBulkReviewNotes("");
}

onRefresh?.();
} catch {
toast({
Expand Down Expand Up @@ -408,26 +419,13 @@ export function DocumentsTab({
<span className="text-sm font-medium text-blue-800">
{selectedIds.size} selected
</span>
<Select
value=""
onValueChange={handleBulkStatusUpdate}
<Button
size="sm"
onClick={() => setBulkReviewDialogOpen(true)}
disabled={isBulkUpdating}
>
<SelectTrigger className="w-[180px] h-8">
<SelectValue
placeholder={
isBulkUpdating ? "Updating..." : "Set status..."
}
/>
</SelectTrigger>
<SelectContent>
<SelectItem value="PENDING">Pending</SelectItem>
<SelectItem value="IN_REVIEW">In Review</SelectItem>
<SelectItem value="APPROVED">Approved</SelectItem>
<SelectItem value="REJECTED">Rejected</SelectItem>
<SelectItem value="NEEDS_REVISION">Needs Revision</SelectItem>
</SelectContent>
</Select>
{isBulkUpdating ? "Updating..." : "Review Selected"}
</Button>
<Button
variant="ghost"
size="sm"
Expand Down Expand Up @@ -712,6 +710,92 @@ export function DocumentsTab({
</DialogContent>
</Dialog>

{/* Bulk Review Dialog */}
<Dialog
open={bulkReviewDialogOpen}
onOpenChange={(open) => {
setBulkReviewDialogOpen(open);
if (!open) {
setBulkReviewStatus("");
setBulkReviewNotes("");
}
}}
>
<DialogContent className="sm:max-w-[500px]">
<DialogHeader>
<DialogTitle>Review {selectedIds.size} Documents</DialogTitle>
<DialogDescription>
Set a review status and optional notes for all selected documents.
</DialogDescription>
</DialogHeader>
<div className="grid gap-4 py-4">
<div className="space-y-2">
<label className="text-sm font-medium">Review Status</label>
<Select value={bulkReviewStatus} onValueChange={setBulkReviewStatus}>
<SelectTrigger>
<SelectValue placeholder="Select status" />
</SelectTrigger>
<SelectContent>
<SelectItem value="PENDING">Pending</SelectItem>
<SelectItem value="IN_REVIEW">In Review</SelectItem>
<SelectItem value="APPROVED">Approved</SelectItem>
<SelectItem value="REJECTED">Rejected</SelectItem>
<SelectItem value="NEEDS_REVISION">Needs Revision</SelectItem>
Comment on lines +739 to +743

Copilot AI Apr 11, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The review status options are duplicated in multiple places in this file (status filter at ~360-365, single review dialog at ~658-663, and here). To avoid drift when statuses/labels change, consider extracting a shared REVIEW_STATUSES list (value + label) and mapping it into each <SelectContent>.

Copilot uses AI. Check for mistakes.
</SelectContent>
</Select>
</div>
<div className="space-y-2">
<label className="text-sm font-medium">
Shared Notes{" "}
<span className="text-gray-400 font-normal">(optional)</span>
</label>
<Textarea
placeholder="Add notes that will apply to all selected documents..."
value={bulkReviewNotes}
onChange={(e) => setBulkReviewNotes(e.target.value)}
className="min-h-[100px]"
/>
</div>
<div className="space-y-2">
<label className="text-sm font-medium">Selected Documents</label>
<div className="max-h-[200px] overflow-y-auto border rounded-md p-2 space-y-2">
{documents
.filter((d) => selectedIds.has(d.id))
.map((doc) => (
<div
key={doc.id}
className="flex items-center justify-between text-sm py-1"
>
<span className="truncate mr-2">{doc.originalName}</span>
<Badge
variant="secondary"
className={`shrink-0 ${getStatusColor(doc.reviewStatus)}`}
>
{getStatusLabel(doc.reviewStatus)}
</Badge>
</div>
))}
</div>
</div>
</div>
<DialogFooter>
<Button
variant="outline"
onClick={() => setBulkReviewDialogOpen(false)}
disabled={isBulkUpdating}
>
Cancel
</Button>
<Button
onClick={() => handleBulkStatusUpdate(bulkReviewStatus, bulkReviewNotes)}
disabled={isBulkUpdating || !bulkReviewStatus}
>
{isBulkUpdating ? "Updating..." : "Review All"}
</Button>
</DialogFooter>
</DialogContent>
</Dialog>

{/* Response Upload Dialog */}
{documentForResponse && (
<ConsultantResponseUpload
Expand Down
Loading