Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new “Projects” area (list + new project form) and refactors API/environment wiring to support configurable backend URLs, along with profile updates for EcoTaxa account linking.
Changes:
- Introduces Projects list page, New Project page, and supporting hooks/components/types.
- Refactors frontend API base URL handling (proxy +
API_BASE_URL) and improves error typing for register. - Enhances Profile page EcoTaxa linking UX (list, link form extraction, unlink flow), plus CI/Docker workflow additions.
Reviewed changes
Copilot reviewed 37 out of 40 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| vite.config.ts | Switches to env-based proxy target and keeps Vitest config in Vite config. |
| src/vite-env.d.ts | Adds Vite client type references. |
| src/shared/types/apiError.ts | Adds a typed API error response shape. |
| src/shared/components/TopBar.tsx | Uses API_BASE_URL for logout and adds Register CTA. |
| src/shared/api/http.ts | Prefixes requests with API_BASE_URL and adjusts refresh/retry fetch URLs. |
| src/features/userProfile/pages/ProfilePage.tsx | Adds EcoTaxa accounts tab, list/unlink flow, and refactors linking form out. |
| src/features/userProfile/components/EcoTaxaLoginForm.tsx | New extracted EcoTaxa link/login form component. |
| src/features/userProfile/api/profile.api.ts | Adds EcoTaxa linked-accounts APIs/types and link/unlink endpoints. |
| src/features/projects/types/newProject.types.ts | Defines typed form state for new project flow. |
| src/features/projects/pages/ProjectsPage.tsx | New projects listing UI using MUI DataGrid with search/filter/selection. |
| src/features/projects/pages/NewProjectPage.tsx | New project creation UI assembling multiple form sections. |
| src/features/projects/hooks/useProjectsTable.ts | New hook to drive server-side pagination/search and row selection. |
| src/features/projects/hooks/useNewProjectForm.ts | New hook for new-project form state, user loading, and payload mapping. |
| src/features/projects/components/RootFolderSection.tsx | New root-folder picker/entry component with metadata load trigger. |
| src/features/projects/components/ProjectPeopleSection.tsx | New people section with validation UX. |
| src/features/projects/components/ProjectMetadataSection.tsx | New metadata section (incl. ship chips + switches). |
| src/features/projects/components/PrivilegesSection.tsx | New privileges editor (add/remove rows, exclusive contact selection). |
| src/features/projects/components/InstrumentMetadataSection.tsx | New instrument selection section. |
| src/features/projects/components/ImportSettingsSection.tsx | New import settings section. |
| src/features/projects/components/EcoTaxaLinkSection.tsx | New EcoTaxa linking section (loads linked accounts, selects instance/account/project). |
| src/features/projects/components/DataServerSection.tsx | New data-server section shown only for “remote” projects. |
| src/features/projects/components/DataPrivacySection.tsx | New data privacy delay timeline/inputs. |
| src/features/projects/api/projects.api.ts | Adds projects search and create endpoints + request models. |
| src/features/auth/store/auth.store.ts | Adjusts auth loading defaults and adds a setter for loading state. |
| src/features/auth/api/users.api.ts | Adds API to fetch active users for privileges selection. |
| src/features/auth/api/register.api.ts | Uses API_BASE_URL and typed error extraction for register failures. |
| src/features/auth/api/passwordReset.api.ts | Uses API_BASE_URL for password reset endpoints. |
| src/features/auth/api/auth.api.ts | Uses API_BASE_URL for login request. |
| src/config/api.ts | Introduces centralized API_BASE_URL selection/normalization + prod fail-fast. |
| src/app/router.tsx | Adds protected routes for /projects and /new-project. |
| package.json | Adds coverage script and new deps; updates MUI icons; adjusts scripts. |
| package-lock.json | Locks new deps and version bumps. |
| mt.env | Adds env template for backend URL configuration. |
| docker-compose.yml | Adds a basic container runtime definition for the frontend image. |
| README.md | Adds CI and Codecov badges. |
| Dockerfile | Adds multi-stage build producing a static bundle served by serve. |
| .gitignore | Adds .env to ignored files. |
| .github/workflows/pr-check.yml | Adds PR checks (lint/test/build) on PRs and main pushes. |
| .github/workflows/ci.yml | Adds tag-based CI/CD: tests+coverage+Codecov and Docker build/push. |
| .env | Adds a local env file. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| server: { | ||
| proxy: { | ||
| "/auth": { | ||
| target: env.VITE_BACKEND_URL, | ||
| changeOrigin: true, | ||
| }, | ||
| "/users": { | ||
| target: env.VITE_BACKEND_URL, | ||
| changeOrigin: true, | ||
| }, |
There was a problem hiding this comment.
Dev API calls introduced in this PR hit /projects/... via the shared http() helper, but the Vite dev proxy is only configured for /auth and /users. Unless the backend is served from the same origin, /projects requests will fail in dev; add a proxy entry for /projects (and any other backend routes used by the frontend) or proxy a common prefix such as /api.
| "/auth": { | ||
| target: env.VITE_BACKEND_URL, | ||
| changeOrigin: true, | ||
| }, |
There was a problem hiding this comment.
env.VITE_BACKEND_URL can be empty/undefined (e.g., when .env isn’t present), which would produce an invalid proxy target and confusing dev behavior. Consider providing a default (like http://localhost:4000) or failing fast with a clear error when the env var is missing.
| "preview": "vite preview", | ||
| "test": "vitest", | ||
| "test:ui": "vitest --ui", | ||
| "test:watch": "vitest --watch" | ||
| "test:watch": "vitest --watch", | ||
| "test:coverage": "vitest run --coverage" | ||
| }, |
There was a problem hiding this comment.
npm run build runs tsc -b, but typescript is no longer listed in devDependencies. This can make builds fail (or become non-deterministic via peer-dep auto-installs); add typescript back to devDependencies to ensure tsc is always available in CI and for developers.
|
|
||
| if (typeof first === "string") return first; | ||
| if (typeof first?.msg === "string") return first.msg; | ||
| if (typeof first === "object" && first?.msg === "string") return first.msg; |
There was a problem hiding this comment.
This type check is incorrect: first?.msg === "string" compares the value to the literal string "string". It should check the type (e.g., typeof first.msg === "string") before returning first.msg, otherwise backend validation errors won’t be surfaced correctly.
| if (typeof first === "object" && first?.msg === "string") return first.msg; | |
| if (typeof first === "object" && typeof first?.msg === "string") return first.msg; |
| // Privileges (Transforming our simple array into the array of objects the backend needs) | ||
| // Ex: [{ user_id: 1 }] | ||
| contact: { user_id: 1 }, // TODO: Remplacer par le vrai user_id sélectionné dans l'UI | ||
| members: values.privileges.filter(p => p.role === 'Member').map(p => ({ user_id: parseInt(p.userId) })), | ||
| managers: values.privileges.filter(p => p.role === 'Manager').map(p => ({ user_id: parseInt(p.userId) })), |
There was a problem hiding this comment.
The project creation payload currently hardcodes contact: { user_id: 1 }, ignoring the contact boolean on values.privileges. This will create incorrect privileges for most users; derive contact from the selected privileges (and validate exactly one contact is chosen) instead of hardcoding an ID.
| console.log("Mapped Payload ready for API:", payload); | ||
|
|
||
| // 2. APPEL API | ||
| // const response = await createProject(payload); | ||
| // alert("Projet créé avec succès !"); |
There was a problem hiding this comment.
handleSubmit currently only logs the mapped payload; the actual createProject(payload) call is commented out, so the New Project flow can’t succeed. If this page is meant to be functional in this PR, wire up the API call and surface success/error feedback in the UI.
| @@ -0,0 +1,7 @@ | |||
| services: | |||
| web: | |||
| image: 'ecotaxa/ecopart_front:latest' | |||
There was a problem hiding this comment.
env_file: .env won’t affect import.meta.env values in a Vite-built static bundle at container runtime (those are injected at build time). If the intent is to configure the backend URL at deploy time, you’ll need a runtime config mechanism (or build the image with the correct VITE_BACKEND_URL) and document that here.
| image: 'ecotaxa/ecopart_front:latest' | |
| image: 'ecotaxa/ecopart_front:latest' | |
| # Note: This env_file is for container runtime environment variables only. | |
| # Vite's import.meta.env values (e.g. VITE_BACKEND_URL) are baked into the | |
| # static bundle at build time and are NOT affected by this .env file. | |
| # To change such values, rebuild the ecotaxa/ecopart_front image with the | |
| # desired Vite env vars (or use a dedicated runtime config mechanism). |
| useEffect(() => { | ||
| const fetchAccounts = async () => { | ||
| if (!user) return; | ||
| setLoadingAccounts(true); | ||
| try { |
There was a problem hiding this comment.
Including onChange in this effect’s dependency array is likely to trigger repeated refetches, because the parent passes an inline (data) => updateField(...) function that changes identity on every render. Consider making onChange stable (useCallback in parent) or restructuring the effect to depend only on stable values (e.g., user?.user_id).
| <TextField | ||
| select={!values.createNewProject} // If new project, it becomes a disabled text field showing the new name | ||
| fullWidth | ||
| label="EcoTaxa project*" | ||
| value={values.createNewProject ? "new" : values.project} | ||
| onChange={(e) => onChange({ project: e.target.value })} | ||
| size="small" | ||
| disabled={values.createNewProject || !values.account} | ||
| > | ||
| {values.createNewProject ? ( | ||
| // Show a fake disabled option to mimic the mockup "uvp5_sn000_tara (new)" | ||
| <MenuItem value="new">{projectTitle || "New project"} (new)</MenuItem> | ||
| ) : ( | ||
| // TODO: This should be populated by an API call fetching projects for the selected account | ||
| // For now, we mock an existing project for visual completeness | ||
| [ | ||
| <MenuItem key="1" value="existing_1">existing_project_1</MenuItem>, | ||
| <MenuItem key="2" value="existing_2">existing_project_2</MenuItem> | ||
| ] | ||
| )} | ||
| </TextField> |
There was a problem hiding this comment.
When createNewProject is true, this TextField is rendered with select={false} but still receives <MenuItem> children. MUI TextField expects MenuItem children only when select is enabled; consider rendering a separate plain disabled TextField (no children) for the “new project” case.
| <TextField | |
| select={!values.createNewProject} // If new project, it becomes a disabled text field showing the new name | |
| fullWidth | |
| label="EcoTaxa project*" | |
| value={values.createNewProject ? "new" : values.project} | |
| onChange={(e) => onChange({ project: e.target.value })} | |
| size="small" | |
| disabled={values.createNewProject || !values.account} | |
| > | |
| {values.createNewProject ? ( | |
| // Show a fake disabled option to mimic the mockup "uvp5_sn000_tara (new)" | |
| <MenuItem value="new">{projectTitle || "New project"} (new)</MenuItem> | |
| ) : ( | |
| // TODO: This should be populated by an API call fetching projects for the selected account | |
| // For now, we mock an existing project for visual completeness | |
| [ | |
| <MenuItem key="1" value="existing_1">existing_project_1</MenuItem>, | |
| <MenuItem key="2" value="existing_2">existing_project_2</MenuItem> | |
| ] | |
| )} | |
| </TextField> | |
| {values.createNewProject ? ( | |
| // When creating a new project, show a plain disabled text field with the new project name | |
| <TextField | |
| fullWidth | |
| label="EcoTaxa project*" | |
| value={`${projectTitle || "New project"} (new)`} | |
| size="small" | |
| disabled | |
| /> | |
| ) : ( | |
| <TextField | |
| select | |
| fullWidth | |
| label="EcoTaxa project*" | |
| value={values.project} | |
| onChange={(e) => onChange({ project: e.target.value })} | |
| size="small" | |
| disabled={!values.account} | |
| > | |
| {/* TODO: This should be populated by an API call fetching projects for the selected account */} | |
| {/* For now, we mock an existing project for visual completeness */} | |
| <MenuItem key="1" value="existing_1">existing_project_1</MenuItem> | |
| <MenuItem key="2" value="existing_2">existing_project_2</MenuItem> | |
| </TextField> | |
| )} |
| import { defineConfig, loadEnv } from 'vite'; | ||
| import react from "@vitejs/plugin-react"; | ||
| import path from "path"; |
There was a problem hiding this comment.
Because tsconfig.node.json type-checks vite.config.ts in strict mode, importing defineConfig from vite may cause a TS error for the test: block (Vite’s UserConfig doesn’t include Vitest options by default). Prefer defineConfig from vitest/config (and import loadEnv from vite) so the test config remains typed and tsc -b doesn’t fail.
No description provided.