diff --git a/server/router/api/v1/memo_service.go b/server/router/api/v1/memo_service.go index 4475f213..7f222575 100644 --- a/server/router/api/v1/memo_service.go +++ b/server/router/api/v1/memo_service.go @@ -406,6 +406,9 @@ func (s *APIV1Service) UpdateMemo(ctx context.Context, request *v1pb.UpdateMemoR } else if path == "update_time" { updatedTsSec := time.Now().Unix() if request.Memo.UpdateTime != nil { + if !request.Memo.UpdateTime.IsValid() { + return nil, status.Errorf(codes.InvalidArgument, "update_time is invalid") + } updatedTsSec = request.Memo.UpdateTime.AsTime().Unix() } update.UpdatedTs = &updatedTsSec diff --git a/server/router/api/v1/test/memo_timestamp_test.go b/server/router/api/v1/test/memo_timestamp_test.go new file mode 100644 index 00000000..fc057db7 --- /dev/null +++ b/server/router/api/v1/test/memo_timestamp_test.go @@ -0,0 +1,123 @@ +package test + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "google.golang.org/protobuf/types/known/fieldmaskpb" + "google.golang.org/protobuf/types/known/timestamppb" + + apiv1 "github.com/usememos/memos/proto/gen/api/v1" + "github.com/usememos/memos/store" +) + +func TestUpdateMemoTimestamps(t *testing.T) { + ctx := context.Background() + ts := NewTestService(t) + defer ts.Cleanup() + user, err := ts.CreateRegularUser(ctx, "timestamp-owner") + require.NoError(t, err) + userCtx := ts.CreateUserContext(ctx, user.ID) + originalCreate := int64(1577836800) + originalUpdate := originalCreate + 3600 + customCreate := originalCreate - 86400 + customUpdate := originalUpdate - 86400 + + for _, tc := range []struct { + name string + paths []string + patch *apiv1.Memo + wantCreate int64 + wantUpdate int64 + autoUpdate bool + }{ + {"creation only", []string{"create_time"}, &apiv1.Memo{CreateTime: ×tamppb.Timestamp{Seconds: customCreate}}, customCreate, originalUpdate, false}, + {"modification only", []string{"update_time"}, &apiv1.Memo{UpdateTime: ×tamppb.Timestamp{Seconds: customUpdate}}, originalCreate, customUpdate, false}, + {"both", []string{"create_time", "update_time"}, &apiv1.Memo{CreateTime: ×tamppb.Timestamp{Seconds: customCreate}, UpdateTime: ×tamppb.Timestamp{Seconds: customUpdate}}, customCreate, customUpdate, false}, + {"with content", []string{"content", "create_time", "update_time"}, &apiv1.Memo{Content: "changed", CreateTime: ×tamppb.Timestamp{Seconds: customCreate}, UpdateTime: ×tamppb.Timestamp{Seconds: customUpdate}}, customCreate, customUpdate, false}, + {"automatic modification", []string{"content", "update_time"}, &apiv1.Memo{Content: "changed"}, originalCreate, 0, true}, + {"unmasked timestamps ignored", []string{"content"}, &apiv1.Memo{Content: "changed", CreateTime: ×tamppb.Timestamp{Seconds: customCreate}, UpdateTime: ×tamppb.Timestamp{Seconds: customUpdate}}, originalCreate, originalUpdate, false}, + } { + t.Run(tc.name, func(t *testing.T) { + memo, err := ts.Service.CreateMemo(userCtx, &apiv1.CreateMemoRequest{Memo: &apiv1.Memo{ + Content: "original", Visibility: apiv1.Visibility_PRIVATE, + CreateTime: ×tamppb.Timestamp{Seconds: originalCreate}, UpdateTime: ×tamppb.Timestamp{Seconds: originalUpdate}, + }}) + require.NoError(t, err) + tc.patch.Name = memo.Name + before := time.Now().Unix() + updated, err := ts.Service.UpdateMemo(userCtx, &apiv1.UpdateMemoRequest{ + Memo: tc.patch, UpdateMask: &fieldmaskpb.FieldMask{Paths: tc.paths}, + }) + require.NoError(t, err) + require.Equal(t, tc.wantCreate, updated.CreateTime.Seconds) + if tc.autoUpdate { + require.GreaterOrEqual(t, updated.UpdateTime.Seconds, before) + require.LessOrEqual(t, updated.UpdateTime.Seconds, time.Now().Unix()) + } else { + require.Equal(t, tc.wantUpdate, updated.UpdateTime.Seconds) + } + // Verify both API reads and raw stored values, not just the update response. + fetched, err := ts.Service.GetMemo(userCtx, &apiv1.GetMemoRequest{Name: memo.Name}) + require.NoError(t, err) + require.Equal(t, updated.CreateTime, fetched.CreateTime) + require.Equal(t, updated.UpdateTime, fetched.UpdateTime) + id := parseMemoIDFromNameForTest(t, ts, memo.Name) + stored, err := ts.Store.GetMemo(ctx, &store.FindMemo{ID: &id}) + require.NoError(t, err) + require.Equal(t, updated.CreateTime.Seconds, stored.CreatedTs) + require.Equal(t, updated.UpdateTime.Seconds, stored.UpdatedTs) + wantContent := "original" + if tc.patch.Content != "" { + wantContent = tc.patch.Content + } + require.Equal(t, wantContent, stored.Content) + }) + } +} + +func TestUpdateMemoRejectsInvalidTimestamps(t *testing.T) { + ctx := context.Background() + ts := NewTestService(t) + defer ts.Cleanup() + user, err := ts.CreateRegularUser(ctx, "invalid-timestamp-owner") + require.NoError(t, err) + userCtx := ts.CreateUserContext(ctx, user.ID) + memo, err := ts.Service.CreateMemo(userCtx, &apiv1.CreateMemoRequest{Memo: &apiv1.Memo{ + Content: "original", Visibility: apiv1.Visibility_PRIVATE, + }}) + require.NoError(t, err) + for _, path := range []string{"create_time", "update_time"} { + for _, timestamp := range []*timestamppb.Timestamp{ + {Seconds: 253402300800}, + {Nanos: -1}, + {Nanos: 1000000000}, + nil, + } { + if path == "update_time" && timestamp == nil { + continue // An omitted update_time explicitly requests the current time. + } + t.Run(path+"/"+timestamp.String(), func(t *testing.T) { + patch := &apiv1.Memo{Name: memo.Name, Content: "must not persist"} + if path == "create_time" { + patch.CreateTime = timestamp + } else { + patch.UpdateTime = timestamp + } + _, err := ts.Service.UpdateMemo(userCtx, &apiv1.UpdateMemoRequest{ + Memo: patch, UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"content", path}}, + }) + require.Equal(t, codes.InvalidArgument, status.Code(err)) + fetched, err := ts.Service.GetMemo(userCtx, &apiv1.GetMemoRequest{Name: memo.Name}) + require.NoError(t, err) + require.Equal(t, memo.Content, fetched.Content) + require.Equal(t, memo.CreateTime, fetched.CreateTime) + require.Equal(t, memo.UpdateTime, fetched.UpdateTime) + }) + } + } +} diff --git a/store/test/memo_timestamp_test.go b/store/test/memo_timestamp_test.go new file mode 100644 index 00000000..fc428f88 --- /dev/null +++ b/store/test/memo_timestamp_test.go @@ -0,0 +1,51 @@ +package test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/usememos/memos/store" +) + +func TestMemoTimestampUpdates(t *testing.T) { + t.Parallel() + ctx := context.Background() + ts := NewTestingStore(ctx, t) + defer ts.Close() + user, err := createTestingHostUser(ctx, ts) + require.NoError(t, err) + originalCreate, originalUpdate := int64(1577836800), int64(1577840400) + created, updated := originalCreate-86400, originalUpdate-86400 + + for _, tc := range []struct { + name string + patch *store.UpdateMemo + wantCreate int64 + wantUpdate int64 + }{ + {"creation-only", &store.UpdateMemo{CreatedTs: &created}, created, originalUpdate}, + {"modification-only", &store.UpdateMemo{UpdatedTs: &updated}, originalCreate, updated}, + {"both", &store.UpdateMemo{CreatedTs: &created, UpdatedTs: &updated}, created, updated}, + {"unrelated-update", &store.UpdateMemo{Content: new("changed")}, originalCreate, originalUpdate}, + } { + t.Run(tc.name, func(t *testing.T) { + memo, err := ts.CreateMemo(ctx, &store.Memo{ + UID: tc.name, CreatorID: user.ID, Content: "original", Visibility: store.Private, + CreatedTs: originalCreate, UpdatedTs: originalUpdate, + }) + require.NoError(t, err) + tc.patch.ID = memo.ID + require.NoError(t, ts.UpdateMemo(ctx, tc.patch)) + got, err := ts.GetMemo(ctx, &store.FindMemo{ID: &memo.ID}) + require.NoError(t, err) + require.NotNil(t, got) + require.Equal(t, tc.wantCreate, got.CreatedTs) + require.Equal(t, tc.wantUpdate, got.UpdatedTs) + if tc.patch.Content != nil { + require.Equal(t, *tc.patch.Content, got.Content) + } + }) + } +} diff --git a/web/src/components/MemoEditor/components/TimestampPopover.tsx b/web/src/components/MemoEditor/components/TimestampPopover.tsx index 02c3c144..1b5420c3 100644 --- a/web/src/components/MemoEditor/components/TimestampPopover.tsx +++ b/web/src/components/MemoEditor/components/TimestampPopover.tsx @@ -1,4 +1,4 @@ -import { type FC, useRef, useState } from "react"; +import { type FC, useId, useRef, useState } from "react"; import { Popover, PopoverContent, PopoverTrigger } from "@/components/ui/popover"; import { useTranslate } from "@/utils/i18n"; import { useEditorContext, useEditorSelector } from "../state"; @@ -12,9 +12,12 @@ function formatDate(date: Date): string { function parseDate(value: string): Date | undefined { const match = value.match(/^(\d{4})-(\d{2})-(\d{2}) (\d{2}):(\d{2}):(\d{2})$/); - if (!match) return undefined; - const date = new Date(Number(match[1]), Number(match[2]) - 1, Number(match[3]), Number(match[4]), Number(match[5]), Number(match[6])); - return Number.isNaN(date.getTime()) ? undefined : date; + if (!match || Number(match[1]) === 0) return undefined; + const date = new Date(0); + date.setFullYear(Number(match[1]), Number(match[2]) - 1, Number(match[3])); + date.setHours(Number(match[4]), Number(match[5]), Number(match[6]), 0); + // Date normalizes overflowing fields (e.g. February 30) instead of rejecting them. + return formatDate(date) === value ? date : undefined; } const TimestampInput: FC<{ @@ -22,34 +25,38 @@ const TimestampInput: FC<{ date: Date | undefined; onChange: (date: Date) => void; }> = ({ label, date, onChange }) => { + const id = useId(); const initialValue = useRef(date ? formatDate(date) : ""); const [value, setValue] = useState(initialValue.current); const [invalid, setInvalid] = useState(false); - const handleBlur = () => { - const parsed = parseDate(value); + const handleChange = (nextValue: string) => { + setValue(nextValue); + const parsed = parseDate(nextValue); + setInvalid(false); if (parsed) { - setInvalid(false); + // Closing the popover can unmount the input without firing blur. + // Keep valid edits in the editor store as soon as they are entered. onChange(parsed); - } else { - setInvalid(true); } }; return (
-
); diff --git a/web/tests/memo-editor-timestamps.test.tsx b/web/tests/memo-editor-timestamps.test.tsx new file mode 100644 index 00000000..6504cf1d --- /dev/null +++ b/web/tests/memo-editor-timestamps.test.tsx @@ -0,0 +1,104 @@ +import { create } from "@bufbuild/protobuf"; +import { timestampFromDate } from "@bufbuild/protobuf/wkt"; +import { act, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { TimestampPopover } from "@/components/MemoEditor/components/TimestampPopover"; +import { memoService } from "@/components/MemoEditor/services/memoService"; +import { createInitialState, EditorProvider, useEditorContext } from "@/components/MemoEditor/state"; +import { type Memo, MemoSchema } from "@/types/proto/api/v1/memo_service_pb"; + +const clients = vi.hoisted(() => ({ getMemo: vi.fn(), updateMemo: vi.fn() })); +vi.mock("@/connect", () => ({ + attachmentServiceClient: { createAttachment: vi.fn() }, + memoServiceClient: clients, +})); +vi.mock("@/utils/i18n", () => ({ useTranslate: () => (key: string) => key })); + +const originalCreate = new Date(2020, 0, 1, 10, 20, 30); +const originalUpdate = new Date(2020, 0, 2, 11, 22, 33); +const customTime = new Date(2024, 1, 29, 12, 34, 56); +let editor: ReturnType; + +function Probe() { + editor = useEditorContext(); + return ; +} + +function renderEditor() { + const memo = create(MemoSchema, { + name: "memos/timestamps", + content: "Original content", + createTime: timestampFromDate(originalCreate), + updateTime: timestampFromDate(originalUpdate), + }); + clients.getMemo.mockResolvedValue(memo); + render( + + + , + ); + fireEvent.click(screen.getByRole("button")); + return memo; +} + +describe("memo editor timestamps", () => { + beforeEach(() => { + clients.updateMemo.mockImplementation(async ({ memo }: { memo: Memo }) => memo); + }); + + it.each([ + [0, "createTime", "create_time"], + [1, "updateTime", "update_time"], + ] as const)("saves %s after dismissing the focused input with Escape", async (index, field, path) => { + const memo = renderEditor(); + const input = screen.getAllByRole("textbox")[index]; + act(() => input.focus()); + fireEvent.change(input, { target: { value: "2024-02-29 12:34:56" } }); + fireEvent.keyDown(input, { key: "Escape" }); + await waitFor(() => expect(screen.queryByRole("textbox")).not.toBeInTheDocument()); + + await memoService.save(editor.getState(), { memoName: memo.name }); + + expect(clients.updateMemo).toHaveBeenCalledOnce(); + const request = clients.updateMemo.mock.calls[0][0]; + expect(request.updateMask.paths).toEqual([path]); + expect(request.memo[field]).toEqual(timestampFromDate(customTime)); + }); + + it.each([ + "2024-02-30 12:34:56", + "2024-13-01 12:34:56", + "2024-02-29 24:34:56", + "invalid", + ])("rejects invalid local time %s without changing the saved timestamp", async (value) => { + const memo = renderEditor(); + const input = screen.getAllByRole("textbox")[0]; + fireEvent.change(input, { target: { value } }); + fireEvent.blur(input); + expect(input).toHaveAttribute("data-invalid", "true"); + expect(editor.getState().timestamps.createTime).toEqual(originalCreate); + await memoService.save(editor.getState(), { memoName: memo.name }); + expect(clients.updateMemo).not.toHaveBeenCalled(); + }); + + it("automatically updates the modification time for content edits", async () => { + const memo = renderEditor(); + const state = { ...editor.getState(), content: "Changed content" }; + await memoService.save(state, { memoName: memo.name }); + const request = clients.updateMemo.mock.calls[0][0]; + expect(request.updateMask.paths).toEqual(["content", "update_time"]); + expect(request.memo.updateTime).toBeUndefined(); + expect(request.memo.createTime).toBeUndefined(); + }); + + it("keeps the explicitly chosen modification time when content also changes", async () => { + const memo = renderEditor(); + const input = screen.getAllByRole("textbox")[1]; + fireEvent.change(input, { target: { value: "2024-02-29 12:34:56" } }); + // Saving must see the edit even if the input has not blurred yet. + await memoService.save({ ...editor.getState(), content: "Changed content" }, { memoName: memo.name }); + const request = clients.updateMemo.mock.calls[0][0]; + expect(request.updateMask.paths).toEqual(["content", "update_time"]); + expect(request.memo.updateTime).toEqual(timestampFromDate(customTime)); + }); +});