From 7194a19dd0041ee244a5200b281f1ab378fb187e Mon Sep 17 00:00:00 2001 From: Kaan Karaca Date: Tue, 1 Sep 2026 23:29:31 +0300 Subject: [PATCH 1/5] test(projects): cover removing an item from a project Reproduces #1941 on the surface the report names. A binary file's project membership is only ever shown by ItemProjectChips, and those chips have no remove control, so the renderer cases and the E2E file case fail against current source. The E2E spec drives both item kinds through the real app, and the main cases pin the file branch of unlinkProjectItem that the missing UI never reached. --- .../src/main/tasks/project-item-links.test.ts | 31 +++ .../projects/item-project-chips.test.tsx | 83 +++++- .../desktop/tests/e2e/project-unassign.e2e.ts | 244 ++++++++++++++++++ 3 files changed, 353 insertions(+), 5 deletions(-) create mode 100644 apps/desktop/tests/e2e/project-unassign.e2e.ts diff --git a/apps/desktop/src/main/tasks/project-item-links.test.ts b/apps/desktop/src/main/tasks/project-item-links.test.ts index 6c0da7e056..ad9cb04896 100644 --- a/apps/desktop/src/main/tasks/project-item-links.test.ts +++ b/apps/desktop/src/main/tasks/project-item-links.test.ts @@ -81,6 +81,37 @@ describe('project item link reroute', () => { expect(setEntityProperties).toHaveBeenCalledWith('n1', { project: ['Beta'] }) }) + // The chips on the file page are the only unassign surface a binary file + // has. It owns no frontmatter, so the row in `project_links` is the whole of + // its membership and this branch is the entire write. + it('deletes the link row for a file on unlink, writing no frontmatter', async () => { + isMarkdownNote.mockReturnValue(false) + domainUnlink.mockResolvedValue({ success: true }) + + const result = await unlinkProjectItem({} as never, domain as never, { + projectId: 'p1', + itemType: 'file', + itemId: 'f1' + }) + + expect(result).toEqual({ success: true }) + expect(domainUnlink).toHaveBeenCalledWith({ projectId: 'p1', itemType: 'file', itemId: 'f1' }) + expect(setEntityProperties).not.toHaveBeenCalled() + }) + + it('reports a failed file unlink instead of claiming success', async () => { + isMarkdownNote.mockReturnValue(false) + domainUnlink.mockResolvedValue({ success: false }) + + expect( + await unlinkProjectItem({} as never, domain as never, { + projectId: 'p1', + itemType: 'file', + itemId: 'f1' + }) + ).toEqual({ success: false, error: 'Failed to unlink item' }) + }) + it('errors when the project does not exist', async () => { isMarkdownNote.mockReturnValue(true) getProjectById.mockReturnValue(undefined) diff --git a/apps/desktop/src/renderer/src/components/tasks/projects/item-project-chips.test.tsx b/apps/desktop/src/renderer/src/components/tasks/projects/item-project-chips.test.tsx index 5c3f3462de..5f9a70af38 100644 --- a/apps/desktop/src/renderer/src/components/tasks/projects/item-project-chips.test.tsx +++ b/apps/desktop/src/renderer/src/components/tasks/projects/item-project-chips.test.tsx @@ -2,22 +2,30 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import { render, screen, fireEvent, waitFor } from '@testing-library/react' import { ItemProjectChips } from './item-project-chips' -const { mockListForItem, mockOnProjectUpdated } = vi.hoisted(() => ({ - mockListForItem: vi.fn(), - mockOnProjectUpdated: vi.fn(() => () => {}) -})) +const { mockListForItem, mockOnProjectUpdated, mockUnlinkProjectItem, mockToastError } = vi.hoisted( + () => ({ + mockListForItem: vi.fn(), + mockOnProjectUpdated: vi.fn(() => () => {}), + mockUnlinkProjectItem: vi.fn(), + mockToastError: vi.fn() + }) +) vi.mock('@/services/tasks-service', () => ({ tasksService: { - listForItem: mockListForItem + listForItem: mockListForItem, + unlinkProjectItem: mockUnlinkProjectItem }, onProjectUpdated: mockOnProjectUpdated })) +vi.mock('sonner', () => ({ toast: { error: mockToastError } })) + describe('ItemProjectChips', () => { beforeEach(() => { vi.clearAllMocks() mockOnProjectUpdated.mockReturnValue(() => {}) + mockUnlinkProjectItem.mockResolvedValue({ success: true }) }) it('renders a chip per linked project', async () => { @@ -79,4 +87,69 @@ describe('ItemProjectChips', () => { expect(await screen.findByText('Launch')).toBeInTheDocument() expect(mockListForItem).toHaveBeenCalledTimes(2) }) + + // A binary file has no frontmatter, so these chips are the whole of its + // project membership UI. Without a remove control on them the only exits + // from a project are destructive (issue #1941). + it('unlinks the item when a chip remove control is used', async () => { + mockListForItem + .mockResolvedValueOnce([{ id: 'p1', name: 'Launch', color: '#f00', icon: null }]) + .mockResolvedValue([]) + + render() + + fireEvent.click(await screen.findByRole('button', { name: /remove from launch/i })) + + await waitFor(() => + expect(mockUnlinkProjectItem).toHaveBeenCalledWith({ + projectId: 'p1', + itemType: 'file', + itemId: 'f1' + }) + ) + await waitFor(() => expect(screen.queryByText('Launch')).not.toBeInTheDocument()) + }) + + it('gives each chip its own remove control, naming the project it drops', async () => { + mockListForItem.mockResolvedValue([ + { id: 'p1', name: 'Launch', color: '#f00', icon: null }, + { id: 'p2', name: 'Finance', color: '#0f0', icon: null } + ]) + + render() + + fireEvent.click(await screen.findByRole('button', { name: /remove from finance/i })) + + await waitFor(() => + expect(mockUnlinkProjectItem).toHaveBeenCalledWith({ + projectId: 'p2', + itemType: 'file', + itemId: 'f1' + }) + ) + }) + + it('renders the name as plain text where there is nowhere to navigate', async () => { + mockListForItem.mockResolvedValue([{ id: 'p1', name: 'Launch', color: '#f00', icon: null }]) + + render() + + expect(await screen.findByText('Launch')).toBeInTheDocument() + expect(screen.queryByRole('button', { name: /open project launch/i })).not.toBeInTheDocument() + expect(screen.getByRole('button', { name: /remove from launch/i })).toBeInTheDocument() + }) + + // The main side answers a failed unlink with an envelope rather than a + // rejection, so a bare await would read it as success and blank the chip. + it('keeps the chip and reports a rejected unlink', async () => { + mockListForItem.mockResolvedValue([{ id: 'p1', name: 'Launch', color: '#f00', icon: null }]) + mockUnlinkProjectItem.mockResolvedValue({ success: false, error: 'link is gone' }) + + render() + + fireEvent.click(await screen.findByRole('button', { name: /remove from launch/i })) + + await waitFor(() => expect(mockToastError).toHaveBeenCalledWith('link is gone')) + expect(screen.getByText('Launch')).toBeInTheDocument() + }) }) diff --git a/apps/desktop/tests/e2e/project-unassign.e2e.ts b/apps/desktop/tests/e2e/project-unassign.e2e.ts new file mode 100644 index 0000000000..fb1c36503c --- /dev/null +++ b/apps/desktop/tests/e2e/project-unassign.e2e.ts @@ -0,0 +1,244 @@ +import fs from 'fs' +import os from 'os' +import path from 'path' + +import type { Page } from '@playwright/test' + +import { test, expect } from './fixtures' +import { PNG_BYTES, ready, uniqueLabel } from './utils/desktop-test-helpers' +import { seedNote, tabSessionStorageKey } from './utils/electron-helpers' + +/** + * Issue #1941: a file assigned to a project could not be unassigned without + * deleting the project or the file. + * + * The two item kinds reach `project_links` by different routes, so both are + * proved here. A markdown note carries its membership in frontmatter and the + * projector derives the row, which is why the note block asserts on the raw + * file bytes as well as on the link. A binary file has no frontmatter, so its + * row in `project_links` is the whole of its membership and the chips on the + * file page are the only place it is ever shown. + * + * Inbox is covered alongside a user-created project because the report named + * it, and because `listProjects` does not treat it differently — a regression + * that special-cased it would only show up here. + */ + +interface SeededProjects { + inboxId: string + inboxName: string + projectId: string +} + +async function seedProjects(page: Page, projectName: string): Promise { + return page.evaluate(async (name) => { + const created = await window.api.tasks.createProject({ name, color: '#6366f1' }) + if (!created.success || !created.project) { + throw new Error(created.error ?? 'project create failed') + } + + const { projects } = await window.api.tasks.listProjects() + const inbox = projects.find((project) => project.isInbox) + if (!inbox) throw new Error('inbox project missing') + + return { inboxId: inbox.id, inboxName: inbox.name, projectId: created.project.id } + }, projectName) +} + +async function linkedItemIds(page: Page, projectId: string): Promise { + return page.evaluate(async (id) => { + const links = await window.api.tasks.listProjectLinks(id) + return Array.isArray(links) ? links.map((link) => link.itemId) : [] + }, projectId) +} + +test.describe('Unassigning from a project', () => { + test('a note leaves both a user project and Inbox from its project property', async ({ + page, + testVaultPath + }) => { + await ready(page) + + const projectName = uniqueLabel('Unassign Project') + const { inboxId, inboxName, projectId } = await seedProjects(page, projectName) + const noteId = await seedNote(page, uniqueLabel('Unassign Note'), 'Body') + + await page.evaluate( + async ({ noteId, projectId, inboxId }) => { + for (const id of [projectId, inboxId]) { + const linked = await window.api.tasks.linkProjectItem({ + projectId: id, + itemType: 'note', + itemId: noteId + }) + if (!linked.success) throw new Error(linked.error ?? 'link failed') + } + }, + { noteId, projectId, inboxId } + ) + + await expect.poll(() => linkedItemIds(page, projectId)).toContain(noteId) + await expect.poll(() => linkedItemIds(page, inboxId)).toContain(noteId) + + const notePath = await page.evaluate(async (id) => { + const note = await window.api.notes.get(id) + return note?.path ?? null + }, noteId) + expect(notePath).toBeTruthy() + expect(fs.readFileSync(path.join(testVaultPath, notePath!), 'utf8')).toContain(projectName) + + await openNoteTab(page, noteId) + + const properties = page.getByRole('list', { name: 'Properties list' }).first() + await expect(properties).toBeVisible() + + for (const name of [projectName, inboxName]) { + // Exact: the property row's own wrapper is a role="button" whose + // accessible name concatenates the chip label with this one, so a + // substring match would resolve to two elements. + const remove = properties.getByRole('button', { name: `Remove from ${name}`, exact: true }) + await expect(remove).toBeVisible() + await remove.click() + await expect(remove).toHaveCount(0) + } + + await expect.poll(() => linkedItemIds(page, projectId)).not.toContain(noteId) + await expect.poll(() => linkedItemIds(page, inboxId)).not.toContain(noteId) + + await page.reload() + await page.waitForLoadState('domcontentloaded') + + await expect.poll(() => linkedItemIds(page, projectId)).not.toContain(noteId) + await expect.poll(() => linkedItemIds(page, inboxId)).not.toContain(noteId) + + // The file is the sync payload and the reindex source, so the link has to + // be gone from the bytes, not only from the index. `Note.content` is the + // body with the frontmatter stripped, so the property is read off + // `frontmatter` and confirmed against the file on disk. + const stored = await page.evaluate(async (id) => { + const note = await window.api.notes.get(id) + return note ? JSON.stringify(note.frontmatter) : null + }, noteId) + expect(stored).not.toContain(projectName) + expect(fs.readFileSync(path.join(testVaultPath, notePath!), 'utf8')).not.toContain(projectName) + }) + + test('a file leaves both a user project and Inbox from its project chips', async ({ page }) => { + await ready(page) + + const projectName = uniqueLabel('Unassign File Project') + const { inboxId, inboxName, projectId } = await seedProjects(page, projectName) + + const importDir = fs.mkdtempSync(path.join(os.tmpdir(), 'memry-e2e-unassign-')) + const fileName = `unassign-${Date.now()}.png` + fs.writeFileSync(path.join(importDir, fileName), Buffer.from(PNG_BYTES)) + const fileTitle = path.basename(fileName, path.extname(fileName)) + + try { + const imported = await page.evaluate( + async (sourcePath) => window.api.notes.importFiles([sourcePath], ''), + path.join(importDir, fileName) + ) + expect(imported.success).toBe(true) + + const findFileId = async (): Promise => + page.evaluate(async (title) => { + const list = await window.api.notes.list({ limit: 200 }) + return list.notes.find((note) => note.title === title)?.id ?? null + }, fileTitle) + + // The import writes the file; the indexer gives it an id a moment later. + await expect.poll(findFileId, { timeout: 20_000 }).not.toBeNull() + const fileId = await findFileId() + expect(fileId).toBeTruthy() + + await page.evaluate( + async ({ fileId, projectId, inboxId }) => { + for (const id of [projectId, inboxId]) { + const linked = await window.api.tasks.linkProjectItem({ + projectId: id, + itemType: 'file', + itemId: fileId + }) + if (!linked.success) throw new Error(linked.error ?? 'link failed') + } + }, + { fileId: fileId!, projectId, inboxId } + ) + + await expect.poll(() => linkedItemIds(page, projectId)).toContain(fileId) + await expect.poll(() => linkedItemIds(page, inboxId)).toContain(fileId) + + await openFileTab(page, fileId!, fileTitle) + + for (const name of [projectName, inboxName]) { + const remove = page.getByRole('button', { name: `Remove from ${name}`, exact: true }) + await expect(remove).toBeVisible() + await remove.click() + await expect(remove).toHaveCount(0) + } + + await expect.poll(() => linkedItemIds(page, projectId)).not.toContain(fileId) + await expect.poll(() => linkedItemIds(page, inboxId)).not.toContain(fileId) + + await page.reload() + await page.waitForLoadState('domcontentloaded') + + await expect.poll(() => linkedItemIds(page, projectId)).not.toContain(fileId) + await expect.poll(() => linkedItemIds(page, inboxId)).not.toContain(fileId) + } finally { + fs.rmSync(importDir, { recursive: true, force: true }) + } + }) +}) + +async function openNoteTab(page: Page, noteId: string): Promise { + await seedTab(page, { type: 'note', id: noteId, title: 'Unassign Note', path: `/note/${noteId}` }) + await expect(page.getByRole('list', { name: 'Properties list' }).first()).toBeVisible() +} + +async function openFileTab(page: Page, fileId: string, title: string): Promise { + await seedTab(page, { type: 'file', id: fileId, title, path: `/file/${fileId}` }) + await expect(page.getByRole('heading', { name: title })).toBeVisible() +} + +async function seedTab( + page: Page, + tab: { type: string; id: string; title: string; path: string } +): Promise { + const storageKey = await tabSessionStorageKey(page) + await page.addInitScript( + ({ tab, storageKey }) => { + localStorage.setItem( + storageKey, + JSON.stringify({ + version: 2, + tabGroups: { + g1: { + id: 'g1', + activeTabId: 'seeded-tab', + tabs: [ + { + id: 'seeded-tab', + type: tab.type, + title: tab.title, + icon: tab.type, + path: tab.path, + entityId: tab.id, + isPinned: false + } + ] + } + }, + layout: { type: 'leaf', tabGroupId: 'g1' }, + activeGroupId: 'g1', + settings: { restoreSessionOnStart: true, tabCloseButton: 'hover' }, + savedAt: Date.now() + }) + ) + }, + { tab, storageKey } + ) + await page.reload() + await page.waitForLoadState('domcontentloaded') +} From 510c625508e77972a48c6a71e02c8fc8a469f0aa Mon Sep 17 00:00:00 2001 From: Kaan Karaca Date: Tue, 1 Sep 2026 23:29:32 +0300 Subject: [PATCH 2/5] fix(projects): let an item leave a project from its project chips A file assigned to a project could only be unassigned by deleting the project or the file. Three surfaces add a file to a project (a sidebar drag, the file page's Add to project, the project hub's paperclip import) and none could undo it: the file page shows the membership as chips that only navigate, and a file has no frontmatter, so unlike a markdown note it has no project property row to clear instead. Every chip now carries a remove control that calls the unlinkProjectItem IPC that already existed and had no caller outside the calendar. For a file that deletes the project_links row, which is the whole of its membership and cannot be resurrected by a reindex; for a markdown note the same call rewrites the name out of frontmatter, so the file stays the payload sync carries. The project name is now plain text where the row is mounted without onProjectClick, as the file page does, rather than a second tab stop that does nothing beside the real control. Closes #1941 --- .../tasks/projects/item-project-chips.tsx | 86 +++++++++++++++---- packages/i18n/src/locales/en/tasks.json | 4 +- 2 files changed, 71 insertions(+), 19 deletions(-) diff --git a/apps/desktop/src/renderer/src/components/tasks/projects/item-project-chips.tsx b/apps/desktop/src/renderer/src/components/tasks/projects/item-project-chips.tsx index 80f00d98ee..ea4704ff0c 100644 --- a/apps/desktop/src/renderer/src/components/tasks/projects/item-project-chips.tsx +++ b/apps/desktop/src/renderer/src/components/tasks/projects/item-project-chips.tsx @@ -1,4 +1,6 @@ import { useCallback, useEffect, useState } from 'react' +import { toast } from 'sonner' +import { X } from '@/lib/icons' import { cn } from '@/lib/utils' import { tasksService, @@ -12,6 +14,19 @@ import { useT } from '@memry/i18n/renderer' const log = createLogger('ItemProjectChips') +function ProjectChipLabel({ project }: { project: ProjectRef }): React.JSX.Element { + return ( + <> +