Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
274 changes: 274 additions & 0 deletions apps/admin/src/app/(admin)/assets/[id]/asset-edit-client.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,274 @@
'use client';

import { useState, useTransition } from 'react';
import { useRouter } from 'next/navigation';
import { toast } from 'sonner';
import { Archive, ArchiveRestore, Trash2 } from 'lucide-react';
import { Button } from '@/components/ui/button';
import { Input } from '@/components/ui/input';
import { Textarea } from '@/components/ui/textarea';
import { Separator } from '@/components/ui/separator';
import {
Select,
SelectContent,
SelectItem,
SelectTrigger,
SelectValue,
} from '@/components/ui/select';
import { confirm } from '@/components/ui/confirm-dialog';
import {
updateAsset,
disposeAsset,
reactivateAsset,
deleteAsset,
} from '@/app/actions/assets-actions';
import type { AssetRow } from '@pmg/db';

type AssetKind = 'fixed_asset' | 'investment';

interface AssetEditClientProps {
asset: AssetRow;
}

export function AssetEditClient({ asset }: AssetEditClientProps) {
const router = useRouter();
const [, startTransition] = useTransition();

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.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Use the pending flag from useTransition.

Line 35 discards the pending value. isSubmitting is set only in handleSave. handleDispose, handleReactivate, and handleDelete never set it, so the buttons on lines 240-270 stay enabled while those actions run. A user can trigger the same mutation several times.

🐛 Proposed fix
-  const [, startTransition] = useTransition();
+  const [isPending, startTransition] = useTransition();

Then replace the local flag with a combined value and remove the manual setIsSubmitting calls:

-  const [isSubmitting, setIsSubmitting] = useState(false);
+  const busy = isPending;

Use busy for every disabled prop and for the "Saving…" label.

Also applies to: 89-122

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/admin/src/app/`(admin)/assets/[id]/asset-edit-client.tsx at line 35,
Update the useTransition call in the asset edit client to retain its pending
flag, combine it with the existing mutation state as the shared busy value, and
remove the manual setIsSubmitting updates from handleSave, handleDispose,
handleReactivate, and handleDelete. Use busy for every action button’s disabled
prop and the “Saving…” label so all mutations prevent repeated submissions while
running.


const [kind, setKind] = useState<AssetKind>(asset.kind as AssetKind);
const [name, setName] = useState(asset.name);
const [category, setCategory] = useState(asset.category);
const [acquisitionDate, setAcquisitionDate] = useState(asset.acquisitionDate);
const [cost, setCost] = useState(asset.cost);
const [currentValue, setCurrentValue] = useState(asset.currentValue ?? '');
const [notes, setNotes] = useState(asset.notes ?? '');
const [serialNumber, setSerialNumber] = useState(asset.serialNumber ?? '');
const [location, setLocation] = useState(asset.location ?? '');
const [assignedTo, setAssignedTo] = useState(asset.assignedTo ?? '');
const [quantity, setQuantity] = useState(asset.quantity ?? '');
const [unitType, setUnitType] = useState(asset.unitType ?? '');
const [isSubmitting, setIsSubmitting] = useState(false);
const [error, setError] = useState<string | null>(null);

function handleSave() {
setError(null);
if (!name.trim()) { setError('Name is required.'); return; }
if (!category.trim()) { setError('Category is required.'); return; }
if (!acquisitionDate) { setError('Acquisition date is required.'); return; }
if (!cost || parseFloat(cost) < 0) { setError('A valid cost is required.'); return; }
if (kind === 'investment' && (!quantity || !unitType.trim())) {
setError('Quantity and unit type are required for investments.');
return;
}

setIsSubmitting(true);
startTransition(async () => {
const result = await updateAsset(asset.id, {
kind,
name: name.trim(),
category: category.trim(),
acquisitionDate,
cost: parseFloat(cost),
currentValue: currentValue !== '' && currentValue != null ? parseFloat(String(currentValue)) : null,
notes: notes.trim() || null,
serialNumber: serialNumber.trim() || null,
location: location.trim() || null,
assignedTo: assignedTo.trim() || null,
quantity: quantity !== '' && quantity != null ? parseFloat(String(quantity)) : null,
unitType: unitType.trim() || null,
});
setIsSubmitting(false);
if (result.error) {
setError(result.error);
} else {
toast.success('Asset saved.');
router.push('/assets');
}
});
}

function handleDispose() {
startTransition(async () => {
const result = await disposeAsset(asset.id);
if (result.error) toast.error(result.error);
else { toast.success('Asset marked as disposed.'); router.refresh(); }
});
}

function handleReactivate() {
startTransition(async () => {
const result = await reactivateAsset(asset.id);
if (result.error) toast.error(result.error);
else { toast.success('Asset reactivated.'); router.refresh(); }
});
}

async function handleDelete() {
const confirmed = await confirm({
title: 'Delete this asset?',
description: 'This cannot be undone.',
confirmText: 'Delete',
variant: 'destructive',
});
if (!confirmed) return;
startTransition(async () => {
const result = await deleteAsset(asset.id);
if (result.error) {
toast.error(result.error);
} else {
toast.success('Asset deleted.');
router.push('/assets');
}
});
}

return (
<div className="flex flex-col gap-4">
<div className="flex flex-col gap-1.5">
<label className="text-sm font-medium">
Type <span className="text-destructive">*</span>
</label>
<Select value={kind} onValueChange={(v) => setKind(v as AssetKind)} disabled={isSubmitting}>
<SelectTrigger className="w-full">
<SelectValue />
</SelectTrigger>
<SelectContent>
<SelectItem value="fixed_asset">Fixed Asset</SelectItem>
<SelectItem value="investment">Investment</SelectItem>
</SelectContent>
</Select>
</div>

<div className="flex flex-col gap-1.5">
<label className="text-sm font-medium">
Name <span className="text-destructive">*</span>
</label>
<Input value={name} onChange={(e) => setName(e.target.value)} disabled={isSubmitting} />
</div>

<div className="grid grid-cols-2 gap-4">
<div className="flex flex-col gap-1.5">
<label className="text-sm font-medium">Category <span className="text-destructive">*</span></label>
<Input value={category} onChange={(e) => setCategory(e.target.value)} disabled={isSubmitting} />
</div>
<div className="flex flex-col gap-1.5">
<label className="text-sm font-medium">Acquisition Date <span className="text-destructive">*</span></label>
<Input
type="date"
value={acquisitionDate}
onChange={(e) => setAcquisitionDate(e.target.value)}
disabled={isSubmitting}
/>
</div>
</div>

<div className="grid grid-cols-2 gap-4">
<div className="flex flex-col gap-1.5">
<label className="text-sm font-medium">Cost <span className="text-destructive">*</span></label>
<Input
type="number"
min="0"
step="0.01"
value={cost}
onChange={(e) => setCost(e.target.value)}
disabled={isSubmitting}
/>
</div>
<div className="flex flex-col gap-1.5">
<label className="text-sm font-medium">Current Value</label>
<Input
type="number"
min="0"
step="0.01"
value={currentValue}
onChange={(e) => setCurrentValue(e.target.value)}
disabled={isSubmitting}
/>
</div>
</div>

{kind === 'fixed_asset' ? (
<div className="grid grid-cols-2 gap-4">
<div className="flex flex-col gap-1.5">
<label className="text-sm font-medium">Serial Number</label>
<Input value={serialNumber} onChange={(e) => setSerialNumber(e.target.value)} disabled={isSubmitting} />
</div>
<div className="flex flex-col gap-1.5">
<label className="text-sm font-medium">Location</label>
<Input value={location} onChange={(e) => setLocation(e.target.value)} disabled={isSubmitting} />
</div>
<div className="flex flex-col gap-1.5 col-span-2">
<label className="text-sm font-medium">Assigned To</label>
<Input value={assignedTo} onChange={(e) => setAssignedTo(e.target.value)} disabled={isSubmitting} />
</div>
</div>
) : (
<div className="grid grid-cols-2 gap-4">
<div className="flex flex-col gap-1.5">
<label className="text-sm font-medium">Quantity <span className="text-destructive">*</span></label>
<Input
type="number"
min="0"
step="any"
value={quantity}
onChange={(e) => setQuantity(e.target.value)}
disabled={isSubmitting}
/>
</div>
<div className="flex flex-col gap-1.5">
<label className="text-sm font-medium">Unit Type <span className="text-destructive">*</span></label>
<Input value={unitType} onChange={(e) => setUnitType(e.target.value)} disabled={isSubmitting} />
</div>
</div>
)}

<div className="flex flex-col gap-1.5">
<label className="text-sm font-medium">Notes</label>
<Textarea
value={notes}
onChange={(e) => setNotes(e.target.value)}
rows={3}
disabled={isSubmitting}
className="min-h-[80px]"
/>
</div>
Comment on lines +126 to +233

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.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Associate each label with its input.

Every <label> in this form lacks htmlFor, and every Input lacks an id. Screen readers announce these controls without a name. A click on the label does not move focus to the control.

add-asset-dialog.tsx already pairs FieldLabel htmlFor with an input id. Use the same pattern here.

♿ Proposed fix, applied to the Name field
-        <label className="text-sm font-medium">
+        <label htmlFor="asset-edit-name" className="text-sm font-medium">
           Name <span className="text-destructive">*</span>
         </label>
-        <Input value={name} onChange={(e) => setName(e.target.value)} disabled={isSubmitting} />
+        <Input id="asset-edit-name" value={name} onChange={(e) => setName(e.target.value)} disabled={isSubmitting} />

Repeat for Category, Acquisition Date, Cost, Current Value, Serial Number, Location, Assigned To, Quantity, Unit Type, and Notes. Add aria-label to the SelectTrigger on line 131.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/admin/src/app/`(admin)/assets/[id]/asset-edit-client.tsx around lines
126 - 233, Associate every form label in the asset edit form with its control by
adding matching htmlFor and id attributes, following the pattern used in
add-asset-dialog.tsx. Update the Name, Category, Acquisition Date, Cost, Current
Value, Serial Number, Location, Assigned To, Quantity, Unit Type, and Notes
controls, and add an aria-label to the SelectTrigger for Type.


<Separator className="my-2" />

{error && <p className="text-sm text-destructive">{error}</p>}

<div className="fixed md:relative bottom-0 left-0 right-0 p-4 md:p-0 bg-card/95 md:bg-transparent backdrop-blur-md md:backdrop-blur-none border-t md:border-none z-50 flex gap-2 pb-[max(env(safe-area-inset-bottom),16px)] md:pb-0 shadow-[0_-4px_12px_rgba(0,0,0,0.05)] md:shadow-none dark:shadow-[0_-4px_12px_rgba(0,0,0,0.2)]">
<Button className="flex-1 bg-blue-600 hover:bg-blue-700 text-white" onClick={handleSave} disabled={isSubmitting}>
{isSubmitting ? 'Saving…' : 'Save Changes'}
</Button>
{asset.status === 'active' ? (
<Button
variant="outline"
onClick={handleDispose}
disabled={isSubmitting}
title="Mark as disposed"
>
<Archive className="size-4" />
</Button>
) : (
<Button
variant="outline"
onClick={handleReactivate}
disabled={isSubmitting}
title="Restore this asset"
>
<ArchiveRestore className="size-4" />
</Button>
)}
<Button
variant="outline"
className="text-destructive hover:bg-destructive/10"
onClick={handleDelete}
disabled={isSubmitting}
title="Delete asset"
>
<Trash2 className="size-4" />
</Button>
</div>
</div>
);
}
Loading
Loading