diff --git a/README.md b/README.md index 5d6d81f..3b6087b 100644 --- a/README.md +++ b/README.md @@ -65,6 +65,9 @@ Apple Reminders, forward mail into it, and add it to Claude as a connector. | REST API: projects, sections, tasks, sub-tasks, ordering | Done, tested | | Today, Upcoming, search, activity log | Done, tested | | Waiting on others: what you have delegated, by person | Done, tested | +| Assignee shown on every task that is somebody else's | Done, smoke-tested | +| Activity log, visible per project | Done, smoke-tested | +| Change a member's role without removing them | Done, tested | | Sharing: invites, roles, members | Done, tested | | Live sync over WebSocket | Done | | Web interface: sign-in, views, quick add, ⌘K, shortcuts | Done | diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 23ecaa1..ea77339 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -1352,6 +1352,36 @@ paths: type: array items: { $ref: "#/components/schemas/Task" } + /api/v1/people: + get: + tags: [views] + summary: Everybody you share a project with, and yourself. + description: | + What an interface needs to put a name and a face on an `assignee_id` — in + a task row, in a filter, anywhere a person appears beside work. One + request per session rather than a lookup per row, and per project members + would mean knowing which project every task belongs to first. + + Deliberately not the instance's user list: that is + `/users`, administrators only. A task list has no business enumerating + everybody with an account here. + responses: + "200": + description: OK + content: + application/json: + schema: + type: object + properties: + people: + type: array + items: + type: object + properties: + id: { type: string } + name: { type: string } + avatar_color: { type: string } + /api/v1/delegated: get: tags: [views] diff --git a/internal/httpapi/server.go b/internal/httpapi/server.go index 850ee56..b49daca 100644 --- a/internal/httpapi/server.go +++ b/internal/httpapi/server.go @@ -264,6 +264,7 @@ func New(cfg *config.Config, db *store.DB, log *slog.Logger, web fs.FS) *Server }) r.Get("/upcoming", s.handleUpcoming) r.Get("/delegated", s.handleDelegated) + r.Get("/people", s.handlePeople) r.Get("/search", s.handleSearch) r.Route("/projects", func(r chi.Router) { diff --git a/internal/httpapi/views_handlers.go b/internal/httpapi/views_handlers.go index e150a87..6646ca9 100644 --- a/internal/httpapi/views_handlers.go +++ b/internal/httpapi/views_handlers.go @@ -92,6 +92,31 @@ func (s *Server) handleUpcoming(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, map[string]any{"days": out}) } +// handlePeople lists everybody the caller shares a project with, so an interface +// can put a name and a face on an assignee id. +// +// One request per session rather than a lookup per row, and not the instance's +// user list — that is the administrator's page. A task list has no business +// enumerating everybody with an account here. +func (s *Server) handlePeople(w http.ResponseWriter, r *http.Request) { + people, err := s.db.ListPeople(r.Context(), userFrom(r.Context()).ID) + if err != nil { + s.internal(w, "list people", err) + return + } + out := make([]personJSON, 0, len(people)) + for _, p := range people { + out = append(out, personJSON{ID: p.ID, Name: p.Name, AvatarColor: p.AvatarColor}) + } + writeJSON(w, http.StatusOK, map[string]any{"people": out}) +} + +type personJSON struct { + ID string `json:"id"` + Name string `json:"name"` + AvatarColor string `json:"avatar_color"` +} + type delegatedPerson struct { UserID string `json:"user_id"` Name string `json:"name"` diff --git a/internal/httpapi/views_test.go b/internal/httpapi/views_test.go index 960b8ba..371ef5e 100644 --- a/internal/httpapi/views_test.go +++ b/internal/httpapi/views_test.go @@ -153,3 +153,50 @@ func TestDelegatedIsPerPersonAndRespectsAccess(t *testing.T) { t.Errorf("a task in a project they cannot see: %v", people) } } + +// /people is what puts a name on an assignee id. It must be the people you share +// work with — not the instance's user list, which is the administrator's page. +func TestPeopleIsWhoYouShareWorkWith(t *testing.T) { + ts := newTestServer(t) + ts.bootstrap(t) + colleague := ts.newUser(t, "anders@example.dk", "Anders") + ts.newUser(t, "fremmed@example.dk", "Fremmed") + + _, project := ts.do(t, "POST", "/api/v1/projects", map[string]any{"name": "Delt"}) + ts.do(t, "POST", "/api/v1/projects/"+project["id"].(string)+"/invites", map[string]any{ + "email": "anders@example.dk", "role": "editor", + }) + + _, body := ts.do(t, "GET", "/api/v1/people", nil) + names := map[string]bool{} + for _, raw := range body["people"].([]any) { + p := raw.(map[string]any) + names[p["name"].(string)] = true + if p["avatar_color"] == "" { + t.Errorf("no avatar colour for %v — the row cannot draw a face", p["name"]) + } + } + + if !names["Kristian"] { + t.Error("yourself is missing, so a client cannot tell 'me' from 'somebody else' against one list") + } + if !names["Anders"] { + t.Error("somebody sharing a project is missing") + } + if names["Fremmed"] { + t.Error("an account with no project in common is listed — this is not the user directory") + } + + // And it is symmetric: the person invited sees the person who invited them. + _, theirs := colleague.do(t, "GET", "/api/v1/people", nil) + seen := map[string]bool{} + for _, raw := range theirs["people"].([]any) { + seen[raw.(map[string]any)["name"].(string)] = true + } + if !seen["Kristian"] || !seen["Anders"] { + t.Errorf("the member cannot see who they share with: %v", seen) + } + if seen["Fremmed"] { + t.Errorf("a stranger is listed: %v", seen) + } +} diff --git a/internal/store/users.go b/internal/store/users.go index 0340ac6..7c3de83 100644 --- a/internal/store/users.go +++ b/internal/store/users.go @@ -274,6 +274,50 @@ func (db *DB) DeleteUser(ctx context.Context, userID string) error { return nil } +// ListPeople returns everybody the caller shares at least one project with, plus +// the caller. +// +// The set the interface needs to put a name on an `assignee_id` — in a task row, +// in a filter, anywhere a person appears next to work. Fetched once rather than +// looked up per row, and deliberately *not* the instance's user list: that is the +// administrator's page, and a task list has no business enumerating everybody with +// an account here. +// +// Themselves included, because a client comparing "is this me" against the same +// list it draws from has one source rather than two. +func (db *DB) ListPeople(ctx context.Context, userID string) ([]Person, error) { + rows, err := db.QueryContext(ctx, ` + SELECT DISTINCT u.id, u.name, u.avatar_color + FROM users u + WHERE u.id = ? + -- the owners of projects the caller is a member of + OR u.id IN (SELECT p.owner_id FROM projects p + JOIN project_members m ON m.project_id = p.id + WHERE m.user_id = ? AND p.deleted_at IS NULL) + -- and the members of every project the caller can see + OR u.id IN (SELECT m2.user_id FROM project_members m2 + JOIN projects p2 ON p2.id = m2.project_id + WHERE p2.deleted_at IS NULL + AND (p2.owner_id = ? + OR EXISTS (SELECT 1 FROM project_members m3 + WHERE m3.project_id = p2.id AND m3.user_id = ?))) + ORDER BY u.name`, userID, userID, userID, userID) + if err != nil { + return nil, err + } + defer rows.Close() + + people := []Person{} + for rows.Next() { + var p Person + if err := rows.Scan(&p.ID, &p.Name, &p.AvatarColor); err != nil { + return nil, err + } + people = append(people, p) + } + return people, rows.Err() +} + // UserCount is what decides whether the instance still needs its first admin. func (db *DB) UserCount(ctx context.Context) (int, error) { var n int diff --git a/web/e2e/smoke.spec.js b/web/e2e/smoke.spec.js index 73ac68d..8bad45c 100644 --- a/web/e2e/smoke.spec.js +++ b/web/e2e/smoke.spec.js @@ -570,7 +570,10 @@ test('et invitationslink opretter kontoen og giver adgang til projektet', async await sidebar.getByRole('link', { name: 'Venter på andre' }).click(); await expect(page.getByRole('heading', { name: 'Venter på andre' })).toBeVisible(); await expect(page.getByRole('heading', { name: 'Nabo' })).toBeVisible(); - await expect(page.getByText('males inden fredag')).toBeVisible(); + // The task's own text, not the whole row's: quick add read "fredag" as the due + // date, so the title is "males inden" and the rest of that line is metadata — + // which now has the assignee in it too. + await expect(page.getByText('males inden', { exact: true })).toBeVisible(); expect(trouble).toEqual([]); }); @@ -757,8 +760,32 @@ test('en rolle kan rettes uden at fjerne personen', async ({ browser, page }) => await andreas.reload(); await expect(andreas.getByLabel('Ny opgave')).toBeVisible(); await expect(andreas.getByText('Du kan se dette projekt')).toHaveCount(0); - await context.close(); + // Now that they can edit, hand them something — and the row says whose it is. + // Only when it is somebody else's: your own list would otherwise carry your own + // face on every line, which is a column of one initial telling you nothing. + await page.getByLabel('Ny opgave').fill('brænde kaffe'); + await page.getByLabel('Ny opgave').press('Enter'); + await page.getByText('brænde kaffe', { exact: true }).click(); + const drawer = page.getByRole('complementary', { name: 'Opgave' }); + await drawer.getByLabel('Ansvarlig').selectOption({ label: 'andreas' }); + await page.keyboard.press('Escape'); + + const row = page.locator('.row').filter({ hasText: 'brænde kaffe' }); + await expect(row.getByTitle('Ansvarlig: andreas')).toBeVisible(); + // And not on a task that is nobody's. + await page.getByLabel('Ny opgave').fill('min egen'); + await page.getByLabel('Ny opgave').press('Enter'); + await expect( + page.locator('.row').filter({ hasText: 'min egen' }).locator('.assignee') + ).toHaveCount(0); + + // The log has recorded all of this since the beginning and nothing showed it. + await page.getByRole('button', { name: 'Historik' }).click(); + await expect(page.getByText('oprettede projektet')).toBeVisible(); + await expect(page.getByText('ændrede rollen for')).toBeVisible(); + + await context.close(); expect(trouble).toEqual([]); }); diff --git a/web/src/lib/api.js b/web/src/lib/api.js index 5f8ca70..2690407 100644 --- a/web/src/lib/api.js +++ b/web/src/lib/api.js @@ -307,5 +307,6 @@ export const api = { today: () => get('/today'), upcoming: (days) => get(`/upcoming${days ? `?days=${days}` : ''}`), delegated: () => get('/delegated'), + people: () => get('/people'), search: (q) => get(`/search?q=${encodeURIComponent(q)}`) }; diff --git a/web/src/lib/components/TaskRow.svelte b/web/src/lib/components/TaskRow.svelte index 4b71f3f..787bf07 100644 --- a/web/src/lib/components/TaskRow.svelte +++ b/web/src/lib/components/TaskRow.svelte @@ -48,6 +48,8 @@ }; } + // Only when it is somebody else's: see app.assigneeOf. + let assignee = $derived(app.assigneeOf(task)); let due = $derived(dueLabel(task.due_date)); let time = $derived( task.due_datetime @@ -81,8 +83,19 @@ {task.description} {/if} - {#if due || task.labels?.length || task.recurrence_text} + {#if assignee || due || task.labels?.length || task.recurrence_text} + {#if assignee} + + + + {assignee.name[0]?.toUpperCase() ?? '?'} + + {assignee.name} + + {/if} {#if due} {due.text}{#if time} {time}{/if} @@ -259,6 +272,25 @@ stroke-linejoin: round; } + .assignee { + display: inline-flex; + align-items: center; + gap: var(--s1); + color: var(--ink-muted); + } + + .face { + width: 14px; + height: 14px; + border-radius: var(--radius-full); + display: inline-grid; + place-items: center; + font-size: 9px; + font-weight: 560; + color: #fff; + flex: none; + } + .label { color: var(--ink-muted); background: var(--surface-raised); diff --git a/web/src/lib/stores.svelte.js b/web/src/lib/stores.svelte.js index bd11fea..c38f525 100644 --- a/web/src/lib/stores.svelte.js +++ b/web/src/lib/stores.svelte.js @@ -18,6 +18,14 @@ class AppState { projects = $state([]); /** The foldable headings over the projects in the sidebar. */ groups = $state([]); + /** + * Everybody you share a project with, and yourself. + * + * Loaded once so a task row can put a name on an `assignee_id` without a + * request per row — and without every list having to know which project each + * task belongs to in order to ask for its members. + */ + people = $state([]); tasks = $state([]); loading = $state(true); /** Transient messages: a failed save, a rolled-back change. */ @@ -52,6 +60,23 @@ class AppState { return this.projects.find((p) => p.id === id); } + personById(id) { + return this.people.find((p) => p.id === id) ?? null; + } + + /** + * The person a task is waiting on, or null when it is nobody or yourself. + * + * Yourself is nobody, on purpose: every row in your own list would otherwise + * carry your own face, which is a column of the same initial telling you + * nothing. The point of showing an assignee is that it is *not* you. + */ + assigneeOf(task) { + const id = task?.assignee_id; + if (!id || id === this.user?.id) return null; + return this.personById(id) ?? { id, name: 'Ukendt', avatar_color: '#8a8f98' }; + } + get detailTask() { return this.tasks.find((t) => t.id === this.detailId) ?? null; } @@ -68,12 +93,14 @@ class AppState { this.loading = true; try { this.user = await api.me(); - const [{ projects }, { groups }] = await Promise.all([ + const [{ projects }, { groups }, { people }] = await Promise.all([ api.listProjects(), - api.listProjectGroups() + api.listProjectGroups(), + api.people() ]); this.projects = projects; this.groups = groups; + this.people = people; this.connect(); } catch (e) { this.user = null; diff --git a/web/src/routes/projekt/[id]/+page.svelte b/web/src/routes/projekt/[id]/+page.svelte index c2b9a53..672c8ac 100644 --- a/web/src/routes/projekt/[id]/+page.svelte +++ b/web/src/routes/projekt/[id]/+page.svelte @@ -26,6 +26,8 @@ let sections = $state([]); let members = $state([]); let showShare = $state(false); + let showLog = $state(false); + let activity = $state([]); let inviteEmail = $state(''); let inviteRole = $state('editor'); let inviteLink = $state(''); @@ -65,6 +67,63 @@ } }); + // Loaded when it is opened rather than with the page: it is a record you go + // looking for, not something every visit should pay for. + $effect(() => { + if (showLog && project) { + api.activity(project.id).then((r) => (activity = r.activity)).catch(() => {}); + } + }); + + /** + * What each recorded event is called. + * + * The log has been written since the beginning — nineteen kinds of entry — and + * nothing has ever read it. The keys are the server's and they are stable; this + * is the only place they become Danish. + * + * An unknown key falls through to the key itself rather than being hidden: a + * build that records something this table has not learned yet should still show + * that it happened. + */ + const EVENTS = { + 'project.created': 'oprettede projektet', + 'project.updated': 'ændrede projektet', + 'project.deleted': 'slettede projektet', + 'project.imported': 'importerede projektet', + 'section.created': 'oprettede en sektion', + 'section.updated': 'ændrede en sektion', + 'section.deleted': 'slettede en sektion', + 'member.invited': 'inviterede', + 'member.added': 'tilføjede', + 'member.removed': 'fjernede', + 'member.role_changed': 'ændrede rollen for', + 'task.created': 'oprettede en opgave', + 'task.updated': 'ændrede en opgave', + 'task.completed': 'lukkede en opgave', + 'task.reopened': 'genåbnede en opgave', + 'task.moved': 'flyttede en opgave', + 'task.deleted': 'slettede en opgave', + 'task.split': 'delte en opgave op', + 'comment.created': 'skrev en kommentar' + }; + + /** The bit of context an entry carries, when it has one worth reading. */ + function detail(entry) { + const p = entry.payload ?? {}; + return p.name ?? p.email ?? p.role ?? ''; + } + + function when(iso) { + const then = new Date(iso); + const seconds = Math.round((Date.now() - then) / 1000); + if (seconds < 60) return 'lige nu'; + if (seconds < 3600) return `for ${Math.floor(seconds / 60)} min. siden`; + if (seconds < 86400) return `for ${Math.floor(seconds / 3600)} timer siden`; + if (seconds < 604800) return `for ${Math.floor(seconds / 86400)} dage siden`; + return then.toLocaleDateString('da-DK', { day: 'numeric', month: 'short', year: 'numeric' }); + } + // The chosen view is kept locally as well as on the project, so switching is // instant and only persists once the request lands. let view = $state(null); @@ -353,6 +412,12 @@ {/if} + {#if !project.is_inbox} + + {/if} + {#if isOwner && !project.is_inbox}