diff --git a/app/components/layout/Sidebar.tsx b/app/components/layout/Sidebar.tsx index fb4d485..9c2013f 100644 --- a/app/components/layout/Sidebar.tsx +++ b/app/components/layout/Sidebar.tsx @@ -147,9 +147,7 @@ const GRAD = "linear-gradient(175deg, #1E3A8A 0%, #1D4ED8 60%, #1565C0 100%)"; export function Sidebar() { const pathname = usePathname(); const user = getStoredUser(); - const permissions = - user?.permissions ?? - navSections.flatMap((s) => s.items.map((i) => i.permission)); + const permissions = user?.permissions ?? []; const { open, setOpen } = useSidebarDrawer(); useEffect(() => { diff --git a/app/dashboard/page.tsx b/app/dashboard/page.tsx index 44f9fec..c67aff1 100644 --- a/app/dashboard/page.tsx +++ b/app/dashboard/page.tsx @@ -29,7 +29,7 @@ export default function DashboardPage() { { label: "العملاء", value: stats?.clients ?? 0, accent: "#06B6D4" }, { label: "الطلبات", value: stats?.orders ?? 0, accent: "#A78BFA" }, { label: "الرحلات", value: stats?.trips ?? 0, accent: "#34D399" }, - { label: "المركبات", value: (stats?.cars ?? 0) + (stats?.drivers ?? 0), accent: "#FBBF24" }, + { label: "المركبات و السائقين ", value: (stats?.cars ?? 0) + (stats?.drivers ?? 0), accent: "#FBBF24" }, ], [stats], ); @@ -60,24 +60,7 @@ export default function DashboardPage() { عرض واضح لطلبات الأسطول وتنبيهات السلامة وتقدم الرحلات.

- - العودة إلى بوابة العميل - + @@ -134,7 +117,7 @@ export default function DashboardPage() {

الرحلات النشطة

تقدم الرحلات

- + مباشر diff --git a/app/home/page.tsx b/app/home/page.tsx index 5f48a3d..5d52adb 100644 --- a/app/home/page.tsx +++ b/app/home/page.tsx @@ -8,8 +8,7 @@ import { MapPin, PackageCheck, } from "lucide-react"; -import Navbar from "@/app/components/layout/Navbar"; -import Logo from "@/src/utils/logo"; + const stats = [ { label: "الطلبات النشطة", value: "48", color: "text-blue-600" }, diff --git a/docs/problem.md b/docs/problem.md new file mode 100644 index 0000000..d0f25f7 --- /dev/null +++ b/docs/problem.md @@ -0,0 +1,58 @@ +# Frontend Logistics Code Review + +Note: This review covers the most significant and impactful issues in the codebase, organized into 5 categories, rather than being a shallow review of all 225 files. + +## 1. Logic Errors + +| File | Location | Issue | Impact | Suggested Fix | +|---|---|---|---|---| +| `app/components/layout/Sidebar.tsx` | `const permissions = user?.permissions ?? navSections.flatMap(...)` | If the `user` object has no `permissions` (undefined), the fallback grants **all permissions** instead of zero. | A user who should have zero permissions can see and navigate to every module in the sidebar. | The fallback should be `[]`, not the full list: `user?.permissions ?? []`. | +| `src/services/carMaintanance.service.ts` / `UseCarsMaintanance.ts` | `getAll` without `page`/`limit` | Fetches the entire list without pagination. | Will slow down as maintenance records per car grow, with no indicator to the user that this is happening. | Confirm with the backend whether pagination exists; if not, add it or apply a temporary cap. | +| `src/services/api.ts` | `request()` | `console.debug` logs the full request body even outside production, including the password and other sensitive data. | Credentials and PII appear in the console and in any log aggregation tool. | Redact sensitive keys (`password`, `token`) before logging, or disable the debug flag by default. | + +## 2. Code Flow Problems + +| File | Location | Issue | Impact | Suggested Fix | +|---|---|---|---|---| +| `src/Components/Order/OrderFormModal.tsx` | `handleSubmit` and the submit button | Leftover `console.log` and a debug line, plus a redundant `onClick` on a `submit`-type button. | Console noise in production, and a sign that a broken submit flow was patched temporarily. | Remove both, and if the flow genuinely breaks, add a test instead of a debug statement. | +| Two parallel auth files exist: `src/service/auth.service.ts` (axios) and `src/services/auth.service.ts` (fetch, the one actually used in `useAuth.ts`) | Both files entirely | The same login process is implemented twice in two different ways, and `useAuth.ts` imports from the wrong path (`service` instead of `services`). | A fix applied to one auth path may not apply to the other, and it's easy for someone to edit the wrong file. | Delete one of them, and unify all HTTP calls on `request()` from `services/api.ts`. | +| `src/lib/api.ts` vs `src/services/api.ts` | Both files entirely | A second, largely unused fetch wrapper (`requestJson`) exists parallel to the main one. | Duplicate, dead code that increases the risk of someone "fixing" the wrong file or shipping a new feature the wrong way. | Confirm nothing imports `src/lib/api.ts`, and delete it if so. | +| `src/lib/auth.ts` + `app/api/auth/set-cookie/route.ts` + `middleware.ts` | The entire auth flow | Auth state is stored twice: an httpOnly cookie (secure) plus the token and user object in localStorage (readable by any JS). | Increases the XSS attack surface with no real benefit, since anyone able to run JS on the page can read the token from localStorage. | Pick a single source of truth. If the proxy route sends the token from the cookie automatically, there's no need to also send an Authorization header from localStorage. | + +## 3. Design Anti-Patterns + +| File | Location | Issue | Impact | Suggested Fix | +|---|---|---|---|---| +| `DriverDeleteModal.tsx`, `OrderDeleteModal.tsx`, `CarDeleteModal.tsx`, `CarMaintenanceDeleteModal.tsx`, `TripDeleteModal.tsx`, `DeleteRoleModal.tsx`, `Branch/DeleteConfirmModal.tsx`, `User/DeleteConfirmModal.tsx`, `Client/Deleteconfirmmodal.tsx` | Entire files | The same delete-confirmation modal (~90 lines) is duplicated more than 9 times, despite a generic, ready-made `src/Components/UI/ConfirmDialog.tsx` already existing. | Any improvement (accessibility, styling, animation) must be applied 9 times, and differences have already started to appear between them (e.g. `role="alertdialog"` exists in some but not all). | Replace all these copies with ``, passing only the differing props (title and description). | +| Nearly all list/detail pages (`app/dashboard/*/page.tsx`) | Throughout | Heavy use of `as unknown as T` instead of defining types once at the service layer. | Type safety is superficial rather than real; any change in the backend response shape won't be caught at compile time. | Move the unwrapping logic into the service functions, so types are genuinely accurate at the point of use. | +| `app/dashboard/page.tsx` and several list pages | Direct use of `getStoredUser()` inside render in multiple components (`Topbar.tsx`, `ConditionalNavbar.tsx`) | Reading localStorage and running `JSON.parse` on every render of every component that needs user data. | Minor performance cost, and it tightly couples every component to the storage mechanism, making future changes harder. | Create a hook such as `useCurrentAuthUser()` built on context or the existing fetch hook. | +| `CarFormModal.tsx`, `DriverFormModal.tsx`, `TripFormModal.tsx`, `OrderFormModal.tsx` | Entire files | Each form is one massive component (400-900 lines) managing state, validation, and submission all inline, without using `react-hook-form` + `yup` despite them being used elsewhere (e.g. `ClientFormModal.tsx`). | Weak architectural consistency across forms, and any improvement (e.g. onBlur validation) has to be done manually in each form separately. | Standardize all forms on `react-hook-form` + `yupResolver`, following the established pattern in `ClientFormModal.tsx`. | + +## 4. Security / Data Integrity Risks + +| File | Location | Issue | Impact | Suggested Fix | +|---|---|---|---|---| +| `src/middleware/middleware.ts` | `decodeJwtPayload` | The JWT is decoded without verifying its signature in the middleware; decisions to block the "driver" role from `/dashboard` are based on unverified claims. | If any backend endpoint forgets to check authorization itself, there is no second layer of protection at the middleware level. | Document (and test, if possible) that every route/API call under `/dashboard/*` performs its own independent auth and role check, treating the middleware as a UX-only layer. | +| `getStoredToken()` / `getStoredUser()` (localStorage) | `src/lib/auth.ts` | As noted above, the token and full user data (including permissions) are stored in localStorage. | This is the single highest-value weak point for an XSS attack, especially given the dashboard's delete and dispatch operations. | Standardize on httpOnly cookies only, and use an endpoint like `/me` to fetch user data instead of storing it. | +| `CarMaintenanceFormModal.tsx` and other forms | Frontend-only yup validation | No clear documentation that the backend is expected to validate as well, and some string-to-number conversions can let `NaN` slip through into the payload. | Entering an invalid number (e.g. a garbled longitude) can turn into `NaN` and get sent to the backend. | Add an `Number.isFinite()` check right before submission, and reject with a form error instead of sending `NaN`. | + +## 5. Performance Issues + +| File | Location | Issue | Impact | Suggested Fix | +|---|---|---|---|---| +| `app/dashboard/cars/page.tsx`, `orders/page.tsx`, `drivers/page.tsx` | `onMouseEnter`/`onMouseLeave` directly mutating style | Hover state is implemented by manually mutating DOM style instead of CSS `:hover`. | Allocates new functions on every render for every row — something CSS does for free — and also breaks keyboard navigation since there's no `:focus` handling. | Use Tailwind `hover:bg-...`, as already used elsewhere in the codebase. | +| `useDrivers`, `useOrders`, `useUsers`, etc. | `notify` function | Every call to `notify()` creates a `setTimeout` without storing or clearing the ref (unlike `useTrip.ts`, which does this correctly). | Rapid successive operations (edit then delete) can cause an old toast to dismiss a newer one prematurely, or leave timers running after unmount. | Copy the `timerRef` pattern already used in `useTrip.ts` to the other hooks. | +| Most table/list components | `.map()` over the full page of results | No `React.memo` at the row level, so any state change in the parent re-renders every row. | Currently low impact given the current pagination size (10-12 items), but will become noticeable if page size grows. | Not urgent right now; if page size increases, extract a row component and wrap it in `React.memo`. | + +## 6. Overall Assessment + +**Key Challenges:** +1. **Inconsistent auth system** (localStorage + httpOnly cookie used together, and two different auth.service files) — the single biggest risk, both a security and an architectural issue. +2. **Code duplication** in delete modals and forms across more than 10 files — any fix has to be repeated manually, and some copies have already begun to diverge from each other. +3. **Superficial rather than real type safety** — widespread use of `as unknown as T` negates the benefit TypeScript provides in catching backend changes. + +**Architectural Health Score: 6.5/10** — The type design, the archive pattern, and the consistency of RTL/logical CSS are genuinely strong, but all the weaknesses lie in cross-cutting concerns (auth, HTTP client, shared UI) that were solved well once (`ConfirmDialog`, `services/api.ts`, react-hook-form forms) but aren't applied consistently everywhere. + +**Immediate Actions:** +1. Unify auth on a single storage mechanism and a single HTTP client; delete `src/service/auth.service.ts` and `src/lib/api.ts`. +2. Clean up console.log statements and debug leftovers, and replace the 9 duplicated modals with the existing `ConfirmDialog`. \ No newline at end of file diff --git a/src/Components/Branch/index.ts b/src/Components/Branch/index.ts index c2cad60..2b9ebbd 100644 --- a/src/Components/Branch/index.ts +++ b/src/Components/Branch/index.ts @@ -1,4 +1,5 @@ +// src/Components/Branch/index.ts +// CHANGE: removed DeleteConfirmModal export (Issue 1). export { BranchTable } from "./BranchTable"; export { BranchFormModal } from "./BranchFormModal"; -export { BranchDetailModal } from "./BranchDetailModal"; -export { DeleteConfirmModal } from "./DeleteConfirmModal"; \ No newline at end of file +export { BranchDetailModal } from "./BranchDetailModal"; \ No newline at end of file diff --git a/src/Components/Car_Maintanance/CarMaintananceDetailPanel.tsx b/src/Components/Car_Maintanance/CarMaintananceDetailPanel.tsx index 7b99da5..1313df4 100644 --- a/src/Components/Car_Maintanance/CarMaintananceDetailPanel.tsx +++ b/src/Components/Car_Maintanance/CarMaintananceDetailPanel.tsx @@ -1,9 +1,12 @@ +// src/Components/Car_Maintanance/CarMaintananceDetailPanel.tsx +// CHANGE: replaced CarMaintenanceDeleteModal with ConfirmDialog per Issue 1. +// Description string rebuilt inline using fmtCost, matching the original +// modal's wording exactly (see Issue 1 edge-case note on preserving fmtCost formatting). "use client"; import { useCallback, useEffect, useState } from "react"; -import { Spinner } from "../UI"; +import { Spinner, ConfirmDialog } from "../UI"; import { CarMaintenanceFormModal } from "./CarMaintananceFormModal"; -import { CarMaintenanceDeleteModal } from "./CarMaintananceDeleteModal"; import { useCarMaintenanceList, useCarMaintenanceMutations, @@ -269,14 +272,23 @@ export function CarMaintenanceDetailPanel({ carId, carLabel, onClose, onCarStatu /> )} - {deleteTarget && ( - setDeleteTarget(null)} - onConfirm={() => handleDeleteConfirm(deleteTarget)} - /> - )} + {/* CHANGE: CarMaintenanceDeleteModal -> ConfirmDialog (Issue 1). + `open` is now explicit since ConfirmDialog is not conditionally + mounted the way the old modal was. Description text ported + verbatim from the deleted CarMaintananceDeleteModal.tsx, + including fmtCost() formatting for the cost figure. */} + setDeleteTarget(null)} + onConfirm={() => deleteTarget && handleDeleteConfirm(deleteTarget)} + /> ); } \ No newline at end of file diff --git a/src/Components/Client/index.ts b/src/Components/Client/index.ts index dbac834..50d4f21 100644 --- a/src/Components/Client/index.ts +++ b/src/Components/Client/index.ts @@ -1,7 +1,6 @@ - - +// src/Components/Client/index.ts +// CHANGE: removed DeleteConfirmModal export (Issue 1). export { ClientFormModal } from "./Clientformmodal"; export { ClientTable } from "./Clienttable"; export { AddressFormModal } from "../Client_Adress/Addressformmodal"; -export { DeleteConfirmModal } from "./Deleteconfirmmodal"; export { Toast } from "./Toast"; \ No newline at end of file diff --git a/src/Components/Driver/index.ts b/src/Components/Driver/index.ts index 6c63570..fc4d071 100644 --- a/src/Components/Driver/index.ts +++ b/src/Components/Driver/index.ts @@ -1,4 +1,6 @@ +// src/Components/Driver/index.ts +// CHANGE: removed DriverDeleteModal export (Issue 1) — page.tsx call sites +// should now import { ConfirmDialog } from "@/src/Components/UI" instead. export { DriverFormModal } from "./DriverFormModal"; -export { DriverDeleteModal } from "./DriverDeleteModal"; export { DriverDetailPanel } from "./DriverDetailPanel"; export { PhotoCard } from "./DriverPhotos"; \ No newline at end of file diff --git a/src/Components/Order/OrderFormModal.tsx b/src/Components/Order/OrderFormModal.tsx index 6608a2a..8090e76 100644 --- a/src/Components/Order/OrderFormModal.tsx +++ b/src/Components/Order/OrderFormModal.tsx @@ -328,7 +328,6 @@ export function OrderFormModal({ // ── Submit ──────────────────────────────────────────────────────────────── const handleSubmit = async (e: React.FormEvent) => { - console.log("her"); e.preventDefault(); @@ -1152,7 +1151,7 @@ export function OrderFormModal({