Ip 91 widok dla wniosków projektów i wykres zaangażowania#43
Hidden character warning
Conversation
There was a problem hiding this comment.
Pull request overview
PR dodaje widok dla managera liniowego do przeglądania i obsługi wniosków o przydział pracowników do projektów oraz podgląd obciążenia (wykres), wraz z dopięciem potrzebnych endpointów i DTO po stronie backendu.
Changes:
- Zastąpienie placeholdera
RequestsPagenową stronąEmployeeAssignmentsPagez tabelą i modalem szczegółów. - Dodanie feature
employee-assignments(hooki, serwis API, kolumny tabeli, szczegóły + wykres obciążenia). - Aktualizacja backendu: nowe/zmienione endpointy
/api/employee/assignments, DTO i dopięcieidw odpowiedzi.
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/routes/AppRoutes.tsx | Podpięcie nowej strony pod trasę LM (/requests). |
| frontend/src/pages/RequestsPage.tsx | Usunięcie placeholdera strony. |
| frontend/src/pages/EmployeeAssignmentsPage.tsx | Nowa strona z tabelą wniosków i modalem szczegółów. |
| frontend/src/features/employee-assignments/employee-assignments.utils.ts | Utility do przygotowania danych do wykresu (stack). |
| frontend/src/features/employee-assignments/employee-assignments.types.ts | Typy dla listy i szczegółów wniosków + interwały wykresu. |
| frontend/src/features/employee-assignments/employee-assignments.service.ts | Warstwa wywołań API dla assignments. |
| frontend/src/features/employee-assignments/employee-assignments.hooks.ts | React Query: list/details + akcje accept/reject. |
| frontend/src/features/employee-assignments/employee-assignments.columns.tsx | Definicja kolumn tabeli + akcje. |
| frontend/src/features/employee-assignments/components/EmployeeWorkloadChart.tsx | Wykres obciążenia (Recharts). |
| frontend/src/features/employee-assignments/components/EmployeeAssignmentDetails.tsx | Widok szczegółów + akcja akceptacji. |
| frontend/src/components/ui/table.tsx | Bazowe komponenty tabeli. |
| frontend/src/components/ui/data-table.tsx | Generyczna tabela oparta o TanStack React Table. |
| frontend/src/components/ui/chart.tsx | Kontener/utility dla wykresów (config/tooltip/legend). |
| frontend/src/api/index.ts | Nowe query keys dla feature (React Query). |
| frontend/src/api/endpoints.ts | Dodanie endpointów EMPLOYEE ASSIGNMENTS. |
| frontend/package.json | Dodanie zależności: react-table, recharts. |
| frontend/package-lock.json | Aktualizacja lockfile pod nowe zależności. |
| backend/.../EmployeeAssignmentsServiceTest.java | Aktualizacja asercji pod zmianę pola DTO. |
| backend/.../application.properties | Zmiany w konfiguracji DB/JPA. |
| backend/.../EmployeeAssignmentsService.java | Dopięcie id w EmployeeAssignmentResponse. |
| backend/.../EmployeeAssignmentResponse.java | Dodanie UUID id do odpowiedzi. |
| backend/.../EmployeeAssignmentDetailsResponse.java | Zmiana nazwy pola na requestedWorkload. |
| backend/.../EmployeeAssignmentsController.java | Zmiana base path na /api/employee/assignments i SpEL w @PreAuthorize. |
Files not reviewed (1)
- frontend/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export const employeeRequestsKeys = createQueryKeys('employee-requests', { | ||
| detail: (requestId: string) => [requestId], | ||
| list: null | ||
| }) | ||
|
|
||
| export const queryKeys = mergeQueryKeys(employeeRequestsKeys) No newline at end of file |
There was a problem hiding this comment.
employeeRequestsKeys/namespace 'employee-requests' jest niespójne z nowym API i nazewnictwem („assignments”). To potem przenika do queryKey i utrudnia debugowanie cache. Proponuję zmienić na employeeAssignmentsKeys oraz namespace np. 'employee-assignments'.
| export const employeeRequestsKeys = createQueryKeys('employee-requests', { | |
| detail: (requestId: string) => [requestId], | |
| list: null | |
| }) | |
| export const queryKeys = mergeQueryKeys(employeeRequestsKeys) | |
| export const employeeAssignmentsKeys = createQueryKeys('employee-assignments', { | |
| detail: (requestId: string) => [requestId], | |
| list: null | |
| }) | |
| export const queryKeys = mergeQueryKeys(employeeAssignmentsKeys) |
| <Button | ||
| onClick={handleAccept} | ||
| disabled={isAccepting} | ||
| > | ||
| Zatwierdź | ||
| </Button> |
There was a problem hiding this comment.
Przycisk „Zatwierdź” jest aktywny nawet gdy assignment.status != PENDING, mimo że backend zwróci błąd (validatePendingStatus). Proponuję ukryć/disabled przycisk i ewentualnie pokazać informację, że wniosek jest już rozpatrzony.
| spring.datasource.url=${SPRING_DATASOURCE_URL:jdbc:postgresql://localhost:5432/projectmanager} | ||
| spring.datasource.username=${SPRING_DATASOURCE_USERNAME:postgres} | ||
| spring.datasource.password=${SPRING_DATASOURCE_PASSWORD:change_me} | ||
| spring.datasource.password=${SPRING_DATASOURCE_PASSWORD:postgres} |
There was a problem hiding this comment.
W application.properties zmieniono domyślne hasło DB na „postgres”. To ryzykowne (łatwe do pozostawienia w środowiskach nie-dev) i obniża bezpieczeństwo. Proponuję przywrócić neutralny fallback (np. change_me / pusty) i wymagać ustawienia SPRING_DATASOURCE_PASSWORD w środowisku.
| spring.datasource.password=${SPRING_DATASOURCE_PASSWORD:postgres} | |
| spring.datasource.password=${SPRING_DATASOURCE_PASSWORD:change_me} |
| spring.datasource.password=${SPRING_DATASOURCE_PASSWORD:change_me} | ||
| spring.datasource.password=${SPRING_DATASOURCE_PASSWORD:postgres} | ||
| spring.jpa.hibernate.ddl-auto=${SPRING_JPA_HIBERNATE_DDL_AUTO:update} | ||
| spring.jpa.show-sql=true |
There was a problem hiding this comment.
spring.jpa.show-sql=true włączone w głównym application.properties będzie logować SQL we wszystkich środowiskach (jeśli nie nadpisane), co jest niepożądane operacyjnie i może ujawnić dane w logach. Lepiej przenieść to do profilu dev lub sterować flagą z env (domyślnie false).
| spring.jpa.show-sql=true | |
| spring.jpa.show-sql=${SPRING_JPA_SHOW_SQL:false} |
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| className="rounded-r-none h-8" | ||
| onClick={() => onVerify?.(assignment)} | ||
| > | ||
| Weryfikuj | ||
| </Button> | ||
| <DropdownMenu> | ||
| <DropdownMenuTrigger asChild> | ||
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| className="rounded-l-none h-8 px-2 border-l-0" | ||
| > | ||
| <ChevronDown className="size-4" /> | ||
| </Button> | ||
| </DropdownMenuTrigger> | ||
| <DropdownMenuContent align="end"> | ||
| <DropdownMenuItem | ||
| variant="destructive" | ||
| onClick={() => onReject?.(assignment)} | ||
| > | ||
| Odrzuć | ||
| </DropdownMenuItem> |
There was a problem hiding this comment.
Akcje „Weryfikuj”/„Odrzuć” są dostępne niezależnie od statusu, ale backend odrzuca operacje accept/reject dla innych statusów niż PENDING (EmployeeAssignmentsService.validatePendingStatus). W UI warto ukryć/wyłączyć te akcje dla ACCEPTED/REJECTED (i ewentualnie dodać komunikat błędu), żeby nie generować przewidywalnych błędów 4xx.
|
|
||
| function fmtDate(d: string) { | ||
| try { | ||
| return new Date(d).toLocaleDateString() |
There was a problem hiding this comment.
W większości UI formatowanie dat jest jawnie w locale pl-PL (np. ProjectHeader.tsx). Tutaj toLocaleDateString() używa domyślnego locale przeglądarki, co może dać niespójne formaty. Warto przekazać "pl-PL" dla spójności.
| return new Date(d).toLocaleDateString() | |
| return new Date(d).toLocaleDateString("pl-PL") |
| ) : ( | ||
| <TableRow> | ||
| <TableCell colSpan={columns.length} className="h-24 text-center"> | ||
| No results. |
There was a problem hiding this comment.
Tekst „No results.” jest po angielsku, podczas gdy reszta UI jest po polsku. Dla spójności (i ewentualnego i18n) warto go zlokalizować (np. „Brak wyników”). Przykład polskich komunikatów: frontend/src/providers/AuthProvider.tsx:54.
| No results. | |
| Brak wyników. |
| import type { EmployeeAssignment, EmployeeAssignmentDetails } from "./employee-assignments.types"; | ||
|
|
||
|
|
||
| export const employeeRequestsService = { |
There was a problem hiding this comment.
Nazwa eksportu employeeRequestsService jest niespójna z resztą feature (employee-assignments, endpointy /employee/assignments). Warto zmienić na employeeAssignmentsService, żeby uniknąć mylenia „requests” vs „assignments” w kolejnych PR-ach.
| export const employeeRequestsService = { | |
| export const employeeAssignmentsService = { |
mistermotopl
left a comment
There was a problem hiding this comment.
wygląda dobrze i jakby miało działać
| onClose: () => void | ||
| } | ||
|
|
||
| function fmtDate(d: string) { |
There was a problem hiding this comment.
to możesz przenieść do jakiś utils bo tak dziwnie to tu wyglądaz takich kosmetycznych rzeczy ktoórych nie trzeba teraz robić
Dodałem stronę wniosków o pracownika dla managera liniowego i możliwość podglądania tych wniosków - wyświetlanie wykresu wykorzystania pracownika