Просмотр исходного кода

fix(projects): carry tags, due date and priority in the list payload (#2536)

The edit dialog is shared between the projects list and the project detail
page and seeds itself from whichever project object it is handed. The list
payload never carried tags, due_date or priority, so editing from the list
showed a blank tags field -- and, unreported, submitted the dialog's default
priority over a stored high/urgent one. The component read those fields
through a cast, so the compiler never flagged that they were always absent.

Put them on ProjectListResponse and ProjectListItem, drop the casts, and let
an explicit null clear tags and due date the way it already clears budget and
url -- an emptied field was previously sent as undefined and silently reverted.
The template list was missing target_parts_count, which the same dialog edits.
maziggy 1 месяц назад
Родитель
Сommit
c640ddc1f7

Разница между файлами не показана из-за своего большого размера
+ 0 - 0
CHANGELOG.md


+ 12 - 2
backend/app/api/routes/projects.py

@@ -263,6 +263,9 @@ async def list_projects(
                 target_count=project.target_count,
                 target_parts_count=project.target_parts_count,
                 budget=project.budget,
+                tags=project.tags,
+                due_date=project.due_date,
+                priority=project.priority,
                 created_at=project.created_at,
                 archive_count=archive_count,
                 total_items=total_items,
@@ -370,7 +373,11 @@ async def list_templates(
                 color=project.color,
                 status=project.status,
                 target_count=project.target_count,
+                target_parts_count=project.target_parts_count,
                 budget=project.budget,
+                tags=project.tags,
+                due_date=project.due_date,
+                priority=project.priority,
                 created_at=project.created_at,
                 archive_count=archive_count,
                 queue_count=0,
@@ -585,9 +592,12 @@ async def update_project(
         project.target_parts_count = data.target_parts_count
     if data.notes is not None:
         project.notes = data.notes
-    if data.tags is not None:
+    # Sent-but-null clears the field; omitted leaves it alone. Guarding on
+    # ``is not None`` would make an emptied tags field or a removed due date
+    # silently revert to the stored value (#2536).
+    if "tags" in data.model_fields_set:
         project.tags = data.tags
-    if data.due_date is not None:
+    if "due_date" in data.model_fields_set:
         project.due_date = data.due_date
     if data.priority is not None:
         if data.priority not in ["low", "normal", "high", "urgent"]:

+ 7 - 0
backend/app/schemas/project.py

@@ -151,6 +151,13 @@ class ProjectListResponse(BaseModel):
     target_count: int | None
     target_parts_count: int | None = None
     budget: float | None = None
+    # The edit dialog is shared with the project detail page and seeds its fields
+    # from whichever project object it is handed, so the list payload has to carry
+    # everything the dialog edits — otherwise a save from the list view submits a
+    # blank tags field and a default priority over the stored values (#2536).
+    tags: str | None = None
+    due_date: datetime | None = None
+    priority: str = "normal"
     created_at: datetime
     # Quick stats
     archive_count: int = 0  # Number of print jobs

+ 95 - 0
backend/tests/integration/test_projects_api.py

@@ -1298,3 +1298,98 @@ class TestProjectExportImport:
         assert response.status_code == 200, response.text
         data = response.json()
         assert data["name"] == "nested-ok"
+
+
+class TestProjectListEditableFields:
+    """Tests for #2536 — the project list payload must carry every field the
+    shared edit dialog renders. The dialog is opened from both the project list
+    and the project detail page and seeds itself from whichever project object it
+    is handed, so a field missing from the list payload shows up blank there and
+    is saved back over the stored value."""
+
+    @pytest.fixture
+    async def project_factory(self, db_session):
+        async def _create(**kwargs):
+            from backend.app.models.project import Project
+
+            defaults = {"name": "Editable Fields Project", "color": "#123456"}
+            defaults.update(kwargs)
+            project = Project(**defaults)
+            db_session.add(project)
+            await db_session.commit()
+            await db_session.refresh(project)
+            return project
+
+        return _create
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_list_carries_the_fields_the_edit_dialog_renders(self, async_client: AsyncClient, project_factory):
+        """The list view is where the reporter saw an empty tags field."""
+        from datetime import datetime
+
+        await project_factory(
+            name="Tagged Project",
+            tags="prototype,client-work",
+            due_date=datetime(2026, 8, 1, 12, 0, 0),
+            priority="high",
+            target_parts_count=7,
+        )
+
+        response = await async_client.get("/api/v1/projects/")
+        assert response.status_code == 200
+        item = next(p for p in response.json() if p["name"] == "Tagged Project")
+
+        assert item["tags"] == "prototype,client-work"
+        assert item["due_date"].startswith("2026-08-01")
+        assert item["priority"] == "high"
+        assert item["target_parts_count"] == 7
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_template_list_carries_them_too(self, async_client: AsyncClient, project_factory):
+        """Templates feed the same dialog, so they need the same payload."""
+        await project_factory(
+            name="Tagged Template",
+            is_template=True,
+            tags="reusable",
+            priority="urgent",
+            target_parts_count=3,
+        )
+
+        response = await async_client.get("/api/v1/projects/templates")
+        assert response.status_code == 200
+        item = next(p for p in response.json() if p["name"] == "Tagged Template")
+
+        assert item["tags"] == "reusable"
+        assert item["priority"] == "urgent"
+        assert item["target_parts_count"] == 3
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_priority_survives_an_edit_that_does_not_touch_it(self, async_client: AsyncClient, project_factory):
+        """A save from the list view used to submit the default priority over a
+        stored 'high' — the dialog never received the real one."""
+        project = await project_factory(name="Important", priority="high", tags="keep-me")
+
+        response = await async_client.patch(f"/api/v1/projects/{project.id}", json={"name": "Still Important"})
+        assert response.status_code == 200
+
+        result = response.json()
+        assert result["priority"] == "high"
+        assert result["tags"] == "keep-me"
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_explicit_null_clears_tags_and_due_date(self, async_client: AsyncClient, project_factory):
+        """Emptying the field in the dialog has to actually remove the value."""
+        from datetime import datetime
+
+        project = await project_factory(name="Clearable", tags="obsolete", due_date=datetime(2026, 8, 1, 12, 0, 0))
+
+        response = await async_client.patch(f"/api/v1/projects/{project.id}", json={"tags": None, "due_date": None})
+        assert response.status_code == 200
+
+        result = response.json()
+        assert result["tags"] is None
+        assert result["due_date"] is None

+ 88 - 1
frontend/src/__tests__/pages/ProjectsPage.test.tsx

@@ -2,7 +2,7 @@
  * Tests for the ProjectsPage component.
  */
 
-import { describe, it, expect, beforeEach } from 'vitest';
+import { describe, it, expect, beforeEach, vi } from 'vitest';
 import { screen, waitFor } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import { render } from '../utils';
@@ -379,4 +379,91 @@ describe('ProjectsPage', () => {
       expect(card).not.toBeNull();
     });
   });
+
+  describe('edit dialog seeds itself from the list payload (#2536)', () => {
+    /**
+     * The same ProjectModal is opened from the projects list and from the
+     * project detail page. The detail page hands it a full project; the list
+     * hands it a list item. Reporter saw an empty tags field when editing from
+     * the list, because the list payload didn't carry tags — and, unreported,
+     * the dialog then submitted its default priority over the stored one.
+     */
+    const listItem = {
+      id: 7,
+      name: 'Spool holder',
+      description: null,
+      color: '#00ae42',
+      status: 'active',
+      target_count: null,
+      target_parts_count: null,
+      budget: null,
+      tags: 'prototype,client-work',
+      due_date: '2026-08-01T12:00:00Z',
+      priority: 'high',
+      created_at: '2024-01-01T00:00:00Z',
+      archive_count: 0,
+      total_items: 0,
+      completed_count: 0,
+      failed_count: 0,
+      queue_count: 0,
+      progress_percent: null,
+      archives: [],
+      url: null,
+      cover_image_filename: null,
+    };
+
+    const renderModal = (onSave: (data: unknown) => void) =>
+      render(
+        <ProjectModal
+          project={listItem}
+          onClose={() => {}}
+          onSave={onSave as never}
+          isLoading={false}
+          currencySymbol="€"
+          t={((k: string) => k) as never}
+        />,
+      );
+
+    const tagsInput = () => screen.getByPlaceholderText('projects.tagsPlaceholder') as HTMLInputElement;
+    const dueDateInput = () => document.querySelector('input[type="date"]') as HTMLInputElement;
+    const prioritySelect = () =>
+      Array.from(document.querySelectorAll('select')).find((s) =>
+        s.querySelector('option[value="urgent"]'),
+      ) as HTMLSelectElement;
+
+    it('prefills tags, due date and priority from a list item', () => {
+      renderModal(() => {});
+
+      expect(tagsInput().value).toBe('prototype,client-work');
+      expect(dueDateInput().value).toBe('2026-08-01');
+      expect(prioritySelect().value).toBe('high');
+    });
+
+    it('does not downgrade a stored priority when saving an untouched field', async () => {
+      const user = userEvent.setup();
+      const onSave = vi.fn();
+      renderModal(onSave);
+
+      await user.click(screen.getByRole('button', { name: 'common.save' }));
+
+      // Before the fix the dialog fell back to its 'normal' default and sent
+      // that, silently demoting a high/urgent project edited from the list.
+      expect(onSave).toHaveBeenCalledWith(
+        expect.objectContaining({ priority: 'high', tags: 'prototype,client-work' }),
+      );
+    });
+
+    it('sends null when an existing tag list is cleared', async () => {
+      const user = userEvent.setup();
+      const onSave = vi.fn();
+      renderModal(onSave);
+
+      await user.clear(tagsInput());
+      await user.click(screen.getByRole('button', { name: 'common.save' }));
+
+      // undefined would drop the key from the PATCH body and the backend would
+      // keep the old tags — the field has to be cleared explicitly.
+      expect(onSave).toHaveBeenCalledWith(expect.objectContaining({ tags: null }));
+    });
+  });
 });

+ 5 - 2
frontend/src/api/client.ts

@@ -923,6 +923,9 @@ export interface ProjectListItem {
   target_count: number | null;  // Target number of plates/print jobs
   target_parts_count: number | null;  // Target number of parts/objects
   budget: number | null;
+  tags: string | null;  // #2536 — the shared edit dialog seeds itself from this
+  due_date: string | null;  // #2536
+  priority: string;  // #2536
   created_at: string;
   archive_count: number;  // Number of print jobs (plates)
   total_items: number;  // Sum of quantities (total items printed, including failed)
@@ -958,8 +961,8 @@ export interface ProjectUpdate {
   target_count?: number;
   target_parts_count?: number;
   notes?: string;
-  tags?: string;
-  due_date?: string;
+  tags?: string | null;  // #2536 — explicit null clears the tags
+  due_date?: string | null;  // #2536 — explicit null clears the due date
   priority?: string;
   budget?: number | null;
   parent_id?: number;

+ 8 - 7
frontend/src/pages/ProjectsPage.tsx

@@ -62,9 +62,9 @@ export function ProjectModal({ project, onClose, onSave, isLoading, currencySymb
   const [targetCount, setTargetCount] = useState(project?.target_count?.toString() || '');
   const [targetPartsCount, setTargetPartsCount] = useState(project?.target_parts_count?.toString() || '');
   const [status, setStatus] = useState(project?.status || 'active');
-  const [tags, setTags] = useState((project as ProjectListItem & { tags?: string })?.tags || '');
-  const [dueDate, setDueDate] = useState((project as ProjectListItem & { due_date?: string })?.due_date?.split('T')[0] || '');
-  const [priority, setPriority] = useState((project as ProjectListItem & { priority?: string })?.priority || 'normal');
+  const [tags, setTags] = useState(project?.tags || '');
+  const [dueDate, setDueDate] = useState(project?.due_date?.split('T')[0] || '');
+  const [priority, setPriority] = useState(project?.priority || 'normal');
   const [budget, setBudget] = useState(project?.budget?.toString() || '');
   const [url, setUrl] = useState(project?.url || '');
   const [urlError, setUrlError] = useState<string | null>(null);
@@ -120,13 +120,14 @@ export function ProjectModal({ project, onClose, onSave, isLoading, currencySymb
       color,
       target_count: targetCount ? parseInt(targetCount, 10) : undefined,
       target_parts_count: targetPartsCount ? parseInt(targetPartsCount, 10) : undefined,
-      tags: tags.trim() || undefined,
-      due_date: dueDate || undefined,
+      // Null clears the stored value on edit; undefined omits the key on create.
+      // Sending undefined on edit would make an emptied field un-clearable.
+      tags: project ? (tags.trim() || null) : (tags.trim() || undefined),
+      due_date: project ? (dueDate || null) : (dueDate || undefined),
       priority,
       budget: budget.trim() ? parseFloat(budget) : null,
       // Pydantic accepts null to clear the URL; an empty string would fail the
-      // http(s) prefix validator. Use undefined for create (omit) and null for
-      // edit-with-cleared-value.
+      // http(s) prefix validator.
       url: project ? (trimmedUrl || null) : (trimmedUrl || undefined),
       ...(project && { status }),
     });

Разница между файлами не показана из-за своего большого размера
+ 0 - 0
static/assets/index-xvC2pmGi.js


+ 1 - 1
static/index.html

@@ -26,7 +26,7 @@
 
     <!-- Splash screens for iOS -->
     <link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
-    <script type="module" crossorigin src="/assets/index-PHP-xpqR.js"></script>
+    <script type="module" crossorigin src="/assets/index-xvC2pmGi.js"></script>
     <link rel="stylesheet" crossorigin href="/assets/index-DASc8Ke0.css">
   </head>
   <body>

Некоторые файлы не были показаны из-за большого количества измененных файлов